Skip to content

Commit

Permalink
fix: track endport policies instead of namedport
Browse files Browse the repository at this point in the history
Signed-off-by: Hunter Gregory <42728408+huntergregory@users.noreply.github.com>
  • Loading branch information
huntergregory committed Jan 9, 2025
1 parent 22931a8 commit 49771e3
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 57 deletions.
4 changes: 2 additions & 2 deletions npm/metrics/ai-utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
16 changes: 8 additions & 8 deletions npm/metrics/counts.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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
}
12 changes: 6 additions & 6 deletions npm/pkg/controlplane/controllers/v2/networkPolicyController.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
90 changes: 52 additions & 38 deletions npm/pkg/controlplane/controllers/v2/networkPolicyController_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
},
}

Expand Down Expand Up @@ -807,28 +817,28 @@ 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{
{0, 0, netPolPromVals{0, 1, 0, 1}},
}
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",
Expand Down Expand Up @@ -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,
Expand All @@ -951,7 +961,8 @@ func TestCountsUpdateNetPol(t *testing.T) {
{
Ports: []networkingv1.NetworkPolicyPort{
{
Port: &intstr.IntOrString{StrVal: "abc"},
Port: &intstr.IntOrString{IntVal: 80},
EndPort: eightyFivePointer,
},
},
},
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -1017,7 +1029,8 @@ func TestCountsUpdateNetPol(t *testing.T) {
{
Ports: []networkingv1.NetworkPolicyPort{
{
Port: &intstr.IntOrString{StrVal: "abc"},
Port: &intstr.IntOrString{IntVal: 80},
EndPort: eightyFivePointer,
},
},
},
Expand All @@ -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,
},
}

Expand Down Expand Up @@ -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
Expand All @@ -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())
})
}
}
14 changes: 11 additions & 3 deletions npm/pkg/controlplane/translation/translatePolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -705,18 +705,26 @@ 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
}
}
}

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
}
}
Expand Down

0 comments on commit 49771e3

Please sign in to comment.