Skip to content

Commit

Permalink
Add NoCredentialsCache that offers no caching ability (#2720)
Browse files Browse the repository at this point in the history
Related to awslabs/aws-sdk-rust#809
It has been discovered that when `AssumeRoleProvider` is used, the Rust
SDK emits `credentials cache miss occurred` twice per request. The
reason why that log is shown twice is illustrated in the following
diagram:

![Screenshot 2023-05-19 at 4 10 20
PM](https://github.com/awslabs/smithy-rs/assets/15333866/c6cce018-c821-4b46-8d47-b414af7b4d1e)

One of the cache miss messages is due to the fact `AssumeRoleProvider`
internally uses an STS client, which, in turn, is wrapped by a
`LazyCredentialsCache` by default. However, that use of
`LazyCredentialsCache` is pointless because caching is already in effect
with the outermost `LazyCredentialsCache`.

This PR adds a new kind of `CredentialsCache`, `NoCredentialsCache`. As
its name suggests, it simplify delegates `provide_cached_credentials` to
the underlying provider's `provide_credentials` with no caching
functionality. We then update `SsoCredentialsProvider`,
`AssumeRoleProvider`, and `WebIdentityTokenCredentialsProvider` to use
`NoCredentialsCache` for their STS clients so the logs won't show
`credentials cache miss occurred` twice per request.
- Added unit tests for `NoCredentialsCache`
- Updated unit test for `AssumeRoleProvider` to verify
`NoCredentialsCache` is used by default
- [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: Yuki Saito <[email protected]>
  • Loading branch information
2 people authored and Harry Barber committed Jun 20, 2023
1 parent e6051a8 commit f7fc8ce
Show file tree
Hide file tree
Showing 6 changed files with 157 additions and 17 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,9 @@ message = "Fix server SDK bug with directly constrained list/map shapes in opera
references = ["smithy-rs#2761"]
meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "server"}
author = "david-perez"

[[aws-sdk-rust]]
message = "`SsoCredentialsProvider`, `AssumeRoleProvider`, and `WebIdentityTokenCredentialsProvider` now use `NoCredentialsCache` internally when fetching credentials using an STS client. This avoids double-caching when these providers are wrapped by `LazyCredentialsCache` when a service client is created."
references = ["smithy-rs#2720"]
meta = { "breaking" = false, "tada" = false, "bug" = true }
author = "ysaito1001"
2 changes: 2 additions & 0 deletions aws/rust-runtime/aws-config/src/sso.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use crate::fs_util::{home_dir, Os};
use crate::json_credentials::{json_parse_loop, InvalidJsonCredentials};
use crate::provider_config::ProviderConfig;

use aws_credential_types::cache::CredentialsCache;
use aws_credential_types::provider::{self, error::CredentialsError, future, ProvideCredentials};
use aws_credential_types::Credentials;
use aws_sdk_sso::middleware::DefaultMiddleware as SsoMiddleware;
Expand Down Expand Up @@ -211,6 +212,7 @@ async fn load_sso_credentials(
.map_err(CredentialsError::provider_error)?;
let config = aws_sdk_sso::Config::builder()
.region(sso_config.region.clone())
.credentials_cache(CredentialsCache::no_caching())
.build();
let operation = GetRoleCredentialsInput::builder()
.role_name(&sso_config.role_name)
Expand Down
70 changes: 53 additions & 17 deletions aws/rust-runtime/aws-config/src/sts/assume_role.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,13 @@ impl AssumeRoleProviderBuilder {
self
}

#[deprecated(
note = "This should not be necessary as the default, no caching, is usually what you want."
)]
/// Set the [`CredentialsCache`] for credentials retrieved from STS.
///
/// By default, an [`AssumeRoleProvider`] internally uses `NoCredentialsCache` because the
/// provider itself will be wrapped by `LazyCredentialsCache` when a service client is created.
pub fn credentials_cache(mut self, cache: CredentialsCache) -> Self {
self.credentials_cache = Some(cache);
self
Expand All @@ -198,11 +204,9 @@ impl AssumeRoleProviderBuilder {
pub fn build(self, provider: impl ProvideCredentials + 'static) -> AssumeRoleProvider {
let conf = self.conf.unwrap_or_default();

let credentials_cache = self.credentials_cache.unwrap_or_else(|| {
let mut builder = CredentialsCache::lazy_builder().time_source(conf.time_source());
builder.set_sleep(conf.sleep());
builder.into_credentials_cache()
});
let credentials_cache = self
.credentials_cache
.unwrap_or_else(CredentialsCache::no_caching);

let config = aws_sdk_sts::Config::builder()
.credentials_cache(credentials_cache)
Expand Down Expand Up @@ -333,39 +337,71 @@ mod test {
}

#[tokio::test]
async fn provider_caches_credentials() {
async fn provider_does_not_cache_credentials_by_default() {
let conn = TestConnection::new(vec![
(http::Request::new(SdkBody::from("request body")),
http::Response::builder().status(200).body(SdkBody::from(
"<AssumeRoleResponse xmlns=\"https://sts.amazonaws.com/doc/2011-06-15/\">\n <AssumeRoleResult>\n <AssumedRoleUser>\n <AssumedRoleId>AROAR42TAWARILN3MNKUT:assume-role-from-profile-1632246085998</AssumedRoleId>\n <Arn>arn:aws:sts::130633740322:assumed-role/imds-chained-role-test/assume-role-from-profile-1632246085998</Arn>\n </AssumedRoleUser>\n <Credentials>\n <AccessKeyId>ASIARCORRECT</AccessKeyId>\n <SecretAccessKey>secretkeycorrect</SecretAccessKey>\n <SessionToken>tokencorrect</SessionToken>\n <Expiration>2009-02-13T23:31:30Z</Expiration>\n </Credentials>\n </AssumeRoleResult>\n <ResponseMetadata>\n <RequestId>d9d47248-fd55-4686-ad7c-0fb7cd1cddd7</RequestId>\n </ResponseMetadata>\n</AssumeRoleResponse>\n"
"<AssumeRoleResponse xmlns=\"https://sts.amazonaws.com/doc/2011-06-15/\">\n <AssumeRoleResult>\n <AssumedRoleUser>\n <AssumedRoleId>AROAR42TAWARILN3MNKUT:assume-role-from-profile-1632246085998</AssumedRoleId>\n <Arn>arn:aws:sts::130633740322:assumed-role/assume-provider-test/assume-role-from-profile-1632246085998</Arn>\n </AssumedRoleUser>\n <Credentials>\n <AccessKeyId>ASIARCORRECT</AccessKeyId>\n <SecretAccessKey>secretkeycorrect</SecretAccessKey>\n <SessionToken>tokencorrect</SessionToken>\n <Expiration>2009-02-13T23:31:30Z</Expiration>\n </Credentials>\n </AssumeRoleResult>\n <ResponseMetadata>\n <RequestId>d9d47248-fd55-4686-ad7c-0fb7cd1cddd7</RequestId>\n </ResponseMetadata>\n</AssumeRoleResponse>\n"
)).unwrap()),
(http::Request::new(SdkBody::from("request body")),
http::Response::builder().status(200).body(SdkBody::from(
"<AssumeRoleResponse xmlns=\"https://sts.amazonaws.com/doc/2011-06-15/\">\n <AssumeRoleResult>\n <AssumedRoleUser>\n <AssumedRoleId>AROAR42TAWARILN3MNKUT:assume-role-from-profile-1632246085998</AssumedRoleId>\n <Arn>arn:aws:sts::130633740322:assumed-role/imds-chained-role-test/assume-role-from-profile-1632246085998</Arn>\n </AssumedRoleUser>\n <Credentials>\n <AccessKeyId>ASIARCORRECT</AccessKeyId>\n <SecretAccessKey>secretkeycorrect</SecretAccessKey>\n <SessionToken>tokencorrect</SessionToken>\n <Expiration>2009-02-13T23:31:30Z</Expiration>\n </Credentials>\n </AssumeRoleResult>\n <ResponseMetadata>\n <RequestId>d9d47248-fd55-4686-ad7c-0fb7cd1cddd7</RequestId>\n </ResponseMetadata>\n</AssumeRoleResponse>\n"
"<AssumeRoleResponse xmlns=\"https://sts.amazonaws.com/doc/2011-06-15/\">\n <AssumeRoleResult>\n <AssumedRoleUser>\n <AssumedRoleId>AROAR42TAWARILN3MNKUT:assume-role-from-profile-1632246085998</AssumedRoleId>\n <Arn>arn:aws:sts::130633740322:assumed-role/assume-provider-test/assume-role-from-profile-1632246085998</Arn>\n </AssumedRoleUser>\n <Credentials>\n <AccessKeyId>ASIARCORRECT</AccessKeyId>\n <SecretAccessKey>TESTSECRET</SecretAccessKey>\n <SessionToken>tokencorrect</SessionToken>\n <Expiration>2009-02-13T23:33:30Z</Expiration>\n </Credentials>\n </AssumeRoleResult>\n <ResponseMetadata>\n <RequestId>c2e971c2-702d-4124-9b1f-1670febbea18</RequestId>\n </ResponseMetadata>\n</AssumeRoleResponse>\n"
)).unwrap()),
]);

let mut testing_time_source = TestingTimeSource::new(
UNIX_EPOCH + Duration::from_secs(1234567890 - 120), // 1234567890 since UNIX_EPOCH is 2009-02-13T23:31:30Z
);

let provider_conf = ProviderConfig::empty()
.with_sleep(TokioSleep::new())
.with_time_source(TimeSource::testing(&TestingTimeSource::new(
UNIX_EPOCH + Duration::from_secs(1234567890 - 120),
)))
.with_time_source(TimeSource::testing(&testing_time_source))
.with_http_connector(DynConnector::new(conn));
let credentials_list = std::sync::Arc::new(std::sync::Mutex::new(vec![
Credentials::new(
"test",
"test",
None,
Some(UNIX_EPOCH + Duration::from_secs(1234567890 + 1)),
"test",
),
Credentials::new(
"test",
"test",
None,
Some(UNIX_EPOCH + Duration::from_secs(1234567890 + 120)),
"test",
),
]));
let credentials_list_cloned = credentials_list.clone();
let provider = AssumeRoleProvider::builder("myrole")
.configure(&provider_conf)
.region(Region::new("us-east-1"))
.build(provide_credentials_fn(|| async {
Ok(Credentials::for_tests())
.build(provide_credentials_fn(move || {
let list = credentials_list.clone();
async move {
let next = list.lock().unwrap().remove(0);
Ok(next)
}
}));

tokio::time::pause();

let creds_first = provider
.provide_credentials()
.await
.expect("should return valid credentials");
// The effect of caching is implicitly enabled by a `LazyCredentialsCache`
// baked in the configuration for STS stored in `provider.inner.conf`.

// After time has been advanced by 120 seconds, the first credentials _could_ still be valid
// if `LazyCredentialsCache` were used, but the provider uses `NoCredentialsCache` by default
// so the first credentials will not be used.
testing_time_source.advance(Duration::from_secs(120));

let creds_second = provider
.provide_credentials()
.await
.expect("cached credentials should be returned");
assert_eq!(creds_first, creds_second);
.expect("should return the second credentials");
assert_ne!(creds_first, creds_second);
assert!(credentials_list_cloned.lock().unwrap().is_empty());
}
}
2 changes: 2 additions & 0 deletions aws/rust-runtime/aws-config/src/web_identity_token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@

use crate::provider_config::ProviderConfig;
use crate::sts;
use aws_credential_types::cache::CredentialsCache;
use aws_credential_types::provider::{self, error::CredentialsError, future, ProvideCredentials};
use aws_sdk_sts::config::Region;
use aws_sdk_sts::middleware::DefaultMiddleware;
Expand Down Expand Up @@ -235,6 +236,7 @@ async fn load_credentials(
})?;
let conf = aws_sdk_sts::Config::builder()
.region(region.clone())
.credentials_cache(CredentialsCache::no_caching())
.build();

let operation = AssumeRoleWithWebIdentityInput::builder()
Expand Down
11 changes: 11 additions & 0 deletions aws/rust-runtime/aws-credential-types/src/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@

mod expiring_cache;
mod lazy_caching;
mod no_caching;

pub use expiring_cache::ExpiringCache;
pub use lazy_caching::Builder as LazyBuilder;
use no_caching::NoCredentialsCache;

use crate::provider::{future, SharedCredentialsProvider};
use std::sync::Arc;
Expand Down Expand Up @@ -63,6 +65,7 @@ impl ProvideCachedCredentials for SharedCredentialsCache {
#[derive(Clone, Debug)]
pub(crate) enum Inner {
Lazy(lazy_caching::Builder),
NoCaching,
}

/// `CredentialsCache` allows for configuring and creating a credentials cache.
Expand Down Expand Up @@ -104,10 +107,18 @@ impl CredentialsCache {
lazy_caching::Builder::new()
}

/// Creates a [`CredentialsCache`] that offers no caching ability.
pub fn no_caching() -> Self {
Self {
inner: Inner::NoCaching,
}
}

/// Creates a [`SharedCredentialsCache`] wrapping a concrete caching implementation.
pub fn create_cache(self, provider: SharedCredentialsProvider) -> SharedCredentialsCache {
match self.inner {
Inner::Lazy(builder) => SharedCredentialsCache::new(builder.build(provider)),
Inner::NoCaching => SharedCredentialsCache::new(NoCredentialsCache::new(provider)),
}
}
}
83 changes: 83 additions & 0 deletions aws/rust-runtime/aws-credential-types/src/cache/no_caching.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

//! Credentials cache that offers no caching ability

use crate::cache::ProvideCachedCredentials;
use crate::provider::SharedCredentialsProvider;
use crate::provider::{future, ProvideCredentials};
use tracing::debug;

#[derive(Debug)]
pub(crate) struct NoCredentialsCache {
provider: SharedCredentialsProvider,
}

impl NoCredentialsCache {
pub(crate) fn new(provider: SharedCredentialsProvider) -> Self {
Self { provider }
}
}

impl ProvideCachedCredentials for NoCredentialsCache {
fn provide_cached_credentials<'a>(&'a self) -> future::ProvideCredentials<'_>
where
Self: 'a,
{
debug!("Delegating `provide_cached_credentials` to `provide_credentials` on the provider");
self.provider.provide_credentials()
}
}

#[cfg(test)]
mod tests {
use super::*;
use crate::credential_fn::provide_credentials_fn;
use crate::Credentials;
use std::sync::{Arc, Mutex};
use std::time::{Duration, SystemTime};

fn test_provider(load_list: Vec<crate::provider::Result>) -> NoCredentialsCache {
let load_list = Arc::new(Mutex::new(load_list));
NoCredentialsCache::new(SharedCredentialsProvider::new(provide_credentials_fn(
move || {
let list = load_list.clone();
async move {
let next = list.lock().unwrap().remove(0);
next
}
},
)))
}

fn epoch_secs(secs: u64) -> SystemTime {
SystemTime::UNIX_EPOCH + Duration::from_secs(secs)
}

fn credentials(expired_secs: u64) -> Credentials {
Credentials::new("test", "test", None, Some(epoch_secs(expired_secs)), "test")
}

async fn expect_creds(expired_secs: u64, provider: &NoCredentialsCache) {
let creds = provider
.provide_cached_credentials()
.await
.expect("expected credentials");
assert_eq!(Some(epoch_secs(expired_secs)), creds.expiry());
}

#[tokio::test]
async fn no_caching() {
let credentials_cache = test_provider(vec![
Ok(credentials(1000)),
Ok(credentials(2000)),
Ok(credentials(3000)),
]);

expect_creds(1000, &credentials_cache).await;
expect_creds(2000, &credentials_cache).await;
expect_creds(3000, &credentials_cache).await;
}
}

0 comments on commit f7fc8ce

Please sign in to comment.