Get off Docker Hub, add a catalog values passthrough, fix the dind prune
ci / lint (push) Successful in 24s
ci / types (push) Successful in 34s
ci / unit (push) Successful in 26s
ci / security (push) Successful in 37s
ci / dockerfile (push) Successful in 6s
ci / chart (push) Successful in 7s
ci / integration (push) Successful in 41s
ci / image (api) (push) Successful in 2m9s
ci / image (reconciler) (push) Successful in 2m1s
ci / image (worker) (push) Successful in 2m7s
ci / bump (push) Successful in 13s
ci / lint (push) Successful in 24s
ci / types (push) Successful in 34s
ci / unit (push) Successful in 26s
ci / security (push) Successful in 37s
ci / dockerfile (push) Successful in 6s
ci / chart (push) Successful in 7s
ci / integration (push) Successful in 41s
ci / image (api) (push) Successful in 2m9s
ci / image (reconciler) (push) Successful in 2m1s
ci / image (worker) (push) Successful in 2m7s
ci / bump (push) Successful in 13s
Docker Hub rate-limits anonymous pulls per source IP and every node here shares one NAT address, so a busy afternoon fails an unrelated build with `toomanyrequests`. Nothing in this repo needs to be there. Every base image now comes from mirror.gcr.io (python, alpine/helm, postgres) or ghcr.io (uv, trivy). Verified digest-for-digest against Docker Hub before switching, including the superseded postgres digest this repo still pins, so every existing pin stays valid — same bytes, different transport. catalog.yaml: the three bitnami entries named `bitnamilegacy/<chart>`, a repo alias nothing in the worker image configures, so they could never resolve at provision time. All five entries are now `oci://` refs, which need no `helm repo add`, and all are on latest stable: elasticsearch 21.3.15 -> 22.1.6 redis 20.6.2 -> 27.0.15 postgresql 16.4.5 -> 18.8.0 podinfo 6.7.1 -> 6.14.0 Moving the chart pull is only half of it, though: a bitnami chart defaults its own images to registry-1.docker.io. CatalogEntry gains a `values:` dict, merged under the size's replicas and resources, so an entry can set `global.imageRegistry` and move the image pull too. Size wins on conflict — otherwise an entry setting replicaCount would make every size deploy the same shape. Deep merge, because a shallow one drops sibling keys of a shared nested map. Bitnami charts reject a substituted registry unless `global.security.allowInsecureImages` is set. That check is about provenance, and the mirror serves byte-identical manifests, so it is set deliberately and only for entries whose digests were verified. The dind prune had `--filter until=168h` on both prunes, and it got both cases exactly backwards. `until` reads an image's CREATED time, so it deleted trivy every leg (a released tool image is always older than any window) while protecting the dangling build layers it existed to remove. Measured on node0: 21 dangling images / 5.96GB, and exactly 1 of them older than 168h. Trivy is protected by a tag now, so the image prune drops the filter; buildx keeps it, where age genuinely matters. Tests: +10 unit (deep merge, precedence, no-mutation, and a guard that fails if any catalog entry points at Docker Hub). Both new guards were control-tested by breaking the code and watching them fail. The API test that hardcoded `21.3.15` now reads the catalog — its subject is where the value comes from, not what it is.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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"
|
||||
|
||||
@@ -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())
|
||||
Reference in New Issue
Block a user