Improving explainability print format.

pkg/netpol/connlist/connlist.go

@@ -550,8 +550,8 @@ func (ca *ConnlistAnalyzer) getConnectionsBetweenPeers(pe *eval.PolicyEngine, pe
 					return nil, nil, err
 				}
 			}
-			// skip empty connections
-			if allowedConnections.IsEmpty() {
+			// skip empty connections when running without explainability
+			if allowedConnections.IsEmpty() && !ca.explain {
 				continue
 			}
 			p2pConnection, err := ca.getP2PConnOrUpdateExposureConn(pe, allowedConnections, srcPeer, dstPeer, exposureMaps)

pkg/netpol/connlist/conns_formatter.go

@@ -74,7 +74,7 @@ func (c singleConnFields) string() string {
 }
 
 func (c singleConnFields) stringWithExplanation() string {
-	return fmt.Sprintf("%s => %s :\n%s", c.Src, c.Dst, c.Explanation)
+	return fmt.Sprintf("CONNECTIONS BETWEEN %s => %s:\n\n%s", c.Src, c.Dst, c.Explanation)
 }
 
 // formSingleP2PConn returns a string representation of single connection fields as singleConnFields object

pkg/netpol/eval/check.go

@@ -151,7 +151,10 @@ func (pe *PolicyEngine) allAllowedConnectionsBetweenPeers(srcPeer, dstPeer Peer)
 	var err error
 	// cases where any connection is always allowed
 	if isPodToItself(srcK8sPeer, dstK8sPeer) || isPeerNodeIP(srcK8sPeer, dstK8sPeer) || isPeerNodeIP(dstK8sPeer, srcK8sPeer) {
-		return common.MakeConnectionSet(true), nil
+		res = common.MakeConnectionSet(true)
+		res.AddCommonImplyingRule(common.PodToItselfRule, true)
+		res.AddCommonImplyingRule(common.PodToItselfRule, false)
+		return res, nil
 	}
 	// egress: get egress allowed connections between the src and dst by
 	// walking through all k8s egress policies capturing the src;
@@ -576,8 +579,6 @@ func (pe *PolicyEngine) getAllAllowedXgressConnectionsFromANPs(src, dst k8s.Peer
 	return policiesConns, true, nil
 }
 
-const systemDefaultRule = "system default: allow all"
-
 // analyzing baseline-admin-network-policy for conns between peers (object kind == BaselineAdminNetworkPolicy):
 
 // getXgressDefaultConns returns the default connections between src and dst on the given direction (ingress/egress);
@@ -589,8 +590,7 @@ const systemDefaultRule = "system default: allow all"
 func (pe *PolicyEngine) getXgressDefaultConns(src, dst k8s.Peer, isIngress bool) (*k8s.PolicyConnections, error) {
 	res := k8s.NewPolicyConnections()
 	if pe.baselineAdminNetpol == nil {
-		res.AllowedConns = common.MakeConnectionSet(true)
-		res.AllowedConns.AddCommonImplyingRule(systemDefaultRule, isIngress)
+		res.AllowedConns = common.MakeAllConnectionSetWithRule(common.SystemDefaultRule, isIngress)
 		return res, nil
 	}
 	if isIngress { // ingress
@@ -621,7 +621,6 @@ func (pe *PolicyEngine) getXgressDefaultConns(src, dst k8s.Peer, isIngress bool)
 	// if banp rules didn't capture xgress conn between src and dst, return system-default: allow-all;
 	// if banp rule captured xgress conn, only DeniedConns should be impacted by banp rule,
 	// whenever AllowedConns should anyway be system-default: allow-all
-	res.AllowedConns = common.MakeConnectionSet(true)
-	res.AllowedConns.AddCommonImplyingRule(systemDefaultRule, isIngress)
+	res.AllowedConns = common.MakeAllConnectionSetWithRule(common.SystemDefaultRule, isIngress)
 	return res, nil
 }

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

@@ -169,7 +169,7 @@ func updatePolicyConns(rulePorts *[]apisv1a.AdminNetworkPolicyPort, ruleName str
 // ruleConnections returns the connectionSet from the current rule.Ports
 func ruleConnections(ports *[]apisv1a.AdminNetworkPolicyPort, ruleName string, dst Peer, isIngress bool) (*common.ConnectionSet, error) {
 	if ports == nil {
-		return common.MakeConnectionSet(true), nil // If Ports is not set then the rule does not filter traffic via port.
+		return common.MakeAllConnectionSetWithRule(ruleName, isIngress), nil // If Ports is not set then the rule does not filter traffic via port.
 	}
 	res := common.MakeConnectionSet(false)
 	for _, anpPort := range *ports {
@@ -282,7 +282,7 @@ func (anp *AdminNetworkPolicy) anpRuleErr(ruleName, description string) error {
 }
 
 func (anp *AdminNetworkPolicy) fullName() string {
-	return anp.Name
+	return "[ANP] " + anp.Name
 }
 
 func ruleFullName(policyName, ruleName, action string, isIngress bool) string {
@@ -300,7 +300,7 @@ func (anp *AdminNetworkPolicy) GetIngressPolicyConns(src, dst Peer) (*PolicyConn
 		rulePeers := rule.From
 		rulePorts := rule.Ports
 		if err := updateConnsIfIngressRuleSelectsPeer(rulePeers, rulePorts,
-			ruleFullName("ANP "+anp.fullName(), rule.Name, string(rule.Action), true),
+			ruleFullName(anp.fullName(), rule.Name, string(rule.Action), true),
 			src, dst, res, string(rule.Action), false); err != nil {
 			return nil, anp.anpRuleErr(rule.Name, err.Error())
 		}
@@ -315,7 +315,7 @@ func (anp *AdminNetworkPolicy) GetEgressPolicyConns(dst Peer) (*PolicyConnection
 		rulePeers := rule.To
 		rulePorts := rule.Ports
 		if err := updateConnsIfEgressRuleSelectsPeer(rulePeers, rulePorts,
-			ruleFullName("ANP "+anp.fullName(), rule.Name, string(rule.Action), false),
+			ruleFullName(anp.fullName(), rule.Name, string(rule.Action), false),
 			dst, res, string(rule.Action), false); err != nil {
 			return nil, anp.anpRuleErr(rule.Name, err.Error())
 		}

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

@@ -47,7 +47,7 @@ func banpRuleErr(ruleName, description string) error {
 }
 
 func (banp *BaselineAdminNetworkPolicy) fullName() string {
-	return banp.Name
+	return "[BANP] " + banp.Name
 }
 
 // GetEgressPolicyConns returns the connections from the egress rules selecting the dst in spec of the baselineAdminNetworkPolicy
@@ -57,7 +57,7 @@ func (banp *BaselineAdminNetworkPolicy) GetEgressPolicyConns(dst Peer) (*PolicyC
 		rulePeers := rule.To
 		rulePorts := rule.Ports
 		if err := updateConnsIfEgressRuleSelectsPeer(rulePeers, rulePorts,
-			ruleFullName("BANP "+banp.fullName(), rule.Name, string(rule.Action), false),
+			ruleFullName(banp.fullName(), rule.Name, string(rule.Action), false),
 			dst, res, string(rule.Action), true); err != nil {
 			return nil, banpRuleErr(rule.Name, err.Error())
 		}
@@ -72,7 +72,7 @@ func (banp *BaselineAdminNetworkPolicy) GetIngressPolicyConns(src, dst Peer) (*P
 		rulePeers := rule.From
 		rulePorts := rule.Ports
 		if err := updateConnsIfIngressRuleSelectsPeer(rulePeers, rulePorts,
-			ruleFullName("BANP "+banp.fullName(), rule.Name, string(rule.Action), true),
+			ruleFullName(banp.fullName(), rule.Name, string(rule.Action), true),
 			src, dst, res, string(rule.Action), true); err != nil {
 			return nil, banpRuleErr(rule.Name, err.Error())
 		}

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

@@ -121,10 +121,9 @@ func isEmptyPortRange(start, end int32) bool {
 func (np *NetworkPolicy) ruleConnections(rulePorts []netv1.NetworkPolicyPort, dst Peer,
 	ruleIdx int, isIngress bool) (*common.ConnectionSet, error) {
 	if len(rulePorts) == 0 {
-		res := common.MakeConnectionSet(true) // If this field is empty or missing, this rule matches all ports
-		res.AddCommonImplyingRule(np.ruleName(ruleIdx, isIngress), isIngress)
-		return res, nil
+		// If this field is empty or missing, this rule matches all ports
 		// (traffic not restricted by port)
+		return common.MakeAllConnectionSetWithRule(np.ruleName(ruleIdx, isIngress), isIngress), nil
 	}
 	res := common.MakeConnectionSet(false)
 	ruleName := np.ruleName(ruleIdx, isIngress)
@@ -358,7 +357,11 @@ func (np *NetworkPolicy) nameWithDirection(isIngress bool) string {
 // GetEgressAllowedConns returns the set of allowed connections from any captured pod to the destination peer
 func (np *NetworkPolicy) GetEgressAllowedConns(dst Peer) (*common.ConnectionSet, error) {
 	res := common.MakeConnectionSet(false)
-	res.AddCommonImplyingRule(np.nameWithDirection(false), false)
+	if len(np.Spec.Egress) == 0 {
+		res.AddCommonImplyingRule(np.nameWithDirection(false), false)
+		return res, nil
+	}
+	dstSelectedByAnyRule := false
 	for idx, rule := range np.Spec.Egress {
 		rulePeers := rule.To
 		rulePorts := rule.Ports
@@ -369,6 +372,7 @@ func (np *NetworkPolicy) GetEgressAllowedConns(dst Peer) (*common.ConnectionSet,
 		if !peerSelected {
 			continue
 		}
+		dstSelectedByAnyRule = true
 		ruleConns, err := np.ruleConnections(rulePorts, dst, idx, false)
 		if err != nil {
 			return res, err
@@ -378,13 +382,20 @@ func (np *NetworkPolicy) GetEgressAllowedConns(dst Peer) (*common.ConnectionSet,
 			return res, nil
 		}
 	}
+	if !dstSelectedByAnyRule {
+		res.AddCommonImplyingRule(np.nameWithDirection(false), false)
+	}
 	return res, nil
 }
 
 // GetIngressAllowedConns returns the set of allowed connections to a captured dst pod from the src peer
 func (np *NetworkPolicy) GetIngressAllowedConns(src, dst Peer) (*common.ConnectionSet, error) {
 	res := common.MakeConnectionSet(false)
-	res.AddCommonImplyingRule(np.nameWithDirection(true), true)
+	if len(np.Spec.Ingress) == 0 {
+		res.AddCommonImplyingRule(np.nameWithDirection(true), true)
+		return res, nil
+	}
+	srcSelectedByAnyRule := false
 	for idx, rule := range np.Spec.Ingress {
 		rulePeers := rule.From
 		rulePorts := rule.Ports
@@ -395,7 +406,7 @@ func (np *NetworkPolicy) GetIngressAllowedConns(src, dst Peer) (*common.Connecti
 		if !peerSelected {
 			continue
 		}
-
+		srcSelectedByAnyRule = true
 		ruleConns, err := np.ruleConnections(rulePorts, dst, idx, true)
 		if err != nil {
 			return res, err
@@ -405,6 +416,9 @@ func (np *NetworkPolicy) GetIngressAllowedConns(src, dst Peer) (*common.Connecti
 			return res, nil
 		}
 	}
+	if !srcSelectedByAnyRule {
+		res.AddCommonImplyingRule(np.nameWithDirection(true), true)
+	}
 	return res, nil
 }
 
@@ -512,15 +526,15 @@ func (np *NetworkPolicy) Selects(p *Pod, direction netv1.PolicyType) (bool, erro
 }
 
 func (np *NetworkPolicy) fullName() string {
-	return types.NamespacedName{Name: np.Name, Namespace: np.Namespace}.String()
+	return "[NP] " + types.NamespacedName{Name: np.Name, Namespace: np.Namespace}.String()
 }
 
 func (np *NetworkPolicy) ruleName(ruleIdx int, isIngress bool) string {
 	xgress := egressName
 	if isIngress {
 		xgress = ingressName
 	}
-	return fmt.Sprintf("NP %s//%s rule #%d", np.fullName(), xgress, ruleIdx+1)
+	return fmt.Sprintf("%s//%s rule #%d", np.fullName(), xgress, ruleIdx+1)
 }
 
 // /////////////////////////////////////////////////////////////////////////////////////////////

pkg/netpol/internal/common/augmented_intervalset.go

@@ -62,10 +62,22 @@ func (rules ImplyingRulesType) Copy() ImplyingRulesType {
 }
 
 const (
-	explTitle = "due to the following policices//rules:"
-	newLine   = "\n"
+	ExplWithRulesTitle    = "due to the following policices//rules:"
+	IngressDirectionTitle = "INGRESS DIRECTION:"
+	EgressDirectionTitle  = "EGRESS DIRECTION:"
+	NewLine               = "\n"
+	SystemDefaultRule     = "the system default: allow all"
+	PodToItselfRule       = "pod to itself: allow all"
+	ExplSystemDefault     = "due to " + SystemDefaultRule
 )
 
+func (rules *ImplyingXgressRulesType) onlySystemDefaultRule() bool {
+	if _, ok := (*rules)[SystemDefaultRule]; ok {
+		return len(*rules) == 1
+	}
+	return false
+}
+
 func (rules *ImplyingXgressRulesType) String() string {
 	if rules.Empty() {
 		return ""
@@ -76,21 +88,34 @@ func (rules *ImplyingXgressRulesType) String() string {
 		formattedRules = append(formattedRules, fmt.Sprintf("%d) %s", order+1, name))
 	}
 	sort.Strings(formattedRules) // the rule index begins the string, like "2)"
-	return strings.Join(formattedRules, newLine)
+	return strings.Join(formattedRules, NewLine)
 }
 
 func (rules ImplyingRulesType) String() string {
+	if rules.Ingress.onlySystemDefaultRule() && rules.Egress.onlySystemDefaultRule() {
+		return " " + ExplSystemDefault + NewLine
+	}
 	res := ""
 	if !rules.Ingress.Empty() {
-		res += fmt.Sprintf("INGRESS DIRECTION:\n%s\n", rules.Ingress.String())
+		res += IngressDirectionTitle
+		if rules.Ingress.onlySystemDefaultRule() {
+			res += " " + ExplSystemDefault + NewLine
+		} else {
+			res += NewLine + rules.Ingress.String() + NewLine
+		}
 	}
 	if !rules.Egress.Empty() {
-		res += fmt.Sprintf("EGRESS DIRECTION:\n%s\n", rules.Egress.String())
+		res += EgressDirectionTitle
+		if rules.Egress.onlySystemDefaultRule() {
+			res += " " + ExplSystemDefault + NewLine
+		} else {
+			res += NewLine + rules.Egress.String() + NewLine
+		}
 	}
 	if res == "" {
-		return ""
+		return NewLine
 	}
-	return " " + explTitle + newLine + res
+	return " " + ExplWithRulesTitle + NewLine + res
 }
 
 func (rules *ImplyingXgressRulesType) Empty() bool {

pkg/netpol/internal/common/connectionset.go

@@ -40,6 +40,11 @@ func MakeConnectionSet(all bool) *ConnectionSet {
 	return &ConnectionSet{AllowedProtocols: map[v1.Protocol]*PortSet{}, CommonImplyingRules: MakeImplyingRules()}
 }
 
+func MakeAllConnectionSetWithRule(rule string, isIngress bool) *ConnectionSet {
+	return &ConnectionSet{AllowAll: true, AllowedProtocols: map[v1.Protocol]*PortSet{},
+		CommonImplyingRules: MakeImplyingRulesWithRule(rule, isIngress)}
+}
+
 // Add common implying rule, i.e., a rule that is relevant for the whole ConnectionSet
 func (conn *ConnectionSet) AddCommonImplyingRule(implyingRule string, isIngress bool) {
 	conn.CommonImplyingRules.AddRule(implyingRule, isIngress)
@@ -158,9 +163,10 @@ func (conn *ConnectionSet) rebuildAllowAllExplicitly() {
 
 // Union updates ConnectionSet object to be the union result with other ConnectionSet
 func (conn *ConnectionSet) Union(other *ConnectionSet) {
-	if conn.IsEmpty() && other.IsEmpty() && len(conn.AllowedProtocols) == 0 && len(other.AllowedProtocols) == 0 {
+	if conn.IsEmpty() && (other.IsEmpty() || other.AllowAll) && len(conn.AllowedProtocols) == 0 && len(other.AllowedProtocols) == 0 {
 		// a special case when we should union implying rules
 		conn.CommonImplyingRules.Union(other.CommonImplyingRules)
+		conn.AllowAll = other.AllowAll
 		return
 	}
 	if conn.AllowAll || other.IsEmpty() {
@@ -189,6 +195,13 @@ func (conn *ConnectionSet) Subtract(other *ConnectionSet) {
 	if other.IsEmpty() { // nothing to subtract
 		return
 	}
+	if other.AllowAll && len(other.AllowedProtocols) == 0 {
+		// a special case when we should replace current common implying rules by others'
+		conn.CommonImplyingRules = other.CommonImplyingRules.Copy()
+		conn.AllowAll = false
+		conn.AllowedProtocols = map[v1.Protocol]*PortSet{}
+		return
+	}
 	if conn.AllowAll {
 		conn.rebuildAllowAllExplicitly()
 		conn.AllowAll = false
@@ -451,7 +464,7 @@ func portsStringWithExplanation(ports []PortRange, protocolString string) (strin
 	if len(portsStr) == 0 {
 		return noConnsStr + noPortsExplanation.String(), false
 	}
-	return strings.Join(portsStr, newLine), true
+	return strings.Join(portsStr, NewLine), true
 }
 
 func protocolAndPortsStr(protocol v1.Protocol, ports string) string {
@@ -460,7 +473,7 @@ func protocolAndPortsStr(protocol v1.Protocol, ports string) string {
 
 func ExplanationFromConnProperties(allProtocolsAndPorts bool, commonImplyingRules ImplyingRulesType,
 	protocolsAndPorts map[v1.Protocol][]PortRange) string {
-	if allProtocolsAndPorts || len(protocolsAndPorts) == 0 {
+	if len(protocolsAndPorts) == 0 {
 		connStr := noConnsStr
 		if allProtocolsAndPorts {
 			connStr = allConnsStr
@@ -477,6 +490,6 @@ func ExplanationFromConnProperties(allProtocolsAndPorts bool, commonImplyingRule
 		}
 	}
 	sort.Strings(connStrings)
-	connStr = strings.Join(connStrings, connsAndPortRangeSeparator)
+	connStr = strings.Join(connStrings, NewLine)
 	return connStr
 }