From 341c52f4914cb528b29f903361e71691862edbd0 Mon Sep 17 00:00:00 2001 From: Arthur Date: Tue, 22 Sep 2026 14:40:23 +0200 Subject: [PATCH 1/7] fix(infra): show the next-step hints after stack up infra The hints were written below `[ports]`, so TOML read them as a port named `hints`. `stack up infra` never showed them, and the runtime built a `{infra_hints_url}` template variable from the list. They are now a top-level key. A new test checks that every `[ports]` entry in a shipped manifest is a port number. --- stacklets/infra/stacklet.toml | 10 +++--- .../unit/framework/test_shipped_manifests.py | 32 +++++++++++++++++++ 2 files changed, 37 insertions(+), 5 deletions(-) create mode 100644 tests/unit/framework/test_shipped_manifests.py diff --git a/stacklets/infra/stacklet.toml b/stacklets/infra/stacklet.toml index 099c1b90..22b078c9 100644 --- a/stacklets/infra/stacklet.toml +++ b/stacklets/infra/stacklet.toml @@ -19,6 +19,11 @@ stage = "beta" # Caddy is built locally with the DNS provider plugins (Dockerfile). build = true +hints = [ + "Point your router's DNS to {ip}", + "Services are now available at *.{domain}", +] + [ports] dns = 53 dns_setup = 42081 @@ -26,11 +31,6 @@ dns_admin = 42080 http = 80 https = 443 -hints = [ - "Point your router's DNS to {ip}", - "Services are now available at *.{domain}", -] - [env.defaults] TZ = "{timezone}" STACK_DOMAIN = "{domain}" diff --git a/tests/unit/framework/test_shipped_manifests.py b/tests/unit/framework/test_shipped_manifests.py new file mode 100644 index 00000000..bbc22cd3 --- /dev/null +++ b/tests/unit/framework/test_shipped_manifests.py @@ -0,0 +1,32 @@ +"""The shipped stacklet.toml files have the shape the runtime reads. + +TOML closes a table only when the next one opens, so a key written below +`[ports]` lands inside it, and nothing complains. The runtime turns each +`[ports]` entry into a `{__url}` template variable and never +looks for the key where the author meant it, so the mistake shows up +only as something quietly missing. +""" + +from __future__ import annotations + +import tomllib +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent.parent.parent + + +def _manifests() -> dict[str, dict]: + return {p.parent.name: tomllib.loads(p.read_text()) + for p in sorted(REPO_ROOT.glob("stacklets/*/stacklet.toml"))} + + +def test_manifests_are_found(): + # Guard the guard: an empty glob would make the check below pass. + assert len(_manifests()) > 5 + + +def test_every_ports_entry_is_a_port_number(): + misplaced = {sid: [k for k, v in m.get("ports", {}).items() if not isinstance(v, int)] + for sid, m in _manifests().items()} + misplaced = {sid: keys for sid, keys in misplaced.items() if keys} + assert not misplaced, f"keys inside [ports] that are not ports: {misplaced}" From 5d9c305ee05d31aa79fe9062bda412a394f83754 Mon Sep 17 00:00:00 2001 From: Arthur Date: Tue, 22 Sep 2026 14:40:47 +0200 Subject: [PATCH 2/7] fix(stack): treat https = false as plain HTTP `[core] https` reached the URL scheme as the string "False", which is truthy, so `https = false` made every domain-mode URL https just like `https = true`. The value is now compared with "true". Nothing else reads a boolean from stack.toml this way. --- lib/stack/stack.py | 6 ++++-- tests/unit/framework/test_caddy.py | 10 ++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/stack/stack.py b/lib/stack/stack.py index 86f7a02c..3e7cf0ab 100644 --- a/lib/stack/stack.py +++ b/lib/stack/stack.py @@ -497,9 +497,11 @@ def _domain_scheme(self) -> str: The proxy has certificates when `[core] dns_provider` is set. Set `[core] https = true` instead when some other reverse proxy in - front of the stack terminates TLS. + front of the stack terminates TLS. `_cfg` returns the TOML boolean + as the string "True" or "False", and "False" is truthy, so the + value is compared rather than tested. """ - if self._cfg("core", "dns_provider") or self._cfg("core", "https"): + if self._cfg("core", "dns_provider") or self._cfg("core", "https").lower() == "true": return "https" return "http" diff --git a/tests/unit/framework/test_caddy.py b/tests/unit/framework/test_caddy.py index 411204d4..7b9511c6 100644 --- a/tests/unit/framework/test_caddy.py +++ b/tests/unit/framework/test_caddy.py @@ -156,6 +156,16 @@ def test_without_one_they_stay_http(self, tmp_path): assert env["URL"] == "http://photos.home.example.family" assert env["HOME"] == "http://home.example.family" + def test_https_true_is_for_a_proxy_in_front_of_the_stack(self, tmp_path): + stck = _stack(tmp_path, 'domain = "home.example.family"\nhttps = true', + {"photos": self.PHOTOS}) + assert stck.env("photos")["URL"] == "https://photos.home.example.family" + + def test_https_false_means_http(self, tmp_path): + stck = _stack(tmp_path, 'domain = "home.example.family"\nhttps = false', + {"photos": self.PHOTOS}) + assert stck.env("photos")["URL"] == "http://photos.home.example.family" + # ── Lifecycle ───────────────────────────────────────────────────────────── From bc6276fe4e6d0aad82e4f660e521ee5c205a18b7 Mon Sep 17 00:00:00 2001 From: Arthur Date: Tue, 22 Sep 2026 14:41:53 +0200 Subject: [PATCH 3/7] fix(stack): remove the containers of a stacklet whose first up failed When a first `stack up` failed after compose had started containers, on a port conflict for instance, the stacklet had no setup-done marker. `stack destroy` then treated it as never set up and only deleted its data. The containers kept their names and ports, and the next `stack up` ran into them. A stacklet that is not installed but has containers now goes through the full destroy: confirmation, `docker compose down`, and the usual cleanup. Without containers, destroy still just deletes leftover data. --- lib/stack/cli.py | 5 ++- tests/unit/framework/test_cli_class.py | 42 ++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/lib/stack/cli.py b/lib/stack/cli.py index a18c69da..fbb9e64c 100644 --- a/lib/stack/cli.py +++ b/lib/stack/cli.py @@ -978,7 +978,10 @@ def _destroy_one(stck, args): name = stacklet.get("name", args.stacklet) data_path = stck.data / args.stacklet - if not stck.is_installed(args.stacklet): + # Not installed and no containers: only data a failed setup left behind. + # A first `up` that failed after starting containers leaves those too, + # holding their names and ports, so that case takes the full path. + if not stck.is_installed(args.stacklet) and args.stacklet not in docker.all_project_ids(): if data_path.exists(): import shutil shutil.rmtree(data_path) diff --git a/tests/unit/framework/test_cli_class.py b/tests/unit/framework/test_cli_class.py index bdebbca2..eb525ea4 100644 --- a/tests/unit/framework/test_cli_class.py +++ b/tests/unit/framework/test_cli_class.py @@ -370,6 +370,48 @@ def test_destroy_with_an_unknown_id_destroys_nothing(self, tmp_path, monkeypatch assert cli.stack.is_installed("a") +class TestDestroyAfterAFailedFirstUp: + """A first `stack up` can fail after compose has started containers, + on a port conflict for instance. The stacklet is then not installed + (no setup-done marker) but has containers, which keep their names and + ports until something removes them. `stack destroy` is that something.""" + + def _stacklet_with_compose(self, tmp_path): + cli, _ = _make_cli(tmp_path, {"myapp": {}}) + (tmp_path / "stacklets" / "myapp" / "docker-compose.yml").write_text( + "name: stack-myapp\nservices: {}\n") + (tmp_path / "data" / "myapp").mkdir(parents=True) + return cli + + def test_its_containers_are_removed(self, tmp_path, monkeypatch): + from argparse import Namespace + from stack.cli import handle_destroy + cli = self._stacklet_with_compose(tmp_path) + monkeypatch.setattr("stack.docker.project_states", lambda: {"myapp": "failing"}) + removed = [] + monkeypatch.setattr("stack.docker.compose_down", + lambda f: (removed.append(Path(f).parent.name), (0, ""))[1]) + + handle_destroy(cli.stack, Namespace(stacklet=["myapp"], yes=True)) + + assert removed == ["myapp"] + assert not (tmp_path / "data" / "myapp").exists() + + def test_without_containers_only_the_leftover_data_goes(self, tmp_path, monkeypatch): + from argparse import Namespace + from stack.cli import handle_destroy + cli = self._stacklet_with_compose(tmp_path) + monkeypatch.setattr("stack.docker.project_states", lambda: {}) + removed = [] + monkeypatch.setattr("stack.docker.compose_down", + lambda f: (removed.append(f), (0, ""))[1]) + + handle_destroy(cli.stack, Namespace(stacklet=["myapp"], yes=True)) + + assert removed == [] + assert not (tmp_path / "data" / "myapp").exists() + + class TestCLIDestroy: """CLI.destroy() runs Docker compose down then Stack.destroy().""" From 0c536bc05c5a70c7c6736bba3244ecec35c89e4e Mon Sep 17 00:00:00 2001 From: Arthur Date: Tue, 22 Sep 2026 14:42:33 +0200 Subject: [PATCH 4/7] fix(stack): show why compose failed when a stacklet cannot start `stack up` printed "Failed to start services" and dropped compose's output, which holds the reason, such as a port another container already uses. The admin had to rerun compose by hand to find out. Errors now show the last lines of the tool's output below the message. This covers `stack up`, `stack down` and `stack destroy`, which all carry compose's output in the same field. --- lib/stack/cli.py | 7 +++++++ tests/unit/framework/test_cli_class.py | 23 +++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/lib/stack/cli.py b/lib/stack/cli.py index fbb9e64c..abb41eed 100644 --- a/lib/stack/cli.py +++ b/lib/stack/cli.py @@ -639,6 +639,13 @@ def print_error(result: dict) -> None: """Print an error with stack colors.""" error = result.get("error", "unknown error") print(f"\n {RED}✗{RESET} {error}\n", file=sys.stderr) + # The tool's own output says why (compose names the port, the mount, + # the image). Its last lines carry the error; the rest is progress. + output = (result.get("output") or "").strip() + if output and output != str(error).strip(): + for line in [ln for ln in output.splitlines() if ln.strip()][-5:]: + print(f" {DIM}{line.strip()}{RESET}", file=sys.stderr) + print(file=sys.stderr) for p in result.get("problems", result.get("dependents", [])): print(f" {p}", file=sys.stderr) if result.get("hint"): diff --git a/tests/unit/framework/test_cli_class.py b/tests/unit/framework/test_cli_class.py index eb525ea4..a4ac4d12 100644 --- a/tests/unit/framework/test_cli_class.py +++ b/tests/unit/framework/test_cli_class.py @@ -370,6 +370,29 @@ def test_destroy_with_an_unknown_id_destroys_nothing(self, tmp_path, monkeypatch assert cli.stack.is_installed("a") +class TestUpFailureNamesTheCause: + """When compose cannot start a container, the reason is in its output + (a port another process holds, a missing mount), and the admin + cannot act on "Failed to start services" alone.""" + + def test_composes_error_is_shown(self, tmp_path, monkeypatch, capsys): + from argparse import Namespace + from stack.cli import handle_up + cli, _ = _make_cli(tmp_path, {"myapp": {}}) + (tmp_path / "stacklets" / "myapp" / "docker-compose.yml").write_text( + "name: stack-myapp\nservices: {}\n") + monkeypatch.setattr("stack.docker.compose_pull", lambda *a, **kw: None) + monkeypatch.setattr("stack.docker.compose_up", lambda *a, **kw: (1, ( + " Container stack-myapp Starting\n" + "Error response from daemon: Bind for 0.0.0.0:42080 failed: " + "port is already allocated\n"))) + + with pytest.raises(SystemExit): + handle_up(cli.stack, Namespace(stacklet=["myapp"], no_voice=False)) + + assert "Bind for 0.0.0.0:42080 failed: port is already allocated" in capsys.readouterr().err + + class TestDestroyAfterAFailedFirstUp: """A first `stack up` can fail after compose has started containers, on a port conflict for instance. The stacklet is then not installed From bface2eb49e5f78d7f0caac22818f909cf46f087 Mon Sep 17 00:00:00 2001 From: Arthur Date: Tue, 22 Sep 2026 14:43:48 +0200 Subject: [PATCH 5/7] fix(messages): point Element and phones at the current homeserver URL Element's homeserver URL was written into `element-config.json` once, at install. A move to domain mode, or HTTPS turned on later, left Element calling the old address. Served over https, the browser blocks calls to an http homeserver as mixed content, so nobody could log in. The `stack up messages` hint for phones named the port-mode address in every mode. `stack up messages` now sets Element's `base_url` to the Synapse URL the stack renders, leaving the rest of the file as it is, and the phone hint shows the same URL. --- stacklets/messages/hooks/on_start.py | 67 ++++++++++++----- stacklets/messages/stacklet.toml | 2 +- .../stacklets/test_messages_element_config.py | 71 +++++++++++++++++++ .../stacklets/test_messages_rate_limits.py | 2 +- 4 files changed, 123 insertions(+), 19 deletions(-) create mode 100644 tests/unit/stacklets/test_messages_element_config.py diff --git a/stacklets/messages/hooks/on_start.py b/stacklets/messages/hooks/on_start.py index 36d4f5c3..f11ca252 100644 --- a/stacklets/messages/hooks/on_start.py +++ b/stacklets/messages/hooks/on_start.py @@ -1,15 +1,23 @@ -"""Keep the rate limits famstack relaxes in Synapse's config. - -Synapse's defaults are sized for a public homeserver with strangers on -it. On a family server the ones that bite are the per-account limits: -an account may receive five invites in a burst and then one every 333 -seconds, so a bot that sets up a sixth room for the same person waits -five and a half minutes before the invite goes through. - -`homeserver.yaml` is generated once, at install, and never again, so a -limit added to the installer would reach new servers only. This hook -runs on every start and adds whichever of these keys the file lacks. -A key already present is the admin's and is left exactly as it is. +"""Keep the config files the installer writes once in step with the stack. + +`homeserver.yaml` and `element-config.json` are generated at install and +never again, so a change made later reaches neither. This hook runs on +every start and brings two things up to date. + +Rate limits. Synapse's defaults are sized for a public homeserver with +strangers on it. On a family server the ones that bite are the +per-account limits: an account may receive five invites in a burst and +then one every 333 seconds, so a bot that sets up a sixth room for the +same person waits five and a half minutes before the invite goes +through. Whichever of the relaxed keys the file lacks is added. A key +already present is the admin's and is left exactly as it is. + +Element's homeserver. Element finds Synapse through `base_url`, which is +the URL the stack hands out for Synapse and changes with it: a move to +domain mode, or HTTPS turned on. An Element served over https that still +calls an http homeserver is blocked by the browser. `base_url` follows +the rendered `SYNAPSE_PUBLIC_URL`; the rest of the file stays the +admin's. """ from __future__ import annotations @@ -34,9 +42,18 @@ def run(ctx): - conf = Path(ctx.stack.data) / "messages" / "synapse" / "homeserver.yaml" + synapse_dir = Path(ctx.stack.data) / "messages" / "synapse" + notes = [note for note in ( + _add_rate_limits(synapse_dir / "homeserver.yaml"), + _follow_homeserver_url(synapse_dir / "element-config.json", + ctx.env.get("SYNAPSE_PUBLIC_URL", "")), + ) if note] + return {"ok": True, **({"message": "; ".join(notes)} if notes else {})} + + +def _add_rate_limits(conf: Path) -> str: if not conf.exists(): - return {"ok": True} + return "" # The installer writes JSON, which Synapse reads as YAML. A file an # admin rewrote as YAML cannot be parsed without a YAML library, and @@ -44,11 +61,27 @@ def run(ctx): try: config = json.loads(conf.read_text()) except ValueError: - return {"ok": True, "message": f"{conf} is not JSON; rate limits left as they are"} + return f"{conf} is not JSON; rate limits left as they are" missing = {k: v for k, v in RATE_LIMITS.items() if k not in config} if not missing: - return {"ok": True} + return "" conf.write_text(json.dumps({**config, **missing}, indent=2)) - return {"ok": True, "message": f"added Synapse rate limits: {', '.join(missing)}"} + return f"added Synapse rate limits: {', '.join(missing)}" + + +def _follow_homeserver_url(conf: Path, synapse_url: str) -> str: + if not synapse_url or not conf.exists(): + return "" + try: + config = json.loads(conf.read_text()) + except ValueError: + return f"{conf} is not JSON; Element's homeserver left as it is" + + homeserver = config.setdefault("default_server_config", {}).setdefault("m.homeserver", {}) + if homeserver.get("base_url") == synapse_url: + return "" + homeserver["base_url"] = synapse_url + conf.write_text(json.dumps(config, indent=2)) + return f"Element now connects to {synapse_url}" diff --git a/stacklets/messages/stacklet.toml b/stacklets/messages/stacklet.toml index 4ecbafe4..3f105ede 100644 --- a/stacklets/messages/stacklet.toml +++ b/stacklets/messages/stacklet.toml @@ -29,7 +29,7 @@ login_field = "username" hints = [ "Open {url} in your browser", - "On phones: install Element, use http://{ip}:42031 as the server", + "On phones: install Element, use {messages_synapse_url} as the server", ] # Secondary ports — the main 'port' is Element (web client), but Synapse diff --git a/tests/unit/stacklets/test_messages_element_config.py b/tests/unit/stacklets/test_messages_element_config.py new file mode 100644 index 00000000..b2d7cecd --- /dev/null +++ b/tests/unit/stacklets/test_messages_element_config.py @@ -0,0 +1,71 @@ +"""Element follows the homeserver URL the stack hands out. + +Element Web finds Synapse through `base_url` in `element-config.json`, +which the installer writes once. The URL it holds changes when the +instance moves to domain mode, and again when `dns_provider` turns on +HTTPS: an Element served over https that still calls an http homeserver +is blocked by the browser as mixed content, and the family sees a login +page that cannot log in. `on_start` sets `base_url` to the rendered +`SYNAPSE_PUBLIC_URL` on every start. Everything else in the file is left +as the admin has it. +""" + +from __future__ import annotations + +import importlib.util +import json +import sys +from pathlib import Path +from types import SimpleNamespace + +_REPO_ROOT = Path(__file__).resolve().parent.parent.parent.parent +sys.path.insert(0, str(_REPO_ROOT / "lib")) + +_spec = importlib.util.spec_from_file_location( + "messages_hooks_on_start_element", + _REPO_ROOT / "stacklets" / "messages" / "hooks" / "on_start.py") +assert _spec and _spec.loader +on_start = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(on_start) + +PORT_MODE_URL = "http://192.0.2.10:42031" +DOMAIN_URL = "https://messages.home.example.family" + + +def _element_config(tmp_path, base_url: str) -> Path: + conf = tmp_path / "messages" / "synapse" / "element-config.json" + conf.parent.mkdir(parents=True) + conf.write_text(json.dumps({ + "default_server_config": { + "m.homeserver": {"base_url": base_url, "server_name": "simpson"}}, + "brand": "Simpsons Chat", + "default_theme": "light", + })) + return conf + + +def _start(tmp_path, synapse_url: str): + return on_start.run(SimpleNamespace( + stack=SimpleNamespace(data=tmp_path), + env={"SYNAPSE_PUBLIC_URL": synapse_url})) + + +def test_element_follows_the_homeserver_to_domain_mode(tmp_path): + conf = _element_config(tmp_path, PORT_MODE_URL) + _start(tmp_path, DOMAIN_URL) + homeserver = json.loads(conf.read_text())["default_server_config"]["m.homeserver"] + assert homeserver["base_url"] == DOMAIN_URL + assert homeserver["server_name"] == "simpson" + + +def test_the_rest_of_the_config_stays_the_admins(tmp_path): + conf = _element_config(tmp_path, PORT_MODE_URL) + _start(tmp_path, DOMAIN_URL) + config = json.loads(conf.read_text()) + assert config["brand"] == "Simpsons Chat" + assert config["default_theme"] == "light" + + +def test_before_the_installer_has_written_it_nothing_is_created(tmp_path): + _start(tmp_path, DOMAIN_URL) + assert not (tmp_path / "messages" / "synapse" / "element-config.json").exists() diff --git a/tests/unit/stacklets/test_messages_rate_limits.py b/tests/unit/stacklets/test_messages_rate_limits.py index 02d2e8cf..c8aa6222 100644 --- a/tests/unit/stacklets/test_messages_rate_limits.py +++ b/tests/unit/stacklets/test_messages_rate_limits.py @@ -37,7 +37,7 @@ def _server(tmp_path, config) -> Path: def _start(tmp_path): - return on_start.run(SimpleNamespace(stack=SimpleNamespace(data=tmp_path))) + return on_start.run(SimpleNamespace(stack=SimpleNamespace(data=tmp_path), env={})) def test_an_existing_server_gains_the_invite_limit(tmp_path): From 7f8a9267404a55bfbca293678255ce45dd2a0ee1 Mon Sep 17 00:00:00 2001 From: Arthur Date: Tue, 22 Sep 2026 14:44:45 +0200 Subject: [PATCH 6/7] fix(stack): rebuild locally built images on current base images Stacklets with `build = true` (agent, core, infra, memory) were built without `--pull`, so the build cache kept the base image first pulled for good. Watchtower does not update locally built images, so security fixes in those bases, and newer Caddy releases for infra, never arrived. `stack up` now builds with `--pull`. An unchanged base costs a registry lookup and the cached layers are reused; a newer one triggers a rebuild. When the pull fails, offline for instance, the build runs again from the cache so the stacklet still starts. --- lib/stack/docker.py | 18 +++++++++-- tests/unit/framework/test_docker_runtime.py | 34 +++++++++++++++++++++ 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/lib/stack/docker.py b/lib/stack/docker.py index 7ff8d07d..f1d34045 100644 --- a/lib/stack/docker.py +++ b/lib/stack/docker.py @@ -101,12 +101,24 @@ def compose_pull(compose_file: str | Path, env: dict | None = None) -> None: def compose_build(compose_file: str | Path, env: dict | None = None) -> None: - """Build images for a compose file. Streams output.""" + """Build images for a compose file, on current base images. Streams output. + + Without `--pull` the build cache keeps the base image it first pulled + for good, and Watchtower does not update built images, so the base + would never be patched. With it, a new base means a rebuild and an + unchanged one costs a registry lookup. Offline the pull fails, and + the build runs again from what is cached so the stacklet still starts. + """ full_env = {**__import__("os").environ, **(env or {})} - _docker( - "compose", "-f", str(compose_file), "build", + pulled = _docker( + "compose", "-f", str(compose_file), "build", "--pull", timeout=600, env=full_env, ) + if pulled.returncode != 0: + _docker( + "compose", "-f", str(compose_file), "build", + timeout=600, env=full_env, + ) def exec_in(container: str, *cmd: str) -> tuple[int, str]: diff --git a/tests/unit/framework/test_docker_runtime.py b/tests/unit/framework/test_docker_runtime.py index b9b4421c..e0dbb5d2 100644 --- a/tests/unit/framework/test_docker_runtime.py +++ b/tests/unit/framework/test_docker_runtime.py @@ -177,3 +177,37 @@ def test_real_failures_still_propagate(self): def test_success_is_untouched(self): assert self._run(0, "") == (0, "") + + +class TestComposeBuildRefreshesBaseImages: + """A locally built image starts from a base image (caddy:2, a Python + slim image). Without `--pull` the build cache reuses the base it + first pulled for good: Watchtower does not touch built images, so + that base, and whatever it carries, would never be patched. With it, + a rebuild picks up a newer base when there is one and costs a + registry lookup when there is not. + + Offline the pull fails, and a stacklet must still start from what is + cached, so the build runs again without it.""" + + def _build(self, *returncodes): + from stack import docker + docker._context = None + + results = [] + for code in returncodes: + result = MagicMock() + result.returncode = code + results.append(result) + with patch("subprocess.run", side_effect=results) as run: + docker.compose_build("/tmp/compose.yml") + return [c[0][0] for c in run.call_args_list] + + def test_the_build_pulls_newer_base_images(self): + (cmd,) = self._build(0) + assert cmd[cmd.index("build"):] == ["build", "--pull"] + + def test_offline_it_builds_from_the_cached_base(self): + pulled, cached = self._build(1, 0) + assert "--pull" in pulled + assert cached[cached.index("build"):] == ["build"] From b61c9c01f07185772cbfe5eed4cd2f28572da1ec Mon Sep 17 00:00:00 2001 From: Arthur Date: Tue, 22 Sep 2026 14:46:26 +0200 Subject: [PATCH 7/7] fix(code): make SSH clone URLs work in domain mode In domain mode Forgejo's SSH port was published on the Mac's loopback only, like every service behind the proxy, and the clone URLs Forgejo and `stack up code` showed named the LAN IP instead of the web host. Neither worked from another machine. - SSH is published on every interface in both modes. It does not go through Caddy, and it is encrypted and authenticated by itself. - The clone URLs name the host of the web UI: `code.` in domain mode, the Mac's address in port mode. A new template variable, `{url_host}`, is the host of a stacklet's `{url}`. --- docs/stack-reference.md | 5 +++ lib/stack/stack.py | 17 +++++++-- stacklets/code/docker-compose.yml | 9 +++-- stacklets/code/stacklet.toml | 6 ++- tests/unit/stacklets/test_code_ssh.py | 53 +++++++++++++++++++++++++++ 5 files changed, 81 insertions(+), 9 deletions(-) create mode 100644 tests/unit/stacklets/test_code_ssh.py diff --git a/docs/stack-reference.md b/docs/stack-reference.md index 5f7255dd..3e11f7f5 100644 --- a/docs/stack-reference.md +++ b/docs/stack-reference.md @@ -203,6 +203,7 @@ Available template variables: | `{ai_tts_voice}` | Derived from `[ai].language` | | `{messages_server_name}` | `stack.toml` → `[messages].server_name` | | `{url}`, `{_url}`, `{home_url}` | Public URLs: `http://:` in port mode; `https://.` in domain mode once `[core].dns_provider` is set, `http://` before that | +| `{url_host}` | The host of `{url}`: `.` in domain mode, `{ip}` in port mode. For a protocol the stacklet serves under the same name besides HTTP, such as Forgejo's SSH | | `{__}` | A secret from `secrets.toml`, e.g. `{docs__API_TOKEN}`, `{infra__DNS_API_TOKEN}` | ### Hints @@ -822,6 +823,10 @@ The `PORT_BIND_IP` variable controls access scope. The runtime sets it: - Port mode: `0.0.0.0` (reachable from the LAN) - Domain mode: `127.0.0.1` (only Caddy reaches it) +A port the LAN needs in domain mode too is bound to `0.0.0.0` literally +instead: infra's DNS (53) and proxy (80, 443), and Forgejo's SSH, which +does not go through Caddy and is encrypted and authenticated by itself. + The host port is the stacklet's declared `port` from `stacklet.toml`. The container port is whatever the upstream service uses internally. diff --git a/lib/stack/stack.py b/lib/stack/stack.py index 3e7cf0ab..d27991db 100644 --- a/lib/stack/stack.py +++ b/lib/stack/stack.py @@ -492,6 +492,17 @@ def _public_url(self, stacklet_id: str, port: int) -> str: return f"{self._domain_scheme()}://{stacklet_id}.{domain}" return f"http://{self._lan_ip()}:{port}" + def _own_template_vars(self, stacklet_id: str, port: int) -> dict: + """The template variables whose value depends on the stacklet. + + `{url_host}` is the host of `{url}`: `.` in domain + mode, the address `{ip}` names in port mode. It is for a stacklet + that serves another protocol under the same name, such as SSH. + """ + from urllib.parse import urlsplit + url = self._public_url(stacklet_id, port) + return {"url": url, "url_host": urlsplit(url).hostname or "", "ip": self._lan_ip()} + def _domain_scheme(self) -> str: """https once the proxy serves certificates, http until then. @@ -549,8 +560,7 @@ def env(self, stacklet_id: str) -> dict: template_vars = self._build_template_vars() template_vars["stacklet_id"] = stacklet_id - template_vars["url"] = self._public_url(stacklet_id, s.get("port", 0)) - template_vars["ip"] = self._lan_ip() + template_vars.update(self._own_template_vars(stacklet_id, s.get("port", 0))) # Render templates — warn on missing vars (typos cause silent failures) import re @@ -1175,8 +1185,7 @@ def _step(msg): # Render manifest hints with template vars so credentials are visible template_vars = self._build_template_vars() - template_vars["url"] = self._public_url(stacklet_id, s.get("port", 0)) - template_vars["ip"] = self._lan_ip() + template_vars.update(self._own_template_vars(stacklet_id, s.get("port", 0))) raw_hints = manifest.get("hints", []) hints = [] for h in raw_hints: diff --git a/stacklets/code/docker-compose.yml b/stacklets/code/docker-compose.yml index 1d14fdde..526ba553 100644 --- a/stacklets/code/docker-compose.yml +++ b/stacklets/code/docker-compose.yml @@ -1,7 +1,7 @@ # Forgejo — lightweight self-hosted Git server # -# Web UI: http://:42040 -# SSH: ssh://git@:222/user/repo.git +# Web UI: http://:42040, or https://code. in domain mode +# SSH: ssh://git@:222/user/repo.git, or code. in place of # # First user to register becomes admin. # All data stored in CODE_DATA_DIR (~/famstack-data/code/). @@ -20,7 +20,10 @@ services: - stack ports: - "${PORT_BIND_IP:-0.0.0.0}:42040:3000" - - "${PORT_BIND_IP:-0.0.0.0}:${CODE_SSH_PORT:-222}:22" + # SSH does not go through the proxy, so it faces the LAN in both + # modes. It is encrypted and authenticated by itself, which is what + # keeping plain-HTTP services on the loopback in domain mode is for. + - "0.0.0.0:${CODE_SSH_PORT:-222}:22" volumes: - ${CODE_DATA_DIR}:/data - /etc/localtime:/etc/localtime:ro diff --git a/stacklets/code/stacklet.toml b/stacklets/code/stacklet.toml index f89673ee..d584de74 100644 --- a/stacklets/code/stacklet.toml +++ b/stacklets/code/stacklet.toml @@ -18,7 +18,7 @@ login_field = "username" hints = [ "Open {url}", - "SSH clone: ssh://git@{ip}:222/user/repo.git", + "SSH clone: ssh://git@{url_host}:222/user/repo.git", ] [upstream] @@ -34,7 +34,9 @@ admin_email = "{admin_email}" admin_password = "{admin_password}" FORGEJO__server__ROOT_URL = "{code_url}/" FORGEJO__server__DOMAIN = "{ip}" -FORGEJO__server__SSH_DOMAIN = "{ip}" +# The host in the SSH clone URLs Forgejo shows: the web UI's host +# (code., or the Mac's address in port mode). +FORGEJO__server__SSH_DOMAIN = "{url_host}" FORGEJO__server__SSH_PORT = "222" FORGEJO__database__DB_TYPE = "sqlite3" FORGEJO__security__INSTALL_LOCK = "true" diff --git a/tests/unit/stacklets/test_code_ssh.py b/tests/unit/stacklets/test_code_ssh.py new file mode 100644 index 00000000..371f706f --- /dev/null +++ b/tests/unit/stacklets/test_code_ssh.py @@ -0,0 +1,53 @@ +"""Git over SSH works in both hosting modes. + +Forgejo shows an SSH clone URL on every repository page, and the code +stacklet's hint prints one. In domain mode the port was published on +the Mac's loopback only, like every service behind the proxy, and the +URL named the LAN IP rather than the host the web UI is on. Neither +worked from another machine. + +SSH does not go through Caddy: it is encrypted and authenticated on its +own, so it is published on every interface in both modes, and the clone +URL names the same host as the stacklet's web URL. +""" + +from __future__ import annotations + +import re +import sys +from pathlib import Path + +import pytest + +REPO = Path(__file__).resolve().parent.parent.parent.parent +sys.path.insert(0, str(REPO / "lib")) + +from stack import Stack # noqa: E402 + + +@pytest.fixture +def stack(tmp_path): + """The repo's stacklets over a throwaway instance.""" + def _make(core: str) -> Stack: + (tmp_path / "stack.toml").write_text( + f'[core]\n{core}\nhost = "192.0.2.10"\n' + f'extension_dirs = ["{tmp_path / "ext"}"]\n') + (tmp_path / ".stack").mkdir(exist_ok=True) + return Stack(root=REPO, data=tmp_path / "data", instance_dir=tmp_path) + return _make + + +def test_in_domain_mode_the_clone_url_names_the_web_host(stack): + env = stack('domain = "home.example.family"').env("code") + assert env["FORGEJO__server__SSH_DOMAIN"] == "code.home.example.family" + + +def test_in_port_mode_it_names_the_macs_address(stack): + env = stack('domain = ""').env("code") + assert env["FORGEJO__server__SSH_DOMAIN"] == "192.0.2.10" + + +def test_ssh_is_published_on_every_interface(): + compose = (REPO / "stacklets" / "code" / "docker-compose.yml").read_text() + (ssh,) = re.findall(r'-\s*"([^"]*):22"', compose) + assert ssh.startswith("0.0.0.0:")