From 0bedc86eca97972760e662beba807fdfcd5ad793 Mon Sep 17 00:00:00 2001 From: Yongxuan Zhang Date: Wed, 25 May 2022 18:20:03 +0000 Subject: [PATCH] [TEP-0076]Support Array Results substitution This is part of work in TEP-0076. This commit provides the support to apply array results replacements. Previous this commit we support emitting array results so users can write array results to task level, but we cannot pass array results from tasks within one pipeline. This commit adds the support for this. --- docs/pipelines.md | 51 ++++++++------- docs/variables.md | 3 + .../alpha/pipelinerun-array-results.yaml | 56 ++++++++++++++++ pkg/apis/pipeline/v1beta1/param_types.go | 1 - pkg/apis/pipeline/v1beta1/result_types.go | 7 ++ pkg/apis/pipeline/v1beta1/resultref.go | 5 +- pkg/apis/pipeline/v1beta1/when_types.go | 3 + pkg/apis/pipeline/v1beta1/when_types_test.go | 37 +++++++++++ pkg/reconciler/pipeline/dag/dag_test.go | 22 +++++-- pkg/reconciler/pipelinerun/resources/apply.go | 7 +- .../pipelinerun/resources/apply_test.go | 65 +++++++++++++++++++ .../resources/resultrefresolution.go | 19 ++++++ pkg/reconciler/taskrun/validate_resources.go | 9 +++ .../taskrun/validate_resources_test.go | 6 ++ 14 files changed, 258 insertions(+), 33 deletions(-) create mode 100644 examples/v1beta1/pipelineruns/alpha/pipelinerun-array-results.yaml diff --git a/docs/pipelines.md b/docs/pipelines.md index f75ca3ba75b..be57c8e84da 100644 --- a/docs/pipelines.md +++ b/docs/pipelines.md @@ -95,7 +95,7 @@ A `Pipeline` definition supports the following fields: a `Task` requires. - [`from`](#using-the-from-field) - Indicates the data for a [`PipelineResource`](resources.md) originates from the output of a previous `Task`. - - [`runAfter`](#using-the-runafter-field) - Indicates that a `Task` should execute after one or more other + - [`runAfter`](#using-the-runafter-field) - Indicates that a `Task` should execute after one or more other `Tasks` without output linking. - [`retries`](#using-the-retries-field) - Specifies the number of times to retry the execution of a `Task` after a failure. Does not apply to execution cancellations. @@ -109,7 +109,7 @@ A `Pipeline` definition supports the following fields: - [`results`](#emitting-results-from-a-pipeline) - Specifies the location to which the `Pipeline` emits its execution results. - [`description`](#adding-a-description) - Holds an informative description of the `Pipeline` object. - - [`finally`](#adding-finally-to-the-pipeline) - Specifies one or more `Tasks` to be executed in parallel after + - [`finally`](#adding-finally-to-the-pipeline) - Specifies one or more `Tasks` to be executed in parallel after all other tasks have completed. - [`name`](#adding-finally-to-the-pipeline) - the name of this `Task` within the context of this `Pipeline`. - [`taskRef`](#adding-finally-to-the-pipeline) - a reference to a `Task` definition. @@ -174,7 +174,7 @@ spec: workspace: pipeline-ws1 ``` -For simplicity you can also map the name of the `Workspace` in `PipelineTask` to match with +For simplicity you can also map the name of the `Workspace` in `PipelineTask` to match with the `Workspace` from the `Pipeline`. For example: @@ -191,12 +191,12 @@ spec: taskRef: name: gen-code # gen-code expects a Workspace named "source" workspaces: - - name: source # <- mapping workspace name + - name: source # <- mapping workspace name - name: commit taskRef: name: commit # commit expects a Workspace named "source" workspaces: - - name: source # <- mapping workspace name + - name: source # <- mapping workspace name runAfter: - gen-code ``` @@ -402,7 +402,7 @@ spec: `"true"` in the `feature-flags` configmap, see [`install.md`](./install.md#customizing-the-pipelines-controller-behavior)** You may also specify your `Task` reference using a `Tekton Bundle`. A `Tekton Bundle` is an OCI artifact that -contains Tekton resources like `Tasks` which can be referenced within a `taskRef`. +contains Tekton resources like `Tasks` which can be referenced within a `taskRef`. There is currently a hard limit of 20 objects in a bundle. @@ -628,7 +628,7 @@ To guard a `Task` and its dependent Tasks: ##### Cascade `when` expressions to the specific dependent `Tasks` -Pick and choose which specific dependent `Tasks` to guard as well, and cascade the `when` expressions to those `Tasks`. +Pick and choose which specific dependent `Tasks` to guard as well, and cascade the `when` expressions to those `Tasks`. Taking the use case below, a user who wants to guard `manual-approval` and its dependent `Tasks`: @@ -689,12 +689,12 @@ tasks: value: $(tasks.manual-approval.results.approver) taskRef: name: slack-msg -``` +``` ##### Compose using Pipelines in Pipelines -Compose a set of `Tasks` as a unit of execution using `Pipelines` in `Pipelines`, which allows for guarding a `Task` and -its dependent `Tasks` (as a sub-`Pipeline`) using `when` expressions. +Compose a set of `Tasks` as a unit of execution using `Pipelines` in `Pipelines`, which allows for guarding a `Task` and +its dependent `Tasks` (as a sub-`Pipeline`) using `when` expressions. **Note:** `Pipelines` in `Pipelines` is an [experimental feature](https://github.com/tektoncd/experimental/tree/main/pipelines-in-pipelines) @@ -742,7 +742,7 @@ tasks: value: $(tasks.manual-approval.results.approver) taskRef: name: slack-msg - + --- ## main pipeline tasks: @@ -765,12 +765,12 @@ tasks: When `when` expressions evaluate to `False`, the `Task` will be skipped and: - The ordering-dependent `Tasks` will be executed -- The resource-dependent `Tasks` (and their dependencies) will be skipped because of missing `Results` from the skipped - parent `Task`. When we add support for [default `Results`](https://github.com/tektoncd/community/pull/240), then the - resource-dependent `Tasks` may be executed if the default `Results` from the skipped parent `Task` are specified. In +- The resource-dependent `Tasks` (and their dependencies) will be skipped because of missing `Results` from the skipped + parent `Task`. When we add support for [default `Results`](https://github.com/tektoncd/community/pull/240), then the + resource-dependent `Tasks` may be executed if the default `Results` from the skipped parent `Task` are specified. In addition, if a resource-dependent `Task` needs a file from a guarded parent `Task` in a shared `Workspace`, make sure - to handle the execution of the child `Task` in case the expected file is missing from the `Workspace` because the - guarded parent `Task` is skipped. + to handle the execution of the child `Task` in case the expected file is missing from the `Workspace` because the + guarded parent `Task` is skipped. On the other hand, the rest of the `Pipeline` will continue executing. @@ -823,12 +823,12 @@ tasks: name: slack-msg ``` -If `manual-approval` is skipped, execution of its dependent `Tasks` (`slack-msg`, `build-image` and `deploy-image`) +If `manual-approval` is skipped, execution of its dependent `Tasks` (`slack-msg`, `build-image` and `deploy-image`) would be unblocked regardless: - `build-image` and `deploy-image` should be executed successfully - `slack-msg` will be skipped because it is missing the `approver` `Result` from `manual-approval` - dependents of `slack-msg` would have been skipped too if it had any of them - - if `manual-approval` specifies a default `approver` `Result`, such as "None", then `slack-msg` would be executed + - if `manual-approval` specifies a default `approver` `Result`, such as "None", then `slack-msg` would be executed ([supporting default `Results` is in progress](https://github.com/tektoncd/community/pull/240)) ### Configuring the failure timeout @@ -908,7 +908,10 @@ Tasks can emit [`Results`](tasks.md#emitting-results) when they execute. A Pipel Sharing `Results` between `Tasks` in a `Pipeline` happens via [variable substitution](variables.md#variables-available-in-a-pipeline) - one `Task` emits a `Result` and another receives it as a `Parameter` with a variable such as -`$(tasks..results.)`. +`$(tasks..results.)`. Array `Results` is supported as alpha feature and +can be referer as `$(tasks..results.[*])`. + +**Note:** Array `Result` cannot be used in `script`. When one `Task` receives the `Results` of another, there is a dependency created between those two `Tasks`. In order for the receiving `Task` to get data from another `Task's` `Result`, @@ -923,6 +926,8 @@ before this one. params: - name: foo value: "$(tasks.checkout-source.results.commit)" + - name: array-params + value: "$(tasks.checkout-source.results.array-results[*])" ``` **Note:** If `checkout-source` exits successfully without initializing `commit` `Result`, @@ -944,7 +949,7 @@ when: For an end-to-end example, see [`Task` `Results` in a `PipelineRun`](../examples/v1beta1/pipelineruns/task_results_example.yaml). -Note that `when` expressions are whitespace-sensitive. In particular, when producing results intended for inputs to `when` +Note that `when` expressions are whitespace-sensitive. In particular, when producing results intended for inputs to `when` expressions that may include newlines at their close (e.g. `cat`, `jq`), you may wish to truncate them. ```yaml @@ -1009,9 +1014,9 @@ without getting stuck in an infinite loop. This is done using: - _resource dependencies_: - [`from`](#using-the-from-field) clauses on the [`PipelineResources`](resources.md) used by each `Task` - - [`results`](#emitting-results-from-a-pipeline) of one `Task` being passed into `params` or `when` expressions of + - [`results`](#emitting-results-from-a-pipeline) of one `Task` being passed into `params` or `when` expressions of another - + - _ordering dependencies_: - [`runAfter`](#using-the-runafter-field) clauses on the corresponding `Tasks` @@ -1197,7 +1202,7 @@ spec: value: "someURL" matrix: - name: slack-channel - value: + value: - "foo" - "bar" ``` diff --git a/docs/variables.md b/docs/variables.md index 7cfd9929e39..3f99ca7e389 100644 --- a/docs/variables.md +++ b/docs/variables.md @@ -25,6 +25,9 @@ For instructions on using variable substitutions see the relevant section of [th | `tasks..results.[i]` | The ith value of the `Task's` array result. Can alter `Task` execution order within a `Pipeline`.) | | `tasks..results[''][i]` | (see above)) | | `tasks..results[""][i]` | (see above)) | +| `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[''][*]` | (see above)) | +| `tasks..results[""][*]` | (see above)) | | `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/v1beta1/pipelineruns/alpha/pipelinerun-array-results.yaml b/examples/v1beta1/pipelineruns/alpha/pipelinerun-array-results.yaml new file mode 100644 index 00000000000..18e2f8f8615 --- /dev/null +++ b/examples/v1beta1/pipelineruns/alpha/pipelinerun-array-results.yaml @@ -0,0 +1,56 @@ +apiVersion: tekton.dev/v1beta1 +kind: PipelineRun +metadata: + name: pipelinerun-array-results +spec: + pipelineSpec: + tasks: + - name: task1 + taskSpec: + results: + - name: array-results + type: array + description: The array results + steps: + - name: write-array + image: bash:latest + script: | + #!/usr/bin/env bash + echo -n "[\"1\",\"2\",\"3\"]" | tee $(results.array-results.path) + - name: task2 + params: + - name: foo + value: "$(tasks.task1.results.array-results[*])" + - name: bar + value: "$(tasks.task1.results.array-results[2])" + taskSpec: + params: + - name: foo + type: array + default: + - "defaultparam1" + - "defaultparam2" + - name: bar + type: string + default: "defaultparam1" + steps: + - name: print-foo + image: bash:latest + args: [ + "echo", + "$(params.foo[*])" + ] + - name: print-bar + image: ubuntu + script: | + #!/bin/bash + VALUE=$(params.bar) + EXPECTED=3 + diff=$(diff <(printf "%s\n" "${VALUE[@]}") <(printf "%s\n" "${EXPECTED[@]}")) + if [[ -z "$diff" ]]; then + echo "Get expected: ${VALUE}" + exit 0 + else + echo "Want: ${EXPECTED} Got: ${VALUE}" + exit 1 + fi diff --git a/pkg/apis/pipeline/v1beta1/param_types.go b/pkg/apis/pipeline/v1beta1/param_types.go index 4da80ca5e07..f7df5f539d1 100644 --- a/pkg/apis/pipeline/v1beta1/param_types.go +++ b/pkg/apis/pipeline/v1beta1/param_types.go @@ -234,7 +234,6 @@ func (arrayOrString *ArrayOrString) applyOrCorrect(stringReplacements map[string if _, ok := stringReplacements[trimedStringVal]; ok { arrayOrString.StringVal = substitution.ApplyReplacements(arrayOrString.StringVal, stringReplacements) } - // if the stringVal is a reference to an array param, we need to change the type other than apply replacement if _, ok := arrayReplacements[trimedStringVal]; ok { arrayOrString.StringVal = "" diff --git a/pkg/apis/pipeline/v1beta1/result_types.go b/pkg/apis/pipeline/v1beta1/result_types.go index 21a4e1ae166..2130613be4e 100644 --- a/pkg/apis/pipeline/v1beta1/result_types.go +++ b/pkg/apis/pipeline/v1beta1/result_types.go @@ -13,6 +13,8 @@ limitations under the License. package v1beta1 +import "strings" + // TaskResult used to describe the results of a task type TaskResult struct { // Name the given name @@ -60,3 +62,8 @@ const ( // AllResultsTypes can be used for ResultsTypes validation. var AllResultsTypes = []ResultsType{ResultsTypeString, ResultsTypeArray, ResultsTypeObject} + +// ResultsArrayReference returns the reference of the result. e.g. results.resultname from $(results.resultname[*]) +func ResultsArrayReference(a string) string { + return strings.TrimSuffix(strings.TrimSuffix(strings.TrimPrefix(a, "$("), ")"), "[*]") +} diff --git a/pkg/apis/pipeline/v1beta1/resultref.go b/pkg/apis/pipeline/v1beta1/resultref.go index 34997128838..8bd42653156 100644 --- a/pkg/apis/pipeline/v1beta1/resultref.go +++ b/pkg/apis/pipeline/v1beta1/resultref.go @@ -53,7 +53,8 @@ const ( ResultNameFormat = `^([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$` ) -var variableSubstitutionRegex = regexp.MustCompile(variableSubstitutionFormat) +// VariableSubstitutionRegex is a regex to find all result matching substitutions +var VariableSubstitutionRegex = regexp.MustCompile(variableSubstitutionFormat) var exactVariableSubstitutionRegex = regexp.MustCompile(exactVariableSubstitutionFormat) var resultNameFormatRegex = regexp.MustCompile(ResultNameFormat) var arrayIndexingRegex = regexp.MustCompile(arrayIndexing) @@ -129,7 +130,7 @@ func GetVarSubstitutionExpressionsForPipelineResult(result PipelineResult) ([]st } func validateString(value string) []string { - expressions := variableSubstitutionRegex.FindAllString(value, -1) + expressions := VariableSubstitutionRegex.FindAllString(value, -1) if expressions == nil { return nil } diff --git a/pkg/apis/pipeline/v1beta1/when_types.go b/pkg/apis/pipeline/v1beta1/when_types.go index 0d6f170d5e6..ac53da805d4 100644 --- a/pkg/apis/pipeline/v1beta1/when_types.go +++ b/pkg/apis/pipeline/v1beta1/when_types.go @@ -62,9 +62,12 @@ func (we *WhenExpression) applyReplacements(replacements map[string]string, arra for _, val := range we.Values { // arrayReplacements holds a list of array parameters with a pattern - params.arrayParam1 // array params are referenced using $(params.arrayParam1[*]) + // array results are referenced using $(results.resultname[*]) // check if the param exist in the arrayReplacements to replace it with a list of values if _, ok := arrayReplacements[fmt.Sprintf("%s.%s", ParamsPrefix, ArrayReference(val))]; ok { replacedValues = append(replacedValues, substitution.ApplyArrayReplacements(val, replacements, arrayReplacements)...) + } else if _, ok := arrayReplacements[ResultsArrayReference(val)]; ok { + replacedValues = append(replacedValues, substitution.ApplyArrayReplacements(val, replacements, arrayReplacements)...) } else { replacedValues = append(replacedValues, substitution.ApplyReplacements(val, replacements)) } diff --git a/pkg/apis/pipeline/v1beta1/when_types_test.go b/pkg/apis/pipeline/v1beta1/when_types_test.go index 5f9dbbe195e..408e81a289b 100644 --- a/pkg/apis/pipeline/v1beta1/when_types_test.go +++ b/pkg/apis/pipeline/v1beta1/when_types_test.go @@ -248,6 +248,43 @@ func TestApplyReplacements(t *testing.T) { Operator: selection.In, Values: []string{"barfoo"}, }, + }, { + name: "replace array results variables", + original: &WhenExpression{ + Input: "$(tasks.foo.results.bar)", + Operator: selection.In, + Values: []string{"$(tasks.aTask.results.aResult[*])"}, + }, + replacements: map[string]string{ + "tasks.foo.results.bar": "foobar", + }, + arrayReplacements: map[string][]string{ + "tasks.aTask.results.aResult": {"dev", "stage"}, + }, + expected: &WhenExpression{ + Input: "foobar", + Operator: selection.In, + Values: []string{"dev", "stage"}, + }, + }, { + name: "invaliad array results replacements", + original: &WhenExpression{ + Input: "$(tasks.foo.results.bar)", + Operator: selection.In, + Values: []string{"$(tasks.aTask.results.aResult[invalid])"}, + }, + replacements: map[string]string{ + "tasks.foo.results.bar": "foobar", + "tasks.aTask.results.aResult[*]": "barfoo", + }, + arrayReplacements: map[string][]string{ + "tasks.aTask.results.aResult[*]": {"dev", "stage"}, + }, + expected: &WhenExpression{ + Input: "foobar", + Operator: selection.In, + Values: []string{"$(tasks.aTask.results.aResult[invalid])"}, + }, }, { name: "replace array params", original: &WhenExpression{ diff --git a/pkg/reconciler/pipeline/dag/dag_test.go b/pkg/reconciler/pipeline/dag/dag_test.go index abd0e3401b6..4c1ba1893fa 100644 --- a/pkg/reconciler/pipeline/dag/dag_test.go +++ b/pkg/reconciler/pipeline/dag/dag_test.go @@ -292,6 +292,7 @@ func TestBuild_TaskParamsFromTaskResults(t *testing.T) { c := v1beta1.PipelineTask{Name: "c"} d := v1beta1.PipelineTask{Name: "d"} e := v1beta1.PipelineTask{Name: "e"} + f := v1beta1.PipelineTask{Name: "f"} xDependsOnA := v1beta1.PipelineTask{ Name: "x", Params: []v1beta1.Param{{ @@ -314,27 +315,38 @@ func TestBuild_TaskParamsFromTaskResults(t *testing.T) { Value: *v1beta1.NewArrayOrString("$(tasks.d.results.resultD) $(tasks.e.results.resultE)"), }}, } + wDependsOnF := v1beta1.PipelineTask{ + Name: "w", + Params: []v1beta1.Param{{ + Name: "paramw", + Value: *v1beta1.NewArrayOrString("$(tasks.f.results.resultF[*])"), + }}, + } - // a b c d e - // | \ / \ / - // x y z + // a b c d e f + // | \ / \ / | + // x y z w nodeA := &dag.Node{Task: a} nodeB := &dag.Node{Task: b} nodeC := &dag.Node{Task: c} nodeD := &dag.Node{Task: d} nodeE := &dag.Node{Task: e} + nodeF := &dag.Node{Task: f} nodeX := &dag.Node{Task: xDependsOnA} nodeY := &dag.Node{Task: yDependsOnBRunsAfterC} nodeZ := &dag.Node{Task: zDependsOnDAndE} + nodeW := &dag.Node{Task: wDependsOnF} nodeA.Next = []*dag.Node{nodeX} nodeB.Next = []*dag.Node{nodeY} nodeC.Next = []*dag.Node{nodeY} nodeD.Next = []*dag.Node{nodeZ} nodeE.Next = []*dag.Node{nodeZ} + nodeF.Next = []*dag.Node{nodeW} nodeX.Prev = []*dag.Node{nodeA} nodeY.Prev = []*dag.Node{nodeB, nodeC} nodeZ.Prev = []*dag.Node{nodeD, nodeE} + nodeW.Prev = []*dag.Node{nodeF} expectedDAG := &dag.Graph{ Nodes: map[string]*dag.Node{ @@ -343,15 +355,17 @@ func TestBuild_TaskParamsFromTaskResults(t *testing.T) { "c": nodeC, "d": nodeD, "e": nodeE, + "f": nodeF, "x": nodeX, "y": nodeY, "z": nodeZ, + "w": nodeW, }, } p := &v1beta1.Pipeline{ ObjectMeta: metav1.ObjectMeta{Name: "pipeline"}, Spec: v1beta1.PipelineSpec{ - Tasks: []v1beta1.PipelineTask{a, b, c, d, e, xDependsOnA, yDependsOnBRunsAfterC, zDependsOnDAndE}, + Tasks: []v1beta1.PipelineTask{a, b, c, d, e, f, xDependsOnA, yDependsOnBRunsAfterC, zDependsOnDAndE, wDependsOnF}, }, } tasks := v1beta1.PipelineTaskList(p.Spec.Tasks) diff --git a/pkg/reconciler/pipelinerun/resources/apply.go b/pkg/reconciler/pipelinerun/resources/apply.go index 3e192369b7f..7a853e583e2 100644 --- a/pkg/reconciler/pipelinerun/resources/apply.go +++ b/pkg/reconciler/pipelinerun/resources/apply.go @@ -122,12 +122,13 @@ func ApplyPipelineTaskContexts(pt *v1beta1.PipelineTask) *v1beta1.PipelineTask { // ApplyTaskResults applies the ResolvedResultRef to each PipelineTask.Params and Pipeline.WhenExpressions in targets func ApplyTaskResults(targets PipelineRunState, resolvedResultRefs ResolvedResultRefs) { stringReplacements := resolvedResultRefs.getStringReplacements() + arrayReplacements := resolvedResultRefs.getArrayReplacements() for _, resolvedPipelineRunTask := range targets { if resolvedPipelineRunTask.PipelineTask != nil { pipelineTask := resolvedPipelineRunTask.PipelineTask.DeepCopy() - pipelineTask.Params = replaceParamValues(pipelineTask.Params, stringReplacements, nil, nil) - pipelineTask.Matrix = replaceParamValues(pipelineTask.Matrix, stringReplacements, nil, nil) - pipelineTask.WhenExpressions = pipelineTask.WhenExpressions.ReplaceWhenExpressionsVariables(stringReplacements, nil) + pipelineTask.Params = replaceParamValues(pipelineTask.Params, stringReplacements, arrayReplacements, nil) + pipelineTask.Matrix = replaceParamValues(pipelineTask.Matrix, stringReplacements, arrayReplacements, nil) + pipelineTask.WhenExpressions = pipelineTask.WhenExpressions.ReplaceWhenExpressionsVariables(stringReplacements, arrayReplacements) resolvedPipelineRunTask.PipelineTask = pipelineTask } } diff --git a/pkg/reconciler/pipelinerun/resources/apply_test.go b/pkg/reconciler/pipelinerun/resources/apply_test.go index b8df4bd6f2b..dc7d7e0df3e 100644 --- a/pkg/reconciler/pipelinerun/resources/apply_test.go +++ b/pkg/reconciler/pipelinerun/resources/apply_test.go @@ -1051,6 +1051,38 @@ func TestApplyTaskResults_MinimalExpression(t *testing.T) { }}, }, }}, + }, { + name: "Test array result substitution on minimal variable substitution expression - params", + resolvedResultRefs: ResolvedResultRefs{{ + Value: *v1beta1.NewArrayOrString("arrayResultValueOne", "arrayResultValueTwo"), + ResultReference: v1beta1.ResultRef{ + PipelineTask: "aTask", + Result: "a.Result", + }, + FromTaskRun: "aTaskRun", + }}, + targets: PipelineRunState{{ + PipelineTask: &v1beta1.PipelineTask{ + Name: "bTask", + TaskRef: &v1beta1.TaskRef{Name: "bTask"}, + Params: []v1beta1.Param{{ + Name: "bParam", + Value: v1beta1.ArrayOrString{Type: v1beta1.ParamTypeArray, + ArrayVal: []string{`$(tasks.aTask.results["a.Result"][*])`}, + }, + }}, + }, + }}, + want: PipelineRunState{{ + PipelineTask: &v1beta1.PipelineTask{ + Name: "bTask", + TaskRef: &v1beta1.TaskRef{Name: "bTask"}, + Params: []v1beta1.Param{{ + Name: "bParam", + Value: *v1beta1.NewArrayOrString("arrayResultValueOne", "arrayResultValueTwo"), + }}, + }, + }}, }, { name: "Test result substitution on minimal variable substitution expression - matrix", resolvedResultRefs: ResolvedResultRefs{{ @@ -1141,6 +1173,39 @@ func TestApplyTaskResults_MinimalExpression(t *testing.T) { }}, }, }}, + }, { + name: "Test array result substitution on minimal variable substitution expression - when expressions", + resolvedResultRefs: ResolvedResultRefs{{ + Value: *v1beta1.NewArrayOrString("arrayResultValueOne", "arrayResultValueTwo"), + ResultReference: v1beta1.ResultRef{ + PipelineTask: "aTask", + Result: "aResult", + }, + FromTaskRun: "aTaskRun", + }}, + targets: PipelineRunState{{ + PipelineTask: &v1beta1.PipelineTask{ + Name: "bTask", + TaskRef: &v1beta1.TaskRef{Name: "bTask"}, + WhenExpressions: []v1beta1.WhenExpression{{ + // Note that Input doesn't support array replacement. + Input: "anInput", + Operator: selection.In, + Values: []string{"$(tasks.aTask.results.aResult[*])"}, + }}, + }, + }}, + want: PipelineRunState{{ + PipelineTask: &v1beta1.PipelineTask{ + Name: "bTask", + TaskRef: &v1beta1.TaskRef{Name: "bTask"}, + WhenExpressions: []v1beta1.WhenExpression{{ + Input: "anInput", + Operator: selection.In, + Values: []string{"arrayResultValueOne", "arrayResultValueTwo"}, + }}, + }, + }}, }, { name: "Test result substitution on minimal variable substitution expression - when expressions", resolvedResultRefs: ResolvedResultRefs{{ diff --git a/pkg/reconciler/pipelinerun/resources/resultrefresolution.go b/pkg/reconciler/pipelinerun/resources/resultrefresolution.go index 7349f46ff37..11c54e681bb 100644 --- a/pkg/reconciler/pipelinerun/resources/resultrefresolution.go +++ b/pkg/reconciler/pipelinerun/resources/resultrefresolution.go @@ -215,6 +215,18 @@ func (rs ResolvedResultRefs) getStringReplacements() map[string]string { return replacements } +func (rs ResolvedResultRefs) getArrayReplacements() map[string][]string { + replacements := map[string][]string{} + for _, r := range rs { + if r.Value.Type == v1beta1.ParamType(v1beta1.ResultsTypeArray) { + for _, target := range r.getArrayReplaceTarget() { + replacements[target] = r.Value.ArrayVal + } + } + } + return replacements +} + func (r *ResolvedResultRef) getReplaceTarget() []string { return []string{ fmt.Sprintf("%s.%s.%s.%s", v1beta1.ResultTaskPart, r.ResultReference.PipelineTask, v1beta1.ResultResultPart, r.ResultReference.Result), @@ -230,3 +242,10 @@ func (r *ResolvedResultRef) getReplaceTargetfromArrayIndex(idx int) []string { fmt.Sprintf("%s.%s.%s['%s'][%d]", v1beta1.ResultTaskPart, r.ResultReference.PipelineTask, v1beta1.ResultResultPart, r.ResultReference.Result, idx), } } +func (r *ResolvedResultRef) getArrayReplaceTarget() []string { + return []string{ + fmt.Sprintf("%s.%s.%s.%s", v1beta1.ResultTaskPart, r.ResultReference.PipelineTask, v1beta1.ResultResultPart, r.ResultReference.Result), + fmt.Sprintf("%s.%s.%s[%q]", v1beta1.ResultTaskPart, r.ResultReference.PipelineTask, v1beta1.ResultResultPart, r.ResultReference.Result), + fmt.Sprintf("%s.%s.%s['%s']", v1beta1.ResultTaskPart, r.ResultReference.PipelineTask, v1beta1.ResultResultPart, r.ResultReference.Result), + } +} diff --git a/pkg/reconciler/taskrun/validate_resources.go b/pkg/reconciler/taskrun/validate_resources.go index b8e00143cba..170a5c7d86e 100644 --- a/pkg/reconciler/taskrun/validate_resources.go +++ b/pkg/reconciler/taskrun/validate_resources.go @@ -133,6 +133,8 @@ func extraParamsNames(ctx context.Context, neededParams []string, providedParams } func wrongTypeParamsNames(params []v1beta1.Param, matrix []v1beta1.Param, neededParamsTypes map[string]v1beta1.ParamType) []string { + //TODO(#4723): validate that $(task.taskname.result.resultname) is invalid for array and object type. + // It should be used to refer string and need to add [*] to refer to array or object. var wrongTypeParamNames []string for _, param := range params { if _, ok := neededParamsTypes[param.Name]; !ok { @@ -140,6 +142,13 @@ func wrongTypeParamsNames(params []v1beta1.Param, matrix []v1beta1.Param, needed // passed to the task that aren't being used. continue } + // This is needed to support array replacements in params. Users want to use $(tasks.taskName.results.resultname[*]) + // to pass array result to array param, yet in yaml format this will be + // unmarshalled to string for ArrayOrString. So we need to check and skip this validation. + // Please refer issue #4879 for more details and examples. + if param.Value.Type == v1beta1.ParamTypeString && (neededParamsTypes[param.Name] == v1beta1.ParamTypeArray || (neededParamsTypes[param.Name] == v1beta1.ParamTypeObject)) && v1beta1.VariableSubstitutionRegex.MatchString(param.Value.StringVal) { + continue + } if param.Value.Type != neededParamsTypes[param.Name] { wrongTypeParamNames = append(wrongTypeParamNames, param.Name) } diff --git a/pkg/reconciler/taskrun/validate_resources_test.go b/pkg/reconciler/taskrun/validate_resources_test.go index 1d64b8e5cab..fa2db3367e7 100644 --- a/pkg/reconciler/taskrun/validate_resources_test.go +++ b/pkg/reconciler/taskrun/validate_resources_test.go @@ -140,6 +140,9 @@ func TestValidateResolvedTaskResources_ValidParams(t *testing.T) { { Name: "zoo", Type: v1beta1.ParamTypeString, + }, { + Name: "arrayResultRef", + Type: v1beta1.ParamTypeArray, }, { Name: "myobj", Type: v1beta1.ParamTypeObject, @@ -160,6 +163,9 @@ func TestValidateResolvedTaskResources_ValidParams(t *testing.T) { }, { Name: "bar", Value: *v1beta1.NewArrayOrString("somethinggood"), + }, { + Name: "arrayResultRef", + Value: *v1beta1.NewArrayOrString("$(results.resultname[*])"), }, { Name: "myobj", Value: *v1beta1.NewObject(map[string]string{