fix: config loader accepts multiple parameters with the same key #24938
Conversation
❗ Preview Environment deployment failed on BunnyshellSee: Environment Details | Pipeline Logs Available commands (reply to this comment):
|
| } | ||
| // pkg shellquota doesn't recognize `=` so that the opts in format `foo=bar` could not work. | ||
| // issue ref: https://github.com/argoproj/argo-cd/issues/6822 | ||
| for k, v := range flags { |
There was a problem hiding this comment.
avoid map iteration so that the parsed flag map becomes deterministic
| } | ||
| if len(val) == 0 { | ||
| // emtpy slice means bool flag. | ||
| return "true", true |
There was a problem hiding this comment.
keep returning "true" because some caller expects it
https://github.com/shota3506/argo-cd/blob/95b191d349448b56ccd258ca9e44f41352e0b0af/util/localconfig/localconfig.go#L315-L319
shota3506
force-pushed
the
argocdopts-accepts-multiple-parameters
branch
from
d6ce57c to
b23e575
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #24938 +/- ##
=======================================
Coverage 60.79% 60.80%
=======================================
Files 351 351
Lines 60439 60455 +16
=======================================
+ Hits 36745 36760 +15
- Misses 20772 20773 +1
Partials 2922 2922 ☔ View full report in Codecov by Sentry. |
Signed-off-by: shota3506 <[email protected]>
shota3506
force-pushed
the
argocdopts-accepts-multiple-parameters
branch
from
b23e575 to
1d53395
Compare
util/config/env.go
Outdated
| setKey := func(key string) { | ||
| // pkg shellquota doesn't recognize `=` so that the opts in format `foo=bar` could not work. | ||
| // issue ref: https://github.com/argoproj/argo-cd/issues/6822 | ||
| if strings.Contains(key, "=") { | ||
| kv := strings.SplitN(key, "=", 2) | ||
| actualKey, actualValue := kv[0], kv[1] | ||
| flags[actualKey] = append(flags[actualKey], actualValue) | ||
| } else { | ||
| if _, ok := flags[key]; !ok { | ||
| // empty slice means bool flag. | ||
| flags[key] = []string{} | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
I'd prefer keeping it as a separate function and adding tests for it too.
There was a problem hiding this comment.
@nitishfy I added processKey method and write some tests for it.
Signed-off-by: shota3506 <[email protected]>
shota3506
force-pushed
the
argocdopts-accepts-multiple-parameters
branch
from
41cee0e to
9f8d51e
Compare
2 participants
This PR resolves #24065.
This PR fixes an issue where the
ARGOCD_OPTSenvironment variable config loader could not accept multiple parameters with the same key.Setting ARGOCD_OPTS would be identically to setting the CLI options.
I modified the internal flags storage from
map[string]stringtomap[string][]stringto support multiple values per key.GetStringSliceFlagproperly handle multiple flag occurrences, whileGetFlag,GetIntFlagkeeps returning the last specified value.Checklist: