Allow to pass more sops arguments #47
Conversation
Codecov Report
@@ Coverage Diff @@
## main #47 +/- ##
===========================================
- Coverage 86.45% 62.23% -24.23%
===========================================
Files 6 8 +2
Lines 347 895 +548
Branches 62 196 +134
===========================================
+ Hits 300 557 +257
- Misses 32 260 +228
- Partials 15 78 +63
Continue to review full report at Codecov.
|
|
ready_for_review |
|
|
||
| - name: Test fake sops binary | ||
| set_fact: | ||
| fake_sops_output: "{{ lookup('community.sops.sops', 'simple.sops.yaml', sops_binary=role_path ~ '/files/fake-sops.sh', enable_local_keyservice=True, aws_access_key_id='xxx') }}" |
There was a problem hiding this comment.
is ~ used as string concatenation here?
There was a problem hiding this comment.
Yes, that's Jinja2 standard syntax for string concatenation (https://jinja.palletsprojects.com/en/2.11.x/templates/#other-operators).
| } | ||
|
|
||
|
|
||
| ENCRYPT_OPTIONS = { |
There was a problem hiding this comment.
I was wondering why dividing the options in 2 different sets. Even if they do apply only to encryption phase, would not be simpler to have them available as in the standard sops cli?
There was a problem hiding this comment.
This reduces the number of options the plugins/modules that do not encrypt have (and thus makes their docs less confusing).
| (output, err) = process.communicate() | ||
| exit_code = process.returncode | ||
|
|
||
| if module: |
There was a problem hiding this comment.
May you clarify the usage of module here? I think I got the idea. Is something made available by ActionModuleBase?
There was a problem hiding this comment.
No, it's for classical AnsibleModules. The Ansible coding guidelines say that modules must never use subprocess directly, but use AnsibleModule.run_command. This implements that requirement.
|
I like this addition. It makes everything more complex but I see value in being able to specify options from Ansible code. @felixfontein as a general question, I think the amount of code may deserve some dedicated unit testing, is something advisable or the current test set is considered enough? (as you manage more collections I expect you have a broader overview than me) |
|
@endorama (more) unit tests would definitely be nice. I've tried to cover most code paths with the existing integration tests, but I didn't manage to cover everything. I think the current test coverage is already pretty good. (Of course it can always be increased :) ) |
…ars action plugin.
felixfontein
force-pushed
the
sops-arguments
branch
from
efca1ef
to
28d8a0d
Compare
|
(Rebased against |
Motivation
Fixes #46.
Changes description
Adds a set of options to every module and plugin which allows to pass more options that are passed to the sops executable (including an override of the sops binary path).
Additional notes
The plugin_utils/action_module.py is from https://github.com/ansible-collections/community.crypto/blob/main/plugins/plugin_utils/action_module.py and https://github.com/ansible-collections/community.crypto/blob/main/plugins/module_utils/crypto/module_backends/common.py. I hope that sooner or later this (resp. something similar) is either provided by ansible-base/ansible-core itself, or some common utility collection. For now, I think it's better to vendor the code here instead of making this collection depend on community.crypto just for this piece of utility code.