ansible-collections / community.general

Ansible Community General Collection
https://galaxy.ansible.com/ui/repo/published/community/general/
GNU General Public License v3.0
823 stars 1.52k forks source link

passwordstore removes trailing newlines #3616

Open alan-strohm opened 3 years ago

alan-strohm commented 3 years ago

Summary

lookup('community.general.passwordstore', 'foo returnall=true') never includes a trailing newline even if pass show does return one.

Issue Type

Bug Report

Component Name

plugins/lookup/passwordstore.py

Ansible Version

$ ansible --version
ansible [core 2.11.6] 
  config file = /home/astrohm/src/github.com/alan-strohm/config/ansible.cfg
  configured module search path = ['/home/astrohm/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/lib/python3.8/dist-packages/ansible
  ansible collection location = /home/astrohm/.ansible/collections:/usr/share/ansible/collections     
  executable location = /usr/local/bin/ansible
  python version = 3.8.10 (default, Sep 28 2021, 16:10:42) [GCC 9.3.0]
  jinja version = 2.10.1
  libyaml = True

Community.general Version

$ ansible-galaxy collection list community.general
# /usr/local/lib/python3.8/dist-packages/ansible_collections
Collection        Version
----------------- -------
community.general 3.8.0

Configuration

$ ansible-config dump --only-changed

OS / Environment

Ubuntu 20.04.3 LTS (GNU/Linux 4.19.84-microsoft-standard+ x86_64)

Steps to Reproduce

❯ pass insert -m test
An entry already exists for test. Overwrite it? [y/N] y
Enter contents of test and press Ctrl+D when finished:

foo
bar
[master 84f3730] Add given password for test to store.
 1 file changed, 0 insertions(+), 0 deletions(-)
 rewrite test.gpg (100%)
❯ pass show test | cat -vet
foo$
bar$
---
- hosts: localhost
  tasks:
    - debug: msg="{{ lookup('community.general.passwordstore', 'test returnall=true') }}"

Expected Results

"msg": "foo\nbar\n"

Actual Results

❯ ansible-playbook -vvv test1.yml ansible-playbook [core 2.11.6] config file = /home/astrohm/src/github.com/alan-strohm/config/ansible.cfg configured module search path = ['/home/astrohm/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules'] ansible python module location = /usr/local/lib/python3.8/dist-packages/ansible ansible collection location = /home/astrohm/.ansible/collections:/usr/share/ansible/collections
executable location = /usr/local/bin/ansible-playbook python version = 3.8.10 (default, Sep 28 2021, 16:10:42) [GCC 9.3.0] jinja version = 2.10.1 libyaml = True Using /home/astrohm/src/github.com/alan-strohm/config/ansible.cfg as config file 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' 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: test1.yml **1 plays in test1.yml

PLAY [localhost] *** TASK [Gathering Facts] *task path: /home/astrohm/src/github.com/alan-strohm/config/test1.yml:2 <127.0.0.1> ESTABLISH LOCAL CONNECTION FOR USER: astrohm <127.0.0.1> EXEC /bin/sh -c 'echo ~astrohm && sleep 0' <127.0.0.1> EXEC /bin/sh -c '( umask 77 && mkdir -p "echo /home/astrohm/.ansible/tmp"&& mkdir "echo /home/astrohm/.ansible/tmp/ansible-tmp-1635140287.845894-20049-260425160856505" && echo ansible-tmp-1635140287.845894-20049-260425160856505="echo /home/astrohm/.ansible/tmp/ansible-tmp-1635140287.845894-20049-260425160856505" ) && sleep 0' Using module file /usr/local/lib/python3.8/dist-packages/ansible/modules/setup.py <127.0.0.1> PUT /home/astrohm/.ansible/tmp/ansible-local-2004441t38kxl/tmpi3cge7u6 TO /home/astrohm/.ansible/tmp/ansible-tmp-1635140287.845894-20049-260425160856505/AnsiballZ_setup.py <127.0.0.1> EXEC /bin/sh -c 'chmod u+x /home/astrohm/.ansible/tmp/ansible-tmp-1635140287.845894-20049-260425160856505/ /home/astrohm/.ansible/tmp/ansible-tmp-1635140287.845894-20049-260425160856505/AnsiballZ_setup.py && sleep 0' <127.0.0.1> EXEC /bin/sh -c '/usr/bin/python3 /home/astrohm/.ansible/tmp/ansible-tmp-1635140287.845894-20049-260425160856505/AnsiballZ_setup.py && sleep 0' <127.0.0.1> EXEC /bin/sh -c 'rm -f -r /home/astrohm/.ansible/tmp/ansible-tmp-1635140287.845894-20049-260425160856505/ > /dev/null 2>&1 && sleep 0' ok: [localhost] META: ran handlers

TASK [debug] *****task path: /home/astrohm/src/github.com/alan-strohm/config/test1.yml:4 ok: [localhost] => { "msg": "foo\nbar" } META: ran handlers META: ran handlers

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

Code of Conduct

ansibullbot commented 3 years ago

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

alan-strohm commented 3 years ago

I'd be open to submitting a pull request to fix this. But I do note that the current tests explicitly check that the trailing newline is removed:

https://github.com/ansible-collections/community.general/blob/350380ba8c91030b69ec4fe2b087fb62ee82c389/tests/integration/targets/lookup_passwordstore/tasks/tests.yml#L121

So I'm not sure how folks would want to handle dependencies on the current behavior.

felixfontein commented 3 years ago

You would have to add a new option to control this behavior. We try to be as backwards-compatible as possible, so simply changing this behavior would be a breaking change and wouldn't get merged with a good reason.

alan-strohm commented 3 years ago

Thanks for the feedback!

Adding \n: msg="{{ lookup('community.general.passwordstore', 'test returnall=true') }}\n" Seems better (less obtrusive, more readable) than: msg="{{ lookup('community.general.passwordstore', 'test returnall=true dontstriptrailingnewline=true') }}"

If backwards compatibility is that important, I might just update the docs on returnall to warn folks of this behavior.