Skip to content

Commit

Permalink
Fixes #37823 - Add Secure Boot and Virtual TPM support for VMware
Browse files Browse the repository at this point in the history 
- Implement Secure Boot and Virtual TPM functionality for VMware.
- Move default firmware setting from `create_vm` to `parse_args`.
  • Loading branch information
@nofaralfasi
nofaralfasi committed Sep 24, 2024
1 parent 36adaed commit f4d3ce0
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 34 deletions.
45 changes: 29 additions & 16 deletions app/models/compute_resources/foreman/model/vmware.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,14 +201,6 @@ def storage_controller_types
}
end

def firmware_types
{
"automatic" => N_("Automatic"),
"bios" => N_("BIOS"),
"efi" => N_("EFI"),
}
end

def disk_mode_types
{
"persistent" => _("Persistent"),
Expand Down Expand Up @@ -487,8 +479,9 @@ 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
args.merge!(process_firmware_attributes(args[:firmware], firmware_type))
args[:virtual_tpm] = validate_tpm_compatibility(args[:virtual_tpm], args[:firmware])

args.reject! { |k, v| v.nil? }
args
Expand Down Expand Up @@ -521,7 +514,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 @@ -827,11 +820,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 Expand Up @@ -862,5 +850,30 @@ def valid_cloudinit_for_customspec?(cloudinit)
def cachekey_with_cluster(key, cluster_id = nil)
cluster_id.nil? ? key.to_sym : "#{key}-#{cluster_id}".to_sym
end

# Generates Secure Boot settings for VMware, based on the provided firmware type.
#
# @param firmware [String] The firmware type.
# @return [Hash] A hash with secure boot settings if applicable.
def generate_secure_boot_settings(firmware)
firmware == 'uefi_secure_boot' ? { "secure_boot" => true } : {}
end

# Validates TPM compatibility based on the firmware type and virtual TPM setting.
# Adds an error if TPM is enabled with BIOS firmware, which is incompatible.
# This error is later raised as an `ArgumentError` in the `#create_vm` method.
#
# @param virtual_tpm [String] indicates if the virtual TPM is enabled ('1') or disabled ('0').
# @param firmware [String] the firmware type.
# @return [Boolean] the cast value of virtual_tpm after validation.
def validate_tpm_compatibility(virtual_tpm, firmware)
virtual_tpm = ActiveModel::Type::Boolean.new.cast(virtual_tpm)

if virtual_tpm && firmware == 'bios'
errors.add :base, _('TPM is not compatible with BIOS firmware. Please change Firmware or disable TPM.')
end

virtual_tpm
end
end
end
12 changes: 10 additions & 2 deletions app/views/compute_resources_vms/form/vmware/_base.html.erb
View file
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)") %>
<%= field(f, :firmware, :label => _('Firmware'), :label_size => "col-md-2") do
<% firmware_type = compute_resource.firmware_type(f.object.firmware, f.object.secure_boot) %>
<%= field(f, :firmware, :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, {: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,6 +51,12 @@ end %>
{ :disabled => images.empty?, :label => _('Image'), :label_size => "col-md-2" } %>
</div>

<%= 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 } %>
<%= compute_specific_js(compute_resource, "nic_info") %>
<%= react_component('StorageContainer', { data: {
Expand Down
67 changes: 51 additions & 16 deletions test/models/compute_resources/vmware_test.rb
View file
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 @@ -145,17 +144,6 @@ class Foreman::Model::VmwareTest < ActiveSupport::TestCase
@cr.expects(:new_vm).returns(mock_vm)
@cr.create_vm(args)
end

test 'converts automatic firmware to bios default' do
args = {"provision_method" => "build"}
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)
end
end

test "#create_vm calls clone_vm when image provisioning" do
Expand Down Expand Up @@ -266,8 +254,11 @@ class Foreman::Model::VmwareTest < ActiveSupport::TestCase
@cr = FactoryBot.build_stubbed(:vmware_cr)
end

test "converts empty hash" do
assert_empty(@cr.parse_args(HashWithIndifferentAccess.new))
test "defaults to BIOS firmware when no firmware is provided" do
args = HashWithIndifferentAccess.new
expected_firmware = { firmware: "bios" }

assert_equal expected_firmware, @cr.parse_args(args)
end

test "converts form attrs to fog attrs" do
Expand Down Expand Up @@ -297,7 +288,7 @@ class Foreman::Model::VmwareTest < ActiveSupport::TestCase
}
)
# All keys must be symbolized
attrs_out = {:cpus => "1", :interfaces => [{:type => "VirtualVmxnet3", :network => "network-17", :_delete => ""}], :volumes => [{:size_gb => "1", :_delete => ""}]}
attrs_out = { :cpus => "1", :interfaces => [{ :type => "VirtualVmxnet3", :network => "network-17", :_delete => "" }], :volumes => [{ :size_gb => "1", :_delete => "" }], :firmware => "bios" }
assert_equal attrs_out, @cr.parse_args(attrs_in)
end

Expand Down Expand Up @@ -328,7 +319,7 @@ class Foreman::Model::VmwareTest < ActiveSupport::TestCase
},
}
)
attrs_out = {:cpus => "1", :interfaces => [{:type => "VirtualVmxnet3", :network => "network-17", :_delete => ""}], :volumes => [{:size_gb => "1", :_delete => ""}]}
attrs_out = {:cpus => "1", :interfaces => [{:type => "VirtualVmxnet3", :network => "network-17", :_delete => ""}], :volumes => [{:size_gb => "1", :_delete => ""}], :firmware => "bios" }
assert_equal attrs_out, @cr.parse_args(attrs_in)
end

