Skip to content

feat(contracts): migrate iac.provider to strict gRPC proto contracts + CI validation#41

Merged
intel352 merged 13 commits intomainfrom
copilot/migrate-workflow-plugin-to-strict-grpc
May 3, 2026
Merged

feat(contracts): migrate iac.provider to strict gRPC proto contracts + CI validation#41
intel352 merged 13 commits intomainfrom
copilot/migrate-workflow-plugin-to-strict-grpc

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 1, 2026

The plugin advertised iac.provider with no strict contract descriptor, failing strict plugin validation with missing_module_contract_descriptor.

Changes

  • Added proto/digitalocean.proto and generated Go bindings for IacProviderConfig; documented pinned protoc v25.3 / protoc-gen-go v1.34.2 versions in proto/generate.go for reproducible regeneration.
  • Implemented sdk.TypedModuleProvider and sdk.ContractProvider for the DigitalOcean IaC provider while preserving the legacy sdk.ModuleProvider fallback (compile-time assertion included).
  • Added plugin.contracts.json and included it in release archives via .goreleaser.yaml.
  • Added strict contract CI validation that validates plugin.json both as checked-in and after simulating the GoReleaser sed rewrites (mirrors .goreleaser.yaml before-hooks), plus release-time validation after GoReleaser rewrites plugin.json; both derive wfctl from the Workflow SDK version pinned in go.mod.
  • Kept checked-in plugin.json download URLs pointing to the latest published v0.8.4 hyphenated archives; GoReleaser before-hooks rewrite the version and URLs atomically at release time.
  • Moved the gRPC end-to-end strict-contracts test (TestPlugin_GRPCStrictContractsEndToEnd) to internal/plugin_integration_test.go with //go:build integration so go test ./... stays hermetic; integration tests run with -tags integration.
  • Added TestPluginDownloadsMatchGoReleaserArchives to cross-check download URL format and verify GoReleaser archive files list includes plugin.contracts.json.
  • Added TestPlugin_StaticContractManifestMatchesRuntimeRegistry to keep plugin.contracts.json in sync with ContractRegistry().
  • Merged current main to clear the PR conflict and retain the latest DigitalOcean provider updates.

Validation

GOWORK=off go test ./...
GOWORK=off go test -race ./...
GOWORK=off go vet ./...
GOWORK=off go vet -tags integration ./internal/
WFCTL_VERSION="$(go list -m github.com/GoCodeAlone/workflow | awk '{print $2}')" && GOWORK=off go run "github.com/GoCodeAlone/workflow/cmd/wfctl@${WFCTL_VERSION}" plugin validate --file plugin.json --strict-contracts

Strict validation now reports:

OK  workflow-plugin-digitalocean v0.8.4 (plugin.json)

Copilot AI requested review from Copilot and removed request for Copilot May 1, 2026 06:12
@intel352 intel352 marked this pull request as ready for review May 1, 2026 06:24
Copilot AI review requested due to automatic review settings May 1, 2026 06:24
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review any files in this pull request.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@intel352
Copy link
Copy Markdown
Contributor

intel352 commented May 1, 2026

@copilot This PR currently has 0 changed files, so the assigned task is not implemented yet. Please continue the original issue task using Claude Sonnet 4.6: make the required Workflow strict-contract or downstream dependency updates, push actual code/config/doc changes to this PR branch, run the requested validation, and update the PR body with the validation results. Do not leave this as an empty PR.

@intel352
Copy link
Copy Markdown
Contributor

intel352 commented May 1, 2026

@copilot Add this as part of the PR scope: include or update a GitHub Actions PR check that validates strict Workflow plugin contracts. The check should fail when plugin.json is missing and should run the current Workflow CLI strict validation, for example:

name: wfctl strict contracts
on:
  pull_request:
  push:
    branches: [main, master]

jobs:
  wfctl-strict-contracts:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - uses: actions/setup-go@v5
        with:
          go-version-file: go.mod
      - name: Validate strict plugin contracts
        run: go run github.com/GoCodeAlone/workflow/cmd/wfctl@v0.20.1 plugin validate --file plugin.json --strict-contracts

Adjust branch filters and Go setup to match this repository. If the repo uses an existing CI workflow, add this as a job there instead of creating duplicate workflow clutter. Include the check result in the PR validation notes.

