Skip to content

fix: httproute precedence by considering header/query match type #5740

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
arkodg merged 5 commits into from
May 4, 2025
Merged

fix: httproute precedence by considering header/query match type #5740

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
87f05b2
fix httproute precedence by considering match type
kkk777-7 Apr 16, 2025
2e9145c
add gatewayapi testdata for httproute precedence
kkk777-7 Apr 16, 2025
c51db61
fix precedence to use the exact number of characters
kkk777-7 Apr 17, 2025
7a84618
fix precedence to use number of exact matches
kkk777-7 Apr 19, 2025
42c8552
update gatewayapi testdata output files
kkk777-7 May 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 31 additions & 1 deletion internal/gatewayapi/sort.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
// Equal case

// 3. Sort based on the number of Header matches.
// When the number is same, sort based on number of Exact Header matches.
Copy link
Contributor

@arkodg arkodg Apr 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this part of the spec, dont see it in https://gateway-api.sigs.k8s.io/reference/spec/#httprouterule

the issue has an example across HTTPRoutes #5736
and for this case, the spec says

If ties still exist across multiple Routes, matching precedence MUST be
determined in order of the following criteria, continuing on ties:
The oldest Route based on creation timestamp.
* The Route appearing first in alphabetical order by
"{namespace}/{name}".

Copy link
Contributor

@arkodg arkodg Apr 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we dont have access to the timestamp here, but can rely on alphabetical order
cc @guydc

Copy link
Member Author

@kkk777-7 kkk777-7 Apr 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I mistakenly thought that we should prioritize Exact header match .

If we sort the resources in alphabetical order for this case, there may be cases where a Regex header match is preferred over Exact.
is it correct to understand that in accordance with GWAPI spec, the header match type does not affect priority?

If my understanding is correct, I will fix on the approach you suggested!

Copy link
Contributor

@arkodg arkodg Apr 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will bring this up in the community meeting today, and get more clarity on this

hCountI := len(x[i].HeaderMatches)
hCountJ := len(x[j].HeaderMatches)
if hCountI < hCountJ {
Expand All @@ -71,12 +72,31 @@
if hCountI > hCountJ {
return false
}

hExtNumberI := numberOfExactMatches(x[i].HeaderMatches)
hExtNumberJ := numberOfExactMatches(x[j].HeaderMatches)
if hExtNumberI < hExtNumberJ {
return true
}
if hExtNumberI > hExtNumberJ {
return false
}

Check warning on line 83 in internal/gatewayapi/sort.go

View check run for this annotation

Codecov / codecov/patch

internal/gatewayapi/sort.go#L82-L83 

Added lines #L82 - L83 were not covered by tests
// Equal case

// 4. Sort based on the number of Query param matches.
// When the number is same, sort based on number of Exact Query param matches.
qCountI := len(x[i].QueryParamMatches)
qCountJ := len(x[j].QueryParamMatches)
return qCountI < qCountJ
if qCountI < qCountJ {
return true
}

Check warning on line 92 in internal/gatewayapi/sort.go

View check run for this annotation

Codecov / codecov/patch

internal/gatewayapi/sort.go#L91-L92 

Added lines #L91 - L92 were not covered by tests
if qCountI > qCountJ {
return false
}

qExtNumberI := numberOfExactMatches(x[i].QueryParamMatches)
qExtNumberJ := numberOfExactMatches(x[j].QueryParamMatches)
return qExtNumberI < qExtNumberJ
}

// sortXdsIR sorts the xdsIR based on the match precedence
Expand Down Expand Up @@ -107,3 +127,13 @@
}
return 0
}

