Add ability to read fence config(s) from env vars #1088
Conversation
Pull Request Test Coverage Report for Build 13452
💛 - Coveralls |
There was a problem hiding this comment.
The logic lgtm, but tagging @Avantol13 in case he has any concerns about this (this is for Helm charts support)
| @@ -49,6 +49,15 @@ def post_process(self): | |||
| for default in defaults: | |||
| self.force_default_if_none(default, default_cfg=default_config) | |||
|
|
|||
| # Read in all environment variables starting with FENCE_ | |||
| for env_var, env_val in os.environ.items(): | |||
There was a problem hiding this comment.
do you think we can revert this then and just use this logic for the DB url?
There was a problem hiding this comment.
How are these getting to env vars for Helm Charts?
https://github.com/uc-cdis/gen3-helm/blob/master/helm/fence/values.yaml#L200-L255
We put a lot of work into the YAML-based configuration support for default values and allowing overrides.
We need a way to be able to mount k8s secrets into specific fields. Env var seems to be the best option, I am all ears for other ideas.
It's not clear to me how this fully replaces that. How does this handle nested configurations like OIDC providers?
It does not replace anything, it's just a supplement, and it does not support nested configs as far as I can tell.
This is to be used for DB password, and the INDEXD_PASSWORD as those are k8s secrets that are potentially created by other charts, and consumed by fence.
There was a problem hiding this comment.
I have also removed the DB specific ones, and I am adding an extra env var in fence helm chart to support the new and old format for backwards compatibility.
|
How are these getting to env vars for Helm Charts? We put a lot of work into the YAML-based configuration support for default values and allowing overrides. It's not clear to me how this fully replaces that. How does this handle nested configurations like OIDC providers? |
New Features
Breaking Changes
Bug Fixes
Improvements
Dependency updates
Deployment changes