diff --git a/README.md b/README.md index 1652f76b9..1cd121236 100644 --- a/README.md +++ b/README.md @@ -1168,9 +1168,11 @@ For remote/Kubernetes deployments (the provisioner backend), the sandbox copies the binaries into a shared `emptyDir` — no install-time GitHub download and no hostPath/PVC runtime mount. Publish the image under [`docker/lark-cli-init`](docker/lark-cli-init/README.md) and set -`LARK_CLI_INIT_IMAGE` on the provisioner; it stays off (legacy behavior) when -unset. The Lark integration status (`GET /api/integrations/lark/status`) reports -`sandbox_runtime_mode`, `sandbox_runtime_probed`, and `sandbox_runtime_ready`. +`LARK_CLI_INIT_IMAGE` on the provisioner (with the Helm chart, +`provisioner.larkCliInitImage` / `provisioner.larkCliBrokerImage`); it stays off +(legacy behavior) when unset. The Lark integration status +(`GET /api/integrations/lark/status`) reports `sandbox_runtime_mode`, +`sandbox_runtime_probed`, and `sandbox_runtime_ready`. `sandbox_runtime_probed` marks whether runtime readiness was actually evaluated; responses from older backends may omit the flag, in which case the Settings mutation cache keeps the last probed runtime fields instead of diff --git a/backend/tests/test_helm_provisioner_lark_cli_images.py b/backend/tests/test_helm_provisioner_lark_cli_images.py new file mode 100644 index 000000000..f0a85f079 --- /dev/null +++ b/backend/tests/test_helm_provisioner_lark_cli_images.py @@ -0,0 +1,92 @@ +"""Regression tests for the Helm provisioner lark-cli sandbox runtime images. + +The provisioner reads ``LARK_CLI_INIT_IMAGE`` (Pattern A: init container + +shared ``emptyDir``) and ``LARK_CLI_BROKER_IMAGE`` (Pattern B: shim + credential +broker sidecar) and treats either as off when it is empty — +see ``docker/provisioner/app.py``. ``docker/docker-compose.yaml`` and the root +``README.md`` both expose the two variables, but the chart never plumbed them +into the provisioner Pod, so a Helm operator following the README had no value +to set and silently got a sandbox without a ``lark-cli`` runtime. +""" + +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" +VALUES = CHART / "values.yaml" + +INIT_IMAGE = "registry.example.com/deer-flow/lark-cli-init:v1.0.65" +BROKER_IMAGE = "registry.example.com/deer-flow/lark-cli-broker:v1.0.65" + +LARK_CLI_ENV_VARS = ("LARK_CLI_INIT_IMAGE", "LARK_CLI_BROKER_IMAGE") + + +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).stdout + return [document for document in yaml.safe_load_all(rendered) if isinstance(document, dict)] + + +def _provisioner_env(documents: list[dict]) -> dict[str, dict]: + deployment = next(document for document in documents if document.get("kind") == "Deployment" and document["metadata"]["name"].endswith("-provisioner")) + container = next(item for item in deployment["spec"]["template"]["spec"]["containers"] if item["name"] == "provisioner") + return {item["name"]: item for item in container.get("env", [])} + + +def test_values_declare_both_lark_cli_image_keys_as_empty() -> None: + provisioner = yaml.safe_load(VALUES.read_text(encoding="utf-8"))["provisioner"] + assert provisioner["larkCliInitImage"] == "" + assert provisioner["larkCliBrokerImage"] == "" + + +def test_default_chart_omits_both_lark_cli_image_env_vars() -> None: + env = _provisioner_env(_render_chart()) + # The provisioner keys off ``bool(env)``, so the chart must omit the variable + # rather than inject an empty string: an empty value would still be "off", + # but only absence keeps the rendered Pod byte-identical to a chart that + # never knew about lark-cli. + for name in LARK_CLI_ENV_VARS: + assert name not in env + + +@pytest.mark.parametrize( + ("settings", "expected"), + [ + ((f"provisioner.larkCliInitImage={INIT_IMAGE}",), {"LARK_CLI_INIT_IMAGE": INIT_IMAGE}), + ((f"provisioner.larkCliBrokerImage={BROKER_IMAGE}",), {"LARK_CLI_BROKER_IMAGE": BROKER_IMAGE}), + ( + ( + f"provisioner.larkCliInitImage={INIT_IMAGE}", + f"provisioner.larkCliBrokerImage={BROKER_IMAGE}", + ), + {"LARK_CLI_INIT_IMAGE": INIT_IMAGE, "LARK_CLI_BROKER_IMAGE": BROKER_IMAGE}, + ), + ], +) +def test_rendered_provisioner_env_carries_lark_cli_images(settings: tuple[str, ...], expected: dict[str, str]) -> None: + env = _provisioner_env(_render_chart(*settings)) + for name, value in expected.items(): + assert env[name]["value"] == value + for name in LARK_CLI_ENV_VARS: + if name not in expected: + assert name not in env + + +def test_explicitly_empty_lark_cli_images_stay_omitted() -> None: + env = _provisioner_env( + _render_chart("provisioner.larkCliInitImage=", "provisioner.larkCliBrokerImage="), + ) + for name in LARK_CLI_ENV_VARS: + assert name not in env diff --git a/deploy/helm/deer-flow/README.md b/deploy/helm/deer-flow/README.md index 818ace754..a03c9d948 100644 --- a/deploy/helm/deer-flow/README.md +++ b/deploy/helm/deer-flow/README.md @@ -462,6 +462,44 @@ On multi-node clusters, also switch `persistence.home.accessMode` to `ReadWriteMany` (this is orthogonal to the Service type - it governs whether a sandbox Pod can be scheduled on a node other than the gateway's). +## Sandbox lark-cli runtime (optional) + +The Lark/Feishu `lark-cli` integration needs a `lark-cli` binary inside the +sandbox. For remote/Kubernetes (provisioner) deployments the sandbox-side path +comes from an optional runtime image instead of an install-time GitHub +download. The chart exposes the same two knobs the Compose stack reads on the +provisioner: + +```yaml +provisioner: + # Pattern A - an init container copies the binaries into a shared emptyDir. + larkCliInitImage: deer-flow/lark-cli-init:v1.0.65 + # Pattern B - a shim init container + broker sidecar owns the credentials, so + # the plaintext config/data dirs are never mounted into the sandbox. + # Supersedes larkCliInitImage when both are set. + larkCliBrokerImage: deer-flow/lark-cli-broker:v1.0.65 +``` + +Both default to empty, which leaves the feature off (legacy behavior) and makes +an in-sandbox `lark-cli` call fail with exit 127 (`command not found`). When +set, they render `LARK_CLI_INIT_IMAGE` / `LARK_CLI_BROKER_IMAGE` on the +provisioner Deployment - the names `docker/docker-compose.yaml` uses - and the +variable is omitted entirely while empty. Point them at a tag that exists in a +registry your nodes can pull from (mirror the registry prefix if you do not use +Docker Hub). The images are built from `docker/lark-cli-init` and +`docker/lark-cli-broker`; see those READMEs and the root README's Lark section +for the build/publish flow and the credential model. Broker mode is the safer +choice on a shared cluster: the app secret and OAuth tokens stay in the sidecar +instead of the sandbox container. + +> An image here does not authenticate anyone by itself. The Gateway only asks +the provisioner to attach the runtime once the Lark integration pack is +installed for the user, so the sandbox gets the binary but the per-user +credentials still follow the normal install/authorize flow. The Lark +integration status reports `sandbox_runtime_mode` / `sandbox_runtime_ready` so +the Settings UI surfaces a missing runtime instead of a later +`command not found`. + ## Lint / dry-run ```bash diff --git a/deploy/helm/deer-flow/templates/provisioner-deployment.yaml b/deploy/helm/deer-flow/templates/provisioner-deployment.yaml index 23d27f96a..d117c6660 100644 --- a/deploy/helm/deer-flow/templates/provisioner-deployment.yaml +++ b/deploy/helm/deer-flow/templates/provisioner-deployment.yaml @@ -88,6 +88,19 @@ spec: {{- end }} - name: SANDBOX_CONTAINER_PORT value: {{ .Values.provisioner.sandboxPort | quote }} + # Optional lark-cli sandbox runtime images. The provisioner keys off + # "non-empty", so render the variables only when set; an empty + # string would still mean "off" but would churn the Pod spec on + # every values edit. Broker (Pattern B) supersedes init (Pattern A) + # in the provisioner when both are set. + {{- with .Values.provisioner.larkCliInitImage }} + - name: LARK_CLI_INIT_IMAGE + value: {{ . | quote }} + {{- end }} + {{- with .Values.provisioner.larkCliBrokerImage }} + - name: LARK_CLI_BROKER_IMAGE + value: {{ . | quote }} + {{- end }} readinessProbe: httpGet: path: /health diff --git a/deploy/helm/deer-flow/values.yaml b/deploy/helm/deer-flow/values.yaml index 2744f020b..1d610434b 100644 --- a/deploy/helm/deer-flow/values.yaml +++ b/deploy/helm/deer-flow/values.yaml @@ -111,6 +111,20 @@ provisioner: nodeHost: "" # -- Sandbox container port (must match the sandboxImage's listening port). sandboxPort: 8080 + # -- Optional lark-cli init image (Pattern A). Empty (default) leaves the + # sandbox without a lark-cli runtime, so an in-sandbox `lark-cli` call + # fails with "command not found". When set (e.g. + # `deer-flow/lark-cli-init:v1.0.65`), sandbox Pods requesting the runtime + # get an init container + shared emptyDir instead of a hostPath/PVC mount. + # Mirrors compose's `LARK_CLI_INIT_IMAGE`. See `docker/lark-cli-init/`. + larkCliInitImage: "" + # -- Optional lark-cli broker image (Pattern B, issue #4338). When set, the + # sandbox gets a shim + a `lark-cli-broker` sidecar that holds the + # credentials, so the plaintext config/data dirs are never mounted into + # the sandbox. Supersedes `larkCliInitImage` when both are set. Empty + # (default) ⇒ broker off. Mirrors compose's `LARK_CLI_BROKER_IMAGE`. + # See `docker/lark-cli-broker/`. + larkCliBrokerImage: "" # -- PostgreSQL database. Bundled mode (default) deploys a single-instance # postgres StatefulSet; set `enabled: false` to use an external managed DB.