Check if vm_id is in valid range.

Technically Proxmox VMIDs are unsigned 32bit integers, but are limited to the range 100-999999999. Source: https://pve-devel.pve.proxmox.narkive.com/Pa6mH1OP/avoiding-vmid-reuse#post8 Check if vm_id is in that range.
hashicorp · Feb 24, 2023 · 9cb889b · 9cb889b
1 parent 4842a84
commit 9cb889b
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 4 deletions.
diff --git a/builder/proxmox/common/config.go b/builder/proxmox/common/config.go
@@ -169,6 +169,12 @@ func (c *Config) Prepare(upper interface{}, raws ...interface{}) ([]string, []st
 		c.BootKeyInterval = 5 * time.Millisecond
 	}
 
+	// Technically Proxmox VMIDs are unsigned 32bit integers, but are limited to
+	// the range 100-999999999. Source:
+	// https://pve-devel.pve.proxmox.narkive.com/Pa6mH1OP/avoiding-vmid-reuse#post8
+	if c.VMID != 0 && (c.VMID < 100 || c.VMID > 999999999) {
+		errs = packersdk.MultiErrorAppend(errs, errors.New("vm_id must be in range 100-999999999"))
+	}
 	if c.VMName == "" {
 		// Default to packer-[time-ordered-uuid]
 		c.VMName = fmt.Sprintf("packer-%s", uuid.TimeOrderedUUID())

diff --git a/builder/proxmox/common/config_test.go b/builder/proxmox/common/config_test.go
@@ -286,3 +286,52 @@ func TestSerials(t *testing.T) {
 		})
 	}
 }
+
+func TestVMID(t *testing.T) {
+	serialsTest := []struct {
+		name          string
+		VMID          int
+		expectFailure bool
+	}{
+		{
+			name:          "VMID zero, no error",
+			expectFailure: false,
+			VMID:          0,
+		},
+		{
+			name:          "VMID in range, no error",
+			expectFailure: false,
+			VMID:          1000,
+		},
+		{
+			name:          "VMID above range, fail",
+			expectFailure: true,
+			VMID:          1000000000,
+		},
+		{
+			name:          "VMID below range, fail",
+			expectFailure: true,
+			VMID:          50,
+		},
+	}
+
+	for _, tt := range serialsTest {
+		t.Run(tt.name, func(t *testing.T) {
+			cfg := mandatoryConfig(t)
+			cfg["vm_id"] = tt.VMID
+
+			var c Config
+			_, _, err := c.Prepare(&c, cfg)
+			if err != nil {
+				if !tt.expectFailure {
+					t.Fatalf("unexpected failure to prepare config: %s", err)
+				}
+				t.Logf("got expected failure: %s", err)
+			}
+
+			if err == nil && tt.expectFailure {
+				t.Errorf("expected failure, but prepare succeeded")
+			}
+		})
+	}
+}
diff --git a/docs/builders/clone.mdx b/docs/builders/clone.mdx
@@ -87,8 +87,9 @@ in the image's Cloud-Init settings for provisioning.
   given, a random uuid will be used.
 
 - `vm_id` (int) - The ID used to reference the virtual machine. This will
-  also be the ID of the final template. If not given, the next free ID on
-  the node will be used.
+  also be the ID of the final template. Proxmox VMIDs are unique cluster-wide
+  and are limited to the range 100-999999999.
+  If not given, the next free ID on the cluster will be used.
 
 - `memory` (int) - How much memory, in megabytes, to give the virtual
   machine. Defaults to `512`.

diff --git a/docs/builders/iso.mdx b/docs/builders/iso.mdx
@@ -98,8 +98,9 @@ in the image's Cloud-Init settings for provisioning.
   given, a random uuid will be used.
 
 - `vm_id` (int) - The ID used to reference the virtual machine. This will
-  also be the ID of the final template. If not given, the next free ID on
-  the node will be used.
+  also be the ID of the final template. Proxmox VMIDs are unique cluster-wide
+  and are limited to the range 100-999999999.
+  If not given, the next free ID on the cluster will be used.
 
 - `memory` (int) - How much memory, in megabytes, to give the virtual
   machine. Defaults to `512`.