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

[DO NOT MERGE] Feat flex checksum #2808

Open
wants to merge 41 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
e89e0c9
add config and change default behavior of checksum middleware
Sep 25, 2024
ee0fb5b
merge main into branch
Sep 25, 2024
0efe635
regenerate client
Sep 25, 2024
bc3f437
regenerate clients
Sep 26, 2024
58bf509
merge protocol test code from main
Sep 26, 2024
ab72530
merge from main
Sep 26, 2024
b0a53a4
add more test cases
Sep 26, 2024
f7b5a2d
add changelog
Sep 26, 2024
b93f439
modify s3 internal test
Sep 26, 2024
72c3ba6
separate checksum config check and workflow
Sep 29, 2024
f70fb1d
Merge branch 'main' into feat-flex-checksum
Sep 29, 2024
7bc5326
restore s3 test
Sep 29, 2024
28e8ac4
remove unused md5 header
Sep 30, 2024
23a7d0e
separate checksum config and workflow
Oct 1, 2024
e12cb30
Merge branch 'main' into feat-flex-checksum
Oct 1, 2024
df759d3
change default checksum to const
Oct 1, 2024
93aee16
add checksum unset enum and modify comment of cfg
Oct 2, 2024
b147f0f
change comment
Oct 7, 2024
db21450
Merge branch 'main' into feat-flex-checksum
Oct 7, 2024
3eb7ed7
Update aws/checksum.go
lucix-aws Oct 8, 2024
6be56d3
change checksum value check logic
Oct 21, 2024
05ed9a0
remove old check
Oct 21, 2024
3b801c2
correct unseekable stream logic without tls and its test cases
Nov 20, 2024
606d1ad
resolve merge conflict
Nov 20, 2024
59e520b
revert extra codegen
Nov 20, 2024
cda7bcc
change tmv1 upload test cases after introducing flex checksum
Nov 22, 2024
e912c0a
Merge branch 'main' into feat-flex-checksum
Nov 22, 2024
f84cffb
add error test case for crc64
Nov 26, 2024
2df24d6
change test name
Nov 26, 2024
1cc4bfc
Merge branch 'main' into feat-flex-checksum
Nov 26, 2024
16f3efe
default tmv1 checksum and add flex checksum metrics tracking
Dec 7, 2024
2288595
Merge branch 'main' into feat-flex-checksum
Dec 7, 2024
24c37b1
regenerate client and add metrics mw test
Dec 9, 2024
7dde0a9
add comment to exported type
Dec 9, 2024
c6f34d2
update s3 snapshot
Dec 9, 2024
29f1c9c
update tmv1 integ test
Dec 10, 2024
3f79e71
Merge branch 'main' into feat-flex-checksum
Dec 10, 2024
9ff1219
exclude default checksum from presign op
Dec 11, 2024
4dd0258
Merge branch 'main' into feat-flex-checksum
Dec 11, 2024
25d943c
reorder feature id and simplify metric tracking test
Dec 18, 2024
bf57f67
Merge branch 'main' into feat-flex-checksum
Dec 18, 2024
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
17 changes: 17 additions & 0 deletions config/load_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,10 @@ type LoadOptions struct {

// Specifies if response checksum should be validated
ResponseChecksumValidation aws.ResponseChecksumValidation

// Service endpoint override. This value is not necessarily final and is
// passed to the service's EndpointResolverV2 for further delegation.
BaseEndpoint string
}

