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 <amorenoz@redhat.com>
k8snetworkplumbingwg · Feb 11, 2021 · 4cee6b3 · 4cee6b3
1 parent bd90c26
commit 4cee6b3
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 21 deletions.
diff --git a/pkg/types/conf.go b/pkg/types/conf.go
@@ -145,52 +145,58 @@ 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
 // exists, merge data with the runtime config.
 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
@@ -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,15 +795,18 @@ 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))
 		Expect(runtimeConf.Bandwidth).To(Equal(bandwidthEntry1))
 		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{}))
 	})
 })