Validate PipelineRun Parameters

docs/pipelineruns.md

@@ -186,6 +186,9 @@ to all `persistentVolumeClaims` generated internally.
 You can specify `Parameters` that you want to pass to the `Pipeline` during execution,
 including different values of the same parameter for different `Tasks` in the `Pipeline`.
 
+**Note:** You must specify all the `Parameters` that the `Pipeline` expects. Parameters 
+that have default values specified in Pipeline are not required to be provided by PipelineRun.
+
 For example:
 
 ```yaml
@@ -196,6 +199,10 @@ spec:
   - name: pl-param-y
     value: "500"
 ```
+You can pass in extra `Parameters` if needed depending on your use cases. An example use 
+case is when your CI system autogenerates `PipelineRuns` and it has `Parameters` it wants to 
+provide to all `PipelineRuns`. Because you can pass in extra `Parameters`, you don't have to 
+go through the complexity of checking each `Pipeline` and providing only the required params.
 
 ### Specifying custom `ServiceAccount` credentials
 

examples/v1beta1/pipelineruns/pipelinerun-with-extra-params.yaml

@@ -0,0 +1,56 @@
+apiVersion: tekton.dev/v1beta1
+kind: Pipeline
+metadata:
+  name: pipeline-with-extra-params
+spec:
+  params:
+  - name: pl-param-x
+    type: string
+  - name: pl-param-y
+    type: string
+  tasks:
+  - name: add-params
+    taskRef:
+      name: add-params
+    params:
+    - name: a
+      value: "$(params.pl-param-x)"
+    - name: b
+      value: "$(params.pl-param-y)"
+---
+apiVersion: tekton.dev/v1beta1
+kind: Task
+metadata:
+  name: add-params
+  annotations:
+    description: |
+      A simple task that sums the two provided integers
+spec:
+  params:
+  - name: a
+    type: string
+    description: The first integer
+  - name: b
+    type: string
+    description: The second integer
+  steps:
+  - name: sum
+    image: bash:latest
+    script: |
+      #!/usr/bin/env bash
+      echo -n $(( "$(inputs.params.a)" + "$(inputs.params.b)" ))
+---
+apiVersion: tekton.dev/v1beta1
+kind: PipelineRun
+metadata:
+  generateName: pipelinerun-with-extra-params
+spec:
+  params:
+  - name: pl-param-x
+    value: "100"
+  - name: pl-param-y
+    value: "200"
+  - name: pl-param-z # the extra parameter
+    value: "300"
+  pipelineRef:
+    name: pipeline-with-extra-params

pkg/reconciler/pipelinerun/pipelinerun.go

@@ -80,6 +80,9 @@ const (
 	// ReasonCouldntGetCondition indicates that the reason for the failure status is that the
 	// associated Pipeline's Conditions couldn't all be retrieved
 	ReasonCouldntGetCondition = "CouldntGetCondition"
+	// ReasonParameterMissing indicates that the reason for the failure status is that the
+	// associated PipelineRun didn't provide all the required parameters
+	ReasonParameterMissing = "ParameterMissing"
 	// ReasonFailedValidation indicates that the reason for failure status is
 	// that pipelinerun failed runtime validation
 	ReasonFailedValidation = "PipelineValidationFailed"
@@ -315,6 +318,14 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun) err
 			pipelineMeta.Namespace, pr.Name, err)
 		return nil
 	}
+	// Ensure that the PipelineRun provides all the parameters required by the Pipeline
+	if err := resources.ValidateRequiredParametersProvided(&pipelineSpec.Params, &pr.Spec.Params); err != nil {
+		// This Run has failed, so we need to mark it as failed and stop reconciling it
+		pr.Status.MarkFailed(ReasonParameterMissing,
+			"PipelineRun %s parameters is missing some parameters required by Pipeline %s's parameters: %s",
+			pr.Namespace, pr.Name, err)
+		return nil
+	}
 
 	// Ensure that the parameters from the PipelineRun are overriding Pipeline parameters with the same type.
 	// Weird substitution issues can occur if this is not validated (ApplyParameters() does not verify type).

pkg/reconciler/pipelinerun/pipelinerun_test.go

@@ -434,6 +434,10 @@ func TestReconcile_InvalidPipelineRuns(t *testing.T) {
 			tb.PipelineTask("some-task", "a-task-that-needs-array-params")),
 			tb.PipelineRunParam("some-param", "stringval"),
 		)),
+		tb.PipelineRun("pipelinerun-missing-params", tb.PipelineRunNamespace("foo"), tb.PipelineRunSpec("", tb.PipelineRunPipelineSpec(
+			tb.PipelineParamSpec("some-param", v1beta1.ParamTypeString),
+			tb.PipelineTask("some-task", "a-task-that-needs-params")),
+		)),
 	}
 	d := test.Data{
 		Tasks:        ts,
@@ -491,6 +495,10 @@ func TestReconcile_InvalidPipelineRuns(t *testing.T) {
 			name:        "invalid-embedded-pipeline-mismatching-parameter-types",
 			pipelineRun: prs[10],
 			reason:      ReasonParameterTypeMismatch,
+		}, {
+			name:        "invalid-pipeline-run-missing-params-shd-stop-reconciling",
+			pipelineRun: prs[11],
+			reason:      ReasonParameterMissing,
 		},
 	}
 

