From 5a7898c292f711d9a50cd8f19583add785164e55 Mon Sep 17 00:00:00 2001 From: Nick's Hermes <1572453+Nickfost@users.noreply.github.com> Date: Wed, 26 Aug 2026 14:32:49 -0500 Subject: [PATCH 1/8] feat: define Docker network capacity policy --- docs/DESIRED-STATE.md | 18 ++- docs/HEALTH-MONITORING.md | 12 ++ docs/STATUS-REPORTING.md | 4 +- schemas/status-report-v1.json | 14 +- scripts/desired_state.py | 68 ++++++++++ scripts/health.py | 122 ++++++++++++++++++ scripts/test_desired_state.py | 34 +++++ scripts/test_health.py | 70 ++++++++++ templates/config-repository/README.md | 16 ++- .../examples/multi-host/fleet.json | 8 ++ templates/config-repository/fleet.json | 9 ++ templates/config-repository/fleet.schema.json | 23 +++- templates/config-repository/scripts/init.py | 6 + .../config-repository/scripts/test_policy.py | 39 ++++++ .../config-repository/scripts/validate.py | 66 ++++++++++ 15 files changed, 504 insertions(+), 5 deletions(-) diff --git a/docs/DESIRED-STATE.md b/docs/DESIRED-STATE.md index dca40f5c..fec16eaf 100644 --- a/docs/DESIRED-STATE.md +++ b/docs/DESIRED-STATE.md @@ -32,10 +32,26 @@ Each controller has a unique object key and declares: - `experimental`, `stable`, or `retiring` lifecycle; - a full pinned ci-fleet engine commit; - a zero managed minimum and reviewed maximum runner capacity; -- CPU cores and memory per ephemeral runner. +- CPU cores and memory per ephemeral runner; +- reviewed Docker default-address pools and a reserved subnet count. Active and drained controllers reserve their configured maximum against the pool budget. A drained controller has zero effective runtime capacity but keeps its reservation, so an undrain cannot silently overcommit the pool. Disabled controllers reserve no capacity. +The Docker network policy uses IPv4 CIDR `base` values and a Docker subnet +prefix `size`. Validation rejects malformed or overlapping pools, impossible +prefix relationships, and active or drained policies with fewer subnets than +`max_runners + reserve_subnets`. Disabled controllers retain a structurally +valid policy but do not reserve runner subnet capacity. Real pool values belong +only in the private desired-state repository; public examples use RFC 5737 +documentation ranges. + +This phase renders the policy solely for read-only health inspection. It does +not write `daemon.json`, restart Docker, create or remove networks, prune +resources, drain runners, or alter controller scale. Circuit breaking, +frequent orphan reconciliation, daemon configuration rollout, transactional +exhausted-pool recovery, and MTFM/TF2 consumer-label changes remain later +issue #81 slices. + Managed prewarmed runners are not currently supported: `min_runners` is fixed at zero in schema, semantic validation, rendering, and preflight. This keeps idle privileged workers absent and prevents reviewed configuration from passing validation only to fail host adoption. The authoritative fictional contract is in [`templates/config-repository`](../templates/config-repository/README.md). diff --git a/docs/HEALTH-MONITORING.md b/docs/HEALTH-MONITORING.md index 5791750f..7f188467 100644 --- a/docs/HEALTH-MONITORING.md +++ b/docs/HEALTH-MONITORING.md @@ -17,6 +17,7 @@ The check covers: - root and Docker filesystem space and inodes; - available memory, swap use, per-CPU load, and OOM evidence from the last 24 hours; - Docker availability, controller state/restarts, and configured versus effective capacity; +- configured/used/free Docker subnet headroom and legacy networks, without exposing addresses; - inactive, unhealthy, restarting, and stale fleet-labelled resources, including week-old build cache; - cleanup, drift, health, and update services/timers; - failed package state, pending reboot, and clock synchronization; @@ -25,6 +26,17 @@ The check covers: It reports but never prunes, restarts, or repairs resources. Project source, logs, environment values, tokens, and private keys are never included. +Docker network inspection is read-only. Healthy headroom is reported when free +subnets remain above the reviewed reserve, low water and legacy/nonconforming +networks are warnings, and exhaustion is critical. A malformed policy or failed +Docker network listing/inspection is critical rather than falsely healthy. +Only aggregate configured, used, free, and legacy counts enter status reports. + +This detection-only phase does not mutate the Docker daemon or networks. +Controller circuit breaking, frequent orphan reconciliation, daemon policy +application, transactional recovery, cleanup, and consumer-label migrations +remain later issue #81 work. + ## Threshold overrides and hooks Defaults are intentionally conservative: disk and inode warning/critical at 80/90%, available memory warning/critical at 15/8%, sustained swap use under five-minute memory pressure warning/critical at 25/50%, per-CPU fifteen-minute load warning/critical at 1.0/1.5, and controller restart warning at 3. diff --git a/docs/STATUS-REPORTING.md b/docs/STATUS-REPORTING.md index 69b2d8b5..d180e0d5 100644 --- a/docs/STATUS-REPORTING.md +++ b/docs/STATUS-REPORTING.md @@ -24,12 +24,14 @@ The machine-readable contract is `schemas/status-report-v1.json`. It reports: - reconciliation, drift, health, and cleanup timer states; - current, busy, and configured-maximum runner counts; - CPU use, logical CPU count, memory, swap, root/Docker disk and inode use, and 1/5/15-minute load; -- Docker availability and OOM evidence; +- Docker availability, OOM evidence, and aggregate configured/used/free/legacy subnet counts; - one controlled error code/message, report generation time, and schema version. All times are Unix seconds. Commit values are empty when unavailable. Receiver validation rejects unknown fields and unsupported schema versions rather than guessing at compatibility. `error.message` is derived only from a controlled error code (`_` becomes a space). Raw exception text is never transmitted. +Docker pool prefixes and network addresses are intentionally absent from the +status contract; they remain in private desired state and host-local inspection. ## Authentication diff --git a/schemas/status-report-v1.json b/schemas/status-report-v1.json index d5a12ec6..335f1f4a 100644 --- a/schemas/status-report-v1.json +++ b/schemas/status-report-v1.json @@ -102,7 +102,19 @@ }, "docker": { "type": "object", "additionalProperties": false, "required": ["healthy", "oom"], - "properties": {"healthy": {"type": "boolean"}, "oom": {"type": "boolean"}} + "properties": { + "healthy": {"type": "boolean"}, + "oom": {"type": "boolean"}, + "network": { + "type": "object", "additionalProperties": false, "required": ["configured", "used", "free", "legacy"], + "properties": { + "configured": {"type": "integer", "minimum": 0}, + "used": {"type": "integer", "minimum": 0}, + "free": {"type": "integer", "minimum": 0}, + "legacy": {"type": "integer", "minimum": 0} + } + } + } }, "error": { "oneOf": [ diff --git a/scripts/desired_state.py b/scripts/desired_state.py index f19af22e..d4e394ca 100755 --- a/scripts/desired_state.py +++ b/scripts/desired_state.py @@ -5,6 +5,7 @@ import argparse import importlib.util +import ipaddress import json import os import re @@ -137,6 +138,58 @@ def validate_host_values(values: dict[str, str]) -> dict[str, str]: } +def validate_docker_network_policy(policy: dict[str, Any], *, path: str, max_runners: int) -> tuple[int, int, list[dict[str, Any]]]: + if not isinstance(policy, dict): + raise DesiredStateError(f"{path}: must be an object") + required = {"default_address_pools", "reserve_subnets"} + if set(policy) != required: + unknown = sorted(set(policy) - required) + missing = sorted(required - set(policy)) + messages: list[str] = [] + if missing: + messages.append(f"missing keys: {', '.join(missing)}") + if unknown: + messages.append(f"unknown keys: {', '.join(unknown)}") + raise DesiredStateError(f"{path}: " + "; ".join(messages)) + reserve = policy.get("reserve_subnets") + if type(reserve) is not int or reserve < 1: + raise DesiredStateError(f"{path}.reserve_subnets: must be a positive integer") + pools = policy.get("default_address_pools") + if type(pools) is not list or not pools: + raise DesiredStateError(f"{path}.default_address_pools: must be a non-empty list") + parsed: list[dict[str, Any]] = [] + for index, pool in enumerate(pools): + pool_path = f"{path}.default_address_pools[{index}]" + if not isinstance(pool, dict) or set(pool) != {"base", "size"}: + raise DesiredStateError(f"{pool_path}: must contain only base and size") + base = pool.get("base") + size = pool.get("size") + if not isinstance(base, str): + raise DesiredStateError(f"{pool_path}.base: must be a CIDR prefix") + if type(size) is not int or size < 0 or size > 32: + raise DesiredStateError(f"{pool_path}.size: must be an IPv4 prefix length between 0 and 32") + try: + network = ipaddress.ip_network(base, strict=True) + except ValueError as exc: + raise DesiredStateError(f"{pool_path}.base: malformed IPv4 prefix") from exc + if network.version != 4: + raise DesiredStateError(f"{pool_path}.base: malformed IPv4 prefix") + if size < network.prefixlen: + raise DesiredStateError(f"{pool_path}.size: impossible subnet count for {base}") + parsed.append({"base": base, "network": network, "size": size}) + for left, item in enumerate(parsed): + for right in range(left + 1, len(parsed)): + other = parsed[right] + if item["network"].overlaps(other["network"]): + raise DesiredStateError( + f"{path}.default_address_pools[{left}].base: overlaps configured pool {right}" + ) + configured = sum(1 << (item["size"] - item["network"].prefixlen) for item in parsed) + if configured < max_runners + reserve: + raise DesiredStateError(f"{path}: policy cannot satisfy max_runners plus reserve") + return configured, reserve, parsed + + def select_controller(config: dict[str, Any], controller_id: str) -> tuple[dict[str, Any], dict[str, Any]]: controllers = config["controllers"] if controller_id not in controllers: @@ -169,6 +222,12 @@ def build_rendered_env( state = controller["state"] configured_max = controller["max_runners"] effective_max = configured_max if state == "active" else 0 + network_policy = controller.get("docker_network_policy") or {} + configured_subnets, reserve_subnets, parsed_pools = validate_docker_network_policy( + network_policy, + path=f"$.controllers.{controller_id}.docker_network_policy", + max_runners=configured_max if state != "disabled" else 0, + ) short_commit = engine_commit[:12] rendered = { "CI_FLEET_CAPACITY_BUDGET": str(pool["capacity_budget"]), @@ -179,6 +238,8 @@ def build_rendered_env( "CI_FLEET_CONTROLLER_IMAGE": f"ci-fleet-controller:{short_commit}", "CI_FLEET_CONTROLLER_STATE": state, "CI_FLEET_DESIRED_STATE_SCHEMA": "3", + "CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_COUNT": str(len(parsed_pools)), + "CI_FLEET_DOCKER_NETWORK_RESERVE_SUBNETS": str(reserve_subnets), "CI_FLEET_DOCKER_GID": str(docker_gid), "CI_FLEET_ENGINE_REF": engine_commit, "CI_FLEET_GITHUB_URL": f"https://github.com/{config['organization']['slug']}", @@ -194,6 +255,9 @@ def build_rendered_env( "CI_FLEET_VERSION": short_commit, **validate_host_values(host_values), } + for index, pool_config in enumerate(parsed_pools): + rendered[f"CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_{index}_BASE"] = pool_config["base"] + rendered[f"CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_{index}_SIZE"] = str(pool_config["size"]) reporting_configured = "status_reporting" in controller reporting_required = (controller.get("status_reporting") or {}).get("enabled") is True if reporting_required and REQUIRED_STATUS_CAPABILITY not in (engine_capabilities or set()): @@ -222,6 +286,10 @@ def build_rendered_env( "engine_repository": config["organization"]["delivery_engine"], "status_reporting_configured": reporting_configured, "status_reporting_required": reporting_required, + "docker_network_policy_configured": True, + "docker_network_default_address_pools": len(parsed_pools), + "docker_network_reserve_subnets": reserve_subnets, + "docker_network_configured_subnets": configured_subnets, } return rendered, metadata diff --git a/scripts/health.py b/scripts/health.py index f47b9e1d..ce1d9a70 100644 --- a/scripts/health.py +++ b/scripts/health.py @@ -3,6 +3,7 @@ import argparse import http.client +import ipaddress import json import os import re @@ -133,6 +134,21 @@ def add(check_id: str, severity: str, **details: Any) -> None: add("swap", "critical" if swap >= thresholds.swap_critical_percent else "warning" if swap >= thresholds.swap_warn_percent else "ok", used_percent=swap) add("oom", "critical" if snapshot["recent_oom"] or snapshot["controller"]["oom_killed"] else "ok") add("docker", "ok" if snapshot["docker_available"] else "critical") + network = snapshot.get("docker_network_headroom") + if network and network.get("state") != "not_configured": + if network.get("state") == "unavailable": + add("docker_network_inspection", "critical") + else: + severity = {"healthy": "ok", "warning": "warning", "critical": "critical"}.get(network.get("state"), "critical") + add( + "docker_network_headroom", + severity, + configured=network.get("configured", 0), + used=network.get("used", 0), + free=network.get("free", 0), + reserve=network.get("reserve", 0), + ) + add("docker_network_legacy", "warning" if network.get("legacy", 0) else "ok", count=network.get("legacy", 0)) desired = snapshot["desired_state"] controller_state = snapshot["controller"]["state"] @@ -252,6 +268,10 @@ def build_status_report(snapshot: dict[str, Any], health_report: dict[str, Any], "docker": { "healthy": bool(snapshot.get("docker_available")), "oom": bool(snapshot.get("recent_oom") or snapshot["controller"].get("oom_killed")), + "network": { + key: snapshot.get("docker_network_headroom", {}).get(key, 0) + for key in ("configured", "used", "free", "legacy") + }, }, "error": error, "generated_at": generated_at, @@ -301,6 +321,106 @@ def _stale_resources(run: Runner, instance: str) -> dict[str, int]: return stale +def _parse_network_pool(values: dict[str, str], index: int) -> ipaddress.IPv4Network | None: + base = values.get(f"CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_{index}_BASE") + size = values.get(f"CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_{index}_SIZE") + if not base or not size: + return None + try: + network = ipaddress.ip_network(base, strict=True) + except ValueError: + return None + if network.version != 4: + return None + try: + subnet_size = int(size) + except ValueError: + return None + if subnet_size < network.prefixlen or subnet_size > 32: + return None + return network + + +def _docker_network_headroom(run: Runner, values: dict[str, str], *, docker_ok: bool) -> dict[str, Any]: + empty = {"configured": 0, "used": 0, "free": 0, "reserve": 0, "legacy": 0, "state": "unavailable"} + if not docker_ok: + return empty + try: + configured_count = int(values.get("CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_COUNT", "0")) + reserve = int(values.get("CI_FLEET_DOCKER_NETWORK_RESERVE_SUBNETS", "0")) + except ValueError: + return empty + if configured_count == 0 and "CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_COUNT" not in values: + return {**empty, "state": "not_configured"} + pools: list[ipaddress.IPv4Network] = [] + for index in range(configured_count): + pool = _parse_network_pool(values, index) + if pool is None: + return empty + pools.append(pool) + if not pools or reserve < 1: + return empty + listed = run(["docker", "network", "ls", "--format", "{{.Name}}"]) + if listed.returncode != 0: + return empty + configured = sum(1 << (int(values.get(f"CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_{index}_SIZE", "0")) - pools[index].prefixlen) for index in range(len(pools))) + used_subnets: set[str] = set() + legacy_networks = 0 + for name in [line.strip() for line in listed.stdout.splitlines() if line.strip()]: + inspected = run(["docker", "network", "inspect", name]) + if inspected.returncode != 0: + return empty + try: + payload = json.loads(inspected.stdout) + except json.JSONDecodeError: + return empty + if not isinstance(payload, list) or not payload: + return empty + subnets: list[ipaddress.IPv4Network] = [] + for entry in payload: + configs = entry.get("IPAM", {}).get("Config", []) if isinstance(entry, dict) else [] + if not isinstance(configs, list): + return empty + for config in configs: + subnet = config.get("Subnet") if isinstance(config, dict) else None + if not isinstance(subnet, str): + return empty + try: + network = ipaddress.ip_network(subnet, strict=False) + except ValueError: + return empty + if network.version != 4: + return empty + subnets.append(network) + if not subnets: + legacy_networks += 1 + continue + network_legacy = False + for subnet in subnets: + if any(subnet.subnet_of(pool) for pool in pools): + used_subnets.add(str(subnet)) + else: + network_legacy = True + if network_legacy: + legacy_networks += 1 + used = len(used_subnets) + free = max(configured - used, 0) + if free == 0: + state = "critical" + elif legacy_networks > 0 or free <= reserve: + state = "warning" + else: + state = "healthy" + return { + "configured": configured, + "used": used, + "free": free, + "reserve": reserve, + "legacy": legacy_networks, + "state": state, + } + + def _timespan_seconds(value: str) -> float | None: units = {"y": 365.25 * 86400, "month": 365.25 * 86400 / 12, "w": 7 * 86400, "d": 86400, "h": 3600, "min": 60, "s": 1, "ms": 0.001, "us": 0.000001, "µs": 0.000001, "ns": 0.000000001} matches = list(re.finditer(r"([0-9]+(?:\.[0-9]+)?)(month|min|ms|us|µs|ns|y|w|d|h|s)", value)) @@ -523,6 +643,7 @@ def collect_snapshot(values: dict[str, str], *, root: Path = Path("/"), run: Run stale = _stale_resources(run, instance) if docker_ok else {"containers": 0, "networks": 0, "volumes": 0} stale["images"] = _count(run, ["docker", "images", "-q", "--filter", "dangling=true", "--filter", "label=io.randomdevelopment.ci-fleet.managed=true"]) if docker_ok else 0 stale["build_cache"] = _count(run, ["docker", "buildx", "du", "--filter", "until=168h", "--format", "json"]) if docker_ok else 0 + docker_network_headroom = _docker_network_headroom(run, values, docker_ok=docker_ok) return { "controller_id": instance, "desired_state": values.get("CI_FLEET_CONTROLLER_STATE", "active"), @@ -553,6 +674,7 @@ def collect_snapshot(values: dict[str, str], *, root: Path = Path("/"), run: Run "clock_synchronized": run(["timedatectl", "show", "--property=NTPSynchronized", "--value"]).stdout.strip() == "yes", "backup": _backup_state(values, run), "reconciliation": reconciliation, + "docker_network_headroom": docker_network_headroom, } diff --git a/scripts/test_desired_state.py b/scripts/test_desired_state.py index c37a441f..5dc96796 100755 --- a/scripts/test_desired_state.py +++ b/scripts/test_desired_state.py @@ -36,6 +36,15 @@ def host_values() -> dict[str, str]: } +def docker_network_policy() -> dict: + return { + "reserve_subnets": 1, + "default_address_pools": [ + {"base": "198.51.100.0/24", "size": 28}, + ], + } + + class DesiredStateTests(unittest.TestCase): def render(self, value: dict | None = None, capabilities: set[str] | None = None): return build_rendered_env( @@ -118,6 +127,31 @@ def test_disabled_status_reporting_requires_schema_capability(self) -> None: self.assertTrue(metadata["status_reporting_configured"]) self.assertFalse(metadata["status_reporting_required"]) + def test_docker_network_policy_renders_read_only_inspection_values(self) -> None: + value = config() + value["controllers"]["example-ci-01"]["docker_network_policy"] = docker_network_policy() + environment, metadata = self.render(value) + self.assertEqual(environment["CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_COUNT"], "1") + self.assertEqual(environment["CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_0_BASE"], "198.51.100.0/24") + self.assertEqual(environment["CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_0_SIZE"], "28") + self.assertEqual(environment["CI_FLEET_DOCKER_NETWORK_RESERVE_SUBNETS"], "1") + self.assertTrue(metadata["docker_network_policy_configured"]) + self.assertEqual(metadata["docker_network_default_address_pools"], 1) + self.assertEqual(metadata["docker_network_reserve_subnets"], 1) + + def test_docker_network_policy_requires_capacity_for_reserve(self) -> None: + value = config() + value["controllers"]["example-ci-01"]["max_runners"] = 2 + value["controllers"]["example-ci-01"]["docker_network_policy"] = { + "reserve_subnets": 1, + "default_address_pools": [{"base": "198.51.100.0/30", "size": 30}], + } + with tempfile.TemporaryDirectory() as directory: + path = Path(directory) / "fleet.json" + path.write_text(json.dumps(value), encoding="utf-8") + with self.assertRaisesRegex(DesiredStateError, "capacity"): + load_and_validate_config(path) + def test_drained_controller_renders_zero_effective_capacity(self) -> None: value = config() value["controllers"]["example-ci-01"]["state"] = "drained" diff --git a/scripts/test_health.py b/scripts/test_health.py index ed4cf561..80a18be7 100644 --- a/scripts/test_health.py +++ b/scripts/test_health.py @@ -39,6 +39,15 @@ def healthy_snapshot(): } +def network_policy_values() -> dict[str, str]: + return { + "CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_COUNT": "1", + "CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_0_BASE": "198.51.100.0/24", + "CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_0_SIZE": "28", + "CI_FLEET_DOCKER_NETWORK_RESERVE_SUBNETS": "1", + } + + class HealthTests(unittest.TestCase): def test_healthy_active_host(self) -> None: report = health.evaluate(healthy_snapshot(), health.Thresholds()) @@ -59,6 +68,67 @@ def test_disk_warning_and_critical_thresholds(self) -> None: self.assertEqual((report["status"], report["exit_code"]), ("unhealthy", 2)) self.assertIn("disk_docker", {check["id"] for check in report["checks"] if check["status"] == "critical"}) + def test_docker_network_headroom_reports_low_water_exhaustion_and_legacy_networks(self) -> None: + healthy = health.evaluate({**healthy_snapshot(), "docker_network_headroom": {"configured": 16, "used": 2, "free": 14, "reserve": 1, "legacy": 0, "state": "healthy"}}, health.Thresholds()) + self.assertEqual(healthy["status"], "healthy") + self.assertIn("docker_network_headroom", {check["id"] for check in healthy["checks"] if check["status"] == "ok"}) + + warning = health.evaluate({**healthy_snapshot(), "docker_network_headroom": {"configured": 2, "used": 1, "free": 1, "reserve": 1, "legacy": 0, "state": "warning"}}, health.Thresholds()) + self.assertEqual((warning["status"], warning["exit_code"]), ("warning", 1)) + self.assertIn("docker_network_headroom", {check["id"] for check in warning["checks"] if check["status"] == "warning"}) + + critical = health.evaluate({**healthy_snapshot(), "docker_network_headroom": {"configured": 1, "used": 1, "free": 0, "reserve": 1, "legacy": 0, "state": "critical"}}, health.Thresholds()) + self.assertEqual((critical["status"], critical["exit_code"]), ("unhealthy", 2)) + self.assertIn("docker_network_headroom", {check["id"] for check in critical["checks"] if check["status"] == "critical"}) + + legacy = health.evaluate({**healthy_snapshot(), "docker_network_headroom": {"configured": 2, "used": 0, "free": 2, "reserve": 1, "legacy": 1, "state": "warning"}}, health.Thresholds()) + self.assertEqual(legacy["status"], "warning") + self.assertIn("docker_network_legacy", {check["id"] for check in legacy["checks"] if check["status"] == "warning"}) + + def test_docker_network_headroom_collection_handles_inspection_failure(self) -> None: + def run(args): + if args[:2] == ["docker", "info"]: + return health.subprocess.CompletedProcess(args, 0, "", "") + return health.subprocess.CompletedProcess(args, 1, "", "") + + snapshot = health.collect_snapshot({**network_policy_values(), "CI_FLEET_CONTROLLER_STATE": "active", "CI_FLEET_INSTANCE": "example-ci-01"}, run=run) + self.assertEqual(snapshot["docker_network_headroom"]["state"], "unavailable") + report = health.evaluate({**healthy_snapshot(), "docker_network_headroom": snapshot["docker_network_headroom"]}, health.Thresholds()) + self.assertEqual((report["status"], report["exit_code"]), ("unhealthy", 2)) + self.assertIn("docker_network_inspection", {check["id"] for check in report["checks"] if check["status"] == "critical"}) + + def test_legacy_rendered_state_without_network_policy_remains_compatible(self) -> None: + network = health._docker_network_headroom(lambda args: health.subprocess.CompletedProcess(args, 0, "", ""), {}, docker_ok=True) + report = health.evaluate({**healthy_snapshot(), "docker_network_headroom": network}, health.Thresholds()) + self.assertEqual(network["state"], "not_configured") + self.assertNotIn("docker_network_inspection", {check["id"] for check in report["checks"]}) + + def test_docker_network_headroom_collection_counts_legacy_networks(self) -> None: + networks = { + "managed": [{"IPAM": {"Config": [{"Subnet": "198.51.100.0/28"}]}}], + "legacy": [{"IPAM": {"Config": [{"Subnet": "10.0.0.0/24"}]}}], + } + + def run(args): + if args[:2] == ["docker", "info"]: + return health.subprocess.CompletedProcess(args, 0, "", "") + if args[:3] == ["docker", "network", "ls"]: + return health.subprocess.CompletedProcess(args, 0, "managed\nlegacy\n", "") + if args[:3] == ["docker", "network", "inspect"]: + return health.subprocess.CompletedProcess(args, 0, json.dumps(networks[args[-1]]), "") + return health.subprocess.CompletedProcess(args, 0, "", "") + + snapshot = health.collect_snapshot({**network_policy_values(), "CI_FLEET_CONTROLLER_STATE": "active", "CI_FLEET_INSTANCE": "example-ci-01"}, run=run) + self.assertEqual(snapshot["docker_network_headroom"], {"configured": 16, "used": 1, "free": 15, "reserve": 1, "legacy": 1, "state": "warning"}) + report = health.evaluate({**healthy_snapshot(), "docker_network_headroom": snapshot["docker_network_headroom"]}, health.Thresholds()) + self.assertEqual(report["status"], "warning") + + def test_status_report_redacts_network_addresses(self) -> None: + snapshot = {**healthy_snapshot(), "docker_network_headroom": {"configured": 16, "used": 2, "free": 14, "reserve": 1, "legacy": 1, "state": "warning"}} + report = health.build_status_report(snapshot, health.evaluate(snapshot, health.Thresholds()), generated_at=1_000) + self.assertEqual(report["docker"]["network"], {"configured": 16, "used": 2, "free": 14, "legacy": 1}) + self.assertNotIn("198.51.100", json.dumps(report)) + def test_health_contract_classifies_host_failures(self) -> None: cases = { "inode_root": (lambda s: s["disks"]["root"].update(inode_used_percent=90), "unhealthy"), diff --git a/templates/config-repository/README.md b/templates/config-repository/README.md index e54b315e..053e8dac 100644 --- a/templates/config-repository/README.md +++ b/templates/config-repository/README.md @@ -60,7 +60,8 @@ The initializer refuses to replace a configured file unless `--force` is explici - an `experimental`, `stable`, or `retiring` lifecycle; - the full reviewed ci-fleet commit SHA it runs; - a zero managed minimum and reviewed maximum runner capacity; -- CPU and memory available to each ephemeral runner. +- CPU and memory available to each ephemeral runner; +- Docker default-address pools and a reserved subnet count for health inspection. `status_reporting` is deliberately omitted from initialized and reference configurations. For an existing controller, roll out schema support in three @@ -84,6 +85,19 @@ Each runner pool has a `capacity_budget` and a runner group that must not be ass The validator totals the maximum capacity of every active or drained controller assigned to the pool and rejects overcommit. Drained capacity remains reserved so an undrain cannot silently exceed the reviewed budget. Disabled controllers do not reserve capacity. +For `docker_network_policy`, each pool has an IPv4 CIDR `base` and Docker +subnet prefix `size`. Pools must not overlap, and active or drained controllers +must provide at least `max_runners + reserve_subnets` subnets. Real pool values +belong in the private configuration; this template uses RFC 5737 documentation +ranges only. + +The public engine renders these values only for read-only health inspection. +This phase detects low water, exhaustion, failed inspection, and legacy +networks; it does not configure or restart Docker, create/delete networks, +clean resources, drain runners, or change controller scale. Circuit breaking, +frequent orphan reconciliation, daemon mutation, transactional recovery, and +consumer-label changes are later issue #81 slices. + Application repositories do not encode the number of available workers. They submit all independent tasks and shards. Do not use GitHub Actions `strategy.max-parallel` to model fleet size; controllers and the private configuration decide how many jobs run simultaneously. An application may limit concurrency only for a separately documented external-system constraint, not worker availability. This separation lets one infrastructure change add, remove, drain, or resize controllers without editing every project workflow. diff --git a/templates/config-repository/examples/multi-host/fleet.json b/templates/config-repository/examples/multi-host/fleet.json index 4d7acece..23d43a70 100644 --- a/templates/config-repository/examples/multi-host/fleet.json +++ b/templates/config-repository/examples/multi-host/fleet.json @@ -30,6 +30,10 @@ "runner_resources": { "cpu_cores": 4, "memory_mib": 8192 + }, + "docker_network_policy": { + "default_address_pools": [{"base": "198.51.100.0/24", "size": 28}], + "reserve_subnets": 1 } }, "sample-ci-remote": { @@ -44,6 +48,10 @@ "runner_resources": { "cpu_cores": 2, "memory_mib": 4096 + }, + "docker_network_policy": { + "default_address_pools": [{"base": "203.0.113.0/24", "size": 28}], + "reserve_subnets": 1 } } }, diff --git a/templates/config-repository/fleet.json b/templates/config-repository/fleet.json index bf3ccea4..0ea789a7 100644 --- a/templates/config-repository/fleet.json +++ b/templates/config-repository/fleet.json @@ -30,6 +30,15 @@ "runner_resources": { "cpu_cores": 2, "memory_mib": 4096 + }, + "docker_network_policy": { + "reserve_subnets": 1, + "default_address_pools": [ + { + "base": "198.51.100.0/24", + "size": 28 + } + ] } } }, diff --git a/templates/config-repository/fleet.schema.json b/templates/config-repository/fleet.schema.json index c08e7920..e2a95aa1 100644 --- a/templates/config-repository/fleet.schema.json +++ b/templates/config-repository/fleet.schema.json @@ -70,7 +70,7 @@ "controller": { "type": "object", "additionalProperties": false, - "required": ["pool", "location", "state", "scale_set_name", "lifecycle", "engine_ref", "min_runners", "max_runners", "runner_resources"], + "required": ["pool", "location", "state", "scale_set_name", "lifecycle", "engine_ref", "min_runners", "max_runners", "runner_resources", "docker_network_policy"], "properties": { "pool": {"$ref": "#/$defs/slug"}, "location": {"$ref": "#/$defs/slug"}, @@ -81,6 +81,27 @@ "min_runners": {"const": 0}, "max_runners": {"type": "integer", "minimum": 1}, "runner_resources": {"$ref": "#/$defs/runner_resources"}, + "docker_network_policy": { + "type": "object", + "additionalProperties": false, + "required": ["default_address_pools", "reserve_subnets"], + "properties": { + "reserve_subnets": {"type": "integer", "minimum": 1}, + "default_address_pools": { + "type": "array", + "minItems": 1, + "items": { + "type": "object", + "additionalProperties": false, + "required": ["base", "size"], + "properties": { + "base": {"type": "string"}, + "size": {"type": "integer", "minimum": 0, "maximum": 32} + } + } + } + } + }, "status_reporting": { "type": "object", "additionalProperties": false, diff --git a/templates/config-repository/scripts/init.py b/templates/config-repository/scripts/init.py index 3d3ac48b..d10b7e9c 100755 --- a/templates/config-repository/scripts/init.py +++ b/templates/config-repository/scripts/init.py @@ -115,6 +115,12 @@ def main() -> int: "cpu_cores": args.runner_cpu_cores, "memory_mib": args.runner_memory_mib, }, + "docker_network_policy": { + "reserve_subnets": 1, + "default_address_pools": [ + {"base": "198.51.100.0/24", "size": 28}, + ], + }, } }, "host_groups": { diff --git a/templates/config-repository/scripts/test_policy.py b/templates/config-repository/scripts/test_policy.py index 77b0ad75..25df4a57 100755 --- a/templates/config-repository/scripts/test_policy.py +++ b/templates/config-repository/scripts/test_policy.py @@ -52,6 +52,15 @@ def first_controller(config: dict) -> dict: return next(iter(config["controllers"].values())) +def docker_network_policy() -> dict: + return { + "reserve_subnets": 1, + "default_address_pools": [ + {"base": "198.51.100.0/24", "size": 28}, + ], + } + + class PolicyTests(unittest.TestCase): def assert_rejected(self, config: dict, expected: str, *, strict: bool = False) -> None: errors = errors_for(config, strict=strict) @@ -72,6 +81,36 @@ def assert_delivery_engine_contract(self, value: str, accepted: bool) -> None: def test_reference_configuration_is_valid(self) -> None: self.assertEqual(errors_for(reference_config()), []) + def test_schema_defines_docker_network_policy_contract(self) -> None: + controller = contract_schema()["$defs"]["controller"]["properties"]["docker_network_policy"] + self.assertEqual(set(controller), {"type", "additionalProperties", "required", "properties"}) + self.assertEqual(controller["required"], ["default_address_pools", "reserve_subnets"]) + pool = controller["properties"]["default_address_pools"]["items"] + self.assertEqual(set(pool["properties"]), {"base", "size"}) + + def test_docker_network_policy_is_required_and_capacity_checked(self) -> None: + config = copy.deepcopy(reference_config()) + first_controller(config)["docker_network_policy"] = docker_network_policy() + self.assertEqual(errors_for(config), []) + + first_controller(config)["docker_network_policy"] = { + "reserve_subnets": 1, + "default_address_pools": [{"base": "198.51.100.0/30", "size": 30}], + } + self.assert_rejected(config, "capacity") + + def test_docker_network_policy_rejects_overlapping_or_malformed_pools(self) -> None: + config = copy.deepcopy(reference_config()) + policy = docker_network_policy() + policy["default_address_pools"] = [ + {"base": "198.51.100.0/24", "size": 28}, + {"base": "198.51.100.128/25", "size": 28}, + ] + first_controller(config)["docker_network_policy"] = policy + self.assert_rejected(config, "overlap") + policy["default_address_pools"] = [{"base": "not-a-subnet", "size": 28}] + self.assert_rejected(config, "address pool") + def test_status_reporting_null_is_rejected(self) -> None: config = copy.deepcopy(reference_config()) first_controller(config)["status_reporting"] = None diff --git a/templates/config-repository/scripts/validate.py b/templates/config-repository/scripts/validate.py index db70032a..8c488de0 100755 --- a/templates/config-repository/scripts/validate.py +++ b/templates/config-repository/scripts/validate.py @@ -4,6 +4,7 @@ from __future__ import annotations import argparse +import ipaddress import json import re import sys @@ -140,6 +141,67 @@ def scan_keys(value: Any, path: str = "$") -> None: break +def validate_docker_network_policy(policy: Any, path: str, max_runners: int, validation: Validation) -> tuple[int, int, list[dict[str, Any]]]: + if not isinstance(policy, dict): + validation.errors.append(f"{path}: must be an object") + return 0, 0, [] + required = {"default_address_pools", "reserve_subnets"} + keys = set(policy) + missing = sorted(required - keys) + unknown = sorted(keys - required) + if missing or unknown: + parts = [] + if missing: + parts.append(f"missing keys: {', '.join(missing)}") + if unknown: + parts.append(f"unknown keys: {', '.join(unknown)}") + validation.errors.append(f"{path}: {'; '.join(parts)}") + return 0, 0, [] + reserve = policy["reserve_subnets"] + if type(reserve) is not int or reserve < 1: + validation.errors.append(f"{path}.reserve_subnets: must be a positive integer") + return 0, 0, [] + pools = policy["default_address_pools"] + if type(pools) is not list or not pools: + validation.errors.append(f"{path}.default_address_pools: must be a non-empty list") + return 0, 0, [] + parsed: list[dict[str, Any]] = [] + for index, pool in enumerate(pools): + pool_path = f"{path}.default_address_pools[{index}]" + if not isinstance(pool, dict) or set(pool) != {"base", "size"}: + validation.errors.append(f"{pool_path}: must contain only base and size") + return 0, 0, [] + base = pool["base"] + size = pool["size"] + if not isinstance(base, str): + validation.errors.append(f"{pool_path}.base: must be a CIDR prefix") + return 0, 0, [] + if type(size) is not int or size < 0 or size > 32: + validation.errors.append(f"{pool_path}.size: must be an IPv4 prefix length between 0 and 32") + return 0, 0, [] + try: + network = ipaddress.ip_network(base, strict=True) + except ValueError: + validation.errors.append(f"{pool_path}.base: malformed address pool IPv4 prefix") + return 0, 0, [] + if network.version != 4: + validation.errors.append(f"{pool_path}.base: malformed address pool IPv4 prefix") + return 0, 0, [] + if size < network.prefixlen: + validation.errors.append(f"{pool_path}.size: impossible subnet count for {base}") + return 0, 0, [] + parsed.append({"base": base, "network": network, "size": size}) + for left, item in enumerate(parsed): + for right in range(left + 1, len(parsed)): + if item["network"].overlaps(parsed[right]["network"]): + validation.errors.append(f"{path}.default_address_pools[{left}].base: overlaps configured pool {right}") + return 0, 0, [] + configured = sum(1 << (item["size"] - item["network"].prefixlen) for item in parsed) + if configured < max_runners + reserve: + validation.errors.append(f"{path}: network capacity cannot satisfy max_runners plus reserve") + return configured, reserve, parsed + + def scan_forbidden_paths(repo_root: Path, validation: Validation) -> None: for path in repo_root.rglob("*"): try: @@ -267,6 +329,7 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: "min_runners", "max_runners", "runner_resources", + "docker_network_policy", } for name, controller in controllers.items(): path = f"$.controllers.{name}" @@ -309,6 +372,9 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: memory = resources.get("memory_mib") validation.require(type(cpu) is int and cpu > 0, f"{path}.runner_resources.cpu_cores", "must be a positive integer") validation.require(type(memory) is int and memory >= 512, f"{path}.runner_resources.memory_mib", "must be at least 512 MiB") + network_policy = controller.get("docker_network_policy") + capacity_maximum = maximum if state != "disabled" and type(maximum) is int and maximum > 0 else 0 + validate_docker_network_policy(network_policy, f"{path}.docker_network_policy", capacity_maximum, validation) if isinstance(pool_name, str) and pool_name in pools and state != "disabled" and type(maximum) is int and maximum > 0: reserved_capacity[pool_name] += maximum From bbe3be5c493c9df29dea50b31d6b5ce7cbf6d057 Mon Sep 17 00:00:00 2001 From: Nick's Hermes <1572453+Nickfost@users.noreply.github.com> Date: Wed, 26 Aug 2026 15:17:50 -0500 Subject: [PATCH 2/8] fix: address Docker network policy review findings --- docs/DESIRED-STATE.md | 7 ++++ scripts/desired_state.py | 27 +++++++------- scripts/health.py | 33 +++++++++-------- scripts/status_receiver.py | 7 +++- scripts/test_desired_state.py | 7 ++++ scripts/test_health.py | 36 +++++++++++++++++++ scripts/test_status_receiver.py | 16 +++++++++ templates/config-repository/README.md | 5 +++ templates/config-repository/fleet.schema.json | 2 +- templates/config-repository/scripts/init.py | 4 ++- .../config-repository/scripts/test_policy.py | 33 +++++++++++++++-- .../config-repository/scripts/validate.py | 5 +-- 12 files changed, 149 insertions(+), 33 deletions(-) diff --git a/docs/DESIRED-STATE.md b/docs/DESIRED-STATE.md index fec16eaf..355ff6cc 100644 --- a/docs/DESIRED-STATE.md +++ b/docs/DESIRED-STATE.md @@ -45,6 +45,13 @@ valid policy but do not reserve runner subnet capacity. Real pool values belong only in the private desired-state repository; public examples use RFC 5737 documentation ranges. +`docker_network_policy` is optional only to preserve a staged upgrade path from +older schema-v3 engines whose exact-key validator does not recognize it. Upgrade +an existing controller in two reviewed desired-state commits: first change only +`engine_ref` and verify that this compatible engine is active; then add the +reviewed network policy in a second commit. Do not add the field while the old +engine still performs reconciliation. + This phase renders the policy solely for read-only health inspection. It does not write `daemon.json`, restart Docker, create or remove networks, prune resources, drain runners, or alter controller scale. Circuit breaking, diff --git a/scripts/desired_state.py b/scripts/desired_state.py index d4e394ca..d545287a 100755 --- a/scripts/desired_state.py +++ b/scripts/desired_state.py @@ -222,12 +222,14 @@ def build_rendered_env( state = controller["state"] configured_max = controller["max_runners"] effective_max = configured_max if state == "active" else 0 - network_policy = controller.get("docker_network_policy") or {} - configured_subnets, reserve_subnets, parsed_pools = validate_docker_network_policy( - network_policy, - path=f"$.controllers.{controller_id}.docker_network_policy", - max_runners=configured_max if state != "disabled" else 0, - ) + network_policy = controller.get("docker_network_policy") + configured_subnets, reserve_subnets, parsed_pools = (0, 0, []) + if network_policy is not None: + configured_subnets, reserve_subnets, parsed_pools = validate_docker_network_policy( + network_policy, + path=f"$.controllers.{controller_id}.docker_network_policy", + max_runners=configured_max if state != "disabled" else 0, + ) short_commit = engine_commit[:12] rendered = { "CI_FLEET_CAPACITY_BUDGET": str(pool["capacity_budget"]), @@ -238,8 +240,6 @@ def build_rendered_env( "CI_FLEET_CONTROLLER_IMAGE": f"ci-fleet-controller:{short_commit}", "CI_FLEET_CONTROLLER_STATE": state, "CI_FLEET_DESIRED_STATE_SCHEMA": "3", - "CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_COUNT": str(len(parsed_pools)), - "CI_FLEET_DOCKER_NETWORK_RESERVE_SUBNETS": str(reserve_subnets), "CI_FLEET_DOCKER_GID": str(docker_gid), "CI_FLEET_ENGINE_REF": engine_commit, "CI_FLEET_GITHUB_URL": f"https://github.com/{config['organization']['slug']}", @@ -255,9 +255,12 @@ def build_rendered_env( "CI_FLEET_VERSION": short_commit, **validate_host_values(host_values), } - for index, pool_config in enumerate(parsed_pools): - rendered[f"CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_{index}_BASE"] = pool_config["base"] - rendered[f"CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_{index}_SIZE"] = str(pool_config["size"]) + if network_policy is not None: + rendered["CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_COUNT"] = str(len(parsed_pools)) + rendered["CI_FLEET_DOCKER_NETWORK_RESERVE_SUBNETS"] = str(reserve_subnets) + for index, pool_config in enumerate(parsed_pools): + rendered[f"CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_{index}_BASE"] = pool_config["base"] + rendered[f"CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_{index}_SIZE"] = str(pool_config["size"]) reporting_configured = "status_reporting" in controller reporting_required = (controller.get("status_reporting") or {}).get("enabled") is True if reporting_required and REQUIRED_STATUS_CAPABILITY not in (engine_capabilities or set()): @@ -286,7 +289,7 @@ def build_rendered_env( "engine_repository": config["organization"]["delivery_engine"], "status_reporting_configured": reporting_configured, "status_reporting_required": reporting_required, - "docker_network_policy_configured": True, + "docker_network_policy_configured": network_policy is not None, "docker_network_default_address_pools": len(parsed_pools), "docker_network_reserve_subnets": reserve_subnets, "docker_network_configured_subnets": configured_subnets, diff --git a/scripts/health.py b/scripts/health.py index ce1d9a70..875b0651 100644 --- a/scripts/health.py +++ b/scripts/health.py @@ -321,24 +321,19 @@ def _stale_resources(run: Runner, instance: str) -> dict[str, int]: return stale -def _parse_network_pool(values: dict[str, str], index: int) -> ipaddress.IPv4Network | None: +def _parse_network_pool(values: dict[str, str], index: int) -> tuple[ipaddress.IPv4Network, int] | None: base = values.get(f"CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_{index}_BASE") size = values.get(f"CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_{index}_SIZE") if not base or not size: return None try: network = ipaddress.ip_network(base, strict=True) - except ValueError: - return None - if network.version != 4: - return None - try: subnet_size = int(size) except ValueError: return None - if subnet_size < network.prefixlen or subnet_size > 32: + if network.version != 4 or subnet_size < network.prefixlen or subnet_size > 32: return None - return network + return network, subnet_size def _docker_network_headroom(run: Runner, values: dict[str, str], *, docker_ok: bool) -> dict[str, Any]: @@ -352,7 +347,7 @@ def _docker_network_headroom(run: Runner, values: dict[str, str], *, docker_ok: return empty if configured_count == 0 and "CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_COUNT" not in values: return {**empty, "state": "not_configured"} - pools: list[ipaddress.IPv4Network] = [] + pools: list[tuple[ipaddress.IPv4Network, int]] = [] for index in range(configured_count): pool = _parse_network_pool(values, index) if pool is None: @@ -363,7 +358,7 @@ def _docker_network_headroom(run: Runner, values: dict[str, str], *, docker_ok: listed = run(["docker", "network", "ls", "--format", "{{.Name}}"]) if listed.returncode != 0: return empty - configured = sum(1 << (int(values.get(f"CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_{index}_SIZE", "0")) - pools[index].prefixlen) for index in range(len(pools))) + configured = sum(1 << (size - pool.prefixlen) for pool, size in pools) used_subnets: set[str] = set() legacy_networks = 0 for name in [line.strip() for line in listed.stdout.splitlines() if line.strip()]: @@ -377,6 +372,7 @@ def _docker_network_headroom(run: Runner, values: dict[str, str], *, docker_ok: if not isinstance(payload, list) or not payload: return empty subnets: list[ipaddress.IPv4Network] = [] + saw_ipv6 = False for entry in payload: configs = entry.get("IPAM", {}).get("Config", []) if isinstance(entry, dict) else [] if not isinstance(configs, list): @@ -389,17 +385,26 @@ def _docker_network_headroom(run: Runner, values: dict[str, str], *, docker_ok: network = ipaddress.ip_network(subnet, strict=False) except ValueError: return empty - if network.version != 4: - return empty + if network.version == 6: + saw_ipv6 = True + continue subnets.append(network) if not subnets: + if saw_ipv6 or name in {"host", "none"}: + continue legacy_networks += 1 continue network_legacy = False for subnet in subnets: - if any(subnet.subnet_of(pool) for pool in pools): - used_subnets.add(str(subnet)) + match = next(((pool, size) for pool, size in pools if subnet.subnet_of(pool)), None) + if match is None: + network_legacy = True + continue + pool, size = match + if subnet.prefixlen <= size: + used_subnets.update(str(slot) for slot in subnet.subnets(new_prefix=size)) else: + used_subnets.add(str(subnet.supernet(new_prefix=size))) network_legacy = True if network_legacy: legacy_networks += 1 diff --git a/scripts/status_receiver.py b/scripts/status_receiver.py index d0914259..e406845c 100644 --- a/scripts/status_receiver.py +++ b/scripts/status_receiver.py @@ -239,8 +239,13 @@ def enum(value: Any, choices: set[str]) -> bool: if not exact(load, {"one", "five", "fifteen"}) or not all(number(value) for value in load.values()): raise StatusError(400, "invalid_report") docker = report["docker"] - if not exact(docker, {"healthy", "oom"}) or not all(isinstance(value, bool) for value in docker.values()): + if set(docker) not in ({"healthy", "oom"}, {"healthy", "oom", "network"}) or not all(isinstance(docker[key], bool) for key in ("healthy", "oom")): raise StatusError(400, "invalid_report") + network = docker.get("network") + if network is not None: + keys = {"configured", "used", "free", "legacy"} + if not exact(network, keys) or not all(integer(network[key]) for key in keys) or network["used"] + network["free"] != network["configured"]: + raise StatusError(400, "invalid_report") error = report["error"] if error is not None and (not exact(error, {"code", "message"}) or not isinstance(error["code"], str) or not re.fullmatch(r"[a-z0-9_]{1,64}", error["code"]) or error["message"] != error["code"].replace("_", " ")): raise StatusError(400, "invalid_report") diff --git a/scripts/test_desired_state.py b/scripts/test_desired_state.py index 5dc96796..2dab1541 100755 --- a/scripts/test_desired_state.py +++ b/scripts/test_desired_state.py @@ -139,6 +139,13 @@ def test_docker_network_policy_renders_read_only_inspection_values(self) -> None self.assertEqual(metadata["docker_network_default_address_pools"], 1) self.assertEqual(metadata["docker_network_reserve_subnets"], 1) + def test_docker_network_policy_can_be_staged_after_engine_upgrade(self) -> None: + value = config() + value["controllers"]["example-ci-01"].pop("docker_network_policy", None) + environment, metadata = self.render(value) + self.assertNotIn("CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_COUNT", environment) + self.assertFalse(metadata["docker_network_policy_configured"]) + def test_docker_network_policy_requires_capacity_for_reserve(self) -> None: value = config() value["controllers"]["example-ci-01"]["max_runners"] = 2 diff --git a/scripts/test_health.py b/scripts/test_health.py index 80a18be7..f9a6ccae 100644 --- a/scripts/test_health.py +++ b/scripts/test_health.py @@ -123,6 +123,42 @@ def run(args): report = health.evaluate({**healthy_snapshot(), "docker_network_headroom": snapshot["docker_network_headroom"]}, health.Thresholds()) self.assertEqual(report["status"], "warning") + def test_builtin_addressless_networks_are_ignored_but_custom_ones_are_legacy(self) -> None: + networks = {name: [{"IPAM": {"Config": []}}] for name in ("host", "none", "custom")} + def run(args): + if args[:3] == ["docker", "network", "ls"]: + return health.subprocess.CompletedProcess(args, 0, "host\nnone\ncustom\n", "") + return health.subprocess.CompletedProcess(args, 0, json.dumps(networks[args[-1]]), "") + result = health._docker_network_headroom(run, network_policy_values(), docker_ok=True) + self.assertEqual(result["legacy"], 1) + + def test_broader_network_consumes_all_allocation_slots(self) -> None: + def run(args): + if args[:3] == ["docker", "network", "ls"]: + return health.subprocess.CompletedProcess(args, 0, "broad\n", "") + return health.subprocess.CompletedProcess(args, 0, json.dumps([{"IPAM": {"Config": [{"Subnet": "198.51.100.0/24"}]}}]), "") + result = health._docker_network_headroom(run, network_policy_values(), docker_ok=True) + self.assertEqual((result["used"], result["free"], result["state"]), (16, 0, "critical")) + + def test_ipv6_ipam_is_ignored_without_hiding_ipv4(self) -> None: + networks = { + "dual": [{"IPAM": {"Config": [{"Subnet": "2001:db8::/64"}, {"Subnet": "198.51.100.0/28"}]}}], + "v6-only": [{"IPAM": {"Config": [{"Subnet": "2001:db8:1::/64"}]}}], + } + def run(args): + if args[:3] == ["docker", "network", "ls"]: + return health.subprocess.CompletedProcess(args, 0, "dual\nv6-only\n", "") + return health.subprocess.CompletedProcess(args, 0, json.dumps(networks[args[-1]]), "") + result = health._docker_network_headroom(run, network_policy_values(), docker_ok=True) + self.assertEqual((result["used"], result["legacy"], result["state"]), (1, 0, "healthy")) + + def test_malformed_ipv4_ipam_fails_inspection(self) -> None: + def run(args): + if args[:3] == ["docker", "network", "ls"]: + return health.subprocess.CompletedProcess(args, 0, "broken\n", "") + return health.subprocess.CompletedProcess(args, 0, json.dumps([{"IPAM": {"Config": [{"Subnet": "198.51.100.999/28"}]}}]), "") + self.assertEqual(health._docker_network_headroom(run, network_policy_values(), docker_ok=True)["state"], "unavailable") + def test_status_report_redacts_network_addresses(self) -> None: snapshot = {**healthy_snapshot(), "docker_network_headroom": {"configured": 16, "used": 2, "free": 14, "reserve": 1, "legacy": 1, "state": "warning"}} report = health.build_status_report(snapshot, health.evaluate(snapshot, health.Thresholds()), generated_at=1_000) diff --git a/scripts/test_status_receiver.py b/scripts/test_status_receiver.py index ff67a417..b6b77783 100644 --- a/scripts/test_status_receiver.py +++ b/scripts/test_status_receiver.py @@ -92,6 +92,22 @@ def test_authenticated_report_is_stored_and_read_as_latest(self) -> None: self.submit(report) self.assertEqual(self.receiver.latest("example-ci-01", "reader-token"), report) + def test_network_aggregates_are_optional_but_strict_when_present(self) -> None: + current = valid_report() + current["docker"]["network"] = {"configured": 16, "used": 2, "free": 14, "legacy": 0} + self.submit(current) + self.assertEqual(self.receiver.latest("example-ci-01", "reader-token"), current) + + for malformed in ( + {"configured": 16, "used": 2, "free": 14}, + {"configured": 16, "used": -1, "free": 17, "legacy": 0}, + {"configured": 16, "used": 2, "free": 15, "legacy": 0}, + {"configured": 16, "used": True, "free": 15, "legacy": 0}, + ): + report = valid_report(generated_at=1_001) + report["docker"]["network"] = malformed + self.assert_status_error(400, "invalid_report", lambda report=report: self.submit(report, timestamp=1_001, nonce="b" * 32)) + def test_concurrent_report_writes_are_serialized(self) -> None: body, headers = self.signed( valid_report("other-ci-01"), controller="other-ci-01", key=b"other-key" diff --git a/templates/config-repository/README.md b/templates/config-repository/README.md index 053e8dac..41e082be 100644 --- a/templates/config-repository/README.md +++ b/templates/config-repository/README.md @@ -91,6 +91,11 @@ must provide at least `max_runners + reserve_subnets` subnets. Real pool values belong in the private configuration; this template uses RFC 5737 documentation ranges only. +The field is optional solely for staged upgrades from older schema-v3 engines. +First pin and activate this compatible engine without adding the field. In a +second reviewed desired-state commit, add the reviewed policy. The older engine +rejects the new key, so combining those steps prevents reconciliation. + The public engine renders these values only for read-only health inspection. This phase detects low water, exhaustion, failed inspection, and legacy networks; it does not configure or restart Docker, create/delete networks, diff --git a/templates/config-repository/fleet.schema.json b/templates/config-repository/fleet.schema.json index e2a95aa1..82bca12f 100644 --- a/templates/config-repository/fleet.schema.json +++ b/templates/config-repository/fleet.schema.json @@ -70,7 +70,7 @@ "controller": { "type": "object", "additionalProperties": false, - "required": ["pool", "location", "state", "scale_set_name", "lifecycle", "engine_ref", "min_runners", "max_runners", "runner_resources", "docker_network_policy"], + "required": ["pool", "location", "state", "scale_set_name", "lifecycle", "engine_ref", "min_runners", "max_runners", "runner_resources"], "properties": { "pool": {"$ref": "#/$defs/slug"}, "location": {"$ref": "#/$defs/slug"}, diff --git a/templates/config-repository/scripts/init.py b/templates/config-repository/scripts/init.py index d10b7e9c..0e795392 100755 --- a/templates/config-repository/scripts/init.py +++ b/templates/config-repository/scripts/init.py @@ -67,6 +67,8 @@ def main() -> int: fail("--engine-ref must be a nonzero full lowercase commit SHA") if args.max_runners > args.capacity_budget: fail("--max-runners must not exceed --capacity-budget") + if args.max_runners > 255: + fail("--max-runners must not exceed 255 for the fictional /24 Docker address pool") if args.runner_memory_mib < 512: fail("--runner-memory-mib must be at least 512") @@ -118,7 +120,7 @@ def main() -> int: "docker_network_policy": { "reserve_subnets": 1, "default_address_pools": [ - {"base": "198.51.100.0/24", "size": 28}, + {"base": "198.51.100.0/24", "size": 24 + args.max_runners.bit_length()}, ], }, } diff --git a/templates/config-repository/scripts/test_policy.py b/templates/config-repository/scripts/test_policy.py index 25df4a57..22059563 100755 --- a/templates/config-repository/scripts/test_policy.py +++ b/templates/config-repository/scripts/test_policy.py @@ -82,14 +82,18 @@ def test_reference_configuration_is_valid(self) -> None: self.assertEqual(errors_for(reference_config()), []) def test_schema_defines_docker_network_policy_contract(self) -> None: - controller = contract_schema()["$defs"]["controller"]["properties"]["docker_network_policy"] + controller_schema = contract_schema()["$defs"]["controller"] + self.assertNotIn("docker_network_policy", controller_schema["required"]) + controller = controller_schema["properties"]["docker_network_policy"] self.assertEqual(set(controller), {"type", "additionalProperties", "required", "properties"}) self.assertEqual(controller["required"], ["default_address_pools", "reserve_subnets"]) pool = controller["properties"]["default_address_pools"]["items"] self.assertEqual(set(pool["properties"]), {"base", "size"}) - def test_docker_network_policy_is_required_and_capacity_checked(self) -> None: + def test_docker_network_policy_is_optional_and_capacity_checked_when_present(self) -> None: config = copy.deepcopy(reference_config()) + first_controller(config).pop("docker_network_policy") + self.assertEqual(errors_for(config), []) first_controller(config)["docker_network_policy"] = docker_network_policy() self.assertEqual(errors_for(config), []) @@ -127,6 +131,31 @@ def test_initializer_omits_status_reporting_for_staged_adoption(self) -> None: controller = first_controller(json.loads(output.read_text())) self.assertNotIn("status_reporting", controller) + def test_initializer_sizes_policy_for_sixteen_runners(self) -> None: + with tempfile.TemporaryDirectory() as directory: + output = Path(directory) / "fleet.json" + subprocess.run([ + sys.executable, str(ROOT / "scripts" / "init.py"), + "--organization", "sample-org", "--project", "sample-app", + "--engine-ref", "1" * 40, "--max-runners", "16", + "--capacity-budget", "16", "--output", str(output), + ], check=True, stdout=subprocess.DEVNULL) + config = json.loads(output.read_text()) + self.assertEqual(errors_for(config), []) + policy = first_controller(config)["docker_network_policy"] + self.assertGreaterEqual(1 << (policy["default_address_pools"][0]["size"] - 24), 17) + + def test_initializer_rejects_more_than_documentation_pool_can_hold(self) -> None: + with tempfile.TemporaryDirectory() as directory: + result = subprocess.run([ + sys.executable, str(ROOT / "scripts" / "init.py"), + "--organization", "sample-org", "--project", "sample-app", + "--engine-ref", "1" * 40, "--max-runners", "256", + "--capacity-budget", "256", "--output", str(Path(directory) / "fleet.json"), + ], text=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + self.assertNotEqual(result.returncode, 0) + self.assertIn("must not exceed 255", result.stderr) + def test_status_reporting_requires_a_separate_engine_rollout(self) -> None: previous = reference_config() current = copy.deepcopy(previous) diff --git a/templates/config-repository/scripts/validate.py b/templates/config-repository/scripts/validate.py index 8c488de0..465107f3 100755 --- a/templates/config-repository/scripts/validate.py +++ b/templates/config-repository/scripts/validate.py @@ -334,7 +334,7 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: for name, controller in controllers.items(): path = f"$.controllers.{name}" validation.require(isinstance(name, str) and bool(SLUG.fullmatch(name)), path, "controller ID must be a unique lowercase slug") - if not validation.exact_keys(controller, path, controller_keys, {"status_reporting"}): + if not validation.exact_keys(controller, path, controller_keys - {"docker_network_policy"}, {"status_reporting", "docker_network_policy"}): continue pool_name = controller.get("pool") location = controller.get("location") @@ -374,7 +374,8 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: validation.require(type(memory) is int and memory >= 512, f"{path}.runner_resources.memory_mib", "must be at least 512 MiB") network_policy = controller.get("docker_network_policy") capacity_maximum = maximum if state != "disabled" and type(maximum) is int and maximum > 0 else 0 - validate_docker_network_policy(network_policy, f"{path}.docker_network_policy", capacity_maximum, validation) + if network_policy is not None: + validate_docker_network_policy(network_policy, f"{path}.docker_network_policy", capacity_maximum, validation) if isinstance(pool_name, str) and pool_name in pools and state != "disabled" and type(maximum) is int and maximum > 0: reserved_capacity[pool_name] += maximum From c87fb643f22c01f4ddd1ca56b0fc6784b75a3a7a Mon Sep 17 00:00:00 2001 From: Nick's Hermes <1572453+Nickfost@users.noreply.github.com> Date: Wed, 26 Aug 2026 15:55:02 -0500 Subject: [PATCH 3/8] fix: harden Docker network inspection --- scripts/health.py | 37 +++++++++++++++++++-------- scripts/status_receiver.py | 2 +- scripts/test_health.py | 44 +++++++++++++++++++++++++++++++++ scripts/test_status_receiver.py | 10 ++++++++ 4 files changed, 82 insertions(+), 11 deletions(-) diff --git a/scripts/health.py b/scripts/health.py index 875b0651..9072dc92 100644 --- a/scripts/health.py +++ b/scripts/health.py @@ -359,11 +359,16 @@ def _docker_network_headroom(run: Runner, values: dict[str, str], *, docker_ok: if listed.returncode != 0: return empty configured = sum(1 << (size - pool.prefixlen) for pool, size in pools) - used_subnets: set[str] = set() + occupied: list[list[tuple[int, int]]] = [[] for _ in pools] legacy_networks = 0 for name in [line.strip() for line in listed.stdout.splitlines() if line.strip()]: inspected = run(["docker", "network", "inspect", name]) if inspected.returncode != 0: + refreshed = run(["docker", "network", "ls", "--format", "{{.Name}}"]) + if refreshed.returncode != 0: + return empty + if name not in {line.strip() for line in refreshed.stdout.splitlines() if line.strip()}: + continue return empty try: payload = json.loads(inspected.stdout) @@ -396,19 +401,31 @@ def _docker_network_headroom(run: Runner, values: dict[str, str], *, docker_ok: continue network_legacy = False for subnet in subnets: - match = next(((pool, size) for pool, size in pools if subnet.subnet_of(pool)), None) - if match is None: + overlaps = 0 + for index, (pool, size) in enumerate(pools): + first = max(int(subnet.network_address), int(pool.network_address)) + last = min(int(subnet.broadcast_address), int(pool.broadcast_address)) + if first > last: + continue + block_size = 1 << (32 - size) + pool_start = int(pool.network_address) + occupied[index].append(((first - pool_start) // block_size, (last - pool_start) // block_size)) + overlaps += 1 + if overlaps == 0: network_legacy = True - continue - pool, size = match - if subnet.prefixlen <= size: - used_subnets.update(str(slot) for slot in subnet.subnets(new_prefix=size)) - else: - used_subnets.add(str(subnet.supernet(new_prefix=size))) + elif overlaps != 1 or not any(subnet.subnet_of(pool) and subnet.prefixlen == size for pool, size in pools): network_legacy = True if network_legacy: legacy_networks += 1 - used = len(used_subnets) + used = 0 + for intervals in occupied: + end = -1 + for start, stop in sorted(intervals): + if start > end: + used += stop - start + 1 + elif stop > end: + used += stop - end + end = max(end, stop) free = max(configured - used, 0) if free == 0: state = "critical" diff --git a/scripts/status_receiver.py b/scripts/status_receiver.py index e406845c..8c862d54 100644 --- a/scripts/status_receiver.py +++ b/scripts/status_receiver.py @@ -239,7 +239,7 @@ def enum(value: Any, choices: set[str]) -> bool: if not exact(load, {"one", "five", "fifteen"}) or not all(number(value) for value in load.values()): raise StatusError(400, "invalid_report") docker = report["docker"] - if set(docker) not in ({"healthy", "oom"}, {"healthy", "oom", "network"}) or not all(isinstance(docker[key], bool) for key in ("healthy", "oom")): + if not isinstance(docker, dict) or set(docker) not in ({"healthy", "oom"}, {"healthy", "oom", "network"}) or not all(isinstance(docker[key], bool) for key in ("healthy", "oom")): raise StatusError(400, "invalid_report") network = docker.get("network") if network is not None: diff --git a/scripts/test_health.py b/scripts/test_health.py index f9a6ccae..f5b8876c 100644 --- a/scripts/test_health.py +++ b/scripts/test_health.py @@ -140,6 +140,50 @@ def run(args): result = health._docker_network_headroom(run, network_policy_values(), docker_ok=True) self.assertEqual((result["used"], result["free"], result["state"]), (16, 0, "critical")) + def test_network_containing_pool_consumes_all_intersecting_slots(self) -> None: + def run(args): + if args[:3] == ["docker", "network", "ls"]: + return health.subprocess.CompletedProcess(args, 0, "inverse\n", "") + return health.subprocess.CompletedProcess(args, 0, json.dumps([{"IPAM": {"Config": [{"Subnet": "198.51.100.0/23"}]}}]), "") + result = health._docker_network_headroom(run, network_policy_values(), docker_ok=True) + self.assertEqual((result["used"], result["free"], result["state"]), (16, 0, "critical")) + + def test_large_pool_gap_counts_overlaps_without_materializing_slots(self) -> None: + values = { + "CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_COUNT": "1", + "CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_0_BASE": "10.0.0.0/8", + "CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_0_SIZE": "32", + "CI_FLEET_DOCKER_NETWORK_RESERVE_SUBNETS": "1", + } + networks = { + "broad": [{"IPAM": {"Config": [{"Subnet": "10.0.0.0/8"}]}}], + "duplicate": [{"IPAM": {"Config": [{"Subnet": "10.0.0.0/9"}]}}], + } + def run(args): + if args[:3] == ["docker", "network", "ls"]: + return health.subprocess.CompletedProcess(args, 0, "broad\nduplicate\n", "") + return health.subprocess.CompletedProcess(args, 0, json.dumps(networks[args[-1]]), "") + result = health._docker_network_headroom(run, values, docker_ok=True) + self.assertEqual((result["configured"], result["used"], result["free"]), (1 << 24, 1 << 24, 0)) + + def test_network_removed_during_inspection_is_skipped_after_refresh(self) -> None: + listings = iter(("vanished\nmanaged\n", "managed\n")) + def run(args): + if args[:3] == ["docker", "network", "ls"]: + return health.subprocess.CompletedProcess(args, 0, next(listings), "") + if args[-1] == "vanished": + return health.subprocess.CompletedProcess(args, 1, "", "not found") + return health.subprocess.CompletedProcess(args, 0, json.dumps([{"IPAM": {"Config": [{"Subnet": "198.51.100.0/28"}]}}]), "") + result = health._docker_network_headroom(run, network_policy_values(), docker_ok=True) + self.assertEqual((result["used"], result["state"]), (1, "healthy")) + + def test_network_inspection_error_fails_closed_when_network_still_exists(self) -> None: + def run(args): + if args[:3] == ["docker", "network", "ls"]: + return health.subprocess.CompletedProcess(args, 0, "broken\n", "") + return health.subprocess.CompletedProcess(args, 1, "", "permission denied") + self.assertEqual(health._docker_network_headroom(run, network_policy_values(), docker_ok=True)["state"], "unavailable") + def test_ipv6_ipam_is_ignored_without_hiding_ipv4(self) -> None: networks = { "dual": [{"IPAM": {"Config": [{"Subnet": "2001:db8::/64"}, {"Subnet": "198.51.100.0/28"}]}}], diff --git a/scripts/test_status_receiver.py b/scripts/test_status_receiver.py index b6b77783..cc479e64 100644 --- a/scripts/test_status_receiver.py +++ b/scripts/test_status_receiver.py @@ -108,6 +108,16 @@ def test_network_aggregates_are_optional_but_strict_when_present(self) -> None: report["docker"]["network"] = malformed self.assert_status_error(400, "invalid_report", lambda report=report: self.submit(report, timestamp=1_001, nonce="b" * 32)) + def test_non_object_docker_sections_are_rejected_cleanly(self) -> None: + for index, malformed in enumerate((None, [], "docker", 1)): + report = valid_report(generated_at=1_010 + index) + report["docker"] = malformed + self.assert_status_error( + 400, + "invalid_report", + lambda report=report, index=index: self.submit(report, timestamp=1_010 + index, nonce=f"{index:032x}"), + ) + def test_concurrent_report_writes_are_serialized(self) -> None: body, headers = self.signed( valid_report("other-ci-01"), controller="other-ci-01", key=b"other-key" From 8ae88b83b93c6c214f85135017022f821b8b6aa3 Mon Sep 17 00:00:00 2001 From: Nick's Hermes <1572453+Nickfost@users.noreply.github.com> Date: Wed, 26 Aug 2026 15:55:53 -0500 Subject: [PATCH 4/8] test: use reserved network capacity range --- scripts/test_health.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/test_health.py b/scripts/test_health.py index f5b8876c..fa30fe46 100644 --- a/scripts/test_health.py +++ b/scripts/test_health.py @@ -151,13 +151,13 @@ def run(args): def test_large_pool_gap_counts_overlaps_without_materializing_slots(self) -> None: values = { "CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_COUNT": "1", - "CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_0_BASE": "10.0.0.0/8", + "CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_0_BASE": "240.0.0.0/8", "CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_0_SIZE": "32", "CI_FLEET_DOCKER_NETWORK_RESERVE_SUBNETS": "1", } networks = { - "broad": [{"IPAM": {"Config": [{"Subnet": "10.0.0.0/8"}]}}], - "duplicate": [{"IPAM": {"Config": [{"Subnet": "10.0.0.0/9"}]}}], + "broad": [{"IPAM": {"Config": [{"Subnet": "240.0.0.0/8"}]}}], + "duplicate": [{"IPAM": {"Config": [{"Subnet": "240.0.0.0/9"}]}}], } def run(args): if args[:3] == ["docker", "network", "ls"]: From 5247252b50e5682f880097555efd3e3458bec565 Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Wed, 26 Aug 2026 17:17:39 -0500 Subject: [PATCH 5/8] fix: harden Docker network policy validation --- docs/DESIRED-STATE.md | 13 ++-- scripts/desired_state.py | 6 +- scripts/health.py | 20 +++--- scripts/test-install-worker-controller.sh | 1 + scripts/test_desired_state.py | 12 ++++ scripts/test_health.py | 11 +++ templates/config-repository/README.md | 17 +++-- templates/config-repository/scripts/init.py | 10 +-- .../config-repository/scripts/test_policy.py | 67 ++++++++++++++++++- .../config-repository/scripts/validate.py | 37 ++++++++-- 10 files changed, 160 insertions(+), 34 deletions(-) diff --git a/docs/DESIRED-STATE.md b/docs/DESIRED-STATE.md index 355ff6cc..0be1dca7 100644 --- a/docs/DESIRED-STATE.md +++ b/docs/DESIRED-STATE.md @@ -40,17 +40,20 @@ Active and drained controllers reserve their configured maximum against the pool The Docker network policy uses IPv4 CIDR `base` values and a Docker subnet prefix `size`. Validation rejects malformed or overlapping pools, impossible prefix relationships, and active or drained policies with fewer subnets than -`max_runners + reserve_subnets`. Disabled controllers retain a structurally -valid policy but do not reserve runner subnet capacity. Real pool values belong -only in the private desired-state repository; public examples use RFC 5737 -documentation ranges. +`max_runners + reserve_subnets + 1`. The final subnet is reserved for the +persistent controller Compose network. Disabled controllers do not reserve +runner subnet capacity, but their retained policy must still cover the reserve +and controller network. Real pool values belong only in the private desired-state +repository. Public examples use RFC 5737 documentation ranges, which strict +validation rejects until the operator supplies a reviewed non-overlapping pool. `docker_network_policy` is optional only to preserve a staged upgrade path from older schema-v3 engines whose exact-key validator does not recognize it. Upgrade an existing controller in two reviewed desired-state commits: first change only `engine_ref` and verify that this compatible engine is active; then add the reviewed network policy in a second commit. Do not add the field while the old -engine still performs reconciliation. +engine still performs reconciliation. Transition validation rejects a commit +that changes `engine_ref` while introducing the policy. This phase renders the policy solely for read-only health inspection. It does not write `daemon.json`, restart Docker, create or remove networks, prune diff --git a/scripts/desired_state.py b/scripts/desired_state.py index d545287a..cd37676c 100755 --- a/scripts/desired_state.py +++ b/scripts/desired_state.py @@ -185,8 +185,10 @@ def validate_docker_network_policy(policy: dict[str, Any], *, path: str, max_run f"{path}.default_address_pools[{left}].base: overlaps configured pool {right}" ) configured = sum(1 << (item["size"] - item["network"].prefixlen) for item in parsed) - if configured < max_runners + reserve: - raise DesiredStateError(f"{path}: policy cannot satisfy max_runners plus reserve") + if configured < max_runners + reserve + 1: + raise DesiredStateError( + f"{path}: network capacity cannot satisfy max_runners + reserve_subnets + one controller Compose network" + ) return configured, reserve, parsed diff --git a/scripts/health.py b/scripts/health.py index 9072dc92..5dc98a42 100644 --- a/scripts/health.py +++ b/scripts/health.py @@ -232,6 +232,13 @@ def build_status_report(snapshot: dict[str, Any], health_report: dict[str, Any], last_success = reconciliation.get("last_success_at") if not isinstance(last_success, int) or isinstance(last_success, bool) or last_success > generated_at: last_success = None + docker = { + "healthy": bool(snapshot.get("docker_available")), + "oom": bool(snapshot.get("recent_oom") or snapshot["controller"].get("oom_killed")), + } + network = snapshot.get("docker_network_headroom") + if network and network.get("state") != "not_configured": + docker["network"] = {key: network.get(key, 0) for key in ("configured", "used", "free", "legacy")} return { "schema_version": 1, "controller": { @@ -265,14 +272,7 @@ def build_status_report(snapshot: dict[str, Any], health_report: dict[str, Any], "inodes": {name: {"total": value.get("inode_total", 0), "used": value.get("inode_used", 0)} for name, value in disks.items()}, "load": snapshot.get("load", {"one": 0, "five": 0, "fifteen": 0}), }, - "docker": { - "healthy": bool(snapshot.get("docker_available")), - "oom": bool(snapshot.get("recent_oom") or snapshot["controller"].get("oom_killed")), - "network": { - key: snapshot.get("docker_network_headroom", {}).get(key, 0) - for key in ("configured", "used", "free", "legacy") - }, - }, + "docker": docker, "error": error, "generated_at": generated_at, } @@ -338,8 +338,6 @@ def _parse_network_pool(values: dict[str, str], index: int) -> tuple[ipaddress.I def _docker_network_headroom(run: Runner, values: dict[str, str], *, docker_ok: bool) -> dict[str, Any]: empty = {"configured": 0, "used": 0, "free": 0, "reserve": 0, "legacy": 0, "state": "unavailable"} - if not docker_ok: - return empty try: configured_count = int(values.get("CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_COUNT", "0")) reserve = int(values.get("CI_FLEET_DOCKER_NETWORK_RESERVE_SUBNETS", "0")) @@ -347,6 +345,8 @@ def _docker_network_headroom(run: Runner, values: dict[str, str], *, docker_ok: return empty if configured_count == 0 and "CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_COUNT" not in values: return {**empty, "state": "not_configured"} + if not docker_ok: + return empty pools: list[tuple[ipaddress.IPv4Network, int]] = [] for index in range(configured_count): pool = _parse_network_pool(values, index) diff --git a/scripts/test-install-worker-controller.sh b/scripts/test-install-worker-controller.sh index 445a77d0..3d4c9a53 100755 --- a/scripts/test-install-worker-controller.sh +++ b/scripts/test-install-worker-controller.sh @@ -250,6 +250,7 @@ value["organization"]["slug"] = "fixture-org" value["runner_pools"]["trusted-ci"]["allowed_repositories"] = ["fixture-org/example-app"] value["projects"]["example-app"]["repository"] = "fixture-org/example-app" controller = value["controllers"]["example-ci-01"] +controller["docker_network_policy"]["default_address_pools"][0]["base"] = "10.64.0.0/24" controller["engine_ref"] = engine_ref controller["state"] = state controller["max_runners"] = int(maximum) diff --git a/scripts/test_desired_state.py b/scripts/test_desired_state.py index 2dab1541..bdcfa59c 100755 --- a/scripts/test_desired_state.py +++ b/scripts/test_desired_state.py @@ -15,6 +15,7 @@ load_engine_capabilities, load_and_validate_config, parse_env, + validate_docker_network_policy, validate_host_values, ) @@ -159,6 +160,17 @@ def test_docker_network_policy_requires_capacity_for_reserve(self) -> None: with self.assertRaisesRegex(DesiredStateError, "capacity"): load_and_validate_config(path) + def test_docker_network_policy_reserves_controller_compose_subnet(self) -> None: + with self.assertRaisesRegex(DesiredStateError, "controller Compose network"): + validate_docker_network_policy( + { + "reserve_subnets": 1, + "default_address_pools": [{"base": "198.51.100.0/30", "size": 31}], + }, + path="$.controllers.example-ci-01.docker_network_policy", + max_runners=1, + ) + def test_drained_controller_renders_zero_effective_capacity(self) -> None: value = config() value["controllers"]["example-ci-01"]["state"] = "drained" diff --git a/scripts/test_health.py b/scripts/test_health.py index fa30fe46..119f40b7 100644 --- a/scripts/test_health.py +++ b/scripts/test_health.py @@ -103,6 +103,17 @@ def test_legacy_rendered_state_without_network_policy_remains_compatible(self) - self.assertEqual(network["state"], "not_configured") self.assertNotIn("docker_network_inspection", {check["id"] for check in report["checks"]}) + def test_legacy_status_report_omits_unconfigured_docker_network(self) -> None: + for docker_ok in (True, False): + with self.subTest(docker_ok=docker_ok): + network = health._docker_network_headroom( + lambda args: health.subprocess.CompletedProcess(args, 0, "", ""), {}, docker_ok=docker_ok + ) + snapshot = {**healthy_snapshot(), "docker_network_headroom": network} + report = health.build_status_report(snapshot, health.evaluate(snapshot, health.Thresholds()), generated_at=1_000) + self.assertEqual(network["state"], "not_configured") + self.assertNotIn("network", report["docker"]) + def test_docker_network_headroom_collection_counts_legacy_networks(self) -> None: networks = { "managed": [{"IPAM": {"Config": [{"Subnet": "198.51.100.0/28"}]}}], diff --git a/templates/config-repository/README.md b/templates/config-repository/README.md index 41e082be..5744e77b 100644 --- a/templates/config-repository/README.md +++ b/templates/config-repository/README.md @@ -39,7 +39,8 @@ flowchart LR --engine-ref ``` -3. Edit `fleet.json` to add the organization's real logical mappings. +3. Edit `fleet.json` to add the organization's real logical mappings and replace + the generated RFC 5737 Docker pool with a reviewed non-overlapping pool. 4. Run the strict policy check: ```bash @@ -48,7 +49,7 @@ flowchart LR 5. Configure secret **values** in GitHub Environments, root-owned host files, or an external secret manager. The repository stores only names such as `DEPLOY_AUTH`. -The initializer refuses to replace a configured file unless `--force` is explicit. Run `./scripts/init.sh --help` for repository, registry, runner-group, controller, location, capacity, resource, and output options. +The initializer refuses to replace a configured file unless `--force` is explicit. It accepts at most 30 runners so its fictional `/24` pool never produces networks smaller than `/29` after reserving the controller and operator headroom. It runs non-strict validation because the generated documentation pool is intentionally not deployable. Run `./scripts/init.sh --help` for repository, registry, runner-group, controller, location, capacity, resource, and output options. ## Schema v3: Git-authored controller desired state @@ -87,14 +88,18 @@ The validator totals the maximum capacity of every active or drained controller For `docker_network_policy`, each pool has an IPv4 CIDR `base` and Docker subnet prefix `size`. Pools must not overlap, and active or drained controllers -must provide at least `max_runners + reserve_subnets` subnets. Real pool values -belong in the private configuration; this template uses RFC 5737 documentation -ranges only. +must provide at least `max_runners + reserve_subnets + 1` subnets. The final +subnet is reserved for the persistent controller Compose network. Real pool +values belong in the private configuration; this template uses RFC 5737 +documentation ranges only. Non-strict validation accepts those public examples. +Strict validation rejects them until the private configuration uses a reviewed +non-overlapping pool. The field is optional solely for staged upgrades from older schema-v3 engines. First pin and activate this compatible engine without adding the field. In a second reviewed desired-state commit, add the reviewed policy. The older engine -rejects the new key, so combining those steps prevents reconciliation. +rejects the new key, so combining those steps prevents reconciliation. Transition +validation enforces the separate commits for existing controllers. The public engine renders these values only for read-only health inspection. This phase detects low water, exhaustion, failed inspection, and legacy diff --git a/templates/config-repository/scripts/init.py b/templates/config-repository/scripts/init.py index 0e795392..fb3be08d 100755 --- a/templates/config-repository/scripts/init.py +++ b/templates/config-repository/scripts/init.py @@ -67,8 +67,8 @@ def main() -> int: fail("--engine-ref must be a nonzero full lowercase commit SHA") if args.max_runners > args.capacity_budget: fail("--max-runners must not exceed --capacity-budget") - if args.max_runners > 255: - fail("--max-runners must not exceed 255 for the fictional /24 Docker address pool") + if args.max_runners > 30: + fail("--max-runners must not exceed 30 so generated Docker networks stay at /29 or larger with reserved capacity") if args.runner_memory_mib < 512: fail("--runner-memory-mib must be at least 512") @@ -120,7 +120,7 @@ def main() -> int: "docker_network_policy": { "reserve_subnets": 1, "default_address_pools": [ - {"base": "198.51.100.0/24", "size": 24 + args.max_runners.bit_length()}, + {"base": "198.51.100.0/24", "size": 24 + (args.max_runners + 1).bit_length()}, ], }, } @@ -172,7 +172,7 @@ def main() -> int: handle.flush() os.fsync(handle.fileno()) subprocess.run( - [str(ROOT / "scripts" / "validate.sh"), "--strict", "--skip-path-scan", "--config", str(temporary)], + [str(ROOT / "scripts" / "validate.sh"), "--skip-path-scan", "--config", str(temporary)], check=True, ) os.chmod(temporary, 0o644) @@ -180,7 +180,7 @@ def main() -> int: finally: temporary.unlink(missing_ok=True) print(f"Initialized {output}") - print("Next: review controller capacity, configure GitHub policy, and keep every secret value outside Git.") + print("Next: replace the RFC 5737 Docker pool, run ./scripts/validate.sh --strict, and keep every secret value outside Git.") return 0 diff --git a/templates/config-repository/scripts/test_policy.py b/templates/config-repository/scripts/test_policy.py index 22059563..65b940ce 100755 --- a/templates/config-repository/scripts/test_policy.py +++ b/templates/config-repository/scripts/test_policy.py @@ -103,6 +103,14 @@ def test_docker_network_policy_is_optional_and_capacity_checked_when_present(sel } self.assert_rejected(config, "capacity") + def test_docker_network_policy_reserves_controller_compose_subnet(self) -> None: + config = copy.deepcopy(reference_config()) + first_controller(config)["docker_network_policy"] = { + "reserve_subnets": 1, + "default_address_pools": [{"base": "198.51.100.0/30", "size": 31}], + } + self.assert_rejected(config, "controller Compose network") + def test_docker_network_policy_rejects_overlapping_or_malformed_pools(self) -> None: config = copy.deepcopy(reference_config()) policy = docker_network_policy() @@ -143,7 +151,30 @@ def test_initializer_sizes_policy_for_sixteen_runners(self) -> None: config = json.loads(output.read_text()) self.assertEqual(errors_for(config), []) policy = first_controller(config)["docker_network_policy"] - self.assertGreaterEqual(1 << (policy["default_address_pools"][0]["size"] - 24), 17) + self.assertGreaterEqual(1 << (policy["default_address_pools"][0]["size"] - 24), 18) + + def test_initializer_accepts_largest_practical_network_allocation(self) -> None: + with tempfile.TemporaryDirectory() as directory: + output = Path(directory) / "fleet.json" + subprocess.run([ + sys.executable, str(ROOT / "scripts" / "init.py"), + "--organization", "sample-org", "--project", "sample-app", + "--engine-ref", "1" * 40, "--max-runners", "30", + "--capacity-budget", "30", "--output", str(output), + ], check=True, stdout=subprocess.DEVNULL) + config = json.loads(output.read_text()) + self.assertEqual(first_controller(config)["docker_network_policy"]["default_address_pools"][0]["size"], 29) + + def test_initializer_rejects_impractically_small_network_allocations(self) -> None: + with tempfile.TemporaryDirectory() as directory: + result = subprocess.run([ + sys.executable, str(ROOT / "scripts" / "init.py"), + "--organization", "sample-org", "--project", "sample-app", + "--engine-ref", "1" * 40, "--max-runners", "31", + "--capacity-budget", "31", "--output", str(Path(directory) / "fleet.json"), + ], text=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + self.assertNotEqual(result.returncode, 0) + self.assertIn("generated Docker networks stay at /29 or larger with reserved capacity", result.stderr) def test_initializer_rejects_more_than_documentation_pool_can_hold(self) -> None: with tempfile.TemporaryDirectory() as directory: @@ -154,7 +185,7 @@ def test_initializer_rejects_more_than_documentation_pool_can_hold(self) -> None "--capacity-budget", "256", "--output", str(Path(directory) / "fleet.json"), ], text=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) self.assertNotEqual(result.returncode, 0) - self.assertIn("must not exceed 255", result.stderr) + self.assertIn("must not exceed 30", result.stderr) def test_status_reporting_requires_a_separate_engine_rollout(self) -> None: previous = reference_config() @@ -193,6 +224,25 @@ def test_status_reporting_requires_a_separate_engine_rollout(self) -> None: }, validation, {}) self.assertTrue(any("capability evidence" in error for error in validation.errors), validation.errors) + def test_docker_network_policy_requires_a_separate_engine_rollout(self) -> None: + previous = reference_config() + first_controller(previous).pop("docker_network_policy") + current = copy.deepcopy(previous) + first_controller(current)["engine_ref"] = "2" * 40 + first_controller(current)["docker_network_policy"] = docker_network_policy() + validation = Validation() + validate_transition(previous, current, {}, validation) + self.assertTrue(any("docker_network_policy" in error and "later commit" in error for error in validation.errors), validation.errors) + + def test_docker_network_policy_accepts_an_unchanged_staged_engine(self) -> None: + previous = reference_config() + first_controller(previous).pop("docker_network_policy") + current = copy.deepcopy(previous) + first_controller(current)["docker_network_policy"] = docker_network_policy() + validation = Validation() + validate_transition(previous, current, {}, validation) + self.assertEqual(validation.errors, []) + def test_retained_reporting_requires_target_engine_capabilities(self) -> None: previous = reference_config() first_controller(previous)["status_reporting"] = { @@ -797,6 +847,19 @@ def test_strict_mode_rejects_unchanged_example(self) -> None: project["repository"] = "example-org/example-app" self.assert_rejected(config, "replace the example organization", strict=True) + def test_strict_mode_requires_replacing_documentation_address_pools(self) -> None: + config = copy.deepcopy(reference_config()) + project = first_project(config) + config["organization"].update(slug="sample-org", registry="ghcr.io/sample-org") + config["runner_pools"][project["ci_pool"]]["allowed_repositories"] = ["sample-org/sample-app"] + project.update(repository="sample-org/sample-app", image="ghcr.io/sample-org/sample-app") + for base in ("192.0.2.0/24", "198.51.100.0/24", "203.0.113.0/24"): + with self.subTest(base=base): + first_controller(config)["docker_network_policy"]["default_address_pools"][0]["base"] = base + self.assert_rejected(config, "replace the RFC 5737 documentation address pool", strict=True) + first_controller(config)["docker_network_policy"]["default_address_pools"][0]["base"] = "10.64.0.0/24" + self.assertEqual(errors_for(config, strict=True), []) + def test_nonstandard_ci_entrypoint_is_rejected(self) -> None: config = copy.deepcopy(reference_config()) first_project(config)["ci_contract"]["aggregate_entrypoints"]["fast"] = "npm test" diff --git a/templates/config-repository/scripts/validate.py b/templates/config-repository/scripts/validate.py index 465107f3..99f5d6c9 100755 --- a/templates/config-repository/scripts/validate.py +++ b/templates/config-repository/scripts/validate.py @@ -56,6 +56,9 @@ } FORBIDDEN_FILENAMES = re.compile(r"(?:^|/)(?:\.env(?:\..+)?|host\.env|ci-fleet\.env)$|\.(?:key|pem|p12|pfx)$", re.IGNORECASE) FORBIDDEN_DIRECTORIES = {"credentials", "private", "secrets"} +RFC_5737_NETWORKS = tuple( + ipaddress.ip_network(value) for value in ("192.0.2.0/24", "198.51.100.0/24", "203.0.113.0/24") +) class Validation: @@ -141,7 +144,14 @@ def scan_keys(value: Any, path: str = "$") -> None: break -def validate_docker_network_policy(policy: Any, path: str, max_runners: int, validation: Validation) -> tuple[int, int, list[dict[str, Any]]]: +def validate_docker_network_policy( + policy: Any, + path: str, + max_runners: int, + validation: Validation, + *, + strict: bool = False, +) -> tuple[int, int, list[dict[str, Any]]]: if not isinstance(policy, dict): validation.errors.append(f"{path}: must be an object") return 0, 0, [] @@ -187,6 +197,11 @@ def validate_docker_network_policy(policy: Any, path: str, max_runners: int, val if network.version != 4: validation.errors.append(f"{pool_path}.base: malformed address pool IPv4 prefix") return 0, 0, [] + if strict and any(network.overlaps(documentation) for documentation in RFC_5737_NETWORKS): + validation.errors.append( + f"{pool_path}.base: replace the RFC 5737 documentation address pool with a reviewed non-overlapping pool" + ) + return 0, 0, [] if size < network.prefixlen: validation.errors.append(f"{pool_path}.size: impossible subnet count for {base}") return 0, 0, [] @@ -197,8 +212,10 @@ def validate_docker_network_policy(policy: Any, path: str, max_runners: int, val validation.errors.append(f"{path}.default_address_pools[{left}].base: overlaps configured pool {right}") return 0, 0, [] configured = sum(1 << (item["size"] - item["network"].prefixlen) for item in parsed) - if configured < max_runners + reserve: - validation.errors.append(f"{path}: network capacity cannot satisfy max_runners plus reserve") + if configured < max_runners + reserve + 1: + validation.errors.append( + f"{path}: network capacity cannot satisfy max_runners + reserve_subnets + one controller Compose network" + ) return configured, reserve, parsed @@ -375,7 +392,13 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: network_policy = controller.get("docker_network_policy") capacity_maximum = maximum if state != "disabled" and type(maximum) is int and maximum > 0 else 0 if network_policy is not None: - validate_docker_network_policy(network_policy, f"{path}.docker_network_policy", capacity_maximum, validation) + validate_docker_network_policy( + network_policy, + f"{path}.docker_network_policy", + capacity_maximum, + validation, + strict=strict, + ) if isinstance(pool_name, str) and pool_name in pools and state != "disabled" and type(maximum) is int and maximum > 0: reserved_capacity[pool_name] += maximum @@ -567,6 +590,12 @@ def validate_transition( previous_evidence = previous_evidence_source.get(name, {}) old_reporting = old.get("status_reporting") new_reporting = new.get("status_reporting") + if "docker_network_policy" not in old and "docker_network_policy" in new: + validation.require( + old.get("engine_ref") == new.get("engine_ref"), + f"$.controllers.{name}.docker_network_policy", + "must be introduced in a later commit after the compatible engine_ref is active", + ) staged_capability_required = ( "status_reporting" not in old or ( From 80da5050b3e85e43627585b739310d6bb8064cfe Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Wed, 26 Aug 2026 17:51:34 -0500 Subject: [PATCH 6/8] fix: harden network policy rollout validation --- docs/DESIRED-STATE.md | 13 ++++--- engine-capabilities.json | 1 + scripts/desired_state.py | 13 +++++-- scripts/health.py | 2 + scripts/status_receiver.py | 2 +- scripts/test-install-worker-controller.sh | 15 +++++--- scripts/test_desired_state.py | 27 +++++++++---- scripts/test_health.py | 12 ++++++ scripts/test_status_receiver.py | 5 +++ scripts/validate.sh | 2 +- templates/config-repository/README.md | 25 ++++++++++-- .../config-repository/scripts/test_policy.py | 38 ++++++++++++++++++- .../config-repository/scripts/validate.py | 29 ++++++++++++-- 13 files changed, 152 insertions(+), 32 deletions(-) diff --git a/docs/DESIRED-STATE.md b/docs/DESIRED-STATE.md index 0be1dca7..5f98062d 100644 --- a/docs/DESIRED-STATE.md +++ b/docs/DESIRED-STATE.md @@ -49,11 +49,14 @@ validation rejects until the operator supplies a reviewed non-overlapping pool. `docker_network_policy` is optional only to preserve a staged upgrade path from older schema-v3 engines whose exact-key validator does not recognize it. Upgrade -an existing controller in two reviewed desired-state commits: first change only -`engine_ref` and verify that this compatible engine is active; then add the -reviewed network policy in a second commit. Do not add the field while the old -engine still performs reconciliation. Transition validation rejects a commit -that changes `engine_ref` while introducing the policy. +an existing controller in three reviewed desired-state commits. First change +only `engine_ref`. After routine reconciliation shows that exact engine is active, +record `docker_network_policy_config: true` for that controller and ref in +`engine-rollout-evidence.json`. Only then add the reviewed network policy without +changing the engine or evidence. Transition validation reads the evidence from +the previous integrated state, so a commit that adds evidence and policy together +cannot satisfy the gate. Do not add the field while the old engine still performs +reconciliation. This phase renders the policy solely for read-only health inspection. It does not write `daemon.json`, restart Docker, create or remove networks, prune diff --git a/engine-capabilities.json b/engine-capabilities.json index 12c4d6de..a5b6b839 100644 --- a/engine-capabilities.json +++ b/engine-capabilities.json @@ -1,6 +1,7 @@ { "schema_version": 1, "capabilities": { + "docker_network_policy_config": true, "status_reporting_config": true, "required_status_reporting": true } diff --git a/scripts/desired_state.py b/scripts/desired_state.py index cd37676c..42d788cf 100755 --- a/scripts/desired_state.py +++ b/scripts/desired_state.py @@ -31,6 +31,7 @@ HOST_OPTIONAL = {"CI_FLEET_RUNNER_TTL"} REQUIRED_STATUS_CAPABILITY = "required_status_reporting" STATUS_REPORTING_CONFIG_CAPABILITY = "status_reporting_config" +DOCKER_NETWORK_POLICY_CONFIG_CAPABILITY = "docker_network_policy_config" class DesiredStateError(ValueError): @@ -224,14 +225,17 @@ def build_rendered_env( state = controller["state"] configured_max = controller["max_runners"] effective_max = configured_max if state == "active" else 0 + network_policy_configured = "docker_network_policy" in controller network_policy = controller.get("docker_network_policy") configured_subnets, reserve_subnets, parsed_pools = (0, 0, []) - if network_policy is not None: + if network_policy_configured: configured_subnets, reserve_subnets, parsed_pools = validate_docker_network_policy( network_policy, path=f"$.controllers.{controller_id}.docker_network_policy", max_runners=configured_max if state != "disabled" else 0, ) + if DOCKER_NETWORK_POLICY_CONFIG_CAPABILITY not in (engine_capabilities or set()): + raise DesiredStateError("selected engine does not support Docker network policy configuration") short_commit = engine_commit[:12] rendered = { "CI_FLEET_CAPACITY_BUDGET": str(pool["capacity_budget"]), @@ -257,7 +261,7 @@ def build_rendered_env( "CI_FLEET_VERSION": short_commit, **validate_host_values(host_values), } - if network_policy is not None: + if network_policy_configured: rendered["CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_COUNT"] = str(len(parsed_pools)) rendered["CI_FLEET_DOCKER_NETWORK_RESERVE_SUBNETS"] = str(reserve_subnets) for index, pool_config in enumerate(parsed_pools): @@ -291,7 +295,7 @@ def build_rendered_env( "engine_repository": config["organization"]["delivery_engine"], "status_reporting_configured": reporting_configured, "status_reporting_required": reporting_required, - "docker_network_policy_configured": network_policy is not None, + "docker_network_policy_configured": network_policy_configured, "docker_network_default_address_pools": len(parsed_pools), "docker_network_reserve_subnets": reserve_subnets, "docker_network_configured_subnets": configured_subnets, @@ -361,6 +365,8 @@ def command_engine(args: argparse.Namespace) -> None: def command_validate_engine_capabilities(args: argparse.Namespace) -> None: capabilities = load_engine_capabilities(args.manifest) + if args.require_docker_network_policy_config and DOCKER_NETWORK_POLICY_CONFIG_CAPABILITY not in capabilities: + raise DesiredStateError("selected engine does not support Docker network policy configuration") if args.require_status_reporting_config and STATUS_REPORTING_CONFIG_CAPABILITY not in capabilities: raise DesiredStateError("selected engine does not support status reporting configuration") if args.require_status_reporting and REQUIRED_STATUS_CAPABILITY not in capabilities: @@ -400,6 +406,7 @@ def parse_args() -> argparse.Namespace: capabilities = subparsers.add_parser("validate-engine-capabilities", help="validate an engine capability declaration") capabilities.add_argument("--manifest", type=Path, required=True) + capabilities.add_argument("--require-docker-network-policy-config", action="store_true") capabilities.add_argument("--require-status-reporting-config", action="store_true") capabilities.add_argument("--require-status-reporting", action="store_true") capabilities.set_defaults(function=command_validate_engine_capabilities) diff --git a/scripts/health.py b/scripts/health.py index 5dc98a42..430ad560 100644 --- a/scripts/health.py +++ b/scripts/health.py @@ -362,6 +362,8 @@ def _docker_network_headroom(run: Runner, values: dict[str, str], *, docker_ok: occupied: list[list[tuple[int, int]]] = [[] for _ in pools] legacy_networks = 0 for name in [line.strip() for line in listed.stdout.splitlines() if line.strip()]: + if name == "bridge": + continue inspected = run(["docker", "network", "inspect", name]) if inspected.returncode != 0: refreshed = run(["docker", "network", "ls", "--format", "{{.Name}}"]) diff --git a/scripts/status_receiver.py b/scripts/status_receiver.py index 8c862d54..651a9988 100644 --- a/scripts/status_receiver.py +++ b/scripts/status_receiver.py @@ -242,7 +242,7 @@ def enum(value: Any, choices: set[str]) -> bool: if not isinstance(docker, dict) or set(docker) not in ({"healthy", "oom"}, {"healthy", "oom", "network"}) or not all(isinstance(docker[key], bool) for key in ("healthy", "oom")): raise StatusError(400, "invalid_report") network = docker.get("network") - if network is not None: + if "network" in docker: keys = {"configured", "used", "free", "legacy"} if not exact(network, keys) or not all(integer(network[key]) for key in keys) or network["used"] + network["free"] != network["configured"]: raise StatusError(400, "invalid_report") diff --git a/scripts/test-install-worker-controller.sh b/scripts/test-install-worker-controller.sh index 3d4c9a53..2b4c6043 100755 --- a/scripts/test-install-worker-controller.sh +++ b/scripts/test-install-worker-controller.sh @@ -240,11 +240,11 @@ git -C "$config_repo" config user.email fixture@example.invalid write_config() { local state=$1 maximum=$2 budget=$3 - local desired_engine=${4:-$engine_ref} reporting=${5:-false} - python3 - "$repo_root/templates/config-repository/fleet.json" "$config_repo/fleet.json" "$config_repo/engine-rollout-evidence.json" "$desired_engine" "$state" "$maximum" "$budget" "$reporting" <<'PY' + local desired_engine=${4:-$engine_ref} reporting=${5:-false} network_policy=${6:-present} + python3 - "$repo_root/templates/config-repository/fleet.json" "$config_repo/fleet.json" "$config_repo/engine-rollout-evidence.json" "$desired_engine" "$state" "$maximum" "$budget" "$reporting" "$network_policy" <<'PY' import json import sys -source, target, evidence_target, engine_ref, state, maximum, budget, reporting = sys.argv[1:] +source, target, evidence_target, engine_ref, state, maximum, budget, reporting, network_policy = sys.argv[1:] value = json.load(open(source, encoding="utf-8")) value["organization"]["slug"] = "fixture-org" value["runner_pools"]["trusted-ci"]["allowed_repositories"] = ["fixture-org/example-app"] @@ -254,6 +254,8 @@ controller["docker_network_policy"]["default_address_pools"][0]["base"] = "10.64 controller["engine_ref"] = engine_ref controller["state"] = state controller["max_runners"] = int(maximum) +if network_policy == "omit": + controller.pop("docker_network_policy") if reporting == "omit": controller.pop("status_reporting", None) else: @@ -273,6 +275,7 @@ with open(evidence_target, "w", encoding="utf-8") as handle: "engine_ref": engine_ref, "status_reporting_config": True, "required_status_reporting": True, + "docker_network_policy_config": network_policy != "omit", }, }, }, handle, indent=2) @@ -683,11 +686,11 @@ unset FAKE_RUNNER_STATE_ONCE FAKE_COMPOSE_LOG # Public pre-health engine fixture; do not depend on a local remote-tracking ref. legacy_engine_ref=af9c0c13cd12866ce75dd6c43a4cda01915507e1 -legacy_disabled_ref=$(write_config active 1 1 "$legacy_engine_ref" false) +legacy_disabled_ref=$(write_config active 1 1 "$legacy_engine_ref" false omit) expect_failure 'selected engine does not support status reporting configuration' "$installer" --upgrade "${base_args[@]}" --ref "$legacy_disabled_ref" -legacy_required_ref=$(write_config active 1 1 "$legacy_engine_ref" true) +legacy_required_ref=$(write_config active 1 1 "$legacy_engine_ref" true omit) expect_failure 'selected engine does not advertise required status reporting' "$installer" --upgrade "${base_args[@]}" --ref "$legacy_required_ref" -legacy_ref=$(write_config active 1 1 "$legacy_engine_ref" omit) +legacy_ref=$(write_config active 1 1 "$legacy_engine_ref" omit omit) export FAKE_ENGINE_REF=$legacy_engine_ref export FAKE_RUNNER_IMAGE=ci-fleet-runner:${legacy_engine_ref:0:12} export FAKE_CONTROLLER_IMAGE=ci-fleet-controller:${legacy_engine_ref:0:12} diff --git a/scripts/test_desired_state.py b/scripts/test_desired_state.py index bdcfa59c..79933f90 100755 --- a/scripts/test_desired_state.py +++ b/scripts/test_desired_state.py @@ -55,7 +55,7 @@ def render(self, value: dict | None = None, capabilities: set[str] | None = None config_repository="example-org/example-fleet-config", config_ref=CONFIG_COMMIT, docker_gid=998, - engine_capabilities={"status_reporting_config"} if capabilities is None else capabilities, + engine_capabilities={"status_reporting_config", "docker_network_policy_config"} if capabilities is None else capabilities, ) def test_active_controller_renders_configured_capacity(self) -> None: @@ -72,7 +72,10 @@ def test_status_reporting_requires_fixed_host_local_configuration(self) -> None: "enabled": True, "config_file": "/etc/ci-fleet/monitoring.env", } - environment, _ = self.render(value, {"status_reporting_config", "required_status_reporting"}) + environment, _ = self.render( + value, + {"status_reporting_config", "required_status_reporting", "docker_network_policy_config"}, + ) self.assertEqual(environment["CI_FLEET_STATUS_REPORTING_REQUIRED"], "1") value["controllers"]["example-ci-01"]["status_reporting"]["config_file"] = "https://example.invalid/v1/status" with tempfile.TemporaryDirectory() as directory: @@ -94,7 +97,7 @@ def test_status_reporting_requires_engine_capability(self) -> None: "config_file": "/etc/ci-fleet/monitoring.env", } with self.assertRaisesRegex(DesiredStateError, "does not advertise"): - self.render(value, set()) + self.render(value, {"docker_network_policy_config"}) with tempfile.TemporaryDirectory() as directory: manifest = Path(directory) / "engine-capabilities.json" manifest.write_text("not json", encoding="utf-8") @@ -110,7 +113,7 @@ def test_status_reporting_requires_engine_capability(self) -> None: def test_omitted_status_reporting_accepts_older_engine(self) -> None: value = config() value["controllers"]["example-ci-01"].pop("status_reporting", None) - environment, metadata = self.render(value, set()) + environment, metadata = self.render(value, {"docker_network_policy_config"}) self.assertNotIn("CI_FLEET_STATUS_REPORTING_REQUIRED", environment) self.assertFalse(metadata["status_reporting_configured"]) self.assertFalse(metadata["status_reporting_required"]) @@ -122,8 +125,8 @@ def test_disabled_status_reporting_requires_schema_capability(self) -> None: "config_file": "/etc/ci-fleet/monitoring.env", } with self.assertRaisesRegex(DesiredStateError, "does not support status reporting configuration"): - self.render(value, set()) - environment, metadata = self.render(value, {"status_reporting_config"}) + self.render(value, {"docker_network_policy_config"}) + environment, metadata = self.render(value, {"status_reporting_config", "docker_network_policy_config"}) self.assertNotIn("CI_FLEET_STATUS_REPORTING_REQUIRED", environment) self.assertTrue(metadata["status_reporting_configured"]) self.assertFalse(metadata["status_reporting_required"]) @@ -143,10 +146,20 @@ def test_docker_network_policy_renders_read_only_inspection_values(self) -> None def test_docker_network_policy_can_be_staged_after_engine_upgrade(self) -> None: value = config() value["controllers"]["example-ci-01"].pop("docker_network_policy", None) - environment, metadata = self.render(value) + environment, metadata = self.render(value, set()) self.assertNotIn("CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_COUNT", environment) self.assertFalse(metadata["docker_network_policy_configured"]) + def test_docker_network_policy_requires_engine_capability(self) -> None: + with self.assertRaisesRegex(DesiredStateError, "network policy configuration"): + self.render(config(), {"status_reporting_config"}) + + def test_render_rejects_present_null_docker_network_policy(self) -> None: + value = config() + value["controllers"]["example-ci-01"]["docker_network_policy"] = None + with self.assertRaisesRegex(DesiredStateError, "must be an object"): + self.render(value) + def test_docker_network_policy_requires_capacity_for_reserve(self) -> None: value = config() value["controllers"]["example-ci-01"]["max_runners"] = 2 diff --git a/scripts/test_health.py b/scripts/test_health.py index 119f40b7..28fc0c20 100644 --- a/scripts/test_health.py +++ b/scripts/test_health.py @@ -143,6 +143,18 @@ def run(args): result = health._docker_network_headroom(run, network_policy_values(), docker_ok=True) self.assertEqual(result["legacy"], 1) + def test_builtin_bridge_is_ignored_but_similar_user_network_is_legacy(self) -> None: + inspected = json.dumps([{"IPAM": {"Config": [{"Subnet": "172.17.0.0/16"}]}}]) + for name, expected in (("bridge", (0, "healthy")), ("bridge-copy", (1, "warning"))): + with self.subTest(name=name): + def run(args): + if args[:3] == ["docker", "network", "ls"]: + return health.subprocess.CompletedProcess(args, 0, f"{name}\n", "") + return health.subprocess.CompletedProcess(args, 0, inspected, "") + + result = health._docker_network_headroom(run, network_policy_values(), docker_ok=True) + self.assertEqual((result["legacy"], result["state"]), expected) + def test_broader_network_consumes_all_allocation_slots(self) -> None: def run(args): if args[:3] == ["docker", "network", "ls"]: diff --git a/scripts/test_status_receiver.py b/scripts/test_status_receiver.py index cc479e64..b8725a97 100644 --- a/scripts/test_status_receiver.py +++ b/scripts/test_status_receiver.py @@ -108,6 +108,11 @@ def test_network_aggregates_are_optional_but_strict_when_present(self) -> None: report["docker"]["network"] = malformed self.assert_status_error(400, "invalid_report", lambda report=report: self.submit(report, timestamp=1_001, nonce="b" * 32)) + def test_present_null_network_is_rejected(self) -> None: + report = valid_report() + report["docker"]["network"] = None + self.assert_status_error(400, "invalid_report", lambda: self.submit(report)) + def test_non_object_docker_sections_are_rejected_cleanly(self) -> None: for index, malformed in enumerate((None, [], "docker", 1)): report = valid_report(generated_at=1_010 + index) diff --git a/scripts/validate.sh b/scripts/validate.sh index 9660aaa2..271977ea 100755 --- a/scripts/validate.sh +++ b/scripts/validate.sh @@ -23,7 +23,7 @@ python3 scripts/test_health.py python3 scripts/test_status_receiver.py python3 scripts/test_quickstart.py python3 -m json.tool schemas/status-report-v1.json >/dev/null -python3 scripts/desired_state.py validate-engine-capabilities --manifest engine-capabilities.json --require-status-reporting-config --require-status-reporting >/dev/null +python3 scripts/desired_state.py validate-engine-capabilities --manifest engine-capabilities.json --require-docker-network-policy-config --require-status-reporting-config --require-status-reporting >/dev/null python3 .github/actions/plan/plan.py --plan examples/project/scripts/ci/plan.json --group fast >/dev/null python3 .github/actions/plan/plan.py --plan examples/project/scripts/ci/plan.json --group full >/dev/null scripts/test-capacity-preflight.sh diff --git a/templates/config-repository/README.md b/templates/config-repository/README.md index 5744e77b..9fe83822 100644 --- a/templates/config-repository/README.md +++ b/templates/config-repository/README.md @@ -78,6 +78,20 @@ This staging prevents an older active manager from rejecting the new property be can upgrade itself. Endpoint and key values remain host-local and never enter Git. +The same per-controller evidence record may declare +`docker_network_policy_config`. Existing status-reporting records may omit this +new boolean, so their behavior does not change. A complete record has this shape +after an operator has verified the named engine is active: + +```json +{ + "engine_ref": "1111111111111111111111111111111111111111", + "status_reporting_config": false, + "required_status_reporting": false, + "docker_network_policy_config": true +} +``` + The controller ID is how a target host selects its declaration. A location is a non-sensitive logical slug such as `primary-site` or `remote-site`, never an address. Runtime-generated configuration and credentials remain host-local. ### Pool capacity is infrastructure policy @@ -96,10 +110,13 @@ Strict validation rejects them until the private configuration uses a reviewed non-overlapping pool. The field is optional solely for staged upgrades from older schema-v3 engines. -First pin and activate this compatible engine without adding the field. In a -second reviewed desired-state commit, add the reviewed policy. The older engine -rejects the new key, so combining those steps prevents reconciliation. Transition -validation enforces the separate commits for existing controllers. +First pin and activate a compatible engine without adding the field. In a second +reviewed desired-state commit, record the active ref and +`docker_network_policy_config: true` in `engine-rollout-evidence.json`. Add the +reviewed policy in a third commit, retaining the same ref and evidence. The older +engine rejects the new key, so skipped commits must not satisfy the gate. +Transition validation requires the activation evidence to exist in the previous +integrated state. The public engine renders these values only for read-only health inspection. This phase detects low water, exhaustion, failed inspection, and legacy diff --git a/templates/config-repository/scripts/test_policy.py b/templates/config-repository/scripts/test_policy.py index 65b940ce..571ecc69 100755 --- a/templates/config-repository/scripts/test_policy.py +++ b/templates/config-repository/scripts/test_policy.py @@ -103,6 +103,11 @@ def test_docker_network_policy_is_optional_and_capacity_checked_when_present(sel } self.assert_rejected(config, "capacity") + def test_present_null_docker_network_policy_is_rejected(self) -> None: + config = copy.deepcopy(reference_config()) + first_controller(config)["docker_network_policy"] = None + self.assert_rejected(config, "must be an object") + def test_docker_network_policy_reserves_controller_compose_subnet(self) -> None: config = copy.deepcopy(reference_config()) first_controller(config)["docker_network_policy"] = { @@ -234,13 +239,42 @@ def test_docker_network_policy_requires_a_separate_engine_rollout(self) -> None: validate_transition(previous, current, {}, validation) self.assertTrue(any("docker_network_policy" in error and "later commit" in error for error in validation.errors), validation.errors) - def test_docker_network_policy_accepts_an_unchanged_staged_engine(self) -> None: + def test_docker_network_policy_requires_previous_applied_engine_evidence(self) -> None: previous = reference_config() first_controller(previous).pop("docker_network_policy") current = copy.deepcopy(previous) first_controller(current)["docker_network_policy"] = docker_network_policy() + controller = next(iter(current["controllers"])) + evidence = { + controller: { + "engine_ref": first_controller(current)["engine_ref"], + "status_reporting_config": False, + "required_status_reporting": False, + "docker_network_policy_config": True, + } + } + validation = Validation() - validate_transition(previous, current, {}, validation) + validate_transition(previous, current, evidence, validation, {}) + self.assertTrue(any("previous integrated" in error for error in validation.errors), validation.errors) + + validation = Validation() + validate_transition(previous, current, evidence, validation, evidence) + self.assertEqual(validation.errors, []) + + def test_rollout_evidence_accepts_network_policy_capability(self) -> None: + evidence = { + "engine_ref": "1" * 40, + "status_reporting_config": False, + "required_status_reporting": False, + "docker_network_policy_config": True, + } + validation = Validation() + refs = validate_rollout_evidence({ + "schema_version": 1, + "status_reporting_engine_capabilities": {"example-ci-01": evidence}, + }, validation) + self.assertEqual(refs, {"example-ci-01": evidence}) self.assertEqual(validation.errors, []) def test_retained_reporting_requires_target_engine_capabilities(self) -> None: diff --git a/templates/config-repository/scripts/validate.py b/templates/config-repository/scripts/validate.py index 99f5d6c9..a88f0106 100755 --- a/templates/config-repository/scripts/validate.py +++ b/templates/config-repository/scripts/validate.py @@ -391,7 +391,7 @@ def validate_config(config: Any, validation: Validation, strict: bool) -> None: validation.require(type(memory) is int and memory >= 512, f"{path}.runner_resources.memory_mib", "must be at least 512 MiB") network_policy = controller.get("docker_network_policy") capacity_maximum = maximum if state != "disabled" and type(maximum) is int and maximum > 0 else 0 - if network_policy is not None: + if "docker_network_policy" in controller: validate_docker_network_policy( network_policy, f"{path}.docker_network_policy", @@ -514,21 +514,32 @@ def validate_rollout_evidence(value: Any, validation: Validation) -> dict[str, d path = f"engine-rollout-evidence.json.status_reporting_engine_capabilities.{controller}" controller_valid = bool(SLUG.fullmatch(controller)) validation.require(controller_valid, path, "controller ID must be a lowercase slug") - if not validation.exact_keys(evidence, path, {"engine_ref", "status_reporting_config", "required_status_reporting"}): + if not validation.exact_keys( + evidence, + path, + {"engine_ref", "status_reporting_config", "required_status_reporting"}, + {"docker_network_policy_config"}, + ): continue ref = evidence.get("engine_ref") configured = evidence.get("status_reporting_config") required = evidence.get("required_status_reporting") + network_policy = evidence.get("docker_network_policy_config") ref_valid = isinstance(ref, str) and bool(COMMIT_SHA.fullmatch(ref)) and ref != "0" * 40 validation.require(ref_valid, f"{path}.engine_ref", "must be a nonzero full lowercase commit SHA") validation.require(type(configured) is bool, f"{path}.status_reporting_config", "must be a boolean") validation.require(type(required) is bool, f"{path}.required_status_reporting", "must be a boolean") - if controller_valid and ref_valid and type(configured) is bool and type(required) is bool: + if "docker_network_policy_config" in evidence: + validation.require(type(network_policy) is bool, f"{path}.docker_network_policy_config", "must be a boolean") + network_policy_valid = "docker_network_policy_config" not in evidence or type(network_policy) is bool + if controller_valid and ref_valid and type(configured) is bool and type(required) is bool and network_policy_valid: valid[controller] = { "engine_ref": ref, "status_reporting_config": configured, "required_status_reporting": required, } + if "docker_network_policy_config" in evidence: + valid[controller]["docker_network_policy_config"] = network_policy return valid @@ -596,6 +607,18 @@ def validate_transition( f"$.controllers.{name}.docker_network_policy", "must be introduced in a later commit after the compatible engine_ref is active", ) + validation.require( + previous_evidence.get("engine_ref") == new.get("engine_ref") + and previous_evidence.get("docker_network_policy_config") is True, + f"$.controllers.{name}.docker_network_policy", + "requires reviewed evidence from the previous integrated state that this controller activated the same engine_ref with Docker network policy configuration capability", + ) + validation.require( + current_evidence.get("engine_ref") == new.get("engine_ref") + and current_evidence.get("docker_network_policy_config") is True, + f"$.controllers.{name}.docker_network_policy", + "requires retaining Docker network policy rollout evidence for this controller and engine_ref", + ) staged_capability_required = ( "status_reporting" not in old or ( From 9679136e1b66bedbe07a82ca835f484fae918079 Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Wed, 26 Aug 2026 20:33:30 -0500 Subject: [PATCH 7/8] fix: enforce Docker network capacity policy --- AGENTS.md | 2 +- README.md | 6 ++ docs/DESIRED-STATE.md | 22 +++-- scripts/desired_state.py | 23 +++-- scripts/health.py | 31 +++++-- scripts/test_desired_state.py | 66 ++++++++++++++- scripts/test_health.py | 67 ++++++++++++--- templates/config-repository/AGENTS.md | 2 +- templates/config-repository/README.md | 24 ++++-- .../examples/multi-host/fleet.json | 2 + templates/config-repository/fleet.json | 1 + templates/config-repository/fleet.schema.json | 5 +- templates/config-repository/scripts/init.py | 9 +- .../config-repository/scripts/test_policy.py | 83 ++++++++++++++++++- .../config-repository/scripts/validate.py | 16 ++-- 15 files changed, 298 insertions(+), 61 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index bbd965a1..320dd96d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -25,7 +25,7 @@ Agents modifying this repository or adapting a project MUST apply these document - Keep original project code compatible with the Unlicense. - Do not copy GPL-licensed implementation code into this repository. - Preserve required notices for third-party code or substantial examples. -- Never commit credentials, registration tokens, private keys, real environment files, internal host inventories, private IP addresses, or unredacted infrastructure reports. +- Never commit credentials, registration tokens, private keys, real environment files, internal host inventories, host addresses, service addresses, or unredacted infrastructure reports. The sole address exception is reviewed Docker `default_address_pools[].base` CIDRs in private Git-authored fleet policy because they are required capacity policy. This exception does not allow credentials, SSH details, secrets, VM, storage, or backup identifiers, unrelated infrastructure addresses, or rendered runtime configuration. - Use examples and placeholders for organization-specific configuration. - Store long-lived GitHub credentials only in a controller or external secret manager. - Do not expose long-lived controller credentials to job runner containers. diff --git a/README.md b/README.md index 85bdab0e..b5d987b3 100644 --- a/README.md +++ b/README.md @@ -95,6 +95,12 @@ GitHub runner-group policy decides which repositories may schedule work. A share | Project repository | Test Dockerfile, services, fixtures, migrations, test plan, `scripts/ci/run.sh` | Fleet controller credentials or host-specific setup | | Private installation configuration | Organization settings, repository authorization, logical controller state, capacity budgets, network policy, and required secret names | Secret values, host addresses, project runtime dependencies, or test logic | +One narrow exception lets a private installation configuration commit exact, +reviewed Docker `default_address_pools[].base` CIDRs. These ranges are allocation +capacity policy, not host addresses, credentials, host identity, or routable +service endpoints. VM, storage, backup, SSH, rendered runtime, and unrelated +infrastructure details remain outside Git. + A public application can use the same fleet indirectly. Its public repository keeps pull-request validation unprivileged, while a separate private delivery repository checks out an approved immutable commit and performs protected CI, release, or deployment work. The public repository itself never receives privileged runner-group access or fleet credentials. See [Public projects, private delivery, and private configuration](docs/PUBLIC-PRIVATE-CONFIGURATION.md). ## What makes it different? diff --git a/docs/DESIRED-STATE.md b/docs/DESIRED-STATE.md index 5f98062d..3de7a4d4 100644 --- a/docs/DESIRED-STATE.md +++ b/docs/DESIRED-STATE.md @@ -13,6 +13,13 @@ Schema v3 makes a reviewed private configuration repository the authority for co Host addresses, VM IDs, storage names, backup identifiers, SSH details, tokens, private keys, and rendered `.env` files are rejected from the Git-authored configuration. +Private policy has one explicit address exception. Each reviewed +`docker_network_policy.default_address_pools[].base` CIDR is committed because +the controller must render and inspect that exact allocation pool. It is capacity +policy, not a host address, host identity, credential, or routable service +endpoint. The exception does not admit any other infrastructure address or +runtime detail. + ## Schema v3 Each runner pool declares: @@ -33,19 +40,22 @@ Each controller has a unique object key and declares: - a full pinned ci-fleet engine commit; - a zero managed minimum and reviewed maximum runner capacity; - CPU cores and memory per ephemeral runner; -- reviewed Docker default-address pools and a reserved subnet count. +- reviewed Docker default-address pools, a positive per-runner network bound, + and a reserved subnet count. Active and drained controllers reserve their configured maximum against the pool budget. A drained controller has zero effective runtime capacity but keeps its reservation, so an undrain cannot silently overcommit the pool. Disabled controllers reserve no capacity. The Docker network policy uses IPv4 CIDR `base` values and a Docker subnet -prefix `size`. Validation rejects malformed or overlapping pools, impossible -prefix relationships, and active or drained policies with fewer subnets than -`max_runners + reserve_subnets + 1`. The final subnet is reserved for the +prefix `size` no longer than `/29`, which leaves enough addresses for an +ordinary Compose network. Validation rejects malformed or overlapping pools, +allocation prefixes broader than their base, and active or drained policies with fewer subnets than +`max_runners * networks_per_runner + reserve_subnets + 1`. The final subnet is reserved for the persistent controller Compose network. Disabled controllers do not reserve runner subnet capacity, but their retained policy must still cover the reserve and controller network. Real pool values belong only in the private desired-state -repository. Public examples use RFC 5737 documentation ranges, which strict -validation rejects until the operator supplies a reviewed non-overlapping pool. +repository under the narrow address-pool exception above. Public examples use +RFC 5737 documentation ranges, which strict validation rejects until the operator +supplies a reviewed operational Docker pool CIDR. `docker_network_policy` is optional only to preserve a staged upgrade path from older schema-v3 engines whose exact-key validator does not recognize it. Upgrade diff --git a/scripts/desired_state.py b/scripts/desired_state.py index 42d788cf..8e28b96d 100755 --- a/scripts/desired_state.py +++ b/scripts/desired_state.py @@ -139,10 +139,10 @@ def validate_host_values(values: dict[str, str]) -> dict[str, str]: } -def validate_docker_network_policy(policy: dict[str, Any], *, path: str, max_runners: int) -> tuple[int, int, list[dict[str, Any]]]: +def validate_docker_network_policy(policy: dict[str, Any], *, path: str, max_runners: int) -> tuple[int, int, int, list[dict[str, Any]]]: if not isinstance(policy, dict): raise DesiredStateError(f"{path}: must be an object") - required = {"default_address_pools", "reserve_subnets"} + required = {"default_address_pools", "networks_per_runner", "reserve_subnets"} if set(policy) != required: unknown = sorted(set(policy) - required) missing = sorted(required - set(policy)) @@ -155,6 +155,9 @@ def validate_docker_network_policy(policy: dict[str, Any], *, path: str, max_run reserve = policy.get("reserve_subnets") if type(reserve) is not int or reserve < 1: raise DesiredStateError(f"{path}.reserve_subnets: must be a positive integer") + networks_per_runner = policy.get("networks_per_runner") + if type(networks_per_runner) is not int or networks_per_runner < 1: + raise DesiredStateError(f"{path}.networks_per_runner: must be a positive integer") pools = policy.get("default_address_pools") if type(pools) is not list or not pools: raise DesiredStateError(f"{path}.default_address_pools: must be a non-empty list") @@ -167,8 +170,8 @@ def validate_docker_network_policy(policy: dict[str, Any], *, path: str, max_run size = pool.get("size") if not isinstance(base, str): raise DesiredStateError(f"{pool_path}.base: must be a CIDR prefix") - if type(size) is not int or size < 0 or size > 32: - raise DesiredStateError(f"{pool_path}.size: must be an IPv4 prefix length between 0 and 32") + if type(size) is not int or size < 0 or size > 29: + raise DesiredStateError(f"{pool_path}.size: must be an IPv4 prefix length between 0 and 29") try: network = ipaddress.ip_network(base, strict=True) except ValueError as exc: @@ -186,11 +189,11 @@ def validate_docker_network_policy(policy: dict[str, Any], *, path: str, max_run f"{path}.default_address_pools[{left}].base: overlaps configured pool {right}" ) configured = sum(1 << (item["size"] - item["network"].prefixlen) for item in parsed) - if configured < max_runners + reserve + 1: + if configured < max_runners * networks_per_runner + reserve + 1: raise DesiredStateError( - f"{path}: network capacity cannot satisfy max_runners + reserve_subnets + one controller Compose network" + f"{path}: network capacity cannot satisfy max_runners * networks_per_runner + reserve_subnets + one controller Compose network" ) - return configured, reserve, parsed + return configured, reserve, networks_per_runner, parsed def select_controller(config: dict[str, Any], controller_id: str) -> tuple[dict[str, Any], dict[str, Any]]: @@ -227,9 +230,9 @@ def build_rendered_env( effective_max = configured_max if state == "active" else 0 network_policy_configured = "docker_network_policy" in controller network_policy = controller.get("docker_network_policy") - configured_subnets, reserve_subnets, parsed_pools = (0, 0, []) + configured_subnets, reserve_subnets, networks_per_runner, parsed_pools = (0, 0, 0, []) if network_policy_configured: - configured_subnets, reserve_subnets, parsed_pools = validate_docker_network_policy( + configured_subnets, reserve_subnets, networks_per_runner, parsed_pools = validate_docker_network_policy( network_policy, path=f"$.controllers.{controller_id}.docker_network_policy", max_runners=configured_max if state != "disabled" else 0, @@ -263,6 +266,7 @@ def build_rendered_env( } if network_policy_configured: rendered["CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_COUNT"] = str(len(parsed_pools)) + rendered["CI_FLEET_DOCKER_NETWORKS_PER_RUNNER"] = str(networks_per_runner) rendered["CI_FLEET_DOCKER_NETWORK_RESERVE_SUBNETS"] = str(reserve_subnets) for index, pool_config in enumerate(parsed_pools): rendered[f"CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_{index}_BASE"] = pool_config["base"] @@ -297,6 +301,7 @@ def build_rendered_env( "status_reporting_required": reporting_required, "docker_network_policy_configured": network_policy_configured, "docker_network_default_address_pools": len(parsed_pools), + "docker_networks_per_runner": networks_per_runner, "docker_network_reserve_subnets": reserve_subnets, "docker_network_configured_subnets": configured_subnets, } diff --git a/scripts/health.py b/scripts/health.py index 430ad560..6c30645f 100644 --- a/scripts/health.py +++ b/scripts/health.py @@ -331,7 +331,7 @@ def _parse_network_pool(values: dict[str, str], index: int) -> tuple[ipaddress.I subnet_size = int(size) except ValueError: return None - if network.version != 4 or subnet_size < network.prefixlen or subnet_size > 32: + if network.version != 4 or subnet_size < network.prefixlen or subnet_size > 29: return None return network, subnet_size @@ -340,6 +340,8 @@ def _docker_network_headroom(run: Runner, values: dict[str, str], *, docker_ok: empty = {"configured": 0, "used": 0, "free": 0, "reserve": 0, "legacy": 0, "state": "unavailable"} try: configured_count = int(values.get("CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_COUNT", "0")) + configured_max = int(values.get("CI_FLEET_CONFIGURED_MAX_RUNNERS", values.get("CI_FLEET_MAX_RUNNERS", "0"))) + networks_per_runner = int(values.get("CI_FLEET_DOCKER_NETWORKS_PER_RUNNER", "1")) reserve = int(values.get("CI_FLEET_DOCKER_NETWORK_RESERVE_SUBNETS", "0")) except ValueError: return empty @@ -353,17 +355,16 @@ def _docker_network_headroom(run: Runner, values: dict[str, str], *, docker_ok: if pool is None: return empty pools.append(pool) - if not pools or reserve < 1: + if not pools or configured_max < 0 or networks_per_runner < 1 or reserve < 1: return empty listed = run(["docker", "network", "ls", "--format", "{{.Name}}"]) if listed.returncode != 0: return empty configured = sum(1 << (size - pool.prefixlen) for pool, size in pools) occupied: list[list[tuple[int, int]]] = [[] for _ in pools] + bridge_occupied: list[list[tuple[int, int]]] = [[] for _ in pools] legacy_networks = 0 for name in [line.strip() for line in listed.stdout.splitlines() if line.strip()]: - if name == "bridge": - continue inspected = run(["docker", "network", "inspect", name]) if inspected.returncode != 0: refreshed = run(["docker", "network", "ls", "--format", "{{.Name}}"]) @@ -397,7 +398,7 @@ def _docker_network_headroom(run: Runner, values: dict[str, str], *, docker_ok: continue subnets.append(network) if not subnets: - if saw_ipv6 or name in {"host", "none"}: + if saw_ipv6 or name in {"bridge", "host", "none"}: continue legacy_networks += 1 continue @@ -411,13 +412,16 @@ def _docker_network_headroom(run: Runner, values: dict[str, str], *, docker_ok: continue block_size = 1 << (32 - size) pool_start = int(pool.network_address) - occupied[index].append(((first - pool_start) // block_size, (last - pool_start) // block_size)) + interval = ((first - pool_start) // block_size, (last - pool_start) // block_size) + occupied[index].append(interval) + if name == "bridge": + bridge_occupied[index].append(interval) overlaps += 1 if overlaps == 0: network_legacy = True elif overlaps != 1 or not any(subnet.subnet_of(pool) and subnet.prefixlen == size for pool, size in pools): network_legacy = True - if network_legacy: + if network_legacy and name != "bridge": legacy_networks += 1 used = 0 for intervals in occupied: @@ -428,10 +432,21 @@ def _docker_network_headroom(run: Runner, values: dict[str, str], *, docker_ok: elif stop > end: used += stop - end end = max(end, stop) + bridge_used = 0 + for intervals in bridge_occupied: + end = -1 + for start, stop in sorted(intervals): + if start > end: + bridge_used += stop - start + 1 + elif stop > end: + bridge_used += stop - end + end = max(end, stop) free = max(configured - used, 0) + policy_max = 0 if values.get("CI_FLEET_CONTROLLER_STATE") == "disabled" else configured_max + required = policy_max * networks_per_runner + reserve + 1 if free == 0: state = "critical" - elif legacy_networks > 0 or free <= reserve: + elif legacy_networks > 0 or configured - bridge_used < required or free <= reserve: state = "warning" else: state = "healthy" diff --git a/scripts/test_desired_state.py b/scripts/test_desired_state.py index 79933f90..2b88ee8d 100755 --- a/scripts/test_desired_state.py +++ b/scripts/test_desired_state.py @@ -39,6 +39,7 @@ def host_values() -> dict[str, str]: def docker_network_policy() -> dict: return { + "networks_per_runner": 1, "reserve_subnets": 1, "default_address_pools": [ {"base": "198.51.100.0/24", "size": 28}, @@ -133,14 +134,18 @@ def test_disabled_status_reporting_requires_schema_capability(self) -> None: def test_docker_network_policy_renders_read_only_inspection_values(self) -> None: value = config() - value["controllers"]["example-ci-01"]["docker_network_policy"] = docker_network_policy() + policy = docker_network_policy() + policy["networks_per_runner"] = 2 + value["controllers"]["example-ci-01"]["docker_network_policy"] = policy environment, metadata = self.render(value) self.assertEqual(environment["CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_COUNT"], "1") self.assertEqual(environment["CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_0_BASE"], "198.51.100.0/24") self.assertEqual(environment["CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_0_SIZE"], "28") + self.assertEqual(environment["CI_FLEET_DOCKER_NETWORKS_PER_RUNNER"], "2") self.assertEqual(environment["CI_FLEET_DOCKER_NETWORK_RESERVE_SUBNETS"], "1") self.assertTrue(metadata["docker_network_policy_configured"]) self.assertEqual(metadata["docker_network_default_address_pools"], 1) + self.assertEqual(metadata["docker_networks_per_runner"], 2) self.assertEqual(metadata["docker_network_reserve_subnets"], 1) def test_docker_network_policy_can_be_staged_after_engine_upgrade(self) -> None: @@ -164,8 +169,9 @@ def test_docker_network_policy_requires_capacity_for_reserve(self) -> None: value = config() value["controllers"]["example-ci-01"]["max_runners"] = 2 value["controllers"]["example-ci-01"]["docker_network_policy"] = { + "networks_per_runner": 1, "reserve_subnets": 1, - "default_address_pools": [{"base": "198.51.100.0/30", "size": 30}], + "default_address_pools": [{"base": "198.51.100.0/29", "size": 29}], } with tempfile.TemporaryDirectory() as directory: path = Path(directory) / "fleet.json" @@ -177,13 +183,67 @@ def test_docker_network_policy_reserves_controller_compose_subnet(self) -> None: with self.assertRaisesRegex(DesiredStateError, "controller Compose network"): validate_docker_network_policy( { + "networks_per_runner": 1, "reserve_subnets": 1, - "default_address_pools": [{"base": "198.51.100.0/30", "size": 31}], + "default_address_pools": [{"base": "198.51.100.0/28", "size": 29}], }, path="$.controllers.example-ci-01.docker_network_policy", max_runners=1, ) + def test_docker_network_policy_accepts_29_and_rejects_smaller_allocations(self) -> None: + policy = { + "networks_per_runner": 1, + "reserve_subnets": 1, + "default_address_pools": [{"base": "198.51.100.0/24", "size": 29}], + } + validate_docker_network_policy( + policy, + path="$.controllers.example-ci-01.docker_network_policy", + max_runners=1, + ) + for size in (30, 31, 32): + with self.subTest(size=size): + policy["default_address_pools"][0]["size"] = size + with self.assertRaisesRegex(DesiredStateError, "between 0 and 29"): + validate_docker_network_policy( + policy, + path="$.controllers.example-ci-01.docker_network_policy", + max_runners=1, + ) + + def test_docker_network_policy_accounts_for_every_runner_network(self) -> None: + policy = { + "networks_per_runner": 2, + "reserve_subnets": 1, + "default_address_pools": [{"base": "198.51.100.0/27", "size": 29}], + } + with self.assertRaisesRegex(DesiredStateError, r"max_runners \* networks_per_runner"): + validate_docker_network_policy( + policy, + path="$.controllers.example-ci-01.docker_network_policy", + max_runners=2, + ) + policy["default_address_pools"].append({"base": "203.0.113.0/28", "size": 29}) + configured, reserve, networks_per_runner, _ = validate_docker_network_policy( + policy, + path="$.controllers.example-ci-01.docker_network_policy", + max_runners=2, + ) + self.assertEqual((configured, reserve, networks_per_runner), (6, 1, 2)) + + def test_disabled_docker_network_policy_keeps_reserve_and_controller_capacity(self) -> None: + configured, _, _, _ = validate_docker_network_policy( + { + "networks_per_runner": 100, + "reserve_subnets": 1, + "default_address_pools": [{"base": "198.51.100.0/28", "size": 29}], + }, + path="$.controllers.example-ci-01.docker_network_policy", + max_runners=0, + ) + self.assertEqual(configured, 2) + def test_drained_controller_renders_zero_effective_capacity(self) -> None: value = config() value["controllers"]["example-ci-01"]["state"] = "drained" diff --git a/scripts/test_health.py b/scripts/test_health.py index 28fc0c20..063313cc 100644 --- a/scripts/test_health.py +++ b/scripts/test_health.py @@ -44,6 +44,7 @@ def network_policy_values() -> dict[str, str]: "CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_COUNT": "1", "CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_0_BASE": "198.51.100.0/24", "CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_0_SIZE": "28", + "CI_FLEET_DOCKER_NETWORKS_PER_RUNNER": "1", "CI_FLEET_DOCKER_NETWORK_RESERVE_SUBNETS": "1", } @@ -114,6 +115,17 @@ def test_legacy_status_report_omits_unconfigured_docker_network(self) -> None: self.assertEqual(network["state"], "not_configured") self.assertNotIn("network", report["docker"]) + def test_network_pool_parser_accepts_29_and_rejects_smaller_allocations(self) -> None: + values = { + "CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_0_BASE": "198.51.100.0/24", + } + values["CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_0_SIZE"] = "29" + self.assertIsNotNone(health._parse_network_pool(values, 0)) + for size in (30, 31, 32): + with self.subTest(size=size): + values["CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_0_SIZE"] = str(size) + self.assertIsNone(health._parse_network_pool(values, 0)) + def test_docker_network_headroom_collection_counts_legacy_networks(self) -> None: networks = { "managed": [{"IPAM": {"Config": [{"Subnet": "198.51.100.0/28"}]}}], @@ -143,17 +155,48 @@ def run(args): result = health._docker_network_headroom(run, network_policy_values(), docker_ok=True) self.assertEqual(result["legacy"], 1) - def test_builtin_bridge_is_ignored_but_similar_user_network_is_legacy(self) -> None: - inspected = json.dumps([{"IPAM": {"Config": [{"Subnet": "172.17.0.0/16"}]}}]) - for name, expected in (("bridge", (0, "healthy")), ("bridge-copy", (1, "warning"))): - with self.subTest(name=name): - def run(args): - if args[:3] == ["docker", "network", "ls"]: - return health.subprocess.CompletedProcess(args, 0, f"{name}\n", "") - return health.subprocess.CompletedProcess(args, 0, inspected, "") + def test_overlapping_builtin_bridge_consumes_allocation_without_legacy_warning(self) -> None: + def run(args): + if args[:3] == ["docker", "network", "ls"]: + return health.subprocess.CompletedProcess(args, 0, "bridge\n", "") + return health.subprocess.CompletedProcess(args, 0, json.dumps([{"IPAM": {"Config": [{"Subnet": "198.51.100.0/28"}]}}]), "") + result = health._docker_network_headroom(run, network_policy_values(), docker_ok=True) + self.assertEqual((result["used"], result["free"], result["legacy"], result["state"]), (1, 15, 0, "healthy")) + + def test_overlapping_builtin_bridge_warns_when_it_breaks_reviewed_capacity(self) -> None: + values = network_policy_values() + values.update({ + "CI_FLEET_CONFIGURED_MAX_RUNNERS": "1", + "CI_FLEET_CONTROLLER_STATE": "active", + "CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_0_BASE": "198.51.100.0/27", + "CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_0_SIZE": "29", + "CI_FLEET_DOCKER_NETWORKS_PER_RUNNER": "2", + }) + def run(args): + if args[:3] == ["docker", "network", "ls"]: + return health.subprocess.CompletedProcess(args, 0, "bridge\n", "") + return health.subprocess.CompletedProcess(args, 0, json.dumps([{"IPAM": {"Config": [{"Subnet": "198.51.100.0/29"}]}}]), "") + result = health._docker_network_headroom(run, values, docker_ok=True) + self.assertEqual((result["configured"], result["used"], result["free"], result["legacy"], result["state"]), (4, 1, 3, 0, "warning")) + + def test_non_overlapping_builtin_bridge_is_not_legacy(self) -> None: + inspected = [] + def run(args): + if args[:3] == ["docker", "network", "ls"]: + return health.subprocess.CompletedProcess(args, 0, "bridge\n", "") + inspected.append(args[-1]) + return health.subprocess.CompletedProcess(args, 0, json.dumps([{"IPAM": {"Config": [{"Subnet": "172.17.0.0/16"}]}}]), "") + result = health._docker_network_headroom(run, network_policy_values(), docker_ok=True) + self.assertEqual((result["used"], result["legacy"], result["state"]), (0, 0, "healthy")) + self.assertEqual(inspected, ["bridge"]) - result = health._docker_network_headroom(run, network_policy_values(), docker_ok=True) - self.assertEqual((result["legacy"], result["state"]), expected) + def test_similar_user_network_is_legacy(self) -> None: + def run(args): + if args[:3] == ["docker", "network", "ls"]: + return health.subprocess.CompletedProcess(args, 0, "bridge-copy\n", "") + return health.subprocess.CompletedProcess(args, 0, json.dumps([{"IPAM": {"Config": [{"Subnet": "172.17.0.0/16"}]}}]), "") + result = health._docker_network_headroom(run, network_policy_values(), docker_ok=True) + self.assertEqual((result["used"], result["legacy"], result["state"]), (0, 1, "warning")) def test_broader_network_consumes_all_allocation_slots(self) -> None: def run(args): @@ -175,7 +218,7 @@ def test_large_pool_gap_counts_overlaps_without_materializing_slots(self) -> Non values = { "CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_COUNT": "1", "CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_0_BASE": "240.0.0.0/8", - "CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_0_SIZE": "32", + "CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_0_SIZE": "29", "CI_FLEET_DOCKER_NETWORK_RESERVE_SUBNETS": "1", } networks = { @@ -187,7 +230,7 @@ def run(args): return health.subprocess.CompletedProcess(args, 0, "broad\nduplicate\n", "") return health.subprocess.CompletedProcess(args, 0, json.dumps(networks[args[-1]]), "") result = health._docker_network_headroom(run, values, docker_ok=True) - self.assertEqual((result["configured"], result["used"], result["free"]), (1 << 24, 1 << 24, 0)) + self.assertEqual((result["configured"], result["used"], result["free"]), (1 << 21, 1 << 21, 0)) def test_network_removed_during_inspection_is_skipped_after_refresh(self) -> None: listings = iter(("vanished\nmanaged\n", "managed\n")) diff --git a/templates/config-repository/AGENTS.md b/templates/config-repository/AGENTS.md index 1332cfbf..e6c0d78c 100644 --- a/templates/config-repository/AGENTS.md +++ b/templates/config-repository/AGENTS.md @@ -15,7 +15,7 @@ Before committing configuration changes, run: ## Hard rules - Never add real `.env` files, credentials, tokens, private keys, cookies, or passwords. -- Never add addresses, VM IDs, storage identifiers, backup identifiers, SSH details, or rendered runtime configuration. +- Never add host addresses, service addresses, VM IDs, storage identifiers, backup identifiers, SSH details, credentials, secrets, unrelated infrastructure addresses, or rendered runtime configuration. The sole address exception is reviewed Docker `default_address_pools[].base` CIDRs because they are required private Git-authored fleet capacity policy. - Do not weaken `public_repositories: false` for Docker-socket runner pools. - Infrastructure configuration owns capacity. Application workflows submit all independent jobs and do not use `max-parallel` to model fleet size. - Each GitHub runner group belongs to exactly one runner pool; do not create ambiguous cross-pool assignments. diff --git a/templates/config-repository/README.md b/templates/config-repository/README.md index 9fe83822..284577e8 100644 --- a/templates/config-repository/README.md +++ b/templates/config-repository/README.md @@ -4,6 +4,13 @@ This is the public, secret-free starting point for an organization's private `ci It does **not** contain runner registration tokens, deploy credentials, private keys, host addresses, VM IDs, storage names, backup identifiers, or `.env` files. +One explicit exception permits reviewed operational Docker +`default_address_pools[].base` CIDRs in this private Git-authored policy. The +controller must render and inspect those exact capacity ranges. They are not host +addresses, credentials, host identity, or routable service endpoints. All VM, +storage, backup, SSH, rendered runtime, and unrelated infrastructure details stay +outside Git. + ```mermaid flowchart LR E[Public ci-fleet engine] -->|pinned engine commit| C[Private configuration] @@ -36,11 +43,12 @@ flowchart LR --location primary-site \ --capacity-budget 1 \ --max-runners 1 \ + --networks-per-runner 1 \ --engine-ref ``` 3. Edit `fleet.json` to add the organization's real logical mappings and replace - the generated RFC 5737 Docker pool with a reviewed non-overlapping pool. + the generated RFC 5737 Docker pool with a reviewed operational Docker pool. 4. Run the strict policy check: ```bash @@ -49,7 +57,7 @@ flowchart LR 5. Configure secret **values** in GitHub Environments, root-owned host files, or an external secret manager. The repository stores only names such as `DEPLOY_AUTH`. -The initializer refuses to replace a configured file unless `--force` is explicit. It accepts at most 30 runners so its fictional `/24` pool never produces networks smaller than `/29` after reserving the controller and operator headroom. It runs non-strict validation because the generated documentation pool is intentionally not deployable. Run `./scripts/init.sh --help` for repository, registry, runner-group, controller, location, capacity, resource, and output options. +The initializer refuses to replace a configured file unless `--force` is explicit. The product of `--max-runners` and `--networks-per-runner` cannot exceed 30, so its fictional `/24` pool never produces networks smaller than `/29` after reserving the controller and operator headroom. It runs non-strict validation because the generated documentation pool is intentionally not deployable. Run `./scripts/init.sh --help` for repository, registry, runner-group, controller, location, capacity, network, resource, and output options. ## Schema v3: Git-authored controller desired state @@ -62,7 +70,8 @@ The initializer refuses to replace a configured file unless `--force` is explici - the full reviewed ci-fleet commit SHA it runs; - a zero managed minimum and reviewed maximum runner capacity; - CPU and memory available to each ephemeral runner; -- Docker default-address pools and a reserved subnet count for health inspection. +- Docker default-address pools, a positive reviewed maximum number of Compose + networks per runner, and a reserved subnet count for health inspection. `status_reporting` is deliberately omitted from initialized and reference configurations. For an existing controller, roll out schema support in three @@ -101,13 +110,16 @@ Each runner pool has a `capacity_budget` and a runner group that must not be ass The validator totals the maximum capacity of every active or drained controller assigned to the pool and rejects overcommit. Drained capacity remains reserved so an undrain cannot silently exceed the reviewed budget. Disabled controllers do not reserve capacity. For `docker_network_policy`, each pool has an IPv4 CIDR `base` and Docker -subnet prefix `size`. Pools must not overlap, and active or drained controllers -must provide at least `max_runners + reserve_subnets + 1` subnets. The final +subnet prefix `size`. The size must be no longer than `/29` and cannot be +broader than its base. Pools must not overlap, and active or drained controllers +must provide at least `max_runners * networks_per_runner + reserve_subnets + 1` +subnets. The final subnet is reserved for the persistent controller Compose network. Real pool values belong in the private configuration; this template uses RFC 5737 documentation ranges only. Non-strict validation accepts those public examples. Strict validation rejects them until the private configuration uses a reviewed -non-overlapping pool. +operational Docker pool CIDR. This is the narrow capacity-policy exception +described above, not permission to commit host or service addresses. The field is optional solely for staged upgrades from older schema-v3 engines. First pin and activate a compatible engine without adding the field. In a second diff --git a/templates/config-repository/examples/multi-host/fleet.json b/templates/config-repository/examples/multi-host/fleet.json index 23d43a70..94c7f067 100644 --- a/templates/config-repository/examples/multi-host/fleet.json +++ b/templates/config-repository/examples/multi-host/fleet.json @@ -33,6 +33,7 @@ }, "docker_network_policy": { "default_address_pools": [{"base": "198.51.100.0/24", "size": 28}], + "networks_per_runner": 1, "reserve_subnets": 1 } }, @@ -51,6 +52,7 @@ }, "docker_network_policy": { "default_address_pools": [{"base": "203.0.113.0/24", "size": 28}], + "networks_per_runner": 1, "reserve_subnets": 1 } } diff --git a/templates/config-repository/fleet.json b/templates/config-repository/fleet.json index 0ea789a7..55f00ebc 100644 --- a/templates/config-repository/fleet.json +++ b/templates/config-repository/fleet.json @@ -32,6 +32,7 @@ "memory_mib": 4096 }, "docker_network_policy": { + "networks_per_runner": 1, "reserve_subnets": 1, "default_address_pools": [ { diff --git a/templates/config-repository/fleet.schema.json b/templates/config-repository/fleet.schema.json index 82bca12f..9407dd73 100644 --- a/templates/config-repository/fleet.schema.json +++ b/templates/config-repository/fleet.schema.json @@ -84,8 +84,9 @@ "docker_network_policy": { "type": "object", "additionalProperties": false, - "required": ["default_address_pools", "reserve_subnets"], + "required": ["default_address_pools", "networks_per_runner", "reserve_subnets"], "properties": { + "networks_per_runner": {"type": "integer", "minimum": 1}, "reserve_subnets": {"type": "integer", "minimum": 1}, "default_address_pools": { "type": "array", @@ -96,7 +97,7 @@ "required": ["base", "size"], "properties": { "base": {"type": "string"}, - "size": {"type": "integer", "minimum": 0, "maximum": 32} + "size": {"type": "integer", "minimum": 0, "maximum": 29} } } } diff --git a/templates/config-repository/scripts/init.py b/templates/config-repository/scripts/init.py index fb3be08d..17527e87 100755 --- a/templates/config-repository/scripts/init.py +++ b/templates/config-repository/scripts/init.py @@ -38,6 +38,7 @@ def parse_args() -> argparse.Namespace: parser.add_argument("--location", default="primary-site", help="logical location slug; never an address") parser.add_argument("--capacity-budget", type=positive_integer, default=1, help="maximum capacity reserved by the pool") parser.add_argument("--max-runners", type=positive_integer, default=1, help="initial controller maximum") + parser.add_argument("--networks-per-runner", type=positive_integer, default=1, help="reviewed maximum Compose networks per runner") parser.add_argument("--runner-cpu-cores", type=positive_integer, default=2, help="CPU cores available to each runner") parser.add_argument("--runner-memory-mib", type=positive_integer, default=4096, help="memory available to each runner") parser.add_argument("--engine-ref", required=True, help="reviewed full ci-fleet commit SHA") @@ -67,8 +68,9 @@ def main() -> int: fail("--engine-ref must be a nonzero full lowercase commit SHA") if args.max_runners > args.capacity_budget: fail("--max-runners must not exceed --capacity-budget") - if args.max_runners > 30: - fail("--max-runners must not exceed 30 so generated Docker networks stay at /29 or larger with reserved capacity") + runner_networks = args.max_runners * args.networks_per_runner + if runner_networks > 30: + fail("--max-runners multiplied by --networks-per-runner must not exceed 30 so generated Docker networks stay at /29 or larger with reserved capacity") if args.runner_memory_mib < 512: fail("--runner-memory-mib must be at least 512") @@ -118,9 +120,10 @@ def main() -> int: "memory_mib": args.runner_memory_mib, }, "docker_network_policy": { + "networks_per_runner": args.networks_per_runner, "reserve_subnets": 1, "default_address_pools": [ - {"base": "198.51.100.0/24", "size": 24 + (args.max_runners + 1).bit_length()}, + {"base": "198.51.100.0/24", "size": 24 + (runner_networks + 1).bit_length()}, ], }, } diff --git a/templates/config-repository/scripts/test_policy.py b/templates/config-repository/scripts/test_policy.py index 571ecc69..5b51b5fb 100755 --- a/templates/config-repository/scripts/test_policy.py +++ b/templates/config-repository/scripts/test_policy.py @@ -54,6 +54,7 @@ def first_controller(config: dict) -> dict: def docker_network_policy() -> dict: return { + "networks_per_runner": 1, "reserve_subnets": 1, "default_address_pools": [ {"base": "198.51.100.0/24", "size": 28}, @@ -86,9 +87,11 @@ def test_schema_defines_docker_network_policy_contract(self) -> None: self.assertNotIn("docker_network_policy", controller_schema["required"]) controller = controller_schema["properties"]["docker_network_policy"] self.assertEqual(set(controller), {"type", "additionalProperties", "required", "properties"}) - self.assertEqual(controller["required"], ["default_address_pools", "reserve_subnets"]) + self.assertEqual(controller["required"], ["default_address_pools", "networks_per_runner", "reserve_subnets"]) + self.assertEqual(controller["properties"]["networks_per_runner"], {"type": "integer", "minimum": 1}) pool = controller["properties"]["default_address_pools"]["items"] self.assertEqual(set(pool["properties"]), {"base", "size"}) + self.assertEqual(pool["properties"]["size"]["maximum"], 29) def test_docker_network_policy_is_optional_and_capacity_checked_when_present(self) -> None: config = copy.deepcopy(reference_config()) @@ -98,8 +101,9 @@ def test_docker_network_policy_is_optional_and_capacity_checked_when_present(sel self.assertEqual(errors_for(config), []) first_controller(config)["docker_network_policy"] = { + "networks_per_runner": 1, "reserve_subnets": 1, - "default_address_pools": [{"base": "198.51.100.0/30", "size": 30}], + "default_address_pools": [{"base": "198.51.100.0/29", "size": 29}], } self.assert_rejected(config, "capacity") @@ -111,11 +115,49 @@ def test_present_null_docker_network_policy_is_rejected(self) -> None: def test_docker_network_policy_reserves_controller_compose_subnet(self) -> None: config = copy.deepcopy(reference_config()) first_controller(config)["docker_network_policy"] = { + "networks_per_runner": 1, "reserve_subnets": 1, - "default_address_pools": [{"base": "198.51.100.0/30", "size": 31}], + "default_address_pools": [{"base": "198.51.100.0/28", "size": 29}], } self.assert_rejected(config, "controller Compose network") + def test_docker_network_policy_accepts_29_and_rejects_smaller_allocations(self) -> None: + config = copy.deepcopy(reference_config()) + pool = first_controller(config)["docker_network_policy"]["default_address_pools"][0] + pool["size"] = 29 + self.assertEqual(errors_for(config), []) + for size in (30, 31, 32): + with self.subTest(size=size): + pool["size"] = size + self.assert_rejected(config, "between 0 and 29") + + def test_docker_network_policy_accounts_for_every_runner_network(self) -> None: + config = copy.deepcopy(reference_config()) + controller = first_controller(config) + controller["max_runners"] = 2 + config["runner_pools"][controller["pool"]]["capacity_budget"] = 2 + controller["docker_network_policy"] = { + "networks_per_runner": 2, + "reserve_subnets": 1, + "default_address_pools": [{"base": "198.51.100.0/27", "size": 29}], + } + self.assert_rejected(config, "max_runners * networks_per_runner") + controller["docker_network_policy"]["default_address_pools"].append( + {"base": "203.0.113.0/28", "size": 29} + ) + self.assertEqual(errors_for(config), []) + + def test_disabled_docker_network_policy_keeps_reserve_and_controller_capacity(self) -> None: + config = copy.deepcopy(reference_config()) + controller = first_controller(config) + controller["state"] = "disabled" + controller["docker_network_policy"] = { + "networks_per_runner": 100, + "reserve_subnets": 1, + "default_address_pools": [{"base": "198.51.100.0/28", "size": 29}], + } + self.assertEqual(errors_for(config), []) + def test_docker_network_policy_rejects_overlapping_or_malformed_pools(self) -> None: config = copy.deepcopy(reference_config()) policy = docker_network_policy() @@ -158,6 +200,21 @@ def test_initializer_sizes_policy_for_sixteen_runners(self) -> None: policy = first_controller(config)["docker_network_policy"] self.assertGreaterEqual(1 << (policy["default_address_pools"][0]["size"] - 24), 18) + def test_initializer_sizes_policy_for_networks_per_runner(self) -> None: + with tempfile.TemporaryDirectory() as directory: + output = Path(directory) / "fleet.json" + subprocess.run([ + sys.executable, str(ROOT / "scripts" / "init.py"), + "--organization", "sample-org", "--project", "sample-app", + "--engine-ref", "1" * 40, "--max-runners", "2", + "--capacity-budget", "2", "--networks-per-runner", "2", + "--output", str(output), + ], check=True, stdout=subprocess.DEVNULL) + config = json.loads(output.read_text()) + policy = first_controller(config)["docker_network_policy"] + self.assertEqual(policy["networks_per_runner"], 2) + self.assertGreaterEqual(1 << (policy["default_address_pools"][0]["size"] - 24), 6) + def test_initializer_accepts_largest_practical_network_allocation(self) -> None: with tempfile.TemporaryDirectory() as directory: output = Path(directory) / "fleet.json" @@ -890,10 +947,28 @@ def test_strict_mode_requires_replacing_documentation_address_pools(self) -> Non for base in ("192.0.2.0/24", "198.51.100.0/24", "203.0.113.0/24"): with self.subTest(base=base): first_controller(config)["docker_network_policy"]["default_address_pools"][0]["base"] = base - self.assert_rejected(config, "replace the RFC 5737 documentation address pool", strict=True) + self.assert_rejected(config, "reviewed operational Docker pool CIDR", strict=True) first_controller(config)["docker_network_policy"]["default_address_pools"][0]["base"] = "10.64.0.0/24" self.assertEqual(errors_for(config, strict=True), []) + def test_private_address_pool_exception_is_narrow_and_documented(self) -> None: + repository_root = ROOT.parents[1] + for path in ( + repository_root / "AGENTS.md", + repository_root / "README.md", + repository_root / "docs" / "DESIRED-STATE.md", + ROOT / "AGENTS.md", + ROOT / "README.md", + ): + with self.subTest(path=path): + text = path.read_text(encoding="utf-8") + self.assertIn("default_address_pools[].base", text) + self.assertIn("host addresses", text.lower()) + + config = copy.deepcopy(reference_config()) + config["host_address"] = "10.0.0.1" + self.assert_rejected(config, "host-local infrastructure details are forbidden", strict=True) + def test_nonstandard_ci_entrypoint_is_rejected(self) -> None: config = copy.deepcopy(reference_config()) first_project(config)["ci_contract"]["aggregate_entrypoints"]["fast"] = "npm test" diff --git a/templates/config-repository/scripts/validate.py b/templates/config-repository/scripts/validate.py index a88f0106..5d1d2800 100755 --- a/templates/config-repository/scripts/validate.py +++ b/templates/config-repository/scripts/validate.py @@ -155,7 +155,7 @@ def validate_docker_network_policy( if not isinstance(policy, dict): validation.errors.append(f"{path}: must be an object") return 0, 0, [] - required = {"default_address_pools", "reserve_subnets"} + required = {"default_address_pools", "networks_per_runner", "reserve_subnets"} keys = set(policy) missing = sorted(required - keys) unknown = sorted(keys - required) @@ -171,6 +171,10 @@ def validate_docker_network_policy( if type(reserve) is not int or reserve < 1: validation.errors.append(f"{path}.reserve_subnets: must be a positive integer") return 0, 0, [] + networks_per_runner = policy["networks_per_runner"] + if type(networks_per_runner) is not int or networks_per_runner < 1: + validation.errors.append(f"{path}.networks_per_runner: must be a positive integer") + return 0, 0, [] pools = policy["default_address_pools"] if type(pools) is not list or not pools: validation.errors.append(f"{path}.default_address_pools: must be a non-empty list") @@ -186,8 +190,8 @@ def validate_docker_network_policy( if not isinstance(base, str): validation.errors.append(f"{pool_path}.base: must be a CIDR prefix") return 0, 0, [] - if type(size) is not int or size < 0 or size > 32: - validation.errors.append(f"{pool_path}.size: must be an IPv4 prefix length between 0 and 32") + if type(size) is not int or size < 0 or size > 29: + validation.errors.append(f"{pool_path}.size: must be an IPv4 prefix length between 0 and 29") return 0, 0, [] try: network = ipaddress.ip_network(base, strict=True) @@ -199,7 +203,7 @@ def validate_docker_network_policy( return 0, 0, [] if strict and any(network.overlaps(documentation) for documentation in RFC_5737_NETWORKS): validation.errors.append( - f"{pool_path}.base: replace the RFC 5737 documentation address pool with a reviewed non-overlapping pool" + f"{pool_path}.base: replace the RFC 5737 documentation address pool with a reviewed operational Docker pool CIDR" ) return 0, 0, [] if size < network.prefixlen: @@ -212,9 +216,9 @@ def validate_docker_network_policy( validation.errors.append(f"{path}.default_address_pools[{left}].base: overlaps configured pool {right}") return 0, 0, [] configured = sum(1 << (item["size"] - item["network"].prefixlen) for item in parsed) - if configured < max_runners + reserve + 1: + if configured < max_runners * networks_per_runner + reserve + 1: validation.errors.append( - f"{path}: network capacity cannot satisfy max_runners + reserve_subnets + one controller Compose network" + f"{path}: network capacity cannot satisfy max_runners * networks_per_runner + reserve_subnets + one controller Compose network" ) return configured, reserve, parsed From ce621fb68f8aaccea4a5c6358a7061140d08c6b5 Mon Sep 17 00:00:00 2001 From: Nickfost <1572453+Nickfost@users.noreply.github.com> Date: Thu, 27 Aug 2026 00:55:08 -0500 Subject: [PATCH 8/8] fix: harden network policy rollout reporting --- docs/DESIRED-STATE.md | 4 +- docs/HEALTH-MONITORING.md | 5 +- docs/STATUS-REPORTING.md | 5 +- scripts/health.py | 35 ++++---- scripts/test_health.py | 51 ++++++++++-- templates/config-repository/README.md | 11 ++- .../config-repository/scripts/test_policy.py | 81 +++++++++++++++++++ .../config-repository/scripts/validate.py | 3 +- 8 files changed, 162 insertions(+), 33 deletions(-) diff --git a/docs/DESIRED-STATE.md b/docs/DESIRED-STATE.md index 3de7a4d4..f7e2d2a0 100644 --- a/docs/DESIRED-STATE.md +++ b/docs/DESIRED-STATE.md @@ -66,7 +66,9 @@ record `docker_network_policy_config: true` for that controller and ref in changing the engine or evidence. Transition validation reads the evidence from the previous integrated state, so a commit that adds evidence and policy together cannot satisfy the gate. Do not add the field while the old engine still performs -reconciliation. +reconciliation. Once present, the policy requires current evidence naming the +selected engine and declaring `docker_network_policy_config: true`; remove the +policy before selecting an engine without that evidence. This phase renders the policy solely for read-only health inspection. It does not write `daemon.json`, restart Docker, create or remove networks, prune diff --git a/docs/HEALTH-MONITORING.md b/docs/HEALTH-MONITORING.md index 7f188467..bfa3df18 100644 --- a/docs/HEALTH-MONITORING.md +++ b/docs/HEALTH-MONITORING.md @@ -30,7 +30,10 @@ Docker network inspection is read-only. Healthy headroom is reported when free subnets remain above the reviewed reserve, low water and legacy/nonconforming networks are warnings, and exhaustion is critical. A malformed policy or failed Docker network listing/inspection is critical rather than falsely healthy. -Only aggregate configured, used, free, and legacy counts enter status reports. +When policy parsing succeeds during an inspection outage, the local snapshot +retains the configured subnet count and reserve without inventing usage counts. +Status reports include aggregate configured, used, free, and legacy counts only +after a successful measurement; otherwise they omit the optional network field. This detection-only phase does not mutate the Docker daemon or networks. Controller circuit breaking, frequent orphan reconciliation, daemon policy diff --git a/docs/STATUS-REPORTING.md b/docs/STATUS-REPORTING.md index d180e0d5..fc2374d6 100644 --- a/docs/STATUS-REPORTING.md +++ b/docs/STATUS-REPORTING.md @@ -24,7 +24,7 @@ The machine-readable contract is `schemas/status-report-v1.json`. It reports: - reconciliation, drift, health, and cleanup timer states; - current, busy, and configured-maximum runner counts; - CPU use, logical CPU count, memory, swap, root/Docker disk and inode use, and 1/5/15-minute load; -- Docker availability, OOM evidence, and aggregate configured/used/free/legacy subnet counts; +- Docker availability, OOM evidence, and optional measured configured/used/free/legacy subnet counts; - one controlled error code/message, report generation time, and schema version. All times are Unix seconds. Commit values are empty when unavailable. Receiver validation rejects unknown fields and unsupported schema versions rather than guessing at compatibility. @@ -32,6 +32,9 @@ All times are Unix seconds. Commit values are empty when unavailable. Receiver v `error.message` is derived only from a controlled error code (`_` becomes a space). Raw exception text is never transmitted. Docker pool prefixes and network addresses are intentionally absent from the status contract; they remain in private desired state and host-local inspection. +The reporter omits the optional `docker.network` aggregate when Docker network +inspection is unavailable. It does not publish zero usage or capacity as a +substitute for missing measurements. ## Authentication diff --git a/scripts/health.py b/scripts/health.py index 6c30645f..688d393c 100644 --- a/scripts/health.py +++ b/scripts/health.py @@ -237,7 +237,7 @@ def build_status_report(snapshot: dict[str, Any], health_report: dict[str, Any], "oom": bool(snapshot.get("recent_oom") or snapshot["controller"].get("oom_killed")), } network = snapshot.get("docker_network_headroom") - if network and network.get("state") != "not_configured": + if network and network.get("state") in {"healthy", "warning", "critical"}: docker["network"] = {key: network.get(key, 0) for key in ("configured", "used", "free", "legacy")} return { "schema_version": 1, @@ -337,30 +337,31 @@ def _parse_network_pool(values: dict[str, str], index: int) -> tuple[ipaddress.I def _docker_network_headroom(run: Runner, values: dict[str, str], *, docker_ok: bool) -> dict[str, Any]: - empty = {"configured": 0, "used": 0, "free": 0, "reserve": 0, "legacy": 0, "state": "unavailable"} + unavailable = {"state": "unavailable"} try: configured_count = int(values.get("CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_COUNT", "0")) configured_max = int(values.get("CI_FLEET_CONFIGURED_MAX_RUNNERS", values.get("CI_FLEET_MAX_RUNNERS", "0"))) networks_per_runner = int(values.get("CI_FLEET_DOCKER_NETWORKS_PER_RUNNER", "1")) reserve = int(values.get("CI_FLEET_DOCKER_NETWORK_RESERVE_SUBNETS", "0")) except ValueError: - return empty + return unavailable if configured_count == 0 and "CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_COUNT" not in values: - return {**empty, "state": "not_configured"} - if not docker_ok: - return empty + return {"state": "not_configured"} pools: list[tuple[ipaddress.IPv4Network, int]] = [] for index in range(configured_count): pool = _parse_network_pool(values, index) if pool is None: - return empty + return unavailable pools.append(pool) if not pools or configured_max < 0 or networks_per_runner < 1 or reserve < 1: - return empty + return unavailable + configured = sum(1 << (size - pool.prefixlen) for pool, size in pools) + unavailable = {"configured": configured, "reserve": reserve, "state": "unavailable"} + if not docker_ok: + return unavailable listed = run(["docker", "network", "ls", "--format", "{{.Name}}"]) if listed.returncode != 0: - return empty - configured = sum(1 << (size - pool.prefixlen) for pool, size in pools) + return unavailable occupied: list[list[tuple[int, int]]] = [[] for _ in pools] bridge_occupied: list[list[tuple[int, int]]] = [[] for _ in pools] legacy_networks = 0 @@ -369,30 +370,30 @@ def _docker_network_headroom(run: Runner, values: dict[str, str], *, docker_ok: if inspected.returncode != 0: refreshed = run(["docker", "network", "ls", "--format", "{{.Name}}"]) if refreshed.returncode != 0: - return empty + return unavailable if name not in {line.strip() for line in refreshed.stdout.splitlines() if line.strip()}: continue - return empty + return unavailable try: payload = json.loads(inspected.stdout) except json.JSONDecodeError: - return empty + return unavailable if not isinstance(payload, list) or not payload: - return empty + return unavailable subnets: list[ipaddress.IPv4Network] = [] saw_ipv6 = False for entry in payload: configs = entry.get("IPAM", {}).get("Config", []) if isinstance(entry, dict) else [] if not isinstance(configs, list): - return empty + return unavailable for config in configs: subnet = config.get("Subnet") if isinstance(config, dict) else None if not isinstance(subnet, str): - return empty + return unavailable try: network = ipaddress.ip_network(subnet, strict=False) except ValueError: - return empty + return unavailable if network.version == 6: saw_ipv6 = True continue diff --git a/scripts/test_health.py b/scripts/test_health.py index 063313cc..bb5340c0 100644 --- a/scripts/test_health.py +++ b/scripts/test_health.py @@ -50,6 +50,12 @@ def network_policy_values() -> dict[str, str]: class HealthTests(unittest.TestCase): + def assert_unavailable_network(self, network: dict) -> None: + self.assertEqual(network, {"configured": 16, "reserve": 1, "state": "unavailable"}) + snapshot = {**healthy_snapshot(), "docker_network_headroom": network} + report = health.build_status_report(snapshot, health.evaluate(snapshot, health.Thresholds()), generated_at=1_000) + self.assertNotIn("network", report["docker"]) + def test_healthy_active_host(self) -> None: report = health.evaluate(healthy_snapshot(), health.Thresholds()) self.assertEqual(report["status"], "healthy") @@ -93,7 +99,7 @@ def run(args): return health.subprocess.CompletedProcess(args, 1, "", "") snapshot = health.collect_snapshot({**network_policy_values(), "CI_FLEET_CONTROLLER_STATE": "active", "CI_FLEET_INSTANCE": "example-ci-01"}, run=run) - self.assertEqual(snapshot["docker_network_headroom"]["state"], "unavailable") + self.assert_unavailable_network(snapshot["docker_network_headroom"]) report = health.evaluate({**healthy_snapshot(), "docker_network_headroom": snapshot["docker_network_headroom"]}, health.Thresholds()) self.assertEqual((report["status"], report["exit_code"]), ("unhealthy", 2)) self.assertIn("docker_network_inspection", {check["id"] for check in report["checks"] if check["status"] == "critical"}) @@ -115,6 +121,14 @@ def test_legacy_status_report_omits_unconfigured_docker_network(self) -> None: self.assertEqual(network["state"], "not_configured") self.assertNotIn("network", report["docker"]) + def test_docker_unavailable_preserves_configured_network_policy_only_locally(self) -> None: + network = health._docker_network_headroom( + lambda args: health.subprocess.CompletedProcess(args, 0, "", ""), + network_policy_values(), + docker_ok=False, + ) + self.assert_unavailable_network(network) + def test_network_pool_parser_accepts_29_and_rejects_smaller_allocations(self) -> None: values = { "CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_0_BASE": "198.51.100.0/24", @@ -126,6 +140,14 @@ def test_network_pool_parser_accepts_29_and_rejects_smaller_allocations(self) -> values["CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_0_SIZE"] = str(size) self.assertIsNone(health._parse_network_pool(values, 0)) + def test_incomplete_network_policy_does_not_fabricate_an_aggregate(self) -> None: + values = network_policy_values() + values.pop("CI_FLEET_DOCKER_DEFAULT_ADDRESS_POOL_0_SIZE") + self.assertEqual( + health._docker_network_headroom(lambda args: health.subprocess.CompletedProcess(args, 0, "", ""), values, docker_ok=True), + {"state": "unavailable"}, + ) + def test_docker_network_headroom_collection_counts_legacy_networks(self) -> None: networks = { "managed": [{"IPAM": {"Config": [{"Subnet": "198.51.100.0/28"}]}}], @@ -145,6 +167,8 @@ def run(args): self.assertEqual(snapshot["docker_network_headroom"], {"configured": 16, "used": 1, "free": 15, "reserve": 1, "legacy": 1, "state": "warning"}) report = health.evaluate({**healthy_snapshot(), "docker_network_headroom": snapshot["docker_network_headroom"]}, health.Thresholds()) self.assertEqual(report["status"], "warning") + external = health.build_status_report({**healthy_snapshot(), "docker_network_headroom": snapshot["docker_network_headroom"]}, report, generated_at=1_000) + self.assertEqual(external["docker"]["network"], {"configured": 16, "used": 1, "free": 15, "legacy": 1}) def test_builtin_addressless_networks_are_ignored_but_custom_ones_are_legacy(self) -> None: networks = {name: [{"IPAM": {"Config": []}}] for name in ("host", "none", "custom")} @@ -243,12 +267,21 @@ def run(args): result = health._docker_network_headroom(run, network_policy_values(), docker_ok=True) self.assertEqual((result["used"], result["state"]), (1, "healthy")) + def test_disappearing_network_retry_failure_preserves_only_policy_counts(self) -> None: + listings = iter(((0, "vanished\n"), (1, ""))) + def run(args): + if args[:3] == ["docker", "network", "ls"]: + returncode, stdout = next(listings) + return health.subprocess.CompletedProcess(args, returncode, stdout, "") + return health.subprocess.CompletedProcess(args, 1, "", "not found") + self.assert_unavailable_network(health._docker_network_headroom(run, network_policy_values(), docker_ok=True)) + def test_network_inspection_error_fails_closed_when_network_still_exists(self) -> None: def run(args): if args[:3] == ["docker", "network", "ls"]: return health.subprocess.CompletedProcess(args, 0, "broken\n", "") return health.subprocess.CompletedProcess(args, 1, "", "permission denied") - self.assertEqual(health._docker_network_headroom(run, network_policy_values(), docker_ok=True)["state"], "unavailable") + self.assert_unavailable_network(health._docker_network_headroom(run, network_policy_values(), docker_ok=True)) def test_ipv6_ipam_is_ignored_without_hiding_ipv4(self) -> None: networks = { @@ -267,13 +300,15 @@ def run(args): if args[:3] == ["docker", "network", "ls"]: return health.subprocess.CompletedProcess(args, 0, "broken\n", "") return health.subprocess.CompletedProcess(args, 0, json.dumps([{"IPAM": {"Config": [{"Subnet": "198.51.100.999/28"}]}}]), "") - self.assertEqual(health._docker_network_headroom(run, network_policy_values(), docker_ok=True)["state"], "unavailable") + self.assert_unavailable_network(health._docker_network_headroom(run, network_policy_values(), docker_ok=True)) - def test_status_report_redacts_network_addresses(self) -> None: - snapshot = {**healthy_snapshot(), "docker_network_headroom": {"configured": 16, "used": 2, "free": 14, "reserve": 1, "legacy": 1, "state": "warning"}} - report = health.build_status_report(snapshot, health.evaluate(snapshot, health.Thresholds()), generated_at=1_000) - self.assertEqual(report["docker"]["network"], {"configured": 16, "used": 2, "free": 14, "legacy": 1}) - self.assertNotIn("198.51.100", json.dumps(report)) + def test_status_report_publishes_all_measured_network_states_without_addresses(self) -> None: + for state in ("healthy", "warning", "critical"): + with self.subTest(state=state): + snapshot = {**healthy_snapshot(), "docker_network_headroom": {"configured": 16, "used": 2, "free": 14, "reserve": 1, "legacy": 1, "state": state}} + report = health.build_status_report(snapshot, health.evaluate(snapshot, health.Thresholds()), generated_at=1_000) + self.assertEqual(report["docker"]["network"], {"configured": 16, "used": 2, "free": 14, "legacy": 1}) + self.assertNotIn("198.51.100", json.dumps(report)) def test_health_contract_classifies_host_failures(self) -> None: cases = { diff --git a/templates/config-repository/README.md b/templates/config-repository/README.md index 284577e8..b31e6979 100644 --- a/templates/config-repository/README.md +++ b/templates/config-repository/README.md @@ -88,9 +88,10 @@ can upgrade itself. Endpoint and key values remain host-local and never enter Git. The same per-controller evidence record may declare -`docker_network_policy_config`. Existing status-reporting records may omit this -new boolean, so their behavior does not change. A complete record has this shape -after an operator has verified the named engine is active: +`docker_network_policy_config`. A controller may omit this boolean only while it +omits `docker_network_policy`. A retained policy requires current evidence for +the selected engine with this boolean set to `true`. A complete record has this +shape after an operator has verified the named engine is active: ```json { @@ -128,7 +129,9 @@ reviewed desired-state commit, record the active ref and reviewed policy in a third commit, retaining the same ref and evidence. The older engine rejects the new key, so skipped commits must not satisfy the gate. Transition validation requires the activation evidence to exist in the previous -integrated state. +integrated state when introducing the policy. It also requires matching current +evidence whenever a controller retains the policy, including across engine +changes or rollbacks. The public engine renders these values only for read-only health inspection. This phase detects low water, exhaustion, failed inspection, and legacy diff --git a/templates/config-repository/scripts/test_policy.py b/templates/config-repository/scripts/test_policy.py index 5b51b5fb..385af024 100755 --- a/templates/config-repository/scripts/test_policy.py +++ b/templates/config-repository/scripts/test_policy.py @@ -273,6 +273,7 @@ def test_status_reporting_requires_a_separate_engine_rollout(self) -> None: "engine_ref": first_controller(staged)["engine_ref"], "status_reporting_config": True, "required_status_reporting": False, + "docker_network_policy_config": True, }, }, validation) self.assertEqual(validation.errors, []) @@ -282,6 +283,7 @@ def test_status_reporting_requires_a_separate_engine_rollout(self) -> None: "engine_ref": first_controller(staged)["engine_ref"], "status_reporting_config": True, "required_status_reporting": False, + "docker_network_policy_config": True, }, }, validation, {}) self.assertTrue(any("capability evidence" in error for error in validation.errors), validation.errors) @@ -348,6 +350,7 @@ def test_retained_reporting_requires_target_engine_capabilities(self) -> None: "engine_ref": "2" * 40, "status_reporting_config": True, "required_status_reporting": False, + "docker_network_policy_config": True, } } validation = Validation() @@ -373,8 +376,84 @@ def test_retained_reporting_requires_target_engine_capabilities(self) -> None: validate_transition(previous, current, compatible, validation) self.assertEqual(validation.errors, []) + def test_retained_network_policy_requires_target_engine_capability(self) -> None: + previous = reference_config() + current = copy.deepcopy(previous) + first_controller(current)["engine_ref"] = "2" * 40 + controller = next(iter(current["controllers"])) + compatible = { + controller: { + "engine_ref": "2" * 40, + "status_reporting_config": False, + "required_status_reporting": False, + "docker_network_policy_config": True, + } + } + + validation = Validation() + validate_transition(previous, current, compatible, validation) + self.assertEqual(validation.errors, []) + + for evidence in ( + {}, + {controller: {**compatible[controller], "engine_ref": "3" * 40}}, + {controller: {**compatible[controller], "docker_network_policy_config": False}}, + ): + with self.subTest(evidence=evidence): + validation = Validation() + validate_transition(previous, current, evidence, validation) + self.assertTrue(any("Docker network policy configuration capability" in error for error in validation.errors), validation.errors) + + def test_network_policy_removal_needs_no_capability_evidence(self) -> None: + previous = reference_config() + current = copy.deepcopy(previous) + first_controller(current).pop("docker_network_policy") + first_controller(current)["engine_ref"] = "2" * 40 + validation = Validation() + validate_transition(previous, current, {}, validation) + self.assertEqual(validation.errors, []) + + def test_cli_retained_network_policy_uses_current_rollout_evidence(self) -> None: + previous = reference_config() + current = copy.deepcopy(previous) + controller = next(iter(current["controllers"])) + engine_ref = first_controller(current)["engine_ref"] + previous_evidence = { + "schema_version": 1, + "status_reporting_engine_capabilities": { + controller: { + "engine_ref": engine_ref, + "status_reporting_config": False, + "required_status_reporting": False, + "docker_network_policy_config": True, + }, + }, + } + current_evidence = copy.deepcopy(previous_evidence) + current_evidence["status_reporting_engine_capabilities"][controller]["docker_network_policy_config"] = False + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + for name, value in ( + ("previous.json", previous), + ("current.json", current), + ("previous-evidence.json", previous_evidence), + ("evidence.json", current_evidence), + ): + (root / name).write_text(json.dumps(value), encoding="utf-8") + result = subprocess.run([ + sys.executable, str(ROOT / "scripts" / "validate.py"), + "--config", str(root / "current.json"), + "--previous-config", str(root / "previous.json"), + "--rollout-evidence", str(root / "evidence.json"), + "--previous-rollout-evidence", str(root / "previous-evidence.json"), + "--skip-path-scan", + ], stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True) + self.assertNotEqual(result.returncode, 0) + self.assertIn("Docker network policy configuration capability", result.stderr) + def test_reporting_removal_before_engine_change_needs_no_evidence(self) -> None: previous = reference_config() + first_controller(previous).pop("docker_network_policy") first_controller(previous)["status_reporting"] = { "enabled": True, "config_file": "/etc/ci-fleet/monitoring.env", @@ -538,6 +617,7 @@ def test_enabling_required_reporting_needs_required_capability_evidence(self) -> "engine_ref": first_controller(previous)["engine_ref"], "status_reporting_config": True, "required_status_reporting": False, + "docker_network_policy_config": True, }, }, } @@ -566,6 +646,7 @@ def test_enabling_required_reporting_needs_required_capability_evidence(self) -> "engine_ref": first_controller(previous)["engine_ref"], "status_reporting_config": True, "required_status_reporting": True, + "docker_network_policy_config": True, }, }, validation) self.assertEqual(validation.errors, []) diff --git a/templates/config-repository/scripts/validate.py b/templates/config-repository/scripts/validate.py index 5d1d2800..7830d4a0 100755 --- a/templates/config-repository/scripts/validate.py +++ b/templates/config-repository/scripts/validate.py @@ -617,11 +617,12 @@ def validate_transition( f"$.controllers.{name}.docker_network_policy", "requires reviewed evidence from the previous integrated state that this controller activated the same engine_ref with Docker network policy configuration capability", ) + if "docker_network_policy" in new: validation.require( current_evidence.get("engine_ref") == new.get("engine_ref") and current_evidence.get("docker_network_policy_config") is True, f"$.controllers.{name}.docker_network_policy", - "requires retaining Docker network policy rollout evidence for this controller and engine_ref", + "requires Docker network policy configuration capability evidence for this controller and engine_ref", ) staged_capability_required = ( "status_reporting" not in old