Commit e8f1a7a
authored
fix(helm): expand CLICKHOUSE_PASSWORD in webapp CLICKHOUSE_URL via kubelet (#3449)
## Summary
When the official Helm chart is deployed with an external ClickHouse and
`clickhouse.external.existingSecret` set — the documented path for not
committing secrets to `values.yaml` — the webapp pod crash-loops on
startup:
```
goose run: parse "http://default:${CLICKHOUSE_PASSWORD}@<host>:8123?secure=false": net/url: invalid userinfo
```
Context in vouch request #3443. Re-opening in draft status per bot
policy (previous attempt was #3445, closed by automation because it
wasn't draft; no changes to the patch).
## Root cause
Two pieces interact:
1. `hosting/k8s/helm/templates/_helpers.tpl` renders `CLICKHOUSE_URL`
(and `RUN_REPLICATION_CLICKHOUSE_URL`) with a shell-style literal
`${CLICKHOUSE_PASSWORD}` expecting bash expansion at container start.
2. `docker/scripts/entrypoint.sh` does `export
GOOSE_DBSTRING="$CLICKHOUSE_URL"` — single-pass POSIX sh substitution,
so the inner `${...}` survives as literal text and goose rejects it.
Reproduces against the latest published chart
(`oci://ghcr.io/triggerdotdev/charts/trigger:4.0.5`) and `main`.
## Fix
Switch the two helpers (external + `existingSecret` branch) from
shell-style `${CLICKHOUSE_PASSWORD}` to Kubernetes'
`$(CLICKHOUSE_PASSWORD)`. Kubelet substitutes `$(VAR)` at pod-creation
time from earlier env entries, and the chart already declares
`CLICKHOUSE_PASSWORD` from the Secret immediately before
`CLICKHOUSE_URL`, so the URL reaches the entrypoint with the real
password already inlined. No entrypoint change, no image change. The
plain-password branch (no `existingSecret`) is unchanged.
Operator caveat added as template comments: `CLICKHOUSE_PASSWORD` must
be URL-userinfo-safe since kubelet substitutes verbatim without
percent-encoding. Hex-encoded passwords (e.g. `openssl rand -hex 32`)
are safe by construction.
## Verification
- `helm template` against `external.existingSecret` now renders `value:
"http://default:$(CLICKHOUSE_PASSWORD)@<host>:8123?secure=false"` (was
`${CLICKHOUSE_PASSWORD}`).
- `helm template` against the plain-password branch is byte-identical to
before.
- Deployed end-to-end on a staging EKS cluster (Meistrari platform):
webapp container reaches `goose: successfully migrated database to
version: 6`, Node.js ClickHouse client connects at runtime.
## Alternatives considered
- **Change `entrypoint.sh`** to `eval` / `envsubst` the URL — larger
surface, touches every deployment mode (Docker Compose + k8s) and every
container image.
- **Mirror the Postgres pattern** (chart reads the full URL via
`valueFrom.secretKeyRef`, as in `trigger-v4.postgres.useSecretUrl`) —
cleaner long-term but requires a new `values.yaml` field and a migration
path for existing users. Happy to follow up with that as a separate PR
if the minimal fix here isn't the preferred direction.
## Changeset
None added — the Helm chart isn't versioned through `@changesets/cli`
(docs/chart-only PRs historically merge without a changeset, e.g.
#2671). Happy to add one if the policy changed.
Closes #3443.1 parent 4dced14 commit e8f1a7a
1 file changed
Lines changed: 18 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
400 | 400 | | |
401 | 401 | | |
402 | 402 | | |
| 403 | + | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
403 | 416 | | |
404 | 417 | | |
405 | 418 | | |
| |||
410 | 423 | | |
411 | 424 | | |
412 | 425 | | |
413 | | - | |
| 426 | + | |
414 | 427 | | |
415 | 428 | | |
416 | 429 | | |
| |||
419 | 432 | | |
420 | 433 | | |
421 | 434 | | |
| 435 | + | |
| 436 | + | |
| 437 | + | |
422 | 438 | | |
423 | 439 | | |
424 | 440 | | |
| |||
427 | 443 | | |
428 | 444 | | |
429 | 445 | | |
430 | | - | |
| 446 | + | |
431 | 447 | | |
432 | 448 | | |
433 | 449 | | |
| |||
0 commit comments