diff --git a/changelogs/fragments/include_tasks_fail_rescue.yml b/changelogs/fragments/include_tasks_fail_rescue.yml new file mode 100644 index 00000000000..9c8a9a78a2d --- /dev/null +++ b/changelogs/fragments/include_tasks_fail_rescue.yml @@ -0,0 +1,5 @@ +minor_changes: + - include_role has new option `rescuable` to allow it to toggle between task failure and syntax errors. +bugfixes: + - include_role would emit a syntax error on X_from options errors, but a task failure when missing a role to make it consistent now it also emits a task failure on missing tasks_from, which makes it subject to error handling in the play. + - include_role, would ignore missing X_from files if the subdir (tasks/vars/handlers/defaults) did not exist, now it is a proper error. diff --git a/lib/ansible/modules/include_role.py b/lib/ansible/modules/include_role.py index e800c5e61c9..d42a05c5e70 100644 --- a/lib/ansible/modules/include_role.py +++ b/lib/ansible/modules/include_role.py @@ -71,6 +71,12 @@ options: type: bool default: yes version_added: '2.11' + rescuable: + description: + - This toggle allows for errors from the include itself to either be a task failure, which is 'rescuable', or fatal syntax errors. + type: bool + default: yes + version_added: '2.21' extends_documentation_fragment: - action_common_attributes - action_common_attributes.conn @@ -83,6 +89,8 @@ attributes: diff_mode: support: none notes: + - Beginning in ansible 2.21 we have normalized how error types from the include itself are emitted. + Using the O(rescuable) option, you can control if it is a task failure or a syntax error. - Handlers and are made available to the whole play. - After Ansible 2.4, you can use M(ansible.builtin.import_role) for B(static) behaviour and this action for B(dynamic) one. seealso: diff --git a/lib/ansible/playbook/role/__init__.py b/lib/ansible/playbook/role/__init__.py index ba0f1747a90..ab79c55765b 100644 --- a/lib/ansible/playbook/role/__init__.py +++ b/lib/ansible/playbook/role/__init__.py @@ -17,6 +17,7 @@ from __future__ import annotations +import functools as _functools import os import typing as _t @@ -24,7 +25,7 @@ from collections.abc import Container, Mapping, Set, Sequence from types import MappingProxyType from ansible import constants as C -from ansible.errors import AnsibleError, AnsibleParserError, AnsibleAssertionError +from ansible.errors import AnsibleError, AnsibleParserError, AnsibleAssertionError, AnsibleActionFail from ansible.module_utils.common.sentinel import Sentinel from ansible.module_utils.common.text.converters import to_text from ansible.playbook.base import Base @@ -118,13 +119,16 @@ class Role(Base, Conditional, Taggable, CollectionSearch, Delegatable): from_include: bool = False, validate: bool = True, public: bool = None, - static: bool = True) -> None: + static: bool = True, + rescuable: bool = True) -> None: + self._role_name: str = None self._role_path: str = None self._role_collection: str = None self._role_params: dict[str, dict[str, str]] = dict() self._loader = None self.static: bool = static + self._rescuable: bool = rescuable # includes (static=false) default to private, while imports (static=true) default to public # but both can be overridden by global config if set @@ -165,6 +169,10 @@ class Role(Base, Conditional, Taggable, CollectionSearch, Delegatable): def __repr__(self): return self.get_name() + @_functools.cached_property + def _FAIL(self): + return AnsibleActionFail if self._rescuable else AnsibleParserError + def get_name(self, include_role_fqcn=True): if include_role_fqcn: return '.'.join(x for x in (self._role_collection, self._role_name) if x) @@ -198,7 +206,7 @@ class Role(Base, Conditional, Taggable, CollectionSearch, Delegatable): return self._get_hash_dict() == other._get_hash_dict() @staticmethod - def load(role_include, play, parent_role=None, from_files=None, from_include=False, validate=True, public=None, static=True): + def load(role_include, play, parent_role=None, from_files=None, from_include=False, validate=True, public=None, static=True, rescuable=True): if from_files is None: from_files = {} try: @@ -206,7 +214,7 @@ class Role(Base, Conditional, Taggable, CollectionSearch, Delegatable): # for the in-flight in role cache as a sentinel that we're already trying to load # that role?) # see https://github.com/ansible/ansible/issues/61527 - r = Role(play=play, from_files=from_files, from_include=from_include, validate=validate, public=public, static=static) + r = Role(play=play, from_files=from_files, from_include=from_include, validate=validate, public=public, static=static, rescuable=rescuable) r._load_role_data(role_include, parent_role=parent_role) role_path = r.get_role_path() @@ -428,8 +436,8 @@ class Role(Base, Conditional, Taggable, CollectionSearch, Delegatable): for found in found_files: if not is_subpath(found, file_path): - raise AnsibleParserError("Failed loading '%s' for role (%s) as it is not inside the expected role path: '%s'" % - (to_text(found), self._role_name, to_text(file_path))) + raise self._FAIL(f"Failed loading '{found!r}' for role ({self._role_name}) " + f"as it is not inside the expected role path: {file_path!r}") new_data = self._loader.load_from_file(found, trusted_as_template=True) if new_data: @@ -444,7 +452,10 @@ class Role(Base, Conditional, Taggable, CollectionSearch, Delegatable): elif main is not None: # this won't trigger with default only when _from is specified - raise AnsibleParserError("Could not find specified file in role: %s/%s" % (subdir, main)) + raise self._FAIL(f"Could not find specified file in role: {subdir}/{main}") + elif main is not None: + # this won't trigger with default only when _from is specified + raise self._FAIL(f"Could not find specified file in role, its '{subdir}/' is not usable.") return data diff --git a/lib/ansible/playbook/role_include.py b/lib/ansible/playbook/role_include.py index e9a6d7072b2..3c44ed340ab 100644 --- a/lib/ansible/playbook/role_include.py +++ b/lib/ansible/playbook/role_include.py @@ -17,7 +17,7 @@ from __future__ import annotations import ansible.constants as C -from ansible.errors import AnsibleParserError +from ansible.errors import AnsibleError, AnsibleParserError from ansible.playbook.attribute import NonInheritableFieldAttribute from ansible.playbook.task_include import TaskInclude from ansible.playbook.role import Role @@ -39,7 +39,7 @@ class IncludeRole(TaskInclude): BASE = frozenset(('name', 'role')) # directly assigned FROM_ARGS = frozenset(('tasks_from', 'vars_from', 'defaults_from', 'handlers_from')) # used to populate from dict in role - OTHER_ARGS = frozenset(('apply', 'public', 'allow_duplicates', 'rolespec_validate')) # assigned to matching property + OTHER_ARGS = frozenset(('apply', 'public', 'allow_duplicates', 'rolespec_validate', 'rescuable')) # assigned to matching property VALID_ARGS = BASE | FROM_ARGS | OTHER_ARGS # all valid args # ================================================================================= @@ -49,6 +49,7 @@ class IncludeRole(TaskInclude): # private as this is a 'module options' vs a task property allow_duplicates = NonInheritableFieldAttribute(isa='bool', default=True, private=True, always_post_validate=True) rolespec_validate = NonInheritableFieldAttribute(isa='bool', default=True, private=True, always_post_validate=True) + rescuable = NonInheritableFieldAttribute(isa='bool', default=True, private=True, always_post_validate=True) def __init__(self, block=None, role=None, task_include=None): @@ -71,7 +72,13 @@ class IncludeRole(TaskInclude): else: myplay = play - ri = RoleInclude.load(self._role_name, play=myplay, variable_manager=variable_manager, loader=loader, collection_list=self.collections) + try: + ri = RoleInclude.load(self._role_name, play=myplay, variable_manager=variable_manager, loader=loader, collection_list=self.collections) + except AnsibleError as e: + if not self.rescuable: + raise AnsibleParserError("Could not include role.") from e + raise + ri.vars |= self.vars if variable_manager is not None: @@ -82,8 +89,8 @@ class IncludeRole(TaskInclude): from_files = templar.template(self._from_files) # build role - actual_role = Role.load(ri, myplay, parent_role=self._parent_role, from_files=from_files, - from_include=True, validate=self.rolespec_validate, public=self.public, static=self.statically_loaded) + actual_role = Role.load(ri, myplay, parent_role=self._parent_role, from_files=from_files, from_include=True, + validate=self.rolespec_validate, public=self.public, static=self.statically_loaded, rescuable=self.rescuable) actual_role._metadata.allow_duplicates = self.allow_duplicates # add role to play @@ -140,13 +147,17 @@ class IncludeRole(TaskInclude): raise AnsibleParserError('Expected a string for %s but got %s instead' % (key, type(args_value))) ir._from_files[from_key] = args_value - # apply is only valid for includes, not imports as they inherit directly + # apply and rescuable are only valid for includes, not imports as they inherit directly apply_attrs = ir.args.get('apply', {}) if apply_attrs and ir.action not in C._ACTION_INCLUDE_ROLE: raise AnsibleParserError('Invalid options for %s: apply' % ir.action, obj=data) elif not isinstance(apply_attrs, dict): raise AnsibleParserError('Expected a dict for apply but got %s instead' % type(apply_attrs), obj=data) + resc_attr = ir.args.get('rescuable', None) + if resc_attr and ir.action not in C._ACTION_INCLUDE_ROLE: + raise AnsibleParserError(f'Invalid options for {ir.action}: rescuable', obj=data) + # manual list as otherwise the options would set other task parameters we don't want. for option in my_arg_names.intersection(IncludeRole.OTHER_ARGS): setattr(ir, option, ir.args.get(option)) diff --git a/test/integration/targets/includes/import_no_rescue.yml b/test/integration/targets/includes/import_no_rescue.yml new file mode 100644 index 00000000000..7d9e9085662 --- /dev/null +++ b/test/integration/targets/includes/import_no_rescue.yml @@ -0,0 +1,6 @@ +- hosts: localhost + gather_facts: no + tasks: + - import_role: + name: test_includes + rescuable: true diff --git a/test/integration/targets/includes/include_role_error_handling.yml b/test/integration/targets/includes/include_role_error_handling.yml new file mode 100644 index 00000000000..e32e6800585 --- /dev/null +++ b/test/integration/targets/includes/include_role_error_handling.yml @@ -0,0 +1,37 @@ +- hosts: localhost + gather_facts: false + tasks: + - name: test missing role rescue + vars: + rescue_1: false + block: + - name: Include a role that doesn't exist + include_role: + name: missing_role + rescuable: '{{ rescueme | default(omit) }}' + + rescue: + - set_fact: + rescue_1: true + always: + - assert: + that: + - rescue_1 == rescueme|default(True) + + - name: Test _from rescue + vars: + rescue_2: false + block: + - name: Include a task file that doesn't exist, but role exists + include_role: + name: include_roles + tasks_from: missing_task_list + rescuable: '{{ rescueme | default(omit) }}' + + rescue: + - set_fact: + rescue_2: true + always: + - assert: + that: + - rescue_2 == rescueme|default(True) diff --git a/test/integration/targets/includes/include_role_missing.yml b/test/integration/targets/includes/include_role_missing.yml new file mode 100644 index 00000000000..bdb6f1cb346 --- /dev/null +++ b/test/integration/targets/includes/include_role_missing.yml @@ -0,0 +1,24 @@ +- hosts: localhost + gather_facts: no + tasks: + - file: + path: roles/rolename + state: "{{ item }}" + loop: + - absent + - directory + + - name: ensure we fail when missing both file and tasks/ subdir + block: + - include_role: + name: rolename + tasks_from: missing.yml + rescue: + - set_fact: + wefailed: True + always: + - name: check we actually failed + assert: + that: + - wefailed is defined + - wefailed diff --git a/test/integration/targets/includes/runme.sh b/test/integration/targets/includes/runme.sh index f87e4048b6d..587e311a8d6 100755 --- a/test/integration/targets/includes/runme.sh +++ b/test/integration/targets/includes/runme.sh @@ -18,3 +18,15 @@ ansible-playbook includes_loop_rescue.yml --extra-vars strategy=linear "$@" ansible-playbook includes_loop_rescue.yml --extra-vars strategy=free "$@" ansible-playbook includes_from_dedup.yml -i ../../inventory "$@" + +# test 'rescueable' default (true) +ansible-playbook include_role_error_handling.yml "$@" +# test 'rescueable' explicit true +ansible-playbook include_role_error_handling.yml "$@" -e '{"rescueme": true}' +# test 'rescueable' explicit false +[[ $(ansible-playbook include_role_error_handling.yml "$@" -e '{"rescueme": false}') != 0 ]] +# ensure imports are not rescuable +[[ $(ansible-playbook import_no_rescue.yml "$@") != 0 ]] + +# test for missing task_from when missing tasks/ +ansible-playbook include_role_missing.yml "$@"