feat(wfctl): dns-policy + OnBeforeAction hook + relocate dns packages (Phase 3a)

cmd/wfctl/dns_policy_test.go

@@ -0,0 +1,210 @@
+package main
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/GoCodeAlone/workflow/dns/policy"
+)
+
+// TestRunDNSPolicy_usage pins the no-arg behavior: print usage + return
+// an error so the wfctl dispatcher exits non-zero.
+func TestRunDNSPolicy_usage(t *testing.T) {
+	if err := runDNSPolicy([]string{}); err == nil {
+		t.Fatal("expected error for no subcommand; got nil")
+	}
+}
+
+func TestRunDNSPolicy_unknownSubcommand(t *testing.T) {
+	err := runDNSPolicy([]string{"frobnicate"})
+	if err == nil || !strings.Contains(err.Error(), "unknown subcommand") {
+		t.Fatalf("expected unknown-subcommand error; got %v", err)
+	}
+}
+
+// TestRunDNSPolicyShow_requiresZone + provider pins the basic flag-gates
+// for the show subcommand; resolveDNSPolicyReader returns an error before
+// any plugin lookup happens when --zone is missing. Catches the regression
+// where the flag-required guard is silently skipped + the empty zone
+// reaches the provider plugin as a remote lookup of "".
+func TestRunDNSPolicyShow_requiresZone(t *testing.T) {
+	err := runDNSPolicy([]string{"show", "--provider", "do-prod"})
+	if err == nil || !strings.Contains(err.Error(), "--zone") {
+		t.Fatalf("want --zone required error; got %v", err)
+	}
+}
+
+func TestRunDNSPolicyShow_requiresProvider(t *testing.T) {
+	err := runDNSPolicy([]string{"show", "--zone", "z.com"})
+	if err == nil || !strings.Contains(err.Error(), "--provider") {
+		t.Fatalf("want --provider required error; got %v", err)
+	}
+}
+
+func TestRunDNSPolicySet_requiresOwner(t *testing.T) {
+	err := runDNSPolicy([]string{"set", "--provider", "do-prod", "--zone", "z.com"})
+	if err == nil || !strings.Contains(err.Error(), "--owner") {
+		t.Fatalf("want --owner required error; got %v", err)
+	}
+}
+
+func TestRunDNSPolicyTransfer_requiresName(t *testing.T) {
+	err := runDNSPolicy([]string{"transfer-ownership", "--provider", "do-prod", "--zone", "z.com", "--new-owner", "ratchet"})
+	if err == nil || !strings.Contains(err.Error(), "--name") {
+		t.Fatalf("want --name required error; got %v", err)
+	}
+}
+
+func TestRunDNSPolicyTransfer_requiresNewOwner(t *testing.T) {
+	err := runDNSPolicy([]string{"transfer-ownership", "--provider", "do-prod", "--zone", "z.com", "--name", "www"})
+	if err == nil || !strings.Contains(err.Error(), "--new-owner") {
+		t.Fatalf("want --new-owner required error; got %v", err)
+	}
+}
+
+func TestRunDNSPolicyDrift_requiresExpectFile(t *testing.T) {
+	err := runDNSPolicy([]string{"drift", "--provider", "do-prod", "--zone", "z.com"})
+	if err == nil || !strings.Contains(err.Error(), "--expect-file") {
+		t.Fatalf("want --expect-file required error; got %v", err)
+	}
+}
+
+// ── policy-mutation helper tests ──────────────────────────────────────────────
+
+func TestMergeEntry_replacesSameOwner(t *testing.T) {
+	existing := []policy.Entry{
+		{Owner: "sre", Default: true},
+		{Owner: "multisite", Patterns: []string{"www"}},
+	}
+	updated := policy.Entry{Owner: "multisite", Patterns: []string{"www", "admin"}}
+	out := mergeEntry(existing, updated)
+	if len(out) != 2 {
+		t.Fatalf("want 2 entries; got %d: %+v", len(out), out)
+	}
+	if out[0].Owner != "sre" {
+		t.Errorf("first entry owner = %q; want sre (order preserved)", out[0].Owner)
+	}
+	if out[1].Owner != "multisite" || len(out[1].Patterns) != 2 {
+		t.Errorf("multisite entry not updated; got %+v", out[1])
+	}
+}
+
+func TestMergeEntry_appendsNewOwner(t *testing.T) {
+	existing := []policy.Entry{{Owner: "sre", Default: true}}
+	updated := policy.Entry{Owner: "ratchet", Patterns: []string{"api"}}
+	out := mergeEntry(existing, updated)
+	if len(out) != 2 || out[1].Owner != "ratchet" {
+		t.Errorf("append failed; got %+v", out)
+	}
+}
+
+func TestTransferPatternOwnership_moves(t *testing.T) {
+	entries := []policy.Entry{
+		{Owner: "multisite", Patterns: []string{"www", "admin"}},
+		{Owner: "ratchet", Patterns: []string{"api"}},
+	}
+	prev, out, err := transferPatternOwnership(entries, "www", "ratchet")
+	if err != nil {
+		t.Fatalf("transfer: %v", err)
+	}
+	if prev != "multisite" {
+		t.Errorf("prevOwner = %q; want multisite", prev)
+	}
+	// multisite should now have only "admin"; ratchet should have "api" + "www"
+	var multisitePatterns, ratchetPatterns []string
+	for _, e := range out {
+		switch e.Owner {
+		case "multisite":
+			multisitePatterns = e.Patterns
+		case "ratchet":
+			ratchetPatterns = e.Patterns
+		}
+	}
+	if len(multisitePatterns) != 1 || multisitePatterns[0] != "admin" {
+		t.Errorf("multisite patterns = %v; want [admin]", multisitePatterns)
+	}
+	if !containsStringDNSPolicy(ratchetPatterns, "www") || !containsStringDNSPolicy(ratchetPatterns, "api") {
+		t.Errorf("ratchet patterns = %v; want both [api, www]", ratchetPatterns)
+	}
+}
+
+func TestTransferPatternOwnership_dropsEmptyEntry(t *testing.T) {
+	entries := []policy.Entry{
+		{Owner: "old", Patterns: []string{"www"}}, // single-pattern entry
+		{Owner: "new", Patterns: []string{"api"}},
+	}
+	prev, out, err := transferPatternOwnership(entries, "www", "new")
+	if err != nil {
+		t.Fatalf("transfer: %v", err)
+	}
+	if prev != "old" {
+		t.Errorf("prevOwner = %q; want old", prev)
+	}
+	for _, e := range out {
+		if e.Owner == "old" {
+			t.Errorf("emptied 'old' entry should have been dropped; got %+v", e)
+		}
+	}
+}
+
+func TestTransferPatternOwnership_errorOnMissingPattern(t *testing.T) {
+	entries := []policy.Entry{{Owner: "sre", Default: true}}
+	_, _, err := transferPatternOwnership(entries, "ghost", "ratchet")
+	if err == nil {
+		t.Fatal("expected error for missing pattern; got nil")
+	}
+}
+
+func TestComparePolicyEntries_detectsMissingExtraMismatch(t *testing.T) {
+	expected := []policy.Entry{
+		{Owner: "sre", Default: true},
+		{Owner: "multisite", Patterns: []string{"www"}},
+	}
+	live := []policy.Entry{
+		{Owner: "sre", Default: false},                // default flag mismatch
+		{Owner: "ratchet", Patterns: []string{"api"}}, // extra
+		// multisite missing
+	}
+	diffs := comparePolicyEntries(expected, live)
+	if len(diffs) == 0 {
+		t.Fatal("expected diffs; got none")
+	}
+	joined := strings.Join(diffs, "|")
+	if !strings.Contains(joined, "MISSING") || !strings.Contains(joined, "multisite") {
+		t.Errorf("missing-detection failed; diffs=%v", diffs)
+	}
+	if !strings.Contains(joined, "EXTRA") || !strings.Contains(joined, "ratchet") {
+		t.Errorf("extra-detection failed; diffs=%v", diffs)
+	}
+	if !strings.Contains(joined, "MISMATCH default") {
+		t.Errorf("default-mismatch detection failed; diffs=%v", diffs)
+	}
+}
+
+func TestComparePolicyEntries_noDriftReturnsEmpty(t *testing.T) {
+	entries := []policy.Entry{
+		{Owner: "sre", Default: true},
+		{Owner: "multisite", Patterns: []string{"www"}},
+	}
+	diffs := comparePolicyEntries(entries, entries)
+	if len(diffs) != 0 {
+		t.Errorf("identical policies should produce zero diffs; got %v", diffs)
+	}
+}
+
+func TestPolicyDigest_orderIndependent(t *testing.T) {
+	a := []string{"heritage=wfinfra-v1 o=sre d=true", "heritage=wfinfra-v1 o=multisite p=www"}
+	b := []string{"heritage=wfinfra-v1 o=multisite p=www", "heritage=wfinfra-v1 o=sre d=true"}
+	if policyDigest(a) != policyDigest(b) {
+		t.Errorf("digest should be order-invariant; a=%q b=%q", policyDigest(a), policyDigest(b))
+	}
+}
+
+func containsStringDNSPolicy(s []string, target string) bool {
+	for _, v := range s {
+		if v == target {
+			return true
+		}
+	}
+	return false
+}

