diff --git a/pkg/apis/pipeline/v1/pipeline_validation.go b/pkg/apis/pipeline/v1/pipeline_validation.go index db7dd5edd47..d15619af3d9 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1/pipeline_validation.go @@ -429,10 +429,6 @@ func validateResultsFromMatrixedPipelineTasksNotConsumed(tasks []PipelineTask, f // error is returned when the array indexing reference is out of bound of the array param // e.g. if a param reference of $(params.array-param[2]) and the array param is of length 2. func (ps *PipelineSpec) ValidateParamArrayIndex(ctx context.Context, params Params) error { - if !config.CheckAlphaOrBetaAPIFields(ctx) { - return nil - } - // Collect all array params lengths arrayParamsLengths := ps.Params.extractParamArrayLengths() for k, v := range params.extractParamArrayLengths() { @@ -470,5 +466,10 @@ func (ps *PipelineSpec) ValidateParamArrayIndex(ctx context.Context, params Para arrayIndexParamRefs = append(arrayIndexParamRefs, extractArrayIndexingParamRefs(p)...) } + // if there are array indexing param references, the api feature gate needs to be set to `alpha` or `beta` + if len(arrayIndexParamRefs) > 0 && !config.CheckAlphaOrBetaAPIFields(ctx) { + return fmt.Errorf(`indexing into array params: %v require "enable-api-fields" feature gate to be "alpha" or "beta"`, arrayIndexParamRefs) + } + return validateOutofBoundArrayParams(arrayIndexParamRefs, arrayParamsLengths) } diff --git a/pkg/apis/pipeline/v1/pipeline_validation_test.go b/pkg/apis/pipeline/v1/pipeline_validation_test.go index d6f6efe4eeb..91236e130dd 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1/pipeline_validation_test.go @@ -3276,9 +3276,10 @@ func TestValidateParamArrayIndex_valid(t *testing.T) { cfg.FeatureFlags.EnableAPIFields = config.BetaAPIFields ctx = config.ToContext(ctx, cfg) for _, tt := range []struct { - name string - original PipelineSpec - params []Param + name string + original PipelineSpec + params []Param + apiFields string }{{ name: "single parameter", original: PipelineSpec{ @@ -3434,9 +3435,29 @@ func TestValidateParamArrayIndex_valid(t *testing.T) { }}, }, params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + }, { + name: "validation with alpha gate", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeString}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {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: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + apiFields: config.AlphaAPIFields, }, } { tt := tt // capture range variable + if tt.apiFields == config.AlphaAPIFields { + ctx = config.EnableAlphaAPIFields(ctx) + } t.Run(tt.name, func(t *testing.T) { t.Parallel() err := tt.original.ValidateParamArrayIndex(ctx, tt.params) @@ -3453,10 +3474,11 @@ func TestValidateParamArrayIndex_invalid(t *testing.T) { cfg.FeatureFlags.EnableAPIFields = config.BetaAPIFields ctx = config.ToContext(ctx, cfg) for _, tt := range []struct { - name string - original PipelineSpec - params []Param - expected error + name string + original PipelineSpec + params []Param + apiFields string + expected error }{{ name: "single parameter reference out of bound", original: PipelineSpec{ @@ -3640,11 +3662,31 @@ func TestValidateParamArrayIndex_invalid(t *testing.T) { }, params: []Param{{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])]"), + }, { + name: "stable gate not allowed", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeString}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {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: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + apiFields: config.StableAPIFields, + expected: fmt.Errorf(`indexing into array params: %v require "enable-api-fields" feature gate to be "alpha" or "beta"`, []string{"$(params.first-param[2])", "$(params.second-param[2])"}), }, } { tt := tt // capture range variable t.Run(tt.name, func(t *testing.T) { - t.Parallel() + if tt.apiFields == config.StableAPIFields { + ctx = config.EnableStableAPIFields(ctx) + } 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)) diff --git a/pkg/apis/pipeline/v1/task_validation.go b/pkg/apis/pipeline/v1/task_validation.go index fdf30e52dcb..8f98ac39501 100644 --- a/pkg/apis/pipeline/v1/task_validation.go +++ b/pkg/apis/pipeline/v1/task_validation.go @@ -607,11 +607,6 @@ func isParamRefs(s string) bool { // - `trParams` are params from taskrun. // - `taskSpec` contains params declarations. func (ts *TaskSpec) ValidateParamArrayIndex(ctx context.Context, params Params) error { - cfg := config.FromContextOrDefaults(ctx) - if cfg.FeatureFlags.EnableAPIFields != config.AlphaAPIFields { - return nil - } - // Collect all array params lengths arrayParamsLengths := ts.Params.extractParamArrayLengths() for k, v := range params.extractParamArrayLengths() { @@ -634,5 +629,10 @@ func (ts *TaskSpec) ValidateParamArrayIndex(ctx context.Context, params Params) arrayIndexParamRefs = append(arrayIndexParamRefs, extractArrayIndexingParamRefs(p)...) } + // if there are array indexing param references, the api feature gate needs to be set to `alpha` or `beta` + if len(arrayIndexParamRefs) > 0 && !config.CheckAlphaOrBetaAPIFields(ctx) { + return fmt.Errorf(`indexing into array params: %v require "enable-api-fields" feature gate to be "alpha" or "beta"`, arrayIndexParamRefs) + } + return validateOutofBoundArrayParams(arrayIndexParamRefs, arrayParamsLengths) } diff --git a/pkg/apis/pipeline/v1/task_validation_test.go b/pkg/apis/pipeline/v1/task_validation_test.go index 5fa4d763f19..31d5c2f229d 100644 --- a/pkg/apis/pipeline/v1/task_validation_test.go +++ b/pkg/apis/pipeline/v1/task_validation_test.go @@ -1745,6 +1745,7 @@ func TestValidateParamArrayIndex(t *testing.T) { name string params []v1.Param taskspec *v1.TaskSpec + apiFields string expectedError error }{{ name: "steps reference invalid", @@ -1913,11 +1914,56 @@ func TestValidateParamArrayIndex(t *testing.T) { }, }, expectedError: fmt.Errorf("non-existent param references:[%v]", "$(params.array-params[3])"), + }, { + name: "stable gate not allowed", + params: []v1.Param{{ + 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])", + }, + }, + }, + apiFields: config.StableAPIFields, + expectedError: fmt.Errorf(`indexing into array params: %v require "enable-api-fields" feature gate to be "alpha" or "beta"`, []string{"$(params.array-params[3])"}), + }, { + name: "alpha gate allowed", + params: []v1.Param{{ + 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])", + }, + }, + }, + apiFields: config.AlphaAPIFields, + expectedError: fmt.Errorf("non-existent param references:[%v]", "$(params.array-params[3])"), }, } for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - ctx := config.ToContext(context.Background(), &config.Config{FeatureFlags: &config.FeatureFlags{EnableAPIFields: "alpha"}}) + // set default API fields to "beta" if not specified + ctx := context.Background() + switch tc.apiFields { + case config.AlphaAPIFields: + ctx = config.EnableAlphaAPIFields(ctx) + case config.StableAPIFields: + ctx = config.EnableStableAPIFields(ctx) + default: + ctx = config.EnableBetaAPIFields(ctx) + } err := tc.taskspec.ValidateParamArrayIndex(ctx, 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/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go index b1af8de0c26..b3cace2a722 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -540,10 +540,6 @@ func validateResultsFromMatrixedPipelineTasksNotConsumed(tasks []PipelineTask, f // error is returned when the array indexing reference is out of bound of the array param // e.g. if a param reference of $(params.array-param[2]) and the array param is of length 2. func (ps *PipelineSpec) ValidateParamArrayIndex(ctx context.Context, params Params) error { - if !config.CheckAlphaOrBetaAPIFields(ctx) { - return nil - } - // Collect all array params lengths arrayParamsLengths := ps.Params.extractParamArrayLengths() for k, v := range params.extractParamArrayLengths() { @@ -581,5 +577,10 @@ func (ps *PipelineSpec) ValidateParamArrayIndex(ctx context.Context, params Para arrayIndexParamRefs = append(arrayIndexParamRefs, extractArrayIndexingParamRefs(p)...) } + // if there are array indexing param references, the api feature gate needs to be set to `alpha` or `beta` + if len(arrayIndexParamRefs) > 0 && !config.CheckAlphaOrBetaAPIFields(ctx) { + return fmt.Errorf(`indexing into array params: %v require "enable-api-fields" feature gate to be "alpha" or "beta"`, arrayIndexParamRefs) + } + return validateOutofBoundArrayParams(arrayIndexParamRefs, arrayParamsLengths) } diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go index 1bc03563ec3..407e784bf07 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go @@ -3736,9 +3736,10 @@ func TestValidateParamArrayIndex_valid(t *testing.T) { cfg.FeatureFlags.EnableAPIFields = config.BetaAPIFields ctx = config.ToContext(ctx, cfg) for _, tt := range []struct { - name string - original PipelineSpec - params []Param + name string + original PipelineSpec + params []Param + apiFields string }{{ name: "single parameter", original: PipelineSpec{ @@ -3894,9 +3895,29 @@ func TestValidateParamArrayIndex_valid(t *testing.T) { }}, }, params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + }, { + name: "validation with alpha gate", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeString}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {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: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + apiFields: config.AlphaAPIFields, }, } { tt := tt // capture range variable + if tt.apiFields == config.AlphaAPIFields { + ctx = config.EnableAlphaAPIFields(ctx) + } t.Run(tt.name, func(t *testing.T) { t.Parallel() err := tt.original.ValidateParamArrayIndex(ctx, tt.params) @@ -3913,10 +3934,11 @@ func TestValidateParamArrayIndex_invalid(t *testing.T) { cfg.FeatureFlags.EnableAPIFields = config.BetaAPIFields ctx = config.ToContext(ctx, cfg) for _, tt := range []struct { - name string - original PipelineSpec - params []Param - expected error + name string + original PipelineSpec + params []Param + apiFields string + expected error }{{ name: "single parameter reference out of bound", original: PipelineSpec{ @@ -4100,11 +4122,31 @@ func TestValidateParamArrayIndex_invalid(t *testing.T) { }, params: []Param{{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])]"), + }, { + name: "stable gate not allowed", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeString}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {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: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + apiFields: config.StableAPIFields, + expected: fmt.Errorf(`indexing into array params: %v require "enable-api-fields" feature gate to be "alpha" or "beta"`, []string{"$(params.first-param[2])", "$(params.second-param[2])"}), }, } { tt := tt // capture range variable t.Run(tt.name, func(t *testing.T) { - t.Parallel() + if tt.apiFields == config.StableAPIFields { + ctx = config.EnableStableAPIFields(ctx) + } 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)) diff --git a/pkg/apis/pipeline/v1beta1/task_validation.go b/pkg/apis/pipeline/v1beta1/task_validation.go index 892aa1b35f3..5c74b2e622a 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation.go +++ b/pkg/apis/pipeline/v1beta1/task_validation.go @@ -628,11 +628,6 @@ func isParamRefs(s string) bool { // - `trParams` are params from taskrun. // - `taskSpec` contains params declarations. func (ts *TaskSpec) ValidateParamArrayIndex(ctx context.Context, params Params) error { - cfg := config.FromContextOrDefaults(ctx) - if cfg.FeatureFlags.EnableAPIFields != config.AlphaAPIFields { - return nil - } - // Collect all array params lengths arrayParamsLengths := ts.Params.extractParamArrayLengths() for k, v := range params.extractParamArrayLengths() { @@ -655,5 +650,10 @@ func (ts *TaskSpec) ValidateParamArrayIndex(ctx context.Context, params Params) arrayIndexParamRefs = append(arrayIndexParamRefs, extractArrayIndexingParamRefs(p)...) } + // if there are array indexing param references, the api feature gate needs to be set to `alpha` or `beta` + if len(arrayIndexParamRefs) > 0 && !config.CheckAlphaOrBetaAPIFields(ctx) { + return fmt.Errorf(`indexing into array params: %v require "enable-api-fields" feature gate to be "alpha" or "beta"`, arrayIndexParamRefs) + } + return validateOutofBoundArrayParams(arrayIndexParamRefs, arrayParamsLengths) } diff --git a/pkg/apis/pipeline/v1beta1/task_validation_test.go b/pkg/apis/pipeline/v1beta1/task_validation_test.go index 0a42616e981..f7325fb23f9 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/task_validation_test.go @@ -1858,6 +1858,7 @@ func TestValidateParamArrayIndex(t *testing.T) { name string params []v1beta1.Param taskspec *v1beta1.TaskSpec + apiFields string expectedError error }{{ name: "steps reference invalid", @@ -2026,11 +2027,56 @@ func TestValidateParamArrayIndex(t *testing.T) { }, }, expectedError: fmt.Errorf("non-existent param references:[%v]", "$(params.array-params[3])"), + }, { + name: "stable gate not allowed", + params: []v1beta1.Param{{ + 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])", + }, + }, + }, + apiFields: config.StableAPIFields, + expectedError: fmt.Errorf(`indexing into array params: %v require "enable-api-fields" feature gate to be "alpha" or "beta"`, []string{"$(params.array-params[3])"}), + }, { + name: "alpha gate allowed", + params: []v1beta1.Param{{ + 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])", + }, + }, + }, + apiFields: config.AlphaAPIFields, + expectedError: fmt.Errorf("non-existent param references:[%v]", "$(params.array-params[3])"), }, } for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - ctx := config.ToContext(context.Background(), &config.Config{FeatureFlags: &config.FeatureFlags{EnableAPIFields: "alpha"}}) + // set default API fields to "beta" if not specified + ctx := context.Background() + switch tc.apiFields { + case config.AlphaAPIFields: + ctx = config.EnableAlphaAPIFields(ctx) + case config.StableAPIFields: + ctx = config.EnableStableAPIFields(ctx) + default: + ctx = config.EnableBetaAPIFields(ctx) + } err := tc.taskspec.ValidateParamArrayIndex(ctx, tc.params) if d := cmp.Diff(tc.expectedError.Error(), err.Error()); d != "" { t.Errorf("validateParamArrayIndex() errors diff %s", diff.PrintWantGot(d))