From 7c7587c6de96b647a3003040be577269fe808497 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 17 May 2026 05:01:28 -0400 Subject: [PATCH 1/3] feat(v2): IaCProviderFinalizer.FinalizeApply + declare ComputePlanVersion=v2 + workflow v0.55.0 (Phase 2.5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 "" 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 --- go.mod | 2 +- go.sum | 4 +-- internal/iacserver.go | 63 ++++++++++++++++++++++++++++++++++++++++++- plugin.json | 12 ++++----- 4 files changed, 71 insertions(+), 10 deletions(-) diff --git a/go.mod b/go.mod index b7a58c4..65a2360 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/GoCodeAlone/workflow-plugin-digitalocean go 1.26.0 require ( - github.com/GoCodeAlone/workflow v0.54.0 + github.com/GoCodeAlone/workflow v0.55.0 github.com/aws/aws-sdk-go-v2 v1.41.6 github.com/aws/aws-sdk-go-v2/config v1.32.16 github.com/aws/aws-sdk-go-v2/credentials v1.19.15 diff --git a/go.sum b/go.sum index 733b9b5..324d481 100644 --- a/go.sum +++ b/go.sum @@ -22,8 +22,8 @@ github.com/GoCodeAlone/modular/modules/jsonschema v1.15.0 h1:xb1mI4NZkzvNKQ2F6nk github.com/GoCodeAlone/modular/modules/jsonschema v1.15.0/go.mod h1:hhGouwAVsonmJ4Lain4jINZ9nZCoc9l9eF3BHbmR8eE= github.com/GoCodeAlone/modular/modules/reverseproxy/v2 v2.8.0 h1:cvdLHbM/vzvygQTcAWSJsy+dAPzzwWyjzKMmTBFcFIo= github.com/GoCodeAlone/modular/modules/reverseproxy/v2 v2.8.0/go.mod h1:/9ipMG4qM2CHQ14BfXKdVlYRJelef6M8MFI5TbZv67M= -github.com/GoCodeAlone/workflow v0.54.0 h1:8x5iVQ8di2BAAyr/uAD/PllTa1Sw9OFElMkje0onKB8= -github.com/GoCodeAlone/workflow v0.54.0/go.mod h1:RrmHdww6gAIUfyjJ7stz0fegxRmVGZvQ1FsuJ2F0C+U= +github.com/GoCodeAlone/workflow v0.55.0 h1:oHTvXDBVOvJbDtfYQ1LmMkF5hGUL6YHuwbGYcG95CXc= +github.com/GoCodeAlone/workflow v0.55.0/go.mod h1:RrmHdww6gAIUfyjJ7stz0fegxRmVGZvQ1FsuJ2F0C+U= github.com/GoCodeAlone/yaegi v0.17.2 h1:WK6Y6e0t1a6U7r+S2dN3CGWW1PizYD3zO0zneToZPxM= github.com/GoCodeAlone/yaegi v0.17.2/go.mod h1:z5Pr6Wse6QJcQvpgxTxzMAevFarH0N37TG88Y9dprx0= github.com/IBM/sarama v1.47.0 h1:GcQFEd12+KzfPYeLgN69Fh7vLCtYRhVIx0rO4TZO318= diff --git a/internal/iacserver.go b/internal/iacserver.go index 8e52084..3ca7fc3 100644 --- a/internal/iacserver.go +++ b/internal/iacserver.go @@ -54,6 +54,12 @@ type doIaCServer struct { pb.UnimplementedIaCProviderMigrationRepairerServer pb.UnimplementedIaCProviderValidatorServer pb.UnimplementedIaCProviderDriftConfigDetectorServer + // pb.UnimplementedIaCProviderFinalizerServer satisfies the + // mustEmbedUnimplementedIaCProviderFinalizerServer() forward-compat + // requirement on pb.IaCProviderFinalizerServer (workflow#695 Phase 2.5). + // The actual FinalizeApply method is overridden below; the embed is + // required by the gRPC codegen contract. + pb.UnimplementedIaCProviderFinalizerServer // pb.UnimplementedResourceDriverServer satisfies the // mustEmbedUnimplementedResourceDriverServer() forward-compat // requirement on pb.ResourceDriverServer; the per-type CRUD @@ -109,6 +115,12 @@ var ( _ pb.IaCProviderMigrationRepairerServer = (*doIaCServer)(nil) _ pb.IaCProviderValidatorServer = (*doIaCServer)(nil) _ pb.IaCProviderDriftConfigDetectorServer = (*doIaCServer)(nil) + // IaCProviderFinalizer is the workflow#695 Phase 2.5 optional service + // — DO plugin implements FinalizeApply server-side to host the + // deferred-flush iteration previously held inline in the v1 + // DOProvider.Apply wrapper. Required by the v2 dispatch declared + // via ComputePlanVersion="v2" below. + _ pb.IaCProviderFinalizerServer = (*doIaCServer)(nil) // doIaCServer also SERVES the typed IaC state-backend contract (spaces // backend). The SDK serve hook auto-registers this via type-assertion at // plugin startup — see cmd/plugin/main.go. @@ -159,7 +171,56 @@ func (s *doIaCServer) Capabilities(_ context.Context, _ *pb.CapabilitiesRequest) 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 +} + +// FinalizeApply implements pb.IaCProviderFinalizerServer for the v2 +// dispatch path. Inlines the per-driver flush loop from +// DOProvider.Apply (internal/provider.go's post-loop block iterating +// p.drivers and calling deferredUpdater.FlushDeferredUpdates) since +// DOProvider does not expose a public FlushDeferredUpdates method — +// the flush iteration was inline in the v1 Apply wrapper. Per +// workflow#695 Phase 2.5. +// +// Per-driver error attribution is preserved by returning ActionError +// entries on the response, mirroring the v1 wrapper shape that wfctl +// previously read directly from result.Errors via +// ActionError{Resource: resourceType, Action: "deferred_update", +// Error: flushErr.Error()}. wfctl-side OnPlanComplete handler appends +// these entries onward into result.Errors as a "" entry, +// preserving the operator-facing diagnostic shape. +// +// Empty errors slice = success (gRPC status OK; wire-status invariant +// per FinalizeApplyResponse godoc). +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(), + }) + } + } + return &pb.FinalizeApplyResponse{Errors: errs}, nil } func (s *doIaCServer) Plan(ctx context.Context, req *pb.PlanRequest) (*pb.PlanResponse, error) { diff --git a/plugin.json b/plugin.json index 2ba11e4..f134b82 100644 --- a/plugin.json +++ b/plugin.json @@ -1,12 +1,12 @@ { "name": "workflow-plugin-digitalocean", - "version": "1.2.0", + "version": "1.3.0", "description": "DigitalOcean IaC provider: App Platform, DOKS, databases, Redis cache, load balancers, VPC, firewall, DNS, Spaces, DOCR, certificates, Droplets, Block Storage volumes, IAM (declared), and API gateway", "author": "GoCodeAlone", "license": "MIT", "type": "external", "tier": "community", - "minEngineVersion": "0.54.0", + "minEngineVersion": "0.55.0", "keywords": [ "digitalocean", "iac", @@ -23,22 +23,22 @@ { "os": "linux", "arch": "amd64", - "url": "https://github.com/GoCodeAlone/workflow-plugin-digitalocean/releases/download/v1.2.0/workflow-plugin-digitalocean-linux-amd64.tar.gz" + "url": "https://github.com/GoCodeAlone/workflow-plugin-digitalocean/releases/download/v1.3.0/workflow-plugin-digitalocean-linux-amd64.tar.gz" }, { "os": "linux", "arch": "arm64", - "url": "https://github.com/GoCodeAlone/workflow-plugin-digitalocean/releases/download/v1.2.0/workflow-plugin-digitalocean-linux-arm64.tar.gz" + "url": "https://github.com/GoCodeAlone/workflow-plugin-digitalocean/releases/download/v1.3.0/workflow-plugin-digitalocean-linux-arm64.tar.gz" }, { "os": "darwin", "arch": "amd64", - "url": "https://github.com/GoCodeAlone/workflow-plugin-digitalocean/releases/download/v1.2.0/workflow-plugin-digitalocean-darwin-amd64.tar.gz" + "url": "https://github.com/GoCodeAlone/workflow-plugin-digitalocean/releases/download/v1.3.0/workflow-plugin-digitalocean-darwin-amd64.tar.gz" }, { "os": "darwin", "arch": "arm64", - "url": "https://github.com/GoCodeAlone/workflow-plugin-digitalocean/releases/download/v1.2.0/workflow-plugin-digitalocean-darwin-arm64.tar.gz" + "url": "https://github.com/GoCodeAlone/workflow-plugin-digitalocean/releases/download/v1.3.0/workflow-plugin-digitalocean-darwin-arm64.tar.gz" } ], "iacProvider": { From 07f4af4b981d04379e8678c7881ce99324ff5a10 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 17 May 2026 05:27:25 -0400 Subject: [PATCH 2/3] test(v2): regression-gate FinalizeApply fires FlushDeferredUpdates + preserves per-driver attribution (Phase 2.5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: } 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 :" 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 --- internal/iacserver.go | 6 + internal/iacserver_finalize_test.go | 171 ++++++++++++++++++++++++++++ internal/provider.go | 8 +- internal/provider_deferred_test.go | 9 +- 4 files changed, 191 insertions(+), 3 deletions(-) create mode 100644 internal/iacserver_finalize_test.go diff --git a/internal/iacserver.go b/internal/iacserver.go index 3ca7fc3..a6d364d 100644 --- a/internal/iacserver.go +++ b/internal/iacserver.go @@ -201,6 +201,12 @@ func (s *doIaCServer) Capabilities(_ context.Context, _ *pb.CapabilitiesRequest) // // Empty errors slice = success (gRPC status OK; wire-status invariant // per FinalizeApplyResponse godoc). +// +// Best-effort fan-out: per-driver flushes that succeed remain committed +// even if later drivers fail; this method does not coordinate rollback +// across drivers (same semantics as v1 DOProvider.Apply's post-loop +// block). Operators reconcile partial failures from the per-driver +// errors[] entries surfaced by the wfctl-side OnPlanComplete handler. 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 diff --git a/internal/iacserver_finalize_test.go b/internal/iacserver_finalize_test.go new file mode 100644 index 0000000..7798410 --- /dev/null +++ b/internal/iacserver_finalize_test.go @@ -0,0 +1,171 @@ +package internal + +// TDD regression gate for workflow#695 Phase 2.5: FinalizeApply MUST fire +// FlushDeferredUpdates on every driver that holds queued state, AND MUST +// preserve per-driver error attribution (Resource/Action/Error shape) when +// a driver's flush fails. Mirrors the v1 wrapper coverage in +// provider_deferred_test.go but exercises the v2 dispatch path via the +// gRPC server's FinalizeApply method. +// +// Coverage: +// - TestDOIaCServer_FinalizeApply_FlushesDeferredUpdates — happy path: +// queued deferred update flushes through FinalizeApply; resp.errors is +// empty; UpdateFirewallRules received the full rule set. +// - TestDOIaCServer_FinalizeApply_PreservesPerDriverErrorAttribution — +// failure path: driver flush returns error; FinalizeApply itself +// succeeds (per the wire-status invariant — gRPC OK with per-driver +// errors in response.errors[]); resp.errors carries the v1 wrapper's +// ActionError{Resource:"infra.database", Action:"deferred_update", +// Error: } attribution shape. +// +// Fixtures reuse fakeAppForDeferred + minimalDBMock from +// provider_deferred_test.go (same package); the new flushErr field on +// minimalDBMock is the only mock-struct extension. Two new setup helpers +// (extracted from the existing TestDOProvider_Apply_FlushesDeferred_* +// fixture patterns) seed a deferred update via direct dbDriver.Create +// call so the test path is independent of DOProvider.Apply (which is the +// v1 path under cleanup post-Phase-2.5). + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/GoCodeAlone/workflow-plugin-digitalocean/internal/drivers" + "github.com/GoCodeAlone/workflow/interfaces" + pb "github.com/GoCodeAlone/workflow/plugin/external/proto" + "github.com/digitalocean/godo" +) + +const finalizeTestAppUUID = "f8b6200c-3bba-48a7-8bf1-7a3e3a885eb5" + +// setupProviderWithQueuedDBDeferredUpdate constructs a minimal DOProvider +// whose database driver has one queued deferred update (trusted_sources +// app-ref → UUID resolution). Extracted from the +// TestDOProvider_Apply_FlushesDeferred_WhenTypeAbsentFromPlan fixture +// pattern to keep the v2-dispatch FinalizeApply tests independent of the +// v1 Apply wrapper path. +func setupProviderWithQueuedDBDeferredUpdate(t *testing.T) (*DOProvider, *minimalDBMock) { + t.Helper() + dbMock := &minimalDBMock{db: &godo.Database{ + ID: "db-abc", + Name: "coredump-staging-db", + Connection: &godo.DatabaseConnection{ + Host: "host.db.ondigitalocean.com", + Port: 5432, + }, + }} + // First Apps.List (during seed Create) returns empty → driver queues + // the deferred update. Subsequent calls (during FinalizeApply's + // FlushDeferredUpdates) return the app so UUID resolution succeeds. + appSeq := &sequencedAppsForDeferred{ + apps: []*godo.App{fakeAppForDeferred("coredump-staging", finalizeTestAppUUID)}, + } + dbDriver := drivers.NewDatabaseDriverWithClients(dbMock, appSeq, "nyc3") + + // Seed via direct Create — first Apps.List returns empty so the + // driver queues a deferred trusted_sources update for the app ref. + if _, err := dbDriver.Create(context.Background(), interfaces.ResourceSpec{ + Name: "coredump-staging-db", + Config: map[string]any{ + "engine": "pg", + "trusted_sources": []any{ + map[string]any{"type": "app", "value": "coredump-staging"}, + }, + }, + }); err != nil { + t.Fatalf("seed Create: %v", err) + } + if !dbDriver.HasDeferredUpdates() { + t.Fatal("test fixture invariant: expected deferred update queued after seed Create") + } + return &DOProvider{ + drivers: map[string]interfaces.ResourceDriver{ + "infra.database": dbDriver, + }, + }, dbMock +} + +// setupProviderWithFailingDBDeferredUpdate is the failure-injection +// counterpart of setupProviderWithQueuedDBDeferredUpdate. Sets the new +// minimalDBMock.flushErr field so UpdateFirewallRules returns the +// configured error when FinalizeApply triggers FlushDeferredUpdates. +// Returns the sentinel error too so tests can assert it round-trips +// into resp.Errors[0].Error. +func setupProviderWithFailingDBDeferredUpdate(t *testing.T) (*DOProvider, *minimalDBMock, error) { + t.Helper() + provider, dbMock := setupProviderWithQueuedDBDeferredUpdate(t) + dbMock.flushErr = errors.New("update firewall rules failed") + return provider, dbMock, dbMock.flushErr +} + +// TestDOIaCServer_FinalizeApply_FlushesDeferredUpdates is the happy-path +// regression gate. 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. +func TestDOIaCServer_FinalizeApply_FlushesDeferredUpdates(t *testing.T) { + provider, dbMock := setupProviderWithQueuedDBDeferredUpdate(t) + s := newDOIaCServer(provider) + + resp, err := s.FinalizeApply(context.Background(), &pb.FinalizeApplyRequest{PlanId: "test-plan-success"}) + + if err != nil { + t.Fatalf("FinalizeApply returned gRPC error (should be in resp.Errors, not top-level): %v", err) + } + if len(resp.GetErrors()) != 0 { + t.Errorf("expected no per-driver errors; got: %+v", resp.GetErrors()) + } + if dbMock.lastFirewallReq == nil { + t.Fatal("UpdateFirewallRules was never called — deferred flush did not run under v2 dispatch via FinalizeApply") + } + if len(dbMock.lastFirewallReq.Rules) != 1 { + t.Fatalf("expected 1 rule in deferred flush, got %d: %+v", + len(dbMock.lastFirewallReq.Rules), dbMock.lastFirewallReq.Rules) + } + rule := dbMock.lastFirewallReq.Rules[0] + if rule.Type != "app" || rule.Value != finalizeTestAppUUID { + t.Errorf("deferred flush rule = {%s %s}, want {app %s}", rule.Type, rule.Value, finalizeTestAppUUID) + } +} + +// TestDOIaCServer_FinalizeApply_PreservesPerDriverErrorAttribution +// is the failure-path regression gate. Locks the wire contract that +// per-driver flush failures surface via response.errors[] (gRPC OK +// status), preserving the v1 wrapper's +// ActionError{Resource, Action, Error} attribution shape so the +// wfctl-side OnPlanComplete handler can render the operator-facing +// diagnostic distinctly per driver. Per ADR 0040 invariants 1 + 2. +func TestDOIaCServer_FinalizeApply_PreservesPerDriverErrorAttribution(t *testing.T) { + provider, _, sentinel := setupProviderWithFailingDBDeferredUpdate(t) + s := newDOIaCServer(provider) + + resp, err := s.FinalizeApply(context.Background(), &pb.FinalizeApplyRequest{PlanId: "test-plan-fail"}) + + // Wire-status invariant (FinalizeApplyResponse godoc, mirrors + // ADR 0040 inv 2): per-driver errors live in response.errors[]; + // gRPC status MUST be OK. A non-OK status would signal transport + // failure distinct from per-driver finalize errors. + if err != nil { + t.Fatalf("FinalizeApply gRPC error (should be in resp.Errors, not top-level): %v", err) + } + if len(resp.GetErrors()) != 1 { + t.Fatalf("expected 1 ActionError in resp; got %d: %+v", len(resp.GetErrors()), resp.GetErrors()) + } + got := resp.GetErrors()[0] + if got.GetResource() != "infra.database" { + t.Errorf("expected Resource=%q; got %q", "infra.database", got.GetResource()) + } + if got.GetAction() != "deferred_update" { + t.Errorf("expected Action=%q; got %q", "deferred_update", got.GetAction()) + } + // Driver wraps flush failures with a "deferred firewall update :" + // prefix before bubbling out of FlushDeferredUpdates; assert substring + // presence rather than full equality so the per-driver attribution + // shape is locked without coupling to the driver-side prefix wording. + // strings.Contains also survives the response.errors[] map-iteration + // order non-determinism if multiple drivers fail in a future test. + if !strings.Contains(got.GetError(), sentinel.Error()) { + t.Errorf("expected Error to contain %q; got %q", sentinel.Error(), got.GetError()) + } +} diff --git a/internal/provider.go b/internal/provider.go index 1a71ae2..e8ca766 100644 --- a/internal/provider.go +++ b/internal/provider.go @@ -246,8 +246,12 @@ type deferredUpdater interface { // PR P-DO TP2: under iacProvider.computePlanVersion: v2 wfctl dispatches // directly through wfctlhelpers.ApplyPlan and does not call this method. // The implementation here remains for legacy v1 callers (wfctl < v0.21.0 -// or any in-process embedder of the gRPC plugin) and for the deferred-flush -// behavior that wfctlhelpers does not yet hoist. +// or any in-process embedder of the gRPC plugin). Post-workflow#695 +// Phase 2.5, v2 callers route the deferred-flush behavior through the +// FinalizeApply RPC (see iacserver.go) which the workflow engine invokes +// via the ApplyPlanHooks.OnPlanComplete hook — wfctlhelpers no longer +// "doesn't hoist" the flush, it just dispatches it through the gRPC +// boundary instead of an in-process method call. // // Per-action upsert recovery, JIT substitution, the Replace cascade, and // the input-drift postcondition all live in wfctlhelpers.ApplyPlan now — diff --git a/internal/provider_deferred_test.go b/internal/provider_deferred_test.go index a7ac479..6beb336 100644 --- a/internal/provider_deferred_test.go +++ b/internal/provider_deferred_test.go @@ -25,10 +25,14 @@ func fakeAppForDeferred(name, id string) *godo.App { // minimalDBMock satisfies drivers.DatabaseClient for the provider-level // deferred flush test. Stores the last UpdateFirewallRules request so the -// test can assert the flush occurred. +// test can assert the flush occurred. The optional flushErr field — when +// non-nil — is returned from UpdateFirewallRules so tests can exercise +// the per-driver error-attribution path (workflow#695 Phase 2.5 +// FinalizeApply regression test). type minimalDBMock struct { db *godo.Database lastFirewallReq *godo.DatabaseUpdateFirewallRulesRequest + flushErr error } func (m *minimalDBMock) Create(_ context.Context, _ *godo.DatabaseCreateRequest) (*godo.Database, *godo.Response, error) { @@ -51,6 +55,9 @@ func (m *minimalDBMock) Delete(_ context.Context, _ string) (*godo.Response, err } func (m *minimalDBMock) UpdateFirewallRules(_ context.Context, _ string, req *godo.DatabaseUpdateFirewallRulesRequest) (*godo.Response, error) { m.lastFirewallReq = req + if m.flushErr != nil { + return nil, m.flushErr + } return nil, nil } From 523c7952e9dc1d71c13abb41c6245edef1963acf Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sun, 17 May 2026 05:35:44 -0400 Subject: [PATCH 3/3] test(v2): lock Capabilities ComputePlanVersion=v2 + tighten failure-path assert + drop self-referential godoc clause MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- internal/iacserver_finalize_test.go | 32 ++++++++++++++++++++++++++++- internal/provider.go | 10 ++++----- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/internal/iacserver_finalize_test.go b/internal/iacserver_finalize_test.go index 7798410..4fcd254 100644 --- a/internal/iacserver_finalize_test.go +++ b/internal/iacserver_finalize_test.go @@ -137,11 +137,22 @@ func TestDOIaCServer_FinalizeApply_FlushesDeferredUpdates(t *testing.T) { // wfctl-side OnPlanComplete handler can render the operator-facing // diagnostic distinctly per driver. Per ADR 0040 invariants 1 + 2. func TestDOIaCServer_FinalizeApply_PreservesPerDriverErrorAttribution(t *testing.T) { - provider, _, sentinel := setupProviderWithFailingDBDeferredUpdate(t) + provider, dbMock, sentinel := setupProviderWithFailingDBDeferredUpdate(t) s := newDOIaCServer(provider) resp, err := s.FinalizeApply(context.Background(), &pb.FinalizeApplyRequest{PlanId: "test-plan-fail"}) + // Belt+suspenders — the error round-trip below implicitly proves the + // flush chain ran, but locking lastFirewallReq != nil directly guards + // against a future refactor where the mock's UpdateFirewallRules + // short-circuits before recording the request (e.g., if flushErr is + // moved to a pre-call gate). The contract under test is "flush was + // attempted AND failed"; the implicit-only assert would mask a + // regression that drops the attempt. + if dbMock.lastFirewallReq == nil { + t.Fatal("UpdateFirewallRules was not attempted — failure-path coverage degraded to flush-never-fired") + } + // Wire-status invariant (FinalizeApplyResponse godoc, mirrors // ADR 0040 inv 2): per-driver errors live in response.errors[]; // gRPC status MUST be OK. A non-OK status would signal transport @@ -169,3 +180,22 @@ func TestDOIaCServer_FinalizeApply_PreservesPerDriverErrorAttribution(t *testing t.Errorf("expected Error to contain %q; got %q", sentinel.Error(), got.GetError()) } } + +// TestDOIaCServer_Capabilities_DeclaresV2 locks the load-bearing +// ComputePlanVersion="v2" opt-in literal in the Capabilities RPC +// return. wfctl's DispatchVersionFor reads this string to route the +// plugin through the v2 apply path (wfctlhelpers.ApplyPlan + per-action +// hooks); a future refactor accidentally dropping the literal (or a +// struct re-init forgetting to set it) silently reverts the plugin to +// v1 dispatch — and the compile-time assert can't catch a string +// omission. Per workflow#695 Phase 2.5. +func TestDOIaCServer_Capabilities_DeclaresV2(t *testing.T) { + s := newDOIaCServer(&DOProvider{}) + caps, err := s.Capabilities(context.Background(), &pb.CapabilitiesRequest{}) + if err != nil { + t.Fatalf("Capabilities: %v", err) + } + if got := caps.GetComputePlanVersion(); got != "v2" { + t.Errorf("CapabilitiesResponse.ComputePlanVersion = %q; want %q (v2 dispatch opt-in lost)", got, "v2") + } +} diff --git a/internal/provider.go b/internal/provider.go index e8ca766..f32404c 100644 --- a/internal/provider.go +++ b/internal/provider.go @@ -246,12 +246,10 @@ type deferredUpdater interface { // PR P-DO TP2: under iacProvider.computePlanVersion: v2 wfctl dispatches // directly through wfctlhelpers.ApplyPlan and does not call this method. // The implementation here remains for legacy v1 callers (wfctl < v0.21.0 -// or any in-process embedder of the gRPC plugin). Post-workflow#695 -// Phase 2.5, v2 callers route the deferred-flush behavior through the -// FinalizeApply RPC (see iacserver.go) which the workflow engine invokes -// via the ApplyPlanHooks.OnPlanComplete hook — wfctlhelpers no longer -// "doesn't hoist" the flush, it just dispatches it through the gRPC -// boundary instead of an in-process method call. +// or any in-process embedder of the gRPC plugin). 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. // // Per-action upsert recovery, JIT substitution, the Replace cascade, and // the input-drift postcondition all live in wfctlhelpers.ApplyPlan now —