Unverified Commit 40aaa1e4 authored by ysaito1001's avatar ysaito1001 Committed by GitHub
Browse files

Ensure identity resolver exists when a credentials provider is given only at...

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: default avatarJohn DiSanti <jdisanti@amazon.com>
parent cd09fd27
Loading
Loading
Loading
Loading
+6 −0
Original line number Diff line number Diff line
@@ -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"
+16 −3
Original line number Diff line number Diff line
@@ -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),
                                ),
                            );
                        }
                    }
                    """,
+13 −0
Original line number Diff line number Diff line
@@ -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
        }
+34 −54
Original line number Diff line number Diff line
@@ -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,
                    )