From cd8f494a1401c1dbaaa41d429534046a811d2f94 Mon Sep 17 00:00:00 2001
From: nofaralfasi <nalfassi@redhat.com>
Date: Thu, 18 Jul 2024 12:21:07 +0300
Subject: [PATCH] Optimize Secure Boot & TPM Support for VMware

- Added a new firmware type for Secure Boot.
- Hide the TPM option from the UI when it isn't relevant.
- Removed unnecessary methods from the VMware model.
---
 .../compute_resources/foreman/model/vmware.rb | 32 ++++++++-----------
 app/models/concerns/pxe_loader_support.rb     |  2 ++
 .../form/vmware/_base.html.erb                | 19 ++++++-----
 .../javascripts/compute_resource/vmware.js    |  7 ++++
 4 files changed, 32 insertions(+), 28 deletions(-)

diff --git a/app/models/compute_resources/foreman/model/vmware.rb b/app/models/compute_resources/foreman/model/vmware.rb
index 3e43da47952..c87367d67c3 100644
--- a/app/models/compute_resources/foreman/model/vmware.rb
+++ b/app/models/compute_resources/foreman/model/vmware.rb
@@ -206,6 +206,7 @@ def firmware_types
         "automatic" => N_("Automatic"),
         "bios" => N_("BIOS"),
         "efi" => N_("EFI"),
+        "uefi_sb" => N_("UEFI Secure Boot"),
       }
     end
 
@@ -494,6 +495,11 @@ def parse_args(args)
       firmware_type = args.delete(:firmware_type)
       args[:firmware] = firmware_mapping(firmware_type) if args[:firmware] == 'automatic'
 
+      if args[:firmware] == 'uefi_sb'
+        args[:firmware] = 'efi'
+        args[:secure_boot] = true
+      end
+
       args.reject! { |k, v| v.nil? }
       args
     end
@@ -761,22 +767,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
@@ -832,8 +822,14 @@ def vm_instance_defaults
     end
 
     def firmware_mapping(firmware_type)
-      return 'efi' if firmware_type == :uefi
-      'bios'
+      case firmware_type
+      when :uefi
+        'efi'
+      when :uefi_sb
+        'uefi_sb'
+      else
+        'bios'
+      end
     end
 
     def set_vm_volumes_attributes(vm, vm_attrs)
diff --git a/app/models/concerns/pxe_loader_support.rb b/app/models/concerns/pxe_loader_support.rb
index c98632d7691..add442bef15 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_sb
       when /UEFI/
         :uefi
       else
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 2de064da822..be818baae1f 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,10 @@
   <%= 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
   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, {:disabled => !new_vm, :value => type, :text => _(name), :onchange => 'tfm.computeResource.vmware.onFirmwareChange(this)'}
   end.join(' ').html_safe
 end %>
 <%= selectable_f f, :cluster, compute_resource.clusters, { :include_blank => _('Please select a cluster') },
@@ -49,15 +50,13 @@ 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_size => "col-md-2",
-                                  :disabled => !new_vm } %>
+<!-- # BIOS firmware doesn't support virtual TPM. -->
+<div id='efi_features' <%= 'class=hide' if !new_vm || f.object.firmware == 'bios' %>>
+  <%= 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" } %>
+</div>
 
 <%= compute_specific_js(compute_resource, "nic_info") %>
 
diff --git a/webpack/assets/javascripts/compute_resource/vmware.js b/webpack/assets/javascripts/compute_resource/vmware.js
index fa87bc95728..a752fbf4866 100644
--- a/webpack/assets/javascripts/compute_resource/vmware.js
+++ b/webpack/assets/javascripts/compute_resource/vmware.js
@@ -83,3 +83,10 @@ function fetchNetworks(url, clusterId) {
     },
   });
 }
+
+export function onFirmwareChange(item) {
+  const selected = $(item).val();
+  const inputs = $('#efi_features');
+
+  inputs.toggleClass('hide', selected === 'bios');
+}