Skip to content

Commit

Permalink
Add placeholder types for S3 Express and enable control flow to be re…
Browse files Browse the repository at this point in the history
…directed for S3 Express use case (#3386)

## Motivation and Context
This PR is the first in the series to support the S3 Express feature in
the Rust SDK. The work will be done in the feature branch, and once it
is code complete, the branch will be merged to main.

## Description
This PR adds placeholder types for S3 Express and enables control flow
to be redirected for S3 Express use case. For instance, if we run the
following example code against a generated SDK from this PR:
```rust
let shared_config = aws_config::from_env().region(aws_sdk_s3::config::Region::new("us-east-1")).load().await;
let client = aws_sdk_s3::Client::new(&shared_config);
client.list_objects_v2().bucket("testbucket--use1-az4--x-s3";).send().await.unwrap();
```
it will end up
```
thread 's3_express' panicked at 'not yet implemented', /Users/awsaito/src/smithy-rs/aws/sdk/build/aws-sdk/sdk/s3/src/s3_express.rs:104:13
```
which points to
```
impl ProvideCredentials for DefaultS3ExpressIdentityProvider {
    fn provide_credentials<'a>(&'a self) -> aws_credential_types::provider::future::ProvideCredentials<'a>
    where
        Self: 'a,
    {
        todo!() <---
    }
}
```

### Implementation decisions
- `DefaultS3ExpressIdentityProvider` has an accompanying identity cache.
That identity cache cannot be configured by customers so it makes sense
for the provider itself to internally own it. In that case, we do NOT
want to use the identity cache stored in `RuntimeComponents`, since it
interferes with the S3 Express's caching policy. To that end, I added an
enum `CacheLocation` to `SharedIdentityResolver` (it already had the
`cache_partition` field so it was kind of aware of caching).
- Two reasons why `CacheLocation` is added to `SharedIdentityResolver`,
but not to individual, concrete `IdentityResolver`s. One is
`SharedIdentityResolver` was already cache aware, as mentioned above.
The other is that it is more flexible that way; The cache location is
not tied to a type of identity resolver, but we can select it when
creating a `SharedIdentityResolver`.
- I considered but did not add a field `cacheable` to `Identity` since I
wanted to keep `Identity` as plain data, keeping the concept of
"caching" somewhere outside.
- I've added a separate `Config` method,
`set_express_credentials_provider`, to override credentials provider for
S3 Express. There are other SDKs (e.g.
[Ruby](https://www.rubydoc.info/gems/aws-sdk-s3/Aws/S3/Client)) that
follow this style and it makes it clear to the customers that this is
the method to use when overriding the express credentials provider. The
existing `set_credentials_provider`, given its input type, cannot tell
whether a passed-in credentials provider is for a regular `sigv4` or for
S3 Express.

## Testing
Only verified that control flow could be altered for an S3 Express use
case, as shown above. Further testing will be added in subsequent PRs.

## Checklist
I am planning to include in `CHANGELOG.next.toml` a user guide for S3
Express once the feature branch `ysaito/s3express` is ready to be merged
to main.

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._

---------

Co-authored-by: John DiSanti <[email protected]>
Co-authored-by: AWS SDK Rust Bot <[email protected]>
Co-authored-by: AWS SDK Rust Bot <[email protected]>
Co-authored-by: Zelda Hessler <[email protected]>
  • Loading branch information
5 people authored Feb 10, 2024
1 parent ae52b69 commit 39be6ce
Show file tree
Hide file tree
Showing 28 changed files with 988 additions and 139 deletions.
28 changes: 27 additions & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,37 @@ jobs:
run_sdk_examples: false
git_ref: ${{ inputs.commit_sha }}

check-semver-hazards:
name: Check for semver hazards
needs:
- acquire-base-image
# We need `always` here otherwise this job won't run if the previous job has been skipped
# See https://samanpavel.medium.com/github-actions-conditional-job-execution-e6aa363d2867
if: always()
runs-on: smithy_ubuntu-latest_8-core
steps:
- uses: actions/checkout@v3
with:
path: smithy-rs
ref: ${{ inputs.commit_sha }}
fetch-depth: 0
- uses: actions/checkout@v3
with:
repository: awslabs/aws-sdk-rust
path: aws-sdk-rust
fetch-depth: 0
- name: Run check-semver-hazards
uses: ./smithy-rs/.github/actions/docker-build
with:
action: check-semver-hazards
action-arguments: ${{ inputs.stable_semantic_version }} ${{ inputs.unstable_semantic_version }}

get-or-create-release-branch:
name: Get or create a release branch
needs:
- release-ci
- acquire-base-image
- check-semver-hazards
- release-ci
# We need `always` here otherwise this job won't run if the previous job has been skipped
# See https://samanpavel.medium.com/github-actions-conditional-job-execution-e6aa363d2867
if: |
Expand Down
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
<!-- Do not manually edit this file. Use the `changelogger` tool. -->
February 8th, 2024
==================

January 24th, 2024
==================

Expand Down
52 changes: 52 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,55 @@ message = "Added impl `Display` to Enums."
references = ["smithy-rs#3336", "smithy-rs#3391"]
meta = { "breaking" = false, "tada" = false, "bug" = false}
author = "iampkmone"

[[aws-sdk-rust]]
message = """
Retry classifiers will now be sorted by priority. This change only affects requests
that are retried. Some requests that were previously been classified as transient
errors may now be classified as throttling errors.
If you were
- configuring multiple custom retry classifiers
- that would disagree on how to classify a response
- that have differing priorities
you may see a behavior change in that classification for the same response is now
dependent on the classifier priority instead of the order in which the classifier
was added.
"""
references = ["smithy-rs#3322"]
meta = { "breaking" = false, "bug" = true, "tada" = false }
author = "Velfi"

[[smithy-rs]]
message = """
Retry classifiers will now be sorted by priority. This change only affects requests
that are retried. Some requests that were previously been classified as transient
errors may now be classified as throttling errors.
If you were
- configuring multiple custom retry classifiers
- that would disagree on how to classify a response
- that have differing priorities
you may see a behavior change in that classification for the same response is now
dependent on the classifier priority instead of the order in which the classifier
was added.
"""
references = ["smithy-rs#3322"]
meta = { "breaking" = false, "bug" = true, "tada" = false }
author = "Velfi"

[[smithy-rs]]
message = "Cap the maximum jitter fraction for identity cache refresh buffer time to 0.5. It was previously 1.0, and if the fraction was randomly set to 1.0, it was equivalent to disregarding the buffer time for cache refresh."
references = ["smithy-rs#3402"]
meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "client" }
author = "ysaito1001"

[[aws-sdk-rust]]
message = "Cap the maximum jitter fraction for credentials cache refresh buffer time to 0.5. It was previously 1.0, and if the fraction was randomly set to 1.0, it was equivalent to disregarding the buffer time for cache refresh."
references = ["smithy-rs#3402"]
meta = { "breaking" = false, "tada" = false, "bug" = true }
author = "ysaito1001"
36 changes: 11 additions & 25 deletions aws/SDK_CHANGELOG.next.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,6 @@
{
"smithy-rs": [],
"aws-sdk-rust": [
{
"message": "`crate::event_receiver::EventReceiver` is now re-exported as `crate::primitives::event_stream::EventReceiver` when a service supports event stream operations.",
"meta": {
"bug": true,
"breaking": false,
"tada": false
},
"author": "ysaito1001",
"references": [
"smithy-rs#3305"
],
"since-commit": "9f0ba850e03241f657e2e40ca185780e0a5878cb",
"age": 5
},
{
"message": "Add support for constructing [`SdkBody`] and [`ByteStream`] from `http-body` 1.0 bodies. Note that this is initial support and works via a backwards compatibility shim to http-body 0.4. Hyper 1.0 is not supported.",
"meta": {
Expand All @@ -32,7 +18,7 @@
"aws-sdk-rust#977"
],
"since-commit": "e235a2fd9ec45335a3b2018028c2d3a2ac13ffdf",
"age": 3
"age": 4
},
{
"message": " Add `PaginationStreamExt` extension trait to `aws-smithy-types-convert` behind the `convert-streams` feature. This makes it possible to treat a paginator as a [`futures_core::Stream`](https://docs.rs/futures-core/latest/futures_core/stream/trait.Stream.html), allowing customers to use stream combinators like [`map`](https://docs.rs/tokio-stream/latest/tokio_stream/trait.StreamExt.html#method.map) and [`filter`](https://docs.rs/tokio-stream/latest/tokio_stream/trait.StreamExt.html#method.filter).\n\nExample:\n\n```rust\nuse aws_smithy_types_convert::stream::PaginationStreamExt\nlet stream = s3_client.list_objects_v2().bucket(\"...\").into_paginator().send().into_stream_03x();\n```\n",
Expand All @@ -46,7 +32,7 @@
"smithy-rs#3299"
],
"since-commit": "e235a2fd9ec45335a3b2018028c2d3a2ac13ffdf",
"age": 3
"age": 4
},
{
"message": "Serialize 0/false in query parameters, and ignore actual default value during serialization instead of just 0/false. See [changelog discussion](https://github.com/smithy-lang/smithy-rs/discussions/3312) for details.",
Expand All @@ -61,7 +47,7 @@
"smithy-rs#3312"
],
"since-commit": "e235a2fd9ec45335a3b2018028c2d3a2ac13ffdf",
"age": 3
"age": 4
},
{
"message": "Add `as_service_err()` to `SdkError` to allow checking the type of an error is without taking ownership.",
Expand All @@ -77,7 +63,7 @@
"aws-sdk-rust#1010"
],
"since-commit": "e235a2fd9ec45335a3b2018028c2d3a2ac13ffdf",
"age": 3
"age": 4
},
{
"message": "Fix bug in `CredentialsProcess` provider where `expiry` was incorrectly treated as a required field.",
Expand All @@ -92,7 +78,7 @@
"aws-sdk-rust#1021"
],
"since-commit": "e235a2fd9ec45335a3b2018028c2d3a2ac13ffdf",
"age": 3
"age": 4
},
{
"message": "~/.aws/config and ~/.aws/credentials now parse keys in a case-insensitive way. This means the `AWS_SECRET_ACCESS_KEY` is supported in addition to `aws_secret_access_key`.",
Expand All @@ -108,7 +94,7 @@
"smithy-rs#3344"
],
"since-commit": "e235a2fd9ec45335a3b2018028c2d3a2ac13ffdf",
"age": 3
"age": 4
},
{
"message": "`EndpointPrefix` and `apply_endpoint` moved from aws-smithy-http to aws-smithy-runtime-api so that is in a stable (1.x) crate. A deprecated type alias was left in place with a note showing the new location.",
Expand All @@ -122,7 +108,7 @@
"smithy-rs#3318"
],
"since-commit": "edf6e77bfa991aef9afa5acf293a911f7982511a",
"age": 2
"age": 3
},
{
"message": "Fix bug where overriding the credentials at the operation level failed if credentials were already set.",
Expand All @@ -137,7 +123,7 @@
"smithy-rs#3363"
],
"since-commit": "edf6e77bfa991aef9afa5acf293a911f7982511a",
"age": 2
"age": 3
},
{
"message": "Add `apply_to_request_http1x` to `aws-sigv4` to enable signing http = 1.0 requests.",
Expand All @@ -152,7 +138,7 @@
"smithy-rs#3366"
],
"since-commit": "edf6e77bfa991aef9afa5acf293a911f7982511a",
"age": 2
"age": 3
},
{
"message": "The types in the aws-http crate were moved into aws-runtime. Deprecated type aliases were put in place to point to the new locations.",
Expand All @@ -166,7 +152,7 @@
"smithy-rs#3355"
],
"since-commit": "a781be3cd8d22f4ebb5c06a758ddd5f1d6824ded",
"age": 1
"age": 2
},
{
"message": "Add support for `[sso-session]` in AWS config file for AWS Identity Center SSO credentials. Note that this does not include support for AWS Builder ID SSO sessions for services such as Code Catalyst (these lack the `sso_account_id` and `sso_role_name` fields in the profile config). Support for AWS Builder IDs is still being tracked in https://github.com/awslabs/aws-sdk-rust/issues/703.",
Expand All @@ -181,7 +167,7 @@
"smithy-rs#3379"
],
"since-commit": "a781be3cd8d22f4ebb5c06a758ddd5f1d6824ded",
"age": 1
"age": 2
}
],
"aws-sdk-model": []
Expand Down
5 changes: 5 additions & 0 deletions aws/rust-runtime/aws-inlineable/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ pub mod presigning;
/// Presigning interceptors
pub mod presigning_interceptors;

