add support for combination of podSelector and namespaceSelector

in network policies

- enhance selectorSpec to accomodate both pod and namespace selectors
- enhance analysePolicy to handle policies with both selectors

Fixes #3312

npc/analyser.go

@@ -31,7 +31,7 @@ func (ns *ns) analysePolicy(policy *networkingv1.NetworkPolicy) (
 		}
 	}
 	// If empty, matches all pods in a namespace
-	targetSelector, err := newSelectorSpec(&policy.Spec.PodSelector, policyTypes, ns.name, ipset.HashIP)
+	targetSelector, err := newSelectorSpec(&policy.Spec.PodSelector, nil, policyTypes, ns.name, ipset.HashIP)
 	if err != nil {
 		return nil, nil, nil, nil, err
 	}
@@ -72,16 +72,22 @@ func (ns *ns) analysePolicy(policy *networkingv1.NetworkPolicy) (
 					var srcRuleHost ruleHost
 
 					// 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, nil, ns.name, ipset.HashIP)
+					if peer.PodSelector != nil && peer.NamespaceSelector != nil {
+						srcSelector, err = newSelectorSpec(peer.PodSelector, peer.NamespaceSelector, nil, ns.name, ipset.HashIP)
+						if err != nil {
+							return nil, nil, nil, nil, err
+						}
+						addIfNotExist(srcSelector, podSelectors)
+						srcRuleHost = srcSelector
+					} else if peer.PodSelector != nil {
+						srcSelector, err = newSelectorSpec(peer.PodSelector, nil, nil, ns.name, ipset.HashIP)
 						if err != nil {
 							return nil, nil, nil, nil, err
 						}
 						addIfNotExist(srcSelector, podSelectors)
 						srcRuleHost = srcSelector
 					} else if peer.NamespaceSelector != nil {
-						srcSelector, err = newSelectorSpec(peer.NamespaceSelector, nil, "", ipset.ListSet)
+						srcSelector, err = newSelectorSpec(nil, peer.NamespaceSelector, nil, "", ipset.ListSet)
 						if err != nil {
 							return nil, nil, nil, nil, err
 						}
@@ -137,9 +143,15 @@ func (ns *ns) analysePolicy(policy *networkingv1.NetworkPolicy) (
 					var dstRuleHost ruleHost
 
 					// NetworkPolicyPeer describes a peer to allow traffic to.
-					// Exactly one of its fields must be specified.
-					if peer.PodSelector != nil {
-						dstSelector, err = newSelectorSpec(peer.PodSelector, nil, ns.name, ipset.HashIP)
+					if peer.PodSelector != nil && peer.NamespaceSelector != nil {
+						dstSelector, err = newSelectorSpec(peer.PodSelector, peer.NamespaceSelector, nil, ns.name, ipset.HashIP)
+						if err != nil {
+							return nil, nil, nil, nil, err
+						}
+						addIfNotExist(dstSelector, podSelectors)
+						dstRuleHost = dstSelector
+					} else if peer.PodSelector != nil {
+						dstSelector, err = newSelectorSpec(peer.PodSelector, nil, nil, ns.name, ipset.HashIP)
 						if err != nil {
 							return nil, nil, nil, nil, err
 						}
@@ -147,7 +159,7 @@ func (ns *ns) analysePolicy(policy *networkingv1.NetworkPolicy) (
 						dstRuleHost = dstSelector
 
 					} else if peer.NamespaceSelector != nil {
-						dstSelector, err = newSelectorSpec(peer.NamespaceSelector, nil, "", ipset.ListSet)
+						dstSelector, err = newSelectorSpec(peer.NamespaceSelector, nil, nil, "", ipset.ListSet)
 						if err != nil {
 							return nil, nil, nil, nil, err
 						}

npc/controller.go

@@ -57,7 +57,7 @@ func New(nodeName string, ipt iptables.Interface, ips ipset.Interface) NetworkPo
 func (npc *controller) onNewNsSelector(selector *selector) error {
 	for _, ns := range npc.nss {
 		if ns.namespace != nil {
-			if selector.matches(ns.namespace.ObjectMeta.Labels) {
+			if selector.matches(nil, &ns.namespace.ObjectMeta.Labels) {
 				if err := selector.addEntry(ns.namespace.ObjectMeta.UID, string(ns.allPods.ipsetName), namespaceComment(ns)); err != nil {
 					return err
 				}

npc/namespace.go

@@ -44,7 +44,7 @@ type ns struct {
 }
 
 func newNS(name, nodeName string, ipt iptables.Interface, ips ipset.Interface, nsSelectors *selectorSet) (*ns, error) {
-	allPods, err := newSelectorSpec(&metav1.LabelSelector{}, nil, name, ipset.HashIP)
+	allPods, err := newSelectorSpec(&metav1.LabelSelector{}, nil, nil, name, ipset.HashIP)
 	if err != nil {
 		return nil, err
 	}
@@ -95,7 +95,7 @@ func (ns *ns) destroy() error {
 func (ns *ns) onNewPodSelector(selector *selector) error {
 	for _, pod := range ns.pods {
 		if hasIP(pod) {
-			if selector.matches(pod.ObjectMeta.Labels) {
+			if selector.matches(&pod.ObjectMeta.Labels, nil) {
 				if err := selector.addEntry(pod.ObjectMeta.UID, pod.Status.PodIP, podComment(pod)); err != nil {
 					return err
 				}
@@ -111,7 +111,7 @@ func (ns *ns) onNewTargetPodSelector(selector *selector, policyType policyType)
 		if hasIP(pod) {
 			// Remove the pod from default-allow if dst podselector matches the pod
 			ipset := ns.defaultAllowIPSetName(policyType)
-			if selector.matches(pod.ObjectMeta.Labels) {
+			if selector.matches(&pod.ObjectMeta.Labels, nil) {
 				if err := ns.ips.DelEntry(pod.ObjectMeta.UID, ipset, pod.Status.PodIP); err != nil {
 					return err
 				}
@@ -125,7 +125,7 @@ func (ns *ns) onNewTargetPodSelector(selector *selector, policyType policyType)
 func (ns *ns) onDestroyTargetPodSelector(selector *selector, policyType policyType) error {
 	for _, pod := range ns.pods {
 		if hasIP(pod) {
-			if selector.matches(pod.ObjectMeta.Labels) {
+			if selector.matches(&pod.ObjectMeta.Labels, nil) {
 				if err := ns.addToDefaultAllowIfNoMatching(pod, policyType); err != nil {
 					return err
 				}
@@ -141,7 +141,7 @@ func (ns *ns) addToDefaultAllowIfNoMatching(pod *coreapi.Pod, policyType policyT
 	found := false
 	// TODO(mp) optimize (avoid iterating over selectors) by ref counting IP addrs.
 	for _, s := range ns.podSelectors.entries {
-		if ns.podSelectors.targetSelectorExist(s, policyType) && s.matches(pod.ObjectMeta.Labels) {
+		if ns.podSelectors.targetSelectorExist(s, policyType) && s.matches(&pod.ObjectMeta.Labels, nil) {
 			found = true
 			break
 		}
@@ -169,7 +169,7 @@ func (ns *ns) addPod(obj *coreapi.Pod) error {
 		return nil
 	}
 
-	foundIngress, foundEgress, err := ns.podSelectors.addToMatching(obj.ObjectMeta.UID, obj.ObjectMeta.Labels, obj.Status.PodIP, podComment(obj))
+	foundIngress, foundEgress, err := ns.podSelectors.addToMatching(obj.ObjectMeta.UID, &obj.ObjectMeta.Labels, nil, obj.Status.PodIP, podComment(obj))
 	if err != nil {
 		return err
 	}
@@ -204,11 +204,11 @@ func (ns *ns) updatePod(oldObj, newObj *coreapi.Pod) error {
 			return err
 		}
 
-		return ns.podSelectors.delFromMatching(oldObj.ObjectMeta.UID, oldObj.ObjectMeta.Labels, oldObj.Status.PodIP)
+		return ns.podSelectors.delFromMatching(oldObj.ObjectMeta.UID, &oldObj.ObjectMeta.Labels, nil, oldObj.Status.PodIP)
 	}
 
 	if !hasIP(oldObj) && hasIP(newObj) {
-		foundIngress, foundEgress, err := ns.podSelectors.addToMatching(newObj.ObjectMeta.UID, newObj.ObjectMeta.Labels, newObj.Status.PodIP, podComment(newObj))
+		foundIngress, foundEgress, err := ns.podSelectors.addToMatching(newObj.ObjectMeta.UID, &newObj.ObjectMeta.Labels, nil, newObj.Status.PodIP, podComment(newObj))
 		if err != nil {
 			return err
 		}
@@ -240,8 +240,8 @@ func (ns *ns) updatePod(oldObj, newObj *coreapi.Pod) error {
 		oldObj.Status.PodIP != newObj.Status.PodIP {
 
 		for _, ps := range ns.podSelectors.entries {
-			oldMatch := ps.matches(oldObj.ObjectMeta.Labels)
-			newMatch := ps.matches(newObj.ObjectMeta.Labels)
+			oldMatch := ps.matches(&oldObj.ObjectMeta.Labels, nil)
+			newMatch := ps.matches(&newObj.ObjectMeta.Labels, nil)
 			if oldMatch == newMatch && oldObj.Status.PodIP == newObj.Status.PodIP {
 				continue
 			}
@@ -299,7 +299,7 @@ func (ns *ns) deletePod(obj *coreapi.Pod) error {
 		return err
 	}
 
-	return ns.podSelectors.delFromMatching(obj.ObjectMeta.UID, obj.ObjectMeta.Labels, obj.Status.PodIP)
+	return ns.podSelectors.delFromMatching(obj.ObjectMeta.UID, &obj.ObjectMeta.Labels, nil, obj.Status.PodIP)
 }
 
 func (ns *ns) addNetworkPolicy(obj interface{}) error {
@@ -454,7 +454,7 @@ func (ns *ns) addNamespace(obj *coreapi.Namespace) error {
 	}
 
 	// Add namespace ipset to matching namespace selectors
-	_, _, err := ns.nsSelectors.addToMatching(obj.ObjectMeta.UID, obj.ObjectMeta.Labels, string(ns.allPods.ipsetName), namespaceComment(ns))
+	_, _, err := ns.nsSelectors.addToMatching(obj.ObjectMeta.UID, nil, &obj.ObjectMeta.Labels, string(ns.allPods.ipsetName), namespaceComment(ns))
 	return err
 }
 
@@ -464,8 +464,8 @@ func (ns *ns) updateNamespace(oldObj, newObj *coreapi.Namespace) error {
 	// Re-evaluate namespace selector membership if labels have changed
 	if !equals(oldObj.ObjectMeta.Labels, newObj.ObjectMeta.Labels) {
 		for _, selector := range ns.nsSelectors.entries {
-			oldMatch := selector.matches(oldObj.ObjectMeta.Labels)
-			newMatch := selector.matches(newObj.ObjectMeta.Labels)
+			oldMatch := selector.matches(nil, &oldObj.ObjectMeta.Labels)
+			newMatch := selector.matches(nil, &newObj.ObjectMeta.Labels)
 			if oldMatch == newMatch {
 				continue
 			}
@@ -494,7 +494,7 @@ func (ns *ns) deleteNamespace(obj *coreapi.Namespace) error {
 	}
 
 	// Remove namespace ipset from any matching namespace selectors
-	err := ns.nsSelectors.delFromMatching(obj.ObjectMeta.UID, obj.ObjectMeta.Labels, string(ns.allPods.ipsetName))
+	err := ns.nsSelectors.delFromMatching(obj.ObjectMeta.UID, nil, &obj.ObjectMeta.Labels, string(ns.allPods.ipsetName))
 	if err != nil {
 		return err
 	}

npc/selector.go

@@ -12,25 +12,39 @@ import (
 )
 
 type selectorSpec struct {
-	key         string          // string representation (for hash keying/equality comparison)
-	selector    labels.Selector // k8s Selector object (for matching)
-	policyTypes []policyType    // If non-empty, then selectorSpec is a target selector for given policyTypes.
+	key               string          // string representation (for hash keying/equality comparison)
+	podSelector       labels.Selector // k8s Selector object (for matching pods)
+	namespaceSelector labels.Selector // k8s Selector object (for matching namespaces)
+	policyTypes       []policyType    // If non-empty, then selectorSpec is a target selector for given policyTypes.
 
 	ipsetType ipset.Type // type of ipset to provision
 	ipsetName ipset.Name // generated ipset name
 	nsName    string     // Namespace name
 }
 
-func newSelectorSpec(json *metav1.LabelSelector, policyType []policyType, nsName string, ipsetType ipset.Type) (*selectorSpec, error) {
-	selector, err := metav1.LabelSelectorAsSelector(json)
-	if err != nil {
-		return nil, err
+func newSelectorSpec(podJSON *metav1.LabelSelector, namespaceJSON *metav1.LabelSelector, policyType []policyType, nsName string, ipsetType ipset.Type) (*selectorSpec, error) {
+	var podSelector, namespaceSelector labels.Selector
+	var err error
+	var key string
+	if namespaceJSON != nil {
+		namespaceSelector, err = metav1.LabelSelectorAsSelector(namespaceJSON)
+		if err != nil {
+			return nil, err
+		}
+		key = namespaceSelector.String()
+	}
+	if podJSON != nil {
+		podSelector, err = metav1.LabelSelectorAsSelector(podJSON)
+		if err != nil {
+			return nil, err
+		}
+		key = key + podSelector.String()
 	}
-	key := selector.String()
 	return &selectorSpec{
-		key:         key,
-		selector:    selector,
-		policyTypes: policyType,
+		key:               key,
+		podSelector:       podSelector,
+		namespaceSelector: namespaceSelector,
+		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
@@ -61,8 +75,15 @@ type selector struct {
 	spec *selectorSpec
 }
 
-func (s *selector) matches(labelMap map[string]string) bool {
-	return s.spec.selector.Matches(labels.Set(labelMap))
+func (s *selector) matches(podsLabelMap *map[string]string, namespaceLabelMap *map[string]string) bool {
+	if podsLabelMap != nil && namespaceLabelMap != nil {
+		return s.spec.podSelector.Matches(labels.Set(*podsLabelMap)) && s.spec.namespaceSelector.Matches(labels.Set(*namespaceLabelMap))
+	} else if podsLabelMap != nil {
+		return s.spec.podSelector.Matches(labels.Set(*podsLabelMap))
+	} else if namespaceLabelMap != nil {
+		return s.spec.namespaceSelector.Matches(labels.Set(*namespaceLabelMap))
+	}
+	return false
 }
 
 func (s *selector) addEntry(user types.UID, entry string, comment string) error {
@@ -105,11 +126,11 @@ func newSelectorSet(ips ipset.Interface, onNewSelector selectorFn, onNewTargetSe
 		targetSelectorsCount: make(map[string]map[policyType]int)}
 }
 
-func (ss *selectorSet) addToMatching(user types.UID, labelMap map[string]string, entry string, comment string) (bool, bool, error) {
+func (ss *selectorSet) addToMatching(user types.UID, podLabelsMap *map[string]string, namespaceLabelsMap *map[string]string, entry string, comment string) (bool, bool, error) {
 	foundIngress := false
 	foundEgress := false
 	for _, s := range ss.entries {
-		if s.matches(labelMap) {
+		if s.matches(podLabelsMap, namespaceLabelsMap) {
 			if ss.targetSelectorExist(s, policyTypeIngress) {
 				foundIngress = true
 			}
@@ -124,9 +145,9 @@ func (ss *selectorSet) addToMatching(user types.UID, labelMap map[string]string,
 	return foundIngress, foundEgress, nil
 }
 
-func (ss *selectorSet) delFromMatching(user types.UID, labelMap map[string]string, entry string) error {
+func (ss *selectorSet) delFromMatching(user types.UID, podLabelsMap *map[string]string, namespaceLabelsMap *map[string]string, entry string) error {
 	for _, s := range ss.entries {
-		if s.matches(labelMap) {
+		if s.matches(podLabelsMap, namespaceLabelsMap) {
 			if err := s.delEntry(user, entry); err != nil {
 				return err
 			}