From 7436b6b81577c7d9b5b39105cafdc84cead37e64 Mon Sep 17 00:00:00 2001 From: Dmitrii Creed Date: Sat, 9 May 2026 12:16:21 +0400 Subject: [PATCH] fix(rds): flip StorageEncrypted default to true (CIS-AWS RDS.3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #239 added the `StorageEncrypted *bool` field to MysqlConfig / PostgresConfig as opt-in (nil → false → unencrypted), preserving pre-2026.5 SC behaviour. That keeps every customer who hasn't read the new field's docs on unencrypted RDS, violating CIS-AWS Foundations RDS.3. This change flips the resolution helper from `lo.FromPtr(*bool)` to `lo.FromPtrOr(*bool, true)` at both call sites: pkg/clouds/pulumi/aws/rds_mysql.go:137 pkg/clouds/pulumi/aws/rds_postgres.go:121 Resolution table: before: after: nil → false (unencrypted) nil → true (encrypted, secure default) &true → true &true → true (unchanged) &false → false &false → false (unchanged, explicit opt-out) Why this is safe for existing instances ======================================= PR #239 already added `IgnoreChanges([]string{"storageEncrypted"})` on the resource opts for exactly this scenario. Pulumi's IgnoreChanges diffing rule: "treat the desired value as the recorded state value for the field." So: Existing unencrypted instance, customer never set the field in YAML: state.storageEncrypted = false new spec.storageEncrypted = true (from lo.FromPtrOr(nil, true)) diff: IgnoreChanges → no change registered → no operation ↳ existing DB stays as-is, NOT replaced. No data loss. Existing encrypted instance, customer set true previously: state.storageEncrypted = true new spec.storageEncrypted = true diff: trivially no change ↳ unchanged. Customer explicitly sets `storageEncrypted: false` in YAML: state.storageEncrypted = false (or whatever it was) new spec.storageEncrypted = false (lo.FromPtrOr(&false, true) = false) diff: IgnoreChanges → no change ↳ honoured; no second-guessing. Brand-new stack, customer never sets the field: no prior state — IgnoreChanges has no effect on initial create new spec.storageEncrypted = true ↳ AWS creates an encrypted instance. ✓ secure default. Out-of-band migration ===================== Customers who actually want to migrate an existing unencrypted RDS to encrypted still have to do it the AWS-immutable-field way: snapshot → encrypted-copy → restore → re-import. The IgnoreChanges silences SC's spec from triggering Pulumi to attempt that migration via destructive replacement. Documented on the struct field comments. Tests ===== `pkg/clouds/aws/rds_storage_encrypted_test.go` updated: - "omitted → nil (legacy default, encryption off)" → "omitted → nil (secure default, encryption on)" - resolution helper changed from `lo.FromPtr(cfg.StorageEncrypted)` to `lo.FromPtrOr(cfg.StorageEncrypted, true)` - expected value changed from `wantSet && wantVal` to `!wantSet || wantVal` - assertion message updated accordingly. Build, vet, and TestRead{Mysql,Postgres}Config_StorageEncrypted all pass locally. Companion: simple-container-com/actions PR #9 (open) tightens `go-aws-rds-no-storage-encryption` to whitelist only the `lo.FromPtrOr(_, true)` shape — bare `lo.FromPtr(*ptr)` and `lo.FromPtrOr(*ptr, false)` both still fire the rule, so this codebase will pass the rule going forward and any regression to the old shape gets flagged at PR time. Signed-off-by: Dmitrii Creed --- pkg/clouds/aws/rds_mysql.go | 25 +++++++----- pkg/clouds/aws/rds_postgres.go | 25 +++++++----- pkg/clouds/aws/rds_storage_encrypted_test.go | 43 +++++++++++--------- pkg/clouds/pulumi/aws/rds_mysql.go | 29 +++++++------ pkg/clouds/pulumi/aws/rds_postgres.go | 16 +++++--- 5 files changed, 78 insertions(+), 60 deletions(-) diff --git a/pkg/clouds/aws/rds_mysql.go b/pkg/clouds/aws/rds_mysql.go index 85f16bd0..574526e3 100644 --- a/pkg/clouds/aws/rds_mysql.go +++ b/pkg/clouds/aws/rds_mysql.go @@ -18,18 +18,21 @@ type MysqlConfig struct { Password string `json:"password" yaml:"password"` DatabaseName *string `json:"databaseName" yaml:"databaseName"` EngineName *string `json:"engineName,omitempty" yaml:"engineName,omitempty"` - // StorageEncrypted opts into AWS-side encryption-at-rest for the - // underlying EBS volume. When unset (nil), the instance is created - // with the AWS default — currently UNENCRYPTED — preserving exact - // behaviour for stacks that pre-date this field. Set `true` to opt - // the resource into encryption (uses the AWS-managed `aws/rds` KMS - // key by default). + // StorageEncrypted controls AWS-side encryption-at-rest for the + // underlying EBS volume. When unset (nil), new instances default + // to ENCRYPTED (AWS-managed `aws/rds` KMS key), matching CIS-AWS + // Foundations RDS.3. Set `false` explicitly to opt out for legacy + // unencrypted stacks; set `true` to be explicit. // - // AWS RDS `storage_encrypted` is IMMUTABLE post-creation. Toggling - // this field on an existing unencrypted instance does NOT migrate - // data — it is silenced via `pulumi.IgnoreChanges` to prevent a - // destructive replacement. To convert an existing unencrypted RDS - // to encrypted, snapshot → encrypted-copy → restore → re-import. + // AWS RDS `storage_encrypted` is IMMUTABLE post-creation. The + // default flip is safe for existing instances because the + // `pulumi.IgnoreChanges` on the resource opts (see + // pkg/clouds/pulumi/aws/rds_mysql.go) silences storage_encrypted + // drift — Pulumi will not propose a destructive replacement when + // the spec value differs from the cloud-actual value. Customers + // who want to genuinely migrate an existing unencrypted RDS to + // encrypted must do it out-of-band: snapshot → encrypted-copy → + // restore → re-import. StorageEncrypted *bool `json:"storageEncrypted,omitempty" yaml:"storageEncrypted,omitempty"` } diff --git a/pkg/clouds/aws/rds_postgres.go b/pkg/clouds/aws/rds_postgres.go index 646ee92b..7ee3d037 100644 --- a/pkg/clouds/aws/rds_postgres.go +++ b/pkg/clouds/aws/rds_postgres.go @@ -18,18 +18,21 @@ type PostgresConfig struct { Password string `json:"password" yaml:"password"` DatabaseName *string `json:"databaseName" yaml:"databaseName"` InitSQL *string `json:"initSQL,omitempty" yaml:"initSQL,omitempty"` - // StorageEncrypted opts into AWS-side encryption-at-rest for the - // underlying EBS volume. When unset (nil), the instance is created - // with the AWS default — currently UNENCRYPTED — preserving exact - // behaviour for stacks that pre-date this field. Set `true` to opt - // the resource into encryption (uses the AWS-managed `aws/rds` KMS - // key by default). + // StorageEncrypted controls AWS-side encryption-at-rest for the + // underlying EBS volume. When unset (nil), new instances default + // to ENCRYPTED (AWS-managed `aws/rds` KMS key), matching CIS-AWS + // Foundations RDS.3. Set `false` explicitly to opt out for legacy + // unencrypted stacks; set `true` to be explicit. // - // AWS RDS `storage_encrypted` is IMMUTABLE post-creation. Toggling - // this field on an existing unencrypted instance does NOT migrate - // data — it is silenced via `pulumi.IgnoreChanges` to prevent a - // destructive replacement. To convert an existing unencrypted RDS - // to encrypted, snapshot → encrypted-copy → restore → re-import. + // AWS RDS `storage_encrypted` is IMMUTABLE post-creation. The + // default flip is safe for existing instances because the + // `pulumi.IgnoreChanges` on the resource opts (see + // pkg/clouds/pulumi/aws/rds_postgres.go) silences storage_encrypted + // drift — Pulumi will not propose a destructive replacement when + // the spec value differs from the cloud-actual value. Customers + // who want to genuinely migrate an existing unencrypted RDS to + // encrypted must do it out-of-band: snapshot → encrypted-copy → + // restore → re-import. StorageEncrypted *bool `json:"storageEncrypted,omitempty" yaml:"storageEncrypted,omitempty"` } diff --git a/pkg/clouds/aws/rds_storage_encrypted_test.go b/pkg/clouds/aws/rds_storage_encrypted_test.go index 7494cea8..0eb8a432 100644 --- a/pkg/clouds/aws/rds_storage_encrypted_test.go +++ b/pkg/clouds/aws/rds_storage_encrypted_test.go @@ -9,20 +9,23 @@ import ( "github.com/simple-container-com/api/pkg/api" ) -// Tests for the opt-in `StorageEncrypted` field on MysqlConfig / -// PostgresConfig. Three states matter: +// Tests for the `StorageEncrypted` field on MysqlConfig / PostgresConfig. +// Three states matter: // -// 1. omitted from YAML / JSON → field stays nil → `lo.FromPtr(nil)` -// collapses to `false`, which preserves pre-2026.5 SC behaviour -// for stacks created without the field. -// 2. explicit `true` → encrypted instance. -// 3. explicit `false` → still unencrypted (caller asked for it -// explicitly; we don't second-guess). +// 1. omitted from YAML / JSON → field stays nil → +// `lo.FromPtrOr(nil, true)` collapses to `true`, the secure +// default per CIS-AWS RDS.3. +// 2. explicit `true` → encrypted instance (same). +// 3. explicit `false` → unencrypted (caller asked for it explicitly; +// the secure default does not override their choice). // -// The actual replacement-safety guarantee for existing instances comes -// from `pulumi.IgnoreChanges([]{"storageEncrypted"})` on the resource -// opts (see pkg/clouds/pulumi/aws/rds_{mysql,postgres}.go) and is -// covered by integration / e2e tests, not here. +// The replacement-safety guarantee for existing unencrypted instances +// comes from `pulumi.IgnoreChanges([]{"storageEncrypted"})` on the +// resource opts (see pkg/clouds/pulumi/aws/rds_{mysql,postgres}.go) — +// the default flip from nil→true does NOT propose a destructive +// replace, because Pulumi diffs against the recorded state value +// (which has storage_encrypted=false on legacy instances) rather than +// the new spec value. Covered by integration / e2e tests, not here. func TestReadRdsMysqlConfig_StorageEncrypted(t *testing.T) { RegisterTestingT(t) @@ -34,7 +37,7 @@ func TestReadRdsMysqlConfig_StorageEncrypted(t *testing.T) { wantVal bool }{ { - name: "omitted → nil (legacy default, encryption off)", + name: "omitted → nil (secure default, encryption on)", config: &api.Config{Config: map[string]any{ "instanceClass": "db.t3.micro", "engineVersion": "8.0", @@ -79,18 +82,18 @@ func TestReadRdsMysqlConfig_StorageEncrypted(t *testing.T) { if !tt.wantSet { Expect(cfg.StorageEncrypted).To(BeNil(), - "unset field should round-trip as nil so `lo.FromPtr` resolves to false") + "unset field should round-trip as nil so `lo.FromPtrOr(_, true)` resolves to true") } else { Expect(cfg.StorageEncrypted).ToNot(BeNil()) Expect(*cfg.StorageEncrypted).To(Equal(tt.wantVal)) } - // `lo.FromPtr(nil)` is `false` — explicitly assert the - // resolved Pulumi flag matches the documented contract. - resolved := lo.FromPtr(cfg.StorageEncrypted) - expected := tt.wantSet && tt.wantVal + // `lo.FromPtrOr(nil, true)` is `true` — secure-by-default. + // nil → true / true → true / false → false. + resolved := lo.FromPtrOr(cfg.StorageEncrypted, true) + expected := !tt.wantSet || tt.wantVal Expect(resolved).To(Equal(expected), - "resolved flag passed to `rds.NewInstance` must match nil → false / true → true / false → false") + "resolved flag passed to `rds.NewInstance` must match nil → true / true → true / false → false") }) } } @@ -105,7 +108,7 @@ func TestReadRdsPostgresConfig_StorageEncrypted(t *testing.T) { wantVal bool }{ { - name: "omitted → nil (legacy default, encryption off)", + name: "omitted → nil (secure default, encryption on)", config: &api.Config{Config: map[string]any{ "instanceClass": "db.t3.micro", "engineVersion": "16", diff --git a/pkg/clouds/pulumi/aws/rds_mysql.go b/pkg/clouds/pulumi/aws/rds_mysql.go index 1cccf32c..413ffe5b 100644 --- a/pkg/clouds/pulumi/aws/rds_mysql.go +++ b/pkg/clouds/pulumi/aws/rds_mysql.go @@ -40,16 +40,18 @@ func RdsMysql(ctx *sdk.Context, stack api.Stack, input api.ResourceInput, params // false to true triggers a full replacement of the instance, // which destroys the underlying volume and all its data. // - // The `StorageEncrypted` config field below is opt-in (nil = - // keep AWS default = unencrypted, preserving pre-2026.5 SC - // behaviour). But even with opt-in semantics on the SC side, - // once a customer flips the bit on a stack with a pre-existing - // unencrypted instance, Pulumi would still propose a destructive - // replacement. This `IgnoreChanges` silences that drift so a - // config change does NOT nuke the database. Customers who want - // to genuinely migrate an existing unencrypted RDS to encrypted - // must do it out-of-band: snapshot → encrypted-copy → restore → - // re-import. Documented on `MysqlConfig.StorageEncrypted`. + // New instances default to ENCRYPTED (CIS-AWS RDS.3); the + // resolved spec for `StorageEncrypted` is `true` whenever the + // caller didn't explicitly set `false` (see the call site + // below). For existing stacks that were created unencrypted, + // the default flip would otherwise propose a destructive + // replacement on the next `pulumi up`. `IgnoreChanges` silences + // that storage_encrypted drift so an upgrade of SC does NOT + // nuke the database — Pulumi treats the desired value as the + // recorded state value for the diff. Customers who want to + // genuinely migrate an existing unencrypted RDS to encrypted + // must do it out-of-band: snapshot → encrypted-copy → restore + // → re-import. Documented on `MysqlConfig.StorageEncrypted`. sdk.IgnoreChanges([]string{"storageEncrypted"}), } @@ -133,8 +135,11 @@ func RdsMysql(ctx *sdk.Context, stack api.Stack, input api.ResourceInput, params Username: sdk.String(lo.If(dbConfig.Username != "", dbConfig.Username).Else("root")), Password: sdk.String(lo.If(dbConfig.Password != "", dbConfig.Password).Else("root")), SkipFinalSnapshot: sdk.Bool(true), - // nil → false (legacy default). See MysqlConfig.StorageEncrypted. - StorageEncrypted: sdk.Bool(lo.FromPtr(dbConfig.StorageEncrypted)), + // nil → true (secure-by-default per CIS-AWS RDS.3). Existing + // unencrypted instances are protected from destructive + // replacement by `IgnoreChanges([]string{"storageEncrypted"})` + // in opts above. See MysqlConfig.StorageEncrypted. + StorageEncrypted: sdk.Bool(lo.FromPtrOr(dbConfig.StorageEncrypted, true)), Tags: tags, }, opts...) if err != nil { diff --git a/pkg/clouds/pulumi/aws/rds_postgres.go b/pkg/clouds/pulumi/aws/rds_postgres.go index a912cbe9..381c8fd5 100644 --- a/pkg/clouds/pulumi/aws/rds_postgres.go +++ b/pkg/clouds/pulumi/aws/rds_postgres.go @@ -37,10 +37,11 @@ func RdsPostgres(ctx *sdk.Context, stack api.Stack, input api.ResourceInput, par opts := []sdk.ResourceOption{ sdk.Provider(params.Provider), // See rds_mysql.go for the full rationale. Same shape applies - // to postgres: opt-in via `PostgresConfig.StorageEncrypted` - // (nil = AWS default = unencrypted, pre-2026.5 SC behaviour), - // and `IgnoreChanges` silences drift so flipping the bit on - // an existing stack does not trigger a destructive replacement. + // to postgres: new instances default to ENCRYPTED via + // `PostgresConfig.StorageEncrypted` (nil = secure-by-default + // per CIS-AWS RDS.3), and `IgnoreChanges` silences drift so + // the default flip does not propose a destructive replacement + // on existing unencrypted stacks. sdk.IgnoreChanges([]string{"storageEncrypted"}), } @@ -117,8 +118,11 @@ func RdsPostgres(ctx *sdk.Context, stack api.Stack, input api.ResourceInput, par Username: sdk.String(lo.If(postgresCfg.Username != "", postgresCfg.Username).Else("postgres")), Password: sdk.String(lo.If(postgresCfg.Password != "", postgresCfg.Password).Else("postgres")), SkipFinalSnapshot: sdk.Bool(true), - // nil → false (legacy default). See PostgresConfig.StorageEncrypted. - StorageEncrypted: sdk.Bool(lo.FromPtr(postgresCfg.StorageEncrypted)), + // nil → true (secure-by-default per CIS-AWS RDS.3). Existing + // unencrypted instances are protected from destructive + // replacement by `IgnoreChanges([]string{"storageEncrypted"})` + // in opts above. See PostgresConfig.StorageEncrypted. + StorageEncrypted: sdk.Bool(lo.FromPtrOr(postgresCfg.StorageEncrypted, true)), Tags: tags, }, opts...) if err != nil {