TEP-0090: Matrix - Consume Results

[TEP-0090: Matrix][tep-0090] proposed executing a `PipelineTask` in
parallel `TaskRuns` and `Runs` with substitutions from combinations
of `Parameters` in a `Matrix`.

In this change, we implement consuming `Results` in a `Matrix`. This
was disallowed in previous iterations of `Matrix` - validation failed.

With this change, `Matrix` supports Results of type String that are
passed in individually. `Results` of type Array are not yet available
at the `Pipeline` level - this will be handled when it's available.

[tep-0090]: https://github.com/tektoncd/community/blob/main/teps/0090-matrix.md

Author: jerop

docs/matrix.md

@@ -160,8 +160,39 @@ Similarly to the `Parameters` in the `Params` field, the `Parameters` in the `Ma
 #### Specifying Results in a Matrix
 
 Consuming `Results` from previous `TaskRuns` or `Runs` in a `Matrix`, which would dynamically generate 
-`TaskRuns` or `Runs` from the fanned out `PipelineTask`, is not yet supported. This dynamic fan out of
-`PipelineTasks` through consuming `Results` will be supported soon. 
+`TaskRuns` or `Runs` from the fanned out `PipelineTask`, is supported.
+
+`Matrix` supports Results of type String that are passed in individually:
+
+```yaml
+tasks:
+...
+- name: task-4
+  taskRef:
+    name: task-4
+  matrix:
+  - name: values
+    value: 
+    - (tasks.task-1.results.foo) # string
+    - (tasks.task-2.results.bar) # string
+    - (tasks.task-3.results.rad) # string
+```
+
+For further information, see the example in [`PipelineRun` with `Matrix` and `Results`][pr-with-matrix-and-results].
+
+When we support `Results` of type Array at the `Pipeline` level, we will support passing Results into the `Matrix`.
+> Note: Results of type Array are not yet supported in the Pipeline level.
+
+```yaml
+tasks:
+...
+- name: task-5
+  taskRef:
+    name: task-5
+  matrix:
+  - name: values
+    value: (tasks.task-4.results.foo) # array
+```
 
 #### Results from fanned out PipelineTasks
 
@@ -490,4 +521,5 @@ status:
 ```
 
 [cel]: https://github.com/tektoncd/experimental/tree/1609827ea81d05c8d00f8933c5c9d6150cd36989/cel
-[pr-with-matrix]: ../examples/v1beta1/pipelineruns/alpha/pipelinerun-with-matrix.yaml
+[pr-with-matrix]: ../examples/v1beta1/pipelineruns/alpha/pipelinerun-with-matrix.yaml
+[pr-with-matrix-and-results]: ../examples/v1beta1/pipelineruns/alpha/pipelinerun-with-matrix-and-results.yaml

examples/v1beta1/pipelineruns/alpha/pipelinerun-with-matrix-and-results.yaml

@@ -0,0 +1,64 @@
+apiVersion: tekton.dev/v1beta1
+kind: Task
+metadata:
+  name: platform-browsers
+  annotations:
+    description: |
+      A task that does something cool with platforms and browsers
+spec:
+  params:
+    - name: platform
+    - name: browser
+  steps:
+    - name: echo
+      image: alpine
+      script: |
+        echo "$(params.platform) and $(params.browser)"
+---
+apiVersion: tekton.dev/v1beta1
+kind: PipelineRun
+metadata:
+  generateName: matrixed-pr-
+spec:
+  serviceAccountName: 'default'
+  pipelineSpec:
+    tasks:
+      - name: get-platforms
+        taskSpec:
+          results:
+            - name: one
+            - name: two
+            - name: three
+          steps:
+            - name: echo
+              image: alpine
+              script: |
+                printf linux | tee /tekton/results/one
+                printf mac | tee /tekton/results/two
+                printf windows | tee /tekton/results/three
+      - name: get-browsers
+        taskSpec:
+          results:
+            - name: one
+            - name: two
+            - name: three
+          steps:
+            - name: echo
+              image: alpine
+              script: |
+                printf chrome | tee /tekton/results/one
+                printf safari | tee /tekton/results/two
+                printf firefox | tee /tekton/results/three
+      - name: platforms-and-browsers-dag
+        matrix:
+          - name: platform
+            value:
+              - $(tasks.get-platforms.results.one)
+              - $(tasks.get-platforms.results.two)
+              - $(tasks.get-platforms.results.three)
+          - name: browser
+            value:
+              - $(tasks.get-browsers.results.one)
+              - $(tasks.get-browsers.results.two)
+        taskRef:
+          name: platform-browsers

