From 9ea2db17f5325ed819b14975776727448b33ba13 Mon Sep 17 00:00:00 2001 From: Yi Tao Date: Tue, 12 Dec 2023 11:45:51 +0800 Subject: [PATCH] extract checking parentRefs before validating features --- internal/admission/handler.go | 1 - .../admission/validation/gateway/httproute.go | 48 ++++++++-------- .../validation/gateway/httproute_test.go | 55 ------------------- internal/admission/validator.go | 3 + internal/admission/validator_test.go | 21 +++++++ 5 files changed, 49 insertions(+), 79 deletions(-) diff --git a/internal/admission/handler.go b/internal/admission/handler.go index 7c1b84075d..bb1b6b35ae 100644 --- 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 index df49a7a8da..dde0fa8f57 100644 --- 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 index 83bcc8d737..7ef0ebd875 100644 --- 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 index 4c6bcb5b37..3a26ff8026 100644 --- 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 index 55d4aceeba..1bc011bdc1 100644 --- 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{