Refactor Secure Boot support for Libvirt

- Updated the Libvirt VM creation form.
- Added a new firmware type for Secure Boot.
- Enable `enrolled-keys` by default when Secure Boot is activated.
- Removed unnecessary methods from the Libvirt model.
- Moved firmware-related methods to the ComputeResource model
  for shared use between VMware and Libvirt.

app/models/compute_resource.rb

@@ -399,6 +399,26 @@ def normalize_vm_attrs(vm_attrs)
     vm_attrs
   end
 
+  def firmware_types
+    {
+      "automatic" => N_("Automatic"),
+      "bios" => N_("BIOS"),
+      "efi" => N_("EFI"),
+      "uefi_sb" => N_("UEFI Secure Boot"),
+    }.freeze
+  end
+
+  def firmware_mapping(firmware_type)
+    case firmware_type
+    when :uefi
+      'efi'
+    when :uefi_sb
+      'uefi_sb'
+    else
+      'bios'
+    end
+  end
+
   protected
 
   def memory_gb_to_bytes(memory_size)

app/models/compute_resources/foreman/model/libvirt.rb

@@ -14,29 +14,6 @@ def self.available?
       Fog::Compute.providers.include?(:libvirt)
     end
 
-    def self.firmware_types
-      {
-        "efi" => N_("EFI"),
-        "bios" => N_("BIOS"),
-      }.freeze
-    end
-
-    def os_firmware
-      attrs[:os_firmware].presence || "efi"
-    end
-
-    def os_firmware=(firmware)
-      attrs[:os_firmware] = firmware
-    end
-
-    def os_firmware_features
-      attrs[:os_firmware_features].presence || {}
-    end
-
-    def os_firmware_features=(attrs)
-      attrs[:os_firmware_features].merge attrs
-    end
-
     def display_type
       attrs[:display].presence || 'vnc'
     end
@@ -173,6 +150,14 @@ def new_vm(attr = { })
       opts[:boot_order] = %w[hd]
       opts[:boot_order].unshift 'network' unless attr[:image_id]
 
+      firmware_type = opts.delete(:firmware_type)
+      opts[:firmware] = firmware_mapping(firmware_type) if opts[:firmware] == 'automatic'
+
+      if opts[:firmware] == 'uefi_sb'
+        opts[:firmware] = 'efi'
+        opts[:firmware_features] = { 'secure-boot' => 'yes', 'enrolled-keys' => 'yes' }
+      end
+
       vm = client.servers.new opts
       vm.memory = opts[:memory] if opts[:memory]
       vm
@@ -284,6 +269,14 @@ def disconnect
       self.class.terminate_connection(url)
     end
 
+    def firmware(firmware, firmware_features)
+      if firmware == 'efi' && firmware_features['secure-boot'] == 'yes' && firmware_features['enrolled-keys'] == 'yes'
+        'uefi_sb' # special case for secure boot
+      else
+        firmware
+      end
+    end
+
     protected
 
     def libvirt_connection_error
@@ -314,10 +307,9 @@ def vm_instance_defaults
                          :listen   => Setting[:libvirt_default_console_address],
                          :password => random_password(console_password_length(display_type)),
                          :port     => '-1' },
-        :os_firmware => 'efi',
-        :os_firmware_features => {
+        :firmware => 'automatic',
+        :firmware_features => {
           "secure-boot" => "no",
-          "enrolled-keys" => "no",
         }
       )
     end

app/models/concerns/pxe_loader_support.rb

@@ -50,6 +50,8 @@ def firmware_type(pxe_loader)
       case pxe_loader
       when 'None'
         :none
+      when /SecureBoot/
+        :uefi_sb
       when /UEFI/
         :uefi
       else

app/views/compute_resources_vms/form/libvirt/_base.html.erb

@@ -9,6 +9,13 @@
 <% checked = params[:host] && params[:host][:compute_attributes] && params[:host][:compute_attributes][:start] || '1' %>
 <%= checkbox_f f, :start, { :checked => (checked == '1'), :help_inline => _("Power ON this machine"), :label => _('Start'), :label_size => "col-md-2"} if new_vm && controller_name != "compute_attributes" %>
 
+<% firmware = compute_resource.firmware(f.object.firmware, f.object.firmware_features) %>
+<%= field(f, :firmware, :disabled => !new_vm, :label => _('Firmware'), :label_size => "col-md-2") do
+  compute_resource.firmware_types.collect do |type, name|
+    radio_button_f f, :firmware, {:checked => (firmware == type), :disabled => !new_vm, :value => type, :text => _(name) }
+  end.join(' ').html_safe
+end %>
+
 <%
    arch ||= nil ; os ||= nil
    images = possible_images(compute_resource, arch, os)
@@ -24,27 +31,3 @@
 </div>
 
 <%= compute_specific_js(compute_resource, "nic_info") %>
-
-<%= select_f f, :os_firmware,
-                Foreman::Model::Libvirt.firmware_types,
-                :first,
-                :last,
-                {},
-                { :label => _("Firmware"),
-                  :label_size => "col-md-2",
-                  :onchange => "tfm.computeResource.libvirt.firmwareSelected(this);",
-                }
-%>
-<%
-  feature_attrs = ActiveSupport::HashWithIndifferentAccess.new(f.object.os_firmware_features)
-  is_bios = f.object.os_firmware == 'bios'
-%>
-
-<div id="os_firmware_features">
-<%= f.fields_for :os_firmware_features do |ff|
-    secure_field = checkbox_f(ff, 'secure-boot', { checked: feature_attrs['secure-boot'] == 'yes', label: _("Secure Boot"), disabled: is_bios }, 'yes', 'no')
-    keys_field = checkbox_f(ff, 'enrolled-keys', { checked: feature_attrs['enrolled-keys'] == 'yes', label: _("Enrolled keys"), disabled: is_bios }, 'yes', 'no')
-    secure_field + keys_field
-  end
-%>
-</div>

webpack/assets/javascripts/compute_resource/libvirt.js

@@ -118,10 +118,3 @@ export function allocationSwitcher(element, action) {
   $(element).button('toggle');
   return false;
 }
-
-export function firmwareSelected(item) {
-  const selected = $(item).val();
-  const inputs = $("#os_firmware_features input[type='checkbox']");
-
-  inputs.attr('disabled', selected === 'bios');
-}