helm: drop fromToml dependency in security-configmap.yaml (fixes #11611) (#11614)

* helm: drop fromToml dependency in security-configmap.yaml (fixes #11611)

fromToml was added to Helm in v3.17.0 (helm/helm#12026, merged
2024-09-12, one day after v3.16.0 was cut). This chart declares no
minimum Helm version (no Chart.yaml kubeVersion, nothing in the
README), and the call in security-configmap.yaml:21 is an
unconditional *parse*-time failure on Helm < v3.17.0 - Go's
text/template parses a file's entire body before evaluating any
{{if}}, so this breaks the chart (any topology, any values) even when
securityConfigEnabled is false and the ConfigMap would render nothing.

Replaces the fromToml-based dig lookup with a small regex-based helper
(seaweedfs.existingTomlKey) that reads the same four "key = ..." JWT
signing-key values out of a previously-rendered security.toml,
preserving the existing fallback-to-random behavior exactly.

Verified:
- helm lint (v3.16.3 and v4.3.0): clean
- helm template with chart defaults: byte-identical output to the
  unpatched chart rendered via Helm v4 (which has fromToml) - the
  disabled/default path is untouched
- helm template with security enabled, no prior ConfigMap: identical
  structure to the unpatched chart (helm v4), modulo the expected
  random key
- Real helm install + helm upgrade round trip (live lookup, since
  "helm template" never evaluates lookup, even under the original
  fromToml code): the JWT signing key is identical across both
  releases - confirms key persistence across upgrades is preserved,
  not just "renders without erroring"
- helm template with chart defaults, Helm v3.16.3: previously failed
  with a parse error naming fromToml as undefined; now renders
  successfully

Fixes #11611.

* helm: harden existingTomlKey against commented key lines and CRLF

Addresses two review findings from greptile-apps on PR #11614:

- The key-line regex matched the first "key = ..." anywhere in the
  section block, including a commented-out "# key = ..." line, which
  would shadow a real active key on a hand-edited or otherwise
  non-chart-generated ConfigMap. Anchored to line start with the
  Go regexp multiline flag ((?m)^key...), which a line starting with
  "#" cannot match.
- The section-header match required an exact "]\n", so a ConfigMap
  with CRLF line endings would fail to match the block at all and
  regenerate the key instead of reusing it. Changed to "]\r?\n".

Also adds a CI test ("Verify JWT signing key persistence across
upgrades") exercising all of this end to end with a real
helm install -> edit the live ConfigMap -> helm upgrade cycle,
matching the existing "Verify SFTP host key secret lifecycle" test's
shape: both edge cases are reproduced against a real ConfigMap and
asserted on the post-upgrade rendered security.toml.

Verified locally (same commands as the new CI step) against a real
cluster before pushing.

* helm: preserve JWT keys across supported TOML layouts

* ci: use setup-python interpreter for JWT upgrade checks

* helm: preserve keys under quoted TOML section headers

* helm: ignore unrelated quoted TOML section headers

.github/scripts/helm_jwt_keys.py

@@ -0,0 +1,240 @@
+#!/usr/bin/env python3
+"""Check JWT extraction and, with --context, real Helm upgrade persistence."""
+
+import argparse
+import base64
+import json
+from pathlib import Path
+import shutil
+import subprocess
+import sys
+import tempfile
+import uuid
+
+try:
+    import tomllib
+except ModuleNotFoundError:  # CI also exercises Python 3.10.
+    import tomli as tomllib
+
+
+ROOT = Path(__file__).resolve().parents[2]
+CHART = ROOT / "k8s/charts/seaweedfs"
+SECTIONS = ("jwt.signing", "jwt.signing.read", "jwt.filer_signing", "jwt.filer_signing.read")
+KEYS = {section: f"active-{index}" for index, section in enumerate(SECTIONS)}
+ESCAPED_HEADER = '["jw\\u0074".signing]\nkey = "existing"'
+
+
+def run(*args):
+    return subprocess.run(args, check=True, text=True, capture_output=True).stdout
+
+
+def keys(raw):
+    document = tomllib.loads(raw)
+    result = {}
+    for section in SECTIONS:
+        table = document
+        for part in section.split("."):
+            table = table.get(part, {})
+        if "key" in table:
+            result[section] = table["key"]
+    return result
+
+
+def fixtures():
+    canonical = "\n".join(f'[{section}]\nkey = "{value}"' for section, value in KEYS.items())
+    yield "canonical / final line without newline", canonical
+    yield "commented stale keys", canonical.replace("key =", '# key = "stale"\nkey =')
+    yield "commented headers / mixed line endings", "\n".join(
+        f'# [{section}]\r\n# key = "stale"\r\n[{section}]\nkey = "{value}"'
+        for section, value in KEYS.items()
+    )
+    yield "indented headers and keys / trailing comments", "\n".join(
+        f' \t[ {section} ] \t# header [note]\n \tkey \t= \t"{value}" # key comment'
+        for section, value in KEYS.items()
+    )
+    yield "CRLF", canonical.replace("\n", "\r\n")
+    yield "quoted and spaced section names", "\n".join(
+        f'["{section.split(".")[0]}" . \'{section.split(".")[1]}\''
+        + (f' . "{section.split(".")[2]}"' if section.count(".") == 2 else "")
+        + f'] # original table\nkey = "{value}"'
+        for section, value in KEYS.items()
+    )
+    yield "unrelated quoted header before JWT keys", '["custom section"]\nkey = "other"\n' + canonical
+    yield "brackets in comments", canonical.replace("key =", "# consult [notes]\nkey =")
+    yield "quoted values and quoted key names", "\n".join((
+        '[jwt.signing]\n"key" = "brackets[inside]#value"',
+        "[jwt.signing.read]\n'key' = 'literal\\path[#value]'",
+        r'[jwt.filer_signing]' + '\n' + r'key = "escaped\"quote\\slash\u0041"',
+        '[jwt.filer_signing.read]\nkey = ""',
+    ))
+    yield "absent keys and sections / unrelated key", '\n'.join((
+        '[jwt.signing]\nexpires_after_seconds = 10',
+        '[jwt.signing.read]\nkey = "read-only"',
+        '[unrelated]\nkey = "not-a-jwt-key"',
+        '# [jwt.filer_signing]\n# key = "not-active"',
+    ))
+    yield "no existing security.toml", ""
+
+
+def check_generated(value):
+    decoded = base64.b64decode(value, validate=True).decode("ascii")
+    assert len(decoded) == 10 and decoded.isascii() and decoded.isalnum(), "invalid generated JWT key"
+
+
+def check_helpers(helm, reference_helm):
+    # Exercise the real helper, not a second implementation of its matching rules.
+    with tempfile.TemporaryDirectory(prefix="helm-jwt-helper-") as directory:
+        chart = Path(directory)
+        (chart / "templates").mkdir()
+        (chart / "Chart.yaml").write_text("apiVersion: v2\nname: jwt-regression\nversion: 0.0.0\n")
+        shutil.copyfile(CHART / "templates/shared/_helpers.tpl", chart / "templates/_helpers.tpl")
+        entries = [
+            json.dumps(section) + ': {{ include "seaweedfs.existingTomlKey" (list '
+            + json.dumps(section) + ' .Values.raw) | toJson }}'
+            for section in SECTIONS
+        ]
+        template = chart / "templates/keys.yaml"
+        prefix = '{"apiVersion":"v1","kind":"ConfigMap","metadata":{"name":"keys"},"data":{'
+        helper_template = prefix + ",".join(entries) + "}}"
+        reference_entries = [
+            json.dumps(section) + ': {{ dig '
+            + " ".join(json.dumps(part) for part in section.split("."))
+            + ' "key" "__ABSENT__" (fromToml .Values.raw) | toJson }}'
+            for section in SECTIONS
+        ]
+        reference_template = prefix + ",".join(reference_entries) + "}}"
+
+        def render(binary, source, raw):
+            template.write_text(source)
+            values = chart / "input.json"
+            values.write_text(json.dumps({"raw": raw}))
+            output = run(binary, "template", "keys", str(chart), "-f", str(values))
+            return json.loads(output[output.index("{"):])["data"]
+
+        for name, raw in fixtures():
+            expected = keys(raw)
+            tokens = render(helm, helper_template, raw)
+            actual = {section: tomllib.loads("key = " + token)["key"]
+                      for section, token in tokens.items() if token != ""}
+            assert actual == expected, f"{name}: extracted keys differ from stored TOML"
+            if reference_helm:
+                reference = render(reference_helm, reference_template, raw)
+                reference = {section: value for section, value in reference.items() if value != "__ABSENT__"}
+                assert actual == reference, f"{name}: keys differ from fromToml/dig"
+            print(f"PASS helper: {name}")
+
+        # A present but unsupported value must not silently become a fresh key.
+        try:
+            render(helm, helper_template, '[jwt.signing]\nkey = """multi\nline"""')
+        except subprocess.CalledProcessError as error:
+            assert "refusing to replace an existing key" in error.stderr, error.stderr
+        else:
+            raise AssertionError("multiline existing key was silently accepted or replaced")
+        print("PASS helper: unsupported existing value fails without rotation")
+
+        try:
+            render(helm, helper_template, ESCAPED_HEADER)
+        except subprocess.CalledProcessError as error:
+            assert "unsupported quoted section header" in error.stderr, error.stderr
+        else:
+            raise AssertionError("unsupported quoted header silently rotated its key")
+        print("PASS helper: unsupported quoted header fails without rotation")
+
+
+def check_upgrades(helm, context):
+    namespace = "jwt-key-persist-" + uuid.uuid4().hex[:8]
+    current = "jk-seaweedfs-security-config"
+    legacy = "seaweedfs-security-config"
+    kubectl = ["kubectl", "--context", context, "-n", namespace]
+    release_args = ["jk", str(CHART), "--kube-context", context, "-n", namespace]
+    # No workload is needed to exercise Helm's real ConfigMap lookup and update.
+    for setting in (
+        "master.enabled=false", "volume.enabled=false", "filer.enabled=false",
+        "global.seaweedfs.createClusterRole=false",
+        "global.seaweedfs.securityConfig.jwtSigning.volumeWrite=true",
+        "global.seaweedfs.securityConfig.jwtSigning.volumeRead=true",
+        "global.seaweedfs.securityConfig.jwtSigning.filerWrite=true",
+        "global.seaweedfs.securityConfig.jwtSigning.filerRead=true",
+    ):
+        release_args += ["--set", setting]
+
+    def stored():
+        cm = json.loads(run(*kubectl, "get", "configmap", current, "-o", "json"))
+        return keys(cm["data"]["security.toml"])
+
+    def upgrade():
+        run(helm, "upgrade", *release_args)
+        return stored()
+
+    def patch(raw):
+        # Seed previous-release content without taking Helm 4's SSA ownership.
+        run(*kubectl, "patch", "configmap", current, "--type=merge", "--field-manager=helm", "-p",
+            json.dumps({"data": {"security.toml": raw}}))
+
+    run(*kubectl, "create", "namespace", namespace)
+    try:
+        run(helm, "install", *release_args)
+        initial = stored()
+        assert set(initial) == set(SECTIONS), "install omitted a JWT section"
+        for value in initial.values():
+            check_generated(value)
+        assert upgrade() == initial, "no-op upgrade changed an existing key"
+        print("PASS upgrade: all four generated keys persist")
+
+        for name, raw in fixtures():
+            patch(raw)
+            actual = upgrade()
+            expected = keys(raw)
+            assert set(actual) == set(SECTIONS), f"{name}: upgrade omitted a JWT section"
+            for section in SECTIONS:
+                if section in expected:
+                    assert actual[section] == expected[section], f"{name}: changed {section}"
+                else:
+                    check_generated(actual[section])
+            assert upgrade() == actual, f"{name}: subsequent upgrade changed a key"
+            print(f"PASS upgrade: {name}")
+
+        # Migration from the old chart name, followed by precedence of the current name.
+        legacy_raw = "\n".join(f'[{section}]\nkey = "legacy-{index}"'
+                               for index, section in enumerate(SECTIONS))
+        run(*kubectl, "create", "configmap", legacy, "--from-literal=security.toml=" + legacy_raw)
+        run(*kubectl, "delete", "configmap", current)
+        assert upgrade() == keys(legacy_raw), "legacy ConfigMap keys were not preserved"
+        print("PASS upgrade: legacy ConfigMap migration")
+        current_raw = next(fixtures())[1]
+        patch(current_raw)
+        assert upgrade() == keys(current_raw), "legacy ConfigMap overrode current ConfigMap"
+        print("PASS upgrade: current ConfigMap takes precedence")
+
+        patch(ESCAPED_HEADER)
+        try:
+            upgrade()
+        except subprocess.CalledProcessError as error:
+            assert "unsupported quoted section header" in error.stderr, error.stderr
+        else:
+            raise AssertionError("unsupported quoted header silently rotated its key")
+        assert keys(run(*kubectl, "get", "configmap", current, "-o",
+                        "jsonpath={.data.security\\.toml}"))["jwt.signing"] == "existing"
+        print("PASS upgrade: unsupported quoted header leaves stored key untouched")
+    finally:
+        run(*kubectl, "delete", "namespace", namespace, "--wait=false")
+
+
+def main():
+    parser = argparse.ArgumentParser(description=__doc__)
+    parser.add_argument("--helm", default="helm")
+    parser.add_argument("--reference-helm", help="Helm >=3.17 binary for differential checks")
+    parser.add_argument("--context", help="explicit disposable Kubernetes context for live upgrade checks")
+    args = parser.parse_args()
+    print(run(args.helm, "version", "--short").strip())
+    check_helpers(args.helm, args.reference_helm)
+    if args.context:
+        check_upgrades(args.helm, args.context)
+
+
+if __name__ == "__main__":
+    try:
+        main()
+    except subprocess.CalledProcessError as error:
+        print(error.stderr, file=sys.stderr)
+        sys.exit(error.returncode)

.github/workflows/helm_ci.yml

@@ -3,10 +3,10 @@ name: "helm: lint and test charts"
 on:
   push:
     branches: [ master ]
-    paths: ['k8s/**', '.github/workflows/helm_ci.yml']
+    paths: ['k8s/**', '.github/workflows/helm_ci.yml', '.github/scripts/helm_jwt_keys.py']
   pull_request:
     branches: [ master ]
-    paths: ['k8s/**', '.github/workflows/helm_ci.yml']
+    paths: ['k8s/**', '.github/workflows/helm_ci.yml', '.github/scripts/helm_jwt_keys.py']
 
 permissions:
   contents: read
@@ -20,6 +20,14 @@ jobs:
         with:
           fetch-depth: 0
 
+      - name: Set up Helm before fromToml was available
+        uses: azure/setup-helm@v5
+        with:
+          version: v3.16.3
+
+      - name: Record legacy Helm binary
+        run: echo "HELM_LEGACY=$(command -v helm)" >> "$GITHUB_ENV"
+
       - name: Set up Helm
         uses: azure/setup-helm@v5
         with:
@@ -44,6 +52,17 @@ jobs:
       - name: Run chart-testing (lint)
         run: ct lint --target-branch ${{ github.event.repository.default_branch }} --all --validate-maintainers=false --chart-dirs k8s/charts
 
+      - name: Verify legacy Helm rendering
+        run: |
+          "$HELM_LEGACY" lint k8s/charts/seaweedfs
+          "$HELM_LEGACY" template test k8s/charts/seaweedfs > "$RUNNER_TEMP/legacy-default.yaml"
+          "$HELM_LEGACY" template test k8s/charts/seaweedfs \
+            --set global.seaweedfs.securityConfig.jwtSigning.volumeRead=true \
+            --set global.seaweedfs.securityConfig.jwtSigning.filerWrite=true \
+            --set global.seaweedfs.securityConfig.jwtSigning.filerRead=true \
+            > "$RUNNER_TEMP/legacy-jwt.yaml"
+
+
       - name: Verify template rendering
         run: |
           set -e
@@ -1922,6 +1941,16 @@ jobs:
           kubectl delete namespace "$NS"
           echo "SFTP host key lifecycle tests passed"
 
+      - name: Verify JWT signing key persistence across upgrades
+        run: |
+          # chart-testing puts its pip-less venv first on PATH; use setup-python.
+          PYTHON="$pythonLocation/bin/python3"
+          "$PYTHON" -m pip install tomli==2.2.1
+          CONTEXT=$(kubectl config current-context)
+          "$PYTHON" .github/scripts/helm_jwt_keys.py --context "$CONTEXT"
+          "$PYTHON" .github/scripts/helm_jwt_keys.py --helm "$HELM_LEGACY" \
+            --reference-helm helm --context "$CONTEXT"
+
       - name: Verify install into a default-deny namespace
         run: |
           set -e

k8s/charts/seaweedfs/README.md

@@ -27,6 +27,19 @@ so your deployment will be spread/HA.
 * cert config exists and can be enabled, but not been tested, requires cert-manager to be installed.
 
 ## Prerequisites
+
+The chart's templates render with Helm 3.16.3 and newer versions tested in CI.
+Earlier Helm versions are not covered by this chart's compatibility checks.
+When JWT signing is enabled, a live upgrade reuses keys from the existing
+security ConfigMap (including the legacy ConfigMap name); a key absent from
+that ConfigMap is generated. Plain `helm template` does not read cluster state;
+use an install and upgrade against a cluster to check key persistence.
+
+The key reader supports single-line quoted TOML strings in JWT sections,
+including indented assignments, quoted key names, and bare or simply quoted
+section-name segments. An unsupported value or quoted header fails the upgrade
+rather than silently rotating a key.
+
 ### Database
 
 leveldb is the default database, this supports multiple filer replicas that will [sync automatically](https://github.com/seaweedfs/seaweedfs/wiki/Filer-Store-Replication), with some [limitations](https://github.com/seaweedfs/seaweedfs/wiki/Filer-Store-Replication#limitation).

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

@@ -444,6 +444,46 @@ true
 {{- end -}}
 {{- end -}}
 
+{{/* Read a JWT key from the chart's existing security.toml without fromToml
+     (which requires Helm >=3.17). Args: (list "<section>" $raw).
+     Return the single-line TOML string token, including its quotes, so escapes
+     and explicitly empty values survive unchanged. No output means absent.
+     Accept bare or simply quoted dotted header segments. Fail on other quoted
+     headers rather than risk rotating a key hidden by unsupported syntax.
+     This reads the chart's section/key layout, not arbitrary TOML syntax. */}}
+{{- define "seaweedfs.existingTomlKey" -}}
+{{- $section := index . 0 -}}
+{{- $raw := index . 1 -}}
+{{- $parts := list -}}
+{{- range $part := splitList "." $section -}}
+  {{- $escaped := regexQuoteMeta $part -}}
+  {{- $parts = append $parts (printf `(?:%s|"%s"|'%s')` $escaped $escaped $escaped) -}}
+{{- end -}}
+{{- $header := printf `^\[[ \t]*%s[ \t]*\][ \t]*(#.*)?$` (join `[ \t]*\.[ \t]*` $parts) -}}
+{{- $segment := `(?:[A-Za-z0-9_-]+|"[^"\\]*"|'[^']*')` -}}
+{{- $simpleHeader := printf `^\[[ \t]*%s(?:[ \t]*\.[ \t]*%s)*[ \t]*\][ \t]*(#.*)?$` $segment $segment -}}
+{{- $assignment := `^(key|"key"|'key')[ \t]*=[ \t]*` -}}
+{{- $string := `"([^"\\]|\\.)*"|'[^']*'` -}}
+{{- $active := false -}}
+{{- $key := "" -}}
+{{- range $rawLine := splitList "\n" $raw -}}
+  {{- $line := trim $rawLine -}}
+  {{- if hasPrefix "[" $line -}}
+    {{- if and (regexMatch `^\[[^]]*["']` $line) (not (regexMatch $simpleHeader $line)) (eq $key "") -}}
+      {{- fail (printf "security.toml has an unsupported quoted section header; refusing to replace [%s].key" $section) -}}
+    {{- end -}}
+    {{- $active = regexMatch $header $line -}}
+  {{- else if and $active (eq $key "") (regexMatch $assignment $line) -}}
+    {{- $value := regexReplaceAll $assignment $line "" -}}
+    {{- if not (regexMatch (printf `^(%s)[ \t]*(#.*)?$` $string) $value) -}}
+      {{- fail (printf "security.toml [%s].key must be a single-line quoted TOML string; refusing to replace an existing key" $section) -}}
+    {{- end -}}
+    {{- $key = regexFind $string $value -}}
+  {{- end -}}
+{{- end -}}
+{{- $key -}}
+{{- end -}}
+
 {{/* True when the post-install bucket hook Job renders: an S3 endpoint, plus
      buckets to create on it. Read by the Job itself and by its NetworkPolicy,
      which has to appear exactly when the Job does - a Job without its policy

k8s/charts/seaweedfs/templates/shared/security-configmap.yaml

@@ -18,7 +18,7 @@ data:
   {{- $legacyName := printf "%s-%s" (include "seaweedfs.name" .) "security-config" }}
   {{- $existing = lookup "v1" "ConfigMap" .Release.Namespace $legacyName }}
   {{- end }}
-  {{- $securityConfig := fromToml (dig "data" "security.toml" "" $existing) }}
+  {{- $existingToml := dig "data" "security.toml" "" $existing }}
   {{- $securityConfigValues := .Values.global.seaweedfs.securityConfig | default dict }}
   {{- $jwtSigning := $securityConfigValues.jwtSigning | default dict }}
   {{- $expiresAfterSeconds := $jwtSigning.expiresAfterSeconds | default dict }}
@@ -29,7 +29,7 @@ data:
     # the jwt signing key is read by master and volume server
     # the jwt defaults to expire after 10 seconds
     [jwt.signing]
-    key = "{{ dig "jwt" "signing" "key" (randAlphaNum 10 | b64enc) $securityConfig }}"
+    key = {{ include "seaweedfs.existingTomlKey" (list "jwt.signing" $existingToml) | default (randAlphaNum 10 | b64enc | quote) }}
     {{- if gt (int $expiresAfterSeconds.volumeWrite) 0 }}
     expires_after_seconds = {{ int $expiresAfterSeconds.volumeWrite }}
     {{- end }}
@@ -41,7 +41,7 @@ data:
     # - the Volume server validates the JWT on reading
     # the jwt defaults to expire after 60 seconds
     [jwt.signing.read]
-    key = "{{ dig "jwt" "signing" "read" "key" (randAlphaNum 10 | b64enc) $securityConfig }}"
+    key = {{ include "seaweedfs.existingTomlKey" (list "jwt.signing.read" $existingToml) | default (randAlphaNum 10 | b64enc | quote) }}
     {{- if gt (int $expiresAfterSeconds.volumeRead) 0 }}
     expires_after_seconds = {{ int $expiresAfterSeconds.volumeRead }}
     {{- end }}
@@ -53,7 +53,7 @@ data:
     # - the Filer server validates the JWT on writing
     # the jwt defaults to expire after 10 seconds
     [jwt.filer_signing]
-    key = "{{ dig "jwt" "filer_signing" "key" (randAlphaNum 10 | b64enc) $securityConfig }}"
+    key = {{ include "seaweedfs.existingTomlKey" (list "jwt.filer_signing" $existingToml) | default (randAlphaNum 10 | b64enc | quote) }}
     {{- if gt (int $expiresAfterSeconds.filerWrite) 0 }}
     expires_after_seconds = {{ int $expiresAfterSeconds.filerWrite }}
     {{- end }}
@@ -65,7 +65,7 @@ data:
     # - the Filer server validates the JWT on reading
     # the jwt defaults to expire after 60 seconds
     [jwt.filer_signing.read]
-    key = "{{ dig "jwt" "filer_signing" "read" "key" (randAlphaNum 10 | b64enc) $securityConfig }}"
+    key = {{ include "seaweedfs.existingTomlKey" (list "jwt.filer_signing.read" $existingToml) | default (randAlphaNum 10 | b64enc | quote) }}
     {{- if gt (int $expiresAfterSeconds.filerRead) 0 }}
     expires_after_seconds = {{ int $expiresAfterSeconds.filerRead }}
     {{- end }}