From 6c2b372d1b481d6e82d25fec12372033359ef4a0 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Mon, 23 Mar 2020 14:47:58 -0500 Subject: [PATCH] [stable-2.9] Address fixme and handle filter/test errors for collections better (#68047) * Address fixme and handle fitler/test errors for collections better. Fixes #66721 * Re-arrange code (cherry picked from commit ee6413a) Co-authored-by: Matt Martz --- ...etter-jinja2-collection-error-handling.yml | 4 ++++ lib/ansible/template/__init__.py | 12 ++++++++---- .../plugins/filter/broken_filter.py | 13 +++++++++++++ .../integration/targets/collections/posix.yml | 19 +++++++++++++++++++ 4 files changed, 44 insertions(+), 4 deletions(-) create mode 100644 changelogs/fragments/66721-better-jinja2-collection-error-handling.yml create mode 100644 test/integration/targets/collections/collection_root_user/ansible_collections/testns/testbroken/plugins/filter/broken_filter.py diff --git a/changelogs/fragments/66721-better-jinja2-collection-error-handling.yml b/changelogs/fragments/66721-better-jinja2-collection-error-handling.yml new file mode 100644 index 00000000000..b64494389f1 --- /dev/null +++ b/changelogs/fragments/66721-better-jinja2-collection-error-handling.yml @@ -0,0 +1,4 @@ +bugfixes: +- collections - Handle errors better for filters and tests in collections, + where a non-existent collection is specified, or importing the plugin + results in an exception (https://github.com/ansible/ansible/issues/66721) diff --git a/lib/ansible/template/__init__.py b/lib/ansible/template/__init__.py index 54b84b670eb..1343bf227cc 100644 --- a/lib/ansible/template/__init__.py +++ b/lib/ansible/template/__init__.py @@ -335,9 +335,10 @@ class JinjaPluginIntercept(MutableMapping): if not acr: raise KeyError('invalid plugin name: {0}'.format(key)) - # FIXME: error handling for bogus plugin name, bogus impl, bogus filter/test - - pkg = import_module(acr.n_python_package_name) + try: + pkg = import_module(acr.n_python_package_name) + except ImportError: + raise KeyError() parent_prefix = acr.collection @@ -348,7 +349,10 @@ class JinjaPluginIntercept(MutableMapping): if ispkg: continue - plugin_impl = self._pluginloader.get(module_name) + try: + plugin_impl = self._pluginloader.get(module_name) + except Exception as e: + raise TemplateSyntaxError(to_native(e), 0) method_map = getattr(plugin_impl, self._method_map_name) diff --git a/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testbroken/plugins/filter/broken_filter.py b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testbroken/plugins/filter/broken_filter.py new file mode 100644 index 00000000000..64c25dc2166 --- /dev/null +++ b/test/integration/targets/collections/collection_root_user/ansible_collections/testns/testbroken/plugins/filter/broken_filter.py @@ -0,0 +1,13 @@ +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + + +class FilterModule(object): + + def filters(self): + return { + 'broken': lambda x: 'broken', + } + + +raise Exception('This is a broken filter plugin') diff --git a/test/integration/targets/collections/posix.yml b/test/integration/targets/collections/posix.yml index 521e44e0c62..66567a3025e 100644 --- a/test/integration/targets/collections/posix.yml +++ b/test/integration/targets/collections/posix.yml @@ -95,6 +95,25 @@ - lookup('testns.testcoll.mylookup2') == 'mylookup2_from_user_dir' - lookup('testns.testcoll.lookup_subdir.my_subdir_lookup') == 'subdir_lookup_from_user_dir' + - debug: + msg: "{{ 'foo'|testns.testbroken.broken }}" + register: result + ignore_errors: true + + - assert: + that: + - | + 'This is a broken filter plugin.' in result.msg + + - debug: + msg: "{{ 'foo'|missing.collection.filter }}" + register: result + ignore_errors: true + + - assert: + that: + - result is failed + # 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 - hosts: testhost