From 18e2ee16ef0895831ead312550eb5de44c99524c Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Thu, 10 Sep 2015 12:55:59 -0700 Subject: [PATCH] Fix for user defined modules not overriding modules from core. This fix takes into account that powershell modules are somewhat different than regular modules and have to be kept separate. --- lib/ansible/plugins/__init__.py | 142 ++++++++++++++------- lib/ansible/plugins/action/__init__.py | 40 +++--- lib/ansible/plugins/connection/__init__.py | 4 + lib/ansible/plugins/connection/winrm.py | 3 +- 4 files changed, 128 insertions(+), 61 deletions(-) diff --git a/lib/ansible/plugins/__init__.py b/lib/ansible/plugins/__init__.py index 06f2261138..9b0471a216 100644 --- a/lib/ansible/plugins/__init__.py +++ b/lib/ansible/plugins/__init__.py @@ -26,10 +26,9 @@ import inspect import os import os.path import sys +from collections import defaultdict from ansible import constants as C -from ansible.utils.unicode import to_unicode -from ansible import errors try: from __main__ import display @@ -37,6 +36,7 @@ except ImportError: from ansible.utils.display import Display display = Display() +# Global so that all instances of a PluginLoader will share the caches MODULE_CACHE = {} PATH_CACHE = {} PLUGIN_PATH_CACHE = {} @@ -68,7 +68,7 @@ class PluginLoader: if not class_name in PATH_CACHE: PATH_CACHE[class_name] = None if not class_name in PLUGIN_PATH_CACHE: - PLUGIN_PATH_CACHE[class_name] = {} + PLUGIN_PATH_CACHE[class_name] = defaultdict(dict) self._module_cache = MODULE_CACHE[class_name] self._paths = PATH_CACHE[class_name] @@ -169,9 +169,30 @@ class PluginLoader: # look for any plugins installed in the package subtree ret.extend(self._get_package_paths()) + # HACK: because powershell modules are in the same directory + # hierarchy as other modules we have to process them last. This is + # because powershell only works on windows but the other modules work + # anywhere (possibly including windows if the correct language + # interpreter is installed). the non-powershell modules can have any + # file extension and thus powershell modules are picked up in that. + # The non-hack way to fix this is to have powershell modules be + # a different PluginLoader/ModuleLoader. But that requires changing + # other things too (known thing to change would be PATHS_CACHE, + # PLUGIN_PATHS_CACHE, and MODULE_CACHE. Since those three dicts key + # on the class_name and neither regular modules nor powershell modules + # would have class_names, they would not work as written. + reordered_paths = [] + win_dirs = [] + for path in ret: + if path.endswith('windows'): + win_dirs.append(path) + else: + reordered_paths.append(path) + reordered_paths.extend(win_dirs) + # cache and return the result - self._paths = ret - return ret + self._paths = reordered_paths + return reordered_paths def add_directory(self, directory, with_subdir=False): @@ -187,55 +208,90 @@ class PluginLoader: self._extra_dirs.append(directory) self._paths = None - def find_plugin(self, name, suffixes=None): + def find_plugin(self, name, mod_type=''): ''' Find a plugin named name ''' - if not suffixes: - if self.class_name: - suffixes = ['.py'] - else: - suffixes = ['.py', ''] + # The particular cache to look for modules within. This matches the + # requested mod_type + pull_cache = self._plugin_path_cache[mod_type] + try: + return pull_cache[name] + except KeyError: + # Cache miss. Now let's find the plugin + pass - potential_names = frozenset('%s%s' % (name, s) for s in suffixes) - for full_name in potential_names: - if full_name in self._plugin_path_cache: - return self._plugin_path_cache[full_name] + if mod_type: + suffix = mod_type + elif self.class_name: + # Ansible plugins that run in the controller process (most plugins) + suffix = '.py' + else: + # Only Ansible Modules. Ansible modules can be any executable so + # they can have any suffix + suffix = '' - found = None - for path in [p for p in self._get_paths() if p not in self._searched_paths]: - if os.path.isdir(path): + ### FIXME: + # Instead of using the self._paths cache (PATH_CACHE) and + # self._searched_paths we could use an iterator. Before enabling that + # we need to make sure we don't want to add additional directories + # (add_directory()) once we start using the iterator. Currently, it + # looks like _get_paths() never forces a cache refresh so if we expect + # additional directories to be added later, it is buggy. + for path in (p for p in self._get_paths() if p not in self._searched_paths and os.path.isdir(p)): + try: + full_paths = (os.path.join(path, f) for f in os.listdir(path)) + except OSError as e: + display.warning("Error accessing plugin paths: %s" % str(e)) + + for full_path in (f for f in full_paths if os.path.isfile(f) and not f.endswith('__init__.py')): + full_name = os.path.basename(full_path) + + # HACK: We have no way of executing python byte + # compiled files as ansible modules so specifically exclude them + if full_path.endswith(('.pyc', '.pyo')): + continue + + splitname = os.path.splitext(full_name) + base_name = splitname[0] try: - full_paths = (os.path.join(path, f) for f in os.listdir(path)) - except OSError as e: - display.warning("Error accessing plugin paths: %s" % str(e)) - for full_path in (f for f in full_paths if os.path.isfile(f)): - for suffix in suffixes: - if full_path.endswith(suffix): - full_name = os.path.basename(full_path) - break - else: # Yes, this is a for-else: http://bit.ly/1ElPkyg - continue + extension = splitname[1] + except IndexError: + extension = '' - if full_name not in self._plugin_path_cache: - self._plugin_path_cache[full_name] = full_path + # Module found, now enter it into the caches that match + # this file + if base_name not in self._plugin_path_cache['']: + self._plugin_path_cache[''][base_name] = full_path + + if full_name not in self._plugin_path_cache['']: + self._plugin_path_cache[''][full_name] = full_path + + if base_name not in self._plugin_path_cache[extension]: + self._plugin_path_cache[extension][base_name] = full_path + + if full_name not in self._plugin_path_cache[extension]: + self._plugin_path_cache[extension][full_name] = full_path self._searched_paths.add(path) - for full_name in potential_names: - if full_name in self._plugin_path_cache: - return self._plugin_path_cache[full_name] + try: + return pull_cache[name] + except KeyError: + # Didn't find the plugin in this directory. Load modules from + # the next one + pass # if nothing is found, try finding alias/deprecated if not name.startswith('_'): - for alias_name in ('_%s' % n for n in potential_names): - # We've already cached all the paths at this point - if alias_name in self._plugin_path_cache: - if not os.path.islink(self._plugin_path_cache[alias_name]): - display.deprecated('%s is kept for backwards compatibility ' - 'but usage is discouraged. The module ' - 'documentation details page may explain ' - 'more about this rationale.' % - name.lstrip('_')) - return self._plugin_path_cache[alias_name] + alias_name = '_' + name + # We've already cached all the paths at this point + if alias_name in pull_cache: + if not os.path.islink(pull_cache[alias_name]): + display.deprecated('%s is kept for backwards compatibility ' + 'but usage is discouraged. The module ' + 'documentation details page may explain ' + 'more about this rationale.' % + name.lstrip('_')) + return pull_cache[alias_name] return None diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 335bcc80eb..744eea7f8b 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -69,30 +69,36 @@ class ActionBase: ''' # Search module path(s) for named module. - module_suffixes = getattr(self._connection, 'default_suffixes', None) + for mod_type in self._connection.module_implementation_preferences: + # Check to determine if PowerShell modules are supported, and apply + # some fixes (hacks) to module name + args. + if mod_type == '.ps1': + # win_stat, win_file, and win_copy are not just like their + # python counterparts but they are compatible enough for our + # internal usage + if module_name in ('stat', 'file', 'copy') and self._task.action != module_name: + module_name = 'win_%s' % module_name - # Check to determine if PowerShell modules are supported, and apply - # some fixes (hacks) to module name + args. - if module_suffixes and '.ps1' in module_suffixes: - # Use Windows versions of stat/file/copy modules when called from - # within other action plugins. - if module_name in ('stat', 'file', 'copy') and self._task.action != module_name: - module_name = 'win_%s' % module_name - # Remove extra quotes surrounding path parameters before sending to module. - if module_name in ('win_stat', 'win_file', 'win_copy', 'slurp') and module_args and hasattr(self._connection._shell, '_unquote'): - for key in ('src', 'dest', 'path'): - if key in module_args: - module_args[key] = self._connection._shell._unquote(module_args[key]) + # Remove extra quotes surrounding path parameters before sending to module. + if module_name in ('win_stat', 'win_file', 'win_copy', 'slurp') and module_args and hasattr(self._connection._shell, '_unquote'): + for key in ('src', 'dest', 'path'): + if key in module_args: + module_args[key] = self._connection._shell._unquote(module_args[key]) - module_path = self._shared_loader_obj.module_loader.find_plugin(module_name, module_suffixes) - if module_path is None: + module_path = self._shared_loader_obj.module_loader.find_plugin(module_name, mod_type) + if module_path: + break + else: # This is a for-else: http://bit.ly/1ElPkyg + # FIXME: Why is it necessary to look for the windows version? + # Shouldn't all modules be installed? + # # Use Windows version of ping module to check module paths when # using a connection that supports .ps1 suffixes. - if module_suffixes and '.ps1' in module_suffixes: + if '.ps1' in self._connection.module_implementation_preferences: ping_module = 'win_ping' else: ping_module = 'ping' - module_path2 = self._shared_loader_obj.module_loader.find_plugin(ping_module, module_suffixes) + module_path2 = self._shared_loader_obj.module_loader.find_plugin(ping_module, self._connection.module_implementation_preferences) if module_path2 is not None: raise AnsibleError("The module %s was not found in configured module paths" % (module_name)) else: diff --git a/lib/ansible/plugins/connection/__init__.py b/lib/ansible/plugins/connection/__init__.py index 5dfcf4c344..188b227e46 100644 --- a/lib/ansible/plugins/connection/__init__.py +++ b/lib/ansible/plugins/connection/__init__.py @@ -57,6 +57,10 @@ class ConnectionBase(with_metaclass(ABCMeta, object)): has_pipelining = False become_methods = C.BECOME_METHODS + # When running over this connection type, prefer modules written in a certain language + # as discovered by the specified file extension. An empty string as the + # language means any language. + module_implementation_preferences = ('',) def __init__(self, play_context, new_stdin, *args, **kwargs): # All these hasattrs allow subclasses to override these parameters diff --git a/lib/ansible/plugins/connection/winrm.py b/lib/ansible/plugins/connection/winrm.py index 6289318c03..d1a6f57c53 100644 --- a/lib/ansible/plugins/connection/winrm.py +++ b/lib/ansible/plugins/connection/winrm.py @@ -52,10 +52,11 @@ from ansible.utils.unicode import to_bytes, to_unicode class Connection(ConnectionBase): '''WinRM connections over HTTP/HTTPS.''' + module_implementation_preferences = ('.ps1', '') + def __init__(self, *args, **kwargs): self.has_pipelining = False - self.default_suffixes = ['.ps1', ''] self.protocol = None self.shell_id = None self.delegate = None