fix: Allow script and container image to be set in templateDefault. F…

…ixes #9633 (#10784)

Signed-off-by: Rory Doherty <rory@tigera.io>

test/e2e/fixtures/given.go

@@ -43,7 +43,7 @@ func (g *Given) Workflow(text string) *Given {
 	g.t.Helper()
 	g.wf = &wfv1.Workflow{}
 	g.readResource(text, g.wf)
-	g.checkImages(g.wf.Spec.Templates)
+	g.checkImages(g.wf)
 	return g
 }
 
@@ -81,8 +81,29 @@ func (g *Given) readResource(text string, v metav1.Object) {
 	}
 }
 
-func (g *Given) checkImages(templates []wfv1.Template) {
+func (g *Given) checkImages(wf interface{}) {
 	g.t.Helper()
+	var defaultImage string
+	var templates []wfv1.Template
+	switch baseTemplate := wf.(type) {
+	case *wfv1.Workflow:
+		if baseTemplate.Spec.TemplateDefaults != nil && baseTemplate.Spec.TemplateDefaults.Container != nil && baseTemplate.Spec.TemplateDefaults.Container.Image != "" {
+			defaultImage = baseTemplate.Spec.TemplateDefaults.Container.Image
+			templates = baseTemplate.Spec.Templates
+		}
+	case *wfv1.WorkflowTemplate:
+		if baseTemplate.Spec.TemplateDefaults != nil && baseTemplate.Spec.TemplateDefaults.Container != nil && baseTemplate.Spec.TemplateDefaults.Container.Image != "" {
+			defaultImage = baseTemplate.Spec.TemplateDefaults.Container.Image
+			templates = baseTemplate.Spec.Templates
+		}
+	case *wfv1.CronWorkflow:
+		if baseTemplate.Spec.WorkflowSpec.TemplateDefaults != nil && baseTemplate.Spec.WorkflowSpec.TemplateDefaults.Container != nil && baseTemplate.Spec.WorkflowSpec.TemplateDefaults.Container.Image != "" {
+			defaultImage = baseTemplate.Spec.WorkflowSpec.TemplateDefaults.Container.Image
+			templates = baseTemplate.Spec.WorkflowSpec.Templates
+		}
+	default:
+		g.t.Fatalf("Unsupported checkImage workflow type: %s", wf)
+	}
 
 	// discouraged
 	discouraged := func(image string) bool {
@@ -96,7 +117,12 @@ func (g *Given) checkImages(templates []wfv1.Template) {
 	for _, t := range templates {
 		container := t.Container
 		if container != nil {
-			image := container.Image
+			var image string
+			if container.Image != "" {
+				image = container.Image
+			} else {
+				image = defaultImage
+			}
 			if !allowed(image) {
 				g.t.Fatalf("image not allowed in tests: %s", image)
 			}
@@ -140,7 +166,7 @@ func (g *Given) WorkflowTemplate(text string) *Given {
 	g.t.Helper()
 	wfTemplate := &wfv1.WorkflowTemplate{}
 	g.readResource(text, wfTemplate)
-	g.checkImages(wfTemplate.Spec.Templates)
+	g.checkImages(wfTemplate)
 	g.wfTemplates = append(g.wfTemplates, wfTemplate)
 	return g
 }
@@ -149,7 +175,7 @@ func (g *Given) CronWorkflow(text string) *Given {
 	g.t.Helper()
 	g.cronWf = &wfv1.CronWorkflow{}
 	g.readResource(text, g.cronWf)
-	g.checkImages(g.cronWf.Spec.WorkflowSpec.Templates)
+	g.checkImages(g.cronWf)
 	return g
 }
 

test/e2e/functional/template-default-image.yaml

@@ -0,0 +1,31 @@
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  generateName: template-default-
+spec:
+  entrypoint: entrypoint
+  templateDefaults:
+    script:
+      image: argoproj/argosay:v1
+      command: [sh]
+      workingDir: "/src"
+    container:
+      image: argoproj/argosay:v1
+      command: [sh, -c]
+      workingDir: "/src"
+  templates:
+    - name: entrypoint
+      steps:
+      - - name: test-script
+          template: test-script
+        - name: test-container
+          template: test-container
+
+    - name: test-script
+      script:
+        source: |
+          pwd
+
+    - name: test-container
+      container:
+        args: ["pwd"]

test/e2e/functional_test.go

@@ -1179,3 +1179,11 @@ func (s *FunctionalSuite) TestTTY() {
 		SubmitWorkflow().
 		WaitForWorkflow(fixtures.ToBeSucceeded)
 }
+
+func (s *FunctionalSuite) TestTemplateDefaultImage() {
+	s.Given().
+		Workflow(`@functional/template-default-image.yaml`).
+		When().
+		SubmitWorkflow().
+		WaitForWorkflow(fixtures.ToBeSucceeded)
+}

workflow/validate/validate.go

@@ -450,7 +450,7 @@ func (ctx *templateValidationCtx) validateTemplate(tmpl *wfv1.Template, tmplCtx
 	case wfv1.TemplateTypeDAG:
 		err = ctx.validateDAG(scope, tmplCtx, newTmpl, workflowTemplateValidation)
 	default:
-		err = ctx.validateLeaf(scope, newTmpl, workflowTemplateValidation)
+		err = ctx.validateLeaf(scope, tmplCtx, newTmpl, workflowTemplateValidation)
 	}
 	if err != nil {
 		return err
@@ -668,7 +668,7 @@ func validateNonLeaf(tmpl *wfv1.Template) error {
 	return nil
 }
 
-func (ctx *templateValidationCtx) validateLeaf(scope map[string]interface{}, tmpl *wfv1.Template, workflowTemplateValidation bool) error {
+func (ctx *templateValidationCtx) validateLeaf(scope map[string]interface{}, tmplCtx *templateresolution.Context, tmpl *wfv1.Template, workflowTemplateValidation bool) error {
 	tmplBytes, err := json.Marshal(tmpl)
 	if err != nil {
 		return errors.InternalWrapError(err)
@@ -693,7 +693,18 @@ func (ctx *templateValidationCtx) validateLeaf(scope map[string]interface{}, tmp
 			mountPaths[art.Path] = fmt.Sprintf("inputs.artifacts.%s", art.Name)
 		}
 		if tmpl.Container.Image == "" {
-			return errors.Errorf(errors.CodeBadRequest, "templates.%s.container.image may not be empty", tmpl.Name)
+			switch baseTemplate := tmplCtx.GetCurrentTemplateBase().(type) {
+			case *wfv1.Workflow:
+				if !(baseTemplate.Spec.TemplateDefaults != nil && baseTemplate.Spec.TemplateDefaults.Container != nil && baseTemplate.Spec.TemplateDefaults.Container.Image != "") {
+					return errors.Errorf(errors.CodeBadRequest, "templates.%s.container.image may not be empty", tmpl.Name)
+				}
+			case *wfv1.WorkflowTemplate:
+				if !(baseTemplate.Spec.TemplateDefaults != nil && baseTemplate.Spec.TemplateDefaults.Container != nil && baseTemplate.Spec.TemplateDefaults.Container.Image != "") {
+					return errors.Errorf(errors.CodeBadRequest, "templates.%s.container.image may not be empty", tmpl.Name)
+				}
+			default:
+				return errors.Errorf(errors.CodeBadRequest, "templates.%s.container.image may not be empty", tmpl.Name)
+			}
 		}
 	}
 	if tmpl.ContainerSet != nil {
@@ -748,7 +759,18 @@ func (ctx *templateValidationCtx) validateLeaf(scope map[string]interface{}, tmp
 	}
 	if tmpl.Script != nil {
 		if tmpl.Script.Image == "" {
-			return errors.Errorf(errors.CodeBadRequest, "templates.%s.script.image may not be empty", tmpl.Name)
+			switch baseTemplate := tmplCtx.GetCurrentTemplateBase().(type) {
+			case *wfv1.Workflow:
+				if !(baseTemplate.Spec.TemplateDefaults != nil && baseTemplate.Spec.TemplateDefaults.Script != nil && baseTemplate.Spec.TemplateDefaults.Script.Image != "") {
+					return errors.Errorf(errors.CodeBadRequest, "templates.%s.script.image may not be empty", tmpl.Name)
+				}
+			case *wfv1.WorkflowTemplate:
+				if !(baseTemplate.Spec.TemplateDefaults != nil && baseTemplate.Spec.TemplateDefaults.Script != nil && baseTemplate.Spec.TemplateDefaults.Script.Image != "") {
+					return errors.Errorf(errors.CodeBadRequest, "templates.%s.script.image may not be empty", tmpl.Name)
+				}
+			default:
+				return errors.Errorf(errors.CodeBadRequest, "templates.%s.script.image may not be empty", tmpl.Name)
+			}
 		}
 	}
 	// we don't validate tmpl.Plugin, because this is done by Plugin.UnmarshallJSON

workflow/validate/validate_test.go

@@ -2282,6 +2282,143 @@ func TestInvalidWfNoImageFieldScript(t *testing.T) {
 	assert.EqualError(t, err, "templates.whalesay.script.image may not be empty")
 }
 
+var invalidWfNoImageScriptInTemplateDefault = `apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  name: hello-world-right-env-12
+spec:
+  entrypoint: whalesay
+  templateDefaults:
+    script:
+      command: [cowsay]
+  templates:
+  - name: whalesay
+    script:
+      args:
+      - hello world
+      env: []`
+
+func TestIinvalidWfNoImageScriptInTemplateDefault(t *testing.T) {
+	err := validate(invalidWfNoImageScriptInTemplateDefault)
+	assert.EqualError(t, err, "templates.whalesay.script.image may not be empty")
+}
+
+var validWfImageScriptInTemplateDefault = `apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  name: hello-world-right-env-12
+spec:
+  entrypoint: whalesay
+  templateDefaults:
+    script:
+      image: alpine:latest
+  templates:
+  - name: whalesay
+    script:
+      command:
+      - cowsay
+      args:
+      - hello world
+      env: []`
+
+func TestValidWfImageScriptInTemplateDefault(t *testing.T) {
+	err := validate(validWfImageScriptInTemplateDefault)
+	assert.NoError(t, err)
+}
+
+var validWfImageContainerInTemplateDefault = `apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  name: hello-world-right-env-12
+spec:
+  entrypoint: whalesay
+  templateDefaults:
+    container:
+      image: alpine:latest
+  templates:
+  - name: whalesay
+    container:
+      command:
+      - cowsay
+      args:
+      - hello world
+      env: []`
+
+func TestValidWfImageContainerInTemplateDefault(t *testing.T) {
+	err := validate(validWfImageContainerInTemplateDefault)
+	assert.NoError(t, err)
+}
+
+var templateRefScriptImageDefaultTarget = `
+apiVersion: argoproj.io/v1alpha1
+kind: WorkflowTemplate
+metadata:
+  name: template-ref-no-script-image
+spec:
+  entrypoint: whalesay
+  templateDefaults:
+    script:
+      image: alpine:latest
+  templates:
+  - name: whalesay
+    script:
+      command: [cowsay]
+      args: [hello world]
+`
+
+var wfWithWFTRefScriptImageInDefault = `
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  generateName: hello-world-
+  namespace: default
+spec:
+  workflowTemplateRef:
+    name: template-ref-no-script-image
+`
+
+func TestValidateFieldsWithWFTRefScriptImageInDefault(t *testing.T) {
+	err := createWorkflowTemplateFromSpec(templateRefScriptImageDefaultTarget)
+	assert.NoError(t, err)
+	err = validate(wfWithWFTRefScriptImageInDefault)
+	assert.NoError(t, err)
+}
+
+var templateRefContainerImageDefaultTarget = `
+apiVersion: argoproj.io/v1alpha1
+kind: WorkflowTemplate
+metadata:
+  name: template-ref-no-container-image
+spec:
+  entrypoint: whalesay
+  templateDefaults:
+    container:
+      image: alpine:latest
+  templates:
+  - name: whalesay
+    container:
+      command: [cowsay]
+      args: [hello world]
+`
+
+var wfWithWFTRefContainerImageInDefault = `
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  generateName: hello-world-
+  namespace: default
+spec:
+  workflowTemplateRef:
+    name: template-ref-no-container-image
+`
+
+func TestValidateFieldsWithWFTRefContainerImageInDefault(t *testing.T) {
+	err := createWorkflowTemplateFromSpec(templateRefContainerImageDefaultTarget)
+	assert.NoError(t, err)
+	err = validate(wfWithWFTRefContainerImageInDefault)
+	assert.NoError(t, err)
+}
+
 var templateRefWithParam = `
 apiVersion: argoproj.io/v1alpha1
 kind: WorkflowTemplate