Skip to content

Commit

Permalink
Update PipelineSpec Task name and TaskRef name validation to prevents…
Browse files Browse the repository at this point in the history
… errors at runtime
  • Loading branch information
Vincent-DeSousa-Tereso authored and Vincent-DeSousa-Tereso committed Sep 27, 2019
1 parent 40e340f commit 9eb296c
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 2 deletions.
7 changes: 5 additions & 2 deletions docs/pipelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,11 @@ spec:
### Pipeline Tasks

A `Pipeline` will execute a graph of [`Tasks`](tasks.md) (see
[ordering](#ordering) for how to express this graph). At a minimum, this
declaration must include a reference to the [`Task`](tasks.md):
[ordering](#ordering) for how to express this graph). A valid `Pipeline`
declaration must include a reference to at least one [`Task`](tasks.md). Each
`Task` within a `Pipeline` must have a
[valid](https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names)
name and task reference, for example:

```yaml
tasks:
Expand Down
10 changes: 10 additions & 0 deletions pkg/apis/pipeline/v1alpha1/pipeline_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/tektoncd/pipeline/pkg/list"
"golang.org/x/xerrors"
"k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/util/validation"
"knative.dev/pkg/apis"
)

Expand Down Expand Up @@ -124,6 +125,15 @@ func (ps *PipelineSpec) Validate(ctx context.Context) *apis.FieldError {
// Names cannot be duplicated
taskNames := map[string]struct{}{}
for _, t := range ps.Tasks {
// Task names are appended to the container name, which must exist and
// must be a valid k8s name
if errSlice := validation.IsQualifiedName(t.Name); len(errSlice) != 0 {
return apis.ErrInvalidValue(errSlice[0], "spec.tasks.name")
}
// TaskRef name must be a valid k8s name
if errSlice := validation.IsQualifiedName(t.TaskRef.Name); len(errSlice) != 0 {
return apis.ErrInvalidValue(errSlice[0], "spec.tasks.taskRef")
}
if _, ok := taskNames[t.Name]; ok {
return apis.ErrMultipleOneOf("spec.tasks.name")
}
Expand Down
18 changes: 18 additions & 0 deletions pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,24 @@ func TestPipeline_Validate(t *testing.T) {
tb.PipelineTask("foo", "foo-task"),
)),
failureExpected: true,
}, {
name: "pipeline spec empty task name",
p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec(
tb.PipelineTask("", "foo-task"),
)),
failureExpected: true,
}, {
name: "pipeline spec invalid task name",
p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec(
tb.PipelineTask("_foo", "foo-task"),
)),
failureExpected: true,
}, {
name: "pipeline spec invalid taskref name",
p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec(
tb.PipelineTask("foo", "_foo-task"),
)),
failureExpected: true,
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down

0 comments on commit 9eb296c

Please sign in to comment.