Add efi_config attribute

To avoid breaking existing templates.
Improve naming of other attributes and add sanity checks.

builder/proxmox/common/config.go

@@ -52,7 +52,8 @@ type Config struct {
 	Sockets        int            `mapstructure:"sockets"`
 	OS             string         `mapstructure:"os"`
 	BIOS           string         `mapstructure:"bios"`
-	EFIDisk        efiConfig      `mapstructure:"efidisk"`
+	EFIConfig      efiConfig      `mapstructure:"efi_config"`
+	EFIDisk        string         `mapstructure:"efidisk"`
 	Machine        string         `mapstructure:"machine"`
 	VGA            vgaConfig      `mapstructure:"vga"`
 	NICs           []nicConfig    `mapstructure:"network_adapters"`
@@ -103,9 +104,9 @@ type diskConfig struct {
 	IOThread        bool   `mapstructure:"io_thread"`
 }
 type efiConfig struct {
-	Storage         string `mapstructure:"storage"`
-	PreEnrolledKeys int    `mapstructure:"pre_enrolled_keys"`
-	EfiType         string `mapstructure:"efitype"`
+	EFIStoragePool  string `mapstructure:"efi_storage_pool"`
+	PreEnrolledKeys bool   `mapstructure:"pre_enrolled_keys"`
+	EFIType         string `mapstructure:"efi_type"`
 }
 type vgaConfig struct {
 	Type   string `mapstructure:"type"`
@@ -351,6 +352,24 @@ func (c *Config) Prepare(upper interface{}, raws ...interface{}) ([]string, []st
 			errs = packersdk.MultiErrorAppend(errs, fmt.Errorf("one of iso_file, iso_url, or a combination of cd_files and cd_content must be specified for AdditionalISO file %s", c.AdditionalISOFiles[idx].Device))
 		}
 	}
+	if c.EFIDisk != "" {
+		if c.EFIConfig != (efiConfig{}) {
+			errs = packersdk.MultiErrorAppend(errs, fmt.Errorf("both efi_config and efidisk cannot be set at the same time, consider defining only efi_config as efidisk is deprecated"))
+		} else {
+			warnings = append(warnings, "efidisk is deprecated, please use efi_config instead")
+			c.EFIConfig.EFIStoragePool = c.EFIDisk
+		}
+	}
+	if c.EFIConfig.EFIStoragePool != "" {
+		if c.EFIConfig.EFIType == "" {
+			log.Printf("EFI disk defined, but no efi_type given, using 4m")
+			c.EFIConfig.EFIType = "4m"
+		}
+	} else {
+		if c.EFIConfig.EFIType != "" || c.EFIConfig.PreEnrolledKeys {
+			errs = packersdk.MultiErrorAppend(errs, errors.New("efi_storage_pool not set for efi_config"))
+		}
+	}
 
 	if errs != nil && len(errs.Errors) > 0 {
 		return nil, warnings, errs

builder/proxmox/common/step_start_vm.go

@@ -60,7 +60,7 @@ func (s *stepStartVM) Run(ctx context.Context, state multistep.StateBag) multist
 		QemuSockets:  c.Sockets,
 		QemuOs:       c.OS,
 		Bios:         c.BIOS,
-		EFIDisk:      generateProxmoxEfi(c.EFIDisk),
+		EFIDisk:      generateProxmoxEfi(c.EFIConfig),
 		Machine:      c.Machine,
 		QemuVga:      generateProxmoxVga(c.VGA),
 		QemuNetworks: generateProxmoxNetworkAdapters(c.NICs),
@@ -124,6 +124,25 @@ func (s *stepStartVM) Run(ctx context.Context, state multistep.StateBag) multist
 		}
 	}
 
+	// The EFI disk doesn't get created reliably during initial VM creation,
+	// so let's make sure it's there.
+	if c.EFIConfig != (efiConfig{}) {
+		addEFIConfig := make(map[string]interface{})
+		err := config.CreateQemuEfiParams(addEFIConfig)
+		if err != nil {
+			err := fmt.Errorf("error creating EFI parameters: %s", err)
+			state.Put("error", err)
+			ui.Error(err.Error())
+		}
+		_, err = client.SetVmConfig(vmRef, addEFIConfig)
+		if err != nil {
+			err := fmt.Errorf("error updating template: %s", err)
+			state.Put("error", err)
+			ui.Error(err.Error())
+			return multistep.ActionHalt
+		}
+	}
+
 	// Store the vm id for later
 	state.Put("vmRef", vmRef)
 	// instance_id is the generic term used so that users can have access to the
