From 94a0d2afb4b7b74bbefd5ab57d459f0e74b060a2 Mon Sep 17 00:00:00 2001 From: Matt Clay Date: Tue, 6 Sep 2016 16:49:59 -0700 Subject: [PATCH] Add partially backwards compatible version of _fixup_perms. (#17427) Also added a deprecation notice for _fixup_perms. Resolves issue #17352 (assumes custom actions use recursive=False). --- CHANGELOG.md | 14 +++++++++++++ lib/ansible/plugins/action/__init__.py | 26 ++++++++++++++++++++++-- lib/ansible/plugins/action/assemble.py | 2 +- lib/ansible/plugins/action/async.py | 2 +- lib/ansible/plugins/action/copy.py | 2 +- lib/ansible/plugins/action/patch.py | 2 +- lib/ansible/plugins/action/script.py | 2 +- lib/ansible/plugins/action/template.py | 2 +- lib/ansible/plugins/action/unarchive.py | 2 +- test/units/plugins/action/test_action.py | 2 +- 10 files changed, 46 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4774b209d71..8705a84c43a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -86,6 +86,20 @@ Ansible Changes By Release * loop_control now has a label option to allow fine grained control what gets displayed per item * loop_control now has a pause option to allow pausing for N seconds between loop iterations of a task. +## 2.1.2 "The Song Remains the Same" + +###Deprecations: + +* Deprecated the use of `_fixup_perms`. Use `_fixup_perms2` instead. + This change only impacts custom action plugins using `_fixup_perms`. + +###Incompatible Changes: + +* Use of `_fixup_perms` with `recursive=True` (the default) is no longer supported. + Custom action plugins using `_fixup_perms` will require changes unless they already use `recursive=False`. + Use `_fixup_perms2` if support for previous releases is not required. + Otherwise use `_fixup_perms` with `recursive=False`. + ## 2.1 "The Song Remains the Same" ###Major Changes: diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index aecbecad3d1..e6ed4aa80f7 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -293,7 +293,29 @@ class ActionBase(with_metaclass(ABCMeta, object)): return remote_path - def _fixup_perms(self, remote_paths, remote_user, execute=True): + def _fixup_perms(self, remote_path, remote_user, execute=True, recursive=True): + """ + We need the files we upload to be readable (and sometimes executable) + by the user being sudo'd to but we want to limit other people's access + (because the files could contain passwords or other private + information. + + Deprecated in favor of _fixup_perms2. Ansible code has been updated to + use _fixup_perms2. This code is maintained to provide partial support + for custom actions (non-recursive mode only). + + """ + + display.deprecated('_fixup_perms is deprecated. Use _fixup_perms2 instead.', version='2.4', removed=False) + + if recursive: + raise AnsibleError('_fixup_perms with recursive=True (the default) is no longer supported. ' + + 'Use _fixup_perms2 if support for previous releases is not required. ' + 'Otherwise use fixup_perms with recursive=False.') + + return self._fixup_perms2([remote_path], remote_user, execute) + + def _fixup_perms2(self, remote_paths, remote_user, execute=True): """ We need the files we upload to be readable (and sometimes executable) by the user being sudo'd to but we want to limit other people's access @@ -618,7 +640,7 @@ class ActionBase(with_metaclass(ABCMeta, object)): # Fix permissions of the tmp path and tmp files. This should be # called after all files have been transferred. if remote_files: - self._fixup_perms(remote_files, remote_user) + self._fixup_perms2(remote_files, remote_user) cmd = "" in_data = None diff --git a/lib/ansible/plugins/action/assemble.py b/lib/ansible/plugins/action/assemble.py index d40067aac79..4d1f7a6cc09 100644 --- a/lib/ansible/plugins/action/assemble.py +++ b/lib/ansible/plugins/action/assemble.py @@ -159,7 +159,7 @@ class ActionModule(ActionBase): xfered = self._transfer_file(path, remote_path) # fix file permissions when the copy is done as a different user - self._fixup_perms((tmp, remote_path), remote_user) + self._fixup_perms2((tmp, remote_path), remote_user) new_module_args.update( dict( src=xfered,)) diff --git a/lib/ansible/plugins/action/async.py b/lib/ansible/plugins/action/async.py index abe0abe5e27..5d2b821ca18 100644 --- a/lib/ansible/plugins/action/async.py +++ b/lib/ansible/plugins/action/async.py @@ -81,7 +81,7 @@ class ActionModule(ActionBase): if argsfile: remote_paths += argsfile, - self._fixup_perms(remote_paths, remote_user, execute=True) + self._fixup_perms2(remote_paths, remote_user, execute=True) async_limit = self._task.async async_jid = str(random.randint(0, 999999999999)) diff --git a/lib/ansible/plugins/action/copy.py b/lib/ansible/plugins/action/copy.py index 68b60d35da0..140c714c27d 100644 --- a/lib/ansible/plugins/action/copy.py +++ b/lib/ansible/plugins/action/copy.py @@ -226,7 +226,7 @@ class ActionModule(ActionBase): # fix file permissions when the copy is done as a different user if remote_path: - self._fixup_perms((tmp, remote_path), remote_user) + self._fixup_perms2((tmp, remote_path), remote_user) if raw: # Continue to next iteration if raw is defined. diff --git a/lib/ansible/plugins/action/patch.py b/lib/ansible/plugins/action/patch.py index bd5eb92f301..db1a2eaba7e 100644 --- a/lib/ansible/plugins/action/patch.py +++ b/lib/ansible/plugins/action/patch.py @@ -63,7 +63,7 @@ class ActionModule(ActionBase): tmp_src = self._connection._shell.join_path(tmp, os.path.basename(src)) self._transfer_file(src, tmp_src) - self._fixup_perms((tmp, tmp_src), remote_user) + self._fixup_perms2((tmp, tmp_src), remote_user) new_module_args = self._task.args.copy() new_module_args.update( diff --git a/lib/ansible/plugins/action/script.py b/lib/ansible/plugins/action/script.py index 7b280ea5427..286142fafc3 100644 --- a/lib/ansible/plugins/action/script.py +++ b/lib/ansible/plugins/action/script.py @@ -81,7 +81,7 @@ class ActionModule(ActionBase): self._transfer_file(source, tmp_src) # set file permissions, more permissive when the copy is done as a different user - self._fixup_perms((tmp, tmp_src), remote_user, execute=True) + self._fixup_perms2((tmp, tmp_src), remote_user, execute=True) # add preparation steps to one ssh roundtrip executing the script env_string = self._compute_environment_string() diff --git a/lib/ansible/plugins/action/template.py b/lib/ansible/plugins/action/template.py index 919972d8061..895ac835c89 100644 --- a/lib/ansible/plugins/action/template.py +++ b/lib/ansible/plugins/action/template.py @@ -167,7 +167,7 @@ class ActionModule(ActionBase): xfered = self._transfer_data(self._connection._shell.join_path(tmp, 'source'), resultant) # fix file permissions when the copy is done as a different user - self._fixup_perms((tmp, xfered), remote_user) + self._fixup_perms2((tmp, xfered), remote_user) # 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 a1a5fb82a1f..5884cb4a95c 100644 --- a/lib/ansible/plugins/action/unarchive.py +++ b/lib/ansible/plugins/action/unarchive.py @@ -108,7 +108,7 @@ class ActionModule(ActionBase): if not remote_src: # fix file permissions when the copy is done as a different user - self._fixup_perms((tmp, tmp_src), remote_user) + self._fixup_perms2((tmp, tmp_src), remote_user) # Build temporary module_args. new_module_args = self._task.args.copy() new_module_args.update( diff --git a/test/units/plugins/action/test_action.py b/test/units/plugins/action/test_action.py index e2481daa1c4..9a4cb74f3e7 100644 --- a/test/units/plugins/action/test_action.py +++ b/test/units/plugins/action/test_action.py @@ -489,7 +489,7 @@ class TestActionBase(unittest.TestCase): action_base._transfer_data = MagicMock() action_base._compute_environment_string = MagicMock() action_base._low_level_execute_command = MagicMock() - action_base._fixup_perms = MagicMock() + action_base._fixup_perms2 = MagicMock() action_base._configure_module.return_value = ('new', '#!/usr/bin/python', 'this is the module data', 'path') action_base._late_needs_tmp_path.return_value = False