search

oauth2-proxy / manifests

For hosting manifests to allow for the deployment of OAuth2-Proxy/OAuth2-Proxy
Apache License 2.0
170 stars 153 forks source link

Support `envFrom` as way to configure Pod #198

Closed StianOvrevage closed 6 months ago

StianOvrevage commented 6 months ago

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 commented 6 months ago

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

StianOvrevage commented 6 months ago

I updated Chart.yaml with version bump and artifacthub.io/changes. Note that this PR has version 7.4.3 and the previous PR (https://github.com/oauth2-proxy/manifests/pull/200) I set to 7.4.2.