[CI-only, do not merge] RDS Data SQL engine — CI validation against main#2379
Closed
agbishop wants to merge 232 commits into
Closed
[CI-only, do not merge] RDS Data SQL engine — CI validation against main#2379agbishop wants to merge 232 commits into
agbishop wants to merge 232 commits into
Conversation
…ings RevokeSecurityGroupEgress rule validation (InvalidPermission.NotFound), DescribeInstances allocation perf, and remaining ec2 Parity/Performance/Leak findings. Table-driven tests. build+vet+test+lint green.
CompleteMultipartUpload empty-parts rejection, S3 Select validation, list pagination, object-lambda/tags leak fixes, replication goroutine drain on Shutdown. 9 table-driven test funcs / 18 sub-cases. build+vet+test+lint green.
…md findings DescribeType full schema, StackSet drift ops (DetectStackSetDrift/ListStackSetOperations/DescribeStackSetOperation), event/stack/operation eviction caps. Table-driven tests. build+vet+test+lint green.
RedrivePolicy/DLQ delivery, DLQ target validation, archive/replay history, region isolation, pagination, FIFO dedup, seq-counter cleanup. Table-driven tests. build+vet+test+lint green.
…ndings Parity fixes: - handleInvokeAsync: fire-and-forget (HTTP 202 immediately, asyncWG.Go for background invocation) - handleInvokeWithResponseStream: proper AWS event stream binary protocol (CRC32/IEEE frames) - sweepESMs: mark enabled ESMs with missing functions as LastProcessingResult="PROBLEM" Performance / leak fixes: - withInvocationChain: []string slice (make+copy) instead of map — no per-call heap alloc - invocationChainContains: slices.Contains — cleaner hot-path - activeConcurrencies: delete map entry when count reaches zero - cleanupSem / logSem: replaced with fresh channels in Reset() so post-reset goroutines don't block on pre-reset channel references Lint / modernize: - Remove //nolint annotations: use loop counters for int→byte/uint16 narrowing (G115) - WaitGroup.Go, maps.Copy, slices.Contains modernize patterns - nlreturn, goconst, gocritic, govet shadow, testifylint, revive fixes Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…d findings PITR snapshot capture, GSI/LSI LastEvaluatedKey base-keys, PartiQL NextToken/ORDER BY/DuplicateItem/error-codes, ExecuteTransaction atomicity, export/import async status, restore GSI/LSI/billing fidelity, OnDemandThroughput, expr cache (parse-once), backup/iterator leak fixes. 65 files, table-driven tests. build+vet+test+lint green.
…ndings Durable-execution + capacity-provider ops implemented (no more no-op stubs), SnapStart, CreateFunction Pending->Active state + LastUpdateStatus, memory validation, ESM health checking. Table-driven tests. build+vet+test+lint green.
…ix parity.md findings TestState nextState, real MapRun data (Describe/List/UpdateMapRun), O(1) status-filtered ListExecutions, SweepTaskTokens lock fix, orphaned-tombstone cleanup, history TTL. Table-driven tests. build+vet+test+lint green.
Parity: - Replace shared package-level mock KMS key with per-instance random AES-256 key (newInstanceGCM). Each InMemoryBackend now generates its own key so ciphertexts are not interchangeable across backend instances, matching AWS KMS key-isolation semantics. Performance: - Remove O(n) expireCommandsLocked call from SendCommand write path; expired-command eviction is the janitor's job (sweepExpiredCommands runs on a configurable interval). SendCommand is now O(1) again. - Remove paramNamesSorted sorted-slice index and its O(n) insert on every PutParameter. Replace collectPathParamsSorted (binary-search) with collectPathParams (linear scan + sort-at-read). PutParameter write path drops from O(n) to O(1); GetParametersByPath is O(n log n) which is acceptable for an emulator. Also fixes a silent post-Restore bug where GetParametersByPath returned empty (sorted slice was never rebuilt from the snapshot). Leaks: - Add cleanupEmptyParamRegion: after DeleteParameter / DeleteParameters removes the last entry in a region, the empty parameters/history/tags inner maps are deleted so they don't accumulate indefinitely. Tests: - Update leak_test.go: TestSendCommand_PrunesExpiredCommands → TestJanitor_PrunesExpiredCommands; verify janitor sweeps expired commands and that SendCommand no longer does so. - Add parity_fixes_test.go with table-driven tests for all four fixes: per-instance key isolation, self-roundtrip encryption, linear-scan path lookup (sort order, recursive/non-recursive, no-match), and region-map cleanup via single and batch delete. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ndexes, fast policy lookup Parity: - Give each NoSuchEntity sentinel a distinct message suffix (user, role, policy, group, access key, instance profile, inline policy, SAML provider, OIDC provider, login profile) so callers can identify the missing resource type via .Error() inspection without relying on errors.Is pointer identity alone. HTTP error code is still "NoSuchEntity" matching AWS. - Fix collectNamedEntityPolicies to use policyByARN map for O(1) ARN → name resolution instead of the previous O(n) linear scan over all policies. Performance: - Add sortedUserNames, sortedRoleNames, sortedPolicyNames, sortedGroupNames, sortedIPNames []string indexes to InMemoryBackend, maintained via binary-search insert/delete on every create/delete. ListUsers, ListRoles, ListPolicies, ListGroups, ListInstanceProfiles now paginate via pageFromSortedNames which resolves the base64-encoded integer marker token in O(1) and builds each page in O(k) (page size), eliminating the previous O(n log n) sort rebuild on every list call. - rebuildIndexesLocked and Purge both rebuild sorted name indexes via rebuildSortedNames, keeping persistence and compaction paths consistent. Tests: - TestParityIAM_ErrorSentinelDistinctness: table-driven, verifies each sentinel has a unique .Error() string. - TestParityIAM_ErrorSentinelUniqueness: all pairs are not-equal via errors.Is. - TestParityIAM_ErrorSentinelWrapping: backend errors wrap the correct sentinel. - TestParityIAM_HandlerNoSuchEntityCode: handler returns 400 + "NoSuchEntity" for all not-found operations. - TestParityIAM_SimulatePrincipalPolicy: real policy evaluation (allow, deny, no policy, explicit deny overrides allow). - TestParityIAM_CredentialReportColumns: all 22 AWS-required CSV columns present. - TestParityIAM_ListPaginationSortedOrder: multi-page pagination stays sorted. - TestParityIAM_SortedIndexMaintainedAfterDelete: index correct after deletion. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ons, generic cleanup helper - Add cleanupEmptyInnerMap[V any] generic helper that removes a region key from any two-level map when the inner map becomes empty; replaces the three-field cleanupEmptyParamRegion body with calls to this helper - Wire cleanup to all delete paths: DeleteActivation, DeleteAssociation, DeregisterTargetFromMaintenanceWindow, DeregisterTaskFromMaintenanceWindow, DeleteMaintenanceWindow, DeletePatchBaseline, DeleteOpsItem, DeleteOpsMetadata, DeleteInventory, DeleteResourceDataSync, DeleteResourcePolicy, DeleteDocument - Janitor sweepExpiredCommands now cleans up commands/commandInvocations maps after eviction so empty region buckets do not accumulate - GetMaintenanceWindowExecution/Task/Invocation now populate StartTime, EndTime, StatusDetails, TaskARN, TaskType, Priority, MaxConcurrency, MaxErrors, InvocationID, WindowTargetID from stored window/task data - Expand three MW execution output types with full AWS-accurate fields - Fix handler_batch2_test to create a real window before GetMaintenanceWindowExecution - Add TestGetMaintenanceWindowExecution_FullOutput and TestOtherMapsRegionCleanup table-driven tests covering new behavior (all t.Parallel) - Zero golangci-lint issues; struct field order corrected by fieldalignment
…+ fixes ~120 stubbed ops given real state mutation (ResourceDataSync, Inventory, Activations, maintenance windows w/ MaxConcurrency/MaxErrors/InvocationID/WindowTargetID), GetParametersByPath prefix index, param/doc/command history AWS caps, MaxResults bounds. Table-driven tests, 0 lint.
Policy-eval correctness, pagination/validation fidelity, sub-ops. Table-driven tests, 0 lint.
20+ empty-struct stubs given real data/state (GetBlueprintRun, GetPlan, ImportCatalogToGlue, column-statistics, schema-versions-diff, usage-profile), StopCrawler STOPPING->READY reconcile. Table-driven tests, 0 lint.
Four parity/leak/perf fixes for services/kms: Parity: keyIDResolutionCache not invalidated on DisableKey / ScheduleKeyDeletion. Alias→keyID entries cached before a key state change persisted in the cache, allowing stale hits to bypass the aliasesStore and return the old keyID. Fix: call evictAliasesFromCache (O(aliases-for-key)) in both operations. Performance: clearResolutionCache used sync.Map.Range+Delete (O(n)) and was called on every alias mutation (Create/Update/Delete). Fix: change keyIDResolutionCache to *sync.Map so clearResolutionCache swaps the pointer in O(1); replace full-cache clears in alias mutations with single targeted Delete calls; remove the post-sweep clearResolutionCache call from sweepExpiredKeys since purgeKey now evicts targeted entries. Leaks: purgeKey in the janitor deleted keys, aliases, grants, and key material but never removed the corresponding lastUsage sync.Map entry, causing unbounded growth. Fix: add lastUsage.Delete in purgeKey. Tests: table-driven tests in parity_fixes_test.go covering all four fixes; test helpers (ResolutionCacheLen, ResolutionCacheHas, LastUsageExists) added to export_test.go.
…ty.md findings Parity: - GetMetricWidgetImage: return minimal valid 1×1 PNG (base64) instead of empty stub - ListAlarmMuteRules: add backend method + wire handler to return stored rules - ListManagedInsightRules: add backend method filtering ManagedRule=true; wire handler - PutManagedInsightRules: parse ManagedRules.member.N.* and store with ManagedRule=true - EC2/AutoScaling alarm actions: log explicit warnings instead of silent no-op Performance: - SweepExpiredMetrics: two-phase sweep (read-lock snapshot → out-of-lock filter → write-lock apply) avoids holding global write lock for full O(series×points) scan - PutMetricData: extract storeDatum helper; move stream delivery timestamp update to a second, shorter write-lock acquisition outside the main metrics write section - Extract sweepScanCandidates / sweepApplyResults / hasExpiredPoint helpers to reduce cognitive complexity below gocognit threshold Leaks: - deleteResourceTags: call t.Close() before removing from map to deregister the per-resource Prometheus lockmetrics entry and prevent unbounded registry growth Tests: new parity_test.go with table-driven tests for all fixed ops Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…service revisions, state changes - Implement all 12 daemon CRUD operations with full state tracking (CreateDaemon, DeleteDaemon, DescribeDaemon, ListDaemons, UpdateDaemon and task-def/deployment/revision ops) - Track service revisions on CreateService and UpdateService; implement DescribeServiceRevisions - Fix DiscoverPollEndpoint to return region-specific endpoints (was hardcoded) - Implement SubmitTaskStateChange, SubmitContainerStateChange, SubmitAttachmentStateChanges with cluster/task validation instead of no-op stubs - Fix enrichCluster O(n) task scan: cache RunningTasksCount/PendingTasksCount on Cluster and maintain counters at every state transition (RunTask, StopTask, startTasksOutsideLock) - Add Backend interface methods for all new ops; add GetRegion() on InMemoryBackend - Add region field to Handler populated from backend for regional URL generation - 19 table-driven tests covering all new operations and counter correctness Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ty.md findings findGrantByToken token index (O(1)), metric-datapoint ring buffer, alarmHistory cross-alarm cap. Table-driven tests. build+vet+test+lint green.
Extract reEncryptDecrypt (source key lookup, validation, decrypt) and reEncryptEncrypt (dest key lookup, validation, encrypt) so ReEncrypt itself is ~34 lines, well under the 100-line funlen limit. No behaviour change.
findGrantByToken O(1) token index, keyMaterialHistory migration (decrypt older ciphertexts), grant/material leak fixes, ReEncrypt refactored under funlen. Table-driven tests. build+vet+test+lint green.
getServicesForReconciler bounded iteration, docker containers map leak on failed StopTask, reconciler perf. Table-driven tests across 28 files. build+vet+test+lint green.
GetCallerIdentity InvalidClientTokenId (400 not 403), session TOCTOU fix, session-store sweep/leak bound. Table-driven tests. build+vet+test+lint green.
…loudFormation (intrinsics, real resource provisioning) toward LocalStack parity
…cas, param groups) toward LocalStack parity
… applies now (matches AWS deferral semantics added in audit)
…d LocalStack parity The RDS Data API previously executed no SQL: ExecuteStatement returned an empty record set and ExecuteSql always reported 0 rows updated, so the service modelled the control plane only. This closes the #1 functional gap versus LocalStack (which runs a real database behind the Data API). - Add services/rdsdata/engine.go: a pure-Go (modernc.org/sqlite, no cgo) per-(region, resourceARN) in-memory SQL engine. Each Aurora resource gets an isolated shared-cache database pinned by a keep-alive connection, scoped by an engine-instance nonce so two backends never alias the same store. - ExecuteStatement / BatchExecuteStatement / ExecuteSql now run statements for real, returning genuine records, column metadata and update counts. Anything the engine rejects (e.g. DML against a table that was never created) degrades to the historical empty-success envelope, preserving backward compatibility. - BeginTransaction opens a matching engine transaction; Commit/Rollback finalize it, giving real atomic visibility for statements tagged with a transactionId. - Restore replays the recorded statement log so table state survives a snapshot/restore cycle (best-effort for parameterised/trimmed statements). - README: correct the stale "image-based only" Lambda claim — Zip packaging across the managed runtimes is supported and already implemented. Adds engine_test.go (round-trip, isolation, transactions, batch, snapshot replay, reset, lenient fallback); package coverage 89.6%. Existing unit and integration tests are unchanged and still pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GmKxLo2kS6qNWrTFuHhaEb
…tions) + KMS (key lifecycle, grants, rotation) toward LocalStack parity
…a, query, destroy Adds an `engine_roundtrip` case to TestTerraform_RDSData that provisions an Aurora cluster via Terraform, then drives the full Data API data path against it: CREATE TABLE, INSERT rows, and SELECT them back asserting the real round-tripped values (long and string fields) plus column metadata. The terraform harness tears the cluster down on cleanup, so the case covers create-engine -> add-data -> query -> destroy end to end and exercises the new in-memory SQL engine through the SDK + Terraform path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GmKxLo2kS6qNWrTFuHhaEb
…ller identity) + CloudWatch (metric aggregation, alarm state machine) toward LocalStack parity
ExecuteStatement previously dropped bound parameters — only the batch path threaded them — so the common Aurora Data API pattern (e.g. `WHERE id = :id`) fell back to the empty-result envelope. The backend method now accepts variadic SQLParameters (keeping every existing call site source-compatible) and the handler forwards req.Parameters, so named placeholders are substituted on both the write and read paths. Adds a parameterised round-trip test (insert with :id/:name, then select by :id). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GmKxLo2kS6qNWrTFuHhaEb
…ery, scheduled rules) toward LocalStack parity
…class change TestSDK_RDS_FullLifecycle modified DBInstanceClass without ApplyImmediately and asserted the new class took effect immediately. Since RDS ModifyDBInstance was deepened to defer instance-class changes to PendingModifiedValues (AWS-accurate behaviour when ApplyImmediately is unset), the live class correctly stayed unchanged and the assertion failed. The test now passes ApplyImmediately=true, which is how a real client forces an immediate instance-class change. Pre-existing failure on the parity-sweep base, unrelated to the rdsdata engine change; surfaced by running CI for the first time. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GmKxLo2kS6qNWrTFuHhaEb
…ts Manager (version staging, rotation, recovery) toward LocalStack parity
Collaborator
Author
📊 Code Coverage Report
Tip This project maintains a minimum coverage threshold of 85%. Maintain or improve coverage on new code to ensure long-term stability. Last updated: Sun, 28 Jun 2026 17:59:15 GMT |
…nd is allowed (matches AWS opt-in added in audit)
… + Route53 (record types, routing policies, health checks) toward LocalStack parity
…s) + ECR (layer upload, manifests, lifecycle policies) toward LocalStack parity
…eets From a fresh, adversarially-verified parity audit. Three confirmed "stub that lies" gaps fixed, each covered by an SDK-driven integration test: - cloudformation StopStackSetOperation: handler discarded the backend error and always returned HTTP 200. It now propagates OperationNotFoundException (missing op) / InvalidOperationException (op not RUNNING). - cloudformation ListStackSetAutoDeploymentTargets: handler discarded the backend StackSetNotFound error and returned an empty list. It now returns StackSetNotFoundException for a missing stack set. - ec2 DescribeFleets: a stub registered in registerStubOps ran after registerBatch5Ops and shadowed the real handler, so DescribeFleets always returned an empty fleet set. Removed the stub override (and the now-unused stub method); the real handler that reads backend fleet state now serves it. Existing cloudformation unit tests that documented the old error-swallowing behaviour are updated to assert the corrected error responses. Integration tests: test/integration/parity_audit_fixes_test.go covers the two CFN not-found error paths and a CreateFleet -> DescribeFleets round-trip. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GmKxLo2kS6qNWrTFuHhaEb
… issuance, auth challenges, user pools) toward LocalStack parity
…e (partition expression eval, catalog) toward LocalStack parity
…lates) + Athena (query-execution lifecycle, result sets, workgroups) toward LocalStack parity
…ified-identity gating added in audit)
Closes the remaining confirmed gaps from the adversarially-verified parity
audit (the 3 prior CFN/EC2 gaps were fixed earlier). Each service's "stubs that
lie" / missing / incorrect operations now read or mutate real backend state,
validate existence with AWS-accurate errors, and are covered by SDK-driven
integration tests under test/integration/<svc>_audit_test.go (plus in-package
unit tests where the SDK has no create path).
Highlights by service:
- iam: GetMFADevice/ResyncMFADevice validate real devices; GetContextKeysForCustomPolicy parses policy condition keys.
- ec2: ModifyInstance{Placement,CpuOptions,MaintenanceOptions,NetworkPerformanceOptions} and AssociateInstanceEventWindow persist real state (real handlers unshadowed from the stub path).
- cloudformation: PublishType/DeleteGeneratedTemplate/DeactivateType propagate not-found errors; UpdateTerminationProtection returns StackId.
- cloudwatch: TestMetricFilter/GetInsightRuleReport/GetMetricWidgetImage/List{AlarmMuteRules,ManagedInsightRules}/PutManagedInsightRules now have working CBOR dispatch.
- cloudwatchlogs: GetLogFields/TestTransformer/ListAggregateLogGroupSummaries compute real results; StartLiveTail/DescribeImportTaskBatches validate inputs.
- route53: GetAccountLimit/GetHostedZoneLimit/GetReusableDelegationSetLimit report real backend-derived counts.
- cognitoidp: GetSigningCertificate generates a stable X.509 PEM; AdminLinkProviderForUser links identities.
- sesv2: PutConfigurationSet{Archiving,Vdm}Options, PutDedicatedIP{InPool,WarmupAttributes}, UpdateReputationEntity* store and read back state.
- elbv2: GetResourcePolicy/DescribeCapacityReservation/ModifyCapacityReservation/ModifyIpPools/DeleteSharedTrustStoreAssociation use real state.
- ecs: ListServices/ListContainerInstances honor cluster/status filters and pagination.
- ecr: DescribeImageReplicationStatus computes per-destination status from the replication config.
- apigateway: ImportApiKeys/GetDomainNameAccessAssociations are real; GetSdk validates input.
- apigatewayv2: ImportApi/ReimportApi parse the OpenAPI spec into routes/integrations.
- sqs: 7 tag/DLQ/message-move ops wired into the Query-protocol dispatch.
- rds: DescribeDBLogFiles/DownloadDBLogFilePortion serve seeded log files; ModifyCertificates persists the default CA.
- sns/lambda: ListOriginationNumbers/ListFunctionVersionsByCapacityProvider are stateful with correct pagination (population via internal seeding; AWS exposes no public create API).
Where AWS has no API to populate state, the operation now does AWS-accurate
validation instead of fabricating data. New backend fields are added to each
service's persistence snapshot. Existing unit tests that asserted the old
buggy behavior were updated to the corrected behavior. Full module builds,
golangci-lint is clean, and unit tests pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GmKxLo2kS6qNWrTFuHhaEb
…AC) + ELBv2 (target health, listener rules, conditions) toward LocalStack parity
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| out, err := h.Backend.StartMessageMoveTask(&StartMessageMoveTaskInput{ | ||
| SourceArn: vals.Get("SourceArn"), | ||
| DestinationArn: vals.Get("DestinationArn"), | ||
| MaxNumberOfMessagesPerSecond: int32(maxPerSec), //nolint:gosec // bounded form-supplied value |
|
|
||
| out, err := h.Backend.ListMessageMoveTasks(&ListMessageMoveTasksInput{ | ||
| SourceArn: vals.Get("SourceArn"), | ||
| MaxResults: int32(maxResults), //nolint:gosec // bounded form-supplied value |
…a-sql-engine # Conflicts: # test/integration/rds_test.go
…r parity-sweep merge Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GmKxLo2kS6qNWrTFuHhaEb
…a-sql-engine # Conflicts: # services/elbv2/backend.go
Collaborator
Author
|
Closing — this was a CI-validation-only PR. The work (RDS Data SQL engine + 63 LocalStack-parity gap fixes) has been merged into Generated by Claude Code |
| tokenStr = strings.TrimPrefix(tokenStr, "bearer ") | ||
|
|
||
| // ParseUnverified: we check structural validity, exp, iss, aud — not signature. | ||
| token, _, parseErr := new(jwt.Parser).ParseUnverified(tokenStr, jwt.MapClaims{}) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Do not merge. This PR exists only to run the full CI suite (
ci.ymllint + unit + integration + e2e + coverage) against theclaude/rdsdata-sql-enginebranch, because CI does not trigger for the real PR (#2378) which targetsparity-sweep.The intended merge target is #2378 →
parity-sweep. Once CI here is green, this PR will be closed.Changes under validation (same commits as #2378):
modernc.org/sqlite, pure Go).ExecuteStatement.🤖 Generated with Claude Code
Generated by Claude Code