diff --git a/backend/tests/AGENTS.md b/backend/tests/AGENTS.md index 02653e71ae..e98283a7a4 100644 --- a/backend/tests/AGENTS.md +++ b/backend/tests/AGENTS.md @@ -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 diff --git a/backend/tests/test_helm_redis_existing_secret.py b/backend/tests/test_helm_redis_existing_secret.py new file mode 100644 index 0000000000..c1d00528b8 --- /dev/null +++ b/backend/tests/test_helm_redis_existing_secret.py @@ -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]} diff --git a/deploy/helm/deer-flow/README.md b/deploy/helm/deer-flow/README.md index db6d38879d..deec377e43 100644 --- a/deploy/helm/deer-flow/README.md +++ b/deploy/helm/deer-flow/README.md @@ -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 (`-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 diff --git a/deploy/helm/deer-flow/templates/_helpers.tpl b/deploy/helm/deer-flow/templates/_helpers.tpl index cd3093845f..d3bbb6959c 100644 --- a/deploy/helm/deer-flow/templates/_helpers.tpl +++ b/deploy/helm/deer-flow/templates/_helpers.tpl @@ -157,9 +157,44 @@ imagePullSecrets: 3. chart-managed secret `-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, diff --git a/deploy/helm/deer-flow/templates/redis-secret.yaml b/deploy/helm/deer-flow/templates/redis-secret.yaml index 50035545c6..b11b726fd2 100644 --- a/deploy/helm/deer-flow/templates/redis-secret.yaml +++ b/deploy/helm/deer-flow/templates/redis-secret.yaml @@ -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://:@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://:@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 -}} diff --git a/deploy/helm/deer-flow/templates/redis-statefulset.yaml b/deploy/helm/deer-flow/templates/redis-statefulset.yaml index a094830919..a846a021af 100644 --- a/deploy/helm/deer-flow/templates/redis-statefulset.yaml +++ b/deploy/helm/deer-flow/templates/redis-statefulset.yaml @@ -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 }}