From f697eeafd2bb9f5df0ce7b193d97f023bea3f976 Mon Sep 17 00:00:00 2001 From: agbishop Date: Sat, 15 Aug 2026 21:48:02 -0500 Subject: [PATCH 1/3] feat(parity): add LocalStack health parity, buffer pooling, and DDB/S3 optimizations --- cli.go | 60 ++ cli_test.go | 103 ++++ pkgs/awsmeta/awsmeta.go | 24 +- pkgs/awsmeta/awsmeta_test.go | 68 ++- pkgs/httputils/httputils.go | 39 ++ pkgs/httputils/pool.go | 144 +++++ pkgs/httputils/pool_test.go | 157 ++++++ services/dynamodb/capacity.go | 107 ++++ services/dynamodb/capacity_test.go | 640 ++++++++++++---------- services/dynamodb/handler.go | 151 +++-- services/dynamodb/item_ops_crud.go | 54 +- services/dynamodb/item_ops_query.go | 33 +- services/dynamodb/item_ops_scan.go | 49 +- services/dynamodb/scan_test.go | 96 ++++ services/s3/PARITY.md | 3 +- services/s3/README.md | 1 - services/s3/bucket_analytics_test.go | 3 +- services/s3/bucket_encryption_test.go | 3 +- services/s3/bucket_lifecycle_test.go | 3 +- services/s3/bucket_tagging_test.go | 3 +- services/s3/bucket_versioning_test.go | 3 +- services/s3/checksum_test.go | 3 +- services/s3/compression_test.go | 1 + services/s3/handler.go | 1 + services/s3/handler_routing_test.go | 5 +- services/s3/multipart.go | 41 +- services/s3/notification_dispatch_test.go | 3 +- services/s3/object_ops_copy_test.go | 3 +- services/s3/object_ops_delete_test.go | 3 +- services/s3/object_ops_head.go | 106 +++- services/s3/object_ops_head_test.go | 107 +++- services/s3/objects.go | 23 + services/s3/post_object_test.go | 3 +- services/s3/store_listing_test.go | 3 +- services/s3/types.go | 108 ++-- services/s3/website_test.go | 5 +- 36 files changed, 1621 insertions(+), 538 deletions(-) create mode 100644 pkgs/httputils/pool.go create mode 100644 pkgs/httputils/pool_test.go diff --git a/cli.go b/cli.go index 28583625c7..722f554314 100644 --- a/cli.go +++ b/cli.go @@ -2130,6 +2130,11 @@ func buildEchoServer( return c.Redirect(http.StatusFound, "/dashboard/static/favicon.png") }) e.GET("/_gopherstack/health", buildHealthHandler(services)) + e.GET("/_localstack/health", buildLocalstackHealthHandler(services)) + e.GET("/_aws/health", buildLocalstackHealthHandler(services)) + e.GET("/_localstack/init", buildLocalstackInitHandler()) + e.GET("/_localstack/init/ready", buildLocalstackInitHandler()) + e.GET("/_localstack/info", buildLocalstackInfoHandler()) e.POST("/_gopherstack/reset", buildResetHandler(services)) e.POST("/_gopherstack/snapshot", buildSnapshotHandler(persistManager)) e.POST("/_gopherstack/load", buildLoadHandler(persistManager)) @@ -10434,6 +10439,61 @@ type healthResponse struct { NumGC uint32 `json:"num_gc"` } +// localstackHealthResponse is the JSON body returned by LocalStack-compatible health endpoints. +type localstackHealthResponse struct { + Services map[string]string `json:"services"` + Version string `json:"version"` + Edition string `json:"edition"` +} + +// localstackInitResponse is the JSON body returned by LocalStack-compatible init endpoints. +type localstackInitResponse struct { + Scripts []string `json:"scripts"` + Completed bool `json:"completed"` +} + +func buildLocalstackHealthHandler(services []service.Registerable) echo.HandlerFunc { + return func(c *echo.Context) error { + svcMap := make(map[string]string, len(services)) + for _, svc := range services { + svcMap[strings.ToLower(svc.Name())] = "available" + } + + return c.JSON(http.StatusOK, localstackHealthResponse{ + Services: svcMap, + Version: version.Get(), + Edition: "community", + }) + } +} + +func buildLocalstackInitHandler() echo.HandlerFunc { + return func(c *echo.Context) error { + return c.JSON(http.StatusOK, localstackInitResponse{ + Completed: true, + Scripts: []string{}, + }) + } +} + +type localstackInfoResponse struct { + Version string `json:"version"` + Edition string `json:"edition"` + SessionID string `json:"session_id"` + IsAuth bool `json:"is_auth"` +} + +func buildLocalstackInfoHandler() echo.HandlerFunc { + return func(c *echo.Context) error { + return c.JSON(http.StatusOK, localstackInfoResponse{ + Version: version.Get(), + Edition: "community", + IsAuth: false, + SessionID: "00000000-0000-0000-0000-000000000000", + }) + } +} + func setupChaosAndRegistry( e *echo.Echo, log *slog.Logger, diff --git a/cli_test.go b/cli_test.go index f7a63a1928..f545c4b3c9 100644 --- a/cli_test.go +++ b/cli_test.go @@ -3061,6 +3061,109 @@ func TestHealthEndpoint_GoroutineAndMemStats(t *testing.T) { } } +//nolint:paralleltest // uses a fixed port that cannot be parallelised +func TestLocalstackCompatibilityEndpoints(t *testing.T) { + port := freeTCPPort(t) + cli := parseCLI(t, map[string]string{"PORT": strconv.Itoa(port)}) + + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + + errCh := make(chan error, 1) + go func() { + errCh <- run(ctx, cli) + }() + + require.Eventually(t, func() bool { + resp, err := http.Get(fmt.Sprintf("http://localhost:%d/_gopherstack/health", port)) + if err != nil { + return false + } + resp.Body.Close() + + return resp.StatusCode == http.StatusOK + }, 3*time.Second, 50*time.Millisecond, "server did not become ready") + + tests := []struct { + validate func(t *testing.T, body map[string]any) + name string + path string + }{ + { + name: "localstack_health", + path: "/_localstack/health", + validate: func(t *testing.T, body map[string]any) { + t.Helper() + assert.Equal(t, "community", body["edition"]) + assert.NotEmpty(t, body["version"]) + services, ok := body["services"].(map[string]any) + require.True(t, ok) + assert.Equal(t, "available", services["s3"]) + assert.Equal(t, "available", services["dynamodb"]) + }, + }, + { + name: "aws_health", + path: "/_aws/health", + validate: func(t *testing.T, body map[string]any) { + t.Helper() + assert.Equal(t, "community", body["edition"]) + services, ok := body["services"].(map[string]any) + require.True(t, ok) + assert.Equal(t, "available", services["s3"]) + }, + }, + { + name: "localstack_init", + path: "/_localstack/init", + validate: func(t *testing.T, body map[string]any) { + t.Helper() + assert.Equal(t, true, body["completed"]) + assert.NotNil(t, body["scripts"]) + }, + }, + { + name: "localstack_init_ready", + path: "/_localstack/init/ready", + validate: func(t *testing.T, body map[string]any) { + t.Helper() + assert.Equal(t, true, body["completed"]) + }, + }, + { + name: "localstack_info", + path: "/_localstack/info", + validate: func(t *testing.T, body map[string]any) { + t.Helper() + assert.Equal(t, "community", body["edition"]) + assert.Equal(t, false, body["is_auth"]) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + resp, err := http.Get(fmt.Sprintf("http://localhost:%d%s", port, tt.path)) + require.NoError(t, err) + defer resp.Body.Close() + + assert.Equal(t, http.StatusOK, resp.StatusCode) + var body map[string]any + require.NoError(t, json.NewDecoder(resp.Body).Decode(&body)) + tt.validate(t, body) + }) + } + + cancel() + + select { + case shutdownErr := <-errCh: + require.NoError(t, shutdownErr) + case <-time.After(5 * time.Second): + require.FailNow(t, "server did not shut down within timeout") + } +} + // TestCustomHTTPErrorHandler_LogsServerErrors verifies that the custom Echo error // handler returns the correct status code for server errors (5xx). func TestCustomHTTPErrorHandler_LogsServerErrors(t *testing.T) { diff --git a/pkgs/awsmeta/awsmeta.go b/pkgs/awsmeta/awsmeta.go index c2c8788104..8d2ac66cd7 100644 --- a/pkgs/awsmeta/awsmeta.go +++ b/pkgs/awsmeta/awsmeta.go @@ -25,6 +25,10 @@ type Metadata struct { Partition string // RequestID is the X-Amz-Request-Id correlation value. RequestID string + // AccessKeyID is the AWS access key ID extracted from the SigV4 credential scope. + AccessKeyID string + // Service is the AWS service name extracted from the SigV4 credential scope. + Service string } // DefaultAccount is the gopherstack default 12-digit account ID. @@ -73,6 +77,16 @@ func Partition(ctx context.Context) string { return Get(ctx).Partition } +// AccessKeyID returns Get(ctx).AccessKeyID. +func AccessKeyID(ctx context.Context) string { + return Get(ctx).AccessKeyID +} + +// Service returns Get(ctx).Service. +func Service(ctx context.Context) string { + return Get(ctx).Service +} + // FromRequest builds a Metadata from r. defaultRegion is applied when no // region is derivable from the SigV4 scope. Always returns non-nil with // Account and Partition populated. @@ -85,10 +99,12 @@ func FromRequest(r *http.Request, defaultRegion string) *Metadata { } m := &Metadata{ - Account: DefaultAccount, - Region: httputils.ExtractRegionFromRequest(r, defaultRegion), - Partition: DefaultPartition, - RequestID: httputils.SanitizeHeaderString(r.Header.Get("X-Amz-Request-Id")), + Account: DefaultAccount, + Region: httputils.ExtractRegionFromRequest(r, defaultRegion), + Partition: DefaultPartition, + RequestID: httputils.SanitizeHeaderString(r.Header.Get("X-Amz-Request-Id")), + AccessKeyID: httputils.ExtractAccessKeyFromRequest(r), + Service: httputils.ExtractServiceFromRequest(r), } if v := r.Header.Get("X-Amz-Account-Id"); v != "" { diff --git a/pkgs/awsmeta/awsmeta_test.go b/pkgs/awsmeta/awsmeta_test.go index 17ef2ff9e8..278173802b 100644 --- a/pkgs/awsmeta/awsmeta_test.go +++ b/pkgs/awsmeta/awsmeta_test.go @@ -75,36 +75,46 @@ func TestConvenienceAccessors(t *testing.T) { t.Parallel() tests := []struct { - name string - ctx context.Context //nolint:containedctx // table-driven test input - region string - account string - partition string + name string + ctx context.Context //nolint:containedctx // table-driven test input + region string + account string + partition string + accessKeyID string + service string }{ { - name: "nil-context-returns-defaults", - ctx: nil, - region: "", - account: awsmeta.DefaultAccount, - partition: awsmeta.DefaultPartition, + name: "nil-context-returns-defaults", + ctx: nil, + region: "", + account: awsmeta.DefaultAccount, + partition: awsmeta.DefaultPartition, + accessKeyID: "", + service: "", }, { - name: "empty-context-returns-defaults", - ctx: context.Background(), - region: "", - account: awsmeta.DefaultAccount, - partition: awsmeta.DefaultPartition, + name: "empty-context-returns-defaults", + ctx: context.Background(), + region: "", + account: awsmeta.DefaultAccount, + partition: awsmeta.DefaultPartition, + accessKeyID: "", + service: "", }, { name: "populated-context", ctx: awsmeta.Set(context.Background(), &awsmeta.Metadata{ - Account: "111111111111", - Region: "eu-west-1", - Partition: "aws", + Account: "111111111111", + Region: "eu-west-1", + Partition: "aws", + AccessKeyID: "AKIAIOSFODNN7EXAMPLE", + Service: "dynamodb", }), - region: "eu-west-1", - account: "111111111111", - partition: "aws", + region: "eu-west-1", + account: "111111111111", + partition: "aws", + accessKeyID: "AKIAIOSFODNN7EXAMPLE", + service: "dynamodb", }, } @@ -114,6 +124,8 @@ func TestConvenienceAccessors(t *testing.T) { assert.Equal(t, tc.region, awsmeta.Region(tc.ctx)) assert.Equal(t, tc.account, awsmeta.Account(tc.ctx)) assert.Equal(t, tc.partition, awsmeta.Partition(tc.ctx)) + assert.Equal(t, tc.accessKeyID, awsmeta.AccessKeyID(tc.ctx)) + assert.Equal(t, tc.service, awsmeta.Service(tc.ctx)) }) } } @@ -128,6 +140,8 @@ func TestFromRequest(t *testing.T) { wantRegion string wantAccount string wantRequestID string + wantAKID string + wantService string }{ { name: "nil-request-uses-default-region", @@ -150,11 +164,17 @@ func TestFromRequest(t *testing.T) { defaultRegion: "us-east-1", wantRegion: "eu-central-1", wantAccount: awsmeta.DefaultAccount, + wantAKID: "AKIA", + wantService: "s3", }, { name: "request-id-and-account-headers", buildRequest: func() *http.Request { - r := httptest.NewRequest(http.MethodPost, "/", nil) + r := httptest.NewRequest( + http.MethodPost, + "/?X-Amz-Credential=MYAKID/20260606/us-east-2/dynamodb/aws4_request", + nil, + ) r.Header.Set("X-Amz-Account-Id", "222222222222") r.Header.Set("X-Amz-Request-Id", "abc-123") @@ -164,6 +184,8 @@ func TestFromRequest(t *testing.T) { wantRegion: "us-east-2", wantAccount: "222222222222", wantRequestID: "abc-123", + wantAKID: "MYAKID", + wantService: "dynamodb", }, } @@ -175,6 +197,8 @@ func TestFromRequest(t *testing.T) { assert.Equal(t, tc.wantRegion, m.Region) assert.Equal(t, tc.wantAccount, m.Account) assert.Equal(t, tc.wantRequestID, m.RequestID) + assert.Equal(t, tc.wantAKID, m.AccessKeyID) + assert.Equal(t, tc.wantService, m.Service) assert.Equal(t, awsmeta.DefaultPartition, m.Partition) }) } diff --git a/pkgs/httputils/httputils.go b/pkgs/httputils/httputils.go index df45fecda1..c36817ef64 100644 --- a/pkgs/httputils/httputils.go +++ b/pkgs/httputils/httputils.go @@ -327,6 +327,10 @@ func ExtractRegionFromRequest(r *http.Request, defaultRegion string) string { // header credential scope (AKID/date/region/service/aws4_request). // Returns an empty string if the service name cannot be determined. func ExtractServiceFromRequest(r *http.Request) string { + if r == nil { + return "" + } + if auth := r.Header.Get("Authorization"); auth != "" && strings.Contains(auth, "Credential=") { parts := strings.Split(auth, "Credential=") if len(parts) > 1 { @@ -337,6 +341,41 @@ func ExtractServiceFromRequest(r *http.Request) string { } } + if cred := r.URL.Query().Get("X-Amz-Credential"); cred != "" { + credParts := strings.Split(cred, "/") + if len(credParts) > sigV4ServiceIndex { + return SanitizeHeaderString(credParts[sigV4ServiceIndex]) + } + } + + return "" +} + +// ExtractAccessKeyFromRequest extracts the AWS access key ID from an HTTP request. +// It checks the SigV4 Authorization header credential scope (AKID/date/region/service/aws4_request) +// first, then the X-Amz-Credential query parameter, and returns an empty string if none is found. +func ExtractAccessKeyFromRequest(r *http.Request) string { + if r == nil { + return "" + } + + if auth := r.Header.Get("Authorization"); auth != "" && strings.Contains(auth, "Credential=") { + parts := strings.Split(auth, "Credential=") + if len(parts) > 1 { + credParts := strings.Split(parts[1], "/") + if len(credParts) > 0 { + return SanitizeHeaderString(credParts[0]) + } + } + } + + if cred := r.URL.Query().Get("X-Amz-Credential"); cred != "" { + credParts := strings.Split(cred, "/") + if len(credParts) > 0 { + return SanitizeHeaderString(credParts[0]) + } + } + return "" } diff --git a/pkgs/httputils/pool.go b/pkgs/httputils/pool.go new file mode 100644 index 0000000000..d830dfd7f6 --- /dev/null +++ b/pkgs/httputils/pool.go @@ -0,0 +1,144 @@ +package httputils + +import ( + "bytes" + "crypto/md5" //nolint:gosec // MD5 is required for AWS S3 Content-MD5 and ETag compatibility. + "crypto/sha256" + "hash" + "hash/crc32" + "sync" +) + +const maxPooledBufferSize = 64 * 1024 // 64 KiB + +var bufferPool = sync.Pool{ //nolint:gochecknoglobals // sync.Pool requires package-level allocation + New: func() any { + return new(bytes.Buffer) + }, +} + +// GetBuffer acquires a clean *bytes.Buffer from the pool. +func GetBuffer() *bytes.Buffer { + buf, ok := bufferPool.Get().(*bytes.Buffer) + if !ok { + return new(bytes.Buffer) + } + buf.Reset() + + return buf +} + +// PutBuffer returns a *bytes.Buffer to the pool. +// Buffers larger than maxPooledBufferSize are discarded to bound memory retention. +func PutBuffer(buf *bytes.Buffer) { + if buf == nil { + return + } + if buf.Cap() > maxPooledBufferSize { + return + } + buf.Reset() + bufferPool.Put(buf) +} + +var crc32Pool = sync.Pool{ //nolint:gochecknoglobals // sync.Pool requires package-level allocation + New: func() any { + return crc32.NewIEEE() + }, +} + +// GetCRC32 retrieves a pooled CRC32 IEEE hasher with state reset. +func GetCRC32() hash.Hash32 { + h, ok := crc32Pool.Get().(hash.Hash32) + if !ok { + return crc32.NewIEEE() + } + h.Reset() + + return h +} + +// PutCRC32 returns a CRC32 IEEE hasher to the pool. +func PutCRC32(h hash.Hash32) { + if h != nil { + h.Reset() + crc32Pool.Put(h) + } +} + +var crc32cTable = crc32.MakeTable(crc32.Castagnoli) //nolint:gochecknoglobals // read-only lookup table + +var crc32cPool = sync.Pool{ //nolint:gochecknoglobals // sync.Pool requires package-level allocation + New: func() any { + return crc32.New(crc32cTable) + }, +} + +// GetCRC32C retrieves a pooled CRC32C (Castagnoli) hasher with state reset. +func GetCRC32C() hash.Hash32 { + h, ok := crc32cPool.Get().(hash.Hash32) + if !ok { + return crc32.New(crc32cTable) + } + h.Reset() + + return h +} + +// PutCRC32C returns a CRC32C hasher to the pool. +func PutCRC32C(h hash.Hash32) { + if h != nil { + h.Reset() + crc32cPool.Put(h) + } +} + +var sha256Pool = sync.Pool{ //nolint:gochecknoglobals // sync.Pool requires package-level allocation + New: func() any { + return sha256.New() + }, +} + +// GetSHA256 retrieves a pooled SHA256 hasher with state reset. +func GetSHA256() hash.Hash { + h, ok := sha256Pool.Get().(hash.Hash) + if !ok { + return sha256.New() + } + h.Reset() + + return h +} + +// PutSHA256 returns a SHA256 hasher to the pool. +func PutSHA256(h hash.Hash) { + if h != nil { + h.Reset() + sha256Pool.Put(h) + } +} + +var md5Pool = sync.Pool{ //nolint:gochecknoglobals // sync.Pool requires package-level allocation + New: func() any { + return md5.New() //nolint:gosec // MD5 is required for S3 Content-MD5 and ETag compatibility. + }, +} + +// GetMD5 retrieves a pooled MD5 hasher with state reset. +func GetMD5() hash.Hash { + h, ok := md5Pool.Get().(hash.Hash) + if !ok { + return md5.New() //nolint:gosec // MD5 is required for S3 Content-MD5 and ETag compatibility. + } + h.Reset() + + return h +} + +// PutMD5 returns an MD5 hasher to the pool. +func PutMD5(h hash.Hash) { + if h != nil { + h.Reset() + md5Pool.Put(h) + } +} diff --git a/pkgs/httputils/pool_test.go b/pkgs/httputils/pool_test.go new file mode 100644 index 0000000000..a781ab7785 --- /dev/null +++ b/pkgs/httputils/pool_test.go @@ -0,0 +1,157 @@ +package httputils_test + +import ( + "bytes" + "crypto/md5" + "crypto/sha256" + "hash/crc32" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/pkgs/httputils" +) + +func TestBufferPool(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + inputData []byte + }{ + { + name: "empty_buffer", + inputData: []byte{}, + }, + { + name: "small_data", + inputData: []byte("hello world"), + }, + { + name: "large_data", + inputData: bytes.Repeat([]byte("a"), 1024), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + buf := httputils.GetBuffer() + require.NotNil(t, buf) + assert.Equal(t, 0, buf.Len()) + + n, err := buf.Write(tt.inputData) + require.NoError(t, err) + assert.Equal(t, len(tt.inputData), n) + if len(tt.inputData) == 0 { + assert.Empty(t, buf.Bytes()) + } else { + assert.Equal(t, tt.inputData, buf.Bytes()) + } + + httputils.PutBuffer(buf) + }) + } +} + +func TestHasherPools(t *testing.T) { + t.Parallel() + + payload := []byte("gopherstack fast hashing test payload") + + tests := []struct { + getHasher func() any + compute func([]byte) []byte + name string + }{ + { + name: "crc32", + getHasher: func() any { + return httputils.GetCRC32() + }, + compute: func(b []byte) []byte { + h := crc32.NewIEEE() + h.Write(b) + + return h.Sum(nil) + }, + }, + { + name: "crc32c", + getHasher: func() any { + return httputils.GetCRC32C() + }, + compute: func(b []byte) []byte { + h := crc32.New(crc32.MakeTable(crc32.Castagnoli)) + h.Write(b) + + return h.Sum(nil) + }, + }, + { + name: "sha256", + getHasher: func() any { + return httputils.GetSHA256() + }, + compute: func(b []byte) []byte { + h := sha256.New() + h.Write(b) + + return h.Sum(nil) + }, + }, + { + name: "md5", + getHasher: func() any { + return httputils.GetMD5() + }, + compute: func(b []byte) []byte { + h := md5.New() + h.Write(b) + + return h.Sum(nil) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + expected := tt.compute(payload) + + h1 := httputils.GetCRC32() + h1.Write(payload) + sum1 := h1.Sum(nil) + httputils.PutCRC32(h1) + + h2 := httputils.GetCRC32C() + h2.Write(payload) + sum2 := h2.Sum(nil) + httputils.PutCRC32C(h2) + + h3 := httputils.GetSHA256() + h3.Write(payload) + sum3 := h3.Sum(nil) + httputils.PutSHA256(h3) + + h4 := httputils.GetMD5() + h4.Write(payload) + sum4 := h4.Sum(nil) + httputils.PutMD5(h4) + + switch tt.name { + case "crc32": + assert.Equal(t, expected, sum1) + case "crc32c": + assert.Equal(t, expected, sum2) + case "sha256": + assert.Equal(t, expected, sum3) + case "md5": + assert.Equal(t, expected, sum4) + } + }) + } +} diff --git a/services/dynamodb/capacity.go b/services/dynamodb/capacity.go index afaa3d89b3..b7718235d0 100644 --- a/services/dynamodb/capacity.go +++ b/services/dynamodb/capacity.go @@ -163,3 +163,110 @@ func applyConsistentReadMultiplier(rcu float64, consistentRead bool) float64 { return rcu } + +// consumedCapacityForReadOp returns a populated ConsumedCapacity for Query or Scan operations. +func consumedCapacityForReadOp( + tableName string, + req types.ReturnConsumedCapacity, + count int, + consistentRead bool, + indexName string, + table *Table, +) *types.ConsumedCapacity { + if req == "" || req == types.ReturnConsumedCapacityNone { + return nil + } + const halfRCU = 0.5 + cu := float64(count) * halfRCU + if cu < halfRCU { + cu = halfRCU + } + cu = applyConsistentReadMultiplier(cu, consistentRead) + + var ( + tableRCU float64 + gsiRCU map[string]float64 + lsiRCU map[string]float64 + ) + + if indexName != "" && table != nil { + if isIndexGSI(table, indexName) { + gsiRCU = map[string]float64{indexName: cu} + } else { + lsiRCU = map[string]float64{indexName: cu} + } + } else { + tableRCU = cu + } + + return buildConsumedCapacityWithIndexes( + tableName, + req, + tableRCU, 0, + gsiRCU, nil, + lsiRCU, nil, + ) +} + +func isIndexGSI(table *Table, indexName string) bool { + for i := range table.GlobalSecondaryIndexes { + if table.GlobalSecondaryIndexes[i].IndexName == indexName { + return true + } + } + + return false +} + +// calculateWriteIndexBreakdowns determines WCU consumed on GSIs and LSIs populated by item. +func calculateWriteIndexBreakdowns( + table *Table, + item map[string]any, + writeUnits float64, +) (map[string]float64, map[string]float64) { + if item == nil || writeUnits <= 0 || table == nil { + return nil, nil + } + + return calculateGSIWriteBreakdowns(table, item, writeUnits), calculateLSIWriteBreakdowns(table, item, writeUnits) +} + +func calculateGSIWriteBreakdowns(table *Table, item map[string]any, writeUnits float64) map[string]float64 { + if len(table.GlobalSecondaryIndexes) == 0 { + return nil + } + + var gsiWCU map[string]float64 + for i := range table.GlobalSecondaryIndexes { + gsi := &table.GlobalSecondaryIndexes[i] + pkDef, skDef := getPKAndSK(gsi.KeySchema) + if _, _, ok := secondaryItemKeyValues(item, pkDef, skDef); ok { + if gsiWCU == nil { + gsiWCU = make(map[string]float64) + } + gsiWCU[gsi.IndexName] = writeUnits + } + } + + return gsiWCU +} + +func calculateLSIWriteBreakdowns(table *Table, item map[string]any, writeUnits float64) map[string]float64 { + if len(table.LocalSecondaryIndexes) == 0 { + return nil + } + + var lsiWCU map[string]float64 + for i := range table.LocalSecondaryIndexes { + lsi := &table.LocalSecondaryIndexes[i] + pkDef, skDef := getPKAndSK(lsi.KeySchema) + if _, _, ok := secondaryItemKeyValues(item, pkDef, skDef); ok { + if lsiWCU == nil { + lsiWCU = make(map[string]float64) + } + lsiWCU[lsi.IndexName] = writeUnits + } + } + + return lsiWCU +} diff --git a/services/dynamodb/capacity_test.go b/services/dynamodb/capacity_test.go index 185f626105..108199b459 100644 --- a/services/dynamodb/capacity_test.go +++ b/services/dynamodb/capacity_test.go @@ -1,180 +1,99 @@ package dynamodb_test import ( - "context" - "fmt" "strings" "testing" "github.com/aws/aws-sdk-go-v2/aws" dynamodb_sdk "github.com/aws/aws-sdk-go-v2/service/dynamodb" "github.com/aws/aws-sdk-go-v2/service/dynamodb/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/blackbirdworks/gopherstack/services/dynamodb" ) -func TestConsumedCapacityIndexes_PutItem(t *testing.T) { - t.Parallel() - db := newInMemoryTestDB(t) - ctx := context.Background() - - _, err := db.CreateTable(ctx, &dynamodb_sdk.CreateTableInput{ - TableName: aws.String("TestCC"), - KeySchema: []types.KeySchemaElement{ - {AttributeName: aws.String("pk"), KeyType: types.KeyTypeHash}, - }, - AttributeDefinitions: []types.AttributeDefinition{ - {AttributeName: aws.String("pk"), AttributeType: types.ScalarAttributeTypeS}, - {AttributeName: aws.String("gsi_pk"), AttributeType: types.ScalarAttributeTypeS}, - }, - GlobalSecondaryIndexes: []types.GlobalSecondaryIndex{ - { - IndexName: aws.String("gsi1"), - KeySchema: []types.KeySchemaElement{ - {AttributeName: aws.String("gsi_pk"), KeyType: types.KeyTypeHash}, - }, - Projection: &types.Projection{ProjectionType: types.ProjectionTypeAll}, - ProvisionedThroughput: &types.ProvisionedThroughput{ - ReadCapacityUnits: aws.Int64(5), - WriteCapacityUnits: aws.Int64(5), - }, - }, - }, - BillingMode: types.BillingModeProvisioned, - ProvisionedThroughput: &types.ProvisionedThroughput{ - ReadCapacityUnits: aws.Int64(10), - WriteCapacityUnits: aws.Int64(10), - }, - }) - if err != nil { - t.Fatalf("CreateTable: %v", err) +func assertConsumedCapacityBreakdown( + t *testing.T, + cc *types.ConsumedCapacity, + tableName string, + wantMinTotal float64, + wantTable, wantGSI, wantLSI bool, +) { + t.Helper() + require.NotNil(t, cc) + assert.Equal(t, tableName, aws.ToString(cc.TableName)) + assert.GreaterOrEqual(t, aws.ToFloat64(cc.CapacityUnits), wantMinTotal) + + if wantTable { + assert.NotNil(t, cc.Table) + } else { + assert.Nil(t, cc.Table) } - out, err := db.PutItem(ctx, &dynamodb_sdk.PutItemInput{ - TableName: aws.String("TestCC"), - Item: map[string]types.AttributeValue{ - "pk": &types.AttributeValueMemberS{Value: "k1"}, - "gsi_pk": &types.AttributeValueMemberS{Value: "g1"}, - }, - ReturnConsumedCapacity: types.ReturnConsumedCapacityTotal, - }) - if err != nil { - t.Fatalf("PutItem: %v", err) + if wantGSI { + require.NotNil(t, cc.GlobalSecondaryIndexes) + _, hasGSI := cc.GlobalSecondaryIndexes["gsi1"] + assert.True(t, hasGSI) + } else { + assert.Nil(t, cc.GlobalSecondaryIndexes) } - if out.ConsumedCapacity == nil { - t.Fatal("expected ConsumedCapacity, got nil") - } - - if aws.ToString(out.ConsumedCapacity.TableName) != "TestCC" { - t.Errorf("unexpected table name: %s", aws.ToString(out.ConsumedCapacity.TableName)) - } - - if out.ConsumedCapacity.CapacityUnits == nil || *out.ConsumedCapacity.CapacityUnits <= 0 { - t.Error("expected positive CapacityUnits") + if wantLSI { + require.NotNil(t, cc.LocalSecondaryIndexes) + _, hasLSI := cc.LocalSecondaryIndexes["lsi1"] + assert.True(t, hasLSI) + } else { + assert.Nil(t, cc.LocalSecondaryIndexes) } } -func TestConsumedCapacityIndexes_None(t *testing.T) { +func TestConsumedCapacity_Indexes_TableDriven(t *testing.T) { t.Parallel() - db := newInMemoryTestDB(t) - ctx := context.Background() - createSimpleTestTable(t, db, "CCNone") - out, err := db.PutItem(ctx, &dynamodb_sdk.PutItemInput{ - TableName: aws.String("CCNone"), - Item: map[string]types.AttributeValue{ - "pk": &types.AttributeValueMemberS{Value: "k1"}, - "sk": &types.AttributeValueMemberS{Value: "s1"}, + tests := []struct { + name string + reqCC types.ReturnConsumedCapacity + wantMinTotal float64 + wantTable bool + wantGSI bool + wantLSI bool + wantNil bool + }{ + { + name: "indexes_requested", + reqCC: types.ReturnConsumedCapacityIndexes, + wantMinTotal: 1.0, + wantTable: true, + wantGSI: true, + wantLSI: true, + wantNil: false, }, - ReturnConsumedCapacity: types.ReturnConsumedCapacityNone, - }) - if err != nil { - t.Fatalf("PutItem: %v", err) - } - - if out.ConsumedCapacity != nil { - t.Error("expected nil ConsumedCapacity for NONE") - } -} - -func TestConsistentRead_GetItem_DoesNotError(t *testing.T) { - t.Parallel() - db := newInMemoryTestDB(t) - ctx := context.Background() - createSimpleTestTable(t, db, "CRTable") - - putTestItem(t, db, "CRTable", map[string]types.AttributeValue{ - "pk": &types.AttributeValueMemberS{Value: "pk1"}, - "sk": &types.AttributeValueMemberS{Value: "sk1"}, - }) - - out, err := db.GetItem(ctx, &dynamodb_sdk.GetItemInput{ - TableName: aws.String("CRTable"), - ConsistentRead: aws.Bool(true), - Key: map[string]types.AttributeValue{ - "pk": &types.AttributeValueMemberS{Value: "pk1"}, - "sk": &types.AttributeValueMemberS{Value: "sk1"}, + { + name: "total_requested", + reqCC: types.ReturnConsumedCapacityTotal, + wantMinTotal: 1.0, + wantTable: false, + wantGSI: false, + wantLSI: false, + wantNil: false, }, - }) - if err != nil { - t.Fatalf("GetItem with ConsistentRead: %v", err) - } - - if out.Item == nil { - t.Error("expected item, got nil") - } -} - -func TestConsistentRead_Query_DoesNotError(t *testing.T) { - t.Parallel() - db := newInMemoryTestDB(t) - ctx := context.Background() - createSimpleTestTable(t, db, "CRQuery") - - putTestItem(t, db, "CRQuery", map[string]types.AttributeValue{ - "pk": &types.AttributeValueMemberS{Value: "p1"}, - "sk": &types.AttributeValueMemberS{Value: "s1"}, - }) - - _, err := db.Query(ctx, &dynamodb_sdk.QueryInput{ - TableName: aws.String("CRQuery"), - ConsistentRead: aws.Bool(true), - KeyConditionExpression: aws.String("pk = :pk"), - ExpressionAttributeValues: map[string]types.AttributeValue{ - ":pk": &types.AttributeValueMemberS{Value: "p1"}, + { + name: "none_requested", + reqCC: types.ReturnConsumedCapacityNone, + wantNil: true, }, - }) - if err != nil { - t.Fatalf("Query with ConsistentRead: %v", err) - } -} - -// TestConsistentRead_Query_OnGSI_Rejected verifies that a strongly-consistent Query -// against a global secondary index is rejected with a ValidationException (AWS does not -// support consistent reads on a GSI), while the same query on the primary index and on a -// local secondary index is allowed. -func TestConsistentRead_Query_OnGSI_Rejected(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - indexName string - wantErr bool - }{ - {name: "primary_index_allowed", indexName: "", wantErr: false}, - {name: "lsi_allowed", indexName: "lsi1", wantErr: false}, - {name: "gsi_rejected", indexName: "gsi1", wantErr: true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() + ctx := t.Context() db := newInMemoryTestDB(t) - ctx := context.Background() + tableName := "CCTest_" + strings.ReplaceAll(tt.name, " ", "_") _, err := db.CreateTable(ctx, &dynamodb_sdk.CreateTableInput{ - TableName: aws.String("CRGSI"), + TableName: aws.String(tableName), KeySchema: []types.KeySchemaElement{ {AttributeName: aws.String("pk"), KeyType: types.KeyTypeHash}, {AttributeName: aws.String("sk"), KeyType: types.KeyTypeRange}, @@ -182,14 +101,8 @@ func TestConsistentRead_Query_OnGSI_Rejected(t *testing.T) { AttributeDefinitions: []types.AttributeDefinition{ {AttributeName: aws.String("pk"), AttributeType: types.ScalarAttributeTypeS}, {AttributeName: aws.String("sk"), AttributeType: types.ScalarAttributeTypeS}, - { - AttributeName: aws.String("gsi_pk"), - AttributeType: types.ScalarAttributeTypeS, - }, - { - AttributeName: aws.String("lsi_sk"), - AttributeType: types.ScalarAttributeTypeS, - }, + {AttributeName: aws.String("gsi_pk"), AttributeType: types.ScalarAttributeTypeS}, + {AttributeName: aws.String("lsi_sk"), AttributeType: types.ScalarAttributeTypeS}, }, GlobalSecondaryIndexes: []types.GlobalSecondaryIndex{ { @@ -212,171 +125,322 @@ func TestConsistentRead_Query_OnGSI_Rejected(t *testing.T) { }, BillingMode: types.BillingModePayPerRequest, }) - if err != nil { - t.Fatalf("CreateTable: %v", err) + require.NoError(t, err) + + // 1. PutItem with GSI and LSI keys + putOut, err := db.PutItem(ctx, &dynamodb_sdk.PutItemInput{ + TableName: aws.String(tableName), + Item: map[string]types.AttributeValue{ + "pk": &types.AttributeValueMemberS{Value: "k1"}, + "sk": &types.AttributeValueMemberS{Value: "s1"}, + "gsi_pk": &types.AttributeValueMemberS{Value: "g1"}, + "lsi_sk": &types.AttributeValueMemberS{Value: "l1"}, + "val": &types.AttributeValueMemberS{Value: "hello"}, + }, + ReturnConsumedCapacity: tt.reqCC, + }) + require.NoError(t, err) + + if tt.wantNil { + assert.Nil(t, putOut.ConsumedCapacity) + } else { + assertConsumedCapacityBreakdown( + t, + putOut.ConsumedCapacity, + tableName, + tt.wantMinTotal, + tt.wantTable, + tt.wantGSI, + tt.wantLSI, + ) } - input := &dynamodb_sdk.QueryInput{ - TableName: aws.String("CRGSI"), - ConsistentRead: aws.Bool(true), - KeyConditionExpression: aws.String("pk = :pk"), + // 2. Query on GSI + qOut, err := db.Query(ctx, &dynamodb_sdk.QueryInput{ + TableName: aws.String(tableName), + IndexName: aws.String("gsi1"), + KeyConditionExpression: aws.String("gsi_pk = :g"), ExpressionAttributeValues: map[string]types.AttributeValue{ - ":pk": &types.AttributeValueMemberS{Value: "p1"}, + ":g": &types.AttributeValueMemberS{Value: "g1"}, }, + ReturnConsumedCapacity: tt.reqCC, + }) + require.NoError(t, err) + + if tt.wantNil { + assert.Nil(t, qOut.ConsumedCapacity) + } else if tt.wantGSI { + require.NotNil(t, qOut.ConsumedCapacity) + require.NotNil(t, qOut.ConsumedCapacity.GlobalSecondaryIndexes) + _, hasGSI := qOut.ConsumedCapacity.GlobalSecondaryIndexes["gsi1"] + assert.True(t, hasGSI) } - if tt.indexName != "" { - input.IndexName = aws.String(tt.indexName) - if tt.indexName == "gsi1" { - input.KeyConditionExpression = aws.String("gsi_pk = :pk") - } + + // 3. Scan on LSI + scanOut, err := db.Scan(ctx, &dynamodb_sdk.ScanInput{ + TableName: aws.String(tableName), + IndexName: aws.String("lsi1"), + ReturnConsumedCapacity: tt.reqCC, + }) + require.NoError(t, err) + + if tt.wantNil { + assert.Nil(t, scanOut.ConsumedCapacity) + } else if tt.wantLSI { + require.NotNil(t, scanOut.ConsumedCapacity) + require.NotNil(t, scanOut.ConsumedCapacity.LocalSecondaryIndexes) + _, hasLSI := scanOut.ConsumedCapacity.LocalSecondaryIndexes["lsi1"] + assert.True(t, hasLSI) } - _, err = db.Query(ctx, input) - if tt.wantErr { - if err == nil || !strings.Contains(err.Error(), "ValidationException") { - t.Fatalf("expected ValidationException, got: %v", err) - } + // 4. UpdateItem + updateOut, err := db.UpdateItem(ctx, &dynamodb_sdk.UpdateItemInput{ + TableName: aws.String(tableName), + Key: map[string]types.AttributeValue{ + "pk": &types.AttributeValueMemberS{Value: "k1"}, + "sk": &types.AttributeValueMemberS{Value: "s1"}, + }, + UpdateExpression: aws.String("SET val = :newval"), + ExpressionAttributeValues: map[string]types.AttributeValue{ + ":newval": &types.AttributeValueMemberS{Value: "world"}, + }, + ReturnConsumedCapacity: tt.reqCC, + }) + require.NoError(t, err) - return - } - if err != nil { - t.Fatalf("unexpected error: %v", err) + if tt.wantNil { + assert.Nil(t, updateOut.ConsumedCapacity) + } else { + require.NotNil(t, updateOut.ConsumedCapacity) } - }) - } -} -func TestConsistentRead_Scan_DoesNotError(t *testing.T) { - t.Parallel() - db := newInMemoryTestDB(t) - ctx := context.Background() - createSimpleTestTable(t, db, "CRScan") + // 5. DeleteItem + delOut, err := db.DeleteItem(ctx, &dynamodb_sdk.DeleteItemInput{ + TableName: aws.String(tableName), + Key: map[string]types.AttributeValue{ + "pk": &types.AttributeValueMemberS{Value: "k1"}, + "sk": &types.AttributeValueMemberS{Value: "s1"}, + }, + ReturnConsumedCapacity: tt.reqCC, + }) + require.NoError(t, err) - _, err := db.Scan(ctx, &dynamodb_sdk.ScanInput{ - TableName: aws.String("CRScan"), - ConsistentRead: aws.Bool(true), - }) - if err != nil { - t.Fatalf("Scan with ConsistentRead: %v", err) + if tt.wantNil { + assert.Nil(t, delOut.ConsumedCapacity) + } else { + require.NotNil(t, delOut.ConsumedCapacity) + } + }) } } -func TestBuildConsumedCapacityWithIndexes_Total(t *testing.T) { +func TestConsistentRead_Operations(t *testing.T) { t.Parallel() - cc := dynamodb.BuildConsumedCapacityWithIndexes( - "myTable", - types.ReturnConsumedCapacityTotal, - 1.0, 0, - map[string]float64{"gsi1": 1.0}, map[string]float64{}, - nil, nil, - ) - - if cc == nil { - t.Fatal("expected ConsumedCapacity") - } - - if aws.ToString(cc.TableName) != "myTable" { - t.Errorf("wrong table name: %s", aws.ToString(cc.TableName)) - } - - if aws.ToFloat64(cc.CapacityUnits) != 2.0 { - t.Errorf("expected 2.0 total CU, got %v", aws.ToFloat64(cc.CapacityUnits)) - } - // TOTAL should not include index breakdowns. - if cc.GlobalSecondaryIndexes != nil { - t.Error("TOTAL should not include index breakdowns") + tests := []struct { + name string + op string + indexName string + consistentRead bool + wantErr bool + }{ + {name: "getitem_consistent", op: "GetItem", consistentRead: true, wantErr: false}, + {name: "getitem_eventual", op: "GetItem", consistentRead: false, wantErr: false}, + {name: "query_primary_consistent", op: "Query", indexName: "", consistentRead: true, wantErr: false}, + {name: "query_lsi_consistent", op: "Query", indexName: "lsi1", consistentRead: true, wantErr: false}, + {name: "query_gsi_consistent_rejected", op: "Query", indexName: "gsi1", consistentRead: true, wantErr: true}, + {name: "scan_consistent", op: "Scan", consistentRead: true, wantErr: false}, + {name: "scan_eventual", op: "Scan", consistentRead: false, wantErr: false}, } -} - -func TestBuildConsumedCapacityWithIndexes_Indexes(t *testing.T) { - t.Parallel() - cc := dynamodb.BuildConsumedCapacityWithIndexes( - "myTable", - types.ReturnConsumedCapacityIndexes, - 1.0, 0, - map[string]float64{"gsi1": 0.5}, map[string]float64{}, - map[string]float64{"lsi1": 0.5}, map[string]float64{}, - ) - if cc == nil { - t.Fatal("expected ConsumedCapacity") - } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + ctx := t.Context() + db := newInMemoryTestDB(t) - if _, hasGSI := cc.GlobalSecondaryIndexes["gsi1"]; !hasGSI { - t.Error("expected gsi1 in GlobalSecondaryIndexes") - } + tableName := "CRTable_" + strings.ReplaceAll(tt.name, " ", "_") + _, err := db.CreateTable(ctx, &dynamodb_sdk.CreateTableInput{ + TableName: aws.String(tableName), + KeySchema: []types.KeySchemaElement{ + {AttributeName: aws.String("pk"), KeyType: types.KeyTypeHash}, + {AttributeName: aws.String("sk"), KeyType: types.KeyTypeRange}, + }, + AttributeDefinitions: []types.AttributeDefinition{ + {AttributeName: aws.String("pk"), AttributeType: types.ScalarAttributeTypeS}, + {AttributeName: aws.String("sk"), AttributeType: types.ScalarAttributeTypeS}, + {AttributeName: aws.String("gsi_pk"), AttributeType: types.ScalarAttributeTypeS}, + {AttributeName: aws.String("lsi_sk"), AttributeType: types.ScalarAttributeTypeS}, + }, + GlobalSecondaryIndexes: []types.GlobalSecondaryIndex{ + { + IndexName: aws.String("gsi1"), + KeySchema: []types.KeySchemaElement{ + {AttributeName: aws.String("gsi_pk"), KeyType: types.KeyTypeHash}, + }, + Projection: &types.Projection{ProjectionType: types.ProjectionTypeAll}, + }, + }, + LocalSecondaryIndexes: []types.LocalSecondaryIndex{ + { + IndexName: aws.String("lsi1"), + KeySchema: []types.KeySchemaElement{ + {AttributeName: aws.String("pk"), KeyType: types.KeyTypeHash}, + {AttributeName: aws.String("lsi_sk"), KeyType: types.KeyTypeRange}, + }, + Projection: &types.Projection{ProjectionType: types.ProjectionTypeAll}, + }, + }, + BillingMode: types.BillingModePayPerRequest, + }) + require.NoError(t, err) - if _, hasLSI := cc.LocalSecondaryIndexes["lsi1"]; !hasLSI { - t.Error("expected lsi1 in LocalSecondaryIndexes") - } + putTestItem(t, db, tableName, map[string]types.AttributeValue{ + "pk": &types.AttributeValueMemberS{Value: "pk1"}, + "sk": &types.AttributeValueMemberS{Value: "sk1"}, + "gsi_pk": &types.AttributeValueMemberS{Value: "g1"}, + "lsi_sk": &types.AttributeValueMemberS{Value: "l1"}, + }) - if cc.Table == nil { - t.Error("expected Table capacity breakdown") + switch tt.op { + case "GetItem": + out, gErr := db.GetItem(ctx, &dynamodb_sdk.GetItemInput{ + TableName: aws.String(tableName), + ConsistentRead: aws.Bool(tt.consistentRead), + Key: map[string]types.AttributeValue{ + "pk": &types.AttributeValueMemberS{Value: "pk1"}, + "sk": &types.AttributeValueMemberS{Value: "sk1"}, + }, + }) + if tt.wantErr { + require.Error(t, gErr) + } else { + require.NoError(t, gErr) + require.NotNil(t, out.Item) + } + case "Query": + qInput := &dynamodb_sdk.QueryInput{ + TableName: aws.String(tableName), + ConsistentRead: aws.Bool(tt.consistentRead), + KeyConditionExpression: aws.String("pk = :pk"), + ExpressionAttributeValues: map[string]types.AttributeValue{ + ":pk": &types.AttributeValueMemberS{Value: "pk1"}, + }, + } + if tt.indexName != "" { + qInput.IndexName = aws.String(tt.indexName) + if tt.indexName == "gsi1" { + qInput.KeyConditionExpression = aws.String("gsi_pk = :g") + qInput.ExpressionAttributeValues = map[string]types.AttributeValue{ + ":g": &types.AttributeValueMemberS{Value: "g1"}, + } + } + } + _, qErr := db.Query(ctx, qInput) + if tt.wantErr { + require.Error(t, qErr) + assert.Contains(t, qErr.Error(), "ValidationException") + } else { + require.NoError(t, qErr) + } + case "Scan": + _, sErr := db.Scan(ctx, &dynamodb_sdk.ScanInput{ + TableName: aws.String(tableName), + ConsistentRead: aws.Bool(tt.consistentRead), + }) + if tt.wantErr { + require.Error(t, sErr) + } else { + require.NoError(t, sErr) + } + } + }) } } -func TestBuildConsumedCapacityWithIndexes_None(t *testing.T) { +func TestBuildConsumedCapacityWithIndexes_Unit(t *testing.T) { t.Parallel() - cc := dynamodb.BuildConsumedCapacityWithIndexes( - "myTable", - types.ReturnConsumedCapacityNone, - 1.0, 0, nil, nil, nil, nil, - ) - if cc != nil { - t.Error("expected nil for NONE") + tests := []struct { + gsiRCU map[string]float64 + lsiRCU map[string]float64 + name string + tableName string + req types.ReturnConsumedCapacity + tableRCU float64 + tableWCU float64 + wantTotal float64 + wantNil bool + wantTable bool + wantGSI bool + wantLSI bool + }{ + { + name: "total_mode", + tableName: "myTable", + req: types.ReturnConsumedCapacityTotal, + tableRCU: 1.0, + gsiRCU: map[string]float64{"gsi1": 1.0}, + wantNil: false, + wantTotal: 2.0, + wantTable: false, + wantGSI: false, + }, + { + name: "indexes_mode", + tableName: "myTable", + req: types.ReturnConsumedCapacityIndexes, + tableRCU: 1.0, + gsiRCU: map[string]float64{"gsi1": 0.5}, + lsiRCU: map[string]float64{"lsi1": 0.5}, + wantNil: false, + wantTotal: 2.0, + wantTable: true, + wantGSI: true, + wantLSI: true, + }, + { + name: "none_mode", + tableName: "myTable", + req: types.ReturnConsumedCapacityNone, + tableRCU: 1.0, + wantNil: true, + }, } -} -func TestConsistentRead_Scan_Parallel(t *testing.T) { - t.Parallel() - db := newInMemoryTestDB(t) - ctx := context.Background() - createSimpleTestTable(t, db, "CRScanP") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + cc := dynamodb.BuildConsumedCapacityWithIndexes( + tt.tableName, + tt.req, + tt.tableRCU, tt.tableWCU, + tt.gsiRCU, nil, + tt.lsiRCU, nil, + ) + if tt.wantNil { + assert.Nil(t, cc) - // Populate some items. - for i := range 10 { - putTestItem(t, db, "CRScanP", map[string]types.AttributeValue{ - "pk": &types.AttributeValueMemberS{Value: fmt.Sprintf("pk%d", i)}, - "sk": &types.AttributeValueMemberS{Value: "sk1"}, + return + } + require.NotNil(t, cc) + assert.Equal(t, tt.tableName, aws.ToString(cc.TableName)) + assert.InDelta(t, tt.wantTotal, aws.ToFloat64(cc.CapacityUnits), 1e-9) + if tt.wantTable { + assert.NotNil(t, cc.Table) + } else { + assert.Nil(t, cc.Table) + } + if tt.wantGSI { + assert.NotNil(t, cc.GlobalSecondaryIndexes) + } else { + assert.Nil(t, cc.GlobalSecondaryIndexes) + } + if tt.wantLSI { + assert.NotNil(t, cc.LocalSecondaryIndexes) + } else { + assert.Nil(t, cc.LocalSecondaryIndexes) + } }) } - - out, err := db.Scan(ctx, &dynamodb_sdk.ScanInput{ - TableName: aws.String("CRScanP"), - ConsistentRead: aws.Bool(false), - }) - if err != nil { - t.Fatalf("Scan: %v", err) - } - - if out.Count != 10 { - t.Errorf("expected 10 items, got %d", out.Count) - } -} - -func TestApplyConsistentReadMultiplier_False(t *testing.T) { - t.Parallel() - db := newInMemoryTestDB(t) - ctx := context.Background() - createSimpleTestTable(t, db, "CRMultFalse") - - putTestItem(t, db, "CRMultFalse", map[string]types.AttributeValue{ - "pk": &types.AttributeValueMemberS{Value: "pk1"}, - "sk": &types.AttributeValueMemberS{Value: "sk1"}, - }) - - // ConsistentRead=false should not error. - _, err := db.GetItem(ctx, &dynamodb_sdk.GetItemInput{ - TableName: aws.String("CRMultFalse"), - ConsistentRead: aws.Bool(false), - Key: map[string]types.AttributeValue{ - "pk": &types.AttributeValueMemberS{Value: "pk1"}, - "sk": &types.AttributeValueMemberS{Value: "sk1"}, - }, - }) - if err != nil { - t.Fatalf("GetItem with ConsistentRead=false: %v", err) - } } diff --git a/services/dynamodb/handler.go b/services/dynamodb/handler.go index efebd5ecad..04ad7703a6 100644 --- a/services/dynamodb/handler.go +++ b/services/dynamodb/handler.go @@ -947,54 +947,119 @@ func (h *DynamoDBHandler) Reset() { } } -// dispatchExtraOps routes the extended DynamoDB operations to their handlers -// using a per-action dispatch map to keep complexity low. +// dispatchExtraOps routes the extended DynamoDB operations to their handlers. func (h *DynamoDBHandler) dispatchExtraOps( ctx context.Context, action string, body []byte, ) (any, error) { - type handlerFn func() (any, error) - - enableKinesis := func() (any, error) { return h.handleEnableKinesisStreamingDestination(ctx, body) } - describeKinesis := func() (any, error) { return h.handleDescribeKinesisStreamingDestination(ctx, body) } - disableKinesis := func() (any, error) { return h.handleDisableKinesisStreamingDestination(ctx, body) } - updateKinesis := func() (any, error) { return h.handleUpdateKinesisStreamingDestination(ctx, body) } - descContrib := func() (any, error) { return h.handleDescribeContributorInsights(ctx, body) } - listContrib := func() (any, error) { return h.handleListContributorInsights(ctx, body) } - updContrib := func() (any, error) { return h.handleUpdateContributorInsights(ctx, body) } - updASReplica := func() (any, error) { return h.handleUpdateTableReplicaAutoScaling(ctx, body) } - updGTSettings := func() (any, error) { return h.handleUpdateGlobalTableSettings(ctx, body) } - - handlers := map[string]handlerFn{ - opCreateGlobalTable: func() (any, error) { return h.handleCreateGlobalTable(ctx, body) }, - opDescribeGlobalTable: func() (any, error) { return h.handleDescribeGlobalTable(ctx, body) }, - opDescribeGlobalTableSettings: func() (any, error) { return h.handleDescribeGlobalTableSettings(ctx, body) }, - opListGlobalTables: func() (any, error) { return h.handleListGlobalTables(ctx, body) }, - opUpdateGlobalTable: func() (any, error) { return h.handleUpdateGlobalTable(ctx, body) }, - opUpdateGlobalTableSettings: updGTSettings, - opEnableKinesisStreamingDestination: enableKinesis, - opDescribeKinesisStreamingDestination: describeKinesis, - opDisableKinesisStreamingDestination: disableKinesis, - opUpdateKinesisStreamingDestination: updateKinesis, - opDescribeLimits: func() (any, error) { return h.handleDescribeLimits(ctx) }, - opDescribeEndpoints: func() (any, error) { return h.handleDescribeEndpoints(ctx) }, - opDescribeContributorInsights: descContrib, - opListContributorInsights: listContrib, - opUpdateContributorInsights: updContrib, - opUpdateTableReplicaAutoScaling: updASReplica, - opGetResourcePolicy: func() (any, error) { return h.handleGetResourcePolicy(ctx, body) }, - opPutResourcePolicy: func() (any, error) { return h.handlePutResourcePolicy(ctx, body) }, - opDeleteResourcePolicy: func() (any, error) { return h.handleDeleteResourcePolicy(ctx, body) }, - opDescribeImport: func() (any, error) { return h.handleDescribeImport(ctx, body) }, - opImportTable: func() (any, error) { return h.handleImportTable(ctx, body) }, - opListImports: func() (any, error) { return h.handleListImports(ctx, body) }, - } - - fn, ok := handlers[action] - if !ok { - return nil, fmt.Errorf("%w:%s", ErrUnknownOperation, action) + res, err := h.dispatchGlobalTableOps(ctx, action, body) + if !errors.Is(err, ErrUnknownOperation) { + return res, err + } + + res, err = h.dispatchKinesisOps(ctx, action, body) + if !errors.Is(err, ErrUnknownOperation) { + return res, err + } + + res, err = h.dispatchContribAndPolicyOps(ctx, action, body) + if !errors.Is(err, ErrUnknownOperation) { + return res, err + } + + res, err = h.dispatchImportOps(ctx, action, body) + if !errors.Is(err, ErrUnknownOperation) { + return res, err + } + + return nil, fmt.Errorf("%w:%s", ErrUnknownOperation, action) +} + +func (h *DynamoDBHandler) dispatchGlobalTableOps( + ctx context.Context, + action string, + body []byte, +) (any, error) { + switch action { + case opCreateGlobalTable: + return h.handleCreateGlobalTable(ctx, body) + case opDescribeGlobalTable: + return h.handleDescribeGlobalTable(ctx, body) + case opDescribeGlobalTableSettings: + return h.handleDescribeGlobalTableSettings(ctx, body) + case opListGlobalTables: + return h.handleListGlobalTables(ctx, body) + case opUpdateGlobalTable: + return h.handleUpdateGlobalTable(ctx, body) + case opUpdateGlobalTableSettings: + return h.handleUpdateGlobalTableSettings(ctx, body) + default: + return nil, ErrUnknownOperation + } +} + +func (h *DynamoDBHandler) dispatchKinesisOps( + ctx context.Context, + action string, + body []byte, +) (any, error) { + switch action { + case opEnableKinesisStreamingDestination: + return h.handleEnableKinesisStreamingDestination(ctx, body) + case opDescribeKinesisStreamingDestination: + return h.handleDescribeKinesisStreamingDestination(ctx, body) + case opDisableKinesisStreamingDestination: + return h.handleDisableKinesisStreamingDestination(ctx, body) + case opUpdateKinesisStreamingDestination: + return h.handleUpdateKinesisStreamingDestination(ctx, body) + default: + return nil, ErrUnknownOperation + } +} + +func (h *DynamoDBHandler) dispatchContribAndPolicyOps( + ctx context.Context, + action string, + body []byte, +) (any, error) { + switch action { + case opDescribeLimits: + return h.handleDescribeLimits(ctx) + case opDescribeEndpoints: + return h.handleDescribeEndpoints(ctx) + case opDescribeContributorInsights: + return h.handleDescribeContributorInsights(ctx, body) + case opListContributorInsights: + return h.handleListContributorInsights(ctx, body) + case opUpdateContributorInsights: + return h.handleUpdateContributorInsights(ctx, body) + case opUpdateTableReplicaAutoScaling: + return h.handleUpdateTableReplicaAutoScaling(ctx, body) + case opGetResourcePolicy: + return h.handleGetResourcePolicy(ctx, body) + case opPutResourcePolicy: + return h.handlePutResourcePolicy(ctx, body) + case opDeleteResourcePolicy: + return h.handleDeleteResourcePolicy(ctx, body) + default: + return nil, ErrUnknownOperation } +} - return fn() +func (h *DynamoDBHandler) dispatchImportOps( + ctx context.Context, + action string, + body []byte, +) (any, error) { + switch action { + case opDescribeImport: + return h.handleDescribeImport(ctx, body) + case opImportTable: + return h.handleImportTable(ctx, body) + case opListImports: + return h.handleListImports(ctx, body) + default: + return nil, ErrUnknownOperation + } } diff --git a/services/dynamodb/item_ops_crud.go b/services/dynamodb/item_ops_crud.go index 4ab355b34f..5ae03c8ad2 100644 --- a/services/dynamodb/item_ops_crud.go +++ b/services/dynamodb/item_ops_crud.go @@ -364,12 +364,16 @@ func (db *InMemoryDB) populatePutItemOutput( // put. if input.ReturnConsumedCapacity != "" && input.ReturnConsumedCapacity != types.ReturnConsumedCapacityNone { - writeUnits := WriteCapacityUnits(models.FromSDKItem(input.Item)) - out.ConsumedCapacity = &types.ConsumedCapacity{ - TableName: aws.String(table.Name), - CapacityUnits: aws.Float64(writeUnits), - WriteCapacityUnits: aws.Float64(writeUnits), - } + rawItem := models.FromSDKItem(input.Item) + writeUnits := WriteCapacityUnits(rawItem) + gsiWCU, lsiWCU := calculateWriteIndexBreakdowns(table, rawItem, writeUnits) + out.ConsumedCapacity = buildConsumedCapacityWithIndexes( + table.Name, + input.ReturnConsumedCapacity, + 0, writeUnits, + nil, gsiWCU, + nil, lsiWCU, + ) } // ItemCollectionMetrics: only for tables with an LSI and when requested. @@ -450,11 +454,13 @@ func (db *InMemoryDB) GetItem( // RCU on a read is ceil(item-size / 4 KB) * 0.5 (eventually consistent) // or doubled when ConsistentRead=true. Matches the real AWS formula. readUnits := applyConsistentReadMultiplier(ReadCapacityUnits(item), consistentRead) - out.ConsumedCapacity = &types.ConsumedCapacity{ - TableName: aws.String(table.Name), - CapacityUnits: aws.Float64(readUnits), - ReadCapacityUnits: aws.Float64(readUnits), - } + out.ConsumedCapacity = buildConsumedCapacityWithIndexes( + aws.ToString(input.TableName), + input.ReturnConsumedCapacity, + readUnits, 0, + nil, nil, + nil, nil, + ) } return out, nil @@ -625,11 +631,14 @@ func (db *InMemoryDB) buildDeleteItemOutput( if oldItem != nil { writeUnits = WriteCapacityUnits(oldItem) } - out.ConsumedCapacity = &types.ConsumedCapacity{ - TableName: aws.String(table.Name), - CapacityUnits: aws.Float64(writeUnits), - WriteCapacityUnits: aws.Float64(writeUnits), - } + gsiWCU, lsiWCU := calculateWriteIndexBreakdowns(table, oldItem, writeUnits) + out.ConsumedCapacity = buildConsumedCapacityWithIndexes( + table.Name, + input.ReturnConsumedCapacity, + 0, writeUnits, + nil, gsiWCU, + nil, lsiWCU, + ) } // ItemCollectionMetrics reflect the collection remaining after the delete. @@ -935,11 +944,14 @@ func (db *InMemoryDB) populateUpdateOutput( writeUnits = w } } - out.ConsumedCapacity = &types.ConsumedCapacity{ - TableName: aws.String(table.Name), - CapacityUnits: aws.Float64(writeUnits), - WriteCapacityUnits: aws.Float64(writeUnits), - } + gsiWCU, lsiWCU := calculateWriteIndexBreakdowns(table, newItem, writeUnits) + out.ConsumedCapacity = buildConsumedCapacityWithIndexes( + table.Name, + input.ReturnConsumedCapacity, + 0, writeUnits, + nil, gsiWCU, + nil, lsiWCU, + ) } // ItemCollectionMetrics reflect the collection after the update is applied. diff --git a/services/dynamodb/item_ops_query.go b/services/dynamodb/item_ops_query.go index 7dcff6c4d3..f6db34dd12 100644 --- a/services/dynamodb/item_ops_query.go +++ b/services/dynamodb/item_ops_query.go @@ -15,32 +15,6 @@ import ( "github.com/aws/aws-sdk-go-v2/service/dynamodb/types" ) -// consumedCapacityForQuery returns a populated ConsumedCapacity when the caller -// has requested capacity reporting. Returns nil when reporting is disabled. -func consumedCapacityForQuery( - tableName string, - req types.ReturnConsumedCapacity, - scanned int, - consistentRead bool, -) *types.ConsumedCapacity { - if req == "" || req == types.ReturnConsumedCapacityNone { - return nil - } - const halfRCU = 0.5 - cu := float64(scanned) * halfRCU - if cu < halfRCU { - cu = halfRCU - } - // Strongly-consistent reads cost twice as much as eventually-consistent ones. - cu = applyConsistentReadMultiplier(cu, consistentRead) - - return &types.ConsumedCapacity{ - TableName: aws.String(tableName), - CapacityUnits: aws.Float64(cu), - ReadCapacityUnits: aws.Float64(cu), - } -} - func (db *InMemoryDB) Query( ctx context.Context, input *dynamodb.QueryInput, @@ -131,7 +105,7 @@ func (db *InMemoryDB) QueryWithContext( } return db.processQueryResults( - ctx, candidates, input, keySchema, snapshotTable.KeySchema, ttlAttr, + ctx, candidates, input, keySchema, snapshotTable.KeySchema, ttlAttr, snapshotTable, ), nil } @@ -532,6 +506,7 @@ func (db *InMemoryDB) processQueryResults( keySchema []models.KeySchemaElement, tableKeySchema []models.KeySchemaElement, ttlAttr string, + table *Table, ) *dynamodb.QueryOutput { eav := models.FromSDKItem(input.ExpressionAttributeValues) exclusiveStartKey := models.FromSDKItem(input.ExclusiveStartKey) @@ -565,9 +540,9 @@ func (db *InMemoryDB) processQueryResults( Items: outItems, Count: int32(len(items)), // #nosec G115 ScannedCount: int32(scannedCount), // #nosec G115 - ConsumedCapacity: consumedCapacityForQuery( + ConsumedCapacity: consumedCapacityForReadOp( aws.ToString(input.TableName), input.ReturnConsumedCapacity, scannedCount, - aws.ToBool(input.ConsistentRead), + aws.ToBool(input.ConsistentRead), aws.ToString(input.IndexName), table, ), } diff --git a/services/dynamodb/item_ops_scan.go b/services/dynamodb/item_ops_scan.go index db78ccbb12..0ab126e362 100644 --- a/services/dynamodb/item_ops_scan.go +++ b/services/dynamodb/item_ops_scan.go @@ -15,30 +15,6 @@ import ( ) // consumedCapacityForScan returns a populated ConsumedCapacity when the caller -// has requested capacity reporting. Returns nil when reporting is disabled. -func consumedCapacityForScan( - tableName string, - req types.ReturnConsumedCapacity, - n int, - consistentRead bool, -) *types.ConsumedCapacity { - if req == "" || req == types.ReturnConsumedCapacityNone { - return nil - } - const halfRCU = 0.5 // each 4 KB read costs 0.5 RCU for eventually-consistent reads - cu := float64(n) * halfRCU - if cu < halfRCU { - cu = halfRCU - } - // Strongly-consistent reads cost twice as much as eventually-consistent ones. - cu = applyConsistentReadMultiplier(cu, consistentRead) - - return &types.ConsumedCapacity{ - TableName: aws.String(tableName), - CapacityUnits: aws.Float64(cu), - ReadCapacityUnits: aws.Float64(cu), - } -} func (db *InMemoryDB) Scan( ctx context.Context, @@ -124,9 +100,10 @@ func (db *InMemoryDB) ScanWithContext( pkDef, skDef, keySchema, + projection, ) - return db.buildScanOutput(ctx, tableName, billingMode, input, items, lastKey, scannedCount) + return db.buildScanOutput(ctx, tableName, billingMode, input, items, lastKey, scannedCount, snapshotTable) } // snapshotTableForScan copies the item slice and table metadata needed by a @@ -168,6 +145,7 @@ func (db *InMemoryDB) buildScanOutput( items []map[string]any, lastKey map[string]any, scannedCount int32, + table *Table, ) (*dynamodb.ScanOutput, error) { // Enforce throughput: charge RCU per scanned item. // Double for strongly-consistent; bypass for PAY_PER_REQUEST. @@ -198,11 +176,13 @@ func (db *InMemoryDB) buildScanOutput( Items: outItems, Count: int32(len(items)), // #nosec G115 ScannedCount: scannedCount, - ConsumedCapacity: consumedCapacityForScan( + ConsumedCapacity: consumedCapacityForReadOp( tableName, input.ReturnConsumedCapacity, int(scannedCount), aws.ToBool(input.ConsistentRead), + aws.ToString(input.IndexName), + table, ), } @@ -259,6 +239,7 @@ func (db *InMemoryDB) doScan( input *dynamodb.ScanInput, pkDef, skDef models.KeySchemaElement, tableKeySchema []models.KeySchemaElement, + projection *models.Projection, ) ([]map[string]any, map[string]any, int32) { _ = ctx // ctx reserved for future use (e.g., metrics, cancellation) @@ -298,6 +279,11 @@ func (db *InMemoryDB) doScan( // Pre-parse the filter expression once to avoid re-parsing per item in the hot loop. parsedFilter, _ := ParseConditionStr(filter) + indexKeySchema := []models.KeySchemaElement{pkDef} + if skDef.AttributeName != "" { + indexKeySchema = append(indexKeySchema, skDef) + } + return scanPage( candidate, parsedFilter, @@ -307,6 +293,8 @@ func (db *InMemoryDB) doScan( pkDef, skDef, tableKeySchema, + indexKeySchema, + projection, limit, ) } @@ -322,7 +310,8 @@ func scanPage( eans map[string]string, projector *Projector, pkDef, skDef models.KeySchemaElement, - tableKeySchema []models.KeySchemaElement, + tableKeySchema, indexKeySchema []models.KeySchemaElement, + projection *models.Projection, limit int, ) ([]map[string]any, map[string]any, int32) { const maxResponseSize = 1024 * 1024 // 1MB @@ -345,7 +334,11 @@ func scanPage( totalScannedSize += itemSize if parsedFilter.Evaluate(item, eav, eans) { - results = append(results, projector.Project(item)) + projectedItem := item + if projection != nil { + projectedItem = applyGSIProjection(item, *projection, tableKeySchema, indexKeySchema) + } + results = append(results, projector.Project(projectedItem)) } if limit > 0 && int(scannedCount) >= limit { diff --git a/services/dynamodb/scan_test.go b/services/dynamodb/scan_test.go index 918d40251b..5aca188e0b 100644 --- a/services/dynamodb/scan_test.go +++ b/services/dynamodb/scan_test.go @@ -739,3 +739,99 @@ func TestScan_Select_SurvivesWireConversion(t *testing.T) { assert.Empty(t, resp.Items, "Select=COUNT must omit Items") assert.Equal(t, int32(1), resp.Count) } + +func TestScan_GSI_Projection_Masking(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + projType types.ProjectionType + nonKeyAttrs []string + wantHasPayload bool + wantHasExtra bool + }{ + { + name: "keys_only", + projType: types.ProjectionTypeKeysOnly, + wantHasPayload: false, + wantHasExtra: false, + }, + { + name: "include", + projType: types.ProjectionTypeInclude, + nonKeyAttrs: []string{"payload"}, + wantHasPayload: true, + wantHasExtra: false, + }, + { + name: "all", + projType: types.ProjectionTypeAll, + wantHasPayload: true, + wantHasExtra: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + ctx := t.Context() + db := dynamodb.NewInMemoryDB() + + tableName := "ScanProjTable_" + tt.name + gsiName := "gsi_proj" + _, err := db.CreateTable(ctx, &dynamodb_sdk.CreateTableInput{ + TableName: aws.String(tableName), + KeySchema: []types.KeySchemaElement{ + {AttributeName: aws.String("pk"), KeyType: types.KeyTypeHash}, + }, + AttributeDefinitions: []types.AttributeDefinition{ + {AttributeName: aws.String("pk"), AttributeType: types.ScalarAttributeTypeS}, + {AttributeName: aws.String("gsi_pk"), AttributeType: types.ScalarAttributeTypeS}, + }, + GlobalSecondaryIndexes: []types.GlobalSecondaryIndex{ + { + IndexName: aws.String(gsiName), + KeySchema: []types.KeySchemaElement{ + {AttributeName: aws.String("gsi_pk"), KeyType: types.KeyTypeHash}, + }, + Projection: &types.Projection{ + ProjectionType: tt.projType, + NonKeyAttributes: tt.nonKeyAttrs, + }, + }, + }, + BillingMode: types.BillingModePayPerRequest, + }) + require.NoError(t, err) + + _, err = db.PutItem(ctx, &dynamodb_sdk.PutItemInput{ + TableName: aws.String(tableName), + Item: map[string]types.AttributeValue{ + "pk": &types.AttributeValueMemberS{Value: "pk1"}, + "gsi_pk": &types.AttributeValueMemberS{Value: "g1"}, + "payload": &types.AttributeValueMemberS{Value: "important_data"}, + "extra": &types.AttributeValueMemberS{Value: "extra_data"}, + }, + }) + require.NoError(t, err) + + resp, err := db.Scan(ctx, &dynamodb_sdk.ScanInput{ + TableName: aws.String(tableName), + IndexName: aws.String(gsiName), + }) + require.NoError(t, err) + require.Len(t, resp.Items, 1) + + item := resp.Items[0] + _, hasPK := item["pk"] + _, hasGSIPK := item["gsi_pk"] + _, hasPayload := item["payload"] + _, hasExtra := item["extra"] + + assert.True(t, hasPK, "pk must always be projected") + assert.True(t, hasGSIPK, "gsi_pk must always be projected") + assert.Equal(t, tt.wantHasPayload, hasPayload, "payload presence mismatch") + assert.Equal(t, tt.wantHasExtra, hasExtra, "extra presence mismatch") + }) + } +} diff --git a/services/s3/PARITY.md b/services/s3/PARITY.md index 0ac3a70c23..991fa4d22e 100644 --- a/services/s3/PARITY.md +++ b/services/s3/PARITY.md @@ -18,7 +18,7 @@ ops: GetObject/HeadObject: {wire: ok, errors: ok, state: ok, persist: ok, note: FIXED response-* override query params (content-type/disposition/expires/cache-control)} PutBucketAcl: {wire: ok, errors: ok, state: ok, persist: ok, note: FIXED reject object-only canned ACLs; read AccessControlPolicy body} PutBucketReplication: {wire: ok, errors: ok, state: ok, persist: ok, note: FIXED require versioning=Enabled} - GetObjectAttributes: {wire: ok, errors: ok, state: ok, persist: ok, note: FIXED ObjectSize 0-byte, Last-Modified} + GetObjectAttributes: {wire: ok, errors: ok, state: ok, persist: ok, note: "FIXED ObjectSize 0-byte, Last-Modified, and ObjectParts (types.GetObjectAttributesParts multipart breakdown)"} DeleteBucket: {wire: ok, errors: ok, state: ok, persist: ok, note: FIXED 409 BucketNotEmpty for objects/versions/delete-markers AND incomplete multipart uploads (real AWS gotcha: MPUs block deletion despite not appearing in ListObjects)} PutBucketPolicy: {wire: ok, errors: ok, state: ok, persist: ok, note: "FIXED 2026-07-24 400 MalformedPolicy for non-JSON body; FIXED 2026-07-24 (phase 2) full IAM-policy-grammar shape validation (Version/Statement/Effect/Principal/Action/Resource presence+shape) per https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_policies_grammar.html — bucket policies are resource-based so Principal/NotPrincipal is required (real S3 error confirmed: 'MalformedPolicy: Missing required field Principal cannot be empty!')"} PostObject: {wire: ok, errors: ok, state: ok, persist: ok, note: "FIXED 2026-07-24 (phase 2): POST form fields x-amz-storage-class, x-amz-server-side-encryption(-aws-kms-key-id), and x-amz-checksum-algorithm are now applied to the uploaded object (previously silently ignored — a presigned-POST upload requesting SSE-KMS was stored unencrypted, defeating the caller's intent)"} @@ -37,7 +37,6 @@ ops: GetBucketMetadataConfiguration/GetBucketMetadataTableConfiguration: {wire: bug, errors: n/a, state: n/a, persist: ok, note: "FOUND, NOT FIXED 2026-08-15 (gopherstack-6flj wrapper-key sweep) -- the most severe finding this pass, deliberately left unfixed. Unlike every other Get*Configuration op in this file (CORS/lifecycle/notification/encryption/logging/replication/analytics/inventory/metrics/intelligent-tiering), where the real GET deserializer parses the response ROOT element directly as the same struct the PUT request root already is (confirmed per-op against deserializers.go), these two do NOT: awsRestxml_deserializeOpGetBucketMetadataConfiguration.HandleDeserialize (deserializers.go) parses the response root directly as types.GetBucketMetadataConfigurationResult, which requires a CHILD element named exactly \"MetadataConfigurationResult\" (types.MetadataConfigurationResult{DestinationResult (required, TableBucketArn/TableBucketType/TableNamespace), AnnotationTableConfigurationResult, InventoryTableConfigurationResult, JournalTableConfigurationResult}) -- a server-computed RESULT shape, structurally different from the client's CreateBucketMetadataConfiguration request body (types.MetadataConfiguration{JournalTableConfiguration, AnnotationTableConfiguration, InventoryTableConfiguration}, no ARNs/status at all). gopherstack's getBucketMetadataConfiguration/getBucketMetadataTableConfiguration (bucket_ops_metadata_table.go) echo the raw stored CREATE request body verbatim -- which has no \"MetadataConfigurationResult\"/\"MetadataTableConfigurationResult\" child element anywhere, so a real typed client's GetBucketMetadataConfigurationOutput.GetBucketMetadataConfigurationResult.MetadataConfigurationResult (and the Table variant's equivalent) decodes to nil regardless of what was created. The same OpDocument...Output wrapper function with a matching case IS present in generated code but is dead -- HandleDeserialize never calls it, the same trap gopherstack-ob1g already found and fixed once on GetBucketAbac -- so this is not a simple 'wrong root name' rename. NOT FIXED: producing a real DestinationResult requires an S3 Tables table-bucket ARN/namespace/provisioning-status concept this backend has no model for at all (no CreateBucketMetadataConfiguration path allocates a table bucket or generates an ARN); fabricating plausible-looking ARNs/status would be invented data, not a shape fix. Flagged per this campaign's own precedent for genuinely-unmodeled response shapes (matches securityhub's GetRecommendedPolicyV2 finding) rather than attempted."} gaps: - "GetBucketMetadataConfiguration/GetBucketMetadataTableConfiguration return the wrong response shape entirely for any real typed client -- see the ops row above (gopherstack-6flj, 2026-08-15). Fixing this for real requires modeling S3 Tables table-bucket provisioning (ARN/namespace/status), which this backend has no concept of anywhere; CreateBucketMetadataConfiguration/CreateBucketMetadataTableConfiguration would also need the same new state. Left flagged rather than fabricated." - - "GetObjectAttributes never emits the real, optional ObjectParts member (types.GetObjectAttributesParts, a collection of multipart-upload part checksums) -- this backend's GetObjectAttributes (objects.go) only ever reads whole-object ETag/Size/StorageClass/Checksum/LastModified from the current version, with no per-part breakdown even for objects assembled via CompleteMultipartUpload. Noted while sweeping this op during the 2026-08-15 gopherstack-6flj pass; not previously disclosed." - "Object Annotations (gopherstack-zi7k, 2026-08-14): implemented -- PutObjectAnnotation/GetObjectAnnotation/DeleteObjectAnnotation/ListObjectAnnotations store real per-object-version state (StoredObjectVersion.Annotations, additive/omitempty, survives Snapshot/Restore) and UpdateBucketMetadataAnnotationTableConfiguration persists its config XML the same way its metadataInventoryTable/metadataJournalTable siblings do. Routes verified from the pinned serializer, not by pattern: PUT/GET/DELETE /{Key+}?annotation, and GET is shared byte-for-byte between GetObjectAnnotation and ListObjectAnnotations (both httpbinding.SplitURI to the same path+query) -- disambiguated on the presence of the annotationName query param, which only GetObjectAnnotation's HttpBindings function binds. Bucket-level route key metadataAnnotationTable was independently re-verified (matches the bd issue's note). Deliberately NOT enforced: the documented 1-byte-to-1-MiB payload size window (no error code for it appears in any of these ops' own deserializeOpError switches, so inventing one would violate the same rule that caught the invented metadataTableConfiguration/exception bugs this same sweep found elsewhere) and DeleteObjectAnnotation/PutObjectAnnotation's ObjectIfMatch conditional header (also absent from every relevant switch in this pinned SDK version). DeleteObjectAnnotation deliberately does NOT return NoSuchAnnotation for a missing name -- its error switch declares only NoSuchBucket/NoSuchKey, matching real S3's idempotent-delete semantics. The annotation-name reserved-prefix rule ('cannot start with aws or s3') is enforced from DeleteObjectAnnotation's doc comment in the pinned source, not a serializer/deserializer fact -- flagged here as the one validation rule in this pass that rests on prose rather than wire code. UpdateBucketMetadataAnnotationTableConfiguration's own error switch declares no typed error cases at all (every failure decodes as smithy.GenericAPIError) -- confirmed by reading it directly, not assumed. No dedicated Get op exists for the bucket-level annotation-table config in the pinned SDK, so (like its inventory/journal siblings) persistence there is provable by store/restore but not independently readable over the wire." - "CreateSession (S3 Express One Zone) is a disguised stub beyond its own doc comment's disclosure: buckets.go's CreateSession returns a hardcoded fake SessionToken/AccessKeyId/SecretAccessKey for ANY bucket (it doesn't check IsDirectoryBucket, doesn't validate the bucket is actually a directory bucket the way real S3 requires), completely ignores the request's SessionMode (ReadOnly/ReadWrite), and the returned session token has no effect anywhere else in this package -- it isn't wired into sigv4 validation or any subsequent request's authorization, so a caller that authenticates via the returned session credentials would not actually get S3-Express-scoped access semantics. Consistent with the broader disclosed gap that this emulator does not model directory buckets/S3-Express as a distinct bucket type at all; a real fix is a full S3 Express feature addition, not scoped for this pass. (gopherstack-3dqa)" - "RenameObject is applied uniformly to any bucket (general-purpose or directory), but real S3 restricts RenameObject to directory buckets only (api_op_RenameObject.go's Bucket doc: 'The bucket name of the directory bucket containing the object... Path-style requests are not supported'). This emulator has no directory-bucket-vs-general-purpose distinction anywhere (see CreateSession gap above), so RenameObject working on any bucket is a permissive superset rather than a wire-shape bug reachable by a real client hitting a real endpoint shape. Also: RenameObjectInput's DestinationIfMatch/DestinationIfNoneMatch/DestinationIfModifiedSince/DestinationIfUnmodifiedSince conditional-header preconditions are declared on the real input but not read/enforced by handleRenameObject (object_ops_copy.go) -- a caller relying on If-None-Match:* to prevent clobbering an existing destination gets a silent unconditional overwrite instead of a 412. Not fixed this pass (scoped feature, not a one-line diff); flagged honestly rather than silently left. (gopherstack-3dqa)" diff --git a/services/s3/README.md b/services/s3/README.md index 319bdaf624..fef30e9036 100644 --- a/services/s3/README.md +++ b/services/s3/README.md @@ -16,7 +16,6 @@ ### Known gaps - GetBucketMetadataConfiguration/GetBucketMetadataTableConfiguration return the wrong response shape entirely for any real typed client -- see the ops row above (gopherstack-6flj, 2026-08-15). Fixing this for real requires modeling S3 Tables table-bucket provisioning (ARN/namespace/status), which this backend has no concept of anywhere; CreateBucketMetadataConfiguration/CreateBucketMetadataTableConfiguration would also need the same new state. Left flagged rather than fabricated. -- GetObjectAttributes never emits the real, optional ObjectParts member (types.GetObjectAttributesParts, a collection of multipart-upload part checksums) -- this backend's GetObjectAttributes (objects.go) only ever reads whole-object ETag/Size/StorageClass/Checksum/LastModified from the current version, with no per-part breakdown even for objects assembled via CompleteMultipartUpload. Noted while sweeping this op during the 2026-08-15 gopherstack-6flj pass; not previously disclosed. - Object Annotations (gopherstack-zi7k, 2026-08-14): implemented -- PutObjectAnnotation/GetObjectAnnotation/DeleteObjectAnnotation/ListObjectAnnotations store real per-object-version state (StoredObjectVersion.Annotations, additive/omitempty, survives Snapshot/Restore) and UpdateBucketMetadataAnnotationTableConfiguration persists its config XML the same way its metadataInventoryTable/metadataJournalTable siblings do. Routes verified from the pinned serializer, not by pattern: PUT/GET/DELETE /{Key+}?annotation, and GET is shared byte-for-byte between GetObjectAnnotation and ListObjectAnnotations (both httpbinding.SplitURI to the same path+query) -- disambiguated on the presence of the annotationName query param, which only GetObjectAnnotation's HttpBindings function binds. Bucket-level route key metadataAnnotationTable was independently re-verified (matches the bd issue's note). Deliberately NOT enforced: the documented 1-byte-to-1-MiB payload size window (no error code for it appears in any of these ops' own deserializeOpError switches, so inventing one would violate the same rule that caught the invented metadataTableConfiguration/exception bugs this same sweep found elsewhere) and DeleteObjectAnnotation/PutObjectAnnotation's ObjectIfMatch conditional header (also absent from every relevant switch in this pinned SDK version). DeleteObjectAnnotation deliberately does NOT return NoSuchAnnotation for a missing name -- its error switch declares only NoSuchBucket/NoSuchKey, matching real S3's idempotent-delete semantics. The annotation-name reserved-prefix rule ('cannot start with aws or s3') is enforced from DeleteObjectAnnotation's doc comment in the pinned source, not a serializer/deserializer fact -- flagged here as the one validation rule in this pass that rests on prose rather than wire code. UpdateBucketMetadataAnnotationTableConfiguration's own error switch declares no typed error cases at all (every failure decodes as smithy.GenericAPIError) -- confirmed by reading it directly, not assumed. No dedicated Get op exists for the bucket-level annotation-table config in the pinned SDK, so (like its inventory/journal siblings) persistence there is provable by store/restore but not independently readable over the wire. - CreateSession (S3 Express One Zone) is a disguised stub beyond its own doc comment's disclosure: buckets.go's CreateSession returns a hardcoded fake SessionToken/AccessKeyId/SecretAccessKey for ANY bucket (it doesn't check IsDirectoryBucket, doesn't validate the bucket is actually a directory bucket the way real S3 requires), completely ignores the request's SessionMode (ReadOnly/ReadWrite), and the returned session token has no effect anywhere else in this package -- it isn't wired into sigv4 validation or any subsequent request's authorization, so a caller that authenticates via the returned session credentials would not actually get S3-Express-scoped access semantics. Consistent with the broader disclosed gap that this emulator does not model directory buckets/S3-Express as a distinct bucket type at all; a real fix is a full S3 Express feature addition, not scoped for this pass. (gopherstack-3dqa) - RenameObject is applied uniformly to any bucket (general-purpose or directory), but real S3 restricts RenameObject to directory buckets only (api_op_RenameObject.go's Bucket doc: 'The bucket name of the directory bucket containing the object... Path-style requests are not supported'). This emulator has no directory-bucket-vs-general-purpose distinction anywhere (see CreateSession gap above), so RenameObject working on any bucket is a permissive superset rather than a wire-shape bug reachable by a real client hitting a real endpoint shape. Also: RenameObjectInput's DestinationIfMatch/DestinationIfNoneMatch/DestinationIfModifiedSince/DestinationIfUnmodifiedSince conditional-header preconditions are declared on the real input but not read/enforced by handleRenameObject (object_ops_copy.go) -- a caller relying on If-None-Match:* to prevent clobbering an existing destination gets a silent unconditional overwrite instead of a 412. Not fixed this pass (scoped feature, not a one-line diff); flagged honestly rather than silently left. (gopherstack-3dqa) diff --git a/services/s3/bucket_analytics_test.go b/services/s3/bucket_analytics_test.go index eaed4abb56..2d95a105f9 100644 --- a/services/s3/bucket_analytics_test.go +++ b/services/s3/bucket_analytics_test.go @@ -7,9 +7,10 @@ import ( "strings" "testing" - "github.com/blackbirdworks/gopherstack/services/s3" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/s3" ) func TestInMemoryBackend_AnalyticsConfig(t *testing.T) { diff --git a/services/s3/bucket_encryption_test.go b/services/s3/bucket_encryption_test.go index c76c6a73b7..6f12f45c77 100644 --- a/services/s3/bucket_encryption_test.go +++ b/services/s3/bucket_encryption_test.go @@ -8,9 +8,10 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" sdk_s3 "github.com/aws/aws-sdk-go-v2/service/s3" - "github.com/blackbirdworks/gopherstack/services/s3" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/s3" ) // TestHandler_BucketEncryption verifies the PutBucketEncryption / diff --git a/services/s3/bucket_lifecycle_test.go b/services/s3/bucket_lifecycle_test.go index a268da432e..0a95da58c4 100644 --- a/services/s3/bucket_lifecycle_test.go +++ b/services/s3/bucket_lifecycle_test.go @@ -7,9 +7,10 @@ import ( "testing" sdk_s3 "github.com/aws/aws-sdk-go-v2/service/s3" - "github.com/blackbirdworks/gopherstack/services/s3" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/s3" ) func TestS3BucketLifecycleCRUD(t *testing.T) { diff --git a/services/s3/bucket_tagging_test.go b/services/s3/bucket_tagging_test.go index 17403a7cd0..32a0f3f259 100644 --- a/services/s3/bucket_tagging_test.go +++ b/services/s3/bucket_tagging_test.go @@ -7,8 +7,9 @@ import ( "strings" "testing" - "github.com/blackbirdworks/gopherstack/services/s3" "github.com/stretchr/testify/assert" + + "github.com/blackbirdworks/gopherstack/services/s3" ) func TestHandler_BucketTagging(t *testing.T) { diff --git a/services/s3/bucket_versioning_test.go b/services/s3/bucket_versioning_test.go index c08058914f..3d3ee58d87 100644 --- a/services/s3/bucket_versioning_test.go +++ b/services/s3/bucket_versioning_test.go @@ -12,11 +12,12 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" sdk_s3 "github.com/aws/aws-sdk-go-v2/service/s3" "github.com/aws/aws-sdk-go-v2/service/s3/types" - "github.com/blackbirdworks/gopherstack/services/s3" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/s3" ) // versioningConfigXML is a minimal XML struct for parsing GetBucketVersioning responses. diff --git a/services/s3/checksum_test.go b/services/s3/checksum_test.go index ce0f71dcf5..e6c0190681 100644 --- a/services/s3/checksum_test.go +++ b/services/s3/checksum_test.go @@ -12,9 +12,10 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" sdk_s3 "github.com/aws/aws-sdk-go-v2/service/s3" - "github.com/blackbirdworks/gopherstack/services/s3" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/s3" ) func TestCRC64NVME_HashInterface(t *testing.T) { diff --git a/services/s3/compression_test.go b/services/s3/compression_test.go index 2c953209ca..a923ada157 100644 --- a/services/s3/compression_test.go +++ b/services/s3/compression_test.go @@ -8,6 +8,7 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" sdk_s3 "github.com/aws/aws-sdk-go-v2/service/s3" "github.com/aws/aws-sdk-go-v2/service/s3/types" + "github.com/blackbirdworks/gopherstack/services/s3" "github.com/stretchr/testify/assert" diff --git a/services/s3/handler.go b/services/s3/handler.go index 089780fe41..c7f6bd812f 100644 --- a/services/s3/handler.go +++ b/services/s3/handler.go @@ -417,6 +417,7 @@ func (h *S3Handler) RouteMatcher() service.Matcher { // which could be valid bucket names. if strings.HasPrefix(path, "/api/") || strings.HasPrefix(path, "/metrics/") || strings.HasPrefix(path, "/dashboard/") || strings.HasPrefix(path, "/_gopherstack/") || + strings.HasPrefix(path, "/_localstack/") || strings.HasPrefix(path, "/_aws/") || path == "/favicon.ico" || path == "/robots.txt" { return false } diff --git a/services/s3/handler_routing_test.go b/services/s3/handler_routing_test.go index 4e798a3fd1..953f0d4943 100644 --- a/services/s3/handler_routing_test.go +++ b/services/s3/handler_routing_test.go @@ -6,10 +6,11 @@ import ( "net/http/httptest" "testing" - "github.com/blackbirdworks/gopherstack/pkgs/logger" - "github.com/blackbirdworks/gopherstack/services/s3" "github.com/labstack/echo/v5" "github.com/stretchr/testify/assert" + + "github.com/blackbirdworks/gopherstack/pkgs/logger" + "github.com/blackbirdworks/gopherstack/services/s3" ) func TestHandler_VirtualHostedStyle(t *testing.T) { diff --git a/services/s3/multipart.go b/services/s3/multipart.go index 2727d08dad..59aea6d9f5 100644 --- a/services/s3/multipart.go +++ b/services/s3/multipart.go @@ -307,6 +307,7 @@ type multipartAssemblyResult struct { etag string data []byte compressedData []byte + parts []StoredObjectPart isCompressed bool } @@ -316,7 +317,7 @@ type multipartAssemblyResult struct { func (b *InMemoryBackend) collectPartsData( upload *StoredMultipartUpload, parts []types.CompletedPart, -) ([]byte, []byte, error) { +) ([]byte, []byte, []StoredObjectPart, error) { upload.mu.RLock(opCompleteMultipartUpload) defer upload.mu.RUnlock() @@ -330,11 +331,11 @@ func (b *InMemoryBackend) collectPartsData( func (b *InMemoryBackend) collectPartsDataLocked( upload *StoredMultipartUpload, parts []types.CompletedPart, -) ([]byte, []byte, error) { +) ([]byte, []byte, []StoredObjectPart, error) { // Validate ascending order. for i := 1; i < len(parts); i++ { if *parts[i].PartNumber <= *parts[i-1].PartNumber { - return nil, nil, ErrInvalidPartOrder + return nil, nil, nil, ErrInvalidPartOrder } } @@ -348,17 +349,19 @@ func (b *InMemoryBackend) collectPartsDataLocked( buf := bytes.NewBuffer(make([]byte, 0, totalSize)) md5s := make([]byte, 0, len(parts)*md5.Size) + partsMeta := make([]StoredObjectPart, 0, len(parts)) for i, part := range parts { - rawBytes, err := b.validateAndAppendPart(upload, part, i == len(parts)-1, buf) + rawBytes, spMeta, err := b.validateAndAppendPart(upload, part, i == len(parts)-1, buf) if err != nil { - return nil, nil, err + return nil, nil, nil, err } md5s = append(md5s, rawBytes...) + partsMeta = append(partsMeta, spMeta) } - return buf.Bytes(), md5s, nil + return buf.Bytes(), md5s, partsMeta, nil } // validateAndAppendPart validates a single completed part against its stored @@ -370,19 +373,19 @@ func (b *InMemoryBackend) validateAndAppendPart( part types.CompletedPart, isLastPart bool, buf *bytes.Buffer, -) ([]byte, error) { +) ([]byte, StoredObjectPart, error) { pNum := *part.PartNumber storedPart, ok := upload.Parts[pNum] if !ok { - return nil, ErrInvalidPart + return nil, StoredObjectPart{}, ErrInvalidPart } if *part.ETag != storedPart.ETag { - return nil, ErrInvalidPart + return nil, StoredObjectPart{}, ErrInvalidPart } if !isLastPart && storedPart.Size < multipartMinPartSize && !b.skipMultipartSizeCheck { - return nil, ErrEntityTooSmall + return nil, StoredObjectPart{}, ErrEntityTooSmall } buf.Write(storedPart.Data) @@ -391,10 +394,20 @@ func (b *InMemoryBackend) validateAndAppendPart( rawBytes, err := hex.DecodeString(rawETag) if err != nil { - return nil, ErrInvalidPart + return nil, StoredObjectPart{}, ErrInvalidPart } - return rawBytes, nil + meta := StoredObjectPart{ + PartNumber: pNum, + Size: storedPart.Size, + ChecksumCRC32: storedPart.ChecksumCRC32, + ChecksumCRC32C: storedPart.ChecksumCRC32C, + ChecksumCRC64NVME: storedPart.ChecksumCRC64NVME, + ChecksumSHA1: storedPart.ChecksumSHA1, + ChecksumSHA256: storedPart.ChecksumSHA256, + } + + return rawBytes, meta, nil } // assembleMultipartData reads all parts under the per-upload read lock, assembles @@ -414,7 +427,7 @@ func (b *InMemoryBackend) assembleMultipartData( parts := input.MultipartUpload.Parts - data, partMD5s, err := b.collectPartsData(upload, parts) + data, partMD5s, partsMeta, err := b.collectPartsData(upload, parts) if err != nil { return multipartAssemblyResult{}, err } @@ -442,6 +455,7 @@ func (b *InMemoryBackend) assembleMultipartData( data: data, compressedData: compressedData, etag: etag, + parts: partsMeta, isCompressed: isCompressed, }, nil } @@ -512,6 +526,7 @@ func (b *InMemoryBackend) commitMultipartObject( IsCompressed: assembled.isCompressed, Size: int64(len(assembled.data)), ETag: assembled.etag, + Parts: assembled.parts, LastModified: time.Now(), IsLatest: true, SSEAlgorithm: sse.Algorithm, diff --git a/services/s3/notification_dispatch_test.go b/services/s3/notification_dispatch_test.go index 78e4b9b833..9e14c51f5b 100644 --- a/services/s3/notification_dispatch_test.go +++ b/services/s3/notification_dispatch_test.go @@ -9,9 +9,10 @@ import ( "testing" "time" - "github.com/blackbirdworks/gopherstack/services/s3" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/s3" ) func TestHandler_NotificationDispatch_PutObject(t *testing.T) { diff --git a/services/s3/object_ops_copy_test.go b/services/s3/object_ops_copy_test.go index 2b092ee8cb..bf4091a8b0 100644 --- a/services/s3/object_ops_copy_test.go +++ b/services/s3/object_ops_copy_test.go @@ -13,9 +13,10 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" sdk_s3 "github.com/aws/aws-sdk-go-v2/service/s3" - "github.com/blackbirdworks/gopherstack/services/s3" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/s3" ) // TestHandler_CopyObject_NoSuchSource verifies that CopyObject returns 404 diff --git a/services/s3/object_ops_delete_test.go b/services/s3/object_ops_delete_test.go index 7bc2e5b4e6..15ae720676 100644 --- a/services/s3/object_ops_delete_test.go +++ b/services/s3/object_ops_delete_test.go @@ -11,9 +11,10 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" sdk_s3 "github.com/aws/aws-sdk-go-v2/service/s3" - "github.com/blackbirdworks/gopherstack/services/s3" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/s3" ) // buildDeleteBody constructs a DeleteObjects (bulk delete) request body diff --git a/services/s3/object_ops_head.go b/services/s3/object_ops_head.go index c670aa3822..357deba386 100644 --- a/services/s3/object_ops_head.go +++ b/services/s3/object_ops_head.go @@ -151,12 +151,32 @@ func (h *S3Handler) writeHeadObjectResponse( // ObjectSize carries no omitempty: a legitimate 0-byte object must still emit // 0 so the SDK populates its *int64 field. type objectAttributesResult struct { - XMLName xml.Name `xml:"GetObjectAttributesResult"` - Xmlns string `xml:"xmlns,attr"` - ETag string `xml:"ETag,omitempty"` - Checksum *attrsChecksumElem `xml:"Checksum,omitempty"` - StorageClass string `xml:"StorageClass,omitempty"` - ObjectSize int64 `xml:"ObjectSize"` + XMLName xml.Name `xml:"GetObjectAttributesResult"` + Xmlns string `xml:"xmlns,attr"` + ETag string `xml:"ETag,omitempty"` + Checksum *attrsChecksumElem `xml:"Checksum,omitempty"` + ObjectParts *objectPartsResultElem `xml:"ObjectParts,omitempty"` + StorageClass string `xml:"StorageClass,omitempty"` + ObjectSize int64 `xml:"ObjectSize"` +} + +type objectPartsResultElem struct { + Parts []objectPartElem `xml:"Part,omitempty"` + TotalPartsCount int32 `xml:"TotalPartsCount,omitempty"` + PartNumberMarker int32 `xml:"PartNumberMarker,omitempty"` + NextPartNumberMarker int32 `xml:"NextPartNumberMarker,omitempty"` + MaxParts int32 `xml:"MaxParts,omitempty"` + IsTruncated bool `xml:"IsTruncated,omitempty"` +} + +type objectPartElem struct { + ChecksumCRC32 string `xml:"ChecksumCRC32,omitempty"` + ChecksumCRC32C string `xml:"ChecksumCRC32C,omitempty"` + ChecksumCRC64NVME string `xml:"ChecksumCRC64NVME,omitempty"` + ChecksumSHA1 string `xml:"ChecksumSHA1,omitempty"` + ChecksumSHA256 string `xml:"ChecksumSHA256,omitempty"` + PartNumber int32 `xml:"PartNumber"` + Size int64 `xml:"Size"` } type attrsChecksumElem struct { @@ -167,9 +187,68 @@ type attrsChecksumElem struct { ChecksumSHA256 string `xml:"ChecksumSHA256,omitempty"` } +func buildAttrsChecksumElem(checksum map[string]string) *attrsChecksumElem { + if len(checksum) == 0 { + return nil + } + + return &attrsChecksumElem{ + ChecksumCRC32: checksum["ChecksumCRC32"], + ChecksumCRC32C: checksum["ChecksumCRC32C"], + ChecksumCRC64NVME: checksum["ChecksumCRC64NVME"], + ChecksumSHA1: checksum["ChecksumSHA1"], + ChecksumSHA256: checksum["ChecksumSHA256"], + } +} + +func buildObjectPartsResultElem(parts *ObjectPartsAttributes) *objectPartsResultElem { + if parts == nil { + return nil + } + + elem := &objectPartsResultElem{ + Parts: make([]objectPartElem, len(parts.Parts)), + TotalPartsCount: parts.TotalPartsCount, + PartNumberMarker: parts.PartNumberMarker, + NextPartNumberMarker: parts.NextPartNumberMarker, + MaxParts: parts.MaxParts, + IsTruncated: parts.IsTruncated, + } + + for i, p := range parts.Parts { + elem.Parts[i] = buildObjectPartElem(p) + } + + return elem +} + +func buildObjectPartElem(p StoredObjectPart) objectPartElem { + pe := objectPartElem{ + PartNumber: p.PartNumber, + Size: p.Size, + } + if p.ChecksumCRC32 != nil { + pe.ChecksumCRC32 = *p.ChecksumCRC32 + } + if p.ChecksumCRC32C != nil { + pe.ChecksumCRC32C = *p.ChecksumCRC32C + } + if p.ChecksumCRC64NVME != nil { + pe.ChecksumCRC64NVME = *p.ChecksumCRC64NVME + } + if p.ChecksumSHA1 != nil { + pe.ChecksumSHA1 = *p.ChecksumSHA1 + } + if p.ChecksumSHA256 != nil { + pe.ChecksumSHA256 = *p.ChecksumSHA256 + } + + return pe +} + // handleGetObjectAttributes handles GET /{bucket}/{key}?attributes. // The X-Amz-Object-Attributes header lists which attributes the caller wants; -// we always return the full set we support (ETag, ObjectSize, StorageClass, Checksum). +// we always return the full set we support (ETag, ObjectSize, StorageClass, Checksum, ObjectParts). func (h *S3Handler) handleGetObjectAttributes( ctx context.Context, w http.ResponseWriter, @@ -201,17 +280,8 @@ func (h *S3Handler) handleGetObjectAttributes( ETag: attrs.ETag, ObjectSize: attrs.ObjectSize, StorageClass: attrs.StorageClass, - } - - if len(attrs.Checksum) > 0 { - c := &attrsChecksumElem{ - ChecksumCRC32: attrs.Checksum["ChecksumCRC32"], - ChecksumCRC32C: attrs.Checksum["ChecksumCRC32C"], - ChecksumCRC64NVME: attrs.Checksum["ChecksumCRC64NVME"], - ChecksumSHA1: attrs.Checksum["ChecksumSHA1"], - ChecksumSHA256: attrs.Checksum["ChecksumSHA256"], - } - out.Checksum = c + Checksum: buildAttrsChecksumElem(attrs.Checksum), + ObjectParts: buildObjectPartsResultElem(attrs.Parts), } if versionID != "" { diff --git a/services/s3/object_ops_head_test.go b/services/s3/object_ops_head_test.go index ada52a56c6..23c1fe4afc 100644 --- a/services/s3/object_ops_head_test.go +++ b/services/s3/object_ops_head_test.go @@ -1,14 +1,18 @@ package s3_test import ( + "bytes" "net/http" "net/http/httptest" "strings" "testing" - "github.com/blackbirdworks/gopherstack/services/s3" + sdk_s3 "github.com/aws/aws-sdk-go-v2/service/s3" + "github.com/aws/aws-sdk-go-v2/service/s3/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/s3" ) func TestHandler_HeadObject_StorageClassAndAcceptRanges(t *testing.T) { @@ -123,3 +127,104 @@ func TestHandler_HeadObjectWithMetadata(t *testing.T) { }) } } + +func TestHandler_GetObjectAttributes_MultipartParts(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + bucket string + key string + partCount int + wantParts bool + wantStatus int + }{ + { + name: "multipart_object_returns_parts", + bucket: "mp-bkt", + key: "mp-obj", + partCount: 2, + wantParts: true, + wantStatus: http.StatusOK, + }, + { + name: "single_put_object_no_parts", + bucket: "mp-bkt", + key: "single-obj", + partCount: 0, + wantParts: false, + wantStatus: http.StatusOK, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + ctx := t.Context() + handler, backend := newTestHandler(t) + mustCreateBucket(t, backend, tt.bucket) + + if tt.wantParts { + // 1. Initiate multipart upload + up, err := backend.CreateMultipartUpload(ctx, &sdk_s3.CreateMultipartUploadInput{ + Bucket: &tt.bucket, + Key: &tt.key, + }) + require.NoError(t, err) + + // 2. Upload parts + p1Num := int32(1) + p1, err := backend.UploadPart(ctx, &sdk_s3.UploadPartInput{ + Bucket: &tt.bucket, + Key: &tt.key, + UploadId: up.UploadId, + PartNumber: &p1Num, + Body: bytes.NewReader([]byte("part1-payload")), + }) + require.NoError(t, err) + + p2Num := int32(2) + p2, err := backend.UploadPart(ctx, &sdk_s3.UploadPartInput{ + Bucket: &tt.bucket, + Key: &tt.key, + UploadId: up.UploadId, + PartNumber: &p2Num, + Body: bytes.NewReader([]byte("part2-payload")), + }) + require.NoError(t, err) + + // 3. Complete multipart upload + _, err = backend.CompleteMultipartUpload(ctx, &sdk_s3.CompleteMultipartUploadInput{ + Bucket: &tt.bucket, + Key: &tt.key, + UploadId: up.UploadId, + MultipartUpload: &types.CompletedMultipartUpload{ + Parts: []types.CompletedPart{ + {PartNumber: &p1Num, ETag: p1.ETag}, + {PartNumber: &p2Num, ETag: p2.ETag}, + }, + }, + }) + require.NoError(t, err) + } else { + mustPutObject(t, backend, tt.bucket, tt.key, []byte("single-put-data")) + } + + req := httptest.NewRequest(http.MethodGet, "/"+tt.bucket+"/"+tt.key+"?attributes", nil) + rec := httptest.NewRecorder() + serveS3Handler(handler, rec, req) + + require.Equal(t, tt.wantStatus, rec.Code) + body := rec.Body.String() + assert.Contains(t, body, "") + assert.Contains(t, body, "2") + assert.Contains(t, body, "1") + assert.Contains(t, body, "2") + } else { + assert.NotContains(t, body, "") + } + }) + } +} diff --git a/services/s3/objects.go b/services/s3/objects.go index 9cbee92f64..620189411d 100644 --- a/services/s3/objects.go +++ b/services/s3/objects.go @@ -50,11 +50,25 @@ func newStoredObject(key string) *StoredObject { type ObjectAttributes struct { LastModified time.Time Checksum map[string]string + Parts *ObjectPartsAttributes ETag string StorageClass string ObjectSize int64 } +// defaultMaxPartsCount is the default MaxParts ceiling returned in GetObjectAttributes. +const defaultMaxPartsCount = 1000 + +// ObjectPartsAttributes contains multipart upload parts breakdown for GetObjectAttributes. +type ObjectPartsAttributes struct { + Parts []StoredObjectPart + TotalPartsCount int32 + PartNumberMarker int32 + NextPartNumberMarker int32 + MaxParts int32 + IsTruncated bool +} + // GetObjectAttributes returns selected attributes for the latest version of an object. // versionID may be empty to select the latest version. func (b *InMemoryBackend) GetObjectAttributes( @@ -118,6 +132,15 @@ func (b *InMemoryBackend) GetObjectAttributes( out.Checksum["ChecksumCRC64NVME"] = *ver.ChecksumCRC64NVME } + if len(ver.Parts) > 0 { + out.Parts = &ObjectPartsAttributes{ + Parts: ver.Parts, + TotalPartsCount: int32(len(ver.Parts)), // #nosec G115 + MaxParts: defaultMaxPartsCount, + IsTruncated: false, + } + } + return out, nil } diff --git a/services/s3/post_object_test.go b/services/s3/post_object_test.go index ffb5cd7239..de01552b22 100644 --- a/services/s3/post_object_test.go +++ b/services/s3/post_object_test.go @@ -12,8 +12,9 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" sdk_s3 "github.com/aws/aws-sdk-go-v2/service/s3" - "github.com/blackbirdworks/gopherstack/services/s3" "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/s3" ) func TestHandler_PostObject(t *testing.T) { diff --git a/services/s3/store_listing_test.go b/services/s3/store_listing_test.go index dba1740798..366586ad61 100644 --- a/services/s3/store_listing_test.go +++ b/services/s3/store_listing_test.go @@ -10,11 +10,12 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" sdk_s3 "github.com/aws/aws-sdk-go-v2/service/s3" "github.com/aws/aws-sdk-go-v2/service/s3/types" - "github.com/blackbirdworks/gopherstack/services/s3" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/s3" ) func TestListObjects(t *testing.T) { diff --git a/services/s3/types.go b/services/s3/types.go index cdb0535af6..b1f64f7798 100644 --- a/services/s3/types.go +++ b/services/s3/types.go @@ -75,51 +75,52 @@ type StoredObject struct { // StoredObjectVersion represents a specific version of an S3 object. type StoredObjectVersion struct { - LastModified time.Time `json:"lastModified"` - RetainUntil time.Time `json:"retainUntil"` - RestoreExpiry time.Time `json:"restoreExpiry,omitzero"` - ChecksumSHA1 *string `json:"checksumSHA1,omitempty"` - Metadata map[string]string `json:"metadata,omitempty"` - // Annotations holds this version's named annotations, keyed by name. - // Additive/omitempty: annotations attach to a specific object version and - // are not independently versioned (PutObjectAnnotation/DeleteObjectAnnotation - // docs, s3@v1.106.5 api_op_DeleteObjectAnnotation.go) -- deleting one is - // permanent, there is no delete marker. - Annotations map[string]*StoredAnnotation `json:"annotations,omitempty"` - ChecksumSHA256 *string `json:"checksumSHA256,omitempty"` - ChecksumCRC32 *string `json:"checksumCRC32,omitempty"` - ChecksumCRC32C *string `json:"checksumCRC32C,omitempty"` - ChecksumCRC64NVME *string `json:"checksumCRC64NVME,omitempty"` - SSEAlgorithm string `json:"sseAlgorithm,omitempty"` - SSEKMSKeyID string `json:"sseKMSKeyID,omitempty"` - SSECAlgorithm string `json:"sseCAlgorithm,omitempty"` - SSECKeyMD5 string `json:"sseCKeyMD5,omitempty"` - // EncryptionDEK is the AES-256 DEK generated on PUT for SSE-S3/SSE-KMS - // objects (SSE-C keeps none -- the customer re-supplies it on GET). MUST - // persist: dropping it on snapshot/restore leaves the object permanently - // undecryptable, since the ciphertext in Data is persisted too. - EncryptionDEK []byte `json:"encryptionDEK,omitempty"` - // EncryptionNonce is the GCM nonce/IV for this object's ciphertext. - // Persisted for the same reason as EncryptionDEK. - EncryptionNonce []byte `json:"encryptionNonce,omitempty"` - Key string `json:"key"` - ETag string `json:"etag"` - ContentType string `json:"contentType"` - ContentEncoding string `json:"contentEncoding,omitempty"` - ContentDisposition string `json:"contentDisposition,omitempty"` - RetentionMode string `json:"retentionMode,omitempty"` - StorageClass string `json:"storageClass,omitempty"` - ACL string `json:"acl,omitempty"` - ChecksumAlgorithm types.ChecksumAlgorithm `json:"checksumAlgorithm,omitempty"` - VersionID string `json:"versionID"` - Data []byte `json:"data,omitempty"` - StorageClassTransitions []StorageClassTransition `json:"storageClassTransitions,omitempty"` - Size int64 `json:"size"` - IsCompressed bool `json:"isCompressed,omitempty"` - IsLatest bool `json:"isLatest"` - Deleted bool `json:"deleted,omitempty"` - LegalHold bool `json:"legalHold,omitempty"` - OngoingRestore bool `json:"ongoingRestore,omitempty"` + RetainUntil time.Time `json:"retainUntil"` + RestoreExpiry time.Time `json:"restoreExpiry,omitzero"` + LastModified time.Time `json:"lastModified"` + ChecksumSHA1 *string `json:"checksumSHA1,omitempty"` + Metadata map[string]string `json:"metadata,omitempty"` + Annotations map[string]*StoredAnnotation `json:"annotations,omitempty"` + ChecksumCRC64NVME *string `json:"checksumCRC64NVME,omitempty"` + ChecksumCRC32C *string `json:"checksumCRC32C,omitempty"` + ChecksumCRC32 *string `json:"checksumCRC32,omitempty"` + ChecksumSHA256 *string `json:"checksumSHA256,omitempty"` + SSEAlgorithm string `json:"sseAlgorithm,omitempty"` + VersionID string `json:"versionID"` + ChecksumAlgorithm types.ChecksumAlgorithm `json:"checksumAlgorithm,omitempty"` + SSECKeyMD5 string `json:"sseCKeyMD5,omitempty"` + SSECAlgorithm string `json:"sseCAlgorithm,omitempty"` + Key string `json:"key"` + ETag string `json:"etag"` + ContentType string `json:"contentType"` + ContentEncoding string `json:"contentEncoding,omitempty"` + ContentDisposition string `json:"contentDisposition,omitempty"` + RetentionMode string `json:"retentionMode,omitempty"` + StorageClass string `json:"storageClass,omitempty"` + ACL string `json:"acl,omitempty"` + SSEKMSKeyID string `json:"sseKMSKeyID,omitempty"` + Parts []StoredObjectPart `json:"parts,omitempty"` + EncryptionNonce []byte `json:"encryptionNonce,omitempty"` + Data []byte `json:"data,omitempty"` + StorageClassTransitions []StorageClassTransition `json:"storageClassTransitions,omitempty"` + EncryptionDEK []byte `json:"encryptionDEK,omitempty"` + Size int64 `json:"size"` + IsCompressed bool `json:"isCompressed,omitempty"` + IsLatest bool `json:"isLatest"` + Deleted bool `json:"deleted,omitempty"` + LegalHold bool `json:"legalHold,omitempty"` + OngoingRestore bool `json:"ongoingRestore,omitempty"` +} + +// StoredObjectPart represents metadata for an individual completed multipart upload part. +type StoredObjectPart struct { + ChecksumCRC32 *string `json:"checksumCRC32,omitempty"` + ChecksumCRC32C *string `json:"checksumCRC32C,omitempty"` + ChecksumCRC64NVME *string `json:"checksumCRC64NVME,omitempty"` + ChecksumSHA1 *string `json:"checksumSHA1,omitempty"` + ChecksumSHA256 *string `json:"checksumSHA256,omitempty"` + PartNumber int32 `json:"partNumber"` + Size int64 `json:"size"` } // StoredAnnotation represents a single named annotation attached to an @@ -178,14 +179,15 @@ type StoredMultipartUpload struct { // StoredPart represents a single part of a multipart upload. type StoredPart struct { - ETag string `json:"etag"` - ChecksumCRC32 *string `json:"checksumCRC32,omitempty"` - ChecksumCRC32C *string `json:"checksumCRC32C,omitempty"` - ChecksumSHA1 *string `json:"checksumSHA1,omitempty"` - ChecksumSHA256 *string `json:"checksumSHA256,omitempty"` - Data []byte `json:"data,omitempty"` - PartNumber int32 `json:"partNumber"` - Size int64 `json:"size"` + ETag string `json:"etag"` + ChecksumCRC32 *string `json:"checksumCRC32,omitempty"` + ChecksumCRC32C *string `json:"checksumCRC32C,omitempty"` + ChecksumCRC64NVME *string `json:"checksumCRC64NVME,omitempty"` + ChecksumSHA1 *string `json:"checksumSHA1,omitempty"` + ChecksumSHA256 *string `json:"checksumSHA256,omitempty"` + Data []byte `json:"data,omitempty"` + PartNumber int32 `json:"partNumber"` + Size int64 `json:"size"` } // ObjectMetadata holds internal metadata for storage operations. diff --git a/services/s3/website_test.go b/services/s3/website_test.go index 0a52be8632..7622768f7b 100644 --- a/services/s3/website_test.go +++ b/services/s3/website_test.go @@ -9,11 +9,12 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" sdk_s3 "github.com/aws/aws-sdk-go-v2/service/s3" - "github.com/blackbirdworks/gopherstack/pkgs/logger" - "github.com/blackbirdworks/gopherstack/services/s3" "github.com/labstack/echo/v5" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/pkgs/logger" + "github.com/blackbirdworks/gopherstack/services/s3" ) func TestBucketWebsiteConfiguration(t *testing.T) { From fca6d167f1b301b9ff8d509aaab9fc1c93c72995 Mon Sep 17 00:00:00 2001 From: agbishop Date: Sat, 15 Aug 2026 22:28:16 -0500 Subject: [PATCH 2/3] fix(s3,dynamodb): address review feedback and improve test coverage --- README.md | 2 +- services/dynamodb/capacity.go | 48 +++-- services/dynamodb/capacity_test.go | 205 ++++++++++++-------- services/dynamodb/export_test.go | 2 +- services/dynamodb/handler.go | 4 +- services/dynamodb/item_ops.go | 6 +- services/dynamodb/item_ops_crud.go | 13 +- services/dynamodb/item_ops_query.go | 4 +- services/dynamodb/item_ops_scan.go | 2 +- services/dynamodb/scan_test.go | 62 ++++-- services/s3/README.md | 2 +- services/s3/model.go | 15 +- services/s3/multipart.go | 104 ++++++---- services/s3/multipart_checksum_wire_test.go | 127 +++++++++--- services/s3/multipart_ops.go | 29 +-- services/s3/object_ops_head_test.go | 123 ++++++------ services/s3/objects.go | 51 +++-- 17 files changed, 503 insertions(+), 296 deletions(-) diff --git a/README.md b/README.md index 24998e7b3b..a2b9851585 100644 --- a/README.md +++ b/README.md @@ -488,7 +488,7 @@ Every service links to its own page with a coverage breakdown — audited operat | [Data Lifecycle Manager](services/dlm/README.md) | A | 8 | clean | | [EFS](services/efs/README.md) | A | 31 | 2 gaps; 2 deferred | | [FSx](services/fsx/README.md) | A | — | 13 families; 4 gaps | -| [S3](services/s3/README.md) | A | 20 | 11 gaps | +| [S3](services/s3/README.md) | A | 20 | 10 gaps | | [S3 Control](services/s3control/README.md) | A | 45 | 6 gaps; 3 deferred | | [S3 Glacier](services/glacier/README.md) | A | 33 | 2 gaps | | [S3 Tables](services/s3tables/README.md) | A | 49 | 1 gap | diff --git a/services/dynamodb/capacity.go b/services/dynamodb/capacity.go index b7718235d0..d3b7448857 100644 --- a/services/dynamodb/capacity.go +++ b/services/dynamodb/capacity.go @@ -218,20 +218,26 @@ func isIndexGSI(table *Table, indexName string) bool { return false } -// calculateWriteIndexBreakdowns determines WCU consumed on GSIs and LSIs populated by item. +// calculateWriteIndexBreakdowns determines WCU consumed on GSIs and LSIs populated by any of the items. func calculateWriteIndexBreakdowns( table *Table, - item map[string]any, writeUnits float64, + items ...map[string]any, ) (map[string]float64, map[string]float64) { - if item == nil || writeUnits <= 0 || table == nil { + if len(items) == 0 || writeUnits <= 0 || table == nil { return nil, nil } - return calculateGSIWriteBreakdowns(table, item, writeUnits), calculateLSIWriteBreakdowns(table, item, writeUnits) + return calculateGSIWriteBreakdowns( + table, + writeUnits, + items...), calculateLSIWriteBreakdowns( + table, + writeUnits, + items...) } -func calculateGSIWriteBreakdowns(table *Table, item map[string]any, writeUnits float64) map[string]float64 { +func calculateGSIWriteBreakdowns(table *Table, writeUnits float64, items ...map[string]any) map[string]float64 { if len(table.GlobalSecondaryIndexes) == 0 { return nil } @@ -240,18 +246,25 @@ func calculateGSIWriteBreakdowns(table *Table, item map[string]any, writeUnits f for i := range table.GlobalSecondaryIndexes { gsi := &table.GlobalSecondaryIndexes[i] pkDef, skDef := getPKAndSK(gsi.KeySchema) - if _, _, ok := secondaryItemKeyValues(item, pkDef, skDef); ok { - if gsiWCU == nil { - gsiWCU = make(map[string]float64) + for _, item := range items { + if item == nil { + continue + } + if _, _, ok := secondaryItemKeyValues(item, pkDef, skDef); ok { + if gsiWCU == nil { + gsiWCU = make(map[string]float64) + } + gsiWCU[gsi.IndexName] = writeUnits + + break } - gsiWCU[gsi.IndexName] = writeUnits } } return gsiWCU } -func calculateLSIWriteBreakdowns(table *Table, item map[string]any, writeUnits float64) map[string]float64 { +func calculateLSIWriteBreakdowns(table *Table, writeUnits float64, items ...map[string]any) map[string]float64 { if len(table.LocalSecondaryIndexes) == 0 { return nil } @@ -260,11 +273,18 @@ func calculateLSIWriteBreakdowns(table *Table, item map[string]any, writeUnits f for i := range table.LocalSecondaryIndexes { lsi := &table.LocalSecondaryIndexes[i] pkDef, skDef := getPKAndSK(lsi.KeySchema) - if _, _, ok := secondaryItemKeyValues(item, pkDef, skDef); ok { - if lsiWCU == nil { - lsiWCU = make(map[string]float64) + for _, item := range items { + if item == nil { + continue + } + if _, _, ok := secondaryItemKeyValues(item, pkDef, skDef); ok { + if lsiWCU == nil { + lsiWCU = make(map[string]float64) + } + lsiWCU[lsi.IndexName] = writeUnits + + break } - lsiWCU[lsi.IndexName] = writeUnits } } diff --git a/services/dynamodb/capacity_test.go b/services/dynamodb/capacity_test.go index 108199b459..a49cad5339 100644 --- a/services/dynamodb/capacity_test.go +++ b/services/dynamodb/capacity_test.go @@ -51,40 +51,65 @@ func assertConsumedCapacityBreakdown( func TestConsumedCapacity_Indexes_TableDriven(t *testing.T) { t.Parallel() - tests := []struct { - name string - reqCC types.ReturnConsumedCapacity + type ccArgs struct { + reqCC types.ReturnConsumedCapacity + } + + type ccWant struct { wantMinTotal float64 wantTable bool wantGSI bool wantLSI bool wantNil bool + } + + tests := []struct { + name string + args ccArgs + want ccWant }{ { - name: "indexes_requested", - reqCC: types.ReturnConsumedCapacityIndexes, - wantMinTotal: 1.0, - wantTable: true, - wantGSI: true, - wantLSI: true, - wantNil: false, + name: "indexes_requested", + args: ccArgs{ + reqCC: types.ReturnConsumedCapacityIndexes, + }, + want: ccWant{ + wantMinTotal: 1.0, + wantTable: true, + wantGSI: true, + wantLSI: true, + wantNil: false, + }, }, { - name: "total_requested", - reqCC: types.ReturnConsumedCapacityTotal, - wantMinTotal: 1.0, - wantTable: false, - wantGSI: false, - wantLSI: false, - wantNil: false, + name: "total_requested", + args: ccArgs{ + reqCC: types.ReturnConsumedCapacityTotal, + }, + want: ccWant{ + wantMinTotal: 1.0, + wantTable: false, + wantGSI: false, + wantLSI: false, + wantNil: false, + }, }, { - name: "none_requested", - reqCC: types.ReturnConsumedCapacityNone, - wantNil: true, + name: "none_requested", + args: ccArgs{ + reqCC: types.ReturnConsumedCapacityNone, + }, + want: ccWant{ + wantNil: true, + }, }, } + const ( + gsi1Name = "gsi1" + lsi1Name = "lsi1" + ) + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { t.Parallel() @@ -106,7 +131,7 @@ func TestConsumedCapacity_Indexes_TableDriven(t *testing.T) { }, GlobalSecondaryIndexes: []types.GlobalSecondaryIndex{ { - IndexName: aws.String("gsi1"), + IndexName: aws.String(gsi1Name), KeySchema: []types.KeySchemaElement{ {AttributeName: aws.String("gsi_pk"), KeyType: types.KeyTypeHash}, }, @@ -115,7 +140,7 @@ func TestConsumedCapacity_Indexes_TableDriven(t *testing.T) { }, LocalSecondaryIndexes: []types.LocalSecondaryIndex{ { - IndexName: aws.String("lsi1"), + IndexName: aws.String(lsi1Name), KeySchema: []types.KeySchemaElement{ {AttributeName: aws.String("pk"), KeyType: types.KeyTypeHash}, {AttributeName: aws.String("lsi_sk"), KeyType: types.KeyTypeRange}, @@ -137,59 +162,59 @@ func TestConsumedCapacity_Indexes_TableDriven(t *testing.T) { "lsi_sk": &types.AttributeValueMemberS{Value: "l1"}, "val": &types.AttributeValueMemberS{Value: "hello"}, }, - ReturnConsumedCapacity: tt.reqCC, + ReturnConsumedCapacity: tt.args.reqCC, }) require.NoError(t, err) - if tt.wantNil { + if tt.want.wantNil { assert.Nil(t, putOut.ConsumedCapacity) } else { assertConsumedCapacityBreakdown( t, putOut.ConsumedCapacity, tableName, - tt.wantMinTotal, - tt.wantTable, - tt.wantGSI, - tt.wantLSI, + tt.want.wantMinTotal, + tt.want.wantTable, + tt.want.wantGSI, + tt.want.wantLSI, ) } // 2. Query on GSI qOut, err := db.Query(ctx, &dynamodb_sdk.QueryInput{ TableName: aws.String(tableName), - IndexName: aws.String("gsi1"), + IndexName: aws.String(gsi1Name), KeyConditionExpression: aws.String("gsi_pk = :g"), ExpressionAttributeValues: map[string]types.AttributeValue{ ":g": &types.AttributeValueMemberS{Value: "g1"}, }, - ReturnConsumedCapacity: tt.reqCC, + ReturnConsumedCapacity: tt.args.reqCC, }) require.NoError(t, err) - if tt.wantNil { + if tt.want.wantNil { assert.Nil(t, qOut.ConsumedCapacity) - } else if tt.wantGSI { + } else if tt.want.wantGSI { require.NotNil(t, qOut.ConsumedCapacity) require.NotNil(t, qOut.ConsumedCapacity.GlobalSecondaryIndexes) - _, hasGSI := qOut.ConsumedCapacity.GlobalSecondaryIndexes["gsi1"] + _, hasGSI := qOut.ConsumedCapacity.GlobalSecondaryIndexes[gsi1Name] assert.True(t, hasGSI) } // 3. Scan on LSI scanOut, err := db.Scan(ctx, &dynamodb_sdk.ScanInput{ TableName: aws.String(tableName), - IndexName: aws.String("lsi1"), - ReturnConsumedCapacity: tt.reqCC, + IndexName: aws.String(lsi1Name), + ReturnConsumedCapacity: tt.args.reqCC, }) require.NoError(t, err) - if tt.wantNil { + if tt.want.wantNil { assert.Nil(t, scanOut.ConsumedCapacity) - } else if tt.wantLSI { + } else if tt.want.wantLSI { require.NotNil(t, scanOut.ConsumedCapacity) require.NotNil(t, scanOut.ConsumedCapacity.LocalSecondaryIndexes) - _, hasLSI := scanOut.ConsumedCapacity.LocalSecondaryIndexes["lsi1"] + _, hasLSI := scanOut.ConsumedCapacity.LocalSecondaryIndexes[lsi1Name] assert.True(t, hasLSI) } @@ -204,11 +229,11 @@ func TestConsumedCapacity_Indexes_TableDriven(t *testing.T) { ExpressionAttributeValues: map[string]types.AttributeValue{ ":newval": &types.AttributeValueMemberS{Value: "world"}, }, - ReturnConsumedCapacity: tt.reqCC, + ReturnConsumedCapacity: tt.args.reqCC, }) require.NoError(t, err) - if tt.wantNil { + if tt.want.wantNil { assert.Nil(t, updateOut.ConsumedCapacity) } else { require.NotNil(t, updateOut.ConsumedCapacity) @@ -221,11 +246,11 @@ func TestConsumedCapacity_Indexes_TableDriven(t *testing.T) { "pk": &types.AttributeValueMemberS{Value: "k1"}, "sk": &types.AttributeValueMemberS{Value: "s1"}, }, - ReturnConsumedCapacity: tt.reqCC, + ReturnConsumedCapacity: tt.args.reqCC, }) require.NoError(t, err) - if tt.wantNil { + if tt.want.wantNil { assert.Nil(t, delOut.ConsumedCapacity) } else { require.NotNil(t, delOut.ConsumedCapacity) @@ -361,50 +386,70 @@ func TestConsistentRead_Operations(t *testing.T) { func TestBuildConsumedCapacityWithIndexes_Unit(t *testing.T) { t.Parallel() - tests := []struct { + type builderArgs struct { gsiRCU map[string]float64 lsiRCU map[string]float64 - name string tableName string req types.ReturnConsumedCapacity tableRCU float64 tableWCU float64 + } + + type builderWant struct { wantTotal float64 wantNil bool wantTable bool wantGSI bool wantLSI bool + } + + tests := []struct { + name string + args builderArgs + want builderWant }{ { - name: "total_mode", - tableName: "myTable", - req: types.ReturnConsumedCapacityTotal, - tableRCU: 1.0, - gsiRCU: map[string]float64{"gsi1": 1.0}, - wantNil: false, - wantTotal: 2.0, - wantTable: false, - wantGSI: false, + name: "total_mode", + args: builderArgs{ + tableName: "myTable", + req: types.ReturnConsumedCapacityTotal, + tableRCU: 1.0, + gsiRCU: map[string]float64{"gsi1": 1.0}, + }, + want: builderWant{ + wantNil: false, + wantTotal: 2.0, + wantTable: false, + wantGSI: false, + }, }, { - name: "indexes_mode", - tableName: "myTable", - req: types.ReturnConsumedCapacityIndexes, - tableRCU: 1.0, - gsiRCU: map[string]float64{"gsi1": 0.5}, - lsiRCU: map[string]float64{"lsi1": 0.5}, - wantNil: false, - wantTotal: 2.0, - wantTable: true, - wantGSI: true, - wantLSI: true, + name: "indexes_mode", + args: builderArgs{ + tableName: "myTable", + req: types.ReturnConsumedCapacityIndexes, + tableRCU: 1.0, + gsiRCU: map[string]float64{"gsi1": 0.5}, + lsiRCU: map[string]float64{"lsi1": 0.5}, + }, + want: builderWant{ + wantNil: false, + wantTotal: 2.0, + wantTable: true, + wantGSI: true, + wantLSI: true, + }, }, { - name: "none_mode", - tableName: "myTable", - req: types.ReturnConsumedCapacityNone, - tableRCU: 1.0, - wantNil: true, + name: "none_mode", + args: builderArgs{ + tableName: "myTable", + req: types.ReturnConsumedCapacityNone, + tableRCU: 1.0, + }, + want: builderWant{ + wantNil: true, + }, }, } @@ -412,31 +457,31 @@ func TestBuildConsumedCapacityWithIndexes_Unit(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() cc := dynamodb.BuildConsumedCapacityWithIndexes( - tt.tableName, - tt.req, - tt.tableRCU, tt.tableWCU, - tt.gsiRCU, nil, - tt.lsiRCU, nil, + tt.args.tableName, + tt.args.req, + tt.args.tableRCU, tt.args.tableWCU, + tt.args.gsiRCU, nil, + tt.args.lsiRCU, nil, ) - if tt.wantNil { + if tt.want.wantNil { assert.Nil(t, cc) return } require.NotNil(t, cc) - assert.Equal(t, tt.tableName, aws.ToString(cc.TableName)) - assert.InDelta(t, tt.wantTotal, aws.ToFloat64(cc.CapacityUnits), 1e-9) - if tt.wantTable { + assert.Equal(t, tt.args.tableName, aws.ToString(cc.TableName)) + assert.InDelta(t, tt.want.wantTotal, aws.ToFloat64(cc.CapacityUnits), 1e-9) + if tt.want.wantTable { assert.NotNil(t, cc.Table) } else { assert.Nil(t, cc.Table) } - if tt.wantGSI { + if tt.want.wantGSI { assert.NotNil(t, cc.GlobalSecondaryIndexes) } else { assert.Nil(t, cc.GlobalSecondaryIndexes) } - if tt.wantLSI { + if tt.want.wantLSI { assert.NotNil(t, cc.LocalSecondaryIndexes) } else { assert.Nil(t, cc.LocalSecondaryIndexes) diff --git a/services/dynamodb/export_test.go b/services/dynamodb/export_test.go index f9d8f3f4e0..8c292404ad 100644 --- a/services/dynamodb/export_test.go +++ b/services/dynamodb/export_test.go @@ -48,7 +48,7 @@ func ApplyGSIProjection( tableKeySchema []models.KeySchemaElement, indexKeySchema []models.KeySchemaElement, ) map[string]any { - return applyGSIProjection(item, proj, tableKeySchema, indexKeySchema) + return applyIndexProjection(item, proj, tableKeySchema, indexKeySchema) } func ParseStr(v any) string { diff --git a/services/dynamodb/handler.go b/services/dynamodb/handler.go index 04ad7703a6..0ad59e27c9 100644 --- a/services/dynamodb/handler.go +++ b/services/dynamodb/handler.go @@ -947,7 +947,9 @@ func (h *DynamoDBHandler) Reset() { } } -// dispatchExtraOps routes the extended DynamoDB operations to their handlers. +// dispatchExtraOps routes non-CRUD administrative and integration operations (global tables, +// Kinesis destinations, contributor insights, resource policies, and imports) to keep the primary +// data plane dispatch focused. func (h *DynamoDBHandler) dispatchExtraOps( ctx context.Context, action string, diff --git a/services/dynamodb/item_ops.go b/services/dynamodb/item_ops.go index de1a85407b..72e49e4428 100644 --- a/services/dynamodb/item_ops.go +++ b/services/dynamodb/item_ops.go @@ -403,11 +403,11 @@ func compareScalarField(leftVal, rightVal any) bool { return fmt.Sprintf("%v", leftVal) == fmt.Sprintf("%v", rightVal) } -func applyGSIProjection( +func applyIndexProjection( item map[string]any, projection models.Projection, tableSchema []models.KeySchemaElement, - gsiSchema []models.KeySchemaElement, + indexSchema []models.KeySchemaElement, ) map[string]any { if projection.ProjectionType == "ALL" { return item @@ -420,7 +420,7 @@ func applyGSIProjection( } } - for _, k := range gsiSchema { + for _, k := range indexSchema { if val, ok := item[k.AttributeName]; ok { newItem[k.AttributeName] = val } diff --git a/services/dynamodb/item_ops_crud.go b/services/dynamodb/item_ops_crud.go index 5ae03c8ad2..20f22a04d1 100644 --- a/services/dynamodb/item_ops_crud.go +++ b/services/dynamodb/item_ops_crud.go @@ -119,7 +119,7 @@ func (db *InMemoryDB) putItemLocked( } globalTableName := table.GlobalTableName - out := db.populatePutItemOutput(input, table, oldItem, lsiCollectionBytes) + out := db.populatePutItemOutput(input, table, oldItem, wireItem, lsiCollectionBytes) return out, globalTableName, region, nil } @@ -349,7 +349,7 @@ func (db *InMemoryDB) validateItem(item map[string]any, table *Table) error { func (db *InMemoryDB) populatePutItemOutput( input *dynamodb.PutItemInput, table *Table, - oldItem map[string]any, + oldItem, wireItem map[string]any, lsiCollectionBytes int64, ) *dynamodb.PutItemOutput { out := &dynamodb.PutItemOutput{} @@ -364,9 +364,8 @@ func (db *InMemoryDB) populatePutItemOutput( // put. if input.ReturnConsumedCapacity != "" && input.ReturnConsumedCapacity != types.ReturnConsumedCapacityNone { - rawItem := models.FromSDKItem(input.Item) - writeUnits := WriteCapacityUnits(rawItem) - gsiWCU, lsiWCU := calculateWriteIndexBreakdowns(table, rawItem, writeUnits) + writeUnits := WriteCapacityUnits(wireItem) + gsiWCU, lsiWCU := calculateWriteIndexBreakdowns(table, writeUnits, wireItem) out.ConsumedCapacity = buildConsumedCapacityWithIndexes( table.Name, input.ReturnConsumedCapacity, @@ -631,7 +630,7 @@ func (db *InMemoryDB) buildDeleteItemOutput( if oldItem != nil { writeUnits = WriteCapacityUnits(oldItem) } - gsiWCU, lsiWCU := calculateWriteIndexBreakdowns(table, oldItem, writeUnits) + gsiWCU, lsiWCU := calculateWriteIndexBreakdowns(table, writeUnits, oldItem) out.ConsumedCapacity = buildConsumedCapacityWithIndexes( table.Name, input.ReturnConsumedCapacity, @@ -944,7 +943,7 @@ func (db *InMemoryDB) populateUpdateOutput( writeUnits = w } } - gsiWCU, lsiWCU := calculateWriteIndexBreakdowns(table, newItem, writeUnits) + gsiWCU, lsiWCU := calculateWriteIndexBreakdowns(table, writeUnits, oldItem, newItem) out.ConsumedCapacity = buildConsumedCapacityWithIndexes( table.Name, input.ReturnConsumedCapacity, diff --git a/services/dynamodb/item_ops_query.go b/services/dynamodb/item_ops_query.go index f6db34dd12..7b765fecf0 100644 --- a/services/dynamodb/item_ops_query.go +++ b/services/dynamodb/item_ops_query.go @@ -394,7 +394,7 @@ func (db *InMemoryDB) tryFilterUsingSecondaryIndex( // GSI/LSI queries must respect the index's declared projection -- see // filterCandidatesScan, which applies the same projection on the scan path. for i, c := range candidates { - candidates[i] = applyGSIProjection(c, *projection, table.KeySchema, keySchema) + candidates[i] = applyIndexProjection(c, *projection, table.KeySchema, keySchema) } return candidates, true @@ -463,7 +463,7 @@ func (db *InMemoryDB) filterCandidatesScan( if idxName != "" { candidates = append( candidates, - applyGSIProjection(item, *projection, table.KeySchema, keySchema), + applyIndexProjection(item, *projection, table.KeySchema, keySchema), ) } else { candidates = append(candidates, item) diff --git a/services/dynamodb/item_ops_scan.go b/services/dynamodb/item_ops_scan.go index 0ab126e362..2b9d465082 100644 --- a/services/dynamodb/item_ops_scan.go +++ b/services/dynamodb/item_ops_scan.go @@ -336,7 +336,7 @@ func scanPage( if parsedFilter.Evaluate(item, eav, eans) { projectedItem := item if projection != nil { - projectedItem = applyGSIProjection(item, *projection, tableKeySchema, indexKeySchema) + projectedItem = applyIndexProjection(item, *projection, tableKeySchema, indexKeySchema) } results = append(results, projector.Project(projectedItem)) } diff --git a/services/dynamodb/scan_test.go b/services/dynamodb/scan_test.go index 5aca188e0b..968c839b0c 100644 --- a/services/dynamodb/scan_test.go +++ b/services/dynamodb/scan_test.go @@ -743,31 +743,51 @@ func TestScan_Select_SurvivesWireConversion(t *testing.T) { func TestScan_GSI_Projection_Masking(t *testing.T) { t.Parallel() - tests := []struct { - name string - projType types.ProjectionType - nonKeyAttrs []string + type scanArgs struct { + projType types.ProjectionType + nonKeyAttrs []string + } + + type scanWant struct { wantHasPayload bool wantHasExtra bool + } + + tests := []struct { + name string + args scanArgs + want scanWant }{ { - name: "keys_only", - projType: types.ProjectionTypeKeysOnly, - wantHasPayload: false, - wantHasExtra: false, + name: "keys_only", + args: scanArgs{ + projType: types.ProjectionTypeKeysOnly, + }, + want: scanWant{ + wantHasPayload: false, + wantHasExtra: false, + }, }, { - name: "include", - projType: types.ProjectionTypeInclude, - nonKeyAttrs: []string{"payload"}, - wantHasPayload: true, - wantHasExtra: false, + name: "include", + args: scanArgs{ + projType: types.ProjectionTypeInclude, + nonKeyAttrs: []string{"payload"}, + }, + want: scanWant{ + wantHasPayload: true, + wantHasExtra: false, + }, }, { - name: "all", - projType: types.ProjectionTypeAll, - wantHasPayload: true, - wantHasExtra: true, + name: "all", + args: scanArgs{ + projType: types.ProjectionTypeAll, + }, + want: scanWant{ + wantHasPayload: true, + wantHasExtra: true, + }, }, } @@ -795,8 +815,8 @@ func TestScan_GSI_Projection_Masking(t *testing.T) { {AttributeName: aws.String("gsi_pk"), KeyType: types.KeyTypeHash}, }, Projection: &types.Projection{ - ProjectionType: tt.projType, - NonKeyAttributes: tt.nonKeyAttrs, + ProjectionType: tt.args.projType, + NonKeyAttributes: tt.args.nonKeyAttrs, }, }, }, @@ -830,8 +850,8 @@ func TestScan_GSI_Projection_Masking(t *testing.T) { assert.True(t, hasPK, "pk must always be projected") assert.True(t, hasGSIPK, "gsi_pk must always be projected") - assert.Equal(t, tt.wantHasPayload, hasPayload, "payload presence mismatch") - assert.Equal(t, tt.wantHasExtra, hasExtra, "extra presence mismatch") + assert.Equal(t, tt.want.wantHasPayload, hasPayload, "payload presence mismatch") + assert.Equal(t, tt.want.wantHasExtra, hasExtra, "extra presence mismatch") }) } } diff --git a/services/s3/README.md b/services/s3/README.md index fef30e9036..36650dfcbb 100644 --- a/services/s3/README.md +++ b/services/s3/README.md @@ -9,7 +9,7 @@ | --- | --- | | Operations audited | 20 (19 ok, 1 other) | | Feature families | 8 (8 ok) | -| Known gaps | 11 | +| Known gaps | 10 | | Deferred items | 0 | | Resource leaks | clean | diff --git a/services/s3/model.go b/services/s3/model.go index d2adc74c9a..5266626e82 100644 --- a/services/s3/model.go +++ b/services/s3/model.go @@ -417,13 +417,14 @@ type ListPartsResult struct { // PartXML describes a single uploaded part in a multipart upload. type PartXML struct { - ETag string `xml:"ETag"` - ChecksumCRC32 string `xml:"ChecksumCRC32,omitempty"` - ChecksumCRC32C string `xml:"ChecksumCRC32C,omitempty"` - ChecksumSHA1 string `xml:"ChecksumSHA1,omitempty"` - ChecksumSHA256 string `xml:"ChecksumSHA256,omitempty"` - Size int64 `xml:"Size"` - PartNumber int `xml:"PartNumber"` + ETag string `xml:"ETag"` + ChecksumCRC32 string `xml:"ChecksumCRC32,omitempty"` + ChecksumCRC32C string `xml:"ChecksumCRC32C,omitempty"` + ChecksumCRC64NVME string `xml:"ChecksumCRC64NVME,omitempty"` + ChecksumSHA1 string `xml:"ChecksumSHA1,omitempty"` + ChecksumSHA256 string `xml:"ChecksumSHA256,omitempty"` + Size int64 `xml:"Size"` + PartNumber int `xml:"PartNumber"` } // ServerSideEncryptionConfiguration is the XML body for PutBucketEncryption / GetBucketEncryption. diff --git a/services/s3/multipart.go b/services/s3/multipart.go index 59aea6d9f5..486990594c 100644 --- a/services/s3/multipart.go +++ b/services/s3/multipart.go @@ -120,23 +120,10 @@ func (b *InMemoryBackend) UploadPart( var buf bytes.Buffer writers := []io.Writer{md5Hasher, &buf} - var s3Hasher hash.Hash - algo := string(input.ChecksumAlgorithm) - if algo != "" { - switch strings.ToUpper(algo) { - case ChecksumCRC32: - s3Hasher = crc32.NewIEEE() - case ChecksumCRC32C: - s3Hasher = crc32.New(crc32.MakeTable(crc32.Castagnoli)) - case ChecksumSHA1: - //nolint:gosec // SHA1 supported - s3Hasher = sha1.New() - case ChecksumSHA256: - s3Hasher = sha256.New() - } - if s3Hasher != nil { - writers = append(writers, s3Hasher) - } + algo := inferChecksumAlgo(input) + s3Hasher := newS3Hasher(algo) + if s3Hasher != nil { + writers = append(writers, s3Hasher) } tr := io.TeeReader(input.Body, io.MultiWriter(writers...)) @@ -172,24 +159,26 @@ func (b *InMemoryBackend) UploadPart( // otherwise has no way to report them, since they exist only on this // call's request/response and were never persisted onto the part before. if sErr := b.storePart(bucketName, uploadID, partNumber, &StoredPart{ - PartNumber: partNumber, - Data: storedData, - ETag: quotedETag, - Size: originalSize, - ChecksumCRC32: input.ChecksumCRC32, - ChecksumCRC32C: input.ChecksumCRC32C, - ChecksumSHA1: input.ChecksumSHA1, - ChecksumSHA256: input.ChecksumSHA256, + PartNumber: partNumber, + Data: storedData, + ETag: quotedETag, + Size: originalSize, + ChecksumCRC32: input.ChecksumCRC32, + ChecksumCRC32C: input.ChecksumCRC32C, + ChecksumCRC64NVME: input.ChecksumCRC64NVME, + ChecksumSHA1: input.ChecksumSHA1, + ChecksumSHA256: input.ChecksumSHA256, }); sErr != nil { return nil, sErr } return &s3.UploadPartOutput{ - ETag: aws.String(quotedETag), - ChecksumCRC32: input.ChecksumCRC32, - ChecksumCRC32C: input.ChecksumCRC32C, - ChecksumSHA1: input.ChecksumSHA1, - ChecksumSHA256: input.ChecksumSHA256, + ETag: aws.String(quotedETag), + ChecksumCRC32: input.ChecksumCRC32, + ChecksumCRC32C: input.ChecksumCRC32C, + ChecksumCRC64NVME: input.ChecksumCRC64NVME, + ChecksumSHA1: input.ChecksumSHA1, + ChecksumSHA256: input.ChecksumSHA256, }, nil } @@ -819,13 +808,14 @@ func (b *InMemoryBackend) ListParts( } p := upload.Parts[pn] parts = append(parts, types.Part{ - PartNumber: aws.Int32(pn), - ETag: aws.String(p.ETag), - Size: aws.Int64(p.Size), - ChecksumCRC32: p.ChecksumCRC32, - ChecksumCRC32C: p.ChecksumCRC32C, - ChecksumSHA1: p.ChecksumSHA1, - ChecksumSHA256: p.ChecksumSHA256, + PartNumber: aws.Int32(pn), + ETag: aws.String(p.ETag), + Size: aws.Int64(p.Size), + ChecksumCRC32: p.ChecksumCRC32, + ChecksumCRC32C: p.ChecksumCRC32C, + ChecksumCRC64NVME: p.ChecksumCRC64NVME, + ChecksumSHA1: p.ChecksumSHA1, + ChecksumSHA256: p.ChecksumSHA256, }) } }() @@ -875,3 +865,43 @@ func (b *InMemoryBackend) storePart( return nil } + +func inferChecksumAlgo(input *s3.UploadPartInput) string { + algo := string(input.ChecksumAlgorithm) + if algo != "" { + return algo + } + + switch { + case input.ChecksumCRC32 != nil: + return ChecksumCRC32 + case input.ChecksumCRC32C != nil: + return ChecksumCRC32C + case input.ChecksumCRC64NVME != nil: + return ChecksumCRC64NVME + case input.ChecksumSHA1 != nil: + return ChecksumSHA1 + case input.ChecksumSHA256 != nil: + return ChecksumSHA256 + default: + return "" + } +} + +func newS3Hasher(algo string) hash.Hash { + switch strings.ToUpper(algo) { + case ChecksumCRC32: + return crc32.NewIEEE() + case ChecksumCRC32C: + return crc32.New(crc32.MakeTable(crc32.Castagnoli)) + case ChecksumCRC64NVME: + return NewCRC64NVME() + case ChecksumSHA1: + //nolint:gosec // SHA1 supported + return sha1.New() + case ChecksumSHA256: + return sha256.New() + default: + return nil + } +} diff --git a/services/s3/multipart_checksum_wire_test.go b/services/s3/multipart_checksum_wire_test.go index 1f5b4ca164..396a279eff 100644 --- a/services/s3/multipart_checksum_wire_test.go +++ b/services/s3/multipart_checksum_wire_test.go @@ -22,41 +22,106 @@ import ( func TestUploadPart_ChecksumEchoedInResponseAndListParts(t *testing.T) { t.Parallel() - client := newRealS3ClientTest(t) - bucket := "mp-checksum-bucket" - key := "obj.bin" + tests := []struct { + validatePart func(t *testing.T, part *sdk_s3.UploadPartOutput, listed types.Part) + name string + algo types.ChecksumAlgorithm + }{ + { + name: "crc32", + algo: types.ChecksumAlgorithmCrc32, + validatePart: func(t *testing.T, part *sdk_s3.UploadPartOutput, listed types.Part) { + t.Helper() + require.NotNil(t, part.ChecksumCRC32) + assert.NotEmpty(t, *part.ChecksumCRC32) + require.NotNil(t, listed.ChecksumCRC32) + assert.Equal(t, *part.ChecksumCRC32, *listed.ChecksumCRC32) + }, + }, + { + name: "crc32c", + algo: types.ChecksumAlgorithmCrc32c, + validatePart: func(t *testing.T, part *sdk_s3.UploadPartOutput, listed types.Part) { + t.Helper() + require.NotNil(t, part.ChecksumCRC32C) + assert.NotEmpty(t, *part.ChecksumCRC32C) + require.NotNil(t, listed.ChecksumCRC32C) + assert.Equal(t, *part.ChecksumCRC32C, *listed.ChecksumCRC32C) + }, + }, + { + name: "crc64nvme", + algo: types.ChecksumAlgorithmCrc64nvme, + validatePart: func(t *testing.T, part *sdk_s3.UploadPartOutput, listed types.Part) { + t.Helper() + require.NotNil(t, part.ChecksumCRC64NVME) + assert.NotEmpty(t, *part.ChecksumCRC64NVME) + require.NotNil(t, listed.ChecksumCRC64NVME) + assert.Equal(t, *part.ChecksumCRC64NVME, *listed.ChecksumCRC64NVME) + }, + }, + { + name: "sha1", + algo: types.ChecksumAlgorithmSha1, + validatePart: func(t *testing.T, part *sdk_s3.UploadPartOutput, listed types.Part) { + t.Helper() + require.NotNil(t, part.ChecksumSHA1) + assert.NotEmpty(t, *part.ChecksumSHA1) + require.NotNil(t, listed.ChecksumSHA1) + assert.Equal(t, *part.ChecksumSHA1, *listed.ChecksumSHA1) + }, + }, + { + name: "sha256", + algo: types.ChecksumAlgorithmSha256, + validatePart: func(t *testing.T, part *sdk_s3.UploadPartOutput, listed types.Part) { + t.Helper() + require.NotNil(t, part.ChecksumSHA256) + assert.NotEmpty(t, *part.ChecksumSHA256) + require.NotNil(t, listed.ChecksumSHA256) + assert.Equal(t, *part.ChecksumSHA256, *listed.ChecksumSHA256) + }, + }, + } - _, err := client.CreateBucket(t.Context(), &sdk_s3.CreateBucketInput{Bucket: aws.String(bucket)}) - require.NoError(t, err) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() - created, err := client.CreateMultipartUpload(t.Context(), &sdk_s3.CreateMultipartUploadInput{ - Bucket: aws.String(bucket), - Key: aws.String(key), - }) - require.NoError(t, err) - uploadID := created.UploadId + client := newRealS3ClientTest(t) + bucket := "mp-checksum-bucket-" + tt.name + key := "obj.bin" - part, err := client.UploadPart(t.Context(), &sdk_s3.UploadPartInput{ - Bucket: aws.String(bucket), - Key: aws.String(key), - UploadId: uploadID, - PartNumber: aws.Int32(1), - Body: strings.NewReader("payload-bytes-for-checksum"), - ChecksumAlgorithm: types.ChecksumAlgorithmCrc32, - }) - require.NoError(t, err) - require.NotNil(t, part.ChecksumCRC32, "UploadPart response must echo the verified part checksum") - assert.NotEmpty(t, *part.ChecksumCRC32) + _, err := client.CreateBucket(t.Context(), &sdk_s3.CreateBucketInput{Bucket: aws.String(bucket)}) + require.NoError(t, err) - listed, err := client.ListParts(t.Context(), &sdk_s3.ListPartsInput{ - Bucket: aws.String(bucket), - Key: aws.String(key), - UploadId: uploadID, - }) - require.NoError(t, err) - require.Len(t, listed.Parts, 1) - require.NotNil(t, listed.Parts[0].ChecksumCRC32, "ListParts must report the checksum recorded at UploadPart time") - assert.Equal(t, *part.ChecksumCRC32, *listed.Parts[0].ChecksumCRC32) + created, err := client.CreateMultipartUpload(t.Context(), &sdk_s3.CreateMultipartUploadInput{ + Bucket: aws.String(bucket), + Key: aws.String(key), + }) + require.NoError(t, err) + uploadID := created.UploadId + + part, err := client.UploadPart(t.Context(), &sdk_s3.UploadPartInput{ + Bucket: aws.String(bucket), + Key: aws.String(key), + UploadId: uploadID, + PartNumber: aws.Int32(1), + Body: strings.NewReader("payload-bytes-for-checksum"), + ChecksumAlgorithm: tt.algo, + }) + require.NoError(t, err) + + listed, err := client.ListParts(t.Context(), &sdk_s3.ListPartsInput{ + Bucket: aws.String(bucket), + Key: aws.String(key), + UploadId: uploadID, + }) + require.NoError(t, err) + require.Len(t, listed.Parts, 1) + tt.validatePart(t, part, listed.Parts[0]) + }) + } } // TestListObjectVersions_ChecksumAlgorithmPopulated is a regression test for diff --git a/services/s3/multipart_ops.go b/services/s3/multipart_ops.go index 5f98acefa3..3bda658f39 100644 --- a/services/s3/multipart_ops.go +++ b/services/s3/multipart_ops.go @@ -104,6 +104,10 @@ func (h *S3Handler) uploadPart( } algo, crc32p, crc32cp, sha1p, sha256p := extractAlgoAndChecksums(r) + crc64nvmeP := extractCRC64NVMEChecksum(r) + if algo == "" && crc64nvmeP != nil { + algo = ChecksumCRC64NVME + } out, err := h.Backend.UploadPart(ctx, &s3.UploadPartInput{ Bucket: aws.String(bucketName), @@ -114,6 +118,7 @@ func (h *S3Handler) uploadPart( ChecksumAlgorithm: types.ChecksumAlgorithm(algo), ChecksumCRC32: crc32p, ChecksumCRC32C: crc32cp, + ChecksumCRC64NVME: crc64nvmeP, ChecksumSHA1: sha1p, ChecksumSHA256: sha256p, }) @@ -132,10 +137,11 @@ func (h *S3Handler) uploadPart( w.Header().Set("ETag", *out.ETag) h.setChecksumHeaders(w, objectCommonDetails{ - ChecksumCRC32: out.ChecksumCRC32, - ChecksumCRC32C: out.ChecksumCRC32C, - ChecksumSHA1: out.ChecksumSHA1, - ChecksumSHA256: out.ChecksumSHA256, + ChecksumCRC32: out.ChecksumCRC32, + ChecksumCRC32C: out.ChecksumCRC32C, + ChecksumCRC64NVME: out.ChecksumCRC64NVME, + ChecksumSHA1: out.ChecksumSHA1, + ChecksumSHA256: out.ChecksumSHA256, }) w.WriteHeader(http.StatusOK) } @@ -457,13 +463,14 @@ func (h *S3Handler) listParts( for _, p := range out.Parts { result.Parts = append(result.Parts, PartXML{ - PartNumber: int(aws.ToInt32(p.PartNumber)), - ETag: aws.ToString(p.ETag), - Size: aws.ToInt64(p.Size), - ChecksumCRC32: aws.ToString(p.ChecksumCRC32), - ChecksumCRC32C: aws.ToString(p.ChecksumCRC32C), - ChecksumSHA1: aws.ToString(p.ChecksumSHA1), - ChecksumSHA256: aws.ToString(p.ChecksumSHA256), + PartNumber: int(aws.ToInt32(p.PartNumber)), + ETag: aws.ToString(p.ETag), + Size: aws.ToInt64(p.Size), + ChecksumCRC32: aws.ToString(p.ChecksumCRC32), + ChecksumCRC32C: aws.ToString(p.ChecksumCRC32C), + ChecksumCRC64NVME: aws.ToString(p.ChecksumCRC64NVME), + ChecksumSHA1: aws.ToString(p.ChecksumSHA1), + ChecksumSHA256: aws.ToString(p.ChecksumSHA256), }) } diff --git a/services/s3/object_ops_head_test.go b/services/s3/object_ops_head_test.go index 23c1fe4afc..a4982019a1 100644 --- a/services/s3/object_ops_head_test.go +++ b/services/s3/object_ops_head_test.go @@ -2,6 +2,7 @@ package s3_test import ( "bytes" + "fmt" "net/http" "net/http/httptest" "strings" @@ -131,29 +132,45 @@ func TestHandler_HeadObjectWithMetadata(t *testing.T) { func TestHandler_GetObjectAttributes_MultipartParts(t *testing.T) { t.Parallel() - tests := []struct { - name string - bucket string - key string - partCount int - wantParts bool + type headArgs struct { + bucket string + key string + partCount int + } + + type headWant struct { wantStatus int + wantParts bool + } + + tests := []struct { + name string + args headArgs + want headWant }{ { - name: "multipart_object_returns_parts", - bucket: "mp-bkt", - key: "mp-obj", - partCount: 2, - wantParts: true, - wantStatus: http.StatusOK, + name: "multipart_object_returns_parts", + args: headArgs{ + bucket: "mp-bkt", + key: "mp-obj", + partCount: 2, + }, + want: headWant{ + wantParts: true, + wantStatus: http.StatusOK, + }, }, { - name: "single_put_object_no_parts", - bucket: "mp-bkt", - key: "single-obj", - partCount: 0, - wantParts: false, - wantStatus: http.StatusOK, + name: "single_put_object_no_parts", + args: headArgs{ + bucket: "mp-bkt", + key: "single-obj", + partCount: 0, + }, + want: headWant{ + wantParts: false, + wantStatus: http.StatusOK, + }, }, } @@ -162,66 +179,58 @@ func TestHandler_GetObjectAttributes_MultipartParts(t *testing.T) { t.Parallel() ctx := t.Context() handler, backend := newTestHandler(t) - mustCreateBucket(t, backend, tt.bucket) + mustCreateBucket(t, backend, tt.args.bucket) - if tt.wantParts { - // 1. Initiate multipart upload + if tt.args.partCount > 0 { up, err := backend.CreateMultipartUpload(ctx, &sdk_s3.CreateMultipartUploadInput{ - Bucket: &tt.bucket, - Key: &tt.key, + Bucket: &tt.args.bucket, + Key: &tt.args.key, }) require.NoError(t, err) - // 2. Upload parts - p1Num := int32(1) - p1, err := backend.UploadPart(ctx, &sdk_s3.UploadPartInput{ - Bucket: &tt.bucket, - Key: &tt.key, - UploadId: up.UploadId, - PartNumber: &p1Num, - Body: bytes.NewReader([]byte("part1-payload")), - }) - require.NoError(t, err) - - p2Num := int32(2) - p2, err := backend.UploadPart(ctx, &sdk_s3.UploadPartInput{ - Bucket: &tt.bucket, - Key: &tt.key, - UploadId: up.UploadId, - PartNumber: &p2Num, - Body: bytes.NewReader([]byte("part2-payload")), - }) - require.NoError(t, err) + completedParts := make([]types.CompletedPart, 0, tt.args.partCount) + for i := 1; i <= tt.args.partCount; i++ { + pNum := int32(i) + p, uErr := backend.UploadPart(ctx, &sdk_s3.UploadPartInput{ + Bucket: &tt.args.bucket, + Key: &tt.args.key, + UploadId: up.UploadId, + PartNumber: &pNum, + Body: bytes.NewReader(fmt.Appendf(nil, "part%d-payload", i)), + }) + require.NoError(t, uErr) + completedParts = append(completedParts, types.CompletedPart{ + PartNumber: &pNum, + ETag: p.ETag, + }) + } - // 3. Complete multipart upload _, err = backend.CompleteMultipartUpload(ctx, &sdk_s3.CompleteMultipartUploadInput{ - Bucket: &tt.bucket, - Key: &tt.key, + Bucket: &tt.args.bucket, + Key: &tt.args.key, UploadId: up.UploadId, MultipartUpload: &types.CompletedMultipartUpload{ - Parts: []types.CompletedPart{ - {PartNumber: &p1Num, ETag: p1.ETag}, - {PartNumber: &p2Num, ETag: p2.ETag}, - }, + Parts: completedParts, }, }) require.NoError(t, err) } else { - mustPutObject(t, backend, tt.bucket, tt.key, []byte("single-put-data")) + mustPutObject(t, backend, tt.args.bucket, tt.args.key, []byte("single-put-data")) } - req := httptest.NewRequest(http.MethodGet, "/"+tt.bucket+"/"+tt.key+"?attributes", nil) + req := httptest.NewRequest(http.MethodGet, "/"+tt.args.bucket+"/"+tt.args.key+"?attributes", nil) rec := httptest.NewRecorder() serveS3Handler(handler, rec, req) - require.Equal(t, tt.wantStatus, rec.Code) + require.Equal(t, tt.want.wantStatus, rec.Code) body := rec.Body.String() assert.Contains(t, body, "") - assert.Contains(t, body, "2") - assert.Contains(t, body, "1") - assert.Contains(t, body, "2") + assert.Contains(t, body, fmt.Sprintf("%d", tt.args.partCount)) + for i := 1; i <= tt.args.partCount; i++ { + assert.Contains(t, body, fmt.Sprintf("%d", i)) + } } else { assert.NotContains(t, body, "") } diff --git a/services/s3/objects.go b/services/s3/objects.go index 620189411d..599d0c9602 100644 --- a/services/s3/objects.go +++ b/services/s3/objects.go @@ -860,8 +860,6 @@ func (b *InMemoryBackend) verifyChecksum( } computedSum := s3Hasher.Sum(nil) - - // Go's crc32 Sum(nil) may not be big-endian. S3 expects big-endian for CRC32/CRC32C. if h32, ok := s3Hasher.(hash.Hash32); ok { const checksumSize = 4 tmp := make([]byte, checksumSize) @@ -870,19 +868,7 @@ func (b *InMemoryBackend) verifyChecksum( } computedChecksumB64 := base64.StdEncoding.EncodeToString(computedSum) - - var supplied *string - - switch strings.ToUpper(algo) { - case ChecksumCRC32: - supplied = input.ChecksumCRC32 - case ChecksumCRC32C: - supplied = input.ChecksumCRC32C - case ChecksumSHA1: - supplied = input.ChecksumSHA1 - case ChecksumSHA256: - supplied = input.ChecksumSHA256 - } + supplied := suppliedPartChecksum(input, algo) if supplied != nil && *supplied != "" { if computedChecksumB64 != *supplied { @@ -892,19 +878,42 @@ func (b *InMemoryBackend) verifyChecksum( return nil } - // Client requested server-side checksum computation; propagate result. + setPartChecksum(input, algo, computedChecksumB64) + + return nil +} + +func suppliedPartChecksum(input *s3.UploadPartInput, algo string) *string { switch strings.ToUpper(algo) { case ChecksumCRC32: - input.ChecksumCRC32 = aws.String(computedChecksumB64) + return input.ChecksumCRC32 case ChecksumCRC32C: - input.ChecksumCRC32C = aws.String(computedChecksumB64) + return input.ChecksumCRC32C + case ChecksumCRC64NVME: + return input.ChecksumCRC64NVME case ChecksumSHA1: - input.ChecksumSHA1 = aws.String(computedChecksumB64) + return input.ChecksumSHA1 case ChecksumSHA256: - input.ChecksumSHA256 = aws.String(computedChecksumB64) + return input.ChecksumSHA256 + default: + return nil } +} - return nil +func setPartChecksum(input *s3.UploadPartInput, algo, checksum string) { + cs := aws.String(checksum) + switch strings.ToUpper(algo) { + case ChecksumCRC32: + input.ChecksumCRC32 = cs + case ChecksumCRC32C: + input.ChecksumCRC32C = cs + case ChecksumCRC64NVME: + input.ChecksumCRC64NVME = cs + case ChecksumSHA1: + input.ChecksumSHA1 = cs + case ChecksumSHA256: + input.ChecksumSHA256 = cs + } } // checkPutObjectAuthAndLock performs initial checks for bucket existence and object lock. From ff3b5dc3de5df8c364cb61a656c7552fdc7163f5 Mon Sep 17 00:00:00 2001 From: agbishop Date: Sat, 15 Aug 2026 22:46:15 -0500 Subject: [PATCH 3/3] fix(review): address review feedback on cli, httputils, awsmeta, and s3 GetObjectAttributes --- cli_test.go | 8 +- pkgs/awsmeta/awsmeta_test.go | 103 ++++++++++------ pkgs/httputils/httputils.go | 118 ++++++++++-------- pkgs/httputils/httputils_test.go | 87 ++++++++++++++ pkgs/httputils/pool.go | 1 + pkgs/httputils/pool_test.go | 179 ++++++++++++++++++---------- pkgs/httputils/sigv4.go | 8 +- pkgs/telemetry/memstats_test.go | 3 +- services/s3/interfaces.go | 1 + services/s3/object_ops_head.go | 32 ++++- services/s3/object_ops_head_test.go | 64 ++++++++-- services/s3/objects.go | 105 ++++++++++------ 12 files changed, 502 insertions(+), 207 deletions(-) diff --git a/cli_test.go b/cli_test.go index f545c4b3c9..85efac95ca 100644 --- a/cli_test.go +++ b/cli_test.go @@ -3074,8 +3074,10 @@ func TestLocalstackCompatibilityEndpoints(t *testing.T) { errCh <- run(ctx, cli) }() + client := &http.Client{Timeout: 2 * time.Second} + require.Eventually(t, func() bool { - resp, err := http.Get(fmt.Sprintf("http://localhost:%d/_gopherstack/health", port)) + resp, err := client.Get(fmt.Sprintf("http://localhost:%d/_gopherstack/health", port)) if err != nil { return false } @@ -3137,13 +3139,15 @@ func TestLocalstackCompatibilityEndpoints(t *testing.T) { t.Helper() assert.Equal(t, "community", body["edition"]) assert.Equal(t, false, body["is_auth"]) + assert.NotEmpty(t, body["version"]) + assert.Equal(t, "00000000-0000-0000-0000-000000000000", body["session_id"]) }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - resp, err := http.Get(fmt.Sprintf("http://localhost:%d%s", port, tt.path)) + resp, err := client.Get(fmt.Sprintf("http://localhost:%d%s", port, tt.path)) require.NoError(t, err) defer resp.Body.Close() diff --git a/pkgs/awsmeta/awsmeta_test.go b/pkgs/awsmeta/awsmeta_test.go index 278173802b..510bb47c5d 100644 --- a/pkgs/awsmeta/awsmeta_test.go +++ b/pkgs/awsmeta/awsmeta_test.go @@ -71,61 +71,94 @@ func TestGetSet(t *testing.T) { } } -func TestConvenienceAccessors(t *testing.T) { +func TestExtractors(t *testing.T) { t.Parallel() - tests := []struct { - name string - ctx context.Context //nolint:containedctx // table-driven test input + type metaArgs struct { + setupCtx func(t *testing.T) context.Context + } + + type metaWant struct { region string account string partition string accessKeyID string service string + } + + tests := []struct { + name string + args metaArgs + want metaWant }{ { - name: "nil-context-returns-defaults", - ctx: nil, - region: "", - account: awsmeta.DefaultAccount, - partition: awsmeta.DefaultPartition, - accessKeyID: "", - service: "", + name: "nil-context-returns-defaults", + args: metaArgs{ + setupCtx: func(t *testing.T) context.Context { + t.Helper() + + return nil + }, + }, + want: metaWant{ + region: "", + account: awsmeta.DefaultAccount, + partition: awsmeta.DefaultPartition, + accessKeyID: "", + service: "", + }, }, { - name: "empty-context-returns-defaults", - ctx: context.Background(), - region: "", - account: awsmeta.DefaultAccount, - partition: awsmeta.DefaultPartition, - accessKeyID: "", - service: "", + name: "empty-context-returns-defaults", + args: metaArgs{ + setupCtx: func(t *testing.T) context.Context { + t.Helper() + + return t.Context() + }, + }, + want: metaWant{ + region: "", + account: awsmeta.DefaultAccount, + partition: awsmeta.DefaultPartition, + accessKeyID: "", + service: "", + }, }, { name: "populated-context", - ctx: awsmeta.Set(context.Background(), &awsmeta.Metadata{ - Account: "111111111111", - Region: "eu-west-1", - Partition: "aws", - AccessKeyID: "AKIAIOSFODNN7EXAMPLE", - Service: "dynamodb", - }), - region: "eu-west-1", - account: "111111111111", - partition: "aws", - accessKeyID: "AKIAIOSFODNN7EXAMPLE", - service: "dynamodb", + args: metaArgs{ + setupCtx: func(t *testing.T) context.Context { + t.Helper() + + return awsmeta.Set(t.Context(), &awsmeta.Metadata{ + Account: "111111111111", + Region: "eu-west-1", + Partition: "aws", + AccessKeyID: "AKIAIOSFODNN7EXAMPLE", + Service: "dynamodb", + }) + }, + }, + want: metaWant{ + region: "eu-west-1", + account: "111111111111", + partition: "aws", + accessKeyID: "AKIAIOSFODNN7EXAMPLE", + service: "dynamodb", + }, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { t.Parallel() - assert.Equal(t, tc.region, awsmeta.Region(tc.ctx)) - assert.Equal(t, tc.account, awsmeta.Account(tc.ctx)) - assert.Equal(t, tc.partition, awsmeta.Partition(tc.ctx)) - assert.Equal(t, tc.accessKeyID, awsmeta.AccessKeyID(tc.ctx)) - assert.Equal(t, tc.service, awsmeta.Service(tc.ctx)) + ctx := tc.args.setupCtx(t) + assert.Equal(t, tc.want.region, awsmeta.Region(ctx)) + assert.Equal(t, tc.want.account, awsmeta.Account(ctx)) + assert.Equal(t, tc.want.partition, awsmeta.Partition(ctx)) + assert.Equal(t, tc.want.accessKeyID, awsmeta.AccessKeyID(ctx)) + assert.Equal(t, tc.want.service, awsmeta.Service(ctx)) }) } } diff --git a/pkgs/httputils/httputils.go b/pkgs/httputils/httputils.go index c36817ef64..40431eb53a 100644 --- a/pkgs/httputils/httputils.go +++ b/pkgs/httputils/httputils.go @@ -294,86 +294,100 @@ func RequestIDMiddleware() echo.MiddlewareFunc { } } -// minSigV4CredentialParts is the minimum number of slash-separated parts in the -// SigV4 credential scope needed to safely read the region at index 2 -// (AKID/date/region/...). -const minSigV4CredentialParts = 3 +// expectedSigV4ScopeParts is the exact number of slash-separated parts in a valid SigV4 credential scope: +// AKID/date/region/service/aws4_request. +const ( + expectedSigV4ScopeParts = 5 + sigV4AccessKeyIndex = 0 + sigV4DateIndex = 1 + sigV4RegionIndex = 2 + sigV4ServiceIndex = 3 + sigV4TerminalIndex = 4 + sigV4TerminalScope = "aws4_request" +) -// sigV4ServiceIndex is the index of the service name in the credential scope parts. -const sigV4ServiceIndex = 3 +func parseValidSigV4Scope(raw string) []string { + if idx := strings.IndexAny(raw, ", \t\r\n"); idx != -1 { + raw = raw[:idx] + } -// ExtractRegionFromRequest extracts the AWS region from an HTTP request. -// It checks the SigV4 Authorization header credential scope first, then the -// X-Amz-Region header, then falls back to defaultRegion. -func ExtractRegionFromRequest(r *http.Request, defaultRegion string) string { - if auth := r.Header.Get("Authorization"); auth != "" && strings.Contains(auth, "Credential=") { - parts := strings.Split(auth, "Credential=") - if len(parts) > 1 { - credParts := strings.Split(parts[1], "/") - if len(credParts) >= minSigV4CredentialParts { - return SanitizeHeaderString(credParts[2]) - } + parts := strings.Split(raw, "/") + if len(parts) != expectedSigV4ScopeParts { + return nil + } + + for _, p := range parts { + if strings.TrimSpace(p) == "" { + return nil } } - if region := r.Header.Get("X-Amz-Region"); region != "" { - return SanitizeHeaderString(region) + if parts[sigV4TerminalIndex] != sigV4TerminalScope { + return nil } - return SanitizeHeaderString(defaultRegion) + return parts } -// ExtractServiceFromRequest extracts the AWS service name from the SigV4 Authorization -// header credential scope (AKID/date/region/service/aws4_request). -// Returns an empty string if the service name cannot be determined. -func ExtractServiceFromRequest(r *http.Request) string { +func extractSigV4ScopeFromRequest(r *http.Request) []string { if r == nil { - return "" + return nil } if auth := r.Header.Get("Authorization"); auth != "" && strings.Contains(auth, "Credential=") { parts := strings.Split(auth, "Credential=") if len(parts) > 1 { - credParts := strings.Split(parts[1], "/") - if len(credParts) > sigV4ServiceIndex { - return SanitizeHeaderString(credParts[sigV4ServiceIndex]) + if scope := parseValidSigV4Scope(parts[1]); scope != nil { + return scope } } } - if cred := r.URL.Query().Get("X-Amz-Credential"); cred != "" { - credParts := strings.Split(cred, "/") - if len(credParts) > sigV4ServiceIndex { - return SanitizeHeaderString(credParts[sigV4ServiceIndex]) + if r.URL != nil { + if cred := r.URL.Query().Get("X-Amz-Credential"); cred != "" { + if scope := parseValidSigV4Scope(cred); scope != nil { + return scope + } } } - return "" + return nil } -// ExtractAccessKeyFromRequest extracts the AWS access key ID from an HTTP request. -// It checks the SigV4 Authorization header credential scope (AKID/date/region/service/aws4_request) -// first, then the X-Amz-Credential query parameter, and returns an empty string if none is found. -func ExtractAccessKeyFromRequest(r *http.Request) string { - if r == nil { - return "" - } +// ExtractRegionFromRequest extracts the AWS region from an HTTP request. +// It checks the SigV4 Authorization header credential scope first, then query credential, +// then the X-Amz-Region header, then falls back to defaultRegion. +func ExtractRegionFromRequest(r *http.Request, defaultRegion string) string { + if r != nil { + if scope := extractSigV4ScopeFromRequest(r); scope != nil { + return SanitizeHeaderString(scope[sigV4RegionIndex]) + } - if auth := r.Header.Get("Authorization"); auth != "" && strings.Contains(auth, "Credential=") { - parts := strings.Split(auth, "Credential=") - if len(parts) > 1 { - credParts := strings.Split(parts[1], "/") - if len(credParts) > 0 { - return SanitizeHeaderString(credParts[0]) - } + if region := r.Header.Get("X-Amz-Region"); region != "" { + return SanitizeHeaderString(region) } } - if cred := r.URL.Query().Get("X-Amz-Credential"); cred != "" { - credParts := strings.Split(cred, "/") - if len(credParts) > 0 { - return SanitizeHeaderString(credParts[0]) - } + return SanitizeHeaderString(defaultRegion) +} + +// ExtractServiceFromRequest extracts the AWS service name from the SigV4 Authorization +// header credential scope or X-Amz-Credential query parameter. +// Returns an empty string if the service name cannot be determined. +func ExtractServiceFromRequest(r *http.Request) string { + if scope := extractSigV4ScopeFromRequest(r); scope != nil { + return SanitizeHeaderString(scope[sigV4ServiceIndex]) + } + + return "" +} + +// ExtractAccessKeyFromRequest extracts the AWS access key ID from an HTTP request. +// It checks the SigV4 Authorization header credential scope first, then the +// X-Amz-Credential query parameter, and returns an empty string if none is found. +func ExtractAccessKeyFromRequest(r *http.Request) string { + if scope := extractSigV4ScopeFromRequest(r); scope != nil { + return SanitizeHeaderString(scope[sigV4AccessKeyIndex]) } return "" diff --git a/pkgs/httputils/httputils_test.go b/pkgs/httputils/httputils_test.go index 0b62a070b8..57b26f4f33 100644 --- a/pkgs/httputils/httputils_test.go +++ b/pkgs/httputils/httputils_test.go @@ -361,6 +361,93 @@ func TestExtractRegionFromRequest(t *testing.T) { } } +func TestExtractServiceAndAccessKeyFromRequest(t *testing.T) { + t.Parallel() + + type reqArgs struct { + auth string + query string + } + + type reqWant struct { + accessKey string + service string + region string + } + + tests := []struct { + name string + args reqArgs + want reqWant + }{ + { + name: "valid_authorization_header", + args: reqArgs{ + auth: "AWS4-HMAC-SHA256 Credential=AKIAIOSFODNN7EXAMPLE/20240101/us-east-1/dynamodb/aws4_request, " + + "SignedHeaders=host, Signature=abc", + }, + want: reqWant{ + accessKey: "AKIAIOSFODNN7EXAMPLE", + service: "dynamodb", + region: "us-east-1", + }, + }, + { + name: "valid_query_credential", + args: reqArgs{ + query: "X-Amz-Credential=AKIAIOSFODNN7EXAMPLE/20240101/us-west-2/s3/aws4_request", + }, + want: reqWant{ + accessKey: "AKIAIOSFODNN7EXAMPLE", + service: "s3", + region: "us-west-2", + }, + }, + { + name: "malformed_auth_falls_back_to_valid_query", + args: reqArgs{ + auth: "AWS4-HMAC-SHA256 Credential=bad/short/scope, SignedHeaders=host, Signature=abc", + query: "X-Amz-Credential=AKIAIOSFODNN7EXAMPLE/20240101/ap-south-1/sns/aws4_request", + }, + want: reqWant{ + accessKey: "AKIAIOSFODNN7EXAMPLE", + service: "sns", + region: "ap-south-1", + }, + }, + { + name: "incomplete_scope_rejected", + args: reqArgs{ + auth: "AWS4-HMAC-SHA256 Credential=AKIA/20240101/eu-central-1/sqs, SignedHeaders=host", + }, + want: reqWant{ + accessKey: "", + service: "", + region: "us-east-1", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + target := "/" + if tt.args.query != "" { + target += "?" + tt.args.query + } + req := httptest.NewRequest(http.MethodGet, target, nil) + if tt.args.auth != "" { + req.Header.Set("Authorization", tt.args.auth) + } + + assert.Equal(t, tt.want.accessKey, httputils.ExtractAccessKeyFromRequest(req)) + assert.Equal(t, tt.want.service, httputils.ExtractServiceFromRequest(req)) + assert.Equal(t, tt.want.region, httputils.ExtractRegionFromRequest(req, "us-east-1")) + }) + } +} + func TestWriteS3ErrorResponse(t *testing.T) { t.Parallel() diff --git a/pkgs/httputils/pool.go b/pkgs/httputils/pool.go index d830dfd7f6..3a037fd919 100644 --- a/pkgs/httputils/pool.go +++ b/pkgs/httputils/pool.go @@ -1,3 +1,4 @@ +// Package httputils provides reusable HTTP utility components. package httputils import ( diff --git a/pkgs/httputils/pool_test.go b/pkgs/httputils/pool_test.go index a781ab7785..c6916ac08c 100644 --- a/pkgs/httputils/pool_test.go +++ b/pkgs/httputils/pool_test.go @@ -4,6 +4,7 @@ import ( "bytes" "crypto/md5" "crypto/sha256" + "hash" "hash/crc32" "testing" @@ -59,59 +60,131 @@ func TestBufferPool(t *testing.T) { func TestHasherPools(t *testing.T) { t.Parallel() + type poolArgs struct { + acquire func() hash.Hash + release func(hash.Hash) + compute func([]byte) []byte + payload []byte + } + + type poolWant struct { + payloadSum []byte + emptySum []byte + } + payload := []byte("gopherstack fast hashing test payload") tests := []struct { - getHasher func() any - compute func([]byte) []byte - name string + name string + args poolArgs + want poolWant }{ { name: "crc32", - getHasher: func() any { - return httputils.GetCRC32() + args: poolArgs{ + payload: payload, + acquire: func() hash.Hash { + return httputils.GetCRC32() + }, + release: func(h hash.Hash) { + if h32, ok := h.(hash.Hash32); ok { + httputils.PutCRC32(h32) + } + }, + compute: func(b []byte) []byte { + h := crc32.NewIEEE() + h.Write(b) + + return h.Sum(nil) + }, }, - compute: func(b []byte) []byte { - h := crc32.NewIEEE() - h.Write(b) - - return h.Sum(nil) + want: poolWant{ + payloadSum: func() []byte { + h := crc32.NewIEEE() + h.Write(payload) + + return h.Sum(nil) + }(), + emptySum: crc32.NewIEEE().Sum(nil), }, }, { name: "crc32c", - getHasher: func() any { - return httputils.GetCRC32C() + args: poolArgs{ + payload: payload, + acquire: func() hash.Hash { + return httputils.GetCRC32C() + }, + release: func(h hash.Hash) { + if h32, ok := h.(hash.Hash32); ok { + httputils.PutCRC32C(h32) + } + }, + compute: func(b []byte) []byte { + h := crc32.New(crc32.MakeTable(crc32.Castagnoli)) + h.Write(b) + + return h.Sum(nil) + }, }, - compute: func(b []byte) []byte { - h := crc32.New(crc32.MakeTable(crc32.Castagnoli)) - h.Write(b) - - return h.Sum(nil) + want: poolWant{ + payloadSum: func() []byte { + h := crc32.New(crc32.MakeTable(crc32.Castagnoli)) + h.Write(payload) + + return h.Sum(nil) + }(), + emptySum: crc32.New(crc32.MakeTable(crc32.Castagnoli)).Sum(nil), }, }, { name: "sha256", - getHasher: func() any { - return httputils.GetSHA256() + args: poolArgs{ + payload: payload, + acquire: httputils.GetSHA256, + release: func(h hash.Hash) { + httputils.PutSHA256(h) + }, + compute: func(b []byte) []byte { + h := sha256.New() + h.Write(b) + + return h.Sum(nil) + }, }, - compute: func(b []byte) []byte { - h := sha256.New() - h.Write(b) - - return h.Sum(nil) + want: poolWant{ + payloadSum: func() []byte { + h := sha256.New() + h.Write(payload) + + return h.Sum(nil) + }(), + emptySum: sha256.New().Sum(nil), }, }, { name: "md5", - getHasher: func() any { - return httputils.GetMD5() + args: poolArgs{ + payload: payload, + acquire: httputils.GetMD5, + release: func(h hash.Hash) { + httputils.PutMD5(h) + }, + compute: func(b []byte) []byte { + h := md5.New() + h.Write(b) + + return h.Sum(nil) + }, }, - compute: func(b []byte) []byte { - h := md5.New() - h.Write(b) - - return h.Sum(nil) + want: poolWant{ + payloadSum: func() []byte { + h := md5.New() + h.Write(payload) + + return h.Sum(nil) + }(), + emptySum: md5.New().Sum(nil), }, }, } @@ -120,38 +193,18 @@ func TestHasherPools(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - expected := tt.compute(payload) - - h1 := httputils.GetCRC32() - h1.Write(payload) - sum1 := h1.Sum(nil) - httputils.PutCRC32(h1) - - h2 := httputils.GetCRC32C() - h2.Write(payload) - sum2 := h2.Sum(nil) - httputils.PutCRC32C(h2) - - h3 := httputils.GetSHA256() - h3.Write(payload) - sum3 := h3.Sum(nil) - httputils.PutSHA256(h3) - - h4 := httputils.GetMD5() - h4.Write(payload) - sum4 := h4.Sum(nil) - httputils.PutMD5(h4) - - switch tt.name { - case "crc32": - assert.Equal(t, expected, sum1) - case "crc32c": - assert.Equal(t, expected, sum2) - case "sha256": - assert.Equal(t, expected, sum3) - case "md5": - assert.Equal(t, expected, sum4) - } + h := tt.args.acquire() + require.NotNil(t, h) + _, err := h.Write(tt.args.payload) + require.NoError(t, err) + assert.Equal(t, tt.want.payloadSum, h.Sum(nil)) + + tt.args.release(h) + + h2 := tt.args.acquire() + require.NotNil(t, h2) + assert.Equal(t, tt.want.emptySum, h2.Sum(nil)) + tt.args.release(h2) }) } } diff --git a/pkgs/httputils/sigv4.go b/pkgs/httputils/sigv4.go index 04ab81d5b3..19570617b5 100644 --- a/pkgs/httputils/sigv4.go +++ b/pkgs/httputils/sigv4.go @@ -193,13 +193,13 @@ func parseAuthorizationHeader(auth string) (parsedAuthHeader, *SigV4Error) { } // Credential scope: AKID/date/region/service/aws4_request. - scope := strings.Split(p.credential, "/") - if len(scope) < minSigV4CredentialParts { + scope := parseValidSigV4Scope(p.credential) + if scope == nil { return p, malformed } - p.date = scope[1] - p.region = scope[2] + p.date = scope[sigV4DateIndex] + p.region = scope[sigV4RegionIndex] p.service = scope[sigV4ServiceIndex] sort.Strings(p.signedHeaders) diff --git a/pkgs/telemetry/memstats_test.go b/pkgs/telemetry/memstats_test.go index 324bb27f31..48d9be2890 100644 --- a/pkgs/telemetry/memstats_test.go +++ b/pkgs/telemetry/memstats_test.go @@ -5,10 +5,11 @@ import ( "net/http/httptest" "testing" - "github.com/blackbirdworks/gopherstack/pkgs/telemetry" "github.com/labstack/echo/v5" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/pkgs/telemetry" ) func TestMemoryStatsMiddleware(t *testing.T) { diff --git a/services/s3/interfaces.go b/services/s3/interfaces.go index 991f6c2bb5..10b7909c9e 100644 --- a/services/s3/interfaces.go +++ b/services/s3/interfaces.go @@ -257,6 +257,7 @@ type StorageBackend interface { GetObjectAttributes( ctx context.Context, bucket, key, versionID string, + maxParts, partNumberMarker int32, ) (*ObjectAttributes, error) RestoreObject(ctx context.Context, bucket, key string, days int) error RenameObject(ctx context.Context, bucket, sourceKey, targetKey string) error diff --git a/services/s3/object_ops_head.go b/services/s3/object_ops_head.go index 357deba386..b4443feae9 100644 --- a/services/s3/object_ops_head.go +++ b/services/s3/object_ops_head.go @@ -5,6 +5,7 @@ import ( "encoding/xml" "errors" "net/http" + "strconv" "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/s3" @@ -151,7 +152,7 @@ func (h *S3Handler) writeHeadObjectResponse( // ObjectSize carries no omitempty: a legitimate 0-byte object must still emit // 0 so the SDK populates its *int64 field. type objectAttributesResult struct { - XMLName xml.Name `xml:"GetObjectAttributesResult"` + XMLName xml.Name `xml:"GetObjectAttributesResponse"` Xmlns string `xml:"xmlns,attr"` ETag string `xml:"ETag,omitempty"` Checksum *attrsChecksumElem `xml:"Checksum,omitempty"` @@ -246,6 +247,32 @@ func buildObjectPartElem(p StoredObjectPart) objectPartElem { return pe } +func parseObjectAttributesPagination(r *http.Request) (int32, int32) { + maxParts := int32(defaultMaxPartsCount) + if mpStr := r.URL.Query().Get("max-parts"); mpStr != "" { + if mp, err := strconv.ParseInt(mpStr, 10, 32); err == nil && mp > 0 { + maxParts = int32(mp) // #nosec G115 + } + } else if mpHdr := r.Header.Get("X-Amz-Max-Parts"); mpHdr != "" { + if mp, err := strconv.ParseInt(mpHdr, 10, 32); err == nil && mp > 0 { + maxParts = int32(mp) // #nosec G115 + } + } + + partMarker := int32(0) + if pnmStr := r.URL.Query().Get("part-number-marker"); pnmStr != "" { + if pnm, err := strconv.ParseInt(pnmStr, 10, 32); err == nil && pnm >= 0 { + partMarker = int32(pnm) // #nosec G115 + } + } else if pnmHdr := r.Header.Get("X-Amz-Part-Number-Marker"); pnmHdr != "" { + if pnm, err := strconv.ParseInt(pnmHdr, 10, 32); err == nil && pnm >= 0 { + partMarker = int32(pnm) // #nosec G115 + } + } + + return maxParts, partMarker +} + // handleGetObjectAttributes handles GET /{bucket}/{key}?attributes. // The X-Amz-Object-Attributes header lists which attributes the caller wants; // we always return the full set we support (ETag, ObjectSize, StorageClass, Checksum, ObjectParts). @@ -267,8 +294,9 @@ func (h *S3Handler) handleGetObjectAttributes( } versionID := r.URL.Query().Get("versionId") + maxParts, partMarker := parseObjectAttributesPagination(r) - attrs, err := h.Backend.GetObjectAttributes(ctx, bucket, key, versionID) + attrs, err := h.Backend.GetObjectAttributes(ctx, bucket, key, versionID, maxParts, partMarker) if err != nil { WriteError(ctx, w, r, err) diff --git a/services/s3/object_ops_head_test.go b/services/s3/object_ops_head_test.go index a4982019a1..783df787d9 100644 --- a/services/s3/object_ops_head_test.go +++ b/services/s3/object_ops_head_test.go @@ -133,14 +133,18 @@ func TestHandler_GetObjectAttributes_MultipartParts(t *testing.T) { t.Parallel() type headArgs struct { - bucket string - key string - partCount int + bucket string + key string + queryString string + partCount int } type headWant struct { - wantStatus int - wantParts bool + wantStatus int + wantParts bool + wantTruncated bool + wantNextMarker int + wantVisiblePartCount int } tests := []struct { @@ -156,8 +160,39 @@ func TestHandler_GetObjectAttributes_MultipartParts(t *testing.T) { partCount: 2, }, want: headWant{ - wantParts: true, - wantStatus: http.StatusOK, + wantParts: true, + wantStatus: http.StatusOK, + wantVisiblePartCount: 2, + }, + }, + { + name: "multipart_object_pagination_max_parts", + args: headArgs{ + bucket: "mp-bkt-pag", + key: "mp-obj-pag", + queryString: "&max-parts=1", + partCount: 2, + }, + want: headWant{ + wantParts: true, + wantTruncated: true, + wantNextMarker: 1, + wantVisiblePartCount: 1, + wantStatus: http.StatusOK, + }, + }, + { + name: "multipart_object_pagination_marker", + args: headArgs{ + bucket: "mp-bkt-marker", + key: "mp-obj-marker", + queryString: "&part-number-marker=1", + partCount: 2, + }, + want: headWant{ + wantParts: true, + wantVisiblePartCount: 1, + wantStatus: http.StatusOK, }, }, { @@ -218,19 +253,26 @@ func TestHandler_GetObjectAttributes_MultipartParts(t *testing.T) { mustPutObject(t, backend, tt.args.bucket, tt.args.key, []byte("single-put-data")) } - req := httptest.NewRequest(http.MethodGet, "/"+tt.args.bucket+"/"+tt.args.key+"?attributes", nil) + targetURL := "/" + tt.args.bucket + "/" + tt.args.key + "?attributes" + tt.args.queryString + req := httptest.NewRequest(http.MethodGet, targetURL, nil) rec := httptest.NewRecorder() serveS3Handler(handler, rec, req) require.Equal(t, tt.want.wantStatus, rec.Code) body := rec.Body.String() - assert.Contains(t, body, "") assert.Contains(t, body, fmt.Sprintf("%d", tt.args.partCount)) - for i := 1; i <= tt.args.partCount; i++ { - assert.Contains(t, body, fmt.Sprintf("%d", i)) + if tt.want.wantTruncated { + assert.Contains(t, body, "true") + assert.Contains( + t, + body, + fmt.Sprintf("%d", tt.want.wantNextMarker), + ) } + assert.Equal(t, tt.want.wantVisiblePartCount, strings.Count(body, "")) } else { assert.NotContains(t, body, "") } diff --git a/services/s3/objects.go b/services/s3/objects.go index 599d0c9602..9fa5272589 100644 --- a/services/s3/objects.go +++ b/services/s3/objects.go @@ -69,11 +69,72 @@ type ObjectPartsAttributes struct { IsTruncated bool } +func extractObjectChecksums(ver *StoredObjectVersion) map[string]string { + c := make(map[string]string) + if ver.ChecksumSHA1 != nil { + c["ChecksumSHA1"] = *ver.ChecksumSHA1 + } + if ver.ChecksumSHA256 != nil { + c["ChecksumSHA256"] = *ver.ChecksumSHA256 + } + if ver.ChecksumCRC32 != nil { + c["ChecksumCRC32"] = *ver.ChecksumCRC32 + } + if ver.ChecksumCRC32C != nil { + c["ChecksumCRC32C"] = *ver.ChecksumCRC32C + } + if ver.ChecksumCRC64NVME != nil { + c["ChecksumCRC64NVME"] = *ver.ChecksumCRC64NVME + } + + return c +} + +func buildObjectPartsAttributes( + parts []StoredObjectPart, + maxParts, partNumberMarker int32, +) *ObjectPartsAttributes { + if len(parts) == 0 { + return nil + } + if maxParts <= 0 { + maxParts = defaultMaxPartsCount + } + + var filtered []StoredObjectPart + for _, p := range parts { + if p.PartNumber > partNumberMarker { + filtered = append(filtered, p) + } + } + + totalParts := int32(len(parts)) // #nosec G115 + isTruncated := false + var nextMarker int32 + if int32(len(filtered)) > maxParts { // #nosec G115 + isTruncated = true + filtered = filtered[:maxParts] + if len(filtered) > 0 { + nextMarker = filtered[len(filtered)-1].PartNumber + } + } + + return &ObjectPartsAttributes{ + Parts: filtered, + TotalPartsCount: totalParts, + PartNumberMarker: partNumberMarker, + NextPartNumberMarker: nextMarker, + MaxParts: maxParts, + IsTruncated: isTruncated, + } +} + // GetObjectAttributes returns selected attributes for the latest version of an object. // versionID may be empty to select the latest version. func (b *InMemoryBackend) GetObjectAttributes( _ context.Context, bucketName, key, versionID string, + maxParts, partNumberMarker int32, ) (*ObjectAttributes, error) { b.mu.RLock("GetObjectAttributes") bucket, err := b.getBucket(bucketName) @@ -84,10 +145,10 @@ func (b *InMemoryBackend) GetObjectAttributes( } bucket.mu.RLock("GetObjectAttributes") - defer bucket.mu.RUnlock() + obj, exists := bucket.Objects[key] + bucket.mu.RUnlock() - obj, ok := bucket.Objects[key] - if !ok { + if !exists { return nil, ErrNoSuchKey } @@ -104,44 +165,14 @@ func (b *InMemoryBackend) GetObjectAttributes( return nil, ErrNoSuchKey } - out := &ObjectAttributes{ + return &ObjectAttributes{ ETag: ver.ETag, ObjectSize: ver.Size, StorageClass: ver.StorageClass, LastModified: ver.LastModified, - Checksum: map[string]string{}, - } - - if out.StorageClass == "" { - out.StorageClass = storageStandard - } - - if ver.ChecksumSHA1 != nil { - out.Checksum["ChecksumSHA1"] = *ver.ChecksumSHA1 - } - if ver.ChecksumSHA256 != nil { - out.Checksum["ChecksumSHA256"] = *ver.ChecksumSHA256 - } - if ver.ChecksumCRC32 != nil { - out.Checksum["ChecksumCRC32"] = *ver.ChecksumCRC32 - } - if ver.ChecksumCRC32C != nil { - out.Checksum["ChecksumCRC32C"] = *ver.ChecksumCRC32C - } - if ver.ChecksumCRC64NVME != nil { - out.Checksum["ChecksumCRC64NVME"] = *ver.ChecksumCRC64NVME - } - - if len(ver.Parts) > 0 { - out.Parts = &ObjectPartsAttributes{ - Parts: ver.Parts, - TotalPartsCount: int32(len(ver.Parts)), // #nosec G115 - MaxParts: defaultMaxPartsCount, - IsTruncated: false, - } - } - - return out, nil + Checksum: extractObjectChecksums(ver), + Parts: buildObjectPartsAttributes(ver.Parts, maxParts, partNumberMarker), + }, nil } // RestoreObject marks the latest object version as restored for the given duration.