From 1649080a35d3e831cc3e6d8f862fba4bf2f1fef4 Mon Sep 17 00:00:00 2001
From: Martynas Pumputis <martynas@weave.works>
Date: Sun, 15 Jul 2018 10:17:01 +0200
Subject: [PATCH] npc: Add ns name into ipBlock ipset name

Otherwise, the same ipBlock ipset can be used among different namespaces which
results in panic as refcounting of ipsets happens only per namespace, so
npc will try to create the same ipset twice.
---
 npc/analyser.go        |  2 +-
 npc/controller_test.go | 41 +++++++++++++++++++++++++++++++++++++----
 npc/ipblock.go         |  7 ++++---
 3 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/npc/analyser.go b/npc/analyser.go
index f94dde8f..a19bb711 100644
--- a/npc/analyser.go
+++ b/npc/analyser.go
@@ -136,7 +136,7 @@ func (ns *ns) analysePolicy(policy *networkingv1.NetworkPolicy) (
 						nsSelectors[dstSelector.key] = dstSelector
 						dstRuleHost = dstSelector
 					} else if peer.IPBlock != nil {
-						ipBlock := newIPBlockSpec(peer.IPBlock)
+						ipBlock := newIPBlockSpec(peer.IPBlock, ns.name)
 						ipBlocks[ipBlock.key] = ipBlock
 						dstRuleHost = ipBlock
 					}
diff --git a/npc/controller_test.go b/npc/controller_test.go
index fac426dd..21488f03 100644
--- a/npc/controller_test.go
+++ b/npc/controller_test.go
@@ -514,15 +514,16 @@ func TestNewTargetSelector(t *testing.T) {
 
 func TestEgressPolicyWithIPBlock(t *testing.T) {
 	const (
-		fooPodIP        = "10.32.0.10"
-		exceptIPSetName = "weave-LKf)d{$q[dkYlH3Ko6eFcdc7z"
+		fooPodIP                    = "10.32.0.10"
+		exceptIPSetName             = "weave-j:W$5om!$8JS})buYAD7q^#sX"
+		exceptIPSetNameInNonDefault = "weave-|*P{aL2C@#N/1IZ!IvQItv_pt"
 	)
 
 	m := newMockIPSet()
 	ipt := newMockIPTables()
 	controller := New("foo", ipt, &m)
 
-	netpol := &networkingv1.NetworkPolicy{
+	netpolFoo := &networkingv1.NetworkPolicy{
 		ObjectMeta: metav1.ObjectMeta{
 			UID:       "netpol-foo",
 			Name:      "allow-from-bar-to-foo",
@@ -546,7 +547,8 @@ func TestEgressPolicyWithIPBlock(t *testing.T) {
 			}},
 		},
 	}
-	controller.AddNetworkPolicy(netpol)
+	err := controller.AddNetworkPolicy(netpolFoo)
+	require.NoError(t, err)
 
 	require.Equal(t, 2, len(m.sets[exceptIPSetName].subSets))
 	require.True(t, m.entryExists(exceptIPSetName, "192.168.48.1/32"))
@@ -558,4 +560,35 @@ func TestEgressPolicyWithIPBlock(t *testing.T) {
 		require.Contains(t, rule, "-d 192.168.48.0/24 -m set ! --match-set "+exceptIPSetName+" dst")
 	}
 
+	// Check that we create a new ipset for the ipBlock bellow. An ipset with the
+	// same content already exists (created by netpolFoo), but we need to create
+	// a new one, as netpolBar is in a different namespace and weave-npc does
+	// object refcounting only within namespace boundaries.
+	netpolBar := &networkingv1.NetworkPolicy{
+		ObjectMeta: metav1.ObjectMeta{
+			UID:       "netpol-foo-bar",
+			Name:      "allow-from-bar-to-foo-in-non-default",
+			Namespace: "non-default",
+		},
+		Spec: networkingv1.NetworkPolicySpec{
+			PolicyTypes: []networkingv1.PolicyType{
+				networkingv1.PolicyTypeEgress,
+			},
+			PodSelector: metav1.LabelSelector{MatchLabels: map[string]string{"run": "bar"}},
+			Egress: []networkingv1.NetworkPolicyEgressRule{{
+				To: []networkingv1.NetworkPolicyPeer{{
+					IPBlock: &networkingv1.IPBlock{
+						CIDR: "192.168.48.0/24",
+						Except: []string{
+							"192.168.48.1/32",
+							"192.168.48.2/32",
+						},
+					},
+				}},
+			}},
+		},
+	}
+	err = controller.AddNetworkPolicy(netpolBar)
+	require.NoError(t, err)
+	require.Equal(t, 2, len(m.sets[exceptIPSetNameInNonDefault].subSets))
 }
diff --git a/npc/ipblock.go b/npc/ipblock.go
index 517ffb20..7d2579d0 100644
--- a/npc/ipblock.go
+++ b/npc/ipblock.go
@@ -16,15 +16,16 @@ type ipBlockSpec struct {
 	key       string
 	ipsetName ipset.Name // ipset for storing excepted CIDRs
 	ipBlock   *networkingv1.IPBlock
+	nsName    string // Namespace name
 }
 
-func newIPBlockSpec(ipb *networkingv1.IPBlock) *ipBlockSpec {
-	spec := &ipBlockSpec{ipBlock: ipb}
+func newIPBlockSpec(ipb *networkingv1.IPBlock, nsName string) *ipBlockSpec {
+	spec := &ipBlockSpec{ipBlock: ipb, nsName: nsName}
 
 	if len(ipb.Except) > 0 {
 		sort.Strings(ipb.Except)
 		spec.key = strings.Join(ipb.Except, " ")
-		spec.ipsetName = ipset.Name(IpsetNamePrefix + shortName("ipblock-except:"+spec.key))
+		spec.ipsetName = ipset.Name(IpsetNamePrefix + shortName(nsName+":"+"ipblock-except:"+spec.key))
 	}
 
 	return spec