Fixes #36626 - safe checks for the pxe_loader

app/models/concerns/pxe_loader_suggestion.rb

@@ -1,7 +1,12 @@
 module PxeLoaderSuggestion
   extend ActiveSupport::Concern
   def suggest_default_pxe_loader
-    self.pxe_loader ||= try(:operatingsystem).try(:parameters).try(:find_by_name, "pxe-loader").try(:value)
-    self.pxe_loader ||= try(:operatingsystem).try(:preferred_loader) if respond_to?(:pxe_loader)
+    return if pxe_loader.presence
+
+    from_os_params = try(:operatingsystem).try(:parameters).try(:find_by_name, "pxe-loader")
+    self.pxe_loader = from_os_params.try(:value)
+    return if pxe_loader.presence
+
+    self.pxe_loader = try(:operatingsystem).try(:preferred_loader).presence || ''
   end
 end

app/models/concerns/pxe_loader_support.rb

@@ -70,9 +70,11 @@ def pxe_loader_kind(host)
   # Suggested PXE loader when template kind is available (PXEGrub2, then PXELinux, then PXEGrub in this order)
   def preferred_loader
     associated_templates = os_default_templates.map(&:template_kind).compact.map(&:name)
+
     template_kinds.each do |loader|
       return PREFERRED_KINDS[loader] if associated_templates.include? loader
     end
-    nil
+
+    ''
   end
 end

app/models/host/managed.rb

@@ -729,8 +729,9 @@ def explicit_pxe_loader
     self[:pxe_loader].presence
   end
 
+  # @return [String] The name of the PXE loader to be used or empty string if none
   def pxe_loader
-    explicit_pxe_loader || hostgroup.try(:pxe_loader)
+    explicit_pxe_loader || hostgroup.try(:pxe_loader) || ''
   end
 
   def pxe_loader_efi?

app/views/common/os_selection/_pxe_loader.html.erb

@@ -6,7 +6,7 @@
   %>
     <%= selectable_f f, :pxe_loader,
       loaders.to_a, {
-        :include_blank => pxe_loader_selected,
+        :include_blank => false,
         :selected => pxe_loader_selected == true ? f.object.pxe_loader : pxe_loader_selected
       },
       :label => _('PXE loader'),

test/models/concerns/pxe_loader_suggestion_test.rb

@@ -10,7 +10,7 @@ def setup
 
     test 'host does not suggest PXEloader when OS is not set' do
       @host.suggest_default_pxe_loader
-      assert_nil @host.pxe_loader
+      assert_empty @host.pxe_loader
     end
 
     test 'host suggests default PXEloader for OS' do
@@ -56,7 +56,7 @@ def setup
 
     test 'hostgroup does not suggest PXEloader when OS is not set' do
       @hostgroup.suggest_default_pxe_loader
-      assert_nil @hostgroup.pxe_loader
+      assert_empty @hostgroup.pxe_loader
     end
 
     test 'hostgroup suggests default PXEloader for OS' do
@@ -72,4 +72,17 @@ def setup
       assert_equal 'PXELinux UEFI', @hostgroup.pxe_loader
     end
   end
+
+  context 'validations' do
+    def setup
+      @os = FactoryBot.create(:operatingsystem)
+      @host = FactoryBot.create(:host, operatingsystem: @os)
+      Operatingsystem.any_instance.stubs(:preferred_loader).returns('')
+    end
+
+    test 'always returns string' do
+      @os.os_parameters.create!(:name => 'pxe-loader', :value => '')
+      assert_empty @host.pxe_loader
+    end
+  end
 end

test/models/concerns/pxe_loader_support_test.rb

@@ -139,18 +139,18 @@ def setup
     test "is none for zero template kinds and templates" do
       @subject.expects(:template_kinds).returns([])
       @subject.expects(:os_default_templates).returns([])
-      assert_nil @subject.preferred_loader
+      assert_empty @subject.preferred_loader
     end
 
     test "is none for zero templates" do
       @subject.expects(:os_default_templates).returns([])
-      assert_nil @subject.preferred_loader
+      assert_empty @subject.preferred_loader
     end
 
     test "is none for zero template kinds" do
       @subject.expects(:template_kinds).returns([])
       @subject.expects(:os_default_templates).returns([@template_pxelinux, @template_pxegrub, @template_pxegrub2])
-      assert_nil @subject.preferred_loader
+      assert_empty @subject.preferred_loader
     end
 
     test "is PXELinux for all associated template kinds" do

test/models/operatingsystem_test.rb

@@ -258,11 +258,11 @@ class OperatingsystemTest < ActiveSupport::TestCase
   end
 
   test "should not have preferred pxe loader for an OS without architecture associated" do
-    assert_nil Operatingsystem.new.preferred_loader
+    assert_empty Operatingsystem.new.preferred_loader
   end
 
   test "should have preferred pxe loader for an Solaris OS without any templates" do
-    assert_nil Solaris.new.preferred_loader
+    assert_empty Solaris.new.preferred_loader
   end
 
   test "should have preferred pxe loader for OS with PXELinux template" do