From 098b0caffc8b828a769ce5271b7465b0bda5f3f6 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Thu, 24 Sep 2026 22:42:55 +0200 Subject: [PATCH] fix(ci): offer multi to bond-less rows whose deploy pulls Tor in A role without a Tor bond only took clearnet rows, and clearnet rows disable Tor. svc-ai-litellm (variants 0 and 2) and web-app-seaweedfs (variant 0) still pull the Tor provider in through their dependencies, so they ran on a node with Tor about half the time under the tor axis and never under the network axis. row_states now gives such a row clearnet and multi. Whether the deploy pulls the provider in is read from the services closure the discovery row already carries, which matched the deploy's own include resolution on all 33 rows without a bond. A row whose deploy contains no Tor provider stays clearnet only, because NETWORK_MODE=multi without the provider fails the deploy, and a gate pinned false still means clearnet. The matrix and the selection validator pass the same flag, so a +multi pin agrees with the matrix. tor_capable lost its last caller and is gone. Co-Authored-By: Claude Opus 5.5 (1M context) --- .github/workflows/README.md | 13 +++-- cli/meta/ci/validate.py | 12 +++-- docs/contributing/design/network_modes.md | 2 +- .../unit/python/cli/meta/ci/test_validate.py | 30 ++++++++++- .../python/utils/github/variant/test_axes.py | 50 ++++++++++++----- utils/github/variant/axes.py | 8 ++- utils/github/variant/network.py | 53 ++++++++++--------- 7 files changed, 117 insertions(+), 51 deletions(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index e6c15f8cff..9484520859 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -127,8 +127,9 @@ A 3-variant role offering compose and swarm therefore becomes An explicit `network` input still wins over the full coverage: `clearnet`, `tor` and `multi` are operator narrowings. A variant that pins -`services.tor.enabled` to false, or a role without a `tor` bond, only ever runs -on clearnet regardless. +`services.tor.enabled` to false only ever runs on clearnet regardless, and so +does a role without a `tor` bond unless its deploy pulls the Tor provider in, +which adds `multi`. ### Selection tokens @@ -172,9 +173,11 @@ demand, failing the retrigger on the very condition the fallback absorbs. A row deploys its node in one network mode, handed to the inventory as `NETWORK_MODE`. The modes a row can take follow its role -([network_modes.md](../../docs/contributing/design/network_modes.md)): a role -without a `tor` bond, or a variant that pins `services.tor.enabled` false, only -takes `clearnet`; `reachability.modes` drops every mode that needs a network +([network_modes.md](../../docs/contributing/design/network_modes.md)): a +variant that pins `services.tor.enabled` false only takes `clearnet`; a role +without a `tor` bond takes `clearnet`, plus `multi` when the `services` closure +of its discovery row names the Tor provider, where the node runs Tor and the +role stays on clearnet; `reachability.modes` drops every mode that needs a network the role excludes; `reachability.single_mode` drops `multi`; the Tor provider never takes `clearnet`. The `network` input decides what the axis is allowed to do at all: diff --git a/cli/meta/ci/validate.py b/cli/meta/ci/validate.py index a471e12d29..653b91c06f 100644 --- a/cli/meta/ci/validate.py +++ b/cli/meta/ci/validate.py @@ -75,12 +75,13 @@ def problems( if not pins: return [], [] - rows = { - (row["name"], row["variant"]): query.row_modes(row, modes) + discovered = { + (row["name"], row["variant"]): row for row in query.discover_rows( modes, whitelist=selection.names(pins), lifecycles=lifecycles ) } + rows = {key: query.row_modes(row, modes) for key, row in discovered.items()} discovered_apps = {app for app, _variant in rows} declared = get_variants() errors: list[str] = [] @@ -117,7 +118,12 @@ def problems( pin_network=pin.network, pin_distro=pin.distro, pin_filesystem=pin.filesystem, - states=network.row_states(pin.app, variant, declared), + states=network.row_states( + pin.app, + variant, + declared, + pulls_tor=network.row_pulls_tor(discovered[(pin.app, variant)]), + ), network_input=network_input, distros=distros, filesystems=filesystems, diff --git a/docs/contributing/design/network_modes.md b/docs/contributing/design/network_modes.md index 9a9516f2f8..1fde924c97 100644 --- a/docs/contributing/design/network_modes.md +++ b/docs/contributing/design/network_modes.md @@ -61,7 +61,7 @@ The effective networks of a role are the node's networks that the role allows, w ## CI 🚀 -- [network.py](../../../utils/github/variant/network.py) derives the node modes a matrix row can take from the role's Tor bond and `reachability`; the `network` input (`auto`, `clearnet`, `tor`, `multi`) narrows them. See [README.md](../../../.github/workflows/README.md). +- [network.py](../../../utils/github/variant/network.py) derives the node modes a matrix row can take from the role's Tor bond, `reachability` and whether its deploy pulls the Tor provider in; the `network` input (`auto`, `clearnet`, `tor`, `multi`) narrows them. See [README.md](../../../.github/workflows/README.md). - A row hands its mode to the deploy as the `network` variable, which provisioning writes into the inventory as `NETWORK_MODE` ([network_mode.py](../../../cli/administration/inventory/provision/network_mode.py)). `make compose-deploy apps= network=tor` does the same locally. - [test-e2e-playwright](../../../roles/test-e2e-playwright/) runs the role's suite once, on the vhost of one network sibling. On a node serving the role on several networks the sibling is picked at random and kept for the flake retry, and [network-leak.spec.js](../../../roles/test-e2e-playwright/files/network-leak.spec.js) joins the run with the guest persona. The leak spec fails when a document requests a host of another network or the browser blocks mixed content; the SSO issuer is the one host every network may reach. diff --git a/tests/unit/python/cli/meta/ci/test_validate.py b/tests/unit/python/cli/meta/ci/test_validate.py index f2ab826de0..bea22fe379 100644 --- a/tests/unit/python/cli/meta/ci/test_validate.py +++ b/tests/unit/python/cli/meta/ci/test_validate.py @@ -6,7 +6,7 @@ import unittest.mock as mock from contextlib import redirect_stderr, redirect_stdout from cli.meta.ci import validate -from utils.github.variant import axes +from utils.github.variant import axes, network _ROWS = [ {"name": "web-app-a", "variant": 0, "test_compose": True, "test_swarm": True}, @@ -91,6 +91,34 @@ class TestProblems(unittest.TestCase): ) self.assertEqual((errors, warnings), ([], [])) + def test_a_role_without_a_bond_may_pin_multi_when_its_deploy_pulls_tor( + self, + ) -> None: + rows = [ + { + "name": "web-app-c", + "variant": 0, + "test_compose": True, + "services": [network.tor_provider()], + } + ] + with ( + mock.patch.object(validate.query, "discover_rows", return_value=rows), + mock.patch.object( + validate, "get_variants", return_value={"web-app-c": [{"services": {}}]} + ), + ): + problems = validate.problems( + "web-app-c#0@compose+multi", + modes=("compose",), + network_input="auto", + distros=axes.DISTROS, + filesystems=axes.FILESYSTEMS, + lifecycles="", + label="priority", + ) + self.assertEqual(problems, ([], [])) + def test_a_warning_still_separates_a_missing_role_from_a_present_one(self) -> None: _errors, warnings = _problems("web-app-a web-app-gone web-app-b") self.assertEqual(len(warnings), 1) diff --git a/tests/unit/python/utils/github/variant/test_axes.py b/tests/unit/python/utils/github/variant/test_axes.py index 2713c8f96c..822cbafed2 100644 --- a/tests/unit/python/utils/github/variant/test_axes.py +++ b/tests/unit/python/utils/github/variant/test_axes.py @@ -74,23 +74,28 @@ class TestResolveSweep(unittest.TestCase): self.assertEqual(axes.resolve_sweep(""), 0) -class TestTorCapable(unittest.TestCase): - def test_a_variant_pinning_the_gate_false_is_incapable(self) -> None: - self.assertFalse(network.tor_capable("web-app-a", 1, _VARIANTS)) - - def test_a_variant_pinning_the_gate_true_is_capable(self) -> None: - self.assertTrue(network.tor_capable("web-app-a", 0, _VARIANTS)) - - def test_a_reactive_gate_counts_as_capable(self) -> None: - self.assertTrue(network.tor_capable("web-app-b", 0, _VARIANTS)) - - def test_a_role_without_a_tor_bond_is_incapable(self) -> None: - self.assertFalse(network.tor_capable("web-app-c", 0, _VARIANTS)) - - class TestRowStates(unittest.TestCase): def test_a_capable_row_takes_every_network_mode(self) -> None: self.assertEqual(network.row_states("web-app-b", 0, _VARIANTS), _ALL) + self.assertEqual(network.row_states("web-app-a", 0, _VARIANTS), _ALL) + + def test_a_role_without_a_bond_takes_multi_when_its_deploy_pulls_tor(self) -> None: + self.assertEqual( + network.row_states("web-app-c", 0, _VARIANTS, pulls_tor=True), + ("clearnet", "multi"), + ) + + def test_a_gate_pinned_false_ignores_a_pulled_in_provider(self) -> None: + self.assertEqual( + network.row_states("web-app-a", 1, _VARIANTS, pulls_tor=True), + ("clearnet",), + ) + + def test_the_services_closure_decides_whether_tor_is_pulled_in(self) -> None: + provider = network.tor_provider() + self.assertTrue(network.row_pulls_tor({"services": ["svc-db-x", provider]})) + self.assertFalse(network.row_pulls_tor({"services": ["svc-db-x"]})) + self.assertFalse(network.row_pulls_tor({})) def test_an_incapable_row_only_takes_clearnet(self) -> None: self.assertEqual(network.row_states("web-app-a", 1, _VARIANTS), ("clearnet",)) @@ -415,6 +420,23 @@ class TestAssign(unittest.TestCase): [], ) + def test_a_row_without_a_bond_rides_the_multi_node_its_deploy_pulls_tor_into( + self, + ) -> None: + provider = network.tor_provider() + pulling = [_row("web-app-c", 0, ("compose",), services=[provider])] + entries = _assign( + pulling, sweep=0, network_input="multi", variants_per_app=_VARIANTS + ) + self.assertEqual( + [(e["network"], e["disable"]) for e in entries], [("multi", "")] + ) + plain = [_row("web-app-c", 0, ("compose",))] + self.assertEqual( + _assign(plain, sweep=0, network_input="multi", variants_per_app=_VARIANTS), + [], + ) + def test_the_clearnet_input_serves_every_row_without_tor(self) -> None: rows = [_row("web-app-a", 0, ("compose",)), _row("web-app-a", 1, ("compose",))] entries = _assign( diff --git a/utils/github/variant/axes.py b/utils/github/variant/axes.py index 4196a9c4f8..9b92c418d9 100644 --- a/utils/github/variant/axes.py +++ b/utils/github/variant/axes.py @@ -49,6 +49,7 @@ from utils.github.variant.network import ( combinations, disabled_services, network_states, + row_pulls_tor, row_states, ) from utils.github.variant.pools import DISTROS, FILESYSTEMS, rotate @@ -290,7 +291,8 @@ def assign( Args: rows: discovery rows, each carrying ``name``, ``variant``, ``modes`` - (the offered subset) and ``priority``, in global query order. + (the offered subset), ``priority`` and the ``services`` closure + (:func:`row_pulls_tor`), in global query order. ``pin_mode``/``pin_network``/``pin_distro``/``pin_filesystem``, when a selection token set them, pin that axis: the priority line then covers only the combinations that match, and the regular line @@ -332,7 +334,9 @@ def assign( app = row["name"] variant = row.get("variant") priority = bool(row.get("priority")) - states = row_states(app, variant, variants_per_app) + states = row_states( + app, variant, variants_per_app, pulls_tor=row_pulls_tor(row) + ) offered = tuple(row["modes"]) variant_csv = "" if variant is None else str(variant) pin_mode = row.get("pin_mode") diff --git a/utils/github/variant/network.py b/utils/github/variant/network.py index 2e361f3c06..1acf3d11a2 100644 --- a/utils/github/variant/network.py +++ b/utils/github/variant/network.py @@ -60,31 +60,18 @@ def _tor_flag(config: Mapping[str, Any]) -> Any: return tor.get("enabled") if isinstance(tor, dict) else None -def tor_capable( +def _row_tor_flag( app: str, - variant: int | None = None, - variants_per_app: Mapping[str, Sequence[Mapping[str, Any]]] | None = None, -) -> bool: - """Whether a matrix row may be deployed on a node that runs Tor. - - Args: - app: application id. - variant: the row's variant index; ``None`` for a role declaring none. - variants_per_app: rendered variant configs per app; ``None`` falls back - to the role's base ``meta/services.yml``. - - Returns: - ``False`` when the role has no ``tor`` bond or the covered variant pins - ``services.tor.enabled`` to a literal false, ``True`` otherwise. A - reactive Jinja flag counts as capable. Consulting the variant matters: - roles pin the gate ``true`` in variant 0 and ``false`` in the rest. - """ + variant: int | None, + variants_per_app: Mapping[str, Sequence[Mapping[str, Any]]] | None, +) -> Any: + """The row's ``services.tor.enabled``: the covered variant's value, else + the role's base ``meta/services.yml`` one; ``None`` without a Tor bond. + Roles pin the gate ``true`` in variant 0 and ``false`` in the rest.""" declared = (variants_per_app or {}).get(app) or [] if variant is None or not 0 <= variant < len(declared): - flag = _tor_flag({"services": _meta(app, ROLE_FILE_META_SERVICES)}) - else: - flag = _tor_flag(declared[variant]) - return flag is not None and flag is not False + return _tor_flag({"services": _meta(app, ROLE_FILE_META_SERVICES)}) + return _tor_flag(declared[variant]) def _reachability( @@ -110,10 +97,17 @@ def tor_provider() -> str | None: return role if isinstance(role, str) and role else None +def row_pulls_tor(row: Mapping[str, Any]) -> bool: + """Whether a discovery row's ``services`` closure takes the Tor provider in.""" + return tor_provider() in (row.get("services") or ()) + + def row_states( app: str, variant: int | None = None, variants_per_app: Mapping[str, Sequence[Mapping[str, Any]]] | None = None, + *, + pulls_tor: bool = False, ) -> tuple[str, ...]: """Every node network mode the row's role can be served in. @@ -121,15 +115,24 @@ def row_states( app: application id. variant: the row's variant index; ``None`` for a role declaring none. variants_per_app: rendered variant configs per app. + pulls_tor: the row's deploy takes the Tor provider in + (:func:`row_pulls_tor`). Returns: - ``clearnet`` alone for a row that cannot take Tor. Otherwise the modes - whose networks all lie inside the role's ``reachability.modes``, + ``clearnet`` alone when the covered variant pins the Tor gate to a + literal false, or when the role has no Tor bond and nothing pulls the + provider in. ``clearnet`` and ``multi`` for a role without a bond whose + deploy pulls the provider in anyway: the node then runs Tor while the + role stays on clearnet. Otherwise, a reactive Jinja gate included, the + modes whose networks all lie inside the role's ``reachability.modes``, without ``multi`` for a ``single_mode`` role and without ``clearnet`` for the Tor provider, which disabling Tor would strip out of its own deploy. """ - if not tor_capable(app, variant, variants_per_app): + flag = _row_tor_flag(app, variant, variants_per_app) + if flag is None: + return (CLEARNET, MULTI) if pulls_tor else (CLEARNET,) + if flag is False: return (CLEARNET,) modes, single_mode = _reachability(app, variant, variants_per_app) allowed = modes or tuple(NETWORKS)