diff --git a/CHANGELOG.md b/CHANGELOG.md index a8c643d836..d1a2d4009c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -144,6 +144,9 @@ 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. + [#4928](https://github.com/Kong/kubernetes-ingress-controller/pull/4928) ### Changed diff --git a/internal/dataplane/parser/translate_routes_helpers.go b/internal/dataplane/parser/translate_routes_helpers.go index d91ae8050e..9e0293cb5f 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" @@ -62,6 +63,7 @@ func getBackendRefs[TRouteRule tRouteRule](t TRouteRule) ([]gatewayapi.BackendRe // 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) { @@ -69,12 +71,19 @@ func generateKongRoutesFromRouteRule[T tRoute, TRule tRouteRule]( if err != nil { return []kongstate.Route{}, err } - - tags := util.GenerateTagsForObject(route) + // When no ports are explicitly configured, use the same as target ports of backend services specified in the TCPRoutes. + if len(gwPorts) == 0 { + gwPorts = lo.FilterMap(backendRefs, func(br gatewayapi.BackendRef, _ int) (gatewayapi.PortNumber, bool) { + if br.Port == nil { + return 0, false + } + return *br.Port, true + }) + } return []kongstate.Route{ { Ingress: util.FromK8sObject(route), - Route: routeToKongRoute(route, backendRefs, ruleNumber, tags), + Route: routeToKongRoute(route, gwPorts, ruleNumber, util.GenerateTagsForObject(route)), }, }, nil } @@ -82,16 +91,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 +119,29 @@ 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)), 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..5f7eba1d79 100644 --- a/internal/dataplane/parser/translate_routes_helpers_test.go +++ b/internal/dataplane/parser/translate_routes_helpers_test.go @@ -15,9 +15,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 }{ @@ -62,12 +63,59 @@ func TestGenerateKongRoutesFromRouteRule_TCP(t *testing.T) { }, }, }, + { + 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}, + 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), + }, + }, + Protocols: []*string{ + lo.ToPtr("tcp"), + }, + Tags: []*string{ + kong.String("k8s-name:mytcproute-name"), + kong.String("k8s-namespace:mynamespace"), + }, + }, + }, + }, + }, } - 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) { @@ -79,9 +127,10 @@ func TestGenerateKongRoutesFromRouteRule_TCP(t *testing.T) { } func TestGenerateKongRoutesFromRouteRule_UDP(t *testing.T) { - testcases := []struct { + testCases := []struct { name string route *gatewayapi.UDPRoute + gwPorts []gatewayapi.PortNumber routeRule gatewayapi.UDPRouteRule expected []kongstate.Route }{ @@ -126,12 +175,59 @@ func TestGenerateKongRoutesFromRouteRule_UDP(t *testing.T) { }, }, }, + { + name: "UDPRoute with multiple backends and different Gateway port get 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}, + 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), + }, + }, + Protocols: []*string{ + lo.ToPtr("udp"), + }, + Tags: []*string{ + kong.String("k8s-name:myudproute-name"), + kong.String("k8s-namespace:mynamespace"), + }, + }, + }, + }, + }, } - 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) { @@ -222,7 +318,8 @@ 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) { diff --git a/internal/dataplane/parser/translate_tcproute.go b/internal/dataplane/parser/translate_tcproute.go index 5af1be158a..1fdab5ee41 100644 --- a/internal/dataplane/parser/translate_tcproute.go +++ b/internal/dataplane/parser/translate_tcproute.go @@ -3,6 +3,8 @@ package parser import ( "fmt" + "github.com/samber/lo" + "github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/parser/translators" "github.com/kong/kubernetes-ingress-controller/v2/internal/gatewayapi" ) @@ -47,11 +49,35 @@ func (p *Parser) ingressRulesFromTCPRoutes() ingressRules { return result } +func (p *Parser) gwr(routeNamespace string, prs []gatewayapi.ParentReference) []gatewayapi.PortNumber { + var gwPorts []gatewayapi.PortNumber + for _, pr := range prs { + // When namespace is not explicitly specified in the parentRef, + // the namespace of TCPRoute should be used. + 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 TCPRoute configuration. + 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 { + return l.Port, true + } + return 0, false + })...) + } + return gwPorts +} + 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 +85,20 @@ 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.gwr(tcproute.Namespace, 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 + // simply be calling validation on each tcproute object at the beginning // 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) + routes, err := generateKongRoutesFromRouteRule(tcproute, gwPorts, ruleNumber, rule) if err != nil { return err } diff --git a/internal/dataplane/parser/translate_tlsroute.go b/internal/dataplane/parser/translate_tlsroute.go index 5202e59110..4075843ff3 100644 --- a/internal/dataplane/parser/translate_tlsroute.go +++ b/internal/dataplane/parser/translate_tlsroute.go @@ -53,7 +53,6 @@ func (p *Parser) ingressRulesFromTLSRoutes() ingressRules { } 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 +67,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..288298c598 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.gwr(udproute.Namespace, 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 }