Skip to content

Commit

Permalink
Added Validation: Matrix parameters cannot be empty arrays
Browse files Browse the repository at this point in the history
Added a check in Pipeline validation to verify that matrix parameters are not just arrays, but that they are not empty arrays. This won't change existing functionality/capabilities, but will get rid of the panic in the reconciler.
  • Loading branch information
EmmaMunley committed Feb 9, 2023
1 parent 2da10fc commit f30249a
Show file tree
Hide file tree
Showing 11 changed files with 92 additions and 55 deletions.
27 changes: 14 additions & 13 deletions docs/matrix.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ weight: 11
## Overview

`Matrix` is used to fan out `Tasks` in a `Pipeline`. This doc will explain the details of `matrix` support in
Tekton.
Tekton.

Documentation for specifying `Matrix` in a `Pipeline`:
- [Specifying `Matrix` in `Tasks`](pipelines.md#specifying-matrix-in-pipelinetasks)
Expand All @@ -40,12 +40,12 @@ A `Matrix` supports the following features:
* [Concurrency Control](#concurrency-control)
* [Parameters](#parameters)
* [Context Variables](#context-variables)
* [Results](#results)
* [Results](#results)

### Concurrency Control

The default maximum count of `TaskRuns` or `Runs` from a given `Matrix` is **256**. To customize the maximum count of
`TaskRuns` or `Runs` generated from a given `Matrix`, configure the `default-max-matrix-combinations-count` in
`TaskRuns` or `Runs` generated from a given `Matrix`, configure the `default-max-matrix-combinations-count` in
[config defaults](/config/config-defaults.yaml). When a `Matrix` in `PipelineTask` would generate more than the maximum
`TaskRuns` or `Runs`, the `Pipeline` validation would fail.

Expand All @@ -69,6 +69,7 @@ The `Matrix` will take `Parameters` of type `"array"` only, which will be suppli
`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.
Note: The `Paramaters` of type `array` cannot be empty.

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`
Expand All @@ -92,7 +93,7 @@ spec:
default:
- chrome
- safari
- firefox
- firefox
tasks:
- name: fetch-repository
taskRef:
Expand All @@ -110,7 +111,7 @@ spec:
...
```

A `Parameter` can be passed to either the `matrix` or `params` field, not both.
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).
Expand Down Expand Up @@ -150,7 +151,7 @@ spec:

### Context Variables

Similarly to the `Parameters` in the `Params` field, the `Parameters` in the `Matrix` field will accept
Similarly to the `Parameters` in the `Params` field, the `Parameters` in the `Matrix` field will accept
[context variables](variables.md) that will be substituted, including:

* `PipelineRun` name, namespace and uid
Expand All @@ -161,7 +162,7 @@ Similarly to the `Parameters` in the `Params` field, the `Parameters` in the `Ma

#### Specifying Results in a Matrix

Consuming `Results` from previous `TaskRuns` or `Runs` in a `Matrix`, which would dynamically generate
Consuming `Results` from previous `TaskRuns` or `Runs` in a `Matrix`, which would dynamically generate
`TaskRuns` or `Runs` from the fanned out `PipelineTask`, is supported. Producing `Results` in from a
`PipelineTask` with a `Matrix` is not yet supported - see [further details](#results-from-fanned-out-pipelinetasks).

Expand Down Expand Up @@ -203,7 +204,7 @@ tasks:

Consuming `Results` from fanned out `PipelineTasks` will not be in the supported in the initial iteration
of `Matrix`. Supporting consuming `Results` from fanned out `PipelineTasks` will be revisited after array
and object `Results` are supported.
and object `Results` are supported.

## Fan Out

Expand Down Expand Up @@ -487,15 +488,15 @@ status:
name: platforms-and-browsers
taskRef:
apiVersion: cel.tekton.dev/v1alpha1
kind: CEL
kind: CEL
startTime: "2022-06-28T20:49:40Z"
completionTime: "2022-06-28T20:49:41Z"
conditions:
- lastTransitionTime: "2022-06-28T20:49:41Z"
message: 'Tasks Completed: 1 (Failed: 0, Cancelled 0), Skipped: 0'
reason: Succeeded
status: "True"
type: Succeeded
type: Succeeded
childReferences:
- apiVersion: tekton.dev/v1alpha1
kind: Run
Expand Down Expand Up @@ -533,11 +534,11 @@ status:

## Retries

The `retries` field is used to specify the number of times a `PipelineTask` should be retried when its `TaskRun` or
`Run` fails, see the [documentation][retries] for further details. When a `PipelineTask` is fanned out using `Matrix`,
The `retries` field is used to specify the number of times a `PipelineTask` should be retried when its `TaskRun` or
`Run` fails, see the [documentation][retries] for further details. When a `PipelineTask` is fanned out using `Matrix`,
a given `TaskRun` or `Run` executed will be retried as much as the field in the `retries` field of the `PipelineTask`.

For example, the `PipelineTask` in this `PipelineRun` will be fanned out into three `TaskRuns` each of which will be
For example, the `PipelineTask` in this `PipelineRun` will be fanned out into three `TaskRuns` each of which will be
retried once:

```yaml
Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/pipeline/v1/param_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,9 @@ func validateParametersInTaskMatrix(matrix *Matrix) (errs *apis.FieldError) {
if param.Value.Type != ParamTypeArray {
errs = errs.Also(apis.ErrInvalidValue("parameters of type array only are allowed in matrix", "").ViaFieldKey("matrix", param.Name))
}
if param.Value.Type == ParamTypeArray && len(param.Value.ArrayVal) == 0 {
errs = errs.Also(apis.ErrInvalidValue("empty arrays not allowed as parameters in matrix", "").ViaFieldKey("matrix", param.Name))
}
}
}
return errs
Expand Down
15 changes: 15 additions & 0 deletions pkg/apis/pipeline/v1/pipeline_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,21 @@ func TestPipelineTask_validateMatrix(t *testing.T) {
Name: "barfoo", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}},
}}},
},
}, {
name: "parameters in matrix are not empty arrays",
pt: &PipelineTask{
Name: "task",
Matrix: &Matrix{
Params: []Param{{
Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{}},
}, {
Name: "barfoo", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{}},
}}},
},
wantErrs: &apis.FieldError{
Message: "invalid value: empty arrays not allowed as parameters in matrix",
Paths: []string{"matrix[barfoo]", "matrix[foobar]"},
},
}, {
name: "parameters in matrix contain results references",
pt: &PipelineTask{
Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/pipeline/v1beta1/param_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,9 @@ func validateParametersInTaskMatrix(matrix *Matrix) (errs *apis.FieldError) {
if param.Value.Type != ParamTypeArray {
errs = errs.Also(apis.ErrInvalidValue("parameters of type array only are allowed in matrix", "").ViaFieldKey("matrix", param.Name))
}
if param.Value.Type == ParamTypeArray && len(param.Value.ArrayVal) == 0 {
errs = errs.Also(apis.ErrInvalidValue("empty arrays not allowed as parameters in matrix", "").ViaFieldKey("matrix", param.Name))
}
}
}
return errs
Expand Down
15 changes: 15 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipeline_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,21 @@ func TestPipelineTask_validateMatrix(t *testing.T) {
Name: "barfoo", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"bar", "foo"}},
}}},
},
}, {
name: "parameters in matrix are not empty arrays",
pt: &PipelineTask{
Name: "task",
Matrix: &Matrix{
Params: []Param{{
Name: "foobar", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{}},
}, {
Name: "barfoo", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{}},
}}},
},
wantErrs: &apis.FieldError{
Message: "invalid value: empty arrays not allowed as parameters in matrix",
Paths: []string{"matrix[barfoo]", "matrix[foobar]"},
},
}, {
name: "parameters in matrix contain results references",
pt: &PipelineTask{
Expand Down
14 changes: 7 additions & 7 deletions pkg/client/clientset/versioned/fake/register.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 7 additions & 7 deletions pkg/client/clientset/versioned/scheme/register.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 7 additions & 7 deletions pkg/client/resolution/clientset/versioned/fake/register.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 7 additions & 7 deletions pkg/client/resolution/clientset/versioned/scheme/register.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 7 additions & 7 deletions pkg/client/resource/clientset/versioned/fake/register.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 7 additions & 7 deletions pkg/client/resource/clientset/versioned/scheme/register.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit f30249a

Please sign in to comment.