func (o LoadOptions) getDefaultsMode(ctx context.Context) (aws.DefaultsMode, bool, error) {
Expand Down Expand Up @@ -299,6 +303,19 @@ func (o LoadOptions) getResponseChecksumValidation(ctx context.Context) (aws.Res
return o.ResponseChecksumValidation, o.ResponseChecksumValidation > 0, nil
}

func (o LoadOptions) getBaseEndpoint(context.Context) (string, bool, error) {
return o.BaseEndpoint, o.BaseEndpoint != "", nil
}

// GetServiceBaseEndpoint satisfies (internal/configsources).ServiceBaseEndpointProvider.
//
// The sdkID value is unused because LoadOptions only supports setting a GLOBAL
// endpoint override. In-code, per-service endpoint overrides are performed via
// functional options in service client space.
func (o LoadOptions) GetServiceBaseEndpoint(context.Context, string) (string, bool, error) {
wty-Bryant marked this conversation as resolved.
Show resolved Hide resolved
return o.BaseEndpoint, o.BaseEndpoint != "", nil
}

// WithRegion is a helper function to construct functional options
// that sets Region on config's LoadOptions. Setting the region to
// an empty string, will result in the region value being ignored.
Expand Down
30 changes: 7 additions & 23 deletions feature/s3/manager/integ_upload_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,9 @@ func TestInteg_UploadPresetChecksum(t *testing.T) {
}{
"auto single part": {
"no checksum": {
payload: bytes.NewReader(singlePartBytes),
expectETag: singlePartETag,
payload: bytes.NewReader(singlePartBytes),
expectChecksumCRC32: singlePartCRC32,
wty-Bryant marked this conversation as resolved.
Show resolved Hide resolved
expectETag: singlePartETag,
},
"CRC32": {
algorithm: s3types.ChecksumAlgorithmCrc32,
Expand Down Expand Up @@ -215,30 +216,13 @@ func TestInteg_UploadPresetChecksum(t *testing.T) {
expectETag: singlePartETag,
},
"MD5": {
payload: bytes.NewReader(singlePartBytes),
contentMD5: singlePartMD5,
expectETag: singlePartETag,
payload: bytes.NewReader(singlePartBytes),
contentMD5: singlePartMD5,
expectChecksumCRC32: singlePartCRC32,
expectETag: singlePartETag,
},
},
"auto multipart part": {
"no checksum": {
payload: bytes.NewReader(multiPartBytes),
expectParts: []s3types.CompletedPart{
{
ETag: aws.String(singlePartETag),
PartNumber: aws.Int32(1),
},
{
ETag: aws.String(singlePartETag),
PartNumber: aws.Int32(2),
},
{
ETag: aws.String(multiPartTailETag),
PartNumber: aws.Int32(3),
},
},
expectETag: multiPartETag,
},
"CRC32": {
algorithm: s3types.ChecksumAlgorithmCrc32,
payload: bytes.NewReader(multiPartBytes),
Expand Down
3 changes: 3 additions & 0 deletions service/internal/checksum/algorithms.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ const (

// AlgorithmSHA256 represents SHA256 hash algorithm
AlgorithmSHA256 Algorithm = "SHA256"

// AlgorithmCRC64NVME represents CRC64NVME hash algorithm
AlgorithmCRC64NVME Algorithm = "CRC64NVME"
wty-Bryant marked this conversation as resolved.
Show resolved Hide resolved
)

var supportedAlgorithms = []Algorithm{
Expand Down
25 changes: 13 additions & 12 deletions service/internal/checksum/middleware_compute_input_checksum.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"hash"
"io"
"strconv"
"strings"

v4 "github.com/aws/aws-sdk-go-v2/aws/signer/v4"
internalcontext "github.com/aws/aws-sdk-go-v2/internal/context"
Expand Down Expand Up @@ -129,13 +130,12 @@ func (m *computeInputPayloadChecksum) HandleFinalize(
})
}()

// If the checksum header is already set nothing to do.
for _, supportedAlgorithm := range supportedAlgorithms {
header := AlgorithmHTTPHeader(supportedAlgorithm)
if cs := req.Header.Get(header); cs != "" {
// need to add header value to metadata
algorithm = supportedAlgorithm
checksum = cs
// If any checksum header is already set nothing to do.
for header := range req.Header {
h := strings.ToUpper(header)
if strings.HasPrefix(h, "X-AMZ-CHECKSUM-") {
algorithm = Algorithm(strings.TrimPrefix(h, "X-AMZ-CHECKSUM-"))
checksum = req.Header.Get(header)
return next.HandleFinalize(ctx, in)
}
}
Expand Down Expand Up @@ -180,7 +180,9 @@ func (m *computeInputPayloadChecksum) HandleFinalize(
// Only seekable streams are supported for non-trailing checksums, because
// the stream needs to be rewound before the handler can continue.
if stream != nil && !req.IsStreamSeekable() {
return next.HandleFinalize(ctx, in)
return out, metadata, computeInputHeaderChecksumError{
Msg: "unseekable stream is not supported without TLS and trailing checksum",
}
}

var sha256Checksum string
Expand Down Expand Up @@ -274,10 +276,9 @@ func (m *addInputChecksumTrailer) HandleFinalize(
}
}

// If the checksum header is already set nothing to do.
for _, supportedAlgorithm := range supportedAlgorithms {
header := AlgorithmHTTPHeader(supportedAlgorithm)
if req.Header.Get(header) != "" {
// If any checksum header is already set nothing to do.
for header := range req.Header {
if strings.HasPrefix(strings.ToLower(header), "x-amz-checksum-") {
return next.HandleFinalize(ctx, in)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,52 @@ func TestComputeInputPayloadChecksum(t *testing.T) {
expectHeader: http.Header{},
expectPayload: []byte("hello world"),
},
"http crc64 checksum header preset": {
initContext: func(ctx context.Context) context.Context {
return internalcontext.SetChecksumInputAlgorithm(ctx, string(AlgorithmCRC32))
},
buildInput: middleware.BuildInput{
Request: func() *smithyhttp.Request {
r := smithyhttp.NewStackRequest().(*smithyhttp.Request)
r.URL, _ = url.Parse("http://example.aws")
r.ContentLength = 11
r.Header.Set(AlgorithmHTTPHeader(AlgorithmCRC64NVME), "S2Zv/ZHmbVs=")
r = requestMust(r.SetStream(bytes.NewReader([]byte("hello world"))))
return r
}(),
},
expectHeader: http.Header{
"X-Amz-Checksum-Crc64nvme": []string{"S2Zv/ZHmbVs="},
},
expectContentLength: 11,
expectPayload: []byte("hello world"),
expectChecksumMetadata: map[string]string{
"CRC64NVME": "S2Zv/ZHmbVs=",
},
},
"https crc64 checksum header preset": {
initContext: func(ctx context.Context) context.Context {
return internalcontext.SetChecksumInputAlgorithm(ctx, string(AlgorithmCRC32))
},
buildInput: middleware.BuildInput{
Request: func() *smithyhttp.Request {
r := smithyhttp.NewStackRequest().(*smithyhttp.Request)
r.URL, _ = url.Parse("https://example.aws")
r.ContentLength = 11
r.Header.Set(AlgorithmHTTPHeader(AlgorithmCRC64NVME), "S2Zv/ZHmbVs=")
r = requestMust(r.SetStream(bytes.NewReader([]byte("hello world"))))
return r
}(),
},
expectHeader: http.Header{
"X-Amz-Checksum-Crc64nvme": []string{"S2Zv/ZHmbVs="},
},
expectContentLength: 11,
expectPayload: []byte("hello world"),
expectChecksumMetadata: map[string]string{
"CRC64NVME": "S2Zv/ZHmbVs=",
},
},
},

"build handled": {
Expand Down Expand Up @@ -449,9 +495,8 @@ func TestComputeInputPayloadChecksum(t *testing.T) {
return r
}(),
},
expectContentLength: -1,
expectHeader: http.Header{},
expectPayload: []byte("hello world"),
expectErr: "unseekable stream is not supported",
expectBuildErr: true,
},
"http stream read error": {
initContext: func(ctx context.Context) context.Context {
Expand Down Expand Up @@ -504,9 +549,8 @@ func TestComputeInputPayloadChecksum(t *testing.T) {
return r
}(),
},
expectContentLength: -1,
expectHeader: http.Header{},
expectPayload: []byte("hello world"),
expectErr: "unseekable stream is not supported",
expectBuildErr: true,
},
},

Expand Down
42 changes: 35 additions & 7 deletions service/internal/integrationtest/s3/checksum_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,14 @@ func TestInteg_ObjectChecksums(t *testing.T) {
},
getObjectChecksumMode: s3types.ChecksumModeEnabled,
expectPayload: []byte("abc123"),
expectLogged: "Response has no supported checksum.",
expectComputedChecksums: &s3.ComputedInputChecksumsMetadata{
ComputedChecksums: map[string]string{
"CRC32": "zwK7XA==",
},
},
expectAlgorithmsUsed: &s3.ChecksumValidationMetadata{
AlgorithmsUsed: []string{"CRC32"},
},
},
"preset checksum": {
params: &s3.PutObjectInput{
Expand Down Expand Up @@ -170,7 +177,14 @@ func TestInteg_ObjectChecksums(t *testing.T) {
},
getObjectChecksumMode: s3types.ChecksumModeEnabled,
expectPayload: []byte("abc123"),
expectLogged: "Response has no supported checksum.",
expectComputedChecksums: &s3.ComputedInputChecksumsMetadata{
ComputedChecksums: map[string]string{
"CRC32": "zwK7XA==",
},
},
expectAlgorithmsUsed: &s3.ChecksumValidationMetadata{
AlgorithmsUsed: []string{"CRC32"},
},
},
"preset checksum": {
params: &s3.PutObjectInput{
Expand Down Expand Up @@ -251,7 +265,14 @@ func TestInteg_ObjectChecksums(t *testing.T) {
"no checksum": {
params: &s3.PutObjectInput{},
getObjectChecksumMode: s3types.ChecksumModeEnabled,
expectLogged: "Response has no supported checksum.",
expectComputedChecksums: &s3.ComputedInputChecksumsMetadata{
ComputedChecksums: map[string]string{
"CRC32": "AAAAAA==",
},
},
expectAlgorithmsUsed: &s3.ChecksumValidationMetadata{
AlgorithmsUsed: []string{"CRC32"},
},
},
"preset checksum": {
params: &s3.PutObjectInput{
Expand Down Expand Up @@ -304,7 +325,14 @@ func TestInteg_ObjectChecksums(t *testing.T) {
Body: bytes.NewBuffer([]byte{}),
},
getObjectChecksumMode: s3types.ChecksumModeEnabled,
expectLogged: "Response has no supported checksum.",
expectComputedChecksums: &s3.ComputedInputChecksumsMetadata{
ComputedChecksums: map[string]string{
"CRC32": "AAAAAA==",
},
},
expectAlgorithmsUsed: &s3.ChecksumValidationMetadata{
AlgorithmsUsed: []string{"CRC32"},
},
},
"preset checksum": {
params: &s3.PutObjectInput{
Expand Down Expand Up @@ -394,7 +422,7 @@ func TestInteg_ObjectChecksums(t *testing.T) {
}
// assert computed input checksums metadata
computedChecksums, ok := s3.GetComputedInputChecksumsMetadata(putResult.ResultMetadata)
if e, a := ok, (c.expectComputedChecksums != nil); e != a {
wty-Bryant marked this conversation as resolved.
Show resolved Hide resolved
if e, a := (c.expectComputedChecksums != nil), ok; e != a {
t.Fatalf("expect computed checksum metadata %t, got %t, %v", e, a, computedChecksums)
}
if c.expectComputedChecksums != nil {
Expand Down Expand Up @@ -442,7 +470,7 @@ func TestInteg_ObjectChecksums(t *testing.T) {

// assert checksum validation metadata
algorithmsUsed, ok := s3.GetChecksumValidationMetadata(getResult.ResultMetadata)
if e, a := ok, (c.expectAlgorithmsUsed != nil); e != a {
if e, a := (c.expectAlgorithmsUsed != nil), ok; e != a {
t.Fatalf("expect algorithms used metadata %t, got %t, %v", e, a, algorithmsUsed)
}
if c.expectAlgorithmsUsed != nil {
Expand All @@ -468,7 +496,7 @@ func TestInteg_RequireChecksum(t *testing.T) {
expectComputedChecksums []string
}{
"no algorithm": {
expectComputedChecksums: []string{"MD5"},
expectComputedChecksums: []string{"CRC32"},
},
"with algorithm": {
checksumAlgorithm: types.ChecksumAlgorithmCrc32c,
Expand Down
Loading