From 93b46c01682b492efec9c4661990d89976d2e3e5 Mon Sep 17 00:00:00 2001 From: Levente Kale Date: Mon, 14 Oct 2019 12:47:53 +0200 Subject: [PATCH] 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. --- pkg/confman/confman.go | 8 +++++++- test/uts/confman_test/confman_test.go | 17 +++++++++++++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/pkg/confman/confman.go b/pkg/confman/confman.go index b8b553fa..2b373270 100644 --- 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 index f6a0cb38..f2017aa2 100644 --- 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) {