From a99220e9fb170935026edc5449fce8d0a388abc8 Mon Sep 17 00:00:00 2001 From: Pavel Boldyrev <627562+bpg@users.noreply.github.com> Date: Tue, 29 Apr 2025 21:15:20 -0400 Subject: [PATCH] feat(lxc): increase number of supported mount points to 256 (#1939) * feat(lxc): increase number of supported mount points to 256 * fix(container): correct condition for setting replicate value for rootfs Signed-off-by: Pavel Boldyrev <627562+bpg@users.noreply.github.com> --- fwprovider/test/resource_container_test.go | 10 +- proxmox/nodes/containers/containers_types.go | 248 +++++++++++-------- proxmox/nodes/vms/vms_types.go | 19 +- proxmoxtf/resource/container/container.go | 223 +++++++---------- utils/maps.go | 57 ++++- utils/maps_test.go | 46 ++++ 6 files changed, 357 insertions(+), 246 deletions(-) diff --git a/fwprovider/test/resource_container_test.go b/fwprovider/test/resource_container_test.go index f54e71bf..9b6171f2 100644 --- a/fwprovider/test/resource_container_test.go +++ b/fwprovider/test/resource_container_test.go @@ -120,7 +120,10 @@ func TestAccResourceContainer(t *testing.T) { ctInfo, err := ct.GetContainer(ctx) require.NoError(te.t, err, "failed to get container") - require.NotNil(te.t, ctInfo.DevicePassthrough0) + dev0, ok := ctInfo.PassthroughDevices["dev0"] + require.True(te.t, ok, `"dev0" passthrough device not found`) + require.NotNil(te.t, dev0, `"dev0" passthrough device is `) + require.Equal(te.t, "/dev/zero", dev0.Path) return nil }, @@ -309,7 +312,10 @@ func TestAccResourceContainer(t *testing.T) { ctInfo, err := ct.GetContainer(ctx) require.NoError(te.t, err, "failed to get container") - require.NotNil(te.t, ctInfo.DevicePassthrough0) + dev0, ok := ctInfo.PassthroughDevices["dev0"] + require.True(te.t, ok, `"dev0" passthrough device not found`) + require.NotNil(te.t, dev0, `"dev0" passthrough device is `) + require.Equal(te.t, "/dev/zero", dev0.Path) return nil }, diff --git a/proxmox/nodes/containers/containers_types.go b/proxmox/nodes/containers/containers_types.go index 34e8187f..da13cd53 100644 --- a/proxmox/nodes/containers/containers_types.go +++ b/proxmox/nodes/containers/containers_types.go @@ -10,12 +10,22 @@ import ( "encoding/json" "fmt" "net/url" + "regexp" "strconv" "strings" "github.com/bpg/terraform-provider-proxmox/proxmox/types" ) +var ( + // regexDeviceKey matches device keys like dev0, dev1, etc. + regexDeviceKey = regexp.MustCompile(`^dev\d+$`) + // regexNetworkKey matches network interface keys like net0, net1, etc. + regexNetworkKey = regexp.MustCompile(`^net\d+$`) + // regexMountPointKey matches mount point keys like mp0, mp1, etc. + regexMountPointKey = regexp.MustCompile(`^mp\d+$`) +) + // CloneRequestBody contains the data for an container clone request. type CloneRequestBody struct { BandwidthLimit *int `json:"bwlimit,omitempty" url:"bwlimit,omitempty"` @@ -31,46 +41,46 @@ type CloneRequestBody struct { // CreateRequestBody contains the data for a user create request. type CreateRequestBody struct { - BandwidthLimit *float64 `json:"bwlimit,omitempty" url:"bwlimit,omitempty"` - ConsoleEnabled *types.CustomBool `json:"console,omitempty" url:"console,omitempty,int"` - ConsoleMode *string `json:"cmode,omitempty" url:"cmode,omitempty"` - CPUArchitecture *string `json:"arch,omitempty" url:"arch,omitempty"` - CPUCores *int `json:"cores,omitempty" url:"cores,omitempty"` - CPULimit *int `json:"cpulimit,omitempty" url:"cpulimit,omitempty"` - CPUUnits *int `json:"cpuunits,omitempty" url:"cpuunits,omitempty"` - DatastoreID *string `json:"storage,omitempty" url:"storage,omitempty"` - DedicatedMemory *int `json:"memory,omitempty" url:"memory,omitempty"` - Delete []string `json:"delete,omitempty" url:"delete,omitempty"` - Description *string `json:"description,omitempty" url:"description,omitempty"` - DNSDomain *string `json:"searchdomain,omitempty" url:"searchdomain,omitempty"` - DNSServer *string `json:"nameserver,omitempty" url:"nameserver,omitempty"` - Features *CustomFeatures `json:"features,omitempty" url:"features,omitempty"` - Force *types.CustomBool `json:"force,omitempty" url:"force,omitempty,int"` - HookScript *string `json:"hookscript,omitempty" url:"hookscript,omitempty"` - Hostname *string `json:"hostname,omitempty" url:"hostname,omitempty"` - IgnoreUnpackErrors *types.CustomBool `json:"ignore-unpack-errors,omitempty" url:"force,omitempty,int"` - Lock *string `json:"lock,omitempty" url:"lock,omitempty,int"` - MountPoints CustomMountPointArray `json:"mp,omitempty" url:"mp,omitempty,numbered"` - DevicePassthrough CustomDevicePassthroughArray `json:"dev,omitempty" url:"dev,omitempty,numbered"` - NetworkInterfaces CustomNetworkInterfaceArray `json:"net,omitempty" url:"net,omitempty,numbered"` - OSTemplateFileVolume *string `json:"ostemplate,omitempty" url:"ostemplate,omitempty"` - OSType *string `json:"ostype,omitempty" url:"ostype,omitempty"` - Password *string `json:"password,omitempty" url:"password,omitempty"` - PoolID *string `json:"pool,omitempty" url:"pool,omitempty"` - Protection *types.CustomBool `json:"protection,omitempty" url:"protection,omitempty,int"` - Restore *types.CustomBool `json:"restore,omitempty" url:"restore,omitempty,int"` - RootFS *CustomRootFS `json:"rootfs,omitempty" url:"rootfs,omitempty"` - SSHKeys *CustomSSHKeys `json:"ssh-public-keys,omitempty" url:"ssh-public-keys,omitempty"` - Start *types.CustomBool `json:"start,omitempty" url:"start,omitempty,int"` - StartOnBoot *types.CustomBool `json:"onboot,omitempty" url:"onboot,omitempty,int"` - StartupBehavior *CustomStartupBehavior `json:"startup,omitempty" url:"startup,omitempty"` - Swap *int `json:"swap,omitempty" url:"swap,omitempty"` - Tags *string `json:"tags,omitempty" url:"tags,omitempty"` - Template *types.CustomBool `json:"template,omitempty" url:"template,omitempty,int"` - TTY *int `json:"tty,omitempty" url:"tty,omitempty"` - Unique *types.CustomBool `json:"unique,omitempty" url:"unique,omitempty,int"` - Unprivileged *types.CustomBool `json:"unprivileged,omitempty" url:"unprivileged,omitempty,int"` - VMID *int `json:"vmid,omitempty" url:"vmid,omitempty"` + BandwidthLimit *float64 `json:"bwlimit,omitempty" url:"bwlimit,omitempty"` + ConsoleEnabled *types.CustomBool `json:"console,omitempty" url:"console,omitempty,int"` + ConsoleMode *string `json:"cmode,omitempty" url:"cmode,omitempty"` + CPUArchitecture *string `json:"arch,omitempty" url:"arch,omitempty"` + CPUCores *int `json:"cores,omitempty" url:"cores,omitempty"` + CPULimit *int `json:"cpulimit,omitempty" url:"cpulimit,omitempty"` + CPUUnits *int `json:"cpuunits,omitempty" url:"cpuunits,omitempty"` + DatastoreID *string `json:"storage,omitempty" url:"storage,omitempty"` + DedicatedMemory *int `json:"memory,omitempty" url:"memory,omitempty"` + Delete []string `json:"delete,omitempty" url:"delete,omitempty"` + Description *string `json:"description,omitempty" url:"description,omitempty"` + DNSDomain *string `json:"searchdomain,omitempty" url:"searchdomain,omitempty"` + DNSServer *string `json:"nameserver,omitempty" url:"nameserver,omitempty"` + Features *CustomFeatures `json:"features,omitempty" url:"features,omitempty"` + Force *types.CustomBool `json:"force,omitempty" url:"force,omitempty,int"` + HookScript *string `json:"hookscript,omitempty" url:"hookscript,omitempty"` + Hostname *string `json:"hostname,omitempty" url:"hostname,omitempty"` + IgnoreUnpackErrors *types.CustomBool `json:"ignore-unpack-errors,omitempty" url:"force,omitempty,int"` + Lock *string `json:"lock,omitempty" url:"lock,omitempty,int"` + MountPoints CustomMountPoints `json:"mp,omitempty" url:"mp,omitempty,numbered"` + PassthroughDevices CustomPassthroughDevices `json:"dev,omitempty" url:"dev,omitempty,numbered"` + NetworkInterfaces CustomNetworkInterfaces `json:"net,omitempty" url:"net,omitempty,numbered"` + OSTemplateFileVolume *string `json:"ostemplate,omitempty" url:"ostemplate,omitempty"` + OSType *string `json:"ostype,omitempty" url:"ostype,omitempty"` + Password *string `json:"password,omitempty" url:"password,omitempty"` + PoolID *string `json:"pool,omitempty" url:"pool,omitempty"` + Protection *types.CustomBool `json:"protection,omitempty" url:"protection,omitempty,int"` + Restore *types.CustomBool `json:"restore,omitempty" url:"restore,omitempty,int"` + RootFS *CustomRootFS `json:"rootfs,omitempty" url:"rootfs,omitempty"` + SSHKeys *CustomSSHKeys `json:"ssh-public-keys,omitempty" url:"ssh-public-keys,omitempty"` + Start *types.CustomBool `json:"start,omitempty" url:"start,omitempty,int"` + StartOnBoot *types.CustomBool `json:"onboot,omitempty" url:"onboot,omitempty,int"` + StartupBehavior *CustomStartupBehavior `json:"startup,omitempty" url:"startup,omitempty"` + Swap *int `json:"swap,omitempty" url:"swap,omitempty"` + Tags *string `json:"tags,omitempty" url:"tags,omitempty"` + Template *types.CustomBool `json:"template,omitempty" url:"template,omitempty,int"` + TTY *int `json:"tty,omitempty" url:"tty,omitempty"` + Unique *types.CustomBool `json:"unique,omitempty" url:"unique,omitempty,int"` + Unprivileged *types.CustomBool `json:"unprivileged,omitempty" url:"unprivileged,omitempty,int"` + VMID *int `json:"vmid,omitempty" url:"vmid,omitempty"` } // CustomFeatures contains the values for the "features" property. @@ -96,8 +106,8 @@ type CustomMountPoint struct { Volume string `json:"volume" url:"volume"` } -// CustomMountPointArray is an array of CustomMountPoint. -type CustomMountPointArray []CustomMountPoint +// CustomMountPoints is a map of CustomMountPoint per mount point ID (i.e. `mp1`). +type CustomMountPoints map[string]*CustomMountPoint // CustomNetworkInterface contains the values for the "net[n]" properties. type CustomNetworkInterface struct { @@ -117,11 +127,11 @@ type CustomNetworkInterface struct { Type *string `json:"type,omitempty" url:"type,omitempty"` } -// CustomDevicePassthroughArray is an array of CustomDevicePassthrough. -type CustomDevicePassthroughArray []CustomDevicePassthrough +// CustomPassthroughDevices is a map of CustomPassthroughDevice per passthrough device ID (i.e. `dev0`). +type CustomPassthroughDevices map[string]*CustomPassthroughDevice -// CustomDevicePassthrough contains the values for the "dev[n]" properties. -type CustomDevicePassthrough struct { +// CustomPassthroughDevice contains the values for the "dev[n]" properties. +type CustomPassthroughDevice struct { DenyWrite *types.CustomBool `json:"deny-write,omitempty" url:"deny-write,omitempty,int"` Path string `json:"path" url:"path"` UID *int `json:"uid,omitempty" url:"uid,omitempty"` @@ -129,8 +139,8 @@ type CustomDevicePassthrough struct { Mode *string `json:"mode,omitempty" url:"mode,omitempty"` } -// CustomNetworkInterfaceArray is an array of CustomNetworkInterface. -type CustomNetworkInterfaceArray []CustomNetworkInterface +// CustomNetworkInterfaces is a map of CustomNetworkInterface per network interface ID (i.e. `net0`). +type CustomNetworkInterfaces map[string]*CustomNetworkInterface // CustomRootFS contains the values for the "rootfs" property. type CustomRootFS struct { @@ -184,30 +194,9 @@ type GetResponseData struct { Hostname *string `json:"hostname,omitempty"` Lock *types.CustomBool `json:"lock,omitempty"` LXCConfiguration *[][2]string `json:"lxc,omitempty"` - DevicePassthrough0 *CustomDevicePassthrough `json:"dev0,omitempty"` - DevicePassthrough1 *CustomDevicePassthrough `json:"dev1,omitempty"` - DevicePassthrough2 *CustomDevicePassthrough `json:"dev2,omitempty"` - DevicePassthrough3 *CustomDevicePassthrough `json:"dev3,omitempty"` - DevicePassthrough4 *CustomDevicePassthrough `json:"dev4,omitempty"` - DevicePassthrough5 *CustomDevicePassthrough `json:"dev5,omitempty"` - DevicePassthrough6 *CustomDevicePassthrough `json:"dev6,omitempty"` - DevicePassthrough7 *CustomDevicePassthrough `json:"dev7,omitempty"` - MountPoint0 *CustomMountPoint `json:"mp0,omitempty"` - MountPoint1 *CustomMountPoint `json:"mp1,omitempty"` - MountPoint2 *CustomMountPoint `json:"mp2,omitempty"` - MountPoint3 *CustomMountPoint `json:"mp3,omitempty"` - MountPoint4 *CustomMountPoint `json:"mp4,omitempty"` - MountPoint5 *CustomMountPoint `json:"mp5,omitempty"` - MountPoint6 *CustomMountPoint `json:"mp6,omitempty"` - MountPoint7 *CustomMountPoint `json:"mp7,omitempty"` - NetworkInterface0 *CustomNetworkInterface `json:"net0,omitempty"` - NetworkInterface1 *CustomNetworkInterface `json:"net1,omitempty"` - NetworkInterface2 *CustomNetworkInterface `json:"net2,omitempty"` - NetworkInterface3 *CustomNetworkInterface `json:"net3,omitempty"` - NetworkInterface4 *CustomNetworkInterface `json:"net4,omitempty"` - NetworkInterface5 *CustomNetworkInterface `json:"net5,omitempty"` - NetworkInterface6 *CustomNetworkInterface `json:"net6,omitempty"` - NetworkInterface7 *CustomNetworkInterface `json:"net7,omitempty"` + MountPoints CustomMountPoints `json:"mp,omitempty"` + PassthroughDevices CustomPassthroughDevices `json:"dev,omitempty"` + NetworkInterfaces CustomNetworkInterfaces `json:"net,omitempty"` OSType *string `json:"ostype,omitempty"` Protection *types.CustomBool `json:"protection,omitempty"` RootFS *CustomRootFS `json:"rootfs,omitempty"` @@ -299,8 +288,8 @@ func (r *CustomFeatures) EncodeValues(key string, v *url.Values) error { return nil } -// EncodeValues converts a CustomDevicePassthrough struct to a URL value. -func (r *CustomDevicePassthrough) EncodeValues(key string, v *url.Values) error { +// EncodeValues converts a CustomPassthroughDevice struct to a URL value. +func (r *CustomPassthroughDevice) EncodeValues(key string, v *url.Values) error { var values []string if r.DenyWrite != nil { @@ -334,14 +323,14 @@ func (r *CustomDevicePassthrough) EncodeValues(key string, v *url.Values) error return nil } -// EncodeValues converts a CustomDevicePassthroughArray array to multiple URL values. -func (r CustomDevicePassthroughArray) EncodeValues( - key string, +// EncodeValues converts a CustomPassthroughDevices array to multiple URL values. +func (r CustomPassthroughDevices) EncodeValues( + _ string, v *url.Values, ) error { - for i, d := range r { - if err := d.EncodeValues(fmt.Sprintf("%s%d", key, i), v); err != nil { - return fmt.Errorf("failed to encode CustomDevicePassthroughArray: %w", err) + for key, d := range r { + if err := d.EncodeValues(key, v); err != nil { + return fmt.Errorf("failed to encode CustomPassthroughDevices: %w", err) } } @@ -354,7 +343,7 @@ func (r *CustomMountPoint) EncodeValues(key string, v *url.Values) error { if r.ACL != nil { if *r.ACL { - values = append(values, "acl=%d") + values = append(values, "acl=1") } else { values = append(values, "acl=0") } @@ -421,14 +410,14 @@ func (r *CustomMountPoint) EncodeValues(key string, v *url.Values) error { return nil } -// EncodeValues converts a CustomMountPointArray array to multiple URL values. -func (r CustomMountPointArray) EncodeValues( - key string, +// EncodeValues converts a CustomMountPoints array to multiple URL values. +func (r CustomMountPoints) EncodeValues( + _ string, v *url.Values, ) error { - for i, d := range r { - if err := d.EncodeValues(fmt.Sprintf("%s%d", key, i), v); err != nil { - return fmt.Errorf("failed to encode CustomMountPointArray: %w", err) + for key, d := range r { + if err := d.EncodeValues(key, v); err != nil { + return fmt.Errorf("failed to encode CustomMountPoints: %w", err) } } @@ -509,14 +498,14 @@ func (r *CustomNetworkInterface) EncodeValues( return nil } -// EncodeValues converts a CustomNetworkInterfaceArray array to multiple URL values. -func (r CustomNetworkInterfaceArray) EncodeValues( - key string, +// EncodeValues converts a CustomNetworkInterfaces array to multiple URL values. +func (r CustomNetworkInterfaces) EncodeValues( + _ string, v *url.Values, ) error { - for i, d := range r { - if err := d.EncodeValues(fmt.Sprintf("%s%d", key, i), v); err != nil { - return fmt.Errorf("failed to encode CustomNetworkInterfaceArray: %w", err) + for key, d := range r { + if err := d.EncodeValues(key, v); err != nil { + return fmt.Errorf("failed to encode CustomNetworkInterfaces: %w", err) } } @@ -529,7 +518,7 @@ func (r *CustomRootFS) EncodeValues(key string, v *url.Values) error { if r.ACL != nil { if *r.ACL { - values = append(values, "acl=%d") + values = append(values, "acl=1") } else { values = append(values, "acl=0") } @@ -562,7 +551,7 @@ func (r *CustomRootFS) EncodeValues(key string, v *url.Values) error { } if r.Replicate != nil { - if *r.ReadOnly { + if *r.Replicate { values = append(values, "replicate=1") } else { values = append(values, "replicate=0") @@ -659,13 +648,13 @@ func (r *CustomFeatures) UnmarshalJSON(b []byte) error { return nil } -// UnmarshalJSON converts a CustomDevicePassthrough string to an object. -func (r *CustomDevicePassthrough) UnmarshalJSON(b []byte) error { +// UnmarshalJSON converts a CustomPassthroughDevice string to an object. +func (r *CustomPassthroughDevice) UnmarshalJSON(b []byte) error { var s string err := json.Unmarshal(b, &s) if err != nil { - return fmt.Errorf("unable to unmarshal CustomDevicePassthrough: %w", err) + return fmt.Errorf("unable to unmarshal CustomPassthroughDevice: %w", err) } pairs := strings.Split(s, ",") @@ -947,3 +936,66 @@ func (r *CustomStartupBehavior) UnmarshalJSON(b []byte) error { return nil } + +// UnmarshalJSON unmarshals the data from the JSON response, populating the CustomStorageDevices field. +func (d *GetResponseData) UnmarshalJSON(b []byte) error { + type Alias GetResponseData + + var data Alias + + // get original struct + if err := json.Unmarshal(b, &data); err != nil { + return fmt.Errorf("failed to unmarshal data: %w", err) + } + + var byAttr map[string]interface{} + + // now get map by attribute name + err := json.Unmarshal(b, &byAttr) + if err != nil { + return fmt.Errorf("failed to unmarshal data: %w", err) + } + + data.PassthroughDevices = make(CustomPassthroughDevices) + data.NetworkInterfaces = make(CustomNetworkInterfaces) + data.MountPoints = make(CustomMountPoints) + + for key, value := range byAttr { + valueStr, ok := value.(string) + if !ok { + continue // Skip non-string values + } + + jsonValue := []byte(`"` + valueStr + `"`) + + switch { + case regexDeviceKey.MatchString(key): + var device CustomPassthroughDevice + if e := json.Unmarshal(jsonValue, &device); e != nil { + return fmt.Errorf("failed to unmarshal %s with value %q: %w", key, valueStr, e) + } + + data.PassthroughDevices[key] = &device + + case regexNetworkKey.MatchString(key): + var net CustomNetworkInterface + if e := json.Unmarshal(jsonValue, &net); e != nil { + return fmt.Errorf("failed to unmarshal %s with value %q: %w", key, valueStr, e) + } + + data.NetworkInterfaces[key] = &net + + case regexMountPointKey.MatchString(key): + var mp CustomMountPoint + if e := json.Unmarshal(jsonValue, &mp); e != nil { + return fmt.Errorf("failed to unmarshal %s with value %q: %w", key, valueStr, e) + } + + data.MountPoints[key] = &mp + } + } + + *d = GetResponseData(data) + + return nil +} diff --git a/proxmox/nodes/vms/vms_types.go b/proxmox/nodes/vms/vms_types.go index 8a0b4fe2..27dc5b87 100644 --- a/proxmox/nodes/vms/vms_types.go +++ b/proxmox/nodes/vms/vms_types.go @@ -17,6 +17,18 @@ import ( "github.com/bpg/terraform-provider-proxmox/proxmox/types" ) +var ( + //nolint:gochecknoglobals + // regexStorageInterface is a regex pattern for matching storage interface names. + regexStorageInterface = func(prefix string) *regexp.Regexp { + return regexp.MustCompile(`^` + prefix + `\d+$`) + } + // regexPCIDevice is a regex pattern for matching PCI device names. + regexPCIDevice = regexp.MustCompile(`^hostpci\d+$`) + // regexVirtiofsShare is a regex pattern for matching virtiofs share names. + regexVirtiofsShare = regexp.MustCompile(`^virtiofs\d+$`) +) + // CloneRequestBody contains the data for an virtual machine clone request. type CloneRequestBody struct { BandwidthLimit *int `json:"bwlimit,omitempty" url:"bwlimit,omitempty"` @@ -477,8 +489,7 @@ func (d *GetResponseData) UnmarshalJSON(b []byte) error { for _, prefix := range StorageInterfaces { // the device names can overlap with other fields, for example`scsi0` and `scsihw`, so just checking // the prefix is not enough - r := regexp.MustCompile(`^` + prefix + `\d+$`) - if r.MatchString(key) { + if regexStorageInterface(prefix).MatchString(key) { var device CustomStorageDevice if err := json.Unmarshal([]byte(`"`+value.(string)+`"`), &device); err != nil { return fmt.Errorf("failed to unmarshal %s: %w", key, err) @@ -488,7 +499,7 @@ func (d *GetResponseData) UnmarshalJSON(b []byte) error { } } - if r := regexp.MustCompile(`^hostpci\d+$`); r.MatchString(key) { + if regexPCIDevice.MatchString(key) { var device CustomPCIDevice if err := json.Unmarshal([]byte(`"`+value.(string)+`"`), &device); err != nil { return fmt.Errorf("failed to unmarshal %s: %w", key, err) @@ -497,7 +508,7 @@ func (d *GetResponseData) UnmarshalJSON(b []byte) error { data.PCIDevices[key] = &device } - if r := regexp.MustCompile(`^virtiofs\d+$`); r.MatchString(key) { + if regexVirtiofsShare.MatchString(key) { var share CustomVirtiofsShare if err := json.Unmarshal([]byte(`"`+value.(string)+`"`), &share); err != nil { return fmt.Errorf("failed to unmarshal %s: %w", key, err) diff --git a/proxmoxtf/resource/container/container.go b/proxmoxtf/resource/container/container.go index 8bdc0cd7..997dbf85 100644 --- a/proxmoxtf/resource/container/container.go +++ b/proxmoxtf/resource/container/container.go @@ -89,7 +89,9 @@ const ( dvTimeoutDelete = 60 dvUnprivileged = false - maxResourceVirtualEnvironmentContainerNetworkInterfaces = 8 + maxNetworkInterfaces = 10 + maxPassthroughDevices = 8 + maxMountPoints = 256 mkClone = "clone" mkCloneDatastoreID = "datastore_id" @@ -686,7 +688,7 @@ func Container() *schema.Resource { }, }, }, - MaxItems: 8, + MaxItems: maxMountPoints, MinItems: 0, }, mkDevicePassthrough: { @@ -730,7 +732,7 @@ func Container() *schema.Resource { }, }, }, - MaxItems: 8, + MaxItems: maxPassthroughDevices, MinItems: 0, }, mkNetworkInterface: { @@ -795,7 +797,7 @@ func Container() *schema.Resource { }, }, }, - MaxItems: maxResourceVirtualEnvironmentContainerNetworkInterfaces, + MaxItems: maxNetworkInterfaces, MinItems: 0, }, mkNodeName: { @@ -1264,14 +1266,14 @@ func containerCreateClone(ctx context.Context, d *schema.ResourceData, m interfa devicePassthrough := d.Get(mkDevicePassthrough).([]interface{}) - devicePassthroughArray := make( - containers.CustomDevicePassthroughArray, + passthroughDevices := make( + containers.CustomPassthroughDevices, len(devicePassthrough), ) for di, dv := range devicePassthrough { devicePassthroughMap := dv.(map[string]interface{}) - devicePassthroughObject := containers.CustomDevicePassthrough{} + devicePassthroughObject := containers.CustomPassthroughDevice{} denyWrite := types.CustomBool( devicePassthroughMap[mkDevicePassthroughDenyWrite].(bool), @@ -1287,10 +1289,10 @@ func containerCreateClone(ctx context.Context, d *schema.ResourceData, m interfa devicePassthroughObject.Path = path devicePassthroughObject.UID = &uid - devicePassthroughArray[di] = devicePassthroughObject + passthroughDevices[fmt.Sprintf("dev%d", di)] = &devicePassthroughObject } - updateBody.DevicePassthrough = devicePassthroughArray + updateBody.PassthroughDevices = passthroughDevices networkInterface := d.Get(mkNetworkInterface).([]interface{}) @@ -1301,8 +1303,8 @@ func containerCreateClone(ctx context.Context, d *schema.ResourceData, m interfa } } - networkInterfaceArray := make( - containers.CustomNetworkInterfaceArray, + networkInterfaces := make( + containers.CustomNetworkInterfaces, len(networkInterface), ) @@ -1364,18 +1366,18 @@ func containerCreateClone(ctx context.Context, d *schema.ResourceData, m interfa networkInterfaceObject.MTU = &mtu } - networkInterfaceArray[ni] = networkInterfaceObject + networkInterfaces[fmt.Sprintf("net%d", ni)] = &networkInterfaceObject } - updateBody.NetworkInterfaces = networkInterfaceArray + updateBody.NetworkInterfaces = networkInterfaces - for i, ni := range updateBody.NetworkInterfaces { + for key, ni := range updateBody.NetworkInterfaces { if !ni.Enabled { - updateBody.Delete = append(updateBody.Delete, fmt.Sprintf("net%d", i)) + updateBody.Delete = append(updateBody.Delete, key) } } - for i := len(updateBody.NetworkInterfaces); i < maxResourceVirtualEnvironmentContainerNetworkInterfaces; i++ { + for i := len(updateBody.NetworkInterfaces); i < maxNetworkInterfaces; i++ { updateBody.Delete = append(updateBody.Delete, fmt.Sprintf("net%d", i)) } @@ -1604,11 +1606,11 @@ func containerCreateCustom(ctx context.Context, d *schema.ResourceData, m interf memorySwap := memoryBlock[mkMemorySwap].(int) mountPoint := d.Get(mkMountPoint).([]interface{}) - mountPointArray := make(containers.CustomMountPointArray, 0, len(mountPoint)) + mountPoints := make(containers.CustomMountPoints, len(mountPoint)) // because of default bool values: - for _, mp := range mountPoint { + for mi, mp := range mountPoint { mountPointMap := mp.(map[string]interface{}) mountPointObject := containers.CustomMountPoint{} @@ -1675,13 +1677,13 @@ func containerCreateCustom(ctx context.Context, d *schema.ResourceData, m interf mountPointObject.MountOptions = &mountOptionsArray } - mountPointArray = append(mountPointArray, mountPointObject) + mountPoints[fmt.Sprintf("mp%d", mi)] = &mountPointObject } var rootFS *containers.CustomRootFS diskSize := diskBlock[mkDiskSize].(int) - if diskDatastoreID != "" && (diskSize != dvDiskSize || len(mountPointArray) > 0) { + if diskDatastoreID != "" && (diskSize != dvDiskSize || len(mountPoints) > 0) { // This is a special case where the rootfs size is set to a non-default value at creation time. // see https://pve.proxmox.com/pve-docs/chapter-pct.html#_storage_backed_mount_points rootFS = &containers.CustomRootFS{ @@ -1690,7 +1692,7 @@ func containerCreateCustom(ctx context.Context, d *schema.ResourceData, m interf } networkInterface := d.Get(mkNetworkInterface).([]interface{}) - networkInterfaceArray := make(containers.CustomNetworkInterfaceArray, len(networkInterface)) + networkInterfaces := make(containers.CustomNetworkInterfaces, len(networkInterface)) for ni, nv := range networkInterface { networkInterfaceMap := nv.(map[string]interface{}) @@ -1750,19 +1752,19 @@ func containerCreateCustom(ctx context.Context, d *schema.ResourceData, m interf networkInterfaceObject.MTU = &mtu } - networkInterfaceArray[ni] = networkInterfaceObject + networkInterfaces[fmt.Sprintf("net%d", ni)] = &networkInterfaceObject } devicePassthrough := d.Get(mkDevicePassthrough).([]interface{}) - devicePassthroughArray := make( - containers.CustomDevicePassthroughArray, + passthroughDevices := make( + containers.CustomPassthroughDevices, len(devicePassthrough), ) for di, dv := range devicePassthrough { devicePassthroughMap := dv.(map[string]interface{}) - devicePassthroughObject := containers.CustomDevicePassthrough{} + devicePassthroughObject := containers.CustomPassthroughDevice{} denyWrite := types.CustomBool( devicePassthroughMap[mkDevicePassthroughDenyWrite].(bool), @@ -1778,7 +1780,7 @@ func containerCreateCustom(ctx context.Context, d *schema.ResourceData, m interf devicePassthroughObject.Path = path devicePassthroughObject.UID = &uid - devicePassthroughArray[di] = devicePassthroughObject + passthroughDevices[fmt.Sprintf("dev%d", di)] = &devicePassthroughObject } operatingSystem := d.Get(mkOperatingSystem).([]interface{}) @@ -1828,10 +1830,10 @@ func containerCreateCustom(ctx context.Context, d *schema.ResourceData, m interf CPUUnits: &cpuUnits, DatastoreID: &diskDatastoreID, DedicatedMemory: &memoryDedicated, - DevicePassthrough: devicePassthroughArray, + PassthroughDevices: passthroughDevices, Features: features, - MountPoints: mountPointArray, - NetworkInterfaces: networkInterfaceArray, + MountPoints: mountPoints, + NetworkInterfaces: networkInterfaces, OSTemplateFileVolume: &operatingSystemTemplateFileID, OSType: &operatingSystemType, Protection: &protection, @@ -1941,25 +1943,9 @@ func containerGetExistingNetworkInterface( return []interface{}{}, fmt.Errorf("error getting container information: %w", err) } - //nolint:prealloc - var networkInterfaces []interface{} - - networkInterfaceArray := []*containers.CustomNetworkInterface{ - containerInfo.NetworkInterface0, - containerInfo.NetworkInterface1, - containerInfo.NetworkInterface2, - containerInfo.NetworkInterface3, - containerInfo.NetworkInterface4, - containerInfo.NetworkInterface5, - containerInfo.NetworkInterface6, - containerInfo.NetworkInterface7, - } - - for _, nv := range networkInterfaceArray { - if nv == nil { - continue - } + networkInterfaces := make([]interface{}, 0, len(containerInfo.NetworkInterfaces)) + for _, nv := range containerInfo.NetworkInterfaces { networkInterface := map[string]interface{}{} networkInterface[mkNetworkInterfaceEnabled] = true @@ -2343,85 +2329,56 @@ func containerRead(ctx context.Context, d *schema.ResourceData, m interface{}) d initialization[mkInitializationHostname] = "" } - devicePassthroughArray := []*containers.CustomDevicePassthrough{ - containerConfig.DevicePassthrough0, - containerConfig.DevicePassthrough1, - containerConfig.DevicePassthrough2, - containerConfig.DevicePassthrough3, - containerConfig.DevicePassthrough4, - containerConfig.DevicePassthrough5, - containerConfig.DevicePassthrough6, - containerConfig.DevicePassthrough7, - } + passthroughDevicesMap := make(map[string]interface{}, len(containerConfig.PassthroughDevices)) - devicePassthroughList := make([]interface{}, 0, len(devicePassthroughArray)) - - for _, dp := range devicePassthroughArray { - if dp == nil { - continue - } - - devicePassthrough := map[string]interface{}{} + for key, dp := range containerConfig.PassthroughDevices { + passthroughDevice := map[string]interface{}{} if dp.DenyWrite != nil { - devicePassthrough[mkDevicePassthroughDenyWrite] = *dp.DenyWrite + passthroughDevice[mkDevicePassthroughDenyWrite] = *dp.DenyWrite } else { - devicePassthrough[mkDevicePassthroughDenyWrite] = false + passthroughDevice[mkDevicePassthroughDenyWrite] = false } if dp.GID != nil { - devicePassthrough[mkDevicePassthroughGID] = *dp.GID + passthroughDevice[mkDevicePassthroughGID] = *dp.GID } else { - devicePassthrough[mkDevicePassthroughGID] = 0 + passthroughDevice[mkDevicePassthroughGID] = 0 } if dp.Mode != nil { - devicePassthrough[mkDevicePassthroughMode] = *dp.Mode + passthroughDevice[mkDevicePassthroughMode] = *dp.Mode } else { - devicePassthrough[mkDevicePassthroughMode] = dvDevicePassthroughMode + passthroughDevice[mkDevicePassthroughMode] = dvDevicePassthroughMode } - devicePassthrough[mkDevicePassthroughPath] = dp.Path + passthroughDevice[mkDevicePassthroughPath] = dp.Path if dp.UID != nil { - devicePassthrough[mkDevicePassthroughUID] = *dp.UID + passthroughDevice[mkDevicePassthroughUID] = *dp.UID } else { - devicePassthrough[mkDevicePassthroughUID] = 0 + passthroughDevice[mkDevicePassthroughUID] = 0 } - devicePassthroughList = append(devicePassthroughList, devicePassthrough) + passthroughDevicesMap[key] = passthroughDevice } - currentDevicePassthrough := d.Get(mkDevicePassthrough).([]interface{}) + passthroughDevices := utils.OrderedListFromMap(passthroughDevicesMap) + currentPassthroughDevices := d.Get(mkDevicePassthrough).([]interface{}) if len(clone) > 0 { - if len(currentDevicePassthrough) > 0 { - err := d.Set(mkDevicePassthrough, devicePassthroughList) + if len(currentPassthroughDevices) > 0 { + err := d.Set(mkDevicePassthrough, passthroughDevices) diags = append(diags, diag.FromErr(err)...) } - } else if len(devicePassthroughList) > 0 { - err := d.Set(mkDevicePassthrough, devicePassthroughList) + } else if len(passthroughDevicesMap) > 0 { + err := d.Set(mkDevicePassthrough, passthroughDevices) diags = append(diags, diag.FromErr(err)...) } - mountPointArray := []*containers.CustomMountPoint{ - containerConfig.MountPoint0, - containerConfig.MountPoint1, - containerConfig.MountPoint2, - containerConfig.MountPoint3, - containerConfig.MountPoint4, - containerConfig.MountPoint5, - containerConfig.MountPoint6, - containerConfig.MountPoint7, - } - - mountPointList := make([]interface{}, 0, len(mountPointArray)) - - for _, mp := range mountPointArray { - if mp == nil { - continue - } + mountPointsMap := make(map[string]interface{}, len(containerConfig.MountPoints)) + for key, mp := range containerConfig.MountPoints { mountPoint := map[string]interface{}{} if mp.ACL != nil { @@ -2476,42 +2433,27 @@ func containerRead(ctx context.Context, d *schema.ResourceData, m interface{}) d mountPoint[mkMountPointVolume] = mp.Volume - mountPointList = append(mountPointList, mountPoint) + mountPointsMap[key] = mountPoint } - currentMountPoint := d.Get(mkMountPoint).([]interface{}) + mountPoints := utils.OrderedListFromMap(mountPointsMap) + currentMountPoints := d.Get(mkMountPoint).([]interface{}) if len(clone) > 0 { - if len(currentMountPoint) > 0 { - err := d.Set(mkMountPoint, mountPointList) + if len(currentMountPoints) > 0 { + err := d.Set(mkMountPoint, mountPoints) diags = append(diags, diag.FromErr(err)...) } - } else if len(mountPointList) > 0 { - err := d.Set(mkMountPoint, mountPointList) + } else if len(mountPointsMap) > 0 { + err := d.Set(mkMountPoint, mountPoints) diags = append(diags, diag.FromErr(err)...) } var ipConfigList []interface{} - networkInterfaceArray := []*containers.CustomNetworkInterface{ - containerConfig.NetworkInterface0, - containerConfig.NetworkInterface1, - containerConfig.NetworkInterface2, - containerConfig.NetworkInterface3, - containerConfig.NetworkInterface4, - containerConfig.NetworkInterface5, - containerConfig.NetworkInterface6, - containerConfig.NetworkInterface7, - } - - //nolint:prealloc - var networkInterfaceList []interface{} - - for _, nv := range networkInterfaceArray { - if nv == nil { - continue - } + networkInterfacesMap := make(map[string]interface{}, len(containerConfig.NetworkInterfaces)) + for key, nv := range containerConfig.NetworkInterfaces { if nv.IPv4Address != nil || nv.IPv4Gateway != nil || nv.IPv6Address != nil || nv.IPv6Gateway != nil { ipConfig := map[string]interface{}{} @@ -2604,9 +2546,10 @@ func containerRead(ctx context.Context, d *schema.ResourceData, m interface{}) d networkInterface[mkNetworkInterfaceMTU] = 0 } - networkInterfaceList = append(networkInterfaceList, networkInterface) + networkInterfacesMap[key] = networkInterface } + networkInterfaces := utils.OrderedListFromMap(networkInterfacesMap) initialization[mkInitializationIPConfig] = ipConfigList currentInitialization := d.Get(mkInitialization).([]interface{}) @@ -2655,7 +2598,7 @@ func containerRead(ctx context.Context, d *schema.ResourceData, m interface{}) d if len(currentNetworkInterface) > 0 { err := d.Set( mkNetworkInterface, - networkInterfaceList, + networkInterfaces, ) diags = append(diags, diag.FromErr(err)...) } @@ -2668,7 +2611,7 @@ func containerRead(ctx context.Context, d *schema.ResourceData, m interface{}) d diags = append(diags, diag.FromErr(e)...) - err := d.Set(mkNetworkInterface, networkInterfaceList) + err := d.Set(mkNetworkInterface, networkInterfaces) diags = append(diags, diag.FromErr(err)...) } @@ -3051,14 +2994,14 @@ func containerUpdate(ctx context.Context, d *schema.ResourceData, m interface{}) _, newDevicePassthrough := d.GetChange(mkDevicePassthrough) devicePassthrough := newDevicePassthrough.([]interface{}) - devicePassthroughArray := make( - containers.CustomDevicePassthroughArray, + passthroughDevices := make( + containers.CustomPassthroughDevices, len(devicePassthrough), ) for i, dp := range devicePassthrough { devicePassthroughMap := dp.(map[string]interface{}) - devicePassthroughObject := containers.CustomDevicePassthrough{} + devicePassthroughObject := containers.CustomPassthroughDevice{} denyWrite := types.CustomBool(devicePassthroughMap[mkDevicePassthroughDenyWrite].(bool)) gid := devicePassthroughMap[mkDevicePassthroughGID].(int) @@ -3072,10 +3015,10 @@ func containerUpdate(ctx context.Context, d *schema.ResourceData, m interface{}) devicePassthroughObject.Path = path devicePassthroughObject.UID = &uid - devicePassthroughArray[i] = devicePassthroughObject + passthroughDevices[fmt.Sprintf("dev%d", i)] = &devicePassthroughObject } - updateBody.DevicePassthrough = devicePassthroughArray + updateBody.PassthroughDevices = passthroughDevices rebootRequired = true } @@ -3085,8 +3028,8 @@ func containerUpdate(ctx context.Context, d *schema.ResourceData, m interface{}) _, newMountPoints := d.GetChange(mkMountPoint) mountPoints := newMountPoints.([]interface{}) - mountPointArray := make( - containers.CustomMountPointArray, + mountPointsMap := make( + containers.CustomMountPoints, len(mountPoints), ) @@ -3143,10 +3086,10 @@ func containerUpdate(ctx context.Context, d *schema.ResourceData, m interface{}) mountPointObject.MountOptions = &mountOptionsArray } - mountPointArray[i] = mountPointObject + mountPointsMap[fmt.Sprintf("mp%d", i)] = &mountPointObject } - updateBody.MountPoints = mountPointArray + updateBody.MountPoints = mountPointsMap rebootRequired = true } @@ -3163,8 +3106,8 @@ func containerUpdate(ctx context.Context, d *schema.ResourceData, m interface{}) if d.HasChange(mkInitialization) || d.HasChange(mkNetworkInterface) { - networkInterfaceArray := make( - containers.CustomNetworkInterfaceArray, + networkInterfaces := make( + containers.CustomNetworkInterfaces, len(networkInterface), ) @@ -3226,18 +3169,18 @@ func containerUpdate(ctx context.Context, d *schema.ResourceData, m interface{}) networkInterfaceObject.MTU = &mtu } - networkInterfaceArray[ni] = networkInterfaceObject + networkInterfaces[fmt.Sprintf("net%d", ni)] = &networkInterfaceObject } - updateBody.NetworkInterfaces = networkInterfaceArray + updateBody.NetworkInterfaces = networkInterfaces - for i, ni := range updateBody.NetworkInterfaces { + for key, ni := range updateBody.NetworkInterfaces { if !ni.Enabled { - updateBody.Delete = append(updateBody.Delete, fmt.Sprintf("net%d", i)) + updateBody.Delete = append(updateBody.Delete, key) } } - for i := len(updateBody.NetworkInterfaces); i < maxResourceVirtualEnvironmentContainerNetworkInterfaces; i++ { + for i := len(updateBody.NetworkInterfaces); i < maxNetworkInterfaces; i++ { updateBody.Delete = append(updateBody.Delete, fmt.Sprintf("net%d", i)) } diff --git a/utils/maps.go b/utils/maps.go index e67cbbfd..f94dcf5f 100644 --- a/utils/maps.go +++ b/utils/maps.go @@ -8,10 +8,14 @@ package utils import ( "reflect" - "sort" + "slices" + "strconv" + "strings" ) // OrderedListFromMap generates a list from a map's values. The values are sorted based on the map's keys. +// The sorting is done using a custom comparison function that compares the keys with special rules, assuming they +// are strings representing device names or similar, i.e. "disk0", "net1", etc. func OrderedListFromMap(inputMap map[string]interface{}) []interface{} { itemCount := len(inputMap) keyList := make([]string, itemCount) @@ -22,11 +26,60 @@ func OrderedListFromMap(inputMap map[string]interface{}) []interface{} { i++ } - sort.Strings(keyList) + slices.SortFunc(keyList, compareWithPrefix) return OrderedListFromMapByKeyValues(inputMap, keyList) } +// CompareWithPrefix compares two string values with special rules: +// - If both start with the same prefix, trims the prefix and compares the rest as numbers if possible. +// - If numbers are equal, falls back to string comparison (preserving digit formatting). +// - If numeric parsing fails, falls back to string comparison. +// - If prefixes differ, compares the whole values as strings. +func compareWithPrefix(a, b string) int { + prefix := commonPrefix(a, b) + + if prefix != "" { + aRest := strings.TrimPrefix(a, prefix) + bRest := strings.TrimPrefix(b, prefix) + + aNum, aErr := strconv.Atoi(aRest) + bNum, bErr := strconv.Atoi(bRest) + + if aErr == nil && bErr == nil { + if aNum != bNum { + if aNum < bNum { + return -1 + } + + return 1 + } + // numeric values equal, fallback to string comparison + return strings.Compare(aRest, bRest) + } + + return strings.Compare(aRest, bRest) + } + + return strings.Compare(a, b) +} + +// commonPrefix returns the longest common prefix of two strings. +func commonPrefix(a, b string) string { + minLen := len(a) + if len(b) < minLen { + minLen = len(b) + } + + for i := range minLen { + if a[i] != b[i] { + return a[:i] + } + } + + return a[:minLen] +} + // ListResourcesAttributeValue generates a list of strings from a Terraform resource list (which is list of maps). // The list is generated by extracting a specific key attribute from each resource. If the attribute is not found in a // resource, it is skipped. diff --git a/utils/maps_test.go b/utils/maps_test.go index 399681b7..6bf33161 100644 --- a/utils/maps_test.go +++ b/utils/maps_test.go @@ -1,8 +1,16 @@ +/* + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. + */ + package utils import ( "reflect" "testing" + + "github.com/stretchr/testify/assert" ) func TestOrderedListFromMap(t *testing.T) { @@ -72,3 +80,41 @@ func TestOrderedListFromMapByKeyValues(t *testing.T) { t.Errorf("OrderedListFromMapByKeyValues() = %v, want %v", result, expected) } } + +func TestCompareWithPrefix(t *testing.T) { + t.Parallel() + + type args struct { + a string + b string + } + + tests := []struct { + name string + args args + want int + }{ + {"equal", args{"a", "a"}, 0}, + {"a < b", args{"a", "b"}, -1}, + {"b > a", args{"b", "a"}, 1}, + {"a < b with prefix", args{"a1", "a2"}, -1}, + {"b > a with prefix", args{"a2", "a1"}, 1}, + {"a < b with different prefix", args{"a1", "b1"}, -1}, + {"b > a with different prefix", args{"b1", "a1"}, 1}, + {"a < b with different prefix and numbers", args{"a1", "a10"}, -1}, + {"b > a with different prefix and numbers", args{"a10", "a1"}, 1}, + {"a < b with different prefix and numbers", args{"a10", "b1"}, -1}, + {"b > a with different prefix and numbers", args{"b1", "a10"}, 1}, + {"a < b with different prefix and numbers", args{"a10", "b10"}, -1}, + {"b > a with different prefix and numbers", args{"b10", "a10"}, 1}, + {"b > a with different numbers leading zeros", args{"dev01", "dev001"}, 1}, + {"a > b with different numbers leading zeros", args{"dev001", "dev01"}, -1}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + assert.Equalf(t, tt.want, compareWithPrefix(tt.args.a, tt.args.b), "compareWithPrefix(%v, %v)", tt.args.a, tt.args.b) + }) + } +}