npc: Rename dstSelector to targetSelector

dstSelector becomes ambiguous once we introduce the egress.

So, from now on, we call selector a target selector if it selects
pods to which a netpol applies (aka NetworkPolicySpec.podSelector).

npc/analyser.go

@@ -25,11 +25,11 @@ func (ns *ns) analysePolicyLegacy(policy *extnapi.NetworkPolicy) (
 	rules = make(map[string]*ruleSpec)
 
 	policyTypes := []policyType{ingressPolicy}
-	dstSelector, err := newSelectorSpec(&policy.Spec.PodSelector, true, policyTypes, ns.name, ipset.HashIP)
+	targetSelector, err := newSelectorSpec(&policy.Spec.PodSelector, true, policyTypes, ns.name, ipset.HashIP)
 	if err != nil {
 		return nil, nil, nil, err
 	}
-	podSelectors[dstSelector.key] = dstSelector
+	podSelectors[targetSelector.key] = targetSelector
 
 	for _, ingressRule := range policy.Spec.Ingress {
 		if ingressRule.Ports != nil && len(ingressRule.Ports) == 0 {
@@ -46,13 +46,13 @@ func (ns *ns) analysePolicyLegacy(policy *extnapi.NetworkPolicy) (
 			// From is not provided, this rule matches all sources (traffic not restricted by source).
 			if ingressRule.Ports == nil {
 				// Ports is not provided, this rule matches all ports (traffic not restricted by port).
-				rule := newRuleSpec(ingressPolicy, nil, nil, dstSelector, nil)
+				rule := newRuleSpec(ingressPolicy, nil, nil, targetSelector, nil)
 				rules[rule.key] = rule
 			} else {
 				// Ports is present and contains at least one item, then this rule allows traffic
 				// only if the traffic matches at least one port in the ports list.
 				withNormalisedProtoAndPortLegacy(ingressRule.Ports, func(proto, port string) {
-					rule := newRuleSpec(ingressPolicy, &proto, nil, dstSelector, &port)
+					rule := newRuleSpec(ingressPolicy, &proto, nil, targetSelector, &port)
 					rules[rule.key] = rule
 				})
 			}
@@ -78,13 +78,13 @@ func (ns *ns) analysePolicyLegacy(policy *extnapi.NetworkPolicy) (
 
 				if ingressRule.Ports == nil {
 					// Ports is not provided, this rule matches all ports (traffic not restricted by port).
-					rule := newRuleSpec(ingressPolicy, nil, srcSelector, dstSelector, nil)
+					rule := newRuleSpec(ingressPolicy, nil, srcSelector, targetSelector, nil)
 					rules[rule.key] = rule
 				} else {
 					// Ports is present and contains at least one item, then this rule allows traffic
 					// only if the traffic matches at least one port in the ports list.
 					withNormalisedProtoAndPortLegacy(ingressRule.Ports, func(proto, port string) {
-						rule := newRuleSpec(ingressPolicy, &proto, srcSelector, dstSelector, &port)
+						rule := newRuleSpec(ingressPolicy, &proto, srcSelector, targetSelector, &port)
 						rules[rule.key] = rule
 					})
 				}
@@ -114,16 +114,16 @@ func (ns *ns) analysePolicy(policy *networkingv1.NetworkPolicy) (
 		}
 	}
 	// If empty, matches all pods in a namespace
-	dstSelector, err := newSelectorSpec(&policy.Spec.PodSelector, true, policyTypes, ns.name, ipset.HashIP)
+	targetSelector, err := newSelectorSpec(&policy.Spec.PodSelector, true, policyTypes, ns.name, ipset.HashIP)
 	if err != nil {
 		return nil, nil, nil, err
 	}
 
-	// To prevent dstSelector being overwritten by a subsequent selector with
+	// To prevent targetSelector being overwritten by a subsequent selector with
 	// the same key, addIfNotExist MUST be used when adding to podSelectors.
-	// Otherwise, dstSelector' "dst" property might be lost resulting in
+	// Otherwise, targetSelector' "dst" property might be lost resulting in
 	// an invalid content of the "default-allow" ipset.
-	addIfNotExist(dstSelector, podSelectors)
+	addIfNotExist(targetSelector, podSelectors)
 
 	// If ingress is empty then this NetworkPolicy does not allow any ingress traffic
 	if policy.Spec.Ingress != nil && len(policy.Spec.Ingress) != 0 {
@@ -135,11 +135,11 @@ func (ns *ns) analysePolicy(policy *networkingv1.NetworkPolicy) (
 
 			if allSources {
 				if allPorts {
-					rule := newRuleSpec(ingressPolicy, nil, nil, dstSelector, nil)
+					rule := newRuleSpec(ingressPolicy, nil, nil, targetSelector, nil)
 					rules[rule.key] = rule
 				} else {
 					withNormalisedProtoAndPort(ingressRule.Ports, func(proto, port string) {
-						rule := newRuleSpec(ingressPolicy, &proto, nil, dstSelector, &port)
+						rule := newRuleSpec(ingressPolicy, &proto, nil, targetSelector, &port)
 						rules[rule.key] = rule
 					})
 				}
@@ -165,11 +165,11 @@ func (ns *ns) analysePolicy(policy *networkingv1.NetworkPolicy) (
 					}
 
 					if allPorts {
-						rule := newRuleSpec(ingressPolicy, nil, srcSelector, dstSelector, nil)
+						rule := newRuleSpec(ingressPolicy, nil, srcSelector, targetSelector, nil)
 						rules[rule.key] = rule
 					} else {
 						withNormalisedProtoAndPort(ingressRule.Ports, func(proto, port string) {
-							rule := newRuleSpec(ingressPolicy, &proto, srcSelector, dstSelector, &port)
+							rule := newRuleSpec(ingressPolicy, &proto, srcSelector, targetSelector, &port)
 							rules[rule.key] = rule
 						})
 					}
@@ -188,42 +188,41 @@ func (ns *ns) analysePolicy(policy *networkingv1.NetworkPolicy) (
 
 			if allDestinations {
 				if allPorts {
-					rule := newRuleSpec(egressPolicy, nil, dstSelector, nil, nil)
+					rule := newRuleSpec(egressPolicy, nil, targetSelector, nil, nil)
 					rules[rule.key] = rule
 				} else {
 					withNormalisedProtoAndPort(egressRule.Ports, func(proto, port string) {
-						rule := newRuleSpec(egressPolicy, &proto, dstSelector, nil, &port)
+						rule := newRuleSpec(egressPolicy, &proto, targetSelector, nil, &port)
 						rules[rule.key] = rule
 					})
 				}
 			} else {
 				for _, peer := range egressRule.To {
-					// TODO(brb) s/selector/dstSelector/ after s/dstSelector/targetSelector/
-					var selector *selectorSpec
+					var dstSelector *selectorSpec
 
 					// NetworkPolicyPeer describes a peer to allow traffic from.
 					// Exactly one of its fields must be specified.
 					if peer.PodSelector != nil {
-						selector, err = newSelectorSpec(peer.PodSelector, false, nil, ns.name, ipset.HashIP)
+						dstSelector, err = newSelectorSpec(peer.PodSelector, false, nil, ns.name, ipset.HashIP)
 						if err != nil {
 							return nil, nil, nil, err
 						}
-						addIfNotExist(selector, podSelectors)
+						addIfNotExist(dstSelector, podSelectors)
 
 					} else if peer.NamespaceSelector != nil {
-						selector, err = newSelectorSpec(peer.NamespaceSelector, false, nil, "", ipset.ListSet)
+						dstSelector, err = newSelectorSpec(peer.NamespaceSelector, false, nil, "", ipset.ListSet)
 						if err != nil {
 							return nil, nil, nil, err
 						}
-						nsSelectors[selector.key] = selector
+						nsSelectors[dstSelector.key] = dstSelector
 					}
 
 					if allPorts {
-						rule := newRuleSpec(egressPolicy, nil, dstSelector, selector, nil)
+						rule := newRuleSpec(egressPolicy, nil, targetSelector, dstSelector, nil)
 						rules[rule.key] = rule
 					} else {
 						withNormalisedProtoAndPort(egressRule.Ports, func(proto, port string) {
-							rule := newRuleSpec(egressPolicy, &proto, dstSelector, selector, &port)
+							rule := newRuleSpec(egressPolicy, &proto, targetSelector, dstSelector, &port)
 							rules[rule.key] = rule
 						})
 					}

npc/controller_test.go

@@ -386,7 +386,7 @@ func TestOutOfOrderPodEvents(t *testing.T) {
 }
 
 // Test case for https://github.com/weaveworks/weave/issues/3222
-func TestNewDstSelector(t *testing.T) {
+func TestNewTargetSelector(t *testing.T) {
 	const (
 		ingressDefaultAllowIPSetName = "weave-;rGqyMIl1HN^cfDki~Z$3]6!N"
 		podIP                        = "10.32.0.10"
@@ -431,7 +431,7 @@ func TestNewDstSelector(t *testing.T) {
 	}
 	controller.AddNetworkPolicy(netpolBar)
 
-	// netpolBar dst selector selects podFoo
+	// netpolBar target selector selects podFoo
 	require.False(t, m.entryExists(ingressDefaultAllowIPSetName, podIP))
 
 	netpolFoo := &networkingv1.NetworkPolicy{
@@ -453,9 +453,9 @@ func TestNewDstSelector(t *testing.T) {
 	controller.AddNetworkPolicy(netpolFoo)
 
 	controller.DeleteNetworkPolicy(netpolBar)
-	// netpolFoo dst-selects podFoo
+	// netpolFoo target-selects podFoo
 	require.False(t, m.entryExists(ingressDefaultAllowIPSetName, podIP))
 	controller.DeleteNetworkPolicy(netpolFoo)
-	// No netpol dst-selects podFoo
+	// No netpol target-selects podFoo
 	require.True(t, m.entryExists(ingressDefaultAllowIPSetName, podIP))
 }

npc/namespace.go

@@ -32,8 +32,8 @@ type ns struct {
 	uid     types.UID     // surrogate UID to own allPods selector
 	allPods *selectorSpec // hash:ip ipset of all pod IPs in this namespace
 
-	// stores IP addrs of pods which are not selected by any dst podSelector of
-	// any netpol; used only in non-legacy mode and is used as a dst in
+	// stores IP addrs of pods which are not selected by any target podSelector of
+	// any netpol; used only in non-legacy mode and is used as a target in
 	// the WEAVE-NPC-{INGRESS,EGRESS}-DEFAULT iptables chain.
 	ingressDefaultAllowIPSet ipset.Name
 	egressDefaultAllowIPSet  ipset.Name
@@ -64,7 +64,7 @@ func newNS(name, nodeName string, legacy bool, ipt iptables.Interface, ips ipset
 		rules:       newRuleSet(ipt),
 		legacy:      legacy}
 
-	ns.podSelectors = newSelectorSet(ips, ns.onNewPodSelector, ns.onNewDstPodSelector, ns.onDestroyDstPodSelector)
+	ns.podSelectors = newSelectorSet(ips, ns.onNewPodSelector, ns.onNewTargetPodSelector, ns.onDestroyTargetPodSelector)
 
 	if !legacy {
 		ingressDefaultAllowIPSet := ipset.Name(IpsetNamePrefix + shortName("ingress-default-allow:"+name))
@@ -110,7 +110,7 @@ func (ns *ns) onNewPodSelector(selector *selector) error {
 	return nil
 }
 
-func (ns *ns) onNewDstPodSelector(selector *selector, policyType policyType) error {
+func (ns *ns) onNewTargetPodSelector(selector *selector, policyType policyType) error {
 	if ns.legacy {
 		return nil
 	}
@@ -130,7 +130,7 @@ func (ns *ns) onNewDstPodSelector(selector *selector, policyType policyType) err
 	return nil
 }
 
-func (ns *ns) onDestroyDstPodSelector(selector *selector, policyType policyType) error {
+func (ns *ns) onDestroyTargetPodSelector(selector *selector, policyType policyType) error {
 	if ns.legacy {
 		return nil
 	}
@@ -148,12 +148,12 @@ func (ns *ns) onDestroyDstPodSelector(selector *selector, policyType policyType)
 	return nil
 }
 
-// Add pod IP addr to default-allow ipset if there are no matching dst selectors
+// Add pod IP addr to default-allow ipset if there are no matching target selectors
 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, policyType) && s.matches(pod.ObjectMeta.Labels) {
+		if ns.podSelectors.targetSelectorExist(s, policyType) && s.matches(pod.ObjectMeta.Labels) {
 			found = true
 			break
 		}
@@ -189,7 +189,7 @@ func (ns *ns) addPod(obj *coreapi.Pod) error {
 	if err != nil {
 		return err
 	}
-	// If there are no matching dst selectors, add the pod to default-allow
+	// If there are no matching target selectors, add the pod to default-allow
 	if !ns.legacy {
 		if !foundIngress {
 			if err := ns.ips.AddEntry(obj.ObjectMeta.UID, ns.ingressDefaultAllowIPSet, obj.Status.PodIP, podComment(obj)); err != nil {
@@ -301,7 +301,7 @@ func (ns *ns) updatePod(oldObj, newObj *coreapi.Pod) error {
 
 func (ns *ns) addOrRemoveToDefaultAllowIPSet(ps *selector, oldObj, newObj *coreapi.Pod, oldMatch, newMatch bool, policyType policyType) error {
 	ipset := ns.defaultAllowIPSetName(policyType)
-	if ns.podSelectors.dstSelectorExist(ps, policyType) {
+	if ns.podSelectors.targetSelectorExist(ps, policyType) {
 		switch {
 		case !oldMatch && newMatch:
 			if err := ns.ips.DelEntry(oldObj.ObjectMeta.UID, ipset, oldObj.Status.PodIP); err != nil {

npc/selector.go

@@ -13,7 +13,7 @@ type selectorSpec struct {
 	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
+	policyTypes []policyType    // TODO(brb) repharse: netpol types (egress, ingress or both) which initiated the target selector
 
 	ipsetType ipset.Type // type of ipset to provision
 	ipsetName ipset.Name // generated ipset name
@@ -63,30 +63,29 @@ type selectorSet struct {
 	ips           ipset.Interface
 	onNewSelector selectorFn
 
-	// invoked after dst selector has been provisioned for the first time
-	onNewDstSelector selectorWithPolicyTypeFn
-	// invoked after the last instance of dst selector has been deprovisioned
-	onDestroyDstSelector selectorWithPolicyTypeFn
+	// invoked after target selector has been provisioned for the first time
+	onNewTargetSelector selectorWithPolicyTypeFn
+	// invoked after the last instance of target selector has been deprovisioned
+	onDestroyTargetSelector 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;
+	// We need to keep track of target selector instances to be able to invoke
+	// onNewTargetSelector and onDestroyTargetSelector callbacks at the right time;
 	// selectorSpec.Key -> policyType -> count
-	// TODO(brb) dst -> target?
-	dstSelectorsCount map[string]map[policyType]int
+	targetSelectorsCount map[string]map[policyType]int
 }
 
-func newSelectorSet(ips ipset.Interface, onNewSelector selectorFn, onNewDstSelector, onDestroyDstSelector selectorWithPolicyTypeFn) *selectorSet {
+func newSelectorSet(ips ipset.Interface, onNewSelector selectorFn, onNewTargetSelector, onDestroyTargetSelector selectorWithPolicyTypeFn) *selectorSet {
 	return &selectorSet{
-		ips:                  ips,
-		onNewSelector:        onNewSelector,
-		onNewDstSelector:     onNewDstSelector,
-		onDestroyDstSelector: onDestroyDstSelector,
+		ips:                     ips,
+		onNewSelector:           onNewSelector,
+		onNewTargetSelector:     onNewTargetSelector,
+		onDestroyTargetSelector: onDestroyTargetSelector,
 		users:                make(map[string]map[types.UID]struct{}),
 		entries:              make(map[string]*selector),
-		dstSelectorsCount:    make(map[string]map[policyType]int)}
+		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) {
@@ -94,10 +93,10 @@ func (ss *selectorSet) addToMatching(user types.UID, labelMap map[string]string,
 	foundEgress := false
 	for _, s := range ss.entries {
 		if s.matches(labelMap) {
-			if ss.dstSelectorExist(s, ingressPolicy) {
+			if ss.targetSelectorExist(s, ingressPolicy) {
 				foundIngress = true
 			}
-			if ss.dstSelectorExist(s, egressPolicy) {
+			if ss.targetSelectorExist(s, egressPolicy) {
 				foundEgress = true
 			}
 			if err := s.addEntry(user, entry, comment); err != nil {
@@ -119,8 +118,8 @@ func (ss *selectorSet) delFromMatching(user types.UID, labelMap map[string]strin
 	return nil
 }
 
-func (ss *selectorSet) dstSelectorExist(s *selector, policyType policyType) bool {
-	return ss.dstSelectorsCount[s.spec.key][policyType] > 0
+func (ss *selectorSet) targetSelectorExist(s *selector, policyType policyType) bool {
+	return ss.targetSelectorsCount[s.spec.key][policyType] > 0
 }
 
 func (ss *selectorSet) deprovision(user types.UID, current, desired map[string]*selectorSpec) error {
@@ -138,9 +137,9 @@ func (ss *selectorSet) deprovision(user types.UID, current, desired map[string]*
 			}
 
 			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 {
+				ss.targetSelectorsCount[key][policyType]--
+				if ss.targetSelectorsCount[key][policyType] == 0 {
+					if err := ss.onDestroyTargetSelector(&selector{ss.ips, spec}, policyType); err != nil {
 						return err
 					}
 				}
@@ -170,12 +169,12 @@ func (ss *selectorSet) provision(user types.UID, current, desired map[string]*se
 			ss.users[key][user] = struct{}{}
 
 			for _, pt := range spec.policyTypes {
-				if _, found := ss.dstSelectorsCount[key]; !found {
-					ss.dstSelectorsCount[key] = make(map[policyType]int)
+				if _, found := ss.targetSelectorsCount[key]; !found {
+					ss.targetSelectorsCount[key] = make(map[policyType]int)
 				}
-				ss.dstSelectorsCount[key][pt]++
-				if ss.dstSelectorsCount[key][pt] == 1 {
-					if err := ss.onNewDstSelector(selector, pt); err != nil {
+				ss.targetSelectorsCount[key][pt]++
+				if ss.targetSelectorsCount[key][pt] == 1 {
+					if err := ss.onNewTargetSelector(selector, pt); err != nil {
 						return err
 					}
 				}