Merge branch 'support_admin_netpolicy' into support_anp_for_eval_cmd

.github/workflows/codeql-analysis.yml

@@ -41,11 +41,11 @@ jobs:
 
     steps:
     - name: Checkout repository
-      uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332
+      uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683
 
     # Initializes the CodeQL tools for scanning.
     - name: Initialize CodeQL
-      uses: github/codeql-action/init@eb055d739abdc2e8de2e5f4ba1a8b246daa779aa
+      uses: github/codeql-action/init@f779452ac5af1c261dce0346a8f964149f49322b
       with:
         languages: ${{ matrix.language }}
         # If you wish to specify custom queries, you can do so here or in a config file.
@@ -56,7 +56,7 @@ jobs:
     # Autobuild attempts to build any compiled languages  (C/C++, C#, or Java).
     # If this step fails, then you should remove it and run the build manually (see below)
     - name: Autobuild
-      uses: github/codeql-action/autobuild@eb055d739abdc2e8de2e5f4ba1a8b246daa779aa
+      uses: github/codeql-action/autobuild@f779452ac5af1c261dce0346a8f964149f49322b
 
     # ℹ️ Command-line programs to run using the OS shell.
     # 📚 https://git.io/JvXDl
@@ -70,4 +70,4 @@ jobs:
     #   make release
 
     - name: Perform CodeQL Analysis
-      uses: github/codeql-action/analyze@eb055d739abdc2e8de2e5f4ba1a8b246daa779aa
+      uses: github/codeql-action/analyze@f779452ac5af1c261dce0346a8f964149f49322b

.github/workflows/go-build.yml

@@ -13,7 +13,7 @@ jobs:
   build-and-test:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332
+    - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683
 
     - name: Set up Go
       uses: actions/setup-go@0a12ed9d6a96ab950c8f026ed9f722fe0da7ef32

.github/workflows/golangci-lint.yml

@@ -11,11 +11,11 @@ jobs:
     name: golangci-lint
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332
+      - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683
       - uses: actions/setup-go@0a12ed9d6a96ab950c8f026ed9f722fe0da7ef32
         with:
           go-version-file: ./go.mod
           cache: false
-      - uses: golangci/golangci-lint-action@aaa42aa0628b4ae2578232a66b541047968fac86
+      - uses: golangci/golangci-lint-action@971e284b6050e8a5849b72094c50ab08da042db8
         with:
           version: latest

.github/workflows/make-release.yaml

@@ -14,7 +14,7 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - name: Check out the repo
-        uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332
+        uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683
 
       - name: Set up Go
         uses: actions/setup-go@0a12ed9d6a96ab950c8f026ed9f722fe0da7ef32

go.mod

@@ -1,10 +1,10 @@
 module github.com/np-guard/netpol-analyzer
 
-go 1.21
+go 1.22
 
 require (
 	github.com/hashicorp/golang-lru/v2 v2.0.7
-	github.com/np-guard/models v0.3.4
+	github.com/np-guard/models v0.5.2
 	github.com/openshift/api v0.0.0-20230502160752-c71432710382
 	github.com/spf13/cobra v1.8.1
 	github.com/stretchr/testify v1.9.0

go.sum

@@ -96,8 +96,8 @@ github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 h1:n6/
 github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00/go.mod h1:Pm3mSP3c5uWn86xMLZ5Sa7JB9GsEZySvHYXCTK4E9q4=
 github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA=
 github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ=
-github.com/np-guard/models v0.3.4 h1:HOhVi6wyGvo+KmYBnQ5Km5HYCF+/PQlDs1v7mL1v05g=
-github.com/np-guard/models v0.3.4/go.mod h1:mqE2Irf8r+7HWh8fII0fWbWyQRMHGEo2SgSLN/6VKs8=
+github.com/np-guard/models v0.5.2 h1:lty+shExffJpMQyu36a/NBYEky/rjEddQid4GOVHnhs=
+github.com/np-guard/models v0.5.2/go.mod h1:dqRdt5EQID1GmHuYsMOJzg4sS104om6NwEZ6sVO55z8=
 github.com/onsi/ginkgo/v2 v2.13.0 h1:0jY9lJquiL8fcf3M4LAXN5aMlS/b2BV86HFFPCPMgE4=
 github.com/onsi/ginkgo/v2 v2.13.0/go.mod h1:TE309ZR8s5FsKKpuB1YAQYBzCaAfUgatB/xlT/ETL/o=
 github.com/onsi/gomega v1.29.0 h1:KIA/t2t5UBzoirT4H9tsML45GEbo3ouUnBHsCfD2tVg=

pkg/netpol/connlist/connlist_test.go

@@ -15,6 +15,7 @@ import (
 	"github.com/np-guard/netpol-analyzer/pkg/internal/output"
 	"github.com/np-guard/netpol-analyzer/pkg/internal/testutils"
 	"github.com/np-guard/netpol-analyzer/pkg/manifests/fsscanner"
+	"github.com/np-guard/netpol-analyzer/pkg/netpol/internal/examples"
 
 	"github.com/stretchr/testify/require"
 )
@@ -1341,7 +1342,7 @@ var goodPathTests = []struct {
 	},
 }
 
