Catch errors getting filters (#74127)

* Catch errors getting filters, and fail

* Add changelog

* Switch to warnings instead of errors, to match other plugin loader behavior

* Add tests

* Handle collections

changelogs/fragments/74127-bad-filter.yml

@@ -0,0 +1,5 @@
+bugfixes:
+- Templating - Ensure we catch exceptions when calling ``.filters()`` or
+  ``.tests()`` on their respective plugins and properly error, instead of
+  aborting which results in no filters being added to the jinja2 environment
+  (https://github.com/ansible/ansible/pull/74127)

lib/ansible/template/__init__.py

@@ -438,8 +438,12 @@ class JinjaPluginIntercept(MutableMapping):
             return
 
         for plugin in self._pluginloader.all():
+            try:
                 method_map = getattr(plugin, self._method_map_name)
                 self._delegatee.update(method_map())
+            except Exception as e:
+                display.warning("Skipping %s plugin %s as it seems to be invalid: %r" % (self._dirname, to_text(plugin._original_path), e))
+                continue
 
         if self._pluginloader.class_name == 'FilterModule':
             for plugin_name, plugin in self._delegatee.items():
@@ -546,7 +550,15 @@ class JinjaPluginIntercept(MutableMapping):
 
                 method_map = getattr(plugin_impl, self._method_map_name)
 
-                for func_name, func in iteritems(method_map()):
+                try:
+                    func_items = iteritems(method_map())
+                except Exception as e:
+                    display.warning(
+                        "Skipping %s plugin %s as it seems to be invalid: %r" % (self._dirname, to_text(plugin_impl._original_path), e),
+                    )
+                    continue
+
+                for func_name, func in func_items:
                     fq_name = '.'.join((parent_prefix, func_name))
                     # FIXME: detect/warn on intra-collection function name collisions
                     if self._pluginloader.class_name == 'FilterModule':

lib/ansible/template/safe_eval.py

@@ -106,11 +106,19 @@ def safe_eval(expr, locals=None, include_exceptions=False):
 
     filter_list = []
     for filter_ in filter_loader.all():
+        try:
             filter_list.extend(filter_.filters().keys())
+        except Exception:
+            # This is handled and displayed in JinjaPluginIntercept._load_ansible_plugins
+            continue
 
     test_list = []
     for test in test_loader.all():
+        try:
             test_list.extend(test.tests().keys())
+        except Exception:
+            # This is handled and displayed in JinjaPluginIntercept._load_ansible_plugins
+            continue
 
     CALL_ENABLED = C.CALLABLE_ACCEPT_LIST + filter_list + test_list
 

test/integration/targets/jinja_plugins/aliases

@@ -0,0 +1 @@
+shippable/posix/group5

test/integration/targets/jinja_plugins/collections/ansible_collections/foo/bar/plugins/filter/bad_collection_filter.py

@@ -0,0 +1,11 @@
+# Copyright (c) 2021 Matt Martz <matt@sivel.net>
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+# Make coding more python3-ish
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+
+class FilterModule:
+    def filters(self):
+        raise TypeError('bad_collection_filter')

test/integration/targets/jinja_plugins/collections/ansible_collections/foo/bar/plugins/filter/good_collection_filter.py

@@ -0,0 +1,13 @@
+# Copyright (c) 2021 Matt Martz <matt@sivel.net>
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+# Make coding more python3-ish
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+
+class FilterModule:
+    def filters(self):
+        return {
+            'hello': lambda x: 'Hello, %s!' % x,
+        }

test/integration/targets/jinja_plugins/collections/ansible_collections/foo/bar/plugins/test/bad_collection_test.py

@@ -0,0 +1,11 @@
+# Copyright (c) 2021 Matt Martz <matt@sivel.net>
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+# Make coding more python3-ish
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+
+class TestModule:
+    def tests(self):
+        raise TypeError('bad_collection_test')

test/integration/targets/jinja_plugins/collections/ansible_collections/foo/bar/plugins/test/good_collection_test.py

@@ -0,0 +1,13 @@
+# Copyright (c) 2021 Matt Martz <matt@sivel.net>
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+# Make coding more python3-ish
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+
+class TestModule:
+    def tests(self):
+        return {
+            'world': lambda x: x.lower() == 'world',
+        }

test/integration/targets/jinja_plugins/filter_plugins/bad_filter.py

@@ -0,0 +1,11 @@
+# Copyright (c) 2021 Matt Martz <matt@sivel.net>
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+# Make coding more python3-ish
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+
+class FilterModule:
+    def filters(self):
+        raise TypeError('bad_filter')

test/integration/targets/jinja_plugins/filter_plugins/good_filter.py

@@ -0,0 +1,13 @@
+# Copyright (c) 2021 Matt Martz <matt@sivel.net>
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+# Make coding more python3-ish
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+
+class FilterModule:
+    def filters(self):
+        return {
+            'hello': lambda x: 'Hello, %s!' % x,
+        }

test/integration/targets/jinja_plugins/playbook.yml

@@ -0,0 +1,10 @@
+- hosts: localhost
+  gather_facts: false
+  tasks:
+    - assert:
+        that:
+          - '"World"|hello == "Hello, World!"'
+          - '"World" is world'
+
+          - '"World"|foo.bar.hello == "Hello, World!"'
+          - '"World" is foo.bar.world'

test/integration/targets/jinja_plugins/tasks/main.yml

@@ -0,0 +1,22 @@
+- shell: ansible-playbook {{ verbosity }} playbook.yml
+  args:
+    chdir: '{{ role_path }}'
+  vars:
+    verbosity: "{{ '' if not ansible_verbosity else '-' ~ ('v' * ansible_verbosity) }}"
+  register: result
+
+- debug:
+    var: result
+
+- assert:
+    that:
+      - '"[WARNING]: Skipping filter plugin" in result.stderr'
+      - '"[WARNING]: Skipping test plugin" in result.stderr'
+      - |
+        result.stderr|regex_findall('bad_filter')|length == 2
+      - |
+        result.stderr|regex_findall('bad_test')|length == 2
+      - |
+        result.stderr|regex_findall('bad_collection_filter')|length == 2
+      - |
+        result.stderr|regex_findall('bad_collection_test')|length == 2

test/integration/targets/jinja_plugins/test_plugins/bad_test.py

@@ -0,0 +1,11 @@
+# Copyright (c) 2021 Matt Martz <matt@sivel.net>
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+# Make coding more python3-ish
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+
+class TestModule:
+    def tests(self):
+        raise TypeError('bad_test')

test/integration/targets/jinja_plugins/test_plugins/good_test.py

@@ -0,0 +1,13 @@
+# Copyright (c) 2021 Matt Martz <matt@sivel.net>
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+# Make coding more python3-ish
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+
+class TestModule:
+    def tests(self):
+        return {
+            'world': lambda x: x.lower() == 'world',
+        }