Skip to content

Commit 2a248f3

Browse files
committed
feat: implement deep comparison between network rules
we don't rely anymore on `reflect.DeepEqual`. For now we do a manual comparsion of the inner fields, maybe in the future we can improve the solution using an hash key or similar approaches Signed-off-by: Andrea Terzolo <andrea.terzolo@suse.com>
1 parent e826f86 commit 2a248f3

3 files changed

Lines changed: 543 additions & 46 deletions

File tree

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ vet: ## Run go vet against code.
7575

7676
.PHONY: test
7777
test: manifests generate fmt vet setup-envtest ## Run tests.
78-
KUBEBUILDER_ASSETS="$(shell "$(ENVTEST)" use $(ENVTEST_K8S_VERSION) --bin-dir "$(LOCALBIN)" -p path)" go test $$(go list ./... | grep -v /e2e) -coverprofile cover.out
78+
KUBEBUILDER_ASSETS="$(shell "$(ENVTEST)" use $(ENVTEST_K8S_VERSION) --bin-dir "$(LOCALBIN)" -p path)" go test $$(go list ./... | grep -v /e2e) -count=1 -coverprofile cover.out
7979

8080
# TODO(user): To use a different vendor for e2e tests, modify the setup under 'tests/e2e'.
8181
# The default setup assumes Kind is pre-installed and builds/loads the Manager Docker image locally.

internal/controller/topology_scanner.go

Lines changed: 100 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ import (
2020
"secuity.rancher.io/network-enforcer/internal/topology"
2121
)
2222

