Skip to content

Commit

Permalink
Add validation for duplicated param names in TaskSpec
Browse files Browse the repository at this point in the history
Prior to this, when duplicated names for parameters were assigned in `taskSpec`, the param value would be overwritten by the last value.

To avoid this, a validation check was added to ensure that no param name is duplicated in TaskSpec.
  • Loading branch information
chitrangpatel committed May 9, 2022
1 parent e46259e commit 09e8c44
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 4 deletions.
14 changes: 10 additions & 4 deletions pkg/apis/pipeline/v1beta1/task_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func (ts *TaskSpec) Validate(ctx context.Context) (errs *apis.FieldError) {

errs = errs.Also(validateSteps(ctx, mergedSteps).ViaField("steps"))
errs = errs.Also(ts.Resources.Validate(ctx).ViaField("resources"))
errs = errs.Also(ValidateParameterTypes(ts.Params).ViaField("params"))
errs = errs.Also(validateParameterSpecs(ts.Params).ViaField("params"))
errs = errs.Also(ValidateParameterVariables(ts.Steps, ts.Params))
errs = errs.Also(ValidateResourcesVariables(ts.Steps, ts.Resources))
errs = errs.Also(validateTaskContextVariables(ts.Steps))
Expand Down Expand Up @@ -244,12 +244,18 @@ func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.Fi
return errs
}

// ValidateParameterTypes validates all the types within a slice of ParamSpecs
func ValidateParameterTypes(params []ParamSpec) (errs *apis.FieldError) {
// ValidateParameterSpecs validates that there are no duplicate names in the TaskSpec and all the types within a slice of ParamSpecs.
func validateParameterSpecs(params []ParamSpec) (errs *apis.FieldError) {
// validate type
for _, p := range params {
errs = errs.Also(p.ValidateType())
}
return errs
// validate no duplicate names
var names []string
for _, p := range params {
names = append(names, p.Name)
}
return errs.Also(validateNoDuplicateNames(names, false))
}

// ValidateType checks that the type of a ParamSpec is allowed and its default value matches that type
Expand Down
20 changes: 20 additions & 0 deletions pkg/apis/pipeline/v1beta1/task_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,26 @@ func TestTaskSpecValidateError(t *testing.T) {
Message: `expected exactly one, got both`,
Paths: []string{"resources.outputs.name"},
},
}, {
name: "duplicated param names",
fields: fields{
Params: []v1beta1.ParamSpec{{
Name: "foo",
Type: v1beta1.ParamTypeString,
Description: "parameter",
Default: v1beta1.NewArrayOrString("value1"),
}, {
Name: "foo",
Type: v1beta1.ParamTypeString,
Description: "parameter",
Default: v1beta1.NewArrayOrString("value2"),
}},
Steps: validSteps,
},
expectedError: apis.FieldError{
Message: `expected exactly one, got both`,
Paths: []string{"params[foo].name"},
},
}, {
name: "invalid param type",
fields: fields{
Expand Down

0 comments on commit 09e8c44

Please sign in to comment.