Skip to content

Commit

Permalink
[TEP-0144] Add enum API field
Browse files Browse the repository at this point in the history
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]: #7270
[tep-0144]: https://github.com/tektoncd/community/blob/main/teps/0144-param-enum.md
  • Loading branch information
QuanZhang-William committed Nov 1, 2023
1 parent ae17f1c commit 9eadaf3
Show file tree
Hide file tree
Showing 23 changed files with 776 additions and 223 deletions.
26 changes: 26 additions & 0 deletions docs/pipeline-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1691,6 +1691,19 @@ default is set, a Task may be executed without a supplied value for the
parameter.</p>
</td>
</tr>
<tr>
<td>
<code>enum</code><br/>
<em>
[]string
</em>
</td>
<td>
<em>(Optional)</em>
<p>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.</p>
</td>
</tr>
</tbody>
</table>
<h3 id="tekton.dev/v1.ParamSpecs">ParamSpecs
Expand Down Expand Up @@ -9963,6 +9976,19 @@ default is set, a Task may be executed without a supplied value for the
parameter.</p>
</td>
</tr>
<tr>
<td>
<code>enum</code><br/>
<em>
[]string
</em>
</td>
<td>
<em>(Optional)</em>
<p>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.</p>
</td>
</tr>
</tbody>
</table>
<h3 id="tekton.dev/v1beta1.ParamSpecs">ParamSpecs
Expand Down
2 changes: 2 additions & 0 deletions hack/ignored-openapi-violations.list
Original file line number Diff line number Diff line change
Expand Up @@ -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
15 changes: 15 additions & 0 deletions pkg/apis/pipeline/v1/openapi_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

48 changes: 39 additions & 9 deletions pkg/apis/pipeline/v1/param_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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), "").ViaKey(p.Name))
}
if p.Type != ParamTypeString {
errs = errs.Also(apis.ErrGeneric("enum can only be set with string type param", "").ViaKey(p.Name))
}
for dup := range findDups(p.Enum) {
errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("parameter enum value %v appears more than once", dup), "").ViaKey(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"`
Expand Down
47 changes: 30 additions & 17 deletions pkg/apis/pipeline/v1/pipeline_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion pkg/apis/pipeline/v1/pipeline_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 duplication, 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).ViaField("params"))
for i, task := range tasks {
errs = errs.Also(task.Params.validateDuplicateParameters().ViaField("params").ViaIndex(i))
}
Expand Down
93 changes: 89 additions & 4 deletions pkg/apis/pipeline/v1/pipeline_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{{
Expand All @@ -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{{
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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))
Expand All @@ -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",
Expand Down Expand Up @@ -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")
Expand Down
8 changes: 8 additions & 0 deletions pkg/apis/pipeline/v1/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions pkg/apis/pipeline/v1/task_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).ViaField("params"))
stringParams, arrayParams, objectParams := params.sortByType()
stringParameterNames := sets.NewString(stringParams.getNames()...)
arrayParameterNames := sets.NewString(arrayParams.getNames()...)
Expand Down
Loading

0 comments on commit 9eadaf3

Please sign in to comment.