From 354aa8d602e95037ce647675a6fc47fa9196304f Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Mon, 16 Apr 2018 16:46:47 -0500 Subject: [PATCH] Skip self._parent on dynamic, defer to grandparent for attr lookup (#38827) * Skip self._parent on dynamic, defer to grandparent for attr lookup * Revert _inheritable * Add tests for include inheritance from static blocks Fixes #38037 #36194 --- lib/ansible/playbook/base.py | 2 -- lib/ansible/playbook/block.py | 17 +++++++---- lib/ansible/playbook/role_include.py | 2 -- lib/ansible/playbook/task.py | 17 +++++++---- lib/ansible/playbook/task_include.py | 2 -- .../grandchild/block_include_tasks.yml | 2 ++ .../include_import/grandchild/import.yml | 1 + .../import_include_include_tasks.yml | 2 ++ .../grandchild/include_level_1.yml | 1 + .../targets/include_import/runme.sh | 4 +++ .../test_grandparent_inheritance.yml | 29 +++++++++++++++++++ 11 files changed, 63 insertions(+), 16 deletions(-) create mode 100644 test/integration/targets/include_import/grandchild/block_include_tasks.yml create mode 100644 test/integration/targets/include_import/grandchild/import.yml create mode 100644 test/integration/targets/include_import/grandchild/import_include_include_tasks.yml create mode 100644 test/integration/targets/include_import/grandchild/include_level_1.yml create mode 100644 test/integration/targets/include_import/test_grandparent_inheritance.yml diff --git a/lib/ansible/playbook/base.py b/lib/ansible/playbook/base.py index 9662e913b50..d007dc09326 100644 --- a/lib/ansible/playbook/base.py +++ b/lib/ansible/playbook/base.py @@ -174,8 +174,6 @@ class Base(with_metaclass(BaseMeta, object)): 'su', 'su_user', 'su_pass', 'su_exe', 'su_flags', ] - _inheritable = True - def __init__(self): # initialize the data loader and variable manager, which will be provided diff --git a/lib/ansible/playbook/block.py b/lib/ansible/playbook/block.py index d8a0087fcb1..9bbe8883859 100644 --- a/lib/ansible/playbook/block.py +++ b/lib/ansible/playbook/block.py @@ -298,13 +298,20 @@ class Block(Base, Become, Conditional, Taggable): prepend = self._valid_attrs[attr].prepend try: value = self._attributes[attr] - if self._parent and (value is None or extend): + # If parent is static, we can grab attrs from the parent + # otherwise, defer to the grandparent + if getattr(self._parent, 'statically_loaded', True): + _parent = self._parent + else: + _parent = self._parent._parent + + if _parent and (value is None or extend): try: - if getattr(self._parent, 'statically_loaded', True): - if hasattr(self._parent, '_get_parent_attribute'): - parent_value = self._parent._get_parent_attribute(attr) + if getattr(_parent, 'statically_loaded', True): + if hasattr(_parent, '_get_parent_attribute'): + parent_value = _parent._get_parent_attribute(attr) else: - parent_value = self._parent._attributes.get(attr, None) + parent_value = _parent._attributes.get(attr, None) if extend: value = self._extend_value(value, parent_value, prepend) else: diff --git a/lib/ansible/playbook/role_include.py b/lib/ansible/playbook/role_include.py index 423fcdd4e63..f2d50c49d3d 100644 --- a/lib/ansible/playbook/role_include.py +++ b/lib/ansible/playbook/role_include.py @@ -48,8 +48,6 @@ class IncludeRole(TaskInclude): OTHER_ARGS = ('private', 'allow_duplicates') # assigned to matching property VALID_ARGS = tuple(frozenset(BASE + FROM_ARGS + OTHER_ARGS)) # all valid args - _inheritable = False - # ================================================================================= # ATTRIBUTES diff --git a/lib/ansible/playbook/task.py b/lib/ansible/playbook/task.py index 226380c3c0b..3956a5835a7 100644 --- a/lib/ansible/playbook/task.py +++ b/lib/ansible/playbook/task.py @@ -423,13 +423,20 @@ class Task(Base, Conditional, Taggable, Become): prepend = self._valid_attrs[attr].prepend try: value = self._attributes[attr] - if self._parent and (value is None or extend): - if getattr(self._parent, 'statically_loaded', True): + # If parent is static, we can grab attrs from the parent + # otherwise, defer to the grandparent + if getattr(self._parent, 'statically_loaded', True): + _parent = self._parent + else: + _parent = self._parent._parent + + if _parent and (value is None or extend): + if getattr(_parent, 'statically_loaded', True): # vars are always inheritable, other attributes might not be for the partent but still should be for other ancestors - if attr != 'vars' and getattr(self._parent, '_inheritable', True) and hasattr(self._parent, '_get_parent_attribute'): - parent_value = self._parent._get_parent_attribute(attr) + if attr != 'vars' and hasattr(_parent, '_get_parent_attribute'): + parent_value = _parent._get_parent_attribute(attr) else: - parent_value = self._parent._attributes.get(attr, None) + parent_value = _parent._attributes.get(attr, None) if extend: value = self._extend_value(value, parent_value, prepend) diff --git a/lib/ansible/playbook/task_include.py b/lib/ansible/playbook/task_include.py index 55da790b670..f8612a02d30 100644 --- a/lib/ansible/playbook/task_include.py +++ b/lib/ansible/playbook/task_include.py @@ -50,8 +50,6 @@ class TaskInclude(Task): @staticmethod def load(data, block=None, role=None, task_include=None, variable_manager=None, loader=None): t = TaskInclude(block=block, role=role, task_include=task_include) - if t.action == 'include_task': - t._inheritable = False return t.load_data(data, variable_manager=variable_manager, loader=loader) def copy(self, exclude_parent=False, exclude_tasks=False): diff --git a/test/integration/targets/include_import/grandchild/block_include_tasks.yml b/test/integration/targets/include_import/grandchild/block_include_tasks.yml new file mode 100644 index 00000000000..f8addcf46a4 --- /dev/null +++ b/test/integration/targets/include_import/grandchild/block_include_tasks.yml @@ -0,0 +1,2 @@ +- command: "true" + register: block_include_result diff --git a/test/integration/targets/include_import/grandchild/import.yml b/test/integration/targets/include_import/grandchild/import.yml new file mode 100644 index 00000000000..ef6990e2af1 --- /dev/null +++ b/test/integration/targets/include_import/grandchild/import.yml @@ -0,0 +1 @@ +- include_tasks: include_level_1.yml diff --git a/test/integration/targets/include_import/grandchild/import_include_include_tasks.yml b/test/integration/targets/include_import/grandchild/import_include_include_tasks.yml new file mode 100644 index 00000000000..dae3a245488 --- /dev/null +++ b/test/integration/targets/include_import/grandchild/import_include_include_tasks.yml @@ -0,0 +1,2 @@ +- command: "true" + register: import_include_include_result diff --git a/test/integration/targets/include_import/grandchild/include_level_1.yml b/test/integration/targets/include_import/grandchild/include_level_1.yml new file mode 100644 index 00000000000..e323511f83d --- /dev/null +++ b/test/integration/targets/include_import/grandchild/include_level_1.yml @@ -0,0 +1 @@ +- include_tasks: import_include_include_tasks.yml diff --git a/test/integration/targets/include_import/runme.sh b/test/integration/targets/include_import/runme.sh index 0b907f6e877..8c989518586 100755 --- a/test/integration/targets/include_import/runme.sh +++ b/test/integration/targets/include_import/runme.sh @@ -55,3 +55,7 @@ gen_task_files ANSIBLE_STRATEGY='linear' ansible-playbook test_copious_include_tasks.yml -i ../../inventory "$@" --skip-tags never ANSIBLE_STRATEGY='free' ansible-playbook test_copious_include_tasks.yml -i ../../inventory "$@" --skip-tags never rm -f tasks/hello/*.yml + +# Inlcuded tasks should inherit attrs from non-dynamic blocks in parent chain +# https://github.com/ansible/ansible/pull/38827 +ANSIBLE_STRATEGY='linear' ansible-playbook test_grandparent_inheritance.yml -i ../../inventory "$@" diff --git a/test/integration/targets/include_import/test_grandparent_inheritance.yml b/test/integration/targets/include_import/test_grandparent_inheritance.yml new file mode 100644 index 00000000000..45a3d83655e --- /dev/null +++ b/test/integration/targets/include_import/test_grandparent_inheritance.yml @@ -0,0 +1,29 @@ +--- +- hosts: testhost + gather_facts: false + tasks: + - debug: + var: inventory_hostname + + - name: Test included tasks inherit from block + check_mode: true + block: + - include_tasks: grandchild/block_include_tasks.yml + + - debug: + var: block_include_result + + - assert: + that: + - block_include_result is skipped + + - name: Test included tasks inherit deeply from import + import_tasks: grandchild/import.yml + check_mode: true + + - debug: + var: import_include_include_result + + - assert: + that: + - import_include_include_result is skipped