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 credstash lookup plugin #11778

Merged
merged 6 commits into from Aug 7, 2015
Merged

Conversation

scottcunningham
Copy link
Contributor

Credstash is a small utility for managing secrets using AWS's KMS and DynamoDB: https://github.com/LuminalOSS/credstash

Example usage:

- name: "Test credstash lookup plugin"
  debug: msg="Credstash lookup! {{ lookup('credstash', 'my-github-password') }}"

All credstash options (region, table, etc) are also configurable:

- name: "Test credstash lookup plugin"
  debug: msg="Credstash lookup! {{ lookup('credstash', 'my-other-password', region='us-west-1') }}"

I have some small questions about contributing. This lookup plugin has a dependency on the credstash module. Is there a way for me to add this as a dependency for the plugin? Is there also a good way for me to add documentation?

from ansible.errors import AnsibleError
from ansible.plugins.lookup import LookupBase

import credstash
Copy link
Member

Choose a reason for hiding this comment

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

we should capture possible import exception and explain that this lookup requires the credstash python library when it fails

@bcoca
Copy link
Member

bcoca commented Jul 30, 2015

for documentation add it and an example to docsites/rst//playbooks_lookups.rst

@scottcunningham
Copy link
Contributor Author

Great, thanks a lot for the feedback @bcoca. I've added both now. The build is failing but it looks unrelated to me:

Compiling lib/ansible/module_utils/vmware.py ...
  File "lib/ansible/module_utils/vmware.py", line 125
    except vim.fault.InvalidLogin as invalid_login:

@scottcunningham
Copy link
Contributor Author

Fixed exception handling syntax that would've caused an error in Python 2.4.

Could I get an update on this please? Is there anything else that I can do to help move this forward?

@bcoca
Copy link
Member

bcoca commented Aug 5, 2015

no, sorry, just for us to have enough time to review and merge


CREDSTASH_INSTALLED = False

try:
Copy link
Member

Choose a reason for hiding this comment

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

this should be in the run method, otherwise ansible itself will fail to run

Copy link
Contributor

Choose a reason for hiding this comment

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

By this we mean the

if not CREDSTASH_INSTALLED:
    raise AnsibleError([...])

If importring credstash takes a long time it might be good to move that to inside the lookup module as well but that's an unrelated issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Very good point - thanks! I've moved this to inside of the run function.

I don't think that credstash takes a particularly long time to import, but if you feel it'd be best to keep it inside the module then I can move it there.

@scottcunningham
Copy link
Contributor Author

Update - I've fixed the issues raised above. Thanks a lot for taking the time to check this out.

bcoca added a commit that referenced this pull request Aug 7, 2015
@bcoca bcoca merged commit 3c57018 into ansible:devel Aug 7, 2015
@scottcunningham
Copy link
Contributor Author

Thank you! :)

@rowleyaj rowleyaj deleted the add_credstash_plugin branch August 11, 2015 11:32
@ansibot ansibot added feature This issue/PR relates to a feature request. and removed feature_pull_request labels Mar 4, 2018
@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
feature This issue/PR relates to a feature request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants