argoproj / argoproj/argo-workflows

MergeTo inverts Patch and Target when passing workflows into StrategicMergePatch

Open
#14,554 5 comments 0 reactions 0 assignees View on GitHub
area/workflow-templates
Dominant language
Go
Stars
17k
Forks
3.7k
Avg merge
1d 20h
Merged PRs (30d)
138

Description

### Pre-requisites

- [x] I have double-checked my configuration
- [x] I have tested with the `:latest` image tag (i.e. `quay.io/argoproj/workflow-controller:latest`) and can confirm the issue still exists on `:latest`. If not, I have explained why, **in detail**, in my description below.
- [x] I have searched existing issues and could not find a match for this bug
- [x] I'd like to contribute the fix myself (see [contributing guide](https://github.com/argoproj/argo-workflows/blob/main/docs/CONTRIBUTING.md))

### What happened? What did you expect to happen?

I have two yaml files:
1.) A kind of Workflow that has a reference template
2.) A kind of WorkflowTemplate that is to be referenced.

I load these two yaml files into respective objects via `common.ParseObjects(bytes, false)` the bytes being the loaded file.

then switch on `*v1alpha1.WorkflowTemplate` and `*v1alpha1.Workflow`.

I then use these objects to merge the template into the workflow if it is detected as a reference in the workflow.

I do not provide a default workflow to the JoinWorkflowSpec function call to create this behavior see repro.

I would expect it would merge them and keep the `Workflows` spec.entrypoint.

### Version(s)

v3.6.4

### Paste a minimal workflow that reproduces the issue. We must be able to run the workflow; don't enter a workflow that uses private images.

The base workflow
```YAML
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
name: workflow-with-templateref
annotations:
transformer.rsi/Test: WorkflowTest
labels:
transformer.rsi/Test: WorkflowTest
spec:
entrypoint: top-level-template
serviceAccountName: argo-workflow
podGC:
strategy: OnPodCompletion
archiveLogs: true
templates:
- name: top-level-template
steps:
- - name: do-echo-reference
templateRef:
name: do-echo
template: do-echo
arguments:
parameters:
- name: test-parameter
value: "simple-template was called from {{workflow.name}}"
```
This references the following WorkflowTemplate
```
apiVersion: argoproj.io/v1alpha1
kind: WorkflowTemplate
metadata:
name: do-echo
annotations:
transformer.rsi/Test: TemplateTest
labels:
transformer.rsi/Test: TemplateTest
spec:
templates:
- name: do-echo
nodeSelector:
kubernetes.io/os: linux
tolerations:
- key: kubernetes.io/os
value: linux
operator: "Equal"
effect: NoSchedule
inputs:
parameters:
- name: test-parameter
default: This is the Test parameter
container:
name: test-container
image: ubuntu:22.04
args:
- echo
- "{{inputs.parameters.test-parameter}}"
```

// temp is the workflow, registeredTemplate is the WorkflowTemplate
I am using `util.JoinWorkflowSpec(&temp.Spec, ®isteredTemplate.Spec, nil)` [here](https://github.com/argoproj/argo-workflows/blob/029ce12020430ff4e1d9933cd577f35c8391fcbd/workflow/util/merge.go#L77) Which calls down into MergeTo.

The `util.MergeTo` in the merge.go file [here](https://github.com/argoproj/argo-workflows/blob/029ce12020430ff4e1d9933cd577f35c8391fcbd/workflow/util/merge.go#L14) function takes patch and target in that order. It then passes these values in that same order to the `StrategicMergePatch` function on line 34.

Once I have merged the workflow templates I am calling `validate.ValidateWorkflow(f, nil, &wf, *opts)` the opts used are the default from constructing a new object. This validation fails for missing entry point.

### Logs from the workflow controller

```text
kubectl logs -n argo deploy/workflow-controller | grep ${workflow}
```
No logs as this appears after I merge the object and use the validate.Validate function on the resulting workflow.

### Logs from in your workflow's wait container

```text
kubectl logs -n argo -c wait -l workflows.argoproj.io/workflow=${workflow},workflow.argoproj.io/phase!=Succeeded
```
No logs as this appears after I merge the object and use the validate.Validate function on the resulting workflow.

Contributor guide

Open the contributing guide

Research direction

Start in workflow/util/merge.go at MergeTo and JoinWorkflowSpec, then trace how their arguments reach StrategicMergePatch. Reproduce the provided Workflow and WorkflowTemplate merge and run the resulting object through validate.ValidateWorkflow. Done means the merged workflow retains its spec.entrypoint and validation no longer reports it missing.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, kubernetes
Domain
backend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.