From df074f20304fdef96c6f04dcf3bb7fecc0e72aa0 Mon Sep 17 00:00:00 2001
From: Jerop <jerop@google.com>
Date: Tue, 28 Jun 2022 22:22:50 -0400
Subject: [PATCH] TEP-0090: Support both `matrix` and `params` in a
 `PipelineTask`

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

This change implements the fan out of `PipelineTasks` that have both
`matrix` and `params` specified. The `matrix` is used to fan out the
`PipelineTask` and the `params` are the same in all the `TaskRuns`.

[tep-0090]: https://github.com/tektoncd/community/blob/main/teps/0090-matrix.md
---
 docs/matrix.md                                | 33 ++++++++++++++++
 .../alpha/pipelinerun-with-matrix.yaml        | 12 ++++++
 pkg/reconciler/pipelinerun/pipelinerun.go     |  4 +-
 .../pipelinerun/pipelinerun_test.go           | 39 ++++++++++++++++++-
 4 files changed, 84 insertions(+), 4 deletions(-)

diff --git a/docs/matrix.md b/docs/matrix.md
index 68cd41b4b4d..4e1f43c9137 100644
--- a/docs/matrix.md
+++ b/docs/matrix.md
@@ -11,6 +11,7 @@ weight: 11
 - [Configuring a Matrix](#configuring-a-matrix)
   - [Concurrency Control](#concurrency-control)
   - [Parameters](#parameters)
+    - [Specifying both `params` and `matrix` in a `PipelineTask`](#specifying-both-params-and-matrix-in-a-pipelinetask)
   - [Context Variables](#context-variables)
   - [Results](#results)
     - [Specifying Results in a Matrix](#specifying-results-in-a-matrix)
@@ -113,6 +114,38 @@ A `Parameter` can be passed to either the `matrix` or `params` field, not both.
 For further details on specifying `Parameters` in the `Pipeline` and passing them to
 `PipelineTasks`, see [documentation](pipelines.md#specifying-parameters).
 
+#### Specifying both `params` and `matrix` in a `PipelineTask`
+
+In the example below, the *test* `Task` takes *browser* and *platform* `Parameters` of type
+`"string"`. A `Pipeline` used to run the `Task` on three browsers (using `matrix`) and one
+platform (using `params`) would be specified as such and execute three `TaskRuns`:
+
+```yaml
+apiVersion: tekton.dev/v1beta1
+kind: Pipeline
+metadata:
+  name: platform-browser-tests
+spec:
+  tasks:
+  - name: fetch-repository
+    taskRef:
+      name: git-clone
+    ...
+  - name: test
+    matrix:
+      - name: browser
+        value:
+          - chrome
+          - safari
+          - firefox
+    params:
+      - name: platform
+        value: linux
+    taskRef:
+      name: browser-test
+  ...
+```
+
 ### Context Variables
 
 Similarly to the `Parameters` in the `Params` field, the `Parameters` in the `Matrix` field will accept 
diff --git a/examples/v1beta1/pipelineruns/alpha/pipelinerun-with-matrix.yaml b/examples/v1beta1/pipelineruns/alpha/pipelinerun-with-matrix.yaml
index 405d6a71b9c..e5f2ec582b9 100644
--- a/examples/v1beta1/pipelineruns/alpha/pipelinerun-with-matrix.yaml
+++ b/examples/v1beta1/pipelineruns/alpha/pipelinerun-with-matrix.yaml
@@ -37,3 +37,15 @@ spec:
               - firefox
         taskRef:
           name: platform-browsers
+      - name: matrix-and-params
+        matrix:
+          - name: platform
+            value:
+              - linux
+              - mac
+              - windows
+        params:
+          - name: browser
+            value: chrome
+        taskRef:
+          name: platform-browsers
diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go
index 6d50cbcd379..821be2f18b2 100644
--- a/pkg/reconciler/pipelinerun/pipelinerun.go
+++ b/pkg/reconciler/pipelinerun/pipelinerun.go
@@ -803,9 +803,7 @@ func (c *Reconciler) createTaskRun(ctx context.Context, taskRunName string, para
 
 	rpt.PipelineTask = resources.ApplyPipelineTaskContexts(rpt.PipelineTask)
 	taskRunSpec := pr.GetTaskRunSpec(rpt.PipelineTask.Name)
-	if len(params) == 0 {
-		params = rpt.PipelineTask.Params
-	}
+	params = append(params, rpt.PipelineTask.Params...)
 	tr = &v1beta1.TaskRun{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:            taskRunName,
diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go
index 269a7f714d6..b2ab8a61c69 100644
--- a/pkg/reconciler/pipelinerun/pipelinerun_test.go
+++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go
@@ -7513,11 +7513,12 @@ spec:
   params:
     - name: platform
     - name: browser
+    - name: version
   steps:
     - name: echo
       image: alpine
       script: |
-        echo "$(params.platform) and $(params.browser)"
+        echo "$(params.platform) and $(params.browser) and $(params.version)"
 `)
 
 	expectedTaskRuns := []*v1beta1.TaskRun{
@@ -7531,6 +7532,8 @@ spec:
     value: linux
   - name: browser
     value: chrome
+  - name: version
+    value: v0.33.0
   resources: {}
   serviceAccountName: test-sa
   taskRef:
@@ -7547,6 +7550,8 @@ spec:
     value: mac
   - name: browser
     value: chrome
+  - name: version
+    value: v0.33.0
   resources: {}
   serviceAccountName: test-sa
   taskRef:
@@ -7563,6 +7568,8 @@ spec:
     value: windows
   - name: browser
     value: chrome
+  - name: version
+    value: v0.33.0
   resources: {}
   serviceAccountName: test-sa
   taskRef:
@@ -7579,6 +7586,8 @@ spec:
     value: linux
   - name: browser
     value: safari
+  - name: version
+    value: v0.33.0
   resources: {}
   serviceAccountName: test-sa
   taskRef:
@@ -7595,6 +7604,8 @@ spec:
     value: mac
   - name: browser
     value: safari
+  - name: version
+    value: v0.33.0
   resources: {}
   serviceAccountName: test-sa
   taskRef:
@@ -7611,6 +7622,8 @@ spec:
     value: windows
   - name: browser
     value: safari
+  - name: version
+    value: v0.33.0
   resources: {}
   serviceAccountName: test-sa
   taskRef:
@@ -7627,6 +7640,8 @@ spec:
     value: linux
   - name: browser
     value: firefox
+  - name: version
+    value: v0.33.0
   resources: {}
   serviceAccountName: test-sa
   taskRef:
@@ -7643,6 +7658,8 @@ spec:
     value: mac
   - name: browser
     value: firefox
+  - name: version
+    value: v0.33.0
   resources: {}
   serviceAccountName: test-sa
   taskRef:
@@ -7659,6 +7676,8 @@ spec:
     value: windows
   - name: browser
     value: firefox
+  - name: version
+    value: v0.33.0
   resources: {}
   serviceAccountName: test-sa
   taskRef:
@@ -7698,6 +7717,9 @@ spec:
             - chrome
             - safari
             - firefox
+      params:
+        - name: version
+          value: v0.33.0
 `, "p-dag")),
 		expectedPipelineRun: parse.MustParsePipelineRun(t, `
 metadata:
@@ -7727,6 +7749,9 @@ status:
             - chrome
             - safari
             - firefox
+      params:
+        - name: version
+          value: v0.33.0
   conditions:
   - type: Succeeded
     status: "Unknown"
@@ -7787,6 +7812,8 @@ spec:
           value: linux
         - name: browser
           value: chrome
+        - name: version
+          value: v0.22.0
       taskRef:
         name: mytask
   finally:
@@ -7804,6 +7831,9 @@ spec:
             - chrome
             - safari
             - firefox
+      params:
+        - name: version
+          value: v0.33.0
 `, "p-finally")),
 		tr: mustParseTaskRunWithObjectMeta(t,
 			taskRunObjectMeta("pr-unmatrixed-pt", "foo",
@@ -7815,6 +7845,8 @@ spec:
     value: linux
   - name: browser
     value: chrome
+  - name: version
+    value: v0.22.0
   resources: {}
   serviceAccountName: test-sa
   taskRef:
@@ -7847,6 +7879,8 @@ status:
           value: linux
         - name: browser
           value: chrome
+        - name: version
+          value: v0.22.0
       taskRef:
         name: mytask
     finally:
@@ -7864,6 +7898,9 @@ status:
             - chrome
             - safari
             - firefox
+      params:
+        - name: version
+          value: v0.33.0
   conditions:
   - type: Succeeded
     status: "Unknown"