pkg/reconciler/pipelinerun/resources/validate_params.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
+	"github.com/tektoncd/pipeline/pkg/list"
 )
 
 // ValidateParamTypesMatching validate that parameters in PipelineRun override corresponding parameters in Pipeline of the same type.
@@ -46,3 +47,29 @@ func ValidateParamTypesMatching(p *v1beta1.PipelineSpec, pr *v1beta1.PipelineRun
 	}
 	return nil
 }
+
+// ValidateRequiredParametersProvided validates that all the parameters expected by the Pipeline are provided by the PipelineRun.
+// Extra Parameters are allowed, the Pipeline will use the Parameters it needs and ignore the other Parameters.
+func ValidateRequiredParametersProvided(pipelineParameters *[]v1beta1.ParamSpec, pipelineRunParameters *[]v1beta1.Param) error {
+	// Build a list of parameter names declared in pr.
+	var providedParams []string
+	for _, param := range *pipelineRunParameters {
+		providedParams = append(providedParams, param.Name)
+	}
+
+	var requiredParams []string
+	for _, param := range *pipelineParameters {
+		if param.Default == nil { // include only parameters that don't have default values specified in the Pipeline
+			requiredParams = append(requiredParams, param.Name)
+		}
+	}
+
+	// Build a list of parameter names in p that are missing from pr.
+	missingParams := list.DiffLeft(requiredParams, providedParams)
+
+	// Return an error with the missing parameters' names, or return nil if there are none.
+	if len(missingParams) != 0 {
+		return fmt.Errorf("PipelineRun missing parameters: %s", missingParams)
+	}
+	return nil
+}

pkg/reconciler/pipelinerun/resources/validate_params_test.go

@@ -121,3 +121,104 @@ func TestValidateParamTypesMatching_Invalid(t *testing.T) {
 		})
 	}
 }
+
+func TestValidateRequiredParametersProvided_Valid(t *testing.T) {
+	tcs := []struct {
+		name string
+		pp   []v1beta1.ParamSpec
+		prp  []v1beta1.Param
+	}{{
+		name: "required string params provided",
+		pp: []v1beta1.ParamSpec{
+			{
+				Name: "required-string-param",
+				Type: v1beta1.ParamTypeString,
+			},
+		},
+		prp: []v1beta1.Param{
+			{
+				Name:  "required-string-param",
+				Value: *tb.ArrayOrString("somestring"),
+			},
+		},
+	}, {
+		name: "required array params provided",
+		pp: []v1beta1.ParamSpec{
+			{
+				Name: "required-array-param",
+				Type: v1beta1.ParamTypeArray,
+			},
+		},
+		prp: []v1beta1.Param{
+			{
+				Name:  "required-array-param",
+				Value: *tb.ArrayOrString("another", "array"),
+			},
+		},
+	}, {
+		name: "string params provided in default",
+		pp: []v1beta1.ParamSpec{
+			{
+				Name:    "string-param",
+				Type:    v1beta1.ParamTypeString,
+				Default: tb.ArrayOrString("somedefault"),
+			},
+		},
+		prp: []v1beta1.Param{
+			{
+				Name:  "another-string-param",
+				Value: *tb.ArrayOrString("somestring"),
+			},
+		},
+	}}
+	for _, tc := range tcs {
+		t.Run(tc.name, func(t *testing.T) {
+			if err := ValidateRequiredParametersProvided(&tc.pp, &tc.prp); err != nil {
+				t.Errorf("Didn't expect to see error when validating valid PipelineRun parameters but got: %v", err)
+			}
+		})
+	}
+}
+
+func TestValidateRequiredParametersProvided_Invalid(t *testing.T) {
+	tcs := []struct {
+		name string
+		pp   []v1beta1.ParamSpec
+		prp  []v1beta1.Param
+	}{{
+		name: "required string param missing",
+		pp: []v1beta1.ParamSpec{
+			{
+				Name: "required-string-param",
+				Type: v1beta1.ParamTypeString,
+			},
+		},
+		prp: []v1beta1.Param{
+			{
+				Name:  "another-string-param",
+				Value: *tb.ArrayOrString("anotherstring"),
+			},
+		},
+	}, {
+		name: "required array param missing",
+		pp: []v1beta1.ParamSpec{
+			{
+				Name: "required-array-param",
+				Type: v1beta1.ParamTypeArray,
+			},
+		},
+		prp: []v1beta1.Param{
+			{
+				Name:  "another-array-param",
+				Value: *tb.ArrayOrString("anotherstring"),
+			},
+		},
+	}}
+	for _, tc := range tcs {
+		t.Run(tc.name, func(t *testing.T) {
+			if err := ValidateRequiredParametersProvided(&tc.pp, &tc.prp); err == nil {
+				t.Errorf("Expected to see error when validating invalid PipelineRun parameters but saw none")
+			}
+		})
+	}
+}