use correct executable and options in all cases (#73323)

Use correct ssh executable and options in all cases on connection plugin

  * Also nicer naming/comments

Co-authored-by: Sviatoslav Sydorenko <wk.cvs.github@sydorenko.org.ua>

changelogs/fragments/fix_ssh_executable_options.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - Ensure the correct options are used when ssh executables are used that don't match ssh executable names.

lib/ansible/plugins/connection/ssh.py

@@ -558,10 +558,15 @@ class Connection(ConnectionBase):
         display.vvvvv(u'SSH: %s: (%s)' % (explanation, ')('.join(to_text(a) for a in b_args)), host=self._play_context.remote_addr)
         b_command += b_args
 
-    def _build_command(self, binary, *other_args):
+    def _build_command(self, binary, subsystem, *other_args):
         '''
-        Takes a binary (ssh, scp, sftp) and optional extra arguments and returns
+        Takes a executable (ssh, scp, sftp or wrapper) and optional extra arguments and returns the remote command
-        a command line as an array that can be passed to subprocess.Popen.
+        wrapped in local ssh shell commands and ready for execution.
+
+        :arg binary: actual executable to use to execute command.
+        :arg subsystem: type of executable provided, ssh/sftp/scp, needed because wrappers for ssh might have diff names.
+        :arg other_args: dict of, value pairs passed as arguments to the ssh binary
+
         '''
 
         b_command = []
@@ -585,10 +590,7 @@ class Connection(ConnectionBase):
             if password_prompt:
                 b_command += [b'-P', to_bytes(password_prompt, errors='surrogate_or_strict')]
 
-        if binary == 'ssh':
+        b_command += [to_bytes(binary, errors='surrogate_or_strict')]
-            b_command += [to_bytes(self._play_context.ssh_executable, errors='surrogate_or_strict')]
-        else:
-            b_command += [to_bytes(binary, errors='surrogate_or_strict')]
 
         #
         # Next, additional arguments based on the configuration.
@@ -598,7 +600,7 @@ class Connection(ConnectionBase):
         # be disabled if the client side doesn't support the option. However,
         # sftp batch mode does not prompt for passwords so it must be disabled
         # if not using controlpersist and using sshpass
-        if binary == 'sftp' and C.DEFAULT_SFTP_BATCH_MODE:
+        if subsystem == 'sftp' and C.DEFAULT_SFTP_BATCH_MODE:
             if conn_password:
                 b_args = [b'-o', b'BatchMode=no']
                 self._add_args(b_command, b_args, u'disable batch mode for sshpass')
@@ -661,7 +663,7 @@ class Connection(ConnectionBase):
         # Add in any common or binary-specific arguments from the PlayContext
         # (i.e. inventory or task settings or overrides on the command line).
 
-        for opt in (u'ssh_common_args', u'{0}_extra_args'.format(binary)):
+        for opt in (u'ssh_common_args', u'{0}_extra_args'.format(subsystem)):
             attr = getattr(self._play_context, opt, None)
             if attr is not None:
                 b_args = [to_bytes(a, errors='surrogate_or_strict') for a in self._split_ssh_args(attr)]
@@ -1126,7 +1128,7 @@ class Connection(ConnectionBase):
         for method in methods:
             returncode = stdout = stderr = None
             if method == 'sftp':
-                cmd = self._build_command(self.get_option('sftp_executable'), to_bytes(host))
+                cmd = self._build_command(self.get_option('sftp_executable'), 'sftp', to_bytes(host))
                 in_data = u"{0} {1} {2}\n".format(sftp_action, shlex_quote(in_path), shlex_quote(out_path))
                 in_data = to_bytes(in_data, nonstring='passthru')
                 (returncode, stdout, stderr) = self._bare_run(cmd, in_data, checkrc=False)
@@ -1134,9 +1136,9 @@ class Connection(ConnectionBase):
                 scp = self.get_option('scp_executable')
 
                 if sftp_action == 'get':
-                    cmd = self._build_command(scp, u'{0}:{1}'.format(host, self._shell.quote(in_path)), out_path)
+                    cmd = self._build_command(scp, 'scp', u'{0}:{1}'.format(host, self._shell.quote(in_path)), out_path)
                 else:
-                    cmd = self._build_command(scp, in_path, u'{0}:{1}'.format(host, self._shell.quote(out_path)))
+                    cmd = self._build_command(scp, 'scp', in_path, u'{0}:{1}'.format(host, self._shell.quote(out_path)))
                 in_data = None
                 (returncode, stdout, stderr) = self._bare_run(cmd, in_data, checkrc=False)
             elif method == 'piped':
@@ -1208,18 +1210,18 @@ class Connection(ConnectionBase):
         # python interactive-mode but the modules are not compatible with the
         # interactive-mode ("unexpected indent" mainly because of empty lines)
 
-        ssh_executable = self._play_context.ssh_executable
+        ssh_executable = self.get_option('ssh_executable') or self._play_context.ssh_executable
 
         # -tt can cause various issues in some environments so allow the user
         # to disable it as a troubleshooting method.
         use_tty = self.get_option('use_tty')
 
         if not in_data and sudoable and use_tty:
-            args = (ssh_executable, '-tt', self.host, cmd)
+            args = ('-tt', self.host, cmd)
         else:
-            args = (ssh_executable, self.host, cmd)
+            args = (self.host, cmd)
 
-        cmd = self._build_command(*args)
+        cmd = self._build_command(ssh_executable, 'ssh', *args)
         (returncode, stdout, stderr) = self._run(cmd, in_data, sudoable=sudoable)
 
         # When running on Windows, stderr may contain CLIXML encoded output
@@ -1257,7 +1259,7 @@ class Connection(ConnectionBase):
 
     def reset(self):
         # If we have a persistent ssh connection (ControlPersist), we can ask it to stop listening.
-        cmd = self._build_command(self._play_context.ssh_executable, '-O', 'stop', self.host)
+        cmd = self._build_command(self.get_option('ssh_executable') or self._play_context.ssh_executable, 'ssh', '-O', 'stop', self.host)
         controlpersist, controlpath = self._persistence_controls(cmd)
         cp_arg = [a for a in cmd if a.startswith(b"ControlPath=")]
 

test/units/plugins/connection/test_ssh.py

@@ -80,7 +80,7 @@ class TestConnectionBaseClass(unittest.TestCase):
         pc = PlayContext()
         new_stdin = StringIO()
         conn = connection_loader.get('ssh', pc, new_stdin)
-        conn._build_command('ssh')
+        conn._build_command('ssh', 'ssh')
 
     def test_plugins_connection_ssh_exec_command(self):
         pc = PlayContext()