From c441d04788c8262ec53ab38dc2a1519a71032761 Mon Sep 17 00:00:00 2001 From: Romain Date: Wed, 9 Oct 2024 15:50:04 +0200 Subject: [PATCH] Avoid updating Accepted status for routes matching no Gateways Co-authored-by: Kevin Pollet --- pkg/provider/kubernetes/gateway/grpcroute.go | 13 +- pkg/provider/kubernetes/gateway/httproute.go | 13 +- pkg/provider/kubernetes/gateway/kubernetes.go | 42 +++-- .../kubernetes/gateway/kubernetes_test.go | 166 +++++++++++------- pkg/provider/kubernetes/gateway/tcproute.go | 13 +- pkg/provider/kubernetes/gateway/tlsroute.go | 13 +- 6 files changed, 164 insertions(+), 96 deletions(-) diff --git a/pkg/provider/kubernetes/gateway/grpcroute.go b/pkg/provider/kubernetes/gateway/grpcroute.go index 21fc5e5d6..8f8fac13c 100644 --- a/pkg/provider/kubernetes/gateway/grpcroute.go +++ b/pkg/provider/kubernetes/gateway/grpcroute.go @@ -32,6 +32,11 @@ func (p *Provider) loadGRPCRoutes(ctx context.Context, gatewayListeners []gatewa Str("namespace", route.Namespace). Logger() + routeListeners := matchingGatewayListeners(gatewayListeners, route.Namespace, route.Spec.ParentRefs) + if len(routeListeners) == 0 { + continue + } + var parentStatuses []gatev1.RouteParentStatus for _, parentRef := range route.Spec.ParentRefs { parentStatus := &gatev1.RouteParentStatus{ @@ -48,11 +53,9 @@ func (p *Provider) loadGRPCRoutes(ctx context.Context, gatewayListeners []gatewa }, } - for _, listener := range gatewayListeners { - accepted := true - if !matchListener(listener, route.Namespace, parentRef) { - accepted = false - } + for _, listener := range routeListeners { + accepted := matchListener(listener, parentRef) + if accepted && !allowRoute(listener, route.Namespace, kindGRPCRoute) { parentStatus.Conditions = updateRouteConditionAccepted(parentStatus.Conditions, string(gatev1.RouteReasonNotAllowedByListeners)) accepted = false diff --git a/pkg/provider/kubernetes/gateway/httproute.go b/pkg/provider/kubernetes/gateway/httproute.go index 5fa5ef9ff..cda154240 100644 --- a/pkg/provider/kubernetes/gateway/httproute.go +++ b/pkg/provider/kubernetes/gateway/httproute.go @@ -36,6 +36,11 @@ func (p *Provider) loadHTTPRoutes(ctx context.Context, gatewayListeners []gatewa Str("namespace", route.Namespace). Logger() + routeListeners := matchingGatewayListeners(gatewayListeners, route.Namespace, route.Spec.ParentRefs) + if len(routeListeners) == 0 { + continue + } + var parentStatuses []gatev1.RouteParentStatus for _, parentRef := range route.Spec.ParentRefs { parentStatus := &gatev1.RouteParentStatus{ @@ -52,11 +57,9 @@ func (p *Provider) loadHTTPRoutes(ctx context.Context, gatewayListeners []gatewa }, } - for _, listener := range gatewayListeners { - accepted := true - if !matchListener(listener, route.Namespace, parentRef) { - accepted = false - } + for _, listener := range routeListeners { + accepted := matchListener(listener, parentRef) + if accepted && !allowRoute(listener, route.Namespace, kindHTTPRoute) { parentStatus.Conditions = updateRouteConditionAccepted(parentStatus.Conditions, string(gatev1.RouteReasonNotAllowedByListeners)) accepted = false diff --git a/pkg/provider/kubernetes/gateway/kubernetes.go b/pkg/provider/kubernetes/gateway/kubernetes.go index faea170e3..1a4356757 100644 --- a/pkg/provider/kubernetes/gateway/kubernetes.go +++ b/pkg/provider/kubernetes/gateway/kubernetes.go @@ -1119,24 +1119,36 @@ func allowRoute(listener gatewayListener, routeNamespace, routeKind string) bool }) } -func matchListener(listener gatewayListener, routeNamespace string, parentRef gatev1.ParentReference) bool { - if ptr.Deref(parentRef.Group, gatev1.GroupName) != gatev1.GroupName { - return false +func matchingGatewayListeners(gatewayListeners []gatewayListener, routeNamespace string, parentRefs []gatev1.ParentReference) []gatewayListener { + var listeners []gatewayListener + + for _, listener := range gatewayListeners { + for _, parentRef := range parentRefs { + if ptr.Deref(parentRef.Group, gatev1.GroupName) != gatev1.GroupName { + continue + } + + if ptr.Deref(parentRef.Kind, kindGateway) != kindGateway { + continue + } + + parentRefNamespace := string(ptr.Deref(parentRef.Namespace, gatev1.Namespace(routeNamespace))) + if listener.GWNamespace != parentRefNamespace { + continue + } + + if string(parentRef.Name) != listener.GWName { + continue + } + + listeners = append(listeners, listener) + } } - if ptr.Deref(parentRef.Kind, kindGateway) != kindGateway { - return false - } - - parentRefNamespace := string(ptr.Deref(parentRef.Namespace, gatev1.Namespace(routeNamespace))) - if listener.GWNamespace != parentRefNamespace { - return false - } - - if string(parentRef.Name) != listener.GWName { - return false - } + return listeners +} +func matchListener(listener gatewayListener, parentRef gatev1.ParentReference) bool { sectionName := string(ptr.Deref(parentRef.SectionName, "")) if sectionName != "" && sectionName != listener.Name { return false diff --git a/pkg/provider/kubernetes/gateway/kubernetes_test.go b/pkg/provider/kubernetes/gateway/kubernetes_test.go index f885515c3..985096362 100644 --- a/pkg/provider/kubernetes/gateway/kubernetes_test.go +++ b/pkg/provider/kubernetes/gateway/kubernetes_test.go @@ -6779,127 +6779,171 @@ func TestLoadRoutesWithReferenceGrants(t *testing.T) { } } -func Test_matchListener(t *testing.T) { +func Test_matchingGatewayListener(t *testing.T) { testCases := []struct { desc string - gwListener gatewayListener - parentRef gatev1.ParentReference + gwListeners []gatewayListener + parentRefs []gatev1.ParentReference routeNamespace string - wantMatch bool + wantLen int }{ { desc: "Unsupported group", - gwListener: gatewayListener{ + gwListeners: []gatewayListener{{ Name: "foo", GWName: "gateway", GWNamespace: "default", - }, - parentRef: gatev1.ParentReference{ + }}, + parentRefs: []gatev1.ParentReference{{ Group: ptr.To(gatev1.Group("foo")), - }, - wantMatch: false, + }}, + wantLen: 0, }, { desc: "Unsupported kind", - gwListener: gatewayListener{ + gwListeners: []gatewayListener{{ Name: "foo", GWName: "gateway", GWNamespace: "default", - }, - parentRef: gatev1.ParentReference{ + }}, + parentRefs: []gatev1.ParentReference{{ Group: ptr.To(gatev1.Group(gatev1.GroupName)), Kind: ptr.To(gatev1.Kind("foo")), - }, - wantMatch: false, + }}, + wantLen: 0, }, { desc: "Namespace does not match the listener", - gwListener: gatewayListener{ + gwListeners: []gatewayListener{{ Name: "foo", GWName: "gateway", GWNamespace: "default", - }, - parentRef: gatev1.ParentReference{ + }}, + parentRefs: []gatev1.ParentReference{{ Namespace: ptr.To(gatev1.Namespace("foo")), Group: ptr.To(gatev1.Group(gatev1.GroupName)), Kind: ptr.To(gatev1.Kind("Gateway")), - }, - wantMatch: false, + }}, + wantLen: 0, }, { desc: "Route namespace defaulting does not match the listener", - gwListener: gatewayListener{ + gwListeners: []gatewayListener{{ Name: "foo", GWName: "gateway", GWNamespace: "default", - }, + }}, routeNamespace: "foo", - parentRef: gatev1.ParentReference{ + parentRefs: []gatev1.ParentReference{{ Group: ptr.To(gatev1.Group(gatev1.GroupName)), Kind: ptr.To(gatev1.Kind("Gateway")), - }, - wantMatch: false, + }}, + wantLen: 0, }, { desc: "Name does not match the listener", - gwListener: gatewayListener{ - Name: "foo", + gwListeners: []gatewayListener{{ GWName: "gateway", GWNamespace: "default", - }, - parentRef: gatev1.ParentReference{ + }}, + parentRefs: []gatev1.ParentReference{{ Namespace: ptr.To(gatev1.Namespace("default")), Name: "foo", Group: ptr.To(gatev1.Group(gatev1.GroupName)), Kind: ptr.To(gatev1.Kind("Gateway")), - }, - wantMatch: false, - }, - { - desc: "SectionName does not match a listener", - gwListener: gatewayListener{ - Name: "foo", - GWName: "gateway", - GWNamespace: "default", - }, - parentRef: gatev1.ParentReference{ - SectionName: ptr.To(gatev1.SectionName("bar")), - Name: "gateway", - Namespace: ptr.To(gatev1.Namespace("default")), - Group: ptr.To(gatev1.Group(gatev1.GroupName)), - Kind: ptr.To(gatev1.Kind("Gateway")), - }, - wantMatch: false, + }}, + wantLen: 0, }, { desc: "Match", - gwListener: gatewayListener{ - Name: "foo", + gwListeners: []gatewayListener{{ GWName: "gateway", GWNamespace: "default", + }}, + parentRefs: []gatev1.ParentReference{{ + Name: "gateway", + Namespace: ptr.To(gatev1.Namespace("default")), + Group: ptr.To(gatev1.Group(gatev1.GroupName)), + Kind: ptr.To(gatev1.Kind("Gateway")), + }}, + wantLen: 1, + }, + { + desc: "Match with route namespace defaulting", + gwListeners: []gatewayListener{{ + GWName: "gateway", + GWNamespace: "default", + }}, + routeNamespace: "default", + parentRefs: []gatev1.ParentReference{{ + Name: "gateway", + Group: ptr.To(gatev1.Group(gatev1.GroupName)), + Kind: ptr.To(gatev1.Kind("Gateway")), + }}, + wantLen: 1, + }, + } + + for _, test := range testCases { + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + listeners := matchingGatewayListeners(test.gwListeners, test.routeNamespace, test.parentRefs) + assert.Len(t, listeners, test.wantLen) + }) + } +} + +func Test_matchListener(t *testing.T) { + testCases := []struct { + desc string + gwListener gatewayListener + parentRef gatev1.ParentReference + wantMatch bool + }{ + { + desc: "Section do not match", + gwListener: gatewayListener{ + Name: "foo", + Port: gatev1.PortNumber(80), + }, + parentRef: gatev1.ParentReference{ + SectionName: ptr.To(gatev1.SectionName("bar")), + Port: ptr.To(gatev1.PortNumber(80)), + }, + }, + { + desc: "Section matches", + gwListener: gatewayListener{ + Name: "foo", + Port: gatev1.PortNumber(80), }, parentRef: gatev1.ParentReference{ SectionName: ptr.To(gatev1.SectionName("foo")), - Name: "gateway", - Namespace: ptr.To(gatev1.Namespace("default")), - Group: ptr.To(gatev1.Group(gatev1.GroupName)), - Kind: ptr.To(gatev1.Kind("Gateway")), + Port: ptr.To(gatev1.PortNumber(80)), }, wantMatch: true, }, { - desc: "Match with route namespace defaulting", + desc: "Port do not match", gwListener: gatewayListener{ - Name: "foo", - GWName: "gateway", - GWNamespace: "default", + Name: "foo", + Port: gatev1.PortNumber(90), }, - routeNamespace: "default", parentRef: gatev1.ParentReference{ SectionName: ptr.To(gatev1.SectionName("foo")), - Name: "gateway", - Group: ptr.To(gatev1.Group(gatev1.GroupName)), - Kind: ptr.To(gatev1.Kind("Gateway")), + Port: ptr.To(gatev1.PortNumber(80)), + }, + }, + { + desc: "Port matches", + gwListener: gatewayListener{ + Name: "foo", + Port: gatev1.PortNumber(80), + }, + parentRef: gatev1.ParentReference{ + SectionName: ptr.To(gatev1.SectionName("foo")), + Port: ptr.To(gatev1.PortNumber(80)), }, wantMatch: true, }, @@ -6909,7 +6953,7 @@ func Test_matchListener(t *testing.T) { t.Run(test.desc, func(t *testing.T) { t.Parallel() - gotMatch := matchListener(test.gwListener, test.routeNamespace, test.parentRef) + gotMatch := matchListener(test.gwListener, test.parentRef) assert.Equal(t, test.wantMatch, gotMatch) }) } diff --git a/pkg/provider/kubernetes/gateway/tcproute.go b/pkg/provider/kubernetes/gateway/tcproute.go index 90e98b8ae..0f859b5ae 100644 --- a/pkg/provider/kubernetes/gateway/tcproute.go +++ b/pkg/provider/kubernetes/gateway/tcproute.go @@ -32,6 +32,11 @@ func (p *Provider) loadTCPRoutes(ctx context.Context, gatewayListeners []gateway Str("namespace", route.Namespace). Logger() + routeListeners := matchingGatewayListeners(gatewayListeners, route.Namespace, route.Spec.ParentRefs) + if len(routeListeners) == 0 { + continue + } + var parentStatuses []gatev1alpha2.RouteParentStatus for _, parentRef := range route.Spec.ParentRefs { parentStatus := &gatev1alpha2.RouteParentStatus{ @@ -48,11 +53,9 @@ func (p *Provider) loadTCPRoutes(ctx context.Context, gatewayListeners []gateway }, } - for _, listener := range gatewayListeners { - accepted := true - if !matchListener(listener, route.Namespace, parentRef) { - accepted = false - } + for _, listener := range routeListeners { + accepted := matchListener(listener, parentRef) + if accepted && !allowRoute(listener, route.Namespace, kindTCPRoute) { parentStatus.Conditions = updateRouteConditionAccepted(parentStatus.Conditions, string(gatev1.RouteReasonNotAllowedByListeners)) accepted = false diff --git a/pkg/provider/kubernetes/gateway/tlsroute.go b/pkg/provider/kubernetes/gateway/tlsroute.go index ea3a54270..42b715de1 100644 --- a/pkg/provider/kubernetes/gateway/tlsroute.go +++ b/pkg/provider/kubernetes/gateway/tlsroute.go @@ -32,6 +32,11 @@ func (p *Provider) loadTLSRoutes(ctx context.Context, gatewayListeners []gateway Str("tls_route", route.Name). Str("namespace", route.Namespace).Logger() + routeListeners := matchingGatewayListeners(gatewayListeners, route.Namespace, route.Spec.ParentRefs) + if len(routeListeners) == 0 { + continue + } + var parentStatuses []gatev1alpha2.RouteParentStatus for _, parentRef := range route.Spec.ParentRefs { parentStatus := &gatev1alpha2.RouteParentStatus{ @@ -48,11 +53,9 @@ func (p *Provider) loadTLSRoutes(ctx context.Context, gatewayListeners []gateway }, } - for _, listener := range gatewayListeners { - accepted := true - if !matchListener(listener, route.Namespace, parentRef) { - accepted = false - } + for _, listener := range routeListeners { + accepted := matchListener(listener, parentRef) + if accepted && !allowRoute(listener, route.Namespace, kindTLSRoute) { parentStatus.Conditions = updateRouteConditionAccepted(parentStatus.Conditions, string(gatev1.RouteReasonNotAllowedByListeners)) accepted = false