fix: Enhanced WorkflowTemplate and ClusterWorkflowTemplate validation to support Global Variables

pkg/apis/workflow/v1alpha1/cluster_workflow_template_types.go

@@ -56,7 +56,3 @@ func (cwftmpl *ClusterWorkflowTemplate) GetResourceScope() ResourceScope {
 	return ResourceScopeCluster
 }
 
-// GetTemplates returns the list of templates of cluster workflow template
-func (cwftmpl *ClusterWorkflowTemplate) GetTemplates() []Template {
-	return cwftmpl.Spec.Templates
-}

pkg/apis/workflow/v1alpha1/common.go

@@ -19,7 +19,6 @@ type TemplateHolder interface {
 	GroupVersionKind() schema.GroupVersionKind
 	GetTemplateByName(name string) *Template
 	GetResourceScope() ResourceScope
-	GetTemplates() []Template
 }
 
 // TemplateReferenceHolder is an object that holds a reference to other templates; e.g. WorkflowStep, DAGTask, and NodeStatus

pkg/apis/workflow/v1alpha1/workflow_template_types.go

@@ -60,7 +60,3 @@ func (wftmpl *WorkflowTemplate) GetResourceScope() ResourceScope {
 	return ResourceScopeNamespaced
 }
 
-// GetTemplates returns the list of templates of workflow template
-func (wftmpl *WorkflowTemplate) GetTemplates() []Template {
-	return wftmpl.Spec.Templates
-}

pkg/apis/workflow/v1alpha1/workflow_types.go

@@ -1690,10 +1690,6 @@ func (wf *Workflow) GetResourceScope() ResourceScope {
 	return ResourceScopeLocal
 }
 
