Task.resolved_action - fix resolving static actions consistently for callback plugins (#85524)

* Resolve static actions when the FQCN is already known or demanded by a callback plugin

shorthand syntax (e.g. "- ping:") is resolved by ModuleArgsParser

action/local_action syntax (e.g. "- action: ping") is resolved on demand

* Emit a warning if a callback plugin accesses the property when it's None. This is expected if action/local_action is a template and a callback plugin uses this value too early (like in v2_playbook_on_task_start) or late (like in v2_runner_on_ok for a task with a loop).

changelogs/fragments/85524-resolve-task-resolved_action-early.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - callback plugins - improve consistency accessing the Task object's resolved_action attribute.

lib/ansible/parsing/mod_args.py

@@ -130,6 +130,7 @@ class ModuleArgsParser:
         # HACK: why are these not FieldAttributes on task with a post-validate to check usage?
         self._task_attrs.update(['local_action', 'static'])
         self._task_attrs = frozenset(self._task_attrs)
+        self._resolved_action = None
 
     def _split_module_string(self, module_string: str) -> tuple[str, str]:
         """
@@ -344,6 +345,8 @@ class ModuleArgsParser:
                     raise e
 
                 is_action_candidate = context.resolved and bool(context.redirect_list)
+                if is_action_candidate:
+                    self._resolved_action = context.resolved_fqcn
 
             if is_action_candidate:
                 # finding more than one module name is a problem

lib/ansible/playbook/play.py

@@ -303,7 +303,7 @@ class Play(Base, Taggable, CollectionSearch):
 
         t = Task(block=flush_block)
         t.action = 'meta'
-        t.resolved_action = 'ansible.builtin.meta'
+        t._resolved_action = 'ansible.builtin.meta'
         t.args['_raw_params'] = 'flush_handlers'
         t.implicit = True
         t.set_loader(self._loader)

lib/ansible/playbook/task.py

@@ -41,7 +41,7 @@ from ansible.playbook.role import Role
 from ansible.playbook.taggable import Taggable
 from ansible._internal import _task
 from ansible._internal._templating import _marker_behaviors
-from ansible._internal._templating._jinja_bits import is_possibly_all_template
+from ansible._internal._templating._jinja_bits import is_possibly_all_template, is_possibly_template
 from ansible._internal._templating._engine import TemplateEngine, TemplateOptions
 from ansible.utils.collection_loader import AnsibleCollectionConfig
 from ansible.utils.display import Display
@@ -101,7 +101,7 @@ class Task(Base, Conditional, Taggable, CollectionSearch, Notifiable, Delegatabl
         self._role = role
         self._parent = None
         self.implicit = False
-        self.resolved_action: str | None = None
+        self._resolved_action: str | None = None
 
         if task_include:
             self._parent = task_include
@@ -110,6 +110,38 @@ class Task(Base, Conditional, Taggable, CollectionSearch, Notifiable, Delegatabl
 
         super(Task, self).__init__()
 
+    _resolved_action_warning = (
+        "A plugin is sampling the task's resolved_action when it is not resolved. "
+        "This can be caused by callback plugins using the resolved_action attribute too "
+        "early (such as in v2_playbook_on_task_start for a task using the action/local_action "
+        "keyword), or too late (such as in v2_runner_on_ok for a task with a loop). "
+        "To maximize compatibility with user features, callback plugins should "
+        "only use this attribute in v2_runner_on_ok/v2_runner_on_failed for tasks "
+        "without a loop, and v2_runner_item_on_ok/v2_runner_item_on_failed otherwise."
+    )
+
+    @property
+    def resolved_action(self) -> str | None:
+        """The templated and resolved FQCN of the task action or None.
+
+        If the action is a template, callback plugins can only use this value in certain methods.
+        - v2_runner_on_ok and v2_runner_on_failed if there's no task loop
+        - v2_runner_item_on_ok and v2_runner_item_on_failed if there is a task loop
+        """
+        # Consider deprecating this because it's difficult to use?
+        # Moving it to the task result would improve the no-loop limitation on v2_runner_on_ok
+        # but then wouldn't be accessible to v2_playbook_on_task_start, *_on_skipped, etc.
+        if self._resolved_action is not None:
+            return self._resolved_action
+        if not is_possibly_template(self.action):
+            try:
+                return self._resolve_action(self.action)
+            except AnsibleParserError:
+                display.warning(self._resolved_action_warning, obj=self.action)
+        else:
+            display.warning(self._resolved_action_warning, obj=self.action)
+        return None
+
     def get_name(self, include_role_fqcn=True):
         """ return the name of the task """
 
@@ -168,7 +200,7 @@ class Task(Base, Conditional, Taggable, CollectionSearch, Notifiable, Delegatabl
         else:
             module_or_action_context = action_context.plugin_load_context
 
-        self.resolved_action = module_or_action_context.resolved_fqcn
+        self._resolved_action = module_or_action_context.resolved_fqcn
 
         action_type: type[ActionBase] = action_context.object
 
@@ -282,6 +314,9 @@ 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("Error parsing task arguments.", obj=ds) from ex
 
+        if args_parser._resolved_action is not None:
+            self._resolved_action = args_parser._resolved_action
+
         new_ds['action'] = action
         new_ds['args'] = args
         new_ds['delegate_to'] = delegate_to
@@ -465,7 +500,7 @@ class Task(Base, Conditional, Taggable, CollectionSearch, Notifiable, Delegatabl
             new_me._role = self._role
 
         new_me.implicit = self.implicit
-        new_me.resolved_action = self.resolved_action
+        new_me._resolved_action = self._resolved_action
         new_me._uuid = self._uuid
 
         return new_me
@@ -482,7 +517,7 @@ class Task(Base, Conditional, Taggable, CollectionSearch, Notifiable, Delegatabl
                 data['role'] = self._role.serialize()
 
             data['implicit'] = self.implicit
-            data['resolved_action'] = self.resolved_action
+            data['_resolved_action'] = self._resolved_action
 
         return data
 
@@ -513,7 +548,7 @@ class Task(Base, Conditional, Taggable, CollectionSearch, Notifiable, Delegatabl
             del data['role']
 
         self.implicit = data.get('implicit', False)
-        self.resolved_action = data.get('resolved_action')
+        self._resolved_action = data.get('_resolved_action')
 
         super(Task, self).deserialize(data)
 
@@ -591,7 +626,7 @@ class Task(Base, Conditional, Taggable, CollectionSearch, Notifiable, Delegatabl
     def dump_attrs(self):
         """Override to smuggle important non-FieldAttribute values back to the controller."""
         attrs = super().dump_attrs()
-        attrs.update(resolved_action=self.resolved_action)
+        attrs.update(_resolved_action=self._resolved_action)
         return attrs
 
     def _resolve_conditional(

lib/ansible/plugins/strategy/__init__.py

@@ -903,7 +903,7 @@ class StrategyBase:
         display.warning("%s task does not support when conditional" % task_name)
 
     def _execute_meta(self, task: Task, play_context, iterator, target_host: Host):
-        task.resolved_action = 'ansible.builtin.meta'  # _post_validate_args is never called for meta actions, so resolved_action hasn't been set
+        task._resolved_action = 'ansible.builtin.meta'  # _post_validate_args is never called for meta actions, so resolved_action hasn't been set
 
         # meta tasks store their args in the _raw_params field of args,
         # since they do not use k=v pairs, so get that

test/integration/targets/collections/test_task_resolved_plugin.sh

@@ -15,6 +15,22 @@ for result in "${action_resolution[@]}"; do
     grep -q out.txt -e "$result"
 done
 
+# Test local_action/action warning
+export ANSIBLE_TEST_ON_TASK_START=True
+ansible-playbook -i debug, test_task_resolved_plugin/dynamic_action.yml "$@" 2>&1 | tee out.txt
+grep -q out.txt -e "A plugin is sampling the task's resolved_action when it is not resolved"
+grep -q out.txt -e "v2_playbook_on_task_start: {{ inventory_hostname }} == None"
+grep -q out.txt -e "v2_runner_on_ok: debug == ansible.builtin.debug"
+grep -q out.txt -e "v2_runner_item_on_ok: debug == ansible.builtin.debug"
+
+# Test static actions don't cause a warning
+ansible-playbook test_task_resolved_plugin/unqualified.yml "$@" 2>&1 | tee out.txt
+grep -v out.txt -e "A plugin is sampling the task's resolved_action when it is not resolved"
+for result in "${action_resolution[@]}"; do
+    grep -q out.txt -e "v2_playbook_on_task_start: $result"
+done
+unset ANSIBLE_TEST_ON_TASK_START
+
 ansible-playbook test_task_resolved_plugin/unqualified_and_collections_kw.yml "$@" | tee out.txt
 action_resolution=(
     "legacy_action == legacy_action"

test/integration/targets/collections/test_task_resolved_plugin/callback_plugins/display_resolved_action.py

@@ -9,6 +9,12 @@ DOCUMENTATION = """
     short_description: Displays the requested and resolved actions at the end of a playbook.
     description:
         - Displays the requested and resolved actions in the format "requested == resolved".
+    options:
+      test_on_task_start:
+        description: Test using task.resolved_action before it is reliably resolved.
+        default: False
+        env:
+          - name: ANSIBLE_TEST_ON_TASK_START
     requirements:
       - Enable in configuration.
 """
@@ -25,11 +31,14 @@ class CallbackModule(CallbackBase):
 
     def __init__(self, *args, **kwargs):
         super(CallbackModule, self).__init__(*args, **kwargs)
-        self.requested_to_resolved = {}
+
+    def v2_playbook_on_task_start(self, task, is_conditional):
+        if self.get_option("test_on_task_start"):
+            self._display.display(f"v2_playbook_on_task_start: {task.action} == {task.resolved_action}")
+
+    def v2_runner_item_on_ok(self, result):
+        self._display.display(f"v2_runner_item_on_ok: {result.task.action} == {result.task.resolved_action}")
 
     def v2_runner_on_ok(self, result):
-        self.requested_to_resolved[result.task.action] = result.task.resolved_action
-
-    def v2_playbook_on_stats(self, stats):
-        for requested, resolved in self.requested_to_resolved.items():
-            self._display.display("%s == %s" % (requested, resolved), screen_only=True)
+        if not result.task.loop:
+            self._display.display(f"v2_runner_on_ok: {result.task.action} == {result.task.resolved_action}")

test/integration/targets/collections/test_task_resolved_plugin/dynamic_action.yml

@@ -0,0 +1,10 @@
+---
+- hosts: all
+  gather_facts: no
+  tasks:
+    - name: Run dynamic action
+      action: "{{ inventory_hostname }}"
+
+    - name: Run dynamic action in loop
+      action: "{{ inventory_hostname }}"
+      loop: [1]

test/integration/targets/collections/test_task_resolved_plugin/unqualified.yml

@@ -4,5 +4,5 @@
   tasks:
     - legacy_action:
     - legacy_module:
-    - debug:
-    - ping:
+    - local_action: debug
+    - action: ping