From ee6413af471adb46ba6011662e17e52c1655c26c Mon Sep 17 00:00:00 2001
From: Matt Martz <matt@sivel.net>
Date: Mon, 23 Mar 2020 14:47:58 -0500
Subject: [PATCH] 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
---
 ...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 2a13a9fc1b4..fe20267e017 100644
--- a/lib/ansible/template/__init__.py
+++ b/lib/ansible/template/__init__.py
@@ -332,9 +332,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
 
@@ -345,7 +346,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 63937dc5c31..89e2a654bf3 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