Issue 455 admin netpols test coverage

pkg/netpol/connlist/connlist_test.go

@@ -269,6 +269,16 @@ func TestConnlistAnalyzeFatalErrors(t *testing.T) {
 			dirName:          "anp_bad_path_test_18",
 			errorStrContains: netpolerrors.UnknownRuleActionErr,
 		},
+		{
+			name:             "Input_dir_has_an_admin_netpol_with_an_invalid_egress_cidr_peer_should_return_fatal_error",
+			dirName:          "anp_bad_path_test_19",
+			errorStrContains: netpolerrors.InvalidCIDRAddr,
+		},
+		{
+			name:             "Input_dir_has_admin_netpols_one_with_invalid_priority_should_return_fatal_error",
+			dirName:          "anp_bad_path_test_20",
+			errorStrContains: netpolerrors.PriorityValueErr("invalid-priority", 1001),
+		},
 		{
 			name:             "Input_dir_has_more_than_one_baseline_admin_netpol_should_return_fatal_error",
 			dirName:          "banp_bad_path_test_1",
@@ -339,6 +349,11 @@ func TestConnlistAnalyzeFatalErrors(t *testing.T) {
 			dirName:          "banp_bad_path_test_14",
 			errorStrContains: netpolerrors.UnknownRuleActionErr,
 		},
+		{
+			name:             "Input_dir_has_baseline_admin_netpol_with_an_invalid_egress_cidr_peer_should_return_fatal_error",
+			dirName:          "banp_bad_path_test_15",
+			errorStrContains: netpolerrors.InvalidCIDRAddr,
+		},
 	}
 	for _, tt := range cases {
 		tt := tt
@@ -1741,6 +1756,12 @@ var goodPathTests = []struct {
 		outputFormats:    []string{output.DefaultFormat},
 		exposureAnalysis: true,
 	},
+	{
+		// exposure test with excluded labeled pod from any namespace
+		testDirName:      "exposure_test_with_anp_16",
+		outputFormats:    ValidFormats,
+		exposureAnalysis: true,
+	},
 }
 
 func runParsedResourcesConnlistTests(t *testing.T, testList []examples.ParsedResourcesTest) {

pkg/netpol/diff/diff_test.go

@@ -802,4 +802,70 @@ var commonBadPathTestsFatalErr = []struct {
 		ref2:             "semanticDiff-same-topologies-old1",
 		errorStrContains: netpolerrors.NotSupportedPodResourcesErrorStr("demo/cog-agents"),
 	},
+	{
+		name:             "first_input_dir_has_two_admin_netpols_with_same_priority_should_return_fatal_error",
+		ref1:             "anp_bad_path_test_1",
+		ref2:             "anp_test_4",
+		errorStrContains: netpolerrors.PriorityErrExplain,
+	},
+	{
+		name:             "second_input_dir_has_an_admin_netpol_with_invalid_priority_should_return_fatal_error",
+		ref1:             "anp_test_4",
+		ref2:             "anp_bad_path_test_2",
+		errorStrContains: netpolerrors.PriorityValueErr("invalid-priority", 1001),
+	},
+	{
+		name:             "first_input_dir_has_two_admin_netpols_with_same_name_should_return_fatal_error",
+		ref1:             "anp_bad_path_test_3",
+		ref2:             "anp_test_4",
+		errorStrContains: netpolerrors.ANPsWithSameNameErr("same-name"),
+	},
+	{
+		name:             "first_input_dir_has_two_netpols_with_same_name_in_one_namespace_should_return_fatal_error",
+		ref1:             "np_bad_path_test_1",
+		ref2:             "ipblockstest",
+		errorStrContains: netpolerrors.NPWithSameNameError("default/backend-netpol"),
+	},
+	{
+		name:             "first_input_dir_has_an_admin_netpol_with_empty_subject_should_return_fatal_error",
+		ref1:             "anp_bad_path_test_4",
+		ref2:             "anp_test_4",
+		errorStrContains: netpolerrors.OneFieldSetSubjectErr,
+	},
+	{
+		name:             "second_input_dir_has_an_admin_netpol_with_an_invalid_egress_rule_peer_should_return_fatal_error",
+		ref1:             "anp_test_4",
+		ref2:             "anp_bad_path_test_7",
+		errorStrContains: netpolerrors.OneFieldSetRulePeerErr,
+	},
+	{
+		name:             "first_input_dir_has_an_admin_netpol_missing_ingress_rule_peer_should_return_fatal_error",
+		ref1:             "anp_bad_path_test_14",
+		ref2:             "anp_test_4",
+		errorStrContains: netpolerrors.ANPIngressRulePeersErr,
+	},
+	{
+		name:             "first_input_dir_has_an_admin_netpol_with_an_invalid_ingress_rule_port_should_return_fatal_error",
+		ref1:             "anp_bad_path_test_17",
+		ref2:             "anp_test_4",
+		errorStrContains: netpolerrors.ANPPortsError,
+	},
+	{
+		name:             "second_input_dir_has_baseline_admin_netpol_with_an_invalid_egress_rule_action_should_return_fatal_error",
+		ref1:             "banp_test_core_egress_sctp_rules",
+		ref2:             "banp_bad_path_test_8",
+		errorStrContains: netpolerrors.UnknownRuleActionErr,
+	},
+	{
+		name:             "first_input_dir_has_baseline_admin_netpol_with_an_invalid_ingress_rule_peer_should_return_fatal_error",
+		ref1:             "banp_bad_path_test_12",
+		ref2:             "banp_test_core_egress_sctp_rules",
+		errorStrContains: netpolerrors.OneFieldSetRulePeerErr,
+	},
+	{
+		name:             "second_input_dir_has_baseline_admin_netpol_with_an_invalid_egress_cidr_peer_should_return_fatal_error",
+		ref1:             "banp_test_core_egress_sctp_rules",
+		ref2:             "banp_bad_path_test_15",
+		errorStrContains: netpolerrors.InvalidCIDRAddr,
+	},
 }

