diff options
23 files changed, 145 insertions, 9 deletions
diff --git a/docs/docsite/rst/porting_guides/porting_guide_2.7.rst b/docs/docsite/rst/porting_guides/porting_guide_2.7.rst index cb7db78bb4..5f370bc2c9 100644 --- a/docs/docsite/rst/porting_guides/porting_guide_2.7.rst +++ b/docs/docsite/rst/porting_guides/porting_guide_2.7.rst @@ -26,6 +26,15 @@ Before Ansible 2.7, when loading a role, the variables defined in the role's ``v In Ansible 2.7, role ``vars`` and ``defaults`` are now parsed before ``tasks/main.yml``. This can cause a change in behavior if the same variable is defined at the play level and the role level with different values, and leveraged in ``import_tasks`` or ``import_role`` to define the role or file to import. +include_role and import_role variable exposure +---------------------------------------------- + +In Ansible 2.7 a new module argument named ``public`` was added to the ``include_role`` module that dictates whether or not the role's ``defaults`` and ``vars`` will be exposed outside of the role, allowing those variables to be used by later tasks. This value defaults to ``public: False``, matching current behavior. + +``import_role`` does not support the ``public`` argument, and will unconditionally expose the role's ``defaults`` and ``vars`` to the rest of the playbook. This functinality brings ``import_role`` into closer alignment with roles listed within the ``roles`` header in a play. + +There is an important difference in the way that ``include_role`` (dynamic) will expose the role's variables, as opposed to ``import_role`` (static). ``import_role`` is a pre-processor, and the ``defaults`` and ``vars`` are evaluated at playbook parsing, making the variables available to tasks and roles listed at any point in the play. ``include_role`` is a conditional task, and the ``defaults`` and ``vars`` are evaluated at execution time, making the variables available to tasks and roles listed *after* the ``include_role`` task. + Deprecated ========== diff --git a/lib/ansible/modules/utilities/logic/import_role.py b/lib/ansible/modules/utilities/logic/import_role.py index 79dc0af361..e60577e128 100644 --- a/lib/ansible/modules/utilities/logic/import_role.py +++ b/lib/ansible/modules/utilities/logic/import_role.py @@ -56,6 +56,9 @@ options: default: 'no' notes: - Handlers are made available to the whole play. + - Variables defined in C(vars) and C(default) for the role are exposed at playbook parsing time. Due to this, + these variables will be accessible to roles and tasks executed before the the location of the C(import_role) task. + - Unlike C(include_role) variable exposure is not configurable, and will always be exposed. ''' EXAMPLES = """ diff --git a/lib/ansible/modules/utilities/logic/include_role.py b/lib/ansible/modules/utilities/logic/include_role.py index 010f597323..155ce10f00 100644 --- a/lib/ansible/modules/utilities/logic/include_role.py +++ b/lib/ansible/modules/utilities/logic/include_role.py @@ -55,8 +55,15 @@ options: description: - This option is a no op, and the functionality described in previous versions was not implemented. This option will be removed in Ansible v2.8. + public: + description: + - This option dictates whether the role's C(vars) and C(defaults) are exposed to the playbook. If set to C(yes) + the variables will be available to tasks following the C(include_role) task. This functionality differs from + standard variable exposure for roles listed under the C(roles) header or C(import_role) as they are exposed at + playbook parsing time, and available to earlier roles and tasks as well. type: bool default: 'no' + version_added: '2.7' 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/play.py b/lib/ansible/playbook/play.py index 7554b92b8e..8933a21fd7 100644 --- a/lib/ansible/playbook/play.py +++ b/lib/ansible/playbook/play.py @@ -233,6 +233,10 @@ class Play(Base, Taggable, Become): if len(self.roles) > 0: for r in self.roles: + # Don't insert tasks from ``import/include_role``, preventing + # duplicate execution at the wrong time + if r.from_include: + continue block_list.extend(r.compile(play=self)) return block_list diff --git a/lib/ansible/playbook/role/__init__.py b/lib/ansible/playbook/role/__init__.py index 18776a0953..51ee5a0b58 100644 --- a/lib/ansible/playbook/role/__init__.py +++ b/lib/ansible/playbook/role/__init__.py @@ -96,7 +96,7 @@ class Role(Base, Become, Conditional, Taggable): _delegate_to = FieldAttribute(isa='string') _delegate_facts = FieldAttribute(isa='bool', default=False) - def __init__(self, play=None, from_files=None): + def __init__(self, play=None, from_files=None, from_include=False): self._role_name = None self._role_path = None self._role_params = dict() @@ -108,6 +108,7 @@ class Role(Base, Become, Conditional, Taggable): self._dependencies = [] self._task_blocks = [] self._handler_blocks = [] + self._compiled_handler_blocks = None self._default_vars = dict() self._role_vars = dict() self._had_task_run = dict() @@ -117,6 +118,9 @@ class Role(Base, Become, Conditional, Taggable): from_files = {} self._from_files = from_files + # Indicates whether this role was included via include/import_role + self.from_include = from_include + super(Role, self).__init__() def __repr__(self): @@ -126,7 +130,7 @@ class Role(Base, Become, Conditional, Taggable): return self._role_name @staticmethod - def load(role_include, play, parent_role=None, from_files=None): + def load(role_include, play, parent_role=None, from_files=None, from_include=False): if from_files is None: from_files = {} @@ -153,7 +157,7 @@ class Role(Base, Become, Conditional, Taggable): role_obj.add_parent(parent_role) return role_obj - r = Role(play=play, from_files=from_files) + r = Role(play=play, from_files=from_files, from_include=from_include) r._load_role_data(role_include, parent_role=parent_role) if role_include.role not in play.ROLE_CACHE: @@ -359,7 +363,15 @@ class Role(Base, Become, Conditional, Taggable): return self._task_blocks[:] def get_handler_blocks(self, play, dep_chain=None): - block_list = [] + # Do not recreate this list each time ``get_handler_blocks`` is called. + # Cache the results so that we don't potentially overwrite with copied duplicates + # + # ``get_handler_blocks`` may be called when handling ``import_role`` during parsing + # as well as with ``Play.compile_roles_handlers`` from ``TaskExecutor`` + if self._compiled_handler_blocks: + return self._compiled_handler_blocks + + self._compiled_handler_blocks = block_list = [] # update the dependency chain here if dep_chain is None: diff --git a/lib/ansible/playbook/role_include.py b/lib/ansible/playbook/role_include.py index e924422ee9..37fc0ad683 100644 --- a/lib/ansible/playbook/role_include.py +++ b/lib/ansible/playbook/role_include.py @@ -46,7 +46,7 @@ class IncludeRole(TaskInclude): BASE = ('name', 'role') # directly assigned FROM_ARGS = ('tasks_from', 'vars_from', 'defaults_from') # used to populate from dict in role - OTHER_ARGS = ('apply', 'private', 'allow_duplicates') # assigned to matching property + OTHER_ARGS = ('apply', 'private', 'public', 'allow_duplicates') # assigned to matching property VALID_ARGS = tuple(frozenset(BASE + FROM_ARGS + OTHER_ARGS)) # all valid args # ================================================================================= @@ -55,6 +55,7 @@ class IncludeRole(TaskInclude): # private as this is a 'module options' vs a task property _allow_duplicates = FieldAttribute(isa='bool', default=True, private=True) _private = FieldAttribute(isa='bool', default=None, private=True) + _public = FieldAttribute(isa='bool', default=False, private=True) def __init__(self, block=None, role=None, task_include=None): @@ -81,9 +82,13 @@ class IncludeRole(TaskInclude): ri.vars.update(self.vars) # build role - actual_role = Role.load(ri, myplay, parent_role=self._parent_role, from_files=self._from_files) + actual_role = Role.load(ri, myplay, parent_role=self._parent_role, from_files=self._from_files, + from_include=True) actual_role._metadata.allow_duplicates = self.allow_duplicates + if self.statically_loaded or self.public: + myplay.roles.append(actual_role) + # save this for later use self._role_path = actual_role._role_path @@ -119,9 +124,12 @@ class IncludeRole(TaskInclude): if ir._role_name is None: raise AnsibleParserError("'name' is a required field for %s." % ir.action, obj=data) - if ir.private is not None: + if 'public' in ir.args and ir.action != 'include_role': + raise AnsibleParserError('Invalid options for %s: private' % ir.action, obj=data) + + if 'private' in ir.args: display.deprecated( - msg='Supplying "private" for "include_role" is a no op, and is deprecated', + msg='Supplying "private" for "%s" is a no op, and is deprecated' % ir.action, version='2.8' ) diff --git a/test/integration/targets/include_import/public_exposure/playbook.yml b/test/integration/targets/include_import/public_exposure/playbook.yml new file mode 100644 index 0000000000..11735e7731 --- /dev/null +++ b/test/integration/targets/include_import/public_exposure/playbook.yml @@ -0,0 +1,56 @@ +--- +- hosts: testhost + gather_facts: false + roles: + - regular + tasks: + - debug: + msg: start tasks + + - name: Static imports should expose vars at parse time, not at execution time + assert: + that: + - static_defaults_var == 'static_defaults' + - static_vars_var == 'static_vars' + - import_role: + name: static + - assert: + that: + - static_tasks_var == 'static_tasks' + - static_defaults_var == 'static_defaults' + - static_vars_var == 'static_vars' + + - include_role: + name: dynamic_private + - assert: + that: + - private_tasks_var == 'private_tasks' + - private_defaults_var is undefined + - private_vars_var is undefined + + - name: Dynamic include should not expose vars until execution time + assert: + that: + - dynamic_tasks_var is undefined + - dynamic_defaults_var is undefined + - dynamic_vars_var is undefined + - include_role: + name: dynamic + public: true + - assert: + that: + - dynamic_tasks_var == 'dynamic_tasks' + - dynamic_defaults_var == 'dynamic_defaults' + - dynamic_vars_var == 'dynamic_vars' + + - include_role: + name: from + public: true + tasks_from: from.yml + vars_from: from.yml + defaults_from: from.yml + - assert: + that: + - from_tasks_var == 'from_tasks' + - from_defaults_var == 'from_defaults' + - from_vars_var == 'from_vars' diff --git a/test/integration/targets/include_import/public_exposure/roles/dynamic/defaults/main.yml b/test/integration/targets/include_import/public_exposure/roles/dynamic/defaults/main.yml new file mode 100644 index 0000000000..099ac29b71 --- /dev/null +++ b/test/integration/targets/include_import/public_exposure/roles/dynamic/defaults/main.yml @@ -0,0 +1 @@ +dynamic_defaults_var: dynamic_defaults diff --git a/test/integration/targets/include_import/public_exposure/roles/dynamic/tasks/main.yml b/test/integration/targets/include_import/public_exposure/roles/dynamic/tasks/main.yml new file mode 100644 index 0000000000..e9b9ad3df7 --- /dev/null +++ b/test/integration/targets/include_import/public_exposure/roles/dynamic/tasks/main.yml @@ -0,0 +1,5 @@ +- debug: + msg: dynamic + +- set_fact: + dynamic_tasks_var: dynamic_tasks diff --git a/test/integration/targets/include_import/public_exposure/roles/dynamic/vars/main.yml b/test/integration/targets/include_import/public_exposure/roles/dynamic/vars/main.yml new file mode 100644 index 0000000000..b33c12df33 --- /dev/null +++ b/test/integration/targets/include_import/public_exposure/roles/dynamic/vars/main.yml @@ -0,0 +1 @@ +dynamic_vars_var: dynamic_vars diff --git a/test/integration/targets/include_import/public_exposure/roles/dynamic_private/defaults/main.yml b/test/integration/targets/include_import/public_exposure/roles/dynamic_private/defaults/main.yml new file mode 100644 index 0000000000..b19ef72c2d --- /dev/null +++ b/test/integration/targets/include_import/public_exposure/roles/dynamic_private/defaults/main.yml @@ -0,0 +1 @@ +private_defaults_var: private_defaults diff --git a/test/integration/targets/include_import/public_exposure/roles/dynamic_private/tasks/main.yml b/test/integration/targets/include_import/public_exposure/roles/dynamic_private/tasks/main.yml new file mode 100644 index 0000000000..1c7f653d41 --- /dev/null +++ b/test/integration/targets/include_import/public_exposure/roles/dynamic_private/tasks/main.yml @@ -0,0 +1,5 @@ +- debug: + msg: private + +- set_fact: + private_tasks_var: private_tasks diff --git a/test/integration/targets/include_import/public_exposure/roles/dynamic_private/vars/main.yml b/test/integration/targets/include_import/public_exposure/roles/dynamic_private/vars/main.yml new file mode 100644 index 0000000000..60f7ca8102 --- /dev/null +++ b/test/integration/targets/include_import/public_exposure/roles/dynamic_private/vars/main.yml @@ -0,0 +1 @@ +private_vars_var: private_vars diff --git a/test/integration/targets/include_import/public_exposure/roles/from/defaults/from.yml b/test/integration/targets/include_import/public_exposure/roles/from/defaults/from.yml new file mode 100644 index 0000000000..6729c4b480 --- /dev/null +++ b/test/integration/targets/include_import/public_exposure/roles/from/defaults/from.yml @@ -0,0 +1 @@ +from_defaults_var: from_defaults diff --git a/test/integration/targets/include_import/public_exposure/roles/from/tasks/from.yml b/test/integration/targets/include_import/public_exposure/roles/from/tasks/from.yml new file mode 100644 index 0000000000..932efc9f86 --- /dev/null +++ b/test/integration/targets/include_import/public_exposure/roles/from/tasks/from.yml @@ -0,0 +1,5 @@ +- debug: + msg: from + +- set_fact: + from_tasks_var: from_tasks diff --git a/test/integration/targets/include_import/public_exposure/roles/from/vars/from.yml b/test/integration/targets/include_import/public_exposure/roles/from/vars/from.yml new file mode 100644 index 0000000000..98b2ad4745 --- /dev/null +++ b/test/integration/targets/include_import/public_exposure/roles/from/vars/from.yml @@ -0,0 +1 @@ +from_vars_var: from_vars diff --git a/test/integration/targets/include_import/public_exposure/roles/regular/defaults/main.yml b/test/integration/targets/include_import/public_exposure/roles/regular/defaults/main.yml new file mode 100644 index 0000000000..21a6967c31 --- /dev/null +++ b/test/integration/targets/include_import/public_exposure/roles/regular/defaults/main.yml @@ -0,0 +1 @@ +regular_defaults_var: regular_defaults diff --git a/test/integration/targets/include_import/public_exposure/roles/regular/tasks/main.yml b/test/integration/targets/include_import/public_exposure/roles/regular/tasks/main.yml new file mode 100644 index 0000000000..eafa141a65 --- /dev/null +++ b/test/integration/targets/include_import/public_exposure/roles/regular/tasks/main.yml @@ -0,0 +1,5 @@ +- debug: + msg: regular + +- set_fact: + regular_tasks_var: regular_tasks diff --git a/test/integration/targets/include_import/public_exposure/roles/regular/vars/main.yml b/test/integration/targets/include_import/public_exposure/roles/regular/vars/main.yml new file mode 100644 index 0000000000..3d06546f7e --- /dev/null +++ b/test/integration/targets/include_import/public_exposure/roles/regular/vars/main.yml @@ -0,0 +1 @@ +regular_vars_var: regular_vars diff --git a/test/integration/targets/include_import/public_exposure/roles/static/defaults/main.yml b/test/integration/targets/include_import/public_exposure/roles/static/defaults/main.yml new file mode 100644 index 0000000000..d88f5559af --- /dev/null +++ b/test/integration/targets/include_import/public_exposure/roles/static/defaults/main.yml @@ -0,0 +1 @@ +static_defaults_var: static_defaults diff --git a/test/integration/targets/include_import/public_exposure/roles/static/tasks/main.yml b/test/integration/targets/include_import/public_exposure/roles/static/tasks/main.yml new file mode 100644 index 0000000000..5a6488c180 --- /dev/null +++ b/test/integration/targets/include_import/public_exposure/roles/static/tasks/main.yml @@ -0,0 +1,5 @@ +- debug: + msg: static + +- set_fact: + static_tasks_var: static_tasks diff --git a/test/integration/targets/include_import/public_exposure/roles/static/vars/main.yml b/test/integration/targets/include_import/public_exposure/roles/static/vars/main.yml new file mode 100644 index 0000000000..982e34d053 --- /dev/null +++ b/test/integration/targets/include_import/public_exposure/roles/static/vars/main.yml @@ -0,0 +1 @@ +static_vars_var: static_vars diff --git a/test/integration/targets/include_import/runme.sh b/test/integration/targets/include_import/runme.sh index c2b4abf4a7..cc3c1010b6 100755 --- a/test/integration/targets/include_import/runme.sh +++ b/test/integration/targets/include_import/runme.sh @@ -78,4 +78,6 @@ fi ANSIBLE_STRATEGY='linear' ansible-playbook tasks/test_include_dupe_loop.yml -i ../../inventory "$@" | tee test_include_dupe_loop.out test "$(grep -c '"item=foo"' test_include_dupe_loop.out)" = 3 ANSIBLE_STRATEGY='free' ansible-playbook tasks/test_include_dupe_loop.yml -i ../../inventory "$@" | tee test_include_dupe_loop.out -test "$(grep -c '"item=foo"' test_include_dupe_loop.out)" = 3
\ No newline at end of file +test "$(grep -c '"item=foo"' test_include_dupe_loop.out)" = 3 + +ansible-playbook public_exposure/playbook.yml -i ../../inventory "$@" |