From 1e88c545eb68473cd1d9f97798fa09781531abae Mon Sep 17 00:00:00 2001 From: nofaralfasi Date: Mon, 22 Jul 2024 10:45:13 +0300 Subject: [PATCH] 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 | 9 +++ .../foreman/model/libvirt.rb | 62 ++++++++++--------- app/models/concerns/pxe_loader_support.rb | 2 + .../form/libvirt/_base.html.erb | 31 +++------- .../javascripts/compute_resource/libvirt.js | 7 --- 5 files changed, 52 insertions(+), 59 deletions(-) diff --git a/app/models/compute_resource.rb b/app/models/compute_resource.rb index a555156e9bd..6f9c15caff2 100644 --- a/app/models/compute_resource.rb +++ b/app/models/compute_resource.rb @@ -399,6 +399,15 @@ def normalize_vm_attrs(vm_attrs) vm_attrs end + def firmware_types + { + "automatic" => N_("Automatic"), + "bios" => N_("BIOS"), + "uefi" => N_("UEFI"), + "uefi_secure_boot" => N_("UEFI Secure Boot"), + }.freeze + end + protected def memory_gb_to_bytes(memory_size) diff --git a/app/models/compute_resources/foreman/model/libvirt.rb b/app/models/compute_resources/foreman/model/libvirt.rb index 7030410eba2..9cbfd0eda2c 100644 --- a/app/models/compute_resources/foreman/model/libvirt.rb +++ b/app/models/compute_resources/foreman/model/libvirt.rb @@ -11,29 +11,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 @@ -170,6 +147,23 @@ def new_vm(attr = { }) opts[:boot_order] = %w[hd] opts[:boot_order].unshift 'network' unless attr[:image_id] + # This value comes from PxeLoaderSupport#firmware_type + firmware_type = opts.delete(:firmware_type).to_s + + # automatic firmware type is determined by the PXE Loader + # if no PXE Loader is set, we will set it to bios by default + if opts[:firmware] == 'automatic' + opts[:firmware] = (firmware_type == 'none' || firmware_type.empty?) ? 'bios' : firmware_type + end + + # Adjust firmware and secure_boot values for Libvirt compatibility + if opts[:firmware] == 'uefi_secure_boot' + opts[:firmware_features] = { 'secure-boot' => 'yes', 'enrolled-keys' => 'yes' } + opts[:loader] = { 'secure' => 'yes' } + end + # Libvirt expects the firmware type to be 'efi' + opts[:firmware] = 'efi' if opts[:firmware]&.start_with?('uefi') + vm = client.servers.new opts vm.memory = opts[:memory] if opts[:memory] vm @@ -281,6 +275,21 @@ def disconnect self.class.terminate_connection(url) end + # TODO - requires fog-libvirt to be updated to support this + # Returns the firmware type based on the VM object + def firmware_type(vm_obj) + require 'pry-byebug'; binding.pry + if vm_obj.firmware == 'efi' + if vm_obj.secure_boot + 'uefi_secure_boot' # special case for secure boot + else + 'uefi' + end + else + vm_obj.firmware + end + end + protected def libvirt_connection_error @@ -311,11 +320,8 @@ 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 => { - "secure-boot" => "no", - "enrolled-keys" => "no", - } + :firmware => 'automatic', + :firmware_features => { "secure-boot" => "no", } ) end diff --git a/app/models/concerns/pxe_loader_support.rb b/app/models/concerns/pxe_loader_support.rb index c98632d7691..5fbeed8fbef 100644 --- a/app/models/concerns/pxe_loader_support.rb +++ b/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_secure_boot when /UEFI/ :uefi else diff --git a/app/views/compute_resources_vms/form/libvirt/_base.html.erb b/app/views/compute_resources_vms/form/libvirt/_base.html.erb index d48502854fb..eb0f16c342d 100644 --- a/app/views/compute_resources_vms/form/libvirt/_base.html.erb +++ b/app/views/compute_resources_vms/form/libvirt/_base.html.erb @@ -8,6 +8,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_type = new_vm ? 'automatic' : compute_resource.firmware_type(f.object) %> +<%= field(f, :firmware, :disabled => !new_vm, :label => _('Firmware'), :label_help => _("Choose 'Automatic' to set the firmware based on the PXE Loader. If no PXE Loader is selected, it defaults to BIOS."), :label_size => "col-md-2") do + compute_resource.firmware_types.collect do |type, name| + radio_button_f f, :firmware, {:checked => (firmware_type == type), :disabled => !new_vm, :value => type, :text => _(name) } + end.join(' ').html_safe +end %> + <% arch ||= nil ; os ||= nil images = possible_images(compute_resource, arch, os) @@ -23,27 +30,3 @@ <%= 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' -%> - -
-<%= 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 -%> -
diff --git a/webpack/assets/javascripts/compute_resource/libvirt.js b/webpack/assets/javascripts/compute_resource/libvirt.js index 539407dcb36..d56714df610 100644 --- a/webpack/assets/javascripts/compute_resource/libvirt.js +++ b/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'); -}