diff --git a/changelogs/fragments/73657-include-parser-error-fail.yml b/changelogs/fragments/73657-include-parser-error-fail.yml new file mode 100644 index 00000000000..909115ed935 --- /dev/null +++ b/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) diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py index 2cb5e24f575..581334546b4 100644 --- a/lib/ansible/plugins/strategy/__init__.py +++ b/lib/ansible/plugins/strategy/__init__.py @@ -35,7 +35,7 @@ from jinja2.exceptions import UndefinedError 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 @@ class StrategyBase: # 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 @@ class StrategyBase: ) if not result: break + except AnsibleParserError: + raise except AnsibleError as e: for host in included_file._hosts: iterator.mark_host_failed(host) diff --git a/lib/ansible/plugins/strategy/free.py b/lib/ansible/plugins/strategy/free.py index 326c347ad3e..75aa643e5e6 100644 --- a/lib/ansible/plugins/strategy/free.py +++ b/lib/ansible/plugins/strategy/free.py @@ -34,7 +34,7 @@ DOCUMENTATION = ''' 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 @@ class StrategyModule(StrategyBase): ) 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) diff --git a/lib/ansible/plugins/strategy/linear.py b/lib/ansible/plugins/strategy/linear.py index cc2148c6d04..4d355b56c74 100644 --- a/lib/ansible/plugins/strategy/linear.py +++ b/lib/ansible/plugins/strategy/linear.py @@ -32,7 +32,7 @@ DOCUMENTATION = ''' ''' 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 @@ class StrategyModule(StrategyBase): 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 diff --git a/test/integration/targets/include_import/issue73657.yml b/test/integration/targets/include_import/issue73657.yml new file mode 100644 index 00000000000..b692ccb53e8 --- /dev/null +++ b/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 diff --git a/test/integration/targets/include_import/issue73657_tasks.yml b/test/integration/targets/include_import/issue73657_tasks.yml new file mode 100644 index 00000000000..7247d76922b --- /dev/null +++ b/test/integration/targets/include_import/issue73657_tasks.yml @@ -0,0 +1,2 @@ +- wrong.wrong.wrong: + parser: error diff --git a/test/integration/targets/include_import/runme.sh b/test/integration/targets/include_import/runme.sh index 7029ab6d627..b60ecffbc59 100755 --- a/test/integration/targets/include_import/runme.sh +++ b/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 diff --git a/test/units/plugins/strategy/test_strategy.py b/test/units/plugins/strategy/test_strategy.py index 6b60e692e82..433ec064e02 100644 --- a/test/units/plugins/strategy/test_strategy.py +++ b/test/units/plugins/strategy/test_strategy.py @@ -29,6 +29,7 @@ from ansible.executor.task_queue_manager import TaskQueueManager 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 @@ class TestStrategyBase(unittest.TestCase): 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 @@ class TestStrategyBase(unittest.TestCase): 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)