From fedcfeb8ecdd5085fd8f280e8a93c20fff5ed542 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 22 Apr 2026 21:35:47 -0400 Subject: [PATCH 1/3] =?UTF-8?q?feat(do):=20v0.7.0=20=E2=80=94=20sidecars?= =?UTF-8?q?=20+=20DatabaseDriver.trusted=5Fsources=20(Tasks=2037-39)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task 37 — Sidecars as sibling AppServiceSpec: - Remove interfaces.KeySidecars from doUnsupportedCanonicalKeys; sidecars now advertised in SupportedCanonicalKeys(). - sidecarsFromConfig() maps each canonical sidecar to a godo.AppServiceSpec: image (ParseImageRef), run_command, env_vars/env_vars_secret, and first public port → HTTPPort. - buildAppSpec appends sidecar specs to spec.Services after the primary svc. - Tests: TestBuildAppSpec_Sidecars (2 sidecars, secret env, port mapping), TestBuildAppSpec_Sidecars_Empty (no sidecars → only main service). - provider_test.go: "sidecars" added to supported key list; negative assertion removed. Task 38 — appHealthResult: already fully implemented in v0.6.x (no-op). Task 39 — DatabaseDriver.trusted_sources: - DatabaseClient interface gains UpdateFirewallRules(). - trustedSourceCreateRulesFromConfig() → []*DatabaseCreateFirewallRule for Create. - trustedSourceFirewallRulesFromConfig() → []*DatabaseFirewallRule for Update. - Create wires Rules field; Update calls UpdateFirewallRules when present. - Tests: Create_WithTrustedSources, Update_WithTrustedSources, Update_NoTrustedSources_SkipsFirewall. - mockDatabaseClient extended with lastCreateReq/lastFirewallReq captures. Co-Authored-By: Claude Sonnet 4.6 --- internal/drivers/app_platform_buildspec.go | 56 ++++++++++- .../drivers/app_platform_buildspec_test.go | 75 +++++++++++++++ internal/drivers/database.go | 72 +++++++++++++- internal/drivers/database_test.go | 95 ++++++++++++++++++- internal/provider.go | 6 +- internal/provider_test.go | 9 +- 6 files changed, 294 insertions(+), 19 deletions(-) diff --git a/internal/drivers/app_platform_buildspec.go b/internal/drivers/app_platform_buildspec.go index f0314b1..f979480 100644 --- a/internal/drivers/app_platform_buildspec.go +++ b/internal/drivers/app_platform_buildspec.go @@ -72,10 +72,14 @@ func buildAppSpec(name string, cfg map[string]any, region string) (*godo.AppSpec } } + // Sidecars run as sibling AppServiceSpec entries (DO App Platform does not have + // a native sidecar concept; each sidecar becomes an independent service). + services := append([]*godo.AppServiceSpec{svc}, sidecarsFromConfig(cfg)...) + spec := &godo.AppSpec{ Name: name, Region: region, - Services: []*godo.AppServiceSpec{svc}, + Services: services, Jobs: jobsFromConfig(cfg), Workers: workersFromConfig(cfg), StaticSites: staticSitesFromConfig(cfg), @@ -721,6 +725,56 @@ func stringsFromConfig(cfg map[string]any, key string) []string { return out } +// sidecarsFromConfig converts canonical "sidecars" list to sibling []*godo.AppServiceSpec. +// DO App Platform has no native sidecar model; each sidecar is mapped as an independent +// service in the same app, sharing the app's network namespace via the platform routing. +func sidecarsFromConfig(cfg map[string]any) []*godo.AppServiceSpec { + raw, ok := cfg["sidecars"].([]any) + if !ok || len(raw) == 0 { + return nil + } + out := make([]*godo.AppServiceSpec, 0, len(raw)) + for _, v := range raw { + m, ok := v.(map[string]any) + if !ok { + continue + } + name := strFromConfig(m, "name", "") + if name == "" { + continue + } + svc := &godo.AppServiceSpec{ + Name: name, + RunCommand: strFromConfig(m, "run_command", ""), + Envs: envVarsFromConfig(m), + } + if imgStr := strFromConfig(m, "image", ""); imgStr != "" { + img, err := ParseImageRef(imgStr) + if err == nil { + svc.Image = img + } + } + // Map the first public port to HTTPPort if specified. + if ports, ok := m["ports"].([]any); ok { + for _, p := range ports { + pm, ok := p.(map[string]any) + if !ok { + continue + } + pub, _ := pm["public"].(bool) + if pub { + if port, ok2 := intFromConfig(pm, "port", 0); ok2 && port > 0 { + svc.HTTPPort = int64(port) + break + } + } + } + } + out = append(out, svc) + } + return out +} + // stringMatchesFromConfig converts a []any of strings to []*godo.AppStringMatch using exact matching. func stringMatchesFromConfig(cfg map[string]any, key string) []*godo.AppStringMatch { raw, ok := cfg[key].([]any) diff --git a/internal/drivers/app_platform_buildspec_test.go b/internal/drivers/app_platform_buildspec_test.go index be0f57d..1c8ceca 100644 --- a/internal/drivers/app_platform_buildspec_test.go +++ b/internal/drivers/app_platform_buildspec_test.go @@ -946,6 +946,81 @@ func TestBuildAppSpec_LogDestinations(t *testing.T) { } } +// ── Sidecars ───────────────────────────────────────────────────────────────── + +func TestBuildAppSpec_Sidecars(t *testing.T) { + cfg := map[string]any{ + "image": "registry.digitalocean.com/myrepo/myapp:v1", + "sidecars": []any{ + map[string]any{ + "name": "tailscale", + "image": "docker.io/tailscale/tailscale:latest", + "run_command": "/usr/local/bin/containerboot", + "env_vars_secret": map[string]any{ + "TS_AUTH_KEY": "ts-secret", + }, + }, + map[string]any{ + "name": "envoy-proxy", + "image": "docker.io/envoyproxy/envoy:v1.29", + "run_command": "/usr/local/bin/envoy -c /config/envoy.yaml", + "ports": []any{map[string]any{"port": 9901, "public": true}}, + }, + }, + } + spec := buildSpecViaCreate(t, cfg) + // Main service + 2 sidecars = 3 services total. + if len(spec.Services) != 3 { + t.Fatalf("expected 3 services (main + 2 sidecars), got %d", len(spec.Services)) + } + if spec.Services[0].Name != "test-app" { + t.Errorf("services[0].Name = %q, want test-app", spec.Services[0].Name) + } + ts := spec.Services[1] + if ts.Name != "tailscale" { + t.Errorf("sidecar[0].Name = %q, want tailscale", ts.Name) + } + if ts.RunCommand != "/usr/local/bin/containerboot" { + t.Errorf("sidecar[0].RunCommand = %q", ts.RunCommand) + } + if ts.Image == nil || ts.Image.Repository != "tailscale" { + t.Errorf("sidecar[0].Image.Repository = %q, want tailscale", func() string { + if ts.Image == nil { + return "" + } + return ts.Image.Repository + }()) + } + // Secret env var forwarded. + foundSecret := false + for _, e := range ts.Envs { + if e.Key == "TS_AUTH_KEY" && e.Type == godo.AppVariableType_Secret { + foundSecret = true + } + } + if !foundSecret { + t.Error("expected TS_AUTH_KEY secret env var in tailscale sidecar") + } + + // Envoy sidecar: public port mapped to HTTPPort. + envoy := spec.Services[2] + if envoy.Name != "envoy-proxy" { + t.Errorf("sidecar[1].Name = %q, want envoy-proxy", envoy.Name) + } + if envoy.HTTPPort != 9901 { + t.Errorf("sidecar[1].HTTPPort = %d, want 9901", envoy.HTTPPort) + } +} + +func TestBuildAppSpec_Sidecars_Empty(t *testing.T) { + cfg := map[string]any{"image": "registry.digitalocean.com/myrepo/myapp:v1"} + spec := buildSpecViaCreate(t, cfg) + // Only the main service, no sidecars. + if len(spec.Services) != 1 { + t.Errorf("expected 1 service (no sidecars), got %d", len(spec.Services)) + } +} + // ── Update propagates buildAppSpec ────────────────────────────────────────── func TestBuildAppSpec_UpdateUsesBuildSpec(t *testing.T) { diff --git a/internal/drivers/database.go b/internal/drivers/database.go index dd0b3f9..e09086d 100644 --- a/internal/drivers/database.go +++ b/internal/drivers/database.go @@ -14,6 +14,7 @@ type DatabaseClient interface { Get(ctx context.Context, databaseID string) (*godo.Database, *godo.Response, error) Resize(ctx context.Context, databaseID string, req *godo.DatabaseResizeRequest) (*godo.Response, error) Delete(ctx context.Context, databaseID string) (*godo.Response, error) + UpdateFirewallRules(ctx context.Context, databaseID string, req *godo.DatabaseUpdateFirewallRulesRequest) (*godo.Response, error) } // DatabaseDriver manages DigitalOcean Managed Databases (infra.database). @@ -47,6 +48,7 @@ func (d *DatabaseDriver) Create(ctx context.Context, spec interfaces.ResourceSpe SizeSlug: size, Region: region, NumNodes: numNodes, + Rules: trustedSourceCreateRulesFromConfig(spec.Config), } db, _, err := d.client.Create(ctx, req) @@ -74,6 +76,15 @@ func (d *DatabaseDriver) Update(ctx context.Context, ref interfaces.ResourceRef, if err != nil { return nil, fmt.Errorf("database update %q: %w", ref.Name, WrapGodoError(err)) } + // Sync firewall rules when trusted_sources is specified. + if rules := trustedSourceFirewallRulesFromConfig(spec.Config); rules != nil { + _, err = d.client.UpdateFirewallRules(ctx, ref.ProviderID, &godo.DatabaseUpdateFirewallRulesRequest{ + Rules: rules, + }) + if err != nil { + return nil, fmt.Errorf("database update firewall %q: %w", ref.Name, WrapGodoError(err)) + } + } return d.Read(ctx, ref) } @@ -126,12 +137,65 @@ func (d *DatabaseDriver) SensitiveKeys() []string { return []string{"uri", "password", "user"} } +// trustedSourceCreateRulesFromConfig converts canonical "trusted_sources" to +// []*godo.DatabaseCreateFirewallRule for use in a DatabaseCreateRequest. +// Each entry must have "type" (ip_addr|k8s|app|droplet_id|tag) and "value". +func trustedSourceCreateRulesFromConfig(cfg map[string]any) []*godo.DatabaseCreateFirewallRule { + raw, ok := cfg["trusted_sources"].([]any) + if !ok || len(raw) == 0 { + return 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 + } + rules = append(rules, &godo.DatabaseCreateFirewallRule{ + Type: ruleType, + Value: ruleValue, + }) + } + return rules +} + +// trustedSourceFirewallRulesFromConfig converts canonical "trusted_sources" to +// []*godo.DatabaseFirewallRule for use in a DatabaseUpdateFirewallRulesRequest. +func trustedSourceFirewallRulesFromConfig(cfg map[string]any) []*godo.DatabaseFirewallRule { + raw, ok := cfg["trusted_sources"].([]any) + if !ok || len(raw) == 0 { + return nil + } + rules := make([]*godo.DatabaseFirewallRule, 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 + } + rules = append(rules, &godo.DatabaseFirewallRule{ + Type: ruleType, + Value: ruleValue, + }) + } + return rules +} + func dbOutput(db *godo.Database) *interfaces.ResourceOutput { outputs := map[string]any{ - "engine": db.EngineSlug, - "region": db.RegionSlug, - "size": db.SizeSlug, - "version": db.VersionSlug, + "engine": db.EngineSlug, + "region": db.RegionSlug, + "size": db.SizeSlug, + "version": db.VersionSlug, } if db.Connection != nil { outputs["host"] = db.Connection.Host diff --git a/internal/drivers/database_test.go b/internal/drivers/database_test.go index 08803a3..f7e49a7 100644 --- a/internal/drivers/database_test.go +++ b/internal/drivers/database_test.go @@ -11,11 +11,15 @@ import ( ) type mockDatabaseClient struct { - db *godo.Database - err error + db *godo.Database + err error + firewallErr error + lastCreateReq *godo.DatabaseCreateRequest + lastFirewallReq *godo.DatabaseUpdateFirewallRulesRequest } -func (m *mockDatabaseClient) Create(_ context.Context, _ *godo.DatabaseCreateRequest) (*godo.Database, *godo.Response, error) { +func (m *mockDatabaseClient) Create(_ context.Context, req *godo.DatabaseCreateRequest) (*godo.Database, *godo.Response, error) { + m.lastCreateReq = req return m.db, nil, m.err } func (m *mockDatabaseClient) Get(_ context.Context, _ string) (*godo.Database, *godo.Response, error) { @@ -27,6 +31,10 @@ func (m *mockDatabaseClient) Resize(_ context.Context, _ string, _ *godo.Databas func (m *mockDatabaseClient) Delete(_ context.Context, _ string) (*godo.Response, error) { return nil, m.err } +func (m *mockDatabaseClient) UpdateFirewallRules(_ context.Context, _ string, req *godo.DatabaseUpdateFirewallRulesRequest) (*godo.Response, error) { + m.lastFirewallReq = req + return nil, m.firewallErr +} func testDatabase() *godo.Database { return &godo.Database{ @@ -159,6 +167,87 @@ func TestDatabaseDriver_Delete_Error(t *testing.T) { } } +func TestDatabaseDriver_Create_WithTrustedSources(t *testing.T) { + mock := &mockDatabaseClient{db: testDatabase()} + d := drivers.NewDatabaseDriverWithClient(mock, "nyc3") + + _, err := d.Create(context.Background(), interfaces.ResourceSpec{ + Name: "my-db", + Config: map[string]any{ + "engine": "pg", + "trusted_sources": []any{ + map[string]any{"type": "ip_addr", "value": "10.0.0.1/32"}, + map[string]any{"type": "k8s", "value": "k8s-cluster-uuid"}, + }, + }, + }) + if err != nil { + t.Fatalf("Create: %v", err) + } + if mock.lastCreateReq == nil { + t.Fatal("no create request captured") + } + if len(mock.lastCreateReq.Rules) != 2 { + t.Fatalf("expected 2 firewall rules, got %d", len(mock.lastCreateReq.Rules)) + } + if mock.lastCreateReq.Rules[0].Type != "ip_addr" || mock.lastCreateReq.Rules[0].Value != "10.0.0.1/32" { + t.Errorf("rule[0] = {%s %s}, want {ip_addr 10.0.0.1/32}", + mock.lastCreateReq.Rules[0].Type, mock.lastCreateReq.Rules[0].Value) + } + if mock.lastCreateReq.Rules[1].Type != "k8s" || mock.lastCreateReq.Rules[1].Value != "k8s-cluster-uuid" { + t.Errorf("rule[1] = {%s %s}, want {k8s k8s-cluster-uuid}", + mock.lastCreateReq.Rules[1].Type, mock.lastCreateReq.Rules[1].Value) + } +} + +func TestDatabaseDriver_Update_WithTrustedSources(t *testing.T) { + mock := &mockDatabaseClient{db: testDatabase()} + d := drivers.NewDatabaseDriverWithClient(mock, "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": "ip_addr", "value": "192.168.1.0/24"}, + }, + }, + }) + if err != nil { + t.Fatalf("Update: %v", err) + } + if mock.lastFirewallReq == nil { + t.Fatal("UpdateFirewallRules was not called") + } + if len(mock.lastFirewallReq.Rules) != 1 { + t.Fatalf("expected 1 firewall rule, got %d", len(mock.lastFirewallReq.Rules)) + } + if mock.lastFirewallReq.Rules[0].Type != "ip_addr" || mock.lastFirewallReq.Rules[0].Value != "192.168.1.0/24" { + t.Errorf("rule[0] = {%s %s}, want {ip_addr 192.168.1.0/24}", + mock.lastFirewallReq.Rules[0].Type, mock.lastFirewallReq.Rules[0].Value) + } +} + +func TestDatabaseDriver_Update_NoTrustedSources_SkipsFirewall(t *testing.T) { + mock := &mockDatabaseClient{db: testDatabase()} + d := drivers.NewDatabaseDriverWithClient(mock, "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"}, + }) + if err != nil { + t.Fatalf("Update: %v", err) + } + if mock.lastFirewallReq != nil { + t.Error("UpdateFirewallRules should not be called when trusted_sources is absent") + } +} + func TestDatabaseDriver_Diff_HasChanges(t *testing.T) { mock := &mockDatabaseClient{} d := drivers.NewDatabaseDriverWithClient(mock, "nyc3") diff --git a/internal/provider.go b/internal/provider.go index 08bf03d..e7f5943 100644 --- a/internal/provider.go +++ b/internal/provider.go @@ -110,10 +110,8 @@ func (p *DOProvider) ResourceDriver(resourceType string) (interfaces.ResourceDri // doUnsupportedCanonicalKeys lists canonical keys that the DO plugin does not yet map. // Each entry is removed from SupportedCanonicalKeys() so wfctl validate can warn callers. -// "sidecars" is implemented in v0.7.0 Task 37 (feat/v0.7.0-sidecars). -var doUnsupportedCanonicalKeys = map[string]struct{}{ - interfaces.KeySidecars: {}, -} +// "sidecars" is now mapped in v0.7.0 Task 37 as sibling AppServiceSpec entries. +var doUnsupportedCanonicalKeys = map[string]struct{}{} // SupportedCanonicalKeys returns the canonical IaC config keys that this DO provider // currently maps. Keys in doUnsupportedCanonicalKeys are excluded until their Task diff --git a/internal/provider_test.go b/internal/provider_test.go index 08d41f0..38491b4 100644 --- a/internal/provider_test.go +++ b/internal/provider_test.go @@ -106,25 +106,20 @@ func TestDOProvider_SupportedCanonicalKeys(t *testing.T) { keySet[k] = true } - // Keys the DO provider actively maps in this release. + // Keys the DO provider actively maps in this release (v0.7.0). supported := []string{ "name", "region", "image", "http_port", "instance_count", "size", "env_vars", "env_vars_secret", "autoscaling", "routes", "health_check", "liveness_check", "cors", "internal_ports", "build_command", "run_command", "dockerfile_path", "source_dir", "termination", "domains", "alerts", "log_destinations", "ingress", "egress", "maintenance", "vpc_ref", - "jobs", "workers", "static_sites", "provider_specific", + "jobs", "workers", "static_sites", "sidecars", "provider_specific", } for _, k := range supported { if !keySet[k] { t.Errorf("SupportedCanonicalKeys missing expected key %q", k) } } - - // "sidecars" is not yet mapped (Task 37); must NOT appear until then. - if keySet["sidecars"] { - t.Error("SupportedCanonicalKeys must not include \"sidecars\" until Task 37 lands") - } } func TestConfigHash_Deterministic(t *testing.T) { From 299f4c46c8649f3043fe41d382abbbebeabe50b9 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 22 Apr 2026 21:37:25 -0400 Subject: [PATCH 2/3] =?UTF-8?q?fix:=20correct=20trusted=5Fsources=20type?= =?UTF-8?q?=20list=20in=20doc=20comment=20(droplet=5Fid=20=E2=86=92=20drop?= =?UTF-8?q?let)?= 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 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/drivers/database.go b/internal/drivers/database.go index e09086d..7713431 100644 --- a/internal/drivers/database.go +++ b/internal/drivers/database.go @@ -139,7 +139,7 @@ func (d *DatabaseDriver) SensitiveKeys() []string { // trustedSourceCreateRulesFromConfig converts canonical "trusted_sources" to // []*godo.DatabaseCreateFirewallRule for use in a DatabaseCreateRequest. -// Each entry must have "type" (ip_addr|k8s|app|droplet_id|tag) and "value". +// Each entry must have "type" (ip_addr|k8s|app|droplet|tag) and "value". func trustedSourceCreateRulesFromConfig(cfg map[string]any) []*godo.DatabaseCreateFirewallRule { raw, ok := cfg["trusted_sources"].([]any) if !ok || len(raw) == 0 { From 56d2fc1ab609b1b1c4d64188143f27af61a64f23 Mon Sep 17 00:00:00 2001 From: Jon Langevin Date: Wed, 22 Apr 2026 21:46:26 -0400 Subject: [PATCH 3/3] fix: address 5 Copilot comments on feat/v0.7.0-sidecars-trusted-sources 1. sidecarsFromConfig returns ([]*godo.AppServiceSpec, error); invalid image ref now propagates as an error instead of being silently dropped. buildAppSpec plumbs the error up so misconfigured sidecar images fail fast. 2+3. trustedSourceFirewallRulesFromConfig returns ([]*DatabaseFirewallRule, bool) where bool=false means "key absent" (leave existing rules unchanged) and bool=true means "key present" (sync rules, including clearing when empty). Update() now uses the bool to distinguish the two cases. 4. Add TestDatabaseDriver_Update_EmptyTrustedSources_ClearsFirewall: verifies that trusted_sources present but empty causes UpdateFirewallRules to be called with an empty rule set (clearing all existing rules). 5. Fix misleading comment in sidecarsFromConfig: replace "sharing the app's network namespace via the platform routing" with accurate description: "Components communicate via the app's internal networking (platform-managed DNS/routing), not via a shared Linux network namespace." Co-Authored-By: Claude Sonnet 4.6 --- internal/drivers/app_platform_buildspec.go | 25 ++++++++++++------- internal/drivers/database.go | 28 +++++++++++++--------- internal/drivers/database_test.go | 25 +++++++++++++++++++ 3 files changed, 58 insertions(+), 20 deletions(-) diff --git a/internal/drivers/app_platform_buildspec.go b/internal/drivers/app_platform_buildspec.go index f979480..4bbc497 100644 --- a/internal/drivers/app_platform_buildspec.go +++ b/internal/drivers/app_platform_buildspec.go @@ -73,8 +73,12 @@ func buildAppSpec(name string, cfg map[string]any, region string) (*godo.AppSpec } // Sidecars run as sibling AppServiceSpec entries (DO App Platform does not have - // a native sidecar concept; each sidecar becomes an independent service). - services := append([]*godo.AppServiceSpec{svc}, sidecarsFromConfig(cfg)...) + // a native sidecar concept; each sidecar becomes an independent service component). + sidecars, err := sidecarsFromConfig(cfg) + if err != nil { + return nil, fmt.Errorf("app platform sidecars: %w", err) + } + services := append([]*godo.AppServiceSpec{svc}, sidecars...) spec := &godo.AppSpec{ Name: name, @@ -726,12 +730,14 @@ func stringsFromConfig(cfg map[string]any, key string) []string { } // sidecarsFromConfig converts canonical "sidecars" list to sibling []*godo.AppServiceSpec. -// DO App Platform has no native sidecar model; each sidecar is mapped as an independent -// service in the same app, sharing the app's network namespace via the platform routing. -func sidecarsFromConfig(cfg map[string]any) []*godo.AppServiceSpec { +// DO App Platform has no native sidecar concept; each sidecar becomes an independent +// service component in the same app. Components communicate via the app's internal +// networking (platform-managed DNS/routing), not via a shared Linux network namespace. +// An invalid image ref in any sidecar is returned as an error so misconfiguration fails fast. +func sidecarsFromConfig(cfg map[string]any) ([]*godo.AppServiceSpec, error) { raw, ok := cfg["sidecars"].([]any) if !ok || len(raw) == 0 { - return nil + return nil, nil } out := make([]*godo.AppServiceSpec, 0, len(raw)) for _, v := range raw { @@ -750,9 +756,10 @@ func sidecarsFromConfig(cfg map[string]any) []*godo.AppServiceSpec { } if imgStr := strFromConfig(m, "image", ""); imgStr != "" { img, err := ParseImageRef(imgStr) - if err == nil { - svc.Image = img + if err != nil { + return nil, fmt.Errorf("sidecar %q: %w", name, err) } + svc.Image = img } // Map the first public port to HTTPPort if specified. if ports, ok := m["ports"].([]any); ok { @@ -772,7 +779,7 @@ func sidecarsFromConfig(cfg map[string]any) []*godo.AppServiceSpec { } out = append(out, svc) } - return out + return out, nil } // stringMatchesFromConfig converts a []any of strings to []*godo.AppStringMatch using exact matching. diff --git a/internal/drivers/database.go b/internal/drivers/database.go index 7713431..54a8f7f 100644 --- a/internal/drivers/database.go +++ b/internal/drivers/database.go @@ -76,8 +76,9 @@ func (d *DatabaseDriver) Update(ctx context.Context, ref interfaces.ResourceRef, if err != nil { return nil, fmt.Errorf("database update %q: %w", ref.Name, WrapGodoError(err)) } - // Sync firewall rules when trusted_sources is specified. - if rules := trustedSourceFirewallRulesFromConfig(spec.Config); rules != nil { + // 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 { _, err = d.client.UpdateFirewallRules(ctx, ref.ProviderID, &godo.DatabaseUpdateFirewallRulesRequest{ Rules: rules, }) @@ -165,14 +166,19 @@ func trustedSourceCreateRulesFromConfig(cfg map[string]any) []*godo.DatabaseCrea } // trustedSourceFirewallRulesFromConfig converts canonical "trusted_sources" to -// []*godo.DatabaseFirewallRule for use in a DatabaseUpdateFirewallRulesRequest. -func trustedSourceFirewallRulesFromConfig(cfg map[string]any) []*godo.DatabaseFirewallRule { - raw, ok := cfg["trusted_sources"].([]any) - if !ok || len(raw) == 0 { - return nil - } - rules := make([]*godo.DatabaseFirewallRule, 0, len(raw)) - for _, v := range raw { +// ([]*godo.DatabaseFirewallRule, bool) for use in a DatabaseUpdateFirewallRulesRequest. +// The bool indicates whether the "trusted_sources" key was present in config at all: +// - 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. +func trustedSourceFirewallRulesFromConfig(cfg map[string]any) ([]*godo.DatabaseFirewallRule, bool) { + raw, ok := cfg["trusted_sources"] + if !ok { + return nil, false // 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 @@ -187,7 +193,7 @@ func trustedSourceFirewallRulesFromConfig(cfg map[string]any) []*godo.DatabaseFi Value: ruleValue, }) } - return rules + return rules, true } func dbOutput(db *godo.Database) *interfaces.ResourceOutput { diff --git a/internal/drivers/database_test.go b/internal/drivers/database_test.go index f7e49a7..75a20a2 100644 --- a/internal/drivers/database_test.go +++ b/internal/drivers/database_test.go @@ -248,6 +248,31 @@ func TestDatabaseDriver_Update_NoTrustedSources_SkipsFirewall(t *testing.T) { } } +func TestDatabaseDriver_Update_EmptyTrustedSources_ClearsFirewall(t *testing.T) { + // trusted_sources present but empty → UpdateFirewallRules called with empty rules (clears all). + mock := &mockDatabaseClient{db: testDatabase()} + d := drivers.NewDatabaseDriverWithClient(mock, "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{}, // key present, but empty + }, + }) + if err != nil { + t.Fatalf("Update: %v", err) + } + if mock.lastFirewallReq == nil { + t.Fatal("UpdateFirewallRules must be called when trusted_sources key is present (even if empty)") + } + if len(mock.lastFirewallReq.Rules) != 0 { + t.Errorf("expected 0 rules to clear firewall, got %d", len(mock.lastFirewallReq.Rules)) + } +} + func TestDatabaseDriver_Diff_HasChanges(t *testing.T) { mock := &mockDatabaseClient{} d := drivers.NewDatabaseDriverWithClient(mock, "nyc3")