diff --git a/pkg/apis/config/feature_flags.go b/pkg/apis/config/feature_flags.go index 305200b7fe9..bcbfcdc4c1d 100644 --- a/pkg/apis/config/feature_flags.go +++ b/pkg/apis/config/feature_flags.go @@ -315,17 +315,17 @@ func NewFeatureFlagsFromConfigMap(config *corev1.ConfigMap) (*FeatureFlags, erro // EnableAlphaAPIFields enables alpha features in an existing context (for use in testing) func EnableAlphaAPIFields(ctx context.Context) context.Context { - return setEnableAPIFields(ctx, AlphaAPIFields) + return SetEnableAPIFields(ctx, AlphaAPIFields) } // EnableBetaAPIFields enables beta features in an existing context (for use in testing) func EnableBetaAPIFields(ctx context.Context) context.Context { - return setEnableAPIFields(ctx, BetaAPIFields) + return SetEnableAPIFields(ctx, BetaAPIFields) } // EnableStableAPIFields enables stable features in an existing context (for use in testing) func EnableStableAPIFields(ctx context.Context) context.Context { - return setEnableAPIFields(ctx, StableAPIFields) + return SetEnableAPIFields(ctx, StableAPIFields) } // CheckEnforceResourceVerificationMode returns true if the ResourceVerificationMode is EnforceResourceVerificationMode @@ -348,7 +348,8 @@ func CheckAlphaOrBetaAPIFields(ctx context.Context) bool { return cfg.FeatureFlags.EnableAPIFields == AlphaAPIFields || cfg.FeatureFlags.EnableAPIFields == BetaAPIFields } -func setEnableAPIFields(ctx context.Context, want string) context.Context { +// SetEnableAPIFields sets the config "enable-api-fields" to the "want" value +func SetEnableAPIFields(ctx context.Context, want string) context.Context { featureFlags, _ := NewFeatureFlagsFromMap(map[string]string{ "enable-api-fields": want, }) diff --git a/pkg/apis/pipeline/v1/pipeline_validation.go b/pkg/apis/pipeline/v1/pipeline_validation.go index 1c8ab05e6df..50569b24514 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(`invalid parameter expression %s: indexing into array params requires "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 a18e4f6025c..c3cdde68144 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1/pipeline_validation_test.go @@ -3339,14 +3339,11 @@ 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 []Param + name string + original PipelineSpec + params []Param + apiFields string }{{ name: "single parameter", original: PipelineSpec{ @@ -3362,7 +3359,8 @@ func TestValidateParamArrayIndex_valid(t *testing.T) { }, }}, }, - params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + apiFields: config.BetaAPIFields, }, { name: "single parameter with when expression", original: PipelineSpec{ @@ -3378,7 +3376,8 @@ func TestValidateParamArrayIndex_valid(t *testing.T) { }}, }}, }, - params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + apiFields: config.BetaAPIFields, }, { name: "pipeline parameter nested inside task parameter", original: PipelineSpec{ @@ -3393,7 +3392,8 @@ func TestValidateParamArrayIndex_valid(t *testing.T) { }, }}, }, - params: nil, // no parameter values. + params: nil, // no parameter values. + apiFields: config.BetaAPIFields, }, { name: "array parameter", original: PipelineSpec{ @@ -3411,6 +3411,7 @@ func TestValidateParamArrayIndex_valid(t *testing.T) { params: []Param{ {Name: "second-param", Value: *NewStructuredValues("second-value", "array")}, }, + apiFields: config.BetaAPIFields, }, { name: "parameter evaluation with final tasks", original: PipelineSpec{ @@ -3430,7 +3431,8 @@ func TestValidateParamArrayIndex_valid(t *testing.T) { }}, }}, }, - params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + apiFields: config.BetaAPIFields, }, { name: "parameter evaluation with both tasks and final tasks", original: PipelineSpec{ @@ -3456,7 +3458,8 @@ func TestValidateParamArrayIndex_valid(t *testing.T) { }}, }}, }, - params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + apiFields: config.BetaAPIFields, }, { name: "parameter references with bracket notation and special characters", original: PipelineSpec{ @@ -3480,6 +3483,7 @@ func TestValidateParamArrayIndex_valid(t *testing.T) { {Name: "second/param", Value: *NewStructuredValues("second-value", "second-value-again")}, {Name: "fourth/param", Value: *NewStructuredValues("fourth-value", "fourth-value-again")}, }, + apiFields: config.BetaAPIFields, }, { name: "single parameter in workspace subpath", original: PipelineSpec{ @@ -3501,10 +3505,29 @@ func TestValidateParamArrayIndex_valid(t *testing.T) { }, }}, }, - params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + apiFields: config.BetaAPIFields, + }, { + 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 + ctx := config.SetEnableAPIFields(context.Background(), tt.apiFields) t.Run(tt.name, func(t *testing.T) { t.Parallel() err := tt.original.ValidateParamArrayIndex(ctx, tt.params) @@ -3516,15 +3539,12 @@ func TestValidateParamArrayIndex_valid(t *testing.T) { } func TestValidateParamArrayIndex_invalid(t *testing.T) { - ctx := context.Background() - cfg := config.FromContextOrDefaults(ctx) - cfg.FeatureFlags.EnableAPIFields = config.BetaAPIFields - ctx = config.ToContext(ctx, cfg) for _, tt := range []struct { - name string - original PipelineSpec - params []Param - expected error + name string + original PipelineSpec + params []Param + apiFields string + expected error }{{ name: "single parameter reference out of bound", original: PipelineSpec{ @@ -3540,8 +3560,9 @@ 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[2])]"), + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + apiFields: config.BetaAPIFields, + 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{ @@ -3557,8 +3578,9 @@ 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[2])]"), + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + apiFields: config.BetaAPIFields, + 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{ @@ -3573,8 +3595,9 @@ func TestValidateParamArrayIndex_invalid(t *testing.T) { }, }}, }, - params: nil, // no parameter values. - expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[2])]"), + params: nil, // no parameter values. + apiFields: config.BetaAPIFields, + expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[2])]"), }, { name: "array parameter reference out of bound", original: PipelineSpec{ @@ -3592,7 +3615,8 @@ func TestValidateParamArrayIndex_invalid(t *testing.T) { params: []Param{ {Name: "second-param", Value: *NewStructuredValues("second-value", "array")}, }, - expected: fmt.Errorf("non-existent param references:[$(params.first-param[3]) $(params.second-param[4])]"), + apiFields: config.BetaAPIFields, + expected: fmt.Errorf("non-existent param references:[$(params.first-param[3]) $(params.second-param[4])]"), }, { name: "object parameter reference out of bound", original: PipelineSpec{ @@ -3612,7 +3636,8 @@ func TestValidateParamArrayIndex_invalid(t *testing.T) { params: []Param{ {Name: "second-param", Value: *NewStructuredValues("second-value", "array")}, }, - expected: fmt.Errorf("non-existent param references:[$(params.first-param[4]) $(params.second-param[4])]"), + apiFields: config.BetaAPIFields, + 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{ @@ -3632,8 +3657,9 @@ 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[2])]"), + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + apiFields: config.BetaAPIFields, + 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: PipelineSpec{ @@ -3664,8 +3690,9 @@ 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.first-param[3]) $(params.first-param[4]) $(params.second-param[2]) $(params.second-param[3]) $(params.second-param[4])]"), + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + apiFields: config.BetaAPIFields, + 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{ @@ -3682,8 +3709,9 @@ 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: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + apiFields: config.BetaAPIFields, + 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{ @@ -3707,7 +3735,8 @@ func TestValidateParamArrayIndex_invalid(t *testing.T) { {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])]"), + apiFields: config.BetaAPIFields, + 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{ @@ -3729,13 +3758,57 @@ 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])]"), + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + apiFields: config.BetaAPIFields, + expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[3])]"), + }, { + name: "validation alpha feature gate", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeArray}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {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: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + apiFields: config.AlphaAPIFields, + 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(`invalid parameter expression %s: indexing into array params requires "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() + ctx := config.SetEnableAPIFields(context.Background(), tt.apiFields) 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..94b291d78d6 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(`invalid parameter expression %s: indexing into array params requires "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..1ed46b6644e 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(`invalid parameter expression %s: indexing into array params requires "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 c2456a51480..1143c7f550f 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -430,10 +430,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() { @@ -471,5 +467,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(`invalid parameter expression %s: indexing into array params requires "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 bb4e2e5ab5c..c2d39958265 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go @@ -3358,14 +3358,11 @@ 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 []Param + name string + original PipelineSpec + params []Param + apiFields string }{{ name: "single parameter", original: PipelineSpec{ @@ -3381,7 +3378,8 @@ func TestValidateParamArrayIndex_valid(t *testing.T) { }, }}, }, - params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + apiFields: config.BetaAPIFields, }, { name: "single parameter with when expression", original: PipelineSpec{ @@ -3397,7 +3395,8 @@ func TestValidateParamArrayIndex_valid(t *testing.T) { }}, }}, }, - params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + apiFields: config.BetaAPIFields, }, { name: "pipeline parameter nested inside task parameter", original: PipelineSpec{ @@ -3412,7 +3411,8 @@ func TestValidateParamArrayIndex_valid(t *testing.T) { }, }}, }, - params: nil, // no parameter values. + params: nil, // no parameter values. + apiFields: config.BetaAPIFields, }, { name: "array parameter", original: PipelineSpec{ @@ -3430,6 +3430,7 @@ func TestValidateParamArrayIndex_valid(t *testing.T) { params: []Param{ {Name: "second-param", Value: *NewStructuredValues("second-value", "array")}, }, + apiFields: config.BetaAPIFields, }, { name: "parameter evaluation with final tasks", original: PipelineSpec{ @@ -3449,7 +3450,8 @@ func TestValidateParamArrayIndex_valid(t *testing.T) { }}, }}, }, - params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + apiFields: config.BetaAPIFields, }, { name: "parameter evaluation with both tasks and final tasks", original: PipelineSpec{ @@ -3475,7 +3477,8 @@ func TestValidateParamArrayIndex_valid(t *testing.T) { }}, }}, }, - params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + apiFields: config.BetaAPIFields, }, { name: "parameter references with bracket notation and special characters", original: PipelineSpec{ @@ -3499,6 +3502,7 @@ func TestValidateParamArrayIndex_valid(t *testing.T) { {Name: "second/param", Value: *NewStructuredValues("second-value", "second-value-again")}, {Name: "fourth/param", Value: *NewStructuredValues("fourth-value", "fourth-value-again")}, }, + apiFields: config.BetaAPIFields, }, { name: "single parameter in workspace subpath", original: PipelineSpec{ @@ -3520,10 +3524,29 @@ func TestValidateParamArrayIndex_valid(t *testing.T) { }, }}, }, - params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + apiFields: config.BetaAPIFields, + }, { + 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 + ctx := config.SetEnableAPIFields(context.Background(), tt.apiFields) t.Run(tt.name, func(t *testing.T) { t.Parallel() err := tt.original.ValidateParamArrayIndex(ctx, tt.params) @@ -3535,15 +3558,12 @@ func TestValidateParamArrayIndex_valid(t *testing.T) { } func TestValidateParamArrayIndex_invalid(t *testing.T) { - ctx := context.Background() - cfg := config.FromContextOrDefaults(ctx) - cfg.FeatureFlags.EnableAPIFields = config.BetaAPIFields - ctx = config.ToContext(ctx, cfg) for _, tt := range []struct { - name string - original PipelineSpec - params []Param - expected error + name string + original PipelineSpec + params []Param + apiFields string + expected error }{{ name: "single parameter reference out of bound", original: PipelineSpec{ @@ -3559,8 +3579,9 @@ 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[2])]"), + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + apiFields: config.BetaAPIFields, + 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{ @@ -3576,8 +3597,9 @@ 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[2])]"), + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + apiFields: config.BetaAPIFields, + 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{ @@ -3592,8 +3614,9 @@ func TestValidateParamArrayIndex_invalid(t *testing.T) { }, }}, }, - params: nil, // no parameter values. - expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[2])]"), + params: nil, // no parameter values. + apiFields: config.BetaAPIFields, + expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[2])]"), }, { name: "array parameter reference out of bound", original: PipelineSpec{ @@ -3611,7 +3634,8 @@ func TestValidateParamArrayIndex_invalid(t *testing.T) { params: []Param{ {Name: "second-param", Value: *NewStructuredValues("second-value", "array")}, }, - expected: fmt.Errorf("non-existent param references:[$(params.first-param[3]) $(params.second-param[4])]"), + apiFields: config.BetaAPIFields, + expected: fmt.Errorf("non-existent param references:[$(params.first-param[3]) $(params.second-param[4])]"), }, { name: "object parameter reference out of bound", original: PipelineSpec{ @@ -3631,7 +3655,8 @@ func TestValidateParamArrayIndex_invalid(t *testing.T) { params: []Param{ {Name: "second-param", Value: *NewStructuredValues("second-value", "array")}, }, - expected: fmt.Errorf("non-existent param references:[$(params.first-param[4]) $(params.second-param[4])]"), + apiFields: config.BetaAPIFields, + 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{ @@ -3651,8 +3676,9 @@ 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[2])]"), + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + apiFields: config.BetaAPIFields, + 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: PipelineSpec{ @@ -3683,8 +3709,9 @@ 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.first-param[3]) $(params.first-param[4]) $(params.second-param[2]) $(params.second-param[3]) $(params.second-param[4])]"), + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + apiFields: config.BetaAPIFields, + 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{ @@ -3701,8 +3728,9 @@ 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: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + apiFields: config.BetaAPIFields, + 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{ @@ -3726,7 +3754,8 @@ func TestValidateParamArrayIndex_invalid(t *testing.T) { {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])]"), + apiFields: config.BetaAPIFields, + 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{ @@ -3748,13 +3777,57 @@ 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])]"), + params: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + apiFields: config.BetaAPIFields, + expected: fmt.Errorf("non-existent param references:[$(params.first-param[2]) $(params.second-param[3])]"), + }, { + name: "validation alpha feature gate", + original: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + {Name: "second-param", Type: ParamTypeArray}, + }, + Tasks: []PipelineTask{{ + Params: []Param{ + {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: []Param{{Name: "second-param", Value: *NewStructuredValues("second-value", "second-value-again")}}, + apiFields: config.AlphaAPIFields, + 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(`invalid parameter expression %s: indexing into array params requires "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() + ctx := config.SetEnableAPIFields(context.Background(), tt.apiFields) 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 1486c38aaa6..ee28b926ff0 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation.go +++ b/pkg/apis/pipeline/v1beta1/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(`invalid parameter expression %s: indexing into array params requires "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 982655e2540..153b3b34615 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/task_validation_test.go @@ -1748,6 +1748,7 @@ func TestValidateParamArrayIndex(t *testing.T) { name string params []v1beta1.Param taskspec *v1beta1.TaskSpec + apiFields string expectedError error }{{ name: "steps reference invalid", @@ -1916,11 +1917,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(`invalid parameter expression %s: indexing into array params requires "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))