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 Jul 31, 2024
1 parent 9a0f5f4 commit f12806d
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 33 deletions.
58 changes: 33 additions & 25 deletions app/models/compute_resources/foreman/model/vmware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,16 @@ def firmware_types
"automatic" => N_("Automatic"),
"bios" => N_("BIOS"),
"efi" => N_("EFI"),
}
"uefi_secure_boot" => N_("UEFI Secure Boot"),
}.freeze
end

def firmware_type(vm_obj)
if vm_obj.firmware == 'efi'
vm_obj.secure_boot ? "uefi_secure_boot" : "efi"
else
vm_obj.firmware
end
end

def disk_mode_types
Expand Down Expand Up @@ -488,8 +497,22 @@ 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'
firmware_type = args.delete(:firmware_type).to_s

if args[:firmware] == 'automatic'
if firmware_type == 'none' || firmware_type.empty?
args[:firmware] = 'bios'
else
args[:firmware] = firmware_type
end
end

if args[:firmware]&.start_with?('uefi')
args[:secure_boot] = true if args[:firmware] == 'uefi_secure_boot'
args[:firmware] = 'efi'
end

args[:virtual_tpm] = ActiveRecord::Type::Boolean.new.cast(args.delete(:virtual_tpm))

args.reject! { |k, v| v.nil? }
args
Expand All @@ -511,6 +534,12 @@ def parse_networks(args)
args
end

def check_tpm_compatibility(vm)
if vm.firmware == 'bios' && vm.virtual_tpm
raise ArgumentError, _('TPM is not compatible with BIOS firmware. Please change Firmware or disable TPM.')
end
end

def create_vm(args = { })
vm = nil
test_connection
Expand All @@ -522,7 +551,7 @@ def create_vm(args = { })
clone_vm(args)
else
vm = new_vm(args)
vm.firmware = 'bios' if vm.firmware == 'automatic'
check_tpm_compatibility(vm)
vm.save
end
rescue Fog::Vsphere::Compute::NotFound => e
Expand Down Expand Up @@ -772,22 +801,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 @@ -843,11 +856,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 EFI firmware."),
:label_size => "col-md-2",
:disabled => !new_vm } %>
Expand Down
4 changes: 3 additions & 1 deletion bundler.d/vmware.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
group :vmware do
gem 'fog-vsphere', '>= 3.6.4', '< 4.0'
# gem 'fog-vsphere', '>= 3.6.4', '< 4.0'
# TODO: Remove this once the fog-vsphere gem is released, and upgrade to the latest version
gem 'fog-vsphere', :git => "https://github.com/fog/fog-vsphere.git", :branch => "master"
end
16 changes: 15 additions & 1 deletion test/models/compute_resources/vmware_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ class Foreman::Model::VmwareTest < ActiveSupport::TestCase
mock_vm = mock('vm')
mock_vm.expects(:save).returns(mock_vm)
mock_vm.stubs(:firmware).returns('bios')
mock_vm.stubs(:virtual_tpm).returns(false)
@cr.stubs(:parse_networks).returns(args)
@cr.expects(:clone_vm).times(0)
@cr.expects(:new_vm).returns(mock_vm)
Expand All @@ -151,7 +152,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 +398,27 @@ 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 EFI and virtual_tpm is enabled' do
attrs_in = HashWithIndifferentAccess.new(:firmware => 'efi', :virtual_tpm => '1')
attrs_out = { :firmware => 'efi', :virtual_tpm => true }
assert_equal attrs_out, @cr.parse_args(attrs_in)
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 @@ -3281,6 +3281,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 f12806d

Please sign in to comment.