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.

Sign up for GitHub

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

Jump to bottom

s3: make streaming operations use unsigned payload by default #1354

Merged
merged 5 commits into from
Oct 21, 2021
Merged

s3: make streaming operations use unsigned payload by default #1354

merged 5 commits into from
Oct 21, 2021

Conversation

skotambkar
Copy link
Contributor

Adds SwapPayloadSHA256ResolverMiddleware that swaps computPayloadSHA256 middleware if an S3 operation uses streaming payload.

For S3 operations with streaming payload, we use unsigned payload strategy if TLS is enabled.

@skotambkar skotambkar force-pushed the s3-default-to-unsignedpayload branch from 9dd369e to 83bbc10 Compare August 4, 2021 07:00
@skotambkar skotambkar requested review from skmcgrail and jasdel August 4, 2021 17:36
@skotambkar skotambkar force-pushed the s3-default-to-unsignedpayload branch 2 times, most recently from d794480 to b8c9e38 Compare October 15, 2021 08:01
@skotambkar skotambkar force-pushed the s3-default-to-unsignedpayload branch from b8c9e38 to febfd4c Compare October 15, 2021 08:06
@skotambkar skotambkar requested a review from jasdel October 19, 2021 07:14
jasdel
jasdel suggested changes Oct 20, 2021
View reviewed changes
Copy link
Contributor

@jasdel jasdel left a comment

Choose a reason for hiding this comment

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

Looks good. It would be really helpful to include benchmark tests for this change vs before this change to compare the impact of switching to signing.

Also this should include a feature bump change note for the S3 API client.

aws/signer/v4/middleware.go Outdated Show resolved Hide resolved
...src/main/java/software/amazon/smithy/aws/go/codegen/customization/S3ContentSHA256Header.java Outdated Show resolved Hide resolved
service/s3/internal/customizations/write_get_object_response_test.go Show resolved Hide resolved
@skotambkar skotambkar force-pushed the s3-default-to-unsignedpayload branch from 457deca to 8711da2 Compare October 20, 2021 21:48
@jasdel jasdel merged commit 98b705e into aws:main Oct 21, 2021
jrichard8 pushed a commit to jrichard8/aws-sdk-go-v2 that referenced this pull request Feb 14, 2022
@skotambkar @jrichardpfs
service/s3: make streaming operations use unsigned payload by default (…
9a71d11 
…aws#1354)

Adds SwapPayloadSHA256ResolverMiddleware that swaps computPayloadSHA256 middleware if an S3 operation uses streaming payload.

For S3 operations with streaming payload, we use unsigned payload strategy if TLS is enabled.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasdel jasdel jasdel approved these changes

@skmcgrail skmcgrail skmcgrail approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@skotambkar @skmcgrail @jasdel