Merge pull request #1397 from montag451/maps-clone

Add a function to clone map and use it where appropriate

Author: stgraber

cmd/incusd/api_1.0.go

@@ -29,6 +29,7 @@ import (
 	"github.com/lxc/incus/v6/shared/osarch"
 	"github.com/lxc/incus/v6/shared/revert"
 	localtls "github.com/lxc/incus/v6/shared/tls"
+	"github.com/lxc/incus/v6/shared/util"
 )
 
 var api10Cmd = APIEndpoint{
@@ -459,10 +460,7 @@ func api10Put(d *Daemon, r *http.Request) response.Response {
 	// for reacting to the values that changed.
 	if isClusterNotification(r) {
 		logger.Debug("Handling config changed notification")
-		changed := make(map[string]string)
-		for key, value := range req.Config {
-			changed[key] = value
-		}
+		changed := util.CloneMap(req.Config)
 
 		// Get the current (updated) config.
 		var config *clusterConfig.Config
@@ -587,7 +585,7 @@ func doApi10Update(d *Daemon, r *http.Request, req api.ServerPut, patch bool) re
 
 	nodeChanged := map[string]string{}
 	var newNodeConfig *node.Config
-	oldNodeConfig := make(map[string]string)
+	var oldNodeConfig map[string]string
 
 	err := s.DB.Node.Transaction(r.Context(), func(ctx context.Context, tx *db.NodeTx) error {
 		var err error
@@ -597,9 +595,7 @@ func doApi10Update(d *Daemon, r *http.Request, req api.ServerPut, patch bool) re
 		}
 
 		// Keep old config around in case something goes wrong. In that case the config will be reverted.
-		for k, v := range newNodeConfig.Dump() {
-			oldNodeConfig[k] = v
-		}
+		oldNodeConfig = util.CloneMap(newNodeConfig.Dump())
 
 		// We currently don't allow changing the cluster.https_address once it's set.
 		if s.ServerClustered {
@@ -687,7 +683,7 @@ func doApi10Update(d *Daemon, r *http.Request, req api.ServerPut, patch bool) re
 	// Then deal with cluster wide configuration
 	var clusterChanged map[string]string
 	var newClusterConfig *clusterConfig.Config
-	oldClusterConfig := make(map[string]string)
+	var oldClusterConfig map[string]string
 
 	err = s.DB.Cluster.Transaction(r.Context(), func(ctx context.Context, tx *db.ClusterTx) error {
 		var err error
@@ -697,9 +693,7 @@ func doApi10Update(d *Daemon, r *http.Request, req api.ServerPut, patch bool) re
 		}
 
 		// Keep old config around in case something goes wrong. In that case the config will be reverted.
-		for k, v := range newClusterConfig.Dump() {
-			oldClusterConfig[k] = v
-		}
+		oldClusterConfig = util.CloneMap(newClusterConfig.Dump())
 
 		if patch {
 			clusterChanged, err = newClusterConfig.Patch(req.Config)
@@ -760,11 +754,8 @@ func doApi10Update(d *Daemon, r *http.Request, req api.ServerPut, patch bool) re
 		}
 
 		serverPut := server.Writable()
-		serverPut.Config = make(map[string]string)
 		// Only propagated cluster-wide changes
-		for key, value := range clusterChanged {
-			serverPut.Config[key] = value
-		}
+		serverPut.Config = util.CloneMap(clusterChanged)
 
 		return client.UpdateServer(serverPut, etag)
 	})

cmd/incusd/api_cluster.go

@@ -759,11 +759,7 @@ func clusterPutJoin(d *Daemon, r *http.Request, req api.ClusterPut) response.Res
 		d.globalConfig = currentClusterConfig
 		d.globalConfigMu.Unlock()
 
-		existingConfigDump := currentClusterConfig.Dump()
-		changes := make(map[string]string, len(existingConfigDump))
-		for k, v := range existingConfigDump {
-			changes[k] = v
-		}
+		changes := util.CloneMap(currentClusterConfig.Dump())
 
 		err = doApi10UpdateTriggers(d, nil, changes, nodeConfig, currentClusterConfig)
 		if err != nil {

cmd/incusd/networks.go

@@ -716,10 +716,7 @@ func networksPostCluster(ctx context.Context, s *state.State, projectName string
 
 		// Clone the network config for this node so we don't modify it and potentially end up sending
 		// this node's config to another node.
-		nodeConfig := make(map[string]string, len(netConfig))
-		for k, v := range netConfig {
-			nodeConfig[k] = v
-		}
+		nodeConfig := util.CloneMap(netConfig)
 
 		// Merge node specific config items into global config.
 		for key, value := range nodeConfigs[server.Environment.ServerName] {

cmd/incusd/storage_pools.go

@@ -27,6 +27,7 @@ import (
 	"github.com/lxc/incus/v6/internal/version"
 	"github.com/lxc/incus/v6/shared/api"
 	"github.com/lxc/incus/v6/shared/logger"
+	"github.com/lxc/incus/v6/shared/util"
 )
 
 // Lock to prevent concurent storage pools creation.
@@ -554,10 +555,7 @@ func storagePoolsPostCluster(ctx context.Context, s *state.State, pool *api.Stor
 
 		// Clone fresh node config so we don't modify req.Config with this node's specific config which
 		// could result in it being sent to other nodes later.
-		nodeReq.Config = make(map[string]string, len(req.Config))
-		for k, v := range req.Config {
-			nodeReq.Config[k] = v
-		}
+		nodeReq.Config = util.CloneMap(req.Config)
 
 		// Merge node specific config items into global config.
 		for key, value := range configs[server.Environment.ServerName] {

internal/server/device/config/devices.go

@@ -6,20 +6,15 @@ import (
 	"strings"
 
 	"github.com/lxc/incus/v6/shared/api"
+	"github.com/lxc/incus/v6/shared/util"
 )
 
 // Device represents an instance device.
 type Device map[string]string
 
 // Clone returns a copy of the Device.
 func (device Device) Clone() Device {
-	copy := make(map[string]string, len(device))
-
-	for k, v := range device {
-		copy[k] = v
-	}
-
-	return copy
+	return util.CloneMap(device)
 }
 
 // Validate accepts a map of field/validation functions to run against the device's config.

internal/server/operations/operations.go

@@ -19,6 +19,7 @@ import (
 	"github.com/lxc/incus/v6/shared/api"
 	"github.com/lxc/incus/v6/shared/cancel"
 	"github.com/lxc/incus/v6/shared/logger"
+	"github.com/lxc/incus/v6/shared/util"
 )
 
 var debug bool
@@ -66,12 +67,7 @@ func Clone() map[string]*Operation {
 	operationsLock.Lock()
 	defer operationsLock.Unlock()
 
-	localOperations := make(map[string]*Operation, len(operations))
-	for k, v := range operations {
-		localOperations[k] = v
-	}
-
-	return localOperations
+	return util.CloneMap(operations)
 }
 
 // OperationGetInternal returns the operation with the given id. It returns an

internal/server/storage/backend.go

@@ -4032,10 +4032,7 @@ func (b *backend) ImportBucket(projectName string, poolVol *backupConfig.Config,
 	defer revert.Fail()
 
 	// Copy bucket config from backup file if present (so BucketDBCreate can safely modify the copy if needed).
-	bucketConfig := make(map[string]string, len(poolVol.Bucket.Config))
-	for k, v := range poolVol.Bucket.Config {
-		bucketConfig[k] = v
-	}
+	bucketConfig := util.CloneMap(poolVol.Bucket.Config)
 
 	bucket := &api.StorageBucketsPost{
 		Name:             poolVol.Bucket.Name,
@@ -5810,10 +5807,7 @@ func (b *backend) ImportCustomVolume(projectName string, poolVol *backupConfig.C
 	defer revert.Fail()
 
 	// Copy volume config from backup file if present (so VolumeDBCreate can safely modify the copy if needed).
-	volumeConfig := make(map[string]string, len(poolVol.Volume.Config))
-	for k, v := range poolVol.Volume.Config {
-		volumeConfig[k] = v
-	}
+	volumeConfig := util.CloneMap(poolVol.Volume.Config)
 
 	// Validate config and create database entry for restored storage volume.
 	err := VolumeDBCreate(b, projectName, poolVol.Volume.Name, poolVol.Volume.Description, drivers.VolumeTypeCustom, false, volumeConfig, poolVol.Volume.CreatedAt, time.Time{}, drivers.ContentType(poolVol.Volume.ContentType), false, true)
@@ -5829,10 +5823,7 @@ func (b *backend) ImportCustomVolume(projectName string, poolVol *backupConfig.C
 
 		// Copy volume config from backup file if present
 		// (so VolumeDBCreate can safely modify the copy if needed).
-		snapVolumeConfig := make(map[string]string, len(poolVolSnap.Config))
-		for k, v := range poolVolSnap.Config {
-			snapVolumeConfig[k] = v
-		}
+		snapVolumeConfig := util.CloneMap(poolVolSnap.Config)
 
 		// Validate config and create database entry for restored storage volume.
 		err = VolumeDBCreate(b, projectName, fullSnapName, poolVolSnap.Description, drivers.VolumeTypeCustom, true, snapVolumeConfig, poolVolSnap.CreatedAt, time.Time{}, drivers.ContentType(poolVolSnap.ContentType), false, true)
@@ -6881,10 +6872,7 @@ func (b *backend) ImportInstance(inst instance.Instance, poolVol *backupConfig.C
 		// Copy volume config from backup file config if present,
 		// so VolumeDBCreate can safely modify the copy if needed.
 		if poolVol.Volume != nil {
-			volumeConfig = make(map[string]string, len(poolVol.Volume.Config))
-			for k, v := range poolVol.Volume.Config {
-				volumeConfig[k] = v
-			}
+			volumeConfig = util.CloneMap(poolVol.Volume.Config)
 
 			if !poolVol.Volume.CreatedAt.IsZero() {
 				creationDate = poolVol.Volume.CreatedAt
@@ -6906,10 +6894,7 @@ func (b *backend) ImportInstance(inst instance.Instance, poolVol *backupConfig.C
 
 				// Copy volume config from backup file if present,
 				// so VolumeDBCreate can safely modify the copy if needed.
-				snapVolumeConfig := make(map[string]string, len(poolVolSnap.Config))
-				for k, v := range poolVolSnap.Config {
-					snapVolumeConfig[k] = v
-				}
+				snapVolumeConfig := util.CloneMap(poolVolSnap.Config)
 
 				// Validate config and create database entry for recovered storage volume.
 				err = VolumeDBCreate(b, inst.Project().Name, fullSnapName, poolVolSnap.Description, volType, true, snapVolumeConfig, poolVolSnap.CreatedAt, time.Time{}, contentType, false, true)

internal/server/storage/drivers/driver_common.go

@@ -240,12 +240,7 @@ func (d *common) Logger() logger.Logger {
 
 // Config returns the storage pool config (as a copy, so not modifiable).
 func (d *common) Config() map[string]string {
-	confCopy := make(map[string]string, len(d.config))
-	for k, v := range d.config {
-		confCopy[k] = v
-	}
-
-	return confCopy
+	return util.CloneMap(d.config)
 }
 
 // ApplyPatch looks for a suitable patch and runs it.

internal/server/storage/drivers/volume.go

@@ -577,16 +577,10 @@ func (v *Volume) SetHasSource(hasSource bool) {
 // Clone returns a copy of the volume.
 func (v Volume) Clone() Volume {
 	// Copy the config map to avoid internal modifications affecting external state.
-	newConfig := make(map[string]string, len(v.config))
-	for k, v := range v.config {
-		newConfig[k] = v
-	}
+	newConfig := util.CloneMap(v.config)
 
 	// Copy the pool config map to avoid internal modifications affecting external state.
-	newPoolConfig := make(map[string]string, len(v.poolConfig))
-	for k, v := range v.poolConfig {
-		newPoolConfig[k] = v
-	}
+	newPoolConfig := util.CloneMap(v.poolConfig)
 
 	return NewVolume(v.driver, v.pool, v.volType, v.contentType, v.name, newConfig, newPoolConfig)
 }

internal/server/util/config.go

@@ -5,6 +5,8 @@ import (
 	"slices"
 	"sort"
 	"strings"
+
+	"github.com/lxc/incus/v6/shared/util"
 )
 
 // CompareConfigs compares two config maps and returns an error if they differ.
@@ -52,10 +54,5 @@ func CompareConfigs(config1, config2 map[string]string, exclude []string) error
 
 // CopyConfig creates a new map with a copy of the given config.
 func CopyConfig(config map[string]string) map[string]string {
-	copy := make(map[string]string, len(config))
-	for key, value := range config {
-		copy[key] = value
-	}
-
-	return copy
+	return util.CloneMap(config)
 }

shared/cliconfig/default.go

@@ -1,5 +1,9 @@
 package cliconfig
 
+import (
+	"github.com/lxc/incus/v6/shared/util"
+)
+
 // LocalRemote is the default local remote (over the unix socket).
 var LocalRemote = Remote{
 	Addr:     "unix://",
@@ -28,14 +32,8 @@ var DefaultRemotes = map[string]Remote{
 
 // DefaultConfig returns the default configuration.
 func DefaultConfig() *Config {
-	// Duplicate remotes from DefaultRemotes.
-	defaultRoutes := make(map[string]Remote, len(DefaultRemotes))
-	for k, v := range DefaultRemotes {
-		defaultRoutes[k] = v
-	}
-
 	return &Config{
-		Remotes:       defaultRoutes,
+		Remotes:       util.CloneMap(DefaultRemotes),
 		Aliases:       make(map[string]string),
 		DefaultRemote: "local",
 	}

shared/util/map.go

@@ -0,0 +1,15 @@
+package util
+
+import (
+	"maps"
+)
+
+// CloneMap returns a copy of m. This is a shallow clone: the new keys
+// and values are set using ordinary assignment.
+func CloneMap[M ~map[K]V, K comparable, V any](m M) M {
+	if m == nil {
+		return make(map[K]V)
+	}
+
+	return maps.Clone(m)
+}