From 1b7a8d663a783e4cb9b5de9e71e4989927cf02ca Mon Sep 17 00:00:00 2001 From: Jenny Shu <28537278+jenshu@users.noreply.github.com> Date: Fri, 11 Apr 2025 14:53:18 -0400 Subject: [PATCH 1/3] [1.19] HTTPRoute delegation parentRefs fix (#10763) --- .../delegation-parentref-fix.yaml | 7 ++ .../translator/gateway_translator_test.go | 1 + .../httproute/delegation_helpers.go | 64 ++++++---- .../translator/httproute/delegation_test.go | 22 +--- .../inputs/delegation/inherit_parentref.yaml | 111 ++++++++++++++++++ .../outputs/delegation/inherit_parentref.yaml | 59 ++++++++++ 6 files changed, 221 insertions(+), 43 deletions(-) create mode 100644 changelog/v1.19.0-beta18/delegation-parentref-fix.yaml create mode 100644 projects/gateway2/translator/testutils/inputs/delegation/inherit_parentref.yaml create mode 100644 projects/gateway2/translator/testutils/outputs/delegation/inherit_parentref.yaml diff --git a/changelog/v1.19.0-beta18/delegation-parentref-fix.yaml b/changelog/v1.19.0-beta18/delegation-parentref-fix.yaml new file mode 100644 index 00000000000..df2fed2623c --- /dev/null +++ b/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. diff --git a/projects/gateway2/translator/gateway_translator_test.go b/projects/gateway2/translator/gateway_translator_test.go index 9454a729500..37dccbe60ed 100644 --- a/projects/gateway2/translator/gateway_translator_test.go +++ b/projects/gateway2/translator/gateway_translator_test.go @@ -307,6 +307,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"), diff --git a/projects/gateway2/translator/httproute/delegation_helpers.go b/projects/gateway2/translator/httproute/delegation_helpers.go index 2c7544409c8..bb28dda0312 100644 --- a/projects/gateway2/translator/httproute/delegation_helpers.go +++ b/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 diff --git a/projects/gateway2/translator/httproute/delegation_test.go b/projects/gateway2/translator/httproute/delegation_test.go index eca648ad9b0..e514d769f54 100644 --- a/projects/gateway2/translator/httproute/delegation_test.go +++ b/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,15 +519,6 @@ 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, }, } @@ -546,7 +526,7 @@ func TestIsDelegatedRouteMatch(t *testing.T) { 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) }) diff --git a/projects/gateway2/translator/testutils/inputs/delegation/inherit_parentref.yaml b/projects/gateway2/translator/testutils/inputs/delegation/inherit_parentref.yaml new file mode 100644 index 00000000000..c97063d79f6 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/projects/gateway2/translator/testutils/outputs/delegation/inherit_parentref.yaml b/projects/gateway2/translator/testutils/outputs/delegation/inherit_parentref.yaml new file mode 100644 index 00000000000..af2b864de59 --- /dev/null +++ b/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 From cdb470695c53188d6581a626a5ebb6ac41a47113 Mon Sep 17 00:00:00 2001 From: Jenny Shu Date: Fri, 11 Apr 2025 15:08:43 -0400 Subject: [PATCH 2/3] move changelog --- .../{v1.19.0-beta18 => v1.18.14}/delegation-parentref-fix.yaml | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog/{v1.19.0-beta18 => v1.18.14}/delegation-parentref-fix.yaml (100%) diff --git a/changelog/v1.19.0-beta18/delegation-parentref-fix.yaml b/changelog/v1.18.14/delegation-parentref-fix.yaml similarity index 100% rename from changelog/v1.19.0-beta18/delegation-parentref-fix.yaml rename to changelog/v1.18.14/delegation-parentref-fix.yaml From 795539b5685b16faad7fa781e6c4edae014679b9 Mon Sep 17 00:00:00 2001 From: Jenny Shu Date: Fri, 11 Apr 2025 15:30:44 -0400 Subject: [PATCH 3/3] fix test --- .../testutils/outputs/delegation/inherit_parentref.yaml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/projects/gateway2/translator/testutils/outputs/delegation/inherit_parentref.yaml b/projects/gateway2/translator/testutils/outputs/delegation/inherit_parentref.yaml index af2b864de59..d5475dd4165 100644 --- a/projects/gateway2/translator/testutils/outputs/delegation/inherit_parentref.yaml +++ b/projects/gateway2/translator/testutils/outputs/delegation/inherit_parentref.yaml @@ -44,12 +44,6 @@ listeners: namespace: infra bindAddress: '::' bindPort: 8080 - metadataStatic: - sources: - - resourceKind: gateway.networking.k8s.io/Gateway - resourceRef: - name: http - namespace: infra name: http metadata: labels: