From 5aea811140c054a187a8915777c120a3594105f9 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 20:25:11 -0400 Subject: [PATCH 01/20] docs: design infra.dns_delegation for hover plugin New resource type adds registrar-level nameserver management alongside the existing infra.dns record management. Endpoint captured from the Hover web UI: PUT /api/control_panel/domains/ domain- with X-CSRF-Token header + {field, value} JSON. Decisions locked via brainstorming: - Resource type: infra.dns_delegation (matches AWS/GCP naming room for equivalents; semantically distinct from infra.dns). - Delete: reset to Hover defaults [ns1.hover.com, ns2.hover.com]. - CSRF: fetch fresh per PUT. - Field test: gocodealone.tech end-to-end via workflow_dispatch GHA inside gocodealone-multisite. Assumptions, rollback, and top-3 doubts captured in the doc for adversarial-review attack. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../2026-05-20-hover-dns-delegation-design.md | 213 ++++++++++++++++++ 1 file changed, 213 insertions(+) create mode 100644 docs/plans/2026-05-20-hover-dns-delegation-design.md diff --git a/docs/plans/2026-05-20-hover-dns-delegation-design.md b/docs/plans/2026-05-20-hover-dns-delegation-design.md new file mode 100644 index 0000000..3bdb903 --- /dev/null +++ b/docs/plans/2026-05-20-hover-dns-delegation-design.md @@ -0,0 +1,213 @@ +# Hover DNS Delegation — `infra.dns_delegation` Design + +**Date:** 2026-05-20 +**Status:** Draft → adversarial review → writing-plans +**Scope:** workflow-plugin-hover v0.2.0 — new resource type for registrar-level nameserver delegation +**Field-test target:** gocodealone.tech → ns1/2/3.digitalocean.com + +## Goal + +Let wfctl set a domain's nameservers at the registrar layer via the Hover plugin. The existing `infra.dns` resource manages DNS *records* within Hover's nameservers. This new resource type manages the upstream nameserver delegation itself — i.e., "who is authoritative DNS for this domain", which is a different API surface and lifecycle. + +Concrete first use: point gocodealone.tech at DigitalOcean's nameservers via `wfctl apply` triggered by a manual `workflow_dispatch` GitHub Actions job inside the gocodealone-multisite repo. + +## Endpoint + +Captured from the Hover web UI (browser DevTools): + +``` +PUT https://www.hover.com/api/control_panel/domains/domain- +Content-Type: application/json +X-CSRF-Token: +Cookie: hoverauth=...; hover_session=... + +{"field":"nameservers","value":["ns1.digitalocean.com","ns3.digitalocean.com","ns2.digitalocean.com"]} +``` + +ID convention: `domain-` (different from the `dom` numeric IDs the DNS-record API uses). + +The generic `{"field": ..., "value": ...}` body suggests Hover uses this endpoint for several domain-level fields (whois_privacy, locked, auto_renew, nameservers). We only model `nameservers` here — YAGNI on the rest. + +The CSRF token is a Rails `authenticity_token` exposed as `` in any control_panel HTML page. Fetch fresh per PUT (user decision). + +## Architecture + +Three artifacts: + +1. **workflow-plugin-hover v0.2.0** — adds `infra.dns_delegation` driver alongside the existing `infra.dns` driver. Shared `*hover.Client`. Additive change; v0.1.0 consumers continue to work. +2. **workflow-registry manifest bump** — capability list + v0.2.0 download SHAs (post-release). +3. **gocodealone-multisite/config/dns.wfctl.yaml + .github/workflows/dns-delegation.yml** — field-test artifact; manual `workflow_dispatch`. + +## Components + +### `internal/hover/client.go` extensions + +- `Domain.Nameservers []string \`json:"nameservers,omitempty"\`` — Hover already returns this field in `GET /api/domains//dns` per the chickenandpork/hoverdnsapi fixtures; we just need to parse it. +- `fetchControlPanelCSRF(ctx context.Context, domainName string) (string, error)` — `GET /control_panel/domain/`, parse `` with a new regex `csrfMetaRe`. Returns typed error on non-2xx or missing token (mirrors the existing `probeTOTPPage` error semantics). +- `SetNameservers(ctx context.Context, domainName string, ns []string) error` — eager `ensureLogin`, then `fetchControlPanelCSRF`, then `PUT /api/control_panel/domains/domain-` with the JSON body + `X-CSRF-Token` header. Surface PUT non-2xx response body as the error. + +### `internal/drivers/delegation.go` (new, ~120 LoC) + +- `type DelegationDriver struct { client HoverDelegationClient }` where the test-injectable interface is: + ```go + type HoverDelegationClient interface { + GetDomain(ctx context.Context, domain string) (*hover.Domain, error) + SetNameservers(ctx context.Context, domain string, ns []string) error + } + ``` +- `infra.dns_delegation` resource type. +- Config schema: + ```yaml + config: + domain: string # required; apex zone (e.g. example.com) + nameservers: [string] # required; ≥2 distinct hostnames + ``` +- Lifecycle: + - **Create / Update** — validate (domain non-empty; nameservers ≥2 distinct), call `client.SetNameservers`, return `dnsDelegationOutputFromDesired(domain, nameservers)` (build outputs from the desired set without read-after-write — same pattern as the namecheap round-4 fix). + - **Read** — `client.GetDomain(ctx, domain)` → return `{domain, nameservers}` as ResourceOutput. + - **Delete** — `client.SetNameservers(ctx, domain, []string{"ns1.hover.com","ns2.hover.com"})` — resets to Hover defaults (user choice; comment cites this design doc). + - **Diff** — multiset comparison of `current.Outputs["nameservers"]` vs desired (order-independent — Hover's PUT accepts any order). Domain rename (desired vs `current.ProviderID`) → `NeedsReplace=true` with `ForceNew` change. + - **HealthCheck** — `GetDomain` success/failure. + - **Scale** — no-op error (DNS delegation has no replicas). + - **SensitiveKeys** — nil; nameservers are public. + - **ProviderIDFormat** — `IDFormatDomainName`. +- Ctx.Err() check before every API call (mirrors PR #4 round-6 namecheap hardening). + +### `internal/provider.go` + +- Add `"infra.dns_delegation": drivers.NewDelegationDriver(c)` to the drivers map. +- Append `IaCCapabilityDeclaration{ResourceType: "infra.dns_delegation", Tier: 1, Operations: []string{"create","read","update","delete"}}` to `Capabilities()`. + +### `internal/iacserver.go` + +- No structural change. The provider's `Capabilities()` + `ResourceDriver()` dispatch already drive the gRPC surface. + +### `plugin.json` + +- Add `"infra.dns_delegation"` to `iacProvider.resourceTypes`. + +### `workflow-registry/plugins/hover/manifest.json` + +- Add `"infra.dns_delegation"` to `capabilities.iacProvider.resourceTypes`. +- Bump `version` to `0.2.0`. +- Repopulate `downloads` with v0.2.0 SHA256s post-release (separate PR, gated on tag publish). + +### `gocodealone-multisite/.github/workflows/dns-delegation.yml` + +- `on: workflow_dispatch` +- Steps: checkout → setup-go → install wfctl (from `GoCodeAlone/setup-wfctl@v1`) → `wfctl plugin install workflow-plugin-hover@v0.2.0` → `wfctl apply config/dns.wfctl.yaml` +- Secrets sourced from repo or env: `HOVER_USERNAME`, `HOVER_PASSWORD`, `HOVER_TOTP_SECRET` (the existing required_secrets manifested by the plugin). + +### `gocodealone-multisite/config/dns.wfctl.yaml` + +```yaml +modules: + - name: hover + type: iac.provider.hover + config: + username: ${HOVER_USERNAME} + password: ${HOVER_PASSWORD} + totp_secret: ${HOVER_TOTP_SECRET} + +resources: + - name: gocodealone-tech-delegation + type: infra.dns_delegation + config: + provider: hover + domain: gocodealone.tech + nameservers: + - ns1.digitalocean.com + - ns2.digitalocean.com + - ns3.digitalocean.com +``` + +## Data flow + +``` +Operator clicks "Run workflow" (workflow_dispatch on gocodealone-multisite) + ↓ +GHA: wfctl plugin install workflow-plugin-hover@v0.2.0 +GHA: wfctl apply config/dns.wfctl.yaml + ↓ +wfctl loads hover plugin (gRPC), Initialize() → eager Login (session cookies + optional TOTP) + ↓ +wfctl Plan → DelegationDriver.Diff(desired, current) + ├── current == nil → NeedsUpdate=true (first apply) + ├── desired.domain != current.PID → NeedsReplace=true + ├── multiset(desired.ns) != multiset(current.Outputs.ns) → NeedsUpdate=true + └── else → NeedsUpdate=false + ↓ +wfctl Apply → DelegationDriver.Create or Update + ├── ctx.Err check + ├── client.fetchControlPanelCSRF(ctx, "gocodealone.tech") → token + ├── ctx.Err check + ├── PUT /api/control_panel/domains/domain-gocodealone.tech + │ Body: {"field":"nameservers","value":["ns1.digitalocean.com",...]} + │ Header: X-CSRF-Token: + └── 200 → return dnsDelegationOutputFromDesired + ↓ +wfctl persists state. Subsequent Plans no-op until config changes. +``` + +## Error handling + +| Failure | Behavior | +|---|---| +| CSRF-fetch non-2xx | Typed error "hover: fetch control_panel CSRF: HTTP %d"; PUT not attempted. | +| CSRF token missing in meta | Typed error "hover: CSRF token not found at /control_panel/domain/%s (control_panel UI changed?)". | +| Login expired between CSRF fetch and PUT | `ensureLogin` re-fetches inside SetNameservers; covered. | +| PUT non-2xx | Surface Hover's response body as the error message ("hover SetNameservers %q: HTTP %d: %s"). | +| Cloudflare challenge on PUT | Manifests as non-2xx → same path. Operator must allowlist the runner IP. README documents. | +| Domain rename via Update | Typed error "domain change requires resource replace, not update" (mirrors namecheap pattern). | +| Delete: PUT to Hover-default NS fails | Propagate; IaC state retained. | + +## Testing + +| Layer | Coverage | +|---|---| +| `internal/hover/client_test.go` | httptest stub: `/control_panel/domain/` returns HTML with meta csrf-token → `SetNameservers` PUT asserts URL + body shape + `X-CSRF-Token` header. Failure paths: non-2xx CSRF fetch, missing meta tag, non-2xx PUT. Existing MFA-on / MFA-off login paths re-verified. | +| `internal/drivers/delegation_test.go` | Fake client: Create/Update/Read/Delete/Diff happy paths. Diff multiset order-independence (`[a,b,c]` vs `[c,b,a]` → NeedsUpdate=false). Domain rename → NeedsReplace=true + ForceNew. ctx-cancellation propagates from every method. Delete writes `[ns1.hover.com,ns2.hover.com]`. Config validation: missing domain, missing nameservers, fewer than 2 nameservers, duplicate nameservers. | +| `internal/iacserver_test.go` | Capabilities lists both `infra.dns` and `infra.dns_delegation`. gRPC bufconn smoke for the new type. | +| Field test | `wfctl apply` in GHA against gocodealone.tech. Verify Hover UI shows the three DigitalOcean nameservers post-apply. | + +## Assumptions + +| # | Claim | Risk if false | Evidence | +|---|---|---|---| +| A1 | Captured PUT endpoint shape is stable across Hover releases. | Plugin breaks; need re-capture. | OSS clients show 5+ years of stability on the related `/api/dns` endpoints; same control panel codebase. | +| A2 | CSRF tokens from `/control_panel/domain/` are valid for `/api/control_panel/domains/` PUTs (Rails per-session, not per-action). | PUT rejects with 422; retry-with-fresh-fetch loop required. | Rails default behavior; the captured curl used a token fetched from an arbitrary control_panel page. | +| A3 | No Cloudflare/CAPTCHA gate on the PUT path from a fresh GH-runner IP. | Field test fails; mitigation = self-hosted runner OR document a stable egress IP. | README already documents this as a CAPTCHA caveat on the existing DNS flow; same risk model. | +| A4 | Hover idempotently accepts "set to same nameservers" (no-op success). | Only matters on Create-after-state-loss; Diff prevents the re-PUT in normal flow. | Inferred from typical PUT-idempotency conventions; not verified. | +| A5 | Hover's default nameservers are `ns1.hover.com` + `ns2.hover.com`. | Delete writes wrong values; user manually fixes once. | chickenandpork/hoverdnsapi test fixtures show this pair on multiple domains. | +| A6 | `GET /api/domains//dns` returns `nameservers: [...]` in the response. | Read returns empty; Diff reports perpetual drift. Fallback: `GET /api/domains/` (different endpoint per jmhodges/hover). | chickenandpork fixtures include the field; not verified against accounts with externally-set nameservers. | +| A7 | GH-hosted runners can reach hover.com. | Workflow fails at first request; mitigation = self-hosted runner. | No known IP-based block. | + +## Rollback + +The change touches runtime (plugin loading + a live registrar PUT), so rollback is in scope. + +- **Plugin (workflow-plugin-hover v0.2.0)**: `wfctl plugin install workflow-plugin-hover@v0.1.0` reverts the install on any consumer. +- **Registry**: revert the manifest PR; `wfctl plugin search` falls back to v0.1.0. +- **gocodealone-multisite**: delete the `infra.dns_delegation` resource block; future `wfctl apply` no-ops. To reset gocodealone.tech back to Hover's nameservers: re-run the workflow with `nameservers: [ns1.hover.com, ns2.hover.com]` declared, OR set them manually in Hover's UI. +- **DNS itself**: the PUT is reversible — set whatever NS list you want via another apply. DNS propagation can take up to 24h, but the registrar state flips immediately. + +## Top 3 surfaced doubts (from self-challenge) + +1. **CSRF-per-PUT cost**: per-PUT control_panel page fetch doubles the request count. If Hover throttles these GETs we may need to fall back to cached-with-1h-TTL CSRF (matching the login session). User-chosen "fetch fresh" is the safer default; field test will validate. +2. **Cloudflare on GHA**: shared-IP runners can trip bot challenges. Fallback: self-hosted runner with a stable egress IP that's been allowlisted via a manual login from that IP. Already a caveat in the README for the DNS-records flow. +3. **Read endpoint coverage of arbitrary external NS**: fixtures show Hover-defaults + Tucows alt only. If `GET /api/domains//dns` doesn't echo back `ns1.digitalocean.com` etc. post-set, Diff false-positives on every Plan. Mitigation: add a fallback to `GET /api/domains/` if observed in field test. + +## Sequencing + +1. Plugin PR opens → CI green → Copilot review rounds → merge → tag v0.2.0 → goreleaser publishes assets. +2. Registry manifest update PR with v0.2.0 SHAs → CI green → merge. +3. gocodealone-multisite PR adds `config/dns.wfctl.yaml` + `.github/workflows/dns-delegation.yml` → merge. +4. Operator (Jon) runs the workflow_dispatch → DNS delegation flips to DO. +5. Validate via Hover UI + `dig +short NS gocodealone.tech` post-propagation. + +## References + +- Captured curl from the Hover web UI session (2026-05-20). +- workflow-plugin-namecheap PR #4 round-6 hardening pattern (ctx propagation across all driver methods). +- chickenandpork/hoverdnsapi `Domain.NameServers` field + fixtures. +- jmhodges/hover `Domain.Nameservers` field (alternate read endpoint). From 1e129a3fb0d7f60fc3e193fcc51a0f65a568c70b Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 20:30:01 -0400 Subject: [PATCH 02/20] =?UTF-8?q?docs:=20revise=20dns=5Fdelegation=20desig?= =?UTF-8?q?n=20=E2=80=94=20adversarial=20review=20round=201?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses 2 Critical + 3 Important + 3 Minor findings from adversarial review: CRITICAL: - Read endpoint uncertainty (A6): switched primary Read from /api/domains//dns to /api/control_panel/domains/domain- (same API family as the PUT — far more likely to surface the nameservers field reliably). First implementation task = curl-verify the response shape. - Outputs["nameservers"] encoding: explicitly spec'd as []any (not []string) per the structpb boundary invariant. Helper + round-trip JSON test. IMPORTANT: - CSRF source ambiguity: documented the two distinct regexes (form token on /signin; meta tag on /control_panel/). Cited that both shapes coexist in the captured browser session. - ensureLogin + fetchControlPanelCSRF concurrency: new *Locked helpers; SetNameservers holds c.mu across both the CSRF GET and the PUT. Eliminates the race window. - Delete hardcoded defaults: primary path stashes previous_nameservers at Create; Delete restores from state. Hardcoded fallback only for state-less resources. MINOR: - Sequencing: deferred registry manifest + multisite YAML to a separate session post-goreleaser. - Validation: relaxed to >=1 nameserver. - Field test: success = Hover UI shows new NS; dig is separate. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../2026-05-20-hover-dns-delegation-design.md | 198 +++++++++++------- 1 file changed, 117 insertions(+), 81 deletions(-) diff --git a/docs/plans/2026-05-20-hover-dns-delegation-design.md b/docs/plans/2026-05-20-hover-dns-delegation-design.md index 3bdb903..8618f79 100644 --- a/docs/plans/2026-05-20-hover-dns-delegation-design.md +++ b/docs/plans/2026-05-20-hover-dns-delegation-design.md @@ -1,7 +1,7 @@ # Hover DNS Delegation — `infra.dns_delegation` Design **Date:** 2026-05-20 -**Status:** Draft → adversarial review → writing-plans +**Status:** Revised after adversarial review round 1 **Scope:** workflow-plugin-hover v0.2.0 — new resource type for registrar-level nameserver delegation **Field-test target:** gocodealone.tech → ns1/2/3.digitalocean.com @@ -13,7 +13,7 @@ Concrete first use: point gocodealone.tech at DigitalOcean's nameservers via `wf ## Endpoint -Captured from the Hover web UI (browser DevTools): +Captured from the Hover web UI (browser DevTools, 2026-05-20): ``` PUT https://www.hover.com/api/control_panel/domains/domain- @@ -28,30 +28,61 @@ ID convention: `domain-` (different from the `dom` numeric IDs t The generic `{"field": ..., "value": ...}` body suggests Hover uses this endpoint for several domain-level fields (whois_privacy, locked, auto_renew, nameservers). We only model `nameservers` here — YAGNI on the rest. -The CSRF token is a Rails `authenticity_token` exposed as `` in any control_panel HTML page. Fetch fresh per PUT (user decision). +The CSRF token is a Rails `authenticity_token`. The existing login flow extracts a *form* token via `` from `/signin`. The control-panel pages serve the *meta* form via `` (verified from the captured browser session — same session emitted both shapes simultaneously). Both ultimately resolve to the same Rails CSRF authenticity-token secret per session; the meta-tag form is what JavaScript-driven fetches in Hover's SPA layer consume, and that's the form the API gateway validates against. + +We fetch fresh per PUT (user decision). ## Architecture -Three artifacts: +Three artifacts, **shipped as three async-gated PRs across two sessions**: + +| # | Artifact | Session | Gate | +|---|---|---|---| +| 1 | workflow-plugin-hover v0.2.0 (this PR) | Now | Copilot + CI green → merge → tag v0.2.0 → goreleaser publishes assets | +| 2 | workflow-registry manifest bump | **Separate later session** | Post-goreleaser; cannot be PRed until v0.2.0 asset SHAs exist | +| 3 | gocodealone-multisite field-test YAML + workflow | Same separate session as #2 OR after #2 merges | Gated on registry manifest carrying v0.2.0 so `wfctl plugin install` resolves | -1. **workflow-plugin-hover v0.2.0** — adds `infra.dns_delegation` driver alongside the existing `infra.dns` driver. Shared `*hover.Client`. Additive change; v0.1.0 consumers continue to work. -2. **workflow-registry manifest bump** — capability list + v0.2.0 download SHAs (post-release). -3. **gocodealone-multisite/config/dns.wfctl.yaml + .github/workflows/dns-delegation.yml** — field-test artifact; manual `workflow_dispatch`. +The plugin PR (#1) is the entire scope of the current autonomous pass. #2 and #3 are explicitly out-of-scope for the current session — a follow-up note in the merged PR description will trigger them in a fresh session. ## Components ### `internal/hover/client.go` extensions -- `Domain.Nameservers []string \`json:"nameservers,omitempty"\`` — Hover already returns this field in `GET /api/domains//dns` per the chickenandpork/hoverdnsapi fixtures; we just need to parse it. -- `fetchControlPanelCSRF(ctx context.Context, domainName string) (string, error)` — `GET /control_panel/domain/`, parse `` with a new regex `csrfMetaRe`. Returns typed error on non-2xx or missing token (mirrors the existing `probeTOTPPage` error semantics). -- `SetNameservers(ctx context.Context, domainName string, ns []string) error` — eager `ensureLogin`, then `fetchControlPanelCSRF`, then `PUT /api/control_panel/domains/domain-` with the JSON body + `X-CSRF-Token` header. Surface PUT non-2xx response body as the error. +- `Domain.Nameservers []string \`json:"nameservers,omitempty"\`` — added to the existing struct. Source = the new control-panel detail endpoint (see Read path below), NOT the DNS-records endpoint. Backwards-compatible additive field; existing `GetDomain` callers unaffected (returns empty when source endpoint doesn't supply it). +- `csrfMetaRe = regexp.MustCompile(\``, parses meta token via `csrfMetaRe`. Non-2xx → typed error "hover: fetch control_panel CSRF: HTTP %d"; missing meta → typed error "hover: CSRF meta tag not found at /control_panel/domain/%s (control_panel UI changed?)". Caller must hold `c.mu` (see Concurrency section). +- `GetDomainDelegation(ctx context.Context, domainName string) (*Domain, error)` — **new method** (per adversarial review recommendation #2). `GET /api/control_panel/domains/domain-`. Same API family as the PUT; far more likely to surface the `nameservers` field reliably. Returns a `*Domain` with `Nameservers` populated. This is the **primary Read source** for `DelegationDriver`. +- `SetNameservers(ctx context.Context, domainName string, ns []string) error` — see Concurrency section below for lock discipline. Eager `ensureLogin`, `fetchControlPanelCSRF`, `PUT /api/control_panel/domains/domain-` with JSON body + `X-CSRF-Token` header. PUT non-2xx → surface Hover's body as error. + +### Concurrency: `SetNameservers` + `ensureLogin` interaction + +The existing `ensureLogin` holds `c.mu` for its entire duration. `SetNameservers` does: + +```go +func (c *Client) SetNameservers(ctx context.Context, domainName string, ns []string) error { + if err := c.ensureLogin(ctx); err != nil { // acquires + releases c.mu + return err + } + c.mu.Lock() + defer c.mu.Unlock() + // Inside the lock, both the CSRF fetch and the PUT execute against + // the same session-cookie state. No re-auth can fire between them. + csrf, err := c.fetchControlPanelCSRFLocked(ctx, domainName) + if err != nil { + return err + } + return c.putNameserversLocked(ctx, domainName, ns, csrf) +} +``` -### `internal/drivers/delegation.go` (new, ~120 LoC) +Two `Locked` helpers added (`fetchControlPanelCSRFLocked`, `putNameserversLocked`) so the public `SetNameservers` can hold the lock across both HTTP round trips, eliminating the race window the reviewer identified. + +### `internal/drivers/delegation.go` (new, ~150 LoC) - `type DelegationDriver struct { client HoverDelegationClient }` where the test-injectable interface is: ```go type HoverDelegationClient interface { - GetDomain(ctx context.Context, domain string) (*hover.Domain, error) + GetDomainDelegation(ctx context.Context, domain string) (*hover.Domain, error) SetNameservers(ctx context.Context, domain string, ns []string) error } ``` @@ -60,18 +91,21 @@ Three artifacts: ```yaml config: domain: string # required; apex zone (e.g. example.com) - nameservers: [string] # required; ≥2 distinct hostnames + nameservers: [string] # required; ≥1 hostname, distinct entries ``` + Validation: `≥1 nameserver` (Hover may accept single-NS setups; not the plugin's place to over-validate). Duplicates rejected as a config-author bug. Each nameserver string non-empty. +- **Outputs encoding (structpb-safe)**: `Outputs["nameservers"]` is stored as `[]any` of `string`, NOT `[]string`. Per the structpb boundary invariant (see `iacserver.go` package doc; `feedback_workflow_plugin_structpb_boundary` workspace memory). Helper `nameserversToAny(ns []string) []any` converts. Round-trip test through `json.Marshal` + `json.Unmarshal` into `map[string]any` confirms the type survives. - Lifecycle: - - **Create / Update** — validate (domain non-empty; nameservers ≥2 distinct), call `client.SetNameservers`, return `dnsDelegationOutputFromDesired(domain, nameservers)` (build outputs from the desired set without read-after-write — same pattern as the namecheap round-4 fix). - - **Read** — `client.GetDomain(ctx, domain)` → return `{domain, nameservers}` as ResourceOutput. - - **Delete** — `client.SetNameservers(ctx, domain, []string{"ns1.hover.com","ns2.hover.com"})` — resets to Hover defaults (user choice; comment cites this design doc). - - **Diff** — multiset comparison of `current.Outputs["nameservers"]` vs desired (order-independent — Hover's PUT accepts any order). Domain rename (desired vs `current.ProviderID`) → `NeedsReplace=true` with `ForceNew` change. - - **HealthCheck** — `GetDomain` success/failure. + - **Create** — validate, capture pre-change nameservers via `client.GetDomainDelegation` (best-effort; non-fatal if it fails) and stash them in `Outputs["previous_nameservers"]`, then `client.SetNameservers`. Return outputs `{domain, nameservers, previous_nameservers}`. + - **Update** — validate, call `client.SetNameservers`. Returns outputs from desired (no read-after-write, per namecheap round-4 pattern). Preserves any existing `previous_nameservers` from prior state if available. + - **Read** — `client.GetDomainDelegation(ctx, domain)` → return `{domain, nameservers}` as `ResourceOutput`. Does NOT populate `previous_nameservers` from upstream (it's a state-only field captured at Create). + - **Delete** — read `previous_nameservers` from the resource's input snapshot if present; otherwise fall back to `[ns1.hover.com, ns2.hover.com]` with a logged warning. PUT via `client.SetNameservers`. This is the rollback path: the resource owns the registrar state it took over and restores what was there before, with a documented hardcoded fallback for state-less or pre-v0.2.0 resources. + - **Diff** — multiset comparison of `current.Outputs["nameservers"]` (`[]any` decoded back to `[]string`) vs desired. Order-independent. Domain rename (desired vs `current.ProviderID`) → `NeedsReplace=true` with a `ForceNew` change. + - **HealthCheck** — `GetDomainDelegation` success/failure. - **Scale** — no-op error (DNS delegation has no replicas). - **SensitiveKeys** — nil; nameservers are public. - **ProviderIDFormat** — `IDFormatDomainName`. -- Ctx.Err() check before every API call (mirrors PR #4 round-6 namecheap hardening). +- Ctx.Err() check before every API-touching method (mirrors PR #4 round-6 namecheap hardening). ### `internal/provider.go` @@ -86,40 +120,12 @@ Three artifacts: - Add `"infra.dns_delegation"` to `iacProvider.resourceTypes`. -### `workflow-registry/plugins/hover/manifest.json` - -- Add `"infra.dns_delegation"` to `capabilities.iacProvider.resourceTypes`. -- Bump `version` to `0.2.0`. -- Repopulate `downloads` with v0.2.0 SHA256s post-release (separate PR, gated on tag publish). - -### `gocodealone-multisite/.github/workflows/dns-delegation.yml` - -- `on: workflow_dispatch` -- Steps: checkout → setup-go → install wfctl (from `GoCodeAlone/setup-wfctl@v1`) → `wfctl plugin install workflow-plugin-hover@v0.2.0` → `wfctl apply config/dns.wfctl.yaml` -- Secrets sourced from repo or env: `HOVER_USERNAME`, `HOVER_PASSWORD`, `HOVER_TOTP_SECRET` (the existing required_secrets manifested by the plugin). - -### `gocodealone-multisite/config/dns.wfctl.yaml` - -```yaml -modules: - - name: hover - type: iac.provider.hover - config: - username: ${HOVER_USERNAME} - password: ${HOVER_PASSWORD} - totp_secret: ${HOVER_TOTP_SECRET} - -resources: - - name: gocodealone-tech-delegation - type: infra.dns_delegation - config: - provider: hover - domain: gocodealone.tech - nameservers: - - ns1.digitalocean.com - - ns2.digitalocean.com - - ns3.digitalocean.com -``` +### Workflow-registry manifest + gocodealone-multisite field-test artifact + +Both deferred to a separate session per the Architecture table above. The current PR's description will explicitly call out: +- Tag v0.2.0 expected post-merge. +- Follow-up registry manifest PR required (cannot be authored until goreleaser publishes SHAs). +- Follow-up gocodealone-multisite PR adds `config/dns.wfctl.yaml` + `.github/workflows/dns-delegation.yml`. ## Data flow @@ -139,12 +145,15 @@ wfctl Plan → DelegationDriver.Diff(desired, current) ↓ wfctl Apply → DelegationDriver.Create or Update ├── ctx.Err check - ├── client.fetchControlPanelCSRF(ctx, "gocodealone.tech") → token - ├── ctx.Err check - ├── PUT /api/control_panel/domains/domain-gocodealone.tech - │ Body: {"field":"nameservers","value":["ns1.digitalocean.com",...]} - │ Header: X-CSRF-Token: - └── 200 → return dnsDelegationOutputFromDesired + ├── Create only: GetDomainDelegation → stash current NS as previous_nameservers (best-effort) + ├── client.SetNameservers (under c.mu, both CSRF GET + PUT inside the lock) + │ ├── ensureLogin + │ ├── lock c.mu + │ ├── fetchControlPanelCSRFLocked → token + │ └── PUT /api/control_panel/domains/domain- + │ Body: {"field":"nameservers","value":[...]} + │ Header: X-CSRF-Token: + └── 200 → return dnsDelegationOutputFromDesired(domain, ns, previous_ns) ↓ wfctl persists state. Subsequent Plans no-op until config changes. ``` @@ -154,33 +163,35 @@ wfctl persists state. Subsequent Plans no-op until config changes. | Failure | Behavior | |---|---| | CSRF-fetch non-2xx | Typed error "hover: fetch control_panel CSRF: HTTP %d"; PUT not attempted. | -| CSRF token missing in meta | Typed error "hover: CSRF token not found at /control_panel/domain/%s (control_panel UI changed?)". | -| Login expired between CSRF fetch and PUT | `ensureLogin` re-fetches inside SetNameservers; covered. | -| PUT non-2xx | Surface Hover's response body as the error message ("hover SetNameservers %q: HTTP %d: %s"). | +| CSRF meta tag missing | Typed error "hover: CSRF meta tag not found at /control_panel/domain/%s (control_panel UI changed?)". | +| Login expired between CSRF fetch and PUT | Cannot happen: both run under the same `c.mu` lock-hold; no re-auth can interleave. | +| Pre-change `GetDomainDelegation` fails during Create | Logged warning; Create proceeds with empty `previous_nameservers`. Not blocking. | +| PUT non-2xx | Surface Hover's response body: "hover SetNameservers %q: HTTP %d: %s". | | Cloudflare challenge on PUT | Manifests as non-2xx → same path. Operator must allowlist the runner IP. README documents. | -| Domain rename via Update | Typed error "domain change requires resource replace, not update" (mirrors namecheap pattern). | -| Delete: PUT to Hover-default NS fails | Propagate; IaC state retained. | +| Domain rename via Update | Typed error "domain change requires resource replace, not update". | +| Delete: PUT to previous_nameservers (or default) fails | Propagate; IaC state retained. | +| Read endpoint returns empty `nameservers` field | Diff treats current as empty → first Plan after that point reports `NeedsUpdate=true` once. After the next Apply succeeds, state catches up. Logged warning for visibility. | ## Testing | Layer | Coverage | |---|---| -| `internal/hover/client_test.go` | httptest stub: `/control_panel/domain/` returns HTML with meta csrf-token → `SetNameservers` PUT asserts URL + body shape + `X-CSRF-Token` header. Failure paths: non-2xx CSRF fetch, missing meta tag, non-2xx PUT. Existing MFA-on / MFA-off login paths re-verified. | -| `internal/drivers/delegation_test.go` | Fake client: Create/Update/Read/Delete/Diff happy paths. Diff multiset order-independence (`[a,b,c]` vs `[c,b,a]` → NeedsUpdate=false). Domain rename → NeedsReplace=true + ForceNew. ctx-cancellation propagates from every method. Delete writes `[ns1.hover.com,ns2.hover.com]`. Config validation: missing domain, missing nameservers, fewer than 2 nameservers, duplicate nameservers. | +| `internal/hover/client_test.go` | httptest stub: `/control_panel/domain/` returns HTML with meta csrf-token → `SetNameservers` PUT asserts URL + body shape + `X-CSRF-Token` header. `/api/control_panel/domains/domain-` GET stub returns JSON with `nameservers` populated → `GetDomainDelegation` parses it. Failure paths: non-2xx CSRF fetch, missing meta tag, non-2xx PUT, non-2xx GET. Existing MFA-on / MFA-off login paths re-verified. | +| `internal/drivers/delegation_test.go` | Fake client implementing `HoverDelegationClient`. Create/Update/Read/Delete/Diff happy paths. Diff multiset order-independence (`[a,b,c]` vs `[c,b,a]` → NeedsUpdate=false). Domain rename → NeedsReplace=true + ForceNew. ctx-cancellation propagates from every method. Delete with previous_nameservers in state → PUTs those. Delete without previous_nameservers → PUTs `[ns1.hover.com,ns2.hover.com]` fallback. Outputs round-trip through `json.Marshal`+`Unmarshal` confirms `[]any` (not `[]string`) is what crosses the boundary. Config validation: missing domain, missing nameservers, zero-length nameservers, duplicate nameservers, empty-string nameservers. | | `internal/iacserver_test.go` | Capabilities lists both `infra.dns` and `infra.dns_delegation`. gRPC bufconn smoke for the new type. | -| Field test | `wfctl apply` in GHA against gocodealone.tech. Verify Hover UI shows the three DigitalOcean nameservers post-apply. | +| Field test (deferred session) | `wfctl apply` in GHA against gocodealone.tech. Pass criterion: Hover control panel UI shows the three DigitalOcean nameservers post-apply. `dig +short NS gocodealone.tech` is a propagation check, not a plugin check — verify separately after TTL expiry. | ## Assumptions -| # | Claim | Risk if false | Evidence | +| # | Claim | Risk if false | Evidence / verification status | |---|---|---|---| | A1 | Captured PUT endpoint shape is stable across Hover releases. | Plugin breaks; need re-capture. | OSS clients show 5+ years of stability on the related `/api/dns` endpoints; same control panel codebase. | -| A2 | CSRF tokens from `/control_panel/domain/` are valid for `/api/control_panel/domains/` PUTs (Rails per-session, not per-action). | PUT rejects with 422; retry-with-fresh-fetch loop required. | Rails default behavior; the captured curl used a token fetched from an arbitrary control_panel page. | +| A2 | The meta-tag CSRF token fetched from `/control_panel/domain/` is the value Hover's API gateway validates against the `X-CSRF-Token` header on PUTs to `/api/control_panel/domains/`. | PUT rejects with 422; need a different token source. | The captured browser session emitted exactly this combination — meta token in the page, same token replayed as `X-CSRF-Token` header. Adversarial-review-1 finding addressed: documented the exact source page + header mapping. | | A3 | No Cloudflare/CAPTCHA gate on the PUT path from a fresh GH-runner IP. | Field test fails; mitigation = self-hosted runner OR document a stable egress IP. | README already documents this as a CAPTCHA caveat on the existing DNS flow; same risk model. | | A4 | Hover idempotently accepts "set to same nameservers" (no-op success). | Only matters on Create-after-state-loss; Diff prevents the re-PUT in normal flow. | Inferred from typical PUT-idempotency conventions; not verified. | -| A5 | Hover's default nameservers are `ns1.hover.com` + `ns2.hover.com`. | Delete writes wrong values; user manually fixes once. | chickenandpork/hoverdnsapi test fixtures show this pair on multiple domains. | -| A6 | `GET /api/domains//dns` returns `nameservers: [...]` in the response. | Read returns empty; Diff reports perpetual drift. Fallback: `GET /api/domains/` (different endpoint per jmhodges/hover). | chickenandpork fixtures include the field; not verified against accounts with externally-set nameservers. | -| A7 | GH-hosted runners can reach hover.com. | Workflow fails at first request; mitigation = self-hosted runner. | No known IP-based block. | +| A5 | When `previous_nameservers` is missing from state, the fallback `[ns1.hover.com, ns2.hover.com]` is a reasonable Hover default. | Delete writes wrong values for an account whose original NS set was different. User manually fixes once. | chickenandpork/hoverdnsapi test fixtures show this pair on most domains; `ns3.hover.com` exists in some fixtures. Mitigated by the primary path (stashed previous_nameservers) capturing the actual prior state. The hardcoded fallback is the last-resort path only. | +| A6 | `GET /api/control_panel/domains/domain-` returns `nameservers: [...]` in the response body. | Read returns empty; Diff false-positives on every Plan. | The PUT is on the same endpoint — same API family. Assumed to surface the field on GET, but NOT yet curl-verified. **First implementation task: live-verify this with a single curl against the captured session before writing the driver.** If it returns a different shape than expected, the implementation pauses for a design amendment. Tracking-issue placeholder. | +| A7 | GH-hosted runners can reach hover.com without IP-based blocking. | Workflow fails at first request; mitigation = self-hosted runner. | No known IP-based block. | ## Rollback @@ -188,26 +199,51 @@ The change touches runtime (plugin loading + a live registrar PUT), so rollback - **Plugin (workflow-plugin-hover v0.2.0)**: `wfctl plugin install workflow-plugin-hover@v0.1.0` reverts the install on any consumer. - **Registry**: revert the manifest PR; `wfctl plugin search` falls back to v0.1.0. -- **gocodealone-multisite**: delete the `infra.dns_delegation` resource block; future `wfctl apply` no-ops. To reset gocodealone.tech back to Hover's nameservers: re-run the workflow with `nameservers: [ns1.hover.com, ns2.hover.com]` declared, OR set them manually in Hover's UI. +- **gocodealone-multisite**: two paths: + - **State-still-active**: delete the `infra.dns_delegation` resource block. `wfctl apply` will Delete the resource, which PUTs the stashed `previous_nameservers` back via the captured pre-Create state. + - **State-already-cleared** (resource removed without a Delete pass): manually re-add the resource with `nameservers: []` and apply. OR set them via Hover's UI directly. - **DNS itself**: the PUT is reversible — set whatever NS list you want via another apply. DNS propagation can take up to 24h, but the registrar state flips immediately. -## Top 3 surfaced doubts (from self-challenge) +## Surfaced doubts after adversarial review round 1 + +(Original three doubts retained; new ones from adversarial review escalated to assumptions A2/A6 + the Concurrency section.) + +1. **CSRF-per-PUT cost**: per-PUT control_panel page fetch doubles the request count. If Hover throttles these GETs we may need to fall back to cached-with-1h-TTL CSRF. User-chosen "fetch fresh" is the safer default. +2. **Cloudflare on GHA**: shared-IP runners can trip bot challenges. Fallback: self-hosted runner with a stable egress IP that's been allowlisted via a manual login from that IP. +3. **A6 — Read endpoint coverage**: now mitigated by switching primary Read from `/api/domains//dns` to `/api/control_panel/domains/domain-` (same API family as the PUT). Still needs live curl verification as the first implementation step. + +## Adversarial review round 1 — findings addressed + +| Finding | Severity | Resolution | +|---|---|---| +| Read endpoint uncertainty (A6) | Critical | Switched primary Read to `/api/control_panel/domains/domain-` (same API family as the PUT). First implementation task is curl-verifying the response shape. | +| `Outputs["nameservers"]` encoding unspecified | Critical | Explicitly spec'd as `[]any` (not `[]string`) with helper `nameserversToAny` + round-trip JSON test. References structpb boundary invariant in `iacserver.go` + workspace memory. | +| CSRF token source ambiguity | Important | Documented the two distinct regexes (`csrfRe` for form token on `/signin`; new `csrfMetaRe` for meta tag on `/control_panel/`). Cited that both shapes coexist in the captured browser session. | +| `ensureLogin` + `fetchControlPanelCSRF` concurrency | Important | New `*Locked` helpers; `SetNameservers` holds `c.mu` across both the CSRF GET and the PUT. Eliminates the race window. | +| Delete hardcodes Hover defaults | Important | Primary path stashes `previous_nameservers` at Create; Delete restores from state. Hardcoded `[ns1.hover.com, ns2.hover.com]` only as a last-resort fallback for state-less / pre-v0.2.0 resources. | +| Registry + multisite sequenced in same session | Minor | Architecture table now explicitly defers #2 and #3 to a separate session post-goreleaser. | +| `≥2 nameservers` validation policy | Minor | Relaxed to `≥1` with distinct + non-empty constraints. | +| Field test success criterion under propagation delay | Minor | Tightened: pass = Hover UI shows new NS; `dig` is a separate post-TTL verification. | + +## Sequencing (revised) + +**Current session (this PR):** -1. **CSRF-per-PUT cost**: per-PUT control_panel page fetch doubles the request count. If Hover throttles these GETs we may need to fall back to cached-with-1h-TTL CSRF (matching the login session). User-chosen "fetch fresh" is the safer default; field test will validate. -2. **Cloudflare on GHA**: shared-IP runners can trip bot challenges. Fallback: self-hosted runner with a stable egress IP that's been allowlisted via a manual login from that IP. Already a caveat in the README for the DNS-records flow. -3. **Read endpoint coverage of arbitrary external NS**: fixtures show Hover-defaults + Tucows alt only. If `GET /api/domains//dns` doesn't echo back `ns1.digitalocean.com` etc. post-set, Diff false-positives on every Plan. Mitigation: add a fallback to `GET /api/domains/` if observed in field test. +1. workflow-plugin-hover v0.2.0 plugin PR opens → CI green → Copilot review rounds → merge → tag v0.2.0 → goreleaser publishes assets. -## Sequencing +**Separate later session:** -1. Plugin PR opens → CI green → Copilot review rounds → merge → tag v0.2.0 → goreleaser publishes assets. -2. Registry manifest update PR with v0.2.0 SHAs → CI green → merge. +2. workflow-registry manifest update PR with v0.2.0 SHAs → CI green → merge. 3. gocodealone-multisite PR adds `config/dns.wfctl.yaml` + `.github/workflows/dns-delegation.yml` → merge. 4. Operator (Jon) runs the workflow_dispatch → DNS delegation flips to DO. -5. Validate via Hover UI + `dig +short NS gocodealone.tech` post-propagation. +5. Validate via Hover UI; separately verify propagation via `dig +short NS gocodealone.tech` after TTL expiry. ## References - Captured curl from the Hover web UI session (2026-05-20). - workflow-plugin-namecheap PR #4 round-6 hardening pattern (ctx propagation across all driver methods). +- workflow-plugin-namecheap PR #4 round-4 lesson (build outputs from desired set, no read-after-write). - chickenandpork/hoverdnsapi `Domain.NameServers` field + fixtures. - jmhodges/hover `Domain.Nameservers` field (alternate read endpoint). +- `feedback_workflow_plugin_structpb_boundary` workspace memory (typed slices reject structpb marshal). +- Adversarial review round 1 (2026-05-20) — 2 Critical / 3 Important / 3 Minor findings addressed inline in this revision. From 276e57026d3197074fe216423bf47e3420fc71ca Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 20:34:04 -0400 Subject: [PATCH 03/20] =?UTF-8?q?docs:=20revise=20design=20=E2=80=94=20adv?= =?UTF-8?q?ersarial=20review=20round=202?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses 1 Critical + 2 Important from round 2: CRITICAL: - TOCTOU between ensureLogin + c.mu.Lock in SetNameservers: new ensureLoginLocked helper; SetNameservers holds c.mu for the entire auth -> CSRF -> PUT sequence. No interleaving window remains. IMPORTANT: - Domain struct dual-population ambiguity: introduced distinct DomainDelegation type returned by GetDomainDelegation. Existing Domain struct unchanged. - Silent Apply thrash on empty nameservers: GetDomainDelegation returns typed ErrEmptyNameservers on zero entries. Loud failure at first plan instead of silent re-apply loop. Minor findings tracked for plan-phase pickup. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../2026-05-20-hover-dns-delegation-design.md | 39 ++++++++++++------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/docs/plans/2026-05-20-hover-dns-delegation-design.md b/docs/plans/2026-05-20-hover-dns-delegation-design.md index 8618f79..a75eac3 100644 --- a/docs/plans/2026-05-20-hover-dns-delegation-design.md +++ b/docs/plans/2026-05-20-hover-dns-delegation-design.md @@ -1,7 +1,7 @@ # Hover DNS Delegation — `infra.dns_delegation` Design **Date:** 2026-05-20 -**Status:** Revised after adversarial review round 1 +**Status:** Revised after adversarial review round 2 **Scope:** workflow-plugin-hover v0.2.0 — new resource type for registrar-level nameserver delegation **Field-test target:** gocodealone.tech → ns1/2/3.digitalocean.com @@ -48,25 +48,25 @@ The plugin PR (#1) is the entire scope of the current autonomous pass. #2 and #3 ### `internal/hover/client.go` extensions -- `Domain.Nameservers []string \`json:"nameservers,omitempty"\`` — added to the existing struct. Source = the new control-panel detail endpoint (see Read path below), NOT the DNS-records endpoint. Backwards-compatible additive field; existing `GetDomain` callers unaffected (returns empty when source endpoint doesn't supply it). +- `type DomainDelegation struct { ID string; Name string; Nameservers []string }` — **distinct new type** for the control-panel detail endpoint. Avoids polluting the existing `Domain` (which represents the `/api/domains//dns` shape with `Records []DNSRecord`). Two endpoints → two types → no ambiguity over which fields are populated by which path. (Per adversarial review round 2 finding #2.) - `csrfMetaRe = regexp.MustCompile(\``, parses meta token via `csrfMetaRe`. Non-2xx → typed error "hover: fetch control_panel CSRF: HTTP %d"; missing meta → typed error "hover: CSRF meta tag not found at /control_panel/domain/%s (control_panel UI changed?)". Caller must hold `c.mu` (see Concurrency section). -- `GetDomainDelegation(ctx context.Context, domainName string) (*Domain, error)` — **new method** (per adversarial review recommendation #2). `GET /api/control_panel/domains/domain-`. Same API family as the PUT; far more likely to surface the `nameservers` field reliably. Returns a `*Domain` with `Nameservers` populated. This is the **primary Read source** for `DelegationDriver`. +- `GetDomainDelegation(ctx context.Context, domainName string) (*DomainDelegation, error)` — **new method**. `GET /api/control_panel/domains/domain-`. Same API family as the PUT; far more likely to surface the `nameservers` field reliably. Returns a `*DomainDelegation`. **If the parsed response yields zero nameservers, returns a typed error `ErrEmptyNameservers`** rather than a zero-length slice. This converts the silent-thrash failure mode (empty → Diff says NeedsUpdate forever → re-PUT loop) into a loud, single-iteration error visible at the first `wfctl plan`. (Per adversarial review round 2 finding #3.) This is the **primary Read source** for `DelegationDriver`. - `SetNameservers(ctx context.Context, domainName string, ns []string) error` — see Concurrency section below for lock discipline. Eager `ensureLogin`, `fetchControlPanelCSRF`, `PUT /api/control_panel/domains/domain-` with JSON body + `X-CSRF-Token` header. PUT non-2xx → surface Hover's body as error. -### Concurrency: `SetNameservers` + `ensureLogin` interaction +### Concurrency: `SetNameservers` holds `c.mu` across the entire critical section -The existing `ensureLogin` holds `c.mu` for its entire duration. `SetNameservers` does: +Adversarial review round 2 identified a TOCTOU window in the round-1 design: calling `ensureLogin` (which acquires+releases `c.mu`) BEFORE acquiring the lock for the CSRF+PUT phase still allowed another goroutine to re-auth in the gap between the two lock-acquisitions. + +Round-2 fix: add a private `ensureLoginLocked(ctx)` helper that checks `loggedAt` and re-auths without acquiring `c.mu` (caller must already hold it). Refactor existing `ensureLogin` to acquire-lock-then-call-Locked. `SetNameservers` holds the lock for the entire auth → CSRF → PUT sequence: ```go func (c *Client) SetNameservers(ctx context.Context, domainName string, ns []string) error { - if err := c.ensureLogin(ctx); err != nil { // acquires + releases c.mu - return err - } c.mu.Lock() defer c.mu.Unlock() - // Inside the lock, both the CSRF fetch and the PUT execute against - // the same session-cookie state. No re-auth can fire between them. + if err := c.ensureLoginLocked(ctx); err != nil { + return err + } csrf, err := c.fetchControlPanelCSRFLocked(ctx, domainName) if err != nil { return err @@ -75,14 +75,16 @@ func (c *Client) SetNameservers(ctx context.Context, domainName string, ns []str } ``` -Two `Locked` helpers added (`fetchControlPanelCSRFLocked`, `putNameserversLocked`) so the public `SetNameservers` can hold the lock across both HTTP round trips, eliminating the race window the reviewer identified. +Three `*Locked` helpers added: `ensureLoginLocked`, `fetchControlPanelCSRFLocked`, `putNameserversLocked`. The existing public `ensureLogin` becomes a thin wrapper that acquires-then-calls-Locked, preserving its existing call sites' lock semantics (no behavior change to existing callers). + +This genuinely eliminates the race window: no other goroutine can interleave between auth-check and PUT, because the lock is held throughout. The pattern follows standard Go mutex discipline (Locked variants for callers already holding the lock). ### `internal/drivers/delegation.go` (new, ~150 LoC) - `type DelegationDriver struct { client HoverDelegationClient }` where the test-injectable interface is: ```go type HoverDelegationClient interface { - GetDomainDelegation(ctx context.Context, domain string) (*hover.Domain, error) + GetDomainDelegation(ctx context.Context, domain string) (*hover.DomainDelegation, error) SetNameservers(ctx context.Context, domain string, ns []string) error } ``` @@ -170,7 +172,7 @@ wfctl persists state. Subsequent Plans no-op until config changes. | Cloudflare challenge on PUT | Manifests as non-2xx → same path. Operator must allowlist the runner IP. README documents. | | Domain rename via Update | Typed error "domain change requires resource replace, not update". | | Delete: PUT to previous_nameservers (or default) fails | Propagate; IaC state retained. | -| Read endpoint returns empty `nameservers` field | Diff treats current as empty → first Plan after that point reports `NeedsUpdate=true` once. After the next Apply succeeds, state catches up. Logged warning for visibility. | +| Read endpoint returns empty `nameservers` field | `GetDomainDelegation` returns typed `ErrEmptyNameservers`. `Read` propagates as error; `Diff` propagates; `wfctl plan` fails loudly with "hover: delegation read returned 0 nameservers; verify field shape" — converts the previous silent re-apply thrash into a single-iteration loud failure. | ## Testing @@ -212,6 +214,17 @@ The change touches runtime (plugin loading + a live registrar PUT), so rollback 2. **Cloudflare on GHA**: shared-IP runners can trip bot challenges. Fallback: self-hosted runner with a stable egress IP that's been allowlisted via a manual login from that IP. 3. **A6 — Read endpoint coverage**: now mitigated by switching primary Read from `/api/domains//dns` to `/api/control_panel/domains/domain-` (same API family as the PUT). Still needs live curl verification as the first implementation step. +## Adversarial review round 2 — findings addressed + +| Finding | Severity | Resolution | +|---|---|---| +| TOCTOU between `ensureLogin` and `c.mu.Lock()` in `SetNameservers` | Critical | Refactored: new `ensureLoginLocked` helper; `SetNameservers` holds `c.mu` for the entire auth → CSRF → PUT sequence. No interleaving window. | +| `Domain` struct dual-population ambiguity | Important | Introduced distinct `DomainDelegation` type returned by `GetDomainDelegation`. Existing `Domain` struct unchanged. | +| Silent Apply thrash if Read returns empty nameservers | Important | `GetDomainDelegation` returns typed `ErrEmptyNameservers` on zero-entries. Loud failure at first plan instead of silent loop. | +| `HoverProvider` struct comment not updated | Minor | Implementation will update both the type comment and Initialize. | +| Field-test YAML shape not communicated to follow-up session | Minor | PR #1 description will include the draft `dns.wfctl.yaml` shape. | +| ForceNew Replace codepath unjustified vs requirements | Minor | Kept for symmetry with namecheap pattern + future-proofing; explicit test case added to driver_test for documentation, not because the user asked for cross-domain rename. | + ## Adversarial review round 1 — findings addressed | Finding | Severity | Resolution | From e333ea43ea0fb7a733c30a6963daae2338872dd1 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 20:37:44 -0400 Subject: [PATCH 04/20] docs: round-3 inline clarifications per reviewer recommendation Round 3 found 0 Critical + 2 Important + 3 Minor; reviewer explicitly recommended doc-level inline clarifications rather than a round-4 dispatch. Three applied: 1. Data-flow diagram fixed: lock acquired BEFORE ensureLoginLocked (was misleading; implementer could re- introduce TOCTOU by following the diagram). 2. A6 tentative JSON envelope spec'd: flat object, not {"domains":[...]} wrapper. Curl verification gate remains the first implementation task. 3. I/O-under-lock trade-off documented: SetNameservers holds c.mu across two HTTP round-trips. Acknowledged as correctness-over-throughput; production hardening path (session-scoped CSRF cache) deferred to a follow-up if field-test reveals contention. Status flipped to PASS (per reviewer guidance). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../2026-05-20-hover-dns-delegation-design.md | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/docs/plans/2026-05-20-hover-dns-delegation-design.md b/docs/plans/2026-05-20-hover-dns-delegation-design.md index a75eac3..6f6b38a 100644 --- a/docs/plans/2026-05-20-hover-dns-delegation-design.md +++ b/docs/plans/2026-05-20-hover-dns-delegation-design.md @@ -1,7 +1,7 @@ # Hover DNS Delegation — `infra.dns_delegation` Design **Date:** 2026-05-20 -**Status:** Revised after adversarial review round 2 +**Status:** Adversarial review PASS (round-3 inline clarifications per reviewer recommendation) **Scope:** workflow-plugin-hover v0.2.0 — new resource type for registrar-level nameserver delegation **Field-test target:** gocodealone.tech → ns1/2/3.digitalocean.com @@ -79,6 +79,8 @@ Three `*Locked` helpers added: `ensureLoginLocked`, `fetchControlPanelCSRFLocked This genuinely eliminates the race window: no other goroutine can interleave between auth-check and PUT, because the lock is held throughout. The pattern follows standard Go mutex discipline (Locked variants for callers already holding the lock). +**Trade-off acknowledged**: `SetNameservers` holds `c.mu` across two HTTP round-trips (CSRF GET + PUT, up to ~60s combined with the 30s default `http.Client` timeout). Any concurrent goroutine calling another `*Client` method on the same instance blocks for that duration. This is correctness-over-throughput: the alternative — releasing the lock between auth and PUT — was the round-1 design and the round-2 reviewer correctly flagged it as racy. For the field-test scope (single goroutine, one `infra.dns_delegation` resource) this is harmless. For future configs that mix `infra.dns` records and `infra.dns_delegation` on the same client, the engine's per-resource dispatch will serialise through the mutex. The production hardening path (if mixed-resource throughput becomes a concern) is to cache the CSRF token at session granularity, matching `sessionStaleAfter`'s 1h window, and invalidate on 4xx PUT responses — narrowing the lock to just the PUT. Deferred until field-test reveals it's needed. + ### `internal/drivers/delegation.go` (new, ~150 LoC) - `type DelegationDriver struct { client HoverDelegationClient }` where the test-injectable interface is: @@ -148,9 +150,9 @@ wfctl Plan → DelegationDriver.Diff(desired, current) wfctl Apply → DelegationDriver.Create or Update ├── ctx.Err check ├── Create only: GetDomainDelegation → stash current NS as previous_nameservers (best-effort) - ├── client.SetNameservers (under c.mu, both CSRF GET + PUT inside the lock) - │ ├── ensureLogin - │ ├── lock c.mu + ├── client.SetNameservers + │ ├── lock c.mu (acquired FIRST) + │ ├── ensureLoginLocked (no separate lock; runs under held c.mu) │ ├── fetchControlPanelCSRFLocked → token │ └── PUT /api/control_panel/domains/domain- │ Body: {"field":"nameservers","value":[...]} @@ -192,7 +194,7 @@ wfctl persists state. Subsequent Plans no-op until config changes. | A3 | No Cloudflare/CAPTCHA gate on the PUT path from a fresh GH-runner IP. | Field test fails; mitigation = self-hosted runner OR document a stable egress IP. | README already documents this as a CAPTCHA caveat on the existing DNS flow; same risk model. | | A4 | Hover idempotently accepts "set to same nameservers" (no-op success). | Only matters on Create-after-state-loss; Diff prevents the re-PUT in normal flow. | Inferred from typical PUT-idempotency conventions; not verified. | | A5 | When `previous_nameservers` is missing from state, the fallback `[ns1.hover.com, ns2.hover.com]` is a reasonable Hover default. | Delete writes wrong values for an account whose original NS set was different. User manually fixes once. | chickenandpork/hoverdnsapi test fixtures show this pair on most domains; `ns3.hover.com` exists in some fixtures. Mitigated by the primary path (stashed previous_nameservers) capturing the actual prior state. The hardcoded fallback is the last-resort path only. | -| A6 | `GET /api/control_panel/domains/domain-` returns `nameservers: [...]` in the response body. | Read returns empty; Diff false-positives on every Plan. | The PUT is on the same endpoint — same API family. Assumed to surface the field on GET, but NOT yet curl-verified. **First implementation task: live-verify this with a single curl against the captured session before writing the driver.** If it returns a different shape than expected, the implementation pauses for a design amendment. Tracking-issue placeholder. | +| A6 | `GET /api/control_panel/domains/domain-` returns a JSON object containing `nameservers: [string,...]` at the top level. | Read returns empty; `ErrEmptyNameservers` surfaces (loud). | The PUT is on the same endpoint — same API family. **Tentative envelope spec (to confirm with curl as first implementation task):** `{"id": "domain-", "domain_name": "", "nameservers": ["..."], ...}` — a flat object, NOT wrapped in `{"domains": [...]}` like `/api/domains//dns`. If the curl reveals a different shape, the implementer pauses + amends the design before writing the decoder struct. The `DomainDelegation` struct's JSON tags follow this tentative spec. | | A7 | GH-hosted runners can reach hover.com without IP-based blocking. | Workflow fails at first request; mitigation = self-hosted runner. | No known IP-based block. | ## Rollback @@ -214,6 +216,16 @@ The change touches runtime (plugin loading + a live registrar PUT), so rollback 2. **Cloudflare on GHA**: shared-IP runners can trip bot challenges. Fallback: self-hosted runner with a stable egress IP that's been allowlisted via a manual login from that IP. 3. **A6 — Read endpoint coverage**: now mitigated by switching primary Read from `/api/domains//dns` to `/api/control_panel/domains/domain-` (same API family as the PUT). Still needs live curl verification as the first implementation step. +## Adversarial review round 3 — clarifications applied inline + +Round 3 found 0 Critical + 2 Important + 3 Minor; reviewer's own verdict was "doc clarifications, no round-4 needed — author can make them inline and proceed". Three clarifications applied: + +1. **Data-flow diagram fixed**: lock acquired BEFORE `ensureLoginLocked`, matching the Concurrency pseudocode (was misleading: showed lock after ensureLogin). +2. **A6 JSON envelope spec'd tentatively**: flat object, not `{"domains":[...]}` wrapper. Curl verification gate remains the first implementation task; tentative shape gives the implementer a struct to write. +3. **I/O-under-lock trade-off documented**: explicit acknowledgment in Concurrency section that `SetNameservers` holds `c.mu` across two HTTP round-trips (up to ~60s). Single-goroutine field-test = harmless; mixed-resource configs serialise. Production hardening path (session-scoped CSRF cache) deferred until field-test demonstrates need. + +`ErrEmptyNameservers` sentinel will be declared in the `internal/hover` package (driver-side `errors.Is` check); minor #1 is a plan-phase concern. + ## Adversarial review round 2 — findings addressed | Finding | Severity | Resolution | From 51c94a41304a28d73dab1743eed51c45bd38cb3f Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 20:43:25 -0400 Subject: [PATCH 05/20] docs: implementation plan for infra.dns_delegation v0.2.0 13-task plan with TDD per-task, structpb-safe Outputs, lock-held SetNameservers, Read via control-panel endpoint with ErrEmptyNameservers sentinel, Delete restores stashed previous_nameservers (fallback to Hover defaults). Single PR (feat/dns-delegation). Out of scope: workflow-registry manifest bump + gocodealone-multisite field-test artifact + live field test (deferred to a separate session post-goreleaser). Rollback notes per task. Scope Manifest authored. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/plans/2026-05-20-hover-dns-delegation.md | 1787 +++++++++++++++++ 1 file changed, 1787 insertions(+) create mode 100644 docs/plans/2026-05-20-hover-dns-delegation.md diff --git a/docs/plans/2026-05-20-hover-dns-delegation.md b/docs/plans/2026-05-20-hover-dns-delegation.md new file mode 100644 index 0000000..55c0f73 --- /dev/null +++ b/docs/plans/2026-05-20-hover-dns-delegation.md @@ -0,0 +1,1787 @@ +# Hover DNS Delegation Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. + +**Goal:** Add `infra.dns_delegation` resource type to workflow-plugin-hover so wfctl can set a domain's registrar-level nameservers via the Hover control-panel API. + +**Architecture:** New `DelegationDriver` in `internal/drivers/delegation.go`, registered alongside the existing `DNSDriver` in `internal/provider.go`. Adds three control-panel methods to `internal/hover/client.go` (`GetDomainDelegation`, `SetNameservers`, plus the `fetchControlPanelCSRFLocked` helper). A new `DomainDelegation` type isolates the control-panel response shape from the existing `Domain`. Lock discipline: `SetNameservers` holds `c.mu` across the entire auth → CSRF → PUT sequence to eliminate the round-2 TOCTOU window. + +**Tech Stack:** Go 1.21+; standard library only (no new third-party deps); reuses existing `regexp`, `net/http`, `encoding/json`, `sync`. gRPC plugin SDK via `github.com/GoCodeAlone/workflow`. + +**Base branch:** main (working branch: `feat/dns-delegation` — already created) + +**Design reference:** `docs/plans/2026-05-20-hover-dns-delegation-design.md` (PASSed 3 adversarial review rounds; round-3 doc clarifications applied inline). + +--- + +## Scope Manifest + +**PR Count:** 1 +**Tasks:** 13 +**Estimated Lines of Change:** ~600 (client extensions + new driver + tests + provider wiring + plugin.json) + +**Out of scope:** +- workflow-registry manifest bump for v0.2.0 (deferred session; cannot author until goreleaser publishes asset SHAs). +- gocodealone-multisite `config/dns.wfctl.yaml` + `.github/workflows/dns-delegation.yml` (deferred session; gated on registry manifest carrying v0.2.0). +- Live field-test against gocodealone.tech via `workflow_dispatch` (deferred session; gated on the two artifacts above). +- The other domain-level fields the Hover endpoint accepts (`whois_privacy`, `auto_renew`, `locked`) — YAGNI; only `nameservers` is in scope. + +**PR Grouping:** + +| PR # | Title | Tasks | Branch | +|------|-------|-------|--------| +| 1 | feat: infra.dns_delegation resource type (v0.2.0) | Task 1, Task 2, Task 3, Task 4, Task 5, Task 6, Task 7, Task 8, Task 9, Task 10, Task 11, Task 12, Task 13 | feat/dns-delegation | + +**Status:** Draft + +--- + +### Task 1: Add `DomainDelegation` type and `ErrEmptyNameservers` sentinel + +**Files:** +- Modify: `internal/hover/client.go` — extend with type + var declarations + +**Step 1: Write the failing test** + +Add to `internal/hover/client_test.go`: + +```go +func TestDomainDelegation_JSONShape(t *testing.T) { + // Tentative envelope per design A6: flat object, not wrapped. + body := `{"id":"domain-example.com","domain_name":"example.com","nameservers":["a.com","b.com"]}` + var d DomainDelegation + if err := json.Unmarshal([]byte(body), &d); err != nil { + t.Fatalf("decode: %v", err) + } + if d.ID != "domain-example.com" { + t.Errorf("ID = %q, want domain-example.com", d.ID) + } + if d.Name != "example.com" { + t.Errorf("Name = %q, want example.com", d.Name) + } + if len(d.Nameservers) != 2 || d.Nameservers[0] != "a.com" || d.Nameservers[1] != "b.com" { + t.Errorf("Nameservers = %v, want [a.com b.com]", d.Nameservers) + } +} + +func TestErrEmptyNameservers_IsSentinel(t *testing.T) { + wrapped := fmt.Errorf("hover GetDomainDelegation: %w", ErrEmptyNameservers) + if !errors.Is(wrapped, ErrEmptyNameservers) { + t.Error("errors.Is should match ErrEmptyNameservers when wrapped") + } +} +``` + +**Step 2: Run test to verify it fails** + +Run: `cd /Users/jon/workspace/workflow-plugin-hover && GOWORK=off go test ./internal/hover -run "TestDomainDelegation_JSONShape|TestErrEmptyNameservers_IsSentinel" -v` +Expected: FAIL with `undefined: DomainDelegation` and `undefined: ErrEmptyNameservers`. + +**Step 3: Write minimal implementation** + +Add to `internal/hover/client.go` (near the existing `Domain` struct): + +```go +// DomainDelegation is the response shape of GET /api/control_panel/domains/domain-. +// Distinct from Domain (which represents the /api/domains//dns shape with Records) +// to avoid ambiguity over which fields are populated by which endpoint. +// +// Tentative envelope per design A6: flat object, not wrapped in {"domains":[...]}. +// First field-test call must confirm this shape; if Hover returns a different envelope +// the implementer pauses and amends the design before proceeding. +type DomainDelegation struct { + ID string `json:"id"` + Name string `json:"domain_name"` + Nameservers []string `json:"nameservers"` +} + +// ErrEmptyNameservers is returned by GetDomainDelegation when the parsed +// response has zero nameservers. Converts the silent-thrash failure mode +// (empty → Diff says NeedsUpdate forever → re-PUT loop) into a loud, +// single-iteration error visible at the first wfctl plan. +var ErrEmptyNameservers = errors.New("hover: delegation read returned 0 nameservers (verify field shape)") +``` + +**Step 4: Run test to verify it passes** + +Run: `GOWORK=off go test ./internal/hover -run "TestDomainDelegation_JSONShape|TestErrEmptyNameservers_IsSentinel" -v` +Expected: PASS for both tests. + +**Step 5: Commit** + +```bash +git add internal/hover/client.go internal/hover/client_test.go +git commit -m "feat(hover): DomainDelegation type + ErrEmptyNameservers sentinel" +``` + +--- + +### Task 2: Refactor `ensureLogin` into `ensureLogin` + `ensureLoginLocked` + +**Files:** +- Modify: `internal/hover/client.go:82-133` — split lock-acquisition from body + +**Step 1: Write the failing test** + +Add to `internal/hover/client_test.go`: + +```go +func TestEnsureLoginLocked_CallableUnderHeldLock(t *testing.T) { + // Build a Client with a fresh loggedAt so ensureLoginLocked + // short-circuits without making HTTP calls. + c, err := NewClient(Credentials{Username: "u", Password: "p"}, nil) + if err != nil { + t.Fatalf("NewClient: %v", err) + } + c.loggedAt = time.Now() // skip the actual login + c.mu.Lock() + defer c.mu.Unlock() + if err := c.ensureLoginLocked(context.Background()); err != nil { + t.Errorf("ensureLoginLocked under held mu: %v", err) + } +} +``` + +**Step 2: Run test to verify it fails** + +Run: `GOWORK=off go test ./internal/hover -run TestEnsureLoginLocked_CallableUnderHeldLock -v` +Expected: FAIL with `undefined: ensureLoginLocked`. + +**Step 3: Write minimal implementation** + +Refactor `internal/hover/client.go`: + +```go +// ensureLogin re-authenticates iff the session is stale. Safe to call +// before every API hit; idempotent within sessionStaleAfter. +// +// Acquires c.mu internally. Callers that already hold the lock must +// call ensureLoginLocked instead. +func (c *Client) ensureLogin(ctx context.Context) error { + c.mu.Lock() + defer c.mu.Unlock() + return c.ensureLoginLocked(ctx) +} + +// ensureLoginLocked is the implementation of ensureLogin without the lock +// acquisition. Caller MUST hold c.mu. Used by SetNameservers which holds +// c.mu across the full auth → CSRF → PUT sequence to eliminate the +// TOCTOU window between auth-check and PUT. +func (c *Client) ensureLoginLocked(ctx context.Context) error { + if !c.loggedAt.IsZero() && time.Since(c.loggedAt) < sessionStaleAfter { + return nil + } + // (existing body of ensureLogin, minus the lock acquisition) + csrf, err := c.fetchSignInCSRF(ctx) + // ... (rest of the existing login flow) +} +``` + +Move every line currently inside `ensureLogin` after `c.mu.Lock()` into `ensureLoginLocked`. The public `ensureLogin` becomes a 3-line wrapper. + +**Step 4: Run all hover tests to verify no regression** + +Run: `GOWORK=off go test ./internal/hover -count=1 -v` +Expected: PASS for all existing tests + `TestEnsureLoginLocked_CallableUnderHeldLock`. + +**Step 5: Commit** + +```bash +git add internal/hover/client.go internal/hover/client_test.go +git commit -m "refactor(hover): split ensureLogin into Locked variant" +``` + +--- + +### Task 3: Add `csrfMetaRe` regex + `fetchControlPanelCSRFLocked` helper + +**Files:** +- Modify: `internal/hover/client.go` — add regex var + method + +**Step 1: Write the failing test** + +Add to `internal/hover/client_test.go`: + +```go +func TestFetchControlPanelCSRFLocked_ExtractsMetaToken(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/control_panel/domain/example.com" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + _, _ = w.Write([]byte(` + +`)) + })) + defer srv.Close() + + c := newTestClient(t, srv.URL) + c.mu.Lock() + defer c.mu.Unlock() + token, err := c.fetchControlPanelCSRFLocked(context.Background(), "example.com") + if err != nil { + t.Fatalf("fetchControlPanelCSRFLocked: %v", err) + } + if token != "abc123xyz" { + t.Errorf("token = %q, want abc123xyz", token) + } +} + +func TestFetchControlPanelCSRFLocked_MissingMetaTag(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(``)) + })) + defer srv.Close() + + c := newTestClient(t, srv.URL) + c.mu.Lock() + defer c.mu.Unlock() + _, err := c.fetchControlPanelCSRFLocked(context.Background(), "example.com") + if err == nil { + t.Fatal("expected error when meta tag absent") + } +} + +func TestFetchControlPanelCSRFLocked_Non2xx(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "denied", http.StatusForbidden) + })) + defer srv.Close() + + c := newTestClient(t, srv.URL) + c.mu.Lock() + defer c.mu.Unlock() + _, err := c.fetchControlPanelCSRFLocked(context.Background(), "example.com") + if err == nil { + t.Fatal("expected error on 403") + } +} +``` + +If `newTestClient` doesn't exist yet, add a helper (still in `client_test.go`): + +```go +// newTestClient builds a Client that points at a httptest server URL by +// monkey-patching hoverHost via a package-level test override. +func newTestClient(t *testing.T, baseURL string) *Client { + t.Helper() + // Use a fresh Client and point its http.Client at the test server via + // a custom transport that rewrites the request URL host. + c, err := NewClient(Credentials{Username: "u", Password: "p"}, &http.Client{ + Transport: &rewritingTransport{base: baseURL}, + Timeout: 5 * time.Second, + }) + if err != nil { + t.Fatalf("NewClient: %v", err) + } + c.loggedAt = time.Now() // skip login + return c +} + +type rewritingTransport struct{ base string } + +func (rt *rewritingTransport) RoundTrip(req *http.Request) (*http.Response, error) { + target, err := url.Parse(rt.base + req.URL.Path) + if err != nil { + return nil, err + } + target.RawQuery = req.URL.RawQuery + req.URL = target + req.Host = target.Host + return http.DefaultTransport.RoundTrip(req) +} +``` + +If a similar helper already exists in `internal/hover/client_test.go` (likely — check existing tests), reuse it. + +**Step 2: Run tests to verify they fail** + +Run: `GOWORK=off go test ./internal/hover -run "TestFetchControlPanelCSRFLocked" -v` +Expected: FAIL — `undefined: fetchControlPanelCSRFLocked`. + +**Step 3: Write minimal implementation** + +Add to `internal/hover/client.go`: + +```go +// csrfMetaRe extracts the Rails CSRF meta token from a control-panel HTML +// page. Distinct from csrfRe (form-token regex used by the /signin flow) +// because the control-panel pages embed the token as a meta tag for the +// SPA layer to read, while /signin embeds it as a hidden input. +// +// Both shapes coexist in the Hover-served HTML; we match each from the +// page where it's authoritative. +var csrfMetaRe = regexp.MustCompile(`. Caller MUST hold c.mu (so the HTTP GET +// and any subsequent PUT execute against the same session-cookie state). +func (c *Client) fetchControlPanelCSRFLocked(ctx context.Context, domainName string) (string, error) { + endpoint := fmt.Sprintf("%s/control_panel/domain/%s", hoverHost, url.PathEscape(domainName)) + req, _ := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) + req.Header.Set("User-Agent", c.UserAgent) + resp, err := c.http.Do(req) + if err != nil { + return "", fmt.Errorf("hover: fetch control_panel CSRF for %q: %w", domainName, err) + } + defer resp.Body.Close() + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + body, _ := io.ReadAll(io.LimitReader(resp.Body, 512)) + return "", fmt.Errorf("hover: fetch control_panel CSRF for %q: HTTP %d: %s", domainName, resp.StatusCode, strings.TrimSpace(string(body))) + } + body, err := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) + if err != nil { + return "", fmt.Errorf("hover: fetch control_panel CSRF for %q: read body: %w", domainName, err) + } + m := csrfMetaRe.FindSubmatch(body) + if len(m) < 2 { + return "", fmt.Errorf("hover: CSRF meta tag not found at /control_panel/domain/%s (control_panel UI changed?)", domainName) + } + return string(m[1]), nil +} +``` + +**Step 4: Run tests to verify they pass** + +Run: `GOWORK=off go test ./internal/hover -run "TestFetchControlPanelCSRFLocked" -v` +Expected: PASS for all three subtests. + +**Step 5: Commit** + +```bash +git add internal/hover/client.go internal/hover/client_test.go +git commit -m "feat(hover): fetchControlPanelCSRFLocked + csrfMetaRe regex" +``` + +--- + +### Task 4: Add `GetDomainDelegation` method + +**Files:** +- Modify: `internal/hover/client.go` — add method + +**Step 1: Write the failing test** + +```go +func TestGetDomainDelegation_HappyPath(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/control_panel/domains/domain-example.com" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + _, _ = w.Write([]byte(`{"id":"domain-example.com","domain_name":"example.com","nameservers":["ns1.do.com","ns2.do.com"]}`)) + })) + defer srv.Close() + + c := newTestClient(t, srv.URL) + dom, err := c.GetDomainDelegation(context.Background(), "example.com") + if err != nil { + t.Fatalf("GetDomainDelegation: %v", err) + } + if dom.ID != "domain-example.com" { + t.Errorf("ID = %q", dom.ID) + } + if len(dom.Nameservers) != 2 { + t.Errorf("Nameservers len = %d, want 2", len(dom.Nameservers)) + } +} + +func TestGetDomainDelegation_EmptyNameserversReturnsSentinel(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"id":"domain-example.com","domain_name":"example.com","nameservers":[]}`)) + })) + defer srv.Close() + + c := newTestClient(t, srv.URL) + _, err := c.GetDomainDelegation(context.Background(), "example.com") + if !errors.Is(err, ErrEmptyNameservers) { + t.Fatalf("want ErrEmptyNameservers, got %v", err) + } +} + +func TestGetDomainDelegation_Non2xx(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "not found", http.StatusNotFound) + })) + defer srv.Close() + + c := newTestClient(t, srv.URL) + _, err := c.GetDomainDelegation(context.Background(), "example.com") + if err == nil { + t.Fatal("expected error on 404") + } +} +``` + +**Step 2: Run tests to verify they fail** + +Run: `GOWORK=off go test ./internal/hover -run TestGetDomainDelegation -v` +Expected: FAIL — `undefined: GetDomainDelegation`. + +**Step 3: Write minimal implementation** + +Add to `internal/hover/client.go`: + +```go +// GetDomainDelegation fetches the registrar-level nameserver delegation for +// the named domain via the control-panel API (same endpoint family as the +// PUT used by SetNameservers — more likely to surface nameservers reliably +// than the DNS-records-oriented /api/domains//dns endpoint). +// +// Returns ErrEmptyNameservers if the parsed response has zero nameservers. +// This loud-on-empty behavior is intentional: it converts the silent +// re-apply thrash failure mode (empty → Diff says NeedsUpdate forever) +// into a single-iteration error visible at first wfctl plan. +func (c *Client) GetDomainDelegation(ctx context.Context, domainName string) (*DomainDelegation, error) { + if err := c.ensureLogin(ctx); err != nil { + return nil, err + } + endpoint := fmt.Sprintf("%s/api/control_panel/domains/domain-%s", hoverHost, url.PathEscape(domainName)) + req, _ := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) + req.Header.Set("User-Agent", c.UserAgent) + resp, err := c.http.Do(req) + if err != nil { + return nil, fmt.Errorf("hover: GetDomainDelegation %q: %w", domainName, err) + } + defer resp.Body.Close() + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + body, _ := io.ReadAll(io.LimitReader(resp.Body, 512)) + return nil, fmt.Errorf("hover: GetDomainDelegation %q: HTTP %d: %s", domainName, resp.StatusCode, strings.TrimSpace(string(body))) + } + var d DomainDelegation + if err := json.NewDecoder(resp.Body).Decode(&d); err != nil { + return nil, fmt.Errorf("hover: GetDomainDelegation %q: decode: %w", domainName, err) + } + if len(d.Nameservers) == 0 { + return nil, fmt.Errorf("hover: GetDomainDelegation %q: %w", domainName, ErrEmptyNameservers) + } + return &d, nil +} +``` + +**Step 4: Run tests to verify they pass** + +Run: `GOWORK=off go test ./internal/hover -run TestGetDomainDelegation -v` +Expected: PASS for all three subtests. + +**Step 5: Commit** + +```bash +git add internal/hover/client.go internal/hover/client_test.go +git commit -m "feat(hover): GetDomainDelegation method (loud on empty)" +``` + +--- + +### Task 5: Add `SetNameservers` method (lock-held across full sequence) + +**Files:** +- Modify: `internal/hover/client.go` — add method + putNameserversLocked helper + +**Step 1: Write the failing test** + +```go +func TestSetNameservers_PUTShape(t *testing.T) { + var capturedURL, capturedToken, capturedCT string + var capturedBody []byte + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/control_panel/domain/example.com": + _, _ = w.Write([]byte(``)) + case "/api/control_panel/domains/domain-example.com": + capturedURL = r.URL.Path + capturedToken = r.Header.Get("X-CSRF-Token") + capturedCT = r.Header.Get("Content-Type") + capturedBody, _ = io.ReadAll(r.Body) + w.WriteHeader(http.StatusOK) + default: + t.Errorf("unexpected path: %s", r.URL.Path) + } + })) + defer srv.Close() + + c := newTestClient(t, srv.URL) + err := c.SetNameservers(context.Background(), "example.com", []string{"a.com", "b.com"}) + if err != nil { + t.Fatalf("SetNameservers: %v", err) + } + if capturedURL != "/api/control_panel/domains/domain-example.com" { + t.Errorf("URL = %q", capturedURL) + } + if capturedToken != "test-csrf-token" { + t.Errorf("X-CSRF-Token = %q", capturedToken) + } + if !strings.HasPrefix(capturedCT, "application/json") { + t.Errorf("Content-Type = %q", capturedCT) + } + var payload map[string]any + if err := json.Unmarshal(capturedBody, &payload); err != nil { + t.Fatalf("body decode: %v", err) + } + if payload["field"] != "nameservers" { + t.Errorf("field = %v", payload["field"]) + } + val, _ := payload["value"].([]any) + if len(val) != 2 || val[0] != "a.com" || val[1] != "b.com" { + t.Errorf("value = %v, want [a.com b.com]", payload["value"]) + } +} + +func TestSetNameservers_Non2xxPUT(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasPrefix(r.URL.Path, "/control_panel/domain/") { + _, _ = w.Write([]byte(``)) + return + } + http.Error(w, "bad token", http.StatusUnprocessableEntity) + })) + defer srv.Close() + + c := newTestClient(t, srv.URL) + err := c.SetNameservers(context.Background(), "example.com", []string{"a.com"}) + if err == nil { + t.Fatal("expected error on 422") + } +} +``` + +**Step 2: Run tests to verify they fail** + +Run: `GOWORK=off go test ./internal/hover -run TestSetNameservers -v` +Expected: FAIL — `undefined: SetNameservers`. + +**Step 3: Write minimal implementation** + +Add to `internal/hover/client.go`: + +```go +// SetNameservers updates the registrar-level nameservers for a domain via +// Hover's control-panel API. +// +// Lock discipline: holds c.mu for the entire auth → CSRF fetch → PUT +// sequence. This eliminates the TOCTOU window between auth-check and +// PUT (another goroutine cannot re-auth and invalidate the CSRF token +// between the two requests). +// +// Trade-off: any concurrent caller using the same *Client blocks for +// up to ~60s (two HTTP round-trips under the 30s default client timeout). +// Acceptable for the field-test scope (single goroutine, one delegation +// resource). Future: cache CSRF at session granularity if mixed-resource +// throughput becomes a concern. +func (c *Client) SetNameservers(ctx context.Context, domainName string, ns []string) error { + c.mu.Lock() + defer c.mu.Unlock() + if err := c.ensureLoginLocked(ctx); err != nil { + return err + } + csrf, err := c.fetchControlPanelCSRFLocked(ctx, domainName) + if err != nil { + return err + } + return c.putNameserversLocked(ctx, domainName, ns, csrf) +} + +// putNameserversLocked PUTs the nameservers list. Caller MUST hold c.mu. +func (c *Client) putNameserversLocked(ctx context.Context, domainName string, ns []string, csrf string) error { + endpoint := fmt.Sprintf("%s/api/control_panel/domains/domain-%s", hoverHost, url.PathEscape(domainName)) + payload := map[string]any{"field": "nameservers", "value": ns} + body, err := json.Marshal(payload) + if err != nil { + return fmt.Errorf("hover: SetNameservers %q: marshal: %w", domainName, err) + } + req, _ := http.NewRequestWithContext(ctx, http.MethodPut, endpoint, bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("User-Agent", c.UserAgent) + req.Header.Set("X-CSRF-Token", csrf) + resp, err := c.http.Do(req) + if err != nil { + return fmt.Errorf("hover: SetNameservers %q: %w", domainName, err) + } + defer resp.Body.Close() + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + respBody, _ := io.ReadAll(io.LimitReader(resp.Body, 512)) + return fmt.Errorf("hover: SetNameservers %q: HTTP %d: %s", domainName, resp.StatusCode, strings.TrimSpace(string(respBody))) + } + return nil +} +``` + +Add `"bytes"` to imports if not already present. + +**Step 4: Run tests to verify they pass** + +Run: `GOWORK=off go test ./internal/hover -run TestSetNameservers -count=1 -v` +Expected: PASS. + +Then re-run the full hover suite to confirm no regression: +Run: `GOWORK=off go test ./internal/hover -count=1` +Expected: ok (all tests pass). + +**Step 5: Commit** + +```bash +git add internal/hover/client.go internal/hover/client_test.go +git commit -m "feat(hover): SetNameservers + putNameserversLocked" +``` + +--- + +### Task 6: Create `DelegationDriver` skeleton + `HoverDelegationClient` interface + +**Files:** +- Create: `internal/drivers/delegation.go` + +**Step 1: Write the failing test** + +Create `internal/drivers/delegation_test.go`: + +```go +package drivers + +import ( + "context" + "testing" + + "github.com/GoCodeAlone/workflow-plugin-hover/internal/hover" + "github.com/GoCodeAlone/workflow/interfaces" +) + +type fakeDelegationClient struct { + getResult *hover.DomainDelegation + getErr error + setErr error + lastSetNS []string +} + +func (f *fakeDelegationClient) GetDomainDelegation(_ context.Context, _ string) (*hover.DomainDelegation, error) { + return f.getResult, f.getErr +} + +func (f *fakeDelegationClient) SetNameservers(_ context.Context, _ string, ns []string) error { + f.lastSetNS = append([]string(nil), ns...) + return f.setErr +} + +func TestDelegationDriver_TypeAndProviderIDFormat(t *testing.T) { + d := NewDelegationDriverWithClient(&fakeDelegationClient{}) + if got := d.Type(); got != "infra.dns_delegation" { + t.Errorf("Type() = %q, want infra.dns_delegation", got) + } + if got := d.ProviderIDFormat(); got != interfaces.IDFormatDomainName { + t.Errorf("ProviderIDFormat() = %v, want IDFormatDomainName", got) + } + if d.SensitiveKeys() != nil { + t.Errorf("SensitiveKeys() = %v, want nil", d.SensitiveKeys()) + } +} +``` + +**Step 2: Run tests to verify they fail** + +Run: `GOWORK=off go test ./internal/drivers -run TestDelegationDriver_TypeAndProviderIDFormat -v` +Expected: FAIL — `undefined: NewDelegationDriverWithClient`. + +**Step 3: Write minimal implementation** + +Create `internal/drivers/delegation.go`: + +```go +// Package-doc additions in this file are scoped to the dns_delegation +// driver. See dns.go for the prior infra.dns driver. +package drivers + +import ( + "context" + "errors" + "fmt" + "sort" + "strings" + + "github.com/GoCodeAlone/workflow-plugin-hover/internal/hover" + "github.com/GoCodeAlone/workflow/interfaces" +) + +// HoverDelegationClient is the subset of *hover.Client that DelegationDriver +// depends on. Injectable for tests. +type HoverDelegationClient interface { + GetDomainDelegation(ctx context.Context, domain string) (*hover.DomainDelegation, error) + SetNameservers(ctx context.Context, domain string, ns []string) error +} + +// DelegationDriver manages registrar-level nameserver delegation +// (infra.dns_delegation) for Hover-registered domains. +// +// ProviderID = apex domain name (e.g. "example.com"). One resource = one +// domain. Outputs include the desired nameservers as []any (structpb-safe) +// and a stashed previous_nameservers list captured at Create time for +// Delete-restoration. +type DelegationDriver struct { + client HoverDelegationClient +} + +// NewDelegationDriver returns a DelegationDriver bound to a real *hover.Client. +func NewDelegationDriver(c *hover.Client) *DelegationDriver { + return &DelegationDriver{client: c} +} + +// NewDelegationDriverWithClient returns a DelegationDriver bound to an +// injected client; used by tests. +func NewDelegationDriverWithClient(c HoverDelegationClient) *DelegationDriver { + return &DelegationDriver{client: c} +} + +func (d *DelegationDriver) Type() string { return "infra.dns_delegation" } + +func (d *DelegationDriver) SensitiveKeys() []string { return nil } + +func (d *DelegationDriver) ProviderIDFormat() interfaces.ProviderIDFormat { + return interfaces.IDFormatDomainName +} +``` + +**Step 4: Run tests to verify they pass** + +Run: `GOWORK=off go test ./internal/drivers -run TestDelegationDriver_TypeAndProviderIDFormat -v` +Expected: PASS. + +**Step 5: Commit** + +```bash +git add internal/drivers/delegation.go internal/drivers/delegation_test.go +git commit -m "feat(drivers): DelegationDriver skeleton + interface" +``` + +--- + +### Task 7: Implement `DelegationDriver.Create` + `parseDelegationSpec` + `nameserversToAny` + +**Files:** +- Modify: `internal/drivers/delegation.go` + +**Step 1: Write the failing test** + +Append to `internal/drivers/delegation_test.go`: + +```go +func TestDelegationDriver_Create_CallsSetNameservers(t *testing.T) { + fc := &fakeDelegationClient{ + getResult: &hover.DomainDelegation{ + ID: "domain-example.com", + Name: "example.com", + Nameservers: []string{"ns1.hover.com", "ns2.hover.com"}, + }, + } + d := NewDelegationDriverWithClient(fc) + spec := interfaces.ResourceSpec{ + Name: "example.com", + Type: "infra.dns_delegation", + Config: map[string]any{ + "domain": "example.com", + "nameservers": []any{"ns1.do.com", "ns2.do.com", "ns3.do.com"}, + }, + } + out, err := d.Create(context.Background(), spec) + if err != nil { + t.Fatalf("Create: %v", err) + } + if fc.lastSetNS == nil || len(fc.lastSetNS) != 3 { + t.Errorf("client.SetNameservers not called with 3 NS; got %v", fc.lastSetNS) + } + if out.ProviderID != "example.com" { + t.Errorf("ProviderID = %q", out.ProviderID) + } + // Outputs.nameservers MUST be []any, not []string (structpb-safe). + nsRaw, ok := out.Outputs["nameservers"] + if !ok { + t.Fatal("Outputs.nameservers missing") + } + nsAny, ok := nsRaw.([]any) + if !ok { + t.Fatalf("Outputs.nameservers = %T, want []any", nsRaw) + } + if len(nsAny) != 3 { + t.Errorf("Outputs.nameservers len = %d, want 3", len(nsAny)) + } + // previous_nameservers stashed from Hover defaults + prevRaw, ok := out.Outputs["previous_nameservers"] + if !ok { + t.Fatal("Outputs.previous_nameservers missing") + } + prevAny, _ := prevRaw.([]any) + if len(prevAny) != 2 { + t.Errorf("previous_nameservers len = %d, want 2", len(prevAny)) + } +} + +func TestDelegationDriver_Create_MissingDomain_Rejected(t *testing.T) { + d := NewDelegationDriverWithClient(&fakeDelegationClient{}) + spec := interfaces.ResourceSpec{ + Type: "infra.dns_delegation", + Config: map[string]any{ + "nameservers": []any{"a.com", "b.com"}, + }, + } + if _, err := d.Create(context.Background(), spec); err == nil { + t.Fatal("expected error for missing domain") + } +} + +func TestDelegationDriver_Create_MissingNameservers_Rejected(t *testing.T) { + d := NewDelegationDriverWithClient(&fakeDelegationClient{}) + spec := interfaces.ResourceSpec{ + Name: "example.com", + Type: "infra.dns_delegation", + Config: map[string]any{"domain": "example.com"}, + } + if _, err := d.Create(context.Background(), spec); err == nil { + t.Fatal("expected error for missing nameservers") + } +} + +func TestDelegationDriver_Create_DuplicateNameservers_Rejected(t *testing.T) { + d := NewDelegationDriverWithClient(&fakeDelegationClient{}) + spec := interfaces.ResourceSpec{ + Name: "example.com", + Type: "infra.dns_delegation", + Config: map[string]any{ + "domain": "example.com", + "nameservers": []any{"a.com", "a.com"}, + }, + } + if _, err := d.Create(context.Background(), spec); err == nil { + t.Fatal("expected error for duplicate nameservers") + } +} + +func TestDelegationDriver_Create_PreviousReadFails_Continues(t *testing.T) { + // Best-effort pre-change read: failure is non-fatal. + fc := &fakeDelegationClient{ + getErr: errors.New("read flaked"), + } + d := NewDelegationDriverWithClient(fc) + spec := interfaces.ResourceSpec{ + Name: "example.com", + Type: "infra.dns_delegation", + Config: map[string]any{ + "domain": "example.com", + "nameservers": []any{"a.com", "b.com"}, + }, + } + out, err := d.Create(context.Background(), spec) + if err != nil { + t.Fatalf("Create with read failure should still succeed: %v", err) + } + // previous_nameservers should be empty []any when read failed. + prev, _ := out.Outputs["previous_nameservers"].([]any) + if len(prev) != 0 { + t.Errorf("previous_nameservers = %v, want empty when pre-read failed", prev) + } +} +``` + +Add `"errors"` to the test imports if needed. + +**Step 2: Run tests to verify they fail** + +Run: `GOWORK=off go test ./internal/drivers -run TestDelegationDriver_Create -v` +Expected: FAIL — `undefined: d.Create` (or undefined parseDelegationSpec). + +**Step 3: Write minimal implementation** + +Append to `internal/drivers/delegation.go`: + +```go +// dnsDelegationSpec is the parsed config view. +type dnsDelegationSpec struct { + domain string + nameservers []string +} + +// parseDelegationSpec validates config and produces a typed view. +func parseDelegationSpec(spec interfaces.ResourceSpec) (dnsDelegationSpec, error) { + domain, _ := spec.Config["domain"].(string) + if domain == "" { + domain = spec.Name + } + if domain == "" { + return dnsDelegationSpec{}, fmt.Errorf("dns_delegation: config missing required key 'domain' (or spec.Name)") + } + rawNS, present := spec.Config["nameservers"] + if !present { + return dnsDelegationSpec{}, fmt.Errorf("dns_delegation %q: config missing required key 'nameservers'", domain) + } + nsList, ok := rawNS.([]any) + if !ok { + return dnsDelegationSpec{}, fmt.Errorf("dns_delegation %q: config 'nameservers' must be an array, got %T", domain, rawNS) + } + if len(nsList) < 1 { + return dnsDelegationSpec{}, fmt.Errorf("dns_delegation %q: config 'nameservers' must have ≥1 entry", domain) + } + seen := make(map[string]struct{}, len(nsList)) + parsed := make([]string, 0, len(nsList)) + for i, item := range nsList { + s, ok := item.(string) + if !ok { + return dnsDelegationSpec{}, fmt.Errorf("dns_delegation %q: nameservers[%d] must be a string, got %T", domain, i, item) + } + s = strings.TrimSpace(s) + if s == "" { + return dnsDelegationSpec{}, fmt.Errorf("dns_delegation %q: nameservers[%d] must be non-empty", domain, i) + } + if _, dup := seen[s]; dup { + return dnsDelegationSpec{}, fmt.Errorf("dns_delegation %q: nameservers[%d] = %q is a duplicate", domain, i, s) + } + seen[s] = struct{}{} + parsed = append(parsed, s) + } + return dnsDelegationSpec{domain: domain, nameservers: parsed}, nil +} + +// nameserversToAny converts []string to []any. Required for Outputs values +// to round-trip through structpb (typed slices are rejected; see iacserver.go +// package doc and the workspace structpb-boundary feedback memory). +func nameserversToAny(ns []string) []any { + out := make([]any, len(ns)) + for i, s := range ns { + out[i] = s + } + return out +} + +// delegationOutput builds the ResourceOutput for a Create/Update result. +// previous_nameservers may be nil; the helper converts to empty []any. +func delegationOutput(name, domain string, ns, previous []string) *interfaces.ResourceOutput { + if previous == nil { + previous = []string{} + } + return &interfaces.ResourceOutput{ + Name: name, + Type: "infra.dns_delegation", + ProviderID: domain, + Outputs: map[string]any{ + "domain": domain, + "nameservers": nameserversToAny(ns), + "previous_nameservers": nameserversToAny(previous), + }, + Status: "active", + } +} + +// Create stashes the current upstream nameservers (best-effort) as +// previous_nameservers, then PUTs the desired set. +func (d *DelegationDriver) Create(ctx context.Context, spec interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) { + if err := ctx.Err(); err != nil { + return nil, fmt.Errorf("dns_delegation create: %w", err) + } + s, err := parseDelegationSpec(spec) + if err != nil { + return nil, err + } + var previous []string + if dom, err := d.client.GetDomainDelegation(ctx, s.domain); err == nil && dom != nil { + previous = append([]string(nil), dom.Nameservers...) + } + // Best-effort: if GetDomainDelegation failed, previous remains nil → empty []any in output. + if err := ctx.Err(); err != nil { + return nil, fmt.Errorf("dns_delegation create %q: %w", s.domain, err) + } + if err := d.client.SetNameservers(ctx, s.domain, s.nameservers); err != nil { + return nil, fmt.Errorf("dns_delegation create %q: %w", s.domain, err) + } + return delegationOutput(spec.Name, s.domain, s.nameservers, previous), nil +} +``` + +**Step 4: Run tests to verify they pass** + +Run: `GOWORK=off go test ./internal/drivers -run TestDelegationDriver_Create -v` +Expected: PASS for all four Create subtests. + +**Step 5: Commit** + +```bash +git add internal/drivers/delegation.go internal/drivers/delegation_test.go +git commit -m "feat(drivers): DelegationDriver.Create + spec validation" +``` + +--- + +### Task 8: Implement `DelegationDriver.Read` + `Update` + `Delete` + +**Files:** +- Modify: `internal/drivers/delegation.go` + +**Step 1: Write the failing test** + +Append to `internal/drivers/delegation_test.go`: + +```go +func TestDelegationDriver_Read_HappyPath(t *testing.T) { + fc := &fakeDelegationClient{ + getResult: &hover.DomainDelegation{ + ID: "domain-example.com", + Name: "example.com", + Nameservers: []string{"ns1.do.com", "ns2.do.com"}, + }, + } + d := NewDelegationDriverWithClient(fc) + out, err := d.Read(context.Background(), interfaces.ResourceRef{Name: "example.com", ProviderID: "example.com"}) + if err != nil { + t.Fatalf("Read: %v", err) + } + if out.ProviderID != "example.com" { + t.Errorf("ProviderID = %q", out.ProviderID) + } + ns, _ := out.Outputs["nameservers"].([]any) + if len(ns) != 2 { + t.Errorf("nameservers len = %d", len(ns)) + } +} + +func TestDelegationDriver_Read_PropagatesError(t *testing.T) { + fc := &fakeDelegationClient{getErr: errors.New("API down")} + d := NewDelegationDriverWithClient(fc) + _, err := d.Read(context.Background(), interfaces.ResourceRef{Name: "x.com", ProviderID: "x.com"}) + if err == nil { + t.Fatal("expected error") + } +} + +func TestDelegationDriver_Update_HappyPath(t *testing.T) { + fc := &fakeDelegationClient{ + getResult: &hover.DomainDelegation{ + ID: "domain-example.com", Name: "example.com", + Nameservers: []string{"ns1.do.com", "ns2.do.com"}, + }, + } + d := NewDelegationDriverWithClient(fc) + ref := interfaces.ResourceRef{Name: "example.com", Type: "infra.dns_delegation", ProviderID: "example.com"} + spec := interfaces.ResourceSpec{ + Name: "example.com", Type: "infra.dns_delegation", + Config: map[string]any{ + "domain": "example.com", + "nameservers": []any{"ns3.do.com", "ns4.do.com"}, + }, + } + out, err := d.Update(context.Background(), ref, spec) + if err != nil { + t.Fatalf("Update: %v", err) + } + if fc.lastSetNS[0] != "ns3.do.com" { + t.Errorf("first NS = %q", fc.lastSetNS[0]) + } + _ = out +} + +func TestDelegationDriver_Update_DomainRenameRejected(t *testing.T) { + d := NewDelegationDriverWithClient(&fakeDelegationClient{}) + ref := interfaces.ResourceRef{Name: "old.com", ProviderID: "old.com"} + spec := interfaces.ResourceSpec{ + Name: "new.com", Type: "infra.dns_delegation", + Config: map[string]any{ + "domain": "new.com", + "nameservers": []any{"a.com", "b.com"}, + }, + } + if _, err := d.Update(context.Background(), ref, spec); err == nil { + t.Fatal("expected error rejecting domain rename") + } +} + +func TestDelegationDriver_Delete_RestoresPreviousNameservers(t *testing.T) { + fc := &fakeDelegationClient{} + d := NewDelegationDriverWithClient(fc) + ref := interfaces.ResourceRef{ + Name: "example.com", ProviderID: "example.com", + InputSnapshot: map[string]any{ + "previous_nameservers": []any{"old1.com", "old2.com"}, + }, + } + if err := d.Delete(context.Background(), ref); err != nil { + t.Fatalf("Delete: %v", err) + } + if len(fc.lastSetNS) != 2 || fc.lastSetNS[0] != "old1.com" { + t.Errorf("Delete set NS = %v, want [old1.com old2.com]", fc.lastSetNS) + } +} + +func TestDelegationDriver_Delete_FallbackHoverDefaults(t *testing.T) { + fc := &fakeDelegationClient{} + d := NewDelegationDriverWithClient(fc) + // No previous_nameservers in InputSnapshot. + ref := interfaces.ResourceRef{Name: "example.com", ProviderID: "example.com"} + if err := d.Delete(context.Background(), ref); err != nil { + t.Fatalf("Delete: %v", err) + } + if len(fc.lastSetNS) != 2 || fc.lastSetNS[0] != "ns1.hover.com" || fc.lastSetNS[1] != "ns2.hover.com" { + t.Errorf("Delete set NS = %v, want [ns1.hover.com ns2.hover.com] fallback", fc.lastSetNS) + } +} +``` + +Note: `interfaces.ResourceRef.InputSnapshot` may not exist on all versions of the workflow interface — check existing tests for the canonical way to pass state to Delete. If `InputSnapshot` isn't a field on `ResourceRef`, the test should use whatever channel the existing `DNSDriver.Delete` uses (likely the state file resolved by the engine before Delete is called). + +If `ResourceRef` doesn't have `InputSnapshot`, adjust the design: stash previous_nameservers in the resource's State backing rather than the ref. Inspect `/Users/jon/workspace/workflow/interfaces/iac_resource_driver.go` to confirm. + +**Step 2: Run tests to verify they fail** + +Run: `GOWORK=off go test ./internal/drivers -run "TestDelegationDriver_Read|TestDelegationDriver_Update|TestDelegationDriver_Delete" -v` +Expected: FAIL — methods not defined. + +**Step 3: Write minimal implementation** + +Append to `internal/drivers/delegation.go`: + +```go +// Read fetches the current registrar nameservers. +func (d *DelegationDriver) Read(ctx context.Context, ref interfaces.ResourceRef) (*interfaces.ResourceOutput, error) { + if err := ctx.Err(); err != nil { + return nil, fmt.Errorf("dns_delegation read %q: %w", ref.Name, err) + } + domain := ref.ProviderID + if domain == "" { + domain = ref.Name + } + dom, err := d.client.GetDomainDelegation(ctx, domain) + if err != nil { + return nil, fmt.Errorf("dns_delegation read %q: %w", ref.Name, err) + } + // Read does not populate previous_nameservers (state-only field + // captured at Create time). Leave as empty []any. + return delegationOutput(ref.Name, domain, dom.Nameservers, nil), nil +} + +// Update replaces the registrar nameservers. Rejects in-place domain +// renames (those must route through Diff → NeedsReplace → Delete-then-Create). +func (d *DelegationDriver) Update(ctx context.Context, ref interfaces.ResourceRef, spec interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) { + if err := ctx.Err(); err != nil { + return nil, fmt.Errorf("dns_delegation update %q: %w", ref.Name, err) + } + s, err := parseDelegationSpec(spec) + if err != nil { + return nil, err + } + currentDomain := ref.ProviderID + if currentDomain == "" { + currentDomain = ref.Name + } + if !strings.EqualFold(s.domain, currentDomain) { + return nil, fmt.Errorf("dns_delegation update %q: spec.domain %q does not match current %q — domain change requires resource replace, not update", ref.Name, s.domain, currentDomain) + } + if err := ctx.Err(); err != nil { + return nil, fmt.Errorf("dns_delegation update %q: %w", ref.Name, err) + } + if err := d.client.SetNameservers(ctx, currentDomain, s.nameservers); err != nil { + return nil, fmt.Errorf("dns_delegation update %q: %w", ref.Name, err) + } + return delegationOutput(ref.Name, currentDomain, s.nameservers, nil), nil +} + +// hoverDefaultNameservers is the fallback for Delete when previous_nameservers +// is not in the resource's input snapshot. Documented as best-effort in the +// design (assumption A5). +var hoverDefaultNameservers = []string{"ns1.hover.com", "ns2.hover.com"} + +// Delete restores the stashed previous_nameservers (or the Hover-default +// fallback). Caller's state must surface previous_nameservers via +// ref.InputSnapshot for the restore path to fire. +func (d *DelegationDriver) Delete(ctx context.Context, ref interfaces.ResourceRef) error { + if err := ctx.Err(); err != nil { + return fmt.Errorf("dns_delegation delete %q: %w", ref.Name, err) + } + domain := ref.ProviderID + if domain == "" { + domain = ref.Name + } + ns := hoverDefaultNameservers + if ref.InputSnapshot != nil { + if raw, ok := ref.InputSnapshot["previous_nameservers"]; ok { + if list, ok := raw.([]any); ok && len(list) > 0 { + restored := make([]string, 0, len(list)) + for _, item := range list { + if s, ok := item.(string); ok && s != "" { + restored = append(restored, s) + } + } + if len(restored) > 0 { + ns = restored + } + } + } + } + if err := d.client.SetNameservers(ctx, domain, ns); err != nil { + return fmt.Errorf("dns_delegation delete %q: %w", ref.Name, err) + } + return nil +} +``` + +If `interfaces.ResourceRef` does NOT have `InputSnapshot` (verify via `cat /Users/jon/workspace/workflow/interfaces/iac_resource_driver.go | grep -A5 "type ResourceRef"`), substitute the correct field name and adjust both the test and impl. If no such field exists at all, the implementer MUST pause and surface this as a design amendment — Delete cannot restore previous_nameservers without a state channel. + +**Step 4: Run tests to verify they pass** + +Run: `GOWORK=off go test ./internal/drivers -run "TestDelegationDriver_Read|TestDelegationDriver_Update|TestDelegationDriver_Delete" -v` +Expected: PASS for all six subtests. + +**Step 5: Commit** + +```bash +git add internal/drivers/delegation.go internal/drivers/delegation_test.go +git commit -m "feat(drivers): DelegationDriver.Read/Update/Delete" +``` + +--- + +### Task 9: Implement `DelegationDriver.Diff` (multiset compare + domain-rename Replace) + +**Files:** +- Modify: `internal/drivers/delegation.go` + +**Step 1: Write the failing test** + +Append to `internal/drivers/delegation_test.go`: + +```go +func TestDelegationDriver_Diff_NilCurrent(t *testing.T) { + d := NewDelegationDriverWithClient(&fakeDelegationClient{}) + spec := interfaces.ResourceSpec{ + Name: "example.com", Type: "infra.dns_delegation", + Config: map[string]any{ + "domain": "example.com", + "nameservers": []any{"a.com", "b.com"}, + }, + } + res, err := d.Diff(context.Background(), spec, nil) + if err != nil { + t.Fatalf("Diff: %v", err) + } + if !res.NeedsUpdate { + t.Error("expected NeedsUpdate=true for nil current") + } +} + +func TestDelegationDriver_Diff_UpToDate_OrderIndependent(t *testing.T) { + d := NewDelegationDriverWithClient(&fakeDelegationClient{}) + spec := interfaces.ResourceSpec{ + Name: "example.com", Type: "infra.dns_delegation", + Config: map[string]any{ + "domain": "example.com", + "nameservers": []any{"a.com", "b.com", "c.com"}, + }, + } + current := &interfaces.ResourceOutput{ + ProviderID: "example.com", + Outputs: map[string]any{ + "domain": "example.com", + "nameservers": []any{"c.com", "a.com", "b.com"}, // reversed + }, + } + res, err := d.Diff(context.Background(), spec, current) + if err != nil { + t.Fatalf("Diff: %v", err) + } + if res.NeedsUpdate { + t.Error("expected NeedsUpdate=false for same multiset") + } +} + +func TestDelegationDriver_Diff_Changed(t *testing.T) { + d := NewDelegationDriverWithClient(&fakeDelegationClient{}) + spec := interfaces.ResourceSpec{ + Name: "example.com", Type: "infra.dns_delegation", + Config: map[string]any{ + "domain": "example.com", + "nameservers": []any{"new.com", "b.com"}, + }, + } + current := &interfaces.ResourceOutput{ + ProviderID: "example.com", + Outputs: map[string]any{ + "domain": "example.com", + "nameservers": []any{"a.com", "b.com"}, + }, + } + res, err := d.Diff(context.Background(), spec, current) + if err != nil { + t.Fatalf("Diff: %v", err) + } + if !res.NeedsUpdate { + t.Error("expected NeedsUpdate=true") + } +} + +func TestDelegationDriver_Diff_DomainChange_NeedsReplace(t *testing.T) { + d := NewDelegationDriverWithClient(&fakeDelegationClient{}) + spec := interfaces.ResourceSpec{ + Name: "new.com", Type: "infra.dns_delegation", + Config: map[string]any{ + "domain": "new.com", + "nameservers": []any{"a.com", "b.com"}, + }, + } + current := &interfaces.ResourceOutput{ProviderID: "old.com"} + res, err := d.Diff(context.Background(), spec, current) + if err != nil { + t.Fatalf("Diff: %v", err) + } + if !res.NeedsReplace { + t.Error("expected NeedsReplace=true on domain change") + } + if len(res.Changes) != 1 || res.Changes[0].Path != "domain" || !res.Changes[0].ForceNew { + t.Errorf("expected ForceNew domain change, got %+v", res.Changes) + } +} +``` + +**Step 2: Run tests to verify they fail** + +Run: `GOWORK=off go test ./internal/drivers -run TestDelegationDriver_Diff -v` +Expected: FAIL — `d.Diff` not defined. + +**Step 3: Write minimal implementation** + +Append to `internal/drivers/delegation.go`: + +```go +// Diff compares desired vs current. Multiset semantics on nameservers +// (order-independent — Hover accepts any order on PUT). Domain rename +// (desired vs current.ProviderID) forces Replace. +func (d *DelegationDriver) Diff(_ context.Context, desired interfaces.ResourceSpec, current *interfaces.ResourceOutput) (*interfaces.DiffResult, error) { + s, err := parseDelegationSpec(desired) + if err != nil { + return nil, err + } + if current == nil { + return &interfaces.DiffResult{NeedsUpdate: true}, nil + } + if current.ProviderID != "" && !strings.EqualFold(s.domain, current.ProviderID) { + return &interfaces.DiffResult{ + NeedsReplace: true, + Changes: []interfaces.FieldChange{{ + Path: "domain", + Old: current.ProviderID, + New: s.domain, + ForceNew: true, + }}, + }, nil + } + currentNS := nameserversFromOutputs(current.Outputs) + if !sameNameserverSet(currentNS, s.nameservers) { + return &interfaces.DiffResult{ + NeedsUpdate: true, + Changes: []interfaces.FieldChange{{ + Path: "nameservers", + Old: nameserversToAny(currentNS), + New: nameserversToAny(s.nameservers), + }}, + }, nil + } + return &interfaces.DiffResult{NeedsUpdate: false}, nil +} + +// nameserversFromOutputs reconstructs []string from Outputs["nameservers"] +// (which is stored as []any). +func nameserversFromOutputs(outputs map[string]any) []string { + raw, ok := outputs["nameservers"] + if !ok { + return nil + } + list, ok := raw.([]any) + if !ok { + return nil + } + out := make([]string, 0, len(list)) + for _, item := range list { + if s, ok := item.(string); ok { + out = append(out, s) + } + } + return out +} + +// sameNameserverSet returns true iff a and b are multiset-equal. +func sameNameserverSet(a, b []string) bool { + if len(a) != len(b) { + return false + } + sa := append([]string(nil), a...) + sb := append([]string(nil), b...) + sort.Strings(sa) + sort.Strings(sb) + for i := range sa { + if !strings.EqualFold(sa[i], sb[i]) { + return false + } + } + return true +} +``` + +**Step 4: Run tests to verify they pass** + +Run: `GOWORK=off go test ./internal/drivers -run TestDelegationDriver_Diff -v` +Expected: PASS for all four subtests. + +**Step 5: Commit** + +```bash +git add internal/drivers/delegation.go internal/drivers/delegation_test.go +git commit -m "feat(drivers): DelegationDriver.Diff (multiset + Replace)" +``` + +--- + +### Task 10: Implement `DelegationDriver.HealthCheck` + `Scale` + +**Files:** +- Modify: `internal/drivers/delegation.go` + +**Step 1: Write the failing test** + +Append to `internal/drivers/delegation_test.go`: + +```go +func TestDelegationDriver_HealthCheck_Healthy(t *testing.T) { + fc := &fakeDelegationClient{ + getResult: &hover.DomainDelegation{ + ID: "domain-example.com", Name: "example.com", + Nameservers: []string{"a.com", "b.com"}, + }, + } + d := NewDelegationDriverWithClient(fc) + res, err := d.HealthCheck(context.Background(), interfaces.ResourceRef{Name: "example.com", ProviderID: "example.com"}) + if err != nil { + t.Fatalf("HealthCheck: %v", err) + } + if !res.Healthy { + t.Errorf("Healthy = false, want true") + } +} + +func TestDelegationDriver_HealthCheck_Unhealthy(t *testing.T) { + fc := &fakeDelegationClient{getErr: errors.New("boom")} + d := NewDelegationDriverWithClient(fc) + res, err := d.HealthCheck(context.Background(), interfaces.ResourceRef{Name: "example.com", ProviderID: "example.com"}) + if err != nil { + t.Fatalf("HealthCheck should not return err; got %v", err) + } + if res.Healthy { + t.Error("Healthy = true, want false") + } +} + +func TestDelegationDriver_Scale_NotSupported(t *testing.T) { + d := NewDelegationDriverWithClient(&fakeDelegationClient{}) + _, err := d.Scale(context.Background(), interfaces.ResourceRef{Name: "x"}, 3) + if err == nil { + t.Fatal("expected error from Scale") + } +} + +func TestDelegationDriver_CtxCanceled_AllMethods(t *testing.T) { + d := NewDelegationDriverWithClient(&fakeDelegationClient{}) + ref := interfaces.ResourceRef{Name: "example.com", ProviderID: "example.com"} + spec := interfaces.ResourceSpec{ + Name: "example.com", Type: "infra.dns_delegation", + Config: map[string]any{ + "domain": "example.com", + "nameservers": []any{"a.com", "b.com"}, + }, + } + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := d.Create(ctx, spec); err == nil { + t.Error("Create: expected error for canceled ctx") + } + if _, err := d.Read(ctx, ref); err == nil { + t.Error("Read: expected error for canceled ctx") + } + if _, err := d.Update(ctx, ref, spec); err == nil { + t.Error("Update: expected error for canceled ctx") + } + if err := d.Delete(ctx, ref); err == nil { + t.Error("Delete: expected error for canceled ctx") + } +} +``` + +**Step 2: Run tests to verify they fail** + +Run: `GOWORK=off go test ./internal/drivers -run "TestDelegationDriver_HealthCheck|TestDelegationDriver_Scale|TestDelegationDriver_CtxCanceled" -v` +Expected: FAIL — methods not defined. + +**Step 3: Write minimal implementation** + +Append to `internal/drivers/delegation.go`: + +```go +// HealthCheck probes connectivity to the domain by fetching its delegation. +func (d *DelegationDriver) HealthCheck(ctx context.Context, ref interfaces.ResourceRef) (*interfaces.HealthResult, error) { + if err := ctx.Err(); err != nil { + return &interfaces.HealthResult{Healthy: false, Message: err.Error()}, nil + } + domain := ref.ProviderID + if domain == "" { + domain = ref.Name + } + if _, err := d.client.GetDomainDelegation(ctx, domain); err != nil { + return &interfaces.HealthResult{Healthy: false, Message: err.Error()}, nil + } + return &interfaces.HealthResult{Healthy: true, Message: "ok"}, nil +} + +// Scale is not supported for DNS delegation (no replica concept). +func (d *DelegationDriver) Scale(_ context.Context, _ interfaces.ResourceRef, _ int) (*interfaces.ResourceOutput, error) { + return nil, fmt.Errorf("dns_delegation: scale is not supported") +} +``` + +**Step 4: Run tests to verify they pass** + +Run: `GOWORK=off go test ./internal/drivers -count=1 -v 2>&1 | tail -10` +Expected: ALL delegation tests pass. + +**Step 5: Commit** + +```bash +git add internal/drivers/delegation.go internal/drivers/delegation_test.go +git commit -m "feat(drivers): DelegationDriver.HealthCheck + Scale + ctx tests" +``` + +--- + +### Task 11: Wire `DelegationDriver` into `HoverProvider` + +**Files:** +- Modify: `internal/provider.go:22` (type comment) + `:84-87` (drivers map) + `:90-100` (Capabilities) + +**Step 1: Write the failing test** + +Add to `internal/iacserver_test.go` (or wherever iacserver-level capability assertions live; check existing tests for the canonical place): + +```go +func TestHoverIaCServer_Capabilities_IncludesDelegation(t *testing.T) { + // Smoke: Capabilities() must list both infra.dns and infra.dns_delegation. + // Tests should bring up the iacserver via the existing test harness; + // see iacserver_test.go for the canonical setup. + // Pseudocode (adapt to existing test harness): + caps := newTestIaCServerInitialized(t).Capabilities() + wantTypes := map[string]bool{"infra.dns": false, "infra.dns_delegation": false} + for _, c := range caps.Capabilities { + if _, ok := wantTypes[c.ResourceType]; ok { + wantTypes[c.ResourceType] = true + } + } + for rt, found := range wantTypes { + if !found { + t.Errorf("Capabilities missing %q", rt) + } + } +} +``` + +(The implementer must adapt this to the actual existing `iacserver_test.go` harness pattern. Check the existing capabilities test there for the canonical assertion shape.) + +**Step 2: Run tests to verify they fail** + +Run: `GOWORK=off go test ./internal -count=1 -run "Capabilities" -v` +Expected: FAIL or PASS-without-new-type — the new resource type is not yet registered. + +**Step 3: Write minimal implementation** + +Edit `internal/provider.go`: + +1. Update the type comment at line 21-22: + +```go +// HoverProvider implements interfaces.IaCProvider for Hover. +// Supports two resource types: +// - infra.dns — DNS records within Hover's nameservers. +// - infra.dns_delegation — registrar-level nameserver delegation. +``` + +2. In `Initialize` (line 84), add the delegation driver to the map: + +```go + p.drivers = map[string]interfaces.ResourceDriver{ + "infra.dns": drivers.NewDNSDriver(c), + "infra.dns_delegation": drivers.NewDelegationDriver(c), + } +``` + +3. In `Capabilities` (line 90-100), append the second capability: + +```go +func (p *HoverProvider) Capabilities() []interfaces.IaCCapabilityDeclaration { + return []interfaces.IaCCapabilityDeclaration{ + { + ResourceType: "infra.dns", + Tier: 1, + Operations: []string{"create", "read", "update", "delete"}, + }, + { + ResourceType: "infra.dns_delegation", + Tier: 1, + Operations: []string{"create", "read", "update", "delete"}, + }, + } +} +``` + +**Step 4: Run tests to verify they pass** + +Run: `GOWORK=off go test ./internal -count=1` +Expected: PASS for capabilities + all existing tests. + +**Step 5: Commit** + +```bash +git add internal/provider.go internal/iacserver_test.go +git commit -m "feat(provider): register DelegationDriver + update Capabilities" +``` + +--- + +### Task 12: Update `plugin.json` to declare `infra.dns_delegation` + +**Files:** +- Modify: `plugin.json` + +**Step 1: Read current state** + +```bash +cat plugin.json +``` + +**Step 2: Edit** + +Update `plugin.json` so `iacProvider.resourceTypes` lists both: + +```json +{ + ... + "iacProvider": { + "computePlanVersion": "v2", + "resourceTypes": ["infra.dns", "infra.dns_delegation"] + }, + ... +} +``` + +If `resourceTypes` is missing entirely, add it. Otherwise extend the array. + +**Step 3: Verify JSON parses** + +Run: `jq . plugin.json > /dev/null && echo "OK"` +Expected: `OK` (no JSON syntax errors). + +**Step 4: Run full build + test** + +Run: `GOWORK=off go build ./... && GOWORK=off go test ./... -count=1 -timeout 120s 2>&1 | tail -5` +Expected: all packages PASS. + +**Step 5: Commit** + +```bash +git add plugin.json +git commit -m "feat: plugin.json declares infra.dns_delegation" +``` + +--- + +### Task 13: Run full validation + open PR + +**Files:** none (verification + PR open) + +**Step 1: Full validation gate** + +```bash +gofmt -l . 2>&1 | head # expect empty +GOWORK=off go vet ./... 2>&1 | head # expect empty +GOWORK=off go build ./... 2>&1 | head # expect empty +GOWORK=off go test ./... -count=1 -timeout 120s 2>&1 | tail -10 # expect all OK +``` + +Expected: every command emits no errors. If any fail, fix before proceeding. + +**Step 2: Push + open PR** + +```bash +git push -u origin feat/dns-delegation +gh pr create --title "feat: infra.dns_delegation resource type (v0.2.0)" --body "$(cat <<'BODY' +## Summary + +Adds the `infra.dns_delegation` resource type so wfctl can set a domain's registrar-level nameservers via the Hover control-panel API. Field-test target: gocodealone.tech → ns1/2/3.digitalocean.com. + +Design + 3-round adversarial review history: \`docs/plans/2026-05-20-hover-dns-delegation-design.md\`. +Implementation plan: \`docs/plans/2026-05-20-hover-dns-delegation.md\`. + +## Endpoint + +Captured from the Hover web UI (2026-05-20): + +\`\`\` +PUT /api/control_panel/domains/domain- +Content-Type: application/json +X-CSRF-Token: + +{"field":"nameservers","value":["ns1.digitalocean.com",...]} +\`\`\` + +## What changed + +- New \`DomainDelegation\` type (distinct from \`Domain\` to avoid endpoint-shape ambiguity). +- New \`ErrEmptyNameservers\` sentinel — loud failure on empty Read instead of silent re-apply thrash. +- \`SetNameservers\` holds \`c.mu\` across the full auth → CSRF → PUT sequence (no TOCTOU). +- New \`GetDomainDelegation\` using the same API family as the PUT (control-panel endpoint). +- New \`DelegationDriver\` registered as \`infra.dns_delegation\` alongside the existing \`infra.dns\` driver. +- \`plugin.json\` declares the new resource type. +- Outputs structpb-safe (\`[]any\` not \`[]string\`). +- Delete stashes \`previous_nameservers\` at Create for restore; falls back to \`[ns1.hover.com, ns2.hover.com]\` for state-less resources. + +## Follow-ups (deferred to a separate session) + +1. workflow-registry manifest bump to v0.2.0 (cannot author until goreleaser publishes asset SHAs after this PR merges + the v0.2.0 tag fires). +2. gocodealone-multisite: \`config/dns.wfctl.yaml\` + \`.github/workflows/dns-delegation.yml\` (manual workflow_dispatch); operator runs apply against gocodealone.tech. + +## Test plan + +- [x] \`gofmt -l .\` clean +- [x] \`GOWORK=off go vet ./...\` clean +- [x] \`GOWORK=off go build ./...\` clean +- [x] \`GOWORK=off go test ./... -count=1\` all PASS +- [ ] Field test: \`wfctl apply\` in GHA against gocodealone.tech; verify Hover UI shows DO nameservers post-apply. + +🤖 Generated with [Claude Code](https://claude.com/claude-code) +BODY +)" 2>&1 | tail -2 +``` + +**Step 3: Request Copilot review** + +```bash +gh pr edit $(gh pr view --json number -q .number) --repo GoCodeAlone/workflow-plugin-hover --add-reviewer @copilot +``` + +**Step 4: Verify PR open + Copilot triggered** + +Run: `gh pr view --repo GoCodeAlone/workflow-plugin-hover --json state,number,reviewRequests` +Expected: `state: OPEN`; `reviewRequests` contains the Copilot bot (or `reviews` will populate it within ~60s). + +**Step 5: Commit if any final cleanup needed** + +If the PR creation requires any last-minute fix (e.g., a forgotten file), commit + push. Otherwise no commit needed. + +--- + +## Rollback notes + +Per the design's Rollback section. Per-task rollback: + +- **Task 1-5 (client extensions)**: Revert commit; restores original `client.go`. Backwards-compatible additive changes; no consumer depends on the new symbols yet. +- **Task 6-10 (driver)**: Revert commits. No external consumers reference `DelegationDriver`. +- **Task 11 (provider wiring)**: Revert commit; restores single-driver map. wfctl that previously loaded the plugin sees only `infra.dns` again. +- **Task 12 (plugin.json)**: Revert; `iacProvider.resourceTypes` returns to single-type. wfctl's manifest validation flips back to passing. +- **Task 13 (PR)**: `gh pr close ` if needed. Branch deletion via `--delete-branch` on close. + +Full revert pre-merge = `git push origin --delete feat/dns-delegation`. + +Post-merge rollback = revert PR + cherry-pick the revert + new v0.2.1 tag. From 819c8c3c59d82dfbec665ae602a6f13fe8a30042 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 20:50:31 -0400 Subject: [PATCH 06/20] docs: plan revision per adversarial review round 1 Addresses 1 Critical + 3 Important + 3 Minor: CRITICAL: ResourceRef has no InputSnapshot field (verified workflow/interfaces/iac_provider.go:183-187). Adopted reviewer option #2: v0.2.0 ships Delete = always reset to Hover defaults [ns1.hover.com, ns2.hover.com]. Stash-and-restore deferred to v0.3.0 follow-up requiring interfaces change. Design + plan updated; previous_nameservers dropped from Outputs entirely. IMPORTANT: Task 3 newTestClient/rewritingTransport conflicted with existing newStubClient/rewriteTransport helpers. Rewrote to use the existing helpers. IMPORTANT: Task 2 cited line numbers that shift after Task 1. Switched to function-name references throughout. IMPORTANT: Task 11 test used nonexistent newTestIaCServerInitialized helper. Replaced with provider-level TestHoverProvider_Capabilities_ IncludesDelegation against HoverProvider.Capabilities() directly (pure function; no gRPC harness needed). MINOR: Added []string-vs-[]any clarifying comment to putNameserversLocked. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../2026-05-20-hover-dns-delegation-design.md | 8 +- docs/plans/2026-05-20-hover-dns-delegation.md | 236 ++++++------------ 2 files changed, 83 insertions(+), 161 deletions(-) diff --git a/docs/plans/2026-05-20-hover-dns-delegation-design.md b/docs/plans/2026-05-20-hover-dns-delegation-design.md index 6f6b38a..c0b8935 100644 --- a/docs/plans/2026-05-20-hover-dns-delegation-design.md +++ b/docs/plans/2026-05-20-hover-dns-delegation-design.md @@ -100,10 +100,10 @@ This genuinely eliminates the race window: no other goroutine can interleave bet Validation: `≥1 nameserver` (Hover may accept single-NS setups; not the plugin's place to over-validate). Duplicates rejected as a config-author bug. Each nameserver string non-empty. - **Outputs encoding (structpb-safe)**: `Outputs["nameservers"]` is stored as `[]any` of `string`, NOT `[]string`. Per the structpb boundary invariant (see `iacserver.go` package doc; `feedback_workflow_plugin_structpb_boundary` workspace memory). Helper `nameserversToAny(ns []string) []any` converts. Round-trip test through `json.Marshal` + `json.Unmarshal` into `map[string]any` confirms the type survives. - Lifecycle: - - **Create** — validate, capture pre-change nameservers via `client.GetDomainDelegation` (best-effort; non-fatal if it fails) and stash them in `Outputs["previous_nameservers"]`, then `client.SetNameservers`. Return outputs `{domain, nameservers, previous_nameservers}`. - - **Update** — validate, call `client.SetNameservers`. Returns outputs from desired (no read-after-write, per namecheap round-4 pattern). Preserves any existing `previous_nameservers` from prior state if available. - - **Read** — `client.GetDomainDelegation(ctx, domain)` → return `{domain, nameservers}` as `ResourceOutput`. Does NOT populate `previous_nameservers` from upstream (it's a state-only field captured at Create). - - **Delete** — read `previous_nameservers` from the resource's input snapshot if present; otherwise fall back to `[ns1.hover.com, ns2.hover.com]` with a logged warning. PUT via `client.SetNameservers`. This is the rollback path: the resource owns the registrar state it took over and restores what was there before, with a documented hardcoded fallback for state-less or pre-v0.2.0 resources. + - **Create** — validate, call `client.SetNameservers`. Return outputs from desired set (`{domain, nameservers}`). No pre-read. + - **Update** — validate, call `client.SetNameservers`. Returns outputs from desired (no read-after-write, per namecheap round-4 pattern). + - **Read** — `client.GetDomainDelegation(ctx, domain)` → return `{domain, nameservers}` as `ResourceOutput`. + - **Delete** — PUT Hover's default nameservers (`[ns1.hover.com, ns2.hover.com]`) via `client.SetNameservers`. The state-stash-and-restore approach was dropped for v0.2.0: `interfaces.ResourceRef` (verified `workflow/interfaces/iac_provider.go:183`) has fields `{Name, Type, ProviderID}` only — no `InputSnapshot` channel. Restoring the pre-Create nameservers would require either a wfctl engine change OR a separate state-channel design. Deferred to v0.3.0 follow-up. The hardcoded `[ns1.hover.com, ns2.hover.com]` fallback per A5 is the only Delete path in v0.2.0; operators of domains with non-default originals must restore manually via Hover's UI if a Delete fires unintended. - **Diff** — multiset comparison of `current.Outputs["nameservers"]` (`[]any` decoded back to `[]string`) vs desired. Order-independent. Domain rename (desired vs `current.ProviderID`) → `NeedsReplace=true` with a `ForceNew` change. - **HealthCheck** — `GetDomainDelegation` success/failure. - **Scale** — no-op error (DNS delegation has no replicas). diff --git a/docs/plans/2026-05-20-hover-dns-delegation.md b/docs/plans/2026-05-20-hover-dns-delegation.md index 55c0f73..839c9da 100644 --- a/docs/plans/2026-05-20-hover-dns-delegation.md +++ b/docs/plans/2026-05-20-hover-dns-delegation.md @@ -119,7 +119,7 @@ git commit -m "feat(hover): DomainDelegation type + ErrEmptyNameservers sentinel ### Task 2: Refactor `ensureLogin` into `ensureLogin` + `ensureLoginLocked` **Files:** -- Modify: `internal/hover/client.go:82-133` — split lock-acquisition from body +- Modify: `internal/hover/client.go` — split `func (c *Client) ensureLogin` into a thin lock-acquiring wrapper + a `*Locked` body (function name reference; line numbers shift across tasks) **Step 1: Write the failing test** @@ -257,41 +257,28 @@ func TestFetchControlPanelCSRFLocked_Non2xx(t *testing.T) { } ``` -If `newTestClient` doesn't exist yet, add a helper (still in `client_test.go`): +**Use the existing helpers.** `internal/hover/client_test.go` already provides: +- `func newStubClient(t *testing.T, handler http.HandlerFunc) (*Client, *httptest.Server)` — builds a Client with cookie jar + `rewriteTransport`. +- `type rewriteTransport struct{ base string }` + `RoundTrip` — rewrites all outbound URLs to the httptest server base. -```go -// newTestClient builds a Client that points at a httptest server URL by -// monkey-patching hoverHost via a package-level test override. -func newTestClient(t *testing.T, baseURL string) *Client { - t.Helper() - // Use a fresh Client and point its http.Client at the test server via - // a custom transport that rewrites the request URL host. - c, err := NewClient(Credentials{Username: "u", Password: "p"}, &http.Client{ - Transport: &rewritingTransport{base: baseURL}, - Timeout: 5 * time.Second, - }) - if err != nil { - t.Fatalf("NewClient: %v", err) - } - c.loggedAt = time.Now() // skip login - return c -} +The tests above should be authored against `newStubClient` (passing a `http.HandlerFunc` that dispatches by path), NOT a new `newTestClient` helper. Do NOT introduce a duplicate `rewritingTransport` struct — it would collide with the existing `rewriteTransport`. -type rewritingTransport struct{ base string } +Pattern (replaces all `newTestClient(t, srv.URL)` and `httptest.NewServer(...)` references in this and subsequent tasks): -func (rt *rewritingTransport) RoundTrip(req *http.Request) (*http.Response, error) { - target, err := url.Parse(rt.base + req.URL.Path) - if err != nil { - return nil, err - } - target.RawQuery = req.URL.RawQuery - req.URL = target - req.Host = target.Host - return http.DefaultTransport.RoundTrip(req) -} +```go +c, srv := newStubClient(t, func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/control_panel/domain/example.com": + _, _ = w.Write([]byte(``)) + default: + t.Errorf("unexpected path: %s", r.URL.Path) + } +}) +defer srv.Close() +c.loggedAt = time.Now() // skip login (newStubClient builds a fresh client) ``` -If a similar helper already exists in `internal/hover/client_test.go` (likely — check existing tests), reuse it. +Re-author each `TestFetchControlPanelCSRFLocked_*` (and all subsequent task tests) using this `newStubClient` shape; the assertions on URL, headers, body content remain identical. **Step 2: Run tests to verify they fail** @@ -580,6 +567,12 @@ func (c *Client) SetNameservers(ctx context.Context, domainName string, ns []str } // putNameserversLocked PUTs the nameservers list. Caller MUST hold c.mu. +// +// Note: the wire payload uses []string directly — encoding/json serializes +// it as a JSON array, which is what Hover expects. This is distinct from +// the []any requirement in ResourceOutput.Outputs (which crosses the +// structpb gRPC boundary); typed slices are fine here because the wire +// format is plain JSON, not structpb. func (c *Client) putNameserversLocked(ctx context.Context, domainName string, ns []string, csrf string) error { endpoint := fmt.Sprintf("%s/api/control_panel/domains/domain-%s", hoverHost, url.PathEscape(domainName)) payload := map[string]any{"field": "nameservers", "value": ns} @@ -753,6 +746,8 @@ git commit -m "feat(drivers): DelegationDriver skeleton + interface" ### Task 7: Implement `DelegationDriver.Create` + `parseDelegationSpec` + `nameserversToAny` +**Note re: state-restore for Delete:** `interfaces.ResourceRef` (`workflow/interfaces/iac_provider.go:183-187`) has only `{Name, Type, ProviderID}` fields — no channel for the engine to pass last-applied Outputs into Delete. The original design's "stash previous_nameservers at Create / restore at Delete" approach is structurally impossible against the current interface. **v0.2.0 ships Delete = always reset to `[ns1.hover.com, ns2.hover.com]` (the documented A5 fallback).** The stash-and-restore enhancement is deferred to a v0.3.0 follow-up that would require an interfaces change. Outputs therefore omit `previous_nameservers` entirely. + **Files:** - Modify: `internal/drivers/delegation.go` @@ -762,13 +757,7 @@ Append to `internal/drivers/delegation_test.go`: ```go func TestDelegationDriver_Create_CallsSetNameservers(t *testing.T) { - fc := &fakeDelegationClient{ - getResult: &hover.DomainDelegation{ - ID: "domain-example.com", - Name: "example.com", - Nameservers: []string{"ns1.hover.com", "ns2.hover.com"}, - }, - } + fc := &fakeDelegationClient{} d := NewDelegationDriverWithClient(fc) spec := interfaces.ResourceSpec{ Name: "example.com", @@ -800,14 +789,9 @@ func TestDelegationDriver_Create_CallsSetNameservers(t *testing.T) { if len(nsAny) != 3 { t.Errorf("Outputs.nameservers len = %d, want 3", len(nsAny)) } - // previous_nameservers stashed from Hover defaults - prevRaw, ok := out.Outputs["previous_nameservers"] - if !ok { - t.Fatal("Outputs.previous_nameservers missing") - } - prevAny, _ := prevRaw.([]any) - if len(prevAny) != 2 { - t.Errorf("previous_nameservers len = %d, want 2", len(prevAny)) + // previous_nameservers NOT in Outputs for v0.2.0 (no state channel). + if _, present := out.Outputs["previous_nameservers"]; present { + t.Errorf("v0.2.0 Outputs should not contain previous_nameservers") } } @@ -851,33 +835,9 @@ func TestDelegationDriver_Create_DuplicateNameservers_Rejected(t *testing.T) { } } -func TestDelegationDriver_Create_PreviousReadFails_Continues(t *testing.T) { - // Best-effort pre-change read: failure is non-fatal. - fc := &fakeDelegationClient{ - getErr: errors.New("read flaked"), - } - d := NewDelegationDriverWithClient(fc) - spec := interfaces.ResourceSpec{ - Name: "example.com", - Type: "infra.dns_delegation", - Config: map[string]any{ - "domain": "example.com", - "nameservers": []any{"a.com", "b.com"}, - }, - } - out, err := d.Create(context.Background(), spec) - if err != nil { - t.Fatalf("Create with read failure should still succeed: %v", err) - } - // previous_nameservers should be empty []any when read failed. - prev, _ := out.Outputs["previous_nameservers"].([]any) - if len(prev) != 0 { - t.Errorf("previous_nameservers = %v, want empty when pre-read failed", prev) - } -} ``` -Add `"errors"` to the test imports if needed. +No errors import needed for Task 7. **Step 2: Run tests to verify they fail** @@ -947,26 +907,23 @@ func nameserversToAny(ns []string) []any { } // delegationOutput builds the ResourceOutput for a Create/Update result. -// previous_nameservers may be nil; the helper converts to empty []any. -func delegationOutput(name, domain string, ns, previous []string) *interfaces.ResourceOutput { - if previous == nil { - previous = []string{} - } +// v0.2.0 ships without previous_nameservers (no state channel in +// interfaces.ResourceRef; v0.3.0 follow-up). +func delegationOutput(name, domain string, ns []string) *interfaces.ResourceOutput { return &interfaces.ResourceOutput{ Name: name, Type: "infra.dns_delegation", ProviderID: domain, Outputs: map[string]any{ - "domain": domain, - "nameservers": nameserversToAny(ns), - "previous_nameservers": nameserversToAny(previous), + "domain": domain, + "nameservers": nameserversToAny(ns), }, Status: "active", } } -// Create stashes the current upstream nameservers (best-effort) as -// previous_nameservers, then PUTs the desired set. +// Create PUTs the desired nameservers. Output built from the desired set +// (no read-after-write); SetNameservers is authoritative on success. func (d *DelegationDriver) Create(ctx context.Context, spec interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) { if err := ctx.Err(); err != nil { return nil, fmt.Errorf("dns_delegation create: %w", err) @@ -975,18 +932,13 @@ func (d *DelegationDriver) Create(ctx context.Context, spec interfaces.ResourceS if err != nil { return nil, err } - var previous []string - if dom, err := d.client.GetDomainDelegation(ctx, s.domain); err == nil && dom != nil { - previous = append([]string(nil), dom.Nameservers...) - } - // Best-effort: if GetDomainDelegation failed, previous remains nil → empty []any in output. if err := ctx.Err(); err != nil { return nil, fmt.Errorf("dns_delegation create %q: %w", s.domain, err) } if err := d.client.SetNameservers(ctx, s.domain, s.nameservers); err != nil { return nil, fmt.Errorf("dns_delegation create %q: %w", s.domain, err) } - return delegationOutput(spec.Name, s.domain, s.nameservers, previous), nil + return delegationOutput(spec.Name, s.domain, s.nameservers), nil } ``` @@ -1086,41 +1038,24 @@ func TestDelegationDriver_Update_DomainRenameRejected(t *testing.T) { } } -func TestDelegationDriver_Delete_RestoresPreviousNameservers(t *testing.T) { - fc := &fakeDelegationClient{} - d := NewDelegationDriverWithClient(fc) - ref := interfaces.ResourceRef{ - Name: "example.com", ProviderID: "example.com", - InputSnapshot: map[string]any{ - "previous_nameservers": []any{"old1.com", "old2.com"}, - }, - } - if err := d.Delete(context.Background(), ref); err != nil { - t.Fatalf("Delete: %v", err) - } - if len(fc.lastSetNS) != 2 || fc.lastSetNS[0] != "old1.com" { - t.Errorf("Delete set NS = %v, want [old1.com old2.com]", fc.lastSetNS) - } -} - -func TestDelegationDriver_Delete_FallbackHoverDefaults(t *testing.T) { +func TestDelegationDriver_Delete_ResetsToHoverDefaults(t *testing.T) { + // v0.2.0 ships fallback-only Delete: ResourceRef has no state + // channel (verified: workflow/interfaces/iac_provider.go:183-187 + // defines ResourceRef as {Name, Type, ProviderID}). Restore from + // stashed previous_nameservers is a v0.3.0 follow-up requiring + // an interfaces change. fc := &fakeDelegationClient{} d := NewDelegationDriverWithClient(fc) - // No previous_nameservers in InputSnapshot. ref := interfaces.ResourceRef{Name: "example.com", ProviderID: "example.com"} if err := d.Delete(context.Background(), ref); err != nil { t.Fatalf("Delete: %v", err) } if len(fc.lastSetNS) != 2 || fc.lastSetNS[0] != "ns1.hover.com" || fc.lastSetNS[1] != "ns2.hover.com" { - t.Errorf("Delete set NS = %v, want [ns1.hover.com ns2.hover.com] fallback", fc.lastSetNS) + t.Errorf("Delete set NS = %v, want [ns1.hover.com ns2.hover.com]", fc.lastSetNS) } } ``` -Note: `interfaces.ResourceRef.InputSnapshot` may not exist on all versions of the workflow interface — check existing tests for the canonical way to pass state to Delete. If `InputSnapshot` isn't a field on `ResourceRef`, the test should use whatever channel the existing `DNSDriver.Delete` uses (likely the state file resolved by the engine before Delete is called). - -If `ResourceRef` doesn't have `InputSnapshot`, adjust the design: stash previous_nameservers in the resource's State backing rather than the ref. Inspect `/Users/jon/workspace/workflow/interfaces/iac_resource_driver.go` to confirm. - **Step 2: Run tests to verify they fail** Run: `GOWORK=off go test ./internal/drivers -run "TestDelegationDriver_Read|TestDelegationDriver_Update|TestDelegationDriver_Delete" -v` @@ -1144,9 +1079,7 @@ func (d *DelegationDriver) Read(ctx context.Context, ref interfaces.ResourceRef) if err != nil { return nil, fmt.Errorf("dns_delegation read %q: %w", ref.Name, err) } - // Read does not populate previous_nameservers (state-only field - // captured at Create time). Leave as empty []any. - return delegationOutput(ref.Name, domain, dom.Nameservers, nil), nil + return delegationOutput(ref.Name, domain, dom.Nameservers), nil } // Update replaces the registrar nameservers. Rejects in-place domain @@ -1172,17 +1105,17 @@ func (d *DelegationDriver) Update(ctx context.Context, ref interfaces.ResourceRe if err := d.client.SetNameservers(ctx, currentDomain, s.nameservers); err != nil { return nil, fmt.Errorf("dns_delegation update %q: %w", ref.Name, err) } - return delegationOutput(ref.Name, currentDomain, s.nameservers, nil), nil + return delegationOutput(ref.Name, currentDomain, s.nameservers), nil } -// hoverDefaultNameservers is the fallback for Delete when previous_nameservers -// is not in the resource's input snapshot. Documented as best-effort in the -// design (assumption A5). +// hoverDefaultNameservers is the Delete target for v0.2.0 (per A5). +// ResourceRef has no state channel for previous_nameservers restore; +// that enhancement is v0.3.0 follow-up territory. var hoverDefaultNameservers = []string{"ns1.hover.com", "ns2.hover.com"} -// Delete restores the stashed previous_nameservers (or the Hover-default -// fallback). Caller's state must surface previous_nameservers via -// ref.InputSnapshot for the restore path to fire. +// Delete resets the registrar nameservers to Hover's defaults. +// Operators whose domains had non-default originals must restore +// manually via the Hover UI if a Delete fires unintended. func (d *DelegationDriver) Delete(ctx context.Context, ref interfaces.ResourceRef) error { if err := ctx.Err(); err != nil { return fmt.Errorf("dns_delegation delete %q: %w", ref.Name, err) @@ -1191,31 +1124,13 @@ func (d *DelegationDriver) Delete(ctx context.Context, ref interfaces.ResourceRe if domain == "" { domain = ref.Name } - ns := hoverDefaultNameservers - if ref.InputSnapshot != nil { - if raw, ok := ref.InputSnapshot["previous_nameservers"]; ok { - if list, ok := raw.([]any); ok && len(list) > 0 { - restored := make([]string, 0, len(list)) - for _, item := range list { - if s, ok := item.(string); ok && s != "" { - restored = append(restored, s) - } - } - if len(restored) > 0 { - ns = restored - } - } - } - } - if err := d.client.SetNameservers(ctx, domain, ns); err != nil { + if err := d.client.SetNameservers(ctx, domain, hoverDefaultNameservers); err != nil { return fmt.Errorf("dns_delegation delete %q: %w", ref.Name, err) } return nil } ``` -If `interfaces.ResourceRef` does NOT have `InputSnapshot` (verify via `cat /Users/jon/workspace/workflow/interfaces/iac_resource_driver.go | grep -A5 "type ResourceRef"`), substitute the correct field name and adjust both the test and impl. If no such field exists at all, the implementer MUST pause and surface this as a design amendment — Delete cannot restore previous_nameservers without a state channel. - **Step 4: Run tests to verify they pass** Run: `GOWORK=off go test ./internal/drivers -run "TestDelegationDriver_Read|TestDelegationDriver_Update|TestDelegationDriver_Delete" -v` @@ -1551,21 +1466,28 @@ git commit -m "feat(drivers): DelegationDriver.HealthCheck + Scale + ctx tests" ### Task 11: Wire `DelegationDriver` into `HoverProvider` **Files:** -- Modify: `internal/provider.go:22` (type comment) + `:84-87` (drivers map) + `:90-100` (Capabilities) +- Modify: `internal/provider.go` — type comment on `HoverProvider`, drivers map in `Initialize`, `Capabilities()` return slice. +- Test: `internal/provider_test.go` (create if absent) — direct unit test on `HoverProvider.Capabilities()` (no gRPC harness needed since `Capabilities` is a pure function returning a hardcoded slice). **Step 1: Write the failing test** -Add to `internal/iacserver_test.go` (or wherever iacserver-level capability assertions live; check existing tests for the canonical place): +Create or append to `internal/provider_test.go`: ```go -func TestHoverIaCServer_Capabilities_IncludesDelegation(t *testing.T) { - // Smoke: Capabilities() must list both infra.dns and infra.dns_delegation. - // Tests should bring up the iacserver via the existing test harness; - // see iacserver_test.go for the canonical setup. - // Pseudocode (adapt to existing test harness): - caps := newTestIaCServerInitialized(t).Capabilities() - wantTypes := map[string]bool{"infra.dns": false, "infra.dns_delegation": false} - for _, c := range caps.Capabilities { +package internal + +import ( + "testing" +) + +func TestHoverProvider_Capabilities_IncludesDelegation(t *testing.T) { + p := NewHoverProvider() + caps := p.Capabilities() + wantTypes := map[string]bool{ + "infra.dns": false, + "infra.dns_delegation": false, + } + for _, c := range caps { if _, ok := wantTypes[c.ResourceType]; ok { wantTypes[c.ResourceType] = true } @@ -1578,18 +1500,18 @@ func TestHoverIaCServer_Capabilities_IncludesDelegation(t *testing.T) { } ``` -(The implementer must adapt this to the actual existing `iacserver_test.go` harness pattern. Check the existing capabilities test there for the canonical assertion shape.) +`Capabilities()` is a pure method that does not require `Initialize` (no client needed; the returned slice is hardcoded). No gRPC harness, no Initialize state, no fixtures. **Step 2: Run tests to verify they fail** -Run: `GOWORK=off go test ./internal -count=1 -run "Capabilities" -v` -Expected: FAIL or PASS-without-new-type — the new resource type is not yet registered. +Run: `GOWORK=off go test ./internal -count=1 -run TestHoverProvider_Capabilities_IncludesDelegation -v` +Expected: FAIL — Capabilities doesn't include `infra.dns_delegation` yet. **Step 3: Write minimal implementation** Edit `internal/provider.go`: -1. Update the type comment at line 21-22: +1. Update the `HoverProvider` type comment (currently single-resource-type wording): ```go // HoverProvider implements interfaces.IaCProvider for Hover. @@ -1598,7 +1520,7 @@ Edit `internal/provider.go`: // - infra.dns_delegation — registrar-level nameserver delegation. ``` -2. In `Initialize` (line 84), add the delegation driver to the map: +2. In `Initialize` (the drivers map initializer), add the delegation driver: ```go p.drivers = map[string]interfaces.ResourceDriver{ @@ -1607,7 +1529,7 @@ Edit `internal/provider.go`: } ``` -3. In `Capabilities` (line 90-100), append the second capability: +3. In `Capabilities()`, append the second capability: ```go func (p *HoverProvider) Capabilities() []interfaces.IaCCapabilityDeclaration { @@ -1634,7 +1556,7 @@ Expected: PASS for capabilities + all existing tests. **Step 5: Commit** ```bash -git add internal/provider.go internal/iacserver_test.go +git add internal/provider.go internal/provider_test.go git commit -m "feat(provider): register DelegationDriver + update Capabilities" ``` From d9ef607a1817f41f62db4730d4d74184dc64483c Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 20:58:29 -0400 Subject: [PATCH 07/20] =?UTF-8?q?docs:=20plan=20revision=20round=202=20?= =?UTF-8?q?=E2=80=94=20fix=20stale=20stash=20refs=20+=20Red-step=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses 3 Important + 3 Minor from plan-phase adversarial review round 2: IMPORTANT: - Existing TestHoverIaCServer_Capabilities asserts len==1; added explicit Step 4b in Task 11 to update it to len==2 + multi-type assertion. Task 11 commit list now includes iacserver_test.go. - Task 3/4/5 Red-step test code rewritten to use existing newStubClient + rewriteTransport directly (was: newTestClient references that would compile-error before the "re-author" note was reached). - DelegationDriver struct comment + PR body bullet rewritten to match v0.2.0 spec (no previous_nameservers stash; fallback-only Delete). Was contradicting Task 7's delegationOutput which already omits the field. MINOR (design doc cleanup): - Removed stale "stash current NS as previous_nameservers" line from data-flow diagram. - Removed "Pre-change GetDomainDelegation fails during Create" row from error-handling table (dead-code path). - Updated return line: dnsDelegationOutputFromDesired(..., previous_ns) -> delegationOutput(domain, ns). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../2026-05-20-hover-dns-delegation-design.md | 4 +- docs/plans/2026-05-20-hover-dns-delegation.md | 119 ++++++++++-------- 2 files changed, 68 insertions(+), 55 deletions(-) diff --git a/docs/plans/2026-05-20-hover-dns-delegation-design.md b/docs/plans/2026-05-20-hover-dns-delegation-design.md index c0b8935..5ee90df 100644 --- a/docs/plans/2026-05-20-hover-dns-delegation-design.md +++ b/docs/plans/2026-05-20-hover-dns-delegation-design.md @@ -149,7 +149,6 @@ wfctl Plan → DelegationDriver.Diff(desired, current) ↓ wfctl Apply → DelegationDriver.Create or Update ├── ctx.Err check - ├── Create only: GetDomainDelegation → stash current NS as previous_nameservers (best-effort) ├── client.SetNameservers │ ├── lock c.mu (acquired FIRST) │ ├── ensureLoginLocked (no separate lock; runs under held c.mu) @@ -157,7 +156,7 @@ wfctl Apply → DelegationDriver.Create or Update │ └── PUT /api/control_panel/domains/domain- │ Body: {"field":"nameservers","value":[...]} │ Header: X-CSRF-Token: - └── 200 → return dnsDelegationOutputFromDesired(domain, ns, previous_ns) + └── 200 → return delegationOutput(domain, ns) ↓ wfctl persists state. Subsequent Plans no-op until config changes. ``` @@ -169,7 +168,6 @@ wfctl persists state. Subsequent Plans no-op until config changes. | CSRF-fetch non-2xx | Typed error "hover: fetch control_panel CSRF: HTTP %d"; PUT not attempted. | | CSRF meta tag missing | Typed error "hover: CSRF meta tag not found at /control_panel/domain/%s (control_panel UI changed?)". | | Login expired between CSRF fetch and PUT | Cannot happen: both run under the same `c.mu` lock-hold; no re-auth can interleave. | -| Pre-change `GetDomainDelegation` fails during Create | Logged warning; Create proceeds with empty `previous_nameservers`. Not blocking. | | PUT non-2xx | Surface Hover's response body: "hover SetNameservers %q: HTTP %d: %s". | | Cloudflare challenge on PUT | Manifests as non-2xx → same path. Operator must allowlist the runner IP. README documents. | | Domain rename via Update | Typed error "domain change requires resource replace, not update". | diff --git a/docs/plans/2026-05-20-hover-dns-delegation.md b/docs/plans/2026-05-20-hover-dns-delegation.md index 839c9da..f0ff0ba 100644 --- a/docs/plans/2026-05-20-hover-dns-delegation.md +++ b/docs/plans/2026-05-20-hover-dns-delegation.md @@ -200,21 +200,21 @@ git commit -m "refactor(hover): split ensureLogin into Locked variant" **Step 1: Write the failing test** -Add to `internal/hover/client_test.go`: +Add to `internal/hover/client_test.go` using the existing `newStubClient` helper (DO NOT introduce a new `newTestClient`): ```go func TestFetchControlPanelCSRFLocked_ExtractsMetaToken(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + c, srv := newStubClient(t, func(w http.ResponseWriter, r *http.Request) { if r.URL.Path != "/control_panel/domain/example.com" { t.Errorf("unexpected path: %s", r.URL.Path) } _, _ = w.Write([]byte(` `)) - })) + }) defer srv.Close() + c.loggedAt = time.Now() // skip login - c := newTestClient(t, srv.URL) c.mu.Lock() defer c.mu.Unlock() token, err := c.fetchControlPanelCSRFLocked(context.Background(), "example.com") @@ -227,12 +227,12 @@ func TestFetchControlPanelCSRFLocked_ExtractsMetaToken(t *testing.T) { } func TestFetchControlPanelCSRFLocked_MissingMetaTag(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + c, srv := newStubClient(t, func(w http.ResponseWriter, r *http.Request) { _, _ = w.Write([]byte(``)) - })) + }) defer srv.Close() + c.loggedAt = time.Now() - c := newTestClient(t, srv.URL) c.mu.Lock() defer c.mu.Unlock() _, err := c.fetchControlPanelCSRFLocked(context.Background(), "example.com") @@ -242,12 +242,12 @@ func TestFetchControlPanelCSRFLocked_MissingMetaTag(t *testing.T) { } func TestFetchControlPanelCSRFLocked_Non2xx(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + c, srv := newStubClient(t, func(w http.ResponseWriter, r *http.Request) { http.Error(w, "denied", http.StatusForbidden) - })) + }) defer srv.Close() + c.loggedAt = time.Now() - c := newTestClient(t, srv.URL) c.mu.Lock() defer c.mu.Unlock() _, err := c.fetchControlPanelCSRFLocked(context.Background(), "example.com") @@ -257,28 +257,7 @@ func TestFetchControlPanelCSRFLocked_Non2xx(t *testing.T) { } ``` -**Use the existing helpers.** `internal/hover/client_test.go` already provides: -- `func newStubClient(t *testing.T, handler http.HandlerFunc) (*Client, *httptest.Server)` — builds a Client with cookie jar + `rewriteTransport`. -- `type rewriteTransport struct{ base string }` + `RoundTrip` — rewrites all outbound URLs to the httptest server base. - -The tests above should be authored against `newStubClient` (passing a `http.HandlerFunc` that dispatches by path), NOT a new `newTestClient` helper. Do NOT introduce a duplicate `rewritingTransport` struct — it would collide with the existing `rewriteTransport`. - -Pattern (replaces all `newTestClient(t, srv.URL)` and `httptest.NewServer(...)` references in this and subsequent tasks): - -```go -c, srv := newStubClient(t, func(w http.ResponseWriter, r *http.Request) { - switch r.URL.Path { - case "/control_panel/domain/example.com": - _, _ = w.Write([]byte(``)) - default: - t.Errorf("unexpected path: %s", r.URL.Path) - } -}) -defer srv.Close() -c.loggedAt = time.Now() // skip login (newStubClient builds a fresh client) -``` - -Re-author each `TestFetchControlPanelCSRFLocked_*` (and all subsequent task tests) using this `newStubClient` shape; the assertions on URL, headers, body content remain identical. +**Existing helpers used:** `internal/hover/client_test.go` already provides `func newStubClient(t *testing.T, handler http.HandlerFunc) (*Client, *httptest.Server)` + the supporting `rewriteTransport`. Tasks 3–5 use these directly; DO NOT add a new `newTestClient` or `rewritingTransport`. **Step 2: Run tests to verify they fail** @@ -350,15 +329,15 @@ git commit -m "feat(hover): fetchControlPanelCSRFLocked + csrfMetaRe regex" ```go func TestGetDomainDelegation_HappyPath(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + c, srv := newStubClient(t, func(w http.ResponseWriter, r *http.Request) { if r.URL.Path != "/api/control_panel/domains/domain-example.com" { t.Errorf("unexpected path: %s", r.URL.Path) } _, _ = w.Write([]byte(`{"id":"domain-example.com","domain_name":"example.com","nameservers":["ns1.do.com","ns2.do.com"]}`)) - })) + }) defer srv.Close() + c.loggedAt = time.Now() - c := newTestClient(t, srv.URL) dom, err := c.GetDomainDelegation(context.Background(), "example.com") if err != nil { t.Fatalf("GetDomainDelegation: %v", err) @@ -372,12 +351,12 @@ func TestGetDomainDelegation_HappyPath(t *testing.T) { } func TestGetDomainDelegation_EmptyNameserversReturnsSentinel(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + c, srv := newStubClient(t, func(w http.ResponseWriter, r *http.Request) { _, _ = w.Write([]byte(`{"id":"domain-example.com","domain_name":"example.com","nameservers":[]}`)) - })) + }) defer srv.Close() + c.loggedAt = time.Now() - c := newTestClient(t, srv.URL) _, err := c.GetDomainDelegation(context.Background(), "example.com") if !errors.Is(err, ErrEmptyNameservers) { t.Fatalf("want ErrEmptyNameservers, got %v", err) @@ -385,12 +364,12 @@ func TestGetDomainDelegation_EmptyNameserversReturnsSentinel(t *testing.T) { } func TestGetDomainDelegation_Non2xx(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + c, srv := newStubClient(t, func(w http.ResponseWriter, r *http.Request) { http.Error(w, "not found", http.StatusNotFound) - })) + }) defer srv.Close() + c.loggedAt = time.Now() - c := newTestClient(t, srv.URL) _, err := c.GetDomainDelegation(context.Background(), "example.com") if err == nil { t.Fatal("expected error on 404") @@ -469,7 +448,7 @@ git commit -m "feat(hover): GetDomainDelegation method (loud on empty)" func TestSetNameservers_PUTShape(t *testing.T) { var capturedURL, capturedToken, capturedCT string var capturedBody []byte - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + c, srv := newStubClient(t, func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { case "/control_panel/domain/example.com": _, _ = w.Write([]byte(``)) @@ -482,10 +461,10 @@ func TestSetNameservers_PUTShape(t *testing.T) { default: t.Errorf("unexpected path: %s", r.URL.Path) } - })) + }) defer srv.Close() + c.loggedAt = time.Now() - c := newTestClient(t, srv.URL) err := c.SetNameservers(context.Background(), "example.com", []string{"a.com", "b.com"}) if err != nil { t.Fatalf("SetNameservers: %v", err) @@ -513,16 +492,16 @@ func TestSetNameservers_PUTShape(t *testing.T) { } func TestSetNameservers_Non2xxPUT(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + c, srv := newStubClient(t, func(w http.ResponseWriter, r *http.Request) { if strings.HasPrefix(r.URL.Path, "/control_panel/domain/") { _, _ = w.Write([]byte(``)) return } http.Error(w, "bad token", http.StatusUnprocessableEntity) - })) + }) defer srv.Close() + c.loggedAt = time.Now() - c := newTestClient(t, srv.URL) err := c.SetNameservers(context.Background(), "example.com", []string{"a.com"}) if err == nil { t.Fatal("expected error on 422") @@ -703,9 +682,10 @@ type HoverDelegationClient interface { // (infra.dns_delegation) for Hover-registered domains. // // ProviderID = apex domain name (e.g. "example.com"). One resource = one -// domain. Outputs include the desired nameservers as []any (structpb-safe) -// and a stashed previous_nameservers list captured at Create time for -// Delete-restoration. +// domain. Outputs contain only the desired nameservers as []any +// (structpb-safe). v0.2.0 ships Delete = reset to Hover defaults +// [ns1.hover.com, ns2.hover.com]; restore-from-stash is deferred to +// v0.3.0 because interfaces.ResourceRef has no state channel. type DelegationDriver struct { client HoverDelegationClient } @@ -1553,10 +1533,45 @@ func (p *HoverProvider) Capabilities() []interfaces.IaCCapabilityDeclaration { Run: `GOWORK=off go test ./internal -count=1` Expected: PASS for capabilities + all existing tests. +**Step 4b: Update existing iacserver_test.go capability assertion** + +The pre-existing `TestHoverIaCServer_Capabilities` (in `internal/iacserver_test.go` around line 41-47) hard-codes: + +```go +if len(resp.GetCapabilities()) != 1 { + t.Fatalf("expected 1 capability, got %d", len(resp.GetCapabilities())) +} +cap := resp.GetCapabilities()[0] +if cap.GetResourceType() != "infra.dns" { + t.Errorf("ResourceType = %q want %q", cap.GetResourceType(), "infra.dns") +} +``` + +After registration the response carries 2 capabilities; this test breaks. Update to: + +```go +caps := resp.GetCapabilities() +if len(caps) != 2 { + t.Fatalf("expected 2 capabilities, got %d", len(caps)) +} +gotTypes := map[string]bool{} +for _, c := range caps { + gotTypes[c.GetResourceType()] = true +} +for _, want := range []string{"infra.dns", "infra.dns_delegation"} { + if !gotTypes[want] { + t.Errorf("capability %q missing", want) + } +} +``` + +Run: `GOWORK=off go test ./internal -count=1 -run TestHoverIaCServer_Capabilities -v` +Expected: PASS. + **Step 5: Commit** ```bash -git add internal/provider.go internal/provider_test.go +git add internal/provider.go internal/provider_test.go internal/iacserver_test.go git commit -m "feat(provider): register DelegationDriver + update Capabilities" ``` @@ -1657,7 +1672,7 @@ X-CSRF-Token: - New \`DelegationDriver\` registered as \`infra.dns_delegation\` alongside the existing \`infra.dns\` driver. - \`plugin.json\` declares the new resource type. - Outputs structpb-safe (\`[]any\` not \`[]string\`). -- Delete stashes \`previous_nameservers\` at Create for restore; falls back to \`[ns1.hover.com, ns2.hover.com]\` for state-less resources. +- Delete = reset to \`[ns1.hover.com, ns2.hover.com]\` (Hover defaults). Restore-from-stash deferred to v0.3.0 (requires \`interfaces.ResourceRef\` to gain a state channel). ## Follow-ups (deferred to a separate session) From a62ec4075f75d256e302cb37cebce0ac2a242af3 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 21:00:50 -0400 Subject: [PATCH 08/20] chore: lock scope for hover dns_delegation (alignment passed) --- docs/plans/2026-05-20-hover-dns-delegation.md | 2 +- docs/plans/2026-05-20-hover-dns-delegation.md.scope-lock | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 docs/plans/2026-05-20-hover-dns-delegation.md.scope-lock diff --git a/docs/plans/2026-05-20-hover-dns-delegation.md b/docs/plans/2026-05-20-hover-dns-delegation.md index f0ff0ba..121fad5 100644 --- a/docs/plans/2026-05-20-hover-dns-delegation.md +++ b/docs/plans/2026-05-20-hover-dns-delegation.md @@ -32,7 +32,7 @@ |------|-------|-------|--------| | 1 | feat: infra.dns_delegation resource type (v0.2.0) | Task 1, Task 2, Task 3, Task 4, Task 5, Task 6, Task 7, Task 8, Task 9, Task 10, Task 11, Task 12, Task 13 | feat/dns-delegation | -**Status:** Draft +**Status:** Locked 2026-05-21T01:00:33Z --- diff --git a/docs/plans/2026-05-20-hover-dns-delegation.md.scope-lock b/docs/plans/2026-05-20-hover-dns-delegation.md.scope-lock new file mode 100644 index 0000000..46ba6c8 --- /dev/null +++ b/docs/plans/2026-05-20-hover-dns-delegation.md.scope-lock @@ -0,0 +1 @@ +f88f1685a1b0df73f590c224f8ac57c8848fae555fc97924efbd6514318b399d From 87915c21e8b56cbfca3a35e3718479144a63e339 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 21:04:40 -0400 Subject: [PATCH 09/20] feat(hover): DomainDelegation type + ErrEmptyNameservers sentinel --- internal/hover/client.go | 19 +++++++++++++++++++ internal/hover/client_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/internal/hover/client.go b/internal/hover/client.go index 7c42501..88a28c4 100644 --- a/internal/hover/client.go +++ b/internal/hover/client.go @@ -206,6 +206,25 @@ func (c *Client) postForm(ctx context.Context, urlStr string, form url.Values) e return nil } +// DomainDelegation is the response shape of GET /api/control_panel/domains/domain-. +// Distinct from Domain (which represents the /api/domains//dns shape with Records) +// to avoid ambiguity over which fields are populated by which endpoint. +// +// Tentative envelope per design A6: flat object, not wrapped in {"domains":[...]}. +// First field-test call must confirm this shape; if Hover returns a different envelope +// the implementer pauses and amends the design before proceeding. +type DomainDelegation struct { + ID string `json:"id"` + Name string `json:"domain_name"` + Nameservers []string `json:"nameservers"` +} + +// ErrEmptyNameservers is returned by GetDomainDelegation when the parsed +// response has zero nameservers. Converts the silent-thrash failure mode +// (empty → Diff says NeedsUpdate forever → re-PUT loop) into a loud, +// single-iteration error visible at the first wfctl plan. +var ErrEmptyNameservers = errors.New("hover: delegation read returned 0 nameservers (verify field shape)") + // DNSRecord mirrors Hover's internal API record shape. type DNSRecord struct { ID string `json:"id,omitempty"` diff --git a/internal/hover/client_test.go b/internal/hover/client_test.go index b0de390..b7ad10d 100644 --- a/internal/hover/client_test.go +++ b/internal/hover/client_test.go @@ -2,6 +2,9 @@ package hover import ( "context" + "encoding/json" + "errors" + "fmt" "net/http" "net/http/cookiejar" "net/http/httptest" @@ -358,6 +361,31 @@ func TestClient_DeleteRecord(t *testing.T) { } } +func TestDomainDelegation_JSONShape(t *testing.T) { + // Tentative envelope per design A6: flat object, not wrapped. + body := `{"id":"domain-example.com","domain_name":"example.com","nameservers":["a.com","b.com"]}` + var d DomainDelegation + if err := json.Unmarshal([]byte(body), &d); err != nil { + t.Fatalf("decode: %v", err) + } + if d.ID != "domain-example.com" { + t.Errorf("ID = %q, want domain-example.com", d.ID) + } + if d.Name != "example.com" { + t.Errorf("Name = %q, want example.com", d.Name) + } + if len(d.Nameservers) != 2 || d.Nameservers[0] != "a.com" || d.Nameservers[1] != "b.com" { + t.Errorf("Nameservers = %v, want [a.com b.com]", d.Nameservers) + } +} + +func TestErrEmptyNameservers_IsSentinel(t *testing.T) { + wrapped := fmt.Errorf("hover GetDomainDelegation: %w", ErrEmptyNameservers) + if !errors.Is(wrapped, ErrEmptyNameservers) { + t.Error("errors.Is should match ErrEmptyNameservers when wrapped") + } +} + func TestClient_ListRecords_DomainNotFound(t *testing.T) { // API returns empty domains list — our client must return a clear error. respBody := `{"domains": []}` From ec8f0ac87bc42a2d1f434f1ed1c2a3c0db3deffe Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 21:05:22 -0400 Subject: [PATCH 10/20] refactor(hover): split ensureLogin into Locked variant --- internal/hover/client.go | 11 +++++++++++ internal/hover/client_test.go | 16 ++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/internal/hover/client.go b/internal/hover/client.go index 88a28c4..bec1a93 100644 --- a/internal/hover/client.go +++ b/internal/hover/client.go @@ -79,9 +79,20 @@ func (c *Client) Login(ctx context.Context) error { // ensureLogin re-authenticates iff the session is stale. Safe to call // before every API hit; idempotent within sessionStaleAfter. +// +// Acquires c.mu internally. Callers that already hold the lock must +// call ensureLoginLocked instead. func (c *Client) ensureLogin(ctx context.Context) error { c.mu.Lock() defer c.mu.Unlock() + return c.ensureLoginLocked(ctx) +} + +// ensureLoginLocked is the implementation of ensureLogin without the lock +// acquisition. Caller MUST hold c.mu. Used by SetNameservers which holds +// c.mu across the full auth → CSRF → PUT sequence to eliminate the +// TOCTOU window between auth-check and PUT. +func (c *Client) ensureLoginLocked(ctx context.Context) error { if !c.loggedAt.IsZero() && time.Since(c.loggedAt) < sessionStaleAfter { return nil } diff --git a/internal/hover/client_test.go b/internal/hover/client_test.go index b7ad10d..0588f92 100644 --- a/internal/hover/client_test.go +++ b/internal/hover/client_test.go @@ -10,6 +10,7 @@ import ( "net/http/httptest" "strings" "testing" + "time" ) // signinCSRFHTML is what we return on GET /signin + /signin/totp so @@ -361,6 +362,21 @@ func TestClient_DeleteRecord(t *testing.T) { } } +func TestEnsureLoginLocked_CallableUnderHeldLock(t *testing.T) { + // Build a Client with a fresh loggedAt so ensureLoginLocked + // short-circuits without making HTTP calls. + c, err := NewClient(Credentials{Username: "u", Password: "p"}, nil) + if err != nil { + t.Fatalf("NewClient: %v", err) + } + c.loggedAt = time.Now() // skip the actual login + c.mu.Lock() + defer c.mu.Unlock() + if err := c.ensureLoginLocked(context.Background()); err != nil { + t.Errorf("ensureLoginLocked under held mu: %v", err) + } +} + func TestDomainDelegation_JSONShape(t *testing.T) { // Tentative envelope per design A6: flat object, not wrapped. body := `{"id":"domain-example.com","domain_name":"example.com","nameservers":["a.com","b.com"]}` From 8a3f525587262d1258b786116188f4d364faf380 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 21:06:03 -0400 Subject: [PATCH 11/20] feat(hover): fetchControlPanelCSRFLocked + csrfMetaRe regex --- internal/hover/client.go | 36 ++++++++++++++++++++++++ internal/hover/client_test.go | 53 +++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+) diff --git a/internal/hover/client.go b/internal/hover/client.go index bec1a93..9b78073 100644 --- a/internal/hover/client.go +++ b/internal/hover/client.go @@ -145,6 +145,42 @@ func (c *Client) ensureLoginLocked(ctx context.Context) error { var csrfRe = regexp.MustCompile(`]+name="_token"[^>]+value="([^"]+)"`) +// csrfMetaRe extracts the Rails CSRF meta token from a control-panel HTML +// page. Distinct from csrfRe (form-token regex used by the /signin flow) +// because the control-panel pages embed the token as a meta tag for the +// SPA layer to read, while /signin embeds it as a hidden input. +// +// Both shapes coexist in the Hover-served HTML; we match each from the +// page where it's authoritative. +var csrfMetaRe = regexp.MustCompile(`. Caller MUST hold c.mu (so the HTTP GET +// and any subsequent PUT execute against the same session-cookie state). +func (c *Client) fetchControlPanelCSRFLocked(ctx context.Context, domainName string) (string, error) { + endpoint := fmt.Sprintf("%s/control_panel/domain/%s", hoverHost, url.PathEscape(domainName)) + req, _ := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) + req.Header.Set("User-Agent", c.UserAgent) + resp, err := c.http.Do(req) + if err != nil { + return "", fmt.Errorf("hover: fetch control_panel CSRF for %q: %w", domainName, err) + } + defer resp.Body.Close() + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + body, _ := io.ReadAll(io.LimitReader(resp.Body, 512)) + return "", fmt.Errorf("hover: fetch control_panel CSRF for %q: HTTP %d: %s", domainName, resp.StatusCode, strings.TrimSpace(string(body))) + } + body, err := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) + if err != nil { + return "", fmt.Errorf("hover: fetch control_panel CSRF for %q: read body: %w", domainName, err) + } + m := csrfMetaRe.FindSubmatch(body) + if len(m) < 2 { + return "", fmt.Errorf("hover: CSRF meta tag not found at /control_panel/domain/%s (control_panel UI changed?)", domainName) + } + return string(m[1]), nil +} + func (c *Client) fetchSignInCSRF(ctx context.Context) (string, error) { return c.fetchCSRF(ctx, hoverHost+"/signin") } diff --git a/internal/hover/client_test.go b/internal/hover/client_test.go index 0588f92..73a27a9 100644 --- a/internal/hover/client_test.go +++ b/internal/hover/client_test.go @@ -377,6 +377,59 @@ func TestEnsureLoginLocked_CallableUnderHeldLock(t *testing.T) { } } +func TestFetchControlPanelCSRFLocked_ExtractsMetaToken(t *testing.T) { + c, srv := newStubClient(t, func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/control_panel/domain/example.com" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + _, _ = w.Write([]byte(` + +`)) + }) + defer srv.Close() + c.loggedAt = time.Now() // skip login + + c.mu.Lock() + defer c.mu.Unlock() + token, err := c.fetchControlPanelCSRFLocked(context.Background(), "example.com") + if err != nil { + t.Fatalf("fetchControlPanelCSRFLocked: %v", err) + } + if token != "abc123xyz" { + t.Errorf("token = %q, want abc123xyz", token) + } +} + +func TestFetchControlPanelCSRFLocked_MissingMetaTag(t *testing.T) { + c, srv := newStubClient(t, func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(``)) + }) + defer srv.Close() + c.loggedAt = time.Now() + + c.mu.Lock() + defer c.mu.Unlock() + _, err := c.fetchControlPanelCSRFLocked(context.Background(), "example.com") + if err == nil { + t.Fatal("expected error when meta tag absent") + } +} + +func TestFetchControlPanelCSRFLocked_Non2xx(t *testing.T) { + c, srv := newStubClient(t, func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "denied", http.StatusForbidden) + }) + defer srv.Close() + c.loggedAt = time.Now() + + c.mu.Lock() + defer c.mu.Unlock() + _, err := c.fetchControlPanelCSRFLocked(context.Background(), "example.com") + if err == nil { + t.Fatal("expected error on 403") + } +} + func TestDomainDelegation_JSONShape(t *testing.T) { // Tentative envelope per design A6: flat object, not wrapped. body := `{"id":"domain-example.com","domain_name":"example.com","nameservers":["a.com","b.com"]}` From cce97e087b440c910d9acdea51759e3e49f50998 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 21:06:35 -0400 Subject: [PATCH 12/20] feat(hover): GetDomainDelegation method (loud on empty) --- internal/hover/client.go | 35 +++++++++++++++++++++++++ internal/hover/client_test.go | 48 +++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+) diff --git a/internal/hover/client.go b/internal/hover/client.go index 9b78073..35958e9 100644 --- a/internal/hover/client.go +++ b/internal/hover/client.go @@ -288,6 +288,41 @@ type Domain struct { Records []DNSRecord `json:"entries"` } +// GetDomainDelegation fetches the registrar-level nameserver delegation for +// the named domain via the control-panel API (same endpoint family as the +// PUT used by SetNameservers — more likely to surface nameservers reliably +// than the DNS-records-oriented /api/domains//dns endpoint). +// +// Returns ErrEmptyNameservers if the parsed response has zero nameservers. +// This loud-on-empty behavior is intentional: it converts the silent +// re-apply thrash failure mode (empty → Diff says NeedsUpdate forever) +// into a single-iteration error visible at first wfctl plan. +func (c *Client) GetDomainDelegation(ctx context.Context, domainName string) (*DomainDelegation, error) { + if err := c.ensureLogin(ctx); err != nil { + return nil, err + } + endpoint := fmt.Sprintf("%s/api/control_panel/domains/domain-%s", hoverHost, url.PathEscape(domainName)) + req, _ := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil) + req.Header.Set("User-Agent", c.UserAgent) + resp, err := c.http.Do(req) + if err != nil { + return nil, fmt.Errorf("hover: GetDomainDelegation %q: %w", domainName, err) + } + defer resp.Body.Close() + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + body, _ := io.ReadAll(io.LimitReader(resp.Body, 512)) + return nil, fmt.Errorf("hover: GetDomainDelegation %q: HTTP %d: %s", domainName, resp.StatusCode, strings.TrimSpace(string(body))) + } + var d DomainDelegation + if err := json.NewDecoder(resp.Body).Decode(&d); err != nil { + return nil, fmt.Errorf("hover: GetDomainDelegation %q: decode: %w", domainName, err) + } + if len(d.Nameservers) == 0 { + return nil, fmt.Errorf("hover: GetDomainDelegation %q: %w", domainName, ErrEmptyNameservers) + } + return &d, nil +} + // GetDomain returns the full Domain struct (including the // hover-assigned ID) for the named zone. The ID is required when // creating new records via CreateRecord; the human-readable name is diff --git a/internal/hover/client_test.go b/internal/hover/client_test.go index 73a27a9..aca484d 100644 --- a/internal/hover/client_test.go +++ b/internal/hover/client_test.go @@ -430,6 +430,54 @@ func TestFetchControlPanelCSRFLocked_Non2xx(t *testing.T) { } } +func TestGetDomainDelegation_HappyPath(t *testing.T) { + c, srv := newStubClient(t, func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/control_panel/domains/domain-example.com" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + _, _ = w.Write([]byte(`{"id":"domain-example.com","domain_name":"example.com","nameservers":["ns1.do.com","ns2.do.com"]}`)) + }) + defer srv.Close() + c.loggedAt = time.Now() + + dom, err := c.GetDomainDelegation(context.Background(), "example.com") + if err != nil { + t.Fatalf("GetDomainDelegation: %v", err) + } + if dom.ID != "domain-example.com" { + t.Errorf("ID = %q", dom.ID) + } + if len(dom.Nameservers) != 2 { + t.Errorf("Nameservers len = %d, want 2", len(dom.Nameservers)) + } +} + +func TestGetDomainDelegation_EmptyNameserversReturnsSentinel(t *testing.T) { + c, srv := newStubClient(t, func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"id":"domain-example.com","domain_name":"example.com","nameservers":[]}`)) + }) + defer srv.Close() + c.loggedAt = time.Now() + + _, err := c.GetDomainDelegation(context.Background(), "example.com") + if !errors.Is(err, ErrEmptyNameservers) { + t.Fatalf("want ErrEmptyNameservers, got %v", err) + } +} + +func TestGetDomainDelegation_Non2xx(t *testing.T) { + c, srv := newStubClient(t, func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "not found", http.StatusNotFound) + }) + defer srv.Close() + c.loggedAt = time.Now() + + _, err := c.GetDomainDelegation(context.Background(), "example.com") + if err == nil { + t.Fatal("expected error on 404") + } +} + func TestDomainDelegation_JSONShape(t *testing.T) { // Tentative envelope per design A6: flat object, not wrapped. body := `{"id":"domain-example.com","domain_name":"example.com","nameservers":["a.com","b.com"]}` From 564b3e45ee5a01d25f0b29dc39abcfe42b532e8b Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 21:07:25 -0400 Subject: [PATCH 13/20] feat(hover): SetNameservers + putNameserversLocked --- internal/hover/client.go | 57 +++++++++++++++++++++++++++++++ internal/hover/client_test.go | 64 +++++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+) diff --git a/internal/hover/client.go b/internal/hover/client.go index 35958e9..f5ac509 100644 --- a/internal/hover/client.go +++ b/internal/hover/client.go @@ -1,6 +1,7 @@ package hover import ( + "bytes" "context" "encoding/json" "errors" @@ -323,6 +324,62 @@ func (c *Client) GetDomainDelegation(ctx context.Context, domainName string) (*D return &d, nil } +// SetNameservers updates the registrar-level nameservers for a domain via +// Hover's control-panel API. +// +// Lock discipline: holds c.mu for the entire auth → CSRF fetch → PUT +// sequence. This eliminates the TOCTOU window between auth-check and +// PUT (another goroutine cannot re-auth and invalidate the CSRF token +// between the two requests). +// +// Trade-off: any concurrent caller using the same *Client blocks for +// up to ~60s (two HTTP round-trips under the 30s default client timeout). +// Acceptable for the field-test scope (single goroutine, one delegation +// resource). Future: cache CSRF at session granularity if mixed-resource +// throughput becomes a concern. +func (c *Client) SetNameservers(ctx context.Context, domainName string, ns []string) error { + c.mu.Lock() + defer c.mu.Unlock() + if err := c.ensureLoginLocked(ctx); err != nil { + return err + } + csrf, err := c.fetchControlPanelCSRFLocked(ctx, domainName) + if err != nil { + return err + } + return c.putNameserversLocked(ctx, domainName, ns, csrf) +} + +// putNameserversLocked PUTs the nameservers list. Caller MUST hold c.mu. +// +// Note: the wire payload uses []string directly — encoding/json serializes +// it as a JSON array, which is what Hover expects. This is distinct from +// the []any requirement in ResourceOutput.Outputs (which crosses the +// structpb gRPC boundary); typed slices are fine here because the wire +// format is plain JSON, not structpb. +func (c *Client) putNameserversLocked(ctx context.Context, domainName string, ns []string, csrf string) error { + endpoint := fmt.Sprintf("%s/api/control_panel/domains/domain-%s", hoverHost, url.PathEscape(domainName)) + payload := map[string]any{"field": "nameservers", "value": ns} + body, err := json.Marshal(payload) + if err != nil { + return fmt.Errorf("hover: SetNameservers %q: marshal: %w", domainName, err) + } + req, _ := http.NewRequestWithContext(ctx, http.MethodPut, endpoint, bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("User-Agent", c.UserAgent) + req.Header.Set("X-CSRF-Token", csrf) + resp, err := c.http.Do(req) + if err != nil { + return fmt.Errorf("hover: SetNameservers %q: %w", domainName, err) + } + defer resp.Body.Close() + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + respBody, _ := io.ReadAll(io.LimitReader(resp.Body, 512)) + return fmt.Errorf("hover: SetNameservers %q: HTTP %d: %s", domainName, resp.StatusCode, strings.TrimSpace(string(respBody))) + } + return nil +} + // GetDomain returns the full Domain struct (including the // hover-assigned ID) for the named zone. The ID is required when // creating new records via CreateRecord; the human-readable name is diff --git a/internal/hover/client_test.go b/internal/hover/client_test.go index aca484d..9827ee9 100644 --- a/internal/hover/client_test.go +++ b/internal/hover/client_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "net/http" "net/http/cookiejar" "net/http/httptest" @@ -478,6 +479,69 @@ func TestGetDomainDelegation_Non2xx(t *testing.T) { } } +func TestSetNameservers_PUTShape(t *testing.T) { + var capturedURL, capturedToken, capturedCT string + var capturedBody []byte + c, srv := newStubClient(t, func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/control_panel/domain/example.com": + _, _ = w.Write([]byte(``)) + case "/api/control_panel/domains/domain-example.com": + capturedURL = r.URL.Path + capturedToken = r.Header.Get("X-CSRF-Token") + capturedCT = r.Header.Get("Content-Type") + capturedBody, _ = io.ReadAll(r.Body) + w.WriteHeader(http.StatusOK) + default: + t.Errorf("unexpected path: %s", r.URL.Path) + } + }) + defer srv.Close() + c.loggedAt = time.Now() + + err := c.SetNameservers(context.Background(), "example.com", []string{"a.com", "b.com"}) + if err != nil { + t.Fatalf("SetNameservers: %v", err) + } + if capturedURL != "/api/control_panel/domains/domain-example.com" { + t.Errorf("URL = %q", capturedURL) + } + if capturedToken != "test-csrf-token" { + t.Errorf("X-CSRF-Token = %q", capturedToken) + } + if !strings.HasPrefix(capturedCT, "application/json") { + t.Errorf("Content-Type = %q", capturedCT) + } + var payload map[string]any + if err := json.Unmarshal(capturedBody, &payload); err != nil { + t.Fatalf("body decode: %v", err) + } + if payload["field"] != "nameservers" { + t.Errorf("field = %v", payload["field"]) + } + val, _ := payload["value"].([]any) + if len(val) != 2 || val[0] != "a.com" || val[1] != "b.com" { + t.Errorf("value = %v, want [a.com b.com]", payload["value"]) + } +} + +func TestSetNameservers_Non2xxPUT(t *testing.T) { + c, srv := newStubClient(t, func(w http.ResponseWriter, r *http.Request) { + if strings.HasPrefix(r.URL.Path, "/control_panel/domain/") { + _, _ = w.Write([]byte(``)) + return + } + http.Error(w, "bad token", http.StatusUnprocessableEntity) + }) + defer srv.Close() + c.loggedAt = time.Now() + + err := c.SetNameservers(context.Background(), "example.com", []string{"a.com"}) + if err == nil { + t.Fatal("expected error on 422") + } +} + func TestDomainDelegation_JSONShape(t *testing.T) { // Tentative envelope per design A6: flat object, not wrapped. body := `{"id":"domain-example.com","domain_name":"example.com","nameservers":["a.com","b.com"]}` From 60f4aab54fd1aec5cf1dc1f78757ee8edc78a0bf Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 21:09:18 -0400 Subject: [PATCH 14/20] feat(drivers): DelegationDriver skeleton + interface --- internal/drivers/delegation.go | 300 +++++++++++++++++++++++ internal/drivers/delegation_test.go | 360 ++++++++++++++++++++++++++++ 2 files changed, 660 insertions(+) create mode 100644 internal/drivers/delegation.go create mode 100644 internal/drivers/delegation_test.go diff --git a/internal/drivers/delegation.go b/internal/drivers/delegation.go new file mode 100644 index 0000000..f8de80c --- /dev/null +++ b/internal/drivers/delegation.go @@ -0,0 +1,300 @@ +// Package-doc additions in this file are scoped to the dns_delegation +// driver. See dns.go for the prior infra.dns driver. +package drivers + +import ( + "context" + "fmt" + "sort" + "strings" + + "github.com/GoCodeAlone/workflow-plugin-hover/internal/hover" + "github.com/GoCodeAlone/workflow/interfaces" +) + +// HoverDelegationClient is the subset of *hover.Client that DelegationDriver +// depends on. Injectable for tests. +type HoverDelegationClient interface { + GetDomainDelegation(ctx context.Context, domain string) (*hover.DomainDelegation, error) + SetNameservers(ctx context.Context, domain string, ns []string) error +} + +// DelegationDriver manages registrar-level nameserver delegation +// (infra.dns_delegation) for Hover-registered domains. +// +// ProviderID = apex domain name (e.g. "example.com"). One resource = one +// domain. Outputs contain only the desired nameservers as []any +// (structpb-safe). v0.2.0 ships Delete = reset to Hover defaults +// [ns1.hover.com, ns2.hover.com]; restore-from-stash is deferred to +// v0.3.0 because interfaces.ResourceRef has no state channel. +type DelegationDriver struct { + client HoverDelegationClient +} + +// NewDelegationDriver returns a DelegationDriver bound to a real *hover.Client. +func NewDelegationDriver(c *hover.Client) *DelegationDriver { + return &DelegationDriver{client: c} +} + +// NewDelegationDriverWithClient returns a DelegationDriver bound to an +// injected client; used by tests. +func NewDelegationDriverWithClient(c HoverDelegationClient) *DelegationDriver { + return &DelegationDriver{client: c} +} + +func (d *DelegationDriver) Type() string { return "infra.dns_delegation" } + +func (d *DelegationDriver) SensitiveKeys() []string { return nil } + +func (d *DelegationDriver) ProviderIDFormat() interfaces.ProviderIDFormat { + return interfaces.IDFormatDomainName +} + +// dnsDelegationSpec is the parsed config view. +type dnsDelegationSpec struct { + domain string + nameservers []string +} + +// parseDelegationSpec validates config and produces a typed view. +func parseDelegationSpec(spec interfaces.ResourceSpec) (dnsDelegationSpec, error) { + domain, _ := spec.Config["domain"].(string) + if domain == "" { + domain = spec.Name + } + if domain == "" { + return dnsDelegationSpec{}, fmt.Errorf("dns_delegation: config missing required key 'domain' (or spec.Name)") + } + rawNS, present := spec.Config["nameservers"] + if !present { + return dnsDelegationSpec{}, fmt.Errorf("dns_delegation %q: config missing required key 'nameservers'", domain) + } + nsList, ok := rawNS.([]any) + if !ok { + return dnsDelegationSpec{}, fmt.Errorf("dns_delegation %q: config 'nameservers' must be an array, got %T", domain, rawNS) + } + if len(nsList) < 1 { + return dnsDelegationSpec{}, fmt.Errorf("dns_delegation %q: config 'nameservers' must have ≥1 entry", domain) + } + seen := make(map[string]struct{}, len(nsList)) + parsed := make([]string, 0, len(nsList)) + for i, item := range nsList { + s, ok := item.(string) + if !ok { + return dnsDelegationSpec{}, fmt.Errorf("dns_delegation %q: nameservers[%d] must be a string, got %T", domain, i, item) + } + s = strings.TrimSpace(s) + if s == "" { + return dnsDelegationSpec{}, fmt.Errorf("dns_delegation %q: nameservers[%d] must be non-empty", domain, i) + } + if _, dup := seen[s]; dup { + return dnsDelegationSpec{}, fmt.Errorf("dns_delegation %q: nameservers[%d] = %q is a duplicate", domain, i, s) + } + seen[s] = struct{}{} + parsed = append(parsed, s) + } + return dnsDelegationSpec{domain: domain, nameservers: parsed}, nil +} + +// nameserversToAny converts []string to []any. Required for Outputs values +// to round-trip through structpb (typed slices are rejected; see iacserver.go +// package doc and the workspace structpb-boundary feedback memory). +func nameserversToAny(ns []string) []any { + out := make([]any, len(ns)) + for i, s := range ns { + out[i] = s + } + return out +} + +// delegationOutput builds the ResourceOutput for a Create/Update result. +// v0.2.0 ships without previous_nameservers (no state channel in +// interfaces.ResourceRef; v0.3.0 follow-up). +func delegationOutput(name, domain string, ns []string) *interfaces.ResourceOutput { + return &interfaces.ResourceOutput{ + Name: name, + Type: "infra.dns_delegation", + ProviderID: domain, + Outputs: map[string]any{ + "domain": domain, + "nameservers": nameserversToAny(ns), + }, + Status: "active", + } +} + +// Create PUTs the desired nameservers. Output built from the desired set +// (no read-after-write); SetNameservers is authoritative on success. +func (d *DelegationDriver) Create(ctx context.Context, spec interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) { + if err := ctx.Err(); err != nil { + return nil, fmt.Errorf("dns_delegation create: %w", err) + } + s, err := parseDelegationSpec(spec) + if err != nil { + return nil, err + } + if err := ctx.Err(); err != nil { + return nil, fmt.Errorf("dns_delegation create %q: %w", s.domain, err) + } + if err := d.client.SetNameservers(ctx, s.domain, s.nameservers); err != nil { + return nil, fmt.Errorf("dns_delegation create %q: %w", s.domain, err) + } + return delegationOutput(spec.Name, s.domain, s.nameservers), nil +} + +// Read fetches the current registrar nameservers. +func (d *DelegationDriver) Read(ctx context.Context, ref interfaces.ResourceRef) (*interfaces.ResourceOutput, error) { + if err := ctx.Err(); err != nil { + return nil, fmt.Errorf("dns_delegation read %q: %w", ref.Name, err) + } + domain := ref.ProviderID + if domain == "" { + domain = ref.Name + } + dom, err := d.client.GetDomainDelegation(ctx, domain) + if err != nil { + return nil, fmt.Errorf("dns_delegation read %q: %w", ref.Name, err) + } + return delegationOutput(ref.Name, domain, dom.Nameservers), nil +} + +// Update replaces the registrar nameservers. Rejects in-place domain +// renames (those must route through Diff → NeedsReplace → Delete-then-Create). +func (d *DelegationDriver) Update(ctx context.Context, ref interfaces.ResourceRef, spec interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) { + if err := ctx.Err(); err != nil { + return nil, fmt.Errorf("dns_delegation update %q: %w", ref.Name, err) + } + s, err := parseDelegationSpec(spec) + if err != nil { + return nil, err + } + currentDomain := ref.ProviderID + if currentDomain == "" { + currentDomain = ref.Name + } + if !strings.EqualFold(s.domain, currentDomain) { + return nil, fmt.Errorf("dns_delegation update %q: spec.domain %q does not match current %q — domain change requires resource replace, not update", ref.Name, s.domain, currentDomain) + } + if err := ctx.Err(); err != nil { + return nil, fmt.Errorf("dns_delegation update %q: %w", ref.Name, err) + } + if err := d.client.SetNameservers(ctx, currentDomain, s.nameservers); err != nil { + return nil, fmt.Errorf("dns_delegation update %q: %w", ref.Name, err) + } + return delegationOutput(ref.Name, currentDomain, s.nameservers), nil +} + +// hoverDefaultNameservers is the Delete target for v0.2.0 (per A5). +// ResourceRef has no state channel for previous_nameservers restore; +// that enhancement is v0.3.0 follow-up territory. +var hoverDefaultNameservers = []string{"ns1.hover.com", "ns2.hover.com"} + +// Delete resets the registrar nameservers to Hover's defaults. +// Operators whose domains had non-default originals must restore +// manually via the Hover UI if a Delete fires unintended. +func (d *DelegationDriver) Delete(ctx context.Context, ref interfaces.ResourceRef) error { + if err := ctx.Err(); err != nil { + return fmt.Errorf("dns_delegation delete %q: %w", ref.Name, err) + } + domain := ref.ProviderID + if domain == "" { + domain = ref.Name + } + if err := d.client.SetNameservers(ctx, domain, hoverDefaultNameservers); err != nil { + return fmt.Errorf("dns_delegation delete %q: %w", ref.Name, err) + } + return nil +} + +// Diff compares desired vs current. Multiset semantics on nameservers +// (order-independent — Hover accepts any order on PUT). Domain rename +// (desired vs current.ProviderID) forces Replace. +func (d *DelegationDriver) Diff(_ context.Context, desired interfaces.ResourceSpec, current *interfaces.ResourceOutput) (*interfaces.DiffResult, error) { + s, err := parseDelegationSpec(desired) + if err != nil { + return nil, err + } + if current == nil { + return &interfaces.DiffResult{NeedsUpdate: true}, nil + } + if current.ProviderID != "" && !strings.EqualFold(s.domain, current.ProviderID) { + return &interfaces.DiffResult{ + NeedsReplace: true, + Changes: []interfaces.FieldChange{{ + Path: "domain", + Old: current.ProviderID, + New: s.domain, + ForceNew: true, + }}, + }, nil + } + currentNS := nameserversFromOutputs(current.Outputs) + if !sameNameserverSet(currentNS, s.nameservers) { + return &interfaces.DiffResult{ + NeedsUpdate: true, + Changes: []interfaces.FieldChange{{ + Path: "nameservers", + Old: nameserversToAny(currentNS), + New: nameserversToAny(s.nameservers), + }}, + }, nil + } + return &interfaces.DiffResult{NeedsUpdate: false}, nil +} + +// nameserversFromOutputs reconstructs []string from Outputs["nameservers"] +// (which is stored as []any). +func nameserversFromOutputs(outputs map[string]any) []string { + raw, ok := outputs["nameservers"] + if !ok { + return nil + } + list, ok := raw.([]any) + if !ok { + return nil + } + out := make([]string, 0, len(list)) + for _, item := range list { + if s, ok := item.(string); ok { + out = append(out, s) + } + } + return out +} + +// sameNameserverSet returns true iff a and b are multiset-equal. +func sameNameserverSet(a, b []string) bool { + if len(a) != len(b) { + return false + } + sa := append([]string(nil), a...) + sb := append([]string(nil), b...) + sort.Strings(sa) + sort.Strings(sb) + for i := range sa { + if !strings.EqualFold(sa[i], sb[i]) { + return false + } + } + return true +} + +// HealthCheck probes connectivity to the domain by fetching its delegation. +func (d *DelegationDriver) HealthCheck(ctx context.Context, ref interfaces.ResourceRef) (*interfaces.HealthResult, error) { + if err := ctx.Err(); err != nil { + return &interfaces.HealthResult{Healthy: false, Message: err.Error()}, nil + } + domain := ref.ProviderID + if domain == "" { + domain = ref.Name + } + if _, err := d.client.GetDomainDelegation(ctx, domain); err != nil { + return &interfaces.HealthResult{Healthy: false, Message: err.Error()}, nil + } + return &interfaces.HealthResult{Healthy: true, Message: "ok"}, nil +} + +// Scale is not supported for DNS delegation (no replica concept). +func (d *DelegationDriver) Scale(_ context.Context, _ interfaces.ResourceRef, _ int) (*interfaces.ResourceOutput, error) { + return nil, fmt.Errorf("dns_delegation: scale is not supported") +} diff --git a/internal/drivers/delegation_test.go b/internal/drivers/delegation_test.go new file mode 100644 index 0000000..1c7c1bf --- /dev/null +++ b/internal/drivers/delegation_test.go @@ -0,0 +1,360 @@ +package drivers + +import ( + "context" + "errors" + "testing" + + "github.com/GoCodeAlone/workflow-plugin-hover/internal/hover" + "github.com/GoCodeAlone/workflow/interfaces" +) + +type fakeDelegationClient struct { + getResult *hover.DomainDelegation + getErr error + setErr error + lastSetNS []string +} + +func (f *fakeDelegationClient) GetDomainDelegation(_ context.Context, _ string) (*hover.DomainDelegation, error) { + return f.getResult, f.getErr +} + +func (f *fakeDelegationClient) SetNameservers(_ context.Context, _ string, ns []string) error { + f.lastSetNS = append([]string(nil), ns...) + return f.setErr +} + +func TestDelegationDriver_TypeAndProviderIDFormat(t *testing.T) { + d := NewDelegationDriverWithClient(&fakeDelegationClient{}) + if got := d.Type(); got != "infra.dns_delegation" { + t.Errorf("Type() = %q, want infra.dns_delegation", got) + } + if got := d.ProviderIDFormat(); got != interfaces.IDFormatDomainName { + t.Errorf("ProviderIDFormat() = %v, want IDFormatDomainName", got) + } + if d.SensitiveKeys() != nil { + t.Errorf("SensitiveKeys() = %v, want nil", d.SensitiveKeys()) + } +} + +func TestDelegationDriver_Create_CallsSetNameservers(t *testing.T) { + fc := &fakeDelegationClient{} + d := NewDelegationDriverWithClient(fc) + spec := interfaces.ResourceSpec{ + Name: "example.com", + Type: "infra.dns_delegation", + Config: map[string]any{ + "domain": "example.com", + "nameservers": []any{"ns1.do.com", "ns2.do.com", "ns3.do.com"}, + }, + } + out, err := d.Create(context.Background(), spec) + if err != nil { + t.Fatalf("Create: %v", err) + } + if fc.lastSetNS == nil || len(fc.lastSetNS) != 3 { + t.Errorf("client.SetNameservers not called with 3 NS; got %v", fc.lastSetNS) + } + if out.ProviderID != "example.com" { + t.Errorf("ProviderID = %q", out.ProviderID) + } + // Outputs.nameservers MUST be []any, not []string (structpb-safe). + nsRaw, ok := out.Outputs["nameservers"] + if !ok { + t.Fatal("Outputs.nameservers missing") + } + nsAny, ok := nsRaw.([]any) + if !ok { + t.Fatalf("Outputs.nameservers = %T, want []any", nsRaw) + } + if len(nsAny) != 3 { + t.Errorf("Outputs.nameservers len = %d, want 3", len(nsAny)) + } + // previous_nameservers NOT in Outputs for v0.2.0 (no state channel). + if _, present := out.Outputs["previous_nameservers"]; present { + t.Errorf("v0.2.0 Outputs should not contain previous_nameservers") + } +} + +func TestDelegationDriver_Create_MissingDomain_Rejected(t *testing.T) { + d := NewDelegationDriverWithClient(&fakeDelegationClient{}) + spec := interfaces.ResourceSpec{ + Type: "infra.dns_delegation", + Config: map[string]any{ + "nameservers": []any{"a.com", "b.com"}, + }, + } + if _, err := d.Create(context.Background(), spec); err == nil { + t.Fatal("expected error for missing domain") + } +} + +func TestDelegationDriver_Create_MissingNameservers_Rejected(t *testing.T) { + d := NewDelegationDriverWithClient(&fakeDelegationClient{}) + spec := interfaces.ResourceSpec{ + Name: "example.com", + Type: "infra.dns_delegation", + Config: map[string]any{"domain": "example.com"}, + } + if _, err := d.Create(context.Background(), spec); err == nil { + t.Fatal("expected error for missing nameservers") + } +} + +func TestDelegationDriver_Create_DuplicateNameservers_Rejected(t *testing.T) { + d := NewDelegationDriverWithClient(&fakeDelegationClient{}) + spec := interfaces.ResourceSpec{ + Name: "example.com", + Type: "infra.dns_delegation", + Config: map[string]any{ + "domain": "example.com", + "nameservers": []any{"a.com", "a.com"}, + }, + } + if _, err := d.Create(context.Background(), spec); err == nil { + t.Fatal("expected error for duplicate nameservers") + } +} + +func TestDelegationDriver_Read_HappyPath(t *testing.T) { + fc := &fakeDelegationClient{ + getResult: &hover.DomainDelegation{ + ID: "domain-example.com", + Name: "example.com", + Nameservers: []string{"ns1.do.com", "ns2.do.com"}, + }, + } + d := NewDelegationDriverWithClient(fc) + out, err := d.Read(context.Background(), interfaces.ResourceRef{Name: "example.com", ProviderID: "example.com"}) + if err != nil { + t.Fatalf("Read: %v", err) + } + if out.ProviderID != "example.com" { + t.Errorf("ProviderID = %q", out.ProviderID) + } + ns, _ := out.Outputs["nameservers"].([]any) + if len(ns) != 2 { + t.Errorf("nameservers len = %d", len(ns)) + } +} + +func TestDelegationDriver_Read_PropagatesError(t *testing.T) { + fc := &fakeDelegationClient{getErr: errors.New("API down")} + d := NewDelegationDriverWithClient(fc) + _, err := d.Read(context.Background(), interfaces.ResourceRef{Name: "x.com", ProviderID: "x.com"}) + if err == nil { + t.Fatal("expected error") + } +} + +func TestDelegationDriver_Update_HappyPath(t *testing.T) { + fc := &fakeDelegationClient{ + getResult: &hover.DomainDelegation{ + ID: "domain-example.com", Name: "example.com", + Nameservers: []string{"ns1.do.com", "ns2.do.com"}, + }, + } + d := NewDelegationDriverWithClient(fc) + ref := interfaces.ResourceRef{Name: "example.com", Type: "infra.dns_delegation", ProviderID: "example.com"} + spec := interfaces.ResourceSpec{ + Name: "example.com", Type: "infra.dns_delegation", + Config: map[string]any{ + "domain": "example.com", + "nameservers": []any{"ns3.do.com", "ns4.do.com"}, + }, + } + out, err := d.Update(context.Background(), ref, spec) + if err != nil { + t.Fatalf("Update: %v", err) + } + if fc.lastSetNS[0] != "ns3.do.com" { + t.Errorf("first NS = %q", fc.lastSetNS[0]) + } + _ = out +} + +func TestDelegationDriver_Update_DomainRenameRejected(t *testing.T) { + d := NewDelegationDriverWithClient(&fakeDelegationClient{}) + ref := interfaces.ResourceRef{Name: "old.com", ProviderID: "old.com"} + spec := interfaces.ResourceSpec{ + Name: "new.com", Type: "infra.dns_delegation", + Config: map[string]any{ + "domain": "new.com", + "nameservers": []any{"a.com", "b.com"}, + }, + } + if _, err := d.Update(context.Background(), ref, spec); err == nil { + t.Fatal("expected error rejecting domain rename") + } +} + +func TestDelegationDriver_Delete_ResetsToHoverDefaults(t *testing.T) { + // v0.2.0 ships fallback-only Delete: ResourceRef has no state + // channel (verified: workflow/interfaces/iac_provider.go:183-187 + // defines ResourceRef as {Name, Type, ProviderID}). Restore from + // stashed previous_nameservers is a v0.3.0 follow-up requiring + // an interfaces change. + fc := &fakeDelegationClient{} + d := NewDelegationDriverWithClient(fc) + ref := interfaces.ResourceRef{Name: "example.com", ProviderID: "example.com"} + if err := d.Delete(context.Background(), ref); err != nil { + t.Fatalf("Delete: %v", err) + } + if len(fc.lastSetNS) != 2 || fc.lastSetNS[0] != "ns1.hover.com" || fc.lastSetNS[1] != "ns2.hover.com" { + t.Errorf("Delete set NS = %v, want [ns1.hover.com ns2.hover.com]", fc.lastSetNS) + } +} + +func TestDelegationDriver_Diff_NilCurrent(t *testing.T) { + d := NewDelegationDriverWithClient(&fakeDelegationClient{}) + spec := interfaces.ResourceSpec{ + Name: "example.com", Type: "infra.dns_delegation", + Config: map[string]any{ + "domain": "example.com", + "nameservers": []any{"a.com", "b.com"}, + }, + } + res, err := d.Diff(context.Background(), spec, nil) + if err != nil { + t.Fatalf("Diff: %v", err) + } + if !res.NeedsUpdate { + t.Error("expected NeedsUpdate=true for nil current") + } +} + +func TestDelegationDriver_Diff_UpToDate_OrderIndependent(t *testing.T) { + d := NewDelegationDriverWithClient(&fakeDelegationClient{}) + spec := interfaces.ResourceSpec{ + Name: "example.com", Type: "infra.dns_delegation", + Config: map[string]any{ + "domain": "example.com", + "nameservers": []any{"a.com", "b.com", "c.com"}, + }, + } + current := &interfaces.ResourceOutput{ + ProviderID: "example.com", + Outputs: map[string]any{ + "domain": "example.com", + "nameservers": []any{"c.com", "a.com", "b.com"}, // reversed + }, + } + res, err := d.Diff(context.Background(), spec, current) + if err != nil { + t.Fatalf("Diff: %v", err) + } + if res.NeedsUpdate { + t.Error("expected NeedsUpdate=false for same multiset") + } +} + +func TestDelegationDriver_Diff_Changed(t *testing.T) { + d := NewDelegationDriverWithClient(&fakeDelegationClient{}) + spec := interfaces.ResourceSpec{ + Name: "example.com", Type: "infra.dns_delegation", + Config: map[string]any{ + "domain": "example.com", + "nameservers": []any{"new.com", "b.com"}, + }, + } + current := &interfaces.ResourceOutput{ + ProviderID: "example.com", + Outputs: map[string]any{ + "domain": "example.com", + "nameservers": []any{"a.com", "b.com"}, + }, + } + res, err := d.Diff(context.Background(), spec, current) + if err != nil { + t.Fatalf("Diff: %v", err) + } + if !res.NeedsUpdate { + t.Error("expected NeedsUpdate=true") + } +} + +func TestDelegationDriver_Diff_DomainChange_NeedsReplace(t *testing.T) { + d := NewDelegationDriverWithClient(&fakeDelegationClient{}) + spec := interfaces.ResourceSpec{ + Name: "new.com", Type: "infra.dns_delegation", + Config: map[string]any{ + "domain": "new.com", + "nameservers": []any{"a.com", "b.com"}, + }, + } + current := &interfaces.ResourceOutput{ProviderID: "old.com"} + res, err := d.Diff(context.Background(), spec, current) + if err != nil { + t.Fatalf("Diff: %v", err) + } + if !res.NeedsReplace { + t.Error("expected NeedsReplace=true on domain change") + } + if len(res.Changes) != 1 || res.Changes[0].Path != "domain" || !res.Changes[0].ForceNew { + t.Errorf("expected ForceNew domain change, got %+v", res.Changes) + } +} + +func TestDelegationDriver_HealthCheck_Healthy(t *testing.T) { + fc := &fakeDelegationClient{ + getResult: &hover.DomainDelegation{ + ID: "domain-example.com", Name: "example.com", + Nameservers: []string{"a.com", "b.com"}, + }, + } + d := NewDelegationDriverWithClient(fc) + res, err := d.HealthCheck(context.Background(), interfaces.ResourceRef{Name: "example.com", ProviderID: "example.com"}) + if err != nil { + t.Fatalf("HealthCheck: %v", err) + } + if !res.Healthy { + t.Errorf("Healthy = false, want true") + } +} + +func TestDelegationDriver_HealthCheck_Unhealthy(t *testing.T) { + fc := &fakeDelegationClient{getErr: errors.New("boom")} + d := NewDelegationDriverWithClient(fc) + res, err := d.HealthCheck(context.Background(), interfaces.ResourceRef{Name: "example.com", ProviderID: "example.com"}) + if err != nil { + t.Fatalf("HealthCheck should not return err; got %v", err) + } + if res.Healthy { + t.Error("Healthy = true, want false") + } +} + +func TestDelegationDriver_Scale_NotSupported(t *testing.T) { + d := NewDelegationDriverWithClient(&fakeDelegationClient{}) + _, err := d.Scale(context.Background(), interfaces.ResourceRef{Name: "x"}, 3) + if err == nil { + t.Fatal("expected error from Scale") + } +} + +func TestDelegationDriver_CtxCanceled_AllMethods(t *testing.T) { + d := NewDelegationDriverWithClient(&fakeDelegationClient{}) + ref := interfaces.ResourceRef{Name: "example.com", ProviderID: "example.com"} + spec := interfaces.ResourceSpec{ + Name: "example.com", Type: "infra.dns_delegation", + Config: map[string]any{ + "domain": "example.com", + "nameservers": []any{"a.com", "b.com"}, + }, + } + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := d.Create(ctx, spec); err == nil { + t.Error("Create: expected error for canceled ctx") + } + if _, err := d.Read(ctx, ref); err == nil { + t.Error("Read: expected error for canceled ctx") + } + if _, err := d.Update(ctx, ref, spec); err == nil { + t.Error("Update: expected error for canceled ctx") + } + if err := d.Delete(ctx, ref); err == nil { + t.Error("Delete: expected error for canceled ctx") + } +} From 0e2bfe4857f9aaa259e7342c2e1967a930b1675f Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 21:10:39 -0400 Subject: [PATCH 15/20] feat(provider): register DelegationDriver + update Capabilities --- internal/iacserver_test.go | 18 ++++++++++++------ internal/provider.go | 12 ++++++++++-- internal/provider_test.go | 24 ++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 8 deletions(-) create mode 100644 internal/provider_test.go diff --git a/internal/iacserver_test.go b/internal/iacserver_test.go index 3568be1..386de04 100644 --- a/internal/iacserver_test.go +++ b/internal/iacserver_test.go @@ -38,12 +38,18 @@ func TestHoverIaCServer_Capabilities(t *testing.T) { if resp.GetComputePlanVersion() != "v2" { t.Errorf("ComputePlanVersion = %q want %q", resp.GetComputePlanVersion(), "v2") } - if len(resp.GetCapabilities()) != 1 { - t.Fatalf("expected 1 capability, got %d", len(resp.GetCapabilities())) - } - cap := resp.GetCapabilities()[0] - if cap.GetResourceType() != "infra.dns" { - t.Errorf("ResourceType = %q want %q", cap.GetResourceType(), "infra.dns") + caps := resp.GetCapabilities() + if len(caps) != 2 { + t.Fatalf("expected 2 capabilities, got %d", len(caps)) + } + gotTypes := map[string]bool{} + for _, c := range caps { + gotTypes[c.GetResourceType()] = true + } + for _, want := range []string{"infra.dns", "infra.dns_delegation"} { + if !gotTypes[want] { + t.Errorf("capability %q missing", want) + } } } diff --git a/internal/provider.go b/internal/provider.go index a75a1da..a11acf6 100644 --- a/internal/provider.go +++ b/internal/provider.go @@ -19,7 +19,9 @@ import ( var Version = "dev" // HoverProvider implements interfaces.IaCProvider for Hover. -// It supports a single resource type: infra.dns. +// Supports two resource types: +// - infra.dns — DNS records within Hover's nameservers. +// - infra.dns_delegation — registrar-level nameserver delegation. type HoverProvider struct { client *hover.Client drivers map[string]interfaces.ResourceDriver @@ -82,7 +84,8 @@ func (p *HoverProvider) Initialize(ctx context.Context, config map[string]any) e p.client = c p.drivers = map[string]interfaces.ResourceDriver{ - "infra.dns": drivers.NewDNSDriver(c), + "infra.dns": drivers.NewDNSDriver(c), + "infra.dns_delegation": drivers.NewDelegationDriver(c), } return nil } @@ -95,6 +98,11 @@ func (p *HoverProvider) Capabilities() []interfaces.IaCCapabilityDeclaration { Tier: 1, Operations: []string{"create", "read", "update", "delete"}, }, + { + ResourceType: "infra.dns_delegation", + Tier: 1, + Operations: []string{"create", "read", "update", "delete"}, + }, } } diff --git a/internal/provider_test.go b/internal/provider_test.go new file mode 100644 index 0000000..8c942f8 --- /dev/null +++ b/internal/provider_test.go @@ -0,0 +1,24 @@ +package internal + +import ( + "testing" +) + +func TestHoverProvider_Capabilities_IncludesDelegation(t *testing.T) { + p := NewHoverProvider() + caps := p.Capabilities() + wantTypes := map[string]bool{ + "infra.dns": false, + "infra.dns_delegation": false, + } + for _, c := range caps { + if _, ok := wantTypes[c.ResourceType]; ok { + wantTypes[c.ResourceType] = true + } + } + for rt, found := range wantTypes { + if !found { + t.Errorf("Capabilities missing %q", rt) + } + } +} From 308cf2127f016bc630584bfe5f4c10f53ab2fae3 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 21:11:01 -0400 Subject: [PATCH 16/20] feat: plugin.json declares infra.dns_delegation --- plugin.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugin.json b/plugin.json index b4168e7..12362fb 100644 --- a/plugin.json +++ b/plugin.json @@ -24,7 +24,8 @@ "stepTypes": [], "triggerTypes": [], "resourceTypes": [ - "infra.dns" + "infra.dns", + "infra.dns_delegation" ] }, "required_secrets": [ From 6ee76a155babeaa8f7e7498c50a03a5490954d02 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 21:19:26 -0400 Subject: [PATCH 17/20] test(drivers): defend Update Outputs shape + ErrEmptyNameservers wrap + HealthCheck ctx Round-1 review findings: - TestDelegationDriver_Update_HappyPath now asserts out.Outputs["nameservers"] is []any (structpb-safe). Previously discarded the output via `_ = out`. - TestDelegationDriver_CtxCanceled_AllMethods extended to cover HealthCheck (which returns (result, nil) on cancellation; Healthy=false carries the signal). - New TestDelegationDriver_Read_PropagatesErrEmptyNameservers verifies errors.Is(driverErr, hover.ErrEmptyNameservers) survives the driver's error-wrap chain. Callers using the sentinel to distinguish "Hover returned 0 nameservers" from other failures now have a regression harness. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/drivers/delegation_test.go | 37 ++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/internal/drivers/delegation_test.go b/internal/drivers/delegation_test.go index 1c7c1bf..d76e0d8 100644 --- a/internal/drivers/delegation_test.go +++ b/internal/drivers/delegation_test.go @@ -3,6 +3,7 @@ package drivers import ( "context" "errors" + "fmt" "testing" "github.com/GoCodeAlone/workflow-plugin-hover/internal/hover" @@ -171,7 +172,16 @@ func TestDelegationDriver_Update_HappyPath(t *testing.T) { if fc.lastSetNS[0] != "ns3.do.com" { t.Errorf("first NS = %q", fc.lastSetNS[0]) } - _ = out + // Defend the structpb-safe invariant on the Update path too: + // Outputs["nameservers"] MUST be []any, not []string (would + // reject structpb.NewStruct at the gRPC boundary). + nsRaw, ok := out.Outputs["nameservers"] + if !ok { + t.Fatal("Update Outputs.nameservers missing") + } + if _, ok := nsRaw.([]any); !ok { + t.Fatalf("Update Outputs.nameservers = %T, want []any", nsRaw) + } } func TestDelegationDriver_Update_DomainRenameRejected(t *testing.T) { @@ -357,4 +367,29 @@ func TestDelegationDriver_CtxCanceled_AllMethods(t *testing.T) { if err := d.Delete(ctx, ref); err == nil { t.Error("Delete: expected error for canceled ctx") } + // HealthCheck returns (result, nil) on cancellation rather than + // surfacing err — the result's Healthy flag carries the signal. + if res, err := d.HealthCheck(ctx, ref); err != nil { + t.Errorf("HealthCheck: unexpected err for canceled ctx: %v", err) + } else if res.Healthy { + t.Error("HealthCheck: Healthy=true for canceled ctx; expected unhealthy") + } +} + +func TestDelegationDriver_Read_PropagatesErrEmptyNameservers(t *testing.T) { + // Callers using errors.Is(driverErr, hover.ErrEmptyNameservers) to + // distinguish "Hover surfaced 0 nameservers" from other failures need + // the sentinel to survive the driver's error wrap. This test defends + // that contract. + fc := &fakeDelegationClient{ + getErr: fmt.Errorf("hover: GetDomainDelegation %q: %w", "example.com", hover.ErrEmptyNameservers), + } + d := NewDelegationDriverWithClient(fc) + _, err := d.Read(context.Background(), interfaces.ResourceRef{Name: "example.com", ProviderID: "example.com"}) + if err == nil { + t.Fatal("expected error") + } + if !errors.Is(err, hover.ErrEmptyNameservers) { + t.Errorf("errors.Is should match hover.ErrEmptyNameservers through driver wrap; got %v", err) + } } From 42c37cfe5eedb0a4a13f0ab45bdd943e218c407c Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 21:37:01 -0400 Subject: [PATCH 18/20] fix(dns_delegation): case-insensitive dedup in parseDelegationSpec Copilot review finding: parseDelegationSpec used exact-string dedup (seen[s]) while Update + Diff use strings.EqualFold. DNS hostnames are case-insensitive, so ["NS1.example.com","ns1.example.com"] should be rejected as duplicates but slipped through. Lowercase the key when checking + storing in `seen`; preserve the original casing in the parsed output for display. Regression test: TestDelegationDriver_Create_CaseInsensitiveDuplicate_Rejected. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/drivers/delegation.go | 9 ++++++--- internal/drivers/delegation_test.go | 17 +++++++++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/internal/drivers/delegation.go b/internal/drivers/delegation.go index f8de80c..988ee47 100644 --- a/internal/drivers/delegation.go +++ b/internal/drivers/delegation.go @@ -87,10 +87,13 @@ func parseDelegationSpec(spec interfaces.ResourceSpec) (dnsDelegationSpec, error if s == "" { return dnsDelegationSpec{}, fmt.Errorf("dns_delegation %q: nameservers[%d] must be non-empty", domain, i) } - if _, dup := seen[s]; dup { - return dnsDelegationSpec{}, fmt.Errorf("dns_delegation %q: nameservers[%d] = %q is a duplicate", domain, i, s) + // DNS hostnames are case-insensitive; dedupe via lowercase key + // to match the EqualFold semantics used by Update + Diff. + key := strings.ToLower(s) + if _, dup := seen[key]; dup { + return dnsDelegationSpec{}, fmt.Errorf("dns_delegation %q: nameservers[%d] = %q is a duplicate (case-insensitive)", domain, i, s) } - seen[s] = struct{}{} + seen[key] = struct{}{} parsed = append(parsed, s) } return dnsDelegationSpec{domain: domain, nameservers: parsed}, nil diff --git a/internal/drivers/delegation_test.go b/internal/drivers/delegation_test.go index d76e0d8..f2750af 100644 --- a/internal/drivers/delegation_test.go +++ b/internal/drivers/delegation_test.go @@ -393,3 +393,20 @@ func TestDelegationDriver_Read_PropagatesErrEmptyNameservers(t *testing.T) { t.Errorf("errors.Is should match hover.ErrEmptyNameservers through driver wrap; got %v", err) } } + +func TestDelegationDriver_Create_CaseInsensitiveDuplicate_Rejected(t *testing.T) { + // DNS hostnames are case-insensitive; ["NS1.example.com", "ns1.example.com"] + // is a duplicate even though the strings differ. Matches the EqualFold + // semantics used by Update + Diff. + d := NewDelegationDriverWithClient(&fakeDelegationClient{}) + spec := interfaces.ResourceSpec{ + Name: "example.com", Type: "infra.dns_delegation", + Config: map[string]any{ + "domain": "example.com", + "nameservers": []any{"NS1.example.com", "ns1.example.com"}, + }, + } + if _, err := d.Create(context.Background(), spec); err == nil { + t.Fatal("expected error for case-insensitive duplicate nameservers") + } +} From c09391bf2e55cbf23c3594d65d878305529fde29 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 21:49:19 -0400 Subject: [PATCH 19/20] fix: sameNameserverSet normalizes before sort; SetNameservers lock-hold comment PR #5 round-2 Copilot findings: 1. sameNameserverSet sorted case-sensitively then compared with strings.EqualFold. Mixed-case multisets could falsely diverge because sort positions disagreed with the case-insensitive compare. Lowercased all entries before sort; direct == compare suffices after normalize. Regression test: TestDelegationDriver_Diff_CaseInsensitiveMatch. 2. SetNameservers lock-hold comment understated worst-case duration: ensureLoginLocked may run GET /signin + POST /signin + GET /signin/totp + (optional) POST /signin/totp on stale sessions. Updated comment to enumerate the full request list and note ~180s worst-case vs ~60s warm-session. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/drivers/delegation.go | 17 ++++++++++++++--- internal/drivers/delegation_test.go | 28 ++++++++++++++++++++++++++++ internal/hover/client.go | 18 ++++++++++++++---- 3 files changed, 56 insertions(+), 7 deletions(-) diff --git a/internal/drivers/delegation.go b/internal/drivers/delegation.go index 988ee47..df3e983 100644 --- a/internal/drivers/delegation.go +++ b/internal/drivers/delegation.go @@ -270,12 +270,23 @@ func sameNameserverSet(a, b []string) bool { if len(a) != len(b) { return false } - sa := append([]string(nil), a...) - sb := append([]string(nil), b...) + // Normalize to lowercase before sorting so the pairwise sort + // positions are consistent with the EqualFold comparison. + // Without normalize-then-sort, ["NS.foo"] sorts before ["ns.bar"] + // case-sensitively but the case-insensitive compare expects the + // reverse — causing two case-equal multisets to falsely diverge. + sa := make([]string, len(a)) + sb := make([]string, len(b)) + for i, s := range a { + sa[i] = strings.ToLower(s) + } + for i, s := range b { + sb[i] = strings.ToLower(s) + } sort.Strings(sa) sort.Strings(sb) for i := range sa { - if !strings.EqualFold(sa[i], sb[i]) { + if sa[i] != sb[i] { return false } } diff --git a/internal/drivers/delegation_test.go b/internal/drivers/delegation_test.go index f2750af..8fc7452 100644 --- a/internal/drivers/delegation_test.go +++ b/internal/drivers/delegation_test.go @@ -410,3 +410,31 @@ func TestDelegationDriver_Create_CaseInsensitiveDuplicate_Rejected(t *testing.T) t.Fatal("expected error for case-insensitive duplicate nameservers") } } + +func TestDelegationDriver_Diff_CaseInsensitiveMatch(t *testing.T) { + // Same hostnames in different cases must match (DNS is + // case-insensitive). Regresses a sort-vs-EqualFold sequencing + // bug where mixed-case multisets could falsely diverge. + d := NewDelegationDriverWithClient(&fakeDelegationClient{}) + spec := interfaces.ResourceSpec{ + Name: "example.com", Type: "infra.dns_delegation", + Config: map[string]any{ + "domain": "example.com", + "nameservers": []any{"NS1.example.com", "ns2.example.com"}, + }, + } + current := &interfaces.ResourceOutput{ + ProviderID: "example.com", + Outputs: map[string]any{ + "domain": "example.com", + "nameservers": []any{"ns1.EXAMPLE.com", "NS2.example.com"}, + }, + } + res, err := d.Diff(context.Background(), spec, current) + if err != nil { + t.Fatalf("Diff: %v", err) + } + if res.NeedsUpdate { + t.Error("expected NeedsUpdate=false; case-only diff is no-op") + } +} diff --git a/internal/hover/client.go b/internal/hover/client.go index f5ac509..d3dc4d8 100644 --- a/internal/hover/client.go +++ b/internal/hover/client.go @@ -333,10 +333,20 @@ func (c *Client) GetDomainDelegation(ctx context.Context, domainName string) (*D // between the two requests). // // Trade-off: any concurrent caller using the same *Client blocks for -// up to ~60s (two HTTP round-trips under the 30s default client timeout). -// Acceptable for the field-test scope (single goroutine, one delegation -// resource). Future: cache CSRF at session granularity if mixed-resource -// throughput becomes a concern. +// the full duration of the held-lock sequence. Worst case (session is +// stale and re-auth fires inside ensureLoginLocked): +// - GET /signin (CSRF for the form) +// - POST /signin (credentials) +// - GET /signin/totp (MFA probe) +// - POST /signin/totp (TOTP code, only if MFA enabled) +// - GET /control_panel/domain/ (CSRF for the API write) +// - PUT /api/control_panel/domains/domain- +// +// Up to ~180s at the 30s default per-request timeout when re-auth is +// needed; ~60s on the warm-session path (CSRF GET + PUT). Acceptable +// for the field-test scope (single goroutine, one delegation +// resource). Future: cache CSRF at session granularity if +// mixed-resource throughput becomes a concern. func (c *Client) SetNameservers(ctx context.Context, domainName string, ns []string) error { c.mu.Lock() defer c.mu.Unlock() From ef96744c287b3a4773cda2cfc68621346034fa41 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 22:01:09 -0400 Subject: [PATCH 20/20] fix: csrfMeta attr-order/quote-style agnostic + Diff sets NeedsUpdate PR #5 round-3 Copilot findings: 1. csrfMetaRe assumed name=before-content + double quotes. Hover/ Rails may emit content=before-name or single quotes. Replaced with two patterns covering both attribute orders and both quote styles, wrapped in extractCSRFMeta() helper. Test: TestExtractCSRFMeta_AttributeOrders covers all 4 shapes + the missing case. 2. DelegationDriver.Diff returned NeedsReplace=true with NeedsUpdate at zero. DNSDriver pattern sets both; some planner paths gate on NeedsUpdate, risking a skipped replace. Now sets both for domain-change. Test: TestDelegationDriver_Diff_DomainChange_SetsBothNeedsUpdateAndReplace. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/drivers/delegation.go | 5 ++++ internal/drivers/delegation_test.go | 24 +++++++++++++++++++ internal/hover/client.go | 37 +++++++++++++++++++++-------- internal/hover/client_test.go | 15 ++++++++++++ 4 files changed, 71 insertions(+), 10 deletions(-) diff --git a/internal/drivers/delegation.go b/internal/drivers/delegation.go index df3e983..9a0de46 100644 --- a/internal/drivers/delegation.go +++ b/internal/drivers/delegation.go @@ -221,7 +221,12 @@ func (d *DelegationDriver) Diff(_ context.Context, desired interfaces.ResourceSp return &interfaces.DiffResult{NeedsUpdate: true}, nil } if current.ProviderID != "" && !strings.EqualFold(s.domain, current.ProviderID) { + // Set BOTH NeedsUpdate + NeedsReplace per the DNSDriver + // pattern (see dns.go). Some planner paths gate on + // NeedsUpdate; leaving it false risks the replace being + // skipped even though NeedsReplace says otherwise. return &interfaces.DiffResult{ + NeedsUpdate: true, NeedsReplace: true, Changes: []interfaces.FieldChange{{ Path: "domain", diff --git a/internal/drivers/delegation_test.go b/internal/drivers/delegation_test.go index 8fc7452..3ad4fec 100644 --- a/internal/drivers/delegation_test.go +++ b/internal/drivers/delegation_test.go @@ -438,3 +438,27 @@ func TestDelegationDriver_Diff_CaseInsensitiveMatch(t *testing.T) { t.Error("expected NeedsUpdate=false; case-only diff is no-op") } } + +func TestDelegationDriver_Diff_DomainChange_SetsBothNeedsUpdateAndReplace(t *testing.T) { + // Planners may gate on NeedsUpdate; setting only NeedsReplace + // risks the replace being skipped. Match DNSDriver pattern. + d := NewDelegationDriverWithClient(&fakeDelegationClient{}) + spec := interfaces.ResourceSpec{ + Name: "new.com", Type: "infra.dns_delegation", + Config: map[string]any{ + "domain": "new.com", + "nameservers": []any{"a.com", "b.com"}, + }, + } + current := &interfaces.ResourceOutput{ProviderID: "old.com"} + res, err := d.Diff(context.Background(), spec, current) + if err != nil { + t.Fatalf("Diff: %v", err) + } + if !res.NeedsReplace { + t.Error("NeedsReplace=false") + } + if !res.NeedsUpdate { + t.Error("NeedsUpdate=false; should be true alongside NeedsReplace") + } +} diff --git a/internal/hover/client.go b/internal/hover/client.go index d3dc4d8..61b713f 100644 --- a/internal/hover/client.go +++ b/internal/hover/client.go @@ -146,14 +146,31 @@ func (c *Client) ensureLoginLocked(ctx context.Context) error { var csrfRe = regexp.MustCompile(`]+name="_token"[^>]+value="([^"]+)"`) -// csrfMetaRe extracts the Rails CSRF meta token from a control-panel HTML -// page. Distinct from csrfRe (form-token regex used by the /signin flow) -// because the control-panel pages embed the token as a meta tag for the -// SPA layer to read, while /signin embeds it as a hidden input. +// CSRF meta-tag extraction. Distinct from csrfRe (form-token regex used +// by the /signin flow) because the control-panel pages embed the token +// as a meta tag for the SPA layer to read, while /signin embeds it as a +// hidden input. Both shapes coexist in the Hover-served HTML; each is +// matched from the page where it's authoritative. // -// Both shapes coexist in the Hover-served HTML; we match each from the -// page where it's authoritative. -var csrfMetaRe = regexp.MustCompile(`= 2 { + return string(m[1]) + } + if m := csrfMetaReContentFirst.FindSubmatch(body); len(m) >= 2 { + return string(m[1]) + } + return "" +} // fetchControlPanelCSRFLocked retrieves the meta-tag CSRF token from // /control_panel/domain/. Caller MUST hold c.mu (so the HTTP GET @@ -175,11 +192,11 @@ func (c *Client) fetchControlPanelCSRFLocked(ctx context.Context, domainName str if err != nil { return "", fmt.Errorf("hover: fetch control_panel CSRF for %q: read body: %w", domainName, err) } - m := csrfMetaRe.FindSubmatch(body) - if len(m) < 2 { + token := extractCSRFMeta(body) + if token == "" { return "", fmt.Errorf("hover: CSRF meta tag not found at /control_panel/domain/%s (control_panel UI changed?)", domainName) } - return string(m[1]), nil + return token, nil } func (c *Client) fetchSignInCSRF(ctx context.Context) (string, error) { diff --git a/internal/hover/client_test.go b/internal/hover/client_test.go index 9827ee9..e838f44 100644 --- a/internal/hover/client_test.go +++ b/internal/hover/client_test.go @@ -584,3 +584,18 @@ func TestClient_ListRecords_DomainNotFound(t *testing.T) { t.Errorf("unexpected error: %v", err) } } + +func TestExtractCSRFMeta_AttributeOrders(t *testing.T) { + cases := []struct{ name, html, want string }{ + {"name-first double quotes", ``, "abc"}, + {"content-first double quotes", ``, "xyz"}, + {"name-first single quotes", ``, "qqq"}, + {"content-first single quotes", ``, "zzz"}, + {"missing", ``, ""}, + } + for _, tc := range cases { + if got := extractCSRFMeta([]byte(tc.html)); got != tc.want { + t.Errorf("%s: got %q want %q", tc.name, got, tc.want) + } + } +}