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.

Author: EmmaMunley

docs/matrix.md

@@ -188,6 +188,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

@@ -3868,7 +3868,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

@@ -383,6 +383,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
 	}
@@ -499,6 +501,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.PipelineTask.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)
@@ -521,6 +536,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)
@@ -2421,6 +2492,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 {
@@ -2440,6 +2521,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",
@@ -2452,6 +2534,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",
@@ -2464,6 +2547,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",
@@ -2476,6 +2560,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",
@@ -2488,6 +2573,7 @@ func TestResolvedPipelineRunTask_IsFinallySkipped(t *testing.T) {
 				"final-task-4": true,
 				"final-task-5": true,
 				"final-task-6": true,
+				"final-task-7": true,
 			},
 		},
 	}

pkg/reconciler/taskrun/validate_taskrun.go

@@ -33,6 +33,9 @@ import (
 // validateParams validates that all Pipeline Task, Matrix.Params and Matrix.Include parameters all have values, match the specified
 // type and object params have all the keys required
 func validateParams(ctx context.Context, paramSpecs []v1beta1.ParamSpec, params v1beta1.Params, matrixParams v1beta1.Params) error {
+	if paramSpecs == nil {
+		return nil
+	}
 	neededParamsNames, neededParamsTypes := neededParamsNamesAndTypes(paramSpecs)
 	providedParams := params
 	providedParams = append(providedParams, matrixParams...)
@@ -99,6 +102,7 @@ func wrongTypeParamsNames(params []v1beta1.Param, matrix v1beta1.Params, neededP
 			// passed to the task that aren't being used.
 			continue
 		}
+		// Matrix param replacements must be of type String
 		if neededParamsTypes[param.Name] != v1beta1.ParamTypeString {
 			wrongTypeParamNames = append(wrongTypeParamNames, param.Name)
 		}
@@ -160,7 +164,11 @@ func findMissingKeys(neededKeys, providedKeys map[string][]string) map[string][]
 // It also validates that all parameters have values, parameter types match the specified type and
 // object params have all the keys required
 func ValidateResolvedTask(ctx context.Context, params v1beta1.Params, matrix *v1beta1.Matrix, rtr *resources.ResolvedTask) error {
-	if err := validateParams(ctx, rtr.TaskSpec.Params, params, matrix.GetAllParams()); err != nil {
+	var paramSpecs v1beta1.ParamSpecs
+	if rtr != nil {
+		paramSpecs = rtr.TaskSpec.Params
+	}
+	if err := validateParams(ctx, paramSpecs, params, matrix.GetAllParams()); err != nil {
 		return fmt.Errorf("invalid input params for task %s: %w", rtr.TaskName, err)
 	}
 	return nil

pkg/reconciler/taskrun/validate_taskrun_test.go

@@ -47,6 +47,9 @@ func TestValidateResolvedTask_ValidParams(t *testing.T) {
 				}, {
 					Name: "zoo",
 					Type: v1beta1.ParamTypeString,
+				}, {
+					Name: "matrixParam",
+					Type: v1beta1.ParamTypeString,
 				}, {
 					Name: "include",
 					Type: v1beta1.ParamTypeString,
@@ -110,6 +113,8 @@ func TestValidateResolvedTask_ValidParams(t *testing.T) {
 		Params: v1beta1.Params{{
 			Name:  "zoo",
 			Value: *v1beta1.NewStructuredValues("a", "b", "c"),
+		}, {
+			Name: "matrixParam", Value: v1beta1.ParamValue{Type: v1beta1.ParamTypeArray, ArrayVal: []string{}},
 		}},
 		Include: []v1beta1.IncludeParams{{
 			Name: "build-1",