From bb4f51499da14598d304f6275f529398ac34d3bc Mon Sep 17 00:00:00 2001 From: Murali Reddy Date: Tue, 28 May 2019 12:33:14 +0530 Subject: [PATCH 1/4] 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 | 30 ++++++++++++++++++-------- npc/controller.go | 2 +- npc/namespace.go | 30 +++++++++++++------------- npc/selector.go | 55 ++++++++++++++++++++++++++++++++--------------- 4 files changed, 75 insertions(+), 42 deletions(-) diff --git a/npc/analyser.go b/npc/analyser.go index b444f60e..9e70b33a 100644 --- a/npc/analyser.go +++ b/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 } diff --git a/npc/controller.go b/npc/controller.go index eef4af7f..7c625d8b 100644 --- a/npc/controller.go +++ b/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 } diff --git a/npc/namespace.go b/npc/namespace.go index 76a44993..1e51734d 100644 --- a/npc/namespace.go +++ b/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 } diff --git a/npc/selector.go b/npc/selector.go index 6280f26a..1a321b94 100644 --- a/npc/selector.go +++ b/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 } From 75e106b6670e7e18c8edd5cdf45db1f4891bed8f Mon Sep 17 00:00:00 2001 From: Murali Reddy Date: Thu, 30 May 2019 12:03:28 +0530 Subject: [PATCH 2/4] using an explicti selector set for network policies with both namespace and pod lable selectors --- npc/analyser.go | 34 +++++++------- npc/controller.go | 29 ++++++++++-- npc/namespace.go | 114 ++++++++++++++++++++++++++++++++++++---------- 3 files changed, 132 insertions(+), 45 deletions(-) diff --git a/npc/analyser.go b/npc/analyser.go index 9e70b33a..ccfce02a 100644 --- a/npc/analyser.go +++ b/npc/analyser.go @@ -12,12 +12,14 @@ import ( func (ns *ns) analysePolicy(policy *networkingv1.NetworkPolicy) ( rules map[string]*ruleSpec, - nsSelectors, podSelectors map[string]*selectorSpec, + nsSelectors, podSelectors, namespacedPodSelectors map[string]*selectorSpec, ipBlocks map[string]*ipBlockSpec, err error) { nsSelectors = make(map[string]*selectorSpec) podSelectors = make(map[string]*selectorSpec) + namespacedPodSelectors = make(map[string]*selectorSpec) + ipBlocks = make(map[string]*ipBlockSpec) rules = make(map[string]*ruleSpec) policyTypes := make([]policyType, 0) @@ -33,7 +35,7 @@ func (ns *ns) analysePolicy(policy *networkingv1.NetworkPolicy) ( // If empty, matches all pods in a namespace targetSelector, err := newSelectorSpec(&policy.Spec.PodSelector, nil, policyTypes, ns.name, ipset.HashIP) if err != nil { - return nil, nil, nil, nil, err + return nil, nil, nil, nil, nil, err } // To prevent targetSelector being overwritten by a subsequent selector with @@ -52,7 +54,7 @@ func (ns *ns) analysePolicy(policy *networkingv1.NetworkPolicy) ( if !allPorts { if err = checkForNamedPorts(ingressRule.Ports); err != nil { - return nil, nil, nil, nil, fmt.Errorf("named ports in network policies is not supported yet. "+ + return nil, nil, nil, nil, nil, fmt.Errorf("named ports in network policies is not supported yet. "+ "Rejecting network policy: %s from further processing. "+err.Error(), policy.Name) } } @@ -73,23 +75,23 @@ func (ns *ns) analysePolicy(policy *networkingv1.NetworkPolicy) ( // NetworkPolicyPeer describes a peer to allow traffic from. if peer.PodSelector != nil && peer.NamespaceSelector != nil { - srcSelector, err = newSelectorSpec(peer.PodSelector, peer.NamespaceSelector, nil, ns.name, ipset.HashIP) + srcSelector, err = newSelectorSpec(peer.PodSelector, peer.NamespaceSelector, nil, "", ipset.HashIP) if err != nil { - return nil, nil, nil, nil, err + return nil, nil, nil, nil, nil, err } - addIfNotExist(srcSelector, podSelectors) + addIfNotExist(srcSelector, namespacedPodSelectors) 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 + return nil, nil, nil, nil, nil, err } addIfNotExist(srcSelector, podSelectors) srcRuleHost = srcSelector } else if peer.NamespaceSelector != nil { srcSelector, err = newSelectorSpec(nil, peer.NamespaceSelector, nil, "", ipset.ListSet) if err != nil { - return nil, nil, nil, nil, err + return nil, nil, nil, nil, nil, err } nsSelectors[srcSelector.key] = srcSelector srcRuleHost = srcSelector @@ -123,7 +125,7 @@ func (ns *ns) analysePolicy(policy *networkingv1.NetworkPolicy) ( if !allPorts { if err = checkForNamedPorts(egressRule.Ports); err != nil { - return nil, nil, nil, nil, fmt.Errorf("named ports in network policies is not supported yet. "+ + return nil, nil, nil, nil, nil, fmt.Errorf("named ports in network policies is not supported yet. "+ "Rejecting network policy: %s from further processing. "+err.Error(), policy.Name) } } @@ -144,24 +146,24 @@ func (ns *ns) analysePolicy(policy *networkingv1.NetworkPolicy) ( // NetworkPolicyPeer describes a peer to allow traffic to. if peer.PodSelector != nil && peer.NamespaceSelector != nil { - dstSelector, err = newSelectorSpec(peer.PodSelector, peer.NamespaceSelector, nil, ns.name, ipset.HashIP) + dstSelector, err = newSelectorSpec(peer.PodSelector, peer.NamespaceSelector, nil, "", ipset.HashIP) if err != nil { - return nil, nil, nil, nil, err + return nil, nil, nil, nil, nil, err } - addIfNotExist(dstSelector, podSelectors) + addIfNotExist(dstSelector, namespacedPodSelectors) 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 + return nil, nil, nil, nil, nil, err } addIfNotExist(dstSelector, podSelectors) dstRuleHost = dstSelector } else if peer.NamespaceSelector != nil { - dstSelector, err = newSelectorSpec(peer.NamespaceSelector, nil, nil, "", ipset.ListSet) + dstSelector, err = newSelectorSpec(nil, peer.NamespaceSelector, nil, "", ipset.ListSet) if err != nil { - return nil, nil, nil, nil, err + return nil, nil, nil, nil, nil, err } nsSelectors[dstSelector.key] = dstSelector dstRuleHost = dstSelector @@ -185,7 +187,7 @@ func (ns *ns) analysePolicy(policy *networkingv1.NetworkPolicy) ( } } - return rules, nsSelectors, podSelectors, ipBlocks, nil + return rules, nsSelectors, podSelectors, namespacedPodSelectors, ipBlocks, nil } func addIfNotExist(s *selectorSpec, ss map[string]*selectorSpec) { diff --git a/npc/controller.go b/npc/controller.go index 7c625d8b..45c34541 100644 --- a/npc/controller.go +++ b/npc/controller.go @@ -36,9 +36,10 @@ type controller struct { ipt iptables.Interface ips ipset.Interface - nss map[string]*ns // ns name -> ns struct - nsSelectors *selectorSet // selector string -> nsSelector - defaultEgressDrop bool // flag to track if base iptable rule to drop egress traffic is added or not + nss map[string]*ns // ns name -> ns struct + nsSelectors *selectorSet // selector string -> nsSelector + namespacedPodSelectors *selectorSet + defaultEgressDrop bool // flag to track if base iptable rule to drop egress traffic is added or not } func New(nodeName string, ipt iptables.Interface, ips ipset.Interface) NetworkPolicyController { @@ -50,7 +51,7 @@ func New(nodeName string, ipt iptables.Interface, ips ipset.Interface) NetworkPo doNothing := func(*selector, policyType) error { return nil } c.nsSelectors = newSelectorSet(ips, c.onNewNsSelector, doNothing, doNothing) - + c.namespacedPodSelectors = newSelectorSet(ips, c.onNewNamespacePodsSelector, doNothing, doNothing) return c } @@ -67,10 +68,28 @@ func (npc *controller) onNewNsSelector(selector *selector) error { return nil } +func (npc *controller) onNewNamespacePodsSelector(selector *selector) error { + for _, ns := range npc.nss { + if ns.namespace != nil && len(ns.pods) > 0 { + for _, pod := range ns.pods { + if hasIP(pod) { + if selector.matches(&pod.ObjectMeta.Labels, &ns.namespace.ObjectMeta.Labels) { + if err := selector.addEntry(pod.ObjectMeta.UID, pod.Status.PodIP, podComment(pod)); err != nil { + return err + } + + } + } + } + } + } + return nil +} + func (npc *controller) withNS(name string, f func(ns *ns) error) error { ns, found := npc.nss[name] if !found { - newNs, err := newNS(name, npc.nodeName, npc.ipt, npc.ips, npc.nsSelectors) + newNs, err := newNS(name, npc.nodeName, npc.ipt, npc.ips, npc.nsSelectors, npc.namespacedPodSelectors) if err != nil { return err } diff --git a/npc/namespace.go b/npc/namespace.go index 1e51734d..6959fcd0 100644 --- a/npc/namespace.go +++ b/npc/namespace.go @@ -10,6 +10,8 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/uuid" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/rest" "github.com/weaveworks/weave/common" "github.com/weaveworks/weave/net/ipset" @@ -37,30 +39,48 @@ type ns struct { ingressDefaultAllowIPSet ipset.Name egressDefaultAllowIPSet ipset.Name - nsSelectors *selectorSet - podSelectors *selectorSet - ipBlocks *ipBlockSet - rules *ruleSet + nsSelectors *selectorSet // reference to global selectorSet that is shared across the `ns`. Used to represent all pods in the matching namespaces + podSelectors *selectorSet // used to represent the matching pods in namespace respresented by this `ns` + namespacedPodsSelectors *selectorSet // reference to global selectorSet that is shared across the `ns`. Used to represent matching pods in matching namespace + ipBlocks *ipBlockSet + rules *ruleSet } -func newNS(name, nodeName string, ipt iptables.Interface, ips ipset.Interface, nsSelectors *selectorSet) (*ns, error) { +func newNS(name, nodeName string, ipt iptables.Interface, ips ipset.Interface, nsSelectors *selectorSet, namespacedPodsSelectors *selectorSet) (*ns, error) { allPods, err := newSelectorSpec(&metav1.LabelSelector{}, nil, nil, name, ipset.HashIP) if err != nil { return nil, err } + config, err := rest.InClusterConfig() + if err != nil { + return nil, err + } + + client, err := kubernetes.NewForConfig(config) + if err != nil { + return nil, err + } + + namespace, err := client.CoreV1().Namespaces().Get(name, metav1.GetOptions{}) + if err != nil { + return nil, err + } + ns := &ns{ - ipt: ipt, - ips: ips, - name: name, - nodeName: nodeName, - pods: make(map[types.UID]*coreapi.Pod), - policies: make(map[types.UID]interface{}), - uid: uuid.NewUUID(), - allPods: allPods, - nsSelectors: nsSelectors, - ipBlocks: newIPBlockSet(ips), - rules: newRuleSet(ipt), + ipt: ipt, + ips: ips, + name: name, + namespace: namespace, + nodeName: nodeName, + pods: make(map[types.UID]*coreapi.Pod), + policies: make(map[types.UID]interface{}), + uid: uuid.NewUUID(), + allPods: allPods, + nsSelectors: nsSelectors, + namespacedPodsSelectors: namespacedPodsSelectors, + ipBlocks: newIPBlockSet(ips), + rules: newRuleSet(ipt), } ns.podSelectors = newSelectorSet(ips, ns.onNewPodSelector, ns.onNewTargetPodSelector, ns.onDestroyTargetPodSelector) @@ -185,6 +205,11 @@ func (ns *ns) addPod(obj *coreapi.Pod) error { } } + _, _, err = ns.namespacedPodsSelectors.addToMatching(obj.ObjectMeta.UID, &obj.ObjectMeta.Labels, &ns.namespace.ObjectMeta.Labels, obj.Status.PodIP, podComment(obj)) + if err != nil { + return err + } + return nil } @@ -203,6 +228,9 @@ func (ns *ns) updatePod(oldObj, newObj *coreapi.Pod) error { if err := ns.ips.DelEntry(oldObj.ObjectMeta.UID, ns.egressDefaultAllowIPSet, oldObj.Status.PodIP); err != nil { return err } + if err := ns.namespacedPodsSelectors.delFromMatching(oldObj.ObjectMeta.UID, &oldObj.ObjectMeta.Labels, &ns.namespace.ObjectMeta.Labels, oldObj.Status.PodIP); err != nil { + return err + } return ns.podSelectors.delFromMatching(oldObj.ObjectMeta.UID, &oldObj.ObjectMeta.Labels, nil, oldObj.Status.PodIP) } @@ -223,6 +251,10 @@ func (ns *ns) updatePod(oldObj, newObj *coreapi.Pod) error { return err } } + _, _, err = ns.namespacedPodsSelectors.addToMatching(newObj.ObjectMeta.UID, &newObj.ObjectMeta.Labels, &ns.namespace.ObjectMeta.Labels, newObj.Status.PodIP, podComment(newObj)) + if err != nil { + return err + } return nil } @@ -263,6 +295,23 @@ func (ns *ns) updatePod(oldObj, newObj *coreapi.Pod) error { return err } } + for _, ps := range ns.namespacedPodsSelectors.entries { + oldMatch := ps.matches(&oldObj.ObjectMeta.Labels, &ns.namespace.ObjectMeta.Labels) + newMatch := ps.matches(&newObj.ObjectMeta.Labels, &ns.namespace.ObjectMeta.Labels) + if oldMatch == newMatch && oldObj.Status.PodIP == newObj.Status.PodIP { + continue + } + if oldMatch { + if err := ps.delEntry(oldObj.ObjectMeta.UID, oldObj.Status.PodIP); err != nil { + return err + } + } + if newMatch { + if err := ps.addEntry(newObj.ObjectMeta.UID, newObj.Status.PodIP, podComment(newObj)); err != nil { + return err + } + } + } } return nil @@ -298,14 +347,19 @@ func (ns *ns) deletePod(obj *coreapi.Pod) error { if err := ns.ips.DelEntry(obj.ObjectMeta.UID, ns.egressDefaultAllowIPSet, obj.Status.PodIP); err != nil { return err } - - return ns.podSelectors.delFromMatching(obj.ObjectMeta.UID, &obj.ObjectMeta.Labels, nil, obj.Status.PodIP) + if err := ns.podSelectors.delFromMatching(obj.ObjectMeta.UID, &obj.ObjectMeta.Labels, nil, obj.Status.PodIP); err != nil { + return err + } + if err := ns.namespacedPodsSelectors.delFromMatching(obj.ObjectMeta.UID, &obj.ObjectMeta.Labels, &ns.namespace.ObjectMeta.Labels, obj.Status.PodIP); err != nil { + return err + } + return nil } func (ns *ns) addNetworkPolicy(obj interface{}) error { // Analyse policy, determine which rules and ipsets are required - uid, rules, nsSelectors, podSelectors, ipBlocks, err := ns.analyse(obj) + uid, rules, nsSelectors, podSelectors, namespacedPodsSelectors, ipBlocks, err := ns.analyse(obj) if err != nil { return err } @@ -317,6 +371,9 @@ func (ns *ns) addNetworkPolicy(obj interface{}) error { if err := ns.podSelectors.provision(uid, nil, podSelectors); err != nil { return err } + if err := ns.namespacedPodsSelectors.provision(uid, nil, namespacedPodsSelectors); err != nil { + return err + } if err := ns.ipBlocks.provision(uid, nil, ipBlocks); err != nil { return err } @@ -325,11 +382,11 @@ func (ns *ns) addNetworkPolicy(obj interface{}) error { func (ns *ns) updateNetworkPolicy(oldObj, newObj interface{}) error { // Analyse the old and the new policy so we can determine differences - oldUID, oldRules, oldNsSelectors, oldPodSelectors, oldIPBlocks, err := ns.analyse(oldObj) + oldUID, oldRules, oldNsSelectors, oldPodSelectors, oldNamespacedPodsSelectors, oldIPBlocks, err := ns.analyse(oldObj) if err != nil { return err } - newUID, newRules, newNsSelectors, newPodSelectors, newIPBlocks, err := ns.analyse(newObj) + newUID, newRules, newNsSelectors, newPodSelectors, newNamespacedPodsSelectors, newIPBlocks, err := ns.analyse(newObj) if err != nil { return err } @@ -347,6 +404,9 @@ func (ns *ns) updateNetworkPolicy(oldObj, newObj interface{}) error { if err := ns.podSelectors.deprovision(oldUID, oldPodSelectors, newPodSelectors); err != nil { return err } + if err := ns.namespacedPodsSelectors.deprovision(oldUID, oldNamespacedPodsSelectors, newNamespacedPodsSelectors); err != nil { + return err + } if err := ns.ipBlocks.deprovision(oldUID, oldIPBlocks, newIPBlocks); err != nil { return err } @@ -356,6 +416,9 @@ func (ns *ns) updateNetworkPolicy(oldObj, newObj interface{}) error { if err := ns.podSelectors.provision(oldUID, oldPodSelectors, newPodSelectors); err != nil { return err } + if err := ns.namespacedPodsSelectors.provision(oldUID, oldNamespacedPodsSelectors, newNamespacedPodsSelectors); err != nil { + return err + } if err := ns.ipBlocks.provision(oldUID, oldIPBlocks, newIPBlocks); err != nil { return err } @@ -364,7 +427,7 @@ func (ns *ns) updateNetworkPolicy(oldObj, newObj interface{}) error { func (ns *ns) deleteNetworkPolicy(obj interface{}) error { // Analyse network policy to free resources - uid, rules, nsSelectors, podSelectors, ipBlocks, err := ns.analyse(obj) + uid, rules, nsSelectors, podSelectors, namespacedPodsSelectors, ipBlocks, err := ns.analyse(obj) if err != nil { return err } @@ -381,6 +444,9 @@ func (ns *ns) deleteNetworkPolicy(obj interface{}) error { if err := ns.podSelectors.deprovision(uid, podSelectors, nil); err != nil { return err } + if err := ns.namespacedPodsSelectors.deprovision(uid, namespacedPodsSelectors, nil); err != nil { + return err + } if err := ns.ipBlocks.deprovision(uid, ipBlocks, nil); err != nil { return err } @@ -546,7 +612,7 @@ func (ns *ns) defaultAllowIPSetName(pt policyType) ipset.Name { func (ns *ns) analyse(obj interface{}) ( uid types.UID, rules map[string]*ruleSpec, - nsSelectors, podSelectors map[string]*selectorSpec, + nsSelectors, podSelectors, namespacedPodsSelectors map[string]*selectorSpec, ipBlocks map[string]*ipBlockSpec, err error) { @@ -562,7 +628,7 @@ func (ns *ns) analyse(obj interface{}) ( ns.policies[uid] = obj // Analyse policy, determine which rules and ipsets are required - rules, nsSelectors, podSelectors, ipBlocks, err = ns.analysePolicy(obj.(*networkingv1.NetworkPolicy)) + rules, nsSelectors, podSelectors, namespacedPodsSelectors, ipBlocks, err = ns.analysePolicy(obj.(*networkingv1.NetworkPolicy)) if err != nil { return } From 7a6ba40982ba69e74e005da150ab674c23ee8887 Mon Sep 17 00:00:00 2001 From: Murali Reddy Date: Thu, 30 May 2019 15:58:18 +0530 Subject: [PATCH 3/4] fix unit test failures --- npc/controller.go | 32 +++++++++++++++++++------------- npc/controller_test.go | 34 ++++++++++++++++++++++++++++------ npc/namespace.go | 21 ++------------------- prog/weave-npc/main.go | 2 +- 4 files changed, 50 insertions(+), 39 deletions(-) diff --git a/npc/controller.go b/npc/controller.go index 45c34541..bf2ee837 100644 --- a/npc/controller.go +++ b/npc/controller.go @@ -5,13 +5,14 @@ import ( "sync" "github.com/pkg/errors" - coreapi "k8s.io/api/core/v1" - extnapi "k8s.io/api/extensions/v1beta1" - networkingv1 "k8s.io/api/networking/v1" - "github.com/weaveworks/weave/common" "github.com/weaveworks/weave/net/ipset" "github.com/weaveworks/weave/npc/iptables" + coreapi "k8s.io/api/core/v1" + extnapi "k8s.io/api/extensions/v1beta1" + networkingv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" ) type NetworkPolicyController interface { @@ -33,21 +34,22 @@ type controller struct { nodeName string // my node name - ipt iptables.Interface - ips ipset.Interface - + ipt iptables.Interface + ips ipset.Interface + clientset kubernetes.Interface nss map[string]*ns // ns name -> ns struct nsSelectors *selectorSet // selector string -> nsSelector namespacedPodSelectors *selectorSet defaultEgressDrop bool // flag to track if base iptable rule to drop egress traffic is added or not } -func New(nodeName string, ipt iptables.Interface, ips ipset.Interface) NetworkPolicyController { +func New(nodeName string, ipt iptables.Interface, ips ipset.Interface, clientset kubernetes.Interface) NetworkPolicyController { c := &controller{ - nodeName: nodeName, - ipt: ipt, - ips: ips, - nss: make(map[string]*ns)} + nodeName: nodeName, + ipt: ipt, + ips: ips, + clientset: clientset, + nss: make(map[string]*ns)} doNothing := func(*selector, policyType) error { return nil } c.nsSelectors = newSelectorSet(ips, c.onNewNsSelector, doNothing, doNothing) @@ -89,7 +91,11 @@ func (npc *controller) onNewNamespacePodsSelector(selector *selector) error { func (npc *controller) withNS(name string, f func(ns *ns) error) error { ns, found := npc.nss[name] if !found { - newNs, err := newNS(name, npc.nodeName, npc.ipt, npc.ips, npc.nsSelectors, npc.namespacedPodSelectors) + namespace, err := npc.clientset.CoreV1().Namespaces().Get(name, metav1.GetOptions{}) + if err != nil { + return err + } + newNs, err := newNS(name, npc.nodeName, npc.ipt, npc.ips, npc.nsSelectors, npc.namespacedPodSelectors, namespace) if err != nil { return err } diff --git a/npc/controller_test.go b/npc/controller_test.go index 21488f03..9ee42445 100644 --- a/npc/controller_test.go +++ b/npc/controller_test.go @@ -14,6 +14,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/client-go/kubernetes/fake" ) type mockSet struct { @@ -198,7 +199,10 @@ func TestRegressionPolicyNamespaceOrdering3059(t *testing.T) { // Namespaces first m := newMockIPSet() - controller := New("foo", newMockIPTables(), &m) + client := fake.NewSimpleClientset() + controller := New("foo", newMockIPTables(), &m, client) + client.CoreV1().Namespaces().Create(sourceNamespace) + client.CoreV1().Namespaces().Create(destinationNamespace) const ( selectorIPSetName = "weave-I239Zp%sCvoVt*D6u=A!2]YEk" @@ -214,7 +218,7 @@ func TestRegressionPolicyNamespaceOrdering3059(t *testing.T) { // NetworkPolicy first m = newMockIPSet() - controller = New("foo", newMockIPTables(), &m) + controller = New("foo", newMockIPTables(), &m, &fake.Clientset{}) controller.AddNetworkPolicy(networkPolicy) @@ -235,7 +239,7 @@ func TestDefaultAllow(t *testing.T) { ) m := newMockIPSet() - controller := New("bar", newMockIPTables(), &m) + controller := New("bar", newMockIPTables(), &m, &fake.Clientset{}) defaultNamespace := &coreapi.Namespace{ ObjectMeta: metav1.ObjectMeta{ @@ -363,13 +367,15 @@ func TestOutOfOrderPodEvents(t *testing.T) { ) m := newMockIPSet() - controller := New("qux", newMockIPTables(), &m) + client := fake.NewSimpleClientset() + controller := New("qux", newMockIPTables(), &m, client) defaultNamespace := &coreapi.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: "default", }, } + client.CoreV1().Namespaces().Create(defaultNamespace) controller.AddNamespace(defaultNamespace) podFoo := &coreapi.Pod{ @@ -445,13 +451,15 @@ func TestNewTargetSelector(t *testing.T) { ) m := newMockIPSet() - controller := New("baz", newMockIPTables(), &m) + client := fake.NewSimpleClientset() + controller := New("baz", newMockIPTables(), &m, client) defaultNamespace := &coreapi.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: "default", }, } + client.CoreV1().Namespaces().Create(defaultNamespace) controller.AddNamespace(defaultNamespace) podFoo := &coreapi.Pod{ @@ -521,7 +529,21 @@ func TestEgressPolicyWithIPBlock(t *testing.T) { m := newMockIPSet() ipt := newMockIPTables() - controller := New("foo", ipt, &m) + client := fake.NewSimpleClientset() + controller := New("foo", ipt, &m, client) + + defaultNamespace := &coreapi.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + }, + } + nonDefaultNamespace := &coreapi.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "non-default", + }, + } + client.CoreV1().Namespaces().Create(defaultNamespace) + client.CoreV1().Namespaces().Create(nonDefaultNamespace) netpolFoo := &networkingv1.NetworkPolicy{ ObjectMeta: metav1.ObjectMeta{ diff --git a/npc/namespace.go b/npc/namespace.go index 6959fcd0..5ea24cea 100644 --- a/npc/namespace.go +++ b/npc/namespace.go @@ -10,8 +10,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/uuid" - "k8s.io/client-go/kubernetes" - "k8s.io/client-go/rest" "github.com/weaveworks/weave/common" "github.com/weaveworks/weave/net/ipset" @@ -46,32 +44,17 @@ type ns struct { rules *ruleSet } -func newNS(name, nodeName string, ipt iptables.Interface, ips ipset.Interface, nsSelectors *selectorSet, namespacedPodsSelectors *selectorSet) (*ns, error) { +func newNS(name, nodeName string, ipt iptables.Interface, ips ipset.Interface, nsSelectors *selectorSet, namespacedPodsSelectors *selectorSet, namespaceObj *coreapi.Namespace) (*ns, error) { allPods, err := newSelectorSpec(&metav1.LabelSelector{}, nil, nil, name, ipset.HashIP) if err != nil { return nil, err } - config, err := rest.InClusterConfig() - if err != nil { - return nil, err - } - - client, err := kubernetes.NewForConfig(config) - if err != nil { - return nil, err - } - - namespace, err := client.CoreV1().Namespaces().Get(name, metav1.GetOptions{}) - if err != nil { - return nil, err - } - ns := &ns{ ipt: ipt, ips: ips, name: name, - namespace: namespace, + namespace: namespaceObj, nodeName: nodeName, pods: make(map[types.UID]*coreapi.Pod), policies: make(map[types.UID]interface{}), diff --git a/prog/weave-npc/main.go b/prog/weave-npc/main.go index 3944cb88..d5231b7e 100644 --- a/prog/weave-npc/main.go +++ b/prog/weave-npc/main.go @@ -254,7 +254,7 @@ func root(cmd *cobra.Command, args []string) { handleError(resetIPSets(ips)) handleError(createBaseRules(ipt, ips)) - npc := npc.New(nodeName, ipt, ips) + npc := npc.New(nodeName, ipt, ips, client) nsController := makeController(client.Core().RESTClient(), "namespaces", &coreapi.Namespace{}, cache.ResourceEventHandlerFuncs{ From a6f2ee5ccc9159cee79cad5cacc0620fc027570b Mon Sep 17 00:00:00 2001 From: Murali Reddy Date: Fri, 7 Jun 2019 11:52:40 +0530 Subject: [PATCH 4/4] add seperate match function to matach pod/namespace/namespacedpod selectors --- npc/controller.go | 11 ++++---- npc/namespace.go | 40 +++++++++++++------------- npc/selector.go | 71 ++++++++++++++++++++++++++++++++++++++--------- 3 files changed, 84 insertions(+), 38 deletions(-) diff --git a/npc/controller.go b/npc/controller.go index bf2ee837..ecba6056 100644 --- a/npc/controller.go +++ b/npc/controller.go @@ -5,14 +5,15 @@ import ( "sync" "github.com/pkg/errors" - "github.com/weaveworks/weave/common" - "github.com/weaveworks/weave/net/ipset" - "github.com/weaveworks/weave/npc/iptables" coreapi "k8s.io/api/core/v1" extnapi "k8s.io/api/extensions/v1beta1" networkingv1 "k8s.io/api/networking/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" + + "github.com/weaveworks/weave/common" + "github.com/weaveworks/weave/net/ipset" + "github.com/weaveworks/weave/npc/iptables" ) type NetworkPolicyController interface { @@ -60,7 +61,7 @@ func New(nodeName string, ipt iptables.Interface, ips ipset.Interface, clientset func (npc *controller) onNewNsSelector(selector *selector) error { for _, ns := range npc.nss { if ns.namespace != nil { - if selector.matches(nil, &ns.namespace.ObjectMeta.Labels) { + if selector.matchesNamespaceSelector(ns.namespace.ObjectMeta.Labels) { if err := selector.addEntry(ns.namespace.ObjectMeta.UID, string(ns.allPods.ipsetName), namespaceComment(ns)); err != nil { return err } @@ -75,7 +76,7 @@ func (npc *controller) onNewNamespacePodsSelector(selector *selector) error { if ns.namespace != nil && len(ns.pods) > 0 { for _, pod := range ns.pods { if hasIP(pod) { - if selector.matches(&pod.ObjectMeta.Labels, &ns.namespace.ObjectMeta.Labels) { + if selector.matchesNamespacedPodSelector(pod.ObjectMeta.Labels, ns.namespace.ObjectMeta.Labels) { if err := selector.addEntry(pod.ObjectMeta.UID, pod.Status.PodIP, podComment(pod)); err != nil { return err } diff --git a/npc/namespace.go b/npc/namespace.go index 5ea24cea..3c061111 100644 --- a/npc/namespace.go +++ b/npc/namespace.go @@ -98,7 +98,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, nil) { + if selector.matchesPodSelector(pod.ObjectMeta.Labels) { if err := selector.addEntry(pod.ObjectMeta.UID, pod.Status.PodIP, podComment(pod)); err != nil { return err } @@ -114,7 +114,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, nil) { + if selector.matchesPodSelector(pod.ObjectMeta.Labels) { if err := ns.ips.DelEntry(pod.ObjectMeta.UID, ipset, pod.Status.PodIP); err != nil { return err } @@ -128,7 +128,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, nil) { + if selector.matchesPodSelector(pod.ObjectMeta.Labels) { if err := ns.addToDefaultAllowIfNoMatching(pod, policyType); err != nil { return err } @@ -144,7 +144,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, nil) { + if ns.podSelectors.targetSelectorExist(s, policyType) && s.matchesPodSelector(pod.ObjectMeta.Labels) { found = true break } @@ -172,7 +172,7 @@ func (ns *ns) addPod(obj *coreapi.Pod) error { return nil } - foundIngress, foundEgress, err := ns.podSelectors.addToMatching(obj.ObjectMeta.UID, &obj.ObjectMeta.Labels, nil, obj.Status.PodIP, podComment(obj)) + foundIngress, foundEgress, err := ns.podSelectors.addToMatchingPodSelector(obj.ObjectMeta.UID, obj.ObjectMeta.Labels, obj.Status.PodIP, podComment(obj)) if err != nil { return err } @@ -188,7 +188,7 @@ func (ns *ns) addPod(obj *coreapi.Pod) error { } } - _, _, err = ns.namespacedPodsSelectors.addToMatching(obj.ObjectMeta.UID, &obj.ObjectMeta.Labels, &ns.namespace.ObjectMeta.Labels, obj.Status.PodIP, podComment(obj)) + err = ns.namespacedPodsSelectors.addToMatchingNamespacedPodSelector(obj.ObjectMeta.UID, obj.ObjectMeta.Labels, ns.namespace.ObjectMeta.Labels, obj.Status.PodIP, podComment(obj)) if err != nil { return err } @@ -211,15 +211,15 @@ func (ns *ns) updatePod(oldObj, newObj *coreapi.Pod) error { if err := ns.ips.DelEntry(oldObj.ObjectMeta.UID, ns.egressDefaultAllowIPSet, oldObj.Status.PodIP); err != nil { return err } - if err := ns.namespacedPodsSelectors.delFromMatching(oldObj.ObjectMeta.UID, &oldObj.ObjectMeta.Labels, &ns.namespace.ObjectMeta.Labels, oldObj.Status.PodIP); err != nil { + if err := ns.namespacedPodsSelectors.delFromMatchingNamespacedPodSelector(oldObj.ObjectMeta.UID, oldObj.ObjectMeta.Labels, ns.namespace.ObjectMeta.Labels, oldObj.Status.PodIP); err != nil { return err } - return ns.podSelectors.delFromMatching(oldObj.ObjectMeta.UID, &oldObj.ObjectMeta.Labels, nil, oldObj.Status.PodIP) + return ns.podSelectors.delFromMatchingPodSelector(oldObj.ObjectMeta.UID, oldObj.ObjectMeta.Labels, oldObj.Status.PodIP) } if !hasIP(oldObj) && hasIP(newObj) { - foundIngress, foundEgress, err := ns.podSelectors.addToMatching(newObj.ObjectMeta.UID, &newObj.ObjectMeta.Labels, nil, newObj.Status.PodIP, podComment(newObj)) + foundIngress, foundEgress, err := ns.podSelectors.addToMatchingPodSelector(newObj.ObjectMeta.UID, newObj.ObjectMeta.Labels, newObj.Status.PodIP, podComment(newObj)) if err != nil { return err } @@ -234,7 +234,7 @@ func (ns *ns) updatePod(oldObj, newObj *coreapi.Pod) error { return err } } - _, _, err = ns.namespacedPodsSelectors.addToMatching(newObj.ObjectMeta.UID, &newObj.ObjectMeta.Labels, &ns.namespace.ObjectMeta.Labels, newObj.Status.PodIP, podComment(newObj)) + err = ns.namespacedPodsSelectors.addToMatchingNamespacedPodSelector(newObj.ObjectMeta.UID, newObj.ObjectMeta.Labels, ns.namespace.ObjectMeta.Labels, newObj.Status.PodIP, podComment(newObj)) if err != nil { return err } @@ -255,8 +255,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, nil) - newMatch := ps.matches(&newObj.ObjectMeta.Labels, nil) + oldMatch := ps.matchesPodSelector(oldObj.ObjectMeta.Labels) + newMatch := ps.matchesPodSelector(newObj.ObjectMeta.Labels) if oldMatch == newMatch && oldObj.Status.PodIP == newObj.Status.PodIP { continue } @@ -279,8 +279,8 @@ func (ns *ns) updatePod(oldObj, newObj *coreapi.Pod) error { } } for _, ps := range ns.namespacedPodsSelectors.entries { - oldMatch := ps.matches(&oldObj.ObjectMeta.Labels, &ns.namespace.ObjectMeta.Labels) - newMatch := ps.matches(&newObj.ObjectMeta.Labels, &ns.namespace.ObjectMeta.Labels) + oldMatch := ps.matchesNamespacedPodSelector(oldObj.ObjectMeta.Labels, ns.namespace.ObjectMeta.Labels) + newMatch := ps.matchesNamespacedPodSelector(newObj.ObjectMeta.Labels, ns.namespace.ObjectMeta.Labels) if oldMatch == newMatch && oldObj.Status.PodIP == newObj.Status.PodIP { continue } @@ -330,10 +330,10 @@ func (ns *ns) deletePod(obj *coreapi.Pod) error { if err := ns.ips.DelEntry(obj.ObjectMeta.UID, ns.egressDefaultAllowIPSet, obj.Status.PodIP); err != nil { return err } - if err := ns.podSelectors.delFromMatching(obj.ObjectMeta.UID, &obj.ObjectMeta.Labels, nil, obj.Status.PodIP); err != nil { + if err := ns.podSelectors.delFromMatchingPodSelector(obj.ObjectMeta.UID, obj.ObjectMeta.Labels, obj.Status.PodIP); err != nil { return err } - if err := ns.namespacedPodsSelectors.delFromMatching(obj.ObjectMeta.UID, &obj.ObjectMeta.Labels, &ns.namespace.ObjectMeta.Labels, obj.Status.PodIP); err != nil { + if err := ns.namespacedPodsSelectors.delFromMatchingNamespacedPodSelector(obj.ObjectMeta.UID, obj.ObjectMeta.Labels, ns.namespace.ObjectMeta.Labels, obj.Status.PodIP); err != nil { return err } return nil @@ -503,7 +503,7 @@ func (ns *ns) addNamespace(obj *coreapi.Namespace) error { } // Add namespace ipset to matching namespace selectors - _, _, err := ns.nsSelectors.addToMatching(obj.ObjectMeta.UID, nil, &obj.ObjectMeta.Labels, string(ns.allPods.ipsetName), namespaceComment(ns)) + err := ns.nsSelectors.addToMatchingNamespaceSelector(obj.ObjectMeta.UID, obj.ObjectMeta.Labels, string(ns.allPods.ipsetName), namespaceComment(ns)) return err } @@ -513,8 +513,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(nil, &oldObj.ObjectMeta.Labels) - newMatch := selector.matches(nil, &newObj.ObjectMeta.Labels) + oldMatch := selector.matchesNamespaceSelector(oldObj.ObjectMeta.Labels) + newMatch := selector.matchesNamespaceSelector(newObj.ObjectMeta.Labels) if oldMatch == newMatch { continue } @@ -543,7 +543,7 @@ func (ns *ns) deleteNamespace(obj *coreapi.Namespace) error { } // Remove namespace ipset from any matching namespace selectors - err := ns.nsSelectors.delFromMatching(obj.ObjectMeta.UID, nil, &obj.ObjectMeta.Labels, string(ns.allPods.ipsetName)) + err := ns.nsSelectors.delFromMatchingNamespaceSelector(obj.ObjectMeta.UID, obj.ObjectMeta.Labels, string(ns.allPods.ipsetName)) if err != nil { return err } diff --git a/npc/selector.go b/npc/selector.go index 1a321b94..1be5f9aa 100644 --- a/npc/selector.go +++ b/npc/selector.go @@ -75,15 +75,16 @@ type selector struct { spec *selectorSpec } -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) matchesPodSelector(podsLabelMap map[string]string) bool { + return s.spec.podSelector.Matches(labels.Set(podsLabelMap)) +} + +func (s *selector) matchesNamespaceSelector(namespaceLabelMap map[string]string) bool { + return s.spec.namespaceSelector.Matches(labels.Set(namespaceLabelMap)) +} + +func (s *selector) matchesNamespacedPodSelector(podsLabelMap, namespaceLabelMap map[string]string) bool { + return s.spec.podSelector.Matches(labels.Set(podsLabelMap)) && s.spec.namespaceSelector.Matches(labels.Set(namespaceLabelMap)) } func (s *selector) addEntry(user types.UID, entry string, comment string) error { @@ -126,11 +127,11 @@ func newSelectorSet(ips ipset.Interface, onNewSelector selectorFn, onNewTargetSe targetSelectorsCount: make(map[string]map[policyType]int)} } -func (ss *selectorSet) addToMatching(user types.UID, podLabelsMap *map[string]string, namespaceLabelsMap *map[string]string, entry string, comment string) (bool, bool, error) { +func (ss *selectorSet) addToMatchingPodSelector(user types.UID, podLabelsMap map[string]string, entry string, comment string) (bool, bool, error) { foundIngress := false foundEgress := false for _, s := range ss.entries { - if s.matches(podLabelsMap, namespaceLabelsMap) { + if s.matchesPodSelector(podLabelsMap) { if ss.targetSelectorExist(s, policyTypeIngress) { foundIngress = true } @@ -145,9 +146,53 @@ func (ss *selectorSet) addToMatching(user types.UID, podLabelsMap *map[string]st return foundIngress, foundEgress, nil } -func (ss *selectorSet) delFromMatching(user types.UID, podLabelsMap *map[string]string, namespaceLabelsMap *map[string]string, entry string) error { +func (ss *selectorSet) addToMatchingNamespaceSelector(user types.UID, namespaceLabelsMap map[string]string, entry string, comment string) error { for _, s := range ss.entries { - if s.matches(podLabelsMap, namespaceLabelsMap) { + if s.matchesNamespaceSelector(namespaceLabelsMap) { + if err := s.addEntry(user, entry, comment); err != nil { + return err + } + } + } + return nil +} + +func (ss *selectorSet) addToMatchingNamespacedPodSelector(user types.UID, podLabelsMap map[string]string, namespaceLabelsMap map[string]string, entry string, comment string) error { + for _, s := range ss.entries { + if s.matchesNamespacedPodSelector(podLabelsMap, namespaceLabelsMap) { + if err := s.addEntry(user, entry, comment); err != nil { + return err + } + } + } + return nil +} + +func (ss *selectorSet) delFromMatchingPodSelector(user types.UID, podLabelsMap map[string]string, entry string) error { + for _, s := range ss.entries { + if s.matchesPodSelector(podLabelsMap) { + if err := s.delEntry(user, entry); err != nil { + return err + } + } + } + return nil +} + +func (ss *selectorSet) delFromMatchingNamespaceSelector(user types.UID, namespaceLabelsMap map[string]string, entry string) error { + for _, s := range ss.entries { + if s.matchesNamespaceSelector(namespaceLabelsMap) { + if err := s.delEntry(user, entry); err != nil { + return err + } + } + } + return nil +} + +func (ss *selectorSet) delFromMatchingNamespacedPodSelector(user types.UID, podLabelsMap map[string]string, namespaceLabelsMap map[string]string, entry string) error { + for _, s := range ss.entries { + if s.matchesNamespacedPodSelector(podLabelsMap, namespaceLabelsMap) { if err := s.delEntry(user, entry); err != nil { return err }