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

fix: Allow hooks to be specified in workflowDefaults #11214

Merged
merged 1 commit into from
Jun 21, 2023

Conversation

Joibel
Copy link
Member

@Joibel Joibel commented Jun 14, 2023

Any hook in workflowDefaults will cause Error: merging object in json but data type is not struct, instead is: map.

This is down to how StrategicMergePatch handles maps of objects and limitations in merging them (a starting point for understading this is golang/go#33487).

Instead, we copy the map of hooks, patch it out of the patch, perform the merge as before and then manually apply it.

Fixes: #11095

Verification

There is a new test that covers the merging. Verified by changing workflowDefaults to add a hook and checking that I could run the resulting workflow.

Any hook in workflowDefaults will cause `Error: merging object in json
but data type is not struct, instead is: map`.

This is down to how StrategicMergePatch handles maps of objects and
limitations in merging them (a starting point for understading this is
golang/go#33487).

Instead, we copy the map of hooks, patch it out of the patch, perform
the merge as before and then manually apply it.

fixes: argoproj#11095

Signed-off-by: Alan Clucas <alan@clucas.org>
@Joibel Joibel marked this pull request as ready for review June 15, 2023 07:12
@terrytangyuan terrytangyuan merged commit 0ad4169 into argoproj:master Jun 21, 2023
tico24 pushed a commit to tico24/argo-workflows that referenced this pull request Jun 22, 2023
Signed-off-by: Tim Collins <tim@thecollins.team>
JPZ13 pushed a commit to pipekit/argo-workflows that referenced this pull request Jul 4, 2023
jeremyhager pushed a commit to jeremyhager/argo-workflows that referenced this pull request Jul 7, 2023
Signed-off-by: Jeremy Hager <47301461+jeremyhager@users.noreply.github.com>
@Joibel Joibel deleted the defaultHook branch January 15, 2024 15:27
dpadhiar pushed a commit to dpadhiar/argo-workflows that referenced this pull request May 9, 2024
Signed-off-by: Dillen Padhiar <dillen_padhiar@intuit.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lifecycle hooks are broken when applying via ConfigMap
4 participants