stable/redis-ha: Helm templates in provided secret names. #253

pavel-jancik · 2023-04-24T14:50:24Z

Added support for helm templates in the following entries:

.existingSecret
.sentinel.existingSecret
.haproxy.tls.secretName
.tls.secretName
.restore.existingSecret

We are using this great redis-ha chart as a sub-chart and we provide TLS secret and credentials via secrets (existing). We would like not to hardcode these name in values.yaml (i.e., tweak them for each installation)
but instead use some reasonable default like {{ .Release.Name }}-redis-creds

Replacing:
.Values.existingSecret by tpl (.Values.existingSecret | default "" ) .

Note that | default "" is required because the existingSecret is commented in the chart default values.yaml and thus undefined for helm templating --> without the default values the rendering would result into errors.

Special notes for your reviewer:

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

DCO signed
Chart Version bumped
Variables are documented in the README.md
Title of the PR starts with chart name (e.g. [stable/mychartname])

Added support for helm templates in the following entries: * `.existingSecret` * `.sentinel.existingSecret` * `.haproxy.tls.secretName` * `.tls.secretName` * `.restore.existingSecret` We are using this great `redis-ha` chart as a sub-chart and we provide TLS secret and credentials via secrets (existing). We would like not to hardcode these name in values.yaml (i.e., tweak them for each installation) but instead use some reasonable default like `{{ .Release.Name }}-redis-creds` Replacing: `.Values.existingSecret` by ` tpl (.Values.existingSecret | default "" ) .` Note that `| default ""` is required because the `existingSecret` is commented in the chart default values.yaml and thus undefined for helm templating --> without the default values the rendering would result into errors. Signed-off-by: Pavel Jancik <[email protected]>

Signed-off-by: Pavel Jancik <[email protected]>

pavel-jancik · 2023-04-24T14:51:13Z

Manual tests of the changes in PR

Preparing test values.yaml files

# Test 1 - Used for test that master and PR/branch with default values do not differ
cat <<EOF >values-empty.yaml
EOF

# Test 2 - Used for test that master and PR/branch with provided passwords in values do not differ
cat <<EOF >values-all-generated.yaml
auth: true
redisPassword: redisPassword

redis:
  tlsPort: 6385

sentinel:
  tlsPort: 26385
  auth: true
  password: sentinelPassword

haproxy:
  enabled: true
  tls:
    enabled: true
restore:
  s3:
    source: "s3://xxx"
    secret_key: restoreS3Secret_key
    access_key: restoreS3Access_key
EOF


# Test 3 - Used for test that master and PR/branch with provided non-templated names in values do not differ
cat <<EOF > values-provided-secrets.yaml
auth: true
existingSecret: "PROVIDED-existingSecret"

redis:
  tlsPort: 6385

sentinel:
  tlsPort: 26385
  auth: true
  existingSecret: "PROVIDED-sentinel.existingSecret"

tls:
  secretName: "PROVIDED-tls.secretName"


haproxy:
  enabled: true
  tls:
    enabled: true
    secretName: "PROVIDED-haproxy.tls.secretName"

restore:
  s3:
    source: "s3://xxx"
  existingSecret: "PROVIDED-restore.secretName"
EOF

# Test 4 - Used for PR/branch with provided templated renders correctly and as expected
cat <<EOF > values-provided-secrets-with-templates.yaml
auth: true
existingSecret: "PROVIDED-{{ .Release.Name }}-existingSecret"

redis:
  tlsPort: 6385

sentinel:
  tlsPort: 26385
  auth: true
  existingSecret: "PROVIDED-{{ .Release.Name }}-sentinel.existingSecret"

tls:
  secretName: "PROVIDED-{{ .Release.Name }}-tls.secretName"

haproxy:
  enabled: true
  tls:
    enabled: true
    secretName: "PROVIDED-{{ .Release.Name }}-haproxy.tls.secretName"

restore:
  s3:
    source: "s3://xxx"
  existingSecret: "PROVIDED-{{ .Release.Name }}-restore.secretName"
EOF


# Test 5 - if provided secret names are empty / renders to empty string then it is the same as if the values are not provided
cat <<EOF > values-provided-empty-string.yaml
# Expected the same behavior as if not provided at all
auth: true
existingSecret: '{{ "" }}'
redisPassword: redisPassword

redis:
  tlsPort: 6385

