From 24dcaf8974f27bb16577975cf46a36334f37784b Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Sat, 11 Jul 2020 00:24:08 +0200 Subject: [PATCH] plugin loader: return collection name; ansible-doc: handle ansible.builtin correctly (#70026) * Determine collection in plugin loader. * Fix test. * Use PluginPathContext objects in PluginLoader._plugin_path_cache instead of tuples. --- .../plugin-loader-collection-name.yml | 2 + lib/ansible/cli/doc.py | 36 +++++------ lib/ansible/plugins/loader.py | 64 +++++++++++++------ test/units/plugins/test_plugins.py | 5 +- 4 files changed, 66 insertions(+), 41 deletions(-) create mode 100644 changelogs/fragments/plugin-loader-collection-name.yml diff --git a/changelogs/fragments/plugin-loader-collection-name.yml b/changelogs/fragments/plugin-loader-collection-name.yml new file mode 100644 index 00000000000..22b6b38b53b --- /dev/null +++ b/changelogs/fragments/plugin-loader-collection-name.yml @@ -0,0 +1,2 @@ +minor_changes: +- "The plugin loader now keeps track of the collection where a plugin was resolved to, in particular whether the plugin was loaded from ansible-base's internal paths (``ansible.builtin``) or from user-supplied paths (no collection name)." diff --git a/lib/ansible/cli/doc.py b/lib/ansible/cli/doc.py index f9df9690be0..d20dc6d88a5 100644 --- a/lib/ansible/cli/doc.py +++ b/lib/ansible/cli/doc.py @@ -55,7 +55,7 @@ def add_collection_plugins(plugin_list, plugin_type, coll_filter=None): path = to_text(b_path, errors='surrogate_or_strict') collname = _get_collection_name_from_path(b_path) ptype = C.COLLECTION_PTYPE_COMPAT.get(plugin_type, plugin_type) - plugin_list.update(DocCLI.find_plugins(os.path.join(path, 'plugins', ptype), plugin_type, collection=collname)) + plugin_list.update(DocCLI.find_plugins(os.path.join(path, 'plugins', ptype), False, plugin_type, collection=collname)) class PluginNotFound(Exception): @@ -181,9 +181,10 @@ class DocCLI(CLI): coll_filter = context.CLIARGS['args'][0] if coll_filter in ('', None): - paths = loader._get_paths() - for path in paths: - self.plugin_list.update(DocCLI.find_plugins(path, plugin_type)) + paths = loader._get_paths_with_context() + for path_context in paths: + self.plugin_list.update( + DocCLI.find_plugins(path_context.path, path_context.internal, plugin_type)) add_collection_plugins(self.plugin_list, plugin_type, coll_filter=coll_filter) @@ -258,9 +259,9 @@ class DocCLI(CLI): def get_all_plugins_of_type(plugin_type): loader = getattr(plugin_loader, '%s_loader' % plugin_type) plugin_list = set() - paths = loader._get_paths() - for path in paths: - plugins_to_add = DocCLI.find_plugins(path, plugin_type) + paths = loader._get_paths_with_context() + for path_context in paths: + plugins_to_add = DocCLI.find_plugins(path_context.path, path_context.internal, plugin_type) plugin_list.update(plugins_to_add) return sorted(set(plugin_list)) @@ -268,13 +269,11 @@ class DocCLI(CLI): def get_plugin_metadata(plugin_type, plugin_name): # if the plugin lives in a non-python file (eg, win_X.ps1), require the corresponding python file for docs loader = getattr(plugin_loader, '%s_loader' % plugin_type) - filename = loader.find_plugin(plugin_name, mod_type='.py', ignore_deprecated=True, check_aliases=True) - if filename is None: + result = loader.find_plugin_with_context(plugin_name, mod_type='.py', ignore_deprecated=True, check_aliases=True) + if not result.resolved: raise AnsibleError("unable to load {0} plugin named {1} ".format(plugin_type, plugin_name)) - - collection_name = '' - if plugin_name.startswith('ansible_collections.'): - collection_name = '.'.join(plugin_name.split('.')[1:3]) + filename = result.plugin_resolved_path + collection_name = result.plugin_resolved_collection try: doc, __, __, __ = get_docstring(filename, fragment_loader, verbose=(context.CLIARGS['verbosity'] > 0), @@ -313,11 +312,9 @@ class DocCLI(CLI): result = loader.find_plugin_with_context(plugin, mod_type='.py', ignore_deprecated=True, check_aliases=True) if not result.resolved: raise PluginNotFound('%s was not found in %s' % (plugin, search_paths)) - plugin_name, filename = result.plugin_resolved_name, result.plugin_resolved_path - - collection_name = '' - if plugin_name.startswith('ansible_collections.'): - collection_name = '.'.join(plugin_name.split('.')[1:3]) + plugin_name = result.plugin_resolved_name + filename = result.plugin_resolved_path + collection_name = result.plugin_resolved_collection doc, plainexamples, returndocs, metadata = get_docstring( filename, fragment_loader, verbose=(context.CLIARGS['verbosity'] > 0), @@ -369,7 +366,8 @@ class DocCLI(CLI): return text @staticmethod - def find_plugins(path, ptype, collection=None): + def find_plugins(path, internal, ptype, collection=None): + # if internal, collection could be set to `ansible.builtin` display.vvvv("Searching %s for plugins" % path) diff --git a/lib/ansible/plugins/loader.py b/lib/ansible/plugins/loader.py index 755be2ffb8b..28f19691f5d 100644 --- a/lib/ansible/plugins/loader.py +++ b/lib/ansible/plugins/loader.py @@ -112,6 +112,12 @@ def add_dirs_to_loader(which_loader, paths): loader.add_directory(path, with_subdir=True) +class PluginPathContext(object): + def __init__(self, path, internal): + self.path = path + self.internal = internal + + class PluginLoadContext(object): def __init__(self): self.original_name = None @@ -123,6 +129,7 @@ class PluginLoadContext(object): self.exit_reason = None self.plugin_resolved_path = None self.plugin_resolved_name = None + self.plugin_resolved_collection = None # empty string for resolved plugins from user-supplied paths self.deprecated = False self.removal_date = None self.removal_version = None @@ -152,10 +159,11 @@ class PluginLoadContext(object): self.deprecation_warnings.append(warning_text) return self - def resolve(self, resolved_name, resolved_path, exit_reason): + def resolve(self, resolved_name, resolved_path, resolved_collection, exit_reason): self.pending_redirect = None self.plugin_resolved_name = resolved_name self.plugin_resolved_path = resolved_path + self.plugin_resolved_collection = resolved_collection self.exit_reason = exit_reason self.resolved = True return self @@ -309,8 +317,8 @@ class PluginLoader: return self._all_directories(self.package_path) return [self.package_path] - def _get_paths(self, subdirs=True): - ''' Return a list of paths to search for plugins in ''' + def _get_paths_with_context(self, subdirs=True): + ''' Return a list of PluginPathContext objects to search for plugins in ''' # FIXME: This is potentially buggy if subdirs is sometimes True and sometimes False. # In current usage, everything calls this with subdirs=True except for module_utils_loader and ansible-doc @@ -318,7 +326,7 @@ class PluginLoader: if self._paths is not None: return self._paths - ret = self._extra_dirs[:] + ret = [PluginPathContext(p, False) for p in self._extra_dirs] # look in any configured plugin paths, allow one level deep for subcategories if self.config is not None: @@ -328,14 +336,14 @@ class PluginLoader: contents = glob.glob("%s/*" % path) + glob.glob("%s/*/*" % path) for c in contents: if os.path.isdir(c) and c not in ret: - ret.append(c) + ret.append(PluginPathContext(c, False)) if path not in ret: - ret.append(path) + ret.append(PluginPathContext(path, False)) # look for any plugins installed in the package subtree # Note package path always gets added last so that every other type of # path is searched before it. - ret.extend(self._get_package_paths(subdirs=subdirs)) + ret.extend([PluginPathContext(p, True) for p in self._get_package_paths(subdirs=subdirs)]) # HACK: because powershell modules are in the same directory # hierarchy as other modules we have to process them last. This is @@ -353,12 +361,18 @@ class PluginLoader: # The expected sort order is paths in the order in 'ret' with paths ending in '/windows' at the end, # also in the original order they were found in 'ret'. # The .sort() method is guaranteed to be stable, so original order is preserved. - ret.sort(key=lambda p: p.endswith('/windows')) + ret.sort(key=lambda p: p.path.endswith('/windows')) # cache and return the result self._paths = ret return ret + def _get_paths(self, subdirs=True): + ''' Return a list of paths to search for plugins in ''' + + paths_with_context = self._get_paths_with_context(subdirs=subdirs) + return [path_with_context.path for path_with_context in paths_with_context] + def _load_config_defs(self, name, module, path): ''' Reads plugin docs to find configuration setting definitions, to push to config manager for later use ''' @@ -487,7 +501,8 @@ class PluginLoader: # FIXME: and is file or file link or ... if os.path.exists(n_resource_path): - return plugin_load_context.resolve(full_name, to_text(n_resource_path), 'found exact match for {0} in {1}'.format(full_name, acr.collection)) + return plugin_load_context.resolve( + full_name, to_text(n_resource_path), acr.collection, 'found exact match for {0} in {1}'.format(full_name, acr.collection)) if extension: # the request was extension-specific, don't try for an extensionless match @@ -505,7 +520,8 @@ class PluginLoader: # TODO: warn? pass - return plugin_load_context.resolve(full_name, to_text(found_files[0]), 'found fuzzy extension match for {0} in {1}'.format(full_name, acr.collection)) + return plugin_load_context.resolve( + full_name, to_text(found_files[0]), acr.collection, 'found fuzzy extension match for {0} in {1}'.format(full_name, acr.collection)) def find_plugin(self, name, mod_type='', ignore_deprecated=False, check_aliases=False, collection_list=None): ''' Find a plugin named name ''' @@ -626,8 +642,10 @@ class PluginLoader: # requested mod_type pull_cache = self._plugin_path_cache[suffix] try: - plugin_load_context.plugin_resolved_path = pull_cache[name] + path_with_context = pull_cache[name] + plugin_load_context.plugin_resolved_path = path_with_context.path plugin_load_context.plugin_resolved_name = name + plugin_load_context.plugin_resolved_collection = 'ansible.builtin' if path_with_context.internal else '' plugin_load_context.resolved = True return plugin_load_context except KeyError: @@ -638,8 +656,9 @@ class PluginLoader: # 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. - # We can use _get_paths() since add_directory() forces a cache refresh. - for path in (p for p in self._get_paths() if p not in self._searched_paths and os.path.isdir(p)): + # We can use _get_paths_with_context() since add_directory() forces a cache refresh. + for path_context in (p for p in self._get_paths_with_context() if p.path not in self._searched_paths and os.path.isdir(p.path)): + path = path_context.path display.debug('trying %s' % path) plugin_load_context.load_attempts.append(path) try: @@ -658,6 +677,7 @@ class PluginLoader: splitname = os.path.splitext(full_name) base_name = splitname[0] + internal = path_context.internal try: extension = splitname[1] except IndexError: @@ -665,21 +685,23 @@ class PluginLoader: # 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 + self._plugin_path_cache[''][base_name] = PluginPathContext(full_path, internal) if full_name not in self._plugin_path_cache['']: - self._plugin_path_cache[''][full_name] = full_path + self._plugin_path_cache[''][full_name] = PluginPathContext(full_path, internal) if base_name not in self._plugin_path_cache[extension]: - self._plugin_path_cache[extension][base_name] = full_path + self._plugin_path_cache[extension][base_name] = PluginPathContext(full_path, internal) if full_name not in self._plugin_path_cache[extension]: - self._plugin_path_cache[extension][full_name] = full_path + self._plugin_path_cache[extension][full_name] = PluginPathContext(full_path, internal) self._searched_paths.add(path) try: - plugin_load_context.plugin_resolved_path = pull_cache[name] + path_with_context = pull_cache[name] + plugin_load_context.plugin_resolved_path = path_with_context.path plugin_load_context.plugin_resolved_name = name + plugin_load_context.plugin_resolved_collection = 'ansible.builtin' if path_with_context.internal else '' plugin_load_context.resolved = True return plugin_load_context except KeyError: @@ -691,12 +713,14 @@ class PluginLoader: alias_name = '_' + name # We've already cached all the paths at this point if alias_name in pull_cache: - if not ignore_deprecated and not os.path.islink(pull_cache[alias_name]): + path_with_context = pull_cache[alias_name] + if not ignore_deprecated and not os.path.islink(path_with_context.path): # FIXME: this is not always the case, some are just aliases display.deprecated('%s is kept for backwards compatibility but usage is discouraged. ' # pylint: disable=ansible-deprecated-no-version 'The module documentation details page may explain more about this rationale.' % name.lstrip('_')) - plugin_load_context.plugin_resolved_path = pull_cache[alias_name] + plugin_load_context.plugin_resolved_path = path_with_context.path plugin_load_context.plugin_resolved_name = alias_name + plugin_load_context.plugin_resolved_collection = 'ansible.builtin' if path_with_context.internal else '' plugin_load_context.resolved = True return plugin_load_context diff --git a/test/units/plugins/test_plugins.py b/test/units/plugins/test_plugins.py index 51548500d22..c9d80cda6d4 100644 --- a/test/units/plugins/test_plugins.py +++ b/test/units/plugins/test_plugins.py @@ -25,7 +25,7 @@ import os from units.compat import unittest from units.compat.builtins import BUILTINS from units.compat.mock import patch, MagicMock -from ansible.plugins.loader import PluginLoader +from ansible.plugins.loader import PluginLoader, PluginPathContext class TestErrors(unittest.TestCase): @@ -59,7 +59,8 @@ class TestErrors(unittest.TestCase): def test_plugins__get_paths(self): pl = PluginLoader('test', '', 'test', 'test_plugin') - pl._paths = ['/path/one', '/path/two'] + pl._paths = [PluginPathContext('/path/one', False), + PluginPathContext('/path/two', True)] self.assertEqual(pl._get_paths(), ['/path/one', '/path/two']) # NOT YET WORKING