mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-09 03:18:44 +00:00
fix(helm): stop mangling the bundled postgres password in the migrations Job
With db.deployStandalone the migrations Job built DATABASE_URL by interpolating postgresql.auth.password straight into the string. Any of : / ? # [ ] @ + in the password reshapes the URL, and the Job connects somewhere else or not at all: --set postgresql.auth.password='p@ss/w+rd=' -> postgresql://litellm:p@ss/w+rd=@rel-postgresql/litellm which urlparse reads as password "p" on host "ss". The real host and database are gone, so the Job fails with prisma P1013 and the release never gets a migrated schema. Reported in #24331, closed by the stale bot with the bug still present. Hand the four components over instead and let the entrypoint assemble the URL, which is what the proxy Deployment already does for this same case: construct_database_url_from_env_vars() percent-encodes username, password and database name before joining them. The Job and the Deployment now take one code path rather than two, so they cannot disagree about what the bundled database is called or how a password is escaped. Sourcing username and password from the dbcredentials Secret the chart already creates also keeps the password out of the rendered Job, where it was previously readable through `kubectl get job -o yaml` and in the Helm release secret. Adds three tests: the Job exposes the components and no DATABASE_URL, a password holding URL-special characters never reaches the rendered Job, and DATABASE_NAME follows postgresql.auth.database. Verified by mutation: restoring the old hand-built URL fails two of them, hardcoding the database name fails the third. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
4ece6c9fb8
commit
3db5ba01a1
2 changed files with 104 additions and 2 deletions
|
|
@ -75,8 +75,28 @@ spec:
|
|||
- name: DATABASE_URL
|
||||
value: {{ .Values.db.url | quote }}
|
||||
{{- else if .Values.db.deployStandalone }}
|
||||
- name: DATABASE_URL
|
||||
value: postgresql://{{ .Values.postgresql.auth.username }}:{{ .Values.postgresql.auth.password }}@{{ .Release.Name }}-postgresql/{{ .Values.postgresql.auth.database }}
|
||||
{{- /*
|
||||
Hand the credentials over as separate variables and let the entrypoint
|
||||
assemble DATABASE_URL, exactly as the proxy Deployment does. Building the
|
||||
URL here instead interpolated the password raw, so any of : / ? # [ ] @ +
|
||||
or a space in postgresql.auth.password silently reshaped the URL and the
|
||||
Job failed to connect. Sourcing the two secret values also keeps the
|
||||
password out of the rendered Job and the Helm release secret.
|
||||
*/}}
|
||||
- name: DATABASE_USERNAME
|
||||
valueFrom:
|
||||
secretKeyRef:
|
||||
name: {{ include "litellm.fullname" . }}-dbcredentials
|
||||
key: username
|
||||
- name: DATABASE_PASSWORD
|
||||
valueFrom:
|
||||
secretKeyRef:
|
||||
name: {{ include "litellm.fullname" . }}-dbcredentials
|
||||
key: password
|
||||
- name: DATABASE_HOST
|
||||
value: {{ .Release.Name }}-postgresql
|
||||
- name: DATABASE_NAME
|
||||
value: {{ .Values.postgresql.auth.database }}
|
||||
{{- end }}
|
||||
{{- if .Values.envVars }}
|
||||
{{- range $key, $val := .Values.envVars }}
|
||||
|
|
|
|||
|
|
@ -130,6 +130,88 @@ tests:
|
|||
name: DISABLE_SCHEMA_UPDATE
|
||||
value: "false"
|
||||
|
||||
- it: should hand deployStandalone credentials over as separate env vars, not a built URL
|
||||
template: migrations-job.yaml
|
||||
set:
|
||||
migrationJob:
|
||||
enabled: true
|
||||
db:
|
||||
deployStandalone: true
|
||||
useExisting: false
|
||||
asserts:
|
||||
# Assembling the URL in the template interpolated the password raw, so a
|
||||
# password holding any of : / ? # [ ] @ + reshaped the URL and the Job
|
||||
# connected to the wrong host, or to none. The entrypoint builds the URL
|
||||
# from these four instead, percent-encoding as it goes, which is what the
|
||||
# proxy Deployment already relies on for this same case.
|
||||
- notContains:
|
||||
path: spec.template.spec.containers[0].env
|
||||
content:
|
||||
name: DATABASE_URL
|
||||
any: true
|
||||
- contains:
|
||||
path: spec.template.spec.containers[0].env
|
||||
content:
|
||||
name: DATABASE_USERNAME
|
||||
valueFrom:
|
||||
secretKeyRef:
|
||||
name: RELEASE-NAME-litellm-dbcredentials
|
||||
key: username
|
||||
- contains:
|
||||
path: spec.template.spec.containers[0].env
|
||||
content:
|
||||
name: DATABASE_PASSWORD
|
||||
valueFrom:
|
||||
secretKeyRef:
|
||||
name: RELEASE-NAME-litellm-dbcredentials
|
||||
key: password
|
||||
- contains:
|
||||
path: spec.template.spec.containers[0].env
|
||||
content:
|
||||
name: DATABASE_HOST
|
||||
value: RELEASE-NAME-postgresql
|
||||
- contains:
|
||||
path: spec.template.spec.containers[0].env
|
||||
content:
|
||||
name: DATABASE_NAME
|
||||
value: litellm
|
||||
|
||||
- it: should keep a password holding URL-special characters out of the rendered Job
|
||||
template: migrations-job.yaml
|
||||
set:
|
||||
migrationJob:
|
||||
enabled: true
|
||||
db:
|
||||
deployStandalone: true
|
||||
useExisting: false
|
||||
postgresql:
|
||||
auth:
|
||||
password: "p@ss/w+rd="
|
||||
asserts:
|
||||
# The password now only ever travels through the dbcredentials Secret, so
|
||||
# it is neither mangled into a URL nor readable in `kubectl get job -o yaml`
|
||||
# and the Helm release secret.
|
||||
- notMatchRegexRaw:
|
||||
pattern: "p@ss/w\\+rd="
|
||||
|
||||
- it: should follow postgresql.auth.database for the standalone database name
|
||||
template: migrations-job.yaml
|
||||
set:
|
||||
migrationJob:
|
||||
enabled: true
|
||||
db:
|
||||
deployStandalone: true
|
||||
useExisting: false
|
||||
postgresql:
|
||||
auth:
|
||||
database: custom-db
|
||||
asserts:
|
||||
- contains:
|
||||
path: spec.template.spec.containers[0].env
|
||||
content:
|
||||
name: DATABASE_NAME
|
||||
value: custom-db
|
||||
|
||||
- it: should not include DATABASE_URL when deployStandalone is false
|
||||
template: migrations-job.yaml
|
||||
set:
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue