fix(helm): render the chart Secret for every inline credential (#948)

* fix(helm): render the chart Secret for every inline credential

Setting llm.existingSecret suppressed the chart's whole Secret, not just
the LLM API key it replaces. POSTGRES_PASSWORD and TURNSTONE_JWT_SECRET
went unrendered with it while server, console and the migrate Job went on
referencing them, so every pod stalled in CreateContainerConfigError.
Supplying an LLM Secret is a supported, documented configuration, and it
took the install down on both the bundled and external database paths.

turnstone.db.secretName compounded it by falling back to
turnstone.llm.secretName, pointing the password lookup at the operator's
LLM Secret — which has no reason to carry a database password.

Both now derive from one predicate. turnstone.db.inlinePassword returns
the password when the chart stores it itself and empty when an operator
supplies it, so secret.yaml renders on exactly the condition under which
turnstone.db.secretName resolves to <fullname>-secrets. The two cannot
disagree about where the password lives, which is what the earlier
llm.secretName fallback was working around. Each key keeps its own
condition, so an existingSecret still suppresses the value it replaces
and nothing else.

Verified by rendering nine values permutations against both this and the
previous templates and diffing every secretKeyRef against the Secrets
each tree creates: three permutations fixed, six byte-identical, none
regressed. helm lint passes on all nine.

The bundled-PostgreSQL default is unaffected and still broken: the
subchart generates its password into <fullname>-postgresql, which the
chart never reads. It is separately blocked by the migrate hook running
before the database exists, so it needs the design decision called for
in #932 rather than a secret-name change.

* fix(helm): default the inline password so an unset key cannot become one

turnstone.db.inlinePassword is reached through include, which captures
rendered text rather than a value. A key that is unset rather than empty
— "password:" with nothing after it, or --set database.external.password=null
— renders as the literal "<no value>", and a ten-character string is
truthy, so it satisfied the gate in templates/secret.yaml and landed
base64-encoded in POSTGRES_PASSWORD. Workloads then authenticated with
the string "<no value>".

Reaching the values through default "" keeps unset and empty equivalent,
which is what the previous templates got for free by testing the value
directly instead of the rendered text. Introduced by the commit before
this one; caught in review.

The two null spellings are now permanent cases in the render matrix.
Across eleven permutations, three are fixed relative to main, eight are
byte-identical, none regress, and the inline password still round-trips
byte-exact. helm lint passes on all eleven.

* docs(helm): narrow the inlinePassword guarantee to what it holds

The comment claimed secret.yaml and turnstone.db.secretName cannot
disagree about where the password lives. That holds wherever the chart
or the operator supplies the password, but not where the bundled
subchart generates its own — that lands in the subchart's Secret, which
neither helper reads. State the two guarantees that do hold instead.
This commit is contained in:
Patrick Buckley
2026-08-02 14:42:33 -07:00
committed by GitHub
parent f8f2ba03d3
commit 989f51edc5
2 changed files with 52 additions and 13 deletions
+32 -6
View File
@@ -110,6 +110,31 @@ Determine the PostgreSQL username.
{{- end }}
{{- end }}
{{/*
The PostgreSQL password when the chart stores it itself, empty when it
does not. Doubles as the predicate for "does <fullname>-secrets need to
carry POSTGRES_PASSWORD", so an inline password is never written
anywhere but <fullname>-secrets, and an operator-supplied Secret is
never duplicated into it.
An operator-supplied existingSecret wins outright: writing the value
into a second Secret nothing reads would only duplicate a credential.
Both branches need "default" because this is reached through include,
which captures rendered text rather than a value: a key that is unset
rather than empty "password:" with nothing after it renders as the
literal "<no value>", and a ten-character string is truthy. Without the
default that lands base64-encoded in POSTGRES_PASSWORD and the workloads
authenticate with it.
*/}}
{{- define "turnstone.db.inlinePassword" -}}
{{- if .Values.postgresql.enabled }}
{{- .Values.postgresql.auth.password | default "" }}
{{- else if not .Values.database.external.existingSecret }}
{{- .Values.database.external.password | default "" }}
{{- end }}
{{- end }}
{{/*
Determine the secret holding the PostgreSQL password.
@@ -118,17 +143,18 @@ CloudNativePG-generated secret, an External Secrets target, ...), in
which case the key name is rarely "POSTGRES_PASSWORD" hence the
companion existingSecretPasswordKey.
Otherwise fall back to the chart's application secret, which is
llm.existingSecret when the operator supplies one. That fallback must
not be hardcoded to "<fullname>-secrets": templates/secret.yaml is
skipped entirely when llm.existingSecret is set, so hardcoding it would
point every workload at a Secret that is never created.
Otherwise the password is inline in values, and the chart writes it to
its own <fullname>-secrets. Note this is deliberately not
turnstone.llm.secretName: that resolves to llm.existingSecret when the
operator supplies one, which holds LLM API keys and has no reason to
carry a database password. templates/secret.yaml renders on exactly the
condition above, so the two stay in agreement.
*/}}
{{- define "turnstone.db.secretName" -}}
{{- if and (not .Values.postgresql.enabled) .Values.database.external.existingSecret }}
{{- .Values.database.external.existingSecret }}
{{- else }}
{{- include "turnstone.llm.secretName" . }}
{{- printf "%s-secrets" (include "turnstone.fullname" .) }}
{{- end }}
{{- end }}
+20 -7
View File
@@ -1,4 +1,19 @@
{{- if not .Values.llm.existingSecret }}
{{/*
This Secret backs every credential supplied inline in values, so it is
rendered whenever any one of them is set — not, as it once was, only
when llm.existingSecret is empty. Under that older gate an operator who
supplied an LLM Secret lost the unrelated inline values with it: both
POSTGRES_PASSWORD and TURNSTONE_JWT_SECRET silently went unrendered
while the workloads went on referencing them, so every pod stalled in
CreateContainerConfigError.
Each key keeps its own condition, so an operator-supplied Secret still
suppresses the value it replaces and nothing else.
*/}}
{{- $apiKey := and .Values.llm.apiKey (not .Values.llm.existingSecret) }}
{{- $dbPassword := include "turnstone.db.inlinePassword" . }}
{{- $jwtSecret := and .Values.auth.jwtSecret (not .Values.auth.existingSecret) }}
{{- if or $apiKey $dbPassword $jwtSecret }}
apiVersion: v1
kind: Secret
metadata:
@@ -16,15 +31,13 @@ metadata:
"helm.sh/hook-delete-policy": before-hook-creation
type: Opaque
data:
{{- if .Values.llm.apiKey }}
{{- if $apiKey }}
OPENAI_API_KEY: {{ .Values.llm.apiKey | b64enc | quote }}
{{- end }}
{{- if and .Values.postgresql.enabled .Values.postgresql.auth.password }}
POSTGRES_PASSWORD: {{ .Values.postgresql.auth.password | b64enc | quote }}
{{- else if and (not .Values.postgresql.enabled) .Values.database.external.password }}
POSTGRES_PASSWORD: {{ .Values.database.external.password | b64enc | quote }}
{{- if $dbPassword }}
POSTGRES_PASSWORD: {{ $dbPassword | b64enc | quote }}
{{- end }}
{{- if and .Values.auth.jwtSecret (not .Values.auth.existingSecret) }}
{{- if $jwtSecret }}
TURNSTONE_JWT_SECRET: {{ .Values.auth.jwtSecret | b64enc | quote }}
{{- end }}
{{- end }}