fix(relay-proxy): Allow overide of array configuration by environment variables #1902
Conversation
✅ Deploy Preview for go-feature-flag-doc-preview canceled.
|
luanxuechao
force-pushed
the
luanx/issue-1841
branch
from
eecab94
to
19b55a0
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1902 +/- ##
==========================================
+ Coverage 86.81% 86.99% +0.18%
==========================================
Files 96 96
Lines 3450 3498 +48
==========================================
+ Hits 2995 3043 +48
Misses 350 350
Partials 105 105 ☔ View full report in Codecov by Sentry. |
luanxuechao
force-pushed
the
luanx/issue-1841
branch
2 times, most recently
from
3dfd84a
to
769d6c6
Compare
luanxuechao
force-pushed
the
luanx/issue-1841
branch
2 times, most recently
from
b1d1ef5
to
11dd493
Compare
thomaspoignant
changed the title
There was a problem hiding this comment.
Hey @luanxuechao thanks a lot for this pull request and this looks super promising.
I've made some small comments, but the main thing I see is that we cannot add a new entry to the array.
PS: I have changed the title of the PR to make it more explicit.
luanxuechao
force-pushed
the
luanx/issue-1841
branch
from
48f5540
to
6bf3c1a
Compare
There was a problem hiding this comment.
This version is now accepting env variables outside of the range BUT it does not take into consideration multiple variables in the same object.
Something like
RETRIEVERS_1_KIND=github
RETRIEVERS_1_REPOSITORYSLUG=thomaspoignant/go-feature-flag
RETRIEVERS_1_BRANCH=main
RETRIEVERS_1_FILEPATH=testdata/flag-config.yaml
With the actual version of the code, it will only take the last env variable.
When we move the location of the configMap initialization it will work.
I'll suggest that you double-check and that it works, and it is worth adding a test to validate it.
cmd/relayproxy/config/config.go
Outdated
| @@ -91,10 +92,15 @@ func New(flagSet *pflag.FlagSet, log *zap.Logger, version string) (*Config, erro | |||
| log.Error("error loading file", zap.Error(errBindFile)) | |||
| } | |||
| } | |||
|
|
|||
| configMap := k.Raw() | |||
There was a problem hiding this comment.
If we keep this outside of the function we are not able to pass multiple environment variables.
If we have something like
RETRIEVERS_1_KIND=github
RETRIEVERS_1_REPOSITORYSLUG=thomaspoignant/go-feature-flag
RETRIEVERS_1_BRANCH=main
RETRIEVERS_1_FILEPATH=testdata/flag-config.yaml
Only the last environment variable will be taken into consideration.
(check proposal bellow to make it work)
| configMap := k.Raw() |
luanxuechao
force-pushed
the
luanx/issue-1841
branch
from
3785b37
to
4213d3c
Compare
luanxuechao
force-pushed
the
luanx/issue-1841
branch
from
4213d3c
to
f23509d
Compare
|
There was a problem hiding this comment.
Thanks @luanxuechao 🚀
It will be part of the next GO Feature Flag release.
… variables (#1902) * fix: Load array env * Fix comment issue * Fix comment issue * Fix comment issue
Description
Closes issue(s)
Resolve 1841
Checklist
README.mdand/website/docs)