From 1082e2ab794940f3114c29199a765edee47aabbe Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Thu, 15 Apr 2021 15:51:41 -0500 Subject: [PATCH] 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 | 5 +++++ lib/ansible/template/__init__.py | 18 ++++++++++++--- lib/ansible/template/safe_eval.py | 12 ++++++++-- .../integration/targets/jinja_plugins/aliases | 1 + .../plugins/filter/bad_collection_filter.py | 11 ++++++++++ .../plugins/filter/good_collection_filter.py | 13 +++++++++++ .../bar/plugins/test/bad_collection_test.py | 11 ++++++++++ .../bar/plugins/test/good_collection_test.py | 13 +++++++++++ .../filter_plugins/bad_filter.py | 11 ++++++++++ .../filter_plugins/good_filter.py | 13 +++++++++++ .../targets/jinja_plugins/playbook.yml | 10 +++++++++ .../targets/jinja_plugins/tasks/main.yml | 22 +++++++++++++++++++ .../jinja_plugins/test_plugins/bad_test.py | 11 ++++++++++ .../jinja_plugins/test_plugins/good_test.py | 13 +++++++++++ 14 files changed, 159 insertions(+), 5 deletions(-) create mode 100644 changelogs/fragments/74127-bad-filter.yml create mode 100644 test/integration/targets/jinja_plugins/aliases create mode 100644 test/integration/targets/jinja_plugins/collections/ansible_collections/foo/bar/plugins/filter/bad_collection_filter.py create mode 100644 test/integration/targets/jinja_plugins/collections/ansible_collections/foo/bar/plugins/filter/good_collection_filter.py create mode 100644 test/integration/targets/jinja_plugins/collections/ansible_collections/foo/bar/plugins/test/bad_collection_test.py create mode 100644 test/integration/targets/jinja_plugins/collections/ansible_collections/foo/bar/plugins/test/good_collection_test.py create mode 100644 test/integration/targets/jinja_plugins/filter_plugins/bad_filter.py create mode 100644 test/integration/targets/jinja_plugins/filter_plugins/good_filter.py create mode 100644 test/integration/targets/jinja_plugins/playbook.yml create mode 100644 test/integration/targets/jinja_plugins/tasks/main.yml create mode 100644 test/integration/targets/jinja_plugins/test_plugins/bad_test.py create mode 100644 test/integration/targets/jinja_plugins/test_plugins/good_test.py diff --git a/changelogs/fragments/74127-bad-filter.yml b/changelogs/fragments/74127-bad-filter.yml new file mode 100644 index 00000000000..a89cc66663b --- /dev/null +++ b/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) diff --git a/lib/ansible/template/__init__.py b/lib/ansible/template/__init__.py index cf955f75c66..e90712a30d0 100644 --- a/lib/ansible/template/__init__.py +++ b/lib/ansible/template/__init__.py @@ -438,8 +438,12 @@ class JinjaPluginIntercept(MutableMapping): return for plugin in self._pluginloader.all(): - method_map = getattr(plugin, self._method_map_name) - self._delegatee.update(method_map()) + 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': diff --git a/lib/ansible/template/safe_eval.py b/lib/ansible/template/safe_eval.py index 06bc7687198..3ed46addaf9 100644 --- a/lib/ansible/template/safe_eval.py +++ b/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(): - filter_list.extend(filter_.filters().keys()) + 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(): - test_list.extend(test.tests().keys()) + 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 diff --git a/test/integration/targets/jinja_plugins/aliases b/test/integration/targets/jinja_plugins/aliases new file mode 100644 index 00000000000..70a7b7a9f32 --- /dev/null +++ b/test/integration/targets/jinja_plugins/aliases @@ -0,0 +1 @@ +shippable/posix/group5 diff --git a/test/integration/targets/jinja_plugins/collections/ansible_collections/foo/bar/plugins/filter/bad_collection_filter.py b/test/integration/targets/jinja_plugins/collections/ansible_collections/foo/bar/plugins/filter/bad_collection_filter.py new file mode 100644 index 00000000000..366695321a0 --- /dev/null +++ b/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 +# 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') diff --git a/test/integration/targets/jinja_plugins/collections/ansible_collections/foo/bar/plugins/filter/good_collection_filter.py b/test/integration/targets/jinja_plugins/collections/ansible_collections/foo/bar/plugins/filter/good_collection_filter.py new file mode 100644 index 00000000000..e2e7ffcd5ce --- /dev/null +++ b/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 +# 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, + } diff --git a/test/integration/targets/jinja_plugins/collections/ansible_collections/foo/bar/plugins/test/bad_collection_test.py b/test/integration/targets/jinja_plugins/collections/ansible_collections/foo/bar/plugins/test/bad_collection_test.py new file mode 100644 index 00000000000..9fce558107e --- /dev/null +++ b/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 +# 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') diff --git a/test/integration/targets/jinja_plugins/collections/ansible_collections/foo/bar/plugins/test/good_collection_test.py b/test/integration/targets/jinja_plugins/collections/ansible_collections/foo/bar/plugins/test/good_collection_test.py new file mode 100644 index 00000000000..a4ca2ff2f7c --- /dev/null +++ b/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 +# 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', + } diff --git a/test/integration/targets/jinja_plugins/filter_plugins/bad_filter.py b/test/integration/targets/jinja_plugins/filter_plugins/bad_filter.py new file mode 100644 index 00000000000..eebf39c9a47 --- /dev/null +++ b/test/integration/targets/jinja_plugins/filter_plugins/bad_filter.py @@ -0,0 +1,11 @@ +# Copyright (c) 2021 Matt Martz +# 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') diff --git a/test/integration/targets/jinja_plugins/filter_plugins/good_filter.py b/test/integration/targets/jinja_plugins/filter_plugins/good_filter.py new file mode 100644 index 00000000000..e2e7ffcd5ce --- /dev/null +++ b/test/integration/targets/jinja_plugins/filter_plugins/good_filter.py @@ -0,0 +1,13 @@ +# Copyright (c) 2021 Matt Martz +# 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, + } diff --git a/test/integration/targets/jinja_plugins/playbook.yml b/test/integration/targets/jinja_plugins/playbook.yml new file mode 100644 index 00000000000..789be659aa3 --- /dev/null +++ b/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' diff --git a/test/integration/targets/jinja_plugins/tasks/main.yml b/test/integration/targets/jinja_plugins/tasks/main.yml new file mode 100644 index 00000000000..012ec9542cf --- /dev/null +++ b/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 diff --git a/test/integration/targets/jinja_plugins/test_plugins/bad_test.py b/test/integration/targets/jinja_plugins/test_plugins/bad_test.py new file mode 100644 index 00000000000..0cc7a5a8857 --- /dev/null +++ b/test/integration/targets/jinja_plugins/test_plugins/bad_test.py @@ -0,0 +1,11 @@ +# Copyright (c) 2021 Matt Martz +# 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') diff --git a/test/integration/targets/jinja_plugins/test_plugins/good_test.py b/test/integration/targets/jinja_plugins/test_plugins/good_test.py new file mode 100644 index 00000000000..a4ca2ff2f7c --- /dev/null +++ b/test/integration/targets/jinja_plugins/test_plugins/good_test.py @@ -0,0 +1,13 @@ +# Copyright (c) 2021 Matt Martz +# 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', + }