feat(wfctl): dns-policy + OnBeforeAction hook + relocate dns packages (Phase 3a)#787
Merged
Merged
Conversation
…olicyReader PR 7's first push left this method out — spec-reviewer caught a broken build: cmd/wfctl/dns_policy.go and cmd/wfctl/infra_apply_dns_gate.go both pass *gate.DriverReader where a policy.DNSPolicyReader is expected, but the production adapter only implemented GetTXT. The package tests pass in isolation because gate_test.go's test fakes (fakeReader, countingReader) implement both halves, masking the regression. Adds UpsertTXT mirroring the design: Read current zone → rewrite records slice replacing TXT entries at the target name → Driver.Update with a synthesized ResourceSpec. Includes the recordsToMaps + upsertTXTInRecords helpers needed for the slice transform. Pins the regression with a compile-time interface assertion: var _ policy.DNSPolicyReader = (*DriverReader)(nil) Verified: - GOWORK=off go build ./... - GOWORK=off go test ./cmd/wfctl/... ./dns/... ./iac/wfctlhelpers/... all green (128s)
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Lint nilerr fired at audit.go:65: `legacyData, err := os.ReadFile(legacy);
if err != nil { return nil }`. Swallowing all errors is wrong — only the
NotExist case represents "no legacy file, nothing to migrate"; permission
denied / IO errors should propagate so operators see them rather than
silently losing the migration on a transient problem.
Wrap propagated errors so the caller knows it came from the legacy-read
path specifically.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| if err != nil { | ||
| return err | ||
| } | ||
| defer f.Close() |
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
Phase 3a of the cross-repo DNS provider cascade. Six commits across seven tasks (19-25 of the locked plan):
OnBeforeAction hook (Task 19) — adds a FATAL pre-dispatch hook to
wfctlhelpers.ApplyPlanHooks. Fires afterctx.Err()and before JIT/driver/cloud-side mutation. Non-nil return aborts the whole apply. Per cycle-3.5 I-NEW-2: hard-stop semantics, not best-effort.dns/policy relocate (Task 20) — moves
workflow-plugin-infra/internal/dnspolicy/→workflow/dns/policy/. Package renameddnspolicy→policy. Import paths swept. All tests preserved.dns/gate relocate + ResourceDriver adapter (Task 21) — moves dnsgate →
dns/gate/. NewDriverReaderstruct adaptsinterfaces.ResourceDriver.Readto satisfypolicy.DNSPolicyReader. Tolerates both[]map[string]anyand[]anyrecords-slice variants. IncludesUpsertTXTwrite half for the dns-policy mutating commands (Read current zone → modify TXT records → driver.Update).CachingGateper-apply caching preserved.dns/audit relocate + migration (Task 22) — moves dnsaudit →
dns/audit/. Path migrated:${XDG_STATE_HOME}/wfctl/plugins/workflow-plugin-infra/dns-policy-audit.jsonl→${XDG_STATE_HOME}/wfctl/plugins/wfctl/dns-audit.jsonl. One-time migration on first append (additive — legacy file retained for one release cycle).wfctl dns-policy commands (Task 23) — four subcommands as a cross-cutting orchestrator builtin per design-guidance §CLI:
show— pretty-print parsed policy (read-only; supports--raw)set— upsert a policy entry (owner / patterns / types / default)transfer-ownership— move a pattern from one owner to another, dropping the old entry if it becomes emptydrift— compare configured vs live policy; non-zero exit on driftMutating commands append a
LogPolicyEditaudit-trail entry with SHA-256 short digests of prior + new policy (order-invariant).dns-policyregistered inmain.gocommands map ANDplugin_cli_commands.goreservedCLICommands ANDwfctl.yamlworkflow surface.dns-gate OnBeforeAction wiring (Task 24) —
wireDNSGateIntoHooksattaches agate.CachingGate-backedOnBeforeActionto the hooks struct inrunInfraApply. For each infra.dns action, iteratesConfig["records"]and gate-checks each(record_name, record_type, owner)tuple. Owner comes fromWORKFLOW_DNS_OWNERenv; unset → warning + pass-through (so legacy applies still work). Per-apply scope: one CachingGate per apply, garbage-collected with the hooks struct.Runtime validation (Task 25) — verified
wfctl dns-policy --help,wfctl dns-policy show(errors with required-flag messages),wfctl dns-policy(usage banner). All flag-gate errors fire pre-plugin-load.Verification
```
GOWORK=off go test ./cmd/wfctl/... ./dns/... ./iac/wfctlhelpers/... -timeout 300s
```
All green (137s).
Runtime smoke against built binary:
```
$ /tmp/wfctl-phase3a dns-policy
Usage: wfctl dns-policy [flags]
...
$ /tmp/wfctl-phase3a dns-policy show
error: --provider required
$ /tmp/wfctl-phase3a dns-policy show --provider stub
error: --zone required
```
Rollback
Revert this PR. workflow-plugin-infra still has its admincli + internal/dnspolicy + dnsgate + dnsaudit packages intact pre-Phase-3b — the system continues to function via the old plugin cliCommand surface. No state migration to undo (audit-trail migration is additive; legacy path retained).
Part of cross-repo cascade
`docs/plans/2026-05-26-dns-provider-contract.md` (workflow-plugin-infra). PR 7 of 9. Pairs with PR 8 (workflow-plugin-infra strip) — Phase 3b.
Test plan
GOWORK=off go test ./cmd/wfctl/... ./dns/... ./iac/wfctlhelpers/...— green🤖 Generated with Claude Code