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

vars plugin: make valid_extensions configurable #186

Merged
merged 2 commits into from
Jun 23, 2024

Conversation

felixfontein
Copy link
Collaborator

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.

Copy link

codecov bot commented Jun 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

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

Additional details and impacted files
@@           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 Coverage Δ
integration 65.06% <100.00%> (ø)
sanity 22.72% <0.00%> (-0.11%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

github-actions bot commented Jun 15, 2024

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
Copy link
Collaborator

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:

  • it works as expected on partially encrypted files
  • it works on non encrypted files

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
Copy link
Collaborator Author

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 getsops/sops#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
Copy link
Collaborator Author

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

@felixfontein
Copy link
Collaborator Author

@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
Copy link
Collaborator

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 getsops/sops#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 felixfontein merged commit c291624 into ansible-collections:main Jun 23, 2024
58 checks passed
@felixfontein felixfontein deleted the ext branch June 23, 2024 15:45
@felixfontein
Copy link
Collaborator Author

@endorama @markuman thanks a lot for reviewing this!

@ZzenlD
Copy link

ZzenlD commented Jul 26, 2024

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
Copy link
Collaborator Author

@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
Copy link

ZzenlD commented Jul 26, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cant specify parameter: _valid_extensions in ansible.cfg
4 participants