From a5d79a39d51a256937d6e5742befa169629b7caf Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Wed, 20 Apr 2016 13:39:12 -0400 Subject: [PATCH] Ensure action plugins remove tmp dirs created (#15501) fixes #14917 --- lib/ansible/plugins/action/__init__.py | 3 ++- lib/ansible/plugins/action/assemble.py | 9 +++------ lib/ansible/plugins/action/async.py | 4 ++-- lib/ansible/plugins/action/copy.py | 12 +++++++++--- lib/ansible/plugins/action/patch.py | 2 ++ lib/ansible/plugins/action/script.py | 6 ++++-- lib/ansible/plugins/action/template.py | 6 ++---- lib/ansible/plugins/action/unarchive.py | 5 +++++ 8 files changed, 29 insertions(+), 18 deletions(-) diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 067f608106..3f9ef230fd 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -64,6 +64,7 @@ class ActionBase(with_metaclass(ABCMeta, object)): # Backwards compat: self._display isn't really needed, just import the global display and use that. self._display = display + self._cleanup_remote_tmp = False self._supports_check_mode = True @abstractmethod @@ -254,7 +255,7 @@ class ActionBase(with_metaclass(ABCMeta, object)): def _remove_tmp_path(self, tmp_path): '''Remove a temporary path we created. ''' - if tmp_path and "-tmp-" in tmp_path: + if tmp_path and self._cleanup_remote_tmp and not C.DEFAULT_KEEP_REMOTE_FILES and "-tmp-" in tmp_path: cmd = self._connection._shell.remove(tmp_path, recurse=True) # If we have gotten here we have a working ssh configuration. # If ssh breaks we could leave tmp directories out on the remote system. diff --git a/lib/ansible/plugins/action/assemble.py b/lib/ansible/plugins/action/assemble.py index 9c7e709122..5cdfc7cbfb 100644 --- a/lib/ansible/plugins/action/assemble.py +++ b/lib/ansible/plugins/action/assemble.py @@ -97,16 +97,14 @@ class ActionModule(ActionBase): result['msg'] = "src and dest are required" return result - cleanup_remote_tmp = False remote_user = task_vars.get('ansible_ssh_user') or self._play_context.remote_user if not tmp: tmp = self._make_tmp_path(remote_user) - cleanup_remote_tmp = True + self._cleanup_remote_tmp = True if boolean(remote_src): result.update(self._execute_module(tmp=tmp, task_vars=task_vars, delete_remote_tmp=False)) - if cleanup_remote_tmp: - self._remove_tmp_path(tmp) + self._remove_tmp_path(tmp) return result elif self._task._role is not None: src = self._loader.path_dwim_relative(self._task._role._role_path, 'files', src) @@ -166,7 +164,6 @@ class ActionModule(ActionBase): else: result.update(self._execute_module(module_name='file', module_args=new_module_args, task_vars=task_vars, tmp=tmp, delete_remote_tmp=False)) - if tmp and cleanup_remote_tmp: - self._remove_tmp_path(tmp) + self._remove_tmp_path(tmp) return result diff --git a/lib/ansible/plugins/action/async.py b/lib/ansible/plugins/action/async.py index 2002d248bd..4f7aa634ce 100644 --- a/lib/ansible/plugins/action/async.py +++ b/lib/ansible/plugins/action/async.py @@ -41,6 +41,7 @@ class ActionModule(ActionBase): remote_user = task_vars.get('ansible_ssh_user') or self._play_context.remote_user if not tmp: tmp = self._make_tmp_path(remote_user) + self._cleanup_remote_tmp=True module_name = self._task.action async_module_path = self._connection._shell.join_path(tmp, 'async_wrapper') @@ -91,8 +92,7 @@ class ActionModule(ActionBase): result.update(self._low_level_execute_command(cmd=async_cmd)) # clean up after - if tmp and "tmp" in tmp and not C.DEFAULT_KEEP_REMOTE_FILES: - self._remove_tmp_path(tmp) + self._remove_tmp_path(tmp) result['changed'] = True diff --git a/lib/ansible/plugins/action/copy.py b/lib/ansible/plugins/action/copy.py index f798f8c4b8..1c40e0a4e0 100644 --- a/lib/ansible/plugins/action/copy.py +++ b/lib/ansible/plugins/action/copy.py @@ -145,6 +145,7 @@ class ActionModule(ActionBase): if not delete_remote_tmp: if tmp is None or "-tmp-" not in tmp: tmp = self._make_tmp_path(remote_user) + self._cleanup_remote_tmp = True # expand any user home dir specifier dest = self._remote_expand_user(dest) @@ -161,6 +162,7 @@ class ActionModule(ActionBase): if local_checksum is None: result['failed'] = True result['msg'] = "could not find src=%s" % source_full + self._remove_tmp_path(tmp) return result # This is kind of optimization - if user told us destination is @@ -179,6 +181,7 @@ class ActionModule(ActionBase): if content is not None: # If source was defined as content remove the temporary file and fail out. self._remove_tempfile_if_content_defined(content, content_tempfile) + self._remove_tmp_path(tmp) result['failed'] = True result['msg'] = "can not use content with a dir as dest" return result @@ -200,6 +203,7 @@ class ActionModule(ActionBase): if delete_remote_tmp: if tmp is None or "-tmp-" not in tmp: tmp = self._make_tmp_path(remote_user) + self._cleanup_remote_tmp = True if self._play_context.diff and not raw: diffs.append(self._get_diff_data(dest_file, source_full, task_vars)) @@ -242,7 +246,7 @@ class ActionModule(ActionBase): ) ) - module_return = self._execute_module(module_name='copy', module_args=new_module_args, task_vars=task_vars, delete_remote_tmp=delete_remote_tmp) + module_return = self._execute_module(module_name='copy', module_args=new_module_args, task_vars=task_vars, tmp=tmp, delete_remote_tmp=delete_remote_tmp) module_executed = True else: @@ -267,13 +271,15 @@ class ActionModule(ActionBase): ) # Execute the file module. - module_return = self._execute_module(module_name='file', module_args=new_module_args, task_vars=task_vars, delete_remote_tmp=delete_remote_tmp) + module_return = self._execute_module(module_name='file', module_args=new_module_args, task_vars=task_vars, tmp=tmp, delete_remote_tmp=delete_remote_tmp) module_executed = True if not module_return.get('checksum'): module_return['checksum'] = local_checksum if module_return.get('failed'): result.update(module_return) + if not delete_remote_tmp: + self._remove_tmp_path(tmp) return result if module_return.get('changed'): changed = True @@ -284,7 +290,7 @@ class ActionModule(ActionBase): module_return['dest'] = module_return['path'] # Delete tmp path if we were recursive or if we did not execute a module. - if (not C.DEFAULT_KEEP_REMOTE_FILES and not delete_remote_tmp) or (not C.DEFAULT_KEEP_REMOTE_FILES and delete_remote_tmp and not module_executed): + if not delete_remote_tmp or (delete_remote_tmp and not module_executed): self._remove_tmp_path(tmp) if module_executed and len(source_files) == 1: diff --git a/lib/ansible/plugins/action/patch.py b/lib/ansible/plugins/action/patch.py index 4fbc69b66a..fa7a3f514a 100644 --- a/lib/ansible/plugins/action/patch.py +++ b/lib/ansible/plugins/action/patch.py @@ -54,6 +54,7 @@ class ActionModule(ActionBase): # create the remote tmp dir if needed, and put the source file there if tmp is None or "-tmp-" not in tmp: tmp = self._make_tmp_path(remote_user) + self._cleanup_remote_tmp = True tmp_src = self._connection._shell.join_path(tmp, os.path.basename(src)) self._transfer_file(src, tmp_src) @@ -68,4 +69,5 @@ class ActionModule(ActionBase): ) result.update(self._execute_module('patch', module_args=new_module_args, task_vars=task_vars)) + self._remove_tmp_path(tmp) return result diff --git a/lib/ansible/plugins/action/script.py b/lib/ansible/plugins/action/script.py index f7363cf32c..76699a2916 100644 --- a/lib/ansible/plugins/action/script.py +++ b/lib/ansible/plugins/action/script.py @@ -42,6 +42,7 @@ class ActionModule(ActionBase): remote_user = task_vars.get('ansible_ssh_user') or self._play_context.remote_user if not tmp: tmp = self._make_tmp_path(remote_user) + self._cleanup_remote_tmp = True creates = self._task.args.get('creates') if creates: @@ -49,6 +50,7 @@ class ActionModule(ActionBase): # and the filename already exists. This allows idempotence # of command executions. if self._remote_file_exists(creates): + self._remove_tmp_path(tmp) return dict(skipped=True, msg=("skipped, since %s exists" % creates)) removes = self._task.args.get('removes') @@ -57,6 +59,7 @@ class ActionModule(ActionBase): # and the filename does not exist. This allows idempotence # of command executions. if not self._remote_file_exists(removes): + self._remove_tmp_path(tmp) return dict(skipped=True, msg=("skipped, since %s does not exist" % removes)) # the script name is the first item in the raw params, so we split it @@ -87,8 +90,7 @@ class ActionModule(ActionBase): result.update(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: - self._remove_tmp_path(tmp) + self._remove_tmp_path(tmp) result['changed'] = True diff --git a/lib/ansible/plugins/action/template.py b/lib/ansible/plugins/action/template.py index 2410ffe820..9f033e2940 100644 --- a/lib/ansible/plugins/action/template.py +++ b/lib/ansible/plugins/action/template.py @@ -138,11 +138,10 @@ class ActionModule(ActionBase): result['msg'] = type(e).__name__ + ": " + str(e) return result - cleanup_remote_tmp = False remote_user = task_vars.get('ansible_ssh_user') or self._play_context.remote_user if not tmp: tmp = self._make_tmp_path(remote_user) - cleanup_remote_tmp = True + self._cleanup_remote_tmp = True local_checksum = checksum_s(resultant) remote_checksum = self.get_checksum(dest, task_vars, not directory_prepended, source=source, tmp=tmp) @@ -197,7 +196,6 @@ class ActionModule(ActionBase): ) result.update(self._execute_module(module_name='file', module_args=new_module_args, task_vars=task_vars, tmp=tmp, delete_remote_tmp=False)) - if tmp and cleanup_remote_tmp: - self._remove_tmp_path(tmp) + self._remove_tmp_path(tmp) return result diff --git a/lib/ansible/plugins/action/unarchive.py b/lib/ansible/plugins/action/unarchive.py index 6ace6f9975..590881165e 100644 --- a/lib/ansible/plugins/action/unarchive.py +++ b/lib/ansible/plugins/action/unarchive.py @@ -48,6 +48,7 @@ class ActionModule(ActionBase): remote_user = task_vars.get('ansible_ssh_user') or self._play_context.remote_user if not tmp: tmp = self._make_tmp_path(remote_user) + self._cleanup_remote_tmp = True if creates: # do not run the command if the line contains creates=filename @@ -58,6 +59,7 @@ class ActionModule(ActionBase): if stat and stat.get('exists', False): result['skipped'] = True result['msg'] = "skipped, since %s exists" % creates + self._remove_tmp_path(tmp) return result dest = self._remote_expand_user(dest) # CCTODO: Fix path for Windows hosts. @@ -73,10 +75,12 @@ class ActionModule(ActionBase): if remote_checksum == '4': result['failed'] = True result['msg'] = "python isn't present on the system. Unable to compute checksum" + self._remove_tmp_path(tmp) return result elif remote_checksum != '3': result['failed'] = True result['msg'] = "dest '%s' must be an existing dir" % dest + self._remove_tmp_path(tmp) return result if copy: @@ -109,4 +113,5 @@ class ActionModule(ActionBase): # execute the unarchive module now, with the updated args result.update(self._execute_module(module_args=new_module_args, task_vars=task_vars)) + self._remove_tmp_path(tmp) return result