Unverified Commit 7ccac060 authored by ysaito1001's avatar ysaito1001 Committed by GitHub
Browse files

Add `NoCredentialsCache` that offers no caching ability (#2720) 

## Motivation and Context
Related to https://github.com/awslabs/aws-sdk-rust/issues/809

## Description
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.

## Testing
- Added unit tests for `NoCredentialsCache`
- Updated unit test for `AssumeRoleProvider` to verify
`NoCredentialsCache` is used by default

## 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._

---------

Co-authored-by: default avatarYuki Saito <awsaito@amazon.com>
parent bbe9d52a

CHANGELOG.next.toml

+6 −0
Original line number Diff line number Diff line
@@ -41,3 +41,9 @@ message = "Fix error message when `credentials-sso` feature is not enabled on `a 
references = ["smithy-rs#2722", "aws-sdk-rust#703"]

meta = { "breaking" = false, "tada" = false, "bug" = true }

author = "rcoh"



[[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"

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

+2 −0
Original line number Diff line number Diff line
@@ -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;
@@ -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)

aws/rust-runtime/aws-config/src/sts/assume_role.rs

+53 −17
Original line number Diff line number Diff line
@@ -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
@@ -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)
@@ -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());

    }

}

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

+2 −0
Original line number Diff line number Diff line
@@ -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;
@@ -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()

aws/rust-runtime/aws-credential-types/src/cache.rs

+11 −0
Original line number Diff line number Diff line
@@ -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;
@@ -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.
@@ -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)),

        }

    }

}