gateway2/delegation: fix cycle detection

changelog/v1.18.0-rc3/fix-10379.yaml

@@ -0,0 +1,11 @@
+changelog:
+  - type: FIX
+    issueLink: https://github.com/k8sgateway/k8sgateway/issues/10379
+    resolvesIssue: false
+    description: |
+      There's a bug in how cycles are detected when evaluating a
+      delegation chain, because of which a multi-level delegation
+      tree with the same child route being referenced by multiple
+      parent HTTP routes is broken. This change fixes the detection
+      of cycles during the evaluation of the delegation chain.
+

projects/gateway2/query/httproute.go

@@ -245,7 +245,13 @@ func (r *gatewayQueries) getDelegatedChildren(
 		visited = sets.New[types.NamespacedName]()
 	}
 	parentRef := namespacedName(parent)
+	// `visited` is used to detect cyclic references to routes in the delegation chain.
+	// It is important to remove the route from the set once all its children have been evaluated
+	// in the recursion stack, because a route may have multiple parents that have the same ancestor:
+	// e.g., A -> B1, A -> B2, B1 -> C, B2 -> C. So in this case, even though C is visited twice,
+	// the delegation chain is valid as it is evaluated only once for each parent.
 	visited.Insert(parentRef)
+	defer visited.Delete(parentRef)
 
 	children := NewBackendMap[[]*RouteInfo]()
 	for _, parentRule := range parent.Spec.Rules {
@@ -264,7 +270,7 @@ func (r *gatewayQueries) getDelegatedChildren(
 			for _, childRoute := range referencedRoutes {
 				childRef := namespacedName(&childRoute)
 				if visited.Has(childRef) {
-					err := fmt.Errorf("ignoring child route %s for parent %s: %w", parentRef, childRef, ErrCyclicReference)
+					err := fmt.Errorf("ignoring child route %s for parent %s: %w", childRef, parentRef, ErrCyclicReference)
 					children.AddError(backendRef.BackendObjectReference, err)
 					// Don't resolve invalid child route
 					continue

projects/gateway2/translator/gateway_translator_test.go

@@ -316,4 +316,6 @@ var _ = DescribeTable("Route Delegation translator",
 	Entry("RouteOptions multi level inheritance with child override", "route_options_multi_level_inheritance_override_ok.yaml"),
 	Entry("RouteOptions filter override merge", "route_options_filter_override_merge.yaml"),
 	Entry("Child route matcher does not match parent", "bug-6621.yaml"),
+	// https://github.com/k8sgateway/k8sgateway/issues/10379
+	Entry("Multi-level multiple parents delegation", "bug-10379.yaml"),
 )

projects/gateway2/translator/testutils/inputs/delegation/bug-10379.yaml

@@ -0,0 +1,127 @@
+apiVersion: gateway.networking.k8s.io/v1
+kind: Gateway
+metadata:
+  name: example-gateway
+  namespace: infra
+spec:
+  gatewayClassName: example-gateway-class
+  listeners:
+  - name: http
+    protocol: HTTP
+    port: 80
+---
+apiVersion: gateway.networking.k8s.io/v1
+kind: HTTPRoute
+metadata:
+  name: api-example-com
+  namespace: infra
+  labels:
+    app: apis
+spec:
+  parentRefs:
+    - name: example-gateway
+  hostnames:
+    - "api.example.com"
+  rules:
+    - matches:
+      - path:
+          type: PathPrefix
+          value: /api1
+      backendRefs:
+      - group: gateway.networking.k8s.io
+        kind: HTTPRoute
+        name: apiproduct-1
+        namespace: default
+    - matches:
+      - path:
+          type: PathPrefix
+          value: /api2
+      backendRefs:
+      - group: gateway.networking.k8s.io
+        kind: HTTPRoute
+        name: apiproduct-2
+        namespace: default
+---
+apiVersion: gateway.networking.k8s.io/v1
+kind: HTTPRoute
+metadata:
+  name: apiproduct-1
+  namespace: default
+  labels:
+    app: apis
+  annotations:
+    delegation.gateway.solo.io/inherit-parent-matcher: "true"
+spec:
+  rules:
+    - matches:
+      - path:
+          type: PathPrefix
+          value: /
+      filters:
+      - type: URLRewrite
+        urlRewrite:
+          path:
+            type: ReplacePrefixMatch
+            replacePrefixMatch: /
+      backendRefs:
+      - group: gateway.networking.k8s.io
+        kind: HTTPRoute
+        name: httpbin
+        namespace: default
+---
+apiVersion: gateway.networking.k8s.io/v1
+kind: HTTPRoute
+metadata:
+  name: apiproduct-2
+  namespace: default
+  labels:
+    app: apis
+  annotations:
+    delegation.gateway.solo.io/inherit-parent-matcher: "true"
+spec:
+  rules:
+    - matches:
+      - path:
+          type: PathPrefix
+          value: /
+      filters:
+      - type: URLRewrite
+        urlRewrite:
+          path:
+            type: ReplacePrefixMatch
+            replacePrefixMatch: /
+      backendRefs:
+      - group: gateway.networking.k8s.io
+        kind: HTTPRoute
+        name: httpbin
+        namespace: default
+---
+apiVersion: gateway.networking.k8s.io/v1
+kind: HTTPRoute
+metadata:
+  name: httpbin
+  namespace: default
+  labels:
+    app: apis
+  annotations:
+    delegation.gateway.solo.io/inherit-parent-matcher: "true"
+spec:
+  rules:
+    - matches:
+      - path:
+          type: PathPrefix
+          value: /
+      backendRefs:
+      - name: httpbin
+        namespace: default
+        port: 8000
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: httpbin
+  namespace: default
+spec:
+  ports:
+    - protocol: TCP
+      port: 8000

projects/gateway2/translator/testutils/outputs/delegation/bug-10379.yaml

@@ -0,0 +1,53 @@
+---
+listeners:
+- aggregateListener:
+    httpFilterChains:
+    - matcher: {}
+      virtualHostRefs:
+      - http~api_example_com
+    httpResources:
+      virtualHosts:
+        http~api_example_com:
+          domains:
+          - api.example.com
+          name: http~api_example_com
+          routes:
+          - matchers:
+            - prefix: /api1
+            name: httproute-httpbin-default-0-0
+            options:
+              regexRewrite:
+                pattern:
+                  regex: ^/api1\/*
+                substitution: /
+            routeAction:
+              single:
+                kube:
+                  port: 8000
+                  ref:
+                    name: httpbin
+                    namespace: default
+          - matchers:
+            - prefix: /api2
+            name: httproute-httpbin-default-0-0
+            options:
+              regexRewrite:
+                pattern:
+                  regex: ^/api2\/*
+                substitution: /
+            routeAction:
+              single:
+                kube:
+                  port: 8000
+                  ref:
+                    name: httpbin
+                    namespace: default
+  bindAddress: '::'
+  bindPort: 8080
+  name: http
+metadata:
+  labels:
+    created_by: gloo-kube-gateway-api
+    gateway_namespace: infra
+  name: infra-example-gateway
+  namespace: gloo-system