Gateway API: Fix for Listener/Route hostname isolation (projectcontou…

…r#6162) Requests should be "isolated" to the most specific Listener and it's attached routes. This means our existing logic on finding intersecting route and Listener hostnames needs an update to factor in the other Listeners on a Gateway that the route in question may not actually be attached to. Fix for conformance test: kubernetes-sigs/gateway-api#2669 kubernetes-sigs/gateway-api#2465 for spec Signed-off-by: Sunjay Bhatia <[email protected]> Signed-off-by: Saman Mahdanian <[email protected]>
SamMHD · Sep 8, 2024 · be4777e · be4777e
1 parent f90fe48
commit be4777e
Show file tree

Hide file tree

Showing 4 changed files with 125 additions and 23 deletions.
diff --git a/changelogs/unreleased/6162-sunjayBhatia-minor.md b/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.
diff --git a/internal/dag/gatewayapi_processor.go b/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)
 			}
 

diff --git a/internal/dag/gatewayapi_processor_test.go b/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)

diff --git a/test/conformance/gatewayapi/gateway_conformance_test.go b/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,