feat(dns): capture registrar NS delegation in the portfolio (Authority + type-namespaced import IDs)#839
Merged
Merged
Conversation
…uthority Capture registrar NS delegation alongside hosted records in the canonical portfolio (both layers, per user req). 3-repo cascade: hover EnumerateAll delegation + workflow FromResourceStates -> Authority + gocodealone-dns import. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…, read model, fail-gate) I-1: capture registrar_nameservers (GetDomainDelegation, authoritative) + live_nameservers (public DNS, propagation); gap = NS-switch signal; source from registrar not the live-first Read (drift unchanged). I-2: consumer read model. I-3: import-dns fail-gate includes delegation step. C-1: enumerate emit shape + skip-with-warning. m-1: delegation-only snapshot. m-2: shared browser profile. m-3: Sanitize Authority key allow-list. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ead untouched Verified provider.Import calls d.Read (provider.go:271); fix gives delegation Import its own GetDomainDelegation+live path bypassing the live-first Read, so drift/apply semantics are unchanged. EnumerateAll just lists; Import fetches. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… ownership), T-1 (test) Import emits nameservers(=registrar) primary key for Diff-compat + supplemental registrar/live keys (no spurious drift). Delegation import step runs second + owns the merged-portfolio write. Update TestFromResourceStatesSkipsNonDNS. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ormat portfolio) + I-1/2/3 Adds Task 2 (type-namespaced import-all state IDs so dns+delegation for one domain don't overwrite on disk) + a pre-merge wfctl merge test. Task 6 gets explicit --format portfolio + exact insertion point + fail-gate. Records:[] on authority-only snapshots. 7 tasks / 3 PRs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…id, clarify o.Type advisory Resolves plan-adversarial rev2 Importants: I-NEW-1 (snap.ID from provider+domain, not type-namespaced st.ID), I-NEW-2 (o.Type advisory; --type is authoritative), domain_id dropped from delegation Outputs + Sanitize allow-list. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
FromResourceStates now groups states by (provider, domain) key so that infra.dns records and infra.dns_delegation authority NS data are merged into a single Snapshot per zone. Adds sanitizeDomainForID helper for deterministic slug IDs. Unknown state types continue to be skipped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…verwrite Importing infra.dns then infra.dns_delegation for the same domain previously produced the same state ID (e.g. "example-com") and therefore the same on-disk filename, causing the second import to silently overwrite the first. buildResourceStateFromImport now prefixes the sanitized zone name with the resource type using "/" as a separator; sanitizeStateID maps "/" → "_", yielding distinct filenames: infra.dns_example-com.json infra.dns_delegation_example-com.json ProviderID is unchanged (bare domain) so record.FromResourceStates continues to merge both types into a single portfolio snapshot. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Guard Snapshot.Authority with an allow-list {registrar_nameservers,
live_nameservers} so Sanitize strips any unknown/sensitive keys that a
future caller might put there; nil Authority is handled without panic.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds registrar NS delegation data to the canonical DNS portfolio so each domain snapshot can represent both (a) hosted DNS records (infra.dns) and (b) delegation/authority (infra.dns_delegation → Snapshot.Authority), and updates wfctl import-all state IDs to avoid cross-type overwrite collisions.
Changes:
- Merge
infra.dns+infra.dns_delegationstate into a singlerecord.Portfoliosnapshot per(provider, domain), populatingSnapshot.Authority. - Namespace import-all state IDs by resource type (
Type + "/" + sanitizedZoneName) so multi-type imports for the same domain don’t overwrite on disk. - Extend
record.Sanitizeto allow-list only{registrar_nameservers, live_nameservers}underSnapshot.Authority, with new unit tests and an ADR/design doc.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
dns/record/canonicalize.go |
Groups states by provider+domain; merges records + authority; generates stable snapshot IDs. |
dns/record/canonicalize_test.go |
Adds unit tests for delegation-only and merged-layer snapshots; updates non-DNS skip behavior expectations. |
dns/record/sanitize.go |
Allow-lists Snapshot.Authority keys during --sanitize. |
dns/record/sanitize_test.go |
Adds coverage ensuring unknown authority keys are stripped and record redaction still works. |
cmd/wfctl/infra_import_all.go |
Type-namespaces import-all IDs to prevent cross-type file collisions. |
cmd/wfctl/infra_import_all_format_test.go |
Adds tests pinning the ID collision fix and validating merged portfolio output. |
docs/plans/2026-06-02-dns-delegation-portfolio.md.scope-lock |
Locks the implementation plan scope. |
docs/plans/2026-06-02-dns-delegation-portfolio.md |
Implementation plan describing the 3-repo cascade and tasks. |
docs/plans/2026-06-02-dns-delegation-portfolio-design.md |
Design doc explaining rationale, data model, and import semantics. |
decisions/0047-dns-delegation-portfolio-authority.md |
ADR capturing the decision to represent delegation via Snapshot.Authority. |
Comment on lines
+35
to
39
| getOrCreate := func(provider, domain string) *Snapshot { | ||
| k := key{provider, domain} | ||
| if s, ok := snapByKey[k]; ok { | ||
| return s | ||
| } |
Comment on lines
55
to
58
| func TestFromResourceStatesSkipsNonDNS(t *testing.T) { | ||
| // infra.compute is a truly-unknown type and must be silently skipped. | ||
| // infra.dns_delegation is now CONSUMED (not skipped), so it produces a snapshot. | ||
| states := []interfaces.ResourceState{ |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
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
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.
Makes the canonical DNS portfolio carry BOTH layers per domain: hosted records (
infra.dns) AND registrar NS delegation (infra.dns_delegation→Snapshot.Authority). Part 1 of 3 (engine); see designdocs/plans/2026-06-02-dns-delegation-portfolio-design.md+ ADR 0047.Why
The catalog imported only hosted records — Hover parking/placeholder records for domains delegated elsewhere, with no NS/delegation. Tracking only live DNS loses records staged ahead of an NS switch. The
Snapshot.Authorityfield existed but was never populated.Changes
FromResourceStates(dns/record/canonicalize.go): groups states by(provider, domain);infra.dns→Records,infra.dns_delegation→Authority{registrar_nameservers, live_nameservers}; one merged snapshot per domain; delegation-only → authority-only snapshot (records:[]).snap.IDderived fromprovider+domain(no slash).cmd/wfctl/infra_import_all.go):buildResourceStateFromImportprefixes the state ID with the resource type so importinginfra.dnstheninfra.dns_delegationfor one domain no longer overwrites the same.jsonfile (the merge depends on both surviving).ProviderID(the bare domain) unchanged → portfolio domain unaffected. Single-type imports functionally unaffected.Sanitizeallow-listsAuthoritykeys ({registrar_nameservers, live_nameservers}) so the free-form map can't leak non-NS data under--sanitize.Additive schema use (
Authorityisomitempty);DelegationDriver.Read/drift untouched. Unit + merge-integration tests;go test ./dns/record ./cmd/wfctlgreen.Cascade: PR2 = workflow-plugin-hover delegation enumerate/import; PR3 = gocodealone-dns catalog import. Needs a wfctl minor release before PR3.
🤖 Generated with Claude Code