fix: Adjust parameter value overriding. Fixes argoproj#14426

Signed-off-by: Nicola Dardanis <nicdard@gmail.com>

Author: nicdard

workflow/controller/operator_workflow_template_ref_test.go

@@ -216,6 +216,60 @@ func TestWorkflowTemplateRefParamMerge(t *testing.T) {
 	})
 }
 
+var wftWithValueFromParam = `
+apiVersion: argoproj.io/v1alpha1
+kind: WorkflowTemplate
+metadata:
+  name: wf-template-echo
+  namespace: argo
+spec:
+  entrypoint: echo
+  arguments:
+    parameters:
+      - name: message
+        valueFrom:
+          configMapKeyRef:
+            name: config-properties
+            key: message
+  templates:
+    - name: echo
+      container:
+        image: busybox
+        command: [echo]
+        args: ["{{workflow.parameters.message}}"]
+`
+
+var wfWithValueParamOverride = `
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  generateName: wf-parameter-overwrite-
+  namespace: argo
+spec:
+  entrypoint: echo
+  arguments:
+    parameters:
+      - name: message
+        value: "configmap argument overwrite with argument"
+  workflowTemplateRef:
+    name: wf-template-echo
+`
+
+// https://github.com/argoproj/argo-workflows/issues/14426
+func TestWorkflowTemplateRefValueFromParamOverwrite(t *testing.T) {
+	wf := wfv1.MustUnmarshalWorkflow(wfWithValueParamOverride)
+	wftmpl := wfv1.MustUnmarshalWorkflowTemplate(wftWithValueFromParam)
+	t.Run("CheckArgumentFromWFT", func(t *testing.T) {
+		cancel, controller := newController(wf, wftmpl)
+		defer cancel()
+		ctx := context.Background()
+		woc := newWorkflowOperationCtx(wf, controller)
+		woc.operate(ctx)
+		assert.Equal(t, wf.Spec.Arguments.Parameters, woc.wf.Spec.Arguments.Parameters)
+		assert.Equal(t, "configmap argument overwrite with argument", woc.wf.Spec.Arguments.Parameters[0].Value.String())
+	})
+}
+
 var wftWithArtifact = `
 apiVersion: argoproj.io/v1alpha1
 kind: WorkflowTemplate

workflow/util/merge.go

@@ -97,9 +97,36 @@ func JoinWorkflowSpec(wfSpec, wftSpec, wfDefaultSpec *wfv1.WorkflowSpec) (*wfv1.
 	if wfSpec.Suspend != targetWf.Spec.Suspend {
 		targetWf.Spec.Suspend = wfSpec.Suspend
 	}
+	// The value of an argument should respect the priority order of the merge.
+	// However, if Value, ValueFrom and Default are set in different places,
+	// i.e. the Workflow, WorflowTemplate and WorkflowDefault, we should only keep the one with the greatest priority.
+	wfParamsMap := parametersToMapByName(wfSpec)
+	wftParamsMap := parametersToMapByName(wfSpec)
+	for index, parameter := range targetWf.Spec.Arguments.Parameters {
+		if parameter.HasValue() {
+			if param, ok := wfParamsMap[parameter.Name]; ok {
+				targetWf.Spec.Arguments.Parameters[index] = *param.DeepCopy()
+			} else if param, ok := wftParamsMap[parameter.Name]; ok {
+				targetWf.Spec.Arguments.Parameters[index] = *param.DeepCopy()
+			}
+			// If none of the above conditions are met, we can safely assume that the parameter is set from the wfDefaultSpec.
+		}
+	}
 	return &targetWf, nil
 }
 
+func parametersToMapByName(spec *wfv1.WorkflowSpec) map[string]wfv1.Parameter {
+	parameterMap := make(map[string]wfv1.Parameter)
+	if spec != nil {
+		for _, param := range spec.Arguments.Parameters {
+			if param.HasValue() {
+				parameterMap[param.Name] = param
+			}
+		}
+	}
+	return parameterMap
+}
+
 // mergeMetadata will merge the labels and annotations into the target metadata.
 func mergeMetaDataTo(from, to *metav1.ObjectMeta) {
 	if from == nil {

workflow/util/merge_test.go

@@ -267,6 +267,7 @@ spec:
   arguments:
     parameters:
       - name: PARAM1
+        value: WorkflowTemplate value 1 ignored
       - name: PARAM2
       - name: PARAM3
         value: WorkflowTemplate value 3
@@ -386,6 +387,15 @@ func TestJoinWfSpecArguments(t *testing.T) {
 	assert.Equal(result.Spec.Arguments, targetWf.Spec.Arguments)
 }
 
+func TestJoinWfSpecArgumentsWithNil(t *testing.T) {
+	assert := assert.New(t)
+	wf := wfv1.MustUnmarshalWorkflow(wfArguments)
+	result := wfv1.MustUnmarshalWorkflow(wfArguments)
+	targetWf, err := JoinWorkflowSpec(&wf.Spec, nil, nil)
+	require.NoError(t, err)
+	assert.Equal(result.Spec.Arguments, targetWf.Spec.Arguments)
+}
+
 func TestJoinWorkflowMetaData(t *testing.T) {
 	assert := assert.New(t)
 	t.Run("WfDefaultMetaData", func(t *testing.T) {