From 1537e0bed12bdd9377d49eba0209312b95accbbc Mon Sep 17 00:00:00 2001 From: Kenjiro Nakayama Date: Wed, 8 Dec 2021 22:45:39 +0900 Subject: [PATCH] Relax label validation for networking.knative.dev/visibility (#12397) This patch removes `networking.knative.dev/visibility` validation. Curently only `cluster-local` can be specified. Fix https://github.com/knative/serving/issues/8185 Part of https://github.com/knative-sandbox/net-gateway-api/issues/198 --- pkg/apis/serving/metadata_validation.go | 8 ----- pkg/apis/serving/metadata_validation_test.go | 30 ------------------- pkg/apis/serving/v1/route_validation.go | 13 -------- pkg/apis/serving/v1/route_validation_test.go | 16 ++-------- pkg/apis/serving/v1/service_validation.go | 10 ------- .../serving/v1/service_validation_test.go | 15 ---------- 6 files changed, 2 insertions(+), 90 deletions(-) diff --git a/pkg/apis/serving/metadata_validation.go b/pkg/apis/serving/metadata_validation.go index 0ca4c31c7399..e98d5c5447d4 100644 --- a/pkg/apis/serving/metadata_validation.go +++ b/pkg/apis/serving/metadata_validation.go @@ -107,14 +107,6 @@ func ValidateContainerConcurrency(ctx context.Context, containerConcurrency *int return nil } -// validateClusterVisibilityLabel function validates the visibility label on a Route -func validateClusterVisibilityLabel(label, key string) (errs *apis.FieldError) { - if label != VisibilityClusterLocal { - errs = apis.ErrInvalidValue(label, key) - } - return errs -} - // SetUserInfo sets creator and updater annotations func SetUserInfo(ctx context.Context, oldSpec, newSpec, resource interface{}) { if ui := apis.GetUserInfo(ctx); ui != nil { diff --git a/pkg/apis/serving/metadata_validation_test.go b/pkg/apis/serving/metadata_validation_test.go index ab2c7d55fce6..bbdc3c82be98 100644 --- a/pkg/apis/serving/metadata_validation_test.go +++ b/pkg/apis/serving/metadata_validation_test.go @@ -25,7 +25,6 @@ import ( authv1 "k8s.io/api/authentication/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - network "knative.dev/networking/pkg" "knative.dev/pkg/apis" "knative.dev/pkg/ptr" "knative.dev/serving/pkg/apis/autoscaling" @@ -317,35 +316,6 @@ func TestValidateContainerConcurrency(t *testing.T) { } } -func TestValidateClusterVisibilityLabel(t *testing.T) { - tests := []struct { - name string - label string - expectErr *apis.FieldError - }{{ - name: "empty label", - label: "", - expectErr: apis.ErrInvalidValue("", network.VisibilityLabelKey), - }, { - name: "valid label", - label: VisibilityClusterLocal, - }, { - name: "invalid label", - label: "not-cluster-local", - expectErr: apis.ErrInvalidValue("not-cluster-local", network.VisibilityLabelKey), - }} - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - err := validateClusterVisibilityLabel(test.label, network.VisibilityLabelKey) - if got, want := err.Error(), test.expectErr.Error(); got != want { - t.Errorf("\nGot: %q\nwant: %q", got, want) - } - }) - } - -} - type withPod struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` diff --git a/pkg/apis/serving/v1/route_validation.go b/pkg/apis/serving/v1/route_validation.go index f0ba152139f4..80a94412c193 100644 --- a/pkg/apis/serving/v1/route_validation.go +++ b/pkg/apis/serving/v1/route_validation.go @@ -21,7 +21,6 @@ import ( "fmt" "k8s.io/apimachinery/pkg/util/validation" - network "knative.dev/networking/pkg" "knative.dev/pkg/apis" "knative.dev/serving/pkg/apis/serving" ) @@ -193,20 +192,8 @@ func (tt *TrafficTarget) validateURL(ctx context.Context, errs *apis.FieldError) return errs } -func validateClusterVisibilityLabel(label string) *apis.FieldError { - if label != serving.VisibilityClusterLocal { - return apis.ErrInvalidValue(label, network.VisibilityLabelKey) - } - - return nil -} - // validateLabels function validates route labels. func (r *Route) validateLabels() (errs *apis.FieldError) { - if val, ok := r.Labels[network.VisibilityLabelKey]; ok { - errs = errs.Also(validateClusterVisibilityLabel(val)) - } - if val, ok := r.Labels[serving.ServiceLabelKey]; ok { errs = errs.Also(verifyLabelOwnerRef(val, serving.ServiceLabelKey, "Service", r.GetOwnerReferences())) } diff --git a/pkg/apis/serving/v1/route_validation_test.go b/pkg/apis/serving/v1/route_validation_test.go index 76678adfa94d..ffda85b896ca 100644 --- a/pkg/apis/serving/v1/route_validation_test.go +++ b/pkg/apis/serving/v1/route_validation_test.go @@ -480,28 +480,16 @@ func TestRouteLabelValidation(t *testing.T) { r *Route want *apis.FieldError }{{ - name: "valid visibility name", + name: "visibility label specified", r: &Route{ ObjectMeta: metav1.ObjectMeta{ Name: "byo-name", Labels: map[string]string{ - network.VisibilityLabelKey: "cluster-local", + network.VisibilityLabelKey: "my-visibility", }, }, Spec: validRouteSpec, }, - }, { - name: "invalid visibility name", - r: &Route{ - ObjectMeta: metav1.ObjectMeta{ - Name: "byo-name", - Labels: map[string]string{ - network.VisibilityLabelKey: "bad-value", - }, - }, - Spec: validRouteSpec, - }, - want: apis.ErrInvalidValue("bad-value", "metadata.labels."+network.VisibilityLabelKey), }, { name: "valid knative service name", r: &Route{ diff --git a/pkg/apis/serving/v1/service_validation.go b/pkg/apis/serving/v1/service_validation.go index 33b54aadb242..4f63ced3f777 100644 --- a/pkg/apis/serving/v1/service_validation.go +++ b/pkg/apis/serving/v1/service_validation.go @@ -19,7 +19,6 @@ package v1 import ( "context" - network "knative.dev/networking/pkg" "knative.dev/pkg/apis" "knative.dev/serving/pkg/apis/serving" ) @@ -32,7 +31,6 @@ func (s *Service) Validate(ctx context.Context) (errs *apis.FieldError) { // spec validation. if !apis.IsInStatusUpdate(ctx) { errs = errs.Also(serving.ValidateObjectMetadata(ctx, s.GetObjectMeta(), false)) - errs = errs.Also(s.validateLabels().ViaField("labels")) errs = errs.Also(serving.ValidateRolloutDurationAnnotation( s.GetAnnotations()).ViaField("annotations")) errs = errs.ViaField("metadata") @@ -61,11 +59,3 @@ func (ss *ServiceSpec) Validate(ctx context.Context) *apis.FieldError { // configurationName. ss.RouteSpec.Validate(WithDefaultConfigurationName(ctx))) } - -// validateLabels function validates service labels -func (s *Service) validateLabels() (errs *apis.FieldError) { - if val, ok := s.Labels[network.VisibilityLabelKey]; ok { - errs = errs.Also(validateClusterVisibilityLabel(val)) - } - return errs -} diff --git a/pkg/apis/serving/v1/service_validation_test.go b/pkg/apis/serving/v1/service_validation_test.go index 07cd57dbe82c..b482d1e7de84 100644 --- a/pkg/apis/serving/v1/service_validation_test.go +++ b/pkg/apis/serving/v1/service_validation_test.go @@ -97,21 +97,6 @@ func TestServiceValidation(t *testing.T) { RouteSpec: goodRouteSpec, }, }, - }, { - name: "invalid visibility label value", - r: &Service{ - ObjectMeta: metav1.ObjectMeta{ - Name: "valid", - Labels: map[string]string{ - network.VisibilityLabelKey: "bad-label", - }, - }, - Spec: ServiceSpec{ - ConfigurationSpec: goodConfigSpec, - RouteSpec: goodRouteSpec, - }, - }, - wantErr: apis.ErrInvalidValue("bad-label", "metadata.labels."+network.VisibilityLabelKey), }, { name: "valid release", r: &Service{