From a1428d6bed4517383549dbd14b99653462d4aefb Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Thu, 24 Sep 2015 13:29:36 -0700 Subject: [PATCH] Remove tmp as a parameter to the connection plugins There doesn't appear to be anything that actually uses tmp_path in the connection plugins so we don't need to pass that in to exec_command. That change also means that we don't need to pass tmp_path around in many places in the action plugins any more. there may be more cleanup that can be done there as well (the action plugin's public run() method takes tmp as a keyword arg but that may not be necessary). As a sideeffect of this patch, some potential problems with chmod and the patch, assemble, copy, and template modules has been fixed (those modules called _remote_chmod() with the wrong order for their parameters. Removing the tmp parameter fixed them.) --- lib/ansible/modules/core | 2 +- lib/ansible/plugins/action/__init__.py | 28 +++++++++---------- lib/ansible/plugins/action/assemble.py | 8 +++--- lib/ansible/plugins/action/async.py | 6 ++-- lib/ansible/plugins/action/copy.py | 10 +++---- lib/ansible/plugins/action/fetch.py | 4 +-- lib/ansible/plugins/action/patch.py | 2 +- lib/ansible/plugins/action/raw.py | 2 +- lib/ansible/plugins/action/script.py | 4 +-- lib/ansible/plugins/action/template.py | 14 +++++----- lib/ansible/plugins/action/unarchive.py | 6 ++-- lib/ansible/plugins/connection/__init__.py | 13 +++++++-- lib/ansible/plugins/connection/accelerate.py | 3 +- lib/ansible/plugins/connection/chroot.py | 10 +++---- lib/ansible/plugins/connection/docker.py | 4 +-- lib/ansible/plugins/connection/funcd.py | 2 +- lib/ansible/plugins/connection/jail.py | 6 ++-- lib/ansible/plugins/connection/libvirt_lxc.py | 2 +- lib/ansible/plugins/connection/local.py | 4 +-- .../plugins/connection/paramiko_ssh.py | 4 +-- lib/ansible/plugins/connection/ssh.py | 4 +-- lib/ansible/plugins/connection/winrm.py | 4 +-- lib/ansible/plugins/connection/zone.py | 6 ++-- test/units/plugins/action/test_action.py | 6 ++-- 24 files changed, 81 insertions(+), 73 deletions(-) diff --git a/lib/ansible/modules/core b/lib/ansible/modules/core index e8227dea7a..a580acc12a 160000 --- a/lib/ansible/modules/core +++ b/lib/ansible/modules/core @@ -1 +1 @@ -Subproject commit e8227dea7a28bc39bf15f3bb26b76f3f29d5b60d +Subproject commit a580acc12a28b48b432607ad506f1d410db742ae diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index ee104bd139..d9ee141e0e 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -177,7 +177,7 @@ class ActionBase: cmd = self._connection._shell.mkdtemp(basefile, use_system_tmp, tmp_mode) self._display.debug("executing _low_level_execute_command to create the tmp path") - result = self._low_level_execute_command(cmd, None, sudoable=False) + result = self._low_level_execute_command(cmd, sudoable=False) self._display.debug("done with creation of tmp path") # error handling on this seems a little aggressive? @@ -218,7 +218,7 @@ class ActionBase: # If we have gotten here we have a working ssh configuration. # If ssh breaks we could leave tmp directories out on the remote system. self._display.debug("calling _low_level_execute_command to remove the tmp path") - self._low_level_execute_command(cmd, None, sudoable=False) + self._low_level_execute_command(cmd, sudoable=False) self._display.debug("done removing the tmp path") def _transfer_data(self, remote_path, data): @@ -248,18 +248,18 @@ class ActionBase: return remote_path - def _remote_chmod(self, tmp, mode, path, sudoable=False): + def _remote_chmod(self, mode, path, sudoable=False): ''' Issue a remote chmod command ''' cmd = self._connection._shell.chmod(mode, path) self._display.debug("calling _low_level_execute_command to chmod the remote path") - res = self._low_level_execute_command(cmd, tmp, sudoable=sudoable) + res = self._low_level_execute_command(cmd, sudoable=sudoable) self._display.debug("done with chmod call") return res - def _remote_checksum(self, tmp, path, all_vars): + def _remote_checksum(self, path, all_vars): ''' Takes a remote checksum and returns 1 if no file ''' @@ -268,7 +268,7 @@ class ActionBase: cmd = self._connection._shell.checksum(path, python_interp) self._display.debug("calling _low_level_execute_command to get the remote checksum") - data = self._low_level_execute_command(cmd, tmp, sudoable=True) + data = self._low_level_execute_command(cmd, sudoable=True) self._display.debug("done getting the remote checksum") # FIXME: implement this function? #data2 = utils.last_non_blank_line(data['stdout']) @@ -286,7 +286,7 @@ class ActionBase: # this will signal that it changed and allow things to keep going return "INVALIDCHECKSUM" - def _remote_expand_user(self, path, tmp): + def _remote_expand_user(self, path): ''' takes a remote path and performs tilde expansion on the remote host ''' if not path.startswith('~'): # FIXME: Windows paths may start with "~ instead of just ~ return path @@ -300,7 +300,7 @@ class ActionBase: cmd = self._connection._shell.expand_user(expand_path) self._display.debug("calling _low_level_execute_command to expand the remote user path") - data = self._low_level_execute_command(cmd, tmp, sudoable=False) + data = self._low_level_execute_command(cmd, sudoable=False) self._display.debug("done expanding the remote user path") #initial_fragment = utils.last_non_blank_line(data['stdout']) initial_fragment = data['stdout'].strip().splitlines()[-1] @@ -377,7 +377,7 @@ class ActionBase: if tmp and "tmp" in tmp and self._play_context.become and self._play_context.become_user != 'root': # deal with possible umask issues once sudo'ed to other user - self._remote_chmod(tmp, 'a+r', remote_module_path) + self._remote_chmod('a+r', remote_module_path) cmd = "" in_data = None @@ -407,7 +407,7 @@ class ActionBase: sudoable = False self._display.debug("calling _low_level_execute_command() for command %s" % cmd) - res = self._low_level_execute_command(cmd, tmp, sudoable=sudoable, in_data=in_data) + res = self._low_level_execute_command(cmd, sudoable=sudoable, in_data=in_data) self._display.debug("_low_level_execute_command returned ok") if tmp and "tmp" in tmp and not C.DEFAULT_KEEP_REMOTE_FILES and not persist_files and delete_remote_tmp: @@ -415,7 +415,7 @@ class ActionBase: # not sudoing to root, so maybe can't delete files as that other user # have to clean up temp files as original user in a second step cmd2 = self._connection._shell.remove(tmp, recurse=True) - self._low_level_execute_command(cmd2, tmp, sudoable=False) + self._low_level_execute_command(cmd2, sudoable=False) try: data = json.loads(self._filter_leading_non_json_lines(res.get('stdout', ''))) @@ -444,7 +444,7 @@ class ActionBase: self._display.debug("done with _execute_module (%s, %s)" % (module_name, module_args)) return data - def _low_level_execute_command(self, cmd, tmp, sudoable=True, in_data=None, executable=None): + def _low_level_execute_command(self, cmd, sudoable=True, in_data=None, executable=None): ''' This is the function which executes the low level shell command, which may be commands to create/remove directories for temporary files, or to @@ -467,7 +467,7 @@ class ActionBase: cmd = self._play_context.make_become_cmd(cmd, executable=executable) self._display.debug("executing the command %s through the connection" % cmd) - rc, stdout, stderr = self._connection.exec_command(cmd, tmp, in_data=in_data, sudoable=sudoable) + rc, stdout, stderr = self._connection.exec_command(cmd, in_data=in_data, sudoable=sudoable) self._display.debug("command execution done") if not isinstance(stdout, string_types): @@ -510,7 +510,7 @@ class ActionBase: return None - def _get_diff_data(self, tmp, destination, source, task_vars, source_file=True): + def _get_diff_data(self, destination, source, task_vars, source_file=True): diff = {} self._display.debug("Going to peek to see if file has changed permissions") diff --git a/lib/ansible/plugins/action/assemble.py b/lib/ansible/plugins/action/assemble.py index 1f74ffe7bc..34dc98be06 100644 --- a/lib/ansible/plugins/action/assemble.py +++ b/lib/ansible/plugins/action/assemble.py @@ -106,21 +106,21 @@ class ActionModule(ActionBase): path = self._assemble_from_fragments(src, delimiter, _re, ignore_hidden) path_checksum = checksum_s(path) - dest = self._remote_expand_user(dest, tmp) - remote_checksum = self._remote_checksum(tmp, dest, all_vars=task_vars) + dest = self._remote_expand_user(dest) + remote_checksum = self._remote_checksum(dest, all_vars=task_vars) diff = {} if path_checksum != remote_checksum: resultant = file(path).read() if self._play_context.diff: - diff = self._get_diff_data(tmp, dest, path, task_vars) + diff = self._get_diff_data(dest, path, task_vars) xfered = self._transfer_data('src', resultant) # fix file permissions when the copy is done as a different user if self._play_context.become and self._play_context.become_user != 'root': - self._remote_chmod('a+r', xfered, tmp) + self._remote_chmod('a+r', xfered) # run the copy module diff --git a/lib/ansible/plugins/action/async.py b/lib/ansible/plugins/action/async.py index af850b41cb..39794e7b35 100644 --- a/lib/ansible/plugins/action/async.py +++ b/lib/ansible/plugins/action/async.py @@ -43,12 +43,12 @@ class ActionModule(ActionBase): # configure, upload, and chmod the target module (module_style, shebang, module_data) = self._configure_module(module_name=module_name, module_args=self._task.args, task_vars=task_vars) self._transfer_data(remote_module_path, module_data) - self._remote_chmod(tmp, 'a+rx', remote_module_path) + self._remote_chmod('a+rx', remote_module_path) # configure, upload, and chmod the async_wrapper module (async_module_style, shebang, async_module_data) = self._configure_module(module_name='async_wrapper', module_args=dict(), task_vars=task_vars) self._transfer_data(async_module_path, async_module_data) - self._remote_chmod(tmp, 'a+rx', async_module_path) + self._remote_chmod('a+rx', async_module_path) argsfile = self._transfer_data(self._connection._shell.join_path(tmp, 'arguments'), json.dumps(self._task.args)) @@ -56,7 +56,7 @@ class ActionModule(ActionBase): async_jid = str(random.randint(0, 999999999999)) async_cmd = " ".join([str(x) for x in [env_string, async_module_path, async_jid, async_limit, remote_module_path, argsfile]]) - result = self._low_level_execute_command(cmd=async_cmd, tmp=None) + result = self._low_level_execute_command(cmd=async_cmd) # clean up after if tmp and "tmp" in tmp and not C.DEFAULT_KEEP_REMOTE_FILES: diff --git a/lib/ansible/plugins/action/copy.py b/lib/ansible/plugins/action/copy.py index ecb1775a99..aca7bd3eb5 100644 --- a/lib/ansible/plugins/action/copy.py +++ b/lib/ansible/plugins/action/copy.py @@ -132,7 +132,7 @@ class ActionModule(ActionBase): tmp = self._make_tmp_path() # expand any user home dir specifier - dest = self._remote_expand_user(dest, tmp) + dest = self._remote_expand_user(dest) diffs = [] for source_full, source_rel in source_files: @@ -153,7 +153,7 @@ class ActionModule(ActionBase): dest_file = self._connection._shell.join_path(dest) # Attempt to get the remote checksum - remote_checksum = self._remote_checksum(tmp, dest_file, all_vars=task_vars) + remote_checksum = self._remote_checksum(dest_file, all_vars=task_vars) if remote_checksum == '3': # The remote_checksum was executed on a directory. @@ -164,7 +164,7 @@ class ActionModule(ActionBase): else: # Append the relative source location to the destination and retry remote_checksum dest_file = self._connection._shell.join_path(dest, source_rel) - remote_checksum = self._remote_checksum(tmp, dest_file, all_vars=task_vars) + remote_checksum = self._remote_checksum(dest_file, all_vars=task_vars) if remote_checksum != '1' and not force: # remote_file does not exist so continue to next iteration. @@ -181,7 +181,7 @@ class ActionModule(ActionBase): tmp = self._make_tmp_path() if self._play_context.diff and not raw: - diffs.append(self._get_diff_data(tmp, dest_file, source_full, task_vars)) + diffs.append(self._get_diff_data(dest_file, source_full, task_vars)) if self._play_context.check_mode: self._remove_tempfile_if_content_defined(content, content_tempfile) @@ -202,7 +202,7 @@ class ActionModule(ActionBase): # fix file permissions when the copy is done as a different user if self._play_context.become and self._play_context.become_user != 'root': - self._remote_chmod('a+r', tmp_src, tmp) + self._remote_chmod('a+r', tmp_src) if raw: # Continue to next iteration if raw is defined. diff --git a/lib/ansible/plugins/action/fetch.py b/lib/ansible/plugins/action/fetch.py index f963a07cb0..0d649a52c7 100644 --- a/lib/ansible/plugins/action/fetch.py +++ b/lib/ansible/plugins/action/fetch.py @@ -52,10 +52,10 @@ class ActionModule(ActionBase): return dict(failed=True, msg="src and dest are required") source = self._connection._shell.join_path(source) - source = self._remote_expand_user(source, tmp) + source = self._remote_expand_user(source) # calculate checksum for the remote file - remote_checksum = self._remote_checksum(tmp, source, all_vars=task_vars) + remote_checksum = self._remote_checksum(source, all_vars=task_vars) # use slurp if sudo and permissions are lacking remote_data = None diff --git a/lib/ansible/plugins/action/patch.py b/lib/ansible/plugins/action/patch.py index 5355ad6823..b411fcdc35 100644 --- a/lib/ansible/plugins/action/patch.py +++ b/lib/ansible/plugins/action/patch.py @@ -52,7 +52,7 @@ class ActionModule(ActionBase): if self._play_context.become and self._play_context.become_user != 'root': if not self._play_context.check_mode: - self._remote_chmod('a+r', tmp_src, tmp) + self._remote_chmod('a+r', tmp_src) new_module_args = self._task.args.copy() new_module_args.update( diff --git a/lib/ansible/plugins/action/raw.py b/lib/ansible/plugins/action/raw.py index 4d862d9ebb..8fd32c5342 100644 --- a/lib/ansible/plugins/action/raw.py +++ b/lib/ansible/plugins/action/raw.py @@ -31,7 +31,7 @@ class ActionModule(ActionBase): return dict(skipped=True) executable = self._task.args.get('executable') - result = self._low_level_execute_command(self._task.args.get('_raw_params'), tmp=tmp, executable=executable) + result = self._low_level_execute_command(self._task.args.get('_raw_params'), executable=executable) # for some modules (script, raw), the sudo success key # may leak into the stdout due to the way the sudo/su diff --git a/lib/ansible/plugins/action/script.py b/lib/ansible/plugins/action/script.py index d2fbf21cf0..31e0a1736c 100644 --- a/lib/ansible/plugins/action/script.py +++ b/lib/ansible/plugins/action/script.py @@ -78,13 +78,13 @@ class ActionModule(ActionBase): sudoable = False else: chmod_mode = '+rx' - self._remote_chmod(tmp, chmod_mode, tmp_src, sudoable=sudoable) + self._remote_chmod(chmod_mode, tmp_src, sudoable=sudoable) # add preparation steps to one ssh roundtrip executing the script env_string = self._compute_environment_string() script_cmd = ' '.join([env_string, tmp_src, args]) - result = self._low_level_execute_command(cmd=script_cmd, tmp=None, sudoable=True) + result = self._low_level_execute_command(cmd=script_cmd, sudoable=True) # clean up after if tmp and "tmp" in tmp and not C.DEFAULT_KEEP_REMOTE_FILES: diff --git a/lib/ansible/plugins/action/template.py b/lib/ansible/plugins/action/template.py index 6f24fb9c1c..3378b1d2b1 100644 --- a/lib/ansible/plugins/action/template.py +++ b/lib/ansible/plugins/action/template.py @@ -31,8 +31,8 @@ class ActionModule(ActionBase): TRANSFERS_FILES = True - def get_checksum(self, tmp, dest, all_vars, try_directory=False, source=None): - remote_checksum = self._remote_checksum(tmp, dest, all_vars=all_vars) + def get_checksum(self, dest, all_vars, try_directory=False, source=None): + remote_checksum = self._remote_checksum(dest, all_vars=all_vars) if remote_checksum in ('0', '2', '3', '4'): # Note: 1 means the file is not present which is fine; template @@ -40,7 +40,7 @@ class ActionModule(ActionBase): if try_directory and remote_checksum == '3' and source: base = os.path.basename(source) dest = os.path.join(dest, base) - remote_checksum = self.get_checksum(tmp, dest, all_vars=all_vars, try_directory=False) + remote_checksum = self.get_checksum(dest, all_vars=all_vars, try_directory=False) if remote_checksum not in ('0', '2', '3', '4'): return remote_checksum @@ -74,7 +74,7 @@ class ActionModule(ActionBase): source = self._loader.path_dwim_relative(self._loader.get_basedir(), 'templates', source) # Expand any user home dir specification - dest = self._remote_expand_user(dest, tmp) + dest = self._remote_expand_user(dest) directory_prepended = False if dest.endswith(os.sep): @@ -128,7 +128,7 @@ class ActionModule(ActionBase): return dict(failed=True, msg=type(e).__name__ + ": " + str(e)) local_checksum = checksum_s(resultant) - remote_checksum = self.get_checksum(tmp, dest, task_vars, not directory_prepended, source=source) + remote_checksum = self.get_checksum(dest, task_vars, not directory_prepended, source=source) if isinstance(remote_checksum, dict): # Error from remote_checksum is a dict. Valid return is a str return remote_checksum @@ -141,14 +141,14 @@ class ActionModule(ActionBase): # if showing diffs, we need to get the remote value if self._play_context.diff: - diff = self._get_diff_data(tmp, dest, resultant, task_vars, source_file=False) + diff = self._get_diff_data(dest, resultant, task_vars, source_file=False) if not self._play_context.check_mode: # do actual work thorugh copy xfered = self._transfer_data(self._connection._shell.join_path(tmp, 'source'), resultant) # fix file permissions when the copy is done as a different user if self._play_context.become and self._play_context.become_user != 'root': - self._remote_chmod('a+r', xfered, tmp) + self._remote_chmod('a+r', xfered) # run the copy module new_module_args.update( diff --git a/lib/ansible/plugins/action/unarchive.py b/lib/ansible/plugins/action/unarchive.py index 8151efe428..06c7c102dd 100644 --- a/lib/ansible/plugins/action/unarchive.py +++ b/lib/ansible/plugins/action/unarchive.py @@ -53,7 +53,7 @@ class ActionModule(ActionBase): if stat and stat.get('exists', False): return dict(skipped=True, msg=("skipped, since %s exists" % creates)) - dest = self._remote_expand_user(dest, tmp) # CCTODO: Fix path for Windows hosts. + dest = self._remote_expand_user(dest) # CCTODO: Fix path for Windows hosts. source = os.path.expanduser(source) if copy: @@ -66,7 +66,7 @@ class ActionModule(ActionBase): else: source = self._loader.path_dwim_relative(self._loader.get_basedir(), 'files', source) - remote_checksum = self._remote_checksum(tmp, dest, all_vars=task_vars) + remote_checksum = self._remote_checksum(dest, all_vars=task_vars) if remote_checksum != '3': return dict(failed=True, msg="dest '%s' must be an existing dir" % dest) elif remote_checksum == '4': @@ -83,7 +83,7 @@ class ActionModule(ActionBase): if copy: if self._play_context.become and self._play_context.become_user != 'root': if not self._play_context.check_mode: - self._remote_chmod(tmp, 'a+r', tmp_src) + self._remote_chmod('a+r', tmp_src) # Build temporary module_args. new_module_args = self._task.args.copy() diff --git a/lib/ansible/plugins/connection/__init__.py b/lib/ansible/plugins/connection/__init__.py index 9bd3375730..e37543fd69 100644 --- a/lib/ansible/plugins/connection/__init__.py +++ b/lib/ansible/plugins/connection/__init__.py @@ -122,8 +122,17 @@ class ConnectionBase(with_metaclass(ABCMeta, object)): @ensure_connect @abstractmethod - def exec_command(self, cmd, tmp_path, in_data=None, executable=None, sudoable=True): - """Run a command on the remote host + def exec_command(self, cmd, in_data=None, sudoable=True): + """Run a command on the remote host. + + :arg cmd: byte string containing the command + :kwarg in_data: If set, this data is passed to the command's stdin. + This is used to implement pipelining. Currently not all + connection plugins implement pipelining. + :kwarg sudoable: Tell the connection plugin if we're executing + a command via a privilege escalation mechanism. This may affect + how the connection plugin returns data. Note that not all + connections can handle privilege escalation. :returns: a tuple of (return code, stdout, stderr) The return code is an int while stdout and stderr are both byte strings. diff --git a/lib/ansible/plugins/connection/accelerate.py b/lib/ansible/plugins/connection/accelerate.py index 67c070dc71..dfff616703 100644 --- a/lib/ansible/plugins/connection/accelerate.py +++ b/lib/ansible/plugins/connection/accelerate.py @@ -236,7 +236,7 @@ class Connection(ConnectionBase): else: return response.get('rc') == 0 - def exec_command(self, cmd, tmp_path, become_user=None, sudoable=False, executable='/bin/sh', in_data=None): + def exec_command(self, cmd, become_user=None, sudoable=False, executable='/bin/sh', in_data=None): ''' run a command on the remote host ''' if sudoable and self.runner.become and self.runner.become_method not in self.become_methods_supported: @@ -256,7 +256,6 @@ class Connection(ConnectionBase): data = dict( mode='command', cmd=cmd, - tmp_path=tmp_path, executable=executable, ) data = utils.jsonify(data) diff --git a/lib/ansible/plugins/connection/chroot.py b/lib/ansible/plugins/connection/chroot.py index 82b3bbd383..c7d6b6b1b8 100644 --- a/lib/ansible/plugins/connection/chroot.py +++ b/lib/ansible/plugins/connection/chroot.py @@ -83,7 +83,7 @@ class Connection(ConnectionBase): local_cmd += cmd return local_cmd - def _buffered_exec_command(self, cmd, tmp_path, become_user=None, sudoable=False, executable='/bin/sh', in_data=None, stdin=subprocess.PIPE): + def _buffered_exec_command(self, cmd, become_user=None, sudoable=False, executable='/bin/sh', in_data=None, stdin=subprocess.PIPE): ''' run a command on the chroot. This is only needed for implementing put_file() get_file() so that we don't have to read the whole file into memory. @@ -110,10 +110,10 @@ class Connection(ConnectionBase): return p - def exec_command(self, cmd, tmp_path, become_user=None, sudoable=False, executable='/bin/sh', in_data=None): + def exec_command(self, cmd, become_user=None, sudoable=False, executable='/bin/sh', in_data=None): ''' run a command on the chroot ''' - p = self._buffered_exec_command(cmd, tmp_path, become_user, sudoable, executable, in_data) + p = self._buffered_exec_command(cmd, become_user, sudoable, executable, in_data) stdout, stderr = p.communicate() return (p.returncode, stdout, stderr) @@ -126,7 +126,7 @@ class Connection(ConnectionBase): try: with open(in_path, 'rb') as in_file: try: - p = self._buffered_exec_command('dd of=%s bs=%s' % (out_path, self.BUFSIZE), None, stdin=in_file) + p = self._buffered_exec_command('dd of=%s bs=%s' % (out_path, self.BUFSIZE), stdin=in_file) except OSError: raise AnsibleError("chroot connection requires dd command in the chroot") try: @@ -145,7 +145,7 @@ class Connection(ConnectionBase): self._display.vvv("FETCH %s TO %s" % (in_path, out_path), host=self.chroot) try: - p = self._buffered_exec_command('dd if=%s bs=%s' % (in_path, self.BUFSIZE), None) + p = self._buffered_exec_command('dd if=%s bs=%s' % (in_path, self.BUFSIZE)) except OSError: raise AnsibleError("chroot connection requires dd command in the chroot") diff --git a/lib/ansible/plugins/connection/docker.py b/lib/ansible/plugins/connection/docker.py index 9fa9da8c6d..a46b6b2157 100644 --- a/lib/ansible/plugins/connection/docker.py +++ b/lib/ansible/plugins/connection/docker.py @@ -88,9 +88,9 @@ class Connection(ConnectionBase): return self - def exec_command(self, cmd, tmp_path, in_data=None, sudoable=False): + def exec_command(self, cmd, in_data=None, sudoable=False): """ Run a command on the local host """ - super(Connection, self).exec_command(cmd, tmp_path, in_data=in_data, sudoable=sudoable) + super(Connection, self).exec_command(cmd, in_data=in_data, sudoable=sudoable) # Don't currently support su if in_data: diff --git a/lib/ansible/plugins/connection/funcd.py b/lib/ansible/plugins/connection/funcd.py index 4d73fc4717..4c9e09be65 100644 --- a/lib/ansible/plugins/connection/funcd.py +++ b/lib/ansible/plugins/connection/funcd.py @@ -55,7 +55,7 @@ class Connection(object): self.client = fc.Client(self.host) return self - def exec_command(self, cmd, tmp_path, become_user=None, sudoable=False, + def exec_command(self, cmd, become_user=None, sudoable=False, executable='/bin/sh', in_data=None): ''' run a command on the remote minion ''' diff --git a/lib/ansible/plugins/connection/jail.py b/lib/ansible/plugins/connection/jail.py index 71430e72bc..102760065b 100644 --- a/lib/ansible/plugins/connection/jail.py +++ b/lib/ansible/plugins/connection/jail.py @@ -101,7 +101,7 @@ class Connection(object): local_cmd += cmd return local_cmd - def _buffered_exec_command(self, cmd, tmp_path, become_user=None, sudoable=False, executable='/bin/sh', in_data=None, stdin=subprocess.PIPE): + def _buffered_exec_command(self, cmd, become_user=None, sudoable=False, executable='/bin/sh', in_data=None, stdin=subprocess.PIPE): ''' run a command on the jail. This is only needed for implementing put_file() get_file() so that we don't have to read the whole file into memory. @@ -127,10 +127,10 @@ class Connection(object): return p - def exec_command(self, cmd, tmp_path, become_user=None, sudoable=False, executable='/bin/sh', in_data=None): + def exec_command(self, cmd, become_user=None, sudoable=False, executable='/bin/sh', in_data=None): ''' run a command on the jail ''' - p = self._buffered_exec_command(cmd, tmp_path, become_user, sudoable, executable, in_data) + p = self._buffered_exec_command(cmd, become_user, sudoable, executable, in_data) stdout, stderr = p.communicate() return (p.returncode, stdout, stderr) diff --git a/lib/ansible/plugins/connection/libvirt_lxc.py b/lib/ansible/plugins/connection/libvirt_lxc.py index 162c94f3d0..527bfd6c6f 100644 --- a/lib/ansible/plugins/connection/libvirt_lxc.py +++ b/lib/ansible/plugins/connection/libvirt_lxc.py @@ -69,7 +69,7 @@ class Connection(object): local_cmd = '%s -q -c lxc:/// lxc-enter-namespace %s -- %s' % (self.cmd, self.lxc, cmd) return local_cmd - def exec_command(self, cmd, tmp_path, become_user, sudoable=False, executable='/bin/sh', in_data=None): + def exec_command(self, cmd, become_user, sudoable=False, executable='/bin/sh', in_data=None): ''' run a command on the chroot ''' if sudoable and self.runner.become and self.runner.become_method not in self.become_methods_supported: diff --git a/lib/ansible/plugins/connection/local.py b/lib/ansible/plugins/connection/local.py index 060b383a35..1838f3e554 100644 --- a/lib/ansible/plugins/connection/local.py +++ b/lib/ansible/plugins/connection/local.py @@ -46,10 +46,10 @@ class Connection(ConnectionBase): self._connected = True return self - def exec_command(self, cmd, tmp_path, in_data=None, sudoable=True): + def exec_command(self, cmd, in_data=None, sudoable=True): ''' run a command on the local host ''' - super(Connection, self).exec_command(cmd, tmp_path, in_data=in_data, sudoable=sudoable) + super(Connection, self).exec_command(cmd, in_data=in_data, sudoable=sudoable) self._display.debug("in local.exec_command()") diff --git a/lib/ansible/plugins/connection/paramiko_ssh.py b/lib/ansible/plugins/connection/paramiko_ssh.py index dc98256ce0..2199bcf647 100644 --- a/lib/ansible/plugins/connection/paramiko_ssh.py +++ b/lib/ansible/plugins/connection/paramiko_ssh.py @@ -189,10 +189,10 @@ class Connection(ConnectionBase): return ssh - def exec_command(self, cmd, tmp_path, in_data=None, sudoable=True): + def exec_command(self, cmd, in_data=None, sudoable=True): ''' run a command on the remote host ''' - super(Connection, self).exec_command(cmd, tmp_path, in_data=in_data, sudoable=sudoable) + super(Connection, self).exec_command(cmd, in_data=in_data, sudoable=sudoable) if in_data: raise AnsibleError("Internal Error: this module does not support optimized module pipelining") diff --git a/lib/ansible/plugins/connection/ssh.py b/lib/ansible/plugins/connection/ssh.py index 801df55a01..651dc4d351 100644 --- a/lib/ansible/plugins/connection/ssh.py +++ b/lib/ansible/plugins/connection/ssh.py @@ -285,10 +285,10 @@ class Connection(ConnectionBase): return return_tuple - def _exec_command(self, cmd, tmp_path, in_data=None, sudoable=True): + def _exec_command(self, cmd, in_data=None, sudoable=True): ''' run a command on the remote host ''' - super(Connection, self).exec_command(cmd, tmp_path, in_data=in_data, sudoable=sudoable) + super(Connection, self).exec_command(cmd, in_data=in_data, sudoable=sudoable) self._display.vvv("ESTABLISH SSH CONNECTION FOR USER: {0}".format(self._play_context.remote_user), host=self._play_context.remote_addr) diff --git a/lib/ansible/plugins/connection/winrm.py b/lib/ansible/plugins/connection/winrm.py index 1242cfeaf5..89ce9296b0 100644 --- a/lib/ansible/plugins/connection/winrm.py +++ b/lib/ansible/plugins/connection/winrm.py @@ -172,8 +172,8 @@ class Connection(ConnectionBase): self.protocol = self._winrm_connect() return self - def exec_command(self, cmd, tmp_path, in_data=None, sudoable=True): - super(Connection, self).exec_command(cmd, tmp_path, in_data=in_data, sudoable=sudoable) + def exec_command(self, cmd, in_data=None, sudoable=True): + super(Connection, self).exec_command(cmd, in_data=in_data, sudoable=sudoable) cmd_parts = shlex.split(to_bytes(cmd), posix=False) cmd_parts = map(to_unicode, cmd_parts) script = None diff --git a/lib/ansible/plugins/connection/zone.py b/lib/ansible/plugins/connection/zone.py index 120e6969b3..96996084f2 100644 --- a/lib/ansible/plugins/connection/zone.py +++ b/lib/ansible/plugins/connection/zone.py @@ -110,7 +110,7 @@ class Connection(object): local_cmd += cmd return local_cmd - def _buffered_exec_command(self, cmd, tmp_path, become_user=None, sudoable=False, executable=None, in_data=None, stdin=subprocess.PIPE): + def _buffered_exec_command(self, cmd, become_user=None, sudoable=False, executable=None, in_data=None, stdin=subprocess.PIPE): ''' run a command on the zone. This is only needed for implementing put_file() get_file() so that we don't have to read the whole file into memory. @@ -136,14 +136,14 @@ class Connection(object): return p - def exec_command(self, cmd, tmp_path, become_user=None, sudoable=False, executable=None, in_data=None): + def exec_command(self, cmd, become_user=None, sudoable=False, executable=None, in_data=None): ''' run a command on the zone ''' ### TODO: Why all the precautions not to specify /bin/sh? (vs jail.py) if executable == '/bin/sh': executable = None - p = self._buffered_exec_command(cmd, tmp_path, become_user, sudoable, executable, in_data) + p = self._buffered_exec_command(cmd, become_user, sudoable, executable, in_data) stdout, stderr = p.communicate() return (p.returncode, stdout, stderr) diff --git a/test/units/plugins/action/test_action.py b/test/units/plugins/action/test_action.py index 68f787641f..acf297a8a8 100644 --- a/test/units/plugins/action/test_action.py +++ b/test/units/plugins/action/test_action.py @@ -38,11 +38,11 @@ class TestActionBase(unittest.TestCase): play_context.become_user = play_context.remote_user = 'root' play_context.make_become_cmd = Mock(return_value='CMD') - action_base._low_level_execute_command('ECHO', '/tmp', sudoable=True) + action_base._low_level_execute_command('ECHO', sudoable=True) play_context.make_become_cmd.assert_not_called() play_context.remote_user = 'apo' - action_base._low_level_execute_command('ECHO', '/tmp', sudoable=True) + action_base._low_level_execute_command('ECHO', sudoable=True) play_context.make_become_cmd.assert_called_once_with('ECHO', executable=None) play_context.make_become_cmd.reset_mock() @@ -51,7 +51,7 @@ class TestActionBase(unittest.TestCase): C.BECOME_ALLOW_SAME_USER = True try: play_context.remote_user = 'root' - action_base._low_level_execute_command('ECHO SAME', '/tmp', sudoable=True) + action_base._low_level_execute_command('ECHO SAME', sudoable=True) play_context.make_become_cmd.assert_called_once_with('ECHO SAME', executable=None) finally: C.BECOME_ALLOW_SAME_USER = become_allow_same_user