Merge pull request #18309 from djberg96/conversion_host_auth

[V2V] Refactor ConversionHost to use AuthenticationMixin

app/models/conversion_host.rb

@@ -1,11 +1,16 @@
 class ConversionHost < ApplicationRecord
   include NewWithTypeStiMixin
+  include AuthenticationMixin
 
   acts_as_miq_taggable
 
   belongs_to :resource, :polymorphic => true
   has_many :service_template_transformation_plan_tasks, :dependent => :nullify
-  has_many :active_tasks, -> { where(:state => ['active', 'migrate']) }, :class_name => ServiceTemplateTransformationPlanTask, :inverse_of => :conversion_host
+
+  has_many :active_tasks, -> { where(:state => ['active', 'migrate']) },
+    :class_name => ServiceTemplateTransformationPlanTask,
+    :inverse_of => :conversion_host
+
   delegate :ext_management_system, :hostname, :ems_ref, :to => :resource, :allow_nil => true
 
   validates :name, :presence => true
@@ -26,31 +31,98 @@ class ConversionHost < ApplicationRecord
   after_create :tag_resource_as_enabled
   after_destroy :tag_resource_as_disabled
 
-  # To be eligible, a conversion host must have the following properties
-  #  - A transport mechanism is configured for source (set by 3rd party)
-  #  - Credentials are set on the resource and SSH connection works
-  #  - The number of concurrent tasks has not reached the limit
+  # Use the +auth_type+ if present, or check the first associated authentication
+  # if any are directly associated with the conversion host. Otherwise, use the
+  # default check which uses the associated resource's authentications.
+  #
+  # In practice there should only be one associated authentication.
+  #
+  # Subclasses should pass provider-specific +options+, such as proxy information.
+  #
+  # This method is necessary to comply with AuthenticationMixin interface.
+  #--
+  # TODO: Use the verify_credentials_ssh method in host.rb? Move that to the
+  # AuthenticationMixin?
+  #
+  def verify_credentials(auth_type = nil, options = {})
+    if authentications.empty?
+      check_ssh_connection
+    else
+      require 'net/ssh'
+      host = hostname || ipaddress
+
+      auth = authentication_type(auth_type) || authentications.first
+
+      ssh_options = { :timeout => 10, :logger => $log, :verbose => :error }
+
+      case auth
+      when AuthUseridPassword
+        ssh_options[:auth_methods] = %w[password]
+        ssh_options[:password] = auth.password
+      when AuthPrivateKey
+        ssh_options[:auth_methods] = %w[publickey hostbased]
+        ssh_options[:key_data] = auth.auth_key
+      else
+        raise MiqException::MiqInvalidCredentialsError, _("Unknown auth type: #{auth.authtype}")
+      end
+
+      # Options from STI subclasses will override the defaults we've set above.
+      ssh_options.merge!(options)
+
+      Net::SSH.start(host, auth.userid, ssh_options) { |ssh| ssh.exec!('uname -a') }
+    end
+  rescue Net::SSH::AuthenticationFailed => err
+    raise MiqException::MiqInvalidCredentialsError, _("Incorrect credentials - %{error_message}") % {:error_message => err.message}
+  rescue Net::SSH::HostKeyMismatch => err
+    raise MiqException::MiqSshUtilHostKeyMismatch, _("Host key mismatch - %{error_message}") % {:error_message => err.message}
+  rescue Exception => err
+    raise _("Unknown error - %{error_message}") % {:error_message => err.message}
+  else
+    true
+  end
+
+  # Returns a boolean indicating whether or not the conversion host is eligible
+  # for use. To be eligible, a conversion host must have the following properties:
+  #
+  #  - A transport mechanism is configured for source (set by 3rd party).
+  #  - Credentials are set on the conversion host and the SSH connection works.
+  #  - The number of concurrent tasks has not reached the limit.
+  #
   def eligible?
-    source_transport_method.present? && check_ssh_connection && check_concurrent_tasks
+    source_transport_method.present? && verify_credentials && check_concurrent_tasks
   end
 
+  # Returns a boolean indicating whether or not the current number of active tasks
+  # exceeds the maximum number of allowable concurrent tasks specified in settings.
+  #
   def check_concurrent_tasks
     max_tasks = max_concurrent_tasks || Settings.transformation.limits.max_concurrent_tasks_per_host
     active_tasks.size < max_tasks
   end
 
+  # Check to see if we can connect to the conversion host using a simple 'uname -a'
+  # command on the connection. The exact nature of the connection will depend on the
+  # underlying provider.
+  #
   def check_ssh_connection
     connect_ssh { |ssu| ssu.shell_exec('uname -a') }
     true
