jkroepke / helm-secrets

A helm plugin that help manage secrets with Git workflow and store them anywhere
https://github.com/jkroepke/helm-secrets/wiki
Apache License 2.0
1.53k stars 129 forks source link

Helm Secrets does not parse array values passed through --set properly #418

Closed ABWassim closed 1 year ago

ABWassim commented 1 year ago

Current Behavior

Hello :)

In my CI, I need to pass a array value using the --set option of helm secrets template. I do it using the command helm secrets template --set locales={fr,de,en,ko,zh} .. I later use this value and iterate over it in my resources, which we could simplify to this Helm code :

...
{{ range .Values.locales }}
{{ . }}
{{ end }}
...

I would expect Helm to output something like this :

fr
de
en
ko
zh

But instead, what i get from helm secrets template --set locales={fr,de,en,ko,zh} . is :

fr
=de=
=en=
=ko=
zh

When using the --debug option, I saw that the final Helm command was : helm template . --set 'locales={fr,de=de,en=en,ko=ko,zh}=zh}'

After looking at the code, I think it comes from this file, line 87 :

opt_prefix="${literal%%=*}="

When iterating over literals (line 72), literal will be equal to de in the second iteration. (second value of the array) opt_prefix will then be equal to de= when it should be equal to empty string from my understanding.

It doesn't happen first iteration because literal will be equal to locales={fr, and here we can actually split by =.

I would be happy to open a PR, but I would need your validation first on what I said above :)

Thank you, Wassim.

Expected Behavior

I would expect helm secrets template to parse the value correctly as described above.

Steps To Reproduce

Create a minimal Helm Chart with one template containing
{{ range .Values.locales }}
{{ . }}
{{ end }}

Then pass a array to helm secrets template using --set locales={fr,de,en,ko,zh}

Environment

Anything else?

--debug logs :

