From da3fd2d5880c813e412f509fe909c2094eb6b0cb Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Thu, 11 Aug 2016 12:23:20 -0500 Subject: [PATCH] Several fixes for includes * when including statically, make sure that all parents were also included statically (issue #16990) * properly resolve nested static include paths * print a message when a file is statically included Fixes #16990 (cherry picked from commit 1c7e0c73c94a800a189f3504738a43c1e38e09bd) --- lib/ansible/playbook/block.py | 21 ++++++++++ lib/ansible/playbook/helpers.py | 62 ++++++++++++++++------------ lib/ansible/playbook/task.py | 39 +++++++++++++++++ lib/ansible/playbook/task_include.py | 9 ++++ 4 files changed, 104 insertions(+), 27 deletions(-) diff --git a/lib/ansible/playbook/block.py b/lib/ansible/playbook/block.py index 392b00b2948..9896c7b5362 100644 --- a/lib/ansible/playbook/block.py +++ b/lib/ansible/playbook/block.py @@ -383,3 +383,24 @@ class Block(Base, Become, Conditional, Taggable): def has_tasks(self): return len(self.block) > 0 or len(self.rescue) > 0 or len(self.always) > 0 + def get_include_params(self): + if self._parent: + return self._parent.get_include_params() + else: + return dict() + + def all_parents_static(self): + ''' + Determine if all of the parents of this block were statically loaded + or not. Since Task/TaskInclude objects may be in the chain, they simply + call their parents all_parents_static() method. Only Block objects in + the chain check the statically_loaded value of the parent. + ''' + from ansible.playbook.task_include import TaskInclude + if self._task_include and not self._task_include.statically_loaded: + return False + elif self._parent_block: + return self._parent_block.all_parents_static() + + return True + diff --git a/lib/ansible/playbook/helpers.py b/lib/ansible/playbook/helpers.py index 4d14076643d..412c746d6e4 100644 --- a/lib/ansible/playbook/helpers.py +++ b/lib/ansible/playbook/helpers.py @@ -119,38 +119,41 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h else: is_static = C.DEFAULT_TASK_INCLUDES_STATIC or \ (use_handlers and C.DEFAULT_HANDLER_INCLUDES_STATIC) or \ - (not templar._contains_vars(t.args['_raw_params']) and not t.loop) + (not templar._contains_vars(t.args['_raw_params']) and t.all_parents_static() and not t.loop) if is_static: if t.loop is not None: raise AnsibleParserError("You cannot use 'static' on an include with a loop", obj=task_ds) - # FIXME: all of this code is very similar (if not identical) to that in - # plugins/strategy/__init__.py, and should be unified to avoid - # patches only being applied to one or the other location - if task_include: - # handle relative includes by walking up the list of parent include - # tasks and checking the relative result to see if it exists - parent_include = task_include - cumulative_path = None - while parent_include is not None: - parent_include_dir = templar.template(os.path.dirname(parent_include.args.get('_raw_params'))) - if cumulative_path is None: - cumulative_path = parent_include_dir - elif not os.path.isabs(cumulative_path): - cumulative_path = os.path.join(parent_include_dir, cumulative_path) - include_target = templar.template(t.args['_raw_params']) - if t._role: - new_basedir = os.path.join(t._role._role_path, 'tasks', cumulative_path) - include_file = loader.path_dwim_relative(new_basedir, 'tasks', include_target) - else: - include_file = loader.path_dwim_relative(loader.get_basedir(), cumulative_path, include_target) + # we set a flag to indicate this include was static + t.statically_loaded = True - if os.path.exists(include_file): - break - else: - parent_include = parent_include._task_include - else: + # handle relative includes by walking up the list of parent include + # tasks and checking the relative result to see if it exists + parent_include = task_include + cumulative_path = None + + found = False + while parent_include is not None: + parent_include_dir = templar.template(os.path.dirname(parent_include.args.get('_raw_params'))) + if cumulative_path is None: + cumulative_path = parent_include_dir + elif not os.path.isabs(cumulative_path): + cumulative_path = os.path.join(parent_include_dir, cumulative_path) + include_target = templar.template(t.args['_raw_params']) + if t._role: + new_basedir = os.path.join(t._role._role_path, 'tasks', cumulative_path) + include_file = loader.path_dwim_relative(new_basedir, 'tasks', include_target) + else: + include_file = loader.path_dwim_relative(loader.get_basedir(), cumulative_path, include_target) + + if os.path.exists(include_file): + found = True + break + else: + parent_include = parent_include._task_include + + if not found: try: include_target = templar.template(t.args['_raw_params']) except AnsibleUndefinedVariable as e: @@ -176,6 +179,12 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h return [] elif not isinstance(data, list): raise AnsibleError("included task files must contain a list of tasks", obj=data) + + # since we can't send callbacks here, we display a message directly in + # the same fashion used by the on_include callback. We also do it here, + # because the recursive nature of helper methods means we may be loading + # nested includes, and we want the include order printed correctly + display.display("statically included: %s" % include_file, color=C.COLOR_SKIP) except AnsibleFileNotFound as e: if t.static or \ C.DEFAULT_TASK_INCLUDES_STATIC or \ @@ -227,7 +236,6 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h b.tags = list(set(b.tags).union(tags)) # END FIXME - # FIXME: send callback here somehow... # FIXME: handlers shouldn't need this special handling, but do # right now because they don't iterate blocks correctly if use_handlers: diff --git a/lib/ansible/playbook/task.py b/lib/ansible/playbook/task.py index 88afa6e8c41..92f559784ab 100644 --- a/lib/ansible/playbook/task.py +++ b/lib/ansible/playbook/task.py @@ -447,3 +447,42 @@ class Task(Base, Conditional, Taggable, Become): ''' return self._get_parent_attribute('any_errors_fatal') + def _get_attr_loop(self): + return self._attributes['loop'] + + def _get_attr_loop_control(self): + return self._attributes['loop_control'] + + def get_dep_chain(self): + if self._parent: + return self._parent.get_dep_chain() + else: + return None + + def get_search_path(self): + ''' + Return the list of paths you should search for files, in order. + This follows role/playbook dependency chain. + ''' + path_stack = [] + + dep_chain = self.get_dep_chain() + # inside role: add the dependency chain from current to dependant + if dep_chain: + path_stack.extend(reversed([x._role_path for x in dep_chain])) + + # add path of task itself, unless it is already in the list + task_dir = os.path.dirname(self.get_path()) + if task_dir not in path_stack: + path_stack.append(task_dir) + + return path_stack + + def all_parents_static(self): + if self._task_include and not self._task_include.statically_loaded: + return False + elif self._block: + return self._block.all_parents_static() + + return True + diff --git a/lib/ansible/playbook/task_include.py b/lib/ansible/playbook/task_include.py index 14fe36c3a18..68b125f311d 100644 --- a/lib/ansible/playbook/task_include.py +++ b/lib/ansible/playbook/task_include.py @@ -43,11 +43,20 @@ class TaskInclude(Task): _static = FieldAttribute(isa='bool', default=None) + def __init__(self, block=None, role=None, task_include=None): + super(TaskInclude, self).__init__(block=block, role=role, task_include=task_include) + self.statically_loaded = False + @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) return t.load_data(data, variable_manager=variable_manager, loader=loader) + def copy(self, exclude_block=False): + new_me = super(TaskInclude, self).copy(exclude_block=exclude_block) + new_me.statically_loaded = self.statically_loaded + return new_me + def get_vars(self): ''' We override the parent Task() classes get_vars here because