diff --git a/app/models/compute_resources/foreman/model/libvirt.rb b/app/models/compute_resources/foreman/model/libvirt.rb index e843dff51c1..78271ce0317 100644 --- a/app/models/compute_resources/foreman/model/libvirt.rb +++ b/app/models/compute_resources/foreman/model/libvirt.rb @@ -147,26 +147,8 @@ 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' instead of 'uefi' - if opts[:firmware]&.start_with?('uefi') - opts[:firmware] = 'efi' - else - opts[:firmware] = 'bios' - end + handle_automatic_firmware(opts) + configure_firmware_settings(opts) vm = client.servers.new opts vm.memory = opts[:memory] if opts[:memory] @@ -348,5 +330,26 @@ def validate_volume_capacity(vol) raise Foreman::Exception.new(N_("Please specify volume size. You may optionally use suffix 'G' to specify volume size in gigabytes.")) end end + + def handle_automatic_firmware(opts) + # This value comes from PxeLoaderSupport#firmware_type + firmware_type = opts.delete(:firmware_type).to_s + + # Handle automatic firmware setting based on PXE Loader + # if no PXE Loader is set, we will set it to bios by default + if opts[:firmware] == 'automatic' + opts[:firmware] = firmware_type.presence || 'bios' + end + end + + def configure_firmware_settings(opts) + # Adjust firmware and secure boot settings for Libvirt compatibility + if opts[:firmware] == 'uefi_secure_boot' + opts[:firmware_features] = { 'secure-boot' => 'yes', 'enrolled-keys' => 'yes' } + opts[:loader] = { 'secure' => 'yes' } + end + + opts[:firmware] = opts[:firmware]&.start_with?('uefi') ? 'efi' : 'bios' + end end end diff --git a/test/models/compute_resources/libvirt_test.rb b/test/models/compute_resources/libvirt_test.rb index 9bf15229ad9..fc6fdad458a 100644 --- a/test/models/compute_resources/libvirt_test.rb +++ b/test/models/compute_resources/libvirt_test.rb @@ -124,6 +124,57 @@ class Foreman::Model::LibvirtTest < ActiveSupport::TestCase end end + describe '#handle_automatic_firmware' do + setup do + @cr = FactoryBot.build_stubbed(:libvirt_cr) + end + + test 'sets firmware based on automatic and firmware_type scenarios' do + scenarios = [ + [{ firmware: 'automatic' }, { firmware: 'bios' }], + [{ firmware: 'automatic', firmware_type: :bios }, { firmware: 'bios' }], + [{ firmware: 'automatic', firmware_type: :uefi }, { firmware: 'uefi' }], + [{ firmware: 'automatic', firmware_type: :uefi_secure_boot }, { firmware: 'uefi_secure_boot' }], + ] + + scenarios.each do |input, expected| + @cr.send(:handle_automatic_firmware, input) + assert_equal(expected, input) + end + end + + test 'does not change firmware if it is not automatic' do + attrs_in = { firmware_type: :uefi_secure_boot, firmware: 'uefi' } + @cr.send(:handle_automatic_firmware, attrs_in) + assert_equal({ firmware: 'uefi' }, attrs_in) + end + end + + describe '#configure_firmware_settings' do + setup do + @cr = FactoryBot.build_stubbed(:libvirt_cr) + end + + test 'sets firmware to BIOS when no firmware is provided' do + attrs_in = { firmware: '' } + @cr.send(:configure_firmware_settings, attrs_in) + assert_equal({ firmware: 'bios' }, attrs_in) + end + + test 'sets firmware to EFI when firmware_type is uefi' do + attrs_in = { firmware: 'uefi' } + @cr.send(:configure_firmware_settings, attrs_in) + assert_equal({ firmware: 'efi' }, attrs_in) + end + + test 'sets EFI firmware with SecureBoot enabled and secure loader when firmware type is uefi_secure_boot' do + attrs_in = { firmware: 'uefi_secure_boot' } + attrs_out = { firmware: 'efi', firmware_features: { 'secure-boot' => 'yes', 'enrolled-keys' => 'yes' }, loader: { 'secure' => 'yes' } } + @cr.send(:configure_firmware_settings, attrs_in) + assert_equal attrs_out, attrs_in + end + end + describe '#new_volume' do let(:cr) { FactoryBot.build_stubbed(:libvirt_cr) }