From 4cee6b3cb1638ac3c1efed7cf5acb842ba15502e Mon Sep 17 00:00:00 2001 From: Adrian Moreno Date: Tue, 9 Feb 2021 10:03:17 +0100 Subject: [PATCH] conf: make a copy of global RuntimeConfig on merge When we call mergeRuntimeConfig, the global RuntimeConfig gets overwritten with the result of the merging, thus affecting the subsequent delegates. Do not modify the global RuntimeConfig and instead make a copy when merging it. Also, if a value has been provided for CNIDeviceInfoFile in the delegate's runtimeconfig, overwrite it to avoid possible name colissions. Signed-off-by: Adrian Moreno --- pkg/types/conf.go | 42 ++++++++++++++++++++++++------------------ pkg/types/conf_test.go | 12 +++++++++--- 2 files changed, 33 insertions(+), 21 deletions(-) diff --git a/pkg/types/conf.go b/pkg/types/conf.go index b0ca49176..b477ccafb 100644 --- a/pkg/types/conf.go +++ b/pkg/types/conf.go @@ -145,35 +145,38 @@ func LoadDelegateNetConf(bytes []byte, net *NetworkSelectionElement, deviceID st // mergeCNIRuntimeConfig creates CNI runtimeconfig from delegate func mergeCNIRuntimeConfig(runtimeConfig *RuntimeConfig, delegate *DelegateNetConf) *RuntimeConfig { logging.Debugf("mergeCNIRuntimeConfig: %v %v", runtimeConfig, delegate) + var mergedRuntimeConfig RuntimeConfig + if runtimeConfig == nil { - runtimeConfig = &RuntimeConfig{} + mergedRuntimeConfig = RuntimeConfig{} + } else { + mergedRuntimeConfig = *runtimeConfig } // multus inject RuntimeConfig only in case of non MasterPlugin. if delegate.MasterPlugin != true { - logging.Debugf("mergeCNIRuntimeConfig: add runtimeConfig for net-attach-def: %v", runtimeConfig) + logging.Debugf("mergeCNIRuntimeConfig: add runtimeConfig for net-attach-def: %v", mergedRuntimeConfig) if delegate.PortMappingsRequest != nil { - runtimeConfig.PortMaps = delegate.PortMappingsRequest + mergedRuntimeConfig.PortMaps = delegate.PortMappingsRequest } if delegate.BandwidthRequest != nil { - runtimeConfig.Bandwidth = delegate.BandwidthRequest + mergedRuntimeConfig.Bandwidth = delegate.BandwidthRequest } if delegate.IPRequest != nil { - runtimeConfig.IPs = delegate.IPRequest + mergedRuntimeConfig.IPs = delegate.IPRequest } if delegate.MacRequest != "" { - runtimeConfig.Mac = delegate.MacRequest + mergedRuntimeConfig.Mac = delegate.MacRequest } if delegate.InfinibandGUIDRequest != "" { - runtimeConfig.InfinibandGUID = delegate.InfinibandGUIDRequest + mergedRuntimeConfig.InfinibandGUID = delegate.InfinibandGUIDRequest } if delegate.DeviceID != "" { - runtimeConfig.DeviceID = delegate.DeviceID + mergedRuntimeConfig.DeviceID = delegate.DeviceID } - logging.Debugf("mergeCNIRuntimeConfig: add runtimeConfig for net-attach-def: %v", runtimeConfig) + logging.Debugf("mergeCNIRuntimeConfig: add mergedRuntimeConfig for net-attach-def: %v", mergedRuntimeConfig) } - - return runtimeConfig + return &mergedRuntimeConfig } // CreateCNIRuntimeConf create CNI RuntimeConf for a delegate. If delegate configuration @@ -181,16 +184,19 @@ func mergeCNIRuntimeConfig(runtimeConfig *RuntimeConfig, delegate *DelegateNetCo func CreateCNIRuntimeConf(args *skel.CmdArgs, k8sArgs *K8sArgs, ifName string, rc *RuntimeConfig, delegate *DelegateNetConf) (*libcni.RuntimeConf, string) { logging.Debugf("LoadCNIRuntimeConf: %v, %v, %s, %v %v", args, k8sArgs, ifName, rc, delegate) var cniDeviceInfoFile string + var delegateRc *RuntimeConfig - delegateRc := rc if delegate != nil { - delegateRc = mergeCNIRuntimeConfig(delegateRc, delegate) - if delegateRc.CNIDeviceInfoFile == "" && delegate.Name != "" { - autoDeviceInfo := fmt.Sprintf("%s-%s_%s", delegate.Name, args.ContainerID, ifName) - delegateRc.CNIDeviceInfoFile = nadutils.GetCNIDeviceInfoPath(autoDeviceInfo) - cniDeviceInfoFile = delegateRc.CNIDeviceInfoFile - logging.Debugf("Adding auto-generated CNIDeviceInfoFile: %s", delegateRc.CNIDeviceInfoFile) + delegateRc = mergeCNIRuntimeConfig(rc, delegate) + if delegateRc.CNIDeviceInfoFile != "" { + logging.Debugf("Warning: Existing value of CNIDeviceInfoFile will be overwritten %s", delegateRc.CNIDeviceInfoFile) } + autoDeviceInfo := fmt.Sprintf("%s-%s_%s", delegate.Name, args.ContainerID, ifName) + delegateRc.CNIDeviceInfoFile = nadutils.GetCNIDeviceInfoPath(autoDeviceInfo) + cniDeviceInfoFile = delegateRc.CNIDeviceInfoFile + logging.Debugf("Adding auto-generated CNIDeviceInfoFile: %s", delegateRc.CNIDeviceInfoFile) + } else { + delegateRc = rc } // In part, adapted from K8s pkg/kubelet/dockershim/network/cni/cni.go#buildCNIRuntimeConf diff --git a/pkg/types/conf_test.go b/pkg/types/conf_test.go index 58e156af8..3101d8d47 100644 --- a/pkg/types/conf_test.go +++ b/pkg/types/conf_test.go @@ -26,8 +26,8 @@ import ( types020 "github.com/containernetworking/cni/pkg/types/020" "github.com/containernetworking/plugins/pkg/ns" "github.com/containernetworking/plugins/pkg/testutils" - testhelpers "gopkg.in/intel/multus-cni.v3/pkg/testing" netutils "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/utils" + testhelpers "gopkg.in/intel/multus-cni.v3/pkg/testing" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -757,11 +757,14 @@ var _ = Describe("config operations", func() { } delegate, err := LoadDelegateNetConf([]byte(conf), networkSelection, "", "") delegate.MasterPlugin = true + origRuntimeConfig := RuntimeConfig{} Expect(err).NotTo(HaveOccurred()) - runtimeConf := mergeCNIRuntimeConfig(&RuntimeConfig{}, delegate) + runtimeConf := mergeCNIRuntimeConfig(&origRuntimeConfig, delegate) Expect(runtimeConf.PortMaps).To(BeNil()) Expect(runtimeConf.Bandwidth).To(BeNil()) Expect(runtimeConf.InfinibandGUID).To(Equal("")) + // The original RuntimeConfig must have not been overwritten + Expect(origRuntimeConfig).To(Equal(RuntimeConfig{})) }) It("test mergeCNIRuntimeConfig with delegate plugin", func() { @@ -792,9 +795,10 @@ var _ = Describe("config operations", func() { BandwidthRequest: bandwidthEntry1, PortMappingsRequest: []*PortMapEntry{portMapEntry1}, } + origRuntimeConfig := RuntimeConfig{} delegate, err := LoadDelegateNetConf([]byte(conf), networkSelection, "", "") Expect(err).NotTo(HaveOccurred()) - runtimeConf := mergeCNIRuntimeConfig(&RuntimeConfig{}, delegate) + runtimeConf := mergeCNIRuntimeConfig(&origRuntimeConfig, delegate) Expect(runtimeConf.PortMaps).NotTo(BeNil()) Expect(len(runtimeConf.PortMaps)).To(BeEquivalentTo(1)) Expect(runtimeConf.PortMaps[0]).To(Equal(portMapEntry1)) @@ -802,5 +806,7 @@ var _ = Describe("config operations", func() { Expect(len(runtimeConf.IPs)).To(BeEquivalentTo(1)) Expect(runtimeConf.Mac).To(Equal("c2:11:22:33:44:66")) Expect(runtimeConf.InfinibandGUID).To(Equal("24:8a:07:03:00:8d:ae:2e")) + // The original RuntimeConfig must have not been overwritten + Expect(origRuntimeConfig).To(Equal(RuntimeConfig{})) }) })