Skip to content

Commit

Permalink
Prioritize method matching (#789)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sjberman authored Jun 26, 2023
1 parent 2db8d68 commit e04cc27
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 4 deletions.
18 changes: 14 additions & 4 deletions internal/state/dataplane/sort.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)

Expand Down
17 changes: 17 additions & 0 deletions internal/state/dataplane/sort_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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
},
},
},
Expand Down Expand Up @@ -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,
Expand All @@ -166,6 +178,11 @@ func TestSort(t *testing.T) {
}

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

0 comments on commit e04cc27

Please sign in to comment.