-// GetTemplates returns the list of templates of workflow.
-func (wf *Workflow) GetTemplates() []Template {
-	return wf.Spec.Templates
-}
 
 // NodeID creates a deterministic node ID based on a node name
 func (wf *Workflow) NodeID(name string) string {

server/clusterworkflowtemplate/cluster_workflow_template_server.go

@@ -29,7 +29,7 @@ func (cwts *ClusterWorkflowTemplateServer) CreateClusterWorkflowTemplate(ctx con
 
 	cwftmplGetter := templateresolution.WrapClusterWorkflowTemplateInterface(wfClient.ArgoprojV1alpha1().ClusterWorkflowTemplates())
 
-	err := validate.ValidateWorkflowTemplate(nil, cwftmplGetter, req.Template)
+	_, err := validate.ValidateClusterWorkflowTemplate(nil, cwftmplGetter, req.Template)
 	if err != nil {
 		return nil, err
 	}
@@ -82,7 +82,7 @@ func (cwts *ClusterWorkflowTemplateServer) LintClusterWorkflowTemplate(ctx conte
 
 	cwftmplGetter := templateresolution.WrapClusterWorkflowTemplateInterface(wfClient.ArgoprojV1alpha1().ClusterWorkflowTemplates())
 
-	err := validate.ValidateWorkflowTemplate(nil, cwftmplGetter, req.Template)
+	_, err := validate.ValidateClusterWorkflowTemplate(nil, cwftmplGetter, req.Template)
 	if err != nil {
 		return nil, err
 	}
@@ -97,7 +97,7 @@ func (cwts *ClusterWorkflowTemplateServer) UpdateClusterWorkflowTemplate(ctx con
 	wfClient := auth.GetWfClient(ctx)
 	cwftmplGetter := templateresolution.WrapClusterWorkflowTemplateInterface(wfClient.ArgoprojV1alpha1().ClusterWorkflowTemplates())
 
-	err := validate.ValidateWorkflowTemplate(nil, cwftmplGetter, req.Template)
+	_, err := validate.ValidateClusterWorkflowTemplate(nil, cwftmplGetter, req.Template)
 	if err != nil {
 		return nil, err
 	}

server/workflowtemplate/workflow_template_server.go

@@ -30,7 +30,7 @@ func (wts *WorkflowTemplateServer) CreateWorkflowTemplate(ctx context.Context, r
 
 	cwftmplGetter := templateresolution.WrapClusterWorkflowTemplateInterface(wfClient.ArgoprojV1alpha1().ClusterWorkflowTemplates())
 
-	err := validate.ValidateWorkflowTemplate(wftmplGetter, cwftmplGetter, req.Template)
+	_, err := validate.ValidateWorkflowTemplate(wftmplGetter, cwftmplGetter, req.Template)
 	if err != nil {
 		return nil, err
 	}
@@ -85,7 +85,7 @@ func (wts *WorkflowTemplateServer) LintWorkflowTemplate(ctx context.Context, req
 
 	cwftmplGetter := templateresolution.WrapClusterWorkflowTemplateInterface(wfClient.ArgoprojV1alpha1().ClusterWorkflowTemplates())
 
-	err := validate.ValidateWorkflowTemplate(wftmplGetter, cwftmplGetter, req.Template)
+	_, err := validate.ValidateWorkflowTemplate(wftmplGetter, cwftmplGetter, req.Template)
 	if err != nil {
 		return nil, err
 	}
@@ -102,7 +102,7 @@ func (wts *WorkflowTemplateServer) UpdateWorkflowTemplate(ctx context.Context, r
 
 	cwftmplGetter := templateresolution.WrapClusterWorkflowTemplateInterface(wfClient.ArgoprojV1alpha1().ClusterWorkflowTemplates())
 
-	err := validate.ValidateWorkflowTemplate(wftmplGetter, cwftmplGetter, req.Template)
+	_, err := validate.ValidateWorkflowTemplate(wftmplGetter, cwftmplGetter, req.Template)
 	if err != nil {
 		return nil, err
 	}

test/e2e/testdata/cluster-workflow-template-nested-template.yaml

@@ -1,3 +1,19 @@
+kind: ClusterWorkflowTemplate
+metadata:
+  name: cluster-workflow-template-whalesay-template
+  labels:
+    argo-e2e: true
+spec:
+  templates:
+    - name: whalesay-template
+      inputs:
+        parameters:
+          - name: message
+      container:
+        image: docker/whalesay
+        command: [cowsay]
+        args: ["{{inputs.parameters.message}}"]
+---
 apiVersion: argoproj.io/v1alpha1
 kind: ClusterWorkflowTemplate
 metadata:
@@ -8,8 +24,9 @@ spec:
   templates:
     - name: whalesay-inner-template
       templateRef:
-        name: workflow-template-whalesay-template
+        name: cluster-workflow-template-whalesay-template
         template: whalesay-template
+        clusterscope: true
       inputs:
         parameters:
           - name: message

workflow/validate/lint.go

@@ -127,7 +127,7 @@ func LintWorkflowTemplateFile(wftmplGetter templateresolution.WorkflowTemplateNa
 		return errors.Errorf(errors.CodeBadRequest, "%s failed to parse: %v", filePath, err)
 	}
 	for _, wftmpl := range workflowTemplates {
-		err = ValidateWorkflowTemplate(wftmplGetter, cwftmplGetter, &wftmpl)
+		_, err = ValidateWorkflowTemplate(wftmplGetter, cwftmplGetter, &wftmpl)
 		if err != nil {
 			return errors.Errorf(errors.CodeBadRequest, "%s: %s", filePath, err.Error())
 		}

workflow/validate/validate.go

@@ -35,6 +35,10 @@ type ValidateOpts struct {
 	// types of executors. For example, the inability of kubelet/k8s executors to copy artifacts
 	// out of the base image layer. If unspecified, will use docker executor validation
 	ContainerRuntimeExecutor string
+
+	// IgnoreEntrypoint indicates to skip/ignore the EntryPoint validation on workflow spec.
+	// Entrypoint is optional for WorkflowTemplate and ClusterWorkflowTemplate
+	IgnoreEntrypoint bool
 }
 
 // templateValidationCtx is the context for validating a workflow spec
@@ -134,7 +138,7 @@ func ValidateWorkflow(wftmplGetter templateresolution.WorkflowTemplateNamespaced
 		ctx.globalParams[common.GlobalVarWorkflowPriority] = strconv.Itoa(int(*wf.Spec.Priority))
 	}
 
-	if wf.Spec.Entrypoint == "" {
+	if !opts.IgnoreEntrypoint && wf.Spec.Entrypoint == "" {
 		return nil, errors.New(errors.CodeBadRequest, "spec.entrypoint is required")
 	}
 
@@ -166,9 +170,11 @@ func ValidateWorkflow(wftmplGetter templateresolution.WorkflowTemplateNamespaced
 		}
 	}
 
-	_, err = ctx.validateTemplateHolder(&wfv1.WorkflowStep{Template: wf.Spec.Entrypoint}, tmplCtx, &wf.Spec.Arguments, map[string]interface{}{})
-	if err != nil {
-		return nil, err
+	if !opts.IgnoreEntrypoint {
+		_, err = ctx.validateTemplateHolder(&wfv1.WorkflowStep{Template: wf.Spec.Entrypoint}, tmplCtx, &wf.Spec.Arguments, map[string]interface{}{})
+		if err != nil {
+			return nil, err
+		}
 	}
 	if wf.Spec.OnExit != "" {
 		// now when validating onExit, {{workflow.status}} is now available as a global
@@ -198,19 +204,16 @@ func ValidateWorkflow(wftmplGetter templateresolution.WorkflowTemplateNamespaced
 	return wfConditions, nil
 }
 
-// ValidateWorkflow accepts a workflow template and performs validation against it.
-func ValidateWorkflowTemplate(wftmplGetter templateresolution.WorkflowTemplateNamespacedGetter, cwftmplGetter templateresolution.ClusterWorkflowTemplateGetter, wftmpl wfv1.TemplateHolder) error {
-	ctx := newTemplateValidationCtx(nil, ValidateOpts{})
-	tmplCtx := templateresolution.NewContext(wftmplGetter, cwftmplGetter, wftmpl, nil)
+// ValidateWorkflowTemplate accepts a workflow template and performs validation against it.
+func ValidateWorkflowTemplate(wftmplGetter templateresolution.WorkflowTemplateNamespacedGetter, cwftmplGetter templateresolution.ClusterWorkflowTemplateGetter, wftmpl *wfv1.WorkflowTemplate) (*wfv1.WorkflowConditions, error) {
+	wf := common.ConvertWorkflowTemplateToWorkflow(wftmpl)
+	return ValidateWorkflow(wftmplGetter, cwftmplGetter, wf, ValidateOpts{IgnoreEntrypoint: wf.Spec.Entrypoint == ""})
+}
 
-	// Check if all templates can be resolved.
-	for _, template := range wftmpl.GetTemplates() {
-		_, err := ctx.validateTemplateHolder(&wfv1.WorkflowStep{Template: template.Name}, tmplCtx, &FakeArguments{}, map[string]interface{}{})
-		if err != nil {
-			return errors.Errorf(errors.CodeBadRequest, "templates.%s %s", template.Name, err.Error())
-		}
-	}
-	return nil
+// ValidateClusterWorkflowTemplate accepts a cluster workflow template and performs validation against it.
+func ValidateClusterWorkflowTemplate(wftmplGetter templateresolution.WorkflowTemplateNamespacedGetter, cwftmplGetter templateresolution.ClusterWorkflowTemplateGetter, cwftmpl *wfv1.ClusterWorkflowTemplate) (*wfv1.WorkflowConditions, error) {
+	wf := common.ConvertClusterWorkflowTemplateToWorkflow(cwftmpl)
+	return ValidateWorkflow(wftmplGetter, cwftmplGetter, wf, ValidateOpts{IgnoreEntrypoint: wf.Spec.Entrypoint == ""})
 }
 
 // ValidateCronWorkflow validates a CronWorkflow

workflow/validate/validate_test.go

@@ -39,7 +39,8 @@ func validate(yamlStr string) (*wfv1.WorkflowConditions, error) {
 // its validation result.
 func validateWorkflowTemplate(yamlStr string) error {
 	wftmpl := unmarshalWftmpl(yamlStr)
-	return ValidateWorkflowTemplate(wftmplGetter, cwftmplGetter, wftmpl)
+	_, err := ValidateWorkflowTemplate(wftmplGetter, cwftmplGetter, wftmpl)
+	return err
 }
 
 func unmarshalWf(yamlStr string) *wfv1.Workflow {