Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #37566 - Libvirt - UEFI & SecureBoot support #10209

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions app/models/compute_resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,24 @@ 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

# Returns the firmware type based on the VM object
def firmware_type(vm_obj)
if vm_obj.firmware == 'efi'
vm_obj.secure_boot ? 'uefi_secure_boot' : 'uefi' # special case for secure boot
else
vm_obj.firmware
end
end

protected

def memory_gb_to_bytes(memory_size)
Expand Down
29 changes: 27 additions & 2 deletions app/models/compute_resources/foreman/model/libvirt.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ def self.available?
Fog::Compute.providers.include?(:libvirt)
end

# Some getters/setters for the attrs Hash
def display_type
attrs[:display].presence || 'vnc'
end
Expand Down Expand Up @@ -148,6 +147,9 @@ def new_vm(attr = { })
opts[:boot_order] = %w[hd]
opts[:boot_order].unshift 'network' unless attr[:image_id]

handle_automatic_firmware(opts)
configure_firmware_settings(opts)

vm = client.servers.new opts
vm.memory = opts[:memory] if opts[:memory]
vm
Expand Down Expand Up @@ -289,7 +291,9 @@ def vm_instance_defaults
:display => { :type => display_type,
:listen => Setting[:libvirt_default_console_address],
:password => random_password(console_password_length(display_type)),
:port => '-1' }
:port => '-1' },
:firmware => 'automatic',
:firmware_features => { "secure-boot" => "no" }
)
end

Expand Down Expand Up @@ -326,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
Comment on lines +334 to +343
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is also used in https://github.com/theforeman/foreman/pull/10225/files#diff-be4ac4f95deb530eb0fbcc834a4940573ba45b8d5923433305ac3aa1e0703328R501-R508.
I suggest moving this method into app/models/compute_resource.rb for shared use between VMware and Libvirt.


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
2 changes: 2 additions & 0 deletions app/models/concerns/pxe_loader_support.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ def firmware_type(pxe_loader)
case pxe_loader
when 'None'
:none
when /SecureBoot/
:uefi_secure_boot
when /UEFI/
:uefi
else
Expand Down
7 changes: 7 additions & 0 deletions app/views/compute_resources_vms/form/libvirt/_base.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 3 additions & 1 deletion bundler.d/libvirt.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
group :libvirt do
gem 'fog-libvirt', '>= 0.12.0'
# gem 'fog-libvirt', '>= 0.9.0'
gem 'ruby-libvirt', '~> 0.5', :require => 'libvirt'
# TODO: Remove this line after merging the PR
gem 'fog-libvirt', github: 'nofaralfasi/fog-libvirt', branch: 'sb_libvirt'
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
Loading