From 37806f33e85fc6e2dab967f17f73607abbaa381e Mon Sep 17 00:00:00 2001 From: savitaashture Date: Fri, 9 Aug 2019 11:45:28 +0530 Subject: [PATCH 1/6] Add changes to validate name and generateName for revision template --- pkg/apis/serving/metadata_validation.go | 41 ++++++++ pkg/apis/serving/metadata_validation_test.go | 56 +++++++++++ pkg/apis/serving/v1/revision_validation.go | 16 +-- .../v1alpha1/configuration_validation_test.go | 66 +++++++++++++ .../serving/v1alpha1/revision_validation.go | 23 +---- .../v1alpha1/revision_validation_test.go | 48 +++++++++ .../v1beta1/configuration_validation_test.go | 99 ++++++++++++++++++- .../v1beta1/revision_validation_test.go | 54 ++++++++++ 8 files changed, 364 insertions(+), 39 deletions(-) diff --git a/pkg/apis/serving/metadata_validation.go b/pkg/apis/serving/metadata_validation.go index cac595f4c3be..9b0411a3eef3 100644 --- a/pkg/apis/serving/metadata_validation.go +++ b/pkg/apis/serving/metadata_validation.go @@ -18,10 +18,12 @@ package serving import ( "context" + "fmt" "strconv" "strings" "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/api/validation" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "knative.dev/pkg/apis" "knative.dev/serving/pkg/apis/autoscaling" @@ -132,3 +134,42 @@ func SetUserInfo(ctx context.Context, oldSpec, newSpec, resource interface{}) { } } } + +// ValidateRevisionName validates name and generateName for the revisionTemplate +func ValidateRevisionName(ctx context.Context, name, generateName string) *apis.FieldError { + if generateName != "" { + msgs := validation.NameIsDNS1035Label(generateName, true) + if len(msgs) > 0 { + return &apis.FieldError{ + Message: fmt.Sprintf("not a DNS 1035 label prefix: %v", msgs), + Paths: []string{"metadata.generateName"}, + } + } + } + if name != "" { + msgs := validation.NameIsDNS1035Label(name, false) + if len(msgs) > 0 { + return &apis.FieldError{ + Message: fmt.Sprintf("not a DNS 1035 label: %v", msgs), + Paths: []string{"metadata.name"}, + } + } + om := apis.ParentMeta(ctx) + prefix := om.Name + "-" + if om.Name != "" { + // Even if there is GenerateName, allow the use + // of Name post-creation. + } else if om.GenerateName != "" { + // We disallow bringing your own name when the parent + // resource uses generateName (at creation). + return apis.ErrDisallowedFields("metadata.name") + } + + if !strings.HasPrefix(name, prefix) { + return apis.ErrInvalidValue( + fmt.Sprintf("%q must have prefix %q", name, prefix), + "metadata.name") + } + } + return nil +} diff --git a/pkg/apis/serving/metadata_validation_test.go b/pkg/apis/serving/metadata_validation_test.go index 4c9b817ebbd7..0bfc20483f0a 100644 --- a/pkg/apis/serving/metadata_validation_test.go +++ b/pkg/apis/serving/metadata_validation_test.go @@ -441,3 +441,59 @@ func TestAnnotationUpdate(t *testing.T) { }) } } + +func TestValidateRevisionName(t *testing.T) { + cases := []struct { + name string + revName string + revGenerateName string + objectMeta metav1.ObjectMeta + expectErr error + }{{ + name: "invalid revision generateName - dots", + revGenerateName: "foo.bar", + expectErr: &apis.FieldError{ + Message: "not a DNS 1035 label prefix: [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.generateName"}, + }, + }, { + name: "invalid revision name - dots", + revName: "foo.bar", + expectErr: &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 (not prefixed)", + objectMeta: metav1.ObjectMeta{ + Name: "bar", + }, + revName: "foo", + expectErr: apis.ErrInvalidValue(`"foo" must have prefix "bar-"`, + "metadata.name"), + }, { + name: "invalid name (with generateName)", + objectMeta: metav1.ObjectMeta{ + GenerateName: "foo-bar-", + }, + revName: "foo-bar-foo", + expectErr: apis.ErrDisallowedFields("metadata.name"), + }, { + name: "valid name", + objectMeta: metav1.ObjectMeta{ + Name: "valid", + }, + revName: "valid-name", + expectErr: (*apis.FieldError)(nil), + }} + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + ctx := context.Background() + ctx = apis.WithinParent(ctx, c.objectMeta) + if err := ValidateRevisionName(ctx, c.revName, c.revGenerateName); !reflect.DeepEqual(c.expectErr, err) { + t.Errorf("Expected: '%#v', Got: '%#v'", c.expectErr, err) + } + }) + } +} diff --git a/pkg/apis/serving/v1/revision_validation.go b/pkg/apis/serving/v1/revision_validation.go index 4662c595a796..74eee7e38829 100644 --- a/pkg/apis/serving/v1/revision_validation.go +++ b/pkg/apis/serving/v1/revision_validation.go @@ -18,7 +18,6 @@ package v1 import ( "context" - "fmt" "strings" "knative.dev/pkg/apis" @@ -62,20 +61,7 @@ func (rts *RevisionTemplateSpec) Validate(ctx context.Context) *apis.FieldError // If the RevisionTemplateSpec has a name specified, then check that // it follows the requirements on the name. - if rts.Name != "" { - var prefix string - if om := apis.ParentMeta(ctx); om.Name == "" { - prefix = om.GenerateName - } else { - prefix = om.Name + "-" - } - - if !strings.HasPrefix(rts.Name, prefix) { - errs = errs.Also(apis.ErrInvalidValue( - fmt.Sprintf("%q must have prefix %q", rts.Name, prefix), - "metadata.name")) - } - } + errs = errs.Also(serving.ValidateRevisionName(ctx, rts.Name, rts.GenerateName)) errs = errs.Also(serving.ValidateQueueSidecarAnnotation(rts.Annotations).ViaField("metadata.annotations")) return errs diff --git a/pkg/apis/serving/v1alpha1/configuration_validation_test.go b/pkg/apis/serving/v1alpha1/configuration_validation_test.go index 8707ba05f7d7..fb8133dc842c 100644 --- a/pkg/apis/serving/v1alpha1/configuration_validation_test.go +++ b/pkg/apis/serving/v1alpha1/configuration_validation_test.go @@ -268,6 +268,26 @@ func TestConfigurationValidation(t *testing.T) { }, }, want: nil, + }, { + name: "invalid name", + c: &Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "", + }, + Spec: ConfigurationSpec{ + DeprecatedRevisionTemplate: &RevisionTemplateSpec{ + Spec: RevisionSpec{ + DeprecatedContainer: &corev1.Container{ + Image: "hellworld", + }, + }, + }, + }, + }, + want: &apis.FieldError{ + Message: "name or generateName is required", + Paths: []string{"metadata.name"}, + }, }, { name: "invalid BYO name (with generateName)", c: &Configuration{ @@ -309,6 +329,52 @@ func TestConfigurationValidation(t *testing.T) { }, want: apis.ErrInvalidValue(`"foo" must have prefix "byo-name-"`, "spec.revisionTemplate.metadata.name"), + }, { + name: "invalid name for configuration spec", + c: &Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "byo-name", + }, + Spec: ConfigurationSpec{ + DeprecatedRevisionTemplate: &RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo@bar", + }, + Spec: RevisionSpec{ + DeprecatedContainer: &corev1.Container{ + Image: "hellworld", + }, + }, + }, + }, + }, + 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{"spec.revisionTemplate.metadata.name"}, + }, + }, { + name: "invalid generate name for configuration spec", + c: &Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "byo-name", + }, + Spec: ConfigurationSpec{ + DeprecatedRevisionTemplate: &RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "foo@bar", + }, + Spec: RevisionSpec{ + DeprecatedContainer: &corev1.Container{ + Image: "hellworld", + }, + }, + }, + }, + }, + want: &apis.FieldError{ + Message: "not a DNS 1035 label prefix: [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{"spec.revisionTemplate.metadata.generateName"}, + }, }} for _, test := range tests { diff --git a/pkg/apis/serving/v1alpha1/revision_validation.go b/pkg/apis/serving/v1alpha1/revision_validation.go index 765ad15b7f77..aef133d30739 100644 --- a/pkg/apis/serving/v1alpha1/revision_validation.go +++ b/pkg/apis/serving/v1alpha1/revision_validation.go @@ -18,8 +18,6 @@ package v1alpha1 import ( "context" - "fmt" - "strings" "k8s.io/apimachinery/pkg/api/equality" "knative.dev/pkg/apis" @@ -61,28 +59,9 @@ func (r *Revision) Validate(ctx context.Context) *apis.FieldError { func (rt *RevisionTemplateSpec) Validate(ctx context.Context) *apis.FieldError { errs := rt.Spec.Validate(ctx).ViaField("spec") errs = errs.Also(autoscaling.ValidateAnnotations(rt.GetAnnotations()).ViaField("metadata.annotations")) - // If the DeprecatedRevisionTemplate has a name specified, then check that // it follows the requirements on the name. - if rt.Name != "" { - om := apis.ParentMeta(ctx) - prefix := om.Name + "-" - if om.Name != "" { - // Even if there is GenerateName, allow the use - // of Name post-creation. - } else if om.GenerateName != "" { - // We disallow bringing your own name when the parent - // resource uses generateName (at creation). - return apis.ErrDisallowedFields("metadata.name") - } - - if !strings.HasPrefix(rt.Name, prefix) { - errs = errs.Also(apis.ErrInvalidValue( - fmt.Sprintf("%q must have prefix %q", rt.Name, prefix), - "metadata.name")) - } - } - + errs = errs.Also(serving.ValidateRevisionName(ctx, rt.Name, rt.GenerateName)) errs = errs.Also(serving.ValidateQueueSidecarAnnotation(rt.Annotations).ViaField("metadata.annotations")) return errs } diff --git a/pkg/apis/serving/v1alpha1/revision_validation_test.go b/pkg/apis/serving/v1alpha1/revision_validation_test.go index 282058fe79a0..bd1ad2ad44be 100644 --- a/pkg/apis/serving/v1alpha1/revision_validation_test.go +++ b/pkg/apis/serving/v1alpha1/revision_validation_test.go @@ -354,6 +354,54 @@ func TestRevisionTemplateSpecValidation(t *testing.T) { }, }, want: nil, + }, { + name: "valid name for revision template", + rts: &RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + // When user provides empty string in the name field it will behave like no name provided. + Name: "", + }, + Spec: RevisionSpec{ + DeprecatedContainer: &corev1.Container{ + Image: "helloworld", + }, + }, + }, + want: nil, + }, { + name: "invalid name for revision template", + rts: &RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + // We let users bring their own revision name. + Name: "parent-@foo-bar", + }, + Spec: RevisionSpec{ + DeprecatedContainer: &corev1.Container{ + Image: "helloworld", + }, + }, + }, + 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 generate name for revision template", + rts: &RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + // We let users bring their own revision generate name. + GenerateName: "parent-@foo-bar", + }, + Spec: RevisionSpec{ + DeprecatedContainer: &corev1.Container{ + Image: "helloworld", + }, + }, + }, + want: &apis.FieldError{ + Message: "not a DNS 1035 label prefix: [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.generateName"}, + }, }, { name: "Queue sidecar resource percentage annotation more than 100", rts: &RevisionTemplateSpec{ diff --git a/pkg/apis/serving/v1beta1/configuration_validation_test.go b/pkg/apis/serving/v1beta1/configuration_validation_test.go index 232cbe513c12..1105cc9cc561 100644 --- a/pkg/apis/serving/v1beta1/configuration_validation_test.go +++ b/pkg/apis/serving/v1beta1/configuration_validation_test.go @@ -80,7 +80,29 @@ func TestConfigurationValidation(t *testing.T) { }, want: nil, }, { - name: "valid BYO name (with generateName)", + name: "invalid name", + c: &Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "", + }, + Spec: ConfigurationSpec{ + Template: RevisionTemplateSpec{ + Spec: RevisionSpec{ + PodSpec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "hellworld", + }}, + }, + }, + }, + }, + }, + want: &apis.FieldError{ + Message: "name or generateName is required", + Paths: []string{"metadata.name"}, + }, + }, { + name: "invalid BYO name (with generateName)", c: &Configuration{ ObjectMeta: metav1.ObjectMeta{ GenerateName: "byo-name-", @@ -100,7 +122,80 @@ func TestConfigurationValidation(t *testing.T) { }, }, }, - want: nil, + want: apis.ErrDisallowedFields("spec.template.metadata.name"), + }, { + name: "invalid BYO name (not prefixed)", + c: &Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "byo-name", + }, + Spec: ConfigurationSpec{ + Template: RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + }, + Spec: RevisionSpec{ + PodSpec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "hellworld", + }}, + }, + }, + }, + }, + }, + want: apis.ErrInvalidValue(`"foo" must have prefix "byo-name-"`, + "spec.template.metadata.name"), + }, { + name: "invalid name for configuration spec", + c: &Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "byo-name", + }, + Spec: ConfigurationSpec{ + Template: RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo.bar", + }, + Spec: RevisionSpec{ + PodSpec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "hellworld", + }}, + }, + }, + }, + }, + }, + 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{"spec.template.metadata.name"}, + }, + }, { + name: "invalid generate name for configuration spec", + c: &Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "byo-name", + }, + Spec: ConfigurationSpec{ + Template: RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "foo.bar", + }, + Spec: RevisionSpec{ + PodSpec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "hellworld", + }}, + }, + }, + }, + }, + }, + want: &apis.FieldError{ + Message: "not a DNS 1035 label prefix: [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{"spec.template.metadata.generateName"}, + }, }} for _, test := range tests { diff --git a/pkg/apis/serving/v1beta1/revision_validation_test.go b/pkg/apis/serving/v1beta1/revision_validation_test.go index 68cab977e428..2f7fc95342e4 100644 --- a/pkg/apis/serving/v1beta1/revision_validation_test.go +++ b/pkg/apis/serving/v1beta1/revision_validation_test.go @@ -780,6 +780,60 @@ func TestRevisionTemplateSpecValidation(t *testing.T) { }, }, want: nil, + }, { + name: "valid name for revision template", + rts: &RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + // When user provides empty string in the name field it will behave like no name provided. + Name: "", + }, + Spec: RevisionSpec{ + PodSpec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "helloworld", + }}, + }, + }, + }, + want: nil, + }, { + name: "invalid name for revision template", + rts: &RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + // We let users bring their own revision name. + Name: "parent-@foo-bar", + }, + Spec: RevisionSpec{ + PodSpec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "helloworld", + }}, + }, + }, + }, + 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 generate name for revision template", + rts: &RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + // We let users bring their own revision generate name. + GenerateName: "parent-@foo-bar", + }, + Spec: RevisionSpec{ + PodSpec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "helloworld", + }}, + }, + }, + }, + want: &apis.FieldError{ + Message: "not a DNS 1035 label prefix: [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.generateName"}, + }, }, { name: "invalid metadata.annotations for scale", rts: &v1.RevisionTemplateSpec{ From 7f83cf8a540dc2902951b87a7a9ebd6aaef56ab9 Mon Sep 17 00:00:00 2001 From: savitaashture Date: Fri, 9 Aug 2019 14:41:04 +0530 Subject: [PATCH 2/6] Fix review comment --- pkg/apis/serving/metadata_validation.go | 20 ++++++++----------- pkg/apis/serving/metadata_validation_test.go | 12 ++++------- .../v1alpha1/configuration_validation_test.go | 12 ++++------- .../v1alpha1/revision_validation_test.go | 12 ++++------- .../v1beta1/configuration_validation_test.go | 12 ++++------- .../v1beta1/revision_validation_test.go | 12 ++++------- 6 files changed, 28 insertions(+), 52 deletions(-) diff --git a/pkg/apis/serving/metadata_validation.go b/pkg/apis/serving/metadata_validation.go index 9b0411a3eef3..034af7c2317b 100644 --- a/pkg/apis/serving/metadata_validation.go +++ b/pkg/apis/serving/metadata_validation.go @@ -138,21 +138,17 @@ func SetUserInfo(ctx context.Context, oldSpec, newSpec, resource interface{}) { // ValidateRevisionName validates name and generateName for the revisionTemplate func ValidateRevisionName(ctx context.Context, name, generateName string) *apis.FieldError { if generateName != "" { - msgs := validation.NameIsDNS1035Label(generateName, true) - if len(msgs) > 0 { - return &apis.FieldError{ - Message: fmt.Sprintf("not a DNS 1035 label prefix: %v", msgs), - Paths: []string{"metadata.generateName"}, - } + if msgs := validation.NameIsDNS1035Label(generateName, true); len(msgs) > 0 { + return apis.ErrInvalidValue( + fmt.Sprintf("not a DNS 1035 label prefix: %v", msgs), + "metadata.generateName") } } if name != "" { - msgs := validation.NameIsDNS1035Label(name, false) - if len(msgs) > 0 { - return &apis.FieldError{ - Message: fmt.Sprintf("not a DNS 1035 label: %v", msgs), - Paths: []string{"metadata.name"}, - } + if msgs := validation.NameIsDNS1035Label(name, false); len(msgs) > 0 { + return apis.ErrInvalidValue( + fmt.Sprintf("not a DNS 1035 label: %v", msgs), + "metadata.name") } om := apis.ParentMeta(ctx) prefix := om.Name + "-" diff --git a/pkg/apis/serving/metadata_validation_test.go b/pkg/apis/serving/metadata_validation_test.go index 0bfc20483f0a..dbabadda3c0d 100644 --- a/pkg/apis/serving/metadata_validation_test.go +++ b/pkg/apis/serving/metadata_validation_test.go @@ -452,17 +452,13 @@ func TestValidateRevisionName(t *testing.T) { }{{ name: "invalid revision generateName - dots", revGenerateName: "foo.bar", - expectErr: &apis.FieldError{ - Message: "not a DNS 1035 label prefix: [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.generateName"}, - }, + expectErr: apis.ErrInvalidValue("not a DNS 1035 label prefix: [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])?')]", + "metadata.generateName"), }, { name: "invalid revision name - dots", revName: "foo.bar", - expectErr: &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"}, - }, + expectErr: apis.ErrInvalidValue("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])?')]", + "metadata.name"), }, { name: "invalid name (not prefixed)", objectMeta: metav1.ObjectMeta{ diff --git a/pkg/apis/serving/v1alpha1/configuration_validation_test.go b/pkg/apis/serving/v1alpha1/configuration_validation_test.go index fb8133dc842c..5cfb4022f3bf 100644 --- a/pkg/apis/serving/v1alpha1/configuration_validation_test.go +++ b/pkg/apis/serving/v1alpha1/configuration_validation_test.go @@ -348,10 +348,8 @@ func TestConfigurationValidation(t *testing.T) { }, }, }, - 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{"spec.revisionTemplate.metadata.name"}, - }, + want: apis.ErrInvalidValue("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])?')]", + "spec.revisionTemplate.metadata.name"), }, { name: "invalid generate name for configuration spec", c: &Configuration{ @@ -371,10 +369,8 @@ func TestConfigurationValidation(t *testing.T) { }, }, }, - want: &apis.FieldError{ - Message: "not a DNS 1035 label prefix: [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{"spec.revisionTemplate.metadata.generateName"}, - }, + want: apis.ErrInvalidValue("not a DNS 1035 label prefix: [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])?')]", + "spec.revisionTemplate.metadata.generateName"), }} for _, test := range tests { diff --git a/pkg/apis/serving/v1alpha1/revision_validation_test.go b/pkg/apis/serving/v1alpha1/revision_validation_test.go index bd1ad2ad44be..947e840e38c8 100644 --- a/pkg/apis/serving/v1alpha1/revision_validation_test.go +++ b/pkg/apis/serving/v1alpha1/revision_validation_test.go @@ -381,10 +381,8 @@ func TestRevisionTemplateSpecValidation(t *testing.T) { }, }, }, - 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"}, - }, + want: apis.ErrInvalidValue("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])?')]", + "metadata.name"), }, { name: "invalid generate name for revision template", rts: &RevisionTemplateSpec{ @@ -398,10 +396,8 @@ func TestRevisionTemplateSpecValidation(t *testing.T) { }, }, }, - want: &apis.FieldError{ - Message: "not a DNS 1035 label prefix: [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.generateName"}, - }, + want: apis.ErrInvalidValue("not a DNS 1035 label prefix: [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])?')]", + "metadata.generateName"), }, { name: "Queue sidecar resource percentage annotation more than 100", rts: &RevisionTemplateSpec{ diff --git a/pkg/apis/serving/v1beta1/configuration_validation_test.go b/pkg/apis/serving/v1beta1/configuration_validation_test.go index 1105cc9cc561..25549c6e9ad1 100644 --- a/pkg/apis/serving/v1beta1/configuration_validation_test.go +++ b/pkg/apis/serving/v1beta1/configuration_validation_test.go @@ -167,10 +167,8 @@ func TestConfigurationValidation(t *testing.T) { }, }, }, - 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{"spec.template.metadata.name"}, - }, + want: apis.ErrInvalidValue("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])?')]", + "spec.template.metadata.name"), }, { name: "invalid generate name for configuration spec", c: &Configuration{ @@ -192,10 +190,8 @@ func TestConfigurationValidation(t *testing.T) { }, }, }, - want: &apis.FieldError{ - Message: "not a DNS 1035 label prefix: [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{"spec.template.metadata.generateName"}, - }, + want: apis.ErrInvalidValue("not a DNS 1035 label prefix: [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])?')]", + "spec.template.metadata.generateName"), }} for _, test := range tests { diff --git a/pkg/apis/serving/v1beta1/revision_validation_test.go b/pkg/apis/serving/v1beta1/revision_validation_test.go index 2f7fc95342e4..0ab768f19478 100644 --- a/pkg/apis/serving/v1beta1/revision_validation_test.go +++ b/pkg/apis/serving/v1beta1/revision_validation_test.go @@ -811,10 +811,8 @@ func TestRevisionTemplateSpecValidation(t *testing.T) { }, }, }, - 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"}, - }, + want: apis.ErrInvalidValue("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])?')]", + "metadata.name"), }, { name: "invalid generate name for revision template", rts: &RevisionTemplateSpec{ @@ -830,10 +828,8 @@ func TestRevisionTemplateSpecValidation(t *testing.T) { }, }, }, - want: &apis.FieldError{ - Message: "not a DNS 1035 label prefix: [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.generateName"}, - }, + want: apis.ErrInvalidValue("not a DNS 1035 label prefix: [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])?')]", + "metadata.generateName"), }, { name: "invalid metadata.annotations for scale", rts: &v1.RevisionTemplateSpec{ From ae41cc795e6a9845d457e05e0d9c1f19cf0ac7f4 Mon Sep 17 00:00:00 2001 From: savitaashture Date: Tue, 13 Aug 2019 17:33:15 +0530 Subject: [PATCH 3/6] Fix review comment --- .../v1alpha1/configuration_validation_test.go | 20 +++++++++++++++++ .../v1beta1/configuration_validation_test.go | 22 +++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/pkg/apis/serving/v1alpha1/configuration_validation_test.go b/pkg/apis/serving/v1alpha1/configuration_validation_test.go index 5cfb4022f3bf..c18bfc72a294 100644 --- a/pkg/apis/serving/v1alpha1/configuration_validation_test.go +++ b/pkg/apis/serving/v1alpha1/configuration_validation_test.go @@ -371,6 +371,26 @@ func TestConfigurationValidation(t *testing.T) { }, want: apis.ErrInvalidValue("not a DNS 1035 label prefix: [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])?')]", "spec.revisionTemplate.metadata.generateName"), + }, { + name: "valid generate name for configuration spec", + c: &Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "byo-name", + }, + Spec: ConfigurationSpec{ + DeprecatedRevisionTemplate: &RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "valid-generatename", + }, + Spec: RevisionSpec{ + DeprecatedContainer: &corev1.Container{ + Image: "hellworld", + }, + }, + }, + }, + }, + want: nil, }} for _, test := range tests { diff --git a/pkg/apis/serving/v1beta1/configuration_validation_test.go b/pkg/apis/serving/v1beta1/configuration_validation_test.go index 25549c6e9ad1..4388bca9784d 100644 --- a/pkg/apis/serving/v1beta1/configuration_validation_test.go +++ b/pkg/apis/serving/v1beta1/configuration_validation_test.go @@ -192,6 +192,28 @@ func TestConfigurationValidation(t *testing.T) { }, want: apis.ErrInvalidValue("not a DNS 1035 label prefix: [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])?')]", "spec.template.metadata.generateName"), + }, { + name: "valid generate name for configuration spec", + c: &Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "byo-name", + }, + Spec: ConfigurationSpec{ + Template: RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "valid-generatename", + }, + Spec: RevisionSpec{ + PodSpec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "hellworld", + }}, + }, + }, + }, + }, + }, + want: nil, }} for _, test := range tests { From c4d7ccdc96f3835a4a31c8e423da7f2d218884a0 Mon Sep 17 00:00:00 2001 From: savitaashture Date: Wed, 28 Aug 2019 11:06:44 +0530 Subject: [PATCH 4/6] Added testcase for v1alpha1/revision_validation.go --- .../v1alpha1/revision_validation_test.go | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/pkg/apis/serving/v1alpha1/revision_validation_test.go b/pkg/apis/serving/v1alpha1/revision_validation_test.go index 947e840e38c8..7362075688ca 100644 --- a/pkg/apis/serving/v1alpha1/revision_validation_test.go +++ b/pkg/apis/serving/v1alpha1/revision_validation_test.go @@ -74,6 +74,47 @@ func TestConcurrencyModelValidation(t *testing.T) { } } +func TestServingStateType(t *testing.T) { + tests := []struct { + name string + cm DeprecatedRevisionServingStateType + want *apis.FieldError + }{{ + name: "active", + cm: DeprecatedRevisionServingStateActive, + want: nil, + }, { + name: "reserve", + cm: DeprecatedRevisionServingStateReserve, + want: nil, + }, { + name: "retired", + cm: DeprecatedRevisionServingStateRetired, + want: nil, + }, { + name: "empty", + cm: "", + want: nil, + }, { + name: "bogus", + cm: "bogus", + want: apis.ErrInvalidValue("bogus", apis.CurrentField), + }, { + name: "balderdash", + cm: "balderdash", + want: apis.ErrInvalidValue("balderdash", apis.CurrentField), + }} + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got := test.cm.Validate(context.Background()) + if diff := cmp.Diff(test.want.Error(), got.Error()); diff != "" { + t.Errorf("Validate (-want, +got) = %v", diff) + } + }) + } +} + func TestRevisionSpecValidation(t *testing.T) { tests := []struct { name string From 2b4c2978005d562d3d3a6a780cbdb2da04cc3ace Mon Sep 17 00:00:00 2001 From: savitaashture Date: Wed, 18 Sep 2019 22:22:29 +0530 Subject: [PATCH 5/6] Rebase --- .../v1/configuration_validation_test.go | 92 ++++++++++++++++++- pkg/apis/serving/v1/revision_validation.go | 3 +- .../serving/v1/revision_validation_test.go | 50 ++++++++++ .../v1beta1/configuration_validation_test.go | 30 +++--- .../v1beta1/revision_validation_test.go | 50 ---------- 5 files changed, 157 insertions(+), 68 deletions(-) diff --git a/pkg/apis/serving/v1/configuration_validation_test.go b/pkg/apis/serving/v1/configuration_validation_test.go index 9ff10d305876..002951c7e8da 100644 --- a/pkg/apis/serving/v1/configuration_validation_test.go +++ b/pkg/apis/serving/v1/configuration_validation_test.go @@ -100,6 +100,28 @@ func TestConfigurationValidation(t *testing.T) { }, }, want: nil, + }, { + name: "invalid name", + c: &Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "", + }, + Spec: ConfigurationSpec{ + Template: RevisionTemplateSpec{ + Spec: RevisionSpec{ + PodSpec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "hellworld", + }}, + }, + }, + }, + }, + }, + want: &apis.FieldError{ + Message: "name or generateName is required", + Paths: []string{"metadata.name"}, + }, }, { name: "valid BYO name (with generateName)", c: &Configuration{ @@ -121,7 +143,7 @@ func TestConfigurationValidation(t *testing.T) { }, }, }, - want: nil, + want: apis.ErrDisallowedFields("spec.template.metadata.name"), }, { name: "invalid BYO name (not prefixed)", c: &Configuration{ @@ -145,6 +167,74 @@ func TestConfigurationValidation(t *testing.T) { }, want: apis.ErrInvalidValue(`"foo" must have prefix "byo-name-"`, "spec.template.metadata.name"), + }, { + name: "invalid name for configuration spec", + c: &Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "byo-name", + }, + Spec: ConfigurationSpec{ + Template: RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Name: "foo.bar", + }, + Spec: RevisionSpec{ + PodSpec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "hellworld", + }}, + }, + }, + }, + }, + }, + want: apis.ErrInvalidValue("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])?')]", + "spec.template.metadata.name"), + }, { + name: "invalid generate name for configuration spec", + c: &Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "byo-name", + }, + Spec: ConfigurationSpec{ + Template: RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "foo.bar", + }, + Spec: RevisionSpec{ + PodSpec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "hellworld", + }}, + }, + }, + }, + }, + }, + want: apis.ErrInvalidValue("not a DNS 1035 label prefix: [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])?')]", + "spec.template.metadata.generateName"), + }, { + name: "valid generate name for configuration spec", + c: &Configuration{ + ObjectMeta: metav1.ObjectMeta{ + Name: "byo-name", + }, + Spec: ConfigurationSpec{ + Template: RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "valid-generatename", + }, + Spec: RevisionSpec{ + PodSpec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "hellworld", + }}, + }, + }, + }, + }, + }, + want: nil, }} // TODO(dangerd): PodSpec validation failures. diff --git a/pkg/apis/serving/v1/revision_validation.go b/pkg/apis/serving/v1/revision_validation.go index 74eee7e38829..5b9c7d0fa833 100644 --- a/pkg/apis/serving/v1/revision_validation.go +++ b/pkg/apis/serving/v1/revision_validation.go @@ -61,8 +61,7 @@ func (rts *RevisionTemplateSpec) Validate(ctx context.Context) *apis.FieldError // If the RevisionTemplateSpec has a name specified, then check that // it follows the requirements on the name. - errs = errs.Also(serving.ValidateRevisionName(ctx, rts.Name, rts.GenerateName)) - + errs = errs.Also(serving.ValidateRevisionName(ctx, rts.Name, rts.GenerateName)) errs = errs.Also(serving.ValidateQueueSidecarAnnotation(rts.Annotations).ViaField("metadata.annotations")) return errs } diff --git a/pkg/apis/serving/v1/revision_validation_test.go b/pkg/apis/serving/v1/revision_validation_test.go index 89301a13faa4..21aa7e2d1bed 100644 --- a/pkg/apis/serving/v1/revision_validation_test.go +++ b/pkg/apis/serving/v1/revision_validation_test.go @@ -778,6 +778,56 @@ func TestRevisionTemplateSpecValidation(t *testing.T) { }, }, want: nil, + }, { + name: "valid name for revision template", + rts: &RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + // When user provides empty string in the name field it will behave like no name provided. + Name: "", + }, + Spec: RevisionSpec{ + PodSpec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "helloworld", + }}, + }, + }, + }, + want: nil, + }, { + name: "invalid name for revision template", + rts: &RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + // We let users bring their own revision name. + Name: "parent-@foo-bar", + }, + Spec: RevisionSpec{ + PodSpec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "helloworld", + }}, + }, + }, + }, + want: apis.ErrInvalidValue("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])?')]", + "metadata.name"), + }, { + name: "invalid generate name for revision template", + rts: &RevisionTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + // We let users bring their own revision generate name. + GenerateName: "parent-@foo-bar", + }, + Spec: RevisionSpec{ + PodSpec: corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "helloworld", + }}, + }, + }, + }, + want: apis.ErrInvalidValue("not a DNS 1035 label prefix: [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])?')]", + "metadata.generateName"), }, { name: "invalid metadata.annotations for scale", rts: &RevisionTemplateSpec{ diff --git a/pkg/apis/serving/v1beta1/configuration_validation_test.go b/pkg/apis/serving/v1beta1/configuration_validation_test.go index 4388bca9784d..5bcd0d661415 100644 --- a/pkg/apis/serving/v1beta1/configuration_validation_test.go +++ b/pkg/apis/serving/v1beta1/configuration_validation_test.go @@ -85,9 +85,9 @@ func TestConfigurationValidation(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "", }, - Spec: ConfigurationSpec{ - Template: RevisionTemplateSpec{ - Spec: RevisionSpec{ + Spec: v1.ConfigurationSpec{ + Template: v1.RevisionTemplateSpec{ + Spec: v1.RevisionSpec{ PodSpec: corev1.PodSpec{ Containers: []corev1.Container{{ Image: "hellworld", @@ -129,12 +129,12 @@ func TestConfigurationValidation(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "byo-name", }, - Spec: ConfigurationSpec{ - Template: RevisionTemplateSpec{ + Spec: v1.ConfigurationSpec{ + Template: v1.RevisionTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Name: "foo", }, - Spec: RevisionSpec{ + Spec: v1.RevisionSpec{ PodSpec: corev1.PodSpec{ Containers: []corev1.Container{{ Image: "hellworld", @@ -152,12 +152,12 @@ func TestConfigurationValidation(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "byo-name", }, - Spec: ConfigurationSpec{ - Template: RevisionTemplateSpec{ + Spec: v1.ConfigurationSpec{ + Template: v1.RevisionTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ Name: "foo.bar", }, - Spec: RevisionSpec{ + Spec: v1.RevisionSpec{ PodSpec: corev1.PodSpec{ Containers: []corev1.Container{{ Image: "hellworld", @@ -175,12 +175,12 @@ func TestConfigurationValidation(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "byo-name", }, - Spec: ConfigurationSpec{ - Template: RevisionTemplateSpec{ + Spec: v1.ConfigurationSpec{ + Template: v1.RevisionTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ GenerateName: "foo.bar", }, - Spec: RevisionSpec{ + Spec: v1.RevisionSpec{ PodSpec: corev1.PodSpec{ Containers: []corev1.Container{{ Image: "hellworld", @@ -198,12 +198,12 @@ func TestConfigurationValidation(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ Name: "byo-name", }, - Spec: ConfigurationSpec{ - Template: RevisionTemplateSpec{ + Spec: v1.ConfigurationSpec{ + Template: v1.RevisionTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ GenerateName: "valid-generatename", }, - Spec: RevisionSpec{ + Spec: v1.RevisionSpec{ PodSpec: corev1.PodSpec{ Containers: []corev1.Container{{ Image: "hellworld", diff --git a/pkg/apis/serving/v1beta1/revision_validation_test.go b/pkg/apis/serving/v1beta1/revision_validation_test.go index 0ab768f19478..68cab977e428 100644 --- a/pkg/apis/serving/v1beta1/revision_validation_test.go +++ b/pkg/apis/serving/v1beta1/revision_validation_test.go @@ -780,56 +780,6 @@ func TestRevisionTemplateSpecValidation(t *testing.T) { }, }, want: nil, - }, { - name: "valid name for revision template", - rts: &RevisionTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - // When user provides empty string in the name field it will behave like no name provided. - Name: "", - }, - Spec: RevisionSpec{ - PodSpec: corev1.PodSpec{ - Containers: []corev1.Container{{ - Image: "helloworld", - }}, - }, - }, - }, - want: nil, - }, { - name: "invalid name for revision template", - rts: &RevisionTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - // We let users bring their own revision name. - Name: "parent-@foo-bar", - }, - Spec: RevisionSpec{ - PodSpec: corev1.PodSpec{ - Containers: []corev1.Container{{ - Image: "helloworld", - }}, - }, - }, - }, - want: apis.ErrInvalidValue("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])?')]", - "metadata.name"), - }, { - name: "invalid generate name for revision template", - rts: &RevisionTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - // We let users bring their own revision generate name. - GenerateName: "parent-@foo-bar", - }, - Spec: RevisionSpec{ - PodSpec: corev1.PodSpec{ - Containers: []corev1.Container{{ - Image: "helloworld", - }}, - }, - }, - }, - want: apis.ErrInvalidValue("not a DNS 1035 label prefix: [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])?')]", - "metadata.generateName"), }, { name: "invalid metadata.annotations for scale", rts: &v1.RevisionTemplateSpec{ From 8ccdcae1b5e6858964a58e232a1357ed1b3580e8 Mon Sep 17 00:00:00 2001 From: savitaashture Date: Thu, 24 Oct 2019 13:04:10 +0530 Subject: [PATCH 6/6] fix conflict issue --- pkg/apis/serving/metadata_validation_test.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/pkg/apis/serving/metadata_validation_test.go b/pkg/apis/serving/metadata_validation_test.go index dbabadda3c0d..83386083a2d7 100644 --- a/pkg/apis/serving/metadata_validation_test.go +++ b/pkg/apis/serving/metadata_validation_test.go @@ -312,7 +312,7 @@ func TestValidateClusterVisibilityLabel(t *testing.T) { } -type WithPod struct { +type withPod struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` Spec corev1.PodSpec `json:"spec,omitempty"` @@ -334,12 +334,12 @@ func TestAnnotationCreate(t *testing.T) { tests := []struct { name string user string - this *WithPod + this *withPod want map[string]string }{{ name: "create annotation", user: u1, - this: &WithPod{ + this: &withPod{ Spec: getSpec("foo"), }, want: map[string]string{ @@ -349,7 +349,7 @@ func TestAnnotationCreate(t *testing.T) { }, { name: "create annotation should override user provided annotations", user: u1, - this: &WithPod{ + this: &withPod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ CreatorAnnotation: u2, @@ -384,13 +384,13 @@ func TestAnnotationUpdate(t *testing.T) { tests := []struct { name string user string - prev *WithPod - this *WithPod + prev *withPod + this *withPod want map[string]string }{{ name: "update annotation without spec changes", user: u2, - this: &WithPod{ + this: &withPod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ CreatorAnnotation: u1, @@ -399,7 +399,7 @@ func TestAnnotationUpdate(t *testing.T) { }, Spec: getSpec("foo"), }, - prev: &WithPod{ + prev: &withPod{ Spec: getSpec("foo"), }, want: map[string]string{ @@ -409,7 +409,7 @@ func TestAnnotationUpdate(t *testing.T) { }, { name: "update annotation with spec changes", user: u2, - this: &WithPod{ + this: &withPod{ ObjectMeta: metav1.ObjectMeta{ Annotations: map[string]string{ CreatorAnnotation: u1, @@ -418,7 +418,7 @@ func TestAnnotationUpdate(t *testing.T) { }, Spec: getSpec("bar"), }, - prev: &WithPod{ + prev: &withPod{ Spec: getSpec("foo"), }, want: map[string]string{