From 786dd54257f78f95d3496a9eaefdf0faaa57b8f7 Mon Sep 17 00:00:00 2001
From: nexxo <nexxo@clupilot.local>
Date: Sat, 1 Aug 2026 13:02:23 +0200
Subject: [PATCH] EnsureNetworkBridge: schon da heisst nichts anfassen, und
 keine Bruecke auf einer Bruecke
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Der Schritt, aber erst sein billigster Teil. Zwei Zusicherungen:

- Traegt vmbr0 schon Standardroute UND Adresse, dann advance() ohne einen
  einzigen veraendernden Befehl. pve-fns-1 hat die Bruecke von Hand; ein
  Wiederanlauf, der sie umbaut, baut ein funktionierendes Netz um.
- Ist die Karte mit der Standardroute keine physische (Bond, Bridge,
  VLAN), dann fail() mit Klartext statt bauen. bridge_ports darauf waere
  falsch, und was dabei herauskommt, ist aus der Ferne nicht mehr zu
  reparieren.

readBridgeState fragt beides in einem Rundlauf und leitet aus dem
LAUFENDEN Zustand ab (ip, /sys/class/net), nie aus der Datei des
Anbieters. 'up' sind absichtlich alle drei Fakten: eine vmbr0 ohne
Adresse und ohne Standardroute ist eine Bruecke im Sinne von 'ip link'
und sonst nichts.

Start, Poll und Abbestellen folgen. Bis dahin steht dort ein fail() —
gefahrlos, weil der Schritt noch nicht in der Pipeline haengt.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 .../Steps/Host/EnsureNetworkBridge.php        | 172 ++++++++++++++++++
 tests/Feature/Provisioning/HostStepsTest.php  |  76 ++++++++
 2 files changed, 248 insertions(+)
 create mode 100644 app/Provisioning/Steps/Host/EnsureNetworkBridge.php

