Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Namespace resources are tracked by graph #1320

Merged
merged 27 commits into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
46edfcf
Add namespace changes and adjust test
bjee19 Nov 29, 2023
c77304d
Remove namespace and gateway from capturer and tests
bjee19 Nov 29, 2023
49e00c2
Add test cases in graph_test
bjee19 Nov 30, 2023
56233e6
Add nit change in graph_test
bjee19 Nov 30, 2023
cf77769
Add some comments and some changes to namespace
bjee19 Dec 1, 2023
42cbf60
Add small function name change
bjee19 Dec 1, 2023
46baf27
Replace Namespace resource with Kubernetes resource
bjee19 Dec 4, 2023
d1762cd
Add more change processor tests
bjee19 Dec 4, 2023
36181a6
Add namespace tests and more test cases to change processor
bjee19 Dec 6, 2023
116eae5
Add predicate function for namespace
bjee19 Dec 6, 2023
7b4b9ee
Add small change to comments
bjee19 Dec 6, 2023
8471179
Add nit grammar change
bjee19 Dec 6, 2023
990d1c5
Fix test description typo
bjee19 Dec 12, 2023
25e8b6d
Refactor function name to add more description
bjee19 Dec 12, 2023
2b8e462
Simplify IsReferenced test
bjee19 Dec 12, 2023
652cb3b
Add subtests
bjee19 Dec 12, 2023
1b60dd8
Add comments for test result and small fix to function parameters
bjee19 Dec 13, 2023
c0911be
Check changed in change process test for gateway
bjee19 Dec 13, 2023
04233e0
Add more namespace test cases
bjee19 Dec 13, 2023
8dc4787
Refactor change processor tests to make them ordered
bjee19 Dec 13, 2023
876f97c
Add small fixes to comments and wrapping BeforeAll function in change…
bjee19 Dec 13, 2023
131396b
Add small readability fixes
bjee19 Dec 13, 2023
dcf671e
Add small refactor to namespaces
bjee19 Dec 13, 2023
919a73e
Add default case for IsReferenced
bjee19 Dec 13, 2023
98fed71
Update Listener spec
bjee19 Dec 14, 2023
335a517
Remove unncessary test cases
bjee19 Dec 15, 2023
5159893
Add tests for isNamespaceReferenced
bjee19 Dec 15, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal/mode/static/state/change_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}),
Expand Down
169 changes: 141 additions & 28 deletions internal/mode/static/state/change_processor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
kate-osborn marked this conversation as resolved.
Show resolved Hide resolved

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())
})
})
})
})

Expand Down
27 changes: 24 additions & 3 deletions internal/mode/static/state/graph/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,22 +44,40 @@ 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.
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
bjee19 marked this conversation as resolved.
Show resolved Hide resolved
// 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
}
Expand Down Expand Up @@ -92,13 +110,16 @@ func BuildGraph(
bindRoutesToListeners(routes, gw, state.Namespaces)
addBackendRefsToRouteRules(routes, refGrantResolver, state.Services)

referencedNamespaces := buildReferencedNamespaces(state.Namespaces, gw)

g := &Graph{
GatewayClass: gc,
Gateway: gw,
Routes: routes,
IgnoredGatewayClasses: processedGwClasses.Ignored,
IgnoredGateways: processedGws.Ignored,
ReferencedSecrets: secretResolver.getResolvedSecrets(),
ReferencedNamespaces: referencedNamespaces,
}

return g
Expand Down
Loading
Loading