Fix validation (#1508)

workflow/common/util.go

@@ -244,8 +244,8 @@ func ProcessArgs(tmpl *wfv1.Template, args wfv1.Arguments, globalParams, localPa
 	// 1) check if was supplied as argument. if so use the supplied value from arg
 	// 2) if not, use default value.
 	// 3) if no default value, it is an error
-	tmpl = tmpl.DeepCopy()
-	for i, inParam := range tmpl.Inputs.Parameters {
+	newTmpl := tmpl.DeepCopy()
+	for i, inParam := range newTmpl.Inputs.Parameters {
 		if inParam.Default != nil {
 			// first set to default value
 			inParam.Value = inParam.Default
@@ -259,12 +259,12 @@ func ProcessArgs(tmpl *wfv1.Template, args wfv1.Arguments, globalParams, localPa
 		if inParam.Value == nil {
 			return nil, errors.Errorf(errors.CodeBadRequest, "inputs.parameters.%s was not supplied", inParam.Name)
 		}
-		tmpl.Inputs.Parameters[i] = inParam
+		newTmpl.Inputs.Parameters[i] = inParam
 	}
 
 	// Performs substitutions of input artifacts
-	newInputArtifacts := make([]wfv1.Artifact, len(tmpl.Inputs.Artifacts))
-	for i, inArt := range tmpl.Inputs.Artifacts {
+	newInputArtifacts := make([]wfv1.Artifact, len(newTmpl.Inputs.Artifacts))
+	for i, inArt := range newTmpl.Inputs.Artifacts {
 		// if artifact has hard-wired location, we prefer that
 		if inArt.HasLocation() {
 			newInputArtifacts[i] = inArt
@@ -288,9 +288,10 @@ func ProcessArgs(tmpl *wfv1.Template, args wfv1.Arguments, globalParams, localPa
 			newInputArtifacts[i] = inArt
 		}
 	}
-	tmpl.Inputs.Artifacts = newInputArtifacts
+	newTmpl.Inputs.Artifacts = newInputArtifacts
+
+	return substituteParams(newTmpl, globalParams, localParams)
 
-	return substituteParams(tmpl, globalParams, localParams)
 }
 
 // substituteParams returns a new copy of the template with global, pod, and input parameters substituted

workflow/validate/validate.go

@@ -79,7 +79,13 @@ func ValidateWorkflow(wf *wfv1.Workflow, opts ValidateOpts) error {
 	ctx.globalParams[common.GlobalVarWorkflowNamespace] = placeholderValue
 	ctx.globalParams[common.GlobalVarWorkflowUID] = placeholderValue
 	for _, param := range ctx.wf.Spec.Arguments.Parameters {
-		ctx.globalParams["workflow.parameters."+param.Name] = placeholderValue
+		if param.Name != "" {
+			if param.Value != nil {
+				ctx.globalParams["workflow.parameters."+param.Name] = *param.Value
+			} else {
+				ctx.globalParams["workflow.parameters."+param.Name] = placeholderValue
+			}
+		}
 	}
 
 	for k := range ctx.wf.ObjectMeta.Annotations {
@@ -96,7 +102,7 @@ func ValidateWorkflow(wf *wfv1.Workflow, opts ValidateOpts) error {
 	if entryTmpl == nil {
 		return errors.Errorf(errors.CodeBadRequest, "spec.entrypoint template '%s' undefined", ctx.wf.Spec.Entrypoint)
 	}
-	err = ctx.validateTemplate(entryTmpl, ctx.wf.Spec.Arguments)
+	err = ctx.validateTemplate(entryTmpl, ctx.wf.Spec.Arguments, map[string]interface{}{})
 	if err != nil {
 		return err
 	}
@@ -107,15 +113,15 @@ func ValidateWorkflow(wf *wfv1.Workflow, opts ValidateOpts) error {
 		}
 		// now when validating onExit, {{workflow.status}} is now available as a global
 		ctx.globalParams[common.GlobalVarWorkflowStatus] = placeholderValue
-		err = ctx.validateTemplate(exitTmpl, ctx.wf.Spec.Arguments)
+		err = ctx.validateTemplate(exitTmpl, ctx.wf.Spec.Arguments, map[string]interface{}{})
 		if err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
-func (ctx *wfValidationCtx) validateTemplate(tmpl *wfv1.Template, args wfv1.Arguments) error {
+func (ctx *wfValidationCtx) validateTemplate(tmpl *wfv1.Template, args wfv1.Arguments, extraScope map[string]interface{}) error {
 	_, ok := ctx.results[tmpl.Name]
 	if ok {
 		// we already processed this template
@@ -125,7 +131,7 @@ func (ctx *wfValidationCtx) validateTemplate(tmpl *wfv1.Template, args wfv1.Argu
 	if err := validateTemplateType(tmpl); err != nil {
 		return err
 	}
-	scope, err := validateInputs(tmpl)
+	scope, err := validateInputs(tmpl, extraScope)
 	if err != nil {
 		return err
 	}
@@ -147,34 +153,34 @@ func (ctx *wfValidationCtx) validateTemplate(tmpl *wfv1.Template, args wfv1.Argu
 		}
 	}
 
-	_, err = common.ProcessArgs(tmpl, args, ctx.globalParams, localParams, true)
+	newTmpl, err := common.ProcessArgs(tmpl, args, ctx.globalParams, localParams, true)
 	if err != nil {
 		return errors.Errorf(errors.CodeBadRequest, "templates.%s %s", tmpl.Name, err)
 	}
 	for globalVar, val := range ctx.globalParams {
 		scope[globalVar] = val
 	}
-	switch tmpl.GetType() {
+	switch newTmpl.GetType() {
 	case wfv1.TemplateTypeSteps:
-		err = ctx.validateSteps(scope, tmpl)
+		err = ctx.validateSteps(scope, newTmpl)
 	case wfv1.TemplateTypeDAG:
-		err = ctx.validateDAG(scope, tmpl)
+		err = ctx.validateDAG(scope, newTmpl)
 	default:
-		err = validateLeaf(scope, tmpl)
+		err = validateLeaf(scope, newTmpl)
 	}
 	if err != nil {
 		return err
 	}
-	err = validateOutputs(scope, tmpl)
+	err = validateOutputs(scope, newTmpl)
 	if err != nil {
 		return err
 	}
-	err = ctx.validateBaseImageOutputs(tmpl)
+	err = ctx.validateBaseImageOutputs(newTmpl)
 	if err != nil {
 		return err
 	}
-	if tmpl.ArchiveLocation != nil {
-		err = validateArtifactLocation("templates.archiveLocation", *tmpl.ArchiveLocation)
+	if newTmpl.ArchiveLocation != nil {
+		err = validateArtifactLocation("templates.archiveLocation", *newTmpl.ArchiveLocation)
 		if err != nil {
 			return err
 		}
@@ -200,7 +206,7 @@ func validateTemplateType(tmpl *wfv1.Template) error {
 	return nil
 }
 
-func validateInputs(tmpl *wfv1.Template) (map[string]interface{}, error) {
+func validateInputs(tmpl *wfv1.Template, extraScope map[string]interface{}) (map[string]interface{}, error) {
 	err := validateWorkflowFieldNames(tmpl.Inputs.Parameters)
 	if err != nil {
 		return nil, errors.Errorf(errors.CodeBadRequest, "templates.%s.inputs.parameters%s", tmpl.Name, err.Error())
@@ -210,6 +216,9 @@ func validateInputs(tmpl *wfv1.Template) (map[string]interface{}, error) {
 		return nil, errors.Errorf(errors.CodeBadRequest, "templates.%s.inputs.artifacts%s", tmpl.Name, err.Error())
 	}
 	scope := make(map[string]interface{})
+	for name, value := range extraScope {
+		scope[name] = value
+	}
 	for _, param := range tmpl.Inputs.Parameters {
 		scope[fmt.Sprintf("inputs.parameters.%s", param.Name)] = true
 	}
@@ -429,7 +438,7 @@ func (ctx *wfValidationCtx) validateSteps(scope map[string]interface{}, tmpl *wf
 			if err != nil {
 				return err
 			}
-			err = ctx.validateTemplate(childTmpl, step.Arguments)
+			err = ctx.validateTemplate(childTmpl, step.Arguments, scope)
 			if err != nil {
 				return err
 			}
@@ -795,7 +804,7 @@ func (ctx *wfValidationCtx) validateDAG(scope map[string]interface{}, tmpl *wfv1
 			return err
 		}
 		taskTmpl := ctx.wf.GetTemplate(task.Template)
-		err = ctx.validateTemplate(taskTmpl, task.Arguments)
+		err = ctx.validateTemplate(taskTmpl, task.Arguments, scope)
 		if err != nil {
 			return err
 		}

workflow/validate/validate_dag_test.go

@@ -116,7 +116,7 @@ spec:
       - name: A
         template: echo
         arguments:
-          parameters: 
+          parameters:
           - name: message
             value: val
       - name: B
@@ -154,14 +154,14 @@ spec:
       - name: A
         template: echo
         arguments:
-          parameters: 
+          parameters:
           - name: message
             value: val
       - name: B
         dependencies: [A]
         template: echo
         arguments:
-          parameters: 
+          parameters:
           - name: message
             value: "{{tasks.A.outputs.parameters.hosts}}"
       - name: C
@@ -199,14 +199,14 @@ spec:
       - name: A
         template: echo
         arguments:
-          parameters: 
+          parameters:
           - name: message
             value: val
       - name: B
         dependencies: [A]
         template: echo
         arguments:
-          parameters: 
+          parameters:
           - name: message
             value: "{{tasks.A.outputs.parameters.hosts}}"
       - name: C
@@ -246,7 +246,7 @@ spec:
       command: [echo, generate]
     outputs:
       artifacts:
-      - name: hosts
+      - name: generated_hosts
         path: /etc/hosts
 
   - name: echo
@@ -282,7 +282,7 @@ spec:
             value: val
           artifacts:
           - name: passthrough
-            from: "{{tasks.A.outputs.artifacts.hosts}}"
+            from: "{{tasks.A.outputs.artifacts.generated_hosts}}"
 `
 
 func TestDAGArtifactResolution(t *testing.T) {

workflow/validate/validate_test.go

@@ -239,7 +239,7 @@ var stepOutputReferences = `
 apiVersion: argoproj.io/v1alpha1
 kind: Workflow
 metadata:
-  generateName: hello-world-
+  generateName: step-output-ref-
 spec:
   entrypoint: whalesay
   templates:
@@ -267,11 +267,67 @@ spec:
             value: "{{steps.one.outparam}}"
 `
 
-func TestStepReference(t *testing.T) {
+func TestStepOutputReference(t *testing.T) {
 	err := validate(stepOutputReferences)
 	assert.Nil(t, err)
 }
 
+var stepArtReferences = `
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  generateName: step-art-ref-
+spec:
+  entrypoint: stepref
+  templates:
+  - name: generate
+    container:
+      image: alpine:3.7
+      command: [echo, generate]
+    outputs:
+      artifacts:
+      - name: generated_hosts
+        path: /etc/hosts
+
+  - name: echo
+    inputs:
+      parameters:
+      - name: message
+      artifacts:
+      - name: passthrough
+        path: /tmp/passthrough
+    container:
+      image: alpine:3.7
+      command: [echo, "{{inputs.parameters.message}}"]
+    outputs:
+      parameters:
+      - name: hosts
+        valueFrom:
+          path: /etc/hosts
+      artifacts:
+      - name: someoutput
+        path: /tmp/passthrough
+
+  - name: stepref
+    steps:
+    - - name: one
+        template: generate
+    - - name: two
+        template: echo
+        arguments:
+          parameters:
+          - name: message
+            value: val
+          artifacts:
+          - name: passthrough
+            from: "{{steps.one.outputs.artifacts.generated_hosts}}"
+`
+
+func TestStepArtReference(t *testing.T) {
+	err := validate(stepArtReferences)
+	assert.Nil(t, err)
+}
+
 var unsatisfiedParam = `
 apiVersion: argoproj.io/v1alpha1
 kind: Workflow