fix(MeshGateway): tweak route precedence to match Gateway API (#6843)

* chore(deps): bump gateway-api with conformance fix * fix(MeshGateway): tweak route precedence to match Gateway API Signed-off-by: Mike Beaumont <mjboamail@gmail.com>
kumahq · May 25, 2023 · 313b48f · 313b48f
1 parent 2a87927
commit 313b48f
Show file tree

Hide file tree

Showing 5 changed files with 89 additions and 13 deletions.
diff --git a/go.mod b/go.mod
@@ -79,7 +79,7 @@ require (
 	sigs.k8s.io/controller-runtime v0.15.0
 	sigs.k8s.io/controller-tools v0.12.0
 	// When updating this also update version in: test/framework/k8s.go
-	sigs.k8s.io/gateway-api v0.0.0-20230523135628-4738c6c6e981
+	sigs.k8s.io/gateway-api v0.0.0-20230524175050-d673e16c31ac
 	sigs.k8s.io/yaml v1.3.0
 )
 

diff --git a/go.sum b/go.sum
@@ -2208,8 +2208,8 @@ sigs.k8s.io/controller-runtime v0.15.0 h1:ML+5Adt3qZnMSYxZ7gAverBLNPSMQEibtzAgp0
 sigs.k8s.io/controller-runtime v0.15.0/go.mod h1:7ngYvp1MLT+9GeZ+6lH3LOlcHkp/+tzA/fmHa4iq9kk=
 sigs.k8s.io/controller-tools v0.12.0 h1:TY6CGE6+6hzO7hhJFte65ud3cFmmZW947jajXkuDfBw=
 sigs.k8s.io/controller-tools v0.12.0/go.mod h1:rXlpTfFHZMpZA8aGq9ejArgZiieHd+fkk/fTatY8A2M=
-sigs.k8s.io/gateway-api v0.0.0-20230523135628-4738c6c6e981 h1:ZvlzHM9lf4uPB8RbpFrHV9mwHJ4r8DD1e8nq10bnlwg=
-sigs.k8s.io/gateway-api v0.0.0-20230523135628-4738c6c6e981/go.mod h1:C4D46Yu5uO6TuP9vXgUmkDlKkV2W8uJqZ3/aNuOZx5k=
+sigs.k8s.io/gateway-api v0.0.0-20230524175050-d673e16c31ac h1:Hy4uZbktIo2jh9Fafs9Ey1y4vZ0pPw0oXjvQ0zj/s3I=
+sigs.k8s.io/gateway-api v0.0.0-20230524175050-d673e16c31ac/go.mod h1:C4D46Yu5uO6TuP9vXgUmkDlKkV2W8uJqZ3/aNuOZx5k=
 sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo=
 sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd/go.mod h1:B8JuhiUyNFVKdsE8h686QcCxMaH6HrOAZj4vswFpcB0=
 sigs.k8s.io/structured-merge-diff/v4 v4.0.1/go.mod h1:bJZC9H9iH24zzfZ/41RGcq60oK1F7G282QMXDPYydCw=

diff --git a/pkg/plugins/runtime/gateway/route/sorter.go b/pkg/plugins/runtime/gateway/route/sorter.go
@@ -35,6 +35,8 @@ func isMoreSpecific(lhs *Match, rhs *Match) bool {
 		}
 	case rhs.ExactPath != "":
 		return false
+	}
+	switch {
 	case lhs.PrefixPath != "":
 		// Prefix match is more specific than regex.
 		if rhs.PrefixPath == "" {
@@ -50,7 +52,9 @@ func isMoreSpecific(lhs *Match, rhs *Match) bool {
 		}
 	case rhs.PrefixPath != "":
 		return false
-	default:
+	}
+	switch {
+	case lhs.RegexPath != "":
 		// Regex match is more specific than no path match.
 		if rhs.RegexPath == "" {
 			return true
@@ -63,20 +67,30 @@ func isMoreSpecific(lhs *Match, rhs *Match) bool {
 		if len(lhs.RegexPath) < len(rhs.RegexPath) {
 			return false
 		}
+		fallthrough
+	case rhs.RegexPath != "":
+		return false
 	}
-
-	if lhs.Method != "" && rhs.Method == "" {
-		return true
+	switch {
+	case lhs.Method != "":
+		if rhs.Method == "" {
+			return true
+		}
+	case rhs.Method != "":
+		return false
 	}
-
-	if (len(lhs.ExactHeader) + len(lhs.ExactQuery)) >
-		(len(rhs.ExactHeader) + len(rhs.ExactQuery)) {
+	switch {
+	case lhs.numHeaderMatches() > rhs.numHeaderMatches():
 		return true
+	case lhs.numHeaderMatches() < rhs.numHeaderMatches():
+		return false
 	}
-
-	if (len(lhs.RegexHeader) + len(lhs.RegexQuery)) >
-		(len(rhs.RegexHeader) + len(rhs.RegexQuery)) {
+	switch {
+	case lhs.numQueryParamMatches() > rhs.numQueryParamMatches():
 		return true
+	case lhs.numQueryParamMatches() < rhs.numQueryParamMatches():
+		return false
+	default:
 	}
 
 	// NOTE: this is a partial ordering, since we don't (yet?) order on

diff --git a/pkg/plugins/runtime/gateway/route/sorter_test.go b/pkg/plugins/runtime/gateway/route/sorter_test.go
@@ -37,4 +37,58 @@ var _ = Describe("sorting", func() {
 
 		Expect(routes).To(Equal(swappedRoutes))
 	})
+	It("sorts equal path match first", func() {
+		routes := []route.Entry{{
+			Match: route.Match{
+				PrefixPath: "/go",
+			},
+		}, {
+			Match: route.Match{
+				ExactPath: "/go",
+			},
+		}}
+
+		expectedRoutes := []route.Entry{{
+			Match: route.Match{
+				ExactPath: "/go",
+			},
+		}, {
+			Match: route.Match{
+				PrefixPath: "/go",
+			},
+		}}
+
+		sort.Sort(route.Sorter(routes))
+		Expect(routes).To(Equal(expectedRoutes))
+	})
+	It("sorts method before header", func() {
+		routes := []route.Entry{{
+			Match: route.Match{
+				Method: "PATCH",
+			},
+		}, {
+			Match: route.Match{
+				ExactHeader: []route.KeyValue{{
+					Key:   "header",
+					Value: "value",
+				}},
+			},
+		}}
+
+		expectedRoutes := []route.Entry{{
+			Match: route.Match{
+				Method: "PATCH",
+			},
+		}, {
+			Match: route.Match{
+				ExactHeader: []route.KeyValue{{
+					Key:   "header",
+					Value: "value",
+				}},
+			},
+		}}
+
+		sort.Sort(route.Sorter(routes))
+		Expect(routes).To(Equal(expectedRoutes))
+	})
 })
diff --git a/pkg/plugins/runtime/gateway/route/table.go b/pkg/plugins/runtime/gateway/route/table.go
@@ -77,6 +77,14 @@ type Match struct {
 	RegexQuery []KeyValue // param -> regex
 }
 
+func (m Match) numHeaderMatches() int {
+	return len(m.ExactHeader) + len(m.RegexHeader) + len(m.AbsentHeader) + len(m.PresentHeader)
+}
+
+func (m Match) numQueryParamMatches() int {
+	return len(m.ExactQuery) + len(m.RegexQuery)
+}
+
 // Action describes how a HTTP request should be dispatched.
 type Action struct {
 	Forward  []Destination