-
Notifications
You must be signed in to change notification settings - Fork 124
Fix inconsistency between postgress password key for langfuse and postgress. #142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@JSchuurmans Thank you for the contribution here! Could you sign the CLA? |
|
@JSchuurmans And are you sure that this is backwards compatible to existing installations? How would that behavior change for someone who already has the chart installed? |
|
@Steffen911 good question. I am curious how someone was able to install the Langfuse server with this Helm chart and the default settings. Using the following settings for the postgress sub chart: postgresql:
auth:
existingSecret: langfuse-postgresql
secretKeys:
userPasswordKey: passwordThe subchart of postgres modifies the secret key, by pre-pending the user name to the key. By default the username is postgres so it prepends with postgres-. If I use a secret with key I used two patches for the langfuse-web and langfuse-worker pods, e.g.: spec:
template:
spec:
containers:
- name: langfuse-worker
env:
- name: DATABASE_PASSWORD
valueFrom:
secretKeyRef:
key: postgres-password
name: langfuse-postgresqlI think the PR is a more sustainable approach than this patch, because it aligns langfuse with its usage of the postgres subchart. I hope this make the fix and its implications more clear. |
|
Hi @Steffen911, did my response answer your question and does the proposed solution pass your tests? |
|
@JSchuurmans I attempted to run this locally again and fail to reproduce the problem you report. I'm installing the latest Langfuse chart using the If I leave the postgresql.auth.username blank (i.e. let it default to Can you share screenshots, error logs or yaml files from the containers that represent the unexpected state from your end and perhaps any errors you observe? It would also be helpful if you can share which versions of the sub-charts you're using. As it stands, I cannot reproduce this problem. |
|
@Steffen911 Thank you for referring to the minimal installation example. I can highly recommend that people read it. I noticed that I only set the It is still unclear how "not providing the adminPasswordKey" can lead to a valid deployment, but only if patches or a fix like this PR are applied. I can imagine it is undesired to support this workflow, but I will leave that up to you @Steffen911. Again, thank you! |
Fixes #141
Fix
Use the same naming convention for DATABASE_PASSWORD used in
langfuse-webandlangfuse-workerpods as subcharts of postgress uses.Before
The subchart of postgress modifies the secret key, by pre-pending the user name to the key. By default
postgress-.This results in a mismatch with the environment variables used in the
langfuse-webandlangfuse-workerpods.After
Environment variable
DATABASE_PASSWORDnow also prepends db username:<username>-<userPasswordKey>.Important
Fixes
DATABASE_PASSWORDnaming inconsistency by prepending db username to key inlangfuse-webandlangfuse-workerpods.DATABASE_PASSWORDenvironment variable naming inlangfuse-webandlangfuse-workerpods.<username>-<userPasswordKey>.langfuse.getEnvVarin_helpers.tplto reflect new key format.This description was created by
for d424c4e. You can customize this summary. It will automatically update as commits are pushed.