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

Quoted strings get concatenated when using jinja call block with callback #80605

Closed
1 task done
corubba opened this issue Apr 22, 2023 · 6 comments · Fixed by #80705
Closed
1 task done

Quoted strings get concatenated when using jinja call block with callback #80605

corubba opened this issue Apr 22, 2023 · 6 comments · Fixed by #80705
Labels
affects_2.14 bug This issue/PR relates to a bug. has_pr This issue has an associated PR. module This issue/PR relates to a module. native_jinja issues related to jinja native types

Comments

@corubba
Copy link

corubba commented Apr 22, 2023

Summary

Under certain conditions, using a template task with a jinja call block and a callback that returns a list of quoted strings, the inner quotes are collapsed/removed and the list of quoted strings is turned into a single quoted concatenated string. So instead of "a" "b" the resulting file contains "ab".

Issue Type

Bug Report

Component Name

template

Ansible Version

$ ansible --version
ansible [core 2.14.4]
  config file = /home/user/exchange/repro/ansible.cfg
  configured module search path = ['/home/user/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3.10/site-packages/ansible
  ansible collection location = /home/user/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/sbin/ansible
  python version = 3.10.10 (main, Mar  5 2023, 22:26:53) [GCC 12.2.1 20230201] (/usr/bin/python)
  jinja version = 3.1.2
  libyaml = True

Configuration

$ ansible-config dump --only-changed -t all
CONFIG_FILE() = /home/user/repro/ansible.cfg
DEFAULT_JINJA2_NATIVE(/home/user/repro/ansible.cfg) = True

OS / Environment

Initially encountered on ArchLinux with the distribution-packages. Later confirmed using the example below on ArchLinux with the version from PyPI and the devel branch.

Steps to Reproduce

ansible.cfg

[defaults]
jinja2_native = yes

playbook.yml

---
- hosts: localhost
  tasks:
  - template:
      src: "{{ 'template.j2' }}"
      dest: "/tmp/output"

template.j2

#jinja2: foo:True

{% macro my_macro() %}
  {{ caller() }}
{% endmacro %}

{% call my_macro() -%}
  "a" "b"
{% endcall %}

In a directory with those three files, execute

$ ansible-playbook playbook.yml

A few observations I made while reducing the original setup to the minimal example above:

  • If you turn off the jinja native mode by setting jinja2_native = no in ansible.cfg, it works as expected. This is contrary to the template module docs which state "The jinja2_native setting has no effect."
  • If the definition of the template task in the playbook does not contain a jinja expression, it works as expected. This is the reason why in the example src is an expression with a constant instead of just a plaintext value.
  • If the template does not contain a jinja override (the first #jinja2 line), it works as expected. You get an error if you don't set at least one override, so in the example I set a unused foo value to make it effectively a no-op.
  • If the - whitespace modifier is removed from the end-delimiter of the call start-statement in the template, it works as expected.
  • It does not matter what type of quotes (single or double), what whitespace (space or tabs) between them, or how many elements or whitespace between elements: they all get collapsed. "a" "b" "c" "d" 'e' 'f' "g" 'h' in the template results in "abcdefgh" in the file. It basically behaves like python's string literal concatenation.
  • Putting anything but whitespace between the quoted strings makes it work as expected, e.g. "a"+ "b" in the template will be written exactly like that into the file.

Expected Results

A file /tmp/output is created that contains "a" "b" (plus some whitespace around it, but that's not really important here).

$ ansible-playbook playbook.yml -D
[WARNING]: No inventory was parsed, only implicit localhost is available
[WARNING]: provided hosts list is empty, only localhost is available. Note that the implicit localhost does not match 'all'

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

TASK [Gathering Facts] **************************************************************************************************************************************************************************************************************************************************************
ok: [localhost]

TASK [template] *********************************************************************************************************************************************************************************************************************************************************************
--- before
+++ after: /home/user/.ansible/tmp/ansible-local-209txmb3l0y/tmp4vuswx84/template.j2
@@ -0,0 +1,4 @@
+
+
+  "a" "b"
+

changed: [localhost]

PLAY RECAP **************************************************************************************************************************************************************************************************************************************************************************
localhost                  : ok=2    changed=1    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   

Actual Results

$ ansible-playbook playbook.yml -D -vvvv
ansible-playbook [core 2.14.4]
  config file = /home/user/repro/ansible.cfg
  configured module search path = ['/home/user/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3.10/site-packages/ansible
  ansible collection location = /home/user/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/sbin/ansible-playbook
  python version = 3.10.10 (main, Mar  5 2023, 22:26:53) [GCC 12.2.1 20230201] (/usr/bin/python)
  jinja version = 3.1.2
  libyaml = True
Using /home/user/repro/ansible.cfg as config file
setting up inventory plugins
host_list declined parsing /etc/ansible/hosts as it did not pass its verify_file() method
Skipping due to inventory source not existing or not being readable by the current user
script declined parsing /etc/ansible/hosts as it did not pass its verify_file() method
auto declined parsing /etc/ansible/hosts as it did not pass its verify_file() method
Skipping due to inventory source not existing or not being readable by the current user
yaml declined parsing /etc/ansible/hosts as it did not pass its verify_file() method
Skipping due to inventory source not existing or not being readable by the current user
ini declined parsing /etc/ansible/hosts as it did not pass its verify_file() method
Skipping due to inventory source not existing or not being readable by the current user
toml declined parsing /etc/ansible/hosts as it did not pass its verify_file() method
[WARNING]: No inventory was parsed, only implicit localhost is available
[WARNING]: provided hosts list is empty, only localhost is available. Note that the implicit localhost does not match 'all'
Loading callback plugin default of type stdout, v2.0 from /usr/lib/python3.10/site-packages/ansible/plugins/callback/default.py
Skipping callback 'default', as we already have a stdout callback.
Skipping callback 'minimal', as we already have a stdout callback.
Skipping callback 'oneline', as we already have a stdout callback.

PLAYBOOK: playbook.yml **************************************************************************************************************************************************************************************************************************************************************
Positional arguments: playbook.yml
verbosity: 4
connection: smart
timeout: 10
become_method: sudo
tags: ('all',)
diff: True
inventory: ('/etc/ansible/hosts',)
forks: 5
1 plays in playbook.yml

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

TASK [Gathering Facts] **************************************************************************************************************************************************************************************************************************************************************
task path: /home/user/repro/playbook.yml:2
<127.0.0.1> ESTABLISH LOCAL CONNECTION FOR USER: user
<127.0.0.1> EXEC /bin/sh -c 'echo ~user && sleep 0'
<127.0.0.1> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo /home/user/.ansible/tmp `"&& mkdir "` echo /home/user/.ansible/tmp/ansible-tmp-1682203193.5547385-136-66384164090029 `" && echo ansible-tmp-1682203193.5547385-136-66384164090029="` echo /home/user/.ansible/tmp/ansible-tmp-1682203193.5547385-136-66384164090029 `" ) && sleep 0'
Using module file /usr/lib/python3.10/site-packages/ansible/modules/setup.py
<127.0.0.1> PUT /home/user/.ansible/tmp/ansible-local-1323lqb200c/tmp3htqbzr4 TO /home/user/.ansible/tmp/ansible-tmp-1682203193.5547385-136-66384164090029/AnsiballZ_setup.py
<127.0.0.1> EXEC /bin/sh -c 'chmod u+x /home/user/.ansible/tmp/ansible-tmp-1682203193.5547385-136-66384164090029/ /home/user/.ansible/tmp/ansible-tmp-1682203193.5547385-136-66384164090029/AnsiballZ_setup.py && sleep 0'
<127.0.0.1> EXEC /bin/sh -c '/usr/bin/python /home/user/.ansible/tmp/ansible-tmp-1682203193.5547385-136-66384164090029/AnsiballZ_setup.py && sleep 0'
<127.0.0.1> EXEC /bin/sh -c 'rm -f -r /home/user/.ansible/tmp/ansible-tmp-1682203193.5547385-136-66384164090029/ > /dev/null 2>&1 && sleep 0'
ok: [localhost]

TASK [template] *********************************************************************************************************************************************************************************************************************************************************************
task path: /home/user/repro/playbook.yml:4
<127.0.0.1> ESTABLISH LOCAL CONNECTION FOR USER: user
<127.0.0.1> EXEC /bin/sh -c 'echo ~user && sleep 0'
<127.0.0.1> EXEC /bin/sh -c '( umask 77 && mkdir -p "` echo /home/user/.ansible/tmp `"&& mkdir "` echo /home/user/.ansible/tmp/ansible-tmp-1682203194.4804294-179-10085099603156 `" && echo ansible-tmp-1682203194.4804294-179-10085099603156="` echo /home/user/.ansible/tmp/ansible-tmp-1682203194.4804294-179-10085099603156 `" ) && sleep 0'
Using module file /usr/lib/python3.10/site-packages/ansible/modules/stat.py
<127.0.0.1> PUT /home/user/.ansible/tmp/ansible-local-1323lqb200c/tmpal8urkxn TO /home/user/.ansible/tmp/ansible-tmp-1682203194.4804294-179-10085099603156/AnsiballZ_stat.py
<127.0.0.1> EXEC /bin/sh -c 'chmod u+x /home/user/.ansible/tmp/ansible-tmp-1682203194.4804294-179-10085099603156/ /home/user/.ansible/tmp/ansible-tmp-1682203194.4804294-179-10085099603156/AnsiballZ_stat.py && sleep 0'
<127.0.0.1> EXEC /bin/sh -c '/usr/bin/python /home/user/.ansible/tmp/ansible-tmp-1682203194.4804294-179-10085099603156/AnsiballZ_stat.py && sleep 0'
Using module file /usr/lib/python3.10/site-packages/ansible/modules/file.py
<127.0.0.1> PUT /home/user/.ansible/tmp/ansible-local-1323lqb200c/tmpd272p4fp TO /home/user/.ansible/tmp/ansible-tmp-1682203194.4804294-179-10085099603156/AnsiballZ_file.py
<127.0.0.1> EXEC /bin/sh -c 'chmod u+x /home/user/.ansible/tmp/ansible-tmp-1682203194.4804294-179-10085099603156/ /home/user/.ansible/tmp/ansible-tmp-1682203194.4804294-179-10085099603156/AnsiballZ_file.py && sleep 0'
<127.0.0.1> EXEC /bin/sh -c '/usr/bin/python /home/user/.ansible/tmp/ansible-tmp-1682203194.4804294-179-10085099603156/AnsiballZ_file.py && sleep 0'
<127.0.0.1> PUT /home/user/.ansible/tmp/ansible-local-1323lqb200c/tmpwxlecmmr/template.j2 TO /home/user/.ansible/tmp/ansible-tmp-1682203194.4804294-179-10085099603156/source
<127.0.0.1> EXEC /bin/sh -c 'chmod u+x /home/user/.ansible/tmp/ansible-tmp-1682203194.4804294-179-10085099603156/ /home/user/.ansible/tmp/ansible-tmp-1682203194.4804294-179-10085099603156/source && sleep 0'
Using module file /usr/lib/python3.10/site-packages/ansible/modules/copy.py
<127.0.0.1> PUT /home/user/.ansible/tmp/ansible-local-1323lqb200c/tmpmvc5jmva TO /home/user/.ansible/tmp/ansible-tmp-1682203194.4804294-179-10085099603156/AnsiballZ_copy.py
<127.0.0.1> EXEC /bin/sh -c 'chmod u+x /home/user/.ansible/tmp/ansible-tmp-1682203194.4804294-179-10085099603156/ /home/user/.ansible/tmp/ansible-tmp-1682203194.4804294-179-10085099603156/AnsiballZ_copy.py && sleep 0'
<127.0.0.1> EXEC /bin/sh -c '/usr/bin/python /home/user/.ansible/tmp/ansible-tmp-1682203194.4804294-179-10085099603156/AnsiballZ_copy.py && sleep 0'
<127.0.0.1> EXEC /bin/sh -c 'rm -f -r /home/user/.ansible/tmp/ansible-tmp-1682203194.4804294-179-10085099603156/ > /dev/null 2>&1 && sleep 0'
--- before
+++ after: /home/user/.ansible/tmp/ansible-local-1323lqb200c/tmpwxlecmmr/template.j2
@@ -0,0 +1,3 @@
+
+
+  "ab"

changed: [localhost] => {
    "changed": true,
    "checksum": "c8d2785230875caee6e9935c9fe1e63788783d8f",
    "dest": "/tmp/output",
    "diff": [
        {
            "after": "\n\n  \"ab\"\n",
            "after_header": "/home/user/.ansible/tmp/ansible-local-1323lqb200c/tmpwxlecmmr/template.j2",
            "before": ""
        }
    ],
    "gid": 1000,
    "group": "user",
    "invocation": {
        "module_args": {
            "_original_basename": "template.j2",
            "attributes": null,
            "backup": false,
            "checksum": "c8d2785230875caee6e9935c9fe1e63788783d8f",
            "content": null,
            "dest": "/tmp/output",
            "directory_mode": null,
            "follow": false,
            "force": true,
            "group": null,
            "local_follow": null,
            "mode": null,
            "owner": null,
            "remote_src": null,
            "selevel": null,
            "serole": null,
            "setype": null,
            "seuser": null,
            "src": "/home/user/.ansible/tmp/ansible-tmp-1682203194.4804294-179-10085099603156/source",
            "unsafe_writes": false,
            "validate": null
        }
    },
    "md5sum": "daa70962b93278078ee3b9b6825bd9fd",
    "mode": "0644",
    "owner": "user",
    "size": 9,
    "src": "/home/user/.ansible/tmp/ansible-tmp-1682203194.4804294-179-10085099603156/source",
    "state": "file",
    "uid": 1000
}

PLAY RECAP **************************************************************************************************************************************************************************************************************************************************************************
localhost                  : ok=2    changed=1    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   
$ cat /tmp/output 


  "ab"
$

Code of Conduct

  • I agree to follow the Ansible Code of Conduct
@ansibot
Copy link
Contributor

ansibot commented Apr 22, 2023

Files identified in the description:

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

@ansibot ansibot added affects_2.14 bug This issue/PR relates to a bug. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. labels Apr 22, 2023
@corubba
Copy link
Author

corubba commented Apr 23, 2023

While debugging this I found the "point of quote removal" is the call to ast.literal_eval in ansible_native_concat, which makes this issue very much related to issue #79083. I did not find a workaround using the |string filter, but using either """a" "b""" (ast.literal_eval strips the outer triple-quotes, and then double quotes are put around the whole thing again) or "" 'a" "b' "" (ast.literal_eval removes the two outer sets of double quotes and the single quotes, and then double quotes are put around the whole thing again) works; but that's rather ridiculous.

When removing the jinja expression from the playbook, the template uses ansible_eval_concat instead of ansible_native_concat. Could it be that the jinja env for the playbook/task (which uses native mode) bleeds/leaks into the env used for the template itself? Removing the jinja2 override from the template will make it use ansible_concat.

@mkrizek
Copy link
Contributor

mkrizek commented Apr 24, 2023

The issue with turning "a" "b" into "ab" is a known issue that we have seen in #67919. As you have correctly identified it is a result of Python's ast.literal_eval that the jinja2_native feature relies on for converting strings into native types. While it is an expected result of ast.literal_eval it is likely not something users of Ansible expect and we may need to implement some kind of heuristic to prevent such conversion.

A workaround in a template in a play could be:

- debug:
    var: s1
  vars:
    s1: "{{ \"'a' 'b'\"|string }}"

As for this particular issue, the following diff appears to fix it:

diff --git a/lib/ansible/template/__init__.py b/lib/ansible/template/__init__.py
index a623ba52fe9..e3976655491 100644
--- a/lib/ansible/template/__init__.py
+++ b/lib/ansible/template/__init__.py
@@ -593,6 +593,7 @@ class Templar:
         # environment there only to override it below
         new_env = object.__new__(environment_class)
         new_env.__dict__.update(self.environment.__dict__)
+        new_env.concat = environment_class.concat
 
         new_templar = object.__new__(Templar)
         new_templar.__dict__.update(self.__dict__)

but it needs more testing, I will work on making the diff into a PR.

@mkrizek mkrizek added native_jinja issues related to jinja native types and removed needs_triage Needs a first human triage before being processed. labels Apr 24, 2023
@corubba
Copy link
Author

corubba commented Apr 25, 2023

Thank you very much for the confirmation and the proposed fix. In the hopes of providing a bit of that needed "more testing", I ran it through the following test matrix. The only change in behaviour was that ansible_native_concat is never used in the template, which seems desirable given that the template task is always non-native by design.

jinja2_native playbook with expression override in template without fix with fix
concat function in playbook concat function in template my usecase works concat function in playbook concat function in template my usecase works
✔️ ✔️ ✔️ ansible_native_concat ansible_native_concat ansible_native_concat ansible_eval_concat ✔️
✔️ ✔️ ansible_native_concat ansible_concat ✔️ ansible_native_concat ansible_concat ✔️
✔️ ✔️ ansible_native_concat ansible_eval_concat ✔️ ansible_native_concat ansible_eval_concat ✔️
✔️ ansible_native_concat ansible_concat ✔️ ansible_native_concat ansible_concat ✔️
✔️ ✔️ ansible_eval_concat ansible_eval_concat ✔️ ansible_eval_concat ansible_eval_concat ✔️
✔️ ansible_eval_concat ansible_concat ✔️ ansible_eval_concat ansible_concat ✔️
✔️ ansible_eval_concat ansible_eval_concat ✔️ ansible_eval_concat ansible_eval_concat ✔️
ansible_eval_concat ansible_concat ✔️ ansible_eval_concat ansible_concat ✔️

In the meantime I use the below workaround. The not-trimmed-anymore (indentation-) whitespace at the beginning of the call block prevents ast.literal_eval from treating the returned content as string literals and concatenating them; instead it's returned as-is. The now additional whitespace is trimmed away on the calling side, giving me the wanted result. The original setup where I ran into this issue is a lot more complex, and this is the most practical solution I found. Plus it has the same result with and without the fix.

diff --git a/template.j2 b/template.j2
index 07a41a9..14bf684 100644
--- a/template.j2
+++ b/template.j2
@@ -1,9 +1,9 @@
 #jinja2: foo:True
 
 {% macro my_macro() %}
-  {{ caller() }}
+  {{ caller() |trim }}
 {% endmacro %}
 
-{% call my_macro() -%}
+{% call my_macro() %}
   "a" "b"
 {% endcall %}

mkrizek added a commit to mkrizek/ansible that referenced this issue May 3, 2023
Instead of using Templar.environment in Templar.do_template for
accessing/mutating the environment, myenv local variable should be used
because it is the environment used for actual templating. It can either
point to Templar.environment or newly created environment overlay.

Fixes ansible#80605
@ansibot ansibot added the has_pr This issue has an associated PR. label May 3, 2023
@mkrizek
Copy link
Contributor

mkrizek commented May 3, 2023

@corubba I ended up doing a little different fix in #80705 than the one I posted in my previous comment, in case you want to run your tests against the PR too.

In any case thank you for a great bug report and testing!

@corubba
Copy link
Author

corubba commented May 3, 2023

Glad I could be of any help. I tested the PR with both the minimal example above as well as the full setup where I first encountered the issue, both work as expected. So I can confirm the PR fixes my issue.

And just for prosperity I ran through the test matrix again. No change in the playbook, but the template now always uses the ansible_concat function. Looking back, it did strike me as a bit odd that the template switched between ansible_concat and ansible_eval_concat in unison with the override in the template, but wasn't thinking much of it.

jinja2_native playbook with expression override in template without fix (devel) with fix (PR#80705)
concat function in playbook concat function in template my usecase works concat function in playbook concat function in template my usecase works
✔️ ✔️ ✔️ ansible_native_concat ansible_native_concat ansible_native_concat ansible_concat ✔️
✔️ ✔️ ansible_native_concat ansible_concat ✔️ ansible_native_concat ansible_concat ✔️
✔️ ✔️ ansible_native_concat ansible_eval_concat ✔️ ansible_native_concat ansible_concat ✔️
✔️ ansible_native_concat ansible_concat ✔️ ansible_native_concat ansible_concat ✔️
✔️ ✔️ ansible_eval_concat ansible_eval_concat ✔️ ansible_eval_concat ansible_concat ✔️
✔️ ansible_eval_concat ansible_concat ✔️ ansible_eval_concat ansible_concat ✔️
✔️ ansible_eval_concat ansible_eval_concat ✔️ ansible_eval_concat ansible_concat ✔️
ansible_eval_concat ansible_concat ✔️ ansible_eval_concat ansible_concat ✔️

Keeping my fingers crossed that the fix will only break a few workflows.

mkrizek added a commit that referenced this issue May 4, 2023
Instead of using Templar.environment in Templar.do_template for
accessing/mutating the environment, myenv local variable should be used
because it is the environment used for actual templating. It can either
point to Templar.environment or newly created environment overlay.

Fixes #80605
mkrizek added a commit to mkrizek/ansible that referenced this issue May 4, 2023
Instead of using Templar.environment in Templar.do_template for
accessing/mutating the environment, myenv local variable should be used
because it is the environment used for actual templating. It can either
point to Templar.environment or newly created environment overlay.

Fixes ansible#80605

(cherry picked from commit 8cd95a8)
mkrizek added a commit to mkrizek/ansible that referenced this issue May 4, 2023
Instead of using Templar.environment in Templar.do_template for
accessing/mutating the environment, myenv local variable should be used
because it is the environment used for actual templating. It can either
point to Templar.environment or newly created environment overlay.

Fixes ansible#80605

(cherry picked from commit 8cd95a8)
sivel pushed a commit that referenced this issue May 9, 2023
Instead of using Templar.environment in Templar.do_template for
accessing/mutating the environment, myenv local variable should be used
because it is the environment used for actual templating. It can either
point to Templar.environment or newly created environment overlay.

Fixes #80605

(cherry picked from commit 8cd95a8)
@ansible ansible locked and limited conversation to collaborators May 11, 2023
sivel pushed a commit that referenced this issue Jun 7, 2023
Instead of using Templar.environment in Templar.do_template for
accessing/mutating the environment, myenv local variable should be used
because it is the environment used for actual templating. It can either
point to Templar.environment or newly created environment overlay.

Fixes #80605

(cherry picked from commit 8cd95a8)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.14 bug This issue/PR relates to a bug. has_pr This issue has an associated PR. module This issue/PR relates to a module. native_jinja issues related to jinja native types
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants