From f2ab920822459a4b723273e43180f986cf13b3eb Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Thu, 14 Apr 2022 13:07:27 -0400 Subject: [PATCH] Better info sourcing (#77511) Task is authoritative also includes latest per loop info and fix tests --- changelogs/fragments/better_info_sources.yml | 2 ++ lib/ansible/plugins/action/__init__.py | 28 +++++++++++++++----- test/units/plugins/action/test_action.py | 10 +++++++ 3 files changed, 33 insertions(+), 7 deletions(-) create mode 100644 changelogs/fragments/better_info_sources.yml diff --git a/changelogs/fragments/better_info_sources.yml b/changelogs/fragments/better_info_sources.yml new file mode 100644 index 00000000000..3b0cece2b18 --- /dev/null +++ b/changelogs/fragments/better_info_sources.yml @@ -0,0 +1,2 @@ +bugfixes: + - action plugins now pass cannonical info to modules instead of 'temporary' info from play_context diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 88ce93390b8..7db6137805e 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -103,9 +103,9 @@ class ActionBase(ABC): if self._task.async_val and not self._supports_async: raise AnsibleActionFail('async is not supported for this task.') - elif self._play_context.check_mode and not self._supports_check_mode: + elif self._task.check_mode and not self._supports_check_mode: raise AnsibleActionSkip('check mode is not supported for this task.') - elif self._task.async_val and self._play_context.check_mode: + elif self._task.async_val and self._task.check_mode: raise AnsibleActionFail('check mode and async cannot be used on same task.') # Error if invalid argument is passed @@ -395,6 +395,20 @@ class ActionBase(ABC): return self.get_shell_option('admin_users', ['root']) + def _get_remote_addr(self, tvars): + ''' consistently get the 'remote_address' for the action plugin ''' + remote_addr = tvars.get('delegated_vars', {}).get('ansible_host', tvars.get('ansible_host', tvars.get('inventory_hostname', None))) + for variation in ('remote_addr', 'host'): + try: + remote_addr = self._connection.get_option(variation) + except KeyError: + continue + break + else: + # plugin does not have, fallback to play_context + remote_addr = self._play_context.remote_addr + return remote_addr + def _get_remote_user(self): ''' consistently get the 'remote_user' for the action plugin ''' # TODO: use 'current user running ansible' as fallback when moving away from play_context @@ -929,7 +943,7 @@ class ActionBase(ABC): expanded = initial_fragment if '..' in os.path.dirname(expanded).split('/'): - raise AnsibleError("'%s' returned an invalid relative home directory path containing '..'" % self._play_context.remote_addr) + raise AnsibleError("'%s' returned an invalid relative home directory path containing '..'" % self._get_remote_addr({})) return expanded @@ -944,7 +958,7 @@ class ActionBase(ABC): def _update_module_args(self, module_name, module_args, task_vars): # set check mode in the module arguments, if required - if self._play_context.check_mode: + if self._task.check_mode: if not self._supports_check_mode: raise AnsibleError("check mode is not supported for this operation") module_args['_ansible_check_mode'] = True @@ -953,13 +967,13 @@ class ActionBase(ABC): # set no log in the module arguments, if required no_target_syslog = C.config.get_config_value('DEFAULT_NO_TARGET_SYSLOG', variables=task_vars) - module_args['_ansible_no_log'] = self._play_context.no_log or no_target_syslog + module_args['_ansible_no_log'] = self._task.no_log or no_target_syslog # set debug in the module arguments, if required module_args['_ansible_debug'] = C.DEFAULT_DEBUG # let module know we are in diff mode - module_args['_ansible_diff'] = self._play_context.diff + module_args['_ansible_diff'] = self._task.diff # let module know our verbosity module_args['_ansible_verbosity'] = display.verbosity @@ -1395,7 +1409,7 @@ class ActionBase(ABC): diff['after_header'] = u'dynamically generated' diff['after'] = source - if self._play_context.no_log: + if self._task.no_log: if 'before' in diff: diff["before"] = u"" if 'after' in diff: diff --git a/test/units/plugins/action/test_action.py b/test/units/plugins/action/test_action.py index 0a50bdd4651..d69f28df8ad 100644 --- a/test/units/plugins/action/test_action.py +++ b/test/units/plugins/action/test_action.py @@ -283,6 +283,9 @@ class TestActionBase(unittest.TestCase): play_context.become = True play_context.become_user = 'foo' + mock_task.become = True + mock_task.become_user = True + # our test class action_base = DerivedActionBase( task=mock_task, @@ -654,6 +657,9 @@ class TestActionBase(unittest.TestCase): mock_task = MagicMock() mock_task.action = 'copy' mock_task.args = dict(a=1, b=2, c=3) + mock_task.diff = False + mock_task.check_mode = False + mock_task.no_log = False # create a mock connection, so we don't actually try and connect to things def build_module_command(env_string, shebang, cmd, arg_path=None): @@ -728,6 +734,8 @@ class TestActionBase(unittest.TestCase): play_context.become = True play_context.become_user = 'foo' + mock_task.become = True + mock_task.become_user = True self.assertEqual(action_base._execute_module(), dict(_ansible_parsed=True, rc=0, stdout="ok", stdout_lines=['ok'])) # test an invalid shebang return @@ -739,6 +747,7 @@ class TestActionBase(unittest.TestCase): # test with check mode enabled, once with support for check # mode and once with support disabled to raise an error play_context.check_mode = True + mock_task.check_mode = True action_base._configure_module.return_value = ('new', '#!/usr/bin/python', 'this is the module data', 'path') self.assertEqual(action_base._execute_module(), dict(_ansible_parsed=True, rc=0, stdout="ok", stdout_lines=['ok'])) action_base._supports_check_mode = False @@ -777,6 +786,7 @@ class TestActionBase(unittest.TestCase): def test__remote_expand_user_relative_pathing(self): action_base = _action_base() action_base._play_context.remote_addr = 'bar' + action_base._connection.get_option.return_value = 'bar' action_base._low_level_execute_command = MagicMock(return_value={'stdout': b'../home/user'}) action_base._connection._shell.join_path.return_value = '../home/user/foo' with self.assertRaises(AnsibleError) as cm: