From 76c7dfd968a2ace43507fe081e10f81db4607d0e Mon Sep 17 00:00:00 2001 From: Sloane Hertel <19572925+s-hertel@users.noreply.github.com> Date: Thu, 6 Jun 2024 16:58:04 -0400 Subject: [PATCH] Fix task.resolved_action callbacks (#82003) (#83329) * Fix task.resolved_action for callbacks when playbooks use action or local_action * Fix using module_defaults with 'action' and 'local_action' task FA and add a test case Fixes #81905 (cherry picked from commit f2435375a8084c0af60c21a58497789116241750) --- ...lback-fqcn-old-style-action-invocation.yml | 3 +++ lib/ansible/parsing/mod_args.py | 20 ++++++++++++++++--- lib/ansible/playbook/task.py | 1 + .../unqualified_and_collections_kw.yml | 6 ++++-- .../targets/module_defaults/tasks/main.yml | 11 ++++++++-- test/units/playbook/test_block.py | 4 ++++ test/units/playbook/test_helpers.py | 5 ++++- 7 files changed, 42 insertions(+), 8 deletions(-) create mode 100644 changelogs/fragments/correct-callback-fqcn-old-style-action-invocation.yml diff --git a/changelogs/fragments/correct-callback-fqcn-old-style-action-invocation.yml b/changelogs/fragments/correct-callback-fqcn-old-style-action-invocation.yml new file mode 100644 index 00000000000..675439604ea --- /dev/null +++ b/changelogs/fragments/correct-callback-fqcn-old-style-action-invocation.yml @@ -0,0 +1,3 @@ +bugfixes: + - Fix the task attribute ``resolved_action`` to show the FQCN instead of ``None`` when ``action`` or ``local_action`` is used in the playbook. + - Fix using ``module_defaults`` with ``local_action``/``action`` (https://github.com/ansible/ansible/issues/81905). diff --git a/lib/ansible/parsing/mod_args.py b/lib/ansible/parsing/mod_args.py index 56e7e98151e..fb982f54426 100644 --- a/lib/ansible/parsing/mod_args.py +++ b/lib/ansible/parsing/mod_args.py @@ -53,6 +53,17 @@ BUILTIN_TASKS = frozenset(add_internal_fqcns(( ))) +def _get_action_context(action_or_module, collection_list): + module_context = module_loader.find_plugin_with_context(action_or_module, collection_list=collection_list) + if module_context and module_context.resolved and module_context.action_plugin: + action_or_module = module_context.action_plugin + + context = action_loader.find_plugin_with_context(action_or_module, collection_list=collection_list) + if not context or not context.resolved: + context = module_context + return context + + class ModuleArgsParser: """ @@ -291,6 +302,11 @@ class ModuleArgsParser: delegate_to = 'localhost' action, args = self._normalize_parameters(thing, action=action, additional_args=additional_args) + if action is not None and not skip_action_validation: + context = _get_action_context(action, self._collection_list) + if context is not None and context.resolved: + self.resolved_action = context.resolved_fqcn + # module: is the more new-style invocation # filter out task attributes so we're only querying unrecognized keys as actions/modules @@ -306,9 +322,7 @@ class ModuleArgsParser: is_action_candidate = True else: try: - context = action_loader.find_plugin_with_context(item, collection_list=self._collection_list) - if not context.resolved: - context = module_loader.find_plugin_with_context(item, collection_list=self._collection_list) + context = _get_action_context(item, self._collection_list) except AnsibleError as e: if e.obj is None: e.obj = self._task_ds diff --git a/lib/ansible/playbook/task.py b/lib/ansible/playbook/task.py index 7f54639ad2f..8345869a956 100644 --- a/lib/ansible/playbook/task.py +++ b/lib/ansible/playbook/task.py @@ -208,6 +208,7 @@ class Task(Base, Conditional, Taggable, CollectionSearch, Notifiable, Delegatabl # But if it wasn't, we can add the yaml object now to get more detail raise AnsibleParserError(to_native(e), obj=ds, orig_exc=e) else: + # Set the resolved action plugin (or if it does not exist, module) for callbacks. self.resolved_action = args_parser.resolved_action # the command/shell/script modules used to support the `cmd` arg, diff --git a/test/integration/targets/collections/test_task_resolved_plugin/unqualified_and_collections_kw.yml b/test/integration/targets/collections/test_task_resolved_plugin/unqualified_and_collections_kw.yml index 5af4eda9ce6..ac707783f71 100644 --- a/test/integration/targets/collections/test_task_resolved_plugin/unqualified_and_collections_kw.yml +++ b/test/integration/targets/collections/test_task_resolved_plugin/unqualified_and_collections_kw.yml @@ -10,5 +10,7 @@ - ping: - collection_action: - collection_module: - - formerly_action: - - formerly_module: + - local_action: + module: formerly_action + - action: + module: formerly_module diff --git a/test/integration/targets/module_defaults/tasks/main.yml b/test/integration/targets/module_defaults/tasks/main.yml index 747c2f9233f..04832785ec4 100644 --- a/test/integration/targets/module_defaults/tasks/main.yml +++ b/test/integration/targets/module_defaults/tasks/main.yml @@ -10,16 +10,23 @@ - debug: register: foo + - local_action: + module: debug + register: local_action_foo + - name: test that 'debug' task used default 'msg' param assert: - that: foo.msg == "test default" + that: + - foo.msg == "test default" + - local_action_foo.msg == "test default" - name: remove test file file: state: absent - name: touch test file - file: + local_action: + module: file state: touch - name: stat test file diff --git a/test/units/playbook/test_block.py b/test/units/playbook/test_block.py index aac5f718c31..3c4dfb1ef91 100644 --- a/test/units/playbook/test_block.py +++ b/test/units/playbook/test_block.py @@ -20,6 +20,10 @@ from __future__ import annotations import unittest from ansible.playbook.block import Block from ansible.playbook.task import Task +from ansible.plugins.loader import init_plugin_loader + + +init_plugin_loader() class TestBlock(unittest.TestCase): diff --git a/test/units/playbook/test_helpers.py b/test/units/playbook/test_helpers.py index 2977b0d6d5c..fb6170cf885 100644 --- a/test/units/playbook/test_helpers.py +++ b/test/units/playbook/test_helpers.py @@ -24,13 +24,16 @@ from unittest.mock import MagicMock from units.mock.loader import DictDataLoader from ansible import errors +from ansible.playbook import helpers from ansible.playbook.block import Block from ansible.playbook.handler import Handler from ansible.playbook.task import Task from ansible.playbook.task_include import TaskInclude from ansible.playbook.role.include import RoleInclude +from ansible.plugins.loader import init_plugin_loader -from ansible.playbook import helpers + +init_plugin_loader() class MixinForMocks(object):