Copilot AI and others added 2 commits May 1, 2026 06:49
- Add proto/digitalocean.proto with IacProviderConfig message
- Generate proto/digitalocean.pb.go via protoc
- Implement sdk.TypedModuleProvider on doPlugin (strict proto config path)
- Implement sdk.ContractProvider on doPlugin (GetContractRegistry RPC)
- Add plugin.contracts.json with strict module descriptor for iac.provider
- Fix plugin.json: tier community, downloads, minEngineVersion 0.20.1
- Upgrade workflow SDK v0.19.0 → v0.20.1
- Add tests: ContractRegistry, TypedModuleTypes, CreateTypedModule variants

Agent-Logs-Url: https://github.com/GoCodeAlone/workflow-plugin-digitalocean/sessions/7ff39de7-6d6d-4104-be5b-5685d34865c5

Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
@intel352
Copy link
Copy Markdown
Contributor

intel352 commented May 1, 2026

@copilot please try again using Claude Sonnet 4.6.

Continue the original task for this PR. Make sure the PR is not left empty, add or update strict gRPC contract descriptors as applicable, add a PR check that runs wfctl strict validation, run the repository tests, and keep this PR ready for review with Copilot assigned as reviewer.

Copilot AI requested review from Copilot and removed request for Copilot May 1, 2026 12:51
Comment thread .github/workflows/ci.yml Fixed
Copilot AI requested review from Copilot and removed request for Copilot May 1, 2026 12:51
@intel352
Copy link
Copy Markdown
Contributor

intel352 commented May 2, 2026

Addressed the latest review in 08d0d9e: PR description now matches v0.8.4, module type duplication is centralized in code and cross-checked against static manifests, generated proto now has a go:generate command, release/download matrix is tested against GoReleaser config, and the gRPC strict path test now proves legacy nil-config creation would fail while the adapter-created strict module succeeds. Re-ran go test ./..., go test -race ./..., go vet ./..., and strict wfctl validation.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread proto/generate.go Outdated
Comment thread internal/plugin_test.go Outdated
Comment thread .goreleaser.yaml Outdated
@intel352
Copy link
Copy Markdown
Contributor

intel352 commented May 2, 2026

Addressed the latest package/release review in f97dec3: go:generate now regenerates from repo root using proto/digitalocean.proto so the descriptor source path stays stable; the gRPC end-to-end test stages plugin.contracts.json and runs strict wfctl validation against the staged plugin package; and the GoReleaser validation hook exports GOPRIVATE and GOWORK=off before invoking wfctl. Re-ran go test ./..., go test -race ./..., go vet ./..., focused staged-package test, and strict wfctl validation.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread plugin.json
Comment thread internal/plugin_test.go Outdated
Comment thread .goreleaser.yaml
Comment thread internal/plugin_test.go Outdated
Comment thread internal/plugin_test.go
Comment thread internal/plugin_test.go
@intel352
Copy link
Copy Markdown
Contributor

intel352 commented May 2, 2026

Addressed the latest release-archive review in 93673af: plugin.json now points at the next release tag v0.8.5 so installs will not keep using pre-contract v0.8.4 archives; tests now check GoReleaser archive files include plugin.contracts.json and build a representative tar.gz containing it; the gRPC test disables the legacy module path to prove strict typed dispatch; and TestPlugin_Manifest now catches runtime/plugin.json metadata drift. Re-ran go test ./..., go test -race ./..., go vet ./..., focused package tests, and strict wfctl validation.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread plugin.json Outdated
Comment on lines 3 to 4
"version": "0.8.5",
"description": "DigitalOcean IaC provider: App Platform, DOKS, databases, Redis cache, load balancers, VPC, firewall, DNS, Spaces, DOCR, certificates, Droplets, IAM (declared), and API gateway",
Comment thread .github/workflows/ci.yml
Comment on lines +47 to +50
- name: Validate strict plugin contracts
run: |
WFCTL_VERSION="$(go list -m github.com/GoCodeAlone/workflow | awk '{print $2}')"
go run "github.com/GoCodeAlone/workflow/cmd/wfctl@${WFCTL_VERSION}" plugin validate --file plugin.json --strict-contracts
Comment thread internal/plugin.go Outdated
Comment on lines +65 to +66
if os.Getenv("WORKFLOW_PLUGIN_DIGITALOCEAN_DISABLE_LEGACY_MODULE") == "1" {
return nil, fmt.Errorf("digitalocean plugin: legacy module path disabled")
Comment thread internal/plugin_test.go Outdated
t.Fatalf("write temp plugin.contracts.json: %v", err)
}

