diff options
author | Sloane Hertel <19572925+s-hertel@users.noreply.github.com> | 2022-03-15 14:11:12 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-03-15 11:11:12 -0700 |
commit | 939f8430241a9bafd1fd903ce865b0d422b2218b (patch) | |
tree | 1376f4441b7136c41d2ac186291824ef1f4a358c | |
parent | ca8835c99029e6cbbb64a5510e6b03a943ba1697 (diff) | |
download | ansible-939f8430241a9bafd1fd903ce865b0d422b2218b.tar.gz |
[2.11] Fix collection redirects for filter and test plugins (#77210) (#77228)
* Fix collection redirects for filter and test plugins (#77210)
* Fix collection redirects for jinja2 filters/tests
* Handle recursive redirects
Co-authored-by: Matt Martz <matt@sivel.net>
(cherry picked from commit 8063643b4cec51a72377da5f3fa354d3ff9e737a)
* The error message is only capitalized on 2.13. Make test more flexible.
(cherry picked from commit 734777ef05b8c200f45336a8b9b41f9f9af597c1)
5 files changed, 101 insertions, 36 deletions
diff --git a/changelogs/fragments/77210-fix-collection-filter-test-redirects.yml b/changelogs/fragments/77210-fix-collection-filter-test-redirects.yml new file mode 100644 index 0000000000..2175696796 --- /dev/null +++ b/changelogs/fragments/77210-fix-collection-filter-test-redirects.yml @@ -0,0 +1,2 @@ +bugfixes: + - Fix collection filter/test plugin redirects (https://github.com/ansible/ansible/issues/77192). diff --git a/lib/ansible/template/__init__.py b/lib/ansible/template/__init__.py index 6dc7d8e394..5006c035d4 100644 --- a/lib/ansible/template/__init__.py +++ b/lib/ansible/template/__init__.py @@ -445,6 +445,7 @@ class JinjaPluginIntercept(MutableMapping): # FUTURE: we can cache FQ filter/test calls for the entire duration of a run, since a given collection's impl's # aren't supposed to change during a run def __getitem__(self, key): + original_key = key self._load_ansible_plugins() try: @@ -459,56 +460,61 @@ class JinjaPluginIntercept(MutableMapping): if func: return func - # didn't find it in the pre-built Jinja env, assume it's a former builtin and follow the normal routing path - leaf_key = key - key = 'ansible.builtin.' + key - else: - leaf_key = key.split('.')[-1] + key, leaf_key = get_fqcr_and_name(key) + seen = set() + + while True: + if key in seen: + raise TemplateSyntaxError( + 'recursive collection redirect found for %r' % original_key, + 0 + ) + seen.add(key) - acr = AnsibleCollectionRef.try_parse_fqcr(key, self._dirname) + acr = AnsibleCollectionRef.try_parse_fqcr(key, self._dirname) - if not acr: - raise KeyError('invalid plugin name: {0}'.format(key)) + if not acr: + raise KeyError('invalid plugin name: {0}'.format(key)) - ts = _get_collection_metadata(acr.collection) + ts = _get_collection_metadata(acr.collection) - # TODO: implement support for collection-backed redirect (currently only builtin) - # TODO: implement cycle detection (unified across collection redir as well) + # TODO: implement cycle detection (unified across collection redir as well) - routing_entry = ts.get('plugin_routing', {}).get(self._dirname, {}).get(leaf_key, {}) + routing_entry = ts.get('plugin_routing', {}).get(self._dirname, {}).get(leaf_key, {}) - deprecation_entry = routing_entry.get('deprecation') - if deprecation_entry: - warning_text = deprecation_entry.get('warning_text') - removal_date = deprecation_entry.get('removal_date') - removal_version = deprecation_entry.get('removal_version') + deprecation_entry = routing_entry.get('deprecation') + if deprecation_entry: + warning_text = deprecation_entry.get('warning_text') + removal_date = deprecation_entry.get('removal_date') + removal_version = deprecation_entry.get('removal_version') - if not warning_text: - warning_text = '{0} "{1}" is deprecated'.format(self._dirname, key) + if not warning_text: + warning_text = '{0} "{1}" is deprecated'.format(self._dirname, key) - display.deprecated(warning_text, version=removal_version, date=removal_date, collection_name=acr.collection) + display.deprecated(warning_text, version=removal_version, date=removal_date, collection_name=acr.collection) - tombstone_entry = routing_entry.get('tombstone') + tombstone_entry = routing_entry.get('tombstone') - if tombstone_entry: - warning_text = tombstone_entry.get('warning_text') - removal_date = tombstone_entry.get('removal_date') - removal_version = tombstone_entry.get('removal_version') + if tombstone_entry: + warning_text = tombstone_entry.get('warning_text') + removal_date = tombstone_entry.get('removal_date') + removal_version = tombstone_entry.get('removal_version') - if not warning_text: - warning_text = '{0} "{1}" has been removed'.format(self._dirname, key) + if not warning_text: + warning_text = '{0} "{1}" has been removed'.format(self._dirname, key) - exc_msg = display.get_deprecation_message(warning_text, version=removal_version, date=removal_date, - collection_name=acr.collection, removed=True) + exc_msg = display.get_deprecation_message(warning_text, version=removal_version, date=removal_date, + collection_name=acr.collection, removed=True) - raise AnsiblePluginRemovedError(exc_msg) + raise AnsiblePluginRemovedError(exc_msg) - redirect_fqcr = routing_entry.get('redirect', None) - if redirect_fqcr: - acr = AnsibleCollectionRef.from_fqcr(ref=redirect_fqcr, ref_type=self._dirname) - display.vvv('redirecting {0} {1} to {2}.{3}'.format(self._dirname, key, acr.collection, acr.resource)) - key = redirect_fqcr - # TODO: handle recursive forwarding (not necessary for builtin, but definitely for further collection redirs) + redirect = routing_entry.get('redirect', None) + if redirect: + next_key, leaf_key = get_fqcr_and_name(redirect, collection=acr.collection) + display.vvv('redirecting (type: {0}) {1}.{2} to {3}'.format(self._dirname, acr.collection, acr.resource, next_key)) + key = next_key + else: + break func = self._collection_jinja_func_cache.get(key) @@ -576,6 +582,17 @@ class JinjaPluginIntercept(MutableMapping): return len(self._delegatee) +def get_fqcr_and_name(resource, collection='ansible.builtin'): + if '.' not in resource: + name = resource + fqcr = collection + '.' + resource + else: + name = resource.split('.')[-1] + fqcr = resource + + return fqcr, name + + class AnsibleEnvironment(Environment): ''' Our custom environment, which simply allows us to override the class-level 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 index da8e4901ce..2dcf456e17 100644 --- 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 @@ -2,3 +2,20 @@ plugin_routing: modules: ping: redirect: testns.testcoll.ping + filter: + multi_redirect_filter: + redirect: testns.testredirect.redirect_filter1 + deprecation: + warning_text: deprecation1 + redirect_filter1: + redirect: redirect_filter2 + deprecation: + warning_text: deprecation2 + redirect_filter2: + redirect: testns.testcoll.testfilter + deprecation: + warning_text: deprecation3 + dead_end: + redirect: bad_redirect + recursive_redirect: + redirect: recursive_redirect diff --git a/test/integration/targets/collections/runme.sh b/test/integration/targets/collections/runme.sh index 663b3e1cd7..f7f5aba30c 100755 --- a/test/integration/targets/collections/runme.sh +++ b/test/integration/targets/collections/runme.sh @@ -69,6 +69,16 @@ else ansible-playbook -i "${INVENTORY_PATH}" collection_root_user/ansible_collections/testns/testcoll/playbooks/default_collection_playbook.yml "$@" fi +# test redirects and warnings for filter redirects +echo "testing redirect and deprecation display" +ANSIBLE_DEPRECATION_WARNINGS=yes ansible localhost -m debug -a msg='{{ "data" | testns.testredirect.multi_redirect_filter }}' -vvvvv 2>&1 | tee out.txt +cat out.txt + +test "$(grep out.txt -ce 'deprecation1' -ce 'deprecation2' -ce 'deprecation3')" == 3 +grep out.txt -e 'redirecting (type: filter) testns.testredirect.multi_redirect_filter to testns.testredirect.redirect_filter1' +grep out.txt -e 'redirecting (type: filter) testns.testredirect.redirect_filter1 to testns.testredirect.redirect_filter2' +grep out.txt -e 'redirecting (type: filter) testns.testredirect.redirect_filter2 to testns.testcoll.testfilter' + echo "--- validating collections support in playbooks/roles" # run test playbooks ansible-playbook -i "${INVENTORY_PATH}" -v "${TEST_PLAYBOOK}" "$@" diff --git a/test/integration/targets/collections/test_collection_meta.yml b/test/integration/targets/collections/test_collection_meta.yml index 22a00b2197..b682d220c0 100644 --- a/test/integration/targets/collections/test_collection_meta.yml +++ b/test/integration/targets/collections/test_collection_meta.yml @@ -21,6 +21,25 @@ # redirect filter - assert: that: ('yes' | formerly_core_filter) == True + # redirect filter (multiple levels) + - assert: + that: ('data' | testns.testredirect.multi_redirect_filter) == 'data_via_testfilter_from_userdir' + # invalid filter redirect + - debug: msg="{{ 'data' | testns.testredirect.dead_end }}" + ignore_errors: yes + register: redirect_failure + - assert: + that: + - redirect_failure is failed + - '"no filter named ''testns.testredirect.dead_end''" in (redirect_failure.msg | lower)' + # recursive filter redirect + - debug: msg="{{ 'data' | testns.testredirect.recursive_redirect }}" + ignore_errors: yes + register: redirect_failure + - assert: + that: + - redirect_failure is failed + - '"recursive collection redirect found for ''testns.testredirect.recursive_redirect''" in redirect_failure.msg' # legacy filter should mask redirected - assert: that: ('' | formerly_core_masked_filter) == 'hello from overridden formerly_core_masked_filter' |