Add RFC for improving SDK credential caching through type safety #1842
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
rcoh
approved these changes
Nov 3, 2022
Place the selected/recommended option at the top of the document so that the unselected options don't need to be read to understand the design.
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
Velfi
reviewed
Nov 16, 2022
Velfi
approved these changes
Nov 16, 2022
ysaito1001
reviewed
Dec 2, 2022
ysaito1001
reviewed
Dec 2, 2022
ysaito1001
reviewed
Dec 2, 2022
ysaito1001
reviewed
Dec 2, 2022
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
2 tasks
ysaito1001
added a commit
that referenced
this pull request
Dec 21, 2022
This commit introduces the following items in the `aws-credential-types` crate: * CredentialsCache * ProvideCachedCredentials `CredentialsCache` is a struct a user will be interacting with when creating a credentials cache; the user no longer creates a concrete credentials cache directly, and instead it is taken care of by `CredentialsCache` behind the scene. `ProvideCachedCredentials` is a trait that will be implemented by concrete credentials caches. Furthermore, this commit renames the following structs according to the RFC #1842: * SharedCredentialsProvider -> SharedCredentialsCache * LazyCachingCredentialsProvider -> LazyCredentialsCache
3 tasks
ysaito1001
approved these changes
Dec 21, 2022
|
A new generated diff is ready to view.
A new doc preview is ready to view. |
Merged
ysaito1001
added a commit
that referenced
this pull request
Jan 17, 2023
* Add the `aws-credential-types` crate This commit adds a new crate `aws-credential-types` to the `rust-runtime` workspace. This lays the groundwork for being able to create a `LazyCachingCredentialsProvider` outside the `aws-config` crate according to the proposed solution in #2082. We have moved the following into this new crate: - Items in aws_types::credentials and and their dependencies - Items in aws_config::meta::credentials and their dependencies Finally, the crate comes with auxiliary files that are present in the other crates in the `rust-runtime` workspace such as `external-types.toml`. * Make `aws-types` depend on `aws-credential-types` The credentials module has been moved from the `aws-types` crate to the `aws-credential-types` crate. This leads to some of the items in the `aws-types` crate adjusting their use statements to point to `aws-credential-types`. The `TimeSource` struct has also been moved to `aws-credential-types` because it is used by `LazyCachingCredentialsProvider`. We have decided to move it instead of duplicating it because `aws-config` was creating a `TimeSource` from `aws-types` and then passing it to the builder for `LazyCachingCredentialsProvider`. If we had duplicated the implementation of `TimeSource` in `aws-credential-types`, two `TimeSource` implementations would have been considered different types and the said use case in `aws-config` would have been broken. * Make `aws-config` depend on `aws-credential-types` The `cache` module and modules in `meta::credentials` (except for `chain`) have been moved to `aws-credential-types`. Again, the goal of restructuring is to allow `LazyCachingCredentialsProvider` to be created outside the `aws-config` crate. While doing so, we try not moving all the default credential provider implementations. * Make `aws-http` depend on `aws-credential-types` This commit adjusts the use statements for the items that have been moved from the `aws-types` crate to the `aws-credential-types` crate. * Make `aws-inlineable` depend on `aws-credential-types` This commit adjusts the use statements for the items that have been moved from the `aws-types` crate to the `aws-credential-types` crate. * Make `aws-sig-auth` depend on `aws-credential-types` This commit adjusts the use statements for the items that have been moved from the `aws-types` crate to the `aws-credential-types` crate. * Emit `aws-credential-types` to the build directory This commit adds `aws-credential-types` to AWS_SDK_RUNTIME so that the build command `/gradlew :aws:sdk:assemble` can generate the crate into sdk/build/aws-sdk. * Make codegen aware of `aws-credential-types` This commit allows the codegen to handle the `aws-credential-types` crate. The items that have been moved from `aws-types` should now be prefixed with `aws-credential-types` when generating fully qualified names. * Make `dynamo-tests` depend on `aws-credential-types` This commit adjusts the use statements for the items that have been moved from the `aws-types` crate to the `aws-credential-types` crate. * Make `s3-tests` depend on `aws-credential-types` This commit adjusts the use statements for the items that have been moved from the `aws-types` crate to the `aws-credential-types` crate. * Make `s3control` depend on `aws-credential-types` This commit adjusts the use statements for the items that have been moved from the `aws-types` crate to the `aws-credential-types` crate. * Update external-types.xml in rust-runtime crates This commit fixes CI failures related to `cargo check-external-types` in ec994be. * Update the file permission on additional-ci * Remove unused dependency from aws-credential-types * Clean up features for aws-credential-types This commit fixes a CI failure where the feature hardcoded-credentials needed other features, aws-smithy-async/rt-tokio and tokio/rt, for the test code to compile with --no-default-features. * Update sdk-external-types.toml * Update aws/rust-runtime/aws-credential-types/Cargo.toml Co-authored-by: John DiSanti <jdisanti@amazon.com> * Update aws/rust-runtime/aws-credential-types/README.md Co-authored-by: John DiSanti <jdisanti@amazon.com> * Update aws/rust-runtime/aws-credential-types/README.md Co-authored-by: John DiSanti <jdisanti@amazon.com> * Update aws/rust-runtime/aws-credential-types/additional-ci Co-authored-by: John DiSanti <jdisanti@amazon.com> * Update aws/rust-runtime/aws-credential-types/src/lib.rs Co-authored-by: John DiSanti <jdisanti@amazon.com> * Reduce re-exports from `aws-credential-types` This commit reduces the number of re-exports from `aws-credential-types`. The rationale here is that if we add more items to this crate later on, we may get some name collisions in root. Since this crate is not used by our customers directly, it is acceptable for items to take a bit of typing to get to. * Fix broken intra doc link This commit fixes a broken intra doc link that went unnoticed because the offending link was behind the feature `hardcoded-credentials`. * Introduce type and trait for credential caching This commit introduces the following items in the `aws-credential-types` crate: * CredentialsCache * ProvideCachedCredentials `CredentialsCache` is a struct a user will be interacting with when creating a credentials cache; the user no longer creates a concrete credentials cache directly, and instead it is taken care of by `CredentialsCache` behind the scene. `ProvideCachedCredentials` is a trait that will be implemented by concrete credentials caches. Furthermore, this commit renames the following structs according to the RFC #1842: * SharedCredentialsProvider -> SharedCredentialsCache * LazyCachingCredentialsProvider -> LazyCredentialsCache * Add `credentials_cache` to `SdkConfig` and to builder This commit adds a new field `credentials_cache` to `SdkConfig`. It also adds a new method `credentials_cache` to `sdk_config::Builder`. They will help a `CredentialsCache` be threaded through from `ConfigLoader` to a service specific client's `config::Builder`, which will be implemented in a subsequent commit. * Put `SharedCredentialsCache` into the property bag This commit updates what goes into the property bag. Now that `SharedCredentialsProvider` has been renamed to `SharedCredentialsCache`, that's what goes into the property bag. Once a `SharedCredentialsCache` is retrieved from the bag, credentials can be obtained by calling `provide_cached_credentials`. * Thread through `credentials_cache` to service client This commit threads through `credentials_cache` to service client's `Config` and its builder. The builder will be the single-sourced place for creating a credentials cache. * Update `aws-config` to use `CredentialsCache` This commit updates `aws-config` to use `CredentialsCache`. Specifically, * `ConfigLoader` now has `credentials_cache` to take `CredentialsCache` * No more `LazyCachingCredentialsProvider` in `DefaultCredentialsChain` * No more `LazyCachingCredentialsProvider` in `AssumeRoleProvider` The second and third bullet points are a result of a credentials cache being composed in a service client's `config::Builder` rather than `DefaultCredentialsChain` or `AssumeRoleProvider` holding it as its field. * Update sdk integration tests This commit bulk updates the integration tests for SDK. Most updates replace the previous `SharedCredentialsProvider::new` with `Arc::new`. A more subtle but important change is to respect the `sleep_impl` field within the build method of a Config builder, making sure to thread it a default `LazyCredentialsCache` created within the build method. If we missed this step, the default constructed `LazyCredentialsCache` would later use the default Tokio sleep impl even during tests that exercise different async runtime, causing them to fail. * Update aws/rust-runtime/aws-credential-types/README.md Co-authored-by: Zelda Hessler <zhessler@amazon.com> * Rename variants of `aws_credential_types::time_source::Inner` This commit addresses #2108 (comment) * Split the unit test for `time_source` into two This commit addresses #2108 (comment) * Update CHANGELOG.next.toml * Fix test failures in CI coming from `aws-inlineable` This commit fixes test failures in CI coming from the integration test in the `aws-inlineable` crate. Commit ea47572 should have included this change. * Update external-types TOML files * Clean up offending use statements left after merging main * Remove moved module wrongly brought in after merging main * Remove `credentials_cache` from `Builder` for `DefaultCredentialsChain` This commit removes a field `credentials_cache` from the `Builder` for `DefaultCredentialsChain` as it no longer stores `LazyCredentialsCache`. Furthermore, we have also removed methods on the builder that referred to the field `credentials_cache`. After this commit, certain use cases will be broken, i.e. when a user sets timeout for loading credentials via `load_timeout` on the builder, the configured timeout will be dropped on the floor because it will not be threaded through the field `credentials_cache`. We will later provide instructions for how to update those use cases with our new set of APIs. * Remove `configure` from `LazyCredentialsCache` builder This commit removes the `configure` method from the builder for `LazyCredentialsCache`. We tried our best to keep it when we had moved the builder from `aws-config` but had to modify the method signature to destructure `ProviderCondig` to obtain the two fields out of it (`ProviderConfig` lives in `aws-config` so cannot be passed to the builder for `LazyCredentialsCache` which lives in `aws-credential-types`). Given `configure` is technically meant for credentials providers, which `LazyCredentialsCache` is not anymore, we might as well remove it and accomplish the same effect by having customers use both `time_source` and `set_sleep` on the builder instead. * Update CHANGELOG.next.toml * Update CHANGELOG.next.toml * Use unwrap_or_else to simplify two assignments This commit addresses #2122 (comment) * Add doc link to `CredentialsCache` to builder method This commit addresses #2122 (comment). In addition, it moves rustdoc for `LazyCredentialsCache` to `LazyBuilder` as `LazyCredentialsCache` has been made `pub(crate)` from `pub` and `LazyBuilder` is now a `pub` item instead. * Make `CredentialsCache` configurable in `AssumeRoleProviderBuilder` This commit addresses #2122 (comment). It allows users to pass their `CredentialsCache` to the builder just as they do in `SdkConfig`. * Update CHANGELOG.next.toml This commit addresses #2122 (comment). Co-authored-by: Yuki Saito <awsaito@amazon.com> Co-authored-by: John DiSanti <jdisanti@amazon.com> Co-authored-by: Zelda Hessler <zhessler@amazon.com>
aws-sdk-rust-ci
pushed a commit
to awslabs/aws-sdk-rust
that referenced
this pull request
Jan 26, 2023
* Add the `aws-credential-types` crate This commit adds a new crate `aws-credential-types` to the `rust-runtime` workspace. This lays the groundwork for being able to create a `LazyCachingCredentialsProvider` outside the `aws-config` crate according to the proposed solution in smithy-lang/smithy-rs#2082. We have moved the following into this new crate: - Items in aws_types::credentials and and their dependencies - Items in aws_config::meta::credentials and their dependencies Finally, the crate comes with auxiliary files that are present in the other crates in the `rust-runtime` workspace such as `external-types.toml`. * Make `aws-types` depend on `aws-credential-types` The credentials module has been moved from the `aws-types` crate to the `aws-credential-types` crate. This leads to some of the items in the `aws-types` crate adjusting their use statements to point to `aws-credential-types`. The `TimeSource` struct has also been moved to `aws-credential-types` because it is used by `LazyCachingCredentialsProvider`. We have decided to move it instead of duplicating it because `aws-config` was creating a `TimeSource` from `aws-types` and then passing it to the builder for `LazyCachingCredentialsProvider`. If we had duplicated the implementation of `TimeSource` in `aws-credential-types`, two `TimeSource` implementations would have been considered different types and the said use case in `aws-config` would have been broken. * Make `aws-config` depend on `aws-credential-types` The `cache` module and modules in `meta::credentials` (except for `chain`) have been moved to `aws-credential-types`. Again, the goal of restructuring is to allow `LazyCachingCredentialsProvider` to be created outside the `aws-config` crate. While doing so, we try not moving all the default credential provider implementations. * Make `aws-http` depend on `aws-credential-types` This commit adjusts the use statements for the items that have been moved from the `aws-types` crate to the `aws-credential-types` crate. * Make `aws-inlineable` depend on `aws-credential-types` This commit adjusts the use statements for the items that have been moved from the `aws-types` crate to the `aws-credential-types` crate. * Make `aws-sig-auth` depend on `aws-credential-types` This commit adjusts the use statements for the items that have been moved from the `aws-types` crate to the `aws-credential-types` crate. * Emit `aws-credential-types` to the build directory This commit adds `aws-credential-types` to AWS_SDK_RUNTIME so that the build command `/gradlew :aws:sdk:assemble` can generate the crate into sdk/build/aws-sdk. * Make codegen aware of `aws-credential-types` This commit allows the codegen to handle the `aws-credential-types` crate. The items that have been moved from `aws-types` should now be prefixed with `aws-credential-types` when generating fully qualified names. * Make `dynamo-tests` depend on `aws-credential-types` This commit adjusts the use statements for the items that have been moved from the `aws-types` crate to the `aws-credential-types` crate. * Make `s3-tests` depend on `aws-credential-types` This commit adjusts the use statements for the items that have been moved from the `aws-types` crate to the `aws-credential-types` crate. * Make `s3control` depend on `aws-credential-types` This commit adjusts the use statements for the items that have been moved from the `aws-types` crate to the `aws-credential-types` crate. * Update external-types.xml in rust-runtime crates This commit fixes CI failures related to `cargo check-external-types` in ec994be. * Update the file permission on additional-ci * Remove unused dependency from aws-credential-types * Clean up features for aws-credential-types This commit fixes a CI failure where the feature hardcoded-credentials needed other features, aws-smithy-async/rt-tokio and tokio/rt, for the test code to compile with --no-default-features. * Update sdk-external-types.toml * Update aws/rust-runtime/aws-credential-types/Cargo.toml Co-authored-by: John DiSanti <jdisanti@amazon.com> * Update aws/rust-runtime/aws-credential-types/README.md Co-authored-by: John DiSanti <jdisanti@amazon.com> * Update aws/rust-runtime/aws-credential-types/README.md Co-authored-by: John DiSanti <jdisanti@amazon.com> * Update aws/rust-runtime/aws-credential-types/additional-ci Co-authored-by: John DiSanti <jdisanti@amazon.com> * Update aws/rust-runtime/aws-credential-types/src/lib.rs Co-authored-by: John DiSanti <jdisanti@amazon.com> * Reduce re-exports from `aws-credential-types` This commit reduces the number of re-exports from `aws-credential-types`. The rationale here is that if we add more items to this crate later on, we may get some name collisions in root. Since this crate is not used by our customers directly, it is acceptable for items to take a bit of typing to get to. * Fix broken intra doc link This commit fixes a broken intra doc link that went unnoticed because the offending link was behind the feature `hardcoded-credentials`. * Introduce type and trait for credential caching This commit introduces the following items in the `aws-credential-types` crate: * CredentialsCache * ProvideCachedCredentials `CredentialsCache` is a struct a user will be interacting with when creating a credentials cache; the user no longer creates a concrete credentials cache directly, and instead it is taken care of by `CredentialsCache` behind the scene. `ProvideCachedCredentials` is a trait that will be implemented by concrete credentials caches. Furthermore, this commit renames the following structs according to the RFC smithy-lang/smithy-rs#1842: * SharedCredentialsProvider -> SharedCredentialsCache * LazyCachingCredentialsProvider -> LazyCredentialsCache * Add `credentials_cache` to `SdkConfig` and to builder This commit adds a new field `credentials_cache` to `SdkConfig`. It also adds a new method `credentials_cache` to `sdk_config::Builder`. They will help a `CredentialsCache` be threaded through from `ConfigLoader` to a service specific client's `config::Builder`, which will be implemented in a subsequent commit. * Put `SharedCredentialsCache` into the property bag This commit updates what goes into the property bag. Now that `SharedCredentialsProvider` has been renamed to `SharedCredentialsCache`, that's what goes into the property bag. Once a `SharedCredentialsCache` is retrieved from the bag, credentials can be obtained by calling `provide_cached_credentials`. * Thread through `credentials_cache` to service client This commit threads through `credentials_cache` to service client's `Config` and its builder. The builder will be the single-sourced place for creating a credentials cache. * Update `aws-config` to use `CredentialsCache` This commit updates `aws-config` to use `CredentialsCache`. Specifically, * `ConfigLoader` now has `credentials_cache` to take `CredentialsCache` * No more `LazyCachingCredentialsProvider` in `DefaultCredentialsChain` * No more `LazyCachingCredentialsProvider` in `AssumeRoleProvider` The second and third bullet points are a result of a credentials cache being composed in a service client's `config::Builder` rather than `DefaultCredentialsChain` or `AssumeRoleProvider` holding it as its field. * Update sdk integration tests This commit bulk updates the integration tests for SDK. Most updates replace the previous `SharedCredentialsProvider::new` with `Arc::new`. A more subtle but important change is to respect the `sleep_impl` field within the build method of a Config builder, making sure to thread it a default `LazyCredentialsCache` created within the build method. If we missed this step, the default constructed `LazyCredentialsCache` would later use the default Tokio sleep impl even during tests that exercise different async runtime, causing them to fail. * Update aws/rust-runtime/aws-credential-types/README.md Co-authored-by: Zelda Hessler <zhessler@amazon.com> * Rename variants of `aws_credential_types::time_source::Inner` This commit addresses smithy-lang/smithy-rs#2108 (comment) * Split the unit test for `time_source` into two This commit addresses smithy-lang/smithy-rs#2108 (comment) * Update CHANGELOG.next.toml * Fix test failures in CI coming from `aws-inlineable` This commit fixes test failures in CI coming from the integration test in the `aws-inlineable` crate. Commit ea47572 should have included this change. * Update external-types TOML files * Clean up offending use statements left after merging main * Remove moved module wrongly brought in after merging main * Remove `credentials_cache` from `Builder` for `DefaultCredentialsChain` This commit removes a field `credentials_cache` from the `Builder` for `DefaultCredentialsChain` as it no longer stores `LazyCredentialsCache`. Furthermore, we have also removed methods on the builder that referred to the field `credentials_cache`. After this commit, certain use cases will be broken, i.e. when a user sets timeout for loading credentials via `load_timeout` on the builder, the configured timeout will be dropped on the floor because it will not be threaded through the field `credentials_cache`. We will later provide instructions for how to update those use cases with our new set of APIs. * Remove `configure` from `LazyCredentialsCache` builder This commit removes the `configure` method from the builder for `LazyCredentialsCache`. We tried our best to keep it when we had moved the builder from `aws-config` but had to modify the method signature to destructure `ProviderCondig` to obtain the two fields out of it (`ProviderConfig` lives in `aws-config` so cannot be passed to the builder for `LazyCredentialsCache` which lives in `aws-credential-types`). Given `configure` is technically meant for credentials providers, which `LazyCredentialsCache` is not anymore, we might as well remove it and accomplish the same effect by having customers use both `time_source` and `set_sleep` on the builder instead. * Update CHANGELOG.next.toml * Update CHANGELOG.next.toml * Use unwrap_or_else to simplify two assignments This commit addresses smithy-lang/smithy-rs#2122 (comment) * Add doc link to `CredentialsCache` to builder method This commit addresses smithy-lang/smithy-rs#2122 (comment). In addition, it moves rustdoc for `LazyCredentialsCache` to `LazyBuilder` as `LazyCredentialsCache` has been made `pub(crate)` from `pub` and `LazyBuilder` is now a `pub` item instead. * Make `CredentialsCache` configurable in `AssumeRoleProviderBuilder` This commit addresses smithy-lang/smithy-rs#2122 (comment). It allows users to pass their `CredentialsCache` to the builder just as they do in `SdkConfig`. * Update CHANGELOG.next.toml This commit addresses smithy-lang/smithy-rs#2122 (comment). Co-authored-by: Yuki Saito <awsaito@amazon.com> Co-authored-by: John DiSanti <jdisanti@amazon.com> Co-authored-by: Zelda Hessler <zhessler@amazon.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation and Context
This PR adds an RFC that outlines some possible solutions to awslabs/aws-sdk-rust#629 and recommends we move forward with one of them.
Rendered
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.