From a3ff03fc74e1c77b56f688ef6a3dcd783ee6a7e5 Mon Sep 17 00:00:00 2001 From: Paul Payne Date: Sun, 12 Jul 2026 14:08:56 -0700 Subject: [PATCH] app: enhance gateway change detection for Caddyfile routing --- cli/src/castle_cli/commands/apply.py | 4 ++ core/src/castle_core/deploy.py | 62 +++++++++++++++++++++++++- core/tests/test_apply.py | 66 +++++++++++++++++++++++++++- 3 files changed, 130 insertions(+), 2 deletions(-) diff --git a/cli/src/castle_cli/commands/apply.py b/cli/src/castle_cli/commands/apply.py index e5e005f..cf97701 100644 --- a/cli/src/castle_cli/commands/apply.py +++ b/cli/src/castle_cli/commands/apply.py @@ -48,6 +48,8 @@ def run_apply(args: argparse.Namespace) -> int: _line(_C["activate"], "would activate ", result.activated) _line(_C["restart"], "would restart ", result.restarted) _line(_C["deactivate"], "would deactivate", result.deactivated) + if result.gateway_changed: + print(f" {_C['restart']}would reload {_C['reset']} gateway routes") return 0 print("\n\033[1mApplied\033[0m") @@ -57,4 +59,6 @@ def run_apply(args: argparse.Namespace) -> int: _line(_C["activate"], "activated ", result.activated) _line(_C["restart"], "restarted ", result.restarted) _line(_C["deactivate"], "deactivated", result.deactivated) + if result.gateway_changed: + print(f" {_C['restart']}reloaded {_C['reset']} gateway routes") return 0 diff --git a/core/src/castle_core/deploy.py b/core/src/castle_core/deploy.py index 64885f8..0acbd47 100644 --- a/core/src/castle_core/deploy.py +++ b/core/src/castle_core/deploy.py @@ -86,13 +86,26 @@ class ApplyResult: pruned: list[str] = field(default_factory=list) messages: list[str] = field(default_factory=list) registry: NodeRegistry | None = None + # Gateway routing (the Caddyfile / cloudflared ingress) differs from what's live: + # a caddy static/proxy route was added, removed, or had its root/reach changed. + # Tracked separately because such a delta touches no systemd unit, so the + # activate/restart/deactivate reconcile never classifies it as a change. apply() + # rewrites the artifacts and reloads the gateway regardless; this flag just lets + # the summary report it instead of a false "already converged". + gateway_changed: bool = False # True for a `--plan` run: the diff was computed but nothing was written or # activated. Lets callers render "would activate…" vs "activated…". planned: bool = False @property def changed(self) -> bool: - return bool(self.activated or self.restarted or self.deactivated or self.pruned) + return bool( + self.activated + or self.restarted + or self.deactivated + or self.pruned + or self.gateway_changed + ) def deploy(target_name: str | None = None, root: Path | None = None) -> DeployResult: @@ -205,6 +218,48 @@ def _unit_bytes(name: str, kind: str) -> str | None: return path.read_text() if path.exists() else None +def _desired_registry(config: CastleConfig, target_name: str | None) -> NodeRegistry: + """The registry ``deploy()`` would write for this (optionally scoped) run. + + Mirrors deploy()'s registry build: a scoped run merges the updated target over + the existing on-disk registry; a full run starts fresh. Used to predict + gateway-route deltas without writing anything.""" + node = _node_config(config) + if target_name and REGISTRY_PATH.exists(): + try: + registry = NodeRegistry(node=node, deployed=dict(load_registry().deployed)) + except (FileNotFoundError, ValueError): + registry = NodeRegistry(node=node) + else: + registry = NodeRegistry(node=node) + for _kind, name, dep in config.all_deployments(): + if target_name and name != target_name: + continue + deployed = _build_deployed(config, name, dep, []) + deployed.name = name + registry.put(deployed) + return registry + + +def _gateway_would_change(config: CastleConfig, target_name: str | None) -> bool: + """Whether applying would rewrite the gateway's routing artifacts — the + Caddyfile or the cloudflared ingress — vs. what's on disk. + + A pure caddy route change (new static, changed ``root``/``reach``, toggled + public) touches no systemd unit, so the activate/restart/deactivate reconcile + can't see it; without this the summary reports "already converged" despite a + live routing change. Compared before ``deploy()`` rewrites the artifacts, so it + reflects the pre-apply delta for both the plan and the real run.""" + registry = _desired_registry(config, target_name) + caddyfile = SPECS_DIR / "Caddyfile" + current_caddy = caddyfile.read_text() if caddyfile.exists() else None + if generate_caddyfile_from_registry(registry) != current_caddy: + return True + tunnel_path = SPECS_DIR / "cloudflared.yml" + current_tunnel = tunnel_path.read_text() if tunnel_path.exists() else None + return generate_tunnel_config(registry) != current_tunnel + + def apply( target_name: str | None = None, root: Path | None = None, @@ -264,6 +319,11 @@ def apply( deployed={NodeRegistry.key(k, n): d for (k, n), d in desired.items()}, ) ) + # Gateway routing lives in the Caddyfile / cloudflared ingress, not a systemd + # unit, so _classify above can't see a route-only change. Detect it here against + # the on-disk artifacts (before deploy() rewrites them) so both the plan and the + # real run report it instead of a false "already converged". + result.gateway_changed = _gateway_would_change(config, target_name) if plan: # No writes: for systemd, predict the new unit bytes by rendering to a string diff --git a/core/tests/test_apply.py b/core/tests/test_apply.py index b1100dc..32dbb97 100644 --- a/core/tests/test_apply.py +++ b/core/tests/test_apply.py @@ -11,10 +11,31 @@ from __future__ import annotations from pathlib import Path from unittest.mock import patch -from castle_core.deploy import _render_unit_preview, apply +import castle_core.deploy as deploy_mod +from castle_core.config import load_config +from castle_core.deploy import ( + _desired_registry, + _gateway_would_change, + _render_unit_preview, + apply, + generate_caddyfile_from_registry, +) from castle_core.registry import Deployment +def _add_static(castle_root: Path, name: str = "test-static") -> None: + """Write a caddy (static) program + deployment into an existing castle root.""" + (castle_root / "programs" / f"{name}.yaml").write_text( + f"description: Static {name}\nsource: {castle_root / name}\n" + ) + statics = castle_root / "deployments" / "statics" + statics.mkdir(parents=True, exist_ok=True) + (statics / f"{name}.yaml").write_text( + f"program: {name}\nmanager: caddy\nroot: public\nreach: internal\n" + ) + (castle_root / name / "public").mkdir(parents=True, exist_ok=True) + + def _plan(castle_root: Path, active: dict[str, bool]): """Run apply(plan=True) with is_active stubbed to `active` (default False).""" with patch( @@ -68,6 +89,49 @@ class TestApplyPlan: assert result.changed is True +class TestGatewayChange: + """A caddy route change touches no systemd unit, so the activate/restart/ + deactivate reconcile can't see it. `gateway_changed` catches it by diffing the + would-be Caddyfile/tunnel config against disk — otherwise a new/changed static + route reports a false 'already converged'. + + SPECS_DIR is the real ~/.castle path (unpatched by the fixtures), so these + redirect it to a temp dir to stay hermetic and never touch the live Caddyfile. + """ + + def test_new_route_reports_gateway_changed( + self, castle_root: Path, tmp_path: Path, monkeypatch + ) -> None: + """A static whose route isn't on disk yet → gateway_changed, even when the + assets already exist so the deployment itself classifies 'unchanged'.""" + monkeypatch.setattr(deploy_mod, "SPECS_DIR", tmp_path / "specs") + _add_static(castle_root) + + # Static is 'active' (built) → _classify buckets it 'unchanged'; the route is + # still absent from the (missing) Caddyfile, so the gateway did change. + result = _plan(castle_root, active={"test-static": True}) + + assert "test-static" in result.unchanged + assert result.gateway_changed is True + assert result.changed is True + + def test_converged_caddyfile_is_not_changed( + self, castle_root: Path, tmp_path: Path, monkeypatch + ) -> None: + """When the on-disk Caddyfile already matches the desired one, no change.""" + specs = tmp_path / "specs" + specs.mkdir(parents=True, exist_ok=True) + monkeypatch.setattr(deploy_mod, "SPECS_DIR", specs) + _add_static(castle_root) + config = load_config(castle_root) + + (specs / "Caddyfile").write_text( + generate_caddyfile_from_registry(_desired_registry(config, None)) + ) + + assert _gateway_would_change(config, None) is False + + def test_render_unit_preview_none_for_non_systemd() -> None: """Non-systemd managers have no unit file — preview is None (never 'restart').