From 26707a3c6bd1bc72f59ce6efb188e94c4f58ce86 Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Wed, 27 Oct 2021 18:36:33 +0200 Subject: [PATCH] Heisen jinja2_native (#75587) * Use NativeEnvironment for all templating ci_complete * Keep Templar.copy_with_new_env for backwards compat * Mention that AnsibleUndefined.__repr__ changed in the porting guide * Templar.copy_with_new_env backwards compat * ci_complete --- ...-safe-eval-replaced-native_environment.yml | 6 + .../porting_guide_core_2.13.rst | 14 +- lib/ansible/config/base.yml | 23 --- lib/ansible/playbook/collectionsearch.py | 6 +- lib/ansible/playbook/conditional.py | 4 +- lib/ansible/plugins/action/template.py | 23 ++- lib/ansible/plugins/lookup/template.py | 24 ++-- lib/ansible/template/__init__.py | 134 ++++++++---------- lib/ansible/template/native_helpers.py | 65 ++++++++- lib/ansible/template/safe_eval.py | 129 ----------------- .../targets/template/corner_cases.yml | 4 + .../files/string_type_filters.expected | 4 + .../targets/template/tasks/main.yml | 22 +++ .../targets/templating_lookups/runme.sh | 3 - .../playbook.yml | 8 -- .../template.json.j2 | 4 - .../targets/undefined/tasks/main.yml | 32 ++--- test/units/template/test_native_concat.py | 36 +---- test/units/template/test_safe_eval.py | 44 ------ test/units/template/test_templar.py | 3 +- 20 files changed, 215 insertions(+), 373 deletions(-) create mode 100644 changelogs/fragments/templating-safe-eval-replaced-native_environment.yml delete mode 100644 lib/ansible/template/safe_eval.py create mode 100644 test/integration/targets/template/files/string_type_filters.expected delete mode 100644 test/integration/targets/templating_lookups/template_lookup_safe_eval_unicode/playbook.yml delete mode 100644 test/integration/targets/templating_lookups/template_lookup_safe_eval_unicode/template.json.j2 delete mode 100644 test/units/template/test_safe_eval.py diff --git a/changelogs/fragments/templating-safe-eval-replaced-native_environment.yml b/changelogs/fragments/templating-safe-eval-replaced-native_environment.yml new file mode 100644 index 00000000000..1114079bc48 --- /dev/null +++ b/changelogs/fragments/templating-safe-eval-replaced-native_environment.yml @@ -0,0 +1,6 @@ +major_changes: + - Templating - remove ``safe_eval`` in favor of using ``NativeEnvironment`` but utilizing ``literal_eval`` only in cases when ``safe_eval`` was used (https://github.com/ansible/ansible/pull/75587) +breaking_changes: + - Templating - it is no longer allowed to perform arithmetic and concatenation operations outside of the jinja template (https://github.com/ansible/ansible/pull/75587) +bugfixes: + - Trigger an undefined error when an undefined variable is detected within a dictionary and/or list (https://github.com/ansible/ansible/pull/75587) diff --git a/docs/docsite/rst/porting_guides/porting_guide_core_2.13.rst b/docs/docsite/rst/porting_guides/porting_guide_core_2.13.rst index 39a286efa91..8db1d9a3f95 100644 --- a/docs/docsite/rst/porting_guides/porting_guide_core_2.13.rst +++ b/docs/docsite/rst/porting_guides/porting_guide_core_2.13.rst @@ -19,7 +19,19 @@ This document is part of a collection on porting. The complete list of porting g Playbook ======== -No notable changes +* Templating - You can no longer perform arithmetic and concatenation operations outside of the jinja template. The following statement will need to be rewritten to produce ``[1, 2]``: + + .. code-block:: yaml + + - name: Prior to 2.13 + debug: + msg: '[1] + {{ [2] }}' + + - name: 2.13 and forward + debug: + msg: '{{ [1] + [2] }}' + +* The return value of the ``__repr__`` method of an undefined variable represented by the ``AnsibleUndefined`` object changed. ``{{ '%r'|format(undefined_variable) }}`` returns ``AnsibleUndefined(hint=None, obj=missing, name='undefined_variable')`` in 2.13 as opposed to just ``AnsibleUndefined`` in versions 2.12 and prior. Command Line diff --git a/lib/ansible/config/base.yml b/lib/ansible/config/base.yml index 8a58a947410..70e27fc0599 100644 --- a/lib/ansible/config/base.yml +++ b/lib/ansible/config/base.yml @@ -535,29 +535,6 @@ DEFAULT_CACHE_PLUGIN_PATH: ini: - {key: cache_plugins, section: defaults} type: pathspec -CALLABLE_ACCEPT_LIST: - name: Template 'callable' accept list - default: [] - description: Whitelist of callable methods to be made available to template evaluation - env: - - name: ANSIBLE_CALLABLE_WHITELIST - deprecated: - why: normalizing names to new standard - version: "2.15" - alternatives: 'ANSIBLE_CALLABLE_ENABLED' - - name: ANSIBLE_CALLABLE_ENABLED - version_added: '2.11' - ini: - - key: callable_whitelist - section: defaults - deprecated: - why: normalizing names to new standard - version: "2.15" - alternatives: 'callable_enabled' - - key: callable_enabled - section: defaults - version_added: '2.11' - type: list DEFAULT_CALLBACK_PLUGIN_PATH: name: Callback Plugins Path default: ~/.ansible/plugins/callback:/usr/share/ansible/plugins/callback diff --git a/lib/ansible/playbook/collectionsearch.py b/lib/ansible/playbook/collectionsearch.py index fb69519b40d..f319cfe03a2 100644 --- a/lib/ansible/playbook/collectionsearch.py +++ b/lib/ansible/playbook/collectionsearch.py @@ -7,9 +7,11 @@ __metaclass__ = type from ansible.module_utils.six import string_types from ansible.playbook.attribute import FieldAttribute from ansible.utils.collection_loader import AnsibleCollectionConfig -from ansible.template import is_template, Environment +from ansible.template import is_template from ansible.utils.display import Display +from jinja2.nativetypes import NativeEnvironment + display = Display() @@ -52,7 +54,7 @@ class CollectionSearch: # because if the user attempts to template a collection name, it may # error before it ever gets to the post_validate() warning (e.g. trying # to import a role from the collection). - env = Environment() + env = NativeEnvironment() for collection_name in ds: if is_template(collection_name, env): display.warning('"collections" is not templatable, but we found: %s, ' diff --git a/lib/ansible/playbook/conditional.py b/lib/ansible/playbook/conditional.py index 7afcb41c06a..f3118a81b29 100644 --- a/lib/ansible/playbook/conditional.py +++ b/lib/ansible/playbook/conditional.py @@ -181,8 +181,8 @@ class Conditional: raise AnsibleError("Invalid conditional detected: %s" % to_native(e)) # and finally we generate and template the presented string and look at the resulting string - # NOTE The spaces around True and False are intentional to short-circuit safe_eval and avoid - # its expensive calls. + # NOTE The spaces around True and False are intentional to short-circuit literal_eval for + # jinja2_native=False and avoid its expensive calls. presented = "{%% if %s %%} True {%% else %%} False {%% endif %%}" % conditional # NOTE Convert the result to text to account for both native and non-native jinja. # NOTE The templated result of `presented` is string on native jinja as well prior to Python 3.10. diff --git a/lib/ansible/plugins/action/template.py b/lib/ansible/plugins/action/template.py index d4b26e70349..3773d07f40c 100644 --- a/lib/ansible/plugins/action/template.py +++ b/lib/ansible/plugins/action/template.py @@ -121,21 +121,14 @@ class ActionModule(ActionBase): temp_vars = task_vars.copy() temp_vars.update(generate_ansible_template_vars(self._task.args.get('src', None), source, dest)) - # force templar to use AnsibleEnvironment to prevent issues with native types - # https://github.com/ansible/ansible/issues/46169 - templar = self._templar.copy_with_new_env(environment_class=AnsibleEnvironment, - searchpath=searchpath, - newline_sequence=newline_sequence, - block_start_string=block_start_string, - block_end_string=block_end_string, - variable_start_string=variable_start_string, - variable_end_string=variable_end_string, - comment_start_string=comment_start_string, - comment_end_string=comment_end_string, - trim_blocks=trim_blocks, - lstrip_blocks=lstrip_blocks, - available_variables=temp_vars) - resultant = templar.do_template(template_data, preserve_trailing_newlines=True, escape_backslashes=False) + # force jinja2_native=False to prevent issues with native types: https://github.com/ansible/ansible/issues/46169 + with self._templar.set_temporary_context(searchpath=searchpath, newline_sequence=newline_sequence, + block_start_string=block_start_string, block_end_string=block_end_string, + variable_start_string=variable_start_string, variable_end_string=variable_end_string, + comment_start_string=comment_start_string, comment_end_string=comment_end_string, + trim_blocks=trim_blocks, lstrip_blocks=lstrip_blocks, + available_variables=temp_vars, jinja2_native=False): + resultant = self._templar.do_template(template_data, preserve_trailing_newlines=True, escape_backslashes=False) except AnsibleAction: raise except Exception as e: diff --git a/lib/ansible/plugins/lookup/template.py b/lib/ansible/plugins/lookup/template.py index 9e57e66fcc2..08823680bdf 100644 --- a/lib/ansible/plugins/lookup/template.py +++ b/lib/ansible/plugins/lookup/template.py @@ -86,9 +86,7 @@ from ansible.plugins.lookup import LookupBase from ansible.module_utils._text import to_bytes, to_text from ansible.template import generate_ansible_template_vars, AnsibleEnvironment from ansible.utils.display import Display - -if C.DEFAULT_JINJA2_NATIVE: - from ansible.utils.native_jinja import NativeJinjaText +from ansible.utils.native_jinja import NativeJinjaText display = Display() @@ -111,11 +109,6 @@ class LookupModule(LookupBase): comment_start_string = self.get_option('comment_start_string') comment_end_string = self.get_option('comment_end_string') - if C.DEFAULT_JINJA2_NATIVE and not jinja2_native: - templar = self._templar.copy_with_new_env(environment_class=AnsibleEnvironment) - else: - templar = self._templar - for term in terms: display.debug("File lookup term: %s" % term) @@ -146,13 +139,14 @@ class LookupModule(LookupBase): vars.update(generate_ansible_template_vars(term, lookupfile)) vars.update(lookup_template_vars) - with templar.set_temporary_context(variable_start_string=variable_start_string, - variable_end_string=variable_end_string, - comment_start_string=comment_start_string, - comment_end_string=comment_end_string, - available_variables=vars, searchpath=searchpath): - res = templar.template(template_data, preserve_trailing_newlines=True, - convert_data=convert_data_p, escape_backslashes=False) + with self._templar.set_temporary_context(variable_start_string=variable_start_string, + variable_end_string=variable_end_string, + comment_start_string=comment_start_string, + comment_end_string=comment_end_string, + available_variables=vars, searchpath=searchpath, + jinja2_native=jinja2_native): + res = self._templar.template(template_data, preserve_trailing_newlines=True, + convert_data=convert_data_p, escape_backslashes=False) if C.DEFAULT_JINJA2_NATIVE and not jinja2_native: # jinja2_native is true globally but off for the lookup, we need this text diff --git a/lib/ansible/template/__init__.py b/lib/ansible/template/__init__.py index f8b46cb6fef..1f616fac9be 100644 --- a/lib/ansible/template/__init__.py +++ b/lib/ansible/template/__init__.py @@ -38,6 +38,7 @@ except ImportError: from jinja2.exceptions import TemplateSyntaxError, UndefinedError from jinja2.loaders import FileSystemLoader +from jinja2.nativetypes import NativeEnvironment from jinja2.runtime import Context, StrictUndefined from ansible import constants as C @@ -56,13 +57,14 @@ from ansible.module_utils.common._collections_compat import Iterator, Sequence, from ansible.module_utils.common.collections import is_sequence from ansible.module_utils.compat.importlib import import_module from ansible.plugins.loader import filter_loader, lookup_loader, test_loader -from ansible.template.safe_eval import safe_eval +from ansible.template.native_helpers import ansible_native_concat, ansible_concat from ansible.template.template import AnsibleJ2Template from ansible.template.vars import AnsibleJ2Vars from ansible.utils.collection_loader import AnsibleCollectionRef from ansible.utils.display import Display from ansible.utils.collection_loader._collection_finder import _get_collection_metadata from ansible.utils.listify import listify_lookup_plugin_terms +from ansible.utils.native_jinja import NativeJinjaText from ansible.utils.unsafe_proxy import wrap_var display = Display() @@ -70,28 +72,14 @@ display = Display() __all__ = ['Templar', 'generate_ansible_template_vars'] -# A regex for checking to see if a variable we're trying to -# expand is just a single variable name. - # Primitive Types which we don't want Jinja to convert to strings. NON_TEMPLATED_TYPES = (bool, Number) JINJA2_OVERRIDE = '#jinja2:' -from jinja2 import Environment -from jinja2.utils import concat as j2_concat - - -if C.DEFAULT_JINJA2_NATIVE: - from jinja2.nativetypes import NativeEnvironment - from ansible.template.native_helpers import ansible_native_concat - from ansible.utils.native_jinja import NativeJinjaText - - JINJA2_BEGIN_TOKENS = frozenset(('variable_begin', 'block_begin', 'comment_begin', 'raw_begin')) JINJA2_END_TOKENS = frozenset(('variable_end', 'block_end', 'comment_end', 'raw_end')) - RANGE_TYPE = type(range(0)) @@ -336,7 +324,11 @@ class AnsibleUndefined(StrictUndefined): return self def __repr__(self): - return 'AnsibleUndefined' + return 'AnsibleUndefined(hint={0!r}, obj={1!r}, name={2!r})'.format( + self._undefined_hint, + self._undefined_obj, + self._undefined_name + ) def __contains__(self, item): # Return original Undefined object to preserve the first failure context @@ -426,11 +418,10 @@ class AnsibleContext(Context): class JinjaPluginIntercept(MutableMapping): - def __init__(self, delegatee, pluginloader, jinja2_native, *args, **kwargs): + def __init__(self, delegatee, pluginloader, *args, **kwargs): super(JinjaPluginIntercept, self).__init__(*args, **kwargs) self._delegatee = delegatee self._pluginloader = pluginloader - self._jinja2_native = jinja2_native if self._pluginloader.class_name == 'FilterModule': self._method_map_name = 'filters' @@ -457,7 +448,7 @@ class JinjaPluginIntercept(MutableMapping): if self._pluginloader.class_name == 'FilterModule': for plugin_name, plugin in self._delegatee.items(): - if self._jinja2_native and plugin_name in C.STRING_TYPE_FILTERS: + if plugin_name in C.STRING_TYPE_FILTERS: self._delegatee[plugin_name] = _wrap_native_text(plugin) else: self._delegatee[plugin_name] = _unroll_iterator(plugin) @@ -572,7 +563,7 @@ class JinjaPluginIntercept(MutableMapping): fq_name = '.'.join((parent_prefix, func_name)) # FIXME: detect/warn on intra-collection function name collisions if self._pluginloader.class_name == 'FilterModule': - if self._jinja2_native and fq_name.startswith(('ansible.builtin.', 'ansible.legacy.')) and \ + if fq_name.startswith(('ansible.builtin.', 'ansible.legacy.')) and \ func_name in C.STRING_TYPE_FILTERS: self._collection_jinja_func_cache[fq_name] = _wrap_native_text(func) else: @@ -606,13 +597,10 @@ class JinjaPluginIntercept(MutableMapping): return len(self._delegatee) -class AnsibleEnvironment(Environment): +class AnsibleEnvironment(NativeEnvironment): ''' Our custom environment, which simply allows us to override the class-level values for the Template and Context classes used by jinja2 internally. - - NOTE: Any changes to this class must be reflected in - :class:`AnsibleNativeEnvironment` as well. ''' context_class = AnsibleContext template_class = AnsibleJ2Template @@ -620,27 +608,17 @@ class AnsibleEnvironment(Environment): def __init__(self, *args, **kwargs): super(AnsibleEnvironment, self).__init__(*args, **kwargs) - self.filters = JinjaPluginIntercept(self.filters, filter_loader, jinja2_native=False) - self.tests = JinjaPluginIntercept(self.tests, test_loader, jinja2_native=False) + self.filters = JinjaPluginIntercept(self.filters, filter_loader) + self.tests = JinjaPluginIntercept(self.tests, test_loader) -if C.DEFAULT_JINJA2_NATIVE: - class AnsibleNativeEnvironment(NativeEnvironment): - ''' - Our custom environment, which simply allows us to override the class-level - values for the Template and Context classes used by jinja2 internally. - - NOTE: Any changes to this class must be reflected in - :class:`AnsibleEnvironment` as well. - ''' - context_class = AnsibleContext - template_class = AnsibleJ2Template - - def __init__(self, *args, **kwargs): - super(AnsibleNativeEnvironment, self).__init__(*args, **kwargs) - - self.filters = JinjaPluginIntercept(self.filters, filter_loader, jinja2_native=True) - self.tests = JinjaPluginIntercept(self.tests, test_loader, jinja2_native=True) +class AnsibleNativeEnvironment(NativeEnvironment): + def __new__(cls): + raise AnsibleAssertionError( + 'It is not allowed to create instances of AnsibleNativeEnvironment. ' + 'The class is kept for backwards compatibility of ' + 'Templar.copy_with_new_env, see the method for more information.' + ) class Templar: @@ -660,9 +638,7 @@ class Templar: self._fail_on_undefined_errors = C.DEFAULT_UNDEFINED_VAR_BEHAVIOR - environment_class = AnsibleNativeEnvironment if C.DEFAULT_JINJA2_NATIVE else AnsibleEnvironment - - self.environment = environment_class( + self.environment = AnsibleEnvironment( trim_blocks=True, undefined=AnsibleUndefined, extensions=self._get_extensions(), @@ -683,36 +659,54 @@ class Templar: # the current rendering context under which the templar class is working self.cur_context = None - # FIXME these regular expressions should be re-compiled each time variable_start_string and variable_end_string are changed + # FIXME this regex should be re-compiled each time variable_start_string and variable_end_string are changed self.SINGLE_VAR = re.compile(r"^%s\s*(\w*)\s*%s$" % (self.environment.variable_start_string, self.environment.variable_end_string)) - self._no_type_regex = re.compile(r'.*?\|\s*(?:%s)(?:\([^\|]*\))?\s*\)?\s*(?:%s)' % - ('|'.join(C.STRING_TYPE_FILTERS), self.environment.variable_end_string)) - @property - def jinja2_native(self): - return not isinstance(self.environment, AnsibleEnvironment) + self.jinja2_native = C.DEFAULT_JINJA2_NATIVE def copy_with_new_env(self, environment_class=AnsibleEnvironment, **kwargs): - r"""Creates a new copy of Templar with a new environment. The new environment is based on - given environment class and kwargs. + r"""Creates a new copy of Templar with a new environment. - :kwarg environment_class: Environment class used for creating a new environment. + Since Ansible 2.13 this method is being deprecated and is kept only + for backwards compatibility: + - AnsibleEnvironment is now based on NativeEnvironment + - AnsibleNativeEnvironment is replaced by what is effectively a dummy class + for purposes of this method, see below + - environment_class arg no longer controls what type of environment is created, + AnsibleEnvironment is used regardless of the value passed in environment_class + - environment_class is used to determine the value of jinja2_native of the newly + created Templar; if AnsibleNativeEnvironment is passed in environment_class + new_templar.jinja2_native is set to True, any other value will result in + new_templar.jinja2_native being set to False unless overriden by the value + passed in kwargs + + :kwarg environment_class: See above. :kwarg \*\*kwargs: Optional arguments for the new environment that override existing environment attributes. :returns: Copy of Templar with updated environment. """ + display.deprecated( + 'Templar.copy_with_new_env is no longer used within Ansible codebase and is being deprecated. ' + 'For temporarily creating a new environment with custom arguments use set_temporary_context context manager. ' + 'To control whether the Templar uses the jinja2_native functionality set/unset Templar.jinja2_native instance attribute.', + version='2.14', collection_name='ansible.builtin' + ) + # We need to use __new__ to skip __init__, mainly not to create a new # environment there only to override it below - new_env = object.__new__(environment_class) + new_env = object.__new__(AnsibleEnvironment) new_env.__dict__.update(self.environment.__dict__) new_templar = object.__new__(Templar) new_templar.__dict__.update(self.__dict__) new_templar.environment = new_env + new_templar.jinja2_native = environment_class is AnsibleNativeEnvironment + mapping = { 'available_variables': new_templar, + 'jinja2_native': self, 'searchpath': new_env.loader, } @@ -771,6 +765,7 @@ class Templar: """ mapping = { 'available_variables': self, + 'jinja2_native': self, 'searchpath': self.environment.loader, } original = {} @@ -851,21 +846,9 @@ class Templar: fail_on_undefined=fail_on_undefined, overrides=overrides, disable_lookups=disable_lookups, + convert_data=convert_data, ) - if not self.jinja2_native: - unsafe = hasattr(result, '__UNSAFE__') - if convert_data and not self._no_type_regex.match(variable): - # if this looks like a dictionary or list, convert it to such using the safe_eval method - if (result.startswith("{") and not result.startswith(self.environment.variable_start_string)) or \ - result.startswith("[") or result in ("True", "False"): - eval_results = safe_eval(result, include_exceptions=True) - if eval_results[1] is None: - result = eval_results[0] - if unsafe: - result = wrap_var(result) - # FIXME: if the safe_eval raised an error, should we do something with it? - # we only cache in the case where we have a single variable # name, to make sure we're not putting things which may otherwise # be dynamic in the cache (filters, lookups, etc.) @@ -1027,7 +1010,7 @@ class Templar: return wrap_var(ran) try: - if self.jinja2_native and isinstance(ran[0], NativeJinjaText): + if isinstance(ran[0], NativeJinjaText): ran = wrap_var(NativeJinjaText(",".join(ran))) else: ran = wrap_var(",".join(ran)) @@ -1054,7 +1037,8 @@ class Templar: hint = "Mandatory variable has not been overridden" return AnsibleUndefined(hint) - def do_template(self, data, preserve_trailing_newlines=True, escape_backslashes=True, fail_on_undefined=None, overrides=None, disable_lookups=False): + def do_template(self, data, preserve_trailing_newlines=True, escape_backslashes=True, fail_on_undefined=None, overrides=None, disable_lookups=False, + convert_data=False): if self.jinja2_native and not isinstance(data, string_types): return data @@ -1114,7 +1098,8 @@ class Templar: if self.jinja2_native: res = ansible_native_concat(rf) else: - res = j2_concat(rf) + res = ansible_concat(rf, convert_data, myenv.variable_start_string) + unsafe = getattr(new_context, 'unsafe', False) if unsafe: res = wrap_var(res) @@ -1127,10 +1112,7 @@ class Templar: display.debug("failing because of a type error, template data is: %s" % to_text(data)) raise AnsibleError("Unexpected templating type error occurred on (%s): %s" % (to_native(data), to_native(te))) - if self.jinja2_native and not isinstance(res, string_types): - return res - - if preserve_trailing_newlines: + if isinstance(res, string_types) and preserve_trailing_newlines: # The low level calls above do not preserve the newline # characters at the end of the input data, so we use the # calculate the difference in newlines and append them diff --git a/lib/ansible/template/native_helpers.py b/lib/ansible/template/native_helpers.py index 1f6070ccdd7..63d6bdc6712 100644 --- a/lib/ansible/template/native_helpers.py +++ b/lib/ansible/template/native_helpers.py @@ -6,7 +6,7 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type -from ast import literal_eval +import ast from itertools import islice, chain from jinja2.runtime import StrictUndefined @@ -16,6 +16,21 @@ from ansible.module_utils.common.collections import is_sequence, Mapping from ansible.module_utils.six import string_types from ansible.parsing.yaml.objects import AnsibleVaultEncryptedUnicode from ansible.utils.native_jinja import NativeJinjaText +from ansible.utils.unsafe_proxy import wrap_var + + +_JSON_MAP = { + "true": True, + "false": False, + "null": None, +} + + +class Json2Python(ast.NodeTransformer): + def visit_Name(self, node): + if node.id not in _JSON_MAP: + return node + return ast.Constant(value=_JSON_MAP[node.id]) def _fail_on_undefined(data): @@ -41,6 +56,52 @@ def _fail_on_undefined(data): return data +def ansible_concat(nodes, convert_data, variable_start_string): + head = list(islice(nodes, 2)) + + if not head: + return None + + if len(head) == 1: + out = _fail_on_undefined(head[0]) + + if isinstance(out, NativeJinjaText): + return out + + out = to_text(out) + else: + out = ''.join([to_text(_fail_on_undefined(v)) for v in chain(head, nodes)]) + + if not convert_data: + return out + + # if this looks like a dictionary, list or bool, convert it to such + do_eval = ( + ( + out.startswith(('{', '[')) and + not out.startswith(variable_start_string) + ) or + out in ('True', 'False') + ) + if do_eval: + unsafe = hasattr(out, '__UNSAFE__') + try: + out = ast.literal_eval( + ast.fix_missing_locations( + Json2Python().visit( + ast.parse(out, mode='eval') + ) + ) + ) + except (ValueError, SyntaxError, MemoryError): + pass + else: + if unsafe: + out = wrap_var(out) + + return out + + def ansible_native_concat(nodes): """Return a native Python type from the list of compiled nodes. If the result is a single node, its value is returned. Otherwise, the nodes are @@ -79,6 +140,6 @@ def ansible_native_concat(nodes): out = ''.join([to_text(_fail_on_undefined(v)) for v in chain(head, nodes)]) try: - return literal_eval(out) + return ast.literal_eval(out) except (ValueError, SyntaxError, MemoryError): return out diff --git a/lib/ansible/template/safe_eval.py b/lib/ansible/template/safe_eval.py deleted file mode 100644 index 5510505b9ab..00000000000 --- a/lib/ansible/template/safe_eval.py +++ /dev/null @@ -1,129 +0,0 @@ -# (c) 2012, Michael DeHaan -# -# This file is part of Ansible -# -# Ansible is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# Ansible is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with Ansible. If not, see . - -from __future__ import (absolute_import, division, print_function) -__metaclass__ = type - -import ast -import builtins - -from ansible.module_utils.common.text.converters import to_native -from ansible.module_utils.six import string_types - - -def safe_eval(expr, locals=None, include_exceptions=False): - ''' - This is intended for allowing things like: - with_items: a_list_variable - - Where Jinja2 would return a string but we do not want to allow it to - call functions (outside of Jinja2, where the env is constrained). - - Based on: - http://stackoverflow.com/questions/12523516/using-ast-and-whitelists-to-make-pythons-eval-safe - ''' - locals = {} if locals is None else locals - - # define certain JSON types - # eg. JSON booleans are unknown to python eval() - OUR_GLOBALS = { - '__builtins__': {}, # avoid global builtins as per eval docs - 'false': False, - 'null': None, - 'true': True, - # also add back some builtins we do need - 'True': True, - 'False': False, - 'None': None - } - - # this is the whitelist of AST nodes we are going to - # allow in the evaluation. Any node type other than - # those listed here will raise an exception in our custom - # visitor class defined below. - SAFE_NODES = set( - ( - ast.Add, - ast.BinOp, - # ast.Call, - ast.Compare, - ast.Constant, - ast.Dict, - ast.Div, - ast.Expression, - ast.List, - ast.Load, - ast.Mult, - ast.Num, - ast.Name, - ast.Set, - ast.Str, - ast.Sub, - ast.USub, - ast.Tuple, - ast.UnaryOp, - ) - ) - - CALL_ENABLED = [] - - class CleansingNodeVisitor(ast.NodeVisitor): - def generic_visit(self, node, inside_call=False): - if type(node) not in SAFE_NODES: - raise Exception("invalid expression (%s)" % expr) - elif isinstance(node, ast.Call): - inside_call = True - elif isinstance(node, ast.Name) and inside_call: - # Disallow calls to builtin functions that we have not vetted - # as safe. Other functions are excluded by setting locals in - # the call to eval() later on - if hasattr(builtins, node.id) and node.id not in CALL_ENABLED: - raise Exception("invalid function: %s" % node.id) - # iterate over all child nodes - for child_node in ast.iter_child_nodes(node): - self.generic_visit(child_node, inside_call) - - if not isinstance(expr, string_types): - # already templated to a datastructure, perhaps? - if include_exceptions: - return (expr, None) - return expr - - cnv = CleansingNodeVisitor() - try: - parsed_tree = ast.parse(expr, mode='eval') - cnv.visit(parsed_tree) - compiled = compile(parsed_tree, '' % to_native(expr), 'eval') - # Note: passing our own globals and locals here constrains what - # callables (and other identifiers) are recognized. this is in - # addition to the filtering of builtins done in CleansingNodeVisitor - result = eval(compiled, OUR_GLOBALS, dict(locals)) - - if include_exceptions: - return (result, None) - else: - return result - except SyntaxError as e: - # special handling for syntax errors, we just return - # the expression string back as-is to support late evaluation - if include_exceptions: - return (expr, None) - return expr - except Exception as e: - if include_exceptions: - return (expr, e) - return expr diff --git a/test/integration/targets/template/corner_cases.yml b/test/integration/targets/template/corner_cases.yml index 48782f79590..9d41ed94d90 100644 --- a/test/integration/targets/template/corner_cases.yml +++ b/test/integration/targets/template/corner_cases.yml @@ -12,10 +12,14 @@ - '"I SHOULD NOT BE TEMPLATED" not in adjacent' - globals1 == "[[], globals()]" - globals2 == "[[], globals]" + - left_hand == '[1] + [2]' + - left_hand_2 == '[1 + 2 * 3 / 4] + [-2.5, 2.5, 3.5]' vars: adjacent: "{{ empty_list }} + [dont]" globals1: "[{{ empty_list }}, globals()]" globals2: "[{{ empty_list }}, globals]" + left_hand: '[1] + {{ [2] }}' + left_hand_2: '[1 + 2 * 3 / 4] + {{ [-2.5, +2.5, 1 + 2.5] }}' - name: 'ensure we can add lists' assert: diff --git a/test/integration/targets/template/files/string_type_filters.expected b/test/integration/targets/template/files/string_type_filters.expected new file mode 100644 index 00000000000..989c356ee02 --- /dev/null +++ b/test/integration/targets/template/files/string_type_filters.expected @@ -0,0 +1,4 @@ +{ + "foo": "bar", + "foobar": 1 +} diff --git a/test/integration/targets/template/tasks/main.yml b/test/integration/targets/template/tasks/main.yml index 67d07b146b7..d97aa4d550d 100644 --- a/test/integration/targets/template/tasks/main.yml +++ b/test/integration/targets/template/tasks/main.yml @@ -734,3 +734,25 @@ # aliases file requires root for template tests so this should be safe - import_tasks: backup_test.yml + +- name: test STRING_TYPE_FILTERS + copy: + content: "{{ a_dict | to_nice_json(indent=(indent_value|int))}}\n" + dest: "{{ output_dir }}/string_type_filters.templated" + vars: + a_dict: + foo: bar + foobar: 1 + indent_value: 2 + +- name: copy known good string_type_filters.expected into place + copy: + src: string_type_filters.expected + dest: "{{ output_dir }}/string_type_filters.expected" + +- command: "diff {{ output_dir }}/string_type_filters.templated {{ output_dir}}/string_type_filters.expected" + register: out + +- assert: + that: + - out.rc == 0 diff --git a/test/integration/targets/templating_lookups/runme.sh b/test/integration/targets/templating_lookups/runme.sh index ebfc0d11354..b900c153de7 100755 --- a/test/integration/targets/templating_lookups/runme.sh +++ b/test/integration/targets/templating_lookups/runme.sh @@ -7,6 +7,3 @@ ANSIBLE_ROLES_PATH=./ UNICODE_VAR=café ansible-playbook runme.yml "$@" ansible-playbook template_lookup_vaulted/playbook.yml --vault-password-file template_lookup_vaulted/test_vault_pass "$@" ansible-playbook template_deepcopy/playbook.yml -i template_deepcopy/hosts "$@" - -# https://github.com/ansible/ansible/issues/66943 -ANSIBLE_JINJA2_NATIVE=0 ansible-playbook template_lookup_safe_eval_unicode/playbook.yml "$@" diff --git a/test/integration/targets/templating_lookups/template_lookup_safe_eval_unicode/playbook.yml b/test/integration/targets/templating_lookups/template_lookup_safe_eval_unicode/playbook.yml deleted file mode 100644 index 29e4b615245..00000000000 --- a/test/integration/targets/templating_lookups/template_lookup_safe_eval_unicode/playbook.yml +++ /dev/null @@ -1,8 +0,0 @@ -- hosts: localhost - gather_facts: no - vars: - original_dict: "{{ lookup('template', 'template.json.j2') }}" - copy_dict: {} - tasks: - - set_fact: - copy_dict: "{{ copy_dict | combine(original_dict) }}" diff --git a/test/integration/targets/templating_lookups/template_lookup_safe_eval_unicode/template.json.j2 b/test/integration/targets/templating_lookups/template_lookup_safe_eval_unicode/template.json.j2 deleted file mode 100644 index bc31407ccf4..00000000000 --- a/test/integration/targets/templating_lookups/template_lookup_safe_eval_unicode/template.json.j2 +++ /dev/null @@ -1,4 +0,0 @@ -{ - "key1": "ascii_value", - "key2": "unicode_value_křížek", -} diff --git a/test/integration/targets/undefined/tasks/main.yml b/test/integration/targets/undefined/tasks/main.yml index e50841ef80b..5bf478606cc 100644 --- a/test/integration/targets/undefined/tasks/main.yml +++ b/test/integration/targets/undefined/tasks/main.yml @@ -1,18 +1,16 @@ -- block: - - set_fact: - names: '{{ things|map(attribute="name") }}' - vars: - things: - - name: one - - name: two - - notname: three - - name: four +- set_fact: + names: '{{ things|map(attribute="name") }}' + vars: + things: + - name: one + - name: two + - notname: three + - name: four + ignore_errors: true + register: undefined_set_fact - - assert: - that: - - '"%r"|format(an_undefined_var) == "AnsibleUndefined"' - - '"%r"|format(undef()) == "AnsibleUndefined"' - # The existence of AnsibleUndefined in a templating result - # prevents safe_eval from turning the value into a python object - - names is string - - '", AnsibleUndefined," in names' +- assert: + that: + - '("%r"|format(undefined_variable)).startswith("AnsibleUndefined")' + - undefined_set_fact is failed + - undefined_set_fact.msg is contains 'undefined variable' diff --git a/test/units/template/test_native_concat.py b/test/units/template/test_native_concat.py index 4164bc4578c..ee1b7df1ea5 100644 --- a/test/units/template/test_native_concat.py +++ b/test/units/template/test_native_concat.py @@ -5,45 +5,21 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type -import importlib -import sys - -import pytest - -from ansible import constants as C -from ansible.errors import AnsibleUndefinedVariable from ansible.playbook.conditional import Conditional +from ansible.template import Templar from units.mock.loader import DictDataLoader -@pytest.fixture -def native_template_mod(monkeypatch): - monkeypatch.delitem(sys.modules, 'ansible.template') - monkeypatch.setattr(C, 'DEFAULT_JINJA2_NATIVE', True) - return importlib.import_module('ansible.template') - - -# https://github.com/ansible/ansible/issues/52158 -def test_undefined_variable(native_template_mod): - fake_loader = DictDataLoader({}) - variables = {} - templar = native_template_mod.Templar(loader=fake_loader, variables=variables) - assert isinstance(templar.environment, native_template_mod.AnsibleNativeEnvironment) - - with pytest.raises(AnsibleUndefinedVariable): - templar.template("{{ missing }}") - - -def test_cond_eval(native_template_mod): +def test_cond_eval(): fake_loader = DictDataLoader({}) # True must be stored in a variable to trigger templating. Using True # directly would be caught by optimization for bools to short-circuit # templating. variables = {"foo": True} - templar = native_template_mod.Templar(loader=fake_loader, variables=variables) - assert isinstance(templar.environment, native_template_mod.AnsibleNativeEnvironment) - + templar = Templar(loader=fake_loader, variables=variables) cond = Conditional(loader=fake_loader) cond.when = ["foo"] - assert cond.evaluate_conditional(templar, variables) + + with templar.set_temporary_context(jinja2_native=True): + assert cond.evaluate_conditional(templar, variables) diff --git a/test/units/template/test_safe_eval.py b/test/units/template/test_safe_eval.py deleted file mode 100644 index 89ff8a0e77b..00000000000 --- a/test/units/template/test_safe_eval.py +++ /dev/null @@ -1,44 +0,0 @@ -# (c) 2012-2014, Michael DeHaan -# -# This file is part of Ansible -# -# Ansible is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# Ansible is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with Ansible. If not, see . - -# Make coding more python3-ish -from __future__ import (absolute_import, division, print_function) -__metaclass__ = type - -import sys -from collections import defaultdict - -from units.compat import unittest -from ansible.template.safe_eval import safe_eval - - -class TestSafeEval(unittest.TestCase): - - def test_safe_eval_usage(self): - # test safe eval calls with different possible types for the - # locals dictionary, to ensure we don't run into problems like - # ansible/ansible/issues/12206 again - for locals_vars in (dict(), defaultdict(dict)): - self.assertEqual(safe_eval('True', locals=locals_vars), True) - self.assertEqual(safe_eval('False', locals=locals_vars), False) - self.assertEqual(safe_eval('0', locals=locals_vars), 0) - self.assertEqual(safe_eval('[]', locals=locals_vars), []) - self.assertEqual(safe_eval('{}', locals=locals_vars), {}) - - @unittest.skipUnless(sys.version_info[:2] >= (2, 7), "Python 2.6 has no set literals") - def test_set_literals(self): - self.assertEqual(safe_eval('{0}'), set([0])) diff --git a/test/units/template/test_templar.py b/test/units/template/test_templar.py index dd6985ce3bd..8dd0feb5bf6 100644 --- a/test/units/template/test_templar.py +++ b/test/units/template/test_templar.py @@ -187,8 +187,7 @@ class TestTemplarTemplate(BaseTemplar, unittest.TestCase): self.assertTrue(res) self.assertEqual(res, 'bar') - @patch('ansible.template.safe_eval', side_effect=AnsibleError) - def test_template_convert_data_template_in_data(self, mock_safe_eval): + def test_template_convert_data_template_in_data(self): res = self.templar.template('{{bam}}', convert_data=True) self.assertTrue(res) self.assertEqual(res, 'bar')