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

allow user to control if vault decryption is fatal #37019

Closed
wants to merge 8 commits into from

Conversation

bcoca
Copy link
Member

@bcoca bcoca commented Mar 5, 2018

SUMMARY

fixes #13244 #32687
related #37161 #31141

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

valut

ANSIBLE VERSION
2.7

@ansibot ansibot added feature This issue/PR relates to a feature 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 Mar 5, 2018
@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label Mar 5, 2018
@AlanCoding
Copy link
Member

AlanCoding commented Mar 6, 2018

+1

@AlanCoding
Copy link
Member

AlanCoding commented Mar 12, 2018

I've confirmed my hunch.

You can see that both cases still fail. It doesn't really accomplish a pass-through if the setting is set to False, as one would expect. There is a different message emitted in both cases, so your conditional really is intercepting the missing secrets case, but there appears to be more cases that also need to be handled.

I would still like to have this, I would be happy to incorporate use of this constant into my other PR.

false case
$ ANSIBLE_ERROR_ON_VAULT_FAIL=false ansible-playbook -i scripts/vault/awx_redumper.py debugging/hostvars_print.yml

PLAY [all] ****************************************************************************************************************************************************************************

TASK [debug] **************************************************************************************************************************************************************************
 [WARNING]: Vault decryption failed: Attempting to decrypt but no vault secrets found

fatal: [foobar]: FAILED! => {"msg": "An unhandled exception occurred while templating '{{ hostvars[inventory_hostname] }}'. Error was a <type 'exceptions.UnboundLocalError'>, original message: local variable 'plaintext' referenced before assignment"}
	to retry, use: --limit @/Users/alancoding/Documents/repos/ansible-inventory-file-examples/debugging/hostvars_print.retry

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

true case
$ ANSIBLE_ERROR_ON_VAULT_FAIL=true ansible-playbook -i scripts/vault/awx_redumper.py debugging/hostvars_print.yml

PLAY [all] ****************************************************************************************************************************************************************************

TASK [debug] **************************************************************************************************************************************************************************
fatal: [foobar]: FAILED! => {"msg": "An unhandled exception occurred while templating '{{ hostvars[inventory_hostname] }}'. Error was a <class 'ansible.parsing.vault.AnsibleVaultError'>, original message: Attempting to decrypt but no vault secrets found"}
	to retry, use: --limit @/Users/alancoding/Documents/repos/ansible-inventory-file-examples/debugging/hostvars_print.retry

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

@bcoca
Copy link
Member Author

bcoca commented Mar 12, 2018

might need to change then, ignore_missing_secrets vs ignore_failed_decrypt, probably need the 2nd toggle a few steps up.

@bcoca
Copy link
Member Author

bcoca commented Mar 13, 2018

@AlanCoding i updated to both cover all types of errors and make the distinction between missing/mismatched secrets vs other general vault errors.

lib/ansible/config/base.yml Outdated Show resolved Hide resolved
lib/ansible/config/base.yml Outdated Show resolved Hide resolved
@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Mar 13, 2018
@ansibot ansibot added the test This PR relates to tests. label Mar 14, 2018
@ansibot
Copy link
Contributor

ansibot commented Mar 14, 2018

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

test/units/parsing/vault/test_vault_editor.py:53:1: E302 expected 2 blank lines, found 1

click here for bot help

@AlanCoding
Copy link
Member

AlanCoding commented Mar 16, 2018

There's a much better solution to the particular use case I was interested in via

#37536
#37531

The particular behavior of "don't decrypt vault values" can reasonably be accomplished with a simple flag on ansible-inventory

@ansibot ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Mar 16, 2018
@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 Mar 24, 2018
@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 Apr 1, 2018
@ansibot ansibot added the affects_2.6 This issue/PR affects Ansible v2.6 label May 19, 2018
@ansibot ansibot removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Jul 12, 2019
@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 Jul 20, 2019
@ansibot ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Aug 8, 2019
@ansibot ansibot added the core_review In order to be merged, this PR must follow the core review workflow. label Aug 8, 2019
@ansibot ansibot added support:community This issue/PR relates to code supported by the Ansible community. and removed stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Jun 3, 2020
@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 Jun 11, 2020
@bcoca bcoca added this to TODO: proposed items for 2.11 in ansible-core 2.11 Jun 30, 2020
@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. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Nov 20, 2020
@ansibot ansibot added pre_azp This PR was last tested before migration to Azure Pipelines. and removed stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Dec 6, 2020
@ansibot ansibot removed the support:community This issue/PR relates to code supported by the Ansible community. label Mar 5, 2021
@mattclay mattclay added affects_2.13 and removed affects_2.6 This issue/PR affects Ansible v2.6 labels Apr 13, 2022
@sivel sivel removed this from TODO: proposed items for 2.11 in ansible-core 2.11 Apr 19, 2022
@bcoca bcoca closed this Aug 24, 2022
@bcoca bcoca deleted the vault_to_warn branch Aug 24, 2022
@ansible ansible locked and limited conversation to collaborators Aug 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.13 feature This issue/PR relates to a feature request. has_issue 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. pre_azp This PR was last tested before migration to Azure Pipelines. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make vault error type configurable
5 participants