From d6ddf78af5f0f8dccfb96a84c43beb6cbb620c04 Mon Sep 17 00:00:00 2001
From: Arko Dasgupta <arko@tetrate.io>
Date: Thu, 26 Oct 2023 17:33:33 -0700
Subject: [PATCH] Fix attachedRoutes computation

* Fixes: https://github.com/envoyproxy/gateway/issues/2077
* Fixes: https://github.com/envoyproxy/gateway/issues/1916

Signed-off-by: Arko Dasgupta <arko@tetrate.io>
---
 internal/gatewayapi/route.go                  | 54 +++++++------------
 test/conformance/conformance_test.go          |  1 -
 .../experimental_conformance_test.go          |  1 -
 3 files changed, 20 insertions(+), 36 deletions(-)

diff --git a/internal/gatewayapi/route.go b/internal/gatewayapi/route.go
index fe0ec085def7..f7634248c62f 100644
--- a/internal/gatewayapi/route.go
+++ b/internal/gatewayapi/route.go
@@ -580,12 +580,6 @@ func (t *Translator) processHTTPRouteParentRefListener(route RouteContext, route
 			}
 			irListener.Routes = append(irListener.Routes, perHostRoutes...)
 		}
-		// Theoretically there should only be one parent ref per
-		// Route that attaches to a given Listener, so fine to just increment here, but we
-		// might want to check to ensure we're not double-counting.
-		if len(routeRoutes) > 0 {
-			listener.IncrementAttachedRoutes()
-		}
 	}
 
 	return hasHostnameIntersection
@@ -688,12 +682,6 @@ func (t *Translator) processTLSRouteParentRefs(tlsRoute *TLSRouteContext, resour
 			gwXdsIR := xdsIR[irKey]
 			gwXdsIR.TCP = append(gwXdsIR.TCP, irListener)
 
-			// Theoretically there should only be one parent ref per
-			// Route that attaches to a given Listener, so fine to just increment here, but we
-			// might want to check to ensure we're not double-counting.
-			if len(irListener.Destination.Settings) > 0 {
-				listener.IncrementAttachedRoutes()
-			}
 		}
 
 		if !hasHostnameIntersection {
@@ -825,12 +813,6 @@ func (t *Translator) processUDPRouteParentRefs(udpRoute *UDPRouteContext, resour
 			gwXdsIR := xdsIR[irKey]
 			gwXdsIR.UDP = append(gwXdsIR.UDP, irListener)
 
-			// Theoretically there should only be one parent ref per
-			// Route that attaches to a given Listener, so fine to just increment here, but we
-			// might want to check to ensure we're not double-counting.
-			if len(irListener.Destination.Settings) > 0 {
-				listener.IncrementAttachedRoutes()
-			}
 		}
 
 		// If no negative conditions have been set, the route is considered "Accepted=True".
@@ -962,12 +944,6 @@ func (t *Translator) processTCPRouteParentRefs(tcpRoute *TCPRouteContext, resour
 			gwXdsIR := xdsIR[irKey]
 			gwXdsIR.TCP = append(gwXdsIR.TCP, irListener)
 
-			// Theoretically there should only be one parent ref per
-			// Route that attaches to a given Listener, so fine to just increment here, but we
-			// might want to check to ensure we're not double-counting.
-			if len(irListener.Destination.Settings) > 0 {
-				listener.IncrementAttachedRoutes()
-			}
 		}
 
 		// If no negative conditions have been set, the route is considered "Accepted=True".
@@ -1094,16 +1070,6 @@ func (t *Translator) processAllowedListenersForParentRefs(routeContext RouteCont
 			continue
 		}
 
-		if !HasReadyListener(selectedListeners) {
-			parentRefCtx.SetCondition(routeContext,
-				gwapiv1.RouteConditionAccepted,
-				metav1.ConditionFalse,
-				"NoReadyListeners",
-				"There are no ready listeners for this parent ref",
-			)
-			continue
-		}
-
 		var allowedListeners []*ListenerContext
 		for _, listener := range selectedListeners {
 			acceptedKind := GetRouteType(routeContext)
@@ -1125,6 +1091,26 @@ func (t *Translator) processAllowedListenersForParentRefs(routeContext RouteCont
 
 		parentRefCtx.SetListeners(allowedListeners...)
 
+		// Its safe to increment AttachedRoutes since we've found a valid parentRef
+		// and the listener allows this Route kind
+
+		// Theoretically there should only be one parent ref per
+		// Route that attaches to a given Listener, so fine to just increment here, but we
+		// might want to check to ensure we're not double-counting.
+		for _, listener := range allowedListeners {
+			listener.IncrementAttachedRoutes()
+		}
+
+		if !HasReadyListener(selectedListeners) {
+			parentRefCtx.SetCondition(routeContext,
+				gwapiv1.RouteConditionAccepted,
+				metav1.ConditionFalse,
+				"NoReadyListeners",
+				"There are no ready listeners for this parent ref",
+			)
+			continue
+		}
+
 		parentRefCtx.SetCondition(routeContext,
 			gwapiv1.RouteConditionAccepted,
 			metav1.ConditionTrue,
diff --git a/test/conformance/conformance_test.go b/test/conformance/conformance_test.go
index eeb8898ffc7b..dce91b6940c0 100644
--- a/test/conformance/conformance_test.go
+++ b/test/conformance/conformance_test.go
@@ -50,7 +50,6 @@ func TestGatewayAPIConformance(t *testing.T) {
 		SkipTests: []string{
 			tests.HTTPRouteRewritePath.ShortName,
 			tests.GatewayStaticAddresses.ShortName,
-			tests.GatewayWithAttachedRoutes.ShortName,
 			tests.HTTPRouteBackendProtocolH2C.ShortName,
 		},
 		ExemptFeatures: suite.MeshCoreFeatures,
diff --git a/test/conformance/experimental_conformance_test.go b/test/conformance/experimental_conformance_test.go
index fd4a5620973e..d128025dc0e3 100644
--- a/test/conformance/experimental_conformance_test.go
+++ b/test/conformance/experimental_conformance_test.go
@@ -99,7 +99,6 @@ func experimentalConformance(t *testing.T) {
 				SkipTests: []string{
 					tests.HTTPRouteRewritePath.ShortName,
 					tests.GatewayStaticAddresses.ShortName,
-					tests.GatewayWithAttachedRoutes.ShortName,
 					tests.HTTPRouteBackendProtocolH2C.ShortName,
 				},
 				ExemptFeatures: suite.MeshCoreFeatures,