pkg/apis/pipeline/v1beta1/param_types.go

@@ -306,10 +306,6 @@ func validateParametersInTaskMatrix(matrix []Param) (errs *apis.FieldError) {
 		if param.Value.Type != ParamTypeArray {
 			errs = errs.Also(apis.ErrInvalidValue("parameters of type array only are allowed in matrix", "").ViaFieldKey("matrix", param.Name))
 		}
-		// results are not yet allowed in parameters in a matrix - dynamic fanning out will be supported in future milestone
-		if expressions, ok := GetVarSubstitutionExpressionsForParam(param); ok && LooksLikeContainsResultRefs(expressions) {
-			return errs.Also(apis.ErrInvalidValue("result references are not allowed in parameters in a matrix", "value").ViaFieldKey("matrix", param.Name))
-		}
 	}
 	return errs
 }

pkg/apis/pipeline/v1beta1/pipeline_types_test.go

@@ -469,6 +469,24 @@ func TestPipelineTaskList_Deps(t *testing.T) {
 		expectedDeps: map[string][]string{
 			"task-2": {"task-1"},
 		},
+	}, {
+		name: "valid pipeline with resource deps - Task Results in Matrix",
+		tasks: []PipelineTask{{
+			Name: "task-1",
+		}, {
+			Name: "task-2",
+			Matrix: []Param{{
+				Value: ArrayOrString{
+					Type: ParamTypeArray,
+					ArrayVal: []string{
+						"$(tasks.task-1.results.result)",
+					},
+				}},
+			}},
+		},
+		expectedDeps: map[string][]string{
+			"task-2": {"task-1"},
+		},
 	}, {
 		name: "valid pipeline with resource deps - When Expressions",
 		tasks: []PipelineTask{{
@@ -515,12 +533,25 @@ func TestPipelineTaskList_Deps(t *testing.T) {
 				Operator: "in",
 				Values:   []string{"foo"},
 			}},
+		}, {
+			Name:     "task-6",
+			RunAfter: []string{"task-1"},
+			Matrix: []Param{{
+				Value: ArrayOrString{
+					Type: ParamTypeArray,
+					ArrayVal: []string{
+						"$(tasks.task-2.results.result)",
+						"$(tasks.task-5.results.result)",
+					},
+				}},
+			},
 		}},
 		expectedDeps: map[string][]string{
 			"task-2": {"task-1"},
 			"task-3": {"task-1", "task-2"},
 			"task-4": {"task-1", "task-3"},
 			"task-5": {"task-1", "task-4"},
+			"task-6": {"task-1", "task-2", "task-5"},
 		},
 	}, {
 		name: "valid pipeline with ordering deps and resource deps - verify unique dependencies",
@@ -577,12 +608,45 @@ func TestPipelineTaskList_Deps(t *testing.T) {
 				Operator: "in",
 				Values:   []string{"foo"},
 			}},