@@ -190,9 +209,17 @@ func generateProxmoxVga(vga vgaConfig) proxmox.QemuDevice {
 
 func generateProxmoxEfi(efi efiConfig) proxmox.QemuDevice {
 	dev := make(proxmox.QemuDevice)
-	setDeviceParamIfDefined(dev, "storage", efi.Storage)
-	dev["pre-enrolled-keys"] = efi.PreEnrolledKeys
-	setDeviceParamIfDefined(dev, "efitype", efi.EfiType)
+	setDeviceParamIfDefined(dev, "storage", efi.EFIStoragePool)
+	setDeviceParamIfDefined(dev, "efitype", efi.EFIType)
+	// efi.PreEnrolledKeys can be false, but we only want to set pre-enrolled-keys=0
+	// when other EFI options are set.
+	if len(dev) > 0 {
+		if efi.PreEnrolledKeys {
+			dev["pre-enrolled-keys"] = "1"
+		} else {
+			dev["pre-enrolled-keys"] = "0"
+		}
+	}
 	return dev
 }
 

docs-partials/builder/proxmox/common/Config-not-required.mdx

@@ -36,7 +36,9 @@
 
 - `bios` (string) - BIOS
 
-- `efidisk` (efiConfig) - EFI Disk
+- `efi_config` (efiConfig) - EFI Config
+
+- `efidisk` (string) - EFI Disk
 
 - `machine` (string) - Machine
 

docs-partials/builder/proxmox/common/efiConfig-not-required.mdx

@@ -1,9 +1,9 @@
 <!-- Code generated from the comments of the efiConfig struct in builder/proxmox/common/config.go; DO NOT EDIT MANUALLY -->
 
-- `storage` (string) - Storage
+- `efi_storage_pool` (string) - EFI Storage Pool
 
-- `pre_enrolled_keys` (int) - Pre Enrolled Keys
+- `pre_enrolled_keys` (bool) - Pre Enrolled Keys
 
-- `efitype` (string) - Efi Type
+- `efi_type` (string) - EFI Type
 
 <!-- End of code generated from the comments of the efiConfig struct in builder/proxmox/common/config.go; -->

docs/builders/clone.mdx

@@ -267,17 +267,29 @@ in the image's Cloud-Init settings for provisioning.
 
 - `bios` - (string) - Set the machine bios. This can be set to ovmf or seabios. The default value is seabios.
 
-- `efidisk` - (object) - Set the efidisk storage parameters. This need to be set if you use ovmf uefi.
+- `efi_config` - (object) - Set the efidisk storage options. This needs to be set if you use ovmf uefi boot
+  (supersedes the `efidisk` option).
+
   Usage example (JSON):
 
   ```json
   {
-    "storage": "local",
-    "pre_enrolled_keys": "1",
-    "efitype": "4m"
+    "efi_storage_pool": "local",
+    "pre_enrolled_keys": true,
+    "efi_type": "4m"
   }
   ```
 
+  - `storage_pool` - (string) - Name of the Proxmox storage pool to store the EFI disk on.
+
+  - `efitype` - (string) - Specifies the version of the OVMF firmware to be used. Can be `2m` or `4m`.
+    Defaults to `4m`.
+
+  - `pre_enrolled_keys` - (boolean) - Whether Microsoft Standard Secure Boot keys should be pre-loaded on
+    the EFI disk. Defaults to `false`.
+
+- `efidisk` - (string) - This option is deprecated, please use `efi_config` instead.
+
 - `machine` - (string) - Set themachine type. i440fx or q35.
 
 ## Example: Cloud-Init enabled Debian

docs/builders/iso.mdx

@@ -347,17 +347,29 @@ in the image's Cloud-Init settings for provisioning.
 
 - `bios` - (string) - Set the machine bios. This can be set to ovmf or seabios. The default value is seabios.
 
-- `efidisk` - (object) - Set the efidisk storage options. This need to be set if you use ovmf uefi boot
+- `efi_config` - (object) - Set the efidisk storage options. This needs to be set if you use ovmf uefi boot
+  (supersedes the `efidisk` option).
+
   Usage example (JSON):
 
   ```json
   {
-    "storage": "local",
-    "pre_enrolled_keys": "1",
-    "efitype": "4m"
+    "efi_storage_pool": "local",
+    "pre_enrolled_keys": true,
+    "efi_type": "4m"
   }
   ```
 
+  - `storage_pool` - (string) - Name of the Proxmox storage pool to store the EFI disk on.
+
+  - `efitype` - (string) - Specifies the version of the OVMF firmware to be used. Can be `2m` or `4m`.
+    Defaults to `4m`.
+
+  - `pre_enrolled_keys` - (boolean) - Whether Microsoft Standard Secure Boot keys should be pre-loaded on
+    the EFI disk. Defaults to `false`.
+
+- `efidisk` - (string) - This option is deprecated, please use `efi_config` instead.
+
 - `machine` - (string) - Set themachine type. i440fx or q35.
 
 ## Boot Command