Merge pull request #294 from msherif1234/bz_1928851

Bug 1928851: prevent manually creating netns with existing NetID
openshift · May 10, 2021 · 08deed1 · 08deed1
2 parents 9e25637 + de77f54
commit 08deed1
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 1 deletion.
diff --git a/pkg/network/node/vnids.go b/pkg/network/node/vnids.go
@@ -136,6 +136,16 @@ func (vmap *nodeVNIDMap) getVNID(name string) (uint32, error) {
 	return 0, fmt.Errorf("failed to find netid for namespace: %s in vnid map", name)
 }
 
+func (vmap *nodeVNIDMap) findDuplicateNetID(namespace string, netID uint32) (string, bool) {
+	names := vmap.GetNamespaces(netID)
+	for _, name := range names {
+		if name != namespace {
+			return name, true
+		}
+	}
+	return "", false
+}
+
 func (vmap *nodeVNIDMap) setVNID(name string, id uint32, mcEnabled bool) {
 	vmap.lock.Lock()
 	defer vmap.lock.Unlock()
@@ -204,6 +214,13 @@ func (vmap *nodeVNIDMap) handleAddOrUpdateNetNamespace(obj, _ interface{}, event
 	netns := obj.(*networkapi.NetNamespace)
 	klog.V(5).Infof("Watch %s event for NetNamespace %q", eventType, netns.Name)
 
+	// Skip this event if NetID already exists under different netns name
+	if name, found := vmap.findDuplicateNetID(netns.NetName, netns.NetID); found == true {
+		klog.Warningf("Netid %d for namespace %s already exists under different namespace %s",
+			netns.NetID, netns.NetName, name)
+		return
+	}
+
 	// Skip this event if nothing has changed
 	oldNetID, err := vmap.getVNID(netns.NetName)
 	oldMCEnabled := vmap.mcEnabled[netns.NetName]
@@ -225,6 +242,9 @@ func (vmap *nodeVNIDMap) handleDeleteNetNamespace(obj interface{}) {
 	klog.V(5).Infof("Watch %s event for NetNamespace %q", watch.Deleted, netns.Name)
 
 	// Unset VNID first so further operations don't see the deleted VNID
-	vmap.unsetVNID(netns.NetName)
+	if _, err := vmap.unsetVNID(netns.NetName); err != nil {
+		klog.Warningf("Failed to delete namespace %s with error %v", netns.NetName, err)
+		return
+	}
 	vmap.policy.DeleteNetNamespace(netns)
 }
diff --git a/pkg/network/node/vnids_test.go b/pkg/network/node/vnids_test.go
@@ -143,6 +143,27 @@ func TestNodeVNIDMap(t *testing.T) {
 	checkNamespaces(t, vmap, 5, []string{"foxtrot"})
 
 	checkAllocatedVNIDs(t, vmap, []uint32{1, 2, 5})
+
+	vmap.setVNID("dup_ns1", 10, false)
+	vmap.setVNID("dup_ns2", 20, false)
+
+	checkForDuplicateNetID(t, vmap, "ns1", 10) // duplicate with dup_ns1
+	checkForDuplicateNetID(t, vmap, "ns2", 20) // duplicate with dup_ns2
+
+	checkForNoDuplicateNetID(t, vmap, "ns1", 15)
+	checkForNoDuplicateNetID(t, vmap, "ns2", 25)
+}
+
+func checkForDuplicateNetID(t *testing.T, vmap *nodeVNIDMap, name string, id uint32) {
+	if _, found := vmap.findDuplicateNetID(name, id); found == false {
+		t.Fatalf("Unexpected failure: name %s id %d", name, id)
+	}
+}
+
+func checkForNoDuplicateNetID(t *testing.T, vmap *nodeVNIDMap, name string, id uint32) {
+	if _, found := vmap.findDuplicateNetID(name, id); found == true {
+		t.Fatalf("Unexpected failure: name %s id %d", name, id)
+	}
 }
 
 func checkExists(t *testing.T, vmap *nodeVNIDMap, name string, expected uint32) {