+		}, {
+			Name:     "task-6",
+			RunAfter: []string{"task-1", "task-2", "task-3", "task-4", "task-5"},
+			Resources: &PipelineTaskResources{
+				Inputs: []PipelineTaskInputResource{{
+					From: []string{"task-1", "task-2"},
+				}},
+			},
+			Params: []Param{{
+				Value: ArrayOrString{
+					Type:      "string",
+					StringVal: "$(tasks.task-4.results.result)",
+				}},
+			},
+			WhenExpressions: WhenExpressions{{
+				Input:    "$(tasks.task-3.results.result)",
+				Operator: "in",
+				Values:   []string{"foo"},
+			}, {
+				Input:    "$(tasks.task-4.results.result)",
+				Operator: "in",
+				Values:   []string{"foo"},
+			}},
+			Matrix: []Param{{
+				Value: ArrayOrString{
+					Type: ParamTypeArray,
+					ArrayVal: []string{
+						"$(tasks.task-2.results.result)",
+						"$(tasks.task-5.results.result)",
+					},
+				}},
+			},
 		}},
 		expectedDeps: map[string][]string{
 			"task-2": {"task-1"},
 			"task-3": {"task-1", "task-2"},
 			"task-4": {"task-1", "task-2", "task-3"},
 			"task-5": {"task-1", "task-2", "task-3", "task-4"},
+			"task-6": {"task-1", "task-2", "task-3", "task-4", "task-5"},
 		},
 	}}
 	for _, tc := range pipelines {
@@ -743,10 +807,6 @@ func TestPipelineTask_validateMatrix(t *testing.T) {
 				Name: "a-param", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.foo-task.results.a-result)"}},
 			}},
 		},
-		wantErrs: &apis.FieldError{
-			Message: "invalid value: result references are not allowed in parameters in a matrix",
-			Paths:   []string{"matrix[a-param].value"},
-		},
 	}, {
 		name: "count of combinations of parameters in the matrix exceeds the maximum",
 		pt: &PipelineTask{

pkg/apis/pipeline/v1beta1/pipeline_validation_test.go

@@ -2895,10 +2895,6 @@ func Test_validateMatrix(t *testing.T) {
 				Name: "b-param", Value: ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"$(tasks.bar-task.results.b-result)"}},
 			}},
 		}},
-		wantErrs: &apis.FieldError{
-			Message: "invalid value: result references are not allowed in parameters in a matrix",
-			Paths:   []string{"[0].matrix[a-param].value", "[1].matrix[b-param].value"},
-		},
 	}}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

pkg/apis/pipeline/v1beta1/resultref.go

@@ -185,7 +185,7 @@ func parseExpression(substitutionExpression string) (string, string, int, string
 // in a PipelineTask and returns a list of any references that are found.
 func PipelineTaskResultRefs(pt *PipelineTask) []*ResultRef {
 	refs := []*ResultRef{}
-	for _, p := range pt.Params {
+	for _, p := range append(pt.Params, pt.Matrix...) {
 		expressions, _ := GetVarSubstitutionExpressionsForParam(p)
 		refs = append(refs, NewResultRefs(expressions)...)
 	}

pkg/apis/pipeline/v1beta1/resultref_test.go

@@ -652,6 +652,11 @@ func TestPipelineTaskResultRefs(t *testing.T) {
 				"$(tasks.pt4.results.r4)",
 			},
 		}},
+		Matrix: []v1beta1.Param{{
+			Value: *v1beta1.NewArrayOrString("$(tasks.pt5.results.r5)", "$(tasks.pt6.results.r6)"),
+		}, {
+			Value: *v1beta1.NewArrayOrString("$(tasks.pt7.results.r7)", "$(tasks.pt8.results.r8)"),
+		}},
 	}
 	refs := v1beta1.PipelineTaskResultRefs(&pt)
 	expectedRefs := []*v1beta1.ResultRef{{
@@ -666,8 +671,24 @@ func TestPipelineTaskResultRefs(t *testing.T) {
 	}, {
 		PipelineTask: "pt4",
 		Result:       "r4",
+	}, {
+		PipelineTask: "pt5",
+		Result:       "r5",
+	}, {
+		PipelineTask: "pt6",
+		Result:       "r6",
+	}, {
+		PipelineTask: "pt7",
+		Result:       "r7",
+	}, {
+		PipelineTask: "pt8",
+		Result:       "r8",
 	}}
-	if d := cmp.Diff(refs, expectedRefs); d != "" {
+	if d := cmp.Diff(refs, expectedRefs, cmpopts.SortSlices(lessResultRef)); d != "" {
 		t.Errorf("%v", d)
 	}
 }
+
+func lessResultRef(i, j *v1beta1.ResultRef) bool {
+	return i.PipelineTask+i.Result < j.PipelineTask+i.Result
+}