Skip to content

Commit

Permalink
Add matrix support for using references to entire PipelineRun array p…
Browse files Browse the repository at this point in the history
…arameters

This commit adds support for using whole array 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
  • Loading branch information
EmmaMunley authored and tekton-robot committed Apr 28, 2023
1 parent b8419fc commit 3d6ec63
Show file tree
Hide file tree
Showing 15 changed files with 608 additions and 330 deletions.
14 changes: 14 additions & 0 deletions docs/matrix.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -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
37 changes: 23 additions & 14 deletions pkg/apis/pipeline/v1/matrix_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
82 changes: 15 additions & 67 deletions pkg/apis/pipeline/v1/pipeline_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -687,94 +687,42 @@ func TestPipelineTask_ValidateMatrix(t *testing.T) {
Paths: []string{"matrix.include[0].params[1].name"},
},
}, {
name: "parameters in matrix are strings",
name: "parameters in matrix contain references to param arrays",
pt: &PipelineTask{
Name: "task",
Params: Params{{
Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}},
}, {
Name: "barfoo", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}},
}},
Matrix: &Matrix{
Params: Params{{
Name: "foo", Value: ParamValue{Type: ParamTypeString, StringVal: "foo"},
Name: "foo", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.foobar[*])"},
}, {
Name: "bar", Value: ParamValue{Type: ParamTypeString, StringVal: "bar"},
Name: "bar", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.barfoo[*])"},
}}},
},
wantErrs: &apis.FieldError{
Message: "invalid value: parameters of type array only are allowed, but got param type string",
Paths: []string{"matrix.params[foo]", "matrix.params[bar]"},
},
}, {
name: "parameters in matrix are arrays",
name: "parameters in matrix contain result 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",
Expand Down
3 changes: 2 additions & 1 deletion pkg/apis/pipeline/v1/pipeline_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,9 +155,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
}

Expand Down
69 changes: 0 additions & 69 deletions pkg/apis/pipeline/v1/pipeline_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3078,75 +3078,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{{
Expand Down
Loading

0 comments on commit 3d6ec63

Please sign in to comment.