mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
fix(helm): honor redis-password from a user-managed Secret (#7161)
* fix(helm): honor redis-password from a user-managed Secret The bundled Redis StatefulSet derived its AUTH mode only from `redis.auth.password`, so a `redis.existingSecret` that carried both `redis-url` and `redis-password` left the gateway authenticating against a server started without `--requirepass`. Both probes used the no-auth path as well, so the Pod reported healthy while the gateway could not connect to it. Whether a user-managed Secret carries `redis-password` is only knowable at runtime, so the key is now read with `optional: true` and the entrypoint enables AUTH only when the value is nonempty. A Secret carrying only `redis-url` keeps the no-auth behavior instead of failing to start, and a stale `redis.auth.password` no longer wins over the selected Secret, matching what values.yaml documents. Refs #7149 * test(helm): cover Redis Secret authentication and share resolution Add the issue #7149 render-and-shell regression suite for chart-managed, user-managed and external Redis credentials. Resolve Secret precedence once for the Gateway, bundled server and chart-owned Secret, and clarify how optional user-managed passwords select AUTH at runtime. Regression coverage adapted from dakjdakd's commit 7e20745d678276ed33e8f8eafd2c576fa8ae9aec. Co-authored-by: dakjdakd <1428823446@qq.com> * test(helm): assert the stale value instead of an unused constant `test_bundled_redis_uses_optional_existing_secret_password` checked the module-level `PASSWORD` constant against the rendered manifest, but that constant is never an input to any of its six renders — the parametrized value is `ignored_password`. The guard could therefore never fail, so a template edit that started rendering `redis.auth.password` alongside a user-managed Secret would have gone unnoticed. Assert the parametrized value instead: it is the stale input the case is meant to keep out of the manifest. Verified the value is detectable in a render that does carry it (the chart-managed path renders it twice), so the assertion is no longer vacuous. Also fix the garbled sentence in `tests/AGENTS.md` that described this coverage as "stale values passwords". --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> Co-authored-by: dakjdakd <1428823446@qq.com>
This commit is contained in:
parent
c11ba84708
commit
493178dd86
@ -363,6 +363,14 @@ setup, after the autouse fixtures initialize runtime, to avoid the package cycle
|
||||
|
||||
`test_managed_provider_presets.py` covers encrypted compatibility, auto IDs, key switching, snapshot isolation and native/compatible protocols. Its HTTP double is below actual LangChain/vendor SDK serialization; do not replace these adapters with successful stubs. `test_managed_presets_live.py` requires explicit opt-in and vendor credentials, and remains skipped in CI.
|
||||
|
||||
## Helm Redis credentials
|
||||
|
||||
`test_helm_redis_existing_secret.py` renders the chart and executes its startup
|
||||
and both probe commands with argument-recording Redis stubs through
|
||||
`support.shell.require_posix_sh`. Keep missing/empty password keys, stale
|
||||
`redis.auth.password` values, Secret precedence, external mode and shell
|
||||
metacharacters covered.
|
||||
Render tests skip when Helm is unavailable; they need no Kubernetes cluster.
|
||||
## Multi-instance harness
|
||||
|
||||
Multi-instance harness tests use `support.shell.require_script_bash()` for shell
|
||||
|
||||
118
backend/tests/test_helm_redis_existing_secret.py
Normal file
118
backend/tests/test_helm_redis_existing_secret.py
Normal file
@ -0,0 +1,118 @@
|
||||
"""Bundled Redis and the Gateway must use the same Secret credentials."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import shutil
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
from urllib.parse import unquote, urlsplit
|
||||
|
||||
import pytest
|
||||
import yaml
|
||||
from support.shell import require_posix_sh
|
||||
|
||||
REPO_ROOT = Path(__file__).resolve().parents[2]
|
||||
CHART = REPO_ROOT / "deploy" / "helm" / "deer-flow"
|
||||
PASSWORD = "p a$$'\"@:/?#%[]*"
|
||||
|
||||
|
||||
def _render_chart(*settings: str) -> list[dict]:
|
||||
helm = shutil.which("helm")
|
||||
if helm is None:
|
||||
pytest.skip("helm is unavailable")
|
||||
command = [helm, "template", "deer-flow", str(CHART)]
|
||||
for setting in settings:
|
||||
command.extend(["--set", setting])
|
||||
rendered = subprocess.run(command, check=True, capture_output=True, text=True, encoding="utf-8").stdout
|
||||
return [document for document in yaml.safe_load_all(rendered) if isinstance(document, dict)]
|
||||
|
||||
|
||||
def _container(documents: list[dict], name: str) -> dict:
|
||||
workload = next(document for document in documents if document.get("kind") in {"Deployment", "StatefulSet"} and document["metadata"]["name"].endswith(f"-{name}"))
|
||||
return next(container for container in workload["spec"]["template"]["spec"]["containers"] if container["name"] == name)
|
||||
|
||||
|
||||
def _secret_ref(container: dict, env_name: str) -> dict:
|
||||
return next(item for item in container["env"] if item["name"] == env_name)["valueFrom"]["secretKeyRef"]
|
||||
|
||||
|
||||
def _run_command(command: list[str], tmp_path: Path, password: str | None) -> list[str]:
|
||||
"""Execute the rendered shell with argv-recording Redis executables."""
|
||||
assert command[:2] == ["sh", "-c"]
|
||||
for name in ("redis-server", "redis-cli"):
|
||||
executable = tmp_path / name
|
||||
executable.write_text(f"#!/bin/sh\nprintf '%s\\0' {name} \"$@\"\n", encoding="utf-8", newline="\n")
|
||||
executable.chmod(0o755)
|
||||
env = {key: value for key, value in os.environ.items() if key != "REDIS_PASSWORD"}
|
||||
if password is not None:
|
||||
env["REDIS_PASSWORD"] = password
|
||||
# cd accepts native checkout paths in Git Bash, and pwd supplies its PATH form.
|
||||
script = 'PATH="$(cd "$1" && pwd):$PATH"; export PATH; ' + command[2]
|
||||
result = subprocess.run([require_posix_sh(), "-c", script, "_", tmp_path.as_posix()], env=env, check=True, capture_output=True, text=True, encoding="utf-8")
|
||||
return result.stdout.rstrip("\0").split("\0")
|
||||
|
||||
|
||||
@pytest.mark.parametrize("password", ["", PASSWORD])
|
||||
def test_chart_managed_redis_credentials_match_the_gateway(password: str, tmp_path: Path) -> None:
|
||||
documents = _render_chart(f"redis.auth.password={password}")
|
||||
redis = _container(documents, "redis")
|
||||
gateway_ref = _secret_ref(_container(documents, "gateway"), "DEER_FLOW_STREAM_BRIDGE_REDIS_URL")
|
||||
secret = next(document for document in documents if document.get("kind") == "Secret" and document["metadata"]["name"] == gateway_ref["name"])
|
||||
url = urlsplit(secret["stringData"][gateway_ref["key"]])
|
||||
assert unquote(url.password or "") == password
|
||||
assert url.hostname == gateway_ref["name"]
|
||||
if password:
|
||||
assert secret["stringData"]["redis-password"] == password
|
||||
assert _secret_ref(redis, "REDIS_PASSWORD") == {"name": gateway_ref["name"], "key": "redis-password"}
|
||||
else:
|
||||
assert "redis-password" not in secret["stringData"]
|
||||
assert "env" not in redis
|
||||
expected_auth = ["--requirepass", password] if password else []
|
||||
assert _run_command(redis["command"] + redis["args"], tmp_path, password or None) == ["redis-server", "--appendonly", "yes", *expected_auth]
|
||||
for probe in ("readinessProbe", "livenessProbe"):
|
||||
expected_auth = ["--no-auth-warning", "-a", password] if password else []
|
||||
assert _run_command(redis[probe]["exec"]["command"], tmp_path, password or None) == ["redis-cli", *expected_auth, "ping"]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("password", [None, "", PASSWORD])
|
||||
@pytest.mark.parametrize("ignored_password", ["", "ignored-values-password"])
|
||||
def test_bundled_redis_uses_optional_existing_secret_password(password: str | None, ignored_password: str, tmp_path: Path) -> None:
|
||||
documents = _render_chart("redis.existingSecret=managed-redis", f"redis.auth.password={ignored_password}")
|
||||
redis = _container(documents, "redis")
|
||||
assert _secret_ref(redis, "REDIS_PASSWORD") == {"name": "managed-redis", "key": "redis-password", "optional": True}
|
||||
assert _secret_ref(_container(documents, "gateway"), "DEER_FLOW_STREAM_BRIDGE_REDIS_URL") == {"name": "managed-redis", "key": "redis-url"}
|
||||
assert not any(document.get("kind") == "Secret" and document["metadata"]["name"].endswith("-redis") for document in documents)
|
||||
if ignored_password:
|
||||
# The stale value must not leak into the manifest: it is the only input
|
||||
# here that a template edit could accidentally start rendering.
|
||||
assert ignored_password not in yaml.safe_dump(documents)
|
||||
expected_auth = ["--requirepass", password] if password else []
|
||||
assert _run_command(redis["command"] + redis["args"], tmp_path, password) == ["redis-server", "--appendonly", "yes", *expected_auth]
|
||||
for probe in ("readinessProbe", "livenessProbe"):
|
||||
argv = _run_command(redis[probe]["exec"]["command"], tmp_path, password)
|
||||
assert argv == ["redis-cli", *(["--no-auth-warning", "-a", password] if password else []), "ping"]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("enabled", ["true", "false"])
|
||||
def test_external_secret_precedence_is_unchanged(enabled: str) -> None:
|
||||
# The existing helper gives external.existingSecret precedence in both modes.
|
||||
documents = _render_chart(f"redis.enabled={enabled}", "redis.existingSecret=bundled-secret", "redis.external.existingSecret=external-secret")
|
||||
assert _secret_ref(_container(documents, "gateway"), "DEER_FLOW_STREAM_BRIDGE_REDIS_URL") == {"name": "external-secret", "key": "redis-url"}
|
||||
if enabled == "true":
|
||||
assert _secret_ref(_container(documents, "redis"), "REDIS_PASSWORD") == {"name": "external-secret", "key": "redis-password", "optional": True}
|
||||
else:
|
||||
assert not any(document.get("kind") == "StatefulSet" and document["metadata"]["name"].endswith("-redis") for document in documents)
|
||||
assert not any(document.get("kind") == "Secret" and document["metadata"]["name"].endswith("-redis") for document in documents)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("setting", ["redis.existingSecret=managed-external", "redis.external.redisUrl=redis://:password@managed.example:6380/1"])
|
||||
def test_external_redis_does_not_deploy_a_bundled_instance(setting: str) -> None:
|
||||
documents = _render_chart("redis.enabled=false", setting)
|
||||
assert not any(document.get("kind") == "StatefulSet" and document["metadata"]["name"].endswith("-redis") for document in documents)
|
||||
ref = _secret_ref(_container(documents, "gateway"), "DEER_FLOW_STREAM_BRIDGE_REDIS_URL")
|
||||
if setting.startswith("redis.existingSecret="):
|
||||
assert ref == {"name": "managed-external", "key": "redis-url"}
|
||||
else:
|
||||
secret = next(document for document in documents if document.get("kind") == "Secret" and document["metadata"]["name"] == ref["name"])
|
||||
assert secret["stringData"] == {"redis-url": setting.split("=", 1)[1]}
|
||||
@ -426,11 +426,15 @@ kubectl -n deer-flow exec deploy/deer-flow-provisioner -- curl -s localhost:8002
|
||||
auto-generated into a Secret (key `redis-url`) and injected as
|
||||
`DEER_FLOW_STREAM_BRIDGE_REDIS_URL`; `config.yaml` sets `stream_bridge.type:
|
||||
redis` by default. No-auth by default (ClusterIP isolation, matching compose);
|
||||
set `redis.auth.password` to enable AUTH. For a managed Redis, disable the
|
||||
bundled instance and point at it via `redis.external`. The gateway readiness
|
||||
probe pings this Redis on every check and reports the pod unready while it
|
||||
is unreachable, since without the bridge no run can publish, stream or be
|
||||
cancelled from a peer pod.
|
||||
set `redis.auth.password` to enable AUTH. A user-managed
|
||||
`redis.existingSecret` takes precedence over that value: the bundled server
|
||||
reads `redis-password` from the same Secret that supplies the gateway's
|
||||
`redis-url`, and stays unauthenticated when that key is absent or empty — so
|
||||
keep the URL and the raw password in agreement. For a managed Redis, disable
|
||||
the bundled instance and point at it via `redis.external`. The gateway
|
||||
readiness probe pings this Redis on every check and reports the pod unready
|
||||
while it is unreachable, since without the bridge no run can publish, stream
|
||||
or be cancelled from a peer pod.
|
||||
- **Persistence.** A PVC (`<release>-home`) backs `/app/backend/.deer-flow`
|
||||
(sqlite DB, memory, custom agents, per-thread user-data). The gateway mounts
|
||||
it with `subPath: deer-flow` so the layout matches the provisioner's PVC
|
||||
|
||||
@ -157,9 +157,44 @@ imagePullSecrets:
|
||||
3. chart-managed secret `<release>-redis`
|
||||
Only #3 is created by this chart; #1/#2 must exist already. */}}
|
||||
{{- define "deer-flow.redisUrlSecret" -}}
|
||||
{{- if .Values.redis.external.existingSecret -}}{{- .Values.redis.external.existingSecret -}}
|
||||
{{- else if .Values.redis.existingSecret -}}{{- .Values.redis.existingSecret -}}
|
||||
{{- else -}}{{- include "deer-flow.redisFullname" . -}}{{- end -}}
|
||||
{{- include "deer-flow.redisUserSecret" . | default (include "deer-flow.redisFullname" .) -}}
|
||||
{{- end -}}
|
||||
|
||||
{{/* Name of the operator-managed redis Secret, or empty when the chart owns the
|
||||
credentials. Only the chart-managed Secret is created by this chart, and
|
||||
only there is the AUTH password known at render time — a user-managed
|
||||
Secret may or may not carry `redis-password`. */}}
|
||||
{{- define "deer-flow.redisUserSecret" -}}
|
||||
{{- or .Values.redis.external.existingSecret .Values.redis.existingSecret -}}
|
||||
{{- end -}}
|
||||
|
||||
{{/* Args for the bundled redis container. With a user-managed Secret the
|
||||
password is only knowable at runtime, so the container reads
|
||||
`redis-password` from that Secret and enables AUTH only when the value is
|
||||
nonempty. This keeps a `redis-url`-only Secret in no-auth mode instead of
|
||||
failing to start, and keeps a stale `redis.auth.password` from winning over
|
||||
the user-managed Secret (values.yaml documents it as ignored there). */}}
|
||||
{{- define "deer-flow.redisServerArgs" -}}
|
||||
{{- if include "deer-flow.redisUserSecret" . -}}
|
||||
if [ -n "$REDIS_PASSWORD" ]; then exec redis-server --appendonly yes --requirepass "$REDIS_PASSWORD"; fi; exec redis-server --appendonly yes
|
||||
{{- else if .Values.redis.auth.password -}}
|
||||
exec redis-server --appendonly yes --requirepass "$REDIS_PASSWORD"
|
||||
{{- else -}}
|
||||
exec redis-server --appendonly yes
|
||||
{{- end -}}
|
||||
{{- end -}}
|
||||
|
||||
{{/* Readiness/liveness command for the bundled redis container. Mirrors
|
||||
redisServerArgs so the probes authenticate exactly when the server has a
|
||||
password — otherwise a password-protected server reports itself unhealthy. */}}
|
||||
{{- define "deer-flow.redisPingCommand" -}}
|
||||
{{- if include "deer-flow.redisUserSecret" . -}}
|
||||
if [ -n "$REDIS_PASSWORD" ]; then redis-cli --no-auth-warning -a "$REDIS_PASSWORD" ping; else redis-cli ping; fi
|
||||
{{- else if .Values.redis.auth.password -}}
|
||||
redis-cli --no-auth-warning -a "$REDIS_PASSWORD" ping
|
||||
{{- else -}}
|
||||
redis-cli ping
|
||||
{{- end -}}
|
||||
{{- end -}}
|
||||
|
||||
{{/* Whether any redis stream-bridge backend is configured (bundled StatefulSet,
|
||||
|
||||
@ -8,11 +8,15 @@
|
||||
NOT created when the user manages the Secret themselves:
|
||||
• redis.external.existingSecret OR redis.existingSecret → referenced only
|
||||
|
||||
Auth: empty `redis.auth.password` (default) = no auth, matching the compose
|
||||
deployment (ClusterIP isolation only). Set a password to enable ACL/AUTH;
|
||||
the DSN then becomes redis://:<password>@host:6379/0.
|
||||
Bundled AUTH with chart-managed credentials: empty `redis.auth.password`
|
||||
(default) = no auth, matching compose (ClusterIP isolation only). A nonempty
|
||||
password enables AUTH and a redis://:<password>@host:6379/0 DSN.
|
||||
With a user-managed Secret, `redis.auth.password` is ignored: its optional
|
||||
`redis-password` key enables AUTH at runtime only when nonempty. An absent or
|
||||
empty key keeps no-auth mode; the Secret's `redis-url` must match that choice.
|
||||
External Redis authentication is configured through its `redis-url`.
|
||||
*/ -}}
|
||||
{{- $userSecret := or .Values.redis.external.existingSecret .Values.redis.existingSecret -}}
|
||||
{{- $userSecret := include "deer-flow.redisUserSecret" . -}}
|
||||
{{- $wrapExternal := and (not .Values.redis.enabled) .Values.redis.external.redisUrl -}}
|
||||
{{- if and (not $userSecret) (or .Values.redis.enabled $wrapExternal) -}}
|
||||
{{- $password := .Values.redis.auth.password -}}
|
||||
|
||||
@ -1,5 +1,8 @@
|
||||
{{- if .Values.redis.enabled -}}
|
||||
{{- $password := .Values.redis.auth.password -}}
|
||||
{{- /* A user-managed Secret wins over `redis.auth.password` (see values.yaml),
|
||||
and whether it carries `redis-password` is only knowable at runtime. */ -}}
|
||||
{{- $userSecret := include "deer-flow.redisUserSecret" . -}}
|
||||
apiVersion: apps/v1
|
||||
kind: StatefulSet
|
||||
metadata:
|
||||
@ -43,7 +46,19 @@ spec:
|
||||
ports:
|
||||
- name: tcp-redis
|
||||
containerPort: 6379
|
||||
{{- if $password }}
|
||||
{{- if $userSecret }}
|
||||
# AUTH password is read from the operator-managed Secret at runtime so
|
||||
# it never appears in the pod spec. The key is optional there: a
|
||||
# Secret carrying only `redis-url` keeps the bundled server
|
||||
# unauthenticated, which is what the chart's Secret contract allows.
|
||||
env:
|
||||
- name: REDIS_PASSWORD
|
||||
valueFrom:
|
||||
secretKeyRef:
|
||||
name: {{ include "deer-flow.redisUrlSecret" . }}
|
||||
key: redis-password
|
||||
optional: true
|
||||
{{- else if $password }}
|
||||
# AUTH password is read at runtime so it never appears in the pod spec
|
||||
# args. `redis-server --requirepass` + `redis-cli -a` both read it
|
||||
# from this env.
|
||||
@ -56,13 +71,13 @@ spec:
|
||||
{{- end }}
|
||||
command: ["sh", "-c"]
|
||||
args:
|
||||
- exec redis-server --appendonly yes{{ if $password }} --requirepass "$REDIS_PASSWORD"{{ end }}
|
||||
- {{ include "deer-flow.redisServerArgs" . | quote }}
|
||||
readinessProbe:
|
||||
exec:
|
||||
command:
|
||||
- sh
|
||||
- -c
|
||||
- {{ if $password }}redis-cli --no-auth-warning -a "$REDIS_PASSWORD" ping{{ else }}redis-cli ping{{ end }}
|
||||
- {{ include "deer-flow.redisPingCommand" . | quote }}
|
||||
initialDelaySeconds: 5
|
||||
periodSeconds: 10
|
||||
livenessProbe:
|
||||
@ -70,7 +85,7 @@ spec:
|
||||
command:
|
||||
- sh
|
||||
- -c
|
||||
- {{ if $password }}redis-cli --no-auth-warning -a "$REDIS_PASSWORD" ping{{ else }}redis-cli ping{{ end }}
|
||||
- {{ include "deer-flow.redisPingCommand" . | quote }}
|
||||
initialDelaySeconds: 30
|
||||
periodSeconds: 20
|
||||
{{- with .Values.redis.primary.resources }}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user