diff --git a/mender-auto-accept/test_tunnel_sync.py b/mender-auto-accept/test_tunnel_sync.py new file mode 100644 index 0000000..c9ed095 --- /dev/null +++ b/mender-auto-accept/test_tunnel_sync.py @@ -0,0 +1,101 @@ +"""Tests for the tunnel's ingress routing. + +These simulate cloudflared's own rule matching rather than asserting the shape +of the list, because the failure that matters is not a missing rule. It is a +rule that is present, reads correctly, and never fires because something above +it matched first. That produces 404s indistinguishable from a dead origin, and +it is how a hand-built tunnel in this account was misconfigured. +""" + +import re + +import pytest +import tunnel_sync + +NODE = "ret4c844c20" +HOST = f"{NODE}.{tunnel_sync.REMOTE_ACCESS_DOMAIN}" + + +def serves(path, node_id=NODE, hostname=None): + """Which service answers `path`, under cloudflared's first-match rule. + + cloudflared walks ingress in order and takes the first entry whose hostname + and path both match. Anything with no path matches every path. + """ + hostname = hostname or f"{node_id}.{tunnel_sync.REMOTE_ACCESS_DOMAIN}" + for rule in tunnel_sync.build_ingress(node_id): + if "hostname" in rule and rule["hostname"] != hostname: + continue + if "path" in rule and not re.search(rule["path"], path): + continue + return rule["service"] + raise AssertionError("ingress has no catch-all") + + +# ── the three views support actually uses ──────────────────────── + +@pytest.mark.parametrize("path", [ + "/display/map/", # Passive Radar + "/display/maxhold/", # Max-hold + "/controller/", # Controller + "/lib/blah2.css", # assets those pages load + "/js/plot_map.js", +]) +def test_blah2_pages_reach_the_web_port(path): + assert serves(path) == tunnel_sync.BLAH2_WEB_SERVICE + + +@pytest.mark.parametrize("path", [ + "/api/timestamp", "/api/detection", "/api/map", + "/api/adsb2dd", "/api/config", + "/capture/toggle", "/stash/detection", +]) +def test_blah2_data_reaches_the_api_port(path): + """The pages are useless without these; they are what the JS calls once it + is same-origin, which it is whenever the host is not localhost.""" + assert serves(path) == tunnel_sync.BLAH2_API_SERVICE + + +# ── the collision that would break the interface ───────────────── + +@pytest.mark.parametrize("path", [ + "/api/mode", + "/api/mode/release-spectrum", + "/api/fleet/peers", + "/api/spectrum/ready", + "/api/sdrconnect/ready", +]) +def test_retina_gui_keeps_its_own_api(path): + """The reason blah2's endpoints are named one by one instead of matching + `^/api`. These belong to the GUI, share the hostname, and a blanket prefix + would divert them to blah2: broken over the tunnel, fine on the LAN, so + nobody would find it until support tried to use it.""" + assert serves(path) == tunnel_sync.REMOTE_ACCESS_SERVICE + + +@pytest.mark.parametrize("path", ["/", "/config", "/set-up", "/static/app.css"]) +def test_the_interface_still_answers_everything_else(path): + assert serves(path) == tunnel_sync.REMOTE_ACCESS_SERVICE + + +# ── ordering, which is the whole thing ─────────────────────────── + +def test_every_path_rule_precedes_the_catch_all(): + """A rule without a path matches everything. Put one above the path rules + and they are silently disabled while still looking correct.""" + rules = tunnel_sync.build_ingress(NODE) + first_pathless = next(i for i, r in enumerate(rules) + if "path" not in r and "hostname" in r) + last_pathed = max(i for i, r in enumerate(rules) if "path" in r) + assert last_pathed < first_pathless + + +def test_the_final_rule_is_a_catch_all(): + """cloudflared requires the list to end with a rule that matches anything.""" + last = tunnel_sync.build_ingress(NODE)[-1] + assert "hostname" not in last and last["service"] == "http_status:404" + + +def test_another_node_is_not_served_by_this_tunnel(): + for rule in tunnel_sync.build_ingress(NODE): + assert rule.get("hostname") in (HOST, None) diff --git a/mender-auto-accept/tunnel_sync.py b/mender-auto-accept/tunnel_sync.py index 6066b6c..d737cfb 100644 --- a/mender-auto-accept/tunnel_sync.py +++ b/mender-auto-accept/tunnel_sync.py @@ -84,6 +84,24 @@ #: port without touching the running GUI. REMOTE_ACCESS_SERVICE = os.environ.get("REMOTE_ACCESS_SERVICE", "http://localhost:80") +#: The other two things a node serves that support needs to see: blah2's web +#: assets and blah2's API. They are separate ports on the node, and both are +#: reached through the one support hostname rather than hostnames of their own. +BLAH2_WEB_SERVICE = os.environ.get("BLAH2_WEB_SERVICE", "http://localhost:49152") +BLAH2_API_SERVICE = os.environ.get("BLAH2_API_SERVICE", "http://localhost:3000") + +#: blah2's API endpoints, named one by one rather than matched as a whole +#: `^/api` prefix. retina-gui owns /api/mode, /api/fleet/peers, +#: /api/spectrum/ready and others on this same hostname, so a blanket prefix +#: would divert those to blah2 and break the interface over the tunnel while +#: leaving it working perfectly on the LAN. Naming them fails loudly when blah2 +#: gains an endpoint, which is far better than silently stealing a GUI route. +BLAH2_API_PATH = r"^/(api/(timestamp|detection|map|adsb2dd|config)|capture|stash)" + +#: blah2's web assets. The three views support actually uses live under +#: /display/ and /controller/; /lib and /js are what those pages load. +BLAH2_WEB_PATH = r"^/(display|controller|lib|js)" + CF_API = "https://api.cloudflare.com/client/v4" CF_TOKEN = os.environ.get("CLOUDFLARE_API_TOKEN") CF_ACCOUNT = os.environ.get("CLOUDFLARE_ACCOUNT_ID") @@ -315,6 +333,38 @@ def destroy_access_app(node_id): _cf("DELETE", f"/accounts/{CF_ACCOUNT}/access/apps/{app['id']}") +def build_ingress(node_id): + """The tunnel's complete ingress config, as a list of rules. + + Pure, and separate from the call that writes it, so what a node will serve + can be read and asserted without touching Cloudflare. + + **Order is the whole thing.** cloudflared takes the first rule whose + hostname and path both match, so every rule carrying a path must come + before the one that does not. A catch-all placed above them matches + everything and silently disables the rest: the rules are still listed, still + look right in the dashboard, and never fire. That is not hypothetical, it is + how a hand-built tunnel in this account was configured, and the only symptom + was 404s that looked like the origin was down. + """ + hostname = f"{node_id}.{REMOTE_ACCESS_DOMAIN}" + return [ + {"hostname": hostname, "path": BLAH2_API_PATH, "service": BLAH2_API_SERVICE}, + {"hostname": hostname, "path": BLAH2_WEB_PATH, "service": BLAH2_WEB_SERVICE}, + {"hostname": hostname, "service": REMOTE_ACCESS_SERVICE}, + {"service": "http_status:404"}, + ] + + +def read_ingress(tunnel_id): + """What Cloudflare currently holds for this tunnel, or [] if unreadable.""" + try: + cfg = _cf("GET", f"/accounts/{CF_ACCOUNT}/cfd_tunnel/{tunnel_id}/configurations") + except requests.RequestException: + return [] + return ((cfg or {}).get("config") or {}).get("ingress") or [] + + def ensure_tunnel(node_id): """Find or create this node's tunnel, and return (tunnel_id, token, aud). @@ -331,12 +381,8 @@ def ensure_tunnel(node_id): json={"name": node_id, "config_src": "cloudflare"}) tunnel_id = created["id"] - hostname = f"{node_id}.{REMOTE_ACCESS_DOMAIN}" _cf("PUT", f"/accounts/{CF_ACCOUNT}/cfd_tunnel/{tunnel_id}/configurations", - json={"config": {"ingress": [ - {"hostname": hostname, "service": REMOTE_ACCESS_SERVICE}, - {"service": "http_status:404"}, - ]}}) + json={"config": {"ingress": build_ingress(node_id)}}) # Before the DNS record, always. The moment that record resolves the # hostname serves this node's interface, so creating the policy afterwards @@ -525,6 +571,19 @@ def reconcile(wanted, state, tunnels, dns_records, access_apps): elif not dns.get("proxied"): repairs.append((node_id, "DNS record is not proxied")) + # Costs one call per *provisioned* node, so it scales with opt-ins + # rather than fleet size, which is why it is affordable where a + # per-device check would not be. + # + # Worth the call because ingress fails silently. A rule in the wrong + # order is still listed, still reads correctly to a human, and simply + # never fires; the only symptom is 404s that look like a dead origin. + # Nothing else in this script would ever notice, and a hand-edit in the + # dashboard is exactly how it happens. + if read_ingress(tunnel["id"]) != build_ingress(node_id): + repairs.append((node_id, "ingress does not match: the paths support " + "needs may be unrouted or shadowed")) + if not tunnel.get("connections"): # Not a repair. The node may simply be off, and re-provisioning # would not bring it back; only the node connecting does.