fix: Merge WorkflowTemplateRef with defaults workflow spec

workflow/controller/controller_test.go

@@ -395,7 +395,7 @@ func TestCheckAndInitWorkflowTmplRef(t *testing.T) {
 		wf: wf}
 	_, _, err := woc.loadExecutionSpec()
 	assert.NoError(t, err)
-	assert.Equal(t, &wftmpl.Spec.WorkflowSpec, woc.wfSpec)
+	assert.Equal(t, wftmpl.Spec.WorkflowSpec.Templates, woc.wfSpec.Templates)
 }
 
 func TestIsArchivable(t *testing.T) {

workflow/controller/operator.go

@@ -2829,17 +2829,15 @@ func (woc *wfOperationCtx) loadExecutionSpec() (wfv1.TemplateReferenceHolder, wf
 		}
 	}
 
-	woc.wfSpec = woc.wf.Status.StoredWorkflowSpec
-
-	entrypoint := woc.wf.Spec.Entrypoint
-	if entrypoint == "" {
-		entrypoint = woc.wfSpec.Entrypoint
+	// Set the workflow properties to execution spec.
+	mergedWf, err := wfutil.MergeWorkflows(wfv1.Workflow{Spec: *woc.wf.Status.StoredWorkflowSpec}, wfv1.Workflow{Spec: woc.wf.Spec})
+	if err != nil {
+		return nil, executionParameters, err
 	}
 
+	woc.wfSpec = &mergedWf.Spec
 	woc.volumes = woc.wfSpec.DeepCopy().Volumes
-
-	tmplRef := &wfv1.WorkflowStep{TemplateRef: woc.wf.Spec.WorkflowTemplateRef.ToTemplateRef(entrypoint)}
-
+	tmplRef := &wfv1.WorkflowStep{TemplateRef: woc.wf.Spec.WorkflowTemplateRef.ToTemplateRef(woc.wfSpec.Entrypoint)}
 	if len(woc.wfSpec.Arguments.Parameters) > 0 {
 		executionParameters.Parameters = util.MergeParameters(executionParameters.Parameters, woc.wfSpec.Arguments.Parameters)
 	}

workflow/controller/operator_workflow_template_ref_test.go

@@ -15,7 +15,8 @@ func TestWorkflowTemplateRef(t *testing.T) {
 	defer cancel()
 	woc := newWorkflowOperationCtx(unmarshalWF(wfWithTmplRef), controller)
 	woc.operate()
-	assert.Equal(t, &unmarshalWFTmpl(wfTmpl).Spec.WorkflowSpec, woc.wfSpec)
+	assert.Equal(t, unmarshalWFTmpl(wfTmpl).Spec.WorkflowSpec.Templates, woc.wfSpec.Templates)
+	assert.Equal(t, woc.wf.Spec.Entrypoint, woc.wfSpec.Entrypoint)
 	// verify we copy these values
 	assert.Len(t, woc.volumes, 1, "volumes from workflow template")
 	// and these

workflow/util/util.go

@@ -907,3 +907,29 @@ func GetNodeType(tmpl *wfv1.Template) wfv1.NodeType {
 	}
 	return ""
 }
+
+// MergeWorkflows will do strategic merge the workflows
+func MergeWorkflows(originalWf, patchWf wfv1.Workflow) (*wfv1.Workflow, error) {
+	workflowBytes, err := json.Marshal(wfv1.Workflow{Spec: patchWf.Spec})
+	if err != nil {
+		return nil, err
+	}
+
+	storedWFByte, err := json.Marshal(wfv1.Workflow{Spec: originalWf.Spec})
+	if err != nil {
+		return nil, err
+	}
+
+	mergedWFByte, err := strategicpatch.StrategicMergePatch(storedWFByte, workflowBytes, wfv1.Workflow{})
+	if err != nil {
+		return nil, err
+	}
+
+	var mergedWf wfv1.Workflow
+	err = json.Unmarshal(mergedWFByte, &mergedWf)
+	if err != nil {
+		return nil, err
+	}
+	return &mergedWf, nil
+
+}

workflow/util/util_test.go

@@ -396,3 +396,46 @@ func TestApplySubmitOpts(t *testing.T) {
 		}
 	})
 }
+
+var origWF = `
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  generateName: workflow-template-hello-world-
+spec:
+  arguments:
+    parameters:
+    - name: message
+      value: original
+  entrypoint: start
+  onExit: end
+  serviceAccountName: argo
+  workflowTemplateRef:
+    name: workflow-template-submittable
+`
+var patchWF = `
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  generateName: workflow-template-hello-world-
+spec:
+  arguments:
+    parameters:
+    - name: message
+      value: patch
+  serviceAccountName: argo1
+  podGC:
+    strategy: OnPodSuccess
+`
+
+func TestMergeWorkflows(t *testing.T) {
+	origWf := unmarshalWF(origWF)
+	patchWf := unmarshalWF(patchWF)
+
+	mergedWf, err := MergeWorkflows(*origWf, *patchWf)
+	assert.NoError(t, err)
+	assert.Equal(t, origWf.Spec.Entrypoint, mergedWf.Spec.Entrypoint)
+	assert.Equal(t, patchWf.Spec.ServiceAccountName, mergedWf.Spec.ServiceAccountName)
+	assert.Equal(t, patchWf.Spec.Arguments.Parameters[0].Name, mergedWf.Spec.Arguments.Parameters[0].Name)
+	assert.Equal(t, patchWf.Spec.Arguments.Parameters[0].Value, mergedWf.Spec.Arguments.Parameters[0].Value)
+}

workflow/validate/validate.go

@@ -98,6 +98,13 @@ var wfTmplRefAllowedWfSpecValidFields = map[string]bool{
 	"Arguments":             true,
 	"WorkflowTemplateRef":   true,
 	"TTLStrategy":           true,
+	"Parallelism":           true,
+	"Volumes":               true,
+	"VolumeClaimTemplates":  true,
+	"NodeSelector":          true,
+	"OnExit":                true,
+	"PodGC":                 true,
+	"ServiceAccountName":    true,
 }
 
 type FakeArguments struct{}