mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
* refactor(helm): derive the postgres Secret name from one helper `databaseUrlSecret` re-encoded the `postgresql.external.existingSecret` then `postgresql.existingSecret` chain in its own `if`/`else if`. Three consumers read it — the gateway's `DATABASE_URL`, the postgres StatefulSet's `POSTGRES_PASSWORD`, and `postgres-secret.yaml`'s own `metadata.name` — so a source added to one helper and not the other would make the gateway and the StatefulSet resolve different Secrets. That is the class of bug #7149 fixed for the redis pair. Extract `databaseUserSecret` as the single encoding of the chain and let `databaseUrlSecret` fall back to the chart-managed name when it is empty, mirroring `redisUserSecret` / `redisUrlSecret`. Pure refactor: the rendered manifest is byte-identical before and after across the four configurations (chart-managed, `existingSecret`, `external.existingSecret`, and both set), compared with `helm template`. Adds `test_helm_database_secret_precedence.py`, which pins that both consumers name the same Secret and that only the chart-managed branch renders a Secret. Dropping either precedence source fails the two cases that depend on it. Suggested in review of #7161. * fix(helm): keep the postgres Secret helper's truthiness Review follow-up on the previous commit. Two things were still wrong: `postgres-secret.yaml` kept its own copy of the precedence chain for the render gate, so "the only place the chain is encoded" was not yet true — a source added to `databaseUserSecret` alone would have left that gate empty and rendered an unreferenced `<release>-postgres` Secret. It now reads the helper, like `redis-secret.yaml` already did. `or` returns its last operand even when it is falsy, so `postgresql.existingSecret: false` (how people spell "not set") named the Secret `false` instead of falling back to the chart-managed one. Chaining `default` keeps the old if/else-if truthiness and yields an empty string for any falsy value; the gateway's `$pgConfigured` now reads the same helper rather than re-listing the two keys. Two falsy cases join the parametrize list: `false` in the lower-precedence slot and in both slots. Both fail against the previous `or` version with `assert [] == [False]` — the Secret literally named `false` — so they are regressions this test can actually catch. The other four configurations render byte-identically before and after. * fix(helm): keep the redis user-Secret helper's truthiness Same defect the postgres helper had, one release later: `redisUserSecret` used `or`, which returns its last operand even when falsy, so `redis.existingSecret: false` named both the gateway's `redis-url` ref and the StatefulSet's `redis-password` ref after a Secret called `false`. The render gate in `redis-secret.yaml` saw the string "false" as truthy, so it skipped creating the chart-managed Secret — leaving two dangling refs and a StatefulSet that cannot start. `default` chaining restores the pre-`redisUserSecret` if/else-if behaviour: any falsy value yields an empty string, so the chart-managed branch is taken. `redisConfigured` now reads the helper instead of re-listing the two keys, matching the gateway's `$pgConfigured`. Verified against the four string-valued configurations (chart-managed, `existingSecret`, `external.existingSecret`, both): renders are byte-identical before and after. The two new falsy cases fail on the previous version, and `--basetemp` makes all 14 cases in this file pass. * docs(helm): drop a stray article in the helper comment
95 lines
4.0 KiB
Python
95 lines
4.0 KiB
Python
"""The gateway and the bundled postgres must resolve the same Secret.
|
|
|
|
`deer-flow.databaseUrlSecret` is read by three consumers — the gateway's
|
|
`DATABASE_URL`, the postgres StatefulSet's `POSTGRES_PASSWORD`, and
|
|
`postgres-secret.yaml`'s own `metadata.name` — so the precedence chain
|
|
(`postgresql.external.existingSecret` → `postgresql.existingSecret` →
|
|
`<release>-postgres`) has to exist once. This suite pins that chain and that
|
|
every consumer follows it.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import shutil
|
|
import subprocess
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
import yaml
|
|
|
|
REPO_ROOT = Path(__file__).resolve().parents[2]
|
|
CHART = REPO_ROOT / "deploy" / "helm" / "deer-flow"
|
|
RELEASE = "deer-flow"
|
|
|
|
|
|
def _render_chart(*settings: str) -> list[dict]:
|
|
helm = shutil.which("helm")
|
|
if helm is None:
|
|
pytest.skip("helm is unavailable")
|
|
# Pin the generated app secrets so a render is reproducible.
|
|
command = [helm, "template", RELEASE, str(CHART), "--set", "existingAppSecret=fixed-app-secret"]
|
|
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 _postgres_secrets(documents: list[dict]) -> list[str]:
|
|
return sorted(document["metadata"]["name"] for document in documents if document.get("kind") == "Secret" and document["metadata"]["name"].endswith("-postgres"))
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
("settings", "expected"),
|
|
[
|
|
((), None),
|
|
(("postgresql.existingSecret=custom-pg",), "custom-pg"),
|
|
(("postgresql.external.existingSecret=ext-pg",), "ext-pg"),
|
|
(("postgresql.existingSecret=custom-pg", "postgresql.external.existingSecret=ext-pg"), "ext-pg"),
|
|
# Falsy non-string values are how people spell "not set" in values.yaml.
|
|
# The old if/else-if treated them as unset, and the helper has to keep
|
|
# doing that: `or` returns its last operand even when it is falsy, which
|
|
# would name the Secret `false`. `default ""` yields an empty string
|
|
# instead, so the chart-managed branch is taken.
|
|
(("postgresql.existingSecret=false",), None),
|
|
(("postgresql.external.existingSecret=false", "postgresql.existingSecret=false"), None),
|
|
],
|
|
ids=[
|
|
"chart-managed",
|
|
"existing-secret",
|
|
"external-secret",
|
|
"external-wins",
|
|
"existing-secret-false",
|
|
"both-secrets-false",
|
|
],
|
|
)
|
|
def test_gateway_and_postgres_resolve_the_same_secret(settings: tuple[str, ...], expected: str | None) -> None:
|
|
documents = _render_chart(*settings)
|
|
|
|
gateway_ref = _secret_ref(_container(documents, "gateway"), "DATABASE_URL")
|
|
postgres_ref = _secret_ref(_container(documents, "postgres"), "POSTGRES_PASSWORD")
|
|
assert gateway_ref["key"] == "database-url"
|
|
assert postgres_ref["key"] == "postgres-password"
|
|
|
|
# The point of the shared helper: both consumers name the same Secret, so a
|
|
# source added to one of them cannot silently split the credentials.
|
|
assert gateway_ref["name"] == postgres_ref["name"]
|
|
|
|
secrets = _postgres_secrets(documents)
|
|
if expected is None:
|
|
# Chart-managed branch: the chart creates the Secret the consumers name.
|
|
assert secrets == [gateway_ref["name"]]
|
|
else:
|
|
# User-managed branches: the Secret must already exist, so rendering one
|
|
# would shadow the operator's.
|
|
assert gateway_ref["name"] == expected
|
|
assert secrets == []
|