From 56018ca845f0534a61b3195ca7fc6ca291f4d9eb Mon Sep 17 00:00:00 2001 From: Quan Zhang Date: Tue, 24 Oct 2023 11:26:42 -0400 Subject: [PATCH] [TEP-0144] Add enum API field Part of [#7270][#7270]. In [TEP-0144][tep-0144] we proposed a new `enum` field to support built-in param input validation. This commit adds the `Enum` api field, validation and conversion logic. /kind feature [#7270]: https://github.com/tektoncd/pipeline/issues/7270 [tep-0144]: https://github.com/tektoncd/community/blob/main/teps/0144-param-enum.md --- docs/pipeline-api.md | 26 ++ hack/ignored-openapi-violations.list | 2 + pkg/apis/pipeline/v1/openapi_generated.go | 15 + pkg/apis/pipeline/v1/param_types.go | 48 ++- pkg/apis/pipeline/v1/pipeline_types_test.go | 47 +- pkg/apis/pipeline/v1/pipeline_validation.go | 3 +- .../pipeline/v1/pipeline_validation_test.go | 93 +++- pkg/apis/pipeline/v1/swagger.json | 8 + pkg/apis/pipeline/v1/task_validation.go | 1 + pkg/apis/pipeline/v1/task_validation_test.go | 83 ++++ pkg/apis/pipeline/v1/zz_generated.deepcopy.go | 5 + .../pipeline/v1beta1/openapi_generated.go | 15 + pkg/apis/pipeline/v1beta1/param_conversion.go | 2 + pkg/apis/pipeline/v1beta1/param_types.go | 48 ++- .../v1beta1/pipeline_conversion_test.go | 1 + .../pipeline/v1beta1/pipeline_types_test.go | 53 ++- .../pipeline/v1beta1/pipeline_validation.go | 3 +- .../v1beta1/pipeline_validation_test.go | 407 +++++++++++------- pkg/apis/pipeline/v1beta1/swagger.json | 8 + .../pipeline/v1beta1/task_conversion_test.go | 1 + pkg/apis/pipeline/v1beta1/task_validation.go | 1 + .../pipeline/v1beta1/task_validation_test.go | 83 ++++ .../pipeline/v1beta1/zz_generated.deepcopy.go | 5 + 23 files changed, 735 insertions(+), 223 deletions(-) diff --git a/docs/pipeline-api.md b/docs/pipeline-api.md index f100fbf2561..2b26131cc17 100644 --- a/docs/pipeline-api.md +++ b/docs/pipeline-api.md @@ -1691,6 +1691,19 @@ default is set, a Task may be executed without a supplied value for the parameter.

+ + +enum
+ +[]string + + + +(Optional) +

Enum declares a set of allowed param input values for tasks/pipelines that can be validated. +If Enum is not set, no input validation is performed for the param.

+ +

ParamSpecs @@ -9963,6 +9976,19 @@ default is set, a Task may be executed without a supplied value for the parameter.

+ + +enum
+ +[]string + + + +(Optional) +

Enum declares a set of allowed param input values for tasks/pipelines that can be validated. +If Enum is not set, no input validation is performed for the param.

+ +

ParamSpecs diff --git a/hack/ignored-openapi-violations.list b/hack/ignored-openapi-violations.list index 9badeae797c..fc91c45ba27 100644 --- a/hack/ignored-openapi-violations.list +++ b/hack/ignored-openapi-violations.list @@ -41,3 +41,5 @@ API rule violation: names_match,github.com/tektoncd/pipeline/pkg/apis/resolution API rule violation: list_type_missing,github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1,RunSpec,Workspaces API rule violation: list_type_missing,github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1,VerificationPolicySpec,Authorities API rule violation: list_type_missing,github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1,VerificationPolicySpec,Resources +API rule violation: list_type_missing,github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1,ParamSpec,Enum +API rule violation: list_type_missing,github.com/tektoncd/pipeline/pkg/apis/pipeline/v1,ParamSpec,Enum diff --git a/pkg/apis/pipeline/v1/openapi_generated.go b/pkg/apis/pipeline/v1/openapi_generated.go index 8dc34c03672..f21c7a06c0c 100644 --- a/pkg/apis/pipeline/v1/openapi_generated.go +++ b/pkg/apis/pipeline/v1/openapi_generated.go @@ -789,6 +789,21 @@ func schema_pkg_apis_pipeline_v1_ParamSpec(ref common.ReferenceCallback) common. Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1.ParamValue"), }, }, + "enum": { + SchemaProps: spec.SchemaProps{ + Description: "Enum declares a set of allowed param input values for tasks/pipelines that can be validated. If Enum is not set, no input validation is performed for the param.", + Type: []string{"array"}, + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: "", + Type: []string{"string"}, + Format: "", + }, + }, + }, + }, + }, }, Required: []string{"name"}, }, diff --git a/pkg/apis/pipeline/v1/param_types.go b/pkg/apis/pipeline/v1/param_types.go index 167a1084a20..4852491f0b6 100644 --- a/pkg/apis/pipeline/v1/param_types.go +++ b/pkg/apis/pipeline/v1/param_types.go @@ -22,6 +22,7 @@ import ( "fmt" "strings" + "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/substitution" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" @@ -53,6 +54,10 @@ type ParamSpec struct { // parameter. // +optional Default *ParamValue `json:"default,omitempty"` + // Enum declares a set of allowed param input values for tasks/pipelines that can be validated. + // If Enum is not set, no input validation is performed for the param. + // +optional + Enum []string `json:"enum,omitempty"` } // ParamSpecs is a list of ParamSpec @@ -132,22 +137,47 @@ func (ps ParamSpecs) sortByType() (ParamSpecs, ParamSpecs, ParamSpecs) { // validateNoDuplicateNames returns an error if any of the params have the same name func (ps ParamSpecs) validateNoDuplicateNames() *apis.FieldError { + var errs *apis.FieldError names := ps.getNames() - seen := sets.String{} - dups := sets.String{} + for dup := range findDups(names) { + errs = errs.Also(apis.ErrGeneric("parameter appears more than once", "").ViaFieldKey("params", dup)) + } + return errs +} + +// validateParamEnum validates feature flag, duplication and allowed types for Param Enum +func (ps ParamSpecs) validateParamEnums(ctx context.Context) *apis.FieldError { var errs *apis.FieldError - for _, n := range names { - if seen.Has(n) { - dups.Insert(n) + for _, p := range ps { + if len(p.Enum) == 0 { + continue + } + if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableParamEnum { + errs = errs.Also(errs, apis.ErrGeneric(fmt.Sprintf("feature flag `%s` should be set to true to use Enum", config.EnableParamEnum), "").ViaFieldKey("params", p.Name)) + } + if p.Type != ParamTypeString { + errs = errs.Also(apis.ErrGeneric("enum can only be set with string type param", "").ViaFieldKey("params", p.Name)) + } + for dup := range findDups(p.Enum) { + errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("parameter enum value %v appears more than once", dup), "").ViaFieldKey("params", p.Name)) } - seen.Insert(n) - } - for n := range dups { - errs = errs.Also(apis.ErrGeneric("parameter appears more than once", "").ViaFieldKey("params", n)) } return errs } +// findDups returns the duplicate element in the given slice +func findDups(vals []string) sets.String { + seen := sets.String{} + dups := sets.String{} + for _, val := range vals { + if seen.Has(val) { + dups.Insert(val) + } + seen.Insert(val) + } + return dups +} + // Param declares an ParamValues to use for the parameter called name. type Param struct { Name string `json:"name"` diff --git a/pkg/apis/pipeline/v1/pipeline_types_test.go b/pkg/apis/pipeline/v1/pipeline_types_test.go index 75d8657962a..61771b90f2a 100644 --- a/pkg/apis/pipeline/v1/pipeline_types_test.go +++ b/pkg/apis/pipeline/v1/pipeline_types_test.go @@ -508,10 +508,9 @@ func TestPipelineTask_ValidateCustomTask(t *testing.T) { func TestPipelineTask_ValidateRegularTask_Success(t *testing.T) { tests := []struct { - name string - tasks PipelineTask - enableAlphaAPIFields bool - enableBetaAPIFields bool + name string + tasks PipelineTask + configMap map[string]string }{{ name: "pipeline task - valid taskRef name", tasks: PipelineTask{ @@ -524,37 +523,51 @@ func TestPipelineTask_ValidateRegularTask_Success(t *testing.T) { Name: "foo", TaskSpec: &EmbeddedTask{TaskSpec: getTaskSpec()}, }, + }, { + name: "pipeline task - valid taskSpec with param enum", + tasks: PipelineTask{ + Name: "foo", + TaskSpec: &EmbeddedTask{ + TaskSpec: TaskSpec{ + Steps: []Step{ + { + Name: "foo", + Image: "bar", + }, + }, + Params: []ParamSpec{ + { + Name: "param1", + Type: ParamTypeString, + Enum: []string{"v1", "v2"}, + }, + }, + }, + }, + }, + configMap: map[string]string{"enable-param-enum": "true"}, }, { name: "pipeline task - use of resolver with the feature flag set", tasks: PipelineTask{ TaskRef: &TaskRef{ResolverRef: ResolverRef{Resolver: "bar"}}, }, - enableBetaAPIFields: true, + configMap: map[string]string{"enable-api-field": "beta"}, }, { name: "pipeline task - use of resolver with the feature flag set to alpha", tasks: PipelineTask{ TaskRef: &TaskRef{ResolverRef: ResolverRef{Resolver: "bar"}}, }, - enableAlphaAPIFields: true, + configMap: map[string]string{"enable-api-field": "alpha"}, }, { name: "pipeline task - use of resolver params with the feature flag set", tasks: PipelineTask{ TaskRef: &TaskRef{ResolverRef: ResolverRef{Resolver: "bar", Params: Params{{}}}}, }, - enableBetaAPIFields: true, + configMap: map[string]string{"enable-api-field": "beta"}, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ctx := context.Background() - cfg := &config.Config{ - FeatureFlags: &config.FeatureFlags{}, - } - if tt.enableAlphaAPIFields { - cfg.FeatureFlags.EnableAPIFields = config.AlphaAPIFields - } else if tt.enableBetaAPIFields { - cfg.FeatureFlags.EnableAPIFields = config.BetaAPIFields - } - ctx = config.ToContext(ctx, cfg) + ctx := cfgtesting.SetFeatureFlags(context.Background(), t, tt.configMap) err := tt.tasks.validateTask(ctx) if err != nil { t.Errorf("PipelineTask.validateTask() returned error for valid pipeline task: %v", err) diff --git a/pkg/apis/pipeline/v1/pipeline_validation.go b/pkg/apis/pipeline/v1/pipeline_validation.go index 2552692d87a..06e85050851 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1/pipeline_validation.go @@ -433,11 +433,12 @@ func validatePipelineTasksWorkspacesUsage(wss []PipelineWorkspaceDeclaration, pt // ValidatePipelineParameterVariables validates parameters with those specified by each pipeline task, // (1) it validates the type of parameter is either string or array (2) parameter default value matches -// with the type of that param +// with the type of that param (3) no duplicateion, feature flag and allowed param type when using param enum func ValidatePipelineParameterVariables(ctx context.Context, tasks []PipelineTask, params ParamSpecs) (errs *apis.FieldError) { // validates all the types within a slice of ParamSpecs errs = errs.Also(ValidateParameterTypes(ctx, params).ViaField("params")) errs = errs.Also(params.validateNoDuplicateNames()) + errs = errs.Also(params.validateParamEnums(ctx)) for i, task := range tasks { errs = errs.Also(task.Params.validateDuplicateParameters().ViaField("params").ViaIndex(i)) } diff --git a/pkg/apis/pipeline/v1/pipeline_validation_test.go b/pkg/apis/pipeline/v1/pipeline_validation_test.go index 820e7958e6d..37d7100bc56 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1/pipeline_validation_test.go @@ -1295,9 +1295,10 @@ func TestFinallyTaskResultsToPipelineResults_Failure(t *testing.T) { func TestValidatePipelineParameterVariables_Success(t *testing.T) { tests := []struct { - name string - params []ParamSpec - tasks []PipelineTask + name string + params []ParamSpec + tasks []PipelineTask + configMap map[string]string }{{ name: "valid string parameter variables", params: []ParamSpec{{ @@ -1312,6 +1313,21 @@ func TestValidatePipelineParameterVariables_Success(t *testing.T) { Name: "a-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.baz) and $(params.foo-is-baz)"}, }}, }}, + }, { + name: "valid string parameter variables with enum", + params: []ParamSpec{{ + Name: "baz", Type: ParamTypeString, Enum: []string{"v1", "v2"}, + }, { + Name: "foo-is-baz", Type: ParamTypeString, + }}, + tasks: []PipelineTask{{ + Name: "bar", + TaskRef: &TaskRef{Name: "bar-task"}, + Params: Params{{ + Name: "a-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.baz) and $(params.foo-is-baz)"}, + }}, + }}, + configMap: map[string]string{"enable-param-enum": "true"}, }, { name: "valid string parameter variables in when expression", params: []ParamSpec{{ @@ -1580,6 +1596,9 @@ func TestValidatePipelineParameterVariables_Success(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ctx := cfgtesting.EnableAlphaAPIFields(context.Background()) + if tt.configMap != nil { + ctx = cfgtesting.SetFeatureFlags(ctx, t, tt.configMap) + } err := ValidatePipelineParameterVariables(ctx, tt.tasks, tt.params) if err != nil { t.Errorf("Pipeline.ValidatePipelineParameterVariables() returned error for valid pipeline parameters: %v", err) @@ -1921,7 +1940,7 @@ func TestValidatePipelineDeclaredParameterUsage_Failure(t *testing.T) { t.Run(tt.name, func(t *testing.T) { err := validatePipelineTaskParameterUsage(tt.tasks, tt.params) if err == nil { - t.Errorf("Pipeline.ValidatePipelineParameterVariables() did not return error for invalid pipeline parameters") + t.Errorf("Pipeline.validatePipelineTaskParameterUsage() did not return error for invalid pipeline parameters") } if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" { t.Errorf("PipelineSpec.Validate() errors diff %s", diff.PrintWantGot(d)) @@ -1936,7 +1955,69 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) { params []ParamSpec tasks []PipelineTask expectedError apis.FieldError + configMap map[string]string }{{ + name: "param enum with array type - failure", + params: []ParamSpec{{ + Name: "param1", + Type: ParamTypeArray, + Enum: []string{"v1", "v2"}, + }}, + tasks: []PipelineTask{{ + Name: "foo", + TaskRef: &TaskRef{Name: "foo-task"}, + }}, + configMap: map[string]string{ + "enable-param-enum": "true", + }, + expectedError: apis.FieldError{ + Message: `enum can only be set with string type param`, + Paths: []string{"params[param1]"}, + }, + }, { + name: "param enum with object type - failure", + params: []ParamSpec{{ + Name: "param1", + Type: ParamTypeObject, + Enum: []string{"v1", "v2"}, + }}, + tasks: []PipelineTask{{ + Name: "foo", + TaskRef: &TaskRef{Name: "foo-task"}, + }}, + configMap: map[string]string{ + "enable-param-enum": "true", + }, + expectedError: apis.FieldError{ + Message: `enum can only be set with string type param`, + Paths: []string{"params[param1]"}, + }, + }, { + name: "param enum with duplicate values - failure", + params: []ParamSpec{{ + Name: "param1", + Type: ParamTypeString, + Enum: []string{"v1", "v1", "v2"}, + }}, + configMap: map[string]string{ + "enable-param-enum": "true", + }, + expectedError: apis.FieldError{ + Message: `parameter enum value v1 appears more than once`, + Paths: []string{"params[param1]"}, + }, + }, { + name: "param enum with feature flag disabled - failure", + params: []ParamSpec{{ + Name: "param1", + Type: ParamTypeString, + Enum: []string{"v1", "v2"}, + }}, + expectedError: apis.FieldError{ + Message: "feature flag `enable-param-enum` should be set to true to use Enum", + Paths: []string{"params[param1]"}, + }, + }, { name: "invalid parameter type", params: []ParamSpec{{ Name: "foo", Type: "invalidtype", @@ -2105,6 +2186,10 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ctx := context.Background() + if tt.configMap != nil { + ctx = cfgtesting.SetFeatureFlags(ctx, t, tt.configMap) + } + err := ValidatePipelineParameterVariables(ctx, tt.tasks, tt.params) if err == nil { t.Errorf("Pipeline.ValidatePipelineParameterVariables() did not return error for invalid pipeline parameters") diff --git a/pkg/apis/pipeline/v1/swagger.json b/pkg/apis/pipeline/v1/swagger.json index 03246208237..79d8d7aad38 100644 --- a/pkg/apis/pipeline/v1/swagger.json +++ b/pkg/apis/pipeline/v1/swagger.json @@ -343,6 +343,14 @@ "description": "Description is a user-facing description of the parameter that may be used to populate a UI.", "type": "string" }, + "enum": { + "description": "Enum declares a set of allowed param input values for tasks/pipelines that can be validated. If Enum is not set, no input validation is performed for the param.", + "type": "array", + "items": { + "type": "string", + "default": "" + } + }, "name": { "description": "Name declares the name by which a parameter is referenced.", "type": "string", diff --git a/pkg/apis/pipeline/v1/task_validation.go b/pkg/apis/pipeline/v1/task_validation.go index cdad33d7a80..9ecf3914d88 100644 --- a/pkg/apis/pipeline/v1/task_validation.go +++ b/pkg/apis/pipeline/v1/task_validation.go @@ -435,6 +435,7 @@ func (p ParamSpec) ValidateObjectType(ctx context.Context) *apis.FieldError { func ValidateParameterVariables(ctx context.Context, steps []Step, params ParamSpecs) *apis.FieldError { var errs *apis.FieldError errs = errs.Also(params.validateNoDuplicateNames()) + errs = errs.Also(params.validateParamEnums(ctx)) stringParams, arrayParams, objectParams := params.sortByType() stringParameterNames := sets.NewString(stringParams.getNames()...) arrayParameterNames := sets.NewString(arrayParams.getNames()...) diff --git a/pkg/apis/pipeline/v1/task_validation_test.go b/pkg/apis/pipeline/v1/task_validation_test.go index f3f8326d795..4a9967ec0b8 100644 --- a/pkg/apis/pipeline/v1/task_validation_test.go +++ b/pkg/apis/pipeline/v1/task_validation_test.go @@ -2254,3 +2254,86 @@ func TestGetArrayIndexParamRefs(t *testing.T) { }) } } + +func TestParamEnum(t *testing.T) { + tcs := []struct { + name string + params v1.ParamSpecs + configMap map[string]string + expectedErr error + }{{ + name: "valid param enum - success", + params: []v1.ParamSpec{{ + Name: "param1", + Type: v1.ParamTypeString, + Enum: []string{"v1", "v2"}, + }, { + Name: "param2", + Type: v1.ParamTypeString, + Enum: []string{"v1", "v2"}, + }}, + configMap: map[string]string{ + "enable-param-enum": "true", + }, + }, { + name: "param enum with array type - failure", + params: []v1.ParamSpec{{ + Name: "param1", + Type: v1.ParamTypeArray, + Enum: []string{"v1", "v2"}, + }}, + configMap: map[string]string{ + "enable-param-enum": "true", + }, + expectedErr: fmt.Errorf("enum can only be set with string type param: params[param1]"), + }, { + name: "param enum with object type - failure", + params: []v1.ParamSpec{{ + Name: "param1", + Type: v1.ParamTypeObject, + Enum: []string{"v1", "v2"}, + }}, + configMap: map[string]string{ + "enable-param-enum": "true", + }, + expectedErr: fmt.Errorf("enum can only be set with string type param: params[param1]"), + }, { + name: "param enum with duplicate values - failure", + params: []v1.ParamSpec{{ + Name: "param1", + Type: v1.ParamTypeString, + Enum: []string{"v1", "v1", "v2"}, + }}, + configMap: map[string]string{ + "enable-param-enum": "true", + }, + expectedErr: fmt.Errorf("parameter enum value v1 appears more than once: params[param1]"), + }, { + name: "param enum with feature flag disabled - failure", + params: []v1.ParamSpec{{ + Name: "param1", + Type: v1.ParamTypeString, + Enum: []string{"v1", "v2"}, + }}, + configMap: map[string]string{ + "enable-param-enum": "false", + }, + expectedErr: fmt.Errorf("feature flag `enable-param-enum` should be set to true to use Enum: params[param1]"), + }} + + for _, tc := range tcs { + ctx := cfgtesting.SetFeatureFlags(context.Background(), t, tc.configMap) + + err := v1.ValidateParameterVariables(ctx, []v1.Step{{Image: "foo"}}, tc.params) + + if tc.expectedErr == nil && err != nil { + t.Errorf("No error expected from ValidateParameterVariables() but got = %v", err) + } else if tc.expectedErr != nil { + if err == nil { + t.Errorf("Expected error from ValidateParameterVariables() = %v, but got none", tc.expectedErr) + } else if d := cmp.Diff(tc.expectedErr.Error(), err.Error()); d != "" { + t.Errorf("Rerturned error from ValidateParameterVariables() does not match with the expected error: %s", diff.PrintWantGot(d)) + } + } + } +} diff --git a/pkg/apis/pipeline/v1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1/zz_generated.deepcopy.go index c59e2c4f615..59ad89d8125 100644 --- a/pkg/apis/pipeline/v1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1/zz_generated.deepcopy.go @@ -231,6 +231,11 @@ func (in *ParamSpec) DeepCopyInto(out *ParamSpec) { *out = new(ParamValue) (*in).DeepCopyInto(*out) } + if in.Enum != nil { + in, out := &in.Enum, &out.Enum + *out = make([]string, len(*in)) + copy(*out, *in) + } return } diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index 430d41c2b54..ae0e0b1fa93 100644 --- a/pkg/apis/pipeline/v1beta1/openapi_generated.go +++ b/pkg/apis/pipeline/v1beta1/openapi_generated.go @@ -1330,6 +1330,21 @@ func schema_pkg_apis_pipeline_v1beta1_ParamSpec(ref common.ReferenceCallback) co Ref: ref("github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1.ParamValue"), }, }, + "enum": { + SchemaProps: spec.SchemaProps{ + Description: "Enum declares a set of allowed param input values for tasks/pipelines that can be validated. If Enum is not set, no input validation is performed for the param.", + Type: []string{"array"}, + Items: &spec.SchemaOrArray{ + Schema: &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Default: "", + Type: []string{"string"}, + Format: "", + }, + }, + }, + }, + }, }, Required: []string{"name"}, }, diff --git a/pkg/apis/pipeline/v1beta1/param_conversion.go b/pkg/apis/pipeline/v1beta1/param_conversion.go index 5ac4f50a863..a47206b76e1 100644 --- a/pkg/apis/pipeline/v1beta1/param_conversion.go +++ b/pkg/apis/pipeline/v1beta1/param_conversion.go @@ -30,6 +30,7 @@ func (p ParamSpec) convertTo(ctx context.Context, sink *v1.ParamSpec) { sink.Type = v1.ParamType(ParamTypeString) } sink.Description = p.Description + sink.Enum = p.Enum var properties map[string]v1.PropertySpec if p.Properties != nil { properties = make(map[string]v1.PropertySpec) @@ -54,6 +55,7 @@ func (p *ParamSpec) convertFrom(ctx context.Context, source v1.ParamSpec) { p.Type = ParamTypeString } p.Description = source.Description + p.Enum = source.Enum var properties map[string]PropertySpec if source.Properties != nil { properties = make(map[string]PropertySpec) diff --git a/pkg/apis/pipeline/v1beta1/param_types.go b/pkg/apis/pipeline/v1beta1/param_types.go index c703fcd5a4d..67dd87aa7ca 100644 --- a/pkg/apis/pipeline/v1beta1/param_types.go +++ b/pkg/apis/pipeline/v1beta1/param_types.go @@ -22,6 +22,7 @@ import ( "fmt" "strings" + "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/substitution" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" @@ -53,6 +54,10 @@ type ParamSpec struct { // parameter. // +optional Default *ParamValue `json:"default,omitempty"` + // Enum declares a set of allowed param input values for tasks/pipelines that can be validated. + // If Enum is not set, no input validation is performed for the param. + // +optional + Enum []string `json:"enum,omitempty"` } // ParamSpecs is a list of ParamSpec @@ -123,22 +128,47 @@ func (ps ParamSpecs) sortByType() (ParamSpecs, ParamSpecs, ParamSpecs) { // validateNoDuplicateNames returns an error if any of the params have the same name func (ps ParamSpecs) validateNoDuplicateNames() *apis.FieldError { + var errs *apis.FieldError names := ps.getNames() - seen := sets.String{} - dups := sets.String{} + for dup := range findDups(names) { + errs = errs.Also(apis.ErrGeneric("parameter appears more than once", "").ViaFieldKey("params", dup)) + } + return errs +} + +// validateParamEnum validates feature flag, duplication and allowed types for Param Enum +func (ps ParamSpecs) validateParamEnums(ctx context.Context) *apis.FieldError { var errs *apis.FieldError - for _, n := range names { - if seen.Has(n) { - dups.Insert(n) + for _, p := range ps { + if len(p.Enum) == 0 { + continue + } + if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableParamEnum { + errs = errs.Also(errs, apis.ErrGeneric(fmt.Sprintf("feature flag `%s` should be set to true to use Enum", config.EnableParamEnum), "").ViaFieldKey("params", p.Name)) + } + if p.Type != ParamTypeString { + errs = errs.Also(apis.ErrGeneric("enum can only be set with string type param", "").ViaFieldKey("params", p.Name)) + } + for dup := range findDups(p.Enum) { + errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("parameter enum value %v appears more than once", dup), "").ViaFieldKey("params", p.Name)) } - seen.Insert(n) - } - for n := range dups { - errs = errs.Also(apis.ErrGeneric("parameter appears more than once", "").ViaFieldKey("params", n)) } return errs } +// findDups returns the duplicate element in the given slice +func findDups(vals []string) sets.String { + seen := sets.String{} + dups := sets.String{} + for _, val := range vals { + if seen.Has(val) { + dups.Insert(val) + } + seen.Insert(val) + } + return dups +} + // setDefaultsForProperties sets default type for PropertySpec (string) if it's not specified func (pp *ParamSpec) setDefaultsForProperties() { for key, propertySpec := range pp.Properties { diff --git a/pkg/apis/pipeline/v1beta1/pipeline_conversion_test.go b/pkg/apis/pipeline/v1beta1/pipeline_conversion_test.go index 3b5939ccce3..23b66eabb1d 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_conversion_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_conversion_test.go @@ -69,6 +69,7 @@ func TestPipelineConversion(t *testing.T) { Params: []v1beta1.ParamSpec{{ Name: "param-1", Type: v1beta1.ParamTypeString, + Enum: []string{"v1", "v2"}, Description: "My first param", }}, }, diff --git a/pkg/apis/pipeline/v1beta1/pipeline_types_test.go b/pkg/apis/pipeline/v1beta1/pipeline_types_test.go index 7b58610f61e..d0dc19b4cdf 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_types_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_types_test.go @@ -541,58 +541,69 @@ func TestPipelineTask_ValidateBundle_Failure(t *testing.T) { func TestPipelineTask_ValidateRegularTask_Success(t *testing.T) { tests := []struct { - name string - tasks PipelineTask - enableAPIFields string - enableBundles bool + name string + tasks PipelineTask + configMap map[string]string }{{ name: "pipeline task - valid taskRef name", tasks: PipelineTask{ Name: "foo", TaskRef: &TaskRef{Name: "example.com/my-foo-task"}, }, - enableAPIFields: "stable", + configMap: map[string]string{"enable-api-fields": "stable"}, }, { name: "pipeline task - valid taskSpec", tasks: PipelineTask{ Name: "foo", TaskSpec: &EmbeddedTask{TaskSpec: getTaskSpec()}, }, - enableAPIFields: "stable", + configMap: map[string]string{"enable-api-fields": "stable"}, + }, { + name: "pipeline task - valid taskSpec with param enum", + tasks: PipelineTask{ + Name: "foo", + TaskSpec: &EmbeddedTask{ + TaskSpec: TaskSpec{ + Steps: []Step{ + { + Name: "foo", + Image: "bar", + }, + }, + Params: []ParamSpec{ + { + Name: "param1", + Type: ParamTypeString, + Enum: []string{"v1", "v2"}, + }, + }, + }, + }, + }, + configMap: map[string]string{"enable-param-enum": "true"}, }, { name: "pipeline task - use of resolver", tasks: PipelineTask{ TaskRef: &TaskRef{ResolverRef: ResolverRef{Resolver: "bar"}}, }, - enableAPIFields: "beta", + configMap: map[string]string{"enable-api-fields": "beta"}, }, { name: "pipeline task - use of params", tasks: PipelineTask{ TaskRef: &TaskRef{ResolverRef: ResolverRef{Resolver: "bar", Params: Params{}}}, }, - enableAPIFields: "beta", + configMap: map[string]string{"enable-api-fields": "beta"}, }, { name: "pipeline task - use of bundle with the feature flag set", tasks: PipelineTask{ Name: "foo", TaskRef: &TaskRef{Name: "bar", Bundle: "docker.io/foo"}, }, - enableBundles: true, - enableAPIFields: "stable", + configMap: map[string]string{"enable-tekton-oci-bundles": "true"}, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ctx := context.Background() - cfg := &config.Config{ - FeatureFlags: &config.FeatureFlags{}, - } - if tt.enableAPIFields != "" { - cfg.FeatureFlags.EnableAPIFields = tt.enableAPIFields - } - if tt.enableBundles { - cfg.FeatureFlags.EnableTektonOCIBundles = true - } - ctx = config.ToContext(ctx, cfg) + ctx := cfgtesting.SetFeatureFlags(context.Background(), t, tt.configMap) err := tt.tasks.validateTask(ctx) if err != nil { t.Errorf("PipelineTask.validateTask() returned error for valid pipeline task: %v", err) diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go index bf384b1c815..526e60c094a 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -452,11 +452,12 @@ func validatePipelineTasksWorkspacesUsage(wss []PipelineWorkspaceDeclaration, pt // ValidatePipelineParameterVariables validates parameters with those specified by each pipeline task, // (1) it validates the type of parameter is either string or array (2) parameter default value matches -// with the type of that param +// with the type of that param (3) no duplicateion, feature flag and allowed param type when using param enum func ValidatePipelineParameterVariables(ctx context.Context, tasks []PipelineTask, params ParamSpecs) (errs *apis.FieldError) { // validates all the types within a slice of ParamSpecs errs = errs.Also(ValidateParameterTypes(ctx, params).ViaField("params")) errs = errs.Also(params.validateNoDuplicateNames()) + errs = errs.Also(params.validateParamEnums(ctx)) for i, task := range tasks { errs = errs.Also(task.Params.validateDuplicateParameters().ViaField("params").ViaIndex(i)) } diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go index 00ceb53d210..3de8912aa1f 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go @@ -1338,9 +1338,10 @@ func TestFinallyTaskResultsToPipelineResults_Failure(t *testing.T) { func TestValidatePipelineParameterVariables_Success(t *testing.T) { tests := []struct { - name string - params []ParamSpec - tasks []PipelineTask + name string + params []ParamSpec + tasks []PipelineTask + configMap map[string]string }{{ name: "valid string parameter variables", params: []ParamSpec{{ @@ -1355,6 +1356,21 @@ func TestValidatePipelineParameterVariables_Success(t *testing.T) { Name: "a-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.baz) and $(params.foo-is-baz)"}, }}, }}, + }, { + name: "valid string parameter variables with enum", + params: []ParamSpec{{ + Name: "baz", Type: ParamTypeString, Enum: []string{"v1", "v2"}, + }, { + Name: "foo-is-baz", Type: ParamTypeString, + }}, + tasks: []PipelineTask{{ + Name: "bar", + TaskRef: &TaskRef{Name: "bar-task"}, + Params: Params{{ + Name: "a-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.baz) and $(params.foo-is-baz)"}, + }}, + }}, + configMap: map[string]string{"enable-param-enum": "true"}, }, { name: "valid string parameter variables in when expression", params: []ParamSpec{{ @@ -1622,7 +1638,11 @@ func TestValidatePipelineParameterVariables_Success(t *testing.T) { }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := ValidatePipelineParameterVariables(context.Background(), tt.tasks, tt.params) + ctx := context.Background() + if tt.configMap != nil { + ctx = cfgtesting.SetFeatureFlags(ctx, t, tt.configMap) + } + err := ValidatePipelineParameterVariables(ctx, tt.tasks, tt.params) if err != nil { t.Errorf("Pipeline.ValidatePipelineParameterVariables() returned error for valid pipeline parameters: %v", err) } @@ -1963,7 +1983,7 @@ func TestValidatePipelineDeclaredParameterUsage_Failure(t *testing.T) { t.Run(tt.name, func(t *testing.T) { err := validatePipelineTaskParameterUsage(tt.tasks, tt.params) if err == nil { - t.Errorf("Pipeline.ValidatePipelineParameterVariables() did not return error for invalid pipeline parameters") + t.Errorf("Pipeline.validatePipelineTaskParameterUsage() did not return error for invalid pipeline parameters") } if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" { t.Errorf("PipelineSpec.Validate() errors diff %s", diff.PrintWantGot(d)) @@ -1978,175 +1998,240 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) { params []ParamSpec tasks []PipelineTask expectedError apis.FieldError - }{{ - name: "invalid parameter type", - params: []ParamSpec{{ - Name: "foo", Type: "invalidtype", - }}, - tasks: []PipelineTask{{ - Name: "foo", - TaskRef: &TaskRef{Name: "foo-task"}, - }}, - expectedError: apis.FieldError{ - Message: `invalid value: invalidtype`, - Paths: []string{"params.foo.type"}, - }, - }, { - name: "array parameter mismatching default type", - params: []ParamSpec{{ - Name: "foo", Type: ParamTypeArray, Default: &ParamValue{Type: ParamTypeString, StringVal: "astring"}, - }}, - tasks: []PipelineTask{{ - Name: "foo", - TaskRef: &TaskRef{Name: "foo-task"}, - }}, - expectedError: apis.FieldError{ - Message: `"array" type does not match default value's type: "string"`, - Paths: []string{"params.foo.default.type", "params.foo.type"}, - }, - }, { - name: "string parameter mismatching default type", - params: []ParamSpec{{ - Name: "foo", Type: ParamTypeString, Default: &ParamValue{Type: ParamTypeArray, ArrayVal: []string{"anarray", "elements"}}, - }}, - tasks: []PipelineTask{{ - Name: "foo", - TaskRef: &TaskRef{Name: "foo-task"}, - }}, - expectedError: apis.FieldError{ - Message: `"string" type does not match default value's type: "array"`, - Paths: []string{"params.foo.default.type", "params.foo.type"}, - }, - }, { - name: "array parameter used as string", - params: []ParamSpec{{ - Name: "baz", Type: ParamTypeString, Default: &ParamValue{Type: ParamTypeArray, ArrayVal: []string{"anarray", "elements"}}, - }}, - tasks: []PipelineTask{{ - Name: "bar", - TaskRef: &TaskRef{Name: "bar-task"}, - Params: Params{{ - Name: "a-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.baz)"}, + configMap map[string]string + }{ + {name: "param enum with array type - failure", + params: []ParamSpec{{ + Name: "param2", + Type: ParamTypeArray, + Enum: []string{"v1", "v2"}, + }}, + tasks: []PipelineTask{{ + Name: "foo", + TaskRef: &TaskRef{Name: "foo-task"}, }}, - }}, - expectedError: apis.FieldError{ - Message: `"string" type does not match default value's type: "array"`, - Paths: []string{"params.baz.default.type", "params.baz.type"}, - }, - }, { - name: "star array parameter used as string", - params: []ParamSpec{{ - Name: "baz", Type: ParamTypeString, Default: &ParamValue{Type: ParamTypeArray, ArrayVal: []string{"anarray", "elements"}}, - }}, - tasks: []PipelineTask{{ - Name: "bar", - TaskRef: &TaskRef{Name: "bar-task"}, - Params: Params{{ - Name: "a-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.baz[*])"}, + configMap: map[string]string{ + "enable-param-enum": "true", + }, + expectedError: apis.FieldError{ + Message: `enum can only be set with string type param`, + Paths: []string{"params[param2]"}, + }, + }, { + name: "param enum with object type - failure", + params: []ParamSpec{{ + Name: "param2", + Type: ParamTypeObject, + Enum: []string{"v1", "v2"}, }}, - }}, - expectedError: apis.FieldError{ - Message: `"string" type does not match default value's type: "array"`, - Paths: []string{"params.baz.default.type", "params.baz.type"}, - }, - }, { - name: "array parameter string template not isolated", - params: []ParamSpec{{ - Name: "baz", Type: ParamTypeString, Default: &ParamValue{Type: ParamTypeArray, ArrayVal: []string{"anarray", "elements"}}, - }}, - tasks: []PipelineTask{{ - Name: "bar", - TaskRef: &TaskRef{Name: "bar-task"}, - Params: Params{{ - Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"value: $(params.baz)", "last"}}, + tasks: []PipelineTask{{ + Name: "foo", + TaskRef: &TaskRef{Name: "foo-task"}, }}, - }}, - expectedError: apis.FieldError{ - Message: `"string" type does not match default value's type: "array"`, - Paths: []string{"params.baz.default.type", "params.baz.type"}, - }, - }, { - name: "star array parameter string template not isolated", - params: []ParamSpec{{ - Name: "baz", Type: ParamTypeString, Default: &ParamValue{Type: ParamTypeArray, ArrayVal: []string{"anarray", "elements"}}, - }}, - tasks: []PipelineTask{{ - Name: "bar", - TaskRef: &TaskRef{Name: "bar-task"}, - Params: Params{{ - Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"value: $(params.baz[*])", "last"}}, + configMap: map[string]string{ + "enable-param-enum": "true", + }, + expectedError: apis.FieldError{ + Message: `enum can only be set with string type param`, + Paths: []string{"params[param2]"}, + }, + }, { + name: "param enum with duplicate values - failure", + params: []ParamSpec{{ + Name: "param1", + Type: ParamTypeString, + Enum: []string{"v1", "v1", "v2"}, + }}, + configMap: map[string]string{ + "enable-param-enum": "true", + }, + expectedError: apis.FieldError{ + Message: `parameter enum value v1 appears more than once`, + Paths: []string{"params[param1]"}, + }, + }, { + name: "param enum with feature flag disabled - failure", + params: []ParamSpec{{ + Name: "param1", + Type: ParamTypeString, + Enum: []string{"v1", "v2"}, + }}, + expectedError: apis.FieldError{ + Message: "feature flag `enable-param-enum` should be set to true to use Enum", + Paths: []string{"params[param1]"}, + }, + }, { + name: "invalid parameter type", + params: []ParamSpec{{ + Name: "foo", Type: "invalidtype", }}, - }}, - expectedError: apis.FieldError{ - Message: `"string" type does not match default value's type: "array"`, - Paths: []string{"params.baz.default.type", "params.baz.type"}, - }, - }, { - name: "multiple string parameters with the same name", - params: []ParamSpec{{ - Name: "baz", Type: ParamTypeString, + tasks: []PipelineTask{{ + Name: "foo", + TaskRef: &TaskRef{Name: "foo-task"}, + }}, + expectedError: apis.FieldError{ + Message: `invalid value: invalidtype`, + Paths: []string{"params.foo.type"}, + }, }, { - Name: "baz", Type: ParamTypeString, - }}, - tasks: []PipelineTask{{ - Name: "foo", - TaskRef: &TaskRef{Name: "foo-task"}, - }}, - expectedError: apis.FieldError{ - Message: `parameter appears more than once`, - Paths: []string{"params[baz]"}, - }, - }, { - name: "multiple array parameters with the same name", - params: []ParamSpec{{ - Name: "baz", Type: ParamTypeArray, + name: "array parameter mismatching default type", + params: []ParamSpec{{ + Name: "foo", Type: ParamTypeArray, Default: &ParamValue{Type: ParamTypeString, StringVal: "astring"}, + }}, + tasks: []PipelineTask{{ + Name: "foo", + TaskRef: &TaskRef{Name: "foo-task"}, + }}, + expectedError: apis.FieldError{ + Message: `"array" type does not match default value's type: "string"`, + Paths: []string{"params.foo.default.type", "params.foo.type"}, + }, }, { - Name: "baz", Type: ParamTypeArray, - }}, - tasks: []PipelineTask{{ - Name: "foo", - TaskRef: &TaskRef{Name: "foo-task"}, - }}, - expectedError: apis.FieldError{ - Message: `parameter appears more than once`, - Paths: []string{"params[baz]"}, - }, - }, { - name: "multiple different type parameters with the same name", - params: []ParamSpec{{ - Name: "baz", Type: ParamTypeArray, + name: "string parameter mismatching default type", + params: []ParamSpec{{ + Name: "foo", Type: ParamTypeString, Default: &ParamValue{Type: ParamTypeArray, ArrayVal: []string{"anarray", "elements"}}, + }}, + tasks: []PipelineTask{{ + Name: "foo", + TaskRef: &TaskRef{Name: "foo-task"}, + }}, + expectedError: apis.FieldError{ + Message: `"string" type does not match default value's type: "array"`, + Paths: []string{"params.foo.default.type", "params.foo.type"}, + }, }, { - Name: "baz", Type: ParamTypeString, - }}, - tasks: []PipelineTask{{ - Name: "foo", - TaskRef: &TaskRef{Name: "foo-task"}, - }}, - expectedError: apis.FieldError{ - Message: `parameter appears more than once`, - Paths: []string{"params[baz]"}, - }, - }, { - name: "invalid task use duplicate parameters", - tasks: []PipelineTask{{ - Name: "foo-task", - TaskRef: &TaskRef{Name: "foo-task"}, - Params: Params{{ - Name: "duplicate-param", Value: ParamValue{Type: ParamTypeString, StringVal: "val1"}, + name: "array parameter used as string", + params: []ParamSpec{{ + Name: "baz", Type: ParamTypeString, Default: &ParamValue{Type: ParamTypeArray, ArrayVal: []string{"anarray", "elements"}}, + }}, + tasks: []PipelineTask{{ + Name: "bar", + TaskRef: &TaskRef{Name: "bar-task"}, + Params: Params{{ + Name: "a-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.baz)"}, + }}, + }}, + expectedError: apis.FieldError{ + Message: `"string" type does not match default value's type: "array"`, + Paths: []string{"params.baz.default.type", "params.baz.type"}, + }, + }, { + name: "star array parameter used as string", + params: []ParamSpec{{ + Name: "baz", Type: ParamTypeString, Default: &ParamValue{Type: ParamTypeArray, ArrayVal: []string{"anarray", "elements"}}, + }}, + tasks: []PipelineTask{{ + Name: "bar", + TaskRef: &TaskRef{Name: "bar-task"}, + Params: Params{{ + Name: "a-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.baz[*])"}, + }}, + }}, + expectedError: apis.FieldError{ + Message: `"string" type does not match default value's type: "array"`, + Paths: []string{"params.baz.default.type", "params.baz.type"}, + }, + }, { + name: "array parameter string template not isolated", + params: []ParamSpec{{ + Name: "baz", Type: ParamTypeString, Default: &ParamValue{Type: ParamTypeArray, ArrayVal: []string{"anarray", "elements"}}, + }}, + tasks: []PipelineTask{{ + Name: "bar", + TaskRef: &TaskRef{Name: "bar-task"}, + Params: Params{{ + Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"value: $(params.baz)", "last"}}, + }}, + }}, + expectedError: apis.FieldError{ + Message: `"string" type does not match default value's type: "array"`, + Paths: []string{"params.baz.default.type", "params.baz.type"}, + }, + }, { + name: "star array parameter string template not isolated", + params: []ParamSpec{{ + Name: "baz", Type: ParamTypeString, Default: &ParamValue{Type: ParamTypeArray, ArrayVal: []string{"anarray", "elements"}}, + }}, + tasks: []PipelineTask{{ + Name: "bar", + TaskRef: &TaskRef{Name: "bar-task"}, + Params: Params{{ + Name: "a-param", Value: ParamValue{Type: ParamTypeArray, ArrayVal: []string{"value: $(params.baz[*])", "last"}}, + }}, + }}, + expectedError: apis.FieldError{ + Message: `"string" type does not match default value's type: "array"`, + Paths: []string{"params.baz.default.type", "params.baz.type"}, + }, + }, { + name: "multiple string parameters with the same name", + params: []ParamSpec{{ + Name: "baz", Type: ParamTypeString, }, { - Name: "duplicate-param", Value: ParamValue{Type: ParamTypeString, StringVal: "val2"}, + Name: "baz", Type: ParamTypeString, + }}, + tasks: []PipelineTask{{ + Name: "foo", + TaskRef: &TaskRef{Name: "foo-task"}, + }}, + expectedError: apis.FieldError{ + Message: `parameter appears more than once`, + Paths: []string{"params[baz]"}, + }, + }, { + name: "multiple array parameters with the same name", + params: []ParamSpec{{ + Name: "baz", Type: ParamTypeArray, }, { - Name: "duplicate-param", Value: ParamValue{Type: ParamTypeString, StringVal: "val3"}, + Name: "baz", Type: ParamTypeArray, }}, - }}, - expectedError: apis.FieldError{ - Message: `parameter names must be unique, the parameter "duplicate-param" is also defined at`, - Paths: []string{"[0].params[1].name, [0].params[2].name"}, - }, - }} + tasks: []PipelineTask{{ + Name: "foo", + TaskRef: &TaskRef{Name: "foo-task"}, + }}, + expectedError: apis.FieldError{ + Message: `parameter appears more than once`, + Paths: []string{"params[baz]"}, + }, + }, { + name: "multiple different type parameters with the same name", + params: []ParamSpec{{ + Name: "baz", Type: ParamTypeArray, + }, { + Name: "baz", Type: ParamTypeString, + }}, + tasks: []PipelineTask{{ + Name: "foo", + TaskRef: &TaskRef{Name: "foo-task"}, + }}, + expectedError: apis.FieldError{ + Message: `parameter appears more than once`, + Paths: []string{"params[baz]"}, + }, + }, { + name: "invalid task use duplicate parameters", + tasks: []PipelineTask{{ + Name: "foo-task", + TaskRef: &TaskRef{Name: "foo-task"}, + Params: Params{{ + Name: "duplicate-param", Value: ParamValue{Type: ParamTypeString, StringVal: "val1"}, + }, { + Name: "duplicate-param", Value: ParamValue{Type: ParamTypeString, StringVal: "val2"}, + }, { + Name: "duplicate-param", Value: ParamValue{Type: ParamTypeString, StringVal: "val3"}, + }}, + }}, + expectedError: apis.FieldError{ + Message: `parameter names must be unique, the parameter "duplicate-param" is also defined at`, + Paths: []string{"[0].params[1].name, [0].params[2].name"}, + }, + }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ctx := context.Background() + if tt.configMap != nil { + ctx = cfgtesting.SetFeatureFlags(ctx, t, tt.configMap) + } err := ValidatePipelineParameterVariables(ctx, tt.tasks, tt.params) if err == nil { t.Errorf("Pipeline.ValidatePipelineParameterVariables() did not return error for invalid pipeline parameters") diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json index 1392a89640e..5fc165feec6 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -624,6 +624,14 @@ "description": "Description is a user-facing description of the parameter that may be used to populate a UI.", "type": "string" }, + "enum": { + "description": "Enum declares a set of allowed param input values for tasks/pipelines that can be validated. If Enum is not set, no input validation is performed for the param.", + "type": "array", + "items": { + "type": "string", + "default": "" + } + }, "name": { "description": "Name declares the name by which a parameter is referenced.", "type": "string", diff --git a/pkg/apis/pipeline/v1beta1/task_conversion_test.go b/pkg/apis/pipeline/v1beta1/task_conversion_test.go index 1136be3d78b..9be1e4b997b 100644 --- a/pkg/apis/pipeline/v1beta1/task_conversion_test.go +++ b/pkg/apis/pipeline/v1beta1/task_conversion_test.go @@ -56,6 +56,7 @@ spec: params: - name: param-1 type: string + enum: ["v1", "v2"] description: my first param results: - name: result-1 diff --git a/pkg/apis/pipeline/v1beta1/task_validation.go b/pkg/apis/pipeline/v1beta1/task_validation.go index a4a38800fe7..901591263a2 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation.go +++ b/pkg/apis/pipeline/v1beta1/task_validation.go @@ -441,6 +441,7 @@ func (p ParamSpec) ValidateObjectType(ctx context.Context) *apis.FieldError { func ValidateParameterVariables(ctx context.Context, steps []Step, params ParamSpecs) *apis.FieldError { var errs *apis.FieldError errs = errs.Also(params.validateNoDuplicateNames()) + errs = errs.Also(params.validateParamEnums(ctx)) stringParams, arrayParams, objectParams := params.sortByType() stringParameterNames := sets.NewString(stringParams.getNames()...) arrayParameterNames := sets.NewString(arrayParams.getNames()...) diff --git a/pkg/apis/pipeline/v1beta1/task_validation_test.go b/pkg/apis/pipeline/v1beta1/task_validation_test.go index 962abf02ef6..9022f40cfbe 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/task_validation_test.go @@ -2090,3 +2090,86 @@ func TestGetArrayIndexParamRefs(t *testing.T) { }) } } + +func TestParamEnum(t *testing.T) { + tcs := []struct { + name string + params v1beta1.ParamSpecs + configMap map[string]string + expectedErr error + }{{ + name: "valid param enum - success", + params: []v1beta1.ParamSpec{{ + Name: "param1", + Type: v1beta1.ParamTypeString, + Enum: []string{"v1", "v2"}, + }, { + Name: "param2", + Type: v1beta1.ParamTypeString, + Enum: []string{"v1", "v2"}, + }}, + configMap: map[string]string{ + "enable-param-enum": "true", + }, + }, { + name: "param enum with array type - failure", + params: []v1beta1.ParamSpec{{ + Name: "param1", + Type: v1beta1.ParamTypeArray, + Enum: []string{"v1", "v2"}, + }}, + configMap: map[string]string{ + "enable-param-enum": "true", + }, + expectedErr: fmt.Errorf("enum can only be set with string type param: params[param1]"), + }, { + name: "param enum with object type - failure", + params: []v1beta1.ParamSpec{{ + Name: "param1", + Type: v1beta1.ParamTypeObject, + Enum: []string{"v1", "v2"}, + }}, + configMap: map[string]string{ + "enable-param-enum": "true", + }, + expectedErr: fmt.Errorf("enum can only be set with string type param: params[param1]"), + }, { + name: "param enum with duplicate values - failure", + params: []v1beta1.ParamSpec{{ + Name: "param1", + Type: v1beta1.ParamTypeString, + Enum: []string{"v1", "v1", "v2"}, + }}, + configMap: map[string]string{ + "enable-param-enum": "true", + }, + expectedErr: fmt.Errorf("parameter enum value v1 appears more than once: params[param1]"), + }, { + name: "param enum with feature flag disabled - failure", + params: []v1beta1.ParamSpec{{ + Name: "param1", + Type: v1beta1.ParamTypeString, + Enum: []string{"v1", "v2"}, + }}, + configMap: map[string]string{ + "enable-param-enum": "false", + }, + expectedErr: fmt.Errorf("feature flag `enable-param-enum` should be set to true to use Enum: params[param1]"), + }} + + for _, tc := range tcs { + ctx := cfgtesting.SetFeatureFlags(context.Background(), t, tc.configMap) + + err := v1beta1.ValidateParameterVariables(ctx, []v1beta1.Step{{Image: "foo"}}, tc.params) + + if tc.expectedErr == nil && err != nil { + t.Errorf("No error expected from ValidateParameterVariables() but got = %v", err) + } else if tc.expectedErr != nil { + if err == nil { + t.Errorf("Expected error from ValidateParameterVariables() = %v, but got none", tc.expectedErr) + } else if d := cmp.Diff(tc.expectedErr.Error(), err.Error()); d != "" { + t.Errorf("Returned error from ValidateParameterVariables() does not match with the expected error: %s", diff.PrintWantGot(d)) + } + } + } +} diff --git a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go index db21dad9f79..e5a5987f606 100644 --- a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go @@ -515,6 +515,11 @@ func (in *ParamSpec) DeepCopyInto(out *ParamSpec) { *out = new(ParamValue) (*in).DeepCopyInto(*out) } + if in.Enum != nil { + in, out := &in.Enum, &out.Enum + *out = make([]string, len(*in)) + copy(*out, *in) + } return }