-
Notifications
You must be signed in to change notification settings - Fork 155
Support more file name and output what is decrypted #129
base: master
Are you sure you want to change the base?
Conversation
@szibis @mhyllander any idea if problem with this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@schollii thanks for your contribution! I added two comments.
@@ -438,11 +438,11 @@ EOF | |||
if [[ $yml =~ ^=.*$ ]]; then | |||
yml="${yml/=/}" | |||
fi | |||
if [[ $yml =~ ^(.*/)?secrets(\.[^.]+)*\.yaml$ ]] | |||
if [[ $yml =~ ^(.*/)?secrets([_.-][^.]+)*\.yaml$ ]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only problem with this change is that it allowed us to have something like secrets_-.yaml
. IMHO it should be ^(.*/)?secrets([_.-][^_.-]+)*\.yaml$
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think as long as it's a valid file name and it does not cause problem downstream, you should leave it up to the user to decide. You should not be deciding on naming "style", all you need is a reasonable way of identifying secrets files. I vote to leave the fix as-is.
then | ||
decrypt_helper $yml ymldec decrypted | ||
cmdopts+=("$ymldec") | ||
[[ $decrypted -eq 1 ]] && decfiles+=("$ymldec") | ||
[[ $decrypted -eq 1 ]] && decfiles+=("$ymldec") && echo "DECRYPTED $yml temporarily" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add flag to enable disable this print?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. I imagine this is for backward compat, basically by default you don't want the output? I can add a flag --verbose-decrypt
or --secrets-verbose
or similar. I don't think it is a good idea to use the verbosity setting of helm command itself, because that would turn on way more output than typically desired. Let me know what you think.
Per futuresimple#128. I also added a log message because I think it is really useful to see what secrets files are found.