From ceca736c2f8b3716b444de79bee3cd459acb3899 Mon Sep 17 00:00:00 2001 From: mayor Date: Wed, 10 Jun 2026 14:47:17 -0500 Subject: [PATCH 1/6] fix(parity): close resource leaks across services (bucket D) Address the confirmed "Resource leaks" findings from the full-surface parity audit. Each fix releases state on the resource's delete path or bounds growth with a cap/lazy-eviction so the optional janitor is not the only thing preventing unbounded growth. - cloudwatch: DeleteAlarms now drops the per-alarm history (alarmHistory was unbounded across deleted alarms). - route53: DeleteHealthCheck releases its handler-level tags (only hosted-zone deletes were cleaning up before). - pinpoint: DeleteApp purges all per-app state (events, endpoints, channels, campaign/segment versions, activities, journey runs, campaigns/segments/journeys); PutEvents caps retained events per app. - mwaa: PublishMetrics trims into a right-sized slice so the dropped prefix is GC'd instead of pinned by an oversized backing array. - apprunner: per-service operation history is capped (was append-only). - sts: AssumeRole/etc. opportunistically evict expired sessions above a threshold so b.sessions stays bounded without the janitor. - dynamodb: ShardIteratorStore.Put sweeps expired iterators once it grows past a threshold. - ssm: SendCommand prunes aged-out commands/invocations on the write path. - ecr: layer uploads carry a CreatedAt and abandoned ones are pruned on InitiateLayerUpload; UploadLayerPart refreshes activity. Adds table-driven tests per service proving the resource is released (map size returns to baseline after delete; caps hold; expired entries evicted) with -race where goroutines/timers are involved. Co-Authored-By: Claude Opus 4.8 --- services/apprunner/backend.go | 14 +++++ services/apprunner/export_test.go | 17 ++++++ services/apprunner/leak_test.go | 47 ++++++++++++++++ services/cloudwatch/backend.go | 3 ++ services/cloudwatch/export_test.go | 9 ++++ services/cloudwatch/leak_test.go | 55 +++++++++++++++++++ services/dynamodb/accuracy_audit.go | 18 ++++++- services/dynamodb/export_test.go | 16 ++++++ services/dynamodb/leak_test.go | 54 +++++++++++++++++++ services/ecr/backend.go | 23 +++++++- services/ecr/export_test.go | 17 ++++++ services/ecr/leak_test.go | 80 +++++++++++++++++++++++++++ services/mwaa/backend.go | 9 +++- services/mwaa/export_test.go | 10 ++++ services/mwaa/leak_test.go | 49 +++++++++++++++++ services/pinpoint/backend.go | 54 +++++++++++++++++++ services/pinpoint/backend_ops.go | 13 +++++ services/pinpoint/export_test.go | 69 ++++++++++++++++++++++++ services/pinpoint/leak_test.go | 84 +++++++++++++++++++++++++++++ services/route53/export_test.go | 9 ++++ services/route53/handler.go | 4 ++ services/route53/leak_test.go | 62 +++++++++++++++++++++ services/ssm/backend.go | 22 ++++++++ services/ssm/leak_test.go | 68 +++++++++++++++++++++++ services/sts/backend.go | 68 +++++++++++++---------- services/sts/export_test.go | 3 ++ services/sts/leak_test.go | 70 ++++++++++++++++++++++++ 27 files changed, 914 insertions(+), 33 deletions(-) create mode 100644 services/apprunner/leak_test.go create mode 100644 services/cloudwatch/leak_test.go create mode 100644 services/dynamodb/leak_test.go create mode 100644 services/ecr/leak_test.go create mode 100644 services/mwaa/leak_test.go create mode 100644 services/pinpoint/export_test.go create mode 100644 services/pinpoint/leak_test.go create mode 100644 services/route53/leak_test.go create mode 100644 services/ssm/leak_test.go create mode 100644 services/sts/leak_test.go diff --git a/services/apprunner/backend.go b/services/apprunner/backend.go index c11d82cb7..8e6aa8249 100644 --- a/services/apprunner/backend.go +++ b/services/apprunner/backend.go @@ -34,6 +34,11 @@ const ( opStatusSucceeded = "SUCCEEDED" + // maxOperationsPerService bounds the per-service operation history so a + // long-lived service that is repeatedly updated/paused/resumed cannot grow + // svc.Operations without limit. ListOperations returns the most recent ones. + maxOperationsPerService = 200 + defaultMaxResults = 20 defaultCPU = "1 vCPU" defaultMemory = "2 GB" @@ -439,6 +444,15 @@ func (b *InMemoryBackend) addOperation(svc *storedService, opType string) { EndedAt: now, } svc.Operations = append(svc.Operations, op) + + // Retain only the most recent maxOperationsPerService entries. Copy into a + // fresh slice so the dropped prefix is released for GC rather than pinned by + // the original backing array. + if len(svc.Operations) > maxOperationsPerService { + trimmed := make([]*storedOperation, maxOperationsPerService) + copy(trimmed, svc.Operations[len(svc.Operations)-maxOperationsPerService:]) + svc.Operations = trimmed + } } // CreateService creates a new App Runner service. diff --git a/services/apprunner/export_test.go b/services/apprunner/export_test.go index f47904ed0..a3c1a9a21 100644 --- a/services/apprunner/export_test.go +++ b/services/apprunner/export_test.go @@ -12,3 +12,20 @@ func ServiceCount(b *InMemoryBackend) int { func HandlerOpsLen(h *Handler) int { return len(h.GetSupportedOperations()) } + +// ServiceOperationStats returns the length and capacity of a service's operation +// history slice, for leak-detection tests. +func ServiceOperationStats(b *InMemoryBackend, serviceArn string) (int, int) { + b.mu.RLock("ServiceOperationStats") + defer b.mu.RUnlock() + + svc, ok := b.services[serviceArn] + if !ok { + return 0, 0 + } + + return len(svc.Operations), cap(svc.Operations) +} + +// MaxOperationsPerService exposes the per-service operation cap for tests. +const MaxOperationsPerService = maxOperationsPerService diff --git a/services/apprunner/leak_test.go b/services/apprunner/leak_test.go new file mode 100644 index 000000000..ea5f0ceff --- /dev/null +++ b/services/apprunner/leak_test.go @@ -0,0 +1,47 @@ +package apprunner_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/apprunner" +) + +// TestAddOperation_CapsPerServiceHistory verifies that a service's operation +// history is bounded so repeatedly updating a long-lived service cannot grow +// svc.Operations without limit. +func TestAddOperation_CapsPerServiceHistory(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + updates int + }{ + {name: "just over cap", updates: apprunner.MaxOperationsPerService + 10}, + {name: "far over cap", updates: apprunner.MaxOperationsPerService * 3}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + b := apprunner.NewInMemoryBackend("123456789012", "us-east-1") + + svc, err := b.CreateService("svc", "", "", "public.ecr.aws/x/y:latest", nil) + require.NoError(t, err) + + for range tc.updates { + _, err = b.UpdateService(svc.ServiceArn, "2 vCPU", "", "") + require.NoError(t, err) + } + + length, capacity := apprunner.ServiceOperationStats(b, svc.ServiceArn) + + require.LessOrEqual(t, length, apprunner.MaxOperationsPerService, + "per-service operation history must be capped") + require.LessOrEqual(t, capacity, apprunner.MaxOperationsPerService, + "trimmed operation slice must not retain an oversized backing array") + }) + } +} diff --git a/services/cloudwatch/backend.go b/services/cloudwatch/backend.go index 24ed8a990..d09d99b70 100644 --- a/services/cloudwatch/backend.go +++ b/services/cloudwatch/backend.go @@ -1418,6 +1418,9 @@ func (b *InMemoryBackend) DeleteAlarms(alarmNames []string) error { for _, name := range alarmNames { delete(b.alarms, name) delete(b.compositeAlarms, name) + // Release the per-alarm history so it cannot accumulate across the + // lifetime of the backend once the alarm itself is gone. + delete(b.alarmHistory, name) } return nil diff --git a/services/cloudwatch/export_test.go b/services/cloudwatch/export_test.go index 19233acbf..be572b902 100644 --- a/services/cloudwatch/export_test.go +++ b/services/cloudwatch/export_test.go @@ -80,3 +80,12 @@ func DimensionSetKeyForTest(dims []Dimension) string { func StreamAllowsMetricForTest(s *MetricStream, namespace, metricName string) bool { return streamAllowsMetric(s, namespace, metricName) } + +// AlarmHistoryKeyCountForTest returns the number of distinct alarm names that +// currently have retained history, for leak-detection tests. +func (b *InMemoryBackend) AlarmHistoryKeyCountForTest() int { + b.mu.RLock("AlarmHistoryKeyCountForTest") + defer b.mu.RUnlock() + + return len(b.alarmHistory) +} diff --git a/services/cloudwatch/leak_test.go b/services/cloudwatch/leak_test.go new file mode 100644 index 000000000..e04cdcb53 --- /dev/null +++ b/services/cloudwatch/leak_test.go @@ -0,0 +1,55 @@ +package cloudwatch_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/cloudwatch" +) + +// TestDeleteAlarms_ReleasesAlarmHistory verifies that deleting an alarm also +// drops its retained history, so alarmHistory cannot grow unbounded across the +// lifetime of the backend as alarms are created and deleted. +func TestDeleteAlarms_ReleasesAlarmHistory(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + alarmNames []string + }{ + {name: "single alarm", alarmNames: []string{"alarm-0"}}, + {name: "many alarms", alarmNames: []string{"alarm-0", "alarm-1", "alarm-2", "alarm-3", "alarm-4"}}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + b := cloudwatch.NewInMemoryBackendWithConfig("123456789012", "us-east-1") + ctx := context.Background() + + baseline := b.AlarmHistoryKeyCountForTest() + + for _, name := range tc.alarmNames { + require.NoError(t, b.PutMetricAlarm(&cloudwatch.MetricAlarm{ + AlarmName: name, + StateValue: "OK", + })) + // Force several state transitions so each alarm accrues history. + require.NoError(t, b.SetAlarmState(ctx, name, "ALARM", "r1", "")) + require.NoError(t, b.SetAlarmState(ctx, name, "OK", "r2", "")) + require.NoError(t, b.SetAlarmState(ctx, name, "ALARM", "r3", "")) + } + + require.Greater(t, b.AlarmHistoryKeyCountForTest(), baseline, + "history should have accumulated before delete") + + require.NoError(t, b.DeleteAlarms(tc.alarmNames)) + + require.Equal(t, baseline, b.AlarmHistoryKeyCountForTest(), + "alarm history must return to baseline after all alarms are deleted") + }) + } +} diff --git a/services/dynamodb/accuracy_audit.go b/services/dynamodb/accuracy_audit.go index a45b863f4..8a838a08b 100644 --- a/services/dynamodb/accuracy_audit.go +++ b/services/dynamodb/accuracy_audit.go @@ -39,6 +39,10 @@ const ( maxParallelScanSegments = 1_000_000 // shardIteratorTTL is how long a shard iterator token remains valid (15 min per AWS spec). shardIteratorTTL = 15 * time.Minute + // shardIteratorSweepThreshold is the entry count above which Put performs an + // inline sweep of expired tokens, bounding the store even when the janitor's + // Sweep() is not being called on a schedule. + shardIteratorSweepThreshold = 1024 // shardIteratorTokenLen is the number of random bytes used in each opaque iterator token. shardIteratorTokenLen = 16 // replicationOpDelete is the mutation op string for item deletion in global-table replication. @@ -473,11 +477,23 @@ func (s *ShardIteratorStore) Put(tableName string, startSeq int64) (string, erro return "", err } + now := time.Now() + s.mu.Lock() + // Opportunistically drop expired tokens once the store grows large so it + // stays bounded between scheduled Sweep() calls. The threshold keeps the + // common small-store case allocation- and scan-free. + if len(s.entries) >= shardIteratorSweepThreshold { + for tok, entry := range s.entries { + if now.After(entry.ExpiresAt) { + delete(s.entries, tok) + } + } + } s.entries[token] = &shardIteratorEntry{ TableName: tableName, StartSeq: startSeq, - ExpiresAt: time.Now().Add(shardIteratorTTL), + ExpiresAt: now.Add(shardIteratorTTL), } s.mu.Unlock() diff --git a/services/dynamodb/export_test.go b/services/dynamodb/export_test.go index 783dc5c87..1fdfd14b9 100644 --- a/services/dynamodb/export_test.go +++ b/services/dynamodb/export_test.go @@ -588,3 +588,19 @@ func ExtractExportARNForTest(out any) string { return v.ExportDescription.ExportArn } + +// ExpireAllShardIteratorsForTest backdates every entry's ExpiresAt so a +// subsequent Put can be observed to sweep them. Used to test opportunistic +// eviction without waiting for the real TTL to elapse. +func (s *ShardIteratorStore) ExpireAllShardIteratorsForTest() { + s.mu.Lock() + defer s.mu.Unlock() + + past := time.Now().Add(-time.Hour) + for _, entry := range s.entries { + entry.ExpiresAt = past + } +} + +// ShardIteratorSweepThresholdForTest exposes the inline-sweep threshold. +const ShardIteratorSweepThresholdForTest = shardIteratorSweepThreshold diff --git a/services/dynamodb/leak_test.go b/services/dynamodb/leak_test.go new file mode 100644 index 000000000..80939b543 --- /dev/null +++ b/services/dynamodb/leak_test.go @@ -0,0 +1,54 @@ +package dynamodb_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + ddb "github.com/blackbirdworks/gopherstack/services/dynamodb" +) + +// TestShardIteratorStore_PutEvictsExpiredAboveThreshold verifies that once the +// store grows past the sweep threshold, inserting a new iterator opportunistically +// drops expired entries — bounding the store even when Sweep() is never called. +func TestShardIteratorStore_PutEvictsExpiredAboveThreshold(t *testing.T) { + t.Parallel() + + store := ddb.NewShardIteratorStore() + + // Fill to the threshold so the next Put triggers the inline sweep. + threshold := ddb.ShardIteratorSweepThresholdForTest + for range threshold { + _, err := store.Put("table", 0) + require.NoError(t, err) + } + require.Equal(t, threshold, store.Size()) + + // Backdate everything, then insert one more entry. The expired entries must + // be swept, leaving only the freshly-inserted one. + store.ExpireAllShardIteratorsForTest() + + _, err := store.Put("table", 1) + require.NoError(t, err) + + require.Equal(t, 1, store.Size(), + "expired iterators must be evicted on Put once past the threshold") +} + +// TestShardIteratorStore_PutBelowThresholdKeepsEntries confirms the sweep is a +// no-op below the threshold so steady-state Puts stay cheap. +func TestShardIteratorStore_PutBelowThresholdKeepsEntries(t *testing.T) { + t.Parallel() + + store := ddb.NewShardIteratorStore() + + _, err := store.Put("table", 0) + require.NoError(t, err) + store.ExpireAllShardIteratorsForTest() + + _, err = store.Put("table", 1) + require.NoError(t, err) + + // Below threshold: no eager sweep, so the expired entry remains. + require.Equal(t, 2, store.Size()) +} diff --git a/services/ecr/backend.go b/services/ecr/backend.go index bd0128d3b..4fadbb999 100644 --- a/services/ecr/backend.go +++ b/services/ecr/backend.go @@ -17,6 +17,11 @@ import ( const ( layerUploadPartSize = 20 * 1024 * 1024 + // layerUploadTTL bounds how long an in-progress layer upload is retained + // before it is treated as abandoned and pruned. AWS expires unfinished + // uploads after a period of inactivity; this prevents the layerUploads map + // from leaking entries for uploads that are initiated but never completed. + layerUploadTTL = 24 * time.Hour scanTypeBasic = "BASIC" mutabilityMutable = "MUTABLE" mutabilityImmutable = "IMMUTABLE" @@ -360,6 +365,7 @@ type ImageTagMutabilityExclusionFilter struct { } type layerUploadState struct { + CreatedAt time.Time RepositoryName string Data []byte Size int64 @@ -976,8 +982,18 @@ func (b *InMemoryBackend) InitiateLayerUpload( return nil, fmt.Errorf("%w: %s", ErrRepositoryNotFound, repositoryName) } - uploadID := fmt.Sprintf("upload-%d", time.Now().UnixNano()) - b.layerUploads[uploadID] = &layerUploadState{RepositoryName: repositoryName} + now := time.Now() + + // Prune abandoned uploads (initiated but never completed) so the map cannot + // grow without bound on a long-lived registry. + for id, upload := range b.layerUploads { + if now.Sub(upload.CreatedAt) > layerUploadTTL { + delete(b.layerUploads, id) + } + } + + uploadID := fmt.Sprintf("upload-%d", now.UnixNano()) + b.layerUploads[uploadID] = &layerUploadState{RepositoryName: repositoryName, CreatedAt: now} return &LayerUploadInitiation{PartSize: layerUploadPartSize, UploadID: uploadID}, nil } @@ -1002,6 +1018,9 @@ func (b *InMemoryBackend) UploadLayerPart(ctx context.Context, //nolint:revive / upload.Data = append(upload.Data, blob...) upload.Size = int64(len(upload.Data)) + // Refresh the activity timestamp so an in-progress multi-part upload is not + // pruned as abandoned while parts are still arriving. + upload.CreatedAt = time.Now() if lastByte < 0 && len(blob) > 0 { lastByte = upload.Size - 1 diff --git a/services/ecr/export_test.go b/services/ecr/export_test.go index 6f37e30b4..7b957fe3e 100644 --- a/services/ecr/export_test.go +++ b/services/ecr/export_test.go @@ -1,5 +1,7 @@ package ecr +import "time" + // CreateRepoInternal creates a minimal repository entry directly for testing. // Use this alongside AddImageInternal when a test needs images in a repo that // also satisfies the repo-existence check. @@ -87,6 +89,21 @@ func (b *InMemoryBackend) LayerUploadCount() int { return len(b.layerUploads) } +// AgeAllLayerUploadsForTest backdates every in-progress layer upload's +// CreatedAt by the given duration so abandoned-upload pruning can be exercised +// without waiting for the real TTL. +func (b *InMemoryBackend) AgeAllLayerUploadsForTest(d time.Duration) { + b.mu.Lock("AgeAllLayerUploadsForTest") + defer b.mu.Unlock() + + for _, upload := range b.layerUploads { + upload.CreatedAt = upload.CreatedAt.Add(-d) + } +} + +// LayerUploadTTLForTest exposes the abandoned-upload TTL for tests. +const LayerUploadTTLForTest = layerUploadTTL + // TagIndexCount returns the total number of tag→digest entries across all repos. func (b *InMemoryBackend) TagIndexCount() int { b.mu.RLock("TagIndexCount") diff --git a/services/ecr/leak_test.go b/services/ecr/leak_test.go new file mode 100644 index 000000000..1d635ef2b --- /dev/null +++ b/services/ecr/leak_test.go @@ -0,0 +1,80 @@ +package ecr_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/ecr" +) + +// TestInitiateLayerUpload_PrunesAbandonedUploads verifies that initiating a new +// layer upload prunes uploads that were started but never completed and have +// aged past the TTL, bounding the layerUploads map on a long-lived registry. +func TestInitiateLayerUpload_PrunesAbandonedUploads(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + abandoned int + }{ + {name: "one abandoned", abandoned: 1}, + {name: "many abandoned", abandoned: 6}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + ctx := context.Background() + b := ecr.NewInMemoryBackend("123456789012", "us-east-1", "ecr.local") + + _, err := b.CreateRepository(ctx, "repo", "MUTABLE", false, "", "") + require.NoError(t, err) + + for range tc.abandoned { + _, err = b.InitiateLayerUpload(ctx, "repo") + require.NoError(t, err) + } + require.Equal(t, tc.abandoned, b.LayerUploadCount()) + + // Age the abandoned uploads past the TTL, then start a new one. The + // stale uploads must be pruned, leaving only the fresh session. + b.AgeAllLayerUploadsForTest(ecr.LayerUploadTTLForTest + 1) + + _, err = b.InitiateLayerUpload(ctx, "repo") + require.NoError(t, err) + + require.Equal(t, 1, b.LayerUploadCount(), + "abandoned layer uploads must be pruned on InitiateLayerUpload") + }) + } +} + +// TestUploadLayerPart_KeepsActiveUploadAlive ensures an in-progress multi-part +// upload is not pruned as abandoned while parts are still arriving. +func TestUploadLayerPart_KeepsActiveUploadAlive(t *testing.T) { + t.Parallel() + + ctx := context.Background() + b := ecr.NewInMemoryBackend("123456789012", "us-east-1", "ecr.local") + + _, err := b.CreateRepository(ctx, "repo", "MUTABLE", false, "", "") + require.NoError(t, err) + + init, err := b.InitiateLayerUpload(ctx, "repo") + require.NoError(t, err) + + // Age the upload, but then send a part — activity must refresh CreatedAt. + b.AgeAllLayerUploadsForTest(ecr.LayerUploadTTLForTest + 1) + _, err = b.UploadLayerPart(ctx, "repo", init.UploadID, 0, -1, []byte("data")) + require.NoError(t, err) + + // A subsequent initiate must NOT prune the refreshed upload. + _, err = b.InitiateLayerUpload(ctx, "repo") + require.NoError(t, err) + + require.Equal(t, 2, b.LayerUploadCount(), + "an actively-uploading session must survive pruning") +} diff --git a/services/mwaa/backend.go b/services/mwaa/backend.go index 7326fc79b..275221826 100644 --- a/services/mwaa/backend.go +++ b/services/mwaa/backend.go @@ -1139,8 +1139,13 @@ func (b *InMemoryBackend) PublishMetrics(envName string, req *publishMetricsRequ b.metrics[envName] = append(b.metrics[envName], req.MetricData...) - if len(b.metrics[envName]) > maxMetricsPerEnv { - b.metrics[envName] = b.metrics[envName][len(b.metrics[envName])-maxMetricsPerEnv:] + if data := b.metrics[envName]; len(data) > maxMetricsPerEnv { + // Copy the surviving tail into a right-sized slice so the trimmed-off + // prefix is released for GC instead of being pinned by an oversized + // backing array. + trimmed := make([]MetricDatum, maxMetricsPerEnv) + copy(trimmed, data[len(data)-maxMetricsPerEnv:]) + b.metrics[envName] = trimmed } return nil diff --git a/services/mwaa/export_test.go b/services/mwaa/export_test.go index 162f82ddf..4ca439921 100644 --- a/services/mwaa/export_test.go +++ b/services/mwaa/export_test.go @@ -16,6 +16,16 @@ func MetricsCount(b *InMemoryBackend, envName string) int { return len(b.metrics[envName]) } +// MetricsCapacity returns the capacity of the backing slice for an +// environment's metrics. Used to verify trimming does not retain an oversized +// backing array (a memory leak even though len() is capped). +func MetricsCapacity(b *InMemoryBackend, envName string) int { + b.mu.RLock("MetricsCapacity") + defer b.mu.RUnlock() + + return cap(b.metrics[envName]) +} + // ARNIndexSize returns the number of entries in the ARN index. func ARNIndexSize(b *InMemoryBackend) int { b.mu.RLock("ARNIndexSize") diff --git a/services/mwaa/leak_test.go b/services/mwaa/leak_test.go new file mode 100644 index 000000000..016af2a61 --- /dev/null +++ b/services/mwaa/leak_test.go @@ -0,0 +1,49 @@ +package mwaa_test + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/mwaa" +) + +// TestPublishMetrics_TrimDoesNotRetainOversizedArray verifies that capping the +// per-environment metrics slice copies the survivors into a right-sized backing +// array, so the trimmed-off prefix is released for GC rather than pinned. +func TestPublishMetrics_TrimDoesNotRetainOversizedArray(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + publish int + }{ + {name: "just over cap", publish: 1200}, + {name: "far over cap", publish: 5000}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + b := mwaa.NewInMemoryBackend(testRegion, testAccountID) + _, err := b.CreateEnvironment(testRegion, testAccountID, "leak-env", newCreateReq()) + require.NoError(t, err) + + data := make([]mwaa.ExportedMetricDatum, tc.publish) + for i := range data { + data[i] = mwaa.ExportedMetricDatum{MetricName: fmt.Sprintf("M%d", i)} + } + require.NoError(t, b.PublishMetrics("leak-env", + &mwaa.ExportedPublishMetricsRequest{MetricData: data})) + + // len is capped... + assert.Equal(t, 1000, mwaa.MetricsCount(b, "leak-env")) + // ...and the backing array is right-sized, not the oversized original. + assert.Equal(t, 1000, mwaa.MetricsCapacity(b, "leak-env"), + "backing array must not retain trimmed-off capacity") + }) + } +} diff --git a/services/pinpoint/backend.go b/services/pinpoint/backend.go index b0c2b3be4..4bd55ff09 100644 --- a/services/pinpoint/backend.go +++ b/services/pinpoint/backend.go @@ -4,6 +4,7 @@ import ( "fmt" "maps" "sort" + "strings" "github.com/google/uuid" @@ -248,9 +249,62 @@ func (b *InMemoryBackend) DeleteApp(appID string) (*App, error) { delete(b.apps, appID) delete(b.arnIndex, app.ARN) + b.purgeAppStateLocked(appID) + return cloneApp(app), nil } +// purgeAppStateLocked releases every piece of per-application state so that +// deleting an app does not leak entries in the various app-scoped maps. The +// backend mutex must be held by the caller. +// +// App-scoped maps fall into two shapes: those keyed by a bare application ID +// (appEvents, eventStreams, otpCodes, appSettings, sentMessages) and those +// keyed by an "/" composite (endpoints, channels, +// campaignVersions, segmentVersions, campaignActivities, journeyRuns). The +// campaigns/segments/journeys maps are keyed by child ID but carry the owning +// ApplicationID, so they are filtered by that field. +func (b *InMemoryBackend) purgeAppStateLocked(appID string) { + delete(b.appEvents, appID) + delete(b.eventStreams, appID) + delete(b.otpCodes, appID) + delete(b.appSettings, appID) + delete(b.sentMessages, appID) + + prefix := appID + "/" + deletePrefixed(b.endpoints, prefix) + deletePrefixed(b.channels, prefix) + deletePrefixed(b.campaignVersions, prefix) + deletePrefixed(b.segmentVersions, prefix) + deletePrefixed(b.campaignActivities, prefix) + deletePrefixed(b.journeyRuns, prefix) + + for id, c := range b.campaigns { + if c != nil && c.ApplicationID == appID { + delete(b.campaigns, id) + } + } + for id, s := range b.segments { + if s != nil && s.ApplicationID == appID { + delete(b.segments, id) + } + } + for id, j := range b.journeys { + if j != nil && j.ApplicationID == appID { + delete(b.journeys, id) + } + } +} + +// deletePrefixed removes every entry from m whose key starts with prefix. +func deletePrefixed[V any](m map[string]V, prefix string) { + for k := range m { + if strings.HasPrefix(k, prefix) { + delete(m, k) + } + } +} + // GetApps returns all Pinpoint applications sorted by name. func (b *InMemoryBackend) GetApps() ([]*App, error) { b.mu.RLock("GetApps") diff --git a/services/pinpoint/backend_ops.go b/services/pinpoint/backend_ops.go index 0da0a7dba..bad58f18e 100644 --- a/services/pinpoint/backend_ops.go +++ b/services/pinpoint/backend_ops.go @@ -31,6 +31,10 @@ const ( statusCodeOK = 200 deliveryStatusSuccessful = "SUCCESSFUL" + + // maxAppEvents caps the number of ingested events retained per application so + // PutEvents cannot grow the appEvents slice without bound on a long-lived app. + maxAppEvents = 10000 ) // ────────────────────────────────────────────────── @@ -1880,6 +1884,15 @@ func (b *InMemoryBackend) PutEvents(appID string, req putEventsRequest) (*events } } + // Bound retained events to the most recent maxAppEvents. Copy into a + // fresh slice so the trimmed-off prefix can be garbage collected rather + // than pinned by the original backing array. + if events := b.appEvents[appID]; len(events) > maxAppEvents { + trimmed := make([]storedPinpointEvent, maxAppEvents) + copy(trimmed, events[len(events)-maxAppEvents:]) + b.appEvents[appID] = trimmed + } + results[epID] = endpointItemResponse{EventsItemResponse: evResults} } diff --git a/services/pinpoint/export_test.go b/services/pinpoint/export_test.go new file mode 100644 index 000000000..d329db929 --- /dev/null +++ b/services/pinpoint/export_test.go @@ -0,0 +1,69 @@ +package pinpoint + +import "strconv" + +// MaxAppEvents exposes the per-app event cap for tests. +const MaxAppEvents = maxAppEvents + +// TotalPerAppEntries counts every entry across the per-application maps so a +// leak (an entry surviving DeleteApp) shows up as a non-zero delta from +// baseline in tests. +func TotalPerAppEntries(b *InMemoryBackend) int { + b.mu.RLock("TotalPerAppEntries") + defer b.mu.RUnlock() + + return len(b.appEvents) + len(b.eventStreams) + len(b.otpCodes) + + len(b.appSettings) + len(b.sentMessages) + + len(b.endpoints) + len(b.channels) + + len(b.campaignVersions) + len(b.segmentVersions) + + len(b.campaignActivities) + len(b.journeyRuns) + + len(b.campaigns) + len(b.segments) + len(b.journeys) +} + +// AppEventCount returns the number of retained events for an application. +func AppEventCount(b *InMemoryBackend, appID string) int { + b.mu.RLock("AppEventCount") + defer b.mu.RUnlock() + + return len(b.appEvents[appID]) +} + +// CreateCampaignForTest creates a campaign for the app using a minimal request, +// exercising the per-app campaign maps without exporting the request type. +func CreateCampaignForTest(b *InMemoryBackend, region, accountID, appID string) error { + _, err := b.CreateCampaign(region, accountID, appID, createCampaignRequest{}) + + return err +} + +// CreateSegmentForTest creates a segment for the app using a minimal request. +func CreateSegmentForTest(b *InMemoryBackend, region, accountID, appID string) error { + _, err := b.CreateSegment(region, accountID, appID, createSegmentRequest{}) + + return err +} + +// CreateJourneyForTest creates a journey for the app using a minimal request. +func CreateJourneyForTest(b *InMemoryBackend, region, accountID, appID string) error { + _, err := b.CreateJourney(region, accountID, appID, createJourneyRequest{}) + + return err +} + +// PutEventsForTest records n events for a single endpoint of the app, exercising +// the appEvents append/cap path without exporting the request type. +func PutEventsForTest(b *InMemoryBackend, appID string, n int) error { + events := make(map[string]eventItem, n) + for i := range n { + events[strconv.Itoa(i)] = eventItem{EventType: "custom"} + } + + var req putEventsRequest + req.EventsRequest.BatchItem = map[string]endpointEvents{ + "endpoint-1": {Events: events}, + } + + _, err := b.PutEvents(appID, req) + + return err +} diff --git a/services/pinpoint/leak_test.go b/services/pinpoint/leak_test.go new file mode 100644 index 000000000..5d2dda5da --- /dev/null +++ b/services/pinpoint/leak_test.go @@ -0,0 +1,84 @@ +package pinpoint_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/pinpoint" +) + +const ( + leakRegion = "us-east-1" + leakAccountID = "123456789012" +) + +// TestDeleteApp_ReleasesPerAppState verifies that deleting a Pinpoint app drops +// every piece of per-application state so the backing maps return to baseline. +func TestDeleteApp_ReleasesPerAppState(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + nEvents int + campaign bool + segment bool + journey bool + }{ + {name: "events only", nEvents: 5}, + {name: "events + campaign", nEvents: 3, campaign: true}, + {name: "full graph", nEvents: 4, campaign: true, segment: true, journey: true}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + b := pinpoint.NewInMemoryBackend(leakRegion, leakAccountID) + baseline := pinpoint.TotalPerAppEntries(b) + + app, err := b.CreateApp(leakRegion, leakAccountID, "leak-app", nil) + require.NoError(t, err) + + if tc.campaign { + require.NoError(t, pinpoint.CreateCampaignForTest(b, leakRegion, leakAccountID, app.ID)) + } + if tc.segment { + require.NoError(t, pinpoint.CreateSegmentForTest(b, leakRegion, leakAccountID, app.ID)) + } + if tc.journey { + require.NoError(t, pinpoint.CreateJourneyForTest(b, leakRegion, leakAccountID, app.ID)) + } + if tc.nEvents > 0 { + require.NoError(t, pinpoint.PutEventsForTest(b, app.ID, tc.nEvents)) + } + + require.Greater(t, pinpoint.TotalPerAppEntries(b), baseline, + "per-app state should have accumulated before delete") + + _, err = b.DeleteApp(app.ID) + require.NoError(t, err) + + require.Equal(t, baseline, pinpoint.TotalPerAppEntries(b), + "all per-app state must be released when the app is deleted") + }) + } +} + +// TestPutEvents_CapsAppEvents verifies that ingested events are bounded per app +// so the appEvents slice cannot grow without limit. +func TestPutEvents_CapsAppEvents(t *testing.T) { + t.Parallel() + + b := pinpoint.NewInMemoryBackend(leakRegion, leakAccountID) + app, err := b.CreateApp(leakRegion, leakAccountID, "cap-app", nil) + require.NoError(t, err) + + const batches = 3 + for range batches { + require.NoError(t, pinpoint.PutEventsForTest(b, app.ID, pinpoint.MaxAppEvents)) + } + + require.LessOrEqual(t, pinpoint.AppEventCount(b, app.ID), pinpoint.MaxAppEvents, + "retained events must not exceed the cap") +} diff --git a/services/route53/export_test.go b/services/route53/export_test.go index 85dd6065a..dba0738c1 100644 --- a/services/route53/export_test.go +++ b/services/route53/export_test.go @@ -81,3 +81,12 @@ func VPCAssociationCount(b *InMemoryBackend) int { func HandlerOpsLen(h *Handler) int { return len(h.GetSupportedOperations()) } + +// TagResourceCount returns the number of resource IDs that currently have +// handler-level tags. Used to verify tags are released on resource delete. +func TagResourceCount(h *Handler) int { + h.tagsMu.RLock("TagResourceCount") + defer h.tagsMu.RUnlock() + + return len(h.tags) +} diff --git a/services/route53/handler.go b/services/route53/handler.go index 629633680..55a1b59dd 100644 --- a/services/route53/handler.go +++ b/services/route53/handler.go @@ -1927,6 +1927,10 @@ func (h *Handler) deleteHealthCheck(c *echo.Context, path string) error { return handleBackendError(c, err) } + // Release any handler-level tags for this health check so the tags map + // cannot retain entries for resources that no longer exist. + h.deleteTagsForResource(id) + logger.Load(ctx).DebugContext(ctx, "Route53 DeleteHealthCheck", "id", id) return writeXML(c, http.StatusOK, xmlDeleteHealthCheckResponse{Xmlns: route53Namespace}) diff --git a/services/route53/leak_test.go b/services/route53/leak_test.go new file mode 100644 index 000000000..f2a362d2f --- /dev/null +++ b/services/route53/leak_test.go @@ -0,0 +1,62 @@ +package route53_test + +import ( + "net/http" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/route53" +) + +// TestDeleteHealthCheck_ReleasesTags verifies that deleting a health check +// drops its handler-level tags, so the tags map cannot retain entries for +// resources that no longer exist. +func TestDeleteHealthCheck_ReleasesTags(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + tagAdds string + wantAfter int + }{ + { + name: "tagged health check", + tagAdds: ` + + + envtest + +`, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + backend := route53.NewInMemoryBackend() + h := route53.NewHandler(backend) + + baseline := route53.TagResourceCount(h) + + hc, err := backend.CreateHealthCheck("ref-1", route53.HealthCheckConfig{ + Type: route53.HealthCheckTypeHTTP, + FullyQualifiedDomainName: "example.com", + Port: 80, + }) + require.NoError(t, err) + + tagRec := send(t, h, http.MethodPost, "/2013-04-01/tags/healthcheck/"+hc.ID, tc.tagAdds) + require.Equal(t, http.StatusOK, tagRec.Code) + require.Greater(t, route53.TagResourceCount(h), baseline, + "tag should be recorded before delete") + + delRec := send(t, h, http.MethodDelete, "/2013-04-01/healthcheck/"+hc.ID, "") + require.Equal(t, http.StatusOK, delRec.Code) + + require.Equal(t, baseline, route53.TagResourceCount(h), + "health-check tags must be released when the health check is deleted") + }) + } +} diff --git a/services/ssm/backend.go b/services/ssm/backend.go index 16a593646..a7b649cb9 100644 --- a/services/ssm/backend.go +++ b/services/ssm/backend.go @@ -324,6 +324,24 @@ func (b *InMemoryBackend) commandsStore(region string) map[string]Command { return b.commands[region] } +// expireCommandsLocked removes commands (and their invocations) in the region +// whose ExpiresAfter timestamp has passed. It mirrors the background janitor's +// sweep so commands are pruned on the write path even when the janitor is +// disabled or runs at a long interval. The backend mutex must be held. +func (b *InMemoryBackend) expireCommandsLocked(region string, now float64) { + commands := b.commands[region] + if commands == nil { + return + } + + for id, cmd := range commands { + if cmd.ExpiresAfter > 0 && cmd.ExpiresAfter < now { + delete(commands, id) + delete(b.commandInvocations[region], id) + } + } +} + func (b *InMemoryBackend) commandInvocationsStore(region string) map[string][]CommandInvocation { if b.commandInvocations[region] == nil { b.commandInvocations[region] = make(map[string][]CommandInvocation) @@ -1729,6 +1747,10 @@ func (b *InMemoryBackend) SendCommand(ctx context.Context, input *SendCommandInp now := UnixTimeFloat(time.Now()) cmdID := uuid.NewString() + // Prune any commands that have aged out so the commands/invocations maps do + // not grow unbounded between (or without) janitor runs. + b.expireCommandsLocked(region, now) + timeoutSecs := input.TimeoutSeconds if timeoutSecs == 0 { timeoutSecs = 3600 diff --git a/services/ssm/leak_test.go b/services/ssm/leak_test.go new file mode 100644 index 000000000..8413c3902 --- /dev/null +++ b/services/ssm/leak_test.go @@ -0,0 +1,68 @@ +package ssm_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/ssm" +) + +// TestSendCommand_PrunesExpiredCommands verifies that sending a command +// opportunistically prunes commands (and their invocations) that have aged out, +// so the commands/commandInvocations maps stay bounded even when the background +// janitor never runs. +func TestSendCommand_PrunesExpiredCommands(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + preExpire int + }{ + {name: "one expired", preExpire: 1}, + {name: "several expired", preExpire: 5}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + b := ssm.NewInMemoryBackend() + ctx := context.Background() + + // Send all commands first (AWS-RunShellScript is a built-in document), + // recording their IDs so they can be expired together afterward. + ids := make([]string, 0, tc.preExpire) + for range tc.preExpire { + out, err := b.SendCommand(ctx, &ssm.SendCommandInput{ + DocumentName: "AWS-RunShellScript", + InstanceIDs: []string{"i-1111"}, + }) + require.NoError(t, err) + ids = append(ids, out.Command.CommandID) + } + + require.Equal(t, tc.preExpire, b.CommandCount()) + require.Equal(t, tc.preExpire, b.CommandInvocationCount()) + + // Now force every command into the past. + for _, id := range ids { + b.SetCommandExpiresAfter(id, 1) + } + + // A fresh send should evict every expired command/invocation and add + // exactly one live command. + _, err := b.SendCommand(ctx, &ssm.SendCommandInput{ + DocumentName: "AWS-RunShellScript", + InstanceIDs: []string{"i-2222"}, + }) + require.NoError(t, err) + + require.Equal(t, 1, b.CommandCount(), + "expired commands must be pruned on SendCommand") + require.Equal(t, 1, b.CommandInvocationCount(), + "expired command invocations must be pruned on SendCommand") + }) + } +} diff --git a/services/sts/backend.go b/services/sts/backend.go index ebf20f14b..ea2fc7a44 100644 --- a/services/sts/backend.go +++ b/services/sts/backend.go @@ -201,6 +201,39 @@ func isSessionExpired(s *SessionInfo) bool { return !s.Expiration.IsZero() && !time.Now().UTC().Before(s.Expiration) } +// sessionEvictThreshold is the session count above which inserting a new session +// triggers an opportunistic sweep of expired sessions. This bounds the sessions +// map even when the background janitor is disabled or runs at a long interval, +// while keeping the common (small) case allocation-free. The threshold is high +// enough that the O(n) sweep amortizes cheaply. +const sessionEvictThreshold = 256 + +// evictExpiredSessionsLocked removes expired sessions from the map. It is a no-op +// below sessionEvictThreshold so steady-state inserts stay O(1). The caller must +// hold b.mu. +func (b *InMemoryBackend) evictExpiredSessionsLocked() { + if len(b.sessions) < sessionEvictThreshold { + return + } + + for id, session := range b.sessions { + if isSessionExpired(session) { + delete(b.sessions, id) + } + } +} + +// storeSession registers a new session under its access key ID, increments the +// lifetime counter, and opportunistically evicts expired sessions so the map +// stays bounded even when the background janitor is not running. +func (b *InMemoryBackend) storeSession(accessKeyID string, session *SessionInfo) { + b.mu.Lock() + b.evictExpiredSessionsLocked() + b.sessions[accessKeyID] = session + b.totalSessionsCreated.Add(1) + b.mu.Unlock() +} + // validateRoleArn checks that a role ARN is a valid IAM role ARN: // - format: arn::iam::<12-digit-account>:role/. func validateRoleArn(roleArn string) error { @@ -510,10 +543,7 @@ func (b *InMemoryBackend) issueCredentials(input *AssumeRoleInput, duration int3 Expiration: expiration, } - b.mu.Lock() - b.sessions[creds.AccessKeyID] = session - b.totalSessionsCreated.Add(1) - b.mu.Unlock() + b.storeSession(creds.AccessKeyID, session) result := AssumeRoleResult{ AssumedRoleUser: AssumedRoleUser{ @@ -663,10 +693,7 @@ func (b *InMemoryBackend) GetSessionToken(input *GetSessionTokenInput) (*GetSess AssumedRoleID: MockUserID, } - b.mu.Lock() - b.sessions[creds.AccessKeyID] = session - b.totalSessionsCreated.Add(1) - b.mu.Unlock() + b.storeSession(creds.AccessKeyID, session) return &GetSessionTokenResponse{ Xmlns: STSNamespace, @@ -748,10 +775,7 @@ func (b *InMemoryBackend) GetFederationToken(input *GetFederationTokenInput) (*G Tags: input.Tags, } - b.mu.Lock() - b.sessions[creds.AccessKeyID] = session - b.totalSessionsCreated.Add(1) - b.mu.Unlock() + b.storeSession(creds.AccessKeyID, session) return &GetFederationTokenResponse{ Xmlns: STSNamespace, @@ -921,10 +945,7 @@ func (b *InMemoryBackend) buildWebIdentityResponse( SourceIdentity: input.SourceIdentity, } - b.mu.Lock() - b.sessions[creds.AccessKeyID] = session - b.totalSessionsCreated.Add(1) - b.mu.Unlock() + b.storeSession(creds.AccessKeyID, session) return &AssumeRoleWithWebIdentityResponse{ Xmlns: STSNamespace, @@ -1068,10 +1089,7 @@ func (b *InMemoryBackend) buildSAMLResponse( Tags: input.Tags, } - b.mu.Lock() - b.sessions[creds.AccessKeyID] = session - b.totalSessionsCreated.Add(1) - b.mu.Unlock() + b.storeSession(creds.AccessKeyID, session) issuerParts := strings.Split(input.PrincipalArn, "/") issuer := issuerParts[len(issuerParts)-1] @@ -1156,10 +1174,7 @@ func (b *InMemoryBackend) AssumeRoot(input *AssumeRootInput) (*AssumeRootRespons AssumedRoleID: account + ":" + rootSessionName, } - b.mu.Lock() - b.sessions[creds.AccessKeyID] = session - b.totalSessionsCreated.Add(1) - b.mu.Unlock() + b.storeSession(creds.AccessKeyID, session) return &AssumeRootResponse{ Xmlns: STSNamespace, @@ -1217,10 +1232,7 @@ func (b *InMemoryBackend) GetDelegatedAccessToken( AssumedRoleID: b.accountID + ":" + delegatedSessionName, } - b.mu.Lock() - b.sessions[creds.AccessKeyID] = session - b.totalSessionsCreated.Add(1) - b.mu.Unlock() + b.storeSession(creds.AccessKeyID, session) return &GetDelegatedAccessTokenResponse{ Xmlns: STSNamespace, diff --git a/services/sts/export_test.go b/services/sts/export_test.go index 7529dc1b4..cfc9f0197 100644 --- a/services/sts/export_test.go +++ b/services/sts/export_test.go @@ -5,6 +5,9 @@ import "time" // DefaultJanitorInterval exposes the package default janitor interval for testing. const DefaultJanitorInterval = defaultSTSJanitorInterval +// SessionEvictThreshold exposes the opportunistic-eviction threshold for tests. +const SessionEvictThreshold = sessionEvictThreshold + // SessionCount returns the number of sessions currently stored in the backend. // Used in tests to verify janitor eviction. func (b *InMemoryBackend) SessionCount() int { diff --git a/services/sts/leak_test.go b/services/sts/leak_test.go new file mode 100644 index 000000000..a450f7ade --- /dev/null +++ b/services/sts/leak_test.go @@ -0,0 +1,70 @@ +package sts_test + +import ( + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/sts" +) + +// TestAssumeRole_EvictsExpiredSessionsWithoutJanitor verifies that creating a +// new session opportunistically sweeps expired sessions once the store grows +// past the eviction threshold, so b.sessions stays bounded even when the +// background janitor is disabled. +func TestAssumeRole_EvictsExpiredSessionsWithoutJanitor(t *testing.T) { + t.Parallel() + + b := sts.NewInMemoryBackend() + + // Seed more expired sessions than the eviction threshold so the next insert + // triggers the inline sweep. + expired := sts.SessionEvictThreshold + 16 + past := time.Now().UTC().Add(-time.Hour) + + for i := range expired { + akid := fmt.Sprintf("ASIAEXPIRED%06d", i) + b.AddSessionInternal(&sts.SessionInfo{ + AccessKeyID: akid, + Expiration: past, + }) + } + + require.Equal(t, expired, b.SessionCount(), "all seeded expired sessions present before insert") + + // A single real AssumeRole insert should evict every expired session and add + // exactly one live session. + resp, err := b.AssumeRole(&sts.AssumeRoleInput{ + RoleArn: "arn:aws:iam::123456789012:role/Role1", + RoleSessionName: "live", + DurationSeconds: 900, + }) + require.NoError(t, err) + require.NotEmpty(t, resp.AssumeRoleResult.Credentials.AccessKeyID) + + require.Equal(t, 1, b.SessionCount(), + "expired sessions must be evicted on insert, leaving only the live one") +} + +// TestAssumeRole_BelowThresholdKeepsExpired confirms the sweep is a no-op below +// the threshold (steady-state inserts stay O(1) and do not eagerly scan). +func TestAssumeRole_BelowThresholdKeepsExpired(t *testing.T) { + t.Parallel() + + b := sts.NewInMemoryBackend() + past := time.Now().UTC().Add(-time.Hour) + + b.AddSessionInternal(&sts.SessionInfo{AccessKeyID: "ASIAEXPIRED0", Expiration: past}) + + _, err := b.AssumeRole(&sts.AssumeRoleInput{ + RoleArn: "arn:aws:iam::123456789012:role/Role1", + RoleSessionName: "live", + DurationSeconds: 900, + }) + require.NoError(t, err) + + // Below threshold: the expired session is not eagerly swept, so both remain. + require.Equal(t, 2, b.SessionCount()) +} From 08e8fdd4df58081f55cf522b36b8ed36e929560d Mon Sep 17 00:00:00 2001 From: mayor Date: Wed, 10 Jun 2026 15:36:46 -0500 Subject: [PATCH 2/6] fix(parity): correct AWS emulation gaps (bucket B) Address still-broken "Incorrect / missing AWS emulation" findings from the full-surface parity audit (bucket B), stacked on the bucket-D leak fixes: - STS: GetCallerIdentity token mismatch now returns 400 InvalidClientTokenId (ErrUnknownAccessKeyID) instead of 403 AccessDenied. - DynamoDB: reject ConsistentRead=true on a GSI (ValidationException); reject duplicate keys within a table's BatchGetItem Keys list. - S3: CompleteMultipartUpload rejects an empty parts list (InvalidRequest). - Bedrock: CreateProvisionedModelThroughput enforces an upper modelUnits bound. - MediaConvert: deepClone no longer silently truncates deeply-nested settings. - Elasticsearch: AssociatePackage returns ConflictException on duplicate. - Account: ListRegions honours maxResults/nextToken (opaque cursor, sorted). - Glacier: retrieval jobs start InProgress and complete after a simulated async window instead of being Succeeded at creation. - API Gateway v2: GetModelTemplate returns the model schema, not {}. - RDS: DescribeDBParameterGroups/DescribeDBParameters/DescribeOptionGroups/ DescribeDBClusterParameterGroups now paginate with Marker/MaxRecords. - DirectoryService: unknown operations return 400 InvalidRequestException instead of 501. Already-correct on the current branch (verified, no change): EventBridge DLQ + malformed-pattern validation, Kinesis GetRecords/ListStreamConsumers/CreateStream, DynamoDB UpdateTable 20-GSI ceiling, Neptune ServerlessV2 config, SNS RedrivePolicy propagation, SecurityHub Id/ProductArn validation. Adds table-driven tests asserting the corrected behaviour for each fix. Co-Authored-By: Claude Opus 4.8 --- services/account/backend.go | 59 +++++++++++- services/account/handler.go | 6 ++ services/account/handler_test.go | 64 +++++++++++++ services/apigatewayv2/handler.go | 12 ++- services/apigatewayv2/handler_test.go | 55 ++++++++++++ services/bedrock/backend.go | 11 +++ services/bedrock/handler_accuracy_test.go | 35 ++++++++ services/directoryservice/handler.go | 3 +- .../directoryservice/handler_audit1_test.go | 26 ++++++ services/dynamodb/accuracy_audit_test.go | 90 +++++++++++++++++++ services/dynamodb/batch_accuracy_b2_test.go | 53 +++++++++++ services/dynamodb/export_test.go | 2 +- services/dynamodb/item_ops_batch.go | 59 ++++++++++++ services/dynamodb/item_ops_query.go | 10 ++- services/elasticsearch/backend.go | 8 +- services/elasticsearch/handler.go | 7 +- services/elasticsearch/handler_test.go | 31 +++++++ services/glacier/backend.go | 70 +++++++++++++-- services/glacier/backend_test.go | 69 ++++++++++++++ services/glacier/export_test.go | 11 +++ services/glacier/handler_accuracy_b2_test.go | 11 ++- services/glacier/handler_refinement2_test.go | 1 + services/glacier/handler_test.go | 4 + services/glacier/models.go | 43 +++++---- services/mediaconvert/backend.go | 14 +-- services/mediaconvert/deepclone_test.go | 54 +++++++++++ services/mediaconvert/export_test.go | 6 ++ services/rds/batch1_test.go | 66 ++++++++++++++ services/rds/handler.go | 57 ++++++++---- services/s3/accuracy_test.go | 40 +++++++++ services/s3/backend_memory.go | 6 ++ services/s3/errors.go | 6 ++ services/sts/backend.go | 9 +- services/sts/handler_accuracy_test.go | 10 ++- services/sts/handler_refinement2_test.go | 42 +++++++++ 35 files changed, 982 insertions(+), 68 deletions(-) create mode 100644 services/mediaconvert/deepclone_test.go diff --git a/services/account/backend.go b/services/account/backend.go index 38ed6bb77..dee6ffe72 100644 --- a/services/account/backend.go +++ b/services/account/backend.go @@ -1,15 +1,19 @@ package account import ( + "encoding/base64" "errors" "fmt" "slices" + "strings" "sync" ) var ( errNoAlternateContact = errors.New("ResourceNotFoundException: no alternate contact found") errNoContactInfo = errors.New("ResourceNotFoundException: no contact information set") + // errInvalidNextToken is returned when ListRegions receives an undecodable cursor. + errInvalidNextToken = errors.New("ValidationException: invalid nextToken") ) // RegionOptStatus represents the opt-in status of an AWS region. @@ -146,8 +150,14 @@ func (b *InMemoryBackend) DescribeAccount() (*Details, error) { }, nil } -// ListRegions returns regions filtered by opt-in status. -func (b *InMemoryBackend) ListRegions(statusFilter []RegionOptStatus, _ int, _ string) ([]*Region, string, error) { +// ListRegions returns regions filtered by opt-in status, honouring AWS's +// maxResults/nextToken pagination. nextToken is an opaque cursor (base64 of the +// exclusive-start RegionName); when maxResults <= 0 the full filtered list is returned. +func (b *InMemoryBackend) ListRegions( + statusFilter []RegionOptStatus, + maxResults int, + nextToken string, +) ([]*Region, string, error) { b.mu.RLock() defer b.mu.RUnlock() @@ -159,7 +169,50 @@ func (b *InMemoryBackend) ListRegions(statusFilter []RegionOptStatus, _ int, _ s } } - return filtered, "", nil + // Order deterministically by RegionName so the name-based pagination cursor is + // stable across pages (AWS returns regions in alphabetical order). + slices.SortFunc(filtered, func(a, b *Region) int { + return strings.Compare(a.RegionName, b.RegionName) + }) + + // Apply the exclusive-start cursor: skip everything up to and including the + // region named by the decoded token. + if nextToken != "" { + start, decErr := decodeRegionToken(nextToken) + if decErr != nil { + return nil, "", errInvalidNextToken + } + + idx := 0 + for idx < len(filtered) && filtered[idx].RegionName <= start { + idx++ + } + + filtered = filtered[idx:] + } + + if maxResults <= 0 || maxResults >= len(filtered) { + return filtered, "", nil + } + + page := filtered[:maxResults] + + return page, encodeRegionToken(page[len(page)-1].RegionName), nil +} + +// encodeRegionToken produces an opaque pagination cursor for the given RegionName. +func encodeRegionToken(regionName string) string { + return base64.StdEncoding.EncodeToString([]byte(regionName)) +} + +// decodeRegionToken reverses encodeRegionToken. +func decodeRegionToken(token string) (string, error) { + raw, err := base64.StdEncoding.DecodeString(token) + if err != nil { + return "", err + } + + return string(raw), nil } // GetAlternateContact retrieves an alternate contact by type. diff --git a/services/account/handler.go b/services/account/handler.go index b334ba7ee..3c65c4e9e 100644 --- a/services/account/handler.go +++ b/services/account/handler.go @@ -4,6 +4,7 @@ import ( "encoding/json" "errors" "net/http" + "strconv" "strings" "github.com/labstack/echo/v5" @@ -203,6 +204,11 @@ func (h *Handler) handleListRegions(c *echo.Context, q interface{ Get(string) st } maxResults := 0 + if raw := q.Get(queryMaxResults); raw != "" { + if n, convErr := strconv.Atoi(raw); convErr == nil && n > 0 { + maxResults = n + } + } nextToken := q.Get(queryNextToken) regions, next, err := h.Backend.ListRegions(statusFilter, maxResults, nextToken) diff --git a/services/account/handler_test.go b/services/account/handler_test.go index 6ce1f2826..2c332878d 100644 --- a/services/account/handler_test.go +++ b/services/account/handler_test.go @@ -256,6 +256,70 @@ func TestHandler_ListRegions(t *testing.T) { } } +// TestHandler_ListRegions_Pagination verifies that ListRegions honours maxResults and +// returns an opaque nextToken, and that paging through with the token yields every +// region exactly once with no overlap. +func TestHandler_ListRegions_Pagination(t *testing.T) { + t.Parallel() + + h := newTestHandler(t) + + // Discover the full unpaged region set first. + allRec := doRequest(t, h, http.MethodGet, "/regions", nil) + require.Equal(t, http.StatusOK, allRec.Code) + + var all struct { + NextToken string `json:"NextToken"` + Regions []account.Region `json:"Regions"` + } + require.NoError(t, json.NewDecoder(allRec.Body).Decode(&all)) + require.Empty(t, all.NextToken, "unpaged listing must not return a token") + require.Greater(t, len(all.Regions), 2, "need several regions to exercise paging") + + const pageSize = 2 + seen := make([]string, 0, len(all.Regions)) + token := "" + + for { + path := "/regions?maxResults=2" + if token != "" { + path += "&nextToken=" + token + } + + rec := doRequest(t, h, http.MethodGet, path, nil) + require.Equal(t, http.StatusOK, rec.Code) + + var page struct { + NextToken string `json:"NextToken"` + Regions []account.Region `json:"Regions"` + } + require.NoError(t, json.NewDecoder(rec.Body).Decode(&page)) + require.LessOrEqual(t, len(page.Regions), pageSize, "page must not exceed maxResults") + + for _, r := range page.Regions { + seen = append(seen, r.RegionName) + } + + if page.NextToken == "" { + break + } + token = page.NextToken + } + + // Every region appears exactly once across the pages — no overlap, no gaps. + assert.Len(t, seen, len(all.Regions)) + assert.ElementsMatch(t, regionNames(all.Regions), seen) +} + +func regionNames(regions []account.Region) []string { + names := make([]string, 0, len(regions)) + for _, r := range regions { + names = append(names, r.RegionName) + } + + return names +} + func TestHandler_AlternateContact_PutGetDelete(t *testing.T) { t.Parallel() diff --git a/services/apigatewayv2/handler.go b/services/apigatewayv2/handler.go index cd813921e..af65d4f6e 100644 --- a/services/apigatewayv2/handler.go +++ b/services/apigatewayv2/handler.go @@ -1257,7 +1257,8 @@ func (h *Handler) handleExportAPI(c *echo.Context, apiID, specification string) } func (h *Handler) handleGetModelTemplate(c *echo.Context, apiID, modelID string) error { - if _, err := h.Backend.GetModel(apiID, modelID); err != nil { + model, err := h.Backend.GetModel(apiID, modelID) + if err != nil { if errors.Is(err, ErrAPINotFound) || errors.Is(err, ErrModelNotFound) { return c.JSON(http.StatusNotFound, notFoundResponse{Message: msgNotFound}) } @@ -1265,7 +1266,14 @@ func (h *Handler) handleGetModelTemplate(c *echo.Context, apiID, modelID string) return c.JSON(http.StatusInternalServerError, notFoundResponse{Message: err.Error()}) } - return c.JSON(http.StatusOK, map[string]string{"value": emptyModelTemplate}) + // AWS returns the model's schema as the template value; fall back to an empty + // object only when the model has no schema defined. + value := model.Schema + if value == "" { + value = emptyModelTemplate + } + + return c.JSON(http.StatusOK, map[string]string{"value": value}) } func (h *Handler) handleDeleteAccessLogSettings(c *echo.Context, apiID, stageName string) error { diff --git a/services/apigatewayv2/handler_test.go b/services/apigatewayv2/handler_test.go index bb17b7142..1ac80402a 100644 --- a/services/apigatewayv2/handler_test.go +++ b/services/apigatewayv2/handler_test.go @@ -2131,6 +2131,61 @@ func TestHandler_CreateModel(t *testing.T) { } } +// TestHandler_GetModelTemplate verifies that GetModelTemplate returns the model's +// schema as the template value, falling back to an empty object only when the model +// has no schema defined. +func TestHandler_GetModelTemplate(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + schema string + wantValue string + }{ + { + name: "returns_schema", + schema: `{"type":"object","properties":{"id":{"type":"string"}}}`, + wantValue: `{"type":"object","properties":{"id":{"type":"string"}}}`, + }, + { + name: "empty_schema_falls_back_to_object", + schema: "", + wantValue: "{}", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + h := newTestHandler() + apiID := createAPI(t, h, "tmpl-api") + + body := map[string]any{"name": "TmplModel", "contentType": "application/json"} + if tt.schema != "" { + body["schema"] = tt.schema + } + + createRec := doRequest(t, h, http.MethodPost, + fmt.Sprintf("/v2/apis/%s/models", apiID), body) + require.Equal(t, http.StatusCreated, createRec.Code) + + var model apigatewayv2.Model + require.NoError(t, json.Unmarshal(createRec.Body.Bytes(), &model)) + + rec := doRequest(t, h, http.MethodGet, + fmt.Sprintf("/v2/apis/%s/models/%s/template", apiID, model.ModelID), nil) + require.Equal(t, http.StatusOK, rec.Code) + + var out struct { + Value string `json:"value"` + } + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &out)) + assert.Equal(t, tt.wantValue, out.Value) + }) + } +} + func TestHandler_CreateRouteResponse(t *testing.T) { t.Parallel() diff --git a/services/bedrock/backend.go b/services/bedrock/backend.go index d62439f80..651679454 100644 --- a/services/bedrock/backend.go +++ b/services/bedrock/backend.go @@ -28,6 +28,10 @@ const ( const bedrockDefaultPageSize = 100 +// maxProvisionedModelUnits is the upper bound AWS enforces on the modelUnits value +// for a single CreateProvisionedModelThroughput request. +const maxProvisionedModelUnits = 1000 + // Resource lifecycle status constants. const ( statusCreating = "Creating" @@ -1020,6 +1024,13 @@ func (b *InMemoryBackend) CreateProvisionedModelThroughput( return nil, fmt.Errorf("%w: modelUnits must be greater than 0", ErrValidation) } + if modelUnits > maxProvisionedModelUnits { + return nil, fmt.Errorf( + "%w: modelUnits must be at most %d", + ErrValidation, maxProvisionedModelUnits, + ) + } + if _, exists := b.pmtsByName[name]; exists { return nil, fmt.Errorf( "%w: provisioned model throughput %s already exists", diff --git a/services/bedrock/handler_accuracy_test.go b/services/bedrock/handler_accuracy_test.go index ced54e290..3a24d91e0 100644 --- a/services/bedrock/handler_accuracy_test.go +++ b/services/bedrock/handler_accuracy_test.go @@ -81,6 +81,41 @@ func TestAccuracy_ProvisionedModelThroughput_InitialStatusCreating(t *testing.T) assert.Equal(t, "Creating", pmt.Status) } +// TestAccuracy_CreateProvisionedModelThroughput_ModelUnitsBounds verifies that +// modelUnits is validated against both the lower (>0) and upper bound AWS enforces. +func TestAccuracy_CreateProvisionedModelThroughput_ModelUnitsBounds(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + modelUnits int32 + wantErr bool + }{ + {name: "valid", modelUnits: 1, wantErr: false}, + {name: "zero_rejected", modelUnits: 0, wantErr: true}, + {name: "negative_rejected", modelUnits: -1, wantErr: true}, + {name: "above_upper_bound_rejected", modelUnits: 100000, wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + b := bedrock.NewInMemoryBackend("000000000000", "us-east-1") + _, err := b.CreateProvisionedModelThroughput( + "pmt-"+tt.name, "amazon.titan-text-express-v1", tt.modelUnits, "", nil) + + if tt.wantErr { + require.Error(t, err) + assert.Contains(t, err.Error(), "ValidationException") + + return + } + require.NoError(t, err) + }) + } +} + func TestAccuracy_AdvanceProvisionedModelThroughput_CreatingToInService(t *testing.T) { t.Parallel() diff --git a/services/directoryservice/handler.go b/services/directoryservice/handler.go index 0d4eed83f..c34a35ddb 100644 --- a/services/directoryservice/handler.go +++ b/services/directoryservice/handler.go @@ -163,7 +163,8 @@ func (h *Handler) doDispatch(c *echo.Context) error { return fn(c) } - return c.JSON(http.StatusNotImplemented, errResp("NotImplementedException", "operation not implemented: "+op)) + // AWS rejects an unrecognised operation with a 400 client error rather than 501. + return c.JSON(http.StatusBadRequest, errResp("InvalidRequestException", "unrecognized operation: "+op)) } func (h *Handler) handleCreateDirectory(c *echo.Context) error { //nolint:dupl // existing issue. diff --git a/services/directoryservice/handler_audit1_test.go b/services/directoryservice/handler_audit1_test.go index 01763a450..bc039a1ea 100644 --- a/services/directoryservice/handler_audit1_test.go +++ b/services/directoryservice/handler_audit1_test.go @@ -630,3 +630,29 @@ func TestDirectoryService_DeleteDirectoryRemovesSnapshots(t *testing.T) { snaps := descResp["Snapshots"].([]any) assert.Empty(t, snaps) } + +// TestDirectoryService_UnknownOperation verifies that an unrecognised operation is +// rejected with an AWS-style 400 InvalidRequestException rather than 501. +func TestDirectoryService_UnknownOperation(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + op string + }{ + {name: "bogus_op", op: "NoSuchOperation"}, + {name: "empty_op", op: "Frobnicate"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + h := newTestHandler(t) + rec := doRequest(t, h, tt.op, map[string]any{}) + + assert.Equal(t, http.StatusBadRequest, rec.Code) + assert.Contains(t, rec.Body.String(), "InvalidRequestException") + }) + } +} diff --git a/services/dynamodb/accuracy_audit_test.go b/services/dynamodb/accuracy_audit_test.go index 2c56cc1fa..e7044fd87 100644 --- a/services/dynamodb/accuracy_audit_test.go +++ b/services/dynamodb/accuracy_audit_test.go @@ -231,6 +231,96 @@ func TestConsistentRead_Query_DoesNotError(t *testing.T) { } } +// 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() + db := newAuditTestDB(t) + ctx := context.Background() + + _, err := db.CreateTable(ctx, &dynamodb.CreateTableInput{ + TableName: aws.String("CRGSI"), + 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, + }) + if err != nil { + t.Fatalf("CreateTable: %v", err) + } + + input := &dynamodb.QueryInput{ + TableName: aws.String("CRGSI"), + ConsistentRead: aws.Bool(true), + KeyConditionExpression: aws.String("pk = :pk"), + ExpressionAttributeValues: map[string]types.AttributeValue{ + ":pk": &types.AttributeValueMemberS{Value: "p1"}, + }, + } + if tt.indexName != "" { + input.IndexName = aws.String(tt.indexName) + if tt.indexName == "gsi1" { + input.KeyConditionExpression = aws.String("gsi_pk = :pk") + } + } + + _, err = db.Query(ctx, input) + if tt.wantErr { + if err == nil || !strings.Contains(err.Error(), "ValidationException") { + t.Fatalf("expected ValidationException, got: %v", err) + } + + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + } +} + func TestConsistentRead_Scan_DoesNotError(t *testing.T) { t.Parallel() db := newAuditTestDB(t) diff --git a/services/dynamodb/batch_accuracy_b2_test.go b/services/dynamodb/batch_accuracy_b2_test.go index 065d4d1ed..8a51c863e 100644 --- a/services/dynamodb/batch_accuracy_b2_test.go +++ b/services/dynamodb/batch_accuracy_b2_test.go @@ -116,6 +116,59 @@ func TestBatchGetItem_EmptyKeysForTable_Rejected(t *testing.T) { b2AssertValidationErr(t, err) } +// TestBatchGetItem_DuplicateKeys_Rejected verifies that a per-table Keys list +// containing the same key twice is rejected with a ValidationException, matching AWS, +// rather than returning the matching item more than once. +func TestBatchGetItem_DuplicateKeys_Rejected(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + keys []map[string]types.AttributeValue + wantErr bool + }{ + { + name: "unique_keys_ok", + keys: []map[string]types.AttributeValue{ + {"pk": &types.AttributeValueMemberS{Value: "a"}}, + {"pk": &types.AttributeValueMemberS{Value: "b"}}, + }, + wantErr: false, + }, + { + name: "duplicate_keys_rejected", + keys: []map[string]types.AttributeValue{ + {"pk": &types.AttributeValueMemberS{Value: "a"}}, + {"pk": &types.AttributeValueMemberS{Value: "a"}}, + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + d := b2NewDB(t) + b2CreateTable(t, d) + + _, err := d.BatchGetItem(context.Background(), &dynamodb.BatchGetItemInput{ + RequestItems: map[string]types.KeysAndAttributes{ + b2TableName: {Keys: tt.keys}, + }, + }) + + if tt.wantErr { + b2AssertValidationErr(t, err) + + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + } +} + // ─── Section 31: BatchGetItem AttributesToGet projection ──────────────────── func TestBatchGetItem_AttributesToGet_AppliedWhenNoProjectionExpression(t *testing.T) { diff --git a/services/dynamodb/export_test.go b/services/dynamodb/export_test.go index 1fdfd14b9..407d8a1be 100644 --- a/services/dynamodb/export_test.go +++ b/services/dynamodb/export_test.go @@ -59,7 +59,7 @@ func (db *InMemoryDB) ExtractKeySchema( table *Table, indexName string, ) ([]models.KeySchemaElement, *models.Projection, error) { - return db.extractKeySchema(table, indexName) + return db.extractKeySchema(table, indexName, false) } func (db *InMemoryDB) SortCandidates( diff --git a/services/dynamodb/item_ops_batch.go b/services/dynamodb/item_ops_batch.go index e4780a60e..a2e7d5a80 100644 --- a/services/dynamodb/item_ops_batch.go +++ b/services/dynamodb/item_ops_batch.go @@ -5,6 +5,7 @@ import ( "fmt" "slices" "sort" + "strings" "github.com/blackbirdworks/gopherstack/services/dynamodb/models" @@ -22,6 +23,58 @@ const eventuallyConsistentRCU = 0.5 // wcuBytesPerUnit is the number of bytes per write capacity unit (1 KB). const wcuBytesPerUnit = 1024 +// canonicalKey produces a stable, comparable string for a single item key map. +// Keys only ever contain scalar key attributes (S, N, or B), so we serialize the +// sorted attribute names together with their typed scalar value. +func canonicalKey(key map[string]types.AttributeValue) string { + names := make([]string, 0, len(key)) + for name := range key { + names = append(names, name) + } + sort.Strings(names) + + var sb strings.Builder + for _, name := range names { + sb.WriteString(name) + sb.WriteByte('=') + + switch v := key[name].(type) { + case *types.AttributeValueMemberS: + sb.WriteString("S:") + sb.WriteString(v.Value) + case *types.AttributeValueMemberN: + sb.WriteString("N:") + sb.WriteString(v.Value) + case *types.AttributeValueMemberB: + sb.WriteString("B:") + sb.Write(v.Value) + } + + sb.WriteByte('\x00') + } + + return sb.String() +} + +// validateNoDuplicateBatchKeys returns a ValidationException when a table's Keys list +// contains the same key more than once, matching AWS BatchGetItem behaviour. +func validateNoDuplicateBatchKeys(keys []map[string]types.AttributeValue) error { + seen := make(map[string]struct{}, len(keys)) + + for _, key := range keys { + canon := canonicalKey(key) + if _, dup := seen[canon]; dup { + return NewValidationException( + "Provided list of item keys contains duplicates", + ) + } + + seen[canon] = struct{}{} + } + + return nil +} + func (db *InMemoryDB) BatchGetItem( ctx context.Context, input *dynamodb.BatchGetItemInput, @@ -43,6 +96,12 @@ func (db *InMemoryDB) BatchGetItem( if err := validateProjectionParams(projExpr, keysAndAttrs.AttributesToGet); err != nil { return nil, err } + + // AWS rejects a per-table Keys list that contains duplicate keys with a + // ValidationException rather than returning the matching item twice. + if err := validateNoDuplicateBatchKeys(keysAndAttrs.Keys); err != nil { + return nil, err + } } // Validate size limit (no lock needed — only inspects input). diff --git a/services/dynamodb/item_ops_query.go b/services/dynamodb/item_ops_query.go index 124757330..6d836f6ce 100644 --- a/services/dynamodb/item_ops_query.go +++ b/services/dynamodb/item_ops_query.go @@ -128,7 +128,7 @@ func (db *InMemoryDB) QueryWithContext( pkskIndex: pkskIndexCopy, } - keySchema, projection, err := db.extractKeySchema(snapshotTable, idxName) + keySchema, projection, err := db.extractKeySchema(snapshotTable, idxName, aws.ToBool(input.ConsistentRead)) if err != nil { return nil, err } @@ -172,6 +172,7 @@ func (db *InMemoryDB) QueryWithContext( func (db *InMemoryDB) extractKeySchema( table *Table, indexName string, + consistentRead bool, ) ([]models.KeySchemaElement, *models.Projection, error) { if indexName == "" { return table.KeySchema, nil, nil @@ -179,6 +180,13 @@ func (db *InMemoryDB) extractKeySchema( for _, gsi := range table.GlobalSecondaryIndexes { if gsi.IndexName == indexName { + // A GSI is eventually consistent; AWS rejects ConsistentRead=true on a GSI. + if consistentRead { + return nil, nil, NewValidationException( + "Consistent reads are not supported on global secondary indexes", + ) + } + return gsi.KeySchema, &gsi.Projection, nil } } diff --git a/services/elasticsearch/backend.go b/services/elasticsearch/backend.go index cc022ca4f..d9cc6f59a 100644 --- a/services/elasticsearch/backend.go +++ b/services/elasticsearch/backend.go @@ -35,6 +35,9 @@ var ( ErrConnectionNotFound = errors.New("ResourceNotFoundException") ErrPackageNotFound = errors.New("ResourceNotFoundException") ErrVpcEndpointNotFound = errors.New("ResourceNotFoundException") + // ErrPackageAlreadyAssociated is returned when AssociatePackage targets a + // (package, domain) pair that is already associated. AWS returns ConflictException. + ErrPackageAlreadyAssociated = errors.New("ConflictException") ) // domainNameRe validates Elasticsearch domain names: @@ -495,7 +498,10 @@ func (b *InMemoryBackend) AssociatePackage(packageID, domainName string) error { } if slices.Contains(b.packageAssociations[packageID], domainName) { - return nil // already associated + return fmt.Errorf( + "%w: package %s is already associated with domain %s", + ErrPackageAlreadyAssociated, packageID, domainName, + ) } b.packageAssociations[packageID] = append(b.packageAssociations[packageID], domainName) diff --git a/services/elasticsearch/handler.go b/services/elasticsearch/handler.go index 0c20913e0..287817fe7 100644 --- a/services/elasticsearch/handler.go +++ b/services/elasticsearch/handler.go @@ -1382,9 +1382,12 @@ func (h *Handler) handleAssociatePackage(w http.ResponseWriter, r *http.Request) packageID, domainName := parts[0], parts[1] if assocErr := h.Backend.AssociatePackage(packageID, domainName); assocErr != nil { - if errors.Is(assocErr, ErrDomainNotFound) || errors.Is(assocErr, ErrPackageNotFound) { + switch { + case errors.Is(assocErr, ErrDomainNotFound) || errors.Is(assocErr, ErrPackageNotFound): h.writeError(r, w, http.StatusNotFound, "ResourceNotFoundException", assocErr.Error()) - } else { + case errors.Is(assocErr, ErrPackageAlreadyAssociated): + h.writeError(r, w, http.StatusConflict, "ConflictException", assocErr.Error()) + default: h.writeError(r, w, http.StatusBadRequest, "ValidationException", assocErr.Error()) } diff --git a/services/elasticsearch/handler_test.go b/services/elasticsearch/handler_test.go index 9dbba5468..9635817c8 100644 --- a/services/elasticsearch/handler_test.go +++ b/services/elasticsearch/handler_test.go @@ -964,6 +964,37 @@ func TestElasticsearchHandler_AssociatePackage(t *testing.T) { }, wantCode: http.StatusNotFound, }, + { + name: "duplicate_association_conflict", + setup: func(t *testing.T, h *elasticsearch.Handler) (string, string) { + t.Helper() + + pkgResp := doRequest(t, h, http.MethodPost, "/2015-01-01/packages", map[string]any{ + "PackageName": "dup-dict", + "PackageType": "TXT-DICTIONARY", + }) + defer pkgResp.Body.Close() + + var pkgOut map[string]any + require.NoError(t, json.NewDecoder(pkgResp.Body).Decode(&pkgOut)) + pkgID := pkgOut["PackageDetails"].(map[string]any)["PackageID"].(string) + + domResp := doRequest(t, h, http.MethodPost, "/2015-01-01/es/domain", map[string]any{ + "DomainName": "dup-domain", + }) + domResp.Body.Close() + + // First association succeeds; the test body issues the duplicate. + firstResp := doRequest(t, h, http.MethodPost, + "/2015-01-01/packages/associate/"+pkgID+"/dup-domain", nil) + firstResp.Body.Close() + require.Equal(t, http.StatusOK, firstResp.StatusCode) + + return pkgID, "dup-domain" + }, + wantCode: http.StatusConflict, + wantContains: []string{"ConflictException"}, + }, } for _, tt := range tests { diff --git a/services/glacier/backend.go b/services/glacier/backend.go index 30abdc0b0..9676024ba 100644 --- a/services/glacier/backend.go +++ b/services/glacier/backend.go @@ -43,6 +43,10 @@ const ( lockStateLocked = "Locked" lockStateUnlocked = "Unlocked" + // jobStatusInProgress and jobStatusSucceeded are the retrieval-job status codes. + jobStatusInProgress = "InProgress" + jobStatusSucceeded = "Succeeded" + // archiveIDLength is the length of the random archive ID suffix. archiveIDLength = 60 // jobIDLength is the length of the random job ID. @@ -161,7 +165,11 @@ type InMemoryBackend struct { vaultLocks map[vaultKey]*VaultLock provisionedCapacity map[string][]*ProvisionedCapacity dataRetrievalPolicies map[string]string - mu sync.RWMutex + // retrievalDelay is the simulated asynchronous retrieval window applied to newly + // initiated jobs. Jobs stay InProgress until CreationDate+retrievalDelay, matching + // AWS, which does not make archive/inventory output available immediately. + retrievalDelay time.Duration + mu sync.RWMutex } // NewInMemoryBackend creates a new in-memory Glacier backend. @@ -175,9 +183,16 @@ func NewInMemoryBackend() *InMemoryBackend { vaultLocks: make(map[vaultKey]*VaultLock), provisionedCapacity: make(map[string][]*ProvisionedCapacity), dataRetrievalPolicies: make(map[string]string), + retrievalDelay: defaultRetrievalDelay, } } +// defaultRetrievalDelay is the simulated asynchronous retrieval window applied to +// newly initiated jobs. Kept short so callers and tests are not forced to wait a +// realistic multi-hour window, while still exercising the InProgress -> Succeeded +// lifecycle (real AWS Standard retrievals take 3-5 hours). +const defaultRetrievalDelay = 100 * time.Millisecond + // cloneVault returns a deep copy of the vault with Tags and NotificationEvents cloned. func cloneVault(v *Vault) *Vault { cp := *v @@ -488,6 +503,19 @@ func (b *InMemoryBackend) InitiateJob(accountID, region, vaultName string, req * } now := time.Now() + + // AWS retrievals are asynchronous: a freshly initiated job starts InProgress and + // only completes after a retrieval window elapses. We simulate that window with + // retrievalDelay; reads promote the job to Succeeded once it has passed. A zero + // delay means jobs complete immediately. + readyAt := now.Add(b.retrievalDelay) + ready := !now.Before(readyAt) + + statusCode := jobStatusInProgress + if ready { + statusCode = jobStatusSucceeded + } + j := &Job{ JobID: generateID(jobIDLength), VaultARN: v.VaultARN, @@ -496,14 +524,18 @@ func (b *InMemoryBackend) InitiateJob(accountID, region, vaultName string, req * ArchiveID: req.ArchiveID, InventoryFormat: inventoryFormat, JobDescription: req.Description, - StatusCode: "Succeeded", - StatusMessage: "Succeeded", + StatusCode: statusCode, + StatusMessage: statusCode, CreationDate: formatDate(now), - CompletionDate: formatDate(now), - Completed: true, + Completed: ready, Tier: tier, SNSTopic: req.SNSTopic, RetrievalByteRange: req.RetrievalByteRange, + readyAt: readyAt, + } + + if ready { + j.CompletionDate = formatDate(now) } if action == jobTypeArchiveRetrieval { @@ -524,8 +556,8 @@ func (b *InMemoryBackend) InitiateJob(accountID, region, vaultName string, req * // DescribeJob returns metadata for a job. func (b *InMemoryBackend) DescribeJob(accountID, region, vaultName, jobID string) (*Job, error) { - b.mu.RLock() - defer b.mu.RUnlock() + b.mu.Lock() + defer b.mu.Unlock() key := vaultKey{AccountID: accountID, Region: region, VaultName: vaultName} @@ -538,14 +570,33 @@ func (b *InMemoryBackend) DescribeJob(accountID, region, vaultName, jobID string return nil, ErrJobNotFound } + promoteJobIfReady(j) + return cloneJob(j), nil } +// promoteJobIfReady transitions an InProgress retrieval job to Succeeded once its +// simulated retrieval window (readyAt) has elapsed. Caller must hold b.mu for writing. +func promoteJobIfReady(j *Job) { + if j.Completed { + return + } + + if j.readyAt.IsZero() || time.Now().Before(j.readyAt) { + return + } + + j.Completed = true + j.StatusCode = jobStatusSucceeded + j.StatusMessage = jobStatusSucceeded + j.CompletionDate = formatDate(time.Now()) +} + // ListJobs returns all jobs for the given vault. // Returns ErrVaultNotFound if the vault does not exist. func (b *InMemoryBackend) ListJobs(accountID, region, vaultName string) ([]*Job, error) { - b.mu.RLock() - defer b.mu.RUnlock() + b.mu.Lock() + defer b.mu.Unlock() key := vaultKey{AccountID: accountID, Region: region, VaultName: vaultName} @@ -561,6 +612,7 @@ func (b *InMemoryBackend) ListJobs(accountID, region, vaultName string) ([]*Job, result := make([]*Job, 0, len(jobs)) for _, j := range jobs { + promoteJobIfReady(j) result = append(result, cloneJob(j)) } diff --git a/services/glacier/backend_test.go b/services/glacier/backend_test.go index 01620f99d..95e8d1af0 100644 --- a/services/glacier/backend_test.go +++ b/services/glacier/backend_test.go @@ -2,6 +2,7 @@ package glacier_test import ( "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -193,6 +194,8 @@ func TestInMemoryBackend_JobCRUD(t *testing.T) { t.Parallel() bk := glacier.NewInMemoryBackend() + // This case exercises CRUD, not the async window: complete jobs immediately. + glacier.SetRetrievalDelay(bk, 0) _, err := bk.CreateVault(testAccountID, testRegion, "vault") require.NoError(t, err) @@ -354,3 +357,69 @@ func TestInMemoryBackend_AccessPolicy(t *testing.T) { }) } } + +// TestInMemoryBackend_RetrievalJobAsyncLifecycle verifies that a freshly initiated +// retrieval job starts InProgress and is only promoted to Succeeded once the simulated +// retrieval window has elapsed, matching AWS's asynchronous retrieval semantics. +func TestInMemoryBackend_RetrievalJobAsyncLifecycle(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + wantStatus string + delay time.Duration + waitForReady bool + wantCompleted bool + }{ + { + name: "in_progress_within_window", + delay: time.Hour, + waitForReady: false, + wantCompleted: false, + wantStatus: "InProgress", + }, + { + name: "succeeded_after_window", + delay: 5 * time.Millisecond, + waitForReady: true, + wantCompleted: true, + wantStatus: "Succeeded", + }, + { + name: "immediate_when_zero_delay", + delay: 0, + waitForReady: false, + wantCompleted: true, + wantStatus: "Succeeded", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + bk := glacier.NewInMemoryBackend() + glacier.SetRetrievalDelay(bk, tt.delay) + + _, err := bk.CreateVault(testAccountID, testRegion, "vault") + require.NoError(t, err) + + j, err := bk.InitiateJob(testAccountID, testRegion, "vault", + &glacier.ExportedInitiateJobRequest{Type: "InventoryRetrieval"}) + require.NoError(t, err) + + if tt.waitForReady { + require.Eventually(t, func() bool { + got, descErr := bk.DescribeJob(testAccountID, testRegion, "vault", j.JobID) + + return descErr == nil && got.Completed + }, time.Second, 2*time.Millisecond) + } + + got, err := bk.DescribeJob(testAccountID, testRegion, "vault", j.JobID) + require.NoError(t, err) + assert.Equal(t, tt.wantCompleted, got.Completed) + assert.Equal(t, tt.wantStatus, got.StatusCode) + }) + } +} diff --git a/services/glacier/export_test.go b/services/glacier/export_test.go index ad68ff0ec..2907fdcb9 100644 --- a/services/glacier/export_test.go +++ b/services/glacier/export_test.go @@ -1,9 +1,20 @@ package glacier +import "time" + // ExportedInitiateJobRequest is an alias for the unexported initiateJobRequest type, // made available for use in external (_test) test packages. type ExportedInitiateJobRequest = initiateJobRequest +// SetRetrievalDelay overrides the simulated asynchronous retrieval window for newly +// initiated jobs (for testing only). A zero delay makes jobs complete immediately. +func SetRetrievalDelay(b *InMemoryBackend, d time.Duration) { + b.mu.Lock() + defer b.mu.Unlock() + + b.retrievalDelay = d +} + // ComputeTreeHash exposes the internal tree-hash computation for testing. func ComputeTreeHash(data []byte) string { return computeTreeHash(data) diff --git a/services/glacier/handler_accuracy_b2_test.go b/services/glacier/handler_accuracy_b2_test.go index 8f48864cd..665e37c7d 100644 --- a/services/glacier/handler_accuracy_b2_test.go +++ b/services/glacier/handler_accuracy_b2_test.go @@ -659,9 +659,14 @@ func TestAccuracyB2_ListJobs_CompletedParamValidation(t *testing.T) { func TestAccuracyB2_GetJobOutput_CompletedFilter_Integration(t *testing.T) { t.Parallel() - // Initiate a job (which is immediately Succeeded in our emulator) then verify - // that GetJobOutput works. Then seed an InProgress job and verify rejection. - h := newTestHandler() + // Initiate a job and verify that, with the simulated retrieval window disabled, + // GetJobOutput works immediately. A zero retrieval delay keeps the assertion + // deterministic. Then seed an InProgress job and verify rejection. + bk := glacier.NewInMemoryBackend() + glacier.SetRetrievalDelay(bk, 0) + h := glacier.NewHandler(bk) + h.AccountID = testAccountID + h.DefaultRegion = testRegion createVault(t, h, "job-filter-vault") jobRec := doRequest(t, h, http.MethodPost, diff --git a/services/glacier/handler_refinement2_test.go b/services/glacier/handler_refinement2_test.go index 40cf8ce8a..d9976b9af 100644 --- a/services/glacier/handler_refinement2_test.go +++ b/services/glacier/handler_refinement2_test.go @@ -688,6 +688,7 @@ func TestRefinement2_GetJobOutput_SHA256Header(t *testing.T) { t.Parallel() bk := glacier.NewInMemoryBackend() + glacier.SetRetrievalDelay(bk, 0) h := glacier.NewHandler(bk) h.AccountID = testAccountID h.DefaultRegion = testRegion diff --git a/services/glacier/handler_test.go b/services/glacier/handler_test.go index 289b253e0..b137d37d7 100644 --- a/services/glacier/handler_test.go +++ b/services/glacier/handler_test.go @@ -17,6 +17,10 @@ import ( func newTestHandler() *glacier.Handler { bk := glacier.NewInMemoryBackend() + // Disable the simulated asynchronous retrieval window so handler tests that + // initiate a job and immediately read its output remain deterministic. The + // async lifecycle itself is covered by dedicated tests in backend_test.go. + glacier.SetRetrievalDelay(bk, 0) h := glacier.NewHandler(bk) h.AccountID = testAccountID h.DefaultRegion = testRegion diff --git a/services/glacier/models.go b/services/glacier/models.go index d8dfaa1cd..eaafb5ced 100644 --- a/services/glacier/models.go +++ b/services/glacier/models.go @@ -27,24 +27,31 @@ type Archive struct { // Job stores state for a single Glacier retrieval or inventory job. type Job struct { - VaultARN string `json:"vaultARN"` - VaultName string `json:"vaultName"` - JobID string `json:"jobID"` - JobDescription string `json:"jobDescription,omitempty"` - Action string `json:"action"` - ArchiveID string `json:"archiveID,omitempty"` - InventoryFormat string `json:"inventoryFormat,omitempty"` - StatusCode string `json:"statusCode"` - StatusMessage string `json:"statusMessage,omitempty"` - CreationDate string `json:"creationDate"` - CompletionDate string `json:"completionDate,omitempty"` - Tier string `json:"tier,omitempty"` - SHA256TreeHash string `json:"sha256TreeHash,omitempty"` - SNSTopic string `json:"snsTopic,omitempty"` - RetrievalByteRange string `json:"retrievalByteRange,omitempty"` - ArchiveSizeInBytes int64 `json:"archiveSizeInBytes,omitempty"` - InventorySizeInBytes int64 `json:"inventorySizeInBytes,omitempty"` - Completed bool `json:"completed"` + // readyAt is the simulated time at which an asynchronous retrieval job completes. + // While time.Now() is before readyAt the job stays InProgress; on read it is then + // promoted to Succeeded. It is internal state and never serialized. + readyAt time.Time + + VaultARN string `json:"vaultARN"` + VaultName string `json:"vaultName"` + JobID string `json:"jobID"` + JobDescription string `json:"jobDescription,omitempty"` + Action string `json:"action"` + ArchiveID string `json:"archiveID,omitempty"` + InventoryFormat string `json:"inventoryFormat,omitempty"` + StatusCode string `json:"statusCode"` + StatusMessage string `json:"statusMessage,omitempty"` + CreationDate string `json:"creationDate"` + CompletionDate string `json:"completionDate,omitempty"` + Tier string `json:"tier,omitempty"` + SHA256TreeHash string `json:"sha256TreeHash,omitempty"` + SNSTopic string `json:"snsTopic,omitempty"` + RetrievalByteRange string `json:"retrievalByteRange,omitempty"` + + ArchiveSizeInBytes int64 `json:"archiveSizeInBytes,omitempty"` + InventorySizeInBytes int64 `json:"inventorySizeInBytes,omitempty"` + + Completed bool `json:"completed"` } // vaultLockPolicyRequest is the request body for InitiateVaultLock. diff --git a/services/mediaconvert/backend.go b/services/mediaconvert/backend.go index b21b54788..652d1c7df 100644 --- a/services/mediaconvert/backend.go +++ b/services/mediaconvert/backend.go @@ -57,7 +57,10 @@ const ( // orderAscending is the ascending list order value used by AWS MediaConvert. orderAscending = "ASCENDING" // deepCloneMaxDepth caps recursion in deepCloneValue to prevent stack overflows. - deepCloneMaxDepth = 20 + // deepCloneMaxDepth bounds recursion in deepCloneValueAt to guard against + // pathological/cyclic input. It is set well above the depth of any real + // MediaConvert job-settings document so legitimate settings are never affected. + deepCloneMaxDepth = 100 // tokenTTL is how long a ClientRequestToken deduplication window lasts. tokenTTL = time.Minute // jobEngineVersionUsed is the fixed engine version reported on all jobs. @@ -85,14 +88,11 @@ func deepCloneMap(m map[string]any) map[string]any { } // deepCloneValueAt clones v with a depth counter. When depth >= deepCloneMaxDepth, -// map and slice values are returned as nil to prevent unbounded recursion. +// the value is returned as-is (shared reference) rather than recursing further. This +// preserves the data even for unexpectedly deep documents instead of silently dropping +// it to nil; the bound only exists to guard against pathological/cyclic input. func deepCloneValueAt(v any, depth int) any { if depth >= deepCloneMaxDepth { - switch v.(type) { - case map[string]any, []any: - return nil - } - return v } diff --git a/services/mediaconvert/deepclone_test.go b/services/mediaconvert/deepclone_test.go new file mode 100644 index 000000000..40dc06aff --- /dev/null +++ b/services/mediaconvert/deepclone_test.go @@ -0,0 +1,54 @@ +package mediaconvert_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/mediaconvert" +) + +// TestDeepCloneMap_PreservesDeeplyNestedSettings verifies that deepCloneMap clones a +// settings document without silently truncating deeply-nested values to nil. Previously +// any value nested beyond depth 20 was dropped, corrupting legitimate job settings. +func TestDeepCloneMap_PreservesDeeplyNestedSettings(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + depth int + }{ + {name: "shallow", depth: 5}, + {name: "at_old_limit", depth: 20}, + {name: "beyond_old_limit", depth: 30}, + {name: "deep", depth: 50}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + // Build a chain of nested maps of the requested depth, with a leaf marker. + leafKey := "leaf" + leafVal := "preserved" + + current := map[string]any{leafKey: leafVal} + for range tt.depth { + current = map[string]any{"child": current} + } + + cloned := mediaconvert.DeepCloneMapForTest(current) + + // Walk the clone down to the leaf and assert the marker survived. + node := cloned + for range tt.depth { + next, ok := node["child"].(map[string]any) + require.Truef(t, ok, "expected nested map at each level, missing one (depth %d)", tt.depth) + node = next + } + + assert.Equal(t, leafVal, node[leafKey]) + }) + } +} diff --git a/services/mediaconvert/export_test.go b/services/mediaconvert/export_test.go index 0c0bbb503..e944bc1c0 100644 --- a/services/mediaconvert/export_test.go +++ b/services/mediaconvert/export_test.go @@ -1,5 +1,11 @@ package mediaconvert +// DeepCloneMapForTest exposes deepCloneMap for testing that deeply-nested settings +// documents are cloned without silent truncation. +func DeepCloneMapForTest(m map[string]any) map[string]any { + return deepCloneMap(m) +} + // QueueCount returns the number of queues in the backend. func QueueCount(b *InMemoryBackend) int { b.mu.RLock("QueueCount") diff --git a/services/rds/batch1_test.go b/services/rds/batch1_test.go index 83ba25bff..89627d143 100644 --- a/services/rds/batch1_test.go +++ b/services/rds/batch1_test.go @@ -1,6 +1,7 @@ package rds_test import ( + "encoding/xml" "fmt" "net/http" "sync" @@ -1104,6 +1105,71 @@ func TestHandler_DescribeDBShardGroups_Pagination(t *testing.T) { assert.Contains(t, body, "Marker") } +// TestHandler_DescribeDBParameterGroups_Pagination verifies that +// DescribeDBParameterGroups honours MaxRecords and returns a Marker that pages through +// the full set exactly once with no overlap. +func TestHandler_DescribeDBParameterGroups_Pagination(t *testing.T) { + t.Parallel() + h := newRDSHandler() + + const total = 5 + for i := range total { + rec := postRDSForm(t, h, fmt.Sprintf( + "Action=CreateDBParameterGroup&Version=2014-10-31"+ + "&DBParameterGroupName=pg-%02d&DBParameterGroupFamily=postgres15"+ + "&Description=test", i, + )) + require.Equal(t, http.StatusOK, rec.Code) + } + + // First page: MaxRecords=2 must return a Marker. + rec := postRDSForm(t, h, "Action=DescribeDBParameterGroups&Version=2014-10-31&MaxRecords=2") + require.Equal(t, http.StatusOK, rec.Code) + + type pgResp struct { + Marker string `xml:"DescribeDBParameterGroupsResult>Marker"` + Groups []struct { + Name string `xml:"DBParameterGroupName"` + } `xml:"DescribeDBParameterGroupsResult>DBParameterGroups>DBParameterGroup"` + } + + seen := map[string]int{} + marker := "" + pages := 0 + + for { + q := "Action=DescribeDBParameterGroups&Version=2014-10-31&MaxRecords=2" + if marker != "" { + q += "&Marker=" + marker + } + + pageRec := postRDSForm(t, h, q) + require.Equal(t, http.StatusOK, pageRec.Code) + + var resp pgResp + require.NoError(t, xml.Unmarshal(pageRec.Body.Bytes(), &resp)) + require.LessOrEqual(t, len(resp.Groups), 2) + + for _, g := range resp.Groups { + seen[g.Name]++ + } + + pages++ + if resp.Marker == "" { + break + } + marker = resp.Marker + require.Less(t, pages, 10, "pagination did not terminate") + } + + // 5 user groups + the default parameter group are returned exactly once each. + require.GreaterOrEqual(t, len(seen), total) + for name, count := range seen { + assert.Equalf(t, 1, count, "group %s appeared on more than one page", name) + } + assert.Greater(t, pages, 1, "expected multiple pages") +} + func TestHandler_DescribeIntegrations_Pagination(t *testing.T) { t.Parallel() h := newRDSHandler() diff --git a/services/rds/handler.go b/services/rds/handler.go index 3055288d6..b36d562b8 100644 --- a/services/rds/handler.go +++ b/services/rds/handler.go @@ -1530,15 +1530,21 @@ func (h *Handler) handleDescribeDBParameterGroups(vals url.Values) (any, error) if err != nil { return nil, err } - members := make([]xmlDBParameterGroup, 0, len(groups)) - for _, pg := range groups { - cp := pg - members = append(members, toXMLParameterGroup(&cp)) + members, marker, err := paginateDescribe(vals, groups, func(a, b DBParameterGroup) bool { + return a.DBParameterGroupName < b.DBParameterGroupName + }, func(item DBParameterGroup) xmlDBParameterGroup { + cp := item + + return toXMLParameterGroup(&cp) + }) + if err != nil { + return nil, err } return &describeDBParameterGroupsResponse{ Xmlns: rdsXMLNS, DBParameterGroups: xmlDBParameterGroupList{Members: members}, + Marker: marker, }, nil } @@ -1571,14 +1577,19 @@ func (h *Handler) handleDescribeDBParameters(vals url.Values) (any, error) { if err != nil { return nil, err } - members := make([]xmlDBParameter, 0, len(params)) - for _, p := range params { - members = append(members, xmlDBParameter(p)) + members, marker, err := paginateDescribe(vals, params, func(a, b DBParameter) bool { + return a.ParameterName < b.ParameterName + }, func(item DBParameter) xmlDBParameter { + return xmlDBParameter(item) + }) + if err != nil { + return nil, err } return &describeDBParametersResponse{ Xmlns: rdsXMLNS, Parameters: xmlDBParameterList{Members: members}, + Marker: marker, }, nil } @@ -1628,15 +1639,21 @@ func (h *Handler) handleDescribeOptionGroups(vals url.Values) (any, error) { if err != nil { return nil, err } - members := make([]xmlOptionGroup, 0, len(groups)) - for _, og := range groups { - cp := og - members = append(members, toXMLOptionGroup(&cp)) + members, marker, err := paginateDescribe(vals, groups, func(a, b OptionGroup) bool { + return a.OptionGroupName < b.OptionGroupName + }, func(item OptionGroup) xmlOptionGroup { + cp := item + + return toXMLOptionGroup(&cp) + }) + if err != nil { + return nil, err } return &describeOptionGroupsResponse{ Xmlns: rdsXMLNS, OptionGroupsList: xmlOptionGroupList{Members: members}, + Marker: marker, }, nil } @@ -1874,16 +1891,22 @@ func (h *Handler) handleDescribeDBClusterParameterGroups(vals url.Values) (any, if err != nil { return nil, err } - members := make([]xmlDBClusterParameterGroup, 0, len(groups)) - for _, pg := range groups { - cp := pg - members = append(members, toXMLClusterParameterGroup(&cp)) + members, marker, err := paginateDescribe(vals, groups, func(a, b DBParameterGroup) bool { + return a.DBParameterGroupName < b.DBParameterGroupName + }, func(item DBParameterGroup) xmlDBClusterParameterGroup { + cp := item + + return toXMLClusterParameterGroup(&cp) + }) + if err != nil { + return nil, err } return &describeDBClusterParameterGroupsResponse{ Xmlns: rdsXMLNS, Result: describeDBClusterParameterGroupsResult{ DBClusterParameterGroups: xmlDBClusterParameterGroupList{Members: members}, + Marker: marker, }, }, nil } @@ -2467,6 +2490,7 @@ type createDBParameterGroupResponse struct { type describeDBParameterGroupsResponse struct { XMLName xml.Name `xml:"DescribeDBParameterGroupsResponse"` Xmlns string `xml:"xmlns,attr"` + Marker string `xml:"DescribeDBParameterGroupsResult>Marker,omitempty"` DBParameterGroups xmlDBParameterGroupList `xml:"DescribeDBParameterGroupsResult>DBParameterGroups"` } @@ -2490,6 +2514,7 @@ type resetDBParameterGroupResponse struct { type describeDBParametersResponse struct { XMLName xml.Name `xml:"DescribeDBParametersResponse"` Xmlns string `xml:"xmlns,attr"` + Marker string `xml:"DescribeDBParametersResult>Marker,omitempty"` Parameters xmlDBParameterList `xml:"DescribeDBParametersResult>Parameters"` } @@ -2525,6 +2550,7 @@ type createOptionGroupResponse struct { type describeOptionGroupsResponse struct { XMLName xml.Name `xml:"DescribeOptionGroupsResponse"` Xmlns string `xml:"xmlns,attr"` + Marker string `xml:"DescribeOptionGroupsResult>Marker,omitempty"` OptionGroupsList xmlOptionGroupList `xml:"DescribeOptionGroupsResult>OptionGroupsList"` } @@ -2641,6 +2667,7 @@ type createDBClusterParameterGroupResponse struct { } type describeDBClusterParameterGroupsResult struct { + Marker string `xml:"Marker,omitempty"` DBClusterParameterGroups xmlDBClusterParameterGroupList `xml:"DBClusterParameterGroups"` } diff --git a/services/s3/accuracy_test.go b/services/s3/accuracy_test.go index 2b3e5e3f6..37d5f3e78 100644 --- a/services/s3/accuracy_test.go +++ b/services/s3/accuracy_test.go @@ -451,6 +451,46 @@ func TestCompleteMultipart_EntityTooSmall(t *testing.T) { assert.Equal(t, "EntityTooSmall", errResp.Code) } +func TestCompleteMultipart_EmptyParts_Rejected(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + body string + }{ + { + name: "no_parts_element", + body: ``, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + handler, backend := newTestHandler(t) + mustCreateBucket(t, backend, "mp-empty") + + createRec := doRequest(handler, http.MethodPost, "/mp-empty/obj?uploads", nil, nil) + require.Equal(t, http.StatusOK, createRec.Code) + + var initResult s3.InitiateMultipartUploadResult + require.NoError(t, xml.NewDecoder(createRec.Body).Decode(&initResult)) + uploadID := initResult.UploadID + + completePath := fmt.Sprintf("/mp-empty/obj?uploadId=%s", uploadID) + completeRec := doRequest(handler, http.MethodPost, completePath, + strings.NewReader(tt.body), nil) + + assert.Equal(t, http.StatusBadRequest, completeRec.Code) + + var errResp s3.ErrorResponse + require.NoError(t, xml.NewDecoder(completeRec.Body).Decode(&errResp)) + assert.Equal(t, "InvalidRequest", errResp.Code) + }) + } +} + func TestCompleteMultipart_SinglePartSmall_OK(t *testing.T) { t.Parallel() diff --git a/services/s3/backend_memory.go b/services/s3/backend_memory.go index d25d0789e..b91648d75 100644 --- a/services/s3/backend_memory.go +++ b/services/s3/backend_memory.go @@ -2102,6 +2102,12 @@ func (b *InMemoryBackend) assembleMultipartData( upload *StoredMultipartUpload, input *s3.CompleteMultipartUploadInput, ) (multipartAssemblyResult, error) { + // AWS rejects CompleteMultipartUpload with an empty (or absent) parts list: + // the request must enumerate at least one previously-uploaded part. + if input.MultipartUpload == nil || len(input.MultipartUpload.Parts) == 0 { + return multipartAssemblyResult{}, ErrEmptyParts + } + parts := input.MultipartUpload.Parts data, partMD5s, err := b.collectPartsData(upload, parts) diff --git a/services/s3/errors.go b/services/s3/errors.go index 3e3f44305..65cac8c27 100644 --- a/services/s3/errors.go +++ b/services/s3/errors.go @@ -28,6 +28,7 @@ var ( ErrNoSuchUpload = awserr.New("NoSuchUpload", awserr.ErrNotFound) ErrInvalidPart = errors.New("InvalidPart") ErrInvalidPartOrder = errors.New("InvalidPartOrder") + ErrEmptyParts = errors.New("InvalidRequest") ErrNoCompressor = errors.New("data is compressed but no compressor available") ErrNoBucketPolicy = errors.New("NoSuchBucketPolicy") ErrNoCORSConfig = errors.New("NoSuchCORSConfiguration") @@ -111,6 +112,11 @@ func coreErrorTable() []s3ErrorEntry { "The list of parts was not in ascending order. Parts must be ordered by part number.", http.StatusBadRequest, }}, + {ErrEmptyParts, s3ErrorInfo{ + "InvalidRequest", + "You must specify at least one part", + http.StatusBadRequest, + }}, {ErrInvalidArgument, s3ErrorInfo{errInvalidArgument, "Invalid Argument.", http.StatusBadRequest}}, {ErrMethodNotAllowed, s3ErrorInfo{ "MethodNotAllowed", diff --git a/services/sts/backend.go b/services/sts/backend.go index ea2fc7a44..79eb3794d 100644 --- a/services/sts/backend.go +++ b/services/sts/backend.go @@ -570,7 +570,7 @@ func (b *InMemoryBackend) issueCredentials(input *AssumeRoleInput, duration int3 // GetCallerIdentity returns the mock caller identity. // When accessKeyID corresponds to an assumed-role session, returns the assumed-role ARN and user ID. // When sessionToken is non-empty (ASIA-prefixed key), the stored token must match; a mismatch -// returns ErrAccessDenied mapped to InvalidClientTokenId. +// returns ErrUnknownAccessKeyID mapped to HTTP 400 InvalidClientTokenId (matching AWS). func (b *InMemoryBackend) GetCallerIdentity(accessKeyID, sessionToken string) (*GetCallerIdentityResponse, error) { b.cntGetCallerIdentity.Add(1) @@ -587,8 +587,13 @@ func (b *InMemoryBackend) GetCallerIdentity(accessKeyID, sessionToken string) (* if ok { // When the caller presents a session token, it must match the stored value. + // AWS rejects a mismatched session token with HTTP 400 InvalidClientTokenId, + // not 403 AccessDenied. if sessionToken != "" && session.SessionToken != "" && sessionToken != session.SessionToken { - return nil, fmt.Errorf("%w: session token mismatch", ErrAccessDenied) + return nil, fmt.Errorf( + "%w: the security token included in the request is invalid", + ErrUnknownAccessKeyID, + ) } return &GetCallerIdentityResponse{ diff --git a/services/sts/handler_accuracy_test.go b/services/sts/handler_accuracy_test.go index 442f9e849..76a19c7a4 100644 --- a/services/sts/handler_accuracy_test.go +++ b/services/sts/handler_accuracy_test.go @@ -202,8 +202,10 @@ func TestAccuracy_Gap10_GetCallerIdentity_SessionToken(t *testing.T) { accessKeyID := assumeResp.AssumeRoleResult.Credentials.AccessKeyID + // AWS rejects a mismatched session token with InvalidClientTokenId (HTTP 400), + // surfaced here as ErrUnknownAccessKeyID, not AccessDenied. _, err = b.GetCallerIdentity(accessKeyID, "wrong-token") - require.ErrorIs(t, err, sts.ErrAccessDenied) + require.ErrorIs(t, err, sts.ErrUnknownAccessKeyID) }) t.Run("http_request_uses_x_amz_security_token_header", func(t *testing.T) { @@ -242,7 +244,7 @@ func TestAccuracy_Gap10_GetCallerIdentity_SessionToken(t *testing.T) { assert.Contains(t, resp.GetCallerIdentityResult.Arn, "TestRole") }) - t.Run("http_request_wrong_x_amz_security_token_returns_403", func(t *testing.T) { + t.Run("http_request_wrong_x_amz_security_token_returns_400", func(t *testing.T) { t.Parallel() h, b, e := accuracyHandler(t) @@ -269,7 +271,9 @@ func TestAccuracy_Gap10_GetCallerIdentity_SessionToken(t *testing.T) { req = req.WithContext(logger.Save(req.Context(), nil)) require.NoError(t, h.Handler()(e.NewContext(req, rec))) - assert.Equal(t, http.StatusForbidden, rec.Code) + // AWS returns InvalidClientTokenId (HTTP 400) for a mismatched session token. + assert.Equal(t, http.StatusBadRequest, rec.Code) + assert.Contains(t, rec.Body.String(), "InvalidClientTokenId") }) } diff --git a/services/sts/handler_refinement2_test.go b/services/sts/handler_refinement2_test.go index eb94b892f..0b03b6863 100644 --- a/services/sts/handler_refinement2_test.go +++ b/services/sts/handler_refinement2_test.go @@ -182,6 +182,48 @@ func TestRefinement2_GetSessionTokenSessionStored(t *testing.T) { assert.NotEmpty(t, ci.GetCallerIdentityResult.Arn) } +// TestGetCallerIdentity_SessionTokenMismatch verifies that a mismatched session token +// is rejected as InvalidClientTokenId (HTTP 400 via ErrUnknownAccessKeyID), matching AWS, +// rather than AccessDenied (403). +func TestGetCallerIdentity_SessionTokenMismatch(t *testing.T) { + t.Parallel() + + tests := []struct { + wantErrKind error + name string + wrongToken string + useRealTok bool + wantErr bool + }{ + {name: "matching_token", useRealTok: true, wantErr: false}, + {name: "mismatched_token", wrongToken: "wrong-token", wantErr: true, wantErrKind: sts.ErrUnknownAccessKeyID}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + b := sts.NewInMemoryBackend() + resp, err := b.GetSessionToken(&sts.GetSessionTokenInput{}) + require.NoError(t, err) + + accessKeyID := resp.GetSessionTokenResult.Credentials.AccessKeyID + token := tt.wrongToken + if tt.useRealTok { + token = resp.GetSessionTokenResult.Credentials.SessionToken + } + + _, err = b.GetCallerIdentity(accessKeyID, token) + if tt.wantErr { + require.ErrorIs(t, err, tt.wantErrKind) + + return + } + require.NoError(t, err) + }) + } +} + // TestRefinement2_AssumeRoleWithPolicyArns verifies PolicyArns are parsed and present in input. func TestRefinement2_AssumeRoleWithPolicyArns(t *testing.T) { t.Parallel() From b152bf897509200e331c161e7912f08517d04279 Mon Sep 17 00:00:00 2001 From: mayor Date: Wed, 10 Jun 2026 15:55:59 -0500 Subject: [PATCH 3/6] feat(parity): implement stubbed SDK ops (bucket A) Implement genuinely-stubbed operations identified in the bucket-A audit, backed by real InMemoryBackend state with AWS-accurate shapes/errors: - EC2 RevokeSecurityGroupEgress: validate group, remove matching egress rules, idempotent on absent rules (was a no-op). - CloudTrail LookupEvents: record events; honor time range, lookup attributes (ANDed), ordering, MaxResults and NextToken pagination. - CodePipeline ListActionExecutions: track action executions per pipeline run with pipelineExecutionId filter; ListRuleTypes returns the AWS managed rule catalog. - ApplicationAutoScaling DescribeScalingActivities: record and return scaling activities on RegisterScalableTarget. Most other bucket-A items were verified already-implemented on current code (older audit snapshot). Remaining genuine gaps (AppSync EvaluateCode, Kafka Update* setting payloads, CloudFront long-tail stubs, Pipes silent skip) are documented in parity.md for follow-up. Co-Authored-By: Claude Opus 4.8 --- parity.md | 73 +++++++ services/applicationautoscaling/backend.go | 102 +++++++++- services/applicationautoscaling/handler.go | 19 +- .../applicationautoscaling/handler_test.go | 42 ++++ .../scaling_activities_test.go | 107 ++++++++++ services/cloudtrail/backend.go | 150 ++++++++++++-- services/cloudtrail/lookup_events_test.go | 187 ++++++++++++++++++ .../codepipeline/action_executions_test.go | 134 +++++++++++++ services/codepipeline/backend.go | 106 +++++++++- services/codepipeline/handler.go | 18 +- services/ec2/backend_ext.go | 23 +++ services/ec2/backend_ext_test.go | 50 +++++ services/ec2/backend_iface.go | 3 + services/ec2/handler.go | 15 +- 14 files changed, 988 insertions(+), 41 deletions(-) create mode 100644 services/applicationautoscaling/scaling_activities_test.go create mode 100644 services/cloudtrail/lookup_events_test.go create mode 100644 services/codepipeline/action_executions_test.go diff --git a/parity.md b/parity.md index 8c60cceb2..28e42d530 100644 --- a/parity.md +++ b/parity.md @@ -94,3 +94,76 @@ A previous version of this document listed WAFv2, S3 Tables and SES handlers (~5 - `test/integration/appconfigdata_test.go` - `test/integration/apigatewaymanagementapi_test.go` - `test/integration/acmpca_test.go` + +## Bucket A — Missing functionality / stubbed ops (status as of this PR) + +The bucket-A audit ran on an older snapshot; most items had already been +implemented by the time this PR was written. Each was re-verified against the +current code. + +### Implemented in this PR (real state + table-driven tests) + +- **EC2 `RevokeSecurityGroupEgress`** — was a no-op stub that always returned + `Return: true`. Now validates the group exists (`InvalidGroup.NotFound` + otherwise), removes matching egress rules, and is idempotent on absent rules + (mirrors `RevokeSecurityGroupIngress`). `services/ec2/backend_ext.go`, + `handler.go`, `backend_iface.go`. +- **CloudTrail `LookupEvents`** — was a hardcoded empty list. Added an `events` + store + `RecordEvent`, and `LookupEvents` now honors StartTime/EndTime, + LookupAttributes (ANDed; EventId/EventName/EventSource/Username/ReadOnly/ + AccessKeyId/ResourceName/ResourceType), newest-first ordering, MaxResults, and + NextToken pagination. `services/cloudtrail/backend.go`. +- **CodePipeline `ListActionExecutions`** — was an empty stub. `StartPipelineExecution` + now records an `ActionExecution` per action; `ListActionExecutions` returns + them newest-first and supports the `filter.pipelineExecutionId` filter. + `ListRuleTypes` now returns the AWS-managed rule-type catalog. `ListRuleExecutions` + / `ListDeployActionExecutionTargets` return valid empty lists for known + pipelines and `ErrNotFound` otherwise (the emulator does not run condition + rules or model deploy targets). `services/codepipeline/backend.go`, `handler.go`. +- **ApplicationAutoScaling `DescribeScalingActivities`** — was an empty list. + `RegisterScalableTarget` (create + update) now records a `ScalingActivity`; + `DescribeScalingActivities` returns them filtered by ResourceId/ScalableDimension, + newest-first. `services/applicationautoscaling/backend.go`, `handler.go`. + +### Verified already implemented (no change needed) + +- **Lambda** durable-execution ops + capacity-provider ops — real + `durableExecutionStore` state and capacity-provider CRUD with tests + (`services/lambda/handler_stubs.go`, `models.go`, `new_ops_test.go`). +- **SSM** the ~120 "stub" ops route to real backend methods that mutate + region-scoped state (activations, associations, maintenance windows, ops + items, patch baselines) — `services/ssm/backend_stubs.go`. +- **Glue** `GetBlueprintRun`, `GetUsageProfile`, `GetColumnStatisticsTaskRun`, + etc. — real implementations in `services/glue/backend_batch2.go`. +- **Athena** notebook + named-query ops — implemented in + `services/athena/backend_extra.go`. +- **WAFv2** `DescribeManagedRuleGroup` and `GenerateMobileSdkReleaseUrl` have + real handlers; the `nil, nil` ops (DeleteWebACL etc.) are correct empty-body + responses for void-result operations. +- **API Gateway** `GetAccount` / `GetUsage` are wired in the dispatch table. +- **Firehose** Lambda transform (`LambdaInvoker`) + interval flusher + (`StartWorker`) are implemented. +- **CloudFormation** `DescribeType` (registry + built-in schema) and StackSet + drift ops are routed and implemented. +- **OpsWorks** has real handlers (CreateStack etc.); only genuinely + unsupported ops return `UnsupportedOperationException`. + +### Explicitly deferred (still genuine gaps — for the next agent) + +- **AppSync `EvaluateCode`** (`services/appsync/backend.go:2459`) — still returns + a hardcoded `{"evaluationResult":"{}"}`. A faithful implementation requires a + JS interpreter to run the APPSYNC_JS `request`/`response` handler against the + supplied context; a partial heuristic (e.g. echoing `ctx.result`) would be a + half-broken handler, so it was left as-is rather than shipped incorrectly. +- **Kafka** `Update{Connectivity,Monitoring,Rebalancing,Security,Storage}` + (`services/kafka/backend.go:1466+`) — these validate the cluster and create a + real `ClusterOperation` record, but do not persist the specific setting + payloads (the handlers do not parse/store them). Functional but not fully + field-accurate; revisit if cluster setting round-trips are needed. +- **CloudFront** long-tail stub APIs (FieldLevelEncryption, KeyValueStore, + StreamingDistribution, TrustStore, ConnectionFunction, …) in + `services/cloudfront/handler.go` `dispatchStubs*` — still return minimal + empty/``-only XML rather than real per-resource state. High volume; defer. +- **EventBridge Pipes** (`services/pipes/runner.go`) — when an enrichment/target + invoker is unwired the runner returns `nil, nil` (silent skip) rather than + erroring. Low impact; left as-is. diff --git a/services/applicationautoscaling/backend.go b/services/applicationautoscaling/backend.go index 8dda1f3c6..12b353a28 100644 --- a/services/applicationautoscaling/backend.go +++ b/services/applicationautoscaling/backend.go @@ -3,6 +3,7 @@ package applicationautoscaling import ( "fmt" "maps" + "slices" "time" "github.com/google/uuid" @@ -105,15 +106,31 @@ type ScheduledAction struct { // InMemoryBackend stores Application Auto Scaling state in memory. type InMemoryBackend struct { - scalableTargets map[string]*ScalableTarget - scalingPolicies map[string]*ScalingPolicy - scheduledActions map[string]*ScheduledAction - targetARNIndex map[string]string // ARN → scalableTargetKey (secondary index for O(1) lookup) - policyNameIndex map[string]string // policyNameKey → policyARN (secondary index for O(1) lookup) - actionNameIndex map[string]string // actionNameKey → actionARN (secondary index for O(1) lookup) - mu *lockmetrics.RWMutex - accountID string - region string + scalableTargets map[string]*ScalableTarget + scalingPolicies map[string]*ScalingPolicy + scheduledActions map[string]*ScheduledAction + targetARNIndex map[string]string + policyNameIndex map[string]string + actionNameIndex map[string]string + mu *lockmetrics.RWMutex + accountID string + region string + scalingActivities []*ScalingActivity +} + +// ScalingActivity records a capacity-changing activity on a scalable target, +// returned by DescribeScalingActivities. +type ScalingActivity struct { + StartTime time.Time `json:"StartTime"` + EndTime time.Time `json:"EndTime"` + ActivityID string `json:"ActivityId"` + ServiceNamespace string `json:"ServiceNamespace"` + ResourceID string `json:"ResourceId"` + ScalableDimension string `json:"ScalableDimension"` + Description string `json:"Description"` + Cause string `json:"Cause"` + StatusCode string `json:"StatusCode"` + StatusMessage string `json:"StatusMessage"` } // NewInMemoryBackend creates a new InMemoryBackend. @@ -222,12 +239,70 @@ func (b *InMemoryBackend) RegisterScalableTarget( b.scalableTargets[key] = t b.targetARNIndex[t.ARN] = key + + b.recordActivityLocked( + serviceNamespace, resourceID, scalableDimension, + "Setting min/max capacity", + "target registered via RegisterScalableTarget", + now, + ) + cp := *t cp.Tags = maps.Clone(t.Tags) return &cp, nil } +// recordActivityLocked appends a completed scaling activity. The caller must +// hold the write lock. +func (b *InMemoryBackend) recordActivityLocked( + serviceNamespace, resourceID, scalableDimension, description, cause string, + when time.Time, +) { + b.scalingActivities = append(b.scalingActivities, &ScalingActivity{ + ActivityID: uuid.NewString(), + ServiceNamespace: serviceNamespace, + ResourceID: resourceID, + ScalableDimension: scalableDimension, + Description: description, + Cause: cause, + StartTime: when, + EndTime: when, + StatusCode: "Successful", + StatusMessage: "Successfully set desired capacity.", + }) +} + +// DescribeScalingActivities returns recorded scaling activities filtered by the +// optional resourceID and scalableDimension, most recent first. +func (b *InMemoryBackend) DescribeScalingActivities( + serviceNamespace, resourceID, scalableDimension string, +) []*ScalingActivity { + b.mu.RLock("DescribeScalingActivities") + defer b.mu.RUnlock() + + out := make([]*ScalingActivity, 0, len(b.scalingActivities)) + + for _, a := range slices.Backward(b.scalingActivities) { + if serviceNamespace != "" && a.ServiceNamespace != serviceNamespace { + continue + } + + if resourceID != "" && a.ResourceID != resourceID { + continue + } + + if scalableDimension != "" && a.ScalableDimension != scalableDimension { + continue + } + + cp := *a + out = append(out, &cp) + } + + return out +} + // mergeTags merges src into dst enforcing the per-resource tag limit. // dst must be non-nil; callers are responsible for initialising it before the call. // Returns an error if the merge would exceed the limit. @@ -289,6 +364,13 @@ func (b *InMemoryBackend) updateExistingTarget( } } + b.recordActivityLocked( + existing.ServiceNamespace, existing.ResourceID, existing.ScalableDimension, + "Setting min/max capacity", + "target updated via RegisterScalableTarget", + now, + ) + cp := *existing cp.Tags = maps.Clone(existing.Tags) @@ -954,6 +1036,7 @@ func (b *InMemoryBackend) Reset() { b.targetARNIndex = make(map[string]string) b.policyNameIndex = make(map[string]string) b.actionNameIndex = make(map[string]string) + b.scalingActivities = nil } // Purge removes all resources from the backend. It is safe to call concurrently. @@ -967,4 +1050,5 @@ func (b *InMemoryBackend) Purge() { b.targetARNIndex = make(map[string]string) b.policyNameIndex = make(map[string]string) b.actionNameIndex = make(map[string]string) + b.scalingActivities = nil } diff --git a/services/applicationautoscaling/handler.go b/services/applicationautoscaling/handler.go index cdd85907e..1d33e5483 100644 --- a/services/applicationautoscaling/handler.go +++ b/services/applicationautoscaling/handler.go @@ -449,11 +449,22 @@ type describeScalingActivitiesOutput struct { func (h *Handler) handleDescribeScalingActivities( _ context.Context, - _ *describeScalingActivitiesInput, + in *describeScalingActivitiesInput, ) (*describeScalingActivitiesOutput, error) { - // The in-memory backend does not generate synthetic scaling activities. - // Return an empty but non-nil slice so AWS SDK clients receive `[]` not `null`. - return &describeScalingActivitiesOutput{ScalingActivities: []any{}}, nil + if in.ServiceNamespace == "" { + return nil, fmt.Errorf("%w: ServiceNamespace is required", ErrValidation) + } + + activities := h.Backend.DescribeScalingActivities( + in.ServiceNamespace, in.ResourceID, in.ScalableDimension, + ) + + items := make([]any, 0, len(activities)) + for _, a := range activities { + items = append(items, a) + } + + return &describeScalingActivitiesOutput{ScalingActivities: items}, nil } type scalableTargetActionInput struct { diff --git a/services/applicationautoscaling/handler_test.go b/services/applicationautoscaling/handler_test.go index 068d0fe1c..a30186539 100644 --- a/services/applicationautoscaling/handler_test.go +++ b/services/applicationautoscaling/handler_test.go @@ -414,6 +414,48 @@ func TestHandler_DescribeScalingActivities(t *testing.T) { assert.Empty(t, activities) } +func TestHandler_DescribeScalingActivities_AfterRegister(t *testing.T) { + t.Parallel() + + h := newTestHandler(t) + + regRec := doRequest(t, h, "RegisterScalableTarget", map[string]any{ + "ServiceNamespace": "ecs", + "ResourceId": "service/default/my-svc", + "ScalableDimension": "ecs:service:DesiredCount", + "MinCapacity": int32(1), + "MaxCapacity": int32(5), + }) + require.Equal(t, http.StatusOK, regRec.Code) + + rec := doRequest(t, h, "DescribeScalingActivities", map[string]any{ + "ServiceNamespace": "ecs", + "ResourceId": "service/default/my-svc", + "ScalableDimension": "ecs:service:DesiredCount", + }) + require.Equal(t, http.StatusOK, rec.Code) + + var resp map[string]any + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &resp)) + activities, ok := resp["ScalingActivities"].([]any) + require.True(t, ok) + require.Len(t, activities, 1) + + act, ok := activities[0].(map[string]any) + require.True(t, ok) + assert.Equal(t, "ecs", act["ServiceNamespace"]) + assert.Equal(t, "service/default/my-svc", act["ResourceId"]) + assert.Equal(t, "Successful", act["StatusCode"]) +} + +func TestHandler_DescribeScalingActivities_MissingNamespace(t *testing.T) { + t.Parallel() + + h := newTestHandler(t) + rec := doRequest(t, h, "DescribeScalingActivities", map[string]any{}) + assert.Equal(t, http.StatusBadRequest, rec.Code) +} + func TestHandler_PutScheduledAction(t *testing.T) { t.Parallel() diff --git a/services/applicationautoscaling/scaling_activities_test.go b/services/applicationautoscaling/scaling_activities_test.go new file mode 100644 index 000000000..9dd07813f --- /dev/null +++ b/services/applicationautoscaling/scaling_activities_test.go @@ -0,0 +1,107 @@ +package applicationautoscaling_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/applicationautoscaling" +) + +// TestDescribeScalingActivities_TracksRegistrations verifies that registering and +// updating a scalable target records scaling activities and that +// DescribeScalingActivities returns and filters them correctly. +func TestDescribeScalingActivities_TracksRegistrations(t *testing.T) { + t.Parallel() + + const ( + ns = "ecs" + resA = "service/default/svc-a" + resB = "service/default/svc-b" + dimension = "ecs:service:DesiredCount" + ) + + tests := []struct { + name string + filterRes string + filterDim string + wantNamespace string + wantCount int + }{ + { + name: "all_in_namespace", + wantCount: 3, // svc-a registered+updated (2) + svc-b registered (1) + wantNamespace: ns, + }, + { + name: "filter_by_resource_a", + filterRes: resA, + wantCount: 2, + }, + { + name: "filter_by_resource_b", + filterRes: resB, + wantCount: 1, + }, + { + name: "filter_by_resource_and_dimension", + filterRes: resA, + filterDim: dimension, + wantCount: 2, + }, + { + name: "filter_unknown_resource", + filterRes: "service/default/missing", + wantCount: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + b := applicationautoscaling.NewInMemoryBackend("123456789012", "us-east-1") + + // Register svc-a, then update it (RegisterScalableTarget upserts). + _, err := b.RegisterScalableTarget(ns, resA, dimension, 1, 5, nil, "", nil) + require.NoError(t, err) + _, err = b.RegisterScalableTarget(ns, resA, dimension, 2, 10, nil, "", nil) + require.NoError(t, err) + + // Register svc-b once. + _, err = b.RegisterScalableTarget(ns, resB, dimension, 1, 3, nil, "", nil) + require.NoError(t, err) + + activities := b.DescribeScalingActivities(ns, tt.filterRes, tt.filterDim) + assert.Len(t, activities, tt.wantCount) + + for _, a := range activities { + assert.Equal(t, ns, a.ServiceNamespace) + assert.Equal(t, "Successful", a.StatusCode) + assert.NotEmpty(t, a.ActivityID) + assert.NotEmpty(t, a.Description) + + if tt.filterRes != "" { + assert.Equal(t, tt.filterRes, a.ResourceID) + } + } + }) + } +} + +// TestDescribeScalingActivities_ResetClears verifies Reset clears recorded activities. +func TestDescribeScalingActivities_ResetClears(t *testing.T) { + t.Parallel() + + b := applicationautoscaling.NewInMemoryBackend("123456789012", "us-east-1") + _, err := b.RegisterScalableTarget( + "ecs", "service/default/svc", "ecs:service:DesiredCount", 1, 5, nil, "", nil, + ) + require.NoError(t, err) + + require.Len(t, b.DescribeScalingActivities("ecs", "", ""), 1) + + b.Reset() + assert.Empty(t, b.DescribeScalingActivities("ecs", "", "")) +} diff --git a/services/cloudtrail/backend.go b/services/cloudtrail/backend.go index a1f92fef5..55ba23e60 100644 --- a/services/cloudtrail/backend.go +++ b/services/cloudtrail/backend.go @@ -6,6 +6,8 @@ import ( "strconv" "time" + "github.com/google/uuid" + "github.com/blackbirdworks/gopherstack/pkgs/arn" "github.com/blackbirdworks/gopherstack/pkgs/awserr" "github.com/blackbirdworks/gopherstack/pkgs/lockmetrics" @@ -202,23 +204,24 @@ type InsightSelector struct { // InMemoryBackend is the in-memory store for CloudTrail resources. type InMemoryBackend struct { - mu *lockmetrics.RWMutex - trails map[string]*Trail + edsByARN map[string]string + dashboardsByName map[string]string trailsByARN map[string]string channels map[string]*Channel - channelsByARN map[string]string // ARN → channel ID - channelsByName map[string]string // name → channel ID + channelsByARN map[string]string + channelsByName map[string]string dashboards map[string]*Dashboard - dashboardsByARN map[string]string // ARN → dashboard ID - dashboardsByName map[string]string // name → dashboard ID - eventDataStores map[string]*EventDataStore - edsByARN map[string]string // ARN → EDS ID - edsByName map[string]string // name → EDS ID queries map[string]*Query + edsByName map[string]string + eventDataStores map[string]*EventDataStore + trails map[string]*Trail + mu *lockmetrics.RWMutex + dashboardsByARN map[string]string resourcePolicies map[string]*ResourcePolicy imports map[string]*Import accountID string region string + events []Event channelCounter int dashboardCounter int edsCounter int @@ -281,6 +284,7 @@ func (b *InMemoryBackend) Reset() { b.queries = make(map[string]*Query) b.resourcePolicies = make(map[string]*ResourcePolicy) b.imports = make(map[string]*Import) + b.events = nil b.channelCounter = 0 b.dashboardCounter = 0 b.edsCounter = 0 @@ -1583,11 +1587,129 @@ type LookupEventsOutput struct { Events []Event } -// LookupEvents returns management events matching the given filters. Since the emulator -// does not actively record events from API calls, this returns the stored events slice -// filtered by the provided lookup attributes and time range. -func (b *InMemoryBackend) LookupEvents(_ LookupEventsInput) LookupEventsOutput { - return LookupEventsOutput{Events: []Event{}} +// RecordEvent stores a management/data event so it can later be returned by +// LookupEvents. The event is assigned an EventID and EventTime if not already set. +func (b *InMemoryBackend) RecordEvent(ev Event) { + b.mu.Lock("RecordEvent") + defer b.mu.Unlock() + + if ev.EventID == "" { + ev.EventID = uuid.NewString() + } + + if ev.EventTime.IsZero() { + ev.EventTime = time.Now().UTC() + } + + b.events = append(b.events, ev) +} + +// lookupAttrMatch reports whether an event matches a single lookup attribute. +func lookupAttrMatch(ev Event, attr LookupAttribute) bool { + switch attr.AttributeKey { + case "EventId": + return ev.EventID == attr.AttributeValue + case "EventName": + return ev.EventName == attr.AttributeValue + case "EventSource": + return ev.EventSource == attr.AttributeValue + case "Username": + return ev.Username == attr.AttributeValue + case "ReadOnly": + return ev.ReadOnly == attr.AttributeValue + case "AccessKeyId": + return ev.AccessKeyID == attr.AttributeValue + case "ResourceName": + for _, r := range ev.Resources { + if r.ResourceName == attr.AttributeValue { + return true + } + } + + return false + case "ResourceType": + for _, r := range ev.Resources { + if r.ResourceType == attr.AttributeValue { + return true + } + } + + return false + default: + return false + } +} + +// eventMatchesFilters reports whether an event passes the time range and all +// lookup attributes (AWS ANDs multiple attributes together). +func eventMatchesFilters(ev Event, input LookupEventsInput) bool { + if input.StartTime != nil && ev.EventTime.Before(*input.StartTime) { + return false + } + + if input.EndTime != nil && ev.EventTime.After(*input.EndTime) { + return false + } + + for _, attr := range input.LookupAttributes { + if !lookupAttrMatch(ev, attr) { + return false + } + } + + return true +} + +// LookupEvents returns recorded events matching the given filters. Events are +// returned newest-first (matching AWS) and honor StartTime/EndTime, the lookup +// attributes (ANDed together), MaxResults, and NextToken pagination. +func (b *InMemoryBackend) LookupEvents(input LookupEventsInput) LookupEventsOutput { + b.mu.RLock("LookupEvents") + defer b.mu.RUnlock() + + matched := make([]Event, 0, len(b.events)) + + for _, ev := range b.events { + if eventMatchesFilters(ev, input) { + matched = append(matched, ev) + } + } + + // Newest-first ordering, as the AWS API returns. + sort.SliceStable(matched, func(i, j int) bool { + return matched[i].EventTime.After(matched[j].EventTime) + }) + + // Decode the NextToken (offset into the matched slice). + start := 0 + + if input.NextToken != "" { + if n, err := strconv.Atoi(input.NextToken); err == nil && n >= 0 && n <= len(matched) { + start = n + } + } + + limit := int(input.MaxResults) + if limit <= 0 || limit > 50 { + limit = 50 + } + + end := start + limit + + var nextToken string + + if end < len(matched) { + nextToken = strconv.Itoa(end) + } else { + end = len(matched) + } + + page := make([]Event, 0, end-start) + if start < len(matched) { + page = append(page, matched[start:end]...) + } + + return LookupEventsOutput{Events: page, NextToken: nextToken} } // PutEDSInsightSelectors sets insight selectors for an event data store. diff --git a/services/cloudtrail/lookup_events_test.go b/services/cloudtrail/lookup_events_test.go new file mode 100644 index 000000000..dee0d8af1 --- /dev/null +++ b/services/cloudtrail/lookup_events_test.go @@ -0,0 +1,187 @@ +package cloudtrail_test + +import ( + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/pkgs/config" + "github.com/blackbirdworks/gopherstack/services/cloudtrail" +) + +// TestLookupEvents_RecordAndFilter verifies that recorded events are returned by +// LookupEvents, honoring lookup attributes, time ranges, ordering, and pagination. +func TestLookupEvents_RecordAndFilter(t *testing.T) { + t.Parallel() + + base := time.Date(2024, 1, 1, 12, 0, 0, 0, time.UTC) + + newBackendWithEvents := func() *cloudtrail.InMemoryBackend { + b := cloudtrail.NewInMemoryBackend("123456789012", config.DefaultRegion) + b.RecordEvent(cloudtrail.Event{ + EventID: "evt-1", + EventName: "RunInstances", + EventSource: "ec2.amazonaws.com", + Username: "alice", + ReadOnly: "false", + AccessKeyID: "AKIA1", + EventTime: base, + Resources: []cloudtrail.EventResource{{ResourceName: "i-123", ResourceType: "AWS::EC2::Instance"}}, + }) + b.RecordEvent(cloudtrail.Event{ + EventID: "evt-2", + EventName: "CreateBucket", + EventSource: "s3.amazonaws.com", + Username: "bob", + ReadOnly: "false", + AccessKeyID: "AKIA2", + EventTime: base.Add(time.Hour), + }) + b.RecordEvent(cloudtrail.Event{ + EventID: "evt-3", + EventName: "DescribeInstances", + EventSource: "ec2.amazonaws.com", + Username: "alice", + ReadOnly: "true", + EventTime: base.Add(2 * time.Hour), + }) + + return b + } + + tests := []struct { + name string + wantIDs []string + input cloudtrail.LookupEventsInput + wantNext bool + }{ + { + name: "no_filters_newest_first", + input: cloudtrail.LookupEventsInput{}, + wantIDs: []string{"evt-3", "evt-2", "evt-1"}, + }, + { + name: "filter_event_name", + input: cloudtrail.LookupEventsInput{ + LookupAttributes: []cloudtrail.LookupAttribute{ + {AttributeKey: "EventName", AttributeValue: "CreateBucket"}, + }, + }, + wantIDs: []string{"evt-2"}, + }, + { + name: "filter_event_source", + input: cloudtrail.LookupEventsInput{ + LookupAttributes: []cloudtrail.LookupAttribute{ + {AttributeKey: "EventSource", AttributeValue: "ec2.amazonaws.com"}, + }, + }, + wantIDs: []string{"evt-3", "evt-1"}, + }, + { + name: "filter_username_and_readonly_anded", + input: cloudtrail.LookupEventsInput{ + LookupAttributes: []cloudtrail.LookupAttribute{ + {AttributeKey: "Username", AttributeValue: "alice"}, + {AttributeKey: "ReadOnly", AttributeValue: "true"}, + }, + }, + wantIDs: []string{"evt-3"}, + }, + { + name: "filter_access_key", + input: cloudtrail.LookupEventsInput{ + LookupAttributes: []cloudtrail.LookupAttribute{ + {AttributeKey: "AccessKeyId", AttributeValue: "AKIA1"}, + }, + }, + wantIDs: []string{"evt-1"}, + }, + { + name: "filter_resource_name", + input: cloudtrail.LookupEventsInput{ + LookupAttributes: []cloudtrail.LookupAttribute{ + {AttributeKey: "ResourceName", AttributeValue: "i-123"}, + }, + }, + wantIDs: []string{"evt-1"}, + }, + { + name: "time_range_excludes_outside", + input: cloudtrail.LookupEventsInput{ + StartTime: new(base.Add(30 * time.Minute)), + EndTime: new(base.Add(90 * time.Minute)), + }, + wantIDs: []string{"evt-2"}, + }, + { + name: "max_results_paginates", + input: cloudtrail.LookupEventsInput{ + MaxResults: 2, + }, + wantIDs: []string{"evt-3", "evt-2"}, + wantNext: true, + }, + { + name: "no_match_returns_empty", + input: cloudtrail.LookupEventsInput{ + LookupAttributes: []cloudtrail.LookupAttribute{ + {AttributeKey: "EventName", AttributeValue: "Nonexistent"}, + }, + }, + wantIDs: []string{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + b := newBackendWithEvents() + out := b.LookupEvents(tt.input) + + gotIDs := make([]string, 0, len(out.Events)) + for _, e := range out.Events { + gotIDs = append(gotIDs, e.EventID) + } + + assert.Equal(t, tt.wantIDs, gotIDs) + + if tt.wantNext { + assert.NotEmpty(t, out.NextToken) + } else { + assert.Empty(t, out.NextToken) + } + }) + } +} + +// TestLookupEvents_NextTokenPagination verifies that the NextToken from one page +// returns the remaining events on the subsequent page. +func TestLookupEvents_NextTokenPagination(t *testing.T) { + t.Parallel() + + b := cloudtrail.NewInMemoryBackend("123456789012", config.DefaultRegion) + base := time.Date(2024, 6, 1, 0, 0, 0, 0, time.UTC) + + for i := range 5 { + b.RecordEvent(cloudtrail.Event{ + EventName: "Op", + EventTime: base.Add(time.Duration(i) * time.Minute), + }) + } + + page1 := b.LookupEvents(cloudtrail.LookupEventsInput{MaxResults: 2}) + require.Len(t, page1.Events, 2) + require.NotEmpty(t, page1.NextToken) + + page2 := b.LookupEvents(cloudtrail.LookupEventsInput{MaxResults: 2, NextToken: page1.NextToken}) + require.Len(t, page2.Events, 2) + require.NotEmpty(t, page2.NextToken) + + page3 := b.LookupEvents(cloudtrail.LookupEventsInput{MaxResults: 2, NextToken: page2.NextToken}) + require.Len(t, page3.Events, 1) + require.Empty(t, page3.NextToken) +} diff --git a/services/codepipeline/action_executions_test.go b/services/codepipeline/action_executions_test.go new file mode 100644 index 000000000..49e84440b --- /dev/null +++ b/services/codepipeline/action_executions_test.go @@ -0,0 +1,134 @@ +package codepipeline_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/codepipeline" +) + +// TestListActionExecutions_TracksExecutions verifies that StartPipelineExecution +// records an action execution per action and that ListActionExecutions returns +// them, optionally filtered by pipelineExecutionId. +func TestListActionExecutions_TracksExecutions(t *testing.T) { + t.Parallel() + + tests := []struct { + filterFn func(execID string) string + name string + wantCount int + wantErr bool + unknownPipe bool + }{ + { + name: "all_executions", + filterFn: func(string) string { return "" }, + wantCount: 2, // two StartPipelineExecution calls, one action each + }, + { + name: "filter_by_execution_id", + filterFn: func(execID string) string { return execID }, + wantCount: 1, + }, + { + name: "filter_unknown_execution_id", + filterFn: func(string) string { return "does-not-exist" }, + wantCount: 0, + }, + { + name: "unknown_pipeline", + unknownPipe: true, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + b := codepipeline.NewInMemoryBackend("000000000000", "us-east-1") + + if tt.unknownPipe { + _, err := b.ListActionExecutions("missing", "") + require.Error(t, err) + + return + } + + _, err := b.CreatePipeline(samplePipeline("ae-pipeline"), nil) + require.NoError(t, err) + + exec1, err := b.StartPipelineExecution("ae-pipeline") + require.NoError(t, err) + _, err = b.StartPipelineExecution("ae-pipeline") + require.NoError(t, err) + + items, err := b.ListActionExecutions("ae-pipeline", tt.filterFn(exec1.PipelineExecutionID)) + require.NoError(t, err) + assert.Len(t, items, tt.wantCount) + + for _, it := range items { + assert.Equal(t, "SourceAction", it["actionName"]) + assert.Equal(t, "Source", it["stageName"]) + assert.Equal(t, "Succeeded", it["status"]) + assert.NotEmpty(t, it["actionExecutionId"]) + } + }) + } +} + +// TestListRuleTypes_ReturnsCatalog verifies that ListRuleTypes returns the +// AWS-managed rule type catalog with well-formed identifiers. +func TestListRuleTypes_ReturnsCatalog(t *testing.T) { + t.Parallel() + + b := codepipeline.NewInMemoryBackend("000000000000", "us-east-1") + types := b.ListRuleTypes() + + require.NotEmpty(t, types) + + for _, rt := range types { + id, ok := rt["id"].(map[string]any) + require.True(t, ok) + assert.Equal(t, "Rule", id["category"]) + assert.Equal(t, "AWS", id["owner"]) + assert.NotEmpty(t, id["provider"]) + assert.NotEmpty(t, id["version"]) + } +} + +// TestListRuleExecutions_KnownAndUnknownPipeline verifies error handling. +func TestListRuleExecutions_KnownAndUnknownPipeline(t *testing.T) { + t.Parallel() + + b := codepipeline.NewInMemoryBackend("000000000000", "us-east-1") + + _, err := b.ListRuleExecutions("missing") + require.Error(t, err) + + _, err = b.CreatePipeline(samplePipeline("re-pipeline"), nil) + require.NoError(t, err) + + items, err := b.ListRuleExecutions("re-pipeline") + require.NoError(t, err) + assert.Empty(t, items) +} + +// TestListDeployActionExecutionTargets_KnownAndUnknown verifies error handling. +func TestListDeployActionExecutionTargets_KnownAndUnknown(t *testing.T) { + t.Parallel() + + b := codepipeline.NewInMemoryBackend("000000000000", "us-east-1") + + _, err := b.ListDeployActionExecutionTargets("missing", "exec-1") + require.Error(t, err) + + _, err = b.CreatePipeline(samplePipeline("dt-pipeline"), nil) + require.NoError(t, err) + + items, err := b.ListDeployActionExecutionTargets("dt-pipeline", "exec-1") + require.NoError(t, err) + assert.Empty(t, items) +} diff --git a/services/codepipeline/backend.go b/services/codepipeline/backend.go index 3b1a9f9ae..675138bb3 100644 --- a/services/codepipeline/backend.go +++ b/services/codepipeline/backend.go @@ -4,6 +4,7 @@ package codepipeline import ( "fmt" "maps" + "slices" "sort" "time" @@ -38,6 +39,17 @@ const ( // kindPipeline is the resource kind string for pipelines. kindPipeline = "pipeline" + + // keyPipelineExecutionID and keyStatus are JSON keys shared across the + // execution-detail response maps. + keyPipelineExecutionID = "pipelineExecutionId" + keyStatus = "status" + + // statusSucceeded is the terminal success status for executions and actions. + statusSucceeded = "Succeeded" + + // ruleOwnerAWS is the owner value for AWS-managed CodePipeline rule types. + ruleOwnerAWS = "AWS" ) var ( @@ -336,6 +348,7 @@ type InMemoryBackend struct { webhookARNIndex map[string]string // ARN → webhook name stageTransitions map[stageTransitionKey]*StageTransitionState executions map[string][]*PipelineExecution // pipelineName → executions + actionExecutions map[string][]*ActionExecution // pipelineName → action executions mu *lockmetrics.RWMutex accountID string region string @@ -352,6 +365,7 @@ func NewInMemoryBackend(accountID, region string) *InMemoryBackend { webhookARNIndex: make(map[string]string), stageTransitions: make(map[stageTransitionKey]*StageTransitionState), executions: make(map[string][]*PipelineExecution), + actionExecutions: make(map[string][]*ActionExecution), accountID: accountID, region: region, mu: lockmetrics.New("codepipeline-" + region), @@ -374,6 +388,7 @@ func (b *InMemoryBackend) Reset() { b.webhookARNIndex = make(map[string]string) b.stageTransitions = make(map[stageTransitionKey]*StageTransitionState) b.executions = make(map[string][]*PipelineExecution) + b.actionExecutions = make(map[string][]*ActionExecution) } func (b *InMemoryBackend) buildPipelineARN(name string) string { @@ -1077,6 +1092,25 @@ func (b *InMemoryBackend) StartPipelineExecution(pipelineName string) (*Pipeline b.executions[pipelineName] = append(b.executions[pipelineName], exec) + // Record an action execution for every action in the pipeline so that + // ListActionExecutions reflects the work performed by this execution. + now := time.Now().UTC() + + for _, stage := range p.Declaration.Stages { + for _, action := range stage.Actions { + ae := &ActionExecution{ + PipelineExecutionID: exec.PipelineExecutionID, + ActionExecutionID: uuid.NewString(), + StageName: stage.Name, + ActionName: action.Name, + Status: statusSucceeded, + StartTime: now, + LastUpdateTime: now, + } + b.actionExecutions[pipelineName] = append(b.actionExecutions[pipelineName], ae) + } + } + cp := *exec return &cp, nil @@ -1484,8 +1518,22 @@ func (b *InMemoryBackend) ListActionTypes() []*CustomActionType { return result } -// ListActionExecutions returns stub action executions for a pipeline. -func (b *InMemoryBackend) ListActionExecutions(pipelineName string) ([]map[string]any, error) { +// ActionExecution records a single action's execution within a pipeline run. +type ActionExecution struct { + StartTime time.Time `json:"startTime"` + LastUpdateTime time.Time `json:"lastUpdateTime"` + PipelineExecutionID string `json:"pipelineExecutionId"` + ActionExecutionID string `json:"actionExecutionId"` + StageName string `json:"stageName"` + ActionName string `json:"actionName"` + Status string `json:"status"` +} + +// ListActionExecutions returns the recorded action executions for a pipeline, +// most recent first. An optional pipelineExecutionId filters to a single run. +func (b *InMemoryBackend) ListActionExecutions( + pipelineName, pipelineExecutionID string, +) ([]map[string]any, error) { b.mu.RLock("ListActionExecutions") defer b.mu.RUnlock() @@ -1493,10 +1541,32 @@ func (b *InMemoryBackend) ListActionExecutions(pipelineName string) ([]map[strin return nil, ErrNotFound } - return []map[string]any{}, nil + stored := b.actionExecutions[pipelineName] + out := make([]map[string]any, 0, len(stored)) + + // Iterate in reverse so the most recent execution appears first. + for _, ae := range slices.Backward(stored) { + if pipelineExecutionID != "" && ae.PipelineExecutionID != pipelineExecutionID { + continue + } + + out = append(out, map[string]any{ + keyPipelineExecutionID: ae.PipelineExecutionID, + "actionExecutionId": ae.ActionExecutionID, + "stageName": ae.StageName, + "actionName": ae.ActionName, + "startTime": float64(ae.StartTime.Unix()), + "lastUpdateTime": float64(ae.LastUpdateTime.Unix()), + keyStatus: ae.Status, + }) + } + + return out, nil } -// ListRuleExecutions returns stub rule executions for a pipeline. +// ListRuleExecutions returns rule executions for a pipeline. The emulator does +// not run condition rules, so this returns an empty (but valid) list for a known +// pipeline and ErrNotFound otherwise. func (b *InMemoryBackend) ListRuleExecutions(pipelineName string) ([]map[string]any, error) { b.mu.RLock("ListRuleExecutions") defer b.mu.RUnlock() @@ -1508,13 +1578,33 @@ func (b *InMemoryBackend) ListRuleExecutions(pipelineName string) ([]map[string] return []map[string]any{}, nil } -// ListRuleTypes returns empty rule types (stub). +// ListRuleTypes returns the AWS-managed CodePipeline rule types. These mirror +// the built-in condition rule providers AWS exposes. func (b *InMemoryBackend) ListRuleTypes() []map[string]any { - return []map[string]any{} + providers := []string{"Deployment", "LambdaInvoke", "CloudWatchAlarm", "VariableCheck"} + + out := make([]map[string]any, 0, len(providers)) + + for _, provider := range providers { + out = append(out, map[string]any{ + "id": map[string]any{ + "category": "Rule", + "owner": ruleOwnerAWS, + "provider": provider, + "version": "1", + }, + }) + } + + return out } -// ListDeployActionExecutionTargets returns stub targets. -func (b *InMemoryBackend) ListDeployActionExecutionTargets(pipelineName, executionID string) ([]map[string]any, error) { +// ListDeployActionExecutionTargets returns the deploy targets for an action +// execution. The emulator does not model deployment targets, so it returns an +// empty (but valid) list for a known pipeline and ErrNotFound otherwise. +func (b *InMemoryBackend) ListDeployActionExecutionTargets( + pipelineName, executionID string, +) ([]map[string]any, error) { b.mu.RLock("ListDeployActionExecutionTargets") defer b.mu.RUnlock() diff --git a/services/codepipeline/handler.go b/services/codepipeline/handler.go index 94f296db7..0d468fc4d 100644 --- a/services/codepipeline/handler.go +++ b/services/codepipeline/handler.go @@ -1557,10 +1557,15 @@ func (h *Handler) handlePutApprovalResult( return &putApprovalResultOutput{}, nil } +type actionExecutionFilter struct { + PipelineExecutionID string `json:"pipelineExecutionId"` +} + type listActionExecutionsInput struct { - PipelineName string `json:"pipelineName"` - NextToken string `json:"nextToken"` - MaxResults int32 `json:"maxResults"` + Filter *actionExecutionFilter `json:"filter"` + PipelineName string `json:"pipelineName"` + NextToken string `json:"nextToken"` + MaxResults int32 `json:"maxResults"` } type listActionExecutionsOutput struct { @@ -1575,7 +1580,12 @@ func (h *Handler) handleListActionExecutions( return nil, fmt.Errorf("%w: pipelineName is required", errInvalidRequest) } - items, err := h.Backend.ListActionExecutions(in.PipelineName) + var execFilter string + if in.Filter != nil { + execFilter = in.Filter.PipelineExecutionID + } + + items, err := h.Backend.ListActionExecutions(in.PipelineName, execFilter) if err != nil { return nil, err } diff --git a/services/ec2/backend_ext.go b/services/ec2/backend_ext.go index a6ea20741..ef5dfeb1f 100644 --- a/services/ec2/backend_ext.go +++ b/services/ec2/backend_ext.go @@ -1204,6 +1204,29 @@ func (b *InMemoryBackend) RevokeSecurityGroupIngress( return nil } +// RevokeSecurityGroupEgress removes matching egress rules from a security group. +// It validates the group exists (returning InvalidGroup.NotFound otherwise) and +// removes each rule that matches. Like the AWS API, revoking a rule that is not +// present is not an error — the operation is idempotent on the rule set. +func (b *InMemoryBackend) RevokeSecurityGroupEgress( + groupID string, + rules []SecurityGroupRule, +) error { + b.mu.Lock("RevokeSecurityGroupEgress") + defer b.mu.Unlock() + + sg, ok := b.securityGroups[groupID] + if !ok { + return fmt.Errorf("%w: %s", ErrSecurityGroupNotFound, groupID) + } + + for _, rule := range rules { + sg.EgressRules = removeRule(sg.EgressRules, rule) + } + + return nil +} + // removeRule removes matching SecurityGroupRule entries from a slice. func removeRule(rules []SecurityGroupRule, target SecurityGroupRule) []SecurityGroupRule { out := rules[:0] diff --git a/services/ec2/backend_ext_test.go b/services/ec2/backend_ext_test.go index a858f009b..77998010c 100644 --- a/services/ec2/backend_ext_test.go +++ b/services/ec2/backend_ext_test.go @@ -933,9 +933,12 @@ func TestSecurityGroupRuleOperations(t *testing.T) { {name: "authorize_ingress", op: "auth_ingress", wantErr: false}, {name: "authorize_egress", op: "auth_egress", wantErr: false}, {name: "revoke_ingress", op: "revoke_ingress", wantErr: false}, + {name: "revoke_egress", op: "revoke_egress", wantErr: false}, + {name: "revoke_egress_idempotent", op: "revoke_egress_idempotent", wantErr: false}, {name: "authorize_ingress_bad_sg", op: "auth_ingress_bad_sg", wantErr: true}, {name: "authorize_egress_bad_sg", op: "auth_egress_bad_sg", wantErr: true}, {name: "revoke_ingress_bad_sg", op: "revoke_ingress_bad_sg", wantErr: true}, + {name: "revoke_egress_bad_sg", op: "revoke_egress_bad_sg", wantErr: true}, } for _, tt := range tests { @@ -980,6 +983,31 @@ func TestSecurityGroupRuleOperations(t *testing.T) { require.Len(t, sgs, 1) assert.Empty(t, sgs[0].IngressRules) + case "revoke_egress": + sg, err := b.CreateSecurityGroup("test-sg-revoke-egr", "test", "vpc-default") + require.NoError(t, err) + err = b.AuthorizeSecurityGroupEgress(sg.ID, []ec2.SecurityGroupRule{rule}) + require.NoError(t, err) + err = b.RevokeSecurityGroupEgress(sg.ID, []ec2.SecurityGroupRule{rule}) + require.NoError(t, err) + sgs := b.DescribeSecurityGroups([]string{sg.ID}) + require.Len(t, sgs, 1) + assert.Empty(t, sgs[0].EgressRules) + + case "revoke_egress_idempotent": + // Revoking a rule that was never added must succeed without error. + sg, err := b.CreateSecurityGroup("test-sg-revoke-egr-idem", "test", "vpc-default") + require.NoError(t, err) + err = b.RevokeSecurityGroupEgress(sg.ID, []ec2.SecurityGroupRule{rule}) + require.NoError(t, err) + sgs := b.DescribeSecurityGroups([]string{sg.ID}) + require.Len(t, sgs, 1) + assert.Empty(t, sgs[0].EgressRules) + + case "revoke_egress_bad_sg": + err := b.RevokeSecurityGroupEgress("sg-nonexistent", []ec2.SecurityGroupRule{rule}) + require.Error(t, err) + case "auth_ingress_bad_sg": err := b.AuthorizeSecurityGroupIngress( "sg-nonexistent", @@ -1428,6 +1456,28 @@ func TestHandlerExtOperations(t *testing.T) { wantCode: http.StatusOK, wantContains: []string{"RevokeSecurityGroupIngressResponse"}, }, + { + name: "RevokeSecurityGroupEgress_success", + setupFn: func(h *ec2.Handler) string { + sg, _ := h.Backend.CreateSecurityGroup("test-sg-revoke-egr-h", "test", "vpc-default") + _ = h.Backend.AuthorizeSecurityGroupEgress(sg.ID, []ec2.SecurityGroupRule{ + {Protocol: "tcp", FromPort: 443, ToPort: 443, IPRange: "0.0.0.0/0"}, + }) + + return "Action=RevokeSecurityGroupEgress&Version=2016-11-15" + + "&GroupId=" + sg.ID + + "&IpPermissions.1.IpProtocol=tcp&IpPermissions.1.FromPort=443" + + "&IpPermissions.1.ToPort=443&IpPermissions.1.IpRanges.1.CidrIp=0.0.0.0/0" + }, + wantCode: http.StatusOK, + wantContains: []string{"RevokeSecurityGroupEgressResponse"}, + }, + { + name: "RevokeSecurityGroupEgress_missing_group_id", + body: "Action=RevokeSecurityGroupEgress&Version=2016-11-15", + wantCode: http.StatusBadRequest, + wantContains: []string{"InvalidParameterValue"}, + }, { // StartInstances on a running instance must fail with IncorrectInstanceState name: "StartInstances_invalid_state", diff --git a/services/ec2/backend_iface.go b/services/ec2/backend_iface.go index eba263ade..ca7d6057c 100644 --- a/services/ec2/backend_iface.go +++ b/services/ec2/backend_iface.go @@ -77,6 +77,9 @@ type Backend interface { // RevokeSecurityGroupIngress removes matching ingress rules from a security group. RevokeSecurityGroupIngress(groupID string, rules []SecurityGroupRule) error + // RevokeSecurityGroupEgress removes matching egress rules from a security group. + RevokeSecurityGroupEgress(groupID string, rules []SecurityGroupRule) error + // ---- VPCs ---- // DescribeVpcs returns VPCs, optionally filtered by IDs. diff --git a/services/ec2/handler.go b/services/ec2/handler.go index 603370739..cf99009e2 100644 --- a/services/ec2/handler.go +++ b/services/ec2/handler.go @@ -879,9 +879,20 @@ func (h *Handler) handleDeleteSubnet(vals url.Values, reqID string) (any, error) }, nil } -// handleRevokeSecurityGroupEgress is a no-op stub. +// handleRevokeSecurityGroupEgress removes matching egress rules from a security group. // Terraform calls this to revoke the default egress rule when creating a security group. -func (h *Handler) handleRevokeSecurityGroupEgress(_ url.Values, reqID string) (any, error) { +func (h *Handler) handleRevokeSecurityGroupEgress(vals url.Values, reqID string) (any, error) { + groupID := vals.Get("GroupId") + if groupID == "" { + return nil, fmt.Errorf("%w: GroupId is required", ErrInvalidParameter) + } + + rules := parseIPPermissions(vals) + + if err := h.Backend.RevokeSecurityGroupEgress(groupID, rules); err != nil { + return nil, err + } + return &revokeSecurityGroupEgressResponse{ Xmlns: ec2XMLNS, RequestID: reqID, From b4684b60b33f9a515ad0f45fc30988635a07aa1f Mon Sep 17 00:00:00 2001 From: mayor Date: Wed, 10 Jun 2026 16:12:56 -0500 Subject: [PATCH 4/6] perf(parity): bucket C internal optimizations (eventbridge/kms/opensearch/forecast/ecs) Internal hot-path optimizations from the full-surface parity audit (bucket C), no observable behavior change: - eventbridge: deliverEvents no longer deep-copies all bus rules/index/targets per PutEvents; build a scoped delivery plan under the read lock that snapshots only matched rules' targets for the referenced buses. - kms: add grantsByToken index so findGrantByToken is O(1) on the encrypt/decrypt grant-validation path instead of scanning all grants; index kept consistent on create/revoke/retire/reset. - opensearch: add domainPackages reverse index so ListPackagesForDomain and the DeleteDomain cascade no longer scan every package's association set. - forecast: lookupLocked resolves by name (map key) or by reversing the deterministic ARN to the name, replacing the per-call O(n) scan. - ecs: preallocate the reconciler service snapshot to the exact service count. Adds focused index-consistency tests for the kms grant-token index and the opensearch domain->packages reverse index. Co-Authored-By: Claude Opus 4.8 --- services/ecs/backend.go | 9 +- services/eventbridge/delivery.go | 151 +++++++----------- services/forecast/backend.go | 28 +++- services/kms/backend.go | 35 ++-- services/kms/grant_token_index_test.go | 87 ++++++++++ services/opensearch/backend.go | 96 ++++++----- .../opensearch/domain_packages_index_test.go | 84 ++++++++++ 7 files changed, 346 insertions(+), 144 deletions(-) create mode 100644 services/kms/grant_token_index_test.go create mode 100644 services/opensearch/domain_packages_index_test.go diff --git a/services/ecs/backend.go b/services/ecs/backend.go index 59d57c0e7..5affd8784 100644 --- a/services/ecs/backend.go +++ b/services/ecs/backend.go @@ -1447,7 +1447,14 @@ func (b *InMemoryBackend) getServicesForReconciler() []serviceSnapshot { b.mu.RLock("GetServicesForReconciler") defer b.mu.RUnlock() - var out []serviceSnapshot + // Preallocate to the exact service count so the snapshot does not repeatedly + // grow/reallocate the backing array on every reconcile tick. + total := 0 + for _, svcs := range b.services { + total += len(svcs) + } + + out := make([]serviceSnapshot, 0, total) for clusterName, svcs := range b.services { for _, svc := range svcs { diff --git a/services/eventbridge/delivery.go b/services/eventbridge/delivery.go index c732ef673..c831af4f0 100644 --- a/services/eventbridge/delivery.go +++ b/services/eventbridge/delivery.go @@ -80,18 +80,47 @@ func (b *InMemoryBackend) deliverEvents( targets DeliveryTargets, timeout time.Duration, ) { - b.mu.RLock("deliverEvents") - // Deep copy rules and targets within the lock so concurrent mutations to the - // inner maps (PutRule/DeleteRule/PutTargets/RemoveTargets) cannot race with - // the iteration below. - rules := b.rulesStore(region) - targetsStore := b.targetsStore(region) - busRules := deepCopyBusRules(rules) - busRuleIndex := deepCopyRuleIndex(b.ruleIndexStore(region), busRules) - busTargets := deepCopyBusTargets(targetsStore) + groups := b.buildDeliveryPlan(region, entries) + + // Deliver outside the lock. Targets within a rule run concurrently; each + // gets its own bounded context so a hung downstream service cannot block + // the goroutine beyond the configured timeout. + for _, g := range groups { + var wg sync.WaitGroup + for _, t := range g.targets { + target := t + envelope := g.envelope + wg.Go(func() { + deliverToTargetBounded(ctx, target, envelope, targets, timeout) + }) + } + wg.Wait() + } +} + +// deliveryGroup is one matched rule's delivery work: a shared event envelope and +// the snapshot of targets to deliver it to. +type deliveryGroup struct { + envelope map[string]any + targets []*Target +} + +// buildDeliveryPlan matches each entry against its bus under the read lock and +// returns one delivery group per matched rule. Rather than deep-copying every +// bus's rules, index and targets on the hot path, it snapshots only the matched +// rules' targets, bounding per-PutEvents work to the buses the entries reference +// and the rules that matched. Snapshotting under the lock lets delivery run +// without racing concurrent mutations (PutRule/DeleteRule/PutTargets/RemoveTargets). +// Groups preserve the original rule-by-rule, entry-by-entry ordering. +func (b *InMemoryBackend) buildDeliveryPlan(region string, entries []EventEntry) []deliveryGroup { + b.mu.RLock("buildDeliveryPlan") + defer b.mu.RUnlock() + accountID := b.accountID - b.mu.RUnlock() + ruleIndex := b.ruleIndexStore(region) + targetsStore := b.targetsStore(region) + var groups []deliveryGroup for _, entry := range entries { busName := entry.EventBusName if busName == "" { @@ -100,38 +129,42 @@ func (b *InMemoryBackend) deliverEvents( busKey := ebBusKey(busName) eventEnvelope := buildEventEnvelope(entry) - rules := indexedRulesForEvent(busRuleIndex[busKey], entry.Source, entry.DetailType) - for _, rule := range rules { - if rule.State != "ENABLED" { + for _, rule := range indexedRulesForEvent(ruleIndex[busKey], entry.Source, entry.DetailType) { + if rule.State != "ENABLED" || rule.EventPattern == "" { continue } - if rule.EventPattern == "" { + if !matchCompiledPattern(rule.compiledPattern, eventEnvelope) { continue } - if !matchCompiledPattern(rule.compiledPattern, eventEnvelope) { + storedTargets := targetsStore[b.targetKey(busName, rule.Name)] + if len(storedTargets) == 0 { continue } // Build the delivery envelope once per matched rule so all targets // for this rule share the same event id, matching AWS behaviour. - deliveryEnvelope := buildDeliveryEnvelope(entry, accountID, region) - - // Deliver to all targets for this rule. Each target gets its own - // bounded context so a hung downstream service cannot block the - // goroutine beyond the configured timeout. - key := b.targetKey(busName, rule.Name) - var wg sync.WaitGroup - for _, t := range busTargets[key] { - target := t - wg.Go(func() { - deliverToTargetBounded(ctx, target, deliveryEnvelope, targets, timeout) - }) - } - wg.Wait() + groups = append(groups, deliveryGroup{ + envelope: buildDeliveryEnvelope(entry, accountID, region), + targets: snapshotTargets(storedTargets), + }) } } + + return groups +} + +// snapshotTargets returns copies of the stored target structs so delivery cannot +// race a concurrent PutTargets/RemoveTargets mutating the stored values. +func snapshotTargets(stored map[string]*Target) []*Target { + out := make([]*Target, 0, len(stored)) + for _, t := range stored { + targetCopy := *t + out = append(out, &targetCopy) + } + + return out } const ( @@ -237,50 +270,6 @@ func sendToDLQ( } } -// deepCopyBusRules returns a deep copy of the bus-to-rules map so that the -// caller can iterate it without holding any lock. -func deepCopyBusRules(src map[string]map[string]*Rule) map[string]map[string]*Rule { - dst := make(map[string]map[string]*Rule, len(src)) - for bus, ruleMap := range src { - cp := make(map[string]*Rule, len(ruleMap)) - for k, v := range ruleMap { - r := *v - cp[k] = &r - } - dst[bus] = cp - } - - return dst -} - -func deepCopyRuleIndex( - src map[string]map[ruleIndexKey]map[string]*Rule, - rules map[string]map[string]*Rule, -) map[string]map[ruleIndexKey]map[string]*Rule { - dst := make(map[string]map[ruleIndexKey]map[string]*Rule, len(src)) - for bus, indexMap := range src { - copiedBusRules := rules[bus] - copiedIndexMap := make(map[ruleIndexKey]map[string]*Rule, len(indexMap)) - for indexKey, ruleMap := range indexMap { - copiedRuleMap := make(map[string]*Rule, len(ruleMap)) - for name := range ruleMap { - rule, exists := copiedBusRules[name] - if exists { - copiedRuleMap[name] = rule - } - } - if len(copiedRuleMap) > 0 { - copiedIndexMap[indexKey] = copiedRuleMap - } - } - if len(copiedIndexMap) > 0 { - dst[bus] = copiedIndexMap - } - } - - return dst -} - func indexedRulesForEvent( index map[ruleIndexKey]map[string]*Rule, source, detailType string, @@ -310,22 +299,6 @@ func indexedRulesForEvent( return rules } -// deepCopyBusTargets returns a deep copy of the target-key-to-targets map so -// that the caller can iterate it without holding any lock. -func deepCopyBusTargets(src map[string]map[string]*Target) map[string]map[string]*Target { - dst := make(map[string]map[string]*Target, len(src)) - for key, targetMap := range src { - cp := make(map[string]*Target, len(targetMap)) - for k, v := range targetMap { - t := *v - cp[k] = &t - } - dst[key] = cp - } - - return dst -} - // buildEventEnvelope creates a JSON string representing the normalized event for pattern matching. func buildEventEnvelope(entry EventEntry) string { envelope := map[string]any{ diff --git a/services/forecast/backend.go b/services/forecast/backend.go index ef095ca7d..806d5514f 100644 --- a/services/forecast/backend.go +++ b/services/forecast/backend.go @@ -234,8 +234,18 @@ func (b *InMemoryBackend) ensureKind(kind resourceKind) map[string]*Resource { } func (b *InMemoryBackend) lookupLocked(kind resourceKind, nameOrARN string) (*Resource, bool) { - for name, resource := range b.resources[kind] { - if name == nameOrARN || resource.ARN == nameOrARN { + items := b.resources[kind] + + // Fast path: the map is keyed by name, so a name lookup is O(1). + if resource, ok := items[nameOrARN]; ok { + return resource, true + } + + // ARN lookup: every ARN is built deterministically as + // arn:...:forecast:region:account:/, so reverse it to the name + // and look that up directly rather than scanning the whole kind map. + if name, ok := b.nameFromARN(kind, nameOrARN); ok { + if resource, found := items[name]; found && resource.ARN == nameOrARN { return resource, true } } @@ -243,6 +253,20 @@ func (b *InMemoryBackend) lookupLocked(kind resourceKind, nameOrARN string) (*Re return nil, false } +// nameFromARN extracts the resource name from a Forecast ARN of the form +// arn:...:forecast:region:account:/. It returns false if the string +// is not an ARN with the expected "/" resource prefix. +func (b *InMemoryBackend) nameFromARN(kind resourceKind, candidate string) (string, bool) { + prefix := arn.Build("forecast", b.region, b.accountID, string(kind)+"/") + + name, found := strings.CutPrefix(candidate, prefix) + if !found || name == "" { + return "", false + } + + return name, true +} + func newEvaluation(monitor *Resource) MonitorEvaluation { return MonitorEvaluation{ CreationTime: monitor.CreatedAt, diff --git a/services/kms/backend.go b/services/kms/backend.go index a09f59028..1001d2406 100644 --- a/services/kms/backend.go +++ b/services/kms/backend.go @@ -235,9 +235,13 @@ var _ StorageBackend = (*InMemoryBackend)(nil) // InMemoryBackend is a concurrency-safe in-memory KMS backend. type InMemoryBackend struct { - keys map[string]*Key - aliases map[string]*Alias - grants map[string]*Grant + keys map[string]*Key + aliases map[string]*Alias + grants map[string]*Grant + // grantsByToken indexes grants by their GrantToken for O(1) lookup on the + // encrypt/decrypt grant-validation hot path. Kept consistent with grants on + // every create/revoke/retire. + grantsByToken map[string]*Grant policies map[string]string keyMaterials map[string]*keyMaterial keyMaterialHistory map[string][]*keyMaterial @@ -259,6 +263,7 @@ func NewInMemoryBackendWithConfig(accountID, region string) *InMemoryBackend { keys: make(map[string]*Key), aliases: make(map[string]*Alias), grants: make(map[string]*Grant), + grantsByToken: make(map[string]*Grant), policies: make(map[string]string), keyMaterials: make(map[string]*keyMaterial), keyMaterialHistory: make(map[string][]*keyMaterial), @@ -1975,6 +1980,7 @@ func (b *InMemoryBackend) CreateGrant(input *CreateGrantInput) (*CreateGrantOutp CreationDate: UnixTimeFloat(now), } b.grants[grantID] = grant + b.grantsByToken[grantToken] = grant return &CreateGrantOutput{GrantID: grantID, GrantToken: grantToken}, nil } @@ -2011,10 +2017,8 @@ func grantConstraintsSatisfied(c *GrantConstraints, encCtx map[string]string) bo // Must be called with at least a read lock held. func (b *InMemoryBackend) findGrantByToken(grantTokens []string) *Grant { for _, token := range grantTokens { - for _, g := range b.grants { - if g.GrantToken == token { - return g - } + if g, ok := b.grantsByToken[token]; ok { + return g } } @@ -2124,6 +2128,7 @@ func (b *InMemoryBackend) RevokeGrant(input *RevokeGrantInput) error { } delete(b.grants, input.GrantID) + delete(b.grantsByToken, grant.GrantToken) return nil } @@ -2134,15 +2139,15 @@ func (b *InMemoryBackend) RetireGrant(input *RetireGrantInput) error { defer b.mu.Unlock() if input.GrantToken != "" { - for grantID, g := range b.grants { - if g.GrantToken == input.GrantToken { - delete(b.grants, grantID) - - return nil - } + g, ok := b.grantsByToken[input.GrantToken] + if !ok { + return ErrGrantNotFound } - return ErrGrantNotFound + delete(b.grants, g.GrantID) + delete(b.grantsByToken, input.GrantToken) + + return nil } if input.GrantID == "" { @@ -2166,6 +2171,7 @@ func (b *InMemoryBackend) RetireGrant(input *RetireGrantInput) error { } delete(b.grants, input.GrantID) + delete(b.grantsByToken, grant.GrantToken) return nil } @@ -2711,6 +2717,7 @@ func (b *InMemoryBackend) Reset() { b.keys = make(map[string]*Key) b.aliases = make(map[string]*Alias) b.grants = make(map[string]*Grant) + b.grantsByToken = make(map[string]*Grant) b.policies = make(map[string]string) b.keyMaterials = make(map[string]*keyMaterial) b.keyMaterialHistory = make(map[string][]*keyMaterial) diff --git a/services/kms/grant_token_index_test.go b/services/kms/grant_token_index_test.go new file mode 100644 index 000000000..9fd496c61 --- /dev/null +++ b/services/kms/grant_token_index_test.go @@ -0,0 +1,87 @@ +package kms_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/kms" +) + +// TestKMSGrantTokenIndexConsistency exercises the grantsByToken index that backs +// findGrantByToken on the encrypt/decrypt hot path. It verifies that a grant +// token authorizes an operation after CreateGrant and stops authorizing it once +// the grant is removed (RevokeGrant / RetireGrant by token / RetireGrant by ID), +// proving the index stays consistent across add and delete. +func TestKMSGrantTokenIndexConsistency(t *testing.T) { + t.Parallel() + + tests := []struct { + // remove deletes the grant via the path under test, given the backend, + // keyID, grantID and grantToken. + remove func(t *testing.T, b *kms.InMemoryBackend, keyID, grantID, grantToken string) + name string + }{ + { + name: "revoke_grant_by_id", + remove: func(t *testing.T, b *kms.InMemoryBackend, keyID, grantID, _ string) { + t.Helper() + require.NoError(t, b.RevokeGrant(&kms.RevokeGrantInput{KeyID: keyID, GrantID: grantID})) + }, + }, + { + name: "retire_grant_by_token", + remove: func(t *testing.T, b *kms.InMemoryBackend, _, _, grantToken string) { + t.Helper() + require.NoError(t, b.RetireGrant(&kms.RetireGrantInput{GrantToken: grantToken})) + }, + }, + { + name: "retire_grant_by_id", + remove: func(t *testing.T, b *kms.InMemoryBackend, keyID, grantID, _ string) { + t.Helper() + require.NoError(t, b.RetireGrant(&kms.RetireGrantInput{KeyID: keyID, GrantID: grantID})) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + b := kms.NewInMemoryBackend() + + key, err := b.CreateKey(&kms.CreateKeyInput{}) + require.NoError(t, err) + keyID := key.KeyMetadata.KeyID + + grant, err := b.CreateGrant(&kms.CreateGrantInput{ + KeyID: keyID, + GranteePrincipal: "arn:aws:iam::000000000000:role/test", + Operations: []string{"Encrypt", "Decrypt"}, + }) + require.NoError(t, err) + require.NotEmpty(t, grant.GrantToken) + + // The token resolves through the index, so the encrypt succeeds. + _, err = b.Encrypt(&kms.EncryptInput{ + KeyID: keyID, + Plaintext: []byte("secret"), + GrantTokens: []string{grant.GrantToken}, + }) + require.NoError(t, err) + + // Remove the grant via the path under test. + tt.remove(t, b, keyID, grant.GrantID, grant.GrantToken) + + // The index must no longer resolve the token: the same encrypt now + // fails with an invalid-grant-token error. + _, err = b.Encrypt(&kms.EncryptInput{ + KeyID: keyID, + Plaintext: []byte("secret"), + GrantTokens: []string{grant.GrantToken}, + }) + require.ErrorIs(t, err, kms.ErrInvalidGrantToken) + }) + } +} diff --git a/services/opensearch/backend.go b/services/opensearch/backend.go index a85c36355..adf32984f 100644 --- a/services/opensearch/backend.go +++ b/services/opensearch/backend.go @@ -475,8 +475,13 @@ type DryRunStatus struct { // InMemoryBackend is the in-memory store for OpenSearch domains. type InMemoryBackend struct { - dnsRegistrar DNSRegistrar - packageAssociations map[string]map[string]bool + dnsRegistrar DNSRegistrar + packageAssociations map[string]map[string]bool + // domainPackages is the reverse index of packageAssociations: domain name → + // set of package IDs associated with it. Kept consistent with + // packageAssociations on every associate/dissociate so ListPackagesForDomain + // and DeleteDomain do not have to scan every package's association set. + domainPackages map[string]map[string]bool arnIndex map[string]string inboundConnections map[string]*InboundConnection outboundConnections map[string]*OutboundConnection @@ -524,6 +529,7 @@ func NewInMemoryBackend(accountID, region string) *InMemoryBackend { domainDataSources: make(map[string]map[string]*DataSource), directQueryDataSources: make(map[string]*DirectQueryDataSource), packageAssociations: make(map[string]map[string]bool), + domainPackages: make(map[string]map[string]bool), vpcAuthorizations: make(map[string][]AuthorizedPrincipal), vpcEndpoints: make(map[string]*VpcEndpoint), applications: make(map[string]*Application), @@ -640,13 +646,15 @@ func (b *InMemoryBackend) DeleteDomain(name string) (*Domain, error) { delete(b.domainDataSources, name) delete(b.vpcAuthorizations, name) - for pkgID, domains := range b.packageAssociations { - delete(domains, name) - - if len(domains) == 0 { - delete(b.packageAssociations, pkgID) + for pkgID := range b.domainPackages[name] { + if domains, ok := b.packageAssociations[pkgID]; ok { + delete(domains, name) + if len(domains) == 0 { + delete(b.packageAssociations, pkgID) + } } } + delete(b.domainPackages, name) if b.dnsRegistrar != nil { b.dnsRegistrar.Deregister(cp.Endpoint) @@ -860,11 +868,7 @@ func (b *InMemoryBackend) AssociatePackage( return nil, fmt.Errorf("%w: domain %s not found", ErrDomainNotFound, domainName) } - if b.packageAssociations[packageID] == nil { - b.packageAssociations[packageID] = make(map[string]bool) - } - - b.packageAssociations[packageID][domainName] = true + b.addPackageAssociation(packageID, domainName) return &DomainPackageDetails{ PackageID: packageID, @@ -873,6 +877,39 @@ func (b *InMemoryBackend) AssociatePackage( }, nil } +// addPackageAssociation records a package↔domain association in both the +// forward (packageAssociations) and reverse (domainPackages) indexes. +// Caller must hold the write lock. +func (b *InMemoryBackend) addPackageAssociation(packageID, domainName string) { + if b.packageAssociations[packageID] == nil { + b.packageAssociations[packageID] = make(map[string]bool) + } + b.packageAssociations[packageID][domainName] = true + + if b.domainPackages[domainName] == nil { + b.domainPackages[domainName] = make(map[string]bool) + } + b.domainPackages[domainName][packageID] = true +} + +// removePackageAssociation removes a package↔domain association from both the +// forward and reverse indexes. Caller must hold the write lock. +func (b *InMemoryBackend) removePackageAssociation(packageID, domainName string) { + if domains, ok := b.packageAssociations[packageID]; ok { + delete(domains, domainName) + if len(domains) == 0 { + delete(b.packageAssociations, packageID) + } + } + + if pkgs, ok := b.domainPackages[domainName]; ok { + delete(pkgs, packageID) + if len(pkgs) == 0 { + delete(b.domainPackages, domainName) + } + } +} + // AssociatePackages associates multiple packages with a domain. func (b *InMemoryBackend) AssociatePackages( domainName string, @@ -896,11 +933,7 @@ func (b *InMemoryBackend) AssociatePackages( results := make([]DomainPackageDetails, 0, len(packageIDs)) for _, pkgID := range packageIDs { - if b.packageAssociations[pkgID] == nil { - b.packageAssociations[pkgID] = make(map[string]bool) - } - - b.packageAssociations[pkgID][domainName] = true + b.addPackageAssociation(pkgID, domainName) results = append(results, DomainPackageDetails{ PackageID: pkgID, DomainName: domainName, @@ -1056,6 +1089,7 @@ func (b *InMemoryBackend) Reset() { b.domainDataSources = make(map[string]map[string]*DataSource) b.directQueryDataSources = make(map[string]*DirectQueryDataSource) b.packageAssociations = make(map[string]map[string]bool) + b.domainPackages = make(map[string]map[string]bool) b.vpcAuthorizations = make(map[string][]AuthorizedPrincipal) b.vpcEndpoints = make(map[string]*VpcEndpoint) b.applications = make(map[string]*Application) @@ -1529,13 +1563,11 @@ func (b *InMemoryBackend) ListPackagesForDomain(domainName string) []*Package { var out []*Package - for pkgID, domains := range b.packageAssociations { - if domains[domainName] { - pkg, exists := b.packages[pkgID] - if exists { - cp := *pkg - out = append(out, &cp) - } + for pkgID := range b.domainPackages[domainName] { + pkg, exists := b.packages[pkgID] + if exists { + cp := *pkg + out = append(out, &cp) } } @@ -2583,13 +2615,7 @@ func (b *InMemoryBackend) DissociatePackage( return nil, fmt.Errorf("%w: domain %s not found", ErrDomainNotFound, domainName) } - if domains, ok := b.packageAssociations[packageID]; ok { - delete(domains, domainName) - - if len(domains) == 0 { - delete(b.packageAssociations, packageID) - } - } + b.removePackageAssociation(packageID, domainName) return &DomainPackageDetails{ PackageID: packageID, @@ -2617,13 +2643,7 @@ func (b *InMemoryBackend) DissociatePackages( results := make([]DomainPackageDetails, 0, len(packageIDs)) for _, pkgID := range packageIDs { - if domains, ok := b.packageAssociations[pkgID]; ok { - delete(domains, domainName) - - if len(domains) == 0 { - delete(b.packageAssociations, pkgID) - } - } + b.removePackageAssociation(pkgID, domainName) results = append(results, DomainPackageDetails{ PackageID: pkgID, diff --git a/services/opensearch/domain_packages_index_test.go b/services/opensearch/domain_packages_index_test.go new file mode 100644 index 000000000..0d12e41ea --- /dev/null +++ b/services/opensearch/domain_packages_index_test.go @@ -0,0 +1,84 @@ +package opensearch_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/opensearch" +) + +// pkgNames extracts and returns the package IDs from a ListPackagesForDomain result. +func pkgNames(pkgs []*opensearch.Package) []string { + out := make([]string, 0, len(pkgs)) + for _, p := range pkgs { + out = append(out, p.PackageID) + } + + return out +} + +// TestDomainPackagesReverseIndexConsistency exercises the domainPackages reverse +// index that backs ListPackagesForDomain and the DeleteDomain cascade. It verifies +// that the domain→packages view stays consistent with the package→domains view +// across associate, dissociate, and delete-domain operations. +func TestDomainPackagesReverseIndexConsistency(t *testing.T) { + t.Parallel() + + t.Run("associate_and_dissociate", func(t *testing.T) { + t.Parallel() + + b := opensearch.NewInMemoryBackend(testAccountID, testRegion) + b.AddDomainInternal("dom-a", "") + b.AddDomainInternal("dom-b", "") + b.AddPackageInternal("pkg-1", "p1", "TXT-DICTIONARY") + b.AddPackageInternal("pkg-2", "p2", "TXT-DICTIONARY") + + // pkg-1 -> dom-a, dom-b ; pkg-2 -> dom-a + _, err := b.AssociatePackage("pkg-1", "dom-a") + require.NoError(t, err) + _, err = b.AssociatePackage("pkg-1", "dom-b") + require.NoError(t, err) + _, err = b.AssociatePackage("pkg-2", "dom-a") + require.NoError(t, err) + + // Reverse index (domain -> packages) must agree with the associations. + assert.ElementsMatch(t, []string{"pkg-1", "pkg-2"}, pkgNames(b.ListPackagesForDomain("dom-a"))) + assert.ElementsMatch(t, []string{"pkg-1"}, pkgNames(b.ListPackagesForDomain("dom-b"))) + + // Forward index (package -> domains) must agree too. + assert.ElementsMatch(t, []string{"dom-a", "dom-b"}, b.ListDomainsForPackage("pkg-1")) + assert.ElementsMatch(t, []string{"dom-a"}, b.ListDomainsForPackage("pkg-2")) + + // Dissociate pkg-1 from dom-a; both directions must update. + _, err = b.DissociatePackage("pkg-1", "dom-a") + require.NoError(t, err) + + assert.ElementsMatch(t, []string{"pkg-2"}, pkgNames(b.ListPackagesForDomain("dom-a"))) + assert.ElementsMatch(t, []string{"dom-b"}, b.ListDomainsForPackage("pkg-1")) + }) + + t.Run("delete_domain_clears_associations", func(t *testing.T) { + t.Parallel() + + b := opensearch.NewInMemoryBackend(testAccountID, testRegion) + b.AddDomainInternal("dom-a", "") + b.AddDomainInternal("dom-b", "") + b.AddPackageInternal("pkg-1", "p1", "TXT-DICTIONARY") + + _, err := b.AssociatePackage("pkg-1", "dom-a") + require.NoError(t, err) + _, err = b.AssociatePackage("pkg-1", "dom-b") + require.NoError(t, err) + + // Deleting dom-a must remove dom-a from the package's domain set and from + // the reverse index, while leaving dom-b intact. + _, err = b.DeleteDomain("dom-a") + require.NoError(t, err) + + assert.Empty(t, b.ListPackagesForDomain("dom-a")) + assert.ElementsMatch(t, []string{"dom-b"}, b.ListDomainsForPackage("pkg-1")) + assert.ElementsMatch(t, []string{"pkg-1"}, pkgNames(b.ListPackagesForDomain("dom-b"))) + }) +} From 50ed9453546c63836aa86ecb0e7eddabe5d999a5 Mon Sep 17 00:00:00 2001 From: mayor Date: Wed, 10 Jun 2026 16:32:09 -0500 Subject: [PATCH 5/6] feat(parity): implement deferred bucket-A residuals (appsync/kafka/pipes) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve the four DEFERRED bucket-A items documented in parity.md: - AppSync EvaluateCode: replace hardcoded {"evaluationResult":"{}"} with a faithful APPSYNC_JS evaluator (no JS engine vendored). Selects request/ response handler, evaluates object/JSON literals, ctx.* member paths, and pure util.* helpers; unsupported constructs return ErrUnsupportedJSCode rather than a fabricated result. New services/appsync/jseval.go + tests. - Kafka Update{Connectivity,Monitoring,Security,Storage}: parse request bodies, persist setting payloads onto the cluster, and record a ClusterOperation with SourceClusterInfo/TargetClusterInfo (new MutableClusterInfo) surfaced by DescribeClusterOperation. UpdateRebalancing is action-only (no AWS-exposed setting). Tests added. - EventBridge Pipes: stop silently dropping events. Unwired enrichment/target invokers now return real errors; failed batches route to the configured DLQ (SQS/SNS) and delete source messages, else retain for redelivery. - CloudFront long-tail: re-audited — already backed by real InMemoryBackend state on this branch; stale deferral note corrected in parity.md. Co-Authored-By: Claude Opus 4.8 --- parity.md | 88 ++++- services/appsync/backend.go | 26 +- services/appsync/jseval.go | 477 +++++++++++++++++++++++++ services/appsync/jseval_test.go | 173 +++++++++ services/kafka/backend.go | 266 ++++++++++++-- services/kafka/handler.go | 88 ++++- services/kafka/interfaces.go | 8 +- services/kafka/update_settings_test.go | 206 +++++++++++ services/pipes/runner.go | 95 ++++- services/pipes/runner_dlq_test.go | 234 ++++++++++++ 10 files changed, 1583 insertions(+), 78 deletions(-) create mode 100644 services/appsync/jseval.go create mode 100644 services/appsync/jseval_test.go create mode 100644 services/kafka/update_settings_test.go create mode 100644 services/pipes/runner_dlq_test.go diff --git a/parity.md b/parity.md index 28e42d530..8a5fbddbd 100644 --- a/parity.md +++ b/parity.md @@ -148,22 +148,72 @@ current code. - **OpsWorks** has real handlers (CreateStack etc.); only genuinely unsupported ops return `UnsupportedOperationException`. -### Explicitly deferred (still genuine gaps — for the next agent) - -- **AppSync `EvaluateCode`** (`services/appsync/backend.go:2459`) — still returns - a hardcoded `{"evaluationResult":"{}"}`. A faithful implementation requires a - JS interpreter to run the APPSYNC_JS `request`/`response` handler against the - supplied context; a partial heuristic (e.g. echoing `ctx.result`) would be a - half-broken handler, so it was left as-is rather than shipped incorrectly. -- **Kafka** `Update{Connectivity,Monitoring,Rebalancing,Security,Storage}` - (`services/kafka/backend.go:1466+`) — these validate the cluster and create a - real `ClusterOperation` record, but do not persist the specific setting - payloads (the handlers do not parse/store them). Functional but not fully - field-accurate; revisit if cluster setting round-trips are needed. -- **CloudFront** long-tail stub APIs (FieldLevelEncryption, KeyValueStore, - StreamingDistribution, TrustStore, ConnectionFunction, …) in - `services/cloudfront/handler.go` `dispatchStubs*` — still return minimal - empty/``-only XML rather than real per-resource state. High volume; defer. -- **EventBridge Pipes** (`services/pipes/runner.go`) — when an enrichment/target - invoker is unwired the runner returns `nil, nil` (silent skip) rather than - erroring. Low impact; left as-is. +### Bucket-A residuals — resolved in this PR + +The four items previously deferred below have now been implemented (real state + +table-driven tests). The original deferral notes are retained for history under +"Original deferral notes" at the end of this section. + +- **AppSync `EvaluateCode`** (`services/appsync/backend.go`, new + `services/appsync/jseval.go`) — no longer returns the hardcoded + `{"evaluationResult":"{}"}`. gopherstack does **not** vendor a JavaScript engine + (no goja/otto/dop251/v8 in `go.mod`), and pulling in a full JS runtime was judged + out of scope for parity work. Instead the backend now runs a faithful evaluator + for the documented APPSYNC_JS patterns that resolvers actually use: it selects the + `request`/`response` handler (honoring the `function` field), extracts the final + `return`, and evaluates object literals, JSON literals, context member + expressions (`ctx.arguments.*`, `ctx.args.*`, `ctx.result`, `ctx.prev.result`, + `ctx.identity`, `ctx.source`, `ctx.stash.*`), and the pure `util.*` helpers + (`util.toJson`, `util.parseJson`, `util.error`, `util.appendError`, + `util.unauthorized`). The return value is JSON-serialized into `evaluationResult`. + **Honest limitation:** constructs outside that set (arbitrary JS expressions, + control flow, loops, local variable bindings, string concatenation, etc.) return + `ErrUnsupportedJSCode` (a `BadRequestException`) rather than a fabricated result, + so callers can distinguish "evaluated" from "not supported by the emulator". A + full JS engine remains the only way to support arbitrary APPSYNC_JS; that is + deliberately not done here. Tests: `services/appsync/jseval_test.go`. +- **Kafka** `Update{Connectivity,Monitoring,Security,Storage}` + (`services/kafka/backend.go`) — these now parse the request body (handlers in + `handler.go`), persist the specific setting payloads onto the cluster + (`ConnectivityInfo`, `EnhancedMonitoring`/`OpenMonitoring`/`LoggingInfo`, + `ClientAuthentication`/`EncryptionInfo`, `StorageMode`/EBS volume size + + provisioned throughput), and record a `ClusterOperation` whose new + `SourceClusterInfo`/`TargetClusterInfo` (`MutableClusterInfo`) reflect the + before/after state. `DescribeClusterOperation` returns those. `UpdateRebalancing` + is an action with no AWS-exposed per-field setting, so it validates the cluster + and records the operation without source/target info (documented in code). Tests: + `services/kafka/update_settings_test.go`. +- **CloudFront** long-tail families — re-audited against the current branch: the + named families (FieldLevelEncryption, KeyValueStore, StreamingDistribution, + TrustStore, ConnectionFunction, AnycastIPList, ConnectionGroup, resource policy, + the `ListDistributionsBy-*` family, distribution tenants, monitoring + subscriptions, etc.) are **already backed by real `InMemoryBackend` state** with + CRUD, ETag/IfMatch, and AWS-accurate not-found error codes (see `backend.go`, + `backend_batch2.go`, `backend_new_ops.go`, `handler_new_ops.go`, + `handler_batch2.go`). The `cfStubHelpers` minimal-XML closures in + `handler.go`'s `dispatchStubs*` are now dead scaffolding — every operation routes + to a real handler. The earlier "still return empty/``-only XML" note was stale + relative to the D/B/A/C stack already on this branch. No further CloudFront work + was required. +- **EventBridge Pipes** (`services/pipes/runner.go`) — the runner no longer + silently drops events. `invokeEnrichment` returns `ErrEnrichmentInvokerUnwired` + (invoker not configured) or `ErrUnsupportedPipeEnrichment` (unknown ARN type) + instead of `nil, nil`; all target invokers return `ErrTargetInvokerUnwired` when + their invoker is unwired instead of a false success. On enrichment or target + failure the runner routes the failed batch to the pipe's configured dead-letter + queue (`DeadLetterConfig.Arn`, SQS or SNS) via the new `handlePipeFailure` / + `sendToDLQ` helpers and deletes the source messages; with no DLQ configured the + source messages are retained for the source service's own redelivery. Tests: + `services/pipes/runner_dlq_test.go`. + +#### Original deferral notes (historical) + +- **AppSync `EvaluateCode`** — returned a hardcoded `{"evaluationResult":"{}"}`; + deferred because a faithful implementation seemingly required a JS interpreter. +- **Kafka** `Update{Connectivity,Monitoring,Rebalancing,Security,Storage}` — + validated the cluster and created a `ClusterOperation` but did not persist the + setting payloads. +- **CloudFront** long-tail stub APIs — believed to return minimal empty/``-only + XML; superseded by the real implementations already present on this branch. +- **EventBridge Pipes** — unwired enrichment/target invoker returned `nil, nil` + (silent skip). diff --git a/services/appsync/backend.go b/services/appsync/backend.go index aa6e4f43c..34b156d04 100644 --- a/services/appsync/backend.go +++ b/services/appsync/backend.go @@ -34,6 +34,11 @@ var ( ErrInvalidSchema = errors.New("InvalidSchemaError") // ErrValidation is returned when input validation fails. ErrValidation = awserr.New("BadRequestException", awserr.ErrInvalidParameter) + // ErrUnsupportedJSCode is returned when EvaluateCode is given an APPSYNC_JS + // construct the emulator's evaluator does not support. The request is + // well-formed, but the code uses features beyond the documented patterns the + // emulator faithfully evaluates (the emulator does not embed a JS engine). + ErrUnsupportedJSCode = awserr.New("BadRequestException", awserr.ErrInvalidParameter) ) // LambdaInvoker can invoke a Lambda function by name or ARN. @@ -2456,16 +2461,27 @@ func (b *InMemoryBackend) EvaluateMappingTemplate(template, contextJSON string) return out, nil } -// EvaluateCode evaluates APPSYNC_JS code (basic stub — returns empty result). -func (b *InMemoryBackend) EvaluateCode(code, _, _, _ string) (string, error) { +// EvaluateCode evaluates an APPSYNC_JS module against the supplied context and +// returns the JSON-stringified return value of the selected handler. +// +// gopherstack does not embed a JavaScript engine, so this evaluates the documented +// APPSYNC_JS patterns directly (see appsync_js.go). Constructs beyond that set +// return ErrUnsupportedJSCode rather than a fabricated result, so callers can +// distinguish "evaluated" from "not supported by the emulator". +func (b *InMemoryBackend) EvaluateCode(code, contextJSON, function, runtime string) (string, error) { if code == "" { return "", fmt.Errorf("%w: code is required", ErrValidation) } - // Stub: return an empty evaluationResult. - result := `{"evaluationResult":"{}"}` + if runtime != "" && runtime != "APPSYNC_JS" { + return "", fmt.Errorf("%w: unsupported runtime %q", ErrValidation, runtime) + } + + if function != "" && function != jsHandlerRequest && function != jsHandlerResponse { + return "", fmt.Errorf("%w: function must be 'request' or 'response'", ErrValidation) + } - return result, nil + return evaluateAppSyncJS(code, contextJSON, function) } // StartDataSourceIntrospection starts an introspection job for a data source. diff --git a/services/appsync/jseval.go b/services/appsync/jseval.go new file mode 100644 index 000000000..7a249a132 --- /dev/null +++ b/services/appsync/jseval.go @@ -0,0 +1,477 @@ +package appsync + +import ( + "encoding/json" + "fmt" + "regexp" + "strconv" + "strings" +) + +// AppSync APPSYNC_JS evaluation. +// +// AWS's EvaluateCode runs the exported `request` or `response` handler from the +// supplied APPSYNC_JS module against the provided context object and returns the +// JSON-stringified return value as `evaluationResult`. A full implementation +// requires a JavaScript engine; gopherstack does not vendor one (no goja/otto/v8 +// in go.mod, and adding a heavy JS runtime is out of scope for the parity work). +// +// Instead this implements a faithful evaluator for the documented APPSYNC_JS +// patterns that resolvers and functions actually use in practice: +// +// - Selecting the handler to run (`function` arg, else `request`, else `response`). +// - `return ;` where the object's values are JSON literals or +// context member expressions (ctx.arguments.x, ctx.args.x, ctx.result, +// ctx.prev.result, ctx.identity, ctx.source, ctx.stash). +// - `return ;` (e.g. `return ctx.result;`). +// - `return ;` (object, array, string, number, bool, null). +// - `return util.(...)` for the common pure helpers: util.toJson, +// util.parseJson, util.error, util.appendError, util.unauthorized. +// +// Anything outside this set (control flow, arbitrary JS expressions, loops, +// variable bindings, string concatenation, etc.) returns ErrUnsupportedJSCode +// rather than a fabricated empty result, so callers can tell the difference +// between "evaluated" and "not supported". + +const ( + jsHandlerRequest = "request" + jsHandlerResponse = "response" +) + +var ( + // reExportedFunc matches `export function () { }` and the + // `function (...)` form. Body capture is greedy to the matching final brace + // of the module-level function (handled by brace-balancing in extractFuncBody). + reExportedFunc = regexp.MustCompile(`(?s)(?:export\s+)?function\s+(request|response)\s*\(([^)]*)\)\s*\{`) + // reReturnStmt matches a `return ;` statement (expr may be empty for `return;`). + reReturnStmt = regexp.MustCompile(`(?s)\breturn\b\s*(.*?);`) + // reCtxParamName captures the first parameter name of the handler (the context binding). + reCtxParamName = regexp.MustCompile(`^\s*(\w+)`) +) + +// jsContext mirrors the AppSync request/response context object that the handler +// receives. Unknown JSON keys are preserved in Raw for member resolution. +type jsContext struct { + Raw map[string]any + Arguments map[string]any + Args map[string]any + Result any + Prev map[string]any + Identity any + Source any + Stash map[string]any +} + +// evaluateAppSyncJS evaluates an APPSYNC_JS module against the supplied context +// JSON and returns the JSON-stringified handler return value. +func evaluateAppSyncJS(code, contextJSON, function string) (string, error) { + jc, err := parseJSContext(contextJSON) + if err != nil { + return "", err + } + + body, paramName, err := selectHandlerBody(code, function) + if err != nil { + return "", err + } + + retExpr, ok := lastReturnExpr(body) + if !ok || strings.TrimSpace(retExpr) == "" { + // A handler with no return (or a bare `return;`) yields JS undefined, which + // AppSync serializes as null. + return "null", nil + } + + val, err := evalJSExpr(retExpr, paramName, jc) + if err != nil { + return "", err + } + + out, mErr := json.Marshal(val) + if mErr != nil { + return "", fmt.Errorf("%w: result is not JSON-serializable", ErrValidation) + } + + return string(out), nil +} + +// parseJSContext decodes the context JSON into a jsContext. An empty context is valid. +func parseJSContext(contextJSON string) (*jsContext, error) { + jc := &jsContext{Raw: map[string]any{}} + + if strings.TrimSpace(contextJSON) == "" { + return jc, nil + } + + if err := json.Unmarshal([]byte(contextJSON), &jc.Raw); err != nil { + return nil, fmt.Errorf("%w: invalid context JSON", ErrValidation) + } + + jc.Arguments = mapField(jc.Raw, "arguments") + jc.Args = mapField(jc.Raw, "args") + if jc.Arguments == nil { + jc.Arguments = jc.Args + } + if jc.Args == nil { + jc.Args = jc.Arguments + } + jc.Result = jc.Raw["result"] + jc.Prev = mapField(jc.Raw, "prev") + jc.Identity = jc.Raw["identity"] + jc.Source = jc.Raw["source"] + jc.Stash = mapField(jc.Raw, "stash") + + return jc, nil +} + +func mapField(m map[string]any, key string) map[string]any { + if v, ok := m[key].(map[string]any); ok { + return v + } + + return nil +} + +// selectHandlerBody returns the body and context-parameter name of the handler to +// run. AWS uses the `function` field when set (request/response), else defaults to +// `request`, falling back to `response` when only that is defined. +func selectHandlerBody(code, function string) (string, string, error) { + type handler struct { + body string + param string + } + handlers := map[string]handler{} + + idx := reExportedFunc.FindAllStringSubmatchIndex(code, -1) + for _, m := range idx { + name := code[m[2]:m[3]] + params := code[m[4]:m[5]] + openBrace := m[1] - 1 + fnBody, ok := extractBracedBody(code, openBrace) + if !ok { + continue + } + + param := "" + if pm := reCtxParamName.FindStringSubmatch(params); pm != nil { + param = pm[1] + } + + handlers[name] = handler{body: fnBody, param: param} + } + + var order []string + switch function { + case jsHandlerRequest, jsHandlerResponse: + order = []string{function} + default: + order = []string{jsHandlerRequest, jsHandlerResponse} + } + + for _, name := range order { + if h, ok := handlers[name]; ok { + return h.body, h.param, nil + } + } + + return "", "", fmt.Errorf("%w: no request/response handler found in code", ErrUnsupportedJSCode) +} + +// extractBracedBody returns the contents between the brace at openIdx and its +// matching close brace, respecting string/char literals and nested braces. +func extractBracedBody(s string, openIdx int) (string, bool) { + if openIdx < 0 || openIdx >= len(s) || s[openIdx] != '{' { + return "", false + } + + depth := 0 + inStr := byte(0) + escaped := false + + for i := openIdx; i < len(s); i++ { + ch := s[i] + + if inStr != 0 { + switch { + case escaped: + escaped = false + case ch == '\\': + escaped = true + case ch == inStr: + inStr = 0 + } + + continue + } + + switch ch { + case '"', '\'', '`': + inStr = ch + case '{': + depth++ + case '}': + depth-- + if depth == 0 { + return s[openIdx+1 : i], true + } + } + } + + return "", false +} + +// lastReturnExpr returns the expression of the last top-level `return` statement +// in the body. AppSync handlers are straight-line; we take the final return. +func lastReturnExpr(body string) (string, bool) { + matches := reReturnStmt.FindAllStringSubmatch(body, -1) + if len(matches) == 0 { + return "", false + } + + expr := strings.TrimSpace(matches[len(matches)-1][1]) + + return expr, true +} + +// evalJSExpr evaluates a single APPSYNC_JS return expression against the context. +// The expression is assumed non-empty; an empty `return;` is handled by the caller +// (it evaluates to JS undefined, serialized as null). +func evalJSExpr(expr, ctxParam string, jc *jsContext) (any, error) { + expr = strings.TrimSpace(expr) + + // JSON literal (object, array, string, number, bool, null). + if v, ok := tryParseJSONLiteral(expr); ok { + return v, nil + } + + // util.(...) calls. + if strings.HasPrefix(expr, "util.") { + return evalUtilCall(expr, ctxParam, jc) + } + + // Context member access (ctx.result, ctx.arguments.x, ...). + if ctxParam != "" && (expr == ctxParam || strings.HasPrefix(expr, ctxParam+".")) { + return resolveContextPath(expr, ctxParam, jc), nil + } + + // Object literal whose values are themselves resolvable expressions. + if strings.HasPrefix(expr, "{") && strings.HasSuffix(expr, "}") { + return evalObjectLiteral(expr, ctxParam, jc) + } + + return nil, fmt.Errorf("%w: cannot evaluate expression %q", ErrUnsupportedJSCode, expr) +} + +// tryParseJSONLiteral attempts to parse the expression as a JSON value. JS object +// keys are often unquoted, so this only succeeds for strict-JSON literals. +func tryParseJSONLiteral(expr string) (any, bool) { + switch expr { + case "null", "undefined": + return nil, true + case "true": + return true, true + case "false": + return false, true + } + + if n, err := strconv.ParseFloat(expr, 64); err == nil { + return n, true + } + + var v any + if err := json.Unmarshal([]byte(expr), &v); err == nil { + return v, true + } + + return nil, false +} + +// evalObjectLiteral evaluates a `{ key: , ... }` literal where values may be +// JSON literals or context member expressions. Keys may be quoted or bare. +func evalObjectLiteral(expr, ctxParam string, jc *jsContext) (any, error) { + inner := strings.TrimSpace(expr[1 : len(expr)-1]) + result := map[string]any{} + if inner == "" { + return result, nil + } + + pairs, err := splitTopLevel(inner, ',') + if err != nil { + return nil, err + } + + for _, pair := range pairs { + pair = strings.TrimSpace(pair) + if pair == "" { + continue + } + + kv, kvErr := splitTopLevel(pair, ':') + if kvErr != nil || len(kv) != 2 { + return nil, fmt.Errorf("%w: malformed object entry %q", ErrUnsupportedJSCode, pair) + } + + key := strings.Trim(strings.TrimSpace(kv[0]), `"'`) + val, vErr := evalJSExpr(strings.TrimSpace(kv[1]), ctxParam, jc) + if vErr != nil { + return nil, vErr + } + + result[key] = val + } + + return result, nil +} + +// resolveContextPath resolves a dotted path rooted at the context parameter, +// e.g. ctx.arguments.id, ctx.result, ctx.prev.result, ctx.stash.key. A path that +// does not resolve yields nil (JS undefined); this is never an error. +func resolveContextPath(expr, ctxParam string, jc *jsContext) any { + path := strings.TrimPrefix(expr, ctxParam) + path = strings.TrimPrefix(path, ".") + if path == "" { + return jc.Raw + } + + segments := strings.Split(path, ".") + // Normalize the well-known top-level aliases. + var cur any + switch segments[0] { + case "arguments", "args": + cur = jc.Arguments + case "result": + cur = jc.Result + case "prev": + cur = jc.Prev + case "identity": + cur = jc.Identity + case "source": + cur = jc.Source + case "stash": + cur = jc.Stash + default: + if jc.Raw == nil { + return nil + } + cur = jc.Raw[segments[0]] + } + + for _, seg := range segments[1:] { + m, ok := cur.(map[string]any) + if !ok { + return nil + } + cur = m[seg] + } + + return cur +} + +// evalUtilCall evaluates the supported pure `util.*` helpers. +func evalUtilCall(expr, ctxParam string, jc *jsContext) (any, error) { + open := strings.IndexByte(expr, '(') + if open < 0 || !strings.HasSuffix(expr, ")") { + return nil, fmt.Errorf("%w: malformed util call %q", ErrUnsupportedJSCode, expr) + } + + fn := strings.TrimSpace(expr[:open]) + argStr := strings.TrimSpace(expr[open+1 : len(expr)-1]) + + switch fn { + case "util.toJson": + v, err := evalJSExpr(argStr, ctxParam, jc) + if err != nil { + return nil, err + } + b, mErr := json.Marshal(v) + if mErr != nil { + return nil, fmt.Errorf("%w: util.toJson argument not serializable", ErrValidation) + } + + return string(b), nil + + case "util.parseJson": + v, err := evalJSExpr(argStr, ctxParam, jc) + if err != nil { + return nil, err + } + s, ok := v.(string) + if !ok { + return nil, fmt.Errorf("%w: util.parseJson requires a string", ErrValidation) + } + var parsed any + if uErr := json.Unmarshal([]byte(s), &parsed); uErr != nil { + return nil, fmt.Errorf("%w: util.parseJson invalid JSON", ErrValidation) + } + + return parsed, nil + + case "util.error", "util.appendError": + // These raise an error in AppSync; surface it as a validation error. + return nil, fmt.Errorf("%w: %s", ErrValidation, strings.Trim(argStr, `"'`)) + + case "util.unauthorized": + return nil, fmt.Errorf("%w: unauthorized", ErrValidation) + } + + return nil, fmt.Errorf("%w: unsupported helper %q", ErrUnsupportedJSCode, fn) +} + +// splitTopLevel splits s on sep, ignoring separators inside brackets, braces, +// parens, and string literals. +func splitTopLevel(s string, sep byte) ([]string, error) { + var ( + parts []string + buf strings.Builder + depth int + inStr byte + escaped bool + ) + + for i := range len(s) { + ch := s[i] + + if inStr != 0 { + buf.WriteByte(ch) + switch { + case escaped: + escaped = false + case ch == '\\': + escaped = true + case ch == inStr: + inStr = 0 + } + + continue + } + + switch ch { + case '"', '\'', '`': + inStr = ch + buf.WriteByte(ch) + case '{', '[', '(': + depth++ + buf.WriteByte(ch) + case '}', ']', ')': + depth-- + buf.WriteByte(ch) + case sep: + if depth == 0 { + parts = append(parts, buf.String()) + buf.Reset() + } else { + buf.WriteByte(ch) + } + default: + buf.WriteByte(ch) + } + } + + if depth != 0 || inStr != 0 { + return nil, fmt.Errorf("%w: unbalanced expression", ErrUnsupportedJSCode) + } + + parts = append(parts, buf.String()) + + return parts, nil +} diff --git a/services/appsync/jseval_test.go b/services/appsync/jseval_test.go new file mode 100644 index 000000000..7e2d515f3 --- /dev/null +++ b/services/appsync/jseval_test.go @@ -0,0 +1,173 @@ +package appsync_test + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// twoHandlerCode defines both request and response handlers, used to exercise +// handler selection. +const twoHandlerCode = `export function request(ctx) { return ctx.arguments; } ` + + `export function response(ctx) { return ctx.result; }` + +func TestInMemoryBackend_EvaluateCode_Eval(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + code string + context string + function string + runtime string + want string // expected evaluationResult (JSON-stringified) + wantErr bool + }{ + { + name: "empty_object_request", + code: `export function request(ctx) { return {}; }`, + want: `{}`, + }, + { + name: "json_object_literal", + code: `export function request(ctx) { return {"operation":"GetItem"}; }`, + want: `{"operation":"GetItem"}`, + }, + { + name: "return_context_argument", + code: `export function request(ctx) { return ctx.arguments.id; }`, + context: `{"arguments":{"id":"abc-123"}}`, + want: `"abc-123"`, + }, + { + name: "return_context_args_alias", + code: `export function request(ctx) { return ctx.args.count; }`, + context: `{"args":{"count":42}}`, + want: `42`, + }, + { + name: "response_returns_result", + code: `export function response(ctx) { return ctx.result; }`, + context: `{"result":{"name":"widget"}}`, + function: "response", + want: `{"name":"widget"}`, + }, + { + name: "object_with_context_values", + code: `export function request(ctx) { return {"id": ctx.arguments.id, "static": "v"}; }`, + context: `{"arguments":{"id":"x1"}}`, + want: `{"id":"x1","static":"v"}`, + }, + { + name: "util_toJson", + code: `export function request(ctx) { return util.toJson(ctx.arguments); }`, + context: `{"arguments":{"a":1}}`, + want: `"{\"a\":1}"`, + }, + { + name: "util_parseJson", + code: `export function request(ctx) { return util.parseJson("{\"k\":true}"); }`, + want: `{"k":true}`, + }, + { + name: "return_prev_result", + code: `export function request(ctx) { return ctx.prev.result; }`, + context: `{"prev":{"result":{"step":1}}}`, + want: `{"step":1}`, + }, + { + name: "no_return_yields_null", + code: `export function request(ctx) { const x = 1; }`, + want: `null`, + }, + { + name: "function_selects_response", + code: twoHandlerCode, + context: `{"arguments":{"a":1},"result":"final"}`, + function: "response", + want: `"final"`, + }, + { + name: "default_selects_request", + code: twoHandlerCode, + context: `{"arguments":{"a":1},"result":"final"}`, + want: `{"a":1}`, + }, + { + name: "util_error_is_error", + code: `export function request(ctx) { return util.error("boom"); }`, + wantErr: true, + }, + { + name: "empty_code_errors", + code: "", + wantErr: true, + }, + { + name: "invalid_context_json_errors", + code: `export function request(ctx) { return {}; }`, + context: `{not json`, + wantErr: true, + }, + { + name: "unsupported_runtime_errors", + code: `export function request(ctx) { return {}; }`, + runtime: "VTL", + wantErr: true, + }, + { + name: "invalid_function_name_errors", + code: `export function request(ctx) { return {}; }`, + function: "bogus", + wantErr: true, + }, + { + name: "no_handler_errors", + code: `const x = 1;`, + wantErr: true, + }, + { + name: "unsupported_expression_errors", + code: `export function request(ctx) { return someUndefinedVar + 1; }`, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + b := newTestBackend() + out, err := b.EvaluateCode(tt.code, tt.context, tt.function, tt.runtime) + + if tt.wantErr { + require.Error(t, err) + + return + } + + require.NoError(t, err) + // Compare as canonical JSON to be robust to key ordering. + assert.JSONEq(t, tt.want, out) + }) + } +} + +// TestInMemoryBackend_EvaluateCode_ResultIsValidJSON ensures the evaluationResult +// is always a valid JSON document on success. +func TestInMemoryBackend_EvaluateCode_ResultIsValidJSON(t *testing.T) { + t.Parallel() + + b := newTestBackend() + out, err := b.EvaluateCode( + `export function request(ctx) { return {"version":"2018-05-29","operation":"Scan"}; }`, + "", "", "APPSYNC_JS", + ) + require.NoError(t, err) + + var decoded map[string]any + require.NoError(t, json.Unmarshal([]byte(out), &decoded)) + assert.Equal(t, "Scan", decoded["operation"]) +} diff --git a/services/kafka/backend.go b/services/kafka/backend.go index a974ec222..280fef3c1 100644 --- a/services/kafka/backend.go +++ b/services/kafka/backend.go @@ -1372,8 +1372,10 @@ func (b *InMemoryBackend) UpdateBrokerCount( return nil, ErrNotFound } + source := &MutableClusterInfo{NumberOfBrokerNodes: c.NumberOfBrokerNodes} c.NumberOfBrokerNodes = numBrokers - op := b.newClusterOperationLocked(clusterArn, "UPDATE_BROKER_COUNT") + target := &MutableClusterInfo{NumberOfBrokerNodes: numBrokers} + op := b.newClusterOperationLocked(clusterArn, "UPDATE_BROKER_COUNT", source, target) return op, nil } @@ -1400,7 +1402,8 @@ func (b *InMemoryBackend) UpdateBrokerStorage( } c.BrokerNodeGroupInfo.StorageInfo.EbsStorageInfo.VolumeSize = volumeSize - op := b.newClusterOperationLocked(clusterArn, "UPDATE_BROKER_STORAGE") + target := &MutableClusterInfo{BrokerEBSVolumeInfo: []BrokerEBSVolumeInfo{{VolumeSizeGB: volumeSize}}} + op := b.newClusterOperationLocked(clusterArn, "UPDATE_BROKER_STORAGE", nil, target) return op, nil } @@ -1418,7 +1421,7 @@ func (b *InMemoryBackend) UpdateBrokerType( } c.BrokerNodeGroupInfo.InstanceType = instanceType - op := b.newClusterOperationLocked(clusterArn, "UPDATE_BROKER_TYPE") + op := b.newClusterOperationLocked(clusterArn, "UPDATE_BROKER_TYPE", nil, nil) return op, nil } @@ -1440,7 +1443,7 @@ func (b *InMemoryBackend) UpdateClusterConfiguration( Arn: configArn, Revision: revision, } - op := b.newClusterOperationLocked(clusterArn, "UPDATE_CLUSTER_CONFIGURATION") + op := b.newClusterOperationLocked(clusterArn, "UPDATE_CLUSTER_CONFIGURATION", nil, nil) return op, nil } @@ -1458,40 +1461,106 @@ func (b *InMemoryBackend) UpdateClusterKafkaVersion( } c.KafkaVersion = targetKafkaVersion - op := b.newClusterOperationLocked(clusterArn, "UPDATE_CLUSTER_KAFKA_VERSION") + op := b.newClusterOperationLocked(clusterArn, "UPDATE_CLUSTER_KAFKA_VERSION", nil, nil) return op, nil } -// UpdateConnectivity updates connectivity settings for a cluster (stub: no-op). -func (b *InMemoryBackend) UpdateConnectivity(clusterArn string) (*ClusterOperation, error) { +// UpdateConnectivitySettings is the payload for UpdateConnectivity. +type UpdateConnectivitySettings struct { + ConnectivityInfo *ConnectivityInfo +} + +// UpdateMonitoringSettings is the payload for UpdateMonitoring. +type UpdateMonitoringSettings struct { + OpenMonitoring *OpenMonitoring + LoggingInfo *LoggingInfo + EnhancedMonitoring string +} + +// UpdateSecuritySettings is the payload for UpdateSecurity. +type UpdateSecuritySettings struct { + ClientAuthentication *ClientAuthentication + EncryptionInfo *EncryptionInfo +} + +// UpdateStorageSettings is the payload for UpdateStorage. +type UpdateStorageSettings struct { + ProvisionedThroughput *ProvisionedThroughput + StorageMode string + VolumeSizeGB int32 +} + +// UpdateConnectivity updates broker connectivity settings for a cluster, persisting +// the new ConnectivityInfo onto the broker node group and recording an operation +// whose source/target reflect the before/after state. +func (b *InMemoryBackend) UpdateConnectivity( + clusterArn string, settings UpdateConnectivitySettings, +) (*ClusterOperation, error) { b.mu.Lock("UpdateConnectivity") defer b.mu.Unlock() - if _, ok := b.clusters[clusterArn]; !ok { + c, ok := b.clusters[clusterArn] + if !ok { return nil, ErrNotFound } - op := b.newClusterOperationLocked(clusterArn, "UPDATE_CONNECTIVITY") + source := &MutableClusterInfo{ + ConnectivityInfo: cloneConnectivityInfo(c.BrokerNodeGroupInfo.ConnectivityInfo), + } + target := &MutableClusterInfo{ConnectivityInfo: cloneConnectivityInfo(settings.ConnectivityInfo)} + + if settings.ConnectivityInfo != nil { + c.BrokerNodeGroupInfo.ConnectivityInfo = cloneConnectivityInfo(settings.ConnectivityInfo) + } + + op := b.newClusterOperationLocked(clusterArn, "UPDATE_CONNECTIVITY", source, target) return op, nil } -// UpdateMonitoring updates monitoring settings for a cluster (stub: no-op). -func (b *InMemoryBackend) UpdateMonitoring(clusterArn string) (*ClusterOperation, error) { +// UpdateMonitoring updates monitoring/logging settings for a cluster, persisting the +// new EnhancedMonitoring/OpenMonitoring/LoggingInfo and recording an operation. +func (b *InMemoryBackend) UpdateMonitoring( + clusterArn string, settings UpdateMonitoringSettings, +) (*ClusterOperation, error) { b.mu.Lock("UpdateMonitoring") defer b.mu.Unlock() - if _, ok := b.clusters[clusterArn]; !ok { + c, ok := b.clusters[clusterArn] + if !ok { return nil, ErrNotFound } - op := b.newClusterOperationLocked(clusterArn, "UPDATE_MONITORING") + source := &MutableClusterInfo{ + EnhancedMonitoring: c.EnhancedMonitoring, + OpenMonitoring: cloneOpenMonitoring(c.OpenMonitoring), + LoggingInfo: cloneLoggingInfo(c.LoggingInfo), + } + target := &MutableClusterInfo{ + EnhancedMonitoring: settings.EnhancedMonitoring, + OpenMonitoring: cloneOpenMonitoring(settings.OpenMonitoring), + LoggingInfo: cloneLoggingInfo(settings.LoggingInfo), + } + + if settings.EnhancedMonitoring != "" { + c.EnhancedMonitoring = settings.EnhancedMonitoring + } + if settings.OpenMonitoring != nil { + c.OpenMonitoring = cloneOpenMonitoring(settings.OpenMonitoring) + } + if settings.LoggingInfo != nil { + c.LoggingInfo = cloneLoggingInfo(settings.LoggingInfo) + } + + op := b.newClusterOperationLocked(clusterArn, "UPDATE_MONITORING", source, target) return op, nil } -// UpdateRebalancing updates rebalancing settings for a cluster (stub: no-op). +// UpdateRebalancing records a rebalancing operation for a cluster. AWS MSK exposes +// no per-field rebalancing configuration to persist (it is an action, not a setting), +// so this validates the cluster and records the operation. func (b *InMemoryBackend) UpdateRebalancing(clusterArn string) (*ClusterOperation, error) { b.mu.Lock("UpdateRebalancing") defer b.mu.Unlock() @@ -1500,39 +1569,114 @@ func (b *InMemoryBackend) UpdateRebalancing(clusterArn string) (*ClusterOperatio return nil, ErrNotFound } - op := b.newClusterOperationLocked(clusterArn, "UPDATE_REBALANCING") + op := b.newClusterOperationLocked(clusterArn, "UPDATE_REBALANCING", nil, nil) return op, nil } -// UpdateSecurity updates security settings for a cluster (stub: no-op). -func (b *InMemoryBackend) UpdateSecurity(clusterArn string) (*ClusterOperation, error) { +// UpdateSecurity updates authentication/encryption settings for a cluster, persisting +// the new ClientAuthentication/EncryptionInfo and recording an operation. +func (b *InMemoryBackend) UpdateSecurity( + clusterArn string, settings UpdateSecuritySettings, +) (*ClusterOperation, error) { b.mu.Lock("UpdateSecurity") defer b.mu.Unlock() - if _, ok := b.clusters[clusterArn]; !ok { + c, ok := b.clusters[clusterArn] + if !ok { return nil, ErrNotFound } - op := b.newClusterOperationLocked(clusterArn, "UPDATE_SECURITY") + source := &MutableClusterInfo{ + ClientAuthentication: cloneClientAuth(c.ClientAuthentication), + EncryptionInfo: cloneEncryptionInfo(c.EncryptionInfo), + } + target := &MutableClusterInfo{ + ClientAuthentication: cloneClientAuth(settings.ClientAuthentication), + EncryptionInfo: cloneEncryptionInfo(settings.EncryptionInfo), + } + + if settings.ClientAuthentication != nil { + c.ClientAuthentication = cloneClientAuth(settings.ClientAuthentication) + } + if settings.EncryptionInfo != nil { + c.EncryptionInfo = cloneEncryptionInfo(settings.EncryptionInfo) + } + + op := b.newClusterOperationLocked(clusterArn, "UPDATE_SECURITY", source, target) return op, nil } -// UpdateStorage updates storage settings for a cluster (stub: no-op). -func (b *InMemoryBackend) UpdateStorage(clusterArn string) (*ClusterOperation, error) { +// UpdateStorage updates broker storage settings for a cluster, persisting the new +// StorageMode and EBS volume size/throughput and recording an operation. +func (b *InMemoryBackend) UpdateStorage( + clusterArn string, settings UpdateStorageSettings, +) (*ClusterOperation, error) { b.mu.Lock("UpdateStorage") defer b.mu.Unlock() - if _, ok := b.clusters[clusterArn]; !ok { + c, ok := b.clusters[clusterArn] + if !ok { return nil, ErrNotFound } - op := b.newClusterOperationLocked(clusterArn, "UPDATE_STORAGE") + source := &MutableClusterInfo{StorageMode: c.StorageMode} + if si := c.BrokerNodeGroupInfo.StorageInfo; si != nil && si.EbsStorageInfo != nil { + source.BrokerEBSVolumeInfo = []BrokerEBSVolumeInfo{{ + VolumeSizeGB: si.EbsStorageInfo.VolumeSize, + ProvisionedThroughput: cloneProvisionedThroughput(si.EbsStorageInfo.ProvisionedThroughput), + }} + } + + target := &MutableClusterInfo{StorageMode: settings.StorageMode} + if settings.VolumeSizeGB > 0 || settings.ProvisionedThroughput != nil { + target.BrokerEBSVolumeInfo = []BrokerEBSVolumeInfo{{ + VolumeSizeGB: settings.VolumeSizeGB, + ProvisionedThroughput: cloneProvisionedThroughput(settings.ProvisionedThroughput), + }} + } + + if settings.StorageMode != "" { + c.StorageMode = settings.StorageMode + } + if settings.VolumeSizeGB > 0 || settings.ProvisionedThroughput != nil { + applyStorageUpdateLocked(c, settings) + } + + op := b.newClusterOperationLocked(clusterArn, "UPDATE_STORAGE", source, target) return op, nil } +// applyStorageUpdateLocked mutates the cluster's EBS storage info from an +// UpdateStorage payload. The caller must hold the write lock. +func applyStorageUpdateLocked(c *Cluster, settings UpdateStorageSettings) { + if c.BrokerNodeGroupInfo.StorageInfo == nil { + c.BrokerNodeGroupInfo.StorageInfo = &StorageInfo{} + } + if c.BrokerNodeGroupInfo.StorageInfo.EbsStorageInfo == nil { + c.BrokerNodeGroupInfo.StorageInfo.EbsStorageInfo = &EBSStorageInfo{} + } + ebs := c.BrokerNodeGroupInfo.StorageInfo.EbsStorageInfo + if settings.VolumeSizeGB > 0 { + ebs.VolumeSize = settings.VolumeSizeGB + } + if settings.ProvisionedThroughput != nil { + ebs.ProvisionedThroughput = cloneProvisionedThroughput(settings.ProvisionedThroughput) + } +} + +// cloneProvisionedThroughput returns a deep copy of a ProvisionedThroughput. +func cloneProvisionedThroughput(pt *ProvisionedThroughput) *ProvisionedThroughput { + if pt == nil { + return nil + } + clone := *pt + + return &clone +} + // RebootBroker initiates a broker reboot operation. func (b *InMemoryBackend) RebootBroker(clusterArn string, _ []string) (*ClusterOperation, error) { b.mu.Lock("RebootBroker") @@ -1542,7 +1686,7 @@ func (b *InMemoryBackend) RebootBroker(clusterArn string, _ []string) (*ClusterO return nil, ErrNotFound } - op := b.newClusterOperationLocked(clusterArn, "REBOOT_BROKER") + op := b.newClusterOperationLocked(clusterArn, "REBOOT_BROKER", nil, nil) return op, nil } @@ -1653,7 +1797,7 @@ type MSKVersion struct { // newClusterOperationLocked creates and stores a cluster operation. // MUST be called with b.mu write lock held. func (b *InMemoryBackend) newClusterOperationLocked( - clusterArn, operationType string, + clusterArn, operationType string, source, target *MutableClusterInfo, ) *ClusterOperation { clusterOperationArn := b.clusterOperationARN(clusterArn) op := &ClusterOperation{ @@ -1661,6 +1805,8 @@ func (b *InMemoryBackend) newClusterOperationLocked( ClusterArn: clusterArn, OperationType: operationType, OperationState: ClusterOperationStateUpdateComplete, + SourceClusterInfo: source, + TargetClusterInfo: target, } b.clusterOperations[clusterOperationArn] = op @@ -1846,10 +1992,34 @@ type VpcConnection struct { // ClusterOperation represents an MSK cluster operation. type ClusterOperation struct { - ClusterOperationArn string `json:"clusterOperationArn"` - ClusterArn string `json:"clusterArn"` - OperationType string `json:"operationType"` - OperationState string `json:"operationState"` + SourceClusterInfo *MutableClusterInfo `json:"sourceClusterInfo,omitempty"` + TargetClusterInfo *MutableClusterInfo `json:"targetClusterInfo,omitempty"` + ClusterOperationArn string `json:"clusterOperationArn"` + ClusterArn string `json:"clusterArn"` + OperationType string `json:"operationType"` + OperationState string `json:"operationState"` +} + +// MutableClusterInfo captures the subset of cluster configuration that an update +// operation changes. DescribeClusterOperation returns it as SourceClusterInfo +// (the state before the operation) and TargetClusterInfo (the requested state). +type MutableClusterInfo struct { + ConnectivityInfo *ConnectivityInfo `json:"connectivityInfo,omitempty"` + OpenMonitoring *OpenMonitoring `json:"openMonitoring,omitempty"` + LoggingInfo *LoggingInfo `json:"loggingInfo,omitempty"` + ClientAuthentication *ClientAuthentication `json:"clientAuthentication,omitempty"` + EncryptionInfo *EncryptionInfo `json:"encryptionInfo,omitempty"` + StorageMode string `json:"storageMode,omitempty"` + EnhancedMonitoring string `json:"enhancedMonitoring,omitempty"` + BrokerEBSVolumeInfo []BrokerEBSVolumeInfo `json:"brokerEBSVolumeInfo,omitempty"` + NumberOfBrokerNodes int32 `json:"numberOfBrokerNodes,omitempty"` +} + +// BrokerEBSVolumeInfo describes a per-broker EBS volume target for UpdateStorage. +type BrokerEBSVolumeInfo struct { + ProvisionedThroughput *ProvisionedThroughput `json:"provisionedThroughput,omitempty"` + KafkaBrokerNodeID string `json:"kafkaBrokerNodeId,omitempty"` + VolumeSizeGB int32 `json:"volumeSizeGB,omitempty"` } // cloneCluster creates a deep copy of a cluster. @@ -2182,7 +2352,45 @@ func cloneClusterOperation(op *ClusterOperation) *ClusterOperation { ClusterArn: op.ClusterArn, OperationType: op.OperationType, OperationState: op.OperationState, + SourceClusterInfo: cloneMutableClusterInfo(op.SourceClusterInfo), + TargetClusterInfo: cloneMutableClusterInfo(op.TargetClusterInfo), + } +} + +// cloneMutableClusterInfo deep-copies a MutableClusterInfo, reusing the existing +// per-field clone helpers so the returned operation does not alias backend state. +func cloneMutableClusterInfo(m *MutableClusterInfo) *MutableClusterInfo { + if m == nil { + return nil } + + clone := &MutableClusterInfo{ + ConnectivityInfo: cloneConnectivityInfo(m.ConnectivityInfo), + OpenMonitoring: cloneOpenMonitoring(m.OpenMonitoring), + LoggingInfo: cloneLoggingInfo(m.LoggingInfo), + ClientAuthentication: cloneClientAuth(m.ClientAuthentication), + EncryptionInfo: cloneEncryptionInfo(m.EncryptionInfo), + StorageMode: m.StorageMode, + EnhancedMonitoring: m.EnhancedMonitoring, + NumberOfBrokerNodes: m.NumberOfBrokerNodes, + } + + if len(m.BrokerEBSVolumeInfo) > 0 { + clone.BrokerEBSVolumeInfo = make([]BrokerEBSVolumeInfo, len(m.BrokerEBSVolumeInfo)) + for i, v := range m.BrokerEBSVolumeInfo { + cv := BrokerEBSVolumeInfo{ + KafkaBrokerNodeID: v.KafkaBrokerNodeID, + VolumeSizeGB: v.VolumeSizeGB, + } + if v.ProvisionedThroughput != nil { + pt := *v.ProvisionedThroughput + cv.ProvisionedThroughput = &pt + } + clone.BrokerEBSVolumeInfo[i] = cv + } + } + + return clone } // nonNilTagsCopy returns a new non-nil copy of tags; an empty map if tags is nil. diff --git a/services/kafka/handler.go b/services/kafka/handler.go index edf93bb75..4d60bf2a3 100644 --- a/services/kafka/handler.go +++ b/services/kafka/handler.go @@ -2509,8 +2509,21 @@ func (h *Handler) handleUpdateClusterKafkaVersion( ) } -func (h *Handler) handleUpdateConnectivity(c *echo.Context, clusterArn string, _ []byte) error { - op, err := h.Backend.UpdateConnectivity(clusterArn) +// updateConnectivityInput is the UpdateConnectivity request body. +type updateConnectivityInput struct { + ConnectivityInfo *ConnectivityInfo `json:"connectivityInfo"` + CurrentVersion string `json:"currentVersion"` +} + +func (h *Handler) handleUpdateConnectivity(c *echo.Context, clusterArn string, body []byte) error { + var in updateConnectivityInput + if err := decodeJSONBody(body, &in); err != nil { + return h.writeError(c, http.StatusBadRequest, "BadRequestException", err.Error()) + } + + op, err := h.Backend.UpdateConnectivity(clusterArn, UpdateConnectivitySettings{ + ConnectivityInfo: in.ConnectivityInfo, + }) if err != nil { return h.writeBackendError(c, err) } @@ -2521,8 +2534,25 @@ func (h *Handler) handleUpdateConnectivity(c *echo.Context, clusterArn string, _ ) } -func (h *Handler) handleUpdateMonitoring(c *echo.Context, clusterArn string, _ []byte) error { - op, err := h.Backend.UpdateMonitoring(clusterArn) +// updateMonitoringInput is the UpdateMonitoring request body. +type updateMonitoringInput struct { + OpenMonitoring *OpenMonitoring `json:"openMonitoring"` + LoggingInfo *LoggingInfo `json:"loggingInfo"` + EnhancedMonitoring string `json:"enhancedMonitoring"` + CurrentVersion string `json:"currentVersion"` +} + +func (h *Handler) handleUpdateMonitoring(c *echo.Context, clusterArn string, body []byte) error { + var in updateMonitoringInput + if err := decodeJSONBody(body, &in); err != nil { + return h.writeError(c, http.StatusBadRequest, "BadRequestException", err.Error()) + } + + op, err := h.Backend.UpdateMonitoring(clusterArn, UpdateMonitoringSettings{ + EnhancedMonitoring: in.EnhancedMonitoring, + OpenMonitoring: in.OpenMonitoring, + LoggingInfo: in.LoggingInfo, + }) if err != nil { return h.writeBackendError(c, err) } @@ -2545,8 +2575,23 @@ func (h *Handler) handleUpdateRebalancing(c *echo.Context, clusterArn string, _ ) } -func (h *Handler) handleUpdateSecurity(c *echo.Context, clusterArn string, _ []byte) error { - op, err := h.Backend.UpdateSecurity(clusterArn) +// updateSecurityInput is the UpdateSecurity request body. +type updateSecurityInput struct { + ClientAuthentication *ClientAuthentication `json:"clientAuthentication"` + EncryptionInfo *EncryptionInfo `json:"encryptionInfo"` + CurrentVersion string `json:"currentVersion"` +} + +func (h *Handler) handleUpdateSecurity(c *echo.Context, clusterArn string, body []byte) error { + var in updateSecurityInput + if err := decodeJSONBody(body, &in); err != nil { + return h.writeError(c, http.StatusBadRequest, "BadRequestException", err.Error()) + } + + op, err := h.Backend.UpdateSecurity(clusterArn, UpdateSecuritySettings{ + ClientAuthentication: in.ClientAuthentication, + EncryptionInfo: in.EncryptionInfo, + }) if err != nil { return h.writeBackendError(c, err) } @@ -2557,8 +2602,25 @@ func (h *Handler) handleUpdateSecurity(c *echo.Context, clusterArn string, _ []b ) } -func (h *Handler) handleUpdateStorage(c *echo.Context, clusterArn string, _ []byte) error { - op, err := h.Backend.UpdateStorage(clusterArn) +// updateStorageInput is the UpdateStorage request body. +type updateStorageInput struct { + ProvisionedThroughput *ProvisionedThroughput `json:"provisionedThroughput"` + StorageMode string `json:"storageMode"` + CurrentVersion string `json:"currentVersion"` + VolumeSizeGB int32 `json:"volumeSizeGB"` +} + +func (h *Handler) handleUpdateStorage(c *echo.Context, clusterArn string, body []byte) error { + var in updateStorageInput + if err := decodeJSONBody(body, &in); err != nil { + return h.writeError(c, http.StatusBadRequest, "BadRequestException", err.Error()) + } + + op, err := h.Backend.UpdateStorage(clusterArn, UpdateStorageSettings{ + StorageMode: in.StorageMode, + VolumeSizeGB: in.VolumeSizeGB, + ProvisionedThroughput: in.ProvisionedThroughput, + }) if err != nil { return h.writeBackendError(c, err) } @@ -2569,6 +2631,16 @@ func (h *Handler) handleUpdateStorage(c *echo.Context, clusterArn string, _ []by ) } +// decodeJSONBody unmarshals a (possibly empty) JSON request body. An empty body +// is treated as an empty object so optional update fields default to zero values. +func decodeJSONBody(body []byte, v any) error { + if len(body) == 0 { + return nil + } + + return json.Unmarshal(body, v) +} + // ---------------------------------------- // Error helpers // ---------------------------------------- diff --git a/services/kafka/interfaces.go b/services/kafka/interfaces.go index b0fed0376..a50b2415a 100644 --- a/services/kafka/interfaces.go +++ b/services/kafka/interfaces.go @@ -84,11 +84,11 @@ type StorageBackend interface { UpdateBrokerType(clusterArn, instanceType string) (*ClusterOperation, error) UpdateClusterConfiguration(clusterArn, configArn string, revision int64) (*ClusterOperation, error) UpdateClusterKafkaVersion(clusterArn, targetKafkaVersion string) (*ClusterOperation, error) - UpdateConnectivity(clusterArn string) (*ClusterOperation, error) - UpdateMonitoring(clusterArn string) (*ClusterOperation, error) + UpdateConnectivity(clusterArn string, settings UpdateConnectivitySettings) (*ClusterOperation, error) + UpdateMonitoring(clusterArn string, settings UpdateMonitoringSettings) (*ClusterOperation, error) UpdateRebalancing(clusterArn string) (*ClusterOperation, error) - UpdateSecurity(clusterArn string) (*ClusterOperation, error) - UpdateStorage(clusterArn string) (*ClusterOperation, error) + UpdateSecurity(clusterArn string, settings UpdateSecuritySettings) (*ClusterOperation, error) + UpdateStorage(clusterArn string, settings UpdateStorageSettings) (*ClusterOperation, error) RebootBroker(clusterArn string, brokerIDs []string) (*ClusterOperation, error) // SCRAM secret list diff --git a/services/kafka/update_settings_test.go b/services/kafka/update_settings_test.go new file mode 100644 index 000000000..e2a5d6ed7 --- /dev/null +++ b/services/kafka/update_settings_test.go @@ -0,0 +1,206 @@ +package kafka_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/kafka" +) + +// TestBackend_UpdateSettings verifies that the per-setting Update* operations +// persist their payloads onto the cluster and record a ClusterOperation whose +// SourceClusterInfo/TargetClusterInfo round-trip via DescribeClusterOperation. +func TestBackend_UpdateSettings(t *testing.T) { + t.Parallel() + + tests := []struct { + // apply runs the update against the cluster, returning the operation ARN. + apply func(t *testing.T, b *kafka.InMemoryBackend, arn string) string + // verify asserts both the persisted cluster state and the recorded operation. + verify func(t *testing.T, c *kafka.Cluster, op *kafka.ClusterOperation) + name string + opType string + }{ + { + name: "connectivity", + opType: "UPDATE_CONNECTIVITY", + apply: func(t *testing.T, b *kafka.InMemoryBackend, arn string) string { + t.Helper() + op, err := b.UpdateConnectivity(arn, kafka.UpdateConnectivitySettings{ + ConnectivityInfo: &kafka.ConnectivityInfo{ + PublicAccess: &kafka.PublicAccess{Type: "SERVICE_PROVIDED_EIPS"}, + }, + }) + require.NoError(t, err) + + return op.ClusterOperationArn + }, + verify: func(t *testing.T, c *kafka.Cluster, op *kafka.ClusterOperation) { + t.Helper() + require.NotNil(t, c.BrokerNodeGroupInfo.ConnectivityInfo) + require.NotNil(t, c.BrokerNodeGroupInfo.ConnectivityInfo.PublicAccess) + assert.Equal(t, "SERVICE_PROVIDED_EIPS", c.BrokerNodeGroupInfo.ConnectivityInfo.PublicAccess.Type) + require.NotNil(t, op.TargetClusterInfo) + require.NotNil(t, op.TargetClusterInfo.ConnectivityInfo) + assert.Equal(t, "SERVICE_PROVIDED_EIPS", + op.TargetClusterInfo.ConnectivityInfo.PublicAccess.Type) + }, + }, + { + name: "monitoring", + opType: "UPDATE_MONITORING", + apply: func(t *testing.T, b *kafka.InMemoryBackend, arn string) string { + t.Helper() + op, err := b.UpdateMonitoring(arn, kafka.UpdateMonitoringSettings{ + EnhancedMonitoring: "PER_TOPIC_PER_BROKER", + OpenMonitoring: &kafka.OpenMonitoring{ + Prometheus: &kafka.PrometheusInfo{ + JmxExporter: &kafka.JmxExporter{EnabledInBroker: true}, + }, + }, + }) + require.NoError(t, err) + + return op.ClusterOperationArn + }, + verify: func(t *testing.T, c *kafka.Cluster, op *kafka.ClusterOperation) { + t.Helper() + assert.Equal(t, "PER_TOPIC_PER_BROKER", c.EnhancedMonitoring) + require.NotNil(t, c.OpenMonitoring) + require.NotNil(t, c.OpenMonitoring.Prometheus) + require.NotNil(t, c.OpenMonitoring.Prometheus.JmxExporter) + assert.True(t, c.OpenMonitoring.Prometheus.JmxExporter.EnabledInBroker) + require.NotNil(t, op.TargetClusterInfo) + assert.Equal(t, "PER_TOPIC_PER_BROKER", op.TargetClusterInfo.EnhancedMonitoring) + // Source reflects pre-update state (default monitoring level). + require.NotNil(t, op.SourceClusterInfo) + }, + }, + { + name: "security", + opType: "UPDATE_SECURITY", + apply: func(t *testing.T, b *kafka.InMemoryBackend, arn string) string { + t.Helper() + op, err := b.UpdateSecurity(arn, kafka.UpdateSecuritySettings{ + ClientAuthentication: &kafka.ClientAuthentication{ + Sasl: &kafka.SaslSettings{Iam: &kafka.SaslIam{Enabled: true}}, + }, + EncryptionInfo: &kafka.EncryptionInfo{ + EncryptionInTransit: &kafka.EncryptionInTransit{ClientBroker: "TLS", InCluster: true}, + }, + }) + require.NoError(t, err) + + return op.ClusterOperationArn + }, + verify: func(t *testing.T, c *kafka.Cluster, op *kafka.ClusterOperation) { + t.Helper() + require.NotNil(t, c.ClientAuthentication) + require.NotNil(t, c.ClientAuthentication.Sasl) + require.NotNil(t, c.ClientAuthentication.Sasl.Iam) + assert.True(t, c.ClientAuthentication.Sasl.Iam.Enabled) + require.NotNil(t, c.EncryptionInfo) + require.NotNil(t, c.EncryptionInfo.EncryptionInTransit) + assert.Equal(t, "TLS", c.EncryptionInfo.EncryptionInTransit.ClientBroker) + require.NotNil(t, op.TargetClusterInfo) + require.NotNil(t, op.TargetClusterInfo.ClientAuthentication) + }, + }, + { + name: "storage", + opType: "UPDATE_STORAGE", + apply: func(t *testing.T, b *kafka.InMemoryBackend, arn string) string { + t.Helper() + op, err := b.UpdateStorage(arn, kafka.UpdateStorageSettings{ + StorageMode: "TIERED", + VolumeSizeGB: 2048, + ProvisionedThroughput: &kafka.ProvisionedThroughput{ + Enabled: true, + VolumeThroughput: 250, + }, + }) + require.NoError(t, err) + + return op.ClusterOperationArn + }, + verify: func(t *testing.T, c *kafka.Cluster, op *kafka.ClusterOperation) { + t.Helper() + assert.Equal(t, "TIERED", c.StorageMode) + require.NotNil(t, c.BrokerNodeGroupInfo.StorageInfo) + require.NotNil(t, c.BrokerNodeGroupInfo.StorageInfo.EbsStorageInfo) + ebs := c.BrokerNodeGroupInfo.StorageInfo.EbsStorageInfo + assert.Equal(t, int32(2048), ebs.VolumeSize) + require.NotNil(t, ebs.ProvisionedThroughput) + assert.True(t, ebs.ProvisionedThroughput.Enabled) + assert.Equal(t, int32(250), ebs.ProvisionedThroughput.VolumeThroughput) + require.NotNil(t, op.TargetClusterInfo) + assert.Equal(t, "TIERED", op.TargetClusterInfo.StorageMode) + require.Len(t, op.TargetClusterInfo.BrokerEBSVolumeInfo, 1) + assert.Equal(t, int32(2048), op.TargetClusterInfo.BrokerEBSVolumeInfo[0].VolumeSizeGB) + }, + }, + { + name: "rebalancing_is_action_only", + opType: "UPDATE_REBALANCING", + apply: func(t *testing.T, b *kafka.InMemoryBackend, arn string) string { + t.Helper() + op, err := b.UpdateRebalancing(arn) + require.NoError(t, err) + + return op.ClusterOperationArn + }, + verify: func(t *testing.T, _ *kafka.Cluster, op *kafka.ClusterOperation) { + t.Helper() + // Rebalancing has no persisted setting; the operation is still recorded. + assert.Nil(t, op.TargetClusterInfo) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + b := newTestBackend(t) + cluster, err := b.CreateCluster("c1", "3.5.1", 3, kafka.BrokerNodeGroupInfo{ + InstanceType: "kafka.m5.large", + ClientSubnets: []string{"subnet-1"}, + }, nil, nil) + require.NoError(t, err) + + opArn := tt.apply(t, b, cluster.ClusterArn) + assert.NotEmpty(t, opArn) + + op, descErr := b.DescribeClusterOperation(opArn) + require.NoError(t, descErr) + assert.Equal(t, tt.opType, op.OperationType) + assert.Equal(t, cluster.ClusterArn, op.ClusterArn) + + updated, getErr := b.DescribeCluster(cluster.ClusterArn) + require.NoError(t, getErr) + + tt.verify(t, updated, op) + }) + } +} + +// TestBackend_UpdateSettings_NotFound verifies missing clusters error. +func TestBackend_UpdateSettings_NotFound(t *testing.T) { + t.Parallel() + + b := newTestBackend(t) + missing := "arn:aws:kafka:us-east-1:000000000000:cluster/missing/abc" + + _, err := b.UpdateConnectivity(missing, kafka.UpdateConnectivitySettings{}) + require.Error(t, err) + _, err = b.UpdateMonitoring(missing, kafka.UpdateMonitoringSettings{}) + require.Error(t, err) + _, err = b.UpdateSecurity(missing, kafka.UpdateSecuritySettings{}) + require.Error(t, err) + _, err = b.UpdateStorage(missing, kafka.UpdateStorageSettings{}) + require.Error(t, err) + _, err = b.UpdateRebalancing(missing) + require.Error(t, err) +} diff --git a/services/pipes/runner.go b/services/pipes/runner.go index 2014777ab..964d34540 100644 --- a/services/pipes/runner.go +++ b/services/pipes/runner.go @@ -12,7 +12,22 @@ import ( "github.com/blackbirdworks/gopherstack/pkgs/logger" ) -var ErrUnsupportedPipeTarget = errors.New("pipes: unsupported target ARN") +var ( + // ErrUnsupportedPipeTarget is returned when a pipe target ARN service is not + // handled by the runner. + ErrUnsupportedPipeTarget = errors.New("pipes: unsupported target ARN") + // ErrUnsupportedPipeEnrichment is returned when a pipe enrichment ARN service + // is not handled by the runner. + ErrUnsupportedPipeEnrichment = errors.New("pipes: unsupported enrichment ARN") + // ErrEnrichmentInvokerUnwired is returned when an enrichment ARN is recognized + // but the corresponding invoker (Lambda/StepFunctions) has not been wired into + // the runner. This surfaces a real misconfiguration instead of silently + // dropping the event. + ErrEnrichmentInvokerUnwired = errors.New("pipes: enrichment invoker not configured") + // ErrTargetInvokerUnwired is returned when a target ARN service is recognized but + // the corresponding invoker has not been wired into the runner. + ErrTargetInvokerUnwired = errors.New("pipes: target invoker not configured") +) const ( pipeRunnerTickInterval = 1 * time.Second @@ -262,6 +277,7 @@ func (r *Runner) pollSQSPipe(ctx context.Context, p *Pipe) { if enrichErr != nil { logger.Load(ctx).WarnContext(ctx, "pipes: enrichment invocation failed", "pipe", p.Name, "enrichment", p.Enrichment, "error", enrichErr) + r.handlePipeFailure(ctx, p, msgs, payload, enrichErr) return } @@ -274,6 +290,7 @@ func (r *Runner) pollSQSPipe(ctx context.Context, p *Pipe) { if invokeErr != nil { logger.Load(ctx).WarnContext(ctx, "pipes: target invocation failed", "pipe", p.Name, "target", p.Target, "error", invokeErr) + r.handlePipeFailure(ctx, p, msgs, payload, invokeErr) return } @@ -284,6 +301,57 @@ func (r *Runner) pollSQSPipe(ctx context.Context, p *Pipe) { } } +// handlePipeFailure routes a failed batch to the configured dead-letter queue. When +// a DLQ is configured and the send succeeds, the source messages are deleted so they +// are not reprocessed in a tight loop. When no DLQ is configured, the messages are +// left in the source for the source service's own redelivery/visibility handling. +func (r *Runner) handlePipeFailure( + ctx context.Context, p *Pipe, msgs []*SQSMessage, payload []byte, cause error, +) { + if p.DeadLetterConfig == nil || p.DeadLetterConfig.Arn == "" { + return + } + + dlqARN := p.DeadLetterConfig.Arn + if sendErr := r.sendToDLQ(ctx, dlqARN, payload); sendErr != nil { + logger.Load(ctx).WarnContext(ctx, "pipes: failed to send to DLQ", + "pipe", p.Name, "dlq", dlqARN, "error", sendErr, "cause", cause) + + return + } + + receiptHandles := make([]string, len(msgs)) + for i, m := range msgs { + receiptHandles[i] = m.ReceiptHandle + } + + if delErr := r.sqsReader.DeletePipeMessages(p.Source, receiptHandles); delErr != nil { + logger.Load(ctx).WarnContext(ctx, "pipes: failed to delete source messages after DLQ send", + "pipe", p.Name, "source", p.Source, "error", delErr) + } +} + +// sendToDLQ delivers a failed payload to the pipe's dead-letter queue. Both SQS +// queue and SNS topic dead-letter targets are supported. +func (r *Runner) sendToDLQ(ctx context.Context, dlqARN string, payload []byte) error { + switch { + case strings.HasPrefix(dlqARN, "arn:aws:sqs:"): + if r.sqsSender == nil { + return fmt.Errorf("%w: sqs DLQ %q", ErrEnrichmentInvokerUnwired, dlqARN) + } + + return r.sqsSender.SendMessage(ctx, dlqARN, string(payload), "", "") + case strings.HasPrefix(dlqARN, "arn:aws:sns:"): + if r.sns == nil { + return fmt.Errorf("%w: sns DLQ %q", ErrEnrichmentInvokerUnwired, dlqARN) + } + + return r.sns.PublishMessage(ctx, dlqARN, string(payload)) + } + + return fmt.Errorf("%w: dead-letter target %q", ErrUnsupportedPipeTarget, dlqARN) +} + // invokeEnrichment calls the enrichment endpoint and returns the enriched payload. func (r *Runner) invokeEnrichment(ctx context.Context, p *Pipe, payload []byte) ([]byte, error) { enrichARN := p.Enrichment @@ -291,7 +359,7 @@ func (r *Runner) invokeEnrichment(ctx context.Context, p *Pipe, payload []byte) switch { case strings.HasPrefix(enrichARN, "arn:aws:lambda:"): if r.lambda == nil { - return nil, nil + return nil, fmt.Errorf("%w: lambda enrichment %q", ErrEnrichmentInvokerUnwired, enrichARN) } invocationType := "RequestResponse" @@ -313,7 +381,7 @@ func (r *Runner) invokeEnrichment(ctx context.Context, p *Pipe, payload []byte) case strings.HasPrefix(enrichARN, "arn:aws:states:"): if r.sfn == nil { - return nil, nil + return nil, fmt.Errorf("%w: step functions enrichment %q", ErrEnrichmentInvokerUnwired, enrichARN) } input := string(payload) @@ -324,8 +392,9 @@ func (r *Runner) invokeEnrichment(ctx context.Context, p *Pipe, payload []byte) return nil, r.sfn.StartExecution(enrichARN, "", input) } - // Enrichment ARN type not supported by this runner; skip silently. - return nil, nil + // Enrichment ARN type is not handled by this runner — surface a real error + // rather than silently dropping the event. + return nil, fmt.Errorf("%w %q for pipe %q", ErrUnsupportedPipeEnrichment, enrichARN, p.Name) } func (r *Runner) applyFilters(p *Pipe, msgs []*SQSMessage) []*SQSMessage { @@ -424,7 +493,7 @@ func applyInputTemplate(p *Pipe, defaultPayload []byte) []byte { func (r *Runner) invokeLambdaTarget(ctx context.Context, p *Pipe, payload []byte) error { if r.lambda == nil { - return nil + return fmt.Errorf("%w: lambda target %q", ErrTargetInvokerUnwired, p.Target) } // Map Pipes InvocationType to Lambda InvocationType. @@ -459,7 +528,7 @@ func (r *Runner) invokeLambdaTarget(ctx context.Context, p *Pipe, payload []byte func (r *Runner) invokeSFNTarget(_ context.Context, p *Pipe, payload []byte) error { if r.sfn == nil { - return nil + return fmt.Errorf("%w: step functions target %q", ErrTargetInvokerUnwired, p.Target) } payload = applyInputTemplate(p, payload) @@ -481,7 +550,7 @@ func (r *Runner) invokeSFNTarget(_ context.Context, p *Pipe, payload []byte) err func (r *Runner) invokeSNSTarget(ctx context.Context, p *Pipe, payload []byte) error { if r.sns == nil { - return nil + return fmt.Errorf("%w: sns target %q", ErrTargetInvokerUnwired, p.Target) } payload = applyInputTemplate(p, payload) @@ -491,7 +560,7 @@ func (r *Runner) invokeSNSTarget(ctx context.Context, p *Pipe, payload []byte) e func (r *Runner) invokeSQSTarget(ctx context.Context, p *Pipe, payload []byte) error { if r.sqsSender == nil { - return nil + return fmt.Errorf("%w: sqs target %q", ErrTargetInvokerUnwired, p.Target) } payload = applyInputTemplate(p, payload) @@ -507,7 +576,7 @@ func (r *Runner) invokeSQSTarget(ctx context.Context, p *Pipe, payload []byte) e func (r *Runner) invokeKinesisTarget(ctx context.Context, p *Pipe, payload []byte) error { if r.kinesis == nil { - return nil + return fmt.Errorf("%w: kinesis target %q", ErrTargetInvokerUnwired, p.Target) } payload = applyInputTemplate(p, payload) @@ -523,7 +592,7 @@ func (r *Runner) invokeKinesisTarget(ctx context.Context, p *Pipe, payload []byt func (r *Runner) invokeEventBridgeTarget(ctx context.Context, p *Pipe, payload []byte) error { if r.eventBus == nil { - return nil + return fmt.Errorf("%w: eventbridge target %q", ErrTargetInvokerUnwired, p.Target) } payload = applyInputTemplate(p, payload) @@ -548,7 +617,7 @@ func (r *Runner) invokeEventBridgeTarget(ctx context.Context, p *Pipe, payload [ func (r *Runner) invokeCloudWatchLogsTarget(ctx context.Context, p *Pipe, payload []byte) error { if r.cwLogs == nil { - return nil + return fmt.Errorf("%w: cloudwatch logs target %q", ErrTargetInvokerUnwired, p.Target) } payload = applyInputTemplate(p, payload) @@ -564,7 +633,7 @@ func (r *Runner) invokeCloudWatchLogsTarget(ctx context.Context, p *Pipe, payloa func (r *Runner) invokeFirehoseTarget(ctx context.Context, p *Pipe, payload []byte) error { if r.firehose == nil { - return nil + return fmt.Errorf("%w: firehose target %q", ErrTargetInvokerUnwired, p.Target) } payload = applyInputTemplate(p, payload) diff --git a/services/pipes/runner_dlq_test.go b/services/pipes/runner_dlq_test.go new file mode 100644 index 000000000..c209bb7aa --- /dev/null +++ b/services/pipes/runner_dlq_test.go @@ -0,0 +1,234 @@ +package pipes_test + +import ( + "context" + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/blackbirdworks/gopherstack/services/pipes" +) + +const ( + dlqSource = "arn:aws:sqs:eu-west-1:111122223333:src" + dlqLambdaTgt = "arn:aws:lambda:eu-west-1:111122223333:function:tgt" + dlqQueueARN = "arn:aws:sqs:eu-west-1:111122223333:dlq" + dlqEnrichARN = "arn:aws:lambda:eu-west-1:111122223333:function:enricher" + dlqSNSTopicARN = "arn:aws:sns:eu-west-1:111122223333:dlq-topic" +) + +// dlqMockSQSReader records deleted receipt handles and serves one batch. +type dlqMockSQSReader struct { + messages []*pipes.SQSMessage + deleted []string + mu sync.Mutex +} + +func (m *dlqMockSQSReader) ReceivePipeMessages(_ string, _ int) ([]*pipes.SQSMessage, error) { + m.mu.Lock() + defer m.mu.Unlock() + msgs := m.messages + m.messages = nil + + return msgs, nil +} + +func (m *dlqMockSQSReader) DeletePipeMessages(_ string, receiptHandles []string) error { + m.mu.Lock() + defer m.mu.Unlock() + m.deleted = append(m.deleted, receiptHandles...) + + return nil +} + +func (m *dlqMockSQSReader) deletedHandles() []string { + m.mu.Lock() + defer m.mu.Unlock() + + return append([]string(nil), m.deleted...) +} + +type dlqMockSQSSender struct { + queueURLs []string + bodies []string + mu sync.Mutex +} + +func (m *dlqMockSQSSender) SendMessage(_ context.Context, queueURL, body, _, _ string) error { + m.mu.Lock() + defer m.mu.Unlock() + m.queueURLs = append(m.queueURLs, queueURL) + m.bodies = append(m.bodies, body) + + return nil +} + +func (m *dlqMockSQSSender) sentTo() []string { + m.mu.Lock() + defer m.mu.Unlock() + + return append([]string(nil), m.queueURLs...) +} + +type dlqMockSNSPublisher struct { + topicARNs []string + mu sync.Mutex +} + +func (m *dlqMockSNSPublisher) PublishMessage(_ context.Context, topicARN, _ string) error { + m.mu.Lock() + defer m.mu.Unlock() + m.topicARNs = append(m.topicARNs, topicARN) + + return nil +} + +func (m *dlqMockSNSPublisher) sentTo() []string { + m.mu.Lock() + defer m.mu.Unlock() + + return append([]string(nil), m.topicARNs...) +} + +func dlqBackend() *pipes.InMemoryBackend { + return pipes.NewInMemoryBackend("111122223333", "eu-west-1") +} + +// TestRunner_EnrichmentFailure_RoutesToDLQ verifies that when an enrichment +// invoker is unwired the runner does not silently drop the event: with a DLQ +// configured the failed batch is sent to the DLQ and the source messages are +// deleted; without a DLQ the source messages are retained for redelivery. +func TestRunner_EnrichmentFailure_RoutesToDLQ(t *testing.T) { + t.Parallel() + + tests := []struct { + dlq *pipes.DeadLetterConfig + name string + wantDLQSent bool + wantDeleted bool + }{ + { + name: "enrichment_unwired_with_sqs_dlq", + dlq: &pipes.DeadLetterConfig{Arn: dlqQueueARN}, + wantDLQSent: true, + wantDeleted: true, + }, + { + name: "enrichment_unwired_without_dlq_retains_messages", + dlq: nil, + wantDLQSent: false, + wantDeleted: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + b := dlqBackend() + _, err := b.CreatePipe(pipes.CreatePipeInput{ + Name: tt.name, + RoleARN: "arn:aws:iam::111122223333:role/r", + Source: dlqSource, + Target: dlqLambdaTgt, + Enrichment: dlqEnrichARN, + DesiredState: "RUNNING", + DeadLetterConfig: tt.dlq, + }) + require.NoError(t, err) + pipes.WaitPipeRunning(t, b, tt.name) + + reader := &dlqMockSQSReader{ + messages: []*pipes.SQSMessage{{MessageID: "m1", ReceiptHandle: "rh1", Body: "{}"}}, + } + sender := &dlqMockSQSSender{} + + runner := pipes.NewRunner(b) + runner.SetSQSReader(reader) + runner.SetSQSSender(sender) + // Note: no Lambda invoker wired, so enrichment fails. + + pipes.PollAllPipesOnce(t.Context(), runner) + + if tt.wantDLQSent { + assert.Equal(t, []string{dlqQueueARN}, sender.sentTo(), "failed batch should be sent to DLQ") + } else { + assert.Empty(t, sender.sentTo(), "no DLQ configured: nothing should be sent") + } + + if tt.wantDeleted { + assert.Equal(t, []string{"rh1"}, reader.deletedHandles(), + "source messages should be deleted after DLQ send") + } else { + assert.Empty(t, reader.deletedHandles(), + "without DLQ the source messages must be retained for redelivery") + } + }) + } +} + +// TestRunner_EnrichmentFailure_SNSDLQ verifies SNS-topic dead-letter targets. +func TestRunner_EnrichmentFailure_SNSDLQ(t *testing.T) { + t.Parallel() + + b := dlqBackend() + _, err := b.CreatePipe(pipes.CreatePipeInput{ + Name: "sns-dlq", + RoleARN: "arn:aws:iam::111122223333:role/r", + Source: dlqSource, + Target: dlqLambdaTgt, + Enrichment: dlqEnrichARN, + DesiredState: "RUNNING", + DeadLetterConfig: &pipes.DeadLetterConfig{Arn: dlqSNSTopicARN}, + }) + require.NoError(t, err) + pipes.WaitPipeRunning(t, b, "sns-dlq") + + reader := &dlqMockSQSReader{ + messages: []*pipes.SQSMessage{{MessageID: "m1", ReceiptHandle: "rh1", Body: "{}"}}, + } + sns := &dlqMockSNSPublisher{} + + runner := pipes.NewRunner(b) + runner.SetSQSReader(reader) + runner.SetSNSPublisher(sns) + + pipes.PollAllPipesOnce(t.Context(), runner) + + assert.Equal(t, []string{dlqSNSTopicARN}, sns.sentTo(), "failed batch should be published to SNS DLQ") + assert.Equal(t, []string{"rh1"}, reader.deletedHandles(), "source messages should be deleted after DLQ publish") +} + +// TestRunner_TargetFailure_RoutesToDLQ verifies that an unwired target invoker +// also routes to the DLQ rather than silently succeeding. +func TestRunner_TargetFailure_RoutesToDLQ(t *testing.T) { + t.Parallel() + + b := dlqBackend() + _, err := b.CreatePipe(pipes.CreatePipeInput{ + Name: "target-dlq", + RoleARN: "arn:aws:iam::111122223333:role/r", + Source: dlqSource, + Target: dlqLambdaTgt, // no Lambda invoker wired → target fails + DesiredState: "RUNNING", + DeadLetterConfig: &pipes.DeadLetterConfig{Arn: dlqQueueARN}, + }) + require.NoError(t, err) + pipes.WaitPipeRunning(t, b, "target-dlq") + + reader := &dlqMockSQSReader{ + messages: []*pipes.SQSMessage{{MessageID: "m1", ReceiptHandle: "rh1", Body: "{}"}}, + } + sender := &dlqMockSQSSender{} + + runner := pipes.NewRunner(b) + runner.SetSQSReader(reader) + runner.SetSQSSender(sender) + + pipes.PollAllPipesOnce(t.Context(), runner) + + assert.Equal(t, []string{dlqQueueARN}, sender.sentTo(), "failed target batch should be sent to DLQ") + assert.Equal(t, []string{"rh1"}, reader.deletedHandles(), "source messages should be deleted after DLQ send") +} From cec0f8fa8379b124e05e666e7908b7af22dc368b Mon Sep 17 00:00:00 2001 From: mayor Date: Wed, 10 Jun 2026 17:06:38 -0500 Subject: [PATCH 6/6] test(glacier): wait for async retrieval job to succeed in JobLifecycle The bucket-B parity fix made Glacier retrieval jobs AWS-accurate: jobs now start InProgress and promote to Succeeded only after the simulated retrievalDelay window. The integration test still assumed synchronous completion. Poll DescribeJob until StatusCode=Succeeded/Completed before asserting and calling GetJobOutput. Co-Authored-By: Claude Opus 4.8 --- test/integration/glacier_test.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/test/integration/glacier_test.go b/test/integration/glacier_test.go index 6e92580c6..24e88ff15 100644 --- a/test/integration/glacier_test.go +++ b/test/integration/glacier_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "net/http" "testing" + "time" "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/config" @@ -143,7 +144,8 @@ func TestIntegration_Glacier_JobLifecycle(t *testing.T) { require.NoError(t, err, "InitiateJob should succeed") require.NotEmpty(t, initiateOut.JobId) - // Describe the job. + // Describe the job immediately: AWS retrievals are asynchronous, so a freshly + // initiated job starts InProgress and is not yet completed. descJobOut, err := client.DescribeJob(ctx, &glaciersdk.DescribeJobInput{ AccountId: aws.String("-"), VaultName: aws.String(vaultName), @@ -151,7 +153,18 @@ func TestIntegration_Glacier_JobLifecycle(t *testing.T) { }) require.NoError(t, err, "DescribeJob should succeed") assert.Equal(t, aws.ToString(initiateOut.JobId), aws.ToString(descJobOut.JobId)) - assert.True(t, descJobOut.Completed, "job should complete synchronously in the stub") + + // The job promotes to Succeeded only after the simulated retrieval window + // (services/glacier defaultRetrievalDelay, ~100ms). Poll until it completes. + require.Eventually(t, func() bool { + out, getErr := client.DescribeJob(ctx, &glaciersdk.DescribeJobInput{ + AccountId: aws.String("-"), + VaultName: aws.String(vaultName), + JobId: initiateOut.JobId, + }) + + return getErr == nil && out.Completed && out.StatusCode == glaciertypes.StatusCodeSucceeded + }, 10*time.Second, 50*time.Millisecond, "job should reach Succeeded after the retrieval window") // List jobs. listJobsOut, err := client.ListJobs(ctx, &glaciersdk.ListJobsInput{