func numberOfExactMatches(stringMatches []*ir.StringMatch) int {
var cnt int
for _, stringMatch := range stringMatches {
if stringMatch != nil && stringMatch.Exact != nil {
cnt++
}
}
return cnt
}
68 changes: 68 additions & 0 deletions internal/gatewayapi/testdata/httproute-with-header-match-diff-number.in.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
gateways:
- apiVersion: gateway.networking.k8s.io/v1
kind: Gateway
metadata:
namespace: envoy-gateway
name: gateway-1
spec:
gatewayClassName: envoy-gateway-class
listeners:
- name: http
protocol: HTTP
port: 80
hostname: '*.envoyproxy.io'
allowedRoutes:
namespaces:
from: All
httpRoutes:
- apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
namespace: default
name: httproute-1
spec:
hostnames:
- gateway.envoyproxy.io
parentRefs:
- namespace: envoy-gateway
name: gateway-1
sectionName: http
rules:
- matches:
- path:
type: PathPrefix
value: '/'
headers:
- type: RegularExpression
name: x-org-id
value: '.*'
- type: RegularExpression
name: hostname
value: '.*'
backendRefs:
- name: service-1
port: 8080
- apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
namespace: default
name: httproute-2
spec:
hostnames:
- gateway.envoyproxy.io
parentRefs:
- namespace: envoy-gateway
name: gateway-1
sectionName: http
rules:
- matches:
- path:
type: PathPrefix
value: '/'
headers:
- type: Exact
name: x-org-id
value: test-org
backendRefs:
- name: service-1
port: 8080
228 changes: 228 additions & 0 deletions internal/gatewayapi/testdata/httproute-with-header-match-diff-number.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,228 @@
gateways:
- apiVersion: gateway.networking.k8s.io/v1
kind: Gateway
metadata:
creationTimestamp: null
name: gateway-1
namespace: envoy-gateway
spec:
gatewayClassName: envoy-gateway-class
listeners:
- allowedRoutes:
namespaces:
from: All
hostname: '*.envoyproxy.io'
name: http
port: 80
protocol: HTTP
status:
listeners:
- attachedRoutes: 2
conditions:
- lastTransitionTime: null
message: Sending translated listener configuration to the data plane
reason: Programmed
status: "True"
type: Programmed
- lastTransitionTime: null
message: Listener has been successfully translated
reason: Accepted
status: "True"
type: Accepted
- lastTransitionTime: null
message: Listener references have been resolved
reason: ResolvedRefs
status: "True"
type: ResolvedRefs
name: http
supportedKinds:
- group: gateway.networking.k8s.io
kind: HTTPRoute
- group: gateway.networking.k8s.io
kind: GRPCRoute
httpRoutes:
- apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
creationTimestamp: null
name: httproute-1
namespace: default
spec:
hostnames:
- gateway.envoyproxy.io
parentRefs:
- name: gateway-1
namespace: envoy-gateway
sectionName: http
rules:
- backendRefs:
- name: service-1
port: 8080
matches:
- headers:
- name: x-org-id
type: RegularExpression
value: .*
- name: hostname
type: RegularExpression
value: .*
path:
type: PathPrefix
value: /
status:
parents:
- conditions:
- lastTransitionTime: null
message: Route is accepted
reason: Accepted
status: "True"
type: Accepted
- lastTransitionTime: null
message: Resolved all the Object references for the Route
reason: ResolvedRefs
status: "True"
type: ResolvedRefs
controllerName: gateway.envoyproxy.io/gatewayclass-controller
parentRef:
name: gateway-1
namespace: envoy-gateway
sectionName: http
- apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
creationTimestamp: null
name: httproute-2
namespace: default
spec:
hostnames:
- gateway.envoyproxy.io
parentRefs:
- name: gateway-1
namespace: envoy-gateway
sectionName: http
rules:
- backendRefs:
- name: service-1
port: 8080
matches:
- headers:
- name: x-org-id
type: Exact
value: test-org
path:
type: PathPrefix
value: /
status:
parents:
- conditions:
- lastTransitionTime: null
message: Route is accepted
reason: Accepted
status: "True"
type: Accepted
- lastTransitionTime: null
message: Resolved all the Object references for the Route
reason: ResolvedRefs
status: "True"
type: ResolvedRefs
controllerName: gateway.envoyproxy.io/gatewayclass-controller
parentRef:
name: gateway-1
namespace: envoy-gateway
sectionName: http
infraIR:
envoy-gateway/gateway-1:
proxy:
listeners:
- address: null
name: envoy-gateway/gateway-1/http
ports:
- containerPort: 10080
name: http-80
protocol: HTTP
servicePort: 80
metadata:
labels:
gateway.envoyproxy.io/owning-gateway-name: gateway-1
gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway
name: envoy-gateway/gateway-1
namespace: envoy-gateway-system
xdsIR:
envoy-gateway/gateway-1:
accessLog:
json:
- path: /dev/stdout
http:
- address: 0.0.0.0
hostnames:
- '*.envoyproxy.io'
isHTTP2: false
metadata:
kind: Gateway
name: gateway-1
namespace: envoy-gateway
sectionName: http
name: envoy-gateway/gateway-1/http
path:
escapedSlashesAction: UnescapeAndRedirect
mergeSlashes: true
port: 10080
routes:
- destination:
name: httproute/default/httproute-1/rule/0
settings:
- addressType: IP
endpoints:
- host: 7.7.7.7
port: 8080
name: httproute/default/httproute-1/rule/0/backend/0
protocol: HTTP
weight: 1
headerMatches:
- distinct: false
name: x-org-id
safeRegex: .*
- distinct: false
name: hostname
safeRegex: .*
hostname: gateway.envoyproxy.io
isHTTP2: false
metadata:
kind: HTTPRoute
name: httproute-1
namespace: default
name: httproute/default/httproute-1/rule/0/match/0/gateway_envoyproxy_io
pathMatch:
distinct: false
name: ""
prefix: /
- destination:
name: httproute/default/httproute-2/rule/0
settings:
- addressType: IP
endpoints:
- host: 7.7.7.7
port: 8080
name: httproute/default/httproute-2/rule/0/backend/0
protocol: HTTP
weight: 1
headerMatches:
- distinct: false
exact: test-org
name: x-org-id
hostname: gateway.envoyproxy.io
isHTTP2: false
metadata:
kind: HTTPRoute
name: httproute-2
namespace: default
name: httproute/default/httproute-2/rule/0/match/0/gateway_envoyproxy_io
pathMatch:
distinct: false
name: ""
prefix: /
readyListener:
address: 0.0.0.0
ipFamily: IPv4
path: /ready
port: 19003
Loading