Skip to content

Commit

Permalink
Merge branch 'main' into sh/safe-ports-e2e
Browse files Browse the repository at this point in the history
  • Loading branch information
sam-heilbron authored Nov 26, 2024
2 parents fb71e34 + a5c0926 commit f30fac5
Show file tree
Hide file tree
Showing 5 changed files with 200 additions and 1 deletion.
11 changes: 11 additions & 0 deletions changelog/v1.18.0-rc3/fix-10379.yaml
Original file line number Diff line number Diff line change
@@ -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.
8 changes: 7 additions & 1 deletion projects/gateway2/query/httproute.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down
2 changes: 2 additions & 0 deletions projects/gateway2/translator/gateway_translator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
)
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit f30fac5

Please sign in to comment.