Replace credentials cache with identity cache #3077

jdisanti · 2023-10-18T01:05:50Z

This PR replaces the credentials cache with the new identity cache, and adds config validation via the SharedConfigValidator runtime component and ValidateConfig trait.

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 · 2023-10-18T01:23:40Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
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 · 2023-10-18T21:25:15Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
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.

…-cache

github-actions · 2023-10-18T21:49:45Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
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 · 2023-10-18T22:47:54Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
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.

…-cache

github-actions · 2023-10-20T01:58:30Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
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.

…-cache

github-actions · 2023-10-20T16:51:00Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
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.

ysaito1001

Looks fantastic! With this, have we already addressed the known credentials_cache/credentials_provider duality issue?

ysaito1001 · 2023-10-20T16:24:05Z

aws/rust-runtime/aws-config/src/lib.rs

@@ -827,35 +834,5 @@ mod loader {
            let num_requests = num_requests.load(Ordering::Relaxed);
            assert!(num_requests > 0, "{}", num_requests);
        }
-
-        #[tokio::test]
-        async fn time_source_is_passed() {


Is this test no longer applicable or did it not add much value in the first place?

The time source and sleep aren't stored in the cache anymore, but rather, are passed in on every cache request. So there's nothing to test here.

ysaito1001 · 2023-10-20T16:49:12Z

rust-runtime/aws-smithy-runtime-api/src/client/runtime_components.rs

+///
+/// This trait can be used to validate that certain required components or config values
+/// are available, and provide an error with helpful instructions if they are not.
+pub trait ValidateConfig: fmt::Debug + Send + Sync {


This is great. Love the idea of having a centralized place for validating config 🎉

jdisanti · 2023-10-20T17:26:25Z

With this, have we already addressed the known credentials_cache/credentials_provider duality issue?

Yes. They're completely isolated from each other in config now.

rcoh · 2023-10-20T17:38:38Z

aws/rust-runtime/aws-config/src/imds/client/token.rs

            let token = match preloaded_token {
-                Some(token) => Ok(token),
+                Some(token) => {
+                    tracing::debug!(


we probably want this at trace

rcoh · 2023-10-20T17:45:58Z

aws/rust-runtime/aws-config/src/lib.rs

@@ -102,6 +102,12 @@ pub use aws_types::{
 /// Load default sources for all configuration with override support
 pub use loader::ConfigLoader;

+/// Types for configuring identity caching.
+pub mod identity {
+    pub use aws_smithy_runtime::client::identity::IdentityCache;


any issue with the re-export aws_smithy_runtime here?

I don't think it belongs in aws-smithy-runtime-api since that crate is intended for library authors, and concrete built-in identity cache implementations aren't needed for that. So that leaves us with two options: 1. put them in aws-smithy-runtime, or 2. create a new crate for them.

I opted to go for 1, but that probably means we have to make aws-smithy-runtime a stable crate.

rcoh · 2023-10-20T17:49:35Z

aws/rust-runtime/aws-config/src/lib.rs

-        /// Override the credentials cache used to build [`SdkConfig`](aws_types::SdkConfig).
+        /// Override the identity cache used to build [`SdkConfig`](aws_types::SdkConfig).
+        ///
+        /// The identity cache caches credentials and SSO tokens. By default, a lazy cache is used


AWS credentials?

rcoh · 2023-10-20T17:50:13Z

aws/rust-runtime/aws-config/src/lib.rs

+        ///             // change the load timeout to 10 seconds
+        ///             .load_timeout(Duration::from_secs(10))


may want to note that there may be other timeouts that could trigger

rcoh · 2023-10-20T17:50:38Z

aws/rust-runtime/aws-config/src/lib.rs

        ///     .load()
        ///     .await;
        /// # }
        /// ```
-        pub fn credentials_cache(mut self, credentials_cache: CredentialsCache) -> Self {


deprecate credentials_cache and leave a breadcrumb?

rcoh · 2023-10-20T18:03:05Z

rust-runtime/aws-smithy-runtime-api/src/client/interceptors/error.rs

@@ -44,6 +44,7 @@ pub struct InterceptorError {

 impl InterceptorError {
    interceptor_error_fn!(read_before_execution => ReadBeforeExecution (with source));
+    interceptor_error_fn!(read_after_configuration => ReadAfterConfiguration (with source));


did you still mean to add this error? I could be searching badly but I don't see it used

Nope. I meant to remove this. Good catch!

rcoh · 2023-10-20T18:04:12Z

rust-runtime/aws-smithy-runtime-api/src/client/auth.rs

@@ -187,6 +187,8 @@ impl AuthScheme for SharedAuthScheme {
    }
 }

+impl ValidateConfig for SharedAuthScheme {}


should these be delegating to their inner members? (either now or eventially)

Eventually, yes. I figure we'll add them as they're needed.

rcoh · 2023-10-20T18:06:11Z

rust-runtime/aws-smithy-runtime-api/src/client/runtime_components.rs

+
+#[derive(Clone)]
+enum ValidatorInner {
+    BaseConfigStaticFn(fn(&RuntimeComponentsBuilder, &ConfigBag) -> Result<(), BoxError>),


not sure it's worth having the two variants—I think you could turn one into the other during construction?

doesn't really matter anyway though—it's private.

I was just avoiding unnecessary allocation.

rcoh · 2023-10-20T18:06:59Z

rust-runtime/aws-smithy-runtime-api/src/client/runtime_components.rs

+    /// Creates a base client validator from a function.
+    pub fn base_client_config_fn(


example may be helpful

rcoh · 2023-10-20T18:11:40Z

rust-runtime/aws-smithy-runtime/src/client/interceptors.rs

+            tracing::trace!(concat!(
+                "running `",
+                stringify!($interceptor),
+                "` interceptors"
+            ));
            let mut result: Result<(), (&str, BoxError)> = Ok(());


if we logging here, may be nice to log how many too

We don't actually know the count until we've looped over them since we only have an iterator here.

rcoh

Look great!

github-actions · 2023-10-20T22:57:59Z

A new generated diff is ready to view.

AWS SDK (ignoring whitespace)
Client Test (ignoring whitespace)
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.

…only at operation level (#3156) ## Motivation and Context Fixes awslabs/aws-sdk-rust#901 ## Description This PR is a rework of #3021 whose fix was inadvertently discarded during #3077. The way we fix the issue is slightly different. In this PR, we add an identity resolver to runtime components within `set_credentials_provider`, instead of using `ServiceConfig.OperationConfigOverride`. ## Testing Added a Kotlin integration test to `CredentialProviderConfigTest.kt` based on the customer reported issue. ## 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._

…only at operation level (#3156) Fixes awslabs/aws-sdk-rust#901 This PR is a rework of #3021 whose fix was inadvertently discarded during #3077. The way we fix the issue is slightly different. In this PR, we add an identity resolver to runtime components within `set_credentials_provider`, instead of using `ServiceConfig.OperationConfigOverride`. Added a Kotlin integration test to `CredentialProviderConfigTest.kt` based on the customer reported issue.  - [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._

Replace credentials cache with identity cache

dd06410

Fix several CI issues

0575d20

jdisanti added 2 commits October 18, 2023 14:32

More CI fixes

f61f8d5

Merge remote-tracking branch 'origin/main' into jdisanti-use-identity…

afab3b2

…-cache

Fix

4fc2464

jdisanti added 3 commits October 19, 2023 18:30

Fix retry/timeout validation and improve identity cache validation

c02a0e6

Merge remote-tracking branch 'origin/main' into jdisanti-use-identity…

82b99f8

…-cache

Fix S3 tests

cef15d3

jdisanti marked this pull request as ready for review October 20, 2023 01:41

jdisanti requested a review from a team as a code owner October 20, 2023 01:41

jdisanti added the needs-sdk-review label Oct 20, 2023

jdisanti added 2 commits October 20, 2023 09:18

Fix deprecated import

890a173

Merge remote-tracking branch 'origin/main' into jdisanti-use-identity…

0d3ed15

…-cache

jdisanti added the breaking-change This will require a breaking change label Oct 20, 2023

ysaito1001 approved these changes Oct 20, 2023

View reviewed changes

rcoh approved these changes Oct 20, 2023

View reviewed changes

rcoh reviewed Oct 20, 2023

View reviewed changes

jdisanti added 2 commits October 20, 2023 15:19

Incorporate feedback and seal the ValidateConfig trait

7953c9c

Update the changelog

85bd11a

jdisanti requested a review from a team as a code owner October 20, 2023 22:37

jdisanti removed the needs-sdk-review label Oct 20, 2023

jdisanti added this pull request to the merge queue Oct 20, 2023

Merged via the queue into main with commit 40f4662 Oct 21, 2023
40 of 41 checks passed

jdisanti deleted the jdisanti-use-identity-cache branch October 21, 2023 00:02

ysaito1001 mentioned this pull request Nov 8, 2023

Ensure identity resolver is set when a credentials provider is given only at operation level #3156

Merged

1 task

NickDarvey mentioned this pull request Jun 25, 2024

chore: upgrade aws-config for aarch64-pc-windows support lancedb/lance#2510

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace credentials cache with identity cache #3077

Replace credentials cache with identity cache #3077

jdisanti commented Oct 18, 2023 •

edited

Loading

github-actions bot commented Oct 18, 2023

github-actions bot commented Oct 18, 2023

github-actions bot commented Oct 18, 2023

github-actions bot commented Oct 18, 2023

github-actions bot commented Oct 20, 2023

github-actions bot commented Oct 20, 2023

ysaito1001 left a comment

ysaito1001 Oct 20, 2023

jdisanti Oct 20, 2023

ysaito1001 Oct 20, 2023

jdisanti commented Oct 20, 2023

rcoh Oct 20, 2023

rcoh Oct 20, 2023

jdisanti Oct 20, 2023 •

edited

Loading

rcoh Oct 20, 2023

rcoh Oct 20, 2023

rcoh Oct 20, 2023

rcoh Oct 20, 2023

jdisanti Oct 20, 2023

rcoh Oct 20, 2023

jdisanti Oct 20, 2023

rcoh Oct 20, 2023

jdisanti Oct 20, 2023

rcoh Oct 20, 2023

rcoh Oct 20, 2023

jdisanti Oct 20, 2023

rcoh left a comment

github-actions bot commented Oct 20, 2023

		/// // change the load timeout to 10 seconds
		/// .load_timeout(Duration::from_secs(10))

		/// Creates a base client validator from a function.
		pub fn base_client_config_fn(

Replace credentials cache with identity cache #3077

Replace credentials cache with identity cache #3077

Conversation

jdisanti commented Oct 18, 2023 • edited Loading

github-actions bot commented Oct 18, 2023

github-actions bot commented Oct 18, 2023

github-actions bot commented Oct 18, 2023

github-actions bot commented Oct 18, 2023

github-actions bot commented Oct 20, 2023

github-actions bot commented Oct 20, 2023

ysaito1001 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

jdisanti commented Oct 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdisanti Oct 20, 2023 • 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

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

rcoh left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 20, 2023

jdisanti commented Oct 18, 2023 •

edited

Loading

jdisanti Oct 20, 2023 •

edited

Loading