-
Notifications
You must be signed in to change notification settings - Fork 189
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
Replace credentials cache with identity cache #3077
Changes from 8 commits
dd06410
0575d20
f61f8d5
afab3b2
4fc2464
c02a0e6
82b99f8
cef15d3
890a173
0d3ed15
7953c9c
85bd11a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. any issue with the re-export aws_smithy_runtime here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
pub use aws_smithy_runtime::client::identity::LazyCacheBuilder; | ||
} | ||
|
||
#[allow(dead_code)] | ||
const PKG_VERSION: &str = env!("CARGO_PKG_VERSION"); | ||
|
||
|
@@ -155,11 +161,11 @@ mod loader { | |
use crate::meta::region::ProvideRegion; | ||
use crate::profile::profile_file::ProfileFiles; | ||
use crate::provider_config::ProviderConfig; | ||
use aws_credential_types::cache::CredentialsCache; | ||
use aws_credential_types::provider::{ProvideCredentials, SharedCredentialsProvider}; | ||
use aws_smithy_async::rt::sleep::{default_async_sleep, AsyncSleep, SharedAsyncSleep}; | ||
use aws_smithy_async::time::{SharedTimeSource, TimeSource}; | ||
use aws_smithy_runtime_api::client::http::HttpClient; | ||
use aws_smithy_runtime_api::client::identity::{ResolveCachedIdentity, SharedIdentityCache}; | ||
use aws_smithy_runtime_api::shared::IntoShared; | ||
use aws_smithy_types::retry::RetryConfig; | ||
use aws_smithy_types::timeout::TimeoutConfig; | ||
|
@@ -189,7 +195,7 @@ mod loader { | |
#[derive(Default, Debug)] | ||
pub struct ConfigLoader { | ||
app_name: Option<AppName>, | ||
credentials_cache: Option<CredentialsCache>, | ||
identity_cache: Option<SharedIdentityCache>, | ||
credentials_provider: CredentialsProviderOption, | ||
endpoint_url: Option<String>, | ||
region: Option<Box<dyn ProvideRegion>>, | ||
|
@@ -333,22 +339,36 @@ mod loader { | |
self | ||
} | ||
|
||
/// 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
/// that will load credentials upon first request, cache them, and then reload them during | ||
/// another request when they are close to expiring. | ||
/// | ||
/// # Examples | ||
/// | ||
/// Override the credentials cache but load the default value for region: | ||
/// Change a setting on the default lazy caching implementation: | ||
/// ```no_run | ||
/// # use aws_credential_types::cache::CredentialsCache; | ||
/// use aws_config::identity::IdentityCache; | ||
/// use std::time::Duration; | ||
/// | ||
/// # async fn create_config() { | ||
/// let config = aws_config::from_env() | ||
/// .credentials_cache(CredentialsCache::lazy()) | ||
/// .identity_cache( | ||
/// IdentityCache::lazy() | ||
/// // change the load timeout to 10 seconds | ||
/// .load_timeout(Duration::from_secs(10)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. may want to note that there may be other timeouts that could trigger |
||
/// .build() | ||
/// ) | ||
/// .load() | ||
/// .await; | ||
/// # } | ||
/// ``` | ||
pub fn credentials_cache(mut self, credentials_cache: CredentialsCache) -> Self { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. deprecate |
||
self.credentials_cache = Some(credentials_cache); | ||
pub fn identity_cache( | ||
mut self, | ||
identity_cache: impl ResolveCachedIdentity + 'static, | ||
) -> Self { | ||
self.identity_cache = Some(identity_cache.into_shared()); | ||
self | ||
} | ||
|
||
|
@@ -656,17 +676,6 @@ mod loader { | |
CredentialsProviderOption::ExplicitlyUnset => None, | ||
}; | ||
|
||
let credentials_cache = if credentials_provider.is_some() { | ||
Some(self.credentials_cache.unwrap_or_else(|| { | ||
let mut builder = | ||
CredentialsCache::lazy_builder().time_source(conf.time_source()); | ||
builder.set_sleep_impl(conf.sleep_impl()); | ||
builder.into_credentials_cache() | ||
})) | ||
} else { | ||
None | ||
}; | ||
|
||
let mut builder = SdkConfig::builder() | ||
.region(region) | ||
.retry_config(retry_config) | ||
|
@@ -675,7 +684,7 @@ mod loader { | |
|
||
builder.set_http_client(self.http_client); | ||
builder.set_app_name(app_name); | ||
builder.set_credentials_cache(credentials_cache); | ||
builder.set_identity_cache(self.identity_cache); | ||
builder.set_credentials_provider(credentials_provider); | ||
builder.set_sleep_impl(sleep_impl); | ||
builder.set_endpoint_url(self.endpoint_url); | ||
|
@@ -705,13 +714,11 @@ mod loader { | |
use crate::{from_env, ConfigLoader}; | ||
use aws_credential_types::provider::ProvideCredentials; | ||
use aws_smithy_async::rt::sleep::TokioSleep; | ||
use aws_smithy_async::time::{StaticTimeSource, TimeSource}; | ||
use aws_smithy_runtime::client::http::test_util::{infallible_client_fn, NeverClient}; | ||
use aws_types::app_name::AppName; | ||
use aws_types::os_shim_internal::{Env, Fs}; | ||
use std::sync::atomic::{AtomicUsize, Ordering}; | ||
use std::sync::Arc; | ||
use std::time::{SystemTime, UNIX_EPOCH}; | ||
use tracing_test::traced_test; | ||
|
||
#[tokio::test] | ||
|
@@ -800,7 +807,7 @@ mod loader { | |
#[tokio::test] | ||
async fn disable_default_credentials() { | ||
let config = from_env().no_credentials().load().await; | ||
assert!(config.credentials_cache().is_none()); | ||
assert!(config.identity_cache().is_none()); | ||
assert!(config.credentials_provider().is_none()); | ||
} | ||
|
||
|
@@ -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() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this test no longer applicable or did it not add much value in the first place? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
#[derive(Debug)] | ||
struct PanicTs; | ||
impl TimeSource for PanicTs { | ||
fn now(&self) -> SystemTime { | ||
panic!("timesource-was-used") | ||
} | ||
} | ||
let config = from_env() | ||
.sleep_impl(InstantSleep) | ||
.time_source(StaticTimeSource::new(UNIX_EPOCH)) | ||
.http_client(no_traffic_client()) | ||
.load() | ||
.await; | ||
// assert that the innards contain the customized fields | ||
for inner in ["InstantSleep", "StaticTimeSource"] { | ||
assert!( | ||
format!("{:#?}", config.credentials_cache()).contains(inner), | ||
"{:#?}", | ||
config.credentials_cache() | ||
); | ||
assert!( | ||
format!("{:#?}", config.credentials_provider()).contains(inner), | ||
"{:#?}", | ||
config.credentials_cache() | ||
); | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we probably want this at trace