Connect aws-config token providers to service config via codegen #3443

jdisanti · 2024-02-27T02:02:42Z

This PR connects the token providers added in #3442 to the generated service config.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

github-actions · 2024-02-27T02:19:51Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

github-actions · 2024-02-27T19:26:38Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

github-actions · 2024-02-28T20:56:45Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

rcoh

Looks good—I had some small refactoring suggestions

rcoh · 2024-02-28T20:58:15Z

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

@@ -22,29 +24,38 @@ import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType.Companion.pre
 import software.amazon.smithy.rust.codegen.core.smithy.RustCrate
 import software.amazon.smithy.rust.codegen.core.smithy.customize.AdHocCustomization
 import software.amazon.smithy.rust.codegen.core.smithy.customize.adhocCustomization
+import software.amazon.smithy.rust.codegen.core.util.letIf

 class CredentialsProviderDecorator : ClientCodegenDecorator {


maybe we should rename this to SigV4CredentialsProviderDecorator

I'm slightly hesitant to make it more specific, but could be convinced. It applies to both SigV4 and SigV4a right now, and it could apply to more things in the future.

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

rcoh · 2024-02-28T20:59:26Z

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

+    private fun applies(codegenContext: ClientCodegenContext): Boolean =
+        ServiceIndex.of(codegenContext.model).getEffectiveAuthSchemes(codegenContext.serviceShape)
+            .containsKey(SigV4Trait.ID) || codegenContext.serviceShape.usesSigV4a()
+


these changes are to clean up support for bearer-only services?

Yeah. For Code Catalyst, it doesn't make sense to have methods to configure credential providers since it doesn't use SigV4.

Is that a breaking change?

I suppose it is in a way, in terms of compiling, except for that the service wouldn't have worked to begin with.

Edit: I'll double check that SigV4 auth didn't work with it.

Confirmed the currently released aws-sdk-codecatalyst doesn't actually work:

[src/main.rs:5] client.list_spaces().send().await = Err( DispatchFailure( DispatchFailure { source: ConnectorError { kind: Other( None, ), source: NoMatchingAuthSchemeError( ExploredList { items: [ ExploredAuthOption { scheme_id: AuthSchemeId { scheme_id: "http-bearer-auth", }, result: NoIdentityResolver, }, ], truncated: false, }, ), connection: Unknown, }, }, ), )

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

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

rcoh · 2024-02-28T21:03:23Z

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

+                    val moduleName = ctx.moduleUseName()
+                    rustTemplate(
+                        """
+                        // this should compile
+                        let _ = $moduleName::Config::builder()
+                            .token_provider(#{TestToken}::for_tests())
+                            .build();
+
+                        // this should also compile
+                        $moduleName::Config::builder()
+                            .set_token_provider(Some(#{SharedTokenProvider}::new(#{TestToken}::for_tests())));
+                        """,
+                        "TestToken" to AwsRuntimeType.awsCredentialTypesTestUtil(ctx.runtimeConfig).resolve("Token"),
+                        "SharedTokenProvider" to
+                            AwsRuntimeType.awsCredentialTypes(ctx.runtimeConfig)
+                                .resolve("provider::token::SharedTokenProvider"),
+                    )
+                }


do you want to also test that the token is actually readable back? or do we have enough other tests of that?

There's currently no way to read back the other identity resolvers such as credentials providers. Seems like we should probably address that at some point, but the difference between ProvideCredentials and ResolveIdentity makes that difficult.

I guess we could have a pub(crate) method on config to read it?

rcoh · 2024-02-28T21:05:24Z

rust-runtime/aws-smithy-runtime-api/src/client/identity/http.rs

+    pub fn for_tests() -> Self {
+        Self::new("test-token", None)
+    }


aws-config::ConfigLoader::with_test_credentials should set a token identity as well

Do you mean the test_credentials method? I do have the generated Config::with_test_defaults() method setting a token.

https://docs.rs/aws-config/latest/aws_config/struct.ConfigLoader.html#method.test_credentials

github-actions · 2024-02-29T00:50:58Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

rcoh · 2024-02-29T17:34:02Z

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

