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{}), diff --git a/internal/mode/static/state/change_processor_test.go b/internal/mode/static/state/change_processor_test.go index 31837c7d27..56c6722bd2 100644 --- a/internal/mode/static/state/change_processor_test.go +++ b/internal/mode/static/state/change_processor_test.go @@ -1293,53 +1293,166 @@ 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", - }, + 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", }, - } - 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{ - Listeners: []v1.Listener{ - { - 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.CaptureUpsertChange(gw) + 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.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.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 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 linked to a listener has its labels changed to no longer match a listener", func() { + It("triggers an update", func() { + nsDifferentLabels.Labels = map[string]string{ + "oranges": "bananas", + } + processor.CaptureUpsertChange(nsDifferentLabels) + changed, _ := processor.Process() + 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() { + gwChangedLabel := gw.DeepCopy() + gwChangedLabel.Spec.Listeners[0].AllowedRoutes.Namespaces.Selector.MatchLabels = map[string]string{ + "oranges": "bananas", + } + gwChangedLabel.Generation++ + processor.CaptureUpsertChange(gwChangedLabel) changed, _ := processor.Process() Expect(changed).To(BeTrue()) - newNS := ns.DeepCopy() - newNS.Labels = nil - processor.CaptureUpsertChange(newNS) + // 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()) + processor.CaptureUpsertChange(nsDifferentLabels) 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() { + ns.Labels = nil + processor.CaptureUpsertChange(ns) + 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()) + }) + }) }) }) diff --git a/internal/mode/static/state/graph/graph.go b/internal/mode/static/state/graph/graph.go index ae9f5f8e34..c539886c48 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 with labels that 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. @@ -51,15 +53,31 @@ 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(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 - switch resourceType.(type) { + switch obj := resourceType.(type) { case *v1.Secret: _, exists := g.ReferencedSecrets[nsname] return exists + case *v1.Namespace: + // `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. + // + // 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. + // + // `exists` does not cover the case highlighted above by `existed` and vice versa so both are needed. + + _, existed := g.ReferencedNamespaces[nsname] + exists := isNamespaceReferenced(obj, g.Gateway) + return existed || exists default: return false } @@ -92,6 +110,8 @@ func BuildGraph( bindRoutesToListeners(routes, gw, state.Namespaces) addBackendRefsToRouteRules(routes, refGrantResolver, state.Services) + referencedNamespaces := buildReferencedNamespaces(state.Namespaces, gw) + g := &Graph{ GatewayClass: gc, Gateway: gw, @@ -99,6 +119,7 @@ func BuildGraph( IgnoredGatewayClasses: processedGwClasses.Ignored, IgnoredGateways: processedGws.Ignored, ReferencedSecrets: secretResolver.getResolvedSecrets(), + 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 cae15a3722..bc705d3d7d 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,9 @@ func TestBuildGraph(t *testing.T) { Source: secret, }, }, + ReferencedNamespaces: map[types.NamespacedName]*v1.Namespace{ + client.ObjectKeyFromObject(ns): ns, + }, } } @@ -362,3 +389,130 @@ func TestBuildGraph(t *testing.T) { }) } } + +func TestIsReferenced(t *testing.T) { + baseSecret := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "secret", + }, + } + sameNamespaceDifferentNameSecret := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test", + Name: "secret-different-name", + }, + } + differentNamespaceSameNameSecret := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "test-different-namespace", + Name: "secret", + }, + } + + 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: []*Listener{ + { + Name: "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]*v1.Namespace{ + client.ObjectKeyFromObject(nsInGraph): nsInGraph, + }, + } + + tests := []struct { + graph *Graph + resource client.Object + name string + expected bool + }{ + { + name: "Namespace in graph's ReferencedNamespaces passes", + resource: nsInGraph, + graph: graph, + expected: true, + }, + { + name: "Namespace with a different name but same labels fails", + resource: nsNotInGraph, + 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, + graph: graph, + expected: true, + }, + { + name: "Secret not in ReferencedSecrets with same Namespace and different Name fails", + resource: sameNamespaceDifferentNameSecret, + graph: graph, + expected: false, + }, + { + name: "Secret not in ReferencedSecrets with different Namespace and same Name fails", + resource: differentNamespaceSameNameSecret, + graph: graph, + expected: false, + }, + { + name: "Resource is not supported by IsReferenced", + resource: &v1.Service{}, + graph: graph, + expected: false, + }, + } + for _, test := range tests { + 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.go b/internal/mode/static/state/graph/namespace.go new file mode 100644 index 0000000000..481e4d749b --- /dev/null +++ b/internal/mode/static/state/graph/namespace.go @@ -0,0 +1,48 @@ +package graph + +import ( + v1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/types" +) + +// 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, +) 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 + } + return referencedNamespaces +} + +// 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 { + return false + } + + 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(nsLabels) { + return true + } + } + + return false +} 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..8781fc6f8c --- /dev/null +++ b/internal/mode/static/state/graph/namespace_test.go @@ -0,0 +1,220 @@ +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: []*Listener{ + { + Name: "listener-2", + 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: []*Listener{ + { + Name: "listener-1", + Valid: true, + AllowedRouteLabelSelector: labels.SelectorFromSet(map[string]string{"apples": "oranges"}), + }, + { + Name: "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: []*Listener{}, + Valid: true, + }, + expectedRefNS: nil, + name: "gateway has no Listeners", + }, + { + gw: &Gateway{ + Listeners: []*Listener{ + { + Name: "listener-1", + Valid: true, + }, + { + Name: "listener-2", + Valid: true, + }, + }, + Valid: true, + }, + expectedRefNS: nil, + name: "gateway has multiple listeners with no AllowedRouteLabelSelector set", + }, + { + gw: &Gateway{ + Listeners: []*Listener{ + { + Name: "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: []*Listener{ + { + Name: "listener-1", + Valid: true, + AllowedRouteLabelSelector: labels.SelectorFromSet(map[string]string{"apples": "oranges"}), + }, + { + Name: "listener-2", + Valid: true, + AllowedRouteLabelSelector: labels.SelectorFromSet(map[string]string{"not": "matching"}), + }, + }, + Valid: true, + }, + expectedRefNS: map[types.NamespacedName]*v1.Namespace{ + {Name: "ns2"}: ns2, + }, + name: "gateway has two listeners and only matches labels with one namespace", + }, + { + gw: &Gateway{ + Listeners: []*Listener{ + { + Name: "listener-1", + Valid: true, + AllowedRouteLabelSelector: labels.SelectorFromSet(map[string]string{"apples": "oranges"}), + }, + { + Name: "listener-2", + Valid: true, + }, + }, + Valid: true, + }, + 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", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + g := NewWithT(t) + g.Expect(buildReferencedNamespaces(clusterNamespaces, test.gw)).To(Equal(test.expectedRefNS)) + }) + } +} + +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)) + }) + } +} diff --git a/internal/mode/static/state/relationship/capturer.go b/internal/mode/static/state/relationship/capturer.go index 48d08250d0..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,9 +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 @@ -225,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()