extract checking parentRefs before validating features

Kong · Dec 12, 2023 · 9ea2db1 · 9ea2db1
1 parent dac13a7
commit 9ea2db1
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 79 deletions.
diff --git a/internal/admission/handler.go b/internal/admission/handler.go
@@ -23,7 +23,6 @@ import (
 const (
 	KindKongPlugin        = "KongPlugin"
 	KindKongClusterPlugin = "KongClusterPlugin"
-	KindGateway           = gatewayapi.Kind("Gateway")
 )
 
 // RequestHandler is an HTTP server that can validate Kong Ingress Controllers'

diff --git a/internal/admission/validation/gateway/httproute.go b/internal/admission/validation/gateway/httproute.go
@@ -6,7 +6,6 @@ import (
 	"strings"
 
 	"github.com/kong/go-kong/kong"
-	"github.com/samber/lo"
 
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/translator"
 	"github.com/kong/kubernetes-ingress-controller/v3/internal/dataplane/translator/subtranslator"
@@ -67,6 +66,25 @@ func ValidateHTTPRoute(
 	return ok, msg, nil
 }
 
+// ValidateHTTPRouteParentRefs checks the group/kind of each parentRef in spec and allows only
+// empty or `gateway.networking.k8s.io.Gateway`.
+func ValidateHTTPRouteParentRefs(httproute *gatewayapi.HTTPRoute) error {
+	const KindGateway = gatewayapi.Kind("Gateway")
+
+	for parentRefIndex, parentRef := range httproute.Spec.ParentRefs {
+		if parentRef.Group != nil && *parentRef.Group != "" && *parentRef.Group != gatewayapi.V1Group {
+			return fmt.Errorf("parentRefs[%d]: %s is not a supported group for httproute parentRefs, only %s is supported",
+				parentRefIndex, *parentRef.Group, gatewayapi.V1Group)
+		}
+		if parentRef.Kind != nil && *parentRef.Kind != "" && *parentRef.Kind != KindGateway {
+			return fmt.Errorf("parentRefs[%d]: %s is not a supported kind for httproute parentRefs, only kind %s is supported",
+				parentRefIndex, *parentRef.Kind, KindGateway)
+		}
+	}
+
+	return nil
+}
+
 // -----------------------------------------------------------------------------
 // Validation - HTTPRoute - Private Functions
 // -----------------------------------------------------------------------------
@@ -98,34 +116,17 @@ func validateHTTPRouteListener(listener *gatewayapi.Listener) error {
 // HTTPRoute implementation and validates that the provided object is not using
 // any of those unsupported features.
 func validateHTTPRouteFeatures(httproute *gatewayapi.HTTPRoute, translatorFeatures translator.FeatureFlags) error {
-	unsupportedHTTPRouteFilterTypes := []gatewayapi.HTTPRouteFilterType{
-		gatewayapi.HTTPRouteFilterURLRewrite,
-		gatewayapi.HTTPRouteFilterRequestMirror,
+	unsupportedFilterMap := map[gatewayapi.HTTPRouteFilterType]struct{}{
+		gatewayapi.HTTPRouteFilterURLRewrite:    {},
+		gatewayapi.HTTPRouteFilterRequestMirror: {},
 	}
-	unsupportedFilterMap := lo.SliceToMap(unsupportedHTTPRouteFilterTypes, func(t gatewayapi.HTTPRouteFilterType) (gatewayapi.HTTPRouteFilterType, struct{}) {
-		return t, struct{}{}
-	})
-
 	const (
 		KindService = gatewayapi.Kind("Service")
-		KindGateway = gatewayapi.Kind("Gateway")
 	)
 
-	// Validate parentRefs: only allow empty, `/Gateway` or `gateway.networking.k8s.io/Gateway`
-	// REVIEW: Should this happen before we get here? we search for parent gateways before we reach here.
-	for parentRefIndex, parentRef := range httproute.Spec.ParentRefs {
-		if parentRef.Group != nil && *parentRef.Group != "" && *parentRef.Group != gatewayapi.V1Group {
-			return fmt.Errorf("parentRefs[%d]: %s is not a supported group for httproute parentRefs, only %s is supported",
-				parentRefIndex, *parentRef.Group, gatewayapi.V1Group)
-		}
-		if parentRef.Kind != nil && *parentRef.Kind != "" && *parentRef.Kind != KindGateway {
-			return fmt.Errorf("parentRefs[%d]: %s is not a supported kind for httproute parentRefs, only kind %s is supported",
-				parentRefIndex, *parentRef.Kind, KindGateway)
-		}
-	}
-
 	for ruleIndex, rule := range httproute.Spec.Rules {
-		// rule timeout is not supported.
+		// Rule timeout is not supported.
+		// TODO: remove the check after https://github.com/Kong/kubernetes-ingress-controller/issues/4914 fixed.
 		if rule.Timeouts != nil {
 			return fmt.Errorf("rules[%d]: rule timeout is unsupported", ruleIndex)
 		}
@@ -143,6 +144,7 @@ func validateHTTPRouteFeatures(httproute *gatewayapi.HTTPRoute, translatorFeatur
 				return fmt.Errorf("rules[%d].backendRefs[%d]: filters in backendRef is unsupported",
 					ruleIndex, refIndex)
 			}
+
 			// We don't support any backendRef types except Kubernetes Services.
 			if ref.BackendRef.Group != nil && *ref.BackendRef.Group != "core" && *ref.BackendRef.Group != "" {
 				return fmt.Errorf("rules[%d].backendRefs[%d]: %s is not a supported group for httproute backendRefs, only core is supported",

diff --git a/internal/admission/validation/gateway/httproute_test.go b/internal/admission/validation/gateway/httproute_test.go
@@ -539,61 +539,6 @@ func TestValidateHTTPRoute(t *testing.T) {
 			valid:         false,
 			validationMsg: "HTTPRoute spec did not pass validation: rules[0].backendRefs[0]: filters in backendRef is unsupported",
 		},
-		{
-			msg: "we do not support parentRef groups other than gateway.networking.k8s.io",
-			route: &gatewayapi.HTTPRoute{
-				ObjectMeta: metav1.ObjectMeta{
-					Namespace: corev1.NamespaceDefault,
-					Name:      "testing-httproute",
-				},
-				Spec: gatewayapi.HTTPRouteSpec{
-					CommonRouteSpec: gatewayapi.CommonRouteSpec{
-						ParentRefs: []gatewayapi.ParentReference{{
-							Group: lo.ToPtr(gatewayapi.Group("somegroup")),
-							Name:  "testing-gateway",
-						}},
-					},
-					Rules: []gatewayapi.HTTPRouteRule{{
-						Matches: []gatewayapi.HTTPRouteMatch{{
-							Headers: []gatewayapi.HTTPHeaderMatch{{
-								Name:  "Content-Type",
-								Value: "audio/vorbis",
-							}},
-						}},
-						BackendRefs: []gatewayapi.HTTPBackendRef{
-							{
-								BackendRef: gatewayapi.BackendRef{
-									BackendObjectReference: gatewayapi.BackendObjectReference{
-										Name: "service1",
-									},
-								},
-							},
-						},
-					}},
-				},
-			},
-			gateways: []*gatewayapi.Gateway{{
-				ObjectMeta: metav1.ObjectMeta{
-					Namespace: corev1.NamespaceDefault,
-					Name:      "testing-gateway",
-				},
-				Spec: gatewayapi.GatewaySpec{
-					Listeners: []gatewayapi.Listener{{
-						Name:     "http",
-						Port:     80,
-						Protocol: (gatewayapi.HTTPProtocolType),
-						AllowedRoutes: &gatewayapi.AllowedRoutes{
-							Kinds: []gatewayapi.RouteGroupKind{{
-								Group: &group,
-								Kind:  "HTTPRoute",
-							}},
-						},
-					}},
-				},
-			}},
-			valid:         false,
-			validationMsg: "HTTPRoute spec did not pass validation: parentRefs[0]: somegroup is not a supported group for httproute parentRefs, only gateway.networking.k8s.io is supported",
-		},
 	} {
 		t.Run(tt.msg, func(t *testing.T) {
 			// Passed routesValidator is irrelevant for the above test cases.

diff --git a/internal/admission/validator.go b/internal/admission/validator.go
@@ -441,6 +441,9 @@ func (validator KongHTTPValidator) ValidateGateway(
 func (validator KongHTTPValidator) ValidateHTTPRoute(
 	ctx context.Context, httproute gatewayapi.HTTPRoute,
 ) (bool, string, error) {
+	if err := gatewayvalidation.ValidateHTTPRouteParentRefs(&httproute); err != nil {
+		return false, fmt.Sprintf("HTTPRoute has invalid parentRef: %s", err), nil
+	}
 	// in order to be sure whether or not an HTTPRoute resource is managed by this
 	// controller we disallow references to Gateway resources that do not exist.
 	var managedGateways []*gatewayapi.Gateway

diff --git a/internal/admission/validator_test.go b/internal/admission/validator_test.go
@@ -915,6 +915,27 @@ func TestKongHTTPValidator_ValidateHTTPRoute(t *testing.T) {
 			wantOK:              false,
 			wantMessageContains: "referenced gatewayclass non-exist-gatewayclass not found",
 		},
+		{
+			name: "HTTPRoute with parentRef in kind other than Gateway should not pass the validation",
+			httpRoute: &gatewayapi.HTTPRoute{
+				ObjectMeta: metav1.ObjectMeta{
+					Namespace: "default",
+					Name:      "unsupported-parent-ref-kind",
+				},
+				Spec: gatewayapi.HTTPRouteSpec{
+					CommonRouteSpec: gatewayapi.CommonRouteSpec{
+						ParentRefs: []gatewayapi.ParentReference{
+							{
+								Group: lo.ToPtr(gatewayapi.Group("somegroup")),
+								Name:  "test-validate-httproute-no-gatewayclass",
+							},
+						},
+					},
+				},
+			},
+			wantOK:              false,
+			wantMessageContains: "HTTPRoute has invalid parentRef: parentRefs[0]: somegroup is not a supported group for httproute parentRefs, only gateway.networking.k8s.io is supported",
+		},
 		{
 			name: "HTTPRoute using unsupported filter should not pass the validation",
 			httpRoute: &gatewayapi.HTTPRoute{