Expand Down Expand Up @@ -375,6 +366,7 @@ class Foreman::Model::VmwareTest < ActiveSupport::TestCase
:_delete => "",
},
],
:firmware => "bios",
}
assert_equal attrs_out, @cr.parse_args(attrs_in)
end
Expand Down Expand Up @@ -1072,4 +1064,47 @@ def mock_network(id, name, virtualswitch = nil)
check_vm_attribute_names(cr)
end
end

describe '#generate_secure_boot_settings' do
before do
@cr = FactoryBot.build_stubbed(:vmware_cr)
end

test "returns secure boot settings when firmware is 'uefi_secure_boot'" do
assert_equal({ "secure_boot" => true }, @cr.send(:generate_secure_boot_settings, 'uefi_secure_boot'))
end

test "returns an empty hash for firmware types other than 'uefi_secure_boot'" do
assert_empty @cr.send(:generate_secure_boot_settings, 'uefi')
assert_empty @cr.send(:generate_secure_boot_settings, 'bios')
assert_empty @cr.send(:generate_secure_boot_settings, '')
assert_empty @cr.send(:generate_secure_boot_settings, nil)
end
end

describe '#validate_tpm_compatibility' do
before do
@cr = FactoryBot.build_stubbed(:vmware_cr)
end

test 'returns true and no errors when firmware is EFI and virtual_tpm is enabled' do
assert_equal true, @cr.send(:validate_tpm_compatibility, '1', 'efi')
assert_empty @cr.errors.full_messages
end

test 'returns false and no errors when firmware is EFI and virtual_tpm is disabled' do
assert_equal false, @cr.send(:validate_tpm_compatibility, '0', 'efi')
assert_empty @cr.errors.full_messages
end

test 'returns false and no errors when firmware is BIOS and virtual_tpm is disabled' do
assert_equal false, @cr.send(:validate_tpm_compatibility, '0', 'bios')
assert_empty @cr.errors.full_messages
end

test 'returns true and adds an error when firmware is BIOS and virtual_tpm is enabled' do
assert_equal true, @cr.send(:validate_tpm_compatibility, '1', 'bios')
assert_includes @cr.errors.full_messages, 'TPM is not compatible with BIOS firmware. Please change Firmware or disable TPM.'
end
end
end

0 comments on commit f4d3ce0

Please sign in to comment.