-  rescue => e
+  rescue StandardError
     false
   end
 
+  # If set, returns a string indicating the source transport method. This is
+  # either 'vddk' or 'ssh'. If not set, returns nil.
+  #
   def source_transport_method
     return 'vddk' if vddk_transport_supported?
     return 'ssh' if ssh_transport_supported?
   end
 
+  # Returns the associated IP address for the conversion host in the given +family+.
+  # If an address is set for the conversion host, then that address will be
+  # returned. Otherwise, it will return the IP address of the associated resource.
+  #
   def ipaddress(family = 'ipv4')
     return address if address.present? && IPAddr.new(address).send("#{family}?")
     resource.ipaddresses.detect { |ip| IPAddr.new(ip).send("#{family}?") }
@@ -63,20 +135,31 @@ def run_conversion(conversion_options)
     raise "Starting conversion failed on '#{resource.name}' with [#{e.class}: #{e}]"
   end
 
+  # Kill a specific remote process over ssh, sending the specified +signal+, or 'TERM'
+  # if no signal is specified.
+  #
   def kill_process(pid, signal = 'TERM')
     connect_ssh { |ssu| ssu.shell_exec("/bin/kill -s #{signal} #{pid}") }
     true
   rescue
     false
   end
 
+  # Get the conversion state by reading from a remote json file at +path+
+  # and return the parsed data.
+  #--
+  # TODO: This should be more clear on failure, since it could fail because
+  # it's not found or because the contents were invalid.
+  #
   def get_conversion_state(path)
     json_state = connect_ssh { |ssu| ssu.get_file(path, nil) }
     JSON.parse(json_state)
   rescue => e
     raise "Could not get state file '#{path}' from '#{resource.name}' with [#{e.class}: #{e}"
   end
 
+  # Get and return the contents of the remote conversion log at +path+.
+  #
   def get_conversion_log(path)
     connect_ssh { |ssu| ssu.get_file(path, nil) }
   rescue => e
@@ -124,28 +207,29 @@ def resource_supports_conversion_host
     end
   end
 
-  def check_resource_credentials(fatal = false, extra_msg = nil)
-    success = send("check_resource_credentials_#{resource.ext_management_system.emstype}")
-    if !success and fatal
-      msg = "Credential not found for #{resource.name}."
-      msg += " #{extra_msg}" unless extra_msg.blank?
-      _log.error(:msg)
+  # Find the credentials for the associated resource. By default it will
+  # look for a v2v auth type. If that is not found, it will look for the
+  # authentication associated with the resource using ssh_keypair or default,
+  # in that order, as the authtype.
+  #
+  def find_credentials(msg = nil)
+    authentication = authentication_type('v2v') ||
+      resource.authentication_type('ssh_keypair') ||
+      resource.authentication_type('default')
+
+    unless authentication
+      msg = "Credentials not found for conversion host #{name} or resource #{resource.name}"
+      msg << " #{msg}" if msg
+      _log.error(msg)
       raise MiqException::Error, msg
     end
-    success
-  end
-
-  def check_resource_credentials_rhevm
-    !(resource.authentication_userid.nil? || resource.authentication_password.nil?)
-  end
 
-  def check_resource_credentials_openstack
-    ssh_authentications = resource.ext_management_system.authentications
-                                  .where(:authtype => 'ssh_keypair')
-                                  .where.not(:userid => nil, :auth_key => nil)
-    !ssh_authentications.empty?
+    authentication
   end
 
+  # Connect to the conversion host using the MiqSshUtil wrapper using the authentication
+  # parameters appropriate for that type of resource.
+  #
   def connect_ssh
     require 'MiqSshUtil'
     MiqSshUtil.shell_with_su(*miq_ssh_util_args) do |ssu, _shell|
@@ -156,19 +240,30 @@ def connect_ssh
     raise e
   end
 
+  # Collect appropriate authentication information based on the resource type.
+  #--
+  # TODO: This should be handled by a ConversionHost subclass within each supported provider.
+  #
   def miq_ssh_util_args
     send("miq_ssh_util_args_#{resource.type.gsub('::', '_').downcase}")
   end
 
+  # For the Redhat provider, use the userid and password associated directly with the resource.
+  #--
+  # TODO: Move this to ManageIQ::Providers::Redhat::InfraManager::ConversionHost
+  #
   def miq_ssh_util_args_manageiq_providers_redhat_inframanager_host
-    [hostname || ipaddress, resource.authentication_userid, resource.authentication_password, nil, nil]
+    authentication = find_credentials
+    [hostname || ipaddress, authentication.userid, authentication.password, nil, nil]
   end
 
