From 49ecf2d25643932a63125e52277639b1213027eb Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Tue, 3 Oct 2023 12:58:59 -0500 Subject: [PATCH 1/5] Add failing test per awslabs/aws-sdk-rust#901 --- .../rustsdk/CredentialCacheConfigTest.kt | 38 ++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) 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..d6e971ebf6 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, @@ -58,6 +60,7 @@ internal class CredentialCacheConfigTest { .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) @@ -186,6 +189,39 @@ internal class CredentialCacheConfigTest { *codegenScope, ) } + + tokioTest("test_specifying_credentials_provider_only_at_operation_level_should_work") { + // per https://github.com/awslabs/aws-sdk-rust/issues/901 + rustTemplate( + """ + let client_config = crate::config::Config::builder().build(); + let client = crate::client::Client::from_conf(client_config); + + let credentials = #{Credentials}::new( + "test", + "test", + #{None}, + #{None}, + "test", + ); + 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, + ) + } } } } From 5cf2836d4d867577c5e3d80b61f779fb676f379e Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Tue, 3 Oct 2023 13:34:01 -0500 Subject: [PATCH 2/5] Ensure auth scheme exists for `config_override` --- .../amazon/smithy/rustsdk/CredentialCaches.kt | 20 ++++++++++++++++--- .../smithy/rustsdk/SigV4SigningDecorator.kt | 13 ++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) 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..ae91493bc0 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,14 @@ 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.config_mut().store_put(credentials_cache.clone()); + 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 } From 91a19cebe1df245757bf806b0601377ffcb9604a Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Tue, 3 Oct 2023 14:36:58 -0500 Subject: [PATCH 3/5] Update CHANGELOG.next.toml --- CHANGELOG.next.toml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 75711a0c91..26720ce5b9 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 operation level configuration so that it now sets a) an `IdentityResolver` when a credentials provider is provided only at the operation level and b) a `SigningRegion` when a `Region` is provided only at the operation level." +references = ["smithy-rs#3021", "aws-sdk-rust#901"] +meta = { "breaking" = false, "tada" = false, "bug" = true } +author = "ysaito1001" From e36d943b19a73044faf4d0b69d4bcbfaa2cbb41a Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Tue, 3 Oct 2023 17:18:56 -0500 Subject: [PATCH 4/5] Update CHANGELOG.next.toml Co-authored-by: John DiSanti --- CHANGELOG.next.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 26720ce5b9..f04f291ca1 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -48,7 +48,7 @@ meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "all" } author = "jdisanti" [[aws-sdk-rust]] -message = "Fix operation level configuration so that it now sets a) an `IdentityResolver` when a credentials provider is provided only at the operation level and b) a `SigningRegion` when a `Region` is provided only at the operation level." +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" From 3f095774774680a3b22e2ab443cbae4bbeab5176 Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Tue, 3 Oct 2023 17:27:10 -0500 Subject: [PATCH 5/5] Do not put the cache to config during `config_override` This commit addresses https://github.com/awslabs/smithy-rs/pull/3021#discussion_r1344720911 --- .../amazon/smithy/rustsdk/CredentialCaches.kt | 1 - .../rustsdk/CredentialCacheConfigTest.kt | 56 ------------------- 2 files changed, 57 deletions(-) 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 ae91493bc0..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 @@ -217,7 +217,6 @@ class CredentialCacheConfig(codegenContext: ClientCodegenContext) : ConfigCustom #{Some}(credentials_provider), ) => { let credentials_cache = credentials_cache.create_cache(credentials_provider); - resolver.config_mut().store_put(credentials_cache.clone()); resolver.runtime_components_mut().push_identity_resolver( #{SIGV4_SCHEME_ID}, #{SharedIdentityResolver}::new( 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 d6e971ebf6..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 @@ -58,15 +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( @@ -117,58 +113,6 @@ internal class CredentialCacheConfigTest { ) } - tokioTest("test_overriding_cache_and_provider_leads_to_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 sut = crate::config::ConfigOverrideRuntimePlugin::new( - config_override, - client_config_layer, - &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 - .load::<#{SharedCredentialsCache}>() - .unwrap() - .provide_cached_credentials() - .await - .unwrap() - ); - """, - *codegenScope, - ) - } - unitTest("test_not_overriding_cache_and_provider_leads_to_no_shared_credentials_cache_in_layer") { rustTemplate( """