Parser errors from within includes should not be rescueable

changelogs/fragments/73657-include-parser-error-fail.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - Parser errors from within includes should not be rescueable (https://github.com/ansible/ansible/issues/73657)

lib/ansible/plugins/strategy/__init__.py

@@ -35,7 +35,7 @@
 
 from ansible import constants as C
 from ansible import context
-from ansible.errors import AnsibleError, AnsibleFileNotFound, AnsibleUndefinedVariable
+from ansible.errors import AnsibleError, AnsibleFileNotFound, AnsibleUndefinedVariable, AnsibleParserError
 from ansible.executor import action_write_locks
 from ansible.executor.play_iterator import IteratingStates, FailedStates
 from ansible.executor.process.worker import WorkerProcess
@@ -945,7 +945,8 @@ def _load_included_file(self, included_file, iterator, is_handler=False):
             # first processed, we do so now for each host in the list
             for host in included_file._hosts:
                 self._tqm._stats.increment('ok', host.name)
-
+        except AnsibleParserError:
+            raise
         except AnsibleError as e:
             if isinstance(e, AnsibleFileNotFound):
                 reason = "Could not find or access '%s' on the Ansible Controller." % to_text(e.file_name)
@@ -1061,6 +1062,8 @@ def _do_handler_run(self, handler, handler_name, iterator, play_context, notifie
                             )
                             if not result:
                                 break
+                except AnsibleParserError:
+                    raise
                 except AnsibleError as e:
                     for host in included_file._hosts:
                         iterator.mark_host_failed(host)

lib/ansible/plugins/strategy/free.py

@@ -34,7 +34,7 @@
 import time
 
 from ansible import constants as C
-from ansible.errors import AnsibleError
+from ansible.errors import AnsibleError, AnsibleParserError
 from ansible.playbook.included_file import IncludedFile
 from ansible.plugins.loader import action_loader
 from ansible.plugins.strategy import StrategyBase
@@ -257,6 +257,8 @@ def run(self, iterator, play_context):
                             )
                         else:
                             new_blocks = self._load_included_file(included_file, iterator=iterator)
+                    except AnsibleParserError:
+                        raise
                     except AnsibleError as e:
                         for host in included_file._hosts:
                             iterator.mark_host_failed(host)

lib/ansible/plugins/strategy/linear.py

@@ -32,7 +32,7 @@
 '''
 
 from ansible import constants as C
-from ansible.errors import AnsibleError, AnsibleAssertionError
+from ansible.errors import AnsibleError, AnsibleAssertionError, AnsibleParserError
 from ansible.executor.play_iterator import IteratingStates, FailedStates
 from ansible.module_utils._text import to_text
 from ansible.playbook.block import Block
@@ -382,7 +382,8 @@ def run(self, iterator, play_context):
                                     else:
                                         all_blocks[host].append(noop_block)
                             display.debug("done iterating over new_blocks loaded from include file")
-
+                        except AnsibleParserError:
+                            raise
                         except AnsibleError as e:
                             for host in included_file._hosts:
                                 self._tqm._failed_hosts[host.name] = True

test/integration/targets/include_import/issue73657.yml

@@ -0,0 +1,8 @@
+- hosts: localhost
+  gather_facts: no
+  tasks:
+    - block:
+        - include_tasks: issue73657_tasks.yml
+      rescue:
+        - debug:
+            msg: SHOULD_NOT_EXECUTE

test/integration/targets/include_import/issue73657_tasks.yml

@@ -0,0 +1,2 @@
+- wrong.wrong.wrong:
+    parser: error

test/integration/targets/include_import/runme.sh

@@ -135,3 +135,7 @@ cat out.txt
 test "$(grep out.txt -ce 'In imported playbook')" = 2
 test "$(grep out.txt -ce 'In imported tasks')" = 3
 test "$(grep out.txt -ce 'In imported role')" = 3
+
+# https://github.com/ansible/ansible/issues/73657
+ansible-playbook issue73657.yml 2>&1 | tee issue73657.out
+test "$(grep -c 'SHOULD_NOT_EXECUTE' issue73657.out)" = 0

test/units/plugins/strategy/test_strategy.py

@@ -29,6 +29,7 @@
 from ansible.executor.task_result import TaskResult
 from ansible.inventory.host import Host
 from ansible.module_utils.six.moves import queue as Queue
+from ansible.playbook.block import Block
 from ansible.playbook.handler import Handler
 from ansible.plugins.strategy import StrategyBase
 
@@ -464,7 +465,15 @@ def _queue_put(item, *args, **kwargs):
         mock_task = MagicMock()
         mock_task._block = mock_block
         mock_task._role = None
-        mock_task._parent = None
+
+        # NOTE Mocking calls below to account for passing parent_block=ti_copy.build_parent_block()
+        # into load_list_of_blocks() in _load_included_file. Not doing so meant that retrieving
+        # `collection` attr from parent would result in getting MagicMock instance
+        # instead of an empty list.
+        mock_task._parent = MagicMock()
+        mock_task.copy.return_value = mock_task
+        mock_task.build_parent_block.return_value = mock_block
+        mock_block._get_parent_attribute.return_value = None
 
         mock_iterator = MagicMock()
         mock_iterator.mark_host_failed.return_value = None
@@ -474,6 +483,8 @@ def _queue_put(item, *args, **kwargs):
 
         mock_inc_file._filename = "test.yml"
         res = strategy_base._load_included_file(included_file=mock_inc_file, iterator=mock_iterator)
+        self.assertEqual(len(res), 1)
+        self.assertTrue(isinstance(res[0], Block))
 
         mock_inc_file._filename = "bad.yml"
         res = strategy_base._load_included_file(included_file=mock_inc_file, iterator=mock_iterator)