+ SCRIPT_DIR=/Users/wbelkacem/Library/helm/plugins/helm-secrets/scripts
+ . /Users/wbelkacem/Library/helm/plugins/helm-secrets/scripts/lib/common.sh
++ set -euf
++ case "$(uname -s)" in
+++ uname -s
++ case $(sed --help 2>&1) in
+++ sed --help
+ . /Users/wbelkacem/Library/helm/plugins/helm-secrets/scripts/lib/expand_vars_strict.sh
++ set -euf
+ . /Users/wbelkacem/Library/helm/plugins/helm-secrets/scripts/lib/file.sh
++ set -euf
++ VALUES_ALLOW_SYMLINKS=true
++ VALUES_ALLOW_ABSOLUTE_PATH=true
++ VALUES_ALLOW_PATH_TRAVERSAL=true
++ . /Users/wbelkacem/Library/helm/plugins/helm-secrets/scripts/lib/file/local.sh
+++ set -euf
++ . /Users/wbelkacem/Library/helm/plugins/helm-secrets/scripts/lib/file/http.sh
+++ set -euf
+++ URL_VARIABLE_EXPANSION=false
++ . /Users/wbelkacem/Library/helm/plugins/helm-secrets/scripts/lib/file/custom.sh
+++ set -euf
+ . /Users/wbelkacem/Library/helm/plugins/helm-secrets/scripts/lib/backend.sh
++ set -euf
++ ALLOWED_BACKENDS=
++ . /Users/wbelkacem/Library/helm/plugins/helm-secrets/scripts/lib/backends/noop.sh
++ . /Users/wbelkacem/Library/helm/plugins/helm-secrets/scripts/lib/backends/sops.sh
+++ _SOPS=sops
++ . /Users/wbelkacem/Library/helm/plugins/helm-secrets/scripts/lib/backends/vals.sh
+++ set -euf
+++ _VALS=vals
+ . /Users/wbelkacem/Library/helm/plugins/helm-secrets/scripts/lib/http.sh
++ set -euf
+ HELM_BIN=helm
+ on_cygwin
+ false
++ mktemp -d
+ TMPDIR=/var/folders/6d/4r0kl8pd3tq26w49lm4v8hw40000gp/T/tmp.VdcsGPE8
+ export TMPDIR
+ mkdir -p /var/folders/6d/4r0kl8pd3tq26w49lm4v8hw40000gp/T/tmp.VdcsGPE8
+ '[' -n '' ']'
+ QUIET=false
+ SECRET_BACKEND=sops
+ SECRET_BACKEND_ARGS=
+ DEC_PREFIX=
+ DEC_SUFFIX=.dec
+ DEC_DIR=
+ IGNORE_MISSING_VALUES=false
+ EVALUATE_TEMPLATES=false
+ EVALUATE_TEMPLATES_DECODE_SECRETS=false
+ DECRYPT_SECRETS_IN_TMP_DIR=false
+ LOAD_GPG_KEYS=false
+ trap _trap EXIT
+ trap 'trap - EXIT; _trap; exit 1' HUP INT QUIT TERM
+ load_secret_backend sops
+ backend=sops
+ '[' sops = '' ']'
+ '[' '' '!=' '' ']'
+ '[' -f /Users/wbelkacem/Library/helm/plugins/helm-secrets/scripts/lib/backends/sops.sh ']'
+ SECRET_BACKEND=sops
+ return
+ DEFAULT_SECRET_BACKEND=sops
+ '[' false '!=' false ']'
+ '[' -n '' ']'
+ true
+ case "${1:-}" in
+ . /Users/wbelkacem/Library/helm/plugins/helm-secrets/scripts/commands/helm.sh
++ set -euf
++ . /Users/wbelkacem/Library/helm/plugins/helm-secrets/scripts/commands/decrypt.sh
+++ set -euf
+++ _mktemp
+++ '[' 0 -eq 0 ']'
+++ mktemp /var/folders/6d/4r0kl8pd3tq26w49lm4v8hw40000gp/T/tmp.VdcsGPE8/XXXXXX
++ decrypted_file_list=/var/folders/6d/4r0kl8pd3tq26w49lm4v8hw40000gp/T/tmp.VdcsGPE8/xR8qdt
+ helm_command template . --set 'locales={fr,de,en,ko,zh}' out.txt
+ '[' 5 -lt 2 ']'
+ is_help .
+ case "$1" in
+ false
+ helm_wrapper template . --set 'locales={fr,de,en,ko,zh}' out.txt
+ argc=5
+ j=0
+ '[' 0 -lt 5 ']'
+ case "$1" in
+ '[' -d template ']'
+ '[' -f template ']'
+ set -- template . --set 'locales={fr,de,en,ko,zh}' out.txt template
+ shift
+ j=1
+ '[' 1 -lt 5 ']'
+ case "$1" in
+ '[' -d . ']'
++ _helm_winpath .
++ printf %s .
+ set -- . --set 'locales={fr,de,en,ko,zh}' out.txt template .
+ shift
+ j=2
+ '[' 2 -lt 5 ']'
+ case "$1" in
+ _1=--set
+ case "${_1}" in
+ literals='locales={fr,de,en,ko,zh}'
+ set -- --set 'locales={fr,de,en,ko,zh}' out.txt template . --set
+ shift
+ j=3
+ decrypted_literals=
+ IFS=,
+ _literal=
+ for literal in '${literals}'
+ unset IFS
+ case "${literal}" in
+ '[' '' '!=' '' ']'
+ opt_prefix=locales=
+ literal='{fr'
+ '[' '{fr' '!=' '{fr' ']'
+ load_secret_backend sops
+ backend=sops
+ '[' sops = '' ']'
+ '[' '' '!=' '' ']'
+ '[' -f /Users/wbelkacem/Library/helm/plugins/helm-secrets/scripts/lib/backends/sops.sh ']'
+ SECRET_BACKEND=sops
+ return
++ backend_decrypt_literal '{fr'
++ _sops_backend_decrypt_literal '{fr'
++ printf %s '{fr'
++ _sops_backend_is_encrypted
++ grep -q 'mac.*,type:str]' -
++ printf %s '{fr'
+ decrypted_literal='{fr'
+ '[' '{fr' = '{fr' ']'
+ decrypted_literals='locales={fr,'
+ for literal in '${literals}'
+ unset IFS
+ case "${literal}" in
+ '[' '' '!=' '' ']'
+ opt_prefix=de=
+ literal=de
+ '[' de '!=' de ']'
+ load_secret_backend sops
+ backend=sops
+ '[' sops = '' ']'
+ '[' '' '!=' '' ']'
+ '[' -f /Users/wbelkacem/Library/helm/plugins/helm-secrets/scripts/lib/backends/sops.sh ']'
+ SECRET_BACKEND=sops
+ return
++ backend_decrypt_literal de
++ _sops_backend_decrypt_literal de
++ printf %s de
++ _sops_backend_is_encrypted
++ grep -q 'mac.*,type:str]' -
++ printf %s de
+ decrypted_literal=de
+ '[' de = de ']'
+ decrypted_literals='locales={fr,de=de,'
+ for literal in '${literals}'
+ unset IFS
+ case "${literal}" in
+ '[' '' '!=' '' ']'
+ opt_prefix=en=
+ literal=en
+ '[' en '!=' en ']'
+ load_secret_backend sops
+ backend=sops
+ '[' sops = '' ']'
+ '[' '' '!=' '' ']'
+ '[' -f /Users/wbelkacem/Library/helm/plugins/helm-secrets/scripts/lib/backends/sops.sh ']'
+ SECRET_BACKEND=sops
+ return
++ backend_decrypt_literal en
++ _sops_backend_decrypt_literal en
++ printf %s en
++ _sops_backend_is_encrypted
++ grep -q 'mac.*,type:str]' -
++ printf %s en
+ decrypted_literal=en
+ '[' en = en ']'
+ decrypted_literals='locales={fr,de=de,en=en,'
+ for literal in '${literals}'
+ unset IFS
+ case "${literal}" in
+ '[' '' '!=' '' ']'
+ opt_prefix=ko=
+ literal=ko
+ '[' ko '!=' ko ']'
+ load_secret_backend sops
+ backend=sops
+ '[' sops = '' ']'
+ '[' '' '!=' '' ']'
+ '[' -f /Users/wbelkacem/Library/helm/plugins/helm-secrets/scripts/lib/backends/sops.sh ']'
+ SECRET_BACKEND=sops
+ return
++ backend_decrypt_literal ko
++ _sops_backend_decrypt_literal ko
++ printf %s ko
++ _sops_backend_is_encrypted
++ grep -q 'mac.*,type:str]' -
++ printf %s ko
+ decrypted_literal=ko
+ '[' ko = ko ']'
+ decrypted_literals='locales={fr,de=de,en=en,ko=ko,'
+ for literal in '${literals}'
+ unset IFS
+ case "${literal}" in
+ '[' '' '!=' '' ']'
+ opt_prefix='zh}='
+ literal='zh}'
+ '[' 'zh}' '!=' 'zh}' ']'
+ load_secret_backend sops
+ backend=sops
+ '[' sops = '' ']'
+ '[' '' '!=' '' ']'
+ '[' -f /Users/wbelkacem/Library/helm/plugins/helm-secrets/scripts/lib/backends/sops.sh ']'
+ SECRET_BACKEND=sops
+ return
++ backend_decrypt_literal 'zh}'
++ _sops_backend_decrypt_literal 'zh}'
++ printf %s 'zh}'
++ _sops_backend_is_encrypted
++ grep -q 'mac.*,type:str]' -
++ printf %s 'zh}'
+ decrypted_literal='zh}'
+ '[' 'zh}' = 'zh}' ']'
+ decrypted_literals='locales={fr,de=de,en=en,ko=ko,zh}=zh},'
+ set -- 'locales={fr,de,en,ko,zh}' out.txt template . --set 'locales={fr,de=de,en=en,ko=ko,zh}=zh}'
+ shift
+ j=4
+ '[' 4 -lt 5 ']'
+ case "$1" in
+ '[' -d out.txt ']'
+ '[' -f out.txt ']'
++ _helm_winpath out.txt
++ printf %s out.txt
+ set -- out.txt template . --set 'locales={fr,de=de,en=en,ko=ko,zh}=zh}' out.txt
+ shift
+ j=5
+ '[' 5 -lt 5 ']'
+ '[' false = true ']'
+ helm template . --set 'locales={fr,de=de,en=en,ko=ko,zh}=zh}' out.txt
install.go:214: [debug] Original chart version: ""
install.go:231: [debug] CHART PATH: /Users/wbelkacem/Documents/Repositories/test/out.txt

