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 option to allow users to ignore decryption errors with ansible-inventory #78765

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from

Conversation

AlanCoding
Copy link
Member

SUMMARY

This adds an option to allow ignoring decryption errors from content inside of group_vars/ and host_vars/ folders.

This is really intended for use coupled with ansible-inventory, because the feature was added to that to support pass-through of encrypted variables. If you use that, it looks like this:

    "raleigh": {
        "hosts": [
            "host1",
            "host2"
        ],
        "vars": {
            "should_be_artemis_here": {
                "__ansible_vault": "$ANSIBLE_VAULT;1.2;AES256;alan\n30386264646430643536336230313232653130643332356531633437363837323430663031356364\n3836313935643038306263613631396136663634613066650a303838613532313236663966343433\n37636234366130393131616631663831383237653761373533363666303361333662373664336261\n6136313463383061330a633835643434616562633238383530356632336664316366376139306135\n3534"
            }
        }
    },

from example.

This is very useful, but it creates a problem. We would like to use the output from ansible-inventory to save an inventory snapshot, put it in the database, and produce a copy of it for later use. Vault credentials can latter be used to decrypt the secret from the example above... but it's also common to encrypt a file and this trick doesn't work for that.

Subsequently, we still have a lot of complaints from AWX users that the inventory process is not compatible with their workflow. See ansible/awx#12829

I have an example to demonstrate that and the proposed fix here:

https://github.com/AlanCoding/Ansible-inventory-file-examples/tree/master/vault/file_vars

A key point here is that many users co-locate their inventory with their playbook (same folder), so they really just want ansible-inventory to ignore vault variables.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

lib/ansible/plugins/vars/host_group_vars.py

ADDITIONAL INFORMATION

I would be happy to write tests for this, but the best way for me would be as a bash script, because ansible-inventory is really the important thing, and that requires the inventory structure along with the associated vars files. I can't quite figure out where to do that yet.

@ansibot ansibot added affects_2.14 core_review In order to be merged, this PR must follow the core review workflow. 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 Sep 13, 2022
@bcoca
Copy link
Member

bcoca commented Sep 13, 2022

related #37019

@bcoca
Copy link
Member

bcoca commented Sep 13, 2022

note that while this is the only vars plugin we ship, it is not the only one in use

@ansibot
Copy link
Contributor

ansibot commented Sep 13, 2022

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

lib/ansible/plugins/vars/host_group_vars.py:0:0: option-incorrect-version-added: version_added for new option (vault_error_behavior) should be '2.14'. Currently StrictVersion ('0.0')

click here for bot help

@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. 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 Sep 13, 2022
@AlanCoding
Copy link
Member Author

Yes, and I fully want to use this on its own in AWX, but over the long term I expect the plugin here will diverge and there will be problems. That's why I thought the best solution, ultimately, is to contribute it upstream. I wish that I could subclass the plugin and override just the behavior I want, but that's very brittle because the plugin here isn't written in a way I can do that to begin with, and because the DOCUMENTATION string creates a lot of problems for subclassing.

@bcoca
Copy link
Member

bcoca commented Sep 14, 2022

my point is that this does not really belong in the plugin but as a more general vault option, as 'unvaulting' can happen at other stages, for example the inventory source file itself could be vaulted, called directly or as one of many in a directory.

@AlanCoding
Copy link
Member Author

I agree, #37019 would be better. I need to verify that it doesn't alter the pass-through behavior of ansible-inventory of vaulted variables, but I don't expect it would.

If that was on the agenda, then I could still use a custom vars plugin as a stopgap.

@nitzmahone nitzmahone removed the needs_triage Needs a first human triage before being processed. label Sep 15, 2022
@nitzmahone nitzmahone self-requested a review September 15, 2022 19:08
@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 26, 2022
@bcoca
Copy link
Member

bcoca commented Apr 5, 2023

is this still relevant?
needs_info

@ansibot ansibot added the needs_info This issue requires further information. Please answer any outstanding questions. label Apr 5, 2023
@AlanCoding
Copy link
Member Author

I'm more than happy to accept this statement

this does not really belong in the plugin but as a more general vault option, as 'unvaulting' can happen at other stages, for example the inventory source file itself could be vaulted, called directly or as one of many in a directory.

And to accept #37019 as a solution to that. Its related issue #13244 was closed due to inactivity. At the time it was raised, another enhancement was going on in parallel, and now we are dealing with problems that can't be solved by that (vault pass-through) enhancement which did make it in. A configurable would be the best solution to the linked AWX problems of vault files throwing errors in inventory imports, although it can also be solved by turning off vars plugins entirely. Either way, this accounts for significant user pain (with the increased reliance on SCM inventory) and a somewhat near-term solution, one or the other, should happen.

But it seems no one likes the solution in this PR.

@ansibot ansibot removed the needs_info This issue requires further information. Please answer any outstanding questions. label Apr 5, 2023
@ansibot ansibot removed the has_issue label Jul 12, 2023
@ansibot ansibot removed 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 Oct 5, 2023
@bcoca
Copy link
Member

bcoca commented Oct 5, 2023

rezzed and updated my code #81918

@ansible ansible deleted a comment from ansibot Oct 5, 2023
@ansibot
Copy link
Contributor

ansibot commented Oct 5, 2023

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

lib/ansible/plugins/vars/host_group_vars.py:0:0: option-incorrect-version-added: version_added for new option (vault_error_behavior) should be '2.17'. Currently StrictVersion ('0.0')

click here for bot help

@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 Oct 19, 2023
@ansibot ansibot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.14 ci_verified Changes made in this PR are causing tests to fail. 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. 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.

4 participants