From 46edfcf0d909544b909f4d237dbe0561c2237109 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Tue, 28 Nov 2023 16:25:01 -0800 Subject: [PATCH 01/27] Add namespace changes and adjust test --- .../static/state/change_processor_test.go | 5 +- internal/mode/static/state/graph/graph.go | 12 ++++- internal/mode/static/state/graph/namespace.go | 54 +++++++++++++++++++ .../static/state/relationship/capturer.go | 11 ++-- 4 files changed, 75 insertions(+), 7 deletions(-) create mode 100644 internal/mode/static/state/graph/namespace.go diff --git a/internal/mode/static/state/change_processor_test.go b/internal/mode/static/state/change_processor_test.go index 31837c7d27..e44f45ead2 100644 --- a/internal/mode/static/state/change_processor_test.go +++ b/internal/mode/static/state/change_processor_test.go @@ -1309,8 +1309,11 @@ var _ = Describe("ChangeProcessor", func() { Name: "gw", }, Spec: v1.GatewaySpec{ + GatewayClassName: gcName, Listeners: []v1.Listener{ { + Port: 80, + Protocol: v1.HTTPProtocolType, AllowedRoutes: &v1.AllowedRoutes{ Namespaces: &v1.RouteNamespaces{ From: helpers.GetPointer(v1.NamespacesFromSelector), @@ -1325,7 +1328,7 @@ var _ = Describe("ChangeProcessor", func() { }, }, } - + processor.CaptureUpsertChange(gc) processor.CaptureUpsertChange(gw) processor.CaptureUpsertChange(ns) diff --git a/internal/mode/static/state/graph/graph.go b/internal/mode/static/state/graph/graph.go index ae9f5f8e34..f1b31c2c42 100644 --- a/internal/mode/static/state/graph/graph.go +++ b/internal/mode/static/state/graph/graph.go @@ -44,6 +44,8 @@ type Graph struct { // in the cluster. We need such entries so that we can query the Graph to determine if a Secret is referenced // by the Gateway, including the case when the Secret is newly created. ReferencedSecrets map[types.NamespacedName]*Secret + // ReferencedNamespaces includes Namespaces that have labels that match Gateway listener's label selector. + ReferencedNamespaces map[types.NamespacedName]*Namespace } // ProtectedPorts are the ports that may not be configured by a listener with a descriptive name of each port. @@ -56,10 +58,14 @@ func (g *Graph) IsReferenced(resourceType client.Object, nsname types.Namespaced // as source to determine the relationships. // See https://github.com/nginxinc/nginx-gateway-fabric/issues/824 - switch resourceType.(type) { + switch obj := resourceType.(type) { case *v1.Secret: _, exists := g.ReferencedSecrets[nsname] return exists + case *v1.Namespace: + _, existed := g.ReferencedNamespaces[nsname] + exists := checkNamespace(obj, g.Gateway) + return existed || exists default: return false } @@ -92,6 +98,9 @@ func BuildGraph( bindRoutesToListeners(routes, gw, state.Namespaces) addBackendRefsToRouteRules(routes, refGrantResolver, state.Services) + namespaceResolver := newNamespaceResolver(state.Namespaces) + resolveNamespaces(namespaceResolver, gw) + g := &Graph{ GatewayClass: gc, Gateway: gw, @@ -99,6 +108,7 @@ func BuildGraph( IgnoredGatewayClasses: processedGwClasses.Ignored, IgnoredGateways: processedGws.Ignored, ReferencedSecrets: secretResolver.getResolvedSecrets(), + ReferencedNamespaces: namespaceResolver.getResolvedNamespaces(), } return g diff --git a/internal/mode/static/state/graph/namespace.go b/internal/mode/static/state/graph/namespace.go new file mode 100644 index 0000000000..b22da9b131 --- /dev/null +++ b/internal/mode/static/state/graph/namespace.go @@ -0,0 +1,54 @@ +package graph + +import ( + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/types" +) + +type Namespace struct { + Source *v1.Namespace +} + +type namespaceResolver struct { + clusterNamespaces map[types.NamespacedName]*v1.Namespace + resolvedNamespaces map[types.NamespacedName]*Namespace +} + +func newNamespaceResolver(namespaces map[types.NamespacedName]*v1.Namespace) namespaceResolver { + return namespaceResolver{ + clusterNamespaces: namespaces, + resolvedNamespaces: make(map[types.NamespacedName]*Namespace), + } +} + +func resolveNamespaces(namespaceResolver namespaceResolver, gw *Gateway) { + for name, ns := range namespaceResolver.clusterNamespaces { + if checkNamespace(ns, gw) { + namespaceResolver.resolvedNamespaces[name] = &Namespace{Source: ns} + } + } +} + +func checkNamespace(ns *v1.Namespace, gw *Gateway) bool { + nsLabels := ns.GetLabels() + if gw == nil { + return false + } + for _, listener := range gw.Listeners { + if listener.AllowedRouteLabelSelector == nil { + return false + } + if listener.AllowedRouteLabelSelector.Matches(labels.Set(nsLabels)) { + return true + } + } + return false +} + +func (r *namespaceResolver) getResolvedNamespaces() map[types.NamespacedName]*Namespace { + if len(r.resolvedNamespaces) == 0 { + return nil + } + return r.resolvedNamespaces +} diff --git a/internal/mode/static/state/relationship/capturer.go b/internal/mode/static/state/relationship/capturer.go index 48d08250d0..4c3a6e5259 100644 --- a/internal/mode/static/state/relationship/capturer.go +++ b/internal/mode/static/state/relationship/capturer.go @@ -48,9 +48,9 @@ type ( namespaces map[types.NamespacedName]namespaceCfg ) -func (n namespaceCfg) match() bool { - return len(n.gateways) > 0 -} +//func (n namespaceCfg) match() bool { +// return len(n.gateways) > 0 +//} // CapturerImpl implements the Capturer interface. type CapturerImpl struct { @@ -152,8 +152,9 @@ func (c *CapturerImpl) Exists(resourceType client.Object, nsname types.Namespace svcOwner, exists := c.endpointSliceOwners[nsname] return exists && c.serviceRefCount[svcOwner] > 0 case *v1.Namespace: - cfg, exists := c.namespaces[nsname] - return exists && cfg.match() + // cfg, exists := c.namespaces[nsname] + // return exists && cfg.match() + return false } return false From c77304d50b90af81eb13c18ac746bf323a289fd3 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Wed, 29 Nov 2023 15:08:43 -0800 Subject: [PATCH 02/27] Remove namespace and gateway from capturer and tests --- internal/mode/static/state/graph/graph.go | 7 + internal/mode/static/state/graph/namespace.go | 2 + .../static/state/relationship/capturer.go | 109 +-------------- .../state/relationship/capturer_test.go | 127 ------------------ 4 files changed, 15 insertions(+), 230 deletions(-) diff --git a/internal/mode/static/state/graph/graph.go b/internal/mode/static/state/graph/graph.go index f1b31c2c42..739a803aff 100644 --- a/internal/mode/static/state/graph/graph.go +++ b/internal/mode/static/state/graph/graph.go @@ -63,7 +63,14 @@ func (g *Graph) IsReferenced(resourceType client.Object, nsname types.Namespaced _, exists := g.ReferencedSecrets[nsname] return exists case *v1.Namespace: + // still thinking of why we would need this... aka when existed == true but exists == false + // + // changes when Gateway listener label changes OR when Namespace changes label. Does this handle the case + // when its good -> bad label on the Namespace? _, existed := g.ReferencedNamespaces[nsname] + // Checks if the new resource would be referenced in the new graph + // + // checks the latestGraph's Gateway so even if the gateway changes it'll trigger the change anyways exists := checkNamespace(obj, g.Gateway) return existed || exists default: diff --git a/internal/mode/static/state/graph/namespace.go b/internal/mode/static/state/graph/namespace.go index b22da9b131..d64e1eae70 100644 --- a/internal/mode/static/state/graph/namespace.go +++ b/internal/mode/static/state/graph/namespace.go @@ -30,6 +30,8 @@ func resolveNamespaces(namespaceResolver namespaceResolver, gw *Gateway) { } } +// checkNamespaces returns a boolean that represents whether a given Namespace resource has a label +// that matches any of the Gateway Listener's label selector. func checkNamespace(ns *v1.Namespace, gw *Gateway) bool { nsLabels := ns.GetLabels() if gw == nil { diff --git a/internal/mode/static/state/relationship/capturer.go b/internal/mode/static/state/relationship/capturer.go index 4c3a6e5259..c8af779ea9 100644 --- a/internal/mode/static/state/relationship/capturer.go +++ b/internal/mode/static/state/relationship/capturer.go @@ -3,14 +3,11 @@ package relationship import ( v1 "k8s.io/api/core/v1" discoveryV1 "k8s.io/api/discovery/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/controller/index" - "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/graph" ) //go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . Capturer @@ -22,8 +19,6 @@ import ( // so these relationships are tracked using a counter. // A Service relationship exists if at least one HTTPRoute references it. // An EndpointSlice relationship exists if its Service owner is referenced by at least one HTTPRoute. -// -// A Namespace relationship exists if it has labels that match a Gateway listener's label selector. type Capturer interface { Capture(obj client.Object) Remove(resourceType client.Object, nsname types.NamespacedName) @@ -35,40 +30,21 @@ type ( routeToServicesMap map[types.NamespacedName]map[types.NamespacedName]struct{} // serviceRefCountMap maps Service names to the number of HTTPRoutes that reference it. serviceRefCountMap map[types.NamespacedName]int - // gatewayLabelSelectorsMap maps Gateways to the label selectors that their listeners use for allowed routes - gatewayLabelSelectorsMap map[types.NamespacedName][]labels.Selector - // namespaceCfg holds information about a namespace - // - labels that it contains - // - gateways that reference it (if labels match) - namespaceCfg struct { - labelMap map[string]string - gateways map[types.NamespacedName]struct{} - } - // namespaces is a collection of namespaces in the system - namespaces map[types.NamespacedName]namespaceCfg ) -//func (n namespaceCfg) match() bool { -// return len(n.gateways) > 0 -//} - // CapturerImpl implements the Capturer interface. type CapturerImpl struct { - routesToServices routeToServicesMap - serviceRefCount serviceRefCountMap - gatewayLabelSelectors gatewayLabelSelectorsMap - namespaces namespaces - endpointSliceOwners map[types.NamespacedName]types.NamespacedName + routesToServices routeToServicesMap + serviceRefCount serviceRefCountMap + endpointSliceOwners map[types.NamespacedName]types.NamespacedName } // NewCapturerImpl creates a new instance of CapturerImpl. func NewCapturerImpl() *CapturerImpl { return &CapturerImpl{ - routesToServices: make(routeToServicesMap), - serviceRefCount: make(serviceRefCountMap), - gatewayLabelSelectors: make(gatewayLabelSelectorsMap), - namespaces: make(namespaces), - endpointSliceOwners: make(map[types.NamespacedName]types.NamespacedName), + routesToServices: make(routeToServicesMap), + serviceRefCount: make(serviceRefCountMap), + endpointSliceOwners: make(map[types.NamespacedName]types.NamespacedName), } } @@ -85,47 +61,6 @@ func (c *CapturerImpl) Capture(obj client.Object) { Name: svcName, } } - case *gatewayv1.Gateway: - var selectors []labels.Selector - for _, listener := range o.Spec.Listeners { - if selector := graph.GetAllowedRouteLabelSelector(listener); selector != nil { - convertedSelector, err := metav1.LabelSelectorAsSelector(selector) - if err == nil { - selectors = append(selectors, convertedSelector) - } - } - } - - gatewayName := client.ObjectKeyFromObject(o) - if len(selectors) > 0 { - c.gatewayLabelSelectors[gatewayName] = selectors - for ns, cfg := range c.namespaces { - var gatewayMatches bool - for _, selector := range selectors { - if selector.Matches(labels.Set(cfg.labelMap)) { - gatewayMatches = true - cfg.gateways[gatewayName] = struct{}{} - break - } - } - if !gatewayMatches { - // if gateway was previously referenced by this namespace, clean it up - delete(cfg.gateways, gatewayName) - } - c.namespaces[ns] = cfg - } - } else if _, exists := c.gatewayLabelSelectors[gatewayName]; exists { - // label selectors existed previously for this gateway, so clean up any references to them - c.removeGatewayLabelSelector(gatewayName) - } - case *v1.Namespace: - nsLabels := o.GetLabels() - gateways := c.matchingGateways(nsLabels) - nsCfg := namespaceCfg{ - labelMap: nsLabels, - gateways: gateways, - } - c.namespaces[client.ObjectKeyFromObject(o)] = nsCfg } } @@ -136,10 +71,6 @@ func (c *CapturerImpl) Remove(resourceType client.Object, nsname types.Namespace c.deleteForRoute(nsname) case *discoveryV1.EndpointSlice: delete(c.endpointSliceOwners, nsname) - case *gatewayv1.Gateway: - c.removeGatewayLabelSelector(nsname) - case *v1.Namespace: - delete(c.namespaces, nsname) } } @@ -151,10 +82,6 @@ func (c *CapturerImpl) Exists(resourceType client.Object, nsname types.Namespace case *discoveryV1.EndpointSlice: svcOwner, exists := c.endpointSliceOwners[nsname] return exists && c.serviceRefCount[svcOwner] > 0 - case *v1.Namespace: - // cfg, exists := c.namespaces[nsname] - // return exists && cfg.match() - return false } return false @@ -226,27 +153,3 @@ func getBackendServiceNamesFromRoute(hr *gatewayv1.HTTPRoute) map[types.Namespac return svcNames } - -// matchingGateways looks through all existing label selectors defined by listeners in a gateway, -// and if any matches are found, returns a map of those gateways -func (c *CapturerImpl) matchingGateways(labelMap map[string]string) map[types.NamespacedName]struct{} { - gateways := make(map[types.NamespacedName]struct{}) - for gw, selectors := range c.gatewayLabelSelectors { - for _, selector := range selectors { - if selector.Matches(labels.Set(labelMap)) { - gateways[gw] = struct{}{} - break - } - } - } - - return gateways -} - -func (c *CapturerImpl) removeGatewayLabelSelector(gatewayName types.NamespacedName) { - delete(c.gatewayLabelSelectors, gatewayName) - for ns, cfg := range c.namespaces { - delete(cfg.gateways, gatewayName) - c.namespaces[ns] = cfg - } -} diff --git a/internal/mode/static/state/relationship/capturer_test.go b/internal/mode/static/state/relationship/capturer_test.go index ca66e730e0..38437d34c4 100644 --- a/internal/mode/static/state/relationship/capturer_test.go +++ b/internal/mode/static/state/relationship/capturer_test.go @@ -7,7 +7,6 @@ import ( discoveryV1 "k8s.io/api/discovery/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - "sigs.k8s.io/controller-runtime/pkg/client" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/controller/index" @@ -315,132 +314,6 @@ var _ = Describe("Capturer", func() { }) }) }) - Describe("Capture namespace and gateway relationships", func() { - var gw *gatewayv1.Gateway - var nsNoLabels, ns *v1.Namespace - - BeforeEach(func() { - capturer = relationship.NewCapturerImpl() - gw = &gatewayv1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Name: "gw", - }, - Spec: gatewayv1.GatewaySpec{ - Listeners: []gatewayv1.Listener{ - { - AllowedRoutes: &gatewayv1.AllowedRoutes{ - Namespaces: &gatewayv1.RouteNamespaces{ - From: helpers.GetPointer(gatewayv1.NamespacesFromSelector), - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "app": "valid", - }, - }, - }, - }, - }, - }, - }, - } - nsNoLabels = &v1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: "no-labels", - }, - } - ns = &v1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: "with-labels", - Labels: map[string]string{ - "app": "valid", - }, - }, - } - }) - - When("a gateway with label selectors is created, but no namespace has been captured", func() { - It("does not report a relationship", func() { - capturer.Capture(gw) - - Expect(capturer.Exists(ns, client.ObjectKeyFromObject(ns))).To(BeFalse()) - }) - }) - When("a namespace is created that is not linked to a listener", func() { - It("does not report a relationship", func() { - capturer.Capture(gw) - capturer.Capture(nsNoLabels) - - Expect(capturer.Exists(nsNoLabels, client.ObjectKeyFromObject(nsNoLabels))).To(BeFalse()) - }) - }) - When("a namespace is created that is linked to a listener", func() { - It("reports a relationship", func() { - capturer.Capture(gw) - capturer.Capture(ns) - - Expect(capturer.Exists(ns, client.ObjectKeyFromObject(ns))).To(BeTrue()) - }) - }) - When("a gateway with label selectors is created after a linked namespace", func() { - It("reports a relationship", func() { - capturer.Capture(ns) - Expect(capturer.Exists(ns, client.ObjectKeyFromObject(ns))).To(BeFalse()) - - capturer.Capture(gw) - Expect(capturer.Exists(ns, client.ObjectKeyFromObject(ns))).To(BeTrue()) - }) - }) - When("label selectors are removed from gateway", func() { - It("does not report a relationship", func() { - capturer.Capture(gw) - capturer.Capture(ns) - - Expect(capturer.Exists(ns, client.ObjectKeyFromObject(ns))).To(BeTrue()) - - gw.Spec.Listeners[0].AllowedRoutes = nil - capturer.Capture(gw) - Expect(capturer.Exists(ns, client.ObjectKeyFromObject(ns))).To(BeFalse()) - }) - }) - When("gateway changes its labels", func() { - It("does not report a relationship", func() { - capturer.Capture(gw) - capturer.Capture(ns) - - Expect(capturer.Exists(ns, client.ObjectKeyFromObject(ns))).To(BeTrue()) - - gw.Spec.Listeners[0].AllowedRoutes.Namespaces.Selector.MatchLabels = map[string]string{ - "app": "new-value", - } - capturer.Capture(gw) - Expect(capturer.Exists(ns, client.ObjectKeyFromObject(ns))).To(BeFalse()) - }) - }) - When("gateway is deleted", func() { - It("does not report a relationship", func() { - capturer.Capture(gw) - capturer.Capture(ns) - - Expect(capturer.Exists(ns, client.ObjectKeyFromObject(ns))).To(BeTrue()) - - capturer.Remove(gw, client.ObjectKeyFromObject(gw)) - Expect(capturer.Exists(ns, client.ObjectKeyFromObject(ns))).To(BeFalse()) - }) - }) - When("a namespace has its labels removed after being linked", func() { - It("reports that a relationship once existed", func() { - capturer.Capture(gw) - capturer.Capture(ns) - - Expect(capturer.Exists(ns, client.ObjectKeyFromObject(ns))).To(BeTrue()) - - ns.Labels = nil - Expect(capturer.Exists(ns, client.ObjectKeyFromObject(ns))).To(BeTrue()) - - capturer.Capture(ns) - Expect(capturer.Exists(ns, client.ObjectKeyFromObject(ns))).To(BeFalse()) - }) - }) - }) Describe("Edge cases", func() { BeforeEach(func() { capturer = relationship.NewCapturerImpl() From 49e00c29917ea8f8c934d0e57f47fbca4a7a56a2 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Thu, 30 Nov 2023 14:34:10 -0800 Subject: [PATCH 03/27] Add test cases in graph_test --- .../mode/static/state/graph/graph_test.go | 168 +++++++++++++++++- internal/mode/static/state/graph/namespace.go | 3 +- 2 files changed, 168 insertions(+), 3 deletions(-) diff --git a/internal/mode/static/state/graph/graph_test.go b/internal/mode/static/state/graph/graph_test.go index cae15a3722..8911629fe0 100644 --- a/internal/mode/static/state/graph/graph_test.go +++ b/internal/mode/static/state/graph/graph_test.go @@ -7,10 +7,11 @@ import ( . "github.com/onsi/gomega" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" - v1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" + "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers" "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/state/validation" @@ -119,6 +120,15 @@ func TestBuildGraph(t *testing.T) { Type: v1.SecretTypeTLS, } + ns := &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Labels: map[string]string{ + "app": "allowed", + }, + }, + } + createGateway := func(name string) *gatewayv1.Gateway { return &gatewayv1.Gateway{ ObjectMeta: metav1.ObjectMeta{ @@ -133,6 +143,16 @@ func TestBuildGraph(t *testing.T) { Hostname: nil, Port: 80, Protocol: gatewayv1.HTTPProtocolType, + AllowedRoutes: &gatewayv1.AllowedRoutes{ + Namespaces: &gatewayv1.RouteNamespaces{ + From: helpers.GetPointer(gatewayv1.NamespacesFromSelector), + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "allowed", + }, + }, + }, + }, }, { @@ -220,6 +240,9 @@ func TestBuildGraph(t *testing.T) { Services: map[types.NamespacedName]*v1.Service{ client.ObjectKeyFromObject(svc): svc, }, + Namespaces: map[types.NamespacedName]*v1.Namespace{ + client.ObjectKeyFromObject(ns): ns, + }, ReferenceGrants: map[types.NamespacedName]*v1beta1.ReferenceGrant{ client.ObjectKeyFromObject(rgSecret): rgSecret, client.ObjectKeyFromObject(rgService): rgService, @@ -281,7 +304,8 @@ func TestBuildGraph(t *testing.T) { Routes: map[types.NamespacedName]*Route{ {Namespace: "test", Name: "hr-1"}: routeHR1, }, - SupportedKinds: []gatewayv1.RouteGroupKind{{Kind: "HTTPRoute"}}, + SupportedKinds: []gatewayv1.RouteGroupKind{{Kind: "HTTPRoute"}}, + AllowedRouteLabelSelector: labels.SelectorFromSet(map[string]string{"app": "allowed"}), }, { Name: "listener-443-1", @@ -309,6 +333,11 @@ func TestBuildGraph(t *testing.T) { Source: secret, }, }, + ReferencedNamespaces: map[types.NamespacedName]*Namespace{ + client.ObjectKeyFromObject(ns): { + Source: ns, + }, + }, } } @@ -362,3 +391,138 @@ func TestBuildGraph(t *testing.T) { }) } } + +func TestIsReferenced(t *testing.T) { + baseSecret := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "secret", + }, + Data: map[string][]byte{ + v1.TLSCertKey: cert, + v1.TLSPrivateKeyKey: key, + }, + Type: v1.SecretTypeTLS, + } + sameNamespaceDifferentNameSecret := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "secret-different-name", + }, + Data: map[string][]byte{ + v1.TLSCertKey: cert, + v1.TLSPrivateKeyKey: key, + }, + Type: v1.SecretTypeTLS, + } + differentNamespaceSameNameSecret := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-different-namespace", + Name: "secret", + }, + Data: map[string][]byte{ + v1.TLSCertKey: cert, + v1.TLSPrivateKeyKey: key, + }, + Type: v1.SecretTypeTLS, + } + + nsInGraph := &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Labels: map[string]string{ + "app": "allowed", + }, + }, + } + nsNotInGraph := &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "different-name", + Labels: map[string]string{ + "app": "allowed", + }, + }, + } + + gw := &Gateway{ + Listeners: map[string]*Listener{ + "listener-1": { + Valid: true, + AllowedRouteLabelSelector: labels.SelectorFromSet(map[string]string{"apples": "oranges"}), + }, + }, + Valid: true, + } + + nsNotInGraphButInGateway := &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "notInGraphButInGateway", + Labels: map[string]string{ + "apples": "oranges", + }, + }, + } + + graph := &Graph{ + Gateway: gw, + ReferencedSecrets: map[types.NamespacedName]*Secret{ + client.ObjectKeyFromObject(baseSecret): { + Source: baseSecret, + }, + }, + ReferencedNamespaces: map[types.NamespacedName]*Namespace{ + client.ObjectKeyFromObject(nsInGraph): { + Source: nsInGraph, + }, + }, + } + + tests := []struct { + graph *Graph + resource client.Object + name string + expected bool + }{ + { + name: "Namespace in graph's ReferencedNamespaces passes", + graph: graph, + expected: true, + resource: nsInGraph, + }, + { + name: "Namespace with a different name but same labels fails", + graph: graph, + expected: false, + resource: nsNotInGraph, + }, + { + name: "Secret in graph's ReferencedSecrets passes", + graph: graph, + expected: true, + resource: baseSecret, + }, + { + name: "Secret not in ReferencedSecrets with same Namespace and different Name fails", + graph: graph, + expected: false, + resource: sameNamespaceDifferentNameSecret, + }, + { + name: "Secret not in ReferencedSecrets with different Namespace and same Name fails", + graph: graph, + expected: false, + resource: differentNamespaceSameNameSecret, + }, + { + name: "Namespace not in ReferencedNamespaces but in Gateway Listener's AllowedRouteLabelSelector passes", + graph: graph, + expected: true, + resource: nsNotInGraphButInGateway, + }, + } + for _, test := range tests { + g := NewWithT(t) + result := test.graph.IsReferenced(test.resource, client.ObjectKeyFromObject(test.resource)) + g.Expect(result).To(Equal(test.expected)) + } +} diff --git a/internal/mode/static/state/graph/namespace.go b/internal/mode/static/state/graph/namespace.go index d64e1eae70..2a8a06f0a8 100644 --- a/internal/mode/static/state/graph/namespace.go +++ b/internal/mode/static/state/graph/namespace.go @@ -39,7 +39,8 @@ func checkNamespace(ns *v1.Namespace, gw *Gateway) bool { } for _, listener := range gw.Listeners { if listener.AllowedRouteLabelSelector == nil { - return false + // Can have some listeners with AllowedRouteLabelSelector not set + continue } if listener.AllowedRouteLabelSelector.Matches(labels.Set(nsLabels)) { return true From 56233e61628114cc8fd3243b4c3b6b480313cbdd Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Thu, 30 Nov 2023 14:35:04 -0800 Subject: [PATCH 04/27] Add nit change in graph_test --- internal/mode/static/state/graph/graph_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/mode/static/state/graph/graph_test.go b/internal/mode/static/state/graph/graph_test.go index 8911629fe0..b11389aba6 100644 --- a/internal/mode/static/state/graph/graph_test.go +++ b/internal/mode/static/state/graph/graph_test.go @@ -485,39 +485,39 @@ func TestIsReferenced(t *testing.T) { }{ { name: "Namespace in graph's ReferencedNamespaces passes", + resource: nsInGraph, graph: graph, expected: true, - resource: nsInGraph, }, { name: "Namespace with a different name but same labels fails", + resource: nsNotInGraph, graph: graph, expected: false, - resource: nsNotInGraph, }, { name: "Secret in graph's ReferencedSecrets passes", + resource: baseSecret, graph: graph, expected: true, - resource: baseSecret, }, { name: "Secret not in ReferencedSecrets with same Namespace and different Name fails", + resource: sameNamespaceDifferentNameSecret, graph: graph, expected: false, - resource: sameNamespaceDifferentNameSecret, }, { name: "Secret not in ReferencedSecrets with different Namespace and same Name fails", + resource: differentNamespaceSameNameSecret, graph: graph, expected: false, - resource: differentNamespaceSameNameSecret, }, { name: "Namespace not in ReferencedNamespaces but in Gateway Listener's AllowedRouteLabelSelector passes", + resource: nsNotInGraphButInGateway, graph: graph, expected: true, - resource: nsNotInGraphButInGateway, }, } for _, test := range tests { From cf77769dcc528931aa2ada6184e1590c61b5feb5 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Fri, 1 Dec 2023 12:11:14 -0800 Subject: [PATCH 05/27] Add some comments and some changes to namespace --- internal/mode/static/state/graph/graph.go | 23 ++++++++----- internal/mode/static/state/graph/namespace.go | 34 ++++++++++--------- 2 files changed, 32 insertions(+), 25 deletions(-) diff --git a/internal/mode/static/state/graph/graph.go b/internal/mode/static/state/graph/graph.go index 739a803aff..b624a4d096 100644 --- a/internal/mode/static/state/graph/graph.go +++ b/internal/mode/static/state/graph/graph.go @@ -63,14 +63,19 @@ func (g *Graph) IsReferenced(resourceType client.Object, nsname types.Namespaced _, exists := g.ReferencedSecrets[nsname] return exists case *v1.Namespace: - // still thinking of why we would need this... aka when existed == true but exists == false + // `existed` is needed as it checks the graph's ReferencedNamespaces which stores all the namespaces that + // match the Gateway listener's label selector when the graph was created. This covers the case when + // a Namespace changes its label so it no longer matches a Gateway listener's label selector, but because + // it was in the graph's ReferencedNamespaces we know that the Graph did reference the Namespace. // - // changes when Gateway listener label changes OR when Namespace changes label. Does this handle the case - // when its good -> bad label on the Namespace? - _, existed := g.ReferencedNamespaces[nsname] - // Checks if the new resource would be referenced in the new graph + // However, if there is a Namespace which changes its label (previously it did not match) to match a Gateway + // listener's label selector, it will not be in the current graph's ReferencedNamespaces until it is rebuilt + // and thus not be caught in `existed`. Therefore, we need `exists` to check the graph's Gateway and see if the + // new Namespace actually matches any of the Gateway listener's label selector. // - // checks the latestGraph's Gateway so even if the gateway changes it'll trigger the change anyways + // `exists` does not cover the case highlighted above by `existed` and vice versa so both are needed. + + _, existed := g.ReferencedNamespaces[nsname] exists := checkNamespace(obj, g.Gateway) return existed || exists default: @@ -105,8 +110,8 @@ func BuildGraph( bindRoutesToListeners(routes, gw, state.Namespaces) addBackendRefsToRouteRules(routes, refGrantResolver, state.Services) - namespaceResolver := newNamespaceResolver(state.Namespaces) - resolveNamespaces(namespaceResolver, gw) + namespaceHolder := newNamespaceHolder(state.Namespaces) + buildReferencedNamespaces(namespaceHolder, gw) g := &Graph{ GatewayClass: gc, @@ -115,7 +120,7 @@ func BuildGraph( IgnoredGatewayClasses: processedGwClasses.Ignored, IgnoredGateways: processedGws.Ignored, ReferencedSecrets: secretResolver.getResolvedSecrets(), - ReferencedNamespaces: namespaceResolver.getResolvedNamespaces(), + ReferencedNamespaces: namespaceHolder.getResolvedNamespaces(), } return g diff --git a/internal/mode/static/state/graph/namespace.go b/internal/mode/static/state/graph/namespace.go index 2a8a06f0a8..7733b1ea90 100644 --- a/internal/mode/static/state/graph/namespace.go +++ b/internal/mode/static/state/graph/namespace.go @@ -6,26 +6,28 @@ import ( "k8s.io/apimachinery/pkg/types" ) +// Namespace represents a Namespace resource. type Namespace struct { + // Source holds the actual Namespace resource. Can be nil if the Namespace does not exist. Source *v1.Namespace } -type namespaceResolver struct { - clusterNamespaces map[types.NamespacedName]*v1.Namespace - resolvedNamespaces map[types.NamespacedName]*Namespace +type namespaceHolder struct { + clusterNamespaces map[types.NamespacedName]*v1.Namespace + referencedNamespaces map[types.NamespacedName]*Namespace } -func newNamespaceResolver(namespaces map[types.NamespacedName]*v1.Namespace) namespaceResolver { - return namespaceResolver{ - clusterNamespaces: namespaces, - resolvedNamespaces: make(map[types.NamespacedName]*Namespace), +func newNamespaceHolder(namespaces map[types.NamespacedName]*v1.Namespace) namespaceHolder { + return namespaceHolder{ + clusterNamespaces: namespaces, + referencedNamespaces: make(map[types.NamespacedName]*Namespace), } } -func resolveNamespaces(namespaceResolver namespaceResolver, gw *Gateway) { - for name, ns := range namespaceResolver.clusterNamespaces { +func buildReferencedNamespaces(namespaceHolder namespaceHolder, gw *Gateway) { + for name, ns := range namespaceHolder.clusterNamespaces { if checkNamespace(ns, gw) { - namespaceResolver.resolvedNamespaces[name] = &Namespace{Source: ns} + namespaceHolder.referencedNamespaces[name] = &Namespace{Source: ns} } } } @@ -33,13 +35,13 @@ func resolveNamespaces(namespaceResolver namespaceResolver, gw *Gateway) { // checkNamespaces returns a boolean that represents whether a given Namespace resource has a label // that matches any of the Gateway Listener's label selector. func checkNamespace(ns *v1.Namespace, gw *Gateway) bool { - nsLabels := ns.GetLabels() - if gw == nil { + if gw == nil || ns == nil { return false } + nsLabels := ns.GetLabels() for _, listener := range gw.Listeners { if listener.AllowedRouteLabelSelector == nil { - // Can have some listeners with AllowedRouteLabelSelector not set + // Can have listeners with AllowedRouteLabelSelector not set. continue } if listener.AllowedRouteLabelSelector.Matches(labels.Set(nsLabels)) { @@ -49,9 +51,9 @@ func checkNamespace(ns *v1.Namespace, gw *Gateway) bool { return false } -func (r *namespaceResolver) getResolvedNamespaces() map[types.NamespacedName]*Namespace { - if len(r.resolvedNamespaces) == 0 { +func (r *namespaceHolder) getResolvedNamespaces() map[types.NamespacedName]*Namespace { + if len(r.referencedNamespaces) == 0 { return nil } - return r.resolvedNamespaces + return r.referencedNamespaces } From 42cbf603c069afaff0701b2d32cb0a5cdc7c01f7 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Fri, 1 Dec 2023 12:12:38 -0800 Subject: [PATCH 06/27] Add small function name change --- internal/mode/static/state/graph/graph.go | 2 +- internal/mode/static/state/graph/namespace.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/mode/static/state/graph/graph.go b/internal/mode/static/state/graph/graph.go index b624a4d096..680f3ed78b 100644 --- a/internal/mode/static/state/graph/graph.go +++ b/internal/mode/static/state/graph/graph.go @@ -120,7 +120,7 @@ func BuildGraph( IgnoredGatewayClasses: processedGwClasses.Ignored, IgnoredGateways: processedGws.Ignored, ReferencedSecrets: secretResolver.getResolvedSecrets(), - ReferencedNamespaces: namespaceHolder.getResolvedNamespaces(), + ReferencedNamespaces: namespaceHolder.getReferencedNamespaces(), } return g diff --git a/internal/mode/static/state/graph/namespace.go b/internal/mode/static/state/graph/namespace.go index 7733b1ea90..8cf1012f48 100644 --- a/internal/mode/static/state/graph/namespace.go +++ b/internal/mode/static/state/graph/namespace.go @@ -51,7 +51,7 @@ func checkNamespace(ns *v1.Namespace, gw *Gateway) bool { return false } -func (r *namespaceHolder) getResolvedNamespaces() map[types.NamespacedName]*Namespace { +func (r *namespaceHolder) getReferencedNamespaces() map[types.NamespacedName]*Namespace { if len(r.referencedNamespaces) == 0 { return nil } From 46baf275c29eb6a2907638de1b39d67a7bf0c5a1 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Mon, 4 Dec 2023 13:17:19 -0800 Subject: [PATCH 07/27] Replace Namespace resource with Kubernetes resource --- internal/mode/static/state/graph/graph.go | 9 ++--- .../mode/static/state/graph/graph_test.go | 12 ++---- internal/mode/static/state/graph/namespace.go | 40 ++++++------------- 3 files changed, 20 insertions(+), 41 deletions(-) diff --git a/internal/mode/static/state/graph/graph.go b/internal/mode/static/state/graph/graph.go index 680f3ed78b..62d0326e77 100644 --- a/internal/mode/static/state/graph/graph.go +++ b/internal/mode/static/state/graph/graph.go @@ -44,8 +44,8 @@ type Graph struct { // in the cluster. We need such entries so that we can query the Graph to determine if a Secret is referenced // by the Gateway, including the case when the Secret is newly created. ReferencedSecrets map[types.NamespacedName]*Secret - // ReferencedNamespaces includes Namespaces that have labels that match Gateway listener's label selector. - ReferencedNamespaces map[types.NamespacedName]*Namespace + // ReferencedNamespaces includes Namespaces that have labels which match the Gateway Listener's label selector. + ReferencedNamespaces map[types.NamespacedName]*v1.Namespace } // ProtectedPorts are the ports that may not be configured by a listener with a descriptive name of each port. @@ -110,8 +110,7 @@ func BuildGraph( bindRoutesToListeners(routes, gw, state.Namespaces) addBackendRefsToRouteRules(routes, refGrantResolver, state.Services) - namespaceHolder := newNamespaceHolder(state.Namespaces) - buildReferencedNamespaces(namespaceHolder, gw) + referencedNamespaces := buildReferencedNamespaces(state.Namespaces, gw) g := &Graph{ GatewayClass: gc, @@ -120,7 +119,7 @@ func BuildGraph( IgnoredGatewayClasses: processedGwClasses.Ignored, IgnoredGateways: processedGws.Ignored, ReferencedSecrets: secretResolver.getResolvedSecrets(), - ReferencedNamespaces: namespaceHolder.getReferencedNamespaces(), + ReferencedNamespaces: referencedNamespaces, } return g diff --git a/internal/mode/static/state/graph/graph_test.go b/internal/mode/static/state/graph/graph_test.go index b11389aba6..ff96c88b42 100644 --- a/internal/mode/static/state/graph/graph_test.go +++ b/internal/mode/static/state/graph/graph_test.go @@ -333,10 +333,8 @@ func TestBuildGraph(t *testing.T) { Source: secret, }, }, - ReferencedNamespaces: map[types.NamespacedName]*Namespace{ - client.ObjectKeyFromObject(ns): { - Source: ns, - }, + ReferencedNamespaces: map[types.NamespacedName]*v1.Namespace{ + client.ObjectKeyFromObject(ns): ns, }, } } @@ -470,10 +468,8 @@ func TestIsReferenced(t *testing.T) { Source: baseSecret, }, }, - ReferencedNamespaces: map[types.NamespacedName]*Namespace{ - client.ObjectKeyFromObject(nsInGraph): { - Source: nsInGraph, - }, + ReferencedNamespaces: map[types.NamespacedName]*v1.Namespace{ + client.ObjectKeyFromObject(nsInGraph): nsInGraph, }, } diff --git a/internal/mode/static/state/graph/namespace.go b/internal/mode/static/state/graph/namespace.go index 8cf1012f48..c706ac2d01 100644 --- a/internal/mode/static/state/graph/namespace.go +++ b/internal/mode/static/state/graph/namespace.go @@ -6,30 +6,21 @@ import ( "k8s.io/apimachinery/pkg/types" ) -// Namespace represents a Namespace resource. -type Namespace struct { - // Source holds the actual Namespace resource. Can be nil if the Namespace does not exist. - Source *v1.Namespace -} - -type namespaceHolder struct { - clusterNamespaces map[types.NamespacedName]*v1.Namespace - referencedNamespaces map[types.NamespacedName]*Namespace -} - -func newNamespaceHolder(namespaces map[types.NamespacedName]*v1.Namespace) namespaceHolder { - return namespaceHolder{ - clusterNamespaces: namespaces, - referencedNamespaces: make(map[types.NamespacedName]*Namespace), - } -} - -func buildReferencedNamespaces(namespaceHolder namespaceHolder, gw *Gateway) { - for name, ns := range namespaceHolder.clusterNamespaces { +// buildReferencedNamespaces returns a map of all the Namespace resources in the current clusterState with a label +// that matches any of the Gateway Listener's label selector. +func buildReferencedNamespaces(clusterNamespaces map[types.NamespacedName]*v1.Namespace, + gw *Gateway, +) map[types.NamespacedName]*v1.Namespace { + referencedNamespaces := make(map[types.NamespacedName]*v1.Namespace) + for name, ns := range clusterNamespaces { if checkNamespace(ns, gw) { - namespaceHolder.referencedNamespaces[name] = &Namespace{Source: ns} + referencedNamespaces[name] = ns } } + if len(referencedNamespaces) == 0 { + return nil + } + return referencedNamespaces } // checkNamespaces returns a boolean that represents whether a given Namespace resource has a label @@ -50,10 +41,3 @@ func checkNamespace(ns *v1.Namespace, gw *Gateway) bool { } return false } - -func (r *namespaceHolder) getReferencedNamespaces() map[types.NamespacedName]*Namespace { - if len(r.referencedNamespaces) == 0 { - return nil - } - return r.referencedNamespaces -} From d1762cd6ef52aaea94ec5aa74b0a43d5a58cfccf Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Mon, 4 Dec 2023 14:43:04 -0800 Subject: [PATCH 08/27] Add more change processor tests --- .../static/state/change_processor_test.go | 147 ++++++++++++++---- 1 file changed, 115 insertions(+), 32 deletions(-) diff --git a/internal/mode/static/state/change_processor_test.go b/internal/mode/static/state/change_processor_test.go index e44f45ead2..e5a5fd3237 100644 --- a/internal/mode/static/state/change_processor_test.go +++ b/internal/mode/static/state/change_processor_test.go @@ -1294,52 +1294,135 @@ var _ = Describe("ChangeProcessor", func() { }) }) Describe("namespace changes", func() { - When("namespace is linked via label selectors", func() { - It("triggers an update when labels are removed", func() { - ns := &apiv1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: "ns", - Labels: map[string]string{ - "app": "allowed", - }, - }, - } - gw := &v1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Name: "gw", - }, - Spec: v1.GatewaySpec{ - GatewayClassName: gcName, - Listeners: []v1.Listener{ - { - Port: 80, - Protocol: v1.HTTPProtocolType, - AllowedRoutes: &v1.AllowedRoutes{ - Namespaces: &v1.RouteNamespaces{ - From: helpers.GetPointer(v1.NamespacesFromSelector), - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "app": "allowed", - }, - }, + ns := &apiv1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ns", + Labels: map[string]string{ + "app": "allowed", + }, + }, + } + nsDifferentLabels := &apiv1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ns-different-labels", + Labels: map[string]string{ + "oranges": "bananas", + }, + }, + } + nsNoLabels := &apiv1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "no-labels", + }, + } + gw := &v1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gw", + }, + Spec: v1.GatewaySpec{ + GatewayClassName: gcName, + Listeners: []v1.Listener{ + { + Port: 80, + Protocol: v1.HTTPProtocolType, + AllowedRoutes: &v1.AllowedRoutes{ + Namespaces: &v1.RouteNamespaces{ + From: helpers.GetPointer(v1.NamespacesFromSelector), + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "allowed", }, }, }, }, }, - } - processor.CaptureUpsertChange(gc) - processor.CaptureUpsertChange(gw) + }, + }, + } + BeforeEach(func() { + processor = state.NewChangeProcessorImpl(state.ChangeProcessorConfig{ + GatewayCtlrName: controllerName, + GatewayClassName: gcName, + RelationshipCapturer: relationship.NewCapturerImpl(), + Logger: zap.New(), + Validators: createAlwaysValidValidators(), + Scheme: createScheme(), + }) + + processor.CaptureUpsertChange(gc) + processor.CaptureUpsertChange(gw) + processor.Process() + }) + When("a namespace is created that is not linked to a listener", func() { + It("does not trigger an update", func() { + processor.CaptureUpsertChange(nsNoLabels) + changed, _ := processor.Process() + Expect(changed).To(BeFalse()) + }) + }) + When("a namespace is created that is linked to a listener", func() { + It("triggers an update", func() { + processor.CaptureUpsertChange(ns) + changed, _ := processor.Process() + Expect(changed).To(BeTrue()) + }) + }) + When("a namespace is deleted that is not linked to a listener", func() { + It("does not trigger an update", func() { + processor.CaptureUpsertChange(nsNoLabels) + processor.Process() + + processor.CaptureDeleteChange(nsNoLabels, types.NamespacedName{Name: "no-labels"}) + changed, _ := processor.Process() + Expect(changed).To(BeFalse()) + }) + }) + When("a namespace is deleted that is linked to a listener", func() { + It("triggers an update", func() { processor.CaptureUpsertChange(ns) + processor.Process() + processor.CaptureDeleteChange(nsNoLabels, types.NamespacedName{Name: "ns"}) changed, _ := processor.Process() Expect(changed).To(BeTrue()) + }) + }) + When("a namespace that is not linked to a listener has its labels removed", func() { + It("does not trigger an update", func() { + processor.CaptureUpsertChange(nsDifferentLabels) + processor.Process() + + nsDifferentLabels.Labels = nil + processor.CaptureUpsertChange(nsDifferentLabels) + + changed, _ := processor.Process() + Expect(changed).To(BeFalse()) + }) + }) + When("a namespace that is not linked to a listener has its labels changed to match a listener", func() { + It("does triggers an update", func() { + processor.CaptureUpsertChange(nsDifferentLabels) + processor.Process() + + nsDifferentLabels.Labels = map[string]string{ + "app": "allowed", + } + processor.CaptureUpsertChange(nsDifferentLabels) + + changed, _ := processor.Process() + Expect(changed).To(BeTrue()) + }) + }) + When("a namespace is linked via label selectors", func() { + It("triggers an update when labels are removed", func() { + processor.CaptureUpsertChange(ns) + processor.Process() newNS := ns.DeepCopy() newNS.Labels = nil processor.CaptureUpsertChange(newNS) - changed, _ = processor.Process() + changed, _ := processor.Process() Expect(changed).To(BeTrue()) }) }) From 36181a6299966c2a92dbd438a7aa5f1f54dc7928 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Wed, 6 Dec 2023 14:12:19 -0800 Subject: [PATCH 09/27] Add namespace tests and more test cases to change processor --- .../static/state/change_processor_test.go | 108 +++++++++++------ .../mode/static/state/graph/namespace_test.go | 113 ++++++++++++++++++ 2 files changed, 183 insertions(+), 38 deletions(-) create mode 100644 internal/mode/static/state/graph/namespace_test.go diff --git a/internal/mode/static/state/change_processor_test.go b/internal/mode/static/state/change_processor_test.go index e5a5fd3237..a7996c7369 100644 --- a/internal/mode/static/state/change_processor_test.go +++ b/internal/mode/static/state/change_processor_test.go @@ -1294,52 +1294,57 @@ var _ = Describe("ChangeProcessor", func() { }) }) Describe("namespace changes", func() { - ns := &apiv1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: "ns", - Labels: map[string]string{ - "app": "allowed", + var ( + ns, nsDifferentLabels, nsNoLabels *apiv1.Namespace + gw *v1.Gateway + ) + + BeforeEach(func() { + ns = &apiv1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ns", + Labels: map[string]string{ + "app": "allowed", + }, }, - }, - } - nsDifferentLabels := &apiv1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: "ns-different-labels", - Labels: map[string]string{ - "oranges": "bananas", + } + nsDifferentLabels = &apiv1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ns-different-labels", + Labels: map[string]string{ + "oranges": "bananas", + }, }, - }, - } - nsNoLabels := &apiv1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: "no-labels", - }, - } - gw := &v1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Name: "gw", - }, - Spec: v1.GatewaySpec{ - GatewayClassName: gcName, - Listeners: []v1.Listener{ - { - Port: 80, - Protocol: v1.HTTPProtocolType, - AllowedRoutes: &v1.AllowedRoutes{ - Namespaces: &v1.RouteNamespaces{ - From: helpers.GetPointer(v1.NamespacesFromSelector), - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "app": "allowed", + } + nsNoLabels = &apiv1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "no-labels", + }, + } + gw = &v1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gw", + }, + Spec: v1.GatewaySpec{ + GatewayClassName: gcName, + Listeners: []v1.Listener{ + { + Port: 80, + Protocol: v1.HTTPProtocolType, + AllowedRoutes: &v1.AllowedRoutes{ + Namespaces: &v1.RouteNamespaces{ + From: helpers.GetPointer(v1.NamespacesFromSelector), + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "allowed", + }, }, }, }, }, }, }, - }, - } - BeforeEach(func() { + } processor = state.NewChangeProcessorImpl(state.ChangeProcessorConfig{ GatewayCtlrName: controllerName, GatewayClassName: gcName, @@ -1413,6 +1418,33 @@ var _ = Describe("ChangeProcessor", func() { Expect(changed).To(BeTrue()) }) }) + When("a gateway changes its listener's labels", func() { + It("triggers an update when a namespace that matches the new labels is created", func() { + processor.CaptureUpsertChange(ns) + changed, _ := processor.Process() + Expect(changed).To(BeTrue()) + + processor.CaptureUpsertChange(nsDifferentLabels) + changed, _ = processor.Process() + Expect(changed).To(BeFalse()) + + gwChangedLabel := gw.DeepCopy() + gwChangedLabel.Spec.Listeners[0].AllowedRoutes.Namespaces.Selector.MatchLabels = map[string]string{ + "oranges": "bananas", + } + gwChangedLabel.Generation++ + processor.CaptureUpsertChange(gwChangedLabel) + processor.Process() + + processor.CaptureUpsertChange(ns) + changed, _ = processor.Process() + Expect(changed).To(BeFalse()) + + processor.CaptureUpsertChange(nsDifferentLabels) + changed, _ = processor.Process() + Expect(changed).To(BeTrue()) + }) + }) When("a namespace is linked via label selectors", func() { It("triggers an update when labels are removed", func() { processor.CaptureUpsertChange(ns) diff --git a/internal/mode/static/state/graph/namespace_test.go b/internal/mode/static/state/graph/namespace_test.go new file mode 100644 index 0000000000..f8fc8e3444 --- /dev/null +++ b/internal/mode/static/state/graph/namespace_test.go @@ -0,0 +1,113 @@ +package graph + +import ( + "testing" + + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/types" +) + +func TestBuildReferencedNamespaces(t *testing.T) { + ns1 := &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ns1", + }, + } + + ns2 := &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ns2", + Labels: map[string]string{ + "apples": "oranges", + }, + }, + } + + ns3 := &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ns3", + Labels: map[string]string{ + "peaches": "bananas", + }, + }, + } + + clusterNamespaces := map[types.NamespacedName]*v1.Namespace{ + {Name: "ns1"}: ns1, + {Name: "ns2"}: ns2, + {Name: "ns3"}: ns3, + } + + tests := []struct { + gw *Gateway + expectedRefNS map[types.NamespacedName]*v1.Namespace + name string + }{ + { + gw: &Gateway{ + Listeners: map[string]*Listener{ + "listener-1": { + Valid: true, + AllowedRouteLabelSelector: labels.SelectorFromSet(map[string]string{"apples": "oranges"}), + }, + }, + Valid: true, + }, + expectedRefNS: map[types.NamespacedName]*v1.Namespace{ + {Name: "ns2"}: ns2, + }, + name: "gateway matches labels with one namespace", + }, + { + gw: &Gateway{ + Listeners: map[string]*Listener{}, + Valid: true, + }, + expectedRefNS: nil, + name: "gateway has no Listeners", + }, + { + gw: &Gateway{ + Listeners: map[string]*Listener{ + "listener-1": { + Valid: true, + AllowedRouteLabelSelector: labels.SelectorFromSet(map[string]string{"apples": "oranges"}), + }, + "listener-2": { + Valid: true, + AllowedRouteLabelSelector: labels.SelectorFromSet(map[string]string{"peaches": "bananas"}), + }, + }, + Valid: true, + }, + expectedRefNS: map[types.NamespacedName]*v1.Namespace{ + {Name: "ns2"}: ns2, + {Name: "ns3"}: ns3, + }, + name: "gateway matches labels with two namespaces", + }, + { + gw: &Gateway{ + Listeners: map[string]*Listener{ + "listener-1": { + Valid: true, + AllowedRouteLabelSelector: labels.SelectorFromSet(map[string]string{"not": "matching"}), + }, + }, + Valid: true, + }, + + expectedRefNS: nil, + name: "gateway doesn't match labels with any namespace", + }, + } + + for _, test := range tests { + g := NewWithT(t) + g.Expect(buildReferencedNamespaces(clusterNamespaces, test.gw)).To(Equal(test.expectedRefNS)) + } +} From 116eae533733f19f0758513f68463b27e1ab0439 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Wed, 6 Dec 2023 14:31:18 -0800 Subject: [PATCH 10/27] Add predicate function for namespace --- internal/mode/static/state/change_processor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mode/static/state/change_processor.go b/internal/mode/static/state/change_processor.go index d2ba8eed35..14f8ce1a52 100644 --- a/internal/mode/static/state/change_processor.go +++ b/internal/mode/static/state/change_processor.go @@ -146,7 +146,7 @@ func NewChangeProcessorImpl(cfg ChangeProcessorConfig) *ChangeProcessorImpl { { gvk: extractGVK(&apiv1.Namespace{}), store: newObjectStoreMapAdapter(clusterStore.Namespaces), - predicate: nil, + predicate: funcPredicate{stateChanged: isReferenced}, }, { gvk: extractGVK(&apiv1.Service{}), From 7b4b9eea6f6719ef349b782b3a4726ecaffcab26 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Wed, 6 Dec 2023 14:33:33 -0800 Subject: [PATCH 11/27] Add small change to comments --- internal/mode/static/state/graph/graph.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/mode/static/state/graph/graph.go b/internal/mode/static/state/graph/graph.go index 62d0326e77..cf15a16b3f 100644 --- a/internal/mode/static/state/graph/graph.go +++ b/internal/mode/static/state/graph/graph.go @@ -53,8 +53,8 @@ type ProtectedPorts map[int32]string // IsReferenced returns true if the Graph references the resource. func (g *Graph) IsReferenced(resourceType client.Object, nsname types.NamespacedName) bool { - // FIMXE(pleshakov): For now, only works with Secrets. - // Support EndpointSlices and Namespaces so that we can remove relationship.Capturer and use the Graph + // FIMXE(pleshakov): For now, only works with Secrets and Namespaces. + // Support EndpointSlices so that we can remove relationship.Capturer and use the Graph // as source to determine the relationships. // See https://github.com/nginxinc/nginx-gateway-fabric/issues/824 From 8471179af68854cdef75956ec68f675a44ea329f Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Wed, 6 Dec 2023 14:35:43 -0800 Subject: [PATCH 12/27] Add nit grammar change --- internal/mode/static/state/graph/graph.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mode/static/state/graph/graph.go b/internal/mode/static/state/graph/graph.go index cf15a16b3f..44154e3dc6 100644 --- a/internal/mode/static/state/graph/graph.go +++ b/internal/mode/static/state/graph/graph.go @@ -44,7 +44,7 @@ type Graph struct { // in the cluster. We need such entries so that we can query the Graph to determine if a Secret is referenced // by the Gateway, including the case when the Secret is newly created. ReferencedSecrets map[types.NamespacedName]*Secret - // ReferencedNamespaces includes Namespaces that have labels which match the Gateway Listener's label selector. + // ReferencedNamespaces includes Namespaces with labels that match the Gateway Listener's label selector. ReferencedNamespaces map[types.NamespacedName]*v1.Namespace } From 990d1c5e5a6018901407143c6b5277aee4bcc1e6 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Tue, 12 Dec 2023 14:23:30 -0800 Subject: [PATCH 13/27] Fix test description typo --- internal/mode/static/state/change_processor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mode/static/state/change_processor_test.go b/internal/mode/static/state/change_processor_test.go index a7996c7369..7d53f3d295 100644 --- a/internal/mode/static/state/change_processor_test.go +++ b/internal/mode/static/state/change_processor_test.go @@ -1405,7 +1405,7 @@ var _ = Describe("ChangeProcessor", func() { }) }) When("a namespace that is not linked to a listener has its labels changed to match a listener", func() { - It("does triggers an update", func() { + It("triggers an update", func() { processor.CaptureUpsertChange(nsDifferentLabels) processor.Process() From 25e8b6d88d85b0de001a958e67a81f7576fc67c8 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Tue, 12 Dec 2023 14:29:43 -0800 Subject: [PATCH 14/27] Refactor function name to add more description --- internal/mode/static/state/graph/graph.go | 2 +- internal/mode/static/state/graph/namespace.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/mode/static/state/graph/graph.go b/internal/mode/static/state/graph/graph.go index 44154e3dc6..d0b1e39472 100644 --- a/internal/mode/static/state/graph/graph.go +++ b/internal/mode/static/state/graph/graph.go @@ -76,7 +76,7 @@ func (g *Graph) IsReferenced(resourceType client.Object, nsname types.Namespaced // `exists` does not cover the case highlighted above by `existed` and vice versa so both are needed. _, existed := g.ReferencedNamespaces[nsname] - exists := checkNamespace(obj, g.Gateway) + exists := isNamespaceReferenced(obj, g.Gateway) return existed || exists default: return false diff --git a/internal/mode/static/state/graph/namespace.go b/internal/mode/static/state/graph/namespace.go index c706ac2d01..c1a9a69a6e 100644 --- a/internal/mode/static/state/graph/namespace.go +++ b/internal/mode/static/state/graph/namespace.go @@ -13,7 +13,7 @@ func buildReferencedNamespaces(clusterNamespaces map[types.NamespacedName]*v1.Na ) map[types.NamespacedName]*v1.Namespace { referencedNamespaces := make(map[types.NamespacedName]*v1.Namespace) for name, ns := range clusterNamespaces { - if checkNamespace(ns, gw) { + if isNamespaceReferenced(ns, gw) { referencedNamespaces[name] = ns } } @@ -23,9 +23,9 @@ func buildReferencedNamespaces(clusterNamespaces map[types.NamespacedName]*v1.Na return referencedNamespaces } -// checkNamespaces returns a boolean that represents whether a given Namespace resource has a label +// isNamespaceReferenced returns a boolean that represents whether a given Namespace resource has a label // that matches any of the Gateway Listener's label selector. -func checkNamespace(ns *v1.Namespace, gw *Gateway) bool { +func isNamespaceReferenced(ns *v1.Namespace, gw *Gateway) bool { if gw == nil || ns == nil { return false } From 2b8e462e8618a296141e6a5fcbfdc3b9770e5545 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Tue, 12 Dec 2023 14:42:59 -0800 Subject: [PATCH 15/27] Simplify IsReferenced test --- internal/mode/static/state/graph/graph_test.go | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/internal/mode/static/state/graph/graph_test.go b/internal/mode/static/state/graph/graph_test.go index ff96c88b42..0d78660dc9 100644 --- a/internal/mode/static/state/graph/graph_test.go +++ b/internal/mode/static/state/graph/graph_test.go @@ -396,33 +396,18 @@ func TestIsReferenced(t *testing.T) { Namespace: "test", Name: "secret", }, - Data: map[string][]byte{ - v1.TLSCertKey: cert, - v1.TLSPrivateKeyKey: key, - }, - Type: v1.SecretTypeTLS, } sameNamespaceDifferentNameSecret := &v1.Secret{ ObjectMeta: metav1.ObjectMeta{ Namespace: "test", Name: "secret-different-name", }, - Data: map[string][]byte{ - v1.TLSCertKey: cert, - v1.TLSPrivateKeyKey: key, - }, - Type: v1.SecretTypeTLS, } differentNamespaceSameNameSecret := &v1.Secret{ ObjectMeta: metav1.ObjectMeta{ Namespace: "test-different-namespace", Name: "secret", }, - Data: map[string][]byte{ - v1.TLSCertKey: cert, - v1.TLSPrivateKeyKey: key, - }, - Type: v1.SecretTypeTLS, } nsInGraph := &v1.Namespace{ From 652cb3beb42091f50e2e954f671ceb38ddc0bf7f Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Tue, 12 Dec 2023 15:22:24 -0800 Subject: [PATCH 16/27] Add subtests --- internal/mode/static/state/graph/graph_test.go | 8 +++++--- internal/mode/static/state/graph/namespace_test.go | 6 ++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/internal/mode/static/state/graph/graph_test.go b/internal/mode/static/state/graph/graph_test.go index 0d78660dc9..56f882dd58 100644 --- a/internal/mode/static/state/graph/graph_test.go +++ b/internal/mode/static/state/graph/graph_test.go @@ -502,8 +502,10 @@ func TestIsReferenced(t *testing.T) { }, } for _, test := range tests { - g := NewWithT(t) - result := test.graph.IsReferenced(test.resource, client.ObjectKeyFromObject(test.resource)) - g.Expect(result).To(Equal(test.expected)) + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + result := test.graph.IsReferenced(test.resource, client.ObjectKeyFromObject(test.resource)) + g.Expect(result).To(Equal(test.expected)) + }) } } diff --git a/internal/mode/static/state/graph/namespace_test.go b/internal/mode/static/state/graph/namespace_test.go index f8fc8e3444..0daa9fc0fe 100644 --- a/internal/mode/static/state/graph/namespace_test.go +++ b/internal/mode/static/state/graph/namespace_test.go @@ -107,7 +107,9 @@ func TestBuildReferencedNamespaces(t *testing.T) { } for _, test := range tests { - g := NewWithT(t) - g.Expect(buildReferencedNamespaces(clusterNamespaces, test.gw)).To(Equal(test.expectedRefNS)) + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + g.Expect(buildReferencedNamespaces(clusterNamespaces, test.gw)).To(Equal(test.expectedRefNS)) + }) } } From 1b60dd8bf2cbd6055007b4d41533932e2d74e854 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Wed, 13 Dec 2023 11:05:47 -0800 Subject: [PATCH 17/27] Add comments for test result and small fix to function parameters --- internal/mode/static/state/change_processor_test.go | 7 +++++++ internal/mode/static/state/graph/namespace.go | 3 ++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/internal/mode/static/state/change_processor_test.go b/internal/mode/static/state/change_processor_test.go index 7d53f3d295..50f202692f 100644 --- a/internal/mode/static/state/change_processor_test.go +++ b/internal/mode/static/state/change_processor_test.go @@ -1436,6 +1436,13 @@ var _ = Describe("ChangeProcessor", func() { processor.CaptureUpsertChange(gwChangedLabel) processor.Process() + // After changing the gateway's labels and generation, the processor should be marked to update + // the nginx configuration and build a new graph. When processor.Process() gets called, + // the nginx configuration gets updated and a new graph is built with an updated + // referencedNamespaces. Thus, when the namespace "ns" is upserted with labels that no longer match + // the new labels on the gateway, it would not trigger a change as the namespace would no longer + // be in the updated referencedNamespaces and the labels no longer match the new labels on the + // gateway. processor.CaptureUpsertChange(ns) changed, _ = processor.Process() Expect(changed).To(BeFalse()) diff --git a/internal/mode/static/state/graph/namespace.go b/internal/mode/static/state/graph/namespace.go index c1a9a69a6e..84b27b02c1 100644 --- a/internal/mode/static/state/graph/namespace.go +++ b/internal/mode/static/state/graph/namespace.go @@ -8,7 +8,8 @@ import ( // buildReferencedNamespaces returns a map of all the Namespace resources in the current clusterState with a label // that matches any of the Gateway Listener's label selector. -func buildReferencedNamespaces(clusterNamespaces map[types.NamespacedName]*v1.Namespace, +func buildReferencedNamespaces( + clusterNamespaces map[types.NamespacedName]*v1.Namespace, gw *Gateway, ) map[types.NamespacedName]*v1.Namespace { referencedNamespaces := make(map[types.NamespacedName]*v1.Namespace) From c0911bebd0f73c047887348c3c4dc2d93a60432f Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Wed, 13 Dec 2023 11:09:17 -0800 Subject: [PATCH 18/27] Check changed in change process test for gateway --- internal/mode/static/state/change_processor_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/mode/static/state/change_processor_test.go b/internal/mode/static/state/change_processor_test.go index 50f202692f..18a7f88997 100644 --- a/internal/mode/static/state/change_processor_test.go +++ b/internal/mode/static/state/change_processor_test.go @@ -1434,7 +1434,8 @@ var _ = Describe("ChangeProcessor", func() { } gwChangedLabel.Generation++ processor.CaptureUpsertChange(gwChangedLabel) - processor.Process() + changed, _ = processor.Process() + Expect(changed).To(BeTrue()) // After changing the gateway's labels and generation, the processor should be marked to update // the nginx configuration and build a new graph. When processor.Process() gets called, From 04233e09e2acbae3008c025ca15705bc6be8addb Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Wed, 13 Dec 2023 11:23:28 -0800 Subject: [PATCH 19/27] Add more namespace test cases --- .../mode/static/state/graph/namespace_test.go | 66 +++++++++++++++++-- 1 file changed, 59 insertions(+), 7 deletions(-) diff --git a/internal/mode/static/state/graph/namespace_test.go b/internal/mode/static/state/graph/namespace_test.go index 0daa9fc0fe..f2358eaca8 100644 --- a/internal/mode/static/state/graph/namespace_test.go +++ b/internal/mode/static/state/graph/namespace_test.go @@ -62,6 +62,26 @@ func TestBuildReferencedNamespaces(t *testing.T) { }, name: "gateway matches labels with one namespace", }, + { + gw: &Gateway{ + Listeners: map[string]*Listener{ + "listener-1": { + Valid: true, + AllowedRouteLabelSelector: labels.SelectorFromSet(map[string]string{"apples": "oranges"}), + }, + "listener-2": { + Valid: true, + AllowedRouteLabelSelector: labels.SelectorFromSet(map[string]string{"peaches": "bananas"}), + }, + }, + Valid: true, + }, + expectedRefNS: map[types.NamespacedName]*v1.Namespace{ + {Name: "ns2"}: ns2, + {Name: "ns3"}: ns3, + }, + name: "gateway matches labels with two namespaces", + }, { gw: &Gateway{ Listeners: map[string]*Listener{}, @@ -70,6 +90,35 @@ func TestBuildReferencedNamespaces(t *testing.T) { expectedRefNS: nil, name: "gateway has no Listeners", }, + { + gw: &Gateway{ + Listeners: map[string]*Listener{ + "listener-1": { + Valid: true, + }, + "listener-2": { + Valid: true, + }, + }, + Valid: true, + }, + expectedRefNS: nil, + name: "gateway has multiple listeners with no AllowedRouteLabelSelector set", + }, + { + gw: &Gateway{ + Listeners: map[string]*Listener{ + "listener-1": { + Valid: true, + AllowedRouteLabelSelector: labels.SelectorFromSet(map[string]string{"not": "matching"}), + }, + }, + Valid: true, + }, + + expectedRefNS: nil, + name: "gateway doesn't match labels with any namespace", + }, { gw: &Gateway{ Listeners: map[string]*Listener{ @@ -79,30 +128,33 @@ func TestBuildReferencedNamespaces(t *testing.T) { }, "listener-2": { Valid: true, - AllowedRouteLabelSelector: labels.SelectorFromSet(map[string]string{"peaches": "bananas"}), + AllowedRouteLabelSelector: labels.SelectorFromSet(map[string]string{"not": "matching"}), }, }, Valid: true, }, expectedRefNS: map[types.NamespacedName]*v1.Namespace{ {Name: "ns2"}: ns2, - {Name: "ns3"}: ns3, }, - name: "gateway matches labels with two namespaces", + name: "gateway has two listeners and only matches labels with one namespace", }, { gw: &Gateway{ Listeners: map[string]*Listener{ "listener-1": { Valid: true, - AllowedRouteLabelSelector: labels.SelectorFromSet(map[string]string{"not": "matching"}), + AllowedRouteLabelSelector: labels.SelectorFromSet(map[string]string{"apples": "oranges"}), + }, + "listener-2": { + Valid: true, }, }, Valid: true, }, - - expectedRefNS: nil, - name: "gateway doesn't match labels with any namespace", + expectedRefNS: map[types.NamespacedName]*v1.Namespace{ + {Name: "ns2"}: ns2, + }, + name: "gateway has two listeners, one with a matching AllowedRouteLabelSelector and one without the field set", }, } From 8dc478753591639e3dcd986d1499f1ed8f460543 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Wed, 13 Dec 2023 12:19:53 -0800 Subject: [PATCH 20/27] Refactor change processor tests to make them ordered --- .../static/state/change_processor_test.go | 155 ++++++++---------- 1 file changed, 72 insertions(+), 83 deletions(-) diff --git a/internal/mode/static/state/change_processor_test.go b/internal/mode/static/state/change_processor_test.go index 18a7f88997..12cee66e53 100644 --- a/internal/mode/static/state/change_processor_test.go +++ b/internal/mode/static/state/change_processor_test.go @@ -1294,70 +1294,64 @@ var _ = Describe("ChangeProcessor", func() { }) }) Describe("namespace changes", func() { - var ( - ns, nsDifferentLabels, nsNoLabels *apiv1.Namespace - gw *v1.Gateway - ) - - BeforeEach(func() { - ns = &apiv1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: "ns", - Labels: map[string]string{ - "app": "allowed", - }, - }, - } - nsDifferentLabels = &apiv1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: "ns-different-labels", - Labels: map[string]string{ - "oranges": "bananas", - }, - }, - } - nsNoLabels = &apiv1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: "no-labels", + ns := &apiv1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ns", + Labels: map[string]string{ + "app": "allowed", }, - } - gw = &v1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Name: "gw", + }, + } + nsDifferentLabels := &apiv1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ns-different-labels", + Labels: map[string]string{ + "oranges": "bananas", }, - Spec: v1.GatewaySpec{ - GatewayClassName: gcName, - Listeners: []v1.Listener{ - { - Port: 80, - Protocol: v1.HTTPProtocolType, - AllowedRoutes: &v1.AllowedRoutes{ - Namespaces: &v1.RouteNamespaces{ - From: helpers.GetPointer(v1.NamespacesFromSelector), - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "app": "allowed", - }, + }, + } + nsNoLabels := &apiv1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "no-labels", + }, + } + gw := &v1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gw", + }, + Spec: v1.GatewaySpec{ + GatewayClassName: gcName, + Listeners: []v1.Listener{ + { + Port: 80, + Protocol: v1.HTTPProtocolType, + AllowedRoutes: &v1.AllowedRoutes{ + Namespaces: &v1.RouteNamespaces{ + From: helpers.GetPointer(v1.NamespacesFromSelector), + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "allowed", }, }, }, }, }, }, - } - processor = state.NewChangeProcessorImpl(state.ChangeProcessorConfig{ - GatewayCtlrName: controllerName, - GatewayClassName: gcName, - RelationshipCapturer: relationship.NewCapturerImpl(), - Logger: zap.New(), - Validators: createAlwaysValidValidators(), - Scheme: createScheme(), - }) - - processor.CaptureUpsertChange(gc) - processor.CaptureUpsertChange(gw) - processor.Process() + }, + } + processor := state.NewChangeProcessorImpl(state.ChangeProcessorConfig{ + GatewayCtlrName: controllerName, + GatewayClassName: gcName, + RelationshipCapturer: relationship.NewCapturerImpl(), + Logger: zap.New(), + Validators: createAlwaysValidValidators(), + Scheme: createScheme(), }) + + processor.CaptureUpsertChange(gc) + processor.CaptureUpsertChange(gw) + processor.Process() + When("a namespace is created that is not linked to a listener", func() { It("does not trigger an update", func() { processor.CaptureUpsertChange(nsNoLabels) @@ -1374,9 +1368,6 @@ var _ = Describe("ChangeProcessor", func() { }) When("a namespace is deleted that is not linked to a listener", func() { It("does not trigger an update", func() { - processor.CaptureUpsertChange(nsNoLabels) - processor.Process() - processor.CaptureDeleteChange(nsNoLabels, types.NamespacedName{Name: "no-labels"}) changed, _ := processor.Process() Expect(changed).To(BeFalse()) @@ -1384,36 +1375,31 @@ var _ = Describe("ChangeProcessor", func() { }) When("a namespace is deleted that is linked to a listener", func() { It("triggers an update", func() { - processor.CaptureUpsertChange(ns) - processor.Process() - - processor.CaptureDeleteChange(nsNoLabels, types.NamespacedName{Name: "ns"}) + processor.CaptureDeleteChange(ns, types.NamespacedName{Name: "ns"}) changed, _ := processor.Process() Expect(changed).To(BeTrue()) }) }) - When("a namespace that is not linked to a listener has its labels removed", func() { - It("does not trigger an update", func() { - processor.CaptureUpsertChange(nsDifferentLabels) - processor.Process() - - nsDifferentLabels.Labels = nil + When("a namespace that is not linked to a listener has its labels changed to match a listener", func() { + It("triggers an update", func() { processor.CaptureUpsertChange(nsDifferentLabels) - changed, _ := processor.Process() Expect(changed).To(BeFalse()) + + nsDifferentLabels.Labels = map[string]string{ + "app": "allowed", + } + processor.CaptureUpsertChange(nsDifferentLabels) + changed, _ = processor.Process() + Expect(changed).To(BeTrue()) }) }) - When("a namespace that is not linked to a listener has its labels changed to match a listener", func() { + When("a namespace that is linked to a listener has its labels changed to no longer match a listener", func() { It("triggers an update", func() { - processor.CaptureUpsertChange(nsDifferentLabels) - processor.Process() - nsDifferentLabels.Labels = map[string]string{ - "app": "allowed", + "oranges": "bananas", } processor.CaptureUpsertChange(nsDifferentLabels) - changed, _ := processor.Process() Expect(changed).To(BeTrue()) }) @@ -1453,15 +1439,18 @@ var _ = Describe("ChangeProcessor", func() { Expect(changed).To(BeTrue()) }) }) - When("a namespace is linked via label selectors", func() { - It("triggers an update when labels are removed", func() { + When("a namespace that is not linked to a listener has its labels removed", func() { + It("does not trigger an update", func() { + ns.Labels = nil processor.CaptureUpsertChange(ns) - processor.Process() - - newNS := ns.DeepCopy() - newNS.Labels = nil - processor.CaptureUpsertChange(newNS) - + changed, _ := processor.Process() + Expect(changed).To(BeFalse()) + }) + }) + When("a namespace that is linked to a listener has its labels removed", func() { + It("triggers an update when labels are removed", func() { + nsDifferentLabels.Labels = nil + processor.CaptureUpsertChange(nsDifferentLabels) changed, _ := processor.Process() Expect(changed).To(BeTrue()) }) From 876f97c98478bfceb1a17e7fdcccc2e6e6916c86 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Wed, 13 Dec 2023 14:41:28 -0800 Subject: [PATCH 21/27] Add small fixes to comments and wrapping BeforeAll function in change processor --- .../static/state/change_processor_test.go | 104 +++++++++--------- internal/mode/static/state/graph/graph.go | 2 +- internal/mode/static/state/graph/namespace.go | 6 +- 3 files changed, 59 insertions(+), 53 deletions(-) diff --git a/internal/mode/static/state/change_processor_test.go b/internal/mode/static/state/change_processor_test.go index 12cee66e53..12da0faa69 100644 --- a/internal/mode/static/state/change_processor_test.go +++ b/internal/mode/static/state/change_processor_test.go @@ -1293,65 +1293,71 @@ var _ = Describe("ChangeProcessor", func() { }) }) }) - Describe("namespace changes", func() { - ns := &apiv1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: "ns", - Labels: map[string]string{ - "app": "allowed", + Describe("namespace changes", Ordered, func() { + var ( + ns, nsDifferentLabels, nsNoLabels *apiv1.Namespace + gw *v1.Gateway + ) + + BeforeAll(func() { + ns = &apiv1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ns", + Labels: map[string]string{ + "app": "allowed", + }, }, - }, - } - nsDifferentLabels := &apiv1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: "ns-different-labels", - Labels: map[string]string{ - "oranges": "bananas", + } + nsDifferentLabels = &apiv1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ns-different-labels", + Labels: map[string]string{ + "oranges": "bananas", + }, }, - }, - } - nsNoLabels := &apiv1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: "no-labels", - }, - } - gw := &v1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Name: "gw", - }, - Spec: v1.GatewaySpec{ - GatewayClassName: gcName, - Listeners: []v1.Listener{ - { - Port: 80, - Protocol: v1.HTTPProtocolType, - AllowedRoutes: &v1.AllowedRoutes{ - Namespaces: &v1.RouteNamespaces{ - From: helpers.GetPointer(v1.NamespacesFromSelector), - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{ - "app": "allowed", + } + nsNoLabels = &apiv1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "no-labels", + }, + } + gw = &v1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gw", + }, + Spec: v1.GatewaySpec{ + GatewayClassName: gcName, + Listeners: []v1.Listener{ + { + Port: 80, + Protocol: v1.HTTPProtocolType, + AllowedRoutes: &v1.AllowedRoutes{ + Namespaces: &v1.RouteNamespaces{ + From: helpers.GetPointer(v1.NamespacesFromSelector), + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": "allowed", + }, }, }, }, }, }, }, - }, - } - processor := state.NewChangeProcessorImpl(state.ChangeProcessorConfig{ - GatewayCtlrName: controllerName, - GatewayClassName: gcName, - RelationshipCapturer: relationship.NewCapturerImpl(), - Logger: zap.New(), - Validators: createAlwaysValidValidators(), - Scheme: createScheme(), + } + processor = state.NewChangeProcessorImpl(state.ChangeProcessorConfig{ + GatewayCtlrName: controllerName, + GatewayClassName: gcName, + RelationshipCapturer: relationship.NewCapturerImpl(), + Logger: zap.New(), + Validators: createAlwaysValidValidators(), + Scheme: createScheme(), + }) + processor.CaptureUpsertChange(gc) + processor.CaptureUpsertChange(gw) + processor.Process() }) - processor.CaptureUpsertChange(gc) - processor.CaptureUpsertChange(gw) - processor.Process() - When("a namespace is created that is not linked to a listener", func() { It("does not trigger an update", func() { processor.CaptureUpsertChange(nsNoLabels) diff --git a/internal/mode/static/state/graph/graph.go b/internal/mode/static/state/graph/graph.go index d0b1e39472..c539886c48 100644 --- a/internal/mode/static/state/graph/graph.go +++ b/internal/mode/static/state/graph/graph.go @@ -53,7 +53,7 @@ type ProtectedPorts map[int32]string // IsReferenced returns true if the Graph references the resource. func (g *Graph) IsReferenced(resourceType client.Object, nsname types.NamespacedName) bool { - // FIMXE(pleshakov): For now, only works with Secrets and Namespaces. + // FIMXE(bjee19): For now, only works with Secrets and Namespaces. // Support EndpointSlices so that we can remove relationship.Capturer and use the Graph // as source to determine the relationships. // See https://github.com/nginxinc/nginx-gateway-fabric/issues/824 diff --git a/internal/mode/static/state/graph/namespace.go b/internal/mode/static/state/graph/namespace.go index 84b27b02c1..f33229b7a0 100644 --- a/internal/mode/static/state/graph/namespace.go +++ b/internal/mode/static/state/graph/namespace.go @@ -6,8 +6,8 @@ import ( "k8s.io/apimachinery/pkg/types" ) -// buildReferencedNamespaces returns a map of all the Namespace resources in the current clusterState with a label -// that matches any of the Gateway Listener's label selector. +// buildReferencedNamespaces returns a map of all the Namespace resources from the current clusterNamespaces with +// a label that matches any of the Gateway Listener's label selector. func buildReferencedNamespaces( clusterNamespaces map[types.NamespacedName]*v1.Namespace, gw *Gateway, @@ -24,7 +24,7 @@ func buildReferencedNamespaces( return referencedNamespaces } -// isNamespaceReferenced returns a boolean that represents whether a given Namespace resource has a label +// isNamespaceReferenced returns true if a given Namespace resource has a label // that matches any of the Gateway Listener's label selector. func isNamespaceReferenced(ns *v1.Namespace, gw *Gateway) bool { if gw == nil || ns == nil { From 131396b329c8ddea61e2f3b1e73d6c51f9f8796e Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Wed, 13 Dec 2023 14:59:01 -0800 Subject: [PATCH 22/27] Add small readability fixes --- internal/mode/static/state/graph/namespace.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/mode/static/state/graph/namespace.go b/internal/mode/static/state/graph/namespace.go index f33229b7a0..bc0c65b413 100644 --- a/internal/mode/static/state/graph/namespace.go +++ b/internal/mode/static/state/graph/namespace.go @@ -13,11 +13,13 @@ func buildReferencedNamespaces( gw *Gateway, ) map[types.NamespacedName]*v1.Namespace { referencedNamespaces := make(map[types.NamespacedName]*v1.Namespace) + for name, ns := range clusterNamespaces { if isNamespaceReferenced(ns, gw) { referencedNamespaces[name] = ns } } + if len(referencedNamespaces) == 0 { return nil } @@ -30,6 +32,7 @@ func isNamespaceReferenced(ns *v1.Namespace, gw *Gateway) bool { if gw == nil || ns == nil { return false } + nsLabels := ns.GetLabels() for _, listener := range gw.Listeners { if listener.AllowedRouteLabelSelector == nil { @@ -40,5 +43,6 @@ func isNamespaceReferenced(ns *v1.Namespace, gw *Gateway) bool { return true } } + return false } From dcf671e2b43af7cff33795c225b6d5da95139188 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Wed, 13 Dec 2023 15:01:00 -0800 Subject: [PATCH 23/27] Add small refactor to namespaces --- internal/mode/static/state/graph/namespace.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/mode/static/state/graph/namespace.go b/internal/mode/static/state/graph/namespace.go index bc0c65b413..481e4d749b 100644 --- a/internal/mode/static/state/graph/namespace.go +++ b/internal/mode/static/state/graph/namespace.go @@ -33,13 +33,13 @@ func isNamespaceReferenced(ns *v1.Namespace, gw *Gateway) bool { return false } - nsLabels := ns.GetLabels() + nsLabels := labels.Set(ns.GetLabels()) for _, listener := range gw.Listeners { if listener.AllowedRouteLabelSelector == nil { // Can have listeners with AllowedRouteLabelSelector not set. continue } - if listener.AllowedRouteLabelSelector.Matches(labels.Set(nsLabels)) { + if listener.AllowedRouteLabelSelector.Matches(nsLabels) { return true } } From 919a73e903c3faf0d1f89a93e49e480ca77376fa Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Wed, 13 Dec 2023 15:25:41 -0800 Subject: [PATCH 24/27] Add default case for IsReferenced --- internal/mode/static/state/graph/graph_test.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/internal/mode/static/state/graph/graph_test.go b/internal/mode/static/state/graph/graph_test.go index 56f882dd58..9eb87a1494 100644 --- a/internal/mode/static/state/graph/graph_test.go +++ b/internal/mode/static/state/graph/graph_test.go @@ -476,6 +476,12 @@ func TestIsReferenced(t *testing.T) { graph: graph, expected: false, }, + { + name: "Namespace not in ReferencedNamespaces but in Gateway Listener's AllowedRouteLabelSelector passes", + resource: nsNotInGraphButInGateway, + graph: graph, + expected: true, + }, { name: "Secret in graph's ReferencedSecrets passes", resource: baseSecret, @@ -495,10 +501,10 @@ func TestIsReferenced(t *testing.T) { expected: false, }, { - name: "Namespace not in ReferencedNamespaces but in Gateway Listener's AllowedRouteLabelSelector passes", - resource: nsNotInGraphButInGateway, + name: "Resource is not supported by IsReferenced", + resource: &v1.Service{}, graph: graph, - expected: true, + expected: false, }, } for _, test := range tests { From 98fed71d6fd3ad04474c958a2ef7ea851fd45da5 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Thu, 14 Dec 2023 13:31:29 -0800 Subject: [PATCH 25/27] Update Listener spec --- .../mode/static/state/graph/graph_test.go | 5 ++- .../mode/static/state/graph/namespace_test.go | 44 ++++++++++++------- 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/internal/mode/static/state/graph/graph_test.go b/internal/mode/static/state/graph/graph_test.go index 9eb87a1494..bc705d3d7d 100644 --- a/internal/mode/static/state/graph/graph_test.go +++ b/internal/mode/static/state/graph/graph_test.go @@ -428,8 +428,9 @@ func TestIsReferenced(t *testing.T) { } gw := &Gateway{ - Listeners: map[string]*Listener{ - "listener-1": { + Listeners: []*Listener{ + { + Name: "listener-1", Valid: true, AllowedRouteLabelSelector: labels.SelectorFromSet(map[string]string{"apples": "oranges"}), }, diff --git a/internal/mode/static/state/graph/namespace_test.go b/internal/mode/static/state/graph/namespace_test.go index f2358eaca8..4ab4d05adf 100644 --- a/internal/mode/static/state/graph/namespace_test.go +++ b/internal/mode/static/state/graph/namespace_test.go @@ -49,8 +49,9 @@ func TestBuildReferencedNamespaces(t *testing.T) { }{ { gw: &Gateway{ - Listeners: map[string]*Listener{ - "listener-1": { + Listeners: []*Listener{ + { + Name: "listener-2", Valid: true, AllowedRouteLabelSelector: labels.SelectorFromSet(map[string]string{"apples": "oranges"}), }, @@ -64,12 +65,14 @@ func TestBuildReferencedNamespaces(t *testing.T) { }, { gw: &Gateway{ - Listeners: map[string]*Listener{ - "listener-1": { + Listeners: []*Listener{ + { + Name: "listener-1", Valid: true, AllowedRouteLabelSelector: labels.SelectorFromSet(map[string]string{"apples": "oranges"}), }, - "listener-2": { + { + Name: "listener-2", Valid: true, AllowedRouteLabelSelector: labels.SelectorFromSet(map[string]string{"peaches": "bananas"}), }, @@ -84,7 +87,7 @@ func TestBuildReferencedNamespaces(t *testing.T) { }, { gw: &Gateway{ - Listeners: map[string]*Listener{}, + Listeners: []*Listener{}, Valid: true, }, expectedRefNS: nil, @@ -92,11 +95,13 @@ func TestBuildReferencedNamespaces(t *testing.T) { }, { gw: &Gateway{ - Listeners: map[string]*Listener{ - "listener-1": { + Listeners: []*Listener{ + { + Name: "listener-1", Valid: true, }, - "listener-2": { + { + Name: "listener-2", Valid: true, }, }, @@ -107,8 +112,9 @@ func TestBuildReferencedNamespaces(t *testing.T) { }, { gw: &Gateway{ - Listeners: map[string]*Listener{ - "listener-1": { + Listeners: []*Listener{ + { + Name: "listener-1", Valid: true, AllowedRouteLabelSelector: labels.SelectorFromSet(map[string]string{"not": "matching"}), }, @@ -121,12 +127,14 @@ func TestBuildReferencedNamespaces(t *testing.T) { }, { gw: &Gateway{ - Listeners: map[string]*Listener{ - "listener-1": { + Listeners: []*Listener{ + { + Name: "listener-1", Valid: true, AllowedRouteLabelSelector: labels.SelectorFromSet(map[string]string{"apples": "oranges"}), }, - "listener-2": { + { + Name: "listener-2", Valid: true, AllowedRouteLabelSelector: labels.SelectorFromSet(map[string]string{"not": "matching"}), }, @@ -140,12 +148,14 @@ func TestBuildReferencedNamespaces(t *testing.T) { }, { gw: &Gateway{ - Listeners: map[string]*Listener{ - "listener-1": { + Listeners: []*Listener{ + { + Name: "listener-1", Valid: true, AllowedRouteLabelSelector: labels.SelectorFromSet(map[string]string{"apples": "oranges"}), }, - "listener-2": { + { + Name: "listener-2", Valid: true, }, }, From 335a51726c8ac6bfa72b051c92e8298796c27f42 Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Fri, 15 Dec 2023 10:12:26 -0800 Subject: [PATCH 26/27] Remove unncessary test cases --- internal/mode/static/state/change_processor_test.go | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/internal/mode/static/state/change_processor_test.go b/internal/mode/static/state/change_processor_test.go index 12da0faa69..56c6722bd2 100644 --- a/internal/mode/static/state/change_processor_test.go +++ b/internal/mode/static/state/change_processor_test.go @@ -1412,21 +1412,13 @@ var _ = Describe("ChangeProcessor", func() { }) When("a gateway changes its listener's labels", func() { It("triggers an update when a namespace that matches the new labels is created", func() { - processor.CaptureUpsertChange(ns) - changed, _ := processor.Process() - Expect(changed).To(BeTrue()) - - processor.CaptureUpsertChange(nsDifferentLabels) - changed, _ = processor.Process() - Expect(changed).To(BeFalse()) - gwChangedLabel := gw.DeepCopy() gwChangedLabel.Spec.Listeners[0].AllowedRoutes.Namespaces.Selector.MatchLabels = map[string]string{ "oranges": "bananas", } gwChangedLabel.Generation++ processor.CaptureUpsertChange(gwChangedLabel) - changed, _ = processor.Process() + changed, _ := processor.Process() Expect(changed).To(BeTrue()) // After changing the gateway's labels and generation, the processor should be marked to update From 51598933090aa31b3d1c904e0cc41e43c8aee5ab Mon Sep 17 00:00:00 2001 From: Benjamin Jee Date: Fri, 15 Dec 2023 10:35:58 -0800 Subject: [PATCH 27/27] Add tests for isNamespaceReferenced --- .../mode/static/state/graph/namespace_test.go | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/internal/mode/static/state/graph/namespace_test.go b/internal/mode/static/state/graph/namespace_test.go index 4ab4d05adf..8781fc6f8c 100644 --- a/internal/mode/static/state/graph/namespace_test.go +++ b/internal/mode/static/state/graph/namespace_test.go @@ -175,3 +175,46 @@ func TestBuildReferencedNamespaces(t *testing.T) { }) } } + +func TestIsNamespaceReferenced(t *testing.T) { + tests := []struct { + ns *v1.Namespace + gw *Gateway + name string + exp bool + }{ + { + ns: nil, + gw: nil, + exp: false, + name: "namespace and gateway are nil", + }, + { + ns: &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ns1", + }, + }, + gw: nil, + exp: false, + name: "namespace is valid but gateway is nil", + }, + { + ns: nil, + gw: &Gateway{ + Listeners: []*Listener{}, + Valid: true, + }, + exp: false, + name: "gateway is valid but namespace is nil", + }, + } + + // Other test cases should be covered by testing of BuildReferencedNamespaces + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + g.Expect(isNamespaceReferenced(test.ns, test.gw)).To(Equal(test.exp)) + }) + } +}