From bafc0b7fb9fb091d380ad866df3ec52aeae3f2fc Mon Sep 17 00:00:00 2001 From: Emma Munley <46881325+EmmaMunley@users.noreply.github.com> Date: Fri, 1 Sep 2023 16:37:27 -0400 Subject: [PATCH] TEP-140: Produce Results in Matrix This commit enables producing Results from Matrixed PipelineTasks so that they can be used in subsequent PipelineTasks. A Pipeline author can now declare a matrixed taskRun that emits results of type string that are fanned out over multiple taskRuns and aggregated into an array of results that can then be consumed by another pipelineTask using the syntax `$(tasks..results.[*])`. This commit also introduces 2 context variables to 1) Access Matrix Combinations Length using `tasks..matrix.length` and 2) Access Aggregated Results Length using `tasks..matrix..length` Note: Currently, we don't support consuming a single instance/combinations of results. Authors must consume the entire aggregated results array. Co-authored-by: Priti Desai --- docs/matrix.md | 84 +- docs/variables.md | 2 + ...linerun-with-matrix-context-variables.yaml | 123 ++ ...elinerun-with-matrix-emitting-results.yaml | 110 ++ pkg/apis/pipeline/v1/matrix_types.go | 14 + pkg/apis/pipeline/v1/param_types.go | 22 + pkg/apis/pipeline/v1/param_types_test.go | 35 + pkg/apis/pipeline/v1/pipeline_types_test.go | 197 ++- pkg/apis/pipeline/v1/pipeline_validation.go | 98 +- .../pipeline/v1/pipeline_validation_test.go | 87 +- pkg/apis/pipeline/v1beta1/matrix_types.go | 14 + .../pipeline/v1beta1/pipeline_types_test.go | 199 ++- .../pipeline/v1beta1/pipeline_validation.go | 99 +- .../v1beta1/pipeline_validation_test.go | 87 +- pkg/reconciler/pipelinerun/pipelinerun.go | 20 +- .../pipelinerun/pipelinerun_test.go | 1469 +++++++++++++++++ pkg/reconciler/pipelinerun/resources/apply.go | 58 +- .../pipelinerun/resources/apply_test.go | 150 +- .../resources/pipelinerunresolution.go | 16 + .../resources/pipelinerunresolution_test.go | 83 + .../pipelinerun/resources/pipelinerunstate.go | 26 +- .../resources/pipelinerunstate_test.go | 58 +- .../resources/resultrefresolution.go | 125 +- .../resources/resultrefresolution_test.go | 98 ++ .../pipelinerun/resources/validate_params.go | 7 + .../resources/validate_params_test.go | 11 + 26 files changed, 3060 insertions(+), 232 deletions(-) create mode 100644 examples/v1/pipelineruns/alpha/pipelinerun-with-matrix-context-variables.yaml create mode 100644 examples/v1/pipelineruns/alpha/pipelinerun-with-matrix-emitting-results.yaml diff --git a/docs/matrix.md b/docs/matrix.md index 463ae1c5215..9e7a4c62105 100644 --- a/docs/matrix.md +++ b/docs/matrix.md @@ -17,6 +17,8 @@ weight: 406 - [Parameters in Matrix.Include.Params](#parameters-in-matrixincludeparams) - [Specifying both `params` and `matrix` in a `PipelineTask`](#specifying-both-params-and-matrix-in-a-pipelinetask) - [Context Variables](#context-variables) + - [Access Matrix Combinations Length](#access-matrix-combinations-length) + - [Access Aggregated Results Length](#access-aggregated-results-length) - [Results](#results) - [Specifying Results in a Matrix](#specifying-results-in-a-matrix) - [Results in Matrix.Params](#results-in-matrixparams) @@ -291,6 +293,38 @@ Similarly to the `Parameters` in the `Params` field, the `Parameters` in the `Ma * `Pipeline` name * `PipelineTask` retries + +The following `context` variables allow users to access the `matrix` runtime data. Note: In order to create an ordering dependency, use `runAfter` or `taskResult` consumption as part of the same pipelineTask. + +#### Access Matrix Combinations Length + +The pipeline authors can access the total number of instances created as part of the `matrix` using the syntax: `tasks..matrix.length`. + +```yaml + - name: matrixed-echo-length + runAfter: + - matrix-emitting-results + params: + - name: matrixlength + value: $(tasks.matrix-emitting-results.matrix.length) +``` + +#### Access Aggregated Results Length + +The pipeline authors can access the length of the array of aggregated results that were +actually produced using the syntax: `tasks..matrix..length`. This will allow users to loop over the results produced. + +```yaml + - name: matrixed-echo-results-length + runAfter: + - matrix-emitting-results + params: + - name: matrixlength + value: $(tasks.matrix-emitting-results.matrix.a-result.length) +``` + +See the full example here: [pr-with-matrix-context-variables] + ## Results ### Specifying Results in a Matrix @@ -360,8 +394,51 @@ tasks: ### Results from fanned out Matrixed PipelineTasks -Emitting `Results` from fanned out `PipelineTasks` is not currently supported. -We plan to support emitting `Results` from fanned out `PipelineTasks` in the near future. +Emitting `Results` from fanned out `PipelineTasks` is now supported. Each fanned out +`TaskRun` that produces `Result` of type `string` will be aggregated into an `array` +of `Results` during reconciliation, in which the whole `array` of `Results` can be consumed by another `pipelineTask` using the star notion [*]. +Note: A known limitation is not being able to consume a singular result or specific +combinations of results produced by a previous fanned out `PipelineTask`. + +| Result Type in `taskRef` or `taskSpec` | Parameter Type of Consumer | Specification | +|----------------------------------------|----------------------------|-------------------------------------------------------| +| string | array | `$(tasks..results.[*])` | +| array | Not Supported | Not Supported | +| object | Not Supported | Not Supported | + +```yaml +apiVersion: tekton.dev/v1beta1 +kind: Pipeline +metadata: + name: platform-browser-tests +spec: + tasks: + - name: matrix-emitting-results + matrix: + params: + - name: platform + value: + - linux + - mac + - windows + - name: browser + value: + - chrome + - safari + - firefox + taskRef: + name: taskwithresults + kind: Task + - name: task-consuming-results + taskRef: + name: echoarrayurl + kind: Task + params: + - name: url + value: $(tasks.matrix-emitting-results.results.report-url[*]) + ... +``` +See the full example [pr-with-matrix-emitting-results] ## Retries @@ -851,4 +928,7 @@ status: [cel]: https://github.com/tektoncd/experimental/tree/1609827ea81d05c8d00f8933c5c9d6150cd36989/cel [pr-with-matrix]: ../examples/v1/pipelineruns/alpha/pipelinerun-with-matrix.yaml [pr-with-matrix-and-results]: ../examples/v1/pipelineruns/alpha/pipelinerun-with-matrix-and-results.yaml +[pr-with-matrix-context-variables]: ../examples/v1/pipelineruns/alpha/pipelinerun-with-matrix-context-variables.yaml +[pr-with-matrix-emitting-results]: ../examples/v1/pipelineruns/alpha/pipelinerun-with-matrix-emitting-results.yaml + [retries]: pipelines.md#using-the-retries-field diff --git a/docs/variables.md b/docs/variables.md index 7a831c08921..e1224f0a8a3 100644 --- a/docs/variables.md +++ b/docs/variables.md @@ -28,10 +28,12 @@ For instructions on using variable substitutions see the relevant section of [th | `params[""][i]` | (see above) | | `params.[*]` | Get the value of the whole object param. This is alpha feature, set `enable-api-fields` to `alpha` to use it.| | `params..` | Get the value of an individual child of an object param. This is alpha feature, set `enable-api-fields` to `alpha` to use it. | +| `tasks..matrix.length` | The length of the `Matrix` combination count. | | `tasks..results.` | The value of the `Task's` result. Can alter `Task` execution order within a `Pipeline`.) | | `tasks..results.[i]` | The ith value of the `Task's` array result. Can alter `Task` execution order within a `Pipeline`.) | | `tasks..results.[*]` | The array value of the `Task's` result. Can alter `Task` execution order within a `Pipeline`. Cannot be used in `script`.) | | `tasks..results..key` | The `key` value of the `Task's` object result. Can alter `Task` execution order within a `Pipeline`.) | +| `tasks..matrix..length` | The length of the matrixed `Task's` results. (Can alter `Task` execution order within a `Pipeline`.) | | `workspaces..bound` | Whether a `Workspace` has been bound or not. "false" if the `Workspace` declaration has `optional: true` and the Workspace binding was omitted by the PipelineRun. | | `context.pipelineRun.name` | The name of the `PipelineRun` that this `Pipeline` is running in. | | `context.pipelineRun.namespace` | The namespace of the `PipelineRun` that this `Pipeline` is running in. | diff --git a/examples/v1/pipelineruns/alpha/pipelinerun-with-matrix-context-variables.yaml b/examples/v1/pipelineruns/alpha/pipelinerun-with-matrix-context-variables.yaml new file mode 100644 index 00000000000..cffc5561b82 --- /dev/null +++ b/examples/v1/pipelineruns/alpha/pipelinerun-with-matrix-context-variables.yaml @@ -0,0 +1,123 @@ +apiVersion: tekton.dev/v1 +kind: Task +metadata: + name: validate-matrix-result-length +spec: + params: + - name: matrixlength + type: string + steps: + - name: validate + image: alpine + args: ["$(params.matrixlength)"] + script: | + #!/usr/bin/env sh + echo "Validating the length of the matrix context variable" + echo "The length of the matrix is 3" + if [ "$(params.matrixlength)" != 3 ]; then + echo "Error: expected matrix to have the length 3 but has length $(params.matrixlength)" + exit 1 + fi + echo "Done validating the length of the matrix context variable" +--- +apiVersion: tekton.dev/v1 +kind: Task +metadata: + name: validate-matrix-results-length +spec: + params: + - name: matrixresultslength-1 + type: string + - name: matrixresultslength-2 + type: string + steps: + - name: validate + image: alpine + script: | + #!/usr/bin/env sh + echo "Validating the length of the matrix results context variable" + echo "The length of the matrix results are $(params.matrixresultslength-1) and $(params.matrixresultslength-2)" + if [ "$(params.matrixresultslength-1)" != 3 ]; then + echo "Error: expected matrix results to have the length 3 but has length $(params.matrixresultslength-1)" + exit 1 + fi + if [ "$(params.matrixresultslength-2)" != 1 ]; then + echo "Error: expected matrix results to have the length 1 but has length $(params.matrixresultslength-2)" + exit 1 + fi + echo "Done validating the length of the matrix context variable" +--- +apiVersion: tekton.dev/v1 +kind: Task +metadata: + name: taskwithresults +spec: + params: + - name: IMAGE + - name: DIGEST + default: "" + results: + - name: IMAGE-DIGEST + - name: IMAGE-URL + steps: + - name: produce-results + image: bash:latest + script: | + #!/usr/bin/env bash + echo "Building image for $(params.IMAGE)" + echo -n "$(params.DIGEST)" | sha256sum | tee $(results.IMAGE-DIGEST.path) + if [ -z $(params.DIGEST) ]; then + echo -n "$(params.DIGEST)" | sha256sum | tee $(results.IMAGE-URL.path) + fi +--- +apiVersion: tekton.dev/v1 +kind: PipelineRun +metadata: + generateName: matrix-context-variables- +spec: + taskRunTemplate: + serviceAccountName: "default" + pipelineSpec: + tasks: + - name: matrix-emitting-results + matrix: + include: + - name: build-1 + params: + - name: IMAGE + value: image-1 + - name: DIGEST + value: path/to/Dockerfile1 + - name: build-2 + params: + - name: IMAGE + value: image-2 + - name: DIGEST + value: path/to/Dockerfile2 + - name: build-3 + params: + - name: IMAGE + value: image-3 + taskRef: + name: taskwithresults + kind: Task + - name: matrixed-echo-length + runAfter: + - matrix-emitting-results + params: + - name: matrixlength + value: $(tasks.matrix-emitting-results.matrix.length) + taskRef: + name: validate-matrix-result-length + kind: Task + - name: matrixed-echo-results-length + runAfter: + - matrix-emitting-results + params: + - name: matrixresultslength-1 + value: $(tasks.matrix-emitting-results.matrix.IMAGE-DIGEST.length) + - name: matrixresultslength-2 + value: $(tasks.matrix-emitting-results.matrix.IMAGE-URL.length) + taskRef: + name: validate-matrix-results-length + kind: Task diff --git a/examples/v1/pipelineruns/alpha/pipelinerun-with-matrix-emitting-results.yaml b/examples/v1/pipelineruns/alpha/pipelinerun-with-matrix-emitting-results.yaml new file mode 100644 index 00000000000..b286ff77118 --- /dev/null +++ b/examples/v1/pipelineruns/alpha/pipelinerun-with-matrix-emitting-results.yaml @@ -0,0 +1,110 @@ +apiVersion: tekton.dev/v1 +kind: Task +metadata: + name: echostringurl +spec: + params: + - name: url + type: string + steps: + - name: echo + image: alpine + script: | + echo "$(params.url)" +--- +apiVersion: tekton.dev/v1 +kind: Task +metadata: + name: validate-array-url +spec: + params: + - name: url + type: array + steps: + - name: validate + image: ubuntu + args: ["$(params.url[*])"] + script: | + #!/usr/bin/env bash + args=("$@") + URLS=( ) + URLS[0]="https://api.example/get-report/linux-chrome" + URLS[1]="https://api.example/get-report/mac-chrome" + URLS[2]="https://api.example/get-report/windows-chrome" + URLS[3]="https://api.example/get-report/linux-safari" + URLS[4]="https://api.example/get-report/mac-safari" + URLS[5]="https://api.example/get-report/windows-safari" + URLS[6]="https://api.example/get-report/linux-firefox" + URLS[7]="https://api.example/get-report/mac-firefox" + URLS[8]="https://api.example/get-report/windows-firefox" + for i in "${!URLS[@]}"; do + if [ "${URLS[$i]}" != ${args[$i]} ]; then + echo "Error: expected url to be ${URLS[$i]}, but got ${args[$i]}" + exit 1 + fi + echo "Done validating the url: ${args[$i]}" + done +--- +apiVersion: tekton.dev/v1 +kind: Task +metadata: + name: task-producing-results +spec: + params: + - name: platform + default: "" + - name: browser + default: "" + results: + - name: report-url + type: string + steps: + - name: produce-report-url + image: alpine + script: | + echo "Running tests on $(params.platform)-$(params.browser)" + echo -n "https://api.example/get-report/$(params.platform)-$(params.browser)" | tee $(results.report-url.path) +--- +apiVersion: tekton.dev/v1 +kind: PipelineRun +metadata: + generateName: platforms-with-results +spec: + taskRunTemplate: + serviceAccountName: "default" + pipelineSpec: + results: + - name: pr-result-1 + value: $(tasks.matrix-emitting-results.results.report-url[*]) + tasks: + - name: matrix-emitting-results + matrix: + params: + - name: platform + value: + - linux + - mac + - windows + - name: browser + value: + - chrome + - safari + - firefox + taskRef: + name: task-producing-results + kind: Task + - name: task-consuming-results + taskRef: + name: validate-array-url + kind: Task + params: + - name: url + value: $(tasks.matrix-emitting-results.results.report-url[*]) + - name: matrix-consuming-results + taskRef: + name: echostringurl + kind: Task + matrix: + params: + - name: url + value: $(tasks.matrix-emitting-results.results.report-url[*]) diff --git a/pkg/apis/pipeline/v1/matrix_types.go b/pkg/apis/pipeline/v1/matrix_types.go index f51a0ac4af4..bd9f4ff9ac1 100644 --- a/pkg/apis/pipeline/v1/matrix_types.go +++ b/pkg/apis/pipeline/v1/matrix_types.go @@ -348,3 +348,17 @@ func (m *Matrix) validateParameterInOneOfMatrixOrParams(params []Param) (errs *a } return errs } + +// validateTaskResultsFromMatrixedPipelineTasksConsumed checks that any Matrixed Pipeline Task that the is becing consumed is consumed in +// aggregate [*] since consuming a singular result produced by a matrix is currently not supported. +// It also validates that a matrix emitting results can only emit results with the underlying type string +// if those results are being consumed by another PipelineTask. +func (m *Matrix) validateTaskResultsFromMatrixedPipelineTasksConsumed(tasks []PipelineTask) (errs *apis.FieldError) { + taskMapping := createTaskMapping(tasks) + resultRefs, errs := findAndValidateResultRefsForMatrix(tasks, taskMapping) + if errs != nil { + return errs + } + errs = errs.Also(validateMatrixEmittingStringResults(resultRefs, taskMapping)) + return errs +} diff --git a/pkg/apis/pipeline/v1/param_types.go b/pkg/apis/pipeline/v1/param_types.go index 86fe6ce3e72..00a8d8c686a 100644 --- a/pkg/apis/pipeline/v1/param_types.go +++ b/pkg/apis/pipeline/v1/param_types.go @@ -208,6 +208,28 @@ func (ps Params) extractParamMapArrVals() map[string][]string { return paramsMap } +// ParseTaskandResultName parses "task name", "result name" from a Matrix Context Variable +// Valid Example 1: +// - Input: tasks.myTask.matrix.length +// - Output: "myTask", "" +// Valid Example 2: +// - Input: tasks.myTask.matrix.ResultName.length +// - Output: "myTask", "ResultName" +func (p Param) ParseTaskandResultName() (string, string) { + if expressions, ok := p.GetVarSubstitutionExpressions(); ok { + for _, expression := range expressions { + subExpressions := strings.Split(expression, ".") + pipelineTaskName := subExpressions[1] + if len(subExpressions) == 4 { + return pipelineTaskName, "" + } else if len(subExpressions) == 5 { + return pipelineTaskName, subExpressions[3] + } + } + } + return "", "" +} + // Params is a list of Param type Params []Param diff --git a/pkg/apis/pipeline/v1/param_types_test.go b/pkg/apis/pipeline/v1/param_types_test.go index d01be3d8199..1552a6abd98 100644 --- a/pkg/apis/pipeline/v1/param_types_test.go +++ b/pkg/apis/pipeline/v1/param_types_test.go @@ -653,3 +653,38 @@ func TestExtractDefaultParamArrayLengths(t *testing.T) { }) } } + +func TestParseTaskandResultName(t *testing.T) { + tcs := []struct { + name string + param v1.Param + pipelineTaskName string + resultName string + }{{ + name: "matrix length context var", + param: v1.Param{Name: "foo", Value: v1.ParamValue{StringVal: "$(tasks.matrix-emitting-results.matrix.length)", Type: v1.ParamTypeString}}, + pipelineTaskName: "matrix-emitting-results", + }, { + name: "matrix results length context var", + param: v1.Param{Name: "foo", Value: v1.ParamValue{StringVal: "$(tasks.myTask.matrix.ResultName.length)", Type: v1.ParamTypeString}}, + pipelineTaskName: "myTask", + resultName: "ResultName", + }, { + name: "empty context var", + param: v1.Param{Name: "foo", Value: v1.ParamValue{StringVal: "", Type: v1.ParamTypeString}}, + pipelineTaskName: "", + resultName: "", + }} + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + pipelineTaskName, resultName := tc.param.ParseTaskandResultName() + + if d := cmp.Diff(tc.pipelineTaskName, pipelineTaskName); d != "" { + t.Errorf(diff.PrintWantGot(d)) + } + if d := cmp.Diff(tc.resultName, resultName); d != "" { + t.Errorf(diff.PrintWantGot(d)) + } + }) + } +} diff --git a/pkg/apis/pipeline/v1/pipeline_types_test.go b/pkg/apis/pipeline/v1/pipeline_types_test.go index 0b5ffdc6abd..1ee8d07ea9a 100644 --- a/pkg/apis/pipeline/v1/pipeline_types_test.go +++ b/pkg/apis/pipeline/v1/pipeline_types_test.go @@ -843,10 +843,59 @@ func TestPipelineTaskList_Deps(t *testing.T) { } func TestPipelineTask_ValidateMatrix(t *testing.T) { + tasks := PipelineTaskList{{ + Name: "matrix-emitting-results", + TaskRef: &TaskRef{Name: "taskwithresult"}, + Matrix: &Matrix{ + Params: Params{{ + Name: "platform", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"linux", "mac"}}, + }, { + Name: "browser", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"chrome", "safari"}}, + }}}, + }, { + Name: "echoarrayurl", + TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ + Params: ParamSpecs{{ + Name: "url", Type: "array", + }}, + Steps: []Step{{ + Name: "use-environments", + Image: "bash:latest", + Args: []string{"$(params.url[*])"}, + Script: `for arg in "$@"; do + echo "URL: $arg" + done`, + }}, + }}, + }, { + Name: "matrix-emitting-results-embedded", + TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ + Params: ParamSpecs{{ + Name: "platform", + }, { + Name: "browser"}}, + Results: []TaskResult{{ + Name: "array-result", + Type: ResultsTypeArray, + }}, + Steps: []Step{{ + Name: "produce-array-result", + Image: "alpine", + Script: ` | + echo -n "[\"${params.platform}\",\"${params.browser}\"]" | tee $(results.array-result.path)`}}, + }}, + Matrix: &Matrix{ + Params: Params{{ + Name: "platform", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"linux", "mac", "windows"}}, + }, { + Name: "browser", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"chrome", "safari", "firefox"}}, + }}}, + }} tests := []struct { name string pt *PipelineTask wantErrs *apis.FieldError + tasks []PipelineTask }{{ name: "parameter duplicated in matrix.params and pipelinetask.params", pt: &PipelineTask{ @@ -972,8 +1021,152 @@ func TestPipelineTask_ValidateMatrix(t *testing.T) { Name: "browser", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"chrome", "firefox"}}, }}}, }, + }, { + name: "valid matrix emitting string results consumed in aggregate by another pipelineTask", + pt: &PipelineTask{ + Name: "task-consuming-results", + TaskRef: &TaskRef{Name: "echoarrayurl"}, + Params: Params{{ + Name: "b-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(tasks.matrix-emitting-results-embedded.results.array-result[*])"}, + }}, + }, + tasks: PipelineTaskList{}, + }, { + name: "valid matrix emitting string results consumed in aggregate by another pipelineTask (embedded taskSpec)", + pt: &PipelineTask{ + Name: "task-consuming-results", + TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ + Params: ParamSpecs{{ + Name: "url", Type: "array", + }}, + Steps: []Step{{ + Name: "use-environments", + Image: "bash:latest", + Args: []string{"$(params.url[*])"}, + Script: `for arg in "$@"; do + echo "URL: $arg" + done`, + }}, + }}, + Params: Params{{ + Name: "b-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(tasks.matrix-emitting-results.results.report-url[*])"}, + }}, + }, + tasks: PipelineTaskList{}, + }, { + name: "invalid matrix emitting strings results consumed using array indexing by another pipelineTask", + pt: &PipelineTask{ + Name: "matrix-emitting-results", + TaskRef: &TaskRef{Name: "taskwithresult"}, + Matrix: &Matrix{ + Params: Params{{ + Name: "platform", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"linux", "mac"}}, + }, { + Name: "browser", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"chrome", "safari"}}, + }}}, + }, + tasks: PipelineTaskList{{ + Name: "taskwithresult", + TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ + Params: ParamSpecs{{ + Name: "platform", + }, { + Name: "browser"}}, + Results: []TaskResult{{ + Name: "report-url", + Type: ResultsTypeString, + }}, + Steps: []Step{{ + Name: "produce-report-url", + Image: "alpine", + Script: ` | + echo -n "https://api.example/get-report/$(params.platform)-$(params.browser)" | tee $(results.report-url.path)`}}, + }}, + }, { + Name: "task-consuming-results", + TaskRef: &TaskRef{Name: "echoarrayurl"}, + Params: Params{{ + Name: "b-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(tasks.matrix-emitting-results.results.report-url[0])"}, + }}, + }}, + wantErrs: apis.ErrGeneric("A matrixed pipelineTask can only be consumed in aggregate using [*] notation, but is currently set to tasks.matrix-emitting-results.results.report-url[0]"), + }, { + name: "invalid matrix emitting array results consumed in aggregate by another pipelineTask", + pt: &PipelineTask{ + Name: "matrix-emitting-results", + TaskRef: &TaskRef{Name: "taskwithresult"}, + Matrix: &Matrix{ + Params: Params{{ + Name: "platform", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"linux", "mac"}}, + }, { + Name: "browser", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"chrome", "safari"}}, + }}}, + }, + tasks: PipelineTaskList{{ + Name: "taskwithresult", + TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ + Params: ParamSpecs{{ + Name: "platform", + }, { + Name: "browser"}}, + Results: []TaskResult{{ + Name: "array-result", + Type: ResultsTypeArray, + }}, + Steps: []Step{{ + Name: "produce-array-result", + Image: "alpine", + Script: ` | + echo -n "https://api.example/get-report/$(params.platform)-$(params.browser)" | tee $(results.array-result.path)`}}, + }}, + }, { + Name: "task-consuming-results", + TaskRef: &TaskRef{Name: "echoarrayurl"}, + Params: Params{{ + Name: "b-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(tasks.matrix-emitting-results.results.array-result[*])"}, + }}, + }}, + wantErrs: apis.ErrInvalidValue("Matrixed PipelineTasks emitting results must have an underlying type string, but result array-result has type array in pipelineTask", ""), + }, { + name: "invalid matrix emitting array results consumed in aggregate by another pipelineTask (embedded TaskSpec)", + pt: &PipelineTask{ + Name: "matrix-emitting-results", + TaskRef: &TaskRef{Name: "taskwithresult"}, + Matrix: &Matrix{ + Params: Params{{ + Name: "platform", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"linux", "mac"}}, + }, { + Name: "browser", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"chrome", "safari"}}, + }}}, + }, + tasks: PipelineTaskList{{ + Name: "taskwithresult", + TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ + Params: ParamSpecs{{ + Name: "platform", + }, { + Name: "browser"}}, + Results: []TaskResult{{ + Name: "array-result", + Type: ResultsTypeArray, + }}, + Steps: []Step{{ + Name: "produce-array-result", + Image: "alpine", + Script: ` | + echo -n "https://api.example/get-report/$(params.platform)-$(params.browser)" | tee $(results.array-result.path)`}}, + }}, + }, { + Name: "task-consuming-results", + TaskRef: &TaskRef{Name: "echoarrayurl"}, + Params: Params{{ + Name: "b-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(tasks.matrix-emitting-results-embedded.results.array-result[*])"}, + }}, + }}, + wantErrs: apis.ErrInvalidValue("Matrixed PipelineTasks emitting results must have an underlying type string, but result array-result has type array in pipelineTask", ""), }} for _, tt := range tests { + var tasksForTest []PipelineTask t.Run(tt.name, func(t *testing.T) { featureFlags, _ := config.NewFeatureFlagsFromMap(map[string]string{ "enable-api-fields": "alpha", @@ -985,8 +1178,10 @@ func TestPipelineTask_ValidateMatrix(t *testing.T) { FeatureFlags: featureFlags, Defaults: defaults, } + tasksForTest := append(tasksForTest, tasks...) + tasksForTest = append(tasksForTest, tt.tasks...) ctx := config.ToContext(context.Background(), cfg) - if d := cmp.Diff(tt.wantErrs.Error(), tt.pt.validateMatrix(ctx).Error()); d != "" { + if d := cmp.Diff(tt.wantErrs.Error(), tt.pt.validateMatrix(ctx, tasksForTest).Error()); d != "" { t.Errorf("PipelineTask.validateMatrix() errors diff %s", diff.PrintWantGot(d)) } }) diff --git a/pkg/apis/pipeline/v1/pipeline_validation.go b/pkg/apis/pipeline/v1/pipeline_validation.go index 71b002d9bcd..2bdc09a109c 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1/pipeline_validation.go @@ -92,7 +92,6 @@ func (ps *PipelineSpec) Validate(ctx context.Context) (errs *apis.FieldError) { errs = errs.Also(validateWhenExpressions(ps.Tasks, ps.Finally)) errs = errs.Also(validateMatrix(ctx, ps.Tasks).ViaField("tasks")) errs = errs.Also(validateMatrix(ctx, ps.Finally).ViaField("finally")) - errs = errs.Also(validateResultsFromMatrixedPipelineTasksNotConsumed(ps.Tasks, ps.Finally)) return errs } @@ -228,13 +227,14 @@ func (pt PipelineTask) Validate(ctx context.Context) (errs *apis.FieldError) { return } -func (pt *PipelineTask) validateMatrix(ctx context.Context) (errs *apis.FieldError) { +func (pt *PipelineTask) validateMatrix(ctx context.Context, tasks []PipelineTask) (errs *apis.FieldError) { if pt.IsMatrixed() { // This is an alpha feature and will fail validation if it's used in a pipeline spec // 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.validateUniqueParams()) + errs = errs.Also(pt.Matrix.validateTaskResultsFromMatrixedPipelineTasksConsumed(tasks)) } errs = errs.Also(pt.Matrix.validateParameterInOneOfMatrixOrParams(pt.Params)) return errs @@ -260,15 +260,6 @@ func (pt PipelineTask) validateEmbeddedOrType() (errs *apis.FieldError) { return } -func (pt *PipelineTask) validateResultsFromMatrixedPipelineTasksNotConsumed(matrixedPipelineTasks sets.String) (errs *apis.FieldError) { - for _, ref := range PipelineTaskResultRefs(pt) { - if matrixedPipelineTasks.Has(ref.PipelineTask) { - errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("consuming results from matrixed task %s is not allowed", ref.PipelineTask), "")) - } - } - return errs -} - func (pt *PipelineTask) validateWorkspaces(workspaceNames sets.String) (errs *apis.FieldError) { workspaceBindingNames := sets.NewString() for i, ws := range pt.Workspaces { @@ -766,23 +757,88 @@ func validateGraph(tasks []PipelineTask) (errs *apis.FieldError) { func validateMatrix(ctx context.Context, tasks []PipelineTask) (errs *apis.FieldError) { for idx, task := range tasks { - errs = errs.Also(task.validateMatrix(ctx).ViaIndex(idx)) + errs = errs.Also(task.validateMatrix(ctx, tasks).ViaIndex(idx)) } return errs } -func validateResultsFromMatrixedPipelineTasksNotConsumed(tasks []PipelineTask, finally []PipelineTask) (errs *apis.FieldError) { - matrixedPipelineTasks := sets.String{} - for _, pt := range tasks { - if pt.IsMatrixed() { - matrixedPipelineTasks.Insert(pt.Name) +// createTaskMapping maps the PipelineTaskName to the PipelineTask to easily access +// the pipelineTask by Name +func createTaskMapping(tasks []PipelineTask) (taskMap map[string]PipelineTask) { + taskMapping := make(map[string]PipelineTask) + for _, task := range tasks { + taskMapping[task.Name] = task + } + return taskMapping +} + +// findAndValidateResultRefsForMatrix checks that any result references to Matrixed PipelineTasks if consumed +// by another PipelineTask that the entire array of results produced by a matrix is consumed in aggregate +// since consuming a singular result produced by a matrix is currently not supported +func findAndValidateResultRefsForMatrix(tasks []PipelineTask, taskMapping map[string]PipelineTask) (resultRefs []*ResultRef, errs *apis.FieldError) { + for _, t := range tasks { + for _, p := range t.Params { + if expressions, ok := p.GetVarSubstitutionExpressions(); ok { + if LooksLikeContainsResultRefs(expressions) { + resultRefs, errs = validateMatrixedPipelineTaskConsumed(expressions, taskMapping) + if errs != nil { + return nil, errs + } + } + } } } - for idx, pt := range tasks { - errs = errs.Also(pt.validateResultsFromMatrixedPipelineTasksNotConsumed(matrixedPipelineTasks).ViaFieldIndex("tasks", idx)) + return resultRefs, errs +} + +// validateMatrixedPipelineTaskConsumed checks that any Matrixed Pipeline Task that the is being consumed is consumed in +// aggregate [*] since consuming a singular result produced by a matrix is currently not supported +func validateMatrixedPipelineTaskConsumed(expressions []string, taskMapping map[string]PipelineTask) (resultRefs []*ResultRef, errs *apis.FieldError) { + var filteredExpressions []string + for _, expression := range expressions { + // ie. "tasks..results.[*]" + subExpressions := strings.Split(expression, ".") + pipelineTask := subExpressions[1] // pipelineTaskName + taskConsumed := taskMapping[pipelineTask] + if taskConsumed.IsMatrixed() { + if !strings.HasSuffix(expression, "[*]") { + errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("A matrixed pipelineTask can only be consumed in aggregate using [*] notation, but is currently set to %s", expression))) + } + filteredExpressions = append(filteredExpressions, expression) + } + } + return NewResultRefs(filteredExpressions), errs +} + +// validateMatrixEmittingStringResults checks a matrix emitting results can only emit results with the underlying type string +// if those results are being consumed by another PipelineTask. Note: Is is not possible to validate remote tasks +func validateMatrixEmittingStringResults(resultRefs []*ResultRef, taskMapping map[string]PipelineTask) (errs *apis.FieldError) { + for _, resultRef := range resultRefs { + task := taskMapping[resultRef.PipelineTask] + resultName := resultRef.Result + if task.TaskRef != nil { + referencedTask := taskMapping[task.TaskRef.Name] + if referencedTask.TaskSpec != nil { + errs = errs.Also(validateStringResults(referencedTask.TaskSpec.Results, resultName)) + } + } else if task.TaskSpec != nil { + errs = errs.Also(validateStringResults(task.TaskSpec.Results, resultName)) + } } - for idx, pt := range finally { - errs = errs.Also(pt.validateResultsFromMatrixedPipelineTasksNotConsumed(matrixedPipelineTasks).ViaFieldIndex("finally", idx)) + return errs +} + +// validateStringResults ensure that the result type is string +func validateStringResults(results []TaskResult, resultName string) (errs *apis.FieldError) { + for _, result := range results { + if result.Name == resultName { + if result.Type != ResultsTypeString { + errs = errs.Also(apis.ErrInvalidValue( + fmt.Sprintf("Matrixed PipelineTasks emitting results must have an underlying type string, but result %s has type %s in pipelineTask", resultName, string(result.Type)), + "", + )) + } + } } return errs } diff --git a/pkg/apis/pipeline/v1/pipeline_validation_test.go b/pkg/apis/pipeline/v1/pipeline_validation_test.go index c80b51a4b03..aff37d4557f 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1/pipeline_validation_test.go @@ -3575,6 +3575,7 @@ func Test_validateMatrix(t *testing.T) { tests := []struct { name string tasks []PipelineTask + finally []PipelineTask wantErrs *apis.FieldError }{{ name: "parameter in both matrix and params", @@ -3630,35 +3631,7 @@ func Test_validateMatrix(t *testing.T) { Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.foo-task.results.a-task-results[*])"}}, }}}, }}, - }} - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - featureFlags, _ := config.NewFeatureFlagsFromMap(map[string]string{ - "enable-api-fields": "alpha", - }) - defaults := &config.Defaults{ - DefaultMaxMatrixCombinationsCount: 4, - } - cfg := &config.Config{ - FeatureFlags: featureFlags, - Defaults: defaults, - } - - 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 Test_validateResultsFromMatrixedPipelineTasksNotConsumed(t *testing.T) { - tests := []struct { - name string - tasks []PipelineTask - finally []PipelineTask - wantErrs *apis.FieldError - }{{ + }, { name: "results from matrixed task consumed in tasks through parameters", tasks: PipelineTaskList{{ Name: "a-task", @@ -3671,13 +3644,9 @@ func Test_validateResultsFromMatrixedPipelineTasksNotConsumed(t *testing.T) { Name: "b-task", TaskRef: &TaskRef{Name: "b-task"}, Params: Params{{ - Name: "b-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.a-task.results.a-result)"}}, + Name: "b-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.a-task.results.a-result[*])"}}, }}, }}, - wantErrs: &apis.FieldError{ - Message: "invalid value: consuming results from matrixed task a-task is not allowed", - Paths: []string{"tasks[1]"}, - }, }, { name: "results from matrixed task consumed in finally through parameters", tasks: PipelineTaskList{{ @@ -3692,13 +3661,9 @@ func Test_validateResultsFromMatrixedPipelineTasksNotConsumed(t *testing.T) { Name: "b-task", TaskRef: &TaskRef{Name: "b-task"}, Params: Params{{ - Name: "b-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.a-task.results.a-result)"}}, + Name: "b-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.a-task.results.a-result[*])"}}, }}, }}, - wantErrs: &apis.FieldError{ - Message: "invalid value: consuming results from matrixed task a-task is not allowed", - Paths: []string{"finally[0]"}, - }, }, { name: "results from matrixed task consumed in tasks and finally through parameters", tasks: PipelineTaskList{{ @@ -3712,20 +3677,16 @@ func Test_validateResultsFromMatrixedPipelineTasksNotConsumed(t *testing.T) { Name: "b-task", TaskRef: &TaskRef{Name: "b-task"}, Params: Params{{ - Name: "b-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.a-task.results.a-result)"}}, + Name: "b-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.a-task.results.a-result[*])"}}, }}, }}, finally: PipelineTaskList{{ Name: "c-task", TaskRef: &TaskRef{Name: "c-task"}, Params: Params{{ - Name: "b-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.a-task.results.a-result)"}}, + Name: "b-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.a-task.results.a-result[*])"}}, }}, }}, - wantErrs: &apis.FieldError{ - Message: "invalid value: consuming results from matrixed task a-task is not allowed", - Paths: []string{"tasks[1]", "finally[0]"}, - }, }, { name: "results from matrixed task consumed in tasks through when expressions", tasks: PipelineTaskList{{ @@ -3741,13 +3702,9 @@ func Test_validateResultsFromMatrixedPipelineTasksNotConsumed(t *testing.T) { When: WhenExpressions{{ Input: "foo", Operator: selection.In, - Values: []string{"$(tasks.a-task.results.a-result)"}, + Values: []string{"$(tasks.a-task.results.a-result[*])"}, }}, }}, - wantErrs: &apis.FieldError{ - Message: "invalid value: consuming results from matrixed task a-task is not allowed", - Paths: []string{"tasks[1]"}, - }, }, { name: "results from matrixed task consumed in finally through when expressions", tasks: PipelineTaskList{{ @@ -3762,15 +3719,11 @@ func Test_validateResultsFromMatrixedPipelineTasksNotConsumed(t *testing.T) { Name: "b-task", TaskRef: &TaskRef{Name: "b-task"}, When: WhenExpressions{{ - Input: "$(tasks.a-task.results.a-result)", + Input: "$(tasks.a-task.results.a-result[*])", Operator: selection.In, Values: []string{"foo", "bar"}, }}, }}, - wantErrs: &apis.FieldError{ - Message: "invalid value: consuming results from matrixed task a-task is not allowed", - Paths: []string{"finally[0]"}, - }, }, { name: "results from matrixed task consumed in tasks and finally through when expressions", tasks: PipelineTaskList{{ @@ -3784,7 +3737,7 @@ func Test_validateResultsFromMatrixedPipelineTasksNotConsumed(t *testing.T) { Name: "b-task", TaskRef: &TaskRef{Name: "b-task"}, When: WhenExpressions{{ - Input: "$(tasks.a-task.results.a-result)", + Input: "$(tasks.a-task.results.a-result[*])", Operator: selection.In, Values: []string{"foo", "bar"}, }}, @@ -3795,18 +3748,26 @@ func Test_validateResultsFromMatrixedPipelineTasksNotConsumed(t *testing.T) { When: WhenExpressions{{ Input: "foo", Operator: selection.In, - Values: []string{"$(tasks.a-task.results.a-result)"}, + Values: []string{"$(tasks.a-task.results.a-result)[*]"}, }}, }}, - wantErrs: &apis.FieldError{ - Message: "invalid value: consuming results from matrixed task a-task is not allowed", - Paths: []string{"tasks[1]", "finally[0]"}, - }, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if d := cmp.Diff(tt.wantErrs.Error(), validateResultsFromMatrixedPipelineTasksNotConsumed(tt.tasks, tt.finally).Error()); d != "" { - t.Errorf("validateResultsFromMatrixedPipelineTasksNotConsumed() errors diff %s", diff.PrintWantGot(d)) + featureFlags, _ := config.NewFeatureFlagsFromMap(map[string]string{ + "enable-api-fields": "alpha", + }) + defaults := &config.Defaults{ + DefaultMaxMatrixCombinationsCount: 4, + } + cfg := &config.Config{ + FeatureFlags: featureFlags, + Defaults: defaults, + } + + 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)) } }) } diff --git a/pkg/apis/pipeline/v1beta1/matrix_types.go b/pkg/apis/pipeline/v1beta1/matrix_types.go index 19042e11e4b..1b82f010759 100644 --- a/pkg/apis/pipeline/v1beta1/matrix_types.go +++ b/pkg/apis/pipeline/v1beta1/matrix_types.go @@ -348,3 +348,17 @@ func (m *Matrix) validateParameterInOneOfMatrixOrParams(params Params) (errs *ap } return errs } + +// validateTaskResultsFromMatrixedPipelineTasksConsumed checks that any Matrixed Pipeline Task that the is becing consumed is consumed in +// aggregate [*] since consuming a singular result produced by a matrix is currently not supported. +// It also validates that a matrix emitting results can only emit results with the underlying type string +// if those results are being consumed by another PipelineTask. +func (m *Matrix) validateTaskResultsFromMatrixedPipelineTasksConsumed(tasks []PipelineTask) (errs *apis.FieldError) { + taskMapping := createTaskMapping(tasks) + resultRefs, errs := findAndValidateResultRefsForMatrix(tasks, taskMapping) + if errs != nil { + return errs + } + errs = errs.Also(validateMatrixEmittingStringResults(resultRefs, taskMapping)) + return errs +} diff --git a/pkg/apis/pipeline/v1beta1/pipeline_types_test.go b/pkg/apis/pipeline/v1beta1/pipeline_types_test.go index 34b887d4d27..aa58213cb0a 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_types_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_types_test.go @@ -826,11 +826,60 @@ func TestPipelineTaskList_Validate(t *testing.T) { } } -func TestPipelineTask_validateMatrix(t *testing.T) { +func TestPipelineTask_ValidateMatrix(t *testing.T) { + tasks := PipelineTaskList{{ + Name: "matrix-emitting-results", + TaskRef: &TaskRef{Name: "taskwithresult"}, + Matrix: &Matrix{ + Params: Params{{ + Name: "platform", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"linux", "mac"}}, + }, { + Name: "browser", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"chrome", "safari"}}, + }}}, + }, { + Name: "echoarrayurl", + TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ + Params: ParamSpecs{{ + Name: "url", Type: "array", + }}, + Steps: []Step{{ + Name: "use-environments", + Image: "bash:latest", + Args: []string{"$(params.url[*])"}, + Script: `for arg in "$@"; do + echo "URL: $arg" + done`, + }}, + }}, + }, { + Name: "matrix-emitting-results-embedded", + TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ + Params: ParamSpecs{{ + Name: "platform", + }, { + Name: "browser"}}, + Results: []TaskResult{{ + Name: "array-result", + Type: ResultsTypeArray, + }}, + Steps: []Step{{ + Name: "produce-array-result", + Image: "alpine", + Script: ` | + echo -n "[\"${params.platform}\",\"${params.browser}\"]" | tee $(results.array-result.path)`}}, + }}, + Matrix: &Matrix{ + Params: Params{{ + Name: "platform", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"linux", "mac", "windows"}}, + }, { + Name: "browser", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"chrome", "safari", "firefox"}}, + }}}, + }} tests := []struct { name string pt *PipelineTask wantErrs *apis.FieldError + tasks []PipelineTask }{{ name: "parameter duplicated in matrix.params and pipelinetask.params", pt: &PipelineTask{ @@ -956,8 +1005,152 @@ func TestPipelineTask_validateMatrix(t *testing.T) { Name: "browser", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"chrome", "firefox"}}, }}}, }, + }, { + name: "valid matrix emitting string results consumed in aggregate by another pipelineTask", + pt: &PipelineTask{ + Name: "task-consuming-results", + TaskRef: &TaskRef{Name: "echoarrayurl"}, + Params: Params{{ + Name: "b-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(tasks.matrix-emitting-results-embedded.results.array-result[*])"}, + }}, + }, + tasks: PipelineTaskList{}, + }, { + name: "valid matrix emitting string results consumed in aggregate by another pipelineTask (embedded taskSpec)", + pt: &PipelineTask{ + Name: "task-consuming-results", + TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ + Params: ParamSpecs{{ + Name: "url", Type: "array", + }}, + Steps: []Step{{ + Name: "use-environments", + Image: "bash:latest", + Args: []string{"$(params.url[*])"}, + Script: `for arg in "$@"; do + echo "URL: $arg" + done`, + }}, + }}, + Params: Params{{ + Name: "b-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(tasks.matrix-emitting-results.results.report-url[*])"}, + }}, + }, + tasks: PipelineTaskList{}, + }, { + name: "invalid matrix emitting strings results consumed using array indexing by another pipelineTask", + pt: &PipelineTask{ + Name: "matrix-emitting-results", + TaskRef: &TaskRef{Name: "taskwithresult"}, + Matrix: &Matrix{ + Params: Params{{ + Name: "platform", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"linux", "mac"}}, + }, { + Name: "browser", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"chrome", "safari"}}, + }}}, + }, + tasks: PipelineTaskList{{ + Name: "taskwithresult", + TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ + Params: ParamSpecs{{ + Name: "platform", + }, { + Name: "browser"}}, + Results: []TaskResult{{ + Name: "report-url", + Type: ResultsTypeString, + }}, + Steps: []Step{{ + Name: "produce-report-url", + Image: "alpine", + Script: ` | + echo -n "https://api.example/get-report/$(params.platform)-$(params.browser)" | tee $(results.report-url.path)`}}, + }}, + }, { + Name: "task-consuming-results", + TaskRef: &TaskRef{Name: "echoarrayurl"}, + Params: Params{{ + Name: "b-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(tasks.matrix-emitting-results.results.report-url[0])"}, + }}, + }}, + wantErrs: apis.ErrGeneric("A matrixed pipelineTask can only be consumed in aggregate using [*] notation, but is currently set to tasks.matrix-emitting-results.results.report-url[0]"), + }, { + name: "invalid matrix emitting array results consumed in aggregate by another pipelineTask", + pt: &PipelineTask{ + Name: "matrix-emitting-results", + TaskRef: &TaskRef{Name: "taskwithresult"}, + Matrix: &Matrix{ + Params: Params{{ + Name: "platform", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"linux", "mac"}}, + }, { + Name: "browser", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"chrome", "safari"}}, + }}}, + }, + tasks: PipelineTaskList{{ + Name: "taskwithresult", + TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ + Params: ParamSpecs{{ + Name: "platform", + }, { + Name: "browser"}}, + Results: []TaskResult{{ + Name: "array-result", + Type: ResultsTypeArray, + }}, + Steps: []Step{{ + Name: "produce-array-result", + Image: "alpine", + Script: ` | + echo -n "https://api.example/get-report/$(params.platform)-$(params.browser)" | tee $(results.array-result.path)`}}, + }}, + }, { + Name: "task-consuming-results", + TaskRef: &TaskRef{Name: "echoarrayurl"}, + Params: Params{{ + Name: "b-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(tasks.matrix-emitting-results.results.array-result[*])"}, + }}, + }}, + wantErrs: apis.ErrInvalidValue("Matrixed PipelineTasks emitting results must have an underlying type string, but result array-result has type array in pipelineTask", ""), + }, { + name: "invalid matrix emitting array results consumed in aggregate by another pipelineTask (embedded TaskSpec)", + pt: &PipelineTask{ + Name: "matrix-emitting-results", + TaskRef: &TaskRef{Name: "taskwithresult"}, + Matrix: &Matrix{ + Params: Params{{ + Name: "platform", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"linux", "mac"}}, + }, { + Name: "browser", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"chrome", "safari"}}, + }}}, + }, + tasks: PipelineTaskList{{ + Name: "taskwithresult", + TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ + Params: ParamSpecs{{ + Name: "platform", + }, { + Name: "browser"}}, + Results: []TaskResult{{ + Name: "array-result", + Type: ResultsTypeArray, + }}, + Steps: []Step{{ + Name: "produce-array-result", + Image: "alpine", + Script: ` | + echo -n "https://api.example/get-report/$(params.platform)-$(params.browser)" | tee $(results.array-result.path)`}}, + }}, + }, { + Name: "task-consuming-results", + TaskRef: &TaskRef{Name: "echoarrayurl"}, + Params: Params{{ + Name: "b-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(tasks.matrix-emitting-results-embedded.results.array-result[*])"}, + }}, + }}, + wantErrs: apis.ErrInvalidValue("Matrixed PipelineTasks emitting results must have an underlying type string, but result array-result has type array in pipelineTask", ""), }} for _, tt := range tests { + var tasksForTest []PipelineTask t.Run(tt.name, func(t *testing.T) { featureFlags, _ := config.NewFeatureFlagsFromMap(map[string]string{ "enable-api-fields": "alpha", @@ -969,8 +1162,10 @@ func TestPipelineTask_validateMatrix(t *testing.T) { FeatureFlags: featureFlags, Defaults: defaults, } + tasksForTest := append(tasksForTest, tasks...) + tasksForTest = append(tasksForTest, tt.tasks...) ctx := config.ToContext(context.Background(), cfg) - if d := cmp.Diff(tt.wantErrs.Error(), tt.pt.validateMatrix(ctx).Error()); d != "" { + if d := cmp.Diff(tt.wantErrs.Error(), tt.pt.validateMatrix(ctx, tasksForTest).Error()); d != "" { t.Errorf("PipelineTask.validateMatrix() errors diff %s", diff.PrintWantGot(d)) } }) diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go index 7529c273b4f..bc66ef176c6 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -90,7 +90,6 @@ func (ps *PipelineSpec) Validate(ctx context.Context) (errs *apis.FieldError) { errs = errs.Also(validateWhenExpressions(ps.Tasks, ps.Finally)) errs = errs.Also(validateMatrix(ctx, ps.Tasks).ViaField("tasks")) errs = errs.Also(validateMatrix(ctx, ps.Finally).ViaField("finally")) - errs = errs.Also(validateResultsFromMatrixedPipelineTasksNotConsumed(ps.Tasks, ps.Finally)) return errs } @@ -180,13 +179,14 @@ func (pt PipelineTask) Validate(ctx context.Context) (errs *apis.FieldError) { return //nolint:nakedret } -func (pt *PipelineTask) validateMatrix(ctx context.Context) (errs *apis.FieldError) { +func (pt *PipelineTask) validateMatrix(ctx context.Context, tasks []PipelineTask) (errs *apis.FieldError) { if pt.IsMatrixed() { // This is an alpha feature and will fail validation if it's used in a pipeline spec // 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.validateUniqueParams()) + errs = errs.Also(pt.Matrix.validateTaskResultsFromMatrixedPipelineTasksConsumed(tasks)) } errs = errs.Also(pt.Matrix.validateParameterInOneOfMatrixOrParams(pt.Params)) return errs @@ -212,15 +212,6 @@ func (pt PipelineTask) validateEmbeddedOrType() (errs *apis.FieldError) { return } -func (pt *PipelineTask) validateResultsFromMatrixedPipelineTasksNotConsumed(matrixedPipelineTasks sets.String) (errs *apis.FieldError) { - for _, ref := range PipelineTaskResultRefs(pt) { - if matrixedPipelineTasks.Has(ref.PipelineTask) { - errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("consuming results from matrixed task %s is not allowed", ref.PipelineTask), "")) - } - } - return errs -} - func (pt *PipelineTask) validateWorkspaces(workspaceNames sets.String) (errs *apis.FieldError) { workspaceBindingNames := sets.NewString() for i, ws := range pt.Workspaces { @@ -729,23 +720,89 @@ func validateGraph(tasks []PipelineTask) (errs *apis.FieldError) { func validateMatrix(ctx context.Context, tasks []PipelineTask) (errs *apis.FieldError) { for idx, task := range tasks { - errs = errs.Also(task.validateMatrix(ctx).ViaIndex(idx)) + errs = errs.Also(task.validateMatrix(ctx, tasks).ViaIndex(idx)) } return errs } -func validateResultsFromMatrixedPipelineTasksNotConsumed(tasks []PipelineTask, finally []PipelineTask) (errs *apis.FieldError) { - matrixedPipelineTasks := sets.String{} - for _, pt := range tasks { - if pt.IsMatrixed() { - matrixedPipelineTasks.Insert(pt.Name) +// createTaskMapping maps the PipelineTaskName to the PipelineTask to easily access +// the pipelineTask by Name +func createTaskMapping(tasks []PipelineTask) (taskMap map[string]PipelineTask) { + taskMapping := make(map[string]PipelineTask) + for _, task := range tasks { + taskMapping[task.Name] = task + } + return taskMapping +} + +// findAndValidateResultRefsForMatrix checks that any result references to Matrixed PipelineTasks if consumed +// by another PipelineTask that the entire array of results produced by a matrix is consumed in aggregate +// since consuming a singular result produced by a matrix is currently not supported +func findAndValidateResultRefsForMatrix(tasks []PipelineTask, taskMapping map[string]PipelineTask) (resultRefs []*ResultRef, errs *apis.FieldError) { + for _, t := range tasks { + for _, p := range t.Params { + if expressions, ok := GetVarSubstitutionExpressionsForParam(p); ok { + if LooksLikeContainsResultRefs(expressions) { + resultRefs, errs = validateMatrixedPipelineTaskConsumed(expressions, taskMapping) + if errs != nil { + return nil, errs + } + } + } } } - for idx, pt := range tasks { - errs = errs.Also(pt.validateResultsFromMatrixedPipelineTasksNotConsumed(matrixedPipelineTasks).ViaFieldIndex("tasks", idx)) + return resultRefs, errs +} + +// validateMatrixedPipelineTaskConsumed checks that any Matrixed Pipeline Task that the is becing consumed is consumed in +// aggregate [*] since consuming a singular result produced by a matrix is currently not supported +func validateMatrixedPipelineTaskConsumed(expressions []string, taskMapping map[string]PipelineTask) (resultRefs []*ResultRef, errs *apis.FieldError) { + var filteredExpressions []string + for _, expression := range expressions { + // ie. "tasks..results.[*]" + subExpressions := strings.Split(expression, ".") + pipelineTask := subExpressions[1] // pipelineTaskName + taskConsumed := taskMapping[pipelineTask] + if taskConsumed.IsMatrixed() { + if !strings.HasSuffix(expression, "[*]") { + errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("A matrixed pipelineTask can only be consumed in aggregate using [*] notation, but is currently set to %s", expression))) + } + filteredExpressions = append(filteredExpressions, expression) + } + } + return NewResultRefs(filteredExpressions), errs +} + +// validateMatrixEmittingStringResults checks a matrix emitting results can only emit results with the underlying type string +// if those results are being consumed by another PipelineTask. +func validateMatrixEmittingStringResults(resultRefs []*ResultRef, taskMapping map[string]PipelineTask) (errs *apis.FieldError) { + for _, resultRef := range resultRefs { + task := taskMapping[resultRef.PipelineTask] + resultName := resultRef.Result + if task.TaskRef != nil { + referencedTaskName := task.TaskRef.Name + referencedTask := taskMapping[referencedTaskName] + if referencedTask.TaskSpec != nil { + errs = errs.Also(validateStringResults(referencedTask.TaskSpec.Results, resultName)) + } + } else if task.TaskSpec != nil { + errs = errs.Also(validateStringResults(task.TaskSpec.Results, resultName)) + } } - for idx, pt := range finally { - errs = errs.Also(pt.validateResultsFromMatrixedPipelineTasksNotConsumed(matrixedPipelineTasks).ViaFieldIndex("finally", idx)) + return errs +} + +// validateStringResults ensure that the result type is string +func validateStringResults(results []TaskResult, resultName string) (errs *apis.FieldError) { + for _, result := range results { + if result.Name == resultName { + if result.Type != ResultsTypeString { + errs = errs.Also(apis.ErrInvalidValue( + fmt.Sprintf("Matrixed PipelineTasks emitting results must have an underlying type string, but result %s has type %s in pipelineTask", resultName, string(result.Type)), + "", + )) + } + } } return errs } diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go index 1ebd0604678..051fe6317cf 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go @@ -3618,6 +3618,7 @@ func Test_validateMatrix(t *testing.T) { tests := []struct { name string tasks []PipelineTask + finally []PipelineTask wantErrs *apis.FieldError }{{ name: "parameter in both matrix and params", @@ -3673,35 +3674,7 @@ func Test_validateMatrix(t *testing.T) { Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.foo-task.results.a-task-results[*])"}}, }}}, }}, - }} - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - featureFlags, _ := config.NewFeatureFlagsFromMap(map[string]string{ - "enable-api-fields": "alpha", - }) - defaults := &config.Defaults{ - DefaultMaxMatrixCombinationsCount: 4, - } - cfg := &config.Config{ - FeatureFlags: featureFlags, - Defaults: defaults, - } - - 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 Test_validateResultsFromMatrixedPipelineTasksNotConsumed(t *testing.T) { - tests := []struct { - name string - tasks []PipelineTask - finally []PipelineTask - wantErrs *apis.FieldError - }{{ + }, { name: "results from matrixed task consumed in tasks through parameters", tasks: PipelineTaskList{{ Name: "a-task", @@ -3714,13 +3687,9 @@ func Test_validateResultsFromMatrixedPipelineTasksNotConsumed(t *testing.T) { Name: "b-task", TaskRef: &TaskRef{Name: "b-task"}, Params: Params{{ - Name: "b-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.a-task.results.a-result)"}}, + Name: "b-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.a-task.results.a-result[*])"}}, }}, }}, - wantErrs: &apis.FieldError{ - Message: "invalid value: consuming results from matrixed task a-task is not allowed", - Paths: []string{"tasks[1]"}, - }, }, { name: "results from matrixed task consumed in finally through parameters", tasks: PipelineTaskList{{ @@ -3735,13 +3704,9 @@ func Test_validateResultsFromMatrixedPipelineTasksNotConsumed(t *testing.T) { Name: "b-task", TaskRef: &TaskRef{Name: "b-task"}, Params: Params{{ - Name: "b-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.a-task.results.a-result)"}}, + Name: "b-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.a-task.results.a-result[*])"}}, }}, }}, - wantErrs: &apis.FieldError{ - Message: "invalid value: consuming results from matrixed task a-task is not allowed", - Paths: []string{"finally[0]"}, - }, }, { name: "results from matrixed task consumed in tasks and finally through parameters", tasks: PipelineTaskList{{ @@ -3755,20 +3720,16 @@ func Test_validateResultsFromMatrixedPipelineTasksNotConsumed(t *testing.T) { Name: "b-task", TaskRef: &TaskRef{Name: "b-task"}, Params: Params{{ - Name: "b-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.a-task.results.a-result)"}}, + Name: "b-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.a-task.results.a-result[*])"}}, }}, }}, finally: PipelineTaskList{{ Name: "c-task", TaskRef: &TaskRef{Name: "c-task"}, Params: Params{{ - Name: "b-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.a-task.results.a-result)"}}, + Name: "b-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.a-task.results.a-result[*])"}}, }}, }}, - wantErrs: &apis.FieldError{ - Message: "invalid value: consuming results from matrixed task a-task is not allowed", - Paths: []string{"tasks[1]", "finally[0]"}, - }, }, { name: "results from matrixed task consumed in tasks through when expressions", tasks: PipelineTaskList{{ @@ -3784,13 +3745,9 @@ func Test_validateResultsFromMatrixedPipelineTasksNotConsumed(t *testing.T) { WhenExpressions: WhenExpressions{{ Input: "foo", Operator: selection.In, - Values: []string{"$(tasks.a-task.results.a-result)"}, + Values: []string{"$(tasks.a-task.results.a-result[*])"}, }}, }}, - wantErrs: &apis.FieldError{ - Message: "invalid value: consuming results from matrixed task a-task is not allowed", - Paths: []string{"tasks[1]"}, - }, }, { name: "results from matrixed task consumed in finally through when expressions", tasks: PipelineTaskList{{ @@ -3805,15 +3762,11 @@ func Test_validateResultsFromMatrixedPipelineTasksNotConsumed(t *testing.T) { Name: "b-task", TaskRef: &TaskRef{Name: "b-task"}, WhenExpressions: WhenExpressions{{ - Input: "$(tasks.a-task.results.a-result)", + Input: "$(tasks.a-task.results.a-result[*])", Operator: selection.In, Values: []string{"foo", "bar"}, }}, }}, - wantErrs: &apis.FieldError{ - Message: "invalid value: consuming results from matrixed task a-task is not allowed", - Paths: []string{"finally[0]"}, - }, }, { name: "results from matrixed task consumed in tasks and finally through when expressions", tasks: PipelineTaskList{{ @@ -3827,7 +3780,7 @@ func Test_validateResultsFromMatrixedPipelineTasksNotConsumed(t *testing.T) { Name: "b-task", TaskRef: &TaskRef{Name: "b-task"}, WhenExpressions: WhenExpressions{{ - Input: "$(tasks.a-task.results.a-result)", + Input: "$(tasks.a-task.results.a-result[*])", Operator: selection.In, Values: []string{"foo", "bar"}, }}, @@ -3838,18 +3791,26 @@ func Test_validateResultsFromMatrixedPipelineTasksNotConsumed(t *testing.T) { WhenExpressions: WhenExpressions{{ Input: "foo", Operator: selection.In, - Values: []string{"$(tasks.a-task.results.a-result)"}, + Values: []string{"$(tasks.a-task.results.a-result[*])"}, }}, }}, - wantErrs: &apis.FieldError{ - Message: "invalid value: consuming results from matrixed task a-task is not allowed", - Paths: []string{"tasks[1]", "finally[0]"}, - }, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if d := cmp.Diff(tt.wantErrs.Error(), validateResultsFromMatrixedPipelineTasksNotConsumed(tt.tasks, tt.finally).Error()); d != "" { - t.Errorf("validateResultsFromMatrixedPipelineTasksNotConsumed() errors diff %s", diff.PrintWantGot(d)) + featureFlags, _ := config.NewFeatureFlagsFromMap(map[string]string{ + "enable-api-fields": "alpha", + }) + defaults := &config.Defaults{ + DefaultMaxMatrixCombinationsCount: 4, + } + cfg := &config.Config{ + FeatureFlags: featureFlags, + Defaults: defaults, + } + + 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)) } }) } diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index cea31b17fc8..5c4b7b034c1 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -818,14 +818,14 @@ func (c *Reconciler) runNextSchedulableTask(ctx context.Context, pr *v1.Pipeline } if rpt.IsCustomTask() { - rpt.CustomRuns, err = c.createCustomRuns(ctx, rpt, pr) + rpt.CustomRuns, err = c.createCustomRuns(ctx, rpt, pr, pipelineRunFacts) if err != nil { recorder.Eventf(pr, corev1.EventTypeWarning, "RunsCreationFailed", "Failed to create CustomRuns %q: %v", rpt.CustomRunNames, err) err = fmt.Errorf("error creating CustomRuns called %s for PipelineTask %s from PipelineRun %s: %w", rpt.CustomRunNames, rpt.PipelineTask.Name, pr.Name, err) return err } } else { - rpt.TaskRuns, err = c.createTaskRuns(ctx, rpt, pr) + rpt.TaskRuns, err = c.createTaskRuns(ctx, rpt, pr, pipelineRunFacts) if err != nil { recorder.Eventf(pr, corev1.EventTypeWarning, "TaskRunsCreationFailed", "Failed to create TaskRuns %q: %v", rpt.TaskRunNames, err) err = fmt.Errorf("error creating TaskRuns called %s for PipelineTask %s from PipelineRun %s: %w", rpt.TaskRunNames, rpt.PipelineTask.Name, pr.Name, err) @@ -846,7 +846,7 @@ func (c *Reconciler) setFinallyStartedTimeIfNeeded(pr *v1.PipelineRun, facts *re } } -func (c *Reconciler) createTaskRuns(ctx context.Context, rpt *resources.ResolvedPipelineTask, pr *v1.PipelineRun) ([]*v1.TaskRun, error) { +func (c *Reconciler) createTaskRuns(ctx context.Context, rpt *resources.ResolvedPipelineTask, pr *v1.PipelineRun, facts *resources.PipelineRunFacts) ([]*v1.TaskRun, error) { ctx, span := c.tracerProvider.Tracer(TracerName).Start(ctx, "createTaskRuns") defer span.End() var taskRuns []*v1.TaskRun @@ -860,7 +860,7 @@ func (c *Reconciler) createTaskRuns(ctx context.Context, rpt *resources.Resolved if len(matrixCombinations) > i { params = matrixCombinations[i] } - taskRun, err := c.createTaskRun(ctx, taskRunName, params, rpt, pr) + taskRun, err := c.createTaskRun(ctx, taskRunName, params, rpt, pr, facts) if err != nil { err := c.handleRunCreationError(ctx, pr, err) return nil, err @@ -870,11 +870,11 @@ func (c *Reconciler) createTaskRuns(ctx context.Context, rpt *resources.Resolved return taskRuns, nil } -func (c *Reconciler) createTaskRun(ctx context.Context, taskRunName string, params v1.Params, rpt *resources.ResolvedPipelineTask, pr *v1.PipelineRun) (*v1.TaskRun, error) { +func (c *Reconciler) createTaskRun(ctx context.Context, taskRunName string, params v1.Params, rpt *resources.ResolvedPipelineTask, pr *v1.PipelineRun, facts *resources.PipelineRunFacts) (*v1.TaskRun, error) { ctx, span := c.tracerProvider.Tracer(TracerName).Start(ctx, "createTaskRun") defer span.End() logger := logging.FromContext(ctx) - rpt.PipelineTask = resources.ApplyPipelineTaskContexts(rpt.PipelineTask) + rpt.PipelineTask = resources.ApplyPipelineTaskContexts(rpt.PipelineTask, pr.Status, facts) taskRunSpec := pr.GetTaskRunSpec(rpt.PipelineTask.Name) params = append(params, rpt.PipelineTask.Params...) tr := &v1.TaskRun{ @@ -946,7 +946,7 @@ func (c *Reconciler) handleRunCreationError(ctx context.Context, pr *v1.Pipeline return err } -func (c *Reconciler) createCustomRuns(ctx context.Context, rpt *resources.ResolvedPipelineTask, pr *v1.PipelineRun) ([]*v1beta1.CustomRun, error) { +func (c *Reconciler) createCustomRuns(ctx context.Context, rpt *resources.ResolvedPipelineTask, pr *v1.PipelineRun, facts *resources.PipelineRunFacts) ([]*v1beta1.CustomRun, error) { var customRuns []*v1beta1.CustomRun ctx, span := c.tracerProvider.Tracer(TracerName).Start(ctx, "createCustomRuns") defer span.End() @@ -960,7 +960,7 @@ func (c *Reconciler) createCustomRuns(ctx context.Context, rpt *resources.Resolv if len(matrixCombinations) > i { params = matrixCombinations[i] } - customRun, err := c.createCustomRun(ctx, customRunName, params, rpt, pr) + customRun, err := c.createCustomRun(ctx, customRunName, params, rpt, pr, facts) if err != nil { err := c.handleRunCreationError(ctx, pr, err) return nil, err @@ -970,11 +970,11 @@ func (c *Reconciler) createCustomRuns(ctx context.Context, rpt *resources.Resolv return customRuns, nil } -func (c *Reconciler) createCustomRun(ctx context.Context, runName string, params v1.Params, rpt *resources.ResolvedPipelineTask, pr *v1.PipelineRun) (*v1beta1.CustomRun, error) { +func (c *Reconciler) createCustomRun(ctx context.Context, runName string, params v1.Params, rpt *resources.ResolvedPipelineTask, pr *v1.PipelineRun, facts *resources.PipelineRunFacts) (*v1beta1.CustomRun, error) { ctx, span := c.tracerProvider.Tracer(TracerName).Start(ctx, "createCustomRun") defer span.End() logger := logging.FromContext(ctx) - rpt.PipelineTask = resources.ApplyPipelineTaskContexts(rpt.PipelineTask) + rpt.PipelineTask = resources.ApplyPipelineTaskContexts(rpt.PipelineTask, pr.Status, facts) taskRunSpec := pr.GetTaskRunSpec(rpt.PipelineTask.Name) params = append(params, rpt.PipelineTask.Params...) diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index f61d0cd0e5d..d244ac48709 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -24,6 +24,7 @@ import ( "encoding/json" "errors" "fmt" + "sort" "strconv" "testing" "time" @@ -12340,6 +12341,1474 @@ spec: } } +func TestReconciler_PipelineTaskMatrixExplicitCombosResultsAndMatrixContextVars(t *testing.T) { + names.TestingSeed() + task1 := parse.MustParseV1Task(t, ` +metadata: + name: arraytask + namespace: foo +spec: + params: + - name: DIGEST + type: array + steps: + - name: use-environments + image: bash:latest + args: ["$(params.DIGEST[*])"] + script: | + for arg in "$@"; do + echo "Arg: $arg" + done +`) + task2 := parse.MustParseV1Task(t, ` +metadata: + name: stringtask + namespace: foo +spec: + params: + - name: DIGEST + type: string + steps: + - name: echo + image: alpine + script: | + echo "$(params.DIGEST)" +`) + task3 := parse.MustParseV1Task(t, ` +metadata: + name: echomatrixlength + namespace: foo +spec: + params: + - name: matrixlength + type: string + steps: + - name: echo + image: alpine + script: echo $(params.matrixlength) +`) + task4 := parse.MustParseV1Task(t, ` +metadata: + name: echomatrixresultslength + namespace: foo +spec: + params: + - name: matrixresultslength + type: string + steps: + - name: echo + image: alpine + script: echo $(params.matrixresultslength) +`) + + taskwithresults := parse.MustParseV1Task(t, ` +metadata: + name: taskwithresults + namespace: foo +spec: + params: + - name: IMAGE + - name: DIGEST + default: "" + results: + - name: IMAGE-DIGEST + steps: + - name: produce-results + image: bash:latest + script: | + #!/usr/bin/env bash + echo "Building image for $(params.IMAGE)" + echo -n "$(params.DIGEST)" | sha256sum | tee $(results.IMAGE-DIGEST.path) +`) + taskRuns := []*v1.TaskRun{ + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-matrix-emitting-results-0", "foo", + "pr", "p", "matrix-emitting-results", false), + ` +spec: + params: + - name: DOCKERFILE + value: path/to/Dockerfile1 + - name: IMAGE + value: image-1 + serviceAccountName: test-sa + taskRef: + name: arraytask + kind: Task +status: + conditions: + - type: Succeeded + status: "True" + reason: Succeeded + message: All Tasks have completed executing + results: + - name: IMAGE-DIGEST + value: 0cf457e24a479f02fd4d34540389f720f0807dcff92a7562108165b2637ea82f + - name: IMAGE-NAME + value: image-1 +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-matrix-emitting-results-1", "foo", + "pr", "p", "matrix-emitting-results", false), + ` +spec: + params: + - name: DOCKERFILE + value: path/to/Dockerfile2 + - name: IMAGE + value: image-2 + serviceAccountName: test-sa + taskRef: + name: arraytask + kind: Task +status: + conditions: + - type: Succeeded + status: "True" + reason: Succeeded + message: All Tasks have completed executing + results: + - name: IMAGE-DIGEST + value: 5a0717cb6596468ea1dffa86011f9b0f497348d80421835b51799f9aeb455642 + - name: IMAGE-NAME + value: image-2 +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-matrix-emitting-results-2", "foo", + "pr", "p", "matrix-emitting-results", false), + ` +spec: + params: + - name: DOCKERFILE + value: path/to/Dockerfile3 + - name: IMAGE + value: image-3 + serviceAccountName: test-sa + taskRef: + name: arraytask + kind: Task +status: + conditions: + - type: Succeeded + status: "True" + reason: Succeeded + message: All Tasks have completed executing + results: + - name: IMAGE-DIGEST + value: d9f313aef2d97e58def0511fdc17512d53e6b30d578860ae04b5288c6a239010 + - name: IMAGE-NAME + value: image-3 +`), + } + cms := []*corev1.ConfigMap{withEnabledAlphaAPIFields(newFeatureFlagsConfigMap())} + cms = append(cms, withMaxMatrixCombinationsCount(newDefaultsConfigMap(), 10)) + tests := []struct { + name string + memberOf string + p *v1.Pipeline + expectedTaskRuns []*v1.TaskRun + expectedPipelineRun *v1.PipelineRun + }{{ + name: "p-dag", + memberOf: "tasks", + p: parse.MustParseV1Pipeline(t, fmt.Sprintf(` +metadata: + name: %s + namespace: foo +spec: + tasks: + - name: matrix-emitting-results + matrix: + include: + - name: build-1 + params: + - name: IMAGE + value: image-1 + - name: DOCKERFILE + value: path/to/Dockerfile1 + - name: build-2 + params: + - name: IMAGE + value: image-2 + - name: DOCKERFILE + value: path/to/Dockerfile2 + - name: build-3 + params: + - name: IMAGE + value: image-3 + - name: DOCKERFILE + value: path/to/Dockerfile3 + taskRef: + name: taskwithresults + kind: Task + - name: task-consuming-results + taskRef: + name: arraytask + kind: Task + params: + - name: NAME + value: $(tasks.matrix-emitting-results.results.IMAGE-NAME[*]) + - name: DIGEST + value: $(tasks.matrix-emitting-results.results.IMAGE-DIGEST[*]) + - name: matrix-task-consuming-results + taskRef: + name: stringtask + kind: Task + matrix: + params: + - name: DIGEST + value: $(tasks.matrix-emitting-results.results.IMAGE-DIGEST[*]) +`, "p-dag")), + expectedTaskRuns: []*v1.TaskRun{ + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-matrix-task-consuming-results-0", "foo", + "pr", "p", "matrix-task-consuming-results", false), + ` +spec: + params: + - name: DIGEST + value: 0cf457e24a479f02fd4d34540389f720f0807dcff92a7562108165b2637ea82f + serviceAccountName: test-sa + taskRef: + name: stringtask + kind: Task +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-matrix-task-consuming-results-1", "foo", + "pr", "p", "matrix-task-consuming-results", false), + ` +spec: + params: + - name: DIGEST + value: 5a0717cb6596468ea1dffa86011f9b0f497348d80421835b51799f9aeb455642 + serviceAccountName: test-sa + taskRef: + name: stringtask + kind: Task +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-matrix-task-consuming-results-2", "foo", + "pr", "p", "matrix-task-consuming-results", false), + ` +spec: + params: + - name: DIGEST + value: d9f313aef2d97e58def0511fdc17512d53e6b30d578860ae04b5288c6a239010 + serviceAccountName: test-sa + taskRef: + name: stringtask + kind: Task +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-task-consuming-results", "foo", + "pr", "p", "task-consuming-results", false), + ` +spec: + params: + - name: NAME + value: + - image-1 + - image-2 + - image-3 + - name: DIGEST + value: + - 0cf457e24a479f02fd4d34540389f720f0807dcff92a7562108165b2637ea82f + - 5a0717cb6596468ea1dffa86011f9b0f497348d80421835b51799f9aeb455642 + - d9f313aef2d97e58def0511fdc17512d53e6b30d578860ae04b5288c6a239010 + serviceAccountName: test-sa + taskRef: + name: arraytask + kind: Task +`), + }, + expectedPipelineRun: parse.MustParseV1PipelineRun(t, ` +metadata: + name: pr + namespace: foo + annotations: {} + labels: + tekton.dev/pipeline: p-dag +spec: + taskRunTemplate: + serviceAccountName: test-sa + pipelineRef: + name: p-dag +status: + pipelineSpec: + tasks: + - name: matrix-emitting-results + matrix: + include: + - name: build-1 + params: + - name: IMAGE + value: image-1 + - name: DOCKERFILE + value: path/to/Dockerfile1 + - name: build-2 + params: + - name: IMAGE + value: image-2 + - name: DOCKERFILE + value: path/to/Dockerfile2 + - name: build-3 + params: + - name: IMAGE + value: image-3 + - name: DOCKERFILE + value: path/to/Dockerfile3 + taskRef: + kind: Task + name: taskwithresults + - name: task-consuming-results + taskRef: + name: arraytask + kind: Task + params: + - name: NAME + value: $(tasks.matrix-emitting-results.results.IMAGE-NAME[*]) + - name: DIGEST + value: $(tasks.matrix-emitting-results.results.IMAGE-DIGEST[*]) + - name: matrix-task-consuming-results + taskRef: + name: stringtask + kind: Task + matrix: + params: + - name: DIGEST + value: $(tasks.matrix-emitting-results.results.IMAGE-DIGEST[*]) + conditions: + - type: Succeeded + status: "Unknown" + reason: "Running" + message: "Tasks Completed: 1 (Failed: 0, Cancelled 0), Incomplete: 2, Skipped: 0" + childReferences: + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-emitting-results-0 + pipelineTaskName: matrix-emitting-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-emitting-results-1 + pipelineTaskName: matrix-emitting-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-emitting-results-2 + pipelineTaskName: matrix-emitting-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-task-consuming-results-0 + pipelineTaskName: matrix-task-consuming-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-task-consuming-results-1 + pipelineTaskName: matrix-task-consuming-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-task-consuming-results-2 + pipelineTaskName: matrix-task-consuming-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-task-consuming-results + pipelineTaskName: task-consuming-results +`), + }, { + name: "p-dag-2", + memberOf: "tasks", + p: parse.MustParseV1Pipeline(t, fmt.Sprintf(` +metadata: + name: %s + namespace: foo +spec: + tasks: + - name: matrix-emitting-results + matrix: + include: + - name: build-1 + params: + - name: IMAGE + value: image-1 + - name: DOCKERFILE + value: path/to/Dockerfile1 + - name: build-2 + params: + - name: IMAGE + value: image-2 + - name: DOCKERFILE + value: path/to/Dockerfile2 + - name: build-3 + params: + - name: IMAGE + value: image-3 + - name: DOCKERFILE + value: path/to/Dockerfile3 + taskRef: + name: taskwithresults + kind: Task + - name: matrixed-echo-length + params: + - name: matrixlength + value: $(tasks.matrix-emitting-results.matrix.length) + taskRef: + name: echomatrixlength + kind: Task + - name: matrixed-echo-results-length + params: + - name: matrixresultslength + value: $(tasks.matrix-emitting-results.matrix.IMAGE-DIGEST.length) + taskRef: + name: echomatrixresultslength + kind: Task +`, "p-dag-2")), + expectedTaskRuns: []*v1.TaskRun{ + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-matrixed-echo-length", "foo", + "pr", "p", "matrixed-echo-length", false), + ` +spec: + params: + - name: matrixlength + value: 3 + serviceAccountName: test-sa + taskRef: + name: echomatrixlength + kind: Task +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-matrixed-echo-results-length", "foo", + "pr", "p", "matrixed-echo-results-length", false), + ` +spec: + params: + - name: matrixresultslength + value: 3 + serviceAccountName: test-sa + taskRef: + name: echomatrixresultslength + kind: Task +`), + }, + expectedPipelineRun: parse.MustParseV1PipelineRun(t, ` +metadata: + name: pr + namespace: foo + annotations: {} + labels: + tekton.dev/pipeline: p-dag-2 +spec: + taskRunTemplate: + serviceAccountName: test-sa + pipelineRef: + name: p-dag-2 +status: + pipelineSpec: + tasks: + - name: matrix-emitting-results + matrix: + include: + - name: build-1 + params: + - name: IMAGE + value: image-1 + - name: DOCKERFILE + value: path/to/Dockerfile1 + - name: build-2 + params: + - name: IMAGE + value: image-2 + - name: DOCKERFILE + value: path/to/Dockerfile2 + - name: build-3 + params: + - name: IMAGE + value: image-3 + - name: DOCKERFILE + value: path/to/Dockerfile3 + taskRef: + kind: Task + name: taskwithresults + - name: matrixed-echo-length + params: + - name: matrixlength + value: $(tasks.matrix-emitting-results.matrix.length) + taskRef: + name: echomatrixlength + kind: Task + - name: matrixed-echo-results-length + params: + - name: matrixresultslength + value: $(tasks.matrix-emitting-results.matrix.IMAGE-DIGEST.length) + taskRef: + name: echomatrixresultslength + kind: Task + conditions: + - type: Succeeded + status: "Unknown" + reason: "Running" + message: "Tasks Completed: 1 (Failed: 0, Cancelled 0), Incomplete: 2, Skipped: 0" + childReferences: + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-emitting-results-0 + pipelineTaskName: matrix-emitting-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-emitting-results-1 + pipelineTaskName: matrix-emitting-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-emitting-results-2 + pipelineTaskName: matrix-emitting-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrixed-echo-length + pipelineTaskName: matrixed-echo-length + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrixed-echo-results-length + pipelineTaskName: matrixed-echo-results-length +`), + }, { + name: "p-finally", + memberOf: "finally", + p: parse.MustParseV1Pipeline(t, fmt.Sprintf(` +metadata: + name: %s + namespace: foo +spec: + tasks: + - name: matrix-emitting-results + matrix: + include: + - name: build-1 + params: + - name: IMAGE + value: image-1 + - name: DOCKERFILE + value: path/to/Dockerfile1 + - name: build-2 + params: + - name: IMAGE + value: image-2 + - name: DOCKERFILE + value: path/to/Dockerfile2 + - name: build-3 + params: + - name: IMAGE + value: image-3 + - name: DOCKERFILE + value: path/to/Dockerfile3 + taskRef: + name: taskwithresults + kind: Task + finally: + - name: matrixed-echo-length + params: + - name: matrixlength + value: $(tasks.matrix-emitting-results.matrix.length) + taskRef: + name: echomatrixlength + kind: Task + - name: matrixed-echo-results-length + params: + - name: matrixresultslength + value: $(tasks.matrix-emitting-results.matrix.IMAGE-DIGEST.length) + taskRef: + name: echomatrixresultslength + kind: Task +`, "p-finally")), + + expectedTaskRuns: []*v1.TaskRun{ + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-matrixed-echo-length", "foo", + "pr", "p-finally", "matrixed-echo-length", false), + ` +spec: + params: + - name: matrixlength + value: 3 + serviceAccountName: test-sa + taskRef: + name: echomatrixlength + kind: Task +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-matrixed-echo-results-length", "foo", + "pr", "p-finally", "matrixed-echo-results-length", false), + ` +spec: + params: + - name: matrixresultslength + value: 3 + serviceAccountName: test-sa + taskRef: + name: echomatrixresultslength + kind: Task +`), + }, + expectedPipelineRun: parse.MustParseV1PipelineRun(t, ` +metadata: + name: pr + namespace: foo + annotations: {} + labels: + tekton.dev/pipeline: p-finally +spec: + taskRunTemplate: + serviceAccountName: test-sa + pipelineRef: + name: p-finally +status: + pipelineSpec: + tasks: + - name: matrix-emitting-results + matrix: + include: + - name: build-1 + params: + - name: IMAGE + value: image-1 + - name: DOCKERFILE + value: path/to/Dockerfile1 + - name: build-2 + params: + - name: IMAGE + value: image-2 + - name: DOCKERFILE + value: path/to/Dockerfile2 + - name: build-3 + params: + - name: IMAGE + value: image-3 + - name: DOCKERFILE + value: path/to/Dockerfile3 + taskRef: + name: taskwithresults + kind: Task + finally: + - name: matrixed-echo-length + params: + - name: matrixlength + value: $(tasks.matrix-emitting-results.matrix.length) + taskRef: + name: echomatrixlength + kind: Task + - name: matrixed-echo-results-length + params: + - name: matrixresultslength + value: $(tasks.matrix-emitting-results.matrix.IMAGE-DIGEST.length) + taskRef: + name: echomatrixresultslength + kind: Task + conditions: + - type: Succeeded + status: "Unknown" + reason: "Running" + message: "Tasks Completed: 1 (Failed: 0, Cancelled 0), Incomplete: 2, Skipped: 0" + childReferences: + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-emitting-results-0 + pipelineTaskName: matrix-emitting-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-emitting-results-1 + pipelineTaskName: matrix-emitting-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-emitting-results-2 + pipelineTaskName: matrix-emitting-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrixed-echo-length + pipelineTaskName: matrixed-echo-length + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrixed-echo-results-length + pipelineTaskName: matrixed-echo-results-length +`), + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pr := parse.MustParseV1PipelineRun(t, fmt.Sprintf(` +metadata: + name: pr + namespace: foo +spec: + taskRunTemplate: + serviceAccountName: test-sa + pipelineRef: + name: %s +`, tt.name)) + d := test.Data{ + PipelineRuns: []*v1.PipelineRun{pr}, + Pipelines: []*v1.Pipeline{tt.p}, + Tasks: []*v1.Task{task1, task2, task3, task4, taskwithresults}, + TaskRuns: taskRuns, + ConfigMaps: cms, + } + prt := newPipelineRunTest(t, d) + defer prt.Cancel() + _, clients := prt.reconcileRun("foo", "pr", []string{}, false) + taskRuns, err := clients.Pipeline.TektonV1().TaskRuns("foo").List(prt.TestAssets.Ctx, metav1.ListOptions{ + LabelSelector: fmt.Sprintf("tekton.dev/pipelineRun=pr,tekton.dev/pipeline=%s", tt.name), + Limit: 1, + }) + if err != nil { + t.Fatalf("Failure to list TaskRun's %s", err) + } + if len(taskRuns.Items) != len(tt.expectedTaskRuns) { + t.Fatalf("Expected %d TaskRuns got %d", len(tt.expectedTaskRuns), len(taskRuns.Items)) + } + sort.Slice(taskRuns.Items, func(i, j int) bool { + return taskRuns.Items[i].Name < taskRuns.Items[j].Name + }) + sort.Slice(tt.expectedTaskRuns, func(i, j int) bool { + return tt.expectedTaskRuns[i].Name < tt.expectedTaskRuns[j].Name + }) + for i := range taskRuns.Items { + expectedTaskRun := tt.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", tt.expectedTaskRuns[i].Name, diff.PrintWantGot(d)) + } + } + // pipelineRun, err := clients.Pipeline.TektonV1().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, ignoreProvenance, cmpopts.EquateEmpty(), cmpopts.SortSlices(lessChildReferences)); d != "" { + // t.Errorf("expected PipelineRun was not created. Diff %s", diff.PrintWantGot(d)) + // } + }) + } +} + +func TestReconciler_PipelineTaskMatrixConsumingResults(t *testing.T) { + names.TestingSeed() + task1 := parse.MustParseV1Task(t, ` +metadata: + name: arraytask + namespace: foo +spec: + params: + - name: result + type: array + steps: + - name: echo + image: alpine + script: | + echo "$(params.result)" +`) + + task2 := parse.MustParseV1Task(t, ` +metadata: + name: stringtask + namespace: foo +spec: + params: + - name: result + type: string + steps: + - name: echo + image: alpine + script: | + echo "$(params.result)" +`) + taskwithresults := parse.MustParseV1Task(t, ` +metadata: + name: taskwithresults + namespace: foo +spec: + params: + - name: platform + default: "" + - name: browser + default: "" + results: + - name: report-url + type: string + steps: + - name: produce-results + image: alpine + script: | + #!/usr/bin/env bash + echo "https://api.example/get-report/$(params.platform)-$(params.browser)" | tee $(results.report-url.path) +`) + trs := []*v1.TaskRun{ + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-matrix-emitting-results-0", "foo", + "pr", "p", "matrix-emitting-results", false), + ` +spec: + params: + - name: result + value: https://api.example/get-report/linux-chrome + serviceAccountName: test-sa + taskRef: + name: arraytask + kind: Task +status: + conditions: + - type: Succeeded + status: "True" + reason: Succeeded + message: All Tasks have completed executing + results: + - name: report-url + value: https://api.example/get-report/linux-chrome +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-matrix-emitting-results-1", "foo", + "pr", "p", "matrix-emitting-results", false), + ` +spec: + params: + - name: result + value: https://api.example/get-report/mac-chrome + serviceAccountName: test-sa + taskRef: + name: arraytask + kind: Task +status: + conditions: + - type: Succeeded + status: "True" + reason: Succeeded + message: All Tasks have completed executing + results: + - name: report-url + value: https://api.example/get-report/mac-chrome +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-matrix-emitting-results-2", "foo", + "pr", "p", "matrix-emitting-results", false), + ` +spec: + params: + - name: result + value: https://api.example/get-report/windows-chrome + serviceAccountName: test-sa + taskRef: + name: arraytask + kind: Task +status: + conditions: + - type: Succeeded + status: "True" + reason: Succeeded + message: All Tasks have completed executing + results: + - name: report-url + value: https://api.example/get-report/windows-chrome +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-matrix-emitting-results-3", "foo", + "pr", "p", "matrix-emitting-results", false), + ` +spec: + params: + - name: result + value: https://api.example/get-report/linux-safari + serviceAccountName: test-sa + taskRef: + name: arraytask + kind: Task +status: + conditions: + - type: Succeeded + status: "True" + reason: Succeeded + message: All Tasks have completed executing + results: + - name: report-url + value: https://api.example/get-report/linux-safari +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-matrix-emitting-results-4", "foo", + "pr", "p", "matrix-emitting-results", false), + ` +spec: + params: + - name: result + value: https://api.example/get-report/mac-safari + serviceAccountName: test-sa + taskRef: + name: arraytask + kind: Task +status: + conditions: + - type: Succeeded + status: "True" + reason: Succeeded + message: All Tasks have completed executing + results: + - name: report-url + value: https://api.example/get-report/mac-safari +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-matrix-emitting-results-5", "foo", + "pr", "p", "matrix-emitting-results", false), + ` +spec: + params: + - name: result + value: https://api.example/get-report/windows-safari + serviceAccountName: test-sa + taskRef: + name: arraytask + kind: Task +status: + conditions: + - type: Succeeded + status: "True" + reason: Succeeded + message: All Tasks have completed executing + results: + - name: report-url + value: https://api.example/get-report/windows-safari +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-matrix-emitting-results-6", "foo", + "pr", "p", "matrix-emitting-results", false), + ` +spec: + params: + - name: result + value: https://api.example/get-report/linux-firefox + serviceAccountName: test-sa + taskRef: + name: arraytask + kind: Task +status: + conditions: + - type: Succeeded + status: "True" + reason: Succeeded + message: All Tasks have completed executing + results: + - name: report-url + value: https://api.example/get-report/linux-firefox +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-matrix-emitting-results-7", "foo", + "pr", "p", "matrix-emitting-results", false), + ` +spec: + params: + - name: result + value: https://api.example/get-report/mac-firefox + serviceAccountName: test-sa + taskRef: + name: arraytask + kind: Task +status: + conditions: + - type: Succeeded + status: "True" + reason: Succeeded + message: All Tasks have completed executing + results: + - name: report-url + value: https://api.example/get-report/mac-firefox +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-matrix-emitting-results-8", "foo", + "pr", "p", "matrix-emitting-results", false), + ` +spec: + params: + - name: result + value: https://api.example/get-report/windows-firefox + serviceAccountName: test-sa + taskRef: + name: arraytask + kind: Task +status: + conditions: + - type: Succeeded + status: "True" + reason: Succeeded + message: All Tasks have completed executing + results: + - name: report-url + value: https://api.example/get-report/windows-firefox +`), + } + cms := []*corev1.ConfigMap{withEnabledAlphaAPIFields(newFeatureFlagsConfigMap())} + cms = append(cms, withMaxMatrixCombinationsCount(newDefaultsConfigMap(), 10)) + tests := []struct { + name string + memberOf string + p *v1.Pipeline + expectedTaskRuns []*v1.TaskRun + expectedPipelineRun *v1.PipelineRun + }{{ + name: "p-dag", + memberOf: "tasks", + p: parse.MustParseV1Pipeline(t, fmt.Sprintf(` +metadata: + name: %s + namespace: foo +spec: + tasks: + - name: matrix-emitting-results + matrix: + params: + - name: platform + value: + - linux + - mac + - windows + - name: browser + value: + - chrome + - safari + - firefox + taskRef: + name: taskwithresults + kind: Task + - name: task-consuming-results + taskRef: + name: arraytask + kind: Task + params: + - name: result + value: $(tasks.matrix-emitting-results.results.report-url[*]) +`, "p-dag")), + expectedTaskRuns: []*v1.TaskRun{ + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-task-consuming-results", "foo", + "pr", "p", "task-consuming-results", false), + ` +spec: + params: + - name: result + value: + - https://api.example/get-report/linux-chrome + - https://api.example/get-report/mac-chrome + - https://api.example/get-report/windows-chrome + - https://api.example/get-report/linux-safari + - https://api.example/get-report/mac-safari + - https://api.example/get-report/windows-safari + - https://api.example/get-report/linux-firefox + - https://api.example/get-report/mac-firefox + - https://api.example/get-report/windows-firefox + serviceAccountName: test-sa + taskRef: + name: arraytask + kind: Task +`), + }, + expectedPipelineRun: parse.MustParseV1PipelineRun(t, ` +metadata: + name: pr + namespace: foo + annotations: {} + labels: + tekton.dev/pipeline: p-dag +spec: + taskRunTemplate: + serviceAccountName: test-sa + pipelineRef: + name: p-dag +status: + pipelineSpec: + tasks: + - name: matrix-emitting-results + matrix: + params: + - name: platform + value: + - linux + - mac + - windows + - name: browser + value: + - chrome + - safari + - firefox + taskRef: + name: taskwithresults + kind: Task + - name: task-consuming-results + taskRef: + name: arraytask + kind: Task + params: + - name: result + value: $(tasks.matrix-emitting-results.results.report-url[*]) + conditions: + - type: Succeeded + status: "Unknown" + reason: "Running" + message: "Tasks Completed: 1 (Failed: 0, Cancelled 0), Incomplete: 1, Skipped: 0" + childReferences: + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-emitting-results-0 + pipelineTaskName: matrix-emitting-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-emitting-results-1 + pipelineTaskName: matrix-emitting-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-emitting-results-2 + pipelineTaskName: matrix-emitting-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-emitting-results-3 + pipelineTaskName: matrix-emitting-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-emitting-results-4 + pipelineTaskName: matrix-emitting-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-emitting-results-5 + pipelineTaskName: matrix-emitting-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-emitting-results-6 + pipelineTaskName: matrix-emitting-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-emitting-results-7 + pipelineTaskName: matrix-emitting-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-emitting-results-8 + pipelineTaskName: matrix-emitting-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-task-consuming-results + pipelineTaskName: task-consuming-results +`), + }, + { + name: "p-dag-2", + memberOf: "tasks", + p: parse.MustParseV1Pipeline(t, fmt.Sprintf(` +metadata: + name: %s + namespace: foo +spec: + tasks: + - name: matrix-emitting-results + matrix: + params: + - name: platform + value: + - linux + - mac + - windows + - name: browser + value: + - chrome + - safari + - firefox + taskRef: + name: taskwithresults + kind: Task + - name: matrix-consuming-results + taskRef: + name: stringtask + kind: Task + matrix: + params: + - name: result + value: $(tasks.matrix-emitting-results.results.report-url[*]) +`, "p-dag-2")), + expectedTaskRuns: []*v1.TaskRun{ + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-matrix-consuming-results-0", "foo", + "pr", "p", "matrix-consuming-results", false), + ` +spec: + params: + - name: result + value: https://api.example/get-report/linux-chrome + serviceAccountName: test-sa + taskRef: + name: stringtask + kind: Task +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-matrix-consuming-results-1", "foo", + "pr", "p", "matrix-consuming-results", false), + ` +spec: + params: + - name: result + value: https://api.example/get-report/mac-chrome + serviceAccountName: test-sa + taskRef: + name: stringtask + kind: Task +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-matrix-consuming-results-2", "foo", + "pr", "p", "matrix-consuming-results", false), + ` +spec: + params: + - name: result + value: https://api.example/get-report/windows-chrome + serviceAccountName: test-sa + taskRef: + name: stringtask + kind: Task +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-matrix-consuming-results-3", "foo", + "pr", "p", "matrix-consuming-results", false), + ` +spec: + params: + - name: result + value: https://api.example/get-report/linux-safari + serviceAccountName: test-sa + taskRef: + name: stringtask + kind: Task +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-matrix-consuming-results-4", "foo", + "pr", "p", "matrix-consuming-results", false), + ` +spec: + params: + - name: result + value: https://api.example/get-report/mac-safari + serviceAccountName: test-sa + taskRef: + name: stringtask + kind: Task +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-matrix-consuming-results-5", "foo", + "pr", "p", "matrix-consuming-results", false), + ` +spec: + params: + - name: result + value: https://api.example/get-report/windows-safari + serviceAccountName: test-sa + taskRef: + name: stringtask + kind: Task +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-matrix-consuming-results-6", "foo", + "pr", "p", "matrix-consuming-results", false), + ` +spec: + params: + - name: result + value: https://api.example/get-report/linux-firefox + serviceAccountName: test-sa + taskRef: + name: stringtask + kind: Task +`), mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-matrix-consuming-results-7", "foo", + "pr", "p", "matrix-consuming-results", false), + ` +spec: + params: + - name: result + value: https://api.example/get-report/mac-firefox + serviceAccountName: test-sa + taskRef: + name: stringtask + kind: Task +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-matrix-consuming-results-8", "foo", + "pr", "p", "matrix-consuming-results", false), + ` +spec: + params: + - name: result + value: https://api.example/get-report/windows-firefox + serviceAccountName: test-sa + taskRef: + name: stringtask + kind: Task +`), + }, + expectedPipelineRun: parse.MustParseV1PipelineRun(t, ` +metadata: + name: pr + namespace: foo + annotations: {} + labels: + tekton.dev/pipeline: p-dag-2 +spec: + taskRunTemplate: + serviceAccountName: test-sa + pipelineRef: + name: p-dag-2 +status: + pipelineSpec: + tasks: + - name: matrix-emitting-results + matrix: + params: + - name: platform + value: + - linux + - mac + - windows + - name: browser + value: + - chrome + - safari + - firefox + taskRef: + name: taskwithresults + kind: Task + - name: matrix-consuming-results + taskRef: + name: stringtask + kind: Task + matrix: + params: + - name: result + value: $(tasks.matrix-emitting-results.results.report-url[*]) + conditions: + - type: Succeeded + status: "Unknown" + reason: "Running" + message: "Tasks Completed: 1 (Failed: 0, Cancelled 0), Incomplete: 1, Skipped: 0" + childReferences: + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-emitting-results-0 + pipelineTaskName: matrix-emitting-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-emitting-results-1 + pipelineTaskName: matrix-emitting-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-emitting-results-2 + pipelineTaskName: matrix-emitting-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-emitting-results-3 + pipelineTaskName: matrix-emitting-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-emitting-results-4 + pipelineTaskName: matrix-emitting-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-emitting-results-5 + pipelineTaskName: matrix-emitting-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-emitting-results-6 + pipelineTaskName: matrix-emitting-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-emitting-results-7 + pipelineTaskName: matrix-emitting-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-emitting-results-8 + pipelineTaskName: matrix-emitting-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-consuming-results-0 + pipelineTaskName: matrix-consuming-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-consuming-results-1 + pipelineTaskName: matrix-consuming-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-consuming-results-2 + pipelineTaskName: matrix-consuming-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-consuming-results-3 + pipelineTaskName: matrix-consuming-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-consuming-results-4 + pipelineTaskName: matrix-consuming-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-consuming-results-5 + pipelineTaskName: matrix-consuming-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-consuming-results-6 + pipelineTaskName: matrix-consuming-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-consuming-results-7 + pipelineTaskName: matrix-consuming-results + - apiVersion: tekton.dev/v1 + kind: TaskRun + name: pr-matrix-consuming-results-8 + pipelineTaskName: matrix-consuming-results +`), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pr := parse.MustParseV1PipelineRun(t, fmt.Sprintf(` +metadata: + name: pr + namespace: foo +spec: + taskRunTemplate: + serviceAccountName: test-sa + pipelineRef: + name: %s +`, tt.name)) + d := test.Data{ + PipelineRuns: []*v1.PipelineRun{pr}, + Pipelines: []*v1.Pipeline{tt.p}, + Tasks: []*v1.Task{task1, task2, taskwithresults}, + TaskRuns: trs, + ConfigMaps: cms, + } + prt := newPipelineRunTest(t, d) + defer prt.Cancel() + _, clients := prt.reconcileRun("foo", "pr", []string{}, false) + taskRuns, err := clients.Pipeline.TektonV1().TaskRuns("foo").List(prt.TestAssets.Ctx, metav1.ListOptions{ + LabelSelector: fmt.Sprintf("tekton.dev/pipelineRun=pr,tekton.dev/pipeline=%s", tt.name), + Limit: 1, + }) + if err != nil { + t.Fatalf("Failure to list TaskRun's %s", err) + } + if len(taskRuns.Items) != len(tt.expectedTaskRuns) { + t.Fatalf("Expected %d TaskRuns got %d", len(tt.expectedTaskRuns), len(taskRuns.Items)) + } + for i := range taskRuns.Items { + expectedTaskRun := tt.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", tt.expectedTaskRuns[i].Name, diff.PrintWantGot(d)) + } + } + pipelineRun, err := clients.Pipeline.TektonV1().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, ignoreProvenance, cmpopts.EquateEmpty()); d != "" { + t.Errorf("expected PipelineRun was not created. Diff %s", diff.PrintWantGot(d)) + } + }) + } +} func TestReconcile_SetDefaults(t *testing.T) { names.TestingSeed() diff --git a/pkg/reconciler/pipelinerun/resources/apply.go b/pkg/reconciler/pipelinerun/resources/apply.go index b356947bbbd..7f1a9057900 100644 --- a/pkg/reconciler/pipelinerun/resources/apply.go +++ b/pkg/reconciler/pipelinerun/resources/apply.go @@ -150,13 +150,67 @@ func ApplyContexts(spec *v1.PipelineSpec, pipelineName string, pr *v1.PipelineRu return ApplyReplacements(spec, GetContextReplacements(pipelineName, pr), map[string][]string{}, map[string]map[string]string{}) } +// filterMatrixContextVar returns a list of params which contain any matrix context variables such as +// $(tasks..matrix.length) and $(tasks..matrix..length) +func filterMatrixContextVar(params v1.Params) v1.Params { + var filteredParams v1.Params + for _, param := range params { + if expressions, ok := param.GetVarSubstitutionExpressions(); ok { + for _, expression := range expressions { + // tasks..matrix.length + // tasks..matrix..length + subExpressions := strings.Split(expression, ".") + if subExpressions[2] == "matrix" && subExpressions[len(subExpressions)-1] == "length" { + filteredParams = append(filteredParams, param) + } + } + } + } + return filteredParams +} + // ApplyPipelineTaskContexts applies the substitution from $(context.pipelineTask.*) with the specified values. -// Uses "0" as a default if a value is not available. -func ApplyPipelineTaskContexts(pt *v1.PipelineTask) *v1.PipelineTask { +// Uses "0" as a default if a value is not available as well as matrix context variables +// $(tasks..matrix.length) and $(tasks..matrix..length) +func ApplyPipelineTaskContexts(pt *v1.PipelineTask, pipelineRunStatus v1.PipelineRunStatus, facts *PipelineRunFacts) *v1.PipelineTask { pt = pt.DeepCopy() + var pipelineTaskName string + var resultName string + var matrixLength int + replacements := map[string]string{ "context.pipelineTask.retries": strconv.Itoa(pt.Retries), } + + filteredParams := filterMatrixContextVar(pt.Params) + + for _, p := range filteredParams { + pipelineTaskName, resultName = p.ParseTaskandResultName() + // find the referenced pipelineTask to count the matrix combinations + if pipelineTaskName != "" && pipelineRunStatus.PipelineSpec != nil { + for _, task := range pipelineRunStatus.PipelineSpec.Tasks { + if task.Name == pipelineTaskName { + matrixLength = task.Matrix.CountCombinations() + replacements["tasks."+pipelineTaskName+".matrix.length"] = strconv.Itoa(matrixLength) + continue + } + } + } + // find the resultName from the ResultsCache + if pipelineTaskName != "" && resultName != "" { + for _, pt := range facts.State { + if pt.PipelineTask.Name == pipelineTaskName { + if len(pt.ResultsCache) == 0 { + pt.ResultsCache = createResultsCacheMatrixedTaskRuns(pt) + } + resultLength := len(pt.ResultsCache[resultName]) + replacements["tasks."+pipelineTaskName+".matrix."+resultName+".length"] = strconv.Itoa(resultLength) + continue + } + } + } + } + pt.Params = pt.Params.ReplaceVariables(replacements, map[string][]string{}, map[string]map[string]string{}) if pt.IsMatrixed() { pt.Matrix.Params = pt.Matrix.Params.ReplaceVariables(replacements, map[string][]string{}, map[string]map[string]string{}) diff --git a/pkg/reconciler/pipelinerun/resources/apply_test.go b/pkg/reconciler/pipelinerun/resources/apply_test.go index d03998064aa..2e0d57dc62a 100644 --- a/pkg/reconciler/pipelinerun/resources/apply_test.go +++ b/pkg/reconciler/pipelinerun/resources/apply_test.go @@ -3241,6 +3241,8 @@ func TestApplyPipelineTaskContexts(t *testing.T) { for _, tc := range []struct { description string pt v1.PipelineTask + prstatus v1.PipelineRunStatus + facts *resources.PipelineRunFacts want v1.PipelineTask }{{ description: "context retries replacement", @@ -3324,9 +3326,155 @@ func TestApplyPipelineTaskContexts(t *testing.T) { }}, }, }, + }, { + description: "matrix length context variable", + pt: v1.PipelineTask{ + Params: v1.Params{{ + Name: "matrixlength", + Value: *v1.NewStructuredValues("$(tasks.matrixed-task-run.matrix.length)"), + }}, + }, + prstatus: v1.PipelineRunStatus{ + PipelineRunStatusFields: v1.PipelineRunStatusFields{ + PipelineSpec: &v1.PipelineSpec{ + Tasks: []v1.PipelineTask{{ + Name: "matrixed-task-run", + Matrix: &v1.Matrix{ + Params: v1.Params{ + {Name: "platform", Value: *v1.NewStructuredValues("linux", "mac", "windows")}, + {Name: "browser", Value: *v1.NewStructuredValues("chrome", "firefox", "safari")}, + }}, + }}, + }, + }}, + want: v1.PipelineTask{ + Params: v1.Params{{ + Name: "matrixlength", + Value: *v1.NewStructuredValues("9"), + }}, + }, + }, { + description: "matrix length and matrix results length context variables in matrix include params ", + pt: v1.PipelineTask{ + Params: v1.Params{{ + Name: "matrixlength", + Value: *v1.NewStructuredValues("$(tasks.matrix-emitting-results.matrix.length)"), + }, { + Name: "matrixresultslength", + Value: *v1.NewStructuredValues("$(tasks.matrix-emitting-results.matrix.IMAGE-DIGEST.length)"), + }}, + }, + prstatus: v1.PipelineRunStatus{ + PipelineRunStatusFields: v1.PipelineRunStatusFields{ + PipelineSpec: &v1.PipelineSpec{ + Tasks: []v1.PipelineTask{{ + Name: "matrix-emitting-results", + TaskSpec: &v1.EmbeddedTask{ + TaskSpec: v1.TaskSpec{ + Params: []v1.ParamSpec{{ + Name: "IMAGE", + Type: v1.ParamTypeString, + }, { + Name: "DIGEST", + Type: v1.ParamTypeString, + }}, + Results: []v1.TaskResult{{ + Name: "IMAGE-DIGEST", + }}, + Steps: []v1.Step{{ + Name: "produce-results", + Image: "bash:latest", + Script: `#!/usr/bin/env bash\necho -n "$(params.DIGEST)" | sha256sum | tee $(results.IMAGE-DIGEST.path)"`, + }}, + }, + }, + Matrix: &v1.Matrix{ + Include: []v1.IncludeParams{{ + Name: "build-1", + Params: v1.Params{{ + Name: "DOCKERFILE", Value: *v1.NewStructuredValues("path/to/Dockerfile1"), + }, { + Name: "IMAGE", Value: *v1.NewStructuredValues("image-1"), + }}, + }, { + Name: "build-2", + Params: v1.Params{{ + Name: "DOCKERFILE", Value: *v1.NewStructuredValues("path/to/Dockerfile2"), + }, { + Name: "IMAGE", Value: *v1.NewStructuredValues("image-2"), + }}, + }, { + Name: "build-3", + Params: v1.Params{{ + Name: "DOCKERFILE", Value: *v1.NewStructuredValues("path/to/Dockerfile3"), + }, { + Name: "IMAGE", Value: *v1.NewStructuredValues("image-3"), + }}, + }}, + }}, + }, + }, + }, + }, + facts: &resources.PipelineRunFacts{ + State: resources.PipelineRunState{{ + PipelineTask: &v1.PipelineTask{ + Name: "matrix-emitting-results", + }, + TaskRunNames: []string{"matrix-emitting-results-0"}, + TaskRuns: []*v1.TaskRun{{ + ObjectMeta: metav1.ObjectMeta{ + Name: "matrix-emitting-results-0", + }, + Status: v1.TaskRunStatus{ + TaskRunStatusFields: v1.TaskRunStatusFields{ + Results: []v1.TaskRunResult{{ + Name: "IMAGE-DIGEST", + Value: *v1.NewStructuredValues("123"), + }}, + }, + }, + }, { + ObjectMeta: metav1.ObjectMeta{ + Name: "matrix-emitting-results-1", + }, + Status: v1.TaskRunStatus{ + TaskRunStatusFields: v1.TaskRunStatusFields{ + Results: []v1.TaskRunResult{{ + Name: "IMAGE-DIGEST", + Value: *v1.NewStructuredValues("456"), + }}, + }, + }, + }, { + ObjectMeta: metav1.ObjectMeta{ + Name: "matrix-emitting-results-2", + }, + Status: v1.TaskRunStatus{ + TaskRunStatusFields: v1.TaskRunStatusFields{ + Results: []v1.TaskRunResult{{ + Name: "IMAGE-DIGEST", + Value: *v1.NewStructuredValues("789"), + }}, + }, + }, + }, + }, + ResultsCache: map[string][]string{}, + }}, + }, + want: v1.PipelineTask{ + Params: v1.Params{{ + Name: "matrixlength", + Value: *v1.NewStructuredValues("3"), + }, { + Name: "matrixresultslength", + Value: *v1.NewStructuredValues("3"), + }}, + }, }} { t.Run(tc.description, func(t *testing.T) { - got := resources.ApplyPipelineTaskContexts(&tc.pt) + got := resources.ApplyPipelineTaskContexts(&tc.pt, tc.prstatus, tc.facts) if d := cmp.Diff(&tc.want, got); d != "" { t.Errorf(diff.PrintWantGot(d)) } diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go index 8e47b94a303..abbf5c4b78c 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go @@ -64,6 +64,7 @@ type ResolvedPipelineTask struct { CustomRuns []*v1beta1.CustomRun PipelineTask *v1.PipelineTask ResolvedTask *resources.ResolvedTask + ResultsCache map[string][]string } // isDone returns true only if the task is skipped, succeeded or failed @@ -741,3 +742,18 @@ func CheckMissingResultReferences(pipelineRunState PipelineRunState, targets Pip } return nil } + +// createResultsCacheMatrixedTaskRuns creates a cache of results that have been fanned out from a +// referenced matrixed PipelintTask so that you can easily access these results in subsequent Pipeline Tasks +func createResultsCacheMatrixedTaskRuns(rpt *ResolvedPipelineTask) (resultsCache map[string][]string) { + if len(rpt.ResultsCache) == 0 { + resultsCache = make(map[string][]string) + } + for _, taskRun := range rpt.TaskRuns { + results := taskRun.Status.Results + for _, result := range results { + resultsCache[result.Name] = append(resultsCache[result.Name], result.Value.StringVal) + } + } + return resultsCache +} diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go index 182a02f2e8c..c547db0252a 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go @@ -228,6 +228,21 @@ var customRuns = []v1beta1.CustomRun{{ var matrixedPipelineTask = &v1.PipelineTask{ Name: "task", + TaskSpec: &v1.EmbeddedTask{ + TaskSpec: v1.TaskSpec{ + Params: []v1.ParamSpec{{ + Name: "browser", + Type: v1.ParamTypeString, + }}, + Results: []v1.TaskResult{{ + Name: "BROWSER", + }}, + Steps: []v1.Step{{ + Name: "produce-results", + Image: "bash:latest", + Script: `#!/usr/bin/env bash\necho -n "$(params.browser)" | sha256sum | tee $(results.BROWSER.path)"`, + }}, + }}, Matrix: &v1.Matrix{ Params: v1.Params{{ Name: "browser", @@ -4729,3 +4744,71 @@ func TestIsRunning(t *testing.T) { }) } } + +func TestCreateResultsCacheMatrixedTaskRuns(t *testing.T) { + for _, tc := range []struct { + name string + rpt *ResolvedPipelineTask + want map[string][]string + }{{ + name: "matrixed taskrun with results", + rpt: &ResolvedPipelineTask{ + PipelineTask: matrixedPipelineTask, + TaskRuns: []*v1.TaskRun{{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "namespace", + Name: "matrix-task-with-results", + }, + Spec: v1.TaskRunSpec{}, + Status: v1.TaskRunStatus{ + TaskRunStatusFields: v1.TaskRunStatusFields{ + Results: []v1.TaskRunResult{{ + Name: "browser", + Type: "string", + Value: v1.ParamValue{Type: v1.ParamTypeString, StringVal: "chrome"}, + }, { + Name: "browser", + Type: "string", + Value: v1.ParamValue{Type: v1.ParamTypeString, StringVal: "safari"}, + }, { + Name: "platform", + Type: "string", + Value: v1.ParamValue{Type: v1.ParamTypeString, StringVal: "linux"}, + }}, + }, + }, + }}, + }, + want: map[string][]string{ + "browser": {"chrome", "safari"}, // 1 Child ref + "platform": {"linux"}, // 1 Child ref + }, + }, { + name: "matrixed taskrun without results", + rpt: &ResolvedPipelineTask{ + PipelineTask: matrixedPipelineTask, + TaskRuns: []*v1.TaskRun{{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "namespace", + Name: "matrix-task-with-results", + }, + Spec: v1.TaskRunSpec{}, + Status: v1.TaskRunStatus{ + TaskRunStatusFields: v1.TaskRunStatusFields{ + Results: []v1.TaskRunResult{{}}, + }, + }, + }}, + }, + want: map[string][]string{ + "": {""}, + }, + }} { + t.Run(tc.name, func(t *testing.T) { + got := createResultsCacheMatrixedTaskRuns(tc.rpt) + if !cmp.Equal(got, tc.want) { + t.Errorf("Did not get the expected ResultsCache for %s", tc.rpt.PipelineTask.Name) + } + }) + } +} diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go b/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go index 65882f4c987..088d757f136 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunstate.go @@ -157,14 +157,36 @@ func (state PipelineRunState) GetTaskRunsResults() map[string][]v1.TaskRunResult if !rpt.isSuccessful() { continue } - // Currently a Matrix cannot produce results so this is for a singular TaskRun - if len(rpt.TaskRuns) == 1 { + if rpt.PipelineTask.IsMatrixed() { + taskRunResults := ConvertResultsMapToTaskRunResults(rpt.ResultsCache) + if len(taskRunResults) > 0 { + results[rpt.PipelineTask.Name] = taskRunResults + } + } else { results[rpt.PipelineTask.Name] = rpt.TaskRuns[0].Status.Results } } return results } +// ConvertResultsMapToTaskRunResults converts the map of results from Matrixed PipelineTasks to a list +// of TaskRunResults to standard the format +func ConvertResultsMapToTaskRunResults(resultsMap map[string][]string) []v1.TaskRunResult { + var taskRunResults []v1.TaskRunResult + for result, val := range resultsMap { + taskRunResult := v1.TaskRunResult{ + Name: result, + Type: v1.ResultsTypeArray, + Value: v1.ParamValue{ + Type: v1.ParamTypeArray, + ArrayVal: val, + }, + } + taskRunResults = append(taskRunResults, taskRunResult) + } + return taskRunResults +} + // GetRunsResults returns a map of all successfully completed Runs in the state, with the pipeline task name as the key // and the results from the corresponding TaskRun as the value. It only includes runs which have completed successfully. func (state PipelineRunState) GetRunsResults() map[string][]v1beta1.CustomRunResult { diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go index 918337be4ae..31a75859ba6 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunstate_test.go @@ -2681,7 +2681,7 @@ func TestPipelineRunState_GetResultsFuncs(t *testing.T) { "matrixed-task-run-3", }, PipelineTask: &v1.PipelineTask{ - Name: "matrixed-task", + Name: "matrixed-task-with-results-cache", TaskRef: &v1.TaskRef{ Name: "task", Kind: "Task", @@ -2737,6 +2737,10 @@ func TestPipelineRunState_GetResultsFuncs(t *testing.T) { }}}, }, }}, + ResultsCache: map[string][]string{ + "browser": {"chrome", "safari"}, + "platform": {"linux"}, + }, }, { CustomRunNames: []string{ "matrixed-run-0", @@ -2845,6 +2849,15 @@ func TestPipelineRunState_GetResultsFuncs(t *testing.T) { Value: *v1.NewStructuredValues("rab"), }}, "successful-task-without-results-1": nil, + "matrixed-task-with-results-cache": {{ + Name: "browser", + Type: "array", + Value: v1.ParamValue{Type: v1.ParamTypeArray, ArrayVal: []string{"chrome", "safari"}}, + }, { + Name: "platform", + Type: "array", + Value: v1.ParamValue{Type: v1.ParamTypeArray, ArrayVal: []string{"linux"}}, + }}, } expectedRunResults := map[string][]v1beta1.CustomRunResult{ "successful-run-with-results-1": {{ @@ -2858,12 +2871,14 @@ func TestPipelineRunState_GetResultsFuncs(t *testing.T) { } actualTaskResults := state.GetTaskRunsResults() - if d := cmp.Diff(expectedTaskResults, actualTaskResults); d != "" { + if !cmp.Equal(expectedTaskResults, actualTaskResults) { + d := cmp.Diff(expectedTaskResults, actualTaskResults) t.Errorf("Didn't get expected TaskRun results map: %s", diff.PrintWantGot(d)) } actualRunResults := state.GetRunsResults() - if d := cmp.Diff(expectedRunResults, actualRunResults); d != "" { + if !cmp.Equal(expectedRunResults, actualRunResults) { + d := cmp.Diff(expectedRunResults, actualRunResults) t.Errorf("Didn't get expected Run results map: %s", diff.PrintWantGot(d)) } } @@ -3387,6 +3402,43 @@ func TestPipelineRunState_GetChildReferences(t *testing.T) { } } +func TestConvertResultsMapToTaskRunResults(t *testing.T) { + for _, tc := range []struct { + name string + resultsMap map[string][]string + want []v1.TaskRunResult + }{{ + name: "results map", + resultsMap: map[string][]string{ + "browser": {"chrome", "safari"}, + "platform": {"linux"}, + }, + want: []v1.TaskRunResult{{ + Name: "browser", + Type: "array", + Value: v1.ParamValue{Type: v1.ParamTypeArray, ArrayVal: []string{"chrome", "safari"}}, + }, { + Name: "platform", + Type: "array", + Value: v1.ParamValue{Type: v1.ParamTypeArray, ArrayVal: []string{"linux"}}, + }}, + }, { + name: "empty results map", + resultsMap: map[string][]string{}, + want: nil, + }} { + t.Run(tc.name, func(t *testing.T) { + got := ConvertResultsMapToTaskRunResults(tc.resultsMap) + sortTaskRunResults := func(x, y v1.TaskRunResult) bool { + return x.Name < y.Name + } + if d := cmp.Diff(got, tc.want, cmpopts.SortSlices(sortTaskRunResults)); d != "" { + t.Errorf("TestConvertResultsMapToTaskRunResults() did not produce expected results for test %s: %s", tc.name, diff.PrintWantGot(d)) + } + }) + } +} + func customRunWithName(name string) *v1beta1.CustomRun { return &v1beta1.CustomRun{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/reconciler/pipelinerun/resources/resultrefresolution.go b/pkg/reconciler/pipelinerun/resources/resultrefresolution.go index e4f78aa66b2..ec1a1d943c8 100644 --- a/pkg/reconciler/pipelinerun/resources/resultrefresolution.go +++ b/pkg/reconciler/pipelinerun/resources/resultrefresolution.go @@ -115,58 +115,74 @@ func removeDup(refs ResolvedResultRefs) ResolvedResultRefs { // then a nil list and error is returned instead. func convertToResultRefs(pipelineRunState PipelineRunState, target *ResolvedPipelineTask) (ResolvedResultRefs, string, error) { var resolvedResultRefs ResolvedResultRefs - for _, ref := range v1.PipelineTaskResultRefs(target.PipelineTask) { - resolved, pt, err := resolveResultRef(pipelineRunState, ref) - if err != nil { - return nil, pt, err + for _, resultRef := range v1.PipelineTaskResultRefs(target.PipelineTask) { + referencedPipelineTask := pipelineRunState.ToMap()[resultRef.PipelineTask] + if referencedPipelineTask == nil { + return nil, resultRef.PipelineTask, fmt.Errorf("could not find task %q referenced by result", resultRef.PipelineTask) + } + if !referencedPipelineTask.isSuccessful() && !referencedPipelineTask.isFailure() { + return nil, resultRef.PipelineTask, fmt.Errorf("task %q referenced by result was not finished", referencedPipelineTask.PipelineTask.Name) + } + // Custom Task + switch { + case referencedPipelineTask.IsCustomTask(): + resolved, err := resolveCustomResultRef(referencedPipelineTask.CustomRuns, resultRef) + if err != nil { + return nil, resultRef.PipelineTask, err + } + resolvedResultRefs = append(resolvedResultRefs, resolved) + default: + // Matrixed referenced Pipeline Task + if len(referencedPipelineTask.TaskRuns) > 1 { + arrayValues, err := findResultValuesForMatrix(referencedPipelineTask, resultRef) + if err != nil { + return nil, resultRef.PipelineTask, err + } + for _, taskRun := range referencedPipelineTask.TaskRuns { + resolved := createMatrixedTaskResultForParam(taskRun.Name, arrayValues, resultRef) + resolvedResultRefs = append(resolvedResultRefs, resolved) + } + } else { + // Regular PipelineTask + resolved, err := resolveResultRef(referencedPipelineTask.TaskRuns, resultRef) + if err != nil { + return nil, resultRef.PipelineTask, err + } + resolvedResultRefs = append(resolvedResultRefs, resolved) + } } - resolvedResultRefs = append(resolvedResultRefs, resolved) } return resolvedResultRefs, "", nil } -func resolveResultRef(pipelineState PipelineRunState, resultRef *v1.ResultRef) (*ResolvedResultRef, string, error) { - referencedPipelineTask := pipelineState.ToMap()[resultRef.PipelineTask] - if referencedPipelineTask == nil { - return nil, resultRef.PipelineTask, fmt.Errorf("could not find task %q referenced by result", resultRef.PipelineTask) - } - if !referencedPipelineTask.isSuccessful() && !referencedPipelineTask.isFailure() { - return nil, resultRef.PipelineTask, fmt.Errorf("task %q referenced by result was not finished", referencedPipelineTask.PipelineTask.Name) +func resolveCustomResultRef(customRuns []*v1beta1.CustomRun, resultRef *v1.ResultRef) (*ResolvedResultRef, error) { + customRun := customRuns[0] + runName := customRun.GetObjectMeta().GetName() + runValue, err := findRunResultForParam(customRun, resultRef) + if err != nil { + return nil, err } + return &ResolvedResultRef{ + Value: *v1.NewStructuredValues(runValue), + FromTaskRun: "", + FromRun: runName, + ResultReference: *resultRef, + }, nil +} - var runName, runValue, taskRunName string - var resultValue v1.ResultValue - var err error - if referencedPipelineTask.IsCustomTask() { - if len(referencedPipelineTask.CustomRuns) != 1 { - return nil, resultRef.PipelineTask, fmt.Errorf("referenced tasks can only have length of 1 since a matrixed task does not support producing results, but was length %d", len(referencedPipelineTask.TaskRuns)) - } - customRun := referencedPipelineTask.CustomRuns[0] - runName = customRun.GetObjectMeta().GetName() - runValue, err = findRunResultForParam(customRun, resultRef) - resultValue = *v1.NewStructuredValues(runValue) - if err != nil { - return nil, resultRef.PipelineTask, err - } - } else { - // Check to make sure the referenced task is not a matrix since a matrix does not support producing results - if len(referencedPipelineTask.TaskRuns) != 1 { - return nil, resultRef.PipelineTask, fmt.Errorf("referenced tasks can only have length of 1 since a matrixed task does not support producing results, but was length %d", len(referencedPipelineTask.TaskRuns)) - } - taskRun := referencedPipelineTask.TaskRuns[0] - taskRunName = taskRun.Name - resultValue, err = findTaskResultForParam(taskRun, resultRef) - if err != nil { - return nil, resultRef.PipelineTask, err - } +func resolveResultRef(taskRuns []*v1.TaskRun, resultRef *v1.ResultRef) (*ResolvedResultRef, error) { + taskRun := taskRuns[0] + taskRunName := taskRun.Name + resultValue, err := findTaskResultForParam(taskRun, resultRef) + if err != nil { + return nil, err } - return &ResolvedResultRef{ Value: resultValue, FromTaskRun: taskRunName, - FromRun: runName, + FromRun: "", ResultReference: *resultRef, - }, "", nil + }, nil } func findRunResultForParam(customRun *v1beta1.CustomRun, reference *v1.ResultRef) (string, error) { @@ -178,7 +194,6 @@ func findRunResultForParam(customRun *v1beta1.CustomRun, reference *v1.ResultRef err := fmt.Errorf("%w: Could not find result with name %s for task %s", ErrInvalidTaskResultReference, reference.Result, reference.PipelineTask) return "", err } - func findTaskResultForParam(taskRun *v1.TaskRun, reference *v1.ResultRef) (v1.ResultValue, error) { results := taskRun.Status.TaskRunStatusFields.Results for _, result := range results { @@ -190,6 +205,34 @@ func findTaskResultForParam(taskRun *v1.TaskRun, reference *v1.ResultRef) (v1.Re return v1.ResultValue{}, err } +// findResultValuesForMatrix checks the resultsCache of the referenced Matrixed TaskRun to retrieve the resultValues and aggregate them into +// arrayValues. If the resultCache is empty, it will create the ResultCache so that the results can be accessed in subsequent tasks. +func findResultValuesForMatrix(referencedPipelineTask *ResolvedPipelineTask, resultRef *v1.ResultRef) (v1.ParamValue, error) { + var resultsCache *map[string][]string + if len(referencedPipelineTask.ResultsCache) == 0 { + cache := createResultsCacheMatrixedTaskRuns(referencedPipelineTask) + resultsCache = &cache + referencedPipelineTask.ResultsCache = *resultsCache + } + if arrayValues, ok := referencedPipelineTask.ResultsCache[resultRef.Result]; ok { + return v1.ParamValue{ + Type: v1.ParamTypeArray, + ArrayVal: arrayValues, + }, nil + } + err := fmt.Errorf("%w: Could not find result with name %s for task %s", ErrInvalidTaskResultReference, resultRef.Result, resultRef.PipelineTask) + return v1.ParamValue{}, err +} + +func createMatrixedTaskResultForParam(taskRunName string, paramValue v1.ParamValue, resultRef *v1.ResultRef) *ResolvedResultRef { + return &ResolvedResultRef{ + Value: paramValue, + FromTaskRun: taskRunName, + FromRun: "", + ResultReference: *resultRef, + } +} + func (rs ResolvedResultRefs) getStringReplacements() map[string]string { replacements := map[string]string{} for _, r := range rs { diff --git a/pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go b/pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go index 837d8852db0..9de1d37caf8 100644 --- a/pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go +++ b/pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go @@ -263,6 +263,82 @@ var pipelineRunState = PipelineRunState{{ }}, }, }, +}, { + TaskRunNames: []string{"kTaskRun"}, + TaskRuns: []*v1.TaskRun{{ + ObjectMeta: metav1.ObjectMeta{ + Name: "kTaskRun-0", + }, + Status: v1.TaskRunStatus{ + Status: duckv1.Status{ + Conditions: duckv1.Conditions{successCondition}, + }, + TaskRunStatusFields: v1.TaskRunStatusFields{ + Results: []v1.TaskRunResult{{ + Name: "IMAGE-DIGEST", + Value: *v1.NewStructuredValues("123"), + }}, + }, + }, + }, { + ObjectMeta: metav1.ObjectMeta{ + Name: "kTaskRun-1", + }, + Status: v1.TaskRunStatus{ + Status: duckv1.Status{ + Conditions: duckv1.Conditions{successCondition}, + }, + TaskRunStatusFields: v1.TaskRunStatusFields{ + Results: []v1.TaskRunResult{{ + Name: "IMAGE-DIGEST", + Value: *v1.NewStructuredValues("345"), + }}, + }, + }, + }}, + PipelineTask: &v1.PipelineTask{ + Name: "kTask", + TaskRef: &v1.TaskRef{Name: "kTask"}, + Matrix: &v1.Matrix{ + Include: v1.IncludeParamsList{{ + Name: "build-1", + Params: v1.Params{{ + Name: "NAME", + Value: *v1.NewStructuredValues("image-1"), + }, { + Name: "DOCKERFILE", + Value: *v1.NewStructuredValues("path/to/Dockerfile1"), + }}, + }, { + Name: "build-2", + Params: v1.Params{{ + Name: "NAME", + Value: *v1.NewStructuredValues("image-2"), + }, { + Name: "DOCKERFILE", + Value: *v1.NewStructuredValues("path/to/Dockerfile2"), + }}, + }}, + }, + }, +}, { + PipelineTask: &v1.PipelineTask{ + Name: "hTask", + TaskRef: &v1.TaskRef{Name: "hTask"}, + Params: v1.Params{{ + Name: "image-digest", + Value: *v1.NewStructuredValues("$(tasks.kTask.results.IMAGE-DIGEST)[*]"), + }}, + }, +}, { + PipelineTask: &v1.PipelineTask{ + Name: "iTask", + TaskRef: &v1.TaskRef{Name: "iTask"}, + Params: v1.Params{{ + Name: "image-digest", + Value: *v1.NewStructuredValues("$(tasks.kTask.results.I-DO-NOT-EXIST)[*]"), + }}, + }, }} func TestResolveResultRefs(t *testing.T) { @@ -431,6 +507,28 @@ func TestResolveResultRefs(t *testing.T) { }, FromTaskRun: "eTaskRun", }}, + }, { + name: "Test successful result references matrix emitting results", + pipelineRunState: pipelineRunState, + targets: PipelineRunState{ + pipelineRunState[16], + }, + want: ResolvedResultRefs{{ + Value: *v1.NewStructuredValues("123", "345"), + ResultReference: v1.ResultRef{ + PipelineTask: "kTask", + Result: "IMAGE-DIGEST", + }, + FromTaskRun: "kTaskRun-1", + }}, + }, { + name: "Test unsuccessful result references matrix emitting results", + pipelineRunState: pipelineRunState, + targets: PipelineRunState{ + pipelineRunState[17], + }, + wantPt: "kTask", + wantErr: true, }} { t.Run(tt.name, func(t *testing.T) { got, pt, err := ResolveResultRefs(tt.pipelineRunState, tt.targets) diff --git a/pkg/reconciler/pipelinerun/resources/validate_params.go b/pkg/reconciler/pipelinerun/resources/validate_params.go index d954756f1d0..8cc4ea1be48 100644 --- a/pkg/reconciler/pipelinerun/resources/validate_params.go +++ b/pkg/reconciler/pipelinerun/resources/validate_params.go @@ -105,6 +105,13 @@ func ValidateParameterTypesInMatrix(state PipelineRunState) error { if m.HasParams() { for _, param := range m.Params { if param.Value.Type != v1.ParamTypeArray { + // If it's an array type that contains result references because it's consuming results + // from a Matrixed PipelineTask continue + if ps, ok := param.GetVarSubstitutionExpressions(); ok { + if v1.LooksLikeContainsResultRefs(ps) { + continue + } + } return fmt.Errorf("parameters of type array only are allowed, but param \"%s\" has type \"%s\" in pipelineTask \"%s\"", param.Name, string(param.Value.Type), rpt.PipelineTask.Name) } diff --git a/pkg/reconciler/pipelinerun/resources/validate_params_test.go b/pkg/reconciler/pipelinerun/resources/validate_params_test.go index fefce539c7e..4b6e667d8fc 100644 --- a/pkg/reconciler/pipelinerun/resources/validate_params_test.go +++ b/pkg/reconciler/pipelinerun/resources/validate_params_test.go @@ -410,6 +410,17 @@ func TestValidatePipelineParameterTypes(t *testing.T) { }, }}, wantErrs: "parameters of type string only are allowed, but param \"barfoo\" has type \"object\" in pipelineTask \"task\"", + }, { + desc: "parameters in matrix are result references", + state: resources.PipelineRunState{{ + PipelineTask: &v1.PipelineTask{ + Name: "task", + Matrix: &v1.Matrix{ + Params: v1.Params{{ + Name: "url", Value: v1.ParamValue{Type: v1.ParamTypeString, StringVal: `$(tasks.matrix-emitting-results.results.report-url[*])`}, + }}}, + }, + }}, }} { t.Run(tc.desc, func(t *testing.T) { err := resources.ValidateParameterTypesInMatrix(tc.state)