// This module uses module paths that assume the target crate to which it is copied, e.g.
// `crate::config::endpoint::Params`. If included into `aws-inlineable`, this module would
// fail to compile.
// pub mod s3_express;

/// Special logic for extracting request IDs from S3's responses.
#[allow(dead_code)]
pub mod s3_request_id;
Expand Down
125 changes: 125 additions & 0 deletions aws/rust-runtime/aws-inlineable/src/s3_express.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

/// Supporting code for S3 Express auth
pub(crate) mod auth {
use aws_smithy_runtime_api::box_error::BoxError;
use aws_smithy_runtime_api::client::auth::{
AuthScheme, AuthSchemeEndpointConfig, AuthSchemeId, Sign,
};
use aws_smithy_runtime_api::client::identity::{Identity, SharedIdentityResolver};
use aws_smithy_runtime_api::client::orchestrator::HttpRequest;
use aws_smithy_runtime_api::client::runtime_components::{
GetIdentityResolver, RuntimeComponents,
};
use aws_smithy_types::config_bag::ConfigBag;

/// Auth scheme ID for S3 Express.
pub(crate) const SCHEME_ID: AuthSchemeId = AuthSchemeId::new("sigv4-s3express");

/// S3 Express auth scheme.
#[derive(Debug, Default)]
pub(crate) struct S3ExpressAuthScheme {
signer: S3ExpressSigner,
}

impl S3ExpressAuthScheme {
/// Creates a new `S3ExpressAuthScheme`.
pub(crate) fn new() -> Self {
Default::default()
}
}

impl AuthScheme for S3ExpressAuthScheme {
fn scheme_id(&self) -> AuthSchemeId {
SCHEME_ID
}

fn identity_resolver(
&self,
identity_resolvers: &dyn GetIdentityResolver,
) -> Option<SharedIdentityResolver> {
identity_resolvers.identity_resolver(self.scheme_id())
}

fn signer(&self) -> &dyn Sign {
&self.signer
}
}

/// S3 Express signer.
#[derive(Debug, Default)]
pub(crate) struct S3ExpressSigner;

impl Sign for S3ExpressSigner {
fn sign_http_request(
&self,
_request: &mut HttpRequest,
_identity: &Identity,
_auth_scheme_endpoint_config: AuthSchemeEndpointConfig<'_>,
_runtime_components: &RuntimeComponents,
_config_bag: &ConfigBag,
) -> Result<(), BoxError> {
todo!()
}
}
}

