From 33f05600359f3f89b5073890ab43dfaffbed2891 Mon Sep 17 00:00:00 2001 From: nofaralfasi Date: Thu, 12 Sep 2024 18:16:22 +0300 Subject: [PATCH 1/3] Add Firmware option to Libvirt VM creation --- app/models/compute_resource.rb | 37 ++++++++++++ .../foreman/model/libvirt.rb | 7 ++- .../form/libvirt/_base.html.erb | 7 +++ bundler.d/libvirt.rb | 4 +- test/models/compute_resource_test.rb | 57 +++++++++++++++++++ 5 files changed, 110 insertions(+), 2 deletions(-) diff --git a/app/models/compute_resource.rb b/app/models/compute_resource.rb index a555156e9bd..c0cc01dbef6 100644 --- a/app/models/compute_resource.rb +++ b/app/models/compute_resource.rb @@ -399,6 +399,43 @@ def normalize_vm_attrs(vm_attrs) vm_attrs end + # Returns a hash of firmware type identifiers and their corresponding labels for use in the VM creation form. + # + # @return [Hash] a hash mapping firmware type identifiers to labels. + def firmware_types + { + "automatic" => N_("Automatic"), + "bios" => N_("BIOS"), + "uefi" => N_("UEFI"), + }.freeze + end + + # Converts the firmware type from a VM object to the Foreman-compatible format. + # + # @param firmware [String] The firmware type from the VM object. + # @return [String] The converted firmware type. + def firmware_type(firmware) + firmware == 'efi' ? 'uefi' : firmware + end + + # Normalizes the firmware type to 'efi' or defaults to 'bios'. + # + # @param firmware_type [String] The firmware type in Foreman format. + # @return [String] The converted firmware type. + def normalize_firmware_type(firmware_type) + firmware_type == 'uefi' ? 'efi' : 'bios' + end + + # Resolves the firmware setting when it is 'automatic' based on the provided firmware_type, or defaults to 'bios'. + # + # @param firmware [String] The current firmware setting. + # @param firmware_type [String] The type of firmware to be used if firmware is 'automatic'. + # @return [String] the resolved firmware. + def resolve_automatic_firmware(firmware, firmware_type) + return firmware unless firmware == 'automatic' + firmware_type.presence || 'bios' + end + protected def memory_gb_to_bytes(memory_size) diff --git a/app/models/compute_resources/foreman/model/libvirt.rb b/app/models/compute_resources/foreman/model/libvirt.rb index 23d367de42b..787f73480ed 100644 --- a/app/models/compute_resources/foreman/model/libvirt.rb +++ b/app/models/compute_resources/foreman/model/libvirt.rb @@ -148,6 +148,10 @@ def new_vm(attr = { }) opts[:boot_order] = %w[hd] opts[:boot_order].unshift 'network' unless attr[:image_id] + firmware_type = opts.delete(:firmware_type).to_s + firmware = resolve_automatic_firmware(opts[:firmware], firmware_type) + opts[:firmware] = normalize_firmware_type(firmware) + vm = client.servers.new opts vm.memory = opts[:memory] if opts[:memory] vm @@ -289,7 +293,8 @@ 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' ) end diff --git a/app/views/compute_resources_vms/form/libvirt/_base.html.erb b/app/views/compute_resources_vms/form/libvirt/_base.html.erb index ad88274d096..9e1e8a93c09 100644 --- a/app/views/compute_resources_vms/form/libvirt/_base.html.erb +++ b/app/views/compute_resources_vms/form/libvirt/_base.html.erb @@ -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.firmware) %> +<%= field(f, :firmware, :disabled => !new_vm, :label => _('Firmware'), :label_help => _("Choose 'Automatic' to set the firmware based on the host's 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) diff --git a/bundler.d/libvirt.rb b/bundler.d/libvirt.rb index 80b3f8c5a12..3404cc0313c 100644 --- a/bundler.d/libvirt.rb +++ b/bundler.d/libvirt.rb @@ -1,4 +1,6 @@ group :libvirt do - gem 'fog-libvirt', '>= 0.12.0' + # gem 'fog-libvirt', '>= 0.12.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 diff --git a/test/models/compute_resource_test.rb b/test/models/compute_resource_test.rb index d106e43c955..0f2e51b6bc5 100644 --- a/test/models/compute_resource_test.rb +++ b/test/models/compute_resource_test.rb @@ -391,4 +391,61 @@ def setup assert_equal volume_attributes, volumes end end + + describe '#firmware_type' do + before do + @cr = compute_resources(:mycompute) + end + + test "returns 'uefi' when firmware is 'efi'" do + assert_equal 'uefi', @cr.firmware_type('efi') + end + + test "returns firmware unchanged when firmware is not 'efi'" do + assert_equal 'bios', @cr.firmware_type('bios') + assert_equal '', @cr.firmware_type('') + assert_equal nil, @cr.firmware_type(nil) + end + end + + describe '#normalize_firmware_type' do + before do + @cr = compute_resources(:mycompute) + end + + test "returns 'efi' when firmware is 'uefi'" do + assert_equal 'efi', @cr.normalize_firmware_type('uefi') + end + + test "returns 'bios' when firmware is not 'uefi'" do + assert_equal 'bios', @cr.normalize_firmware_type('bios') + assert_equal 'bios', @cr.normalize_firmware_type('none') + assert_equal 'bios', @cr.normalize_firmware_type('') + assert_equal 'bios', @cr.normalize_firmware_type(nil) + end + end + + describe '#resolve_automatic_firmware' do + before do + @cr = compute_resources(:mycompute) + end + + test "returns firmware_type when firmware is 'automatic' and firmware_type is present" do + assert_equal 'uefi', @cr.send(:resolve_automatic_firmware, 'automatic', 'uefi') + assert_equal 'bios', @cr.send(:resolve_automatic_firmware, 'automatic', 'bios') + assert_equal 'none', @cr.send(:resolve_automatic_firmware, 'automatic', 'none') + end + + test "returns firmware unchanged when not 'automatic'" do + assert_equal 'uefi', @cr.send(:resolve_automatic_firmware, 'uefi', 'bios') + assert_equal 'bios', @cr.send(:resolve_automatic_firmware, 'bios', 'uefi') + assert_equal 'uefi', @cr.send(:resolve_automatic_firmware, 'uefi', false) + assert_equal 'bios', @cr.send(:resolve_automatic_firmware, 'bios', '') + end + + test "returns 'bios' when firmware is 'automatic' and firmware_type is not present" do + assert_equal 'bios', @cr.send(:resolve_automatic_firmware, 'automatic', '') + assert_equal 'bios', @cr.send(:resolve_automatic_firmware, 'automatic', nil) + end + end end From 36adaedd72b14cb09559898eb0e29592bdf8686f Mon Sep 17 00:00:00 2001 From: nofaralfasi Date: Sun, 15 Sep 2024 13:18:20 +0300 Subject: [PATCH 2/3] Fixes #37566 - Add UEFI Secure Boot Firmware to Libvirt - Added a new firmware type for Secure Boot. - Enable `enrolled-keys` by default when Secure Boot is activated. - Added firmware-related methods to the ComputeResource model for shared use between VMware and Libvirt. --- app/models/compute_resource.rb | 33 ++++++++++++++++--- .../foreman/model/libvirt.rb | 21 ++++++++++-- app/models/concerns/pxe_loader_support.rb | 2 ++ .../form/libvirt/_base.html.erb | 2 +- test/models/compute_resource_test.rb | 26 +++++++++++---- test/models/compute_resources/libvirt_test.rb | 22 +++++++++++++ .../concerns/pxe_loader_support_test.rb | 4 +++ test/models/host_test.rb | 5 +++ 8 files changed, 100 insertions(+), 15 deletions(-) diff --git a/app/models/compute_resource.rb b/app/models/compute_resource.rb index c0cc01dbef6..d7ffad1d1df 100644 --- a/app/models/compute_resource.rb +++ b/app/models/compute_resource.rb @@ -407,23 +407,35 @@ def firmware_types "automatic" => N_("Automatic"), "bios" => N_("BIOS"), "uefi" => N_("UEFI"), + "uefi_secure_boot" => N_("UEFI Secure Boot"), }.freeze end # Converts the firmware type from a VM object to the Foreman-compatible format. # # @param firmware [String] The firmware type from the VM object. + # @param secure_boot [Boolean] Indicates if secure boot is enabled for the VM. # @return [String] The converted firmware type. - def firmware_type(firmware) - firmware == 'efi' ? 'uefi' : firmware + def firmware_type(firmware, secure_boot) + if firmware == 'efi' + secure_boot ? 'uefi_secure_boot' : 'uefi' # Adjust for secure boot + else + firmware + end end - # Normalizes the firmware type to 'efi' or defaults to 'bios'. + # Converts a firmware type from Foreman format to a CR-compatible format. + # If no specific type is provided, defaults to 'bios'. # # @param firmware_type [String] The firmware type in Foreman format. # @return [String] The converted firmware type. def normalize_firmware_type(firmware_type) - firmware_type == 'uefi' ? 'efi' : 'bios' + case firmware_type + when 'uefi', 'uefi_secure_boot' + 'efi' + else + 'bios' + end end # Resolves the firmware setting when it is 'automatic' based on the provided firmware_type, or defaults to 'bios'. @@ -436,6 +448,19 @@ def resolve_automatic_firmware(firmware, firmware_type) firmware_type.presence || 'bios' end + # Processes firmware attributes to configure firmware and secure boot settings. + # + # @param firmware [String] The firmware setting to be processed. + # @param firmware_type [String] The firmware type based on the provided PXE Loader. + # @return [Hash] A hash containing the processed firmware attributes. + def process_firmware_attributes(firmware, firmware_type) + firmware = resolve_automatic_firmware(firmware, firmware_type) + + attrs = generate_secure_boot_settings(firmware) + attrs[:firmware] = normalize_firmware_type(firmware) + attrs + end + protected def memory_gb_to_bytes(memory_size) diff --git a/app/models/compute_resources/foreman/model/libvirt.rb b/app/models/compute_resources/foreman/model/libvirt.rb index 787f73480ed..87ed9656d2a 100644 --- a/app/models/compute_resources/foreman/model/libvirt.rb +++ b/app/models/compute_resources/foreman/model/libvirt.rb @@ -149,8 +149,7 @@ def new_vm(attr = { }) opts[:boot_order].unshift 'network' unless attr[:image_id] firmware_type = opts.delete(:firmware_type).to_s - firmware = resolve_automatic_firmware(opts[:firmware], firmware_type) - opts[:firmware] = normalize_firmware_type(firmware) + opts.merge!(process_firmware_attributes(opts[:firmware], firmware_type)) vm = client.servers.new opts vm.memory = opts[:memory] if opts[:memory] @@ -294,7 +293,8 @@ def vm_instance_defaults :listen => Setting[:libvirt_default_console_address], :password => random_password(console_password_length(display_type)), :port => '-1' }, - :firmware => 'automatic' + :firmware => 'automatic', + :firmware_features => { "secure-boot" => "no" } ) end @@ -331,5 +331,20 @@ 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 + + # Generates Secure Boot settings for Libvirt based on the provided firmware type. + # The `secure_boot` setting is used to properly configure and display the Firmware in the `compute_attributes` form. + # + # @param firmware [String] The firmware type. + # @return [Hash] A hash with secure boot settings if applicable. + def generate_secure_boot_settings(firmware) + return {} unless firmware == 'uefi_secure_boot' + + { + firmware_features: { 'secure-boot' => 'yes', 'enrolled-keys' => 'yes' }, + loader: { 'secure' => 'yes' }, + secure_boot: true, + } + end end end diff --git a/app/models/concerns/pxe_loader_support.rb b/app/models/concerns/pxe_loader_support.rb index c98632d7691..5fbeed8fbef 100644 --- a/app/models/concerns/pxe_loader_support.rb +++ b/app/models/concerns/pxe_loader_support.rb @@ -50,6 +50,8 @@ def firmware_type(pxe_loader) case pxe_loader when 'None' :none + when /SecureBoot/ + :uefi_secure_boot when /UEFI/ :uefi else diff --git a/app/views/compute_resources_vms/form/libvirt/_base.html.erb b/app/views/compute_resources_vms/form/libvirt/_base.html.erb index 9e1e8a93c09..ac1b042fafc 100644 --- a/app/views/compute_resources_vms/form/libvirt/_base.html.erb +++ b/app/views/compute_resources_vms/form/libvirt/_base.html.erb @@ -8,7 +8,7 @@ <% 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.firmware) %> +<% firmware_type = compute_resource.firmware_type(f.object.firmware, f.object.secure_boot) %> <%= field(f, :firmware, :disabled => !new_vm, :label => _('Firmware'), :label_help => _("Choose 'Automatic' to set the firmware based on the host's 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) } diff --git a/test/models/compute_resource_test.rb b/test/models/compute_resource_test.rb index 0f2e51b6bc5..4d808102d19 100644 --- a/test/models/compute_resource_test.rb +++ b/test/models/compute_resource_test.rb @@ -397,14 +397,20 @@ def setup @cr = compute_resources(:mycompute) end - test "returns 'uefi' when firmware is 'efi'" do - assert_equal 'uefi', @cr.firmware_type('efi') + test "returns firmware unchanged when firmware is not 'efi'" do + assert_equal 'bios', @cr.firmware_type('bios', true) + assert_equal 'bios', @cr.firmware_type('bios', false) + assert_empty(@cr.firmware_type('', true)) + assert_nil(@cr.firmware_type(nil, false)) end - test "returns firmware unchanged when firmware is not 'efi'" do - assert_equal 'bios', @cr.firmware_type('bios') - assert_equal '', @cr.firmware_type('') - assert_equal nil, @cr.firmware_type(nil) + test "returns 'uefi' when firmware is 'efi' and secure boot is not enabled" do + assert_equal 'uefi', @cr.firmware_type('efi', false) + assert_equal 'uefi', @cr.firmware_type('efi', nil) + end + + test "returns 'uefi_secure_boot' when firmware is 'efi' and secure boot is enabled" do + assert_equal 'uefi_secure_boot', @cr.firmware_type('efi', true) end end @@ -417,12 +423,16 @@ def setup assert_equal 'efi', @cr.normalize_firmware_type('uefi') end - test "returns 'bios' when firmware is not 'uefi'" do + test "returns 'bios' for non-uefi firmware types" do assert_equal 'bios', @cr.normalize_firmware_type('bios') assert_equal 'bios', @cr.normalize_firmware_type('none') assert_equal 'bios', @cr.normalize_firmware_type('') assert_equal 'bios', @cr.normalize_firmware_type(nil) end + + test "returns 'efi' when firmware is 'uefi_secure_boot'" do + assert_equal 'efi', @cr.normalize_firmware_type('uefi_secure_boot') + end end describe '#resolve_automatic_firmware' do @@ -434,6 +444,7 @@ def setup assert_equal 'uefi', @cr.send(:resolve_automatic_firmware, 'automatic', 'uefi') assert_equal 'bios', @cr.send(:resolve_automatic_firmware, 'automatic', 'bios') assert_equal 'none', @cr.send(:resolve_automatic_firmware, 'automatic', 'none') + assert_equal 'uefi_secure_boot', @cr.send(:resolve_automatic_firmware, 'automatic', 'uefi_secure_boot') end test "returns firmware unchanged when not 'automatic'" do @@ -441,6 +452,7 @@ def setup assert_equal 'bios', @cr.send(:resolve_automatic_firmware, 'bios', 'uefi') assert_equal 'uefi', @cr.send(:resolve_automatic_firmware, 'uefi', false) assert_equal 'bios', @cr.send(:resolve_automatic_firmware, 'bios', '') + assert_equal 'uefi_secure_boot', @cr.send(:resolve_automatic_firmware, 'uefi_secure_boot', '') end test "returns 'bios' when firmware is 'automatic' and firmware_type is not present" do diff --git a/test/models/compute_resources/libvirt_test.rb b/test/models/compute_resources/libvirt_test.rb index 9bf15229ad9..8afc9edea25 100644 --- a/test/models/compute_resources/libvirt_test.rb +++ b/test/models/compute_resources/libvirt_test.rb @@ -270,4 +270,26 @@ class Foreman::Model::LibvirtTest < ActiveSupport::TestCase check_vm_attribute_names(cr) end end + + describe '#generate_secure_boot_settings' do + before do + @cr = FactoryBot.build_stubbed(:libvirt_cr) + end + + test "returns secure boot settings when firmware is 'uefi_secure_boot'" do + expected_sb_settings = { + firmware_features: { 'secure-boot' => 'yes', 'enrolled-keys' => 'yes' }, + loader: { 'secure' => 'yes' }, + } + + assert_equal expected_sb_settings, @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 end diff --git a/test/models/concerns/pxe_loader_support_test.rb b/test/models/concerns/pxe_loader_support_test.rb index 9a431957234..6636504b0d5 100644 --- a/test/models/concerns/pxe_loader_support_test.rb +++ b/test/models/concerns/pxe_loader_support_test.rb @@ -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 diff --git a/test/models/host_test.rb b/test/models/host_test.rb index 8865243d3ef..02faa254e3b 100644 --- a/test/models/host_test.rb +++ b/test/models/host_test.rb @@ -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 From f4d3ce0664fb6ab1860e69426df3bf460a309753 Mon Sep 17 00:00:00 2001 From: nofaralfasi Date: Sun, 15 Sep 2024 19:05:05 +0300 Subject: [PATCH 3/3] Fixes #37823 - Add Secure Boot and Virtual TPM support for VMware - Implement Secure Boot and Virtual TPM functionality for VMware. - Move default firmware setting from `create_vm` to `parse_args`. --- .../compute_resources/foreman/model/vmware.rb | 45 ++++++++----- .../form/vmware/_base.html.erb | 12 +++- test/models/compute_resources/vmware_test.rb | 67 ++++++++++++++----- 3 files changed, 90 insertions(+), 34 deletions(-) diff --git a/app/models/compute_resources/foreman/model/vmware.rb b/app/models/compute_resources/foreman/model/vmware.rb index 76c7ea8e9ee..2a3d8ef38ff 100644 --- a/app/models/compute_resources/foreman/model/vmware.rb +++ b/app/models/compute_resources/foreman/model/vmware.rb @@ -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"), @@ -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 @@ -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 @@ -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)] }] @@ -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 diff --git a/app/views/compute_resources_vms/form/vmware/_base.html.erb b/app/views/compute_resources_vms/form/vmware/_base.html.erb index 97d23998118..c32c254e731 100644 --- a/app/views/compute_resources_vms/form/vmware/_base.html.erb +++ b/app/views/compute_resources_vms/form/vmware/_base.html.erb @@ -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) %> <%= 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') }, @@ -49,6 +51,12 @@ end %> { :disabled => images.empty?, :label => _('Image'), :label_size => "col-md-2" } %> +<%= 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: { diff --git a/test/models/compute_resources/vmware_test.rb b/test/models/compute_resources/vmware_test.rb index c88ec6df0ee..a9868e21f72 100644 --- a/test/models/compute_resources/vmware_test.rb +++ b/test/models/compute_resources/vmware_test.rb @@ -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) @@ -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 @@ -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 @@ -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 @@ -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 @@ -375,6 +366,7 @@ class Foreman::Model::VmwareTest < ActiveSupport::TestCase :_delete => "", }, ], + :firmware => "bios", } assert_equal attrs_out, @cr.parse_args(attrs_in) end @@ -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