From 77aaf21a1dbc30f4282668795656c32003f51255 Mon Sep 17 00:00:00 2001 From: Lee Bernick Date: Wed, 3 May 2023 12:37:44 -0400 Subject: [PATCH] Cleanup: Move array indexing validation out of apis package We validate that indexing references to array parameters are in bounds, based on the default values of the parameters and the parameter values passed in from a PipelineRun/TaskRun. This validation happens in the reconciler, since it requires comparing the Task/Pipeline spec with the parameters defined in the TaskRun/PipelineRun. Prior to this commit, validation code was in the apis package, even though it was called in the reconciler. This is confusing, because this code cannot be used to validate a Task/Pipeline spec in isolation; it can only be used in the context of TaskRuns and PipelineRuns. This commit moves this validation logic to the reconciler and does not introduce any functional changes. --- pkg/apis/pipeline/v1/param_types.go | 32 +- pkg/apis/pipeline/v1/pipeline_validation.go | 15 - .../pipeline/v1/pipeline_validation_test.go | 402 ----------------- pkg/apis/pipeline/v1/task_validation.go | 18 - pkg/apis/pipeline/v1/task_validation_test.go | 195 --------- pkg/apis/pipeline/v1beta1/param_types.go | 32 +- .../pipeline/v1beta1/pipeline_validation.go | 15 - .../v1beta1/pipeline_validation_test.go | 403 ------------------ pkg/apis/pipeline/v1beta1/task_validation.go | 17 - .../pipeline/v1beta1/task_validation_test.go | 195 --------- pkg/reconciler/pipelinerun/pipelinerun.go | 2 +- .../pipelinerun/resources/validate_params.go | 42 ++ .../resources/validate_params_test.go | 403 ++++++++++++++++++ .../taskrun/resources/validate_params.go | 51 +++ .../taskrun/resources/validate_params_test.go | 208 +++++++++ pkg/reconciler/taskrun/taskrun.go | 2 +- 16 files changed, 714 insertions(+), 1318 deletions(-) create mode 100644 pkg/reconciler/taskrun/resources/validate_params.go create mode 100644 pkg/reconciler/taskrun/resources/validate_params_test.go diff --git a/pkg/apis/pipeline/v1/param_types.go b/pkg/apis/pipeline/v1/param_types.go index 18f951369b3..14598b96e11 100644 --- a/pkg/apis/pipeline/v1/param_types.go +++ b/pkg/apis/pipeline/v1/param_types.go @@ -195,9 +195,9 @@ func (ps Params) extractParamMapArrVals() map[string][]string { // Params is a list of Param type Params []Param -// extractParamArrayLengths extract and return the lengths of all array params +// ExtractParamArrayLengths extract and return the lengths of all array params // Example of returned value: {"a-array-params": 2,"b-array-params": 2 } -func (ps Params) extractParamArrayLengths() map[string]int { +func (ps Params) ExtractParamArrayLengths() map[string]int { // Collect all array params arrayParamsLengths := make(map[string]int) @@ -232,9 +232,9 @@ func (ps Params) ReplaceVariables(stringReplacements map[string]string, arrayRep return params } -// extractParamArrayLengths extract and return the lengths of all array params +// ExtractDefaultParamArrayLengths extract and return the lengths of all array params // Example of returned value: {"a-array-params": 2,"b-array-params": 2 } -func (ps ParamSpecs) extractParamArrayLengths() map[string]int { +func (ps ParamSpecs) ExtractDefaultParamArrayLengths() map[string]int { // Collect all array params arrayParamsLengths := make(map[string]int) @@ -249,30 +249,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])"]. diff --git a/pkg/apis/pipeline/v1/pipeline_validation.go b/pkg/apis/pipeline/v1/pipeline_validation.go index 4a426ebf0c3..1d3dc8640b9 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1/pipeline_validation.go @@ -700,21 +700,6 @@ 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. -// TODO(#6616): Move this functionality to the reconciler, as it is only used there -func (ps *PipelineSpec) ValidateParamArrayIndex(ctx context.Context, params Params) error { - // Collect all array params lengths - arrayParamsLengths := ps.Params.extractParamArrayLengths() - for k, v := range params.extractParamArrayLengths() { - arrayParamsLengths[k] = v - } - // extract all array indexing references, for example []{"$(params.array-params[1])"} - arrayIndexParamRefs := ps.GetIndexingReferencesToArrayParams().List() - return validateOutofBoundArrayParams(arrayIndexParamRefs, arrayParamsLengths) -} - // ValidateBetaFeaturesEnabledForParamArrayIndexing 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". diff --git a/pkg/apis/pipeline/v1/pipeline_validation_test.go b/pkg/apis/pipeline/v1/pipeline_validation_test.go index a039914dba9..2aca8618729 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1/pipeline_validation_test.go @@ -3293,183 +3293,6 @@ func getTaskSpec() TaskSpec { } } -func TestValidateParamArrayIndex_valid(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 PipelineSpec - params Params - }{{ - name: "single parameter", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeString}, - }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewStructuredValues("$(params.first-param[1])")}, - {Name: "first-task-second-param", Value: *NewStructuredValues("$(params.second-param[0])")}, - {Name: "first-task-third-param", Value: *NewStructuredValues("static value")}, - }, - }}, - }, - params: Params{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, - }, { - name: "single parameter with when expression", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeString}, - }, - Tasks: []PipelineTask{{ - When: []WhenExpression{{ - Input: "$(params.first-param[1])", - Operator: selection.In, - Values: []string{"$(params.second-param[0])"}, - }}, - }}, - }, - params: Params{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, - }, { - name: "pipeline parameter nested inside task parameter", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewStructuredValues("$(input.workspace.$(params.first-param[0]))")}, - {Name: "first-task-second-param", Value: *NewStructuredValues("$(input.workspace.$(params.second-param[1]))")}, - }, - }}, - }, - params: nil, // no parameter values. - }, { - name: "array parameter", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default", "array", "value")}, - {Name: "second-param", Type: ParamTypeArray}, - }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewStructuredValues("firstelement", "$(params.first-param)")}, - {Name: "first-task-second-param", Value: *NewStructuredValues("firstelement", "$(params.second-param[0])")}, - }, - }}, - }, - params: Params{ - {Name: "second-param", Value: *NewStructuredValues("second-value", "array")}, - }, - }, { - name: "parameter evaluation with final tasks", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeArray}, - }, - Finally: []PipelineTask{{ - Params: Params{ - {Name: "final-task-first-param", Value: *NewStructuredValues("$(params.first-param[0])")}, - {Name: "final-task-second-param", Value: *NewStructuredValues("$(params.second-param[1])")}, - }, - When: WhenExpressions{{ - Input: "$(params.first-param[0])", - Operator: selection.In, - Values: []string{"$(params.second-param[1])"}, - }}, - }}, - }, - params: Params{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, - }, { - name: "parameter evaluation with both tasks and final tasks", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeArray}, - }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "final-task-first-param", Value: *NewStructuredValues("$(params.first-param[0])")}, - {Name: "final-task-second-param", Value: *NewStructuredValues("$(params.second-param[1])")}, - }, - }}, - Finally: []PipelineTask{{ - Params: Params{ - {Name: "final-task-first-param", Value: *NewStructuredValues("$(params.first-param[0])")}, - {Name: "final-task-second-param", Value: *NewStructuredValues("$(params.second-param[1])")}, - }, - When: WhenExpressions{{ - Input: "$(params.first-param[0])", - Operator: selection.In, - Values: []string{"$(params.second-param[1])"}, - }}, - }}, - }, - params: Params{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, - }, { - name: "parameter references with bracket notation and special characters", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first.param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second/param", Type: ParamTypeArray}, - {Name: "third.param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "fourth/param", Type: ParamTypeArray}, - }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewStructuredValues(`$(params["first.param"][0])`)}, - {Name: "first-task-second-param", Value: *NewStructuredValues(`$(params["second/param"][0])`)}, - {Name: "first-task-third-param", Value: *NewStructuredValues(`$(params['third.param'][1])`)}, - {Name: "first-task-fourth-param", Value: *NewStructuredValues(`$(params['fourth/param'][1])`)}, - {Name: "first-task-fifth-param", Value: *NewStructuredValues("static value")}, - }, - }}, - }, - params: Params{ - {Name: "second/param", Value: *NewStructuredValues("second-value", "second-value-again")}, - {Name: "fourth/param", Value: *NewStructuredValues("fourth-value", "fourth-value-again")}, - }, - }, { - name: "single parameter in workspace subpath", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeArray}, - }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewStructuredValues("$(params.first-param[0])")}, - {Name: "first-task-second-param", Value: *NewStructuredValues("static value")}, - }, - Workspaces: []WorkspacePipelineTaskBinding{ - { - Name: "first-workspace", - Workspace: "first-workspace", - SubPath: "$(params.second-param[1])", - }, - }, - }}, - }, - params: Params{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, - }, - } { - tt := tt // capture range variable - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - err := tt.original.ValidateParamArrayIndex(ctx, tt.params) - if err != nil { - t.Errorf("ValidateParamArrayIndex() got err %s", err) - } - }) - } -} - func TestPipelineWithBetaFields(t *testing.T) { tts := []struct { name string @@ -3503,231 +3326,6 @@ func TestPipelineWithBetaFields(t *testing.T) { } } -func TestValidateParamArrayIndex_invalid(t *testing.T) { - for _, tt := range []struct { - name string - original PipelineSpec - params Params - expected error - }{{ - name: "single parameter reference out of bound", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeString}, - }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewStructuredValues("$(params.first-param[2])")}, - {Name: "first-task-second-param", Value: *NewStructuredValues("$(params.second-param[2])")}, - {Name: "first-task-third-param", Value: *NewStructuredValues("static value")}, - }, - }}, - }, - params: Params{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, - expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[2])]"), - }, { - name: "single parameter reference with when expression out of bound", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeString}, - }, - Tasks: []PipelineTask{{ - When: []WhenExpression{{ - Input: "$(params.first-param[2])", - Operator: selection.In, - Values: []string{"$(params.second-param[2])"}, - }}, - }}, - }, - params: Params{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, - expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[2])]"), - }, { - name: "pipeline parameter reference nested inside task parameter out of bound", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewStructuredValues("$(input.workspace.$(params.first-param[2]))")}, - {Name: "first-task-second-param", Value: *NewStructuredValues("$(input.workspace.$(params.second-param[2]))")}, - }, - }}, - }, - params: nil, // no parameter values. - expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[2])]"), - }, { - name: "array parameter reference out of bound", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default", "array", "value")}, - {Name: "second-param", Type: ParamTypeArray}, - }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewStructuredValues("firstelement", "$(params.first-param[3])")}, - {Name: "first-task-second-param", Value: *NewStructuredValues("firstelement", "$(params.second-param[4])")}, - }, - }}, - }, - params: Params{ - {Name: "second-param", Value: *NewStructuredValues("second-value", "array")}, - }, - expected: fmt.Errorf("non-existent param references:[$(params.first-param[3]) $(params.second-param[4])]"), - }, { - name: "object parameter reference out of bound", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default", "array", "value")}, - {Name: "second-param", Type: ParamTypeArray}, - }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewObject(map[string]string{ - "val1": "$(params.first-param[4])", - "val2": "$(params.second-param[4])", - })}, - }, - }}, - }, - params: Params{ - {Name: "second-param", Value: *NewStructuredValues("second-value", "array")}, - }, - expected: fmt.Errorf("non-existent param references:[$(params.first-param[4]) $(params.second-param[4])]"), - }, { - name: "parameter evaluation with final tasks reference out of bound", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeArray}, - }, - Finally: []PipelineTask{{ - Params: Params{ - {Name: "final-task-first-param", Value: *NewStructuredValues("$(params.first-param[2])")}, - {Name: "final-task-second-param", Value: *NewStructuredValues("$(params.second-param[2])")}, - }, - When: WhenExpressions{{ - Input: "$(params.first-param[5])", - Operator: selection.In, - Values: []string{"$(params.second-param[1])"}, - }}, - }}, - }, - params: Params{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, - expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.first-param[5]) $(params.second-param[2])]"), - }, { - name: "parameter evaluation with both tasks and final tasks reference out of bound", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeArray}, - }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "final-task-first-param", Value: *NewStructuredValues("$(params.first-param[2])")}, - {Name: "final-task-second-param", Value: *NewStructuredValues("$(params.second-param[2])")}, - }, - }}, - Finally: []PipelineTask{{ - Params: Params{ - {Name: "final-task-first-param", Value: *NewStructuredValues("$(params.first-param[3])")}, - {Name: "final-task-second-param", Value: *NewStructuredValues("$(params.second-param[3])")}, - }, - When: WhenExpressions{{ - Input: "$(params.first-param[2])", - Operator: selection.In, - Values: []string{"$(params.second-param[2])"}, - }}, - Matrix: &Matrix{ - Params: Params{ - {Name: "final-task-first-param", Value: *NewStructuredValues("$(params.first-param[4])")}, - {Name: "final-task-second-param", Value: *NewStructuredValues("$(params.second-param[4])")}, - }}, - }}, - }, - params: Params{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, - expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.first-param[3]) $(params.first-param[4]) $(params.second-param[2]) $(params.second-param[3]) $(params.second-param[4])]"), - }, { - name: "parameter in matrix reference out of bound", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeArray}, - }, - Tasks: []PipelineTask{{ - Matrix: &Matrix{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewStructuredValues("$(params.first-param[2])")}, - {Name: "first-task-second-param", Value: *NewStructuredValues("static value")}, - }, - }, - }}, - }, - params: Params{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, - expected: fmt.Errorf("non-existent param references:[$(params.first-param[2])]"), - }, { - name: "parameter references with bracket notation and special characters reference out of bound", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first.param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second/param", Type: ParamTypeArray}, - {Name: "third.param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "fourth/param", Type: ParamTypeArray}, - }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewStructuredValues(`$(params["first.param"][2])`)}, - {Name: "first-task-second-param", Value: *NewStructuredValues(`$(params["second/param"][2])`)}, - {Name: "first-task-third-param", Value: *NewStructuredValues(`$(params['third.param'][2])`)}, - {Name: "first-task-fourth-param", Value: *NewStructuredValues(`$(params['fourth/param'][2])`)}, - {Name: "first-task-fifth-param", Value: *NewStructuredValues("static value")}, - }, - }}, - }, - params: Params{ - {Name: "second/param", Value: *NewStructuredValues("second-value", "second-value-again")}, - {Name: "fourth/param", Value: *NewStructuredValues("fourth-value", "fourth-value-again")}, - }, - expected: fmt.Errorf("non-existent param references:[$(params[\"first.param\"][2]) $(params[\"second/param\"][2]) $(params['fourth/param'][2]) $(params['third.param'][2])]"), - }, { - name: "single parameter in workspace subpath reference out of bound", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeArray}, - }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewStructuredValues("$(params.first-param[2])")}, - {Name: "first-task-second-param", Value: *NewStructuredValues("static value")}, - }, - Workspaces: []WorkspacePipelineTaskBinding{ - { - Name: "first-workspace", - Workspace: "first-workspace", - SubPath: "$(params.second-param[3])", - }, - }, - }}, - }, - params: Params{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, - expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[3])]"), - }, - } { - tt := tt // capture range variable - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - err := tt.original.ValidateParamArrayIndex(context.Background(), tt.params) - if d := cmp.Diff(tt.expected.Error(), err.Error()); d != "" { - t.Errorf("ValidateParamArrayIndex() errors diff %s", diff.PrintWantGot(d)) - } - }) - } -} - func TestGetIndexingReferencesToArrayParams(t *testing.T) { for _, tt := range []struct { name string diff --git a/pkg/apis/pipeline/v1/task_validation.go b/pkg/apis/pipeline/v1/task_validation.go index cb5e9955390..cc676c9f0ed 100644 --- a/pkg/apis/pipeline/v1/task_validation.go +++ b/pkg/apis/pipeline/v1/task_validation.go @@ -597,24 +597,6 @@ func (ts *TaskSpec) ValidateBetaFeaturesEnabledForParamArrayIndexing(ctx context return apis.ErrGeneric(fmt.Sprintf("cannot index into array parameters when 'enable-api-fields' is 'stable', but found indexing references: %s", arrayIndexParamRefs)) } -// 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. -// - `trParams` are params from taskrun. -// - `taskSpec` contains params declarations. -// TODO(#6616): Move this functionality to the reconciler, as it is only used there -func (ts *TaskSpec) ValidateParamArrayIndex(ctx context.Context, params Params) error { - // Collect all array params lengths - arrayParamsLengths := ts.Params.extractParamArrayLengths() - for k, v := range params.extractParamArrayLengths() { - arrayParamsLengths[k] = v - } - // extract all array indexing references, for example []{"$(params.array-params[1])"} - arrayIndexParamRefs := ts.GetIndexingReferencesToArrayParams().List() - - return validateOutofBoundArrayParams(arrayIndexParamRefs, arrayParamsLengths) -} - // GetIndexingReferencesToArrayParams returns all strings referencing indices of TaskRun array parameters // from parameters, workspaces, and when expressions defined in the Task. // For example, if a Task has a parameter with a value "$(params.array-param-name[1])", diff --git a/pkg/apis/pipeline/v1/task_validation_test.go b/pkg/apis/pipeline/v1/task_validation_test.go index d625b794308..5eae25adcf8 100644 --- a/pkg/apis/pipeline/v1/task_validation_test.go +++ b/pkg/apis/pipeline/v1/task_validation_test.go @@ -16,7 +16,6 @@ package v1_test import ( "context" "fmt" - "strings" "testing" "time" @@ -1681,200 +1680,6 @@ func TestIncompatibleAPIVersions(t *testing.T) { } } -func TestValidateParamArrayIndex(t *testing.T) { - stepsInvalidReferences := []string{} - for i := 10; i <= 26; i++ { - stepsInvalidReferences = append(stepsInvalidReferences, fmt.Sprintf("$(params.array-params[%d])", i)) - } - volumesInvalidReferences := []string{} - for i := 10; i <= 22; i++ { - volumesInvalidReferences = append(volumesInvalidReferences, fmt.Sprintf("$(params.array-params[%d])", i)) - } - - tcs := []struct { - name string - params v1.Params - taskspec *v1.TaskSpec - expectedError error - }{{ - name: "steps reference invalid", - params: v1.Params{{ - Name: "array-params", - Value: *v1.NewStructuredValues("bar", "foo"), - }}, - taskspec: &v1.TaskSpec{ - Params: []v1.ParamSpec{{ - Name: "array-params", - Default: v1.NewStructuredValues("bar", "foo"), - }}, - Steps: []v1.Step{{ - Name: "$(params.array-params[10])", - Image: "$(params.array-params[11])", - Command: []string{"$(params.array-params[12])"}, - Args: []string{"$(params.array-params[13])"}, - Script: "echo $(params.array-params[14])", - Env: []corev1.EnvVar{{ - Value: "$(params.array-params[15])", - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - Key: "$(params.array-params[16])", - LocalObjectReference: corev1.LocalObjectReference{ - Name: "$(params.array-params[17])", - }, - }, - ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ - Key: "$(params.array-params[18])", - LocalObjectReference: corev1.LocalObjectReference{ - Name: "$(params.array-params[19])", - }, - }, - }, - }}, - EnvFrom: []corev1.EnvFromSource{{ - Prefix: "$(params.array-params[20])", - ConfigMapRef: &corev1.ConfigMapEnvSource{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: "$(params.array-params[21])", - }, - }, - SecretRef: &corev1.SecretEnvSource{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: "$(params.array-params[22])", - }, - }, - }}, - WorkingDir: "$(params.array-params[23])", - VolumeMounts: []corev1.VolumeMount{{ - Name: "$(params.array-params[24])", - MountPath: "$(params.array-params[25])", - SubPath: "$(params.array-params[26])", - }}, - }}, - }, - expectedError: fmt.Errorf("non-existent param references:[%v]", strings.Join(stepsInvalidReferences, " ")), - }, { - name: "stepTemplate reference invalid", - params: v1.Params{{ - Name: "array-params", - Value: *v1.NewStructuredValues("bar", "foo"), - }}, - taskspec: &v1.TaskSpec{ - Params: []v1.ParamSpec{{ - Name: "array-params", - Default: v1.NewStructuredValues("bar", "foo"), - }}, - StepTemplate: &v1.StepTemplate{ - Image: "$(params.array-params[3])", - }, - }, - expectedError: fmt.Errorf("non-existent param references:[%v]", "$(params.array-params[3])"), - }, { - name: "volumes reference invalid", - params: v1.Params{{ - Name: "array-params", - Value: *v1.NewStructuredValues("bar", "foo"), - }}, - taskspec: &v1.TaskSpec{ - Params: []v1.ParamSpec{{ - Name: "array-params", - Default: v1.NewStructuredValues("bar", "foo"), - }}, - Volumes: []corev1.Volume{{ - Name: "$(params.array-params[10])", - VolumeSource: corev1.VolumeSource{ - ConfigMap: &corev1.ConfigMapVolumeSource{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: "$(params.array-params[11])", - }, - Items: []corev1.KeyToPath{{ - Key: "$(params.array-params[12])", - Path: "$(params.array-params[13])", - }, - }, - }, - Secret: &corev1.SecretVolumeSource{ - SecretName: "$(params.array-params[14])", - Items: []corev1.KeyToPath{{ - Key: "$(params.array-params[15])", - Path: "$(params.array-params[16])", - }}, - }, - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: "$(params.array-params[17])", - }, - Projected: &corev1.ProjectedVolumeSource{ - Sources: []corev1.VolumeProjection{{ - ConfigMap: &corev1.ConfigMapProjection{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: "$(params.array-params[18])", - }, - }, - Secret: &corev1.SecretProjection{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: "$(params.array-params[19])", - }, - }, - ServiceAccountToken: &corev1.ServiceAccountTokenProjection{ - Audience: "$(params.array-params[20])", - }, - }}, - }, - CSI: &corev1.CSIVolumeSource{ - NodePublishSecretRef: &corev1.LocalObjectReference{ - Name: "$(params.array-params[21])", - }, - VolumeAttributes: map[string]string{"key": "$(params.array-params[22])"}, - }, - }, - }, - }, - }, - expectedError: fmt.Errorf("non-existent param references:[%v]", strings.Join(volumesInvalidReferences, " ")), - }, { - name: "workspaces reference invalid", - params: v1.Params{{ - Name: "array-params", - Value: *v1.NewStructuredValues("bar", "foo"), - }}, - taskspec: &v1.TaskSpec{ - Params: []v1.ParamSpec{{ - Name: "array-params", - Default: v1.NewStructuredValues("bar", "foo"), - }}, - Workspaces: []v1.WorkspaceDeclaration{{ - MountPath: "$(params.array-params[3])", - }}, - }, - expectedError: fmt.Errorf("non-existent param references:[%v]", "$(params.array-params[3])"), - }, { - name: "sidecar reference invalid", - params: v1.Params{{ - Name: "array-params", - Value: *v1.NewStructuredValues("bar", "foo"), - }}, - taskspec: &v1.TaskSpec{ - Params: []v1.ParamSpec{{ - Name: "array-params", - Default: v1.NewStructuredValues("bar", "foo"), - }}, - Sidecars: []v1.Sidecar{{ - Script: "$(params.array-params[3])", - }, - }, - }, - expectedError: fmt.Errorf("non-existent param references:[%v]", "$(params.array-params[3])"), - }, - } - for _, tc := range tcs { - t.Run(tc.name, func(t *testing.T) { - err := tc.taskspec.ValidateParamArrayIndex(context.Background(), tc.params) - if d := cmp.Diff(tc.expectedError.Error(), err.Error()); d != "" { - t.Errorf("validateParamArrayIndex() errors diff %s", diff.PrintWantGot(d)) - } - }) - } -} - func TestTaskSpecBetaFields(t *testing.T) { tests := []struct { name string diff --git a/pkg/apis/pipeline/v1beta1/param_types.go b/pkg/apis/pipeline/v1beta1/param_types.go index 89ee3d56939..c703fcd5a4d 100644 --- a/pkg/apis/pipeline/v1beta1/param_types.go +++ b/pkg/apis/pipeline/v1beta1/param_types.go @@ -188,9 +188,9 @@ func (ps Params) extractParamMapArrVals() map[string][]string { return paramsMap } -// extractParamArrayLengths extract and return the lengths of all array params +// ExtractParamArrayLengths extract and return the lengths of all array params // Example of returned value: {"a-array-params": 2,"b-array-params": 2 } -func (ps Params) extractParamArrayLengths() map[string]int { +func (ps Params) ExtractParamArrayLengths() map[string]int { // Collect all array params arrayParamsLengths := make(map[string]int) @@ -225,9 +225,9 @@ func (ps Params) ReplaceVariables(stringReplacements map[string]string, arrayRep return params } -// extractParamArrayLengths extract and return the lengths of all array params +// ExtractDefaultParamArrayLengths extract and return the lengths of all array param defaults // Example of returned value: {"a-array-params": 2,"b-array-params": 2 } -func (ps ParamSpecs) extractParamArrayLengths() map[string]int { +func (ps ParamSpecs) ExtractDefaultParamArrayLengths() map[string]int { // Collect all array params arrayParamsLengths := make(map[string]int) @@ -242,30 +242,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])"]. diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go index 33f27701af7..6f0602bd2be 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -724,21 +724,6 @@ 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. -// TODO(#6616): Move this functionality to the reconciler, as it is only used there -func (ps *PipelineSpec) ValidateParamArrayIndex(ctx context.Context, params Params) error { - // Collect all array params lengths - arrayParamsLengths := ps.Params.extractParamArrayLengths() - for k, v := range params.extractParamArrayLengths() { - arrayParamsLengths[k] = v - } - // extract all array indexing references, for example []{"$(params.array-params[1])"} - arrayIndexParamRefs := ps.GetIndexingReferencesToArrayParams().List() - return validateOutofBoundArrayParams(arrayIndexParamRefs, arrayParamsLengths) -} - // 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])", diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go index b269844caec..033b0bd8fe0 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go @@ -3352,409 +3352,6 @@ func enableFeatures(t *testing.T, features []string) func(context.Context) conte } } -func TestValidateParamArrayIndex_valid(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 PipelineSpec - params Params - }{{ - name: "single parameter", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeString}, - }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewStructuredValues("$(params.first-param[1])")}, - {Name: "first-task-second-param", Value: *NewStructuredValues("$(params.second-param[0])")}, - {Name: "first-task-third-param", Value: *NewStructuredValues("static value")}, - }, - }}, - }, - params: Params{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, - }, { - name: "single parameter with when expression", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeString}, - }, - Tasks: []PipelineTask{{ - WhenExpressions: []WhenExpression{{ - Input: "$(params.first-param[1])", - Operator: selection.In, - Values: []string{"$(params.second-param[0])"}, - }}, - }}, - }, - params: Params{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, - }, { - name: "pipeline parameter nested inside task parameter", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewStructuredValues("$(input.workspace.$(params.first-param[0]))")}, - {Name: "first-task-second-param", Value: *NewStructuredValues("$(input.workspace.$(params.second-param[1]))")}, - }, - }}, - }, - params: nil, // no parameter values. - }, { - name: "array parameter", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default", "array", "value")}, - {Name: "second-param", Type: ParamTypeArray}, - }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewStructuredValues("firstelement", "$(params.first-param)")}, - {Name: "first-task-second-param", Value: *NewStructuredValues("firstelement", "$(params.second-param[0])")}, - }, - }}, - }, - params: Params{ - {Name: "second-param", Value: *NewStructuredValues("second-value", "array")}, - }, - }, { - name: "parameter evaluation with final tasks", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeArray}, - }, - Finally: []PipelineTask{{ - Params: Params{ - {Name: "final-task-first-param", Value: *NewStructuredValues("$(params.first-param[0])")}, - {Name: "final-task-second-param", Value: *NewStructuredValues("$(params.second-param[1])")}, - }, - WhenExpressions: WhenExpressions{{ - Input: "$(params.first-param[0])", - Operator: selection.In, - Values: []string{"$(params.second-param[1])"}, - }}, - }}, - }, - params: Params{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, - }, { - name: "parameter evaluation with both tasks and final tasks", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeArray}, - }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "final-task-first-param", Value: *NewStructuredValues("$(params.first-param[0])")}, - {Name: "final-task-second-param", Value: *NewStructuredValues("$(params.second-param[1])")}, - }, - }}, - Finally: []PipelineTask{{ - Params: Params{ - {Name: "final-task-first-param", Value: *NewStructuredValues("$(params.first-param[0])")}, - {Name: "final-task-second-param", Value: *NewStructuredValues("$(params.second-param[1])")}, - }, - WhenExpressions: WhenExpressions{{ - Input: "$(params.first-param[0])", - Operator: selection.In, - Values: []string{"$(params.second-param[1])"}, - }}, - }}, - }, - params: Params{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, - }, { - name: "parameter references with bracket notation and special characters", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first.param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second/param", Type: ParamTypeArray}, - {Name: "third.param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "fourth/param", Type: ParamTypeArray}, - }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewStructuredValues(`$(params["first.param"][0])`)}, - {Name: "first-task-second-param", Value: *NewStructuredValues(`$(params["second/param"][0])`)}, - {Name: "first-task-third-param", Value: *NewStructuredValues(`$(params['third.param'][1])`)}, - {Name: "first-task-fourth-param", Value: *NewStructuredValues(`$(params['fourth/param'][1])`)}, - {Name: "first-task-fifth-param", Value: *NewStructuredValues("static value")}, - }, - }}, - }, - params: Params{ - {Name: "second/param", Value: *NewStructuredValues("second-value", "second-value-again")}, - {Name: "fourth/param", Value: *NewStructuredValues("fourth-value", "fourth-value-again")}, - }, - }, { - name: "single parameter in workspace subpath", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeArray}, - }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewStructuredValues("$(params.first-param[0])")}, - {Name: "first-task-second-param", Value: *NewStructuredValues("static value")}, - }, - Workspaces: []WorkspacePipelineTaskBinding{ - { - Name: "first-workspace", - Workspace: "first-workspace", - SubPath: "$(params.second-param[1])", - }, - }, - }}, - }, - params: Params{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, - }, - } { - tt := tt // capture range variable - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - err := tt.original.ValidateParamArrayIndex(ctx, tt.params) - if err != nil { - t.Errorf("ValidateParamArrayIndex() got err %s", err) - } - }) - } -} - -func TestValidateParamArrayIndex_invalid(t *testing.T) { - ctx := context.Background() - for _, tt := range []struct { - name string - original PipelineSpec - params Params - expected error - }{{ - name: "single parameter reference out of bound", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeString}, - }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewStructuredValues("$(params.first-param[2])")}, - {Name: "first-task-second-param", Value: *NewStructuredValues("$(params.second-param[2])")}, - {Name: "first-task-third-param", Value: *NewStructuredValues("static value")}, - }, - }}, - }, - params: Params{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, - expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[2])]"), - }, { - name: "single parameter reference with when expression out of bound", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeString}, - }, - Tasks: []PipelineTask{{ - WhenExpressions: []WhenExpression{{ - Input: "$(params.first-param[2])", - Operator: selection.In, - Values: []string{"$(params.second-param[2])"}, - }}, - }}, - }, - params: Params{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, - expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[2])]"), - }, { - name: "pipeline parameter reference nested inside task parameter out of bound", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewStructuredValues("$(input.workspace.$(params.first-param[2]))")}, - {Name: "first-task-second-param", Value: *NewStructuredValues("$(input.workspace.$(params.second-param[2]))")}, - }, - }}, - }, - params: nil, // no parameter values. - expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[2])]"), - }, { - name: "array parameter reference out of bound", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default", "array", "value")}, - {Name: "second-param", Type: ParamTypeArray}, - }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewStructuredValues("firstelement", "$(params.first-param[3])")}, - {Name: "first-task-second-param", Value: *NewStructuredValues("firstelement", "$(params.second-param[4])")}, - }, - }}, - }, - params: Params{ - {Name: "second-param", Value: *NewStructuredValues("second-value", "array")}, - }, - expected: fmt.Errorf("non-existent param references:[$(params.first-param[3]) $(params.second-param[4])]"), - }, { - name: "object parameter reference out of bound", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default", "array", "value")}, - {Name: "second-param", Type: ParamTypeArray}, - }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewObject(map[string]string{ - "val1": "$(params.first-param[4])", - "val2": "$(params.second-param[4])", - })}, - }, - }}, - }, - params: Params{ - {Name: "second-param", Value: *NewStructuredValues("second-value", "array")}, - }, - expected: fmt.Errorf("non-existent param references:[$(params.first-param[4]) $(params.second-param[4])]"), - }, { - name: "parameter evaluation with final tasks reference out of bound", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeArray}, - }, - Finally: []PipelineTask{{ - Params: Params{ - {Name: "final-task-first-param", Value: *NewStructuredValues("$(params.first-param[2])")}, - {Name: "final-task-second-param", Value: *NewStructuredValues("$(params.second-param[2])")}, - }, - WhenExpressions: WhenExpressions{{ - Input: "$(params.first-param[5])", - Operator: selection.In, - Values: []string{"$(params.second-param[1])"}, - }}, - }}, - }, - params: Params{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, - expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.first-param[5]) $(params.second-param[2])]"), - }, { - name: "parameter evaluation with both tasks and final tasks reference out of bound", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeArray}, - }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "final-task-first-param", Value: *NewStructuredValues("$(params.first-param[2])")}, - {Name: "final-task-second-param", Value: *NewStructuredValues("$(params.second-param[2])")}, - }, - }}, - Finally: []PipelineTask{{ - Params: Params{ - {Name: "final-task-first-param", Value: *NewStructuredValues("$(params.first-param[3])")}, - {Name: "final-task-second-param", Value: *NewStructuredValues("$(params.second-param[3])")}, - }, - WhenExpressions: WhenExpressions{{ - Input: "$(params.first-param[2])", - Operator: selection.In, - Values: []string{"$(params.second-param[2])"}, - }}, - Matrix: &Matrix{ - Params: Params{ - {Name: "final-task-first-param", Value: *NewStructuredValues("$(params.first-param[4])")}, - {Name: "final-task-second-param", Value: *NewStructuredValues("$(params.second-param[4])")}, - }}, - }}, - }, - params: Params{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, - expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.first-param[3]) $(params.first-param[4]) $(params.second-param[2]) $(params.second-param[3]) $(params.second-param[4])]"), - }, { - name: "parameter in matrix reference out of bound", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeArray}, - }, - Tasks: []PipelineTask{{ - Matrix: &Matrix{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewStructuredValues("$(params.first-param[2])")}, - {Name: "first-task-second-param", Value: *NewStructuredValues("static value")}, - }, - }, - }}, - }, - params: Params{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, - expected: fmt.Errorf("non-existent param references:[$(params.first-param[2])]"), - }, { - name: "parameter references with bracket notation and special characters reference out of bound", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first.param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second/param", Type: ParamTypeArray}, - {Name: "third.param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "fourth/param", Type: ParamTypeArray}, - }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewStructuredValues(`$(params["first.param"][2])`)}, - {Name: "first-task-second-param", Value: *NewStructuredValues(`$(params["second/param"][2])`)}, - {Name: "first-task-third-param", Value: *NewStructuredValues(`$(params['third.param'][2])`)}, - {Name: "first-task-fourth-param", Value: *NewStructuredValues(`$(params['fourth/param'][2])`)}, - {Name: "first-task-fifth-param", Value: *NewStructuredValues("static value")}, - }, - }}, - }, - params: Params{ - {Name: "second/param", Value: *NewStructuredValues("second-value", "second-value-again")}, - {Name: "fourth/param", Value: *NewStructuredValues("fourth-value", "fourth-value-again")}, - }, - expected: fmt.Errorf("non-existent param references:[$(params[\"first.param\"][2]) $(params[\"second/param\"][2]) $(params['fourth/param'][2]) $(params['third.param'][2])]"), - }, { - name: "single parameter in workspace subpath reference out of bound", - original: PipelineSpec{ - Params: []ParamSpec{ - {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, - {Name: "second-param", Type: ParamTypeArray}, - }, - Tasks: []PipelineTask{{ - Params: Params{ - {Name: "first-task-first-param", Value: *NewStructuredValues("$(params.first-param[2])")}, - {Name: "first-task-second-param", Value: *NewStructuredValues("static value")}, - }, - Workspaces: []WorkspacePipelineTaskBinding{ - { - Name: "first-workspace", - Workspace: "first-workspace", - SubPath: "$(params.second-param[3])", - }, - }, - }}, - }, - params: Params{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, - expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[3])]"), - }, - } { - tt := tt // capture range variable - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - err := tt.original.ValidateParamArrayIndex(ctx, tt.params) - if d := cmp.Diff(tt.expected.Error(), err.Error()); d != "" { - t.Errorf("ValidateParamArrayIndex() errors diff %s", diff.PrintWantGot(d)) - } - }) - } -} - func TestGetIndexingReferencesToArrayParams(t *testing.T) { for _, tt := range []struct { name string diff --git a/pkg/apis/pipeline/v1beta1/task_validation.go b/pkg/apis/pipeline/v1beta1/task_validation.go index 00386bd5a9f..931370fd0f4 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation.go +++ b/pkg/apis/pipeline/v1beta1/task_validation.go @@ -581,23 +581,6 @@ func isParamRefs(s string) bool { return strings.HasPrefix(s, "$("+ParamsPrefix) } -// 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. -// - `trParams` are params from taskrun. -// - `taskSpec` contains params declarations. -// TODO(#6616): Move this functionality to the reconciler, as it is only used there -func (ts *TaskSpec) ValidateParamArrayIndex(ctx context.Context, params Params) error { - // Collect all array params lengths - arrayParamsLengths := ts.Params.extractParamArrayLengths() - for k, v := range params.extractParamArrayLengths() { - arrayParamsLengths[k] = v - } - // extract all array indexing references, for example []{"$(params.array-params[1])"} - arrayIndexParamRefs := ts.GetIndexingReferencesToArrayParams().List() - return validateOutofBoundArrayParams(arrayIndexParamRefs, arrayParamsLengths) -} - // GetIndexingReferencesToArrayParams returns all strings referencing indices of TaskRun array parameters // from parameters, workspaces, and when expressions defined in the Task. // For example, if a Task has a parameter with a value "$(params.array-param-name[1])", diff --git a/pkg/apis/pipeline/v1beta1/task_validation_test.go b/pkg/apis/pipeline/v1beta1/task_validation_test.go index 7e0fbd5f77b..bbace443d13 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/task_validation_test.go @@ -19,7 +19,6 @@ package v1beta1_test import ( "context" "fmt" - "strings" "testing" "time" @@ -1693,200 +1692,6 @@ func TestIncompatibleAPIVersions(t *testing.T) { } } -func TestValidateParamArrayIndex(t *testing.T) { - stepsInvalidReferences := []string{} - for i := 10; i <= 26; i++ { - stepsInvalidReferences = append(stepsInvalidReferences, fmt.Sprintf("$(params.array-params[%d])", i)) - } - volumesInvalidReferences := []string{} - for i := 10; i <= 22; i++ { - volumesInvalidReferences = append(volumesInvalidReferences, fmt.Sprintf("$(params.array-params[%d])", i)) - } - - tcs := []struct { - name string - params v1beta1.Params - taskspec *v1beta1.TaskSpec - expectedError error - }{{ - name: "steps reference invalid", - params: v1beta1.Params{{ - Name: "array-params", - Value: *v1beta1.NewStructuredValues("bar", "foo"), - }}, - taskspec: &v1beta1.TaskSpec{ - Params: []v1beta1.ParamSpec{{ - Name: "array-params", - Default: v1beta1.NewStructuredValues("bar", "foo"), - }}, - Steps: []v1beta1.Step{{ - Name: "$(params.array-params[10])", - Image: "$(params.array-params[11])", - Command: []string{"$(params.array-params[12])"}, - Args: []string{"$(params.array-params[13])"}, - Script: "echo $(params.array-params[14])", - Env: []corev1.EnvVar{{ - Value: "$(params.array-params[15])", - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - Key: "$(params.array-params[16])", - LocalObjectReference: corev1.LocalObjectReference{ - Name: "$(params.array-params[17])", - }, - }, - ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ - Key: "$(params.array-params[18])", - LocalObjectReference: corev1.LocalObjectReference{ - Name: "$(params.array-params[19])", - }, - }, - }, - }}, - EnvFrom: []corev1.EnvFromSource{{ - Prefix: "$(params.array-params[20])", - ConfigMapRef: &corev1.ConfigMapEnvSource{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: "$(params.array-params[21])", - }, - }, - SecretRef: &corev1.SecretEnvSource{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: "$(params.array-params[22])", - }, - }, - }}, - WorkingDir: "$(params.array-params[23])", - VolumeMounts: []corev1.VolumeMount{{ - Name: "$(params.array-params[24])", - MountPath: "$(params.array-params[25])", - SubPath: "$(params.array-params[26])", - }}, - }}, - }, - expectedError: fmt.Errorf("non-existent param references:[%v]", strings.Join(stepsInvalidReferences, " ")), - }, { - name: "stepTemplate reference invalid", - params: v1beta1.Params{{ - Name: "array-params", - Value: *v1beta1.NewStructuredValues("bar", "foo"), - }}, - taskspec: &v1beta1.TaskSpec{ - Params: []v1beta1.ParamSpec{{ - Name: "array-params", - Default: v1beta1.NewStructuredValues("bar", "foo"), - }}, - StepTemplate: &v1beta1.StepTemplate{ - Image: "$(params.array-params[3])", - }, - }, - expectedError: fmt.Errorf("non-existent param references:[%v]", "$(params.array-params[3])"), - }, { - name: "volumes reference invalid", - params: v1beta1.Params{{ - Name: "array-params", - Value: *v1beta1.NewStructuredValues("bar", "foo"), - }}, - taskspec: &v1beta1.TaskSpec{ - Params: []v1beta1.ParamSpec{{ - Name: "array-params", - Default: v1beta1.NewStructuredValues("bar", "foo"), - }}, - Volumes: []corev1.Volume{{ - Name: "$(params.array-params[10])", - VolumeSource: corev1.VolumeSource{ - ConfigMap: &corev1.ConfigMapVolumeSource{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: "$(params.array-params[11])", - }, - Items: []corev1.KeyToPath{{ - Key: "$(params.array-params[12])", - Path: "$(params.array-params[13])", - }, - }, - }, - Secret: &corev1.SecretVolumeSource{ - SecretName: "$(params.array-params[14])", - Items: []corev1.KeyToPath{{ - Key: "$(params.array-params[15])", - Path: "$(params.array-params[16])", - }}, - }, - PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ - ClaimName: "$(params.array-params[17])", - }, - Projected: &corev1.ProjectedVolumeSource{ - Sources: []corev1.VolumeProjection{{ - ConfigMap: &corev1.ConfigMapProjection{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: "$(params.array-params[18])", - }, - }, - Secret: &corev1.SecretProjection{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: "$(params.array-params[19])", - }, - }, - ServiceAccountToken: &corev1.ServiceAccountTokenProjection{ - Audience: "$(params.array-params[20])", - }, - }}, - }, - CSI: &corev1.CSIVolumeSource{ - NodePublishSecretRef: &corev1.LocalObjectReference{ - Name: "$(params.array-params[21])", - }, - VolumeAttributes: map[string]string{"key": "$(params.array-params[22])"}, - }, - }, - }, - }, - }, - expectedError: fmt.Errorf("non-existent param references:[%v]", strings.Join(volumesInvalidReferences, " ")), - }, { - name: "workspaces reference invalid", - params: v1beta1.Params{{ - Name: "array-params", - Value: *v1beta1.NewStructuredValues("bar", "foo"), - }}, - taskspec: &v1beta1.TaskSpec{ - Params: []v1beta1.ParamSpec{{ - Name: "array-params", - Default: v1beta1.NewStructuredValues("bar", "foo"), - }}, - Workspaces: []v1beta1.WorkspaceDeclaration{{ - MountPath: "$(params.array-params[3])", - }}, - }, - expectedError: fmt.Errorf("non-existent param references:[%v]", "$(params.array-params[3])"), - }, { - name: "sidecar reference invalid", - params: v1beta1.Params{{ - Name: "array-params", - Value: *v1beta1.NewStructuredValues("bar", "foo"), - }}, - taskspec: &v1beta1.TaskSpec{ - Params: []v1beta1.ParamSpec{{ - Name: "array-params", - Default: v1beta1.NewStructuredValues("bar", "foo"), - }}, - Sidecars: []v1beta1.Sidecar{{ - Script: "$(params.array-params[3])", - }, - }, - }, - expectedError: fmt.Errorf("non-existent param references:[%v]", "$(params.array-params[3])"), - }, - } - for _, tc := range tcs { - t.Run(tc.name, func(t *testing.T) { - err := tc.taskspec.ValidateParamArrayIndex(context.Background(), tc.params) - if d := cmp.Diff(tc.expectedError.Error(), err.Error()); d != "" { - t.Errorf("validateParamArrayIndex() errors diff %s", diff.PrintWantGot(d)) - } - }) - } -} - func TestGetArrayIndexParamRefs(t *testing.T) { stepsReferences := []string{} for i := 10; i <= 26; i++ { diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 30e4125b9a8..7c61db093a2 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -482,7 +482,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get } // Ensure that the array reference is not out of bound - if err := pipelineSpec.ValidateParamArrayIndex(ctx, pr.Spec.Params); err != nil { + if err := resources.ValidateParamArrayIndex(ctx, pipelineSpec, pr.Spec.Params); err != nil { // This Run has failed, so we need to mark it as failed and stop reconciling it pr.Status.MarkFailed(ReasonParamArrayIndexingInvalid, "PipelineRun %s/%s failed validation: failed to validate Pipeline %s/%s's parameter which has an invalid index while referring to an array: %s", diff --git a/pkg/reconciler/pipelinerun/resources/validate_params.go b/pkg/reconciler/pipelinerun/resources/validate_params.go index 7659ca3a400..220d1c4876d 100644 --- a/pkg/reconciler/pipelinerun/resources/validate_params.go +++ b/pkg/reconciler/pipelinerun/resources/validate_params.go @@ -17,11 +17,14 @@ limitations under the License. package resources import ( + "context" "fmt" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/list" "github.com/tektoncd/pipeline/pkg/reconciler/taskrun" + "github.com/tektoncd/pipeline/pkg/substitution" + "k8s.io/apimachinery/pkg/util/sets" ) // ValidateParamTypesMatching validate that parameters in PipelineRun override corresponding parameters in Pipeline of the same type. @@ -110,3 +113,42 @@ func ValidateParameterTypesInMatrix(state PipelineRunState) error { } return nil } + +// 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 ValidateParamArrayIndex(ctx context.Context, ps *v1beta1.PipelineSpec, pipelineRunParams v1beta1.Params) error { + // Collect all array params lengths + arrayParamsLengths := ps.Params.ExtractDefaultParamArrayLengths() + for k, v := range pipelineRunParams.ExtractParamArrayLengths() { + arrayParamsLengths[k] = v + } + // extract all array indexing references, for example []{"$(params.array-params[1])"} + arrayIndexParamRefs := ps.GetIndexingReferencesToArrayParams().List() + + return validateOutofBoundArrayParams(arrayIndexParamRefs, 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 +} diff --git a/pkg/reconciler/pipelinerun/resources/validate_params_test.go b/pkg/reconciler/pipelinerun/resources/validate_params_test.go index b66378205a9..c06f0c52b3b 100644 --- a/pkg/reconciler/pipelinerun/resources/validate_params_test.go +++ b/pkg/reconciler/pipelinerun/resources/validate_params_test.go @@ -17,11 +17,16 @@ limitations under the License. package resources_test import ( + "context" + "fmt" "testing" + "github.com/google/go-cmp/cmp" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" resources "github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun/resources" + "github.com/tektoncd/pipeline/test/diff" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/selection" ) func TestValidateParamTypesMatching_Valid(t *testing.T) { @@ -418,3 +423,401 @@ func TestValidatePipelineParameterTypes(t *testing.T) { }) } } + +func TestValidateParamArrayIndex_valid(t *testing.T) { + for _, tt := range []struct { + name string + original v1beta1.PipelineSpec + params v1beta1.Params + }{{ + name: "single parameter", + 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.ParamTypeString}, + }, + Tasks: []v1beta1.PipelineTask{{ + Params: v1beta1.Params{ + {Name: "first-task-first-param", Value: *v1beta1.NewStructuredValues("$(params.first-param[1])")}, + {Name: "first-task-second-param", Value: *v1beta1.NewStructuredValues("$(params.second-param[0])")}, + {Name: "first-task-third-param", Value: *v1beta1.NewStructuredValues("static value")}, + }, + }}, + }, + params: v1beta1.Params{{Name: "second-param", Value: *v1beta1.NewStructuredValues("second-value", "second-value-again")}}, + }, { + name: "single parameter with when expression", + 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.ParamTypeString}, + }, + Tasks: []v1beta1.PipelineTask{{ + WhenExpressions: []v1beta1.WhenExpression{{ + Input: "$(params.first-param[1])", + Operator: selection.In, + Values: []string{"$(params.second-param[0])"}, + }}, + }}, + }, + params: v1beta1.Params{{Name: "second-param", Value: *v1beta1.NewStructuredValues("second-value", "second-value-again")}}, + }, { + name: "pipeline parameter nested inside task parameter", + 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, Default: v1beta1.NewStructuredValues("default-value", "default-value-again")}, + }, + Tasks: []v1beta1.PipelineTask{{ + Params: v1beta1.Params{ + {Name: "first-task-first-param", Value: *v1beta1.NewStructuredValues("$(input.workspace.$(params.first-param[0]))")}, + {Name: "first-task-second-param", Value: *v1beta1.NewStructuredValues("$(input.workspace.$(params.second-param[1]))")}, + }, + }}, + }, + params: nil, // no parameter values. + }, { + name: "array parameter", + original: v1beta1.PipelineSpec{ + Params: []v1beta1.ParamSpec{ + {Name: "first-param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default", "array", "value")}, + {Name: "second-param", Type: v1beta1.ParamTypeArray}, + }, + Tasks: []v1beta1.PipelineTask{{ + Params: v1beta1.Params{ + {Name: "first-task-first-param", Value: *v1beta1.NewStructuredValues("firstelement", "$(params.first-param)")}, + {Name: "first-task-second-param", Value: *v1beta1.NewStructuredValues("firstelement", "$(params.second-param[0])")}, + }, + }}, + }, + params: v1beta1.Params{ + {Name: "second-param", Value: *v1beta1.NewStructuredValues("second-value", "array")}, + }, + }, { + name: "parameter evaluation with final tasks", + 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}, + }, + Finally: []v1beta1.PipelineTask{{ + Params: v1beta1.Params{ + {Name: "final-task-first-param", Value: *v1beta1.NewStructuredValues("$(params.first-param[0])")}, + {Name: "final-task-second-param", Value: *v1beta1.NewStructuredValues("$(params.second-param[1])")}, + }, + WhenExpressions: v1beta1.WhenExpressions{{ + Input: "$(params.first-param[0])", + Operator: selection.In, + Values: []string{"$(params.second-param[1])"}, + }}, + }}, + }, + params: v1beta1.Params{{Name: "second-param", Value: *v1beta1.NewStructuredValues("second-value", "second-value-again")}}, + }, { + name: "parameter evaluation with both tasks and final tasks", + 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.Params{ + {Name: "final-task-first-param", Value: *v1beta1.NewStructuredValues("$(params.first-param[0])")}, + {Name: "final-task-second-param", Value: *v1beta1.NewStructuredValues("$(params.second-param[1])")}, + }, + }}, + Finally: []v1beta1.PipelineTask{{ + Params: v1beta1.Params{ + {Name: "final-task-first-param", Value: *v1beta1.NewStructuredValues("$(params.first-param[0])")}, + {Name: "final-task-second-param", Value: *v1beta1.NewStructuredValues("$(params.second-param[1])")}, + }, + WhenExpressions: v1beta1.WhenExpressions{{ + Input: "$(params.first-param[0])", + Operator: selection.In, + Values: []string{"$(params.second-param[1])"}, + }}, + }}, + }, + params: v1beta1.Params{{Name: "second-param", Value: *v1beta1.NewStructuredValues("second-value", "second-value-again")}}, + }, { + name: "parameter references with bracket notation and special characters", + 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}, + {Name: "third.param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default-value", "default-value-again")}, + {Name: "fourth/param", Type: v1beta1.ParamTypeArray}, + }, + Tasks: []v1beta1.PipelineTask{{ + Params: v1beta1.Params{ + {Name: "first-task-first-param", Value: *v1beta1.NewStructuredValues(`$(params["first.param"][0])`)}, + {Name: "first-task-second-param", Value: *v1beta1.NewStructuredValues(`$(params["second/param"][0])`)}, + {Name: "first-task-third-param", Value: *v1beta1.NewStructuredValues(`$(params['third.param'][1])`)}, + {Name: "first-task-fourth-param", Value: *v1beta1.NewStructuredValues(`$(params['fourth/param'][1])`)}, + {Name: "first-task-fifth-param", Value: *v1beta1.NewStructuredValues("static value")}, + }, + }}, + }, + params: v1beta1.Params{ + {Name: "second/param", Value: *v1beta1.NewStructuredValues("second-value", "second-value-again")}, + {Name: "fourth/param", Value: *v1beta1.NewStructuredValues("fourth-value", "fourth-value-again")}, + }, + }, { + name: "single parameter in workspace subpath", + 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.Params{ + {Name: "first-task-first-param", Value: *v1beta1.NewStructuredValues("$(params.first-param[0])")}, + {Name: "first-task-second-param", Value: *v1beta1.NewStructuredValues("static value")}, + }, + Workspaces: []v1beta1.WorkspacePipelineTaskBinding{ + { + Name: "first-workspace", + Workspace: "first-workspace", + SubPath: "$(params.second-param[1])", + }, + }, + }}, + }, + params: v1beta1.Params{{Name: "second-param", Value: *v1beta1.NewStructuredValues("second-value", "second-value-again")}}, + }, + } { + tt := tt // capture range variable + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := resources.ValidateParamArrayIndex(context.Background(), &tt.original, tt.params) + if err != nil { + t.Errorf("ValidateParamArrayIndex() got err %s", err) + } + }) + } +} + +func TestValidateParamArrayIndex_invalid(t *testing.T) { + for _, tt := range []struct { + name string + original v1beta1.PipelineSpec + params v1beta1.Params + expected error + }{{ + name: "single parameter reference out of bound", + 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.ParamTypeString}, + }, + Tasks: []v1beta1.PipelineTask{{ + Params: v1beta1.Params{ + {Name: "first-task-first-param", Value: *v1beta1.NewStructuredValues("$(params.first-param[2])")}, + {Name: "first-task-second-param", Value: *v1beta1.NewStructuredValues("$(params.second-param[2])")}, + {Name: "first-task-third-param", Value: *v1beta1.NewStructuredValues("static value")}, + }, + }}, + }, + params: v1beta1.Params{{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[2])]"), + }, { + name: "single parameter reference with when expression out of bound", + 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.ParamTypeString}, + }, + Tasks: []v1beta1.PipelineTask{{ + WhenExpressions: []v1beta1.WhenExpression{{ + Input: "$(params.first-param[2])", + Operator: selection.In, + Values: []string{"$(params.second-param[2])"}, + }}, + }}, + }, + params: v1beta1.Params{{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[2])]"), + }, { + name: "pipeline parameter reference nested inside task parameter out of bound", + 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, Default: v1beta1.NewStructuredValues("default-value", "default-value-again")}, + }, + Tasks: []v1beta1.PipelineTask{{ + Params: v1beta1.Params{ + {Name: "first-task-first-param", Value: *v1beta1.NewStructuredValues("$(input.workspace.$(params.first-param[2]))")}, + {Name: "first-task-second-param", Value: *v1beta1.NewStructuredValues("$(input.workspace.$(params.second-param[2]))")}, + }, + }}, + }, + params: nil, // no parameter values. + expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[2])]"), + }, { + name: "array parameter reference out of bound", + original: v1beta1.PipelineSpec{ + Params: []v1beta1.ParamSpec{ + {Name: "first-param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default", "array", "value")}, + {Name: "second-param", Type: v1beta1.ParamTypeArray}, + }, + Tasks: []v1beta1.PipelineTask{{ + Params: v1beta1.Params{ + {Name: "first-task-first-param", Value: *v1beta1.NewStructuredValues("firstelement", "$(params.first-param[3])")}, + {Name: "first-task-second-param", Value: *v1beta1.NewStructuredValues("firstelement", "$(params.second-param[4])")}, + }, + }}, + }, + params: v1beta1.Params{ + {Name: "second-param", Value: *v1beta1.NewStructuredValues("second-value", "array")}, + }, + expected: fmt.Errorf("non-existent param references:[$(params.first-param[3]) $(params.second-param[4])]"), + }, { + name: "object parameter reference out of bound", + original: v1beta1.PipelineSpec{ + Params: []v1beta1.ParamSpec{ + {Name: "first-param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default", "array", "value")}, + {Name: "second-param", Type: v1beta1.ParamTypeArray}, + }, + Tasks: []v1beta1.PipelineTask{{ + Params: v1beta1.Params{ + {Name: "first-task-first-param", Value: *v1beta1.NewObject(map[string]string{ + "val1": "$(params.first-param[4])", + "val2": "$(params.second-param[4])", + })}, + }, + }}, + }, + params: v1beta1.Params{ + {Name: "second-param", Value: *v1beta1.NewStructuredValues("second-value", "array")}, + }, + expected: fmt.Errorf("non-existent param references:[$(params.first-param[4]) $(params.second-param[4])]"), + }, { + name: "parameter evaluation with final tasks reference out of bound", + 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}, + }, + Finally: []v1beta1.PipelineTask{{ + Params: v1beta1.Params{ + {Name: "final-task-first-param", Value: *v1beta1.NewStructuredValues("$(params.first-param[2])")}, + {Name: "final-task-second-param", Value: *v1beta1.NewStructuredValues("$(params.second-param[2])")}, + }, + WhenExpressions: v1beta1.WhenExpressions{{ + Input: "$(params.first-param[0])", + Operator: selection.In, + Values: []string{"$(params.second-param[1])"}, + }}, + }}, + }, + params: v1beta1.Params{{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[2])]"), + }, { + name: "parameter evaluation with both tasks and final tasks reference out of bound", + 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.Params{ + {Name: "final-task-first-param", Value: *v1beta1.NewStructuredValues("$(params.first-param[2])")}, + {Name: "final-task-second-param", Value: *v1beta1.NewStructuredValues("$(params.second-param[2])")}, + }, + }}, + Finally: []v1beta1.PipelineTask{{ + Params: v1beta1.Params{ + {Name: "final-task-first-param", Value: *v1beta1.NewStructuredValues("$(params.first-param[3])")}, + {Name: "final-task-second-param", Value: *v1beta1.NewStructuredValues("$(params.second-param[3])")}, + }, + WhenExpressions: v1beta1.WhenExpressions{{ + Input: "$(params.first-param[2])", + Operator: selection.In, + Values: []string{"$(params.second-param[2])"}, + }}, + Matrix: &v1beta1.Matrix{ + Params: v1beta1.Params{ + {Name: "final-task-first-param", Value: *v1beta1.NewStructuredValues("$(params.first-param[4])")}, + {Name: "final-task-second-param", Value: *v1beta1.NewStructuredValues("$(params.second-param[4])")}, + }}, + }}, + }, + params: v1beta1.Params{{Name: "second-param", Value: *v1beta1.NewStructuredValues("second-value", "second-value-again")}}, + expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.first-param[3]) $(params.first-param[4]) $(params.second-param[2]) $(params.second-param[3]) $(params.second-param[4])]"), + }, { + name: "parameter in matrix reference out of bound", + 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{{ + Matrix: &v1beta1.Matrix{ + Params: v1beta1.Params{ + {Name: "first-task-first-param", Value: *v1beta1.NewStructuredValues("$(params.first-param[2])")}, + {Name: "first-task-second-param", Value: *v1beta1.NewStructuredValues("static value")}, + }, + }, + }}, + }, + params: v1beta1.Params{{Name: "second-param", Value: *v1beta1.NewStructuredValues("second-value", "second-value-again")}}, + expected: fmt.Errorf("non-existent param references:[$(params.first-param[2])]"), + }, { + name: "parameter references with bracket notation and special characters reference out of bound", + 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}, + {Name: "third.param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default-value", "default-value-again")}, + {Name: "fourth/param", Type: v1beta1.ParamTypeArray}, + }, + Tasks: []v1beta1.PipelineTask{{ + Params: v1beta1.Params{ + {Name: "first-task-first-param", Value: *v1beta1.NewStructuredValues(`$(params["first.param"][2])`)}, + {Name: "first-task-second-param", Value: *v1beta1.NewStructuredValues(`$(params["second/param"][2])`)}, + {Name: "first-task-third-param", Value: *v1beta1.NewStructuredValues(`$(params['third.param'][2])`)}, + {Name: "first-task-fourth-param", Value: *v1beta1.NewStructuredValues(`$(params['fourth/param'][2])`)}, + {Name: "first-task-fifth-param", Value: *v1beta1.NewStructuredValues("static value")}, + }, + }}, + }, + params: v1beta1.Params{ + {Name: "second/param", Value: *v1beta1.NewStructuredValues("second-value", "second-value-again")}, + {Name: "fourth/param", Value: *v1beta1.NewStructuredValues("fourth-value", "fourth-value-again")}, + }, + expected: fmt.Errorf("non-existent param references:[$(params[\"first.param\"][2]) $(params[\"second/param\"][2]) $(params['fourth/param'][2]) $(params['third.param'][2])]"), + }, { + name: "single parameter in workspace subpath reference out of bound", + 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.Params{ + {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.Params{{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])]"), + }, + } { + tt := tt // capture range variable + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := resources.ValidateParamArrayIndex(context.Background(), &tt.original, tt.params) + if d := cmp.Diff(tt.expected.Error(), err.Error()); d != "" { + t.Errorf("ValidateParamArrayIndex() errors diff %s", diff.PrintWantGot(d)) + } + }) + } +} diff --git a/pkg/reconciler/taskrun/resources/validate_params.go b/pkg/reconciler/taskrun/resources/validate_params.go new file mode 100644 index 00000000000..edfaae8f6ca --- /dev/null +++ b/pkg/reconciler/taskrun/resources/validate_params.go @@ -0,0 +1,51 @@ +package resources + +import ( + "context" + "fmt" + + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + "github.com/tektoncd/pipeline/pkg/substitution" + "k8s.io/apimachinery/pkg/util/sets" +) + +// 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. +// - `trParams` are params from taskrun. +// - `taskSpec` contains params declarations. +func ValidateParamArrayIndex(ctx context.Context, ts *v1beta1.TaskSpec, params v1beta1.Params) error { + // Collect all array params lengths + arrayParamsLengths := ts.Params.ExtractDefaultParamArrayLengths() + for k, v := range params.ExtractParamArrayLengths() { + arrayParamsLengths[k] = v + } + // extract all array indexing references, for example []{"$(params.array-params[1])"} + arrayIndexParamRefs := ts.GetIndexingReferencesToArrayParams() + + return validateOutofBoundArrayParams(arrayIndexParamRefs.List(), 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 +} diff --git a/pkg/reconciler/taskrun/resources/validate_params_test.go b/pkg/reconciler/taskrun/resources/validate_params_test.go new file mode 100644 index 00000000000..ac454773dfb --- /dev/null +++ b/pkg/reconciler/taskrun/resources/validate_params_test.go @@ -0,0 +1,208 @@ +package resources_test + +import ( + "context" + "fmt" + "strings" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources" + "github.com/tektoncd/pipeline/test/diff" + corev1 "k8s.io/api/core/v1" +) + +func TestValidateParamArrayIndex(t *testing.T) { + stepsInvalidReferences := []string{} + for i := 10; i <= 26; i++ { + stepsInvalidReferences = append(stepsInvalidReferences, fmt.Sprintf("$(params.array-params[%d])", i)) + } + volumesInvalidReferences := []string{} + for i := 10; i <= 22; i++ { + volumesInvalidReferences = append(volumesInvalidReferences, fmt.Sprintf("$(params.array-params[%d])", i)) + } + + tcs := []struct { + name string + params v1beta1.Params + taskspec *v1beta1.TaskSpec + expectedError error + }{{ + name: "steps reference invalid", + params: v1beta1.Params{{ + Name: "array-params", + Value: *v1beta1.NewStructuredValues("bar", "foo"), + }}, + taskspec: &v1beta1.TaskSpec{ + Params: []v1beta1.ParamSpec{{ + Name: "array-params", + Default: v1beta1.NewStructuredValues("bar", "foo"), + }}, + Steps: []v1beta1.Step{{ + Name: "$(params.array-params[10])", + Image: "$(params.array-params[11])", + Command: []string{"$(params.array-params[12])"}, + Args: []string{"$(params.array-params[13])"}, + Script: "echo $(params.array-params[14])", + Env: []corev1.EnvVar{{ + Value: "$(params.array-params[15])", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + Key: "$(params.array-params[16])", + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(params.array-params[17])", + }, + }, + ConfigMapKeyRef: &corev1.ConfigMapKeySelector{ + Key: "$(params.array-params[18])", + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(params.array-params[19])", + }, + }, + }, + }}, + EnvFrom: []corev1.EnvFromSource{{ + Prefix: "$(params.array-params[20])", + ConfigMapRef: &corev1.ConfigMapEnvSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(params.array-params[21])", + }, + }, + SecretRef: &corev1.SecretEnvSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(params.array-params[22])", + }, + }, + }}, + WorkingDir: "$(params.array-params[23])", + VolumeMounts: []corev1.VolumeMount{{ + Name: "$(params.array-params[24])", + MountPath: "$(params.array-params[25])", + SubPath: "$(params.array-params[26])", + }}, + }}, + }, + expectedError: fmt.Errorf("non-existent param references:[%v]", strings.Join(stepsInvalidReferences, " ")), + }, { + name: "stepTemplate reference invalid", + params: v1beta1.Params{{ + Name: "array-params", + Value: *v1beta1.NewStructuredValues("bar", "foo"), + }}, + taskspec: &v1beta1.TaskSpec{ + Params: []v1beta1.ParamSpec{{ + Name: "array-params", + Default: v1beta1.NewStructuredValues("bar", "foo"), + }}, + StepTemplate: &v1beta1.StepTemplate{ + Image: "$(params.array-params[3])", + }, + }, + expectedError: fmt.Errorf("non-existent param references:[%v]", "$(params.array-params[3])"), + }, { + name: "volumes reference invalid", + params: v1beta1.Params{{ + Name: "array-params", + Value: *v1beta1.NewStructuredValues("bar", "foo"), + }}, + taskspec: &v1beta1.TaskSpec{ + Params: []v1beta1.ParamSpec{{ + Name: "array-params", + Default: v1beta1.NewStructuredValues("bar", "foo"), + }}, + Volumes: []corev1.Volume{{ + Name: "$(params.array-params[10])", + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(params.array-params[11])", + }, + Items: []corev1.KeyToPath{{ + Key: "$(params.array-params[12])", + Path: "$(params.array-params[13])", + }, + }, + }, + Secret: &corev1.SecretVolumeSource{ + SecretName: "$(params.array-params[14])", + Items: []corev1.KeyToPath{{ + Key: "$(params.array-params[15])", + Path: "$(params.array-params[16])", + }}, + }, + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: "$(params.array-params[17])", + }, + Projected: &corev1.ProjectedVolumeSource{ + Sources: []corev1.VolumeProjection{{ + ConfigMap: &corev1.ConfigMapProjection{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(params.array-params[18])", + }, + }, + Secret: &corev1.SecretProjection{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(params.array-params[19])", + }, + }, + ServiceAccountToken: &corev1.ServiceAccountTokenProjection{ + Audience: "$(params.array-params[20])", + }, + }}, + }, + CSI: &corev1.CSIVolumeSource{ + NodePublishSecretRef: &corev1.LocalObjectReference{ + Name: "$(params.array-params[21])", + }, + VolumeAttributes: map[string]string{"key": "$(params.array-params[22])"}, + }, + }, + }, + }, + }, + expectedError: fmt.Errorf("non-existent param references:[%v]", strings.Join(volumesInvalidReferences, " ")), + }, { + name: "workspaces reference invalid", + params: v1beta1.Params{{ + Name: "array-params", + Value: *v1beta1.NewStructuredValues("bar", "foo"), + }}, + taskspec: &v1beta1.TaskSpec{ + Params: []v1beta1.ParamSpec{{ + Name: "array-params", + Default: v1beta1.NewStructuredValues("bar", "foo"), + }}, + Workspaces: []v1beta1.WorkspaceDeclaration{{ + MountPath: "$(params.array-params[3])", + }}, + }, + expectedError: fmt.Errorf("non-existent param references:[%v]", "$(params.array-params[3])"), + }, { + name: "sidecar reference invalid", + params: v1beta1.Params{{ + Name: "array-params", + Value: *v1beta1.NewStructuredValues("bar", "foo"), + }}, + taskspec: &v1beta1.TaskSpec{ + Params: []v1beta1.ParamSpec{{ + Name: "array-params", + Default: v1beta1.NewStructuredValues("bar", "foo"), + }}, + Sidecars: []v1beta1.Sidecar{{ + Script: "$(params.array-params[3])", + }, + }, + }, + expectedError: fmt.Errorf("non-existent param references:[%v]", "$(params.array-params[3])"), + }, + } + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + err := resources.ValidateParamArrayIndex(context.Background(), tc.taskspec, tc.params) + if d := cmp.Diff(tc.expectedError.Error(), err.Error()); d != "" { + t.Errorf("validateParamArrayIndex() errors diff %s", diff.PrintWantGot(d)) + } + }) + } +} diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index bbcf5436553..4065264416f 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -370,7 +370,7 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1beta1.TaskRun) (*v1beta1 return nil, nil, controller.NewPermanentError(err) } - if err := rtr.TaskSpec.ValidateParamArrayIndex(ctx, tr.Spec.Params); err != nil { + if err := resources.ValidateParamArrayIndex(ctx, rtr.TaskSpec, tr.Spec.Params); err != nil { logger.Errorf("TaskRun %q Param references are invalid: %v", tr.Name, err) tr.Status.MarkResourceFailed(podconvert.ReasonFailedValidation, err) return nil, nil, controller.NewPermanentError(err)