From d474c1c26daeb4f57e4ff81d37d09514a460ca66 Mon Sep 17 00:00:00 2001 From: Martynas Pumputis Date: Sat, 19 May 2018 11:44:12 +0200 Subject: [PATCH] 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 | 14 +++++---- npc/controller.go | 2 +- npc/controller_test.go | 1 + npc/namespace.go | 36 ++++++++++++++--------- npc/selector.go | 65 ++++++++++++++++++++++++------------------ npc/types.go | 15 ++++++++++ 6 files changed, 84 insertions(+), 49 deletions(-) create mode 100644 npc/types.go diff --git a/npc/analyser.go b/npc/analyser.go index 50246a38..bd7203cf 100644 --- a/npc/analyser.go +++ b/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 } diff --git a/npc/controller.go b/npc/controller.go index 6bd26b1e..4b3415a4 100644 --- a/npc/controller.go +++ b/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 diff --git a/npc/controller_test.go b/npc/controller_test.go index 6928fc52..0336416a 100644 --- a/npc/controller_test.go +++ b/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 diff --git a/npc/namespace.go b/npc/namespace.go index 2fa91887..cf089c04 100644 --- a/npc/namespace.go +++ b/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 } } diff --git a/npc/selector.go b/npc/selector.go index 2a40e185..496cfd8f 100644 --- a/npc/selector.go +++ b/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 } } diff --git a/npc/types.go b/npc/types.go new file mode 100644 index 00000000..3b594ff9 --- /dev/null +++ b/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" +}