From 2bb7feec6d223cae5a3dc20900e4e87f0074735b Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Tue, 28 Jun 2016 17:23:30 -0400 Subject: [PATCH] Search path (#16387) * smarter function to figure out relative paths takes list of paths in order of relevance to current task and does the dwim magic on them * shared function for action plugins using new dwim unify path construction and error info/messaging made include and role non exclusive corrected order and now smarter about tasks includes inside roles are currently broken as they don't provide the correct role data make dirname full match to avoid corner cases * migrated action plugins to new dwim function reported plugins to use exceptions instead of info * clarified needle --- lib/ansible/parsing/dataloader.py | 53 +++++++++++++++++++++- lib/ansible/plugins/action/__init__.py | 28 ++++++++++++ lib/ansible/plugins/action/assemble.py | 19 +++++--- lib/ansible/plugins/action/copy.py | 24 ++++------ lib/ansible/plugins/action/include_vars.py | 33 ++++++-------- lib/ansible/plugins/action/patch.py | 12 +++-- lib/ansible/plugins/action/script.py | 10 ++-- lib/ansible/plugins/action/template.py | 22 +++++---- lib/ansible/plugins/action/unarchive.py | 14 ++++-- 9 files changed, 150 insertions(+), 65 deletions(-) diff --git a/lib/ansible/parsing/dataloader.py b/lib/ansible/parsing/dataloader.py index b217e570aa8..1882e245855 100644 --- a/lib/ansible/parsing/dataloader.py +++ b/lib/ansible/parsing/dataloader.py @@ -38,6 +38,12 @@ from ansible.module_utils.basic import is_executable from ansible.utils.path import unfrackpath from ansible.utils.unicode import to_unicode, to_bytes +try: + from __main__ import display +except ImportError: + from ansible.utils.display import Display + display = Display() + class DataLoader(): ''' @@ -233,12 +239,11 @@ class DataLoader(): isrole = False # I have full path, nothing else needs to be looked at - if source.startswith('~') or source.startswith('/'): + if source.startswith('~') or source.startswith(os.path.sep): search.append(self.path_dwim(source)) else: # base role/play path + templates/files/vars + relative filename search.append(os.path.join(path, dirname, source)) - basedir = unfrackpath(path) # is it a role and if so make sure you get correct base path @@ -271,6 +276,50 @@ class DataLoader(): return candidate + def path_dwim_relative_stack(self, paths, dirname, source): + ''' + find one file in first path in stack taking roles into account and adding play basedir as fallback + ''' + result = None + if source.startswith('~') or source.startswith(os.path.sep): + # path is absolute, no relative needed, check existence and return source + test_path = to_bytes(unfrackpath(source),errors='strict') + if os.path.exists(test_path): + result = test_path + else: + search = [] + for path in paths: + upath = unfrackpath(path) + mydir = os.path.dirname(upath) + + # if path is in role and 'tasks' not there already, add it into the search + if upath.endswith('tasks') and os.path.exists(to_bytes(os.path.join(upath,'main.yml'), errors='strict')) \ + or os.path.exists(to_bytes(os.path.join(upath,'tasks/main.yml'), errors='strict')) \ + or os.path.exists(to_bytes(os.path.join(os.path.dirname(upath),'tasks/main.yml'), errors='strict')): + if mydir.endswith('tasks'): + search.append(os.path.join(os.path.dirname(mydir), dirname, source)) + search.append(os.path.join(mydir, source)) + else: + search.append(os.path.join(upath, dirname, source)) + search.append(os.path.join(upath, 'tasks', source)) + elif dirname not in source.split('/'): + # don't add dirname if user already is using it in source + search.append(os.path.join(upath, dirname, source)) + search.append(os.path.join(upath, source)) + + # always append basedir as last resort + search.append(os.path.join(self.get_basedir(), dirname, source)) + search.append(os.path.join(self.get_basedir(), source)) + + display.debug('search_path:\n\t' + '\n\t'.join(search)) + for candidate in search: + display.vvvvv('looking for "%s" at "%s"' % (source, candidate)) + if os.path.exists(to_bytes(candidate, errors='strict')): + result = candidate + break + + return result + def read_vault_password_file(self, vault_password_file): """ Read a vault password from a file or if executable, execute the script and diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 09571471458..7ad5c76db85 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -823,3 +823,31 @@ class ActionBase(with_metaclass(ABCMeta, object)): diff["after"] = " [[ Diff output has been hidden because 'no_log: true' was specified for this result ]]" return diff + + def _find_needle(self, dirname, needle): + ''' + find a needle in haystack of paths, optionally using 'dirname' as a subdir. + This will build the ordered list of paths to search and pass them to dwim + to get back the first existing file found. + ''' + + path_stack = [] + + dep_chain = self._task._block.get_dep_chain() + # inside role: add the dependency chain + if dep_chain: + path_stack.extend(reversed([x._role_path for x in dep_chain])) + + + task_dir = os.path.dirname(self._task.get_path()) + # include from diff directory: add it to file path + if not task_dir.endswith('tasks') and task_dir != self._loader.get_basedir(): + path_stack.append(task_dir) + + result = self._loader.path_dwim_relative_stack(path_stack, dirname, needle) + + if result is None: + raise AnsibleError("Unable to find '%s' in expected paths." % needle) + + return result + diff --git a/lib/ansible/plugins/action/assemble.py b/lib/ansible/plugins/action/assemble.py index 5cdfc7cbfb5..41c06135951 100644 --- a/lib/ansible/plugins/action/assemble.py +++ b/lib/ansible/plugins/action/assemble.py @@ -23,9 +23,11 @@ import os.path import tempfile import re +from ansible.errors import AnsibleError from ansible.plugins.action import ActionBase from ansible.utils.boolean import boolean from ansible.utils.hashing import checksum_s +from ansible.utils.unicode import to_str class ActionModule(ActionBase): @@ -106,20 +108,23 @@ class ActionModule(ActionBase): result.update(self._execute_module(tmp=tmp, task_vars=task_vars, delete_remote_tmp=False)) self._remove_tmp_path(tmp) return result - elif self._task._role is not None: - src = self._loader.path_dwim_relative(self._task._role._role_path, 'files', src) else: - src = self._loader.path_dwim_relative(self._loader.get_basedir(), 'files', src) - - _re = None - if regexp is not None: - _re = re.compile(regexp) + try: + src = self._find_needle('files', src) + except AnsibleError as e: + result['failed'] = True + result['msg'] = to_str(e) + return result if not os.path.isdir(src): result['failed'] = True result['msg'] = "Source (%s) is not a directory" % src return result + _re = None + if regexp is not None: + _re = re.compile(regexp) + # Does all work assembling the file path = self._assemble_from_fragments(src, delimiter, _re, ignore_hidden) diff --git a/lib/ansible/plugins/action/copy.py b/lib/ansible/plugins/action/copy.py index 4e7cc263064..43a5c6633db 100644 --- a/lib/ansible/plugins/action/copy.py +++ b/lib/ansible/plugins/action/copy.py @@ -23,11 +23,11 @@ import json import os import tempfile -from ansible import constants as C +from ansible.errors import AnsibleError from ansible.plugins.action import ActionBase from ansible.utils.boolean import boolean from ansible.utils.hashing import checksum -from ansible.utils.unicode import to_bytes +from ansible.utils.unicode import to_bytes, to_str class ActionModule(ActionBase): @@ -81,27 +81,23 @@ class ActionModule(ActionBase): source = content_tempfile except Exception as err: result['failed'] = True - result['msg'] = "could not write content temp file: %s" % err + result['msg'] = "could not write content temp file: %s" % to_str(err) return result # if we have first_available_file in our vars # look up the files and use the first one we find as src elif faf: source = self._get_first_available_file(faf, task_vars.get('_original_file', None)) - if source is None: - result['failed'] = True - result['msg'] = "could not find src in first_available_file list" - return result - elif remote_src: result.update(self._execute_module(module_name='copy', module_args=self._task.args, task_vars=task_vars, delete_remote_tmp=False)) return result - - else: - if self._task._role is not None: - source = self._loader.path_dwim_relative(self._task._role._role_path, 'files', source) - else: - source = self._loader.path_dwim_relative(self._loader.get_basedir(), 'files', source) + else: # find in expected paths + try: + source = self._find_needle('files', source) + except AnsibleError as e: + result['failed'] = True + result['msg'] = to_str(e) + return result # A list of source file tuples (full_path, relative_path) which will try to copy to the destination source_files = [] diff --git a/lib/ansible/plugins/action/include_vars.py b/lib/ansible/plugins/action/include_vars.py index cada2f53a03..c7283d14f7a 100644 --- a/lib/ansible/plugins/action/include_vars.py +++ b/lib/ansible/plugins/action/include_vars.py @@ -17,11 +17,9 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type -import os - from ansible.errors import AnsibleError from ansible.plugins.action import ActionBase - +from ansible.utils.unicode import to_str class ActionModule(ActionBase): @@ -33,25 +31,22 @@ class ActionModule(ActionBase): result = super(ActionModule, self).run(tmp, task_vars) - source = self._task.args.get('_raw_params') + try: + source = self._find_needle('vars', self._task.args.get('_raw_params')) + except AnsibleError as e: + result['failed'] = True + result['message'] = to_str(e) + return result - if self._task._role: - source = self._loader.path_dwim_relative(self._task._role._role_path, 'vars', source) + (data, show_content) = self._loader._get_file_contents(source) + data = self._loader.load(data, show_content) + if data is None: + data = {} + if not isinstance(data, dict): + result['failed'] = True + result['message'] = "%s must be stored as a dictionary/hash" % source else: - source = self._loader.path_dwim_relative(self._loader.get_basedir(), 'vars', source) - - if os.path.exists(source): - (data, show_content) = self._loader._get_file_contents(source) - data = self._loader.load(data, show_content) - if data is None: - data = {} - if not isinstance(data, dict): - raise AnsibleError("%s must be stored as a dictionary/hash" % source) result['ansible_facts'] = data result['_ansible_no_log'] = not show_content - else: - result['failed'] = True - result['msg'] = "Source file not found." - result['file'] = source return result diff --git a/lib/ansible/plugins/action/patch.py b/lib/ansible/plugins/action/patch.py index fa7a3f514ab..7f388fce8f9 100644 --- a/lib/ansible/plugins/action/patch.py +++ b/lib/ansible/plugins/action/patch.py @@ -22,6 +22,8 @@ import os from ansible.plugins.action import ActionBase from ansible.utils.boolean import boolean +from ansible.errors import AnsibleError +from ansible.utils.unicode import to_str class ActionModule(ActionBase): @@ -46,10 +48,12 @@ class ActionModule(ActionBase): result.update(self._execute_module(task_vars=task_vars)) return result - if self._task._role is not None: - src = self._loader.path_dwim_relative(self._task._role._role_path, 'files', src) - else: - src = self._loader.path_dwim_relative(self._loader.get_basedir(), 'files', src) + try: + src = self._find_needle('files', src) + except AnsibleError as e: + result['failed'] = True + result['msg'] = to_str(e) + return result # create the remote tmp dir if needed, and put the source file there if tmp is None or "-tmp-" not in tmp: diff --git a/lib/ansible/plugins/action/script.py b/lib/ansible/plugins/action/script.py index 358775c19e2..b468c8df0f8 100644 --- a/lib/ansible/plugins/action/script.py +++ b/lib/ansible/plugins/action/script.py @@ -20,6 +20,8 @@ __metaclass__ = type import os from ansible.plugins.action import ActionBase +from ansible.errors import AnsibleError +from ansible.utils.unicode import to_str class ActionModule(ActionBase): @@ -69,10 +71,10 @@ class ActionModule(ActionBase): source = parts[0] args = ' '.join(parts[1:]) - if self._task._role is not None: - source = self._loader.path_dwim_relative(self._task._role._role_path, 'files', source) - else: - source = self._loader.path_dwim_relative(self._loader.get_basedir(), 'files', source) + try: + source = self._find_needle('files', source) + except AnsibleError as e: + return dict(failed=True, msg=to_str(e)) # transfer the file to a remote tmp location tmp_src = self._connection._shell.join_path(tmp, os.path.basename(source)) diff --git a/lib/ansible/plugins/action/template.py b/lib/ansible/plugins/action/template.py index 9f033e29405..88ccdcd2a51 100644 --- a/lib/ansible/plugins/action/template.py +++ b/lib/ansible/plugins/action/template.py @@ -26,7 +26,9 @@ from ansible import constants as C from ansible.plugins.action import ActionBase from ansible.utils.hashing import checksum_s from ansible.utils.boolean import boolean -from ansible.utils.unicode import to_bytes, to_unicode +from ansible.utils.unicode import to_bytes, to_unicode, to_str +from ansible.errors import AnsibleError + class ActionModule(ActionBase): @@ -63,23 +65,23 @@ class ActionModule(ActionBase): if state is not None: result['failed'] = True result['msg'] = "'state' cannot be specified on a template" - return result elif (source is None and faf is not None) or dest is None: result['failed'] = True result['msg'] = "src and dest are required" - return result - - if faf: + elif faf: source = self._get_first_available_file(faf, task_vars.get('_original_file', None, 'templates')) if source is None: result['failed'] = True result['msg'] = "could not find src in first_available_file list" - return result else: - if self._task._role is not None: - source = self._loader.path_dwim_relative(self._task._role._role_path, 'templates', source) - else: - source = self._loader.path_dwim_relative(self._loader.get_basedir(), 'templates', source) + try: + source = self._find_needle('templates', source) + except AnsibleError as e: + result['failed'] = True + result['msg'] = to_str(e) + + if 'failed' in result: + return result # Expand any user home dir specification dest = self._remote_expand_user(dest) diff --git a/lib/ansible/plugins/action/unarchive.py b/lib/ansible/plugins/action/unarchive.py index 5b8d0499857..307e732089c 100644 --- a/lib/ansible/plugins/action/unarchive.py +++ b/lib/ansible/plugins/action/unarchive.py @@ -22,7 +22,8 @@ import os from ansible.plugins.action import ActionBase from ansible.utils.boolean import boolean - +from ansible.errors import AnsibleError +from ansible.utils.unicode import to_str class ActionModule(ActionBase): @@ -66,10 +67,13 @@ class ActionModule(ActionBase): source = os.path.expanduser(source) if copy: - if self._task._role is not None: - source = self._loader.path_dwim_relative(self._task._role._role_path, 'files', source) - else: - source = self._loader.path_dwim_relative(self._loader.get_basedir(), 'files', source) + try: + source = self._find_needle('files', source) + except AnsibleError as e: + result['failed'] = True + result['msg'] = to_str(e) + self._remove_tmp_path(tmp) + return result remote_checksum = self._remote_checksum(dest, all_vars=task_vars, follow=True) if remote_checksum == '4':