Skip to content

Commit

Permalink
plugin loader: return collection name; ansible-doc: handle ansible.bu…
Browse files Browse the repository at this point in the history
…iltin correctly (#70026)

* Determine collection in plugin loader.

* Fix test.

* Use PluginPathContext objects in PluginLoader._plugin_path_cache instead of tuples.
  • Loading branch information
felixfontein committed Jul 10, 2020
1 parent 1b4fd23 commit 24dcaf8
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 41 deletions.
2 changes: 2 additions & 0 deletions changelogs/fragments/plugin-loader-collection-name.yml
Original file line number Diff line number Diff line change
@@ -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)."
36 changes: 17 additions & 19 deletions lib/ansible/cli/doc.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -181,9 +181,10 @@ def run(self):
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)

Expand Down Expand Up @@ -258,23 +259,21 @@ def run(self):
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))

@staticmethod
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),
Expand Down Expand Up @@ -313,11 +312,9 @@ def _get_plugin_doc(plugin, plugin_type, loader, search_paths):
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),
Expand Down Expand Up @@ -369,7 +366,8 @@ def format_plugin_doc(plugin, plugin_type, doc, plainexamples, returndocs, metad
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)

Expand Down
64 changes: 44 additions & 20 deletions lib/ansible/plugins/loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -123,6 +129,7 @@ def __init__(self):
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
Expand Down Expand Up @@ -152,10 +159,11 @@ def record_deprecation(self, name, deprecation, collection_name):
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
Expand Down Expand Up @@ -309,16 +317,16 @@ def _get_package_paths(self, subdirs=True):
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
# which always calls it with subdirs=False. So there currently isn't a problem with this caching.
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:
Expand All @@ -328,14 +336,14 @@ def _get_paths(self, subdirs=True):
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
Expand All @@ -353,12 +361,18 @@ def _get_paths(self, subdirs=True):
# 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 '''

Expand Down Expand Up @@ -487,7 +501,8 @@ def _find_fq_plugin(self, fq_name, extension, plugin_load_context):

# 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
Expand All @@ -505,7 +520,8 @@ def _find_fq_plugin(self, fq_name, extension, plugin_load_context):
# 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 '''
Expand Down Expand Up @@ -626,8 +642,10 @@ def _find_plugin_legacy(self, name, plugin_load_context, ignore_deprecated=False
# 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:
Expand All @@ -638,8 +656,9 @@ def _find_plugin_legacy(self, name, plugin_load_context, ignore_deprecated=False
# 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:
Expand All @@ -658,28 +677,31 @@ def _find_plugin_legacy(self, name, plugin_load_context, ignore_deprecated=False

splitname = os.path.splitext(full_name)
base_name = splitname[0]
internal = path_context.internal
try:
extension = splitname[1]
except IndexError:
extension = ''

# 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:
Expand All @@ -691,12 +713,14 @@ def _find_plugin_legacy(self, name, plugin_load_context, ignore_deprecated=False
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

Expand Down
5 changes: 3 additions & 2 deletions test/units/plugins/test_plugins.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
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):
Expand Down Expand Up @@ -59,7 +59,8 @@ def test_plugins__get_package_paths_with_package(self):

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
Expand Down

0 comments on commit 24dcaf8

Please sign in to comment.