Skip to content

Commit

Permalink
TEP-0075: Validate object name and key name have no dots
Browse files Browse the repository at this point in the history
Prior to this commit, dots are allowed to be used in param names to
support domain-scoped names. However, object params have already
supported domain-scoped names since it has a list of keys. In addition,
using dots in object param names and key names have conflicts.
See more details in tektoncd/community#711.

In this change, we validate against those object names and key names
that contain dots.
  • Loading branch information
chuangw6 committed Jul 6, 2022
1 parent 38b65fa commit 1b6ecd5
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 10 deletions.
49 changes: 39 additions & 10 deletions pkg/apis/pipeline/v1beta1/task_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ var _ apis.Validatable = (*Task)(nil)

const variableNameFormat = "^[_a-zA-Z][_a-zA-Z0-9.-]*$"

var variableNameFormatRegex = regexp.MustCompile(variableNameFormat)

// Validate implements apis.Validatable
func (t *Task) Validate(ctx context.Context) *apis.FieldError {
errs := validate.ObjectMetadata(t.GetObjectMeta()).ViaField("metadata")
Expand Down Expand Up @@ -337,6 +339,7 @@ func ValidateParameterVariables(ctx context.Context, steps []Step, params []Para
}
}

errs = errs.Also(validateNameFormat(parameterNames, objectParamSpecs))
errs = errs.Also(validateVariables(ctx, steps, "params", parameterNames))
errs = errs.Also(validateArrayUsage(steps, "params", arrayParameterNames))
errs = errs.Also(validateObjectDefault(objectParamSpecs))
Expand Down Expand Up @@ -498,31 +501,57 @@ func validateStepArrayUsage(step Step, prefix string, vars sets.String) *apis.Fi
}

func validateVariables(ctx context.Context, steps []Step, prefix string, vars sets.String) (errs *apis.FieldError) {
// validate that the variable name format follows the rules
// - Must only contain alphanumeric characters, hyphens (-), underscores (_), and dots (.)
// - Must begin with a letter or an underscore (_)
re := regexp.MustCompile(variableNameFormat)
// We've checked param name format. Now, we want to check if param names are referenced correctly in each step
for idx, step := range steps {
errs = errs.Also(validateStepVariables(ctx, step, prefix, vars).ViaFieldIndex("steps", idx))
}
return errs
}

// validateNameFormat validates that the variable name format follows the rules
// - Must only contain alphanumeric characters, hyphens (-), underscores (_), and dots (.)
// - Must begin with a letter or an underscore (_)
// - Exception: Object param name and key names cannot contain dots (.)
func validateNameFormat(allParamNames sets.String, objectParams []ParamSpec) (errs *apis.FieldError) {
invalidNames := []string{}
// Converting to sorted list here rather than just looping map keys
// because we want the order of items in vars to be deterministic for purpose of unit testing
for _, name := range vars.List() {
if !re.MatchString(name) {
for _, name := range allParamNames.List() {
if !variableNameFormatRegex.MatchString(name) {
invalidNames = append(invalidNames, name)
}
}

if len(invalidNames) != 0 {
return &apis.FieldError{
errs = errs.Also(&apis.FieldError{
Message: fmt.Sprintf("The format of following variable names is invalid. %s", invalidNames),
Paths: []string{"params"},
Details: "Names: \nMust only contain alphanumeric characters, hyphens (-), underscores (_), and dots (.)\nMust begin with a letter or an underscore (_)",
})
}

invalidObjectVars := map[string][]string{}
for _, obj := range objectParams {
// check object param name
if strings.Contains(obj.Name, ".") {
invalidObjectVars[obj.Name] = []string{}
}

// check key names
for k := range obj.Properties {
if strings.Contains(k, ".") {
invalidObjectVars[obj.Name] = append(invalidObjectVars[obj.Name], k)
}
}
}

// We've checked param name format. Now, we want to check if param names are referenced correctly in each step
for idx, step := range steps {
errs = errs.Also(validateStepVariables(ctx, step, prefix, vars).ViaFieldIndex("steps", idx))
if len(invalidObjectVars) != 0 {
errs = errs.Also(&apis.FieldError{
Message: fmt.Sprintf("Object param name or key name shouldn't contain dots: %s", invalidObjectVars),
Paths: []string{"params"},
})
}

return errs
}

Expand Down
35 changes: 35 additions & 0 deletions pkg/apis/pipeline/v1beta1/task_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,41 @@ func TestTaskSpecValidateError(t *testing.T) {
Paths: []string{"params"},
Details: "Names: \nMust only contain alphanumeric characters, hyphens (-), underscores (_), and dots (.)\nMust begin with a letter or an underscore (_)",
},
}, {
name: "invalid object param format - object param name and key name shouldn't contain dots.",
fields: fields{
Params: []v1beta1.ParamSpec{{
Name: "invalid.name1",
Description: "object param name contains dots",
Properties: map[string]v1beta1.PropertySpec{
"mykey1": {},
"mykey2": {},
},
}, {
Name: "valid.name",
Description: "object param key name contains dots",
Properties: map[string]v1beta1.PropertySpec{
"invalid.key.name": {},
"validKeyName": {},
},
}, {
Name: "invalid.name2",
Description: "both object param name and key names contain dots",
Properties: map[string]v1beta1.PropertySpec{
"invalid.key.name1": {},
"invalid.key.name2": {},
},
}},
Steps: validSteps,
},
expectedError: apis.FieldError{
Message: fmt.Sprintf("Object param name or key name shouldn't contain dots: %v", map[string][]string{
"invalid.name1": {},
"valid.name": {"invalid.key.name"},
"invalid.name2": {"invalid.key.name1", "invalid.key.name2"},
}),
Paths: []string{"params"},
},
}, {
name: "duplicated param names",
fields: fields{
Expand Down

0 comments on commit 1b6ecd5

Please sign in to comment.