Lazy initialize the default HTTP client

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"
+# 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/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(