From 4a8ebdb5dd949f4518a8737574c280d256f9c60c Mon Sep 17 00:00:00 2001 From: Russell Cohen Date: Mon, 27 Nov 2023 11:42:00 -0500 Subject: [PATCH 1/7] wip --- .../aws-config/src/imds/client.rs | 6 + .../s3/tests/service_timeout_overrides.rs | 10 +- .../client/FluentClientGenerator.kt | 2 + .../src/client/behavior_version.rs | 23 +++- .../aws-smithy-runtime/src/client/defaults.rs | 27 ++++- .../src/client/http/hyper_014.rs | 111 +++++++++++++----- 6 files changed, 146 insertions(+), 33 deletions(-) diff --git a/aws/rust-runtime/aws-config/src/imds/client.rs b/aws/rust-runtime/aws-config/src/imds/client.rs index 154225b29e..3fba407cbd 100644 --- a/aws/rust-runtime/aws-config/src/imds/client.rs +++ b/aws/rust-runtime/aws-config/src/imds/client.rs @@ -990,12 +990,18 @@ pub(crate) mod test { use crate::imds::client::ImdsError; use aws_smithy_types::error::display::DisplayErrorContext; use std::time::SystemTime; + let (_guard, _) = capture_test_logs(); let client = Client::builder() // 240.* can never be resolved .endpoint("http://240.0.0.0") .expect("valid uri") .build(); + // preload the connector so that doesn't impact timing + let _resp = client + .get("/latest/metadata") + .await + .expect_err("240.0.0.0 will never resolve"); let now = SystemTime::now(); let resp = client .get("/latest/metadata") diff --git a/aws/sdk/integration-tests/s3/tests/service_timeout_overrides.rs b/aws/sdk/integration-tests/s3/tests/service_timeout_overrides.rs index 9e4808118c..91e6ea3088 100644 --- a/aws/sdk/integration-tests/s3/tests/service_timeout_overrides.rs +++ b/aws/sdk/integration-tests/s3/tests/service_timeout_overrides.rs @@ -6,6 +6,8 @@ use aws_credential_types::provider::SharedCredentialsProvider; use aws_credential_types::Credentials; use aws_smithy_async::rt::sleep::{SharedAsyncSleep, TokioSleep}; +use aws_smithy_runtime::client::http::test_util::NeverClient; +use aws_smithy_runtime::test_util::capture_test_logs::capture_test_logs; use aws_smithy_runtime_api::client::result::SdkError; use aws_smithy_types::timeout::TimeoutConfig; use aws_types::region::Region; @@ -16,6 +18,7 @@ use tokio::time::Instant; /// Use a 5 second operation timeout on SdkConfig and a 0ms connect timeout on the service config #[tokio::test] async fn timeouts_can_be_set_by_service() { + let (_guard, _) = capture_test_logs(); let sdk_config = SdkConfig::builder() .credentials_provider(SharedCredentialsProvider::new(Credentials::for_tests())) .region(Region::from_static("us-east-1")) @@ -25,6 +28,7 @@ async fn timeouts_can_be_set_by_service() { .operation_timeout(Duration::from_secs(5)) .build(), ) + .http_client(NeverClient::new()) // ip that .endpoint_url( // Emulate a connect timeout error by hitting an unroutable IP @@ -34,7 +38,7 @@ async fn timeouts_can_be_set_by_service() { let config = aws_sdk_s3::config::Builder::from(&sdk_config) .timeout_config( TimeoutConfig::builder() - .connect_timeout(Duration::from_secs(0)) + .operation_timeout(Duration::from_secs(0)) .build(), ) .build(); @@ -48,7 +52,9 @@ async fn timeouts_can_be_set_by_service() { .await .expect_err("unroutable IP should timeout"); match err { - SdkError::DispatchFailure(err) => assert!(err.is_timeout()), + SdkError::TimeoutError(_err) => + /* ok */ + {} // if the connect timeout is not respected, this times out after 1 second because of the operation timeout with `SdkError::Timeout` _other => panic!("unexpected error: {:?}", _other), } diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt index 3865d5a0ab..bb1f235146 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt @@ -45,6 +45,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.withBlockTemplate import software.amazon.smithy.rust.codegen.core.rustlang.writable import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType.Companion.preludeScope +import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType.Companion.smithyRuntimeApiClient import software.amazon.smithy.rust.codegen.core.smithy.RustCrate import software.amazon.smithy.rust.codegen.core.smithy.RustSymbolProvider import software.amazon.smithy.rust.codegen.core.smithy.customize.writeCustomizations @@ -158,6 +159,7 @@ class FluentClientGenerator( *preludeScope, "Arc" to RuntimeType.Arc, "base_client_runtime_plugins" to baseClientRuntimePluginsFn(codegenContext), + "HttpClient" to smithyRuntimeApiClient(runtimeConfig).resolve("client::http::HttpClient"), "BoxError" to RuntimeType.boxError(runtimeConfig), "client_docs" to writable { customizations.forEach { diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/behavior_version.rs b/rust-runtime/aws-smithy-runtime-api/src/client/behavior_version.rs index c44043fe73..89b0bbbea1 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/behavior_version.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/behavior_version.rs @@ -11,7 +11,15 @@ /// compatible. For example, a change which introduces new default timeouts or a new retry-mode for /// all operations might be the ideal behavior but could break existing applications. #[derive(Debug, Clone)] -pub struct BehaviorVersion {} +pub struct BehaviorVersion { + version: Version, +} + +#[derive(PartialOrd, Ord, PartialEq, Eq)] +enum Version { + V2023_11_09, + V2023_11_27, +} impl BehaviorVersion { /// This method will always return the latest major version. @@ -25,13 +33,22 @@ impl BehaviorVersion { /// /// The latest version is currently [`BehaviorVersion::v2023_11_09`] pub fn latest() -> Self { - Self {} + Self { + version: Version::V2023_11_27, + } } /// This method returns the behavior configuration for November 9th, 2023 /// /// When a new behavior major version is released, this method will be deprecated. + #[deprecated(note = "v2023_11_27 has been released")] pub fn v2023_11_09() -> Self { - Self {} + Self { + version: Version::V2023_11_09, + } + } + + pub fn is_at_least_2023_11_27(&self) -> bool { + self.version >= Version::V2023_11_27 } } diff --git a/rust-runtime/aws-smithy-runtime/src/client/defaults.rs b/rust-runtime/aws-smithy-runtime/src/client/defaults.rs index ca2d06a387..1325d5e199 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/defaults.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/defaults.rs @@ -63,6 +63,22 @@ pub fn default_http_client_plugin() -> Option { }) } +/// Runtime plugin that provides a default connector. +/// +/// This connector is lazily created on first-user +pub fn default_http_client_plugin_lazy() -> Option { + let _default: Option = None; + #[cfg(feature = "connector-hyper-0-14-x")] + let _default = crate::client::http::hyper_014::default_client_lazy(); + + _default.map(|default| { + default_plugin("default_http_client_plugin", |components| { + components.with_http_client(Some(default)) + }) + .into_shared() + }) +} + /// Runtime plugin that provides a default async sleep implementation. pub fn default_sleep_impl_plugin() -> Option { default_async_sleep().map(|default| { @@ -252,8 +268,17 @@ impl DefaultPluginParams { pub fn default_plugins( params: DefaultPluginParams, ) -> impl IntoIterator { + let http_client_plugin = match params + .behavior_version + .unwrap_or(BehaviorVersion::latest()) + .is_at_least_2023_11_27() + { + true => default_http_client_plugin_lazy(), + false => default_http_client_plugin(), + }; + [ - default_http_client_plugin(), + http_client_plugin, default_identity_cache_plugin(), default_retry_config_plugin( params diff --git a/rust-runtime/aws-smithy-runtime/src/client/http/hyper_014.rs b/rust-runtime/aws-smithy-runtime/src/client/http/hyper_014.rs index 344c60714a..2a26bd0490 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/http/hyper_014.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/http/hyper_014.rs @@ -36,38 +36,42 @@ use tokio::io::{AsyncRead, AsyncWrite}; mod default_connector { use aws_smithy_async::rt::sleep::SharedAsyncSleep; use aws_smithy_runtime_api::client::http::HttpConnectorSettings; + use hyper_0_14::client::HttpConnector; + use hyper_rustls::HttpsConnector; // Creating a `with_native_roots` HTTP client takes 300ms on OS X. Cache this so that we // don't need to repeatedly incur that cost. - static HTTPS_NATIVE_ROOTS: once_cell::sync::Lazy< + pub(crate) static HTTPS_NATIVE_ROOTS: once_cell::sync::Lazy< hyper_rustls::HttpsConnector, - > = once_cell::sync::Lazy::new(|| { + > = once_cell::sync::Lazy::new(|| std::thread::spawn(default_tls).join().unwrap()); + + fn default_tls() -> HttpsConnector { use hyper_rustls::ConfigBuilderExt; hyper_rustls::HttpsConnectorBuilder::new() - .with_tls_config( - rustls::ClientConfig::builder() - .with_cipher_suites(&[ - // TLS1.3 suites - rustls::cipher_suite::TLS13_AES_256_GCM_SHA384, - rustls::cipher_suite::TLS13_AES_128_GCM_SHA256, - // TLS1.2 suites - rustls::cipher_suite::TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, - rustls::cipher_suite::TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, - rustls::cipher_suite::TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, - rustls::cipher_suite::TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, - rustls::cipher_suite::TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, - ]) - .with_safe_default_kx_groups() - .with_safe_default_protocol_versions() - .expect("Error with the TLS configuration. Please file a bug report under https://github.com/smithy-lang/smithy-rs/issues.") - .with_native_roots() - .with_no_client_auth() - ) - .https_or_http() - .enable_http1() - .enable_http2() - .build() - }); + .with_tls_config( + rustls::ClientConfig::builder() + .with_cipher_suites(&[ + // TLS1.3 suites + rustls::cipher_suite::TLS13_AES_256_GCM_SHA384, + rustls::cipher_suite::TLS13_AES_128_GCM_SHA256, + // TLS1.2 suites + rustls::cipher_suite::TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, + rustls::cipher_suite::TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, + rustls::cipher_suite::TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, + rustls::cipher_suite::TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, + rustls::cipher_suite::TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, + ]) + .with_safe_default_kx_groups() + .with_safe_default_protocol_versions() + .expect("Error with the TLS configuration. Please file a bug report under https://github.com/smithy-lang/smithy-rs/issues.") + .with_native_roots() + .with_no_client_auth() + ) + .https_or_http() + .enable_http1() + .enable_http2() + .build() + } pub(super) fn base( settings: &HttpConnectorSettings, @@ -121,6 +125,20 @@ pub fn default_client() -> Option { } } +/// Creates a hyper-backed HTTPS client from defaults depending on what cargo features are activated. +pub fn default_client_lazy() -> Option { + #[cfg(feature = "tls-rustls")] + { + tracing::trace!("creating a new default hyper 0.14.x client"); + Some(HyperClientBuilder::new().build_https()) + } + #[cfg(not(feature = "tls-rustls"))] + { + tracing::trace!("no default connector available"); + None + } +} + /// [`HttpConnector`] that uses [`hyper_0_14`] to make HTTP requests. /// /// This connector also implements socket connect and read timeouts. @@ -490,7 +508,14 @@ where .connector_settings(settings.clone()); builder.set_sleep_impl(components.sleep_impl()); + let start = components.time_source().map(|ts| ts.now()); let tcp_connector = (self.tcp_connector_fn)(); + let end = components.time_source().map(|ts| ts.now()); + let delta = match (start, end) { + (Some(start), Some(end)) => end.duration_since(start).ok(), + _ => None, + }; + tracing::debug!("new TCP connector created in {:?}", delta); let connector = SharedHttpConnector::new(builder.build(tcp_connector)); cache.insert(key.clone(), connector); } @@ -541,6 +566,12 @@ impl HyperClientBuilder { self.build(default_connector::https()) } + /// Create a [`HyperConnector`] with the default rustls HTTPS implementation. + #[cfg(feature = "tls-rustls")] + pub fn build_https_lazy(self) -> SharedHttpClient { + self.build_with_fn(default_connector::https) + } + /// Create a [`SharedHttpClient`] from this builder and a given connector. /// #[cfg_attr( @@ -562,7 +593,6 @@ impl HyperClientBuilder { }) } - #[cfg(all(test, feature = "test-util"))] fn build_with_fn(self, tcp_connector_fn: F) -> SharedHttpClient where F: Fn() -> C + Send + Sync + 'static, @@ -952,14 +982,18 @@ mod timeout_middleware { mod test { use super::*; use crate::client::http::test_util::NeverTcpConnector; + use aws_smithy_async::assert_elapsed; + use aws_smithy_async::time::SystemTimeSource; use aws_smithy_runtime_api::client::runtime_components::RuntimeComponentsBuilder; use http::Uri; use hyper_0_14::client::connect::{Connected, Connection}; + use once_cell::sync::Lazy; use std::io::{Error, ErrorKind}; use std::pin::Pin; use std::sync::atomic::{AtomicU32, Ordering}; use std::sync::Arc; use std::task::{Context, Poll}; + use std::time::SystemTime; use tokio::io::{AsyncRead, AsyncWrite, ReadBuf}; #[tokio::test] @@ -1015,6 +1049,29 @@ mod test { assert_eq!(4, creation_count.load(Ordering::Relaxed)); } + #[tokio::test] + #[cfg(feature = "tls-rustls")] + async fn dont_preload_connector() { + let _client = HyperClientBuilder::default().build_https(); + assert!(Lazy::get(&default_connector::HTTPS_NATIVE_ROOTS).is_none()); + } + + #[tokio::test] + #[cfg(feature = "tls-rustls")] + async fn connector_loading_time() { + let client = HyperClientBuilder::default().build_https(); + let now = SystemTime::now(); + let conn = client.http_connector( + &HttpConnectorSettings::default(), + &RuntimeComponentsBuilder::for_tests() + .with_time_source(Some(SystemTimeSource::new())) + .build() + .unwrap(), + ); + let dur = now.elapsed().unwrap(); + assert_eq!(dur, Duration::from_millis(300)); + } + #[tokio::test] async fn hyper_io_error() { let connector = TestConnection { From e964711eb12e89fc6167f657ac22ae43363f3784 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Mon, 27 Nov 2023 15:17:43 -0800 Subject: [PATCH 2/7] Finish making HTTP cert initialization lazy --- .../s3/tests/service_timeout_overrides.rs | 8 +-- .../client/FluentClientGenerator.kt | 2 - .../src/client/behavior_version.rs | 57 ++++++++++++++++--- .../aws-smithy-runtime/src/client/defaults.rs | 13 +++-- .../src/client/http/hyper_014.rs | 53 ++++++----------- .../src/client/http/test_util/never.rs | 2 + 6 files changed, 79 insertions(+), 56 deletions(-) diff --git a/aws/sdk/integration-tests/s3/tests/service_timeout_overrides.rs b/aws/sdk/integration-tests/s3/tests/service_timeout_overrides.rs index 91e6ea3088..40a4fb0578 100644 --- a/aws/sdk/integration-tests/s3/tests/service_timeout_overrides.rs +++ b/aws/sdk/integration-tests/s3/tests/service_timeout_overrides.rs @@ -15,7 +15,7 @@ use aws_types::SdkConfig; use std::time::Duration; use tokio::time::Instant; -/// Use a 5 second operation timeout on SdkConfig and a 0ms connect timeout on the service config +/// Use a 5 second operation timeout on SdkConfig and a 0ms operation timeout on the service config #[tokio::test] async fn timeouts_can_be_set_by_service() { let (_guard, _) = capture_test_logs(); @@ -52,10 +52,8 @@ async fn timeouts_can_be_set_by_service() { .await .expect_err("unroutable IP should timeout"); match err { - SdkError::TimeoutError(_err) => - /* ok */ - {} - // if the connect timeout is not respected, this times out after 1 second because of the operation timeout with `SdkError::Timeout` + SdkError::TimeoutError(_err) => { /* ok */ } + // if the connect timeout is not respected, this times out after 5 seconds because of the operation timeout with `SdkError::Timeout` _other => panic!("unexpected error: {:?}", _other), } // there should be a 0ms timeout, we gotta set some stuff up. Just want to make sure diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt index bb1f235146..3865d5a0ab 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt @@ -45,7 +45,6 @@ import software.amazon.smithy.rust.codegen.core.rustlang.withBlockTemplate import software.amazon.smithy.rust.codegen.core.rustlang.writable import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType.Companion.preludeScope -import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType.Companion.smithyRuntimeApiClient import software.amazon.smithy.rust.codegen.core.smithy.RustCrate import software.amazon.smithy.rust.codegen.core.smithy.RustSymbolProvider import software.amazon.smithy.rust.codegen.core.smithy.customize.writeCustomizations @@ -159,7 +158,6 @@ class FluentClientGenerator( *preludeScope, "Arc" to RuntimeType.Arc, "base_client_runtime_plugins" to baseClientRuntimePluginsFn(codegenContext), - "HttpClient" to smithyRuntimeApiClient(runtimeConfig).resolve("client::http::HttpClient"), "BoxError" to RuntimeType.boxError(runtimeConfig), "client_docs" to writable { customizations.forEach { diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/behavior_version.rs b/rust-runtime/aws-smithy-runtime-api/src/client/behavior_version.rs index 89b0bbbea1..df8c4e3f1a 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/behavior_version.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/behavior_version.rs @@ -3,19 +3,19 @@ * SPDX-License-Identifier: Apache-2.0 */ -//! Behavior Major version of the client +//! Behavior major-version of the client /// Behavior major-version of the client /// /// Over time, new best-practice behaviors are introduced. However, these behaviors might not be backwards /// compatible. For example, a change which introduces new default timeouts or a new retry-mode for /// all operations might be the ideal behavior but could break existing applications. -#[derive(Debug, Clone)] +#[derive(Clone, Debug, PartialOrd, Ord, PartialEq, Eq)] pub struct BehaviorVersion { version: Version, } -#[derive(PartialOrd, Ord, PartialEq, Eq)] +#[derive(Clone, Debug, PartialOrd, Ord, PartialEq, Eq)] enum Version { V2023_11_09, V2023_11_27, @@ -38,17 +38,58 @@ impl BehaviorVersion { } } - /// This method returns the behavior configuration for November 9th, 2023 + /// This method returns the behavior configuration for November 27th, 2023 /// /// When a new behavior major version is released, this method will be deprecated. - #[deprecated(note = "v2023_11_27 has been released")] - pub fn v2023_11_09() -> Self { + /// + /// # Changes + /// + /// The default HTTP client is now lazy initialized so that the price of loading + /// trusted certificates isn't paid when overriding the default client. This means + /// that the first request after client initialization will be slower for the default + /// client as it needs to load those certs upon that first request. + /// + /// This first request cost can be eliminated by priming the client immediately after + /// initialization, or by overriding the default HTTP client with one that doesn't lazy + /// initialize. + pub const fn v2023_11_27() -> Self { + Self { + version: Version::V2023_11_27, + } + } + + /// This method returns the behavior configuration for November 9th, 2023 + /// + /// This behavior version has been superceded by v2023_11_27. + #[deprecated( + note = "Superceded by v2023_11_27. See doc comment on v2023_11_27 for details on behavior changes." + )] + pub const fn v2023_11_09() -> Self { Self { version: Version::V2023_11_09, } } - pub fn is_at_least_2023_11_27(&self) -> bool { - self.version >= Version::V2023_11_27 + /// True if this version is greater than or equal to the given `version`. + pub fn is_at_least(&self, version: BehaviorVersion) -> bool { + self >= &version + } + + /// True if this version is before the given `version`. + pub fn is_before(&self, version: BehaviorVersion) -> bool { + self < &version + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[allow(deprecated)] + #[test] + fn comparison() { + assert!(BehaviorVersion::v2023_11_09() < BehaviorVersion::v2023_11_27()); + assert!(BehaviorVersion::v2023_11_27() == BehaviorVersion::v2023_11_27()); + assert!(BehaviorVersion::v2023_11_27() > BehaviorVersion::v2023_11_09()); } } diff --git a/rust-runtime/aws-smithy-runtime/src/client/defaults.rs b/rust-runtime/aws-smithy-runtime/src/client/defaults.rs index 1325d5e199..5402cce029 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/defaults.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/defaults.rs @@ -262,17 +262,20 @@ impl DefaultPluginParams { self.behavior_version = Some(version); self } + + fn behavior_version(&self) -> BehaviorVersion { + self.behavior_version + .clone() + .unwrap_or(BehaviorVersion::latest()) + } } /// All default plugins. pub fn default_plugins( params: DefaultPluginParams, ) -> impl IntoIterator { - let http_client_plugin = match params - .behavior_version - .unwrap_or(BehaviorVersion::latest()) - .is_at_least_2023_11_27() - { + // v2023_11_27 introduced lazy HTTP clients + let http_client_plugin = match params.behavior_version() >= BehaviorVersion::v2023_11_27() { true => default_http_client_plugin_lazy(), false => default_http_client_plugin(), }; diff --git a/rust-runtime/aws-smithy-runtime/src/client/http/hyper_014.rs b/rust-runtime/aws-smithy-runtime/src/client/http/hyper_014.rs index 2a26bd0490..cf63887f9a 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/http/hyper_014.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/http/hyper_014.rs @@ -43,7 +43,7 @@ mod default_connector { // don't need to repeatedly incur that cost. pub(crate) static HTTPS_NATIVE_ROOTS: once_cell::sync::Lazy< hyper_rustls::HttpsConnector, - > = once_cell::sync::Lazy::new(|| std::thread::spawn(default_tls).join().unwrap()); + > = once_cell::sync::Lazy::new(default_tls); fn default_tls() -> HttpsConnector { use hyper_rustls::ConfigBuilderExt; @@ -130,7 +130,7 @@ pub fn default_client_lazy() -> Option { #[cfg(feature = "tls-rustls")] { tracing::trace!("creating a new default hyper 0.14.x client"); - Some(HyperClientBuilder::new().build_https()) + Some(HyperClientBuilder::new().build_https_lazy()) } #[cfg(not(feature = "tls-rustls"))] { @@ -511,11 +511,11 @@ where let start = components.time_source().map(|ts| ts.now()); let tcp_connector = (self.tcp_connector_fn)(); let end = components.time_source().map(|ts| ts.now()); - let delta = match (start, end) { - (Some(start), Some(end)) => end.duration_since(start).ok(), - _ => None, - }; - tracing::debug!("new TCP connector created in {:?}", delta); + if let (Some(start), Some(end)) = (start, end) { + if let Ok(elapsed) = end.duration_since(start) { + tracing::debug!("new TCP connector created in {:?}", elapsed); + } + } let connector = SharedHttpConnector::new(builder.build(tcp_connector)); cache.insert(key.clone(), connector); } @@ -560,13 +560,17 @@ impl HyperClientBuilder { self } - /// Create a [`HyperConnector`] with the default rustls HTTPS implementation. + /// Create a hyper client with the default rustls HTTPS implementation. + /// + /// This client will immediately load trusted certificates. #[cfg(feature = "tls-rustls")] pub fn build_https(self) -> SharedHttpClient { self.build(default_connector::https()) } - /// Create a [`HyperConnector`] with the default rustls HTTPS implementation. + /// Create a lazy hyper client with the default rustls HTTPS implementation. + /// + /// This client won't load trusted certificates until the first request is made. #[cfg(feature = "tls-rustls")] pub fn build_https_lazy(self) -> SharedHttpClient { self.build_with_fn(default_connector::https) @@ -982,18 +986,15 @@ mod timeout_middleware { mod test { use super::*; use crate::client::http::test_util::NeverTcpConnector; - use aws_smithy_async::assert_elapsed; use aws_smithy_async::time::SystemTimeSource; use aws_smithy_runtime_api::client::runtime_components::RuntimeComponentsBuilder; use http::Uri; use hyper_0_14::client::connect::{Connected, Connection}; - use once_cell::sync::Lazy; use std::io::{Error, ErrorKind}; use std::pin::Pin; use std::sync::atomic::{AtomicU32, Ordering}; use std::sync::Arc; use std::task::{Context, Poll}; - use std::time::SystemTime; use tokio::io::{AsyncRead, AsyncWrite, ReadBuf}; #[tokio::test] @@ -1027,7 +1028,10 @@ mod test { ]; // Kick off thousands of parallel tasks that will try to create a connector - let components = RuntimeComponentsBuilder::for_tests().build().unwrap(); + let components = RuntimeComponentsBuilder::for_tests() + .with_time_source(Some(SystemTimeSource::new())) + .build() + .unwrap(); let mut handles = Vec::new(); for setting in &settings { for _ in 0..1000 { @@ -1049,29 +1053,6 @@ mod test { assert_eq!(4, creation_count.load(Ordering::Relaxed)); } - #[tokio::test] - #[cfg(feature = "tls-rustls")] - async fn dont_preload_connector() { - let _client = HyperClientBuilder::default().build_https(); - assert!(Lazy::get(&default_connector::HTTPS_NATIVE_ROOTS).is_none()); - } - - #[tokio::test] - #[cfg(feature = "tls-rustls")] - async fn connector_loading_time() { - let client = HyperClientBuilder::default().build_https(); - let now = SystemTime::now(); - let conn = client.http_connector( - &HttpConnectorSettings::default(), - &RuntimeComponentsBuilder::for_tests() - .with_time_source(Some(SystemTimeSource::new())) - .build() - .unwrap(), - ); - let dur = now.elapsed().unwrap(); - assert_eq!(dur, Duration::from_millis(300)); - } - #[tokio::test] async fn hyper_io_error() { let connector = TestConnection { diff --git a/rust-runtime/aws-smithy-runtime/src/client/http/test_util/never.rs b/rust-runtime/aws-smithy-runtime/src/client/http/test_util/never.rs index d61608cbeb..128579e384 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/http/test_util/never.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/http/test_util/never.rs @@ -146,6 +146,7 @@ async fn never_tcp_connector_plugs_into_hyper_014() { use super::*; use crate::client::http::hyper_014::HyperClientBuilder; use aws_smithy_async::rt::sleep::TokioSleep; + use aws_smithy_async::time::SystemTimeSource; use aws_smithy_runtime_api::client::runtime_components::RuntimeComponentsBuilder; use std::time::Duration; @@ -153,6 +154,7 @@ async fn never_tcp_connector_plugs_into_hyper_014() { let client = HyperClientBuilder::new().build(NeverTcpConnector::new()); let components = RuntimeComponentsBuilder::for_tests() .with_sleep_impl(Some(TokioSleep::new())) + .with_time_source(Some(SystemTimeSource::new())) .build() .unwrap(); let http_connector = client.http_connector( From 73b6aa1b1ea283c9d8e1a63bc9c0e729b9e9add3 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Mon, 27 Nov 2023 18:11:21 -0800 Subject: [PATCH 3/7] Cache result of endpoint partitioning --- .../client/smithy/endpoint/rulesgen/StdLib.kt | 15 ++++++++++++++- .../inlineable/src/endpoint_lib/partition.rs | 8 ++++---- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/rulesgen/StdLib.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/rulesgen/StdLib.kt index 2c16a51bb7..6b16c79e3f 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/rulesgen/StdLib.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/rulesgen/StdLib.kt @@ -8,6 +8,7 @@ package software.amazon.smithy.rust.codegen.client.smithy.endpoint.rulesgen import software.amazon.smithy.model.node.Node import software.amazon.smithy.rust.codegen.client.smithy.endpoint.endpointsLib import software.amazon.smithy.rust.codegen.client.smithy.endpoint.generators.CustomRuntimeFunction +import software.amazon.smithy.rust.codegen.client.smithy.endpoint.generators.EndpointStdLib import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency import software.amazon.smithy.rust.codegen.core.rustlang.Writable import software.amazon.smithy.rust.codegen.core.rustlang.rust @@ -71,13 +72,25 @@ class AwsPartitionResolver(runtimeConfig: RuntimeConfig, private val partitionsD CargoDependency.Regex, ).toType() .resolve("PartitionResolver"), + "Lazy" to CargoDependency.OnceCell.toType().resolve("sync::Lazy"), ) override fun structFieldInit() = writable { val json = Node.printJson(partitionsDotJson).dq() rustTemplate( - """partition_resolver: #{PartitionResolver}::new_from_json(b$json).expect("valid JSON")""", + """partition_resolver: #{DEFAULT_PARTITION_RESOLVER}.clone()""", *codegenScope, + "DEFAULT_PARTITION_RESOLVER" to RuntimeType.forInlineFun("DEFAULT_PARTITION_RESOLVER", EndpointStdLib) { + rustTemplate( + """ + // Loading the partition JSON is expensive since it involves many regex compilations, + // so cache the result so that it only need to be paid for the first constructed client. + pub(crate) static DEFAULT_PARTITION_RESOLVER: #{Lazy}<#{PartitionResolver}> = + #{Lazy}::new(|| #{PartitionResolver}::new_from_json(b$json).expect("valid JSON")); + """, + *codegenScope, + ) + }, ) } diff --git a/rust-runtime/inlineable/src/endpoint_lib/partition.rs b/rust-runtime/inlineable/src/endpoint_lib/partition.rs index 02088d0f93..2f7aa45f7e 100644 --- a/rust-runtime/inlineable/src/endpoint_lib/partition.rs +++ b/rust-runtime/inlineable/src/endpoint_lib/partition.rs @@ -17,7 +17,7 @@ use std::borrow::Cow; use std::collections::HashMap; /// Determine the AWS partition metadata for a given region -#[derive(Debug, Default)] +#[derive(Clone, Debug, Default)] pub(crate) struct PartitionResolver { partitions: Vec, } @@ -151,7 +151,7 @@ impl PartitionResolver { type Str = Cow<'static, str>; -#[derive(Debug)] +#[derive(Clone, Debug)] pub(crate) struct PartitionMetadata { id: Str, region_regex: Regex, @@ -204,7 +204,7 @@ impl PartitionMetadata { } } -#[derive(Debug)] +#[derive(Clone, Debug)] pub(crate) struct PartitionOutput { name: Str, dns_suffix: Str, @@ -213,7 +213,7 @@ pub(crate) struct PartitionOutput { supports_dual_stack: bool, } -#[derive(Debug, Default)] +#[derive(Clone, Debug, Default)] pub(crate) struct PartitionOutputOverride { name: Option, dns_suffix: Option, From c16ece9d6dfa5aae4a3f42d6249ff827c20ae6c3 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Tue, 28 Nov 2023 17:33:59 -0800 Subject: [PATCH 4/7] Revert "Cache result of endpoint partitioning" This reverts commit 8cf672b32416a5b85a8101b027439ecec513207b. --- .../client/smithy/endpoint/rulesgen/StdLib.kt | 15 +-------------- .../inlineable/src/endpoint_lib/partition.rs | 8 ++++---- 2 files changed, 5 insertions(+), 18 deletions(-) diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/rulesgen/StdLib.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/rulesgen/StdLib.kt index 6b16c79e3f..2c16a51bb7 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/rulesgen/StdLib.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/endpoint/rulesgen/StdLib.kt @@ -8,7 +8,6 @@ package software.amazon.smithy.rust.codegen.client.smithy.endpoint.rulesgen import software.amazon.smithy.model.node.Node import software.amazon.smithy.rust.codegen.client.smithy.endpoint.endpointsLib import software.amazon.smithy.rust.codegen.client.smithy.endpoint.generators.CustomRuntimeFunction -import software.amazon.smithy.rust.codegen.client.smithy.endpoint.generators.EndpointStdLib import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency import software.amazon.smithy.rust.codegen.core.rustlang.Writable import software.amazon.smithy.rust.codegen.core.rustlang.rust @@ -72,25 +71,13 @@ class AwsPartitionResolver(runtimeConfig: RuntimeConfig, private val partitionsD CargoDependency.Regex, ).toType() .resolve("PartitionResolver"), - "Lazy" to CargoDependency.OnceCell.toType().resolve("sync::Lazy"), ) override fun structFieldInit() = writable { val json = Node.printJson(partitionsDotJson).dq() rustTemplate( - """partition_resolver: #{DEFAULT_PARTITION_RESOLVER}.clone()""", + """partition_resolver: #{PartitionResolver}::new_from_json(b$json).expect("valid JSON")""", *codegenScope, - "DEFAULT_PARTITION_RESOLVER" to RuntimeType.forInlineFun("DEFAULT_PARTITION_RESOLVER", EndpointStdLib) { - rustTemplate( - """ - // Loading the partition JSON is expensive since it involves many regex compilations, - // so cache the result so that it only need to be paid for the first constructed client. - pub(crate) static DEFAULT_PARTITION_RESOLVER: #{Lazy}<#{PartitionResolver}> = - #{Lazy}::new(|| #{PartitionResolver}::new_from_json(b$json).expect("valid JSON")); - """, - *codegenScope, - ) - }, ) } diff --git a/rust-runtime/inlineable/src/endpoint_lib/partition.rs b/rust-runtime/inlineable/src/endpoint_lib/partition.rs index 2f7aa45f7e..02088d0f93 100644 --- a/rust-runtime/inlineable/src/endpoint_lib/partition.rs +++ b/rust-runtime/inlineable/src/endpoint_lib/partition.rs @@ -17,7 +17,7 @@ use std::borrow::Cow; use std::collections::HashMap; /// Determine the AWS partition metadata for a given region -#[derive(Clone, Debug, Default)] +#[derive(Debug, Default)] pub(crate) struct PartitionResolver { partitions: Vec, } @@ -151,7 +151,7 @@ impl PartitionResolver { type Str = Cow<'static, str>; -#[derive(Clone, Debug)] +#[derive(Debug)] pub(crate) struct PartitionMetadata { id: Str, region_regex: Regex, @@ -204,7 +204,7 @@ impl PartitionMetadata { } } -#[derive(Clone, Debug)] +#[derive(Debug)] pub(crate) struct PartitionOutput { name: Str, dns_suffix: Str, @@ -213,7 +213,7 @@ pub(crate) struct PartitionOutput { supports_dual_stack: bool, } -#[derive(Clone, Debug, Default)] +#[derive(Debug, Default)] pub(crate) struct PartitionOutputOverride { name: Option, dns_suffix: Option, From 15ee8ae31122fcef880d02654662f9d7cb538b41 Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Wed, 29 Nov 2023 15:27:00 -0800 Subject: [PATCH 5/7] Remove BV changes and init trust certs at client validation time --- .../rustdoc/validate_base_client_config.md | 9 +++ .../rustdoc/validate_final_config.md | 7 ++ .../src/client/behavior_version.rs | 72 ++----------------- .../aws-smithy-runtime-api/src/client/http.rs | 43 ++++++++++- .../src/client/identity.rs | 15 +--- .../src/client/runtime_components.rs | 13 +--- .../aws-smithy-runtime/src/client/defaults.rs | 30 +------- .../src/client/http/hyper_014.rs | 47 +++++------- 8 files changed, 88 insertions(+), 148 deletions(-) create mode 100644 rust-runtime/aws-smithy-runtime-api/rustdoc/validate_base_client_config.md create mode 100644 rust-runtime/aws-smithy-runtime-api/rustdoc/validate_final_config.md diff --git a/rust-runtime/aws-smithy-runtime-api/rustdoc/validate_base_client_config.md b/rust-runtime/aws-smithy-runtime-api/rustdoc/validate_base_client_config.md new file mode 100644 index 0000000000..4ac61d51b0 --- /dev/null +++ b/rust-runtime/aws-smithy-runtime-api/rustdoc/validate_base_client_config.md @@ -0,0 +1,9 @@ +Validate the base client configuration. + +This gets called upon client construction. The full config may not be available at +this time (hence why it has [`RuntimeComponentsBuilder`] as an argument rather +than [`RuntimeComponents`]). Any error returned here will become a panic +in the client constructor. + +[`RuntimeComponentsBuilder`]: crate::client::runtime_components::RuntimeComponentsBuilder +[`RuntimeComponents`]: crate::client::runtime_components::RuntimeComponents diff --git a/rust-runtime/aws-smithy-runtime-api/rustdoc/validate_final_config.md b/rust-runtime/aws-smithy-runtime-api/rustdoc/validate_final_config.md new file mode 100644 index 0000000000..768bda5dd5 --- /dev/null +++ b/rust-runtime/aws-smithy-runtime-api/rustdoc/validate_final_config.md @@ -0,0 +1,7 @@ +Validate the final client configuration. + +This gets called immediately after the [`Intercept::read_before_execution`] trait hook +when the final configuration has been resolved. Any error returned here will +cause the operation to return that error. + +[`Intercept::read_before_execution`]: crate::client::interceptors::Intercept::read_before_execution diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/behavior_version.rs b/rust-runtime/aws-smithy-runtime-api/src/client/behavior_version.rs index df8c4e3f1a..c44043fe73 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/behavior_version.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/behavior_version.rs @@ -3,23 +3,15 @@ * SPDX-License-Identifier: Apache-2.0 */ -//! Behavior major-version of the client +//! Behavior Major version of the client /// Behavior major-version of the client /// /// Over time, new best-practice behaviors are introduced. However, these behaviors might not be backwards /// compatible. For example, a change which introduces new default timeouts or a new retry-mode for /// all operations might be the ideal behavior but could break existing applications. -#[derive(Clone, Debug, PartialOrd, Ord, PartialEq, Eq)] -pub struct BehaviorVersion { - version: Version, -} - -#[derive(Clone, Debug, PartialOrd, Ord, PartialEq, Eq)] -enum Version { - V2023_11_09, - V2023_11_27, -} +#[derive(Debug, Clone)] +pub struct BehaviorVersion {} impl BehaviorVersion { /// This method will always return the latest major version. @@ -33,63 +25,13 @@ impl BehaviorVersion { /// /// The latest version is currently [`BehaviorVersion::v2023_11_09`] pub fn latest() -> Self { - Self { - version: Version::V2023_11_27, - } - } - - /// This method returns the behavior configuration for November 27th, 2023 - /// - /// When a new behavior major version is released, this method will be deprecated. - /// - /// # Changes - /// - /// The default HTTP client is now lazy initialized so that the price of loading - /// trusted certificates isn't paid when overriding the default client. This means - /// that the first request after client initialization will be slower for the default - /// client as it needs to load those certs upon that first request. - /// - /// This first request cost can be eliminated by priming the client immediately after - /// initialization, or by overriding the default HTTP client with one that doesn't lazy - /// initialize. - pub const fn v2023_11_27() -> Self { - Self { - version: Version::V2023_11_27, - } + Self {} } /// This method returns the behavior configuration for November 9th, 2023 /// - /// This behavior version has been superceded by v2023_11_27. - #[deprecated( - note = "Superceded by v2023_11_27. See doc comment on v2023_11_27 for details on behavior changes." - )] - pub const fn v2023_11_09() -> Self { - Self { - version: Version::V2023_11_09, - } - } - - /// True if this version is greater than or equal to the given `version`. - pub fn is_at_least(&self, version: BehaviorVersion) -> bool { - self >= &version - } - - /// True if this version is before the given `version`. - pub fn is_before(&self, version: BehaviorVersion) -> bool { - self < &version - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[allow(deprecated)] - #[test] - fn comparison() { - assert!(BehaviorVersion::v2023_11_09() < BehaviorVersion::v2023_11_27()); - assert!(BehaviorVersion::v2023_11_27() == BehaviorVersion::v2023_11_27()); - assert!(BehaviorVersion::v2023_11_27() > BehaviorVersion::v2023_11_09()); + /// When a new behavior major version is released, this method will be deprecated. + pub fn v2023_11_09() -> Self { + Self {} } } diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/http.rs b/rust-runtime/aws-smithy-runtime-api/src/client/http.rs index 1e90d40fb1..aa66f79699 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/http.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/http.rs @@ -50,11 +50,13 @@ //! [`tower`]: https://crates.io/crates/tower //! [`aws-smithy-runtime`]: https://crates.io/crates/aws-smithy-runtime +use crate::box_error::BoxError; use crate::client::orchestrator::{HttpRequest, HttpResponse}; use crate::client::result::ConnectorError; use crate::client::runtime_components::sealed::ValidateConfig; -use crate::client::runtime_components::RuntimeComponents; +use crate::client::runtime_components::{RuntimeComponents, RuntimeComponentsBuilder}; use crate::impl_shared_conversions; +use aws_smithy_types::config_bag::ConfigBag; use std::fmt; use std::sync::Arc; use std::time::Duration; @@ -143,6 +145,26 @@ pub trait HttpClient: Send + Sync + fmt::Debug { settings: &HttpConnectorSettings, components: &RuntimeComponents, ) -> SharedHttpConnector; + + #[doc = include_str!("../../rustdoc/validate_base_client_config.md")] + fn validate_base_client_config( + &self, + runtime_components: &RuntimeComponentsBuilder, + cfg: &ConfigBag, + ) -> Result<(), BoxError> { + let _ = (runtime_components, cfg); + Ok(()) + } + + #[doc = include_str!("../../rustdoc/validate_final_config.md")] + fn validate_final_config( + &self, + runtime_components: &RuntimeComponents, + cfg: &ConfigBag, + ) -> Result<(), BoxError> { + let _ = (runtime_components, cfg); + Ok(()) + } } /// Shared HTTP client for use across multiple clients and requests. @@ -170,7 +192,24 @@ impl HttpClient for SharedHttpClient { } } -impl ValidateConfig for SharedHttpClient {} +impl ValidateConfig for SharedHttpClient { + fn validate_base_client_config( + &self, + runtime_components: &super::runtime_components::RuntimeComponentsBuilder, + cfg: &aws_smithy_types::config_bag::ConfigBag, + ) -> Result<(), crate::box_error::BoxError> { + self.selector + .validate_base_client_config(runtime_components, cfg) + } + + fn validate_final_config( + &self, + runtime_components: &RuntimeComponents, + cfg: &aws_smithy_types::config_bag::ConfigBag, + ) -> Result<(), crate::box_error::BoxError> { + self.selector.validate_final_config(runtime_components, cfg) + } +} impl_shared_conversions!(convert SharedHttpClient from HttpClient using SharedHttpClient::new); diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/identity.rs b/rust-runtime/aws-smithy-runtime-api/src/client/identity.rs index 669c4490ab..3632029e7b 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/identity.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/identity.rs @@ -64,12 +64,7 @@ pub trait ResolveCachedIdentity: fmt::Debug + Send + Sync { config_bag: &'a ConfigBag, ) -> IdentityFuture<'a>; - /// Validate the base client configuration for this implementation. - /// - /// This gets called upon client construction. The full config may not be available at - /// this time (hence why it has [`RuntimeComponentsBuilder`] as an argument rather - /// than [`RuntimeComponents`]). Any error returned here will become a panic - /// in the client constructor. + #[doc = include_str!("../../rustdoc/validate_base_client_config.md")] fn validate_base_client_config( &self, runtime_components: &RuntimeComponentsBuilder, @@ -79,13 +74,7 @@ pub trait ResolveCachedIdentity: fmt::Debug + Send + Sync { Ok(()) } - /// Validate the final client configuration for this implementation. - /// - /// This gets called immediately after the [`Intercept::read_before_execution`] trait hook - /// when the final configuration has been resolved. Any error returned here will - /// cause the operation to return that error. - /// - /// [`Intercept::read_before_execution`]: crate::client::interceptors::Intercept::read_before_execution + #[doc = include_str!("../../rustdoc/validate_final_config.md")] fn validate_final_config( &self, runtime_components: &RuntimeComponents, diff --git a/rust-runtime/aws-smithy-runtime-api/src/client/runtime_components.rs b/rust-runtime/aws-smithy-runtime-api/src/client/runtime_components.rs index 0b0c58c321..19471ddf6b 100644 --- a/rust-runtime/aws-smithy-runtime-api/src/client/runtime_components.rs +++ b/rust-runtime/aws-smithy-runtime-api/src/client/runtime_components.rs @@ -44,12 +44,7 @@ pub(crate) mod sealed { /// This trait can be used to validate that certain required components or config values /// are available, and provide an error with helpful instructions if they are not. pub trait ValidateConfig: fmt::Debug + Send + Sync { - /// Validate the base client configuration. - /// - /// This gets called upon client construction. The full config may not be available at - /// this time (hence why it has [`RuntimeComponentsBuilder`] as an argument rather - /// than [`RuntimeComponents`]). Any error returned here will become a panic - /// in the client constructor. + #[doc = include_str!("../../rustdoc/validate_base_client_config.md")] fn validate_base_client_config( &self, runtime_components: &RuntimeComponentsBuilder, @@ -59,11 +54,7 @@ pub(crate) mod sealed { Ok(()) } - /// Validate the final client configuration. - /// - /// This gets called immediately after the [`Intercept::read_before_execution`] trait hook - /// when the final configuration has been resolved. Any error returned here will - /// cause the operation to return that error. + #[doc = include_str!("../../rustdoc/validate_final_config.md")] fn validate_final_config( &self, runtime_components: &RuntimeComponents, diff --git a/rust-runtime/aws-smithy-runtime/src/client/defaults.rs b/rust-runtime/aws-smithy-runtime/src/client/defaults.rs index 5402cce029..ca2d06a387 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/defaults.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/defaults.rs @@ -63,22 +63,6 @@ pub fn default_http_client_plugin() -> Option { }) } -/// Runtime plugin that provides a default connector. -/// -/// This connector is lazily created on first-user -pub fn default_http_client_plugin_lazy() -> Option { - let _default: Option = None; - #[cfg(feature = "connector-hyper-0-14-x")] - let _default = crate::client::http::hyper_014::default_client_lazy(); - - _default.map(|default| { - default_plugin("default_http_client_plugin", |components| { - components.with_http_client(Some(default)) - }) - .into_shared() - }) -} - /// Runtime plugin that provides a default async sleep implementation. pub fn default_sleep_impl_plugin() -> Option { default_async_sleep().map(|default| { @@ -262,26 +246,14 @@ impl DefaultPluginParams { self.behavior_version = Some(version); self } - - fn behavior_version(&self) -> BehaviorVersion { - self.behavior_version - .clone() - .unwrap_or(BehaviorVersion::latest()) - } } /// All default plugins. pub fn default_plugins( params: DefaultPluginParams, ) -> impl IntoIterator { - // v2023_11_27 introduced lazy HTTP clients - let http_client_plugin = match params.behavior_version() >= BehaviorVersion::v2023_11_27() { - true => default_http_client_plugin_lazy(), - false => default_http_client_plugin(), - }; - [ - http_client_plugin, + default_http_client_plugin(), default_identity_cache_plugin(), default_retry_config_plugin( params diff --git a/rust-runtime/aws-smithy-runtime/src/client/http/hyper_014.rs b/rust-runtime/aws-smithy-runtime/src/client/http/hyper_014.rs index cf63887f9a..d19794a62f 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/http/hyper_014.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/http/hyper_014.rs @@ -15,9 +15,12 @@ use aws_smithy_runtime_api::client::http::{ }; use aws_smithy_runtime_api::client::orchestrator::{HttpRequest, HttpResponse}; use aws_smithy_runtime_api::client::result::ConnectorError; -use aws_smithy_runtime_api::client::runtime_components::RuntimeComponents; +use aws_smithy_runtime_api::client::runtime_components::{ + RuntimeComponents, RuntimeComponentsBuilder, +}; use aws_smithy_runtime_api::shared::IntoShared; use aws_smithy_types::body::SdkBody; +use aws_smithy_types::config_bag::ConfigBag; use aws_smithy_types::error::display::DisplayErrorContext; use aws_smithy_types::retry::ErrorKind; use h2::Reason; @@ -125,20 +128,6 @@ pub fn default_client() -> Option { } } -/// Creates a hyper-backed HTTPS client from defaults depending on what cargo features are activated. -pub fn default_client_lazy() -> Option { - #[cfg(feature = "tls-rustls")] - { - tracing::trace!("creating a new default hyper 0.14.x client"); - Some(HyperClientBuilder::new().build_https_lazy()) - } - #[cfg(not(feature = "tls-rustls"))] - { - tracing::trace!("no default connector available"); - None - } -} - /// [`HttpConnector`] that uses [`hyper_0_14`] to make HTTP requests. /// /// This connector also implements socket connect and read timeouts. @@ -492,6 +481,20 @@ where C::Future: Unpin + Send + 'static, C::Error: Into, { + fn validate_base_client_config( + &self, + _: &RuntimeComponentsBuilder, + _: &ConfigBag, + ) -> Result<(), BoxError> { + // Initialize the TCP connector at this point so that native certs load + // at client initialization time instead of upon first request. We do it + // here rather than at construction so that it won't run if this is not + // the selected HTTP client for the base config (for example, if this was + // the default HTTP client, and it was overridden by a later plugin). + let _ = (self.tcp_connector_fn)(); + Ok(()) + } + fn http_connector( &self, settings: &HttpConnectorSettings, @@ -565,14 +568,6 @@ impl HyperClientBuilder { /// This client will immediately load trusted certificates. #[cfg(feature = "tls-rustls")] pub fn build_https(self) -> SharedHttpClient { - self.build(default_connector::https()) - } - - /// Create a lazy hyper client with the default rustls HTTPS implementation. - /// - /// This client won't load trusted certificates until the first request is made. - #[cfg(feature = "tls-rustls")] - pub fn build_https_lazy(self) -> SharedHttpClient { self.build_with_fn(default_connector::https) } @@ -590,11 +585,7 @@ impl HyperClientBuilder { C::Future: Unpin + Send + 'static, C::Error: Into, { - SharedHttpClient::new(HyperClient { - connector_cache: RwLock::new(HashMap::new()), - client_builder: self.client_builder.unwrap_or_default(), - tcp_connector_fn: move || tcp_connector.clone(), - }) + self.build_with_fn(move || tcp_connector.clone()) } fn build_with_fn(self, tcp_connector_fn: F) -> SharedHttpClient From 0470a59180096fc09bf0bbe350b376054a08671c Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Wed, 29 Nov 2023 16:27:55 -0800 Subject: [PATCH 6/7] Update changelog --- CHANGELOG.next.toml | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index fc4c4c2578..5b5c5fed82 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -9,4 +9,16 @@ # message = "Fix typos in module documentation for generated crates" # references = ["smithy-rs#920"] # meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"} -# author = "rcoh" \ No newline at end of file +# author = "rcoh" + +[[aws-sdk-rust]] +message = "Loading native TLS trusted certs for the default HTTP client now only occurs if the default HTTP client is not overridden in config." +references = ["smithy-rs#3262"] +meta = { "breaking" = false, "tada" = false, "bug" = true } +author = "jdisanti" + +[[smithy-rs]] +message = "Loading native TLS trusted certs for the default HTTP client now only occurs if the default HTTP client is not overridden in config." +references = ["smithy-rs#3262"] +meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "client" } +author = "jdisanti" From d0c26ac42c5fa91f45cab658b390dd92ed9439cd Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Thu, 30 Nov 2023 11:11:16 -0800 Subject: [PATCH 7/7] Incorporate feedback --- aws/rust-runtime/aws-config/src/imds/client.rs | 6 ------ .../aws-smithy-runtime/src/client/http/hyper_014.rs | 3 ++- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/aws/rust-runtime/aws-config/src/imds/client.rs b/aws/rust-runtime/aws-config/src/imds/client.rs index 3fba407cbd..154225b29e 100644 --- a/aws/rust-runtime/aws-config/src/imds/client.rs +++ b/aws/rust-runtime/aws-config/src/imds/client.rs @@ -990,18 +990,12 @@ pub(crate) mod test { use crate::imds::client::ImdsError; use aws_smithy_types::error::display::DisplayErrorContext; use std::time::SystemTime; - let (_guard, _) = capture_test_logs(); let client = Client::builder() // 240.* can never be resolved .endpoint("http://240.0.0.0") .expect("valid uri") .build(); - // preload the connector so that doesn't impact timing - let _resp = client - .get("/latest/metadata") - .await - .expect_err("240.0.0.0 will never resolve"); let now = SystemTime::now(); let resp = client .get("/latest/metadata") diff --git a/rust-runtime/aws-smithy-runtime/src/client/http/hyper_014.rs b/rust-runtime/aws-smithy-runtime/src/client/http/hyper_014.rs index d19794a62f..1359beb4a3 100644 --- a/rust-runtime/aws-smithy-runtime/src/client/http/hyper_014.rs +++ b/rust-runtime/aws-smithy-runtime/src/client/http/hyper_014.rs @@ -565,7 +565,8 @@ impl HyperClientBuilder { /// Create a hyper client with the default rustls HTTPS implementation. /// - /// This client will immediately load trusted certificates. + /// The trusted certificates will be loaded later when this becomes the selected + /// HTTP client for a Smithy client. #[cfg(feature = "tls-rustls")] pub fn build_https(self) -> SharedHttpClient { self.build_with_fn(default_connector::https)