update

tektoncd · Apr 22, 2021 · 0732313 · 0732313
1 parent 7475553
commit 0732313
Show file tree

Hide file tree

Showing 8 changed files with 72 additions and 5 deletions.
diff --git a/pkg/apis/pipeline/v1alpha1/condition_validation_test.go b/pkg/apis/pipeline/v1alpha1/condition_validation_test.go
@@ -54,6 +54,15 @@ func TestCondition_Invalid(t *testing.T) {
 		cond          *v1alpha1.Condition
 		expectedError apis.FieldError
 	}{{
+		name: "invalid meta",
+		cond: &v1alpha1.Condition{
+			ObjectMeta: metav1.ObjectMeta{Name: "invalid,name"},
+		},
+		expectedError: apis.FieldError{
+			Message: `invalid resource name "invalid,name": must be a valid DNS label`,
+			Paths:   []string{"metadata.name"},
+		},
+	}, {
 		name: "no image",
 		cond: &v1alpha1.Condition{
 			ObjectMeta: metav1.ObjectMeta{Name: "condname"},

diff --git a/pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go b/pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go
@@ -44,6 +44,18 @@ func TestPipeline_Validate(t *testing.T) {
 			},
 		},
 		failureExpected: false,
+	}, {
+		name: "comma in name",
+		p: &v1alpha1.Pipeline{
+			ObjectMeta: metav1.ObjectMeta{Name: "pipe,line"},
+			Spec: v1alpha1.PipelineSpec{
+				Tasks: []v1alpha1.PipelineTask{{
+					Name:    "foo",
+					TaskRef: &v1alpha1.TaskRef{Name: "foo-task"},
+				}},
+			},
+		},
+		failureExpected: true,
 	}, {
 		name: "pipeline name too long",
 		p: &v1alpha1.Pipeline{

diff --git a/pkg/apis/pipeline/v1alpha1/pipelinerun_validation_test.go b/pkg/apis/pipeline/v1alpha1/pipelinerun_validation_test.go
@@ -45,6 +45,17 @@ func TestPipelineRun_Invalidate(t *testing.T) {
 			want: apis.ErrMissingField("spec"),
 		},
 		{
+			name: "invalid pipelinerun metadata",
+			pr: v1alpha1.PipelineRun{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "pipelinerun,name",
+				},
+			},
+			want: &apis.FieldError{
+				Message: `invalid resource name "pipelinerun,name": must be a valid DNS label`,
+				Paths:   []string{"metadata.name"},
+			},
+		}, {
 			name: "no pipeline reference",
 			pr: v1alpha1.PipelineRun{
 				ObjectMeta: metav1.ObjectMeta{

diff --git a/pkg/apis/pipeline/v1alpha1/taskrun_validation_test.go b/pkg/apis/pipeline/v1alpha1/taskrun_validation_test.go
@@ -40,6 +40,13 @@ func TestTaskRun_Invalid(t *testing.T) {
 		name: "invalid taskspec",
 		task: tb.TaskRun("taskmetaname"),
 		want: apis.ErrMissingField("spec"),
+	}, {
+		name: "invalid taskrun metadata",
+		task: tb.TaskRun("task,name"),
+		want: &apis.FieldError{
+			Message: `invalid resource name "task,name": must be a valid DNS label`,
+			Paths:   []string{"metadata.name"},
+		},
 	}}
 	for _, ts := range tests {
 		t.Run(ts.name, func(t *testing.T) {

diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go
@@ -135,6 +135,18 @@ func TestPipeline_Validate_Failure(t *testing.T) {
 		expectedError apis.FieldError
 		wc            func(context.Context) context.Context
 	}{{
+		name: "comma in name",
+		p: &Pipeline{
+			ObjectMeta: metav1.ObjectMeta{Name: "pipe,line"},
+			Spec: PipelineSpec{
+				Tasks: []PipelineTask{{Name: "foo", TaskRef: &TaskRef{Name: "foo-task"}}},
+			},
+		},
+		expectedError: apis.FieldError{
+			Message: `invalid resource name "pipe,line": must be a valid DNS label`,
+			Paths:   []string{"metadata.name"},
+		},
+	}, {
 		name: "pipeline name too long",
 		p: &Pipeline{
 			ObjectMeta: metav1.ObjectMeta{Name: "asdf123456789012345678901234567890123456789012345678901234567890"},
@@ -143,7 +155,7 @@ func TestPipeline_Validate_Failure(t *testing.T) {
 			},
 		},
 		expectedError: apis.FieldError{
-			Message: `invalid resource name "asdf123456789012345678901234567890123456789012345678901234567890": must be a valid DNS label`,
+			Message: "Invalid resource name: length must be no more than 63 characters",
 			Paths:   []string{"metadata.name"},
 		},
 	}, {

diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go b/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go
@@ -48,6 +48,22 @@ func TestPipelineRun_Invalid(t *testing.T) {
 			},
 		},
 		want: apis.ErrMissingField("spec.pipelineref.name, spec.pipelinespec"),
+	}, {
+		name: "invalid pipelinerun metadata",
+		pr: v1beta1.PipelineRun{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "pipelinerun,name",
+			},
+			Spec: v1beta1.PipelineRunSpec{
+				PipelineRef: &v1beta1.PipelineRef{
+					Name: "prname",
+				},
+			},
+		},
+		want: &apis.FieldError{
+			Message: `invalid resource name "pipelinerun,name": must be a valid DNS label`,
+			Paths:   []string{"metadata.name"},
+		},
 	}, {
 		name: "negative pipeline timeout",
 		pr: v1beta1.PipelineRun{

diff --git a/pkg/apis/validate/metadata.go b/pkg/apis/validate/metadata.go
@@ -29,17 +29,16 @@ const MaxLength = 63
 func ObjectMetadata(meta metav1.Object) *apis.FieldError {
 	name := meta.GetName()
 
-	// TODO update error message to be about length; using this for testing
-	if len(name) > MaxLength {
+	if err := validation.IsDNS1123Subdomain(name); len(err) > 0 {
 		return &apis.FieldError{
 			Message: fmt.Sprintf("invalid resource name %q: must be a valid DNS label", name),
 			Paths:   []string{"name"},
 		}
 	}
 
-	if err := validation.IsDNS1123Subdomain(name); len(err) > 0 {
+	if len(name) > MaxLength {
 		return &apis.FieldError{
-			Message: fmt.Sprintf("invalid resource name %q: must be a valid DNS label", name),
+			Message: "Invalid resource name: length must be no more than 63 characters",
 			Paths:   []string{"name"},
 		}
 	}

diff --git a/pkg/apis/validate/metadata_test.go b/pkg/apis/validate/metadata_test.go
@@ -28,6 +28,7 @@ func TestMetadataInvalidLongName(t *testing.T) {
 
 	invalidMetas := []*metav1.ObjectMeta{
 		{Name: strings.Repeat("s", validate.MaxLength+1)},
+		{Name: "bad,name"},
 	}
 	for _, invalidMeta := range invalidMetas {
 		if err := validate.ObjectMetadata(invalidMeta); err == nil {