From 20d75e55413b8079897bc0bcedc5575a3d461569 Mon Sep 17 00:00:00 2001 From: Tinyblargon <76069640+Tinyblargon@users.noreply.github.com> Date: Sat, 28 Dec 2024 22:35:50 +0100 Subject: [PATCH] feat: type `NodeName` --- main.go | 2 +- proxmox/client.go | 30 +++++------ proxmox/config_guest.go | 6 +-- proxmox/config_lxc.go | 2 +- proxmox/config_qemu.go | 8 +-- proxmox/data_qemu_agent.go | 2 +- proxmox/node.go | 52 ++++++++++++++++++ proxmox/node_test.go | 65 +++++++++++++++++++++++ proxmox/snapshot.go | 10 ++-- test/data/test_data_node/type_NodeName.go | 58 ++++++++++++++++++++ 10 files changed, 205 insertions(+), 30 deletions(-) create mode 100644 proxmox/node_test.go create mode 100644 test/data/test_data_node/type_NodeName.go diff --git a/main.go b/main.go index 46a766b3..f38fc427 100644 --- a/main.go +++ b/main.go @@ -393,7 +393,7 @@ func main() { fmt.Printf("Missing target node\n") os.Exit(1) } - _, err := c.MigrateNode(ctx, vmr, args[2], true) + _, err := c.MigrateNode(ctx, vmr, proxmox.NodeName(args[2]), true) if err != nil { log.Printf("Error to move %+v\n", err) diff --git a/proxmox/client.go b/proxmox/client.go index 172917c9..25308df3 100644 --- a/proxmox/client.go +++ b/proxmox/client.go @@ -51,7 +51,7 @@ const ( // map[type:qemu node:proxmox1-xx id:qemu/132 diskread:5.57424738e+08 disk:0 netin:5.9297450593e+10 mem:3.3235968e+09 uptime:1.4567097e+07 vmid:132 template:0 maxcpu:2 netout:6.053310416e+09 maxdisk:3.4359738368e+10 maxmem:8.592031744e+09 diskwrite:1.49663619584e+12 status:running cpu:0.00386980694947209 name:appt-app1-dev.xxx.xx] type VmRef struct { vmId int - node string + node NodeName pool string vmType string haState string @@ -59,7 +59,7 @@ type VmRef struct { } func (vmr *VmRef) SetNode(node string) { - vmr.node = node + vmr.node = NodeName(node) } func (vmr *VmRef) SetPool(pool string) { @@ -78,7 +78,7 @@ func (vmr *VmRef) VmId() int { return vmr.vmId } -func (vmr *VmRef) Node() string { +func (vmr *VmRef) Node() NodeName { return vmr.node } @@ -235,7 +235,7 @@ func (c *Client) GetVmInfo(ctx context.Context, vmr *VmRef) (vmInfo map[string]i vm := vms[vmii].(map[string]interface{}) if int(vm["vmid"].(float64)) == vmr.vmId { vmInfo = vm - vmr.node = vmInfo["node"].(string) + vmr.node = NodeName(vmInfo["node"].(string)) vmr.vmType = vmInfo["type"].(string) vmr.pool = "" if vmInfo["pool"] != nil { @@ -268,7 +268,7 @@ func (c *Client) GetVmRefsByName(ctx context.Context, vmName string) (vmrs []*Vm vm := vms[vmii].(map[string]interface{}) if vm["name"] != nil && vm["name"].(string) == vmName { vmr := NewVmRef(int(vm["vmid"].(float64))) - vmr.node = vm["node"].(string) + vmr.node = NodeName(vm["node"].(string)) vmr.vmType = vm["type"].(string) vmr.pool = "" if vm["pool"] != nil { @@ -297,7 +297,7 @@ func (c *Client) GetVmRefById(ctx context.Context, vmId int) (vmr *VmRef, err er vm := vms[vmii].(map[string]interface{}) if int(vm["vmid"].(float64)) != 0 && int(vm["vmid"].(float64)) == vmId { vmr = NewVmRef(int(vm["vmid"].(float64))) - vmr.node = vm["node"].(string) + vmr.node = NodeName(vm["node"].(string)) vmr.vmType = vm["type"].(string) vmr.pool = "" if vm["pool"] != nil { @@ -321,7 +321,7 @@ func (c *Client) GetVmState(ctx context.Context, vmr *VmRef) (vmState map[string if err != nil { return nil, err } - return c.GetItemConfigMapStringInterface(ctx, "/nodes/"+vmr.node+"/"+vmr.vmType+"/"+strconv.Itoa(vmr.vmId)+"/status/current", "vm", "STATE") + return c.GetItemConfigMapStringInterface(ctx, "/nodes/"+vmr.node.String()+"/"+vmr.vmType+"/"+strconv.Itoa(vmr.vmId)+"/status/current", "vm", "STATE") } func (c *Client) GetVmConfig(ctx context.Context, vmr *VmRef) (vmConfig map[string]interface{}, err error) { @@ -329,7 +329,7 @@ func (c *Client) GetVmConfig(ctx context.Context, vmr *VmRef) (vmConfig map[stri if err != nil { return nil, err } - return c.GetItemConfigMapStringInterface(ctx, "/nodes/"+vmr.node+"/"+vmr.vmType+"/"+strconv.Itoa(vmr.vmId)+"/config", "vm", "CONFIG") + return c.GetItemConfigMapStringInterface(ctx, "/nodes/"+vmr.node.String()+"/"+vmr.vmType+"/"+strconv.Itoa(vmr.vmId)+"/config", "vm", "CONFIG") } func (c *Client) GetStorageStatus(ctx context.Context, vmr *VmRef, storageName string) (storageStatus map[string]interface{}, err error) { @@ -588,7 +588,7 @@ func (c *Client) DeleteVmParams(ctx context.Context, vmr *VmRef, params map[stri return } -func (c *Client) CreateQemuVm(ctx context.Context, node string, vmParams map[string]interface{}) (exitStatus string, err error) { +func (c *Client) CreateQemuVm(ctx context.Context, node NodeName, vmParams map[string]interface{}) (exitStatus string, err error) { // Create VM disks first to ensure disks names. createdDisks, createdDisksErr := c.createVMDisks(ctx, node, vmParams) if createdDisksErr != nil { @@ -745,7 +745,7 @@ func (c *Client) RollbackQemuVm(vmr *VmRef, snapshot string) (exitStatus string, // DEPRECATED SetVmConfig - send config options func (c *Client) SetVmConfig(vmr *VmRef, params map[string]interface{}) (exitStatus interface{}, err error) { - return c.PostWithTask(context.Background(), params, "/nodes/"+vmr.node+"/"+vmr.vmType+"/"+strconv.Itoa(vmr.vmId)+"/config") + return c.PostWithTask(context.Background(), params, "/nodes/"+vmr.node.String()+"/"+vmr.vmType+"/"+strconv.Itoa(vmr.vmId)+"/config") } // SetLxcConfig - send config options @@ -767,9 +767,9 @@ func (c *Client) SetLxcConfig(ctx context.Context, vmr *VmRef, vmParams map[stri } // MigrateNode - Migrate a VM -func (c *Client) MigrateNode(ctx context.Context, vmr *VmRef, newTargetNode string, online bool) (exitStatus interface{}, err error) { +func (c *Client) MigrateNode(ctx context.Context, vmr *VmRef, newTargetNode NodeName, online bool) (exitStatus interface{}, err error) { reqbody := ParamsToBody(map[string]interface{}{"target": newTargetNode, "online": online, "with-local-disks": true}) - url := fmt.Sprintf("/nodes/%s/%s/%d/migrate", vmr.node, vmr.vmType, vmr.vmId) + url := fmt.Sprintf("/nodes/%s/%s/%d/migrate", vmr.node.String(), vmr.vmType, vmr.vmId) resp, err := c.session.Post(ctx, url, nil, nil, &reqbody) if err == nil { taskResponse, err := ResponseJSON(resp) @@ -938,7 +938,7 @@ func (c *Client) VMIdExists(ctx context.Context, vmID int) (exists bool, err err // CreateVMDisk - Create single disk for VM on host node. func (c *Client) CreateVMDisk( ctx context.Context, - nodeName string, + nodeName NodeName, storageName string, fullDiskName string, diskParams map[string]interface{}, @@ -966,7 +966,7 @@ var rxStorageModels = regexp.MustCompile(`(ide|sata|scsi|virtio)\d+`) // createVMDisks - Make disks parameters and create all VM disks on host node. func (c *Client) createVMDisks( ctx context.Context, - node string, + node NodeName, vmParams map[string]interface{}, ) (disks []string, err error) { var createdDisks []string @@ -1020,7 +1020,7 @@ func (c *Client) CreateNewDisk(ctx context.Context, vmr *VmRef, disk string, vol // so mainly this is used to delete the disks in case VM creation didn't complete. func (c *Client) DeleteVMDisks( ctx context.Context, - node string, + node NodeName, disks []string, ) error { for _, fullDiskName := range disks { diff --git a/proxmox/config_guest.go b/proxmox/config_guest.go index 12a0a789..30902642 100644 --- a/proxmox/config_guest.go +++ b/proxmox/config_guest.go @@ -162,7 +162,7 @@ func GuestHasFeature(ctx context.Context, vmr *VmRef, client *Client, feature Gu func guestHasFeature(ctx context.Context, vmr *VmRef, client *Client, feature GuestFeature) (bool, error) { var params map[string]interface{} - params, err := client.GetItemConfigMapStringInterface(ctx, "/nodes/"+vmr.node+"/"+vmr.vmType+"/"+strconv.Itoa(vmr.vmId)+"/feature?feature=snapshot", "guest", "FEATURES") + params, err := client.GetItemConfigMapStringInterface(ctx, "/nodes/"+vmr.node.String()+"/"+vmr.vmType+"/"+strconv.Itoa(vmr.vmId)+"/feature?feature=snapshot", "guest", "FEATURES") if err != nil { return false, err } @@ -226,7 +226,7 @@ func GuestShutdown(ctx context.Context, vmr *VmRef, client *Client, force bool) if force { params = map[string]interface{}{"forceStop": force} } - _, err = client.PostWithTask(ctx, params, "/nodes/"+vmr.node+"/"+vmr.vmType+"/"+strconv.Itoa(vmr.vmId)+"/status/shutdown") + _, err = client.PostWithTask(ctx, params, "/nodes/"+vmr.node.String()+"/"+vmr.vmType+"/"+strconv.Itoa(vmr.vmId)+"/status/shutdown") return } @@ -266,5 +266,5 @@ func pendingGuestConfigFromApi(ctx context.Context, vmr *VmRef, client *Client) if err := client.CheckVmRef(ctx, vmr); err != nil { return nil, err } - return client.GetItemConfigInterfaceArray(ctx, "/nodes/"+vmr.node+"/"+vmr.vmType+"/"+strconv.Itoa(vmr.vmId)+"/pending", "Guest", "PENDING CONFIG") + return client.GetItemConfigInterfaceArray(ctx, "/nodes/"+vmr.node.String()+"/"+vmr.vmType+"/"+strconv.Itoa(vmr.vmId)+"/pending", "Guest", "PENDING CONFIG") } diff --git a/proxmox/config_lxc.go b/proxmox/config_lxc.go index a4d26729..425162fc 100644 --- a/proxmox/config_lxc.go +++ b/proxmox/config_lxc.go @@ -339,7 +339,7 @@ func (config ConfigLxc) CreateLxc(ctx context.Context, vmr *VmRef, client *Clien // amend vmid paramMap["vmid"] = vmr.vmId - exitStatus, err := client.CreateLxcContainer(ctx, vmr.node, paramMap) + exitStatus, err := client.CreateLxcContainer(ctx, vmr.node.String(), paramMap) if err != nil { params, _ := json.Marshal(¶mMap) return fmt.Errorf("error creating LXC container: %v, error status: %s (params: %v)", err, exitStatus, string(params)) diff --git a/proxmox/config_qemu.go b/proxmox/config_qemu.go index fde433eb..31d37ce7 100644 --- a/proxmox/config_qemu.go +++ b/proxmox/config_qemu.go @@ -47,7 +47,7 @@ type ConfigQemu struct { Memory *QemuMemory `json:"memory,omitempty"` Name string `json:"name,omitempty"` // TODO should be custom type as there are character and length limitations Networks QemuNetworkInterfaces `json:"networks,omitempty"` - Node string `json:"node,omitempty"` // Only returned setting it has no effect, set node in the VmRef instead + Node NodeName `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"` @@ -480,7 +480,7 @@ func (newConfig ConfigQemu) setAdvanced(ctx context.Context, currentConfig *Conf if currentConfig != nil { // Update // TODO implement tmp move and version change - url := "/nodes/" + vmr.node + "/" + vmr.vmType + "/" + strconv.Itoa(vmr.vmId) + "/config" + url := "/nodes/" + vmr.node.String() + "/" + vmr.vmType + "/" + strconv.Itoa(vmr.vmId) + "/config" var itemsToDeleteBeforeUpdate string // this is for items that should be removed before they can be created again e.g. cloud-init disks. (convert to array when needed) stopped := false @@ -543,13 +543,13 @@ func (newConfig ConfigQemu) setAdvanced(ctx context.Context, currentConfig *Conf } if newConfig.Node != currentConfig.Node { // Migrate VM - vmr.SetNode(currentConfig.Node) + vmr.node = newConfig.Node _, err = client.MigrateNode(ctx, vmr, newConfig.Node, true) if err != nil { return } // Set node to the node the VM was migrated to - vmr.SetNode(newConfig.Node) + vmr.node = newConfig.Node } rebootRequired, params, err = newConfig.mapToAPI(*currentConfig, version) diff --git a/proxmox/data_qemu_agent.go b/proxmox/data_qemu_agent.go index b8752305..1b618d1f 100644 --- a/proxmox/data_qemu_agent.go +++ b/proxmox/data_qemu_agent.go @@ -12,7 +12,7 @@ func (vmr *VmRef) GetAgentInformation(ctx context.Context, c *Client, statistics } vmid := strconv.FormatInt(int64(vmr.vmId), 10) params, err := c.GetItemConfigMapStringInterface(ctx, - "/nodes/"+vmr.node+"/qemu/"+vmid+"/agent/network-get-interfaces", "guest agent", "data", + "/nodes/"+vmr.node.String()+"/qemu/"+vmid+"/agent/network-get-interfaces", "guest agent", "data", "500 QEMU guest agent is not running", "500 VM "+vmid+" is not running") if err != nil { diff --git a/proxmox/node.go b/proxmox/node.go index 0a4ed96d..404144e0 100644 --- a/proxmox/node.go +++ b/proxmox/node.go @@ -2,11 +2,63 @@ package proxmox import ( "context" + "errors" "fmt" "io" "net/http" ) +// Only the following characters are allowed: "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-". +// May not start with a hyphen. +// May not end with a hyphen. +// Must contain at least one alphabetical character. +// Max length 63 characters. +type NodeName string + +const ( + NodeName_Error_Alphabetical string = "Node name must contain at least one alphabetical character" + NodeName_Error_Empty string = "Node name cannot be empty" + NodeName_Error_HyphenEnd string = "Node name cannot end with a hyphen" + NodeName_Error_HyphenStart string = "Node name cannot start with a hyphen" + NodeName_Error_Illegal string = "Node name may only contain the following characters: abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-" + NodeName_Error_Length string = "Node name must be less than 64 characters" +) + +func (name NodeName) Validate() error { + if name == "" { + return errors.New(NodeName_Error_Empty) + } + if len(name) > 63 { + return errors.New(NodeName_Error_Length) + } + if name[0] == '-' { + return errors.New(NodeName_Error_HyphenStart) + } + if name[len(name)-1] == '-' { + return errors.New(NodeName_Error_HyphenEnd) + } + var hasAlpha bool + for i := range name { + if (name[i] >= 'a' && name[i] <= 'z') || (name[i] >= 'A' && name[i] <= 'Z') { + hasAlpha = true + break + } + } + if !hasAlpha { + return errors.New(NodeName_Error_Alphabetical) + } + for i := range name { + if !((name[i] >= 'a' && name[i] <= 'z') || (name[i] >= 'A' && name[i] <= 'Z') || (name[i] >= '0' && name[i] <= '9') || name[i] == '-') { + return errors.New(NodeName_Error_Illegal) + } + } + return nil +} + +func (name NodeName) String() string { + return string(name) +} + func (c *Client) nodeStatusCommand(ctx context.Context, node, command string) (exitStatus string, err error) { nodes, err := c.GetNodeList(ctx) if err != nil { diff --git a/proxmox/node_test.go b/proxmox/node_test.go new file mode 100644 index 00000000..d50fa39b --- /dev/null +++ b/proxmox/node_test.go @@ -0,0 +1,65 @@ +package proxmox + +import ( + "errors" + "testing" + + "github.com/Telmate/proxmox-api-go/test/data/test_data_node" + "github.com/stretchr/testify/require" +) + +func Test_NodeName_Validate(t *testing.T) { + tests := []struct { + name string + input []string + output error + }{ + {name: `Valid NodeName`, + input: test_data_node.NodeName_Legals()}, + {name: `Invalid Empty`, + input: []string{""}, + output: errors.New(NodeName_Error_Empty)}, + {name: `Invalid Length`, + input: []string{test_data_node.NodeName_Max_Illegal()}, + output: errors.New(NodeName_Error_Length)}, + {name: `Invalid Start Hyphen`, + input: test_data_node.NodeName_StartHyphens(), + output: errors.New(NodeName_Error_HyphenStart)}, + {name: `Invalid End Hyphen`, + input: test_data_node.NodeName_EndHyphens(), + output: errors.New(NodeName_Error_HyphenEnd)}, + {name: `Invalid Alphabetical`, + input: test_data_node.NodeName_Numeric_Illegal(), + output: errors.New(NodeName_Error_Alphabetical)}, + {name: `Invalid Characters`, + input: test_data_node.NodeName_Error_Characters(), + output: errors.New(NodeName_Error_Illegal)}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + for _, input := range test.input { + require.Equal(t, test.output, NodeName(input).Validate()) + } + }) + } +} + +func Test_NodeName_String(t *testing.T) { + tests := []struct { + name string + input NodeName + output string + }{ + {name: `Empty`, + input: "", + output: ""}, + {name: `Valid`, + input: "node1", + output: "node1"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + require.Equal(t, test.output, test.input.String()) + }) + } +} diff --git a/proxmox/snapshot.go b/proxmox/snapshot.go index 54c28d66..ee63c627 100644 --- a/proxmox/snapshot.go +++ b/proxmox/snapshot.go @@ -39,7 +39,7 @@ func (config ConfigSnapshot) Create(ctx context.Context, c *Client, vmr *VmRef) // Create a snapshot without validating the input, use ConfigSnapshot.Create() to validate the input. func (config ConfigSnapshot) Create_Unsafe(ctx context.Context, c *Client, vmr *VmRef) error { params := config.mapToApiValues() - _, err := c.PostWithTask(ctx, params, "/nodes/"+vmr.node+"/"+vmr.vmType+"/"+strconv.Itoa(vmr.vmId)+"/snapshot/") + _, err := c.PostWithTask(ctx, params, "/nodes/"+vmr.node.String()+"/"+vmr.vmType+"/"+strconv.Itoa(vmr.vmId)+"/snapshot/") if err != nil { params, _ := json.Marshal(¶ms) return fmt.Errorf("error creating Snapshot: %v, (params: %v)", err, string(params)) @@ -62,7 +62,7 @@ func ListSnapshots(ctx context.Context, c *Client, vmr *VmRef) (rawSnapshots, er if err := c.CheckVmRef(ctx, vmr); err != nil { return nil, err } - return c.GetItemConfigInterfaceArray(ctx, "/nodes/"+vmr.node+"/"+vmr.vmType+"/"+strconv.Itoa(vmr.vmId)+"/snapshot/", "Guest", "SNAPSHOTS") + return c.GetItemConfigInterfaceArray(ctx, "/nodes/"+vmr.node.String()+"/"+vmr.vmType+"/"+strconv.Itoa(vmr.vmId)+"/snapshot/", "Guest", "SNAPSHOTS") } // Updates the description of the specified snapshot, same as SnapshotName.UpdateDescription() @@ -161,7 +161,7 @@ func (snap SnapshotName) Delete(ctx context.Context, c *Client, vmr *VmRef) (exi // Deletes the specified snapshot without validating the input, use SnapshotName.Delete() to validate the input. func (snap SnapshotName) Delete_Unsafe(ctx context.Context, c *Client, vmr *VmRef) (exitStatus string, err error) { - return c.DeleteWithTask(ctx, "/nodes/"+vmr.node+"/"+vmr.vmType+"/"+strconv.Itoa(vmr.vmId)+"/snapshot/"+string(snap)) + return c.DeleteWithTask(ctx, "/nodes/"+vmr.node.String()+"/"+vmr.vmType+"/"+strconv.Itoa(vmr.vmId)+"/snapshot/"+string(snap)) } // Rollback to the specified snapshot, validates the input @@ -178,7 +178,7 @@ func (snap SnapshotName) Rollback(ctx context.Context, c *Client, vmr *VmRef) (e // Rollback to the specified snapshot without validating the input, use SnapshotName.Rollback() to validate the input. func (snap SnapshotName) Rollback_Unsafe(ctx context.Context, c *Client, vmr *VmRef) (exitStatus string, err error) { - return c.PostWithTask(ctx, nil, "/nodes/"+vmr.node+"/"+vmr.vmType+"/"+strconv.FormatInt(int64(vmr.vmId), 10)+"/snapshot/"+string(snap)+"/rollback") + return c.PostWithTask(ctx, nil, "/nodes/"+vmr.node.String()+"/"+vmr.vmType+"/"+strconv.FormatInt(int64(vmr.vmId), 10)+"/snapshot/"+string(snap)+"/rollback") } // Updates the description of the specified snapshot, validates the input @@ -195,7 +195,7 @@ func (snap SnapshotName) UpdateDescription(ctx context.Context, c *Client, vmr * // Updates the description of the specified snapshot without validating the input, use SnapshotName.UpdateDescription() to validate the input. func (snap SnapshotName) UpdateDescription_Unsafe(ctx context.Context, c *Client, vmr *VmRef, description string) error { - return c.Put(ctx, map[string]interface{}{"description": description}, "/nodes/"+vmr.node+"/"+vmr.vmType+"/"+strconv.Itoa(vmr.vmId)+"/snapshot/"+string(snap)+"/config") + return c.Put(ctx, map[string]interface{}{"description": description}, "/nodes/"+vmr.node.String()+"/"+vmr.vmType+"/"+strconv.Itoa(vmr.vmId)+"/snapshot/"+string(snap)+"/config") } func (name SnapshotName) Validate() error { diff --git a/test/data/test_data_node/type_NodeName.go b/test/data/test_data_node/type_NodeName.go new file mode 100644 index 00000000..8e0f2c02 --- /dev/null +++ b/test/data/test_data_node/type_NodeName.go @@ -0,0 +1,58 @@ +package test_data_node + +import "strings" + +func NodeName_Error_Characters() []string { + characters := strings.Split("`_~!@#$%^&*()=+{}[]|\\;:'\"<,>.?/", "") + for i := range characters { + characters[i] = characters[i] + "node" + } + return characters +} + +func NodeName_Max_Legal() string { + return "abcdefghijklmnopqrstuvwxyz-ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890" +} + +func NodeName_Max_Illegal() string { + return NodeName_Max_Legal() + "A" +} + +func NodeName_Numeric_Illegal() []string { + return []string{ + "1", + "12", + "123", + "12-34", + "1234567-8901234", + } +} + +func nodeName_Legals() []string { + return []string{ + "node1", + "nOde-1", + "nOde1"} +} + +func NodeName_Legals() []string { + return append(nodeName_Legals(), NodeName_Max_Legal()) +} + +func NodeName_StartHyphens() []string { + legals := nodeName_Legals() + hyphen := make([]string, len(legals)) + for i := range legals { + hyphen[i] = "-" + legals[i] + } + return hyphen +} + +func NodeName_EndHyphens() []string { + legals := nodeName_Legals() + hyphen := make([]string, len(legals)) + for i := range legals { + hyphen[i] = legals[i] + "-" + } + return hyphen +}