From 7a5a71dc89f5c715ee3eaa6fa8bed2224e03bf4f Mon Sep 17 00:00:00 2001 From: nexxo Date: Wed, 29 Jul 2026 00:18:21 +0200 Subject: [PATCH] Read VPN host state from the synced peer table, not a live hub call --- app/Livewire/Admin/Hosts.php | 29 +--- app/Models/Host.php | 46 ++++-- app/Services/Wireguard/FakeWireguardHub.php | 12 -- .../views/livewire/admin/hosts.blade.php | 8 +- tests/Feature/Admin/HostVpnStateTest.php | 151 +++++++++++------- 5 files changed, 139 insertions(+), 107 deletions(-) diff --git a/app/Livewire/Admin/Hosts.php b/app/Livewire/Admin/Hosts.php index c988954..bf67494 100644 --- a/app/Livewire/Admin/Hosts.php +++ b/app/Livewire/Admin/Hosts.php @@ -4,8 +4,6 @@ namespace App\Livewire\Admin; use App\Models\Datacenter; use App\Models\Host; -use App\Services\Wireguard\PeerSnapshot; -use App\Services\Wireguard\WireguardHub; use Livewire\Attributes\Layout; use Livewire\Attributes\Url; use Livewire\Component; @@ -36,6 +34,11 @@ class Hosts extends Component { $hosts = Host::query() ->withCount('instances') + // The VPN column reads SyncVpnPeers' synced row, not a live hub + // call — the console container has no wg0 (see VpnPeer::vpnPeer() + // and SyncVpnPeers' docblock). One eager-loaded query for the + // whole page, not one per host. + ->with('vpnPeer') ->when($this->search !== '', function ($q) { $term = '%'.$this->search.'%'; $q->where(fn ($w) => $w->where('name', 'like', $term)->orWhere('public_ip', 'like', $term)); @@ -50,28 +53,6 @@ class Hosts extends Component 'datacenters' => Datacenter::query()->orderBy('name')->get(), 'statuses' => ['pending', 'onboarding', 'active', 'error', 'disabled'], 'total' => Host::query()->count(), - ...$this->vpnPeerState(), ]); } - - /** - * One hub call for the whole list, not one per host — peers() already - * returns every peer keyed by public key, so a row just looks itself up. - * - * The hub can be down, or (dev/self-hosted) the `vpn` compose profile not - * running at all; either way this must not turn into a page-breaking - * exception. $hubReachable false is what tells vpnState() to answer - * "unknown" instead of guessing "silent" from an empty map — a wrong green - * (or a wrong amber) is worse than an honest blank. - * - * @return array{vpnPeersByKey: array, vpnHubReachable: bool} - */ - private function vpnPeerState(): array - { - try { - return ['vpnPeersByKey' => app(WireguardHub::class)->peers(), 'vpnHubReachable' => true]; - } catch (\Throwable) { - return ['vpnPeersByKey' => [], 'vpnHubReachable' => false]; - } - } } diff --git a/app/Models/Host.php b/app/Models/Host.php index 05f7afd..0414aad 100644 --- a/app/Models/Host.php +++ b/app/Models/Host.php @@ -4,12 +4,12 @@ namespace App\Models; use App\Models\Concerns\HasUuid; use App\Provisioning\Contracts\ProvisioningSubject; -use App\Services\Wireguard\PeerSnapshot; use Database\Factories\HostFactory; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\BelongsToMany; use Illuminate\Database\Eloquent\Relations\HasMany; +use Illuminate\Database\Eloquent\Relations\HasOne; use Illuminate\Database\Eloquent\Relations\MorphMany; class Host extends Model implements ProvisioningSubject @@ -55,6 +55,16 @@ class Host extends Model implements ProvisioningSubject return $this->hasMany(Instance::class); } + /** + * The synced VPN peer row SyncVpnPeers adopts for this host (see that + * job's docblock — the console container has no wg0, so this table, not a + * live hub call, is the only honest source for handshake state here). + */ + public function vpnPeer(): HasOne + { + return $this->hasOne(VpnPeer::class); + } + public function maintenanceWindows(): BelongsToMany { return $this->belongsToMany(MaintenanceWindow::class); @@ -134,28 +144,46 @@ class Host extends Model implements ProvisioningSubject */ public const VPN_HANDSHAKE_FRESH_SECONDS = 180; + /** + * How old `vpn_peers.observed_at` may be before its `last_handshake_at` is + * no longer trusted. SyncVpnPeers runs every minute (routes/console.php); + * 300s is five missed scheduler ticks — the same order of magnitude as + * healthState()'s own "still counts as online" heartbeat window (5 min) + * for a per-minute signal, generous enough to absorb ordinary queue + * latency without being trigger-happy, tight enough to actually catch the + * sync worker being down rather than the tunnel. + */ + public const VPN_SYNC_STALE_SECONDS = 300; + /** * VPN tunnel state for the console's hosts list. * * `wg_pubkey` only proves onboarding CONFIGURED a peer — a tunnel silent for * two days looks identical in the database to one that handshaked a minute - * ago. $snapshot (from WireguardHub::peers(), live and not persisted) is - * what actually answers "is it up right now"; $hubReachable distinguishes - * "the hub said no recent handshake" from "the hub could not be asked at - * all" — collapsing those would show a false state instead of an honest one. + * ago. $peer (the row SyncVpnPeers keeps current — see that job's docblock: + * the console container has no wg0, so this table is the only honest source + * here) carries both `last_handshake_at` and `observed_at`. + * + * Two different kinds of "we don't know" collapse to `unknown`, not to a + * guessed `silent`: no row at all (never synced yet), and a row whose + * `observed_at` is stale — because a stopped sync worker leaves + * `last_handshake_at` frozen at whatever it last saw, which would + * otherwise be misread as "the tunnel went silent" when the truth is + * "nobody has looked lately". */ - public function vpnState(?PeerSnapshot $snapshot, bool $hubReachable): string + public function vpnState(?VpnPeer $peer): string { if ($this->wg_pubkey === null) { return 'not_configured'; } - if (! $hubReachable) { + if ($peer === null || $peer->observed_at === null + || $peer->observed_at->diffInSeconds(now()) > self::VPN_SYNC_STALE_SECONDS) { return 'unknown'; } - if ($snapshot?->latestHandshake !== null - && $snapshot->latestHandshake->diffInSeconds(now()) <= self::VPN_HANDSHAKE_FRESH_SECONDS) { + if ($peer->last_handshake_at !== null + && $peer->last_handshake_at->diffInSeconds(now()) <= self::VPN_HANDSHAKE_FRESH_SECONDS) { return 'connected'; } diff --git a/app/Services/Wireguard/FakeWireguardHub.php b/app/Services/Wireguard/FakeWireguardHub.php index 160aa95..7e6422a 100644 --- a/app/Services/Wireguard/FakeWireguardHub.php +++ b/app/Services/Wireguard/FakeWireguardHub.php @@ -16,12 +16,6 @@ class FakeWireguardHub implements WireguardHub /** Snapshots the tests can shape; defaults to "configured, never handshaked". */ public array $snapshots = []; - /** When set, peers() throws — simulates the hub being unreachable (down, or the `vpn` profile not running). */ - public bool $failPeers = false; - - /** How many times peers() was called — tests use this to prove a caller fetches once, not once per host. */ - public int $peersCalls = 0; - public function allocateIp(): string { return '10.66.0.'.$this->next++; @@ -39,12 +33,6 @@ class FakeWireguardHub implements WireguardHub public function peers(): array { - $this->peersCalls++; - - if ($this->failPeers) { - throw new \RuntimeException('wg show failed: interface not found'); - } - $peers = []; foreach ($this->peers as $publicKey => $ip) { $peers[$publicKey] = $this->snapshots[$publicKey] ?? new PeerSnapshot( diff --git a/resources/views/livewire/admin/hosts.blade.php b/resources/views/livewire/admin/hosts.blade.php index 1399be7..038af2c 100644 --- a/resources/views/livewire/admin/hosts.blade.php +++ b/resources/views/livewire/admin/hosts.blade.php @@ -61,8 +61,8 @@ $hdot = ['online' => 'bg-success-bright', 'stale' => 'bg-warning', 'offline' => 'bg-danger'][$health]; $usedPct = $host->usedPct(); - $vpnSnapshot = $host->wg_pubkey !== null ? ($vpnPeersByKey[$host->wg_pubkey] ?? null) : null; - $vpnState = $host->vpnState($vpnSnapshot, $vpnHubReachable); + $vpnPeer = $host->vpnPeer; + $vpnState = $host->vpnState($vpnPeer); $vpnIcon = ['not_configured' => 'unlock', 'connected' => 'lock', 'silent' => 'alert-triangle', 'unknown' => 'life-buoy'][$vpnState]; $vpnTone = ['not_configured' => 'text-muted', 'connected' => 'text-success', 'silent' => 'text-warning', 'unknown' => 'text-muted'][$vpnState]; @endphp @@ -85,8 +85,8 @@ - @if ($vpnState === 'silent' && $vpnSnapshot?->latestHandshake) - {{ __('hosts.vpn.silent_since', ['time' => $vpnSnapshot->latestHandshake->diffForHumans()]) }} + @if ($vpnState === 'silent' && $vpnPeer?->last_handshake_at) + {{ __('hosts.vpn.silent_since', ['time' => $vpnPeer->last_handshake_at->diffForHumans()]) }} @else {{ __('hosts.vpn.'.$vpnState) }} @endif diff --git a/tests/Feature/Admin/HostVpnStateTest.php b/tests/Feature/Admin/HostVpnStateTest.php index 4737b80..c4e95dc 100644 --- a/tests/Feature/Admin/HostVpnStateTest.php +++ b/tests/Feature/Admin/HostVpnStateTest.php @@ -1,105 +1,124 @@ subSeconds($seconds), - rxBytes: 100, - txBytes: 100, - ); + return VpnPeer::factory()->forHost()->create([ + 'host_id' => $host->id, + 'name' => $host->name, + 'last_handshake_at' => $handshakeSecondsAgo === null ? null : CarbonImmutable::now()->subSeconds($handshakeSecondsAgo), + 'observed_at' => CarbonImmutable::now()->subSeconds($observedSecondsAgo), + ]); } it('reports not_configured for a host with no wg_pubkey at all', function () { $host = Host::factory()->create(['wg_pubkey' => null]); - expect($host->vpnState(null, hubReachable: true))->toBe('not_configured'); + expect($host->vpnState(null))->toBe('not_configured'); }); it('reports connected for a fresh handshake', function () { $host = Host::factory()->create(['wg_pubkey' => 'HOSTPUB=']); - $snapshot = snapshotAgedSeconds('HOSTPUB=', 30); + $peer = vpnPeerFor($host, handshakeSecondsAgo: 30); - expect($host->vpnState($snapshot, hubReachable: true))->toBe('connected'); + expect($host->vpnState($peer))->toBe('connected'); }); it('reports silent for a stale handshake', function () { $host = Host::factory()->create(['wg_pubkey' => 'HOSTPUB=']); - $snapshot = snapshotAgedSeconds('HOSTPUB=', 60 * 60 * 24 * 2); // two days + $peer = vpnPeerFor($host, handshakeSecondsAgo: 60 * 60 * 24 * 2); // two days - expect($host->vpnState($snapshot, hubReachable: true))->toBe('silent'); + expect($host->vpnState($peer))->toBe('silent'); }); it('reports silent for a configured peer that never handshaked', function () { $host = Host::factory()->create(['wg_pubkey' => 'HOSTPUB=']); - $snapshot = new PeerSnapshot('HOSTPUB=', null, '10.66.0.9/32', null, 0, 0); + $peer = vpnPeerFor($host, handshakeSecondsAgo: null); - expect($host->vpnState($snapshot, hubReachable: true))->toBe('silent'); + expect($host->vpnState($peer))->toBe('silent'); }); -it('reports unknown when the hub could not be asked, even for a configured host', function () { +it('reports unknown when the host is configured but has never been synced', function () { $host = Host::factory()->create(['wg_pubkey' => 'HOSTPUB=']); - expect($host->vpnState(null, hubReachable: false))->toBe('unknown'); + expect($host->vpnState(null))->toBe('unknown'); }); -it('never reports not_configured as unknown or vice versa regardless of hub reachability', function () { +it('reports unknown when the sync itself has gone stale, even with an old handshake on record', function () { + $host = Host::factory()->create(['wg_pubkey' => 'HOSTPUB=']); + // The sync worker stopped an hour ago; the last thing it saw was a fresh + // handshake. Trusting last_handshake_at here would say "connected" for a + // host nobody has actually checked on in an hour. + $peer = vpnPeerFor($host, handshakeSecondsAgo: 10, observedSecondsAgo: 60 * 60); + + expect($host->vpnState($peer))->toBe('unknown'); +}); + +it('never reports not_configured as unknown or vice versa regardless of sync state', function () { $unconfigured = Host::factory()->create(['wg_pubkey' => null]); - expect($unconfigured->vpnState(null, hubReachable: false))->toBe('not_configured'); + expect($unconfigured->vpnState(null))->toBe('not_configured'); }); /** - * Pins the exact threshold boundary — this is the comparison the mutation - * pass targets (`<=` flipped to `<`, or the constant changed) to prove the - * test actually fails when a stale handshake would otherwise read as connected. + * Pins the exact freshness boundary — the comparison a mutation pass targets + * (`<=` flipped to `<`, or the constant changed) to prove the test actually + * fails when a stale handshake would otherwise read as connected. */ it('treats a handshake older than the freshness threshold as silent, not connected', function () { $host = Host::factory()->create(['wg_pubkey' => 'HOSTPUB=']); // Frozen clock: a wall-clock version of this test is one CPU hiccup away // from putting "now" a millisecond past the boundary it means to sit at. - // travelTo() freezes both Carbon and CarbonImmutable's now(). - $now = CarbonImmutable::parse('2026-07-28 12:00:00'); - $this->travelTo($now); + $this->travelTo(CarbonImmutable::parse('2026-07-28 12:00:00')); - $atThreshold = new PeerSnapshot('HOSTPUB=', null, '10.66.0.9/32', $now->subSeconds(Host::VPN_HANDSHAKE_FRESH_SECONDS), 0, 0); - $pastThreshold = new PeerSnapshot('HOSTPUB=', null, '10.66.0.9/32', $now->subSeconds(Host::VPN_HANDSHAKE_FRESH_SECONDS + 1), 0, 0); + $atThreshold = vpnPeerFor($host, handshakeSecondsAgo: Host::VPN_HANDSHAKE_FRESH_SECONDS); + $pastThreshold = vpnPeerFor($host, handshakeSecondsAgo: Host::VPN_HANDSHAKE_FRESH_SECONDS + 1); - expect($host->vpnState($atThreshold, hubReachable: true))->toBe('connected') - ->and($host->vpnState($pastThreshold, hubReachable: true))->toBe('silent'); + expect($host->vpnState($atThreshold))->toBe('connected') + ->and($host->vpnState($pastThreshold))->toBe('silent'); + + $this->travelBack(); +}); + +/** + * Pins the sync-staleness boundary — the second comparison a mutation pass + * targets, proving a frozen sync reads as unknown rather than as whatever + * last_handshake_at happens to say. + */ +it('treats a stale observed_at as unknown, not as whatever the frozen handshake says', function () { + $host = Host::factory()->create(['wg_pubkey' => 'HOSTPUB=']); + + $this->travelTo(CarbonImmutable::parse('2026-07-28 12:00:00')); + + $atThreshold = vpnPeerFor($host, handshakeSecondsAgo: 10, observedSecondsAgo: Host::VPN_SYNC_STALE_SECONDS); + $pastThreshold = vpnPeerFor($host, handshakeSecondsAgo: 10, observedSecondsAgo: Host::VPN_SYNC_STALE_SECONDS + 1); + + expect($host->vpnState($atThreshold))->toBe('connected') + ->and($host->vpnState($pastThreshold))->toBe('unknown'); $this->travelBack(); }); it('renders the three VPN states distinctly in the hosts list', function () { - $hub = new FakeWireguardHub; - app()->instance(WireguardHub::class, $hub); - $unconfigured = Host::factory()->active()->create(['name' => 'pve-none-1', 'wg_pubkey' => null]); $connected = Host::factory()->active()->create(['name' => 'pve-conn-1', 'wg_pubkey' => 'CONNPUB=']); $silent = Host::factory()->active()->create(['name' => 'pve-silent-1', 'wg_pubkey' => 'SILENTPUB=']); - $hub->addPeer('CONNPUB=', '10.66.0.10'); - $hub->addPeer('SILENTPUB=', '10.66.0.11'); - $hub->snapshots['CONNPUB='] = snapshotAgedSeconds('CONNPUB=', 20); - $hub->snapshots['SILENTPUB='] = snapshotAgedSeconds('SILENTPUB=', 60 * 60 * 24 * 3); + vpnPeerFor($connected, handshakeSecondsAgo: 20); + vpnPeerFor($silent, handshakeSecondsAgo: 60 * 60 * 24 * 3); $response = $this->actingAs(admin(), 'operator')->get(route('admin.hosts')); @@ -111,27 +130,43 @@ it('renders the three VPN states distinctly in the hosts list', function () { ->assertSee(__('hosts.vpn.silent')); }); -it('shows unknown, not a false connected or silent, when the hub is unreachable', function () { - $hub = new FakeWireguardHub; - $hub->failPeers = true; - app()->instance(WireguardHub::class, $hub); - - $host = Host::factory()->active()->create(['name' => 'pve-unreachable-1', 'wg_pubkey' => 'HOSTPUB=']); +it('shows unknown, not a false connected or silent, when a configured host was never synced', function () { + $host = Host::factory()->active()->create(['name' => 'pve-unsynced-1', 'wg_pubkey' => 'HOSTPUB=']); $this->actingAs(admin(), 'operator')->get(route('admin.hosts')) ->assertOk() ->assertSeeInOrder([$host->name, __('hosts.vpn.unknown')]); }); -it('calls the hub once for the whole list, not once per host', function () { - $hub = new FakeWireguardHub; - app()->instance(WireguardHub::class, $hub); +it('loads the VPN column with one eager-loaded query, not one per host', function () { + // Isolated to exactly what the VPN column added — Host::query()->with('vpnPeer') + // — rather than the full page response, which also carries an unrelated, + // pre-existing capacity-column query per host (committedGb()) not part of + // this change and out of scope here. + $queryCount = 0; + DB::listen(function () use (&$queryCount) { + $queryCount++; + }); - foreach (range(1, 5) as $i) { - Host::factory()->active()->create(['wg_pubkey' => 'PUB'.$i.'=']); - } + $countQueriesFor = function (int $hostCount) use (&$queryCount) { + Host::query()->delete(); + VpnPeer::query()->forceDelete(); - Livewire::actingAs(admin(), 'operator')->test(Hosts::class); + foreach (range(1, $hostCount) as $i) { + $host = Host::factory()->active()->create(['wg_pubkey' => 'PUB'.$i.'=']); + vpnPeerFor($host, handshakeSecondsAgo: 20); + } - expect($hub->peersCalls)->toBe(1); + // Reset right before the load being measured. + $queryCount = 0; + Host::query()->with('vpnPeer')->get()->each(fn ($h) => $h->vpnPeer); + + return $queryCount; + }; + + $queriesFor2 = $countQueriesFor(2); + $queriesFor10 = $countQueriesFor(10); + + expect($queriesFor2)->toBe(2) // one for hosts, one eager-loaded query for their vpn peers + ->and($queriesFor10)->toBe($queriesFor2); });