feat(gcp): support arbitrary Cloud SQL database flags on gcp-cloudsql-postgres#357
Open
Cre-eD wants to merge 5 commits into
Open
feat(gcp): support arbitrary Cloud SQL database flags on gcp-cloudsql-postgres#357Cre-eD wants to merge 5 commits into
Cre-eD wants to merge 5 commits into
Conversation
…-postgres
Adds a generic databaseFlags map to the resource config, e.g.:
postgres:
type: gcp-cloudsql-postgres
config:
maxConnections: 200
databaseFlags:
cloudsql.iam_authentication: "on"
Previously max_connections (via maxConnections) was the only flag the
resource could manage, so features gated on instance flags — IAM
database authentication in particular — could not be enabled through
IaC. Flags merge with maxConnections (an explicit max_connections
entry in databaseFlags wins) and render sorted by name so the
resulting array is deterministic across updates. The adoption path
preserves unmanaged existing flags and overrides only the configured
ones, generalizing the previous max_connections-only override.
Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
Semgrep Scan ResultsRepository:
Scanned at 2026-07-02 18:40 UTC |
Security Scan ResultsRepository:
Scanned at 2026-07-02 18:51 UTC |
📊 Statement coverageMeasured on the documented included set (see
Baseline: |
This comment has been minimized.
This comment has been minimized.
Placeholder resolution rebuilds maps via reflect.MakeMap, so an absent databaseFlags arrives as an empty (non-nil) map after resolution. Encode that in the resolved fixtures rather than changing resolution behavior, which other code may rely on. Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
Review findings: - The adoption path listed settings.databaseFlags in IgnoreChanges, so the configured-flag override logic was dead code and the override log line lied. Keep ignoring the path only while no flags are configured. - Merge existing + configured flags through one map so the rendered array is fully sorted; a mixed existing-order/sorted-tail array would churn on every update. Extracted as mergeDatabaseFlags with a unit test (preserve unmanaged, override configured, sort all). - Log only overridden flag names — values could carry substituted secrets. - Regenerate docs/schemas (schema-gen): picks up databaseFlags plus fields added since the last regeneration (backup config, availabilityType, aws storageEncrypted, etc.). Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
…Flags
Follow-up review pass:
- Gate the adoption-path IgnoreChanges exemption on the NEW DatabaseFlags
field instead of the merged flag set: legacy adopted stacks carrying
only maxConnections keep their historical no-op instead of suddenly
applying a restart-requiring static flag on the next deploy. Flag
application on adopted instances is now an explicit opt-in, with a
Warn covering the restart risk and the import-must-match caveat.
- Extract adoptIgnoreChanges + sortedFlagNames helpers; unit-test both
ignore-list branches; split flag tests per helper.
- Lock in yaml.v3 scalar coercion with an unquoted-int databaseFlags
test case (users write flag values bare).
- Mark MaxConnections deprecated in favor of databaseFlags.
- Document databaseFlags in supported-resources.md, including the
removal-semantics asymmetry between provisioned and adopted instances.
- Comment the {}-not-nil resolution artifact at the fixture site.
Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
The deprecation marker trips staticcheck on the shim's own read of the field — the one legitimate internal consumer. Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
smecsia
approved these changes
Jul 3, 2026
universe-ops
approved these changes
Jul 3, 2026
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
3 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.
What
Adds a generic
databaseFlagsmap to thegcp-cloudsql-postgresresource config:Why
max_connections(viamaxConnections) was the only database flag the resource could manage. Features gated on instance flags — IAM database authentication in particular (required for Cloud SQL Auth Proxy--auto-iam-authn/ passwordless Workload Identity DB access) — could not be enabled through IaC and forced an out-of-bandgcloud sql instances patch, which the provisioner would then silently revert on the next refresh+update cycle.How
configuredDatabaseFlagsmerges the legacyMaxConnectionsfield with the new map; an explicitmax_connectionsentry indatabaseFlagswins.adopt_postgres.go) preserves unmanaged existing instance flags and overrides only configured ones, generalizing the previous max_connections-only override logic.Compatibility
Existing configs are unaffected:
maxConnectionsbehaves exactly as before;databaseFlagsis optional and omitted from serialization when empty.