diff --git a/.github/workflows/chart.yaml b/.github/workflows/chart.yaml index 6f7fef306..b0ec8d111 100644 --- a/.github/workflows/chart.yaml +++ b/.github/workflows/chart.yaml @@ -4,9 +4,9 @@ name: Publish Helm Chart # container images (see container.yaml). Triggers on the same `v*` tags. # # On pull requests touching the chart or config.example.yaml, `validate-chart` -# runs lint + template render + a config_version drift check (the chart's -# embedded config_version must not lag config.example.yaml) so a broken or -# stale chart fails the PR, not the release. +# runs lint + template render + a sandbox Service-type gating check + a +# config_version drift check (the chart's embedded config_version must not lag +# config.example.yaml) so a broken or stale chart fails the PR, not the release. # # Users then install with: # helm install deer-flow oci://ghcr.io/${{ owner }}/charts/deer-flow --version @@ -21,6 +21,7 @@ on: - "config.example.yaml" - ".github/workflows/chart.yaml" - "scripts/check_config_version.sh" + - "scripts/check_chart_sandbox_service.sh" jobs: validate-chart: @@ -42,6 +43,12 @@ jobs: - name: Validate templates render run: helm template deer-flow deploy/helm/deer-flow --include-crds >/dev/null + # Assert the sandbox Service-type gating: default emits + # SANDBOX_SERVICE_TYPE=ClusterIP and no NODE_HOST; the NodePort opt-in + # emits both. Guards the #3929 default flip against regressions. + - name: Validate sandbox Service-type gating + run: bash scripts/check_chart_sandbox_service.sh + # The chart's `config:` block embeds a config_version that must not fall # behind config.example.yaml. A stale version is silent in-cluster (the # image ships no example to compare against, so _check_config_version diff --git a/CHANGELOG.md b/CHANGELOG.md index b51fbaf44..61955c0e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- **sandbox:** The Helm chart now defaults per-sandbox Services to `ClusterIP` + instead of `NodePort`, so the code-execution sandbox is reachable only inside + the cluster via Service DNS (`http://sandbox--svc..svc.cluster.local`) + and is no longer bound on every node's interfaces - including the + externally-reachable ones on GKE/EKS/AKS. Existing chart installs flip + NodePort -> ClusterIP on upgrade. To preserve the old reachability (an + external probe hitting the 30xxx port, or the Docker-Compose/hybrid path + where the gateway is not in K8s), set `provisioner.sandboxServiceType: NodePort` + (with `provisioner.nodeHost` if needed). The provisioner itself is unchanged + (mode-aware since #4016). ([#4190]) + ### ⚠ Breaking changes - **skills:** A directory containing `SKILL.md` is now a runtime package @@ -590,3 +603,4 @@ with **180 merged pull requests** since the first 2.0 milestone tag. [#4095]: https://github.com/bytedance/deer-flow/issues/4095 [#4098]: https://github.com/bytedance/deer-flow/pull/4098 [#4146]: https://github.com/bytedance/deer-flow/pull/4146 +[#4190]: https://github.com/bytedance/deer-flow/pull/4190 diff --git a/deploy/helm/deer-flow/README.md b/deploy/helm/deer-flow/README.md index 2c6aa9bd5..19c1320b6 100644 --- a/deploy/helm/deer-flow/README.md +++ b/deploy/helm/deer-flow/README.md @@ -376,17 +376,28 @@ an opt-in root `volumePermissions` initContainer that chowns on every start (the Bitnami pattern) — is not yet wired into this chart; it would introduce a root container, so it's left as an operator decision for now. -## Sandbox NodePort reachability +## Sandbox Service type -The provisioner returns `http://{NODE_HOST}:{NodePort}` to the gateway so the -agent can reach its sandbox. In Docker Compose `NODE_HOST=host.docker.internal`; -in Kubernetes `NODE_HOST` **defaults to the provisioner pod's node IP** via the -[downward API](https://kubernetes.io/docs/concepts/workloads/pods/downward-api/) -(`status.hostIP`). Because a NodePort is exposed on every node, the gateway can -reach `:` on most clusters without any configuration. +The provisioner exposes each sandbox Pod behind a per-sandbox Service whose type +is controlled by `provisioner.sandboxServiceType` (default `ClusterIP`). -Override `provisioner.nodeHost` only if your CNI or network policy blocks -pod->node-IP traffic: +**`ClusterIP` (default, recommended).** The provisioner returns a cluster-DNS +URL - `http://sandbox--svc..svc.cluster.local:8080` - so the +gateway reaches its sandbox entirely inside the cluster network. No node IP, no +high port, and the code-execution surface is **not** bound on every node's +interfaces. This is correct for the chart, where the gateway always runs +in-cluster. + +**`NodePort` (Docker-Compose/hybrid escape hatch).** Set +`provisioner.sandboxServiceType: NodePort` only when the gateway is *not* in K8s +(e.g. the compose dev path, where the gateway is a container reaching sandbox +Pods on the host's Docker Desktop K8s). The provisioner then returns +`http://{NODE_HOST}:{NodePort}`. `NODE_HOST` defaults to the provisioner pod's +node IP via the [downward API](https://kubernetes.io/docs/concepts/workloads/pods/downward-api/) +(`status.hostIP`); because a NodePort is exposed on every node, the gateway can +reach `:` on most clusters without configuration. Override +`provisioner.nodeHost` only if your CNI or network policy blocks pod->node-IP +traffic: ```bash kubectl get nodes -o wide # use INTERNAL-IP or EXTERNAL-IP @@ -394,11 +405,13 @@ kubectl get nodes -o wide # use INTERNAL-IP or EXTERNAL-IP ```yaml provisioner: + sandboxServiceType: NodePort nodeHost: 192.168.x.x ``` On multi-node clusters, also switch `persistence.home.accessMode` to -`ReadWriteMany`. +`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). ## Lint / dry-run diff --git a/deploy/helm/deer-flow/templates/NOTES.txt b/deploy/helm/deer-flow/templates/NOTES.txt index 236a75e0b..2003abfc0 100644 --- a/deploy/helm/deer-flow/templates/NOTES.txt +++ b/deploy/helm/deer-flow/templates/NOTES.txt @@ -39,24 +39,33 @@ Provisioner / sandbox notes: • PVC mode is enabled: sandbox Pods mount the `{{ include "deer-flow.homePVC" . }}` PVC for per-thread user-data. - ⚠️ Sandbox NodePort reachability: - The provisioner returns `http://{NODE_HOST}:{NodePort}` to the gateway so - the agent can talk to its sandbox. NODE_HOST defaults to the provisioner - pod's node IP via the Kubernetes downward API, which routes on most clusters - because a NodePort is exposed on every node. - - If your CNI or network policy blocks pod->node-IP traffic, set - `provisioner.nodeHost` to an address the gateway can reach that routes to - a node IP: + • Sandbox reachability: per-sandbox Services are type + `{{ .Values.provisioner.sandboxServiceType | default "ClusterIP" }}`. +{{- if eq (default "ClusterIP" .Values.provisioner.sandboxServiceType) "NodePort" }} + The provisioner returns `http://{NODE_HOST}:{NodePort}` to the gateway. + NODE_HOST defaults to the provisioner pod's node IP via the downward API; a + NodePort is exposed on every node, so `:` routes from the + gateway on most clusters. If your CNI or network policy blocks pod->node-IP + traffic, set `provisioner.nodeHost` to an address the gateway can reach: kubectl get nodes -o wide provisioner: nodeHost: - On multi-node clusters you additionally need `persistence.home.accessMode: - ReadWriteMany` (e.g. NFS) so sandbox Pods on other nodes can mount the - shared user-data PVC. + ⚠️ NodePort binds the code-execution sandbox on every node's interfaces, + including externally-reachable ones. Prefer `ClusterIP` (the default) when + the gateway runs in-cluster. +{{- else }} + The provisioner returns a cluster-DNS URL + (`http://sandbox--svc.{{ include "deer-flow.namespace" . }}.svc.cluster.local:{{ .Values.provisioner.sandboxPort }}`) + so the gateway reaches its sandbox inside the cluster - no node IP, no + external port, no exposure on node interfaces. +{{- end }} + + • On multi-node clusters you additionally need + `persistence.home.accessMode: ReadWriteMany` (e.g. NFS) so sandbox Pods on + other nodes can mount the shared user-data PVC. Generated secrets (persisted across upgrades): diff --git a/deploy/helm/deer-flow/templates/provisioner-deployment.yaml b/deploy/helm/deer-flow/templates/provisioner-deployment.yaml index 013e1eef2..6c26bbd23 100644 --- a/deploy/helm/deer-flow/templates/provisioner-deployment.yaml +++ b/deploy/helm/deer-flow/templates/provisioner-deployment.yaml @@ -51,11 +51,18 @@ spec: value: {{ .Values.provisioner.sandboxImage | quote }} # In-cluster API access via the mounted ServiceAccount token. # K8S_API_SERVER is intentionally unset (do NOT use host.docker.internal). - # NODE_HOST: the address the gateway uses to reach sandbox NodePorts. - # When `provisioner.nodeHost` is set, use it; otherwise default to this - # pod's node IP via the downward API. A NodePort is exposed on every - # node, so : routes from the gateway on most clusters. - # Override only when pod->node-IP traffic is blocked by the CNI/policy. + # Service type for per-sandbox Services. ClusterIP (default) routes the + # gateway->sandbox hop over cluster DNS; NodePort is the compose/hybrid + # escape hatch (gateway not in K8s), which needs NODE_HOST below. + - name: SANDBOX_SERVICE_TYPE + value: {{ .Values.provisioner.sandboxServiceType | default "ClusterIP" | quote }} + {{- if eq (default "ClusterIP" .Values.provisioner.sandboxServiceType) "NodePort" }} + # NODE_HOST: address the gateway uses to reach sandbox NodePorts + # (NodePort mode only). When `provisioner.nodeHost` is set, use it; + # otherwise default to this pod's node IP via the downward API. A + # NodePort is exposed on every node, so : routes + # from the gateway on most clusters. Override only when pod->node-IP + # traffic is blocked by the CNI/policy. - name: NODE_HOST {{- if .Values.provisioner.nodeHost }} value: {{ .Values.provisioner.nodeHost | quote }} @@ -64,6 +71,7 @@ spec: fieldRef: fieldPath: status.hostIP {{- end }} + {{- end }} # PVC mode — sandbox Pods mount the same PVCs the gateway uses. {{- if .Values.persistence.home.enabled }} - name: USERDATA_PVC_NAME diff --git a/deploy/helm/deer-flow/values.yaml b/deploy/helm/deer-flow/values.yaml index a9b9ef5e0..80ed249b5 100644 --- a/deploy/helm/deer-flow/values.yaml +++ b/deploy/helm/deer-flow/values.yaml @@ -94,10 +94,17 @@ provisioner: memory: 512Mi # -- AIO-compatible sandbox container image used for sandbox Pods. sandboxImage: "enterprise-public-cn-beijing.cr.volces.com/vefaas-public/all-in-one-sandbox:latest" - # -- Host the gateway uses to reach sandbox NodePorts. Empty (default) falls - # back to the provisioner pod's node IP via the Kubernetes downward API, - # which routes on most clusters because a NodePort is exposed on every - # node. Set explicitly only when pod->node-IP traffic is blocked by your + # -- Service type for per-sandbox Services. `ClusterIP` (default) exposes + # sandboxes only inside the cluster via Service DNS - correct for the + # in-cluster gateway and avoids binding the code-execution surface on + # every node's interfaces. `NodePort` is the Docker-Compose/hybrid escape + # hatch (used when the gateway is not in K8s); pair with `nodeHost`. + sandboxServiceType: "ClusterIP" + # -- Host the gateway uses to reach sandbox NodePorts. Only consulted when + # `sandboxServiceType: NodePort`. Empty (default) falls back to the + # provisioner pod's node IP via the Kubernetes downward API, which routes + # on most clusters because a NodePort is exposed on every node. Set + # explicitly only when pod->node-IP traffic is blocked by your # CNI/network policy. In Docker Compose this is `host.docker.internal`. nodeHost: "" # -- Sandbox container port (must match the sandboxImage's listening port). diff --git a/scripts/check_chart_sandbox_service.sh b/scripts/check_chart_sandbox_service.sh new file mode 100755 index 000000000..239a829a3 --- /dev/null +++ b/scripts/check_chart_sandbox_service.sh @@ -0,0 +1,89 @@ +#!/usr/bin/env bash +# Assert the Helm chart emits the sandbox Service-type env correctly: +# - default (sandboxServiceType unset -> ClusterIP): +# SANDBOX_SERVICE_TYPE=ClusterIP, NODE_HOST absent +# - provisioner.sandboxServiceType=NodePort: +# SANDBOX_SERVICE_TYPE=NodePort, NODE_HOST present +# - NodePort + provisioner.nodeHost=: +# NODE_HOST takes the literal value (not the downward API) +# +# Locks in the gating added for #3929 so a future change - e.g. re-adding an +# unconditional NODE_HOST, or dropping the `default "ClusterIP"` fallback that +# keeps a stale values.yaml from sending an empty string the provisioner +# rejects - fails CI rather than silently regressing the sandbox exposure +# surface. The provisioner itself is mode-aware since #4016; this guards the +# chart wiring that selects the mode. +# +# Usage: +# scripts/check_chart_sandbox_service.sh +# +# Called by .github/workflows/chart.yaml (validate-chart, on PRs + v* tags). +# Requires `helm` (ubuntu-latest ships helm 3 preinstalled). +# +# Exit status is 0 when all assertions pass, 1 otherwise. + +set -uo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +CHART="$ROOT/deploy/helm/deer-flow" + +if ! command -v helm >/dev/null 2>&1; then + echo "::error::helm is required to run this check" >&2 + exit 1 +fi + +TMP="$(mktemp -d)" +trap 'rm -rf "$TMP"' EXIT + +if ! helm template deer-flow "$CHART" --include-crds >"$TMP/default.yaml"; then + echo "::error::default chart render failed" >&2 + exit 1 +fi +if ! helm template deer-flow "$CHART" --include-crds \ + --set provisioner.sandboxServiceType=NodePort >"$TMP/nodeport.yaml"; then + echo "::error::NodePort chart render failed" >&2 + exit 1 +fi +if ! helm template deer-flow "$CHART" --include-crds \ + --set provisioner.sandboxServiceType=NodePort \ + --set provisioner.nodeHost=192.168.1.10 >"$TMP/nodeport-host.yaml"; then + echo "::error::NodePort+nodeHost chart render failed" >&2 + exit 1 +fi + +# An env-var list item renders as ` - name: `. Matching the item (not the +# comments that merely mention the name) is what makes the NODE_HOST-absent +# assertion meaningful. +has_env() { grep -qE "^[[:space:]]*- name: $1\$" "$2"; } +env_value() { grep -A1 -E "^[[:space:]]*- name: $1\$" "$2" | grep -oE 'value: "[^"]*"' | head -1; } + +errors=0 +check() { # check <0|1> (0 == pass) + if [ "$1" -eq 0 ]; then + echo " PASS $2" + else + echo " FAIL $2" + errors=$((errors + 1)) + fi +} + +echo "## Default render (provisioner.sandboxServiceType unset -> ClusterIP)" +has_env SANDBOX_SERVICE_TYPE "$TMP/default.yaml"; check $? "SANDBOX_SERVICE_TYPE env present" +[ "$(env_value SANDBOX_SERVICE_TYPE "$TMP/default.yaml")" = 'value: "ClusterIP"' ]; check $? "SANDBOX_SERVICE_TYPE == ClusterIP" +if has_env NODE_HOST "$TMP/default.yaml"; then check 1 "NODE_HOST env absent"; else check 0 "NODE_HOST env absent"; fi + +echo "## NodePort opt-in (provisioner.sandboxServiceType=NodePort)" +has_env SANDBOX_SERVICE_TYPE "$TMP/nodeport.yaml"; check $? "SANDBOX_SERVICE_TYPE env present" +[ "$(env_value SANDBOX_SERVICE_TYPE "$TMP/nodeport.yaml")" = 'value: "NodePort"' ]; check $? "SANDBOX_SERVICE_TYPE == NodePort" +has_env NODE_HOST "$TMP/nodeport.yaml"; check $? "NODE_HOST env present" + +echo "## NodePort + provisioner.nodeHost=192.168.1.10" +[ "$(env_value NODE_HOST "$TMP/nodeport-host.yaml")" = 'value: "192.168.1.10"' ]; check $? "NODE_HOST == 192.168.1.10 (literal, not downward API)" + +echo +if [ "$errors" -eq 0 ]; then + echo "All sandbox Service-type render assertions passed." + exit 0 +fi +echo "::error::$errors assertion(s) failed (see above)" >&2 +exit 1