From 441316ab9d586919a79d15b1a73be4ec0b9ab8ac Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Mon, 6 Jan 2025 18:21:05 -0800 Subject: [PATCH 1/7] feat: global store for non-prometheus metrics with cidr and named port counters Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- npm/metrics/counts.go | 66 +++++++++++++++++++++++++++++++ npm/metrics/prometheus-metrics.go | 3 ++ 2 files changed, 69 insertions(+) create mode 100644 npm/metrics/counts.go diff --git a/npm/metrics/counts.go b/npm/metrics/counts.go new file mode 100644 index 0000000000..1f2216f33d --- /dev/null +++ b/npm/metrics/counts.go @@ -0,0 +1,66 @@ +package metrics + +import "sync" + +var nonPrometheusCounts *counts + +// counts is a struct holding non-Prometheus counts. +type counts struct { + sync.Mutex + cidrNetPols int + namedPortNetPols int +} + +func IncCidrNetPols() { + if nonPrometheusCounts == nil { + return + } + nonPrometheusCounts.Lock() + defer nonPrometheusCounts.Unlock() + nonPrometheusCounts.cidrNetPols++ +} + +func DecCidrNetPols() { + if nonPrometheusCounts == nil { + return + } + nonPrometheusCounts.Lock() + defer nonPrometheusCounts.Unlock() + nonPrometheusCounts.cidrNetPols-- +} + +func GetCidrNetPols() int { + if nonPrometheusCounts == nil { + return 0 + } + nonPrometheusCounts.Lock() + defer nonPrometheusCounts.Unlock() + return nonPrometheusCounts.cidrNetPols +} + +func IncNamedPortNetPols() { + if nonPrometheusCounts == nil { + return + } + nonPrometheusCounts.Lock() + defer nonPrometheusCounts.Unlock() + nonPrometheusCounts.namedPortNetPols++ +} + +func DecNamedPortNetPols() { + if nonPrometheusCounts == nil { + return + } + nonPrometheusCounts.Lock() + defer nonPrometheusCounts.Unlock() + nonPrometheusCounts.namedPortNetPols-- +} + +func GetNamedPortNetPols() int { + if nonPrometheusCounts == nil { + return 0 + } + nonPrometheusCounts.Lock() + defer nonPrometheusCounts.Unlock() + return nonPrometheusCounts.namedPortNetPols +} diff --git a/npm/metrics/prometheus-metrics.go b/npm/metrics/prometheus-metrics.go index 4dcc07653f..dadf2c9f17 100644 --- a/npm/metrics/prometheus-metrics.go +++ b/npm/metrics/prometheus-metrics.go @@ -162,6 +162,9 @@ func InitializeAll() { return } + // initialize global variable (see counts.go) + nonPrometheusCounts = &counts{} + initializeDaemonMetrics() initializeControllerMetrics() From 442bad620f836d0a85f475f33e219fcff6dced28 Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Mon, 6 Jan 2025 18:22:54 -0800 Subject: [PATCH 2/7] feat: heartbeat log includes counts for cidr and named port netpols Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- npm/metrics/ai-utils.go | 13 +++--- .../controllers/v2/networkPolicyController.go | 34 ++++++++++++++-- .../translation/translatePolicy.go | 40 +++++++++++++++++++ 3 files changed, 79 insertions(+), 8 deletions(-) diff --git a/npm/metrics/ai-utils.go b/npm/metrics/ai-utils.go index 20de3009ff..fec64bb9a2 100644 --- a/npm/metrics/ai-utils.go +++ b/npm/metrics/ai-utils.go @@ -100,13 +100,16 @@ func SendLog(operationID int, msg string, printLog bool) { } func SendHeartbeatWithNumPolicies() { - var message string numPolicies, err := GetNumPolicies() - if err == nil { - message = fmt.Sprintf("info: NPM heartbeat. Current num policies: %d", numPolicies) + numPoliciesString := "unknown" + if err != nil { + klog.Warningf("warn: NPM hearbeat. Couldn't get number of policies for telemetry log: %s", err.Error()) } else { - message = fmt.Sprintf("warn: NPM hearbeat. Couldn't get number of policies for telemetry log: %v", err) - klog.Warning(message) + numPoliciesString = fmt.Sprint(numPolicies) } + + cidrNetPols := GetCidrNetPols() + namedPortNetPols := GetNamedPortNetPols() + message := fmt.Sprintf("info: NPM hearbeat. Total policies: %s, CIDR policies: %d, NamedPort policies: %d", numPoliciesString, cidrNetPols, namedPortNetPols) SendLog(util.NpmID, message, DonotPrint) } diff --git a/npm/pkg/controlplane/controllers/v2/networkPolicyController.go b/npm/pkg/controlplane/controllers/v2/networkPolicyController.go index cb14840c67..3b26d7b3ee 100644 --- a/npm/pkg/controlplane/controllers/v2/networkPolicyController.go +++ b/npm/pkg/controlplane/controllers/v2/networkPolicyController.go @@ -301,10 +301,14 @@ func (c *NetworkPolicyController) syncAddAndUpdateNetPol(netPolObj *networkingv1 return metrics.NoOp, nil } - _, policyExisted := c.rawNpSpecMap[netpolKey] + oldNetPolSpec, policyExisted := c.rawNpSpecMap[netpolKey] + hadCIDR := false + hadNamedPort := false var operationKind metrics.OperationKind if policyExisted { operationKind = metrics.UpdateOp + hadCIDR = translation.HasCIDRBlock(oldNetPolSpec) + hadNamedPort = translation.HasNamedPort(oldNetPolSpec) } else { operationKind = metrics.CreateOp } @@ -320,9 +324,25 @@ func (c *NetworkPolicyController) syncAddAndUpdateNetPol(netPolObj *networkingv1 return operationKind, fmt.Errorf("[syncAddAndUpdateNetPol] Error: failed to update translated NPMNetworkPolicy into Dataplane due to %w", err) } - if !policyExisted { + if policyExisted { + if hadCIDR && !translation.HasCIDRBlock(&netPolObj.Spec) { + metrics.DecCidrNetPols() + } + + if hadNamedPort && !translation.HasNamedPort(&netPolObj.Spec) { + metrics.DecNamedPortNetPols() + } + } else { // inc metric for NumPolicies only if it a new network policy metrics.IncNumPolicies() + + if translation.HasCIDRBlock(&netPolObj.Spec) { + metrics.IncCidrNetPols() + } + + if translation.HasNamedPort(&netPolObj.Spec) { + metrics.IncNamedPortNetPols() + } } c.rawNpSpecMap[netpolKey] = &netPolObj.Spec @@ -331,7 +351,7 @@ func (c *NetworkPolicyController) syncAddAndUpdateNetPol(netPolObj *networkingv1 // DeleteNetworkPolicy handles deleting network policy based on netPolKey. func (c *NetworkPolicyController) cleanUpNetworkPolicy(netPolKey string) error { - _, cachedNetPolObjExists := c.rawNpSpecMap[netPolKey] + cachedNetPolSpec, cachedNetPolObjExists := c.rawNpSpecMap[netPolKey] // if there is no applied network policy with the netPolKey, do not need to clean up process. if !cachedNetPolObjExists { return nil @@ -342,6 +362,14 @@ func (c *NetworkPolicyController) cleanUpNetworkPolicy(netPolKey string) error { return fmt.Errorf("[cleanUpNetworkPolicy] Error: failed to remove policy due to %w", err) } + if translation.HasCIDRBlock(cachedNetPolSpec) { + metrics.DecCidrNetPols() + } + + if translation.HasNamedPort(cachedNetPolSpec) { + metrics.DecNamedPortNetPols() + } + // Success to clean up ipset and iptables operations in kernel and delete the cached network policy from RawNpMap delete(c.rawNpSpecMap, netPolKey) metrics.DecNumPolicies() diff --git a/npm/pkg/controlplane/translation/translatePolicy.go b/npm/pkg/controlplane/translation/translatePolicy.go index 9b029b4616..c1bb4890b3 100644 --- a/npm/pkg/controlplane/translation/translatePolicy.go +++ b/npm/pkg/controlplane/translation/translatePolicy.go @@ -684,3 +684,43 @@ func checkOnlyPortRuleExists( } return nil } + +func HasCIDRBlock(netPolSpec *networkingv1.NetworkPolicySpec) bool { + for _, ingress := range netPolSpec.Ingress { + for _, from := range ingress.From { + if from.IPBlock != nil && from.IPBlock.CIDR != "" { + return true + } + } + } + + for _, egress := range netPolSpec.Egress { + for _, to := range egress.To { + if to.IPBlock != nil && to.IPBlock.CIDR != "" { + return true + } + } + } + + return false +} + +func HasNamedPort(netPolObj *networkingv1.NetworkPolicySpec) bool { + for _, ingress := range netPolObj.Ingress { + for _, port := range ingress.Ports { + if t, err := portType(port); err != nil && t == namedPortType { + return true + } + } + } + + for _, egress := range netPolObj.Egress { + for _, port := range egress.Ports { + if t, err := portType(port); err != nil && t == namedPortType { + return true + } + } + } + + return false +} From 86a0da366035f05f694060391cc05029b9a09c18 Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Tue, 7 Jan 2025 10:15:52 -0800 Subject: [PATCH 3/7] perf: string conversion lint Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- npm/metrics/ai-utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/npm/metrics/ai-utils.go b/npm/metrics/ai-utils.go index fec64bb9a2..a9aa0ce661 100644 --- a/npm/metrics/ai-utils.go +++ b/npm/metrics/ai-utils.go @@ -105,7 +105,7 @@ func SendHeartbeatWithNumPolicies() { if err != nil { klog.Warningf("warn: NPM hearbeat. Couldn't get number of policies for telemetry log: %s", err.Error()) } else { - numPoliciesString = fmt.Sprint(numPolicies) + numPoliciesString = strconv.Itoa(numPolicies) } cidrNetPols := GetCidrNetPols() From 29137324db2569c8915a651fdc84a1ebf144d5e3 Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Tue, 7 Jan 2025 16:27:14 -0800 Subject: [PATCH 4/7] fix: policy count logic and log typo Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- npm/metrics/ai-utils.go | 4 ++-- .../controllers/v2/networkPolicyController.go | 20 ++++++++----------- .../translation/translatePolicy.go | 4 ++-- 3 files changed, 12 insertions(+), 16 deletions(-) diff --git a/npm/metrics/ai-utils.go b/npm/metrics/ai-utils.go index a9aa0ce661..293c7c12ec 100644 --- a/npm/metrics/ai-utils.go +++ b/npm/metrics/ai-utils.go @@ -103,13 +103,13 @@ func SendHeartbeatWithNumPolicies() { numPolicies, err := GetNumPolicies() numPoliciesString := "unknown" if err != nil { - klog.Warningf("warn: NPM hearbeat. Couldn't get number of policies for telemetry log: %s", err.Error()) + klog.Warningf("warn: NPM heartbeat. Couldn't get number of policies for telemetry log: %s", err.Error()) } else { numPoliciesString = strconv.Itoa(numPolicies) } cidrNetPols := GetCidrNetPols() namedPortNetPols := GetNamedPortNetPols() - message := fmt.Sprintf("info: NPM hearbeat. Total policies: %s, CIDR policies: %d, NamedPort policies: %d", numPoliciesString, cidrNetPols, namedPortNetPols) + message := fmt.Sprintf("info: NPM heartbeat. Total policies: %s, CIDR policies: %d, NamedPort policies: %d", numPoliciesString, cidrNetPols, namedPortNetPols) SendLog(util.NpmID, message, DonotPrint) } diff --git a/npm/pkg/controlplane/controllers/v2/networkPolicyController.go b/npm/pkg/controlplane/controllers/v2/networkPolicyController.go index 3b26d7b3ee..736ba20db4 100644 --- a/npm/pkg/controlplane/controllers/v2/networkPolicyController.go +++ b/npm/pkg/controlplane/controllers/v2/networkPolicyController.go @@ -302,13 +302,9 @@ func (c *NetworkPolicyController) syncAddAndUpdateNetPol(netPolObj *networkingv1 } oldNetPolSpec, policyExisted := c.rawNpSpecMap[netpolKey] - hadCIDR := false - hadNamedPort := false var operationKind metrics.OperationKind if policyExisted { operationKind = metrics.UpdateOp - hadCIDR = translation.HasCIDRBlock(oldNetPolSpec) - hadNamedPort = translation.HasNamedPort(oldNetPolSpec) } else { operationKind = metrics.CreateOp } @@ -325,24 +321,24 @@ func (c *NetworkPolicyController) syncAddAndUpdateNetPol(netPolObj *networkingv1 } if policyExisted { - if hadCIDR && !translation.HasCIDRBlock(&netPolObj.Spec) { + if translation.HasCIDRBlock(oldNetPolSpec) { metrics.DecCidrNetPols() } - if hadNamedPort && !translation.HasNamedPort(&netPolObj.Spec) { + if translation.HasNamedPort(oldNetPolSpec) { metrics.DecNamedPortNetPols() } } else { // inc metric for NumPolicies only if it a new network policy metrics.IncNumPolicies() + } - if translation.HasCIDRBlock(&netPolObj.Spec) { - metrics.IncCidrNetPols() - } + if translation.HasCIDRBlock(&netPolObj.Spec) { + metrics.IncCidrNetPols() + } - if translation.HasNamedPort(&netPolObj.Spec) { - metrics.IncNamedPortNetPols() - } + if translation.HasNamedPort(&netPolObj.Spec) { + metrics.IncNamedPortNetPols() } c.rawNpSpecMap[netpolKey] = &netPolObj.Spec diff --git a/npm/pkg/controlplane/translation/translatePolicy.go b/npm/pkg/controlplane/translation/translatePolicy.go index c1bb4890b3..ebc1e1334b 100644 --- a/npm/pkg/controlplane/translation/translatePolicy.go +++ b/npm/pkg/controlplane/translation/translatePolicy.go @@ -708,7 +708,7 @@ func HasCIDRBlock(netPolSpec *networkingv1.NetworkPolicySpec) bool { func HasNamedPort(netPolObj *networkingv1.NetworkPolicySpec) bool { for _, ingress := range netPolObj.Ingress { for _, port := range ingress.Ports { - if t, err := portType(port); err != nil && t == namedPortType { + if t, err := portType(port); err == nil && t == namedPortType { return true } } @@ -716,7 +716,7 @@ func HasNamedPort(netPolObj *networkingv1.NetworkPolicySpec) bool { for _, egress := range netPolObj.Egress { for _, port := range egress.Ports { - if t, err := portType(port); err != nil && t == namedPortType { + if t, err := portType(port); err == nil && t == namedPortType { return true } } From 2b49e4eacde6c860871582f843b46fafaee32e37 Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Tue, 7 Jan 2025 17:12:54 -0800 Subject: [PATCH 5/7] test: unit tests Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- .../v2/networkPolicyController_test.go | 478 +++++++++++++++++- 1 file changed, 469 insertions(+), 9 deletions(-) diff --git a/npm/pkg/controlplane/controllers/v2/networkPolicyController_test.go b/npm/pkg/controlplane/controllers/v2/networkPolicyController_test.go index 1c531e7dea..4a1d39ab3c 100644 --- a/npm/pkg/controlplane/controllers/v2/networkPolicyController_test.go +++ b/npm/pkg/controlplane/controllers/v2/networkPolicyController_test.go @@ -176,10 +176,13 @@ func addNetPol(f *netPolFixture, netPolObj *networkingv1.NetworkPolicy) { f.netPolController.processNextWorkItem() } -func deleteNetPol(t *testing.T, f *netPolFixture, netPolObj *networkingv1.NetworkPolicy, isDeletedFinalStateUnknownObject IsDeletedFinalStateUnknownObject) { +func addAndDeleteNetPol(t *testing.T, f *netPolFixture, netPolObj *networkingv1.NetworkPolicy, isDeletedFinalStateUnknownObject IsDeletedFinalStateUnknownObject) { addNetPol(f, netPolObj) t.Logf("Complete adding network policy event") + deleteNetPol(t, f, netPolObj, isDeletedFinalStateUnknownObject) +} +func deleteNetPol(t *testing.T, f *netPolFixture, netPolObj *networkingv1.NetworkPolicy, isDeletedFinalStateUnknownObject IsDeletedFinalStateUnknownObject) { // simulate network policy deletion event and delete network policy object from sharedInformer cache err := f.kubeInformer.Networking().V1().NetworkPolicies().Informer().GetIndexer().Delete(netPolObj) if err != nil { @@ -203,16 +206,19 @@ func deleteNetPol(t *testing.T, f *netPolFixture, netPolObj *networkingv1.Networ f.netPolController.processNextWorkItem() } -func updateNetPol(t *testing.T, f *netPolFixture, oldNetPolObj, netNetPolObj *networkingv1.NetworkPolicy) { +func addAndUpdateNetPol(t *testing.T, f *netPolFixture, oldNetPolObj, newNetPolObj *networkingv1.NetworkPolicy) { addNetPol(f, oldNetPolObj) t.Logf("Complete adding network policy event") + updateNetPol(t, f, oldNetPolObj, newNetPolObj) +} +func updateNetPol(t *testing.T, f *netPolFixture, oldNetPolObj, newNetPolObj *networkingv1.NetworkPolicy) { // simulate network policy update event and update the network policy to shared informer's cache - err := f.kubeInformer.Networking().V1().NetworkPolicies().Informer().GetIndexer().Update(netNetPolObj) + err := f.kubeInformer.Networking().V1().NetworkPolicies().Informer().GetIndexer().Update(newNetPolObj) if err != nil { - f.t.Errorf("Failed to update network policy %s to shared informer cache: %v", netNetPolObj.Name, err) + f.t.Errorf("Failed to update network policy %s to shared informer cache: %v", newNetPolObj.Name, err) } - f.netPolController.updateNetworkPolicy(oldNetPolObj, netNetPolObj) + f.netPolController.updateNetworkPolicy(oldNetPolObj, newNetPolObj) if f.netPolController.workqueue.Len() == 0 { return @@ -406,7 +412,7 @@ func TestDeleteNetworkPolicy(t *testing.T) { dp.EXPECT().UpdatePolicy(gomock.Any()).Times(1) dp.EXPECT().RemovePolicy(gomock.Any()).Times(1) - deleteNetPol(t, f, netPolObj, DeletedFinalStateknownObject) + addAndDeleteNetPol(t, f, netPolObj, DeletedFinalStateknownObject) testCases := []expectedNetPolValues{ {0, 0, netPolPromVals{0, 1, 0, 1}}, } @@ -457,7 +463,7 @@ func TestDeleteNetworkPolicyWithTombstoneAfterAddingNetworkPolicy(t *testing.T) dp.EXPECT().UpdatePolicy(gomock.Any()).Times(1) dp.EXPECT().RemovePolicy(gomock.Any()).Times(1) - deleteNetPol(t, f, netPolObj, DeletedFinalStateUnknownObject) + addAndDeleteNetPol(t, f, netPolObj, DeletedFinalStateUnknownObject) testCases := []expectedNetPolValues{ {0, 0, netPolPromVals{0, 1, 0, 1}}, } @@ -486,7 +492,7 @@ func TestUpdateNetworkPolicy(t *testing.T) { newNetPolObj.ResourceVersion = fmt.Sprintf("%d", newRV+1) dp.EXPECT().UpdatePolicy(gomock.Any()).Times(1) - updateNetPol(t, f, oldNetPolObj, newNetPolObj) + addAndUpdateNetPol(t, f, oldNetPolObj, newNetPolObj) testCases := []expectedNetPolValues{ {1, 0, netPolPromVals{1, 1, 0, 0}}, } @@ -520,10 +526,464 @@ func TestLabelUpdateNetworkPolicy(t *testing.T) { newNetPolObj.ResourceVersion = fmt.Sprintf("%d", newRV+1) dp.EXPECT().UpdatePolicy(gomock.Any()).Times(2) - updateNetPol(t, f, oldNetPolObj, newNetPolObj) + addAndUpdateNetPol(t, f, oldNetPolObj, newNetPolObj) testCases := []expectedNetPolValues{ {1, 0, netPolPromVals{1, 1, 1, 0}}, } checkNetPolTestResult("TestUpdateNetPol", f, testCases) } + +func TestCountsAddAndDeleteNetPol(t *testing.T) { + tests := []struct { + name string + // network policy to add + netPolSpec *networkingv1.NetworkPolicySpec + cidrCount int + namedPortCount int + }{ + { + name: "no-cidr-namedPort", + netPolSpec: &networkingv1.NetworkPolicySpec{ + PolicyTypes: []networkingv1.PolicyType{ + networkingv1.PolicyTypeIngress, + networkingv1.PolicyTypeEgress, + }, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + { + From: []networkingv1.NetworkPolicyPeer{ + { + PodSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "test"}, + }, + }, + }, + Ports: []networkingv1.NetworkPolicyPort{ + { + Port: &intstr.IntOrString{IntVal: 8000}, + }, + }, + }, + }, + Egress: []networkingv1.NetworkPolicyEgressRule{ + { + To: []networkingv1.NetworkPolicyPeer{ + { + PodSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "test"}, + }, + }, + }, + Ports: []networkingv1.NetworkPolicyPort{ + { + Port: &intstr.IntOrString{IntVal: 8000}, + }, + }, + }, + }, + }, + }, + { + name: "cidr-ingress", + netPolSpec: &networkingv1.NetworkPolicySpec{ + PolicyTypes: []networkingv1.PolicyType{ + networkingv1.PolicyTypeIngress, + }, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + { + From: []networkingv1.NetworkPolicyPeer{ + { + IPBlock: &networkingv1.IPBlock{ + CIDR: "0.0.0.0/0", + }, + }, + }, + }, + }, + }, + cidrCount: 1, + }, + { + name: "cidr-egress", + netPolSpec: &networkingv1.NetworkPolicySpec{ + PolicyTypes: []networkingv1.PolicyType{ + networkingv1.PolicyTypeEgress, + }, + Egress: []networkingv1.NetworkPolicyEgressRule{ + { + To: []networkingv1.NetworkPolicyPeer{ + { + IPBlock: &networkingv1.IPBlock{ + CIDR: "0.0.0.0/0", + }, + }, + }, + }, + }, + }, + cidrCount: 1, + }, + { + name: "namedPort-ingress", + netPolSpec: &networkingv1.NetworkPolicySpec{ + PolicyTypes: []networkingv1.PolicyType{ + networkingv1.PolicyTypeIngress, + }, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + { + Ports: []networkingv1.NetworkPolicyPort{ + { + Port: &intstr.IntOrString{StrVal: "abc"}, + }, + }, + }, + }, + }, + namedPortCount: 1, + }, + { + name: "namedPort-egress", + netPolSpec: &networkingv1.NetworkPolicySpec{ + PolicyTypes: []networkingv1.PolicyType{ + networkingv1.PolicyTypeEgress, + }, + Egress: []networkingv1.NetworkPolicyEgressRule{ + { + Ports: []networkingv1.NetworkPolicyPort{ + { + Port: &intstr.IntOrString{StrVal: "abc"}, + }, + }, + }, + }, + }, + namedPortCount: 1, + }, + { + name: "cidr-and-namedPort", + netPolSpec: &networkingv1.NetworkPolicySpec{ + PolicyTypes: []networkingv1.PolicyType{ + networkingv1.PolicyTypeIngress, + }, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + { + From: []networkingv1.NetworkPolicyPeer{ + { + IPBlock: &networkingv1.IPBlock{ + CIDR: "0.0.0.0/0", + }, + }, + }, + Ports: []networkingv1.NetworkPolicyPort{ + { + Port: &intstr.IntOrString{StrVal: "abc"}, + }, + }, + }, + }, + }, + cidrCount: 1, + namedPortCount: 1, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + f := newNetPolFixture(t) + netPolObj := createNetPol() + netPolObj.Spec = *tt.netPolSpec + f.netPolLister = append(f.netPolLister, netPolObj) + f.kubeobjects = append(f.kubeobjects, netPolObj) + stopCh := make(chan struct{}) + defer close(stopCh) + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + dp := dpmocks.NewMockGenericDataplane(ctrl) + f.newNetPolController(stopCh, dp, false) + + dp.EXPECT().UpdatePolicy(gomock.Any()).Times(1) + dp.EXPECT().RemovePolicy(gomock.Any()).Times(1) + + addNetPol(f, netPolObj) + testCases := []expectedNetPolValues{ + {1, 0, netPolPromVals{1, 1, 0, 0}}, + } + checkNetPolTestResult("TestCountsCreateNetPol", f, testCases) + require.Equal(t, tt.cidrCount, metrics.GetCidrNetPols()) + require.Equal(t, tt.namedPortCount, metrics.GetNamedPortNetPols()) + + deleteNetPol(t, f, netPolObj, DeletedFinalStateknownObject) + testCases = []expectedNetPolValues{ + {0, 0, netPolPromVals{0, 1, 0, 1}}, + } + checkNetPolTestResult("TestCountsDelNetPol", f, testCases) + require.Equal(t, 0, metrics.GetCidrNetPols()) + require.Equal(t, 0, metrics.GetNamedPortNetPols()) + }) + } +} + +func TestCountsUpdateNetPol(t *testing.T) { + tests := []struct { + name string + netPolSpec *networkingv1.NetworkPolicySpec + updatedNetPolSpec *networkingv1.NetworkPolicySpec + cidrCount int + namedPortCount int + updatedCidrCount int + updatedNamedPortCount int + }{ + { + name: "cidr-to-no-cidr", + netPolSpec: &networkingv1.NetworkPolicySpec{ + PolicyTypes: []networkingv1.PolicyType{ + networkingv1.PolicyTypeIngress, + }, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + { + From: []networkingv1.NetworkPolicyPeer{ + { + IPBlock: &networkingv1.IPBlock{ + CIDR: "0.0.0.0/0", + }, + }, + }, + }, + }, + }, + updatedNetPolSpec: &networkingv1.NetworkPolicySpec{ + PolicyTypes: []networkingv1.PolicyType{ + networkingv1.PolicyTypeIngress, + }, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + { + From: []networkingv1.NetworkPolicyPeer{ + { + PodSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "test"}, + }, + }, + }, + }, + }, + }, + cidrCount: 1, + updatedCidrCount: 0, + }, + { + name: "no-cidr-to-cidr", + netPolSpec: &networkingv1.NetworkPolicySpec{ + PolicyTypes: []networkingv1.PolicyType{ + networkingv1.PolicyTypeIngress, + }, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + { + From: []networkingv1.NetworkPolicyPeer{ + { + PodSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{"app": "test"}, + }, + }, + }, + }, + }, + }, + updatedNetPolSpec: &networkingv1.NetworkPolicySpec{ + PolicyTypes: []networkingv1.PolicyType{ + networkingv1.PolicyTypeIngress, + }, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + { + From: []networkingv1.NetworkPolicyPeer{ + { + IPBlock: &networkingv1.IPBlock{ + CIDR: "0.0.0.0/0", + }, + }, + }, + }, + }, + }, + cidrCount: 0, + updatedCidrCount: 1, + }, + { + name: "cidr-to-cidr", + netPolSpec: &networkingv1.NetworkPolicySpec{ + PolicyTypes: []networkingv1.PolicyType{ + networkingv1.PolicyTypeIngress, + }, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + { + From: []networkingv1.NetworkPolicyPeer{ + { + IPBlock: &networkingv1.IPBlock{ + CIDR: "0.0.0.0/0", + }, + }, + }, + }, + }, + }, + updatedNetPolSpec: &networkingv1.NetworkPolicySpec{ + PolicyTypes: []networkingv1.PolicyType{ + networkingv1.PolicyTypeIngress, + }, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + { + From: []networkingv1.NetworkPolicyPeer{ + { + IPBlock: &networkingv1.IPBlock{ + CIDR: "1.0.0.0/32", + }, + }, + }, + }, + }, + }, + cidrCount: 1, + updatedCidrCount: 1, + }, + { + name: "namedPort-to-no-namedPort", + netPolSpec: &networkingv1.NetworkPolicySpec{ + PolicyTypes: []networkingv1.PolicyType{ + networkingv1.PolicyTypeIngress, + }, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + { + Ports: []networkingv1.NetworkPolicyPort{ + { + Port: &intstr.IntOrString{StrVal: "abc"}, + }, + }, + }, + }, + }, + updatedNetPolSpec: &networkingv1.NetworkPolicySpec{ + PolicyTypes: []networkingv1.PolicyType{ + networkingv1.PolicyTypeIngress, + }, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + { + Ports: []networkingv1.NetworkPolicyPort{ + { + Port: &intstr.IntOrString{IntVal: 8000}, + }, + }, + }, + }, + }, + namedPortCount: 1, + updatedNamedPortCount: 0, + }, + { + name: "no-namedPort-to-namedPort", + netPolSpec: &networkingv1.NetworkPolicySpec{ + PolicyTypes: []networkingv1.PolicyType{ + networkingv1.PolicyTypeIngress, + }, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + { + Ports: []networkingv1.NetworkPolicyPort{ + { + Port: &intstr.IntOrString{IntVal: 8000}, + }, + }, + }, + }, + }, + updatedNetPolSpec: &networkingv1.NetworkPolicySpec{ + PolicyTypes: []networkingv1.PolicyType{ + networkingv1.PolicyTypeIngress, + }, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + { + Ports: []networkingv1.NetworkPolicyPort{ + { + Port: &intstr.IntOrString{StrVal: "abc"}, + }, + }, + }, + }, + }, + namedPortCount: 0, + updatedNamedPortCount: 1, + }, + { + name: "namedPort-to-namedPort", + netPolSpec: &networkingv1.NetworkPolicySpec{ + PolicyTypes: []networkingv1.PolicyType{ + networkingv1.PolicyTypeIngress, + }, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + { + Ports: []networkingv1.NetworkPolicyPort{ + { + Port: &intstr.IntOrString{StrVal: "abc"}, + }, + }, + }, + }, + }, + updatedNetPolSpec: &networkingv1.NetworkPolicySpec{ + PolicyTypes: []networkingv1.PolicyType{ + networkingv1.PolicyTypeIngress, + }, + Ingress: []networkingv1.NetworkPolicyIngressRule{ + { + Ports: []networkingv1.NetworkPolicyPort{ + { + Port: &intstr.IntOrString{StrVal: "xyz"}, + }, + }, + }, + }, + }, + namedPortCount: 1, + updatedNamedPortCount: 1, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + f := newNetPolFixture(t) + netPolObj := createNetPol() + netPolObj.Spec = *tt.netPolSpec + f.netPolLister = append(f.netPolLister, netPolObj) + f.kubeobjects = append(f.kubeobjects, netPolObj) + stopCh := make(chan struct{}) + defer close(stopCh) + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + dp := dpmocks.NewMockGenericDataplane(ctrl) + f.newNetPolController(stopCh, dp, false) + + dp.EXPECT().UpdatePolicy(gomock.Any()).Times(2) + + addNetPol(f, netPolObj) + testCases := []expectedNetPolValues{ + {1, 0, netPolPromVals{1, 1, 0, 0}}, + } + checkNetPolTestResult("TestCountsAddNetPol", f, testCases) + require.Equal(t, tt.cidrCount, metrics.GetCidrNetPols()) + require.Equal(t, tt.namedPortCount, metrics.GetNamedPortNetPols()) + + newNetPolObj := createNetPol() + newNetPolObj.Spec = *tt.updatedNetPolSpec + newRV, _ := strconv.Atoi(netPolObj.ResourceVersion) + newNetPolObj.ResourceVersion = fmt.Sprintf("%d", newRV+1) + updateNetPol(t, f, netPolObj, newNetPolObj) + testCases = []expectedNetPolValues{ + {1, 0, netPolPromVals{1, 1, 1, 0}}, + } + checkNetPolTestResult("TestCountsUpdateNetPol", f, testCases) + require.Equal(t, tt.updatedCidrCount, metrics.GetCidrNetPols()) + require.Equal(t, tt.updatedNamedPortCount, metrics.GetNamedPortNetPols()) + }) + } +} From 22931a8b358e8684e8c7eb1eeb81d239c4cb49cb Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Wed, 8 Jan 2025 17:27:50 -0800 Subject: [PATCH 6/7] style: fix lints in unit tests Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- .../controllers/v2/networkPolicyController_test.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/npm/pkg/controlplane/controllers/v2/networkPolicyController_test.go b/npm/pkg/controlplane/controllers/v2/networkPolicyController_test.go index dcd550aa8c..a3675c6a48 100644 --- a/npm/pkg/controlplane/controllers/v2/networkPolicyController_test.go +++ b/npm/pkg/controlplane/controllers/v2/networkPolicyController_test.go @@ -201,10 +201,12 @@ func deleteNetPol(t *testing.T, f *netPolFixture, netPolObj *networkingv1.Networ } if f.netPolController.workqueue.Len() == 0 { + t.Logf("Complete deleting network policy event. workqueue is empty") return } f.netPolController.processNextWorkItem() + t.Logf("Complete deleting network policy event") } func addAndUpdateNetPol(t *testing.T, f *netPolFixture, oldNetPolObj, newNetPolObj *networkingv1.NetworkPolicy) { @@ -226,6 +228,8 @@ func updateNetPol(t *testing.T, f *netPolFixture, oldNetPolObj, newNetPolObj *ne } f.netPolController.processNextWorkItem() + + t.Logf("Complete updating network policy event") } type expectedNetPolValues struct { @@ -558,7 +562,7 @@ func TestUpdateNetworkPolicy(t *testing.T) { newNetPolObj := oldNetPolObj.DeepCopy() // oldNetPolObj.ResourceVersion value is "0" newRV, _ := strconv.Atoi(oldNetPolObj.ResourceVersion) - newNetPolObj.ResourceVersion = fmt.Sprintf("%d", newRV+1) + newNetPolObj.ResourceVersion = strconv.Itoa(newRV + 1) var testCases []expectedNetPolValues if util.IsWindowsDP() { @@ -603,7 +607,7 @@ func TestLabelUpdateNetworkPolicy(t *testing.T) { } // oldNetPolObj.ResourceVersion value is "0" newRV, _ := strconv.Atoi(oldNetPolObj.ResourceVersion) - newNetPolObj.ResourceVersion = fmt.Sprintf("%d", newRV+1) + newNetPolObj.ResourceVersion = strconv.Itoa(newRV + 1) var testCases []expectedNetPolValues @@ -1067,7 +1071,7 @@ func TestCountsUpdateNetPol(t *testing.T) { newNetPolObj := createNetPol() newNetPolObj.Spec = *tt.updatedNetPolSpec newRV, _ := strconv.Atoi(netPolObj.ResourceVersion) - newNetPolObj.ResourceVersion = fmt.Sprintf("%d", newRV+1) + newNetPolObj.ResourceVersion = strconv.Itoa(newRV + 1) updateNetPol(t, f, netPolObj, newNetPolObj) testCases = []expectedNetPolValues{ {1, 0, netPolPromVals{1, 1, 1, 0}}, From 49771e3d0fdcb1b01ac496cb3f416d621a7a3dba Mon Sep 17 00:00:00 2001 From: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> Date: Thu, 9 Jan 2025 14:03:46 -0800 Subject: [PATCH 7/7] fix: track endport policies instead of namedport Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com> --- npm/metrics/ai-utils.go | 4 +- npm/metrics/counts.go | 16 ++-- .../controllers/v2/networkPolicyController.go | 12 +-- .../v2/networkPolicyController_test.go | 90 +++++++++++-------- .../translation/translatePolicy.go | 14 ++- 5 files changed, 79 insertions(+), 57 deletions(-) diff --git a/npm/metrics/ai-utils.go b/npm/metrics/ai-utils.go index ad2eb4bf22..9329d05329 100644 --- a/npm/metrics/ai-utils.go +++ b/npm/metrics/ai-utils.go @@ -120,7 +120,7 @@ func SendHeartbeatWithNumPolicies() { } cidrNetPols := GetCidrNetPols() - namedPortNetPols := GetNamedPortNetPols() - message := fmt.Sprintf("info: NPM heartbeat. Total policies: %s, CIDR policies: %d, NamedPort policies: %d", numPoliciesString, cidrNetPols, namedPortNetPols) + endPortNetPols := GetEndPortNetPols() + message := fmt.Sprintf("info: NPM heartbeat. Total policies: %s, CIDR policies: %d, EndPort policies: %d", numPoliciesString, cidrNetPols, endPortNetPols) SendLog(util.NpmID, message, DonotPrint) } diff --git a/npm/metrics/counts.go b/npm/metrics/counts.go index 1f2216f33d..f4e3e5b25d 100644 --- a/npm/metrics/counts.go +++ b/npm/metrics/counts.go @@ -7,8 +7,8 @@ var nonPrometheusCounts *counts // counts is a struct holding non-Prometheus counts. type counts struct { sync.Mutex - cidrNetPols int - namedPortNetPols int + cidrNetPols int + endPortNetPols int } func IncCidrNetPols() { @@ -38,29 +38,29 @@ func GetCidrNetPols() int { return nonPrometheusCounts.cidrNetPols } -func IncNamedPortNetPols() { +func IncEndPortNetPols() { if nonPrometheusCounts == nil { return } nonPrometheusCounts.Lock() defer nonPrometheusCounts.Unlock() - nonPrometheusCounts.namedPortNetPols++ + nonPrometheusCounts.endPortNetPols++ } -func DecNamedPortNetPols() { +func DecEndPortNetPols() { if nonPrometheusCounts == nil { return } nonPrometheusCounts.Lock() defer nonPrometheusCounts.Unlock() - nonPrometheusCounts.namedPortNetPols-- + nonPrometheusCounts.endPortNetPols-- } -func GetNamedPortNetPols() int { +func GetEndPortNetPols() int { if nonPrometheusCounts == nil { return 0 } nonPrometheusCounts.Lock() defer nonPrometheusCounts.Unlock() - return nonPrometheusCounts.namedPortNetPols + return nonPrometheusCounts.endPortNetPols } diff --git a/npm/pkg/controlplane/controllers/v2/networkPolicyController.go b/npm/pkg/controlplane/controllers/v2/networkPolicyController.go index 736ba20db4..fe1d85b977 100644 --- a/npm/pkg/controlplane/controllers/v2/networkPolicyController.go +++ b/npm/pkg/controlplane/controllers/v2/networkPolicyController.go @@ -325,8 +325,8 @@ func (c *NetworkPolicyController) syncAddAndUpdateNetPol(netPolObj *networkingv1 metrics.DecCidrNetPols() } - if translation.HasNamedPort(oldNetPolSpec) { - metrics.DecNamedPortNetPols() + if translation.HasEndPort(oldNetPolSpec) { + metrics.DecEndPortNetPols() } } else { // inc metric for NumPolicies only if it a new network policy @@ -337,8 +337,8 @@ func (c *NetworkPolicyController) syncAddAndUpdateNetPol(netPolObj *networkingv1 metrics.IncCidrNetPols() } - if translation.HasNamedPort(&netPolObj.Spec) { - metrics.IncNamedPortNetPols() + if translation.HasEndPort(&netPolObj.Spec) { + metrics.IncEndPortNetPols() } c.rawNpSpecMap[netpolKey] = &netPolObj.Spec @@ -362,8 +362,8 @@ func (c *NetworkPolicyController) cleanUpNetworkPolicy(netPolKey string) error { metrics.DecCidrNetPols() } - if translation.HasNamedPort(cachedNetPolSpec) { - metrics.DecNamedPortNetPols() + if translation.HasEndPort(cachedNetPolSpec) { + metrics.DecEndPortNetPols() } // Success to clean up ipset and iptables operations in kernel and delete the cached network policy from RawNpMap diff --git a/npm/pkg/controlplane/controllers/v2/networkPolicyController_test.go b/npm/pkg/controlplane/controllers/v2/networkPolicyController_test.go index a3675c6a48..60115632fc 100644 --- a/npm/pkg/controlplane/controllers/v2/networkPolicyController_test.go +++ b/npm/pkg/controlplane/controllers/v2/networkPolicyController_test.go @@ -24,6 +24,13 @@ import ( "k8s.io/client-go/tools/cache" ) +var ( + eightyFive int32 = 85 + eightyFivePointer = &eightyFive + eightySix int32 = 86 + eightySixPointer = &eightySix +) + type netPolFixture struct { t *testing.T @@ -633,12 +640,12 @@ func TestCountsAddAndDeleteNetPol(t *testing.T) { tests := []struct { name string // network policy to add - netPolSpec *networkingv1.NetworkPolicySpec - cidrCount int - namedPortCount int + netPolSpec *networkingv1.NetworkPolicySpec + cidrCount int + endPortCount int }{ { - name: "no-cidr-namedPort", + name: "no-cidr-endPort", netPolSpec: &networkingv1.NetworkPolicySpec{ PolicyTypes: []networkingv1.PolicyType{ networkingv1.PolicyTypeIngress, @@ -719,7 +726,7 @@ func TestCountsAddAndDeleteNetPol(t *testing.T) { cidrCount: 1, }, { - name: "namedPort-ingress", + name: "endPort-ingress", netPolSpec: &networkingv1.NetworkPolicySpec{ PolicyTypes: []networkingv1.PolicyType{ networkingv1.PolicyTypeIngress, @@ -728,16 +735,17 @@ func TestCountsAddAndDeleteNetPol(t *testing.T) { { Ports: []networkingv1.NetworkPolicyPort{ { - Port: &intstr.IntOrString{StrVal: "abc"}, + Port: &intstr.IntOrString{IntVal: 80}, + EndPort: eightyFivePointer, }, }, }, }, }, - namedPortCount: 1, + endPortCount: 1, }, { - name: "namedPort-egress", + name: "endPort-egress", netPolSpec: &networkingv1.NetworkPolicySpec{ PolicyTypes: []networkingv1.PolicyType{ networkingv1.PolicyTypeEgress, @@ -746,16 +754,17 @@ func TestCountsAddAndDeleteNetPol(t *testing.T) { { Ports: []networkingv1.NetworkPolicyPort{ { - Port: &intstr.IntOrString{StrVal: "abc"}, + Port: &intstr.IntOrString{IntVal: 80}, + EndPort: eightyFivePointer, }, }, }, }, }, - namedPortCount: 1, + endPortCount: 1, }, { - name: "cidr-and-namedPort", + name: "cidr-and-endPort", netPolSpec: &networkingv1.NetworkPolicySpec{ PolicyTypes: []networkingv1.PolicyType{ networkingv1.PolicyTypeIngress, @@ -771,14 +780,15 @@ func TestCountsAddAndDeleteNetPol(t *testing.T) { }, Ports: []networkingv1.NetworkPolicyPort{ { - Port: &intstr.IntOrString{StrVal: "abc"}, + Port: &intstr.IntOrString{IntVal: 80}, + EndPort: eightyFivePointer, }, }, }, }, }, - cidrCount: 1, - namedPortCount: 1, + cidrCount: 1, + endPortCount: 1, }, } @@ -807,7 +817,7 @@ func TestCountsAddAndDeleteNetPol(t *testing.T) { } checkNetPolTestResult("TestCountsCreateNetPol", f, testCases) require.Equal(t, tt.cidrCount, metrics.GetCidrNetPols()) - require.Equal(t, tt.namedPortCount, metrics.GetNamedPortNetPols()) + require.Equal(t, tt.endPortCount, metrics.GetEndPortNetPols()) deleteNetPol(t, f, netPolObj, DeletedFinalStateknownObject) testCases = []expectedNetPolValues{ @@ -815,20 +825,20 @@ func TestCountsAddAndDeleteNetPol(t *testing.T) { } checkNetPolTestResult("TestCountsDelNetPol", f, testCases) require.Equal(t, 0, metrics.GetCidrNetPols()) - require.Equal(t, 0, metrics.GetNamedPortNetPols()) + require.Equal(t, 0, metrics.GetEndPortNetPols()) }) } } func TestCountsUpdateNetPol(t *testing.T) { tests := []struct { - name string - netPolSpec *networkingv1.NetworkPolicySpec - updatedNetPolSpec *networkingv1.NetworkPolicySpec - cidrCount int - namedPortCount int - updatedCidrCount int - updatedNamedPortCount int + name string + netPolSpec *networkingv1.NetworkPolicySpec + updatedNetPolSpec *networkingv1.NetworkPolicySpec + cidrCount int + endPortCount int + updatedCidrCount int + updatedEndPortCount int }{ { name: "cidr-to-no-cidr", @@ -942,7 +952,7 @@ func TestCountsUpdateNetPol(t *testing.T) { updatedCidrCount: 1, }, { - name: "namedPort-to-no-namedPort", + name: "endPort-to-no-endPort", netPolSpec: &networkingv1.NetworkPolicySpec{ PolicyTypes: []networkingv1.PolicyType{ networkingv1.PolicyTypeIngress, @@ -951,7 +961,8 @@ func TestCountsUpdateNetPol(t *testing.T) { { Ports: []networkingv1.NetworkPolicyPort{ { - Port: &intstr.IntOrString{StrVal: "abc"}, + Port: &intstr.IntOrString{IntVal: 80}, + EndPort: eightyFivePointer, }, }, }, @@ -971,11 +982,11 @@ func TestCountsUpdateNetPol(t *testing.T) { }, }, }, - namedPortCount: 1, - updatedNamedPortCount: 0, + endPortCount: 1, + updatedEndPortCount: 0, }, { - name: "no-namedPort-to-namedPort", + name: "no-endPort-to-endPort", netPolSpec: &networkingv1.NetworkPolicySpec{ PolicyTypes: []networkingv1.PolicyType{ networkingv1.PolicyTypeIngress, @@ -998,17 +1009,18 @@ func TestCountsUpdateNetPol(t *testing.T) { { Ports: []networkingv1.NetworkPolicyPort{ { - Port: &intstr.IntOrString{StrVal: "abc"}, + Port: &intstr.IntOrString{IntVal: 80}, + EndPort: eightyFivePointer, }, }, }, }, }, - namedPortCount: 0, - updatedNamedPortCount: 1, + endPortCount: 0, + updatedEndPortCount: 1, }, { - name: "namedPort-to-namedPort", + name: "endPort-to-endPort", netPolSpec: &networkingv1.NetworkPolicySpec{ PolicyTypes: []networkingv1.PolicyType{ networkingv1.PolicyTypeIngress, @@ -1017,7 +1029,8 @@ func TestCountsUpdateNetPol(t *testing.T) { { Ports: []networkingv1.NetworkPolicyPort{ { - Port: &intstr.IntOrString{StrVal: "abc"}, + Port: &intstr.IntOrString{IntVal: 80}, + EndPort: eightyFivePointer, }, }, }, @@ -1031,14 +1044,15 @@ func TestCountsUpdateNetPol(t *testing.T) { { Ports: []networkingv1.NetworkPolicyPort{ { - Port: &intstr.IntOrString{StrVal: "xyz"}, + Port: &intstr.IntOrString{IntVal: 80}, + EndPort: eightySixPointer, }, }, }, }, }, - namedPortCount: 1, - updatedNamedPortCount: 1, + endPortCount: 1, + updatedEndPortCount: 1, }, } @@ -1066,7 +1080,7 @@ func TestCountsUpdateNetPol(t *testing.T) { } checkNetPolTestResult("TestCountsAddNetPol", f, testCases) require.Equal(t, tt.cidrCount, metrics.GetCidrNetPols()) - require.Equal(t, tt.namedPortCount, metrics.GetNamedPortNetPols()) + require.Equal(t, tt.endPortCount, metrics.GetEndPortNetPols()) newNetPolObj := createNetPol() newNetPolObj.Spec = *tt.updatedNetPolSpec @@ -1078,7 +1092,7 @@ func TestCountsUpdateNetPol(t *testing.T) { } checkNetPolTestResult("TestCountsUpdateNetPol", f, testCases) require.Equal(t, tt.updatedCidrCount, metrics.GetCidrNetPols()) - require.Equal(t, tt.updatedNamedPortCount, metrics.GetNamedPortNetPols()) + require.Equal(t, tt.updatedEndPortCount, metrics.GetEndPortNetPols()) }) } } diff --git a/npm/pkg/controlplane/translation/translatePolicy.go b/npm/pkg/controlplane/translation/translatePolicy.go index ebc1e1334b..bd415d3138 100644 --- a/npm/pkg/controlplane/translation/translatePolicy.go +++ b/npm/pkg/controlplane/translation/translatePolicy.go @@ -705,10 +705,14 @@ func HasCIDRBlock(netPolSpec *networkingv1.NetworkPolicySpec) bool { return false } -func HasNamedPort(netPolObj *networkingv1.NetworkPolicySpec) bool { +func HasEndPort(netPolObj *networkingv1.NetworkPolicySpec) bool { for _, ingress := range netPolObj.Ingress { for _, port := range ingress.Ports { - if t, err := portType(port); err == nil && t == namedPortType { + if port.EndPort == nil { + continue + } + + if t, err := portType(port); err == nil && t == numericPortType { return true } } @@ -716,7 +720,11 @@ func HasNamedPort(netPolObj *networkingv1.NetworkPolicySpec) bool { for _, egress := range netPolObj.Egress { for _, port := range egress.Ports { - if t, err := portType(port); err == nil && t == namedPortType { + if port.EndPort == nil { + continue + } + + if t, err := portType(port); err == nil && t == numericPortType { return true } }