23+
const (
24+
namespaceLabelKey = "kubernetes.io/metadata.name"
25+
)
26+
2327
type TopologyScanner struct {
2428
client client.Client
2529
store *topology.Store
@@ -178,29 +182,75 @@ func (ts *TopologyScanner) reconcileProposal(
178182
return nil
179183
}
180184

181-
func findEgressPeer(
182-
newRule networkingv1.NetworkPolicyEgressRule,
183-
existing []networkingv1.NetworkPolicyEgressRule,
184-
) bool {
185-
for _, peer := range existing {
186-
// todo!: this should be fixed, the order of the internal slices could be different
187-
if reflect.DeepEqual(peer, newRule) {
185+
func containsRule[T any](newRule T, existing []T, equalFn func(T, T) bool) bool {
186+
for _, rule := range existing {
187+
if equalFn(newRule, rule) {
188188
return true
189189
}
190190
}
191191
return false
192192
}
193193

194-
func findIngressPeer(
195-
newRule networkingv1.NetworkPolicyIngressRule,
196-
existing []networkingv1.NetworkPolicyIngressRule,
197-
) bool {
198-
for _, peer := range existing {
199-
if reflect.DeepEqual(peer, newRule) {
200-
return true
194+
func selectorEqual(a, b *metav1.LabelSelector) bool {
195+
return metav1.FormatLabelSelector(a) == metav1.FormatLabelSelector(b)
196+
}
197+
198+
func ipBlockEqual(a, b *networkingv1.IPBlock) bool {
199+
if a == nil || b == nil {
200+
return a == b
201+
}
202+
if a.CIDR != b.CIDR {
203+
return false
204+
}
205+
return equalUnordered(a.Except, b.Except, func(left, right string) bool {
206+
return left == right
207+
})
208+
}
209+
210+
func policyPeerEqual(a, b networkingv1.NetworkPolicyPeer) bool {
211+
return selectorEqual(a.NamespaceSelector, b.NamespaceSelector) &&
212+
selectorEqual(a.PodSelector, b.PodSelector) &&
213+
ipBlockEqual(a.IPBlock, b.IPBlock)
214+
}
215+
216+
func policyPortEqual(a, b networkingv1.NetworkPolicyPort) bool {
217+
return reflect.DeepEqual(a, b)
218+
}
219+
220+
func equalUnordered[T any](left, right []T, equalFn func(T, T) bool) bool {
221+
if len(left) != len(right) {
222+
return false
223+
}
224+
225+
used := make([]bool, len(right))
226+
for _, leftItem := range left {
227+
matched := false
228+
for i, rightItem := range right {
229+
if used[i] {
230+
continue
231+
}
232+
if equalFn(leftItem, rightItem) {
233+
used[i] = true
234+
matched = true
235+
break
236+
}
237+
}
238+
if !matched {
239+
return false
201240
}
202241
}
203-
return false
242+
243+
return true
244+
}
245+
246+
func egressRuleEqual(a, b networkingv1.NetworkPolicyEgressRule) bool {
247+
return equalUnordered(a.To, b.To, policyPeerEqual) &&
248+
equalUnordered(a.Ports, b.Ports, policyPortEqual)
249+
}
250+
251+
func ingressRuleEqual(a, b networkingv1.NetworkPolicyIngressRule) bool {
252+
return equalUnordered(a.From, b.From, policyPeerEqual) &&
253+
equalUnordered(a.Ports, b.Ports, policyPortEqual)
204254
}
205255

206256
func (ts *TopologyScanner) buildSpec(
@@ -216,7 +266,7 @@ func (ts *TopologyScanner) buildSpec(
216266
return err
217267
}
218268
for _, rule := range deltaRules {
219-
if findEgressPeer(rule, spec.Egress) {
269+
if containsRule(rule, spec.Egress, egressRuleEqual) {
220270
continue
221271
}
222272
spec.Egress = append(spec.Egress, rule)
@@ -228,7 +278,7 @@ func (ts *TopologyScanner) buildSpec(
228278
}
229279

230280
for _, rule := range deltaRules {
231-
if findIngressPeer(rule, spec.Ingress) {
281+
if containsRule(rule, spec.Ingress, ingressRuleEqual) {
232282
continue
233283
}
234284
spec.Ingress = append(spec.Ingress, rule)
@@ -248,25 +298,14 @@ func (ts *TopologyScanner) buildEgressRules(
248298

249299
rules := make([]networkingv1.NetworkPolicyEgressRule, 0, len(peerList))
250300
for _, peer := range peerList {
251-
peerSelector, err := selectorFromWorkloadKey(ctx, ts.client, peer.WorkloadKey)
301+
policyPeer, policyPort, err := ts.buildPeerRuleParts(ctx, peer)
252302
if err != nil {
253303
return nil, fmt.Errorf("resolving egress peer selector: %w", err)
254304
}
255305

256-
port := intstr.FromInt32(peer.DstPort)
257306
rules = append(rules, networkingv1.NetworkPolicyEgressRule{
258-
To: []networkingv1.NetworkPolicyPeer{{
259-
NamespaceSelector: &metav1.LabelSelector{
260-
MatchLabels: map[string]string{
261-
"kubernetes.io/metadata.name": peer.Namespace,
262-
},
263-
},
264-
PodSelector: &peerSelector,
265-
}},
266-
Ports: []networkingv1.NetworkPolicyPort{{
267-
Protocol: &peer.Protocol,
268-
Port: &port,
269-
}},
307+
To: []networkingv1.NetworkPolicyPeer{policyPeer},
308+
Ports: []networkingv1.NetworkPolicyPort{policyPort},
270309
})
271310
}
272311

@@ -281,27 +320,43 @@ func (ts *TopologyScanner) buildIngressRules(
281320

282321
rules := make([]networkingv1.NetworkPolicyIngressRule, 0, len(peerList))
283322
for _, peer := range peerList {
284-
peerSelector, err := selectorFromWorkloadKey(ctx, ts.client, peer.WorkloadKey)
323+
policyPeer, policyPort, err := ts.buildPeerRuleParts(ctx, peer)
285324
if err != nil {
286325
return nil, fmt.Errorf("resolving ingress peer selector: %w", err)
287326
}
288327

289-
port := intstr.FromInt32(peer.DstPort)
290328
rules = append(rules, networkingv1.NetworkPolicyIngressRule{
291-
From: []networkingv1.NetworkPolicyPeer{{
292-
NamespaceSelector: &metav1.LabelSelector{
293-
MatchLabels: map[string]string{
294-
"kubernetes.io/metadata.name": peer.Namespace,
295-
},
296-
},
297-
PodSelector: &peerSelector,
298-
}},
299-
Ports: []networkingv1.NetworkPolicyPort{{
300-
Protocol: &peer.Protocol,
301-
Port: &port,
302-
}},
329+
From: []networkingv1.NetworkPolicyPeer{policyPeer},
330+
Ports: []networkingv1.NetworkPolicyPort{policyPort},
303331
})
304332
}
305333

306334
return rules, nil
307335
}
336+
337+
func (ts *TopologyScanner) buildPeerRuleParts(
338+
ctx context.Context,
339+
peer topology.Peer,
340+
) (networkingv1.NetworkPolicyPeer, networkingv1.NetworkPolicyPort, error) {
341+
peerSelector, err := selectorFromWorkloadKey(ctx, ts.client, peer.WorkloadKey)
342+
if err != nil {
343+
return networkingv1.NetworkPolicyPeer{}, networkingv1.NetworkPolicyPort{}, err
344+
}
345+
346+
port := intstr.FromInt32(peer.DstPort)
347+
348+
policyPeer := networkingv1.NetworkPolicyPeer{
349+
NamespaceSelector: &metav1.LabelSelector{
350+
MatchLabels: map[string]string{
351+
namespaceLabelKey: peer.Namespace,
352+
},
353+
},
354+
PodSelector: &peerSelector,
355+
}
356+
policyPort := networkingv1.NetworkPolicyPort{
357+
Protocol: &peer.Protocol,
358+
Port: &port,
359+
}
360+
361+
return policyPeer, policyPort, nil
362+
}

0 commit comments

Comments
 (0)