Skip to content

Commit

Permalink
Validate PipelineTask name as Task names 📛
Browse files Browse the repository at this point in the history
As reported, a `Pipeline` task name (`Pipelinetask`) can be created
that will violate `TaskRun` naming rules, making it fail at runtime.

This changes this by validation PipelineTask name the same way we
validation `TaskRun` names, to make sure we won't fail for this at
runtime.

Signed-off-by: Vincent Demeester <[email protected]>
  • Loading branch information
vdemeester committed Mar 2, 2020
1 parent 97a0c45 commit 64b139f
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 2 deletions.
7 changes: 7 additions & 0 deletions pkg/apis/pipeline/v1alpha1/pipeline_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,13 @@ func (ps *PipelineSpec) Validate(ctx context.Context) *apis.FieldError {
// Names cannot be duplicated
taskNames := map[string]struct{}{}
for i, t := range ps.Tasks {
if errs := validation.IsDNS1123Label(t.Name); len(errs) > 0 {
return &apis.FieldError{
Message: fmt.Sprintf("invalid value %q", t.Name),
Paths: []string{fmt.Sprintf("spec.tasks[%d].name", i)},
Details: "Pipeline Task name must be a valid DNS Label. For more info refer to https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names",
}
}
// can't have both taskRef and taskSpec at the same time
if (t.TaskRef != nil && t.TaskRef.Name != "") && t.TaskSpec != nil {
return apis.ErrMultipleOneOf(fmt.Sprintf("spec.tasks[%d].taskRef", i), fmt.Sprintf("spec.tasks[%d].taskSpec", i))
Expand Down
10 changes: 8 additions & 2 deletions pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,12 @@ func TestPipeline_Validate(t *testing.T) {
tb.PipelineTask("_foo", "foo-task"),
)),
failureExpected: true,
}, {
name: "pipeline spec invalid task name 2",
p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec(
tb.PipelineTask("FooTask", "foo-task"),
)),
failureExpected: true,
}, {
name: "pipeline spec invalid taskref name",
p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec(
Expand Down Expand Up @@ -97,13 +103,13 @@ func TestPipeline_Validate(t *testing.T) {
}, {
name: "pipeline spec invalid taskspec",
p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec(
tb.PipelineTask("", "", tb.PipelineTaskSpec(&v1alpha1.TaskSpec{})),
tb.PipelineTask("foo-task", "", tb.PipelineTaskSpec(&v1alpha1.TaskSpec{})),
)),
failureExpected: true,
}, {
name: "pipeline spec valid taskspec",
p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec(
tb.PipelineTask("", "", tb.PipelineTaskSpec(&v1alpha1.TaskSpec{
tb.PipelineTask("foo-task", "", tb.PipelineTaskSpec(&v1alpha1.TaskSpec{
TaskSpec: v1beta1.TaskSpec{
Steps: []v1alpha1.Step{{Container: corev1.Container{
Name: "foo",
Expand Down
7 changes: 7 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipeline_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,13 @@ func (ps *PipelineSpec) Validate(ctx context.Context) *apis.FieldError {
// Names cannot be duplicated
taskNames := map[string]struct{}{}
for i, t := range ps.Tasks {
if errs := validation.IsDNS1123Label(t.Name); len(errs) > 0 {
return &apis.FieldError{
Message: fmt.Sprintf("invalid value %q", t.Name),
Paths: []string{fmt.Sprintf("spec.tasks[%d].name", i)},
Details: "Pipeline Task name must be a valid DNS Label. For more info refer to https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names",
}
}
// can't have both taskRef and taskSpec at the same time
if (t.TaskRef != nil && t.TaskRef.Name != "") && t.TaskSpec != nil {
return apis.ErrMultipleOneOf(fmt.Sprintf("spec.tasks[%d].taskRef", i), fmt.Sprintf("spec.tasks[%d].taskSpec", i))
Expand Down
9 changes: 9 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipeline_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,15 @@ func TestPipeline_Validate(t *testing.T) {
},
},
failureExpected: true,
}, {
name: "pipeline spec invalid task name 2",
p: &v1beta1.Pipeline{
ObjectMeta: metav1.ObjectMeta{Name: "pipeline"},
Spec: v1beta1.PipelineSpec{
Tasks: []v1beta1.PipelineTask{{Name: "fooTask", TaskRef: &v1beta1.TaskRef{Name: "foo-task"}}},
},
},
failureExpected: true,
}, {
name: "pipeline spec invalid taskref name",
p: &v1beta1.Pipeline{
Expand Down

0 comments on commit 64b139f

Please sign in to comment.