npc: Add policyType to selectorSpec

This allows us to distinguish between ingress and egress
dst selectors (required when managing entries in default-allow
ipsets).

npc/analyser.go

@@ -24,7 +24,8 @@ func (ns *ns) analysePolicyLegacy(policy *extnapi.NetworkPolicy) (
 	podSelectors = make(map[string]*selectorSpec)
 	rules = make(map[string]*ruleSpec)
 
-	dstSelector, err := newSelectorSpec(&policy.Spec.PodSelector, true, ns.name, ipset.HashIP)
+	policyTypes := []policyType{ingressPolicy}
+	dstSelector, err := newSelectorSpec(&policy.Spec.PodSelector, true, policyTypes, ns.name, ipset.HashIP)
 	if err != nil {
 		return nil, nil, nil, err
 	}
@@ -61,14 +62,14 @@ func (ns *ns) analysePolicyLegacy(policy *extnapi.NetworkPolicy) (
 			for _, peer := range ingressRule.From {
 				var srcSelector *selectorSpec
 				if peer.PodSelector != nil {
-					srcSelector, err = newSelectorSpec(peer.PodSelector, false, ns.name, ipset.HashIP)
+					srcSelector, err = newSelectorSpec(peer.PodSelector, false, nil, ns.name, ipset.HashIP)
 					if err != nil {
 						return nil, nil, nil, err
 					}
 					podSelectors[srcSelector.key] = srcSelector
 				}
 				if peer.NamespaceSelector != nil {
-					srcSelector, err = newSelectorSpec(peer.NamespaceSelector, false, "", ipset.ListSet)
+					srcSelector, err = newSelectorSpec(peer.NamespaceSelector, false, nil, "", ipset.ListSet)
 					if err != nil {
 						return nil, nil, nil, err
 					}
@@ -104,7 +105,8 @@ func (ns *ns) analysePolicy(policy *networkingv1.NetworkPolicy) (
 	rules = make(map[string]*ruleSpec)
 
 	// If empty, matches all pods in a namespace
-	dstSelector, err := newSelectorSpec(&policy.Spec.PodSelector, true, ns.name, ipset.HashIP)
+	policyTypes := []policyType{ingressPolicy}
+	dstSelector, err := newSelectorSpec(&policy.Spec.PodSelector, true, policyTypes, ns.name, ipset.HashIP)
 	if err != nil {
 		return nil, nil, nil, err
 	}
@@ -143,14 +145,14 @@ func (ns *ns) analysePolicy(policy *networkingv1.NetworkPolicy) (
 				// NetworkPolicyPeer describes a peer to allow traffic from.
 				// Exactly one of its fields must be specified.
 				if peer.PodSelector != nil {
-					srcSelector, err = newSelectorSpec(peer.PodSelector, false, ns.name, ipset.HashIP)
+					srcSelector, err = newSelectorSpec(peer.PodSelector, false, nil, ns.name, ipset.HashIP)
 					if err != nil {
 						return nil, nil, nil, err
 					}
 					addIfNotExist(srcSelector, podSelectors)
 
 				} else if peer.NamespaceSelector != nil {
-					srcSelector, err = newSelectorSpec(peer.NamespaceSelector, false, "", ipset.ListSet)
+					srcSelector, err = newSelectorSpec(peer.NamespaceSelector, false, nil, "", ipset.ListSet)
 					if err != nil {
 						return nil, nil, nil, err
 					}

npc/controller.go

@@ -49,7 +49,7 @@ func New(nodeName string, legacy bool, ipt iptables.Interface, ips ipset.Interfa
 		ips:      ips,
 		nss:      make(map[string]*ns)}
 
-	doNothing := func(*selector) error { return nil }
+	doNothing := func(*selector, policyType) error { return nil }
 	c.nsSelectors = newSelectorSet(ips, c.onNewNsSelector, doNothing, doNothing)
 
 	return c

npc/controller_test.go

@@ -236,6 +236,7 @@ func TestDefaultAllow(t *testing.T) {
 		Status: coreapi.PodStatus{PodIP: barPodIP}}
 	podBarNoIP := &coreapi.Pod{ObjectMeta: podBar.ObjectMeta}
 	controller.AddPod(podBarNoIP)
+
 	controller.UpdatePod(podBarNoIP, podBar)
 
 	// Should add the bar pod to default-allow

npc/namespace.go

@@ -45,7 +45,7 @@ type ns struct {
 }
 
 func newNS(name, nodeName string, legacy bool, ipt iptables.Interface, ips ipset.Interface, nsSelectors *selectorSet) (*ns, error) {
-	allPods, err := newSelectorSpec(&metav1.LabelSelector{}, false, name, ipset.HashIP)
+	allPods, err := newSelectorSpec(&metav1.LabelSelector{}, false, nil, name, ipset.HashIP)
 	if err != nil {
 		return nil, err
 	}
@@ -102,7 +102,7 @@ func (ns *ns) onNewPodSelector(selector *selector) error {
 	return nil
 }
 
-func (ns *ns) onNewDstPodSelector(selector *selector) error {
+func (ns *ns) onNewDstPodSelector(selector *selector, policyType policyType) error {
 	if ns.legacy {
 		return nil
 	}
@@ -110,8 +110,12 @@ func (ns *ns) onNewDstPodSelector(selector *selector) error {
 	for _, pod := range ns.pods {
 		if hasIP(pod) {
 			// Remove the pod from default-allow if dst podselector matches the pod
+			ipset := ns.ingressDefaultAllowIPSet
+			if policyType == egressPolicy {
+				// TODO(brb) ipset = ns.egressDefaultAllowIPSet
+			}
 			if selector.matches(pod.ObjectMeta.Labels) {
-				if err := ns.ips.DelEntry(pod.ObjectMeta.UID, ns.ingressDefaultAllowIPSet, pod.Status.PodIP); err != nil {
+				if err := ns.ips.DelEntry(pod.ObjectMeta.UID, ipset, pod.Status.PodIP); err != nil {
 					return err
 				}
 			}
@@ -121,7 +125,7 @@ func (ns *ns) onNewDstPodSelector(selector *selector) error {
 	return nil
 }
 
-func (ns *ns) onDestroyDstPodSelector(selector *selector) error {
+func (ns *ns) onDestroyDstPodSelector(selector *selector, policyType policyType) error {
 	if ns.legacy {
 		return nil
 	}
@@ -129,7 +133,7 @@ func (ns *ns) onDestroyDstPodSelector(selector *selector) error {
 	for _, pod := range ns.pods {
 		if hasIP(pod) {
 			if selector.matches(pod.ObjectMeta.Labels) {
-				if err := ns.addToDefaultAllowIfNoMatching(pod); err != nil {
+				if err := ns.addToDefaultAllowIfNoMatching(pod, policyType); err != nil {
 					return err
 				}
 			}
@@ -140,17 +144,21 @@ func (ns *ns) onDestroyDstPodSelector(selector *selector) error {
 }
 
 // Add pod IP addr to default-allow ipset if there are no matching dst selectors
-func (ns *ns) addToDefaultAllowIfNoMatching(pod *coreapi.Pod) error {
+func (ns *ns) addToDefaultAllowIfNoMatching(pod *coreapi.Pod, policyType policyType) error {
 	found := false
 	// TODO(mp) optimize (avoid iterating over selectors) by ref counting IP addrs.
 	for _, s := range ns.podSelectors.entries {
-		if ns.podSelectors.dstSelectorExist(s) && s.matches(pod.ObjectMeta.Labels) {
+		if ns.podSelectors.dstSelectorExist(s, policyType) && s.matches(pod.ObjectMeta.Labels) {
 			found = true
 			break
 		}
 	}
 	if !found {
-		if err := ns.ips.AddEntry(pod.ObjectMeta.UID, ns.ingressDefaultAllowIPSet, pod.Status.PodIP, podComment(pod)); err != nil {
+		ipset := ns.ingressDefaultAllowIPSet
+		if policyType == egressPolicy {
+			// TODO(brb) ipset = ns.egressDefaultAllowIPSet
+		}
+		if err := ns.ips.AddEntry(pod.ObjectMeta.UID, ipset, pod.Status.PodIP, podComment(pod)); err != nil {
 			return err
 		}
 	}
@@ -175,12 +183,12 @@ func (ns *ns) addPod(obj *coreapi.Pod) error {
 		ns.ips.AddEntry(obj.ObjectMeta.UID, LocalIpset, obj.Status.PodIP, podComment(obj))
 	}
 
-	found, err := ns.podSelectors.addToMatching(obj.ObjectMeta.UID, obj.ObjectMeta.Labels, obj.Status.PodIP, podComment(obj))
+	foundIngress, err := ns.podSelectors.addToMatching(obj.ObjectMeta.UID, obj.ObjectMeta.Labels, obj.Status.PodIP, podComment(obj))
 	if err != nil {
 		return err
 	}
 	// If there are no matching dst selectors, add the pod to default-allow
-	if !ns.legacy && !found {
+	if !ns.legacy && !foundIngress {
 		if err := ns.ips.AddEntry(obj.ObjectMeta.UID, ns.ingressDefaultAllowIPSet, obj.Status.PodIP, podComment(obj)); err != nil {
 			return err
 		}
@@ -215,12 +223,12 @@ func (ns *ns) updatePod(oldObj, newObj *coreapi.Pod) error {
 		if ns.checkLocalPod(newObj) {
 			ns.ips.AddEntry(newObj.ObjectMeta.UID, LocalIpset, newObj.Status.PodIP, podComment(newObj))
 		}
-		found, err := ns.podSelectors.addToMatching(newObj.ObjectMeta.UID, newObj.ObjectMeta.Labels, newObj.Status.PodIP, podComment(newObj))
+		foundIngress, err := ns.podSelectors.addToMatching(newObj.ObjectMeta.UID, newObj.ObjectMeta.Labels, newObj.Status.PodIP, podComment(newObj))
 		if err != nil {
 			return err
 		}
 
-		if !ns.legacy && !found {
+		if !ns.legacy && !foundIngress {
 			if err := ns.ips.AddEntry(newObj.ObjectMeta.UID, ns.ingressDefaultAllowIPSet, newObj.Status.PodIP, podComment(newObj)); err != nil {
 				return err
 			}
@@ -264,14 +272,14 @@ func (ns *ns) updatePod(oldObj, newObj *coreapi.Pod) error {
 				}
 			}
 
-			if !ns.legacy && ns.podSelectors.dstSelectorExist(ps) {
+			if !ns.legacy && ns.podSelectors.dstSelectorExist(ps, ingressPolicy) {
 				switch {
 				case !oldMatch && newMatch:
 					if err := ns.ips.DelEntry(oldObj.ObjectMeta.UID, ns.ingressDefaultAllowIPSet, oldObj.Status.PodIP); err != nil {
 						return err
 					}
 				case oldMatch && !newMatch:
-					if err := ns.addToDefaultAllowIfNoMatching(newObj); err != nil {
+					if err := ns.addToDefaultAllowIfNoMatching(newObj, ingressPolicy); err != nil {
 						return err
 					}
 				}

npc/selector.go

@@ -10,25 +10,27 @@ import (
 )
 
 type selectorSpec struct {
-	key      string          // string representation (for hash keying/equality comparison)
-	selector labels.Selector // k8s Selector object (for matching)
-	dst      bool            // destination selector
+	key         string          // string representation (for hash keying/equality comparison)
+	selector    labels.Selector // k8s Selector object (for matching)
+	dst         bool            // destination selector TODO(brb) not relevant
+	policyTypes []policyType    // TODO(brb) repharse: netpol types (egress, ingress or both) which initiated the dst selector
 
 	ipsetType ipset.Type // type of ipset to provision
 	ipsetName ipset.Name // generated ipset name
 	nsName    string     // Namespace name
 }
 
-func newSelectorSpec(json *metav1.LabelSelector, dst bool, nsName string, ipsetType ipset.Type) (*selectorSpec, error) {
+func newSelectorSpec(json *metav1.LabelSelector, dst bool, policyType []policyType, nsName string, ipsetType ipset.Type) (*selectorSpec, error) {
 	selector, err := metav1.LabelSelectorAsSelector(json)
 	if err != nil {
 		return nil, err
 	}
 	key := selector.String()
 	return &selectorSpec{
-		key:      key,
-		selector: selector,
-		dst:      dst,
+		key:         key,
+		selector:    selector,
+		dst:         dst,
+		policyTypes: policyType,
 		// We prefix the selector string with the namespace name when generating
 		// the shortname because you can specify the same selector in multiple
 		// namespaces - we need those to map to distinct ipsets
@@ -55,26 +57,28 @@ func (s *selector) delEntry(user types.UID, entry string) error {
 }
 
 type selectorFn func(selector *selector) error
+type selectorWithPolicyTypeFn func(selector *selector, policyType policyType) error
 
 type selectorSet struct {
 	ips           ipset.Interface
 	onNewSelector selectorFn
 
 	// invoked after dst selector has been provisioned for the first time
-	onNewDstSelector selectorFn
+	onNewDstSelector selectorWithPolicyTypeFn
 	// invoked after the last instance of dst selector has been deprovisioned
-	onDestroyDstSelector selectorFn
+	onDestroyDstSelector selectorWithPolicyTypeFn
 
 	users   map[string]map[types.UID]struct{} // list of users per selector
 	entries map[string]*selector
 
 	// We need to keep track of dst selector instances to be able to invoke
 	// onNewDstSelector and onDestroyDstSelector callbacks at the right time;
-	// selectorSpec.Key -> count
-	dstSelectorsCount map[string]int
+	// selectorSpec.Key -> policyType -> count
+	// TODO(brb) dst -> target?
+	dstSelectorsCount map[string]map[policyType]int
 }
 
-func newSelectorSet(ips ipset.Interface, onNewSelector, onNewDstSelector selectorFn, onDestroyDstSelector selectorFn) *selectorSet {
+func newSelectorSet(ips ipset.Interface, onNewSelector selectorFn, onNewDstSelector, onDestroyDstSelector selectorWithPolicyTypeFn) *selectorSet {
 	return &selectorSet{
 		ips:                  ips,
 		onNewSelector:        onNewSelector,
@@ -82,22 +86,23 @@ func newSelectorSet(ips ipset.Interface, onNewSelector, onNewDstSelector selecto
 		onDestroyDstSelector: onDestroyDstSelector,
 		users:                make(map[string]map[types.UID]struct{}),
 		entries:              make(map[string]*selector),
-		dstSelectorsCount:    make(map[string]int)}
+		dstSelectorsCount:    make(map[string]map[policyType]int)}
 }
 
 func (ss *selectorSet) addToMatching(user types.UID, labelMap map[string]string, entry string, comment string) (bool, error) {
-	found := false
+	foundIngress := false
+	// TODO(brb) foundEgress
 	for _, s := range ss.entries {
 		if s.matches(labelMap) {
-			if ss.dstSelectorExist(s) {
-				found = true
+			if ss.dstSelectorExist(s, ingressPolicy) {
+				foundIngress = true
 			}
 			if err := s.addEntry(user, entry, comment); err != nil {
-				return found, err
+				return foundIngress, err
 			}
 		}
 	}
-	return found, nil
+	return foundIngress, nil
 }
 
 func (ss *selectorSet) delFromMatching(user types.UID, labelMap map[string]string, entry string) error {
@@ -111,8 +116,8 @@ func (ss *selectorSet) delFromMatching(user types.UID, labelMap map[string]strin
 	return nil
 }
 
-func (ss *selectorSet) dstSelectorExist(s *selector) bool {
-	return ss.dstSelectorsCount[s.spec.key] > 0
+func (ss *selectorSet) dstSelectorExist(s *selector, policyType policyType) bool {
+	return ss.dstSelectorsCount[s.spec.key][policyType] > 0
 }
 
 func (ss *selectorSet) deprovision(user types.UID, current, desired map[string]*selectorSpec) error {
@@ -129,13 +134,14 @@ func (ss *selectorSet) deprovision(user types.UID, current, desired map[string]*
 				delete(ss.users, key)
 			}
 
-			if spec.dst {
-				ss.dstSelectorsCount[key]--
-				if ss.dstSelectorsCount[key] == 0 {
-					if err := ss.onDestroyDstSelector(&selector{ss.ips, spec}); err != nil {
+			for _, policyType := range spec.policyTypes {
+				ss.dstSelectorsCount[key][policyType]--
+				if ss.dstSelectorsCount[key][policyType] == 0 {
+					if err := ss.onDestroyDstSelector(&selector{ss.ips, spec}, policyType); err != nil {
 						return err
 					}
 				}
+				// TODO(brb) delete(...)
 			}
 		}
 	}
@@ -160,10 +166,13 @@ func (ss *selectorSet) provision(user types.UID, current, desired map[string]*se
 			}
 			ss.users[key][user] = struct{}{}
 
-			if spec.dst {
-				ss.dstSelectorsCount[key]++
-				if ss.dstSelectorsCount[key] == 1 {
-					if err := ss.onNewDstSelector(selector); err != nil {
+			for _, pt := range spec.policyTypes {
+				if _, found := ss.dstSelectorsCount[key]; !found {
+					ss.dstSelectorsCount[key] = make(map[policyType]int)
+				}
+				ss.dstSelectorsCount[key][pt]++
+				if ss.dstSelectorsCount[key][pt] == 1 {
+					if err := ss.onNewDstSelector(selector, pt); err != nil {
 						return err
 					}
 				}

npc/types.go

@@ -0,0 +1,15 @@
+package npc
+
+type policyType byte
+
+const (
+	ingressPolicy policyType = iota
+	egressPolicy
+)
+
+func policyTypeStr(policyType policyType) string {
+	if policyType == egressPolicy {
+		return "egress"
+	}
+	return "ingress"
+}