chore(helm-chart): use default environment variable for master key

[debdutdeb]

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

[vercel]

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

[GuillermoBlasco]

This makes sense. Or even just bypass the file as an option. Some logic like:

1. is the master key present in the file as env var reference or value?
2. The env var "MASTER_KEY" is set?

Otherwise fallback to disable master key security.

[ishaan-jaff]

good to merge @debdutdeb @GuillermoBlasco ?

[debdutdeb]

@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.

[krrishdholakia]

what're next steps on this? @debdutdeb @GuillermoBlasco @ishaan-jaff

[debdutdeb]

@ishaan-jaff this should be ready to merge :)