Add AWS Route 53 Resolver SDK-Compat Parity (72 Operations) by thzgajendra · Pull Request #328 · stackshy/cloudemu

thzgajendra · 2026-08-05T19:31:02Z

Objective / Issue

Closes #326 — add full aws-sdk-go-v2/service/route53resolver SDK-compat parity to the emulator: all 72 operations, no stubs, so real Route 53 Resolver clients work end-to-end against the in-memory driver.

What we found

The emulator had no Route 53 Resolver service at all. Route 53 Resolver speaks AWS JSON 1.1 (dispatched on X-Amz-Target: Route53Resolver.<Op>) — the same wire family already used by ECS/DynamoDB, so no new wire codec was needed. The 72 operations span seven resource groups: resolver endpoints, resolver rules, query-log configs, resolver/DNSSEC configs, DNS Firewall (the largest — domain lists, rules incl. batch, rule groups, associations, configs), Outpost resolvers, and tagging.

Blast radius: purely additive. New packages under services/route53resolver/, providers/aws/route53resolver/, server/aws/route53resolver/, wired into the existing providers/aws/aws.go + server/aws/aws.go bundles. No existing service is touched.

How we fixed it

Standard 4-layer pattern, mirroring existing AWS-JSON services:

driver (services/route53resolver/driver/) — one composed interface per resource group.
provider (providers/aws/route53resolver/) — in-memory memstore.Store per group, single-mutex read-modify-write, copy-on-write clones on read, idgen IDs/ARNs, injectable clock.
server (server/aws/route53resolver/) — JSON 1.1 handler; request/response shapes verified field-by-field against the vendored SDK serializers/deserializers (no guessed member names).

Behavioral notes (AWS-fidelity):

Per-VPC configs (resolver autodefined-reverse, DNSSEC validation, firewall fail-open) return their AWS defaults (reverse ENABLED, DNSSEC DISABLED, fail-open DISABLED) on a pure Get without persisting — only Update materializes a record, so a read never pollutes the List.
Deletes block on live dependents (InvalidRequestException): a resolver endpoint with referencing rules, a resolver rule / firewall rule group with VPC associations, and a firewall domain list referenced by rules. A rule group additionally cascades to its own rules.
Duplicate resources are rejected: a firewall rule is unique within a group by (FirewallDomainListId, Qtype), and duplicate associations return ResourceExistsException. Batch rule ops are atomic (validate-all-then-apply). Creates are idempotent on CreatorRequestId.
Tagging validates that the ARN names a live resource (ResourceNotFoundException). Update inputs use "field present" (pointer) semantics, so an explicit empty value applies while an omitted field is unchanged. List ordering is deterministic.

Alternatives not taken

Did not adopt the proposed Standardize repo file/folder structure & naming (canonical service names, consistent files, per-feature subdirectories) #325 subdirectory convention — kept the existing flat per-group file layout (like ecs) to stay consistent with the current codebase; Standardize repo file/folder structure & naming (canonical service names, consistent files, per-feature subdirectories) #325 is a separate refactor.
Did not model async status transitions (CREATING→OPERATIONAL) — statuses are returned terminal, matching how the other emulated AWS services behave.

Docs / Tests / Playground

Docs: docs/services.md — new "## 25. DNS Resolver" section (per-family operation table + accepted-but-not-simulated notes), master-table row 25, summary count row, Grand Total 1562 → 1634.
Tests: a real-SDK round-trip lifecycle test per resource group (7 total) driving the genuine aws-sdk-go-v2 client through httptest against the emulator.

Test plan

go build ./...
go vet ./...
gofmt clean
go test -race ./.../route53resolver/... — all 7 lifecycle tests pass
golangci-lint run --new-from-rev=$(git merge-base HEAD stackshy/development) ./... — 0 issues
go mod tidy clean (promotes route53resolver v1.48.3 indirect→direct)
Operation coverage: ls api_op_* (72) == registered routes (72), exact match — 0 missing / 0 extra

Risk & Rollback

Low risk — additive only; no change to existing services or shared wire code. Rollback = revert this commit / drop the three new packages and their two wiring hunks.

Conclusion

Route 53 Resolver reaches full 72/72 SDK-compat parity. Follow-up (separate PR): VPC Lattice (73 ops, REST-JSON — will need a new server/wire REST-JSON helper).

