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 <[email protected]>
tektoncd · Jun 14, 2023 · 2e2f7a1 · 2e2f7a1
1 parent fb292df
commit 2e2f7a1
Show file tree

Hide file tree

Showing 17 changed files with 591 additions and 127 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -349,6 +349,7 @@ func (c *Reconciler) resolvePipelineState(
 			},
 			getCustomRunFunc,
 			task,
+			pst,
 		)
 		if err != nil {
 			if tresources.IsGetTaskErrTransient(err) {
@@ -717,18 +718,14 @@ func (c *Reconciler) runNextSchedulableTask(ctx context.Context, pr *v1beta1.Pip
 		return controller.NewPermanentError(err)
 	}
 
-	resolvedResultRefs, _, err := resources.ResolveResultRefs(pipelineRunFacts.State, nextRpts)
+	// Check for Missing Result References
+	err = resources.CheckMissingResultReferences(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 {
@@ -810,7 +807,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 {
@@ -891,7 +887,6 @@ func (c *Reconciler) createCustomRuns(ctx context.Context, rpt *resources.Resolv
 	if rpt.PipelineTask.IsMatrixed() {
 		matrixCombinations = rpt.PipelineTask.Matrix.FanOut()
 	}
-
 	for i, customRunName := range rpt.CustomRunNames {
 		var params v1beta1.Params
 		if len(matrixCombinations) > i {