Error: file '/Users/wbelkacem/Documents/Repositories/test/out.txt' does not appear to be a gzipped archive; got 'application/octet-stream'
helm.go:84: [debug] file '/Users/wbelkacem/Documents/Repositories/test/out.txt' does not appear to be a gzipped archive; got 'application/octet-stream'
+ _trap
+ command -v _trap_hook
+ _trap_hook
+ '[' -s /var/folders/6d/4r0kl8pd3tq26w49lm4v8hw40000gp/T/tmp.VdcsGPE8/xR8qdt ']'
+ '[' -n '' ']'
+ rm -rf /var/folders/6d/4r0kl8pd3tq26w49lm4v8hw40000gp/T/tmp.VdcsGPE8
Error: plugin "secrets" exited with error
jkroepke commented 1 year ago

You are right. helm-secrets is always splitting on ,, except on \,. I was not aware of this syntax.

The problem also exists with

helm secrets template --set-json='locales=["fr","de","en","ko","zh"]' .

I would be happy to open a PR, but I would need your validation first on what I said above :)

I would happy about an PR it would helps a lot. Please include some test that guaranteed that some edge cases are cover.

Here test-cases like

helm template --set 'locales={fr,d\,e,en,ko,zh},a=b' .
helm template --set 'locales={fr,de,\}3\,2,ko,zh}' .
helm template --set 'locales={fr,d\,e,en,ko,zh},a=b,c=Hello{Wo\,a' .
helm template --set-json='locales=["fr","de","en","ko","zh"],a={"Hello": "world"}' . # optional

Also please net me know, if its too hard. I mean this is silly POSIX shell and it can be quite complex.

ABWassim commented 1 year ago

Hey again :) I made this PR with a couple of comments for you :)