add feature gate check for param array indexing

pkg/reconciler/pipelinerun/pipelinerun.go

@@ -94,6 +94,9 @@ const (
 	// ReasonObjectParameterMissKeys indicates that the object param value provided from PipelineRun spec
 	// misses some keys required for the object param declared in Pipeline spec.
 	ReasonObjectParameterMissKeys = "ObjectParameterMissKeys"
+	// ReasonParamArrayIndexingInvalid indicates that the use of param array indexing is not under correct api fields feature gate
+	// or the array is out of bound.
+	ReasonParamArrayIndexingInvalid = "ParamArrayIndexingInvalid"
 	// ReasonCouldntGetTask indicates that the reason for the failure status is that the
 	// associated Pipeline's Tasks couldn't all be retrieved
 	ReasonCouldntGetTask = "CouldntGetTask"
@@ -509,8 +512,8 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get
 	// Ensure that the array reference is not out of bound
 	if err := resources.ValidateParamArrayIndex(ctx, pipelineSpec, pr); err != nil {
 		// This Run has failed, so we need to mark it as failed and stop reconciling it
-		pr.Status.MarkFailed(ReasonObjectParameterMissKeys,
-			"PipelineRun %s/%s parameters is missing object keys required by Pipeline %s/%s's parameters: %s",
+		pr.Status.MarkFailed(ReasonParamArrayIndexingInvalid,
+			"PipelineRun %s/%s array indexing params fail validation by Pipeline %s/%s's parameters: %s",
 			pr.Namespace, pr.Name, pr.Namespace, pipelineMeta.Name, err)
 		return controller.NewPermanentError(err)
 	}

pkg/reconciler/pipelinerun/pipelinerun_test.go

@@ -1060,6 +1060,22 @@ spec:
         name: a-task-that-needs-array-params
 `, v1beta1.ParamTypeArray)),
 		parse.MustParseV1beta1Pipeline(t, fmt.Sprintf(`
+metadata:
+  name: a-pipeline-with-array-indexing-params
+  namespace: foo
+spec:
+  params:
+    - name: some-param
+      type: %s
+  tasks:
+    - name: some-task
+      taskRef:
+        name: a-task-that-needs-array-params
+      params:
+        - name: param
+          value: "$(params.some-param[2])"
+`, v1beta1.ParamTypeArray)),
+		parse.MustParseV1beta1Pipeline(t, fmt.Sprintf(`
 metadata:
   name: a-pipeline-with-object-params
   namespace: foo
@@ -1224,6 +1240,27 @@ spec:
 			"Normal Started",
 			"Warning Failed PipelineRun foo/pipeline-missing-object-param-keys parameters is missing object keys required by Pipeline foo/a-pipeline-with-object-params's parameters: PipelineRun missing object keys for parameters",
 		},
+	}, {
+		name: "invalid-pipeline-array-index-out-of-bound",
+		pipelineRun: parse.MustParseV1beta1PipelineRun(t, `
+metadata:
+  name: pipeline-param-array-out-of-bound
+  namespace: foo
+spec:
+  pipelineRef:
+    name: a-pipeline-with-array-indexing-params
+  params:
+    - name: some-param
+      value:
+        - "a"
+        - "b"
+`),
+		reason:         ReasonParamArrayIndexingInvalid,
+		permanentError: true,
+		wantEvents: []string{
+			"Normal Started",
+			"Warning Failed PipelineRun foo/pipeline-param-array-out-of-bound array indexing params fail validation by Pipeline foo/a-pipeline-with-array-indexing-params's parameters: non-existent param references:[$(params.some-param[2]",
+		},
 	}, {
 		name: "invalid-embedded-pipeline-resources-bot-bound-shd-stop-reconciling",
 		pipelineRun: parse.MustParseV1beta1PipelineRun(t, fmt.Sprintf(`

pkg/reconciler/pipelinerun/resources/validate_params.go

@@ -91,39 +91,51 @@ func ValidateObjectParamRequiredKeys(pipelineParameters []v1beta1.ParamSpec, pip
 
 // ValidateParamArrayIndex validate if the array indexing param reference target is existent
 func ValidateParamArrayIndex(ctx context.Context, p *v1beta1.PipelineSpec, pr *v1beta1.PipelineRun) error {
-	if !config.CheckAlphaOrBetaAPIFields(ctx) {
-		return nil
-	}
-
 	arrayParams := extractParamIndexes(p.Params, pr.Spec.Params)
 
 	outofBoundParams := sets.String{}
+	arrayIndexingParams := []string{}
 
 	// collect all the references
 	for i := range p.Tasks {
-		findInvalidParamArrayReferences(p.Tasks[i].Params, arrayParams, &outofBoundParams)
+		extractArrayIndexingReferencesFromParams(ctx, p.Tasks[i].Params, arrayParams, &arrayIndexingParams)
 		if p.Tasks[i].IsMatrixed() {
-			findInvalidParamArrayReferences(p.Tasks[i].Matrix.Params, arrayParams, &outofBoundParams)
+			extractArrayIndexingReferencesFromParams(ctx, p.Tasks[i].Matrix.Params, arrayParams, &arrayIndexingParams)
 		}
 		for j := range p.Tasks[i].Workspaces {
-			findInvalidParamArrayReference(p.Tasks[i].Workspaces[j].SubPath, arrayParams, &outofBoundParams)
+			taskrun.ExtractArrayIndexingParamReference(ctx, p.Tasks[i].Workspaces[j].SubPath, arrayParams, &arrayIndexingParams)
 		}
 		for _, wes := range p.Tasks[i].WhenExpressions {
-			findInvalidParamArrayReference(wes.Input, arrayParams, &outofBoundParams)
+			taskrun.ExtractArrayIndexingParamReference(ctx, wes.Input, arrayParams, &arrayIndexingParams)
 			for _, v := range wes.Values {
-				findInvalidParamArrayReference(v, arrayParams, &outofBoundParams)
+				taskrun.ExtractArrayIndexingParamReference(ctx, v, arrayParams, &arrayIndexingParams)
 			}
 		}
 	}
 
 	for i := range p.Finally {
-		findInvalidParamArrayReferences(p.Finally[i].Params, arrayParams, &outofBoundParams)
+		extractArrayIndexingReferencesFromParams(ctx, p.Finally[i].Params, arrayParams, &arrayIndexingParams)
 		if p.Finally[i].IsMatrixed() {
-			findInvalidParamArrayReferences(p.Finally[i].Matrix.Params, arrayParams, &outofBoundParams)
+			extractArrayIndexingReferencesFromParams(ctx, p.Finally[i].Matrix.Params, arrayParams, &arrayIndexingParams)
 		}
 		for _, wes := range p.Finally[i].WhenExpressions {
 			for _, v := range wes.Values {
-				findInvalidParamArrayReference(v, arrayParams, &outofBoundParams)
+				taskrun.ExtractArrayIndexingParamReference(ctx, v, arrayParams, &arrayIndexingParams)
+			}
+		}
+	}
+
+	if len(arrayIndexingParams) > 0 && !config.CheckAlphaOrBetaAPIFields(ctx) {
+		return fmt.Errorf(`indexing into array params: %v require "enable-api-fields" feature gate to be "alpha" or "beta"`, arrayIndexingParams)
+	}
+
+	for _, val := range arrayIndexingParams {
+		indexString := substitution.ExtractIndexString(val)
+		idx, _ := substitution.ExtractIndex(indexString)
+		v := substitution.TrimArrayIndex(val)
+		if paramLength, ok := arrayParams[v]; ok {
+			if idx >= paramLength {
+				outofBoundParams.Insert(val)
 			}
 		}
 	}
@@ -170,28 +182,15 @@ func extractParamIndexes(defaults []v1beta1.ParamSpec, params []v1beta1.Param) m
 	return arrayParams
 }
 
-func findInvalidParamArrayReferences(params []v1beta1.Param, arrayParams map[string]int, outofBoundParams *sets.String) {
+// extractArrayIndexingReferencesFromParams validates all the params' array indexing references and check if they are valid
+func extractArrayIndexingReferencesFromParams(ctx context.Context, params []v1beta1.Param, arrayParams map[string]int, arrayIndexingParams *[]string) {
 	for i := range params {
-		findInvalidParamArrayReference(params[i].Value.StringVal, arrayParams, outofBoundParams)
+		taskrun.ExtractArrayIndexingParamReference(ctx, params[i].Value.StringVal, arrayParams, arrayIndexingParams)
 		for _, v := range params[i].Value.ArrayVal {
-			findInvalidParamArrayReference(v, arrayParams, outofBoundParams)
+			taskrun.ExtractArrayIndexingParamReference(ctx, v, arrayParams, arrayIndexingParams)
 		}
 		for _, v := range params[i].Value.ObjectVal {
-			findInvalidParamArrayReference(v, arrayParams, outofBoundParams)
-		}
-	}
-}
-
-func findInvalidParamArrayReference(paramReference string, arrayParams map[string]int, outofBoundParams *sets.String) {
-	list := substitution.ExtractParamsExpressions(paramReference)
-	for _, val := range list {
-		indexString := substitution.ExtractIndexString(paramReference)
-		idx, _ := substitution.ExtractIndex(indexString)
-		v := substitution.TrimArrayIndex(val)
-		if paramLength, ok := arrayParams[v]; ok {
-			if idx >= paramLength {
-				outofBoundParams.Insert(val)
-			}
+			taskrun.ExtractArrayIndexingParamReference(ctx, v, arrayParams, arrayIndexingParams)
 		}
 	}
 }

pkg/reconciler/pipelinerun/resources/validate_params_test.go

@@ -506,15 +506,12 @@ func TestValidateParamArrayIndex_valid(t *testing.T) {
 }
 
 func TestValidateParamArrayIndex_invalid(t *testing.T) {
-	ctx := context.Background()
-	cfg := config.FromContextOrDefaults(ctx)
-	cfg.FeatureFlags.EnableAPIFields = config.BetaAPIFields
-	ctx = config.ToContext(ctx, cfg)
 	for _, tt := range []struct {
-		name     string
-		original v1beta1.PipelineSpec
-		params   []v1beta1.Param
-		expected error
+		name      string
+		original  v1beta1.PipelineSpec
+		params    []v1beta1.Param
+		apifields string
+		expected  error
 	}{{
 		name: "single parameter reference out of bound",
 		original: v1beta1.PipelineSpec{
@@ -698,11 +695,38 @@ func TestValidateParamArrayIndex_invalid(t *testing.T) {
 		},
 		params:   []v1beta1.Param{{Name: "second-param", Value: *v1beta1.NewStructuredValues("second-value", "second-value-again")}},
 		expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[3])]"),
+	}, {
+		name: "beta gate not enabled",
+		original: v1beta1.PipelineSpec{
+			Params: []v1beta1.ParamSpec{
+				{Name: "first-param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default-value", "default-value-again")},
+				{Name: "second-param", Type: v1beta1.ParamTypeArray},
+			},
+			Tasks: []v1beta1.PipelineTask{{
+				Params: []v1beta1.Param{
+					{Name: "first-task-first-param", Value: *v1beta1.NewStructuredValues("$(params.first-param[2])")},
+					{Name: "first-task-second-param", Value: *v1beta1.NewStructuredValues("static value")},
+				},
+				Workspaces: []v1beta1.WorkspacePipelineTaskBinding{
+					{
+						Name:      "first-workspace",
+						Workspace: "first-workspace",
+						SubPath:   "$(params.second-param[3])",
+					},
+				},
+			}},
+		},
+		params:    []v1beta1.Param{{Name: "second-param", Value: *v1beta1.NewStructuredValues("second-value", "second-value-again")}},
+		apifields: "stable",
+		expected:  fmt.Errorf(`indexing into array params: %v require "enable-api-fields" feature gate to be "alpha" or "beta"`, []string{"$(params.first-param[2])", "$(params.second-param[3])"}),
 	},
 	} {
 		tt := tt // capture range variable
 		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
+			ctx := context.Background()
+			if tt.apifields != "stable" {
+				ctx = config.ToContext(ctx, &config.Config{FeatureFlags: &config.FeatureFlags{EnableAPIFields: "beta"}})
+			}
 			run := &v1beta1.PipelineRun{
 				Spec: v1beta1.PipelineRunSpec{
 					Params: tt.params,