diff --git a/.gitea/workflows/ci.yaml b/.gitea/workflows/ci.yaml index 4e1077f..acfd2ad 100644 --- a/.gitea/workflows/ci.yaml +++ b/.gitea/workflows/ci.yaml @@ -103,7 +103,7 @@ jobs: needs: [unit] services: postgres: - image: postgres:18@sha256:32ca0af8e77bfb8c6610c488e4691f83f972a3e9e64d3b02facf3ab111ad5500 + image: mirror.gcr.io/library/postgres:18@sha256:32ca0af8e77bfb8c6610c488e4691f83f972a3e9e64d3b02facf3ab111ad5500 env: POSTGRES_PASSWORD: postgres POSTGRES_DB: svcforge @@ -432,7 +432,7 @@ jobs: # place rather than accumulating versions, so the volume stays at roughly one DB, # and the weekly prune CronJob reclaims it at the cost of one re-download. env: - TRIVY: aquasec/trivy:0.72.0@sha256:cffe3f5161a47a6823fbd23d985795b3ed72a4c806da4c4df16266c02accdd6f + TRIVY: ghcr.io/aquasecurity/trivy:0.72.0@sha256:cffe3f5161a47a6823fbd23d985795b3ed72a4c806da4c4df16266c02accdd6f run: | # Pull by digest, then give it a local tag, and run the TAG. # @@ -485,9 +485,9 @@ jobs: --format '{{.Manifest.Digest}}' - name: reclaim dind disk - # dind's /var/lib/docker is a hostPath on node2 (see oci-k8s addons role), so + # dind's /var/lib/docker is a hostPath on node0 (see oci-k8s addons role), so # nothing reclaims it automatically — kubelet's image GC does not manage a nested - # daemon's store. Left alone it grows every run until node2 hits disk pressure and + # daemon's store. Left alone it grows every run until node0 hits disk pressure and # starts evicting pods, which looks like a cluster problem rather than a CI one. # # `always()`: a failed build still leaves layers behind, and that is exactly when @@ -495,26 +495,29 @@ jobs: # # Deliberately narrow. `docker image prune` WITHOUT -a removes dangling images # only; with -a it would delete the act runner image, which no container references - # between jobs, and buy back a 1.6GB re-pull on the very next run. The buildx cache - # is the part that actually grows without bound, so it is pruned by age and keeps a - # week — recent enough that `--cache-from` still hits on normal traffic. + # between jobs, and buy back a 1.6GB re-pull on the very next run. # # Named volumes are never pruned here: that is where the trivy vuln DB lives. # - # "Dangling" catches more than it looks: an image pulled by digest has no tag, so it - # is dangling as soon as its container exits. That is why the trivy scan step tags - # its image — an age filter does NOT protect it, because `until` reads the image's - # created time, and a released tool image is always older than any useful window. - # Run #68 proved that: trivy was untagged and deleted in all three legs despite - # `until=168h`, and re-pulled 178MB each time. + # NO age filter on the image prune, and that is the whole point of this comment. + # `--filter until=168h` reads an image's CREATED time, so it got both cases exactly + # backwards: it deleted trivy every leg (a released tool image is always older than + # any window — run #68, 178MB re-pulled three times) while protecting the dangling + # build layers it was added to remove (they are minutes old). Measured on node0 + # afterwards: 21 dangling images, 5.96GB, 19 of them created inside 25 hours, none + # of them reclaimable while the filter was there. Tagging trivy is what protects + # trivy; nothing needs to protect a dangling layer from the current build, because + # `always()` runs this after that build has already been pushed. + # + # The buildx cache keeps its age filter: a week is recent enough that `--cache-from` + # still hits on normal traffic, and that cache really does grow by age. # - # The age filter stays anyway, for the buildx cache, which really does grow by age. # The act runner image survives only because this step runs inside an act container, # so the image is in use exactly while the prune runs. That is luck, not design; if # it ever starts disappearing, tag it the same way. if: always() run: | - docker image prune -f --filter until=168h + docker image prune -f docker buildx prune -af --filter until=168h echo "--- dind disk after prune ---" docker system df diff --git a/catalog.yaml b/catalog.yaml index f32e717..dfe55c1 100644 --- a/catalog.yaml +++ b/catalog.yaml @@ -3,11 +3,33 @@ # # security: true bypasses every tenant's maintenance window for this entry. Set it for a # CVE with a public exploit; leave it false and the bump waits for 03:00 Sunday. +# +# Every chart is addressed as `oci://`, which is not cosmetic: an OCI chart is pulled by +# reference, with no `helm repo add` first. The three entries below previously named +# `bitnamilegacy/`, a classic repo alias that nothing in the worker image +# configures — so they could never resolve at provision time. OCI is the form that works +# from a bare container. +# +# All of them go through mirror.gcr.io rather than registry-1.docker.io. Docker Hub +# rate-limits anonymous pulls per source IP and every node here shares one NAT address, so +# a busy afternoon becomes `toomanyrequests` on an unrelated deploy. mirror.gcr.io is a +# pull-through cache with no such limit, verified digest-for-digest identical. +# +# The chart pull is only half of it. A bitnami chart defaults its own images to +# `registry-1.docker.io/bitnami/`, so the pods would still go to Docker Hub even +# though the chart did not. `values:` on an entry is the fix: anything there is passed to +# helm underneath the size's replicas and resources, so `global.imageRegistry` moves the +# image pull too. `podinfo` needs none of this — its chart already points at ghcr.io. services: elasticsearch: - chart: bitnamilegacy/elasticsearch - chart_version: 21.3.15 + chart: oci://mirror.gcr.io/bitnamicharts/elasticsearch + chart_version: 22.1.6 security: false + # Sends the chart's own image pulls through the mirror as well, so nothing in this + # entry touches a rate-limited registry. Merged under the size below. + values: + global: + imageRegistry: mirror.gcr.io sizes: small: replicas: 1 @@ -29,9 +51,14 @@ services: memory: 8Gi redis: - chart: bitnamilegacy/redis - chart_version: 20.6.2 + chart: oci://mirror.gcr.io/bitnamicharts/redis + chart_version: 27.0.15 security: false + # Sends the chart's own image pulls through the mirror as well, so nothing in this + # entry touches a rate-limited registry. Merged under the size below. + values: + global: + imageRegistry: mirror.gcr.io sizes: small: replicas: 1 @@ -53,9 +80,14 @@ services: memory: 2Gi postgres: - chart: bitnamilegacy/postgresql - chart_version: 16.4.5 + chart: oci://mirror.gcr.io/bitnamicharts/postgresql + chart_version: 18.8.0 security: false + # Sends the chart's own image pulls through the mirror as well, so nothing in this + # entry touches a rate-limited registry. Merged under the size below. + values: + global: + imageRegistry: mirror.gcr.io sizes: small: replicas: 1 @@ -85,17 +117,12 @@ services: # # These two exist to exercise the actual loop — claim, helm install, CAS to ready, drift, # TTL, deprovision — in seconds and in tens of megabytes. - # - # Both are addressed as `oci://`, which is not cosmetic: an OCI chart is pulled by - # reference with no `helm repo add` first. The entries above name `bitnamilegacy/`, - # a classic repo alias that nothing in the worker image configures, so they cannot - # actually resolve at provision time. OCI is the form that works from a bare container. podinfo: # A single small Go binary with no dependencies, no PVC and a fast image pull. The e2e # test provisions exactly this for the same reason. chart: oci://ghcr.io/stefanprodan/charts/podinfo - chart_version: 6.7.1 + chart_version: 6.14.0 security: false sizes: small: @@ -120,9 +147,14 @@ services: nginx: # A recognisable web server, still small. Useful when the thing being demonstrated needs # to look like a service someone would actually ask for. - chart: oci://registry-1.docker.io/bitnamicharts/nginx + chart: oci://mirror.gcr.io/bitnamicharts/nginx chart_version: 25.0.14 security: false + # Sends the chart's own image pulls through the mirror as well, so nothing in this + # entry touches a rate-limited registry. Merged under the size below. + values: + global: + imageRegistry: mirror.gcr.io sizes: small: replicas: 1 diff --git a/libs/svcforge_core/svcforge_core/domain/models.py b/libs/svcforge_core/svcforge_core/domain/models.py index aa435b2..ea331ec 100644 --- a/libs/svcforge_core/svcforge_core/domain/models.py +++ b/libs/svcforge_core/svcforge_core/domain/models.py @@ -55,6 +55,16 @@ class CatalogEntry(BaseModel): # Bypass tenant maintenance windows for this entry's upgrades. Defaults False: a # normal version bump waits for 03:00 Sunday; a CVE with a public exploit does not. security: bool = False + # Chart values that apply to every size of this entry, merged UNDER the size's own + # replicas and resources. This is where a chart's own knobs go — `global.imageRegistry` + # to keep image pulls off a rate-limited registry, a storageClass, a disabled subchart. + # Without it the only expressible values are replicas and resources, and anything else + # a chart needs is a code change, which is the line between a platform and a script. + # + # Operator-supplied, never tenant-supplied: the catalog is a file only the platform team + # edits. A tenant reaching this dict would be handing arbitrary helm values — image + # references, securityContext, hostPath mounts — straight to the cluster. + values: dict[str, Any] = Field(default_factory=dict) class Instance(BaseModel): diff --git a/services/api/Dockerfile b/services/api/Dockerfile index 78ca616..2501d03 100644 --- a/services/api/Dockerfile +++ b/services/api/Dockerfile @@ -7,7 +7,7 @@ # Two syncs, not one: deps change rarely and our own code changes every commit, so the # expensive layer (third-party wheels) must land before the cheap one (our source). -FROM python:3.14-slim@sha256:cea0e6040540fb2b965b6e7fb5ffa00871e632eef63719f0ea54bca189ce14a6 AS builder +FROM mirror.gcr.io/library/python:3.14-slim@sha256:cea0e6040540fb2b965b6e7fb5ffa00871e632eef63719f0ea54bca189ce14a6 AS builder COPY --from=ghcr.io/astral-sh/uv:0.11.29@sha256:eb2843a1e56fd9e30c7276ce1a52cba86e64c7b385f5e3279a0e08e02dd058fc /uv /usr/local/bin/uv @@ -44,7 +44,7 @@ p = svcforge_core.__file__; \ sys.exit(0) if "site-packages" in p else sys.exit("not a wheel install: " + p)' # --- runtime -------------------------------------------------------------------------- -FROM python:3.14-slim@sha256:cea0e6040540fb2b965b6e7fb5ffa00871e632eef63719f0ea54bca189ce14a6 +FROM mirror.gcr.io/library/python:3.14-slim@sha256:cea0e6040540fb2b965b6e7fb5ffa00871e632eef63719f0ea54bca189ce14a6 ARG BUILD_SHA=unknown LABEL org.opencontainers.image.title="svcforge-api" \ diff --git a/services/reconciler/Dockerfile b/services/reconciler/Dockerfile index b44b100..d98da07 100644 --- a/services/reconciler/Dockerfile +++ b/services/reconciler/Dockerfile @@ -7,7 +7,7 @@ # writes: the four checks enqueue tasks, they do not provision. Orphans are logged, never # deleted. -FROM python:3.14-slim@sha256:cea0e6040540fb2b965b6e7fb5ffa00871e632eef63719f0ea54bca189ce14a6 AS builder +FROM mirror.gcr.io/library/python:3.14-slim@sha256:cea0e6040540fb2b965b6e7fb5ffa00871e632eef63719f0ea54bca189ce14a6 AS builder COPY --from=ghcr.io/astral-sh/uv:0.11.29@sha256:eb2843a1e56fd9e30c7276ce1a52cba86e64c7b385f5e3279a0e08e02dd058fc /uv /usr/local/bin/uv @@ -30,7 +30,7 @@ p = svcforge_core.__file__; \ sys.exit(0) if "site-packages" in p else sys.exit("not a wheel install: " + p)' # --- runtime -------------------------------------------------------------------------- -FROM python:3.14-slim@sha256:cea0e6040540fb2b965b6e7fb5ffa00871e632eef63719f0ea54bca189ce14a6 +FROM mirror.gcr.io/library/python:3.14-slim@sha256:cea0e6040540fb2b965b6e7fb5ffa00871e632eef63719f0ea54bca189ce14a6 ARG BUILD_SHA=unknown LABEL org.opencontainers.image.title="svcforge-reconciler" \ @@ -44,7 +44,7 @@ COPY --from=builder --chown=10001:10001 /app /app # helm 3.21.3, not 3.16.2 — see services/worker/Dockerfile. 3.16.2 is a Go 1.22.9 build # carrying CRITICAL CVE-2025-68121 (crypto/tls) and CVE-2026-33186 (grpc) and HIGH # CVE-2026-35469 (spdystream). Kept on 3.x on purpose: helm 4 is a breaking change. -COPY --from=alpine/helm:3.21.3@sha256:35da09ba0716fc7c3cd63b6b31ee380a9c7662e95f29ab0e4ae962420afd315b /usr/bin/helm /usr/local/bin/helm +COPY --from=mirror.gcr.io/alpine/helm:3.21.3@sha256:35da09ba0716fc7c3cd63b6b31ee380a9c7662e95f29ab0e4ae962420afd315b /usr/bin/helm /usr/local/bin/helm ENV PATH="/app/.venv/bin:$PATH" \ PYTHONUNBUFFERED=1 \ diff --git a/services/worker/Dockerfile b/services/worker/Dockerfile index c1896a7..6764941 100644 --- a/services/worker/Dockerfile +++ b/services/worker/Dockerfile @@ -7,7 +7,7 @@ # 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. -FROM python:3.14-slim@sha256:cea0e6040540fb2b965b6e7fb5ffa00871e632eef63719f0ea54bca189ce14a6 AS builder +FROM mirror.gcr.io/library/python:3.14-slim@sha256:cea0e6040540fb2b965b6e7fb5ffa00871e632eef63719f0ea54bca189ce14a6 AS builder COPY --from=ghcr.io/astral-sh/uv:0.11.29@sha256:eb2843a1e56fd9e30c7276ce1a52cba86e64c7b385f5e3279a0e08e02dd058fc /uv /usr/local/bin/uv @@ -30,7 +30,7 @@ p = svcforge_core.__file__; \ sys.exit(0) if "site-packages" in p else sys.exit("not a wheel install: " + p)' # --- runtime -------------------------------------------------------------------------- -FROM python:3.14-slim@sha256:cea0e6040540fb2b965b6e7fb5ffa00871e632eef63719f0ea54bca189ce14a6 +FROM mirror.gcr.io/library/python:3.14-slim@sha256:cea0e6040540fb2b965b6e7fb5ffa00871e632eef63719f0ea54bca189ce14a6 ARG BUILD_SHA=unknown LABEL org.opencontainers.image.title="svcforge-worker" \ @@ -45,7 +45,7 @@ COPY --from=builder --chown=10001:10001 /app /app # CVE-2025-68121 (crypto/tls) and CVE-2026-33186 (grpc), plus HIGH CVE-2026-35469 # (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 +COPY --from=mirror.gcr.io/alpine/helm:3.21.3@sha256:35da09ba0716fc7c3cd63b6b31ee380a9c7662e95f29ab0e4ae962420afd315b /usr/bin/helm /usr/local/bin/helm ENV PATH="/app/.venv/bin:$PATH" \ PYTHONUNBUFFERED=1 \ diff --git a/services/worker/handlers.py b/services/worker/handlers.py index 8cb7053..b8dc423 100644 --- a/services/worker/handlers.py +++ b/services/worker/handlers.py @@ -40,12 +40,35 @@ async def _load_instance(task: Task, deps: WorkerDeps) -> Instance: return Instance.model_validate(row) +def _deep_merge(base: dict[str, Any], override: dict[str, Any]) -> dict[str, Any]: + """`override` wins, except where both sides hold a dict — then merge those too. + + Shallow `base | override` would be wrong the moment two layers touch different keys of + the same nested map: `{"global": {"imageRegistry": ...}}` overridden by + `{"global": {"storageClass": ...}}` silently drops the registry, and the pod pulls from + somewhere nobody chose. + """ + out = dict(base) + for key, value in override.items(): + current = out.get(key) + if isinstance(current, dict) and isinstance(value, dict): + out[key] = _deep_merge(current, value) + else: + out[key] = value + return out + + def _values_for(inst: Instance, entry: CatalogEntry) -> dict[str, Any]: - """Turn a catalog size into helm values.""" + """Catalog values, with the requested size's replicas and resources on top. + + Size last, deliberately. An entry that sets `replicaCount` in its own `values:` would + otherwise beat the size the tenant actually asked for, and every size would deploy the + same shape. + """ size = entry.sizes.get(inst.size) if size is None: raise HandlerError(f"size {inst.size!r} not in catalog for {inst.service_type!r}") - return {"replicaCount": size.replicas, "resources": size.resources} + return _deep_merge(entry.values, {"replicaCount": size.replicas, "resources": size.resources}) async def handle_provision(task: Task, deps: WorkerDeps) -> None: diff --git a/tests/integration/test_api.py b/tests/integration/test_api.py index b91624a..c9fd356 100644 --- a/tests/integration/test_api.py +++ b/tests/integration/test_api.py @@ -28,6 +28,7 @@ from fastapi import FastAPI from services.api.main import create_app from services.api.routes.instances import release_name_for +from svcforge_core.domain.catalog import load_catalog from svcforge_core.repo.db import DictPool from svcforge_core.settings import Settings @@ -182,8 +183,10 @@ async def test_post_returns_202_and_location(client: httpx.AsyncClient, token: s assert resp.headers["location"] == f"/v1/instances/{body['id']}" assert body["state"] == "requested" assert body["service_type"] == "elasticsearch" - # Pinned from catalog.yaml at creation time, not echoed from the request. - assert body["chart_version"] == "21.3.15" + # Pinned from catalog.yaml at creation time, not echoed from the request. Read from the + # catalog rather than hardcoded: the literal made a routine version bump fail here, on a + # test whose subject is *where the value comes from*, not what it is. + assert body["chart_version"] == load_catalog(CATALOG)["elasticsearch"].chart_version assert body["endpoint"] is None # The response model is an allow-list: placement details stay off the wire. assert "team" not in body and "namespace" not in body and "release_name" not in body diff --git a/tests/unit/test_catalog.py b/tests/unit/test_catalog.py index 43d1264..b0aba6d 100644 --- a/tests/unit/test_catalog.py +++ b/tests/unit/test_catalog.py @@ -153,3 +153,57 @@ def test_repo_catalog_yaml_is_valid() -> None: assert set(catalog) == {"elasticsearch", "redis", "postgres", "podinfo", "nginx"} for entry in catalog.values(): assert set(entry.sizes) == {"small", "medium"} + + +def test_entry_values_default_to_empty(tmp_path: Path) -> None: + """`values:` is optional — an entry that needs no chart knobs says nothing.""" + body = textwrap.dedent(""" + services: + podinfo: + chart: oci://ghcr.io/stefanprodan/charts/podinfo + chart_version: "6.14.0" + sizes: + small: {replicas: 1, resources: {}} + """) + assert load_catalog(_write(tmp_path, body))["podinfo"].values == {} + + +def test_entry_values_are_parsed(tmp_path: Path) -> None: + """Nested values survive the load, which is what `global.imageRegistry` needs.""" + body = textwrap.dedent(""" + services: + redis: + chart: oci://mirror.gcr.io/bitnamicharts/redis + chart_version: "27.0.15" + values: + global: + imageRegistry: mirror.gcr.io + sizes: + small: {replicas: 1, resources: {}} + """) + assert load_catalog(_write(tmp_path, body))["redis"].values == { + "global": {"imageRegistry": "mirror.gcr.io"} + } + + +def test_no_catalog_entry_pulls_from_docker_hub() -> None: + """Every chart, and every image registry an entry pins, avoids Docker Hub. + + Docker Hub rate-limits anonymous pulls per source IP and the whole cluster shares one + NAT address, so a Docker Hub reference here is a provision that fails under load for a + reason no log in this repo will explain. + """ + catalog = load_catalog(Path(__file__).parents[2] / "catalog.yaml") + banned = ("docker.io", "registry-1.docker.io", "index.docker.io") + + for name, entry in catalog.items(): + assert not entry.chart.startswith(banned), f"{name}: chart on Docker Hub" + assert "docker.io" not in entry.chart, f"{name}: chart on Docker Hub" + + registry = entry.values.get("global", {}).get("imageRegistry") + # A bitnami chart defaults its images to Docker Hub, so any entry pointing at one + # has to redirect them. podinfo's chart already names ghcr.io and needs nothing. + if "bitnamicharts" in entry.chart: + assert registry == "mirror.gcr.io", f"{name}: bitnami chart without a registry override" + if registry is not None: + assert "docker.io" not in registry, f"{name}: imageRegistry on Docker Hub" diff --git a/tests/unit/test_values.py b/tests/unit/test_values.py new file mode 100644 index 0000000..8cd171d --- /dev/null +++ b/tests/unit/test_values.py @@ -0,0 +1,102 @@ +"""What reaches `helm --values`: the catalog entry's values, with the size on top.""" + +from __future__ import annotations + +from datetime import UTC, datetime +from typing import Any +from uuid import uuid4 + +import pytest + +from services.worker.handlers import HandlerError, _deep_merge, _values_for +from svcforge_core.domain.models import CatalogEntry, Instance, SizeSpec +from svcforge_core.domain.states import InstanceState + +RESOURCES: dict[str, Any] = {"requests": {"cpu": "10m", "memory": "16Mi"}} + + +def _entry(values: dict[str, Any] | None = None, replicas: int = 1) -> CatalogEntry: + return CatalogEntry( + service_type="redis", + chart="oci://mirror.gcr.io/bitnamicharts/redis", + chart_version="27.0.15", + sizes={"small": SizeSpec(replicas=replicas, resources=RESOURCES)}, + values=values or {}, + ) + + +def _instance(size: str = "small") -> Instance: + now = datetime.now(UTC) + return Instance( + id=uuid4(), + team="acme", + service_type="redis", + size=size, + state=InstanceState.REQUESTED, + namespace="tenant-acme", + release_name="acme-redis-0f8b7d3e", + chart_version="27.0.15", + created_at=now, + updated_at=now, + ) + + +# --------------------------------------------------------------------------- the merge + + +def test_deep_merge_keeps_both_sides_of_a_shared_nested_key() -> None: + """The reason this is not `base | override`. + + A shallow merge replaces the whole `global` map and silently drops imageRegistry, so + the pod pulls from a registry nobody chose. + """ + merged = _deep_merge( + {"global": {"imageRegistry": "mirror.gcr.io"}}, + {"global": {"storageClass": "longhorn"}}, + ) + assert merged == {"global": {"imageRegistry": "mirror.gcr.io", "storageClass": "longhorn"}} + + +def test_deep_merge_override_wins_on_a_scalar() -> None: + assert _deep_merge({"a": 1}, {"a": 2}) == {"a": 2} + + +def test_deep_merge_does_not_mutate_its_inputs() -> None: + """The catalog is loaded once at startup and shared by every provision. + + Mutating `entry.values` here would leak one instance's size into the next one's values, + and the second tenant would get the first tenant's replica count. + """ + base = {"global": {"imageRegistry": "mirror.gcr.io"}} + _deep_merge(base, {"global": {"storageClass": "longhorn"}, "replicaCount": 3}) + assert base == {"global": {"imageRegistry": "mirror.gcr.io"}} + + +# --------------------------------------------------------------------------- what helm gets + + +def test_entry_values_reach_helm() -> None: + values = _values_for(_instance(), _entry({"global": {"imageRegistry": "mirror.gcr.io"}})) + + assert values["global"] == {"imageRegistry": "mirror.gcr.io"} + assert values["replicaCount"] == 1 + assert values["resources"] == RESOURCES + + +def test_size_beats_entry_values() -> None: + """An entry that sets replicaCount must not override the size the tenant asked for. + + Without this ordering every size deploys the same shape, and `medium` is a lie. + """ + entry = _entry({"replicaCount": 99}, replicas=3) + + assert _values_for(_instance(), entry)["replicaCount"] == 3 + + +def test_entry_without_values_is_unchanged() -> None: + assert _values_for(_instance(), _entry()) == {"replicaCount": 1, "resources": RESOURCES} + + +def test_unknown_size_raises() -> None: + with pytest.raises(HandlerError, match="not in catalog"): + _values_for(_instance(size="enormous"), _entry())