diff --git a/docs/pipelineruns.md b/docs/pipelineruns.md index 8075beca360..46a2952eff5 100644 --- a/docs/pipelineruns.md +++ b/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 diff --git a/examples/v1beta1/pipelineruns/pipelinerun-with-extra-params.yaml b/examples/v1beta1/pipelineruns/pipelinerun-with-extra-params.yaml new file mode 100644 index 00000000000..05e948f1454 --- /dev/null +++ b/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 diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 963469363d0..c8599dc9267 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/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,15 @@ 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 + err = resources.ValidateRequiredParametersProvided(&pipelineSpec.Params, &pr.Spec.Params) + if 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). diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 42d7f7e65c4..057cdbfa6f4 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/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, }, } diff --git a/pkg/reconciler/pipelinerun/resources/validate_params.go b/pkg/reconciler/pipelinerun/resources/validate_params.go index 80b4d61c0cf..febfe3ee8e3 100644 --- a/pkg/reconciler/pipelinerun/resources/validate_params.go +++ b/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 +} diff --git a/pkg/reconciler/pipelinerun/resources/validate_params_test.go b/pkg/reconciler/pipelinerun/resources/validate_params_test.go index c9a963cbbd2..e9960984000 100644 --- a/pkg/reconciler/pipelinerun/resources/validate_params_test.go +++ b/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") + } + }) + } +}