helm: keep the S3 config checksum stable across the first upgrade (fixes #11719) (#11720)

* helm: keep the S3 config checksum stable across the first upgrade (fixes #11719)

checksum/s3config hashed the rendered s3-secret.yaml. That template
keeps its generated S3 keys with lookup, so rendering it again for the
checksum drew new keys whenever lookup found nothing: on install the
filer, S3 and all-in-one pods each carried a checksum of a different
set of keys than the Secret got, and the first upgrade with unchanged
values corrected it and restarted them.

Hash the values s3-secret.yaml is built from instead: the enable and
auth switches, existingConfigSecret, credentials, the legacy-secret
settings, the audit log configs and the Secret name. Only those, so
scaling the S3 gateway or changing its resources does not restart the
filer. A change to any of them still restarts the pods. Keys that the
chart generated stay out of the hash; they only change if the Secret
is deleted by hand.

The new render test fails on the old template (two checksums for one
Secret, different on every render) and passes now; it also checks that
the checksum follows the credentials and filer.s3.auditLogConfig and
ignores s3.replicas and s3.resources. On kind, an upgrade with unchanged
values no longer replaces the filer pod.

* helm: check the S3 checksum per workload, all-in-one included

Each workload's checksum/s3config is now read on its own (--show-only),
so a workload that loses the annotation fails the test instead of being
hidden by sort -u, and the all-in-one deployment gets the same checks.
Verified against two broken copies: one without the filer annotation,
one with the old all-in-one checksum; the test fails on both.

The helper comment now says what happens when the Secret is deleted to
rotate generated keys.

* ci(helm): fail the S3 checksum test on a missing or unrendered checksum

A failed helm template or a render without the annotation produced an
empty checksum, and the inequality assertions then passed against it.
s3_checksum now returns non-zero unless it found a checksum, and the
changed-value renders are assigned before comparing so failures cannot
hide inside test.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

---------

Co-authored-by: Ege Zambelli <egezamb@users.noreply.github.com>
Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com>
Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

.github/workflows/helm_ci.yml

@@ -95,6 +95,41 @@ jobs:
           grep -A 5 "^          lifecycle:" /tmp/s3-rollout.yaml | grep -q -- "- sleep"
           echo "S3 rollout settings render correctly"
 
+          echo "=== Testing the S3 config checksum is stable across renders ==="
+          # s3-secret.yaml keeps its generated keys with lookup. Hashing the rendered
+          # Secret drew new keys for the checksum whenever lookup found nothing (on
+          # install), so the first upgrade changed the checksum and restarted the
+          # filer and S3 pods although nothing had changed.
+          s3_checksum() {  # <template> <helm args...>: the checksum/s3config of one workload
+            local template=$1; shift
+            local sum
+            sum=$(helm template test $CHART_DIR --show-only "$template" "$@" \
+              | grep 'checksum/s3config:' | awk '{print $2}')
+            test -n "$sum" || { echo "no checksum/s3config in $template" >&2; return 1; }
+            echo "$sum"
+          }
+          s3_auth_args=(--set s3.enabled=true --set s3.enableAuth=true)
+          s3_sum=$(s3_checksum templates/s3/s3-deployment.yaml "${s3_auth_args[@]}")
+          filer_sum=$(s3_checksum templates/filer/filer-statefulset.yaml "${s3_auth_args[@]}")
+          test "$s3_sum" = "$filer_sum"   # one Secret, one checksum
+          test "$s3_sum" = "$(s3_checksum templates/s3/s3-deployment.yaml "${s3_auth_args[@]}")"
+          s3_creds_sum=$(s3_checksum templates/s3/s3-deployment.yaml "${s3_auth_args[@]}" \
+            --set s3.credentials.admin.accessKey=a --set s3.credentials.admin.secretKey=b)
+          test "$s3_sum" != "$s3_creds_sum"
+          filer_audit_sum=$(s3_checksum templates/filer/filer-statefulset.yaml "${s3_auth_args[@]}" \
+            --set-json 'filer.s3.auditLogConfig={"fluent_port":24224}')
+          test "$filer_sum" != "$filer_audit_sum"
+          # Settings the Secret does not read must not restart the S3 and filer pods.
+          test "$filer_sum" = "$(s3_checksum templates/filer/filer-statefulset.yaml "${s3_auth_args[@]}" \
+            --set s3.replicas=3 --set s3.resources.limits.cpu=1)"
+          all_in_one_args=(--set allInOne.enabled=true --set allInOne.s3.enabled=true --set allInOne.s3.enableAuth=true)
+          aio_sum=$(s3_checksum templates/all-in-one/all-in-one-deployment.yaml "${all_in_one_args[@]}")
+          test "$aio_sum" = "$(s3_checksum templates/all-in-one/all-in-one-deployment.yaml "${all_in_one_args[@]}")"
+          aio_creds_sum=$(s3_checksum templates/all-in-one/all-in-one-deployment.yaml "${all_in_one_args[@]}" \
+            --set s3.credentials.admin.accessKey=a --set s3.credentials.admin.secretKey=b)
+          test "$aio_sum" != "$aio_creds_sum"
+          echo "S3 config checksum is stable and follows only what the S3 Secret reads"
+
           echo "=== Testing S3 credentials from an existing secret ==="
           credential_args=(
             --set s3.credentials.admin.existingSecret=minio-root

k8s/charts/seaweedfs/templates/all-in-one/all-in-one-deployment.yaml

@@ -43,7 +43,7 @@ spec:
       {{- $configSecret := (lookup "v1" "Secret" .Release.Namespace $existingS3ConfigSecret) | default dict }}
       {{- $_ := set $podAnnotations "checksum/s3config" ($configSecret | toYaml | sha256sum) }}
       {{- else }}
-      {{- $_ := set $podAnnotations "checksum/s3config" (include (print .Template.BasePath "/s3/s3-secret.yaml") . | sha256sum) }}
+      {{- $_ := set $podAnnotations "checksum/s3config" (include "seaweedfs.s3.configChecksum" .) }}
       {{- end }}
       {{- $_ := set $podAnnotations "checksum/master-config" (include (print .Template.BasePath "/master/master-configmap.yaml") . | sha256sum) }}
       annotations:

k8s/charts/seaweedfs/templates/filer/filer-statefulset.yaml

@@ -48,7 +48,7 @@ spec:
       {{- $configSecret := (lookup "v1" "Secret" .Release.Namespace .Values.filer.s3.existingConfigSecret) | default dict }}
       {{- $_ := set $podAnnotations "checksum/s3config" ($configSecret | toYaml | sha256sum) }}
       {{- else }}
-      {{- $_ := set $podAnnotations "checksum/s3config" (include (print .Template.BasePath "/s3/s3-secret.yaml") . | sha256sum) }}
+      {{- $_ := set $podAnnotations "checksum/s3config" (include "seaweedfs.s3.configChecksum" .) }}
       {{- end }}
       annotations:
         {{- toYaml $podAnnotations | nindent 8 }}

k8s/charts/seaweedfs/templates/s3/s3-deployment.yaml

@@ -44,7 +44,7 @@ spec:
       {{- $configSecret := (lookup "v1" "Secret" .Release.Namespace .Values.s3.existingConfigSecret) | default dict }}
       {{- $_ := set $podAnnotations "checksum/s3config" ($configSecret | toYaml | sha256sum) }}
       {{- else }}
-      {{- $_ := set $podAnnotations "checksum/s3config" (include (print .Template.BasePath "/s3/s3-secret.yaml") . | sha256sum) }}
+      {{- $_ := set $podAnnotations "checksum/s3config" (include "seaweedfs.s3.configChecksum" .) }}
       {{- end }}
       annotations:
         {{- toYaml $podAnnotations | nindent 8 }}

k8s/charts/seaweedfs/templates/shared/_helpers.tpl

@@ -692,3 +692,28 @@ Usage: {{ include "seaweedfs.service.loadBalancerFields" .Values.s3.service }}
 {{- end }}
 {{- end }}
 {{- end -}}
+
+{{/*
+Checksum that restarts the S3-serving pods when the generated S3 config changes. It hashes the
+values s3-secret.yaml is built from, not the rendered Secret: that template keeps its generated keys
+with lookup, so rendering it again for a checksum draws new random keys whenever lookup finds
+nothing (on install), and the first upgrade then changed the checksum and restarted the pods
+although nothing had changed. Only the values that template reads are hashed, so scaling the S3
+gateway or changing its resources does not restart the filer. Keep this list in step with
+s3-secret.yaml.
+Keys the chart generated are not part of it. Deleting the Secret to rotate them regenerates them on
+the next upgrade without restarting the pods; restart them afterwards (kubectl rollout restart).
+Folding the stored keys in would bring the first-upgrade restart back, since lookup finds nothing
+on install.
+*/}}
+{{- define "seaweedfs.s3.configChecksum" -}}
+{{- dict
+  "secretName" (include "seaweedfs.fullname" .)
+  "enabled" (list .Values.s3.enabled .Values.filer.s3.enabled .Values.allInOne.enabled .Values.allInOne.s3.enabled)
+  "enableAuth" (list .Values.s3.enableAuth .Values.filer.s3.enableAuth .Values.allInOne.s3.enableAuth)
+  "existingConfigSecret" (list .Values.s3.existingConfigSecret .Values.filer.s3.existingConfigSecret .Values.allInOne.s3.existingConfigSecret)
+  "credentials" .Values.s3.credentials
+  "legacySecret" (list .Values.s3.reuseLegacySecret .Values.s3.legacySecretName)
+  "auditLogConfig" (list .Values.s3.auditLogConfig .Values.filer.s3.auditLogConfig)
+  | toJson | sha256sum -}}
+{{- end -}}