Skip to content
This repository has been archived by the owner on Jun 2, 2022. It is now read-only.

Support more file name and output what is decrypted #129

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

schollii
Copy link

Per futuresimple#128. I also added a log message because I think it is really useful to see what secrets files are found.

@schollii
Copy link
Author

@szibis @mhyllander any idea if problem with this?

Copy link
Contributor

@kaarolch kaarolch left a 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$ ]]
Copy link
Contributor

@kaarolch kaarolch Jul 8, 2020

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$

Copy link
Author

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"
Copy link
Contributor

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?

Copy link
Author

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.

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

Successfully merging this pull request may close these issues.

None yet

2 participants