Skip to content
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

Support envFrom as way to configure Pod #198

Closed
StianOvrevage opened this issue Apr 17, 2024 · 2 comments · Fixed by #201
Closed

Support envFrom as way to configure Pod #198

StianOvrevage opened this issue Apr 17, 2024 · 2 comments · Fixed by #201

Comments

@StianOvrevage
Copy link
Contributor

It seems the helm chart mixes the use of "args" and "envs" for configuring the container.

One example use case (which I guess is common) is to have an org-wide set of sensible defaults in a values.yaml file, and then override or supplement on an app-by-app or env-by-env basis.

Things like clientId and clientSecret are kind of handled explicitly so they support this easily.

But we have specific settings such as --oidc-issuer-url= that can vary. The problem with the charts extraArgs and extraEnv is they cannot easily be created from multiple sources (values files or use of --set). They both need to be configured in their entirety in one (or every) place. Some more information here: https://helm.sh/docs/chart_best_practices/values/#consider-how-users-will-use-your-values

The "best" way to support this would be to explicitly support every argument to oauth2-proxy in the helm chart. But that is probably too much work and maintenance overhead to be feasible.

Therefore I propose adding support from reading in env vars from a ConfigMap via envFrom. That would give us another tool/angle to work with. For example org-wide configuration could be stored in a ConfigMap that is loaded via envFrom, and then individual deployments are free to "own" and set all necessary configuration with extraEnv without deeper knowledge of the inherited org-wide config that is stored separately in one (or more) ConfigMaps.

@pierluigilenoci
Copy link
Contributor

Feel free to implement this new feature in a PR.
I will be happy to review it as soon as possible.

@StianOvrevage
Copy link
Contributor Author

I updated Chart.yaml with version bump and artifacthub.io/changes. Note that this PR has version 7.4.3 and the previous PR (#200) I set to 7.4.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants