From 4a426dbe50d2f273cc27be2b9b959d4d0dd59460 Mon Sep 17 00:00:00 2001 From: Nguyen Minh Phuc Date: Sat, 18 Jul 2026 13:05:22 +0000 Subject: [PATCH] worker: create namespaces with helm, drop the kubectl binary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit trivy took the worker image 39 -> 18 -> 5 findings across two version bumps, and the last 5 (4x golang.org/x/net, 1x Go stdlib) live in kubectl v1.36.2 — the newest kubectl that exists. No release clears them; upstream has not rebuilt against the patched Go yet. Chasing the version further has no end. kubectl was in that image for exactly one call: ensure_namespace before helm. `helm upgrade --install --create-namespace` does the same thing, idempotently, as part of the install it already runs. So the binary goes, and its vendored CVEs go with it. read_secret had no callers. Tradeoff recorded: the namespace no longer gets an svcforge.io/team label, since --create-namespace makes a bare one. Nothing reads that label today. --- ARCHITECTURE.md | 3 +-- .../svcforge_core/adapters/helm.py | 7 +++++++ services/worker/Dockerfile | 17 +---------------- services/worker/deps.py | 19 ------------------- services/worker/handlers.py | 9 --------- services/worker/main.py | 2 -- tests/integration/test_worker.py | 12 ------------ 7 files changed, 9 insertions(+), 60 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 9e36f2a..9b7b91c 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -128,8 +128,7 @@ sequenceDiagram Note over W,P: every 5s, while a semaphore slot is free W->>P: UPDATE ... FOR UPDATE SKIP LOCKED P-->>W: task (attempts now 1, locked_by=me) - W->>K: kubectl apply namespace (idempotent) - W->>K: helm upgrade --install --wait + W->>K: helm upgrade --install --create-namespace --wait K-->>W: release ready W->>P: CAS provisioning -> ready, set endpoint W->>P: complete(task, worker_id) diff --git a/libs/svcforge_core/svcforge_core/adapters/helm.py b/libs/svcforge_core/svcforge_core/adapters/helm.py index f98cd29..ab1ee80 100644 --- a/libs/svcforge_core/svcforge_core/adapters/helm.py +++ b/libs/svcforge_core/svcforge_core/adapters/helm.py @@ -199,6 +199,13 @@ class HelmProvisioner: entry.chart, "--namespace", ns, + # `--namespace X` does not create X. Every tenant's first provision targets + # a namespace that does not exist yet, and helm fails with "namespaces not + # found". helm creates it here rather than a separate `kubectl apply` step, + # which keeps kubectl out of the worker image entirely — one fewer binary, + # and one fewer set of vendored Go CVEs to track. Idempotent: existing + # namespaces are left alone. + "--create-namespace", "--version", entry.chart_version, "--values", diff --git a/services/worker/Dockerfile b/services/worker/Dockerfile index ed0ca6a..e545033 100644 --- a/services/worker/Dockerfile +++ b/services/worker/Dockerfile @@ -3,7 +3,7 @@ # svcforge worker. Build from the REPO ROOT: # docker buildx build -f services/worker/Dockerfile -t svcforge/worker:dev . # -# The only service that shells out to helm/kubectl, so the only one carrying those two +# The only service that shells out to helm, so the only one carrying that binary # binaries. They are copied from pinned images rather than curl'd, so the version is a # reviewable line in a Dockerfile instead of a network call at build time. @@ -46,21 +46,6 @@ COPY --from=builder --chown=10001:10001 /app /app # (spdystream, fixed in 0.5.1) — trivy fails the build on them and is right to. # Deliberately 3.x: helm 4 is a breaking change and is not a CVE fix. COPY --from=alpine/helm:3.21.3@sha256:35da09ba0716fc7c3cd63b6b31ee380a9c7662e95f29ab0e4ae962420afd315b /usr/bin/helm /usr/local/bin/helm -# kubectl 1.36.2 from the OFFICIAL registry.k8s.io image, replacing -# bitnamilegacy/kubectl:1.31.2. Two reasons, either sufficient: -# - CVEs: the bitnami image is Go 1.22.9 and ships containerd < 1.7.29 (HIGH -# CVE-2024-25621) along with the same crypto/tls and grpc CRITICALs as helm above. -# - Skew: the target cluster runs v1.35.3. 1.31.2 is four minors behind, well outside -# kubectl's supported +/-1 window, so it was unsupported against this cluster. -# bitnamilegacy publishes no 1.35 tag. registry.k8s.io/kubectl is the upstream-maintained -# image, is a manifest list with linux/arm64 (this cluster is Ampere), and puts the binary -# at /bin/kubectl rather than bitnami's /opt/bitnami path. -# -# 1.36.2 rather than 1.36.x-matching-the-cluster: k8s v1.35.x vendors -# github.com/moby/spdystream v0.5.0, which carries CVE-2026-35469 (HIGH, fixed in -# 0.5.1). v1.36.2 vendors 0.5.1. trivy blocks the release on it, correctly. One -# minor ahead of the v1.35.3 API server is inside kubectl's supported +/-1 window. -COPY --from=registry.k8s.io/kubectl:v1.36.2@sha256:b0d792e0d8dfb9bb1b922b78b23137e2a34bb6f9667640353a9d2aadd1fd7761 /bin/kubectl /usr/local/bin/kubectl ENV PATH="/app/.venv/bin:$PATH" \ PYTHONUNBUFFERED=1 \ diff --git a/services/worker/deps.py b/services/worker/deps.py index f37c8d0..83e48f7 100644 --- a/services/worker/deps.py +++ b/services/worker/deps.py @@ -8,7 +8,6 @@ run in milliseconds against a FakeProvisioner instead of needing a cluster. from __future__ import annotations from dataclasses import dataclass -from typing import Protocol from svcforge_core.adapters.clock import Clock from svcforge_core.adapters.helm import Provisioner @@ -20,20 +19,6 @@ from svcforge_core.repo.tasks import TaskRepo from svcforge_core.settings import Settings -class NamespaceEnsurer(Protocol): - """The one thing the worker needs from kubectl. - - Narrower than `KubectlClient` on purpose: the handler creates namespaces and does - nothing else with the cluster, so that is the whole interface. Satisfied by - `KubectlClient` in production and by a fake in tests, which is what keeps the worker - suite running in milliseconds with no cluster. - """ - - async def ensure_namespace(self, ns: str, labels: dict[str, str] | None = None) -> None: - """Create the namespace if absent. Idempotent — safe on every retry.""" - ... - - @dataclass(frozen=True) class WorkerDeps: """Everything a handler is allowed to touch.""" @@ -42,10 +27,6 @@ class WorkerDeps: instances: InstanceRepo tasks: TaskRepo provisioner: Provisioner - # Creates the tenant namespace before helm is pointed at it. `helm --namespace X` - # does NOT create X, so without this the very first provision for a new team fails - # with "namespace not found" — the one path that is guaranteed untested by a fake. - namespaces: NamespaceEnsurer notifier: Notifier clock: Clock catalog: dict[str, CatalogEntry] diff --git a/services/worker/handlers.py b/services/worker/handlers.py index 4190986..fce124b 100644 --- a/services/worker/handlers.py +++ b/services/worker/handlers.py @@ -63,15 +63,6 @@ async def handle_provision(task: Task, deps: WorkerDeps) -> None: # below is idempotent either way, so this is bookkeeping, not a lock. await deps.instances.update_state(inst.id, InstanceState.REQUESTED, InstanceState.PROVISIONING) - # `helm --namespace X` does not create X. Every tenant's first provision lands in a - # namespace that does not exist yet, so this has to happen before helm runs or the - # install fails with "namespaces not found". Idempotent (kubectl apply of a Namespace - # manifest), so it costs one no-op API call on every subsequent provision. - await deps.namespaces.ensure_namespace( - inst.namespace, - labels={"svcforge.io/team": inst.team}, - ) - await deps.provisioner.install( release=inst.release_name, ns=inst.namespace, diff --git a/services/worker/main.py b/services/worker/main.py index f0bfc9c..6ea063e 100644 --- a/services/worker/main.py +++ b/services/worker/main.py @@ -22,7 +22,6 @@ from services.worker.handlers import HANDLERS from svcforge_core import obs from svcforge_core.adapters.clock import SystemClock from svcforge_core.adapters.helm import HelmProvisioner -from svcforge_core.adapters.k8s import KubectlClient from svcforge_core.adapters.notify import LogNotifier from svcforge_core.domain.catalog import load_catalog from svcforge_core.domain.models import Task, TaskKind @@ -185,7 +184,6 @@ async def _amain() -> None: instances=InstanceRepo(pool), tasks=TaskRepo(pool), provisioner=HelmProvisioner(helm_bin=settings.helm_bin, timeout_s=int(settings.helm_timeout_s)), - namespaces=KubectlClient(kubectl_bin=settings.kubectl_bin), notifier=LogNotifier(), clock=SystemClock(), catalog=load_catalog(settings.catalog_path), diff --git a/tests/integration/test_worker.py b/tests/integration/test_worker.py index 2dd84af..72f726d 100644 --- a/tests/integration/test_worker.py +++ b/tests/integration/test_worker.py @@ -41,21 +41,10 @@ def _settings(**over: object) -> Settings: return Settings(**base) # type: ignore[arg-type] -class FakeNamespaceEnsurer: - """Records the namespaces it was asked to create. Satisfies NamespaceEnsurer.""" - - def __init__(self) -> None: - self.ensured: list[str] = [] - - async def ensure_namespace(self, ns: str, labels: dict[str, str] | None = None) -> None: - self.ensured.append(ns) - - def _deps( pool: DictPool, prov: FakeProvisioner, notifier: FakeNotifier | None = None, - namespaces: FakeNamespaceEnsurer | None = None, **over: object, ) -> WorkerDeps: return WorkerDeps( @@ -63,7 +52,6 @@ def _deps( instances=InstanceRepo(pool), tasks=TaskRepo(pool), provisioner=prov, - namespaces=namespaces or FakeNamespaceEnsurer(), notifier=notifier or FakeNotifier(), clock=SystemClock(), catalog=CATALOG,