refactor(svc-net-tor): declare the role's config in the role, and the onion's HTTP forward at all

group_vars/all/README.md forbids role-specific configuration in that directory, and 19_tor.yml held five variables no role but svc-net-tor ever read. They now live where the README points: the values an operator may override in meta/services.yml, where the inventory's applications block is deep-merged on top, and the derived ones in vars/main.yml.

The exception is the egress virtual range, which moves the other way into 08_networks.yml. It is not a property of the role but a claim on shared address space, and 0f6728a4c1 records what a collision costs: tor's upstream default 10.192.0.0/10 swallowed the docker pool at 10.208.0.0/12, and every connection to a container address came back as 'errno 104 Connection reset by peer'. The two numbers that must not overlap now sit one line apart.

The onion's HTTP forward becomes a declaration rather than a derivation. lookup('tor_ports') walks group_names and collects what those roles declare under ports.onion; port 80 belongs to svc-prx-openresty, which a node acquires through sys-stk-front-proxy and which therefore appears in no dependency list an inventory carries. A node provisioned with --include svc-net-tor published an onion forwarding SSH and nothing else, leaving every app it serves unreachable - measured as ERR_SOCKS_CONNECTION_FAILED in the CI instructions replay. The new role-local tor_extra_ports lookup composes the flagged forwards with the derived ones and collapses a port named twice, since torrc writes one HiddenServicePort line per entry.

Not moved: tor_ports stays in plugins/lookup because tor_reachable imports from it and three database roles use that; and the flags sit outside ports.onion because every key there mirrors a port the role declares under ports.local, which neither sshd nor the front proxy is.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

group_vars/all/08_networks.yml

@@ -21,6 +21,7 @@ NETWORK_TRUSTED_PROXY_CIDRS: "{{ ['127.0.0.1', '::1'] + (NETWORK_PRIVATE_CIDRS.v
 NETWORK_DOCKER_ADDRESS_POOLS:
   - base: "10.208.0.0/12"
     size: 24
+NETWORK_TOR_VIRTUAL_CIDR: "10.240.0.0/12" # Range tor answers for .onion instead of routing; must not overlap the pool above
 NETWORK_CONTAINER_BRIDGE_RESOLVER: false
 NETWORK_NESTED_DOCKER_BIP: "192.168.67.1/24"
 NETWORK_NESTED_DOCKER_ADDRESS_POOLS:

group_vars/all/19_tor.yml

@@ -1,7 +0,0 @@
----
-TOR_ONION_SSH_ENABLED: true
-
-TOR_EGRESS_ROUTER_ENABLED: true
-TOR_EGRESS_VIRTUAL_NET_IPV4: "10.240.0.0/12"
-
-TOR_ONION_EXTRA_PORTS: "{{ ([{'onion_port': 22, 'target': '127.0.0.1:22'}] if (TOR_ONION_SSH_ENABLED | bool) else []) + lookup('tor_ports') }}"

plugins/lookup/tor_ports.py

@@ -143,8 +143,8 @@ class LookupModule(LookupBase):
     Both are read from the variant-merged applications view, so a variant that
     drops the opt-in drops the forward with it. The published port binds the
     host interface, so the loopback target reaches it from svc-net-tor's
