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

hashi_vault - Fix regression where VAULT_ADDR is ignored #61

Merged
merged 4 commits into from
Feb 24, 2021

Conversation

briantist
Copy link
Collaborator

@briantist briantist commented Feb 23, 2021

SUMMARY

Fixes #60

Related:

When lowering the precedence of the VAULT_ADDR env var, I failed to take into account that the default value in the option itself would prevent the low pref env var value from ever being used, as implemented.

This would affect any option that has both a default value and a "low preference env var", which at the moment only applies to url / VAULT_ADDR.

This PR fixes that.

First, a unit test catching this is being added, then I will push a commit with the fix.

Note: the unit test only covers this option specifically and could be expanded. This work is actually already in progress in #59 , and I would have surfaced this eventually because of that. I'm keeping this small for this bugfix, but better tests will absolutely be introduced via the refactor.

Affected Users

This bug will have affected version 1.0.0 and 1.1.0, since the env var priority change was released in 1.0.0 via #41 .

Fix Available

Version 1.1.1 of the collection contains this fix:

Workarounds

If upgrading is not possible, the best workaround if you can is to switch to using ANSIBLE_HASHI_VAULT_ADDR or to set the address via ansible.cfg:

[lookup_hashi_vault]
url = https://myvault

If neither of those are possible, setting url explicitly on the plugin call can be done and the value can be read via the env lookup:

- hosts: localhost
  vars:
    vaddr: "{{ lookup('env', 'VAULT_ADDR') }}"
  tasks:
    - debug:
        msg: "{{ lookup('community.hashi_vault.hashi_vault', 'secret=secret/whatever', url=vaddr) }}"
ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

hashi_vault.py

ADDITIONAL INFORMATION

@briantist briantist added the bug Something isn't working label Feb 23, 2021
@briantist briantist self-assigned this Feb 23, 2021
@codecov
Copy link

codecov bot commented Feb 23, 2021

Codecov Report

Merging #61 (ad7c589) into main (1853b19) will increase coverage by 1.64%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #61      +/-   ##
==========================================
+ Coverage   66.58%   68.23%   +1.64%     
==========================================
  Files           9        9              
  Lines         425      447      +22     
  Branches       58       60       +2     
==========================================
+ Hits          283      305      +22     
  Misses        126      126              
  Partials       16       16              
Impacted Files Coverage Δ
plugins/lookup/hashi_vault.py 66.12% <100.00%> (+0.70%) ⬆️
tests/unit/plugins/lookup/test_hashi_vault.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1853b19...2a63a23. Read the comment docs.

@briantist
Copy link
Collaborator Author

@briantist briantist added this to In progress in CI and Testing Improvements Feb 23, 2021
@briantist briantist added this to In progress in Internal Refactor Feb 23, 2021
@briantist
Copy link
Collaborator Author

@elcomtik @transferkraM @erinn would any of you be interested in reviewing or testing this out?

@briantist briantist added this to the v1.1.1 milestone Feb 24, 2021
@briantist briantist merged commit 2b675de into ansible-collections:main Feb 24, 2021
@briantist briantist moved this from In progress to Done in Environment Variable Standardization Feb 24, 2021
@briantist briantist moved this from In progress to Done in CI and Testing Improvements May 22, 2021
@briantist briantist moved this from In progress to Done in Internal Refactor Jun 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

hashi_vault does not use VAULT_ADDR environment variable anymore
3 participants