/// Supporting code for S3 Express identity cache
pub(crate) mod identity_cache {
/// The caching implementation for S3 Express identity.
///
/// While customers can either disable S3 Express itself or provide a custom S3 Express identity
/// provider, configuring S3 Express identity cache is not supported. Thus, this is _the_
/// implementation of S3 Express identity cache.
#[derive(Debug)]
pub(crate) struct S3ExpressIdentityCache;
}

/// Supporting code for S3 Express identity provider
pub(crate) mod identity_provider {
use crate::s3_express::identity_cache::S3ExpressIdentityCache;
use aws_smithy_runtime_api::client::identity::{
IdentityCacheLocation, IdentityFuture, ResolveIdentity,
};
use aws_smithy_runtime_api::client::runtime_components::RuntimeComponents;
use aws_smithy_types::config_bag::ConfigBag;

#[derive(Debug)]
pub(crate) struct DefaultS3ExpressIdentityProvider {
_cache: S3ExpressIdentityCache,
}

#[derive(Default)]
pub(crate) struct Builder;

impl DefaultS3ExpressIdentityProvider {
pub(crate) fn builder() -> Builder {
Builder
}
}

impl Builder {
pub(crate) fn build(self) -> DefaultS3ExpressIdentityProvider {
DefaultS3ExpressIdentityProvider {
_cache: S3ExpressIdentityCache,
}
}
}

impl ResolveIdentity for DefaultS3ExpressIdentityProvider {
fn resolve_identity<'a>(
&'a self,
_runtime_components: &'a RuntimeComponents,
_config_bag: &'a ConfigBag,
) -> IdentityFuture<'a> {
todo!()
}

