Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Credentials are loaded once for each client #3427

Closed
rcoh opened this issue Feb 19, 2024 · 2 comments
Closed

Credentials are loaded once for each client #3427

rcoh opened this issue Feb 19, 2024 · 2 comments
Assignees
Labels
bug Something isn't working sdk

Comments

@rcoh
Copy link
Collaborator

rcoh commented Feb 19, 2024

Each IdentityResolver that is created has its own IdentityPartitionCacheKey. This means that we hit separate caches. We need to create the identity resolver in SdkConfig as part of the call to build().

e.g:

--- a/aws/rust-runtime/aws-types/src/sdk_config.rs
+++ b/aws/rust-runtime/aws-types/src/sdk_config.rs
@@ -55,6 +55,7 @@ pub struct SdkConfig {
     app_name: Option<AppName>,
     identity_cache: Option<SharedIdentityCache>,
     credentials_provider: Option<SharedCredentialsProvider>,
+    credentials_identity_resolver: Option<SharedIdentityResolver>,
     region: Option<Region>,
     endpoint_url: Option<String>,
     retry_config: Option<RetryConfig>,

Logs:

2024-02-19T17:23:02.363590Z  INFO lazy_load_identity: aws_smithy_runtime::client::identity::cache::lazy: identity cache miss occurred; added new identity (took Ok(678.027ms)) new_expiration=SystemTime { tv_sec: 1708366982, tv_nsec: 0 } valid_for=Ok(3599.636442s) partition=IdentityCachePartition(1)
2024-02-19T17:23:03.600080Z  INFO lazy_load_identity: aws_smithy_runtime::client::identity::cache::lazy: identity cache miss occurred; added new identity (took Ok(733.768ms)) new_expiration=SystemTime { tv_sec: 1708366983, tv_nsec: 0 } valid_for=Ok(3599.399947s) partition=IdentityCachePartition(4)
@jdisanti jdisanti added bug Something isn't working sdk labels Apr 5, 2024
github-merge-queue bot pushed a commit that referenced this issue Apr 9, 2024
## Motivation and Context

Original issue: #3427

See the motivation section of the RFC.

## Description

Proposes a new RFC that changes the default behavior of `SdkConfig` and
`SharedCredentialsProvider`/`SharedTokenProvider` w.r.t caching and
cache partitions.

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
github-merge-queue bot pushed a commit that referenced this issue Apr 10, 2024
## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here -->
Original issue: #3427
RFC: #3544

## Description
<!--- Describe your changes in detail -->

Fixes the `SharedCredentialsProvider` and `SharedTokenProvider` to
re-use a consistent cache partition by default.
`SdkConfig` does not create an identity cache by default still, that
will need to be a follow on PR that gates it behind a new behavior
version. Ideally we do that with the stalled stream protection work in
#3527


## Testing
<!--- Please describe in detail how you tested your changes -->
<!--- Include details of your testing environment, and the tests you ran
to -->
<!--- see how your change affects other areas of the code, etc. -->
Added new unit and integration tests.

## 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
smithy-rs codegen or 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._
@jdisanti
Copy link
Collaborator

This was fixed in the April 12th release.

@aajtodd
Copy link
Contributor

aajtodd commented Apr 15, 2024

Well to be clear it is now possible, you still have to pass an identity_cache(..) to the config loader yourself until #3578 is merged and made the default behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working sdk
Projects
None yet
Development

No branches or pull requests

3 participants