Implement the full aws-sdk-go-v2/service/route53resolver control-plane surface against the in-memory driver — all 72 SDK operations, no stubs. Covers resolver endpoints, resolver rules, query-log configs, resolver/DNSSEC configs, the DNS Firewall (domain lists, rules incl. batch, rule groups, associations, configs), Outpost resolvers, and tagging. Wire shapes are AWS JSON 1.1 (X-Amz-Target "Route53Resolver.<Op>") verified against the vendored SDK serializers/deserializers. Each resource group has a real-SDK round-trip lifecycle test. Per-VPC configs (autodefined-reverse, DNSSEC, firewall fail-open) lazily materialize with AWS defaults; firewall rules are keyed by (domain-list, qtype) and deleting a rule group cascades to its rules.

…/edge coverage Add a provider-package unit suite (90.6% statement coverage) exercising every resource group directly: happy paths, NotFound/error paths, edge cases (domain ADD dedup / REMOVE / REPLACE, batch create-update-delete, rule-group cascade delete, association-count recompute, lazy per-VPC config defaults, tag merge-by-key), and clone-on-read isolation. Extend the server round-trip suite to cover the remaining handlers (batch rule ops, policies, list variants, import, association update, tagging) and typed-exception error mapping.

NitinKumar004

Review — AWS Route 53 Resolver (72 operations)

Strong, additive PR. Verified in an isolated worktree: 72 operations, 1:1 with the vendored route53resolver@v1.48.3 SDK (no missing, no invented), all driven by real SDK round-trips, and the gate is green — go build/vet/gofmt/go test ./.../-race all pass, golangci-lint 0 issues. Architecture is idiomatic (memstore + idgen ARNs + injectable clock, canonical errors, no globals, dual-factory wiring), concurrency is sound (single mutex serializes whole methods; copy-on-write verified), and wire member names/casing match the SDK field-by-field.

Requesting changes on a set of AWS-fidelity / robustness gaps — none are crashes, and the highest-value ones are cheap. Details inline.

Should fix

Duplicate FirewallRule create silently overwrites, and batch reports more than it stores — createRuleLocked has no dup check; BatchCreateFirewallRules returns a result per entry, so the response count can exceed the store.
Delete orphans dependents — rule-group delete leaves associations dangling; same for resolver-rule / domain-list / endpoint deletes.
Empty/invalid VpcId materializes a persistent phantom config — a pure Get on a bogus id mints + persists a default that then pollutes the List.
Tagging a nonexistent resource silently succeeds — no existence check.
ListFirewallRuleTypes is effectively a stub — the handler discards the driver return and always writes [].
ListFirewallRules order is nondeterministic for equal-priority rules.

Also (Low): querylogconfig.go → query_log_config.go (STRUCTURE.md snake_case, now enforced for new services); response Content-Type is x-amz-json-1.0 not -1.1; writeErr lacks Throttled/PermissionDenied cases; IpAddressCount omitempty drops a legit 0; batch ops are non-atomic (no rollback); no CreatorRequestID idempotency; "zero means unchanged" blocks legit zero/empty updates; ImportFirewallDomains no-ops on contents; disassociate-IP has no min-2 guard; association dedupe absent.

Nice work overall — the 72-op parity and wire fidelity are genuinely solid; these are the correctness/AWS-semantics polish before merge.

NitinKumar004 · 2026-08-06T10:37:17Z

+	}
+
+	r := m.ruleFromInput(in)
+	m.fwRules.Set(fwRuleKey(in.FirewallRuleGroupID, in.FirewallDomainListID, in.Qtype), r)


createRuleLocked never checks fwRules.Has(key) before Set, so a second create with the same (FirewallRuleGroupID, FirewallDomainListID, Qtype) silently overwrites the first (real AWS returns ValidationException). Via BatchCreateFirewallRules (line 161) it's worse: it appends a result per input entry, so a batch of two dup-keyed rules reports 2 created while the store holds 1 and RuleCount = 1 — the response contradicts state. Add an existence check (AlreadyExists / ValidationException).

Fixed. createRuleLocked now checks fwRules.Has(key) and returns AlreadyExists on a duplicate (FirewallRuleGroupID, FirewallDomainListID, Qtype). BatchCreateFirewallRules is now atomic — it validates every entry (group exists, no existing or in-batch duplicate key) before applying any, so the response count can never exceed what's stored. Added TestFirewallRuleDuplicateAndAtomicBatch.

NitinKumar004 · 2026-08-06T10:37:17Z

