feat: release infra + prune orphans on apply (closes #3)#4
Merged
Conversation
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) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds GoReleaser v2 configuration and a GitHub Actions workflow to build and publish cross-platform release artifacts whenever a v* tag is pushed, producing linux/darwin binaries for amd64/arm64 and attaching archives + checksums to GitHub Releases.
Changes:
- Introduces
.goreleaser.yamlwith a build matrix, tar.gz archives, checksums, and a pre-build hook to stampplugin.jsonwith the release version. - Adds
.github/workflows/release.ymlto run GoReleaser on tag push and then publish the draft release.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
.goreleaser.yaml |
Defines GoReleaser v2 build/archive/release behavior and pre-release plugin.json rewriting. |
.github/workflows/release.yml |
Adds a tag-triggered release pipeline that runs GoReleaser and publishes the GitHub Release. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - 'v*' | ||
| permissions: | ||
| contents: write | ||
| id-token: write |
Comment on lines
+13
to
+16
| - uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 | ||
| - uses: actions/setup-go@v5 |
| fetch-depth: 0 | ||
| - uses: actions/setup-go@v5 | ||
| with: | ||
| go-version-file: go.mod |
| 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\"" |
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) <noreply@anthropic.com>
intel352
changed the title
Comment on lines
+231
to
+233
| // 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. |
Comment on lines
+189
to
+190
| // surfaces the change to the operator; upsertRecords prunes | ||
| // them during apply. |
| - 'v*' | ||
| permissions: | ||
| contents: write | ||
| id-token: write |
- 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) <noreply@anthropic.com>
Comment on lines
+5
to
+7
| 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\"" |
Comment on lines
+12
to
+15
| - uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 | ||
| - uses: actions/setup-go@v5 |
Comment on lines
+18
to
+19
| - name: Configure Git for private repos | ||
| run: git config --global url."https://x-access-token:${{ secrets.RELEASES_TOKEN }}@github.com/".insteadOf "https://github.com/" |
- 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) <noreply@anthropic.com>
Comment on lines
135
to
139
| // 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 { |
Comment on lines
75
to
+80
| ## 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. |
- 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) <noreply@anthropic.com>
Comment on lines
+5
to
+8
| 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\"" | ||
|
|
Comment on lines
118
to
120
| if current == nil { | ||
| return &interfaces.DiffResult{NeedsUpdate: true}, nil | ||
| } |
…lidate - .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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds GoReleaser v2 + GitHub Actions release workflow so tagged versions (
v*) build cross-platform binaries (linux/darwin × amd64/arm64) and publish them to GitHub Releases.Patterns +
RELEASES_TOKENrequirement mirrorworkflow-plugin-digitalocean's release infra.RELEASES_TOKENis already set at the GoCodeAlone org level.Files
.github/workflows/release.ymlv*tags; fail-fasts when RELEASES_TOKEN is unset; runs goreleaser; publishes the draft release. Git insteadOf rewrite is scoped togithub.com/GoCodeAlone/so the token never leaks to other hosts..goreleaser.yamlplugin.jsonrewritten with the tag version (versionfield only — current plugin.json has no asset URL fields to rewrite)..gitignoreTest plan
.goreleaser.yamlbuild path resolves to the cmd dir (exists).v0.1.0+ push → release workflow runs end-to-end.checksums.txtmatch downloaded artifacts.🤖 Generated with Claude Code