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

Drop S3ExpressSigner and override session token name via RuntimePlugin #3457

Merged

Conversation

ysaito1001
Copy link
Contributor

@ysaito1001 ysaito1001 commented Mar 1, 2024

Motivation and Context

In response to #3455 (comment), it's clear that we need to iterate more on the potential signing API.

This PR takes a different approach where S3ExpressSigner is dropped and a session token name override, if any, is placed into and retrieved from ConfigBag, which is used within SigV4Signer::sign_http_request.

Testing

Existing tests in CI

A semver failure in CI can be ignored; A public API SigV4Signer::sign_http_request has been removed that only existed in the branch.


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

Copy link

github-actions bot commented Mar 1, 2024

A new generated diff is ready to view.

  • AWS SDK (ignoring whitespace)
  • No codegen difference in the Client Test
  • No codegen difference in the Server Test
  • No codegen difference in the Server Test Python
  • No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

@ysaito1001 ysaito1001 marked this pull request as ready for review March 1, 2024 22:35
@ysaito1001 ysaito1001 requested a review from a team as a code owner March 1, 2024 22:35
@ysaito1001 ysaito1001 changed the title Inline SigV4Signer::sign_http_request into S3ExpressSigner Drop S3ExpressSigner and override session token name via RuntimePlugin Mar 5, 2024
Copy link
Collaborator

@jdisanti jdisanti left a comment

Choose a reason for hiding this comment

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

Looks great!

aws/rust-runtime/aws-runtime/src/auth.rs Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Mar 5, 2024

A new generated diff is ready to view.

  • AWS SDK (ignoring whitespace)
  • No codegen difference in the Client Test
  • No codegen difference in the Server Test
  • No codegen difference in the Server Test Python
  • No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

Copy link

github-actions bot commented Mar 5, 2024

A new generated diff is ready to view.

  • AWS SDK (ignoring whitespace)
  • No codegen difference in the Client Test
  • No codegen difference in the Server Test
  • No codegen difference in the Server Test Python
  • No codegen difference in the Server Test Typescript

A new doc preview is ready to view.

@ysaito1001 ysaito1001 merged commit 8d056bd into ysaito/s3express Mar 6, 2024
40 of 41 checks passed
@ysaito1001 ysaito1001 deleted the s3express-inline-sigv4-sign-http-request branch March 6, 2024 01:38
github-merge-queue bot pushed a commit that referenced this pull request Mar 11, 2024
## Motivation and Context
Allows the Rust SDK to use [S3 Express One
Zone](https://aws.amazon.com/s3/storage-classes/express-one-zone/)

## Description
The PR adds the said S3-specific functionality to the Rust SDK. The code
changes have already been reviewed by previous sub PRs, but it's worth
going through them again as a whole:
- #3386
- #3388
- #3390
- #3432
- #3433
- #3459
- #3457
- #3462

In addition to the PRs above, commit eebe8af increases the canary
lambda's memory size to 512MB from 128MB (also makes it configurable
through a command line arg for `canary-runner`). By default, lambda's
allowed memory size is 128MB but with the addition of `canary-wasm` in
main, canary lambda's memory usage will be 152MB, causing the lambda to
be killed by a signal during runtime. The commit addresses that issue.

## Testing
- Unit tests in
[aws/rust-runtime/aws-inlineable/src/s3_express.rs](https://github.com/smithy-lang/smithy-rs/blob/7f8c28b7038372927ec6196eff88384452f908dd/aws/rust-runtime/aws-inlineable/src/s3_express.rs)
- Integration tests in
[aws/sdk/integration-tests/s3/tests/express.rs](https://github.com/smithy-lang/smithy-rs/blob/7f8c28b7038372927ec6196eff88384452f908dd/aws/sdk/integration-tests/s3/tests/express.rs)
- Canary in smithy-rs#3462

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [x] I have updated `CHANGELOG.next.toml` if I made changes to the AWS
SDK, generated SDK code, or SDK runtime crates

----

_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]>
Co-authored-by: Russell Cohen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants