Emit warnings when safe_eval() raises a SyntaxError or other Exception

[dagwieers]

This change is related to reported issue #14291 and pull request #14293.

Without the fix from #14293, this change will emit a warning as shown below, on the following playbook:

---
- hosts: localhost
  gather_facts: no
  vars:
    works:
      key1: 'string'
      key2: 1234
    fails:
      key1: 'string'
      key2: 1234
      key3: false
  tasks:
  - debug: msg={{ works | to_json }}
  - debug: msg={{ fails | to_json }}

On error, this results in a proper warning:

[dag@moria ansible.dag]$ ansible-playbook test49.yml

PLAY ***************************************************************************

TASK [debug] *******************************************************************
ok: [localhost] => {
    "msg": {
        "key1": "string",
        "key2": 1234
    }
}

TASK [debug] *******************************************************************
 [WARNING]: Error in expression "{"key3": false, "key2": 1234, "key1": "string"}". (name 'false' is not defined)

ok: [localhost] => {
    "msg": "{\"key3\": false, \"key2\": 1234, \"key1\": \"string\"}"
}

PLAY RECAP *********************************************************************
localhost                  : ok=2    changed=0    unreachable=0    failed=0

[dagwieers]

This issue #14293 exists also on Ansible v1.9.4, so if accepted (in whatever form) I'll look at backporting the logic too. (I have something for 1.9 here working at $company as well)

[dagwieers]

Can we get this into v2.2 please ?

It simply ensures that important syntax errors (that should not happen, but once did) are not silently ignored. Should be a no-brainer to decide to merge it, or improve it even more.

[brandond]

I am not sure that this merits displaying error - as per the comment below this line, there is some special handling for this exception that results in passing the string back as-is for later processing. I appear to have been relying on this behavior, as I am now getting a ton of SyntaxError warnings on Jinja2 templates inlined in my group_vars YAML.

[brandond]

I have a group_vars YAML file that contains entries like this:

bastion_host_ext: '{{ hostvars[bastion_host_name].ec2_ip_address | default(None) }}'
bastion_host_int: '{{ hostvars[bastion_host_name].ec2_private_ip_address | default(None) }}'
bastion_host_addr: >-
  {%- if bastion_host_ext -%}
  {{ bastion_host_ext }}
  {%- elif bastion_host_int -%}
  {{ bastion_host_int }}
  {%- endif -%}
ansible_ssh_extra_args: >-
  {%- if bastion_host_name != 'localhost' -%}
  -o ProxyCommand='{{ bastion_proxy_cmd }} {{ ansible_remote_user }}@{{ bastion_host_addr }}'
  {%- endif -%}

Since this change was committed, I now get a slew of errors on every playbook execution that look like this:

 [WARNING]: SyntaxError in safe_eval() on expr: {%- if bastion_host_ext -%} {{ bastion_host_ext }} {%- elif bastion_host_int -%} {{ bastion_host_int }} {%- endif -%} (invalid
syntax (<unknown>, line 1))
 [WARNING]: SyntaxError in safe_eval() on expr: {%- if bastion_host_name != 'localhost' -%} -o ProxyCommand='{{ bastion_proxy_cmd }} {{ ansible_remote_user }}@{{
bastion_host_addr }}' {%- endif -%} (invalid syntax (<unknown>, line 1))

This is valid YAML as far as I can tell; I see no reason why it should be throwing an error. Perhaps the special behavior noted in the comment should silently succeed without raising a warning?

[dagwieers]

@brandond I have made my own tests in order to reproduce your problem, but I cannot.

group_vars/all

test122_expression: 'Expr: {{ inventory_hostname }}'
test122_statement: >-                                                                                                                               
  {%- if inventory_hostname == 'localhost' -%}
  Local: {{ inventory_hostname }}
  {%- else -%}
  Other: {{ inventory_hostname }}
  {%- endif -%}

test122.yml

- hosts: localhost
  gather_facts: no
  tasks:
  - debug:
      msg: '{{ test122_expression }}'

  - debug:
      msg: '{{ test122_statement }}'

