feat: declare ComputePlanVersion v2 + IaCProviderFinalizer.FinalizeApply impl + bump workflow v0.55.0 pin; release v1.3.0#123
Merged
Conversation
…sion=v2 + workflow v0.55.0 (Phase 2.5)
Phase 2.5 of workflow#640 hard-cutover cascade per ADR 0024 + 0040.
DO plugin opts INTO v2 dispatch by declaring ComputePlanVersion="v2" in
CapabilitiesResponse + implementing FinalizeApply RPC server-side. The
wfctl-side OnPlanComplete hook (added in workflow v0.55.0) calls
FinalizeApply which inlines the per-driver flush loop previously held
in the v1 DOProvider.Apply wrapper — preserves the database
trusted_sources deferred-flush regression-gate that was the original
motivator for this cascade.
Changes:
1. go.mod: workflow pin v0.54.0 → v0.55.0 (resolves
IaCProviderFinalizer service types in the generated proto package).
2. plugin.json: version 1.2.0 → 1.3.0; minEngineVersion 0.54.0 →
0.55.0; download URLs updated to v1.3.0 release assets.
3. internal/iacserver.go: doIaCServer struct gains
pb.UnimplementedIaCProviderFinalizerServer embed (mustEmbedUnimplemented
gRPC forward-compat). Compile-time assertion
`_ pb.IaCProviderFinalizerServer = (*doIaCServer)(nil)` added to the
interface guard block — signature drift fails the build here, not at
first RPC dispatch.
4. internal/iacserver.go: FinalizeApply method implemented on
doIaCServer. Iterates s.provider.drivers (the canonical driver
registry — captures orphaned deferred entries even when their
resource type no longer appears in the current plan, mirroring v1
wrapper semantics). For each driver implementing deferredUpdater
with queued state, calls FlushDeferredUpdates(ctx); per-driver
failures append to response.errors[] preserving the
ActionError{Resource: resourceType, Action: "deferred_update",
Error: flushErr.Error()} attribution shape the v1 wrapper used —
wfctl-side OnPlanComplete handler propagates each entry to the
operator-facing result.Errors via the "<plan-finalize>" diagnostic.
5. internal/iacserver.go: Capabilities return-struct-literal flipped to
add ComputePlanVersion: "v2" (return-literal-only edit per Phase 2's
per-plugin universal pattern — signature/receiver/params untouched).
Verification:
- GOWORK=off go build ./... → exit 0
- GOWORK=off go test ./... -race -count=1 → all packages PASS
(digitalocean, internal, internal/drivers, internal/statebackend,
internal/steps)
Rollback per design §Rollback: revert this commit. iacserver.go drops
ComputePlanVersion="v2" + FinalizeApply impl + UnimplementedIaCProviderFinalizerServer
embed + compile-time assert; go.mod drops to workflow v0.54.0;
plugin.json drops to v1.2.0; minEngineVersion drops to 0.54.0. Cut DO
v1.3.1 restoring v1.2.0 state. DO consumers continue on workflow
v0.54.0 + DO v1.2.0 (v1-dispatched).
Coordinated with workflow v0.55.0 (already tagged + released). Closes
workflow#695 once this plugin's v1.3.0 ships.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
Author
iac-codemod refactor-apply reportMode: dry-run Skipped (// wfctl:skip-iac-codemod)
Full report (90-day retention) attached as workflow artifact. |
There was a problem hiding this comment.
Pull request overview
This PR opts the DigitalOcean workflow plugin into the workflow engine’s v2 IaC apply lifecycle by advertising ComputePlanVersion="v2" and implementing the new FinalizeApply RPC, while bumping the workflow dependency and plugin release metadata to match the required engine version.
Changes:
- Bump
github.com/GoCodeAlone/workflowdependency tov0.55.0(and regeneratego.sumaccordingly). - Declare v2 dispatch capability via
ComputePlanVersion: "v2"and addIaCProviderFinalizerServer.FinalizeApplyimplementation to flush deferred driver updates after apply. - Update
plugin.jsonfor releasev1.3.0, includingminEngineVersionand download URLs.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
internal/iacserver.go |
Adds IaCProviderFinalizer service wiring, declares ComputePlanVersion="v2", and implements FinalizeApply deferred-flush loop. |
go.mod |
Pins workflow module to v0.55.0 to pick up the finalizer service/protos. |
go.sum |
Updates module checksums after the workflow bump. |
plugin.json |
Bumps plugin version/min engine and updates release asset URLs to v1.3.0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
155
to
+184
| func (s *doIaCServer) Capabilities(_ context.Context, _ *pb.CapabilitiesRequest) (*pb.CapabilitiesResponse, error) { | ||
| caps := s.provider.Capabilities() | ||
| out := make([]*pb.IaCCapabilityDeclaration, 0, len(caps)) | ||
| for _, c := range caps { | ||
| // Tier is bounded (1..3) in DOProvider; safe int→int32 conversion. | ||
| // Defensive clamp to int32 range avoids G115 drift if the | ||
| // declaration list grows. | ||
| tier := c.Tier | ||
| if tier < math.MinInt32 { | ||
| tier = math.MinInt32 | ||
| } else if tier > math.MaxInt32 { | ||
| tier = math.MaxInt32 | ||
| } | ||
| out = append(out, &pb.IaCCapabilityDeclaration{ | ||
| ResourceType: c.ResourceType, | ||
| Tier: int32(tier), //nolint:gosec // G115: clamped above | ||
| Operations: append([]string(nil), c.Operations...), | ||
| }) | ||
| } | ||
| return &pb.CapabilitiesResponse{Capabilities: out}, nil | ||
| return &pb.CapabilitiesResponse{ | ||
| Capabilities: out, | ||
| // ComputePlanVersion="v2" opts this plugin into wfctl's v2 apply | ||
| // dispatch (wfctlhelpers.ApplyPlan + per-action hooks). The | ||
| // deferred-flush iteration previously held inline in the v1 | ||
| // DOProvider.Apply wrapper has been hoisted to FinalizeApply (the | ||
| // IaCProviderFinalizer optional service implementation below). Per | ||
| // workflow#695 Phase 2.5 / ADR 0024 / ADR 0040. | ||
| ComputePlanVersion: "v2", | ||
| }, nil | ||
| } |
Comment on lines
+204
to
+222
| func (s *doIaCServer) FinalizeApply(ctx context.Context, _ *pb.FinalizeApplyRequest) (*pb.FinalizeApplyResponse, error) { | ||
| var errs []*pb.ActionError | ||
| // Iterate the driver registry directly (not plan.Actions) so that | ||
| // orphaned deferred entries are flushed even when their resource type | ||
| // no longer appears in the current plan (e.g. after a transient flush | ||
| // failure on a prior Apply run). Mirrors v1 wrapper semantics. | ||
| for resourceType, d := range s.provider.drivers { | ||
| du, ok := d.(deferredUpdater) | ||
| if !ok || !du.HasDeferredUpdates() { | ||
| continue | ||
| } | ||
| if flushErr := du.FlushDeferredUpdates(ctx); flushErr != nil { | ||
| errs = append(errs, &pb.ActionError{ | ||
| Resource: resourceType, | ||
| Action: "deferred_update", | ||
| Error: flushErr.Error(), | ||
| }) | ||
| } | ||
| } |
…preserves per-driver attribution (Phase 2.5)
Adds internal/iacserver_finalize_test.go with 2 regression tests that
exercise the v2 dispatch path's FinalizeApply RPC end-to-end without
going through the v1 DOProvider.Apply wrapper:
1. TestDOIaCServer_FinalizeApply_FlushesDeferredUpdates (happy path)
— seeds a deferred trusted_sources update via direct
dbDriver.Create call (first Apps.List returns empty so the driver
queues the deferred update), then calls FinalizeApply on the gRPC
server. Asserts:
- resp.Errors is empty (no per-driver failures)
- UpdateFirewallRules was invoked with the resolved app UUID
- Single rule of {type: "app", value: <UUID>} appears in the
deferred flush request
Without FinalizeApply firing the per-driver flush loop, v2-
dispatched plans would silently lose deferred trusted_sources
resolution — the exact regression workflow#695 Phase 2.5 closes.
2. TestDOIaCServer_FinalizeApply_PreservesPerDriverErrorAttribution
(failure path) — same fixture but with the new minimalDBMock.flushErr
field set to a sentinel error. Asserts:
- gRPC status is OK (per the wire-status invariant — per-driver
failures live in response.errors[], not the gRPC status)
- response.errors[] has exactly one entry
- Entry's Resource is "infra.database"
- Entry's Action is "deferred_update"
- Entry's Error contains the sentinel string (substring match
accommodates the driver's "deferred firewall update <name>:"
prefix and survives the response.errors[] map-iteration order
non-determinism for any future multi-driver failure tests)
Per ADR 0040 invariants 1 + 2; locks the v1 wrapper's
ActionError{Resource, Action, Error} attribution shape on the wire.
Test scaffolding:
- minimalDBMock gains a flushErr field (existing mock in
provider_deferred_test.go); UpdateFirewallRules returns it when set.
Reused by both new tests via the failing-fixture helper.
- setupProviderWithQueuedDBDeferredUpdate(t) helper extracts the seed
pattern from TestDOProvider_Apply_FlushesDeferred_WhenTypeAbsentFromPlan
so the new tests are independent of the v1 Apply path.
- setupProviderWithFailingDBDeferredUpdate(t) helper wraps the above
and sets flushErr; returns the sentinel for assertion.
- fakeAppForDeferred + minimalDBMock + sequencedAppsForDeferred types
are reused unchanged from provider_deferred_test.go.
Folded T7 follow-up Minors from code-reviewer (PR #123 quality review):
- internal/provider.go: Apply method godoc reworded — the "wfctlhelpers
does not yet hoist" claim was true pre-Phase-2.5 but is now stale;
v2 callers route the deferred-flush through FinalizeApply RPC via
the ApplyPlanHooks.OnPlanComplete hook. Updated to reflect actual
post-cascade dispatch topology while keeping the v1-callers-only
legacy note.
- internal/iacserver.go: FinalizeApply godoc gains explicit best-effort
fan-out semantics block — per-driver flushes that succeed remain
committed even if later drivers fail; no cross-driver rollback
coordination. Matches v1 DOProvider.Apply's post-loop block
semantics; operator-facing reconciliation flows through the per-
driver errors[] entries surfaced by the wfctl-side OnPlanComplete
handler.
Verification:
- GOWORK=off go test ./internal/ -run 'TestDOIaCServer_FinalizeApply' -v -count=1
→ 2/2 PASS
- GOWORK=off go test ./... -race -count=1 → all packages PASS:
digitalocean (2.3s), internal (3.5s), internal/drivers (7.8s),
internal/statebackend (3.6s), internal/steps (2.9s)
- GOWORK=off go build ./... → exit 0
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ath assert + drop self-referential godoc clause Three follow-up Minors on T8 (07f4af4) from code-reviewer (PR #123 quality re-review), folded for cascade cleanliness: 1. Add TestDOIaCServer_Capabilities_DeclaresV2 unit lock for the ComputePlanVersion="v2" string-literal in the Capabilities RPC return. wfctl's DispatchVersionFor reads this string to route the plugin through the v2 apply path; a future refactor accidentally dropping the literal (or a struct re-init forgetting to set it) silently reverts the plugin to v1 dispatch. The compile-time assert can't catch a string-literal omission; T9 smoke catches end-to-end but unit lock is more direct. ~10 LOC. 2. Add explicit `if dbMock.lastFirewallReq == nil` assert at the top of the failure-path test body. The error round-trip below implicitly proves the flush chain ran (driver wouldn't queue without HasDeferredUpdates returning true; mock wouldn't get flushErr injected without UpdateFirewallRules being called), but the implicit-only contract masks a future regression where the mock's UpdateFirewallRules short-circuits before recording the request (e.g., if flushErr moved to a pre-call gate). Belt+ suspenders — locks "flush was attempted AND failed" directly. 3. Drop self-referential "no longer 'doesn't hoist'" past-tense correction clause from internal/provider.go Apply godoc (folded in 07f4af4 from T7 Minor #1). The phrasing referenced the previous wording, awkward for a future maintainer without cascade-history context. Rewords directly: "v2 callers route the deferred-flush through the FinalizeApply RPC (see iacserver.go); the workflow engine invokes it via the ApplyPlanHooks.OnPlanComplete hook. Per workflow#695 Phase 2.5." Verification: - GOWORK=off go test ./internal/ -run 'TestDOIaCServer_FinalizeApply|TestDOIaCServer_Capabilities_DeclaresV2' -v -count=1 → 3/3 PASS - GOWORK=off go test ./... -race -count=1 → all packages PASS race-clean (digitalocean 3.5s, internal 2.1s, internal/drivers 7.9s, internal/statebackend 3.5s, internal/steps 2.7s) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Comment on lines
+210
to
+228
| func (s *doIaCServer) FinalizeApply(ctx context.Context, _ *pb.FinalizeApplyRequest) (*pb.FinalizeApplyResponse, error) { | ||
| var errs []*pb.ActionError | ||
| // Iterate the driver registry directly (not plan.Actions) so that | ||
| // orphaned deferred entries are flushed even when their resource type | ||
| // no longer appears in the current plan (e.g. after a transient flush | ||
| // failure on a prior Apply run). Mirrors v1 wrapper semantics. | ||
| for resourceType, d := range s.provider.drivers { | ||
| du, ok := d.(deferredUpdater) | ||
| if !ok || !du.HasDeferredUpdates() { | ||
| continue | ||
| } | ||
| if flushErr := du.FlushDeferredUpdates(ctx); flushErr != nil { | ||
| errs = append(errs, &pb.ActionError{ | ||
| Resource: resourceType, | ||
| Action: "deferred_update", | ||
| Error: flushErr.Error(), | ||
| }) | ||
| } | ||
| } |
Contributor
Author
Copilot review acknowledgement (sha 523c795)Copilot inline at iacserver.go:228 raises duplication of the deferred-flush loop between DOProvider.Apply and doIaCServer.FinalizeApply. This was explicitly considered + decided during the workflow#695 design phase. Per design doc §E (
Rationale: extracting a private
So the duplication is BOUNDED to the Phase 2.5+Phase 3 window. Phase 3 deletes DOProvider.Apply entirely → only the FinalizeApply copy remains → no duplication. Reviewer adversarial-design-review cycle-1 (alternative option 1 in their report) named this exact tradeoff: Defensible architecturally; tracked for cleanup at Phase 3. CI green; admin-merging. |
6 tasks
intel352
added a commit
that referenced
this pull request
May 17, 2026
) DOProvider.Apply was dead code post-Phase-2.5 v2 cutover (DO declares ComputePlanVersion=v2; wfctl routes via wfctlhelpers.ApplyPlanWithHooks + IaCProviderFinalizer.FinalizeApply, bypassing this method). Method body replaced with sentinel-error stub returning ErrApplyV1Removed for diagnostic classification. interfaces.IaCProvider contract preserved per ADR 0024 — clean deletion not viable without breaking all 4 plugins simultaneously; interface segregation deferred to separate refactor design (filed as followup tracking issue). deferredUpdater interface relocated from provider.go to iacserver.go (sole consumer is FinalizeApply); preserves Go-level type clarity. provider_deferred_test.go deleted — regression coverage migrated to iacserver_finalize_test.go per Phase 2.5 T8 quality-review verdict. Eliminates FinalizeApply-vs-Apply deferred-flush loop duplication Copilot flagged on PR #123 (workflow#695 Phase 2.5). Scope extension (team-lead authorized): delete 11 pre-existing v1 Apply tests across provider_apply_test.go + provider_test.go that exercise the now-deleted v1 wrapper dispatch behavior. Equivalent coverage preserved at wfctlhelpers.ApplyPlan tests + internal/drivers/*_test.go per-driver tests + iacserver_finalize_test.go (Phase 2.5 T8). Helper types relocated to internal/deferred_test_helpers_test.go for use by remaining tests. Closes workflow#695 followup item 'Phase 3: delete DOProvider.Apply v1 wrapper'. Co-authored-by: Claude Opus 4.7 <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
Phase 2.5 of workflow#640 hard-cutover cascade per ADR 0024 + 0040. DO plugin opts INTO v2 dispatch by declaring
ComputePlanVersion="v2"in CapabilitiesResponse + implementing FinalizeApply RPC server-side. wfctl-side OnPlanComplete hook (added in workflow v0.55.0) calls FinalizeApply which inlines the per-driver flush loop from the v1 DOProvider.Apply wrapper — preserves the databasetrusted_sourcesdeferred-flush regression gate that motivated this cascade.Changes
go.mod— workflow pinv0.54.0→v0.55.0(go mod tidyregeneratedgo.sum). Resolves theIaCProviderFinalizerServertypes in the generated proto package.plugin.json—version1.2.0 → 1.3.0;minEngineVersion0.54.0 → 0.55.0; download URLs updated to v1.3.0 release assets.internal/iacserver.go(4 sub-edits):doIaCServerstruct gainspb.UnimplementedIaCProviderFinalizerServerembed (mustEmbedUnimplemented gRPC forward-compat)._ pb.IaCProviderFinalizerServer = (*doIaCServer)(nil)added to interface guard block.FinalizeApplymethod implemented ondoIaCServer. Iteratess.provider.drivers(canonical driver registry — captures orphaned deferred entries even when their resource type no longer appears in the current plan, mirroring v1 wrapper semantics). For each driver implementingdeferredUpdaterwith queued state, callsFlushDeferredUpdates(ctx); per-driver failures append toresponse.errors[]preserving theActionError{Resource: resourceType, Action: "deferred_update", Error: flushErr.Error()}attribution shape the v1 wrapper used.Capabilitiesreturn-struct-literal flipped to addComputePlanVersion: "v2"(return-literal-only edit per Phase 2's per-plugin universal pattern — signature/receiver/params untouched).ADR alignment
pb.IaCProviderFinalizerServersatisfaction at plugin startup (workflow v0.55.0'sregisterIaCServicesOnlyblock) and registers the service. Absence of registration would mean no FinalizeApply RPC — but this PR explicitly opts in.repeated ActionError; wire-status invariant honored (errors[] populated ⇒ gRPC OK; gRPC errors signal transport failure, distinct from per-driver finalize errors).Test plan
GOWORK=off go build ./...→ exit 0GOWORK=off go test ./... -race -count=1→ all packages PASS (digitalocean,internal,internal/drivers,internal/statebackend,internal/steps)Cascade context
aac519da; GoReleaser run 25985943298 success; 18 release assets; marked latest).go.modbump resolves against the already-published v0.55.0 module.Rollback (per design §Rollback)
Revert this PR.
internal/iacserver.godropsComputePlanVersion="v2"line +FinalizeApplyimpl +UnimplementedIaCProviderFinalizerServerembed + compile-time assert;go.moddrops to workflow v0.54.0;plugin.jsondrops to v1.2.0;minEngineVersiondrops to 0.54.0. Cut DO v1.3.1 restoring v1.2.0 state. DO consumers continue on workflow v0.54.0 + DO v1.2.0 (v1-dispatched).Per ADR 0040 matched-pair rollback policy: workflow v0.55.0 stays at HEAD (already shipped); only the DO-side opt-in unwinds.
Plan:
docs/plans/2026-05-17-v2-lifecycle-phase2.5-onplancomplete.md(workflow repo).Design:
docs/plans/2026-05-17-v2-lifecycle-phase2.5-onplancomplete-design.md(workflow repo).Closes workflow#695 (after this plugin's v1.3.0 ships + smoke verification per Task 9).
🤖 Generated with Claude Code