From 7ce0b390b27db007efb3c8417b4bd338a3a989fc Mon Sep 17 00:00:00 2001 From: Sloane Hertel <19572925+s-hertel@users.noreply.github.com> Date: Sun, 4 Apr 2021 22:45:52 -0400 Subject: [PATCH] Fix a bug adding unrelated candidates to the plugin loader redirect_list (#73863) (#73958) * Add tests for the redirect list * test redirect list for builtin module * test redirect list for redirected builtin module * test redirect list for collection module * test redirect list for redirected collection module * test redirect list for legacy module * changelog (cherry picked from commit 48c0fbd1cb92943b992b8a522e40ae6fe668e648) --- .../73863-fix-plugin-redirect-list.yaml | 2 + lib/ansible/plugins/loader.py | 12 ++- .../testcoll/plugins/action/plugin_lookup.py | 17 ++-- .../testns/testredirect/meta/runtime.yml | 4 + test/integration/targets/collections/runme.sh | 3 + .../collections/test_redirect_list.yml | 86 +++++++++++++++++++ 6 files changed, 118 insertions(+), 6 deletions(-) create mode 100644 changelogs/fragments/73863-fix-plugin-redirect-list.yaml create mode 100644 test/integration/targets/collections/collection_root_user/ansible_collections/testns/testredirect/meta/runtime.yml create mode 100644 test/integration/targets/collections/test_redirect_list.yml diff --git a/changelogs/fragments/73863-fix-plugin-redirect-list.yaml b/changelogs/fragments/73863-fix-plugin-redirect-list.yaml new file mode 100644 index 00000000000..9bc7becee17 --- /dev/null +++ b/changelogs/fragments/73863-fix-plugin-redirect-list.yaml @@ -0,0 +1,2 @@ +bugfixes: + - Fix adding unrelated candidate names to the plugin loader redirect list. diff --git a/lib/ansible/plugins/loader.py b/lib/ansible/plugins/loader.py index 3c241274e09..8246a1bf464 100644 --- a/lib/ansible/plugins/loader.py +++ b/lib/ansible/plugins/loader.py @@ -479,6 +479,11 @@ class PluginLoader: if redirect: # FIXME: remove once this is covered in debug or whatever display.vv("redirecting (type: {0}) {1} to {2}".format(plugin_type, fq_name, redirect)) + # The name doing the redirection is added at the beginning of _resolve_plugin_step, + # but if the unqualified name is used in conjunction with the collections keyword, only + # the unqualified name is in the redirect list. + if fq_name not in plugin_load_context.redirect_list: + plugin_load_context.redirect_list.append(fq_name) return plugin_load_context.redirect(redirect) # TODO: non-FQCN case, do we support `.` prefix for current collection, assume it with no dots, require it for subdirs in current, or ? @@ -606,7 +611,12 @@ class PluginLoader: # 'ansible.builtin' should be handled here. This means only internal, or builtin, paths are searched. plugin_load_context = self._find_fq_plugin(candidate_name, suffix, plugin_load_context=plugin_load_context) - if candidate_name != plugin_load_context.original_name and candidate_name not in plugin_load_context.redirect_list: + # Pending redirects are added to the redirect_list at the beginning of _resolve_plugin_step. + # Once redirects are resolved, ensure the final FQCN is added here. + # e.g. 'ns.coll.module' is included rather than only 'module' if a collections list is provided: + # - module: + # collections: ['ns.coll'] + if plugin_load_context.resolved and candidate_name not in plugin_load_context.redirect_list: plugin_load_context.redirect_list.append(candidate_name) if plugin_load_context.resolved or plugin_load_context.pending_redirect: # if we got an answer or need to chase down a redirect, return diff --git a/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/action/plugin_lookup.py b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/action/plugin_lookup.py index 1b882810dac..3fa41e8f5f7 100644 --- a/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/action/plugin_lookup.py +++ b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testcoll/plugins/action/plugin_lookup.py @@ -15,19 +15,26 @@ class ActionModule(ActionBase): result = super(ActionModule, self).run(None, task_vars) - type = self._task.args.get('type') + plugin_type = self._task.args.get('type') name = self._task.args.get('name') result = dict(changed=False, collection_list=self._task.collections) - if all([type, name]): - attr_name = '{0}_loader'.format(type) + if all([plugin_type, name]): + attr_name = '{0}_loader'.format(plugin_type) typed_loader = getattr(loader, attr_name, None) if not typed_loader: - return (dict(failed=True, msg='invalid plugin type {0}'.format(type))) + return (dict(failed=True, msg='invalid plugin type {0}'.format(plugin_type))) - result['plugin_path'] = typed_loader.find_plugin(name, collection_list=self._task.collections) + context = typed_loader.find_plugin_with_context(name, collection_list=self._task.collections) + + if not context.resolved: + result['plugin_path'] = None + result['redirect_list'] = [] + else: + result['plugin_path'] = context.plugin_resolved_path + result['redirect_list'] = context.redirect_list return result diff --git a/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testredirect/meta/runtime.yml b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testredirect/meta/runtime.yml new file mode 100644 index 00000000000..da8e4901ce7 --- /dev/null +++ b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testredirect/meta/runtime.yml @@ -0,0 +1,4 @@ +plugin_routing: + modules: + ping: + redirect: testns.testcoll.ping diff --git a/test/integration/targets/collections/runme.sh b/test/integration/targets/collections/runme.sh index dc01a6c0765..f3e886a5391 100755 --- a/test/integration/targets/collections/runme.sh +++ b/test/integration/targets/collections/runme.sh @@ -75,6 +75,9 @@ echo "--- validating inventory" # test collection inventories ansible-playbook inventory_test.yml -i a.statichost.yml -i redirected.statichost.yml "$@" +# test plugin loader redirect_list +ansible-playbook test_redirect_list.yml -v "$@" + # test adjacent with --playbook-dir export ANSIBLE_COLLECTIONS_PATH='' ANSIBLE_INVENTORY_ANY_UNPARSED_IS_FAILED=1 ansible-inventory --list --export --playbook-dir=. -v "$@" diff --git a/test/integration/targets/collections/test_redirect_list.yml b/test/integration/targets/collections/test_redirect_list.yml new file mode 100644 index 00000000000..8a24b9602de --- /dev/null +++ b/test/integration/targets/collections/test_redirect_list.yml @@ -0,0 +1,86 @@ +--- +- hosts: localhost + gather_facts: no + module_defaults: + testns.testcoll.plugin_lookup: + type: module + tasks: + - name: test builtin + testns.testcoll.plugin_lookup: + name: dnf + register: result + failed_when: + - result['redirect_list'] != ['dnf'] or result['plugin_path'].endswith('library/dnf.py') + + - name: test builtin with collections kw + testns.testcoll.plugin_lookup: + name: dnf + register: result + failed_when: + - result['redirect_list'] != ['dnf'] or result['plugin_path'].endswith('library/dnf.py') + collections: + - testns.unrelatedcoll + + - name: test redirected builtin + testns.testcoll.plugin_lookup: + name: formerly_core_ping + register: result + failed_when: result['redirect_list'] != expected_redirect_list + vars: + expected_redirect_list: + - formerly_core_ping + - ansible.builtin.formerly_core_ping + - testns.testcoll.ping + + - name: test redirected builtin with collections kw + testns.testcoll.plugin_lookup: + name: formerly_core_ping + register: result + failed_when: result['redirect_list'] != expected_redirect_list + vars: + expected_redirect_list: + - formerly_core_ping + - ansible.builtin.formerly_core_ping + - testns.testcoll.ping + collections: + - testns.unrelatedcoll + - testns.testcoll + + - name: test collection module with collections kw + testns.testcoll.plugin_lookup: + name: ping + register: result + failed_when: result['redirect_list'] != expected_redirect_list + vars: + expected_redirect_list: + - ping + - testns.testcoll.ping + collections: + - testns.unrelatedcoll + - testns.testcoll + + - name: test redirected collection module with collections kw + testns.testcoll.plugin_lookup: + name: ping + register: result + failed_when: result['redirect_list'] != expected_redirect_list + vars: + expected_redirect_list: + - ping + - testns.testredirect.ping + - testns.testcoll.ping + collections: + - testns.unrelatedcoll + - testns.testredirect + + - name: test legacy module with collections kw + testns.testcoll.plugin_lookup: + name: ping + register: result + failed_when: + - result['redirect_list'] != expected_redirect_list or not result['plugin_path'].endswith('library/ping.py') + vars: + expected_redirect_list: + - ping + collections: + - testns.unrelatedcoll