Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assorted cleanups of stable runtime crates #3205

Merged
merged 5 commits into from
Nov 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,22 @@
# 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"

[[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"

[[aws-sdk-rust]]
message = "The `AssumeRoleBuilder::policy_arns` now accepts strings instead of an STS specific type"
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
3 changes: 3 additions & 0 deletions aws/rust-runtime/aws-credential-types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,6 @@ all-features = true
targets = ["x86_64-unknown-linux-gnu"]
rustdoc-args = ["--cfg", "docsrs"]
# End of docs.rs metadata

[package.metadata.smithy-rs-release-tooling]
stable = true
3 changes: 3 additions & 0 deletions aws/rust-runtime/aws-runtime/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,6 @@ all-features = true
targets = ["x86_64-unknown-linux-gnu"]
rustdoc-args = ["--cfg", "docsrs"]
# End of docs.rs metadata

[package.metadata.smithy-rs-release-tooling]
stable = true
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
3 changes: 3 additions & 0 deletions aws/rust-runtime/aws-sigv4/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,6 @@ all-features = true
targets = ["x86_64-unknown-linux-gnu"]
rustdoc-args = ["--cfg", "docsrs"]
# End of docs.rs metadata

[package.metadata.smithy-rs-release-tooling]
stable = true
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
Loading