Unverified Commit 81fc83ee authored by ysaito1001's avatar ysaito1001 Committed by GitHub
Browse files

Fix `config::Builder::set_credentials_provider` to override what's previsouly set (#3278) 

## Motivation and Context
Fixes https://github.com/awslabs/aws-sdk-rust/issues/973

## Description
Prior to the PR, if a customer explicitly passed a credentials provider
to a client's config `Builder::set_credentials_provider`, what's passed
did not override a credentials provider previously set ([actual use
case](https://github.com/awslabs/aws-sdk-rust/issues/973#issuecomment-1827224049)).

While in general, we recommend customers single-source a credentials
provider through
[aws_config::ConfigLoader::credentials_provider](https://docs.rs/aws-config/1.0.1/aws_config/struct.ConfigLoader.html#method.credentials_provider),
we should eliminate the said footgun in case they directly pass a
credentials provider to a client's config `Builder`.

The PR reverts test signature updates in
https://github.com/smithy-lang/smithy-rs/pull/3156

 (in hindsight, having
to update test signatures in that PR was a sign of regression).

## Testing
Added a Kotlin test to `CredentialProviderConfigTest.kt` to verify the
fix

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [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 76ae89ae

CHANGELOG.next.toml

+19 −1
Original line number Original line Diff line number Diff line
@@ -10,3 +10,21 @@ 
# references = ["smithy-rs#920"]

# references = ["smithy-rs#920"]

# meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"}

# meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"}

# author = "rcoh"

# author = "rcoh"



[[aws-sdk-rust]]

message = "Fix `config::Builder::set_credentials_provider` to override a credentials provider previously set."

references = ["aws-sdk-rust#973", "smithy-rs#3278"]

meta = { "breaking" = false, "tada" = false, "bug" = true }

author = "ysaito1001"



[[aws-sdk-rust]]

message = "`config::Config::credentials_provider` has been broken since `release-2023-11-15` and is now marked as `deprecated` explicitly."

references = ["smithy-rs#3251", "smithy-rs#3278"]

meta = { "breaking" = false, "tada" = false, "bug" = false }

author = "ysaito1001"



[[smithy-rs]]

message = "`RuntimeComponentsBuilder::push_identity_resolver` is now deprecated since it does not replace the existing identity resolver of a given auth scheme ID. Use `RuntimeComponentsBuilder::set_identity_resolver` instead."

references = ["smithy-rs#3278"]

meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client" }

author = "ysaito1001"

aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/CredentialProviders.kt

+5 −4
Original line number Original line Diff line number Diff line
@@ -81,9 +81,10 @@ class CredentialProviderConfig(private val codegenContext: ClientCodegenContext) 
            ServiceConfig.ConfigImpl -> {

            ServiceConfig.ConfigImpl -> {

                rustTemplate(

                rustTemplate(

                    """

                    """

                    /// Returns the credentials provider for this service

                    /// This function was intended to be removed, and has been broken since release-2023-11-15 as it always returns a `None`. Do not use.

                    ##[deprecated(note = "This function was intended to be removed, and has been broken since release-2023-11-15 as it always returns a `None`. Do not use.")]

                    pub fn credentials_provider(&self) -> Option<#{SharedCredentialsProvider}> {

                    pub fn credentials_provider(&self) -> Option<#{SharedCredentialsProvider}> {

                        self.config.load::<#{SharedCredentialsProvider}>().cloned()

                        #{None}

                    }

                    }

                    """,

                    """,

                    *codegenScope,

                    *codegenScope,
@@ -118,13 +119,13 @@ class CredentialProviderConfig(private val codegenContext: ClientCodegenContext) 
                        if (codegenContext.serviceShape.supportedAuthSchemes().contains("sigv4a")) {

                        if (codegenContext.serviceShape.supportedAuthSchemes().contains("sigv4a")) {

                            featureGateBlock("sigv4a") {

                            featureGateBlock("sigv4a") {

                                rustTemplate(

                                rustTemplate(

                                    "self.runtime_components.push_identity_resolver(#{SIGV4A_SCHEME_ID}, credentials_provider.clone());",

                                    "self.runtime_components.set_identity_resolver(#{SIGV4A_SCHEME_ID}, credentials_provider.clone());",

                                    *codegenScope,

                                    *codegenScope,

                                )

                                )

                            }

                            }

                        }

                        }

                        rustTemplate(

                        rustTemplate(

                            "self.runtime_components.push_identity_resolver(#{SIGV4_SCHEME_ID}, credentials_provider);",

                            "self.runtime_components.set_identity_resolver(#{SIGV4_SCHEME_ID}, credentials_provider);",

                            *codegenScope,

                            *codegenScope,

                        )

                        )

                    }

                    }

aws/sdk-codegen/src/test/kotlin/software/amazon/smithy/rustsdk/CredentialProviderConfigTest.kt

+68 −1
Original line number Original line Diff line number Diff line
@@ -30,7 +30,7 @@ internal class CredentialProviderConfigTest { 
                    val moduleName = ctx.moduleUseName()

                    val moduleName = ctx.moduleUseName()

                    rustTemplate(

                    rustTemplate(

                        """

                        """

                        let (http_client, _rx) = #{capture_request}(None);

                        let (http_client, _rx) = #{capture_request}(#{None});

                        let client_config = $moduleName::Config::builder()

                        let client_config = $moduleName::Config::builder()

                            .http_client(http_client)

                            .http_client(http_client)

                            .build();

                            .build();
@@ -62,4 +62,71 @@ internal class CredentialProviderConfigTest { 
            }

            }

        }

        }

    }

    }



    @Test

    fun `configuring credentials provider on builder should replace what was previously set`() {

        awsSdkIntegrationTest(SdkCodegenIntegrationTest.model) { ctx, rustCrate ->

            val rc = ctx.runtimeConfig

            val codegenScope = arrayOf(

                *RuntimeType.preludeScope,

                "capture_request" to RuntimeType.captureRequest(rc),

                "Credentials" to AwsRuntimeType.awsCredentialTypesTestUtil(rc)

                    .resolve("Credentials"),

                "Region" to AwsRuntimeType.awsTypes(rc).resolve("region::Region"),

                "SdkConfig" to AwsRuntimeType.awsTypes(rc).resolve("sdk_config::SdkConfig"),

                "SharedCredentialsProvider" to AwsRuntimeType.awsCredentialTypes(rc)

                    .resolve("provider::SharedCredentialsProvider"),

            )

            rustCrate.integrationTest("credentials_provider") {

                // per https://github.com/awslabs/aws-sdk-rust/issues/973

                tokioTest("configuring_credentials_provider_on_builder_should_replace_what_was_previously_set") {

                    val moduleName = ctx.moduleUseName()

                    rustTemplate(

                        """

                        let (http_client, rx) = #{capture_request}(#{None});



                        let replace_me = #{Credentials}::new(

                            "replace_me",

                            "replace_me",

                            #{None},

                            #{None},

                            "replace_me",

                        );

                        let sdk_config = #{SdkConfig}::builder()

                            .credentials_provider(

                                #{SharedCredentialsProvider}::new(replace_me),

                            )

                            .region(#{Region}::new("us-west-2"))

                            .build();



                        let expected = #{Credentials}::new(

                            "expected_credential",

                            "expected_credential",

                            #{None},

                            #{None},

                            "expected_credential",

                        );

                        let conf = $moduleName::config::Builder::from(&sdk_config)

                            .http_client(http_client)

                            .credentials_provider(expected)

                            .build();



                        let client = $moduleName::Client::from_conf(conf);



                        let _ = client

                            .some_operation()

                            .send()

                            .await

                            .expect("success");



                        let req = rx.expect_request();

                        let auth_header = req.headers().get("AUTHORIZATION").unwrap();

                        assert!(auth_header.contains("expected_credential"), "{auth_header}");

                        """,

                        *codegenScope,

                    )

                }

            }

        }

    }

}
 
}

