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/param_types_test.go b/pkg/apis/pipeline/v1/param_types_test.go index 484c0941915..d01be3d8199 100644 --- a/pkg/apis/pipeline/v1/param_types_test.go +++ b/pkg/apis/pipeline/v1/param_types_test.go @@ -24,6 +24,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "github.com/tektoncd/pipeline/test/diff" "k8s.io/apimachinery/pkg/util/sets" @@ -564,3 +565,91 @@ func TestParams_ReplaceVariables(t *testing.T) { }) } } + +func TestExtractParamArrayLengths(t *testing.T) { + tcs := []struct { + name string + params v1.Params + want map[string]int + }{{ + name: "string params", + params: v1.Params{{Name: "foo", Value: v1.ParamValue{StringVal: "bar", Type: v1.ParamTypeString}}}, + want: nil, + }, { + name: "one array param", + params: v1.Params{{Name: "foo", Value: v1.ParamValue{ArrayVal: []string{"bar", "baz"}, Type: v1.ParamTypeArray}}}, + want: map[string]int{"foo": 2}, + }, { + name: "object params", + params: v1.Params{{Name: "foo", Value: v1.ParamValue{ObjectVal: map[string]string{"bar": "baz"}, Type: v1.ParamTypeObject}}}, + want: nil, + }, { + name: "multiple array params", + params: v1.Params{ + {Name: "foo", Value: v1.ParamValue{ArrayVal: []string{"bar", "baz"}, Type: v1.ParamTypeArray}}, + {Name: "abc", Value: v1.ParamValue{ArrayVal: []string{"123", "456", "789"}, Type: v1.ParamTypeArray}}, + {Name: "empty", Value: v1.ParamValue{ArrayVal: []string{}, Type: v1.ParamTypeArray}}, + }, + want: map[string]int{"foo": 2, "abc": 3, "empty": 0}, + }, { + name: "mixed param types", + params: v1.Params{ + {Name: "foo", Value: v1.ParamValue{StringVal: "abc", Type: v1.ParamTypeString}}, + {Name: "bar", Value: v1.ParamValue{ArrayVal: []string{"def", "ghi"}, Type: v1.ParamTypeArray}}, + {Name: "baz", Value: v1.ParamValue{ObjectVal: map[string]string{"jkl": "mno"}, Type: v1.ParamTypeObject}}, + }, + want: map[string]int{"bar": 2}, + }} + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + got := tc.params.ExtractParamArrayLengths() + if d := cmp.Diff(tc.want, got, cmpopts.EquateEmpty()); d != "" { + t.Errorf("wrong param array lengths: %s", d) + } + }) + } +} + +func TestExtractDefaultParamArrayLengths(t *testing.T) { + tcs := []struct { + name string + params v1.ParamSpecs + want map[string]int + }{{ + name: "string params", + params: v1.ParamSpecs{{Name: "foo", Default: &v1.ParamValue{StringVal: "bar", Type: v1.ParamTypeString}}}, + want: nil, + }, { + name: "one array param", + params: v1.ParamSpecs{{Name: "foo", Default: &v1.ParamValue{ArrayVal: []string{"bar", "baz"}, Type: v1.ParamTypeArray}}}, + want: map[string]int{"foo": 2}, + }, { + name: "object params", + params: v1.ParamSpecs{{Name: "foo", Default: &v1.ParamValue{ObjectVal: map[string]string{"bar": "baz"}, Type: v1.ParamTypeObject}}}, + want: nil, + }, { + name: "multiple array params", + params: v1.ParamSpecs{ + {Name: "foo", Default: &v1.ParamValue{ArrayVal: []string{"bar", "baz"}, Type: v1.ParamTypeArray}}, + {Name: "abc", Default: &v1.ParamValue{ArrayVal: []string{"123", "456", "789"}, Type: v1.ParamTypeArray}}, + {Name: "empty", Default: &v1.ParamValue{ArrayVal: []string{}, Type: v1.ParamTypeArray}}, + }, + want: map[string]int{"foo": 2, "abc": 3, "empty": 0}, + }, { + name: "mixed param types", + params: v1.ParamSpecs{ + {Name: "foo", Default: &v1.ParamValue{StringVal: "abc", Type: v1.ParamTypeString}}, + {Name: "bar", Default: &v1.ParamValue{ArrayVal: []string{"def", "ghi"}, Type: v1.ParamTypeArray}}, + {Name: "baz", Default: &v1.ParamValue{ObjectVal: map[string]string{"jkl": "mno"}, Type: v1.ParamTypeObject}}, + }, + want: map[string]int{"bar": 2}, + }} + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + got := tc.params.ExtractDefaultParamArrayLengths() + if d := cmp.Diff(tc.want, got, cmpopts.EquateEmpty()); d != "" { + t.Errorf("wrong default param array lengths: %s", d) + } + }) + } +} diff --git a/pkg/apis/pipeline/v1/pipeline_validation.go b/pkg/apis/pipeline/v1/pipeline_validation.go index 4cb892e6062..5f2309b51d4 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1/pipeline_validation.go @@ -758,21 +758,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/v1/pipeline_validation_test.go b/pkg/apis/pipeline/v1/pipeline_validation_test.go index 9b5bc3c2948..3e35c14ee56 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1/pipeline_validation_test.go @@ -3340,183 +3340,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 @@ -3716,231 +3539,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 22e546c3846..9b56bf6789a 100644 --- a/pkg/apis/pipeline/v1/task_validation.go +++ b/pkg/apis/pipeline/v1/task_validation.go @@ -607,24 +607,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/v1/task_validation_test.go b/pkg/apis/pipeline/v1/task_validation_test.go index d9a680822f3..060883000a4 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" @@ -1675,200 +1674,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 TestTaskBetaFields(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/param_types_test.go b/pkg/apis/pipeline/v1beta1/param_types_test.go index 180dcd14f82..eedb2132640 100644 --- a/pkg/apis/pipeline/v1beta1/param_types_test.go +++ b/pkg/apis/pipeline/v1beta1/param_types_test.go @@ -24,6 +24,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/test/diff" "k8s.io/apimachinery/pkg/util/sets" @@ -595,3 +596,91 @@ func TestParams_ReplaceVariables(t *testing.T) { }) } } + +func TestExtractParamArrayLengths(t *testing.T) { + tcs := []struct { + name string + params v1beta1.Params + want map[string]int + }{{ + name: "string params", + params: v1beta1.Params{{Name: "foo", Value: v1beta1.ParamValue{StringVal: "bar", Type: v1beta1.ParamTypeString}}}, + want: nil, + }, { + name: "one array param", + params: v1beta1.Params{{Name: "foo", Value: v1beta1.ParamValue{ArrayVal: []string{"bar", "baz"}, Type: v1beta1.ParamTypeArray}}}, + want: map[string]int{"foo": 2}, + }, { + name: "object params", + params: v1beta1.Params{{Name: "foo", Value: v1beta1.ParamValue{ObjectVal: map[string]string{"bar": "baz"}, Type: v1beta1.ParamTypeObject}}}, + want: nil, + }, { + name: "multiple array params", + params: v1beta1.Params{ + {Name: "foo", Value: v1beta1.ParamValue{ArrayVal: []string{"bar", "baz"}, Type: v1beta1.ParamTypeArray}}, + {Name: "abc", Value: v1beta1.ParamValue{ArrayVal: []string{"123", "456", "789"}, Type: v1beta1.ParamTypeArray}}, + {Name: "empty", Value: v1beta1.ParamValue{ArrayVal: []string{}, Type: v1beta1.ParamTypeArray}}, + }, + want: map[string]int{"foo": 2, "abc": 3, "empty": 0}, + }, { + name: "mixed param types", + params: v1beta1.Params{ + {Name: "foo", Value: v1beta1.ParamValue{StringVal: "abc", Type: v1beta1.ParamTypeString}}, + {Name: "bar", Value: v1beta1.ParamValue{ArrayVal: []string{"def", "ghi"}, Type: v1beta1.ParamTypeArray}}, + {Name: "baz", Value: v1beta1.ParamValue{ObjectVal: map[string]string{"jkl": "mno"}, Type: v1beta1.ParamTypeObject}}, + }, + want: map[string]int{"bar": 2}, + }} + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + got := tc.params.ExtractParamArrayLengths() + if d := cmp.Diff(tc.want, got, cmpopts.EquateEmpty()); d != "" { + t.Errorf("wrong param array lengths: %s", d) + } + }) + } +} + +func TestExtractDefaultParamArrayLengths(t *testing.T) { + tcs := []struct { + name string + params v1beta1.ParamSpecs + want map[string]int + }{{ + name: "string params", + params: v1beta1.ParamSpecs{{Name: "foo", Default: &v1beta1.ParamValue{StringVal: "bar", Type: v1beta1.ParamTypeString}}}, + want: nil, + }, { + name: "one array param", + params: v1beta1.ParamSpecs{{Name: "foo", Default: &v1beta1.ParamValue{ArrayVal: []string{"bar", "baz"}, Type: v1beta1.ParamTypeArray}}}, + want: map[string]int{"foo": 2}, + }, { + name: "object params", + params: v1beta1.ParamSpecs{{Name: "foo", Default: &v1beta1.ParamValue{ObjectVal: map[string]string{"bar": "baz"}, Type: v1beta1.ParamTypeObject}}}, + want: nil, + }, { + name: "multiple array params", + params: v1beta1.ParamSpecs{ + {Name: "foo", Default: &v1beta1.ParamValue{ArrayVal: []string{"bar", "baz"}, Type: v1beta1.ParamTypeArray}}, + {Name: "abc", Default: &v1beta1.ParamValue{ArrayVal: []string{"123", "456", "789"}, Type: v1beta1.ParamTypeArray}}, + {Name: "empty", Default: &v1beta1.ParamValue{ArrayVal: []string{}, Type: v1beta1.ParamTypeArray}}, + }, + want: map[string]int{"foo": 2, "abc": 3, "empty": 0}, + }, { + name: "mixed param types", + params: v1beta1.ParamSpecs{ + {Name: "foo", Default: &v1beta1.ParamValue{StringVal: "abc", Type: v1beta1.ParamTypeString}}, + {Name: "bar", Default: &v1beta1.ParamValue{ArrayVal: []string{"def", "ghi"}, Type: v1beta1.ParamTypeArray}}, + {Name: "baz", Default: &v1beta1.ParamValue{ObjectVal: map[string]string{"jkl": "mno"}, Type: v1beta1.ParamTypeObject}}, + }, + want: map[string]int{"bar": 2}, + }} + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + got := tc.params.ExtractDefaultParamArrayLengths() + if d := cmp.Diff(tc.want, got, cmpopts.EquateEmpty()); d != "" { + t.Errorf("wrong default param array lengths: %s", d) + } + }) + } +} diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go index 2e2ecadf1e8..a5df46ace0a 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -733,21 +733,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 f35aef60ea2..0018425e69e 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go @@ -3399,409 +3399,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 9d289544a34..e9e6d2832df 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation.go +++ b/pkg/apis/pipeline/v1beta1/task_validation.go @@ -576,23 +576,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 93d69216fc8..3272a8739e4 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" @@ -1687,200 +1686,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 5959c989c9d..dee03436b64 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -484,7 +484,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(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..ad30fd8196f 100644 --- a/pkg/reconciler/pipelinerun/resources/validate_params.go +++ b/pkg/reconciler/pipelinerun/resources/validate_params.go @@ -22,6 +22,7 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/list" "github.com/tektoncd/pipeline/pkg/reconciler/taskrun" + trresources "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources" ) // ValidateParamTypesMatching validate that parameters in PipelineRun override corresponding parameters in Pipeline of the same type. @@ -110,3 +111,10 @@ 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(ps *v1beta1.PipelineSpec, params v1beta1.Params) error { + return trresources.ValidateOutOfBoundArrayParams(ps.Params, params, ps.GetIndexingReferencesToArrayParams()) +} diff --git a/pkg/reconciler/pipelinerun/resources/validate_params_test.go b/pkg/reconciler/pipelinerun/resources/validate_params_test.go index b66378205a9..ea7a8cba6f9 100644 --- a/pkg/reconciler/pipelinerun/resources/validate_params_test.go +++ b/pkg/reconciler/pipelinerun/resources/validate_params_test.go @@ -17,11 +17,15 @@ limitations under the License. package resources_test import ( + "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 +422,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(&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(&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..372163cec04 --- /dev/null +++ b/pkg/reconciler/taskrun/resources/validate_params.go @@ -0,0 +1,47 @@ +package resources + +import ( + "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. +// - `params` are params from taskrun. +// - `ts` contains params declarations and references to array params. +func ValidateParamArrayIndex(ts *v1beta1.TaskSpec, params v1beta1.Params) error { + return ValidateOutOfBoundArrayParams(ts.Params, params, ts.GetIndexingReferencesToArrayParams()) +} + +// ValidateOutOfBoundArrayParams returns an error if the array indexing params are out of bounds, +// based on the param declarations, the parameters passed in at runtime, and the indexing references +// to array params from a task or pipeline spec. +// Example of arrayIndexingReferences: ["$(params.a-array-param[1])", "$(params.b-array-param[2])"] +func ValidateOutOfBoundArrayParams(declarations v1beta1.ParamSpecs, params v1beta1.Params, arrayIndexingReferences sets.String) error { + arrayParamLengths := declarations.ExtractDefaultParamArrayLengths() + for k, v := range params.ExtractParamArrayLengths() { + arrayParamLengths[k] = v + } + outofBoundParams := sets.String{} + for val := range arrayIndexingReferences { + 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 := arrayParamLengths[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..3e471fa204d --- /dev/null +++ b/pkg/reconciler/taskrun/resources/validate_params_test.go @@ -0,0 +1,207 @@ +package resources_test + +import ( + "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(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 a75a00f59cf..100ee1dd258 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -369,7 +369,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(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)