From de468b725ece8b0748495e0527bb55b92dd06a58 Mon Sep 17 00:00:00 2001 From: Emma Munley Date: Mon, 10 Apr 2023 15:37:51 -0400 Subject: [PATCH] Add support for using PipelineRun parameters as matrix parameter values This commit adds support for using PipelineRun parameters as matrix parameter values by moving the validation for checking matrix parameter types to after parameter substitution. This is necessary for allowing whole array substitutions using the syntax $(params.foo[*]), rather than an explicit YAML array, to be used as a matrix parameter. Prior to this, validation fails because a string referencing an array type was considered a string type and this was done before the applying array replacements. Note: Whole array replacements for matrix results has not yet been implemented and will be added in a subsequent PR. This issue is described in more detail here: #6056 --- docs/matrix.md | 14 + ...elinerun-with-matrix-array-references.yaml | 69 +++ pkg/apis/pipeline/v1/matrix_types.go | 37 +- pkg/apis/pipeline/v1/pipeline_types_test.go | 79 +--- pkg/apis/pipeline/v1/pipeline_validation.go | 3 +- .../pipeline/v1/pipeline_validation_test.go | 69 --- pkg/apis/pipeline/v1beta1/matrix_types.go | 35 +- .../pipeline/v1beta1/pipeline_types_test.go | 89 +--- .../pipeline/v1beta1/pipeline_validation.go | 3 +- .../v1beta1/pipeline_validation_test.go | 69 --- pkg/reconciler/pipelinerun/pipelinerun.go | 11 + .../pipelinerun/pipelinerun_test.go | 441 +++++++++++++++++- .../resources/pipelinerunresolution.go | 2 +- .../pipelinerun/resources/validate_params.go | 26 ++ .../resources/validate_params_test.go | 101 ++++ 15 files changed, 729 insertions(+), 319 deletions(-) create mode 100644 examples/v1beta1/pipelineruns/alpha/pipelinerun-with-matrix-array-references.yaml 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..7ef31481a7c --- /dev/null +++ b/examples/v1beta1/pipelineruns/alpha/pipelinerun-with-matrix-array-references.yaml @@ -0,0 +1,69 @@ +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: 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 + finally: + - name: platforms-and-browsers-in-finally + matrix: + params: + - name: platform + value: $(params.platforms[*]) + - name: browser + value: $(params.browsers[*]) + taskRef: + name: platform-browsers +--- +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 diff --git a/pkg/apis/pipeline/v1/matrix_types.go b/pkg/apis/pipeline/v1/matrix_types.go index 67fb8a6b811..03fba3f29aa 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 @@ -360,3 +348,24 @@ func (m *Matrix) validateParameterInOneOfMatrixOrParams(params []Param) (errs *a } return errs } + +// validateNoWholeArrayResults() is used to ensure a matrix parameter does not contain result references +// to entire arrays. This is temporary until whole array replacements for matrix parameters are supported. +// See issue #6056 for more details +func (m *Matrix) validateNoWholeArrayResults() (errs *apis.FieldError) { + if m.HasParams() { + for i, param := range m.Params { + val := param.Value.StringVal + expressions, ok := GetVarSubstitutionExpressionsForParam(param) + if ok { + if LooksLikeContainsResultRefs(expressions) { + _, stringIdx := ParseResultName(val) + if stringIdx == "*" { + errs = errs.Also(apis.ErrGeneric("matrix parameters cannot contain whole array result references", "").ViaFieldIndex("matrix.params", i)) + } + } + } + } + } + return errs +} diff --git a/pkg/apis/pipeline/v1/pipeline_types_test.go b/pkg/apis/pipeline/v1/pipeline_types_test.go index dbc29b5d302..56a48bf37a0 100644 --- a/pkg/apis/pipeline/v1/pipeline_types_test.go +++ b/pkg/apis/pipeline/v1/pipeline_types_test.go @@ -668,94 +668,51 @@ 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 results references", pt: &PipelineTask{ Name: "task", Matrix: &Matrix{ Params: Params{{ - Name: "foo", Value: ParamValue{Type: ParamTypeString, StringVal: "foo"}, - }, { - Name: "bar", Value: ParamValue{Type: ParamTypeString, StringVal: "bar"}, + Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.foo-task.results.a-result)"}}, }}}, }, - 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 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: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, + Name: "foo", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.foobar[*])"}, }, { - Name: "barfoo", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}}, + Name: "bar", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.barfoo[*])"}, }}}, }, }, { - 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 results references", 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)", - }}, - }}, + 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 object", - Paths: []string{"matrix.include.params[barfoo]", "matrix.include.params[foobar]"}, - }, }, { - name: "parameters in include matrix are arrays", + name: "parameters in matrix contain whole array 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: ParamTypeString, StringVal: "$(tasks.foo-task.results.arr-results[*])"}, }}}, }, 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", - pt: &PipelineTask{ - Name: "task", - Matrix: &Matrix{ - Params: Params{{ - Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.foo-task.results.a-result)"}}, - }}}, + Message: "matrix parameters cannot contain whole array result references", + Paths: []string{"matrix.params[0]"}, }, }, { name: "count of combinations of parameters in the matrix exceeds the maximum", diff --git a/pkg/apis/pipeline/v1/pipeline_validation.go b/pkg/apis/pipeline/v1/pipeline_validation.go index 0c60fae33bd..4ac2664dbce 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1/pipeline_validation.go @@ -145,9 +145,10 @@ func (pt *PipelineTask) validateMatrix(ctx context.Context) (errs *apis.FieldErr // when the enable-api-fields feature gate is anything but "alpha". errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "matrix", config.AlphaAPIFields)) errs = errs.Also(pt.Matrix.validateCombinationsCount(ctx)) + errs = errs.Also(pt.Matrix.validateNoWholeArrayResults()) + errs = errs.Also(pt.Matrix.validateUniqueParams()) } errs = errs.Also(pt.Matrix.validateParameterInOneOfMatrixOrParams(pt.Params)) - errs = errs.Also(pt.Matrix.validateParams()) return errs } 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..ac44ff6891f 100644 --- a/pkg/apis/pipeline/v1beta1/matrix_types.go +++ b/pkg/apis/pipeline/v1beta1/matrix_types.go @@ -17,6 +17,7 @@ import ( "context" "fmt" "sort" + "strings" "github.com/tektoncd/pipeline/pkg/apis/config" "golang.org/x/exp/maps" @@ -300,29 +301,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 @@ -360,3 +349,21 @@ func (m *Matrix) validateParameterInOneOfMatrixOrParams(params Params) (errs *ap } return errs } + +// validateNoWholeArrayResults() is used to ensure a matrix parameter does not contain result references +// to entire arrays. This is temporary until whole array replacements for matrix paraemeters are supported. +// See issue #6056 for more details +func (m *Matrix) validateNoWholeArrayResults() (errs *apis.FieldError) { + if m.HasParams() { + for i, param := range m.Params { + val := param.Value.StringVal + expressions, ok := GetVarSubstitutionExpressionsForParam(param) + if ok { + if LooksLikeContainsResultRefs(expressions) && strings.Contains(val, "[*]") { + errs = errs.Also(apis.ErrGeneric("matrix parameters cannot contain whole array result references", "").ViaFieldIndex("matrix.params", i)) + } + } + } + } + return errs +} diff --git a/pkg/apis/pipeline/v1beta1/pipeline_types_test.go b/pkg/apis/pipeline/v1beta1/pipeline_types_test.go index 43c13668512..3648e239920 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,69 +642,52 @@ func TestPipelineTask_validateMatrix(t *testing.T) { Paths: []string{"matrix.include[0].params[1].name"}, }, }, { - name: "parameters in include matrix are strings", + name: "parameters in matrix contain results references", 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"}, - }}}, - }}, + Params: Params{{ + Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.foo-task.results.a-result)"}}, + }}}, }, }, { - 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.arr-results[*])"}, }}}, }, + wantErrs: &apis.FieldError{ + Message: "matrix parameters cannot contain whole array result references", + Paths: []string{"matrix.params[0]"}, + }, }, { name: "count of combinations of parameters in the matrix exceeds the maximum", pt: &PipelineTask{ diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go index c170e85a3b9..fa9208647c5 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -157,9 +157,10 @@ func (pt *PipelineTask) validateMatrix(ctx context.Context) (errs *apis.FieldErr // when the enable-api-fields feature gate is anything but "alpha". errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "matrix", config.AlphaAPIFields)) errs = errs.Also(pt.Matrix.validateCombinationsCount(ctx)) + errs = errs.Also(pt.Matrix.validateNoWholeArrayResults()) + errs = errs.Also(pt.Matrix.validateUniqueParams()) } errs = errs.Also(pt.Matrix.validateParameterInOneOfMatrixOrParams(pt.Params)) - errs = errs.Also(pt.Matrix.validateParams()) return errs } 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 50023b58bc2..39f89587e43 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -113,6 +113,8 @@ const ( // ReasonCouldntTimeOut indicates that a PipelineRun was timed out but attempting to update // all of the running TaskRuns as timed out failed. ReasonCouldntTimeOut = "PipelineRunCouldntTimeOut" + // ReasonInvalidMatrixParameterTypes indicates a matrix contains invalid parameter types + ReasonInvalidMatrixParameterTypes = "ReasonInvalidMatrixParameterTypes" // ReasonInvalidTaskResultReference indicates a task result was declared // but was not initialized by that task ReasonInvalidTaskResultReference = "InvalidTaskResultReference" @@ -754,6 +756,15 @@ func (c *Reconciler) runNextSchedulableTask(ctx context.Context, pr *v1beta1.Pip continue } + // Validate parameter types in matrix after apply substitutions from Task Results + if rpt.PipelineTask.IsMatrixed() { + if err := resources.ValidateParameterTypesInMatrix(pipelineRunFacts.State); err != nil { + logger.Errorf("Failed to validate matrix %q with error %v", pr.Name, err) + pr.Status.MarkFailed(ReasonInvalidMatrixParameterTypes, err.Error()) + return controller.NewPermanentError(err) + } + } + defer func() { // If it is a permanent error, set pipelinerun to a failure state directly to avoid unnecessary retries. if err != nil && controller.IsPermanentError(err) { diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 2cffc61939c..9e2fcf70d80 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -6558,10 +6558,36 @@ spec: - name: optional-workspace optional: true serviceAccountName: test-sa +`), + parse.MustParseV1beta1PipelineRun(t, ` +metadata: + name: pipelinerun-matrix-param-invalid-type + namespace: foo +spec: + pipelineSpec: + tasks: + - name: mytask + params: + - name: platform + taskSpec: + steps: + - name: echo + image: alpine + script: | + echo "$(params.platform)" + - name: b-task + taskRef: + name: mytask + matrix: + params: + - name: platform + value: linux `), } + cms := []*corev1.ConfigMap{withEnabledAlphaAPIFields(newFeatureFlagsConfigMap())} d := test.Data{ + ConfigMaps: cms, PipelineRuns: prs, ServiceAccounts: []*corev1.ServiceAccount{{ ObjectMeta: metav1.ObjectMeta{Name: prs[0].Spec.ServiceAccountName, Namespace: "foo"}, @@ -6571,30 +6597,38 @@ spec: prt := newPipelineRunTest(t, d) defer prt.Cancel() - run1, _ := prt.reconcileRun("foo", "pipelinerun-param-invalid-result-variable", nil, true) - run2, _ := prt.reconcileRun("foo", "pipelinerun-pipeline-result-invalid-result-variable", nil, true) - run3, _ := prt.reconcileRun("foo", "pipelinerun-with-optional-workspace-validation", nil, true) - - cond1 := run1.Status.GetCondition(apis.ConditionSucceeded) - cond2 := run2.Status.GetCondition(apis.ConditionSucceeded) - cond3 := run3.Status.GetCondition(apis.ConditionSucceeded) - - for _, c := range []*apis.Condition{cond1, cond2, cond3} { - if c.Status != corev1.ConditionFalse { - t.Errorf("expected Succeeded/False condition but saw: %v", c) - } - } - - if cond1.Reason != ReasonInvalidTaskResultReference { - t.Errorf("expected invalid task reference condition but saw: %v", cond1) - } - - if cond2.Reason != ReasonInvalidTaskResultReference { - t.Errorf("expected invalid task reference condition but saw: %v", cond2) + testCases := []struct { + name string + reason string + }{ + { + name: "pipelinerun-param-invalid-result-variable", + reason: ReasonInvalidTaskResultReference, + }, { + name: "pipelinerun-pipeline-result-invalid-result-variable", + reason: ReasonInvalidTaskResultReference, + }, { + name: "pipelinerun-with-optional-workspace-validation", + reason: ReasonRequiredWorkspaceMarkedOptional, + }, { + name: "pipelinerun-matrix-param-invalid-type", + reason: ReasonInvalidMatrixParameterTypes, + }, } - - if cond3.Reason != ReasonRequiredWorkspaceMarkedOptional { - t.Errorf("expected optional workspace not supported condition but saw: %v", cond3) + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + run, _ := prt.reconcileRun("foo", tc.name, nil, true) + if run == nil { + t.Fatalf("Could not reconcile run %s", tc.name) + } + c := run.Status.GetCondition(apis.ConditionSucceeded) + if c.Status != corev1.ConditionFalse { + t.Errorf("expected Succeeded/False condition but saw: %v", c) + } + if c.Reason != tc.reason { + t.Errorf("want reason %s but got %s", tc.reason, c.Reason) + } + }) } } @@ -7952,6 +7986,367 @@ 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-dag", "platforms-and-browsers", false), + ` +spec: + params: + - name: browser + value: chrome + - name: platform + value: linux + serviceAccountName: test-sa + taskRef: + name: mytask + kind: Task +labels: + tekton.dev/memberOf: tasks + tekton.dev/pipeline: p-dag +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-platforms-and-browsers-1", "foo", + "pr", "p-dag", "platforms-and-browsers", false), + ` +spec: + params: + - name: browser + value: chrome + - name: platform + value: mac + serviceAccountName: test-sa + taskRef: + name: mytask + kind: Task +labels: + tekton.dev/memberOf: tasks + tekton.dev/pipeline: p-dag +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-platforms-and-browsers-2", "foo", + "pr", "p-dag", "platforms-and-browsers", false), + ` +spec: + params: + - name: browser + value: chrome + - name: platform + value: windows + serviceAccountName: test-sa + taskRef: + name: mytask + kind: Task +labels: + tekton.dev/memberOf: tasks + tekton.dev/pipeline: p-dag +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-platforms-and-browsers-3", "foo", + "pr", "p-dag", "platforms-and-browsers", false), + ` +spec: + params: + - name: browser + value: safari + - name: platform + value: linux + serviceAccountName: test-sa + taskRef: + name: mytask + kind: Task +labels: + tekton.dev/memberOf: tasks + tekton.dev/pipeline: p-dag +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-platforms-and-browsers-4", "foo", + "pr", "p-dag", "platforms-and-browsers", false), + ` +spec: + params: + - name: browser + value: safari + - name: platform + value: mac + serviceAccountName: test-sa + taskRef: + name: mytask + kind: Task +labels: + tekton.dev/memberOf: tasks + tekton.dev/pipeline: p-dag +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-platforms-and-browsers-5", "foo", + "pr", "p-dag", "platforms-and-browsers", false), + ` +spec: + params: + - name: browser + value: safari + - name: platform + value: windows + serviceAccountName: test-sa + taskRef: + name: mytask + kind: Task +labels: + tekton.dev/memberOf: tasks + tekton.dev/pipeline: p-dag +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-platforms-and-browsers-6", "foo", + "pr", "p-dag", "platforms-and-browsers", false), + ` +spec: + params: + - name: browser + value: firefox + - name: platform + value: linux + serviceAccountName: test-sa + taskRef: + name: mytask + kind: Task +labels: + tekton.dev/memberOf: tasks + tekton.dev/pipeline: p-dag +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-platforms-and-browsers-7", "foo", + "pr", "p-dag", "platforms-and-browsers", false), + ` +spec: + params: + - name: browser + value: firefox + - name: platform + value: mac + serviceAccountName: test-sa + taskRef: + name: mytask + kind: Task +labels: + tekton.dev/memberOf: tasks + tekton.dev/pipeline: p-dag +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-platforms-and-browsers-8", "foo", + "pr", "p-dag", "platforms-and-browsers", false), + ` +spec: + params: + - name: browser + value: firefox + - name: platform + value: windows + serviceAccountName: test-sa + taskRef: + name: mytask + kind: Task +labels: + tekton.dev/memberOf: tasks + tekton.dev/pipeline: p-dag +`), + } + 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", + 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 := getTaskRunsForPipelineRun(prt.TestAssets.Ctx, t, clients, pr.Namespace, pr.Name) + validateTaskRunsCount(t, taskRuns, len(expectedTaskRuns)) + + for i, expectedTaskRun := range expectedTaskRuns { + trName := expectedTaskRun.Name + actual := getTaskRunByName(t, taskRuns, trName) + if d := cmp.Diff(expectedTaskRun, actual, 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("found PipelineRun does not match expected PipelineRun. Diff %s", diff.PrintWantGot(d)) + } + }) + } +} func TestReconciler_PipelineTaskIncludeParams(t *testing.T) { names.TestingSeed() diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go index 01724843f84..bc70193cd56 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go @@ -477,7 +477,7 @@ func (t *ResolvedPipelineTask) skipBecausePipelineRunFinallyTimeoutReached(facts func (t *ResolvedPipelineTask) skipBecauseEmptyArrayInMatrixParams() bool { if t.PipelineTask.IsMatrixed() { for _, ps := range t.PipelineTask.Matrix.Params { - if len(ps.Value.ArrayVal) == 0 { + if ps.Value.Type == v1beta1.ParamTypeArray && len(ps.Value.ArrayVal) == 0 { return true } } diff --git a/pkg/reconciler/pipelinerun/resources/validate_params.go b/pkg/reconciler/pipelinerun/resources/validate_params.go index b50b0cfe28d..7659ca3a400 100644 --- a/pkg/reconciler/pipelinerun/resources/validate_params.go +++ b/pkg/reconciler/pipelinerun/resources/validate_params.go @@ -84,3 +84,29 @@ func ValidateObjectParamRequiredKeys(pipelineParameters []v1beta1.ParamSpec, pip return nil } + +// ValidateParameterTypesInMatrix 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 ValidateParameterTypesInMatrix(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 param %s has type %s", param.Name, 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 param %s has type %s", param.Name, string(param.Value.Type)) + } + } + } + } + return nil +} diff --git a/pkg/reconciler/pipelinerun/resources/validate_params_test.go b/pkg/reconciler/pipelinerun/resources/validate_params_test.go index 1057785a7b1..b66378205a9 100644 --- a/pkg/reconciler/pipelinerun/resources/validate_params_test.go +++ b/pkg/reconciler/pipelinerun/resources/validate_params_test.go @@ -317,3 +317,104 @@ func TestValidateObjectParamRequiredKeys_Valid(t *testing.T) { }) } } + +// 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 resources.PipelineRunState + wantErrs string + }{{ + desc: "parameters in matrix are arrays", + state: resources.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: resources.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 param foo has type string", + }, { + desc: "parameters in include matrix are strings", + state: resources.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: resources.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 param foobar has type array", + }, { + desc: "parameters in include matrix are objects", + state: resources.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 param barfoo has type object", + }} { + t.Run(tc.desc, func(t *testing.T) { + err := resources.ValidateParameterTypesInMatrix(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) + } + }) + } +}