Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[TEP-0144] Add enum API field #7289

Merged
merged 1 commit into from
Nov 2, 2023
Merged

[TEP-0144] Add enum API field #7289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions docs/pipeline-api.md
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop

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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading