Make pass renderer configurable & other fixes#62120
Conversation
dmach
force-pushed
the
make-pass-renderer-configurable
branch
from
7fa9464 to
a569f0b
Compare
meaksh
left a comment
meaksh
left a comment
There was a problem hiding this comment.
Thanks for this PR! The code looks good to me 👍
It would be definetely great that you could also provide some tests here.
dmach
force-pushed
the
make-pass-renderer-configurable
branch
3 times, most recently
from
981fb5a to
9415498
Compare
meaksh
left a comment
meaksh
left a comment
There was a problem hiding this comment.
Awesome! Thanks for the new enhancements and unit tests! 👍
LGTM
(BTW: the failing test does look related to the changes in this PR)
You mean this one? https://jenkins.saltproject.io/job/pr-macosx-catalina-x86_64-py3-pytest/job/PR-62120/6/ |
|
@dmach sorry I meant it does not look related 😅 . Thanks! |
|
@Ch3LL any chance to have a review of this one? Thanks in advance! |
dmach
force-pushed
the
make-pass-renderer-configurable
branch
from
05c1cd8 to
f1ad67c
Compare
|
lint and pre-commit are failing. Once you get that fixed up we can get this one merged in. |
dmach
force-pushed
the
make-pass-renderer-configurable
branch
from
f1ad67c to
cd24609
Compare
Fixed (I hope). Other 2 tests are failing now, but I don't think they're related. |
The original code would fail on pass paths with spaces, because they would be split into multiple arguments.
Just set $HOME for calling the pass binary to avoid affecting anything outside the pass renderer.
1. Allow us to make the pass renderer fail during pillar rendering when a secret corresponding with a pass path cannot be fetched. For this we add a master config variable pass_strict_fetch. 2. Allow to have prefix for variables that should be processed with the pass renderer. For this we add a master config variable pass_variable_prefix. 3. Allow us to configure pass' GNUPGHOME and PASSWORD_STORE_DIR environmental variables. For this we add master config variables pass_gnupghome and pass_dir.
dmach
force-pushed
the
make-pass-renderer-configurable
branch
from
cad2f29 to
605e55c
Compare
|
For some reason I cannot update the branch. Can you rebase and push? We shouldn't be seeing those test issues so lets re-base and start them again. thanks |
Done via GitHub webui. |
|
Congratulations on your first PR being merged! 🎉 |
4 participants
What does this PR do?
The pass renderer becomes configurable.
Also several issues in the code have been fixed.
Previous Behavior
No changes to the existing behavior, new features must be explicitly enabled.
New Behavior
Config option
pass_variable_prefixallows to distinguish variables that contain paths to pass secrets.Config option
pass_strict_fetchallows to error out when a secret cannot be fetched from pass.Config option
pass_dirallows setting thePASSWORD_STORE_DIRenv for pass.Config option
pass_gnupghomeallows setting the$GNUPGHOMEenv for pass.Pass executable path from _get_path_exec() is used when calling the program.
The
$HOMEenv is no longer modified globally.Only trailing newlines are stripped from the fetched secret.
Pass process arguments are handled in a secure way.
Merge requirements satisfied?
[NOTICE] Bug fixes or features added to Salt require tests.
Commits signed with GPG?
No
Please review Salt's Contributing Guide for best practices.
See GitHub's page on GPG signing for more information about signing commits with GPG.