Skip to content
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.

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

Prioritize method matching #789

Merged
merged 2 commits into from
Jun 26, 2023
Merged
Changes from 1 commit
Commits
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
Next Next commit
Prioritize method matching
Problem: A recent clarification in the gateway API stated that methods are prioritized higher than headers when matching. We were not abiding by this order.

Solution: Check for methods to prioritize rules before we check headers.
sjberman committed Jun 26, 2023

Unverified

This user has not yet uploaded their public signing key.
commit d67a16fa2c06bf6bfabfe1d91489711a8cded166
18 changes: 14 additions & 4 deletions internal/state/dataplane/sort.go
Original file line number Diff line number Diff line change
@@ -24,6 +24,7 @@ Precedence must be given to the Rule with the largest number of (Continuing on t
- Characters in a matching non-wildcard hostname.
- Characters in a matching hostname.
- Characters in a matching path.
- Method match.
- Header matches.
- Query param matches.
@@ -43,16 +44,25 @@ func higherPriority(rule1, rule2 MatchRule) bool {
match1 := rule1.GetMatch()
match2 := rule2.GetMatch()

// If both matches exists then compare the number of header matches
// The match with the largest number of header matches wins
// Compare if a method exists on one of the matches but not the other.
// The match with the method specified wins.
if match1.Method != nil && match2.Method == nil {
return true
}
if match2.Method != nil && match1.Method == nil {
return false
}

// Compare the number of header matches.
// The match with the largest number of header matches wins.
l1 := len(match1.Headers)
l2 := len(match2.Headers)

if l1 != l2 {
return l1 > l2
}
// If the number of headers is equal then compare the number of query param matches
// The match with the most query param matches wins
// If the number of headers is equal then compare the number of query param matches.
// The match with the most query param matches wins.
l1 = len(match1.QueryParams)
l2 = len(match2.QueryParams)

17 changes: 17 additions & 0 deletions internal/state/dataplane/sort_test.go
Original file line number Diff line number Diff line change
@@ -77,6 +77,12 @@ func TestSort(t *testing.T) {
},
},
}
methodMatch := v1beta1.HTTPRouteMatch{
Path: &v1beta1.HTTPPathMatch{
Value: helpers.GetStringPointer("/path"),
},
Method: helpers.GetPointer(v1beta1.HTTPMethodPost),
}

hr1 := v1beta1.HTTPRoute{
ObjectMeta: metav1.ObjectMeta{
@@ -96,6 +102,7 @@ func TestSort(t *testing.T) {
Matches: []v1beta1.HTTPRouteMatch{
twoHeaderOneParamMatch, // tie decided on params
threeHeaderMatch, // tie decided on headers
methodMatch, // tie decided on method
},
},
},
@@ -153,6 +160,11 @@ func TestSort(t *testing.T) {
RuleIdx: 2,
Source: &hr1,
},
{
MatchIdx: 2, // methodMatch
RuleIdx: 2,
Source: &hr1,
},
{
MatchIdx: 0, // twoHeaderMatch / later timestamp / test/hr2
RuleIdx: 0,
@@ -166,6 +178,11 @@ func TestSort(t *testing.T) {
}

sortedRoutes := []MatchRule{
{
MatchIdx: 2, // methodMatch
RuleIdx: 2,
Source: &hr1,
},
{
MatchIdx: 1, // threeHeaderMatch
RuleIdx: 2,