From 64b139f6549b4e077b03d1672b5f3c084f507b94 Mon Sep 17 00:00:00 2001 From: Vincent Demeester Date: Mon, 2 Mar 2020 15:29:54 +0100 Subject: [PATCH] =?UTF-8?q?Validate=20PipelineTask=20name=20as=20Task=20na?= =?UTF-8?q?mes=20=F0=9F=93=9B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- pkg/apis/pipeline/v1alpha1/pipeline_validation.go | 7 +++++++ pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go | 10 ++++++++-- pkg/apis/pipeline/v1beta1/pipeline_validation.go | 7 +++++++ pkg/apis/pipeline/v1beta1/pipeline_validation_test.go | 9 +++++++++ 4 files changed, 31 insertions(+), 2 deletions(-) diff --git a/pkg/apis/pipeline/v1alpha1/pipeline_validation.go b/pkg/apis/pipeline/v1alpha1/pipeline_validation.go index 0fbcbed6a7b..5b5cb0efbe4 100644 --- a/pkg/apis/pipeline/v1alpha1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1alpha1/pipeline_validation.go @@ -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)) diff --git a/pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go b/pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go index 86525794e4f..167be62aaff 100644 --- a/pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go @@ -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( @@ -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", diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go index c58de3580c0..851d3310793 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -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)) diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go index cebe662e7fb..62dc980d704 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go @@ -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{