diff --git a/pkg/apis/pipeline/v1alpha1/condition_validation_test.go b/pkg/apis/pipeline/v1alpha1/condition_validation_test.go index f71e02f768d..2a1a0e80830 100644 --- 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 index c01fc8b5102..eb21c86cc23 100644 --- 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 index 516ee9d60bd..643e3daab91 100644 --- 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 index 7f96fcbb08d..947111ec9d5 100644 --- 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 index fca0d4cae59..80fccc12848 100644 --- 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 index e0ec791424b..ae0fa5108c9 100644 --- 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 index 102055f2171..b8da658e9b8 100644 --- 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 index 1eeabe6c53b..a2d54e2f63b 100644 --- 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 {