validate := exec.Command("go", "run", "github.com/GoCodeAlone/workflow/cmd/wfctl@"+workflowModuleVersion(t, repoRoot), "plugin", "validate", "--file", filepath.Join(pluginDir, "plugin.json"), "--strict-contracts")
Comment thread proto/generate.go
@@ -0,0 +1,3 @@
package proto

//go:generate sh -c "cd .. && protoc --go_out=. --go_opt=paths=source_relative proto/digitalocean.proto"
@intel352
Copy link
Copy Markdown
Contributor

intel352 commented May 3, 2026

@copilot address all unresolved review comments

Copilot AI and others added 2 commits May 3, 2026 01:33
- plugin.json: revert version/URLs to v0.8.4 (latest published); goreleaser
  rewrites to release tag at release time, avoiding 404 during pre-release window
- .github/workflows/ci.yml: add post-rewrite simulation step that applies the
  same sed rewrites as goreleaser before-hooks and validates the result, so a
  broken template change is caught before release
- internal/plugin.go: remove WORKFLOW_PLUGIN_DIGITALOCEAN_DISABLE_LEGACY_MODULE
  env kill-switch from production CreateModule; backward-compat hosts are not
  affected by test internals
- internal/plugin_test.go + plugin_integration_test.go: move
  TestPlugin_GRPCStrictContractsEndToEnd and workflowModuleVersion to a
  //go:build integration file so go test ./... stays hermetic; remove
  t.Setenv kill-switch usage now that env var is gone from production code
- proto/generate.go: document pinned protoc v25.3 / protoc-gen-go v1.34.2
  versions used to generate the checked-in .pb.go

Agent-Logs-Url: https://github.com/GoCodeAlone/workflow-plugin-digitalocean/sessions/55049097-392b-4939-aa13-b98c931fdb0f

Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot May 3, 2026 01:35
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 3, 2026

@copilot address all unresolved review comments

All 5 unresolved review threads addressed in commits 17c23a5 + 945ba40:

  1. plugin.json:3-4 — pre-release 404s: Reverted version/download URLs from v0.8.5 to v0.8.4 (latest published). GoReleaser before-hooks rewrite to the actual release tag atomically at release time, so the checked-in manifest always points to existing artifacts.

  2. .github/workflows/ci.yml — CI doesn't validate post-rewrite manifest: Added a second wfctl validation pass that applies the same sed rewrites as goreleaser's before-hooks to a temp copy of plugin.json, then validates that rewritten form. Uses portable sed -i.bak + cleanup.

  3. internal/plugin.go:65-66 — env kill-switch in production code: Removed WORKFLOW_PLUGIN_DIGITALOCEAN_DISABLE_LEGACY_MODULE check from CreateModule. The env var was test-only logic that could break backward-compat hosts at runtime.

  4. internal/plugin_test.go:171 — non-hermetic go run ...@version in test suite: Moved TestPlugin_GRPCStrictContractsEndToEnd and workflowModuleVersion to internal/plugin_integration_test.go with //go:build integration. go test ./... is now hermetic; integration tests run only with -tags integration.

  5. proto/generate.go:3 — protoc versions not pinned: Added package doc documenting protoc v25.3 / protoc-gen-go v1.34.2 (exact versions used to generate the checked-in .pb.go) and instructions for regeneration.

@intel352 intel352 merged commit bbc8195 into main May 3, 2026
2 checks passed
intel352 added a commit that referenced this pull request May 3, 2026
…side plugin.json

