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)) + } + }) + } +}