-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
chore: Upgrade aws-smithy and aws-sdk crates #17731
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,10 @@ use aws_config::{ | |
}, | ||
sts::AssumeRoleProviderBuilder, | ||
}; | ||
use aws_types::{credentials::SharedCredentialsProvider, region::Region, Credentials}; | ||
use aws_credential_types::{ | ||
cache::CredentialsCache, provider::SharedCredentialsProvider, Credentials, | ||
}; | ||
use aws_types::region::Region; | ||
use serde_with::serde_as; | ||
use vector_common::sensitive_string::SensitiveString; | ||
use vector_config::configurable_component; | ||
|
@@ -165,6 +168,28 @@ fn default_profile() -> String { | |
} | ||
|
||
impl AwsAuthentication { | ||
pub async fn credentials_cache(&self) -> crate::Result<CredentialsCache> { | ||
match self { | ||
AwsAuthentication::Role { | ||
load_timeout_secs, .. | ||
} | ||
| AwsAuthentication::Default { | ||
load_timeout_secs, .. | ||
} => { | ||
let credentials_cache = CredentialsCache::lazy_builder() | ||
.load_timeout( | ||
load_timeout_secs | ||
.map(Duration::from_secs) | ||
.unwrap_or(DEFAULT_LOAD_TIMEOUT), | ||
) | ||
.into_credentials_cache(); | ||
|
||
Ok(credentials_cache) | ||
} | ||
_ => Ok(CredentialsCache::lazy()), | ||
} | ||
} | ||
|
||
pub async fn credentials_provider( | ||
&self, | ||
service_region: Region, | ||
|
@@ -207,28 +232,20 @@ impl AwsAuthentication { | |
} | ||
AwsAuthentication::Role { | ||
assume_role, | ||
load_timeout_secs, | ||
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. For other readers, this is covered by the new Nit: not related to this PR but since it technically could apply the load timeout to all providers now since it happens at a different level, we could pull up |
||
imds, | ||
region, | ||
.. | ||
} => { | ||
let auth_region = region.clone().map(Region::new).unwrap_or(service_region); | ||
let provider = AssumeRoleProviderBuilder::new(assume_role) | ||
.region(auth_region.clone()) | ||
.build( | ||
default_credentials_provider(auth_region, *load_timeout_secs, *imds) | ||
.await?, | ||
); | ||
.build(default_credentials_provider(auth_region, *imds).await?); | ||
|
||
Ok(SharedCredentialsProvider::new(provider)) | ||
} | ||
AwsAuthentication::Default { | ||
load_timeout_secs, | ||
imds, | ||
region, | ||
} => Ok(SharedCredentialsProvider::new( | ||
AwsAuthentication::Default { imds, region, .. } => Ok(SharedCredentialsProvider::new( | ||
default_credentials_provider( | ||
region.clone().map(Region::new).unwrap_or(service_region), | ||
*load_timeout_secs, | ||
*imds, | ||
) | ||
.await?, | ||
|
@@ -249,7 +266,6 @@ impl AwsAuthentication { | |
|
||
async fn default_credentials_provider( | ||
region: Region, | ||
load_timeout_secs: Option<u64>, | ||
imds: ImdsAuthentication, | ||
) -> crate::Result<SharedCredentialsProvider> { | ||
let client = imds::Client::builder() | ||
|
@@ -259,16 +275,13 @@ async fn default_credentials_provider( | |
.build() | ||
.await?; | ||
|
||
let chain = DefaultCredentialsChain::builder() | ||
let credentials_provider = DefaultCredentialsChain::builder() | ||
.region(region) | ||
.imds_client(client) | ||
.load_timeout( | ||
load_timeout_secs | ||
.map(Duration::from_secs) | ||
.unwrap_or(DEFAULT_LOAD_TIMEOUT), | ||
); | ||
.build() | ||
.await; | ||
|
||
Ok(SharedCredentialsProvider::new(chain.build().await)) | ||
Ok(SharedCredentialsProvider::new(credentials_provider)) | ||
} | ||
|
||
#[cfg(test)] | ||
|
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.
Was there any manual testing done? I'm just thinking about recent AWS component regressions we had.
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.
Manually tested against cloudwatch logs! With and without valid credentials. Neither encountered the signing issue we previously encountered.