+class CredentialsProviderDecorator : ConditionalDecorator(
+    predicate = { codegenContext, _ -> codegenContext?.usesSigAuth() ?: false },
+    delegateTo =
+        object : ClientCodegenDecorator {


I wonder if we can make this slightly cleaner like:

private class Unfiltered(....) class CredentialsProviderDecorator: ConditionalDecorator(Unfiltered.Filter, Unfiltered())

might be a bit cleaner / less nesting? anyway, up to you

rcoh · 2024-02-29T17:36:18Z

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

+/**
+ * Delegating decorator that only applies when a condition is true
+ */
+open class ConditionalDecorator(


I guess this could actually live in codegen-client

rcoh · 2024-02-29T17:39:52Z

rust-runtime/aws-smithy-runtime-api/src/client/identity/http.rs

+    pub fn for_tests() -> Self {
+        Self::new("test-token", None)
+    }


https://docs.rs/aws-config/latest/aws_config/struct.ConfigLoader.html#method.test_credentials

rcoh · 2024-02-29T17:40:26Z

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

+                    val moduleName = ctx.moduleUseName()
+                    rustTemplate(
+                        """
+                        // this should compile
+                        let _ = $moduleName::Config::builder()
+                            .token_provider(#{TestToken}::for_tests())
+                            .build();
+
+                        // this should also compile
+                        $moduleName::Config::builder()
+                            .set_token_provider(Some(#{SharedTokenProvider}::new(#{TestToken}::for_tests())));
+                        """,
+                        "TestToken" to AwsRuntimeType.awsCredentialTypesTestUtil(ctx.runtimeConfig).resolve("Token"),
+                        "SharedTokenProvider" to
+                            AwsRuntimeType.awsCredentialTypes(ctx.runtimeConfig)
+                                .resolve("provider::token::SharedTokenProvider"),
+                    )
+                }


I guess we could have a pub(crate) method on config to read it?

github-actions · 2024-03-01T01:48:42Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
No codegen difference in the Client Test
No codegen difference in the Server Test
No codegen difference in the Server Test Python
No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

This PR adds support for SSO bearer token authentication to the AWS SDK, specifically for Code Catalyst, which requires authentication via SSO with a Builder ID using a bearer token rather than SigV4. This functionality was developed in a feature branch, and this PR merely merges that branch to main. The changes consist of the following previous PRs: - #3381 - #3442 - #3443 All these changes have been reviewed in the previous PRs, but it would be good to review this again as a whole to verify it all looks good. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._

jdisanti marked this pull request as ready for review February 27, 2024 19:08

jdisanti requested a review from a team as a code owner February 27, 2024 19:08

jdisanti added the needs-sdk-review label Feb 27, 2024

Base automatically changed from jdisanti-default-token-provider to feature-sso-token-providers February 28, 2024 20:34

jdisanti added 2 commits February 28, 2024 12:37

Connect token providers via codegen

98cb375

Minor fixes

48d3ea4

jdisanti force-pushed the jdisanti-wire-up-token-providers branch from 974a8d2 to 48d3ea4 Compare February 28, 2024 20:37

rcoh approved these changes Feb 28, 2024

View reviewed changes

jdisanti added 4 commits February 28, 2024 14:59

Make the ServiceSpecificDecorator more generic

e7cf976

Use ConditionalDecorator

374b39f

Refactor sig auth detection

51c925b

Use IntoShared

e7bfbce

rcoh approved these changes Feb 29, 2024

View reviewed changes

jdisanti added 3 commits February 29, 2024 16:53

Move ConditionalDecorator into codegen-client

10105e9

Set test token in ConfigLoader::test_credentials

f742a35

Improve doc comments on token provider config methods

3d924d6

jdisanti merged commit b49b88d into feature-sso-token-providers Mar 1, 2024
40 checks passed

jdisanti deleted the jdisanti-wire-up-token-providers branch March 1, 2024 01:54

jdisanti mentioned this pull request Mar 1, 2024

Add support for SSO bearer token authentication to the SDK #3453

Merged

Velfi removed the needs-sdk-review label May 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Connect aws-config token providers to service config via codegen #3443

Connect aws-config token providers to service config via codegen #3443

jdisanti commented Feb 27, 2024

github-actions bot commented Feb 27, 2024

github-actions bot commented Feb 27, 2024

github-actions bot commented Feb 28, 2024

rcoh left a comment

rcoh Feb 28, 2024

jdisanti Feb 28, 2024

rcoh Feb 28, 2024

jdisanti Feb 28, 2024

rcoh Feb 28, 2024

jdisanti Feb 28, 2024 •

edited

Loading

jdisanti Feb 29, 2024

rcoh Feb 28, 2024

jdisanti Feb 29, 2024

rcoh Feb 29, 2024

rcoh Feb 28, 2024

jdisanti Feb 29, 2024

rcoh Feb 29, 2024

github-actions bot commented Feb 29, 2024

rcoh Feb 29, 2024

rcoh Feb 29, 2024

rcoh Feb 29, 2024

rcoh Feb 29, 2024

github-actions bot commented Mar 1, 2024

Connect aws-config token providers to service config via codegen #3443

Connect aws-config token providers to service config via codegen #3443

Conversation

jdisanti commented Feb 27, 2024

github-actions bot commented Feb 27, 2024

github-actions bot commented Feb 27, 2024

github-actions bot commented Feb 28, 2024

rcoh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdisanti Feb 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Feb 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 1, 2024

jdisanti Feb 28, 2024 •

edited

Loading