[1.19] HTTPRoute delegation parentRefs fix

changelog/v1.19.0-beta18/delegation-parentref-fix.yaml

@@ -0,0 +1,7 @@
+changelog:
+- type: FIX
+  issueLink: https://github.com/solo-io/solo-projects/issues/8163
+  resolvesIssue: false
+  description: >-
+    Fixed a bug in route delegation where the `parentRefs` field on child HTTPRoutes
+    were not being respected when parent matcher inheritance was enabled.

projects/gateway2/translator/gateway_translator_test.go

@@ -402,6 +402,7 @@ var _ = DescribeTable("Route Delegation translator",
 	Entry("Basic config", "basic.yaml"),
 	Entry("Child matches parent via parentRefs", "basic_parentref_match.yaml"),
 	Entry("Child doesn't match parent via parentRefs", "basic_parentref_mismatch.yaml"),
+	Entry("Children using parentRefs and inherit-parent-matcher", "inherit_parentref.yaml"),
 	Entry("Parent delegates to multiple chidren", "multiple_children.yaml"),
 	Entry("Child is invalid as it is delegatee and specifies hostnames", "basic_invalid_hostname.yaml"),
 	Entry("Multi-level recursive delegation", "recursive.yaml"),

projects/gateway2/translator/httproute/delegation_helpers.go

@@ -39,6 +39,16 @@ func filterDelegatedChildren(
 	// Select the child routes that match the parent
 	var selected []*query.RouteInfo
 	for _, c := range children {
+		origChild, ok := c.Object.(*gwv1.HTTPRoute)
+		if !ok {
+			continue
+		}
+
+		// Check if the child route is allowed to be delegated to by the parent
+		if !isAllowedParent(parentRef, origChild.GetNamespace(), origChild.Spec.ParentRefs) {
+			continue
+		}
+
 		// make a copy; multiple parents can delegate to the same child so we can't modify a shared reference
 		clone := c.Clone()
 
@@ -73,7 +83,7 @@ func filterDelegatedChildren(
 					// the parent's matcher with the child's.
 					mergeParentChildRouteMatch(&parentMatch, &match)
 					validMatches = append(validMatches, match)
-				} else if ok := isDelegatedRouteMatch(parentMatch, parentRef, match, child.Namespace, child.Spec.ParentRefs); ok {
+				} else if ok := isDelegatedRouteMatch(parentMatch, match); ok {
 					// Non-inherited matcher delegation requires matching child matcher to parent matcher
 					// to delegate from the parent route to the child.
 					validMatches = append(validMatches, match)
@@ -99,34 +109,44 @@ func filterDelegatedChildren(
 	return selected
 }
 
-func isDelegatedRouteMatch(
-	parent gwv1.HTTPRouteMatch,
+// isAllowedParent returns whether the parent specified by `parentRef` is allowed to delegate
+// to the child.
+//   - `childNs` is the namespace of the child route.
+//   - `childParentRefs` is the list of parent references on the child route. If this is empty, then
+//     there are no restrictions on which parents can delegate to this child. If it is not empty,
+//     then `parentRef` must be in this list in order for the parent to delegate to the child.
+func isAllowedParent(
 	parentRef types.NamespacedName,
-	child gwv1.HTTPRouteMatch,
 	childNs string,
-	parentRefs []gwv1.ParentReference,
+	childParentRefs []gwv1.ParentReference,
 ) bool {
-	// If the child has parentRefs set, validate that it matches the parent route
-	if len(parentRefs) > 0 {
-		matched := false
-		for _, ref := range parentRefs {
-			refNs := childNs
-			if ref.Namespace != nil {
-				refNs = string(*ref.Namespace)
-			}
-			if ref.Group != nil && *ref.Group == wellknown.GatewayGroup &&
-				ref.Kind != nil && *ref.Kind == wellknown.HTTPRouteKind &&
-				string(ref.Name) == parentRef.Name &&
-				refNs == parentRef.Namespace {
-				matched = true
-				break
-			}
+	// no explicit parentRefs, so any parent is allowed
+	if len(childParentRefs) == 0 {
+		return true
+	}
+
+	// validate that the child's parentRefs contains the specified parentRef
+	for _, ref := range childParentRefs {
+		// default to the child's namespace if not specified
+		refNs := childNs
+		if ref.Namespace != nil {
+			refNs = string(*ref.Namespace)
 		}
-		if !matched {
-			return false
+		// check if the ref matches the desired parentRef
+		if ref.Group != nil && *ref.Group == wellknown.GatewayGroup &&
+			ref.Kind != nil && *ref.Kind == wellknown.HTTPRouteKind &&
+			string(ref.Name) == parentRef.Name &&
+			refNs == parentRef.Namespace {
+			return true
 		}
 	}
+	return false
+}
 
+func isDelegatedRouteMatch(
+	parent gwv1.HTTPRouteMatch,
+	child gwv1.HTTPRouteMatch,
+) bool {
 	// Validate path
 	if parent.Path == nil || parent.Path.Type == nil || *parent.Path.Type != gwv1.PathMatchPathPrefix {
 		return false

projects/gateway2/translator/httproute/delegation_test.go

@@ -3,7 +3,6 @@ package httproute
 import (
 	"testing"
 
-	"github.com/solo-io/gloo/projects/gateway2/wellknown"
 	"github.com/stretchr/testify/assert"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/utils/ptr"
@@ -446,16 +445,6 @@ func TestIsDelegatedRouteMatch(t *testing.T) {
 					},
 				},
 			},
-			parentRef: types.NamespacedName{Namespace: "default", Name: "parent"},
-			childNs:   "child",
-			parentRefs: []gwv1.ParentReference{
-				{
-					Group:     ptr.To[gwv1.Group](gwv1.Group(wellknown.GatewayGroup)),
-					Kind:      ptr.To[gwv1.Kind](gwv1.Kind(wellknown.HTTPRouteKind)),
-					Name:      "parent",
-					Namespace: ptr.To[gwv1.Namespace](gwv1.Namespace("default")),
-				},
-			},
 			expected: true,
 		},
 		{
@@ -530,23 +519,14 @@ func TestIsDelegatedRouteMatch(t *testing.T) {
 					},
 				},
 			},
-			parentRef: types.NamespacedName{Namespace: "default", Name: "parent"},
-			childNs:   "default",
-			parentRefs: []gwv1.ParentReference{
-				{
-					Group: ptr.To[gwv1.Group](gwv1.Group(wellknown.GatewayGroup)),
-					Kind:  ptr.To[gwv1.Kind](gwv1.Kind(wellknown.HTTPRouteKind)),
-					Name:  "parent",
-				},
-			},
 			expected: true,
 		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			a := assert.New(t)
-			actual := isDelegatedRouteMatch(tc.parent, tc.parentRef, tc.child, tc.childNs, tc.parentRefs)
+			actual := isDelegatedRouteMatch(tc.parent, tc.child)
 
 			a.Equal(tc.expected, actual)
 		})

projects/gateway2/translator/testutils/inputs/delegation/inherit_parentref.yaml

@@ -0,0 +1,111 @@
+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: example-route
+  namespace: infra
+spec:
+  parentRefs:
+  - name: example-gateway
+  hostnames:
+  - "example.com"
+  rules:
+  - backendRefs:
+    - name: example-svc
+      port: 80
+  - matches:
+    - path:
+        type: PathPrefix
+        value: /a
+      headers:
+      - type: Exact
+        name: header1
+        value: val1
+      queryParams:
+      - type: Exact
+        name: query1
+        value: val1
+    backendRefs:
+    - group: gateway.networking.k8s.io
+      kind: HTTPRoute
+      name: "*"
+      namespace: a
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: example-svc
+  namespace: infra
+spec:
+  selector:
+    test: test
+  ports:
+    - protocol: TCP
+      port: 80
+---
+apiVersion: gateway.networking.k8s.io/v1
+kind: HTTPRoute
+metadata:
+  name: route-a
+  namespace: a
+  annotations:
+    delegation.gateway.solo.io/inherit-parent-matcher: "true"
+spec:
+  parentRefs:
+  - name: invalid
+    namespace: infra
+    group: gateway.networking.k8s.io
+    kind: HTTPRoute
+  rules:
+  - matches:
+    - path:
+        type: Exact
+        value: /foo
+      method: PUT
+    backendRefs:
+    - name: svc-a
+      port: 8080
+---
+apiVersion: gateway.networking.k8s.io/v1
+kind: HTTPRoute
+metadata:
+  name: route-b
+  namespace: a
+  annotations:
+    delegation.gateway.solo.io/inherit-parent-matcher: "true"
+spec:
+  parentRefs:
+  - name: example-route
+    namespace: infra
+    group: gateway.networking.k8s.io
+    kind: HTTPRoute
+  rules:
+  - matches:
+    - path:
+        type: Exact
+        value: /bar
+      method: POST
+    backendRefs:
+    - name: svc-a
+      port: 8080
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: svc-a
+  namespace: a
+spec:
+  ports:
+    - protocol: TCP
+      port: 8080

projects/gateway2/translator/testutils/outputs/delegation/inherit_parentref.yaml

@@ -0,0 +1,59 @@
+---
+listeners:
+- aggregateListener:
+    httpFilterChains:
+    - matcher: {}
+      virtualHostRefs:
+      - http~example_com
+    httpResources:
+      virtualHosts:
+        http~example_com:
+          domains:
+          - example.com
+          name: http~example_com
+          routes:
+          - matchers:
+            - exact: /a/bar
+              headers:
+              - name: header1
+                value: val1
+              methods:
+              - POST
+              queryParameters:
+              - name: query1
+                value: val1
+            name: httproute-route-b-a-0-0
+            options: {}
+            routeAction:
+              single:
+                kube:
+                  port: 8080
+                  ref:
+                    name: svc-a
+                    namespace: a
+          - matchers:
+            - prefix: /
+            options: {}
+            name: httproute-example-route-infra-0-0
+            routeAction:
+              single:
+                kube:
+                  port: 80
+                  ref:
+                    name: example-svc
+                    namespace: infra
+  bindAddress: '::'
+  bindPort: 8080
+  metadataStatic:
+    sources:
+    - resourceKind: gateway.networking.k8s.io/Gateway
+      resourceRef:
+        name: http
+        namespace: infra
+  name: http
+metadata:
+  labels:
+    created_by: gloo-kube-gateway-api
+    gateway_namespace: infra
+  name: infra-example-gateway
+  namespace: gloo-system