jhaals / ansible-vault

ansible lookup plugin for secrets stored in Vault(by HashiCorp)
BSD 3-Clause "New" or "Revised" License
347 stars 65 forks source link

Feature/set variables #23

Closed richfromm closed 8 years ago

richfromm commented 8 years ago

Once again, the diffs being shown are cumulative, and also including my other outstanding pull requests:

Again, apologies if there is a better way to be handling this. If you just want to see the commits for this issue, it's the 3 most recent ones, on 07 Nov 2016:

$ git diff HEAD^^^ HEAD
diff --git a/vault.py b/vault.py
index 753df27..707f73e 100644
--- a/vault.py
+++ b/vault.py
@@ -57,11 +57,16 @@ class LookupModule(LookupBase):
         except:
             field = None

-        url = os.getenv('VAULT_ADDR')
+        # the environment variable takes precendence over the Ansible variable.
+        # Ansible variables are passed via "variables" in ansible 2.x, "inject" in 1.9.x
+        url = os.getenv('VAULT_ADDR') or (variables or inject).get('vault_addr')
         if not url:
-            raise AnsibleError('VAULT_ADDR environment variable is missing')
+            raise AnsibleError('Vault address not set. Specify with'
+                               ' VAULT_ADDR environment variable or vault_addr Ansible variable')

-        # the environment variable takes precedence over the file-based token
+        # the environment variable takes precedence over the file-based token.
+        # intentionally do *not* support setting this via an Ansible variable,
+        # so as not to encourage bad security practices.
         token = os.getenv('VAULT_TOKEN')
         if not token:
             try:
@@ -74,8 +79,8 @@ class LookupModule(LookupBase):
             raise AnsibleError('Vault authentication token missing. Specify with'
                                ' VAULT_TOKEN environment variable or $HOME/.vault-token')

-        cafile = os.getenv('VAULT_CACERT')
-        capath = os.getenv('VAULT_CAPATH')
+        cafile = os.getenv('VAULT_CACERT') or (variables or inject).get('vault_cacert')
+        capath = os.getenv('VAULT_CAPATH') or (variables or inject).get('vault_capath')
         try:
             if cafile or capath:
                 context = ssl.create_default_context(cafile=cafile, capath=capath)
rich@rich-trusty [15:20:02 Mon Nov 07] ~/git/github.com/locationlabs/ansible-vault
jhaals commented 8 years ago

Hi Rich, thanks for submitting features and updates to this project. However it's really hard to review /discuss the changes when everything is in one PR. It would be really nice if you could re-open with one PR per feature.

richfromm commented 8 years ago

I tried to open with one pull request per feature (that's why I did each on a separate feature branch), but that's not what github ended up doing. I will try again...

richfromm commented 8 years ago

I can redo this once other outstanding pull requests are complete, but it appears to me that it's not possible in github to create separate pull requests that are truly separate (and not including the preceding commits) if there are open dependent pull requests.

richfromm commented 8 years ago

Replaced by #27