From a647805ef9b72545457bb070afd3414fe880e23c Mon Sep 17 00:00:00 2001
From: nofaralfasi <nalfassi@redhat.com>
Date: Thu, 12 Sep 2024 18:16:22 +0300
Subject: [PATCH 1/2] 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 00a127b435c..e14a7a995bf 100644
--- a/app/models/compute_resource.rb
+++ b/app/models/compute_resource.rb
@@ -398,6 +398,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<String, String>] 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 303e4ad42e9..0f2b984c274 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 990de5d319b93032c71471883ab60d35a8c3dbf3 Mon Sep 17 00:00:00 2001
From: nofaralfasi <nalfassi@redhat.com>
Date: Sun, 15 Sep 2024 13:18:20 +0300
Subject: [PATCH 2/2] 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 +-
 bundler.d/libvirt.rb                          |  4 +--
 test/models/compute_resource_test.rb          | 26 +++++++++++----
 test/models/compute_resources/libvirt_test.rb | 23 +++++++++++++
 .../concerns/pxe_loader_support_test.rb       |  4 +++
 test/models/host_test.rb                      |  5 +++
 9 files changed, 102 insertions(+), 18 deletions(-)

diff --git a/app/models/compute_resource.rb b/app/models/compute_resource.rb
index e14a7a995bf..db39034495a 100644
--- a/app/models/compute_resource.rb
+++ b/app/models/compute_resource.rb
@@ -406,23 +406,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'.
@@ -435,6 +447,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..123f1a18e26 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_attributes: { '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..35815999ea9 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 = new_vm ? 'automatic' : 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/bundler.d/libvirt.rb b/bundler.d/libvirt.rb
index 3404cc0313c..d8a958dbb7e 100644
--- a/bundler.d/libvirt.rb
+++ b/bundler.d/libvirt.rb
@@ -1,6 +1,4 @@
 group :libvirt do
-  # gem 'fog-libvirt', '>= 0.12.0'
+  gem 'fog-libvirt', '>= 0.13.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 0f2b984c274..9604449b99f 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..5bd2d7d83cd 100644
--- a/test/models/compute_resources/libvirt_test.rb
+++ b/test/models/compute_resources/libvirt_test.rb
@@ -270,4 +270,27 @@ 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_attributes: { 'secure' => 'yes' },
+        secure_boot: true,
+      }
+
+      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 49b5d2a1f9e..7b9ed4c67ef 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