-    host-network container. Consumed by ``TOR_ONION_EXTRA_PORTS``
-    (group_vars/all/19_tor.yml) and rendered as ``HiddenServicePort`` lines in
+    host-network container. Composed with the flagged forwards by
+    ``lookup('tor_extra_ports')`` and rendered as ``HiddenServicePort`` lines in
     svc-net-tor's torrc.
     """
 

roles/svc-net-tor/lookup_plugins/tor_extra_ports.py

@@ -0,0 +1,80 @@
+"""Lookup `tor_extra_ports`: every port the node onion forwards.
+
+Two of them are flags rather than derivations, because no inventory carries
+them. ``lookup('tor_ports')`` walks ``group_names`` and collects what the roles
+listed there declare under ``ports.onion``; SSH belongs to no role at all, and
+HTTP belongs to ``svc-prx-openresty``, which a node acquires through
+``sys-stk-front-proxy`` and which therefore appears in no dependency list an
+inventory holds. A node provisioned with ``--include svc-net-tor`` alone
+published an onion forwarding SSH and nothing else, leaving every app it serves
+unreachable over that onion.
+
+Forwarding HTTP is what this role exists for, so it is declared, and a node that
+must not answer HTTP over its onion turns ``TOR_ONION_HTTP_ENABLED`` off.
+
+The derived ports still come from ``tor_ports``; a port named twice collapses,
+since ``torrc.j2`` writes one ``HiddenServicePort`` line per entry.
+
+Usage:
+
+    {{ lookup('tor_extra_ports') }}
+"""
+
+from __future__ import annotations
+
+from typing import Any
+
+from ansible.errors import AnsibleError
+from ansible.module_utils.parsing.convert_bool import boolean
+from ansible.plugins.loader import lookup_loader
+from ansible.plugins.lookup import LookupBase
+
+FLAGGED_PORTS: tuple[tuple[str, int], ...] = (
+    ("TOR_ONION_SSH_ENABLED", 22),
+    ("TOR_ONION_HTTP_ENABLED", 80),
+)
+
+
+def _entry(port: int) -> dict[str, Any]:
+    """One forward, in the shape ``torrc.j2`` consumes.
+
+    Args:
+        port: the port the onion answers on; the backend host is substituted by
+            the template, so the target carries the port alone.
+    """
+    return {"onion_port": port, "target": f"127.0.0.1:{port}"}
+
+
+class LookupModule(LookupBase):
+    def run(
+        self,
+        terms: list[Any] | None,
+        variables: dict[str, Any] | None = None,
+        **kwargs: Any,
+    ) -> list[Any]:
+        if terms:
+            raise AnsibleError("lookup('tor_extra_ports') expects no terms.")
+
+        variables = variables or getattr(self._templar, "available_variables", {}) or {}
+
+        entries: list[dict[str, Any]] = []
+        for name, port in FLAGGED_PORTS:
+            if name not in variables:
+                raise AnsibleError(f"lookup('tor_extra_ports'): {name} is not defined")
+            if boolean(variables[name]):
+                entries.append(_entry(port))
+
+        derived = lookup_loader.get(
+            "tor_ports",
+            loader=getattr(self, "_loader", None),
+            templar=getattr(self, "_templar", None),
+        ).run([], variables=variables, **kwargs)[0]
+
+        seen = {entry["onion_port"] for entry in entries}
+        for entry in derived:
+            if entry["onion_port"] in seen:
+                continue
+            seen.add(entry["onion_port"])
+            entries.append(entry)
+
+        return [entries]

roles/svc-net-tor/meta/services.yml

@@ -26,6 +26,11 @@ tor: # nocheck: category-modes
   node: ""
   use_entry_guards: true
   flavor: public
+  forwards:
+    ssh: true
+    http: true
+  egress:
+    enabled: true
   timeout_multipliers:
     chutney: 2
     public: 5

roles/svc-net-tor/vars/main.yml

@@ -9,6 +9,11 @@ TOR_CONTAINER: "{{ lookup('config', application_id, 'services.tor.name') }}"
 TOR_NODE_ONION: "{{ lookup('config', application_id, 'services.tor.node', '') }}"
 TOR_USE_ENTRY_GUARDS: "{{ lookup('config', application_id, 'services.tor.use_entry_guards') | bool }}"
 TOR_FLAVOR: "{{ lookup('config', application_id, 'services.tor.flavor') }}"
+TOR_ONION_SSH_ENABLED: "{{ lookup('config', application_id, 'services.tor.forwards.ssh') | bool }}"
+TOR_ONION_HTTP_ENABLED: "{{ lookup('config', application_id, 'services.tor.forwards.http') | bool }}"
+TOR_ONION_EXTRA_PORTS: "{{ lookup('tor_extra_ports') }}"
+TOR_EGRESS_ROUTER_ENABLED: "{{ lookup('config', application_id, 'services.tor.egress.enabled') | bool }}"
+TOR_EGRESS_VIRTUAL_NET_IPV4: "{{ NETWORK_TOR_VIRTUAL_CIDR }}"
 TOR_TRANS_PORT: "{{ lookup('config', application_id, 'services.tor.ports.local.trans') }}"
 TOR_DNS_PORT: "{{ lookup('config', application_id, 'services.tor.ports.local.dns') }}"
 TOR_SOCKS_PORT: "{{ lookup('config', application_id, 'services.tor.ports.local.socks') }}"

tests/unit/python/roles/svc-net-tor/test_extra_ports.py

@@ -0,0 +1,105 @@
+"""The node onion forwards HTTP whether or not the proxy is in the inventory.
+
+``lookup('tor_ports')`` collects what the roles in ``group_names`` declare under
+``ports.onion``. Port 80 belongs to ``svc-prx-openresty``, which a node acquires
+through ``sys-stk-front-proxy`` and which therefore appears in no dependency
+list an inventory holds. A node provisioned with ``--include svc-net-tor`` alone
+published an onion forwarding SSH and nothing else.
+
+These run the real lookup over a stubbed ``tor_ports``, so a flag that stops
+reaching the forward list, or a derived port that starts duplicating a flagged
+one, fails here rather than on the node.
+"""
+
+from __future__ import annotations
+
+import importlib.util
+import sys
+import unittest
+from typing import Any
+from unittest import mock
+
+from ansible.errors import AnsibleError
+
+from . import PROJECT_ROOT
+
+LOOKUP = "roles/svc-net-tor/lookup_plugins/tor_extra_ports.py"
+SSH_ENTRY = {"onion_port": 22, "target": "127.0.0.1:22"}
+HTTP_ENTRY = {"onion_port": 80, "target": "127.0.0.1:80"}
+ENABLED = {"TOR_ONION_SSH_ENABLED": True, "TOR_ONION_HTTP_ENABLED": True}
+
+
+def _load_lookup():
+    path = PROJECT_ROOT / LOOKUP
+    spec = importlib.util.spec_from_file_location("tor_extra_ports_lookup", path)
+    module = importlib.util.module_from_spec(spec)
+    assert spec and spec.loader
+    sys.modules["tor_extra_ports_lookup"] = module
+    spec.loader.exec_module(module)
+    return module
+
+
+class _DerivedPorts:
+    """Stands in for the tor_ports lookup the plugin composes with."""
+
+    def __init__(self, entries: list[dict[str, Any]]):
+        self.entries = entries
+
+    def run(self, _terms, **_kwargs):
+        return [self.entries]
+
+
+class TestExtraPorts(unittest.TestCase):
+    @classmethod
+    def setUpClass(cls) -> None:
+        cls.module = _load_lookup()
+
+    def _ports(self, derived: list[dict], **variables) -> list[dict]:
+        lookup = self.module.LookupModule()
+        lookup._templar = None  # noqa: SLF001 - the plugin reads variables we pass
+        lookup._loader = None  # noqa: SLF001
+        with mock.patch.object(
+            self.module.lookup_loader, "get", return_value=_DerivedPorts(derived)
+        ):
+            return lookup.run([], variables={**ENABLED, **variables})[0]
+
+    def test_http_is_forwarded_without_the_proxy_in_the_inventory(self) -> None:
+        self.assertIn(
+            HTTP_ENTRY,
+            self._ports([]),
+            "a node whose inventory lists only svc-net-tor still serves its apps "
+            "over the onion, so the forward cannot depend on the proxy being listed",
+        )
+
+    def test_a_derived_port_does_not_duplicate_a_flagged_one(self) -> None:
+        ports = self._ports([HTTP_ENTRY])
+        self.assertEqual(
+            [entry for entry in ports if entry["onion_port"] == 80],
+            [HTTP_ENTRY],
+            "torrc writes one HiddenServicePort line per entry, so a duplicate "
+            "publishes the same forward twice",
+        )
+
+    def test_derived_ports_still_reach_the_forward_list(self) -> None:
+        onion_dns = {"onion_port": 9053, "target": "127.0.0.1:9053"}
+        self.assertIn(
+            onion_dns,
+            self._ports([onion_dns]),
+            "the flags add to what the services declare, they do not replace it",
+        )
+
+    def test_a_flag_turns_its_forward_off(self) -> None:
+        ports = self._ports([], TOR_ONION_HTTP_ENABLED=False)
+        self.assertNotIn(HTTP_ENTRY, ports)
+        self.assertIn(SSH_ENTRY, ports, "the other flag is untouched")
+
+    def test_an_undeclared_flag_is_refused(self) -> None:
+        lookup = self.module.LookupModule()
+        lookup._templar = None  # noqa: SLF001
+        with self.assertRaises(AnsibleError) as raised:
+            lookup.run([], variables={"TOR_ONION_SSH_ENABLED": True})
+        self.assertIn("TOR_ONION_HTTP_ENABLED", str(raised.exception))
+
+
+if __name__ == "__main__":
+    unittest.main()