pkg/netpol/eval/check.go

@@ -371,28 +371,25 @@ func (pe *PolicyEngine) allAllowedXgressConnections(src, dst k8s.Peer, isIngress
 
 	// in case of exposure-analysis: update cluster wide exposure data for relevant pod (src on egress, dst on ingress)
 	if pe.exposureAnalysisFlag && !podExposureUpdatedFlag {
-		clusterWideExposureFromAllLayers, err := allAllowedXgressConnsConsideringAllLayersConns(anpExposure, npExposure, defaultExposure)
-		if err != nil {
-			return nil, err
-		}
+		clusterWideExposureFromAllLayers := allAllowedXgressConnsConsideringAllLayersConns(anpExposure, npExposure, defaultExposure)
 		updatePeerXgressClusterWideExposure(clusterWideExposureFromAllLayers, src, dst, isIngress)
 	}
 	if anpConnsDeterminedAllFlag { // if all conns between the src and dst were determined by ANP layer, return the allowed
 		// conns from the ANP layer
 		return anpConns.layerConns.AllowedConns, nil
 	}
 	// return all allowed xgress connections between the src and dst (final result computed considering all layers conns)
-	return allAllowedXgressConnsConsideringAllLayersConns(anpConns, npConns, defaultConns)
+	return allAllowedXgressConnsConsideringAllLayersConns(anpConns, npConns, defaultConns), nil
 }
 
 // allAllowedXgressConnsConsideringAllLayersConns gets connections from all policies layers and compute the allowed connections on
 // the given direction considering on which policies-layer the xgress connection was captured and the precedence of each policies layer
 func allAllowedXgressConnsConsideringAllLayersConns(anpConns, npConns,
-	defaultOrBanpConns *policiesLayerXgressConns) (allowedConns *common.ConnectionSet, err error) {
+	defaultOrBanpConns *policiesLayerXgressConns) (allowedConns *common.ConnectionSet) {
 	switch {
 	case npConns.isCaptured && !anpConns.isCaptured:
 		// ANPs don't capture the connection; NPs capture the peers, return allowed conns from netpols
-		return npConns.layerConns.AllowedConns, nil
+		return npConns.layerConns.AllowedConns
 	case npConns.isCaptured && anpConns.isCaptured:
 		// if conns between src and dst (or between peer and entire-cluster) were captured by both the admin-network-policies and
 		// by network-policies
@@ -404,7 +401,7 @@ func allAllowedXgressConnsConsideringAllLayersConns(anpConns, npConns,
 		// 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.layerConns.CollectAllowedConnsFromNetpols(npConns.layerConns)
-		return anpConns.layerConns.AllowedConns, nil
+		return anpConns.layerConns.AllowedConns
 	default: // !npCaptured - netpols don't capture the connections between src and dst - delegate to banp
 		// possible cases :
 		// 1. ANPs capture the connection, netpols don't, return the allowed conns from ANPs considering default conns (& BANP)
@@ -414,7 +411,7 @@ func allAllowedXgressConnsConsideringAllLayersConns(anpConns, npConns,
 		// this also determines what happens on traffic (ports) which are not mentioned in the (B)ANPs;
 		// since (B)ANP rules are read as is only.
 		anpConns.layerConns.CollectConnsFromBANP(defaultOrBanpConns.layerConns)
-		return anpConns.layerConns.AllowedConns, nil
+		return anpConns.layerConns.AllowedConns
 	}
 }
 

pkg/netpol/eval/internal/k8s/adminnetpol.go

@@ -270,6 +270,7 @@ func doesNetworksFieldMatchPeer(networks []apisv1a.CIDR, peer Peer) (bool, error
 		// nothing to do with Peer type which is not IPBlock
 	}
 	for _, cidr := range networks {
+		// note that: if the cidr is invalid (will not get here), an error would be raised earlier by GetReferencedIPBlocks
 		isIPv6, err := isIPv6Cidr(cidr)
 		if err != nil { // invalid cidr
 			return false, err

pkg/netpol/eval/resources.go

@@ -934,6 +934,10 @@ func (pe *PolicyEngine) addRepresentativePod(podNs string, objSelectors *k8s.Sin
 			// if the objSelectors.NsSelector is nil and the podNs is empty, means inferred from an
 			// (baseline)AdminNetworkPolicy rule with nil namespaceSelector, which means that all namespaces match the rule;
 			// so as the RepresentativeNsLabelSelector will assign an empty namespaceSelector (matches all namespaces)
+			// * keeping this else although currently it is unreached - since `v1alpha1 Pods *NamespacedPod` field uses
+			// `metav1.LabelSelector` (and not pointers); which means it can not be nil, either empty or not.(i.e. already empty)
+			// so anyway generating a representative peer with empty namespaceSelector (all namespaces ) for
+			// cluster-scoped policy rule without namespaceSelector
 			nsLabelSelector = &metav1.LabelSelector{} // all namespaces
 		}
 	}

test_outputs/connlist/exposure_test_with_anp_16_exposure_output.csv

@@ -0,0 +1,18 @@
+src,dst,conn
+0.0.0.0-255.255.255.255,hello-world/workload-b[Deployment],All Connections
+hello-world/workload-a[Deployment],hello-world/workload-b[Deployment],All Connections
+hello-world/workload-b[Deployment],0.0.0.0-255.255.255.255,All Connections
+hello-world/workload-b[Deployment],hello-world/workload-a[Deployment],All Connections
+Exposure Analysis Result:,,
+Egress Exposure:,,
+src,dst,conn
+hello-world/workload-a[Deployment],[all namespaces]/[pod with {role=monitoring}],No Connections
+hello-world/workload-a[Deployment],entire-cluster,All Connections
+hello-world/workload-b[Deployment],0.0.0.0-255.255.255.255,All Connections
+hello-world/workload-b[Deployment],entire-cluster,All Connections
+Ingress Exposure:,,
+dst,src,conn
+hello-world/workload-a[Deployment],[all namespaces]/[pod with {role=monitoring}],No Connections
+hello-world/workload-a[Deployment],entire-cluster,All Connections
+hello-world/workload-b[Deployment],0.0.0.0-255.255.255.255,All Connections
+hello-world/workload-b[Deployment],entire-cluster,All Connections

test_outputs/connlist/exposure_test_with_anp_16_exposure_output.dot

@@ -0,0 +1,27 @@
+digraph {
+	subgraph "cluster_hello_world" {
+		color="black"
+		fontcolor="black"
+		"hello-world/workload-a[Deployment]" [label="workload-a[Deployment]" color="blue" fontcolor="blue"]
+		"hello-world/workload-b[Deployment]" [label="workload-b[Deployment]" color="blue" fontcolor="blue"]
+		label="hello-world"
+	}
+	subgraph "cluster_all namespaces" {
+		color="red2"
+		fontcolor="red2"
+		"pod with {role=monitoring}_in_all namespaces" [label="pod with {role=monitoring}" color="red2" fontcolor="red2"]
+		label="all namespaces"
+	}
+	"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" -> "hello-world/workload-b[Deployment]" [label="All Connections" color="gold2" fontcolor="darkgreen" weight=0.5]
+	"entire-cluster" -> "hello-world/workload-a[Deployment]" [label="All Connections" color="darkorange2" fontcolor="darkgreen" weight=1 style=dashed]
+	"entire-cluster" -> "hello-world/workload-b[Deployment]" [label="All Connections" color="darkorange2" fontcolor="darkgreen" weight=1 style=dashed]
+	"hello-world/workload-a[Deployment]" -> "entire-cluster" [label="All Connections" color="darkorange4" fontcolor="darkgreen" weight=0.5 style=dashed]
+	"hello-world/workload-a[Deployment]" -> "hello-world/workload-b[Deployment]" [label="All Connections" color="gold2" fontcolor="darkgreen" weight=0.5]
+	"hello-world/workload-a[Deployment]" -> "pod with {role=monitoring}_in_all namespaces" [label="No Connections" color="darkorange4" fontcolor="darkgreen" weight=0.5 style=dashed]
+	"hello-world/workload-b[Deployment]" -> "0.0.0.0-255.255.255.255" [label="All Connections" color="gold2" fontcolor="darkgreen" weight=1]
+	"hello-world/workload-b[Deployment]" -> "entire-cluster" [label="All Connections" color="darkorange4" fontcolor="darkgreen" weight=0.5 style=dashed]
+	"hello-world/workload-b[Deployment]" -> "hello-world/workload-a[Deployment]" [label="All Connections" color="gold2" fontcolor="darkgreen" weight=1]
+	"pod with {role=monitoring}_in_all namespaces" -> "hello-world/workload-a[Deployment]" [label="No Connections" color="darkorange2" fontcolor="darkgreen" weight=1 style=dashed]
+}