From 59bf126b4d57f133a4cd3f1471b2aea1adba4421 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 15:13:44 -0400 Subject: [PATCH 1/7] chore: add release workflow + goreleaser config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GoReleaser v2 builds linux/darwin × amd64/arm64 binaries + tar.gz archives + checksums; release.yml fires on `v*` tags. plugin.json is rewritten with the tag version + asset URLs during goreleaser's before-hook so the in-archive plugin.json matches the published download URLs. Requires the repo to have a `RELEASES_TOKEN` secret set (PAT or fine-grained token with `contents:read` on the GoCodeAlone org for private dep resolution). Without it, the goreleaser step will fail at the git-config-insteadOf line — that's the signal to add the secret. After tagging v0.1.0 and the release publishes, register the plugin in workflow-registry with the published asset SHAs. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/release.yml | 31 +++++++++++++++++++++++ .goreleaser.yaml | 46 +++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+) create mode 100644 .github/workflows/release.yml create mode 100644 .goreleaser.yaml diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml new file mode 100644 index 0000000..e81105c --- /dev/null +++ b/.github/workflows/release.yml @@ -0,0 +1,31 @@ +name: Release +on: + push: + tags: + - 'v*' +permissions: + contents: write + id-token: write +jobs: + release: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - uses: actions/setup-go@v5 + with: + go-version-file: go.mod + - name: Configure Git for private repos + run: git config --global url."https://x-access-token:${{ secrets.RELEASES_TOKEN }}@github.com/".insteadOf "https://github.com/" + - uses: goreleaser/goreleaser-action@v7 + with: + distribution: goreleaser + version: '~> v2' + args: release --clean + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + - name: Publish release (was draft during asset upload) + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: gh release edit ${{ github.ref_name }} --draft=false --repo ${{ github.repository }} diff --git a/.goreleaser.yaml b/.goreleaser.yaml new file mode 100644 index 0000000..d1cafc6 --- /dev/null +++ b/.goreleaser.yaml @@ -0,0 +1,46 @@ +version: 2 + +project_name: workflow-plugin-hover + +before: + hooks: + - "sh -c \"rm -rf .release && mkdir -p .release && cp plugin.json .release/plugin.json && sed -i.bak 's/\\\"version\\\": \\\".*\\\"/\\\"version\\\": \\\"{{ .Version }}\\\"/' .release/plugin.json && rm -f .release/plugin.json.bak\"" + - "sh -c \"sed -i.bak 's|/releases/download/v[^/]*/|/releases/download/{{ .Tag }}/|g' .release/plugin.json && rm -f .release/plugin.json.bak\"" + +builds: + - id: workflow-plugin-hover + main: ./cmd/workflow-plugin-hover + binary: workflow-plugin-hover + env: + - CGO_ENABLED=0 + - GOPRIVATE=github.com/GoCodeAlone/* + goos: + - linux + - darwin + goarch: + - amd64 + - arm64 + ldflags: + - -s -w -X github.com/GoCodeAlone/workflow-plugin-hover/internal.Version={{.Version}} + +archives: + - id: workflow-plugin-hover + ids: + - workflow-plugin-hover + formats: [tar.gz] + name_template: "{{ .ProjectName }}-{{ .Os }}-{{ .Arch }}" + files: + - src: .release/plugin.json + dst: plugin.json + +checksum: + name_template: "checksums.txt" + +changelog: + sort: asc + +release: + github: + owner: GoCodeAlone + name: workflow-plugin-hover + draft: true From e565925d9e459c36567ffe0f382c40b46662ff97 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 15:27:51 -0400 Subject: [PATCH 2/7] feat(dns): prune orphan records on apply (closes #3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit upsertRecords now deletes records that exist upstream but are not in the desired config, completing the convergence loop. Previously orphans persisted indefinitely; the README documented the gap as a "Limitations" entry. Implementation: - Removed the `len(desired) == 0 → early return` short-circuit so an explicit empty `records: []` flows into the prune step. - Removed the `existingByKey[key] = append(existingByKey[key], *created)` post-create write that incorrectly re-introduced newly-created records into the leftover candidate set — it would trip the prune sweep below and delete the just- created record. Drop both the write and the unused `created` return value. - After the upsert pass, iterate the still-populated existingByKey and call client.DeleteRecord(ctx, id) on every leftover. Error wrapping cites the orphan's type/name/id/domain so failures point at the offending row. Stale Diff comment about "upsertRecords does not currently prune the extras (separate follow-up; see Limitations)" rewritten to match the new behavior. README "Limitations" entry removed; the "No zone delete" caveat remains. Tests: - TestUpsertRecords_PrunesExtraRecords (apex stays, orphan deleted) - TestUpsertRecords_EmptyDesiredDeletesAll (explicit `records: []` deletes everything) Co-Authored-By: Claude Opus 4.7 (1M context) --- README.md | 9 ++----- internal/drivers/dns.go | 35 +++++++++++++++++-------- internal/drivers/dns_test.go | 51 ++++++++++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index 2c23738..961ee8e 100644 --- a/README.md +++ b/README.md @@ -74,15 +74,10 @@ against [RFC 6238 Appendix B vectors](https://datatracker.ietf.org/doc/html/rfc6 ## Limitations -- **No prune on apply**: `upsertRecords` only adds/updates. Records - that exist upstream but are not in the desired config are NOT - deleted on `apply`. `Diff` does flag them (so Plan reports drift), - but converging the actual record set requires manually deleting - the orphan records via Hover's UI or via a future explicit prune - path. Track follow-up via the project issue list. - **No zone delete**: Hover exposes no API to drop a DNS zone. Resource `Delete` is a no-op — the IaC state is cleared but - upstream records remain. + upstream records remain. Operators who want to drop the zone + must do so manually via Hover's UI. ## Development diff --git a/internal/drivers/dns.go b/internal/drivers/dns.go index b3d1b7f..41b4dde 100644 --- a/internal/drivers/dns.go +++ b/internal/drivers/dns.go @@ -185,10 +185,9 @@ func (d *DNSDriver) Diff(_ context.Context, desired interfaces.ResourceSpec, cur currentByKey[key] = append(candidates[:idx], candidates[idx+1:]...) } // Any remaining candidates are records that exist upstream but - // are not in the desired set. Treat that as drift so the engine - // surfaces it during Plan, even though upsertRecords does not - // currently prune the extras (an explicit prune path is a - // separate follow-up; see the "Limitations" section of README.md). + // are not in the desired set. Treat that as drift so Plan + // surfaces the change to the operator; upsertRecords prunes + // them during apply. for _, leftover := range currentByKey { if len(leftover) > 0 { return &interfaces.DiffResult{NeedsUpdate: true}, nil @@ -229,9 +228,9 @@ func (d *DNSDriver) readOutput(ctx context.Context, domain, name string) (*inter } func (d *DNSDriver) upsertRecords(ctx context.Context, domain string, desired []hover.DNSRecord) error { - if len(desired) == 0 { - return nil - } + // An empty desired set means "drop everything" — fall through into + // the prune step rather than short-circuiting. Without this, an + // explicit `records: []` would still leave upstream records intact. // Hover's POST /api/dns endpoint requires `domain_id` (hover-assigned // numeric ID), NOT the apex name. Resolve the domain up front via @@ -293,12 +292,26 @@ func (d *DNSDriver) upsertRecords(ctx context.Context, domain string, desired [] return fmt.Errorf("hover dns update %s/%s %q: %w", dr.Type, dr.Name, domain, err) } } else { - created, err := d.client.CreateRecord(ctx, dom.ID, dr) - if err != nil { + if _, err := d.client.CreateRecord(ctx, dom.ID, dr); err != nil { return fmt.Errorf("hover dns create %s/%s %q: %w", dr.Type, dr.Name, domain, err) } - if created != nil { - existingByKey[key] = append(existingByKey[key], *created) + // Do NOT add the created record back into existingByKey. + // existingByKey is the upstream set we're reconciling + // against; the create is a NEW record, not a leftover. + // Adding it would trip the prune sweep below and delete + // the record we just created. + } + } + + // Prune: any candidates still in existingByKey after both passes + // are upstream records that no longer appear in the desired config. + // Delete them so the upstream zone converges to the declared set. + // (Hover has no whole-zone replace API; this per-record delete is + // the only path to convergence.) + for _, leftovers := range existingByKey { + for _, orphan := range leftovers { + if err := d.client.DeleteRecord(ctx, orphan.ID); err != nil { + return fmt.Errorf("hover dns prune %s/%s %q (id=%s): %w", orphan.Type, orphan.Name, domain, orphan.ID, err) } } } diff --git a/internal/drivers/dns_test.go b/internal/drivers/dns_test.go index 7ccb829..5085f50 100644 --- a/internal/drivers/dns_test.go +++ b/internal/drivers/dns_test.go @@ -531,3 +531,54 @@ func TestDiff_EmptyDesired_WithCurrentRecords_NeedsUpdate(t *testing.T) { t.Error("expected NeedsUpdate=true when desired is empty but current has records") } } + +// TestUpsertRecords_PrunesExtraRecords verifies that records in the +// upstream zone that don't appear in the desired config are deleted +// during apply. Regresses the "no prune on apply" gap that left +// removed records as orphans upstream. +func TestUpsertRecords_PrunesExtraRecords(t *testing.T) { + fc := &fakeClient{ + records: []hover.DNSRecord{ + {ID: "r1", Type: "A", Name: "@", Content: "1.1.1.1"}, + {ID: "r2", Type: "A", Name: "www", Content: "1.1.1.1"}, // orphan + }, + } + d := NewDNSDriverWithClient(fc) + spec := interfaces.ResourceSpec{ + Name: "example.com", Type: "infra.dns", + Config: map[string]any{ + "records": []any{ + map[string]any{"type": "A", "name": "@", "content": "1.1.1.1"}, + }, + }, + } + if _, err := d.Update(context.Background(), interfaces.ResourceRef{Name: "example.com", ProviderID: "example.com"}, spec); err != nil { + t.Fatalf("Update: %v", err) + } + if len(fc.records) != 1 { + t.Fatalf("expected upstream to converge to 1 record after prune; got %d: %+v", len(fc.records), fc.records) + } + if fc.records[0].Name != "@" { + t.Errorf("expected the apex record to remain; got %+v", fc.records[0]) + } +} + +func TestUpsertRecords_EmptyDesiredDeletesAll(t *testing.T) { + fc := &fakeClient{ + records: []hover.DNSRecord{ + {ID: "r1", Type: "A", Name: "@", Content: "1.1.1.1"}, + {ID: "r2", Type: "A", Name: "www", Content: "1.1.1.1"}, + }, + } + d := NewDNSDriverWithClient(fc) + spec := interfaces.ResourceSpec{ + Name: "example.com", Type: "infra.dns", + Config: map[string]any{"records": []any{}}, + } + if _, err := d.Update(context.Background(), interfaces.ResourceRef{Name: "example.com", ProviderID: "example.com"}, spec); err != nil { + t.Fatalf("Update: %v", err) + } + if len(fc.records) != 0 { + t.Errorf("expected all upstream records pruned; got %d: %+v", len(fc.records), fc.records) + } +} From f62958210bb292ab8519a41dbad12bcde3617ff3 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 15:52:40 -0400 Subject: [PATCH 3/7] fix: address PR #4 round-5 Copilot findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - release.yml: drop unused `id-token: write` permission. No OIDC step in the workflow. - .goreleaser.yaml: drop the second before-hook (URL rewrite in .release/plugin.json) — current plugin.json contains no releases/download URLs, so the hook was a no-op. The version- rewrite hook (first one) stays. - declaredRecords: require config.records explicitly. Missing key now returns a typed error pointing the operator at `records: []` for the explicit-prune-everything case. Without this, a caller that forgot the records key would silently prune every upstream record on apply. - Diff: stale comment about "may report drift the engine cannot satisfy" rewritten to match the new prune-on-apply behavior. Tests updated for the records-required validation: - TestDNSDriver_Create_Empty: now uses explicit `records: []`. - TestDNSDriver_Diff_DomainChange_ForceReplace: same. - TestDNSDriver_Update_DomainRenameRejected: same. - New TestDNSDriver_Create_MissingRecordsKey_Rejected covers the typed-error path. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/release.yml | 1 - .goreleaser.yaml | 1 - internal/drivers/dns.go | 21 +++++++++++---------- internal/drivers/dns_test.go | 17 ++++++++++++++--- 4 files changed, 25 insertions(+), 15 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index e81105c..81249ec 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -5,7 +5,6 @@ on: - 'v*' permissions: contents: write - id-token: write jobs: release: runs-on: ubuntu-latest diff --git a/.goreleaser.yaml b/.goreleaser.yaml index d1cafc6..5aab141 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -5,7 +5,6 @@ project_name: workflow-plugin-hover before: hooks: - "sh -c \"rm -rf .release && mkdir -p .release && cp plugin.json .release/plugin.json && sed -i.bak 's/\\\"version\\\": \\\".*\\\"/\\\"version\\\": \\\"{{ .Version }}\\\"/' .release/plugin.json && rm -f .release/plugin.json.bak\"" - - "sh -c \"sed -i.bak 's|/releases/download/v[^/]*/|/releases/download/{{ .Tag }}/|g' .release/plugin.json && rm -f .release/plugin.json.bak\"" builds: - id: workflow-plugin-hover diff --git a/internal/drivers/dns.go b/internal/drivers/dns.go index 41b4dde..4aeb06e 100644 --- a/internal/drivers/dns.go +++ b/internal/drivers/dns.go @@ -134,13 +134,7 @@ func (d *DNSDriver) Diff(_ context.Context, desired interfaces.ResourceSpec, cur // Empty desired record set with no current records → in sync. // Empty desired with leftover current records → drift (everything - // extra needs deletion). Note: upsertRecords today does NOT delete - // extras (it only adds/updates), so this Diff signal currently - // produces a NeedsUpdate the engine cannot fully satisfy. The - // alternative — silently letting extras persist — is worse: the - // declared spec never matches reality. Operators who want strict - // pruning need to either explicitly add `Delete` plumbing or - // document the gap; flagging it is the right Plan signal. + // extra needs deletion); upsertRecords prunes them during apply. if len(desiredRecs) == 0 { if len(currentRecs) == 0 { return &interfaces.DiffResult{NeedsUpdate: false}, nil @@ -348,10 +342,17 @@ func domainFromConfigIfPresent(config map[string]any) (string, bool, error) { } // declaredRecords parses config["records"] into a []hover.DNSRecord slice. +// +// `records` is REQUIRED. A missing key errors out — silently coercing +// to an empty slice would let upsertRecords prune every upstream +// record, which is rarely what an operator intends when they forgot +// to set the key. An explicitly empty `records: []` IS allowed (and +// does deliberately prune everything); only the missing-key / +// wrong-type cases are rejected. func declaredRecords(config map[string]any) ([]hover.DNSRecord, error) { - raw, ok := config["records"] - if !ok { - return nil, nil + raw, present := config["records"] + if !present { + return nil, fmt.Errorf("hover dns: config.records is required (use an explicit 'records: []' to drop every record)") } items, err := toSliceOfMaps(raw) if err != nil { diff --git a/internal/drivers/dns_test.go b/internal/drivers/dns_test.go index 5085f50..ca8061e 100644 --- a/internal/drivers/dns_test.go +++ b/internal/drivers/dns_test.go @@ -92,8 +92,11 @@ func newDriver(records ...hover.DNSRecord) (*DNSDriver, *fakeClient) { } func TestDNSDriver_Create_Empty(t *testing.T) { + // Explicitly-empty records list is the supported way to declare + // "no DNS records on this zone". A missing records key is now an + // error (would otherwise silently prune everything upstream). d, _ := newDriver() - spec := interfaces.ResourceSpec{Name: "example.com", Type: "infra.dns", Config: map[string]any{}} + spec := interfaces.ResourceSpec{Name: "example.com", Type: "infra.dns", Config: map[string]any{"records": []any{}}} out, err := d.Create(context.Background(), spec) if err != nil { t.Fatalf("Create: %v", err) @@ -103,6 +106,14 @@ func TestDNSDriver_Create_Empty(t *testing.T) { } } +func TestDNSDriver_Create_MissingRecordsKey_Rejected(t *testing.T) { + d, _ := newDriver() + spec := interfaces.ResourceSpec{Name: "example.com", Type: "infra.dns", Config: map[string]any{}} + if _, err := d.Create(context.Background(), spec); err == nil { + t.Fatal("expected error for missing config.records key") + } +} + func TestDNSDriver_Create_WithRecords(t *testing.T) { d, fc := newDriver() spec := interfaces.ResourceSpec{ @@ -218,7 +229,7 @@ func TestDNSDriver_Diff_DomainChange_ForceReplace(t *testing.T) { spec := interfaces.ResourceSpec{ Name: "new.com", Type: "infra.dns", - Config: map[string]any{"domain": "new.com"}, + Config: map[string]any{"domain": "new.com", "records": []any{}}, } current := &interfaces.ResourceOutput{ProviderID: "old.com"} diff, err := d.Diff(context.Background(), spec, current) @@ -246,7 +257,7 @@ func TestDNSDriver_Update_DomainRenameRejected(t *testing.T) { d, _ := newDriver() spec := interfaces.ResourceSpec{ Name: "new.com", Type: "infra.dns", - Config: map[string]any{"domain": "new.com"}, + Config: map[string]any{"domain": "new.com", "records": []any{}}, } ref := interfaces.ResourceRef{Name: "old.com", ProviderID: "old.com"} _, err := d.Update(context.Background(), ref, spec) From 387794bf21016550d048d443ef9fae54b769e5c1 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 16:04:18 -0400 Subject: [PATCH 4/7] =?UTF-8?q?fix:=20PR=20#4=20round-6=20=E2=80=94=20scop?= =?UTF-8?q?e=20token,=20fail-fast,=20align=20action=20versions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Git insteadOf rewrite now scoped to https://github.com/GoCodeAlone/ only. Previously the rewrite matched all https://github.com/ traffic, broadening where the RELEASES_TOKEN could be sent (e.g. transitive go module fetches from third-party orgs would have presented the token). - Fail-fast when RELEASES_TOKEN is unset. Without this, the git config line would silently install a rewrite with empty credentials and the subsequent goreleaser fetch of public GoCodeAlone repos would fail with a confusing auth error. - actions/checkout@v5 + actions/setup-go@v6 to match ci.yml's pins. Reduces drift between the two workflows. (PR description rewritten separately on the GitHub side to drop the inaccurate "asset URL rewrite" claim — the goreleaser hook only rewrites the version field, since plugin.json has no releases/download URLs to rewrite.) Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/release.yml | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 81249ec..4b6d752 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -9,14 +9,21 @@ jobs: release: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 with: fetch-depth: 0 - - uses: actions/setup-go@v5 + - uses: actions/setup-go@v6 with: go-version-file: go.mod - - name: Configure Git for private repos - run: git config --global url."https://x-access-token:${{ secrets.RELEASES_TOKEN }}@github.com/".insteadOf "https://github.com/" + - name: Configure Git for private GoCodeAlone repos + env: + RELEASES_TOKEN: ${{ secrets.RELEASES_TOKEN }} + run: | + if [ -z "$RELEASES_TOKEN" ]; then + echo "::error::RELEASES_TOKEN secret is not set. The goreleaser step needs it to resolve private GoCodeAlone module deps." >&2 + exit 1 + fi + git config --global url."https://x-access-token:${RELEASES_TOKEN}@github.com/GoCodeAlone/".insteadOf "https://github.com/GoCodeAlone/" - uses: goreleaser/goreleaser-action@v7 with: distribution: goreleaser From 88458b810c33335c8e028e2d9968875110966e87 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 16:17:15 -0400 Subject: [PATCH 5/7] =?UTF-8?q?fix:=20PR=20#4=20round-7=20=E2=80=94=20Diff?= =?UTF-8?q?=20validates=20config;=20README=20docs=20prune?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Diff: call declaredRecords() up front so config errors (e.g., missing required `records` key, wrong-type value) surface at Plan time for NEW resources too. Previously current==nil short-circuited with NeedsUpdate=true and the validation didn't run until Apply tried to read the config. Existing test updated to use explicit `records: []`; new TestDNSDriver_Diff_MissingRecordsKey_ErrorsAtPlanTime covers the validation-at-Plan-time guarantee. - README Configuration section: spell out the declared-set-is-authoritative contract (upstream-only records are deleted; declared-only are created; differing are updated) and the "records: required; use empty list to wipe" invariant. Operators omitting the key would otherwise be surprised by the typed-error rejection. Co-Authored-By: Claude Opus 4.7 (1M context) --- README.md | 9 +++++++++ internal/drivers/dns.go | 14 ++++++++++---- internal/drivers/dns_test.go | 13 ++++++++++++- 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 961ee8e..8a92651 100644 --- a/README.md +++ b/README.md @@ -43,6 +43,15 @@ resources: - { type: CNAME, name: 'www', content: example.com., ttl: 900 } ``` +The `records` key is **required**. The plugin treats your declared +list as authoritative: on apply, records present upstream but +absent from `records` are deleted, records present in `records` +but absent upstream are created, and records that differ are +updated. To deliberately drop every record from a zone, set +`records: []` — that explicit empty list is the only way to ask +for a wipe. Omitting `records` entirely is rejected at Plan time +to avoid the "I forgot the key and lost my zone" failure mode. + ## Required secrets | Name | Sensitive | Source | diff --git a/internal/drivers/dns.go b/internal/drivers/dns.go index 4aeb06e..af41ccf 100644 --- a/internal/drivers/dns.go +++ b/internal/drivers/dns.go @@ -105,6 +105,16 @@ func (d *DNSDriver) Delete(_ context.Context, _ interfaces.ResourceRef) error { } func (d *DNSDriver) Diff(_ context.Context, desired interfaces.ResourceSpec, current *interfaces.ResourceOutput) (*interfaces.DiffResult, error) { + // Validate the desired spec up front so config errors surface at + // Plan time, even for brand-new resources (current == nil) where + // the engine would otherwise just see NeedsUpdate=true and let + // Apply discover the same problem one stage later. declaredRecords + // also covers the "config.records is required" check. + desiredRecs, err := declaredRecords(desired.Config) + if err != nil { + return nil, err + } + if current == nil { return &interfaces.DiffResult{NeedsUpdate: true}, nil } @@ -123,10 +133,6 @@ func (d *DNSDriver) Diff(_ context.Context, desired interfaces.ResourceSpec, cur }, nil } - desiredRecs, err := declaredRecords(desired.Config) - if err != nil { - return nil, err - } currentRecs, err := dnsRecordsFromOutput(current) if err != nil { return nil, err diff --git a/internal/drivers/dns_test.go b/internal/drivers/dns_test.go index ca8061e..90f86bd 100644 --- a/internal/drivers/dns_test.go +++ b/internal/drivers/dns_test.go @@ -159,8 +159,11 @@ func TestDNSDriver_Create_UpdatesExistingRecord(t *testing.T) { } func TestDNSDriver_Diff_NilCurrent(t *testing.T) { + // Diff now validates config.records up front so config errors + // surface at Plan time even for new resources. Use an explicit + // empty records list to exercise the nil-current early-return path. d, _ := newDriver() - spec := interfaces.ResourceSpec{Name: "example.com", Type: "infra.dns", Config: map[string]any{}} + spec := interfaces.ResourceSpec{Name: "example.com", Type: "infra.dns", Config: map[string]any{"records": []any{}}} diff, err := d.Diff(context.Background(), spec, nil) if err != nil { t.Fatalf("Diff: %v", err) @@ -170,6 +173,14 @@ func TestDNSDriver_Diff_NilCurrent(t *testing.T) { } } +func TestDNSDriver_Diff_MissingRecordsKey_ErrorsAtPlanTime(t *testing.T) { + d, _ := newDriver() + spec := interfaces.ResourceSpec{Name: "example.com", Type: "infra.dns", Config: map[string]any{}} + if _, err := d.Diff(context.Background(), spec, nil); err == nil { + t.Fatal("expected error for missing config.records at Plan time") + } +} + func TestDNSDriver_Diff_UpToDate(t *testing.T) { d, _ := newDriver() spec := interfaces.ResourceSpec{ From 64ff6906b146e9e741f09119eac3d132b9e56fa2 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 16:30:03 -0400 Subject: [PATCH 6/7] =?UTF-8?q?fix:=20PR=20#4=20round-8=20=E2=80=94=20jq?= =?UTF-8?q?=20+=20cleanup=20git=20config=20+=20.gitignore=20+=20domain=20v?= =?UTF-8?q?alidate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - .goreleaser.yaml: switch the version rewrite from sed to jq. sed exits 0 even on no-substitution, which would have shipped a stale plugin.json if the file's formatting changed. Added a follow-up jq -e check that fails the hook with a clear message if .version didn't take. - release.yml: add an `if: always()` cleanup step that unsets the credentialed insteadOf git config after goreleaser runs. Reduces the window during which the runner's global git config carries the RELEASES_TOKEN — only the goreleaser step (and the Publish step that follows) see it. Belt-and-braces: the insteadOf is already scoped to github.com/GoCodeAlone/ so the blast radius from leaving it set was already small. - .gitignore: add .release/ and dist/ so local `goreleaser release --snapshot` runs don't leave untracked artifacts that could be accidentally committed. - Diff: validate config.domain up front via domainFromSpec, same pattern as the round-7 declaredRecords validation. Now both missing/empty domain AND missing/wrong-type records surface at Plan time for brand-new resources. Test: TestDNSDriver_Diff_MissingDomain_ErrorsAtPlanTime. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/release.yml | 3 +++ .gitignore | 2 ++ .goreleaser.yaml | 3 ++- internal/drivers/dns.go | 8 ++++++-- internal/drivers/dns_test.go | 13 +++++++++++++ 5 files changed, 26 insertions(+), 3 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 4b6d752..3fe44f7 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -31,6 +31,9 @@ jobs: args: release --clean env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + - name: Unset credentialed git config + if: always() + run: git config --global --unset-all url."https://x-access-token:${{ secrets.RELEASES_TOKEN }}@github.com/GoCodeAlone/".insteadOf || true - name: Publish release (was draft during asset upload) env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/.gitignore b/.gitignore index 4c5f206..c9b2b08 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,3 @@ .claude/ +.release/ +dist/ diff --git a/.goreleaser.yaml b/.goreleaser.yaml index 5aab141..69b1c6f 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -4,7 +4,8 @@ project_name: workflow-plugin-hover before: hooks: - - "sh -c \"rm -rf .release && mkdir -p .release && cp plugin.json .release/plugin.json && sed -i.bak 's/\\\"version\\\": \\\".*\\\"/\\\"version\\\": \\\"{{ .Version }}\\\"/' .release/plugin.json && rm -f .release/plugin.json.bak\"" + - "sh -c \"rm -rf .release && mkdir -p .release && jq --arg v '{{ .Version }}' '.version = $v' plugin.json > .release/plugin.json\"" + - "sh -c \"jq -e --arg v '{{ .Version }}' '.version == $v' .release/plugin.json > /dev/null || (echo 'plugin.json version rewrite failed' >&2 && exit 1)\"" builds: - id: workflow-plugin-hover diff --git a/internal/drivers/dns.go b/internal/drivers/dns.go index af41ccf..3d9831c 100644 --- a/internal/drivers/dns.go +++ b/internal/drivers/dns.go @@ -108,8 +108,12 @@ func (d *DNSDriver) Diff(_ context.Context, desired interfaces.ResourceSpec, cur // Validate the desired spec up front so config errors surface at // Plan time, even for brand-new resources (current == nil) where // the engine would otherwise just see NeedsUpdate=true and let - // Apply discover the same problem one stage later. declaredRecords - // also covers the "config.records is required" check. + // Apply discover the same problem one stage later. domainFromSpec + // rejects missing/empty domain; declaredRecords rejects missing or + // wrong-type `records`. + if _, err := domainFromSpec(desired); err != nil { + return nil, err + } desiredRecs, err := declaredRecords(desired.Config) if err != nil { return nil, err diff --git a/internal/drivers/dns_test.go b/internal/drivers/dns_test.go index 90f86bd..54c3ea2 100644 --- a/internal/drivers/dns_test.go +++ b/internal/drivers/dns_test.go @@ -604,3 +604,16 @@ func TestUpsertRecords_EmptyDesiredDeletesAll(t *testing.T) { t.Errorf("expected all upstream records pruned; got %d: %+v", len(fc.records), fc.records) } } + +func TestDNSDriver_Diff_MissingDomain_ErrorsAtPlanTime(t *testing.T) { + // No name + no config.domain → domainFromSpec returns error. + // Diff must surface that before short-circuiting on nil current. + d, _ := newDriver() + spec := interfaces.ResourceSpec{ + Type: "infra.dns", + Config: map[string]any{"records": []any{}}, + } + if _, err := d.Diff(context.Background(), spec, nil); err == nil { + t.Fatal("expected error for missing domain at Plan time") + } +} From 25a6ec4dd4ac5c1b9a899ed140b8b186b3637c60 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 20 May 2026 16:41:49 -0400 Subject: [PATCH 7/7] =?UTF-8?q?fix:=20PR=20#4=20round-9=20=E2=80=94=20rout?= =?UTF-8?q?e=20RELEASES=5FTOKEN=20through=20env=20in=20cleanup=20step?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "Unset credentialed git config" step previously interpolated ${{ secrets.RELEASES_TOKEN }} directly into the shell command line. Copilot flagged this as widening the secret-exposure surface vs. routing it through a per-step env mapping (which GitHub's secret masking handles more reliably). Switch to the env: pattern and read $RELEASES_TOKEN inside the shell. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/release.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 3fe44f7..eb9e4c2 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -33,7 +33,10 @@ jobs: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - name: Unset credentialed git config if: always() - run: git config --global --unset-all url."https://x-access-token:${{ secrets.RELEASES_TOKEN }}@github.com/GoCodeAlone/".insteadOf || true + env: + RELEASES_TOKEN: ${{ secrets.RELEASES_TOKEN }} + run: | + git config --global --unset-all "url.https://x-access-token:${RELEASES_TOKEN}@github.com/GoCodeAlone/.insteadOf" || true - name: Publish release (was draft during asset upload) env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}