New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jinja2 2.9+ breaks included template context #20063

Closed
sysadmind opened this Issue Jan 9, 2017 · 22 comments

Comments

Projects
None yet
@sysadmind

sysadmind commented Jan 9, 2017

ISSUE TYPE
  • Bug Report
COMPONENT NAME

Template

ANSIBLE VERSION
ansible 2.3.0 (devel 0aad46c85b) last updated 2017/01/09 16:11:21 (GMT -400)
  config file = /home/jadams/.ansible.cfg
  configured module search path = Default w/o overrides

CONFIGURATION

Tested on vanilla / no extra configuration

OS / ENVIRONMENT

N/A but tested from Fedora 25

SUMMARY

When Jinja2 dependency is >= 2.9.0, nested templates inside of loops defined in a parent template do not receive the local scope. For example:

{% for item in foo %}
    {% include "other_template.j2" %}
{% endfor %

In the above example, other_template.j2 would not have the item variable defined.

STEPS TO REPRODUCE
# Example Playbook
- hosts: localhost
  connection: local
  vars:
    foo:
      - bar
      - baz
  tasks:
    - name: Test Template
      template: src=parent.j2 dest=/tmp/template.out
# parent.j2
{% for item in foo %}
    {% include "subtemplate.j2" %}
{% endfor %}
#subtemplate.j2
{{ item }}
EXPECTED RESULTS

Expected the output of the following file:

bar
baz
ACTUAL RESULTS
TASK [Test Template] ************************************************************************************************************************************************************************************************************************
fatal: [localhost]: FAILED! => {"changed": false, "failed": true, "msg": "AnsibleUndefinedVariable: 'item' is undefined"}

I believe this is because of the Jinja2 Context changes mentioned here in combination with Ansible overriding default Jinja2 functions. I wasn't able to track down the root cause in my limited testing.

@bcoca

This comment has been minimized.

Member

bcoca commented Jan 9, 2017

seems we either need to default to with context or just add that in the template include

@bcoca bcoca removed the needs_triage label Jan 9, 2017

@sivel

This comment has been minimized.

Member

sivel commented Jan 9, 2017

There seem to be some other issues as well with jinja2 2.9 as @mattclay has discovered:

7ba47bf
#20014

@hsoft

This comment has been minimized.

Contributor

hsoft commented Jan 10, 2017

This issue also affects ansible v2.2x (see #20089)

@andymcc

This comment has been minimized.

andymcc commented Jan 12, 2017

I ran into a very similar issue - you can't have an {% include %} and a {% set %} in the same template, or it will fail. You could have a {% set %} without an {% include %} or vice versa, but not both together. Below shows a simple test using very minimal templates (and ad-hoc commands) to show the failure.

This was done using ansible 2.2.0.0

https://gist.github.com/andymcc/2ddf98b50fa07e3f8b8648a440499fb8

@mitsuhiko

This comment has been minimized.

mitsuhiko commented Jan 12, 2017

I'm not able to reproduce this is in plain Jinja2 as you can see from pallets/jinja#659. However there are quite a few changes to clean up scoping behavior internally so if ansible depended on some quirky behavior I would not be surprised if that broke.

If you need insights on the changes done there I'm happy to assist.

@bcoca

This comment has been minimized.

Member

bcoca commented Jan 12, 2017

@andymcc see my previous comment about with_context

@mitsuhiko

This comment has been minimized.

mitsuhiko commented Jan 12, 2017

One thing I just noticed is that Template.new_context / Context.derive might have changed behavior internally that Ansible might have to look into.

@mitsuhiko

This comment has been minimized.

mitsuhiko commented Jan 12, 2017

I wonder if the issue is that ansible uses a custom context. Jinja2 invokes resolve_or_missing rather than resolve now. We can probably look into detecting a changed resolve implementation and switch the implementation for backwards compatibility however that was never a public API.

@bcoca

This comment has been minimized.

Member

bcoca commented Jan 12, 2017

@mitsuhiko

This comment has been minimized.

mitsuhiko commented Jan 12, 2017

I added this. I hope this resolves the ansible issue as well: pallets/jinja@c6ddeb7

@hsoft

This comment has been minimized.

Contributor

hsoft commented Jan 13, 2017

@mitsuhiko I still have the regression on my playbook here. I'm on a freshly updated stable-2.2 ansible branch and on a freshly updated master on jinja and I still have:

An unhandled exception occurred while running the lookup plugin 'template'. Error was a <type 'exceptions.KeyError'>, original message: 'undefined variable: 0'

To make sure that I was testing the right thing, I've switched my jinja branch back and forth between master and 2.8-maintenance and it toggles the error.

@j0hnsmith

This comment has been minimized.

Contributor

j0hnsmith commented Jan 16, 2017

I've experienced the same issue with Jinja2 2.9.4 (although my include isn't in a for loop, just a straight include), when I remove the {% include 'something.j2' %} line the problem goes away, the workaround pip install jinja2==2.8.1 works for me.

@jimi-c

This comment has been minimized.

Member

jimi-c commented Jan 19, 2017

@mitsuhiko I don't think the problem is with resolve, I wrote it to just super() the call, which looking at the code uses resolve_or_missing() under the hood. The class I wrote just does some extra stuff with the result after that.

@jimi-c

This comment has been minimized.

Member

jimi-c commented Jan 19, 2017

@mitsuhiko disregard that, I see that you mean resolve() is just not called internally anymore.

@jimi-c

This comment has been minimized.

Member

jimi-c commented Jan 20, 2017

@mitsuhiko so it looks like in 2.9.x, local vars are no longer prefixed with l_? That's the cause of the problem - our custom AnsibleJ2Vars class was only updating an internal dict when locals started with l_. This patch fixes the problem in my testing:

diff --git a/lib/ansible/template/vars.py b/lib/ansible/template/vars.py
index 0e9b99e..fc6140c 100644
--- a/lib/ansible/template/vars.py
+++ b/lib/ansible/template/vars.py
@@ -50,8 +50,11 @@ class AnsibleJ2Vars:
         self._locals  = dict()
         if isinstance(locals, dict):
             for key, val in iteritems(locals):
-                if key[:2] == 'l_' and val is not missing:
-                    self._locals[key[2:]] = val
+                if val is not missing:
+                    if key[:2] == 'l_':
+                        self._locals[key[2:]] = val
+                    elif key not in ('context', 'environment', 'template'):
+                        self._locals[key] = val
 
     def __contains__(self, k):
         if k in self._templar._available_variables:

I'm guessing there may be other things we want to filter out of that list, but those seemed like the most obvious ones.

@jimi-c

This comment has been minimized.

Member

jimi-c commented Jan 20, 2017

@mitsuhiko found the relevant commit which changed this: pallets/jinja@1205e64#diff-c1d46ddd4606981e74677b39601af358L585

jimi-c added a commit that referenced this issue Jan 20, 2017

Don't restrict local jinja2 variables to those that start with l_
Per a change in jinja2 2.9, local variables no longer are prefixed
with l_, so this updates AnsibleJ2Vars to pull in all locals (while
excluding some) regardless of name.

Fixes #20063
@mitsuhiko

This comment has been minimized.

mitsuhiko commented Jan 20, 2017

@jimi-c just to be aware this was never (and still is not) a public interface. If there is an API missing I'm happy to add something but I can promise you that this will break again in the future as the situation exists.

@jimi-c

This comment has been minimized.

Member

jimi-c commented Jan 20, 2017

@mitsuhiko I'm aware :) I think we'll just need to setup an additional test to pull the devel version of jinja2 and add more tests to try and catch these things prior to release.

@jimi-c jimi-c closed this in 188c3c6 Jan 20, 2017

@jimi-c

This comment has been minimized.

Member

jimi-c commented Jan 20, 2017

Closing This Ticket

Hi!

We believe the above commit should resolve this problem for you. This will also be included in the next release.

If you continue seeing any problems related to this issue, or if you have any further questions, please let us know by stopping by one of the two mailing lists, as appropriate:

Because this project is very active, we're unlikely to see comments made on closed tickets, but the mailing list is a great way to ask questions, or post if you don't think this particular issue is resolved.

Thank you!

@dctremblay

This comment has been minimized.

Contributor

dctremblay commented Apr 21, 2017

As said from #20494, a simple solution that worked for me (using Debian Stretch) is to enter those commands :

pip install git+https://github.com/ansible/ansible.git
pip install Jinja2==2.8.1

Hope this helps someone !

@telbizov

This comment has been minimized.

telbizov commented Oct 4, 2017

Hello,

I am trying to figure out which ansible release has this fix gone out with?
I run 2.3.1.0 and I think I am encountering this exact same problem that @jimi-c claims fixed in January.

My template has set statements and an include in the bottom. ansible was installed via pip and as such it had dragged with itself Jinja2 (2.9.6). Downgrading Jinja2 to 2.8.1 resolves the problem but the default action is to bring the latest jinja thus it leaves your ansible broken if your templates use this combination.

Any advise?

Thank you,
Rumen Telbizov

@peakwinter

This comment has been minimized.

peakwinter commented Oct 5, 2017

@telbizov The fix appears to be present in versions >= 2.4.0.0

cjoe added a commit to Apptimize/ve that referenced this issue Feb 21, 2018

@ansibot ansibot added bug and removed bug_report labels Mar 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment