From 2c8c02c8168e938835844066570792fb59aad277 Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Sun, 7 Feb 2021 08:30:47 +0100 Subject: [PATCH] Local vars should have highest precedence in AnsibleJ2Vars (#72830) (#73370) Ability to add local variables into AnsibleJ2Vars was added in 18a9eff11f0a6e51b17405ce596bd9ff7e676320 to fix #6653. Local variables are added using ``AnsibleJ2Vars.add_locals()`` method when creating a new context - typically when including/importing a template with context. For that use case local template variables created using ``set`` should override variables from higher contexts - either from the play or any parent template, or both; Jinja behaves the same way. Also removes AnsibleJ2Vars.extras instance variable which is not used. Also adds missing test for #6653. Fixes #72262 Fixes #72615 ci_complete (cherry picked from commit a2af8432f36ec8cc5368a747f1211d2b9ba01f2e) --- .../72615-jinja-import-context-fix.yml | 2 ++ lib/ansible/template/vars.py | 36 ++++++++----------- test/integration/targets/template/6653.yml | 10 ++++++ test/integration/targets/template/72262.yml | 6 ++++ test/integration/targets/template/72615.yml | 26 ++++++++++++++ test/integration/targets/template/runme.sh | 9 +++++ .../template/templates/6653-include.j2 | 1 + .../targets/template/templates/6653.j2 | 4 +++ .../template/templates/72262-included.j2 | 1 + .../targets/template/templates/72262-vars.j2 | 1 + .../targets/template/templates/72262.j2 | 3 ++ .../template/templates/72615-macro-nested.j2 | 4 +++ .../targets/template/templates/72615-macro.j2 | 8 +++++ .../targets/template/templates/72615.j2 | 4 +++ 14 files changed, 93 insertions(+), 22 deletions(-) create mode 100644 changelogs/fragments/72615-jinja-import-context-fix.yml create mode 100644 test/integration/targets/template/6653.yml create mode 100644 test/integration/targets/template/72262.yml create mode 100644 test/integration/targets/template/72615.yml create mode 100644 test/integration/targets/template/templates/6653-include.j2 create mode 100644 test/integration/targets/template/templates/6653.j2 create mode 100644 test/integration/targets/template/templates/72262-included.j2 create mode 100644 test/integration/targets/template/templates/72262-vars.j2 create mode 100644 test/integration/targets/template/templates/72262.j2 create mode 100644 test/integration/targets/template/templates/72615-macro-nested.j2 create mode 100644 test/integration/targets/template/templates/72615-macro.j2 create mode 100644 test/integration/targets/template/templates/72615.j2 diff --git a/changelogs/fragments/72615-jinja-import-context-fix.yml b/changelogs/fragments/72615-jinja-import-context-fix.yml new file mode 100644 index 00000000000..0f4ff43141e --- /dev/null +++ b/changelogs/fragments/72615-jinja-import-context-fix.yml @@ -0,0 +1,2 @@ +bugfixes: + - Fix incorrect variable scoping when using ``import with context`` in Jinja2 templates. (https://github.com/ansible/ansible/issues/72615) diff --git a/lib/ansible/template/vars.py b/lib/ansible/template/vars.py index 464deecf808..513c732932a 100644 --- a/lib/ansible/template/vars.py +++ b/lib/ansible/template/vars.py @@ -40,7 +40,7 @@ class AnsibleJ2Vars(Mapping): To facilitate using builtin jinja2 things like range, globals are also handled here. ''' - def __init__(self, templar, globals, locals=None, *extras): + def __init__(self, templar, globals, locals=None): ''' Initializes this object with a valid Templar() object, as well as several dictionaries of variables representing @@ -49,7 +49,6 @@ class AnsibleJ2Vars(Mapping): self._templar = templar self._globals = globals - self._extras = extras self._locals = dict() if isinstance(locals, dict): for key, val in iteritems(locals): @@ -60,40 +59,33 @@ class AnsibleJ2Vars(Mapping): self._locals[key] = val def __contains__(self, k): - if k in self._templar.available_variables: - return True if k in self._locals: return True - for i in self._extras: - if k in i: - return True + if k in self._templar.available_variables: + return True if k in self._globals: return True return False def __iter__(self): keys = set() - keys.update(self._templar.available_variables, self._locals, self._globals, *self._extras) + keys.update(self._templar.available_variables, self._locals, self._globals) return iter(keys) def __len__(self): keys = set() - keys.update(self._templar.available_variables, self._locals, self._globals, *self._extras) + keys.update(self._templar.available_variables, self._locals, self._globals) return len(keys) def __getitem__(self, varname): - if varname not in self._templar.available_variables: - if varname in self._locals: - return self._locals[varname] - for i in self._extras: - if varname in i: - return i[varname] - if varname in self._globals: - return self._globals[varname] - else: - raise KeyError("undefined variable: %s" % varname) - - variable = self._templar.available_variables[varname] + if varname in self._locals: + return self._locals[varname] + if varname in self._templar.available_variables: + variable = self._templar.available_variables[varname] + elif varname in self._globals: + return self._globals[varname] + else: + raise KeyError("undefined variable: %s" % varname) # HostVars is special, return it as-is, as is the special variable # 'vars', which contains the vars structure @@ -127,4 +119,4 @@ class AnsibleJ2Vars(Mapping): new_locals = self._locals.copy() new_locals.update(locals) - return AnsibleJ2Vars(self._templar, self._globals, locals=new_locals, *self._extras) + return AnsibleJ2Vars(self._templar, self._globals, locals=new_locals) diff --git a/test/integration/targets/template/6653.yml b/test/integration/targets/template/6653.yml new file mode 100644 index 00000000000..970478f9be6 --- /dev/null +++ b/test/integration/targets/template/6653.yml @@ -0,0 +1,10 @@ +- hosts: localhost + gather_facts: no + vars: + mylist: + - alpha + - bravo + tasks: + - name: Should not fail on undefined variable + set_fact: + template_result: "{{ lookup('template', '6653.j2') }}" diff --git a/test/integration/targets/template/72262.yml b/test/integration/targets/template/72262.yml new file mode 100644 index 00000000000..33c610d4ec1 --- /dev/null +++ b/test/integration/targets/template/72262.yml @@ -0,0 +1,6 @@ +- hosts: localhost + gather_facts: no + tasks: + - name: Should not fail on undefined variable + set_fact: + template_result: "{{ lookup('template', '72262.j2') }}" diff --git a/test/integration/targets/template/72615.yml b/test/integration/targets/template/72615.yml new file mode 100644 index 00000000000..9a6eb941b4b --- /dev/null +++ b/test/integration/targets/template/72615.yml @@ -0,0 +1,26 @@ +- hosts: localhost + gather_facts: no + vars: + foo: "top-level-foo" + tasks: + - set_fact: + template_result: "{{ lookup('template', '72615.j2') }}" + + - assert: + that: + - "'template-level-bar' in template_result" + - "'template-nested-level-bar' in template_result" + + - assert: + that: + - "'top-level-foo' not in template_result" + - "'template-level-foo' in template_result" + - "'template-nested-level-foo' in template_result" + when: lookup('pipe', ansible_python_interpreter ~ ' -c "import jinja2; print(jinja2.__version__)"') is version('2.9', '>=') + + - assert: + that: + - "'top-level-foo' in template_result" + - "'template-level-foo' not in template_result" + - "'template-nested-level-foo' not in template_result" + when: lookup('pipe', ansible_python_interpreter ~ ' -c "import jinja2; print(jinja2.__version__)"') is version('2.9', '<') diff --git a/test/integration/targets/template/runme.sh b/test/integration/targets/template/runme.sh index 8e21352d29b..cb00df754d7 100755 --- a/test/integration/targets/template/runme.sh +++ b/test/integration/targets/template/runme.sh @@ -25,3 +25,12 @@ ansible-playbook unused_vars_include.yml -v "$@" # https://github.com/ansible/ansible/issues/55152 ansible-playbook undefined_var_info.yml -v "$@" + +# https://github.com/ansible/ansible/issues/72615 +ansible-playbook 72615.yml -v "$@" + +# https://github.com/ansible/ansible/issues/6653 +ansible-playbook 6653.yml -v "$@" + +# https://github.com/ansible/ansible/issues/72262 +ansible-playbook 72262.yml -v "$@" diff --git a/test/integration/targets/template/templates/6653-include.j2 b/test/integration/targets/template/templates/6653-include.j2 new file mode 100644 index 00000000000..26443b151b4 --- /dev/null +++ b/test/integration/targets/template/templates/6653-include.j2 @@ -0,0 +1 @@ +{{ x }} diff --git a/test/integration/targets/template/templates/6653.j2 b/test/integration/targets/template/templates/6653.j2 new file mode 100644 index 00000000000..8026a79befb --- /dev/null +++ b/test/integration/targets/template/templates/6653.j2 @@ -0,0 +1,4 @@ +{% for x in mylist %} +{{ x }} +{% include '6653-include.j2' with context %} +{% endfor %} diff --git a/test/integration/targets/template/templates/72262-included.j2 b/test/integration/targets/template/templates/72262-included.j2 new file mode 100644 index 00000000000..35700cb8efe --- /dev/null +++ b/test/integration/targets/template/templates/72262-included.j2 @@ -0,0 +1 @@ +{{ vars.test }} diff --git a/test/integration/targets/template/templates/72262-vars.j2 b/test/integration/targets/template/templates/72262-vars.j2 new file mode 100644 index 00000000000..6ef92208bb0 --- /dev/null +++ b/test/integration/targets/template/templates/72262-vars.j2 @@ -0,0 +1 @@ +{% set test = "I'm test variable" %} diff --git a/test/integration/targets/template/templates/72262.j2 b/test/integration/targets/template/templates/72262.j2 new file mode 100644 index 00000000000..b72be0d18d5 --- /dev/null +++ b/test/integration/targets/template/templates/72262.j2 @@ -0,0 +1,3 @@ +{% import '72262-vars.j2' as vars with context %} +{% macro included() %}{% include '72262-included.j2' %}{% endmacro %} +{{ included()|indent }} diff --git a/test/integration/targets/template/templates/72615-macro-nested.j2 b/test/integration/targets/template/templates/72615-macro-nested.j2 new file mode 100644 index 00000000000..c47a49922fb --- /dev/null +++ b/test/integration/targets/template/templates/72615-macro-nested.j2 @@ -0,0 +1,4 @@ +{% macro print_context_vars_nested(value) %} +foo: {{ foo }} +bar: {{ value }} +{% endmacro %} diff --git a/test/integration/targets/template/templates/72615-macro.j2 b/test/integration/targets/template/templates/72615-macro.j2 new file mode 100644 index 00000000000..328c271cd50 --- /dev/null +++ b/test/integration/targets/template/templates/72615-macro.j2 @@ -0,0 +1,8 @@ +{% macro print_context_vars(value) %} +{{ foo }} +{{ value }} +{% set foo = "template-nested-level-foo" %} +{% set bar = "template-nested-level-bar" %} +{% from '72615-macro-nested.j2' import print_context_vars_nested with context %} +{{ print_context_vars_nested(bar) }} +{% endmacro %} diff --git a/test/integration/targets/template/templates/72615.j2 b/test/integration/targets/template/templates/72615.j2 new file mode 100644 index 00000000000..b79f88e26d4 --- /dev/null +++ b/test/integration/targets/template/templates/72615.j2 @@ -0,0 +1,4 @@ +{% set foo = "template-level-foo" %} +{% set bar = "template-level-bar" %} +{% from '72615-macro.j2' import print_context_vars with context %} +{{ print_context_vars(bar) }}