Skip to content

Commit

Permalink
TEP-0090: Matrix - Parameters
Browse files Browse the repository at this point in the history
[TEP-0090][tep-0090] proposed executing a `PipelineTask`
in parallel `TaskRuns` and `Runs` with substitutions from
combinations of `Parameters` in a `Matrix`.

In this change, we add validation for `Parameters` including:
- `Parameter` must be of type `Array`
- `Parameter` must be in one of `matrix` or `params`, not both
- `Parameter` in `Matrix` must exist in `Parameters` declarations
   in `Pipeline`

[tep-0090]: https:/tektoncd/community/blob/main/teps/0090-matrix.md
  • Loading branch information
jerop committed Mar 23, 2022
1 parent 7f53bd2 commit 46af2b7
Show file tree
Hide file tree
Showing 6 changed files with 312 additions and 0 deletions.
53 changes: 53 additions & 0 deletions docs/matrix.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ weight: 11

- [Overview](#overview)
- [Configuring a Matrix](#configuring-a-matrix)
- [Parameters](#parameters)
- [Context Variables](#context-variables)

## Overview
Expand All @@ -25,8 +26,60 @@ Tekton.
## Configuring a Matrix

A `Matrix` supports the following features:
* [Parameters](#parameters)
* [Context Variables](#context-variables)

### Parameters

The `Matrix` will take `Parameters` of type `"array"` only, which will be supplied to the
`PipelineTask` by substituting `Parameters` of type `"string"` in the underlying `Task`.
The names of the `Parameters` in the `Matrix` must match the names of the `Parameters`
in the underlying `Task` that they will be substituting.

In the example below, the *test* `Task` takes *browser* and *platform* `Parameters` of type
`"string"`. A `Pipeline` used to fan out the `Task` using `Matrix` would have two `Parameters`
of type `"array"`, and it would execute nine `TaskRuns`:

```yaml
apiVersion: tekton.dev/v1beta1
kind: Pipeline
metadata:
name: platform-browser-tests
spec:
params:
- name: platforms
type: array
default:
- linux
- mac
- windows
- name: browsers
type: array
default:
- chrome
- safari
- firefox
tasks:
- name: fetch-repository
taskRef:
name: git-clone
...
- name: test
matrix:
- name: platform
value: $(params.platforms)
- name: browser
value: $(params.browsers)
taskRef:
name: browser-test
...
```

A `Parameter` can be passed to either the `matrix` or `params` field, not both.

For further details on specifying `Parameters` in the `Pipeline` and passing them to
`PipelineTasks`, see [documentation](pipelines.md#specifying-parameters).

### Context Variables

Similarly to the `Parameters` in the `Params` field, the `Parameters` in the `Matrix` field will accept
Expand Down
31 changes: 31 additions & 0 deletions pkg/apis/pipeline/v1beta1/param_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,37 @@ func validatePipelineParametersVariablesInTaskParameters(params []Param, prefix
return errs
}

func validatePipelineParametersVariablesInMatrixParameters(matrix []Param, prefix string, paramNames sets.String, arrayParamNames sets.String) (errs *apis.FieldError) {
for _, param := range matrix {
for idx, arrayElement := range param.Value.ArrayVal {
errs = errs.Also(validateArrayVariable(arrayElement, prefix, paramNames, arrayParamNames).ViaFieldIndex("value", idx).ViaFieldKey("matrix", param.Name))
}
}
return errs
}

func validateParametersInTaskMatrix(matrix []Param) (errs *apis.FieldError) {
for _, param := range matrix {
if param.Value.Type != ParamTypeArray {
errs = errs.Also(apis.ErrInvalidValue("parameters of type array only are allowed in matrix", "").ViaFieldKey("matrix", param.Name))
}
}
return errs
}

func validateParameterInOneOfMatrixOrParams(matrix []Param, params []Param) (errs *apis.FieldError) {
matrixParameterNames := sets.NewString()
for _, param := range matrix {
matrixParameterNames.Insert(param.Name)
}
for _, param := range params {
if matrixParameterNames.Has(param.Name) {
errs = errs.Also(apis.ErrMultipleOneOf("matrix["+param.Name+"]", "params["+param.Name+"]"))
}
}
return errs
}

func validateStringVariable(value, prefix string, stringVars sets.String, arrayVars sets.String) *apis.FieldError {
errs := substitution.ValidateVariableP(value, prefix, stringVars)
return errs.Also(substitution.ValidateVariableProhibitedP(value, prefix, arrayVars))
Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipeline_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,8 @@ func (pt *PipelineTask) validateMatrix(ctx context.Context) (errs *apis.FieldErr
// when the enable-api-fields feature gate is anything but "alpha".
errs = errs.Also(ValidateEnabledAPIFields(ctx, "matrix", config.AlphaAPIFields))
}
errs = errs.Also(validateParameterInOneOfMatrixOrParams(pt.Matrix, pt.Params))
errs = errs.Also(validateParametersInTaskMatrix(pt.Matrix))
return errs
}

Expand Down
70 changes: 70 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipeline_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/tektoncd/pipeline/pkg/apis/config"
"github.com/tektoncd/pipeline/test/diff"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/sets"
Expand Down Expand Up @@ -649,3 +650,72 @@ func TestPipelineTaskList_Validate(t *testing.T) {
})
}
}

func TestPipelineTask_validateMatrix(t *testing.T) {
tests := []struct {
name string
pt *PipelineTask
wantErrs *apis.FieldError
}{{
name: "parameter duplicated in matrix and params",
pt: &PipelineTask{
Name: "task",
Matrix: []Param{{
Name: "foobar", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}},
}},
Params: []Param{{
Name: "foobar", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}},
}},
},
wantErrs: apis.ErrMultipleOneOf("matrix[foobar]", "params[foobar]"),
}, {
name: "parameters unique in matrix and params",
pt: &PipelineTask{
Name: "task",
Matrix: []Param{{
Name: "foobar", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}},
}},
Params: []Param{{
Name: "barfoo", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}},
}},
},
}, {
name: "parameters in matrix are strings",
pt: &PipelineTask{
Name: "task",
Matrix: []Param{{
Name: "foo", Value: ArrayOrString{Type: ParamTypeString, StringVal: "foo"},
}, {
Name: "bar", Value: ArrayOrString{Type: ParamTypeString, StringVal: "bar"},
}},
},
wantErrs: &apis.FieldError{
Message: "invalid value: parameters of type array only are allowed in matrix",
Paths: []string{"matrix[foo]", "matrix[bar]"},
},
}, {
name: "parameters in matrix are arrays",
pt: &PipelineTask{
Name: "task",
Matrix: []Param{{
Name: "foobar", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}},
}, {
Name: "barfoo", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}},
}},
},
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
featureFlags, _ := config.NewFeatureFlagsFromMap(map[string]string{
"enable-api-fields": "alpha",
})
cfg := &config.Config{
FeatureFlags: featureFlags,
}
ctx := config.ToContext(context.Background(), cfg)
if d := cmp.Diff(tt.wantErrs.Error(), tt.pt.validateMatrix(ctx).Error()); d != "" {
t.Errorf("PipelineTask.validateMatrix() errors diff %s", diff.PrintWantGot(d))
}
})
}
}
1 change: 1 addition & 0 deletions pkg/apis/pipeline/v1beta1/pipeline_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ func validatePipelineParameterVariables(tasks []PipelineTask, params []ParamSpec
func validatePipelineParametersVariables(tasks []PipelineTask, prefix string, paramNames sets.String, arrayParamNames sets.String) (errs *apis.FieldError) {
for idx, task := range tasks {
errs = errs.Also(validatePipelineParametersVariablesInTaskParameters(task.Params, prefix, paramNames, arrayParamNames).ViaIndex(idx))
errs = errs.Also(validatePipelineParametersVariablesInMatrixParameters(task.Matrix, prefix, paramNames, arrayParamNames).ViaIndex(idx))
errs = errs.Also(task.WhenExpressions.validatePipelineParametersVariables(prefix, paramNames, arrayParamNames).ViaIndex(idx))
}
return errs
Expand Down
155 changes: 155 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipeline_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1318,6 +1318,34 @@ func TestValidatePipelineParameterVariables_Success(t *testing.T) {
Name: "a-param", Value: ArrayOrString{StringVal: "$(input.workspace.$(baz))"},
}},
}},
}, {
name: "valid array parameter variables in matrix",
params: []ParamSpec{{
Name: "baz", Type: ParamTypeArray, Default: &ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"some", "default"}},
}, {
Name: "foo-is-baz", Type: ParamTypeArray,
}},
tasks: []PipelineTask{{
Name: "bar",
TaskRef: &TaskRef{Name: "bar-task"},
Matrix: []Param{{
Name: "a-param", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"$(baz)", "and", "$(foo-is-baz)"}},
}},
}},
}, {
name: "valid star array parameter variables in matrix",
params: []ParamSpec{{
Name: "baz", Type: ParamTypeArray, Default: &ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"some", "default"}},
}, {
Name: "foo-is-baz", Type: ParamTypeArray,
}},
tasks: []PipelineTask{{
Name: "bar",
TaskRef: &TaskRef{Name: "bar-task"},
Matrix: []Param{{
Name: "a-param", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"$(baz[*])", "and", "$(foo-is-baz[*])"}},
}},
}},
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down Expand Up @@ -1596,6 +1624,53 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) {
Message: `parameter appears more than once`,
Paths: []string{"params[baz]"},
},
}, {
name: "invalid pipeline task with a matrix parameter which is missing from the param declarations",
tasks: []PipelineTask{{
Name: "foo",
TaskRef: &TaskRef{Name: "foo-task"},
Matrix: []Param{{
Name: "a-param", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"$(params.does-not-exist)"}},
}},
}},
expectedError: apis.FieldError{
Message: `non-existent variable in "$(params.does-not-exist)"`,
Paths: []string{"[0].matrix[a-param].value[0]"},
},
}, {
name: "invalid pipeline task with a matrix parameter combined with missing param from the param declarations",
params: []ParamSpec{{
Name: "foo", Type: ParamTypeString,
}},
tasks: []PipelineTask{{
Name: "foo-task",
TaskRef: &TaskRef{Name: "foo-task"},
Matrix: []Param{{
Name: "a-param", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"$(params.foo)", "and", "$(params.does-not-exist)"}},
}},
}},
expectedError: apis.FieldError{
Message: `non-existent variable in "$(params.does-not-exist)"`,
Paths: []string{"[0].matrix[a-param].value[2]"},
},
}, {
name: "invalid pipeline task with two matrix parameters and one of them missing from the param declarations",
params: []ParamSpec{{
Name: "foo", Type: ParamTypeArray,
}},
tasks: []PipelineTask{{
Name: "foo-task",
TaskRef: &TaskRef{Name: "foo-task"},
Matrix: []Param{{
Name: "a-param", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"$(params.foo)"}},
}, {
Name: "b-param", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"$(params.does-not-exist)"}},
}},
}},
expectedError: apis.FieldError{
Message: `non-existent variable in "$(params.does-not-exist)"`,
Paths: []string{"[0].matrix[b-param].value[0]"},
},
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down Expand Up @@ -2747,6 +2822,86 @@ func TestMatrixIncompatibleAPIVersions(t *testing.T) {
}
}

