diff --git a/internal/drivers/database.go b/internal/drivers/database.go index a8c1485..f645285 100644 --- a/internal/drivers/database.go +++ b/internal/drivers/database.go @@ -577,9 +577,11 @@ func dbOutput(db *godo.Database) *interfaces.ResourceOutput { func (d *DatabaseDriver) ProviderIDFormat() interfaces.ProviderIDFormat { return interfaces.IDFormatUUID } // buildCreateFirewallRulesExcludingApps builds DatabaseCreateFirewallRules from -// the "trusted_sources" config, omitting all type=app entries. Used when -// buildCreateFirewallRules returns ErrAppNotFound so the DB can be created with -// the resolvable subset of rules while app-based rules are deferred. +// the "trusted_sources" config, omitting type=app entries that require name +// resolution (non-UUID values). Used when buildCreateFirewallRules returns +// ErrAppNotFound so the DB can be created with the resolvable subset of rules +// while slug-based app rules are deferred. UUID-shaped type=app values are +// passed through directly since they require no resolution. func (d *DatabaseDriver) buildCreateFirewallRulesExcludingApps(cfg map[string]any) ([]*godo.DatabaseCreateFirewallRule, error) { rawVal, exists := cfg["trusted_sources"] if !exists { @@ -597,8 +599,8 @@ func (d *DatabaseDriver) buildCreateFirewallRulesExcludingApps(cfg map[string]an } ruleType := strFromConfig(m, "type", "") ruleValue := strFromConfig(m, "value", "") - if ruleType == "" || ruleValue == "" || ruleType == "app" { - continue // skip app-type entries; they will be applied in the deferred pass + if ruleType == "" || ruleValue == "" || (ruleType == "app" && !isUUIDLike(ruleValue)) { + continue // skip non-UUID app-type entries; they will be applied in the deferred pass } rules = append(rules, &godo.DatabaseCreateFirewallRule{ Type: ruleType, @@ -609,9 +611,11 @@ func (d *DatabaseDriver) buildCreateFirewallRulesExcludingApps(cfg map[string]an } // buildUpdateFirewallRulesExcludingApps builds DatabaseFirewallRules from the -// "trusted_sources" config, omitting all type=app entries. Used when -// buildUpdateFirewallRules returns ErrAppNotFound so the partial rule set can -// be applied immediately while app-based rules are deferred. +// "trusted_sources" config, omitting type=app entries that require name +// resolution (non-UUID values). Used when buildUpdateFirewallRules returns +// ErrAppNotFound so the partial rule set can be applied immediately while +// slug-based app rules are deferred. UUID-shaped type=app values are passed +// through directly since they require no resolution. func (d *DatabaseDriver) buildUpdateFirewallRulesExcludingApps(cfg map[string]any) ([]*godo.DatabaseFirewallRule, bool, error) { raw, ok := cfg["trusted_sources"] if !ok { @@ -629,8 +633,8 @@ func (d *DatabaseDriver) buildUpdateFirewallRulesExcludingApps(cfg map[string]an } ruleType := strFromConfig(m, "type", "") ruleValue := strFromConfig(m, "value", "") - if ruleType == "" || ruleValue == "" || ruleType == "app" { - continue // skip app-type entries; they will be applied in the deferred pass + if ruleType == "" || ruleValue == "" || (ruleType == "app" && !isUUIDLike(ruleValue)) { + continue // skip non-UUID app-type entries; they will be applied in the deferred pass } rules = append(rules, &godo.DatabaseFirewallRule{ Type: ruleType, diff --git a/internal/drivers/database_deferred_test.go b/internal/drivers/database_deferred_test.go index 2a27354..8392c31 100644 --- a/internal/drivers/database_deferred_test.go +++ b/internal/drivers/database_deferred_test.go @@ -311,3 +311,92 @@ func TestDatabaseDriver_FlushDeferredUpdates_NoopWhenEmpty(t *testing.T) { t.Fatalf("FlushDeferredUpdates on empty driver should return nil; got: %v", err) } } + +// --- UUID passthrough tests --- + +// TestDatabaseDriver_Create_AppUUID_PassedThroughNotDeferred verifies that a +// type=app trusted_sources entry with a UUID-shaped value is included directly +// in the create request (not deferred), even when other slug-style entries +// trigger the deferred path because they cannot be resolved. +func TestDatabaseDriver_Create_AppUUID_PassedThroughNotDeferred(t *testing.T) { + const appUUID = "f8b6200c-3bba-48a7-8bf1-7a3e3a885eb5" + dbMock := &mockDatabaseClient{db: testDatabase()} + // App client returns empty — slug entry unresolvable; UUID entry must pass through. + appMock := &mockAppClient{listApps: []*godo.App{}} + d := drivers.NewDatabaseDriverWithClients(dbMock, appMock, "nyc3") + + _, err := d.Create(context.Background(), interfaces.ResourceSpec{ + Name: "my-db", + Config: map[string]any{ + "engine": "pg", + "trusted_sources": []any{ + map[string]any{"type": "app", "value": appUUID}, // already a UUID — pass through + map[string]any{"type": "app", "value": "coredump-staging"}, // slug — deferred + }, + }, + }) + if err != nil { + t.Fatalf("Create: %v", err) + } + if dbMock.lastCreateReq == nil { + t.Fatal("no create request captured") + } + // UUID entry must appear in the create request; slug entry is deferred. + if len(dbMock.lastCreateReq.Rules) != 1 { + t.Fatalf("expected 1 rule in create request (UUID passthrough only), got %d: %+v", + len(dbMock.lastCreateReq.Rules), dbMock.lastCreateReq.Rules) + } + if dbMock.lastCreateReq.Rules[0].Type != "app" || dbMock.lastCreateReq.Rules[0].Value != appUUID { + t.Errorf("create rule = {%s %s}, want {app %s}", + dbMock.lastCreateReq.Rules[0].Type, dbMock.lastCreateReq.Rules[0].Value, appUUID) + } + // The slug entry must still be deferred. + if !d.HasDeferredUpdates() { + t.Error("HasDeferredUpdates() should be true — slug entry must still be deferred") + } +} + +// TestDatabaseDriver_Update_AppUUID_PassedThroughNotDeferred verifies that a +// type=app trusted_sources entry with a UUID-shaped value is included in the +// immediate partial-update call (not deferred), even when other slug-style +// entries trigger the deferred path. +func TestDatabaseDriver_Update_AppUUID_PassedThroughNotDeferred(t *testing.T) { + const appUUID = "f8b6200c-3bba-48a7-8bf1-7a3e3a885eb5" + dbMock := &mockDatabaseClient{db: testDatabase()} + // App client returns empty — slug entry unresolvable; UUID entry must pass through. + appMock := &mockAppClient{listApps: []*godo.App{}} + d := drivers.NewDatabaseDriverWithClients(dbMock, appMock, "nyc3") + + _, err := d.Update(context.Background(), + interfaces.ResourceRef{Name: "my-db", ProviderID: "db-123"}, + interfaces.ResourceSpec{ + Name: "my-db", + Config: map[string]any{ + "size": "db-s-1vcpu-1gb", + "trusted_sources": []any{ + map[string]any{"type": "app", "value": appUUID}, // already a UUID — pass through + map[string]any{"type": "app", "value": "coredump-staging"}, // slug — deferred + }, + }, + }, + ) + if err != nil { + t.Fatalf("Update: %v", err) + } + if dbMock.lastFirewallReq == nil { + t.Fatal("UpdateFirewallRules should be called with partial rules") + } + // UUID entry must appear in the partial update; slug entry is deferred. + if len(dbMock.lastFirewallReq.Rules) != 1 { + t.Fatalf("expected 1 rule in partial update (UUID passthrough only), got %d: %+v", + len(dbMock.lastFirewallReq.Rules), dbMock.lastFirewallReq.Rules) + } + if dbMock.lastFirewallReq.Rules[0].Type != "app" || dbMock.lastFirewallReq.Rules[0].Value != appUUID { + t.Errorf("partial update rule = {%s %s}, want {app %s}", + dbMock.lastFirewallReq.Rules[0].Type, dbMock.lastFirewallReq.Rules[0].Value, appUUID) + } + // The slug entry must still be deferred. + if !d.HasDeferredUpdates() { + t.Error("HasDeferredUpdates() should be true — slug entry must still be deferred") + } +} diff --git a/internal/provider.go b/internal/provider.go index 026ac29..7a08700 100644 --- a/internal/provider.go +++ b/internal/provider.go @@ -278,25 +278,21 @@ func (p *DOProvider) Apply(ctx context.Context, plan *interfaces.IaCPlan) (*inte // main action loop. These arise when a resource's config (e.g. DB // trusted_sources with type=app) references another resource provisioned // later in the same plan. By this point all plan creates have completed - // and the referenced resources exist. Each driver type is flushed at - // most once. - seen := make(map[string]struct{}, len(plan.Actions)) - for _, action := range plan.Actions { - if _, done := seen[action.Resource.Type]; done { - continue - } - seen[action.Resource.Type] = struct{}{} - d, dErr := p.ResourceDriver(action.Resource.Type) - if dErr != nil { - continue - } + // and the referenced resources exist. + // + // 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). Each driver is checked at most once regardless of how + // many actions reference it. + for resourceType, d := range p.drivers { du, ok := d.(deferredUpdater) if !ok || !du.HasDeferredUpdates() { continue } if flushErr := du.FlushDeferredUpdates(ctx); flushErr != nil { result.Errors = append(result.Errors, interfaces.ActionError{ - Resource: action.Resource.Name, + Resource: resourceType, Action: "deferred_update", Error: flushErr.Error(), }) diff --git a/internal/provider_deferred_test.go b/internal/provider_deferred_test.go index 12b2f50..a7ac479 100644 --- a/internal/provider_deferred_test.go +++ b/internal/provider_deferred_test.go @@ -142,3 +142,87 @@ func TestDOProvider_Apply_FlushesDeferred_AfterAllCreates(t *testing.T) { t.Errorf("deferred flush rule = {%s %s}, want {app %s}", rule.Type, rule.Value, appUUID) } } + +// TestDOProvider_Apply_FlushesDeferred_WhenTypeAbsentFromPlan verifies that +// the deferred-flush second pass iterates the driver registry (not plan.Actions), +// so orphaned deferred entries are flushed even when their resource type does +// not appear in the current plan. +// +// Scenario: +// 1. Seed the database driver with a deferred update by calling Create directly +// (simulating a prior Apply run where the flush failed transiently). +// 2. Run Apply with a plan that has NO infra.database actions. +// 3. Expect FlushDeferredUpdates to still run and call UpdateFirewallRules. +func TestDOProvider_Apply_FlushesDeferred_WhenTypeAbsentFromPlan(t *testing.T) { + const appUUID = "f8b6200c-3bba-48a7-8bf1-7a3e3a885eb5" + + dbMock := &minimalDBMock{db: &godo.Database{ + ID: "db-abc", + Name: "coredump-staging-db", + Connection: &godo.DatabaseConnection{ + Host: "host.db.ondigitalocean.com", + Port: 5432, + }, + }} + // First call (Create, called directly below) returns empty — app absent; + // subsequent calls (FlushDeferredUpdates, called via Apply) return the app. + appSeq := &sequencedAppsForDeferred{ + apps: []*godo.App{fakeAppForDeferred("coredump-staging", appUUID)}, + } + dbDriver := drivers.NewDatabaseDriverWithClients(dbMock, appSeq, "nyc3") + + // Seed the driver with a deferred update by calling Create directly. + // The first Apps.List call returns empty → deferred update queued. + _, 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"}, + }, + }, + }) + if err != nil { + t.Fatalf("seed Create: %v", err) + } + if !dbDriver.HasDeferredUpdates() { + t.Fatal("expected deferred update queued after seed Create") + } + + p := &DOProvider{ + drivers: map[string]interfaces.ResourceDriver{ + "infra.database": dbDriver, + }, + } + + // Apply with an empty plan — no infra.database actions. + // The deferred flush must still run because Apply iterates p.drivers. + plan := &interfaces.IaCPlan{ + ID: "test-plan-no-db-actions", + Actions: []interfaces.PlanAction{}, + } + + result, err := p.Apply(context.Background(), plan) + if err != nil { + t.Fatalf("Apply: %v", err) + } + if len(result.Errors) > 0 { + t.Fatalf("Apply result errors: %+v", result.Errors) + } + + // Flush must have called UpdateFirewallRules with app UUID. + if dbMock.lastFirewallReq == nil { + t.Fatal("UpdateFirewallRules was never called — orphaned deferred entry was not flushed") + } + 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 != appUUID { + t.Errorf("deferred flush rule = {%s %s}, want {app %s}", rule.Type, rule.Value, appUUID) + } + if dbDriver.HasDeferredUpdates() { + t.Error("HasDeferredUpdates() should be false after successful flush") + } +}