Fix array param indexing validation

This commit fixes several issues:
- Array param indexing is in beta, but validation of array param indexing in Tasks
assumes it is an alpha feature.
- Since array param indexing is a beta feature, it doesn't require "enable-api-fields"
to be set to beta when used in a beta API.
- Prior to this commit, logic for validating array params accidentally skipped
pipeline.spec.finally.when.input.
- Validation of the value of "enable-api-fields" is moved from the reconciler to the webhook,
or in the case of referenced tasks, before they are converted to v1beta1.

pkg/apis/pipeline/v1/param_types.go

@@ -195,45 +195,6 @@ func (ps ParamSpecs) extractParamArrayLengths() map[string]int {
 	return arrayParamsLengths
 }
 
-// validateOutofBoundArrayParams validates if the array indexing params are out of bound
-// example of arrayIndexingParams: ["$(params.a-array-param[1])", "$(params.b-array-param[2])"]
-// example of arrayParamsLengths: {"a-array-params": 2,"b-array-params": 2 }
-func validateOutofBoundArrayParams(arrayIndexingParams []string, arrayParamsLengths map[string]int) error {
-	outofBoundParams := sets.String{}
-	for _, val := range arrayIndexingParams {
-		indexString := substitution.ExtractIndexString(val)
-		idx, _ := substitution.ExtractIndex(indexString)
-		// this will extract the param name from reference
-		// e.g. $(params.a-array-param[1]) -> a-array-param
-		paramName, _, _ := substitution.ExtractVariablesFromString(substitution.TrimArrayIndex(val), "params")
-
-		if paramLength, ok := arrayParamsLengths[paramName[0]]; ok {
-			if idx >= paramLength {
-				outofBoundParams.Insert(val)
-			}
-		}
-	}
-	if outofBoundParams.Len() > 0 {
-		return fmt.Errorf("non-existent param references:%v", outofBoundParams.List())
-	}
-	return nil
-}
-
-// extractArrayIndexingParamRefs takes a string of the form `foo-$(params.array-param[1])-bar` and extracts the portions of the string that reference an element in an array param.
-// For example, for the string “foo-$(params.array-param[1])-bar-$(params.other-array-param[2])-$(params.string-param)`,
-// it would return ["$(params.array-param[1])", "$(params.other-array-param[2])"].
-func extractArrayIndexingParamRefs(paramReference string) []string {
-	l := []string{}
-	list := substitution.ExtractParamsExpressions(paramReference)
-	for _, val := range list {
-		indexString := substitution.ExtractIndexString(val)
-		if indexString != "" {
-			l = append(l, val)
-		}
-	}
-	return l
-}
-
 // extractParamRefsFromSteps get all array indexing references from steps
 func extractParamRefsFromSteps(steps []Step) []string {
 	paramsRefs := []string{}

pkg/apis/pipeline/v1/pipeline_validation.go

@@ -66,6 +66,7 @@ func (ps *PipelineSpec) Validate(ctx context.Context) (errs *apis.FieldError) {
 	errs = errs.Also(validatePipelineContextVariables(ps.Tasks).ViaField("tasks"))
 	errs = errs.Also(validatePipelineContextVariables(ps.Finally).ViaField("finally"))
 	errs = errs.Also(validateExecutionStatusVariables(ps.Tasks, ps.Finally))
+	errs = errs.Also(ps.ValidateParamArrayIndex(ctx))
 	// Validate the pipeline's workspaces.
 	errs = errs.Also(validatePipelineWorkspacesDeclarations(ps.Workspaces))
 	errs = errs.Also(validatePipelineWorkspacesUsage(ctx, ps.Workspaces, ps.Tasks).ViaField("tasks"))
@@ -686,20 +687,26 @@ func validateResultsFromMatrixedPipelineTasksNotConsumed(tasks []PipelineTask, f
 	return errs
 }
 
-// ValidateParamArrayIndex validates if the param reference to an array param is out of bound.
-// error is returned when the array indexing reference is out of bound of the array param
-// e.g. if a param reference of $(params.array-param[2]) and the array param is of length 2.
-func (ps *PipelineSpec) ValidateParamArrayIndex(ctx context.Context, params Params) error {
-	if !config.CheckAlphaOrBetaAPIFields(ctx) {
+// ValidateParamArrayIndex validates that "enable-api-fields" is set to "alpha" or "beta" if the pipeline spec
+// contains indexing references to array params.
+// This can be removed when array param indexing is moved to "stable".
+func (ps *PipelineSpec) ValidateParamArrayIndex(ctx context.Context) (errs *apis.FieldError) {
+	if config.CheckAlphaOrBetaAPIFields(ctx) {
 		return nil
 	}
-
-	// Collect all array params lengths
-	arrayParamsLengths := ps.Params.extractParamArrayLengths()
-	for k, v := range params.extractParamArrayLengths() {
-		arrayParamsLengths[k] = v
+	arrayParamIndexingRefs := ps.GetIndexingReferencesToArrayParams()
+	if len(arrayParamIndexingRefs) == 0 {
+		return nil
 	}
+	return apis.ErrGeneric(fmt.Sprintf("cannot index into array parameters when 'enable-api-fields' is 'stable', but found indexing references: %s", arrayParamIndexingRefs))
+}
 
+// GetIndexingReferencesToArrayParams returns all strings referencing indices of PipelineRun array parameters
+// from parameters, workspaces, and when expressions defined in the Pipeline's Tasks and Finally Tasks.
+// For example, if a Task in the Pipeline has a parameter with a value "$(params.array-param-name[1])",
+// this would be one of the strings returned.
+// May contain duplicate values.
+func (ps *PipelineSpec) GetIndexingReferencesToArrayParams() []string {
 	paramsRefs := []string{}
 	for i := range ps.Tasks {
 		paramsRefs = append(paramsRefs, ps.Tasks[i].Params.extractValues()...)
@@ -721,15 +728,14 @@ func (ps *PipelineSpec) ValidateParamArrayIndex(ctx context.Context, params Para
 			paramsRefs = append(paramsRefs, ps.Finally[i].Matrix.Params.extractValues()...)
 		}
 		for _, wes := range ps.Finally[i].When {
+			paramsRefs = append(paramsRefs, wes.Input)
 			paramsRefs = append(paramsRefs, wes.Values...)
 		}
 	}
-
 	// extract all array indexing references, for example []{"$(params.array-params[1])"}
 	arrayIndexParamRefs := []string{}
 	for _, p := range paramsRefs {
 		arrayIndexParamRefs = append(arrayIndexParamRefs, extractArrayIndexingParamRefs(p)...)
 	}
-
-	return validateOutofBoundArrayParams(arrayIndexParamRefs, arrayParamsLengths)
+	return arrayIndexParamRefs
 }