some organizations (mainly comments updates)

pkg/netpol/eval/check.go

@@ -154,11 +154,7 @@ func (pe *PolicyEngine) allAllowedConnectionsBetweenPeers(srcPeer, dstPeer Peer)
 		return common.MakeConnectionSet(true), nil
 	}
 
-	// default connection: (@todo:when supporting BANP, default will be extracted from it)
-	defaultAllowedConns := common.MakeConnectionSet(true) // default is allowAll conns ,  @todo: type will be changed to *PolicyConnections
-
-	// first get conns from AdminNetworkPolicies:
-	// unless one peer is IP, skip, since ANPs are a cluster level resources
+	// first get conns between src and dst from AdminNetworkPolicies, unless one peer is IP, skip, since ANPs are a cluster level resources
 	anpCaptured := false
 	var anpConns *k8s.PolicyConnections
 	if dstK8sPeer.PeerType() != k8s.IPBlockType && srcK8sPeer.PeerType() != k8s.IPBlockType {
@@ -168,12 +164,15 @@ func (pe *PolicyEngine) allAllowedConnectionsBetweenPeers(srcPeer, dstPeer Peer)
 		}
 	}
 
-	// get conns from networkPolicies:
+	// get conns between src and dst from networkPolicies:
 	npAllowedConns, npCaptured, err := pe.getAllAllowedConnsFromNetpols(srcK8sPeer, dstK8sPeer)
 	if err != nil {
 		return nil, err
 	}
 
+	// get default connection between src and dst: (@todo:when supporting BANP, default will be extracted from it)
+	defaultAllowedConns := common.MakeConnectionSet(true) // default is allowAll conns ,  @todo: type will be changed to *PolicyConnections
+
 	// compute the result considering all captured conns
 	if !anpCaptured && !npCaptured {
 		// if no ANPs nor NPs capturing the peers, return the default allowed conns
@@ -533,7 +532,8 @@ func (pe *PolicyEngine) getAllConnsFromAdminNetpols(src, dst k8s.Peer) (anpsConn
 			policiesConns.CollectANPConns(policyConnsPerDirection)
 		}
 		// if the anp captures the dst, get the relevant ingress conns (from src to dst)
-		if dstAdminNetpols[anp] { // @todo should replace with else if (rules in a single policy should be matching for same src, dst?)
+		if dstAdminNetpols[anp] { // @todo should replace with else if (ingress and egress rules in a single
+			// policy should be matching for same src, dst?)
 			policyConnsPerDirection, err := anp.GetIngressPolicyConns(src, dst)
 			if err != nil {
 				return nil, false, err
@@ -582,10 +582,12 @@ func getUniqueAndSortedANPsList(ingressAnps, egressAnps map[*k8s.AdminNetworkPol
 func sortAdminNetpolsByPriority(anpList []*k8s.AdminNetworkPolicy) ([]*k8s.AdminNetworkPolicy, error) {
 	var err error
 	sort.Slice(anpList, func(i, j int) bool {
+		// outcome is non-deterministic if there are two AdminNetworkPolicies at the same priority
 		if anpList[i].Spec.Priority == anpList[j].Spec.Priority {
 			err = errors.New(netpolerrors.SamePriorityErr(anpList[i].Name, anpList[j].Name))
 			return false
 		}
+		// priority values range is defined
 		if !anpList[i].HasValidPriority() {
 			err = errors.New(netpolerrors.PriorityValueErr(anpList[i].Name, anpList[i].Spec.Priority))
 			return false

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

@@ -26,8 +26,9 @@ var ruleErrTitle = fmt.Sprintf("Error in rule:  %%v")
 
 // Selects returns true if the admin network policy's Spec.Subject selects the peer and if the required direction is in the policy spec
 func (anp *AdminNetworkPolicy) Selects(p Peer, isIngress bool) (bool, error) {
-	if p.PeerType() == IPBlockType {
-		// adminNetworkPolicy selects peers with their namespaceSelectors and podSelectors only, so it might not select IPs
+	if p.PeerType() == IPBlockType { // should not get here
+		// adminNetworkPolicy is a cluster level resource which selects peers with their namespaceSelectors and podSelectors only,
+		// so it might not select IPs
 		return false, nil
 	}
 	if !anp.adminPolicyAffectsDirection(isIngress) {
@@ -68,7 +69,7 @@ func onlyOnePeersFieldIsSet(namespacesField *metav1.LabelSelector, podsField *ap
 
 // doesNamespacesFieldMatchPeer returns if the given namespaces LabelSelector matches the given peer
 func (anp *AdminNetworkPolicy) doesNamespacesFieldMatchPeer(namespaces *metav1.LabelSelector, peer Peer) (bool, error) {
-	if peer.PeerType() == IPBlockType {
+	if peer.PeerType() == IPBlockType { // should not get here
 		return false, nil // namespaces does not select IPs
 	}
 	namespacesSelector, err := anp.parseAdminNetpolLabelSelector(namespaces)
@@ -95,7 +96,7 @@ func (anp *AdminNetworkPolicy) doesPodsFieldMatchPeer(pods *apisv1a.NamespacedPo
 	return nsSelector.Matches(labels.Set(peer.GetPeerNamespace().Labels)) && podSelector.Matches(labels.Set(peer.GetPeerPod().Labels)), nil
 }
 
-// parseAdminNetpolLabelSelector returns a selector of type labels.selector from a LabelSelector from the policy.
+// parseAdminNetpolLabelSelector returns a selector of type labels.selector from a LabelSelector in the policy.
 // an error with the admin network policy details returned if fails to convert the selector.
 func (anp *AdminNetworkPolicy) parseAdminNetpolLabelSelector(selector *metav1.LabelSelector) (labels.Selector, error) {
 	selectorRes, err := metav1.LabelSelectorAsSelector(selector)
@@ -256,7 +257,7 @@ func (anp *AdminNetworkPolicy) ruleConnections(ports *[]apisv1a.AdminNetworkPoli
 			portSet.AddPort(intstr.FromInt32(anpPort.PortNumber.Port))
 		case anpPort.NamedPort != nil:
 			podProtocol, podPort := dst.GetPeerPod().ConvertPodNamedPort(*anpPort.NamedPort)
-			if podPort == common.NoPort {
+			if podPort == common.NoPort { // pod does not have this named port in its container
 				continue // or an error should be returned?
 			}
 			if podProtocol != "" {

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

@@ -69,7 +69,7 @@ func (pc *PolicyConnections) UpdateWithRuleConns(ruleConns *common.ConnectionSet
 	return nil
 }
 
-// UpdateWithNetpolsConns updates current policy connections object with connections from a
+// UpdateWithOtherLayerConns updates current policy connections object with connections from a
 // layer with lower precedence (e.g. netpols conns/default conns)
 // ANP allowed and denied conns takes precedence on network-policy conns
 // Pass conns from ANP are determined by the NPs conns