mirror of
https://github.com/ansible/ansible.git
synced 2026-08-02 08:03:18 +02:00
Parser errors from within includes should not be rescueable (#73722)
* Parser errors from within includes should not be rescueable * Also fixes unit tests Fixes #73657
This commit is contained in:
@@ -0,0 +1,2 @@
|
|||||||
|
bugfixes:
|
||||||
|
- Parser errors from within includes should not be rescueable (https://github.com/ansible/ansible/issues/73657)
|
||||||
@@ -35,7 +35,7 @@ from jinja2.exceptions import UndefinedError
|
|||||||
|
|
||||||
from ansible import constants as C
|
from ansible import constants as C
|
||||||
from ansible import context
|
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 import action_write_locks
|
||||||
from ansible.executor.play_iterator import IteratingStates, FailedStates
|
from ansible.executor.play_iterator import IteratingStates, FailedStates
|
||||||
from ansible.executor.process.worker import WorkerProcess
|
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
|
# first processed, we do so now for each host in the list
|
||||||
for host in included_file._hosts:
|
for host in included_file._hosts:
|
||||||
self._tqm._stats.increment('ok', host.name)
|
self._tqm._stats.increment('ok', host.name)
|
||||||
|
except AnsibleParserError:
|
||||||
|
raise
|
||||||
except AnsibleError as e:
|
except AnsibleError as e:
|
||||||
if isinstance(e, AnsibleFileNotFound):
|
if isinstance(e, AnsibleFileNotFound):
|
||||||
reason = "Could not find or access '%s' on the Ansible Controller." % to_text(e.file_name)
|
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:
|
if not result:
|
||||||
break
|
break
|
||||||
|
except AnsibleParserError:
|
||||||
|
raise
|
||||||
except AnsibleError as e:
|
except AnsibleError as e:
|
||||||
for host in included_file._hosts:
|
for host in included_file._hosts:
|
||||||
iterator.mark_host_failed(host)
|
iterator.mark_host_failed(host)
|
||||||
|
|||||||
@@ -34,7 +34,7 @@ DOCUMENTATION = '''
|
|||||||
import time
|
import time
|
||||||
|
|
||||||
from ansible import constants as C
|
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.playbook.included_file import IncludedFile
|
||||||
from ansible.plugins.loader import action_loader
|
from ansible.plugins.loader import action_loader
|
||||||
from ansible.plugins.strategy import StrategyBase
|
from ansible.plugins.strategy import StrategyBase
|
||||||
@@ -257,6 +257,8 @@ class StrategyModule(StrategyBase):
|
|||||||
)
|
)
|
||||||
else:
|
else:
|
||||||
new_blocks = self._load_included_file(included_file, iterator=iterator)
|
new_blocks = self._load_included_file(included_file, iterator=iterator)
|
||||||
|
except AnsibleParserError:
|
||||||
|
raise
|
||||||
except AnsibleError as e:
|
except AnsibleError as e:
|
||||||
for host in included_file._hosts:
|
for host in included_file._hosts:
|
||||||
iterator.mark_host_failed(host)
|
iterator.mark_host_failed(host)
|
||||||
|
|||||||
@@ -32,7 +32,7 @@ DOCUMENTATION = '''
|
|||||||
'''
|
'''
|
||||||
|
|
||||||
from ansible import constants as C
|
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.executor.play_iterator import IteratingStates, FailedStates
|
||||||
from ansible.module_utils._text import to_text
|
from ansible.module_utils._text import to_text
|
||||||
from ansible.playbook.block import Block
|
from ansible.playbook.block import Block
|
||||||
@@ -382,7 +382,8 @@ class StrategyModule(StrategyBase):
|
|||||||
else:
|
else:
|
||||||
all_blocks[host].append(noop_block)
|
all_blocks[host].append(noop_block)
|
||||||
display.debug("done iterating over new_blocks loaded from include file")
|
display.debug("done iterating over new_blocks loaded from include file")
|
||||||
|
except AnsibleParserError:
|
||||||
|
raise
|
||||||
except AnsibleError as e:
|
except AnsibleError as e:
|
||||||
for host in included_file._hosts:
|
for host in included_file._hosts:
|
||||||
self._tqm._failed_hosts[host.name] = True
|
self._tqm._failed_hosts[host.name] = True
|
||||||
|
|||||||
@@ -0,0 +1,8 @@
|
|||||||
|
- hosts: localhost
|
||||||
|
gather_facts: no
|
||||||
|
tasks:
|
||||||
|
- block:
|
||||||
|
- include_tasks: issue73657_tasks.yml
|
||||||
|
rescue:
|
||||||
|
- debug:
|
||||||
|
msg: SHOULD_NOT_EXECUTE
|
||||||
@@ -0,0 +1,2 @@
|
|||||||
|
- wrong.wrong.wrong:
|
||||||
|
parser: error
|
||||||
@@ -135,3 +135,7 @@ cat out.txt
|
|||||||
test "$(grep out.txt -ce 'In imported playbook')" = 2
|
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 tasks')" = 3
|
||||||
test "$(grep out.txt -ce 'In imported role')" = 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
|
||||||
|
|||||||
@@ -29,6 +29,7 @@ from ansible.executor.task_queue_manager import TaskQueueManager
|
|||||||
from ansible.executor.task_result import TaskResult
|
from ansible.executor.task_result import TaskResult
|
||||||
from ansible.inventory.host import Host
|
from ansible.inventory.host import Host
|
||||||
from ansible.module_utils.six.moves import queue as Queue
|
from ansible.module_utils.six.moves import queue as Queue
|
||||||
|
from ansible.playbook.block import Block
|
||||||
from ansible.playbook.handler import Handler
|
from ansible.playbook.handler import Handler
|
||||||
from ansible.plugins.strategy import StrategyBase
|
from ansible.plugins.strategy import StrategyBase
|
||||||
|
|
||||||
@@ -464,7 +465,15 @@ class TestStrategyBase(unittest.TestCase):
|
|||||||
mock_task = MagicMock()
|
mock_task = MagicMock()
|
||||||
mock_task._block = mock_block
|
mock_task._block = mock_block
|
||||||
mock_task._role = None
|
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 = MagicMock()
|
||||||
mock_iterator.mark_host_failed.return_value = None
|
mock_iterator.mark_host_failed.return_value = None
|
||||||
@@ -474,6 +483,8 @@ class TestStrategyBase(unittest.TestCase):
|
|||||||
|
|
||||||
mock_inc_file._filename = "test.yml"
|
mock_inc_file._filename = "test.yml"
|
||||||
res = strategy_base._load_included_file(included_file=mock_inc_file, iterator=mock_iterator)
|
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"
|
mock_inc_file._filename = "bad.yml"
|
||||||
res = strategy_base._load_included_file(included_file=mock_inc_file, iterator=mock_iterator)
|
res = strategy_base._load_included_file(included_file=mock_inc_file, iterator=mock_iterator)
|
||||||
|
|||||||
Reference in New Issue
Block a user