From c8ee8063e5dd73f689fa6a5754aae72994b6b1cc Mon Sep 17 00:00:00 2001
From: Keenan Brock <keenan@thebrocks.net>
Date: Wed, 22 Mar 2023 21:12:21 -0400
Subject: [PATCH] moving conditional supports logic from validate to supports

vnc, webmks, and console are not consistent
webmks also adds login validation checking.

Added vm must be running check. (was present but not called before)
Since other providers seem to have this check, seemed like a good consistency thing
---
 .../providers/vmware/infra_manager.rb         |  6 +-
 .../vmware/infra_manager/vm/remote_console.rb | 59 ++++++++-----------
 .../infra_manager/vm/remote_console_spec.rb   | 21 ++++---
 .../providers/vmware/infra_manager_spec.rb    |  2 +-
 4 files changed, 41 insertions(+), 47 deletions(-)

diff --git a/app/models/manageiq/providers/vmware/infra_manager.rb b/app/models/manageiq/providers/vmware/infra_manager.rb
index 70a5d762a..29250c59b 100644
--- a/app/models/manageiq/providers/vmware/infra_manager.rb
+++ b/app/models/manageiq/providers/vmware/infra_manager.rb
@@ -40,7 +40,11 @@ class Vmware::InfraManager < InfraManager
     supports :metrics
     supports :native_console
     supports :vmrc_console do
-      "vCenter needs to be refreshed to determine remote console support." if api_version.blank? || hostname.blank? || uid_ems.blank?
+      if api_version.blank? || hostname.blank? || uid_ems.blank?
+        "vCenter needs to be refreshed to determine remote console support."
+      elsif ext_management_system.authentication_type(:console).nil?
+        "remote console requires console credentials"
+      end
     end
     supports :webmks_console
 
diff --git a/app/models/manageiq/providers/vmware/infra_manager/vm/remote_console.rb b/app/models/manageiq/providers/vmware/infra_manager/vm/remote_console.rb
index 501ec1b78..202b32710 100644
--- a/app/models/manageiq/providers/vmware/infra_manager/vm/remote_console.rb
+++ b/app/models/manageiq/providers/vmware/infra_manager/vm/remote_console.rb
@@ -2,20 +2,25 @@ module ManageIQ::Providers::Vmware::InfraManager::Vm::RemoteConsole
   extend ActiveSupport::Concern
 
   included do
-    supports :console
+    supports :console do
+      if ext_management_system.nil?
+        "VM must be registered with a management system."
+      elsif state != "on"
+        "VM must be running."
+      end
+    end
     supports :html5_console
-    supports :vmrc_console
-    supports :vnc_console
-    supports :webmks_console
-  end
-
-  def validate_remote_console_acquire_ticket(protocol, options = {})
-    raise(MiqException::RemoteConsoleNotSupportedError, "#{protocol} remote console requires the vm to be registered with a management system.") if ext_management_system.nil?
-
-    raise(MiqException::RemoteConsoleNotSupportedError, "remote console requires console credentials") if ext_management_system.authentication_type(:console).nil? && protocol == "vmrc"
-
-    options[:check_if_running] = true unless options.key?(:check_if_running)
-    raise(MiqException::RemoteConsoleNotSupportedError, "#{protocol} remote console requires the vm to be running.") if options[:check_if_running] && state != "on"
+    supports :vmrc_console do
+      unsupported_reason(:console) ||
+        ext_management_system.unsupported_reason(:vmrc_console)
+    end
+    supports :vnc_console do
+      unsupported_reason(:console)
+    end
+    supports :webmks_console do
+      unsupported_reason(:console) ||
+        ext_management_system.unsupported_reason(:webmks_console)
+    end
   end
 
   def remote_console_acquire_ticket(userid, originating_server, protocol)
@@ -47,7 +52,7 @@ def remote_console_acquire_ticket_queue(protocol, userid)
   #
 
   def remote_console_vmrc_acquire_ticket(_userid = nil, _originating_server = nil)
-    validate_remote_console_acquire_ticket("vmrc")
+    validate_supports(:vmrc_console)
     ticket = ext_management_system.remote_console_vmrc_acquire_ticket
 
     {
@@ -57,18 +62,12 @@ def remote_console_vmrc_acquire_ticket(_userid = nil, _originating_server = nil)
     }
   end
 
-  def validate_remote_console_vmrc_support
-    validate_remote_console_acquire_ticket("vmrc")
-    validate_supports(ext_management_system.unsupported_reason(:vmrc_console))
-    true
-  end
-
   #
   # WebMKS
   #
 
   def remote_console_webmks_acquire_ticket(userid, originating_server = nil)
-    validate_remote_console_acquire_ticket("webmks")
+    validate_supports(:webmks_console)
     ticket = ext_management_system.vm_remote_console_webmks_acquire_ticket(self)
 
     SystemConsole.force_vm_invalid_token(id)
@@ -85,12 +84,6 @@ def remote_console_webmks_acquire_ticket(userid, originating_server = nil)
     SystemConsole.launch_proxy_if_not_local(console_args, originating_server, ticket['host'].to_s, ticket['port'].to_i)
   end
 