aws/sdk/integration-tests/kms/tests/integration.rs

+1 −1
Original line number Original line Diff line number Diff line
@@ -52,7 +52,7 @@ async fn generate_random() { 
            .header("content-type", "application/x-amz-json-1.1")

            .header("content-type", "application/x-amz-json-1.1")

            .header("x-amz-target", "TrentService.GenerateRandom")

            .header("x-amz-target", "TrentService.GenerateRandom")

            .header("content-length", "20")

            .header("content-length", "20")

            .header("authorization", "AWS4-HMAC-SHA256 Credential=ANOTREAL/20090213/us-east-1/kms/aws4_request, SignedHeaders=content-length;content-type;host;x-amz-date;x-amz-security-token;x-amz-target;x-amz-user-agent, Signature=703f72fe50c310e3ee1a7a106df947b980cb91bc8bad7a4a603b057096603aed")

            .header("authorization", "AWS4-HMAC-SHA256 Credential=ANOTREAL/20090213/us-east-1/kms/aws4_request, SignedHeaders=content-length;content-type;host;x-amz-date;x-amz-target;x-amz-user-agent, Signature=53dcf70f6f852cb576185dcabef5aaa3d068704cf1b7ea7dc644efeaa46674d7")

            .header("x-amz-date", "20090213T233130Z")

            .header("x-amz-date", "20090213T233130Z")

            .header("user-agent", "aws-sdk-rust/0.123.test os/windows/XPSP3 lang/rust/1.50.0")

            .header("user-agent", "aws-sdk-rust/0.123.test os/windows/XPSP3 lang/rust/1.50.0")

            .header("x-amz-user-agent", "aws-sdk-rust/0.123.test api/test-service/0.123 os/windows/XPSP3 lang/rust/1.50.0")

            .header("x-amz-user-agent", "aws-sdk-rust/0.123.test api/test-service/0.123 os/windows/XPSP3 lang/rust/1.50.0")

aws/sdk/integration-tests/qldbsession/tests/integration.rs

+1 −1
Original line number Original line Diff line number Diff line
@@ -20,7 +20,7 @@ async fn signv4_use_correct_service_name() { 
            .header("content-type", "application/x-amz-json-1.0")

            .header("content-type", "application/x-amz-json-1.0")

            .header("x-amz-target", "QLDBSession.SendCommand")

            .header("x-amz-target", "QLDBSession.SendCommand")

            .header("content-length", "49")

            .header("content-length", "49")

            .header("authorization", "AWS4-HMAC-SHA256 Credential=ANOTREAL/20090213/us-east-1/qldb/aws4_request, SignedHeaders=content-length;content-type;host;x-amz-date;x-amz-security-token;x-amz-target;x-amz-user-agent, Signature=e8d50282fa369adf05f33a5b32e3ce2a7582edc902312c59de311001a97426d9")

            .header("authorization", "AWS4-HMAC-SHA256 Credential=ANOTREAL/20090213/us-east-1/qldb/aws4_request, SignedHeaders=content-length;content-type;host;x-amz-date;x-amz-target;x-amz-user-agent, Signature=9a07c60550504d015fb9a2b0f1b175a4d906651f9dd4ee44bebb32a802d03815")

            // qldbsession uses the signing name 'qldb' in signature _________________________^^^^

            // qldbsession uses the signing name 'qldb' in signature _________________________^^^^

            .header("x-amz-date", "20090213T233130Z")

            .header("x-amz-date", "20090213T233130Z")

            .header("user-agent", "aws-sdk-rust/0.123.test os/windows/XPSP3 lang/rust/1.50.0")

            .header("user-agent", "aws-sdk-rust/0.123.test os/windows/XPSP3 lang/rust/1.50.0")