From fa0d942318797528e655b0648ea15c76396b8951 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Wed, 17 May 2023 14:20:10 -0600 Subject: [PATCH 1/3] Allow empty HTTPRoute hostnames If an HTTPRoute didn't supply any hostnames, then we wouldn't attach it to any listeners. This fixes that issue and uses the wildcard hostname if a listener doesn't supply a hostname, otherwise just use the hostnames of the listeners. --- internal/state/dataplane/configuration.go | 25 +++++-- .../state/dataplane/configuration_test.go | 70 +++++++++++++++++-- internal/state/graph/httproute.go | 9 +++ internal/state/graph/httproute_test.go | 12 ++++ 4 files changed, 105 insertions(+), 11 deletions(-) diff --git a/internal/state/dataplane/configuration.go b/internal/state/dataplane/configuration.go index 02b17b9a85..a140c31754 100644 --- a/internal/state/dataplane/configuration.go +++ b/internal/state/dataplane/configuration.go @@ -266,13 +266,7 @@ func (hpr *hostPathRules) upsertListener(l *graph.Listener) { } for routeNsName, r := range l.Routes { - var hostnames []string - - for _, h := range r.Source.Spec.Hostnames { - if _, exist := l.AcceptedHostnames[string(h)]; exist { - hostnames = append(hostnames, string(h)) - } - } + hostnames := getHostnames(r, l) for _, h := range hostnames { if prevListener, exists := hpr.listenersForHost[h]; exists { @@ -333,6 +327,23 @@ func (hpr *hostPathRules) upsertListener(l *graph.Listener) { } } +func getHostnames(r *graph.Route, l *graph.Listener) []string { + var hostnames []string + for _, h := range r.Source.Spec.Hostnames { + if _, exist := l.AcceptedHostnames[string(h)]; exist { + hostnames = append(hostnames, string(h)) + } + } + + if len(r.Source.Spec.Hostnames) == 0 { + for hostname := range l.AcceptedHostnames { + hostnames = append(hostnames, hostname) + } + } + + return hostnames +} + func (hpr *hostPathRules) buildServers() []VirtualServer { servers := make([]VirtualServer, 0, len(hpr.rulesPerHost)+len(hpr.httpsListeners)) diff --git a/internal/state/dataplane/configuration_test.go b/internal/state/dataplane/configuration_test.go index 0e5029666d..c1b0651037 100644 --- a/internal/state/dataplane/configuration_test.go +++ b/internal/state/dataplane/configuration_test.go @@ -217,13 +217,21 @@ func TestBuildConfiguration(t *testing.T) { pathAndType{path: "/valid", pathType: prefix}, pathAndType{path: invalidMatchesPath, pathType: prefix}, ) - hr7, hr7Groups, routeHR7 := createTestResources( + hr7, expHR7Groups, routeHR7 := createTestResources( "hr-7", "foo.example.com", "listener-80-1", pathAndType{path: "/valid", pathType: prefix}, pathAndType{path: "/valid", pathType: v1beta1.PathMatchExact}, ) + hrNoHost, expHRNoHostGroups, routeNoHost := createTestResources( + "route-no-host", + "", + "listener-80-1", + pathAndType{path: "/", pathType: prefix}, + ) + routeNoHost.Source.Spec.Hostnames = []v1beta1.Hostname{} + httpsHR1, expHTTPSHR1Groups, httpsRouteHR1 := createTestResources( "https-hr-1", "foo.example.com", @@ -516,6 +524,60 @@ func TestBuildConfiguration(t *testing.T) { }, msg: "one http listener with two routes for different hostnames", }, + { + graph: &graph.Graph{ + GatewayClass: &graph.GatewayClass{ + Source: &v1beta1.GatewayClass{}, + Valid: true, + }, + Gateway: &graph.Gateway{ + Source: &v1beta1.Gateway{}, + Listeners: map[string]*graph.Listener{ + "listener-80-1": { + Source: listener80, + Valid: true, + Routes: map[types.NamespacedName]*graph.Route{ + {Namespace: "test", Name: "route-no-host"}: routeNoHost, + }, + AcceptedHostnames: map[string]struct{}{ + wildcardHostname: {}, + }, + }, + }, + }, + Routes: map[types.NamespacedName]*graph.Route{ + {Namespace: "test", Name: "route-no-host"}: routeNoHost, + }, + }, + expConf: Configuration{ + HTTPServers: []VirtualServer{ + { + IsDefault: true, + }, + { + Hostname: wildcardHostname, + PathRules: []PathRule{ + { + Path: "/", + PathType: PathTypePrefix, + MatchRules: []MatchRule{ + { + MatchIdx: 0, + RuleIdx: 0, + BackendGroup: expHRNoHostGroups[0], + Source: hrNoHost, + }, + }, + }, + }, + }, + }, + SSLServers: []VirtualServer{}, + Upstreams: []Upstream{fooUpstream}, + BackendGroups: []BackendGroup{expHRNoHostGroups[0]}, + }, + msg: "one listener and route without hostnames", + }, { graph: &graph.Graph{ GatewayClass: &graph.GatewayClass{ @@ -1074,7 +1136,7 @@ func TestBuildConfiguration(t *testing.T) { { MatchIdx: 0, RuleIdx: 1, - BackendGroup: hr7Groups[1], + BackendGroup: expHR7Groups[1], Source: hr7, }, }, @@ -1086,7 +1148,7 @@ func TestBuildConfiguration(t *testing.T) { { MatchIdx: 0, RuleIdx: 0, - BackendGroup: hr7Groups[0], + BackendGroup: expHR7Groups[0], Source: hr7, }, }, @@ -1096,7 +1158,7 @@ func TestBuildConfiguration(t *testing.T) { }, SSLServers: []VirtualServer{}, Upstreams: []Upstream{fooUpstream}, - BackendGroups: []BackendGroup{hr7Groups[0], hr7Groups[1]}, + BackendGroups: []BackendGroup{expHR7Groups[0], expHR7Groups[1]}, }, msg: "duplicate paths with different types", }, diff --git a/internal/state/graph/httproute.go b/internal/state/graph/httproute.go index 02990edb02..3cdebeb223 100644 --- a/internal/state/graph/httproute.go +++ b/internal/state/graph/httproute.go @@ -13,6 +13,8 @@ import ( "github.com/nginxinc/nginx-kubernetes-gateway/internal/state/validation" ) +const wildcardHostname = "~^" + // Rule represents a rule of an HTTPRoute. type Rule struct { // BackendRefs is a list of BackendRefs for the rule. @@ -380,6 +382,13 @@ func findValidListeners(sectionName string, listeners map[string]*Listener) ([]* func findAcceptedHostnames(listenerHostname *v1beta1.Hostname, routeHostnames []v1beta1.Hostname) []string { hostname := getHostname(listenerHostname) + if len(routeHostnames) == 0 { + if hostname == "" { + return []string{wildcardHostname} + } + return []string{hostname} + } + match := func(h v1beta1.Hostname) bool { if hostname == "" { return true diff --git a/internal/state/graph/httproute_test.go b/internal/state/graph/httproute_test.go index 80f96f4cef..903911c9aa 100644 --- a/internal/state/graph/httproute_test.go +++ b/internal/state/graph/httproute_test.go @@ -1008,6 +1008,18 @@ func TestFindAcceptedHostnames(t *testing.T) { expected: []string{"foo.example.com", "bar.example.com"}, msg: "nil listener hostname", }, + { + listenerHostname: &listenerHostnameFoo, + routeHostnames: nil, + expected: []string{"foo.example.com"}, + msg: "route has empty hostnames", + }, + { + listenerHostname: nil, + routeHostnames: nil, + expected: []string{wildcardHostname}, + msg: "both listener and route have empty hostnames", + }, } for _, test := range tests { From bd8fb9345a8ac847cff2032e9bf6037dce001585 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Thu, 18 May 2023 10:49:40 -0600 Subject: [PATCH 2/3] Refactor accepted hostnames to parent ref attachment --- internal/state/dataplane/configuration.go | 24 +-- .../state/dataplane/configuration_test.go | 146 ++++-------------- internal/state/graph/gateway_listener.go | 10 +- internal/state/graph/gateway_test.go | 95 +++++------- internal/state/graph/graph_test.go | 12 +- internal/state/graph/httproute.go | 18 ++- internal/state/graph/httproute_test.go | 54 ++++--- 7 files changed, 124 insertions(+), 235 deletions(-) diff --git a/internal/state/dataplane/configuration.go b/internal/state/dataplane/configuration.go index a140c31754..1eb4342ab2 100644 --- a/internal/state/dataplane/configuration.go +++ b/internal/state/dataplane/configuration.go @@ -266,7 +266,12 @@ func (hpr *hostPathRules) upsertListener(l *graph.Listener) { } for routeNsName, r := range l.Routes { - hostnames := getHostnames(r, l) + var hostnames []string + for _, p := range r.ParentRefs { + if val, exist := p.Attachment.AcceptedHostnames[string(l.Source.Name)]; exist { + hostnames = val + } + } for _, h := range hostnames { if prevListener, exists := hpr.listenersForHost[h]; exists { @@ -327,23 +332,6 @@ func (hpr *hostPathRules) upsertListener(l *graph.Listener) { } } -func getHostnames(r *graph.Route, l *graph.Listener) []string { - var hostnames []string - for _, h := range r.Source.Spec.Hostnames { - if _, exist := l.AcceptedHostnames[string(h)]; exist { - hostnames = append(hostnames, string(h)) - } - } - - if len(r.Source.Spec.Hostnames) == 0 { - for hostname := range l.AcceptedHostnames { - hostnames = append(hostnames, hostname) - } - } - - return hostnames -} - func (hpr *hostPathRules) buildServers() []VirtualServer { servers := make([]VirtualServer, 0, len(hpr.rulesPerHost)+len(hpr.httpsListeners)) diff --git a/internal/state/dataplane/configuration_test.go b/internal/state/dataplane/configuration_test.go index c1b0651037..6fefb69206 100644 --- a/internal/state/dataplane/configuration_test.go +++ b/internal/state/dataplane/configuration_test.go @@ -130,11 +130,25 @@ func TestBuildConfiguration(t *testing.T) { createInternalRoute := func( source *v1beta1.HTTPRoute, + listenerName string, paths []pathAndType, ) *graph.Route { + hostnames := make([]string, 0, len(source.Spec.Hostnames)) + for _, h := range source.Spec.Hostnames { + hostnames = append(hostnames, string(h)) + } r := &graph.Route{ Source: source, Rules: createRules(source, paths), + ParentRefs: []graph.ParentRef{ + { + Attachment: &graph.ParentRefAttachmentStatus{ + AcceptedHostnames: map[string][]string{ + listenerName: hostnames, + }, + }, + }, + }, } return r } @@ -162,7 +176,7 @@ func TestBuildConfiguration(t *testing.T) { *v1beta1.HTTPRoute, []BackendGroup, *graph.Route, ) { hr := createRoute(name, hostname, listenerName, paths...) - route := createInternalRoute(hr, paths) + route := createInternalRoute(hr, listenerName, paths) groups := createExpBackendGroupsForRoute(route) return hr, groups, route } @@ -224,14 +238,6 @@ func TestBuildConfiguration(t *testing.T) { pathAndType{path: "/valid", pathType: prefix}, pathAndType{path: "/valid", pathType: v1beta1.PathMatchExact}, ) - hrNoHost, expHRNoHostGroups, routeNoHost := createTestResources( - "route-no-host", - "", - "listener-80-1", - pathAndType{path: "/", pathType: prefix}, - ) - routeNoHost.Source.Spec.Hostnames = []v1beta1.Hostname{} - httpsHR1, expHTTPSHR1Groups, httpsRouteHR1 := createTestResources( "https-hr-1", "foo.example.com", @@ -266,6 +272,8 @@ func TestBuildConfiguration(t *testing.T) { "listener-443-with-hostname", pathAndType{path: "/", pathType: prefix}, ) + // add extra attachment for this route for duplicate listener test + httpsRouteHR5.ParentRefs[0].Attachment.AcceptedHostnames["listener-443-1"] = []string{"example.com"} httpsHR6, expHTTPSHR6Groups, httpsRouteHR6 := createTestResources( "https-hr-6", @@ -360,10 +368,9 @@ func TestBuildConfiguration(t *testing.T) { Source: &v1beta1.Gateway{}, Listeners: map[string]*graph.Listener{ "listener-80-1": { - Source: listener80, - Valid: true, - Routes: map[types.NamespacedName]*graph.Route{}, - AcceptedHostnames: map[string]struct{}{}, + Source: listener80, + Valid: true, + Routes: map[types.NamespacedName]*graph.Route{}, }, }, }, @@ -389,18 +396,16 @@ func TestBuildConfiguration(t *testing.T) { Source: &v1beta1.Gateway{}, Listeners: map[string]*graph.Listener{ "listener-443-1": { - Source: listener443, // nil hostname - Valid: true, - Routes: map[types.NamespacedName]*graph.Route{}, - AcceptedHostnames: map[string]struct{}{}, - SecretPath: secretPath, + Source: listener443, // nil hostname + Valid: true, + Routes: map[types.NamespacedName]*graph.Route{}, + SecretPath: secretPath, }, "listener-443-with-hostname": { - Source: listener443WithHostname, // non-nil hostname - Valid: true, - Routes: map[types.NamespacedName]*graph.Route{}, - AcceptedHostnames: map[string]struct{}{}, - SecretPath: secretPath, + Source: listener443WithHostname, // non-nil hostname + Valid: true, + Routes: map[types.NamespacedName]*graph.Route{}, + SecretPath: secretPath, }, }, }, @@ -466,10 +471,6 @@ func TestBuildConfiguration(t *testing.T) { {Namespace: "test", Name: "hr-1"}: routeHR1, {Namespace: "test", Name: "hr-2"}: routeHR2, }, - AcceptedHostnames: map[string]struct{}{ - "foo.example.com": {}, - "bar.example.com": {}, - }, }, }, }, @@ -524,60 +525,6 @@ func TestBuildConfiguration(t *testing.T) { }, msg: "one http listener with two routes for different hostnames", }, - { - graph: &graph.Graph{ - GatewayClass: &graph.GatewayClass{ - Source: &v1beta1.GatewayClass{}, - Valid: true, - }, - Gateway: &graph.Gateway{ - Source: &v1beta1.Gateway{}, - Listeners: map[string]*graph.Listener{ - "listener-80-1": { - Source: listener80, - Valid: true, - Routes: map[types.NamespacedName]*graph.Route{ - {Namespace: "test", Name: "route-no-host"}: routeNoHost, - }, - AcceptedHostnames: map[string]struct{}{ - wildcardHostname: {}, - }, - }, - }, - }, - Routes: map[types.NamespacedName]*graph.Route{ - {Namespace: "test", Name: "route-no-host"}: routeNoHost, - }, - }, - expConf: Configuration{ - HTTPServers: []VirtualServer{ - { - IsDefault: true, - }, - { - Hostname: wildcardHostname, - PathRules: []PathRule{ - { - Path: "/", - PathType: PathTypePrefix, - MatchRules: []MatchRule{ - { - MatchIdx: 0, - RuleIdx: 0, - BackendGroup: expHRNoHostGroups[0], - Source: hrNoHost, - }, - }, - }, - }, - }, - }, - SSLServers: []VirtualServer{}, - Upstreams: []Upstream{fooUpstream}, - BackendGroups: []BackendGroup{expHRNoHostGroups[0]}, - }, - msg: "one listener and route without hostnames", - }, { graph: &graph.Graph{ GatewayClass: &graph.GatewayClass{ @@ -595,10 +542,6 @@ func TestBuildConfiguration(t *testing.T) { {Namespace: "test", Name: "https-hr-1"}: httpsRouteHR1, {Namespace: "test", Name: "https-hr-2"}: httpsRouteHR2, }, - AcceptedHostnames: map[string]struct{}{ - "foo.example.com": {}, - "bar.example.com": {}, - }, }, "listener-443-with-hostname": { Source: listener443WithHostname, @@ -607,9 +550,6 @@ func TestBuildConfiguration(t *testing.T) { Routes: map[types.NamespacedName]*graph.Route{ {Namespace: "test", Name: "https-hr-5"}: httpsRouteHR5, }, - AcceptedHostnames: map[string]struct{}{ - "example.com": {}, - }, }, }, }, @@ -711,9 +651,6 @@ func TestBuildConfiguration(t *testing.T) { {Namespace: "test", Name: "hr-3"}: routeHR3, {Namespace: "test", Name: "hr-4"}: routeHR4, }, - AcceptedHostnames: map[string]struct{}{ - "foo.example.com": {}, - }, }, "listener-443-1": { Source: listener443, @@ -723,9 +660,6 @@ func TestBuildConfiguration(t *testing.T) { {Namespace: "test", Name: "https-hr-3"}: httpsRouteHR3, {Namespace: "test", Name: "https-hr-4"}: httpsRouteHR4, }, - AcceptedHostnames: map[string]struct{}{ - "foo.example.com": {}, - }, }, }, }, @@ -877,9 +811,6 @@ func TestBuildConfiguration(t *testing.T) { Routes: map[types.NamespacedName]*graph.Route{ {Namespace: "test", Name: "hr-1"}: routeHR1, }, - AcceptedHostnames: map[string]struct{}{ - "foo.example.com": {}, - }, }, }, }, @@ -902,9 +833,6 @@ func TestBuildConfiguration(t *testing.T) { Routes: map[types.NamespacedName]*graph.Route{ {Namespace: "test", Name: "hr-1"}: routeHR1, }, - AcceptedHostnames: map[string]struct{}{ - "foo.example.com": {}, - }, }, }, }, @@ -942,9 +870,6 @@ func TestBuildConfiguration(t *testing.T) { Routes: map[types.NamespacedName]*graph.Route{ {Namespace: "test", Name: "hr-5"}: routeHR5, }, - AcceptedHostnames: map[string]struct{}{ - "foo.example.com": {}, - }, }, }, }, @@ -1014,9 +939,6 @@ func TestBuildConfiguration(t *testing.T) { Routes: map[types.NamespacedName]*graph.Route{ {Namespace: "test", Name: "hr-6"}: routeHR6, }, - AcceptedHostnames: map[string]struct{}{ - "foo.example.com": {}, - }, }, "listener-443-1": { Source: listener443, @@ -1025,9 +947,6 @@ func TestBuildConfiguration(t *testing.T) { Routes: map[types.NamespacedName]*graph.Route{ {Namespace: "test", Name: "https-hr-6"}: httpsRouteHR6, }, - AcceptedHostnames: map[string]struct{}{ - "foo.example.com": {}, - }, }, }, }, @@ -1111,9 +1030,6 @@ func TestBuildConfiguration(t *testing.T) { Routes: map[types.NamespacedName]*graph.Route{ {Namespace: "test", Name: "hr-7"}: routeHR7, }, - AcceptedHostnames: map[string]struct{}{ - "foo.example.com": {}, - }, }, }, }, @@ -1178,9 +1094,6 @@ func TestBuildConfiguration(t *testing.T) { Routes: map[types.NamespacedName]*graph.Route{ {Namespace: "test", Name: "https-hr-5"}: httpsRouteHR5, }, - AcceptedHostnames: map[string]struct{}{ - "example.com": {}, - }, }, "listener-443-1": { Source: listener443, @@ -1189,9 +1102,6 @@ func TestBuildConfiguration(t *testing.T) { Routes: map[types.NamespacedName]*graph.Route{ {Namespace: "test", Name: "https-hr-5"}: httpsRouteHR5, }, - AcceptedHostnames: map[string]struct{}{ - "example.com": {}, - }, }, }, }, diff --git a/internal/state/graph/gateway_listener.go b/internal/state/graph/gateway_listener.go index 7c97c6ae38..5bf859c59c 100644 --- a/internal/state/graph/gateway_listener.go +++ b/internal/state/graph/gateway_listener.go @@ -19,9 +19,6 @@ type Listener struct { // Routes holds the routes attached to the Listener. // Only valid routes are attached. Routes map[types.NamespacedName]*Route - // AcceptedHostnames is an intersection between the hostnames supported by the Listener and the hostnames - // from the attached routes. - AcceptedHostnames map[string]struct{} // SecretPath is the path to the secret on disk. SecretPath string // Conditions holds the conditions of the Listener. @@ -147,10 +144,9 @@ func (c *listenerConfigurator) configure(listener v1beta1.Listener) *Listener { } l := &Listener{ - Source: listener, - Routes: make(map[types.NamespacedName]*Route), - AcceptedHostnames: make(map[string]struct{}), - Valid: true, + Source: listener, + Routes: make(map[types.NamespacedName]*Route), + Valid: true, } // resolvers might add different conditions to the listener, so we run them all. diff --git a/internal/state/graph/gateway_test.go b/internal/state/graph/gateway_test.go index 20ad44e8d9..c1cc34a6c0 100644 --- a/internal/state/graph/gateway_test.go +++ b/internal/state/graph/gateway_test.go @@ -298,10 +298,9 @@ func TestBuildGateway(t *testing.T) { Source: getLastCreatedGetaway(), Listeners: map[string]*Listener{ "listener-80-1": { - Source: listener801, - Valid: true, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, + Source: listener801, + Valid: true, + Routes: map[types.NamespacedName]*Route{}, }, }, Valid: true, @@ -315,11 +314,10 @@ func TestBuildGateway(t *testing.T) { Source: getLastCreatedGetaway(), Listeners: map[string]*Listener{ "listener-443-1": { - Source: listener4431, - Valid: true, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, - SecretPath: secretPath, + Source: listener4431, + Valid: true, + Routes: map[types.NamespacedName]*Route{}, + SecretPath: secretPath, }, }, Valid: true, @@ -418,10 +416,9 @@ func TestBuildGateway(t *testing.T) { Source: getLastCreatedGetaway(), Listeners: map[string]*Listener{ "listener-443-5": { - Source: listener4435, - Valid: false, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, + Source: listener4435, + Valid: false, + Routes: map[types.NamespacedName]*Route{}, Conditions: conditions.NewListenerInvalidCertificateRef( `tls.certificateRefs[0]: Invalid value: test/does-not-exist: secret not found`, ), @@ -440,30 +437,26 @@ func TestBuildGateway(t *testing.T) { Source: getLastCreatedGetaway(), Listeners: map[string]*Listener{ "listener-80-1": { - Source: listener801, - Valid: true, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, + Source: listener801, + Valid: true, + Routes: map[types.NamespacedName]*Route{}, }, "listener-80-3": { - Source: listener803, - Valid: true, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, + Source: listener803, + Valid: true, + Routes: map[types.NamespacedName]*Route{}, }, "listener-443-1": { - Source: listener4431, - Valid: true, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, - SecretPath: secretPath, + Source: listener4431, + Valid: true, + Routes: map[types.NamespacedName]*Route{}, + SecretPath: secretPath, }, "listener-443-2": { - Source: listener4432, - Valid: true, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, - SecretPath: secretPath, + Source: listener4432, + Valid: true, + Routes: map[types.NamespacedName]*Route{}, + SecretPath: secretPath, }, }, Valid: true, @@ -479,34 +472,30 @@ func TestBuildGateway(t *testing.T) { Source: getLastCreatedGetaway(), Listeners: map[string]*Listener{ "listener-80-1": { - Source: listener801, - Valid: false, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, - Conditions: conditions.NewListenerConflictedHostname(conflictedHostnamesMsg), + Source: listener801, + Valid: false, + Routes: map[types.NamespacedName]*Route{}, + Conditions: conditions.NewListenerConflictedHostname(conflictedHostnamesMsg), }, "listener-80-4": { - Source: listener804, - Valid: false, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, - Conditions: conditions.NewListenerConflictedHostname(conflictedHostnamesMsg), + Source: listener804, + Valid: false, + Routes: map[types.NamespacedName]*Route{}, + Conditions: conditions.NewListenerConflictedHostname(conflictedHostnamesMsg), }, "listener-443-1": { - Source: listener4431, - Valid: false, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, - Conditions: conditions.NewListenerConflictedHostname(conflictedHostnamesMsg), - SecretPath: "/etc/nginx/secrets/test_secret", + Source: listener4431, + Valid: false, + Routes: map[types.NamespacedName]*Route{}, + Conditions: conditions.NewListenerConflictedHostname(conflictedHostnamesMsg), + SecretPath: "/etc/nginx/secrets/test_secret", }, "listener-443-3": { - Source: listener4433, - Valid: false, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, - Conditions: conditions.NewListenerConflictedHostname(conflictedHostnamesMsg), - SecretPath: "/etc/nginx/secrets/test_secret", + Source: listener4433, + Valid: false, + Routes: map[types.NamespacedName]*Route{}, + Conditions: conditions.NewListenerConflictedHostname(conflictedHostnamesMsg), + SecretPath: "/etc/nginx/secrets/test_secret", }, }, Valid: true, diff --git a/internal/state/graph/graph_test.go b/internal/state/graph/graph_test.go index 7b292a88b4..1787338bdc 100644 --- a/internal/state/graph/graph_test.go +++ b/internal/state/graph/graph_test.go @@ -173,7 +173,8 @@ func TestBuildGraph(t *testing.T) { Idx: 0, Gateway: client.ObjectKeyFromObject(gw1), Attachment: &ParentRefAttachmentStatus{ - Attached: true, + Attached: true, + AcceptedHostnames: map[string][]string{"listener-80-1": {"foo.example.com"}}, }, }, }, @@ -188,7 +189,8 @@ func TestBuildGraph(t *testing.T) { Idx: 0, Gateway: client.ObjectKeyFromObject(gw1), Attachment: &ParentRefAttachmentStatus{ - Attached: true, + Attached: true, + AcceptedHostnames: map[string][]string{"listener-443-1": {"foo.example.com"}}, }, }, }, @@ -218,9 +220,6 @@ func TestBuildGraph(t *testing.T) { Routes: map[types.NamespacedName]*Route{ {Namespace: "test", Name: "hr-1"}: routeHR1, }, - AcceptedHostnames: map[string]struct{}{ - "foo.example.com": {}, - }, }, "listener-443-1": { Source: gw1.Spec.Listeners[1], @@ -228,9 +227,6 @@ func TestBuildGraph(t *testing.T) { Routes: map[types.NamespacedName]*Route{ {Namespace: "test", Name: "hr-3"}: routeHR3, }, - AcceptedHostnames: map[string]struct{}{ - "foo.example.com": {}, - }, SecretPath: secretPath, }, }, diff --git a/internal/state/graph/httproute.go b/internal/state/graph/httproute.go index 3cdebeb223..64f13db9aa 100644 --- a/internal/state/graph/httproute.go +++ b/internal/state/graph/httproute.go @@ -40,6 +40,9 @@ type ParentRef struct { // ParentRefAttachmentStatus describes the attachment status of a ParentRef. type ParentRefAttachmentStatus struct { + // AcceptedHostnames is an intersection between the hostnames supported by an attached Listener + // and the hostnames from this Route. Key is listener name, value is list of hostnames. + AcceptedHostnames map[string][]string // FailedCondition is the condition that describes why the ParentRef is not attached to the Gateway. It is set // when Attached is false. FailedCondition conditions.Condition @@ -258,7 +261,9 @@ func bindRouteToListeners(r *Route, gw *Gateway) { } for i := 0; i < len(r.ParentRefs); i++ { - attachment := &ParentRefAttachmentStatus{} + attachment := &ParentRefAttachmentStatus{ + AcceptedHostnames: make(map[string][]string), + } ref := &r.ParentRefs[i] ref.Attachment = attachment @@ -293,7 +298,7 @@ func bindRouteToListeners(r *Route, gw *Gateway) { // Case 4 - winning Gateway // Try to attach Route to all matching listeners - cond, attached := tryToAttachRouteToListeners(routeRef, r, gw.Listeners) + cond, attached := tryToAttachRouteToListeners(ref.Attachment, routeRef.SectionName, r, gw.Listeners) if !attached { attachment.FailedCondition = cond continue @@ -310,11 +315,12 @@ func bindRouteToListeners(r *Route, gw *Gateway) { // For now, let's do simple matching. // However, we need to also support wildcard matching. func tryToAttachRouteToListeners( - ref v1beta1.ParentReference, + refStatus *ParentRefAttachmentStatus, + sectionName *v1beta1.SectionName, route *Route, listeners map[string]*Listener, ) (conditions.Condition, bool) { - validListeners, listenerExists := findValidListeners(getSectionName(ref.SectionName), listeners) + validListeners, listenerExists := findValidListeners(getSectionName(sectionName), listeners) if !listenerExists { // FIXME(pleshakov): Add a proper condition once it is available in the Gateway API. @@ -332,9 +338,7 @@ func tryToAttachRouteToListeners( return false } - for _, h := range hostnames { - l.AcceptedHostnames[h] = struct{}{} - } + refStatus.AcceptedHostnames[string(l.Source.Name)] = hostnames l.Routes[client.ObjectKeyFromObject(route.Source)] = route return true diff --git a/internal/state/graph/httproute_test.go b/internal/state/graph/httproute_test.go index 903911c9aa..30d39981cc 100644 --- a/internal/state/graph/httproute_test.go +++ b/internal/state/graph/httproute_test.go @@ -535,9 +535,8 @@ func TestBindRouteToListeners(t *testing.T) { Source: v1beta1.Listener{ Hostname: (*v1beta1.Hostname)(helpers.GetStringPointer("foo.example.com")), }, - Valid: true, - Routes: map[types.NamespacedName]*Route{}, - AcceptedHostnames: map[string]struct{}{}, + Valid: true, + Routes: map[types.NamespacedName]*Route{}, } } createModifiedListener := func(m func(*Listener)) *Listener { @@ -699,6 +698,9 @@ func TestBindRouteToListeners(t *testing.T) { Gateway: client.ObjectKeyFromObject(gw), Attachment: &ParentRefAttachmentStatus{ Attached: true, + AcceptedHostnames: map[string][]string{ + "": {"foo.example.com"}, + }, }, }, }, @@ -707,9 +709,6 @@ func TestBindRouteToListeners(t *testing.T) { l.Routes = map[types.NamespacedName]*Route{ client.ObjectKeyFromObject(hr): getLastNormalRoute(), } - l.AcceptedHostnames = map[string]struct{}{ - "foo.example.com": {}, - } }), }, name: "normal case", @@ -729,6 +728,9 @@ func TestBindRouteToListeners(t *testing.T) { Gateway: client.ObjectKeyFromObject(gw), Attachment: &ParentRefAttachmentStatus{ Attached: true, + AcceptedHostnames: map[string][]string{ + "": {"foo.example.com"}, + }, }, }, }, @@ -737,9 +739,6 @@ func TestBindRouteToListeners(t *testing.T) { l.Routes = map[types.NamespacedName]*Route{ client.ObjectKeyFromObject(hr): routeWithMissingSectionName, } - l.AcceptedHostnames = map[string]struct{}{ - "foo.example.com": {}, - } }), }, name: "section name is nil", @@ -759,6 +758,9 @@ func TestBindRouteToListeners(t *testing.T) { Gateway: client.ObjectKeyFromObject(gw), Attachment: &ParentRefAttachmentStatus{ Attached: true, + AcceptedHostnames: map[string][]string{ + "": {"foo.example.com"}, + }, }, }, }, @@ -767,9 +769,6 @@ func TestBindRouteToListeners(t *testing.T) { l.Routes = map[types.NamespacedName]*Route{ client.ObjectKeyFromObject(hr): routeWithEmptySectionName, } - l.AcceptedHostnames = map[string]struct{}{ - "foo.example.com": {}, - } }), }, name: "section name is empty", @@ -788,8 +787,9 @@ func TestBindRouteToListeners(t *testing.T) { Idx: 0, Gateway: client.ObjectKeyFromObject(gw), Attachment: &ParentRefAttachmentStatus{ - Attached: false, - FailedCondition: conditions.NewRouteInvalidListener(), + Attached: false, + FailedCondition: conditions.NewRouteInvalidListener(), + AcceptedHostnames: map[string][]string{}, }, }, }, @@ -816,6 +816,7 @@ func TestBindRouteToListeners(t *testing.T) { FailedCondition: conditions.NewRouteUnsupportedValue( `spec.parentRefs[0].port: Forbidden: cannot be set`, ), + AcceptedHostnames: map[string][]string{}, }, }, }, @@ -838,8 +839,9 @@ func TestBindRouteToListeners(t *testing.T) { Idx: 0, Gateway: client.ObjectKeyFromObject(gw), Attachment: &ParentRefAttachmentStatus{ - Attached: false, - FailedCondition: conditions.NewTODO("listener is not found"), + Attached: false, + FailedCondition: conditions.NewTODO("listener is not found"), + AcceptedHostnames: map[string][]string{}, }, }, }, @@ -862,8 +864,9 @@ func TestBindRouteToListeners(t *testing.T) { Idx: 0, Gateway: client.ObjectKeyFromObject(gw), Attachment: &ParentRefAttachmentStatus{ - Attached: false, - FailedCondition: conditions.NewRouteInvalidListener(), + Attached: false, + FailedCondition: conditions.NewRouteInvalidListener(), + AcceptedHostnames: map[string][]string{}, }, }, }, @@ -886,8 +889,9 @@ func TestBindRouteToListeners(t *testing.T) { Idx: 0, Gateway: client.ObjectKeyFromObject(gw), Attachment: &ParentRefAttachmentStatus{ - Attached: false, - FailedCondition: conditions.NewRouteNoMatchingListenerHostname(), + Attached: false, + FailedCondition: conditions.NewRouteNoMatchingListenerHostname(), + AcceptedHostnames: map[string][]string{}, }, }, }, @@ -910,8 +914,9 @@ func TestBindRouteToListeners(t *testing.T) { Idx: 0, Gateway: ignoredGwNsName, Attachment: &ParentRefAttachmentStatus{ - Attached: false, - FailedCondition: conditions.NewTODO("Gateway is ignored"), + Attached: false, + FailedCondition: conditions.NewTODO("Gateway is ignored"), + AcceptedHostnames: map[string][]string{}, }, }, }, @@ -955,8 +960,9 @@ func TestBindRouteToListeners(t *testing.T) { Idx: 0, Gateway: client.ObjectKeyFromObject(gw), Attachment: &ParentRefAttachmentStatus{ - Attached: false, - FailedCondition: conditions.NewRouteInvalidGateway(), + Attached: false, + FailedCondition: conditions.NewRouteInvalidGateway(), + AcceptedHostnames: map[string][]string{}, }, }, }, From 70904162f2b699d9469541c86bbba40e9e1dbaf8 Mon Sep 17 00:00:00 2001 From: Saylor Berman Date: Thu, 18 May 2023 12:34:21 -0600 Subject: [PATCH 3/3] Add listener name to test --- internal/state/graph/httproute_test.go | 49 +++++++++++++------------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/internal/state/graph/httproute_test.go b/internal/state/graph/httproute_test.go index 30d39981cc..8a129da6cb 100644 --- a/internal/state/graph/httproute_test.go +++ b/internal/state/graph/httproute_test.go @@ -530,17 +530,18 @@ func TestBuildRoute(t *testing.T) { func TestBindRouteToListeners(t *testing.T) { // we create a new listener each time because the function under test can modify it - createListener := func() *Listener { + createListener := func(name string) *Listener { return &Listener{ Source: v1beta1.Listener{ + Name: v1beta1.SectionName(name), Hostname: (*v1beta1.Hostname)(helpers.GetStringPointer("foo.example.com")), }, Valid: true, Routes: map[types.NamespacedName]*Route{}, } } - createModifiedListener := func(m func(*Listener)) *Listener { - l := createListener() + createModifiedListener := func(name string, m func(*Listener)) *Listener { + l := createListener(name) m(l) return l } @@ -669,10 +670,10 @@ func TestBindRouteToListeners(t *testing.T) { }, } - notValidListener := createModifiedListener(func(l *Listener) { + notValidListener := createModifiedListener("", func(l *Listener) { l.Valid = false }) - nonMatchingHostnameListener := createModifiedListener(func(l *Listener) { + nonMatchingHostnameListener := createModifiedListener("", func(l *Listener) { l.Source.Hostname = helpers.GetPointer[v1beta1.Hostname]("bar.example.com") }) @@ -689,7 +690,7 @@ func TestBindRouteToListeners(t *testing.T) { Source: gw, Valid: true, Listeners: map[string]*Listener{ - "listener-80-1": createListener(), + "listener-80-1": createListener("listener-80-1"), }, }, expectedSectionNameRefs: []ParentRef{ @@ -699,13 +700,13 @@ func TestBindRouteToListeners(t *testing.T) { Attachment: &ParentRefAttachmentStatus{ Attached: true, AcceptedHostnames: map[string][]string{ - "": {"foo.example.com"}, + "listener-80-1": {"foo.example.com"}, }, }, }, }, expectedGatewayListeners: map[string]*Listener{ - "listener-80-1": createModifiedListener(func(l *Listener) { + "listener-80-1": createModifiedListener("listener-80-1", func(l *Listener) { l.Routes = map[types.NamespacedName]*Route{ client.ObjectKeyFromObject(hr): getLastNormalRoute(), } @@ -719,7 +720,7 @@ func TestBindRouteToListeners(t *testing.T) { Source: gw, Valid: true, Listeners: map[string]*Listener{ - "listener-80-1": createListener(), + "listener-80-1": createListener("listener-80-1"), }, }, expectedSectionNameRefs: []ParentRef{ @@ -729,13 +730,13 @@ func TestBindRouteToListeners(t *testing.T) { Attachment: &ParentRefAttachmentStatus{ Attached: true, AcceptedHostnames: map[string][]string{ - "": {"foo.example.com"}, + "listener-80-1": {"foo.example.com"}, }, }, }, }, expectedGatewayListeners: map[string]*Listener{ - "listener-80-1": createModifiedListener(func(l *Listener) { + "listener-80-1": createModifiedListener("listener-80-1", func(l *Listener) { l.Routes = map[types.NamespacedName]*Route{ client.ObjectKeyFromObject(hr): routeWithMissingSectionName, } @@ -749,7 +750,7 @@ func TestBindRouteToListeners(t *testing.T) { Source: gw, Valid: true, Listeners: map[string]*Listener{ - "listener-80-1": createListener(), + "listener-80-1": createListener("listener-80-1"), }, }, expectedSectionNameRefs: []ParentRef{ @@ -759,13 +760,13 @@ func TestBindRouteToListeners(t *testing.T) { Attachment: &ParentRefAttachmentStatus{ Attached: true, AcceptedHostnames: map[string][]string{ - "": {"foo.example.com"}, + "listener-80-1": {"foo.example.com"}, }, }, }, }, expectedGatewayListeners: map[string]*Listener{ - "listener-80-1": createModifiedListener(func(l *Listener) { + "listener-80-1": createModifiedListener("listener-80-1", func(l *Listener) { l.Routes = map[types.NamespacedName]*Route{ client.ObjectKeyFromObject(hr): routeWithEmptySectionName, } @@ -804,7 +805,7 @@ func TestBindRouteToListeners(t *testing.T) { Source: gw, Valid: true, Listeners: map[string]*Listener{ - "listener-80-1": createListener(), + "listener-80-1": createListener("listener-80-1"), }, }, expectedSectionNameRefs: []ParentRef{ @@ -821,7 +822,7 @@ func TestBindRouteToListeners(t *testing.T) { }, }, expectedGatewayListeners: map[string]*Listener{ - "listener-80-1": createListener(), + "listener-80-1": createListener("listener-80-1"), }, name: "port is configured", }, @@ -831,7 +832,7 @@ func TestBindRouteToListeners(t *testing.T) { Source: gw, Valid: true, Listeners: map[string]*Listener{ - "listener-80-1": createListener(), + "listener-80-1": createListener("listener-80-1"), }, }, expectedSectionNameRefs: []ParentRef{ @@ -846,7 +847,7 @@ func TestBindRouteToListeners(t *testing.T) { }, }, expectedGatewayListeners: map[string]*Listener{ - "listener-80-1": createListener(), + "listener-80-1": createListener("listener-80-1"), }, name: "listener doesn't exist", }, @@ -906,7 +907,7 @@ func TestBindRouteToListeners(t *testing.T) { Source: gw, Valid: true, Listeners: map[string]*Listener{ - "listener-80-1": createListener(), + "listener-80-1": createListener("listener-80-1"), }, }, expectedSectionNameRefs: []ParentRef{ @@ -921,7 +922,7 @@ func TestBindRouteToListeners(t *testing.T) { }, }, expectedGatewayListeners: map[string]*Listener{ - "listener-80-1": createListener(), + "listener-80-1": createListener("listener-80-1"), }, name: "gateway is ignored", }, @@ -931,7 +932,7 @@ func TestBindRouteToListeners(t *testing.T) { Source: gw, Valid: true, Listeners: map[string]*Listener{ - "listener-80-1": createListener(), + "listener-80-1": createListener("listener-80-1"), }, }, expectedSectionNameRefs: []ParentRef{ @@ -942,7 +943,7 @@ func TestBindRouteToListeners(t *testing.T) { }, }, expectedGatewayListeners: map[string]*Listener{ - "listener-80-1": createListener(), + "listener-80-1": createListener("listener-80-1"), }, name: "route isn't valid", }, @@ -952,7 +953,7 @@ func TestBindRouteToListeners(t *testing.T) { Source: gw, Valid: false, Listeners: map[string]*Listener{ - "listener-80-1": createListener(), + "listener-80-1": createListener("listener-80-1"), }, }, expectedSectionNameRefs: []ParentRef{ @@ -967,7 +968,7 @@ func TestBindRouteToListeners(t *testing.T) { }, }, expectedGatewayListeners: map[string]*Listener{ - "listener-80-1": createListener(), + "listener-80-1": createListener("listener-80-1"), }, name: "invalid gateway", },