Full AWS RDS parity: management plane, discovery, cost & metrics

[thzgajendra]

Objective

Bring cloudemu's AWS RDS emulation from basic coverage to comprehensive management-plane parity, surface RDS in resource discovery (issue #295 workstream A), and wire it into the cost tracker — as one PR, broken into independently-green commits.

What was there before

24 wired actions: instances, Aurora clusters, instance + cluster snapshots, DB subnet groups; engines mysql/postgres/aurora-*/docdb/neptune; 5 CloudWatch metrics. Not surfaced in discovery; not in the cost catalog.

What this adds (55 new actions across 8 feature groups)

Each is an optional capability discovered by type assertion (mirroring the existing SubnetGroups pattern), so non-AWS relational drivers answer InvalidAction truthfully.

Commit	Feature	Actions
bb5d92c	DB + DB cluster parameter groups (+ params, reset, copy)	14
0adaae6	Option groups (+ copy, DescribeOptionGroupOptions)	6
a2cedf6	Read replicas (create + promote)	2
011bb75	Copy snapshot (instance+cluster) + point-in-time restore	4
1aa5104	RDS Proxy (proxies, targets, target groups)	8
82201aa	Event subscriptions + DescribeEvents + DescribeEventCategories	6
a07bb84	Aurora custom cluster endpoints + FailoverDBCluster + global clusters	10
219d0dd	Describe-metadata (engine versions, orderable options) + tag ops	5

Plus:

• 91e191c Discovery (AWS): a RelationalDatabases capability + rdsDiscovery adapter (mirrors the merged Kubernetes eksDiscovery pattern, keeping services/ free of provider imports) + walkRelationalDB; RDS/Aurora instances, clusters and snapshots now enumerate through Resource Explorer 2 (service:rds). GCP Cloud SQL / Azure SQL discovery are intentionally out of scope.
• 9bfef17 Cost + metrics: relationaldb:* rates added to the cost tracker's catalog; instance metrics enriched (FreeStorageSpace, Read/WriteLatency, Network{Receive,Transmit}Throughput), latency/throughput read zero when stopped.
• 41e897a lint/cleanup sweep.

Design notes / deliberate limitations (emulator honesty)

• DescribeEvents returns an empty list — the emulator retains no event timeline, so there are truthfully no events for any window. Event subscriptions are full CRUD.
• DescribeOptionGroupOptions / DescribeDBEngineVersions / DescribeOrderableDBInstanceOptions return a stable, representative per-engine catalog, not AWS's exhaustive region-specific lists.
• Point-in-time restore clones the source's current spec (no historical timeline); RestoreTime/UseLatestRestorableTime are accepted but not replayed.
• Tagging is addressed by ARN over the tag-bearing stores (instances, clusters, instance/cluster snapshots).
• Cost follows the existing model: the tracker is a rate catalog (as for sagemaker/vertexai/azureai); this adds RDS rates rather than inventing a new hourly-pricing subsystem.

Test plan

• Every commit adds provider unit tests (newTestMock, table-driven, cerrors code assertions) and aws-sdk-go-v2 SDK round-trip tests (real client → in-memory handler), incl. an RE2 indexing round-trip for discovery.
• go build ./..., go vet ./..., gofmt -l, and the full go test ./... suite pass.
• golangci-lint run is clean on all touched packages.

Risk & rollback

Additive and capability-gated; no existing action's behavior changes. Each commit is independently green — rollback = revert the relevant commit(s).

Follow-ups (out of scope)

