Skip to content

Commit 9eadaf3

Browse files
QuanZhang-WilliamQuanZhang-William
committed
[TEP-0144] Add enum API field
Part of [tektoncd#7270][tektoncd#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 [tektoncd#7270]: tektoncd#7270 [tep-0144]: https://github.com/tektoncd/community/blob/main/teps/0144-param-enum.md
1 parent ae17f1c commit 9eadaf3

23 files changed

+776
-223
lines changed

‎docs/pipeline-api.md

+26
Original file line numberDiff line numberDiff line change
@@ -1691,6 +1691,19 @@ default is set, a Task may be executed without a supplied value for the
16911691
parameter.</p>
16921692
</td>
16931693
</tr>
1694+
<tr>
1695+
<td>
1696+
<code>enum</code><br/>
1697+
<em>
1698+
[]string
1699+
</em>
1700+
</td>
1701+
<td>
1702+
<em>(Optional)</em>
1703+
<p>Enum declares a set of allowed param input values for tasks/pipelines that can be validated.
1704+
If Enum is not set, no input validation is performed for the param.</p>
1705+
</td>
1706+
</tr>
16941707
</tbody>
16951708
</table>
16961709
<h3 id="tekton.dev/v1.ParamSpecs">ParamSpecs
@@ -9963,6 +9976,19 @@ default is set, a Task may be executed without a supplied value for the
99639976
parameter.</p>
99649977
</td>
99659978
</tr>
9979+
<tr>
9980+
<td>
9981+
<code>enum</code><br/>
9982+
<em>
9983+
[]string
9984+
</em>
9985+
</td>
9986+
<td>
9987+
<em>(Optional)</em>
9988+
<p>Enum declares a set of allowed param input values for tasks/pipelines that can be validated.
9989+
If Enum is not set, no input validation is performed for the param.</p>
9990+
</td>
9991+
</tr>
99669992
</tbody>
99679993
</table>
99689994
<h3 id="tekton.dev/v1beta1.ParamSpecs">ParamSpecs

‎hack/ignored-openapi-violations.list

+2
Original file line numberDiff line numberDiff line change
@@ -41,3 +41,5 @@ API rule violation: names_match,github.com/tektoncd/pipeline/pkg/apis/resolution
4141
API rule violation: list_type_missing,github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1,RunSpec,Workspaces
4242
API rule violation: list_type_missing,github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1,VerificationPolicySpec,Authorities
4343
API rule violation: list_type_missing,github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1,VerificationPolicySpec,Resources
44+
API rule violation: list_type_missing,github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1,ParamSpec,Enum
45+
API rule violation: list_type_missing,github.com/tektoncd/pipeline/pkg/apis/pipeline/v1,ParamSpec,Enum

‎pkg/apis/pipeline/v1/openapi_generated.go

+15
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/apis/pipeline/v1/param_types.go

+39-9
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"fmt"
2323
"strings"
2424

25+
"github.com/tektoncd/pipeline/pkg/apis/config"
2526
"github.com/tektoncd/pipeline/pkg/substitution"
2627
corev1 "k8s.io/api/core/v1"
2728
"k8s.io/apimachinery/pkg/util/sets"
@@ -53,6 +54,10 @@ type ParamSpec struct {
5354
// parameter.
5455
// +optional
5556
Default *ParamValue `json:"default,omitempty"`
57+
// Enum declares a set of allowed param input values for tasks/pipelines that can be validated.
58+
// If Enum is not set, no input validation is performed for the param.
59+
// +optional
60+
Enum []string `json:"enum,omitempty"`
5661
}
5762

5863
// ParamSpecs is a list of ParamSpec
@@ -132,22 +137,47 @@ func (ps ParamSpecs) sortByType() (ParamSpecs, ParamSpecs, ParamSpecs) {
132137

133138
// validateNoDuplicateNames returns an error if any of the params have the same name
134139
func (ps ParamSpecs) validateNoDuplicateNames() *apis.FieldError {
140+
var errs *apis.FieldError
135141
names := ps.getNames()
136-
seen := sets.String{}
137-
dups := sets.String{}
142+
for dup := range findDups(names) {
143+
errs = errs.Also(apis.ErrGeneric("parameter appears more than once", "").ViaFieldKey("params", dup))
144+
}
145+
return errs
146+
}
147+
148+
// validateParamEnum validates feature flag, duplication and allowed types for Param Enum
149+
func (ps ParamSpecs) validateParamEnums(ctx context.Context) *apis.FieldError {
138150
var errs *apis.FieldError
139-
for _, n := range names {
140-
if seen.Has(n) {
141-
dups.Insert(n)
151+
for _, p := range ps {
152+
if len(p.Enum) == 0 {
153+
continue
154+
}
155+
if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableParamEnum {
156+
errs = errs.Also(errs, apis.ErrGeneric(fmt.Sprintf("feature flag `%s` should be set to true to use Enum", config.EnableParamEnum), "").ViaKey(p.Name))
157+
}
158+
if p.Type != ParamTypeString {
159+
errs = errs.Also(apis.ErrGeneric("enum can only be set with string type param", "").ViaKey(p.Name))
160+
}
161+
for dup := range findDups(p.Enum) {
162+
errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("parameter enum value %v appears more than once", dup), "").ViaKey(p.Name))
142163
}
143-
seen.Insert(n)
144-
}
145-
for n := range dups {
146-
errs = errs.Also(apis.ErrGeneric("parameter appears more than once", "").ViaFieldKey("params", n))
147164
}
148165
return errs
149166
}
150167

168+
// findDups returns the duplicate element in the given slice
169+
func findDups(vals []string) sets.String {
170+
seen := sets.String{}
171+
dups := sets.String{}
172+
for _, val := range vals {
173+
if seen.Has(val) {
174+
dups.Insert(val)
175+
}
176+
seen.Insert(val)
177+
}
178+
return dups
179+
}
180+
151181
// Param declares an ParamValues to use for the parameter called name.
152182
type Param struct {
153183
Name string `json:"name"`

‎pkg/apis/pipeline/v1/pipeline_types_test.go

+30-17
Original file line numberDiff line numberDiff line change
@@ -508,10 +508,9 @@ func TestPipelineTask_ValidateCustomTask(t *testing.T) {
508508

509509
func TestPipelineTask_ValidateRegularTask_Success(t *testing.T) {
510510
tests := []struct {
511-
name string
512-
tasks PipelineTask
513-
enableAlphaAPIFields bool
514-
enableBetaAPIFields bool
511+
name string
512+
tasks PipelineTask
513+
configMap map[string]string
515514
}{{
516515
name: "pipeline task - valid taskRef name",
517516
tasks: PipelineTask{
@@ -524,37 +523,51 @@ func TestPipelineTask_ValidateRegularTask_Success(t *testing.T) {
524523
Name: "foo",
525524
TaskSpec: &EmbeddedTask{TaskSpec: getTaskSpec()},
526525
},
526+
}, {
527+
name: "pipeline task - valid taskSpec with param enum",
528+
tasks: PipelineTask{
529+
Name: "foo",
530+
TaskSpec: &EmbeddedTask{
531+
TaskSpec: TaskSpec{
532+
Steps: []Step{
533+
{
534+
Name: "foo",
535+
Image: "bar",
536+
},
537+
},
538+
Params: []ParamSpec{
539+
{
540+
Name: "param1",
541+
Type: ParamTypeString,
542+
Enum: []string{"v1", "v2"},
543+
},
544+
},
545+
},
546+
},
547+
},
548+
configMap: map[string]string{"enable-param-enum": "true"},
527549
}, {
528550
name: "pipeline task - use of resolver with the feature flag set",
529551
tasks: PipelineTask{
530552
TaskRef: &TaskRef{ResolverRef: ResolverRef{Resolver: "bar"}},
531553
},
532-
enableBetaAPIFields: true,
554+
configMap: map[string]string{"enable-api-field": "beta"},
533555
}, {
534556
name: "pipeline task - use of resolver with the feature flag set to alpha",
535557
tasks: PipelineTask{
536558
TaskRef: &TaskRef{ResolverRef: ResolverRef{Resolver: "bar"}},
537559
},
538-
enableAlphaAPIFields: true,
560+
configMap: map[string]string{"enable-api-field": "alpha"},
539561
}, {
540562
name: "pipeline task - use of resolver params with the feature flag set",
541563
tasks: PipelineTask{
542564
TaskRef: &TaskRef{ResolverRef: ResolverRef{Resolver: "bar", Params: Params{{}}}},
543565
},
544-
enableBetaAPIFields: true,
566+
configMap: map[string]string{"enable-api-field": "beta"},
545567
}}
546568
for _, tt := range tests {
547569
t.Run(tt.name, func(t *testing.T) {
548-
ctx := context.Background()
549-
cfg := &config.Config{
550-
FeatureFlags: &config.FeatureFlags{},
551-
}
552-
if tt.enableAlphaAPIFields {
553-
cfg.FeatureFlags.EnableAPIFields = config.AlphaAPIFields
554-
} else if tt.enableBetaAPIFields {
555-
cfg.FeatureFlags.EnableAPIFields = config.BetaAPIFields
556-
}
557-
ctx = config.ToContext(ctx, cfg)
570+
ctx := cfgtesting.SetFeatureFlags(context.Background(), t, tt.configMap)
558571
err := tt.tasks.validateTask(ctx)
559572
if err != nil {
560573
t.Errorf("PipelineTask.validateTask() returned error for valid pipeline task: %v", err)

‎pkg/apis/pipeline/v1/pipeline_validation.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -433,11 +433,12 @@ func validatePipelineTasksWorkspacesUsage(wss []PipelineWorkspaceDeclaration, pt
433433

434434
// ValidatePipelineParameterVariables validates parameters with those specified by each pipeline task,
435435
// (1) it validates the type of parameter is either string or array (2) parameter default value matches
436-
// with the type of that param
436+
// with the type of that param (3) no duplication, feature flag and allowed param type when using param enum
437437
func ValidatePipelineParameterVariables(ctx context.Context, tasks []PipelineTask, params ParamSpecs) (errs *apis.FieldError) {
438438
// validates all the types within a slice of ParamSpecs
439439
errs = errs.Also(ValidateParameterTypes(ctx, params).ViaField("params"))
440440
errs = errs.Also(params.validateNoDuplicateNames())
441+
errs = errs.Also(params.validateParamEnums(ctx).ViaField("params"))
441442
for i, task := range tasks {
442443
errs = errs.Also(task.Params.validateDuplicateParameters().ViaField("params").ViaIndex(i))
443444
}

‎pkg/apis/pipeline/v1/pipeline_validation_test.go

+89-4
Original file line numberDiff line numberDiff line change
@@ -1295,9 +1295,10 @@ func TestFinallyTaskResultsToPipelineResults_Failure(t *testing.T) {
12951295

12961296
func TestValidatePipelineParameterVariables_Success(t *testing.T) {
12971297
tests := []struct {
1298-
name string
1299-
params []ParamSpec
1300-
tasks []PipelineTask
1298+
name string
1299+
params []ParamSpec
1300+
tasks []PipelineTask
1301+
configMap map[string]string
13011302
}{{
13021303
name: "valid string parameter variables",
13031304
params: []ParamSpec{{
@@ -1312,6 +1313,21 @@ func TestValidatePipelineParameterVariables_Success(t *testing.T) {
13121313
Name: "a-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.baz) and $(params.foo-is-baz)"},
13131314
}},
13141315
}},
1316+
}, {
1317+
name: "valid string parameter variables with enum",
1318+
params: []ParamSpec{{
1319+
Name: "baz", Type: ParamTypeString, Enum: []string{"v1", "v2"},
1320+
}, {
1321+
Name: "foo-is-baz", Type: ParamTypeString,
1322+
}},
1323+
tasks: []PipelineTask{{
1324+
Name: "bar",
1325+
TaskRef: &TaskRef{Name: "bar-task"},
1326+
Params: Params{{
1327+
Name: "a-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.baz) and $(params.foo-is-baz)"},
1328+
}},
1329+
}},
1330+
configMap: map[string]string{"enable-param-enum": "true"},
13151331
}, {
13161332
name: "valid string parameter variables in when expression",
13171333
params: []ParamSpec{{
@@ -1580,6 +1596,9 @@ func TestValidatePipelineParameterVariables_Success(t *testing.T) {
15801596
for _, tt := range tests {
15811597
t.Run(tt.name, func(t *testing.T) {
15821598
ctx := cfgtesting.EnableAlphaAPIFields(context.Background())
1599+
if tt.configMap != nil {
1600+
ctx = cfgtesting.SetFeatureFlags(ctx, t, tt.configMap)
1601+
}
15831602
err := ValidatePipelineParameterVariables(ctx, tt.tasks, tt.params)
15841603
if err != nil {
15851604
t.Errorf("Pipeline.ValidatePipelineParameterVariables() returned error for valid pipeline parameters: %v", err)
@@ -1921,7 +1940,7 @@ func TestValidatePipelineDeclaredParameterUsage_Failure(t *testing.T) {
19211940
t.Run(tt.name, func(t *testing.T) {
19221941
err := validatePipelineTaskParameterUsage(tt.tasks, tt.params)
19231942
if err == nil {
1924-
t.Errorf("Pipeline.ValidatePipelineParameterVariables() did not return error for invalid pipeline parameters")
1943+
t.Errorf("Pipeline.validatePipelineTaskParameterUsage() did not return error for invalid pipeline parameters")
19251944
}
19261945
if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" {
19271946
t.Errorf("PipelineSpec.Validate() errors diff %s", diff.PrintWantGot(d))
@@ -1936,7 +1955,69 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) {
19361955
params []ParamSpec
19371956
tasks []PipelineTask
19381957
expectedError apis.FieldError
1958+
configMap map[string]string
19391959
}{{
1960+
name: "param enum with array type - failure",
1961+
params: []ParamSpec{{
1962+
Name: "param1",
1963+
Type: ParamTypeArray,
1964+
Enum: []string{"v1", "v2"},
1965+
}},
1966+
tasks: []PipelineTask{{
1967+
Name: "foo",
1968+
TaskRef: &TaskRef{Name: "foo-task"},
1969+
}},
1970+
configMap: map[string]string{
1971+
"enable-param-enum": "true",
1972+
},
1973+
expectedError: apis.FieldError{
1974+
Message: `enum can only be set with string type param`,
1975+
Paths: []string{"params[param1]"},
1976+
},
1977+
}, {
1978+
name: "param enum with object type - failure",
1979+
params: []ParamSpec{{
1980+
Name: "param1",
1981+
Type: ParamTypeObject,
1982+
Enum: []string{"v1", "v2"},
1983+
}},
1984+
tasks: []PipelineTask{{
1985+
Name: "foo",
1986+
TaskRef: &TaskRef{Name: "foo-task"},
1987+
}},
1988+
configMap: map[string]string{
1989+
"enable-param-enum": "true",
1990+
},
1991+
expectedError: apis.FieldError{
1992+
Message: `enum can only be set with string type param`,
1993+
Paths: []string{"params[param1]"},
1994+
},
1995+
}, {
1996+
name: "param enum with duplicate values - failure",
1997+
params: []ParamSpec{{
1998+
Name: "param1",
1999+
Type: ParamTypeString,
2000+
Enum: []string{"v1", "v1", "v2"},
2001+
}},
2002+
configMap: map[string]string{
2003+
"enable-param-enum": "true",
2004+
},
2005+
expectedError: apis.FieldError{
2006+
Message: `parameter enum value v1 appears more than once`,
2007+
Paths: []string{"params[param1]"},
2008+
},
2009+
}, {
2010+
name: "param enum with feature flag disabled - failure",
2011+
params: []ParamSpec{{
2012+
Name: "param1",
2013+
Type: ParamTypeString,
2014+
Enum: []string{"v1", "v2"},
2015+
}},
2016+
expectedError: apis.FieldError{
2017+
Message: "feature flag `enable-param-enum` should be set to true to use Enum",
2018+
Paths: []string{"params[param1]"},
2019+
},
2020+
}, {
19402021
name: "invalid parameter type",
19412022
params: []ParamSpec{{
19422023
Name: "foo", Type: "invalidtype",
@@ -2105,6 +2186,10 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) {
21052186
for _, tt := range tests {
21062187
t.Run(tt.name, func(t *testing.T) {
21072188
ctx := context.Background()
2189+
if tt.configMap != nil {
2190+
ctx = cfgtesting.SetFeatureFlags(ctx, t, tt.configMap)
2191+
}
2192+
21082193
err := ValidatePipelineParameterVariables(ctx, tt.tasks, tt.params)
21092194
if err == nil {
21102195
t.Errorf("Pipeline.ValidatePipelineParameterVariables() did not return error for invalid pipeline parameters")

‎pkg/apis/pipeline/v1/swagger.json

+8
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,14 @@
343343
"description": "Description is a user-facing description of the parameter that may be used to populate a UI.",
344344
"type": "string"
345345
},
346+
"enum": {
347+
"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.",
348+
"type": "array",
349+
"items": {
350+
"type": "string",
351+
"default": ""
352+
}
353+
},
346354
"name": {
347355
"description": "Name declares the name by which a parameter is referenced.",
348356
"type": "string",

‎pkg/apis/pipeline/v1/task_validation.go

+1
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,7 @@ func (p ParamSpec) ValidateObjectType(ctx context.Context) *apis.FieldError {
435435
func ValidateParameterVariables(ctx context.Context, steps []Step, params ParamSpecs) *apis.FieldError {
436436
var errs *apis.FieldError
437437
errs = errs.Also(params.validateNoDuplicateNames())
438+
errs = errs.Also(params.validateParamEnums(ctx).ViaField("params"))
438439
stringParams, arrayParams, objectParams := params.sortByType()
439440
stringParameterNames := sets.NewString(stringParams.getNames()...)
440441
arrayParameterNames := sets.NewString(arrayParams.getNames()...)

0 commit comments

Comments
 (0)