Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

plugin loader: return collection name; ansible-doc: handle ansible.builtin correctly #70026

Merged
merged 3 commits into from Jul 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions 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)."
36 changes: 17 additions & 19 deletions lib/ansible/cli/doc.py
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
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
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