+	return &out, nil
+}
+
+func (m *Mock) DeleteFirewallRuleGroup(_ context.Context, id string) (*driver.FirewallRuleGroup, error) {


DeleteFirewallRuleGroup cascades child rules but never touches fwAssocs, so associations dangle at a now-deleted group (real AWS blocks deleting a group that still has associations). Same orphan class in DeleteResolverRule (its associations), DeleteFirewallDomainList (rules referencing it), and DeleteResolverEndpoint (rules referencing it). Either block on live dependents or cascade them.

Fixed. Deletes now block on live dependents with FailedPrecondition → InvalidRequestException: DeleteFirewallRuleGroup (associations), DeleteResolverRule (rule associations), DeleteFirewallDomainList (referencing firewall rules), and DeleteResolverEndpoint (referencing resolver rules). Rule-group→rule cascade is retained. Added TestDeleteBlockedByDependents.

NitinKumar004 · 2026-08-06T10:37:17Z

+		ResourceID:         resourceID,
+		AutodefinedReverse: autodefinedReverseEnabled,
+	}
+	m.rslvrConfigs.Set(resourceID, c)


resolverConfigFor does no validation and keys by the raw string, so GetResolverConfig("") (or any bogus id) mints and persists a default config that then appears in ListResolverConfigs forever. Same for dnssecConfigFor / firewallConfigFor. Validate the VpcId, or don't persist on a pure read.

Fixed. GetResolverConfig/GetResolverDnssecConfig/GetFirewallConfig now return the AWS default without persisting; only Update* materializes a stored record. A pure Get on a bogus VpcId no longer pollutes the List. Updated TestResolverConfigLazyDefaultAndUpdate to assert this.

NitinKumar004 · 2026-08-06T10:37:17Z

+	defer m.mu.Unlock()
+
+	existing, _ := m.tags.Get(arn)
+	m.tags.Set(arn, mergeTags(existing, tags))


TagResource stores tags for any ARN with no check that it names a live resource (real AWS → ResourceNotFoundException), leaving an orphan entry that ListTagsForResource echoes back. UntagResource / ListTagsForResource are likewise unvalidated.

Fixed. TagResource/UntagResource/ListTagsForResource now validate the ARN names a live resource via a new arnExists scan (endpoints/rules/QLCs/domain-lists/rule-groups/associations/outposts) and return ResourceNotFoundException otherwise. Updated TestTaggingMergeAndUntag to tag a real resource + assert NotFound for a bogus ARN.

NitinKumar004 · 2026-08-06T10:37:17Z

+		return
+	}
+
+	wire.WriteJSON(w, map[string]any{"FirewallRuleTypes": []any{}})


listFirewallRuleTypes calls the driver but discards the result and always writes an empty list — the one op among the 72 that can never return data regardless of state. Even if the driver were populated, the handler drops it. Serialize the driver's return instead of the literal [].

Fixed. listFirewallRuleTypes now serializes the driver's return instead of writing a literal [] (still empty until the driver is populated, but no longer discarded).

NitinKumar004 · 2026-08-06T10:37:17Z

