Skip to content
This repository was archived by the owner on Nov 23, 2024. It is now read-only.

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.

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.

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.

2 participants