From 4cf2cc9917f42b8a47fa059dcb8b52530e29dde9 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 | 7 +- docs/variables.md | 3 + ...ipelinerun-array-results-substitution.yaml | 56 ++++++++++++++++ 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 | 12 ++++ pkg/reconciler/taskrun/validate_resources.go | 9 +++ .../taskrun/validate_resources_test.go | 6 ++ 13 files changed, 229 insertions(+), 10 deletions(-) create mode 100644 examples/v1beta1/pipelineruns/alpha/pipelinerun-array-results-substitution.yaml diff --git a/docs/pipelines.md b/docs/pipelines.md index 85506803fb9..80df548c3c5 100644 --- a/docs/pipelines.md +++ b/docs/pipelines.md @@ -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`, 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-substitution.yaml b/examples/v1beta1/pipelineruns/alpha/pipelinerun-array-results-substitution.yaml new file mode 100644 index 00000000000..18e2f8f8615 --- /dev/null +++ b/examples/v1beta1/pipelineruns/alpha/pipelinerun-array-results-substitution.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/result_types.go b/pkg/apis/pipeline/v1beta1/result_types.go index 7ff3c20816a..cbdc5404c3e 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 @@ -64,3 +66,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 9808881c67c..07fe744cdd9 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) @@ -131,7 +132,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 63b7a597f7b..7532a2f1ba3 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 e0766564bb7..6771d936f66 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..69014de59ce 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.getReplaceTarget() { + 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), diff --git a/pkg/reconciler/taskrun/validate_resources.go b/pkg/reconciler/taskrun/validate_resources.go index bdd9a92ab6c..a25c2517940 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 dc34f1f5515..947fbe8be4c 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{