From dd1cde5e04f1c16b0e67cc00d045979cdfc9d0bc Mon Sep 17 00:00:00 2001 From: Jakub Warczarek Date: Fri, 20 Oct 2023 18:13:05 +0200 Subject: [PATCH 1/6] chore(tests): matching ports for TCP and TLS --- test/integration/tcproute_test.go | 66 +++++++++++++++---------------- test/integration/tlsroute_test.go | 11 +++++- 2 files changed, 41 insertions(+), 36 deletions(-) diff --git a/test/integration/tcproute_test.go b/test/integration/tcproute_test.go index 011998a135..8201c6759c 100644 --- a/test/integration/tcproute_test.go +++ b/test/integration/tcproute_test.go @@ -15,6 +15,7 @@ import ( "github.com/kong/kubernetes-testing-framework/pkg/clusters" ktfkong "github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/kong" "github.com/kong/kubernetes-testing-framework/pkg/utils/kubernetes/generators" + "github.com/samber/lo" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" @@ -27,6 +28,8 @@ import ( "github.com/kong/kubernetes-ingress-controller/v2/test/internal/helpers" ) +const gatewayTCPPortName = "tcp" + func TestTCPRouteEssentials(t *testing.T) { ctx := context.Background() RunWhenKongExpressionRouter(t) @@ -49,12 +52,12 @@ func TestTCPRouteEssentials(t *testing.T) { require.NoError(t, err) cleaner.Add(gwc) - t.Log("deploying a gateway to the test cluster using unmanaged gateway mode and port 8888") + t.Logf("deploying a gateway to the test cluster using unmanaged gateway mode and port %d", ktfkong.DefaultTCPServicePort) gatewayName := uuid.NewString() gateway, err := DeployGateway(ctx, gatewayClient, ns.Name, gatewayClassName, func(gw *gatewayapi.Gateway) { gw.Name = gatewayName gw.Spec.Listeners = []gatewayapi.Listener{{ - Name: "tcp", + Name: gatewayTCPPortName, Protocol: gatewayapi.TCPProtocolType, Port: gatewayapi.PortNumber(ktfkong.DefaultTCPServicePort), }} @@ -94,15 +97,13 @@ func TestTCPRouteEssentials(t *testing.T) { t.Logf("exposing deployment %s/%s via service", deployment1.Namespace, deployment1.Name) service1 := generators.NewServiceForDeployment(deployment1, corev1.ServiceTypeLoadBalancer) - // we have to override the ports so that we can map the default TCP port from - // the Kong Gateway deployment to the tcpecho port, as this is what will be - // used to route the traffic at the Gateway (at the time of writing, the - // Kong Gateway doesn't support an API for dynamically adding these ports. The - // ports must be added manually to the config or ENV). + // Use the same port as the default TCP port from the Kong Gateway deployment + // to the tcpecho port, as this is what will be used to route the traffic at the Gateway. + const service1Port = ktfkong.DefaultTCPServicePort service1.Spec.Ports = []corev1.ServicePort{{ Name: "tcp", Protocol: corev1.ProtocolTCP, - Port: ktfkong.DefaultTCPServicePort, + Port: service1Port, TargetPort: intstr.FromInt(test.EchoTCPPort), }} service1, err = env.Cluster().Client().CoreV1().Services(ns.Name).Create(ctx, service1, metav1.CreateOptions{}) @@ -110,24 +111,19 @@ func TestTCPRouteEssentials(t *testing.T) { cleaner.Add(service1) t.Logf("exposing deployment %s/%s via service", deployment2.Namespace, deployment2.Name) + const service2Port = 8080 // Use a different port that listening on the Gateway for TCP. service2 := generators.NewServiceForDeployment(deployment2, corev1.ServiceTypeLoadBalancer) - // we have to override the ports so that we can map the default TCP port from - // the Kong Gateway deployment to the tcpecho port, as this is what will be - // used to route the traffic at the Gateway (at the time of writing, the - // Kong Gateway doesn't support an API for dynamically adding these ports. The - // ports must be added manually to the config or ENV). service2.Spec.Ports = []corev1.ServicePort{{ Name: "tcp", Protocol: corev1.ProtocolTCP, - Port: ktfkong.DefaultTCPServicePort, + Port: service2Port, TargetPort: intstr.FromInt(test.EchoTCPPort), }} service2, err = env.Cluster().Client().CoreV1().Services(ns.Name).Create(ctx, service2, metav1.CreateOptions{}) assert.NoError(t, err) cleaner.Add(service2) - t.Logf("creating a tcproute to access deployment %s via kong", deployment1.Name) - tcpPortDefault := gatewayapi.PortNumber(ktfkong.DefaultTCPServicePort) + t.Logf("creating a TCPRoute to access deployment %s via kong", deployment1.Name) tcpRoute := &gatewayapi.TCPRoute{ ObjectMeta: metav1.ObjectMeta{ Name: uuid.NewString(), @@ -135,14 +131,15 @@ func TestTCPRouteEssentials(t *testing.T) { Spec: gatewayapi.TCPRouteSpec{ CommonRouteSpec: gatewayapi.CommonRouteSpec{ ParentRefs: []gatewayapi.ParentReference{{ - Name: gatewayapi.ObjectName(gatewayName), + Name: gatewayapi.ObjectName(gatewayName), + SectionName: lo.ToPtr(gatewayapi.SectionName(gatewayTCPPortName)), }}, }, Rules: []gatewayapi.TCPRouteRule{{ BackendRefs: []gatewayapi.BackendRef{{ BackendObjectReference: gatewayapi.BackendObjectReference{ Name: gatewayapi.ObjectName(service1.Name), - Port: &tcpPortDefault, + Port: lo.ToPtr(gatewayapi.PortNumber(service1Port)), }, }}, }}, @@ -254,7 +251,7 @@ func TestTCPRouteEssentials(t *testing.T) { gateway, err = DeployGateway(ctx, gatewayClient, ns.Name, gatewayClassName, func(gw *gatewayapi.Gateway) { gw.Name = gatewayName gw.Spec.Listeners = []gatewayapi.Listener{{ - Name: "tcp", + Name: gatewayTCPPortName, Protocol: gatewayapi.TCPProtocolType, Port: gatewayapi.PortNumber(ktfkong.DefaultTCPServicePort), }} @@ -292,7 +289,7 @@ func TestTCPRouteEssentials(t *testing.T) { gateway, err = DeployGateway(ctx, gatewayClient, ns.Name, gatewayClassName, func(gw *gatewayapi.Gateway) { gw.Name = gatewayName gw.Spec.Listeners = []gatewayapi.Listener{{ - Name: "tcp", + Name: gatewayTCPPortName, Protocol: gatewayapi.TCPProtocolType, Port: gatewayapi.PortNumber(ktfkong.DefaultTCPServicePort), }} @@ -317,13 +314,13 @@ func TestTCPRouteEssentials(t *testing.T) { { BackendObjectReference: gatewayapi.BackendObjectReference{ Name: gatewayapi.ObjectName(service1.Name), - Port: &tcpPortDefault, + Port: lo.ToPtr(gatewayapi.PortNumber(service1Port)), }, }, { BackendObjectReference: gatewayapi.BackendObjectReference{ Name: gatewayapi.ObjectName(service2.Name), - Port: &tcpPortDefault, + Port: lo.ToPtr(gatewayapi.PortNumber(service2Port)), }, }, } @@ -365,7 +362,7 @@ func TestTCPRouteEssentials(t *testing.T) { _, err = DeployGateway(ctx, gatewayClient, ns.Name, gatewayClassName, func(gw *gatewayapi.Gateway) { gw.Name = gatewayName gw.Spec.Listeners = []gatewayapi.Listener{{ - Name: "tcp", + Name: gatewayTCPPortName, Protocol: gatewayapi.TCPProtocolType, Port: gatewayapi.PortNumber(ktfkong.DefaultTCPServicePort), }} @@ -428,7 +425,7 @@ func TestTCPRouteReferenceGrant(t *testing.T) { gateway, err := DeployGateway(ctx, gatewayClient, ns.Name, gatewayClassName, func(gw *gatewayapi.Gateway) { gw.Name = gatewayName gw.Spec.Listeners = []gatewayapi.Listener{{ - Name: "tcp", + Name: gatewayTCPPortName, Protocol: gatewayapi.TCPProtocolType, Port: gatewayapi.PortNumber(ktfkong.DefaultTCPServicePort), }} @@ -468,15 +465,13 @@ func TestTCPRouteReferenceGrant(t *testing.T) { t.Logf("exposing deployment %s/%s via service", deployment1.Namespace, deployment1.Name) service1 := generators.NewServiceForDeployment(deployment1, corev1.ServiceTypeLoadBalancer) - // we have to override the ports so that we can map the default TCP port from - // the Kong Gateway deployment to the tcpecho port, as this is what will be - // used to route the traffic at the Gateway (at the time of writing, the - // Kong Gateway doesn't support an API for dynamically adding these ports. The - // ports must be added manually to the config or ENV). + // Use the same port as the default TCP port from the Kong Gateway deployment + // to the tcpecho port, as this is what will be used to route the traffic at the Gateway. + const service1Port = ktfkong.DefaultTCPServicePort service1.Spec.Ports = []corev1.ServicePort{{ Name: "tcp", Protocol: corev1.ProtocolTCP, - Port: ktfkong.DefaultTCPServicePort, + Port: service1Port, TargetPort: intstr.FromInt(test.EchoTCPPort), }} service1, err = env.Cluster().Client().CoreV1().Services(ns.Name).Create(ctx, service1, metav1.CreateOptions{}) @@ -484,11 +479,12 @@ func TestTCPRouteReferenceGrant(t *testing.T) { cleaner.Add(service1) t.Logf("exposing deployment %s/%s via service", deployment2.Namespace, deployment2.Name) + const service2Port = 8080 // Use a different port that listening on the Gateway for TCP. service2 := generators.NewServiceForDeployment(deployment2, corev1.ServiceTypeLoadBalancer) service2.Spec.Ports = []corev1.ServicePort{{ Name: "tcp", Protocol: corev1.ProtocolTCP, - Port: ktfkong.DefaultTCPServicePort, + Port: service2Port, TargetPort: intstr.FromInt(test.EchoTCPPort), }} service2, err = env.Cluster().Client().CoreV1().Services(otherNs.Name).Create(ctx, service2, metav1.CreateOptions{}) @@ -496,7 +492,6 @@ func TestTCPRouteReferenceGrant(t *testing.T) { cleaner.Add(service2) t.Logf("creating a tcproute to access deployment %s via kong", deployment1.Name) - tcpPortDefault := gatewayapi.PortNumber(ktfkong.DefaultTCPServicePort) remoteNamespace := gatewayapi.Namespace(otherNs.Name) tcproute := &gatewayapi.TCPRoute{ ObjectMeta: metav1.ObjectMeta{ @@ -505,7 +500,8 @@ func TestTCPRouteReferenceGrant(t *testing.T) { Spec: gatewayapi.TCPRouteSpec{ CommonRouteSpec: gatewayapi.CommonRouteSpec{ ParentRefs: []gatewayapi.ParentReference{{ - Name: gatewayapi.ObjectName(gatewayName), + Name: gatewayapi.ObjectName(gatewayName), + SectionName: lo.ToPtr(gatewayapi.SectionName(gatewayTCPPortName)), }}, }, Rules: []gatewayapi.TCPRouteRule{{ @@ -513,14 +509,14 @@ func TestTCPRouteReferenceGrant(t *testing.T) { { BackendObjectReference: gatewayapi.BackendObjectReference{ Name: gatewayapi.ObjectName(service1.Name), - Port: &tcpPortDefault, + Port: lo.ToPtr(gatewayapi.PortNumber(service1Port)), }, }, { BackendObjectReference: gatewayapi.BackendObjectReference{ Name: gatewayapi.ObjectName(service2.Name), Namespace: &remoteNamespace, - Port: &tcpPortDefault, + Port: lo.ToPtr(gatewayapi.PortNumber(service2Port)), }, }, }, diff --git a/test/integration/tlsroute_test.go b/test/integration/tlsroute_test.go index 3fb65d7c7a..1b28910588 100644 --- a/test/integration/tlsroute_test.go +++ b/test/integration/tlsroute_test.go @@ -17,10 +17,12 @@ import ( "github.com/kong/kubernetes-testing-framework/pkg/clusters" ktfkong "github.com/kong/kubernetes-testing-framework/pkg/clusters/addons/kong" "github.com/kong/kubernetes-testing-framework/pkg/utils/kubernetes/generators" + "github.com/samber/lo" "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" k8stypes "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/intstr" gatewayclient "sigs.k8s.io/gateway-api/pkg/client/clientset/versioned" "github.com/kong/kubernetes-ingress-controller/v2/internal/gatewayapi" @@ -204,7 +206,14 @@ func TestTLSRoutePassthroughReferenceGrant(t *testing.T) { cleaner.Add(service) t.Logf("exposing deployment %s/%s via service", deployment2.Namespace, deployment2.Name) + const service2Port = 8443 // Use a different port that listening on the Gateway for TLS. service2 := generators.NewServiceForDeployment(deployment2, corev1.ServiceTypeLoadBalancer) + service2.Spec.Ports = []corev1.ServicePort{{ + Name: "tls", + Protocol: corev1.ProtocolTCP, + Port: service2Port, + TargetPort: intstr.FromInt(tlsEchoPort), + }} service2, err = env.Cluster().Client().CoreV1().Services(ns.Name).Create(ctx, service2, metav1.CreateOptions{}) require.NoError(t, err) cleaner.Add(service2) @@ -233,7 +242,7 @@ func TestTLSRoutePassthroughReferenceGrant(t *testing.T) { { BackendObjectReference: gatewayapi.BackendObjectReference{ Name: gatewayapi.ObjectName(service2.Name), - Port: &backendTLSPort, + Port: lo.ToPtr(gatewayapi.PortNumber(service2Port)), }, }, }, From aaa505ae26833118d2f4f7e318d384ba67cfa094 Mon Sep 17 00:00:00 2001 From: Jakub Warczarek Date: Fri, 20 Oct 2023 18:25:30 +0200 Subject: [PATCH 2/6] fix: set proper destination port for TCPRoute and UDPRoute --- CHANGELOG.md | 4 + internal/dataplane/parser/parser.go | 32 ++ .../parser/translate_routes_helpers.go | 68 ++-- .../parser/translate_routes_helpers_test.go | 139 +++++-- .../dataplane/parser/translate_tcproute.go | 23 +- .../parser/translate_tcproute_test.go | 155 +++++++- .../dataplane/parser/translate_tlsroute.go | 16 +- .../dataplane/parser/translate_udproute.go | 16 +- .../parser/translate_udproute_test.go | 372 ++++++++++++++++-- test/integration/tcproute_test.go | 4 +- test/integration/tlsroute_test.go | 4 +- 11 files changed, 690 insertions(+), 143 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dcf149e18c..806677c7f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -144,6 +144,10 @@ Adding a new version? You'll need three changes: [#4813](https://github.com/Kong/kubernetes-ingress-controller/pull/4813) - Fixed an incorrect watch, set in UDPRoute controller watching UDProute status updates. [#4835](https://github.com/Kong/kubernetes-ingress-controller/pull/4835) +- Fixed setting proper destination port for TCPRoute and UDPRoute, now field `SectionName` + for `TCPRoute` and `UDPRoute` works as expected. It **breaks** some configurations that + relied on matching multiple Gateway's listener ports to ports of services automatically. + [#4928](https://github.com/Kong/kubernetes-ingress-controller/pull/4928) ### Changed diff --git a/internal/dataplane/parser/parser.go b/internal/dataplane/parser/parser.go index 98b8d51471..00bc44e76c 100644 --- a/internal/dataplane/parser/parser.go +++ b/internal/dataplane/parser/parser.go @@ -588,6 +588,38 @@ func (p *Parser) getCerts(secretsToSNIs SecretNameToSNIs) []certWrapper { return certs } +func (p *Parser) getGatewayListeningPorts( + routeNamespace string, + protocol gatewayapi.ProtocolType, + prs []gatewayapi.ParentReference, +) []gatewayapi.PortNumber { + var gwPorts []gatewayapi.PortNumber + for _, pr := range prs { + // When namespace is is explicitly specified in the parentRef, + // it should be used instead of namespace of the whole Route. + ns := string(lo.FromPtr(pr.Namespace)) + if ns == "" { + ns = routeNamespace + } + gw, err := p.storer.GetGateway(ns, string(pr.Name)) + if err != nil { + continue // Skip when attached Gateway is not found. + } + + // Get explicitly referenced Gateway listening ports by ParentReference configuration. + // If no sectionName is specified, all ports are used (according to the specification + // "When unspecified (empty string), this will reference the entire resource." - see + // https://github.com/kubernetes-sigs/gateway-api/blob/ebe9f31ef27819c3b29f698a3e9b91d279453c59/apis/v1/shared_types.go#L107). + gwPorts = append(gwPorts, lo.FilterMap(gw.Spec.Listeners, func(l gatewayapi.Listener, i int) (gatewayapi.PortNumber, bool) { + if (pr.SectionName == nil || *pr.SectionName == l.Name) && protocol == l.Protocol { + return l.Port, true + } + return 0, false + })...) + } + return gwPorts +} + func mergeCerts(logger logr.Logger, certLists ...[]certWrapper) []kongstate.Certificate { snisSeen := make(map[string]string) certsSeen := make(map[string]certWrapper) diff --git a/internal/dataplane/parser/translate_routes_helpers.go b/internal/dataplane/parser/translate_routes_helpers.go index d91ae8050e..409121b683 100644 --- a/internal/dataplane/parser/translate_routes_helpers.go +++ b/internal/dataplane/parser/translate_routes_helpers.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/kong/go-kong/kong" + "github.com/samber/lo" "sigs.k8s.io/controller-runtime/pkg/client" "github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/kongstate" @@ -31,8 +32,9 @@ type tRouteRule interface { gatewayapi.UDPRouteRule | gatewayapi.TCPRouteRule | gatewayapi.TLSRouteRule } -// getBackendRefs returns BackendRefs from TCPRouteRule, UDPRouteRule or TLSRouteRule. -func getBackendRefs[TRouteRule tRouteRule](t TRouteRule) ([]gatewayapi.BackendRef, error) { +// checkBackendRefs checks if BackendRefs are properly configured for +// TCPRouteRule, UDPRouteRule or TLSRouteRule. +func checkBackendRefs[TRouteRule tRouteRule](t TRouteRule) error { // This is necessary because as of go1.18 (and go1.19) one cannot use common // struct fields in generic code. // @@ -40,41 +42,33 @@ func getBackendRefs[TRouteRule tRouteRule](t TRouteRule) ([]gatewayapi.BackendRe switch tt := any(t).(type) { case gatewayapi.UDPRouteRule: if len(tt.BackendRefs) == 0 { - return nil, errors.New("UDPRoute rules must include at least one backendRef") + return errors.New("UDPRoute rules must include at least one backendRef") } - return tt.BackendRefs, nil case gatewayapi.TCPRouteRule: if len(tt.BackendRefs) == 0 { - return nil, errors.New("TCPRoute rules must include at least one backendRef") + return errors.New("TCPRoute rules must include at least one backendRef") } - return tt.BackendRefs, nil case gatewayapi.TLSRouteRule: // TLSRoutes don't require BackendRefs. - return tt.BackendRefs, nil } - - // This should never happen because we use type constraints on what types - // are accepted. - return nil, nil + return nil } // generateKongRoutesFromRouteRule converts a Gateway Route (TCP, UDP or TLS) rule // to one or more Kong Route objects to route traffic to services. func generateKongRoutesFromRouteRule[T tRoute, TRule tRouteRule]( route T, + gwPorts []gatewayapi.PortNumber, ruleNumber int, rule TRule, ) ([]kongstate.Route, error) { - backendRefs, err := getBackendRefs(rule) - if err != nil { + if err := checkBackendRefs(rule); err != nil { return []kongstate.Route{}, err } - - tags := util.GenerateTagsForObject(route) return []kongstate.Route{ { Ingress: util.FromK8sObject(route), - Route: routeToKongRoute(route, backendRefs, ruleNumber, tags), + Route: routeToKongRoute(route, gwPorts, ruleNumber, util.GenerateTagsForObject(route)), }, }, nil } @@ -82,16 +76,22 @@ func generateKongRoutesFromRouteRule[T tRoute, TRule tRouteRule]( // routeToKongRoute converts Gateway Route to kong.Route. func routeToKongRoute[TRoute tTCPorUDPorTLSRoute]( r TRoute, - backendRefs []gatewayapi.BackendRef, + gwPorts []gatewayapi.PortNumber, ruleNumber int, tags []*string, ) kong.Route { + destinations := lo.Map(gwPorts, func(p gatewayapi.PortNumber, _ int) *kong.CIDRPort { + return &kong.CIDRPort{ + Port: kong.Int(int(p)), + } + }) + var kr kong.Route switch rr := any(r).(type) { case *gatewayapi.UDPRoute: - kr = udpRouteToKongRoute(rr, backendRefs, ruleNumber) + kr = udpRouteToKongRoute(rr, destinations, ruleNumber) case *gatewayapi.TCPRoute: - kr = tcpRouteToKongRoute(rr, backendRefs, ruleNumber) + kr = tcpRouteToKongRoute(rr, destinations, ruleNumber) case *gatewayapi.TLSRoute: kr = tlsRouteToKongRoute(rr, ruleNumber) default: @@ -104,44 +104,30 @@ func routeToKongRoute[TRoute tTCPorUDPorTLSRoute]( func udpRouteToKongRoute( r *gatewayapi.UDPRoute, - backendRefs []gatewayapi.BackendRef, + destinations []*kong.CIDRPort, ruleNumber int, ) kong.Route { return kong.Route{ Name: kong.String( - generateRouteName(udpRouteType, r.Namespace, r.Name, ruleNumber)), + generateRouteName(udpRouteType, r.Namespace, r.Name, ruleNumber), + ), Protocols: kong.StringSlice("udp"), - Destinations: backendRefsToKongCIDRPorts(backendRefs), + Destinations: destinations, } } func tcpRouteToKongRoute( r *gatewayapi.TCPRoute, - backendRefs []gatewayapi.BackendRef, + destinations []*kong.CIDRPort, ruleNumber int, ) kong.Route { return kong.Route{ Name: kong.String( - generateRouteName(tcpRouteType, r.Namespace, r.Name, ruleNumber)), + generateRouteName(tcpRouteType, r.Namespace, r.Name, ruleNumber), + ), Protocols: kong.StringSlice("tcp"), - Destinations: backendRefsToKongCIDRPorts(backendRefs), - } -} - -func backendRefsToKongCIDRPorts(backendRefs []gatewayapi.BackendRef) []*kong.CIDRPort { - destinations := make([]*kong.CIDRPort, 0, len(backendRefs)) - for _, backendRef := range backendRefs { - if backendRef.Port == nil { - continue // Should we propagate the error? - } - - destinations = append(destinations, - &kong.CIDRPort{ - Port: kong.Int(int(*backendRef.Port)), - }, - ) + Destinations: destinations, } - return destinations } func tlsRouteToKongRoute(r *gatewayapi.TLSRoute, ruleNumber int) kong.Route { diff --git a/internal/dataplane/parser/translate_routes_helpers_test.go b/internal/dataplane/parser/translate_routes_helpers_test.go index 16ca1044c3..7c02a5aecb 100644 --- a/internal/dataplane/parser/translate_routes_helpers_test.go +++ b/internal/dataplane/parser/translate_routes_helpers_test.go @@ -3,7 +3,6 @@ package parser import ( "testing" - "github.com/google/go-cmp/cmp" "github.com/kong/go-kong/kong" "github.com/samber/lo" "github.com/stretchr/testify/require" @@ -15,9 +14,10 @@ import ( ) func TestGenerateKongRoutesFromRouteRule_TCP(t *testing.T) { - testcases := []struct { + testCases := []struct { name string route *gatewayapi.TCPRoute + gwPorts []gatewayapi.PortNumber routeRule gatewayapi.TCPRouteRule expected []kongstate.Route }{ @@ -29,6 +29,7 @@ func TestGenerateKongRoutesFromRouteRule_TCP(t *testing.T) { Namespace: "mynamespace", }, }, + gwPorts: []gatewayapi.PortNumber{8080}, routeRule: gatewayapi.TCPRouteRule{ BackendRefs: []gatewayapi.BackendRef{ { @@ -48,7 +49,57 @@ func TestGenerateKongRoutesFromRouteRule_TCP(t *testing.T) { Name: lo.ToPtr("tcproute.mynamespace.mytcproute-name.0.0"), Destinations: []*kong.CIDRPort{ { - Port: lo.ToPtr(1234), + Port: lo.ToPtr(8080), + }, + }, + Protocols: []*string{ + lo.ToPtr("tcp"), + }, + Tags: []*string{ + kong.String("k8s-name:mytcproute-name"), + kong.String("k8s-namespace:mynamespace"), + }, + }, + }, + }, + }, + { + name: "TCPRoute with multiple backends and different Gateway port get translated correctly to kong.Route", + route: &gatewayapi.TCPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mytcproute-name", + Namespace: "mynamespace", + }, + }, + routeRule: gatewayapi.TCPRouteRule{ + BackendRefs: []gatewayapi.BackendRef{ + { + BackendObjectReference: gatewayapi.BackendObjectReference{ + Port: lo.ToPtr(gatewayapi.PortNumber(1234)), + }, + }, + { + BackendObjectReference: gatewayapi.BackendObjectReference{ + Port: lo.ToPtr(gatewayapi.PortNumber(5678)), + }, + }, + }, + }, + gwPorts: []gatewayapi.PortNumber{8080, 8888}, + expected: []kongstate.Route{ + { + Ingress: util.K8sObjectInfo{ + Name: "mytcproute-name", + Namespace: "mynamespace", + }, + Route: kong.Route{ + Name: lo.ToPtr("tcproute.mynamespace.mytcproute-name.0.0"), + Destinations: []*kong.CIDRPort{ + { + Port: lo.ToPtr(8080), + }, + { + Port: lo.ToPtr(8888), }, }, Protocols: []*string{ @@ -64,24 +115,22 @@ func TestGenerateKongRoutesFromRouteRule_TCP(t *testing.T) { }, } - for _, tc := range testcases { + for _, tc := range testCases { tc := tc t.Run(tc.name, func(t *testing.T) { - kongRoutes, err := generateKongRoutesFromRouteRule(tc.route, 0, tc.routeRule) + kongRoutes, err := generateKongRoutesFromRouteRule(tc.route, tc.gwPorts, 0, tc.routeRule) require.NoError(t, err) require.NotNil(t, kongRoutes) - if !cmp.Equal(tc.expected, kongRoutes) { - t.Logf("actual []kongstate.Route differs from expected\n%s", cmp.Diff(tc.expected, kongRoutes)) - t.Fail() - } + require.Equal(t, tc.expected, kongRoutes) }) } } func TestGenerateKongRoutesFromRouteRule_UDP(t *testing.T) { - testcases := []struct { + testCases := []struct { name string route *gatewayapi.UDPRoute + gwPorts []gatewayapi.PortNumber routeRule gatewayapi.UDPRouteRule expected []kongstate.Route }{ @@ -102,6 +151,7 @@ func TestGenerateKongRoutesFromRouteRule_UDP(t *testing.T) { }, }, }, + gwPorts: []gatewayapi.PortNumber{8080}, expected: []kongstate.Route{ { Ingress: util.K8sObjectInfo{ @@ -112,7 +162,57 @@ func TestGenerateKongRoutesFromRouteRule_UDP(t *testing.T) { Name: lo.ToPtr("udproute.mynamespace.myudproute-name.0.0"), Destinations: []*kong.CIDRPort{ { - Port: lo.ToPtr(1234), + Port: lo.ToPtr(8080), + }, + }, + Protocols: []*string{ + lo.ToPtr("udp"), + }, + Tags: []*string{ + kong.String("k8s-name:myudproute-name"), + kong.String("k8s-namespace:mynamespace"), + }, + }, + }, + }, + }, + { + name: "UDPRoute with multiple backends and multiple Gateway ports gets translated correctly to kong.Route", + route: &gatewayapi.UDPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "myudproute-name", + Namespace: "mynamespace", + }, + }, + routeRule: gatewayapi.UDPRouteRule{ + BackendRefs: []gatewayapi.BackendRef{ + { + BackendObjectReference: gatewayapi.BackendObjectReference{ + Port: lo.ToPtr(gatewayapi.PortNumber(1234)), + }, + }, + { + BackendObjectReference: gatewayapi.BackendObjectReference{ + Port: lo.ToPtr(gatewayapi.PortNumber(5678)), + }, + }, + }, + }, + gwPorts: []gatewayapi.PortNumber{8080, 8888}, + expected: []kongstate.Route{ + { + Ingress: util.K8sObjectInfo{ + Name: "myudproute-name", + Namespace: "mynamespace", + }, + Route: kong.Route{ + Name: lo.ToPtr("udproute.mynamespace.myudproute-name.0.0"), + Destinations: []*kong.CIDRPort{ + { + Port: lo.ToPtr(8080), + }, + { + Port: lo.ToPtr(8888), }, }, Protocols: []*string{ @@ -128,16 +228,13 @@ func TestGenerateKongRoutesFromRouteRule_UDP(t *testing.T) { }, } - for _, tc := range testcases { + for _, tc := range testCases { tc := tc t.Run(tc.name, func(t *testing.T) { - kongRoutes, err := generateKongRoutesFromRouteRule(tc.route, 0, tc.routeRule) + kongRoutes, err := generateKongRoutesFromRouteRule(tc.route, tc.gwPorts, 0, tc.routeRule) require.NoError(t, err) require.NotNil(t, kongRoutes) - if !cmp.Equal(tc.expected, kongRoutes) { - t.Logf("actual []kongstate.Route differs from expected\n%s", cmp.Diff(tc.expected, kongRoutes)) - t.Fail() - } + require.Equal(t, tc.expected, kongRoutes) }) } } @@ -222,13 +319,11 @@ func TestGenerateKongRoutesFromRouteRule_TLS(t *testing.T) { for _, tc := range testcases { tc := tc t.Run(tc.name, func(t *testing.T) { - kongRoutes, err := generateKongRoutesFromRouteRule(tc.route, 0, tc.routeRule) + // TLSRoute matches based on hostname with Gateway listener thus passing gwPorts is pointless. + kongRoutes, err := generateKongRoutesFromRouteRule(tc.route, nil, 0, tc.routeRule) require.NoError(t, err) require.NotNil(t, kongRoutes) - if !cmp.Equal(tc.expected, kongRoutes) { - t.Logf("actual []kongstate.Route differs from expected\n%s", cmp.Diff(tc.expected, kongRoutes)) - t.Fail() - } + require.Equal(t, tc.expected, kongRoutes) }) } } diff --git a/internal/dataplane/parser/translate_tcproute.go b/internal/dataplane/parser/translate_tcproute.go index 5af1be158a..86aaa7cba6 100644 --- a/internal/dataplane/parser/translate_tcproute.go +++ b/internal/dataplane/parser/translate_tcproute.go @@ -38,20 +38,17 @@ func (p *Parser) ingressRulesFromTCPRoutes() ingressRules { applyExpressionToIngressRules(&result) } - if len(errs) > 0 { - for _, err := range errs { - p.logger.Error(err, "could not generate route from TCPRoute") - } + for _, err := range errs { + p.logger.Error(err, "could not generate route from TCPRoute") } return result } func (p *Parser) ingressRulesFromTCPRoute(result *ingressRules, tcproute *gatewayapi.TCPRoute) error { - // first we grab the spec and gather some metdata about the object spec := tcproute.Spec - // validation for TCPRoutes will happen at a higher layer, but in spite of that we run + // Validation for TCPRoutes will happen at a higher layer, but in spite of that we run // validation at this level as well as a fallback so that if routes are posted which // are invalid somehow make it past validation (e.g. the webhook is not enabled) we can // at least try to provide a helpful message about the situation in the manager logs. @@ -59,18 +56,14 @@ func (p *Parser) ingressRulesFromTCPRoute(result *ingressRules, tcproute *gatewa return translators.ErrRouteValidationNoRules } - // each rule may represent a different set of backend services that will be accepting + gwPorts := p.getGatewayListeningPorts(tcproute.Namespace, gatewayapi.TCPProtocolType, spec.CommonRouteSpec.ParentRefs) + + // Each rule may represent a different set of backend services that will be accepting // traffic, so we make separate routes and Kong services for every present rule. for ruleNumber, rule := range spec.Rules { - // TODO: add this to a generic TCPRoute validation, and then we should probably - // simply be calling validation on each tcproute object at the begininning - // of the topmost list. - if len(rule.BackendRefs) == 0 { - return fmt.Errorf("missing backendRef in rule") - } - // determine the routes needed to route traffic to services for this rule - routes, err := generateKongRoutesFromRouteRule(tcproute, ruleNumber, rule) + // Determine the routes needed to route traffic to services for this rule. + routes, err := generateKongRoutesFromRouteRule(tcproute, gwPorts, ruleNumber, rule) if err != nil { return err } diff --git a/internal/dataplane/parser/translate_tcproute_test.go b/internal/dataplane/parser/translate_tcproute_test.go index 3fada76180..50735259ad 100644 --- a/internal/dataplane/parser/translate_tcproute_test.go +++ b/internal/dataplane/parser/translate_tcproute_test.go @@ -1,6 +1,7 @@ package parser import ( + "fmt" "strings" "testing" @@ -24,25 +25,53 @@ func TestIngressRulesFromTCPRoutesUsingExpressionRoutes(t *testing.T) { testCases := []struct { name string + gateways []*gatewayapi.Gateway tcpRoutes []*gatewayapi.TCPRoute expectedKongServices []kongstate.Service expectedKongRoutes map[string][]kongstate.Route expectedFailures []failures.ResourceFailure }{ { - name: "tcproute with single rule and single backendref", + name: "TCPRoute with single rule, single backendref and Gateway in the same namespace", + gateways: []*gatewayapi.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway-1", + Namespace: "default", + }, + Spec: gatewayapi.GatewaySpec{ + Listeners: []gatewayapi.Listener{ + builder.NewListener("tcp80").WithPort(80).TCP().Build(), + builder.NewListener("udp80").WithPort(80).UDP().Build(), + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway-1", + Namespace: "test-1", + }, + }, + }, tcpRoutes: []*gatewayapi.TCPRoute{ { TypeMeta: tcpRouteTypeMeta, ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", Name: "tcproute-1", + Namespace: "default", }, Spec: gatewayapi.TCPRouteSpec{ + CommonRouteSpec: gatewayapi.CommonRouteSpec{ + ParentRefs: []gatewayapi.ParentReference{ + { + Name: "gateway-1", + }, + }, + }, Rules: []gatewayapi.TCPRouteRule{ { BackendRefs: []gatewayapi.BackendRef{ - builder.NewBackendRef("service1").WithPort(80).Build(), + builder.NewBackendRef("service1").WithPort(8080).Build(), }, }, }, @@ -57,7 +86,7 @@ func TestIngressRulesFromTCPRoutesUsingExpressionRoutes(t *testing.T) { Backends: []kongstate.ServiceBackend{ { Name: "service1", - PortDef: kongstate.PortDef{Mode: kongstate.PortModeByNumber, Number: int32(80)}, + PortDef: kongstate.PortDef{Mode: kongstate.PortModeByNumber, Number: int32(8080)}, }, }, }, @@ -77,15 +106,43 @@ func TestIngressRulesFromTCPRoutesUsingExpressionRoutes(t *testing.T) { }, }, { - name: "tcproute with single rule and multiple backendrefs", + name: "TCPRoute with single rule, multiple backendrefs and Gateway in the different namespace", + gateways: []*gatewayapi.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway-1", + Namespace: "test-1", + }, + Spec: gatewayapi.GatewaySpec{ + Listeners: []gatewayapi.Listener{ + builder.NewListener("tcp80").WithPort(80).TCP().Build(), + builder.NewListener("tcp443").WithPort(443).TCP().Build(), + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway-1", + Namespace: "default", + }, + }, + }, tcpRoutes: []*gatewayapi.TCPRoute{ { TypeMeta: tcpRouteTypeMeta, ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", Name: "tcproute-1", + Namespace: "default", }, Spec: gatewayapi.TCPRouteSpec{ + CommonRouteSpec: gatewayapi.CommonRouteSpec{ + ParentRefs: []gatewayapi.ParentReference{ + { + Name: "gateway-1", + Namespace: lo.ToPtr(gatewayapi.Namespace("test-1")), + }, + }, + }, Rules: []gatewayapi.TCPRouteRule{ { BackendRefs: []gatewayapi.BackendRef{ @@ -129,15 +186,60 @@ func TestIngressRulesFromTCPRoutesUsingExpressionRoutes(t *testing.T) { }, }, { - name: "tcproute with multiple rules", + name: "TCPRoute with multiple rules and multiple Gateways with multiple listeners and sectionName for Gateway specified", + gateways: []*gatewayapi.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway-1", + Namespace: "default", + }, + Spec: gatewayapi.GatewaySpec{ + Listeners: []gatewayapi.Listener{ + { + Name: "tcp80", + Port: 80, + Protocol: gatewayapi.TCPProtocolType, + }, + { + Name: "tcp443", + Port: 443, + Protocol: gatewayapi.TCPProtocolType, + }, + { + Name: "tcp8080", + Port: 8080, + Protocol: gatewayapi.TCPProtocolType, + }, + { + Name: "tcp8443", + Port: 8443, + Protocol: gatewayapi.TCPProtocolType, + }, + }, + }, + }, + }, tcpRoutes: []*gatewayapi.TCPRoute{ { TypeMeta: tcpRouteTypeMeta, ObjectMeta: metav1.ObjectMeta{ - Namespace: "default", Name: "tcproute-1", + Namespace: "default", }, Spec: gatewayapi.TCPRouteSpec{ + CommonRouteSpec: gatewayapi.CommonRouteSpec{ + ParentRefs: []gatewayapi.ParentReference{ + { + Name: "gateway-1", + SectionName: lo.ToPtr(gatewayapi.SectionName("tcp80")), + }, + { + Name: "gateway-1", + SectionName: lo.ToPtr(gatewayapi.SectionName("tcp443")), + }, + }, + }, + Rules: []gatewayapi.TCPRouteRule{ { BackendRefs: []gatewayapi.BackendRef{ @@ -145,6 +247,30 @@ func TestIngressRulesFromTCPRoutesUsingExpressionRoutes(t *testing.T) { builder.NewBackendRef("service2").WithPort(443).Build(), }, }, + }, + }, + }, + { + TypeMeta: tcpRouteTypeMeta, + ObjectMeta: metav1.ObjectMeta{ + Name: "tcproute-2", + Namespace: "default", + }, + Spec: gatewayapi.TCPRouteSpec{ + CommonRouteSpec: gatewayapi.CommonRouteSpec{ + ParentRefs: []gatewayapi.ParentReference{ + { + Name: "gateway-1", + SectionName: lo.ToPtr(gatewayapi.SectionName("tcp8080")), + }, + { + Name: "gateway-1", + SectionName: lo.ToPtr(gatewayapi.SectionName("tcp8443")), + }, + }, + }, + + Rules: []gatewayapi.TCPRouteRule{ { BackendRefs: []gatewayapi.BackendRef{ builder.NewBackendRef("service3").WithPort(8080).Build(), @@ -173,7 +299,7 @@ func TestIngressRulesFromTCPRoutesUsingExpressionRoutes(t *testing.T) { }, { Service: kong.Service{ - Name: kong.String("tcproute.default.tcproute-1.1"), + Name: kong.String("tcproute.default.tcproute-2.0"), }, Backends: []kongstate.ServiceBackend{ { @@ -199,10 +325,10 @@ func TestIngressRulesFromTCPRoutesUsingExpressionRoutes(t *testing.T) { ExpressionRoutes: true, }, }, - "tcproute.default.tcproute-1.1": { + "tcproute.default.tcproute-2.0": { { Route: kong.Route{ - Name: kong.String("tcproute.default.tcproute-1.1.0"), + Name: kong.String("tcproute.default.tcproute-2.0.0"), Expression: kong.String(`(net.dst.port == 8080) || (net.dst.port == 8443)`), PreserveHost: kong.Bool(true), Protocols: kong.StringSlice("tcp"), @@ -217,7 +343,10 @@ func TestIngressRulesFromTCPRoutesUsingExpressionRoutes(t *testing.T) { for _, tc := range testCases { tc := tc t.Run(tc.name, func(t *testing.T) { - fakeStore, err := store.NewFakeStore(store.FakeObjects{TCPRoutes: tc.tcpRoutes}) + fakeStore, err := store.NewFakeStore(store.FakeObjects{ + Gateways: tc.gateways, + TCPRoutes: tc.tcpRoutes, + }) require.NoError(t, err) parser := mustNewParser(t, fakeStore) parser.featureFlags.ExpressionRoutes = true @@ -244,7 +373,7 @@ func TestIngressRulesFromTCPRoutesUsingExpressionRoutes(t *testing.T) { routeName := expectedRoute.Name r, ok := kongRouteNameToRoute[*routeName] require.Truef(t, ok, "should find route %s", *routeName) - require.Equal(t, expectedRoute.Expression, r.Expression) + require.Equalf(t, expectedRoute.Expression, r.Expression, fmt.Sprintf("expected >> %s, actual >> %s", *expectedRoute.Expression, *r.Expression)) require.Equal(t, expectedRoute.Protocols, r.Protocols) } } diff --git a/internal/dataplane/parser/translate_tlsroute.go b/internal/dataplane/parser/translate_tlsroute.go index 5202e59110..f869bfad94 100644 --- a/internal/dataplane/parser/translate_tlsroute.go +++ b/internal/dataplane/parser/translate_tlsroute.go @@ -43,17 +43,14 @@ func (p *Parser) ingressRulesFromTLSRoutes() ingressRules { applyExpressionToIngressRules(&result) } - if len(errs) > 0 { - for _, err := range errs { - p.logger.Error(err, "could not generate route from TLSRoute") - } + for _, err := range errs { + p.logger.Error(err, "could not generate route from TLSRoute") } return result } func (p *Parser) ingressRulesFromTLSRoute(result *ingressRules, tlsroute *gatewayapi.TLSRoute) error { - // first we grab the spec and gather some metdata about the object spec := tlsroute.Spec if len(spec.Hostnames) == 0 { @@ -68,12 +65,13 @@ func (p *Parser) ingressRulesFromTLSRoute(result *ingressRules, tlsroute *gatewa return err } - // each rule may represent a different set of backend services that will be accepting + // Each rule may represent a different set of backend services that will be accepting // traffic, so we make separate routes and Kong services for every present rule. for ruleNumber, rule := range spec.Rules { - // determine the routes needed to route traffic to services for this rule - routes, err := generateKongRoutesFromRouteRule(tlsroute, ruleNumber, rule) - // change protocols in route to tls_passthrough. + // Determine the routes needed to route traffic to services for this rule. + // TLSRoute matches based on hostname with Gateway listener thus passing gwPorts is pointless. + routes, err := generateKongRoutesFromRouteRule(tlsroute, nil, ruleNumber, rule) + // Change protocols in route to tls_passthrough. if tlsPassthrough { for i := range routes { routes[i].Protocols = kong.StringSlice("tls_passthrough") diff --git a/internal/dataplane/parser/translate_udproute.go b/internal/dataplane/parser/translate_udproute.go index cb9b120a03..c63ea3bab8 100644 --- a/internal/dataplane/parser/translate_udproute.go +++ b/internal/dataplane/parser/translate_udproute.go @@ -53,14 +53,22 @@ func (p *Parser) ingressRulesFromUDPRoutes() ingressRules { } func (p *Parser) ingressRulesFromUDPRoute(result *ingressRules, udproute *gatewayapi.UDPRoute) error { - // first we grab the spec and gather some metadata about the object spec := udproute.Spec - // each rule may represent a different set of backend services that will be accepting + // Validation for TCPRoutes will happen at a higher layer, but in spite of that we run + // validation at this level as well as a fallback so that if routes are posted which + // are invalid somehow make it past validation (e.g. the webhook is not enabled) we can + // at least try to provide a helpful message about the situation in the manager logs. + if len(spec.Rules) == 0 { + return translators.ErrRouteValidationNoRules + } + + gwPorts := p.getGatewayListeningPorts(udproute.Namespace, gatewayapi.UDPProtocolType, spec.CommonRouteSpec.ParentRefs) + // Each rule may represent a different set of backend services that will be accepting // traffic, so we make separate routes and Kong services for every present rule. for ruleNumber, rule := range spec.Rules { - // determine the routes needed to route traffic to services for this rule - routes, err := generateKongRoutesFromRouteRule(udproute, ruleNumber, rule) + // Determine the routes needed to route traffic to services for this rule. + routes, err := generateKongRoutesFromRouteRule(udproute, gwPorts, ruleNumber, rule) if err != nil { return err } diff --git a/internal/dataplane/parser/translate_udproute_test.go b/internal/dataplane/parser/translate_udproute_test.go index aae5ff0e18..1719b4e784 100644 --- a/internal/dataplane/parser/translate_udproute_test.go +++ b/internal/dataplane/parser/translate_udproute_test.go @@ -25,18 +25,49 @@ var udpRouteTypeMeta = metav1.TypeMeta{Kind: "UDPRoute", APIVersion: gatewayv1al func TestIngressRulesFromUDPRoutes(t *testing.T) { testCases := []struct { name string + gateways []*gatewayapi.Gateway udpRoutes []*gatewayapi.UDPRoute expectedKongServices []kongstate.Service expectedKongRoutes map[string][]kongstate.Route expectedFailures []failures.ResourceFailure }{ { - name: "single UDPRoute with single rule", + name: "single UDPRoute with single rule, single backendref and Gateway in the same namespace", + gateways: []*gatewayapi.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway-1", + Namespace: "default", + }, + Spec: gatewayapi.GatewaySpec{ + Listeners: []gatewayapi.Listener{ + builder.NewListener("udp80").WithPort(80).UDP().Build(), + builder.NewListener("tcp80").WithPort(80).TCP().Build(), + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway-1", + Namespace: "test-1", + }, + }, + }, udpRoutes: []*gatewayapi.UDPRoute{ { - TypeMeta: udpRouteTypeMeta, - ObjectMeta: metav1.ObjectMeta{Name: "single-rule", Namespace: "default"}, + TypeMeta: udpRouteTypeMeta, + ObjectMeta: metav1.ObjectMeta{ + Name: "single-rule", + Namespace: "default", + }, Spec: gatewayapi.UDPRouteSpec{ + CommonRouteSpec: gatewayapi.CommonRouteSpec{ + ParentRefs: []gatewayapi.ParentReference{ + { + Name: "gateway-1", + }, + }, + }, Rules: []gatewayapi.UDPRouteRule{ { BackendRefs: []gatewayapi.BackendRef{ @@ -76,18 +107,68 @@ func TestIngressRulesFromUDPRoutes(t *testing.T) { }, }, { - name: "single UDPRoute with multiple rules", + name: "multiple UDPRoute with single rule, different SectionName and Gateway in the same namespace", + gateways: []*gatewayapi.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway-1", + Namespace: "default", + }, + Spec: gatewayapi.GatewaySpec{ + Listeners: []gatewayapi.Listener{ + builder.NewListener("udp80").WithPort(80).UDP().Build(), + builder.NewListener("udp81").WithPort(81).UDP().Build(), + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway-1", + Namespace: "test-1", + }, + }, + }, udpRoutes: []*gatewayapi.UDPRoute{ { - TypeMeta: udpRouteTypeMeta, - ObjectMeta: metav1.ObjectMeta{Name: "multiple-rules", Namespace: "default"}, + TypeMeta: udpRouteTypeMeta, + ObjectMeta: metav1.ObjectMeta{ + Name: "rule-1", + Namespace: "default", + }, Spec: gatewayapi.UDPRouteSpec{ + CommonRouteSpec: gatewayapi.CommonRouteSpec{ + ParentRefs: []gatewayapi.ParentReference{ + { + Name: "gateway-1", + SectionName: lo.ToPtr(gatewayapi.SectionName("udp80")), + }, + }, + }, Rules: []gatewayapi.UDPRouteRule{ { BackendRefs: []gatewayapi.BackendRef{ builder.NewBackendRef("service1").WithPort(80).Build(), }, }, + }, + }, + }, + { + TypeMeta: udpRouteTypeMeta, + ObjectMeta: metav1.ObjectMeta{ + Name: "rule-2", + Namespace: "default", + }, + Spec: gatewayapi.UDPRouteSpec{ + CommonRouteSpec: gatewayapi.CommonRouteSpec{ + ParentRefs: []gatewayapi.ParentReference{ + { + Name: "gateway-1", + SectionName: lo.ToPtr(gatewayapi.SectionName("udp81")), + }, + }, + }, + Rules: []gatewayapi.UDPRouteRule{ { BackendRefs: []gatewayapi.BackendRef{ builder.NewBackendRef("service2").WithPort(81).Build(), @@ -97,11 +178,10 @@ func TestIngressRulesFromUDPRoutes(t *testing.T) { }, }, }, - expectedKongServices: []kongstate.Service{ { Service: kong.Service{ - Name: kong.String("udproute.default.multiple-rules.0"), + Name: kong.String("udproute.default.rule-1.0"), Protocol: kong.String("udp"), }, Backends: []kongstate.ServiceBackend{ @@ -113,7 +193,7 @@ func TestIngressRulesFromUDPRoutes(t *testing.T) { }, { Service: kong.Service{ - Name: kong.String("udproute.default.multiple-rules.1"), + Name: kong.String("udproute.default.rule-2.0"), Protocol: kong.String("udp"), }, Backends: []kongstate.ServiceBackend{ @@ -125,10 +205,10 @@ func TestIngressRulesFromUDPRoutes(t *testing.T) { }, }, expectedKongRoutes: map[string][]kongstate.Route{ - "udproute.default.multiple-rules.0": { + "udproute.default.rule-1.0": { { Route: kong.Route{ - Name: kong.String("udproute.default.multiple-rules.0.0"), + Name: kong.String("udproute.default.rule-1.0.0"), Destinations: []*kong.CIDRPort{ {Port: kong.Int(80)}, }, @@ -136,10 +216,10 @@ func TestIngressRulesFromUDPRoutes(t *testing.T) { }, }, }, - "udproute.default.multiple-rules.1": { + "udproute.default.rule-2.0": { { Route: kong.Route{ - Name: kong.String("udproute.default.multiple-rules.1.0"), + Name: kong.String("udproute.default.rule-2.0.0"), Destinations: []*kong.CIDRPort{ {Port: kong.Int(81)}, }, @@ -150,12 +230,43 @@ func TestIngressRulesFromUDPRoutes(t *testing.T) { }, }, { - name: "single UDPRoute with single rule and multiple backendRefs", + name: "single UDPRoute with single rule and multiple backendRefs and Gateway in a different namespace", + gateways: []*gatewayapi.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway-1", + Namespace: "test-1", + }, + Spec: gatewayapi.GatewaySpec{ + Listeners: []gatewayapi.Listener{ + builder.NewListener("udp80").WithPort(80).UDP().Build(), + builder.NewListener("udp81").WithPort(81).UDP().Build(), + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway-1", + Namespace: "default", + }, + }, + }, udpRoutes: []*gatewayapi.UDPRoute{ { - TypeMeta: udpRouteTypeMeta, - ObjectMeta: metav1.ObjectMeta{Name: "multiple-backends", Namespace: "default"}, + TypeMeta: udpRouteTypeMeta, + ObjectMeta: metav1.ObjectMeta{ + Name: "multiple-backends", + Namespace: "default", + }, Spec: gatewayapi.UDPRouteSpec{ + CommonRouteSpec: gatewayapi.CommonRouteSpec{ + ParentRefs: []gatewayapi.ParentReference{ + { + Name: "gateway-1", + Namespace: lo.ToPtr(gatewayapi.Namespace("test-1")), + }, + }, + }, Rules: []gatewayapi.UDPRouteRule{ { BackendRefs: []gatewayapi.BackendRef{ @@ -201,12 +312,40 @@ func TestIngressRulesFromUDPRoutes(t *testing.T) { }, }, { - name: "multiple udproutes with translation errors", + name: "multiple UDPRoutes with translation errors", + gateways: []*gatewayapi.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway-1", + Namespace: "default", + }, + Spec: gatewayapi.GatewaySpec{ + Listeners: []gatewayapi.Listener{ + builder.NewListener("udp80").WithPort(80).UDP().Build(), + builder.NewListener("udp8080").WithPort(8080).UDP().Build(), + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway-1", + Namespace: "test-1", + }, + }, + }, udpRoutes: []*gatewayapi.UDPRoute{ { TypeMeta: udpRouteTypeMeta, ObjectMeta: metav1.ObjectMeta{Name: "single-rule", Namespace: "default"}, Spec: gatewayapi.UDPRouteSpec{ + CommonRouteSpec: gatewayapi.CommonRouteSpec{ + ParentRefs: []gatewayapi.ParentReference{ + { + Name: "gateway-1", + SectionName: lo.ToPtr(gatewayapi.SectionName("udp80")), + }, + }, + }, Rules: []gatewayapi.UDPRouteRule{ { BackendRefs: []gatewayapi.BackendRef{ @@ -220,6 +359,14 @@ func TestIngressRulesFromUDPRoutes(t *testing.T) { TypeMeta: udpRouteTypeMeta, ObjectMeta: metav1.ObjectMeta{Name: "single-rule-2", Namespace: "default"}, Spec: gatewayapi.UDPRouteSpec{ + CommonRouteSpec: gatewayapi.CommonRouteSpec{ + ParentRefs: []gatewayapi.ParentReference{ + { + Name: "gateway-1", + SectionName: lo.ToPtr(gatewayapi.SectionName("udp8080")), + }, + }, + }, Rules: []gatewayapi.UDPRouteRule{ { BackendRefs: []gatewayapi.BackendRef{ @@ -301,6 +448,7 @@ func TestIngressRulesFromUDPRoutes(t *testing.T) { tc := tc t.Run(tc.name, func(t *testing.T) { fakestore, err := store.NewFakeStore(store.FakeObjects{ + Gateways: tc.gateways, UDPRoutes: tc.udpRoutes, }) require.NoError(t, err) @@ -352,18 +500,48 @@ func TestIngressRulesFromUDPRoutes(t *testing.T) { func TestIngressRulesFromUDPRoutesUsingExpressionRoutes(t *testing.T) { testCases := []struct { name string + gateways []*gatewayapi.Gateway udpRoutes []*gatewayapi.UDPRoute expectedKongServices []kongstate.Service expectedKongRoutes map[string][]kongstate.Route expectedFailures []failures.ResourceFailure }{ { - name: "single UDPRoute with single rule", + name: "UDPRoute with single rule, single backendref and Gateway in the same namespace", + gateways: []*gatewayapi.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway-1", + Namespace: "default", + }, + Spec: gatewayapi.GatewaySpec{ + Listeners: []gatewayapi.Listener{ + builder.NewListener("udp80").WithPort(80).UDP().Build(), + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway-1", + Namespace: "test-1", + }, + }, + }, udpRoutes: []*gatewayapi.UDPRoute{ { - TypeMeta: udpRouteTypeMeta, - ObjectMeta: metav1.ObjectMeta{Name: "single-rule", Namespace: "default"}, + TypeMeta: udpRouteTypeMeta, + ObjectMeta: metav1.ObjectMeta{ + Name: "single-rule", + Namespace: "default", + }, Spec: gatewayapi.UDPRouteSpec{ + CommonRouteSpec: gatewayapi.CommonRouteSpec{ + ParentRefs: []gatewayapi.ParentReference{ + { + Name: "gateway-1", + }, + }, + }, Rules: []gatewayapi.UDPRouteRule{ { BackendRefs: []gatewayapi.BackendRef{ @@ -401,21 +579,73 @@ func TestIngressRulesFromUDPRoutesUsingExpressionRoutes(t *testing.T) { }, }, { - name: "single UDPRoute with multiple rules", + name: "UDPRoute with single rule, multiple backendrefs and Gateway in a different namespace", + gateways: []*gatewayapi.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway-1", + Namespace: "test-1", + }, + Spec: gatewayapi.GatewaySpec{ + Listeners: []gatewayapi.Listener{ + builder.NewListener("udp80").WithPort(80).UDP().Build(), + builder.NewListener("udp81").WithPort(81).UDP().Build(), + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway-1", + Namespace: "default", + }, + }, + }, udpRoutes: []*gatewayapi.UDPRoute{ { - TypeMeta: udpRouteTypeMeta, - ObjectMeta: metav1.ObjectMeta{Name: "multiple-rules", Namespace: "default"}, + TypeMeta: udpRouteTypeMeta, + ObjectMeta: metav1.ObjectMeta{ + Name: "rule-1", + Namespace: "default", + }, Spec: gatewayapi.UDPRouteSpec{ + CommonRouteSpec: gatewayapi.CommonRouteSpec{ + ParentRefs: []gatewayapi.ParentReference{ + { + Name: "gateway-1", + Namespace: lo.ToPtr(gatewayapi.Namespace("test-1")), + SectionName: lo.ToPtr(gatewayapi.SectionName("udp80")), + }, + }, + }, Rules: []gatewayapi.UDPRouteRule{ { BackendRefs: []gatewayapi.BackendRef{ - builder.NewBackendRef("service1").WithPort(80).Build(), + builder.NewBackendRef("service1").WithPort(8080).Build(), + }, + }, + }, + }, + }, + { + TypeMeta: udpRouteTypeMeta, + ObjectMeta: metav1.ObjectMeta{ + Name: "rule-2", + Namespace: "default", + }, + Spec: gatewayapi.UDPRouteSpec{ + CommonRouteSpec: gatewayapi.CommonRouteSpec{ + ParentRefs: []gatewayapi.ParentReference{ + { + Name: "gateway-1", + Namespace: lo.ToPtr(gatewayapi.Namespace("test-1")), + SectionName: lo.ToPtr(gatewayapi.SectionName("udp81")), }, }, + }, + Rules: []gatewayapi.UDPRouteRule{ { BackendRefs: []gatewayapi.BackendRef{ - builder.NewBackendRef("service2").WithPort(81).Build(), + builder.NewBackendRef("service2").WithPort(8181).Build(), }, }, }, @@ -425,43 +655,43 @@ func TestIngressRulesFromUDPRoutesUsingExpressionRoutes(t *testing.T) { expectedKongServices: []kongstate.Service{ { Service: kong.Service{ - Name: kong.String("udproute.default.multiple-rules.0"), + Name: kong.String("udproute.default.rule-1.0"), Protocol: kong.String("udp"), }, Backends: []kongstate.ServiceBackend{ { Name: "service1", - PortDef: kongstate.PortDef{Mode: kongstate.PortModeByNumber, Number: int32(80)}, + PortDef: kongstate.PortDef{Mode: kongstate.PortModeByNumber, Number: int32(8080)}, }, }, }, { Service: kong.Service{ - Name: kong.String("udproute.default.multiple-rules.1"), + Name: kong.String("udproute.default.rule-2.0"), Protocol: kong.String("udp"), }, Backends: []kongstate.ServiceBackend{ { Name: "service2", - PortDef: kongstate.PortDef{Mode: kongstate.PortModeByNumber, Number: int32(81)}, + PortDef: kongstate.PortDef{Mode: kongstate.PortModeByNumber, Number: int32(8181)}, }, }, }, }, expectedKongRoutes: map[string][]kongstate.Route{ - "udproute.default.multiple-rules.0": { + "udproute.default.rule-1.0": { { Route: kong.Route{ - Name: kong.String("udproute.default.multiple-rules.0.0"), + Name: kong.String("udproute.default.rule-1.0.0"), Expression: kong.String("net.dst.port == 80"), Protocols: kong.StringSlice("udp"), }, }, }, - "udproute.default.multiple-rules.1": { + "udproute.default.rule-2.0": { { Route: kong.Route{ - Name: kong.String("udproute.default.multiple-rules.1.0"), + Name: kong.String("udproute.default.rule-2.0.0"), Expression: kong.String("net.dst.port == 81"), Protocols: kong.StringSlice("udp"), }, @@ -471,11 +701,41 @@ func TestIngressRulesFromUDPRoutesUsingExpressionRoutes(t *testing.T) { }, { name: "single UDPRoute with single rule and multiple backendRefs", + gateways: []*gatewayapi.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway-1", + Namespace: "default", + }, + Spec: gatewayapi.GatewaySpec{ + Listeners: []gatewayapi.Listener{ + builder.NewListener("udp80").WithPort(80).UDP().Build(), + builder.NewListener("udp81").WithPort(81).UDP().Build(), + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway-1", + Namespace: "test-1", + }, + }, + }, udpRoutes: []*gatewayapi.UDPRoute{ { - TypeMeta: udpRouteTypeMeta, - ObjectMeta: metav1.ObjectMeta{Name: "multiple-backends", Namespace: "default"}, + TypeMeta: udpRouteTypeMeta, + ObjectMeta: metav1.ObjectMeta{ + Name: "multiple-backends", + Namespace: "default", + }, Spec: gatewayapi.UDPRouteSpec{ + CommonRouteSpec: gatewayapi.CommonRouteSpec{ + ParentRefs: []gatewayapi.ParentReference{ + { + Name: "gateway-1", + }, + }, + }, Rules: []gatewayapi.UDPRouteRule{ { BackendRefs: []gatewayapi.BackendRef{ @@ -518,12 +778,40 @@ func TestIngressRulesFromUDPRoutesUsingExpressionRoutes(t *testing.T) { }, }, { - name: "multiple udproutes with translation errors", + name: "multiple UDPRoutes with translation errors", + gateways: []*gatewayapi.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway-1", + Namespace: "default", + }, + Spec: gatewayapi.GatewaySpec{ + Listeners: []gatewayapi.Listener{ + builder.NewListener("udp80").WithPort(80).UDP().Build(), + builder.NewListener("udp8080").WithPort(8080).UDP().Build(), + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway-1", + Namespace: "test-1", + }, + }, + }, udpRoutes: []*gatewayapi.UDPRoute{ { TypeMeta: udpRouteTypeMeta, ObjectMeta: metav1.ObjectMeta{Name: "single-rule", Namespace: "default"}, Spec: gatewayapi.UDPRouteSpec{ + CommonRouteSpec: gatewayapi.CommonRouteSpec{ + ParentRefs: []gatewayapi.ParentReference{ + { + Name: "gateway-1", + SectionName: lo.ToPtr(gatewayapi.SectionName("udp80")), + }, + }, + }, Rules: []gatewayapi.UDPRouteRule{ { BackendRefs: []gatewayapi.BackendRef{ @@ -537,6 +825,14 @@ func TestIngressRulesFromUDPRoutesUsingExpressionRoutes(t *testing.T) { TypeMeta: udpRouteTypeMeta, ObjectMeta: metav1.ObjectMeta{Name: "single-rule-2", Namespace: "default"}, Spec: gatewayapi.UDPRouteSpec{ + CommonRouteSpec: gatewayapi.CommonRouteSpec{ + ParentRefs: []gatewayapi.ParentReference{ + { + Name: "gateway-1", + SectionName: lo.ToPtr(gatewayapi.SectionName("udp8080")), + }, + }, + }, Rules: []gatewayapi.UDPRouteRule{ { BackendRefs: []gatewayapi.BackendRef{ @@ -614,6 +910,7 @@ func TestIngressRulesFromUDPRoutesUsingExpressionRoutes(t *testing.T) { tc := tc t.Run(tc.name, func(t *testing.T) { fakestore, err := store.NewFakeStore(store.FakeObjects{ + Gateways: tc.gateways, UDPRoutes: tc.udpRoutes, }) require.NoError(t, err) @@ -645,7 +942,8 @@ func TestIngressRulesFromUDPRoutesUsingExpressionRoutes(t *testing.T) { routeName := expectedRoute.Name r, ok := kongRouteNameToRoute[*routeName] require.Truef(t, ok, "should find route %s", *routeName) - require.Equalf(t, expectedRoute.Expression, r.Expression, "route %s should have expected expression", *routeName) + // require.Equalf(t, expectedRoute.Expression, r.Expression, "route %s should have expected expression", *routeName) + require.Equalf(t, expectedRoute.Expression, r.Expression, "route %s should have expected expression >>> expected: %q >>> actual: %q", *routeName, *expectedRoute.Expression, *r.Expression) require.Equalf(t, expectedRoute.Protocols, r.Protocols, "route %s should have expected protocols", *routeName) } } diff --git a/test/integration/tcproute_test.go b/test/integration/tcproute_test.go index 8201c6759c..de01e318d2 100644 --- a/test/integration/tcproute_test.go +++ b/test/integration/tcproute_test.go @@ -111,7 +111,9 @@ func TestTCPRouteEssentials(t *testing.T) { cleaner.Add(service1) t.Logf("exposing deployment %s/%s via service", deployment2.Namespace, deployment2.Name) - const service2Port = 8080 // Use a different port that listening on the Gateway for TCP. + // Configure service to expose a different port than Gateway's TCP listener port (ktfkong.DefaultTCPServicePort) + // to check whether traffic will be routed correctly. + const service2Port = 8080 service2 := generators.NewServiceForDeployment(deployment2, corev1.ServiceTypeLoadBalancer) service2.Spec.Ports = []corev1.ServicePort{{ Name: "tcp", diff --git a/test/integration/tlsroute_test.go b/test/integration/tlsroute_test.go index 1b28910588..d34328efb3 100644 --- a/test/integration/tlsroute_test.go +++ b/test/integration/tlsroute_test.go @@ -206,7 +206,9 @@ func TestTLSRoutePassthroughReferenceGrant(t *testing.T) { cleaner.Add(service) t.Logf("exposing deployment %s/%s via service", deployment2.Namespace, deployment2.Name) - const service2Port = 8443 // Use a different port that listening on the Gateway for TLS. + // Configure service to expose a different port than Gateway's TLS listener port (ktfkong.DefaultTLSServicePort) + // to check whether traffic will be routed correctly. + const service2Port = 8443 service2 := generators.NewServiceForDeployment(deployment2, corev1.ServiceTypeLoadBalancer) service2.Spec.Ports = []corev1.ServicePort{{ Name: "tls", From c22f43033d10c5eae2fd11bfc6fa30bbcab66a3f Mon Sep 17 00:00:00 2001 From: Jakub Warczarek Date: Wed, 25 Oct 2023 09:58:37 +0200 Subject: [PATCH 3/6] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Patryk Małek --- internal/dataplane/parser/parser.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/dataplane/parser/parser.go b/internal/dataplane/parser/parser.go index 00bc44e76c..81f85c39c3 100644 --- a/internal/dataplane/parser/parser.go +++ b/internal/dataplane/parser/parser.go @@ -595,7 +595,7 @@ func (p *Parser) getGatewayListeningPorts( ) []gatewayapi.PortNumber { var gwPorts []gatewayapi.PortNumber for _, pr := range prs { - // When namespace is is explicitly specified in the parentRef, + // When namespace is explicitly specified in the parentRef, // it should be used instead of namespace of the whole Route. ns := string(lo.FromPtr(pr.Namespace)) if ns == "" { @@ -610,7 +610,7 @@ func (p *Parser) getGatewayListeningPorts( // If no sectionName is specified, all ports are used (according to the specification // "When unspecified (empty string), this will reference the entire resource." - see // https://github.com/kubernetes-sigs/gateway-api/blob/ebe9f31ef27819c3b29f698a3e9b91d279453c59/apis/v1/shared_types.go#L107). - gwPorts = append(gwPorts, lo.FilterMap(gw.Spec.Listeners, func(l gatewayapi.Listener, i int) (gatewayapi.PortNumber, bool) { + gwPorts = append(gwPorts, lo.FilterMap(gw.Spec.Listeners, func(l gatewayapi.Listener, _ int) (gatewayapi.PortNumber, bool) { if (pr.SectionName == nil || *pr.SectionName == l.Name) && protocol == l.Protocol { return l.Port, true } From b9996ee584294b71e994f8e8a00a1d931d3f84b0 Mon Sep 17 00:00:00 2001 From: Jakub Warczarek Date: Wed, 25 Oct 2023 09:59:18 +0200 Subject: [PATCH 4/6] Update internal/dataplane/parser/translate_udproute_test.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Patryk Małek --- internal/dataplane/parser/translate_udproute_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/dataplane/parser/translate_udproute_test.go b/internal/dataplane/parser/translate_udproute_test.go index 1719b4e784..37445799ba 100644 --- a/internal/dataplane/parser/translate_udproute_test.go +++ b/internal/dataplane/parser/translate_udproute_test.go @@ -942,7 +942,6 @@ func TestIngressRulesFromUDPRoutesUsingExpressionRoutes(t *testing.T) { routeName := expectedRoute.Name r, ok := kongRouteNameToRoute[*routeName] require.Truef(t, ok, "should find route %s", *routeName) - // require.Equalf(t, expectedRoute.Expression, r.Expression, "route %s should have expected expression", *routeName) require.Equalf(t, expectedRoute.Expression, r.Expression, "route %s should have expected expression >>> expected: %q >>> actual: %q", *routeName, *expectedRoute.Expression, *r.Expression) require.Equalf(t, expectedRoute.Protocols, r.Protocols, "route %s should have expected protocols", *routeName) } From 2c7eb3d2f2e85bb8de89b46970c30858f71b9bd8 Mon Sep 17 00:00:00 2001 From: Jakub Warczarek Date: Wed, 25 Oct 2023 10:00:32 +0200 Subject: [PATCH 5/6] Fix CR --- internal/dataplane/parser/translate_tcproute_test.go | 3 +-- internal/dataplane/parser/translate_udproute_test.go | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/internal/dataplane/parser/translate_tcproute_test.go b/internal/dataplane/parser/translate_tcproute_test.go index 50735259ad..f3c3d43e4e 100644 --- a/internal/dataplane/parser/translate_tcproute_test.go +++ b/internal/dataplane/parser/translate_tcproute_test.go @@ -1,7 +1,6 @@ package parser import ( - "fmt" "strings" "testing" @@ -373,7 +372,7 @@ func TestIngressRulesFromTCPRoutesUsingExpressionRoutes(t *testing.T) { routeName := expectedRoute.Name r, ok := kongRouteNameToRoute[*routeName] require.Truef(t, ok, "should find route %s", *routeName) - require.Equalf(t, expectedRoute.Expression, r.Expression, fmt.Sprintf("expected >> %s, actual >> %s", *expectedRoute.Expression, *r.Expression)) + require.Equalf(t, expectedRoute.Expression, r.Expression, "route %s should have expected expression", *routeName) require.Equal(t, expectedRoute.Protocols, r.Protocols) } } diff --git a/internal/dataplane/parser/translate_udproute_test.go b/internal/dataplane/parser/translate_udproute_test.go index 37445799ba..fca8ce2767 100644 --- a/internal/dataplane/parser/translate_udproute_test.go +++ b/internal/dataplane/parser/translate_udproute_test.go @@ -942,7 +942,7 @@ func TestIngressRulesFromUDPRoutesUsingExpressionRoutes(t *testing.T) { routeName := expectedRoute.Name r, ok := kongRouteNameToRoute[*routeName] require.Truef(t, ok, "should find route %s", *routeName) - require.Equalf(t, expectedRoute.Expression, r.Expression, "route %s should have expected expression >>> expected: %q >>> actual: %q", *routeName, *expectedRoute.Expression, *r.Expression) + require.Equalf(t, expectedRoute.Expression, r.Expression, "route %s should have expected expression", *routeName) require.Equalf(t, expectedRoute.Protocols, r.Protocols, "route %s should have expected protocols", *routeName) } } From e224eea868413cdac5ae243ebf51f97f19213f94 Mon Sep 17 00:00:00 2001 From: Jakub Warczarek Date: Wed, 25 Oct 2023 12:06:13 +0200 Subject: [PATCH 6/6] remove redundant comment --- internal/dataplane/parser/translate_tcproute.go | 5 ----- internal/dataplane/parser/translate_udproute.go | 5 ----- 2 files changed, 10 deletions(-) diff --git a/internal/dataplane/parser/translate_tcproute.go b/internal/dataplane/parser/translate_tcproute.go index 86aaa7cba6..6a632a8333 100644 --- a/internal/dataplane/parser/translate_tcproute.go +++ b/internal/dataplane/parser/translate_tcproute.go @@ -47,11 +47,6 @@ func (p *Parser) ingressRulesFromTCPRoutes() ingressRules { func (p *Parser) ingressRulesFromTCPRoute(result *ingressRules, tcproute *gatewayapi.TCPRoute) error { spec := tcproute.Spec - - // Validation for TCPRoutes will happen at a higher layer, but in spite of that we run - // validation at this level as well as a fallback so that if routes are posted which - // are invalid somehow make it past validation (e.g. the webhook is not enabled) we can - // at least try to provide a helpful message about the situation in the manager logs. if len(spec.Rules) == 0 { return translators.ErrRouteValidationNoRules } diff --git a/internal/dataplane/parser/translate_udproute.go b/internal/dataplane/parser/translate_udproute.go index c63ea3bab8..eefc4cfef9 100644 --- a/internal/dataplane/parser/translate_udproute.go +++ b/internal/dataplane/parser/translate_udproute.go @@ -54,11 +54,6 @@ func (p *Parser) ingressRulesFromUDPRoutes() ingressRules { func (p *Parser) ingressRulesFromUDPRoute(result *ingressRules, udproute *gatewayapi.UDPRoute) error { spec := udproute.Spec - - // Validation for TCPRoutes will happen at a higher layer, but in spite of that we run - // validation at this level as well as a fallback so that if routes are posted which - // are invalid somehow make it past validation (e.g. the webhook is not enabled) we can - // at least try to provide a helpful message about the situation in the manager logs. if len(spec.Rules) == 0 { return translators.ErrRouteValidationNoRules }