diff --git a/app/Provisioning/Steps/Host/EnsureNetworkBridge.php b/app/Provisioning/Steps/Host/EnsureNetworkBridge.php
new file mode 100644
index 0000000..49085ce
--- /dev/null
+++ b/app/Provisioning/Steps/Host/EnsureNetworkBridge.php
@@ -0,0 +1,172 @@
+<?php
+
+namespace App\Provisioning\Steps\Host;
+
+use App\Models\ProvisioningRun;
+use App\Provisioning\StepResult;
+use App\Services\Ssh\RemoteShell;
+
+/**
+ * Builds vmbr0 on a Debian-installed Proxmox host, under a timer that puts the
+ * old network configuration back if the bridge takes the machine off the net.
+ *
+ * Proxmox installed on top of Debian does not create vmbr0 — only the ISO
+ * installer writes that bridge into /etc/network/interfaces. BuildVmTemplate
+ * creates the golden image with `--net0 virtio,bridge=vmbr0` and every clone
+ * inherits it, so a host without the bridge serves no customer at all.
+ *
+ * ConfigureProxmox used to refuse here and send an operator to the keyboard.
+ * That was right while nobody could do this safely from a remote shell. What
+ * makes it safe is not cleverness about network layouts — it is the way back.
+ *
+ * ---------------------------------------------------------------------------
+ * The step saws through the branch it is sitting on
+ * ---------------------------------------------------------------------------
+ *
+ * By this point SSH runs over the WireGuard tunnel (keyLogin prefers wg_ip once
+ * the handshake is proven, and RebootIntoPveKernel has proven it), and wg0 hangs
+ * off the same NIC that is about to become a bridge port. A wrong bridge takes
+ * the tunnel AND the public address with it.
+ *
+ * So the order is: arm the rollback timer, change, reconnect, check, cancel. If
+ * the connection does not come back, the timer restores the old configuration
+ * and this step lands in a retry rather than on a dead server.
+ *
+ * ---------------------------------------------------------------------------
+ * Why it runs detached
+ * ---------------------------------------------------------------------------
+ *
+ * Not because it is slow — the build takes seconds. Because `ifreload -a` takes
+ * the line the command itself is running over. A synchronous call would not
+ * return a value; it would return a corpse. So the driver is started with
+ * `nohup setsid` and this step comes back every 20 seconds, each poll its own
+ * short job — and poll() does not consume the retry budget.
+ *
+ * The shell library is SHIPPED, never re-implemented here — same as
+ * BuildVmTemplate. A PHP copy of the provider-shape rules would be two
+ * installations to keep level with every Proxmox release, and the second one
+ * only ever reveals itself to whoever runs it.
+ */
+class EnsureNetworkBridge extends HostStep
+{
+    /** Where the library, its env and its status files live on the host. */
+    public const WORK_DIR = '/var/lib/clupilot/bridge';
+
+    /** The rollback timer's fuse, in minutes. */
+    private const ROLLBACK_MINUTES = 5;
+
+    /**
+     * The step's own deadline — deliberately three times the timer's fuse.
+     *
+     * By the time this step gives up, the rollback has necessarily already run,
+     * so "gave up" always means "the host is back on its old configuration".
+     * That is the line that makes "a retry rather than a dead server" true.
+     */
+    private const DEADLINE_MINUTES = 15;
+
+    /** One automatic attempt, plus one for the case an operator fixed something. */
+    private const MAX_ATTEMPTS = 2;
+
+    public function __construct(private RemoteShell $shell) {}
+
+    public function key(): string
+    {
+        return 'ensure_network_bridge';
+    }
+
+    public function maxDuration(): int
+    {
+        // Above DEADLINE_MINUTES so the step's own deadline decides the failure
+        // and can say what went wrong, rather than the generic step timeout —
+        // which would additionally spend a retry. Same shape as
+        // RebootIntoPveKernel and BuildVmTemplate.
+        return 3600;
+    }
+
+    public function execute(ProvisioningRun $run): StepResult
+    {
+        $host = $this->host($run);
+        $this->keyLogin($this->shell, $host);
+
+        $state = $this->readBridgeState();
+
+        // Already there means touch nothing. The same shortcut as "the template
+        // exists and reports template: 1" — and here it matters more, because
+        // rebuilding a working network is the one thing that can end this badly.
+        if ($state['up']) {
+            return StepResult::advance();
+        }
+
+        if ($state['iface'] === '') {
+            return StepResult::fail(
+                'No interface on this host carries the default route, so there is nothing to derive a bridge '.
+                'from. Check the machine on the provider console.'
+            );
+        }
+
+        // bridge_ports on a bond or an existing bridge is wrong: the bridge
+        // would take its own substrate as a port, and what comes out of that
+        // cannot be repaired from a remote shell.
+        if (! $state['physical']) {
+            return StepResult::fail(
+                'The interface carrying the default route ('.$state['iface'].') is not a physical NIC — it is a '.
+                'bridge, a bond or a VLAN. CluPilot will not put a bridge on top of it. Build vmbr0 by hand in '.
+                '/etc/network/interfaces (bridge_ports = the NIC carrying the default route, host address and '.
+                'gateway moved onto vmbr0), apply it with `ifreload -a`, confirm you still have SSH, then retry. '.
+                'Current default route: '.($state['route'] ?: '(none reported)').'.'
+            );
+        }
+
+        // Start, poll and cancel arrive in the next task. Harmless until then:
+        // the step is not in the pipeline yet, so no run can reach this line.
+        return StepResult::fail('not implemented yet');
+    }
+
+    /**
+     * What the host says about its bridge and its primary NIC, in one round trip.
+     *
+     * Derived from the RUNNING state — `ip`, `/sys/class/net` — never from the
+     * provider's file. What runs is structured the same everywhere; how it was
+     * written down is not, and that is the part that carries Hetzner and netcup
+     * at once.
+     *
+     * "up" is deliberately all three facts and not `ip link show`: a vmbr0 with
+     * no address and no default route is a bridge in the sense of `ip link` and
+     * nothing else. The step this replaces checked exactly that and threw the
+     * answer away.
+     *
+     * @return array{up:bool, iface:string, physical:bool, route:string}
+     */
+    private function readBridgeState(): array
+    {
+        $out = $this->shell->run(implode("\n", [
+            ': clupilot-bridge-state',
+            'B=vmbr0',
+            'IF=$(ip -4 route show default 2>/dev/null | awk \'{ for (i = 1; i < NF; i++) if ($i == "dev") { print $(i+1); exit } }\')',
+            'UP=no',
+            'if ip link show "$B" >/dev/null 2>&1 && [ "$IF" = "$B" ] && [ -n "$(ip -4 -o addr show dev "$B" scope global 2>/dev/null)" ]; then UP=yes; fi',
+            'PHY=no',
+            'if [ -n "$IF" ] && [ -e "/sys/class/net/$IF/device" ] && [ ! -d "/sys/class/net/$IF/bridge" ] && [ ! -d "/sys/class/net/$IF/bonding" ]; then PHY=yes; fi',
+            "printf 'up=%s\\n' \"\$UP\"",
+            "printf 'iface=%s\\n' \"\$IF\"",
+            "printf 'physical=%s\\n' \"\$PHY\"",
+            "printf 'route=%s\\n' \"\$(ip -4 route show default 2>/dev/null | head -1)\"",
+        ]))->stdout;
+
+        $parsed = ['up' => 'no', 'iface' => '', 'physical' => 'no', 'route' => ''];
+
+        foreach (preg_split('/\R/', $out) ?: [] as $line) {
+            [$key, $value] = array_pad(explode('=', $line, 2), 2, '');
+            if (array_key_exists($key, $parsed)) {
+                $parsed[$key] = trim($value);
+            }
+        }
+
+        return [
+            'up' => $parsed['up'] === 'yes',
+            'iface' => $parsed['iface'],
+            'physical' => $parsed['physical'] === 'yes',
+            'route' => $parsed['route'],
+        ];
+    }
+}
diff --git a/tests/Feature/Provisioning/HostStepsTest.php b/tests/Feature/Provisioning/HostStepsTest.php
index dad8f2f..b5e74ef 100644
--- a/tests/Feature/Provisioning/HostStepsTest.php
+++ b/tests/Feature/Provisioning/HostStepsTest.php
@@ -13,6 +13,7 @@ use App\Provisioning\Steps\Host\CompleteHostOnboarding;
 use App\Provisioning\Steps\Host\ConfigureProxmox;
 use App\Provisioning\Steps\Host\ConfigureWireguard;
 use App\Provisioning\Steps\Host\CreateAutomationToken;
