Skip to content

Commit

Permalink
fix: set proper destination port for TCPRoute and UDPRoute
Browse files Browse the repository at this point in the history
  • Loading branch information
programmer04 committed Oct 20, 2023
1 parent 1100749 commit bc75c0c
Show file tree
Hide file tree
Showing 6 changed files with 184 additions and 48 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
54 changes: 27 additions & 27 deletions internal/dataplane/parser/translate_routes_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -62,36 +63,50 @@ 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) {
backendRefs, err := getBackendRefs(rule)
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
}

// 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:
Expand All @@ -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 {
Expand Down
111 changes: 104 additions & 7 deletions internal/dataplane/parser/translate_routes_helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}{
Expand Down Expand Up @@ -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) {
Expand All @@ -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
}{
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
38 changes: 33 additions & 5 deletions internal/dataplane/parser/translate_tcproute.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -47,30 +49,56 @@ 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.
if len(spec.Rules) == 0 {
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
}
Expand Down
10 changes: 5 additions & 5 deletions internal/dataplane/parser/translate_tlsroute.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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")
Expand Down

0 comments on commit bc75c0c

Please sign in to comment.