Skip to content
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

Interpolation of undefined variables different between with_items and direct reference from inside templates. #9008

Closed
damncabbage opened this issue Sep 15, 2014 · 12 comments · Fixed by #9286
Labels
bug This issue/PR relates to a bug. needs_info This issue requires further information. Please answer any outstanding questions. P1 Priority 1 - Immediate Attention Required; Release Immediately After Fixed

Comments

@damncabbage
Copy link
Contributor

Issue Type:

Bug Report

Ansible Version:

ansible 1.7.1 (identical behaviour in devel, 267b3fb)

Environments:

Ubuntu 14.04 managing Ubuntu 14.04 machines.

Summary:

The following group_vars/all

things:
- name: "works"
- name: "{{ should_explode }}"

... correctly interpolates should_explode when that variable is provided, but:

  • Results in a verbatim {{ should_explode }} when referenced from within a with_items loop, and
  • Correctly explodes if looped over from within a template.

(error_on_undefined_vars is set to True. And I say {{ should_explode }}, but when used with the copy or template modules I actually get {# should_explode #} instead; I'm considering the {{ vs {# confusion a very secondary issue for the sake of this ticket, though.)

Steps To Reproduce:

Have the following in a group_vars/all file:

things:
- name: "works"
- name: "{{ should_explode }}"

Have two templates: item_name.j2:

{{ item.name }}

... and things_names.j2:

{% for thing in things %}
{{ thing.name }}
{% endfor %}

Have the following tasks in a playbook:

- name: "things with_items"
  template: >
    src=item_name.j2
    dest=/tmp/{{ item.name }}.txt
  with_items: things

- name: "things direct template reference"
  template: >
    src=things_names.j2
    dest=/tmp/things.txt

Add an ansible.cfg with the following if you want to be sure about the error-on-undefined:

[defaults]
error_on_undefined_vars = True

Finally, run the hash integration tests with cd test/integration; make test_hash.

Expected Results:
TASK: [test | things with_items] *************************************
fatal: ... <==== ... of some variety.

TASK: [test | things direct template reference] **********************
fatal: [test] => {'msg': "AnsibleUndefinedVariable: One or more undefined variables: 'should_explode' is undefined", 'failed': True}
fatal: [test] => {'msg': 'One or more items failed.', 'failed': True, 'changed': False, 'results': [{'msg': "AnsibleUndefinedVariable: One or more undefined variables: 'should_explode' is undefined", 'fa}
Actual Results:
TASK: [test | things with_items] *************************************
changed: [test] => (item={'name': 'works'})
changed: [test] => (item={'name': u'{# should_explode #}'})  <==== Instead, this.

TASK: [test | things direct template reference] **********************
fatal: [test] => {'msg': "AnsibleUndefinedVariable: One or more undefined variables: 'should_explode' is undefined", 'failed': True}
fatal: [test] => {'msg': "AnsibleUndefinedVariable: One or more undefined variables: 'should_explode' is undefined", 'failed': True}

And on the second run (no modifications):

TASK: [test | things with_items] *************************************
ok: [test] => (item={'name': 'works'})
failed: [test] => (item={'name': u'{# should_explode #}'}) => {"failed": true, "item": {"name": "{# should_explode #}"}}
msg: this module requires key=value arguments (['src=item_name.j2', 'dest=/tmp/{#', 'should_explode', '#}.txt'])
Notes:

I'm not sure if this is known behaviour. The second run is definitely weird at least, even if the inconsistency in variable dereferencing is expected.

@jimi-c jimi-c added P2 Priority 2 - Issue Blocks Release bug_report labels Sep 15, 2014
@damncabbage
Copy link
Contributor Author

I just got bitten by this:

- hosts: examplehost
  tasks:
  - name: "a task"
    shell: "echo this is just an example it applies to any action"
    sudo: "yes"
    sudo_user: "{{ userr }}" # oops, typo'd variable name
  vars:
    user: "myuser"

Which produces:

TASK: [a task] ****************************************************************
failed: [examplehost] => {"failed": true, "parsed": false}
invalid output was: sudo: unknown user: {{
sudo: unable to initialize policy plugin

I can submit another ticket, or I can just leave this as a note on this ticket.

@damncabbage
Copy link
Contributor Author

And again with the odd {# ... #} case:

TASK: [app | config | Create the basic deploy target directories] *************
failed: [app01] => (item={# app_path #}) => {"failed": true, "item": "{# app_path #}", "parsed": false}
- name: "config | Create the basic deploy target directories"
  file:
    name: "{{ item }}"
    state: "directory"
    owner: "{{ app_user_username }}"
    group: "{{ app_user_username }}"
  with_items:
  - "{{ app_path }}"
  - "{{ app_path_releases }}"
  # ...

@mpdehaan
Copy link
Contributor

mpdehaan commented Oct 3, 2014

This looks like a bug down the lazy evaluation path to me.

The "{{ foo }}" becoming "{# foo #}" is a result of the "anti-new parameter injection" security checking stuff coming in which should not come into play here. I would expect it to remain "{{ foo }}" and result in an undefined variable error.

@mpdehaan mpdehaan added P1 Priority 1 - Immediate Attention Required; Release Immediately After Fixed and removed P2 Priority 2 - Issue Blocks Release labels Oct 3, 2014
@jimi-c
Copy link
Member

jimi-c commented Oct 3, 2014

@damncabbage is this only happening with variables that are undefined, or are you seeing this with all variables?

@damncabbage
Copy link
Contributor Author

Only typo'd / undefined variables.

(In the last example, app_path was a typo of app_path_root. app_path_releases was defined and worked fine.)

@jimi-c
Copy link
Member

jimi-c commented Oct 3, 2014

Great, just wanted to make sure, I'm looking into this now. Thanks!

jimi-c added a commit to jimi-c/ansible that referenced this issue Oct 3, 2014
@jimi-c
Copy link
Member

jimi-c commented Oct 3, 2014

@damncabbage I've created the above patch, if you'd like to test it out. Using your first example, the following output is the result:

PLAY [localhost] ************************************************************** 
TASK: [things with_items] ***************************************************** 
fatal: [127.0.0.1] => undefined variable in items: 'should_explode' is undefined
FATAL: all hosts have already failed -- aborting

@jimi-c jimi-c added the needs_info This issue requires further information. Please answer any outstanding questions. label Oct 3, 2014
@damncabbage
Copy link
Contributor Author

Tested the patch; it explodes as expected. Thanks. 😃

My test case (simpler than the initial setup):

- hosts: localhost
  tasks:
  - debug:
      var: "{{ item }}"
    with_items: a_list
  vars:
    a_list:
    - "{{ foo }}"
    - "{{ foob }}"
    foo: "Hello"

I do agree with @redbaron; please instead consider:

  import subprocess
  import contextlib
+ import jinja2.exceptions

  from vault import VaultLib
              # if not already a list, get ready to evaluate with Jinja2
              # not sure why the "/" is in above code :)
              try:
-                 new_terms = template.template(basedir, "{{ %s }}" % terms, inject)
+                 new_terms = template.template(basedir, terms, inject, convert_bare=True, fail_on_undefined=C.DEFAULT_UNDEFINED_VAR_BEHAVIOR)
                  if isinstance(new_terms, basestring) and "{{" in new_terms:
                      pass
                  else:
                      terms = new_terms
+             except jinja2.exceptions.UndefinedError, e:
+                 raise errors.AnsibleUndefinedVariable('undefined variable in items: %s' % e)
              except:
                  pass

(Is it standard practice with this project to also add a case to the automated test suite?)

@jimi-c
Copy link
Member

jimi-c commented Oct 8, 2014

Feel free to send a PR for that so you get credit, the way I did it was admittedly not pythonic.

damncabbage added a commit to damncabbage/ansible that referenced this issue Oct 8, 2014
(Fixes ansible#9008.)

With credit to jimi-c for the initial pass in this commit:
jimi-c@b18bd6b
@damncabbage
Copy link
Contributor Author

Done; thank you.

@jimi-c
Copy link
Member

jimi-c commented Oct 8, 2014

Closing This Ticket

Hi!

We believe the above commit should resolve this problem for you. This will also be included in the next major 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!

@vermut
Copy link

vermut commented Apr 27, 2015

It was reverted in 97408fe

Can you explain - why? Ansible ignores undefined variables again.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue/PR relates to a bug. needs_info This issue requires further information. Please answer any outstanding questions. P1 Priority 1 - Immediate Attention Required; Release Immediately After Fixed
Projects
None yet
5 participants