Lazy initialize the default HTTP client (#3262)

This change initializes the TLS trust certs at base client validation
time rather than upon creation of the default HTTP client runtime plugin
so that if the default is overridden, that initialization doesn't need
to take place. This is especially helpful on MacOS where that
initialization takes approximately 100 milliseconds.

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._

---------

Co-authored-by: Russell Cohen <[email protected]>

CHANGELOG.next.toml

@@ -11,6 +11,18 @@
 # meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"}
 # 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"
+
 [[aws-sdk-rust]]
 message = """Client creation now takes microseconds instead of milliseconds.
 Previously, it would take 2-3 milliseconds for each client instantiation due to time spent compiling regexes.

aws/sdk/integration-tests/s3/tests/service_timeout_overrides.rs

@@ -6,16 +6,19 @@
 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;
 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();
     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,8 +52,8 @@ async fn timeouts_can_be_set_by_service() {
         .await
         .expect_err("unroutable IP should timeout");
     match err {
-        SdkError::DispatchFailure(err) => assert!(err.is_timeout()),
-        // 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

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

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

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);
 

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,

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,

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;
@@ -36,38 +39,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<hyper_0_14::client::HttpConnector>,
-    > = once_cell::sync::Lazy::new(|| {
+    > = once_cell::sync::Lazy::new(default_tls);
+
+    fn default_tls() -> HttpsConnector<HttpConnector> {
         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,
@@ -474,6 +481,20 @@ where
     C::Future: Unpin + Send + 'static,
     C::Error: Into<BoxError>,
 {
+    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,
@@ -490,7 +511,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());
+                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);
             }
@@ -535,10 +563,13 @@ impl HyperClientBuilder {
         self
     }
 
-    /// Create a [`HyperConnector`] with the default rustls HTTPS implementation.
+    /// Create a hyper client with the default rustls HTTPS implementation.
+    ///
+    /// 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(default_connector::https())
+        self.build_with_fn(default_connector::https)
     }
 
     /// Create a [`SharedHttpClient`] from this builder and a given connector.
@@ -555,14 +586,9 @@ impl HyperClientBuilder {
         C::Future: Unpin + Send + 'static,
         C::Error: Into<BoxError>,
     {
-        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())
     }
 
-    #[cfg(all(test, feature = "test-util"))]
     fn build_with_fn<C, F>(self, tcp_connector_fn: F) -> SharedHttpClient
     where
         F: Fn() -> C + Send + Sync + 'static,
@@ -952,6 +978,7 @@ mod timeout_middleware {
 mod test {
     use super::*;
     use crate::client::http::test_util::NeverTcpConnector;
+    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};
@@ -993,7 +1020,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 {

rust-runtime/aws-smithy-runtime/src/client/http/test_util/never.rs

@@ -146,13 +146,15 @@ 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;
 
     // it should compile
     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(