Gateway API: Fix for Listener/Route hostname isolation

changelogs/unreleased/6162-sunjayBhatia-minor.md

@@ -0,0 +1,5 @@
+## Gateway API: Implement Listener/Route hostname isolation
+
+Gateway API spec update in this [GEP](https://github.com/kubernetes-sigs/gateway-api/pull/2465).
+Updates logic on finding intersecting route and Listener hostnames to factor in the other Listeners on a Gateway that the route in question may not actually be attached to.
+Requests should be "isolated" to the most specific Listener and it's attached routes.

internal/dag/gatewayapi_processor.go

@@ -221,6 +221,16 @@ func (p *GatewayAPIProcessor) processRoute(
 			p.resolveRouteRefs(route, routeParentStatus)
 		}
 
+		// Collect other Listeners with configured hostnames so we can
+		// calculate Listener/Route hostname intersection properly.
+		otherListenerHostnames := []string{}
+		for _, listener := range listeners {
+			name := string(listener.listener.Name)
+			if _, ok := allowedListeners[name]; !ok && listener.listener.Hostname != nil && len(*listener.listener.Hostname) > 0 {
+				otherListenerHostnames = append(otherListenerHostnames, string(*listener.listener.Hostname))
+			}
+		}
+
 		// Keep track of the number of intersecting hosts
 		// between the route and all allowed listeners for
 		// this parent ref so that we can set the appropriate
@@ -244,7 +254,7 @@ func (p *GatewayAPIProcessor) processRoute(
 					routeHostnames = route.Spec.Hostnames
 				}
 
-				hosts, errs = p.computeHosts(routeHostnames, string(ptr.Deref(listener.listener.Hostname, "")))
+				hosts, errs = p.computeHosts(routeHostnames, string(ptr.Deref(listener.listener.Hostname, "")), otherListenerHostnames)
 				for _, err := range errs {
 					// The Gateway API spec does not indicate what to do if syntactically
 					// invalid hostnames make it through, we're using our best judgment here.
@@ -313,7 +323,7 @@ func (p *GatewayAPIProcessor) getListenersForRouteParentRef(
 	listeners []*listenerInfo,
 	attachedRoutes map[string]int,
 	routeParentStatusAccessor *status.RouteParentStatusUpdate,
-) []*listenerInfo {
+) map[string]*listenerInfo {
 	// Find the set of valid listeners that are relevant given this
 	// parent ref (either all of them, if the ref is to the entire
 	// gateway, or one of them, if the ref is to a specific listener,
@@ -331,7 +341,7 @@ func (p *GatewayAPIProcessor) getListenersForRouteParentRef(
 
 	// Now find the subset of those listeners that allow this route
 	// to select them, based on route kind and namespace.
-	var allowedListeners []*listenerInfo
+	allowedListeners := map[string]*listenerInfo{}
 
 	readyListenerCount := 0
 
@@ -356,7 +366,7 @@ func (p *GatewayAPIProcessor) getListenersForRouteParentRef(
 		attachedRoutes[string(selectedListener.listener.Name)]++
 
 		if selectedListener.ready {
-			allowedListeners = append(allowedListeners, selectedListener)
+			allowedListeners[string(selectedListener.listener.Name)] = selectedListener
 		}
 
 	}
@@ -836,7 +846,7 @@ func isSecretRef(certificateRef gatewayapi_v1.SecretObjectReference) bool {
 //     invalid and some condition should be added to the route. This shouldn't be
 //     possible because of kubebuilder+admission webhook validation but we're being
 //     defensive here.
-func (p *GatewayAPIProcessor) computeHosts(routeHostnames []gatewayapi_v1.Hostname, listenerHostname string) (sets.Set[string], []error) {
+func (p *GatewayAPIProcessor) computeHosts(routeHostnames []gatewayapi_v1.Hostname, listenerHostname string, otherListenerHosts []string) (sets.Set[string], []error) {
 	// The listener hostname is assumed to be valid because it's been run
 	// through the `gatewayapi.ValidateListeners` logic, so we don't need
 	// to validate it here.
@@ -854,6 +864,21 @@ func (p *GatewayAPIProcessor) computeHosts(routeHostnames []gatewayapi_v1.Hostna
 	hostnames := sets.New[string]()
 	var errs []error
 
+	otherListenerIntersection := func(routeHostname, actualListenerHostname string) bool {
+		for _, listenerHostname := range otherListenerHosts {
+			if routeHostname == listenerHostname {
+				return true
+			}
+			if strings.HasPrefix(listenerHostname, "*") &&
+				hostnameMatchesWildcardHostname(routeHostname, listenerHostname) &&
+				len(listenerHostname) > len(actualListenerHostname) {
+				return true
+			}
+		}
+
+		return false
+	}
+
 	for i := range routeHostnames {
 		routeHostname := string(routeHostnames[i])
 
@@ -864,17 +889,21 @@ func (p *GatewayAPIProcessor) computeHosts(routeHostnames []gatewayapi_v1.Hostna
 		}
 
 		switch {
-		// No listener hostname: use the route hostname.
+		// No listener hostname: use the route hostname if
+		// it does not also intersect with another Listener.
 		case len(listenerHostname) == 0:
-			hostnames.Insert(routeHostname)
+			if !otherListenerIntersection(routeHostname, listenerHostname) {
+				hostnames.Insert(routeHostname)
+			}
 
 		// Listener hostname matches the route hostname: use it.
 		case listenerHostname == routeHostname:
 			hostnames.Insert(routeHostname)
 
-		// Listener has a wildcard hostname: check if the route hostname matches.
+		// Listener has a wildcard hostname: check if the route hostname matches
+		// but do not use it if it intersects with a more specific other Listener.
 		case strings.HasPrefix(listenerHostname, "*"):
-			if hostnameMatchesWildcardHostname(routeHostname, listenerHostname) {
+			if hostnameMatchesWildcardHostname(routeHostname, listenerHostname) && !otherListenerIntersection(routeHostname, listenerHostname) {
 				hostnames.Insert(routeHostname)
 			}
 

internal/dag/gatewayapi_processor_test.go

@@ -35,10 +35,11 @@ import (
 
 func TestComputeHosts(t *testing.T) {
 	tests := map[string]struct {
-		listenerHost string
-		hostnames    []gatewayapi_v1.Hostname
-		want         sets.Set[string]
-		wantError    []error
+		listenerHost       string
+		otherListenerHosts []string
+		hostnames          []gatewayapi_v1.Hostname
+		want               sets.Set[string]
+		wantError          []error
 	}{
 		"single host": {
 			listenerHost: "",
@@ -230,6 +231,75 @@ func TestComputeHosts(t *testing.T) {
 			want:      nil,
 			wantError: nil,
 		},
+		"empty listener host with other listener specific hosts that match route hostnames": {
+			listenerHost: "",
+			otherListenerHosts: []string{
+				"foo",
+			},
+			hostnames: []gatewayapi_v1.Hostname{
+				"foo",
+				"bar",
+				"*.foo",
+			},
+			want:      sets.New("bar", "*.foo"),
+			wantError: nil,
+		},
+		"empty listener host with other listener wildcard hosts that match route hostnames": {
+			listenerHost: "",
+			otherListenerHosts: []string{
+				"*.foo",
+			},
+			hostnames: []gatewayapi_v1.Hostname{
+				"a.bar",
+				"foo",
+				"a.foo",
+				"*.foo",
+			},
+			want:      sets.New("a.bar", "foo"),
+			wantError: nil,
+		},
+		"wildcard listener host with other listener specific hosts that match route hostnames": {
+			listenerHost: "*.foo",
+			otherListenerHosts: []string{
+				"a.foo",
+			},
+			hostnames: []gatewayapi_v1.Hostname{
+				"a.foo",
+				"c.b.foo",
+				"*.foo",
+			},
+			want:      sets.New("c.b.foo", "*.foo"),
+			wantError: nil,
+		},
+		"wildcard listener host with other listener more specific wildcard hosts that match route hostnames": {
+			listenerHost: "*.foo",
+			otherListenerHosts: []string{
+				"*.a.foo",
+			},
+			hostnames: []gatewayapi_v1.Hostname{
+				"a.foo",
+				"b.a.foo",
+				"d.c.foo",
+				"*.foo",
+				"*.b.a.foo",
+			},
+			want:      sets.New("a.foo", "d.c.foo", "*.foo"),
+			wantError: nil,
+		},
+		"wildcard listener host with other listener less specific wildcard hosts that match route hostnames": {
+			listenerHost: "*.a.foo",
+			otherListenerHosts: []string{
+				"*.foo",
+			},
+			hostnames: []gatewayapi_v1.Hostname{
+				"a.foo",
+				"b.a.foo",
+				"d.c.foo",
+				"*.a.foo",
+			},
+			want:      sets.New("b.a.foo", "*.a.foo"),
+			wantError: nil,
+		},
 	}
 
 	for name, tc := range tests {
@@ -238,7 +308,7 @@ func TestComputeHosts(t *testing.T) {
 				FieldLogger: fixture.NewTestLogger(t),
 			}
 
-			got, gotError := processor.computeHosts(tc.hostnames, tc.listenerHost)
+			got, gotError := processor.computeHosts(tc.hostnames, tc.listenerHost, tc.otherListenerHosts)
 			assert.Equal(t, tc.want, got)
 			assert.Equal(t, tc.wantError, gotError)
 		})
@@ -761,9 +831,13 @@ func TestGetListenersForRouteParentRef(t *testing.T) {
 				map[string]int{},
 				rpsu)
 
-			var want []*listenerInfo
-			for _, i := range tc.want {
-				want = append(want, tc.listeners[i])
+			var want map[string]*listenerInfo
+			if len(tc.want) > 0 {
+				want = map[string]*listenerInfo{}
+				for _, i := range tc.want {
+					listener := tc.listeners[i]
+					want[string(listener.listener.Name)] = listener
+				}
 			}
 
 			assert.Equal(t, want, got)

test/conformance/gatewayapi/gateway_conformance_test.go

@@ -97,12 +97,6 @@ func TestGatewayConformance(t *testing.T) {
 			// sends a request for an unknown (to Contour/Envoy) host which fails
 			// instead of returning a 404.
 			tests.HTTPRouteHTTPSListener.ShortName,
-
-			// The cases in this test that fail involve Listener/HTTPRoute hostname
-			// intersection and ensuring requests are only routed to the most
-			// specific Listeners/Routes that match them.
-			// See: https://github.com/projectcontour/contour/pull/6162
-			tests.GatewayHTTPListenerIsolation.ShortName,
 		},
 		ExemptFeatures: sets.New(
 			features.SupportMesh,