func Test_validateMatrix(t *testing.T) {
tests := []struct {
name string
tasks []PipelineTask
wantErrs *apis.FieldError
}{{
name: "parameter in both matrix and params",
tasks: PipelineTaskList{{
Name: "a-task",
TaskRef: &TaskRef{Name: "a-task"},
Matrix: []Param{{
Name: "foobar", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}},
}},
Params: []Param{{
Name: "foobar", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}},
}},
}},
wantErrs: apis.ErrMultipleOneOf("[0].matrix[foobar]", "[0].params[foobar]"),
}, {
name: "parameters unique in matrix and params",
tasks: PipelineTaskList{{
Name: "a-task",
TaskRef: &TaskRef{Name: "a-task"},
Matrix: []Param{{
Name: "foobar", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}},
}},
Params: []Param{{
Name: "barfoo", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}},
}},
}},
}, {
name: "parameters in matrix are strings",
tasks: PipelineTaskList{{
Name: "a-task",
TaskRef: &TaskRef{Name: "a-task"},
Matrix: []Param{{
Name: "foo", Value: ArrayOrString{Type: ParamTypeString, StringVal: "foo"},
}, {
Name: "bar", Value: ArrayOrString{Type: ParamTypeString, StringVal: "bar"},
}},
}, {
Name: "b-task",
TaskRef: &TaskRef{Name: "b-task"},
Matrix: []Param{{
Name: "baz", Value: ArrayOrString{Type: ParamTypeString, StringVal: "baz"},
}},
}},
wantErrs: &apis.FieldError{
Message: "invalid value: parameters of type array only are allowed in matrix",
Paths: []string{"[0].matrix[foo]", "[0].matrix[bar]", "[1].matrix[baz]"},
},
}, {
name: "parameters in matrix are arrays",
tasks: PipelineTaskList{{
Name: "a-task",
TaskRef: &TaskRef{Name: "a-task"},
Matrix: []Param{{
Name: "foobar", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"foo", "bar"}},
}, {
Name: "barfoo", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}},
}},
}},
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
featureFlags, _ := config.NewFeatureFlagsFromMap(map[string]string{
"enable-api-fields": "alpha",
})
cfg := &config.Config{
FeatureFlags: featureFlags,
}

ctx := config.ToContext(context.Background(), cfg)
if d := cmp.Diff(tt.wantErrs.Error(), validateMatrix(ctx, tt.tasks).Error()); d != "" {
t.Errorf("validateMatrix() errors diff %s", diff.PrintWantGot(d))
}
})
}
}

func getTaskSpec() TaskSpec {
return TaskSpec{
Steps: []Step{{
Expand Down

0 comments on commit 46af2b7

Please sign in to comment.