From 88afcf8c686adf456871e5d55cda6f2a3c0e00b4 Mon Sep 17 00:00:00 2001 From: Russell Cohen Date: Tue, 3 Jan 2023 14:37:33 -0500 Subject: [PATCH] Refactor configuration loading to parse profile files only once (#2152) * Draft PR to only parse configuration once * Update docs & and add test * Fix bug where cache was being cleared by mistake * Add additional docs * Apply suggestions from code review Co-authored-by: Zelda Hessler * Apply suggestions from code review Co-authored-by: Zelda Hessler * fix name of field Co-authored-by: Zelda Hessler --- CHANGELOG.next.toml | 43 +++++++ .../src/default_provider/app_name.rs | 25 +++- .../aws-config/src/imds/client.rs | 22 ++-- .../aws-config/src/imds/client/error.rs | 12 -- aws/rust-runtime/aws-config/src/lib.rs | 114 +++++++++++++++++- .../aws-config/src/profile/app_name.rs | 52 +++----- .../aws-config/src/profile/credentials.rs | 100 ++++++--------- .../src/profile/credentials/repr.rs | 19 ++- .../aws-config/src/profile/mod.rs | 2 +- .../aws-config/src/profile/parser.rs | 73 +++++++++-- .../aws-config/src/profile/parser/parse.rs | 2 +- .../aws-config/src/profile/parser/source.rs | 17 +-- .../aws-config/src/profile/profile_file.rs | 12 +- .../aws-config/src/profile/region.rs | 38 ++---- .../aws-config/src/profile/retry_config.rs | 50 ++------ .../aws-config/src/provider_config.rs | 85 ++++++++++++- aws/rust-runtime/aws-config/src/test_case.rs | 4 +- 17 files changed, 425 insertions(+), 245 deletions(-) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index d75cc4e8f..913d3e352 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -87,3 +87,46 @@ message = "The constraint `@length` on non-streaming blob shapes is supported." references = ["smithy-rs#2131"] meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "server"} author = "82marbag" + +[[aws-sdk-rust]] +references = ["smithy-rs#2152"] +meta = { "breaking" = false, "tada" = false, "bug" = false } +author = "rcoh" +message = """Add support for overriding profile name and profile file location across all providers. Prior to this change, each provider needed to be updated individually. + +### Before +```rust +use aws_config::profile::{ProfileFileCredentialsProvider, ProfileFileRegionProvider}; +use aws_config::profile::profile_file::{ProfileFiles, ProfileFileKind}; + +let profile_files = ProfileFiles::builder() + .with_file(ProfileFileKind::Credentials, "some/path/to/credentials-file") + .build(); +let credentials_provider = ProfileFileCredentialsProvider::builder() + .profile_files(profile_files.clone()) + .build(); +let region_provider = ProfileFileRegionProvider::builder() + .profile_files(profile_files) + .build(); + +let sdk_config = aws_config::from_env() + .credentials_provider(credentials_provider) + .region(region_provider) + .load() + .await; +``` + +### After +```rust +use aws_config::profile::{ProfileFileCredentialsProvider, ProfileFileRegionProvider}; +use aws_config::profile::profile_file::{ProfileFiles, ProfileFileKind}; + +let profile_files = ProfileFiles::builder() + .with_file(ProfileFileKind::Credentials, "some/path/to/credentials-file") + .build(); +let sdk_config = aws_config::from_env() + .profile_files(profile_files) + .load() + .await; +/// ``` +""" diff --git a/aws/rust-runtime/aws-config/src/default_provider/app_name.rs b/aws/rust-runtime/aws-config/src/default_provider/app_name.rs index af8dcfa00..f6fce3d73 100644 --- a/aws/rust-runtime/aws-config/src/default_provider/app_name.rs +++ b/aws/rust-runtime/aws-config/src/default_provider/app_name.rs @@ -52,8 +52,9 @@ impl Builder { #[cfg(test)] mod tests { use super::*; + use crate::profile::profile_file::{ProfileFileKind, ProfileFiles}; use crate::provider_config::ProviderConfig; - use crate::test_case::no_traffic_connector; + use crate::test_case::{no_traffic_connector, InstantSleep}; use aws_types::os_shim_internal::{Env, Fs}; #[tokio::test] @@ -76,6 +77,28 @@ mod tests { assert_eq!(Some(AppName::new("correct").unwrap()), app_name); } + // test that overriding profile_name on the root level is deprecated + #[tokio::test] + async fn profile_name_override() { + let fs = Fs::from_slice(&[("test_config", "[profile custom]\nsdk-ua-app-id = correct")]); + let conf = crate::from_env() + .configure( + ProviderConfig::empty() + .with_fs(fs) + .with_sleep(InstantSleep) + .with_http_connector(no_traffic_connector()), + ) + .profile_name("custom") + .profile_files( + ProfileFiles::builder() + .with_file(ProfileFileKind::Config, "test_config") + .build(), + ) + .load() + .await; + assert_eq!(conf.app_name(), Some(&AppName::new("correct").unwrap())); + } + #[tokio::test] async fn load_from_profile() { let fs = Fs::from_slice(&[("test_config", "[default]\nsdk-ua-app-id = correct")]); diff --git a/aws/rust-runtime/aws-config/src/imds/client.rs b/aws/rust-runtime/aws-config/src/imds/client.rs index c9df16fc5..ba5cea046 100644 --- a/aws/rust-runtime/aws-config/src/imds/client.rs +++ b/aws/rust-runtime/aws-config/src/imds/client.rs @@ -11,7 +11,7 @@ use crate::connector::expect_connector; use crate::imds::client::error::{BuildError, ImdsError, InnerImdsError, InvalidEndpointMode}; use crate::imds::client::token::TokenMiddleware; use crate::provider_config::ProviderConfig; -use crate::{profile, PKG_VERSION}; +use crate::PKG_VERSION; use aws_http::user_agent::{ApiMetadata, AwsUserAgent, UserAgentStage}; use aws_sdk_sso::config::timeout::TimeoutConfig; use aws_smithy_client::http_connector::ConnectorSettings; @@ -28,7 +28,7 @@ use aws_smithy_http_tower::map_request::{ }; use aws_smithy_types::error::display::DisplayErrorContext; use aws_smithy_types::retry::{ErrorKind, RetryKind}; -use aws_types::os_shim_internal::{Env, Fs}; +use aws_types::os_shim_internal::Env; use bytes::Bytes; use http::{Response, Uri}; use std::borrow::Cow; @@ -429,7 +429,7 @@ impl Builder { let connector = expect_connector(config.connector(&connector_settings)); let endpoint_source = self .endpoint - .unwrap_or_else(|| EndpointSource::Env(config.env(), config.fs())); + .unwrap_or_else(|| EndpointSource::Env(config.clone())); let endpoint = endpoint_source.endpoint(self.mode_override).await?; let retry_config = retry::Config::default() .with_max_attempts(self.max_attempts.unwrap_or(DEFAULT_ATTEMPTS)); @@ -475,7 +475,7 @@ mod profile_keys { #[derive(Debug, Clone)] enum EndpointSource { Explicit(Uri), - Env(Env, Fs), + Env(ProviderConfig), } impl EndpointSource { @@ -489,15 +489,16 @@ impl EndpointSource { } Ok(uri.clone()) } - EndpointSource::Env(env, fs) => { + EndpointSource::Env(conf) => { + let env = conf.env(); // load an endpoint override from the environment - let profile = profile::load(fs, env, &Default::default()) - .await - .map_err(BuildError::invalid_profile)?; + let profile = conf.profile().await; let uri_override = if let Ok(uri) = env.get(env::ENDPOINT) { Some(Cow::Owned(uri)) } else { - profile.get(profile_keys::ENDPOINT).map(Cow::Borrowed) + profile + .and_then(|profile| profile.get(profile_keys::ENDPOINT)) + .map(Cow::Borrowed) }; if let Some(uri) = uri_override { return Uri::try_from(uri.as_ref()).map_err(BuildError::invalid_endpoint_uri); @@ -509,7 +510,8 @@ impl EndpointSource { } else if let Ok(mode) = env.get(env::ENDPOINT_MODE) { mode.parse::() .map_err(BuildError::invalid_endpoint_mode)? - } else if let Some(mode) = profile.get(profile_keys::ENDPOINT_MODE) { + } else if let Some(mode) = profile.and_then(|p| p.get(profile_keys::ENDPOINT_MODE)) + { mode.parse::() .map_err(BuildError::invalid_endpoint_mode)? } else { diff --git a/aws/rust-runtime/aws-config/src/imds/client/error.rs b/aws/rust-runtime/aws-config/src/imds/client/error.rs index 72ddb2dab..b9559486a 100644 --- a/aws/rust-runtime/aws-config/src/imds/client/error.rs +++ b/aws/rust-runtime/aws-config/src/imds/client/error.rs @@ -5,7 +5,6 @@ //! Error types for [`ImdsClient`](crate::imds::client::Client) -use crate::profile::credentials::ProfileFileError; use aws_smithy_client::SdkError; use aws_smithy_http::body::SdkBody; use aws_smithy_http::endpoint::error::InvalidEndpointError; @@ -178,9 +177,6 @@ enum BuildErrorKind { /// The endpoint mode was invalid InvalidEndpointMode(InvalidEndpointMode), - /// The AWS Profile (e.g. `~/.aws/config`) was invalid - InvalidProfile(ProfileFileError), - /// The specified endpoint was not a valid URI InvalidEndpointUri(Box), } @@ -198,12 +194,6 @@ impl BuildError { } } - pub(super) fn invalid_profile(source: ProfileFileError) -> Self { - Self { - kind: BuildErrorKind::InvalidProfile(source), - } - } - pub(super) fn invalid_endpoint_uri( source: impl Into>, ) -> Self { @@ -219,7 +209,6 @@ impl fmt::Display for BuildError { write!(f, "failed to build IMDS client: ")?; match self.kind { InvalidEndpointMode(_) => write!(f, "invalid endpoint mode"), - InvalidProfile(_) => write!(f, "profile file error"), InvalidEndpointUri(_) => write!(f, "invalid URI"), } } @@ -230,7 +219,6 @@ impl Error for BuildError { use BuildErrorKind::*; match &self.kind { InvalidEndpointMode(e) => Some(e), - InvalidProfile(e) => Some(e), InvalidEndpointUri(e) => Some(e.as_ref()), } } diff --git a/aws/rust-runtime/aws-config/src/lib.rs b/aws/rust-runtime/aws-config/src/lib.rs index 567cdaff1..a4f77301d 100644 --- a/aws/rust-runtime/aws-config/src/lib.rs +++ b/aws/rust-runtime/aws-config/src/lib.rs @@ -161,6 +161,7 @@ mod loader { use crate::connector::default_connector; use crate::default_provider::{app_name, credentials, region, retry_config, timeout_config}; use crate::meta::region::ProvideRegion; + use crate::profile::profile_file::ProfileFiles; use crate::provider_config::ProviderConfig; /// Load a cross-service [`SdkConfig`](aws_types::SdkConfig) from the environment @@ -181,6 +182,8 @@ mod loader { timeout_config: Option, provider_config: Option, http_connector: Option, + profile_name_override: Option, + profile_files_override: Option, } impl ConfigLoader { @@ -344,6 +347,75 @@ mod loader { self } + /// Provides the ability to programmatically override the profile files that get loaded by the SDK. + /// + /// The [`Default`] for `ProfileFiles` includes the default SDK config and credential files located in + /// `~/.aws/config` and `~/.aws/credentials` respectively. + /// + /// Any number of config and credential files may be added to the `ProfileFiles` file set, with the + /// only requirement being that there is at least one of each. Profile file locations will produce an + /// error if they don't exist, but the default config/credentials files paths are exempt from this validation. + /// + /// # Example: Using a custom profile file path + /// + /// ``` + /// use aws_config::profile::{ProfileFileCredentialsProvider, ProfileFileRegionProvider}; + /// use aws_config::profile::profile_file::{ProfileFiles, ProfileFileKind}; + /// + /// # async fn example() { + /// let profile_files = ProfileFiles::builder() + /// .with_file(ProfileFileKind::Credentials, "some/path/to/credentials-file") + /// .build(); + /// let sdk_config = aws_config::from_env() + /// .profile_files(profile_files) + /// .load() + /// .await; + /// # } + pub fn profile_files(mut self, profile_files: ProfileFiles) -> Self { + self.profile_files_override = Some(profile_files); + self + } + + /// Override the profile name used by configuration providers + /// + /// Profile name is selected from an ordered list of sources: + /// 1. This override. + /// 2. The value of the `AWS_PROFILE` environment variable. + /// 3. `default` + /// + /// Each AWS profile has a name. For example, in the file below, the profiles are named + /// `dev`, `prod` and `staging`: + /// ```ini + /// [dev] + /// ec2_metadata_service_endpoint = http://my-custom-endpoint:444 + /// + /// [staging] + /// ec2_metadata_service_endpoint = http://my-custom-endpoint:444 + /// + /// [prod] + /// ec2_metadata_service_endpoint = http://my-custom-endpoint:444 + /// ``` + /// + /// See [Named profiles](https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-profiles.html) + /// for more information about naming profiles. + /// + /// # Example: Using a custom profile name + /// + /// ``` + /// use aws_config::profile::{ProfileFileCredentialsProvider, ProfileFileRegionProvider}; + /// use aws_config::profile::profile_file::{ProfileFiles, ProfileFileKind}; + /// + /// # async fn example() { + /// let sdk_config = aws_config::from_env() + /// .profile_name("prod") + /// .load() + /// .await; + /// # } + pub fn profile_name(mut self, profile_name: impl Into) -> Self { + self.profile_name_override = Some(profile_name.into()); + self + } + /// Override the endpoint URL used for **all** AWS services. /// /// This method will override the endpoint URL used for **all** AWS services. This primarily @@ -372,16 +444,14 @@ mod loader { /// /// Update the `ProviderConfig` used for all nested loaders. This can be used to override /// the HTTPs connector used by providers or to stub in an in memory `Env` or `Fs` for testing. - /// This **does not set** the HTTP connector used when sending operations. To change that - /// connector, use [ConfigLoader::http_connector]. /// /// # Examples /// ```no_run /// # #[cfg(feature = "hyper-client")] /// # async fn create_config() { /// use aws_config::provider_config::ProviderConfig; - /// let custom_https_connector = hyper_rustls::HttpsConnectorBuilder::new(). - /// with_webpki_roots() + /// let custom_https_connector = hyper_rustls::HttpsConnectorBuilder::new() + /// .with_webpki_roots() /// .https_only() /// .enable_http1() /// .build(); @@ -404,7 +474,10 @@ mod loader { /// This means that if you provide a region provider that does not return a region, no region will /// be set in the resulting [`SdkConfig`](aws_types::SdkConfig) pub async fn load(self) -> SdkConfig { - let conf = self.provider_config.unwrap_or_default(); + let conf = self + .provider_config + .unwrap_or_default() + .with_profile_config(self.profile_files_override, self.profile_name_override); let region = if let Some(provider) = self.region { provider.region().await } else { @@ -497,12 +570,16 @@ mod loader { use aws_smithy_async::rt::sleep::TokioSleep; use aws_smithy_client::erase::DynConnector; use aws_smithy_client::never::NeverConnector; - use aws_types::os_shim_internal::Env; + use aws_types::app_name::AppName; + use aws_types::os_shim_internal::{Env, Fs}; + use tracing_test::traced_test; use crate::from_env; + use crate::profile::profile_file::{ProfileFileKind, ProfileFiles}; use crate::provider_config::ProviderConfig; #[tokio::test] + #[traced_test] async fn provider_config_used() { let env = Env::from_slice(&[ ("AWS_MAX_ATTEMPTS", "10"), @@ -510,13 +587,22 @@ mod loader { ("AWS_ACCESS_KEY_ID", "akid"), ("AWS_SECRET_ACCESS_KEY", "secret"), ]); + let fs = + Fs::from_slice(&[("test_config", "[profile custom]\nsdk-ua-app-id = correct")]); let loader = from_env() .configure( ProviderConfig::empty() .with_sleep(TokioSleep::new()) .with_env(env) + .with_fs(fs) .with_http_connector(DynConnector::new(NeverConnector::new())), ) + .profile_name("custom") + .profile_files( + ProfileFiles::builder() + .with_file(ProfileFileKind::Config, "test_config") + .build(), + ) .load() .await; assert_eq!(loader.retry_config().unwrap().max_attempts(), 10); @@ -531,6 +617,22 @@ mod loader { .access_key_id(), "akid" ); + assert_eq!(loader.app_name(), Some(&AppName::new("correct").unwrap())); + logs_assert(|lines| { + let num_config_loader_logs = lines + .iter() + .filter(|l| l.contains("provider_config_used")) + .filter(|l| l.contains("config file loaded")) + .count(); + match num_config_loader_logs { + 0 => Err("no config file logs found!".to_string()), + 1 => Ok(()), + more => Err(format!( + "the config file was parsed more than once! (parsed {})", + more + )), + } + }); } } } diff --git a/aws/rust-runtime/aws-config/src/profile/app_name.rs b/aws/rust-runtime/aws-config/src/profile/app_name.rs index 65ee943ad..a49fefc8b 100644 --- a/aws/rust-runtime/aws-config/src/profile/app_name.rs +++ b/aws/rust-runtime/aws-config/src/profile/app_name.rs @@ -7,9 +7,7 @@ use super::profile_file::ProfileFiles; use crate::provider_config::ProviderConfig; -use aws_smithy_types::error::display::DisplayErrorContext; use aws_types::app_name::AppName; -use aws_types::os_shim_internal::{Env, Fs}; /// Loads an app name from a profile file /// @@ -36,10 +34,7 @@ use aws_types::os_shim_internal::{Env, Fs}; /// This provider is part of the [default app name provider chain](crate::default_provider::app_name). #[derive(Debug, Default)] pub struct ProfileFileAppNameProvider { - fs: Fs, - env: Env, - profile_override: Option, - profile_files: ProfileFiles, + provider_config: ProviderConfig, } impl ProfileFileAppNameProvider { @@ -48,10 +43,7 @@ impl ProfileFileAppNameProvider { /// To override the selected profile, set the `AWS_PROFILE` environment variable or use the [`Builder`]. pub fn new() -> Self { Self { - fs: Fs::real(), - env: Env::real(), - profile_override: None, - profile_files: Default::default(), + provider_config: ProviderConfig::default(), } } @@ -62,26 +54,14 @@ impl ProfileFileAppNameProvider { /// Parses the profile config and attempts to find an app name. pub async fn app_name(&self) -> Option { - let profile = super::parser::load(&self.fs, &self.env, &self.profile_files) - .await - .map_err( - |err| tracing::warn!(err = %DisplayErrorContext(&err), "failed to parse profile"), - ) - .ok()?; - let selected_profile_name = self - .profile_override - .as_deref() - .unwrap_or_else(|| profile.selected_profile()); - let selected_profile = profile.get_profile(selected_profile_name)?; - selected_profile - .get("sdk-ua-app-id") - .and_then(|name| match AppName::new(name.to_owned()) { - Ok(app_name) => Some(app_name), - Err(err) => { - tracing::warn!(err = %err, "`sdk-ua-app-id` property in profile `{}` was invalid", selected_profile_name); - None - } - }) + let app_id = self.provider_config.profile().await?.get("sdk-ua-app-id")?; + match AppName::new(app_id.to_owned()) { + Ok(app_name) => Some(app_name), + Err(err) => { + tracing::warn!(err = %err, "`sdk-ua-app-id` property `{}` was invalid", app_id); + None + } + } } } @@ -108,12 +88,12 @@ impl Builder { /// Build a [ProfileFileAppNameProvider] from this builder pub fn build(self) -> ProfileFileAppNameProvider { - let conf = self.config.unwrap_or_default(); + let conf = self + .config + .unwrap_or_default() + .with_profile_config(self.profile_files, self.profile_override); ProfileFileAppNameProvider { - env: conf.env(), - fs: conf.fs(), - profile_override: self.profile_override, - profile_files: self.profile_files.unwrap_or_default(), + provider_config: conf, } } } @@ -187,7 +167,7 @@ mod tests { .await ); assert!(logs_contain( - "`sdk-ua-app-id` property in profile `default` was invalid" + "`sdk-ua-app-id` property `definitely invalid` was invalid" )); } } diff --git a/aws/rust-runtime/aws-config/src/profile/credentials.rs b/aws/rust-runtime/aws-config/src/profile/credentials.rs index b9bc11f21..c429c8563 100644 --- a/aws/rust-runtime/aws-config/src/profile/credentials.rs +++ b/aws/rust-runtime/aws-config/src/profile/credentials.rs @@ -24,7 +24,7 @@ use crate::profile::credentials::exec::named::NamedProviderFactory; use crate::profile::credentials::exec::{ClientConfiguration, ProviderChain}; -use crate::profile::parser::ProfileParseError; +use crate::profile::parser::ProfileFileLoadError; use crate::profile::profile_file::ProfileFiles; use crate::profile::Profile; use crate::provider_config::ProviderConfig; @@ -34,7 +34,6 @@ use std::borrow::Cow; use std::collections::HashMap; use std::error::Error; use std::fmt::{Display, Formatter}; -use std::path::PathBuf; use std::sync::Arc; use tracing::Instrument; @@ -145,8 +144,6 @@ pub struct ProfileFileCredentialsProvider { factory: NamedProviderFactory, client_config: ClientConfiguration, provider_config: ProviderConfig, - profile_override: Option, - profile_files: ProfileFiles, } impl ProfileFileCredentialsProvider { @@ -156,23 +153,18 @@ impl ProfileFileCredentialsProvider { } async fn load_credentials(&self) -> provider::Result { - let inner_provider = build_provider_chain( - &self.provider_config, - &self.factory, - self.profile_override.as_deref(), - &self.profile_files, - ) - .await - .map_err(|err| match err { - ProfileFileError::NoProfilesDefined - | ProfileFileError::ProfileDidNotContainCredentials { .. } => { - CredentialsError::not_loaded(err) - } - _ => CredentialsError::invalid_configuration(format!( - "ProfileFile provider could not be built: {}", - &err - )), - })?; + let inner_provider = build_provider_chain(&self.provider_config, &self.factory) + .await + .map_err(|err| match err { + ProfileFileError::NoProfilesDefined + | ProfileFileError::ProfileDidNotContainCredentials { .. } => { + CredentialsError::not_loaded(err) + } + _ => CredentialsError::invalid_configuration(format!( + "ProfileFile provider could not be built: {}", + &err + )), + })?; let mut creds = match inner_provider .base() .provide_credentials() @@ -208,20 +200,13 @@ impl ProfileFileCredentialsProvider { } } -#[doc(hidden)] -#[derive(Debug)] -pub struct CouldNotReadProfileFile { - pub(crate) path: PathBuf, - pub(crate) cause: std::io::Error, -} - /// An Error building a Credential source from an AWS Profile #[derive(Debug)] #[non_exhaustive] pub enum ProfileFileError { /// The profile was not a valid AWS profile #[non_exhaustive] - CouldNotParseProfile(ProfileParseError), + InvalidProfile(ProfileFileLoadError), /// No profiles existed (the profile was empty) #[non_exhaustive] @@ -273,10 +258,6 @@ pub enum ProfileFileError { /// The name of the provider name: String, }, - - /// A custom profile file location didn't exist or could not be read - #[non_exhaustive] - CouldNotReadProfileFile(CouldNotReadProfileFile), } impl ProfileFileError { @@ -288,11 +269,20 @@ impl ProfileFileError { } } +impl Error for ProfileFileError { + fn source(&self) -> Option<&(dyn Error + 'static)> { + match self { + ProfileFileError::InvalidProfile(err) => Some(err), + _ => None, + } + } +} + impl Display for ProfileFileError { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { - ProfileFileError::CouldNotParseProfile(err) => { - write!(f, "could not parse profile file: {}", err) + ProfileFileError::InvalidProfile(err) => { + write!(f, "invalid profile: {}", err) } ProfileFileError::CredentialLoop { profiles, next } => write!( f, @@ -320,33 +310,10 @@ impl Display for ProfileFileError { "profile `{}` did not contain credential information", profile ), - ProfileFileError::CouldNotReadProfileFile(details) => { - write!( - f, - "Failed to read custom profile file at {:?}", - details.path - ) - } - } - } -} - -impl Error for ProfileFileError { - fn source(&self) -> Option<&(dyn Error + 'static)> { - match self { - ProfileFileError::CouldNotParseProfile(err) => Some(err), - ProfileFileError::CouldNotReadProfileFile(details) => Some(&details.cause), - _ => None, } } } -impl From for ProfileFileError { - fn from(err: ProfileParseError) -> Self { - ProfileFileError::CouldNotParseProfile(err) - } -} - /// Builder for [`ProfileFileCredentialsProvider`] #[derive(Debug, Default)] pub struct Builder { @@ -428,7 +395,10 @@ impl Builder { pub fn build(self) -> ProfileFileCredentialsProvider { let build_span = tracing::debug_span!("build_profile_provider"); let _enter = build_span.enter(); - let conf = self.provider_config.unwrap_or_default(); + let conf = self + .provider_config + .unwrap_or_default() + .with_profile_config(self.profile_files, self.profile_override); let mut named_providers = self.custom_providers.clone(); named_providers .entry("Environment".into()) @@ -467,8 +437,6 @@ impl Builder { region: conf.region(), }, provider_config: conf, - profile_override: self.profile_override, - profile_files: self.profile_files.unwrap_or_default(), } } } @@ -476,12 +444,12 @@ impl Builder { async fn build_provider_chain( provider_config: &ProviderConfig, factory: &NamedProviderFactory, - profile_override: Option<&str>, - profile_files: &ProfileFiles, ) -> Result { - let profile_set = - super::parser::load(&provider_config.fs(), &provider_config.env(), profile_files).await?; - let repr = repr::resolve_chain(&profile_set, profile_override)?; + let profile_set = provider_config + .try_profile() + .await + .map_err(|parse_err| ProfileFileError::InvalidProfile(parse_err.clone()))?; + let repr = repr::resolve_chain(profile_set)?; tracing::info!(chain = ?repr, "constructed abstract provider from config file"); exec::ProviderChain::from_repr(provider_config, repr, factory) } diff --git a/aws/rust-runtime/aws-config/src/profile/credentials/repr.rs b/aws/rust-runtime/aws-config/src/profile/credentials/repr.rs index b0eb3205d..666d74ff4 100644 --- a/aws/rust-runtime/aws-config/src/profile/credentials/repr.rs +++ b/aws/rust-runtime/aws-config/src/profile/credentials/repr.rs @@ -108,10 +108,9 @@ pub(super) struct RoleArn<'a> { } /// Resolve a ProfileChain from a ProfileSet or return an error -pub(super) fn resolve_chain<'a>( - profile_set: &'a ProfileSet, - profile_override: Option<&str>, -) -> Result, ProfileFileError> { +pub(super) fn resolve_chain( + profile_set: &ProfileSet, +) -> Result, ProfileFileError> { // If there are no profiles, allow flowing into the next provider if profile_set.is_empty() { return Err(ProfileFileError::NoProfilesDefined); @@ -120,18 +119,14 @@ pub(super) fn resolve_chain<'a>( // If: // - There is no explicit profile override // - We're looking for the default profile (no configuration) - // - There is not default profile + // - There is no default profile // Then: // - Treat this situation as if no profiles were defined - if profile_override.is_none() - && profile_set.selected_profile() == "default" - && profile_set.get_profile("default").is_none() - { + if profile_set.selected_profile() == "default" && profile_set.get_profile("default").is_none() { tracing::debug!("No default profile defined"); return Err(ProfileFileError::NoProfilesDefined); } - let mut source_profile_name = - profile_override.unwrap_or_else(|| profile_set.selected_profile()); + let mut source_profile_name = profile_set.selected_profile(); let mut visited_profiles = vec![]; let mut chain = vec![]; let base = loop { @@ -435,7 +430,7 @@ mod tests { fn check(test_case: TestCase) { let source = ProfileSet::new(test_case.input.profile, test_case.input.selected_profile); - let actual = resolve_chain(&source, None); + let actual = resolve_chain(&source); let expected = test_case.output; match (expected, actual) { (TestOutput::Error(s), Err(e)) => assert!( diff --git a/aws/rust-runtime/aws-config/src/profile/mod.rs b/aws/rust-runtime/aws-config/src/profile/mod.rs index f1a81cad5..6c0a6a551 100644 --- a/aws/rust-runtime/aws-config/src/profile/mod.rs +++ b/aws/rust-runtime/aws-config/src/profile/mod.rs @@ -16,7 +16,7 @@ mod parser; #[doc(inline)] pub use parser::ProfileParseError; #[doc(inline)] -pub use parser::{load, Profile, ProfileSet, Property}; +pub use parser::{load, Profile, ProfileFileLoadError, ProfileSet, Property}; pub mod app_name; pub mod credentials; diff --git a/aws/rust-runtime/aws-config/src/profile/parser.rs b/aws/rust-runtime/aws-config/src/profile/parser.rs index f80a5842a..0692bf376 100644 --- a/aws/rust-runtime/aws-config/src/profile/parser.rs +++ b/aws/rust-runtime/aws-config/src/profile/parser.rs @@ -9,9 +9,12 @@ use crate::profile::profile_file::ProfileFiles; use aws_types::os_shim_internal::{Env, Fs}; use std::borrow::Cow; use std::collections::HashMap; +use std::error::Error; +use std::fmt::{Display, Formatter}; +use std::path::PathBuf; +use std::sync::Arc; pub use self::parse::ProfileParseError; -use super::credentials::ProfileFileError; mod normalize; mod parse; @@ -56,8 +59,13 @@ pub async fn load( fs: &Fs, env: &Env, profile_files: &ProfileFiles, -) -> Result { - let source = source::load(env, fs, profile_files).await?; + selected_profile_override: Option>, +) -> Result { + let mut source = source::load(env, fs, profile_files).await?; + if let Some(profile) = selected_profile_override { + source.profile = profile; + } + Ok(ProfileSet::parse(source)?) } @@ -69,17 +77,11 @@ pub struct ProfileSet { } impl ProfileSet { - #[doc(hidden)] /// Create a new Profile set directly from a HashMap /// - /// This method creates a ProfileSet directly from a hashmap with no normalization. - /// - /// ## Warning - /// - /// This is probably not what you want! In general, [`load`](load) should be used instead - /// because it will perform input normalization. However, for tests which operate on the - /// normalized profile, this method exists to facilitate easy construction of a ProfileSet - pub fn new( + /// This method creates a ProfileSet directly from a hashmap with no normalization for test purposes. + #[cfg(test)] + pub(crate) fn new( profiles: HashMap>, selected_profile: impl Into>, ) -> Self { @@ -195,6 +197,53 @@ impl Property { } } +/// Failed to read or parse the profile file(s) +#[derive(Debug, Clone)] +pub enum ProfileFileLoadError { + /// The profile could not be parsed + #[non_exhaustive] + ParseError(ProfileParseError), + + /// Attempt to read the AWS config file (`~/.aws/config` by default) failed with a filesystem error. + #[non_exhaustive] + CouldNotReadFile(CouldNotReadProfileFile), +} + +impl Display for ProfileFileLoadError { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match self { + ProfileFileLoadError::ParseError(_err) => { + write!(f, "could not parse profile file") + } + ProfileFileLoadError::CouldNotReadFile(err) => { + write!(f, "could not read file `{}`", err.path.display()) + } + } + } +} + +impl Error for ProfileFileLoadError { + fn source(&self) -> Option<&(dyn Error + 'static)> { + match self { + ProfileFileLoadError::ParseError(err) => Some(err), + ProfileFileLoadError::CouldNotReadFile(details) => Some(&details.cause), + } + } +} + +impl From for ProfileFileLoadError { + fn from(err: ProfileParseError) -> Self { + ProfileFileLoadError::ParseError(err) + } +} + +#[doc(hidden)] +#[derive(Debug, Clone)] +pub struct CouldNotReadProfileFile { + pub(crate) path: PathBuf, + pub(crate) cause: Arc, +} + #[cfg(test)] mod test { use crate::profile::parser::source::{File, Source}; diff --git a/aws/rust-runtime/aws-config/src/profile/parser/parse.rs b/aws/rust-runtime/aws-config/src/profile/parser/parse.rs index 4d5999cee..1d3c3acb0 100644 --- a/aws/rust-runtime/aws-config/src/profile/parser/parse.rs +++ b/aws/rust-runtime/aws-config/src/profile/parser/parse.rs @@ -36,7 +36,7 @@ struct Location { } /// An error encountered while parsing a profile -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct ProfileParseError { /// Location where this error occurred location: Location, diff --git a/aws/rust-runtime/aws-config/src/profile/parser/source.rs b/aws/rust-runtime/aws-config/src/profile/parser/source.rs index e19c5a916..d626394aa 100644 --- a/aws/rust-runtime/aws-config/src/profile/parser/source.rs +++ b/aws/rust-runtime/aws-config/src/profile/parser/source.rs @@ -4,13 +4,16 @@ */ use crate::fs_util::{home_dir, Os}; -use crate::profile::credentials::{CouldNotReadProfileFile, ProfileFileError}; + +use crate::profile::parser::{CouldNotReadProfileFile, ProfileFileLoadError}; use crate::profile::profile_file::{ProfileFile, ProfileFileKind, ProfileFiles}; + use aws_smithy_types::error::display::DisplayErrorContext; use aws_types::os_shim_internal; use std::borrow::Cow; use std::io::ErrorKind; use std::path::{Component, Path, PathBuf}; +use std::sync::Arc; use tracing::{warn, Instrument}; const HOME_EXPANSION_FAILURE_WARNING: &str = @@ -40,7 +43,7 @@ pub(super) async fn load( proc_env: &os_shim_internal::Env, fs: &os_shim_internal::Fs, profile_files: &ProfileFiles, -) -> Result { +) -> Result { let home = home_dir(proc_env, Os::real()); let mut files = Vec::new(); @@ -86,7 +89,7 @@ async fn load_config_file( home_directory: &Option, fs: &os_shim_internal::Fs, environment: &os_shim_internal::Env, -) -> Result { +) -> Result { let (path, kind, contents) = match source { ProfileFile::Default(kind) => { let (path_is_default, path) = environment @@ -127,10 +130,10 @@ async fn load_config_file( let data = match fs.read_to_end(&path).await { Ok(data) => data, Err(e) => { - return Err(ProfileFileError::CouldNotReadProfileFile( + return Err(ProfileFileLoadError::CouldNotReadFile( CouldNotReadProfileFile { path: path.clone(), - cause: e, + cause: Arc::new(e), }, )) } @@ -195,10 +198,10 @@ fn expand_home( #[cfg(test)] mod tests { - use crate::profile::credentials::ProfileFileError; use crate::profile::parser::source::{ expand_home, load, load_config_file, HOME_EXPANSION_FAILURE_WARNING, }; + use crate::profile::parser::ProfileFileLoadError; use crate::profile::profile_file::{ProfileFile, ProfileFileKind, ProfileFiles}; use aws_types::os_shim_internal::{Env, Fs}; use futures_util::FutureExt; @@ -477,7 +480,7 @@ mod tests { .build(); assert!(matches!( load(&env, &fs, &profile_files).await, - Err(ProfileFileError::CouldNotReadProfileFile(_)) + Err(ProfileFileLoadError::CouldNotReadFile(_)) )); } } diff --git a/aws/rust-runtime/aws-config/src/profile/profile_file.rs b/aws/rust-runtime/aws-config/src/profile/profile_file.rs index 76bdfb1fb..ac7dee376 100644 --- a/aws/rust-runtime/aws-config/src/profile/profile_file.rs +++ b/aws/rust-runtime/aws-config/src/profile/profile_file.rs @@ -18,7 +18,7 @@ use std::path::PathBuf; /// will produce errors if they don't exist, while the default config/credentials files paths are /// allowed to not exist even if they're included. /// -/// # Example: Using a custom profile file path for credentials and region +/// # Example: Using a custom profile file path /// /// ``` /// use aws_config::profile::{ProfileFileCredentialsProvider, ProfileFileRegionProvider}; @@ -28,16 +28,8 @@ use std::path::PathBuf; /// let profile_files = ProfileFiles::builder() /// .with_file(ProfileFileKind::Credentials, "some/path/to/credentials-file") /// .build(); -/// let credentials_provider = ProfileFileCredentialsProvider::builder() -/// .profile_files(profile_files.clone()) -/// .build(); -/// let region_provider = ProfileFileRegionProvider::builder() -/// .profile_files(profile_files) -/// .build(); -/// /// let sdk_config = aws_config::from_env() -/// .credentials_provider(credentials_provider) -/// .region(region_provider) +/// .profile_files(profile_files) /// .load() /// .await; /// # } diff --git a/aws/rust-runtime/aws-config/src/profile/region.rs b/aws/rust-runtime/aws-config/src/profile/region.rs index 5ea5f2ded..c58fce403 100644 --- a/aws/rust-runtime/aws-config/src/profile/region.rs +++ b/aws/rust-runtime/aws-config/src/profile/region.rs @@ -9,8 +9,6 @@ use crate::meta::region::{future, ProvideRegion}; use crate::profile::profile_file::ProfileFiles; use crate::profile::ProfileSet; use crate::provider_config::ProviderConfig; -use aws_smithy_types::error::display::DisplayErrorContext; -use aws_types::os_shim_internal::{Env, Fs}; use aws_types::region::Region; /// Load a region from a profile file @@ -39,10 +37,7 @@ use aws_types::region::Region; /// This provider is part of the [default region provider chain](crate::default_provider::region). #[derive(Debug, Default)] pub struct ProfileFileRegionProvider { - fs: Fs, - env: Env, - profile_override: Option, - profile_files: ProfileFiles, + provider_config: ProviderConfig, } /// Builder for [ProfileFileRegionProvider] @@ -74,12 +69,12 @@ impl Builder { /// Build a [ProfileFileRegionProvider] from this builder pub fn build(self) -> ProfileFileRegionProvider { - let conf = self.config.unwrap_or_default(); + let conf = self + .config + .unwrap_or_default() + .with_profile_config(self.profile_files, self.profile_override); ProfileFileRegionProvider { - env: conf.env(), - fs: conf.fs(), - profile_override: self.profile_override, - profile_files: self.profile_files.unwrap_or_default(), + provider_config: conf, } } } @@ -90,10 +85,7 @@ impl ProfileFileRegionProvider { /// To override the selected profile, set the `AWS_PROFILE` environment variable or use the [`Builder`]. pub fn new() -> Self { Self { - fs: Fs::real(), - env: Env::real(), - profile_override: None, - profile_files: ProfileFiles::default(), + provider_config: ProviderConfig::default(), } } @@ -103,26 +95,18 @@ impl ProfileFileRegionProvider { } async fn region(&self) -> Option { - let profile_set = super::parser::load(&self.fs, &self.env, &self.profile_files) - .await - .map_err( - |err| tracing::warn!(err = %DisplayErrorContext(&err), "failed to parse profile"), - ) - .ok()?; + let profile_set = self.provider_config.profile().await?; - resolve_profile_chain_for_region(&profile_set, self.profile_override.as_deref()) + resolve_profile_chain_for_region(profile_set) } } -fn resolve_profile_chain_for_region( - profile_set: &'_ ProfileSet, - profile_override: Option<&str>, -) -> Option { +fn resolve_profile_chain_for_region(profile_set: &'_ ProfileSet) -> Option { if profile_set.is_empty() { return None; } - let mut selected_profile = profile_override.unwrap_or_else(|| profile_set.selected_profile()); + let mut selected_profile = profile_set.selected_profile(); let mut visited_profiles = vec![]; loop { diff --git a/aws/rust-runtime/aws-config/src/profile/retry_config.rs b/aws/rust-runtime/aws-config/src/profile/retry_config.rs index 8ec089afa..4a0ec2b3b 100644 --- a/aws/rust-runtime/aws-config/src/profile/retry_config.rs +++ b/aws/rust-runtime/aws-config/src/profile/retry_config.rs @@ -10,8 +10,6 @@ use crate::provider_config::ProviderConfig; use crate::retry::{ error::RetryConfigError, error::RetryConfigErrorKind, RetryConfigBuilder, RetryMode, }; -use aws_smithy_types::error::display::DisplayErrorContext; -use aws_types::os_shim_internal::{Env, Fs}; use std::str::FromStr; /// Load retry configuration properties from a profile file @@ -39,10 +37,7 @@ use std::str::FromStr; /// This provider is part of the [default retry_config provider chain](crate::default_provider::retry_config). #[derive(Debug, Default)] pub struct ProfileFileRetryConfigProvider { - fs: Fs, - env: Env, - profile_override: Option, - profile_files: ProfileFiles, + provider_config: ProviderConfig, } /// Builder for [ProfileFileRetryConfigProvider] @@ -74,12 +69,12 @@ impl Builder { /// Build a [ProfileFileRetryConfigProvider] from this builder pub fn build(self) -> ProfileFileRetryConfigProvider { - let conf = self.config.unwrap_or_default(); + let conf = self + .config + .unwrap_or_default() + .with_profile_config(self.profile_files, self.profile_override); ProfileFileRetryConfigProvider { - env: conf.env(), - fs: conf.fs(), - profile_override: self.profile_override, - profile_files: self.profile_files.unwrap_or_default(), + provider_config: conf, } } } @@ -89,12 +84,7 @@ impl ProfileFileRetryConfigProvider { /// /// To override the selected profile, set the `AWS_PROFILE` environment variable or use the [Builder]. pub fn new() -> Self { - Self { - fs: Fs::real(), - env: Env::real(), - profile_override: None, - profile_files: Default::default(), - } + Self::default() } /// [Builder] to construct a [ProfileFileRetryConfigProvider] @@ -104,32 +94,14 @@ impl ProfileFileRetryConfigProvider { /// Attempt to create a new RetryConfigBuilder from a profile file. pub async fn retry_config_builder(&self) -> Result { - let profile = match super::parser::load(&self.fs, &self.env, &self.profile_files).await { - Ok(profile) => profile, - Err(err) => { - tracing::warn!(err = %DisplayErrorContext(&err), "failed to parse profile"); - // return an empty builder - return Ok(RetryConfigBuilder::new()); - } - }; - - let selected_profile = self - .profile_override - .as_deref() - .unwrap_or_else(|| profile.selected_profile()); - let selected_profile = match profile.get_profile(selected_profile) { + let profile = match self.provider_config.profile().await { Some(profile) => profile, None => { - // Only warn if the user specified a profile name to use. - if self.profile_override.is_some() { - tracing::warn!("failed to get selected '{}' profile", selected_profile); - } - // return an empty builder + // if there is no profile or the profile is invalid, don't update retry config return Ok(RetryConfigBuilder::new()); } }; - - let max_attempts = match selected_profile.get("max_attempts") { + let max_attempts = match profile.get("max_attempts") { Some(max_attempts) => match max_attempts.parse::() { Ok(max_attempts) if max_attempts == 0 => { return Err(RetryConfigErrorKind::MaxAttemptsMustNotBeZero { @@ -149,7 +121,7 @@ impl ProfileFileRetryConfigProvider { None => None, }; - let retry_mode = match selected_profile.get("retry_mode") { + let retry_mode = match profile.get("retry_mode") { Some(retry_mode) => match RetryMode::from_str(retry_mode) { Ok(retry_mode) => Some(retry_mode), Err(retry_mode_err) => { diff --git a/aws/rust-runtime/aws-config/src/provider_config.rs b/aws/rust-runtime/aws-config/src/provider_config.rs index 5b28e1a03..372c72e71 100644 --- a/aws/rust-runtime/aws-config/src/provider_config.rs +++ b/aws/rust-runtime/aws-config/src/provider_config.rs @@ -8,16 +8,23 @@ use aws_credential_types::time_source::TimeSource; use aws_smithy_async::rt::sleep::{default_async_sleep, AsyncSleep}; use aws_smithy_client::erase::DynConnector; +use aws_smithy_types::error::display::DisplayErrorContext; use aws_types::os_shim_internal::{Env, Fs}; use aws_types::{ http_connector::{ConnectorSettings, HttpConnector}, region::Region, }; +use std::borrow::Cow; use std::fmt::{Debug, Formatter}; use std::sync::Arc; +use tokio::sync::OnceCell; use crate::connector::default_connector; +use crate::profile; + +use crate::profile::profile_file::ProfileFiles; +use crate::profile::{ProfileFileLoadError, ProfileSet}; /// Configuration options for Credential Providers /// @@ -35,6 +42,12 @@ pub struct ProviderConfig { connector: HttpConnector, sleep: Option>, region: Option, + // An AWS profile created from `ProfileFiles` and a `profile_name` + parsed_profile: Arc>>, + // A list of [std::path::Path]s to profile files + profile_files: ProfileFiles, + // An override to use when constructing a `ProfileSet` + profile_name_override: Option>, } impl Debug for ProviderConfig { @@ -63,6 +76,9 @@ impl Default for ProviderConfig { connector, sleep: default_async_sleep(), region: None, + parsed_profile: Default::default(), + profile_files: ProfileFiles::default(), + profile_name_override: None, } } } @@ -77,13 +93,18 @@ impl ProviderConfig { use aws_credential_types::time_source::TestingTimeSource; use std::collections::HashMap; use std::time::UNIX_EPOCH; + let fs = Fs::from_raw_map(HashMap::new()); + let env = Env::from_slice(&[]); Self { - env: Env::from_slice(&[]), - fs: Fs::from_raw_map(HashMap::new()), + parsed_profile: Default::default(), + profile_files: ProfileFiles::default(), + env, + fs, time_source: TimeSource::testing(&TestingTimeSource::new(UNIX_EPOCH)), connector: HttpConnector::Prebuilt(None), sleep: None, region: None, + profile_name_override: None, } } } @@ -123,6 +144,9 @@ impl ProviderConfig { connector: HttpConnector::Prebuilt(None), sleep: None, region: None, + parsed_profile: Default::default(), + profile_files: ProfileFiles::default(), + profile_name_override: None, } } @@ -180,12 +204,57 @@ impl ProviderConfig { self.region.clone() } + pub(crate) async fn try_profile(&self) -> Result<&ProfileSet, &ProfileFileLoadError> { + let parsed_profile = self + .parsed_profile + .get_or_init(|| async { + let profile = profile::load( + &self.fs, + &self.env, + &self.profile_files, + self.profile_name_override.clone(), + ) + .await; + if let Err(err) = profile.as_ref() { + tracing::warn!(err = %DisplayErrorContext(&err), "failed to parse profile") + } + profile + }) + .await; + parsed_profile.as_ref() + } + + pub(crate) async fn profile(&self) -> Option<&ProfileSet> { + self.try_profile().await.ok() + } + /// Override the region for the configuration pub fn with_region(mut self, region: Option) -> Self { self.region = region; self } + /// Override the profile file paths (`~/.aws/config` by default) and name (`default` by default) + pub(crate) fn with_profile_config( + self, + profile_files: Option, + profile_name_override: Option, + ) -> Self { + // if there is no override, then don't clear out `parsed_profile`. + if profile_files.is_none() && profile_name_override.is_none() { + return self; + } + ProviderConfig { + // clear out the profile since we need to reparse it + parsed_profile: Default::default(), + profile_files: profile_files.unwrap_or(self.profile_files), + profile_name_override: profile_name_override + .map(Cow::Owned) + .or(self.profile_name_override), + ..self + } + } + /// Use the [default region chain](crate::default_provider::region) to set the /// region for this configuration /// @@ -200,12 +269,20 @@ impl ProviderConfig { #[doc(hidden)] pub fn with_fs(self, fs: Fs) -> Self { - ProviderConfig { fs, ..self } + ProviderConfig { + parsed_profile: Default::default(), + fs, + ..self + } } #[doc(hidden)] pub fn with_env(self, env: Env) -> Self { - ProviderConfig { env, ..self } + ProviderConfig { + parsed_profile: Default::default(), + env, + ..self + } } #[doc(hidden)] diff --git a/aws/rust-runtime/aws-config/src/test_case.rs b/aws/rust-runtime/aws-config/src/test_case.rs index 32a2f420d..c8a5c1323 100644 --- a/aws/rust-runtime/aws-config/src/test_case.rs +++ b/aws/rust-runtime/aws-config/src/test_case.rs @@ -78,7 +78,7 @@ pub(crate) fn no_traffic_connector() -> DynConnector { } #[derive(Debug)] -struct InstantSleep; +pub(crate) struct InstantSleep; impl AsyncSleep for InstantSleep { fn sleep(&self, _duration: Duration) -> Sleep { Sleep::new(std::future::ready(())) @@ -95,6 +95,7 @@ impl GenericTestResult where T: PartialEq + Debug, { + #[track_caller] pub(crate) fn assert_matches(&self, result: Result, impl Error>) { match (result, &self) { (Ok(actual), GenericTestResult::Ok(expected)) => { @@ -247,6 +248,7 @@ impl TestEnvironment { } } + #[track_caller] fn check_results(&self, result: provider::Result) { self.metadata.result.assert_matches(result); } -- GitLab