feat(dnsprovider): DNS provider v2 — registry-map + Route53 adapter#15
Merged
Conversation
…addy, Hover) Extends internal/dnsprovider/NewAdapter switch from 2 → 8 providers. v1 contract NewAdapter(provider string, creds map[string]string) already supports multi-cred; v2 ships per-provider adapters + cred-key docs. 6 PRs (one per provider). PR 6 (Hover) blocked on workflow-plugin-hover#25 pkg/hoverclient extraction. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Cycle 1 had 4 Critical cred-key mismatches against upstream libdns struct JSON tags (verified via go doc 2026-05-26): - Route53: access_key/secret_key → access_key_id/secret_access_key. AWS assume_role_arn deferred to v3 (requires aws-sdk-go-v2 STS chain). - GCP: ServiceAccountJSON is a FILE PATH not inline JSON. Project tag is gcp_project not project_id. Inline-JSON form deferred to v3. - Namecheap: api_user → user. sandbox=true → api_endpoint. client_ip made strictly required (no discovery fallback on private CI subnets). Whole-zone-replace pre-merge verification commit gating for PR 4. - GoDaddy: two-key api_key+api_secret → single api_token (sso-key format). Added 50-domain API-restriction warning. - Azure: managed-identity path surfaced (all-3-empty triggers MI). Additional fixes: - Drop dual-aliasing YAGNI (single canonical libdns-aligned key). - Per-provider docs files (no merge contention across PRs 1-5). - Per-provider RRset-semantics table added (Namecheap gating). - Rollback window assumption stated. - Engine log scrubbing flagged as open question (A8). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Critical:
- Adapter pseudo-code now shows actual signatures from
internal/dnspolicy/{reader,writer}.go (UpsertRecord returns
recordID + err, not just err).
- Namecheap whole-zone risk RESOLVED via upstream-source spike —
libdns/namecheap.SetRecords already does Get-merge-Set per
(name,type) internally. No adapter logic needed.
Important:
- Engine log scrubbing VERIFIED (workflow/engine.go:826/:848 use
module.RedactStepOutput; test coverage at engine_test.go:229/:271).
- ExpandCredsMap semantics CONFIRMED via source (os.ExpandEnv
returns empty for unset env — ambient fallback paths work).
- A4 weakened to honest framing: GoDaddy/Hover/Namecheap lack per-
record scoping; gate is only barrier for those.
- GoDaddy pre-merge live test dropped (contradicted user "unit tests
only" + workspace "Cost discipline" guidance). Operator-side smoke
optional, not gating.
- Hover sequencing explicitly accepted as deferral; user-intent
reconciliation note retained.
- Rollback deprecation cycle defined (1 minor warning + CHANGELOG).
Minor:
- Hover cred table notes "custom client; no upstream JSON tag".
- GoDaddy 50-domain deprecation followup added.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adversarial cycle 3 PASS verdict. Author applied 2 follow-up fixes from cycle-3 reviewer suggestions: - I-c3-1 (Azure MI source citation): added godoc citation from libdns/azure v0.5.0 confirming "Do not set any value to authenticate using a managed identity" for all 3 optional fields. Verified via go doc 2026-05-26. - I-c3-2 (DO precedent rationale): added "why SetRecords works" column to RRset semantics table. Explains asymmetry: v1 DO chose DELETE+APPEND because libdns/digitalocean.SetRecords requires record IDs; v2 providers' libdns wrappers are ID-free. Key asymmetry callout added below table. Design now ready for writing-plans handoff. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
One task per provider; one PR per provider. Each task: TDD test writing → libdns dep add → adapter impl → switch wire → docs file → verify build/test/vet → commit. All adapters mirror v1 digitalocean/cloudflare shape with provider-specific cred-key mappings verified against upstream JSON tags 2026-05-26. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Plan-cycle-1 FAIL findings addressed:
Critical:
- C-1: worktree path corrected (single shared worktree, branch-switch
between PRs; per-PR sub-worktree removed).
- C-2: adapter.go switch refactored to init()-based registry-map.
Each provider file self-registers via Register("<key>", factory).
supportedList() computes from sorted keys. PRs 2-6 now genuinely
parallel — zero adapter.go conflicts.
- C-3: var _ dnspolicy.Adapter = (*<prov>Adapter)(nil) compile-check
added to every adapter file (incl. v1 DO + CF as part of PR 1).
Important:
- I-1: real stub-injection round-trip tests added per provider
(TestX_StubRoundTrip_UpsertTXT) using upsertTXTViaX helper
mirroring adapter shape.
- I-2: plugin-load deviation made explicit in Out-of-scope +
Multi-Component Validation rows of guidance fit table.
- I-3: rollback notes expanded with go.mod ordering caveat +
PR 1 dependency (registry refactor non-revertable in isolation).
- I-5: Task 6 gets Step 0 "go doc hoverclient" gating verification
before any code.
- I-6: priority drop documented in docs/providers/README.md.
Design backport: registry-map dispatch documented at top of design
doc. Scope Manifest unchanged (6 PRs, 6 tasks) — no scope-lock
amendment needed.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Plan-cycle-2 FAIL findings addressed: Critical: - C-1: upsertTXTViaX helpers moved from production source into _test.go files (all 5: route53/gcp/azure/namecheap/godaddy). Production UpsertTXT now calls a.provider.SetRecords directly. Boundary lock fully provided by `var _ <prov>ProviderIface = (*libdns Provider)(nil)` compile assertion. Test helper mirrors production algorithm shape (renamed TestXUpsertTXTAlgorithmShape) — explicitly documents this. - C-3: sequencing reconciled. "Parallelism note" → "Sequencing" block: PR 1 blocking; PRs 2-6 parallelizable AMONG THEMSELVES only post-PR-1-merge; branch-stacking documented as alternative. Important: - I-2: misleading test names renamed: AmbientCredsOK → OnlyRegionRequiredAtConstruction (Route53) ADCMode → OnlyProjectRequiredAtConstruction (GCP) ManagedIdentityMode → OnlySubAndRGRequiredAtConstruction (Azure) Test bodies now clearly state "libdns lazy-resolves at first API call". - I-11: priority drop reframed as explicit wontfix (v1 precedent match; v3 typed-record dispatch followup). README updated. No scope manifest change (still 6 PRs / 6 tasks). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Plan-cycle-3 FAIL findings addressed:
Critical:
- C3-1: time import already present in plan's test file imports
(route53_test.go listed "time" — reviewer mis-flagged, fix
carried forward to other adapter test files which DO need it).
Note: all 5 v2 adapter test files now have "time" import in their
block headers per plan.
- C3-2: adapter struct field changed from *libdns<x>.Provider to
<prov>ProviderIface (5 adapters). Iface def moved from test to
production file. var _ assertion in prod. Tests construct
&<prov>Adapter{provider: stub} and call a.UpsertTXT — real
adapter method runs against stub. Algorithm-shape helpers
removed entirely. Test renamed TestXAdapter_UpsertTXT_ExercisesAdapter.
MapsFieldsExact tests updated with type-assert pattern.
- C3-3: Hover Step 0 gated on tag existence via git ls-remote.
Skeleton explicitly marked TBD-pending-godoc with ⚠ warning.
NewClient I/O check added.
Important:
- I3-1: PR 1 commit message body expanded (mentions DO+CF
re-registration, registry refactor).
- I3-2: TestNewAdapter_RegistryIsSorted guards len(parts) < 2.
- I3-3: per-task git checkout uses BASE=$(git rev-parse ...
feat/dns-provider-v2-route53 || echo master) for branch-stacking
vs master switch.
- I3-4: Hover Step 0 explicitly checks NewClient is no-I/O.
- I3-5: GoDaddy validation strengthened — SplitN+both-non-empty
rejects ":", ":foo", "foo:".
- I3-6: TestXAdapter_UpsertRecord_PriorityDroppedForA test per
adapter (Route53/GCP/Azure/Namecheap/GoDaddy) verifies non-zero
priority on A record doesn't error + payload type=A preserved.
Scope manifest unchanged.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Plan cycle 4 adversarial review: PASS (4 Minors, no Crit/Imp, converged per skill rule). M4-1 trivial fix applied: route53_test.go imports libdnsr53 for type-assert in TestNewRoute53Adapter_MapsFieldsExact. M4-2 (Hover iface deferred until godoc), M4-3 (priority comment spread), M4-4 (tag regex narrow) — cosmetic, accepted. Ready for alignment-check + scope-lock + execute. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Refactor adapter.go switch to init()-based registry-map.
- Re-register v1 digitalocean+cloudflare via init() Register calls.
- Add Route53 adapter (libdns/route53 v1.6.2) with iface-typed
.provider field for test injection.
- Add docs/providers/{README.md,route53.md} skeleton.
4 tasks
This was referenced May 26, 2026
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
1 participant
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.
Summary
DNS provider v2 — PR 1 of 6. Refactors
internal/dnsprovider/NewAdapterswitch to aninit()-based registry map and adds the Route53 (libdns/route53 v1.6.2) adapter. Subsequent PRs (#2-6) each self-register one provider (GCP/Azure/Namecheap/GoDaddy/Hover) with zero conflicts onadapter.go.Scope
Register(key, factory)+ sync.RWMutex + sortedsupportedList()for deterministic error messages.digitalocean.go+cloudflare.gogetinit() { Register(...) }+ compile-timevar _ dnspolicy.Adapterassertions. No other v1 changes.internal/dnsprovider/route53.go+route53_test.go. Iface-typed.providerfield for stub injection (cycle-3 architectural fix). Cred keys verified against upstream JSON tags 2026-05-26:region,access_key_id,secret_access_key,session_token,profile. Ambient/IAM-role fallback supported.docs/providers/README.mdskeleton +docs/providers/route53.md.docs/plans/2026-05-26-dns-provider-v2{-design,}.md+.scope-lock(4 adversarial review cycles).Out of scope
assume_role_arn(v3)aws/gcp/azure(v3)Test plan
GOWORK=off go test ./internal/dnsprovider/...— 15/15 PASSGOWORK=off go vet ./...— cleanGOWORK=off go build -o /tmp/wfinfra ./cmd/workflow-plugin-infra— built (76.7MB)🤖 Generated with Claude Code