diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 7b24ac49ef..36ea8756a5 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -664,3 +664,21 @@ message = "The `alb_health_check` module has been moved out of the `plugin` modu references = ["smithy-rs#2865"] meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "server" } author = "david-perez" + +[[aws-sdk-rust]] +message = "Credential providers now share the HTTP connector used by the SDK. As a result, the `ConfigLoader::configure(...)` method has been removed. If you want to keep a separate connector for clients, use `::ConfigBuilder::http_connector` when constructing the client." +references = ["aws-sdk-rust#579", "aws-sdk-rust#338"] +meta = { "breaking" = true, "tada" = false, "bug" = false } +author = "rcoh" + +[[aws-sdk-rust]] +message = "When creating the default credentials chain with `aws_config::from_env()`, the internal providers now use the configured sleep and time source implmentation. Prior to this fix, they always used the default implementation." +references = ["aws-sdk-rust#59"] +meta = { "breaking" = false, "tada" = false, "bug" = true } +author = "rcoh" + +[[aws-sdk-rust]] +message = "ProviderConfig::with_http_connector now accepts an `HttpConnector` instead of a `DynConnector`. This allows HTTP connectors to be configured which correctly respect timeouts." +references = ["aws-sdk-rust#579", "aws-sdk-rust#338"] +meta = { "breaking" = true, "tada" = false, "bug" = false } +author = "rcoh" 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 7d91c6d363..236def6a87 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 @@ -118,12 +118,9 @@ mod tests { 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()), - ) + .fs(fs) + .sleep_impl(InstantSleep) + .http_connector(no_traffic_connector()) .profile_name("custom") .profile_files( ProfileFiles::builder() diff --git a/aws/rust-runtime/aws-config/src/lib.rs b/aws/rust-runtime/aws-config/src/lib.rs index f9cfdce2e2..fe6d33ea1b 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 aws_smithy_types::timeout::TimeoutConfig; use aws_types::app_name::AppName; use aws_types::docs_for; + use aws_types::os_shim_internal::{Env, Fs}; use aws_types::SdkConfig; use crate::connector::default_connector; @@ -198,13 +199,14 @@ mod loader { retry_config: Option, sleep: Option, timeout_config: Option, - provider_config: Option, http_connector: Option, profile_name_override: Option, profile_files_override: Option, use_fips: Option, use_dual_stack: Option, time_source: Option, + fs: Option, + env: Option, } impl ConfigLoader { @@ -513,30 +515,6 @@ mod loader { self } - /// Set configuration for all sub-loaders (credentials, region etc.) - /// - /// 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. - /// - /// # 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() - /// .https_only() - /// .enable_http1() - /// .build(); - /// let provider_config = ProviderConfig::default().with_tcp_connector(custom_https_connector); - /// let shared_config = aws_config::from_env().configure(provider_config).load().await; - /// # } - /// ``` - pub fn configure(mut self, provider_config: ProviderConfig) -> Self { - self.provider_config = Some(provider_config); - self - } - /// Load the default configuration chain /// /// If fields have been overridden during builder construction, the override values will be used. @@ -547,17 +525,30 @@ 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 mut poisoned_conf = self - .provider_config - .unwrap_or_default() - .with_profile_config(self.profile_files_override, self.profile_name_override); - if let Some(time_source) = &self.time_source { - poisoned_conf = poisoned_conf.with_time_source(time_source.clone()); - } - if let Some(sleep) = &self.sleep { - poisoned_conf = poisoned_conf.with_sleep(sleep.clone()); - } - let conf = poisoned_conf; + let sleep_impl = if self.sleep.is_some() { + self.sleep + } else { + if default_async_sleep().is_none() { + tracing::warn!( + "An implementation of AsyncSleep was requested by calling default_async_sleep \ + but no default was set. + This happened when ConfigLoader::load was called during Config construction. \ + You can fix this by setting a sleep_impl on the ConfigLoader before calling \ + load or by enabling the rt-tokio feature" + ); + } + default_async_sleep() + }; + let http_connector = self + .http_connector + .unwrap_or_else(|| HttpConnector::ConnectorFn(Arc::new(default_connector))); + + let ts = self.time_source.unwrap_or_default(); + let conf = ProviderConfig::init(ts.clone(), sleep_impl.clone()) + .with_profile_config(self.profile_files_override, self.profile_name_override) + .with_fs(self.fs.unwrap_or_default()) + .with_http_connector(http_connector.clone()) + .with_env(self.env.unwrap_or_default()); let region = if let Some(provider) = self.region { provider.region().await } else { @@ -586,21 +577,6 @@ mod loader { .await }; - let sleep_impl = if self.sleep.is_some() { - self.sleep - } else { - if default_async_sleep().is_none() { - tracing::warn!( - "An implementation of AsyncSleep was requested by calling default_async_sleep \ - but no default was set. - This happened when ConfigLoader::load was called during Config construction. \ - You can fix this by setting a sleep_impl on the ConfigLoader before calling \ - load or by enabling the rt-tokio feature" - ); - } - default_async_sleep() - }; - let timeout_config = if let Some(timeout_config) = self.timeout_config { timeout_config } else { @@ -610,10 +586,6 @@ mod loader { .await }; - let http_connector = self - .http_connector - .unwrap_or_else(|| HttpConnector::ConnectorFn(Arc::new(default_connector))); - let credentials_provider = match self.credentials_provider { CredentialsProviderOption::Set(provider) => Some(provider), CredentialsProviderOption::NotSet => { @@ -648,8 +620,6 @@ mod loader { use_dual_stack_provider(&conf).await }; - let ts = self.time_source.unwrap_or_default(); - let mut builder = SdkConfig::builder() .region(region) .retry_config(retry_config) @@ -668,6 +638,19 @@ mod loader { } } + #[cfg(test)] + impl ConfigLoader { + pub(crate) fn env(mut self, env: Env) -> Self { + self.env = Some(env); + self + } + + pub(crate) fn fs(mut self, fs: Fs) -> Self { + self.fs = Some(fs); + self + } + } + #[cfg(test)] mod test { use aws_credential_types::provider::ProvideCredentials; @@ -679,7 +662,6 @@ mod loader { use tracing_test::traced_test; use crate::profile::profile_file::{ProfileFileKind, ProfileFiles}; - use crate::provider_config::ProviderConfig; use crate::test_case::{no_traffic_connector, InstantSleep}; use crate::{from_env, ConfigLoader}; @@ -695,13 +677,10 @@ mod loader { 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())), - ) + .sleep_impl(TokioSleep::new()) + .http_connector(DynConnector::new(NeverConnector::new())) + .env(env) + .fs(fs) .profile_name("custom") .profile_files( ProfileFiles::builder() @@ -741,11 +720,9 @@ mod loader { } fn base_conf() -> ConfigLoader { - from_env().configure( - ProviderConfig::empty() - .with_sleep(InstantSleep) - .with_http_connector(no_traffic_connector()), - ) + from_env() + .sleep_impl(InstantSleep) + .http_connector(no_traffic_connector()) } #[tokio::test] diff --git a/aws/rust-runtime/aws-config/src/provider_config.rs b/aws/rust-runtime/aws-config/src/provider_config.rs index af932c6361..deb63542fb 100644 --- a/aws/rust-runtime/aws-config/src/provider_config.rs +++ b/aws/rust-runtime/aws-config/src/provider_config.rs @@ -150,6 +150,21 @@ impl ProviderConfig { } } + /// Initializer for ConfigBag to avoid possibly setting incorrect defaults. + pub(crate) fn init(time_source: SharedTimeSource, sleep: Option) -> Self { + Self { + parsed_profile: Default::default(), + profile_files: ProfileFiles::default(), + env: Env::default(), + fs: Fs::default(), + time_source, + connector: HttpConnector::Prebuilt(None), + sleep, + region: None, + profile_name_override: None, + } + } + /// Create a default provider config with the region region automatically loaded from the default chain. /// /// # Examples @@ -299,15 +314,9 @@ impl ProviderConfig { } /// Override the HTTPS connector for this configuration - /// - /// **Warning**: Use of this method will prevent you from taking advantage of the HTTP connect timeouts. - /// Consider [`ProviderConfig::with_tcp_connector`]. - /// - /// # Stability - /// This method is expected to change to support HTTP configuration. - pub fn with_http_connector(self, connector: DynConnector) -> Self { + pub fn with_http_connector(self, connector: impl Into) -> Self { ProviderConfig { - connector: HttpConnector::Prebuilt(Some(connector)), + connector: connector.into(), ..self } } diff --git a/aws/rust-runtime/aws-config/src/test_case.rs b/aws/rust-runtime/aws-config/src/test_case.rs index c04cd4ec0e..6a0d37f259 100644 --- a/aws/rust-runtime/aws-config/src/test_case.rs +++ b/aws/rust-runtime/aws-config/src/test_case.rs @@ -8,13 +8,13 @@ use crate::provider_config::ProviderConfig; use aws_credential_types::provider::{self, ProvideCredentials}; use aws_smithy_async::rt::sleep::{AsyncSleep, Sleep}; use aws_smithy_client::dvr::{NetworkTraffic, RecordingConnection, ReplayingConnection}; -use aws_smithy_client::erase::DynConnector; use aws_types::os_shim_internal::{Env, Fs}; use serde::Deserialize; use crate::connector::default_connector; use aws_smithy_async::test_util::instant_time_and_sleep; +use aws_smithy_client::http_connector::HttpConnector; use aws_smithy_types::error::display::DisplayErrorContext; use std::collections::HashMap; use std::env; @@ -79,8 +79,8 @@ pub(crate) struct TestEnvironment { } /// Connector which expects no traffic -pub(crate) fn no_traffic_connector() -> DynConnector { - DynConnector::new(ReplayingConnection::new(vec![])) +pub(crate) fn no_traffic_connector() -> HttpConnector { + ReplayingConnection::new(vec![]).into() } #[derive(Debug)] @@ -234,7 +234,7 @@ impl TestEnvironment { let provider_config = ProviderConfig::empty() .with_fs(fs.clone()) .with_env(env.clone()) - .with_http_connector(DynConnector::new(connector.clone())) + .with_http_connector(connector.clone()) .with_sleep(sleep) .with_time_source(timeource) .load_default_region() @@ -277,7 +277,7 @@ impl TestEnvironment { let config = self .provider_config .clone() - .with_http_connector(DynConnector::new(live_connector.clone())); + .with_http_connector(live_connector.clone()); let provider = make_provider(config).await; let result = provider.provide_credentials().await; std::fs::write( @@ -302,7 +302,7 @@ impl TestEnvironment { let config = self .provider_config .clone() - .with_http_connector(DynConnector::new(recording_connector.clone())); + .with_http_connector(recording_connector.clone()); let provider = make_provider(config).await; let result = provider.provide_credentials().await; std::fs::write(