From d1d94060660f749af479a7dec1067f5f54846064 Mon Sep 17 00:00:00 2001 From: David Shrewsbury Date: Fri, 12 Feb 2021 17:26:33 -0500 Subject: [PATCH] Add rolespec_validate to import/include_role (#73589) * Add rolespec_validate to import/include_role * Add changelog * fix sanity, not private --- .../fragments/73589-rolespec-validate.yml | 3 +++ lib/ansible/modules/import_role.py | 6 +++++ lib/ansible/modules/include_role.py | 6 +++++ lib/ansible/playbook/role/__init__.py | 10 ++++---- lib/ansible/playbook/role_include.py | 5 ++-- .../targets/roles_arg_spec/test.yml | 23 +++++++++++++++++++ 6 files changed, 47 insertions(+), 6 deletions(-) create mode 100644 changelogs/fragments/73589-rolespec-validate.yml diff --git a/changelogs/fragments/73589-rolespec-validate.yml b/changelogs/fragments/73589-rolespec-validate.yml new file mode 100644 index 00000000000..51b3f0e3a3f --- /dev/null +++ b/changelogs/fragments/73589-rolespec-validate.yml @@ -0,0 +1,3 @@ +minor_changes: + - Add new rolespec_validate option to the import/include_role modules do allow + disabling of the implicit role arg validation task on a per-role basis. diff --git a/lib/ansible/modules/import_role.py b/lib/ansible/modules/import_role.py index 60eddbdc45e..8de4f428d83 100644 --- a/lib/ansible/modules/import_role.py +++ b/lib/ansible/modules/import_role.py @@ -51,6 +51,12 @@ options: type: str default: main version_added: '2.8' + rolespec_validate: + description: + - Perform role argument spec validation if an argument spec is defined. + type: bool + default: yes + version_added: '2.11' notes: - Handlers are made available to the whole play. - Since Ansible 2.7 variables defined in C(vars) and C(defaults) for the role are exposed to the play at playbook parsing time. diff --git a/lib/ansible/modules/include_role.py b/lib/ansible/modules/include_role.py index 3cca919ac45..663de725ba0 100644 --- a/lib/ansible/modules/include_role.py +++ b/lib/ansible/modules/include_role.py @@ -67,6 +67,12 @@ options: type: str default: main version_added: '2.8' + rolespec_validate: + description: + - Perform role argument spec validation if an argument spec is defined. + type: bool + default: yes + version_added: '2.11' notes: - Handlers are made available to the whole play. - Before Ansible 2.4, as with C(include), this task could be static or dynamic, If static, it implied that it won't diff --git a/lib/ansible/playbook/role/__init__.py b/lib/ansible/playbook/role/__init__.py index 9c0921794ba..448d2a98031 100644 --- a/lib/ansible/playbook/role/__init__.py +++ b/lib/ansible/playbook/role/__init__.py @@ -100,7 +100,7 @@ class Role(Base, Conditional, Taggable, CollectionSearch): _delegate_to = FieldAttribute(isa='string') _delegate_facts = FieldAttribute(isa='bool') - def __init__(self, play=None, from_files=None, from_include=False): + def __init__(self, play=None, from_files=None, from_include=False, validate=True): self._role_name = None self._role_path = None self._role_collection = None @@ -118,6 +118,7 @@ class Role(Base, Conditional, Taggable, CollectionSearch): self._role_vars = dict() self._had_task_run = dict() self._completed = dict() + self._should_validate = validate if from_files is None: from_files = {} @@ -137,7 +138,7 @@ class Role(Base, Conditional, Taggable, CollectionSearch): return self._role_name @staticmethod - def load(role_include, play, parent_role=None, from_files=None, from_include=False): + def load(role_include, play, parent_role=None, from_files=None, from_include=False, validate=True): if from_files is None: from_files = {} @@ -171,7 +172,7 @@ class Role(Base, Conditional, Taggable, CollectionSearch): # 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) + r = Role(play=play, from_files=from_files, from_include=from_include, validate=validate) r._load_role_data(role_include, parent_role=parent_role) if role_include.get_name() not in play.ROLE_CACHE: @@ -256,7 +257,8 @@ class Role(Base, Conditional, Taggable, CollectionSearch): task_data = self._load_role_yaml('tasks', main=self._from_files.get('tasks')) - task_data = self._prepend_validation_task(task_data) + if self._should_validate: + task_data = self._prepend_validation_task(task_data) if task_data: try: diff --git a/lib/ansible/playbook/role_include.py b/lib/ansible/playbook/role_include.py index 2ae80ca6f43..59803f27c15 100644 --- a/lib/ansible/playbook/role_include.py +++ b/lib/ansible/playbook/role_include.py @@ -44,7 +44,7 @@ class IncludeRole(TaskInclude): BASE = ('name', 'role') # directly assigned FROM_ARGS = ('tasks_from', 'vars_from', 'defaults_from', 'handlers_from') # used to populate from dict in role - OTHER_ARGS = ('apply', 'public', 'allow_duplicates') # assigned to matching property + OTHER_ARGS = ('apply', 'public', 'allow_duplicates', 'rolespec_validate') # assigned to matching property VALID_ARGS = tuple(frozenset(BASE + FROM_ARGS + OTHER_ARGS)) # all valid args # ================================================================================= @@ -53,6 +53,7 @@ class IncludeRole(TaskInclude): # private as this is a 'module options' vs a task property _allow_duplicates = FieldAttribute(isa='bool', default=True, private=True) _public = FieldAttribute(isa='bool', default=False, private=True) + _rolespec_validate = FieldAttribute(isa='bool', default=True) def __init__(self, block=None, role=None, task_include=None): @@ -80,7 +81,7 @@ class IncludeRole(TaskInclude): # build role actual_role = Role.load(ri, myplay, parent_role=self._parent_role, from_files=self._from_files, - from_include=True) + from_include=True, validate=self.rolespec_validate) actual_role._metadata.allow_duplicates = self.allow_duplicates if self.statically_loaded or self.public: diff --git a/test/integration/targets/roles_arg_spec/test.yml b/test/integration/targets/roles_arg_spec/test.yml index 747938bafbe..454b68de20b 100644 --- a/test/integration/targets/roles_arg_spec/test.yml +++ b/test/integration/targets/roles_arg_spec/test.yml @@ -247,3 +247,26 @@ - ansible_failed_result.validate_args_context.name == "role_with_no_tasks" - ansible_failed_result.validate_args_context.type == "role" - "ansible_failed_result.validate_args_context.path is search('roles_arg_spec/roles/role_with_no_tasks')" + +- name: "New play to reset vars: Test disabling role validation with rolespec_validate=False" + hosts: localhost + gather_facts: false + tasks: + - block: + - name: "Test import_role of role C (missing a_str), but validation turned off" + import_role: + name: c + rolespec_validate: False + - fail: + msg: "Should not get here" + + rescue: + - debug: + var: ansible_failed_result + - name: "Validate import_role failure" + assert: + that: + # We expect the role to actually run, but will fail because an undefined variable was referenced + # and validation wasn't performed up front (thus not returning 'argument_errors'). + - "'argument_errors' not in ansible_failed_result" + - "'The task includes an option with an undefined variable.' in ansible_failed_result.msg"