Skip to content

Commit e8a2832

Browse files
committed
fix: track endport policies instead of namedport
Signed-off-by: Hunter Gregory <[email protected]>
1 parent 22931a8 commit e8a2832

File tree

5 files changed

+73
-57
lines changed

5 files changed

+73
-57
lines changed

npm/metrics/ai-utils.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ func SendHeartbeatWithNumPolicies() {
120120
}
121121

122122
cidrNetPols := GetCidrNetPols()
123-
namedPortNetPols := GetNamedPortNetPols()
124-
message := fmt.Sprintf("info: NPM heartbeat. Total policies: %s, CIDR policies: %d, NamedPort policies: %d", numPoliciesString, cidrNetPols, namedPortNetPols)
123+
endPortNetPols := GetEndPortNetPols()
124+
message := fmt.Sprintf("info: NPM heartbeat. Total policies: %s, CIDR policies: %d, EndPort policies: %d", numPoliciesString, cidrNetPols, endPortNetPols)
125125
SendLog(util.NpmID, message, DonotPrint)
126126
}

npm/metrics/counts.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ var nonPrometheusCounts *counts
77
// counts is a struct holding non-Prometheus counts.
88
type counts struct {
99
sync.Mutex
10-
cidrNetPols int
11-
namedPortNetPols int
10+
cidrNetPols int
11+
endPortNetPols int
1212
}
1313

