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 #8185 Part of knative-extensions/net-gateway-api#198
knative · Dec 8, 2021 · 1537e0b · 1537e0b
1 parent 4b9addb
commit 1537e0b
Show file tree

Hide file tree

Showing 6 changed files with 2 additions and 90 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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{