+  # For the OpenStack provider, use the first authentication containing an ssh keypair that has
+  # both a userid and auth key.
+  #--
+  # TODO: Move this to ManageIQ::Providers::OpenStack::CloudManager::ConversionHost
+  #
   def miq_ssh_util_args_manageiq_providers_openstack_cloudmanager_vm
-    authentication = resource.ext_management_system.authentications
-                             .where(:authtype => 'ssh_keypair')
-                             .where.not(:userid => nil, :auth_key => nil)
-                             .first
+    authentication = find_credentials
     [hostname || ipaddress, authentication.userid, nil, nil, nil, { :key_data => authentication.auth_key, :passwordless_sudo => true }]
   end
 

app/models/conversion_host/configurations.rb

@@ -16,14 +16,16 @@ def queue_configuration(op, instance_id, resource, params, auth_user = nil)
         :action => "Configuring a conversion_host: operation=#{op} resource=(name: #{resource.name} type: #{resource.class.name} id: #{resource.id})",
         :userid => auth_user
       }
+
       queue_opts = {
         :class_name  => name,
         :method_name => op,
         :instance_id => instance_id,
         :role        => 'ems_operations',
         :zone        => resource.ext_management_system.my_zone,
-        :args        => params
+        :args        => [params, auth_user]
       }
+
       MiqTask.generic_action_with_callback(task_opts, queue_opts)
     end
 
@@ -34,10 +36,10 @@ def enable_queue(params, auth_user = nil)
       params[:resource_id] = resource.id
       params[:resource_type] = resource.class.name
 
-      queue_configuration('enable', nil, resource, [params], auth_user)
+      queue_configuration('enable', nil, resource, params, auth_user)
     end
 
-    def enable(params)
+    def enable(params, auth_user = nil)
       params = params.symbolize_keys
       _log.info("Enabling a conversion_host with parameters: #{params}")
 
@@ -49,10 +51,21 @@ def enable(params)
       vmware_ssh_private_key = params.delete(:vmware_ssh_private_key)
       params[:ssh_transport_supported] = !vmware_ssh_private_key.nil?
 
-      conversion_host = new(params)
-      conversion_host.enable_conversion_host_role(vmware_vddk_package_url, vmware_ssh_private_key)
-      conversion_host.save!
-      conversion_host
+      ssh_key = params.delete(:conversion_host_ssh_private_key)
+
+      new(params).tap do |conversion_host|
+        if ssh_key
+          conversion_host.authentications << AuthPrivateKey.create!(
+            :name     => conversion_host.name,
+            :auth_key => ssh_key,
+            :userid   => auth_user,
+            :authtype => 'v2v'
+          )
+        end
+
+        conversion_host.enable_conversion_host_role(vmware_vddk_package_url, vmware_ssh_private_key)
+        conversion_host.save!
+      end
     rescue StandardError => error
       raise
     ensure
@@ -62,7 +75,7 @@ def enable(params)
   end
 
   def disable_queue(auth_user = nil)
-    self.class.queue_configuration('disable', id, resource, [], auth_user)
+    self.class.queue_configuration('disable', id, resource, {}, auth_user)
   end
 
   def disable

app/models/mixins/authentication_mixin.rb

@@ -67,7 +67,7 @@ def authentication_tokens
   end
 
   def authentication_key_pairs
-    authentications.select { |a| a.kind_of?(ManageIQ::Providers::Openstack::InfraManager::AuthKeyPair) }
+    authentications.select { |a| a.kind_of?(AuthPrivateKey) }
   end
 
   def authentication_for_providers

spec/models/conversion_host/configurations_spec.rb

@@ -121,7 +121,7 @@
         task_id = described_class.enable_queue(params)
         expect(MiqTask.find(task_id)).to have_attributes(:name => expected_task_action)
         expect(MiqQueue.first).to have_attributes(
-          :args        => [params.merge(:task_id => task_id).except(:resource)],
+          :args        => [params.merge(:task_id => task_id).except(:resource), nil],
           :class_name  => described_class.name,
           :method_name => "enable",
           :priority    => MiqQueue::NORMAL_PRIORITY,
@@ -138,7 +138,7 @@
         task_id = conversion_host.disable_queue
         expect(MiqTask.find(task_id)).to have_attributes(:name => expected_task_action)
         expect(MiqQueue.first).to have_attributes(
-          :args        => [],
+          :args        => [{:task_id => task_id}, nil],
           :class_name  => described_class.name,
           :instance_id => conversion_host.id,
           :method_name => "disable",