From 6193ac81f58d355df2724dcd447c8235f3de6513 Mon Sep 17 00:00:00 2001 From: Emma Munley <46881325+EmmaMunley@users.noreply.github.com> Date: Fri, 28 Apr 2023 17:44:57 -0400 Subject: [PATCH] Add support for consuming whole array results in matrix This commit adds support for referencing whole array results produced by another PipelineTask as a matrix parameter value. This feature request is necessary before promoting Matrix to beta. For more details please see issue #6110. This closes issue #6602. Co-authored-by: Yongxuan Zhang --- docs/matrix.md | 20 +- .../pipelinerun-with-matrix-and-results.yaml | 7 +- pkg/apis/pipeline/v1/matrix_types.go | 21 - pkg/apis/pipeline/v1/pipeline_types_test.go | 13 - pkg/apis/pipeline/v1/pipeline_validation.go | 1 - .../pipeline/v1/pipeline_validation_test.go | 10 + pkg/apis/pipeline/v1beta1/matrix_types.go | 19 - .../pipeline/v1beta1/pipeline_types_test.go | 13 - .../pipeline/v1beta1/pipeline_validation.go | 1 - .../v1beta1/pipeline_validation_test.go | 10 + pkg/reconciler/pipelinerun/pipelinerun.go | 14 +- .../pipelinerun/pipelinerun_test.go | 401 +++++++++++++++++- pkg/reconciler/pipelinerun/resources/apply.go | 5 +- .../resources/pipelinerunresolution.go | 5 +- .../resources/pipelinerunresolution_test.go | 79 ++-- .../example_customrun_matrix_results.yaml | 57 +++ 16 files changed, 548 insertions(+), 128 deletions(-) create mode 100644 test/custom-task-ctrls/wait-task-beta/example_customrun_matrix_results.yaml diff --git a/docs/matrix.md b/docs/matrix.md index fcffb8e7440..9727bc525c5 100644 --- a/docs/matrix.md +++ b/docs/matrix.md @@ -303,7 +303,7 @@ See the end-to-end example in [`PipelineRun` with `Matrix` and `Results`][pr-wit #### Results in Matrix.Params -`Matrix.Params` supports string replacements from `Results` of type String, Array or Object. +`Matrix.Params` supports whole array replacements and string replacements from `Results` of type String, Array or Object ```yaml tasks: @@ -314,16 +314,9 @@ tasks: matrix: params: - name: values - value: - - $(tasks.task-1.results.foo) # string replacement from string result - - $(tasks.task-2.results.bar[0]) # string replacement from array result - - $(tasks.task-3.results.rad.key) # string replacement from object result + value: $(tasks.task-4.results.whole-array[*]) ``` -For further information, see the example in [`PipelineRun` with `Matrix` and `Results`][pr-with-matrix-and-results]. - -We plan to add support passing whole Array `Results` into the `Matrix` (#5925): - ```yaml tasks: ... @@ -333,9 +326,16 @@ tasks: matrix: params: - name: values - value: $(tasks.task-4.results.foo) # array + value: + - $(tasks.task-1.results.a-string-result) + - $(tasks.task-2.results.an-array-result[0]) + - $(tasks.task-3.results.an-object-result.key) ``` + +For further information, see the example in [`PipelineRun` with `Matrix` and `Results`][pr-with-matrix-and-results]. + + #### Results in Matrix.Include.Params `Matrix.Include.Params` supports string replacements from `Results` of type String, Array or Object. diff --git a/examples/v1beta1/pipelineruns/alpha/pipelinerun-with-matrix-and-results.yaml b/examples/v1beta1/pipelineruns/alpha/pipelinerun-with-matrix-and-results.yaml index 20408bb168f..6ebd50e0f8e 100644 --- a/examples/v1beta1/pipelineruns/alpha/pipelinerun-with-matrix-and-results.yaml +++ b/examples/v1beta1/pipelineruns/alpha/pipelinerun-with-matrix-and-results.yaml @@ -21,7 +21,7 @@ kind: PipelineRun metadata: generateName: matrixed-pr- spec: - serviceAccountName: 'default' + serviceAccountName: "default" pipelineSpec: tasks: - name: get-platforms @@ -56,10 +56,7 @@ spec: matrix: params: - name: platform - value: - - $(tasks.get-platforms.results.platforms[0]) - - $(tasks.get-platforms.results.platforms[1]) - - $(tasks.get-platforms.results.platforms[2]) + value: $(tasks.get-platforms.results.platforms[*]) - name: browser value: - $(tasks.get-browsers-and-url.results.browsers[0]) diff --git a/pkg/apis/pipeline/v1/matrix_types.go b/pkg/apis/pipeline/v1/matrix_types.go index 03fba3f29aa..f51a0ac4af4 100644 --- a/pkg/apis/pipeline/v1/matrix_types.go +++ b/pkg/apis/pipeline/v1/matrix_types.go @@ -348,24 +348,3 @@ func (m *Matrix) validateParameterInOneOfMatrixOrParams(params []Param) (errs *a } return errs } - -// validateNoWholeArrayResults() is used to ensure a matrix parameter does not contain result references -// to entire arrays. This is temporary until whole array replacements for matrix parameters are supported. -// See issue #6056 for more details -func (m *Matrix) validateNoWholeArrayResults() (errs *apis.FieldError) { - if m.HasParams() { - for i, param := range m.Params { - val := param.Value.StringVal - expressions, ok := GetVarSubstitutionExpressionsForParam(param) - if ok { - if LooksLikeContainsResultRefs(expressions) { - _, stringIdx := ParseResultName(val) - if stringIdx == "*" { - errs = errs.Also(apis.ErrGeneric("matrix parameters cannot contain whole array result references", "").ViaFieldIndex("matrix.params", i)) - } - } - } - } - } - return errs -} diff --git a/pkg/apis/pipeline/v1/pipeline_types_test.go b/pkg/apis/pipeline/v1/pipeline_types_test.go index e61c2cd008c..4016d447902 100644 --- a/pkg/apis/pipeline/v1/pipeline_types_test.go +++ b/pkg/apis/pipeline/v1/pipeline_types_test.go @@ -696,19 +696,6 @@ func TestPipelineTask_ValidateMatrix(t *testing.T) { Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.foo-task.results.a-result)"}}, }}}, }, - }, { - name: "parameters in matrix contain whole array results references", - pt: &PipelineTask{ - Name: "task", - Matrix: &Matrix{ - Params: Params{{ - Name: "a-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(tasks.foo-task.results.arr-results[*])"}, - }}}, - }, - wantErrs: &apis.FieldError{ - Message: "matrix parameters cannot contain whole array result references", - Paths: []string{"matrix.params[0]"}, - }, }, { name: "count of combinations of parameters in the matrix exceeds the maximum", pt: &PipelineTask{ diff --git a/pkg/apis/pipeline/v1/pipeline_validation.go b/pkg/apis/pipeline/v1/pipeline_validation.go index 5f2309b51d4..53ec3f14af9 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1/pipeline_validation.go @@ -227,7 +227,6 @@ func (pt *PipelineTask) validateMatrix(ctx context.Context) (errs *apis.FieldErr // when the enable-api-fields feature gate is anything but "alpha". errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "matrix", config.AlphaAPIFields)) errs = errs.Also(pt.Matrix.validateCombinationsCount(ctx)) - errs = errs.Also(pt.Matrix.validateNoWholeArrayResults()) errs = errs.Also(pt.Matrix.validateUniqueParams()) } errs = errs.Also(pt.Matrix.validateParameterInOneOfMatrixOrParams(pt.Params)) diff --git a/pkg/apis/pipeline/v1/pipeline_validation_test.go b/pkg/apis/pipeline/v1/pipeline_validation_test.go index 3e35c14ee56..a6b690b4186 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1/pipeline_validation_test.go @@ -3150,6 +3150,16 @@ func Test_validateMatrix(t *testing.T) { Name: "b-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.bar-task.results.b-result)"}}, }}}, }}, + }, { + name: "parameters in matrix contain whole array results references", + tasks: PipelineTaskList{{ + Name: "a-task", + TaskRef: &TaskRef{Name: "a-task"}, + Matrix: &Matrix{ + Params: Params{{ + Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.foo-task.results.a-task-results[*])"}}, + }}}, + }}, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/apis/pipeline/v1beta1/matrix_types.go b/pkg/apis/pipeline/v1beta1/matrix_types.go index ac44ff6891f..19042e11e4b 100644 --- a/pkg/apis/pipeline/v1beta1/matrix_types.go +++ b/pkg/apis/pipeline/v1beta1/matrix_types.go @@ -17,7 +17,6 @@ import ( "context" "fmt" "sort" - "strings" "github.com/tektoncd/pipeline/pkg/apis/config" "golang.org/x/exp/maps" @@ -349,21 +348,3 @@ func (m *Matrix) validateParameterInOneOfMatrixOrParams(params Params) (errs *ap } return errs } - -// validateNoWholeArrayResults() is used to ensure a matrix parameter does not contain result references -// to entire arrays. This is temporary until whole array replacements for matrix paraemeters are supported. -// See issue #6056 for more details -func (m *Matrix) validateNoWholeArrayResults() (errs *apis.FieldError) { - if m.HasParams() { - for i, param := range m.Params { - val := param.Value.StringVal - expressions, ok := GetVarSubstitutionExpressionsForParam(param) - if ok { - if LooksLikeContainsResultRefs(expressions) && strings.Contains(val, "[*]") { - errs = errs.Also(apis.ErrGeneric("matrix parameters cannot contain whole array result references", "").ViaFieldIndex("matrix.params", i)) - } - } - } - } - return errs -} diff --git a/pkg/apis/pipeline/v1beta1/pipeline_types_test.go b/pkg/apis/pipeline/v1beta1/pipeline_types_test.go index 327aed8a2b6..1390b4e0c2c 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_types_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_types_test.go @@ -680,19 +680,6 @@ func TestPipelineTask_validateMatrix(t *testing.T) { Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.foo-task.results.a-result)"}}, }}}, }, - }, { - name: "parameters in matrix contain whole array results references", - pt: &PipelineTask{ - Name: "task", - Matrix: &Matrix{ - Params: Params{{ - Name: "a-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(tasks.foo-task.results.arr-results[*])"}, - }}}, - }, - wantErrs: &apis.FieldError{ - Message: "matrix parameters cannot contain whole array result references", - Paths: []string{"matrix.params[0]"}, - }, }, { name: "count of combinations of parameters in the matrix exceeds the maximum", pt: &PipelineTask{ diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go index a5df46ace0a..fcb87e6b642 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -179,7 +179,6 @@ func (pt *PipelineTask) validateMatrix(ctx context.Context) (errs *apis.FieldErr // when the enable-api-fields feature gate is anything but "alpha". errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "matrix", config.AlphaAPIFields)) errs = errs.Also(pt.Matrix.validateCombinationsCount(ctx)) - errs = errs.Also(pt.Matrix.validateNoWholeArrayResults()) errs = errs.Also(pt.Matrix.validateUniqueParams()) } errs = errs.Also(pt.Matrix.validateParameterInOneOfMatrixOrParams(pt.Params)) diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go index 0018425e69e..a2d691937c8 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go @@ -3193,6 +3193,16 @@ func Test_validateMatrix(t *testing.T) { Name: "b-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.bar-task.results.b-result)"}}, }}}, }}, + }, { + name: "parameters in matrix contain whole array results references", + tasks: PipelineTaskList{{ + Name: "a-task", + TaskRef: &TaskRef{Name: "a-task"}, + Matrix: &Matrix{ + Params: Params{{ + Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.foo-task.results.a-task-results[*])"}}, + }}}, + }}, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index dee03436b64..ff426e4a22f 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -354,6 +354,7 @@ func (c *Reconciler) resolvePipelineState( }, getRunObjectFunc, task, + pst, ) if err != nil { if tresources.IsGetTaskErrTransient(err) { @@ -721,18 +722,6 @@ func (c *Reconciler) runNextSchedulableTask(ctx context.Context, pr *v1beta1.Pip return controller.NewPermanentError(err) } - resolvedResultRefs, _, err := resources.ResolveResultRefs(pipelineRunFacts.State, nextRpts) - if err != nil { - logger.Infof("Failed to resolve task result reference for %q with error %v", pr.Name, err) - pr.Status.MarkFailed(ReasonInvalidTaskResultReference, err.Error()) - return controller.NewPermanentError(err) - } - - resources.ApplyTaskResults(nextRpts, resolvedResultRefs) - // After we apply Task Results, we may be able to evaluate more - // when expressions, so reset the skipped cache - pipelineRunFacts.ResetSkippedCache() - // GetFinalTasks only returns final tasks when a DAG is complete fNextRpts := pipelineRunFacts.GetFinalTasks() if len(fNextRpts) != 0 { @@ -814,7 +803,6 @@ func (c *Reconciler) createTaskRuns(ctx context.Context, rpt *resources.Resolved if rpt.PipelineTask.IsMatrixed() { matrixCombinations = rpt.PipelineTask.Matrix.FanOut() } - for i, taskRunName := range rpt.TaskRunNames { var params v1beta1.Params if len(matrixCombinations) > i { diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 1ca8eb3106d..97b9ebd6623 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -9817,7 +9817,7 @@ spec: } } -func TestReconciler_PipelineTaskMatrixResultsWithArrayIndexing(t *testing.T) { +func TestReconciler_PipelineTaskMatrixResultsWithArrays(t *testing.T) { names.TestingSeed() task := parse.MustParseV1beta1Task(t, ` metadata: @@ -10142,6 +10142,162 @@ status: EnableProvenanceInStatus: true ResultExtractionMethod: "termination-message" MaxResultSize: 4096 +`), + }, { + name: "whole array result replacements in matrix.params", + pName: "p-dag-3", + p: parse.MustParseV1beta1Pipeline(t, fmt.Sprintf(` +metadata: + name: %s + namespace: foo +spec: + tasks: + - name: pt-with-result + params: + - name: platforms + type: array + taskRef: + name: taskwithresults + - name: echo-platforms + matrix: + params: + - name: platform + value: $(tasks.pt-with-result.results.platforms[*]) + taskRef: + name: mytask +`, "p-dag-3")), + tr: mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-pt-with-result", "foo", + "pr", "p-dag-3", "pt-with-result", false), + ` +spec: + serviceAccountName: test-sa + taskRef: + name: taskwithresults +status: + conditions: + - type: Succeeded + status: "True" + reason: Succeeded + message: All Tasks have completed executing + taskResults: + - name: platforms + value: + - linux + - mac + - windows +`), + expectedTaskRuns: []*v1beta1.TaskRun{ + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-echo-platforms-0", "foo", + "pr", "p-dag-3", "echo-platforms", false), + ` +spec: + params: + - name: platform + value: linux + serviceAccountName: test-sa + taskRef: + name: mytask + kind: Task +labels: + tekton.dev/memberOf: tasks + tekton.dev/pipeline: p-dag-3 +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-echo-platforms-1", "foo", + "pr", "p-dag-3", "echo-platforms", false), + ` +spec: + params: + - name: platform + value: mac + serviceAccountName: test-sa + taskRef: + name: mytask + kind: Task +labels: + tekton.dev/memberOf: tasks + tekton.dev/pipeline: p-dag-3 +`), + mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-echo-platforms-2", "foo", + "pr", "p-dag-3", "echo-platforms", false), + ` +spec: + params: + - name: platform + value: windows + serviceAccountName: test-sa + taskRef: + name: mytask + kind: Task +labels: + tekton.dev/memberOf: tasks + tekton.dev/pipeline: p-dag-3 +`), + }, + expectedPipelineRun: parse.MustParseV1beta1PipelineRun(t, ` +metadata: + name: pr + namespace: foo + annotations: {} + labels: + tekton.dev/pipeline: p-dag-3 +spec: + serviceAccountName: test-sa + pipelineRef: + name: p-dag-3 +status: + pipelineSpec: + tasks: + - name: pt-with-result + params: + - name: platforms + type: array + taskRef: + name: taskwithresults + kind: Task + - name: echo-platforms + taskRef: + name: mytask + kind: Task + matrix: + params: + - name: platform + value: $(tasks.pt-with-result.results.platforms[*]) + conditions: + - type: Succeeded + status: "Unknown" + reason: "Running" + message: "Tasks Completed: 1 (Failed: 0, Cancelled 0), Incomplete: 1, Skipped: 0" + childReferences: + - apiVersion: tekton.dev/v1beta1 + kind: TaskRun + name: pr-pt-with-result + pipelineTaskName: pt-with-result + - apiVersion: tekton.dev/v1beta1 + kind: TaskRun + name: pr-echo-platforms-0 + pipelineTaskName: echo-platforms + - apiVersion: tekton.dev/v1beta1 + kind: TaskRun + name: pr-echo-platforms-1 + pipelineTaskName: echo-platforms + - apiVersion: tekton.dev/v1beta1 + kind: TaskRun + name: pr-echo-platforms-2 + pipelineTaskName: echo-platforms + provenance: + featureFlags: + RunningInEnvWithInjectedSidecars: true + EnableTektonOCIBundles: true + EnableAPIFields: "alpha" + AwaitSidecarReadiness: true + VerificationNoMatchPolicy: "ignore" + EnableProvenanceInStatus: true + ResultExtractionMethod: "termination-message" + MaxResultSize: 4096 `), }} for _, tt := range tests { @@ -11191,6 +11347,249 @@ spec: } } +func TestReconciler_CustomPipelineTaskMatrixResultsWholeArray(t *testing.T) { + names.TestingSeed() + + taskwithresults := parse.MustParseV1beta1Task(t, ` +metadata: + name: taskwithresults + namespace: foo +spec: + results: + - name: platforms + type: array + steps: + - name: produce-a-list-of-platforms + image: bash:latest + script: | + #!/usr/bin/env bash + echo -n "[\"linux\",\"mac\",\"windows\"]" | tee $(results.platforms.path) +`) + expectedCustomRuns := []*v1beta1.CustomRun{ + mustParseCustomRunWithObjectMeta(t, + taskRunObjectMeta("pr-pt-matrix-custom-task-0", "foo", + "pr", "p-dag", "pt-matrix-custom-task", false), + ` +spec: + customRef: + apiVersion: example.dev/v0 + kind: Example + params: + - name: platform + value: linux + serviceAccountName: test-sa + taskRef: + name: mytask + kind: Task + labels: + tekton.dev/memberOf: tasks + tekton.dev/pipeline: p-dag +`), + mustParseCustomRunWithObjectMeta(t, + taskRunObjectMeta("pr-pt-matrix-custom-task-1", "foo", + "pr", "p-dag", "pt-matrix-custom-task", false), + ` +spec: + customRef: + apiVersion: example.dev/v0 + kind: Example + params: + - name: platform + value: mac + serviceAccountName: test-sa + taskRef: + name: mytask + kind: Task + labels: + tekton.dev/memberOf: tasks + tekton.dev/pipeline: p-dag +`), + mustParseCustomRunWithObjectMeta(t, + taskRunObjectMeta("pr-pt-matrix-custom-task-2", "foo", + "pr", "p-dag", "pt-matrix-custom-task", false), + ` +spec: + customRef: + apiVersion: example.dev/v0 + kind: Example + params: + - name: platform + value: windows + serviceAccountName: test-sa + taskRef: + name: mytask + kind: Task + labels: + tekton.dev/memberOf: tasks + tekton.dev/pipeline: p-dag +`), + } + + cms := []*corev1.ConfigMap{withEnabledAlphaAPIFields(newFeatureFlagsConfigMap())} + cms = append(cms, withMaxMatrixCombinationsCount(newDefaultsConfigMap(), 10)) + + tests := []struct { + name string + memberOf string + p *v1beta1.Pipeline + tr *v1beta1.TaskRun + expectedPipelineRun *v1beta1.PipelineRun + }{{ + name: "p-dag", + memberOf: "tasks", + p: parse.MustParseV1beta1Pipeline(t, fmt.Sprintf(` +metadata: + name: %s + namespace: foo +spec: + tasks: + - name: pt-with-result + params: + - name: platforms + type: array + taskRef: + name: taskwithresults + - name: pt-matrix-custom-task + matrix: + params: + - name: platform + value: $(tasks.pt-with-result.results.platforms[*]) + taskRef: + apiVersion: example.dev/v0 + kind: Example +`, "p-dag")), + + tr: mustParseTaskRunWithObjectMeta(t, + taskRunObjectMeta("pr-pt-with-result", "foo", + "pr", "p-dag", "pt-with-result", false), + ` +spec: + serviceAccountName: test-sa + taskRef: + name: taskwithresults +status: + conditions: + - type: Succeeded + status: "True" + reason: Succeeded + message: All Tasks have completed executing + taskResults: + - name: platforms + value: + - linux + - mac + - windows +`), + expectedPipelineRun: parse.MustParseV1beta1PipelineRun(t, ` +metadata: + name: pr + namespace: foo + annotations: {} + labels: + tekton.dev/pipeline: p-dag +spec: + serviceAccountName: test-sa + pipelineRef: + name: p-dag +status: + pipelineSpec: + tasks: + - name: pt-with-result + params: + - name: platforms + type: array + taskRef: + name: taskwithresults + kind: Task + - name: pt-matrix-custom-task + taskRef: + apiVersion: example.dev/v0 + kind: Example + matrix: + params: + - name: platform + value: $(tasks.pt-with-result.results.platforms[*]) + conditions: + - type: Succeeded + status: "Unknown" + reason: "Running" + message: "Tasks Completed: 1 (Failed: 0, Cancelled 0), Incomplete: 1, Skipped: 0" + childReferences: + - apiVersion: tekton.dev/v1beta1 + kind: TaskRun + name: pr-pt-with-result + pipelineTaskName: pt-with-result + - apiVersion: tekton.dev/v1beta1 + kind: CustomRun + name: pr-pt-matrix-custom-task-0 + pipelineTaskName: pt-matrix-custom-task + - apiVersion: tekton.dev/v1beta1 + kind: CustomRun + name: pr-pt-matrix-custom-task-1 + pipelineTaskName: pt-matrix-custom-task + - apiVersion: tekton.dev/v1beta1 + kind: CustomRun + name: pr-pt-matrix-custom-task-2 + pipelineTaskName: pt-matrix-custom-task + provenance: + featureFlags: + RunningInEnvWithInjectedSidecars: true + EnableTektonOCIBundles: true + EnableAPIFields: "alpha" + AwaitSidecarReadiness: true + VerificationNoMatchPolicy: "ignore" + EnableProvenanceInStatus: true + ResultExtractionMethod: "termination-message" + MaxResultSize: 4096 +`), + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pr := parse.MustParseV1beta1PipelineRun(t, fmt.Sprintf(` +metadata: + name: pr + namespace: foo +spec: + serviceAccountName: test-sa + pipelineRef: + name: %s +`, tt.name)) + d := test.Data{ + PipelineRuns: []*v1beta1.PipelineRun{pr}, + Pipelines: []*v1beta1.Pipeline{tt.p}, + Tasks: []*v1beta1.Task{taskwithresults}, + ConfigMaps: cms, + } + if tt.tr != nil { + d.TaskRuns = []*v1beta1.TaskRun{tt.tr} + } + prt := newPipelineRunTest(t, d) + defer prt.Cancel() + + pipelineRun, clients := prt.reconcileRun("foo", "pr", []string{}, false) + customRuns, err := clients.Pipeline.TektonV1beta1().CustomRuns("foo").List(prt.TestAssets.Ctx, metav1.ListOptions{}) + if err != nil { + t.Fatalf("Failure to list customRuns's %s", err) + } + + if len(customRuns.Items) != 3 { + t.Fatalf("Expected 3 customRuns got %d", len(customRuns.Items)) + } + + for i := range customRuns.Items { + expectedCustomRun := expectedCustomRuns[i] + if d := cmp.Diff(expectedCustomRun, &customRuns.Items[i], ignoreResourceVersion, ignoreTypeMeta); d != "" { + t.Errorf("expected to see CustomRun %v created. Diff %s", expectedCustomRun.Name, diff.PrintWantGot(d)) + } + } + + if d := cmp.Diff(tt.expectedPipelineRun, pipelineRun, ignoreResourceVersion, ignoreTypeMeta, ignoreLastTransitionTime, ignoreStartTime, ignoreFinallyStartTime, cmpopts.EquateEmpty()); d != "" { + t.Errorf("expected PipelineRun was not created. Diff %s", diff.PrintWantGot(d)) + } + }) + } +} + func TestReconcile_SetDefaults(t *testing.T) { names.TestingSeed() diff --git a/pkg/reconciler/pipelinerun/resources/apply.go b/pkg/reconciler/pipelinerun/resources/apply.go index 100f161116b..6bd107b6501 100644 --- a/pkg/reconciler/pipelinerun/resources/apply.go +++ b/pkg/reconciler/pipelinerun/resources/apply.go @@ -176,9 +176,8 @@ func ApplyTaskResults(targets PipelineRunState, resolvedResultRefs ResolvedResul pipelineTask := resolvedPipelineRunTask.PipelineTask.DeepCopy() pipelineTask.Params = pipelineTask.Params.ReplaceVariables(stringReplacements, arrayReplacements, objectReplacements) if pipelineTask.IsMatrixed() { - // Only string replacements from string, array or object results are supported - // We plan to support array replacements from array results soon (#5925) - pipelineTask.Matrix.Params = pipelineTask.Matrix.Params.ReplaceVariables(stringReplacements, nil, nil) + // String replacements from string, array or object results or array replacements from array results are supported + pipelineTask.Matrix.Params = pipelineTask.Matrix.Params.ReplaceVariables(stringReplacements, arrayReplacements, nil) for i := range pipelineTask.Matrix.Include { // matrix include parameters can only be type string pipelineTask.Matrix.Include[i].Params = pipelineTask.Matrix.Include[i].Params.ReplaceVariables(stringReplacements, nil, nil) diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go index 60bf1e276ea..9a4c08a1b4f 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go @@ -541,14 +541,17 @@ func ResolvePipelineTask( getTaskRun resources.GetTaskRun, getRun GetRun, pipelineTask v1beta1.PipelineTask, + pst PipelineRunState, ) (*ResolvedPipelineTask, error) { rpt := ResolvedPipelineTask{ PipelineTask: &pipelineTask, } rpt.CustomTask = rpt.PipelineTask.TaskRef.IsCustomTask() || rpt.PipelineTask.TaskSpec.IsCustomTask() numCombinations := 1 + resolvedResultRefs, _, _ := ResolveResultRefs(pst, PipelineRunState{&rpt}) + ApplyTaskResults(PipelineRunState{&rpt}, resolvedResultRefs) if rpt.PipelineTask.IsMatrixed() { - numCombinations = pipelineTask.Matrix.CountCombinations() + numCombinations = rpt.PipelineTask.Matrix.CountCombinations() } if rpt.IsCustomTask() { rpt.RunObjectNames = getNamesOfRuns(pipelineRun.Status.ChildReferences, pipelineTask.Name, pipelineRun.Name, numCombinations) diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go index 3cb4ed6c1ee..0a3d4b723e5 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go @@ -2056,7 +2056,7 @@ func TestHasRunObjectsStarted(t *testing.T) { } } -func TestAreRunObjectsConditionStatusFalse(t *testing.T) { +func TestIsConditionStatusFalse(t *testing.T) { for _, tc := range []struct { name string rpt ResolvedPipelineTask @@ -2195,21 +2195,7 @@ func TestAreRunObjectsConditionStatusFalse(t *testing.T) { RunObjects: []v1beta1.RunObject{withCustomRunCancelled(newCustomRun(customRuns[0])), makeCustomRunStarted(customRuns[1])}, }, want: false, - }} { - t.Run(tc.name, func(t *testing.T) { - if got := tc.rpt.areRunObjectsConditionStatusFalse(); got != tc.want { - t.Errorf("expected areRunObjectsConditionStatusFalse: %t but got %t", tc.want, got) - } - }) - } -} - -func TestAreTaskRunsConditionStatusFalse(t *testing.T) { - for _, tc := range []struct { - name string - rpt ResolvedPipelineTask - want bool - }{{ + }, { name: "taskrun not started", rpt: ResolvedPipelineTask{ PipelineTask: &v1beta1.PipelineTask{Name: "task"}, @@ -2335,8 +2321,8 @@ func TestAreTaskRunsConditionStatusFalse(t *testing.T) { want: false, }} { t.Run(tc.name, func(t *testing.T) { - if got := tc.rpt.areTaskRunsConditionStatusFalse(); got != tc.want { - t.Errorf("expected areTaskRunsConditionStatusFalse: %t but got %t", tc.want, got) + if got := tc.rpt.isConditionStatusFalse(); got != tc.want { + t.Errorf("expected isConditionStatusFalse: %t but got %t", tc.want, got) } }) } @@ -2512,7 +2498,7 @@ func TestResolvePipelineRun_CustomTask(t *testing.T) { cfg := config.NewStore(logtesting.TestLogger(t)) ctx = cfg.ToContext(ctx) for _, task := range pts { - ps, err := ResolvePipelineTask(ctx, pr, nopGetTask, nopGetTaskRun, getRun, task) + ps, err := ResolvePipelineTask(ctx, pr, nopGetTask, nopGetTaskRun, getRun, task, nil) if err != nil { t.Fatalf("ResolvePipelineTask: %v", err) } @@ -2563,7 +2549,7 @@ func TestResolvePipelineRun_PipelineTaskHasNoResources(t *testing.T) { } pipelineState := PipelineRunState{} for _, task := range pts { - ps, err := ResolvePipelineTask(context.Background(), pr, getTask, getTaskRun, nopGetRun, task) + ps, err := ResolvePipelineTask(context.Background(), pr, getTask, getTaskRun, nopGetRun, task, nil) if err != nil { t.Errorf("Error getting tasks for fake pipeline %s: %s", p.ObjectMeta.Name, err) } @@ -2614,7 +2600,7 @@ func TestResolvePipelineRun_TaskDoesntExist(t *testing.T) { }, } for _, pt := range pts { - _, err := ResolvePipelineTask(context.Background(), pr, getTask, getTaskRun, nopGetRun, pt) + _, err := ResolvePipelineTask(context.Background(), pr, getTask, getTaskRun, nopGetRun, pt, nil) var tnf *TaskNotFoundError switch { case err == nil: @@ -2654,7 +2640,7 @@ func TestResolvePipelineRun_VerificationFailed(t *testing.T) { }, } for _, pt := range pts { - rt, _ := ResolvePipelineTask(context.Background(), pr, getTask, getTaskRun, nopGetRun, pt) + rt, _ := ResolvePipelineTask(context.Background(), pr, getTask, getTaskRun, nopGetRun, pt, nil) if d := cmp.Diff(verificationResult, rt.ResolvedTask.VerificationResult, cmpopts.EquateErrors()); d != "" { t.Errorf(diff.PrintWantGot(d)) } @@ -2898,7 +2884,7 @@ func TestResolvePipeline_WhenExpressions(t *testing.T) { } t.Run("When Expressions exist", func(t *testing.T) { - _, err := ResolvePipelineTask(context.Background(), pr, getTask, getTaskRun, nopGetRun, pt) + _, err := ResolvePipelineTask(context.Background(), pr, getTask, getTaskRun, nopGetRun, pt, nil) if err != nil { t.Fatalf("Did not expect error when resolving PipelineRun: %v", err) } @@ -3000,7 +2986,7 @@ func TestIsCustomTask(t *testing.T) { ctx := context.Background() cfg := config.NewStore(logtesting.TestLogger(t)) ctx = cfg.ToContext(ctx) - rpt, err := ResolvePipelineTask(ctx, pr, getTask, getTaskRun, getRun, tc.pt) + rpt, err := ResolvePipelineTask(ctx, pr, getTask, getTaskRun, getRun, tc.pt, nil) if err != nil { t.Fatalf("Did not expect error when resolving PipelineRun: %v", err) } @@ -3743,7 +3729,7 @@ func TestIsMatrixed(t *testing.T) { }, }) ctx = cfg.ToContext(ctx) - rpt, err := ResolvePipelineTask(ctx, pr, getTask, getTaskRun, getRun, tc.pt) + rpt, err := ResolvePipelineTask(ctx, pr, getTask, getTaskRun, getRun, tc.pt, nil) if err != nil { t.Fatalf("Did not expect error when resolving PipelineRun: %v", err) } @@ -3803,6 +3789,16 @@ func TestResolvePipelineRunTask_WithMatrix(t *testing.T) { Name: "browsers", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeArray, ArrayVal: []string{"chrome", "safari", "firefox"}}, }}, + }, + }, { + Name: "pipelinetask-with-whole-array-results", + TaskRef: &v1beta1.TaskRef{ + Name: "my-task-with-results", + }, + Matrix: &v1beta1.Matrix{ + Params: v1beta1.Params{{ + Name: "foo", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeString, StringVal: "$(tasks.foo-task.results.arr-results[*])"}, + }}, }}} rtr := &resources.ResolvedTask{ @@ -3840,6 +3836,15 @@ func TestResolvePipelineRunTask_WithMatrix(t *testing.T) { PipelineTask: &pts[1], ResolvedTask: rtr, }, + }, { + name: "task with matrix - whole array results", + pt: pts[2], + want: &ResolvedPipelineTask{ + TaskRunNames: nil, + TaskRuns: nil, + PipelineTask: &pts[2], + ResolvedTask: nil, + }, }} { t.Run(tc.name, func(t *testing.T) { ctx := context.Background() @@ -3851,7 +3856,7 @@ func TestResolvePipelineRunTask_WithMatrix(t *testing.T) { }, }) ctx = cfg.ToContext(ctx) - rpt, err := ResolvePipelineTask(ctx, pr, getTask, getTaskRun, getRun, tc.pt) + rpt, err := ResolvePipelineTask(ctx, pr, getTask, getTaskRun, getRun, tc.pt, nil) if err != nil { t.Fatalf("Did not expect error when resolving PipelineRun: %v", err) } @@ -3914,6 +3919,17 @@ func TestResolvePipelineRunTask_WithMatrixedCustomTask(t *testing.T) { Name: "browsers", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeArray, ArrayVal: []string{"chrome", "safari", "firefox"}}, }}}, + }, { + Name: "pipelinetask-custom-task-with-whole-array-results", + TaskRef: &v1beta1.TaskRef{ + APIVersion: "example.dev/v0", + Kind: "Example", + Name: "my-task", + }, + Matrix: &v1beta1.Matrix{ + Params: v1beta1.Params{{ + Name: "foo", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeString, StringVal: "$(tasks.foo-task.results.arr-results[*])"}}, + }}, }} getTask := func(ctx context.Context, name string) (*v1beta1.Task, *v1beta1.RefSource, *trustedresources.VerificationResult, error) { @@ -3957,6 +3973,15 @@ func TestResolvePipelineRunTask_WithMatrixedCustomTask(t *testing.T) { RunObjects: nil, PipelineTask: &pts[1], }, + }, { + name: "custom task with matrix - whole array results", + pt: pts[2], + want: &ResolvedPipelineTask{ + CustomTask: true, + RunObjectNames: nil, + RunObjects: nil, + PipelineTask: &pts[2], + }, }} { t.Run(tc.name, func(t *testing.T) { ctx := context.Background() @@ -3971,7 +3996,7 @@ func TestResolvePipelineRunTask_WithMatrixedCustomTask(t *testing.T) { if tc.getRun == nil { tc.getRun = getRun } - rpt, err := ResolvePipelineTask(ctx, pr, getTask, getTaskRun, tc.getRun, tc.pt) + rpt, err := ResolvePipelineTask(ctx, pr, getTask, getTaskRun, tc.getRun, tc.pt, nil) if err != nil { t.Fatalf("Did not expect error when resolving PipelineRun: %v", err) } diff --git a/test/custom-task-ctrls/wait-task-beta/example_customrun_matrix_results.yaml b/test/custom-task-ctrls/wait-task-beta/example_customrun_matrix_results.yaml new file mode 100644 index 00000000000..0f43f79d3ab --- /dev/null +++ b/test/custom-task-ctrls/wait-task-beta/example_customrun_matrix_results.yaml @@ -0,0 +1,57 @@ +--- +apiVersion: tekton.dev/v1beta1 +kind: Task +metadata: + name: echo-duration + annotations: + description: | + A task that echos duration +spec: + params: + - name: duration + steps: + - name: echo + image: alpine + script: | + echo "$(params.duration)" +--- +apiVersion: tekton.dev/v1beta1 +kind: Task +metadata: + name: produce-duration +spec: + results: + - name: durations + type: array + steps: + - name: produce-a-list-of-durations + image: bash:latest + script: | + #!/usr/bin/env bash + echo -n "[\"10s\",\"2s\",\"5s\"]" | tee $(results.durations.path) +--- +apiVersion: tekton.dev/v1beta1 +kind: PipelineRun +metadata: + generateName: matrixed-pr- +spec: + pipelineSpec: + tasks: + - name: pt-with-result + taskRef: + name: produce-duration + - name: pt-matrix-echo-duration + matrix: + params: + - name: duration + value: $(tasks.pt-with-result.results.durations[*]) + taskRef: + name: echo-duration + - name: pt-matrix-custom-task + matrix: + params: + - name: duration + value: $(tasks.pt-with-result.results.durations[*]) + taskRef: + apiVersion: wait.testing.tekton.dev/v1beta1 + kind: Wait