Fix attachedRoutes computation (#2085)

* Fix attachedRoutes computation * Fixes: #2077 * Fixes: #1916 Signed-off-by: Arko Dasgupta <[email protected]> * make testdata Signed-off-by: Arko Dasgupta <[email protected]> --------- Signed-off-by: Arko Dasgupta <[email protected]>
envoyproxy · Nov 1, 2023 · 93a12e7 · 93a12e7
1 parent 6d532c1
commit 93a12e7
Show file tree

Hide file tree

Showing 53 changed files with 85 additions and 101 deletions.
diff --git a/internal/cmd/egctl/testdata/translate/out/rejected-http-route.route.yaml b/internal/cmd/egctl/testdata/translate/out/rejected-http-route.route.yaml
@@ -67,8 +67,8 @@ httpRoutes:
     parents:
     - conditions:
       - lastTransitionTime: null
-        message: There are no ready listeners for this parent ref
-        reason: NoReadyListeners
+        message: No listeners included by this parent ref allowed this attachment.
+        reason: NotAllowedByListeners
         status: "False"
         type: Accepted
       - lastTransitionTime: null

diff --git a/internal/gatewayapi/route.go b/internal/gatewayapi/route.go
@@ -572,12 +572,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
@@ -680,12 +674,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 {
@@ -792,7 +780,7 @@ func (t *Translator) processUDPRouteParentRefs(udpRoute *UDPRouteContext, resour
 		accepted := false
 		for _, listener := range parentRef.listeners {
 			// only one route is allowed for a UDP listener
-			if listener.AttachedRoutes() > 0 {
+			if listener.AttachedRoutes() > 1 {
 				continue
 			}
 			if !listener.IsReady() {
@@ -817,12 +805,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".
@@ -929,7 +911,7 @@ func (t *Translator) processTCPRouteParentRefs(tcpRoute *TCPRouteContext, resour
 		accepted := false
 		for _, listener := range parentRef.listeners {
 			// only one route is allowed for a TCP listener
-			if listener.AttachedRoutes() > 0 {
+			if listener.AttachedRoutes() > 1 {
 				continue
 			}
 			if !listener.IsReady() {
@@ -954,12 +936,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".
@@ -1110,16 +1086,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)
@@ -1139,6 +1105,26 @@ func (t *Translator) processAllowedListenersForParentRefs(routeContext RouteCont
 			continue
 		}
 
+		// 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.SetListeners(allowedListeners...)
 
 		parentRefCtx.SetCondition(routeContext,

diff --git a/...nal/gatewayapi/testdata/extensions/httproute-with-extension-filter-invalid-group.out.yaml b/...nal/gatewayapi/testdata/extensions/httproute-with-extension-filter-invalid-group.out.yaml
@@ -17,7 +17,7 @@ gateways:
       protocol: HTTP
   status:
     listeners:
-    - attachedRoutes: 0
+    - attachedRoutes: 1
       conditions:
       - lastTransitionTime: null
         message: Sending translated listener configuration to the data plane

diff --git a/...rnal/gatewayapi/testdata/extensions/httproute-with-non-matching-extension-filter.out.yaml b/...rnal/gatewayapi/testdata/extensions/httproute-with-non-matching-extension-filter.out.yaml
@@ -17,7 +17,7 @@ gateways:
       protocol: HTTP
   status:
     listeners:
-    - attachedRoutes: 0
+    - attachedRoutes: 1
       conditions:
       - lastTransitionTime: null
         message: Sending translated listener configuration to the data plane

diff --git a/internal/gatewayapi/testdata/extensions/httproute-with-unsupported-extension-filter.out.yaml b/internal/gatewayapi/testdata/extensions/httproute-with-unsupported-extension-filter.out.yaml
@@ -17,7 +17,7 @@ gateways:
       protocol: HTTP
   status:
     listeners:
-    - attachedRoutes: 0
+    - attachedRoutes: 1
       conditions:
       - lastTransitionTime: null
         message: Sending translated listener configuration to the data plane

diff --git a/...tewayapi/testdata/gateway-with-listener-with-invalid-allowed-namespaces-selector.out.yaml b/...tewayapi/testdata/gateway-with-listener-with-invalid-allowed-namespaces-selector.out.yaml
@@ -64,8 +64,8 @@ httpRoutes:
     parents:
     - conditions:
       - lastTransitionTime: null
-        message: There are no ready listeners for this parent ref
-        reason: NoReadyListeners
+        message: No listeners included by this parent ref allowed this attachment.
+        reason: NotAllowedByListeners
         status: "False"
         type: Accepted
       - lastTransitionTime: null

diff --git a/...rnal/gatewayapi/testdata/gateway-with-listener-with-invalid-allowed-routes-group.out.yaml b/...rnal/gatewayapi/testdata/gateway-with-listener-with-invalid-allowed-routes-group.out.yaml
@@ -55,8 +55,8 @@ httpRoutes:
     parents:
     - conditions:
       - lastTransitionTime: null
-        message: There are no ready listeners for this parent ref
-        reason: NoReadyListeners
+        message: No listeners included by this parent ref allowed this attachment.
+        reason: NotAllowedByListeners
         status: "False"
         type: Accepted
       - lastTransitionTime: null

diff --git a/...pi/testdata/gateway-with-listener-with-invalid-allowed-routes-kind-and-supported.out.yaml b/...pi/testdata/gateway-with-listener-with-invalid-allowed-routes-kind-and-supported.out.yaml
@@ -57,8 +57,8 @@ httpRoutes:
     parents:
     - conditions:
       - lastTransitionTime: null
-        message: There are no ready listeners for this parent ref
-        reason: NoReadyListeners
+        message: No listeners included by this parent ref allowed this attachment.
+        reason: NotAllowedByListeners
         status: "False"
         type: Accepted
       - lastTransitionTime: null

diff --git a/internal/gatewayapi/testdata/gateway-with-listener-with-invalid-allowed-routes-kind.out.yaml b/internal/gatewayapi/testdata/gateway-with-listener-with-invalid-allowed-routes-kind.out.yaml
@@ -55,8 +55,8 @@ httpRoutes:
     parents:
     - conditions:
       - lastTransitionTime: null
-        message: There are no ready listeners for this parent ref
-        reason: NoReadyListeners
+        message: No listeners included by this parent ref allowed this attachment.
+        reason: NotAllowedByListeners
         status: "False"
         type: Accepted
       - lastTransitionTime: null

diff --git a/...al/gatewayapi/testdata/gateway-with-listener-with-invalid-allowed-tls-route-kind.out.yaml b/...al/gatewayapi/testdata/gateway-with-listener-with-invalid-allowed-tls-route-kind.out.yaml
@@ -65,8 +65,8 @@ tlsRoutes:
     parents:
     - conditions:
       - lastTransitionTime: null
-        message: There are no ready listeners for this parent ref
-        reason: NoReadyListeners
+        message: No listeners included by this parent ref allowed this attachment.
+        reason: NotAllowedByListeners
         status: "False"
         type: Accepted
       - lastTransitionTime: null

diff --git a/...atewayapi/testdata/gateway-with-listener-with-invalid-multiple-tls-configuration.out.yaml b/...atewayapi/testdata/gateway-with-listener-with-invalid-multiple-tls-configuration.out.yaml
@@ -28,7 +28,7 @@ gateways:
         mode: Terminate
   status:
     listeners:
-    - attachedRoutes: 0
+    - attachedRoutes: 1
       conditions:
       - lastTransitionTime: null
         message: Secret envoy-gateway/tls-secret-ecdsa-2 public key algorithm must

diff --git a/...ayapi/testdata/gateway-with-listener-with-invalid-tls-configuration-invalid-mode.out.yaml b/...ayapi/testdata/gateway-with-listener-with-invalid-tls-configuration-invalid-mode.out.yaml
@@ -23,7 +23,7 @@ gateways:
         mode: Passthrough
   status:
     listeners:
-    - attachedRoutes: 0
+    - attachedRoutes: 1
       conditions:
       - lastTransitionTime: null
         message: TLS Passthrough mode is not supported, TLS mode must be Terminate.

diff --git a/...estdata/gateway-with-listener-with-invalid-tls-configuration-no-certificate-refs.out.yaml b/...estdata/gateway-with-listener-with-invalid-tls-configuration-no-certificate-refs.out.yaml
@@ -18,7 +18,7 @@ gateways:
         mode: Terminate
   status:
     listeners:
-    - attachedRoutes: 0
+    - attachedRoutes: 1
       conditions:
       - lastTransitionTime: null
         message: Listener must have at least 1 TLS certificate ref

diff --git a/...teway-with-listener-with-invalid-tls-configuration-no-valid-certificate-for-fqdn.out.yaml b/...teway-with-listener-with-invalid-tls-configuration-no-valid-certificate-for-fqdn.out.yaml
@@ -23,7 +23,7 @@ gateways:
         mode: Terminate
   status:
     listeners:
-    - attachedRoutes: 0
+    - attachedRoutes: 1
       conditions:
       - lastTransitionTime: null
         message: Secret envoy-gateway/tls-secret-1 must contain valid tls.crt and

diff --git a/...tdata/gateway-with-listener-with-invalid-tls-configuration-secret-does-not-exist.out.yaml b/...tdata/gateway-with-listener-with-invalid-tls-configuration-secret-does-not-exist.out.yaml
@@ -22,7 +22,7 @@ gateways:
         mode: Terminate
   status:
     listeners:
-    - attachedRoutes: 0
+    - attachedRoutes: 1
       conditions:
       - lastTransitionTime: null
         message: Secret envoy-gateway/tls-secret-1 does not exist.

diff --git a/...a/gateway-with-listener-with-invalid-tls-configuration-secret-in-other-namespace.out.yaml b/...a/gateway-with-listener-with-invalid-tls-configuration-secret-in-other-namespace.out.yaml
@@ -23,7 +23,7 @@ gateways:
         mode: Terminate
   status:
     listeners:
-    - attachedRoutes: 0
+    - attachedRoutes: 1
       conditions:
       - lastTransitionTime: null
         message: Certificate ref to secret default/tls-secret-1 not permitted by any

diff --git a/...estdata/gateway-with-listener-with-invalid-tls-configuration-secret-is-not-valid.out.yaml b/...estdata/gateway-with-listener-with-invalid-tls-configuration-secret-is-not-valid.out.yaml
@@ -22,7 +22,7 @@ gateways:
         mode: Terminate
   status:
     listeners:
-    - attachedRoutes: 0
+    - attachedRoutes: 1
       conditions:
       - lastTransitionTime: null
         message: Secret envoy-gateway/tls-secret-1 must contain tls.crt and tls.key.

diff --git a/...tewayapi/testdata/gateway-with-listener-with-missing-allowed-namespaces-selector.out.yaml b/...tewayapi/testdata/gateway-with-listener-with-missing-allowed-namespaces-selector.out.yaml
@@ -57,8 +57,8 @@ httpRoutes:
     parents:
     - conditions:
       - lastTransitionTime: null
-        message: There are no ready listeners for this parent ref
-        reason: NoReadyListeners
+        message: No listeners included by this parent ref allowed this attachment.
+        reason: NotAllowedByListeners
         status: "False"
         type: Accepted
       - lastTransitionTime: null

diff --git a/...ewayapi/testdata/gateway-with-listener-with-tcproute-with-mismatch-port-protocol.out.yaml b/...ewayapi/testdata/gateway-with-listener-with-tcproute-with-mismatch-port-protocol.out.yaml
@@ -16,7 +16,7 @@ gateways:
       protocol: TCP
   status:
     listeners:
-    - attachedRoutes: 0
+    - attachedRoutes: 1
       conditions:
       - lastTransitionTime: null
         message: Sending translated listener configuration to the data plane

diff --git a/...l/gatewayapi/testdata/gateway-with-listener-with-tcproute-with-multiple-backends.out.yaml b/...l/gatewayapi/testdata/gateway-with-listener-with-tcproute-with-multiple-backends.out.yaml
@@ -16,7 +16,7 @@ gateways:
       protocol: TCP
   status:
     listeners:
-    - attachedRoutes: 0
+    - attachedRoutes: 1
       conditions:
       - lastTransitionTime: null
         message: Sending translated listener configuration to the data plane

diff --git a/...rnal/gatewayapi/testdata/gateway-with-listener-with-tcproute-with-multiple-rules.out.yaml b/...rnal/gatewayapi/testdata/gateway-with-listener-with-tcproute-with-multiple-rules.out.yaml
@@ -16,7 +16,7 @@ gateways:
       protocol: TCP
   status:
     listeners:
-    - attachedRoutes: 0
+    - attachedRoutes: 1
       conditions:
       - lastTransitionTime: null
         message: Sending translated listener configuration to the data plane

diff --git a/...ewayapi/testdata/gateway-with-listener-with-udproute-with-mismatch-port-protocol.out.yaml b/...ewayapi/testdata/gateway-with-listener-with-udproute-with-mismatch-port-protocol.out.yaml
@@ -16,7 +16,7 @@ gateways:
       protocol: UDP
   status:
     listeners:
-    - attachedRoutes: 0
+    - attachedRoutes: 1
       conditions:
       - lastTransitionTime: null
         message: Sending translated listener configuration to the data plane

diff --git a/...l/gatewayapi/testdata/gateway-with-listener-with-udproute-with-multiple-backends.out.yaml b/...l/gatewayapi/testdata/gateway-with-listener-with-udproute-with-multiple-backends.out.yaml
@@ -16,7 +16,7 @@ gateways:
       protocol: UDP
   status:
     listeners:
-    - attachedRoutes: 0
+    - attachedRoutes: 1
       conditions:
       - lastTransitionTime: null
         message: Sending translated listener configuration to the data plane

diff --git a/...rnal/gatewayapi/testdata/gateway-with-listener-with-udproute-with-multiple-rules.out.yaml b/...rnal/gatewayapi/testdata/gateway-with-listener-with-udproute-with-multiple-rules.out.yaml
@@ -16,7 +16,7 @@ gateways:
       protocol: UDP
   status:
     listeners:
-    - attachedRoutes: 0
+    - attachedRoutes: 1
       conditions:
       - lastTransitionTime: null
         message: Sending translated listener configuration to the data plane

diff --git a/internal/gatewayapi/testdata/gateway-with-listener-with-unsupported-protocol.out.yaml b/internal/gatewayapi/testdata/gateway-with-listener-with-unsupported-protocol.out.yaml
@@ -57,8 +57,8 @@ httpRoutes:
     parents:
     - conditions:
       - lastTransitionTime: null
-        message: There are no ready listeners for this parent ref
-        reason: NoReadyListeners
+        message: No listeners included by this parent ref allowed this attachment.
+        reason: NotAllowedByListeners
         status: "False"
         type: Accepted
       - lastTransitionTime: null

diff --git a/internal/gatewayapi/testdata/gateway-with-single-listener-with-multiple-tcproutes.out.yaml b/internal/gatewayapi/testdata/gateway-with-single-listener-with-multiple-tcproutes.out.yaml
@@ -16,7 +16,7 @@ gateways:
       protocol: TCP
   status:
     listeners:
-    - attachedRoutes: 1
+    - attachedRoutes: 2
       conditions:
       - lastTransitionTime: null
         message: Sending translated listener configuration to the data plane

diff --git a/internal/gatewayapi/testdata/gateway-with-single-listener-with-multiple-udproutes.out.yaml b/internal/gatewayapi/testdata/gateway-with-single-listener-with-multiple-udproutes.out.yaml
@@ -16,7 +16,7 @@ gateways:
       protocol: UDP
   status:
     listeners:
-    - attachedRoutes: 1
+    - attachedRoutes: 2
       conditions:
       - lastTransitionTime: null
         message: Sending translated listener configuration to the data plane

diff --git a/internal/gatewayapi/testdata/gateway-with-two-listeners-on-same-tcp-or-tls-port.out.yaml b/internal/gatewayapi/testdata/gateway-with-two-listeners-on-same-tcp-or-tls-port.out.yaml
@@ -43,7 +43,7 @@ gateways:
       supportedKinds:
       - group: gateway.networking.k8s.io
         kind: TCPRoute
-    - attachedRoutes: 0
+    - attachedRoutes: 1
       conditions:
       - lastTransitionTime: null
         message: Only one TCP/TLS listener is allowed in a given port

diff --git a/internal/gatewayapi/testdata/gateway-with-two-listeners-on-same-udp-port.out.yaml b/internal/gatewayapi/testdata/gateway-with-two-listeners-on-same-udp-port.out.yaml
@@ -43,7 +43,7 @@ gateways:
       supportedKinds:
       - group: gateway.networking.k8s.io
         kind: UDPRoute
-    - attachedRoutes: 0
+    - attachedRoutes: 1
       conditions:
       - lastTransitionTime: null
         message: Only one UDP listener is allowed in a given port

diff --git a/...estdata/gateway-with-two-listeners-with-http-and-tlsroute-same-hostname-and-port.out.yaml b/...estdata/gateway-with-two-listeners-with-http-and-tlsroute-same-hostname-and-port.out.yaml
@@ -26,7 +26,7 @@ gateways:
         mode: Passthrough
   status:
     listeners:
-    - attachedRoutes: 0
+    - attachedRoutes: 1
       conditions:
       - lastTransitionTime: null
         message: All listeners for a given port must use a unique hostname
@@ -49,7 +49,7 @@ gateways:
         kind: HTTPRoute
       - group: gateway.networking.k8s.io
         kind: GRPCRoute
-    - attachedRoutes: 0
+    - attachedRoutes: 1
       conditions:
       - lastTransitionTime: null
         message: All listeners for a given port must use a unique hostname