From d656f4f401d8ca0af4a124b5f362ac2c4fb8fbae 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/variables.md | 3 + .../alpha/pipelinerun-array-results.yaml | 36 ++++++++++ pkg/apis/pipeline/v1beta1/result_types.go | 8 +++ pkg/apis/pipeline/v1beta1/resultref.go | 12 +++- pkg/apis/pipeline/v1beta1/when_types.go | 3 + pkg/apis/pipeline/v1beta1/when_types_test.go | 19 ++++++ pkg/reconciler/pipeline/dag/dag_test.go | 22 +++++-- pkg/reconciler/pipelinerun/resources/apply.go | 7 +- .../pipelinerun/resources/apply_test.go | 65 +++++++++++++++++++ .../resources/resultrefresolution.go | 26 +++++++- pkg/reconciler/taskrun/validate_resources.go | 15 +++++ .../taskrun/validate_resources_test.go | 6 ++ 12 files changed, 210 insertions(+), 12 deletions(-) create mode 100644 examples/v1beta1/pipelineruns/alpha/pipelinerun-array-results.yaml diff --git a/docs/variables.md b/docs/variables.md index ac1d23715e7..6adc5e71a4b 100644 --- a/docs/variables.md +++ b/docs/variables.md @@ -22,6 +22,9 @@ For instructions on using variable substitutions see the relevant section of [th | `tasks..results.` | The value of the `Task's` result. Can alter `Task` execution order within a `Pipeline`.) | | `tasks..results['']` | (see above)) | | `tasks..results[""]` | (see above)) | +| `tasks..results.[*]` | The array value of the `Task's` result. Can alter `Task` execution order within a `Pipeline`.) | +| `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..e72c6d2a239 --- /dev/null +++ b/examples/v1beta1/pipelineruns/alpha/pipelinerun-array-results.yaml @@ -0,0 +1,36 @@ +apiVersion: tekton.dev/v1beta1 +kind: PipelineRun +metadata: + name: pipelinerun-array-results +spec: + pipelineSpec: + tasks: + - name: task1 + taskSpec: + results: + - name: array-results + description: The current date in human readable format + 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[*])" + taskSpec: + params: + - name: foo + type: array + default: + - "defaultparam1" + - "defaultparam2" + steps: + - name: print-param + image: bash:latest + args: [ + "echo", + "$(params.foo)" + ] diff --git a/pkg/apis/pipeline/v1beta1/result_types.go b/pkg/apis/pipeline/v1beta1/result_types.go index a4fd77b26da..fc88b2dd30f 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,9 @@ const ( // AllResultsTypes can be used for ResultsTypes validation. var AllResultsTypes = []ResultsType{ResultsTypeString, ResultsTypeArray, ResultsTypeObject} + +// ResultsArrayReference returns the reference of the result from array parameter reference +// returns results.resultname[*] from $(results.resultname[*]) +func ResultsArrayReference(a string) string { + return strings.TrimSuffix(strings.TrimPrefix(a, "$("), ")") +} diff --git a/pkg/apis/pipeline/v1beta1/resultref.go b/pkg/apis/pipeline/v1beta1/resultref.go index d74e2bcd38f..f39d9c55db5 100644 --- a/pkg/apis/pipeline/v1beta1/resultref.go +++ b/pkg/apis/pipeline/v1beta1/resultref.go @@ -35,13 +35,18 @@ const ( // ResultResultPart Constant used to define the "results" part of a pipeline result reference ResultResultPart = "results" // TODO(#2462) use one regex across all substitutions - variableSubstitutionFormat = `\$\([_a-zA-Z0-9.-]+(\.[_a-zA-Z0-9.-]+)*\)` + // variableSubstitutionFormat matches format like $result.resultname, $result.resultname[int] and $result.resultname[*] + variableSubstitutionFormat = `\$\([_a-zA-Z0-9.-]+(\.[_a-zA-Z0-9.-]+)*(\[([0-9])*\*?\])?\)` + // excludeArrayIndexing will replace all `[int]` and `[*]` for parseExpression to extract result name + excludeArrayIndexing = `\[([0-9])*\*?\]` // ResultNameFormat Constant used to define the the regex Result.Name should follow 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 matching substitutions +var VariableSubstitutionRegex = regexp.MustCompile(variableSubstitutionFormat) var resultNameFormatRegex = regexp.MustCompile(ResultNameFormat) +var excludeArrayIndexingRegex = regexp.MustCompile(excludeArrayIndexing) // NewResultRefs extracts all ResultReferences from a param or a pipeline result. // If the ResultReference can be extracted, they are returned. Expressions which are not @@ -107,7 +112,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 } @@ -127,6 +132,7 @@ func parseExpression(substitutionExpression string) (string, string, error) { if len(subExpressions) != 4 || subExpressions[0] != ResultTaskPart || subExpressions[2] != ResultResultPart { return "", "", fmt.Errorf("Must be of the form %q", resultExpressionFormat) } + subExpressions[3] = excludeArrayIndexingRegex.ReplaceAllString(subExpressions[3], "") return subExpressions[1], subExpressions[3], nil } diff --git a/pkg/apis/pipeline/v1beta1/when_types.go b/pkg/apis/pipeline/v1beta1/when_types.go index 0d6f170d5e6..3fb494ca70b 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[fmt.Sprintf("%s", 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..a1cd100d8df 100644 --- a/pkg/apis/pipeline/v1beta1/when_types_test.go +++ b/pkg/apis/pipeline/v1beta1/when_types_test.go @@ -248,6 +248,25 @@ 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", + "tasks.aTask.results.aResult[*]": "barfoo", + }, + arrayReplacements: map[string][]string{ + "tasks.aTask.results.aResult[*]": {"dev", "stage"}, + }, + expected: &WhenExpression{ + Input: "foobar", + Operator: selection.In, + Values: []string{"dev", "stage"}, + }, }, { 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 a9a7ad7ada3..b084ff8bf7c 100644 --- a/pkg/reconciler/pipeline/dag/dag_test.go +++ b/pkg/reconciler/pipeline/dag/dag_test.go @@ -371,6 +371,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{{ @@ -393,27 +394,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{ @@ -422,15 +434,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 089b5fc7bf8..8b53d82e4a7 100644 --- a/pkg/reconciler/pipelinerun/resources/apply.go +++ b/pkg/reconciler/pipelinerun/resources/apply.go @@ -99,17 +99,18 @@ 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 { // also make substitution for resolved condition checks for _, resolvedConditionCheck := range resolvedPipelineRunTask.ResolvedConditionChecks { pipelineTaskCondition := resolvedConditionCheck.PipelineTaskCondition.DeepCopy() - pipelineTaskCondition.Params = replaceParamValues(pipelineTaskCondition.Params, stringReplacements, nil) + pipelineTaskCondition.Params = replaceParamValues(pipelineTaskCondition.Params, stringReplacements, arrayReplacements) resolvedConditionCheck.PipelineTaskCondition = pipelineTaskCondition } if resolvedPipelineRunTask.PipelineTask != nil { pipelineTask := resolvedPipelineRunTask.PipelineTask.DeepCopy() - pipelineTask.Params = replaceParamValues(pipelineTask.Params, stringReplacements, nil) - pipelineTask.WhenExpressions = pipelineTask.WhenExpressions.ReplaceWhenExpressionsVariables(stringReplacements, nil) + pipelineTask.Params = replaceParamValues(pipelineTask.Params, stringReplacements, arrayReplacements) + 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 495f39b9483..dadac933626 100644 --- a/pkg/reconciler/pipelinerun/resources/apply_test.go +++ b/pkg/reconciler/pipelinerun/resources/apply_test.go @@ -401,6 +401,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 - when expressions", resolvedResultRefs: ResolvedResultRefs{{ @@ -433,6 +465,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"}, + }}, + }, + }}, }} { t.Run(tt.name, func(t *testing.T) { ApplyTaskResults(tt.targets, tt.resolvedResultRefs) diff --git a/pkg/reconciler/pipelinerun/resources/resultrefresolution.go b/pkg/reconciler/pipelinerun/resources/resultrefresolution.go index bd82529e488..489108d92fb 100644 --- a/pkg/reconciler/pipelinerun/resources/resultrefresolution.go +++ b/pkg/reconciler/pipelinerun/resources/resultrefresolution.go @@ -187,8 +187,22 @@ func findTaskResultForParam(taskRun *v1beta1.TaskRun, reference *v1beta1.ResultR func (rs ResolvedResultRefs) getStringReplacements() map[string]string { replacements := map[string]string{} for _, r := range rs { - for _, target := range r.getReplaceTarget() { - replacements[target] = r.Value.StringVal + if r.Value.Type == v1beta1.ParamType(v1beta1.ResultsTypeString) { + for _, target := range r.getReplaceTarget() { + replacements[target] = r.Value.StringVal + } + } + } + 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 @@ -201,3 +215,11 @@ func (r *ResolvedResultRef) getReplaceTarget() []string { fmt.Sprintf("%s.%s.%s['%s']", v1beta1.ResultTaskPart, r.ResultReference.PipelineTask, v1beta1.ResultResultPart, r.ResultReference.Result), } } + +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 96d3a3589bb..6fde16ac8d1 100644 --- a/pkg/reconciler/taskrun/validate_resources.go +++ b/pkg/reconciler/taskrun/validate_resources.go @@ -78,6 +78,11 @@ func validateParams(ctx context.Context, paramSpecs []v1beta1.ParamSpec, params if extraParamsNames := extraParamsNames(ctx, neededParamsNames, providedParamsNames); len(extraParamsNames) != 0 { return fmt.Errorf("didn't need these params but they were provided anyway: %s", extraParamsNames) } + // This is needed to support array replacements in params.Users want to use $(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 correct the param type. + // Please check issue #4879 for more details and examples. + correctAoStype(params, neededParamsTypes) if wrongTypeParamNames := wrongTypeParamsNames(params, matrix, neededParamsTypes); len(wrongTypeParamNames) != 0 { return fmt.Errorf("param types don't match the user-specified type: %s", wrongTypeParamNames) } @@ -88,6 +93,16 @@ func validateParams(ctx context.Context, paramSpecs []v1beta1.ParamSpec, params return nil } +func correctAoStype(params []v1beta1.Param, neededParamsTypes map[string]v1beta1.ParamType) { + for i := 0; i < len(params); i++ { + if params[i].Value.Type == "string" && neededParamsTypes[params[i].Name] == "array" && v1beta1.VariableSubstitutionRegex.MatchString(params[i].Value.StringVal) { + params[i].Value.Type = neededParamsTypes[params[i].Name] + params[i].Value.ArrayVal = []string{params[i].Value.StringVal} + params[i].Value.StringVal = "" + } + } +} + func neededParamsNamesAndTypes(paramSpecs []v1beta1.ParamSpec) ([]string, map[string]v1beta1.ParamType) { var neededParamsNames []string neededParamsTypes := make(map[string]v1beta1.ParamType) diff --git a/pkg/reconciler/taskrun/validate_resources_test.go b/pkg/reconciler/taskrun/validate_resources_test.go index 6feae3f9044..328e80bd3f7 100644 --- a/pkg/reconciler/taskrun/validate_resources_test.go +++ b/pkg/reconciler/taskrun/validate_resources_test.go @@ -141,6 +141,9 @@ func TestValidateResolvedTaskResources_ValidParams(t *testing.T) { { Name: "zoo", Type: v1beta1.ParamTypeString, + }, { + Name: "arrayResultRef", + Type: v1beta1.ParamTypeArray, }, { Name: "myobj", Type: v1beta1.ParamTypeObject, @@ -161,6 +164,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{