From 62d5b9f5fdcd1d1fb385a7e8578633feaad9b940 Mon Sep 17 00:00:00 2001 From: Russell Cohen Date: Wed, 15 Nov 2023 15:42:43 -0500 Subject: [PATCH] Assorted cleanups of stable runtime crates --- CHANGELOG.next.toml | 12 +++++++++++ .../aws-config/external-types.toml | 12 +---------- .../aws-config/src/imds/client.rs | 18 +++++++++------- .../aws-config/src/imds/client/error.rs | 9 -------- .../aws-config/src/imds/credentials.rs | 9 +++++--- aws/rust-runtime/aws-config/src/lib.rs | 1 - .../aws-config/src/sts/assume_role.rs | 9 ++++++-- .../aws-runtime/external-types.toml | 8 ------- .../aws-runtime/src/request_info.rs | 8 +++---- .../aws-sigv4/external-types.toml | 2 +- .../aws-sigv4/src/http_request.rs | 2 +- .../aws-sigv4/src/http_request/sign.rs | 21 ++++++++++--------- aws/sdk/sdk-external-types.toml | 12 ++--------- .../aws-smithy-runtime/external-types.toml | 15 +++++++------ rust-runtime/aws-smithy-runtime/src/lib.rs | 1 + 15 files changed, 64 insertions(+), 75 deletions(-) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index c344eda28d6..2f87d2b9ee3 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -177,3 +177,15 @@ The `Unhandled` variant should never be referenced directly. references = ["smithy-rs#3191"] meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "client" } author = "jdisanti" + +[[smithy-rs]] +message = "SignableRequest::apply_to_request in aws_sigv4 has been renamed `apply_to_request_http0x`" +references = ["smithy-rs#3205"] +meta = { "breaking" = true, "tada" = false, "bug" = false, "target" = "client" } +author = "rcoh" + +[[aws-sdk-rust]] +message = "imds::client::Builder::endpoint has been updated to accept a string instead of a URI. The method now returns a result instead." +references = ["smithy-rs#3205"] +meta = { "breaking" = true, "tada" = false, "bug" = false } +author = "rcoh" diff --git a/aws/rust-runtime/aws-config/external-types.toml b/aws/rust-runtime/aws-config/external-types.toml index bd0ada24610..19c1f6aab8f 100644 --- a/aws/rust-runtime/aws-config/external-types.toml +++ b/aws/rust-runtime/aws-config/external-types.toml @@ -7,13 +7,11 @@ allowed_external_types = [ "aws_credential_types::provider::ProvideCredentials", "aws_credential_types::provider::Result", "aws_credential_types::provider::SharedCredentialsProvider", - "aws_sdk_sts::types::_policy_descriptor_type::PolicyDescriptorType", "aws_smithy_async::rt::sleep::AsyncSleep", "aws_smithy_async::rt::sleep::SharedAsyncSleep", "aws_smithy_async::time::SharedTimeSource", "aws_smithy_async::time::TimeSource", - "aws_smithy_http::endpoint", - "aws_smithy_http::endpoint::error::InvalidEndpointError", + "aws_smithy_runtime_api::box_error::BoxError", "aws_smithy_runtime::client::identity::cache::IdentityCache", "aws_smithy_runtime::client::identity::cache::lazy::LazyCacheBuilder", "aws_smithy_runtime_api::client::dns::ResolveDns", @@ -33,12 +31,4 @@ allowed_external_types = [ "aws_smithy_types::timeout::TimeoutConfig", "aws_smithy_types::timeout::TimeoutConfigBuilder", "aws_types::*", - "http::response::Response", - "http::uri::Uri", - "tower_service::Service", - - # TODO(https://github.com/smithy-lang/smithy-rs/issues/1193): Decide if the following should be exposed - "hyper::client::connect::Connection", - "tokio::io::async_read::AsyncRead", - "tokio::io::async_write::AsyncWrite", ] diff --git a/aws/rust-runtime/aws-config/src/imds/client.rs b/aws/rust-runtime/aws-config/src/imds/client.rs index 1e4d7f02faa..154225b29e8 100644 --- a/aws/rust-runtime/aws-config/src/imds/client.rs +++ b/aws/rust-runtime/aws-config/src/imds/client.rs @@ -15,6 +15,7 @@ use aws_http::user_agent::{ApiMetadata, AwsUserAgent}; use aws_runtime::user_agent::UserAgentInterceptor; use aws_smithy_runtime::client::orchestrator::operation::Operation; use aws_smithy_runtime::client::retries::strategy::StandardRetryStrategy; +use aws_smithy_runtime_api::box_error::BoxError; use aws_smithy_runtime_api::client::auth::AuthSchemeOptionResolverParams; use aws_smithy_runtime_api::client::endpoint::{ EndpointFuture, EndpointResolverParams, ResolveEndpoint, @@ -87,10 +88,9 @@ fn user_agent() -> AwsUserAgent { /// 1. Explicit configuration of `Endpoint` via the [builder](Builder): /// ```no_run /// use aws_config::imds::client::Client; -/// use http::Uri; /// # async fn docs() { /// let client = Client::builder() -/// .endpoint(Uri::from_static("http://customidms:456/")) +/// .endpoint("http://customidms:456/").expect("valid URI") /// .build(); /// # } /// ``` @@ -358,9 +358,10 @@ impl Builder { /// By default, the client will resolve an endpoint from the environment, AWS config, and endpoint mode. /// /// See [`Client`] for more information. - pub fn endpoint(mut self, endpoint: impl Into) -> Self { - self.endpoint = Some(EndpointSource::Explicit(endpoint.into())); - self + pub fn endpoint(mut self, endpoint: impl AsRef) -> Result { + let uri: Uri = endpoint.as_ref().parse()?; + self.endpoint = Some(EndpointSource::Explicit(uri)); + Ok(self) } /// Override the endpoint mode for [`Client`] @@ -992,7 +993,8 @@ pub(crate) mod test { let client = Client::builder() // 240.* can never be resolved - .endpoint(Uri::from_static("http://240.0.0.0")) + .endpoint("http://240.0.0.0") + .expect("valid uri") .build(); let now = SystemTime::now(); let resp = client @@ -1057,7 +1059,9 @@ pub(crate) mod test { .with_http_client(http_client); let mut imds_client = Client::builder().configure(&provider_config); if let Some(endpoint_override) = test_case.endpoint_override { - imds_client = imds_client.endpoint(endpoint_override.parse::().unwrap()); + imds_client = imds_client + .endpoint(endpoint_override) + .expect("invalid URI"); } if let Some(mode_override) = test_case.mode_override { diff --git a/aws/rust-runtime/aws-config/src/imds/client/error.rs b/aws/rust-runtime/aws-config/src/imds/client/error.rs index 05412d75ea0..7e418c28f8e 100644 --- a/aws/rust-runtime/aws-config/src/imds/client/error.rs +++ b/aws/rust-runtime/aws-config/src/imds/client/error.rs @@ -5,7 +5,6 @@ //! Error types for [`ImdsClient`](crate::imds::client::Client) -use aws_smithy_http::endpoint::error::InvalidEndpointError; use aws_smithy_runtime_api::client::orchestrator::HttpResponse; use aws_smithy_runtime_api::client::result::SdkError; use std::error::Error; @@ -224,14 +223,6 @@ impl Error for BuildError { } } -impl From for BuildError { - fn from(err: InvalidEndpointError) -> Self { - Self { - kind: BuildErrorKind::InvalidEndpointUri(err.into()), - } - } -} - #[derive(Debug)] pub(super) enum TokenErrorKind { /// The token was invalid diff --git a/aws/rust-runtime/aws-config/src/imds/credentials.rs b/aws/rust-runtime/aws-config/src/imds/credentials.rs index 845a53b8777..896e66d6df2 100644 --- a/aws/rust-runtime/aws-config/src/imds/credentials.rs +++ b/aws/rust-runtime/aws-config/src/imds/credentials.rs @@ -421,7 +421,8 @@ mod test { async fn read_timeout_during_credentials_refresh_should_yield_last_retrieved_credentials() { let client = crate::imds::Client::builder() // 240.* can never be resolved - .endpoint(http::Uri::from_static("http://240.0.0.0")) + .endpoint("http://240.0.0.0") + .unwrap() .build(); let expected = aws_credential_types::Credentials::for_tests(); let provider = ImdsCredentialsProvider::builder() @@ -439,7 +440,8 @@ mod test { ) { let client = crate::imds::Client::builder() // 240.* can never be resolved - .endpoint(http::Uri::from_static("http://240.0.0.0")) + .endpoint("http://240.0.0.0") + .unwrap() .build(); let provider = ImdsCredentialsProvider::builder() .imds_client(client) @@ -458,7 +460,8 @@ mod test { use aws_smithy_async::rt::sleep::AsyncSleep; let client = crate::imds::Client::builder() // 240.* can never be resolved - .endpoint(http::Uri::from_static("http://240.0.0.0")) + .endpoint("http://240.0.0.0") + .unwrap() .build(); let expected = aws_credential_types::Credentials::for_tests(); let provider = ImdsCredentialsProvider::builder() diff --git a/aws/rust-runtime/aws-config/src/lib.rs b/aws/rust-runtime/aws-config/src/lib.rs index 76fa855538b..73d5106e9d2 100644 --- a/aws/rust-runtime/aws-config/src/lib.rs +++ b/aws/rust-runtime/aws-config/src/lib.rs @@ -96,7 +96,6 @@ //! # } //! ``` -pub use aws_smithy_http::endpoint; pub use aws_smithy_runtime_api::client::behavior_version::BehaviorVersion; // Re-export types from aws-types pub use aws_types::{ diff --git a/aws/rust-runtime/aws-config/src/sts/assume_role.rs b/aws/rust-runtime/aws-config/src/sts/assume_role.rs index 6b827a8b7e3..9605cb0e075 100644 --- a/aws/rust-runtime/aws-config/src/sts/assume_role.rs +++ b/aws/rust-runtime/aws-config/src/sts/assume_role.rs @@ -161,8 +161,13 @@ impl AssumeRoleProviderBuilder { /// This parameter is optional. /// For more information, see /// [policy_arns](aws_sdk_sts::operation::assume_role::builders::AssumeRoleInputBuilder::policy_arns) - pub fn policy_arns(mut self, policy_arns: Vec) -> Self { - self.policy_arns = Some(policy_arns); + pub fn policy_arns(mut self, policy_arns: Vec) -> Self { + self.policy_arns = Some( + policy_arns + .into_iter() + .map(|arn| PolicyDescriptorType::builder().arn(arn).build()) + .collect::>(), + ); self } diff --git a/aws/rust-runtime/aws-runtime/external-types.toml b/aws/rust-runtime/aws-runtime/external-types.toml index 0449ba09bbd..941f09767bf 100644 --- a/aws/rust-runtime/aws-runtime/external-types.toml +++ b/aws/rust-runtime/aws-runtime/external-types.toml @@ -1,14 +1,6 @@ allowed_external_types = [ - "aws_credential_types::*", "aws_sigv4::*", - "aws_smithy_http::*", "aws_smithy_types::*", "aws_smithy_runtime_api::*", "aws_types::*", - # TODO(audit-external-type-usage) We should newtype these or otherwise avoid exposing them - "http::header::name::HeaderName", - "http::header::value::HeaderValue", - "http::request::Request", - "http::response::Response", - "http::uri::Uri", ] diff --git a/aws/rust-runtime/aws-runtime/src/request_info.rs b/aws/rust-runtime/aws-runtime/src/request_info.rs index 1a2e066b49e..b07c3479d70 100644 --- a/aws/rust-runtime/aws-runtime/src/request_info.rs +++ b/aws/rust-runtime/aws-runtime/src/request_info.rs @@ -129,19 +129,19 @@ impl Intercept for RequestInfoInterceptor { /// retry information header that is sent with every request. The information conveyed by this /// header allows services to anticipate whether a client will time out or retry a request. #[derive(Default, Debug)] -pub struct RequestPairs { +struct RequestPairs { inner: Vec<(Cow<'static, str>, Cow<'static, str>)>, } impl RequestPairs { /// Creates a new `RequestPairs` builder. - pub fn new() -> Self { + fn new() -> Self { Default::default() } /// Adds a pair to the `RequestPairs` builder. /// Only strings that can be converted to header values are considered valid. - pub fn with_pair( + fn with_pair( mut self, pair: (impl Into>, impl Into>), ) -> Self { @@ -151,7 +151,7 @@ impl RequestPairs { } /// Converts the `RequestPairs` builder into a `HeaderValue`. - pub fn try_into_header_value(self) -> Result { + fn try_into_header_value(self) -> Result { self.try_into() } } diff --git a/aws/rust-runtime/aws-sigv4/external-types.toml b/aws/rust-runtime/aws-sigv4/external-types.toml index 6c80133e45d..94701a5285b 100644 --- a/aws/rust-runtime/aws-sigv4/external-types.toml +++ b/aws/rust-runtime/aws-sigv4/external-types.toml @@ -1,5 +1,5 @@ allowed_external_types = [ - # TODO(refactorHttp): Remove this and remove the signing helpers + # TODO(https://github.com/smithy-lang/smithy-rs/issues/1193): Once tooling permits it, only allow the following types in the `http0-compat` feature "http::request::Request", # TODO(https://github.com/smithy-lang/smithy-rs/issues/1193): Once tooling permits it, only allow the following types in the `event-stream` feature "aws_smithy_types::event_stream::Message", diff --git a/aws/rust-runtime/aws-sigv4/src/http_request.rs b/aws/rust-runtime/aws-sigv4/src/http_request.rs index 5616509eaf2..6bb6b9235c2 100644 --- a/aws/rust-runtime/aws-sigv4/src/http_request.rs +++ b/aws/rust-runtime/aws-sigv4/src/http_request.rs @@ -49,7 +49,7 @@ //! let mut my_req = http::Request::new("..."); //! // Sign and then apply the signature to the request //! let (signing_instructions, _signature) = sign(signable_request, &signing_params)?.into_parts(); -//! signing_instructions.apply_to_request(&mut my_req); +//! signing_instructions.apply_to_request_http0x(&mut my_req); //! # Ok(()) //! # } //! ``` diff --git a/aws/rust-runtime/aws-sigv4/src/http_request/sign.rs b/aws/rust-runtime/aws-sigv4/src/http_request/sign.rs index 12d1e6cb522..2cef0dbd3ce 100644 --- a/aws/rust-runtime/aws-sigv4/src/http_request/sign.rs +++ b/aws/rust-runtime/aws-sigv4/src/http_request/sign.rs @@ -162,7 +162,7 @@ impl SigningInstructions { #[cfg(any(feature = "http0-compat", test))] /// Applies the instructions to the given `request`. - pub fn apply_to_request(self, request: &mut http::Request) { + pub fn apply_to_request_http0x(self, request: &mut http::Request) { let (new_headers, new_query) = self.into_parts(); for header in new_headers.into_iter() { let mut value = http::HeaderValue::from_str(&header.value).unwrap(); @@ -492,7 +492,7 @@ mod tests { ); let mut signed = original.as_http_request(); - out.output.apply_to_request(&mut signed); + out.output.apply_to_request_http0x(&mut signed); let expected = test::v4::test_signed_request("get-vanilla-query-order-key-case"); assert_req_eq!(expected, signed); @@ -547,7 +547,8 @@ mod tests { let out = sign(signable_req, ¶ms).unwrap(); // Sigv4a signatures are non-deterministic, so we can't compare the signature directly. - out.output.apply_to_request(&mut req.as_http_request()); + out.output + .apply_to_request_http0x(&mut req.as_http_request()); let creds = params.credentials().unwrap(); let signing_key = @@ -777,7 +778,7 @@ mod tests { ); let mut signed = original.as_http_request(); - out.output.apply_to_request(&mut signed); + out.output.apply_to_request_http0x(&mut signed); let expected = test::v4::test_signed_request(test); assert_req_eq!(expected, signed); @@ -809,7 +810,7 @@ mod tests { ); let mut signed = original.as_http_request(); - out.output.apply_to_request(&mut signed); + out.output.apply_to_request_http0x(&mut signed); let expected = test::v4::test_signed_request_query_params("get-vanilla-query-order-key-case"); @@ -843,7 +844,7 @@ mod tests { ); let mut signed = original.as_http_request(); - out.output.apply_to_request(&mut signed); + out.output.apply_to_request_http0x(&mut signed); let expected = http::Request::builder() .uri("https://some-endpoint.some-region.amazonaws.com") @@ -904,7 +905,7 @@ mod tests { let mut signed = original.as_http_request(); out_with_session_token_but_excluded .output - .apply_to_request(&mut signed); + .apply_to_request_http0x(&mut signed); let expected = http::Request::builder() .uri("https://some-endpoint.some-region.amazonaws.com") @@ -961,7 +962,7 @@ mod tests { ); let mut signed = original.as_http_request(); - out.output.apply_to_request(&mut signed); + out.output.apply_to_request_http0x(&mut signed); let expected = http::Request::builder() .uri("https://some-endpoint.some-region.amazonaws.com") @@ -1031,7 +1032,7 @@ mod tests { .body("") .unwrap(); - instructions.apply_to_request(&mut request); + instructions.apply_to_request_http0x(&mut request); let get_header = |n: &str| request.headers().get(n).unwrap().to_str().unwrap(); assert_eq!("foo", get_header("some-header")); @@ -1051,7 +1052,7 @@ mod tests { .body("") .unwrap(); - instructions.apply_to_request(&mut request); + instructions.apply_to_request_http0x(&mut request); assert_eq!( "/some/path?some-param=f%26o%3Fo&some-other-param%3F=bar", diff --git a/aws/sdk/sdk-external-types.toml b/aws/sdk/sdk-external-types.toml index a58f912dff7..0fa9bbc3a21 100644 --- a/aws/sdk/sdk-external-types.toml +++ b/aws/sdk/sdk-external-types.toml @@ -13,19 +13,11 @@ allowed_external_types = [ # TODO(https://github.com/smithy-lang/smithy-rs/issues/1193): Once tooling permits it, only allow the following types in the `event-stream` feature "aws_smithy_eventstream::*", - # Consider moving making this crate 1.0 "aws_smithy_runtime::client::identity::cache::IdentityCache", - ### All below this line need to be fixed: - # The following are not OK - "aws_http::request_id::RequestId", # move to aws-types - # `set_invocation_id_generator` — consider make pub(crate) or moving trait to AwsTypes "aws_runtime::invocation_id::SharedInvocationIdGenerator", "aws_runtime::invocation_id::InvocationIdGenerator", - # Pending fix in open PRs: - "aws_smithy_http::result::*", - - # Pending future fix - "aws_smithy_http::event_stream::*", + # Only exposed in transcribestreaming. This crate will be major version bumped if we MV aws_smithy_http + "aws_smithy_http::event_stream::sender::EventStreamSender", ] diff --git a/rust-runtime/aws-smithy-runtime/external-types.toml b/rust-runtime/aws-smithy-runtime/external-types.toml index 0c0d03c3057..f9dffb758c2 100644 --- a/rust-runtime/aws-smithy-runtime/external-types.toml +++ b/rust-runtime/aws-smithy-runtime/external-types.toml @@ -1,16 +1,9 @@ allowed_external_types = [ "aws_smithy_runtime_api::*", "aws_smithy_async::*", - "aws_smithy_http::*", "aws_smithy_types::*", - # TODO(audit-external-type-usage) We should newtype these or otherwise avoid exposing them - "http::header::name::HeaderName", - "http::request::Request", - "http::response::Response", - "http::uri::Uri", - - # Used for creating hyper connectors + # Used for creating hyper connectors in the hyper feature and test-util features "tower_service::Service", # TODO(https://github.com/smithy-lang/smithy-rs/issues/1193): Once tooling permits it, only allow the following types in the `test-util` feature @@ -19,6 +12,12 @@ allowed_external_types = [ "serde::ser::Serialize", "serde::de::Deserialize", "hyper::client::connect::dns::Name", + # used by dvr + "http::request::Request", + "http::response::Response", + "http::uri::Uri", + + # TODO(https://github.com/smithy-lang/smithy-rs/issues/1193): Once tooling permits it, only allow the following types in the `connector-hyper-0-14-x` feature "hyper::client::client::Builder", diff --git a/rust-runtime/aws-smithy-runtime/src/lib.rs b/rust-runtime/aws-smithy-runtime/src/lib.rs index 2fbb5966576..3a7974e9b5d 100644 --- a/rust-runtime/aws-smithy-runtime/src/lib.rs +++ b/rust-runtime/aws-smithy-runtime/src/lib.rs @@ -17,6 +17,7 @@ unreachable_pub, rust_2018_idioms )] +#![cfg_attr(docsrs, feature(doc_auto_cfg))] /// Runtime support logic for generated clients. #[cfg(feature = "client")]