Added Skip Logic: Matrix parameters cannot be empty arrays

If a matrix parameter contains an empty array it will be skipped - no PipelineTask will be created - this follows the same logic for when expressions.

docs/matrix.md

@@ -116,6 +116,7 @@ Note that:
 - The names of the `Parameters` in the `Matrix` must be unique. Specifying the same parameter multiple times
 will result in a validation error.
 - A `Parameter` can be passed to either the `matrix` or `params` field, not both.
+- If the `Matrix` has an empty array `Parameter`, then the `PipelineTask` will be skipped.
 
 For further details on specifying `Parameters` in the `Pipeline` and passing them to
 `PipelineTasks`, see [documentation](pipelines.md#specifying-parameters).

docs/pipeline-api.md

@@ -3811,7 +3811,10 @@ SkippingReason
 <th>Description</th>
 </tr>
 </thead>
-<tbody><tr><td><p>&#34;PipelineRun Finally timeout has been reached&#34;</p></td>
+<tbody><tr><td><p>&#34;Matrix Parameters have an empty array&#34;</p></td>
+<td><p>EmptyArrayInMatrixParams means the task was skipped because Matrix parameters contain empty array.</p>
+</td>
+</tr><tr><td><p>&#34;PipelineRun Finally timeout has been reached&#34;</p></td>
 <td><p>FinallyTimedOutSkip means the task was skipped because the PipelineRun has passed its Timeouts.Finally.</p>
 </td>
 </tr><tr><td><p>&#34;PipelineRun was gracefully cancelled&#34;</p></td>

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

@@ -20,7 +20,7 @@ kind: PipelineRun
 metadata:
   generateName: matrixed-pr-
 spec:
-  serviceAccountName: 'default'
+  serviceAccountName: "default"
   pipelineSpec:
     tasks:
       - name: platforms-and-browsers
@@ -51,3 +51,28 @@ spec:
             value: chrome
         taskRef:
           name: platform-browsers
+      - name: matrix-params-with-empty-array-skipped
+        matrix:
+          params:
+            - name: version
+              value: []
+        taskSpec:
+          params:
+            - name: version
+          steps:
+            - name: echo
+              image: ubuntu
+              script: exit 1
+    finally:
+      - name: matrix-params-with-empty-array-skipped-in-finally
+        matrix:
+          params:
+            - name: version
+              value: []
+        taskSpec:
+          params:
+            - name: version
+          steps:
+            - name: echo
+              image: ubuntu
+              script: exit 1

pkg/apis/pipeline/v1/pipelinerun_types.go

@@ -469,6 +469,8 @@ const (
 	TasksTimedOutSkip SkippingReason = "PipelineRun Tasks timeout has been reached"
 	// FinallyTimedOutSkip means the task was skipped because the PipelineRun has passed its Timeouts.Finally.
 	FinallyTimedOutSkip SkippingReason = "PipelineRun Finally timeout has been reached"
+	// EmptyArrayInMatrixParams means the task was skipped because Matrix parameters contain empty array.
+	EmptyArrayInMatrixParams SkippingReason = "Matrix Parameters have an empty array"
 	// None means the task was not skipped
 	None SkippingReason = "None"
 )

pkg/apis/pipeline/v1beta1/pipelinerun_types.go

@@ -495,6 +495,8 @@ const (
 	TasksTimedOutSkip SkippingReason = "PipelineRun Tasks timeout has been reached"
 	// FinallyTimedOutSkip means the task was skipped because the PipelineRun has passed its Timeouts.Finally.
 	FinallyTimedOutSkip SkippingReason = "PipelineRun Finally timeout has been reached"
+	// EmptyArrayInMatrixParams means the task was skipped because Matrix parameters contain empty array.
+	EmptyArrayInMatrixParams SkippingReason = "Matrix Parameters have an empty array"
 	// None means the task was not skipped
 	None SkippingReason = "None"
 )

pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go

@@ -388,6 +388,8 @@ func (t *ResolvedPipelineTask) skip(facts *PipelineRunFacts) TaskSkipStatus {
 		skippingReason = v1beta1.PipelineTimedOutSkip
 	case t.skipBecausePipelineRunTasksTimeoutReached(facts):
 		skippingReason = v1beta1.TasksTimedOutSkip
+	case t.skipBecauseEmptyArrayInMatrixParams():
+		skippingReason = v1beta1.EmptyArrayInMatrixParams
 	default:
 		skippingReason = v1beta1.None
 	}
@@ -504,6 +506,19 @@ func (t *ResolvedPipelineTask) skipBecausePipelineRunFinallyTimeoutReached(facts
 	return false
 }
 
+// skipBecauseEmptyArrayInMatrixParams returns true if the matrix parameters contain an empty array
+func (t *ResolvedPipelineTask) skipBecauseEmptyArrayInMatrixParams() bool {
+	if t.IsMatrixed() {
+		for _, ps := range t.PipelineTask.Matrix.Params {
+			if len(ps.Value.ArrayVal) == 0 {
+				return true
+			}
+		}
+	}
+
+	return false
+}
+
 // IsFinalTask returns true if a task is a finally task
 func (t *ResolvedPipelineTask) IsFinalTask(facts *PipelineRunFacts) bool {
 	return facts.isFinalTask(t.PipelineTask.Name)
@@ -526,6 +541,8 @@ func (t *ResolvedPipelineTask) IsFinallySkipped(facts *PipelineRunFacts) TaskSki
 			skippingReason = v1beta1.PipelineTimedOutSkip
 		case t.skipBecausePipelineRunFinallyTimeoutReached(facts):
 			skippingReason = v1beta1.FinallyTimedOutSkip
+		case t.skipBecauseEmptyArrayInMatrixParams():
+			skippingReason = v1beta1.EmptyArrayInMatrixParams
 		default:
 			skippingReason = v1beta1.None
 		}

pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go

@@ -1159,6 +1159,77 @@ func TestIsSkipped(t *testing.T) {
 			"mytask1": false,
 			"mytask2": true,
 		},
+	}, {
+		name: "matrix-params-contain-empty-arr",
+		state: PipelineRunState{{
+			// not skipped no empty arrs
+			PipelineTask: &v1beta1.PipelineTask{
+				Name:    "mytask1",
+				TaskRef: &v1beta1.TaskRef{Name: "matrix-1"},
+				Matrix: &v1beta1.Matrix{
+					Params: []v1beta1.Param{{
+						Name: "a-param",
+						Value: v1beta1.ParamValue{
+							Type:     v1beta1.ParamTypeArray,
+							ArrayVal: []string{"foo", "bar"},
+						},
+					}}},
+			},
+			TaskRunName: "pipelinerun-matrix-empty-params",
+			TaskRun:     nil,
+			ResolvedTask: &resources.ResolvedTask{
+				TaskSpec: &task.Spec,
+			},
+		}, {
+			// skipped empty ArrayVal exist in matrix param
+			PipelineTask: &v1beta1.PipelineTask{
+				Name:    "mytask2",
+				TaskRef: &v1beta1.TaskRef{Name: "matrix-2"},
+				Matrix: &v1beta1.Matrix{
+					Params: []v1beta1.Param{{
+						Name: "a-param",
+						Value: v1beta1.ParamValue{
+							Type:     v1beta1.ParamTypeArray,
+							ArrayVal: []string{},
+						},
+					}}},
+			},
+			TaskRunName: "pipelinerun-matrix-empty-params",
+			TaskRun:     nil,
+			ResolvedTask: &resources.ResolvedTask{
+				TaskSpec: &task.Spec,
+			},
+		}, {
+			// skipped empty ArrayVal exist in matrix param
+			PipelineTask: &v1beta1.PipelineTask{
+				Name:    "mytask3",
+				TaskRef: &v1beta1.TaskRef{Name: "matrix-2"},
+				Matrix: &v1beta1.Matrix{
+					Params: []v1beta1.Param{{
+						Name: "a-param",
+						Value: v1beta1.ParamValue{
+							Type:     v1beta1.ParamTypeArray,
+							ArrayVal: []string{"foo", "bar"},
+						},
+					}, {
+						Name: "b-param",
+						Value: v1beta1.ParamValue{
+							Type:     v1beta1.ParamTypeArray,
+							ArrayVal: []string{},
+						},
+					}}},
+			},
+			TaskRunName: "pipelinerun-matrix-empty-params",
+			TaskRun:     nil,
+			ResolvedTask: &resources.ResolvedTask{
+				TaskSpec: &task.Spec,
+			},
+		}},
+		expected: map[string]bool{
+			"mytask1": false,
+			"mytask2": true,
+			"mytask3": true,
+		},
 	}} {
 		t.Run(tc.name, func(t *testing.T) {
 			d, err := dagFromState(tc.state)
@@ -2436,6 +2507,16 @@ func TestResolvedPipelineRunTask_IsFinallySkipped(t *testing.T) {
 				Values:   []string{"none"},
 			}},
 		},
+	}, {
+		PipelineTask: &v1beta1.PipelineTask{
+			Name:    "final-task-7",
+			TaskRef: &v1beta1.TaskRef{Name: "task"},
+			Matrix: &v1beta1.Matrix{
+				Params: []v1beta1.Param{{
+					Name:  "platform",
+					Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeArray, ArrayVal: []string{}},
+				}}},
+		},
 	}}
 
 	testCases := []struct {
@@ -2455,6 +2536,7 @@ func TestResolvedPipelineRunTask_IsFinallySkipped(t *testing.T) {
 				"final-task-4": true,
 				"final-task-5": false,
 				"final-task-6": true,
+				"final-task-7": true,
 			},
 		}, {
 			name:             "finally timeout not yet reached",
@@ -2467,6 +2549,7 @@ func TestResolvedPipelineRunTask_IsFinallySkipped(t *testing.T) {
 				"final-task-4": true,
 				"final-task-5": false,
 				"final-task-6": true,
+				"final-task-7": true,
 			},
 		}, {
 			name:            "pipeline timeout not yet reached",
@@ -2479,6 +2562,7 @@ func TestResolvedPipelineRunTask_IsFinallySkipped(t *testing.T) {
 				"final-task-4": true,
 				"final-task-5": false,
 				"final-task-6": true,
+				"final-task-7": true,
 			},
 		}, {
 			name:             "finally timeout passed",
@@ -2491,6 +2575,7 @@ func TestResolvedPipelineRunTask_IsFinallySkipped(t *testing.T) {
 				"final-task-4": true,
 				"final-task-5": true,
 				"final-task-6": true,
+				"final-task-7": true,
 			},
 		}, {
 			name:            "pipeline timeout passed",
@@ -2503,6 +2588,7 @@ func TestResolvedPipelineRunTask_IsFinallySkipped(t *testing.T) {
 				"final-task-4": true,
 				"final-task-5": true,
 				"final-task-6": true,
+				"final-task-7": true,
 			},
 		},
 	}