Skip to content

Commit

Permalink
Refactor Libvirt Firmware Handling for Better Testability
Browse files Browse the repository at this point in the history
- Moved firmware logic into dedicated methods.
- Updated tests for improved coverage and workflow alignment.
  • Loading branch information
nofaralfasi committed Sep 9, 2024
1 parent 4c40ca9 commit 81a503a
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 21 deletions.
45 changes: 24 additions & 21 deletions app/models/compute_resources/foreman/model/libvirt.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -311,7 +293,7 @@ def vm_instance_defaults
:password => random_password(console_password_length(display_type)),
:port => '-1' },
:firmware => 'automatic',
:firmware_features => { "secure-boot" => "no", }
:firmware_features => { "secure-boot" => "no" }
)
end

Expand Down Expand Up @@ -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
51 changes: 51 additions & 0 deletions test/models/compute_resources/libvirt_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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) }

Expand Down

0 comments on commit 81a503a

Please sign in to comment.