ansible-collections / community.sops

Simple and flexible tool for managing secrets
https://galaxy.ansible.com/ui/repo/published/community/sops/
GNU General Public License v3.0
76 stars 22 forks source link

vars plugin: make valid_extensions configurable #186

Closed felixfontein closed 2 months ago

felixfontein commented 3 months ago

Motivation

See #183.

Changes description

Makes the valid extensions configurable by the valid_extensions option in the [community.sops] section of ansible.cfg, or by the ANSIBLE_VARS_SOPS_PLUGIN_VALID_EXTENSIONS environment variable.

Additional notes

Fixes #183.

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 66.16%. Comparing base (038577d) to head (81a6484). Report is 8 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #186 +/- ## ======================================= Coverage 66.16% 66.16% ======================================= Files 12 12 Lines 990 990 Branches 231 231 ======================================= Hits 655 655 Misses 255 255 Partials 80 80 ``` | [Flag](https://app.codecov.io/gh/ansible-collections/community.sops/pull/186/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ansible-collections) | Coverage Ξ” | | |---|---|---| | [integration](https://app.codecov.io/gh/ansible-collections/community.sops/pull/186/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ansible-collections) | `65.06% <100.00%> (ΓΈ)` | | | [sanity](https://app.codecov.io/gh/ansible-collections/community.sops/pull/186/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ansible-collections) | `22.72% <0.00%> (-0.11%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ansible-collections#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

github-actions[bot] commented 3 months ago

Docs Build πŸ“

Thank you for contribution!✨

This PR has been merged and the docs are now incorporated into main: https://ansible-collections.github.io/community.sops/branch/main

endorama commented 3 months ago

I agree with this change but there is a caveat we need to double check.

As this uses the generic vars file extension is it now possible for sops to run on non sops encrypted files. I remember choosing a separate extension on purpose, due to limitations on running sops on such files. I don't exactly remember if the issue was just with non sops encrypted files or with partially encrypted files too.

To ensure this functionality works as expected I think we need to add a couple of test to ensure that:

This would address being able to use just yaml/json extensions without unexpected failures.

What do you think? I may have a look during the next week.

felixfontein commented 3 months ago

As long as SOPS can decrypt the files, it will work.

It will definitely fail with non-encrypted files (sops will exit with sops metadata not found). (There's already a test for that, using the original extensions: tests/integration/targets/vars_sops/test-bad-file/.)

There's currently no way to test whether a file is sops-encrypted (I'm still waiting for https://github.com/getsops/sops/pull/545 to get rebased ;) ). It's probably best to add a warning that the plugin will not work if it applies to any file that's not sops-encrypted.

(Once that PR is merged and a new SOPS release is out I'd like to add some code that tests files before trying to decrypt them. There are quite a few things I'm planning to do once a new SOPS release is out. I really hope it happens soon...)

felixfontein commented 3 months ago

I improved the documentation accordingly (and while at it improved/removed some other things).

felixfontein commented 3 months ago

@endorama is the current version of the PR ok enough for you? If yes I'll merge it and create a new release so it can get included in tomorrow's Ansible releases.

endorama commented 2 months ago

As long as SOPS can decrypt the files, it will work. It will definitely fail with non-encrypted files

@felixfontein It make sense and I think it's a good change anyway. It opens the door for shooting yourself in the foot but is also a useful feature for more advanced use cases and users.

I'm still waiting for https://github.com/getsops/sops/pull/545 to get rebased ;)

πŸ™ Found some time to do it finally, it's now up to date.

(Once that PR is merged and a new SOPS release is out I'd like to add some code that tests files before trying to decrypt them. There are quite a few things I'm planning to do once a new SOPS release is out. I really hope it happens soon...)

Looking forward to it!

felixfontein commented 2 months ago

@endorama @markuman thanks a lot for reviewing this!

ZzenlD commented 1 month ago

Is it now possible with the new filestatus in SOPS to encrypt only some of the inventory-files and the sops_vars plugin can automatically recognize if a "sops:"-part is present in the file or not? If not, it will skip the file without throwing an error.

This would allow a seamless integration into ansible.

felixfontein commented 1 month ago

@ZzenlD check out the handle_unencrypted_files option (https://docs.ansible.com/ansible/devel/collections/community/sops/sops_vars.html#parameter-handle_unencrypted_files). You can set it to skip.

ZzenlD commented 1 month ago

Oh, my mistake... I really should read better the documentation. Thanks for the implementation :)