Change route sorting order to Exact > RegularExpression > PathPrefix (envoyproxy#2579)

vixns · web-flow · commit 11f56fd12538 · 2024-04-05T09:24:16.000-07:00
* Change route sorting order to Exact > RegularExpression > PathPrefix kubernetes-sigs/gateway-api#1770 kubernetes-sigs/gateway-api#1855 Signed-off-by: Stéphane Cottin <stephane.cottin@vixns.com>
diff --git a/internal/gatewayapi/sort.go b/internal/gatewayapi/sort.go
@@ -18,33 +18,33 @@ func (x XdsIRRoutes) Swap(i, j int) { x[i], x[j] = x[j], x[i] }
 func (x XdsIRRoutes) Less(i, j int) bool {
 
 	// 1. Sort based on path match type
-	// Exact > PathPrefix > RegularExpression
+	// Exact > RegularExpression > PathPrefix
 	if x[i].PathMatch != nil && x[i].PathMatch.Exact != nil {
 		if x[j].PathMatch != nil {
-			if x[j].PathMatch.Prefix != nil {
+			if x[j].PathMatch.SafeRegex != nil {
 				return false
 			}
-			if x[j].PathMatch.SafeRegex != nil {
+			if x[j].PathMatch.Prefix != nil {
 				return false
 			}
 		}
 	}
-	if x[i].PathMatch != nil && x[i].PathMatch.Prefix != nil {
+	if x[i].PathMatch != nil && x[i].PathMatch.SafeRegex != nil {
 		if x[j].PathMatch != nil {
 			if x[j].PathMatch.Exact != nil {
 				return true
 			}
-			if x[j].PathMatch.SafeRegex != nil {
+			if x[j].PathMatch.Prefix != nil {
 				return false
 			}
 		}
 	}
-	if x[i].PathMatch != nil && x[i].PathMatch.SafeRegex != nil {
+	if x[i].PathMatch != nil && x[i].PathMatch.Prefix != nil {
 		if x[j].PathMatch != nil {
 			if x[j].PathMatch.Exact != nil {
 				return true
 			}
-			if x[j].PathMatch.Prefix != nil {
+			if x[j].PathMatch.SafeRegex != nil {
 				return true
 			}
 		}
@@ -96,12 +96,12 @@ func pathMatchCount(pathMatch *ir.StringMatch) int {
 		if pathMatch.Exact != nil {
 			return len(*pathMatch.Exact)
 		}
-		if pathMatch.Prefix != nil {
-			return len(*pathMatch.Prefix)
-		}
 		if pathMatch.SafeRegex != nil {
 			return len(*pathMatch.SafeRegex)
 		}
+		if pathMatch.Prefix != nil {
+			return len(*pathMatch.Prefix)
+		}
 	}
 	return 0
 }
diff --git a/internal/gatewayapi/testdata/grpcroute-with-service-match.out.yaml b/internal/gatewayapi/testdata/grpcroute-with-service-match.out.yaml
@@ -126,11 +126,11 @@ xdsIR:
             weight: 1
         hostname: '*'
         isHTTP2: true
-        name: grpcroute/default/grpcroute-1/rule/0/match/0/*
+        name: grpcroute/default/grpcroute-1/rule/0/match/1/*
         pathMatch:
           distinct: false
           name: ""
-          prefix: /com.ExampleExact
+          safeRegex: /com.[A-Z]+/[A-Za-z_][A-Za-z_0-9]*
       - backendWeights:
           invalid: 0
           valid: 0
@@ -145,8 +145,8 @@ xdsIR:
             weight: 1
         hostname: '*'
         isHTTP2: true
-        name: grpcroute/default/grpcroute-1/rule/0/match/1/*
+        name: grpcroute/default/grpcroute-1/rule/0/match/0/*
         pathMatch:
           distinct: false
           name: ""
-          safeRegex: /com.[A-Z]+/[A-Za-z_][A-Za-z_0-9]*
+          prefix: /com.ExampleExact

Original file line number	Diff line number	Diff line change
`@@ -18,33 +18,33 @@ func (x XdsIRRoutes) Swap(i, j int) { x[i], x[j] = x[j], x[i] }`
`18`	`18`	`func (x XdsIRRoutes) Less(i, j int) bool {`
`19`	`19`
`20`	`20`	`// 1. Sort based on path match type`
`21`		`- // Exact > PathPrefix > RegularExpression`
	`21`	`+ // Exact > RegularExpression > PathPrefix`
`22`	`22`	`if x[i].PathMatch != nil && x[i].PathMatch.Exact != nil {`
`23`	`23`	`if x[j].PathMatch != nil {`
`24`		`- if x[j].PathMatch.Prefix != nil {`
	`24`	`+ if x[j].PathMatch.SafeRegex != nil {`
`25`	`25`	`return false`
`26`	`26`	`}`
`27`		`- if x[j].PathMatch.SafeRegex != nil {`
	`27`	`+ if x[j].PathMatch.Prefix != nil {`
`28`	`28`	`return false`
`29`	`29`	`}`
`30`	`30`	`}`
`31`	`31`	`}`
`32`		`- if x[i].PathMatch != nil && x[i].PathMatch.Prefix != nil {`
	`32`	`+ if x[i].PathMatch != nil && x[i].PathMatch.SafeRegex != nil {`
`33`	`33`	`if x[j].PathMatch != nil {`
`34`	`34`	`if x[j].PathMatch.Exact != nil {`
`35`	`35`	`return true`
`36`	`36`	`}`
`37`		`- if x[j].PathMatch.SafeRegex != nil {`
	`37`	`+ if x[j].PathMatch.Prefix != nil {`
`38`	`38`	`return false`
`39`	`39`	`}`
`40`	`40`	`}`
`41`	`41`	`}`
`42`		`- if x[i].PathMatch != nil && x[i].PathMatch.SafeRegex != nil {`
	`42`	`+ if x[i].PathMatch != nil && x[i].PathMatch.Prefix != nil {`
`43`	`43`	`if x[j].PathMatch != nil {`
`44`	`44`	`if x[j].PathMatch.Exact != nil {`
`45`	`45`	`return true`
`46`	`46`	`}`
`47`		`- if x[j].PathMatch.Prefix != nil {`
	`47`	`+ if x[j].PathMatch.SafeRegex != nil {`
`48`	`48`	`return true`
`49`	`49`	`}`
`50`	`50`	`}`
`@@ -96,12 +96,12 @@ func pathMatchCount(pathMatch *ir.StringMatch) int {`
`96`	`96`	`if pathMatch.Exact != nil {`
`97`	`97`	`return len(*pathMatch.Exact)`
`98`	`98`	`}`
`99`		`- if pathMatch.Prefix != nil {`
`100`		`- return len(*pathMatch.Prefix)`
`101`		`- }`
`102`	`99`	`if pathMatch.SafeRegex != nil {`
`103`	`100`	`return len(*pathMatch.SafeRegex)`
`104`	`101`	`}`
	`102`	`+ if pathMatch.Prefix != nil {`
	`103`	`+ return len(*pathMatch.Prefix)`
	`104`	`+ }`
`105`	`105`	`}`
`106`	`106`	`return 0`
`107`	`107`	`}`