Repository: argoproj/argo (go)

Issue:
argo lint does not detect missing input parameters
## Summary 

`argo lint` does not detect missing input parameters in a step.

What happened/what you expected to happen?

I made a workflow with missing input parameters in a step. 
`argo lint` didn't detect the issue. (Same with `argo template lint` with changing `kind` to WorkflowTemplate)
The workflow printed "{{inputs.parameters.message}}" as a raw text when the workflow submitted as follows:
```
$ kubectl logs arguments-steps-wpgn4-54317293 -c main
 _______________________________ 
< {{inputs.parameters.message}} >
 ------------------------------- 
    \
     \
      \     
                    ##        .            
              ## ## ##       ==            
           ## ## ## ##      ===            
       /""""""""""""""""___/ ===        
  ~~~ {~~ ~~~~ ~~~ ~~~~ ~~ ~ /  ===- ~~~   
       \______ o          __/            
        \    \        __/             
          \____\______/   

```

"argo lint" should detect missing input parameters.

## Diagnostics

What Kubernetes provider are you using? 
Docker desktop

What version of Argo Workflows are you running? 
argo: v2.11.5
  BuildDate: 2020-10-15T22:27:24Z
  GitCommit: 076bf89c4658adbd3b96050599f81424d1b08d6e
  GitTreeState: clean
  GitTag: v2.11.5
  GoVersion: go1.13.15
  Compiler: gc
  Platform: darwin/amd64

```yaml
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  generateName: arguments-steps-
spec:
  entrypoint: abc
  arguments:
    parameters: []
  templates:
    - steps:
        - - name: a
            template: whalesay
        - - name: b
            template: whalesay
            arguments:
              parameters:
                - name: message
                  value: "banana"
      name: abc
      inputs:
        parameters:
          - name: message
            value: hello argo
    - name: whalesay
      container:
        image: docker/whalesay
        command: [cowsay]
        args: ["{{inputs.parameters.message}}"]
```