+		}
+	}
+
+	sort.Slice(out, func(i, j int) bool {


sort.Slice is not stable and keys only on Priority; two rules with equal Priority (allowed — no uniqueness) come back in a nondeterministic order across identical calls. Add a tiebreaker (FirewallDomainListID, then Qtype) or seed from a key-sorted slice, matching the deterministic ordering every other List uses.

Fixed. ListFirewallRules now sorts with a stable tiebreaker: priority, then FirewallDomainListID, then Qtype — deterministic across identical calls, matching the ordering the other Lists use.

NitinKumar004 · 2026-08-06T10:37:17Z

@@ -0,0 +1,209 @@
+package route53resolver


Filename isn't snake_case per docs/STRUCTURE.md §3 — rename to query_log_config.go in both providers/aws/route53resolver/ and server/aws/route53resolver/ (a feature keeps the same filename across layers). New services now go through the STRUCTURE.md loop, so worth fixing here.

Fixed. Renamed to query_log_config.go in both providers/aws/route53resolver/ and server/aws/route53resolver/ (via git mv).

Resolve the CHANGES_REQUESTED review on stackshy#328: - Firewall rules: reject duplicate (group, domain-list, qtype) with AlreadyExists; BatchCreate/Update/Delete are now atomic (validate all entries before applying) so the response can't exceed stored state. - Deletes block on live dependents (endpoint↔rules, rule↔associations, domain-list↔rules, rule-group↔associations) with InvalidRequestException. - Config reads (resolver/DNSSEC/firewall) no longer persist a phantom record on a pure Get; only Update materializes. - Tagging validates the ARN names a live resource (ResourceNotFoundException). - ListFirewallRuleTypes serializes the driver return; ListFirewallRules has a deterministic tiebreaker. - Association dedupe (rule/QLC/firewall); CreatorRequestID idempotency on all creates; pointer-based "field present" updates; writeErr Throttled/ PermissionDenied; count fields no longer omitempty; disassociate-IP keeps the AWS 2-IP minimum. - Rename querylogconfig.go -> query_log_config.go (both layers). Adds provider unit tests for every new behavior. Full gate green: build/vet/gofmt/-race and golangci-lint --new-from-rev = 0.

thzgajendra · 2026-08-06T11:16:51Z

Thanks for the thorough review — all inline comments are addressed (replied per-thread), and I took the Also (Low) list in full:

Atomic batch ops — BatchUpdate/BatchDelete validate every target exists before applying any; nothing partial-applies.
Association dedupe — AssociateResolverRule, AssociateResolverQueryLogConfig, and AssociateFirewallRuleGroup reject a duplicate (parent, target) with AlreadyExists.
CreatorRequestID idempotency — all six create ops (endpoint, rule, query-log-config, firewall domain-list, firewall rule-group, outpost) return the existing resource when replayed with the same token.
"Zero means unchanged" — Update inputs are now pointer-based (*string/*int32); nil = absent/unchanged, and an explicit empty/zero value now applies. Added TestPointerUpdateAppliesExplicitEmpty.
writeErr — added Throttled → ThrottlingException and PermissionDenied → AccessDeniedException.
omitempty on counts — dropped on IpAddressCount and AssociationCount so a legitimate 0 serializes.
Disassociate-IP — now enforces the AWS 2-IP minimum (TestDisassociateIPMinimumTwo).

Two I did not change, with reasoning:

Response Content-Type x-amz-json-1.0 vs -1.1 — that's the shared server/wire.WriteJSON helper used by every JSON service (ECS, DynamoDB, …), not something specific to this PR. It's pre-existing and systemic (SDKs decode fine; tests green). Fixing it belongs in a dedicated server/wire change so it lands consistently for all services rather than being special-cased here.
ImportFirewallDomains no-ops on file contents — by design: the emulator can't fetch the DomainFileUrl from S3. It's listed under "accepted but not simulated." Happy to revisit if you'd prefer it read from a seeded fixture.

Full gate green after the changes: go build/vet/gofmt/go test -race and golangci-lint --new-from-rev = 0. Added provider unit tests for every new behavior (dup/atomic-batch, dependent-delete blocks, dedupe, idempotency, min-2 IP, pointer-update semantics).

NitinKumar004

Re-reviewed the fix commit (b9a4dcc) — all six findings from my request-changes are resolved, verified against the code, and the gate is green (build/vet/go test/-race/golangci-lint 0, plus the new tests):

M1 — createRuleLocked now rejects a duplicate (group, domainList, Qtype) with AlreadyExists (fixing the batch count-vs-state mismatch too).
M2 — all four delete paths (rule group, resolver rule, domain list, endpoint) now guard live dependents with FailedPrecondition instead of orphaning them.
M3 — GetResolverConfig/GetResolverDnssecConfig return the AWS default on a miss without persisting, so a pure read no longer pollutes the List. Clean fix.
M4 — TagResource/UntagResource now check arnExists → ResourceNotFoundException.
M5 — the handler now serializes the driver's ListFirewallRuleTypes return (empty stays a documented boundary).
M6 — ListFirewallRules now sorts deterministically (Priority → FirewallDomainListId → Qtype).
Naming — querylogconfig.go → query_log_config.go in both layers (snake_case per STRUCTURE.md).

Nice, thorough turnaround. LGTM 👍

thzgajendra added 2 commits August 6, 2026 01:00

NitinKumar004 requested changes Aug 6, 2026

View reviewed changes

NitinKumar004 approved these changes Aug 6, 2026

View reviewed changes

thzgajendra merged commit af60ea1 into stackshy:development Aug 6, 2026
12 checks passed

Uh oh!

Conversation

thzgajendra commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Objective / Issue

What we found

How we fixed it

Alternatives not taken

Docs / Tests / Playground

Test plan

Risk & Rollback

Conclusion

Uh oh!

NitinKumar004 left a comment

Choose a reason for hiding this comment

Review — AWS Route 53 Resolver (72 operations)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thzgajendra commented Aug 6, 2026

Uh oh!

NitinKumar004 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

thzgajendra commented Aug 5, 2026 •

edited

Loading