From bfa004930a02f505ff5e6fd6bcc3aa1f1286a8fc Mon Sep 17 00:00:00 2001 From: Matt Davis Date: Wed, 21 Aug 2019 10:45:04 +0100 Subject: [PATCH] add subdir support to collection loading (#60682) * add subdir support to collection loading * collections may now load plugins from subdirs under a plugin type or roles dir, eg `ns.coll.subdir1.subdir2.myrole`->ns.coll's roles/subdir1/subdir2/myrole, `ns.coll.subdir1.mymodule`->ns.coll's plugins/modules/subdir1/mymodule.py * centralize parsing/validation in AnsibleCollectionRef class * fix issues loading Jinja2 plugins from multiple sources * resolves #59462, #59890, * sanity test fixes * string fixes * add changelog entry --- changelogs/fragments/collections_subdirs.yml | 2 + lib/ansible/executor/task_queue_manager.py | 5 +- lib/ansible/galaxy/collection.py | 4 +- .../modules/network/aci/aci_firmware_group.py | 2 +- .../network/aci/aci_firmware_group_node.py | 2 +- .../network/aci/aci_firmware_policy.py | 2 +- .../network/aci/aci_maintenance_group.py | 2 +- lib/ansible/playbook/role/definition.py | 4 +- lib/ansible/plugins/loader.py | 41 ++-- lib/ansible/template/__init__.py | 20 +- lib/ansible/utils/collection_loader.py | 187 ++++++++++++++++-- lib/ansible/utils/plugin_docs.py | 43 ++-- .../action_subdir/subdir_ping_action.py | 19 ++ .../filter/filter_subdir/my_subdir_filters.py | 14 ++ .../testcoll/plugins/filter/myfilters.py | 2 +- .../testcoll/plugins/filter/myfilters2.py | 14 ++ .../lookup/lookup_subdir/my_subdir_lookup.py | 11 ++ .../testcoll/plugins/lookup/mylookup.py | 2 +- .../testcoll/plugins/lookup/mylookup2.py | 12 ++ .../module_subdir/subdir_ping_module.py | 14 ++ .../modules/testmodule_bad_docfrags.py | 25 +++ .../testns/testcoll/plugins/test/mytests2.py | 13 ++ .../test/test_subdir/my_subdir_tests.py | 13 ++ .../subdir_testrole/tasks/main.yml | 7 + .../roles/testrole_main_yaml/meta/main.yml | 4 + .../roles/testrole_main_yaml/tasks/main.yml | 30 +++ .../integration/targets/collections/posix.yml | 41 +++- test/integration/targets/collections/runme.sh | 4 + .../_data/sanity/code-smell/shebang.py | 2 +- test/units/utils/test_collection_loader.py | 86 ++++++++ 30 files changed, 539 insertions(+), 88 deletions(-) create mode 100644 changelogs/fragments/collections_subdirs.yml create mode 100644 test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/action/action_subdir/subdir_ping_action.py create mode 100644 test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/filter/filter_subdir/my_subdir_filters.py create mode 100644 test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/filter/myfilters2.py create mode 100644 test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/lookup/lookup_subdir/my_subdir_lookup.py create mode 100644 test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/lookup/mylookup2.py create mode 100644 test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/module_subdir/subdir_ping_module.py create mode 100644 test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/testmodule_bad_docfrags.py create mode 100644 test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/test/mytests2.py create mode 100644 test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/test/test_subdir/my_subdir_tests.py create mode 100644 test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/roles/role_subdir/subdir_testrole/tasks/main.yml create mode 100644 test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/roles/testrole_main_yaml/meta/main.yml create mode 100644 test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/roles/testrole_main_yaml/tasks/main.yml diff --git a/changelogs/fragments/collections_subdirs.yml b/changelogs/fragments/collections_subdirs.yml new file mode 100644 index 00000000000..25161b0adcb --- /dev/null +++ b/changelogs/fragments/collections_subdirs.yml @@ -0,0 +1,2 @@ +minor_changes: + - roles and plugins in collections may now be stored in subdirectories under the roles or plugin-type dirs (https://github.com/ansible/ansible/pull/60682) diff --git a/lib/ansible/executor/task_queue_manager.py b/lib/ansible/executor/task_queue_manager.py index a4962617a37..336e1487675 100644 --- a/lib/ansible/executor/task_queue_manager.py +++ b/lib/ansible/executor/task_queue_manager.py @@ -36,7 +36,7 @@ from ansible.playbook.play_context import PlayContext from ansible.plugins.loader import callback_loader, strategy_loader, module_loader from ansible.plugins.callback import CallbackBase from ansible.template import Templar -from ansible.utils.collection_loader import is_collection_ref +from ansible.utils.collection_loader import AnsibleCollectionRef from ansible.utils.helpers import pct_to_int from ansible.vars.hostvars import HostVars from ansible.vars.reserved import warn_if_reserved @@ -158,7 +158,8 @@ class TaskQueueManager: callback_obj.set_options() self._callback_plugins.append(callback_obj) - for callback_plugin_name in (c for c in C.DEFAULT_CALLBACK_WHITELIST if is_collection_ref(c)): + for callback_plugin_name in (c for c in C.DEFAULT_CALLBACK_WHITELIST if AnsibleCollectionRef.is_valid_fqcr(c)): + # TODO: need to extend/duplicate the stdout callback check here (and possible move this ahead of the old way callback_obj = callback_loader.get(callback_plugin_name) self._callback_plugins.append(callback_obj) diff --git a/lib/ansible/galaxy/collection.py b/lib/ansible/galaxy/collection.py index 7c19ee82a15..b999fa303dc 100644 --- a/lib/ansible/galaxy/collection.py +++ b/lib/ansible/galaxy/collection.py @@ -27,7 +27,7 @@ from ansible.galaxy import get_collections_galaxy_meta_info from ansible.galaxy.api import _urljoin from ansible.module_utils._text import to_bytes, to_native, to_text from ansible.module_utils import six -from ansible.utils.collection_loader import is_collection_ref +from ansible.utils.collection_loader import AnsibleCollectionRef from ansible.utils.display import Display from ansible.utils.hashing import secure_hash, secure_hash_s from ansible.module_utils.urls import open_url @@ -471,7 +471,7 @@ def validate_collection_name(name): :return: The input value, required for argparse validation. """ collection, dummy, dummy = name.partition(':') - if is_collection_ref('ansible_collections.{0}'.format(collection)): + if AnsibleCollectionRef.is_valid_collection_name(collection): return name raise AnsibleError("Invalid collection name '%s', name must be in the format .." % name) diff --git a/lib/ansible/modules/network/aci/aci_firmware_group.py b/lib/ansible/modules/network/aci/aci_firmware_group.py index f04c67682e4..40d408bf69a 100644 --- a/lib/ansible/modules/network/aci/aci_firmware_group.py +++ b/lib/ansible/modules/network/aci/aci_firmware_group.py @@ -39,7 +39,7 @@ options: choices: [ absent, present, query ] extends_documentation_fragment: - - ACI + - aci author: - Steven Gerhart (@sgerhart) ''' diff --git a/lib/ansible/modules/network/aci/aci_firmware_group_node.py b/lib/ansible/modules/network/aci/aci_firmware_group_node.py index 2e9ec2018ed..58908579204 100644 --- a/lib/ansible/modules/network/aci/aci_firmware_group_node.py +++ b/lib/ansible/modules/network/aci/aci_firmware_group_node.py @@ -40,7 +40,7 @@ options: choices: [ absent, present, query ] extends_documentation_fragment: - - ACI + - aci author: - Steven Gerhart (@sgerhart) diff --git a/lib/ansible/modules/network/aci/aci_firmware_policy.py b/lib/ansible/modules/network/aci/aci_firmware_policy.py index e4d8885a6bb..35c226c6017 100644 --- a/lib/ansible/modules/network/aci/aci_firmware_policy.py +++ b/lib/ansible/modules/network/aci/aci_firmware_policy.py @@ -49,7 +49,7 @@ options: choices: ['absent', 'present', 'query'] extends_documentation_fragment: - - ACI + - aci author: - Steven Gerhart (@sgerhart) diff --git a/lib/ansible/modules/network/aci/aci_maintenance_group.py b/lib/ansible/modules/network/aci/aci_maintenance_group.py index d85d01e8d3e..cffe015cc20 100644 --- a/lib/ansible/modules/network/aci/aci_maintenance_group.py +++ b/lib/ansible/modules/network/aci/aci_maintenance_group.py @@ -36,7 +36,7 @@ options: default: present choices: ['absent', 'present', 'query'] extends_documentation_fragment: - - ACI + - aci author: - Steven Gerhart (@sgerhart) ''' diff --git a/lib/ansible/playbook/role/definition.py b/lib/ansible/playbook/role/definition.py index 02b15e49c2a..74b83347029 100644 --- a/lib/ansible/playbook/role/definition.py +++ b/lib/ansible/playbook/role/definition.py @@ -31,7 +31,7 @@ from ansible.playbook.collectionsearch import CollectionSearch from ansible.playbook.conditional import Conditional from ansible.playbook.taggable import Taggable from ansible.template import Templar -from ansible.utils.collection_loader import get_collection_role_path, is_collection_ref +from ansible.utils.collection_loader import get_collection_role_path, AnsibleCollectionRef from ansible.utils.path import unfrackpath from ansible.utils.display import Display @@ -154,7 +154,7 @@ class RoleDefinition(Base, Conditional, Taggable, CollectionSearch): role_tuple = None # try to load as a collection-based role first - if self._collection_list or is_collection_ref(role_name): + if self._collection_list or AnsibleCollectionRef.is_valid_fqcr(role_name): role_tuple = get_collection_role_path(role_name, self._collection_list) if role_tuple: diff --git a/lib/ansible/plugins/loader.py b/lib/ansible/plugins/loader.py index f9c0320d6d0..625563cdf23 100644 --- a/lib/ansible/plugins/loader.py +++ b/lib/ansible/plugins/loader.py @@ -22,7 +22,7 @@ from ansible.module_utils.six import string_types from ansible.parsing.utils.yaml import from_yaml from ansible.parsing.yaml.loader import AnsibleLoader from ansible.plugins import get_plugin_class, MODULE_CACHE, PATH_CACHE, PLUGIN_PATH_CACHE -from ansible.utils.collection_loader import AnsibleCollectionLoader, AnsibleFlatMapLoader, is_collection_ref +from ansible.utils.collection_loader import AnsibleCollectionLoader, AnsibleFlatMapLoader, AnsibleCollectionRef from ansible.utils.display import Display from ansible.utils.plugin_docs import add_fragments @@ -317,38 +317,25 @@ class PluginLoader: display.debug('Added %s to loader search path' % (directory)) def _find_fq_plugin(self, fq_name, extension): - fq_name = to_native(fq_name) - # prefix our extension Python namespace if it isn't already there - if not fq_name.startswith('ansible_collections.'): - fq_name = 'ansible_collections.' + fq_name + plugin_type = AnsibleCollectionRef.legacy_plugin_dir_to_plugin_type(self.subdir) - splitname = fq_name.rsplit('.', 1) - if len(splitname) != 2: - raise ValueError('{0} is not a valid namespace-qualified plugin name'.format(to_native(fq_name))) + acr = AnsibleCollectionRef.from_fqcr(fq_name, plugin_type) - package = splitname[0] - resource = splitname[1] - - append_plugin_type = self.subdir.replace('_plugins', '') - - if append_plugin_type == 'library': - append_plugin_type = 'modules' - - package += '.plugins.{0}'.format(append_plugin_type) + n_resource = to_native(acr.resource, errors='strict') if extension: - resource += extension + n_resource += extension - pkg = sys.modules.get(package) + pkg = sys.modules.get(acr.n_python_package_name) if not pkg: # FIXME: there must be cheaper/safer way to do this - pkg = import_module(package) + pkg = import_module(acr.n_python_package_name) # if the package is one of our flatmaps, we need to consult its loader to find the path, since the file could be # anywhere in the tree if hasattr(pkg, '__loader__') and isinstance(pkg.__loader__, AnsibleFlatMapLoader): try: - file_path = pkg.__loader__.find_file(resource) + file_path = pkg.__loader__.find_file(n_resource) return to_text(file_path) except IOError: # this loader already takes care of extensionless files, so if we didn't find it, just bail @@ -356,15 +343,15 @@ class PluginLoader: pkg_path = os.path.dirname(pkg.__file__) - resource_path = os.path.join(pkg_path, resource) + n_resource_path = os.path.join(pkg_path, n_resource) # FIXME: and is file or file link or ... - if os.path.exists(resource_path): - return to_text(resource_path) + if os.path.exists(n_resource_path): + return to_text(n_resource_path) # look for any matching extension in the package location (sans filter) ext_blacklist = ['.pyc', '.pyo'] - found_files = [f for f in glob.iglob(os.path.join(pkg_path, resource) + '.*') if os.path.isfile(f) and os.path.splitext(f)[1] not in ext_blacklist] + found_files = [f for f in glob.iglob(os.path.join(pkg_path, n_resource) + '.*') if os.path.isfile(f) and os.path.splitext(f)[1] not in ext_blacklist] if not found_files: return None @@ -392,8 +379,8 @@ class PluginLoader: # they can have any suffix suffix = '' - # HACK: need this right now so we can still load shipped PS module_utils - if (is_collection_ref(name) or collection_list) and not name.startswith('Ansible'): + # FIXME: need this right now so we can still load shipped PS module_utils- come up with a more robust solution + if (AnsibleCollectionRef.is_valid_fqcr(name) or collection_list) and not name.startswith('Ansible'): if '.' in name or not collection_list: candidates = [name] else: diff --git a/lib/ansible/template/__init__.py b/lib/ansible/template/__init__.py index 2bc63dad581..ac79b4e588e 100644 --- a/lib/ansible/template/__init__.py +++ b/lib/ansible/template/__init__.py @@ -47,6 +47,7 @@ from ansible.plugins.loader import filter_loader, lookup_loader, test_loader from ansible.template.safe_eval import safe_eval from ansible.template.template import AnsibleJ2Template from ansible.template.vars import AnsibleJ2Vars +from ansible.utils.collection_loader import AnsibleCollectionRef from ansible.utils.display import Display from ansible.utils.unsafe_proxy import wrap_var @@ -325,20 +326,21 @@ class JinjaPluginIntercept(MutableMapping): if func: return func - components = key.split('.') + acr = AnsibleCollectionRef.try_parse_fqcr(key, self._dirname) - if len(components) != 3: + if not acr: raise KeyError('invalid plugin name: {0}'.format(key)) - collection_name = '.'.join(components[0:2]) - collection_pkg = 'ansible_collections.{0}.plugins.{1}'.format(collection_name, self._dirname) - # FIXME: error handling for bogus plugin name, bogus impl, bogus filter/test - # FIXME: move this capability into the Jinja plugin loader - pkg = import_module(collection_pkg) + pkg = import_module(acr.n_python_package_name) + + parent_prefix = acr.collection + + if acr.subdirs: + parent_prefix = '{0}.{1}'.format(parent_prefix, acr.subdirs) - for dummy, module_name, ispkg in pkgutil.iter_modules(pkg.__path__, prefix=collection_name + '.'): + for dummy, module_name, ispkg in pkgutil.iter_modules(pkg.__path__, prefix=parent_prefix + '.'): if ispkg: continue @@ -347,7 +349,7 @@ class JinjaPluginIntercept(MutableMapping): method_map = getattr(plugin_impl, self._method_map_name) for f in iteritems(method_map()): - fq_name = '.'.join((collection_name, f[0])) + fq_name = '.'.join((parent_prefix, f[0])) self._collection_jinja_func_cache[fq_name] = f[1] function_impl = self._collection_jinja_func_cache[key] diff --git a/lib/ansible/utils/collection_loader.py b/lib/ansible/utils/collection_loader.py index 9a311eb0c54..0eae71960d7 100644 --- a/lib/ansible/utils/collection_loader.py +++ b/lib/ansible/utils/collection_loader.py @@ -30,9 +30,6 @@ _SYNTHETIC_PACKAGES = { 'ansible_collections.ansible.builtin.plugins.modules': dict(type='flatmap', flatmap='ansible.modules', graft=True), } -# TODO: tighten this up to subset Python identifier requirements (and however we want to restrict ns/collection names) -_collection_qualified_re = re.compile(to_text(r'^(\w+)\.(\w+)\.(\w+)$')) - # FIXME: exception handling/error logging class AnsibleCollectionLoader(with_metaclass(Singleton, object)): @@ -271,32 +268,186 @@ class AnsibleFlatMapLoader(object): # def is_package(self, fullname): +class AnsibleCollectionRef: + # FUTURE: introspect plugin loaders to get these dynamically? + VALID_REF_TYPES = frozenset(to_text(r) for r in ['action', 'become', 'cache', 'callback', 'cliconf', 'connection', + 'doc_fragments', 'filter', 'httpapi', 'inventory', 'lookup', + 'module_utils', 'modules', 'netconf', 'role', 'shell', 'strategy', + 'terminal', 'test', 'vars']) + + # FIXME: tighten this up to match Python identifier reqs, etc + VALID_COLLECTION_NAME_RE = re.compile(to_text(r'^(\w+)\.(\w+)$')) + VALID_SUBDIRS_RE = re.compile(to_text(r'^\w+(\.\w+)*$')) + VALID_FQCR_RE = re.compile(to_text(r'^\w+\.\w+\.\w+(\.\w+)*$')) # can have 0-N included subdirs as well + + def __init__(self, collection_name, subdirs, resource, ref_type): + """ + Create an AnsibleCollectionRef from components + :param collection_name: a collection name of the form 'namespace.collectionname' + :param subdirs: optional subdir segments to be appended below the plugin type (eg, 'subdir1.subdir2') + :param resource: the name of the resource being references (eg, 'mymodule', 'someaction', 'a_role') + :param ref_type: the type of the reference, eg 'module', 'role', 'doc_fragment' + """ + collection_name = to_text(collection_name, errors='strict') + if subdirs is not None: + subdirs = to_text(subdirs, errors='strict') + resource = to_text(resource, errors='strict') + ref_type = to_text(ref_type, errors='strict') + + if not self.is_valid_collection_name(collection_name): + raise ValueError('invalid collection name (must be of the form namespace.collection): {0}'.format(to_native(collection_name))) + + if ref_type not in self.VALID_REF_TYPES: + raise ValueError('invalid collection ref_type: {0}'.format(ref_type)) + + self.collection = collection_name + if subdirs: + if not re.match(self.VALID_SUBDIRS_RE, subdirs): + raise ValueError('invalid subdirs entry: {0} (must be empty/None or of the form subdir1.subdir2)'.format(to_native(subdirs))) + self.subdirs = subdirs + else: + self.subdirs = u'' + + self.resource = resource + self.ref_type = ref_type + + package_components = [u'ansible_collections', self.collection] + + if self.ref_type == u'role': + package_components.append(u'roles') + else: + # we assume it's a plugin + package_components += [u'plugins', self.ref_type] + + if self.subdirs: + package_components.append(self.subdirs) + + if self.ref_type == u'role': + # roles are their own resource + package_components.append(self.resource) + + self.n_python_package_name = to_native('.'.join(package_components)) + + @staticmethod + def from_fqcr(ref, ref_type): + """ + Parse a string as a fully-qualified collection reference, raises ValueError if invalid + :param ref: collection reference to parse (a valid ref is of the form 'ns.coll.resource' or 'ns.coll.subdir1.subdir2.resource') + :param ref_type: the type of the reference, eg 'module', 'role', 'doc_fragment' + :return: a populated AnsibleCollectionRef object + """ + # assuming the fq_name is of the form (ns).(coll).(optional_subdir_N).(resource_name), + # we split the resource name off the right, split ns and coll off the left, and we're left with any optional + # subdirs that need to be added back below the plugin-specific subdir we'll add. So: + # ns.coll.resource -> ansible_collections.ns.coll.plugins.(plugintype).resource + # ns.coll.subdir1.resource -> ansible_collections.ns.coll.plugins.subdir1.(plugintype).resource + # ns.coll.rolename -> ansible_collections.ns.coll.roles.rolename + if not AnsibleCollectionRef.is_valid_fqcr(ref): + raise ValueError('{0} is not a valid collection reference'.format(to_native(ref))) + + ref = to_text(ref, errors='strict') + ref_type = to_text(ref_type, errors='strict') + + resource_splitname = ref.rsplit(u'.', 1) + package_remnant = resource_splitname[0] + resource = resource_splitname[1] + + # split the left two components of the collection package name off, anything remaining is plugin-type + # specific subdirs to be added back on below the plugin type + package_splitname = package_remnant.split(u'.', 2) + if len(package_splitname) == 3: + subdirs = package_splitname[2] + else: + subdirs = u'' + + collection_name = u'.'.join(package_splitname[0:2]) + + return AnsibleCollectionRef(collection_name, subdirs, resource, ref_type) + + @staticmethod + def try_parse_fqcr(ref, ref_type): + """ + Attempt to parse a string as a fully-qualified collection reference, returning None on failure (instead of raising an error) + :param ref: collection reference to parse (a valid ref is of the form 'ns.coll.resource' or 'ns.coll.subdir1.subdir2.resource') + :param ref_type: the type of the reference, eg 'module', 'role', 'doc_fragment' + :return: a populated AnsibleCollectionRef object on successful parsing, else None + """ + try: + return AnsibleCollectionRef.from_fqcr(ref, ref_type) + except ValueError: + pass + + @staticmethod + def legacy_plugin_dir_to_plugin_type(legacy_plugin_dir_name): + """ + Utility method to convert from a PluginLoader dir name to a plugin ref_type + :param legacy_plugin_dir_name: PluginLoader dir name (eg, 'action_plugins', 'library') + :return: the corresponding plugin ref_type (eg, 'action', 'role') + """ + legacy_plugin_dir_name = to_text(legacy_plugin_dir_name) + + plugin_type = legacy_plugin_dir_name.replace(u'_plugins', u'') + + if plugin_type == u'library': + plugin_type = u'modules' + + if plugin_type not in AnsibleCollectionRef.VALID_REF_TYPES: + raise ValueError('{0} cannot be mapped to a valid collection ref type'.format(to_native(legacy_plugin_dir_name))) + + return plugin_type + + @staticmethod + def is_valid_fqcr(ref, ref_type=None): + """ + Validates if is string is a well-formed fully-qualified collection reference (does not look up the collection itself) + :param ref: candidate collection reference to validate (a valid ref is of the form 'ns.coll.resource' or 'ns.coll.subdir1.subdir2.resource') + :param ref_type: optional reference type to enable deeper validation, eg 'module', 'role', 'doc_fragment' + :return: True if the collection ref passed is well-formed, False otherwise + """ + + ref = to_text(ref) + + if not ref_type: + return bool(re.match(AnsibleCollectionRef.VALID_FQCR_RE, ref)) + + return bool(AnsibleCollectionRef.try_parse_fqcr(ref, ref_type)) + + @staticmethod + def is_valid_collection_name(collection_name): + """ + Validates if is string is a well-formed collection name (does not look up the collection itself) + :param collection_name: candidate collection name to validate (a valid name is of the form 'ns.collname') + :return: True if the collection name passed is well-formed, False otherwise + """ + + collection_name = to_text(collection_name) + + return bool(re.match(AnsibleCollectionRef.VALID_COLLECTION_NAME_RE, collection_name)) + + def get_collection_role_path(role_name, collection_list=None): - match = _collection_qualified_re.match(role_name) + acr = AnsibleCollectionRef.try_parse_fqcr(role_name, 'role') - if match: - grps = match.groups() - collection_list = ['.'.join(grps[:2])] - role = grps[2] + if acr: + # looks like a valid qualified collection ref; skip the collection_list + role = acr.resource + collection_list = [acr.collection] elif not collection_list: return None # not a FQ role and no collection search list spec'd, nothing to do else: - role = role_name + role = role_name # treat as unqualified, loop through the collection search list to try and resolve for collection_name in collection_list: try: - role_package = u'ansible_collections.{0}.roles.{1}'.format(collection_name, role) + acr = AnsibleCollectionRef(collection_name=collection_name, subdirs=acr.subdirs, resource=acr.resource, ref_type=acr.ref_type) # FIXME: error handling/logging; need to catch any import failures and move along # FIXME: this line shouldn't be necessary, but py2 pkgutil.get_data is delegating back to built-in loader when it shouldn't - pkg = import_module(role_package + u'.tasks') - - # get_data input must be a native string - tasks_file = pkgutil.get_data(to_native(role_package) + '.tasks', 'main.yml') + pkg = import_module(acr.n_python_package_name) - if tasks_file is not None: + if pkg is not None: # the package is now loaded, get the collection's package and ask where it lives - path = os.path.dirname(to_bytes(sys.modules[role_package].__file__, errors='surrogate_or_strict')) + path = os.path.dirname(to_bytes(sys.modules[acr.n_python_package_name].__file__, errors='surrogate_or_strict')) return role, to_text(path, errors='surrogate_or_strict'), collection_name except IOError: @@ -308,9 +459,5 @@ def get_collection_role_path(role_name, collection_list=None): return None -def is_collection_ref(candidate_name): - return bool(_collection_qualified_re.match(candidate_name)) - - def set_collection_playbook_paths(b_playbook_paths): AnsibleCollectionLoader().set_playbook_paths(b_playbook_paths) diff --git a/lib/ansible/utils/plugin_docs.py b/lib/ansible/utils/plugin_docs.py index 70689d04ebd..af789277084 100644 --- a/lib/ansible/utils/plugin_docs.py +++ b/lib/ansible/utils/plugin_docs.py @@ -47,27 +47,37 @@ def add_fragments(doc, filename, fragment_loader): if isinstance(fragments, string_types): fragments = [fragments] - # Allow the module to specify a var other than DOCUMENTATION - # to pull the fragment from, using dot notation as a separator + unknown_fragments = [] + + # doc_fragments are allowed to specify a fragment var other than DOCUMENTATION + # with a . separator; this is complicated by collections-hosted doc_fragments that + # use the same separator. Assume it's collection-hosted normally first, try to load + # as-specified. If failure, assume the right-most component is a var, split it off, + # and retry the load. for fragment_slug in fragments: - fallback_name = None - fragment_slug = fragment_slug.lower() - if '.' in fragment_slug: - fallback_name = fragment_slug - fragment_name, fragment_var = fragment_slug.rsplit('.', 1) - fragment_var = fragment_var.upper() - else: - fragment_name, fragment_var = fragment_slug, 'DOCUMENTATION' + fragment_name = fragment_slug + fragment_var = 'DOCUMENTATION' fragment_class = fragment_loader.get(fragment_name) - if fragment_class is None and fallback_name: - fragment_class = fragment_loader.get(fallback_name) - fragment_var = 'DOCUMENTATION' + if fragment_class is None and '.' in fragment_slug: + splitname = fragment_slug.rsplit('.', 1) + fragment_name = splitname[0] + fragment_var = splitname[1].upper() + fragment_class = fragment_loader.get(fragment_name) if fragment_class is None: - raise AnsibleAssertionError('fragment_class is None') + unknown_fragments.append(fragment_slug) + continue + + fragment_yaml = getattr(fragment_class, fragment_var, None) + if fragment_yaml is None: + if fragment_var != 'DOCUMENTATION': + # if it's asking for something specific that's missing, that's an error + unknown_fragments.append(fragment_slug) + continue + else: + fragment_yaml = '{}' # TODO: this is still an error later since we require 'options' below... - fragment_yaml = getattr(fragment_class, fragment_var, '{}') fragment = AnsibleLoader(fragment_yaml, file_name=filename).get_single_data() if 'notes' in fragment: @@ -102,6 +112,9 @@ def add_fragments(doc, filename, fragment_loader): except Exception as e: raise AnsibleError("%s (%s) of unknown type: %s" % (to_native(e), fragment_name, filename)) + if unknown_fragments: + raise AnsibleError('unknown doc_fragment(s) in file {0}: {1}'.format(filename, to_native(', '.join(unknown_fragments)))) + def get_docstring(filename, fragment_loader, verbose=False, ignore_errors=False): """ diff --git a/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/action/action_subdir/subdir_ping_action.py b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/action/action_subdir/subdir_ping_action.py new file mode 100644 index 00000000000..5af73342779 --- /dev/null +++ b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/action/action_subdir/subdir_ping_action.py @@ -0,0 +1,19 @@ +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +from ansible.plugins.action import ActionBase + + +class ActionModule(ActionBase): + TRANSFERS_FILES = False + _VALID_ARGS = frozenset() + + def run(self, tmp=None, task_vars=None): + if task_vars is None: + task_vars = dict() + + result = super(ActionModule, self).run(None, task_vars) + + result = dict(changed=False) + + return result diff --git a/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/filter/filter_subdir/my_subdir_filters.py b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/filter/filter_subdir/my_subdir_filters.py new file mode 100644 index 00000000000..a5498a43efd --- /dev/null +++ b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/filter/filter_subdir/my_subdir_filters.py @@ -0,0 +1,14 @@ +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + + +def test_subdir_filter(data): + return "{0}_via_testfilter_from_subdir".format(data) + + +class FilterModule(object): + + def filters(self): + return { + 'test_subdir_filter': test_subdir_filter + } diff --git a/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/filter/myfilters.py b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/filter/myfilters.py index 6c7e34a9b35..0ce239e2f0c 100644 --- a/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/filter/myfilters.py +++ b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/filter/myfilters.py @@ -3,7 +3,7 @@ __metaclass__ = type def testfilter(data): - return "{0}_from_userdir".format(data) + return "{0}_via_testfilter_from_userdir".format(data) class FilterModule(object): diff --git a/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/filter/myfilters2.py b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/filter/myfilters2.py new file mode 100644 index 00000000000..07239222155 --- /dev/null +++ b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/filter/myfilters2.py @@ -0,0 +1,14 @@ +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + + +def testfilter2(data): + return "{0}_via_testfilter2_from_userdir".format(data) + + +class FilterModule(object): + + def filters(self): + return { + 'testfilter2': testfilter2 + } diff --git a/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/lookup/lookup_subdir/my_subdir_lookup.py b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/lookup/lookup_subdir/my_subdir_lookup.py new file mode 100644 index 00000000000..dd9818c964c --- /dev/null +++ b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/lookup/lookup_subdir/my_subdir_lookup.py @@ -0,0 +1,11 @@ +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +from ansible.plugins.lookup import LookupBase + + +class LookupModule(LookupBase): + + def run(self, terms, variables, **kwargs): + + return ['subdir_lookup_from_user_dir'] diff --git a/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/lookup/mylookup.py b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/lookup/mylookup.py index 8bd8deda8ee..1cf3d28f8cc 100644 --- a/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/lookup/mylookup.py +++ b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/lookup/mylookup.py @@ -8,4 +8,4 @@ class LookupModule(LookupBase): def run(self, terms, variables, **kwargs): - return ['lookup_from_user_dir'] + return ['mylookup_from_user_dir'] diff --git a/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/lookup/mylookup2.py b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/lookup/mylookup2.py new file mode 100644 index 00000000000..bda671f579e --- /dev/null +++ b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/lookup/mylookup2.py @@ -0,0 +1,12 @@ +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + + +from ansible.plugins.lookup import LookupBase + + +class LookupModule(LookupBase): + + def run(self, terms, variables, **kwargs): + + return ['mylookup2_from_user_dir'] diff --git a/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/module_subdir/subdir_ping_module.py b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/module_subdir/subdir_ping_module.py new file mode 100644 index 00000000000..5a70174d6d9 --- /dev/null +++ b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/module_subdir/subdir_ping_module.py @@ -0,0 +1,14 @@ +#!/usr/bin/python + +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +import json + + +def main(): + print(json.dumps(dict(changed=False, source='user'))) + + +if __name__ == '__main__': + main() diff --git a/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/testmodule_bad_docfrags.py b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/testmodule_bad_docfrags.py new file mode 100644 index 00000000000..46ccb76c745 --- /dev/null +++ b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/modules/testmodule_bad_docfrags.py @@ -0,0 +1,25 @@ +#!/usr/bin/python +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +import json + +DOCUMENTATION = r''' +module: testmodule +description: for testing +extends_documentation_fragment: + - noncollbogusfrag + - noncollbogusfrag.bogusvar + - bogusns.testcoll.frag + - testns.boguscoll.frag + - testns.testcoll.bogusfrag + - testns.testcoll.frag.bogusvar +''' + + +def main(): + print(json.dumps(dict(changed=False, source='user'))) + + +if __name__ == '__main__': + main() diff --git a/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/test/mytests2.py b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/test/mytests2.py new file mode 100644 index 00000000000..183944ff449 --- /dev/null +++ b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/test/mytests2.py @@ -0,0 +1,13 @@ +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + + +def testtest(data): + return data == 'from_user2' + + +class TestModule(object): + def tests(self): + return { + 'testtest2': testtest + } diff --git a/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/test/test_subdir/my_subdir_tests.py b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/test/test_subdir/my_subdir_tests.py new file mode 100644 index 00000000000..98a8f893f1b --- /dev/null +++ b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/test/test_subdir/my_subdir_tests.py @@ -0,0 +1,13 @@ +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + + +def subdir_test(data): + return data == 'subdir_from_user' + + +class TestModule(object): + def tests(self): + return { + 'subdir_test': subdir_test + } diff --git a/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/roles/role_subdir/subdir_testrole/tasks/main.yml b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/roles/role_subdir/subdir_testrole/tasks/main.yml new file mode 100644 index 00000000000..98445ce3aaf --- /dev/null +++ b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/roles/role_subdir/subdir_testrole/tasks/main.yml @@ -0,0 +1,7 @@ +- debug: + msg: '{{ test_role_input | default("(undefined)") }}' + register: test_role_output + +- assert: + that: + - test_role_input is not defined or test_role_input == test_role_output.msg diff --git a/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/roles/testrole_main_yaml/meta/main.yml b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/roles/testrole_main_yaml/meta/main.yml new file mode 100644 index 00000000000..8c22c1c6213 --- /dev/null +++ b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/roles/testrole_main_yaml/meta/main.yml @@ -0,0 +1,4 @@ +collections: +- ansible.builtin +- testns.coll_in_sys +- bogus.fromrolemeta diff --git a/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/roles/testrole_main_yaml/tasks/main.yml b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/roles/testrole_main_yaml/tasks/main.yml new file mode 100644 index 00000000000..0fa5302eefd --- /dev/null +++ b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/roles/testrole_main_yaml/tasks/main.yml @@ -0,0 +1,30 @@ +- name: check collections list from role meta + plugin_lookup: + register: pluginlookup_out + +- name: call role-local ping module + ping: + register: ping_out + +- name: call unqualified module in another collection listed in role meta (testns.coll_in_sys) + systestmodule: + register: systestmodule_out + +# verify that pluginloader caching doesn't prevent us from explicitly calling a builtin plugin with the same name +- name: call builtin ping module explicitly + ansible.builtin.ping: + register: builtinping_out + +- debug: + msg: '{{ test_role_input | default("(undefined)") }}' + register: test_role_output + +# FIXME: add tests to ensure that block/task level stuff in a collection-hosted role properly inherit role default/meta values + +- assert: + that: + - pluginlookup_out.collection_list == ['testns.testcoll', 'ansible.builtin', 'testns.coll_in_sys', 'bogus.fromrolemeta'] + - ping_out.source is defined and ping_out.source == 'user' + - systestmodule_out.source is defined and systestmodule_out.source == 'sys' + - builtinping_out.ping is defined and builtinping_out.ping == 'pong' + - test_role_input is not defined or test_role_input == test_role_output.msg diff --git a/test/integration/targets/collections/posix.yml b/test/integration/targets/collections/posix.yml index 0e3c020aafb..8e9e263059d 100644 --- a/test/integration/targets/collections/posix.yml +++ b/test/integration/targets/collections/posix.yml @@ -22,6 +22,16 @@ name: testns.testcoll.maskedmodule register: maskedmodule_out + # action in a collection subdir + - name: test subdir action FQ + testns.testcoll.action_subdir.subdir_ping_action: + register: subdir_ping_action_out + + # module in a collection subdir + - name: test subdir module FQ + testns.testcoll.module_subdir.subdir_ping_module: + register: subdir_ping_module_out + # module with a granular module_utils import (from (this collection).module_utils.leaf import thingtocall) - name: exec module with granular module utils import from this collection testns.testcoll.uses_leaf_mu_granular_import: @@ -49,18 +59,28 @@ - systestmodule_out.source == 'sys' - contentadjmodule_out.source == 'content_adj' - not maskedmodule_out.plugin_path + - subdir_ping_action_out is not changed + - subdir_ping_module_out is not changed - granular_out.mu_result == 'thingtocall in leaf' - granular_nested_out.mu_result == 'thingtocall in base called thingtocall in secondary' - flat_out.mu_result == 'thingtocall in leaf' - from_out.mu_result == 'thingtocall in leaf' - from_out.mu2_result == 'thingtocall in secondary' +- hosts: testhost + tasks: - name: exercise filters/tests/lookups assert: that: - - "'data' | testns.testcoll.testfilter == 'data_from_userdir'" + - "'data' | testns.testcoll.testfilter == 'data_via_testfilter_from_userdir'" + - "'data' | testns.testcoll.testfilter2 == 'data_via_testfilter2_from_userdir'" + - "'data' | testns.testcoll.filter_subdir.test_subdir_filter == 'data_via_testfilter_from_subdir'" - "'from_user' is testns.testcoll.testtest" - - lookup('testns.testcoll.mylookup') == 'lookup_from_user_dir' + - "'from_user2' is testns.testcoll.testtest2" + - "'subdir_from_user' is testns.testcoll.test_subdir.subdir_test" + - lookup('testns.testcoll.mylookup') == 'mylookup_from_user_dir' + - lookup('testns.testcoll.mylookup2') == 'mylookup2_from_user_dir' + - lookup('testns.testcoll.lookup_subdir.my_subdir_lookup') == 'subdir_lookup_from_user_dir' # ensure that the synthetic ansible.builtin collection limits to builtin plugins, that ansible.legacy loads overrides # from legacy plugin dirs, and that a same-named plugin loaded from a real collection is not masked by the others @@ -182,7 +202,7 @@ # - lookup('mylookup') == 'lookup_from_user_dir' -# test keyword-static execution of a FQ collection-backed role +# test keyword-static execution of a FQ collection-backed role with "tasks/main.yaml" - name: verify collection-backed role execution (keyword static) hosts: testhost collections: @@ -191,7 +211,7 @@ vars: test_role_input: keyword static roles: - - role: testns.testcoll.testrole + - role: testns.testcoll.testrole_main_yaml tasks: - name: ensure role executed assert: @@ -249,6 +269,19 @@ that: - test_role_output.msg == test_role_input +# test dynamic execution of a FQ collection-backed role +- name: verify collection-backed role execution in subdir (include) + hosts: testhost + vars: + test_role_input: dynamic (subdir) + tasks: + - include_role: + name: testns.testcoll.role_subdir.subdir_testrole + - name: ensure role executed + assert: + that: + - test_role_output.msg == test_role_input + - name: validate static task include behavior hosts: testhost diff --git a/test/integration/targets/collections/runme.sh b/test/integration/targets/collections/runme.sh index 3f6e6d590ac..d14e85b28f8 100755 --- a/test/integration/targets/collections/runme.sh +++ b/test/integration/targets/collections/runme.sh @@ -17,6 +17,10 @@ ANSIBLE_CALLBACK_WHITELIST=testns.testcoll.usercallback ansible localhost -m pin # test documentation ansible-doc testns.testcoll.testmodule -vvv | grep -- "- normal_doc_frag" +echo "testing bad doc_fragments (expected ERROR message follows)" +# test documentation failure +ansible-doc testns.testcoll.testmodule_bad_docfrags -vvv 2>&1 | grep -- "unknown doc_fragment" + # we need multiple plays, and conditional import_playbook is noisy and causes problems, so choose here which one to use... if [[ ${INVENTORY_PATH} == *.winrm ]]; then export TEST_PLAYBOOK=windows.yml diff --git a/test/lib/ansible_test/_data/sanity/code-smell/shebang.py b/test/lib/ansible_test/_data/sanity/code-smell/shebang.py index 0c2de08cd95..48d35520475 100755 --- a/test/lib/ansible_test/_data/sanity/code-smell/shebang.py +++ b/test/lib/ansible_test/_data/sanity/code-smell/shebang.py @@ -77,7 +77,7 @@ def main(): elif path.startswith('test/integration/targets/'): is_integration = True - if dirname.endswith('/library') or dirname.endswith('/plugins/modules') or dirname in ( + if dirname.endswith('/library') or '/plugins/modules' in dirname or dirname in ( # non-standard module library directories 'test/integration/targets/module_precedence/lib_no_extension', 'test/integration/targets/module_precedence/lib_with_extension', diff --git a/test/units/utils/test_collection_loader.py b/test/units/utils/test_collection_loader.py index 5dd6ad76bf2..7800f960071 100644 --- a/test/units/utils/test_collection_loader.py +++ b/test/units/utils/test_collection_loader.py @@ -2,8 +2,12 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type import os +import pytest +import re import sys +from ansible.utils.collection_loader import AnsibleCollectionRef + def test_import_from_collection(monkeypatch): collection_root = os.path.join(os.path.dirname(__file__), 'fixtures', 'collections') @@ -127,3 +131,85 @@ def test_import_from_collection(monkeypatch): # verify that the filename and line number reported by the trace is correct # this makes sure that collection loading preserves file paths and line numbers assert trace_log == expected_trace_log + + +@pytest.mark.parametrize( + 'ref,ref_type,expected_collection,expected_subdirs,expected_resource,expected_python_pkg_name', + [ + ('ns.coll.myaction', 'action', 'ns.coll', '', 'myaction', 'ansible_collections.ns.coll.plugins.action'), + ('ns.coll.subdir1.subdir2.myaction', 'action', 'ns.coll', 'subdir1.subdir2', 'myaction', 'ansible_collections.ns.coll.plugins.action.subdir1.subdir2'), + ('ns.coll.myrole', 'role', 'ns.coll', '', 'myrole', 'ansible_collections.ns.coll.roles.myrole'), + ('ns.coll.subdir1.subdir2.myrole', 'role', 'ns.coll', 'subdir1.subdir2', 'myrole', 'ansible_collections.ns.coll.roles.subdir1.subdir2.myrole'), + ]) +def test_fqcr_parsing_valid(ref, ref_type, expected_collection, + expected_subdirs, expected_resource, expected_python_pkg_name): + assert AnsibleCollectionRef.is_valid_fqcr(ref, ref_type) + + r = AnsibleCollectionRef.from_fqcr(ref, ref_type) + assert r.collection == expected_collection + assert r.subdirs == expected_subdirs + assert r.resource == expected_resource + assert r.n_python_package_name == expected_python_pkg_name + + r = AnsibleCollectionRef.try_parse_fqcr(ref, ref_type) + assert r.collection == expected_collection + assert r.subdirs == expected_subdirs + assert r.resource == expected_resource + assert r.n_python_package_name == expected_python_pkg_name + + +@pytest.mark.parametrize( + 'ref,ref_type,expected_error_type,expected_error_expression', + [ + ('no_dots_at_all_action', 'action', ValueError, 'is not a valid collection reference'), + ('no_nscoll.myaction', 'action', ValueError, 'is not a valid collection reference'), + ('ns.coll.myaction', 'bogus', ValueError, 'invalid collection ref_type'), + ]) +def test_fqcr_parsing_invalid(ref, ref_type, expected_error_type, expected_error_expression): + assert not AnsibleCollectionRef.is_valid_fqcr(ref, ref_type) + + with pytest.raises(expected_error_type) as curerr: + AnsibleCollectionRef.from_fqcr(ref, ref_type) + + assert re.search(expected_error_expression, str(curerr.value)) + + r = AnsibleCollectionRef.try_parse_fqcr(ref, ref_type) + assert r is None + + +@pytest.mark.parametrize( + 'name,subdirs,resource,ref_type,python_pkg_name', + [ + ('ns.coll', None, 'res', 'doc_fragments', 'ansible_collections.ns.coll.plugins.doc_fragments'), + ('ns.coll', 'subdir1', 'res', 'doc_fragments', 'ansible_collections.ns.coll.plugins.doc_fragments.subdir1'), + ('ns.coll', 'subdir1.subdir2', 'res', 'action', 'ansible_collections.ns.coll.plugins.action.subdir1.subdir2'), + ]) +def test_collectionref_components_valid(name, subdirs, resource, ref_type, python_pkg_name): + x = AnsibleCollectionRef(name, subdirs, resource, ref_type) + + assert x.collection == name + if subdirs: + assert x.subdirs == subdirs + else: + assert x.subdirs == '' + + assert x.resource == resource + assert x.ref_type == ref_type + assert x.n_python_package_name == python_pkg_name + + +@pytest.mark.parametrize( + 'name,subdirs,resource,ref_type,expected_error_type,expected_error_expression', + [ + ('bad_ns', '', 'resource', 'action', ValueError, 'invalid collection name'), + ('ns.coll.', '', 'resource', 'action', ValueError, 'invalid collection name'), + ('ns.coll', 'badsubdir#', 'resource', 'action', ValueError, 'invalid subdirs entry'), + ('ns.coll', 'badsubdir.', 'resource', 'action', ValueError, 'invalid subdirs entry'), + ('ns.coll', '.badsubdir', 'resource', 'action', ValueError, 'invalid subdirs entry'), + ('ns.coll', '', 'resource', 'bogus', ValueError, 'invalid collection ref_type'), + ]) +def test_collectionref_components_invalid(name, subdirs, resource, ref_type, expected_error_type, expected_error_expression): + with pytest.raises(expected_error_type) as curerr: + AnsibleCollectionRef(name, subdirs, resource, ref_type) + + assert re.search(expected_error_expression, str(curerr.value))