don't remove representative peers in any-namespace (#352)

* always keep representative peers which match all-namespaces

* update test output after merge

* examples with rules exposing pod to an existing ns

* Update pkg/netpol/eval/exposure.go

Co-authored-by: Adi Sosnovich <[email protected]>

* don't refine rep. peers matching any pod in a namespace + changing some tests to keep initial purpose+updating results of existing tests

* adding same test with nil podSelector instead of empty one

* adding test with inaccurate output

* fixing comment syntax

* Update pkg/netpol/connlist/connlist_test.go

Co-authored-by: Adi Sosnovich <[email protected]>

* gofmt

* updating comments in yaml

* tiny fix

---------

Co-authored-by: Adi Sosnovich <[email protected]>

pkg/netpol/connlist/connlist_test.go

@@ -918,4 +918,42 @@ var goodPathTests = []struct {
 		exposureAnalysis: true,
 		outputFormats:    []string{output.DOTFormat},
 	},
+	{
+		// test that when the rule enable any-namespace with podSelector, a representative peer is created even
+		// if there is a matching pod in a specific namespace
+		testDirName:      "test_exposure_to_any_namespace_with_podSelector",
+		exposureAnalysis: true,
+		outputFormats:    ExposureValidFormats,
+	},
+	{
+		testDirName:      "test_conn_to_all_pods_in_an_existing_ns",
+		exposureAnalysis: true,
+		outputFormats:    ExposureValidFormats,
+	},
+	{
+		testDirName:      "test_conn_to_new_pod_in_an_existing_ns",
+		exposureAnalysis: true,
+		outputFormats:    ExposureValidFormats,
+	},
+	{
+		testDirName:      "test_conn_to_all_pods_in_an_existing_ns_with_ns_selector_only",
+		exposureAnalysis: true,
+		outputFormats:    ExposureValidFormats,
+	},
+	{
+		// following test resources : contains two pods in different namespaces, and two policies, one for each namespace
+		// first policy captures: hello-world/workload-a and exposes it on Ingress to all pods in backend namespace
+		// second policy captures: backend/backend-app and denies all egress from it
+		// so as result hello-world/workload-a is actually exposed to all backend pods except for backend-app
+		// note: following exposure line in output :
+		// `hello-world/workload-a[Deployment]      <=      backend/[all pods] : TCP 8050`
+		// could have been more accurate with:
+		// `hello-world/workload-a[Deployment]      <=      backend/[pods without app: backend-app] : TCP 8050`
+		// but the goal is to hint where policy can be tightened, thus it is ok to ignore policies that capture
+		// representative peers in the analysis
+
+		testDirName:      "test_exposure_to_namespace_except_specific_pod",
+		exposureAnalysis: true,
+		outputFormats:    ExposureValidFormats,
+	},
 }

pkg/netpol/eval/exposure.go

