Skip to content

Commit

Permalink
Validate parameter name format
Browse files Browse the repository at this point in the history
`tasks.md` mentioned that the parameter names should follow the
following rules. But we didn't enforce it.
- Must only contain alphanumeric characters, hyphens (-),
  underscores (_), and dots (.).
- Must begin with a letter or an underscore (_).

In this commit, we enforce the rules. If invalid name format is found,
validation webhook will complain.
  • Loading branch information
chuangw6 authored and tekton-robot committed May 11, 2022
1 parent 1c8447d commit 9c0e081
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 0 deletions.
25 changes: 25 additions & 0 deletions pkg/apis/pipeline/v1beta1/task_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"fmt"
"path/filepath"
"regexp"
"strings"
"time"

Expand All @@ -35,6 +36,8 @@ import (

var _ apis.Validatable = (*Task)(nil)

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

// Validate implements apis.Validatable
func (t *Task) Validate(ctx context.Context) *apis.FieldError {
errs := validate.ObjectMetadata(t.GetObjectMeta()).ViaField("metadata")
Expand Down Expand Up @@ -435,6 +438,28 @@ func validateStepArrayUsage(step Step, prefix string, vars sets.String) *apis.Fi
}

func validateVariables(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)
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) {
invalidNames = append(invalidNames, name)
}
}

if len(invalidNames) != 0 {
return &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 (_)",
}
}

// 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(step, prefix, vars).ViaFieldIndex("steps", idx))
}
Expand Down
29 changes: 29 additions & 0 deletions pkg/apis/pipeline/v1beta1/task_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,35 @@ func TestTaskSpecValidateError(t *testing.T) {
Message: `expected exactly one, got both`,
Paths: []string{"resources.outputs.name"},
},
}, {
name: "invalid param name format",
fields: fields{
Params: []v1beta1.ParamSpec{{
Name: "_validparam1",
Description: "valid param name format",
}, {
Name: "valid_param2",
Description: "valid param name format",
}, {
Name: "",
Description: "invalid param name format",
}, {
Name: "a^b",
Description: "invalid param name format",
}, {
Name: "0ab",
Description: "invalid param name format",
}, {
Name: "f oo",
Description: "invalid param name format",
}},
Steps: validSteps,
},
expectedError: apis.FieldError{
Message: fmt.Sprintf("The format of following variable names is invalid. %s", []string{"", "0ab", "a^b", "f oo"}),
Paths: []string{"params"},
Details: "Names: \nMust only contain alphanumeric characters, hyphens (-), underscores (_), and dots (.)\nMust begin with a letter or an underscore (_)",
},
}, {
name: "invalid param type",
fields: fields{
Expand Down

0 comments on commit 9c0e081

Please sign in to comment.