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

Ansible tries to parse contents of diffs #45717

Closed
duijf opened this issue Sep 17, 2018 · 2 comments · Fixed by #45744
Closed

Ansible tries to parse contents of diffs #45717

duijf opened this issue Sep 17, 2018 · 2 comments · Fixed by #45744
Labels
affects_2.6 This issue/PR affects Ansible v2.6 bug This issue/PR relates to a bug. module This issue/PR relates to a module. python3 support:core This issue/PR relates to code supported by the Ansible Engineering Team.

Comments

@duijf
Copy link

duijf commented Sep 17, 2018

I've tried to search for this problem in the issue tracker with the keywords --diff, crash, and parse, but haven't found anything.

SUMMARY

Diff printing seems to attempt to resolve variables in diffed files/parse them.

ISSUE TYPE
  • Bug Report
COMPONENT NAME

Diff mode and copy module.

ANSIBLE VERSION
ansible 2.6.2                                                                                          
  config file = /home/laurens/repos/channable/devops/ansible/ansible.cfg
  configured module search path = ['/home/laurens/repos/channable/devops/ansible/modules']
  ansible python module location = /home/laurens/env/devops/lib/python3.6/site-packages/ansible
  executable location = /home/laurens/env/devops/bin/ansible
  python version = 3.6.5 (default, Apr  1 2018, 05:46:30) [GCC 7.3.0]

and

