chore(terraform): load dashboard supabase DB URL from Secret Manager#2440
chore(terraform): load dashboard supabase DB URL from Secret Manager#2440
Conversation
PR SummaryMedium Risk Overview Reviewed by Cursor Bugbot for commit 50e0a27. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 409783a8e1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
LGTM — straightforward migration of the Supabase DB connection string from a plain Terraform variable to Secret Manager, following the established pattern already used for other secrets in this codebase.
Extended reasoning...
Overview\nThis PR modifies five Terraform files in iac/provider-gcp to move the supabase_db_connection_string for dashboard-api from a plain sensitive Terraform variable into GCP Secret Manager. A new secret resource and placeholder version (with ignore_changes = [secret_data]) are created in api.tf, the variable is dropped from both variables.tf files, and the Nomad module receives the secret version object instead, reading the actual value via a google_secret_manager_secret_version data source at deploy time.\n\n### Security risks\nThis is a net security improvement. Previously the connection string was passed as a Terraform variable, meaning it would be stored in Terraform state in plaintext. The new approach stores only a placeholder in state and retrieves the real value from Secret Manager at runtime, consistent with how other sensitive credentials (postgres read replica, redis, etc.) are already handled.\n\n### Level of scrutiny\nLow — this is a mechanical refactor following an established pattern already used multiple times in the same module. The author validated with terraform fmt -recursive and terraform validate. No logic changes, only wiring changes.\n\n### Other factors\nNo bugs were found by the bug hunting system. The change is small, self-contained, and mirrors the postgres_read_replica_connection_string pattern exactly.
dobrac
left a comment
There was a problem hiding this comment.
lgtm. What is the migration path from the env var here?
i think we should add back compatibility for TF_VAR_ from infisical and need to open new pr for cleanup after deployment |
|
ok, lets do that then |
done, happy to merge/deploy |
Summary
dashboard-api'sSUPABASE_DB_CONNECTION_STRINGValidation
terraform fmt -recursiveiniac/provider-gcpterraform validateiniac/provider-gcp