-func runParsedResourcesConnlistTests(t *testing.T, testList []testutils.ParsedResourcesTest) {
+func runParsedResourcesConnlistTests(t *testing.T, testList []examples.ParsedResourcesTest) {
 	t.Helper()
 	for i := 0; i < len(testList); i++ {
 		test := &testList[i]
@@ -1359,9 +1360,9 @@ func runParsedResourcesConnlistTests(t *testing.T, testList []testutils.ParsedRe
 }
 
 func TestAllParsedResourcesConnlistTests(t *testing.T) {
-	runParsedResourcesConnlistTests(t, testutils.ANPConnectivityFromParsedResourcesTest)
-	runParsedResourcesConnlistTests(t, testutils.BANPConnectivityFromParsedResourcesTest)
-	runParsedResourcesConnlistTests(t, testutils.ANPWithNetPolV1FromParsedResourcesTest)
-	runParsedResourcesConnlistTests(t, testutils.BANPWithNetPolV1FromParsedResourcesTest)
-	runParsedResourcesConnlistTests(t, testutils.ANPWithBANPFromParsedResourcesTest)
+	runParsedResourcesConnlistTests(t, examples.ANPConnectivityFromParsedResourcesTest)
+	runParsedResourcesConnlistTests(t, examples.BANPConnectivityFromParsedResourcesTest)
+	runParsedResourcesConnlistTests(t, examples.ANPWithNetPolV1FromParsedResourcesTest)
+	runParsedResourcesConnlistTests(t, examples.BANPWithNetPolV1FromParsedResourcesTest)
+	runParsedResourcesConnlistTests(t, examples.ANPWithBANPFromParsedResourcesTest)
 }

pkg/netpol/eval/check.go

@@ -15,7 +15,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 
-	"github.com/np-guard/models/pkg/ipblock"
+	"github.com/np-guard/models/pkg/netset"
 
 	"github.com/np-guard/netpol-analyzer/pkg/internal/netpolerrors"
 	"github.com/np-guard/netpol-analyzer/pkg/netpol/eval/internal/k8s"
@@ -110,7 +110,7 @@ func (pe *PolicyEngine) getPoliciesSelectingPod(peer k8s.Peer, direction netv1.P
 // isPeerNodeIP returns true if peer1 is an IP address of a node and peer2 is a pod on that node
 func isPeerNodeIP(peer1, peer2 k8s.Peer) bool {
 	if peer2.PeerType() == k8s.PodType && peer1.PeerType() == k8s.IPBlockType {
-		ip2, err := ipblock.FromIPAddress(peer2.GetPeerPod().HostIP)
+		ip2, err := netset.IPBlockFromIPAddress(peer2.GetPeerPod().HostIP)
 		if err != nil {
 			return peer1.GetPeerIPBlock().Equal(ip2)
 		}
@@ -133,15 +133,15 @@ func isPodToItself(peer1, peer2 k8s.Peer) bool {
 func (pe *PolicyEngine) getPeer(p string) (k8s.Peer, error) {
 	// check if input peer is cidr
 	if _, _, err := net.ParseCIDR(p); err == nil {
-		peerIPBlock, err := ipblock.FromCidr(p)
+		peerIPBlock, err := netset.IPBlockFromCidr(p)
 		if err != nil {
 			return nil, err
 		}
 		return &k8s.IPBlockPeer{IPBlock: peerIPBlock}, nil
 	}
 	// check if input peer is an ip address
 	if net.ParseIP(p) != nil {
-		peerIPBlock, err := ipblock.FromIPAddress(p)
+		peerIPBlock, err := netset.IPBlockFromIPAddress(p)
 		if err != nil {
 			return nil, err
 		}
@@ -285,63 +285,56 @@ func (pe *PolicyEngine) allAllowedXgressConnections(src, dst k8s.Peer, isIngress
 	if err != nil {
 		return nil, err
 	}
+	// optimization: if all the conns between src and dst were determined by the ANPs : return the allowed conns
+	if anpCaptured && anpConns.DeterminesAllConns() {
+		return anpConns.AllowedConns, nil
+	}
 	// second get the allowed xgress conns between the src and dst from the netpols
 	// note that :
 	// - npConns contains only allowed connections
-	// - npCaptured is true iff there are policies selecting either src or dst - since network-policies' rules contain
-	// implicit deny on Pods selected by them.
+	// - npCaptured is true iff there are policies selecting either src or dst based on the checked direction (ingress/ egress)
 	npConns, npCaptured, err := pe.getAllAllowedXgressConnsFromNetpols(src, dst, isIngress)
 	if err != nil {
 		return nil, err
 	}
 
 	// compute the allowed connections on the given direction considering the which policies captured the xgress connection
 	// and precedence of each policy type:
-	if anpCaptured && npCaptured {
+	switch npCaptured {
+	case true: // npCaptured
+		if !anpCaptured { // npCaptured && !anpCaptured
+			// only NPs capture the peers, return allowed conns from netpols
+			return npConns.AllowedConns, nil
+		}
+		// else: npCaptured && anpCaptured
 		// if conns between src and dst were captured by both the admin-network-policies and by network-policies
 		// collect conns:
 		// - traffic that was allowed or denied by ANPs will not be affected by the netpol conns.
-		// -  traffic that has no match in ANPs but allowed by NPs is added to allowed conns.
+		// - traffic that has no match in ANPs but allowed by NPs is added to allowed conns.
 		// - pass conns from ANPs, are determined by NPs conns;
 		// note that allowed conns by netpols, imply deny on other traffic;
 		// so ANPs.pass conns which intersect with NPs.allowed are added to allowed conns result;
 		// other pass conns (which don't intersect with NPs allowed conns) are not allowed implicitly.
-		anpConns.CollectConnsFromLowerPrecedencePolicyType(npConns)
+		anpConns.CollectAllowedConnsFromNetpols(npConns)
+		return anpConns.AllowedConns, nil
+	default: // !npCaptured - netpols don't capture the connections between src and dst - delegate to banp
+		// get default xgress connection between src and dst from the BANP/ system-default;
+		// note that :
+		// - if there is no banp in the input resources, then default conns is system-default which is allow-all
+		// - if the banp captures the xgress between src and dst; then defaultConns may contain allowed and denied conns
+		defaultConns, err := pe.getXgressDefaultConns(src, dst, isIngress)
+		if err != nil {
+			return nil, err
+		}
+		if !anpCaptured { // !anpCaptured && !npCaptured:
+			return defaultConns.AllowedConns, nil
+		}
+		// else ( anpCaptured && !npCaptured)
+		// ANPs capture the peers, netpols don't , return the allowed conns from ANPs considering default conns
+		// this also determines what happens on traffic (ports) which are not mentioned in the ANPs; since ANP rules are read as is only
+		anpConns.CollectConnsFromBANP(defaultConns)
 		return anpConns.AllowedConns, nil
 	}
-	if !anpCaptured && npCaptured {
-		// only NPs capture the peers, return allowed conns from netpols
-		return npConns.AllowedConns, nil
-	}
-	// otherwise,n getting here means network-policies don't capture the xgress direction traffic between src and dst.
-	// get default xgress connection between src and dst from the BANP/ system-default;
-	// note that :
-	// - if there is no banp in the input resources, then default conns is system-default which is allow-all
-	// - if the banp captures the xgress between src and dst; then defaultConns may contain allowed and denied conns
-	defaultConns, err := pe.getXgressDefaultConns(src, dst, isIngress)
-	if err != nil {
-		return nil, err
-	}
-	if !anpCaptured && !npCaptured { // only BANP captures the xgress between src -> dst (or not captured at all)
-		// if no ANPs nor NPs capturing the xgress connection, return the default allowed conns (from BANP or system-default).
-		// note that: if conns are not captured by an ANP/NP but captured only by BANP, then:
-		// if BANP denies some conns but has no allow rule then, allowed conns are all but the denied conns:
-		if defaultConns.AllowedConns.IsEmpty() && !defaultConns.DeniedConns.IsEmpty() {
-			allowedConns := common.MakeConnectionSet(true)
-			allowedConns.Subtract(defaultConns.DeniedConns)
-			return allowedConns, nil
-		} // else return the allowed conns by BANP
-		return defaultConns.AllowedConns, nil
-	}
-	// else ( anpCaptured && !npCaptured)
-	// ANPs capture the peers, netpols don't , return the allowed conns from ANPs considering default conns
-	// this determines what happens on traffic (ports) which are not mentioned in the ANPs; since ANP rules are read as is only
-	anpConns.CollectConnsFromLowerPrecedencePolicyType(defaultConns)
-	// note that : BANP rules may not match all ANPs.Pass conns, remaining pass conns will be allowed as system-default
-	if !anpConns.PassConns.IsEmpty() {
-		anpConns.AllowedConns.Union(anpConns.PassConns)
-	}
-	return anpConns.AllowedConns, nil
 }
 
 // analyzing network-policies for conns between peers (object kind == NetworkPolicy):
@@ -397,7 +390,7 @@ func (pe *PolicyEngine) getAllAllowedXgressConnsFromNetpols(src, dst k8s.Peer, i
 		allowedConns.Union(policyAllowedConnectionsPerDirection)
 	}
 	// putting the result in policiesConns object to be compared with conns allowed by ANP/BANP later
-	policiesConns = k8s.InitEmptyPolicyConnections()
+	policiesConns = k8s.NewPolicyConnections()
 	policiesConns.AllowedConns = allowedConns
 	return policiesConns, true, nil
 }
@@ -454,13 +447,13 @@ func updatePeerXgressClusterWideExposure(policy *k8s.NetworkPolicy, src, dst k8s
 // the Pods selected by the AdminNetworkPolicy, as opposed to implicit deny NetworkPolicy rules imply.
 func (pe *PolicyEngine) getAllAllowedXgressConnectionsFromANPs(src, dst k8s.Peer, isIngress bool) (policiesConns *k8s.PolicyConnections,
 	captured bool, err error) {
-	policiesConns = k8s.InitEmptyPolicyConnections()
+	policiesConns = k8s.NewPolicyConnections()
 	// iterate the sorted admin network policies in order to compute the allowed, pass, and denied xgress connections between the peers
 	// from the admin netpols capturing the src (if !isIngress)/ capturing the dst (if isIngress true).
 	// connections are computed considering ANPs priorities (rules of an ANP with lower priority take precedence on other ANPs rules)
 	// and rules ordering in single ANP (coming first takes precedence).
 	for _, anp := range pe.sortedAdminNetpols {
-		singleANPConns := k8s.InitEmptyPolicyConnections()
+		singleANPConns := k8s.NewPolicyConnections()
 		// collect the allowed, pass, and denied connectivity from the relevant rules into policiesConns
 		if !isIngress { // egress
 			selectsSrc, err := anp.Selects(src, false)
@@ -508,7 +501,7 @@ func (pe *PolicyEngine) getAllAllowedXgressConnectionsFromANPs(src, dst k8s.Peer
 // if there is no BANP or if the BANP does not capture connections between src and dst, then default allow-all connections is returned.
 // - note that the result may contain allowed / denied connections.
 func (pe *PolicyEngine) getXgressDefaultConns(src, dst k8s.Peer, isIngress bool) (*k8s.PolicyConnections, error) {
-	res := k8s.InitEmptyPolicyConnections()
+	res := k8s.NewPolicyConnections()
 	if pe.baselineAdminNetpol == nil {
 		res.AllowedConns = common.MakeConnectionSet(true)
 		return res, nil
@@ -524,22 +517,28 @@ func (pe *PolicyEngine) getXgressDefaultConns(src, dst k8s.Peer, isIngress bool)
 			if err != nil {
 				return nil, err
 			}
-		} else { // egress (!isIngress)
-			selectsSrc, err := pe.baselineAdminNetpol.Selects(src, false)
+		}
+	} else { // egress (!isIngress)
+		selectsSrc, err := pe.baselineAdminNetpol.Selects(src, false)
+		if err != nil {
+			return nil, err
+		}
+		// if the banp selects the src on egress, get egress conns
+		if selectsSrc {
+			res, err = pe.baselineAdminNetpol.GetEgressPolicyConns(dst)
 			if err != nil {
 				return nil, err
 			}
-			// if the banp selects the src on egress, get egress conns
-			if selectsSrc {
-				res, err = pe.baselineAdminNetpol.GetEgressPolicyConns(dst)
-				if err != nil {
-					return nil, err
-				}
-			}
 		}
 	}
 	if res.IsEmpty() { // banp rules didn't capture xgress conn between src and dst, return system-default: allow-all
 		res.AllowedConns = common.MakeConnectionSet(true)
 	}
+	if res.AllowedConns.IsEmpty() && !res.DeniedConns.IsEmpty() { // banp rules deny some connection but don't capture other conns
+		allowedConns := common.MakeConnectionSet(true)
+		allowedConns.Subtract(res.DeniedConns)
+		res.AllowedConns = allowedConns
+		return res, nil
+	}
 	return res, nil
 }

pkg/netpol/eval/eval_test.go

@@ -32,6 +32,7 @@ import (
 	"github.com/np-guard/netpol-analyzer/pkg/manifests/fsscanner"
 	"github.com/np-guard/netpol-analyzer/pkg/manifests/parser"
 	"github.com/np-guard/netpol-analyzer/pkg/netpol/internal/common"
+	"github.com/np-guard/netpol-analyzer/pkg/netpol/internal/examples"
 )
 
 const (
@@ -1822,7 +1823,7 @@ func pickUncontainedConn(conn *common.ConnectionSet) (resProtocol, resPort strin
 	return pickContainedConn(complementSet)
 }
 
-func runParsedResourcesEvalTests(t *testing.T, testList []testutils.ParsedResourcesTest) {
+func runParsedResourcesEvalTests(t *testing.T, testList []examples.ParsedResourcesTest) {
 	t.Helper()
 	for i := 0; i < len(testList); i++ {
 		test := &testList[i]
@@ -1863,9 +1864,9 @@ func runParsedResourcesEvalTests(t *testing.T, testList []testutils.ParsedResour
 }
 
 func TestAllParsedResourcesEvalTests(t *testing.T) {
-	runParsedResourcesEvalTests(t, testutils.ANPConnectivityFromParsedResourcesTest)
-	runParsedResourcesEvalTests(t, testutils.BANPConnectivityFromParsedResourcesTest)
-	runParsedResourcesEvalTests(t, testutils.ANPWithNetPolV1FromParsedResourcesTest)
-	runParsedResourcesEvalTests(t, testutils.BANPWithNetPolV1FromParsedResourcesTest)
-	runParsedResourcesEvalTests(t, testutils.ANPWithBANPFromParsedResourcesTest)
+	runParsedResourcesEvalTests(t, examples.ANPConnectivityFromParsedResourcesTest)
+	runParsedResourcesEvalTests(t, examples.BANPConnectivityFromParsedResourcesTest)
+	runParsedResourcesEvalTests(t, examples.ANPWithNetPolV1FromParsedResourcesTest)
+	runParsedResourcesEvalTests(t, examples.BANPWithNetPolV1FromParsedResourcesTest)
+	runParsedResourcesEvalTests(t, examples.ANPWithBANPFromParsedResourcesTest)
 }