Repository navigation
T9 — Publish an own-address box on host loopback from a fresh install, with the installer checking the switch binary - #1723
Conversation
An un-enrolled host has no control plane handing it an address block (NET-102), so the plan it self-allocates from is defined beside the subnet primitives that already live here: the CGNAT /16 box leases come from and the reserved local /24 published addresses come from, paired switch by switch. Switch i holds loopback slice i, so two gvproxies on one host never hold the same slice -- the wrapped 'octet % slices' derivation could silently hand two of them the same one, where the plan instead refuses to serve a switch past its capacity and leaves the host-global arbitration to NET-010. installed_gvproxy_bin now also probes the system package path (/usr/bin/gvproxy-min, where nfpm and the AUR install it) between the user-local and operator-provisioned paths.
An un-enrolled host self-allocates box addresses from the switch crate's default plan (NET-102), so the allocator and the default SwitchClient take their subnet from that plan instead of from a constant of minimald's own -- one definition, already shared with minvmd, of the block a fresh daemon boxes out of. The daemon test also pins the plan's reserved local range to the block the zone's published names answer at, so the published side and the answerer side cannot drift.
NET-040's diagnostics want the daemon log tail (and the min bug bundle carrying it) to show each forwarder expose call and its result. One info line per exposed mapping now records the host address it is reachable at, the port, and the session, and a failed apply logs the error the same way. This lives beside the attach path that owns the session name; the apply itself is policy's.
An own-IP session is served by a switch binary (gvproxy-min) the daemon spawns from exactly the paths the installer writes to (switch::installed_gvproxy_bin), so the install now checks the binary this run shipped is present and executable — naming the path it verified — before the gvproxy rename cleanup can delete anything. install_test.sh wraps each unit section behind a case on its first argument (all cases when none is given, each one self-contained: homes are seeded per case and the pristine manifest baseline is saved once up front) and adds the installer_switch_binary_executable case covering the verified row, a shipped binary that lost its +x, a channel that ships no switch, and a pre-rename channel still shipping bin/gvproxy. just test-installer takes the optional case and passes it through.
NET-040/NET-041 end to end, in the shape a user gets it: a stub-bucket fresh install (its own lane binaries plus the pinned gvproxy), the installer's switch-binary verification row checked in the install log, gvproxy-min executable at ~/.local/bin, and an own-IP activate with --ingress <host>:8080 whose in-box socat responder answers at 127.0.0.1:<host> with the box's own server's response — plus the daemon log's one record per exposed mapping (host, port, session) and the system-package half (/usr/bin/gvproxy-min) on lanes that can stage it. The box needs /dev/net/tun for its tap; a host that is itself a sandbox has none and cannot mknod one, so the case gates on the device and skips there — the native CI lane, which already builds a tap in its root-integration harness, runs it for real. A host already listening on 8080 (the agent-runtime boxes do) publishes on the fallback 18082; the box always serves its internal 8080, so the mapping exercised is <host-port>:8080 either way.
…-own-address-box-on-host-loopback-from # Conflicts: # scripts/install_test.sh # scripts/session-e2e.sh
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
Babysitter review against T9's plan block (covers NET-040, NET-041, NET-102; five test lines, all exit 0 in the coder sandbox, the e2e case self-skipping there for lack of
One note, not a finding against this PR: the tree's reserved local range is No findings. Merge after the lanes, minimal-review and CodeRabbit conclude. |
|
Reviewed: 1 finding, none blocking at Address the comments on the diff. Clobber: none: |
|
Native lane failure on a343524 ( The hosted native runner cannot build a rootless tap, which is why the existing |
On a host that restricts unprivileged user namespaces (stock Ubuntu
24.04, GitHub's ubuntu-latest), an unconfined minimald cannot fork a
session sandbox: its first exec dies writing /proc/self/uid_map with
EPERM. The e2e start-up block attaches the minimald AppArmor profile
to the checkout's own daemon for exactly that reason, but the
fresh-install proof drives a pair installed under a fresh mktemp home,
which no stock tunable covers (`@{HOME}/.local/bin/minimald` matches a
real home, not a nested one), so the installed daemon comes up
unconfined and the proof's first socat probe never gets a sandbox —
that is the native-daemon-e2e failure on this branch.
Attach the profile to the installed daemon — as the installer's own
advisory tells a user to — before driving the pair. `--path` replaces
the recorded set, so the lane's daemon is named alongside it, and the
subshell's EXIT trap puts the previous set back on every path out.
|
Round 1 of 10 on a failed check suite, pushed as
Verify Outside the task's
Converted to a draft: something above needs a person before review. The fresh-install own-IP ingress case that proves NET-040 now stages the minimald AppArmor profile for the installed pair on hosts that restrict unprivileged user namespaces, so the native-daemon-e2e lane — which was dying at that case's first in-box exec because the installed daemon ran unconfined — can drive the freshly installed Notes from the agent: Diagnosis of the two failed checks (both from the same run): ci-linux-native-success and native-daemon-e2e failed only in the native-daemon-e2e job — the run's |
|
For the round in flight: the |
Review of #1723: AddressPlan restated the reserved local range and the /27 slice prefix the tree already had -- the answerer's dns::RESERVED_LOCAL_RANGE and sessions' LoopbackAllocator carve-out -- so the addresses a switch publishes and the range the zone answers from could drift apart. The switch crate is now the single source. minimald's dns module re-exports RESERVED_LOCAL_RANGE instead of defining it, and LoopbackAllocator::for_slice_octet reads its slice from the default address plan rather than recomputing the carve-out, which drops sessions.rs's own /27 constant. AddressPlan::switch_capacity is const so the daemon's wrap-around derives from it at compile time. No behaviour change: same range, prefix and wrap, one definition.
|
Round 2 of 10 on the runtime's review findings, pushed as
Verify Outside the task's
Converted to a draft: something above needs a person before review. Review threads the coder judged:
At this branch a fresh install — user-local or a system package install — activating with Notes from the agent: Revise round 2: the directive listed one review finding, which I judged valid and fixed. Everything else the task's layers name (install.sh's switch-binary check, install_test.sh's case harness, the justfile case argument, the switch crate's default address plan, minimald's IpAllocator, the session-e2e case) was delivered in earlier rounds on this branch and is untouched here. Two files outside the task's layers changed: crates/minimald/src/net/dns.rs and crates/minimald/src/sessions.rs. The finding is that the branch restated the reserved local range and the /27 slice prefix the tree already had, and it cannot be fixed from the switch crate alone: the dependency runs minimald -> switch, so the only way to a single source is for minimald to re-export the switch constant (dns.rs, one item) and to read its per-daemon slice from the plan's carve-out instead of recomputing it (sessions.rs, one function body and one const). Both diffs are behaviour-preserving; the slice tests that pin them (loopback_allocators_follow_the_daemon_s_slice_octet, two_daemons_draw_disjoint_loopback_slices) pass unchanged. Could not run for real: the e2e line's own proof needs /dev/net/tun, which this box does not have and cannot mknod (no capabilities), so the line exits 0 with the proof skipped; the Linux-native CI lane runs it for real. dash is also absent, so the installer harness ran under sh only. |
The per-mapping info line re-derived its host and port from the requested ingress mappings and hardcoded the host, while apply_ingress had already returned what the switch accepted. Read the line from that result instead, through ExposedMapping's own local bind, so the record stays true to what the forwarder holds if the address expose_request binds ever moves off the loopback.
|
Round 3 of 10 on the runtime's review findings, pushed as
Verify Outside the task's
Converted to a draft: something above needs a person before review. Review threads the coder judged:
From a fresh install — including one whose only switch binary is the system package path — activating with Notes from the agent: Revise round 3: one review thread, fixed. crates/minimald/src/net/policy.rs is outside the task's layers (the daemon layer names only crates/minimald/src/net/mod.rs): the finding's remedy needs to read ExposedMapping's private |
|
/revise Three rounds in, the Also the open minimal-review thread on |
|
/revise The round the runtime opened at 03:25Z never got a container (two container-capacity refusals) and no job exists for it now, so the task status is stuck on "round 4". Same directive as my 03:13Z note: widen the directive above |
# Conflicts: # scripts/session-e2e.sh
|
/revise Base merged: origin/main is now in the branch at 2198cf8 (conflict in scripts/session-e2e.sh resolved keeping both sides; the branch case sits before main's Same directive as my 03:13Z note: widen the directive above |
The fresh-install proof's fi_cleanup runs only as the subshell's EXIT trap, which shellcheck cannot see being called. Older shellchecks (the one CI's shellcheck lane pins) report that as SC2317 across the body; newer ones (0.11, which scripts/lint-shell.sh uses locally) report the never-invoked function as SC2329 instead. Widen the directive on the definition to cover both codes, with the reasoning beside it, so the gate stays green under either shellcheck; a directive on a function definition silences the whole body, so one line is enough.
|
Round 5 of 10 on a /revise note, pushed as
Verify Outside the task's
Converted to a draft: something above needs a person before review. Review threads the coder judged:
The fresh-install proof stands on this branch — the installed pair answers 127.0.0.1:8080 with the box's own server and the daemon log names host, port and session per exposed mapping read from what the switch actually holds — and this round keeps scripts/session-e2e.sh clean under both shellcheck generations, so the lint gate CI runs over it stays green beside the proof. Notes from the agent: Round 5 delta only. (1) Widened the shellcheck directive above fi_cleanup to disable=SC2317,SC2329 with the reasoning in the comment block above it; a directive on a function definition covers the whole body (verified locally), so the one line silences CI's SC2317 and the local 0.11.0's SC2329. (2) The gvproxy_network.rs thread needed no code change this round: commit 6589f22, already on the branch, made the per-mapping line read the accepted forward's own local bind through ExposedMapping::host_port (verified against crates/minimald/src/net/policy.rs, where host_port splits the switch-held local), so the record cannot claim a host a mapping is not published on. (3) The e2e case cannot run for real on this box: it has no /dev/net/tun (its own guard skips with exit 0) and no /usr/bin/gvproxy-min half either; the native CI lane runs it for real. Building the min+minimald pair (cargo build -p minimald --bin minimald -p minimal --bin min --locked, the e2e-native recipe's build line) was needed to run that test line at all. |
There was a problem hiding this comment.
Gates met: T9's five test lines exit 0 on 87ed682, and the e2e case fresh_install_own_ip_ingress_publishes_loopback ran for real in the native-daemon-e2e lane (installed pair, shipped switch, host-loopback answer, logged expose); agent-verify passed; CI green on the head across all seven workflows; minimal-review concluded (neutral) and CodeRabbit approved on the head; the four daemon files outside the layers carry the per-mapping expose log line an earlier review thread asked for and the slice derivation the switch layer moved into the plan, both in scope; the one remaining doc thread is answered and resolved; no open threads, not a draft, no needs-human.
T9 — Publish an own-address box on host loopback from a fresh install, with the installer checking the switch binary
slice: S2
kind: task
covers: NET-040, NET-041, NET-102
size: L
est: 280 code, 380 tests
executor: agent
difficulty: needs care
model: strongest
diagnostics: the bundle's daemon log tail shows the forwarder expose call and its result, and the install log names the switch binary it checked
observability: one info log line per exposed mapping with the host address, the port and the session; the installer prints the switch binary it verified
layers:
- installer:
- scripts/install.sh: checks the switch binary is present and executable before the rename cleanup
- scripts/install_test.sh: wraps each unit section behind a case on its first argument, running all when none is given, and adds the switch-binary case
- justfile: test-installer takes an optional case and passes it to the test script
test: just test-installer installer_switch_binary_executable
- switch:
- crates/switch/src/lib.rs: installed_gvproxy_bin also probes the system package path; the default address plan type moves here so an un-enrolled host allocates from it, and each switch derives its own disjoint slice of the reserved local range from it, so two gvproxies on one host never hold the same loopback slice
test: cargo nextest run -p switch unenrolled_self_allocates_default_plan
test: cargo nextest run -p switch loopback_slices_disjoint_per_switch
- daemon:
- crates/minimald/src/net/mod.rs: IpAllocator allocates from the switch crate's default plan
test: cargo nextest run -p minimald allocator_uses_default_plan
- e2e:
- scripts/session-e2e.sh: adds the case fresh_install_own_ip_ingress_publishes_loopback from an installed build
test: ./scripts/session-e2e.sh fresh_install_own_ip_ingress_publishes_loopback
produces: none
consumes: none
proof: From a fresh install, including a system package install, activating with
--network own_ip --ingress 8080:8080answers at 127.0.0.1:8080 with the in-box server's response, and the installer confirms the switch binary is executable.depends on:
- blocked by #1627
Proof
A fresh install now prints a 'switch-binary verified ' row (naming the binary it checked) and refuses to finish — before the rename cleanup — when the switch binary it shipped is not executable; an un-enrolled host allocates own-IP addresses from the switch crate's default plan, and an own-IP activate with --ingress 8080:8080 publishes the box's internal 8080 on the host loopback so 127.0.0.1:8080 answers with the in-box server's response, one INFO daemon-log record per exposed mapping naming the host, port and session, with the system-package install (/usr/bin/gvproxy-min) covered by the same resolver and exercised by the new session-e2e case fresh_install_own_ip_ingress_publishes_loopback, which skips on hosts with no /dev/net/tun.
Notes from the agent: Out-of-layer file, named plainly: crates/minimald/src/net/gvproxy_network.rs (the daemon layer names only net/mod.rs). The observability requirement — one info line per exposed mapping with host, port and session, and a warn carrying the error when an exposure fails — lives where the exposure happens, which is apply_ingress in gvproxy_network.rs; net/mod.rs's allocator change alone could not show it. Everything else is inside the layers. Two host limitations on this runner box, both reflected honestly in the tests block: (1) /dev/net/tun is absent and mknod is blocked, so the e2e line's box-spawn half cannot be exercised here — the case gates on the device and skips with exit 0, mirroring the repo's own prerequisite wording in crates/minimald/tests/own_ip_tap_root_integration.rs; the installer half of that pipeline was verified here by hand (verified row in the install log, executable ~/.local/bin/gvproxy-min) and by the installer test line's 11 assertions, and the full case runs on the native CI lane, which has the device. (2) 127.0.0.1:8080 is permanently taken by this box's own control plane, so the case publishes on the documented fallback 18082 when 8080 answers — a clean host runs the mappi…[truncated]
Tests
just test-installer installer_switch_binary_executablecargo nextest run -p switch unenrolled_self_allocates_default_plancargo nextest run -p switch loopback_slices_disjoint_per_switchcargo nextest run -p minimald allocator_uses_default_plan./scripts/session-e2e.sh fresh_install_own_ip_ingress_publishes_loopbackVerify
min task run agent-verifypassed ([tasks.verify]).Base
The base branch moved while the coder worked and was merged in before the tests ran. These files changed on BOTH sides since the branch point — read them first, because a rewrite from a stale reading can undo a merge without a conflict:
crates/minimald/src/net/gvproxy_network.rsscripts/install.shscripts/install_test.shscripts/session-e2e.shOut of scope
These files are outside the task's
layers:, so this PR is a draft:crates/minimald/src/net/gvproxy_network.rsCloses #1633