Skip to content

Commit

Permalink
Refactor Secure Boot & TPM Support for VMware
Browse files Browse the repository at this point in the history
- Added a new firmware type for Secure Boot.
- Handles TPM conflict with BIOS.
- Removed unnecessary methods from the VMware model.
  • Loading branch information
nofaralfasi committed Aug 8, 2024
1 parent 6054631 commit 8d99d94
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 34 deletions.
57 changes: 31 additions & 26 deletions app/models/compute_resources/foreman/model/vmware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -206,8 +206,18 @@ def firmware_types
{
"automatic" => N_("Automatic"),
"bios" => N_("BIOS"),
"efi" => N_("EFI"),
}
"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"
else
vm_obj.firmware
end
end

def disk_mode_types
Expand Down Expand Up @@ -488,8 +498,24 @@ def parse_args(args)

args.except!(:hardware_version) if args[:hardware_version] == 'Default'

firmware_type = args.delete(:firmware_type)
args[:firmware] = firmware_mapping(firmware_type) if args[:firmware] == 'automatic'
# This value comes from PxeLoaderSupport#firmware_type
firmware_type = args.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 args[:firmware] == 'automatic'
args[:firmware] = (firmware_type == 'none' || firmware_type.empty?) ? 'bios' : firmware_type
end

# Adjust firmware and secure_boot values for VMware compatibility
args[:secure_boot] = true if args[:firmware] == 'uefi_secure_boot'
# VMware expects the firmware type to be 'efi'
args[:firmware] = 'efi' if args[:firmware]&.start_with?('uefi')

args[:virtual_tpm] = ActiveRecord::Type::Boolean.new.cast(args[:virtual_tpm])
if args[:firmware] == 'bios' && args[:virtual_tpm]
errors.add :base, _('TPM is not compatible with BIOS firmware. Please change Firmware or disable TPM.')
end

args.reject! { |k, v| v.nil? }
args
Expand Down Expand Up @@ -522,7 +548,7 @@ def create_vm(args = { })
clone_vm(args)
else
vm = new_vm(args)
vm.firmware = 'bios' if vm.firmware == 'automatic'
raise ArgumentError, errors.full_messages.join(';') if errors.any?
vm.save
end
rescue Fog::Vsphere::Compute::NotFound => e
Expand Down Expand Up @@ -772,22 +798,6 @@ def normalize_vm_attrs(vm_attrs)
normalized
end

def secure_boot
attrs[:secure_boot] ||= false
end

def secure_boot=(enabled)
attrs[:secure_boot] = ActiveRecord::Type::Boolean.new.cast(enabled)
end

def virtual_tpm
attrs[:virtual_tpm] ||= false
end

def virtual_tpm=(enabled)
attrs[:virtual_tpm] = ActiveRecord::Type::Boolean.new.cast(enabled)
end

private

def dc
Expand Down Expand Up @@ -844,11 +854,6 @@ def vm_instance_defaults
)
end

def firmware_mapping(firmware_type)
return 'efi' if firmware_type == :uefi
'bios'
end

def set_vm_volumes_attributes(vm, vm_attrs)
volumes = vm.volumes || []
vm_attrs[:volumes_attributes] = Hash[volumes.each_with_index.map { |volume, idx| [idx.to_s, volume.attributes.merge(:size_gb => volume.size_gb)] }]
Expand Down
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
10 changes: 4 additions & 6 deletions app/views/compute_resources_vms/form/vmware/_base.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@
<%= counter_f(f, :corespersocket, label: _('Cores per socket'), recommended_max_value: compute_resource.max_cpu_count, value: f.object.corespersocket || 1) %>
</div>
<%= text_f f, :memory_mb, :class => "col-md-2", :label => _("Memory (MB)") %>

<% firmware_type = compute_resource.firmware_type(f.object) %>
<%= field(f, :firmware, :label => _('Firmware'), :label_size => "col-md-2") do
compute_resource.firmware_types.collect do |type, name|
radio_button_f f, :firmware, {:disabled => !new_vm, :value => type, :text => _(name)}
radio_button_f f, :firmware, {:checked => (firmware_type == type), :disabled => !new_vm, :value => type, :text => _(name)}
end.join(' ').html_safe
end %>
<%= selectable_f f, :cluster, compute_resource.clusters, { :include_blank => _('Please select a cluster') },
Expand Down Expand Up @@ -49,13 +51,9 @@ end %>
{ :disabled => images.empty?, :label => _('Image'), :label_size => "col-md-2" } %>
</div>

