feat: add support for parameters when running actions from the CLI (#28000)#28004
feat: add support for parameters when running actions from the CLI (#28000)#28004olavst-spk wants to merge 7 commits into
Conversation
🔴 Preview Environment stopped on BunnyshellSee: Environment Details | Pipeline Logs Available commands (reply to this comment):
|
olavst-spk
force-pushed
the
cli-action-parameters
branch
4 times, most recently
from
f6e47d7 to
444f7c6
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #28004 +/- ##
=========================================
Coverage ? 64.60%
=========================================
Files ? 424
Lines ? 58248
Branches ? 0
=========================================
Hits ? 37634
Misses ? 17097
Partials ? 3517 ☔ View full report in Codecov by Sentry. |
olavst-spk
force-pushed
the
cli-action-parameters
branch
2 times, most recently
from
e25b87a to
0882d55
Compare
olavst-spk
force-pushed
the
cli-action-parameters
branch
2 times, most recently
from
4833cdf to
f30e92b
Compare
| return nil, fmt.Errorf("invalid parameter format %q: parameter name cannot be empty", param) | ||
| } | ||
| if seen[name] { | ||
| return nil, fmt.Errorf("parameter %q is specified more than once", name) |
There was a problem hiding this comment.
This means duplicate parameter names are not supported after this change. Can you explain the reason for this? AFAIK we support duplicate parameter names where the last value would win at action runtime.
There was a problem hiding this comment.
I thought it would be better to return an error rather than silently choosing the last value since that could be confusing to users, but maybe I am missing something? Is there a reason to support setting the same parameter multiple times? What is the use-case for that?
There was a problem hiding this comment.
I agree with @nitishfy and it was I originally suggested
Also there are several cases in the code base that are using the Last-write-wins pattern so let's stick on it
Also we need to document it properly
There was a problem hiding this comment.
Okay, I have updated the code to print a warning instead of exiting with an error.
Do we need to do the documentation update in this PR?
Fixes argoproj#28000 Adds a `--param key=value` flag to `argocd app actions run`. The parameter parsing logic that already existed in `argocd admin settings` is moved into a shared ParseActionParameters helper in cmd/util and reused by both commands. Also fixes the Lua scale actions to separately handle both the "missing parameter" and "invalid value" cases, using the same logic as the existing scale action for keda.sh/ScaledObject Signed-off-by: Olav Thoresen <Olav.Sortland.Thoresen@spk.no>
Co-authored-by: Papapetrou Patroklos <1743100+ppapapetrou76@users.noreply.github.com> Signed-off-by: Olav Sortland Thoresen <137788666+olavst-spk@users.noreply.github.com>
Signed-off-by: Olav Thoresen <Olav.Sortland.Thoresen@spk.no>
Signed-off-by: Olav Thoresen <Olav.Sortland.Thoresen@spk.no>
Signed-off-by: Olav Thoresen <Olav.Sortland.Thoresen@spk.no>
This reverts commit cc43915. Signed-off-by: Olav Thoresen <Olav.Sortland.Thoresen@spk.no>
Signed-off-by: Olav Thoresen <Olav.Sortland.Thoresen@spk.no>
olavst-spk
force-pushed
the
cli-action-parameters
branch
from
114b91a to
2a442fb
Compare
3 participants
Fixes #28000
Adds a
--param key=valueflag toargocd app actions run. The parameter parsing logic that already existed inargocd admin settingsis moved into a shared ParseActionParameters helper in cmd/util and reused by both commands.Also fixes the Lua scale actions to separately handle both the "missing parameter" and "invalid value" cases, using the same logic as the existing scale action for keda.sh/ScaledObject
Checklist: