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..c8614ba63fa --- /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: 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 + finally: + - name: platforms-and-browsers-in-finally + 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..84e72a99016 100644 --- a/pkg/apis/pipeline/v1/matrix_types.go +++ b/pkg/apis/pipeline/v1/matrix_types.go @@ -15,8 +15,8 @@ package v1 import ( "context" - "fmt" "sort" + "strings" "github.com/tektoncd/pipeline/pkg/apis/config" "golang.org/x/exp/maps" @@ -300,34 +300,6 @@ 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 -// and a unique list of params in each Matrix.Include.Params specification -func (m *Matrix) validateParams() (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 -} - // validatePipelineParametersVariablesInMatrixParameters validates all pipeline parameter variables including Matrix.Params and Matrix.Include.Params // that may contain the reference(s) to other params to make sure those references are used appropriately. func (m *Matrix) validatePipelineParametersVariablesInMatrixParameters(prefix string, paramNames sets.String, arrayParamNames sets.String, objectParamNameKeys map[string][]string) (errs *apis.FieldError) { @@ -360,3 +332,21 @@ 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 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/v1/pipeline_types_test.go b/pkg/apis/pipeline/v1/pipeline_types_test.go index dbc29b5d302..56a48481058 100644 --- a/pkg/apis/pipeline/v1/pipeline_types_test.go +++ b/pkg/apis/pipeline/v1/pipeline_types_test.go @@ -607,155 +607,42 @@ func TestPipelineTask_ValidateMatrix(t *testing.T) { }, wantErrs: apis.ErrMultipleOneOf("matrix[foobar]", "params[foobar]"), }, { - name: "parameter duplicated in matrix.include.params and pipelinetask.params", + name: "parameters in matrix contain references to arrays", pt: &PipelineTask{ Name: "task", - Matrix: &Matrix{ - Include: IncludeParamsList{{ - Name: "duplicate-param", - Params: Params{{ - Name: "duplicate", Value: ParamValue{Type: ParamTypeString, StringVal: "foo"}, - }}}, - }}, - Params: Params{{ - Name: "duplicate", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, - }}, - }, - wantErrs: apis.ErrMultipleOneOf("matrix[duplicate]", "params[duplicate]"), - }, { - name: "duplicate parameters in matrix.params", - pt: &PipelineTask{ - Name: "task", - Matrix: &Matrix{ - Params: Params{{ - Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, - }, { - Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo-1", "bar-1"}}, - }}}, - }, - wantErrs: &apis.FieldError{ - Message: `parameter names must be unique, the parameter "foobar" is also defined at`, - Paths: []string{"matrix.params[1].name"}, - }, - }, { - name: "parameters unique in matrix and params", - pt: &PipelineTask{ - Name: "task", - Matrix: &Matrix{ - Params: Params{{ - Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, - }}}, 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{ - Name: "task", - Matrix: &Matrix{ - Include: IncludeParamsList{{ - Name: "invalid-include", - Params: Params{{ - Name: "foobar", Value: ParamValue{Type: ParamTypeString, StringVal: "foo"}, - }, { - Name: "foobar", Value: ParamValue{Type: ParamTypeString, StringVal: "foo-1"}, - }}}, - }}, - }, - wantErrs: &apis.FieldError{ - Message: `parameter names must be unique, the parameter "foobar" is also defined at`, - Paths: []string{"matrix.include[0].params[1].name"}, - }, - }, { - name: "parameters in matrix are strings", - pt: &PipelineTask{ - Name: "task", 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)", - }}, - }}, + 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..57ef92607f7 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1/pipeline_validation.go @@ -145,9 +145,9 @@ 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.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..38b3977c014 100644 --- a/pkg/apis/pipeline/v1beta1/matrix_types.go +++ b/pkg/apis/pipeline/v1beta1/matrix_types.go @@ -15,8 +15,8 @@ package v1beta1 import ( "context" - "fmt" "sort" + "strings" "github.com/tektoncd/pipeline/pkg/apis/config" "golang.org/x/exp/maps" @@ -300,34 +300,6 @@ 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 -// and a unique list of params in each Matrix.Include.Params specification -func (m *Matrix) validateParams() (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 -} - // validatePipelineParametersVariablesInMatrixParameters validates all pipeline parameter variables including Matrix.Params and Matrix.Include.Params // that may contain the reference(s) to other params to make sure those references are used appropriately. func (m *Matrix) validatePipelineParametersVariablesInMatrixParameters(prefix string, paramNames sets.String, arrayParamNames sets.String, objectParamNameKeys map[string][]string) (errs *apis.FieldError) { @@ -360,3 +332,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..77e6e9d05bc 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_types_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_types_test.go @@ -597,139 +597,42 @@ func TestPipelineTask_validateMatrix(t *testing.T) { }, wantErrs: apis.ErrMultipleOneOf("matrix[duplicate]", "params[duplicate]"), }, { - name: "duplicate parameters in matrix.params", + name: "parameters in matrix contain references to arrays", pt: &PipelineTask{ Name: "task", - Matrix: &Matrix{ - Params: Params{{ - Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, - }, { - Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo-1", "bar-1"}}, - }}}, - }, - wantErrs: &apis.FieldError{ - Message: `parameter names must be unique, the parameter "foobar" is also defined at`, - Paths: []string{"matrix.params[1].name"}, - }, - }, { - name: "parameters unique in matrix and params", - pt: &PipelineTask{ - Name: "task", - Matrix: &Matrix{ - Params: Params{{ - Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, - }}}, 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 matrix are strings", - pt: &PipelineTask{ - Name: "task", 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: "duplicate parameters in matrix.include.params", - pt: &PipelineTask{ - Name: "task", - Matrix: &Matrix{ - Include: IncludeParamsList{{ - Name: "invalid-include", - Params: Params{{ - Name: "foobar", Value: ParamValue{Type: ParamTypeString, StringVal: "foo"}, - }, { - Name: "foobar", Value: ParamValue{Type: ParamTypeString, StringVal: "foo-1"}, - }}}, - }}, - }, - wantErrs: &apis.FieldError{ - Message: `parameter names must be unique, the parameter "foobar" is also defined at`, - 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", - 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)", - }}, - }}, + 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/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go index c170e85a3b9..93f0309c306 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -157,9 +157,9 @@ 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.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..5479e80e98b 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -113,6 +113,10 @@ const ( // ReasonCouldntTimeOut indicates that a PipelineRun was timed out but attempting to update // all of the running TaskRuns as timed out failed. ReasonCouldntTimeOut = "PipelineRunCouldntTimeOut" + // ReasonDuplicateParams indicates a PipelineRun contains duplicate parameter types + ReasonDuplicateParams = "ReasonDuplicateParams" + // 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" @@ -579,6 +583,18 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get } if pipelineRunFacts.State.IsBeforeFirstTaskRun() { + 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) + } + + if err := resources.ValidateUniqueParamsInMatrix(pipelineRunFacts.State); err != nil { + logger.Errorf("Failed to validate matrix %q with error %v", pr.Name, err) + pr.Status.MarkFailed(ReasonDuplicateParams, 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 2cffc61939c..bc4970de8ae 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -6598,6 +6598,99 @@ spec: } } +func TestReconcile_ParamsValidationsImmediatelyFailPipelineRun(t *testing.T) { + names.TestingSeed() + + ctx := context.Background() + cfg := config.NewStore(logtesting.TestLogger(t)) + ctx = cfg.ToContext(ctx) + + prs := []*v1beta1.PipelineRun{ + parse.MustParseV1beta1PipelineRun(t, ` +metadata: + name: pipelinerun-matrix-param-invalid-type + namespace: foo +spec: + pipelineSpec: + tasks: + - name: mytask + taskSpec: + steps: + - name: echo + image: alpine + script: | + echo "test" + - name: platforms-and-browsers + taskRef: + name: mytask + matrix: + params: + - name: platform + value: linux + - name: browser + value: + - chrome + - safari + - firefox + params: + - name: version + value: v0.33.0 +`), + parse.MustParseV1beta1PipelineRun(t, ` +metadata: + name: pipelinerun-matrix-param-duplicates + namespace: foo +spec: + pipelineSpec: + tasks: + - name: mytask + matrix: + params: + - name: browser + value: + - chrome + - name: browser + value: + - safari + - firefox + taskSpec: + steps: + - name: echo + image: alpine + script: | + echo "test" +`), + } + + cms := []*corev1.ConfigMap{withEnabledAlphaAPIFields(newFeatureFlagsConfigMap())} + d := test.Data{ + PipelineRuns: prs, + ConfigMaps: cms, + } + prt := newPipelineRunTest(t, d) + defer prt.Cancel() + + run1, _ := prt.reconcileRun("foo", "pipelinerun-matrix-param-invalid-type", nil, true) + run2, _ := prt.reconcileRun("foo", "pipelinerun-matrix-param-duplicates", nil, true) + + cond1 := run1.Status.GetCondition(apis.ConditionSucceeded) + cond2 := run2.Status.GetCondition(apis.ConditionSucceeded) + + for _, c := range []*apis.Condition{cond1, cond2} { + if c.Status != corev1.ConditionFalse { + t.Errorf("expected Succeeded/False condition but saw: %v", c) + } + } + + if cond1.Reason != ReasonInvalidMatrixParameterTypes { + t.Errorf("expected invalid matrix parameter types condition but saw: %v", cond1) + } + + if cond2.Reason != ReasonDuplicateParams { + t.Errorf("expected invalid duplicate parameters condition but saw: %v", cond2) + } +} + // TestReconcileWithResolver checks that a PipelineRun with a populated Resolver // field creates a ResolutionRequest object for that Resolver's type, and // that when the request is successfully resolved the PipelineRun begins running. @@ -7952,6 +8045,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() diff --git a/pkg/reconciler/pipelinerun/resources/validate_params.go b/pkg/reconciler/pipelinerun/resources/validate_params.go index b50b0cfe28d..811d941b531 100644 --- a/pkg/reconciler/pipelinerun/resources/validate_params.go +++ b/pkg/reconciler/pipelinerun/resources/validate_params.go @@ -22,6 +22,7 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/list" "github.com/tektoncd/pipeline/pkg/reconciler/taskrun" + "k8s.io/apimachinery/pkg/util/sets" ) // ValidateParamTypesMatching validate that parameters in PipelineRun override corresponding parameters in Pipeline of the same type. @@ -84,3 +85,59 @@ 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 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 +} + +// ValidateUniqueParamsInMatrix validates there are no duplicate parameters +// within Matrix.Params or Matrix.Include Params +func ValidateUniqueParamsInMatrix(state PipelineRunState) error { + var errs error + for _, rpt := range state { + m := rpt.PipelineTask.Matrix + if m.HasInclude() { + for _, include := range m.Include { + errs = validateDuplicateParameters(include.Params, "matrix.include.params") + } + } + if m.HasParams() { + errs = validateDuplicateParameters(m.Params, "matrix.params") + } + } + return errs +} + +// validateDuplicateParameters checks if a parameter with the same name is defined more than once +func validateDuplicateParameters(ps v1beta1.Params, typeParam string) (errs error) { + taskParamNames := sets.NewString() + for i, param := range ps { + if taskParamNames.Has(param.Name) { + errs = fmt.Errorf(fmt.Sprintf("parameter names must be unique, the parameter %s is also defined at %s[%d].name", param.Name, typeParam, i)) + } + taskParamNames.Insert(param.Name) + } + return errs +} diff --git a/pkg/reconciler/pipelinerun/resources/validate_params_test.go b/pkg/reconciler/pipelinerun/resources/validate_params_test.go index 1057785a7b1..542cb2ad860 100644 --- a/pkg/reconciler/pipelinerun/resources/validate_params_test.go +++ b/pkg/reconciler/pipelinerun/resources/validate_params_test.go @@ -317,3 +317,221 @@ 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 got param 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 got param 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 got param 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) + } + }) + } +} + +// TestValidateUniqueParamsInMatrix tests that a pipeline task does not have +// any duplicate parameters +func TestValidateUniqueParamsInMatrix(t *testing.T) { + for _, tc := range []struct { + desc string + state resources.PipelineRunState + wantErrs string + }{{ + desc: "parameters are unique", + state: resources.PipelineRunState{{ + PipelineTask: &v1beta1.PipelineTask{ + Name: "task", + Params: v1beta1.Params{{ + Name: "foo", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeString, StringVal: "foo"}, + }}, + 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: "parameter duplicated in pipelinetask", + state: resources.PipelineRunState{{ + PipelineTask: &v1beta1.PipelineTask{ + Name: "task", + Params: v1beta1.Params{{ + Name: "duplicate", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeString, StringVal: "foo"}, + }, { + Name: "duplicate", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeString, StringVal: "foo"}, + }}, + }, + }}, + wantErrs: "parameter names must be unique, the parameter duplicate is also defined at params[1].name", + }, { + desc: "parameter duplicated in matrix and pipelinetask", + state: resources.PipelineRunState{{ + PipelineTask: &v1beta1.PipelineTask{ + Name: "task", + Params: v1beta1.Params{{ + Name: "duplicate", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeString, StringVal: "foo"}, + }}, + Matrix: &v1beta1.Matrix{ + Params: v1beta1.Params{{ + Name: "duplicate", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, + }, { + Name: "barfoo", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeArray, ArrayVal: []string{"bar", "foo"}}}}, + }, + }, + }}, + wantErrs: "parameter names must be unique, the parameter duplicate is also defined at matrix.params[0].name", + }, { + desc: "parameter duplicated in matrix", + state: resources.PipelineRunState{{ + PipelineTask: &v1beta1.PipelineTask{ + Name: "task", + Matrix: &v1beta1.Matrix{ + Params: v1beta1.Params{{ + Name: "duplicate", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeArray, ArrayVal: []string{"foo", "bar"}}, + }, { + Name: "duplicate", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeArray, ArrayVal: []string{"bar", "foo"}}}}, + }, + }, + }}, + wantErrs: "parameter names must be unique, the parameter duplicate is also defined at matrix.params[1].name", + }, { + desc: "parameter duplicated in matrix include params", + state: resources.PipelineRunState{{ + PipelineTask: &v1beta1.PipelineTask{ + Name: "task", + Matrix: &v1beta1.Matrix{ + Include: v1beta1.IncludeParamsList{{ + Name: "build-1", + Params: v1beta1.Params{{ + Name: "duplicate", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeString, StringVal: "foo"}, + }, { + Name: "duplicate", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeString, StringVal: "bar"}, + }}}, + }}, + }, + }}, + wantErrs: "parameter names must be unique, the parameter duplicate is also defined at matrix.include.params[1].name", + }, { + desc: "parameter duplicated in matrix include params and pipelinetask", + state: resources.PipelineRunState{{ + PipelineTask: &v1beta1.PipelineTask{ + Name: "task", + Params: v1beta1.Params{{ + Name: "duplicate", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeString, StringVal: "foo"}, + }}, + Matrix: &v1beta1.Matrix{ + Include: v1beta1.IncludeParamsList{{ + Name: "build-1", + Params: v1beta1.Params{{ + Name: "duplicate", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeString, StringVal: "foo"}, + }, { + Name: "foobar", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeString, StringVal: "bar"}, + }}}, + }}, + }, + }}, + wantErrs: "parameter names must be unique, the parameter duplicate is also defined at matrix.include.params[0].name", + }} { + t.Run(tc.desc, func(t *testing.T) { + err := resources.ValidateUniqueParamsInMatrix(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) + } + }) + } +}