```yaml
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  creationTimestamp: "2020-10-20T06:18:22Z"
  generateName: arguments-steps-
  generation: 8
  labels:
    workflows.argoproj.io/completed: "true"
    workflows.argoproj.io/phase: Succeeded
  managedFields:
  - apiVersion: argoproj.io/v1alpha1
    fieldsType: FieldsV1
    fieldsV1:
      f:metadata:
        f:generateName: {}
      f:spec:
        .: {}
        f:arguments: {}
        f:entrypoint: {}
        f:templates: {}
      f:status: {}
    manager: argo
    operation: Update
    time: "2020-10-20T06:18:22Z"
  - apiVersion: argoproj.io/v1alpha1
    fieldsType: FieldsV1
    fieldsV1:
      f:metadata:
        f:labels:
          .: {}
          f:workflows.argoproj.io/completed: {}
          f:workflows.argoproj.io/phase: {}
      f:status:
        f:conditions: {}
        f:finishedAt: {}
        f:nodes:
          .: {}
          f:arguments-steps-wpgn4:
            .: {}
            f:children: {}
            f:displayName: {}
            f:finishedAt: {}
            f:id: {}
            f:inputs:
              .: {}
              f:parameters: {}
            f:name: {}
            f:outboundNodes: {}
            f:phase: {}
            f:startedAt: {}
            f:templateName: {}
            f:templateScope: {}
            f:type: {}
          f:arguments-steps-wpgn4-54317293:
            .: {}
            f:boundaryID: {}
            f:displayName: {}
            f:finishedAt: {}
            f:hostNodeName: {}
            f:id: {}
            f:name: {}
            f:outputs:
              .: {}
              f:artifacts: {}
              f:exitCode: {}
            f:phase: {}
            f:resourcesDuration:
              .: {}
              f:cpu: {}
              f:memory: {}
            f:startedAt: {}
            f:templateName: {}
            f:templateScope: {}
            f:type: {}
          f:arguments-steps-wpgn4-838246124:
            .: {}
            f:boundaryID: {}
            f:children: {}
            f:displayName: {}
            f:finishedAt: {}
            f:id: {}
            f:name: {}
            f:phase: {}
            f:startedAt: {}
            f:templateName: {}
            f:templateScope: {}
            f:type: {}
          f:arguments-steps-wpgn4-905209505:
            .: {}
            f:boundaryID: {}
            f:children: {}
            f:displayName: {}
            f:finishedAt: {}
            f:id: {}
            f:name: {}
            f:phase: {}
            f:startedAt: {}
            f:templateName: {}
            f:templateScope: {}
            f:type: {}
          f:arguments-steps-wpgn4-2958228485:
            .: {}
            f:boundaryID: {}
            f:children: {}
            f:displayName: {}
            f:finishedAt: {}
            f:hostNodeName: {}
            f:id: {}
            f:name: {}
            f:outputs:
              .: {}
              f:artifacts: {}
              f:exitCode: {}
            f:phase: {}
            f:resourcesDuration:
              .: {}
              f:cpu: {}
              f:memory: {}
            f:startedAt: {}
            f:templateName: {}
            f:templateScope: {}
            f:type: {}
        f:phase: {}
        f:resourcesDuration:
          .: {}
          f:cpu: {}
          f:memory: {}
        f:startedAt: {}
    manager: workflow-controller
    operation: Update
    time: "2020-10-20T06:19:04Z"
  name: arguments-steps-wpgn4
  namespace: argo
  resourceVersion: "67559"
  selfLink: /apis/argoproj.io/v1alpha1/namespaces/argo/workflows/arguments-steps-wpgn4
  uid: 7cca9688-b123-418b-85e6-1acd5d90aee3
spec:
  arguments: {}
  entrypoint: abc
  templates:
  - arguments: {}
    inputs:
      parameters:
      - name: message
        value: hello argo
    metadata: {}
    name: abc
    outputs: {}
    steps:
    - - arguments: {}
        name: a
        template: whalesay
    - - arguments:
          parameters:
          - name: message
            value: banana
        name: b
        template: whalesay
  - arguments: {}
    container:
      args:
      - '{{inputs.parameters.message}}'
      command:
      - cowsay
      image: docker/whalesay
      name: ""
      resources: {}
    inputs: {}
    metadata: {}
    name: whalesay
    outputs: {}
status:
  conditions:
  - status: "True"
    type: Completed
  finishedAt: "2020-10-20T06:19:04Z"
  nodes:
    arguments-steps-wpgn4:
      children:
      - arguments-steps-wpgn4-838246124
      displayName: arguments-steps-wpgn4
      finishedAt: "2020-10-20T06:19:04Z"
      id: arguments-steps-wpgn4
      inputs:
        parameters:
        - name: message
          value: hello argo
      name: arguments-steps-wpgn4
      outboundNodes:
      - arguments-steps-wpgn4-54317293
      phase: Succeeded
      startedAt: "2020-10-20T06:18:22Z"
      templateName: abc
      templateScope: local/arguments-steps-wpgn4
      type: Steps
    arguments-steps-wpgn4-54317293:
      boundaryID: arguments-steps-wpgn4
      displayName: b
      finishedAt: "2020-10-20T06:19:02Z"
      hostNodeName: docker-desktop
      id: arguments-steps-wpgn4-54317293
      name: arguments-steps-wpgn4[1].b
      outputs:
        artifacts:
        - archiveLogs: true
          name: main-logs
          s3:
            accessKeySecret:
              key: accesskey
              name: my-minio-cred
            bucket: my-bucket
            endpoint: minio:9000
            insecure: true
            key: arguments-steps-wpgn4/arguments-steps-wpgn4-54317293/main.log
            secretKeySecret:
              key: secretkey
              name: my-minio-cred
        exitCode: "0"
      phase: Succeeded
      resourcesDuration:
        cpu: 5
        memory: 5
      startedAt: "2020-10-20T06:18:57Z"
      templateName: whalesay
      templateScope: local/arguments-steps-wpgn4
      type: Pod
    arguments-steps-wpgn4-838246124:
      boundaryID: arguments-steps-wpgn4
      children:
      - arguments-steps-wpgn4-2958228485
      displayName: '[0]'
      finishedAt: "2020-10-20T06:18:57Z"
      id: arguments-steps-wpgn4-838246124
      name: arguments-steps-wpgn4[0]
      phase: Succeeded
      startedAt: "2020-10-20T06:18:22Z"
      templateName: abc
      templateScope: local/arguments-steps-wpgn4
      type: StepGroup
    arguments-steps-wpgn4-905209505:
      boundaryID: arguments-steps-wpgn4
      children:
      - arguments-steps-wpgn4-54317293
      displayName: '[1]'
      finishedAt: "2020-10-20T06:19:04Z"
      id: arguments-steps-wpgn4-905209505
      name: arguments-steps-wpgn4[1]
      phase: Succeeded
      startedAt: "2020-10-20T06:18:57Z"
      templateName: abc
      templateScope: local/arguments-steps-wpgn4
      type: StepGroup
    arguments-steps-wpgn4-2958228485:
      boundaryID: arguments-steps-wpgn4
      children:
      - arguments-steps-wpgn4-905209505
      displayName: a
      finishedAt: "2020-10-20T06:18:55Z"
      hostNodeName: docker-desktop
      id: arguments-steps-wpgn4-2958228485
      name: arguments-steps-wpgn4[0].a
      outputs:
        artifacts:
        - archiveLogs: true
          name: main-logs
          s3:
            accessKeySecret:
              key: accesskey
              name: my-minio-cred
            bucket: my-bucket
            endpoint: minio:9000
            insecure: true
            key: arguments-steps-wpgn4/arguments-steps-wpgn4-2958228485/main.log
            secretKeySecret:
              key: secretkey
              name: my-minio-cred
        exitCode: "0"
      phase: Succeeded
      resourcesDuration:
        cpu: 32
        memory: 32
      startedAt: "2020-10-20T06:18:22Z"
      templateName: whalesay
      templateScope: local/arguments-steps-wpgn4
      type: Pod
  phase: Succeeded
  resourcesDuration:
    cpu: 37
    memory: 37
  startedAt: "2020-10-20T06:18:22Z"
```