cmd/wfctl/infra_apply.go

@@ -460,6 +460,7 @@ func applyWithProviderAndStore(ctx context.Context, provider interfaces.IaCProvi
 	// goes through wfctlhelpers.ApplyPlanWithHooks (Replace + drift
 	// postcondition + IaCProviderFinalizer fan-out).
 	hooks := statePersistenceHooks(store, secretsProvider, provider, providerType, plan.ID, hydratedOut)
+	wireDNSGateIntoHooks(&hooks, provider)
 	result, err := applyV2ApplyPlanWithHooksFn(ctx, provider, &plan, hooks)
 	// printDriftReportIfAny surfaces input-drift to the operator on
 	// success OR partial failure — silently no-ops on empty reports.
@@ -1603,6 +1604,7 @@ func applyPrecomputedPlanWithStore(ctx context.Context, plan interfaces.IaCPlan,
 	fmt.Printf("  Plan: %d action(s) to execute.\n", len(plan.Actions))
 	// v2 is the only supported dispatch per ADR 0024 + workflow#699.
 	hooks := statePersistenceHooks(store, secretsProvider, provider, providerType, plan.ID, hydratedOut)
+	wireDNSGateIntoHooks(&hooks, provider)
 	result, err := applyV2ApplyPlanWithHooksFn(ctx, provider, &plan, hooks)
 	if result != nil {
 		printDriftReportIfAny(w, result)

cmd/wfctl/infra_apply_dns_gate.go

@@ -0,0 +1,113 @@
+package main
+
+import (
+	"context"
+	"fmt"
+	"os"
+
+	"github.com/GoCodeAlone/workflow/dns/gate"
+	"github.com/GoCodeAlone/workflow/dns/policy"
+	"github.com/GoCodeAlone/workflow/iac/wfctlhelpers"
+	"github.com/GoCodeAlone/workflow/interfaces"
+)
+
+// dnsGateHook returns a wfctlhelpers.ApplyPlanHooks-compatible
+// OnBeforeAction function that enforces DNS policy on infra.dns resources
+// during `wfctl infra apply`. Wires the workflow/dns/gate package as a
+// FATAL pre-apply gate per design §Phase 3a.
+//
+// Behavior:
+//
+//   - Non-infra.dns actions: pass-through (nil error).
+//   - WORKFLOW_DNS_OWNER env unset: log a warning and pass-through. Gate
+//     cannot be enforced without an owner identity; explicit skip is
+//     better than silently breaking applies that haven't yet adopted the
+//     policy model.
+//   - infra.dns action: build a gate.CachingGate (one TXT-read per zone
+//     for the lifetime of this apply), iterate records in
+//     action.Resource.Config["records"], and gate-check each
+//     (record_name, record_type, owner) tuple. ANY denial aborts the
+//     action (FATAL); the rest of the records under that action are not
+//     checked because OnBeforeAction itself is fatal and aborts the
+//     whole apply per the design's hard-stop semantics.
+//
+// The hook is constructed per-apply so its CachingGate's memo table is
+// scoped to one apply invocation — no cross-apply policy bleed.
+func dnsGateHook(provider interfaces.IaCProvider) func(context.Context, interfaces.PlanAction) error {
+	owner := os.Getenv("WORKFLOW_DNS_OWNER")
+	cachingGate := gate.NewCachingGate()
+	return func(ctx context.Context, action interfaces.PlanAction) error {
+		if action.Resource.Type != "infra.dns" {
+			return nil
+		}
+		if owner == "" {
+			// Surface to stderr so operators see the explicit skip — the
+			// alternative (silently allow every action) would mask config
+			// errors; the alternative (block every infra.dns action) would
+			// break legitimate applies that pre-date Phase 3a.
+			fmt.Fprintf(os.Stderr, "warning: WORKFLOW_DNS_OWNER not set; skipping DNS policy gate for %s/%s\n", action.Resource.Type, action.Resource.Name)
+			return nil
+		}
+		zone, _ := action.Resource.Config["domain"].(string)
+		if zone == "" {
+			// infra.dns ResourceSpec carries the zone in Config["domain"]
+			// (per DO/CF/NC/Hover plugin configSchema). No fallback because
+			// ResourceSpec has no ProviderID field; ProviderID lives on
+			// ResourceState — only available post-apply.
+			return fmt.Errorf("dns-gate: action %s has no Config.domain; cannot read policy", action.Resource.Name)
+		}
+		driver, err := provider.ResourceDriver("infra.dns")
+		if err != nil {
+			return fmt.Errorf("dns-gate: resolve infra.dns driver for %s: %w", zone, err)
+		}
+		reader := &gate.DriverReader{Driver: driver, Zone: zone}
+		records := extractDNSRecords(action.Resource.Config["records"])
+		for _, rec := range records {
+			recName, _ := rec["name"].(string)
+			recType, _ := rec["type"].(string)
+			if recName == "" || recType == "" {
+				continue
+			}
+			if err := cachingGate.Check(ctx, reader, zone, recName, recType, owner); err != nil {
+				return fmt.Errorf("dns-gate: zone=%s record=%s/%s owner=%s: %w", zone, recName, recType, owner, err)
+			}
+		}
+		return nil
+	}
+}
+
+// extractDNSRecords normalises both concrete-type variants of the records
+// slice ([]map[string]any and []any-of-map) returned by config parsers.
+// Returns nil-safe empty slice when records is missing or has an
+// unexpected shape — the gate-check loop then iterates zero times and the
+// action passes.
+func extractDNSRecords(records any) []map[string]any {
+	switch v := records.(type) {
+	case []map[string]any:
+		return v
+	case []any:
+		out := make([]map[string]any, 0, len(v))
+		for _, raw := range v {
+			if rec, ok := raw.(map[string]any); ok {
+				out = append(out, rec)
+			}
+		}
+		return out
+	}
+	return nil
+}
+
+// wireDNSGateIntoHooks attaches the DNS gate as OnBeforeAction on the
+// hooks struct returned by statePersistenceHooks. Caller must invoke this
+// AFTER constructing the hooks via statePersistenceHooks so the OnBefore
+// closure shares the same provider reference.
+func wireDNSGateIntoHooks(hooks *wfctlhelpers.ApplyPlanHooks, provider interfaces.IaCProvider) {
+	hooks.OnBeforeAction = dnsGateHook(provider)
+}
+
+// Compile-time guard that the policy + gate packages stay in dependency
+// reach for this file even if the body changes — catches accidental
+// import-path drift early. The `_ = policy.HeritageV1` line references a
+// stable constant in the policy package; if policy gets refactored to
+// move HeritageV1, this line breaks at compile time.
+var _ = policy.HeritageV1

cmd/wfctl/main.go

@@ -108,6 +108,7 @@ var commands = map[string]func([]string) error{
 	"modernize":       runModernize,
 	"expr-migrate":    runExprMigrate,
 	"infra":           runInfra,
+	"dns-policy":      runDNSPolicy,
 	"docs":            runDocs,
 	"editor-schemas":  runEditorSchemas,
 	"editor-bundle":   runEditorBundle,