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.

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)."

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)
 

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
 

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