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

Add ok_if_missing option to passwordstore lookup (#28702) #28908

Closed
wants to merge 3 commits into from
Closed

Add ok_if_missing option to passwordstore lookup (#28702) #28908

wants to merge 3 commits into from

Conversation

nrwahl2
Copy link
Contributor

@nrwahl2 nrwahl2 commented Sep 1, 2017

SUMMARY

Add ok_if_missing option to passwordstore lookup (#28702)

There is currently no way to default to another value if the password is not found. This PR adds an ok_if_missing option (default False). When set to True, it causes a missing password in a passwordstore lookup to return a False value rather than failing. This allows a playbook to use a Jinja2 default value easily.

Side note: I was reading through the documentation, and it's not clear to me what these two pieces mean. We should clarify them.

Create password and overwrite the password if it exists. As a bonus, this module includes the old password inside the pass file::

    password="{{ lookup('passwordstore', 'example/test create=true overwrite=true')}}"

Return the value for user in the KV pair 'user: username'::

    password="{{ lookup('passwordstore', 'example/test subkey=user')}}"
ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

plugins/lookups/passwordstore

ANSIBLE VERSION
ansible 2.4.0 (issue/28702 fccd9915eb) last updated 2017/08/31 22:20:35 (GMT -500)
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/home/reid/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /home/reid/git/ansible/lib/ansible
  executable location = /home/reid/git/ansible/bin/ansible
  python version = 2.7.13 (default, Jun 26 2017, 10:20:05) [GCC 7.1.1 20170622 (Red Hat 7.1.1-3)]
ADDITIONAL INFORMATION

Test cases:

---
- hosts: localhost
  gather_facts: no
  vars:
    password: "{{ lookup('passwordstore', 'example/test') }}"
  tasks:
    - name: Print password
      debug: var=password

[reid@laptop ~/git]$ ansible-playbook pwstore.yml -i hosts.ini 
...
TASK [Print password] *********************************************************************************************************************************************************************************************
fatal: [localhost]: FAILED! => {"msg": "{{ lookup('passwordstore', 'example/test') }}: An unhandled exception occurred while running the lookup plugin 'passwordstore'. Error was a <class 'ansible.errors.AnsibleError'>, original message: passname: example/test not found, use create=True"}
	to retry, use: --limit @/home/reid/git/pwstore.retry

PLAY RECAP ********************************************************************************************************************************************************************************************************
localhost                  : ok=0    changed=0    unreachable=0    failed=1   
---
- hosts: localhost
  gather_facts: no
  vars:
    password: "{{ lookup('passwordstore', 'example/test ok_if_missing=True') }}"
  tasks:
    - name: Print password
      debug: var=password

[reid@laptop ~/git]$ ansible-playbook pwstore.yml -i hosts.ini 
...
TASK [Print password] *********************************************************************************************************************************************************************************************
ok: [localhost] => {
    "password": []
}

PLAY RECAP ********************************************************************************************************************************************************************************************************
localhost                  : ok=1    changed=0    unreachable=0    failed=0  
# default not using True second parameter, which catches False values in addition to None values
---
- hosts: localhost
  gather_facts: no
  vars:
    password: "{{ lookup('passwordstore', 'example/test ok_if_missing=True') | default('TEST') }}"
  tasks:
    - name: Print password
      debug: var=password

[reid@laptop ~/git]$ ansible-playbook pwstore.yml -i hosts.ini 
...
TASK [Print password] *********************************************************************************************************************************************************************************************
ok: [localhost] => {
    "password": []
}

PLAY RECAP ********************************************************************************************************************************************************************************************************
localhost                  : ok=1    changed=0    unreachable=0    failed=0  
---
- hosts: localhost
  gather_facts: no
  vars:
    password: "{{ lookup('passwordstore', 'example/test ok_if_missing=True') | default('TEST', true) }}"
  tasks:
    - name: Print password
      debug: var=password

[reid@laptop ~/git]$ ansible-playbook pwstore.yml -i hosts.ini 
...
TASK [Print password] *********************************************************************************************************************************************************************************************
ok: [localhost] => {
    "password": "TEST"
}

PLAY RECAP ********************************************************************************************************************************************************************************************************
localhost                  : ok=1    changed=0    unreachable=0    failed=0  
# ok_if_missing not set; default does not handle failures
---
- hosts: localhost
  gather_facts: no
  vars:
    password: "{{ lookup('passwordstore', 'example/test') | default('TEST', true) }}"
  tasks:
    - name: Print password
      debug: var=password

[reid@laptop ~/git]$ ansible-playbook pwstore.yml -i hosts.ini 
...
TASK [Print password] *********************************************************************************************************************************************************************************************
fatal: [localhost]: FAILED! => {"msg": "{{ lookup('passwordstore', 'example/test') | default('TEST', true) }}: An unhandled exception occurred while running the lookup plugin 'passwordstore'. Error was a <class 'ansible.errors.AnsibleError'>, original message: passname: example/test not found, use create=True"}
	to retry, use: --limit @/home/reid/git/pwstore.retry

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

@ansibot ansibot added affects_2.4 This issue/PR affects Ansible v2.4 c:plugins/lookup feature_pull_request needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Sep 1, 2017
@nrwahl2
Copy link
Contributor Author

nrwahl2 commented Sep 1, 2017

Noticing there's no integration test for passwordstore. I'm not familiar with the passwordstore tool beyond what I've done with this PR. If someone would like to add tests, that'd be great. If not, I can add it to my backlog.

@s-hertel s-hertel removed the needs_triage Needs a first human triage before being processed. label Sep 1, 2017
@ansibot
Copy link
Contributor

ansibot commented Sep 1, 2017

Copy link
Contributor

@dharmabumstead dharmabumstead left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs OK with a minor edit. Needs core team tech review before merge. Thanks @nrwahl2!

@nrwahl2
Copy link
Contributor Author

nrwahl2 commented Sep 1, 2017

@dharmabumstead : Thanks! I had used "Return" rather than "Returns" to maintain consistency with existing documentation, as well as PEP-257 recommendations on docstrings (which, granted, this is not):

Existing doc:

Basic lookup. Fails if example/test doesn't exist::

    password="{{ lookup('passwordstore', 'example/test')}}"

Create pass with random 16 character password. If password exists just give the password::

    password="{{ lookup('passwordstore', 'example/test create=true')}}"

Different size password::

    password="{{ lookup('passwordstore', 'example/test create=true length=42')}}"

Create password and overwrite the password if it exists. As a bonus, this module includes the old password inside the pass file::

    password="{{ lookup('passwordstore', 'example/test create=true overwrite=true')}}"

Return the value for user in the KV pair user: username::

    password="{{ lookup('passwordstore', 'example/test subkey=user')}}"

Return the entire password file content::

    password="{{ lookup('passwordstore', 'example/test returnall=true')}}"

PEP-257 recommendation:

The docstring is a phrase ending in a period. It prescribes the function or method's effect as a command ("Do this", "Return that"), not as a description; e.g. don't write "Returns the pathname ...".

@dharmabumstead
Copy link
Contributor

@nrwahl2 you are correct, and I was in a hurry. Changed both instances to just 'return' instead of 'returns'. Thanks.

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Sep 12, 2017
@ansibot ansibot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Oct 14, 2017
@Akasurde
Copy link
Member

Akasurde commented Jan 1, 2018

@nrwahl2 Could you please rebase so that we can make it to 2.5 release?

@ansibot ansibot added docs This issue/PR relates to or includes documentation. feature This issue/PR relates to a feature request. and removed docs_pull_request labels Mar 1, 2018
@itdependsnetworks
Copy link
Contributor

Is this making it into 2.5?

@bcoca
Copy link
Member

bcoca commented Mar 2, 2018

i created similar toggle, but general to all lookups #35932

@dharmabumstead
Copy link
Contributor

Closing this one. Feel free to rebase and reopen.

@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.4 This issue/PR affects Ansible v2.4 c:plugins/lookup docs This issue/PR relates to or includes documentation. feature This issue/PR relates to a feature request. needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. plugins/lookup stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants