fix(rds): set StorageEncrypted=true on mysql + postgres instances#239
Merged
fix(rds): set StorageEncrypted=true on mysql + postgres instances#239
Conversation
AWS RDS instances default to UNENCRYPTED storage at rest unless `StorageEncrypted: pulumi.Bool(true)` is explicitly set. Both `pkg/clouds/pulumi/aws/rds_mysql.go:108` and `pkg/clouds/pulumi/aws/rds_postgres.go:101` were missing the field, which is a CIS AWS Foundations Benchmark RDS.3 violation. Setting it now is also a one-shot decision: encryption cannot be toggled on an existing RDS instance without snapshot-restore. `StorageEncrypted: sdk.Bool(true)` uses the AWS-managed RDS KMS key (`aws/rds`) by default. A future change can supply a customer-managed `KmsKeyId` if SC adopts CMK-per-stack. Surfaced by simple-container-com/actions PR #7 (new `go-aws-rds-no-storage-encryption` semgrep rule) — analysing this codebase was what produced the rule. Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
Semgrep Scan ResultsRepository:
Scanned at 2026-05-08 14:50 UTC |
Security Scan ResultsRepository:
Scanned at 2026-05-08 14:51 UTC |
Without this, the StorageEncrypted: sdk.Bool(true) added in this PR
triggers a REPLACEMENT of any pre-existing unencrypted RDS instance
when the customer next runs `sc deploy` after upgrading SC. AWS RDS's
`storage_encrypted` is immutable — Pulumi can only enforce a change
by destroying the instance and creating a new one, which means data
loss + downtime.
Adding `sdk.IgnoreChanges([]string{"storageEncrypted"})` on both
mysql and postgres RDS resource opts so:
* NEW instances created after this lands → get StorageEncrypted: true
* EXISTING pre-encryption instances → state-vs-code drift on this
field is silently ignored; pulumi does NOT propose a replacement
Customers who want to encrypt an existing instance must follow the
standard AWS migration path:
1. Take a snapshot of the unencrypted instance.
2. Copy the snapshot with encryption enabled (CopyDBSnapshot,
KmsKeyId set).
3. Restore from the encrypted snapshot to a new instance.
4. Cut traffic over (DNS / app config), then delete the old.
5. (If using Pulumi state) re-import the new instance into the SC
stack so subsequent diffs match.
Documented inline as a comment on the IgnoreChanges line so future
maintainers don't accidentally remove it and break customer fleets.
Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
Per PR #239 review feedback (Ilya Sadykov): the previous version forced encryption on every NEW RDS instance after upgrade — a backwards- compatibility break for customers who hadn't asked for it. Even with the IgnoreChanges safety net for existing instances, brand-new stacks created post-upgrade would silently get encryption. Refactor to make encryption an explicit opt-in: * MysqlConfig and PostgresConfig get a new optional field: StorageEncrypted *bool `json:"storageEncrypted,omitempty"` * Pulumi resource uses `sdk.Bool(lo.FromPtr(cfg.StorageEncrypted))`. nil → false (legacy default, AWS-side default = unencrypted) true → encrypted (uses AWS-managed `aws/rds` KMS key by default) false → unencrypted (caller asked for it explicitly; respect that) * `sdk.IgnoreChanges([]string{"storageEncrypted"})` is RETAINED on the resource opts. It's still load-bearing: once a customer flips the bit on a stack with a pre-existing unencrypted instance, the AWS-side `storage_encrypted` attribute is immutable, so without IgnoreChanges Pulumi would propose a destructive replacement. Same change in both rds_mysql.go and rds_postgres.go on both the api- config side and the pulumi-resource side. Tests added in pkg/clouds/aws/rds_storage_encrypted_test.go covering all three states (omitted / explicit-true / explicit-false) for both mysql and postgres. Verifies: - Round-trips through `api.ConvertConfig` with the right pointer semantics (omitted YAML → nil pointer in struct). - The resolved `lo.FromPtr` flag matches the documented contract, which is what gets passed to `rds.NewInstance`. Migration path for existing unencrypted instances stays the same and is documented inline next to the IgnoreChanges line: snapshot → encrypted-copy → restore → re-import. Future-proofing: if more RDS hardening fields land (MultiAZ, DeletionProtection, BackupRetentionPeriod...), each new field should follow the same opt-in shape until the count grows enough to justify a `SchemaVersion`-style bundle (mongodb-atlas precedent in pkg/clouds/mongodb/mongodb.go). Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
Tests in pkg/clouds/aws/rds_storage_encrypted_test.go covering all
three states (omitted / explicit-true / explicit-false) for both
MysqlConfig and PostgresConfig. Verifies:
- Round-trips through `api.ConvertConfig` with correct pointer
semantics (omitted YAML → nil pointer).
- The resolved `lo.FromPtr` flag matches the documented contract
(nil → false / true → true / false → false), which is exactly
what gets passed to `rds.NewInstance(StorageEncrypted: ...)`.
Tests hit the same code path the pulumi handlers use, so any future
rename / serialization breakage surfaces here without needing a
Pulumi mock-resource harness. Replacement-safety + IgnoreChanges
behaviour is integration-only and documented separately on the PR.
Should have been in the previous commit.
Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
smecsia
approved these changes
May 8, 2026
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds opt-in encryption-at-rest for AWS RDS instances managed by SC. Existing customers get zero behaviour change unless they explicitly set the new field.
pkg/clouds/aws/rds_mysql.goMysqlConfig.StorageEncrypted *bool(new optional field)pkg/clouds/aws/rds_postgres.goPostgresConfig.StorageEncrypted *bool(same)pkg/clouds/pulumi/aws/rds_mysql.goStorageEncrypted: sdk.Bool(lo.FromPtr(dbConfig.StorageEncrypted))+sdk.IgnoreChanges([]string{"storageEncrypted"})pkg/clouds/pulumi/aws/rds_postgres.gopkg/clouds/aws/rds_storage_encrypted_test.goOpt-in behaviour (per Ilya's review feedback)
dbConfig.StorageEncryptedStorageEncryptednilfalsestorageEncrypted: true&truetrueaws/rdsKMS key)storageEncrypted: false&falsefalseThis means no customer gets encryption silently — they have to opt in.
Why
IgnoreChangesis still requiredstorage_encryptedis immutable post-creation in AWS RDS. WithoutIgnoreChanges, the moment a customer flips the new field totrueon a stack with a pre-existing unencrypted instance, Pulumi would propose a destructive replacement (drop + recreate, data loss + downtime).sdk.IgnoreChanges([]string{"storageEncrypted"})silences that drift. Behaviour:storageEncrypted: true→ created encrypted from the start.true→ no diff, no replacement; the customer's data is safe but the existing instance stays unencrypted. They have to migrate out-of-band (see below).nil→ no diff. Pre-2026.5 behaviour preserved exactly.Migration path for existing unencrypted instances
sc deploywill not migrate automatically — the AWS encryption switch is destructive. The standard AWS path:CopyDBSnapshotwith encryption enabled (KmsKeyIdset).Documented inline next to the
IgnoreChangesline so a future maintainer doesn't accidentally remove the safety.Why surfaced now
simple-container-com/actionsPR #7 adds a new semgrep rulego-aws-rds-no-storage-encryption(ERROR severity). Walking the api codebase to write that rule revealed two pre-existing instances with unset encryption defaults. Originally I went with hard-codedStorageEncrypted: true; per review feedback this is now opt-in.The semgrep rule on actions repo treats
StorageEncrypted: pulumi.Bool(true)literal as "OK", andStorageEncrypted: pulumi.Bool(false)literal as "still ERROR". Our generated value useslo.FromPtrwhich the rule sees textually aspulumi.Bool(...)— but it'ssdk.Boolhere. Verified: rule'spattern-not-regex: 'StorageEncrypted:\s*\w+\.Bool\(true\)'does NOT matchsdk.Bool(lo.FromPtr(...))because the function insideBool(...)isn'ttrue. So this code WILL still trigger the rule when scanned with PR #7's ruleset — by design. The rule is conservative; it can't statically provelo.FromPtrresolves to true. Acceptable trade-off: code is conservatively flagged, reviewer eyeballs the call site, confirms the opt-in plumbing is correct, dismisses or fixes.KMS
StorageEncrypted: sdk.Bool(true)withoutKmsKeyIduses AWS-managedaws/rds. Customer-managed key per stack is a follow-up — the immediate-controls bar is encrypt-at-all when the customer opts in.Test plan
Unit (in this PR — green)
go test ./pkg/clouds/aws/...—TestReadRdsMysqlConfig_StorageEncryptedandTestReadRdsPostgresConfig_StorageEncrypted, 6 subtests total covering all three states for both enginesgo test ./...— cleanIntegration (manual, after merge)
pulumi previewshows+ Instancewithstorage_encrypted: false(legacy behaviour preserved).storage_encrypted: true, KmsKeyId resolves to the AWS-managed default.pulumi previewshows no diff on the RDS instance (this is the critical migration-safety test — proves IgnoreChanges + nil default both kick in).pulumi previewshows no diff (silenced byIgnoreChanges— explicit demonstration of the safety net).E2E (manual, sandbox AWS account)
aws rds describe-db-instances --query '[*].StorageEncrypted'→false.true.sc deploywith new code (no config change) → describe →StorageEncryptedstillfalse, instance ID unchanged.Out of scope
KmsKeyId) — follow-up.NamingStrategyVersion) exists if the count grows.