The Validate strict plugin contracts step (added in PR #41) copies only
plugin.json to a tempfile when simulating GoReleaser's before-hook
version rewrite. But wfctl plugin validate --strict-contracts looks for
plugin.contracts.json CO-LOCATED with plugin.json (same directory). The
rewrite-validation therefore always fails with
"missing_module_contract_descriptor: module type \"iac.provider\" has no
strict contract descriptor" — even when plugin.contracts.json IS valid
in the repo root.

This was masked on main because main has a single configurable contract
file and any change rebuilds in-place; surfaced now by PR #55 which
introduces a new resource type and exercises the second validation path.

Fix: use a temp DIR and copy both plugin.json + plugin.contracts.json
into it so the rewritten plugin.json's neighbour is intact.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
intel352 added a commit that referenced this pull request May 3, 2026
…+ new infra.volume driver (#55)

* feat(drivers): add boolFromConfig and strSliceFromConfig helpers

Both helpers are structpb-aware: boolFromConfig falls through to the
default for non-bool values, and strSliceFromConfig accepts the typed
[]string shape Go-native callers emit AND the []any shape that survives
a YAML/JSON → structpb round-trip. Empty strings drop silently. Used by
the upcoming droplet/volume drivers; placed in util.go alongside the
existing strFromConfig/intFromConfig helpers so all drivers can reach
them without import cycles.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(droplet): support user_data / vpc_uuid / ssh_keys / volumes / tags / bools

Self-hosted services (e.g. Apache-AGE Postgres on a Droplet, since DO
Managed Postgres has no AGE extension) need more than the size/image/
region the DropletDriver previously honoured. New optional config keys,
all additive and defaulted-empty (no behaviour change for existing
callers):

  - user_data        string         — cloud-init payload
  - vpc_uuid         string         — VPC the Droplet joins
  - ssh_keys         []string|[]int — fingerprints OR numeric IDs;
                                      element type detected at runtime
                                      (mixed lists OK; structpb floats
                                      must be whole numbers)
  - tags             []string
  - enable_backups   bool           — godo Backups field
  - monitoring       bool
  - ipv6             bool
  - volumes          []string of    — names resolved to IDs at create
                     volume names     time via Storage.ListVolumes(name=,
                                      region=). Region-bound: a name not
                                      present in the droplet's region
                                      returns "droplet volumes: volume
                                      %q not found".

Outputs gain `private_ip` (godo droplet.PrivateIPv4()) so downstream
services in the same VPC can be wired without a second Read.

DropletDriver now holds an optional Storage client (set by
NewDropletDriver from c.Storage; tests inject via the variadic
NewDropletDriverWithClient param). When `volumes` is non-empty but the
storage client is nil, Create returns an explicit error rather than
silently dropping the attachment.

mockDropletClient now captures the godo.DropletCreateRequest so tests
can assert on populated struct fields. Adds focused tests for each new
key, plus the SSH-keys fractional-float rejection path that prevents
silent ID truncation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(drivers): add infra.volume driver (DO Block Storage)

New ResourceDriver covering CRUD + Diff + HealthCheck for DigitalOcean
Block Storage volumes. Backed by godo.StorageService; in-place size
growth uses godo.StorageActionsService.Resize. ProviderIDFormat is UUID
(DO volume IDs are 36-character UUIDs).

Config keys:
  - name             from spec.Name
  - region           defaults to provider region
  - size_gb          required, > 0
  - filesystem_type  optional ("ext4" / "xfs"); empty = raw block device
  - description      optional
  - tags             optional []string

Outputs: id (UUID), name, region, size_gb, filesystem_type. Status is a
stable "available" string — godo.Volume exposes no Status field, so a
successful Read on a real volume is the strongest health signal we have.

Update semantics:
  - size growth → in-place StorageActions.Resize, then re-read for state
  - size shrink → explicit error (DO has no shrink API; replace required)
  - region or filesystem_type change → Diff sets ForceNew so plan
    classifies as replace, not update

Tests cover Create / Read / Update (resize, no-op, shrink-rejected,
empty-ProviderID guard) / Delete / Diff (nil current, grow=update-only,
shrink=replace, region=replace, filesystem_type=replace, no-changes)
/ HealthCheck (success, read-error, empty-ProviderID) / ProviderIDFormat
/ Scale-not-supported. The volume mock pool supports filter-by-name so
the same struct exercises both VolumeDriver tests and the droplet
volumes-by-name path.

providerid_format_test.go adds the volume entry to the manually
maintained registry — without it, a future drift in ProviderIDFormat
would not be caught.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(provider): register infra.volume driver and capability

Wire VolumeDriver into Initialize() alongside the other infra drivers
and declare its IaCCapabilityDeclaration with the noScale operation set
(volumes have no scale concept; size_gb is dimensional). Add
"infra.volume" to the sizing map as a noopSizing entry so ResolveSizing
returns "n/a" for every abstract tier — the operator must set size_gb
explicitly. Provider Capabilities test gains the matching required
entry so removal of the driver registration would be caught.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs: declare infra.volume in plugin.json and CHANGELOG

plugin.json: add infra.volume to the iacProvider.resourceTypes array
and update the descriptor to mention Block Storage volumes.

CHANGELOG: document the new infra.volume driver and the extended
Droplet config keys (user_data / vpc_uuid / ssh_keys / volumes / tags
/ enable_backups / monitoring / ipv6) plus the new private_ip Droplet
output. No README exists in this repo, so the CHANGELOG is the
canonical surface for these notes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(droplet): strict validation for volumes config entries (Copilot finding #1)

strSliceFromConfig silently drops non-string and empty entries, which is
dangerous for volume attachments — a typo or wrong type leaves the Droplet
running without an expected disk and emits no diagnostic. Add
dropletVolumesFromConfig that returns an explicit error for any non-string
or empty entry, mirroring dropletSSHKeysFromConfig's error-message style.

Tests cover both shapes: []any{123, "data"} (type mismatch) and []any{""}
(empty string).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(volume): reject fractional size_gb instead of silent truncation (Copilot finding #2)

intFromConfig truncates float64 silently — size_gb: 100.9 (e.g. via JSON or
YAML float coercion) was creating a 100 GB volume with no diagnostic. Add
intStrictFromConfig that mirrors the float64 fractional-rejection logic
already in dropletSSHKeysFromConfig and use it for size_gb in Create,
Update, and Diff.

Whole-valued float64 (the structpb wire shape) still passes through.
intFromConfig is unchanged for callers where rounding is acceptable
(node_count, instance_count, probe seconds, etc.).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(droplet): accept []int and []int64 ssh_keys at top level (Copilot finding #3)

dropletSSHKeysFromConfig accepted []any and []string at the top level but
bailed on Go-native []int{101} or []int64{101} with "expected list, got
[]int" — even though numeric IDs are documented as supported and the
per-element handler already covers int / int64 / float64. Add type-switches
that mirror the per-element validation (non-positive rejection, error
message style).

Tests cover three shapes: []int{101,102}, []int64{555,666}, and the
non-positive rejection path []int{0,101}.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(sizing): cover infra.volume in resolveSizing test cases (Copilot finding #4)

infra.volume was added to doSizingMap as noopSizing but the manually-
maintained TestResolveSizing cases table was not updated. Without the
case, the registration is unverified and a future refactor that drops
the entry would not fail this test. Add SizeS + SizeXL coverage so the
"n/a" contract is locked in alongside every other noop-sized resource.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(volume): treat description/tags changes as ForceNew (Copilot finding #5)

Diff previously tracked only size_gb / region / filesystem_type. Changes to
description and tags were silently ignored because godo.StorageActions has
no update endpoint for either, and the godo Volume API itself sets both at
creation time only — so there is no in-place update path to make available.

Resolution: surface drift as ForceNew (matching the way region / filesystem
shrinks are handled) so the planner emits a planned replace rather than
dropping the change. Document the constraint in the diff comment.

Surface description and tags in volumeOutput so Diff has values to compare.
Reuse outputsAsStringSlice + equalStringSet (firewall.go) so reordered tags
do NOT trigger a needless replace.

Tests: tags-add forces replace; reordered tags do NOT; description change
forces replace.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(droplet): Diff flags vpc/tags/volumes/backups drift as ForceNew (Copilot finding #6)

Extended droplet fields (user_data, vpc_uuid, ssh_keys, volumes, tags,
enable_backups, monitoring, ipv6) were wired into Create only; Diff
compared just "size", so changing any of these on an existing droplet
produced no plan action and the drift was silently dropped. godo
explicitly disallows Update on droplets (PUT only resizes), so Diff is
the only place to flag these — every detected change must be ForceNew.

Surface the comparable fields in dropletOutput (vpc_uuid, enable_backups
derived from BackupIDs, tags, volumes) so Diff has values to compare;
extend Diff with order-irrelevant set comparison for tags / volumes
(reuses outputsAsStringSlice + equalStringSet from firewall.go).

user_data, monitoring, ipv6, ssh_keys are NOT drift-checked: godo.Droplet
does not surface them on Read, so comparing against current Outputs would
produce a perpetually-dirty plan. Documented as a known limitation.

Tests: vpc_uuid change forces replace; tags add forces replace; tags
reorder does NOT force replace; enable_backups toggle forces replace.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(plugin): include 'Block Storage volumes' in Manifest.Description

Manifest.Description must match plugin.json's description (test enforced
in plugin_test.go). The merge from main pulled in a longer description
that didn't yet mention Block Storage volumes; this aligns both.

* fix(ci): wfctl-strict-contracts must copy plugin.contracts.json alongside plugin.json

The Validate strict plugin contracts step (added in PR #41) copies only
plugin.json to a tempfile when simulating GoReleaser's before-hook
version rewrite. But wfctl plugin validate --strict-contracts looks for
plugin.contracts.json CO-LOCATED with plugin.json (same directory). The
rewrite-validation therefore always fails with
"missing_module_contract_descriptor: module type \"iac.provider\" has no
strict contract descriptor" — even when plugin.contracts.json IS valid
in the repo root.

This was masked on main because main has a single configurable contract
file and any change rebuilds in-place; surfaced now by PR #55 which
introduces a new resource type and exercises the second validation path.

Fix: use a temp DIR and copy both plugin.json + plugin.contracts.json
into it so the rewritten plugin.json's neighbour is intact.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(droplet): resolve VolumeIDs to names in Outputs (Copilot round-2 finding #1)

dropletOutput previously stored godo's raw VolumeIDs (UUIDs) in
Outputs["volumes"], while desired config carries volume *names*. After
every successful Create/Read, the next Diff would compare e.g.
["vol-uuid-1"] against ["pg-data"] and force-replace the Droplet —
catastrophic for stateful workloads (the PG data Droplet would be
destroyed and recreated on every deploy, losing all game state).

Resolve each VolumeID to its name via Storage.GetVolume in dropletOutput.
Cache lookups within a single Read so duplicate VolumeIDs only hit the
API once. On resolution failure (e.g. volume deleted out-of-band) fall
back to the raw ID and record the unresolved IDs in
Outputs["volumes_resolution"] so operators can debug.

Update Diff comment to reflect that comparison is now name-vs-name and
stable across plans.

Tests:
- Read with VolumeIDs returns names in Outputs
- Diff with matching names returns no change (regression guard)
- Diff with desired missing a name returns ForceNew
- Resolution failure falls back to ID + records unresolved_ids
- Duplicate IDs only call GetVolume once (cache hit)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(util): intStrictFromConfig surfaces type mismatches explicitly (Copilot round-2 finding #2)

Previously `intStrictFromConfig` returned (defaultVal, false, nil) when
the config value was an unsupported type (e.g. `size_gb: "100"` as a
string). The caller then emitted "size_gb is required" — a misleading
diagnostic that hid the real bug (operator quoted a number).

Return present=true with an explicit "expected integer, got <type>"
error so the caller surfaces the type problem directly.

Tests: string-typed size_gb and bool-typed size_gb both reject with
"expected integer, got <type>" before any CreateVolume call is made.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(droplet): tags clear (non-empty -> empty) surfaces as drift (Copilot round-2 finding #3)

The Diff path skipped tag comparison when len(desired.tags)==0, so
clearing tags on a Droplet was silently ignored. Operators sometimes
need to strip tags to remove a Droplet from a tag-based firewall or
backup schedule; "no diff" is dangerously wrong here.

Switch the guard to "key present in desired" so empty desired vs
non-empty current surfaces as ForceNew, while absent desired still
skips (preserves backwards-compat for YAML that predates the field).

Tests: clearing tags forces replace; absent tags key does NOT trigger
drift; reorder still ignored.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(volume): filesystem_type transitions raw<->ext4 surface as drift (Copilot round-2 finding #4)

The empty-side guard skipped raw(empty)->ext4 and ext4->raw transitions,
so changing filesystem_type after Create produced no diff. DO Block
Storage cannot reformat a volume in place, so any change must surface
as ForceNew rather than being silently ignored.

Switch to "key present in desired" guard so empty<->non-empty surfaces
as drift. Absent desired key still skips (backwards-compat for YAML
predating the field).

Tests: raw->ext4 and ext4->raw both force replace; absent key does NOT
trigger drift.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(volume): description add/clear from empty surfaces as drift (Copilot round-2 finding #5)

The empty-side guard skipped both empty->non-empty (adding a
description after Create) and non-empty->empty (clearing a description)
transitions. DO Block Storage exposes no description-update endpoint,
so any change must surface as ForceNew rather than being silently
ignored.

Switch to "key present in desired" guard. Absent key still skips
(backwards-compat).

Tests: add-from-empty and clear-to-empty both force replace; absent
key does NOT trigger drift.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(volume): tags clear surfaces as drift (Copilot round-2 finding #6)

Same pattern as droplet finding #3 and volume description finding #5.
Clearing tags (non-empty current -> empty desired) was silently ignored
because Diff skipped when len(desired.tags)==0. DO Block Storage has no
tag-update endpoint, so any tag change must surface as ForceNew.

Switch to "key present in desired" guard. Absent key still skips.

Tests: clearing tags forces replace; absent tags key does NOT trigger
drift (existing reorder/equal-set behavior preserved).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(droplet): vpc_uuid add-from-empty surfaces as drift (Copilot round-2 finding #7)

The curVPC != "" guard skipped vpc_uuid drift when the current state
had no vpc_uuid. Pre-release Droplet states (created before vpc_uuid
became part of Outputs) would silently ignore an operator adding a
vpc_uuid pin to YAML — exactly when explicit drift detection matters
most.

Switch to "key present in desired" guard. Empty current vs non-empty
desired now triggers ForceNew. Absent desired key still skips
(backwards-compat).

Tests: vpc_uuid add-from-empty forces replace; absent key does NOT
trigger drift.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(droplet): explicit limitation block for user_data/ssh_keys/monitoring/ipv6 (Copilot round-2 finding #8)

Round-1 documented the gap inline but the silent fallthrough at the end
of Diff was easy to miss. Replace with an explicit block that:

- Names each undetected field and the godo reason
- States the operator workaround (`taint` / delete + re-apply)
- References follow-up issue #56 for the resolution path
- Updates the function-level Diff doc-comment to point at the inline
  rationale

Issue: #56
"Droplet Diff misses user_data / ssh_keys / monitoring / ipv6 (godo Read limitation)"

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
intel352 added a commit that referenced this pull request May 3, 2026
PR #41 added a top-level `after:` hook to .goreleaser.yaml. goreleaser
v2 has no top-level `after:` field — only `before:` exists. The release
workflow for v0.9.0 (run 25274816649) failed instantly with
`yaml: unmarshal errors: line 12: field after not found in type
config.Project`.

The previous `mv plugin.json.orig plugin.json` was theatre anyway: the
GH Actions runner is ephemeral, so the in-place sed mutations to
plugin.json are discarded when the runner shuts down. No need to
restore plugin.json on the runner. Removed the matching `cp
plugin.json plugin.json.orig` from the before block as well — it was
preserving state nothing else reads.

Validated with goreleaser/goreleaser:v2.15.4 check.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
intel352 added a commit that referenced this pull request May 3, 2026
…ession) (#59)

wfctl bootstrap uses `provider: digitalocean` as the discriminator to
identify which iac.provider module owns a given backend (e.g.
`backend: spaces` for IaC state). v0.9.0's strict-contracts proto (PR
#41) didn't declare `provider`, so configs carrying the canonical
discriminator (BMW, core-dump, every existing iac.provider config)
failed typed-config marshal with `protobuf error` because wfctl uses
`protojson UnmarshalOptions{DiscardUnknown: false}` (workflow
plugin/external/convert.go:65) which rejects unknown fields.

The plugin itself doesn't use this field — manifest.iacProvider.name =
digitalocean is the source of truth for provider identity — but the
proto must accept it so strict-contracts unmarshal doesn't reject the
config.

Surfaced by core-dump's first deploy attempt against v0.9.0 (run
25277359916: `error: no iac.provider module found in config — add an
iac.provider module to bootstrap remote state backends
(backend="spaces")` after temporarily removing `provider:` to bypass
the protobuf-error rejection).

Bumps to v0.9.1.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Migrate workflow-plugin-digitalocean to strict gRPC proto contracts

4 participants