From 40aaa1e4ab025c88ac3954c971390edf6f645b6e Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Tue, 3 Oct 2023 22:44:54 -0500 Subject: [PATCH] Ensure identity resolver exists when a credentials provider is given only at operation level (#3021) ## Motivation and Context Fixes https://github.com/awslabs/aws-sdk-rust/issues/901 ## Description When a credentials provider is specified _only_ at the operation level (but not at the service config level), the code in the above PR fails on request dispatch, saying `NoMatchingAuthScheme`. This occurs today because if we [do not set a credentials provider at the service config level](https://github.com/awslabs/aws-sdk-rust/blob/main/sdk/kms/src/config.rs#L757-L769), we will [not set the identity resolver for sigv4](https://github.com/awslabs/aws-sdk-rust/blob/main/sdk/kms/src/config.rs#L811-L818). The same goes for configuring a `SigningRegion` when it is only supplied at the operation level. This PR fixes the said issue so that `config_override` sets - the identity resolver for sigv4 when a credentials provider is supplied only at the operation config level - a `SigningRegion` when a `Region` is given only at the operation level ## Testing Added a Kotlin integ test `test_specifying_credentials_provider_only_at_operation_level_should_work` based on the customer reported PR. ## Checklist - [x] I have updated `CHANGELOG.next.toml` if I made changes to the AWS SDK, generated SDK code, or SDK runtime crates ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --------- Co-authored-by: John DiSanti --- CHANGELOG.next.toml | 6 ++ .../amazon/smithy/rustsdk/CredentialCaches.kt | 19 +++- .../smithy/rustsdk/SigV4SigningDecorator.kt | 13 +++ .../rustsdk/CredentialCacheConfigTest.kt | 88 +++++++------------ 4 files changed, 69 insertions(+), 57 deletions(-) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 75711a0c91..f04f291ca1 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -46,3 +46,9 @@ message = "Fix code generation for union members with the `@httpPayload` trait." references = ["smithy-rs#2969", "smithy-rs#1896"] meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "all" } author = "jdisanti" + +[[aws-sdk-rust]] +message = "Fix exclusively setting the credentials provider and/or region at operation config-override time. It's now possible to set the region or credentials when an operation is sent (via `.config_override()`), rather than at client-creation time." +references = ["smithy-rs#3021", "aws-sdk-rust#901"] +meta = { "breaking" = false, "tada" = false, "bug" = true } +author = "ysaito1001" diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/CredentialCaches.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/CredentialCaches.kt index c25001cb30..5678dcc7cb 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/CredentialCaches.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/CredentialCaches.kt @@ -57,10 +57,17 @@ class CredentialCacheConfig(codegenContext: ClientCodegenContext) : ConfigCustom private val codegenScope = arrayOf( *preludeScope, "CredentialsCache" to AwsRuntimeType.awsCredentialTypes(runtimeConfig).resolve("cache::CredentialsCache"), + "CredentialsIdentityResolver" to AwsRuntimeType.awsRuntime(runtimeConfig) + .resolve("identity::credentials::CredentialsIdentityResolver"), "DefaultProvider" to defaultProvider(), + "SIGV4_SCHEME_ID" to AwsRuntimeType.awsRuntime(runtimeConfig).resolve("auth::sigv4::SCHEME_ID"), "SharedAsyncSleep" to RuntimeType.smithyAsync(runtimeConfig).resolve("rt::sleep::SharedAsyncSleep"), - "SharedCredentialsCache" to AwsRuntimeType.awsCredentialTypes(runtimeConfig).resolve("cache::SharedCredentialsCache"), - "SharedCredentialsProvider" to AwsRuntimeType.awsCredentialTypes(runtimeConfig).resolve("provider::SharedCredentialsProvider"), + "SharedCredentialsCache" to AwsRuntimeType.awsCredentialTypes(runtimeConfig) + .resolve("cache::SharedCredentialsCache"), + "SharedCredentialsProvider" to AwsRuntimeType.awsCredentialTypes(runtimeConfig) + .resolve("provider::SharedCredentialsProvider"), + "SharedIdentityResolver" to RuntimeType.smithyRuntimeApi(runtimeConfig) + .resolve("client::identity::SharedIdentityResolver"), ) override fun section(section: ServiceConfig) = writable { @@ -209,7 +216,13 @@ class CredentialCacheConfig(codegenContext: ClientCodegenContext) : ConfigCustom #{Some}(credentials_cache), #{Some}(credentials_provider), ) => { - resolver.config_mut().store_put(credentials_cache.create_cache(credentials_provider)); + let credentials_cache = credentials_cache.create_cache(credentials_provider); + resolver.runtime_components_mut().push_identity_resolver( + #{SIGV4_SCHEME_ID}, + #{SharedIdentityResolver}::new( + #{CredentialsIdentityResolver}::new(credentials_cache), + ), + ); } } """, diff --git a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/SigV4SigningDecorator.kt b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/SigV4SigningDecorator.kt index c7ba6f9c60..3487530b78 100644 --- a/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/SigV4SigningDecorator.kt +++ b/aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/SigV4SigningDecorator.kt @@ -120,6 +120,19 @@ class SigV4SigningConfig( ) } } + is ServiceConfig.OperationConfigOverride -> { + if (runtimeMode.generateOrchestrator) { + rustTemplate( + """ + resolver.config_mut() + .load::<#{Region}>() + .cloned() + .map(|r| resolver.config_mut().store_put(#{SigningRegion}::from(r))); + """, + *codegenScope, + ) + } + } else -> emptySection } diff --git a/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/CredentialCacheConfigTest.kt b/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/CredentialCacheConfigTest.kt index 960d3adbaf..cb2f679fb3 100644 --- a/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/CredentialCacheConfigTest.kt +++ b/aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/CredentialCacheConfigTest.kt @@ -19,10 +19,13 @@ internal class CredentialCacheConfigTest { namespace com.example use aws.protocols#awsJson1_0 use aws.api#service + use aws.auth#sigv4 use smithy.rules#endpointRuleSet @service(sdkId: "Some Value") @awsJson1_0 + @sigv4(name: "dontcare") + @auth([sigv4]) @endpointRuleSet({ "version": "1.0", "rules": [{ @@ -39,7 +42,6 @@ internal class CredentialCacheConfigTest { version: "1" } - @optionalAuth operation SayHello { input: TestInput } structure TestInput { foo: String, @@ -56,14 +58,11 @@ internal class CredentialCacheConfigTest { .resolve("Credentials"), "CredentialsCache" to AwsRuntimeType.awsCredentialTypes(runtimeConfig) .resolve("cache::CredentialsCache"), - "ProvideCachedCredentials" to AwsRuntimeType.awsCredentialTypes(runtimeConfig) - .resolve("cache::ProvideCachedCredentials"), + "Region" to AwsRuntimeType.awsTypes(runtimeConfig).resolve("region::Region"), "RuntimePlugin" to RuntimeType.smithyRuntimeApi(runtimeConfig) .resolve("client::runtime_plugin::RuntimePlugin"), "SharedCredentialsCache" to AwsRuntimeType.awsCredentialTypes(runtimeConfig) .resolve("cache::SharedCredentialsCache"), - "SharedCredentialsProvider" to AwsRuntimeType.awsCredentialTypes(runtimeConfig) - .resolve("provider::SharedCredentialsProvider"), ) rustCrate.testModule { unitTest( @@ -114,74 +113,55 @@ internal class CredentialCacheConfigTest { ) } - tokioTest("test_overriding_cache_and_provider_leads_to_shared_credentials_cache_in_layer") { + unitTest("test_not_overriding_cache_and_provider_leads_to_no_shared_credentials_cache_in_layer") { rustTemplate( """ - use #{ProvideCachedCredentials}; use #{RuntimePlugin}; - let client_config = crate::config::Config::builder() - .credentials_provider(#{Credentials}::for_tests()) - .build(); - let client_config_layer = client_config.config; - - // make sure test credentials are set in the client config level - assert_eq!(#{Credentials}::for_tests(), - client_config_layer - .load::<#{SharedCredentialsCache}>() - .unwrap() - .provide_cached_credentials() - .await - .unwrap() - ); - - let credentials = #{Credentials}::new( - "test", - "test", - #{None}, - #{None}, - "test", - ); - let config_override = crate::config::Config::builder() - .credentials_cache(#{CredentialsCache}::lazy()) - .credentials_provider(credentials.clone()); + let client_config = crate::config::Config::builder().build(); + let config_override = crate::config::Config::builder(); let sut = crate::config::ConfigOverrideRuntimePlugin::new( config_override, - client_config_layer, + client_config.config, &client_config.runtime_components, ); let sut_layer = sut.config().unwrap(); - - // make sure `.provide_cached_credentials` returns credentials set through `config_override` - assert_eq!(credentials, - sut_layer + assert!(sut_layer .load::<#{SharedCredentialsCache}>() - .unwrap() - .provide_cached_credentials() - .await - .unwrap() - ); + .is_none()); """, *codegenScope, ) } - unitTest("test_not_overriding_cache_and_provider_leads_to_no_shared_credentials_cache_in_layer") { + tokioTest("test_specifying_credentials_provider_only_at_operation_level_should_work") { + // per https://github.com/awslabs/aws-sdk-rust/issues/901 rustTemplate( """ - use #{RuntimePlugin}; - let client_config = crate::config::Config::builder().build(); - let config_override = crate::config::Config::builder(); - let sut = crate::config::ConfigOverrideRuntimePlugin::new( - config_override, - client_config.config, - &client_config.runtime_components, + let client = crate::client::Client::from_conf(client_config); + + let credentials = #{Credentials}::new( + "test", + "test", + #{None}, + #{None}, + "test", ); - let sut_layer = sut.config().unwrap(); - assert!(sut_layer - .load::<#{SharedCredentialsCache}>() - .is_none()); + let operation_config_override = crate::config::Config::builder() + .credentials_cache(#{CredentialsCache}::no_caching()) + .credentials_provider(credentials.clone()) + .region(#{Region}::new("us-west-2")); + + let _ = client + .say_hello() + .customize() + .await + .unwrap() + .config_override(operation_config_override) + .send() + .await + .expect("success"); """, *codegenScope, )