From ff1ba39c8af0bb1e28932823814bbdd5aa45cd37 Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Tue, 14 Apr 2020 10:27:02 +0200 Subject: [PATCH] Prevent templating unused variables for {%include%} (#68749) Fixes #68699 --- ...mplating-all-vars-when-copying-context.yml | 2 + lib/ansible/template/__init__.py | 38 ++++++++++++++++++- lib/ansible/template/template.py | 4 +- test/integration/targets/template/runme.sh | 3 ++ .../template/templates/unused_vars_include.j2 | 1 + .../templates/unused_vars_template.j2 | 2 + .../targets/template/unused_vars_include.yml | 8 ++++ 7 files changed, 55 insertions(+), 3 deletions(-) create mode 100644 changelogs/fragments/68699-prevent-templating-all-vars-when-copying-context.yml create mode 100644 test/integration/targets/template/templates/unused_vars_include.j2 create mode 100644 test/integration/targets/template/templates/unused_vars_template.j2 create mode 100644 test/integration/targets/template/unused_vars_include.yml diff --git a/changelogs/fragments/68699-prevent-templating-all-vars-when-copying-context.yml b/changelogs/fragments/68699-prevent-templating-all-vars-when-copying-context.yml new file mode 100644 index 00000000000..11ed78f59a7 --- /dev/null +++ b/changelogs/fragments/68699-prevent-templating-all-vars-when-copying-context.yml @@ -0,0 +1,2 @@ +bugfixes: + - "Prevent templating unused variables for {% include %} (https://github.com/ansible/ansible/issues/68699)" diff --git a/lib/ansible/template/__init__.py b/lib/ansible/template/__init__.py index fe20267e017..10fa6e128b1 100644 --- a/lib/ansible/template/__init__.py +++ b/lib/ansible/template/__init__.py @@ -28,6 +28,7 @@ import re import time from contextlib import contextmanager +from distutils.version import LooseVersion from numbers import Number try: @@ -67,6 +68,8 @@ NON_TEMPLATED_TYPES = (bool, Number) JINJA2_OVERRIDE = '#jinja2:' +from jinja2 import __version__ as j2_version + USE_JINJA2_NATIVE = False if C.DEFAULT_JINJA2_NATIVE: try: @@ -76,7 +79,6 @@ if C.DEFAULT_JINJA2_NATIVE: except ImportError: from jinja2 import Environment from jinja2.utils import concat as j2_concat - from jinja2 import __version__ as j2_version display.warning( 'jinja2_native requires Jinja 2.10 and above. ' 'Version detected: %s. Falling back to default.' % j2_version @@ -295,6 +297,40 @@ class AnsibleContext(Context): self._update_unsafe(val) return val + def get_all(self): + """Return the complete context as a dict including the exported + variables. For optimizations reasons this might not return an + actual copy so be careful with using it. + + This is to prevent from running ``AnsibleJ2Vars`` through dict(): + + ``dict(self.parent, **self.vars)`` + + In Ansible this means that ALL variables would be templated in the + process of re-creating the parent because ``AnsibleJ2Vars`` templates + each variable in its ``__getitem__`` method. Instead we re-create the + parent via ``AnsibleJ2Vars.add_locals`` that creates a new + ``AnsibleJ2Vars`` copy without templating each variable. + + This will prevent unnecessarily templating unused variables in cases + like setting a local variable and passing it to {% include %} + in a template. + + Also see ``AnsibleJ2Template``and + https://github.com/pallets/jinja/commit/d67f0fd4cc2a4af08f51f4466150d49da7798729 + """ + if LooseVersion(j2_version) >= LooseVersion('2.9'): + if not self.vars: + return self.parent + if not self.parent: + return self.vars + + if isinstance(self.parent, AnsibleJ2Vars): + return self.parent.add_locals(self.vars) + else: + # can this happen in Ansible? + return dict(self.parent, **self.vars) + class JinjaPluginIntercept(MutableMapping): def __init__(self, delegatee, pluginloader, *args, **kwargs): diff --git a/lib/ansible/template/template.py b/lib/ansible/template/template.py index fe11471d6c8..5a555883a02 100644 --- a/lib/ansible/template/template.py +++ b/lib/ansible/template/template.py @@ -26,9 +26,9 @@ __all__ = ['AnsibleJ2Template'] class AnsibleJ2Template(jinja2.environment.Template): ''' - A helper class, which prevents Jinja2 from running _jinja2_vars through dict(). + A helper class, which prevents Jinja2 from running AnsibleJ2Vars through dict(). Without this, {% include %} and similar will create new contexts unlike the special - one created in template_from_file. This ensures they are all alike, except for + one created in Templar.template. This ensures they are all alike, except for potential locals. ''' diff --git a/test/integration/targets/template/runme.sh b/test/integration/targets/template/runme.sh index abdd62f28b0..61511ac82f1 100755 --- a/test/integration/targets/template/runme.sh +++ b/test/integration/targets/template/runme.sh @@ -19,3 +19,6 @@ ansible-playbook corner_cases.yml -v "$@" # Test for #57351 ansible-playbook filter_plugins.yml -v "$@" + +# https://github.com/ansible/ansible/issues/68699 +ansible-playbook unused_vars_include.yml -v "$@" diff --git a/test/integration/targets/template/templates/unused_vars_include.j2 b/test/integration/targets/template/templates/unused_vars_include.j2 new file mode 100644 index 00000000000..457cbbc0e1f --- /dev/null +++ b/test/integration/targets/template/templates/unused_vars_include.j2 @@ -0,0 +1 @@ +{{ var_set_in_template }} diff --git a/test/integration/targets/template/templates/unused_vars_template.j2 b/test/integration/targets/template/templates/unused_vars_template.j2 new file mode 100644 index 00000000000..28afc902140 --- /dev/null +++ b/test/integration/targets/template/templates/unused_vars_template.j2 @@ -0,0 +1,2 @@ +{% set var_set_in_template=test_var %} +{% include "unused_vars_include.j2" %} diff --git a/test/integration/targets/template/unused_vars_include.yml b/test/integration/targets/template/unused_vars_include.yml new file mode 100644 index 00000000000..ff31b70d3cc --- /dev/null +++ b/test/integration/targets/template/unused_vars_include.yml @@ -0,0 +1,8 @@ +- hosts: localhost + gather_facts: no + vars: + test_var: foo + unused_var: "{{ undefined_var }}" + tasks: + - debug: + msg: "{{ lookup('template', 'unused_vars_template.j2') }}"