Skip to content

Commit

Permalink
Assorted cleanups of stable runtime crates
Browse files Browse the repository at this point in the history
  • Loading branch information
rcoh committed Nov 15, 2023
1 parent 572c5d7 commit 62d5b9f
Show file tree
Hide file tree
Showing 15 changed files with 64 additions and 75 deletions.
12 changes: 12 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
12 changes: 1 addition & 11 deletions aws/rust-runtime/aws-config/external-types.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
]
18 changes: 11 additions & 7 deletions aws/rust-runtime/aws-config/src/imds/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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();
/// # }
/// ```
Expand Down Expand Up @@ -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<Uri>) -> Self {
self.endpoint = Some(EndpointSource::Explicit(endpoint.into()));
self
pub fn endpoint(mut self, endpoint: impl AsRef<str>) -> Result<Self, BoxError> {
let uri: Uri = endpoint.as_ref().parse()?;
self.endpoint = Some(EndpointSource::Explicit(uri));
Ok(self)
}

/// Override the endpoint mode for [`Client`]
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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::<Uri>().unwrap());
imds_client = imds_client
.endpoint(endpoint_override)
.expect("invalid URI");
}

if let Some(mode_override) = test_case.mode_override {
Expand Down
9 changes: 0 additions & 9 deletions aws/rust-runtime/aws-config/src/imds/client/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -224,14 +223,6 @@ impl Error for BuildError {
}
}

impl From<InvalidEndpointError> for BuildError {
fn from(err: InvalidEndpointError) -> Self {
Self {
kind: BuildErrorKind::InvalidEndpointUri(err.into()),
}
}
}

#[derive(Debug)]
pub(super) enum TokenErrorKind {
/// The token was invalid
Expand Down
9 changes: 6 additions & 3 deletions aws/rust-runtime/aws-config/src/imds/credentials.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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)
Expand All @@ -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()
Expand Down
1 change: 0 additions & 1 deletion aws/rust-runtime/aws-config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down
9 changes: 7 additions & 2 deletions aws/rust-runtime/aws-config/src/sts/assume_role.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<PolicyDescriptorType>) -> Self {
self.policy_arns = Some(policy_arns);
pub fn policy_arns(mut self, policy_arns: Vec<String>) -> Self {
self.policy_arns = Some(
policy_arns
.into_iter()
.map(|arn| PolicyDescriptorType::builder().arn(arn).build())
.collect::<Vec<_>>(),
);
self
}

Expand Down
8 changes: 0 additions & 8 deletions aws/rust-runtime/aws-runtime/external-types.toml
Original file line number Diff line number Diff line change
@@ -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",
]
8 changes: 4 additions & 4 deletions aws/rust-runtime/aws-runtime/src/request_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Cow<'static, str>>, impl Into<Cow<'static, str>>),
) -> Self {
Expand All @@ -151,7 +151,7 @@ impl RequestPairs {
}

/// Converts the `RequestPairs` builder into a `HeaderValue`.
pub fn try_into_header_value(self) -> Result<HeaderValue, BoxError> {
fn try_into_header_value(self) -> Result<HeaderValue, BoxError> {
self.try_into()
}
}
Expand Down
2 changes: 1 addition & 1 deletion aws/rust-runtime/aws-sigv4/external-types.toml
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
2 changes: 1 addition & 1 deletion aws/rust-runtime/aws-sigv4/src/http_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
//! # }
//! ```
Expand Down
21 changes: 11 additions & 10 deletions aws/rust-runtime/aws-sigv4/src/http_request/sign.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ impl SigningInstructions {

#[cfg(any(feature = "http0-compat", test))]
/// Applies the instructions to the given `request`.
pub fn apply_to_request<B>(self, request: &mut http::Request<B>) {
pub fn apply_to_request_http0x<B>(self, request: &mut http::Request<B>) {
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();
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -547,7 +547,8 @@ mod tests {

let out = sign(signable_req, &params).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 =
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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"));
Expand All @@ -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",
Expand Down
12 changes: 2 additions & 10 deletions aws/sdk/sdk-external-types.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
]
15 changes: 7 additions & 8 deletions rust-runtime/aws-smithy-runtime/external-types.toml
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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",
Expand Down
1 change: 1 addition & 0 deletions rust-runtime/aws-smithy-runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down

0 comments on commit 62d5b9f

Please sign in to comment.