Skip to content

Commit

Permalink
Use util.CloneMap where appropriate
Browse files Browse the repository at this point in the history
Signed-off-by: montag451 <[email protected]>
  • Loading branch information
montag451 committed Nov 19, 2024
1 parent 7eeb255 commit ec48050
Show file tree
Hide file tree
Showing 11 changed files with 34 additions and 91 deletions.
25 changes: 8 additions & 17 deletions cmd/incusd/api_1.0.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"os"
"strings"

"github.com/lxc/incus/v6/client"
incus "github.com/lxc/incus/v6/client"
"github.com/lxc/incus/v6/internal/server/auth"
"github.com/lxc/incus/v6/internal/server/auth/oidc"
"github.com/lxc/incus/v6/internal/server/cluster"
Expand All @@ -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{
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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)
})
Expand Down
8 changes: 2 additions & 6 deletions cmd/incusd/api_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (

"github.com/gorilla/mux"

"github.com/lxc/incus/v6/client"
incus "github.com/lxc/incus/v6/client"
internalInstance "github.com/lxc/incus/v6/internal/instance"
"github.com/lxc/incus/v6/internal/server/auth"
"github.com/lxc/incus/v6/internal/server/certificate"
Expand Down Expand Up @@ -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 {
Expand Down
6 changes: 2 additions & 4 deletions cmd/incusd/networks.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"errors"
"fmt"
"maps"
"net"
"net/http"
"net/url"
Expand Down Expand Up @@ -716,10 +717,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] {
Expand Down
6 changes: 2 additions & 4 deletions cmd/incusd/storage_pools.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"encoding/json"
"fmt"
"maps"
"net/http"
"net/url"
"slices"
Expand Down Expand Up @@ -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] {
Expand Down
9 changes: 2 additions & 7 deletions internal/server/device/config/devices.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 2 additions & 6 deletions internal/server/operations/operations.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
26 changes: 6 additions & 20 deletions internal/server/storage/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"fmt"
"io"
"io/fs"
"maps"
"net/http"
"net/url"
"os"
Expand Down Expand Up @@ -4032,10 +4033,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,
Expand Down Expand Up @@ -5810,10 +5808,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)
Expand All @@ -5829,10 +5824,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)
Expand Down Expand Up @@ -6881,10 +6873,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
Expand All @@ -6906,10 +6895,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)
Expand Down
8 changes: 2 additions & 6 deletions internal/server/storage/drivers/driver_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package drivers
import (
"fmt"
"io"
"maps"
"net/url"
"os/exec"
"regexp"
Expand Down Expand Up @@ -240,12 +241,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.
Expand Down
11 changes: 3 additions & 8 deletions internal/server/storage/drivers/volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"errors"
"fmt"
"maps"
"os"
"slices"
"strings"
Expand Down Expand Up @@ -577,16 +578,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)
}
8 changes: 2 additions & 6 deletions internal/server/util/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package util

import (
"fmt"
"maps"

Check failure on line 5 in internal/server/util/config.go

View workflow job for this annotation

GitHub Actions / Code (stable)

"maps" imported and not used

Check failure on line 5 in internal/server/util/config.go

View workflow job for this annotation

GitHub Actions / Code (oldstable)

"maps" imported and not used

Check failure on line 5 in internal/server/util/config.go

View workflow job for this annotation

GitHub Actions / System (stable, standalone, ceph)

"maps" imported and not used
"slices"
"sort"
"strings"
Expand Down Expand Up @@ -52,10 +53,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)

Check failure on line 56 in internal/server/util/config.go

View workflow job for this annotation

GitHub Actions / Code (stable)

undefined: util

Check failure on line 56 in internal/server/util/config.go

View workflow job for this annotation

GitHub Actions / Code (oldstable)

undefined: util

Check failure on line 56 in internal/server/util/config.go

View workflow job for this annotation

GitHub Actions / System (stable, standalone, ceph)

undefined: util
}
10 changes: 3 additions & 7 deletions shared/cliconfig/default.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
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://",
Expand Down Expand Up @@ -28,14 +30,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",
}
Expand Down

0 comments on commit ec48050

Please sign in to comment.