@@ -68,12 +68,26 @@ func (pe *PolicyEngine) extractLabelsAndRefineRepresentativePeers(podObj *k8s.Po
 
 // refineRepresentativePeersMatchingLabels removes from the policy engine all representative peers
 // with labels matching the given labels of a real pod
+// representative peers matching any-namespace will not be removed.
 func (pe *PolicyEngine) refineRepresentativePeersMatchingLabels(realPodLabels, realNsLabels map[string]string) {
 	keysToDelete := make([]string, 0)
 	// look for representative peers with labels matching the given real pod's (and its namespace) labels
 	for key, peer := range pe.representativePeersMap {
 		potentialPodSelector := labels.SelectorFromSet(labels.Set(peer.Pod.Labels))
 		potentialNsSelector := labels.SelectorFromSet(labels.Set(peer.PotentialNamespaceLabels))
+		if potentialNsSelector.Empty() {
+			// empty --representative peer that matches any-namespace, thus will not be removed
+			// note that if the policy had nil namespaceSelector, it would be converted to the namespace of the policy
+			continue
+		}
+		if potentialPodSelector.Empty() {
+			// empty/nil podSelector means representative peer that matches any-pod in the representative namespace,
+			// thus will not be removed
+			// note that there is no representative peer with both empty namespace and pod selector; that case was handled
+			// in the general conns compute and won't get here.
+			continue
+		}
+		// remove representative peer matching both realPodLabels and realNsLabels.
 		if potentialPodSelector.Matches(labels.Set(realPodLabels)) && potentialNsSelector.Matches(labels.Set(realNsLabels)) {
 			keysToDelete = append(keysToDelete, key)
 		}

test_outputs/connlist/exposure_test_conn_to_all_pods_in_an_existing_ns_connlist_output.dot

@@ -0,0 +1,25 @@
+digraph {
+	subgraph "cluster_backend" {
+		color="black"
+		fontcolor="black"
+		"all pods_in_backend" [label="all pods" color="red2" fontcolor="red2"]
+		"backend/backend-app[Deployment]" [label="backend-app[Deployment]" color="blue" fontcolor="blue"]
+		label="backend"
+	}
+	subgraph "cluster_hello_world" {
+		color="black"
+		fontcolor="black"
+		"hello-world/workload-a[Deployment]" [label="workload-a[Deployment]" color="blue" fontcolor="blue"]
+		label="hello-world"
+	}
+	"0.0.0.0-255.255.255.255" [label="0.0.0.0-255.255.255.255" color="red2" fontcolor="red2"]
+	"entire-cluster" [label="entire-cluster" color="red2" fontcolor="red2" shape=diamond]
+	"0.0.0.0-255.255.255.255" -> "backend/backend-app[Deployment]" [label="All Connections" color="gold2" fontcolor="darkgreen"]
+	"all pods_in_backend" -> "hello-world/workload-a[Deployment]" [label="TCP 8050" color="gold2" fontcolor="darkgreen" weight=1]
+	"backend/backend-app[Deployment]" -> "0.0.0.0-255.255.255.255" [label="All Connections" color="gold2" fontcolor="darkgreen"]
+	"backend/backend-app[Deployment]" -> "entire-cluster" [label="All Connections" color="gold2" fontcolor="darkgreen" weight=0.5]
+	"backend/backend-app[Deployment]" -> "hello-world/workload-a[Deployment]" [label="TCP 8050" color="gold2" fontcolor="darkgreen"]
+	"entire-cluster" -> "backend/backend-app[Deployment]" [label="All Connections" color="gold2" fontcolor="darkgreen" weight=1]
+	"hello-world/workload-a[Deployment]" -> "backend/backend-app[Deployment]" [label="All Connections" color="gold2" fontcolor="darkgreen"]
+	"hello-world/workload-a[Deployment]" -> "entire-cluster" [label="All Connections" color="gold2" fontcolor="darkgreen" weight=0.5]
+}

test_outputs/connlist/exposure_test_conn_to_all_pods_in_an_existing_ns_connlist_output.txt

@@ -0,0 +1,19 @@
+0.0.0.0-255.255.255.255 => backend/backend-app[Deployment] : All Connections
+backend/backend-app[Deployment] => 0.0.0.0-255.255.255.255 : All Connections
+backend/backend-app[Deployment] => hello-world/workload-a[Deployment] : TCP 8050
+hello-world/workload-a[Deployment] => backend/backend-app[Deployment] : All Connections
+
+Exposure Analysis Result:
+Egress Exposure:
+backend/backend-app[Deployment]    	=> 	0.0.0.0-255.255.255.255 : All Connections
+backend/backend-app[Deployment]    	=> 	entire-cluster : All Connections
+hello-world/workload-a[Deployment] 	=> 	entire-cluster : All Connections
+
+Ingress Exposure:
+backend/backend-app[Deployment]    	<= 	0.0.0.0-255.255.255.255 : All Connections
+backend/backend-app[Deployment]    	<= 	entire-cluster : All Connections
+hello-world/workload-a[Deployment] 	<= 	backend/[all pods] : TCP 8050
+
+Workloads not protected by network policies:
+backend/backend-app[Deployment] is not protected on Egress
+backend/backend-app[Deployment] is not protected on Ingress

test_outputs/connlist/exposure_test_conn_to_all_pods_in_an_existing_ns_with_ns_selector_only_connlist_output.dot

@@ -0,0 +1,25 @@
+digraph {
+	subgraph "cluster_backend" {
+		color="black"
+		fontcolor="black"
+		"all pods_in_backend" [label="all pods" color="red2" fontcolor="red2"]
+		"backend/backend-app[Deployment]" [label="backend-app[Deployment]" color="blue" fontcolor="blue"]
+		label="backend"
+	}
+	subgraph "cluster_hello_world" {
+		color="black"
+		fontcolor="black"
+		"hello-world/workload-a[Deployment]" [label="workload-a[Deployment]" color="blue" fontcolor="blue"]
+		label="hello-world"
+	}
+	"0.0.0.0-255.255.255.255" [label="0.0.0.0-255.255.255.255" color="red2" fontcolor="red2"]
+	"entire-cluster" [label="entire-cluster" color="red2" fontcolor="red2" shape=diamond]
+	"0.0.0.0-255.255.255.255" -> "backend/backend-app[Deployment]" [label="All Connections" color="gold2" fontcolor="darkgreen"]
+	"all pods_in_backend" -> "hello-world/workload-a[Deployment]" [label="TCP 8050" color="gold2" fontcolor="darkgreen" weight=1]
+	"backend/backend-app[Deployment]" -> "0.0.0.0-255.255.255.255" [label="All Connections" color="gold2" fontcolor="darkgreen"]
+	"backend/backend-app[Deployment]" -> "entire-cluster" [label="All Connections" color="gold2" fontcolor="darkgreen" weight=0.5]
+	"backend/backend-app[Deployment]" -> "hello-world/workload-a[Deployment]" [label="TCP 8050" color="gold2" fontcolor="darkgreen"]
+	"entire-cluster" -> "backend/backend-app[Deployment]" [label="All Connections" color="gold2" fontcolor="darkgreen" weight=1]
+	"hello-world/workload-a[Deployment]" -> "backend/backend-app[Deployment]" [label="All Connections" color="gold2" fontcolor="darkgreen"]
+	"hello-world/workload-a[Deployment]" -> "entire-cluster" [label="All Connections" color="gold2" fontcolor="darkgreen" weight=0.5]
+}