Skip to content

Commit

Permalink
Share the HTTP connector between the clients and the credentials prov…
Browse files Browse the repository at this point in the history 
…ider
  • Loading branch information
@rcoh
rcoh committed Jul 25, 2023
1 parent a83fc46 commit 6beb6eb
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 90 deletions.
18 changes: 18 additions & 0 deletions CHANGELOG.next.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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."
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"
9 changes: 3 additions & 6 deletions aws/rust-runtime/aws-config/src/default_provider/app_name.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
117 changes: 47 additions & 70 deletions aws/rust-runtime/aws-config/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -198,13 +199,14 @@ mod loader {
retry_config: Option<RetryConfig>,
sleep: Option<SharedAsyncSleep>,
timeout_config: Option<TimeoutConfig>,
provider_config: Option<ProviderConfig>,
http_connector: Option<HttpConnector>,
profile_name_override: Option<String>,
profile_files_override: Option<ProfileFiles>,
use_fips: Option<bool>,
use_dual_stack: Option<bool>,
time_source: Option<SharedTimeSource>,
fs: Option<Fs>,
env: Option<Env>,
}

impl ConfigLoader {
Expand Down Expand Up @@ -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.
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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 => {
Expand Down Expand Up @@ -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)
Expand All @@ -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;
Expand All @@ -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};

Expand All @@ -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()
Expand Down Expand Up @@ -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]
Expand Down
25 changes: 17 additions & 8 deletions aws/rust-runtime/aws-config/src/provider_config.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,21 @@ impl ProviderConfig {
}
}

/// Initializer for ConfigBag to avoid possibly setting incorrect defaults.
pub(crate) fn init(time_source: SharedTimeSource, sleep: Option<SharedAsyncSleep>) -> 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
Expand Down Expand Up @@ -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<HttpConnector>) -> Self {
ProviderConfig {
connector: HttpConnector::Prebuilt(Some(connector)),
connector: connector.into(),
..self
}
}
Expand Down
12 changes: 6 additions & 6 deletions aws/rust-runtime/aws-config/src/test_case.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand Down

0 comments on commit 6beb6eb

Please sign in to comment.