mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 14:21:37 +00:00
fix(config): DEPL-004 — expand $(POSTGRES_PASSWORD) placeholder in CERTCTL_DATABASE_URL
Sprint 3 unified-master-audit closure. The Helm chart's _helpers.tpl
(line 133) renders the bundled-Postgres URL with a literal
'$(POSTGRES_PASSWORD)' placeholder:
postgres://certctl:$(POSTGRES_PASSWORD)@db:5432/certctl?sslmode=disable
Kubernetes' '$(VAR)' env-substitution syntax ONLY expands when the
value is a string literal in the Pod spec. Values sourced from
'valueFrom.secretKeyRef' (which is how the chart wires
CERTCTL_DATABASE_URL) are NOT expanded — the literal makes it all
the way to the server, which tries to dial Postgres with
'$(POSTGRES_PASSWORD)' as the password, fails with auth error, and
leaks the placeholder into application error logs.
Fix: in-process expansion at internal/config/config.expandDatabaseURL.
strings.ReplaceAll of the literal '$(POSTGRES_PASSWORD)' token with
os.Getenv('POSTGRES_PASSWORD') when both the token is present AND
the env var is set. Conservative — no os.ExpandEnv (which would
expand any $VAR), no Docker entrypoint shim, no Helm-template-time
password injection that would inline the secret into a second
Kubernetes resource. External-Postgres deploys whose URL embeds
the real password pass through untouched because the placeholder
doesn't match.
Regression coverage in internal/config/config_test.go pins:
- happy-path placeholder substitution
- non-placeholder URL passes through unchanged
- placeholder + empty POSTGRES_PASSWORD leaves the URL alone
- multi-occurrence safety via ReplaceAll
Closes DEPL-004.
This commit is contained in:
@@ -333,7 +333,23 @@ func Load() (*Config, error) {
|
|||||||
AuditFlushTimeoutSeconds: getEnvInt("CERTCTL_AUDIT_FLUSH_TIMEOUT_SECONDS", 30),
|
AuditFlushTimeoutSeconds: getEnvInt("CERTCTL_AUDIT_FLUSH_TIMEOUT_SECONDS", 30),
|
||||||
},
|
},
|
||||||
Database: DatabaseConfig{
|
Database: DatabaseConfig{
|
||||||
URL: getEnv("CERTCTL_DATABASE_URL", "postgres://localhost/certctl"),
|
// DEPL-004 closure (Sprint 3, 2026-05-16). The Helm chart's
|
||||||
|
// _helpers.tpl renders the bundled-Postgres URL with a literal
|
||||||
|
// `$(POSTGRES_PASSWORD)` placeholder (see
|
||||||
|
// deploy/helm/certctl/templates/_helpers.tpl line 133). The
|
||||||
|
// Kubernetes env-substitution `$(VAR)` syntax ONLY expands
|
||||||
|
// when the value is a string literal in `env:` — values
|
||||||
|
// sourced from Secrets (via `valueFrom.secretKeyRef`) are
|
||||||
|
// passed through verbatim with no expansion. Pre-fix the
|
||||||
|
// server received the literal "postgres://user:$(POSTGRES_PASSWORD)@..."
|
||||||
|
// string and tried to dial Postgres with that as the password,
|
||||||
|
// failing with auth error and leaking the placeholder into
|
||||||
|
// error logs. expandDatabaseURL substitutes the placeholder
|
||||||
|
// with os.Getenv("POSTGRES_PASSWORD") when present; external-
|
||||||
|
// Postgres deploys that bake the password directly into the
|
||||||
|
// URL string are unaffected because there is no placeholder
|
||||||
|
// to match.
|
||||||
|
URL: expandDatabaseURL(getEnv("CERTCTL_DATABASE_URL", "postgres://localhost/certctl")),
|
||||||
// Phase 6 SCALE-M1 closure (2026-05-14): bumped default from
|
// Phase 6 SCALE-M1 closure (2026-05-14): bumped default from
|
||||||
// 25 → 50 to relieve pool-saturation pressure on 1K+ agent /
|
// 25 → 50 to relieve pool-saturation pressure on 1K+ agent /
|
||||||
// 10K+ cert fleets. Postgres default max_connections is 100
|
// 10K+ cert fleets. Postgres default max_connections is 100
|
||||||
@@ -1269,6 +1285,37 @@ func getEnv(key, defaultValue string) string {
|
|||||||
return defaultValue
|
return defaultValue
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// expandDatabaseURL substitutes the literal "$(POSTGRES_PASSWORD)"
|
||||||
|
// placeholder in a database URL with the value of the POSTGRES_PASSWORD
|
||||||
|
// environment variable. DEPL-004 closure (Sprint 3, 2026-05-16).
|
||||||
|
//
|
||||||
|
// Kubernetes ONLY expands `$(VAR)` syntax when the env value is a
|
||||||
|
// string literal in the Pod spec. Values sourced from
|
||||||
|
// `valueFrom.secretKeyRef` (which is how the Helm chart wires
|
||||||
|
// CERTCTL_DATABASE_URL) are NOT expanded — the literal makes it all
|
||||||
|
// the way to the application. This helper does the expansion in-process
|
||||||
|
// so the bundled-Postgres flow Just Works without a per-pod entrypoint
|
||||||
|
// shim.
|
||||||
|
//
|
||||||
|
// Conservative: a strings.Replace on exactly one well-known token
|
||||||
|
// (the chart's `_helpers.tpl` produces `$(POSTGRES_PASSWORD)` and
|
||||||
|
// nothing else). External-Postgres deploys whose URL embeds the
|
||||||
|
// real password don't match the placeholder and pass through untouched.
|
||||||
|
// When POSTGRES_PASSWORD is unset, the URL is left as-is so the
|
||||||
|
// downstream connection failure is the same as before (and a missing
|
||||||
|
// password is the operator's mis-config, not our regression).
|
||||||
|
func expandDatabaseURL(url string) string {
|
||||||
|
const placeholder = "$(POSTGRES_PASSWORD)"
|
||||||
|
if !strings.Contains(url, placeholder) {
|
||||||
|
return url
|
||||||
|
}
|
||||||
|
pw := os.Getenv("POSTGRES_PASSWORD")
|
||||||
|
if pw == "" {
|
||||||
|
return url
|
||||||
|
}
|
||||||
|
return strings.ReplaceAll(url, placeholder, pw)
|
||||||
|
}
|
||||||
|
|
||||||
// getEnvInt reads an integer environment variable with the given key and default value.
|
// getEnvInt reads an integer environment variable with the given key and default value.
|
||||||
func getEnvInt(key string, defaultValue int) int {
|
func getEnvInt(key string, defaultValue int) int {
|
||||||
if value := os.Getenv(key); value != "" {
|
if value := os.Getenv(key); value != "" {
|
||||||
|
|||||||
@@ -1918,3 +1918,54 @@ func TestValidate_Bundle2_CORSConcreteAllowlist_Accepted(t *testing.T) {
|
|||||||
t.Errorf("Validate() returned %v; want nil for concrete CORS allowlist", err)
|
t.Errorf("Validate() returned %v; want nil for concrete CORS allowlist", err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// =============================================================================
|
||||||
|
// DEPL-004 closure (Sprint 3, 2026-05-16). The Helm chart renders the
|
||||||
|
// bundled-Postgres URL with a literal "$(POSTGRES_PASSWORD)"
|
||||||
|
// placeholder. Kubernetes does NOT expand `$(VAR)` syntax when the env
|
||||||
|
// is sourced from a Secret (valueFrom.secretKeyRef), so the server
|
||||||
|
// receives the placeholder verbatim. expandDatabaseURL substitutes the
|
||||||
|
// token with os.Getenv("POSTGRES_PASSWORD") at Load() time.
|
||||||
|
// =============================================================================
|
||||||
|
|
||||||
|
func TestExpandDatabaseURL_SubstitutesPlaceholder(t *testing.T) {
|
||||||
|
t.Setenv("POSTGRES_PASSWORD", "s3cret!")
|
||||||
|
in := "postgres://certctl:$(POSTGRES_PASSWORD)@db:5432/certctl?sslmode=disable"
|
||||||
|
got := expandDatabaseURL(in)
|
||||||
|
want := "postgres://certctl:s3cret!@db:5432/certctl?sslmode=disable"
|
||||||
|
if got != want {
|
||||||
|
t.Errorf("expandDatabaseURL = %q; want %q", got, want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestExpandDatabaseURL_NoPlaceholderPassesThrough(t *testing.T) {
|
||||||
|
// External-Postgres deploys bake the password into the URL string
|
||||||
|
// — the helper must not touch URLs that don't carry the placeholder.
|
||||||
|
t.Setenv("POSTGRES_PASSWORD", "ignored")
|
||||||
|
in := "postgres://user:realpw@external:5432/db?sslmode=require"
|
||||||
|
if got := expandDatabaseURL(in); got != in {
|
||||||
|
t.Errorf("expandDatabaseURL on non-placeholder URL = %q; want %q (no-op)", got, in)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestExpandDatabaseURL_PlaceholderButNoEnvLeftAlone(t *testing.T) {
|
||||||
|
// When POSTGRES_PASSWORD is unset, leave the URL alone so the
|
||||||
|
// downstream connection failure is the same as before (misconfig
|
||||||
|
// is the operator's, not our regression).
|
||||||
|
t.Setenv("POSTGRES_PASSWORD", "")
|
||||||
|
in := "postgres://certctl:$(POSTGRES_PASSWORD)@db:5432/certctl?sslmode=disable"
|
||||||
|
if got := expandDatabaseURL(in); got != in {
|
||||||
|
t.Errorf("expandDatabaseURL with no POSTGRES_PASSWORD = %q; want unchanged %q", got, in)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestExpandDatabaseURL_MultipleOccurrences(t *testing.T) {
|
||||||
|
// Defensive: belt-and-suspenders. The chart only emits one
|
||||||
|
// placeholder today but ReplaceAll guards against future drift.
|
||||||
|
t.Setenv("POSTGRES_PASSWORD", "X")
|
||||||
|
in := "$(POSTGRES_PASSWORD)/$(POSTGRES_PASSWORD)"
|
||||||
|
want := "X/X"
|
||||||
|
if got := expandDatabaseURL(in); got != want {
|
||||||
|
t.Errorf("expandDatabaseURL = %q; want %q", got, want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user