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 Oct 4, 2019
1 parent 40e340f commit 27c91b6
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 4 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
15 changes: 13 additions & 2 deletions pkg/apis/pipeline/v1alpha1/pipeline_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,12 @@ package v1alpha1
import (
"context"
"fmt"
"strings"

"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 @@ -123,9 +125,18 @@ func (ps *PipelineSpec) Validate(ctx context.Context) *apis.FieldError {

// Names cannot be duplicated
taskNames := map[string]struct{}{}
for _, t := range ps.Tasks {
for i, 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(strings.Join(errSlice, ","), fmt.Sprintf("spec.tasks[%d].name", i))
}
// TaskRef name must be a valid k8s name
if errSlice := validation.IsQualifiedName(t.TaskRef.Name); len(errSlice) != 0 {
return apis.ErrInvalidValue(strings.Join(errSlice, ","), fmt.Sprintf("spec.tasks[%d].taskRef.name", i))
}
if _, ok := taskNames[t.Name]; ok {
return apis.ErrMultipleOneOf("spec.tasks.name")
return apis.ErrMultipleOneOf(fmt.Sprintf("spec.tasks[%d].name", i))
}
taskNames[t.Name] = struct{}{}
}
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 27c91b6

Please sign in to comment.