Output

[dag@moria ansible.testing]$ ansible --version
ansible 2.2.0 (devel 032bd1dacf) last updated 2016/09/08 17:28:24 (GMT +200)
  lib/ansible/modules/core: (detached HEAD 8e241a87cc) last updated 2016/07/14 17:49:10 (GMT +200)
  lib/ansible/modules/extras: (detached HEAD 2078c4b4da) last updated 2016/07/14 17:49:12 (GMT +200)
  config file = /home/dag/home-made/ansible.testing/ansible.cfg
  configured module search path = Default w/o overrides

[dag@moria ansible.testing]$ ansible-playbook -vv test122.yml
Using /home/dag/home-made/ansible.testing/ansible.cfg as config file

PLAYBOOK: test122.yml **********************************************************
1 plays in test122.yml

PLAY [localhost] ***************************************************************

TASK [debug] *******************************************************************
task path: /home/dag/home-made/ansible.testing/test122.yml:4
ok: [localhost] => {
    "msg": "Expr: localhost"
}

TASK [debug] *******************************************************************
task path: /home/dag/home-made/ansible.testing/test122.yml:7
ok: [localhost] => {
    "msg": "Local: localhost"
}

PLAY RECAP *********************************************************************
localhost                  : ok=2    changed=0    unreachable=0    failed=0

PS Thanks for showing me that you can use Jinja2 statements inside YAML playbooks. It was my impression that only Jinja2 expressions were allowed in playbooks/vars files and Jinja2 statements were an exclusive to templates. (I wonder if we have such examples in the documentation)

[dagwieers]

Aha, in case of late evaluation, this problem comes into play:

group_vars/all

test122_expression: 'Expr: {{ inventory_hostname }}'
test122_statement: >-
  {%- if test122_variable == 'localhost' -%}
  Local: {{ inventory_hostname }}
  {%- else -%}
  Other: {{ inventory_hostname }}
  {%- endif -%}

test122.yml

- hosts: localhost
  gather_facts: no
  vars:
    test122_variable: 'localhost'
  tasks:
  - debug:
      msg: '{{ test122_expression }}'

  - debug:
      msg: '{{ test122_statement }}'

Output

[dag@moria ansible.testing]$ ansible-playbook -vv test122.yml
Using /home/dag/home-made/ansible.testing/ansible.cfg as config file

PLAYBOOK: test122.yml **********************************************************
1 plays in test122.yml

PLAY [localhost] ***************************************************************

TASK [debug] *******************************************************************
task path: /home/dag/home-made/ansible.testing/test122.yml:6
 [WARNING]: SyntaxError in safe_eval() on expr: {%- if test122_variable == 'localhost' -%} Local: {{ inventory_hostname }} {%- else -%} Other: {{
inventory_hostname }} {%- endif -%} (invalid syntax (<unknown>, line 1))

ok: [localhost] => {
    "msg": "Expr: localhost"
}

TASK [debug] *******************************************************************
task path: /home/dag/home-made/ansible.testing/test122.yml:9
 [WARNING]: SyntaxError in safe_eval() on expr: {%- if test122_variable == 'localhost' -%} Local: {{ inventory_hostname }} {%- else -%} Other: {{
inventory_hostname }} {%- endif -%} (invalid syntax (<unknown>, line 1))

ok: [localhost] => {
    "msg": "Local: localhost"
}

PLAY RECAP *********************************************************************
localhost                  : ok=2    changed=0    unreachable=0    failed=0

[dagwieers]

@brandond The warning is not related to YAML, but it is related to Jinja2 expression/statement evaluation. Since the statement is evaluated before some variables were known/evaluated/parsed, it causes a SyntaxError. In a subsequent (late) evaluation the syntax is considered fine because the variables are defined.

Fixed by PR #17462, however it would have been better if you opened a new issue for this.

[brandond]

@dagwieers - you're probably right, since the change was committed the issue merited a new issue. Thanks for the quick fix anyway!