Structured server registry + generated network-flows.md#430
Conversation
docs/network-flows.md was the only place host OS/role and every
outbound + cross-cutting (N-to-M) flow lived, all as unparsed prose.
Move that into structured inventory so it can be rendered and queried:
- host_meta {os, role, summary, notes} on every host_vars/<h>.yml,
including dom0 (underlay-only hypervisor, still the fleet's most
important server) — this is the server registry in structured form.
- network_flows_outbound per host: [{to, proto, port, purpose}], with
`to` referencing a peer name or a well-known external token. Egress
policy is accept, so these have no firewall_extra_rules form today.
- ansible/inventory/network_flows.yml: repo-level cross_cutting_flows
(the genuinely N-to-M flows like all->rtr:53, mon->all:9100) and a
network_externals token map (github, routers, model-providers, ...).
Peers stay the address book; the cr1_nl1/cr1-nl1 duplicate keys are
left as-is (the knowledge extractor resolves both to the hyphen host).
firewall_extra_rules are untouched — nftables render is byte-identical.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
docs/network-flows.md is now a build artifact, not hand-maintained prose — closing the gap where its "every rule traces back to a row here" claim was unenforced and its data could silently drift. - scripts/render-network-flows.py: deterministic renderer that composes the Hosts table (peers + host_meta), Subnets, per-host inbound (firewall_extra_rules), per-host outbound + cross-cutting flows, and the external-endpoint table from the structured inventory. Resolves peer references to host names; --check exits 1 on drift. - docs/network-flows.md: regenerated from the renderer (restructured; same facts). No behaviour change — it feeds humans + the OKF ingest. - tests/iac/test_network_flows_render.py: unittest.TestCase (so `unittest discover` in iac-static.sh actually runs it — not a silent pytest-only no-op) asserting the committed doc matches a fresh render and that the render is deterministic. - render-check.yml: `render-network-flows.py --check` step so a doc-only hand-edit is caught even when no ansible/ path changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
The flow doc is now generated from structured inventory, so the operator workflow inverts: edit host_vars / network_flows.yml (the new source of truth), then re-render — never hand-edit docs/network-flows.md (the freshness gate rejects it). Update the CLAUDE.md firewall and monitoring runbooks + the repo-layout note to match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2f9e95f73
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| rules = host_vars[host].get("firewall_extra_rules") or [] | ||
| lines.append("**Inbound**") | ||
| lines.append("") | ||
| if rules: | ||
| lines.extend(render_inbound_table(resolver, rules)) |
There was a problem hiding this comment.
For FreeBSD router hosts, active ingress rules are not stored in firewall_extra_rules: I checked ansible/roles/firewall/templates/pf.conf.j2, which renders BGP from pf_bgp_peers, WireGuard from pf_wg_ports, pass quick on wg all, ICMP, and AS215932 transit independently. By rendering inbound rows only from firewall_extra_rules, the new canonical/generated flow data omits those port/proto/source rows (for example cr1-* TCP/179 and UDP 1337-1342), so downstream consumers querying the generated inventory will miss live firewall paths.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0507ff6 (+ knowledge#38 4f33567). The renderer now synthesizes router inbound from pf_bgp_peers (tcp/179) and pf_wg_ports (udp) via synthesize_router_inbound, so cr1-nl1/de1/ch1 regain their per-host inbound tables in the generated doc, and knowledge#38's extractor does the same synthesis so BGP/WG compile into allows_port/allows_inbound claims. Verified: all four routers now answer firewall --port 179, and cr1-nl1's wg ports 1337/1338/1340/1341 are queryable.
| - { from: mon, to: all, proto: tcp, port: 22, purpose: "SSH for Icinga2 by_ssh checks (monitoring user; privileged plugins via sudo/doas)" } | ||
|
|
||
| # --- Centralized logging --- | ||
| - { from: all-infra, to: log, proto: tcp, port: 6000, purpose: "Vector agent to aggregator (Loki ingest)" } |
There was a problem hiding this comment.
This all-infra -> log:6000 row expands through the new all-infra token to include mail and log itself, but mail is explicitly modeled just below as syslog to log:6514, and host_vars/log.yml has no mail or self-ingest allow on TCP/6000. Consumers expanding the token will therefore report non-existent mail -> log:6000 and log -> log:6000 Vector flows, contradicting the actual per-host firewall table.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0507ff6. Narrowed the all-infra token note to name only the Vector→log:6000 shippers and explicitly exclude mail (ships syslog to log:6514) and log itself. Note that knowledge#38 never expanded this external token into per-host claims (only the all special token expands), so no false mail -> log:6000 claim was emitted — but the note was misleading, now corrected.
| - { from: ops-prefix, to: all, proto: tcp, port: 22, purpose: "SSH operator access" } | ||
| - { from: vpn-clients, to: all, proto: tcp, port: 22, purpose: "SSH via VPN" } | ||
| - { from: ci, to: all, proto: tcp, port: 22, purpose: "SSH for CI apply runs" } | ||
| - { from: noc, to: all, proto: tcp, port: 22, purpose: "SSH for hyrule-mcp" } |
There was a problem hiding this comment.
This row makes the generated cross-cutting table claim noc -> all on SSH, but ansible/inventory/host_vars/mail.yml overrides ssh_allow_sources_v6 to only ops-prefix, vpn-clients, ci, and mon, and the new mail host_meta.notes says NOC is omitted. As a result the structured flow registry now reports a noc -> mail:22 path that the mail firewall deliberately does not allow; NOC only has IMAPS to mail on 993.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0507ff6 (+ knowledge#38 4f33567). noc -> all:22 except [mail] — the renderer displays/validates the exception and knowledge#38's _participants subtracts it, so mail no longer gets a false allows_from noc:22. Verified end-to-end: mail:22 sources are now ops-prefix/vpn/ci/mon only (noc excluded).
| - { from: public, to: mail, proto: tcp, port: 465, purpose: "SMTPS authenticated submission" } | ||
| - { from: public, to: mail, proto: tcp, port: 587, purpose: "submission STARTTLS" } | ||
| - { from: public, to: mail, proto: tcp, port: 80, purpose: "ACME HTTP-01 for mail.as215932.net" } | ||
| - { from: public, to: mail, proto: tcp, port: 993, purpose: "Dovecot IMAPS mailbox access (public IPv4)" } |
There was a problem hiding this comment.
This structured row records public IMAPS as generic tcp/993, but the actual mail firewall rule is family: ip for src: any on 993, while IPv6 IMAPS is limited to ops/VPN/mon/noc. Downstream consumers that rely on the structured fields rather than parsing the prose purpose will treat public IPv6 mail:993 as allowed even though only public IPv4 is.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0507ff6 (+ knowledge#38 4f33567). Removed the lossy duplicate cross-cutting rows — mail's IMAPS/SMTP ingress lives in its firewall_extra_rules (family-aware: public 993 is family: ip = IPv4-only, IPv6 scoped to ops/VPN/mon/noc) and renders in mail's per-host inbound table. knowledge#38 now threads family into the flow-claim metadata, so a structured consumer sees the v4/v6 split (verified: mail:993 → {any: ip, mon/noc/ops/vpn: ip6}).
| - { from: all, to: rtr, proto: "tcp+udp", port: 53, purpose: "DNS recursion via Unbound (incl. DNS64 synthesis)" } | ||
|
|
||
| # --- Monitoring scrapes --- | ||
| - { from: mon, to: all, proto: tcp, port: 9100, purpose: "node_exporter scrape" } |
There was a problem hiding this comment.
Now that the generated host registry includes off-net extmon, this mon -> all:9100 row overstates the scrape set: I checked configs/mon/prometheus.yml, and extmon is not a target there, while host_vars/extmon.yml does not open 9100 to mon. Expanding this token will therefore produce a non-existent mon -> extmon:9100 flow in the queryable registry.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
| - { to: github, proto: tcp, port: 443, purpose: "runner queue, action images" } | ||
| - { to: openrouter, proto: tcp, port: 443, purpose: "PR-Agent LLM" } | ||
| - { to: package-registries, proto: tcp, port: 443, purpose: "PyPI/npm/ghcr/Docker Hub toolchain and semgrep image" } | ||
| - { to: rtr, proto: "tcp+udp", port: 53, purpose: "Unbound + DNS64 via 2a0c:b641:b51::1" } |
There was a problem hiding this comment.
This flow records ci-pr DNS as going to 2a0c:b641:b51::1, but the same host_vars file states that rtr's Unbound binds only 2a0c:b641:b50:2::1 and that there is no listener on the customer gateway 2a0c:b641:b51::1; github_runner_docker_dns is also set to 2a0c:b641:b50:2::1. The structured registry will therefore point operators and consumers at an address that should not answer DNS for ci-pr.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0507ff6. The purpose now points at rtr's actual Unbound listener 2a0c:b641:b50:2::1 (matching github_runner_docker_dns and the host's own note that the b51::1 customer gateway has no listener).
|
|
||
| cross_cutting_flows: | ||
| # --- DNS recursion --- | ||
| - { from: all, to: rtr, proto: "tcp+udp", port: 53, purpose: "DNS recursion via Unbound (incl. DNS64 synthesis)" } |
There was a problem hiding this comment.
This structured all -> rtr:53 row expands beyond what the router actually permits: host_vars/rtr.yml only opens DNS recursion from infra_subnet, customer_subnet, vpn_clients_subnet, loop_docker_subnet, and rtr's underlay source. Consumers expanding all will report DNS-recursion paths from off-net hosts such as extmon/ns2 and other hosts that are not in those source ranges.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0507ff6 (+ knowledge#38 4f33567). all -> rtr:53 except [extmon, ns2, dom0] — scoped to the overlay subnets rtr actually permits DNS recursion from (infra/customer/vpn/loop-docker + rtr underlay); the off-net (extmon, ns2) and underlay-only (dom0) hosts are excluded. Verified: rtr:53 inbound no longer includes extmon/ns2/dom0.
…router inbound Codex flagged the new structured flow data over-claiming vs the actual firewall rules (chatgpt-codex-connector on #430). Fixes: - `except: [host, ...]` on cross-cutting `all` flows so broad tokens match the firewall scope: `all -> rtr:53 except [extmon, ns2, dom0]` (rtr only recurses for the overlay subnets), `mon -> all:9100 except [extmon, dom0]` (extmon self-scrapes; dom0 has no node_exporter), `noc -> all:22 except [mail]` (mail's ssh_allow omits noc). The renderer displays and validates the exceptions; the freshness test covers them. - Router BGP/WireGuard inbound is now rendered: `synthesize_router_inbound` turns `pf_bgp_peers` (tcp/179) and `pf_wg_ports` (udp) into inbound rows, restoring the per-router inbound table the generated doc had dropped (routers keep those in the pf template, not firewall_extra_rules). - Removed the mail SMTP/IMAPS/ManageSieve and irc IRCS rows from cross_cutting_flows: they duplicated each host's firewall_extra_rules (which are family-aware and correctly scoped — public IMAPS is IPv4-only) and belong in the per-host inbound table, not here. - ci-pr DNS purpose pointed at the wrong resolver (`b51::1`); rtr's Unbound binds only the infra `b50:2::1`. Fixed to match the host's own config. - Narrowed the `all-infra` token note (it wrongly implied mail + log ship to log:6000). Dropped the redundant `dom0` external token — dom0 is a real registry host now, and the external shadowed it in reference resolution. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0507ff6582
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - { from: ops-prefix, to: all, proto: tcp, port: 22, purpose: "SSH operator access" } | ||
| - { from: vpn-clients, to: all, proto: tcp, port: 22, purpose: "SSH via VPN" } | ||
| - { from: ci, to: all, proto: tcp, port: 22, purpose: "SSH for CI apply runs" } | ||
| - { from: noc, to: all, proto: tcp, port: 22, except: [mail], purpose: "SSH for hyrule-mcp (mail's ssh_allow omits noc)" } | ||
| - { from: mon, to: all, proto: tcp, port: 22, purpose: "SSH for Icinga2 by_ssh checks (monitoring user; privileged plugins via sudo/doas)" } |
There was a problem hiding this comment.
Now that dom0 has host_meta, any consumer that expands all will include it in these SSH rows. However ansible/playbooks/firewall.yml targets all:!dom0, and host_vars/dom0.yml says dom0 is underlay-only and not managed by the firewall roles, so ssh_allow_sources_* is not applied there. This generates non-existent CI/NOC/mon/VPN-to-dom0 SSH paths; add a dom0 exception (while preserving mail's noc exception) or keep dom0 out of the all expansion.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4b2c2f0 (+ knowledge#38 3191905). dom0 is firewall-unmanaged (playbook targets all:!dom0), so a new global all_excludes: [dom0] keeps it out of every all expansion — no more false ci/noc/mon/vpn SSH-to-dom0. Verified: dom0:22 sources are now empty. The extractor subtracts all_excludes only from the all-expanded side, so explicit dom0 flows still resolve.
| cross_cutting_flows: | ||
| # --- DNS recursion (rtr Unbound; scoped to the overlay subnets, so off-net | ||
| # extmon/ns2 and underlay-only dom0 are excluded) --- | ||
| - { from: all, to: rtr, proto: "tcp+udp", port: 53, except: [extmon, ns2, dom0], purpose: "DNS recursion via Unbound (incl. DNS64 synthesis)" } |
There was a problem hiding this comment.
After the existing off-net exclusions, this all -> rtr:53 row still expands to the FreeBSD cr1 routers, but rtr's actual DNS-recursion allow rule only permits infra_subnet, customer_subnet, vpn_clients_subnet, loop_docker_subnet, and peers.rtr.underlay in host_vars/rtr.yml; the cr1 loopbacks live in router_loopback_subnet and are not in that source set. Fresh evidence for this remaining issue is the current except list omitting cr1-nl1, cr1-de1, and cr1-ch1, so generated consumers will still report router-to-rtr DNS paths the firewall does not allow.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4b2c2f0 (+ knowledge#38 3191905). all -> rtr:53 except [extmon, ns2, cr1-nl1, cr1-de1, cr1-ch1] — the cr1 loopbacks are in router_loopback_subnet, which isn't one of rtr's recursion source subnets. Verified: rtr:53 inbound no longer includes any cr1 router.
| notes: | ||
| - "SSH is applied to every host from ssh_allow_sources_* in group_vars/all.yml (plus per-host overrides), not from firewall_extra_rules, so it appears only in the cross-cutting flows table, not the per-host inbound tables. mail overrides ssh_allow_sources_v6 to omit noc, hence `noc -> all:22 except [mail]`." | ||
| - "Public single-host service ingress (mail SMTP tcp/25,465,587 + ACME tcp/80 + Dovecot IMAPS/ManageSieve tcp/993,4190; irc Soju IRCS tcp/6697) is defined in each host's firewall_extra_rules with the correct address family and source scope, and appears in that host's per-host inbound table — not in this cross-cutting file. Public IMAPS is IPv4-only (family: ip); IPv6 IMAPS is scoped to ops/VPN/mon/noc." | ||
| - "FreeBSD core routers (cr1-nl1, cr1-de1, cr1-ch1) forward syslog to log:6514 via native syslogd; this is rendered as log's inbound rule and appears in the log per-host inbound table (not repeated in cross-cutting)." |
There was a problem hiding this comment.
This note says the FreeBSD syslog path for cr1-ch1 is rendered in log's inbound table, but ansible/inventory/host_vars/log.yml only has tcp/6514 firewall entries for cr1_nl1 and cr1_de1; the generated ### log table correspondingly omits cr1-ch1. Because cr1-ch1 has logs_use_freebsd_syslogd: true, the structured registry now documents a logging path that is neither allowed nor rendered, so add the missing log allow/rule or remove cr1-ch1 from this claim.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4b2c2f0 (+ knowledge#38 3191905). The note was over-claiming — log.yml only allows cr1-nl1/cr1-de1 syslog, not cr1-ch1. Corrected the note to name only nl1/de1, and filed the real firewall gap (add cr1-ch1's log 6000/6514 allow) as issue #431 rather than silently claim a path that isn't permitted.
| - { to: public, proto: tcp, port: 443, purpose: "blackbox_exporter HTTPS probes of public services" } | ||
| - { to: public, proto: icmp6, purpose: "blackbox_exporter ICMP reachability probes" } |
There was a problem hiding this comment.
These rows only describe public HTTPS and public ICMP, but configs/mon/prometheus.yml runs blackbox-icmp against router, infra, and WireGuard internal IPv6 targets and blackbox-dns against dns on port 53. Consumers of the generated registry will miss the live mon -> dns:53 and internal mon -> <fleet>:icmp6 monitoring flows, and will misclassify the ICMP probes as public-only.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4b2c2f0 (+ knowledge#38 3191905). Added mon -> dns:53 (blackbox-dns job) as a structured flow. The internal blackbox-icmp reachability sweep (routers + core infra + WG link endpoints) is documented in a network_flows.yml note rather than expanded per-host, since it hits an arbitrary curated subset; the public ICMP row's purpose now says 'public'.
| network_flows_outbound: | ||
| - { to: noc, proto: tcp, port: 8000, purpose: "best-effort external Alertmanager webhook enrichment" } | ||
| - { to: hyrule-cloud, proto: tcp, port: 443, purpose: "BGP intelligence ingest (cloud.hyrule.host/v1/internal/bgp)" } | ||
| - { to: public, proto: tcp, port: 443, purpose: "probe AS215932 public services, Discord webhook, Cloudflare Radar, OVH API" } |
There was a problem hiding this comment.
This collapses extmon's public probing to tcp/443, but the extmon role defaults also configure ICMP v4/v6 probes, TCP checks for mail ports 25/465/587/993, and DNS SOA checks to ns1/ns2 on port 53. Since extmon was added to the structured registry, downstream consumers will now miss most of the external monitoring traffic unless those non-HTTPS probe flows are represented too.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4b2c2f0 (+ knowledge#38 3191905). Added extmon's off-net probe flows: extmon -> dns:53 and extmon -> ns2:53 (blackbox DNS SOA), extmon -> mail:[25,465,587,993] (SMTP/submission/IMAPS), and extmon -> public icmp6 (ICMP reachability).
| - { from: public, to: rtr, proto: "tcp+udp", port: 53, purpose: "DNAT to dns" } | ||
| - { from: public, to: rtr, proto: tcp, port: 80, purpose: "DNAT to proxy" } | ||
| - { from: public, to: rtr, proto: tcp, port: 443, purpose: "DNAT to proxy" } | ||
| - { from: public, to: rtr, proto: udp, port: 51820, purpose: "DNAT to vpn" } |
There was a problem hiding this comment.
These rows are described in the comments as public v4 DNAT, but the structured fields do not carry that family restriction. Consumers that do not parse YAML comments will report public IPv6 ingress to rtr on 53/80/443/51820, even though direct IPv6 service ingress is handled by the destination hosts' own firewall rules and rtr's input rules do not open those public IPv6 paths.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4b2c2f0 (+ knowledge#38 3191905). Added a family: field to cross-cutting flows and marked the four public->rtr DNAT rows family: ipv4. The renderer shows 'tcp+udp (v4)' etc., and knowledge#38 threads family into the flow-claim metadata — verified rtr:80 DNAT carries family=ipv4.
|
|
||
| # --- Monitoring scrapes (extmon self-scrapes; dom0 has no node_exporter yet, | ||
| # issue #24) --- | ||
| - { from: mon, to: all, proto: tcp, port: 9100, except: [extmon, dom0], purpose: "node_exporter scrape" } |
There was a problem hiding this comment.
This exception removes dom0 from the generated node_exporter flow, but configs/mon/prometheus.yml still has the node-hypervisor scrape target 10.0.0.1:9100 and the dom0 Icinga host keys on that Prometheus instance. As a result, consumers of the structured registry will miss the configured mon -> dom0:9100 monitoring path; either keep dom0 in the flow or remove the live scrape target at the same time.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4b2c2f0 (+ knowledge#38 3191905). dom0 IS scraped, but over mgmt v4 (10.0.0.1:9100, the node-hypervisor job) — not the overlay. So dom0 is excepted from the overlay mon -> all:9100 and the mgmt scrape is modelled as an explicit mon -> dom0:9100 family:ipv4 row. Verified: dom0:9100 sources = [mon].
| host_meta: | ||
| os: "XCP-NG 8.3" | ||
| role: "XCP-NG hypervisor (OVH RISE-S dom0)" | ||
| summary: "Underlay-only Xen host for the whole AS215932 fleet; hypervisor every VM runs on." | ||
| notes: >- |
There was a problem hiding this comment.
The generated host table now renders dom0's IPv4 as — because host_ipv4() only reads host_meta.ipv4 or the peer map, but hosts.yml still defines dom0's reachable address as 193.70.32.138 and this same note says it is reached via the public IPv4. This drops the only public/underlay address for the hypervisor from the structured server registry.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4b2c2f0 (+ knowledge#38 3191905). Added ipv4: '193.70.32.138 (public), 10.0.0.1 (mgmt)' to dom0's host_meta, so the host table renders its reachable addresses again.
|
|
||
| # --- VPN client transit --- | ||
| - { from: vpn-clients, to: rtr, proto: "tcp+udp", port: 53, purpose: "DNS64 to rtr; vpn forwards wg0 to enX0" } | ||
| - { from: vpn-clients, to: public, proto: "tcp+udp", purpose: "routed IPv6 / NAT64 egress forwarded by vpn via rtr" } |
There was a problem hiding this comment.
This records VPN client internet transit as only TCP/UDP, but host_vars/vpn.yml forwards ip6 saddr {{ vpn_clients_subnet }} from wg0 to enX0 without an L4 protocol restriction. ICMPv6 and other IPv6 traffic forwarded by the VPN will be absent from generated flow queries unless this row is modeled as all protocols instead of tcp+udp.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4b2c2f0 (+ knowledge#38 3191905). The VPN client transit row is now proto: any (vpn forwards ip6 saddr from wg0 to enX0 with no L4 restriction, so ICMPv6 and everything else transits).
| os: "Debian 13" | ||
| role: "Xen Orchestra" | ||
| summary: "Xen Orchestra appliance: the infra NIC serves the UI (via Caddy on proxy) and the mgmt NIC reaches dom0 XAPI." | ||
| network_flows_outbound: [] |
There was a problem hiding this comment.
The new summary says XOA's mgmt NIC reaches dom0 XAPI, and the deployment docs direct XO to connect to dom0 at 10.0.0.1, but network_flows_outbound is empty and there is no cross-cutting row for that management-plane path. Generated consumers will therefore miss the critical xoa -> dom0 XAPI traffic even though this host metadata now describes it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4b2c2f0 (+ knowledge#38 3191905). Added xoa -> dom0 XAPI to xoa's network_flows_outbound: {to: dom0, proto: tcp, port: [80, 443], family: ipv4} over the mgmt bridge (10.0.0.1). Verified as a resolved host->host flow.
…ane + probes Second Codex pass on the fix commit (chatgpt-codex-connector on #430). Fixes: - dom0 is firewall-unmanaged (playbook targets all:!dom0), so a new global `all_excludes: [dom0]` keeps it out of every `all` expansion (no false ci/noc/mon/vpn SSH to dom0). Its real flows are modelled explicitly: mon->dom0:9100 (mgmt v4 node-hypervisor scrape), dom0->log:6000, and a new xoa->dom0 XAPI (80/443, mgmt v4). dom0's public/mgmt IPv4 is back in the host table. - DNS recursion `all -> rtr:53` now also excepts the cr1 routers — their loopbacks are in router_loopback_subnet, not one of rtr's recursion source subnets. - Address-family precision via a new `family:` field on cross-cutting/outbound flows: rtr's public DNAT ingress (53/80/443/51820) is marked IPv4-only, so consumers no longer read it as public IPv6 ingress. - Protocol/port precision: NAT64 egress and VPN client transit are `any` proto (Jool translates TCP/UDP/ICMP; vpn forwards ip6 with no L4 filter); the WireGuard mesh row carries port 1337-1342. - Monitoring completeness: added mon->dns:53 (blackbox-dns) and extmon's DNS/mail/ICMP probes; noted mon's internal ICMP reachability sweep. - cr1-ch1 syslog note was over-claiming (log.yml has no cr1-ch1 allow) — corrected, and the real firewall gap filed as issue #431. Renderer gains `family` rendering (+ ipv4/ipv6 vocab), `all_excludes` validation + display, and host-name-aware endpoint validation. Tests cover the except scopes and router BGP/WG inbound. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b2c2f0579
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| package-registries: { name: "PyPI / npm / ghcr.io / Docker Hub", note: "toolchain and container image pulls" } | ||
| ops-prefix: { name: "ops workstation prefix (KPN home)", note: "2a02:a442:1016::/48, 77.166.211.126/32; SSH + AXFR allow" } | ||
| vpn-clients: { name: "VPN clients", note: "2a0c:b641:b50:3::/64, routed via the vpn VM" } | ||
| all-infra: { name: "log-shipping infra hosts", note: "infra_vms that run a Vector agent to log:6000 — dns, api, web, proxy, mon, vpn, xoa, irc, noc, vault, ci, netproxy, loop. Excludes mail (ships syslog to log:6514, not 6000) and log itself (the aggregator)." } |
There was a problem hiding this comment.
The current all-infra token includes vault, so the generated cross-cutting flow expands to vault -> log:6000, but the actual log aggregator allowlist in ansible/inventory/host_vars/log.yml has Vector TCP/6000 entries for rtr/dns/api/web/proxy/mon/vpn/xoa/irc/noc/ci/netproxy/loop/ns2/dom0 and no peers.vault.ipv6 rule. Consumers of the new registry will therefore report a vault-to-log path that the firewall does not allow; either add the missing log allow or leave vault out of this token until that path exists.
Useful? React with 👍 / 👎.
| synthesized.append({"proto": "udp", "dport": list(wg_ports), "src": "any", | ||
| "comment": "WireGuard underlay tunnels (pf_wg_ports)"}) |
There was a problem hiding this comment.
For FreeBSD routers with pf_wg_ports, this synthetic row loses the IPv4 half of the listener: ansible/roles/firewall/templates/pf.conf.j2 renders both inet and inet6 UDP rules for these ports, while the generated row omits family: "both" and therefore renders unlike other dual-stack rules. Consumers querying the generated registry by address family will miss public IPv4 WireGuard ingress on the cr1 routers unless the synthesized rule carries the same dual-stack scope as pf.
Useful? React with 👍 / 👎.
| if peer.get("loopback"): | ||
| return f"`{peer['loopback']}` (loopback)" |
There was a problem hiding this comment.
When a router has a loopback, this returns only that address, so the generated host registry now lists cr1-ch1 without its configured SBIX/4IXP peering LAN addresses. I checked configs/cr1-ch1/rc.conf, which still configures 2001:7f8:d0::3:4b7c:1 and 2001:7f8:d9:1::3:4b7c:2; those addresses were present in the old host table but are no longer represented in the generated registry, so downstream inventory/users lose the local IXP addresses needed for BGP/peering operations.
Useful? React with 👍 / 👎.
1 participant
Context
docs/network-flows.mdwas the canonical description of every traffic flow inAS215932 but was pure prose: nothing parsed it, its "every rule traces back
to a row here" claim was unenforced, and it was the only place host OS/role
and all outbound + cross-cutting flows lived (those have no
firewall_extra_rulesform because egress policy isaccept).This PR turns that data into structured inventory and makes the doc a generated,
drift-proof build artifact. It is the network-operations half of a two-repo
change — the knowledge repo consumes this data to make firewall rules queryable
by port/proto/source across servers (AS215932/knowledge#38).
What changed
Structured registry + flow data (
inventory:commit)host_meta {os, role, summary, notes}on everyhost_vars/<h>.yml,including dom0 (underlay-only hypervisor — still the fleet's most important
server) — the server registry in structured form.
network_flows_outboundper host[{to, proto, port, purpose}].ansible/inventory/network_flows.yml: repo-levelcross_cutting_flows(N-to-M flows like
all -> rtr:53,mon -> all:9100) + anetwork_externalstoken map (github, routers, model-providers, ...).Generated doc + freshness gate (
docs:commit)scripts/render-network-flows.py: deterministic renderer;--checkexits 1on drift.
docs/network-flows.md: regenerated (restructured, same facts).tests/iac/test_network_flows_render.py: aunittest.TestCase(sounittest discoveriniac-static.shactually runs it — not a silentpytest-only no-op) asserting the committed doc matches a fresh render.
render-check.yml:--checkstep so a doc-only hand-edit is caught evenwith no
ansible/change.Verification
ansible-playbook playbooks/firewall.yml --tags validate→ansible/generated/byte-identical (this is a data-move;
firewall_extra_rulesuntouched).python scripts/render-network-flows.py --checkclean; renderer idempotent.python3 -m unittest discover -s tests/iac→ 94 tests + 190 subtests pass.Related
🤖 Generated with Claude Code