Local vars should have highest precedence in AnsibleJ2Vars (#72830)

Ability to add local variables into AnsibleJ2Vars was added in
18a9eff11f 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

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)

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)

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') }}"

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') }}"

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', '<')

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 "$@"

test/integration/targets/template/templates/6653-include.j2

@@ -0,0 +1 @@
+{{ x }}

test/integration/targets/template/templates/6653.j2

@@ -0,0 +1,4 @@
+{% for x in mylist %}
+{{ x }}
+{% include '6653-include.j2' with context %}
+{% endfor %}

test/integration/targets/template/templates/72262-included.j2

@@ -0,0 +1 @@
+{{ vars.test }}

test/integration/targets/template/templates/72262-vars.j2

@@ -0,0 +1 @@
+{% set test = "I'm test variable" %}

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 }}

test/integration/targets/template/templates/72615-macro-nested.j2

@@ -0,0 +1,4 @@
+{% macro print_context_vars_nested(value) %}
+foo: {{ foo }}
+bar: {{ value }}
+{% endmacro %}

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 %}

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) }}