From 21d13b65743571cff2829e1b0c06ca4fd51ce3e7 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 25 Apr 2026 02:40:55 -0400 Subject: [PATCH 1/9] fix(database): resolve type=app trusted_source names to UUIDs before DO API call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The DO database firewall API requires an app UUID for type=app rules. Passing an app name directly results in a 422 "invalid app uuid" error. Add DatabaseDriver.resolveAppName which, when the value is not already UUID-shaped, looks up the app by name via the Apps API. Wire AppPlatformClient into DatabaseDriver at construction time (NewDatabaseDriver wires c.Apps; NewDatabaseDriverWithClients supports injection for tests). Replace the plain trustedSourceCreateRulesFromConfig / trustedSourceFirewallRulesFromConfig calls in Create and Update with context-aware buildCreateFirewallRules and buildUpdateFirewallRules methods that call resolveAppName for type=app entries. Add five new unit tests: - AppNameResolved (Create + Update): name → UUID resolved via mock Apps list - AppUUIDPassThrough: UUID-shaped values skip the list call entirely - AppNameNotFound: returns error when app is not in the list - NoAppsClient: returns an actionable error when appsClient is nil Co-Authored-By: Claude Sonnet 4.6 --- internal/drivers/database.go | 138 ++++++++++++++++++++++++-- internal/drivers/database_test.go | 155 ++++++++++++++++++++++++++++++ 2 files changed, 287 insertions(+), 6 deletions(-) diff --git a/internal/drivers/database.go b/internal/drivers/database.go index f72aaee..e10adb0 100644 --- a/internal/drivers/database.go +++ b/internal/drivers/database.go @@ -22,20 +22,30 @@ type DatabaseClient interface { // DatabaseDriver manages DigitalOcean Managed Databases (infra.database). // Supports pg, mysql, redis, mongodb, kafka. type DatabaseDriver struct { - client DatabaseClient - region string + client DatabaseClient + appsClient AppPlatformClient // optional; used to resolve type=app trusted_source names to UUIDs + region string } // NewDatabaseDriver creates a DatabaseDriver backed by a real godo client. +// The godo Apps client is wired automatically to support type=app trusted_source +// name → UUID resolution at apply time. func NewDatabaseDriver(c *godo.Client, region string) *DatabaseDriver { - return &DatabaseDriver{client: c.Databases, region: region} + return &DatabaseDriver{client: c.Databases, appsClient: c.Apps, region: region} } -// NewDatabaseDriverWithClient creates a driver with an injected client (for tests). +// NewDatabaseDriverWithClient creates a driver with an injected database client (for tests). +// appsClient may be nil when tests do not exercise type=app trusted_source rules. func NewDatabaseDriverWithClient(c DatabaseClient, region string) *DatabaseDriver { return &DatabaseDriver{client: c, region: region} } +// NewDatabaseDriverWithClients creates a driver with injected database and apps +// clients (for tests that exercise type=app trusted_source name → UUID resolution). +func NewDatabaseDriverWithClients(c DatabaseClient, apps AppPlatformClient, region string) *DatabaseDriver { + return &DatabaseDriver{client: c, appsClient: apps, region: region} +} + func (d *DatabaseDriver) Create(ctx context.Context, spec interfaces.ResourceSpec) (*interfaces.ResourceOutput, error) { engine := strFromConfig(spec.Config, "engine", "pg") version := strFromConfig(spec.Config, "version", "") @@ -43,6 +53,11 @@ func (d *DatabaseDriver) Create(ctx context.Context, spec interfaces.ResourceSpe region := strFromConfig(spec.Config, "region", d.region) numNodes, _ := intFromConfig(spec.Config, "num_nodes", 1) + createRules, err := d.buildCreateFirewallRules(ctx, spec.Config) + if err != nil { + return nil, fmt.Errorf("database create %q: %w", spec.Name, err) + } + req := &godo.DatabaseCreateRequest{ Name: spec.Name, EngineSlug: engine, @@ -50,7 +65,7 @@ func (d *DatabaseDriver) Create(ctx context.Context, spec interfaces.ResourceSpe SizeSlug: size, Region: region, NumNodes: numNodes, - Rules: trustedSourceCreateRulesFromConfig(spec.Config), + Rules: createRules, } db, _, err := d.client.Create(ctx, req) @@ -133,7 +148,9 @@ func (d *DatabaseDriver) Update(ctx context.Context, ref interfaces.ResourceRef, } // Sync firewall rules when trusted_sources key is present in config. // "present but empty" clears all rules; absent key leaves existing rules unchanged. - if rules, ok := trustedSourceFirewallRulesFromConfig(spec.Config); ok { + if rules, ok, fwErr := d.buildUpdateFirewallRules(ctx, spec.Config); fwErr != nil { + return nil, fmt.Errorf("database update firewall %q: %w", ref.Name, fwErr) + } else if ok { _, err = d.client.UpdateFirewallRules(ctx, providerID, &godo.DatabaseUpdateFirewallRulesRequest{ Rules: rules, }) @@ -207,9 +224,115 @@ func (d *DatabaseDriver) SensitiveKeys() []string { return []string{"uri", "password", "user"} } +// resolveAppName resolves an App Platform app name to its DO UUID for use in +// trusted_sources firewall rules. If name is already UUID-shaped it is returned +// unchanged. When appsClient is nil and the name is not a UUID, an error is +// returned so the caller can surface a clear message rather than letting the +// DO API reject the request with a cryptic 422. +func (d *DatabaseDriver) resolveAppName(ctx context.Context, name string) (string, error) { + if isUUIDLike(name) { + return name, nil + } + if d.appsClient == nil { + return "", fmt.Errorf( + "trusted_source type=app value %q is not a UUID and no Apps client is configured for name lookup; pass the app UUID directly", + name, + ) + } + opts := &godo.ListOptions{Page: 1, PerPage: 200} + for { + apps, resp, err := d.appsClient.List(ctx, opts) + if err != nil { + return "", fmt.Errorf("app list for trusted_source resolution: %w", WrapGodoError(err)) + } + for _, app := range apps { + if app.Spec != nil && app.Spec.Name == name { + return app.ID, nil + } + } + if resp == nil || resp.Links == nil || resp.Links.IsLastPage() { + break + } + opts.Page++ + } + return "", fmt.Errorf("trusted_source app %q: %w", name, ErrResourceNotFound) +} + +// buildCreateFirewallRules converts "trusted_sources" config to +// []*godo.DatabaseCreateFirewallRule, resolving type=app values to UUIDs. +func (d *DatabaseDriver) buildCreateFirewallRules(ctx context.Context, cfg map[string]any) ([]*godo.DatabaseCreateFirewallRule, error) { + raw, ok := cfg["trusted_sources"].([]any) + if !ok || len(raw) == 0 { + return nil, nil + } + rules := make([]*godo.DatabaseCreateFirewallRule, 0, len(raw)) + for _, v := range raw { + m, ok := v.(map[string]any) + if !ok { + continue + } + ruleType := strFromConfig(m, "type", "") + ruleValue := strFromConfig(m, "value", "") + if ruleType == "" || ruleValue == "" { + continue + } + if ruleType == "app" { + resolved, err := d.resolveAppName(ctx, ruleValue) + if err != nil { + return nil, err + } + ruleValue = resolved + } + rules = append(rules, &godo.DatabaseCreateFirewallRule{ + Type: ruleType, + Value: ruleValue, + }) + } + return rules, nil +} + +// buildUpdateFirewallRules converts "trusted_sources" config to +// ([]*godo.DatabaseFirewallRule, bool, error). The bool mirrors +// trustedSourceFirewallRulesFromConfig: false means key absent (leave unchanged); +// true means key present (apply the returned slice, even if empty). type=app +// values are resolved to UUIDs. +func (d *DatabaseDriver) buildUpdateFirewallRules(ctx context.Context, cfg map[string]any) ([]*godo.DatabaseFirewallRule, bool, error) { + raw, ok := cfg["trusted_sources"] + if !ok { + return nil, false, nil // key absent: leave existing rules unchanged + } + list, _ := raw.([]any) + rules := make([]*godo.DatabaseFirewallRule, 0, len(list)) + for _, v := range list { + m, ok := v.(map[string]any) + if !ok { + continue + } + ruleType := strFromConfig(m, "type", "") + ruleValue := strFromConfig(m, "value", "") + if ruleType == "" || ruleValue == "" { + continue + } + if ruleType == "app" { + resolved, err := d.resolveAppName(ctx, ruleValue) + if err != nil { + return nil, false, err + } + ruleValue = resolved + } + rules = append(rules, &godo.DatabaseFirewallRule{ + Type: ruleType, + Value: ruleValue, + }) + } + return rules, true, nil +} + // trustedSourceCreateRulesFromConfig converts canonical "trusted_sources" to // []*godo.DatabaseCreateFirewallRule for use in a DatabaseCreateRequest. // Each entry must have "type" (ip_addr|k8s|app|droplet|tag) and "value". +// NOTE: this function does NOT resolve type=app names to UUIDs. Use +// buildCreateFirewallRules (a DatabaseDriver method) when a context is available. func trustedSourceCreateRulesFromConfig(cfg map[string]any) []*godo.DatabaseCreateFirewallRule { raw, ok := cfg["trusted_sources"].([]any) if !ok || len(raw) == 0 { @@ -240,6 +363,9 @@ func trustedSourceCreateRulesFromConfig(cfg map[string]any) []*godo.DatabaseCrea // - false → key absent → caller should leave existing firewall rules unchanged. // - true, empty slice → key present but empty → caller should clear all rules. // - true, non-empty slice → key present with rules → caller should apply the rules. +// +// NOTE: this function does NOT resolve type=app names to UUIDs. Use +// buildUpdateFirewallRules (a DatabaseDriver method) when a context is available. func trustedSourceFirewallRulesFromConfig(cfg map[string]any) ([]*godo.DatabaseFirewallRule, bool) { raw, ok := cfg["trusted_sources"] if !ok { diff --git a/internal/drivers/database_test.go b/internal/drivers/database_test.go index 5c40801..53798b1 100644 --- a/internal/drivers/database_test.go +++ b/internal/drivers/database_test.go @@ -424,3 +424,158 @@ func TestDatabaseDriver_Create_ProviderIDIsAPIAssigned(t *testing.T) { t.Errorf("ProviderID must not be empty") } } + +// --- type=app trusted_source name→UUID resolution --- + +func makeAppForTrustedSource(name, id string) *godo.App { + return &godo.App{ + ID: id, + Spec: &godo.AppSpec{Name: name}, + ActiveDeployment: &godo.Deployment{ + Phase: godo.DeploymentPhase_Active, + }, + } +} + +func TestDatabaseDriver_Create_WithTrustedSources_AppNameResolved(t *testing.T) { + // type=app rule whose value is an app name (not UUID) should be resolved + // to the app's UUID before sending to the DO API. + const appUUID = "f8b6200c-3bba-48a7-8bf1-7a3e3a885eb5" + dbMock := &mockDatabaseClient{db: testDatabase()} + appMock := &mockAppClient{ + listApps: []*godo.App{makeAppForTrustedSource("bmw-staging", appUUID)}, + } + 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": "bmw-staging"}, + }, + }, + }) + if err != nil { + t.Fatalf("Create: %v", err) + } + if dbMock.lastCreateReq == nil { + t.Fatal("no create request captured") + } + if len(dbMock.lastCreateReq.Rules) != 1 { + t.Fatalf("expected 1 rule, got %d", len(dbMock.lastCreateReq.Rules)) + } + rule := dbMock.lastCreateReq.Rules[0] + if rule.Type != "app" { + t.Errorf("rule.Type = %q, want %q", rule.Type, "app") + } + if rule.Value != appUUID { + t.Errorf("rule.Value = %q, want UUID %q (app name should have been resolved)", rule.Value, appUUID) + } +} + +func TestDatabaseDriver_Create_WithTrustedSources_AppUUIDPassThrough(t *testing.T) { + // type=app rule whose value is already UUID-shaped must NOT trigger a name lookup. + const appUUID = "f8b6200c-3bba-48a7-8bf1-7a3e3a885eb5" + dbMock := &mockDatabaseClient{db: testDatabase()} + appMock := &mockAppClient{listErr: fmt.Errorf("should not be called")} + 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}, + }, + }, + }) + if err != nil { + t.Fatalf("Create: %v (apps List must not be called for UUID-shaped values)", err) + } + if dbMock.lastCreateReq == nil || len(dbMock.lastCreateReq.Rules) != 1 { + t.Fatalf("expected 1 rule") + } + if dbMock.lastCreateReq.Rules[0].Value != appUUID { + t.Errorf("rule.Value = %q, want %q", dbMock.lastCreateReq.Rules[0].Value, appUUID) + } +} + +func TestDatabaseDriver_Update_WithTrustedSources_AppNameResolved(t *testing.T) { + // type=app rule in an Update firewall call should also resolve name→UUID. + const appUUID = "f8b6200c-3bba-48a7-8bf1-7a3e3a885eb5" + dbMock := &mockDatabaseClient{db: testDatabase()} + appMock := &mockAppClient{ + listApps: []*godo.App{makeAppForTrustedSource("bmw-staging", appUUID)}, + } + 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-2vcpu-4gb", + "trusted_sources": []any{ + map[string]any{"type": "app", "value": "bmw-staging"}, + }, + }, + }) + if err != nil { + t.Fatalf("Update: %v", err) + } + if dbMock.lastFirewallReq == nil { + t.Fatal("UpdateFirewallRules was not called") + } + if len(dbMock.lastFirewallReq.Rules) != 1 { + t.Fatalf("expected 1 rule, got %d", len(dbMock.lastFirewallReq.Rules)) + } + rule := dbMock.lastFirewallReq.Rules[0] + if rule.Type != "app" { + t.Errorf("rule.Type = %q, want %q", rule.Type, "app") + } + if rule.Value != appUUID { + t.Errorf("rule.Value = %q, want UUID %q (app name should have been resolved)", rule.Value, appUUID) + } +} + +func TestDatabaseDriver_Create_AppNameNotFound_ReturnsError(t *testing.T) { + // When the app name cannot be found in the Apps list, Create must return an error. + dbMock := &mockDatabaseClient{db: testDatabase()} + appMock := &mockAppClient{listApps: []*godo.App{}} // empty list — name not found + 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": "nonexistent-app"}, + }, + }, + }) + if err == nil { + t.Fatal("expected error when app name not found, got nil") + } +} + +func TestDatabaseDriver_Create_AppType_NoAppsClient_ReturnsError(t *testing.T) { + // When no Apps client is configured and the rule value is not UUID-shaped, + // Create must return an actionable error rather than sending a bad value to DO. + dbMock := &mockDatabaseClient{db: testDatabase()} + // NewDatabaseDriverWithClient does NOT wire an apps client. + d := drivers.NewDatabaseDriverWithClient(dbMock, "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": "bmw-staging"}, + }, + }, + }) + if err == nil { + t.Fatal("expected error when apps client is nil and value is not UUID, got nil") + } +} From 91dc82f81c5739a92c03c0260a96ed594683d41c Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 25 Apr 2026 02:50:27 -0400 Subject: [PATCH 2/9] =?UTF-8?q?fix(database):=20batch=20type=3Dapp=20name?= =?UTF-8?q?=E2=86=92UUID=20resolution;=20strengthen=20error=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address Copilot review on PR #25: - Replace per-rule resolveAppName (one Apps.List scan per rule) with resolveAppNamesMap, which collects all non-UUID app names and resolves them in a single paginated Apps.List pass regardless of how many type=app rules are present, reducing API calls and rate-limit risk. - Remove now-unused resolveAppName helper. - TestDatabaseDriver_Create_AppNameNotFound_ReturnsError: assert errors.Is(err, ErrResourceNotFound) and that the missing name is included in the error message. - TestDatabaseDriver_Create_AppType_NoAppsClient_ReturnsError: assert error message contains "not a UUID" and the offending value so the message cannot silently regress to a cryptic form. Co-Authored-By: Claude Sonnet 4.6 --- internal/drivers/database.go | 88 ++++++++++++++++++++++--------- internal/drivers/database_test.go | 19 ++++++- 2 files changed, 79 insertions(+), 28 deletions(-) diff --git a/internal/drivers/database.go b/internal/drivers/database.go index e10adb0..7e5d866 100644 --- a/internal/drivers/database.go +++ b/internal/drivers/database.go @@ -224,30 +224,53 @@ func (d *DatabaseDriver) SensitiveKeys() []string { return []string{"uri", "password", "user"} } -// resolveAppName resolves an App Platform app name to its DO UUID for use in -// trusted_sources firewall rules. If name is already UUID-shaped it is returned -// unchanged. When appsClient is nil and the name is not a UUID, an error is -// returned so the caller can surface a clear message rather than letting the -// DO API reject the request with a cryptic 422. -func (d *DatabaseDriver) resolveAppName(ctx context.Context, name string) (string, error) { - if isUUIDLike(name) { - return name, nil +// resolveAppNamesMap builds a name→UUID map for all non-UUID app names found +// in the raw trusted_sources list. A single paginated Apps.List call is made +// regardless of how many type=app rules exist, avoiding repeated full scans. +// Returns an error only if the Apps client is unavailable for a non-UUID name, +// or if the DO API call fails. +func (d *DatabaseDriver) resolveAppNamesMap(ctx context.Context, raw []any) (map[string]string, error) { + // Collect the distinct non-UUID app names that need resolution. + needed := map[string]struct{}{} + for _, v := range raw { + m, ok := v.(map[string]any) + if !ok { + continue + } + if strFromConfig(m, "type", "") != "app" { + continue + } + val := strFromConfig(m, "value", "") + if val == "" || isUUIDLike(val) { + continue + } + needed[val] = struct{}{} + } + if len(needed) == 0 { + return nil, nil // nothing to look up } if d.appsClient == nil { - return "", fmt.Errorf( - "trusted_source type=app value %q is not a UUID and no Apps client is configured for name lookup; pass the app UUID directly", - name, - ) + // Surface the first offending name for a clear error message. + for name := range needed { + return nil, fmt.Errorf( + "trusted_source type=app value %q is not a UUID and no Apps client is configured for name lookup; pass the app UUID directly", + name, + ) + } } + resolved := make(map[string]string, len(needed)) opts := &godo.ListOptions{Page: 1, PerPage: 200} for { apps, resp, err := d.appsClient.List(ctx, opts) if err != nil { - return "", fmt.Errorf("app list for trusted_source resolution: %w", WrapGodoError(err)) + return nil, fmt.Errorf("app list for trusted_source resolution: %w", WrapGodoError(err)) } for _, app := range apps { - if app.Spec != nil && app.Spec.Name == name { - return app.ID, nil + if app.Spec == nil { + continue + } + if _, want := needed[app.Spec.Name]; want { + resolved[app.Spec.Name] = app.ID } } if resp == nil || resp.Links == nil || resp.Links.IsLastPage() { @@ -255,16 +278,27 @@ func (d *DatabaseDriver) resolveAppName(ctx context.Context, name string) (strin } opts.Page++ } - return "", fmt.Errorf("trusted_source app %q: %w", name, ErrResourceNotFound) + // Verify every requested name was found. + for name := range needed { + if _, ok := resolved[name]; !ok { + return nil, fmt.Errorf("trusted_source app %q: %w", name, ErrResourceNotFound) + } + } + return resolved, nil } // buildCreateFirewallRules converts "trusted_sources" config to -// []*godo.DatabaseCreateFirewallRule, resolving type=app values to UUIDs. +// []*godo.DatabaseCreateFirewallRule, resolving type=app values to UUIDs via +// a single Apps.List pass (see resolveAppNamesMap). func (d *DatabaseDriver) buildCreateFirewallRules(ctx context.Context, cfg map[string]any) ([]*godo.DatabaseCreateFirewallRule, error) { raw, ok := cfg["trusted_sources"].([]any) if !ok || len(raw) == 0 { return nil, nil } + appUUIDs, err := d.resolveAppNamesMap(ctx, raw) + if err != nil { + return nil, err + } rules := make([]*godo.DatabaseCreateFirewallRule, 0, len(raw)) for _, v := range raw { m, ok := v.(map[string]any) @@ -277,11 +311,10 @@ func (d *DatabaseDriver) buildCreateFirewallRules(ctx context.Context, cfg map[s continue } if ruleType == "app" { - resolved, err := d.resolveAppName(ctx, ruleValue) - if err != nil { - return nil, err + if uuid, found := appUUIDs[ruleValue]; found { + ruleValue = uuid } - ruleValue = resolved + // UUID-shaped values are used as-is (not in the appUUIDs map). } rules = append(rules, &godo.DatabaseCreateFirewallRule{ Type: ruleType, @@ -295,13 +328,17 @@ func (d *DatabaseDriver) buildCreateFirewallRules(ctx context.Context, cfg map[s // ([]*godo.DatabaseFirewallRule, bool, error). The bool mirrors // trustedSourceFirewallRulesFromConfig: false means key absent (leave unchanged); // true means key present (apply the returned slice, even if empty). type=app -// values are resolved to UUIDs. +// values are resolved to UUIDs via a single Apps.List pass (see resolveAppNamesMap). func (d *DatabaseDriver) buildUpdateFirewallRules(ctx context.Context, cfg map[string]any) ([]*godo.DatabaseFirewallRule, bool, error) { raw, ok := cfg["trusted_sources"] if !ok { return nil, false, nil // key absent: leave existing rules unchanged } list, _ := raw.([]any) + appUUIDs, err := d.resolveAppNamesMap(ctx, list) + if err != nil { + return nil, false, err + } rules := make([]*godo.DatabaseFirewallRule, 0, len(list)) for _, v := range list { m, ok := v.(map[string]any) @@ -314,11 +351,10 @@ func (d *DatabaseDriver) buildUpdateFirewallRules(ctx context.Context, cfg map[s continue } if ruleType == "app" { - resolved, err := d.resolveAppName(ctx, ruleValue) - if err != nil { - return nil, false, err + if uuid, found := appUUIDs[ruleValue]; found { + ruleValue = uuid } - ruleValue = resolved + // UUID-shaped values are used as-is (not in the appUUIDs map). } rules = append(rules, &godo.DatabaseFirewallRule{ Type: ruleType, diff --git a/internal/drivers/database_test.go b/internal/drivers/database_test.go index 53798b1..8f8e94e 100644 --- a/internal/drivers/database_test.go +++ b/internal/drivers/database_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "strings" "testing" "github.com/GoCodeAlone/workflow-plugin-digitalocean/internal/drivers" @@ -540,7 +541,8 @@ func TestDatabaseDriver_Update_WithTrustedSources_AppNameResolved(t *testing.T) } func TestDatabaseDriver_Create_AppNameNotFound_ReturnsError(t *testing.T) { - // When the app name cannot be found in the Apps list, Create must return an error. + // When the app name cannot be found in the Apps list, Create must return an + // error that wraps ErrResourceNotFound and names the missing app. dbMock := &mockDatabaseClient{db: testDatabase()} appMock := &mockAppClient{listApps: []*godo.App{}} // empty list — name not found d := drivers.NewDatabaseDriverWithClients(dbMock, appMock, "nyc3") @@ -557,11 +559,18 @@ func TestDatabaseDriver_Create_AppNameNotFound_ReturnsError(t *testing.T) { if err == nil { t.Fatal("expected error when app name not found, got nil") } + if !errors.Is(err, drivers.ErrResourceNotFound) { + t.Errorf("expected error to wrap drivers.ErrResourceNotFound; got: %v", err) + } + if !strings.Contains(err.Error(), "nonexistent-app") { + t.Errorf("expected error to mention missing app name %q; got: %v", "nonexistent-app", err) + } } func TestDatabaseDriver_Create_AppType_NoAppsClient_ReturnsError(t *testing.T) { // When no Apps client is configured and the rule value is not UUID-shaped, - // Create must return an actionable error rather than sending a bad value to DO. + // Create must return an actionable error (not a generic one) that tells the + // caller the value is not a UUID and no client is available for lookup. dbMock := &mockDatabaseClient{db: testDatabase()} // NewDatabaseDriverWithClient does NOT wire an apps client. d := drivers.NewDatabaseDriverWithClient(dbMock, "nyc3") @@ -578,4 +587,10 @@ func TestDatabaseDriver_Create_AppType_NoAppsClient_ReturnsError(t *testing.T) { if err == nil { t.Fatal("expected error when apps client is nil and value is not UUID, got nil") } + if !strings.Contains(err.Error(), "not a UUID") { + t.Errorf("expected error to mention 'not a UUID'; got: %v", err) + } + if !strings.Contains(err.Error(), "bmw-staging") { + t.Errorf("expected error to mention the offending value %q; got: %v", "bmw-staging", err) + } } From 91c3cf684243cb33c44fd2f1ad8eec61a1bcef6e Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 25 Apr 2026 02:59:33 -0400 Subject: [PATCH 3/9] fix(database): accurate docstring + early-exit pagination in resolveAppNamesMap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address Copilot round-3 review on PR #25: - Docstring: "A single paginated Apps.List call is made" was misleading (one call per page). Reworded to "a single paginated listing pass (potentially multiple page requests)" — accurate without overpromising. - Early exit: once len(resolved) == len(needed), return immediately without fetching further pages. Avoids scanning the full Apps list in accounts with many apps when all names are found early. Co-Authored-By: Claude Sonnet 4.6 --- internal/drivers/database.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/internal/drivers/database.go b/internal/drivers/database.go index 7e5d866..9495753 100644 --- a/internal/drivers/database.go +++ b/internal/drivers/database.go @@ -225,10 +225,11 @@ func (d *DatabaseDriver) SensitiveKeys() []string { } // resolveAppNamesMap builds a name→UUID map for all non-UUID app names found -// in the raw trusted_sources list. A single paginated Apps.List call is made -// regardless of how many type=app rules exist, avoiding repeated full scans. -// Returns an error only if the Apps client is unavailable for a non-UUID name, -// or if the DO API call fails. +// in the raw trusted_sources list. A single paginated listing pass is made +// (potentially multiple page requests) regardless of how many type=app rules +// exist, avoiding repeated full scans. Pagination stops early once all +// requested names have been resolved. Returns an error only if the Apps client +// is unavailable for a non-UUID name, or if the DO API call fails. func (d *DatabaseDriver) resolveAppNamesMap(ctx context.Context, raw []any) (map[string]string, error) { // Collect the distinct non-UUID app names that need resolution. needed := map[string]struct{}{} @@ -271,6 +272,9 @@ func (d *DatabaseDriver) resolveAppNamesMap(ctx context.Context, raw []any) (map } if _, want := needed[app.Spec.Name]; want { resolved[app.Spec.Name] = app.ID + if len(resolved) == len(needed) { + return resolved, nil // all names resolved — no more pages needed + } } } if resp == nil || resp.Links == nil || resp.Links.IsLastPage() { From 83b5400b7772dbcd7b39e82337e95b76cc332c1d Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 25 Apr 2026 03:06:13 -0400 Subject: [PATCH 4/9] fix(database): error on wrong-type trusted_sources; clarify round-4 ghost-flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address Copilot round-4 findings on PR #25: Finding 1 (ghost-flag): early-exit `if len(resolved) == len(needed)` is confirmed present at line 275 in 91c3cf6. Copilot re-flagged based on a stale diff view; no code change needed. Finding 2 (real): buildUpdateFirewallRules did `list, _ := raw.([]any)`, silently treating a non-list trusted_sources value (e.g. a YAML scalar) as an empty slice — returning ok=true with zero rules and clearing all firewall rules. Same latent bug existed in buildCreateFirewallRules. Fix both: check key existence separately, then type-assert with the ok form and return an actionable error (mentioning "trusted_sources" and the actual Go type) when the value is not a []any. Add two tests: - TestDatabaseDriver_Create_TrustedSources_WrongType_ReturnsError - TestDatabaseDriver_Update_TrustedSources_WrongType_ReturnsError Co-Authored-By: Claude Sonnet 4.6 --- internal/drivers/database.go | 16 +++++++++-- internal/drivers/database_test.go | 47 +++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/internal/drivers/database.go b/internal/drivers/database.go index 9495753..a90fb74 100644 --- a/internal/drivers/database.go +++ b/internal/drivers/database.go @@ -295,8 +295,15 @@ func (d *DatabaseDriver) resolveAppNamesMap(ctx context.Context, raw []any) (map // []*godo.DatabaseCreateFirewallRule, resolving type=app values to UUIDs via // a single Apps.List pass (see resolveAppNamesMap). func (d *DatabaseDriver) buildCreateFirewallRules(ctx context.Context, cfg map[string]any) ([]*godo.DatabaseCreateFirewallRule, error) { - raw, ok := cfg["trusted_sources"].([]any) - if !ok || len(raw) == 0 { + rawVal, exists := cfg["trusted_sources"] + if !exists { + return nil, nil + } + raw, ok := rawVal.([]any) + if !ok { + return nil, fmt.Errorf("trusted_sources: expected a list of rule objects, got %T; check your config syntax (use a YAML list, not a scalar)", rawVal) + } + if len(raw) == 0 { return nil, nil } appUUIDs, err := d.resolveAppNamesMap(ctx, raw) @@ -338,7 +345,10 @@ func (d *DatabaseDriver) buildUpdateFirewallRules(ctx context.Context, cfg map[s if !ok { return nil, false, nil // key absent: leave existing rules unchanged } - list, _ := raw.([]any) + list, ok := raw.([]any) + if !ok { + return nil, false, fmt.Errorf("trusted_sources: expected a list of rule objects, got %T; check your config syntax (use a YAML list, not a scalar)", raw) + } appUUIDs, err := d.resolveAppNamesMap(ctx, list) if err != nil { return nil, false, err diff --git a/internal/drivers/database_test.go b/internal/drivers/database_test.go index 8f8e94e..da43b2e 100644 --- a/internal/drivers/database_test.go +++ b/internal/drivers/database_test.go @@ -594,3 +594,50 @@ func TestDatabaseDriver_Create_AppType_NoAppsClient_ReturnsError(t *testing.T) { t.Errorf("expected error to mention the offending value %q; got: %v", "bmw-staging", err) } } + +func TestDatabaseDriver_Create_TrustedSources_WrongType_ReturnsError(t *testing.T) { + // trusted_sources present but not a list (e.g. user wrote a scalar in YAML) + // must return an error rather than silently dropping the rule. + dbMock := &mockDatabaseClient{db: testDatabase()} + d := drivers.NewDatabaseDriverWithClient(dbMock, "nyc3") + + _, err := d.Create(context.Background(), interfaces.ResourceSpec{ + Name: "my-db", + Config: map[string]any{ + "engine": "pg", + "trusted_sources": "bmw-staging", // scalar, not a list + }, + }) + if err == nil { + t.Fatal("expected error when trusted_sources is not a list, got nil") + } + if !strings.Contains(err.Error(), "trusted_sources") { + t.Errorf("expected error to mention 'trusted_sources'; got: %v", err) + } +} + +func TestDatabaseDriver_Update_TrustedSources_WrongType_ReturnsError(t *testing.T) { + // trusted_sources present but not a list on Update path must also error, + // not silently clear all firewall rules. + dbMock := &mockDatabaseClient{db: testDatabase()} + d := drivers.NewDatabaseDriverWithClient(dbMock, "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-2vcpu-4gb", + "trusted_sources": "bmw-staging", // scalar, not a list + }, + }) + if err == nil { + t.Fatal("expected error when trusted_sources is not a list, got nil") + } + if !strings.Contains(err.Error(), "trusted_sources") { + t.Errorf("expected error to mention 'trusted_sources'; got: %v", err) + } + if dbMock.lastFirewallReq != nil { + t.Error("UpdateFirewallRules must NOT be called when trusted_sources has wrong type") + } +} From e728fd96144e2363d17c45b848161c4e76399213 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 25 Apr 2026 03:16:16 -0400 Subject: [PATCH 5/9] fix(database): docstring accuracy, deterministic nil-client error, pre-validate firewall MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address Copilot round-5 findings on PR #25: 1. resolveAppNamesMap docstring now lists all 3 error cases: nil Apps client, DO API failure, and ErrResourceNotFound for a missing name. 2. Nil-client error message was non-deterministic when multiple type=app names needed resolution (map iteration order). Sort names before picking the first to report so the message is stable across runs. 3. Update() called buildUpdateFirewallRules AFTER Resize, risking a partial-apply where the DB is resized but firewall rules are left stale on config/resolution errors. Move buildUpdateFirewallRules before Resize so all validation and name→UUID resolution happens upfront, before any DO API mutations. Co-Authored-By: Claude Sonnet 4.6 --- internal/drivers/database.go | 37 ++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/internal/drivers/database.go b/internal/drivers/database.go index a90fb74..7c22202 100644 --- a/internal/drivers/database.go +++ b/internal/drivers/database.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "log" + "sort" "github.com/GoCodeAlone/workflow/interfaces" "github.com/digitalocean/godo" @@ -137,6 +138,14 @@ func (d *DatabaseDriver) Update(ctx context.Context, ref interfaces.ResourceRef, if err != nil { return nil, err } + // Resolve firewall rules BEFORE mutating the database. buildUpdateFirewallRules + // can fail for config errors (wrong type) or app name→UUID resolution failures; + // validating upfront prevents a partial-apply where Resize succeeds but the + // subsequent firewall update fails, leaving the DB resized with stale rules. + fwRules, fwPresent, fwErr := d.buildUpdateFirewallRules(ctx, spec.Config) + if fwErr != nil { + return nil, fmt.Errorf("database update firewall %q: %w", ref.Name, fwErr) + } size := strFromConfig(spec.Config, "size", "db-s-1vcpu-1gb") numNodes, _ := intFromConfig(spec.Config, "num_nodes", 1) _, err = d.client.Resize(ctx, providerID, &godo.DatabaseResizeRequest{ @@ -148,11 +157,9 @@ func (d *DatabaseDriver) Update(ctx context.Context, ref interfaces.ResourceRef, } // Sync firewall rules when trusted_sources key is present in config. // "present but empty" clears all rules; absent key leaves existing rules unchanged. - if rules, ok, fwErr := d.buildUpdateFirewallRules(ctx, spec.Config); fwErr != nil { - return nil, fmt.Errorf("database update firewall %q: %w", ref.Name, fwErr) - } else if ok { + if fwPresent { _, err = d.client.UpdateFirewallRules(ctx, providerID, &godo.DatabaseUpdateFirewallRulesRequest{ - Rules: rules, + Rules: fwRules, }) if err != nil { return nil, fmt.Errorf("database update firewall %q: %w", ref.Name, WrapGodoError(err)) @@ -228,8 +235,10 @@ func (d *DatabaseDriver) SensitiveKeys() []string { // in the raw trusted_sources list. A single paginated listing pass is made // (potentially multiple page requests) regardless of how many type=app rules // exist, avoiding repeated full scans. Pagination stops early once all -// requested names have been resolved. Returns an error only if the Apps client -// is unavailable for a non-UUID name, or if the DO API call fails. +// requested names have been resolved. Returns an error if: +// - the Apps client is nil and a non-UUID name requires resolution, +// - the DO Apps.List API call fails, or +// - a requested app name is not found (wraps ErrResourceNotFound). func (d *DatabaseDriver) resolveAppNamesMap(ctx context.Context, raw []any) (map[string]string, error) { // Collect the distinct non-UUID app names that need resolution. needed := map[string]struct{}{} @@ -251,13 +260,17 @@ func (d *DatabaseDriver) resolveAppNamesMap(ctx context.Context, raw []any) (map return nil, nil // nothing to look up } if d.appsClient == nil { - // Surface the first offending name for a clear error message. - for name := range needed { - return nil, fmt.Errorf( - "trusted_source type=app value %q is not a UUID and no Apps client is configured for name lookup; pass the app UUID directly", - name, - ) + // Sort names so the error message is deterministic regardless of map + // iteration order (important when multiple type=app rules are present). + names := make([]string, 0, len(needed)) + for n := range needed { + names = append(names, n) } + sort.Strings(names) + return nil, fmt.Errorf( + "trusted_source type=app value %q is not a UUID and no Apps client is configured for name lookup; pass the app UUID directly", + names[0], + ) } resolved := make(map[string]string, len(needed)) opts := &godo.ListOptions{Page: 1, PerPage: 200} From 319110cc00199b5f641856e4c9772f9ba96f10aa Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 25 Apr 2026 03:28:40 -0400 Subject: [PATCH 6/9] =?UTF-8?q?fix(database):=20accurate=20partial-apply?= =?UTF-8?q?=20comment=20in=20Update=20=E2=80=94=20reduces=20not=20prevents?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Sonnet 4.6 --- internal/drivers/database.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/drivers/database.go b/internal/drivers/database.go index 7c22202..5455afb 100644 --- a/internal/drivers/database.go +++ b/internal/drivers/database.go @@ -139,9 +139,10 @@ func (d *DatabaseDriver) Update(ctx context.Context, ref interfaces.ResourceRef, return nil, err } // Resolve firewall rules BEFORE mutating the database. buildUpdateFirewallRules - // can fail for config errors (wrong type) or app name→UUID resolution failures; - // validating upfront prevents a partial-apply where Resize succeeds but the - // subsequent firewall update fails, leaving the DB resized with stale rules. + // can fail for config errors (wrong type) or app name→UUID resolution failures, + // so validating upfront avoids partial applies caused by those preflight issues. + // A later UpdateFirewallRules call can still fail after Resize due to API/service + // errors, so this reduces—but does not eliminate—the chance of partial applies. fwRules, fwPresent, fwErr := d.buildUpdateFirewallRules(ctx, spec.Config) if fwErr != nil { return nil, fmt.Errorf("database update firewall %q: %w", ref.Name, fwErr) From 3be59502feecd93041bb54619acb4614ed3dc009 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 25 Apr 2026 03:30:16 -0400 Subject: [PATCH 7/9] fix(database): report all unresolved app names + fix trusted_sources key in error message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Include all non-UUID app names in the nil-client error (not just names[0]) - Fix "trusted_source" → "trusted_sources" to match the config key - Update test assertion to match new "not UUIDs" wording Co-Authored-By: Claude Sonnet 4.6 --- internal/drivers/database.go | 9 +++++++-- internal/drivers/database_test.go | 4 ++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/internal/drivers/database.go b/internal/drivers/database.go index 5455afb..a8d43ff 100644 --- a/internal/drivers/database.go +++ b/internal/drivers/database.go @@ -5,6 +5,7 @@ import ( "fmt" "log" "sort" + "strings" "github.com/GoCodeAlone/workflow/interfaces" "github.com/digitalocean/godo" @@ -268,9 +269,13 @@ func (d *DatabaseDriver) resolveAppNamesMap(ctx context.Context, raw []any) (map names = append(names, n) } sort.Strings(names) + quoted := make([]string, len(names)) + for i, n := range names { + quoted[i] = fmt.Sprintf("%q", n) + } return nil, fmt.Errorf( - "trusted_source type=app value %q is not a UUID and no Apps client is configured for name lookup; pass the app UUID directly", - names[0], + "trusted_sources: type=app value(s) %s are not UUIDs and no Apps client is configured for name lookup; pass app UUIDs directly", + strings.Join(quoted, ", "), ) } resolved := make(map[string]string, len(needed)) diff --git a/internal/drivers/database_test.go b/internal/drivers/database_test.go index da43b2e..39b1d65 100644 --- a/internal/drivers/database_test.go +++ b/internal/drivers/database_test.go @@ -587,8 +587,8 @@ func TestDatabaseDriver_Create_AppType_NoAppsClient_ReturnsError(t *testing.T) { if err == nil { t.Fatal("expected error when apps client is nil and value is not UUID, got nil") } - if !strings.Contains(err.Error(), "not a UUID") { - t.Errorf("expected error to mention 'not a UUID'; got: %v", err) + if !strings.Contains(err.Error(), "not UUIDs") { + t.Errorf("expected error to mention 'not UUIDs'; got: %v", err) } if !strings.Contains(err.Error(), "bmw-staging") { t.Errorf("expected error to mention the offending value %q; got: %v", "bmw-staging", err) From bf1361e7bbc14abea1195379f322b293ec6065d3 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 25 Apr 2026 03:41:49 -0400 Subject: [PATCH 8/9] fix(database): narrow appsClient interface + deterministic missing-app error - Define appNameLister (List only) instead of coupling to full AppPlatformClient - Collect all unresolved names, sort them, report them all in one error rather than returning the first found via non-deterministic map iteration Co-Authored-By: Claude Sonnet 4.6 --- internal/drivers/database.go | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/internal/drivers/database.go b/internal/drivers/database.go index a8d43ff..07868b9 100644 --- a/internal/drivers/database.go +++ b/internal/drivers/database.go @@ -21,11 +21,18 @@ type DatabaseClient interface { UpdateFirewallRules(ctx context.Context, databaseID string, req *godo.DatabaseUpdateFirewallRulesRequest) (*godo.Response, error) } +// appNameLister is a minimal interface for looking up App Platform app names. +// DatabaseDriver only needs List; using the full AppPlatformClient here would +// unnecessarily couple the database driver to app lifecycle operations. +type appNameLister interface { + List(ctx context.Context, opts *godo.ListOptions) ([]*godo.App, *godo.Response, error) +} + // DatabaseDriver manages DigitalOcean Managed Databases (infra.database). // Supports pg, mysql, redis, mongodb, kafka. type DatabaseDriver struct { client DatabaseClient - appsClient AppPlatformClient // optional; used to resolve type=app trusted_source names to UUIDs + appsClient appNameLister // optional; used to resolve type=app trusted_source names to UUIDs region string } @@ -44,7 +51,9 @@ func NewDatabaseDriverWithClient(c DatabaseClient, region string) *DatabaseDrive // NewDatabaseDriverWithClients creates a driver with injected database and apps // clients (for tests that exercise type=app trusted_source name → UUID resolution). -func NewDatabaseDriverWithClients(c DatabaseClient, apps AppPlatformClient, region string) *DatabaseDriver { +// apps must implement appNameLister (i.e. have a List method); any *fakeAppsClient +// or real godo.AppsService satisfies this. +func NewDatabaseDriverWithClients(c DatabaseClient, apps appNameLister, region string) *DatabaseDriver { return &DatabaseDriver{client: c, appsClient: apps, region: region} } @@ -301,13 +310,24 @@ func (d *DatabaseDriver) resolveAppNamesMap(ctx context.Context, raw []any) (map } opts.Page++ } - // Verify every requested name was found. + // Collect every name that was not found and report them all at once. + // Sort for deterministic output regardless of map iteration order. + missing := make([]string, 0, len(needed)) for name := range needed { if _, ok := resolved[name]; !ok { - return nil, fmt.Errorf("trusted_source app %q: %w", name, ErrResourceNotFound) + missing = append(missing, name) } } - return resolved, nil + if len(missing) == 0 { + return resolved, nil + } + sort.Strings(missing) + quotedMissing := make([]string, len(missing)) + for i, name := range missing { + quotedMissing[i] = fmt.Sprintf("%q", name) + } + return nil, fmt.Errorf("trusted_sources app(s) %s: %w", + strings.Join(quotedMissing, ", "), ErrResourceNotFound) } // buildCreateFirewallRules converts "trusted_sources" config to From b678e6e01390e33104d64c39754ef53d00480858 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Sat, 25 Apr 2026 03:47:29 -0400 Subject: [PATCH 9/9] fix(database): buildUpdateFirewallRules returns true on error when key is present When trusted_sources is present but invalid (wrong type or unresolvable app name), return (nil, true, error) so callers can distinguish "key absent" (false) from "key present but invalid" (true+error). Update docstring to match the bool semantics precisely. Co-Authored-By: Claude Sonnet 4.6 --- internal/drivers/database.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/internal/drivers/database.go b/internal/drivers/database.go index 07868b9..9b62437 100644 --- a/internal/drivers/database.go +++ b/internal/drivers/database.go @@ -375,10 +375,14 @@ func (d *DatabaseDriver) buildCreateFirewallRules(ctx context.Context, cfg map[s } // buildUpdateFirewallRules converts "trusted_sources" config to -// ([]*godo.DatabaseFirewallRule, bool, error). The bool mirrors -// trustedSourceFirewallRulesFromConfig: false means key absent (leave unchanged); -// true means key present (apply the returned slice, even if empty). type=app -// values are resolved to UUIDs via a single Apps.List pass (see resolveAppNamesMap). +// ([]*godo.DatabaseFirewallRule, bool, error). The bool reflects whether the +// "trusted_sources" key was present in the config: +// - false: key absent — caller should leave existing firewall rules unchanged. +// - true: key present — caller should apply the returned rules (or surface the error). +// +// Returning true on error paths lets callers distinguish "key absent" from +// "key present but invalid/unresolvable" without inspecting the error type. +// type=app values are resolved to UUIDs via a single Apps.List pass (see resolveAppNamesMap). func (d *DatabaseDriver) buildUpdateFirewallRules(ctx context.Context, cfg map[string]any) ([]*godo.DatabaseFirewallRule, bool, error) { raw, ok := cfg["trusted_sources"] if !ok { @@ -386,11 +390,13 @@ func (d *DatabaseDriver) buildUpdateFirewallRules(ctx context.Context, cfg map[s } list, ok := raw.([]any) if !ok { - return nil, false, fmt.Errorf("trusted_sources: expected a list of rule objects, got %T; check your config syntax (use a YAML list, not a scalar)", raw) + // Key is present but wrong type — return true so callers don't treat this as "absent". + return nil, true, fmt.Errorf("trusted_sources: expected a list of rule objects, got %T; check your config syntax (use a YAML list, not a scalar)", raw) } appUUIDs, err := d.resolveAppNamesMap(ctx, list) if err != nil { - return nil, false, err + // Key is present but resolution failed — return true (key present) + error. + return nil, true, err } rules := make([]*godo.DatabaseFirewallRule, 0, len(list)) for _, v := range list {