From 9c0d4986db900632e4323f7266adb17ae3be92d8 Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Tue, 20 Aug 2024 18:36:43 +0200 Subject: [PATCH 1/5] feat: `ConfigQemu.Serials` Reimplement --- proxmox/config_qemu.go | 150 +++++++++++------------------ proxmox/config_qemu_serial.go | 138 ++++++++++++++++++++++++++ proxmox/config_qemu_serial_test.go | 90 +++++++++++++++++ proxmox/config_qemu_test.go | 99 ++++++++++++++++++- 4 files changed, 382 insertions(+), 95 deletions(-) create mode 100644 proxmox/config_qemu_serial.go create mode 100644 proxmox/config_qemu_serial_test.go diff --git a/proxmox/config_qemu.go b/proxmox/config_qemu.go index 8f7db098..0d2d084f 100644 --- a/proxmox/config_qemu.go +++ b/proxmox/config_qemu.go @@ -30,50 +30,50 @@ type ( // ConfigQemu - Proxmox API QEMU options type ConfigQemu struct { - Agent *QemuGuestAgent `json:"agent,omitempty"` - Args string `json:"args,omitempty"` - Bios string `json:"bios,omitempty"` - Boot string `json:"boot,omitempty"` // TODO should be an array of custom enums - BootDisk string `json:"bootdisk,omitempty"` // TODO discuss deprecation? Only returned as it's deprecated in the proxmox api - CPU *QemuCPU `json:"cpu,omitempty"` - CloudInit *CloudInit `json:"cloudinit,omitempty"` - Description *string `json:"description,omitempty"` - Disks *QemuStorages `json:"disks,omitempty"` - EFIDisk QemuDevice `json:"efidisk,omitempty"` // TODO should be a struct - Storage string `json:"storage,omitempty"` // this value is only used when doing a full clone and is never returned - FullClone *int `json:"fullclone,omitempty"` // TODO should probably be a bool - HaGroup string `json:"hagroup,omitempty"` - HaState string `json:"hastate,omitempty"` // TODO should be custom type with enum - Hookscript string `json:"hookscript,omitempty"` - Hotplug string `json:"hotplug,omitempty"` // TODO should be a struct - Iso *IsoFile `json:"iso,omitempty"` // Same as Disks.Ide.Disk_2.CdRom.Iso - LinkedVmId uint `json:"linked_id,omitempty"` // Only returned setting it has no effect - Machine string `json:"machine,omitempty"` // TODO should be custom type with enum - Memory *QemuMemory `json:"memory,omitempty"` - Name string `json:"name,omitempty"` // TODO should be custom type as there are character and length limitations - Node string `json:"node,omitempty"` // Only returned setting it has no effect, set node in the VmRef instead - Onboot *bool `json:"onboot,omitempty"` - Pool *PoolName `json:"pool,omitempty"` - Protection *bool `json:"protection,omitempty"` - QemuDisks QemuDevices `json:"disk,omitempty"` // DEPRECATED use Disks *QemuStorages instead - QemuIso string `json:"qemuiso,omitempty"` // DEPRECATED use Iso *IsoFile instead - QemuKVM *bool `json:"kvm,omitempty"` - QemuNetworks QemuDevices `json:"network,omitempty"` // TODO should be a struct - QemuOs string `json:"ostype,omitempty"` - QemuPCIDevices QemuDevices `json:"hostpci,omitempty"` // TODO should be a struct - QemuPxe bool `json:"pxe,omitempty"` - QemuSerials QemuDevices `json:"serial,omitempty"` // TODO should be a struct - QemuUnusedDisks QemuDevices `json:"unused,omitempty"` // TODO should be a struct - QemuUsbs QemuDevices `json:"usb,omitempty"` // TODO should be a struct - QemuVga QemuDevice `json:"vga,omitempty"` // TODO should be a struct - RNGDrive QemuDevice `json:"rng0,omitempty"` // TODO should be a struct - Scsihw string `json:"scsihw,omitempty"` // TODO should be custom type with enum - Smbios1 string `json:"smbios1,omitempty"` // TODO should be custom type with enum? - Startup string `json:"startup,omitempty"` // TODO should be a struct? - TPM *TpmState `json:"tpm,omitempty"` - Tablet *bool `json:"tablet,omitempty"` - Tags *[]Tag `json:"tags,omitempty"` - VmID int `json:"vmid,omitempty"` // TODO should be a custom type as there are limitations + Agent *QemuGuestAgent `json:"agent,omitempty"` + Args string `json:"args,omitempty"` + Bios string `json:"bios,omitempty"` + Boot string `json:"boot,omitempty"` // TODO should be an array of custom enums + BootDisk string `json:"bootdisk,omitempty"` // TODO discuss deprecation? Only returned as it's deprecated in the proxmox api + CPU *QemuCPU `json:"cpu,omitempty"` + CloudInit *CloudInit `json:"cloudinit,omitempty"` + Description *string `json:"description,omitempty"` + Disks *QemuStorages `json:"disks,omitempty"` + EFIDisk QemuDevice `json:"efidisk,omitempty"` // TODO should be a struct + FullClone *int `json:"fullclone,omitempty"` // TODO should probably be a bool + HaGroup string `json:"hagroup,omitempty"` + HaState string `json:"hastate,omitempty"` // TODO should be custom type with enum + Hookscript string `json:"hookscript,omitempty"` + Hotplug string `json:"hotplug,omitempty"` // TODO should be a struct + Iso *IsoFile `json:"iso,omitempty"` // Same as Disks.Ide.Disk_2.CdRom.Iso + LinkedVmId uint `json:"linked_id,omitempty"` // Only returned setting it has no effect + Machine string `json:"machine,omitempty"` // TODO should be custom type with enum + Memory *QemuMemory `json:"memory,omitempty"` + Name string `json:"name,omitempty"` // TODO should be custom type as there are character and length limitations + Node string `json:"node,omitempty"` // Only returned setting it has no effect, set node in the VmRef instead + Onboot *bool `json:"onboot,omitempty"` + Pool *PoolName `json:"pool,omitempty"` + Protection *bool `json:"protection,omitempty"` + QemuDisks QemuDevices `json:"disk,omitempty"` // DEPRECATED use Disks *QemuStorages instead + QemuIso string `json:"qemuiso,omitempty"` // DEPRECATED use Iso *IsoFile instead + QemuKVM *bool `json:"kvm,omitempty"` + QemuNetworks QemuDevices `json:"network,omitempty"` // TODO should be a struct + QemuOs string `json:"ostype,omitempty"` + QemuPCIDevices QemuDevices `json:"hostpci,omitempty"` // TODO should be a struct + QemuPxe bool `json:"pxe,omitempty"` + QemuUnusedDisks QemuDevices `json:"unused,omitempty"` // TODO should be a struct + QemuUsbs QemuDevices `json:"usb,omitempty"` // TODO should be a struct + QemuVga QemuDevice `json:"vga,omitempty"` // TODO should be a struct + RNGDrive QemuDevice `json:"rng0,omitempty"` // TODO should be a struct + Scsihw string `json:"scsihw,omitempty"` // TODO should be custom type with enum + Serials SerialInterfaces `json:"serials,omitempty"` + Smbios1 string `json:"smbios1,omitempty"` // TODO should be custom type with enum? + Startup string `json:"startup,omitempty"` // TODO should be a struct? + Storage string `json:"storage,omitempty"` // this value is only used when doing a full clone and is never returned + TPM *TpmState `json:"tpm,omitempty"` + Tablet *bool `json:"tablet,omitempty"` + Tags *[]Tag `json:"tags,omitempty"` + VmID int `json:"vmid,omitempty"` // TODO should be a custom type as there are limitations } const ( @@ -128,9 +128,6 @@ func (config *ConfigQemu) defaults() { if config.QemuPCIDevices == nil { config.QemuPCIDevices = QemuDevices{} } - if config.QemuSerials == nil { - config.QemuSerials = QemuDevices{} - } if config.QemuUnusedDisks == nil { config.QemuUnusedDisks = QemuDevices{} } @@ -256,6 +253,9 @@ func (config ConfigQemu) mapToAPI(currentConfig ConfigQemu, version Version) (re if config.Memory != nil { itemsToDelete += config.Memory.mapToAPI(currentConfig.Memory, params) } + if config.Serials != nil { + itemsToDelete += config.Serials.mapToAPI(currentConfig.Serials, params) + } // Create EFI disk config.CreateQemuEfiParams(params) @@ -272,8 +272,6 @@ func (config ConfigQemu) mapToAPI(currentConfig ConfigQemu, version Version) (re if len(vgaParam) > 0 { params["vga"] = strings.Join(vgaParam, ",") } - // Create serial interfaces - config.CreateQemuSerialsParams(params) // Create usb interfaces config.CreateQemuUsbsParams(params) @@ -483,32 +481,7 @@ func (ConfigQemu) mapToStruct(vmr *VmRef, params map[string]interface{}) (*Confi } } - // Add serials - serialNames := []string{} - - for k := range params { - if serialName := rxSerialName.FindStringSubmatch(k); len(serialName) > 0 { - serialNames = append(serialNames, serialName[0]) - } - } - - if len(serialNames) > 0 { - config.QemuSerials = QemuDevices{} - for _, serialName := range serialNames { - id := rxDeviceID.FindStringSubmatch(serialName) - serialID, _ := strconv.Atoi(id[0]) - - serialConfMap := QemuDevice{ - "id": serialID, - "type": params[serialName], - } - - // And device config to serials map. - if len(serialConfMap) > 0 { - config.QemuSerials[serialID] = serialConfMap - } - } - } + config.Serials = SerialInterfaces{}.mapToSDK(params) // Add usbs usbNames := []string{} @@ -827,6 +800,11 @@ func (config ConfigQemu) Validate(current *ConfigQemu, version Version) (err err return } } + if len(config.Serials) > 0 { + if err = config.Serials.Validate(); err != nil { + return + } + } if config.Tags != nil { if err := Tag("").validate(*config.Tags); err != nil { return err @@ -850,16 +828,13 @@ storage:xxx */ func (config ConfigQemu) CloneVm(sourceVmr *VmRef, vmr *VmRef, client *Client) (err error) { vmr.SetVmType("qemu") - storage := config.Storage + var storage string fullClone := "1" if config.FullClone != nil { fullClone = strconv.Itoa(*config.FullClone) } - // if storage is not set, use the storage of the first disk - if storage == "" && len(config.QemuDisks) > 0 { - if disk0Storage, ok := config.QemuDisks[0]["storage"].(string); ok && len(disk0Storage) > 0 { - storage = disk0Storage - } + if disk0Storage, ok := config.QemuDisks[0]["storage"].(string); ok && len(disk0Storage) > 0 { + storage = disk0Storage } params := map[string]interface{}{ "newid": vmr.vmId, @@ -1292,19 +1267,6 @@ func (c ConfigQemu) CreateQemuPCIsParams(params map[string]interface{}) { } } -// Create parameters for serial interface -func (c ConfigQemu) CreateQemuSerialsParams(params map[string]interface{}) { - // For new style with multi disk device. - for serialID, serialConfMap := range c.QemuSerials { - // Device name. - deviceType := serialConfMap["type"].(string) - qemuSerialName := "serial" + strconv.Itoa(serialID) - - // Add back to Qemu prams. - params[qemuSerialName] = deviceType - } -} - // Create parameters for usb interface func (c ConfigQemu) CreateQemuUsbsParams(params map[string]interface{}) { for usbID, usbConfMap := range c.QemuUsbs { diff --git a/proxmox/config_qemu_serial.go b/proxmox/config_qemu_serial.go new file mode 100644 index 00000000..90786b33 --- /dev/null +++ b/proxmox/config_qemu_serial.go @@ -0,0 +1,138 @@ +package proxmox + +import ( + "errors" + "regexp" + "strconv" +) + +type SerialID uint8 + +const ( + SerialID0 SerialID = 0 + SerialID1 SerialID = 1 + SerialID2 SerialID = 2 + SerialID3 SerialID = 3 + SerialID_Errors_Invalid string = "serial id must be one of 0,1,2,3" +) + +func (id SerialID) String() string { + return strconv.Itoa(int(id)) +} + +func (id SerialID) Validate() error { + if id > 3 { + return errors.New(SerialID_Errors_Invalid) + } + return nil +} + +type SerialInterface struct { + Delete bool `json:"delete,omitempty"` // If true, the serial adapter will be removed. + Path SerialPath `json:"path,omitempty"` // Path to the serial device. Mutually exclusive with socket. + Socket bool `json:"socket,omitempty"` // If true, the serial device is a socket. Mutually exclusive with path. +} + +var regexSerialPortPath = regexp.MustCompile(`^/dev/.*$`) + +func (port SerialInterface) mapToAPI(id SerialID, params map[string]interface{}) { + tmpPath := "socket" + if !port.Socket { + tmpPath = string(port.Path) + } + params["serial"+id.String()] = tmpPath +} + +func (SerialInterface) mapToSDK(v string) SerialInterface { + if v == "socket" { + return SerialInterface{ + Socket: true} + } + return SerialInterface{ + Path: SerialPath(v), + } +} + +func (port SerialInterface) Validate() error { + if port.Path != "" && port.Socket { + return errors.New(SerialPath_Errors_MutualExclusive) + } + if !port.Socket { + if port.Path == "" { + return errors.New(SerialPath_Errors_Empty) + } + matches, _ := regexp.MatchString(regexSerialPortPath.String(), string(port.Path)) + if !matches { + return errors.New(SerialPath_Errors_Invalid) + } + } + return nil +} + +type SerialInterfaces map[SerialID]SerialInterface + +func (config SerialInterfaces) mapToAPI(current SerialInterfaces, params map[string]interface{}) (delete string) { + if len(current) != 0 { // Update + for id, port := range config { + if _, ok := current[id]; ok { + if port.Delete { + delete += ",serial" + id.String() + continue + } + if current[id].Path != port.Path || current[id].Socket != port.Socket { + port.mapToAPI(id, params) + } + } else if !port.Delete { + port.mapToAPI(id, params) + } + } + return + } + // Create + for id, port := range config { + if !port.Delete { + port.mapToAPI(id, params) + } + } + return +} + +func (SerialInterfaces) mapToSDK(params map[string]interface{}) SerialInterfaces { + Serials := SerialInterfaces{} + if v, isSet := params["serial0"]; isSet { + Serials[SerialID0] = SerialInterface{}.mapToSDK(v.(string)) + } + if v, isSet := params["serial1"]; isSet { + Serials[SerialID1] = SerialInterface{}.mapToSDK(v.(string)) + } + if v, isSet := params["serial2"]; isSet { + Serials[SerialID2] = SerialInterface{}.mapToSDK(v.(string)) + } + if v, isSet := params["serial3"]; isSet { + Serials[SerialID3] = SerialInterface{}.mapToSDK(v.(string)) + } + if len(Serials) > 0 { + return Serials + } + return nil +} + +func (s SerialInterfaces) Validate() error { + for id, e := range s { + if err := id.Validate(); err != nil { + return err + } + if err := e.Validate(); err != nil { + return err + } + } + return nil +} + +type SerialPath string + +const ( + SerialPath_Errors_MutualExclusive string = "path and socket are mutually exclusive" + SerialPath_Errors_Empty string = "path or socket must be set" + SerialPath_Errors_Invalid string = "path must start with /dev/" +) diff --git a/proxmox/config_qemu_serial_test.go b/proxmox/config_qemu_serial_test.go new file mode 100644 index 00000000..f1caa174 --- /dev/null +++ b/proxmox/config_qemu_serial_test.go @@ -0,0 +1,90 @@ +package proxmox + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/require" +) + +func Test_SerialID_Validate(t *testing.T) { + tests := []struct { + name string + input SerialID + output error + }{ + {name: `Valid`, + input: 2}, + {name: `Invalid`, + input: 4, + output: errors.New(SerialID_Errors_Invalid)}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require.Equal(t, test.output, test.input.Validate()) + }) + } +} + +func Test_SerialPort_Validate(t *testing.T) { + tests := []struct { + name string + input SerialInterface + output error + }{ + {name: `Valid Path`, + input: SerialInterface{Path: "/dev/ttyS0"}}, + {name: `Valid Path + Delete`, + input: SerialInterface{Path: "/dev/ttyS0", Delete: true}}, + {name: `Valid Socket`, + input: SerialInterface{Socket: true}}, + {name: `Valid Socket + Delete`, + input: SerialInterface{Socket: true, Delete: true}}, + {name: `Invalid errors.New(SerialPath_Errors_MutualExclusive)`, + input: SerialInterface{Path: "/dev/ttyS0", Socket: true}, + output: errors.New(SerialPath_Errors_MutualExclusive)}, + {name: `Invalid errors.New(SerialPath_Errors_Empty)`, + input: SerialInterface{}, + output: errors.New(SerialPath_Errors_Empty)}, + {name: `Invalid errors.New(SerialPath_Errors_Invalid)`, + input: SerialInterface{Path: "ttyS0"}, + output: errors.New(SerialPath_Errors_Invalid)}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require.Equal(t, test.output, test.input.Validate()) + }) + } +} + +func Test_SerialInterfaces_Validate(t *testing.T) { + tests := []struct { + name string + input SerialInterfaces + output error + }{ + {name: `Valid`, + input: SerialInterfaces{ + SerialID0: SerialInterface{Path: "/dev/ttyS0"}, + SerialID1: SerialInterface{Path: "/dev/ttyS1", Delete: true}, + SerialID2: SerialInterface{Socket: true}, + SerialID3: SerialInterface{Socket: true, Delete: true}}}, + {name: `Invalid errors.New(SerialID_Errors_Invalid)`, + input: SerialInterfaces{4: SerialInterface{Delete: true}}, + output: errors.New(SerialID_Errors_Invalid)}, + {name: `Invalid errors.New(SerialPath_Errors_MutualExclusive)`, + input: SerialInterfaces{SerialID0: SerialInterface{Path: "/dev/ttyS0", Socket: true}}, + output: errors.New(SerialPath_Errors_MutualExclusive)}, + {name: `Invalid errors.New(SerialPath_Errors_Empty)`, + input: SerialInterfaces{SerialID0: SerialInterface{}}, + output: errors.New(SerialPath_Errors_Empty)}, + {name: `Invalid errors.New(SerialPath_Errors_Invalid)`, + input: SerialInterfaces{SerialID0: SerialInterface{Path: "ttyS0"}}, + output: errors.New(SerialPath_Errors_Invalid)}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require.Equal(t, test.output, test.input.Validate()) + }) + } +} diff --git a/proxmox/config_qemu_test.go b/proxmox/config_qemu_test.go index 523bd7a7..b425ead2 100644 --- a/proxmox/config_qemu_test.go +++ b/proxmox/config_qemu_test.go @@ -81,7 +81,6 @@ func Test_ConfigQemu_mapToAPI(t *testing.T) { createUpdate []test // value of currentConfig wil be used for update and ignored for create update []test }{ - // TODO add option for create and update, as the crate and update are the same for many settings like 'Agent' {category: `Agent`, create: []test{ {name: `Agent=nil`, @@ -3479,6 +3478,64 @@ func Test_ConfigQemu_mapToAPI(t *testing.T) { config: &ConfigQemu{Memory: &QemuMemory{Shares: util.Pointer(QemuMemoryShares(0))}}, currentConfig: ConfigQemu{Memory: &QemuMemory{Shares: util.Pointer(QemuMemoryShares(20000))}}, output: map[string]interface{}{"delete": "shares"}}}}, + {category: `Serials`, + createUpdate: []test{ + {name: `delete non existing`, + config: &ConfigQemu{Serials: SerialInterfaces{ + SerialID0: SerialInterface{Delete: true}, + SerialID2: SerialInterface{Delete: true}}}, + currentConfig: ConfigQemu{Serials: SerialInterfaces{ + SerialID1: SerialInterface{Socket: true}, + SerialID3: SerialInterface{Path: "/dev/tty2"}}}, + output: map[string]interface{}{}}, + {name: `add`, + config: &ConfigQemu{Serials: SerialInterfaces{ + SerialID1: SerialInterface{Path: "/dev/tty6"}, + SerialID3: SerialInterface{Socket: true}}}, + currentConfig: ConfigQemu{Serials: SerialInterfaces{ + SerialID0: SerialInterface{}, + SerialID2: SerialInterface{}}}, + output: map[string]interface{}{ + "serial1": "/dev/tty6", + "serial3": "socket"}}}, + update: []test{ + {name: `existing socket no change`, + config: &ConfigQemu{Serials: SerialInterfaces{ + SerialID0: SerialInterface{Socket: true}}}, + currentConfig: ConfigQemu{Serials: SerialInterfaces{ + SerialID0: SerialInterface{Socket: true}}}, + output: map[string]interface{}{}}, + {name: `existing path no change`, + config: &ConfigQemu{Serials: SerialInterfaces{ + SerialID1: SerialInterface{Path: "/dev/tty3"}}}, + currentConfig: ConfigQemu{Serials: SerialInterfaces{ + SerialID1: SerialInterface{Path: "/dev/tty3"}}}, + output: map[string]interface{}{}}, + {name: `existing path to path`, + config: &ConfigQemu{Serials: SerialInterfaces{ + SerialID2: SerialInterface{Path: "/dev/tty3"}}}, + currentConfig: ConfigQemu{Serials: SerialInterfaces{ + SerialID2: SerialInterface{Path: "/dev/tty7"}}}, + output: map[string]interface{}{"serial2": "/dev/tty3"}}, + {name: `existing socket to path`, + config: &ConfigQemu{Serials: SerialInterfaces{ + SerialID3: SerialInterface{Path: "/dev/tty2"}}}, + currentConfig: ConfigQemu{Serials: SerialInterfaces{ + SerialID3: SerialInterface{Socket: true}}}, + output: map[string]interface{}{"serial3": "/dev/tty2"}}, + {name: `existing path to socket`, + config: &ConfigQemu{Serials: SerialInterfaces{ + SerialID1: SerialInterface{Socket: true}}}, + currentConfig: ConfigQemu{Serials: SerialInterfaces{ + SerialID1: SerialInterface{Path: "/dev/tty7"}}}, + output: map[string]interface{}{"serial1": "socket"}}, + {name: `delete existing`, + config: &ConfigQemu{Serials: SerialInterfaces{SerialID2: SerialInterface{Delete: true}}}, + currentConfig: ConfigQemu{Serials: SerialInterfaces{ + SerialID0: SerialInterface{Socket: true}, + SerialID2: SerialInterface{Path: "/dev/tty78"}}}, + output: map[string]interface{}{"delete": "serial2"}}}, + }, {category: `Tags`, createUpdate: []test{ {name: `Tags Empty`, @@ -5915,6 +5972,25 @@ func Test_ConfigQemu_mapToStruct(t *testing.T) { {name: `vmr populated`, vmr: &VmRef{pool: "test"}, output: baseConfig(ConfigQemu{Pool: util.Pointer(PoolName("test"))})}}}, + {category: `Serials`, + tests: []test{ + {name: `All`, + input: map[string]interface{}{ + "serial2": "/dev/tty1", + "serial0": "socket", + "serial3": "/dev/tty3", + "serial1": "socket"}, + output: baseConfig(ConfigQemu{Serials: SerialInterfaces{ + SerialID0: SerialInterface{Socket: true}, + SerialID1: SerialInterface{Socket: true}, + SerialID2: SerialInterface{Path: "/dev/tty1"}, + SerialID3: SerialInterface{Path: "/dev/tty3"}}})}, + {name: `single path`, + input: map[string]interface{}{"serial3": "/dev/test/tty7"}, + output: baseConfig(ConfigQemu{Serials: SerialInterfaces{SerialID3: SerialInterface{Path: "/dev/test/tty7"}}})}, + {name: `single socket`, + input: map[string]interface{}{"serial2": "socket"}, + output: baseConfig(ConfigQemu{Serials: SerialInterfaces{SerialID2: SerialInterface{Socket: true}}})}}}, {category: `TPM`, tests: []test{ {name: `All`, @@ -7381,6 +7457,27 @@ func Test_ConfigQemu_Validate(t *testing.T) { {name: `Characters`, input: baseConfig(ConfigQemu{Pool: util.Pointer(PoolName(test_data_pool.PoolName_Error_Characters()[0]))}), err: errors.New(PoolName_Error_Characters)}}}, + {category: `Serials`, + valid: []test{ + {name: `all`, + input: baseConfig(ConfigQemu{Serials: SerialInterfaces{ + SerialID0: SerialInterface{Path: "/dev/ttyS0"}, + SerialID1: SerialInterface{Path: "/dev/ttyS1", Delete: true}, + SerialID2: SerialInterface{Socket: true}, + SerialID3: SerialInterface{Socket: true, Delete: true}}})}}, + invalid: []test{ + {name: `errors.New(SerialID_Errors_Invalid)`, + input: baseConfig(ConfigQemu{Serials: SerialInterfaces{4: SerialInterface{}}}), + err: errors.New(SerialID_Errors_Invalid)}, + {name: `errors.New(SerialPath_Errors_MutualExclusive)`, + input: baseConfig(ConfigQemu{Serials: SerialInterfaces{SerialID0: SerialInterface{Path: "/dev/ttyS1", Socket: true}}}), + err: errors.New(SerialPath_Errors_MutualExclusive)}, + {name: `errors.New(SerialPath_Errors_Empty)`, + input: baseConfig(ConfigQemu{Serials: SerialInterfaces{SerialID1: SerialInterface{}}}), + err: errors.New(SerialPath_Errors_Empty)}, + {name: `errors.New(SerialPath_Errors_Invalid)`, + input: baseConfig(ConfigQemu{Serials: SerialInterfaces{SerialID2: SerialInterface{Path: "invalid"}}}), + err: errors.New(SerialPath_Errors_Invalid)}}}, {category: `Tags`, valid: []test{ {input: baseConfig(ConfigQemu{Tags: util.Pointer(validTags())})}}, From 3257066f9fa42f22c6d7d073df954eeb2c9e05dc Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Wed, 21 Aug 2024 21:08:42 +0200 Subject: [PATCH 2/5] refactor: remove unused variable --- proxmox/config_qemu.go | 1 - 1 file changed, 1 deletion(-) diff --git a/proxmox/config_qemu.go b/proxmox/config_qemu.go index 0d2d084f..12b2a563 100644 --- a/proxmox/config_qemu.go +++ b/proxmox/config_qemu.go @@ -868,7 +868,6 @@ var ( rxUnusedDiskName = regexp.MustCompile(`^(unused)\d+`) rxNicName = regexp.MustCompile(`net\d+`) rxMpName = regexp.MustCompile(`mp\d+`) - rxSerialName = regexp.MustCompile(`serial\d+`) rxUsbName = regexp.MustCompile(`usb\d+`) rxPCIName = regexp.MustCompile(`hostpci\d+`) ) From 9479ef9263bb6a4a71fab3b2efdb617b8d1c64dd Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Mon, 26 Aug 2024 07:51:33 +0200 Subject: [PATCH 3/5] feat: `SerialPath` validate func --- proxmox/config_qemu_serial.go | 28 ++++++++++++++++--------- proxmox/config_qemu_serial_test.go | 33 +++++++++++++++++++++++------- proxmox/config_qemu_test.go | 8 ++++---- 3 files changed, 48 insertions(+), 21 deletions(-) diff --git a/proxmox/config_qemu_serial.go b/proxmox/config_qemu_serial.go index 90786b33..1bb1eec3 100644 --- a/proxmox/config_qemu_serial.go +++ b/proxmox/config_qemu_serial.go @@ -33,6 +33,11 @@ type SerialInterface struct { Socket bool `json:"socket,omitempty"` // If true, the serial device is a socket. Mutually exclusive with path. } +const ( + SerialInterface_Errors_MutualExclusive string = "path and socket are mutually exclusive" + SerialInterface_Errors_Empty string = "path or socket must be set" +) + var regexSerialPortPath = regexp.MustCompile(`^/dev/.*$`) func (port SerialInterface) mapToAPI(id SerialID, params map[string]interface{}) { @@ -55,15 +60,14 @@ func (SerialInterface) mapToSDK(v string) SerialInterface { func (port SerialInterface) Validate() error { if port.Path != "" && port.Socket { - return errors.New(SerialPath_Errors_MutualExclusive) + return errors.New(SerialInterface_Errors_MutualExclusive) } if !port.Socket { if port.Path == "" { - return errors.New(SerialPath_Errors_Empty) + return errors.New(SerialInterface_Errors_Empty) } - matches, _ := regexp.MatchString(regexSerialPortPath.String(), string(port.Path)) - if !matches { - return errors.New(SerialPath_Errors_Invalid) + if err := port.Path.Validate(); err != nil { + return err } } return nil @@ -131,8 +135,12 @@ func (s SerialInterfaces) Validate() error { type SerialPath string -const ( - SerialPath_Errors_MutualExclusive string = "path and socket are mutually exclusive" - SerialPath_Errors_Empty string = "path or socket must be set" - SerialPath_Errors_Invalid string = "path must start with /dev/" -) +const SerialPath_Errors_Invalid string = "path must start with /dev/" + +func (path SerialPath) Validate() error { + matches, _ := regexp.MatchString(regexSerialPortPath.String(), string(path)) + if !matches { + return errors.New(SerialPath_Errors_Invalid) + } + return nil +} diff --git a/proxmox/config_qemu_serial_test.go b/proxmox/config_qemu_serial_test.go index f1caa174..698964f3 100644 --- a/proxmox/config_qemu_serial_test.go +++ b/proxmox/config_qemu_serial_test.go @@ -40,12 +40,12 @@ func Test_SerialPort_Validate(t *testing.T) { input: SerialInterface{Socket: true}}, {name: `Valid Socket + Delete`, input: SerialInterface{Socket: true, Delete: true}}, - {name: `Invalid errors.New(SerialPath_Errors_MutualExclusive)`, + {name: `Invalid errors.New(SerialInterface_Errors_MutualExclusive)`, input: SerialInterface{Path: "/dev/ttyS0", Socket: true}, - output: errors.New(SerialPath_Errors_MutualExclusive)}, - {name: `Invalid errors.New(SerialPath_Errors_Empty)`, + output: errors.New(SerialInterface_Errors_MutualExclusive)}, + {name: `Invalid errors.New(SerialInterface_Errors_Empty)`, input: SerialInterface{}, - output: errors.New(SerialPath_Errors_Empty)}, + output: errors.New(SerialInterface_Errors_Empty)}, {name: `Invalid errors.New(SerialPath_Errors_Invalid)`, input: SerialInterface{Path: "ttyS0"}, output: errors.New(SerialPath_Errors_Invalid)}, @@ -74,10 +74,10 @@ func Test_SerialInterfaces_Validate(t *testing.T) { output: errors.New(SerialID_Errors_Invalid)}, {name: `Invalid errors.New(SerialPath_Errors_MutualExclusive)`, input: SerialInterfaces{SerialID0: SerialInterface{Path: "/dev/ttyS0", Socket: true}}, - output: errors.New(SerialPath_Errors_MutualExclusive)}, - {name: `Invalid errors.New(SerialPath_Errors_Empty)`, + output: errors.New(SerialInterface_Errors_MutualExclusive)}, + {name: `Invalid errors.New(SerialInterface_Errors_Empty)`, input: SerialInterfaces{SerialID0: SerialInterface{}}, - output: errors.New(SerialPath_Errors_Empty)}, + output: errors.New(SerialInterface_Errors_Empty)}, {name: `Invalid errors.New(SerialPath_Errors_Invalid)`, input: SerialInterfaces{SerialID0: SerialInterface{Path: "ttyS0"}}, output: errors.New(SerialPath_Errors_Invalid)}, @@ -88,3 +88,22 @@ func Test_SerialInterfaces_Validate(t *testing.T) { }) } } + +func Test_SerialPath_Validate(t *testing.T) { + tests := []struct { + name string + input SerialPath + output error + }{ + {name: `Valid`, + input: "/dev/ttyS0"}, + {name: `Invalid errors.New(SerialPath_Errors_Invalid)`, + input: "ttyS0", + output: errors.New(SerialPath_Errors_Invalid)}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require.Equal(t, test.output, test.input.Validate()) + }) + } +} diff --git a/proxmox/config_qemu_test.go b/proxmox/config_qemu_test.go index b425ead2..0d5ebb1f 100644 --- a/proxmox/config_qemu_test.go +++ b/proxmox/config_qemu_test.go @@ -7469,12 +7469,12 @@ func Test_ConfigQemu_Validate(t *testing.T) { {name: `errors.New(SerialID_Errors_Invalid)`, input: baseConfig(ConfigQemu{Serials: SerialInterfaces{4: SerialInterface{}}}), err: errors.New(SerialID_Errors_Invalid)}, - {name: `errors.New(SerialPath_Errors_MutualExclusive)`, + {name: `errors.New(SerialInterface_Errors_MutualExclusive)`, input: baseConfig(ConfigQemu{Serials: SerialInterfaces{SerialID0: SerialInterface{Path: "/dev/ttyS1", Socket: true}}}), - err: errors.New(SerialPath_Errors_MutualExclusive)}, - {name: `errors.New(SerialPath_Errors_Empty)`, + err: errors.New(SerialInterface_Errors_MutualExclusive)}, + {name: `errors.New(SerialInterface_Errors_Empty)`, input: baseConfig(ConfigQemu{Serials: SerialInterfaces{SerialID1: SerialInterface{}}}), - err: errors.New(SerialPath_Errors_Empty)}, + err: errors.New(SerialInterface_Errors_Empty)}, {name: `errors.New(SerialPath_Errors_Invalid)`, input: baseConfig(ConfigQemu{Serials: SerialInterfaces{SerialID2: SerialInterface{Path: "invalid"}}}), err: errors.New(SerialPath_Errors_Invalid)}}}, From 8135e8ada99feadc34f46d79af86cdbcbc949988 Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Mon, 26 Aug 2024 07:57:35 +0200 Subject: [PATCH 4/5] fix: missing case `/dev/` --- proxmox/config_qemu_serial.go | 2 +- proxmox/config_qemu_serial_test.go | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/proxmox/config_qemu_serial.go b/proxmox/config_qemu_serial.go index 1bb1eec3..878a6351 100644 --- a/proxmox/config_qemu_serial.go +++ b/proxmox/config_qemu_serial.go @@ -38,7 +38,7 @@ const ( SerialInterface_Errors_Empty string = "path or socket must be set" ) -var regexSerialPortPath = regexp.MustCompile(`^/dev/.*$`) +var regexSerialPortPath = regexp.MustCompile(`^/dev/.+$`) func (port SerialInterface) mapToAPI(id SerialID, params map[string]interface{}) { tmpPath := "socket" diff --git a/proxmox/config_qemu_serial_test.go b/proxmox/config_qemu_serial_test.go index 698964f3..276a9d74 100644 --- a/proxmox/config_qemu_serial_test.go +++ b/proxmox/config_qemu_serial_test.go @@ -100,6 +100,9 @@ func Test_SerialPath_Validate(t *testing.T) { {name: `Invalid errors.New(SerialPath_Errors_Invalid)`, input: "ttyS0", output: errors.New(SerialPath_Errors_Invalid)}, + {name: `Invalid /dev/ only`, + input: "/dev/", + output: errors.New(SerialPath_Errors_Invalid)}, } for _, test := range tests { t.Run(test.name, func(t *testing.T) { From 0430b85d76cee74a83ab14d29b131ca9c3b124d2 Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Mon, 26 Aug 2024 08:28:22 +0200 Subject: [PATCH 5/5] fix: `mutual exclusive` error when `Delete == true` --- proxmox/config_qemu_serial.go | 3 +++ proxmox/config_qemu_serial_test.go | 3 +++ proxmox/config_qemu_test.go | 5 ++++- 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/proxmox/config_qemu_serial.go b/proxmox/config_qemu_serial.go index 878a6351..7f76c866 100644 --- a/proxmox/config_qemu_serial.go +++ b/proxmox/config_qemu_serial.go @@ -59,6 +59,9 @@ func (SerialInterface) mapToSDK(v string) SerialInterface { } func (port SerialInterface) Validate() error { + if port.Delete { + return nil + } if port.Path != "" && port.Socket { return errors.New(SerialInterface_Errors_MutualExclusive) } diff --git a/proxmox/config_qemu_serial_test.go b/proxmox/config_qemu_serial_test.go index 276a9d74..bbebdb1f 100644 --- a/proxmox/config_qemu_serial_test.go +++ b/proxmox/config_qemu_serial_test.go @@ -69,6 +69,9 @@ func Test_SerialInterfaces_Validate(t *testing.T) { SerialID1: SerialInterface{Path: "/dev/ttyS1", Delete: true}, SerialID2: SerialInterface{Socket: true}, SerialID3: SerialInterface{Socket: true, Delete: true}}}, + {name: `Valid delete`, + input: SerialInterfaces{ + SerialID1: SerialInterface{Delete: true}}}, {name: `Invalid errors.New(SerialID_Errors_Invalid)`, input: SerialInterfaces{4: SerialInterface{Delete: true}}, output: errors.New(SerialID_Errors_Invalid)}, diff --git a/proxmox/config_qemu_test.go b/proxmox/config_qemu_test.go index 0d5ebb1f..e98f3c8b 100644 --- a/proxmox/config_qemu_test.go +++ b/proxmox/config_qemu_test.go @@ -7464,7 +7464,10 @@ func Test_ConfigQemu_Validate(t *testing.T) { SerialID0: SerialInterface{Path: "/dev/ttyS0"}, SerialID1: SerialInterface{Path: "/dev/ttyS1", Delete: true}, SerialID2: SerialInterface{Socket: true}, - SerialID3: SerialInterface{Socket: true, Delete: true}}})}}, + SerialID3: SerialInterface{Socket: true, Delete: true}}})}, + {name: `delete`, + input: baseConfig(ConfigQemu{Serials: SerialInterfaces{ + SerialID3: SerialInterface{Delete: true}}})}}, invalid: []test{ {name: `errors.New(SerialID_Errors_Invalid)`, input: baseConfig(ConfigQemu{Serials: SerialInterfaces{4: SerialInterface{}}}),