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

Implement environment variable overrides for config options #15

Open
detjensrobert opened this issue Oct 12, 2024 · 1 comment · May be fixed by #20
Open

Implement environment variable overrides for config options #15

detjensrobert opened this issue Oct 12, 2024 · 1 comment · May be fixed by #20

Comments

@detjensrobert
Copy link
Contributor

detjensrobert commented Oct 12, 2024

Currently all settings are read from config.yaml as is. As documented in the config file design, we want to allow sensitive values to be read from environment variables, instead of needing to be written down and commited in config.yaml.

Unsure if we want this to only be for credential settings, or allow overrides for any setting from an envvar (this might be impossible without restructuring how we parse stuff to allow defaults for all values).

I see two ways to do this:

  • "silent" overrides where the target setting is parsed from the envvar name:
registry:
  build: 
    user:  # --> REGISTRY_BUILD_USER
    pass:  # --> REGISTRY_BUILD_PASS
  cluster:
    user:  # --> REGISTRY_CLUSTER_USER
    pass:  # --> REGISTRY_CLUSTER_USER
  • explicit envvar name set in the config
registry:
  build: 
    user:  
      from_env: REGISTRY_BUILD_USER
    pass: 
      from_env: REGISTRY_BUILD_PASS

The latter might be easier to do with the way Serde wants to parse stuff.

@KekoaM
Copy link

KekoaM commented Oct 14, 2024

I support explicit envvar setting
It makes it clearer what is happening without having to reference the docs for what the silent overrides are. And making Serde happier is a bonus

@detjensrobert detjensrobert linked a pull request Oct 28, 2024 that will close this issue
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