Unverified Commit 88afcf8c authored by Russell Cohen's avatar Russell Cohen Committed by GitHub
Browse files

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: default avatarZelda Hessler <zhessler@amazon.com>

* Apply suggestions from code review

Co-authored-by: default avatarZelda Hessler <zhessler@amazon.com>

* fix name of field

Co-authored-by: default avatarZelda Hessler <zhessler@amazon.com>
parent 5f075609
Loading
Loading
Loading
Loading
+43 −0
Original line number Diff line number Diff line
@@ -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;
/// ```
"""
+24 −1
Original line number Diff line number Diff line
@@ -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")]);
+12 −10
Original line number Diff line number Diff line
@@ -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::<EndpointMode>()
                        .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::<EndpointMode>()
                        .map_err(BuildError::invalid_endpoint_mode)?
                } else {
+0 −12
Original line number Diff line number Diff line
@@ -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<dyn Error + Send + Sync + 'static>),
}
@@ -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<Box<dyn Error + Send + Sync + 'static>>,
    ) -> 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()),
        }
    }
+108 −6
Original line number Diff line number Diff line
@@ -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<TimeoutConfig>,
        provider_config: Option<ProviderConfig>,
        http_connector: Option<HttpConnector>,
        profile_name_override: Option<String>,
        profile_files_override: Option<ProfileFiles>,
    }

    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<String>) -> 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
                    )),
                }
            });
        }
    }
}
Loading