<%= checkbox_f f, :secure_boot, { :help_inline => _("Enable Secure Bott for provisioning."),
:label => _('Secure Boot'),
:label_size => "col-md-2",
:disabled => !new_vm } %>

<%= checkbox_f f, :virtual_tpm, { :help_inline => _("Add Virtual TPM module to the VM."),
:label => _('Virtual TPM'),
:label_help => _("Only compatible with UEFI firmware."),
:label_size => "col-md-2",
:disabled => !new_vm } %>

Expand Down
22 changes: 20 additions & 2 deletions test/models/compute_resources/vmware_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ class Foreman::Model::VmwareTest < ActiveSupport::TestCase

mock_vm = mock('vm')
mock_vm.expects(:save).returns(mock_vm)
mock_vm.expects(:firmware).returns('biod')

cr = FactoryBot.build_stubbed(:vmware_cr)
cr.expects(:parse_networks).with(attrs_in).returns(attrs_parsed)
Expand Down Expand Up @@ -151,7 +150,6 @@ class Foreman::Model::VmwareTest < ActiveSupport::TestCase
mock_vm = mock('vm')
mock_vm.expects(:save).returns(mock_vm)
mock_vm.stubs(:firmware).returns('automatic')
mock_vm.expects(:firmware=).with('bios')
@cr.stubs(:parse_networks).returns(args)
@cr.expects(:new_vm).returns(mock_vm)
@cr.create_vm(args)
Expand Down Expand Up @@ -398,13 +396,33 @@ class Foreman::Model::VmwareTest < ActiveSupport::TestCase
assert_equal attrs_out, @cr.parse_args(attrs_in)
end

test 'chooses EFI firmware with SecureBoot enabled when firmware type is uefi_secure_boot and firmware is automatic' do
attrs_in = HashWithIndifferentAccess.new(:firmware_type => :uefi_secure_boot, 'firmware' => 'automatic')
attrs_out = {:firmware => "efi", :secure_boot => true}
assert_equal attrs_out, @cr.parse_args(attrs_in)
end

test 'chooses BIOS firmware when no pxe loader is set and firmware is automatic' do
attrs_in = HashWithIndifferentAccess.new('firmware' => 'automatic')
attrs_out = {:firmware => "bios"}
assert_equal attrs_out, @cr.parse_args(attrs_in)
end
end

context 'virtual_tpm' do
test 'sets virtual_tpm to true when firmware is UEFI and virtual_tpm is enabled' do
attrs_in = HashWithIndifferentAccess.new(:firmware => 'uefi', :virtual_tpm => '1')
attrs_out = { :firmware => 'efi', :virtual_tpm => true }
assert_equal attrs_out, @cr.parse_args(attrs_in)
end

test 'adds an error when firmware is BIOS and virtual_tpm is enabled' do
args = HashWithIndifferentAccess.new(:firmware => 'bios', :virtual_tpm => '1')
@cr.parse_args(args)
assert_includes @cr.errors.full_messages, 'TPM is not compatible with BIOS firmware. Please change Firmware or disable TPM.'
end
end

test "doesn't modify input hash" do
# else compute profiles won't save properly
attrs_in = HashWithIndifferentAccess.new("interfaces_attributes" => {"0" => {"network" => "network-17"}})
Expand Down
4 changes: 4 additions & 0 deletions test/models/concerns/pxe_loader_support_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -195,5 +195,9 @@ def setup
test 'defaults to bios firmware' do
assert_equal :bios, DummyPxeLoader.firmware_type('Anything')
end

test 'detects uefi_secure_boot firmware' do
assert_equal :uefi_secure_boot, DummyPxeLoader.firmware_type('Grub2 UEFI SecureBoot')
end
end
end
5 changes: 5 additions & 0 deletions test/models/host_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3313,6 +3313,11 @@ def to_managed!
host = FactoryBot.build_stubbed(:host, :managed, :pxe_loader => "Grub2 UEFI")
assert_equal :uefi, host.firmware_type
end

test 'should be :uefi_secure_boot for host with uefi_secure_boot loader' do
host = FactoryBot.build_stubbed(:host, :managed, :pxe_loader => "Grub2 UEFI SecureBoot")
assert_equal :uefi_secure_boot, host.firmware_type
end
end

describe '#templates_used' do
Expand Down

0 comments on commit 8d99d94

Please sign in to comment.