diff --git a/pkg/apis/autoscaling/v1alpha1/pa_validation_test.go b/pkg/apis/autoscaling/v1alpha1/pa_validation_test.go index a0f73ff03059..9b015e1f6a23 100644 --- a/pkg/apis/autoscaling/v1alpha1/pa_validation_test.go +++ b/pkg/apis/autoscaling/v1alpha1/pa_validation_test.go @@ -167,6 +167,7 @@ func TestPodAutoscalerValidation(t *testing.T) { name: "valid", r: &PodAutoscaler{ ObjectMeta: v1.ObjectMeta{ + Name: "valid", Annotations: map[string]string{ "minScale": "2", }, @@ -186,6 +187,7 @@ func TestPodAutoscalerValidation(t *testing.T) { name: "bad scale bounds", r: &PodAutoscaler{ ObjectMeta: v1.ObjectMeta{ + Name: "valid", Annotations: map[string]string{ autoscaling.MinScaleAnnotationKey: "FOO", }, @@ -206,11 +208,18 @@ func TestPodAutoscalerValidation(t *testing.T) { }).ViaField("annotations").ViaField("metadata"), }, { name: "empty spec", - r: &PodAutoscaler{}, + r: &PodAutoscaler{ + ObjectMeta: v1.ObjectMeta{ + Name: "valid", + }, + }, want: apis.ErrMissingField("spec"), }, { name: "nested spec error", r: &PodAutoscaler{ + ObjectMeta: v1.ObjectMeta{ + Name: "valid", + }, Spec: PodAutoscalerSpec{ ConcurrencyModel: "BadValue", ServiceName: "foo", diff --git a/pkg/apis/serving/v1alpha1/configuration_validation_test.go b/pkg/apis/serving/v1alpha1/configuration_validation_test.go index c3c8b221da2c..bba2af5fedc9 100644 --- a/pkg/apis/serving/v1alpha1/configuration_validation_test.go +++ b/pkg/apis/serving/v1alpha1/configuration_validation_test.go @@ -158,6 +158,9 @@ func TestConfigurationValidation(t *testing.T) { }{{ name: "valid", c: &Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + }, Spec: ConfigurationSpec{ RevisionTemplate: RevisionTemplateSpec{ Spec: RevisionSpec{ @@ -172,6 +175,9 @@ func TestConfigurationValidation(t *testing.T) { }, { name: "propagate revision failures", c: &Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + }, Spec: ConfigurationSpec{ RevisionTemplate: RevisionTemplateSpec{ Spec: RevisionSpec{ @@ -186,7 +192,11 @@ func TestConfigurationValidation(t *testing.T) { want: apis.ErrDisallowedFields("spec.revisionTemplate.spec.container.name"), }, { name: "empty spec", - c: &Configuration{}, + c: &Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + }, + }, want: apis.ErrMissingField("spec"), }, { name: "invalid name - dots", @@ -195,17 +205,21 @@ func TestConfigurationValidation(t *testing.T) { Name: "do.not.use.dots", }, }, - want: (&apis.FieldError{Message: "Invalid resource name: special character . must not be present", Paths: []string{"metadata.name"}}). - Also(apis.ErrMissingField("spec")), + want: (&apis.FieldError{ + Message: "not a DNS 1035 label: [a DNS-1035 label must consist of lower case alphanumeric characters or '-', start with an alphabetic character, and end with an alphanumeric character (e.g. 'my-name', or 'abc-123', regex used for validation is '[a-z]([-a-z0-9]*[a-z0-9])?')]", + Paths: []string{"metadata.name"}, + }).Also(apis.ErrMissingField("spec")), }, { name: "invalid name - too long", c: &Configuration{ ObjectMeta: metav1.ObjectMeta{ - Name: strings.Repeat("a", 65), + Name: strings.Repeat("a", 64), }, }, - want: (&apis.FieldError{Message: "Invalid resource name: length must be no more than 63 characters", Paths: []string{"metadata.name"}}). - Also(apis.ErrMissingField("spec")), + want: (&apis.FieldError{ + Message: "not a DNS 1035 label: [must be no more than 63 characters]", + Paths: []string{"metadata.name"}, + }).Also(apis.ErrMissingField("spec")), }} for _, test := range tests { diff --git a/pkg/apis/serving/v1alpha1/metadata_validation.go b/pkg/apis/serving/v1alpha1/metadata_validation.go index 449a318b335f..35083d4b2213 100644 --- a/pkg/apis/serving/v1alpha1/metadata_validation.go +++ b/pkg/apis/serving/v1alpha1/metadata_validation.go @@ -19,11 +19,11 @@ package v1alpha1 import ( "fmt" "strconv" - "strings" "github.com/knative/pkg/apis" "github.com/knative/serving/pkg/apis/autoscaling" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/validation" ) // ValidateObjectMetadata validates that `metadata` stanza of the @@ -31,16 +31,10 @@ import ( func ValidateObjectMetadata(meta metav1.Object) *apis.FieldError { name := meta.GetName() - if strings.Contains(name, ".") { + msgs := validation.IsDNS1035Label(name) + if len(msgs) > 0 { return &apis.FieldError{ - Message: "Invalid resource name: special character . must not be present", - Paths: []string{"name"}, - } - } - - if len(name) > 63 { - return &apis.FieldError{ - Message: "Invalid resource name: length must be no more than 63 characters", + Message: fmt.Sprintf("not a DNS 1035 label: %v", msgs), Paths: []string{"name"}, } } diff --git a/pkg/apis/serving/v1alpha1/revision_validation_test.go b/pkg/apis/serving/v1alpha1/revision_validation_test.go index 8173b5a0cbe5..e98caacbefca 100644 --- a/pkg/apis/serving/v1alpha1/revision_validation_test.go +++ b/pkg/apis/serving/v1alpha1/revision_validation_test.go @@ -628,6 +628,9 @@ func TestRevisionValidation(t *testing.T) { }{{ name: "valid", r: &Revision{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + }, Spec: RevisionSpec{ Container: corev1.Container{ Image: "helloworld", @@ -638,11 +641,18 @@ func TestRevisionValidation(t *testing.T) { want: nil, }, { name: "empty spec", - r: &Revision{}, + r: &Revision{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + }, + }, want: apis.ErrMissingField("spec"), }, { name: "nested spec error", r: &Revision{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + }, Spec: RevisionSpec{ Container: corev1.Container{ Name: "kevin", @@ -653,10 +663,10 @@ func TestRevisionValidation(t *testing.T) { }, want: apis.ErrDisallowedFields("spec.container.name"), }, { - name: "invalid name - dots", + name: "invalid name - dots and too long", r: &Revision{ ObjectMeta: metav1.ObjectMeta{ - Name: "do.not.use.dots", + Name: "a" + strings.Repeat(".", 62) + "a", }, Spec: RevisionSpec{ Container: corev1.Container{ @@ -665,7 +675,10 @@ func TestRevisionValidation(t *testing.T) { ConcurrencyModel: "Multi", }, }, - want: &apis.FieldError{Message: "Invalid resource name: special character . must not be present", Paths: []string{"metadata.name"}}, + want: &apis.FieldError{ + Message: "not a DNS 1035 label: [must be no more than 63 characters a DNS-1035 label must consist of lower case alphanumeric characters or '-', start with an alphabetic character, and end with an alphanumeric character (e.g. 'my-name', or 'abc-123', regex used for validation is '[a-z]([-a-z0-9]*[a-z0-9])?')]", + Paths: []string{"metadata.name"}, + }, }, { name: "invalid metadata.annotations - scale bounds", r: &Revision{ @@ -687,20 +700,6 @@ func TestRevisionValidation(t *testing.T) { Message: fmt.Sprintf("%s=%v is less than %s=%v", autoscaling.MaxScaleAnnotationKey, 2, autoscaling.MinScaleAnnotationKey, 5), Paths: []string{autoscaling.MaxScaleAnnotationKey, autoscaling.MinScaleAnnotationKey}, }).ViaField("annotations").ViaField("metadata"), - }, { - name: "invalid name - too long", - r: &Revision{ - ObjectMeta: metav1.ObjectMeta{ - Name: strings.Repeat("a", 65), - }, - Spec: RevisionSpec{ - Container: corev1.Container{ - Image: "helloworld", - }, - ConcurrencyModel: "Multi", - }, - }, - want: &apis.FieldError{Message: "Invalid resource name: length must be no more than 63 characters", Paths: []string{"metadata.name"}}, }} for _, test := range tests { diff --git a/pkg/apis/serving/v1alpha1/route_validation_test.go b/pkg/apis/serving/v1alpha1/route_validation_test.go index bfba0ce66d88..f72002ba3d6e 100644 --- a/pkg/apis/serving/v1alpha1/route_validation_test.go +++ b/pkg/apis/serving/v1alpha1/route_validation_test.go @@ -34,6 +34,9 @@ func TestRouteValidation(t *testing.T) { }{{ name: "valid", r: &Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + }, Spec: RouteSpec{ Traffic: []TrafficTarget{{ RevisionName: "foo", @@ -45,6 +48,9 @@ func TestRouteValidation(t *testing.T) { }, { name: "valid split", r: &Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + }, Spec: RouteSpec{ Traffic: []TrafficTarget{{ Name: "prod", @@ -61,6 +67,9 @@ func TestRouteValidation(t *testing.T) { }, { name: "invalid traffic entry", r: &Route{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + }, Spec: RouteSpec{ Traffic: []TrafficTarget{{ Name: "foo", @@ -88,7 +97,10 @@ func TestRouteValidation(t *testing.T) { }}, }, }, - want: &apis.FieldError{Message: "Invalid resource name: special character . must not be present", Paths: []string{"metadata.name"}}, + want: &apis.FieldError{ + Message: "not a DNS 1035 label: [a DNS-1035 label must consist of lower case alphanumeric characters or '-', start with an alphabetic character, and end with an alphanumeric character (e.g. 'my-name', or 'abc-123', regex used for validation is '[a-z]([-a-z0-9]*[a-z0-9])?')]", + Paths: []string{"metadata.name"}, + }, }, { name: "invalid name - dots and spec percent is not 100", r: &Route{ @@ -102,13 +114,18 @@ func TestRouteValidation(t *testing.T) { }}, }, }, - want: (&apis.FieldError{Message: "Invalid resource name: special character . must not be present", Paths: []string{"metadata.name"}}). - Also(&apis.FieldError{Message: "Traffic targets sum to 90, want 100", Paths: []string{"spec.traffic"}}), + want: (&apis.FieldError{ + Message: "not a DNS 1035 label: [a DNS-1035 label must consist of lower case alphanumeric characters or '-', start with an alphabetic character, and end with an alphanumeric character (e.g. 'my-name', or 'abc-123', regex used for validation is '[a-z]([-a-z0-9]*[a-z0-9])?')]", + Paths: []string{"metadata.name"}, + }).Also(&apis.FieldError{ + Message: "Traffic targets sum to 90, want 100", + Paths: []string{"spec.traffic"}, + }), }, { name: "invalid name - too long", r: &Route{ ObjectMeta: metav1.ObjectMeta{ - Name: strings.Repeat("a", 65), + Name: strings.Repeat("a", 64), }, Spec: RouteSpec{ Traffic: []TrafficTarget{{ @@ -117,7 +134,10 @@ func TestRouteValidation(t *testing.T) { }}, }, }, - want: &apis.FieldError{Message: "Invalid resource name: length must be no more than 63 characters", Paths: []string{"metadata.name"}}, + want: &apis.FieldError{ + Message: "not a DNS 1035 label: [must be no more than 63 characters]", + Paths: []string{"metadata.name"}, + }, }} for _, test := range tests { diff --git a/pkg/apis/serving/v1alpha1/service_validation_test.go b/pkg/apis/serving/v1alpha1/service_validation_test.go index 6d2b92780a3c..b25194340bbc 100644 --- a/pkg/apis/serving/v1alpha1/service_validation_test.go +++ b/pkg/apis/serving/v1alpha1/service_validation_test.go @@ -35,6 +35,9 @@ func TestServiceValidation(t *testing.T) { }{{ name: "valid runLatest", s: &Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + }, Spec: ServiceSpec{ RunLatest: &RunLatestType{ Configuration: ConfigurationSpec{ @@ -53,6 +56,9 @@ func TestServiceValidation(t *testing.T) { }, { name: "valid pinned", s: &Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + }, Spec: ServiceSpec{ Pinned: &PinnedType{ RevisionName: "asdf", @@ -72,6 +78,9 @@ func TestServiceValidation(t *testing.T) { }, { name: "valid release -- one revision", s: &Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + }, Spec: ServiceSpec{ Release: &ReleaseType{ Revisions: []string{"asdf"}, @@ -91,6 +100,9 @@ func TestServiceValidation(t *testing.T) { }, { name: "valid release -- two revisions", s: &Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + }, Spec: ServiceSpec{ Release: &ReleaseType{ Revisions: []string{"asdf", "fdsa"}, @@ -111,6 +123,9 @@ func TestServiceValidation(t *testing.T) { }, { name: "valid manual", s: &Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + }, Spec: ServiceSpec{ Manual: &ManualType{}, }, @@ -119,6 +134,9 @@ func TestServiceValidation(t *testing.T) { }, { name: "invalid multiple types", s: &Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + }, Spec: ServiceSpec{ RunLatest: &RunLatestType{ Configuration: ConfigurationSpec{ @@ -151,7 +169,11 @@ func TestServiceValidation(t *testing.T) { }, }, { name: "invalid missing type", - s: &Service{}, + s: &Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + }, + }, want: &apis.FieldError{ Message: "expected exactly one, got neither", Paths: []string{"spec.manual", "spec.pinned", "spec.release", "spec.runLatest"}, @@ -159,6 +181,9 @@ func TestServiceValidation(t *testing.T) { }, { name: "invalid runLatest", s: &Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + }, Spec: ServiceSpec{ RunLatest: &RunLatestType{ Configuration: ConfigurationSpec{ @@ -178,6 +203,9 @@ func TestServiceValidation(t *testing.T) { }, { name: "invalid pinned", s: &Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + }, Spec: ServiceSpec{ Pinned: &PinnedType{ RevisionName: "asdf", @@ -198,6 +226,9 @@ func TestServiceValidation(t *testing.T) { }, { name: "invalid release -- too few revisions; nil", s: &Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + }, Spec: ServiceSpec{ Release: &ReleaseType{ Configuration: ConfigurationSpec{ @@ -216,6 +247,9 @@ func TestServiceValidation(t *testing.T) { }, { name: "invalid release -- too few revisions; empty slice", s: &Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + }, Spec: ServiceSpec{ Release: &ReleaseType{ Revisions: []string{}, @@ -235,6 +269,9 @@ func TestServiceValidation(t *testing.T) { }, { name: "invalid release -- too many revisions", s: &Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + }, Spec: ServiceSpec{ Release: &ReleaseType{ Revisions: []string{"asdf", "fdsa", "abcde"}, @@ -254,6 +291,9 @@ func TestServiceValidation(t *testing.T) { }, { name: "invalid release -- rollout greater than 99", s: &Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + }, Spec: ServiceSpec{ Release: &ReleaseType{ Revisions: []string{"asdf", "fdsa"}, @@ -274,6 +314,9 @@ func TestServiceValidation(t *testing.T) { }, { name: "invalid release -- rollout less than 0", s: &Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + }, Spec: ServiceSpec{ Release: &ReleaseType{ Revisions: []string{"asdf", "fdsa"}, @@ -294,6 +337,9 @@ func TestServiceValidation(t *testing.T) { }, { name: "invalid release -- non-zero rollout for single revision", s: &Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + }, Spec: ServiceSpec{ Release: &ReleaseType{ Revisions: []string{"asdf"}, @@ -331,12 +377,15 @@ func TestServiceValidation(t *testing.T) { }, }, }, - want: &apis.FieldError{Message: "Invalid resource name: special character . must not be present", Paths: []string{"metadata.name"}}, + want: &apis.FieldError{ + Message: "not a DNS 1035 label: [a DNS-1035 label must consist of lower case alphanumeric characters or '-', start with an alphabetic character, and end with an alphanumeric character (e.g. 'my-name', or 'abc-123', regex used for validation is '[a-z]([-a-z0-9]*[a-z0-9])?')]", + Paths: []string{"metadata.name"}, + }, }, { name: "invalid name - too long", s: &Service{ ObjectMeta: metav1.ObjectMeta{ - Name: strings.Repeat("a", 65), + Name: strings.Repeat("a", 64), }, Spec: ServiceSpec{ RunLatest: &RunLatestType{ @@ -352,7 +401,10 @@ func TestServiceValidation(t *testing.T) { }, }, }, - want: &apis.FieldError{Message: "Invalid resource name: length must be no more than 63 characters", Paths: []string{"metadata.name"}}, + want: &apis.FieldError{ + Message: "not a DNS 1035 label: [must be no more than 63 characters]", + Paths: []string{"metadata.name"}, + }, }} for _, test := range tests {