Skip to content

Commit

Permalink
Gateway API: Fix for Listener/Route hostname isolation
Browse files Browse the repository at this point in the history 
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 <sunjayb@vmware.com>
  • Loading branch information
@sunjayBhatia
sunjayBhatia committed May 17, 2024
1 parent 7530c06 commit 0ce96b4
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 14 deletions.
45 changes: 36 additions & 9 deletions internal/dag/gatewayapi_processor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,14 @@ func (p *GatewayAPIProcessor) processRoute(
p.resolveRouteRefs(route, routeParentStatus)
}

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
Expand All @@ -244,7 +252,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.
Expand Down Expand Up @@ -313,7 +321,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,
Expand All @@ -331,7 +339,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

Expand All @@ -356,7 +364,7 @@ func (p *GatewayAPIProcessor) getListenersForRouteParentRef(
attachedRoutes[string(selectedListener.listener.Name)]++

if selectedListener.ready {
allowedListeners = append(allowedListeners, selectedListener)
allowedListeners[string(selectedListener.listener.Name)] = selectedListener
}

}
Expand Down Expand Up @@ -836,7 +844,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.
Expand All @@ -854,6 +862,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])

Expand All @@ -864,17 +887,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)
}

Expand Down
80 changes: 75 additions & 5 deletions internal/dag/gatewayapi_processor_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: "",
Expand Down Expand Up @@ -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 {
Expand All @@ -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)
})
Expand Down

0 comments on commit 0ce96b4

Please sign in to comment.