Adding safeguards against a potential core situation in confman.

Both in Reserve() and Free() calls core can theoretically occur when the chosen interface profile does not have "Alloc". This practically can only occur if a network was created in env where webhook was not running.
nokia · Oct 14, 2019 · 93b46c0 · 93b46c0
1 parent b16fdef
commit 93b46c0
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 3 deletions.
diff --git a/pkg/confman/confman.go b/pkg/confman/confman.go
@@ -24,6 +24,9 @@ func GetTenantConfig(danmClient danmclientset.Interface) (*danmtypes.TenantConfi
 
 func Reserve(danmClient danmclientset.Interface, tconf *danmtypes.TenantConfig, iface danmtypes.IfaceProfile) (int,error) {
   allocs := bitarray.NewBitArrayFromBase64(iface.Alloc)
+  if allocs.Len() == 0 {
+    return 0, errors.New("VNI allocations for interface:" + iface.Name + " is corrupt! Are you running without webhook?")
+  }
   vnis, err := cpuset.Parse(iface.VniRange)
   if err != nil {
     return 0, errors.New("vniRange for interface:" + iface.Name + " cannot be parsed because:" + err.Error())
@@ -84,11 +87,14 @@ func Free(danmClient danmclientset.Interface, tconf *danmtypes.TenantConfig, dne
     " as the used network details (interface name, VNI type) doe not match any entries in TenantConfig. This means your APIs were possibly tampered with!")
     return nil
   }
-  allocs := bitarray.NewBitArrayFromBase64(tconf.HostDevices[index].Alloc)
   vni := dnet.Spec.Options.Vlan
   if dnet.Spec.Options.Vxlan != 0 {
     vni = dnet.Spec.Options.Vxlan
   }
+  allocs := bitarray.NewBitArrayFromBase64(tconf.HostDevices[index].Alloc)
+  if allocs.Len() == 0 {
+    return errors.New("VNI allocations for interface:" + tconf.HostDevices[index].Name + " is corrupt! Are you running without webhook?")
+  }
   allocs.Reset(uint32(vni))
   tconf.HostDevices[index].Alloc = allocs.Encode()
   _, err := danmClient.DanmV1().TenantConfigs().Update(tconf)

diff --git a/test/uts/confman_test/confman_test.go b/test/uts/confman_test/confman_test.go
@@ -34,7 +34,7 @@ var (
     danmtypes.TenantConfig{ObjectMeta: meta_v1.ObjectMeta {Name: "secondConf"}},
   }
   reserveConfs = []danmtypes.TenantConfig {
-    danmtypes.TenantConfig{
+    danmtypes.TenantConfig {
       ObjectMeta: meta_v1.ObjectMeta {Name: "tconf"},
       HostDevices: []danmtypes.IfaceProfile {
         danmtypes.IfaceProfile{Name: "ens4", VniType: "vxlan", VniRange: "700-710", Alloc: utils.AllocFor5k},
@@ -44,18 +44,25 @@ var (
         danmtypes.IfaceProfile{Name: "nokia.k8s.io/sriov_ens1f0", VniType: "vxlan", VniRange: "1600-1650", Alloc: utils.AllocFor5k},
       },
     },
-    danmtypes.TenantConfig{
+    danmtypes.TenantConfig {
       ObjectMeta: meta_v1.ObjectMeta {Name: "error"},
       HostDevices: []danmtypes.IfaceProfile {
         danmtypes.IfaceProfile{Name: "ens4", VniType: "vxlan", VniRange: "800-810", Alloc: utils.AllocFor5k},
       },
     },
+    danmtypes.TenantConfig {
+      ObjectMeta: meta_v1.ObjectMeta {Name: "corrupt"},
+      HostDevices: []danmtypes.IfaceProfile {
+        danmtypes.IfaceProfile{Name: "corrupt", VniType: "vxlan", VniRange: "700-710", Alloc: ""},
+      },
+    },
   }
   reserveIfaces = []danmtypes.IfaceProfile {
     danmtypes.IfaceProfile{Name:"invalidVni", VniRange: "invalid"},
     danmtypes.IfaceProfile{Name: "ens4", VniType: "vxlan", VniRange: "700-710", Alloc: utils.AllocFor5k},
     danmtypes.IfaceProfile{Name: "ens4", VniType: "vlan", VniRange: "200,500-510", Alloc: utils.AllocFor5k},
     danmtypes.IfaceProfile{Name: "hupak", VniType: "vlan", VniRange: "1000,1001", Alloc: utils.AllocFor5k},
+    danmtypes.IfaceProfile{Name: "corrupt", VniType: "vxlan", VniRange: "700-710", Alloc: ""},
   }
   tconfSets = []TconfSet {
     TconfSet{name: "emptyTcs", tconfs: emptyTconfs},
@@ -92,6 +99,10 @@ var (
       ObjectMeta: meta_v1.ObjectMeta {Name: "novni"},
       Spec: danmtypes.DanmNetSpec{NetworkID: "internal", NetworkType: "ipvlan", Options: danmtypes.DanmNetOption{Device: "ens4"}},
     },
+    danmtypes.DanmNet {
+      ObjectMeta: meta_v1.ObjectMeta {Name: "corrupt"},
+      Spec: danmtypes.DanmNetSpec{NetworkID: "internal", NetworkType: "ipvlan", Options: danmtypes.DanmNetOption{Device: "corrupt", Vxlan: 700}},
+    },
   }
 )
 
@@ -120,6 +131,7 @@ var reserveTcs = []struct {
   {"noFreeVniInIface", "tconf", "ens4", "vlan", []int{200,510}, true, 0},
   {"errorUpdating", "error", "ens4", "vxlan", nil, true, 0},
   {"nonExistentProfile", "tconf", "hupak", "vlan", nil, true, 0},
+  {"corruptedVniAllocation", "corrupt", "corrupt", "", nil, true, 0},
 }
 
 var freeTcs = []struct {
@@ -139,6 +151,7 @@ var freeTcs = []struct {
   {"devicePoolWithVxlan", "tconf", "sriov_vxlan", "nokia.k8s.io/sriov_ens1f0", "vxlan", false, false},
   {"errorUpdating", "error", "ipvlan_vxlan", "ens4", "vxlan", false, true},
   {"noVnis", "tconf", "novni", "", "", false, false},
+  {"corruptedVniAllocation", "corrupt", "corrupt", "", "", false, true},
 }
 
 func TestGetTenantConfig(t *testing.T) {