From 3279f52eb26282eb91e3e8cef9d64f73ba661ea9 Mon Sep 17 00:00:00 2001 From: "gang.liu" Date: Tue, 14 Nov 2023 17:53:50 +0800 Subject: [PATCH 1/9] fix attached routes Signed-off-by: gang.liu --- internal/dag/gatewayapi_processor.go | 145 ++++++++++++++---- internal/dag/gatewayapi_processor_test.go | 6 +- internal/dag/status.go | 1 + internal/dag/status_test.go | 78 +++++----- .../gatewayapi/gateway_conformance_test.go | 1 - 5 files changed, 166 insertions(+), 65 deletions(-) diff --git a/internal/dag/gatewayapi_processor.go b/internal/dag/gatewayapi_processor.go index b90a05ac92b..42303ae0b07 100644 --- a/internal/dag/gatewayapi_processor.go +++ b/internal/dag/gatewayapi_processor.go @@ -80,6 +80,35 @@ type matchConditions struct { queryParams []QueryParamMatchCondition } +// computeAttachedRoutes compute the attached routes for the listener(s) +func (p *GatewayAPIProcessor) computeAttachedRoutes() map[string]int { + var listeners []*listenerInfo + for _, listener := range p.source.gateway.Spec.Listeners { + listeners = append(listeners, p.computeListenerForAttachedRoutes(listener)) + } + + // Keep track of the number of routes attached + // to each Listener so we can set status properly. + attachedRoutes := map[string]int{} + + for _, httpRoute := range p.source.httproutes { + p.computeAttachedRoutesForKind(KindHTTPRoute, httpRoute.Namespace, httpRoute.Spec.ParentRefs, listeners, attachedRoutes) + } + for _, tlsRoute := range p.source.tlsroutes { + p.computeAttachedRoutesForKind(KindTLSRoute, tlsRoute.Namespace, tlsRoute.Spec.ParentRefs, listeners, attachedRoutes) + } + + for _, grpcRoute := range p.source.grpcroutes { + p.computeAttachedRoutesForKind(KindGRPCRoute, grpcRoute.Namespace, grpcRoute.Spec.ParentRefs, listeners, attachedRoutes) + } + + for _, tcpRoute := range p.source.tcproutes { + p.computeAttachedRoutesForKind(KindTCPRoute, tcpRoute.Namespace, tcpRoute.Spec.ParentRefs, listeners, attachedRoutes) + } + + return attachedRoutes +} + // Run translates Gateway API types into DAG objects and // adds them to the DAG. func (p *GatewayAPIProcessor) Run(dag *DAG, source *KubernetesCache) { @@ -136,38 +165,34 @@ func (p *GatewayAPIProcessor) Run(dag *DAG, source *KubernetesCache) { // Compute listeners and save a list of the valid/ready ones. var readyListeners []*listenerInfo - for _, listener := range p.source.gateway.Spec.Listeners { if ready, listenerInfo := p.computeListener(listener, gwAccessor, validateListenersResult); ready { readyListeners = append(readyListeners, listenerInfo) } - } - // Keep track of the number of routes attached - // to each Listener so we can set status properly. - listenerAttachedRoutes := map[string]int{} + } // Process HTTPRoutes. for _, httpRoute := range p.source.httproutes { - p.processRoute(KindHTTPRoute, httpRoute, httpRoute.Spec.ParentRefs, gatewayNotProgrammedCondition, readyListeners, listenerAttachedRoutes, &gatewayapi_v1beta1.HTTPRoute{}) + p.processRoute(KindHTTPRoute, httpRoute, httpRoute.Spec.ParentRefs, gatewayNotProgrammedCondition, readyListeners, &gatewayapi_v1beta1.HTTPRoute{}) } // Process TLSRoutes. for _, tlsRoute := range p.source.tlsroutes { - p.processRoute(KindTLSRoute, tlsRoute, tlsRoute.Spec.ParentRefs, gatewayNotProgrammedCondition, readyListeners, listenerAttachedRoutes, &gatewayapi_v1alpha2.TLSRoute{}) + p.processRoute(KindTLSRoute, tlsRoute, tlsRoute.Spec.ParentRefs, gatewayNotProgrammedCondition, readyListeners, &gatewayapi_v1alpha2.TLSRoute{}) } // Process GRPCRoutes. for _, grpcRoute := range p.source.grpcroutes { - p.processRoute(KindGRPCRoute, grpcRoute, grpcRoute.Spec.ParentRefs, gatewayNotProgrammedCondition, readyListeners, listenerAttachedRoutes, &gatewayapi_v1alpha2.GRPCRoute{}) + p.processRoute(KindGRPCRoute, grpcRoute, grpcRoute.Spec.ParentRefs, gatewayNotProgrammedCondition, readyListeners, &gatewayapi_v1alpha2.GRPCRoute{}) } // Process TCPRoutes. for _, tcpRoute := range p.source.tcproutes { - p.processRoute(KindTCPRoute, tcpRoute, tcpRoute.Spec.ParentRefs, gatewayNotProgrammedCondition, readyListeners, listenerAttachedRoutes, &gatewayapi_v1alpha2.TCPRoute{}) + p.processRoute(KindTCPRoute, tcpRoute, tcpRoute.Spec.ParentRefs, gatewayNotProgrammedCondition, readyListeners, &gatewayapi_v1alpha2.TCPRoute{}) } - for listenerName, attachedRoutes := range listenerAttachedRoutes { + for listenerName, attachedRoutes := range p.computeAttachedRoutes() { gwAccessor.SetListenerAttachedRoutes(listenerName, attachedRoutes) } @@ -180,7 +205,6 @@ func (p *GatewayAPIProcessor) processRoute( parentRefs []gatewayapi_v1beta1.ParentReference, gatewayNotProgrammedCondition *metav1.Condition, readyListeners []*listenerInfo, - listenerAttachedRoutes map[string]int, emptyResource client.Object, ) { routeStatus, commit := p.dag.StatusCache.RouteConditionsAccessor( @@ -207,6 +231,7 @@ func (p *GatewayAPIProcessor) processRoute( // Get the list of listeners that are (a) included by this parent ref, and // (b) allow the route (based on kind, namespace). allowedListeners := p.getListenersForRouteParentRef(routeParentRef, route.GetNamespace(), routeKind, readyListeners, routeParentStatus) + if len(allowedListeners) == 0 { p.resolveRouteRefs(route, routeParentStatus) } @@ -250,21 +275,15 @@ func (p *GatewayAPIProcessor) processRoute( } } - var attached bool - switch route := route.(type) { case *gatewayapi_v1beta1.HTTPRoute: - attached = p.computeHTTPRouteForListener(route, routeParentStatus, listener, hosts) + p.computeHTTPRouteForListener(route, routeParentStatus, listener, hosts) case *gatewayapi_v1alpha2.TLSRoute: - attached = p.computeTLSRouteForListener(route, routeParentStatus, listener, hosts) + p.computeTLSRouteForListener(route, routeParentStatus, listener, hosts) case *gatewayapi_v1alpha2.GRPCRoute: - attached = p.computeGRPCRouteForListener(route, routeParentStatus, listener, hosts) + p.computeGRPCRouteForListener(route, routeParentStatus, listener, hosts) case *gatewayapi_v1alpha2.TCPRoute: - attached = p.computeTCPRouteForListener(route, routeParentStatus, listener) - } - - if attached { - listenerAttachedRoutes[string(listener.listener.Name)]++ + p.computeTCPRouteForListener(route, routeParentStatus, listener) } hostCount += hosts.Len() @@ -365,6 +384,54 @@ func (p *GatewayAPIProcessor) getListenersForRouteParentRef( return allowedListeners } +// computeAttachedRoutesForKind compute attached routes for the specific kind +func (p *GatewayAPIProcessor) computeAttachedRoutesForKind( + routeKind gatewayapi_v1beta1.Kind, + namespace string, + parentRefs []gatewayapi_v1beta1.ParentReference, + listeners []*listenerInfo, + attachedRoutes map[string]int, +) { + + for _, parentRef := range parentRefs { + + // If this parent ref is to a different Gateway, ignore it. + if !gatewayapi.IsRefToGateway(parentRef, k8s.NamespacedNameOf(p.source.gateway)) { + continue + } + + // Find the set of listeners that are relevant given this + // parent ref (either all of them, if the ref is to the entire + // gateway, or one of them, if the ref is to a specific listener, + // or none of them, if the listener(s) the ref targets are invalid). + var selectedListeners []*listenerInfo + for _, listener := range listeners { + // We've already verified the parent ref is for this Gateway, + // now check if it has a listener name and port specified. + // Both need to match the listener if specified. + if (parentRef.SectionName == nil || *parentRef.SectionName == listener.listener.Name) && + (parentRef.Port == nil || *parentRef.Port == listener.listener.Port) { + selectedListeners = append(selectedListeners, listener) + } + } + + // Now find the subset of those listeners that allow this route + // to select them, based on route kind and namespace. + for _, selectedListener := range selectedListeners { + // Check if the listener allows routes of this kind + if !selectedListener.AllowsKind(routeKind) { + continue + } + + // Check if the route is in a namespace that the listener allows. + if !p.namespaceMatches(selectedListener.listener.AllowedRoutes.Namespaces, selectedListener.namespaceSelector, namespace) { + continue + } + attachedRoutes[string(selectedListener.listener.Name)]++ + } + } +} + type listenerInfo struct { listener gatewayapi_v1beta1.Listener dagListenerName string @@ -601,6 +668,30 @@ func (p *GatewayAPIProcessor) computeListener( } } +// computeListenerForAttachedRoutes processes a Listener's spec. +// It returns a listenerInfo struct with the allowed route kinds and namespace selector +func (p *GatewayAPIProcessor) computeListenerForAttachedRoutes(listener gatewayapi_v1beta1.Listener) *listenerInfo { + var selector labels.Selector + + listenerRouteKinds := p.getListenerRouteKinds(listener, &status.GatewayStatusUpdate{}) + + if listener.AllowedRoutes != nil && listener.AllowedRoutes.Namespaces != nil && + listener.AllowedRoutes.Namespaces.From != nil && *listener.AllowedRoutes.Namespaces.From == gatewayapi_v1.NamespacesFromSelector { + + var err error + selector, err = metav1.LabelSelectorAsSelector(listener.AllowedRoutes.Namespaces.Selector) + if err != nil { + return nil + } + } + + return &listenerInfo{ + listener: listener, + allowedKinds: listenerRouteKinds, + namespaceSelector: selector, + } +} + // getListenerRouteKinds gets a list of the valid route kinds that // the listener accepts. func (p *GatewayAPIProcessor) getListenerRouteKinds(listener gatewayapi_v1beta1.Listener, gwAccessor *status.GatewayStatusUpdate) []gatewayapi_v1beta1.Kind { @@ -1102,8 +1193,12 @@ func (p *GatewayAPIProcessor) resolveRouteRefs(route any, routeAccessor *status. } } -func (p *GatewayAPIProcessor) computeHTTPRouteForListener(route *gatewayapi_v1beta1.HTTPRoute, routeAccessor *status.RouteParentStatusUpdate, listener *listenerInfo, hosts sets.Set[string]) bool { - var programmed bool +func (p *GatewayAPIProcessor) computeHTTPRouteForListener( + route *gatewayapi_v1beta1.HTTPRoute, + routeAccessor *status.RouteParentStatusUpdate, + listener *listenerInfo, + hosts sets.Set[string]) { + for ruleIndex, rule := range route.Spec.Rules { // Get match conditions for the rule. var matchconditions []*matchConditions @@ -1384,13 +1479,9 @@ func (p *GatewayAPIProcessor) computeHTTPRouteForListener(route *gatewayapi_v1be vhost := p.dag.EnsureVirtualHost(listener.dagListenerName, host) vhost.AddRoute(route) } - - programmed = true } } } - - return programmed } func (p *GatewayAPIProcessor) computeGRPCRouteForListener(route *gatewayapi_v1alpha2.GRPCRoute, routeAccessor *status.RouteParentStatusUpdate, listener *listenerInfo, hosts sets.Set[string]) bool { diff --git a/internal/dag/gatewayapi_processor_test.go b/internal/dag/gatewayapi_processor_test.go index bfec5ba9950..3f464d3dc20 100644 --- a/internal/dag/gatewayapi_processor_test.go +++ b/internal/dag/gatewayapi_processor_test.go @@ -742,7 +742,11 @@ func TestGetListenersForRouteParentRef(t *testing.T) { rsu := &status.RouteStatusUpdate{} rpsu := rsu.StatusUpdateFor(tc.routeParentRef) - got := processor.getListenersForRouteParentRef(tc.routeParentRef, tc.routeNamespace, gatewayapi_v1beta1.Kind(tc.routeKind), tc.listeners, rpsu) + got := processor.getListenersForRouteParentRef( + tc.routeParentRef, + tc.routeNamespace, + gatewayapi_v1beta1.Kind(tc.routeKind), + tc.listeners, rpsu) var want []*listenerInfo for _, i := range tc.want { diff --git a/internal/dag/status.go b/internal/dag/status.go index 7649ded33c8..69624ec1bef 100644 --- a/internal/dag/status.go +++ b/internal/dag/status.go @@ -18,6 +18,7 @@ import ( contour_api_v1 "github.com/projectcontour/contour/apis/projectcontour/v1" "github.com/projectcontour/contour/internal/status" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ) diff --git a/internal/dag/status_test.go b/internal/dag/status_test.go index daba96d2405..1de01137c54 100644 --- a/internal/dag/status_test.go +++ b/internal/dag/status_test.go @@ -17,14 +17,15 @@ import ( "fmt" "testing" - "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" contour_api_v1 "github.com/projectcontour/contour/apis/projectcontour/v1" "github.com/projectcontour/contour/internal/fixture" "github.com/projectcontour/contour/internal/gatewayapi" "github.com/projectcontour/contour/internal/k8s" "github.com/projectcontour/contour/internal/ref" "github.com/projectcontour/contour/internal/status" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/stretchr/testify/assert" v1 "k8s.io/api/core/v1" networking_v1 "k8s.io/api/networking/v1" @@ -5217,6 +5218,7 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) { for _, o := range tc.objs { builder.Source.Insert(o) } + dag := builder.Build() gotRouteUpdates := dag.StatusCache.GetRouteUpdates() gotGatewayUpdates := dag.StatusCache.GetGatewayUpdates() @@ -5483,7 +5485,7 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 1), }) run(t, "exact path match not starting with '/' for httproute", testcase{ objs: []any{ @@ -5529,7 +5531,7 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 1), }) run(t, "regular expression path match with invalid value for httproute", testcase{ @@ -5571,7 +5573,7 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 1), }) run(t, "prefix path match with consecutive '/' characters for httproute", testcase{ @@ -5618,7 +5620,7 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 1), }) run(t, "exact path match with consecutive '/' characters for httproute", testcase{ @@ -5665,7 +5667,7 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 1), }) run(t, "invalid path match type for httproute", testcase{ @@ -5711,7 +5713,7 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 1), }) run(t, "invalid header match type not supported for httproute", testcase{ @@ -5764,7 +5766,7 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 1), }) run(t, "regular expression header match with invalid value for httproute", testcase{ @@ -5817,7 +5819,7 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 1), }) run(t, "regular expression query param match with valid value for httproute", testcase{ @@ -5923,7 +5925,7 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 1), }) run(t, "query param match with invalid type for httproute", testcase{ @@ -5976,7 +5978,7 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 1), }) run(t, "spec.rules.backendRef.name not specified", testcase{ @@ -6152,7 +6154,7 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 1), }) run(t, "spec.rules.backendRef.namespace does not match route", testcase{ @@ -6919,7 +6921,7 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 1), }) run(t, "spec.rules.hostname: invalid hostname", testcase{ @@ -6961,7 +6963,7 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 1), }) run(t, "spec.rules.hostname: invalid hostname, ip address", testcase{ @@ -7001,7 +7003,7 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 1), }) run(t, "two HTTP listeners, route's hostname intersects with one of them", testcase{ @@ -7097,7 +7099,7 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) { }, "listener-2": { Name: gatewayapi_v1beta1.SectionName("listener-2"), - AttachedRoutes: int32(0), + AttachedRoutes: int32(1), SupportedKinds: []gatewayapi_v1beta1.RouteGroupKind{ { Group: ref.To(gatewayapi_v1beta1.Group(gatewayapi_v1beta1.GroupName)), @@ -7198,7 +7200,7 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) { ListenerStatus: map[string]*gatewayapi_v1beta1.ListenerStatus{ "listener-1": { Name: gatewayapi_v1beta1.SectionName("listener-1"), - AttachedRoutes: int32(0), + AttachedRoutes: int32(1), SupportedKinds: []gatewayapi_v1beta1.RouteGroupKind{ { Group: ref.To(gatewayapi_v1beta1.Group(gatewayapi_v1beta1.GroupName)), @@ -7213,7 +7215,7 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) { }, "listener-2": { Name: gatewayapi_v1beta1.SectionName("listener-2"), - AttachedRoutes: int32(0), + AttachedRoutes: int32(1), SupportedKinds: []gatewayapi_v1beta1.RouteGroupKind{ { Group: ref.To(gatewayapi_v1beta1.Group(gatewayapi_v1beta1.GroupName)), @@ -9336,7 +9338,7 @@ func TestGatewayAPITLSRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate(string(gw.Spec.Listeners[0].Name), gw.Spec.Listeners[0].Protocol, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate(string(gw.Spec.Listeners[0].Name), gw.Spec.Listeners[0].Protocol, 1), }) run(t, "TLSRoute: spec.rules.backendRef.name invalid on two matches", testcase{ @@ -9379,7 +9381,7 @@ func TestGatewayAPITLSRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate(string(gw.Spec.Listeners[0].Name), gw.Spec.Listeners[0].Protocol, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate(string(gw.Spec.Listeners[0].Name), gw.Spec.Listeners[0].Protocol, 1), }) run(t, "TLSRoute: spec.rules.backendRef.port not specified", testcase{ @@ -9427,7 +9429,7 @@ func TestGatewayAPITLSRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate(string(gw.Spec.Listeners[0].Name), gw.Spec.Listeners[0].Protocol, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate(string(gw.Spec.Listeners[0].Name), gw.Spec.Listeners[0].Protocol, 1), }) run(t, "TLSRoute: spec.rules.backendRefs not specified", testcase{ @@ -9468,7 +9470,7 @@ func TestGatewayAPITLSRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate(string(gw.Spec.Listeners[0].Name), gw.Spec.Listeners[0].Protocol, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate(string(gw.Spec.Listeners[0].Name), gw.Spec.Listeners[0].Protocol, 1), }) run(t, "TLSRoute: spec.rules.hostname: invalid wildcard", testcase{ @@ -9511,7 +9513,7 @@ func TestGatewayAPITLSRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate(string(gw.Spec.Listeners[0].Name), gw.Spec.Listeners[0].Protocol, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate(string(gw.Spec.Listeners[0].Name), gw.Spec.Listeners[0].Protocol, 1), }) run(t, "TLSRoute: spec.rules.hostname: invalid hostname", testcase{ @@ -9554,7 +9556,7 @@ func TestGatewayAPITLSRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate(string(gw.Spec.Listeners[0].Name), gw.Spec.Listeners[0].Protocol, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate(string(gw.Spec.Listeners[0].Name), gw.Spec.Listeners[0].Protocol, 1), }) run(t, "TLSRoute: spec.rules.hostname: invalid hostname, ip address", testcase{ @@ -9595,7 +9597,7 @@ func TestGatewayAPITLSRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate(string(gw.Spec.Listeners[0].Name), gw.Spec.Listeners[0].Protocol, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate(string(gw.Spec.Listeners[0].Name), gw.Spec.Listeners[0].Protocol, 1), }) run(t, "TLSRoute: spec.rules.backendRefs has 0 weight", testcase{ @@ -9642,7 +9644,7 @@ func TestGatewayAPITLSRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate(string(gw.Spec.Listeners[0].Name), gw.Spec.Listeners[0].Protocol, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate(string(gw.Spec.Listeners[0].Name), gw.Spec.Listeners[0].Protocol, 1), }) run(t, "TLSRoute: backendrefs still validated when route not accepted", testcase{ @@ -9784,6 +9786,7 @@ func TestGatewayAPITLSRouteDAGStatus(t *testing.T) { listenerAcceptedCondition(), listenerResolvedRefsCondition(), }, + AttachedRoutes: 1, }, }, }}, @@ -9859,6 +9862,9 @@ func TestGatewayAPIGRPCRouteDAGStatus(t *testing.T) { } } + if desc == "anchorrrr" { + println("hello") + } for _, o := range tc.objs { builder.Source.Insert(o) } @@ -10013,7 +10019,7 @@ func TestGatewayAPIGRPCRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 1), }) run(t, "grpcroute: method match must have Service configured", testcase{ @@ -10056,7 +10062,7 @@ func TestGatewayAPIGRPCRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 1), }) run(t, "grpcroute: method match must have Method configured", testcase{ @@ -10099,7 +10105,7 @@ func TestGatewayAPIGRPCRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 1), }) run(t, "grpcroute: invalid header match type is not supported", testcase{ @@ -10154,7 +10160,7 @@ func TestGatewayAPIGRPCRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 1), }) run(t, "grpcroute: regular expression header match has invalid value", testcase{ @@ -10209,7 +10215,7 @@ func TestGatewayAPIGRPCRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 1), }) run(t, "grpcroute: invalid RequestHeaderModifier due to duplicated headers", testcase{ @@ -10497,7 +10503,7 @@ func TestGatewayAPIGRPCRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 1), }) run(t, "grpcroute: still validate backendrefs when not accepted", testcase{ @@ -10896,7 +10902,7 @@ func TestGatewayAPITCPRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate("tcp", gatewayapi_v1.TCPProtocolType, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate("tcp", gatewayapi_v1.TCPProtocolType, 1), }) run(t, "TCPRoute with rule with no backends", testcase{ objs: []any{ @@ -10927,7 +10933,7 @@ func TestGatewayAPITCPRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate("tcp", gatewayapi_v1.TCPProtocolType, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate("tcp", gatewayapi_v1.TCPProtocolType, 1), }) run(t, "TCPRoute with rule with ref to nonexistent backend", testcase{ objs: []any{ @@ -10960,7 +10966,7 @@ func TestGatewayAPITCPRouteDAGStatus(t *testing.T) { }, }, }}, - wantGatewayStatusUpdate: validGatewayStatusUpdate("tcp", gatewayapi_v1.TCPProtocolType, 0), + wantGatewayStatusUpdate: validGatewayStatusUpdate("tcp", gatewayapi_v1.TCPProtocolType, 1), }) } diff --git a/test/conformance/gatewayapi/gateway_conformance_test.go b/test/conformance/gatewayapi/gateway_conformance_test.go index 0a7c5bcbf6d..b8e4a56a7c0 100644 --- a/test/conformance/gatewayapi/gateway_conformance_test.go +++ b/test/conformance/gatewayapi/gateway_conformance_test.go @@ -78,7 +78,6 @@ func TestGatewayConformance(t *testing.T) { tests.HTTPRouteBackendProtocolH2C.ShortName, tests.HTTPRouteTimeoutBackendRequest.ShortName, tests.HTTPRouteTimeoutRequest.ShortName, - tests.GatewayWithAttachedRoutes.ShortName, tests.GatewayStaticAddresses.ShortName, }, ExemptFeatures: sets.New( From 13fbdc32834b51784438bffd52737354ceaf25f4 Mon Sep 17 00:00:00 2001 From: "gang.liu" Date: Tue, 14 Nov 2023 17:57:01 +0800 Subject: [PATCH 2/9] cleanup Signed-off-by: gang.liu --- internal/dag/status_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/internal/dag/status_test.go b/internal/dag/status_test.go index 1de01137c54..f03cbbb45a7 100644 --- a/internal/dag/status_test.go +++ b/internal/dag/status_test.go @@ -9862,9 +9862,6 @@ func TestGatewayAPIGRPCRouteDAGStatus(t *testing.T) { } } - if desc == "anchorrrr" { - println("hello") - } for _, o := range tc.objs { builder.Source.Insert(o) } From bf2121a7c6ef8e195dae0808a230ba230e7c867d Mon Sep 17 00:00:00 2001 From: "gang.liu" Date: Tue, 14 Nov 2023 18:17:27 +0800 Subject: [PATCH 3/9] add change log Signed-off-by: gang.liu --- changelogs/unreleased/5961-izturn-small.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelogs/unreleased/5961-izturn-small.md diff --git a/changelogs/unreleased/5961-izturn-small.md b/changelogs/unreleased/5961-izturn-small.md new file mode 100644 index 00000000000..a868c17c836 --- /dev/null +++ b/changelogs/unreleased/5961-izturn-small.md @@ -0,0 +1,3 @@ +## Gateway API Attached Routes + +For Gateway API v1.0, the successful attachment of a Route to a Listener is based solely on the combination of the AllowedRoutes field on the corresponding Listener and the Route's ParentRefs field. From af95e98792b7268e3ebab12790d79030f7537465 Mon Sep 17 00:00:00 2001 From: "gang.liu" Date: Thu, 16 Nov 2023 10:11:47 +0800 Subject: [PATCH 4/9] little refactor Signed-off-by: gang.liu --- changelogs/unreleased/5961-izturn-small.md | 2 -- internal/dag/status.go | 1 - 2 files changed, 3 deletions(-) diff --git a/changelogs/unreleased/5961-izturn-small.md b/changelogs/unreleased/5961-izturn-small.md index a868c17c836..130c664704c 100644 --- a/changelogs/unreleased/5961-izturn-small.md +++ b/changelogs/unreleased/5961-izturn-small.md @@ -1,3 +1 @@ -## Gateway API Attached Routes - For Gateway API v1.0, the successful attachment of a Route to a Listener is based solely on the combination of the AllowedRoutes field on the corresponding Listener and the Route's ParentRefs field. diff --git a/internal/dag/status.go b/internal/dag/status.go index 69624ec1bef..7649ded33c8 100644 --- a/internal/dag/status.go +++ b/internal/dag/status.go @@ -18,7 +18,6 @@ import ( contour_api_v1 "github.com/projectcontour/contour/apis/projectcontour/v1" "github.com/projectcontour/contour/internal/status" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" ) From b0ed9f10ff774ef55fc72db00e7de6560925e6c6 Mon Sep 17 00:00:00 2001 From: "gang.liu" Date: Mon, 20 Nov 2023 17:25:39 +0800 Subject: [PATCH 5/9] refactor Signed-off-by: gang.liu --- internal/dag/gatewayapi_processor.go | 191 +++++++++------------- internal/dag/gatewayapi_processor_test.go | 16 +- internal/gatewayapi/listeners.go | 37 ++--- 3 files changed, 107 insertions(+), 137 deletions(-) diff --git a/internal/dag/gatewayapi_processor.go b/internal/dag/gatewayapi_processor.go index 48fff1ed6ee..ddd5b21d5c1 100644 --- a/internal/dag/gatewayapi_processor.go +++ b/internal/dag/gatewayapi_processor.go @@ -81,35 +81,6 @@ type matchConditions struct { queryParams []QueryParamMatchCondition } -// computeAttachedRoutes compute the attached routes for the listener(s) -func (p *GatewayAPIProcessor) computeAttachedRoutes() map[string]int { - var listeners []*listenerInfo - for _, listener := range p.source.gateway.Spec.Listeners { - listeners = append(listeners, p.computeListenerForAttachedRoutes(listener)) - } - - // Keep track of the number of routes attached - // to each Listener so we can set status properly. - attachedRoutes := map[string]int{} - - for _, httpRoute := range p.source.httproutes { - p.computeAttachedRoutesForKind(KindHTTPRoute, httpRoute.Namespace, httpRoute.Spec.ParentRefs, listeners, attachedRoutes) - } - for _, tlsRoute := range p.source.tlsroutes { - p.computeAttachedRoutesForKind(KindTLSRoute, tlsRoute.Namespace, tlsRoute.Spec.ParentRefs, listeners, attachedRoutes) - } - - for _, grpcRoute := range p.source.grpcroutes { - p.computeAttachedRoutesForKind(KindGRPCRoute, grpcRoute.Namespace, grpcRoute.Spec.ParentRefs, listeners, attachedRoutes) - } - - for _, tcpRoute := range p.source.tcproutes { - p.computeAttachedRoutesForKind(KindTCPRoute, tcpRoute.Namespace, tcpRoute.Spec.ParentRefs, listeners, attachedRoutes) - } - - return attachedRoutes -} - // Run translates Gateway API types into DAG objects and // adds them to the DAG. func (p *GatewayAPIProcessor) Run(dag *DAG, source *KubernetesCache) { @@ -165,35 +136,36 @@ func (p *GatewayAPIProcessor) Run(dag *DAG, source *KubernetesCache) { } // Compute listeners and save a list of the valid/ready ones. - var readyListeners []*listenerInfo + var listenerInfos []*listenerInfo for _, listener := range p.source.gateway.Spec.Listeners { - if ready, listenerInfo := p.computeListener(listener, gwAccessor, validateListenersResult); ready { - readyListeners = append(readyListeners, listenerInfo) - } - + listenerInfos = append(listenerInfos, p.computeListener(listener, gwAccessor, validateListenersResult)) } + // Keep track of the number of routes attached + // to each Listener so we can set status properly. + listenerAttachedRoutes := map[string]int{} + // Process HTTPRoutes. for _, httpRoute := range p.source.httproutes { - p.processRoute(KindHTTPRoute, httpRoute, httpRoute.Spec.ParentRefs, gatewayNotProgrammedCondition, readyListeners, &gatewayapi_v1beta1.HTTPRoute{}) + p.processRoute(KindHTTPRoute, httpRoute, httpRoute.Spec.ParentRefs, gatewayNotProgrammedCondition, listenerInfos, listenerAttachedRoutes, &gatewayapi_v1beta1.HTTPRoute{}) } // Process TLSRoutes. for _, tlsRoute := range p.source.tlsroutes { - p.processRoute(KindTLSRoute, tlsRoute, tlsRoute.Spec.ParentRefs, gatewayNotProgrammedCondition, readyListeners, &gatewayapi_v1alpha2.TLSRoute{}) + p.processRoute(KindTLSRoute, tlsRoute, tlsRoute.Spec.ParentRefs, gatewayNotProgrammedCondition, listenerInfos, listenerAttachedRoutes, &gatewayapi_v1alpha2.TLSRoute{}) } // Process GRPCRoutes. for _, grpcRoute := range p.source.grpcroutes { - p.processRoute(KindGRPCRoute, grpcRoute, grpcRoute.Spec.ParentRefs, gatewayNotProgrammedCondition, readyListeners, &gatewayapi_v1alpha2.GRPCRoute{}) + p.processRoute(KindGRPCRoute, grpcRoute, grpcRoute.Spec.ParentRefs, gatewayNotProgrammedCondition, listenerInfos, listenerAttachedRoutes, &gatewayapi_v1alpha2.GRPCRoute{}) } // Process TCPRoutes. for _, tcpRoute := range p.source.tcproutes { - p.processRoute(KindTCPRoute, tcpRoute, tcpRoute.Spec.ParentRefs, gatewayNotProgrammedCondition, readyListeners, &gatewayapi_v1alpha2.TCPRoute{}) + p.processRoute(KindTCPRoute, tcpRoute, tcpRoute.Spec.ParentRefs, gatewayNotProgrammedCondition, listenerInfos, listenerAttachedRoutes, &gatewayapi_v1alpha2.TCPRoute{}) } - for listenerName, attachedRoutes := range p.computeAttachedRoutes() { + for listenerName, attachedRoutes := range listenerAttachedRoutes { gwAccessor.SetListenerAttachedRoutes(listenerName, attachedRoutes) } @@ -205,7 +177,8 @@ func (p *GatewayAPIProcessor) processRoute( route client.Object, parentRefs []gatewayapi_v1beta1.ParentReference, gatewayNotProgrammedCondition *metav1.Condition, - readyListeners []*listenerInfo, + listeners []*listenerInfo, + listenerAttachedRoutes map[string]int, emptyResource client.Object, ) { routeStatus, commit := p.dag.StatusCache.RouteConditionsAccessor( @@ -223,16 +196,18 @@ func (p *GatewayAPIProcessor) processRoute( routeParentStatus := routeStatus.StatusUpdateFor(routeParentRef) + // Get the list of listeners that are + // (a) included by this parent ref, and + // (b) allow the route (based on kind, namespace) + // (c) pass the all check for it + allowedListeners := p.getListenersForRouteParentRef(routeParentRef, route.GetNamespace(), routeKind, listeners, listenerAttachedRoutes, routeParentStatus) + // If the Gateway is invalid, set status on the route and we're done. if gatewayNotProgrammedCondition != nil { routeParentStatus.AddCondition(gatewayapi_v1beta1.RouteConditionAccepted, metav1.ConditionFalse, status.ReasonInvalidGateway, "Invalid Gateway") continue } - // Get the list of listeners that are (a) included by this parent ref, and - // (b) allow the route (based on kind, namespace). - allowedListeners := p.getListenersForRouteParentRef(routeParentRef, route.GetNamespace(), routeKind, readyListeners, routeParentStatus) - if len(allowedListeners) == 0 { p.resolveRouteRefs(route, routeParentStatus) } @@ -326,7 +301,8 @@ func (p *GatewayAPIProcessor) getListenersForRouteParentRef( routeParentRef gatewayapi_v1beta1.ParentReference, routeNamespace string, routeKind gatewayapi_v1beta1.Kind, - validListeners []*listenerInfo, + listeners []*listenerInfo, + attachedRoutes map[string]int, routeParentStatusAccessor *status.RouteParentStatusUpdate, ) []*listenerInfo { @@ -335,30 +311,30 @@ func (p *GatewayAPIProcessor) getListenersForRouteParentRef( // gateway, or one of them, if the ref is to a specific listener, // or none of them, if the listener(s) the ref targets are invalid). var selectedListeners []*listenerInfo - for _, validListener := range validListeners { + for _, listener := range listeners { // We've already verified the parent ref is for this Gateway, // now check if it has a listener name and port specified. // Both need to match the listener if specified. - if (routeParentRef.SectionName == nil || *routeParentRef.SectionName == validListener.listener.Name) && - (routeParentRef.Port == nil || *routeParentRef.Port == validListener.listener.Port) { - selectedListeners = append(selectedListeners, validListener) + if (routeParentRef.SectionName == nil || *routeParentRef.SectionName == listener.listener.Name) && + (routeParentRef.Port == nil || *routeParentRef.Port == listener.listener.Port) { + selectedListeners = append(selectedListeners, listener) } } - if len(selectedListeners) == 0 { - routeParentStatusAccessor.AddCondition( - gatewayapi_v1beta1.RouteConditionAccepted, - metav1.ConditionFalse, - gatewayapi_v1.RouteReasonNoMatchingParent, - "No listeners match this parent ref", - ) - return nil - } - // Now find the subset of those listeners that allow this route // to select them, based on route kind and namespace. var allowedListeners []*listenerInfo + + readyListenerCount := 0 + for _, selectedListener := range selectedListeners { + + // for compute the AttachedRoutes, the listener that not passed its check(s), had been selected too + // so ignore it. + if selectedListener.ready { + readyListenerCount++ + } + // Check if the listener allows routes of this kind if !selectedListener.AllowsKind(routeKind) { continue @@ -369,7 +345,21 @@ func (p *GatewayAPIProcessor) getListenersForRouteParentRef( continue } - allowedListeners = append(allowedListeners, selectedListener) + attachedRoutes[string(selectedListener.listener.Name)]++ + + if selectedListener.ready { + allowedListeners = append(allowedListeners, selectedListener) + } + + } + if readyListenerCount == 0 { + routeParentStatusAccessor.AddCondition( + gatewayapi_v1beta1.RouteConditionAccepted, + metav1.ConditionFalse, + gatewayapi_v1.RouteReasonNoMatchingParent, + "No listeners match this parent ref", + ) + return nil } if len(allowedListeners) == 0 { @@ -439,6 +429,7 @@ type listenerInfo struct { allowedKinds []gatewayapi_v1beta1.Kind namespaceSelector labels.Selector tlsSecret *Secret + ready bool } func (l *listenerInfo) AllowsKind(kind gatewayapi_v1beta1.Kind) bool { @@ -486,7 +477,12 @@ func (p *GatewayAPIProcessor) computeListener( listener gatewayapi_v1beta1.Listener, gwAccessor *status.GatewayStatusUpdate, validateListenersResult gatewayapi.ValidateListenersResult, -) (bool, *listenerInfo) { +) (info *listenerInfo) { + + info = &listenerInfo{ + listener: listener, + dagListenerName: validateListenersResult.ListenerNames[string(listener.Name)], + } addInvalidListenerCondition := func(msg string) { gwAccessor.AddListenerCondition( @@ -574,39 +570,37 @@ func (p *GatewayAPIProcessor) computeListener( } }() - // If the listener had an invalid protocol/port/hostname, we don't need to go - // any further. - if _, ok := validateListenersResult.InvalidListenerConditions[listener.Name]; ok { - return false, nil - } - // Get a list of the route kinds that the listener accepts. - listenerRouteKinds := p.getListenerRouteKinds(listener, gwAccessor) - gwAccessor.SetListenerSupportedKinds(string(listener.Name), listenerRouteKinds) - - var selector labels.Selector + info.allowedKinds = p.getListenerRouteKinds(listener, gwAccessor) + gwAccessor.SetListenerSupportedKinds(string(listener.Name), info.allowedKinds) if listener.AllowedRoutes != nil && listener.AllowedRoutes.Namespaces != nil && listener.AllowedRoutes.Namespaces.From != nil && *listener.AllowedRoutes.Namespaces.From == gatewayapi_v1.NamespacesFromSelector { if listener.AllowedRoutes.Namespaces.Selector == nil { addInvalidListenerCondition("Listener.AllowedRoutes.Namespaces.Selector is required when Listener.AllowedRoutes.Namespaces.From is set to \"Selector\".") - return false, nil + return } if len(listener.AllowedRoutes.Namespaces.Selector.MatchExpressions)+len(listener.AllowedRoutes.Namespaces.Selector.MatchLabels) == 0 { addInvalidListenerCondition("Listener.AllowedRoutes.Namespaces.Selector must specify at least one MatchLabel or MatchExpression.") - return false, nil + return } var err error - selector, err = metav1.LabelSelectorAsSelector(listener.AllowedRoutes.Namespaces.Selector) + info.namespaceSelector, err = metav1.LabelSelectorAsSelector(listener.AllowedRoutes.Namespaces.Selector) if err != nil { addInvalidListenerCondition(fmt.Sprintf("Error parsing Listener.AllowedRoutes.Namespaces.Selector: %v.", err)) - return false, nil + return } } + // If the listener had an invalid protocol/port/hostname, we reach here just for pick the information to compute the AttachedRoutes later, + // we don't need to go any further. + if _, invalid := validateListenersResult.InvalidListenerConditions[listener.Name]; invalid { + return + } + var listenerSecret *Secret // Validate TLS details for HTTPS/TLS protocol listeners. @@ -618,19 +612,19 @@ func (p *GatewayAPIProcessor) computeListener( if listener.TLS == nil { addInvalidListenerCondition(fmt.Sprintf("Listener.TLS is required when protocol is %q.", listener.Protocol)) - return false, nil + return } if listener.TLS.Mode != nil && *listener.TLS.Mode != gatewayapi_v1.TLSModeTerminate { addInvalidListenerCondition(fmt.Sprintf("Listener.TLS.Mode must be %q when protocol is %q.", gatewayapi_v1.TLSModeTerminate, listener.Protocol)) - return false, nil + return } // Resolve the TLS secret. if listenerSecret = p.resolveListenerSecret(listener.TLS.CertificateRefs, string(listener.Name), gwAccessor); listenerSecret == nil { // If TLS was configured on the Listener, but the secret ref is invalid, don't allow any // routes to be bound to this listener since it can't serve TLS traffic. - return false, nil + return } case gatewayapi_v1.TLSProtocolType: // The TLS protocol is used for TCP traffic encrypted with TLS. @@ -638,7 +632,7 @@ func (p *GatewayAPIProcessor) computeListener( // or passed through to the backend. if listener.TLS == nil { addInvalidListenerCondition(fmt.Sprintf("Listener.TLS is required when protocol is %q.", listener.Protocol)) - return false, nil + return } switch { @@ -647,50 +641,23 @@ func (p *GatewayAPIProcessor) computeListener( if listenerSecret = p.resolveListenerSecret(listener.TLS.CertificateRefs, string(listener.Name), gwAccessor); listenerSecret == nil { // If TLS was configured on the Listener, but the secret ref is invalid, don't allow any // routes to be bound to this listener since it can't serve TLS traffic. - return false, nil + return } case *listener.TLS.Mode == gatewayapi_v1.TLSModePassthrough: if len(listener.TLS.CertificateRefs) != 0 { addInvalidListenerCondition(fmt.Sprintf("Listener.TLS.CertificateRefs cannot be defined when Listener.TLS.Mode is %q.", gatewayapi_v1.TLSModePassthrough)) - return false, nil + return } default: addInvalidListenerCondition(fmt.Sprintf("Listener.TLS.Mode must be %q or %q.", gatewayapi_v1.TLSModeTerminate, gatewayapi_v1.TLSModePassthrough)) - return false, nil + return } } - return true, &listenerInfo{ - listener: listener, - dagListenerName: validateListenersResult.ListenerNames[string(listener.Name)], - allowedKinds: listenerRouteKinds, - tlsSecret: listenerSecret, - namespaceSelector: selector, - } -} - -// computeListenerForAttachedRoutes processes a Listener's spec. -// It returns a listenerInfo struct with the allowed route kinds and namespace selector -func (p *GatewayAPIProcessor) computeListenerForAttachedRoutes(listener gatewayapi_v1beta1.Listener) *listenerInfo { - var selector labels.Selector + info.tlsSecret = listenerSecret + info.ready = true + return - listenerRouteKinds := p.getListenerRouteKinds(listener, &status.GatewayStatusUpdate{}) - - if listener.AllowedRoutes != nil && listener.AllowedRoutes.Namespaces != nil && - listener.AllowedRoutes.Namespaces.From != nil && *listener.AllowedRoutes.Namespaces.From == gatewayapi_v1.NamespacesFromSelector { - - var err error - selector, err = metav1.LabelSelectorAsSelector(listener.AllowedRoutes.Namespaces.Selector) - if err != nil { - return nil - } - } - - return &listenerInfo{ - listener: listener, - allowedKinds: listenerRouteKinds, - namespaceSelector: selector, - } } // getListenerRouteKinds gets a list of the valid route kinds that diff --git a/internal/dag/gatewayapi_processor_test.go b/internal/dag/gatewayapi_processor_test.go index 3f464d3dc20..a1cd47da0cb 100644 --- a/internal/dag/gatewayapi_processor_test.go +++ b/internal/dag/gatewayapi_processor_test.go @@ -466,6 +466,7 @@ func TestGetListenersForRouteParentRef(t *testing.T) { }, }, allowedKinds: []gatewayapi_v1beta1.Kind{"HTTPRoute"}, + ready: true, }, { listener: gatewayapi_v1beta1.Listener{ @@ -477,6 +478,7 @@ func TestGetListenersForRouteParentRef(t *testing.T) { }, }, allowedKinds: []gatewayapi_v1beta1.Kind{"HTTPRoute"}, + ready: true, }, }, want: []int{0, 1}, @@ -526,6 +528,7 @@ func TestGetListenersForRouteParentRef(t *testing.T) { }, }, allowedKinds: []gatewayapi_v1beta1.Kind{"HTTPRoute"}, + ready: true, }, { listener: gatewayapi_v1beta1.Listener{ @@ -537,6 +540,7 @@ func TestGetListenersForRouteParentRef(t *testing.T) { }, }, allowedKinds: []gatewayapi_v1beta1.Kind{"HTTPRoute"}, + ready: true, }, }, want: []int{0, 1}, @@ -587,6 +591,7 @@ func TestGetListenersForRouteParentRef(t *testing.T) { }, }, allowedKinds: []gatewayapi_v1beta1.Kind{"HTTPRoute"}, + ready: true, }, { listener: gatewayapi_v1beta1.Listener{ @@ -598,6 +603,7 @@ func TestGetListenersForRouteParentRef(t *testing.T) { }, }, allowedKinds: []gatewayapi_v1beta1.Kind{"HTTPRoute"}, + ready: true, }, }, want: []int{0}, @@ -617,6 +623,7 @@ func TestGetListenersForRouteParentRef(t *testing.T) { }, }, allowedKinds: []gatewayapi_v1beta1.Kind{"HTTPRoute"}, + ready: true, }, { listener: gatewayapi_v1beta1.Listener{ @@ -628,6 +635,7 @@ func TestGetListenersForRouteParentRef(t *testing.T) { }, }, allowedKinds: []gatewayapi_v1beta1.Kind{"HTTPRoute"}, + ready: true, }, }, want: []int{1}, @@ -677,6 +685,7 @@ func TestGetListenersForRouteParentRef(t *testing.T) { }, }, allowedKinds: []gatewayapi_v1beta1.Kind{"TLSRoute"}, + ready: true, }, { listener: gatewayapi_v1beta1.Listener{ @@ -688,6 +697,7 @@ func TestGetListenersForRouteParentRef(t *testing.T) { }, }, allowedKinds: []gatewayapi_v1beta1.Kind{"HTTPRoute"}, + ready: true, }, }, want: []int{1}, @@ -707,6 +717,7 @@ func TestGetListenersForRouteParentRef(t *testing.T) { }, }, allowedKinds: []gatewayapi_v1beta1.Kind{"GRPCRoute"}, + ready: true, }, { listener: gatewayapi_v1beta1.Listener{ @@ -718,6 +729,7 @@ func TestGetListenersForRouteParentRef(t *testing.T) { }, }, allowedKinds: []gatewayapi_v1beta1.Kind{"HTTPRoute"}, + ready: true, }, }, want: []int{0}, @@ -746,7 +758,9 @@ func TestGetListenersForRouteParentRef(t *testing.T) { tc.routeParentRef, tc.routeNamespace, gatewayapi_v1beta1.Kind(tc.routeKind), - tc.listeners, rpsu) + tc.listeners, + map[string]int{}, + rpsu) var want []*listenerInfo for _, i := range tc.want { diff --git a/internal/gatewayapi/listeners.go b/internal/gatewayapi/listeners.go index 54194264747..26077c1be9c 100644 --- a/internal/gatewayapi/listeners.go +++ b/internal/gatewayapi/listeners.go @@ -54,6 +54,15 @@ type ListenerPort struct { Protocol string } +func conflictedCondition(reason gatewayapi_v1.ListenerConditionReason, msg string) metav1.Condition { + return metav1.Condition{ + Type: string(gatewayapi_v1.ListenerConditionConflicted), + Status: metav1.ConditionTrue, + Reason: string(reason), + Message: msg, + } +} + // ValidateListeners validates protocols, ports and hostnames on a set of listeners. // It ensures that: // - protocols are supported @@ -134,44 +143,24 @@ func ValidateListeners(listeners []gatewayapi_v1beta1.Listener) ValidateListener switch { case listener.Protocol == gatewayapi_v1.HTTPProtocolType: if otherListener.Protocol != gatewayapi_v1.HTTPProtocolType { - result.InvalidListenerConditions[listener.Name] = metav1.Condition{ - Type: string(gatewayapi_v1.ListenerConditionConflicted), - Status: metav1.ConditionTrue, - Reason: string(gatewayapi_v1.ListenerReasonProtocolConflict), - Message: "All Listener protocols for a given port must be compatible", - } + result.InvalidListenerConditions[listener.Name] = conflictedCondition(gatewayapi_v1.ListenerReasonProtocolConflict, "All Listener protocols for a given port must be compatible") return true } case compatibleTLSProtocols.Has(listener.Protocol): if !compatibleTLSProtocols.Has(otherListener.Protocol) { - result.InvalidListenerConditions[listener.Name] = metav1.Condition{ - Type: string(gatewayapi_v1.ListenerConditionConflicted), - Status: metav1.ConditionTrue, - Reason: string(gatewayapi_v1.ListenerReasonProtocolConflict), - Message: "All Listener protocols for a given port must be compatible", - } + result.InvalidListenerConditions[listener.Name] = conflictedCondition(gatewayapi_v1.ListenerReasonProtocolConflict, "All Listener protocols for a given port must be compatible") return true } case listener.Protocol == gatewayapi_v1.TCPProtocolType: if otherListener.Protocol != gatewayapi_v1.TCPProtocolType { - result.InvalidListenerConditions[listener.Name] = metav1.Condition{ - Type: string(gatewayapi_v1.ListenerConditionConflicted), - Status: metav1.ConditionTrue, - Reason: string(gatewayapi_v1.ListenerReasonProtocolConflict), - Message: "All Listener protocols for a given port must be compatible", - } + result.InvalidListenerConditions[listener.Name] = conflictedCondition(gatewayapi_v1.ListenerReasonProtocolConflict, "All Listener protocols for a given port must be compatible") return true } } // Hostname conflict if ref.Val(listener.Hostname, "") == ref.Val(otherListener.Hostname, "") { - result.InvalidListenerConditions[listener.Name] = metav1.Condition{ - Type: string(gatewayapi_v1.ListenerConditionConflicted), - Status: metav1.ConditionTrue, - Reason: string(gatewayapi_v1.ListenerReasonHostnameConflict), - Message: "All Listener hostnames for a given port must be unique", - } + result.InvalidListenerConditions[listener.Name] = conflictedCondition(gatewayapi_v1.ListenerReasonHostnameConflict, "All Listener hostnames for a given port must be unique") return true } } From 37a0ce925fc45dd934f6200371228682649cf960 Mon Sep 17 00:00:00 2001 From: "gang.liu" Date: Mon, 20 Nov 2023 17:33:20 +0800 Subject: [PATCH 6/9] make lint happy Signed-off-by: gang.liu --- internal/dag/gatewayapi_processor.go | 48 ---------------------------- 1 file changed, 48 deletions(-) diff --git a/internal/dag/gatewayapi_processor.go b/internal/dag/gatewayapi_processor.go index ddd5b21d5c1..16a918a08ad 100644 --- a/internal/dag/gatewayapi_processor.go +++ b/internal/dag/gatewayapi_processor.go @@ -375,54 +375,6 @@ func (p *GatewayAPIProcessor) getListenersForRouteParentRef( return allowedListeners } -// computeAttachedRoutesForKind compute attached routes for the specific kind -func (p *GatewayAPIProcessor) computeAttachedRoutesForKind( - routeKind gatewayapi_v1beta1.Kind, - namespace string, - parentRefs []gatewayapi_v1beta1.ParentReference, - listeners []*listenerInfo, - attachedRoutes map[string]int, -) { - - for _, parentRef := range parentRefs { - - // If this parent ref is to a different Gateway, ignore it. - if !gatewayapi.IsRefToGateway(parentRef, k8s.NamespacedNameOf(p.source.gateway)) { - continue - } - - // Find the set of listeners that are relevant given this - // parent ref (either all of them, if the ref is to the entire - // gateway, or one of them, if the ref is to a specific listener, - // or none of them, if the listener(s) the ref targets are invalid). - var selectedListeners []*listenerInfo - for _, listener := range listeners { - // We've already verified the parent ref is for this Gateway, - // now check if it has a listener name and port specified. - // Both need to match the listener if specified. - if (parentRef.SectionName == nil || *parentRef.SectionName == listener.listener.Name) && - (parentRef.Port == nil || *parentRef.Port == listener.listener.Port) { - selectedListeners = append(selectedListeners, listener) - } - } - - // Now find the subset of those listeners that allow this route - // to select them, based on route kind and namespace. - for _, selectedListener := range selectedListeners { - // Check if the listener allows routes of this kind - if !selectedListener.AllowsKind(routeKind) { - continue - } - - // Check if the route is in a namespace that the listener allows. - if !p.namespaceMatches(selectedListener.listener.AllowedRoutes.Namespaces, selectedListener.namespaceSelector, namespace) { - continue - } - attachedRoutes[string(selectedListener.listener.Name)]++ - } - } -} - type listenerInfo struct { listener gatewayapi_v1beta1.Listener dagListenerName string From 9559b2f5a03d98f3fbd2b23f7b8b0f9530b22a89 Mon Sep 17 00:00:00 2001 From: "gang.liu" Date: Mon, 20 Nov 2023 21:25:17 +0800 Subject: [PATCH 7/9] add ut Signed-off-by: gang.liu --- internal/dag/status_test.go | 94 +++++++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/internal/dag/status_test.go b/internal/dag/status_test.go index c21f887d242..933d8a526b3 100644 --- a/internal/dag/status_test.go +++ b/internal/dag/status_test.go @@ -9263,6 +9263,100 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) { wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 1), }) + run(t, "HTTP listener, route's parent ref route kind is invalid", testcase{ + objs: []any{ + kuardService, + &gatewayapi_v1beta1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "basic", + Namespace: "default", + }, + Spec: gatewayapi_v1beta1.HTTPRouteSpec{ + CommonRouteSpec: gatewayapi_v1beta1.CommonRouteSpec{ + ParentRefs: []gatewayapi_v1beta1.ParentReference{gatewayapi.GatewayListenerParentRef("projectcontour", "contour", "listener-1", 80)}, + }, + Hostnames: []gatewayapi_v1beta1.Hostname{"foo.projectcontour.io"}, + Rules: []gatewayapi_v1beta1.HTTPRouteRule{{ + Matches: gatewayapi.HTTPRouteMatch(gatewayapi_v1.PathMatchPathPrefix, "/"), + BackendRefs: gatewayapi.HTTPBackendRef("kuard", 8080, 1), + }}, + }, + }}, + gateway: &gatewayapi_v1beta1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "contour", + Namespace: "projectcontour", + }, + Spec: gatewayapi_v1beta1.GatewaySpec{ + Listeners: []gatewayapi_v1beta1.Listener{ + { + Name: "listener-1", + Port: 80, + Protocol: gatewayapi_v1.HTTPProtocolType, + AllowedRoutes: &gatewayapi_v1beta1.AllowedRoutes{ + Kinds: []gatewayapi_v1beta1.RouteGroupKind{ + {Kind: "FooRoute"}, + }, + }, + Hostname: ref.To(gatewayapi_v1beta1.Hostname("*.projectcontour.io")), + }, + }, + }, + }, + wantRouteConditions: []*status.RouteStatusUpdate{{ + FullName: types.NamespacedName{Namespace: "default", Name: "basic"}, + RouteParentStatuses: []*gatewayapi_v1beta1.RouteParentStatus{ + { + ParentRef: gatewayapi.GatewayListenerParentRef("projectcontour", "contour", "listener-1", 80), + Conditions: []metav1.Condition{ + { + Type: string(gatewayapi_v1.RouteConditionAccepted), + Status: contour_api_v1.ConditionFalse, + Reason: string(gatewayapi_v1beta1.RouteReasonNotAllowedByListeners), + Message: "No listeners included by this parent ref allowed this attachment.", + }, + routeResolvedRefsCondition(), + }, + }, + }, + }}, + wantGatewayStatusUpdate: []*status.GatewayStatusUpdate{ + { + FullName: types.NamespacedName{Namespace: "projectcontour", Name: "contour"}, + Conditions: map[gatewayapi_v1.GatewayConditionType]metav1.Condition{ + gatewayapi_v1.GatewayConditionAccepted: gatewayAcceptedCondition(), + gatewayapi_v1.GatewayConditionProgrammed: { + Type: string(gatewayapi_v1.GatewayConditionProgrammed), + Status: contour_api_v1.ConditionFalse, + Reason: string(gatewayapi_v1.GatewayReasonListenersNotValid), + Message: "Listeners are not valid", + }, + }, + ListenerStatus: map[string]*gatewayapi_v1beta1.ListenerStatus{ + "listener-1": { + Name: gatewayapi_v1beta1.SectionName("listener-1"), + AttachedRoutes: int32(0), + Conditions: []metav1.Condition{ + { + Type: string(gatewayapi_v1.ListenerConditionResolvedRefs), + Status: metav1.ConditionFalse, + Reason: string(gatewayapi_v1.ListenerReasonInvalidRouteKinds), + Message: "Kind \"FooRoute\" is not supported, kind must be \"HTTPRoute\", \"TLSRoute\", \"GRPCRoute\" or \"TCPRoute\"", + }, + { + Type: string(gatewayapi_v1.ListenerConditionProgrammed), + Status: metav1.ConditionFalse, + Reason: string(gatewayapi_v1.ListenerReasonInvalid), + Message: "Invalid listener, see other listener conditions for details", + }, + listenerAcceptedCondition(), + }, + }, + }, + }, + }, + }) + } func TestGatewayAPITLSRouteDAGStatus(t *testing.T) { From 86f18a5b9d332f868c5f3f8b593b5496e1342998 Mon Sep 17 00:00:00 2001 From: "gang.liu" Date: Wed, 6 Dec 2023 15:03:36 +0800 Subject: [PATCH 8/9] little refactor Signed-off-by: gang.liu --- .../gatewayapi/blue-green/blue-green.yaml | 4 +- internal/dag/gatewayapi_processor.go | 41 +++++++++---------- internal/dag/status_test.go | 2 +- 3 files changed, 22 insertions(+), 25 deletions(-) diff --git a/examples/example-workload/gatewayapi/blue-green/blue-green.yaml b/examples/example-workload/gatewayapi/blue-green/blue-green.yaml index 3a5cbec2bf8..e6db5031901 100644 --- a/examples/example-workload/gatewayapi/blue-green/blue-green.yaml +++ b/examples/example-workload/gatewayapi/blue-green/blue-green.yaml @@ -118,8 +118,8 @@ spec: - kind: Service name: green port: 80 - weight: 70 + weight: 90 - kind: Service name: blue port: 80 - weight: 30 + weight: 10 diff --git a/internal/dag/gatewayapi_processor.go b/internal/dag/gatewayapi_processor.go index 16a918a08ad..f46b0b50a85 100644 --- a/internal/dag/gatewayapi_processor.go +++ b/internal/dag/gatewayapi_processor.go @@ -195,19 +195,17 @@ func (p *GatewayAPIProcessor) processRoute( } routeParentStatus := routeStatus.StatusUpdateFor(routeParentRef) - - // Get the list of listeners that are - // (a) included by this parent ref, and - // (b) allow the route (based on kind, namespace) - // (c) pass the all check for it - allowedListeners := p.getListenersForRouteParentRef(routeParentRef, route.GetNamespace(), routeKind, listeners, listenerAttachedRoutes, routeParentStatus) - // If the Gateway is invalid, set status on the route and we're done. if gatewayNotProgrammedCondition != nil { routeParentStatus.AddCondition(gatewayapi_v1beta1.RouteConditionAccepted, metav1.ConditionFalse, status.ReasonInvalidGateway, "Invalid Gateway") continue } + // Get the list of listeners that are + // (a) included by this parent ref, and + // (b) allow the route (based on kind, namespace) + // (c) pass the all check for it + allowedListeners := p.getListenersForRouteParentRef(routeParentRef, route.GetNamespace(), routeKind, listeners, listenerAttachedRoutes, routeParentStatus) if len(allowedListeners) == 0 { p.resolveRouteRefs(route, routeParentStatus) } @@ -1123,8 +1121,8 @@ func (p *GatewayAPIProcessor) computeHTTPRouteForListener( // Get match conditions for the rule. var matchconditions []*matchConditions for _, match := range rule.Matches { - pathMatch, ok := gatewayPathMatchCondition(match.Path, routeAccessor) - if !ok { + pathMatch := gatewayPathMatchCondition(match.Path, routeAccessor) + if pathMatch == nil { continue } @@ -1793,9 +1791,9 @@ func validateAppProtocol(svc *v1.ServicePort) error { return fmt.Errorf("AppProtocol: \"%s\" is unsupported", *svc.AppProtocol) } -func gatewayPathMatchCondition(match *gatewayapi_v1beta1.HTTPPathMatch, routeAccessor *status.RouteParentStatusUpdate) (MatchCondition, bool) { +func gatewayPathMatchCondition(match *gatewayapi_v1beta1.HTTPPathMatch, routeAccessor *status.RouteParentStatusUpdate) MatchCondition { if match == nil { - return &PrefixMatchCondition{Prefix: "/"}, true + return &PrefixMatchCondition{Prefix: "/"} } path := ref.Val(match.Value, "/") @@ -1804,41 +1802,40 @@ func gatewayPathMatchCondition(match *gatewayapi_v1beta1.HTTPPathMatch, routeAcc if match.Type == nil || *match.Type == gatewayapi_v1.PathMatchPathPrefix { if !strings.HasPrefix(path, "/") { routeAccessor.AddCondition(status.ConditionValidMatches, metav1.ConditionFalse, status.ReasonInvalidPathMatch, "Match.Path.Value must start with '/'.") - return nil, false + return nil } if strings.Contains(path, "//") { routeAccessor.AddCondition(status.ConditionValidMatches, metav1.ConditionFalse, status.ReasonInvalidPathMatch, "Match.Path.Value must not contain consecutive '/' characters.") - return nil, false + return nil } // As an optimization, if path is just "/", we can use // string prefix matching instead of segment prefix // matching which requires a regex. if path == "/" { - return &PrefixMatchCondition{Prefix: path}, true + return &PrefixMatchCondition{Prefix: path} } - return &PrefixMatchCondition{Prefix: path, PrefixMatchType: PrefixMatchSegment}, true + return &PrefixMatchCondition{Prefix: path, PrefixMatchType: PrefixMatchSegment} } if *match.Type == gatewayapi_v1.PathMatchExact { if !strings.HasPrefix(path, "/") { routeAccessor.AddCondition(status.ConditionValidMatches, metav1.ConditionFalse, status.ReasonInvalidPathMatch, "Match.Path.Value must start with '/'.") - return nil, false + return nil } if strings.Contains(path, "//") { routeAccessor.AddCondition(status.ConditionValidMatches, metav1.ConditionFalse, status.ReasonInvalidPathMatch, "Match.Path.Value must not contain consecutive '/' characters.") - return nil, false + return nil } - - return &ExactMatchCondition{Path: path}, true + return &ExactMatchCondition{Path: path} } if *match.Type == gatewayapi_v1.PathMatchRegularExpression { if err := ValidateRegex(*match.Value); err != nil { routeAccessor.AddCondition(status.ConditionValidMatches, metav1.ConditionFalse, status.ReasonInvalidPathMatch, "Match.Path.Value is invalid for RegularExpression match type.") - return nil, false + return nil } - return &RegexMatchCondition{Regex: path}, true + return &RegexMatchCondition{Regex: path} } routeAccessor.AddCondition( @@ -1847,7 +1844,7 @@ func gatewayPathMatchCondition(match *gatewayapi_v1beta1.HTTPPathMatch, routeAcc gatewayapi_v1beta1.RouteReasonUnsupportedValue, "HTTPRoute.Spec.Rules.PathMatch: Only Prefix match type, Exact match type and RegularExpression match type are supported.", ) - return nil, false + return nil } func gatewayHeaderMatchConditions(matches []gatewayapi_v1beta1.HTTPHeaderMatch) ([]HeaderMatchCondition, error) { diff --git a/internal/dag/status_test.go b/internal/dag/status_test.go index 933d8a526b3..e3c22ef5fa7 100644 --- a/internal/dag/status_test.go +++ b/internal/dag/status_test.go @@ -9263,7 +9263,7 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) { wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 1), }) - run(t, "HTTP listener, route's parent ref route kind is invalid", testcase{ + run(t, "HTTP listener with invalid AllowedRoute kind referenced by route parent ref", testcase{ objs: []any{ kuardService, &gatewayapi_v1beta1.HTTPRoute{ From 1cb7d008b084b2e5f7638911440d57505700c1de Mon Sep 17 00:00:00 2001 From: "gang.liu" Date: Tue, 12 Dec 2023 09:58:31 +0800 Subject: [PATCH 9/9] little refactor Signed-off-by: gang.liu --- .../gatewayapi/blue-green/blue-green.yaml | 4 +-- internal/dag/gatewayapi_processor.go | 32 +++++++++---------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/examples/example-workload/gatewayapi/blue-green/blue-green.yaml b/examples/example-workload/gatewayapi/blue-green/blue-green.yaml index e6db5031901..3a5cbec2bf8 100644 --- a/examples/example-workload/gatewayapi/blue-green/blue-green.yaml +++ b/examples/example-workload/gatewayapi/blue-green/blue-green.yaml @@ -118,8 +118,8 @@ spec: - kind: Service name: green port: 80 - weight: 90 + weight: 70 - kind: Service name: blue port: 80 - weight: 10 + weight: 30 diff --git a/internal/dag/gatewayapi_processor.go b/internal/dag/gatewayapi_processor.go index de05949d601..b76ef7f07a6 100644 --- a/internal/dag/gatewayapi_processor.go +++ b/internal/dag/gatewayapi_processor.go @@ -205,8 +205,8 @@ func (p *GatewayAPIProcessor) processRoute( // Get the list of listeners that are // (a) included by this parent ref, and - // (b) allow the route (based on kind, namespace) - // (c) pass the all check for it + // (b) allow the route (based on kind, namespace), and + // (c) the 'listenerInfo.ready' is true allowedListeners := p.getListenersForRouteParentRef(routeParentRef, route.GetNamespace(), routeKind, listeners, listenerAttachedRoutes, routeParentStatus) if len(allowedListeners) == 0 { p.resolveRouteRefs(route, routeParentStatus) @@ -429,9 +429,9 @@ func (p *GatewayAPIProcessor) computeListener( listener gatewayapi_v1beta1.Listener, gwAccessor *status.GatewayStatusUpdate, validateListenersResult gatewayapi.ValidateListenersResult, -) (info *listenerInfo) { +) *listenerInfo { - info = &listenerInfo{ + info := &listenerInfo{ listener: listener, dagListenerName: validateListenersResult.ListenerNames[string(listener.Name)], } @@ -531,26 +531,26 @@ func (p *GatewayAPIProcessor) computeListener( if listener.AllowedRoutes.Namespaces.Selector == nil { addInvalidListenerCondition("Listener.AllowedRoutes.Namespaces.Selector is required when Listener.AllowedRoutes.Namespaces.From is set to \"Selector\".") - return + return info } if len(listener.AllowedRoutes.Namespaces.Selector.MatchExpressions)+len(listener.AllowedRoutes.Namespaces.Selector.MatchLabels) == 0 { addInvalidListenerCondition("Listener.AllowedRoutes.Namespaces.Selector must specify at least one MatchLabel or MatchExpression.") - return + return info } var err error info.namespaceSelector, err = metav1.LabelSelectorAsSelector(listener.AllowedRoutes.Namespaces.Selector) if err != nil { addInvalidListenerCondition(fmt.Sprintf("Error parsing Listener.AllowedRoutes.Namespaces.Selector: %v.", err)) - return + return info } } // If the listener had an invalid protocol/port/hostname, we reach here just for pick the information to compute the AttachedRoutes later, // we don't need to go any further. if _, invalid := validateListenersResult.InvalidListenerConditions[listener.Name]; invalid { - return + return info } var listenerSecret *Secret @@ -564,19 +564,19 @@ func (p *GatewayAPIProcessor) computeListener( if listener.TLS == nil { addInvalidListenerCondition(fmt.Sprintf("Listener.TLS is required when protocol is %q.", listener.Protocol)) - return + return info } if listener.TLS.Mode != nil && *listener.TLS.Mode != gatewayapi_v1.TLSModeTerminate { addInvalidListenerCondition(fmt.Sprintf("Listener.TLS.Mode must be %q when protocol is %q.", gatewayapi_v1.TLSModeTerminate, listener.Protocol)) - return + return info } // Resolve the TLS secret. if listenerSecret = p.resolveListenerSecret(listener.TLS.CertificateRefs, string(listener.Name), gwAccessor); listenerSecret == nil { // If TLS was configured on the Listener, but the secret ref is invalid, don't allow any // routes to be bound to this listener since it can't serve TLS traffic. - return + return info } case gatewayapi_v1.TLSProtocolType: // The TLS protocol is used for TCP traffic encrypted with TLS. @@ -584,7 +584,7 @@ func (p *GatewayAPIProcessor) computeListener( // or passed through to the backend. if listener.TLS == nil { addInvalidListenerCondition(fmt.Sprintf("Listener.TLS is required when protocol is %q.", listener.Protocol)) - return + return info } switch { @@ -593,22 +593,22 @@ func (p *GatewayAPIProcessor) computeListener( if listenerSecret = p.resolveListenerSecret(listener.TLS.CertificateRefs, string(listener.Name), gwAccessor); listenerSecret == nil { // If TLS was configured on the Listener, but the secret ref is invalid, don't allow any // routes to be bound to this listener since it can't serve TLS traffic. - return + return info } case *listener.TLS.Mode == gatewayapi_v1.TLSModePassthrough: if len(listener.TLS.CertificateRefs) != 0 { addInvalidListenerCondition(fmt.Sprintf("Listener.TLS.CertificateRefs cannot be defined when Listener.TLS.Mode is %q.", gatewayapi_v1.TLSModePassthrough)) - return + return info } default: addInvalidListenerCondition(fmt.Sprintf("Listener.TLS.Mode must be %q or %q.", gatewayapi_v1.TLSModeTerminate, gatewayapi_v1.TLSModePassthrough)) - return + return info } } info.tlsSecret = listenerSecret info.ready = true - return + return info }