sentinel:
  tlsPort: 26385
  auth: true
  existingSecret: '{{ "" }}'
  password: sentinelPassword

tls:
  secretName: '{{ "" }}'

haproxy:
  enabled: true
  tls:
    enabled: true
    secretName: '{{ "" }}'

restore:
  s3:
    source: "s3://xxx"
    secret_key: restoreS3Secret_key
    access_key: restoreS3Access_key
  existingSecret: '{{ "" }}'
EOF

Output once the above commands are executed:

$ ls -la .
total 0
drwxrwxrwx 1 pjancik pjancik 4096 Apr 24 16:13 .
drwxrwxrwx 1 pjancik pjancik 4096 Apr 24 15:36 ..
drwxrwxrwx 1 pjancik pjancik 4096 Apr 24 15:45 charts
-rwxrwxrwx 1 pjancik pjancik  295 Apr 24 16:13 values-all-generated.yaml
-rwxrwxrwx 1 pjancik pjancik    0 Apr 24 16:13 values-empty.yaml
-rwxrwxrwx 1 pjancik pjancik  494 Apr 24 16:13 values-provided-empty-string.yaml
-rwxrwxrwx 1 pjancik pjancik  505 Apr 24 16:13 values-provided-secrets-with-templates.yaml
-rwxrwxrwx 1 pjancik pjancik  406 Apr 24 16:13 values-provided-secrets.yaml```

Rendering helm chart from master and from PR/branch

# Checkout branch, render using values above and store the resulted templates to the file

set -x \
 && echo "Generating templates from master branch" \
 && pushd charts/ \
 && git checkout master \
 && git status \
 && git log -1 \
 && popd \
 && for VALUE in values-* ; \
    do \
      echo "  processing $VALUE" ; \
      helm template ./charts/charts/redis-ha --values $VALUE >rendered-master-$VALUE.log ; \
    done \
 && echo "Generating templates from redis-templated-names branch" \
 && pushd charts/ \
 && git checkout redis-templated-names \
 && git status \
 && git log -1 \
 && git log -2 \
 && popd \
 && for VALUE in values-* ; \
    do \
      echo "  processing $VALUE" ; \
      helm template ./charts/charts/redis-ha --values $VALUE \
      | tee rendered-PR-$VALUE.log \
      | sed 's#chart: redis-ha-4.23.1#chart: redis-ha-4.23.0#' \
         >rendered-PR-chart-version-downgraded-${VALUE}.log ; \
    done \
 && ls -la . \
 && set +x

The above command will generate following output:

+(:1182): echo 'Generating templates from master branch'
Generating templates from master branch
+(:1183): pushd charts/
/mnt/c/Alf/Mama/git/dandy/charts /mnt/c/Alf/Mama/git/dandy
+(:1184): git checkout master
Switched to branch 'master'
Your branch is up to date with 'origin/master'.
+(:1185): git status
On branch master
Your branch is up to date with 'origin/master'.

nothing to commit, working tree clean
+(:1186): git log -1
commit bb6cb8faa0a990a96b9f43ae8e2d32a882895fc3 (HEAD -> master, origin/master, origin/HEAD)
Author: Muhammed Hussein karimi <[email protected]>
Date:   Wed Mar 8 03:53:39 2023 +0330

    🐛 fix: better kubectl commands in NOTES in redis-ha chart (#244)

    Signed-off-by: Muhammed Hussein Karimi <[email protected]>
+(:1188): popd
/mnt/c/Alf/Mama/git/dandy
+(:1188): for VALUE in values-*
+(:1190): echo '  processing values-all-generated.yaml'
  processing values-all-generated.yaml
+(:1191): helm template ./charts/charts/redis-ha --values values-all-generated.yaml
+(:1188): for VALUE in values-*
+(:1190): echo '  processing values-empty.yaml'
  processing values-empty.yaml
+(:1191): helm template ./charts/charts/redis-ha --values values-empty.yaml
+(:1188): for VALUE in values-*
+(:1190): echo '  processing values-provided-empty-string.yaml'
  processing values-provided-empty-string.yaml
+(:1191): helm template ./charts/charts/redis-ha --values values-provided-empty-string.yaml
Error: YAML parse error on redis-ha/templates/redis-ha-serviceaccount.yaml: error converting YAML to JSON: yaml: invalid map key: map[interface {}]interface {}{"":interface {}(nil)}

Use --debug flag to render out invalid YAML
+(:1188): for VALUE in values-*
+(:1190): echo '  processing values-provided-secrets-with-templates.yaml'
  processing values-provided-secrets-with-templates.yaml
+(:1191): helm template ./charts/charts/redis-ha --values values-provided-secrets-with-templates.yaml
+(:1188): for VALUE in values-*
+(:1190): echo '  processing values-provided-secrets.yaml'
  processing values-provided-secrets.yaml
+(:1191): helm template ./charts/charts/redis-ha --values values-provided-secrets.yaml
+(:1193): echo 'Generating templates from redis-templated-names branch'
Generating templates from redis-templated-names branch
+(:1194): pushd charts/
/mnt/c/Alf/Mama/git/dandy/charts /mnt/c/Alf/Mama/git/dandy
+(:1195): git checkout redis-templated-names
Switched to branch 'redis-templated-names'
Your branch is up to date with 'origin/redis-templated-names'.
+(:1196): git status
On branch redis-templated-names
Your branch is up to date with 'origin/redis-templated-names'.

nothing to commit, working tree clean
+(:1197): git log -1
commit fdad3031eb160d2d6ca0ca37fd253949bbb49d40 (HEAD -> redis-templated-names, origin/redis-templated-names)
Author: Pavel Jancik <[email protected]>
Date:   Mon Apr 24 16:31:25 2023 +0200

    increasing version of the chart

    Signed-off-by: Pavel Jancik <[email protected]>
+(:1198): git log -2
commit fdad3031eb160d2d6ca0ca37fd253949bbb49d40 (HEAD -> redis-templated-names, origin/redis-templated-names)
Author: Pavel Jancik <[email protected]>
Date:   Mon Apr 24 16:31:25 2023 +0200

    increasing version of the chart

    Signed-off-by: Pavel Jancik <[email protected]>

commit ab018820cb4465a8335e780eabb787c8ad745504
Author: Pavel Jancik <[email protected]>
Date:   Mon Apr 24 15:36:06 2023 +0200

    feat: Helm templates in provided secret names.

    Added support for helm templates in the following entries:
    * `.existingSecret`
    * `.sentinel.existingSecret`
    * `.haproxy.tls.secretName`
    * `.tls.secretName`
    * `.restore.existingSecret`

    We are using this great `redis-ha` chart as a sub-chart and we provide TLS secret and credentials via secrets (existing).
    We would like not to hardcode these name in values.yaml (i.e., tweak them for each installation)
      but instead use some reasonable default like `{{ .Release.Name }}-redis-creds`

    Replacing:
    `.Values.existingSecret`  by ` tpl (.Values.existingSecret | default "" ) .`

    Note that  `| default ""` is required because the `existingSecret` is commented in the chart default values.yaml and thus undefined for helm templating --> without the default values the rendering would result into errors.

    Signed-off-by: Pavel Jancik <[email protected]>
+(:1200): popd
/mnt/c/Alf/Mama/git/dandy
+(:1200): for VALUE in values-*
+(:1202): echo '  processing values-all-generated.yaml'
  processing values-all-generated.yaml
+(:1204): tee rendered-PR-values-all-generated.yaml.log
+(:1205): sed 's#chart: redis-ha-4.23.1#chart: redis-ha-4.23.0#'
+(:1203): helm template ./charts/charts/redis-ha --values values-all-generated.yaml
+(:1200): for VALUE in values-*
+(:1202): echo '  processing values-empty.yaml'
  processing values-empty.yaml
+(:1204): tee rendered-PR-values-empty.yaml.log
+(:1205): sed 's#chart: redis-ha-4.23.1#chart: redis-ha-4.23.0#'
+(:1203): helm template ./charts/charts/redis-ha --values values-empty.yaml
+(:1200): for VALUE in values-*
+(:1202): echo '  processing values-provided-empty-string.yaml'
  processing values-provided-empty-string.yaml
+(:1204): tee rendered-PR-values-provided-empty-string.yaml.log
+(:1205): sed 's#chart: redis-ha-4.23.1#chart: redis-ha-4.23.0#'
+(:1203): helm template ./charts/charts/redis-ha --values values-provided-empty-string.yaml
+(:1200): for VALUE in values-*
+(:1202): echo '  processing values-provided-secrets-with-templates.yaml'
  processing values-provided-secrets-with-templates.yaml
+(:1204): tee rendered-PR-values-provided-secrets-with-templates.yaml.log
+(:1205): sed 's#chart: redis-ha-4.23.1#chart: redis-ha-4.23.0#'
+(:1203): helm template ./charts/charts/redis-ha --values values-provided-secrets-with-templates.yaml
+(:1200): for VALUE in values-*
+(:1202): echo '  processing values-provided-secrets.yaml'
  processing values-provided-secrets.yaml
+(:1204): tee rendered-PR-values-provided-secrets.yaml.log
+(:1205): sed 's#chart: redis-ha-4.23.1#chart: redis-ha-4.23.0#'
+(:1203): helm template ./charts/charts/redis-ha --values values-provided-secrets.yaml
+(:1208): ls --color=auto -la .
total 736
drwxrwxrwx 1 pjancik pjancik  4096 Apr 24 16:45 .
drwxrwxrwx 1 pjancik pjancik  4096 Apr 24 15:36 ..
drwxrwxrwx 1 pjancik pjancik  4096 Apr 24 15:45 charts
-rwxrwxrwx 1 pjancik pjancik 55441 Apr 24 16:45 rendered-PR-chart-version-downgraded-values-all-generated.yaml.log
-rwxrwxrwx 1 pjancik pjancik 40548 Apr 24 16:45 rendered-PR-chart-version-downgraded-values-empty.yaml.log
-rwxrwxrwx 1 pjancik pjancik 55441 Apr 24 16:45 rendered-PR-chart-version-downgraded-values-provided-empty-string.yaml.log
-rwxrwxrwx 1 pjancik pjancik 54383 Apr 24 16:45 rendered-PR-chart-version-downgraded-values-provided-secrets-with-templates.yaml.log
-rwxrwxrwx 1 pjancik pjancik 54201 Apr 24 16:45 rendered-PR-chart-version-downgraded-values-provided-secrets.yaml.log
-rwxrwxrwx 1 pjancik pjancik 55441 Apr 24 16:45 rendered-PR-values-all-generated.yaml.log
-rwxrwxrwx 1 pjancik pjancik 40548 Apr 24 16:45 rendered-PR-values-empty.yaml.log
-rwxrwxrwx 1 pjancik pjancik 55441 Apr 24 16:45 rendered-PR-values-provided-empty-string.yaml.log
-rwxrwxrwx 1 pjancik pjancik 54383 Apr 24 16:45 rendered-PR-values-provided-secrets-with-templates.yaml.log
-rwxrwxrwx 1 pjancik pjancik 54201 Apr 24 16:45 rendered-PR-values-provided-secrets.yaml.log
-rwxrwxrwx 1 pjancik pjancik 55441 Apr 24 16:45 rendered-master-values-all-generated.yaml.log
-rwxrwxrwx 1 pjancik pjancik 40548 Apr 24 16:45 rendered-master-values-empty.yaml.log
-rwxrwxrwx 1 pjancik pjancik     0 Apr 24 16:45 rendered-master-values-provided-empty-string.yaml.log
-rwxrwxrwx 1 pjancik pjancik 54407 Apr 24 16:45 rendered-master-values-provided-secrets-with-templates.yaml.log
-rwxrwxrwx 1 pjancik pjancik 54127 Apr 24 16:45 rendered-master-values-provided-secrets.yaml.log
-rwxrwxrwx 1 pjancik pjancik   295 Apr 24 16:13 values-all-generated.yaml
-rwxrwxrwx 1 pjancik pjancik     0 Apr 24 16:13 values-empty.yaml
-rwxrwxrwx 1 pjancik pjancik   494 Apr 24 16:13 values-provided-empty-string.yaml
-rwxrwxrwx 1 pjancik pjancik   505 Apr 24 16:13 values-provided-secrets-with-templates.yaml
-rwxrwxrwx 1 pjancik pjancik   406 Apr 24 16:13 values-provided-secrets.yaml
+(:1209): set +x

Note that from master you are not render correctly the chart with values-provided-empty-string.yaml;
this is ok, the version from PR renders the values correctly.

Test results

Test 1 - default values do not differ

set -x \
  && cat values-empty.yaml \
  && diff -Naur rendered-master-values-empty.yaml.log rendered-PR-chart-version-downgraded-values-empty.yaml.log \
  && set +x

# Output of the above command
+(:1211): cat values-empty.yaml
+(:1212): diff -Naur rendered-master-values-empty.yaml.log rendered-PR-chart-version-downgraded-values-empty.yaml.log
+(:1213): set +x

This gets empty diff output as expected.

Test 2 - passwords provided in values cause no diff

set -x \
  && cat values-all-generated.yaml \
  && diff -Naur rendered-master-values-all-generated.yaml.log rendered-PR-chart-version-downgraded-values-all-generated.yaml.log \
  && set +x

# Output of the above command
+(:1215): cat values-all-generated.yaml
auth: true
redisPassword: redisPassword

redis:
  tlsPort: 6385

sentinel:
  tlsPort: 26385
  auth: true
  password: sentinelPassword

haproxy:
  enabled: true
  tls:
    enabled: true
restore:
  s3:
    source: "s3://xxx"
    secret_key: restoreS3Secret_key
    access_key: restoreS3Access_key
+(:1216): diff -Naur rendered-master-values-all-generated.yaml.log rendered-PR-chart-version-downgraded-values-all-generated.yaml.log
+(:1217): set +x

This gets empty diff output as expected.

Test 3 - passwords provided in values cause no diff

set -x \
  && cat values-provided-secrets.yaml \
  && diff -Naur rendered-master-values-provided-secrets.yaml.log rendered-PR-chart-version-downgraded-values-provided-secrets.yaml.log \
  && set +x

# Output of the above command
+(:1222): cat values-provided-secrets.yaml
auth: true
existingSecret: "PROVIDED-existingSecret"

redis:
  tlsPort: 6385

sentinel:
  tlsPort: 26385
  auth: true
  existingSecret: "PROVIDED-sentinel.existingSecret"

tls:
  secretName: "PROVIDED-tls.secretName"


haproxy:
  enabled: true
  tls:
    enabled: true
    secretName: "PROVIDED-haproxy.tls.secretName"

restore:
  s3:
    source: "s3://xxx"
  existingSecret: "PROVIDED-restore.secretName"
+(:1223): diff -Naur rendered-master-values-provided-secrets.yaml.log rendered-PR-chart-version-downgraded-values-provided-secrets.yaml.log
--- rendered-master-values-provided-secrets.yaml.log    2023-04-24 16:45:39.413705600 +0200
+++ rendered-PR-chart-version-downgraded-values-provided-secrets.yaml.log       2023-04-24 16:45:54.844201700 +0200
@@ -1322,7 +1322,7 @@
            && mv -v /data/dump.rdb_ /data/dump.rdb"
         envFrom:
         - secretRef:
-            name: PROVIDED-existingSecret
+            name: PROVIDED-existingSecret # This one is suspicious, one would expect restore.existingSecret as well
         volumeMounts:
         - name: data
           mountPath: /data

This gets only one line diff with the comment of possible issue in the master chart version.

Test 4 - templates in secret names

Here we expect large diff compared to master, since master will not render the values.
But all changes are {{ .Release.Name }} in master is replaced by release-name in the PR.

set -x \
  && cat values-provided-secrets-with-templates.yaml \
  && diff -Naur rendered-master-values-provided-secrets-with-templates.yaml.log rendered-PR-chart-version-downgraded-values-provided-secrets-with-templates.yaml.log \
  && set +x


# Output of the above command
+(:1230): cat values-provided-secrets-with-templates.yaml
auth: true
existingSecret: "PROVIDED-{{ .Release.Name }}-existingSecret"

redis:
  tlsPort: 6385

sentinel:
  tlsPort: 26385
  auth: true
  existingSecret: "PROVIDED-{{ .Release.Name }}-sentinel.existingSecret"

tls:
  secretName: "PROVIDED-{{ .Release.Name }}-tls.secretName"

haproxy:
  enabled: true
  tls:
    enabled: true
    secretName: "PROVIDED-{{ .Release.Name }}-haproxy.tls.secretName"

restore:
  s3:
    source: "s3://xxx"
  existingSecret: "PROVIDED-{{ .Release.Name }}-restore.secretName"
+(:1231): diff -Naur rendered-master-values-provided-secrets-with-templates.yaml.log rendered-PR-chart-version-downgraded-values-provided-secrets-with-templates.yaml.log
--- rendered-master-values-provided-secrets-with-templates.yaml.log     2023-04-24 16:45:36.909947600 +0200
+++ rendered-PR-chart-version-downgraded-values-provided-secrets-with-templates.yaml.log        2023-04-24 16:45:52.332887200 +0200
@@ -11,8 +11,8 @@
     chart: redis-ha-4.23.0
     app: release-name-redis-ha
 secrets:
-- name: PROVIDED-{{ .Release.Name }}-existingSecret
-- name: PROVIDED-{{ .Release.Name }}-sentinel.existingSecret
+- name: PROVIDED-release-name-existingSecret
+- name: PROVIDED-release-name-sentinel.existingSecret
 ---
 # Source: redis-ha/templates/redis-haproxy-serviceaccount.yaml
 apiVersion: v1
@@ -1164,12 +1164,12 @@
         - name: AUTH
           valueFrom:
             secretKeyRef:
-              name: PROVIDED-{{ .Release.Name }}-existingSecret
+              name: PROVIDED-release-name-existingSecret
               key: auth
         - name: SENTINELAUTH
           valueFrom:
             secretKeyRef:
-              name: PROVIDED-{{ .Release.Name }}-sentinel.existingSecret
+              name: PROVIDED-release-name-sentinel.existingSecret
               key: sentinel-password
         livenessProbe:
           httpGet:
@@ -1200,7 +1200,7 @@
       volumes:
       - name: pemfile
         secret:
-          secretName: PROVIDED-{{ .Release.Name }}-haproxy.tls.secretName
+          secretName: PROVIDED-release-name-haproxy.tls.secretName
       - name: config-volume
         configMap:
           name: release-name-redis-ha-configmap
@@ -1289,12 +1289,12 @@
         - name: AUTH
           valueFrom:
             secretKeyRef:
-              name: PROVIDED-{{ .Release.Name }}-existingSecret
+              name: PROVIDED-release-name-existingSecret
               key: auth
         - name: SENTINELAUTH
           valueFrom:
             secretKeyRef:
-              name: PROVIDED-{{ .Release.Name }}-sentinel.existingSecret
+              name: PROVIDED-release-name-sentinel.existingSecret
               key: sentinel-password
         volumeMounts:
         - name: config
@@ -1322,7 +1322,7 @@
            && mv -v /data/dump.rdb_ /data/dump.rdb"
         envFrom:
         - secretRef:
-            name: PROVIDED-{{ .Release.Name }}-existingSecret
+            name: PROVIDED-release-name-existingSecret # This one is suspicious, one would expect restore.existingSecret as well
         volumeMounts:
         - name: data
           mountPath: /data
@@ -1348,7 +1348,7 @@
         - name: AUTH
           valueFrom:
             secretKeyRef:
-              name: PROVIDED-{{ .Release.Name }}-existingSecret
+              name: PROVIDED-release-name-existingSecret
               key: auth
         livenessProbe:
           initialDelaySeconds: 30
@@ -1415,12 +1415,12 @@
         - name: AUTH
           valueFrom:
             secretKeyRef:
-              name: PROVIDED-{{ .Release.Name }}-existingSecret
+              name: PROVIDED-release-name-existingSecret
               key: auth
         - name: SENTINELAUTH
           valueFrom:
             secretKeyRef:
-              name: PROVIDED-{{ .Release.Name }}-sentinel.existingSecret
+              name: PROVIDED-release-name-sentinel.existingSecret
               key: sentinel-password
         livenessProbe:
           initialDelaySeconds: 30
@@ -1487,12 +1487,12 @@
         - name: AUTH
           valueFrom:
             secretKeyRef:
-              name: PROVIDED-{{ .Release.Name }}-existingSecret
+              name: PROVIDED-release-name-existingSecret
               key: auth
         - name: SENTINELAUTH
           valueFrom:
             secretKeyRef:
-              name: PROVIDED-{{ .Release.Name }}-sentinel.existingSecret
+              name: PROVIDED-release-name-sentinel.existingSecret
               key: sentinel-password
         resources:
           {}
@@ -1510,7 +1510,7 @@
           name: release-name-redis-ha-configmap
       - name: tls-certs
         secret:
-          secretName: PROVIDED-{{ .Release.Name }}-tls.secretName
+          secretName: PROVIDED-release-name-tls.secretName
       - name: health
         configMap:
           name: release-name-redis-ha-health-configmap

Test 5 - templated secret name renders to empty string

If provided secret names are empty / renders to empty string
then it is the same as if the values are not provided.
Comparing with master from test 2.

set -x \
  && cat values-provided-empty-string.yaml \
  && diff -Naur rendered-master-values-all-generated.yaml.log rendered-PR-chart-version-downgraded-values-provided-empty-string.yaml.log \
  && set +x

# Output of the above command
+(:1237): cat values-provided-empty-string.yaml
# Expected the same behavior as if not provided at all
auth: true
existingSecret: '{{ "" }}'
redisPassword: redisPassword

redis:
  tlsPort: 6385

sentinel:
  tlsPort: 26385
  auth: true
  existingSecret: '{{ "" }}'
  password: sentinelPassword

tls:
  secretName: '{{ "" }}'

haproxy:
  enabled: true
  tls:
    enabled: true
    secretName: '{{ "" }}'

restore:
  s3:
    source: "s3://xxx"
    secret_key: restoreS3Secret_key
    access_key: restoreS3Access_key
  existingSecret: '{{ "" }}'
+(:1238): diff -Naur rendered-master-values-all-generated.yaml.log rendered-PR-chart-version-downgraded-values-provided-empty-string.yaml.log
+(:1239): set +x

The diff is empty, i.e., if provided secret names renders to empty string, then it is the same as if they are not specified at all.

pavel-jancik · 2023-04-24T14:53:03Z

FYI: Maintainers @DandyDeveloper, @ssalaues

DandyDeveloper

Sorry for the delay!

@pavel-jancik Just one thing: the tpl in the conditional seems excessive.

As an example, the conditional should just look at whether a value is present or not (AKA: "" or "<SOMETHING>"

We're probably good to remove the tpls from the conditionals.

Thoughts?

DandyDeveloper · 2024-04-06T13:04:31Z

charts/redis-ha/templates/redis-ha-statefulset.yaml

@@ -467,8 +467,8 @@ spec:
          - name: REDIS_PASSWORD
            valueFrom:
              secretKeyRef:
-              {{- if .Values.existingSecret }}
-                name: {{ .Values.existingSecret }}
+              {{- if tpl (.Values.existingSecret | default "" ) . }}


Is the tpl replace in the conditional necessary?

Well, the idea is that if the template resolves to the empty string, then it should be equivalent to the case, as if the existing secret is not provided.

Assume, e.g., the case when you have redis as subchart:

global: secret: passwordsSecret: "" # Specify the user provided secret with password, if empty the chars will generate a random password redis: existingSecret: '{{ .Values.global.secret.passwordsSecret }}'

In this case you have non empty (redis).existingSecret value with template, which however resolves to empty string.
I believe in these cases you want to behave exactly as if the existingSecret is empty; that is why we should have tpl even in the conditions.

This behavior is tested in Test 5 (at the bottom of) , in mine comment above.

pavel-jancik added 2 commits April 24, 2023 16:30

increasing version of the chart

fdad303

Signed-off-by: Pavel Jancik <[email protected]>

DandyDeveloper reviewed Apr 6, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

stable/redis-ha: Helm templates in provided secret names. #253

stable/redis-ha: Helm templates in provided secret names. #253

pavel-jancik commented Apr 24, 2023

pavel-jancik commented Apr 24, 2023

pavel-jancik commented Apr 24, 2023

DandyDeveloper left a comment

DandyDeveloper Apr 6, 2024

pavel-jancik Apr 10, 2024

stable/redis-ha: Helm templates in provided secret names. #253

Are you sure you want to change the base?

stable/redis-ha: Helm templates in provided secret names. #253

Conversation

pavel-jancik commented Apr 24, 2023

Special notes for your reviewer:

Checklist

pavel-jancik commented Apr 24, 2023

Manual tests of the changes in PR

Preparing test values.yaml files

Rendering helm chart from master and from PR/branch

Test results

Test 1 - default values do not differ

Test 2 - passwords provided in values cause no diff

Test 3 - passwords provided in values cause no diff

Test 4 - templates in secret names

Test 5 - templated secret name renders to empty string

pavel-jancik commented Apr 24, 2023

DandyDeveloper left a comment

Choose a reason for hiding this comment

DandyDeveloper Apr 6, 2024

Choose a reason for hiding this comment

pavel-jancik Apr 10, 2024

Choose a reason for hiding this comment