diff --git a/docs/matrix.md b/docs/matrix.md index 479f96d0c32..fcffb8e7440 100644 --- a/docs/matrix.md +++ b/docs/matrix.md @@ -213,6 +213,20 @@ tasks: - name: param-two value: $(params.bar) # array replacement from array param ``` + +`Matrix.Params` supports whole array replacements from array `Parameters`. + +```yaml +tasks: +... +- name: task-4 + taskRef: + name: task-4 + matrix: + params: + - name: param-one + value: $(params.bar[*]) # whole array replacement from array param +``` #### Parameters in Matrix.Include.Params `Matrix.Include.Params` takes string replacements from `Parameters` of type String, Array or Object. diff --git a/examples/v1beta1/pipelineruns/alpha/pipelinerun-with-matrix-array-references.yaml b/examples/v1beta1/pipelineruns/alpha/pipelinerun-with-matrix-array-references.yaml new file mode 100644 index 00000000000..2982f139542 --- /dev/null +++ b/examples/v1beta1/pipelineruns/alpha/pipelinerun-with-matrix-array-references.yaml @@ -0,0 +1,59 @@ +apiVersion: tekton.dev/v1beta1 +kind: Task +metadata: + name: platform-browsers + annotations: + description: | + A task that does something cool with platforms and browsers +spec: + params: + - name: platform + default: '' + - name: browser + default: '' + steps: + - name: echo + image: alpine + script: | + echo "$(params.platform) and $(params.browser)" +--- +apiVersion: tekton.dev/v1beta1 +kind: PipelineRun +metadata: + generateName: matrixed-pr- +spec: + serviceAccountName: 'default' + params: + - name: platforms + value: + - linux + - mac + - windows + - name: browsers + value: + - chrome + - safari + - firefox + pipelineRef: + name: matrixed-pipeline +--- +apiVersion: tekton.dev/v1beta1 +kind: Pipeline +metadata: + name: matrixed-pipeline +spec: + params: + - name: platforms + type: array + - name: browsers + type: array + tasks: + - name: platforms-and-browsers + matrix: + params: + - name: platform + value: $(params.platforms[*]) + - name: browser + value: $(params.browsers[*]) + taskRef: + name: platform-browsers diff --git a/pkg/apis/pipeline/v1/matrix_types.go b/pkg/apis/pipeline/v1/matrix_types.go index 67fb8a6b811..f51a0ac4af4 100644 --- a/pkg/apis/pipeline/v1/matrix_types.go +++ b/pkg/apis/pipeline/v1/matrix_types.go @@ -300,29 +300,17 @@ func (m *Matrix) validateCombinationsCount(ctx context.Context) (errs *apis.Fiel return errs } -// validateParams validates the type of Parameter for Matrix.Params and Matrix.Include.Params -// Matrix.Params must be of type array. Matrix.Include.Params must be of type string. -// validateParams also validates Matrix.Params for a unique list of params +// validateUniqueParams validates Matrix.Params for a unique list of params // and a unique list of params in each Matrix.Include.Params specification -func (m *Matrix) validateParams() (errs *apis.FieldError) { +func (m *Matrix) validateUniqueParams() (errs *apis.FieldError) { if m != nil { if m.HasInclude() { for i, include := range m.Include { errs = errs.Also(include.Params.validateDuplicateParameters().ViaField(fmt.Sprintf("matrix.include[%d].params", i))) - for _, param := range include.Params { - if param.Value.Type != ParamTypeString { - errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("parameters of type string only are allowed, but got param type %s", string(param.Value.Type)), "").ViaFieldKey("matrix.include.params", param.Name)) - } - } } } if m.HasParams() { errs = errs.Also(m.Params.validateDuplicateParameters().ViaField("matrix.params")) - for _, param := range m.Params { - if param.Value.Type != ParamTypeArray { - errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("parameters of type array only are allowed, but got param type %s", string(param.Value.Type)), "").ViaFieldKey("matrix.params", param.Name)) - } - } } } return errs diff --git a/pkg/apis/pipeline/v1/pipeline_types.go b/pkg/apis/pipeline/v1/pipeline_types.go index f3515b31552..c6a57ccb19a 100644 --- a/pkg/apis/pipeline/v1/pipeline_types.go +++ b/pkg/apis/pipeline/v1/pipeline_types.go @@ -313,7 +313,7 @@ func (pt *PipelineTask) validateMatrix(ctx context.Context) (errs *apis.FieldErr errs = errs.Also(pt.Matrix.validateCombinationsCount(ctx)) } errs = errs.Also(pt.Matrix.validateParameterInOneOfMatrixOrParams(pt.Params)) - errs = errs.Also(pt.Matrix.validateParams()) + errs = errs.Also(pt.Matrix.validateUniqueParams()) return errs } diff --git a/pkg/apis/pipeline/v1/pipeline_types_test.go b/pkg/apis/pipeline/v1/pipeline_types_test.go index dbc29b5d302..642f7bcbf89 100644 --- a/pkg/apis/pipeline/v1/pipeline_types_test.go +++ b/pkg/apis/pipeline/v1/pipeline_types_test.go @@ -668,93 +668,37 @@ func TestPipelineTask_ValidateMatrix(t *testing.T) { Paths: []string{"matrix.include[0].params[1].name"}, }, }, { - name: "parameters in matrix are strings", + name: "parameters in matrix contain references to arrays", pt: &PipelineTask{ Name: "task", + Params: Params{{ + Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, + }, { + Name: "barfoo", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}}, + }}, Matrix: &Matrix{ Params: Params{{ - Name: "foo", Value: ParamValue{Type: ParamTypeString, StringVal: "foo"}, + Name: "foo", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.foobar[*])"}, }, { - Name: "bar", Value: ParamValue{Type: ParamTypeString, StringVal: "bar"}, + Name: "bar", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.barfoo[*])"}, }}}, }, - wantErrs: &apis.FieldError{ - Message: "invalid value: parameters of type array only are allowed, but got param type string", - Paths: []string{"matrix.params[foo]", "matrix.params[bar]"}, - }, }, { - name: "parameters in matrix are arrays", + name: "parameters in matrix contain results references", pt: &PipelineTask{ Name: "task", Matrix: &Matrix{ Params: Params{{ - Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, - }, { - Name: "barfoo", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}}, - }}}, - }, - }, { - name: "parameters in include matrix are strings", - pt: &PipelineTask{ - Name: "task", - Matrix: &Matrix{ - Include: IncludeParamsList{{ - Name: "build-1", - Params: Params{{ - Name: "IMAGE", Value: ParamValue{Type: ParamTypeString, StringVal: "image-1"}, - }, { - Name: "DOCKERFILE", Value: ParamValue{Type: ParamTypeString, StringVal: "path/to/Dockerfile1"}, - }}}, - }}, - }, - }, { - name: "parameters in include matrix are objects", - pt: &PipelineTask{ - Name: "task", - Matrix: &Matrix{ - Include: IncludeParamsList{{ - Name: "build-1", - Params: Params{{ - Name: "barfoo", Value: ParamValue{Type: ParamTypeObject, ObjectVal: map[string]string{ - "url": "$(params.myObject.non-exist-key)", - "commit": "$(params.myString)", - }}, - }, { - Name: "foobar", Value: ParamValue{Type: ParamTypeObject, ObjectVal: map[string]string{ - "url": "$(params.myObject.non-exist-key)", - "commit": "$(params.myString)", - }}, - }}, - }}}, - }, - wantErrs: &apis.FieldError{ - Message: "invalid value: parameters of type string only are allowed, but got param type object", - Paths: []string{"matrix.include.params[barfoo]", "matrix.include.params[foobar]"}, - }, - }, { - name: "parameters in include matrix are arrays", - pt: &PipelineTask{ - Name: "task", - Matrix: &Matrix{ - Include: IncludeParamsList{{ - Name: "build-1", - Params: Params{{ - Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, - }, { - Name: "barfoo", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}}}}, + Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.foo-task.results.a-result)"}}, }}}, }, - wantErrs: &apis.FieldError{ - Message: "invalid value: parameters of type string only are allowed, but got param type array", - Paths: []string{"matrix.include.params[barfoo]", "matrix.include.params[foobar]"}, - }, }, { - name: "parameters in matrix contain results references", + name: "parameters in matrix contain whole array results references", pt: &PipelineTask{ Name: "task", Matrix: &Matrix{ Params: Params{{ - Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.foo-task.results.a-result)"}}, + Name: "a-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(tasks.foo-task.results.[*])"}, }}}, }, }, { diff --git a/pkg/apis/pipeline/v1/pipeline_validation_test.go b/pkg/apis/pipeline/v1/pipeline_validation_test.go index 7482690323c..88611ddb4d9 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1/pipeline_validation_test.go @@ -3062,75 +3062,6 @@ func Test_validateMatrix(t *testing.T) { Name: "barfoo", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}}, }}, }}, - }, { - name: "parameters in matrix are strings", - tasks: PipelineTaskList{{ - Name: "a-task", - TaskRef: &TaskRef{Name: "a-task"}, - Matrix: &Matrix{ - Params: Params{{ - Name: "foo", Value: ParamValue{Type: ParamTypeString, StringVal: "foo"}, - }, { - Name: "bar", Value: ParamValue{Type: ParamTypeString, StringVal: "bar"}, - }}}, - }, { - Name: "b-task", - TaskRef: &TaskRef{Name: "b-task"}, - Matrix: &Matrix{ - Params: Params{{ - Name: "baz", Value: ParamValue{Type: ParamTypeString, StringVal: "baz"}, - }}}, - }}, - wantErrs: &apis.FieldError{ - Message: "invalid value: parameters of type array only are allowed, but got param type string", - Paths: []string{"[0].matrix.params[foo]", "[0].matrix.params[bar]", "[1].matrix.params[baz]"}, - }, - }, { - name: "parameters in matrix are arrays", - tasks: PipelineTaskList{{ - Name: "a-task", - TaskRef: &TaskRef{Name: "a-task"}, - Matrix: &Matrix{ - Params: Params{{ - Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, - }, { - Name: "barfoo", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}}, - }}}, - }}, - }, { - name: "parameters in include matrix are strings", - tasks: PipelineTaskList{{ - Name: "a-task", - TaskRef: &TaskRef{Name: "a-task"}, - Matrix: &Matrix{ - Include: IncludeParamsList{{ - Name: "test", - Params: Params{{ - Name: "foobar", Value: ParamValue{Type: ParamTypeString, StringVal: "foo"}, - }, { - Name: "barfoo", Value: ParamValue{Type: ParamTypeString, StringVal: "bar"}}, - }}}, - }, - }}, - }, { - name: "parameters in include matrix are arrays", - tasks: PipelineTaskList{{ - Name: "a-task", - TaskRef: &TaskRef{Name: "a-task"}, - Matrix: &Matrix{ - Include: IncludeParamsList{{ - Name: "test", - Params: Params{{ - Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo"}}, - }, { - Name: "barfoo", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"bar"}}}, - }}}, - }, - }}, - wantErrs: &apis.FieldError{ - Message: "invalid value: parameters of type string only are allowed, but got param type array", - Paths: []string{"[0].matrix.include.params[barfoo], [0].matrix.include.params[foobar]"}, - }, }, { name: "parameters in matrix contain results references", tasks: PipelineTaskList{{ diff --git a/pkg/apis/pipeline/v1beta1/matrix_types.go b/pkg/apis/pipeline/v1beta1/matrix_types.go index f1c86d4e062..19042e11e4b 100644 --- a/pkg/apis/pipeline/v1beta1/matrix_types.go +++ b/pkg/apis/pipeline/v1beta1/matrix_types.go @@ -300,29 +300,17 @@ func (m *Matrix) validateCombinationsCount(ctx context.Context) (errs *apis.Fiel return errs } -// validateParams validates the type of Parameter for Matrix.Params and Matrix.Include.Params -// Matrix.Params must be of type array. Matrix.Include.Params must be of type string. -// validateParams also validates Matrix.Params for a unique list of params +// validateUniqueParams validates Matrix.Params for a unique list of params // and a unique list of params in each Matrix.Include.Params specification -func (m *Matrix) validateParams() (errs *apis.FieldError) { +func (m *Matrix) validateUniqueParams() (errs *apis.FieldError) { if m != nil { if m.HasInclude() { for i, include := range m.Include { errs = errs.Also(include.Params.validateDuplicateParameters().ViaField(fmt.Sprintf("matrix.include[%d].params", i))) - for _, param := range include.Params { - if param.Value.Type != ParamTypeString { - errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("parameters of type string only are allowed, but got param type %s", string(param.Value.Type)), "").ViaFieldKey("matrix.include.params", param.Name)) - } - } } } if m.HasParams() { errs = errs.Also(m.Params.validateDuplicateParameters().ViaField("matrix.params")) - for _, param := range m.Params { - if param.Value.Type != ParamTypeArray { - errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("parameters of type array only are allowed, but got param type %s", string(param.Value.Type)), "").ViaFieldKey("matrix.params", param.Name)) - } - } } } return errs diff --git a/pkg/apis/pipeline/v1beta1/pipeline_types.go b/pkg/apis/pipeline/v1beta1/pipeline_types.go index 7f3256387d6..a8ed1f44adb 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_types.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_types.go @@ -336,7 +336,7 @@ func (pt *PipelineTask) validateMatrix(ctx context.Context) (errs *apis.FieldErr errs = errs.Also(pt.Matrix.validateCombinationsCount(ctx)) } errs = errs.Also(pt.Matrix.validateParameterInOneOfMatrixOrParams(pt.Params)) - errs = errs.Also(pt.Matrix.validateParams()) + errs = errs.Also(pt.Matrix.validateUniqueParams()) return errs } diff --git a/pkg/apis/pipeline/v1beta1/pipeline_types_test.go b/pkg/apis/pipeline/v1beta1/pipeline_types_test.go index 43c13668512..5220f955a93 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_types_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_types_test.go @@ -623,32 +623,6 @@ func TestPipelineTask_validateMatrix(t *testing.T) { Name: "barfoo", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}}, }}, }, - }, { - name: "parameters in matrix are strings", - pt: &PipelineTask{ - Name: "task", - Matrix: &Matrix{ - Params: Params{{ - Name: "foo", Value: ParamValue{Type: ParamTypeString, StringVal: "foo"}, - }, { - Name: "bar", Value: ParamValue{Type: ParamTypeString, StringVal: "bar"}, - }}}, - }, - wantErrs: &apis.FieldError{ - Message: "invalid value: parameters of type array only are allowed, but got param type string", - Paths: []string{"matrix.params[foo]", "matrix.params[bar]"}, - }, - }, { - name: "parameters in matrix are arrays", - pt: &PipelineTask{ - Name: "task", - Matrix: &Matrix{ - Params: Params{{ - Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, - }, { - Name: "barfoo", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}}, - }}}, - }, }, { name: "duplicate parameters in matrix.include.params", pt: &PipelineTask{ @@ -668,67 +642,37 @@ func TestPipelineTask_validateMatrix(t *testing.T) { Paths: []string{"matrix.include[0].params[1].name"}, }, }, { - name: "parameters in include matrix are strings", - pt: &PipelineTask{ - Name: "task", - Matrix: &Matrix{ - Include: IncludeParamsList{{ - Name: "build-1", - Params: Params{{ - Name: "IMAGE", Value: ParamValue{Type: ParamTypeString, StringVal: "image-1"}, - }, { - Name: "DOCKERFILE", Value: ParamValue{Type: ParamTypeString, StringVal: "path/to/Dockerfile1"}, - }}}, - }}, - }, - }, { - name: "parameters in include matrix are objects", + name: "parameters in matrix contain references to arrays", pt: &PipelineTask{ Name: "task", + Params: Params{{ + Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, + }, { + Name: "barfoo", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}}, + }}, Matrix: &Matrix{ - Include: IncludeParamsList{{ - Name: "build-1", - Params: Params{{ - Name: "barfoo", Value: ParamValue{Type: ParamTypeObject, ObjectVal: map[string]string{ - "url": "$(params.myObject.non-exist-key)", - "commit": "$(params.myString)", - }}, - }, { - Name: "foobar", Value: ParamValue{Type: ParamTypeObject, ObjectVal: map[string]string{ - "url": "$(params.myObject.non-exist-key)", - "commit": "$(params.myString)", - }}, - }}, + Params: Params{{ + Name: "foo", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.foobar[*])"}, + }, { + Name: "bar", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.barfoo[*])"}, }}}, }, - wantErrs: &apis.FieldError{ - Message: "invalid value: parameters of type string only are allowed, but got param type object", - Paths: []string{"matrix.include.params[barfoo]", "matrix.include.params[foobar]"}, - }, }, { - name: "parameters in include matrix are arrays", + name: "parameters in matrix contain results references", pt: &PipelineTask{ Name: "task", Matrix: &Matrix{ - Include: IncludeParamsList{{ - Name: "build-1", - Params: Params{{ - Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, - }, { - Name: "barfoo", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}}}}, + Params: Params{{ + Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.foo-task.results.a-result)"}}, }}}, }, - wantErrs: &apis.FieldError{ - Message: "invalid value: parameters of type string only are allowed, but got param type array", - Paths: []string{"matrix.include.params[barfoo]", "matrix.include.params[foobar]"}, - }, }, { - name: "parameters in matrix contain results references", + name: "parameters in matrix contain whole array results references", pt: &PipelineTask{ Name: "task", Matrix: &Matrix{ Params: Params{{ - Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.foo-task.results.a-result)"}}, + Name: "a-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(tasks.foo-task.results.[*])"}, }}}, }, }, { diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go index e9d539c0a59..830a56819e5 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go @@ -3106,75 +3106,6 @@ func Test_validateMatrix(t *testing.T) { Name: "barfoo", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}}, }}, }}, - }, { - name: "parameters in matrix are strings", - tasks: PipelineTaskList{{ - Name: "a-task", - TaskRef: &TaskRef{Name: "a-task"}, - Matrix: &Matrix{ - Params: Params{{ - Name: "foo", Value: ParamValue{Type: ParamTypeString, StringVal: "foo"}, - }, { - Name: "bar", Value: ParamValue{Type: ParamTypeString, StringVal: "bar"}, - }}}, - }, { - Name: "b-task", - TaskRef: &TaskRef{Name: "b-task"}, - Matrix: &Matrix{ - Params: Params{{ - Name: "baz", Value: ParamValue{Type: ParamTypeString, StringVal: "baz"}, - }}}, - }}, - wantErrs: &apis.FieldError{ - Message: "invalid value: parameters of type array only are allowed, but got param type string", - Paths: []string{"[0].matrix.params[foo]", "[0].matrix.params[bar]", "[1].matrix.params[baz]"}, - }, - }, { - name: "parameters in matrix are arrays", - tasks: PipelineTaskList{{ - Name: "a-task", - TaskRef: &TaskRef{Name: "a-task"}, - Matrix: &Matrix{ - Params: Params{{ - Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, - }, { - Name: "barfoo", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}}, - }}}, - }}, - }, { - name: "parameters in include matrix are strings", - tasks: PipelineTaskList{{ - Name: "a-task", - TaskRef: &TaskRef{Name: "a-task"}, - Matrix: &Matrix{ - Include: IncludeParamsList{{ - Name: "test", - Params: Params{{ - Name: "foobar", Value: ParamValue{Type: ParamTypeString, StringVal: "foo"}, - }, { - Name: "barfoo", Value: ParamValue{Type: ParamTypeString, StringVal: "bar"}}, - }}}, - }, - }}, - }, { - name: "parameters in include matrix are arrays", - tasks: PipelineTaskList{{ - Name: "a-task", - TaskRef: &TaskRef{Name: "a-task"}, - Matrix: &Matrix{ - Include: IncludeParamsList{{ - Name: "test", - Params: Params{{ - Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo"}}, - }, { - Name: "barfoo", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"bar"}}}, - }}}, - }, - }}, - wantErrs: &apis.FieldError{ - Message: "invalid value: parameters of type string only are allowed, but got param type array", - Paths: []string{"[0].matrix.include.params[barfoo], [0].matrix.include.params[foobar]"}, - }, }, { name: "parameters in matrix contain results references", tasks: PipelineTaskList{{ diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 4cc2f12ea68..edc6a4de3cc 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -569,6 +569,12 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get } if pipelineRunFacts.State.IsBeforeFirstTaskRun() { + if err := resources.ValidateMatrixPipelineParameterTypes(pipelineRunFacts.State); err != nil { + logger.Errorf("Failed to validate matrix %q with error %v", pr.Name, err) + pr.Status.MarkFailed(ReasonInvalidTaskResultReference, err.Error()) + return controller.NewPermanentError(err) + } + if err := resources.ValidatePipelineTaskResults(pipelineRunFacts.State); err != nil { logger.Errorf("Failed to resolve task result reference for %q with error %v", pr.Name, err) pr.Status.MarkFailed(ReasonInvalidTaskResultReference, err.Error()) diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index a0c91bd0f1f..78fa37bad81 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -7796,6 +7796,351 @@ spec: }) } } +func TestReconciler_PipelineTaskMatrixWithArrayReferences(t *testing.T) { + names.TestingSeed() + + task := parse.MustParseV1beta1Task(t, ` +metadata: + name: mytask + namespace: foo +spec: + params: + - name: platform + - name: browser + steps: + - name: echo + image: alpine + script: | + echo "$(params.platform) and $(params.browser)" +`) + + expectedTaskRuns := []*v1beta1.TaskRun{ + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-platforms-and-browsers-0", "foo", + "pr", "p", "platforms-and-browsers", false), + ` +spec: + params: + - name: browser + value: chrome + - name: platform + value: linux + serviceAccountName: test-sa + taskRef: + name: mytask + kind: Task +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-platforms-and-browsers-1", "foo", + "pr", "p", "platforms-and-browsers", false), + ` +spec: + params: + - name: browser + value: chrome + - name: platform + value: mac + serviceAccountName: test-sa + taskRef: + name: mytask + kind: Task +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-platforms-and-browsers-2", "foo", + "pr", "p", "platforms-and-browsers", false), + ` +spec: + params: + - name: browser + value: chrome + - name: platform + value: windows + serviceAccountName: test-sa + taskRef: + name: mytask + kind: Task +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-platforms-and-browsers-3", "foo", + "pr", "p", "platforms-and-browsers", false), + ` +spec: + params: + - name: browser + value: safari + - name: platform + value: linux + serviceAccountName: test-sa + taskRef: + name: mytask + kind: Task +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-platforms-and-browsers-4", "foo", + "pr", "p", "platforms-and-browsers", false), + ` +spec: + params: + - name: browser + value: safari + - name: platform + value: mac + serviceAccountName: test-sa + taskRef: + name: mytask + kind: Task +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-platforms-and-browsers-5", "foo", + "pr", "p", "platforms-and-browsers", false), + ` +spec: + params: + - name: browser + value: safari + - name: platform + value: windows + serviceAccountName: test-sa + taskRef: + name: mytask + kind: Task +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-platforms-and-browsers-6", "foo", + "pr", "p", "platforms-and-browsers", false), + ` +spec: + params: + - name: browser + value: firefox + - name: platform + value: linux + serviceAccountName: test-sa + taskRef: + name: mytask + kind: Task +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-platforms-and-browsers-7", "foo", + "pr", "p", "platforms-and-browsers", false), + ` +spec: + params: + - name: browser + value: firefox + - name: platform + value: mac + serviceAccountName: test-sa + taskRef: + name: mytask + kind: Task +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-platforms-and-browsers-8", "foo", + "pr", "p", "platforms-and-browsers", false), + ` +spec: + params: + - name: browser + value: firefox + - name: platform + value: windows + serviceAccountName: test-sa + taskRef: + name: mytask + kind: Task +`), + } + cms := []*corev1.ConfigMap{withEnabledAlphaAPIFields(newFeatureFlagsConfigMap())} + cms = append(cms, withMaxMatrixCombinationsCount(newDefaultsConfigMap(), 10)) + + tests := []struct { + name string + memberOf string + p *v1beta1.Pipeline + tr *v1beta1.TaskRun + expectedPipelineRun *v1beta1.PipelineRun + }{{ + name: "p-dag", + memberOf: "tasks", + p: parse.MustParseV1beta1Pipeline(t, fmt.Sprintf(` +metadata: + name: %s + namespace: foo +spec: + params: + - name: platforms + type: array + - name: browsers + type: array + tasks: + - name: platforms-and-browsers + taskRef: + name: mytask + matrix: + params: + - name: platform + value: $(params.platforms[*]) + - name: browser + value: $(params.browsers[*]) + +`, "p-dag")), + expectedPipelineRun: parse.MustParseV1beta1PipelineRun(t, ` +metadata: + name: pr + namespace: foo + annotations: {} + labels: + tekton.dev/pipeline: p-dag +spec: + params: + - name: platforms + value: + - linux + - mac + - windows + - name: browsers + value: + - chrome + - safari + - firefox + serviceAccountName: test-sa + pipelineRef: + name: p-dag +status: + pipelineSpec: + params: + - name: platforms + type: array + - name: browsers + type: array + tasks: + - name: platforms-and-browsers + matrix: + params: + - name: platform + value: + - linux + - mac + - windows + - name: browser + value: + - chrome + - safari + - firefox + taskRef: + name: mytask + kind: Task + conditions: + - type: Succeeded + status: "Unknown" + reason: "Running" + message: "Tasks Completed: 0 (Failed: 0, Cancelled 0), Incomplete: 1, Skipped: 0" + childReferences: + - apiVersion: tekton.dev/v1beta1 + kind: TaskRun + name: pr-platforms-and-browsers-0 + pipelineTaskName: platforms-and-browsers + - apiVersion: tekton.dev/v1beta1 + kind: TaskRun + name: pr-platforms-and-browsers-1 + pipelineTaskName: platforms-and-browsers + - apiVersion: tekton.dev/v1beta1 + kind: TaskRun + name: pr-platforms-and-browsers-2 + pipelineTaskName: platforms-and-browsers + - apiVersion: tekton.dev/v1beta1 + kind: TaskRun + name: pr-platforms-and-browsers-3 + pipelineTaskName: platforms-and-browsers + - apiVersion: tekton.dev/v1beta1 + kind: TaskRun + name: pr-platforms-and-browsers-4 + pipelineTaskName: platforms-and-browsers + - apiVersion: tekton.dev/v1beta1 + kind: TaskRun + name: pr-platforms-and-browsers-5 + pipelineTaskName: platforms-and-browsers + - apiVersion: tekton.dev/v1beta1 + kind: TaskRun + name: pr-platforms-and-browsers-6 + pipelineTaskName: platforms-and-browsers + - apiVersion: tekton.dev/v1beta1 + kind: TaskRun + name: pr-platforms-and-browsers-7 + pipelineTaskName: platforms-and-browsers + - apiVersion: tekton.dev/v1beta1 + kind: TaskRun + name: pr-platforms-and-browsers-8 + pipelineTaskName: platforms-and-browsers +`), + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pr := parse.MustParseV1beta1PipelineRun(t, fmt.Sprintf(` +metadata: + name: pr + namespace: foo +spec: + serviceAccountName: test-sa + params: + - name: platforms + value: + - linux + - mac + - windows + - name: browsers + value: + - chrome + - safari + - firefox + pipelineRef: + name: %s +`, tt.name)) + d := test.Data{ + PipelineRuns: []*v1beta1.PipelineRun{pr}, + Pipelines: []*v1beta1.Pipeline{tt.p}, + Tasks: []*v1beta1.Task{task}, + ConfigMaps: cms, + } + if tt.tr != nil { + d.TaskRuns = []*v1beta1.TaskRun{tt.tr} + } + prt := newPipelineRunTest(t, d) + defer prt.Cancel() + + _, clients := prt.reconcileRun("foo", "pr", []string{}, false) + taskRuns, err := clients.Pipeline.TektonV1beta1().TaskRuns("foo").List(prt.TestAssets.Ctx, metav1.ListOptions{ + LabelSelector: fmt.Sprintf("tekton.dev/pipelineRun=pr,tekton.dev/pipeline=%s,tekton.dev/pipelineTask=platforms-and-browsers", tt.name), + Limit: 1, + }) + if err != nil { + t.Fatalf("Failure to list TaskRun's %s", err) + } + + if len(taskRuns.Items) != 9 { + t.Fatalf("Expected 9 TaskRuns got %d", len(taskRuns.Items)) + } + + for i := range taskRuns.Items { + expectedTaskRun := expectedTaskRuns[i] + expectedTaskRun.Labels["tekton.dev/pipeline"] = tt.name + expectedTaskRun.Labels["tekton.dev/memberOf"] = tt.memberOf + if d := cmp.Diff(expectedTaskRun, &taskRuns.Items[i], ignoreResourceVersion, ignoreTypeMeta); d != "" { + t.Errorf("expected to see TaskRun %v created. Diff %s", expectedTaskRuns[i].Name, diff.PrintWantGot(d)) + } + } + + pipelineRun, err := clients.Pipeline.TektonV1beta1().PipelineRuns("foo").Get(prt.TestAssets.Ctx, "pr", metav1.GetOptions{}) + if err != nil { + t.Fatalf("Got an error getting reconciled run out of fake client: %s", err) + } + if d := cmp.Diff(tt.expectedPipelineRun, pipelineRun, ignoreResourceVersion, ignoreTypeMeta, ignoreLastTransitionTime, ignoreStartTime, ignoreFinallyStartTime, cmpopts.EquateEmpty()); d != "" { + t.Errorf("expected PipelineRun was not created. Diff %s", diff.PrintWantGot(d)) + } + }) + } +} func TestReconciler_PipelineTaskIncludeParams(t *testing.T) { names.TestingSeed() @@ -8613,8 +8958,6 @@ spec: value: chrome - name: platform value: linux - - name: version - value: v0.33.0 serviceAccountName: test-sa taskRef: name: mytask @@ -8630,8 +8973,6 @@ spec: value: chrome - name: platform value: mac - - name: version - value: v0.33.0 serviceAccountName: test-sa taskRef: name: mytask @@ -8647,8 +8988,6 @@ spec: value: chrome - name: platform value: windows - - name: version - value: v0.33.0 serviceAccountName: test-sa taskRef: name: mytask @@ -8664,8 +9003,6 @@ spec: value: safari - name: platform value: linux - - name: version - value: v0.33.0 serviceAccountName: test-sa taskRef: name: mytask @@ -8681,8 +9018,6 @@ spec: value: safari - name: platform value: mac - - name: version - value: v0.33.0 serviceAccountName: test-sa taskRef: name: mytask @@ -8698,8 +9033,6 @@ spec: value: safari - name: platform value: windows - - name: version - value: v0.33.0 serviceAccountName: test-sa taskRef: name: mytask @@ -8715,8 +9048,6 @@ spec: value: firefox - name: platform value: linux - - name: version - value: v0.33.0 serviceAccountName: test-sa taskRef: name: mytask @@ -8732,8 +9063,6 @@ spec: value: firefox - name: platform value: mac - - name: version - value: v0.33.0 serviceAccountName: test-sa taskRef: name: mytask @@ -8749,8 +9078,6 @@ spec: value: firefox - name: platform value: windows - - name: version - value: v0.33.0 serviceAccountName: test-sa taskRef: name: mytask diff --git a/pkg/reconciler/pipelinerun/resources/validate_dependencies.go b/pkg/reconciler/pipelinerun/resources/validate_dependencies.go index 11385b1e15e..6600e5a1906 100644 --- a/pkg/reconciler/pipelinerun/resources/validate_dependencies.go +++ b/pkg/reconciler/pipelinerun/resources/validate_dependencies.go @@ -23,6 +23,32 @@ import ( "k8s.io/apimachinery/pkg/util/sets" ) +// ValidateMatrixPipelineParameterTypes validates the type of Parameter for Matrix.Params +// and Matrix.Include.Params after any replacements are made from Task parameters or results +// Matrix.Params must be of type array. Matrix.Include.Params must be of type string. +func ValidateMatrixPipelineParameterTypes(state PipelineRunState) error { + for _, rpt := range state { + m := rpt.PipelineTask.Matrix + if m.HasInclude() { + for _, include := range m.Include { + for _, param := range include.Params { + if param.Value.Type != v1beta1.ParamTypeString { + return fmt.Errorf("parameters of type string only are allowed, but got param type %s", string(param.Value.Type)) + } + } + } + } + if m.HasParams() { + for _, param := range m.Params { + if param.Value.Type != v1beta1.ParamTypeArray { + return fmt.Errorf("parameters of type array only are allowed, but got param type %s", string(param.Value.Type)) + } + } + } + } + return nil +} + // ValidatePipelineTaskResults ensures that any result references used by pipeline tasks // resolve to valid results. This prevents a situation where a PipelineTask references // a result in another PipelineTask that doesn't exist or where the user has either misspelled diff --git a/pkg/reconciler/pipelinerun/resources/validate_dependencies_test.go b/pkg/reconciler/pipelinerun/resources/validate_dependencies_test.go index 34604dabc18..4bbff901088 100644 --- a/pkg/reconciler/pipelinerun/resources/validate_dependencies_test.go +++ b/pkg/reconciler/pipelinerun/resources/validate_dependencies_test.go @@ -25,6 +25,107 @@ import ( "k8s.io/apimachinery/pkg/selection" ) +// ValidateMatrixPipelineParameterTypes tests that a pipeline task with +// a matrix has the correct parameter types after any replacements are made +func TestValidatePipelineParameterTypes(t *testing.T) { + for _, tc := range []struct { + desc string + state PipelineRunState + wantErrs string + }{{ + desc: "parameters in matrix are arrays", + state: PipelineRunState{{ + PipelineTask: &v1beta1.PipelineTask{ + Name: "task", + Matrix: &v1beta1.Matrix{ + Params: v1beta1.Params{{ + Name: "foobar", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, + }, { + Name: "barfoo", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeArray, ArrayVal: []string{"bar", "foo"}}}}, + }, + }, + }}, + }, { + desc: "parameters in matrix are strings", + state: PipelineRunState{{ + PipelineTask: &v1beta1.PipelineTask{ + Name: "task", + Matrix: &v1beta1.Matrix{ + Params: v1beta1.Params{{ + Name: "foo", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeString, StringVal: "foo"}, + }, { + Name: "bar", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeString, StringVal: "bar"}, + }}}, + }, + }}, + wantErrs: "parameters of type array only are allowed, but got param type string", + }, { + desc: "parameters in include matrix are strings", + state: PipelineRunState{{ + PipelineTask: &v1beta1.PipelineTask{ + Name: "task", + Matrix: &v1beta1.Matrix{ + Include: v1beta1.IncludeParamsList{{ + Name: "build-1", + Params: v1beta1.Params{{ + Name: "foo", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeString, StringVal: "foo"}, + }, { + Name: "bar", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeString, StringVal: "bar"}}}, + }}}, + }, + }}, + }, { + desc: "parameters in include matrix are arrays", + state: PipelineRunState{{ + PipelineTask: &v1beta1.PipelineTask{ + Name: "task", + Matrix: &v1beta1.Matrix{ + Include: v1beta1.IncludeParamsList{{ + Name: "build-1", + Params: v1beta1.Params{{ + Name: "foobar", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, + }, { + Name: "barfoo", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeArray, ArrayVal: []string{"bar", "foo"}}}}, + }}}, + }, + }}, + wantErrs: "parameters of type string only are allowed, but got param type array", + }, { + desc: "parameters in include matrix are objects", + state: PipelineRunState{{ + PipelineTask: &v1beta1.PipelineTask{ + Name: "task", + Matrix: &v1beta1.Matrix{ + Include: v1beta1.IncludeParamsList{{ + Name: "build-1", + Params: v1beta1.Params{{ + Name: "barfoo", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeObject, ObjectVal: map[string]string{ + "url": "$(params.myObject.non-exist-key)", + "commit": "$(params.myString)", + }}, + }, { + Name: "foobar", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeObject, ObjectVal: map[string]string{ + "url": "$(params.myObject.non-exist-key)", + "commit": "$(params.myString)", + }}, + }}, + }}}, + }, + }}, + wantErrs: "parameters of type string only are allowed, but got param type object", + }} { + t.Run(tc.desc, func(t *testing.T) { + err := ValidateMatrixPipelineParameterTypes(tc.state) + if (err != nil) && err.Error() != tc.wantErrs { + t.Errorf("expected err: %s, but got err %s", tc.wantErrs, err) + } + if tc.wantErrs == "" && err != nil { + t.Errorf("got unexpected error: %v", err) + } + }) + } +} + // TestValidatePipelineTaskResults_ValidStates tests that a pipeline task with // valid content and result variables does not trigger validation errors. func TestValidatePipelineTaskResults_ValidStates(t *testing.T) {