fn cache_location(&self) -> IdentityCacheLocation {
IdentityCacheLocation::IdentityResolver
}
}
}
2 changes: 1 addition & 1 deletion aws/rust-runtime/aws-runtime/src/retries/classifiers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ where
}

fn priority(&self) -> RetryClassifierPriority {
RetryClassifierPriority::with_lower_priority_than(
RetryClassifierPriority::run_before(
RetryClassifierPriority::modeled_as_retryable_classifier(),
)
}
Expand Down
7 changes: 6 additions & 1 deletion aws/rust-runtime/aws-sigv4/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ zeroize = { version = "^1", optional = true }
aws-credential-types = { path = "../aws-credential-types", features = ["test-util", "hardcoded-credentials"] }
aws-smithy-runtime-api = { path = "../../../rust-runtime/aws-smithy-runtime-api", features = ["client", "test-util"] }
bytes = "1"
criterion = "0.5"
hex-literal = "0.4.1"
httparse = "1.8"
libfuzzer-sys = "0.4.6"
Expand All @@ -55,6 +54,12 @@ serde_derive = "1.0.180"
serde_json = "1.0.104"
time = { version = "0.3.5", features = ["parsing"] }

# TODO(https://github.com/smithy-lang/smithy-rs/issues/3398): Remove clap dependency once the SDK MSRV is 1.74.
# Clap was added and pinned to 4.4.x because it is pulled in by criterion, and 4.5.x requires an MSRV of Rust 1.74.
# Since the SDK MSRV is 1.72, this causes it to fail to compile.
clap = "~4.4"
criterion = "0.5"

[target.'cfg(not(any(target_arch = "powerpc", target_arch = "powerpc64")))'.dev-dependencies]
ring = "0.17.5"

Expand Down
Loading

0 comments on commit 39be6ce

Please sign in to comment.