-  def validate_remote_console_webmks_support
-    validate_remote_console_acquire_ticket("webmks")
-    validate_supports(ext_management_system.unsupported_reason(:webmks_console))
-    true
-  end
-
   #
   # HTML5 selects the best available console type (VNC or WebMKS)
   #
@@ -105,7 +98,7 @@ def remote_console_html5_acquire_ticket(userid, originating_server = nil)
   def remote_console_vnc_acquire_ticket(userid, originating_server)
     require 'securerandom'
 
-    validate_remote_console_acquire_ticket("vnc")
+    validate_supports(:vnc_console)
 
     password     = SecureRandom.base64[0, 8] # Random password from the Base64 character set
     host_port    = host.reserve_next_available_vnc_port
@@ -144,12 +137,10 @@ def remote_console_vnc_acquire_ticket(userid, originating_server)
 
   private
 
-  # @param unsupported_reason [Nil,String, Symbol]
-  #        a symbol will lookup the unsupported reason
-  #        otherwise, will raise an error if there is a reason to
-  def validate_supports(unsupported_reason)
-    unsupported_reason = unsupported_reason(unsupported_reason) if unsupported_reason.kind_of?(Symbol)
-    raise(MiqException::RemoteConsoleNotSupportedError, unsupported_reason) if unsupported_reason
+  def validate_supports(feature)
+    if (unsupported_reason = unsupported_reason(feature))
+      raise(MiqException::RemoteConsoleNotSupportedError, unsupported_reason)
+    end
   end
 
   # Method to generate the remote URI for the VMRC console
diff --git a/spec/models/manageiq/providers/vmware/infra_manager/vm/remote_console_spec.rb b/spec/models/manageiq/providers/vmware/infra_manager/vm/remote_console_spec.rb
index ba5c50ae7..92d44d082 100644
--- a/spec/models/manageiq/providers/vmware/infra_manager/vm/remote_console_spec.rb
+++ b/spec/models/manageiq/providers/vmware/infra_manager/vm/remote_console_spec.rb
@@ -126,25 +126,24 @@
     end
   end
 
-  context '#validate_remote_console_webmks_support' do
+  context '#supports?(:webmks_console)' do
     before do
       ems.authentications << FactoryBot.create(:authentication, :authtype => :console, :userid => "root", :password => "vmware")
     end
 
     it 'normal case' do
       ems.update_attribute(:api_version, '6.0')
-      expect(vm.validate_remote_console_webmks_support).to be_truthy
+      expect(vm.supports?(:webmks_console)).to be_truthy
     end
 
     it 'with vm with no ems' do
-      vm.ext_management_system = nil
-      vm.save!
-      expect { vm.validate_remote_console_webmks_support }.to raise_error MiqException::RemoteConsoleNotSupportedError
+      vm.update!(:ext_management_system => nil)
+      expect(vm.supports?(:webmks_console)).to be_falsey
     end
 
     it 'with vm off' do
-      vm.update_attribute(:raw_power_state, 'poweredOff')
-      expect { vm.validate_remote_console_webmks_support }.to raise_error MiqException::RemoteConsoleNotSupportedError
+      vm.update(:raw_power_state => 'poweredOff')
+      expect(vm.supports?(:webmks_console)).to be_falsey
     end
   end
 
@@ -180,24 +179,24 @@
     end
   end
 
-  context '#validate_remote_console_vmrc_support' do
+  context '#supports?(:vmrc_console)' do
     before do
       ems.authentications << FactoryBot.create(:authentication, :authtype => :console, :userid => "root", :password => "vmware")
     end
 
     it 'normal case' do
-      expect(vm.validate_remote_console_vmrc_support).to be_truthy
+      expect(vm.supports?(:vmrc_console)).to be_truthy
     end
 
     it 'with vm with no ems' do
       vm.ext_management_system = nil
       vm.save!
-      expect { vm.validate_remote_console_vmrc_support }.to raise_error MiqException::RemoteConsoleNotSupportedError
+      expect(vm.supports?(:vmrc_console)).to be_falsey
     end
 
     it 'with vm off' do
       vm.update_attribute(:raw_power_state, 'poweredOff')
-      expect { vm.validate_remote_console_vmrc_support }.to raise_error MiqException::RemoteConsoleNotSupportedError
+      expect(vm.supports?(:vmrc_console)).to be_falsey
     end
   end
 
diff --git a/spec/models/manageiq/providers/vmware/infra_manager_spec.rb b/spec/models/manageiq/providers/vmware/infra_manager_spec.rb
index b10e94508..570284f1b 100644
--- a/spec/models/manageiq/providers/vmware/infra_manager_spec.rb
+++ b/spec/models/manageiq/providers/vmware/infra_manager_spec.rb
@@ -168,7 +168,7 @@
 
   context "#supports?(:vmrc_console)" do
     before(:each) do
-      @ems = FactoryBot.create(:ems_vmware)
+      @ems = FactoryBot.create(:ems_vmware_with_authentication, :authtype => 'console')
     end
 
     it "true with nothing missing/blank" do