From cca94121b0a5d41e998a9e4b702a9a0bfbf7fbf3 Mon Sep 17 00:00:00 2001 From: Victor Agababov Date: Wed, 30 Jan 2019 13:56:59 -0800 Subject: [PATCH 1/2] add validation for the revision entries --- pkg/apis/serving/v1alpha1/service_validation.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/apis/serving/v1alpha1/service_validation.go b/pkg/apis/serving/v1alpha1/service_validation.go index eae300f432c5..bd19f23aefc5 100644 --- a/pkg/apis/serving/v1alpha1/service_validation.go +++ b/pkg/apis/serving/v1alpha1/service_validation.go @@ -20,6 +20,7 @@ import ( "strconv" "github.com/knative/pkg/apis" + "k8s.io/apimachinery/pkg/util/validation" ) // Validate validates the fields belonging to Service @@ -96,6 +97,11 @@ func (rt *ReleaseType) Validate() *apis.FieldError { if numRevisions > 2 { errs = errs.Also(apis.ErrOutOfBoundsValue(strconv.Itoa(numRevisions), "1", "2", "revisions")) } + for i, r := range rt.Revisions { + if msgs := validation.IsDNS1035Label(r); len(msgs) > 0 { + errs.Also(apis.ErrInvalidValue(r, "revisions").ViaIndex(i)) + } + } if numRevisions < 2 && rt.RolloutPercent != 0 { errs = errs.Also(apis.ErrInvalidValue(strconv.Itoa(rt.RolloutPercent), "rolloutPercent")) From 124bdb92842d4da1e39053cba27ad326236628b0 Mon Sep 17 00:00:00 2001 From: Victor Agababov Date: Wed, 30 Jan 2019 17:05:53 -0800 Subject: [PATCH 2/2] Add validation to the revisions provided in the revision list. Since the revisions are supposed to be k8s objects, their names are supposed to follow the same naming convention. Thus validate the revision names to make sure the servive at least has a chance of serving something. --- .../serving/v1alpha1/service_validation.go | 4 +- .../v1alpha1/service_validation_test.go | 48 ++++++++++++++++++- 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/pkg/apis/serving/v1alpha1/service_validation.go b/pkg/apis/serving/v1alpha1/service_validation.go index d655ec6bab84..51f81c1526c6 100644 --- a/pkg/apis/serving/v1alpha1/service_validation.go +++ b/pkg/apis/serving/v1alpha1/service_validation.go @@ -17,6 +17,7 @@ limitations under the License. package v1alpha1 import ( + "fmt" "strconv" "github.com/knative/pkg/apis" @@ -99,7 +100,8 @@ func (rt *ReleaseType) Validate() *apis.FieldError { } for i, r := range rt.Revisions { if msgs := validation.IsDNS1035Label(r); len(msgs) > 0 { - errs.Also(apis.ErrInvalidValue(r, "revisions").ViaIndex(i)) + errs = errs.Also(apis.ErrInvalidValue( + fmt.Sprintf("not a DNS 1035 label: %v", msgs), apis.CurrentField).ViaFieldIndex("revisions", i)) } } diff --git a/pkg/apis/serving/v1alpha1/service_validation_test.go b/pkg/apis/serving/v1alpha1/service_validation_test.go index 7ca741e6825a..6365b69d5f42 100644 --- a/pkg/apis/serving/v1alpha1/service_validation_test.go +++ b/pkg/apis/serving/v1alpha1/service_validation_test.go @@ -27,6 +27,8 @@ import ( "github.com/knative/pkg/apis" ) +const incorrectDNS1035Label = "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])?')]" + func TestServiceValidation(t *testing.T) { tests := []struct { name string @@ -244,6 +246,50 @@ func TestServiceValidation(t *testing.T) { }, }, want: apis.ErrMissingField("spec.release.revisions"), + }, { + name: "invalid release -- revision name invalid, long", + s: &Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + }, + Spec: ServiceSpec{ + Release: &ReleaseType{ + Revisions: []string{strings.Repeat("a", 64)}, + Configuration: ConfigurationSpec{ + RevisionTemplate: RevisionTemplateSpec{ + Spec: RevisionSpec{ + Container: corev1.Container{ + Image: "hellworld", + }, + }, + }, + }, + }, + }, + }, + want: apis.ErrInvalidValue("not a DNS 1035 label: [must be no more than 63 characters]", "spec.release.revisions[0]"), + }, { + name: "invalid release -- revision name invalid, incorrect", + s: &Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "valid", + }, + Spec: ServiceSpec{ + Release: &ReleaseType{ + Revisions: []string{".negative"}, + Configuration: ConfigurationSpec{ + RevisionTemplate: RevisionTemplateSpec{ + Spec: RevisionSpec{ + Container: corev1.Container{ + Image: "hellworld", + }, + }, + }, + }, + }, + }, + }, + want: apis.ErrInvalidValue(incorrectDNS1035Label, "spec.release.revisions[0]"), }, { name: "invalid release -- too few revisions; empty slice", s: &Service{ @@ -378,7 +424,7 @@ func TestServiceValidation(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])?')]", + Message: incorrectDNS1035Label, Paths: []string{"metadata.name"}, }, }, {