From 0d1d87a0a98840f78acd0e47e78ee9360019b2f7 Mon Sep 17 00:00:00 2001 From: Yongxuan Zhang Date: Fri, 10 Jun 2022 15:09:43 +0000 Subject: [PATCH] [TEP-0076]Pipeline results support array This is part of work in TEP-0076. This commit update the PipelineResult and PipelineRunResult to uspport array results, meanwhile support the whole array and array indexing reference from PipelineResult to TaskResult. Before this commit the pipeline level result only support string. --- docs/pipelines.md | 46 ++++---- .../alpha/pipeline-array-results.yaml | 40 +++++++ .../pipeline/v1beta1/openapi_generated.go | 21 +++- pkg/apis/pipeline/v1beta1/param_types.go | 7 +- pkg/apis/pipeline/v1beta1/pipeline_types.go | 7 +- .../v1beta1/pipeline_validation_test.go | 12 +- .../pipeline/v1beta1/pipelinerun_types.go | 2 +- pkg/apis/pipeline/v1beta1/resultref.go | 24 +++- pkg/apis/pipeline/v1beta1/resultref_test.go | 26 +++++ pkg/apis/pipeline/v1beta1/swagger.json | 12 +- .../pipeline/v1beta1/zz_generated.deepcopy.go | 10 +- pkg/reconciler/pipelinerun/resources/apply.go | 30 +++-- .../pipelinerun/resources/apply_test.go | 108 ++++++++++++++---- .../resources/validate_dependencies_test.go | 8 +- test/custom_task_test.go | 4 +- 15 files changed, 273 insertions(+), 84 deletions(-) create mode 100644 examples/v1beta1/pipelineruns/alpha/pipeline-array-results.yaml diff --git a/docs/pipelines.md b/docs/pipelines.md index f75ca3ba75b..85506803fb9 100644 --- a/docs/pipelines.md +++ b/docs/pipelines.md @@ -95,7 +95,7 @@ A `Pipeline` definition supports the following fields: a `Task` requires. - [`from`](#using-the-from-field) - Indicates the data for a [`PipelineResource`](resources.md) originates from the output of a previous `Task`. - - [`runAfter`](#using-the-runafter-field) - Indicates that a `Task` should execute after one or more other + - [`runAfter`](#using-the-runafter-field) - Indicates that a `Task` should execute after one or more other `Tasks` without output linking. - [`retries`](#using-the-retries-field) - Specifies the number of times to retry the execution of a `Task` after a failure. Does not apply to execution cancellations. @@ -109,7 +109,7 @@ A `Pipeline` definition supports the following fields: - [`results`](#emitting-results-from-a-pipeline) - Specifies the location to which the `Pipeline` emits its execution results. - [`description`](#adding-a-description) - Holds an informative description of the `Pipeline` object. - - [`finally`](#adding-finally-to-the-pipeline) - Specifies one or more `Tasks` to be executed in parallel after + - [`finally`](#adding-finally-to-the-pipeline) - Specifies one or more `Tasks` to be executed in parallel after all other tasks have completed. - [`name`](#adding-finally-to-the-pipeline) - the name of this `Task` within the context of this `Pipeline`. - [`taskRef`](#adding-finally-to-the-pipeline) - a reference to a `Task` definition. @@ -174,7 +174,7 @@ spec: workspace: pipeline-ws1 ``` -For simplicity you can also map the name of the `Workspace` in `PipelineTask` to match with +For simplicity you can also map the name of the `Workspace` in `PipelineTask` to match with the `Workspace` from the `Pipeline`. For example: @@ -191,12 +191,12 @@ spec: taskRef: name: gen-code # gen-code expects a Workspace named "source" workspaces: - - name: source # <- mapping workspace name + - name: source # <- mapping workspace name - name: commit taskRef: name: commit # commit expects a Workspace named "source" workspaces: - - name: source # <- mapping workspace name + - name: source # <- mapping workspace name runAfter: - gen-code ``` @@ -402,7 +402,7 @@ spec: `"true"` in the `feature-flags` configmap, see [`install.md`](./install.md#customizing-the-pipelines-controller-behavior)** You may also specify your `Task` reference using a `Tekton Bundle`. A `Tekton Bundle` is an OCI artifact that -contains Tekton resources like `Tasks` which can be referenced within a `taskRef`. +contains Tekton resources like `Tasks` which can be referenced within a `taskRef`. There is currently a hard limit of 20 objects in a bundle. @@ -628,7 +628,7 @@ To guard a `Task` and its dependent Tasks: ##### Cascade `when` expressions to the specific dependent `Tasks` -Pick and choose which specific dependent `Tasks` to guard as well, and cascade the `when` expressions to those `Tasks`. +Pick and choose which specific dependent `Tasks` to guard as well, and cascade the `when` expressions to those `Tasks`. Taking the use case below, a user who wants to guard `manual-approval` and its dependent `Tasks`: @@ -689,12 +689,12 @@ tasks: value: $(tasks.manual-approval.results.approver) taskRef: name: slack-msg -``` +``` ##### Compose using Pipelines in Pipelines -Compose a set of `Tasks` as a unit of execution using `Pipelines` in `Pipelines`, which allows for guarding a `Task` and -its dependent `Tasks` (as a sub-`Pipeline`) using `when` expressions. +Compose a set of `Tasks` as a unit of execution using `Pipelines` in `Pipelines`, which allows for guarding a `Task` and +its dependent `Tasks` (as a sub-`Pipeline`) using `when` expressions. **Note:** `Pipelines` in `Pipelines` is an [experimental feature](https://github.com/tektoncd/experimental/tree/main/pipelines-in-pipelines) @@ -742,7 +742,7 @@ tasks: value: $(tasks.manual-approval.results.approver) taskRef: name: slack-msg - + --- ## main pipeline tasks: @@ -765,12 +765,12 @@ tasks: When `when` expressions evaluate to `False`, the `Task` will be skipped and: - The ordering-dependent `Tasks` will be executed -- The resource-dependent `Tasks` (and their dependencies) will be skipped because of missing `Results` from the skipped - parent `Task`. When we add support for [default `Results`](https://github.com/tektoncd/community/pull/240), then the - resource-dependent `Tasks` may be executed if the default `Results` from the skipped parent `Task` are specified. In +- The resource-dependent `Tasks` (and their dependencies) will be skipped because of missing `Results` from the skipped + parent `Task`. When we add support for [default `Results`](https://github.com/tektoncd/community/pull/240), then the + resource-dependent `Tasks` may be executed if the default `Results` from the skipped parent `Task` are specified. In addition, if a resource-dependent `Task` needs a file from a guarded parent `Task` in a shared `Workspace`, make sure - to handle the execution of the child `Task` in case the expected file is missing from the `Workspace` because the - guarded parent `Task` is skipped. + to handle the execution of the child `Task` in case the expected file is missing from the `Workspace` because the + guarded parent `Task` is skipped. On the other hand, the rest of the `Pipeline` will continue executing. @@ -823,12 +823,12 @@ tasks: name: slack-msg ``` -If `manual-approval` is skipped, execution of its dependent `Tasks` (`slack-msg`, `build-image` and `deploy-image`) +If `manual-approval` is skipped, execution of its dependent `Tasks` (`slack-msg`, `build-image` and `deploy-image`) would be unblocked regardless: - `build-image` and `deploy-image` should be executed successfully - `slack-msg` will be skipped because it is missing the `approver` `Result` from `manual-approval` - dependents of `slack-msg` would have been skipped too if it had any of them - - if `manual-approval` specifies a default `approver` `Result`, such as "None", then `slack-msg` would be executed + - if `manual-approval` specifies a default `approver` `Result`, such as "None", then `slack-msg` would be executed ([supporting default `Results` is in progress](https://github.com/tektoncd/community/pull/240)) ### Configuring the failure timeout @@ -944,7 +944,7 @@ when: For an end-to-end example, see [`Task` `Results` in a `PipelineRun`](../examples/v1beta1/pipelineruns/task_results_example.yaml). -Note that `when` expressions are whitespace-sensitive. In particular, when producing results intended for inputs to `when` +Note that `when` expressions are whitespace-sensitive. In particular, when producing results intended for inputs to `when` expressions that may include newlines at their close (e.g. `cat`, `jq`), you may wish to truncate them. ```yaml @@ -985,6 +985,8 @@ 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). + 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 have failed. @@ -1009,9 +1011,9 @@ without getting stuck in an infinite loop. This is done using: - _resource dependencies_: - [`from`](#using-the-from-field) clauses on the [`PipelineResources`](resources.md) used by each `Task` - - [`results`](#emitting-results-from-a-pipeline) of one `Task` being passed into `params` or `when` expressions of + - [`results`](#emitting-results-from-a-pipeline) of one `Task` being passed into `params` or `when` expressions of another - + - _ordering dependencies_: - [`runAfter`](#using-the-runafter-field) clauses on the corresponding `Tasks` @@ -1197,7 +1199,7 @@ spec: value: "someURL" matrix: - name: slack-channel - value: + value: - "foo" - "bar" ``` diff --git a/examples/v1beta1/pipelineruns/alpha/pipeline-array-results.yaml b/examples/v1beta1/pipelineruns/alpha/pipeline-array-results.yaml new file mode 100644 index 00000000000..618995dbf80 --- /dev/null +++ b/examples/v1beta1/pipelineruns/alpha/pipeline-array-results.yaml @@ -0,0 +1,40 @@ +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/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index df3f70db8ae..72182db1afb 100644 --- a/pkg/apis/pipeline/v1beta1/openapi_generated.go +++ b/pkg/apis/pipeline/v1beta1/openapi_generated.go @@ -1255,6 +1255,13 @@ func schema_pkg_apis_pipeline_v1beta1_PipelineResult(ref common.ReferenceCallbac Format: "", }, }, + "type": { + SchemaProps: spec.SchemaProps{ + Description: "Type is the user-specified type of the result. The possible types are 'string', 'array', and 'object', with 'string' as the default. 'array' and 'object' types are alpha features.", + Type: []string{"string"}, + Format: "", + }, + }, "description": { SchemaProps: spec.SchemaProps{ Description: "Description is a human-readable description of the result", @@ -1266,15 +1273,16 @@ func schema_pkg_apis_pipeline_v1beta1_PipelineResult(ref common.ReferenceCallbac "value": { SchemaProps: spec.SchemaProps{ Description: "Value the expression used to retrieve the value", - Default: "", - Type: []string{"string"}, - Format: "", + Default: map[string]interface{}{}, + Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.ArrayOrString"), }, }, }, Required: []string{"name", "value"}, }, }, + Dependencies: []string{ + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.ArrayOrString"}, } } @@ -1391,15 +1399,16 @@ func schema_pkg_apis_pipeline_v1beta1_PipelineRunResult(ref common.ReferenceCall "value": { SchemaProps: spec.SchemaProps{ Description: "Value is the result returned from the execution of this PipelineRun", - Default: "", - Type: []string{"string"}, - Format: "", + Default: map[string]interface{}{}, + Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.ArrayOrString"), }, }, }, Required: []string{"name", "value"}, }, }, + Dependencies: []string{ + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.ArrayOrString"}, } } diff --git a/pkg/apis/pipeline/v1beta1/param_types.go b/pkg/apis/pipeline/v1beta1/param_types.go index 4da80ca5e07..b58768b19ae 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 := strings.TrimSuffix(strings.TrimSuffix(strings.TrimPrefix(stringVal, "$("), ")"), "[*]") + trimedStringVal := StripStarVarSubExpression(stringVal) // if the stringVal is a reference to a string param if _, ok := stringReplacements[trimedStringVal]; ok { @@ -250,6 +250,11 @@ 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/pipeline_types.go b/pkg/apis/pipeline/v1beta1/pipeline_types.go index 2fc5348bcfd..ecacac87aea 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_types.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_types.go @@ -123,12 +123,17 @@ type PipelineResult struct { // Name the given name Name string `json:"name"` + // Type is the user-specified type of the result. + // The possible types are 'string', 'array', and 'object', with 'string' as the default. + // 'array' and 'object' types are alpha features. + Type ResultsType `json:"type,omitempty"` + // Description is a human-readable description of the result // +optional Description string `json:"description"` // Value the expression used to retrieve the value - Value string `json:"value"` + Value ArrayOrString `json:"value"` } // PipelineTaskMetadata contains the labels or annotations for an EmbeddedTask diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go index 18c023427c3..248e7b16630 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go @@ -129,7 +129,7 @@ func TestPipeline_Validate_Success(t *testing.T) { Results: []PipelineResult{{ Name: "pipeline-result", Description: "this is my pipeline result", - Value: "$(tasks.my-task.results.my-result)", + Value: *NewArrayOrString("$(tasks.my-task.results.my-result)"), }}, }, }, @@ -1143,11 +1143,11 @@ func TestValidatePipelineResults_Success(t *testing.T) { results := []PipelineResult{{ Name: "my-pipeline-result", Description: "this is my pipeline result", - Value: "$(tasks.a-task.results.output)", + Value: *NewArrayOrString("$(tasks.a-task.results.output)"), }, { Name: "my-pipeline-object-result", Description: "this is my pipeline result", - Value: "$(tasks.a-task.results.gitrepo.commit)", + Value: *NewArrayOrString("$(tasks.a-task.results.gitrepo.commit)"), }} if err := validatePipelineResults(results); err != nil { t.Errorf("Pipeline.validatePipelineResults() returned error for valid pipeline: %s: %v", desc, err) @@ -1164,7 +1164,7 @@ func TestValidatePipelineResults_Failure(t *testing.T) { results: []PipelineResult{{ Name: "my-pipeline-result", Description: "this is my pipeline result", - Value: "$(tasks.a-task.results.output.key1.extra)", + Value: *NewArrayOrString("$(tasks.a-task.results.output.key1.extra)"), }}, expectedError: apis.FieldError{ Message: `invalid value: expected all of the expressions [tasks.a-task.results.output.key1.extra] to be result expressions but only [] were`, @@ -1175,7 +1175,7 @@ func TestValidatePipelineResults_Failure(t *testing.T) { results: []PipelineResult{{ Name: "my-pipeline-result", Description: "this is my pipeline result", - Value: "foo.bar", + Value: *NewArrayOrString("foo.bar"), }}, expectedError: apis.FieldError{ Message: `invalid value: expected pipeline results to be task result expressions but no expressions were found`, @@ -1186,7 +1186,7 @@ func TestValidatePipelineResults_Failure(t *testing.T) { results: []PipelineResult{{ Name: "my-pipeline-result", Description: "this is my pipeline result", - Value: "$(foo.bar)", + Value: *NewArrayOrString("$(foo.bar)"), }}, expectedError: apis.FieldError{ Message: `invalid value: expected pipeline results to be task result expressions but an invalid expressions was found`, diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_types.go b/pkg/apis/pipeline/v1beta1/pipelinerun_types.go index bebb7a6eb78..cea20ecf608 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_types.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_types.go @@ -478,7 +478,7 @@ type PipelineRunResult struct { Name string `json:"name"` // Value is the result returned from the execution of this PipelineRun - Value string `json:"value"` + Value ArrayOrString `json:"value"` } // PipelineRunTaskRunStatus contains the name of the PipelineTask for this TaskRun and the TaskRun's Status diff --git a/pkg/apis/pipeline/v1beta1/resultref.go b/pkg/apis/pipeline/v1beta1/resultref.go index 34997128838..9808881c67c 100644 --- a/pkg/apis/pipeline/v1beta1/resultref.go +++ b/pkg/apis/pipeline/v1beta1/resultref.go @@ -56,6 +56,8 @@ const ( var variableSubstitutionRegex = regexp.MustCompile(variableSubstitutionFormat) var exactVariableSubstitutionRegex = regexp.MustCompile(exactVariableSubstitutionFormat) var resultNameFormatRegex = regexp.MustCompile(ResultNameFormat) + +// arrayIndexingRegex is used to match `[int]` and `[*]` var arrayIndexingRegex = regexp.MustCompile(arrayIndexing) // NewResultRefs extracts all ResultReferences from a param or a pipeline result. @@ -124,7 +126,7 @@ 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) + allExpressions := validateString(result.Value.StringVal) return allExpressions, len(allExpressions) != 0 } @@ -164,13 +166,12 @@ func parseExpression(substitutionExpression string) (string, string, int, string // For string result: tasks..results. // For array result: tasks..results.[index] if len(subExpressions) == 4 && subExpressions[0] == ResultTaskPart && subExpressions[2] == ResultResultPart { - stringIdx := strings.TrimSuffix(strings.TrimPrefix(arrayIndexingRegex.FindString(subExpressions[3]), "["), "]") - subExpressions[3] = arrayIndexingRegex.ReplaceAllString(subExpressions[3], "") + resultName, stringIdx := ParseResultName(subExpressions[3]) if stringIdx != "" { intIdx, _ := strconv.Atoi(stringIdx) - return subExpressions[1], subExpressions[3], intIdx, "", nil + return subExpressions[1], resultName, intIdx, "", nil } - return subExpressions[1], subExpressions[3], 0, "", nil + return subExpressions[1], resultName, 0, "", nil } // For object type result: tasks..results.. @@ -181,6 +182,19 @@ func parseExpression(substitutionExpression string) (string, string, int, string return "", "", 0, "", fmt.Errorf("Must be one of the form 1). %q; 2). %q", resultExpressionFormat, objectResultExpressionFormat) } +// ParseResultName parse the input string to extract resultName and result index. +// Array indexing: +// Input: anArrayResult[1] +// Output: anArrayResult, "1" +// Array star reference: +// Input: anArrayResult[*] +// Output: anArrayResult, "*" +func ParseResultName(resultName string) (string, string) { + stringIdx := strings.TrimSuffix(strings.TrimPrefix(arrayIndexingRegex.FindString(resultName), "["), "]") + resultName = arrayIndexingRegex.ReplaceAllString(resultName, "") + return resultName, stringIdx +} + // PipelineTaskResultRefs walks all the places a result reference can be used // in a PipelineTask and returns a list of any references that are found. func PipelineTaskResultRefs(pt *PipelineTask) []*ResultRef { diff --git a/pkg/apis/pipeline/v1beta1/resultref_test.go b/pkg/apis/pipeline/v1beta1/resultref_test.go index 1f4c8caa115..bf9a36468ea 100644 --- a/pkg/apis/pipeline/v1beta1/resultref_test.go +++ b/pkg/apis/pipeline/v1beta1/resultref_test.go @@ -692,3 +692,29 @@ func TestPipelineTaskResultRefs(t *testing.T) { func lessResultRef(i, j *v1beta1.ResultRef) bool { return i.PipelineTask+i.Result < j.PipelineTask+i.Result } + +func TestParseResultName(t *testing.T) { + tests := []struct { + name string + input string + want []string + }{{ + name: "array indexing", + input: "anArrayResult[1]", + want: []string{"anArrayResult", "1"}, + }, + { + name: "array star reference", + input: "anArrayResult[*]", + want: []string{"anArrayResult", "*"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + resultName, idx := v1beta1.ParseResultName(tt.input) + if d := cmp.Diff(tt.want, []string{resultName, idx}); d != "" { + t.Error(diff.PrintWantGot(d)) + } + }) + } +} diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json index 632390a3499..eac3823c29d 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -799,10 +799,14 @@ "type": "string", "default": "" }, + "type": { + "description": "Type is the user-specified type of the result. The possible types are 'string', 'array', and 'object', with 'string' as the default. 'array' and 'object' types are alpha features.", + "type": "string" + }, "value": { "description": "Value the expression used to retrieve the value", - "type": "string", - "default": "" + "default": {}, + "$ref": "#/definitions/v1beta1.ArrayOrString" } } }, @@ -872,8 +876,8 @@ }, "value": { "description": "Value is the result returned from the execution of this PipelineRun", - "type": "string", - "default": "" + "default": {}, + "$ref": "#/definitions/v1beta1.ArrayOrString" } } }, diff --git a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go index 3873c9cee31..cd3408a2095 100644 --- a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go @@ -435,6 +435,7 @@ func (in *PipelineResourceResult) DeepCopy() *PipelineResourceResult { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PipelineResult) DeepCopyInto(out *PipelineResult) { *out = *in + in.Value.DeepCopyInto(&out.Value) return } @@ -512,6 +513,7 @@ func (in *PipelineRunList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PipelineRunResult) DeepCopyInto(out *PipelineRunResult) { *out = *in + in.Value.DeepCopyInto(&out.Value) return } @@ -684,7 +686,9 @@ func (in *PipelineRunStatusFields) DeepCopyInto(out *PipelineRunStatusFields) { if in.PipelineResults != nil { in, out := &in.PipelineResults, &out.PipelineResults *out = make([]PipelineRunResult, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } if in.PipelineSpec != nil { in, out := &in.PipelineSpec, &out.PipelineSpec @@ -776,7 +780,9 @@ func (in *PipelineSpec) DeepCopyInto(out *PipelineSpec) { if in.Results != nil { in, out := &in.Results, &out.Results *out = make([]PipelineResult, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } if in.Finally != nil { in, out := &in.Finally, &out.Finally diff --git a/pkg/reconciler/pipelinerun/resources/apply.go b/pkg/reconciler/pipelinerun/resources/apply.go index 3e192369b7f..63b7a597f7b 100644 --- a/pkg/reconciler/pipelinerun/resources/apply.go +++ b/pkg/reconciler/pipelinerun/resources/apply.go @@ -234,14 +234,13 @@ func replaceParamValues(params []v1beta1.Param, stringReplacements map[string]st // and omitted from the returned slice. A nil slice is returned if no results are passed in or all // results are invalid. func ApplyTaskResultsToPipelineResults( - // TODO(#4723): Change PipelineResult Value to support array, so we can update - // PipelineResults, right now we only update the StringVal from TaskResults to PipelineResults results []v1beta1.PipelineResult, taskRunResults map[string][]v1beta1.TaskRunResult, customTaskResults map[string][]v1alpha1.RunResult) []v1beta1.PipelineRunResult { var runResults []v1beta1.PipelineRunResult stringReplacements := map[string]string{} + arrayReplacements := map[string][]string{} for _, pipelineResult := range results { variablesInPipelineResult, _ := v1beta1.GetVarSubstitutionExpressionsForPipelineResult(pipelineResult) validPipelineResult := true @@ -249,11 +248,27 @@ func ApplyTaskResultsToPipelineResults( if _, isMemoized := stringReplacements[variable]; isMemoized { continue } + if _, isMemoized := arrayReplacements[variable]; isMemoized { + continue + } + // TODO(#4723): Need to consider object case. + // e.g.: tasks.taskname.results.resultname.objectkey variableParts := strings.Split(variable, ".") if len(variableParts) == 4 && variableParts[0] == "tasks" && variableParts[2] == "results" { taskName, resultName := variableParts[1], variableParts[3] + resultName, stringIdx := v1beta1.ParseResultName(resultName) if resultValue := taskResultValue(taskName, resultName, taskRunResults); resultValue != nil { - stringReplacements[variable] = *resultValue + switch resultValue.Type { + case v1beta1.ParamTypeString: + stringReplacements[variable] = resultValue.StringVal + case v1beta1.ParamTypeArray: + if stringIdx != "*" { + intIdx, _ := strconv.Atoi(stringIdx) + stringReplacements[variable] = resultValue.ArrayVal[intIdx] + } else { + arrayReplacements[v1beta1.StripStarVarSubExpression(variable)] = resultValue.ArrayVal + } + } } else if resultValue := runResultValue(taskName, resultName, customTaskResults); resultValue != nil { stringReplacements[variable] = *resultValue } else { @@ -265,10 +280,7 @@ func ApplyTaskResultsToPipelineResults( } if validPipelineResult { finalValue := pipelineResult.Value - for variable, value := range stringReplacements { - v := fmt.Sprintf("$(%s)", variable) - finalValue = strings.ReplaceAll(finalValue, v, value) - } + finalValue.ApplyReplacements(stringReplacements, arrayReplacements, nil) runResults = append(runResults, v1beta1.PipelineRunResult{ Name: pipelineResult.Name, Value: finalValue, @@ -282,10 +294,10 @@ func ApplyTaskResultsToPipelineResults( // taskResultValue returns the result value for a given pipeline task name and result name in a map of TaskRunResults for // pipeline task names. It returns nil if either the pipeline task name isn't present in the map, or if there is no // result with the result name in the pipeline task name's slice of results. -func taskResultValue(taskName string, resultName string, taskResults map[string][]v1beta1.TaskRunResult) *string { +func taskResultValue(taskName string, resultName string, taskResults map[string][]v1beta1.TaskRunResult) *v1beta1.ArrayOrString { for _, trResult := range taskResults[taskName] { if trResult.Name == resultName { - return &trResult.Value.StringVal + return &trResult.Value } } return nil diff --git a/pkg/reconciler/pipelinerun/resources/apply_test.go b/pkg/reconciler/pipelinerun/resources/apply_test.go index b8df4bd6f2b..e0766564bb7 100644 --- a/pkg/reconciler/pipelinerun/resources/apply_test.go +++ b/pkg/reconciler/pipelinerun/resources/apply_test.go @@ -1618,6 +1618,72 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { runResults map[string][]v1alpha1.RunResult expected []v1beta1.PipelineRunResult }{{ + description: "apply-array-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.NewArrayOrString("do", "rae", "mi"), + }, + }, + }, + expected: []v1beta1.PipelineRunResult{{ + Name: "pipeline-result-1", + Value: *v1beta1.NewArrayOrString("do", "rae", "mi"), + }}, + }, { + description: "apply-array-indexing-results", + results: []v1beta1.PipelineResult{{ + Name: "pipeline-result-1", + Value: *v1beta1.NewArrayOrString("$(tasks.pt1.results.foo[1])"), + }}, + taskResults: map[string][]v1beta1.TaskRunResult{ + "pt1": { + { + Name: "foo", + Value: *v1beta1.NewArrayOrString("do", "rae", "mi"), + }, + }, + }, + expected: []v1beta1.PipelineRunResult{{ + Name: "pipeline-result-1", + Value: *v1beta1.NewArrayOrString("rae"), + }}, + }, { + description: "multiple-array-results-multiple-successful-tasks ", + results: []v1beta1.PipelineResult{{ + Name: "pipeline-result-1", + Value: *v1beta1.NewArrayOrString("$(tasks.pt1.results.foo[*])"), + }, { + Name: "pipeline-result-2", + Value: *v1beta1.NewArrayOrString("$(tasks.pt2.results.bar[*])"), + }}, + taskResults: map[string][]v1beta1.TaskRunResult{ + "pt1": { + { + Name: "foo", + Value: *v1beta1.NewArrayOrString("do", "rae"), + }, + }, + "pt2": { + { + Name: "bar", + Value: *v1beta1.NewArrayOrString("do", "rae", "mi"), + }, + }, + }, + expected: []v1beta1.PipelineRunResult{{ + Name: "pipeline-result-1", + Value: *v1beta1.NewArrayOrString("do", "rae"), + }, { + Name: "pipeline-result-2", + Value: *v1beta1.NewArrayOrString("do", "rae", "mi"), + }}, + }, { description: "no-pipeline-results-no-returned-results", results: []v1beta1.PipelineResult{}, taskResults: map[string][]v1beta1.TaskRunResult{ @@ -1631,7 +1697,7 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { description: "invalid-result-variable-no-returned-result", results: []v1beta1.PipelineResult{{ Name: "foo", - Value: "$(tasks.pt1_results.foo)", + Value: *v1beta1.NewArrayOrString("$(tasks.pt1_results.foo)"), }}, taskResults: map[string][]v1beta1.TaskRunResult{ "pt1": {{ @@ -1644,7 +1710,7 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { description: "no-taskrun-results-no-returned-results", results: []v1beta1.PipelineResult{{ Name: "foo", - Value: "$(tasks.pt1.results.foo)", + Value: *v1beta1.NewArrayOrString("$(tasks.pt1.results.foo)"), }}, taskResults: map[string][]v1beta1.TaskRunResult{ "pt1": {}, @@ -1654,7 +1720,7 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { description: "invalid-taskrun-name-no-returned-result", results: []v1beta1.PipelineResult{{ Name: "foo", - Value: "$(tasks.pt1.results.foo)", + Value: *v1beta1.NewArrayOrString("$(tasks.pt1.results.foo)"), }}, taskResults: map[string][]v1beta1.TaskRunResult{ "definitely-not-pt1": {{ @@ -1667,7 +1733,7 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { description: "invalid-result-name-no-returned-result", results: []v1beta1.PipelineResult{{ Name: "foo", - Value: "$(tasks.pt1.results.foo)", + Value: *v1beta1.NewArrayOrString("$(tasks.pt1.results.foo)"), }}, taskResults: map[string][]v1beta1.TaskRunResult{ "pt1": {{ @@ -1680,7 +1746,7 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { description: "unsuccessful-taskrun-no-returned-result", results: []v1beta1.PipelineResult{{ Name: "foo", - Value: "$(tasks.pt1.results.foo)", + Value: *v1beta1.NewArrayOrString("$(tasks.pt1.results.foo)"), }}, taskResults: map[string][]v1beta1.TaskRunResult{}, expected: nil, @@ -1688,10 +1754,10 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { description: "mixed-success-tasks-some-returned-results", results: []v1beta1.PipelineResult{{ Name: "foo", - Value: "$(tasks.pt1.results.foo)", + Value: *v1beta1.NewArrayOrString("$(tasks.pt1.results.foo)"), }, { Name: "bar", - Value: "$(tasks.pt2.results.bar)", + Value: *v1beta1.NewArrayOrString("$(tasks.pt2.results.bar)"), }}, taskResults: map[string][]v1beta1.TaskRunResult{ "pt2": {{ @@ -1701,16 +1767,16 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { }, expected: []v1beta1.PipelineRunResult{{ Name: "bar", - Value: "rae", + Value: *v1beta1.NewArrayOrString("rae"), }}, }, { - description: "multiple-results-multiple-successful-tasks", + description: "multiple-results-multiple-successful-tasks ", results: []v1beta1.PipelineResult{{ Name: "pipeline-result-1", - Value: "$(tasks.pt1.results.foo)", + Value: *v1beta1.NewArrayOrString("$(tasks.pt1.results.foo)"), }, { Name: "pipeline-result-2", - Value: "$(tasks.pt1.results.foo), $(tasks.pt2.results.baz), $(tasks.pt1.results.bar), $(tasks.pt2.results.baz), $(tasks.pt1.results.foo)", + Value: *v1beta1.NewArrayOrString("$(tasks.pt1.results.foo), $(tasks.pt2.results.baz), $(tasks.pt1.results.bar), $(tasks.pt2.results.baz), $(tasks.pt1.results.foo)"), }}, taskResults: map[string][]v1beta1.TaskRunResult{ "pt1": { @@ -1729,16 +1795,16 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { }, expected: []v1beta1.PipelineRunResult{{ Name: "pipeline-result-1", - Value: "do", + Value: *v1beta1.NewArrayOrString("do"), }, { Name: "pipeline-result-2", - Value: "do, rae, mi, rae, do", + Value: *v1beta1.NewArrayOrString("do, rae, mi, rae, do"), }}, }, { description: "no-run-results-no-returned-results", results: []v1beta1.PipelineResult{{ Name: "foo", - Value: "$(tasks.customtask.results.foo)", + Value: *v1beta1.NewArrayOrString("$(tasks.customtask.results.foo)"), }}, runResults: map[string][]v1alpha1.RunResult{}, expected: nil, @@ -1746,7 +1812,7 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { description: "wrong-customtask-name-no-returned-result", results: []v1beta1.PipelineResult{{ Name: "foo", - Value: "$(tasks.customtask.results.foo)", + Value: *v1beta1.NewArrayOrString("$(tasks.customtask.results.foo)"), }}, runResults: map[string][]v1alpha1.RunResult{ "differentcustomtask": {{ @@ -1759,7 +1825,7 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { description: "right-customtask-name-wrong-result-name-no-returned-result", results: []v1beta1.PipelineResult{{ Name: "foo", - Value: "$(tasks.customtask.results.foo)", + Value: *v1beta1.NewArrayOrString("$(tasks.customtask.results.foo)"), }}, runResults: map[string][]v1alpha1.RunResult{ "customtask": {{ @@ -1772,7 +1838,7 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { description: "unsuccessful-run-no-returned-result", results: []v1beta1.PipelineResult{{ Name: "foo", - Value: "$(tasks.customtask.results.foo)", + Value: *v1beta1.NewArrayOrString("$(tasks.customtask.results.foo)"), }}, runResults: map[string][]v1alpha1.RunResult{ "customtask": {}, @@ -1782,10 +1848,10 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { description: "multiple-results-custom-and-normal-tasks", results: []v1beta1.PipelineResult{{ Name: "pipeline-result-1", - Value: "$(tasks.customtask.results.foo)", + Value: *v1beta1.NewArrayOrString("$(tasks.customtask.results.foo)"), }, { Name: "pipeline-result-2", - Value: "$(tasks.customtask.results.foo), $(tasks.normaltask.results.baz), $(tasks.customtask.results.bar), $(tasks.normaltask.results.baz), $(tasks.customtask.results.foo)", + Value: *v1beta1.NewArrayOrString("$(tasks.customtask.results.foo), $(tasks.normaltask.results.baz), $(tasks.customtask.results.bar), $(tasks.normaltask.results.baz), $(tasks.customtask.results.foo)"), }}, runResults: map[string][]v1alpha1.RunResult{ "customtask": { @@ -1806,10 +1872,10 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) { }, expected: []v1beta1.PipelineRunResult{{ Name: "pipeline-result-1", - Value: "do", + Value: *v1beta1.NewArrayOrString("do"), }, { Name: "pipeline-result-2", - Value: "do, rae, mi, rae, do", + Value: *v1beta1.NewArrayOrString("do, rae, mi, rae, do"), }}, }} { t.Run(tc.description, func(t *testing.T) { diff --git a/pkg/reconciler/pipelinerun/resources/validate_dependencies_test.go b/pkg/reconciler/pipelinerun/resources/validate_dependencies_test.go index 6fc565840fe..41358dacf4d 100644 --- a/pkg/reconciler/pipelinerun/resources/validate_dependencies_test.go +++ b/pkg/reconciler/pipelinerun/resources/validate_dependencies_test.go @@ -279,7 +279,7 @@ func TestValidatePipelineResults_ValidStates(t *testing.T) { spec: &v1beta1.PipelineSpec{ Results: []v1beta1.PipelineResult{{ Name: "foo-result", - Value: "just a text pipeline result", + Value: *v1beta1.NewArrayOrString("just a text pipeline result"), }}, }, state: nil, @@ -288,7 +288,7 @@ func TestValidatePipelineResults_ValidStates(t *testing.T) { spec: &v1beta1.PipelineSpec{ Results: []v1beta1.PipelineResult{{ Name: "foo-result", - Value: "test $(tasks.pt1.results.result1) 123", + Value: *v1beta1.NewArrayOrString("test $(tasks.pt1.results.result1) 123"), }}, }, state: PipelineRunState{{ @@ -319,7 +319,7 @@ func TestValidatePipelineResults_IncorrectTaskName(t *testing.T) { spec := &v1beta1.PipelineSpec{ Results: []v1beta1.PipelineResult{{ Name: "foo-result", - Value: "$(tasks.pt1.results.result1)", + Value: *v1beta1.NewArrayOrString("$(tasks.pt1.results.result1)"), }}, } state := PipelineRunState{} @@ -335,7 +335,7 @@ func TestValidatePipelineResults_IncorrectResultName(t *testing.T) { spec := &v1beta1.PipelineSpec{ Results: []v1beta1.PipelineResult{{ Name: "foo-result", - Value: "$(tasks.pt1.results.result1)", + Value: *v1beta1.NewArrayOrString("$(tasks.pt1.results.result1)"), }}, } state := PipelineRunState{{ diff --git a/test/custom_task_test.go b/test/custom_task_test.go index 5ad790789d1..5d7607a08e9 100644 --- a/test/custom_task_test.go +++ b/test/custom_task_test.go @@ -248,10 +248,10 @@ spec: expectedPipelineResults := []v1beta1.PipelineRunResult{{ Name: "prResult-ref", - Value: "aResultValue", + Value: *v1beta1.NewArrayOrString("aResultValue"), }, { Name: "prResult-spec", - Value: "aResultValue", + Value: *v1beta1.NewArrayOrString("aResultValue"), }} if len(pr.Status.PipelineResults) != 2 {