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..a6d364d 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,62 @@ 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). +// +// 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 + // 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/internal/iacserver_finalize_test.go b/internal/iacserver_finalize_test.go new file mode 100644 index 0000000..4fcd254 --- /dev/null +++ b/internal/iacserver_finalize_test.go @@ -0,0 +1,201 @@ +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, 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 + // 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()) + } +} + +// 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 1a71ae2..f32404c 100644 --- a/internal/provider.go +++ b/internal/provider.go @@ -246,8 +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) and for the deferred-flush -// behavior that wfctlhelpers does not yet hoist. +// 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 — 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 } 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": {