GCP Cloud SQL / Azure SQL discovery (issue #295 item 1 for the other clouds); real hourly RDS pricing; a recorded event timeline for DescribeEvents.

[NitinKumar004]

Review — full AWS RDS parity (deep pass: edge cases, blast radius, cascade)

Substantial, well-structured PR — 55 actions across 8 capability groups, correctly using the optional-capability type-assertion pattern, idgen ARNs, cerrors, and mirroring the merged Kubernetes discovery adapter. Build/tests pass. The findings below are what a deep sweep surfaced; the two HIGH items are worth fixing before merge.

✅ Blast radius — clean

• 55 new actions are purely additive: all 24 existing RDS actions route byte-for-byte unchanged, default: still returns InvalidAction. The notFoundCode/alreadyExistsCode refactor to ordered fault-tables preserves every existing code (most-specific-first; "DB cluster snapshot" precedes "DB cluster"; new keywords aren't substrings of existing messages).
• New xml.go fields are ,omitempty → existing response bytes identical.
• All 12 new capabilities are separate optional interfaces (comma-ok assertion, writeUnsupported→InvalidAction); Azure/GCP relational drivers unchanged; no nil-panic in wiring.
• resourceexplorer2 service:rds mapping is additive, no collision.

🔴 HIGH

1. Concurrent-map-write panic (crashes the process) — Tags & Parameters. memstore returns shallow struct copies, so map fields alias stored state. Describe paths leak the live map (DescribeInstances rds.go:289-291, DescribeClusters 491, DescribeSnapshots 645; DescribeDBParameterGroups parametergroup.go:95, cluster variant 248), and the mutators write in place: AddTagsToResource/RemoveTagsFromResource (existing[k]=v / delete, metadata.go:145-159), ModifyDBParameterGroup/ResetDBParameterGroup (parametergroup.go:126,171). A DescribeInstances-range concurrent with a tag write → fatal error: concurrent map read and map write. Same class as the sibling-PR panics. The correct pattern already exists in ModifyInstance (inst.Tags = copyTags(...)) — apply it on the Describe read paths and have the mutators replace rather than mutate.

2. DeleteInstance doesn't block on read replicas. rds.go:350 only unlinks from the cluster; it never inspects inst.ReadReplicaTargets. Deleting a source that still has replicas succeeds and leaves every replica with ReadReplicaSource pointing at a missing instance. Real AWS rejects with InvalidDBInstanceState until replicas are promoted/deleted. Expected: cerrors.FailedPrecondition when len(inst.ReadReplicaTargets) > 0.

🟠 MEDIUM

• Deleting a replica leaves a stale entry on its source (rds.go:350): the deleted replica isn't stripped from src.ReadReplicaTargets, so DescribeInstances still lists a replica that's gone. PromoteReadReplica does this cleanup correctly — DeleteInstance is the inconsistent path.
• DeleteGlobalCluster has no member guard (aurora.go:276): deletes unconditionally regardless of gc.Members; AWS blocks with InvalidGlobalClusterStateFault. Asymmetric vs DeleteCluster, which guards correctly.
• In-place slice filters corrupt aliased backing arrays. DeregisterDBProxyTargets p.Targets[:0] (dbproxy.go:191), RemoveFromGlobalCluster gc.Members[:0] (aurora.go:301), ModifyOptionGroup og.Options[:0] (optiongroup.go:127) — combined with the Describe paths leaking those same slices (DescribeDBProxies Targets/Auth/Subnets/SGs, DescribeGlobalClusters Members, DescribeDBClusterEndpoints Static/Excluded, DescribeEventSubscriptions SourceIDs/EventCategories, DescribeOptionGroups Options), a previously-returned slice gets clobbered underneath a Go-library caller. DescribeDBProxyTargets (dbproxy.go:214) already does the right thing (append([]ProxyTarget(nil), …)) — mirror it.
• DeleteDBSubnetGroup takes no m.mu lock (subnetgroup.go:91) — the only mutating RDS method without one. The in-use scan + delete aren't atomic vs a concurrent CreateInstance placing an instance into the group (strands it in a deleted group).
• Parameter/option groups: no in-use enforcement, attachment unmodeled. Delete{DB,DBCluster}ParameterGroup/DeleteOptionGroup are bare store.Delete — no instance/cluster carries a DBParameterGroupName/OptionGroupName, so "in use" can't be checked and default groups aren't seeded/protected. AWS returns InvalidDBParameterGroupState / refuses default.*. The honest parity gap; modeling attachment is a prerequisite.
• ApplyMethod silently dropped (parametergroup.go:56): mergeParams stores only name→value and paramsToDriver hardcodes pending-reboot, so ModifyDBParameterGroup(...,ApplyMethod:"immediate") reads back as pending-reboot.

🟡 LOW (fidelity / robustness)

• PITR: aliases source's VPCSecurityGroups slice (advancedrestore.go:120) and retains src.ClusterID without adding the clone to cluster.Members (orphan member reference). RestoreTime accepted-but-unused is documented and genuinely inert (good).
• Read replica off a non-available source isn't state-checked (readreplica.go:16).
• RDS Proxy: CreateDBProxy doesn't validate RoleArn/Auth; the targetGroup arg is ignored (typo target group silently succeeds); duplicate register appends a dup; deregister of an unregistered target returns nil (AWS: DBProxyTarget{AlreadyRegistered,NotFound}Fault).
• Global clusters: no way to add a secondary (CreateDBCluster ignores GlobalClusterIdentifier, no AddClusterToGlobalCluster); RemoveFromGlobalCluster of a non-member returns success.
• Event subscriptions: no SourceType/SourceIds validation; Add/RemoveSourceIdentifierFromSubscription unimplemented. DescribeEvents empty list confirmed clean (non-nil, no panic).
• Custom endpoints: EndpointType unvalidated; DescribeDBClusterEndpoints returns the endpoint even when the passed clusterID doesn't match.
• Metadata: DescribeOrderableDBInstanceOptions uses a supplied EngineVersion verbatim (nonexistent version → full results); unknown engine → empty list instead of an error (also DescribeOptionGroupOptions).
• Nondeterministic tag XML order (xml.go:420 toTagListXML ranges the map) — sort keys.
• subnetgroup.go:56 hand-builds the ARN instead of idgen.AWSARN (byte-identical output, but the one ARN bypassing idgen).
• DescribeEventCategories returns the package-global catalog slices verbatim (caller could mutate the shared table).
• Missing compile-time conformance guards (var _ rdsdriver.X = (*Mock)(nil)) for SubnetGroups, ClusterEndpoints, ClusterFailover, GlobalClusters, Metadata, Tagging — a signature drift would silently degrade to InvalidAction at runtime instead of failing the build.
• Metrics: RDS emits a single datapoint at now (no 5-point backfill like EC2), so a GetMetricStatistics window ending before now is empty. emitInstanceMetrics runs under m.mu.Lock across PutMetricData+evaluateAlarms — safe today (RDS isn't an alarm-action target) but the lock-held-across-external-call shape CLAUDE.md warns about.

Verified correct (no action)

Cluster-member delete block; PromoteReadReplica; subnet-group in-use scan; proxy delete cascade; snapshot/PITR existence + AlreadyExists guards; FailoverDBCluster (single-member no-op is a deliberate, tested choice; unknown/non-member checks correct); discovery pattern conformance (fail-loud, engine free of provider imports, copyTags on emitted Resource); cost catalog (additive, correct naming); metrics namespace/dimension/stopped-check; determinism of the describe-metadata catalogs; ARN-parse robustness in tagging (no panic on malformed).

Bottom line

Architecturally sound and blast-radius-clean. The two HIGH items — the Tags/Parameters concurrent-map-write panic (fix by copying on read + replacing on write, mirroring ModifyInstance) and the read-replica delete block — are the ones to fix before merge. The MEDIUM aliasing/slice-mutation set is the same theme and worth sweeping together (mirror DescribeDBProxyTargets). The param/option-group in-use enforcement is the honest parity limitation to call out. A concurrency -race test that runs a Describe alongside a tag/param mutation would catch the HIGH class.

[thzgajendra]

Thanks @NitinKumar004 — thorough pass. All fixed in f224ff8.

🔴 HIGH

1. Concurrent-map-write (Tags & Parameters). Applied both halves of your recommendation: copy-on-read in the Describe paths (DescribeInstances/DescribeClusters/DescribeSnapshots/DescribeClusterSnapshots copy Tags; parameter groups now return copied maps) and replace-on-write in the mutators — AddTagsToResource/RemoveTagsFromResource build a fresh map; ModifyDBParameterGroup/ResetDBParameterGroup build a fresh map (mirroring ModifyInstance). Added TestConcurrentDescribeAndMutate that runs Describe + caller-side map iteration against tag/param writes — passes under go test -race (panicked before the fix).
2. DeleteInstance now blocks on read replicas → FailedPrecondition when len(ReadReplicaTargets) > 0.

🟠 MEDIUM

• Deleting a replica now strips it from its source's ReadReplicaTargets; removeString is non-mutating so it can't corrupt a slice a Describe handed out.
• DeleteGlobalCluster refuses while members remain (FailedPrecondition).
• Fresh-slice rebuilds (no in-place [:0]/append-into-aliased) in DeregisterDBProxyTargets, RegisterDBProxyTargets, RemoveFromGlobalCluster, ModifyOptionGroup — mirroring DescribeDBProxyTargets.
• DeleteDBSubnetGroup now holds m.mu across the in-use scan + delete.
• ApplyMethod preserved — parameter groups store map[string]Parameter now, so ApplyMethod:"immediate" round-trips.

🟡 LOW addressed: deterministic tag XML ordering (sorted keys); DescribeEventCategories/PITR return copied slices; PITR clears ClusterID (no orphan member ref) and copies the SG slice; added the SubnetGroups conformance guard (the other five — ClusterEndpoints/ClusterFailover/GlobalClusters/Metadata/Tagging — already have var _ guards in the aurora/metadata var (...) blocks); subnet-group ARN now via idgen.AWSARN.

Deliberately deferred (documented parity gaps, agree they're the honest limitations): parameter/option-group in-use enforcement requires modeling attachment (DBParameterGroupName/OptionGroupName on instances + seeded default.* groups) — a follow-up; read-replica/endpoint source-state validation; proxy RoleArn/Auth/target-group validation; global-cluster secondary attach; 5-point metric backfill. Happy to take these as a follow-up PR.

New tests: -race concurrency, delete-blocked-by-replica, delete-global-with-members, ApplyMethod round-trip. build/vet/gofmt/full go test ./... green; golangci-lint clean on touched packages.

[NitinKumar004]

Re-review @ f224ff8 — both HIGH + the MEDIUM set fixed, race-tested

Verified each fix directly; go test -race ./providers/aws/rds/... is green.

✅ Fixed & verified

• HIGH — concurrent-map-write panic (Tags/Parameters). Describe paths now copy on read (DescribeInstances v.Tags = copyTags(v.Tags), clusters/snapshots likewise; parameter groups copy too), and the mutators replace-on-write (AddTagsToResource/RemoveTagsFromResource build a fresh map; parameter Modify/Reset rebuild). Mirrors ModifyInstance. New TestConcurrentDescribeAndMutate runs Describe+iteration vs tag/param writes under -race. Panic class closed.
• HIGH — DeleteInstance blocks on read replicas (FailedPrecondition), and now also strips a deleted replica from its source's ReadReplicaTargets. TestDeleteInstanceBlockedByReplica covers it.
• MED — removeString is non-mutating (fresh slice, no [:0]), so it never corrupts a slice a Describe handed out.
• MED — DeleteGlobalCluster blocks while members remain (TestGlobalClusterLifecycle).
• MED — fresh-slice rebuilds in DeregisterDBProxyTargets/RegisterDBProxyTargets/RemoveFromGlobalCluster/ModifyOptionGroup (no in-place [:0]/append into aliased backing).
• MED — DeleteDBSubnetGroup now holds m.mu across the in-use scan + delete.
• MED — ApplyMethod preserved (stores map[string]Parameter); TestDBParameterGroupApplyMethodRoundTrips.
• LOW — deterministic tag XML order, PITR clears ClusterID + copies slices, DescribeEventCategories copies, subnet-group ARN via idgen, SubnetGroups conformance guard added.

🟡 Remaining (all LOW / acknowledged parity)

1. Read-side slice aliasing still present in the Describe paths. The mutator (write) side was fixed, which closes the concurrent-corruption vector — but DescribeDBProxies (Targets/Auth/subnets/SGs), DescribeGlobalClusters (Members), DescribeDBClusterEndpoints (Static/Excluded), DescribeEventSubscriptions (SourceIDs/EventCategories), DescribeOptionGroups (Options) still return slices that alias the store. A Go-library caller mutating a returned slice corrupts internal state. Now inconsistent with the tags path, which copies on read — worth mirroring copyTags for these slices (or DescribeDBProxyTargets's append([]T(nil), …)). Panic-vector-closed, so LOW.
2. Parameter/option-group in-use enforcement still absent (the honest parity gap): no instance/cluster carries a DBParameterGroupName/OptionGroupName, so Delete{,Cluster}ParameterGroup/DeleteOptionGroup can't refuse an in-use or default.* group. Modeling attachment is the prerequisite; reasonable to call out as a scoped limitation rather than block.
3. Missing compile-time conformance guards for ClusterEndpoints, ClusterFailover, GlobalClusters, Metadata, Tagging (SubnetGroups + 6 others now guarded) — a signature drift on these still degrades silently to InvalidAction.
4. Prior LOW fidelity items out of this commit's scope remain: CreateDBProxy RoleArn/Auth validation + ignored targetGroup + dup-register/deregister-unregistered handling; global-cluster secondary-member gap; event-subscription SourceType/SourceIds validation + Add/RemoveSourceIdentifier; custom-endpoint EndpointType validation + cluster-mismatch describe; metadata engine/version validation; single metric datapoint (no EC2-style backfill).
5. Lint (verify against CI): my local golangci-lint flags dupl on the subnet-group/aurora/parameter-group ARN+scan helpers (aurora.go:223↔subnetgroup.go:68, server/aws/rds/parametergroup.go:438↔subnetgroup.go:87). Could be linter-version drift (you report clean) — worth a confirm; extracting the shared helper would settle it.

Bottom line

The two HIGH items and the whole MEDIUM aliasing/delete-guard set are correctly fixed and now race-tested — this addresses everything blocking from the first pass. What's left is LOW: read-side slice copies (mirror the tags fix for consistency), the param/option-group in-use parity gap (scoped limitation), and a handful of fidelity/validation nits. Solid.

[thzgajendra]

Thanks @NitinKumar004 — LOWs swept in 7c58b7a.

#1 — read-side slice aliasing. The Describe paths now copy their slice/map fields so a returned value never aliases the store, consistent with the tags copy-on-read (added a generic cloneSlice[T] helper):

• DescribeDBProxies → Targets/Auth/VpcSubnetIds/VpcSecurityGroupIds
• DescribeGlobalClusters → Members
• DescribeDBClusterEndpoints → StaticMembers/ExcludedMembers
• DescribeEventSubscriptions → SourceIds/EventCategories
• DescribeOptionGroups → Options
• DescribeDB{,Cluster}ParameterGroups → Parameters maps

Regression test TestDescribeDBProxiesReturnsIndependentCopies mutates a returned slice and asserts the store is unchanged.

#2 — parameter/option-group in-use enforcement. Modeled the attachment: instances now carry DBParameterGroupName/OptionGroupName and clusters DBClusterParameterGroupName (parsed from the wire on create). Delete{,Cluster}ParameterGroup and DeleteOptionGroup now refuse an in-use group (FailedPrecondition) and refuse the reserved default.* / default:* names. Tests: TestParameterGroupDeleteGuards, TestClusterParameterGroupDeleteGuard, TestDeleteOptionGroupGuards.

#3 — conformance guards. Already present — ClusterEndpoints/ClusterFailover/GlobalClusters are in the var (...) block in aurora.go and Metadata/Tagging in metadata.go (SubnetGroups I added last round). So all 12 capabilities have a var _ = (*Mock)(nil) guard; no drift-to-InvalidAction risk. (Looks like that one was a miss in the re-review.)

#5 — lint (dupl). Confirmed and resolved: the read-side copies broke most of the duplication; the residual describe-list wire handlers now carry //nolint:dupl. golangci-lint run is clean on all touched packages (build/vet/gofmt + full go test ./... + -race green).

Still deferred (documented follow-ups, #4): proxy RoleArn/Auth validation + targetGroup/dup-register handling, global-cluster secondary attach, event/endpoint/metadata validation nits, and the EC2-style multi-point metric backfill. These are additive fidelity improvements with no correctness/panic impact — happy to take them as a separate PR if you'd like them in.

[NitinKumar004]

Re-review @ 7c58b7a (multi-agent, in depth)

Two-agent verification of the LOW-fix commit + direct checks. go test -race ./providers/aws/rds/... and ./server/aws/rds/... are green.

✅ Correction to my earlier note

The conformance-guard point (#3) was a false alarm on my side — you were right to push back. The var _ rdsdriver.{ClusterEndpoints,ClusterFailover,GlobalClusters} guards live in a grouped var ( … ) block at aurora.go:12-14, and {Metadata,Tagging} at metadata.go:12-13; my single-line grep missed grouped entries (no var keyword on the line). All five are present and load-bearing. Withdrawn.

✅ Fixed & verified

• Read-side slice aliasing — the 6 enumerated Describe paths are complete. cloneSlice[T] is correct (nil→nil, else fresh backing array), applied in both branches (the SortedValues()/len==0 branch and the named-lookup branch) of DescribeDBProxies (Targets/Auth/subnets/SGs), DescribeGlobalClusters (Members), DescribeDBClusterEndpoints (Static/Excluded), DescribeEventSubscriptions (SourceIDs/EventCategories), DescribeOptionGroups (Options), and the parameter-group Parameters maps.
• Parameter/option-group in-use enforcement — complete and correctly wired end-to-end. Right store scanned per type (dbParameterGroupInUseBy→instances, clusterParameterGroupInUseBy→clusters, optionGroupInUseBy→instances; no cross-type confusion); reserved-name refusal uses the correct prefixes (default. for param groups, default: for option groups) with no false-match on user names; CreateDBInstance/CreateDBCluster parse and store DBParameterGroupName/OptionGroupName/DBClusterParameterGroupName (so in-use is genuinely detectable); scan+delete under m.mu. Tests exercise the real in-use path (attach → delete blocked → delete instance → delete clean), not just the default-name path.
• Wire-parsing change is byte-neutral (the describe XML structs carry no param/option-group fields, so stored-but-unrendered — existing response bytes unchanged).

🟠 MEDIUM (new — a gap the enforcement introduces)

ModifyDBInstance/ModifyDBCluster can't change the parameter/option-group attachment. ModifyInstanceInput (driver.go:90) has no DBParameterGroupName/OptionGroupName field and modifyDBInstance doesn't parse them, so a group attached at create can only be released by deleting the instance/cluster. Scenario: Create(pg1) → Modify(pg2) leaves the instance recording pg1, so DeleteDBParameterGroup(pg1) stays FailedPrecondition forever while pg2 (the actually-attached one) can be deleted. Real AWS lets you re-point then delete the old group. Now that in-use is enforced, this stranding is reachable. Fix: add the fields to ModifyInstanceInput + wire the modify handlers to update the attachment.

🟡 LOW

• The commit's "every Describe path copies its slice/map fields" claim is overstated — three still alias: DescribeInstances (VPCSecurityGroups, ReadReplicaTargets — clones Tags only), DescribeClusters (Members, VPCSecurityGroups), DescribeDBSubnetGroups (SubnetIDs — returns SortedValues() raw, no cloning). A caller mutating a returned element in place corrupts the store — the exact contract the fix set out to hold, and these are the two most central RDS Describe paths. Store→caller direction is mostly safe today (internal mutations reallocate), so LOW, but worth finishing for consistency.
• Create{,Cluster}ParameterGroup/CreateOptionGroup don't reject reserved default.*/default:* names, so a user can self-inflict an undeletable group.
• DescribeOptionGroups clones []Option but cloneSlice is shallow — Option.Settings map[string]string still aliases; not exploitable today (Settings is never populated), latent only.
• The new TestDescribeDBProxiesReturnsIndependentCopies only covers the DescribeDBProxies named branch — not the len==0 branch or the other five services, so it wouldn't catch a regression there.

Bottom line

The in-use enforcement (LOW#2) and the six targeted Describe copies (LOW#1) are correctly and completely done, race-tested, and the conformance-guard concern was my own false alarm. What remains: one MEDIUM fidelity gap the enforcement newly exposes (Modify can't re-point/release a param/option group), plus three still-aliased Describe paths that make the "every path" claim overstated (LOW). Solid progress; the Modify-attachment gap is the one worth addressing.

[thzgajendra]

Rebased onto latest development (merge, conflict resolved) and addressed the re-review — d8af059.

Merge conflict: only docs/services.md's grand-total row collided with the Bedrock PR (#298). Resolved to 1047 core (+67 optional) — Bedrock added the core ops (972→1047), this PR added the optional ones (+12→+67). providers/aws/aws.go auto-merged (both the Bedrock services and RelationalDB: rdsDiscovery wiring are present). Full suite green post-merge.

🟠 MEDIUM — Modify couldn't re-point/release a param/option group. Fixed: ModifyInstanceInput gains DBParameterGroupName/OptionGroupName (instance) and DBClusterParameterGroupName (cluster), wired through the modify handlers. So Create(pg1) → Modify(pg2) now releases pg1 (deletable) while pg2 becomes the in-use one. Tests cover instance-param, cluster-param, and option-group re-point/release.

🟡 LOW

• Finished the read-side copies so the "every Describe path copies its slice/map fields" claim actually holds: DescribeInstances (VPCSecurityGroups, ReadReplicaTargets), DescribeClusters (Members, VPCSecurityGroups), DescribeDBSubnetGroups (SubnetIDs) now return independent copies.
• Create{,Cluster}ParameterGroup/CreateOptionGroup reject the reserved default./default: prefixes (no more self-inflicted undeletable groups).
• DescribeOptionGroups deep-copies Option.Settings (the shallow cloneSlice gap you flagged).
• Broadened the copy-on-read tests to cover both the list-all and named branches (proxies + instances), not just the proxy named branch.

Also — thanks for withdrawing the conformance-guard note; confirmed all 12 var _ guards are present.

build/vet/gofmt/full go test ./... + -race on the RDS package all green; golangci-lint clean on the touched packages.

[NitinKumar004]

Final re-review @ d8af059

The last commit closes every item from the prior pass. Verified each directly; go build ./..., go test -race ./providers/aws/rds/... ./server/aws/rds/..., and golangci-lint on the touched packages are all green (0 issues).

✅ Fixed & verified

• MEDIUM — Modify can now re-point/release a parameter/option group. ModifyInstanceInput gained DBParameterGroupName/OptionGroupName, ModifyInstance applies them (rds.go:378), and ModifyCluster applies DBClusterParameterGroupName (rds.go:591); the modify handlers parse them from the wire. Re-pointing an instance/cluster to a new group frees the old one, which then deletes — the stranding I flagged is resolved. Covered by new re-point/release tests (instance param, cluster param, option group).
• LOW — the "every Describe path copies" contract now actually holds. DescribeInstances/DescribeClusters route both branches through new cloneInstance/cloneCluster helpers that copy Tags + VPCSecurityGroups + ReadReplicaTargets (instance) / Tags + Members + VPCSecurityGroups (cluster); DescribeDBSubnetGroups now clones SubnetIDs in the SortedValues() branch too. No returned Describe value aliases the store.
• LOW — reserved-name rejection on create. CreateDBParameterGroup/CreateDBClusterParameterGroup reject default. and CreateOptionGroup rejects default:, so a caller can no longer self-inflict an undeletable group.
• LOW — DescribeOptionGroups deep-copies Option.Settings (the previously-shallow cloneSlice nested-map gap).
• Merge of development (incl. the now-merged Bedrock #298) is clean — full build/tests green.

Convergence

No open findings remain. Across the review rounds every item has been fixed and verified:

• HIGH: Tags/Parameters concurrent-map-write panic; DeleteInstance replica block.
• MEDIUM: stale-replica cleanup; DeleteGlobalCluster member guard; in-place slice-filter corruption; DeleteDBSubnetGroup lock; ApplyMethod preservation; param/option-group in-use enforcement; Modify re-point/release.
• LOW: read-side slice/map copies across all Describe paths; reserved-name rejection; Option.Settings copy; subnet-group idgen ARN; deterministic tag XML.
• The conformance-guard concern was my own false alarm (guards exist in grouped var() blocks).

Blast radius stayed clean throughout (additive, optional-capability-gated, Azure/GCP untouched, existing response bytes unchanged). The documented follow-ups remain out of scope by design: proxy RoleArn/Auth validation, global-cluster secondary attach, EC2-style metric backfill, param-group family defaults, and echoing param/option-group names in the describe response.

Bottom line

Everything raised is resolved and race/lint-tested — this is a clean, converged final state. LGTM on substance.

[NitinKumar004]

Approving @ d8af059

Converged and clean. Every finding across the review rounds is fixed and verified — go build ./..., go test -race ./providers/aws/rds/... ./server/aws/rds/..., and golangci-lint on the touched packages are all green (0 issues).

• HIGH — Tags/Parameters concurrent-map-write panic (copy-on-read + replace-on-write) and DeleteInstance read-replica block: fixed, race-tested.
• MEDIUM — stale-replica cleanup, DeleteGlobalCluster member guard, in-place slice-filter corruption, DeleteDBSubnetGroup lock, ApplyMethod preservation, param/option-group in-use enforcement, and Modify re-point/release: all fixed with tests.
• LOW — read-side slice/map copies across every Describe path (cloneInstance/cloneCluster, subnet-group SubnetIDs, Option.Settings), reserved-name rejection on create, subnet-group idgen ARN, deterministic tag XML: all done.
• The conformance-guard concern was my own false alarm (guards live in grouped var() blocks).

Blast radius stayed clean throughout — additive, optional-capability-gated by type assertion, Azure/GCP relational drivers untouched, and existing action responses byte-unchanged. The out-of-scope items (proxy RoleArn/Auth validation, global-cluster secondary attach, metric backfill, param-group family defaults, echoing param/option-group names in describe) are reasonable documented follow-ups.

Nice, thorough work across the iterations. LGTM.