From 3b9297049624f0a487578854c6e2eaf4ed2a1f40 Mon Sep 17 00:00:00 2001 From: Yongxuan Zhang Date: Wed, 6 Jul 2022 15:32:45 +0000 Subject: [PATCH] [TEP-0075]Pipeline results support object This is part of work in TEP-0075. Previous to this commit, we have added support for pipeline array results. This commit supports object results, so pipeline can emit object results as whole or emit elements of the object from tasks. Before this commit the pipeline level result only support string and array. --- docs/pipelines.md | 15 +- .../alpha/pipeline-array-results.yaml | 40 ---- .../alpha/pipeline-emitting-results.yaml | 61 ++++++ pkg/apis/pipeline/v1beta1/param_types.go | 7 +- pkg/apis/pipeline/v1beta1/resultref.go | 3 + pkg/apis/pipeline/v1beta1/resultref_test.go | 38 ++++ pkg/reconciler/pipelinerun/pipelinerun.go | 6 +- pkg/reconciler/pipelinerun/resources/apply.go | 66 ++++++- .../pipelinerun/resources/apply_test.go | 187 +++++++++++++++--- pkg/substitution/substitution.go | 5 + pkg/substitution/substitution_test.go | 33 ++++ 11 files changed, 378 insertions(+), 83 deletions(-) delete mode 100644 examples/v1beta1/pipelineruns/alpha/pipeline-array-results.yaml create mode 100644 examples/v1beta1/pipelineruns/alpha/pipeline-emitting-results.yaml diff --git a/docs/pipelines.md b/docs/pipelines.md index b442c83638b..d315d5aff3a 100644 --- a/docs/pipelines.md +++ b/docs/pipelines.md @@ -1001,7 +1001,20 @@ results: For an end-to-end example, see [`Results` in a `PipelineRun`](../examples/v1beta1/pipelineruns/pipelinerun-results.yaml). -Array results is supported as alpha feature, see [`Array Results` in a `PipelineRun`](../examples/v1beta1/pipelineruns/alpha/pipelinerun-array-results.yaml). +Array and object results is supported as alpha feature, see [`Array Results` in a `PipelineRun`](../examples/v1beta1/pipelineruns/alpha/pipeline-emitting-results.yaml). + +```yaml + results: + - name: object-results + type: object + description: whole object + value: $(tasks.task2.results.object-results[*]) + - name: object-element + type: string + description: object element + value: $(tasks.task2.results.object-results.foo) +``` + A `Pipeline Result` is not emitted if any of the following are true: - A `PipelineTask` referenced by the `Pipeline Result` failed. The `PipelineRun` will also diff --git a/examples/v1beta1/pipelineruns/alpha/pipeline-array-results.yaml b/examples/v1beta1/pipelineruns/alpha/pipeline-array-results.yaml deleted file mode 100644 index 618995dbf80..00000000000 --- a/examples/v1beta1/pipelineruns/alpha/pipeline-array-results.yaml +++ /dev/null @@ -1,40 +0,0 @@ -apiVersion: tekton.dev/v1beta1 -kind: PipelineRun -metadata: - name: pipelinerun-array-indexing-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 - 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 "[\"4\",\"5\",\"6\"]" | tee $(results.array-results.path) - results: - - name: echo-indexing-array-results - type: string - description: array element - value: $(tasks.task1.results.array-results[1]) - - name: echo-array-results - type: array - description: whole array - value: $(tasks.task2.results.array-results[*]) diff --git a/examples/v1beta1/pipelineruns/alpha/pipeline-emitting-results.yaml b/examples/v1beta1/pipelineruns/alpha/pipeline-emitting-results.yaml new file mode 100644 index 00000000000..10baa456f38 --- /dev/null +++ b/examples/v1beta1/pipelineruns/alpha/pipeline-emitting-results.yaml @@ -0,0 +1,61 @@ +apiVersion: tekton.dev/v1beta1 +kind: PipelineRun +metadata: + name: pipelinerun-array-indexing-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 + taskSpec: + results: + - name: object-results + type: object + description: The object results + properties: + foo: { + type: string + } + hello: { + type: string + } + steps: + - name: write-array + image: bash:latest + script: | + #!/usr/bin/env bash + echo -n "{\"foo\":\"bar\",\"hello\":\"world\"}" | tee $(results.object-results.path) + results: + - name: array-results + type: array + description: whole array + value: $(tasks.task1.results.array-results[*]) + - name: array-indexing-results + type: string + description: array element + value: $(tasks.task1.results.array-results[1]) + - name: object-results + type: object + description: whole object + value: $(tasks.task2.results.object-results[*]) + - name: object-results-from-array-indexing-and-object-elements + type: object + description: whole object + value: + key1: $(tasks.task1.results.array-results[1]) + key2: $(tasks.task2.results.object-results.hello) + - name: object-element + type: string + description: object element + value: $(tasks.task2.results.object-results.foo) diff --git a/pkg/apis/pipeline/v1beta1/param_types.go b/pkg/apis/pipeline/v1beta1/param_types.go index 76a6d198c65..8489c95b295 100644 --- a/pkg/apis/pipeline/v1beta1/param_types.go +++ b/pkg/apis/pipeline/v1beta1/param_types.go @@ -228,7 +228,7 @@ func (arrayOrString *ArrayOrString) applyOrCorrect(stringReplacements map[string // trim the head "$(" and the tail ")" or "[*])" // i.e. get "params.name" from "$(params.name)" or "$(params.name[*])" - trimedStringVal := StripStarVarSubExpression(stringVal) + trimedStringVal := substitution.StripStarVarSubExpression(stringVal) // if the stringVal is a reference to a string param if _, ok := stringReplacements[trimedStringVal]; ok { @@ -250,11 +250,6 @@ func (arrayOrString *ArrayOrString) applyOrCorrect(stringReplacements map[string } } -// StripStarVarSubExpression strips "$(target[*])"" to get "target" -func StripStarVarSubExpression(s string) string { - return strings.TrimSuffix(strings.TrimSuffix(strings.TrimPrefix(s, "$("), ")"), "[*]") -} - // NewArrayOrString creates an ArrayOrString of type ParamTypeString or ParamTypeArray, based on // how many inputs are given (>1 input will create an array, not string). func NewArrayOrString(value string, values ...string) *ArrayOrString { diff --git a/pkg/apis/pipeline/v1beta1/resultref.go b/pkg/apis/pipeline/v1beta1/resultref.go index 07fe744cdd9..6add8177d71 100644 --- a/pkg/apis/pipeline/v1beta1/resultref.go +++ b/pkg/apis/pipeline/v1beta1/resultref.go @@ -128,6 +128,9 @@ func GetVarSubstitutionExpressionsForParam(param Param) ([]string, bool) { // GetVarSubstitutionExpressionsForPipelineResult extracts all the value between "$(" and ")"" for a pipeline result func GetVarSubstitutionExpressionsForPipelineResult(result PipelineResult) ([]string, bool) { allExpressions := validateString(result.Value.StringVal) + for _, v := range result.Value.ObjectVal { + allExpressions = append(allExpressions, validateString(v)...) + } return allExpressions, len(allExpressions) != 0 } diff --git a/pkg/apis/pipeline/v1beta1/resultref_test.go b/pkg/apis/pipeline/v1beta1/resultref_test.go index bf9a36468ea..10b7cd73aa0 100644 --- a/pkg/apis/pipeline/v1beta1/resultref_test.go +++ b/pkg/apis/pipeline/v1beta1/resultref_test.go @@ -718,3 +718,41 @@ func TestParseResultName(t *testing.T) { }) } } + +func TestGetVarSubstitutionExpressionsForPipelineResult(t *testing.T) { + tests := []struct { + name string + result v1beta1.PipelineResult + want []string + }{{ + name: "get string result expressions", + result: v1beta1.PipelineResult{ + Name: "string result", + Type: v1beta1.ResultsTypeString, + Value: *v1beta1.NewArrayOrString("$(tasks.task1.results.result1) and $(tasks.task2.results.result2)"), + }, + want: []string{"tasks.task1.results.result1", "tasks.task2.results.result2"}, + }, + { + name: "get object result expressions", + result: v1beta1.PipelineResult{ + Name: "object result", + Type: v1beta1.ResultsTypeString, + Value: *v1beta1.NewObject(map[string]string{ + "key1": "$(tasks.task1.results.result1)", + "key2": "$(tasks.task2.results.result2) and another one $(tasks.task3.results.result3)", + "key3": "no ref here", + }), + }, + want: []string{"tasks.task1.results.result1", "tasks.task2.results.result2", "tasks.task3.results.result3"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + get, _ := v1beta1.GetVarSubstitutionExpressionsForPipelineResult(tt.result) + if d := cmp.Diff(tt.want, get); d != "" { + t.Error(diff.PrintWantGot(d)) + } + }) + } +} diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 99f63a9758a..6ee83b10a20 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -612,8 +612,12 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get pr.Status.SkippedTasks = pipelineRunFacts.GetSkippedTasks() if after.Status == corev1.ConditionTrue || after.Status == corev1.ConditionFalse { - pr.Status.PipelineResults = resources.ApplyTaskResultsToPipelineResults(pipelineSpec.Results, + pr.Status.PipelineResults, err = resources.ApplyTaskResultsToPipelineResults(pipelineSpec.Results, pipelineRunFacts.State.GetTaskRunsResults(), pipelineRunFacts.State.GetRunsResults()) + if err != nil { + pr.Status.MarkFailed(ReasonFailedValidation, err.Error()) + return err + } } logger.Infof("PipelineRun %s status is being set to %s", pr.Name, after) diff --git a/pkg/reconciler/pipelinerun/resources/apply.go b/pkg/reconciler/pipelinerun/resources/apply.go index 6f93cf454dd..b04e04f266a 100644 --- a/pkg/reconciler/pipelinerun/resources/apply.go +++ b/pkg/reconciler/pipelinerun/resources/apply.go @@ -30,6 +30,14 @@ import ( "github.com/tektoncd/pipeline/pkg/substitution" ) +const ( + // resultsParseNumber is the value of how many parts we split from result reference. e.g. tasks..results. + resultsParseNumber = 4 + // objectElementResultsParseNumber is the value of how many parts we split from + // object attribute result reference. e.g. tasks..results.. + objectElementResultsParseNumber = 5 +) + // ApplyParameters applies the params from a PipelineRun.Params to a PipelineSpec. func ApplyParameters(ctx context.Context, p *v1beta1.PipelineSpec, pr *v1beta1.PipelineRun) *v1beta1.PipelineSpec { // This assumes that the PipelineRun inputs have been validated against what the Pipeline requests. @@ -251,13 +259,17 @@ func replaceParamValues(params []v1beta1.Param, stringReplacements map[string]st func ApplyTaskResultsToPipelineResults( results []v1beta1.PipelineResult, taskRunResults map[string][]v1beta1.TaskRunResult, - customTaskResults map[string][]v1alpha1.RunResult) []v1beta1.PipelineRunResult { - + customTaskResults map[string][]v1alpha1.RunResult) ([]v1beta1.PipelineRunResult, error) { var runResults []v1beta1.PipelineRunResult + var invalidPipelineResults []string stringReplacements := map[string]string{} arrayReplacements := map[string][]string{} + objectReplacements := map[string]map[string]string{} for _, pipelineResult := range results { variablesInPipelineResult, _ := v1beta1.GetVarSubstitutionExpressionsForPipelineResult(pipelineResult) + if len(variablesInPipelineResult) == 0 { + continue + } validPipelineResult := true for _, variable := range variablesInPipelineResult { if _, isMemoized := stringReplacements[variable]; isMemoized { @@ -266,10 +278,20 @@ func ApplyTaskResultsToPipelineResults( if _, isMemoized := arrayReplacements[variable]; isMemoized { continue } - // TODO(#4723): Need to consider object case. - // e.g.: tasks.taskname.results.resultname.objectkey + if _, isMemoized := objectReplacements[variable]; isMemoized { + continue + } variableParts := strings.Split(variable, ".") - if len(variableParts) == 4 && variableParts[0] == "tasks" && variableParts[2] == "results" { + if variableParts[0] != v1beta1.ResultTaskPart || variableParts[2] != v1beta1.ResultResultPart { + validPipelineResult = false + invalidPipelineResults = append(invalidPipelineResults, pipelineResult.Name) + continue + } + switch len(variableParts) { + // For string result: tasks..results. + // For array result: tasks..results.[*], tasks..results.[i] + // For object result: tasks..results.[*], + case resultsParseNumber: taskName, resultName := variableParts[1], variableParts[3] resultName, stringIdx := v1beta1.ParseResultName(resultName) if resultValue := taskResultValue(taskName, resultName, taskRunResults); resultValue != nil { @@ -279,23 +301,43 @@ func ApplyTaskResultsToPipelineResults( case v1beta1.ParamTypeArray: if stringIdx != "*" { intIdx, _ := strconv.Atoi(stringIdx) - stringReplacements[variable] = resultValue.ArrayVal[intIdx] + if intIdx < len(resultValue.ArrayVal) { + stringReplacements[variable] = resultValue.ArrayVal[intIdx] + } } else { - arrayReplacements[v1beta1.StripStarVarSubExpression(variable)] = resultValue.ArrayVal + arrayReplacements[substitution.StripStarVarSubExpression(variable)] = resultValue.ArrayVal } + case v1beta1.ParamTypeObject: + objectReplacements[substitution.StripStarVarSubExpression(variable)] = resultValue.ObjectVal } } else if resultValue := runResultValue(taskName, resultName, customTaskResults); resultValue != nil { stringReplacements[variable] = *resultValue } else { + // referred array index out of bound + invalidPipelineResults = append(invalidPipelineResults, pipelineResult.Name) validPipelineResult = false } - } else { + // For object type result: tasks..results.. + case objectElementResultsParseNumber: + taskName, resultName, objectKey := variableParts[1], variableParts[3], variableParts[4] + resultName, _ = v1beta1.ParseResultName(resultName) + if resultValue := taskResultValue(taskName, resultName, taskRunResults); resultValue != nil { + if _, ok := resultValue.ObjectVal[objectKey]; ok { + stringReplacements[variable] = resultValue.ObjectVal[objectKey] + } else { + // referred object key is not existent + invalidPipelineResults = append(invalidPipelineResults, pipelineResult.Name) + validPipelineResult = false + } + } + default: + invalidPipelineResults = append(invalidPipelineResults, pipelineResult.Name) validPipelineResult = false } } if validPipelineResult { finalValue := pipelineResult.Value - finalValue.ApplyReplacements(stringReplacements, arrayReplacements, nil) + finalValue.ApplyReplacements(stringReplacements, arrayReplacements, objectReplacements) runResults = append(runResults, v1beta1.PipelineRunResult{ Name: pipelineResult.Name, Value: finalValue, @@ -303,7 +345,11 @@ func ApplyTaskResultsToPipelineResults( } } - return runResults + if len(invalidPipelineResults) > 0 { + return runResults, fmt.Errorf("invalid pipelineresults %v, the referred results don't exist", invalidPipelineResults) + } + + return runResults, nil } // taskResultValue returns the result value for a given pipeline task name and result name in a map of TaskRunResults for diff --git a/pkg/reconciler/pipelinerun/resources/apply_test.go b/pkg/reconciler/pipelinerun/resources/apply_test.go index fb5c95cad5b..8946f3ba993 100644 --- a/pkg/reconciler/pipelinerun/resources/apply_test.go +++ b/pkg/reconciler/pipelinerun/resources/apply_test.go @@ -18,6 +18,7 @@ package resources import ( "context" + "fmt" "testing" "github.com/google/go-cmp/cmp" @@ -2057,12 +2058,47 @@ func TestApplyWorkspaces(t *testing.T) { func TestApplyTaskResultsToPipelineResults(t *testing.T) { for _, tc := range []struct { - description string - results []v1beta1.PipelineResult - taskResults map[string][]v1beta1.TaskRunResult - runResults map[string][]v1alpha1.RunResult - expected []v1beta1.PipelineRunResult + description string + results []v1beta1.PipelineResult + taskResults map[string][]v1beta1.TaskRunResult + runResults map[string][]v1alpha1.RunResult + expectedResults []v1beta1.PipelineRunResult + expectedError error }{{ + description: "non-reference-results", + results: []v1beta1.PipelineResult{{ + Name: "pipeline-result-1", + Value: *v1beta1.NewArrayOrString("resultName"), + }}, + taskResults: map[string][]v1beta1.TaskRunResult{ + "pt1": { + { + Name: "foo", + Value: *v1beta1.NewArrayOrString("do", "rae", "mi"), + }, + }, + }, + expectedResults: nil, + }, { + description: "object-reference-not-exist", + results: []v1beta1.PipelineResult{{ + Name: "pipeline-result-1", + Value: *v1beta1.NewArrayOrString("$(tasks.pt1.results.foo.key3)"), + }}, + taskResults: map[string][]v1beta1.TaskRunResult{ + "pt1": { + { + Name: "foo", + Value: *v1beta1.NewObject(map[string]string{ + "key1": "val1", + "key2": "val2", + }), + }, + }, + }, + expectedResults: nil, + expectedError: fmt.Errorf("invalid pipelineresults [pipeline-result-1], the referred results don't exist"), + }, { description: "apply-array-results", results: []v1beta1.PipelineResult{{ Name: "pipeline-result-1", @@ -2076,7 +2112,7 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { }, }, }, - expected: []v1beta1.PipelineRunResult{{ + expectedResults: []v1beta1.PipelineRunResult{{ Name: "pipeline-result-1", Value: *v1beta1.NewArrayOrString("do", "rae", "mi"), }}, @@ -2094,10 +2130,88 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { }, }, }, - expected: []v1beta1.PipelineRunResult{{ + expectedResults: []v1beta1.PipelineRunResult{{ Name: "pipeline-result-1", Value: *v1beta1.NewArrayOrString("rae"), }}, + }, { + description: "apply-object-results", + results: []v1beta1.PipelineResult{{ + Name: "pipeline-result-1", + Value: *v1beta1.NewArrayOrString("$(tasks.pt1.results.foo[*])"), + }}, + taskResults: map[string][]v1beta1.TaskRunResult{ + "pt1": { + { + Name: "foo", + Value: *v1beta1.NewObject(map[string]string{ + "key1": "val1", + "key2": "val2", + }), + }, + }, + }, + expectedResults: []v1beta1.PipelineRunResult{{ + Name: "pipeline-result-1", + Value: *v1beta1.NewObject(map[string]string{ + "key1": "val1", + "key2": "val2", + }), + }}, + }, { + description: "object-results-from-array-indexing-and-object-element", + results: []v1beta1.PipelineResult{{ + Name: "pipeline-result-1", + Value: *v1beta1.NewObject(map[string]string{ + "pkey1": "$(tasks.pt1.results.foo.key1)", + "pkey2": "$(tasks.pt2.results.bar[1])", + }), + }}, + taskResults: map[string][]v1beta1.TaskRunResult{ + "pt1": { + { + Name: "foo", + Value: *v1beta1.NewObject(map[string]string{ + "key1": "val1", + "key2": "val2", + }), + }, + }, + "pt2": { + { + Name: "bar", + Value: *v1beta1.NewArrayOrString("do", "rae", "mi"), + }, + }, + }, + expectedResults: []v1beta1.PipelineRunResult{{ + Name: "pipeline-result-1", + Value: *v1beta1.NewObject(map[string]string{ + "pkey1": "val1", + "pkey2": "rae", + }), + }}, + }, { + description: "apply-object-element", + results: []v1beta1.PipelineResult{{ + Name: "pipeline-result-1", + Value: *v1beta1.NewArrayOrString("$(tasks.pt1.results.foo.key1)"), + }}, + taskResults: map[string][]v1beta1.TaskRunResult{ + "pt1": { + { + Name: "foo", + Value: *v1beta1.NewObject(map[string]string{ + "key1": "val1", + "key2": "val2", + }), + }, + }, + }, + expectedResults: []v1beta1.PipelineRunResult{{ + Name: "pipeline-result-1", + Value: *v1beta1.NewArrayOrString("val1"), + }}, }, { description: "multiple-array-results-multiple-successful-tasks ", results: []v1beta1.PipelineResult{{ @@ -2121,7 +2235,7 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { }, }, }, - expected: []v1beta1.PipelineRunResult{{ + expectedResults: []v1beta1.PipelineRunResult{{ Name: "pipeline-result-1", Value: *v1beta1.NewArrayOrString("do", "rae"), }, { @@ -2137,7 +2251,7 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { Value: *v1beta1.NewArrayOrString("bar"), }}, }, - expected: nil, + expectedResults: nil, }, { description: "invalid-result-variable-no-returned-result", results: []v1beta1.PipelineResult{{ @@ -2150,7 +2264,8 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { Value: *v1beta1.NewArrayOrString("bar"), }}, }, - expected: nil, + expectedResults: nil, + expectedError: fmt.Errorf("invalid pipelineresults [foo], the referred results don't exist"), }, { description: "no-taskrun-results-no-returned-results", results: []v1beta1.PipelineResult{{ @@ -2160,7 +2275,8 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { taskResults: map[string][]v1beta1.TaskRunResult{ "pt1": {}, }, - expected: nil, + expectedResults: nil, + expectedError: fmt.Errorf("invalid pipelineresults [foo], the referred results don't exist"), }, { description: "invalid-taskrun-name-no-returned-result", results: []v1beta1.PipelineResult{{ @@ -2173,7 +2289,7 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { Value: *v1beta1.NewArrayOrString("bar"), }}, }, - expected: nil, + expectedResults: nil, }, { description: "invalid-result-name-no-returned-result", results: []v1beta1.PipelineResult{{ @@ -2186,15 +2302,16 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { Value: *v1beta1.NewArrayOrString("bar"), }}, }, - expected: nil, + expectedResults: nil, + expectedError: fmt.Errorf("invalid pipelineresults [foo], the referred results don't exist"), }, { description: "unsuccessful-taskrun-no-returned-result", results: []v1beta1.PipelineResult{{ Name: "foo", Value: *v1beta1.NewArrayOrString("$(tasks.pt1.results.foo)"), }}, - taskResults: map[string][]v1beta1.TaskRunResult{}, - expected: nil, + taskResults: map[string][]v1beta1.TaskRunResult{}, + expectedResults: nil, }, { description: "mixed-success-tasks-some-returned-results", results: []v1beta1.PipelineResult{{ @@ -2210,10 +2327,11 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { Value: *v1beta1.NewArrayOrString("rae"), }}, }, - expected: []v1beta1.PipelineRunResult{{ + expectedResults: []v1beta1.PipelineRunResult{{ Name: "bar", Value: *v1beta1.NewArrayOrString("rae"), }}, + expectedError: fmt.Errorf("invalid pipelineresults [foo], the referred results don't exist"), }, { description: "multiple-results-multiple-successful-tasks ", results: []v1beta1.PipelineResult{{ @@ -2238,7 +2356,7 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { Value: *v1beta1.NewArrayOrString("rae"), }}, }, - expected: []v1beta1.PipelineRunResult{{ + expectedResults: []v1beta1.PipelineRunResult{{ Name: "pipeline-result-1", Value: *v1beta1.NewArrayOrString("do"), }, { @@ -2251,8 +2369,8 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { Name: "foo", Value: *v1beta1.NewArrayOrString("$(tasks.customtask.results.foo)"), }}, - runResults: map[string][]v1alpha1.RunResult{}, - expected: nil, + runResults: map[string][]v1alpha1.RunResult{}, + expectedResults: nil, }, { description: "wrong-customtask-name-no-returned-result", results: []v1beta1.PipelineResult{{ @@ -2265,7 +2383,8 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { Value: "bar", }}, }, - expected: nil, + expectedResults: nil, + expectedError: fmt.Errorf("invalid pipelineresults [foo], the referred results don't exist"), }, { description: "right-customtask-name-wrong-result-name-no-returned-result", results: []v1beta1.PipelineResult{{ @@ -2278,7 +2397,8 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { Value: "bar", }}, }, - expected: nil, + expectedResults: nil, + expectedError: fmt.Errorf("invalid pipelineresults [foo], the referred results don't exist"), }, { description: "unsuccessful-run-no-returned-result", results: []v1beta1.PipelineResult{{ @@ -2288,7 +2408,19 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { runResults: map[string][]v1alpha1.RunResult{ "customtask": {}, }, - expected: nil, + expectedResults: nil, + expectedError: fmt.Errorf("invalid pipelineresults [foo], the referred results don't exist"), + }, { + description: "wrong-result-reference-expression", + results: []v1beta1.PipelineResult{{ + Name: "foo", + Value: *v1beta1.NewArrayOrString("$(tasks.task.results.foo.foo.foo)"), + }}, + runResults: map[string][]v1alpha1.RunResult{ + "customtask": {}, + }, + expectedResults: nil, + expectedError: fmt.Errorf("invalid pipelineresults [foo], the referred results don't exist"), }, { description: "multiple-results-custom-and-normal-tasks", results: []v1beta1.PipelineResult{{ @@ -2315,7 +2447,7 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { Value: *v1beta1.NewArrayOrString("rae"), }}, }, - expected: []v1beta1.PipelineRunResult{{ + expectedResults: []v1beta1.PipelineRunResult{{ Name: "pipeline-result-1", Value: *v1beta1.NewArrayOrString("do"), }, { @@ -2324,8 +2456,13 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { }}, }} { t.Run(tc.description, func(t *testing.T) { - received := ApplyTaskResultsToPipelineResults(tc.results, tc.taskResults, tc.runResults) - if d := cmp.Diff(tc.expected, received); d != "" { + received, err := ApplyTaskResultsToPipelineResults(tc.results, tc.taskResults, tc.runResults) + if tc.expectedError != nil { + if d := cmp.Diff(tc.expectedError.Error(), err.Error()); d != "" { + t.Errorf("ApplyTaskResultsToPipelineResults() errors diff %s", diff.PrintWantGot(d)) + } + } + if d := cmp.Diff(tc.expectedResults, received); d != "" { t.Errorf(diff.PrintWantGot(d)) } }) diff --git a/pkg/substitution/substitution.go b/pkg/substitution/substitution.go index 93cac332bc9..f98d03569b9 100644 --- a/pkg/substitution/substitution.go +++ b/pkg/substitution/substitution.go @@ -361,3 +361,8 @@ func ExtractIndexString(s string) string { func ExtractIndex(s string) (int, error) { return strconv.Atoi(strings.TrimSuffix(strings.TrimPrefix(s, "["), "]")) } + +// StripStarVarSubExpression strips "$(target[*])"" to get "target" +func StripStarVarSubExpression(s string) string { + return strings.TrimSuffix(strings.TrimSuffix(strings.TrimPrefix(s, "$("), ")"), "[*]") +} diff --git a/pkg/substitution/substitution_test.go b/pkg/substitution/substitution_test.go index ea8d1b9ae29..986f6b9ce4c 100644 --- a/pkg/substitution/substitution_test.go +++ b/pkg/substitution/substitution_test.go @@ -573,3 +573,36 @@ func TestTrimSquareBrackets(t *testing.T) { }) } } + +func TestStripStarVarSubExpression(t *testing.T) { + tests := []struct { + name string + input string + want string + }{{ + name: "normal string", + input: "hello world", + want: "hello world", + }, { + name: "result reference", + input: "$(tasks.task.results.result)", + want: "tasks.task.results.result", + }, { + name: "result star reference", + input: "$(tasks.task.results.result[*])", + want: "tasks.task.results.result", + }, { + name: "result index reference", + input: "$(tasks.task.results.result[1])", + want: "tasks.task.results.result[1]", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := substitution.StripStarVarSubExpression(tt.input) + if d := cmp.Diff(tt.want, got); d != "" { + t.Error(diff.PrintWantGot(d)) + } + }) + } +}