wekan / charts

Wekan Helm Charts to deploy to Kubernetes
MIT License
3 stars 15 forks source link

Helm Chart - secretEnv broken #17

Closed Nightreaver closed 1 year ago

Nightreaver commented 1 year ago

Hello, I found an issue with the secretEnv today after upgrading to latest version. @xet7

https://github.com/wekan/charts/commit/5dfb6b717bf76847ee9b7199507fd4508b11d7d2#diff-297fc75fd4942cbe9e1736dd8b8f814dffe2f59687bebfe92ac1dcf8dc5a4b92L8

The change itself seems fine, but I guess the indentation was lost in the process.

before it was:

data:
  {{- range $key := .Values.secretEnv }}

now it is;

data:
{{ $key.name }}: {{ $key.value | b64enc }}

if I place values now into secretEnv, they will be added to the secret like:

data:
OAUTH2_SECRET: 

with missing indentation and therefore breaking the secret

Helm upgrade failed: error validating "": error validating data: ValidationError(Secret): unknown field "OAUTH2_SECRET" in io.k8s.api.core.v1.Secret

I think i have to open a PR to fix it

xet7 commented 1 year ago

Fixed at already released Helm Chart 1.2.7:

xet7 commented 1 year ago

ArtifactHub page will update usually within 30mins after release, so it will be visible in near future.

Nightreaver commented 1 year ago

Looks good, works for me :)

szechp commented 1 year ago

it would be cool to extend this to include existing secrets as env vars. or is there another way to do this?

xet7 commented 1 year ago

@szechp

I don't know does someone understand what to change, why, where, and send PR this way: https://github.com/wekan/wekan/wiki/Emoji

Nightreaver commented 1 year ago

it would be cool to extend this to include existing secrets as env vars. or is there another way to do this?

what exactly is your idea?

szechp commented 1 year ago

i have figured it now, i now it already does secret retrieval the way i described it. a problem i have with this right now is this hybrid approach, where if you give a name and a key it generates said secret. but if you leave it blank and the secret already exists it gets somehow stitched together. i mean it works, its just not very intuitive. take a look at how its done in grafana: see here: https://github.com/grafana/helm-charts/blob/main/charts/grafana/values.yaml#L452

they have a separate one for getting secrets as env vars:

## The name of a secret in the same kubernetes namespace which contain values to be added to the environment
## This can be useful for auth tokens, etc. Value is templated.
envFromSecret: ""

and also for multiple secrets:

## The names of secrets in the same kubernetes namespace which contain values to be added to the environment
## Each entry should contain a name key, and can optionally specify whether the secret must be defined with an optional key.
## Name is templated.
envFromSecrets: []
## - name: secret-name
##   optional: true

and one for generating env vars as secrets:

## Sensible environment variables that will be rendered as new secret object
## This can be useful for auth tokens, etc.
## If the secret values contains "{{", they'll need to be properly escaped so that they are not interpreted by Helm
## ref: https://helm.sh/docs/howto/charts_tips_and_tricks/#using-the-tpl-function
envRenderSecret: {}

it would be nice to have this here as well.