---
<!-- Issue Author: Don't delete this message to encourage other users to support your issue! -->
**Message from the maintainers**:

Impacted by this bug? Give it a 👍. We prioritise the issues with the most 👍.


Pull request:
fix(validate): Local parameters should be validated locally. Fixes #4326
Checklist:

* [x] Either (a) I've created an [enhancement proposal](https://github.com/argoproj/argo/issues/new/choose) and discussed it with the community, (b) this is a bug fix, or (c) this is a chore.
* [x] The title of the PR is (a) [conventional](https://www.conventionalcommits.org/en/v1.0.0/), (b) states what changed, and (c) suffixes the related issues number. E.g. `"fix(controller): Updates such and such. Fixes #1234"`.  
* [ ] My organization is added to [USERS.md](https://github.com/argoproj/argo/blob/master/USERS.md).
* [x] I've signed the CLA.
* [x] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
* [ ] My builds are green. Try syncing with master if they are not. 

Fixes #4326

Patch:
diff --git a/workflow/validate/validate.go b/workflow/validate/validate.go
index 722d9f9a0..0f15b58c8 100644
--- a/workflow/validate/validate.go
+++ b/workflow/validate/validate.go
@@ -222,7 +222,7 @@ func ValidateWorkflow(wftmplGetter templateresolution.WorkflowTemplateNamespaced
 		if hasWorkflowTemplateRef {
 			tmpl = &wfv1.WorkflowStep{TemplateRef: wfTmplRef}
 		}
-		_, err = ctx.validateTemplateHolder(tmpl, tmplCtx, args, map[string]interface{}{})
+		_, err = ctx.validateTemplateHolder(tmpl, tmplCtx, args)
 		if err != nil {
 			return nil, err
 		}
@@ -231,7 +231,7 @@ func ValidateWorkflow(wftmplGetter templateresolution.WorkflowTemplateNamespaced
 		// now when validating onExit, {{workflow.status}} is now available as a global
 		ctx.globalParams[common.GlobalVarWorkflowStatus] = placeholderGenerator.NextPlaceholder()
 		ctx.globalParams[common.GlobalVarWorkflowFailures] = placeholderGenerator.NextPlaceholder()
-		_, err = ctx.validateTemplateHolder(&wfv1.WorkflowStep{Template: wf.Spec.OnExit}, tmplCtx, &wf.Spec.Arguments, map[string]interface{}{})
+		_, err = ctx.validateTemplateHolder(&wfv1.WorkflowStep{Template: wf.Spec.OnExit}, tmplCtx, &wf.Spec.Arguments)
 		if err != nil {
 			return nil, err
 		}
@@ -247,7 +247,7 @@ func ValidateWorkflow(wftmplGetter templateresolution.WorkflowTemplateNamespaced
 
 	// Check if all templates can be resolved.
 	for _, template := range wf.Spec.Templates {
-		_, err := ctx.validateTemplateHolder(&wfv1.WorkflowStep{Template: template.Name}, tmplCtx, &FakeArguments{}, map[string]interface{}{})
+		_, err := ctx.validateTemplateHolder(&wfv1.WorkflowStep{Template: template.Name}, tmplCtx, &FakeArguments{})
 		if err != nil {
 			return nil, errors.Errorf(errors.CodeBadRequest, "templates.%s %s", template.Name, err.Error())
 		}
@@ -358,12 +358,12 @@ For more information, see: %s
 	return out
 }
 
-func (ctx *templateValidationCtx) validateTemplate(tmpl *wfv1.Template, tmplCtx *templateresolution.Context, args wfv1.ArgumentsProvider, extraScope map[string]interface{}) error {
+func (ctx *templateValidationCtx) validateTemplate(tmpl *wfv1.Template, tmplCtx *templateresolution.Context, args wfv1.ArgumentsProvider) error {
 	if err := validateTemplateType(tmpl); err != nil {
 		return err
 	}
 
-	scope, err := validateInputs(tmpl, extraScope)
+	scope, err := validateInputs(tmpl)
 	if err != nil {
 		return err
 	}
@@ -458,7 +458,7 @@ func (ctx *templateValidationCtx) validateTemplate(tmpl *wfv1.Template, tmplCtx
 }
 
 // validateTemplateHolder validates a template holder and returns the validated template.
-func (ctx *templateValidationCtx) validateTemplateHolder(tmplHolder wfv1.TemplateReferenceHolder, tmplCtx *templateresolution.Context, args wfv1.ArgumentsProvider, extraScope map[string]interface{}) (*wfv1.Template, error) {
+func (ctx *templateValidationCtx) validateTemplateHolder(tmplHolder wfv1.TemplateReferenceHolder, tmplCtx *templateresolution.Context, args wfv1.ArgumentsProvider) (*wfv1.Template, error) {
 	tmplRef := tmplHolder.GetTemplateRef()
 	tmplName := tmplHolder.GetTemplateName()
 	if tmplRef != nil {
@@ -512,7 +512,7 @@ func (ctx *templateValidationCtx) validateTemplateHolder(tmplHolder wfv1.Templat
 		}
 	}
 
-	return resolvedTmpl, ctx.validateTemplate(resolvedTmpl, tmplCtx, args, extraScope)
+	return resolvedTmpl, ctx.validateTemplate(resolvedTmpl, tmplCtx, args)
 }
 
 // validateTemplateType validates that only one template type is defined
@@ -537,7 +537,7 @@ func validateTemplateType(tmpl *wfv1.Template) error {
 	return nil
 }
 
-func validateInputs(tmpl *wfv1.Template, extraScope map[string]interface{}) (map[string]interface{}, error) {
+func validateInputs(tmpl *wfv1.Template) (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())
@@ -547,9 +547,6 @@ func validateInputs(tmpl *wfv1.Template, extraScope map[string]interface{}) (map
 		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
 	}
@@ -815,7 +812,7 @@ func (ctx *templateValidationCtx) validateSteps(scope map[string]interface{}, tm
 			if err != nil {
 				return err
 			}
-			resolvedTmpl, err := ctx.validateTemplateHolder(&step, tmplCtx, &FakeArguments{}, scope)
+			resolvedTmpl, err := ctx.validateTemplateHolder(&step, tmplCtx, &FakeArguments{})
 			if err != nil {
 				return errors.Errorf(errors.CodeBadRequest, "templates.%s.steps[%d].%s %s", tmpl.Name, i, step.Name, err.Error())
 			}
@@ -837,7 +834,7 @@ func (ctx *templateValidationCtx) validateSteps(scope map[string]interface{}, tm
 			ctx.addOutputsToScope(resolvedTmpl, fmt.Sprintf("steps.%s", step.Name), scope, aggregate, false)
 
 			// Validate the template again with actual arguments.
-			_, err = ctx.validateTemplateHolder(&step, tmplCtx, &step.Arguments, scope)
+			_, err = ctx.validateTemplateHolder(&step, tmplCtx, &step.Arguments)
 			if err != nil {
 				return errors.Errorf(errors.CodeBadRequest, "templates.%s.steps[%d].%s %s", tmpl.Name, i, step.Name, err.Error())
 			}
@@ -1216,7 +1213,7 @@ func (ctx *templateValidationCtx) validateDAG(scope map[string]interface{}, tmpl
 			return errors.Errorf(errors.CodeBadRequest, "templates.%s cannot use 'continueOn' when using 'depends'. Instead use 'dep-task.Failed'/'dep-task.Errored'", tmpl.Name)
 		}
 
-		resolvedTmpl, err := ctx.validateTemplateHolder(&task, tmplCtx, &FakeArguments{}, map[string]interface{}{})
+		resolvedTmpl, err := ctx.validateTemplateHolder(&task, tmplCtx, &FakeArguments{})
 		if err != nil {
 			return errors.Errorf(errors.CodeBadRequest, "templates.%s.tasks.%s %s", tmpl.Name, task.Name, err.Error())
 		}
@@ -1286,7 +1283,7 @@ func (ctx *templateValidationCtx) validateDAG(scope map[string]interface{}, tmpl
 			return err
 		}
 		// Validate the template again with actual arguments.
-		_, err = ctx.validateTemplateHolder(&task, tmplCtx, &task.Arguments, scope)
+		_, err = ctx.validateTemplateHolder(&task, tmplCtx, &task.Arguments)
 		if err != nil {
 			return errors.Errorf(errors.CodeBadRequest, "templates.%s.tasks.%s %s", tmpl.Name, task.Name, err.Error())
 		}


Test patch:
diff --git a/workflow/validate/validate_test.go b/workflow/validate/validate_test.go
index e6708d1df..6d427428c 100644
--- a/workflow/validate/validate_test.go
+++ b/workflow/validate/validate_test.go
@@ -170,6 +170,35 @@ spec:
       image: docker/whalesay:{{inputs.parameters.unresolved}}
 `
 
+var unresolvedStepInput = `
+apiVersion: argoproj.io/v1alpha1
+kind: Workflow
+metadata:
+  generateName: hello-world-
+spec:
+  entrypoint: entry-step
+  arguments:
+    parameters: []
+  templates:
+    - steps:
+        - - name: a
+            arguments:
+              parameters:
+                - name: message
+                  value: "{{inputs.parameters.message}}"
+            template: whalesay
+      name: entry-step
+      inputs:
+        parameters:
+          - name: message
+            value: hello world
+    - name: whalesay
+      container:
+        image: docker/whalesay
+        command: [cowsay]
+        args: ["{{inputs.parameters.message}}"]
+`
+
 var unresolvedOutput = `
 apiVersion: argoproj.io/v1alpha1
 kind: Workflow
@@ -197,6 +226,10 @@ func TestUnresolved(t *testing.T) {
 	if assert.NotNil(t, err) {
 		assert.Contains(t, err.Error(), "failed to resolve")
 	}
+	_, err = validate(unresolvedStepInput)
+	if assert.NotNil(t, err) {
+		assert.Contains(t, err.Error(), "failed to resolve")
+	}
 	_, err = validate(unresolvedOutput)
 	if assert.NotNil(t, err) {
 		assert.Contains(t, err.Error(), "failed to resolve")