+use App\Provisioning\Steps\Host\EnsureNetworkBridge;
 use App\Provisioning\Steps\Host\EstablishSshTrust;
 use App\Provisioning\Steps\Host\InstallProxmoxVe;
 use App\Provisioning\Steps\Host\PrepareBaseSystem;
@@ -1646,3 +1647,78 @@ it('asks only for privileges Proxmox 9 still knows', function () {
 
     expect(array_diff($angefordert, $gueltig))->toBe([]);
 });
+
+// --- EnsureNetworkBridge ---
+//
+// Der Schritt, der an dem Ast sägt, auf dem er sitzt: SSH läuft hier über den
+// Tunnel, und WireGuard hängt an derselben Karte, die in die Brücke wandert.
+// Deshalb: erst den Zeitgeber stellen, dann umstellen, dann neu verbinden,
+// nachsehen, abbestellen.
+
+/** Was der Host auf die Zustandsfrage antwortet. */
+function scriptBridgeState(FakeRemoteShell $shell, bool $up, string $iface = 'enp0s31f6', bool $physical = true): void
+{
+    $shell->script('clupilot-bridge-state', CommandResult::success(implode("\n", [
+        'up='.($up ? 'yes' : 'no'),
+        'iface='.$iface,
+        'physical='.($physical ? 'yes' : 'no'),
+        'route=default via 49.12.121.65 dev '.($up ? 'vmbr0' : $iface),
+        '',
+    ])));
+}
+
+/** Was der Host auf die Statusfrage antwortet. */
+function scriptBridgeStatus(FakeRemoteShell $shell, string $state, bool $alive = false, string $phase = '', string $note = '', bool $rolledBack = false): void
+{
+    $shell->script('clupilot-bridge-status', CommandResult::success(implode("\n", [
+        'state='.$state,
+        'alive='.($alive ? 'yes' : 'no'),
+        'phase='.$phase,
+        'note='.$note,
+        'rolledback='.($rolledBack ? 'yes' : 'no'),
+        '',
+    ])));
+}
+
+it('fasst einen Host, der die Brücke schon hat, nicht an', function () {
+    // pve-fns-1 hat sie von Hand. Ein Wiederanlauf, der sie umbaut, baut ein
+    // funktionierendes Netz um — und riskiert dafür genau das, wofür es die
+    // Rückfahrkarte gibt.
+    $s = fakeServices();
+    $host = Host::factory()->active()->create();
+    $run = hostRun($host);
+    proveTunnel($run, $host);
+    scriptBridgeState($s['shell'], up: true);
+
+    expect(app(EnsureNetworkBridge::class)->execute($run)->type)->toBe('advance')
+        ->and($s['shell']->files())->toBe([])
+        ->and($s['shell']->ran('clupilot-bridge-start'))->toBeFalse();
+});
+
+it('baut keine Brücke auf einem Bond oder einer bestehenden Brücke', function () {
+    // bridge_ports darauf ist falsch: die Brücke nähme sich ihren eigenen
+    // Unterbau als Port, und was dabei herauskommt, ist aus der Ferne nicht mehr
+    // zu reparieren.
+    $s = fakeServices();
+    $host = Host::factory()->active()->create();
+    $run = hostRun($host);
+    proveTunnel($run, $host);
+    scriptBridgeState($s['shell'], up: false, iface: 'bond0', physical: false);
+
+    $result = app(EnsureNetworkBridge::class)->execute($run);
+
+    expect($result->type)->toBe('fail')
+        ->and($result->reason)->toContain('bond0')
+        ->and($s['shell']->ran('clupilot-bridge-start'))->toBeFalse();
+});
+
+it('gibt auf, wenn gar keine Karte die Standardroute trägt', function () {
+    $s = fakeServices();
+    $host = Host::factory()->active()->create();
+    $run = hostRun($host);
+    proveTunnel($run, $host);
+    scriptBridgeState($s['shell'], up: false, iface: '', physical: false);
+
+    expect(app(EnsureNetworkBridge::class)->execute($run)->type)->toBe('fail')
+        ->and($s['shell']->ran('clupilot-bridge-start'))->toBeFalse();
+});