gateway: Make Header matching deterministic

[upstream commit 152b439]

This commit is to make sure that header matching rules are sorted in a
deterministic way, so that the behavior is predictable in envoy. The
main changes are as per below:

- Process http routes based on order from spec instead of random order
  from map.
- Use sort.Stable to reverse the original order of equal elements.
- Make sure that less(i, j) and less(j, i) return false if i-th and j-th
  elements are equal.

Kindly note that envoy will just iteratively check rule one by one, if a
match is found, subsequent rule will not be considered.

Fixes: cilium#23999
Signed-off-by: Tam Mach <[email protected]>

.github/workflows/conformance-gateway-api.yaml

@@ -203,7 +203,7 @@ jobs:
           EOF
 
           GATEWAY_API_CONFORMANCE_TESTS=1 go test -v ./operator/pkg/gateway-api --gateway-class cilium --debug -test.run "TestConformance" \
-            -test.run '!TestConformance/HTTPRouteListenerHostnameMatching' # Enable once #24217 is fixed
+            -test.run "TestConformance"
 
       - name: Post-test information gathering
         if: ${{ !success() }}

operator/pkg/model/translation/envoy_virtual_host.go

@@ -90,12 +90,17 @@ func (s SortableRoute) Less(i, j int) bool {
 	queryMatch2 := len(s[j].Match.GetQueryParameters())
 	if queryMatch1 > queryMatch2 {
 		return true
+	} else if queryMatch1 < queryMatch2 {
+		return false
 	}
 
 	// Make sure the longest header match always comes first
 	headerMatch1 := len(s[i].Match.GetHeaders())
 	headerMatch2 := len(s[j].Match.GetHeaders())
-	return headerMatch1 > headerMatch2
+	if headerMatch1 > headerMatch2 {
+		return true
+	}
+	return false
 }
 
 func (s SortableRoute) Swap(i, j int) {
@@ -115,23 +120,18 @@ func NewVirtualHostWithDefaults(hostnames []string, httpsRedirect bool, hostName
 
 // NewVirtualHost creates a new VirtualHost with the given host and routes.
 func NewVirtualHost(hostnames []string, httpsRedirect bool, hostNameSuffixMatch bool, httpRoutes []model.HTTPRoute, mutators ...VirtualHostMutator) (*envoy_config_route_v3.VirtualHost, error) {
-	matchBackendMap := make(map[string][]model.HTTPRoute)
-	for _, r := range httpRoutes {
-		matchBackendMap[r.GetMatchKey()] = append(matchBackendMap[r.GetMatchKey()], r)
-	}
-
 	var routes SortableRoute
 	if httpsRedirect {
-		routes = envoyHTTPSRoutes(matchBackendMap, hostnames, hostNameSuffixMatch)
+		routes = envoyHTTPSRoutes(httpRoutes, hostnames, hostNameSuffixMatch)
 	} else {
-		routes = envoyHTTPRoutes(matchBackendMap, hostnames, hostNameSuffixMatch)
+		routes = envoyHTTPRoutes(httpRoutes, hostnames, hostNameSuffixMatch)
 	}
 
 	// This is to make sure that the Exact match is always having higher priority.
 	// Each route entry in the virtual host is checked, in order. If there is a
 	// match, the route is used and no further route checks are made.
 	// Related docs https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_conn_man/route_matching
-	sort.Sort(routes)
+	sort.Stable(routes)
 
 	var domains []string
 	for _, host := range hostnames {
@@ -159,9 +159,19 @@ func NewVirtualHost(hostnames []string, httpsRedirect bool, hostNameSuffixMatch
 	return res, nil
 }
 
-func envoyHTTPSRoutes(matchBackendMap map[string][]model.HTTPRoute, hostnames []string, hostNameSuffixMatch bool) []*envoy_config_route_v3.Route {
+func envoyHTTPSRoutes(httpRoutes []model.HTTPRoute, hostnames []string, hostNameSuffixMatch bool) []*envoy_config_route_v3.Route {
+	matchBackendMap := make(map[string][]model.HTTPRoute)
+	for _, r := range httpRoutes {
+		matchBackendMap[r.GetMatchKey()] = append(matchBackendMap[r.GetMatchKey()], r)
+	}
+
 	routes := make([]*envoy_config_route_v3.Route, 0, len(matchBackendMap))
-	for _, hRoutes := range matchBackendMap {
+	for _, r := range httpRoutes {
+		hRoutes, exists := matchBackendMap[r.GetMatchKey()]
+		// if not exists, it means this route is already added to the routes
+		if !exists {
+			continue
+		}
 		rRedirect := &envoy_config_route_v3.Route_Redirect{
 			Redirect: &envoy_config_route_v3.RedirectAction{
 				SchemeRewriteSpecifier: &envoy_config_route_v3.RedirectAction_HttpsRedirect{
@@ -179,13 +189,23 @@ func envoyHTTPSRoutes(matchBackendMap map[string][]model.HTTPRoute, hostnames []
 			Action: rRedirect,
 		}
 		routes = append(routes, &route)
+		delete(matchBackendMap, r.GetMatchKey())
 	}
 	return routes
 }
 
-func envoyHTTPRoutes(matchBackendMap map[string][]model.HTTPRoute, hostnames []string, hostNameSuffixMatch bool) []*envoy_config_route_v3.Route {
+func envoyHTTPRoutes(httpRoutes []model.HTTPRoute, hostnames []string, hostNameSuffixMatch bool) []*envoy_config_route_v3.Route {
+	matchBackendMap := make(map[string][]model.HTTPRoute)
+	for _, r := range httpRoutes {
+		matchBackendMap[r.GetMatchKey()] = append(matchBackendMap[r.GetMatchKey()], r)
+	}
+
 	routes := make([]*envoy_config_route_v3.Route, 0, len(matchBackendMap))
-	for _, hRoutes := range matchBackendMap {
+	for _, r := range httpRoutes {
+		hRoutes, exists := matchBackendMap[r.GetMatchKey()]
+		if !exists {
+			continue
+		}
 		var backends []model.Backend
 		for _, r := range hRoutes {
 			backends = append(backends, r.Backends...)
@@ -242,6 +262,7 @@ func envoyHTTPRoutes(matchBackendMap map[string][]model.HTTPRoute, hostnames []s
 			RequestHeadersToRemove: getRequestHeadersToRemove(hRoutes[0]),
 		}
 		routes = append(routes, &route)
+		delete(matchBackendMap, r.GetMatchKey())
 	}
 	return routes
 }