fix: type-aware application of template defaults (#14899)

pkg/apis/workflow/v1alpha1/workflow_types.go

@@ -57,6 +57,24 @@ const (
 	TemplateTypeUnknown      TemplateType = "Unknown"
 )
 
+// IsValid returns true if t exists in the set of possible template types.
+func (t TemplateType) IsValid() bool {
+	switch t {
+	case TemplateTypeContainer,
+		TemplateTypeContainerSet,
+		TemplateTypeSteps,
+		TemplateTypeScript,
+		TemplateTypeResource,
+		TemplateTypeDAG,
+		TemplateTypeSuspend,
+		TemplateTypeData,
+		TemplateTypeHTTP,
+		TemplateTypePlugin:
+		return true
+	}
+	return false
+}
+
 // NodePhase is a label for the condition of a node at the current time.
 type NodePhase string
 

workflow/controller/operator.go

@@ -4303,30 +4303,49 @@ func (woc *wfOperationCtx) setStoredWfSpec(ctx context.Context) error {
 	return nil
 }
 
+// mergedTemplateDefaultsInto modifies originalTmpl, setting any applicable default values.
 func (woc *wfOperationCtx) mergedTemplateDefaultsInto(originalTmpl *wfv1.Template) error {
-	if woc.execWf.Spec.TemplateDefaults != nil {
-		originalTmplType := originalTmpl.GetType()
+	if woc.execWf.Spec.TemplateDefaults == nil {
+		return nil
+	}
 
-		tmplDefaultsJSON, err := json.Marshal(woc.execWf.Spec.TemplateDefaults)
-		if err != nil {
-			return err
-		}
+	originalTmplType := originalTmpl.GetType()
+	applicableDefaults := woc.execWf.Spec.TemplateDefaults.DeepCopy()
 
-		targetTmplJSON, err := json.Marshal(originalTmpl)
-		if err != nil {
-			return err
+	v := reflect.ValueOf(applicableDefaults).Elem()
+	for i := 0; i < v.NumField(); i++ {
+		field := v.Type().Field(i)
+		// Check if the field is a pointer to a struct.
+		if field.Type.Kind() != reflect.Ptr || field.Type.Elem().Kind() != reflect.Struct {
+			continue
 		}
 
-		resultTmpl, err := strategicpatch.StrategicMergePatch(tmplDefaultsJSON, targetTmplJSON, wfv1.Template{})
-		if err != nil {
-			return err
+		// Unset any Template-type defaults not applicable to the target type.
+		if t := wfv1.TemplateType(field.Name); t.IsValid() && t != originalTmplType {
+			v.Field(i).Set(reflect.Zero(v.Field(i).Type()))
 		}
-		err = json.Unmarshal(resultTmpl, originalTmpl)
-		if err != nil {
-			return err
-		}
-		originalTmpl.SetType(originalTmplType)
 	}
+
+	tmplDefaultsJSON, err := json.Marshal(applicableDefaults)
+	if err != nil {
+		return err
+	}
+
+	targetTmplJSON, err := json.Marshal(originalTmpl)
+	if err != nil {
+		return err
+	}
+
+	resultTmpl, err := strategicpatch.StrategicMergePatch(tmplDefaultsJSON, targetTmplJSON, wfv1.Template{})
+	if err != nil {
+		return err
+	}
+
+	if err := json.Unmarshal(resultTmpl, originalTmpl); err != nil {
+		return err
+	}
+
+	originalTmpl.SetType(originalTmplType)
 	return nil
 }
 

workflow/controller/operator_tmpl_default_test.go

@@ -112,6 +112,19 @@ spec:
       args: ["hello world"]
 `
 
+const httpWf = `
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  generateName: http-
+spec:
+  entrypoint: http-template
+  templates:
+  - name: http-template
+    http:
+      url: http://dummy.rest.api/endpoint
+`
+
 func TestSetTemplateDefault(t *testing.T) {
 	cancel, controller := newController(logging.TestContext(t.Context()))
 	defer cancel()
@@ -257,4 +270,21 @@ func TestSetTemplateDefault(t *testing.T) {
 		assert.Equal(t, "test", tmpl2.Container.Env[0].Name)
 		assert.Nil(t, tmpl2.Script)
 	})
+	t.Run("HTTPTmplDefaultWf", func(t *testing.T) {
+		wf := wfv1.MustUnmarshalWorkflow(httpWf)
+		wf.Spec.TemplateDefaults = &wfv1.Template{
+			Container: &apiv1.Container{
+				ImagePullPolicy: apiv1.PullIfNotPresent,
+			},
+		}
+		woc := newWorkflowOperationCtx(ctx, wf, controller)
+		err := woc.setExecWorkflow(ctx)
+		require.NoError(t, err)
+		tmpl := woc.execWf.Spec.Templates[0]
+		err = woc.mergedTemplateDefaultsInto(&tmpl)
+		require.NoError(t, err)
+		assert.NotNil(t, tmpl)
+		assert.Equal(t, wfv1.TemplateTypeHTTP, tmpl.GetType())
+		assert.Nil(t, tmpl.Container)
+	})
 }