fix(helm): expand CLICKHOUSE_PASSWORD in webapp CLICKHOUSE_URL via kubelet#3445
Conversation
…belet
When clickhouse.external.existingSecret is set, the chart rendered the
CLICKHOUSE_URL env var with a literal shell-style ${CLICKHOUSE_PASSWORD}
placeholder, expecting bash to expand it at container start. But
docker/scripts/entrypoint.sh hands the value straight to goose with a
single-pass sh expansion (export GOOSE_DBSTRING="$CLICKHOUSE_URL"), so
the inner ${...} reaches goose as literal text and breaks the
ClickHouse migration:
goose run: parse "http://default:${CLICKHOUSE_PASSWORD}@host:8123?secure=false":
net/url: invalid userinfo
Switch to Kubernetes' $(VAR) syntax in both clickhouse URL helpers.
Kubelet substitutes $(CLICKHOUSE_PASSWORD) at container-creation time
from the CLICKHOUSE_PASSWORD env var the chart already sets just before
CLICKHOUSE_URL, so the URL arrives at the entrypoint with the real
password already inlined — no entrypoint change needed, works for any
container image / shell.
The plain-password branch (no existingSecret) is unchanged.
Operator caveat: CLICKHOUSE_PASSWORD must be URL-userinfo-safe because
kubelet substitutes verbatim without percent-encoding. Hex-encoded
passwords (e.g. openssl rand -hex 32) are safe by construction.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis change modifies Helm template helper functions in Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Thanks for your contribution! We require all external PRs to be opened in draft status first so you can address CodeRabbit review comments and ensure CI passes before requesting a review. Please re-open this PR as a draft. See CONTRIBUTING.md for details. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
@ThullyoCunha make sure to open the PR in draft status or it gets auto-closed |
|
Thanks @ericallam — my bad, missed the draft requirement. Reopened as a draft in #3449 (same patch). |
2 participants
Summary
When the official Helm chart is deployed with an external ClickHouse and
clickhouse.external.existingSecretset — the documented path for not committing secrets tovalues.yaml— the webapp pod crash-loops on startup:Context in vouch request #3443.
Root cause
Two pieces interact:
hosting/k8s/helm/templates/_helpers.tplrendersCLICKHOUSE_URL(andRUN_REPLICATION_CLICKHOUSE_URL) with a shell-style literal${CLICKHOUSE_PASSWORD}expecting bash expansion at container start.docker/scripts/entrypoint.shdoesexport 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) andmain.Fix
Switch the two helpers (external +
existingSecretbranch) from shell-style${CLICKHOUSE_PASSWORD}to Kubernetes'$(CLICKHOUSE_PASSWORD). Kubelet substitutes$(VAR)at pod-creation time from earlier env entries, and the chart already declaresCLICKHOUSE_PASSWORDfrom the Secret immediately beforeCLICKHOUSE_URL, so the URL reaches the entrypoint with the real password already inlined. No entrypoint change, no image change. The plain-password branch (noexistingSecret) is unchanged.Operator caveat added as template comments:
CLICKHOUSE_PASSWORDmust 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 templateagainstexternal.existingSecretnow rendersvalue: "http://default:$(CLICKHOUSE_PASSWORD)@<host>:8123?secure=false"(was${CLICKHOUSE_PASSWORD}).helm templateagainst the plain-password branch is byte-identical to before.goose: successfully migrated database to version: 6, Node.js ClickHouse client connects at runtime.Alternatives considered
entrypoint.shtoeval/envsubstthe URL — larger surface, touches every deployment mode (Docker Compose + k8s) and every container image.valueFrom.secretKeyRef, as intrigger-v4.postgres.useSecretUrl) — cleaner long-term but requires a newvalues.yamlfield 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.Closes the issue raised in #3443.