ansible 2.8.0.dev0
  config file = None
  configured module search path = ['/home/laurens/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/laurens/env/devops/lib/python3.6/site-packages/ansible
  executable location = /home/laurens/env/devops/bin/ansible
  python version = 3.6.5 (default, Apr  1 2018, 05:46:30) [GCC 7.3.0]
CONFIGURATION

No output. Running with default config.

OS / ENVIRONMENT

Originally seen in a more complex setting, but reproducible when running on Ubuntu 18.04, deploying with ansible_connection=local.

STEPS TO REPRODUCE

I've created a git repository which has a minimal test case: https://github.com/duijf/ansible-bug-report

Please tell me if anything is unclear.

EXPECTED RESULTS

Ansible does not crash and reports changed results correctly.

ACTUAL RESULTS

Ansible crashes.

~/repos/channable/ansible-bug-report master
devops ❯ ansible-playbook -i inventory playbook.yml --diff

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

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

TASK [problem : copy problematic file] *****************************************************************
--- before
+++ after: /home/laurens/repos/channable/ansible-bug-report/roles/problem/files/problem.yml
@@ -0,0 +1,7 @@
+The problem was originally found when copying prometheus
+templates, which can contain things like {{ $labels.instance }}.
+
+If this were a file in the `templates/` directory, I'd agree
+that this is asking for problems. However, this only gives
+problems when registering things to a variable and acting on
+it.                                                                                                    
                                                                                                        
changed: [localhost]                                                                                    
                                                                                                        
TASK [problem : inspect results] ***********************************************************************
fatal: [localhost]: FAILED! => {"msg": "An unhandled exception occurred while templating '{'diff': [{'before': '', 'after_header': '/home/laurens/repos/channable/ansible-bug-report/roles/problem/files/problem.yml', 'after': \"The problem was originally found when copying prometheus\\ntemplates, which can contain things like {{ $labels.instance }}.\\n\\nIf this were a file in the `templates/` directory, I'd agree\\nthat this is asking for problems. However, this only gives\\nproblems when registering things to a variable and acting on\\nit.\\n\"}], 'src': '/home/laurens/.ansible/tmp/ansible-tmp-1537170980.7106235-254323551122550/source', 'changed': True, 'group': 'laurens', 'uid': 1000, 'dest': '/tmp/problem.yml', 'checksum': '42a9a3edeb77bac53775ea092fab3cad962c0872', 'md5sum': '6e6ce6086486a66f1cdab91b98d0b7f0', 'owner': 'laurens', 'state': 'file', 'gid': 1000, 'mode': '0664', 'size': 308, 'failed': False}'. Error was a <class 'ansible.errors.AnsibleError'>, original message: template error while templating string: unexpected char '$' at 101. String: The problem was originally found when copying prometheus\ntemplates, which can contain things like {{ $labels.instance }}.\n\nIf this were a file in the `templates/` directory, I'd agree\nthat this is asking for problems. However, this only gives\nproblems when registering things to a variable and acting on\nit.\n"}                                                                       
        to retry, use: --limit @/home/laurens/repos/channable/ansible-bug-report/playbook.retry         
                                                                                                        
PLAY RECAP *********************************************************************************************
localhost                  : ok=2    changed=1    unreachable=0    failed=1   

This only happens when:

  • --diff is used.
  • The file is changed, so the diff contains the variables.

It seems like Ansible is trying to interpret the output of the diff, since it prints unexpected char '$' at 101.

@duijf
Copy link
Author

duijf commented Sep 17, 2018

For anyone reading this from Google searches. You can work around this problem on a per-task basis by setting:

- name: mytask
  copy:
    src: whatevs
    dest: whatevs
  diff: no

@ansibot ansibot added affects_2.6 This issue/PR affects Ansible v2.6 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. python3 support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Sep 17, 2018
@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label Sep 17, 2018
@sivel
Copy link
Member

sivel commented Sep 17, 2018

We've tracked this down to a python3 compatibility issue, and are discussing the correct fix.

sivel added a commit that referenced this issue Sep 20, 2018
#45744)

* Ensure that the src file contents is converted to unicode in diff info. Fixes #45717

* Fix up and cleanup

* The diff functionality in the callback plugins should have the
  to_text() calls removed since we're now doing it in ActionBase
* catching of UnicodeError and warnings in the callback diff
  functionality from 61d01f5 haven't been
  needed since we switched to to_text so remove them.
* Add a note to ActionBase's diff function giving an example of when the
  diff function will be inaccurate and how to fix it

* Fix callback get_diff() tests

I believe the unittests of callback's get_diff() were wrong.  They were
sending in a list where strings were expected.  Because previous code
was transforming the lists into strings via their repr, the previous
tests did not fail but they would have formatted the test cases output
in an odd way if we had looked at it.
sivel added a commit to sivel/ansible that referenced this issue Jan 8, 2019
…e in diff info (ansible#45744)

* Ensure that the src file contents is converted to unicode in diff info. Fixes ansible#45717

* Fix up and cleanup

* The diff functionality in the callback plugins should have the
  to_text() calls removed since we're now doing it in ActionBase
* catching of UnicodeError and warnings in the callback diff
  functionality from 61d01f5 haven't been
  needed since we switched to to_text so remove them.
* Add a note to ActionBase's diff function giving an example of when the
  diff function will be inaccurate and how to fix it

* Fix callback get_diff() tests

I believe the unittests of callback's get_diff() were wrong.  They were
sending in a list where strings were expected.  Because previous code
was transforming the lists into strings via their repr, the previous
tests did not fail but they would have formatted the test cases output
in an odd way if we had looked at it.
(cherry picked from commit 95e77ac)

Co-authored-by: Matt Martz <matt@sivel.net>
abadger pushed a commit that referenced this issue Jan 9, 2019
…e in diff info (#45744)

* Ensure that the src file contents is converted to unicode in diff info. Fixes #45717

* Fix up and cleanup

* The diff functionality in the callback plugins should have the
  to_text() calls removed since we're now doing it in ActionBase
* catching of UnicodeError and warnings in the callback diff
  functionality from 61d01f5 haven't been
  needed since we switched to to_text so remove them.
* Add a note to ActionBase's diff function giving an example of when the
  diff function will be inaccurate and how to fix it

* Fix callback get_diff() tests

I believe the unittests of callback's get_diff() were wrong.  They were
sending in a list where strings were expected.  Because previous code
was transforming the lists into strings via their repr, the previous
tests did not fail but they would have formatted the test cases output
in an odd way if we had looked at it.
(cherry picked from commit 95e77ac)

Co-authored-by: Matt Martz <matt@sivel.net>
@ansible ansible locked and limited conversation to collaborators Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.6 This issue/PR affects Ansible v2.6 bug This issue/PR relates to a bug. module This issue/PR relates to a module. python3 support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants