From 0aef0d0a053f066e17761f3d03bdec98cdae0228 Mon Sep 17 00:00:00 2001 From: Keenan Brock Date: Tue, 7 Mar 2023 17:44:34 -0500 Subject: [PATCH 1/3] convert console_supported? to supports? --- .../vmware/cloud_manager/vm/remote_console.rb | 8 ++++++-- .../vmware/infra_manager/vm/remote_console.rb | 10 ++++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/app/models/manageiq/providers/vmware/cloud_manager/vm/remote_console.rb b/app/models/manageiq/providers/vmware/cloud_manager/vm/remote_console.rb index 69ab52d2a..3c2ccae3c 100644 --- a/app/models/manageiq/providers/vmware/cloud_manager/vm/remote_console.rb +++ b/app/models/manageiq/providers/vmware/cloud_manager/vm/remote_console.rb @@ -1,6 +1,10 @@ module ManageIQ::Providers::Vmware::CloudManager::Vm::RemoteConsole - def console_supported?(type) - %w(WEBMKS).include?(type.upcase) + extend ActiveSupport::Concern + + included do + supports :console + supports :html5_console + supports :webmks_console end def validate_remote_console_acquire_ticket(protocol, options = {}) 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 632c54d0c..e29db89be 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 @@ -1,6 +1,12 @@ module ManageIQ::Providers::Vmware::InfraManager::Vm::RemoteConsole - def console_supported?(type) - %w(VMRC VNC WEBMKS).include?(type.upcase) + extend ActiveSupport::Concern + + included do + supports :console + supports :html5_console + supports :vmrc_console + supports :vnc_console + supports :webmks_console end def validate_remote_console_acquire_ticket(protocol, options = {}) From 9d663a6b3fa0dc4e2bbccc8dd067ade352aa5504 Mon Sep 17 00:00:00 2001 From: Keenan Brock Date: Wed, 22 Mar 2023 20:35:07 -0400 Subject: [PATCH 2/3] convert ems#validate_remote_console_vmrc_support to supports --- .../manageiq/providers/vmware/infra_manager.rb | 18 +++++------------- .../vmware/infra_manager/vm/remote_console.rb | 12 ++++++++++-- .../providers/vmware/infra_manager_spec.rb | 18 +++++++++--------- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/app/models/manageiq/providers/vmware/infra_manager.rb b/app/models/manageiq/providers/vmware/infra_manager.rb index fd8b58e42..70a5d762a 100644 --- a/app/models/manageiq/providers/vmware/infra_manager.rb +++ b/app/models/manageiq/providers/vmware/infra_manager.rb @@ -39,6 +39,11 @@ class Vmware::InfraManager < InfraManager supports :label_mapping 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? + end + supports :webmks_console + supports :provisioning supports :smartstate_analysis supports :streaming_refresh do @@ -332,19 +337,6 @@ def remote_console_vmrc_acquire_ticket ticket end - def remote_console_vmrc_support_known? - !api_version.blank? && !hostname.blank? && !uid_ems.blank? - end - - def validate_remote_console_vmrc_support - raise(MiqException::RemoteConsoleNotSupportedError, "vCenter needs to be refreshed to determine VMRC remote console support.") unless self.remote_console_vmrc_support_known? - true - end - - def validate_remote_console_webmks_support - true - end - def after_update_authentication super stop_refresh_worker_queue_on_credential_change 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 e29db89be..501ec1b78 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 @@ -59,7 +59,7 @@ def remote_console_vmrc_acquire_ticket(_userid = nil, _originating_server = nil) def validate_remote_console_vmrc_support validate_remote_console_acquire_ticket("vmrc") - ext_management_system.validate_remote_console_vmrc_support + validate_supports(ext_management_system.unsupported_reason(:vmrc_console)) true end @@ -87,7 +87,7 @@ def remote_console_webmks_acquire_ticket(userid, originating_server = nil) def validate_remote_console_webmks_support validate_remote_console_acquire_ticket("webmks") - ext_management_system.validate_remote_console_webmks_support + validate_supports(ext_management_system.unsupported_reason(:webmks_console)) true end @@ -144,6 +144,14 @@ 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 + end + # Method to generate the remote URI for the VMRC console def build_vmrc_url(ticket) url = URI::Generic.build(:scheme => "vmrc", diff --git a/spec/models/manageiq/providers/vmware/infra_manager_spec.rb b/spec/models/manageiq/providers/vmware/infra_manager_spec.rb index 44c86e988..b10e94508 100644 --- a/spec/models/manageiq/providers/vmware/infra_manager_spec.rb +++ b/spec/models/manageiq/providers/vmware/infra_manager_spec.rb @@ -155,50 +155,50 @@ end end - context "#validate_remote_console_vmrc_support" do + context "#supports?(:vmrc_console)" do before(:each) do @ems = FactoryBot.create(:ems_vmware) end it "raise for missing/blank values" do @ems.update(:api_version => "", :uid_ems => "2E1C1E82-BD83-4E54-9271-630C6DFAD4D1") - expect { @ems.validate_remote_console_vmrc_support }.to raise_error MiqException::RemoteConsoleNotSupportedError + expect(@ems.supports?(:vmrc_console)).to be_falsey end end - context "#remote_console_vmrc_support_known?" do + context "#supports?(:vmrc_console)" do before(:each) do @ems = FactoryBot.create(:ems_vmware) end it "true with nothing missing/blank" do @ems.update(:api_version => "5.0", :uid_ems => "2E1C1E82-BD83-4E54-9271-630C6DFAD4D1") - expect(@ems.remote_console_vmrc_support_known?).to be_truthy + expect(@ems.supports?(:vmrc_console)).to be_truthy end it "false for blank hostname" do @ems.update(:hostname => "", :api_version => "5.0", :uid_ems => "2E1C1E82-BD83-4E54-9271-630C6DFAD4D1") - expect(@ems.remote_console_vmrc_support_known?).not_to be_truthy + expect(@ems.supports?(:vmrc_console)).not_to be_truthy end it "false for missing api_version" do @ems.update(:api_version => nil, :uid_ems => "2E1C1E82-BD83-4E54-9271-630C6DFAD4D1") - expect(@ems.remote_console_vmrc_support_known?).not_to be_truthy + expect(@ems.supports?(:vmrc_console)).not_to be_truthy end it "false for blank api_version" do @ems.update(:api_version => "", :uid_ems => "2E1C1E82-BD83-4E54-9271-630C6DFAD4D1") - expect(@ems.remote_console_vmrc_support_known?).not_to be_truthy + expect(@ems.supports?(:vmrc_console)).not_to be_truthy end it "false for missing uid_ems" do @ems.update(:api_version => "5.0", :uid_ems => nil) - expect(@ems.remote_console_vmrc_support_known?).not_to be_truthy + expect(@ems.supports?(:vmrc_console)).not_to be_truthy end it "false for blank uid_ems" do @ems.update(:api_version => "5.0", :uid_ems => "") - expect(@ems.remote_console_vmrc_support_known?).not_to be_truthy + expect(@ems.supports?(:vmrc_console)).not_to be_truthy end end From 2e9ced0ff20b85276fdcbe0a3d3e9a2432cc1266 Mon Sep 17 00:00:00 2001 From: Keenan Brock Date: Wed, 22 Mar 2023 21:12:21 -0400 Subject: [PATCH 3/3] moving conditional supports logic from validate and launch to console supports vnc, webmks, and console are not consistent webmks also adds login validation checking. Added the vm must be running check from :launch_{}_console feature. That feature has been rolled into :{}_console --- .../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, 40 insertions(+), 48 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..b567394a7 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,23 @@ module ManageIQ::Providers::Vmware::InfraManager::Vm::RemoteConsole extend ActiveSupport::Concern included do - supports :console - 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 :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) { unsupported_reason(:console) } + supports :vmrc_console do + unsupported_reason(:console) || + ext_management_system.unsupported_reason(:vmrc_console) + end + supports :vnc_console { unsupported_reason(:console) } + 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 +50,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 +60,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 +82,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 +96,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 +135,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