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

add: example showing how to use native-tls #946

Merged
merged 8 commits into from
Dec 9, 2021
Merged
10 changes: 10 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,16 @@
# meta = { "breaking" = false, "tada" = false, "bug" = false }
# author = "rcoh"

[[aws-sdk-rust]]
message = """
`aws-config` will now work as intended for users that want to use `native-tls` instead of `rustls`. Previously, it was
difficult to ensure that `rustls` was not in use. Also, there is now an example of how to use `native-tls` and a test
that ensures `rustls` is not in the dependency tree
"""
references = ["aws-sdk-rust#304"]
meta = { "breaking" = false, "tada" = false, "bug" = true }
author = "zhessler"

[[aws-sdk-rust]]
message = """
Removed inaccurate log message when a client was used without a sleep implementation, and
Expand Down
2 changes: 1 addition & 1 deletion aws/rust-runtime/aws-config/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ dns = ["tokio/rt"]
default = ["default-provider", "rustls", "rt-tokio", "dns", "tcp-connector"]

[dependencies]
aws-sdk-sts = { path = "../../sdk/build/aws-sdk/sdk/sts", optional = true }
aws-sdk-sts = { path = "../../sdk/build/aws-sdk/sdk/sts", default-features = false, optional = true }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the rustls and native-tls features in aws-config also enable the equivalent features on the STS crate? Or to reframe the question, is it working by coincidence right now without explicitly enabling those?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had that previously and removed it because it didn't seem to matter. The test runs and doesn't detect rustls even though aws-config is a dependency so I believe we're good. Personally, my gut tells me to include the features but I can't articulate why so I'm ignoring the feeling.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jdisanti they should not: this is because the only purpose of those in the STS crate are to support from_conf (constructing the client without a connection). But this crate will always provide a connection when constructing a client.

aws-smithy-async = { path = "../../sdk/build/aws-sdk/sdk/aws-smithy-async" }
aws-smithy-client = { path = "../../sdk/build/aws-sdk/sdk/aws-smithy-client" }
aws-smithy-types = { path = "../../sdk/build/aws-sdk/sdk/aws-smithy-types" }
Expand Down
4 changes: 2 additions & 2 deletions aws/rust-runtime/aws-endpoint/src/partition/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ impl ResolveAwsEndpoint for PartitionResolver {

#[derive(Debug)]
pub struct Partition {
id: &'static str,
_id: &'static str,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clippy was complaining about this locally and the field seemed unused so I added the underscore

region_regex: Regex,
partition_endpoint: Option<Region>,
regionalized: Regionalized,
Expand Down Expand Up @@ -114,7 +114,7 @@ impl Builder {
let default_endpoint = self.default_endpoint?;
let endpoints = self.endpoints.into_iter().collect();
Some(Partition {
id: self.id?,
_id: self.id?,
region_regex: self.region_regex?,
partition_endpoint: self.partition_endpoint,
regionalized: self.regionalized.unwrap_or_default(),
Expand Down
18 changes: 18 additions & 0 deletions aws/sdk/examples/using_native_tls_instead_of_rustls/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
[package]
name = "using_native_tls_instead_of_rustls"
version = "0.1.0"
authors = ["Zelda Hessler zhessler@amazon.com>"]
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
# aws-config pulls in rustls and several other things by default. We have to disable defaults in order to use native-tls
# and then manually bring the other defaults back
aws-config = { path = "../../build/aws-sdk/sdk/aws-config", default-features = false, features = ["default-provider", "native-tls", "rt-tokio", "dns", "tcp-connector"] }
# aws-sdk-s3 brings in rustls by default so we disable that in order to use native-tls only
aws-sdk-s3 = { package = "aws-sdk-s3", path = "../../build/aws-sdk/sdk/s3", default-features = false, features = ["native-tls"] }
# aws-sdk-sts is the same as aws-sdk-s3
aws-sdk-sts = { package = "aws-sdk-sts", path = "../../build/aws-sdk/sdk/sts", default-features = false, features = ["native-tls"] }
Comment on lines +10 to +16
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks to John's suggestion of removing default features for runtime crates, we don't even need to include them here!

tokio = { version = "1", features = ["full"] }
tracing-subscriber = "0.2.18"
56 changes: 56 additions & 0 deletions aws/sdk/examples/using_native_tls_instead_of_rustls/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0.
*/

/// The SDK defaults to using RusTLS by default but you can also use [`native_tls`](https://github.com/sfackler/rust-native-tls)
/// which will choose a TLS implementation appropriate for your platform. This example looks much like
/// any other. Activating and deactivating `features` in your app's `Cargo.toml` is all that's needed.
#[tokio::main]
async fn main() -> Result<(), aws_sdk_s3::Error> {
tracing_subscriber::fmt::init();

let shared_config = aws_config::load_from_env().await;

let s3_config = aws_sdk_s3::Config::from(&shared_config);
let client = aws_sdk_s3::Client::from_conf(s3_config);

let resp = client.list_buckets().send().await?;

for bucket in resp.buckets().unwrap_or_default() {
println!("bucket: {:?}", bucket.name().unwrap_or_default())
}

Ok(())
}

#[cfg(test)]
mod tests {
/// You can run this test to ensure that this example is only using `native-tls`
/// and that nothing is pulling in `rustls` as a dependency
#[test]
#[should_panic = "error: package ID specification `rustls` did not match any packages"]
fn test_rustls_is_not_in_dependency_tree() {
let cargo_location = std::env::var("CARGO").unwrap();
let cargo_command = std::process::Command::new(&cargo_location)
.arg("tree")
.arg("--invert")
.arg("rustls")
.output()
Comment on lines +34 to +39
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clever!

Does this differentiate between something being wrong with the cargo binary (not able to execute or something) vs. exiting with a non-zero status?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only checks that Cargo writes a specific message to stderr when the command is run. The possible paths I see are:

  1. rustls is not in tree, message is logged to stderr, we panic with that message and match on it with should_panic, it's a match so that's a test pass
  2. some other failure occurs, message is logged to stderr, we panic with that message and match on it with should_panic, It's not a match so that's a test fail
  3. rustls is in the tree, print tree and exit, test fails cause no panic

So, to answer your question: yes, it does differentiate

.expect("failed to run 'cargo tree'");

let stderr = String::from_utf8_lossy(&cargo_command.stderr);

// We expect the call to `cargo tree` to error out. If it did, we panic with the resulting
// message here. In the case that no error message is set, that's bad.
if !stderr.is_empty() {
panic!("{}", stderr);
}

// Uh oh. We expected an error message but got none, likely because `cargo tree` found
// `rustls` in our dependencies. We'll print out the message we got to see what went wrong.
let stdout = String::from_utf8_lossy(&cargo_command.stdout);

println!("{}", stdout)
}
}
2 changes: 1 addition & 1 deletion rust-runtime/aws-smithy-client/src/hyper_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ impl Builder {
}
}

#[cfg(any(feature = "rustls", feature = "native_tls"))]
#[cfg(any(feature = "rustls", feature = "native-tls"))]
Velfi marked this conversation as resolved.
Show resolved Hide resolved
impl<M> crate::Builder<crate::erase::DynConnector, M>
where
M: Default,
Expand Down