One ensure primitive per rule kind in the natmap daemon

[FAZuH]

Parent

#24 — Spec: One typed seam through which all NAT state flows

What to build

The natmap daemon's apply-a-mapping logic is implemented once per rule kind. A shared ensure_docker_mapping primitive carries the allocate→install→persist→rollback step for port mappings, and an ensure_static_rule primitive does the same for static NAT rules (dnat/hairpin). The reload path, on_container_start, and the reconcile routines all call these primitives instead of re-implementing the step independently. The orchestrators stay — they differ in what set of rules they process — but the per-rule apply step exists once. Behavior is unchanged from the operator's perspective.

Acceptance criteria

• ensure_docker_mapping and ensure_static_rule primitives exist and are the single place the allocate→install→rollback step is implemented for their rule kind
• reload, on_container_start, and the reconcile routines all route through the primitives; duplicated per-rule apply logic is removed
• The primitives are unit-tested with a fake iptables/ports adapter, covering the stale-deallocate, untracked-container, and container-IP-reverify scenarios from recent bug fixes
• Full workspace builds and all existing unit tests pass

Blocked by

None — can start immediately.

[FAZuH]

Whole-repo architecture audit (2026-09-30) added a finding for this spec.

Temporal Coupling, install-then-rollback written six times. The fixed sequence reserve port → install rule → record in state → persist, with a manual compensating rollback on failure, is implemented once in crates/natmap/src/daemon.rs:675 (ensure_docker_mapping) and daemon.rs:696 (ensure_static_rule), then re-implemented inline four more times in crates/natmap/src/api.rs: add_dnat at :110, add_hairpin at :238, add_mapping at :545, add_policy_route at :315.

An illegal order compiles. Nothing stops a future edit from installing the rule before reserving the port, or persisting state before recording the rule, and each of the four inline copies is a separate place for that to happen. The compensation is also easy to get wrong: each site decides independently which steps to undo.

crates/natmap/src/api.rs:110-127, verbatim:

    bind_ports(state.ports.clone(), &config.ext_ip, &config.ports, config.proto).await?;
    if let Err(e) = state.iptables.install_dnat(&config) {
        unbind_ports(state.ports, &config.ext_ip, &config.ports).await;
        return Err((StatusCode::INTERNAL_SERVER_ERROR, Json(ErrorResponse { error: e.to_string() })));
    }
    state.daemon_state.write().await.dnats.push(config.clone());
    state.persist().await;

Idiom: RAII. A PortReservation holding the Vec<SocketAddr> releases on drop, so rollback becomes a property of the type rather than a branch at every call site. install + push + persist then collapse into one entry point per rule kind, and the if let Err ladder disappears from all four. 102 inline lines collapse into the 62 already written in daemon.rs:675.

Note for sequencing: #43 extracts the ErrorResponse helper that the four inline copies all use, and #42 removes the *Request twins they take. Landing this issue after both means the six copies are rewritten once, not twice.

[FAZuH]

A testability finding from #44 that belongs here rather than in a test ticket.

The gap

PolicyRouteManager is still a real type inside natmap's AppState. The iptables and Docker seams in this area were closed in #44 — FakeIptables now serves every unit test, and no unit test constructs a IptablesManager any more. Policy routing was left behind.

Proven, not asserted. A shim directory holding logging stubs for iptables, ip6tables, ip, docker and consul was placed first on PATH and the three lib suites run under it:

suite	result	host binaries reached
lab-ops_natmap --lib	ok. 141 passed	ip × 9
lab-ops_auto-discover --lib	ok. 80 passed	none
lab-ops_lab-lib --lib	ok. 14 passed	none

All nine are policy-route commands from client.rs:

ip rule add from 10.0.0.1 table 100
ip route add default via 192.168.1.1 table 100
ip rule show
ip route show table main
ip route show table 100
ip rule del from 10.0.0.1 table 100   (×2)
ip route del default via 192.168.1.1 table 100   (×2)

Why the test is green anyway

client.rs swallows the error with Err(_) => {}, and the commands fail because the suite does not run as root. The test passes because a permission error is being discarded, which is the same class of defect #44 removed from the iptables and Docker paths: a test that reports green without exercising the thing it names.

What it needs

A PolicyRoute trait in lab-lib, implemented by the real manager and by a fake, exactly as NatmapOps now is for forwarding. The auto-discover side has had NatmapOps for a while; natmap has no equivalent, and handle_docker_event(&self, event, docker: &Docker) taking a concrete bollard type is the same gap in a different place.

This is production code, which is why it is filed against a design ticket and not fixed inside a test ticket.