From efed4e577cc33563276d803513e7b6267daf9fc1 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Fri, 10 Jun 2016 11:37:58 -0400 Subject: [PATCH] raw should not use default executable (#16085) also removed unused cruft in script (cherry picked from commit a529a6047806dc7d1e55e9e36201e5b2ed57bf27) --- lib/ansible/playbook/play_context.py | 23 ++--- lib/ansible/plugins/action/raw.py | 2 +- lib/ansible/plugins/action/script.py | 2 - test/units/modules/core/test_docker.py | 2 +- test/units/plugins/action/test_raw.py | 112 +++++++++++++++++++++++++ 5 files changed, 127 insertions(+), 14 deletions(-) create mode 100644 test/units/plugins/action/test_raw.py diff --git a/lib/ansible/playbook/play_context.py b/lib/ansible/playbook/play_context.py index ad68579a1bf..1f0a04f4cb5 100644 --- a/lib/ansible/playbook/play_context.py +++ b/lib/ansible/playbook/play_context.py @@ -29,11 +29,9 @@ import string from ansible.compat.six import iteritems, string_types from ansible import constants as C from ansible.errors import AnsibleError -from ansible.playbook.attribute import Attribute, FieldAttribute +from ansible.playbook.attribute import FieldAttribute from ansible.playbook.base import Base -from ansible.template import Templar from ansible.utils.boolean import boolean -from ansible.utils.unicode import to_unicode __all__ = ['PlayContext'] @@ -447,16 +445,21 @@ class PlayContext(Base): success_key = None self.prompt = None - if executable is None: - executable = self.executable - if self.become: + if executable is None: + executable = self.executable + becomecmd = None randbits = ''.join(random.choice(string.ascii_lowercase) for x in range(32)) success_key = 'BECOME-SUCCESS-%s' % randbits success_cmd = pipes.quote('echo %s; %s' % (success_key, cmd)) + if executable: + command = '%s -c %s' % (executable, success_cmd) + else: + command = success_cmd + # set executable to use for the privilege escalation method, with various overrides exe = self.become_exe or \ getattr(self, '%s_exe' % self.become_method, None) or \ @@ -485,9 +488,9 @@ class PlayContext(Base): # force quick error if password is required but not supplied, should prevent sudo hangs. if self.become_pass: prompt = '[sudo via ansible, key=%s] password: ' % randbits - becomecmd = '%s %s -p "%s" -u %s %s -c %s' % (exe, flags.replace('-n',''), prompt, self.become_user, executable, success_cmd) + becomecmd = '%s %s -p "%s" -u %s %s' % (exe, flags.replace('-n',''), prompt, self.become_user, command) else: - becomecmd = '%s %s -u %s %s -c %s' % (exe, flags, self.become_user, executable, success_cmd) + becomecmd = '%s %s -u %s %s' % (exe, flags, self.become_user, command) elif self.become_method == 'su': @@ -498,7 +501,7 @@ class PlayContext(Base): return bool(SU_PROMPT_LOCALIZATIONS_RE.match(data)) prompt = detect_su_prompt - becomecmd = '%s %s %s -c %s' % (exe, flags, self.become_user, pipes.quote('%s -c %s' % (executable, success_cmd))) + becomecmd = '%s %s %s -c %s' % (exe, flags, self.become_user, pipes.quote(command)) elif self.become_method == 'pbrun': @@ -534,7 +537,7 @@ class PlayContext(Base): exe = self.become_exe or 'dzdo' - becomecmd = '%s -u %s %s -c %s' % (exe, self.become_user, executable, success_cmd) + becomecmd = '%s -u %s %s' % (exe, self.become_user, command) else: raise AnsibleError("Privilege escalation method not found: %s" % self.become_method) diff --git a/lib/ansible/plugins/action/raw.py b/lib/ansible/plugins/action/raw.py index ae3d53a3660..5209b37a234 100644 --- a/lib/ansible/plugins/action/raw.py +++ b/lib/ansible/plugins/action/raw.py @@ -37,7 +37,7 @@ class ActionModule(ActionBase): result['skipped'] = True return result - executable = self._task.args.get('executable') + executable = self._task.args.get('executable', False) result.update(self._low_level_execute_command(self._task.args.get('_raw_params'), executable=executable)) return result diff --git a/lib/ansible/plugins/action/script.py b/lib/ansible/plugins/action/script.py index 76699a29164..358775c19e2 100644 --- a/lib/ansible/plugins/action/script.py +++ b/lib/ansible/plugins/action/script.py @@ -19,7 +19,6 @@ __metaclass__ = type import os -from ansible import constants as C from ansible.plugins.action import ActionBase @@ -79,7 +78,6 @@ class ActionModule(ActionBase): tmp_src = self._connection._shell.join_path(tmp, os.path.basename(source)) self._transfer_file(source, tmp_src) - sudoable = True # set file permissions, more permissive when the copy is done as a different user self._fixup_perms(tmp, remote_user, execute=True, recursive=True) diff --git a/test/units/modules/core/test_docker.py b/test/units/modules/core/test_docker.py index b8c8cf1e235..2fc31e2771a 100644 --- a/test/units/modules/core/test_docker.py +++ b/test/units/modules/core/test_docker.py @@ -2,7 +2,7 @@ import collections import os import unittest -from ansible.modules.core.cloud.docker.docker import get_split_image_tag +from ansible.modules.core.cloud.docker._docker import get_split_image_tag class DockerSplitImageTagTestCase(unittest.TestCase): diff --git a/test/units/plugins/action/test_raw.py b/test/units/plugins/action/test_raw.py new file mode 100644 index 00000000000..5564a934b29 --- /dev/null +++ b/test/units/plugins/action/test_raw.py @@ -0,0 +1,112 @@ +# (c) 2016, Saran Ahluwalia +# +# This file is part of Ansible +# +# Ansible is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Ansible is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Ansible. If not, see . + +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + + +from ansible.compat.tests import unittest +from ansible.compat.tests.mock import patch, MagicMock, Mock +from ansible.plugins.action.raw import ActionModule +from ansible.playbook.task import Task + +class TestCopyResultExclude(unittest.TestCase): + + def setUp(self): + pass + + def tearDown(self): + pass + + # The current behavior of the raw aciton in regards to executable is currently in question; + # the test_raw_executable_is_not_empty_string verifies the current behavior (whether it is desireed or not. + # Please refer to the following for context: + # Issue: https://github.com/ansible/ansible/issues/16054 + # PR: https://github.com/ansible/ansible/pull/16085 + + + def test_raw_executable_is_not_empty_string(self): + + play_context = Mock() + task = MagicMock(Task) + task.async = MagicMock() + connection = Mock() + + task.args = {'_raw_params': 'Args1'} + play_context.check_mode = False + + self.mock_am = ActionModule(task, connection, play_context, loader=None, templar=None, shared_loader_obj=None) + self.mock_am._low_level_execute_command = Mock(return_value = {}) + self.mock_am.display = Mock() + + self.mock_am.run() + self.mock_am._low_level_execute_command.assert_called_with('Args1', executable=False) + + def test_raw_check_mode_is_True(self): + + play_context = Mock() + task = MagicMock(Task) + task.async = MagicMock() + connection = Mock() + + task.args = {'_raw_params': 'Args1'} + play_context.check_mode = True + + self.mock_am = ActionModule(task, connection, play_context, loader=None, templar=None, shared_loader_obj=None) + self.mock_am._low_level_execute_command = Mock(return_value = {}) + self.mock_am.display = Mock() + + skipped_result = self.mock_am.run() + + self.assertEqual(skipped_result.get('skipped'), True) + + def test_raw_test_environment_is_None(self): + + play_context = Mock() + task = MagicMock(Task) + task.async = MagicMock() + connection = Mock() + + task.args = {'_raw_params': 'Args1'} + task.environment = None + play_context.check_mode = False + + self.mock_am = ActionModule(task, connection, play_context, loader=None, templar=None, shared_loader_obj=None) + self.mock_am._low_level_execute_command = Mock(return_value = {}) + self.mock_am.display = Mock() + + self.assertEqual(task.environment, None) + + def test_raw_task_vars_is_not_None(self): + + play_context = Mock() + task = MagicMock(Task) + task.async = MagicMock() + connection = Mock() + + task.args = {'_raw_params': 'Args1'} + task.environment = None + play_context.check_mode = False + + self.mock_am = ActionModule(task, connection, play_context, loader=None, templar=None, shared_loader_obj=None) + self.mock_am._low_level_execute_command = Mock(return_value = {}) + self.mock_am.display = Mock() + + self.mock_am.run(task_vars={'a': 'b'}) + self.assertEqual(task.environment, None) + +