1414
func IncCidrNetPols() {
@@ -38,29 +38,29 @@ func GetCidrNetPols() int {
3838
return nonPrometheusCounts.cidrNetPols
3939
}
4040

41-
func IncNamedPortNetPols() {
41+
func IncEndPortNetPols() {
4242
if nonPrometheusCounts == nil {
4343
return
4444
}
4545
nonPrometheusCounts.Lock()
4646
defer nonPrometheusCounts.Unlock()
47-
nonPrometheusCounts.namedPortNetPols++
47+
nonPrometheusCounts.endPortNetPols++
4848
}
4949

50-
func DecNamedPortNetPols() {
50+
func DecEndPortNetPols() {
5151
if nonPrometheusCounts == nil {
5252
return
5353
}
5454
nonPrometheusCounts.Lock()
5555
defer nonPrometheusCounts.Unlock()
56-
nonPrometheusCounts.namedPortNetPols--
56+
nonPrometheusCounts.endPortNetPols--
5757
}
5858

59-
func GetNamedPortNetPols() int {
59+
func GetEndPortNetPols() int {
6060
if nonPrometheusCounts == nil {
6161
return 0
6262
}
6363
nonPrometheusCounts.Lock()
6464
defer nonPrometheusCounts.Unlock()
65-
return nonPrometheusCounts.namedPortNetPols
65+
return nonPrometheusCounts.endPortNetPols
6666
}

npm/pkg/controlplane/controllers/v2/networkPolicyController.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -325,8 +325,8 @@ func (c *NetworkPolicyController) syncAddAndUpdateNetPol(netPolObj *networkingv1
325325
metrics.DecCidrNetPols()
326326
}
327327

328-
if translation.HasNamedPort(oldNetPolSpec) {
329-
metrics.DecNamedPortNetPols()
328+
if translation.HasEndPort(oldNetPolSpec) {
329+
metrics.DecEndPortNetPols()
330330
}
331331
} else {
332332
// inc metric for NumPolicies only if it a new network policy
@@ -337,8 +337,8 @@ func (c *NetworkPolicyController) syncAddAndUpdateNetPol(netPolObj *networkingv1
337337
metrics.IncCidrNetPols()
338338
}
339339

340-
if translation.HasNamedPort(&netPolObj.Spec) {
341-
metrics.IncNamedPortNetPols()
340+
if translation.HasEndPort(&netPolObj.Spec) {
341+
metrics.IncEndPortNetPols()
342342
}
343343

344344
c.rawNpSpecMap[netpolKey] = &netPolObj.Spec
@@ -362,8 +362,8 @@ func (c *NetworkPolicyController) cleanUpNetworkPolicy(netPolKey string) error {
362362
metrics.DecCidrNetPols()
363363
}
364364

365-
if translation.HasNamedPort(cachedNetPolSpec) {
366-
metrics.DecNamedPortNetPols()
365+
if translation.HasEndPort(cachedNetPolSpec) {
366+
metrics.DecEndPortNetPols()
367367
}
368368

369369
// Success to clean up ipset and iptables operations in kernel and delete the cached network policy from RawNpMap

npm/pkg/controlplane/controllers/v2/networkPolicyController_test.go

Lines changed: 46 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
kubeinformers "k8s.io/client-go/informers"
2323
k8sfake "k8s.io/client-go/kubernetes/fake"
2424
"k8s.io/client-go/tools/cache"
25+
"k8s.io/utils/pointer"
2526
)
2627

2728
type netPolFixture struct {
@@ -633,12 +634,12 @@ func TestCountsAddAndDeleteNetPol(t *testing.T) {
633634
tests := []struct {
634635
name string
635636
// network policy to add
636-
netPolSpec *networkingv1.NetworkPolicySpec
637-
cidrCount int
638-
namedPortCount int
637+
netPolSpec *networkingv1.NetworkPolicySpec
638+
cidrCount int
639+
endPortCount int
639640
}{
640641
{
641-
name: "no-cidr-namedPort",
642+
name: "no-cidr-endPort",
642643
netPolSpec: &networkingv1.NetworkPolicySpec{
643644
PolicyTypes: []networkingv1.PolicyType{
644645
networkingv1.PolicyTypeIngress,
@@ -719,7 +720,7 @@ func TestCountsAddAndDeleteNetPol(t *testing.T) {
719720
cidrCount: 1,
720721
},
721722
{
722-
name: "namedPort-ingress",
723+
name: "endPort-ingress",
723724
netPolSpec: &networkingv1.NetworkPolicySpec{
724725
PolicyTypes: []networkingv1.PolicyType{
725726
networkingv1.PolicyTypeIngress,
@@ -728,16 +729,17 @@ func TestCountsAddAndDeleteNetPol(t *testing.T) {
728729
{
729730
Ports: []networkingv1.NetworkPolicyPort{
730731
{
731-
Port: &intstr.IntOrString{StrVal: "abc"},
732+
Port: &intstr.IntOrString{IntVal: 80},
733+
EndPort: pointer.Int32(85),
732734
},
733735
},
734736
},
735737
},
736738
},
737-
namedPortCount: 1,
739+
endPortCount: 1,
738740
},
739741
{
740-
name: "namedPort-egress",
742+
name: "endPort-egress",
741743
netPolSpec: &networkingv1.NetworkPolicySpec{
742744
PolicyTypes: []networkingv1.PolicyType{
743745
networkingv1.PolicyTypeEgress,
@@ -746,16 +748,17 @@ func TestCountsAddAndDeleteNetPol(t *testing.T) {
746748
{
747749
Ports: []networkingv1.NetworkPolicyPort{
748750
{
749-
Port: &intstr.IntOrString{StrVal: "abc"},
751+
Port: &intstr.IntOrString{IntVal: 80},
752+
EndPort: pointer.Int32(85),
750753
},
751754
},
752755
},
753756
},
754757
},
755-
namedPortCount: 1,
758+
endPortCount: 1,
756759
},
757760
{
758-
name: "cidr-and-namedPort",
761+
name: "cidr-and-endPort",
759762
netPolSpec: &networkingv1.NetworkPolicySpec{
760763
PolicyTypes: []networkingv1.PolicyType{
761764
networkingv1.PolicyTypeIngress,
@@ -771,14 +774,15 @@ func TestCountsAddAndDeleteNetPol(t *testing.T) {
771774
},
772775
Ports: []networkingv1.NetworkPolicyPort{
773776
{
774-
Port: &intstr.IntOrString{StrVal: "abc"},
777+
Port: &intstr.IntOrString{IntVal: 80},
778+
EndPort: pointer.Int32(85),
775779
},
776780
},
777781
},
778782
},
779783
},
780-
cidrCount: 1,
781-
namedPortCount: 1,
784+
cidrCount: 1,
785+
endPortCount: 1,
782786
},
783787
}
784788

@@ -807,28 +811,28 @@ func TestCountsAddAndDeleteNetPol(t *testing.T) {
807811
}
808812
checkNetPolTestResult("TestCountsCreateNetPol", f, testCases)
809813
require.Equal(t, tt.cidrCount, metrics.GetCidrNetPols())
810-
require.Equal(t, tt.namedPortCount, metrics.GetNamedPortNetPols())
814+
require.Equal(t, tt.endPortCount, metrics.GetEndPortNetPols())
811815

812816
deleteNetPol(t, f, netPolObj, DeletedFinalStateknownObject)
813817
testCases = []expectedNetPolValues{
814818
{0, 0, netPolPromVals{0, 1, 0, 1}},
815819
}
816820
checkNetPolTestResult("TestCountsDelNetPol", f, testCases)
817821
require.Equal(t, 0, metrics.GetCidrNetPols())
818-
require.Equal(t, 0, metrics.GetNamedPortNetPols())
822+
require.Equal(t, 0, metrics.GetEndPortNetPols())
819823
})
820824
}
821825
}
822826

823827
func TestCountsUpdateNetPol(t *testing.T) {
824828
tests := []struct {
825-
name string
826-
netPolSpec *networkingv1.NetworkPolicySpec
827-
updatedNetPolSpec *networkingv1.NetworkPolicySpec
828-
cidrCount int
829-
namedPortCount int
830-
updatedCidrCount int
831-
updatedNamedPortCount int
829+
name string
830+
netPolSpec *networkingv1.NetworkPolicySpec
831+
updatedNetPolSpec *networkingv1.NetworkPolicySpec
832+
cidrCount int
833+
endPortCount int
834+
updatedCidrCount int
835+
updatedEndPortCount int
832836
}{
833837
{
834838
name: "cidr-to-no-cidr",
@@ -942,7 +946,7 @@ func TestCountsUpdateNetPol(t *testing.T) {
942946
updatedCidrCount: 1,
943947
},
944948
{
945-
name: "namedPort-to-no-namedPort",
949+
name: "endPort-to-no-endPort",
946950
netPolSpec: &networkingv1.NetworkPolicySpec{
947951
PolicyTypes: []networkingv1.PolicyType{
948952
networkingv1.PolicyTypeIngress,
@@ -951,7 +955,8 @@ func TestCountsUpdateNetPol(t *testing.T) {
951955
{
952956
Ports: []networkingv1.NetworkPolicyPort{
953957
{
954-
Port: &intstr.IntOrString{StrVal: "abc"},
958+
Port: &intstr.IntOrString{IntVal: 80},
959+
EndPort: pointer.Int32(85),
955960
},
956961
},
957962
},
@@ -971,11 +976,11 @@ func TestCountsUpdateNetPol(t *testing.T) {
971976
},
972977
},
973978
},
974-
namedPortCount: 1,
975-
updatedNamedPortCount: 0,
979+
endPortCount: 1,
980+
updatedEndPortCount: 0,
976981
},
977982
{
978-
name: "no-namedPort-to-namedPort",
983+
name: "no-endPort-to-endPort",
979984
netPolSpec: &networkingv1.NetworkPolicySpec{
980985
PolicyTypes: []networkingv1.PolicyType{
981986
networkingv1.PolicyTypeIngress,
@@ -998,17 +1003,18 @@ func TestCountsUpdateNetPol(t *testing.T) {
9981003
{
9991004
Ports: []networkingv1.NetworkPolicyPort{
10001005
{
1001-
Port: &intstr.IntOrString{StrVal: "abc"},
1006+
Port: &intstr.IntOrString{IntVal: 80},
1007+
EndPort: pointer.Int32(85),
10021008
},
10031009
},
10041010
},
10051011
},
10061012
},
1007-
namedPortCount: 0,
1008-
updatedNamedPortCount: 1,
1013+
endPortCount: 0,
1014+
updatedEndPortCount: 1,
10091015
},
10101016
{
1011-
name: "namedPort-to-namedPort",
1017+
name: "endPort-to-endPort",
10121018
netPolSpec: &networkingv1.NetworkPolicySpec{
10131019
PolicyTypes: []networkingv1.PolicyType{
10141020
networkingv1.PolicyTypeIngress,
@@ -1017,7 +1023,8 @@ func TestCountsUpdateNetPol(t *testing.T) {
10171023
{
10181024
Ports: []networkingv1.NetworkPolicyPort{
10191025
{
1020-
Port: &intstr.IntOrString{StrVal: "abc"},
1026+
Port: &intstr.IntOrString{IntVal: 80},
1027+
EndPort: pointer.Int32(85),
10211028
},
10221029
},
10231030
},
@@ -1031,14 +1038,15 @@ func TestCountsUpdateNetPol(t *testing.T) {
10311038
{
10321039
Ports: []networkingv1.NetworkPolicyPort{
10331040
{
1034-
Port: &intstr.IntOrString{StrVal: "xyz"},
1041+
Port: &intstr.IntOrString{IntVal: 80},
1042+
EndPort: pointer.Int32(86),
10351043
},
10361044
},
10371045
},
10381046
},
10391047
},
1040-
namedPortCount: 1,
1041-
updatedNamedPortCount: 1,
1048+
endPortCount: 1,
1049+
updatedEndPortCount: 1,
10421050
},
10431051
}
10441052

@@ -1066,7 +1074,7 @@ func TestCountsUpdateNetPol(t *testing.T) {
10661074
}
10671075
checkNetPolTestResult("TestCountsAddNetPol", f, testCases)
10681076
require.Equal(t, tt.cidrCount, metrics.GetCidrNetPols())
1069-
require.Equal(t, tt.namedPortCount, metrics.GetNamedPortNetPols())
1077+
require.Equal(t, tt.endPortCount, metrics.GetEndPortNetPols())
10701078

10711079
newNetPolObj := createNetPol()
10721080
newNetPolObj.Spec = *tt.updatedNetPolSpec
@@ -1078,7 +1086,7 @@ func TestCountsUpdateNetPol(t *testing.T) {
10781086
}
10791087
checkNetPolTestResult("TestCountsUpdateNetPol", f, testCases)
10801088
require.Equal(t, tt.updatedCidrCount, metrics.GetCidrNetPols())
1081-
require.Equal(t, tt.updatedNamedPortCount, metrics.GetNamedPortNetPols())
1089+
require.Equal(t, tt.updatedEndPortCount, metrics.GetEndPortNetPols())
10821090
})
10831091
}
10841092
}

npm/pkg/controlplane/translation/translatePolicy.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -705,18 +705,26 @@ func HasCIDRBlock(netPolSpec *networkingv1.NetworkPolicySpec) bool {
705705
return false
706706
}
707707

708-
func HasNamedPort(netPolObj *networkingv1.NetworkPolicySpec) bool {
708+
func HasEndPort(netPolObj *networkingv1.NetworkPolicySpec) bool {
709709
for _, ingress := range netPolObj.Ingress {
710710
for _, port := range ingress.Ports {
711-
if t, err := portType(port); err == nil && t == namedPortType {
711+
if port.EndPort == nil {
712+
continue
713+
}
714+
715+
if t, err := portType(port); err == nil && t == numericPortType {
712716
return true
713717
}
714718
}
715719
}
716720

717721
for _, egress := range netPolObj.Egress {
718722
for _, port := range egress.Ports {
719-
if t, err := portType(port); err == nil && t == namedPortType {
723+
if port.EndPort == nil {
724+
continue
725+
}
726+
727+
if t, err := portType(port); err == nil && t == numericPortType {
720728
return true
721729
}
722730
}

0 commit comments

Comments
 (0)