litellm
litellm copied to clipboard
chore(helm-chart): use default environment variable for master key
Sometimes these manifests are used as a reference to some documentation. Idea is to use a the default environment variable instead of something specific and different baked into just the chart.
It may be that somebody doesn't want to put key in values file. They can set the key something like
master_key: os.environ/MY_KEY
Then have a secret referenced with the key
type: Opaque
data:
MY_KEY: xxxxxx
The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Comments | Updated (UTC) |
|---|---|---|---|---|
| litellm | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | Apr 14, 2024 5:23pm |
| litellm-dashboard | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | Apr 14, 2024 5:23pm |
This makes sense. Or even just bypass the file as an option. Some logic like:
- is the master key present in the file as env var reference or value?
- The env var "MASTER_KEY" is set?
Otherwise fallback to disable master key security.
good to merge @debdutdeb @GuillermoBlasco ?
@GuillermoBlasco you're right .. we should put an {{- if .masterKey }} before.
So if there is the value in Values file, we set LITELLM_MASTER_KEY.
If it's in config like os.environ/SOMETHING_ELSE , in theory user would need to add the secret mnaully anyway since how'd we know what the value is? whether it's a literal or a valueFrom.
And if somebody put it inside the config as a literal, well, if Im not mistaken you're saying we do something like
{{- range lines -}}
{{- if contains . "master_key" -}}
{{- $s := split . ": " -}}
{{- $value := index $s 1 -}}
{{- if contains $value "os.environ" | not -}}
{{/* use $value for a secret then mount, maybe delete the line from config? */}}
...
If that's the case, IMO we let them do it as is? 🙈 this is the kind of helm magic that in my experience creates more pain when multiple team members are considered, and honestly there are better ways to handle it within chart wth envFrom, and whomever deploying should take care of that themselves?
IDK if you folks think we should do it that way, I can add those couple lines in :)
Sorry @ishaan-jaff did not receive the notification first time. Please let's wait for Gui's input.
what're next steps on this? @debdutdeb @GuillermoBlasco @ishaan-jaff
@ishaan-jaff this should be ready to merge :)
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.