feat(configuration): Allow environment variable overrides for config

[lucasreed]

Description

The goal is to support environment variable configuration that overrides the config file at runtime.

The helper bindEnvsFromConfig exists to work around a viper limitation. viper.AutomaticEnv() does not make env vars visible to viper.Unmarshal() if they aren't in the loaded config yaml. For any key that lives only in your struct and isn't present in the loaded YAML, AutomaticEnv silently does nothing on unmarshal, so you have to explicitly BindEnv every key. The helper flattens the default config and does BindEnv each key.

Reference:

• Consider env vars when unmarshalling spf13/viper#188
• Unmarshal non-bound environment variables spf13/viper#761

Related Issue

#1108 and #3165 are marked as completed, but the only env var I've seen is to change the location of the config file.

Motivation and Context

The main motivation here is not storing secrets plainly in a configmap or helm values file in order to run dragonfly with an external database (not using the bundled mysql and redis helm charts)

Screenshots (if appropriate)

N/A

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation Update (if none of the other choices apply)

Checklist

• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.

I updated the docker-compose README, but It looks like the documentation on the website is in a different repo. I'm happy to open another PR there if this is approved/merged/released.

[codecov]

Codecov Report

❌ Patch coverage is 64.28571% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 28.14%. Comparing base (063fcb4) to head (422b4d6).
⚠️ Report is 29 commits behind head on main.

Files with missing lines	Patch %	Lines
cmd/dependency/dependency.go	64.28%	6 Missing and 4 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4818      +/-   ##
==========================================
+ Coverage   28.09%   28.14%   +0.05%     
==========================================
  Files         232      232              
  Lines       23141    23181      +40     
==========================================
+ Hits         6501     6525      +24     
- Misses      16200    16212      +12     
- Partials      440      444       +4     

Flag	Coverage Δ
unittests	28.14% <64.28%> (+0.05%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
cmd/dependency/dependency.go	12.29% <64.28%> (+12.29%)	⬆️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[pmady]

nice approach, the viper AutomaticEnv + Unmarshal gap is a well-known pain point and this workaround is clean.

one thing to watch bindEnvsFromConfig gets the config struct before initConfig reads the YAML, so it's working off zero values. nested fields without defaults won't show up in the marshalled YAML, meaning those env vars silently won't bind.

also didn't see tests in the diff even a small one that sets an env var and asserts the override lands in the final config would be solid.

docs addition is great, especially the camelCase mapping table.

[lucasreed]

nice approach, the viper AutomaticEnv + Unmarshal gap is a well-known pain point and this workaround is clean.

one thing to watch bindEnvsFromConfig gets the config struct before initConfig reads the YAML, so it's working off zero values. nested fields without defaults won't show up in the marshalled YAML, meaning those env vars silently won't bind.

also didn't see tests in the diff even a small one that sets an env var and asserts the override lands in the final config would be solid.

docs addition is great, especially the camelCase mapping table.

Ah great call out thank you! Just updated and include a couple tests. Let me know what you think!

[pmady]

nice, pulled and ran the tests locally — all 4 pass. materializeStructPtrs is a solid fix for the nil pointer case, and testing against the real scheduler config is a good call. lgtm 👍

[lucasreed]

@gaius-qi Let me know if there is anything else I need to do for this PR. Would love to make use of this change soon if possible :)