From e04cc279bcdd70ce31996908be47d970a898e085 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Mon, 26 Jun 2023 15:42:20 -0600 Subject: [PATCH] Prioritize method matching (#789) 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. --- internal/state/dataplane/sort.go | 18 ++++++++++++++---- internal/state/dataplane/sort_test.go | 17 +++++++++++++++++ 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/internal/state/dataplane/sort.go b/internal/state/dataplane/sort.go index 5064bb391f..9237114c38 100644 --- a/internal/state/dataplane/sort.go +++ b/internal/state/dataplane/sort.go @@ -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) diff --git a/internal/state/dataplane/sort_test.go b/internal/state/dataplane/sort_test.go index 9e06434730..42e7fa5cf3 100644 --- a/internal/state/dataplane/sort_test.go +++ b/internal/state/dataplane/sort_test.go @@ -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,