Merge branch 'main' into merge-smithy-rs-release-1.x.y-to-main

This commit is contained in:
ysaito1001 2023-12-04 16:20:37 -06:00 committed by GitHub
commit 6f4427948c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 366 additions and 12 deletions

View File

@ -32,3 +32,32 @@ result of the compilation."""
references = ["aws-sdk-rust#975", "smithy-rs#3269"]
meta = { "breaking" = false, "tada" = true, "bug" = false }
author = "jdisanti"
[[aws-sdk-rust]]
message = """Add `test_credentials` to `ConfigLoader` in `aws_config`. This allows the following pattern during tests:
```rust
async fn main() {
let conf = aws_config::defaults(BehaviorVersion::latest())
.test_credentials()
.await;
}
```
This is designed for unit tests and using local mocks like DynamoDB Local and LocalStack with the SDK.
"""
meta = { "breaking" = false, "tada" = true, "bug" = false }
author = "rcoh"
references = ["smithy-rs#3279", "aws-sdk-rust#971"]
[[aws-sdk-rust]]
message = "Improve the error messages for when auth fails to select an auth scheme for a request."
references = ["aws-sdk-rust#979", "smithy-rs#3277"]
meta = { "breaking" = false, "tada" = false, "bug" = false }
author = "jdisanti"
[[smithy-rs]]
message = "Improve the error messages for when auth fails to select an auth scheme for a request."
references = ["smithy-rs#3277"]
meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client" }
author = "jdisanti"

View File

@ -12,15 +12,17 @@ repository = "https://github.com/smithy-lang/smithy-rs"
behavior-version-latest = []
client-hyper = ["aws-smithy-runtime/connector-hyper-0-14-x"]
rustls = ["aws-smithy-runtime/tls-rustls", "client-hyper"]
allow-compilation = [] # our tests use `cargo test --all-features` and native-tls breaks CI
rt-tokio = ["aws-smithy-async/rt-tokio", "aws-smithy-runtime/rt-tokio", "tokio/rt"]
sso = ["dep:aws-sdk-sso", "dep:aws-sdk-ssooidc", "dep:ring", "dep:hex", "dep:zeroize", "aws-smithy-runtime-api/http-auth"]
credentials-process = ["tokio/process"]
default = ["client-hyper", "rustls", "rt-tokio", "credentials-process", "sso"]
# deprecated: this feature does nothing
allow-compilation = []
[dependencies]
aws-credential-types = { path = "../../sdk/build/aws-sdk/sdk/aws-credential-types" }
aws-credential-types = { path = "../../sdk/build/aws-sdk/sdk/aws-credential-types", features = ["test-util"] }
aws-http = { path = "../../sdk/build/aws-sdk/sdk/aws-http" }
aws-sdk-sts = { path = "../../sdk/build/aws-sdk/sdk/sts", default-features = false }
aws-smithy-async = { path = "../../sdk/build/aws-sdk/sdk/aws-smithy-async" }
@ -52,7 +54,6 @@ zeroize = { version = "1", optional = true }
aws-sdk-ssooidc = { path = "../../sdk/build/aws-sdk/sdk/ssooidc", default-features = false, optional = true }
[dev-dependencies]
aws-credential-types = { path = "../../sdk/build/aws-sdk/sdk/aws-credential-types", features = ["test-util"] }
aws-smithy-runtime = { path = "../../sdk/build/aws-sdk/sdk/aws-smithy-runtime", features = ["client", "connector-hyper-0-14-x", "test-util"] }
aws-smithy-runtime-api = { path = "../../sdk/build/aws-sdk/sdk/aws-smithy-runtime-api", features = ["test-util"] }
futures-util = { version = "0.3.16", default-features = false }

View File

@ -212,6 +212,7 @@ mod loader {
use crate::profile::profile_file::ProfileFiles;
use crate::provider_config::ProviderConfig;
use aws_credential_types::provider::{ProvideCredentials, SharedCredentialsProvider};
use aws_credential_types::Credentials;
use aws_smithy_async::rt::sleep::{default_async_sleep, AsyncSleep, SharedAsyncSleep};
use aws_smithy_async::time::{SharedTimeSource, TimeSource};
use aws_smithy_runtime_api::client::behavior_version::BehaviorVersion;
@ -458,6 +459,10 @@ mod loader {
/// anonymous auth for S3, calling operations in STS that don't require a signature,
/// or using token-based auth.
///
/// **Note**: For tests, e.g. with a service like DynamoDB Local, this is **not** what you
/// want. If credentials are disabled, requests cannot be signed. For these use cases, use
/// [`test_credentials`](Self::test_credentials).
///
/// # Examples
///
/// Turn off credentials in order to call a service without signing:
@ -474,6 +479,11 @@ mod loader {
self
}
/// Set test credentials for use when signing requests
pub fn test_credentials(self) -> Self {
self.credentials_provider(Credentials::for_tests())
}
/// Override the name of the app used to build [`SdkConfig`](aws_types::SdkConfig).
///
/// This _optional_ name is used to identify the application in the user agent that

View File

@ -0,0 +1,32 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/
use aws_sdk_dynamodb::config::Region;
use aws_sdk_dynamodb::error::DisplayErrorContext;
use aws_sdk_dynamodb::{Client, Config};
use aws_smithy_runtime::assert_str_contains;
use aws_smithy_runtime::client::http::test_util::capture_request;
#[tokio::test]
async fn auth_scheme_error() {
let (http_client, _) = capture_request(None);
let config = Config::builder()
.behavior_version_latest()
.http_client(http_client)
.region(Region::new("us-west-2"))
// intentionally omitting credentials_provider
.build();
let client = Client::from_conf(config);
let err = client
.list_tables()
.send()
.await
.expect_err("there is no credential provider, so this must fail");
assert_str_contains!(
DisplayErrorContext(&err).to_string(),
"\"sigv4\" wasn't a valid option because there was no identity resolver for it. Be sure to set an identity"
);
}

View File

@ -20,18 +20,86 @@ use std::error::Error as StdError;
use std::fmt;
use tracing::trace;
#[derive(Debug)]
struct NoMatchingAuthSchemeError(ExploredList);
impl fmt::Display for NoMatchingAuthSchemeError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let explored = &self.0;
// Use the information we have about the auth options that were explored to construct
// as helpful of an error message as possible.
if explored.items().count() == 0 {
return f.write_str(
"no auth options are available. This can happen if there's \
a problem with the service model, or if there is a codegen bug.",
);
}
if explored
.items()
.all(|explored| matches!(explored.result, ExploreResult::NoAuthScheme))
{
return f.write_str(
"no auth schemes are registered. This can happen if there's \
a problem with the service model, or if there is a codegen bug.",
);
}
let mut try_add_identity = false;
let mut likely_bug = false;
f.write_str("failed to select an auth scheme to sign the request with.")?;
for item in explored.items() {
write!(
f,
" \"{}\" wasn't a valid option because ",
item.scheme_id.as_str()
)?;
f.write_str(match item.result {
ExploreResult::NoAuthScheme => {
likely_bug = true;
"no auth scheme was registered for it."
}
ExploreResult::NoIdentityResolver => {
try_add_identity = true;
"there was no identity resolver for it."
}
ExploreResult::MissingEndpointConfig => {
likely_bug = true;
"there is auth config in the endpoint config, but this scheme wasn't listed in it \
(see https://github.com/smithy-lang/smithy-rs/discussions/3281 for more details)."
}
ExploreResult::NotExplored => {
debug_assert!(false, "this should be unreachable");
"<unknown>"
}
})?;
}
if try_add_identity {
f.write_str(" Be sure to set an identity, such as credentials, auth token, or other identity type that is required for this service.")?;
} else if likely_bug {
f.write_str(" This is likely a bug.")?;
}
if explored.truncated {
f.write_str(" Note: there were other auth schemes that were evaluated that weren't listed here.")?;
}
Ok(())
}
}
impl StdError for NoMatchingAuthSchemeError {}
#[derive(Debug)]
enum AuthOrchestrationError {
NoMatchingAuthScheme,
MissingEndpointConfig,
BadAuthSchemeEndpointConfig(Cow<'static, str>),
}
impl fmt::Display for AuthOrchestrationError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::NoMatchingAuthScheme => f.write_str(
"no auth scheme matched auth scheme options. This is a bug. Please file an issue.",
),
// This error is never bubbled up
Self::MissingEndpointConfig => f.write_str("missing endpoint config"),
Self::BadAuthSchemeEndpointConfig(message) => f.write_str(message),
}
}
@ -59,6 +127,8 @@ pub(super) async fn orchestrate_auth(
"orchestrating auth",
);
let mut explored = ExploredList::default();
// Iterate over IDs of possibly-supported auth schemes
for &scheme_id in options.as_ref() {
// For each ID, try to resolve the corresponding auth scheme.
@ -95,16 +165,21 @@ pub(super) async fn orchestrate_auth(
)?;
return Ok(());
}
Err(AuthOrchestrationError::NoMatchingAuthScheme) => {
Err(AuthOrchestrationError::MissingEndpointConfig) => {
explored.push(scheme_id, ExploreResult::MissingEndpointConfig);
continue;
}
Err(other_err) => return Err(other_err.into()),
}
} else {
explored.push(scheme_id, ExploreResult::NoIdentityResolver);
}
} else {
explored.push(scheme_id, ExploreResult::NoAuthScheme);
}
}
Err(AuthOrchestrationError::NoMatchingAuthScheme.into())
Err(NoMatchingAuthSchemeError(explored).into())
}
fn extract_endpoint_auth_scheme_config(
@ -135,10 +210,66 @@ fn extract_endpoint_auth_scheme_config(
.and_then(Document::as_string);
config_scheme_id == Some(scheme_id.as_str())
})
.ok_or(AuthOrchestrationError::NoMatchingAuthScheme)?;
.ok_or(AuthOrchestrationError::MissingEndpointConfig)?;
Ok(AuthSchemeEndpointConfig::from(Some(auth_scheme_config)))
}
#[derive(Debug)]
enum ExploreResult {
NotExplored,
NoAuthScheme,
NoIdentityResolver,
MissingEndpointConfig,
}
/// Information about an evaluated auth option.
/// This should be kept small so it can fit in an array on the stack.
#[derive(Debug)]
struct ExploredAuthOption {
scheme_id: AuthSchemeId,
result: ExploreResult,
}
impl Default for ExploredAuthOption {
fn default() -> Self {
Self {
scheme_id: AuthSchemeId::new(""),
result: ExploreResult::NotExplored,
}
}
}
const MAX_EXPLORED_LIST_LEN: usize = 8;
/// Stack allocated list of explored auth options for error messaging
#[derive(Default)]
struct ExploredList {
items: [ExploredAuthOption; MAX_EXPLORED_LIST_LEN],
len: usize,
truncated: bool,
}
impl ExploredList {
fn items(&self) -> impl Iterator<Item = &ExploredAuthOption> {
self.items.iter().take(self.len)
}
fn push(&mut self, scheme_id: AuthSchemeId, result: ExploreResult) {
if self.len + 1 >= self.items.len() {
self.truncated = true;
} else {
self.items[self.len] = ExploredAuthOption { scheme_id, result };
self.len += 1;
}
}
}
impl fmt::Debug for ExploredList {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("ExploredList")
.field("items", &&self.items[0..self.len])
.field("truncated", &self.truncated)
.finish()
}
}
#[cfg(all(test, feature = "test-util"))]
mod tests {
use super::*;
@ -481,4 +612,86 @@ mod tests {
.unwrap()
);
}
#[test]
fn friendly_error_messages() {
let err = NoMatchingAuthSchemeError(ExploredList::default());
assert_eq!(
"no auth options are available. This can happen if there's a problem with \
the service model, or if there is a codegen bug.",
err.to_string()
);
let mut list = ExploredList::default();
list.push(
AuthSchemeId::new("SigV4"),
ExploreResult::NoIdentityResolver,
);
list.push(
AuthSchemeId::new("SigV4a"),
ExploreResult::NoIdentityResolver,
);
let err = NoMatchingAuthSchemeError(list);
assert_eq!(
"failed to select an auth scheme to sign the request with. \
\"SigV4\" wasn't a valid option because there was no identity resolver for it. \
\"SigV4a\" wasn't a valid option because there was no identity resolver for it. \
Be sure to set an identity, such as credentials, auth token, or other identity \
type that is required for this service.",
err.to_string()
);
// It should prioritize the suggestion to try an identity before saying it's a bug
let mut list = ExploredList::default();
list.push(
AuthSchemeId::new("SigV4"),
ExploreResult::NoIdentityResolver,
);
list.push(
AuthSchemeId::new("SigV4a"),
ExploreResult::MissingEndpointConfig,
);
let err = NoMatchingAuthSchemeError(list);
assert_eq!(
"failed to select an auth scheme to sign the request with. \
\"SigV4\" wasn't a valid option because there was no identity resolver for it. \
\"SigV4a\" wasn't a valid option because there is auth config in the endpoint \
config, but this scheme wasn't listed in it (see \
https://github.com/smithy-lang/smithy-rs/discussions/3281 for more details). \
Be sure to set an identity, such as credentials, auth token, or other identity \
type that is required for this service.",
err.to_string()
);
// Otherwise, it should suggest it's a bug
let mut list = ExploredList::default();
list.push(
AuthSchemeId::new("SigV4a"),
ExploreResult::MissingEndpointConfig,
);
let err = NoMatchingAuthSchemeError(list);
assert_eq!(
"failed to select an auth scheme to sign the request with. \
\"SigV4a\" wasn't a valid option because there is auth config in the endpoint \
config, but this scheme wasn't listed in it (see \
https://github.com/smithy-lang/smithy-rs/discussions/3281 for more details). \
This is likely a bug.",
err.to_string()
);
// Truncation should be indicated
let mut list = ExploredList::default();
for _ in 0..=MAX_EXPLORED_LIST_LEN {
list.push(
AuthSchemeId::new("dontcare"),
ExploreResult::MissingEndpointConfig,
);
}
let err = NoMatchingAuthSchemeError(list).to_string();
if !err.contains(
"Note: there were other auth schemes that were evaluated that weren't listed here",
) {
panic!("The error should indicate that the explored list was truncated.");
}
}
}

View File

@ -5,3 +5,5 @@
/// Utility for capturing and displaying logs during a unit test.
pub mod capture_test_logs;
mod assertions;

View File

@ -0,0 +1,57 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/
/// Asserts that a given string value `$str` contains a substring `$expected`.
///
/// This macro can also take a custom panic message with formatting.
#[macro_export]
macro_rules! assert_str_contains {
($str:expr, $expected:expr) => {
assert_str_contains!($str, $expected, "")
};
($str:expr, $expected:expr, $($fmt_args:tt)+) => {{
let s = $str;
let expected = $expected;
if !s.contains(&expected) {
panic!(
"assertion failed: `str.contains(expected)`\n{:>8}: {expected}\n{:>8}: {s}\n{}",
"expected",
"str",
::std::fmt::format(::std::format_args!($($fmt_args)+)),
);
}
}};
}
#[cfg(test)]
mod tests {
use std::panic::{catch_unwind, UnwindSafe};
fn expect_panic(f: impl FnOnce() -> () + UnwindSafe) -> String {
*catch_unwind(f)
.expect_err("it should fail")
.downcast::<String>()
.expect("it should be a string")
}
#[test]
fn assert_str_contains() {
assert_str_contains!("foobar", "bar");
assert_str_contains!("foobar", "o");
assert_eq!(
"assertion failed: `str.contains(expected)`\nexpected: not-in-it\n str: foobar\n",
expect_panic(|| assert_str_contains!("foobar", "not-in-it"))
);
assert_eq!(
"assertion failed: `str.contains(expected)`\nexpected: not-in-it\n str: foobar\nsome custom message",
expect_panic(|| assert_str_contains!("foobar", "not-in-it", "some custom message"))
);
assert_eq!(
"assertion failed: `str.contains(expected)`\nexpected: not-in-it\n str: foobar\nsome custom message with formatting",
expect_panic(|| assert_str_contains!("foobar", "not-in-it", "some custom message with {}", "formatting"))
);
}
}

View File

@ -35,8 +35,18 @@ cargo "+${RUST_NIGHTLY_VERSION:-nightly}" check-external-types --all-features --
echo "${C_YELLOW}## Checking for duplicate dependency versions in the normal dependency graph with all features enabled${C_RESET}"
cargo tree -d --edges normal --all-features
echo "${C_YELLOW}## Testing every combination of features${C_RESET}"
cargo hack test --feature-powerset --exclude-all-features --exclude-features native-tls
# the crate has `allow-compilation` which needs to be deprecated
echo "${C_YELLOW}## Checking every combination of features${C_RESET}"
cargo hack check --feature-powerset --exclude-all-features --exclude-features allow-compilation
echo "${C_YELLOW}## Testing each feature in isolation${C_RESET}"
# these features are missed by the following check because they are grouped
cargo hack test --each-feature --include-features rt-tokio,client-hyper,rustls --exclude-no-default-features
# This check will check other individual features and no-default-features
# grouping these features because they don't interact
cargo hack test --feature-powerset --exclude-all-features --exclude-features allow-compilation --group-features rt-tokio,client-hyper,rustls
echo "${C_YELLOW}## Checking the wasm32-unknown-unknown and wasm32-wasi targets${C_RESET}"
cargo check --target wasm32-unknown-unknown --no-default-features