Repository navigation
feat(ci-deploy): pg_dump snapshots verify TLS with the application's CA ConfigMap - #244
Conversation
…CA ConfigMap
A pg_dump database entry may name {configMap, key}; its CA becomes
PGSSLROOTCERT, so sslmode=verify-full URLs (Supabase pooler) dump with the
same trust the application uses.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 47 minutes. View limit details
No actionable comments were generated in the recent review. Walkthrough
Merge Risk:
|
There was a problem hiding this comment.
Devin Review found 2 potential issues.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| if [ -n "$ca" ]; then | ||
| kubectl -n "$ns" get configmap "${ca%%/*}" -o json | jq -j --arg k "${ca#*/}" '.data[$k] // empty' > "$WORK/pg-ca-$name.crt" | ||
| [ -s "$WORK/pg-ca-$name.crt" ] || die "snapshot $name: CA ConfigMap $ca is empty or missing" | ||
| export PGSSLROOTCERT="$WORK/pg-ca-$name.crt" |
There was a problem hiding this comment.
🟡 Database URL overrides configured snapshot CA
When a database URL contains a pod-only sslrootcert path, PGSSLROOTCERT cannot override it. Libpq prioritizes URL parameters, so pg_dump fails to read the CA and blocks the apply.
Learn more
A snapshot URL can include an explicit sslrootcert option, as native_control constructs for an application pod. Libpq uses explicit connection-string options before environment defaults. CodeBuild does not mount the application's /etc/postgres-ca/ca.crt, so setting PGSSLROOTCERT to the downloaded ConfigMap file does not help that URL. The snapshot fails before the Helm upgrade.
Example: A Secret holds postgresql://user:pass@db.example.test/app?sslmode=verify-full&sslrootcert=/etc/postgres-ca/ca.crt. The ConfigMap CA is downloaded to a temporary file, but pg_dump still opens /etc/postgres-ca/ca.crt and exits instead of completing the snapshot.
Recommended fix: For a database entry with ca, use a connection string for pg_dump that removes or replaces any explicit sslrootcert parameter with the downloaded path, without logging or exposing the URL. Add a test with an explicit pod-only sslrootcert URL.
Was this helpful? React with 👍 or 👎 to provide feedback.
| def test_pg_dump_uses_configured_ca(self): | ||
| self.render({"a": 2}, DEPLOY) | ||
| self.config({"type": "pg_dump", "databases": [{"label": "control", "namespace": "ns", "secret": "db", "key": "url", | ||
| "ca": {"configMap": "supabase-ca", "key": "ca.crt"}}]}) |
There was a problem hiding this comment.
🟡 Production CA identifier committed in test
The new fixture names supabase-ca, the production CA ConfigMap, rather than an obviously fake resource. Repository contribution rules prohibit production identifiers in tests.
| "ca": {"configMap": "supabase-ca", "key": "ca.crt"}}]}) | |
| "ca": {"configMap": "example-test-ca", "key": "ca.crt"}}]}) |
Was this helpful? React with 👍 or 👎 to provide feedback.
[Medium impact] The PR appears safe to merge; no blocking findings remain.
|
… tests libpq prefers a URL sslrootcert over PGSSLROOTCERT, so a pod-only path in the URL would have defeated the configured CA. Tests use a fake ConfigMap name and assert the exact PEM bytes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Prod (agentrelay-prod) snapshots its Control database on Supabase with
pg_dumpbefore every apply. The URL usessslmode=verify-full, but the CodeBuild job has no Supabase CA. Apg_dumpdatabase entry can now nameca: {configMap, key}, in the same namespace. That CA, the one relay-control already trusts (supabase-ca), becomesPGSSLROOTCERTfor the dump.New test included (30 pass); shellcheck is clean.
🤖 Generated with Claude Code
Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.Summary by cubic
Allows
pg_dumpsnapshot entries to specify a CA ConfigMap, so database URLs withsslmode=verify-full(like Supabase poolers) dump using the same trust anchor the application uses.ca: {configMap, key}field to apg_dumpdatabase entry; when present, that CA is written to a file and exported asPGSSLROOTCERTfor the dump, then unset per entry.sslrootcert(often a pod-only path), the configured CA replaces it, since libpq favors the URL value overPGSSLROOTCERT, while preserving the URL's percent-encoding.sslrootcertreplacement, and preserved percent-encoding.Written for commit 15aa78e. Summary will update on new commits.