Retain failed failover candidate status

src/opnsense/mvc/app/library/OPNsense/NordVPN/Reconciler.php

@@ -102,18 +102,10 @@ class Reconciler
                 }
             }
             if ($next === null) {
-                $next = [
+                $next = $this->failureStatus('unavailable', 'No eligible NordVPN server established a handshake.');
-                    'available' => false,
-                    'state' => 'unavailable',
-                    'message' => 'No eligible NordVPN server established a handshake.',
-                ];
             }
         } catch (\Throwable $exception) {
-            $next = [
+            $next = $this->failureStatus('error', $exception->getMessage());
-                'available' => false,
-                'state' => 'error',
-                'message' => $exception->getMessage(),
-            ];
         }
         return $this->publish($next);
     }
@@ -138,4 +130,16 @@ class Reconciler
             explode(',', $csv)
         ))));
     }
+
+    private function failureStatus(string $state, string $message): array
+    {
+        $settings = $this->configuration->settings();
+        return [
+            'available' => false,
+            'state' => $state,
+            'server_hostname' => (string)$settings->managed->server_hostname,
+            'server_country' => (string)$settings->managed->server_country,
+            'message' => $message,
+        ];
+    }
 }

tests/reconciler.php

@@ -1,235 +0,0 @@
-<?php
-
-// SPDX-License-Identifier: BSD-2-Clause
-// SPDX-FileCopyrightText: 2026 ms2k
-
-namespace OPNsense\NordVPN;
-
-class Settings
-{
-    public object $general;
-    public object $managed;
-
-    public function __construct()
-    {
-        $this->general = (object)['countries' => ''];
-        $this->managed = (object)[
-            'server_country' => 'NL',
-            'server_hostname' => 'nl.example.invalid',
-        ];
-    }
-}
-
-$root = dirname(__DIR__);
-foreach (['NordApi', 'EndpointStore', 'StatusStore', 'SystemRunner', 'TunnelHealth', 'ManagedConfiguration', 'Reconciler'] as $class) {
-    require_once $root . '/src/opnsense/mvc/app/library/OPNsense/NordVPN/' . $class . '.php';
-}
-
-class TestConfiguration extends ManagedConfiguration
-{
-    private Settings $testSettings;
-
-    public function __construct()
-    {
-        $this->testSettings = new Settings();
-    }
-
-    public function settings(): Settings
-    {
-        return $this->testSettings;
-    }
-
-    public function interfaceName(): ?string
-    {
-        return 'wg0';
-    }
-
-    public function repairGatewayInterface(): bool
-    {
-        return false;
-    }
-}
-
-class AvailableHealth extends TunnelHealth
-{
-    public function __construct()
-    {
-    }
-
-    public function check(string $interface): array
-    {
-        return ['available' => true, 'last_handshake' => 1000];
-    }
-}
-
-class FailOnceRunner extends SystemRunner
-{
-    public int $firewallCalls = 0;
-
-    public function applyFirewall(): void
-    {
-        $this->firewallCalls++;
-        if ($this->firewallCalls === 1) {
-            throw new \RuntimeException('Synthetic firewall failure.');
-        }
-    }
-}
-
-class RepairConfiguration extends TestConfiguration
-{
-    public function repairGatewayInterface(): bool
-    {
-        return true;
-    }
-}
-
-class RepairRunner extends SystemRunner
-{
-    public int $routingCalls = 0;
-    public int $firewallCalls = 0;
-
-    public function applyRouting(): void
-    {
-        $this->routingCalls++;
-    }
-
-    public function applyFirewall(): void
-    {
-        $this->firewallCalls++;
-    }
-}
-
-class FailoverConfiguration extends TestConfiguration
-{
-    public bool $candidateUpdated = false;
-    public int $repairCalls = 0;
-
-    public function updateCandidate(array $candidate): void
-    {
-        $this->candidateUpdated = true;
-    }
-
-    public function repairGatewayInterface(): bool
-    {
-        if (!$this->candidateUpdated) {
-            return false;
-        }
-        $this->candidateUpdated = false;
-        $this->repairCalls++;
-        return true;
-    }
-}
-
-class RecoveringHealth extends TunnelHealth
-{
-    private int $checks = 0;
-
-    public function __construct()
-    {
-    }
-
-    public function check(string $interface): array
-    {
-        $this->checks++;
-        return ['available' => $this->checks > 1, 'last_handshake' => $this->checks > 1 ? 1000 : 0];
-    }
-}
-
-class FailoverRunner extends SystemRunner
-{
-    public int $wireGuardCalls = 0;
-    public int $routingCalls = 0;
-    public int $firewallCalls = 0;
-
-    public function applyWireGuard(): void
-    {
-        $this->wireGuardCalls++;
-    }
-
-    public function applyRouting(): void
-    {
-        $this->routingCalls++;
-    }
-
-    public function applyFirewall(): void
-    {
-        $this->firewallCalls++;
-    }
-}
-
-$repairPath = sys_get_temp_dir() . '/nordvpn-repair-' . bin2hex(random_bytes(8)) . '.json';
-$repairRunner = new RepairRunner();
-(new Reconciler(
-    new NordApi(static fn() => []),
-    new RepairConfiguration(),
-    new AvailableHealth(),
-    new StatusStore($repairPath),
-    $repairRunner
-))->run();
-if ($repairRunner->routingCalls !== 1 || $repairRunner->firewallCalls !== 2) {
-    throw new \RuntimeException('Gateway interface repair was not applied.');
-}
-unlink($repairPath);
-echo "ok - reconciliation applies gateway interface repair\n";
-
-$failoverPath = sys_get_temp_dir() . '/nordvpn-failover-' . bin2hex(random_bytes(8)) . '.json';
-$failoverConfiguration = new FailoverConfiguration();
-$failoverRunner = new FailoverRunner();
-$publicKey = base64_encode(str_repeat("\0", 32));
-(new Reconciler(
-    new NordApi(static fn() => [[
-        'hostname' => 'nl2.example.invalid',
-        'station' => '198.51.100.2',
-        'locations' => [['country' => ['code' => 'NL']]],
-        'technologies' => [[
-            'id' => 35,
-            'pivot' => ['status' => 'online'],
-            'metadata' => [['name' => 'public_key', 'value' => $publicKey]],
-        ]],
-    ]]),
-    $failoverConfiguration,
-    new RecoveringHealth(),
-    new StatusStore($failoverPath),
-    $failoverRunner,
-    static fn($seconds) => null
-))->run();
-if ($failoverRunner->wireGuardCalls !== 1 ||
-    $failoverConfiguration->repairCalls !== 1 ||
-    $failoverRunner->routingCalls !== 1 ||
-    $failoverRunner->firewallCalls !== 1) {
-    throw new \RuntimeException('Server failover did not repair the gateway interface.');
-}
-unlink($failoverPath);
-echo "ok - server failover repairs the gateway interface\n";
-
-$path = sys_get_temp_dir() . '/nordvpn-retry-' . bin2hex(random_bytes(8)) . '.json';
-$status = new StatusStore($path);
-$runner = new FailOnceRunner();
-$reconciler = new Reconciler(
-    new NordApi(static fn() => []),
-    new TestConfiguration(),
-    new AvailableHealth(),
-    $status,
-    $runner
-);
-
-try {
-    $reconciler->run();
-    throw new \RuntimeException('The first firewall failure was not reported.');
-} catch (\RuntimeException $exception) {
-    if ($exception->getMessage() !== 'Synthetic firewall failure.') {
-        throw $exception;
-    }
-}
-
-if (empty($status->read()['firewall_pending'])) {
-    throw new \RuntimeException('The failed firewall apply was not retained as pending.');
-}
-
-$reconciler->run();
-if ($runner->firewallCalls !== 2 || !empty($status->read()['firewall_pending'])) {
-    throw new \RuntimeException('The pending firewall apply was not completed on retry.');
-}
-
-unlink($path);
-echo "ok - reconciliation retries a failed firewall apply\n";

tests/unit/ReconcilerTest.php

@@ -0,0 +1,273 @@
+<?php
+
+use OPNsense\NordVPN\ManagedConfiguration;
+use OPNsense\NordVPN\NordApi;
+use OPNsense\NordVPN\Reconciler;
+use OPNsense\NordVPN\Settings;
+use OPNsense\NordVPN\StatusStore;
+use OPNsense\NordVPN\SystemRunner;
+use OPNsense\NordVPN\TunnelHealth;
+use PHPUnit\Framework\TestCase;
+
+class ReconcilerTestConfiguration extends ManagedConfiguration
+{
+    private Settings $testSettings;
+
+    public function __construct()
+    {
+        $this->testSettings = new Settings();
+    }
+
+    public function settings(): Settings
+    {
+        return $this->testSettings;
+    }
+
+    public function interfaceName(): ?string
+    {
+        return 'wg0';
+    }
+
+    public function repairGatewayInterface(): bool
+    {
+        return false;
+    }
+}
+
+class RepairReconcilerConfiguration extends ReconcilerTestConfiguration
+{
+    public function repairGatewayInterface(): bool
+    {
+        return true;
+    }
+}
+
+class FailoverReconcilerConfiguration extends ReconcilerTestConfiguration
+{
+    public bool $candidateUpdated = false;
+    public int $repairCalls = 0;
+
+    public function updateCandidate(array $candidate): void
+    {
+        $this->candidateUpdated = true;
+        $this->settings()->managed->server_hostname = $candidate['hostname'];
+        $this->settings()->managed->server_country = $candidate['country'];
+    }
+
+    public function repairGatewayInterface(): bool
+    {
+        if (!$this->candidateUpdated) {
+            return false;
+        }
+        $this->candidateUpdated = false;
+        $this->repairCalls++;
+        return true;
+    }
+}
+
+class AvailableReconcilerHealth extends TunnelHealth
+{
+    public function __construct()
+    {
+    }
+
+    public function check(string $interface): array
+    {
+        return ['available' => true, 'last_handshake' => 1000];
+    }
+}
+
+class RecoveringReconcilerHealth extends TunnelHealth
+{
+    private int $checks = 0;
+
+    public function __construct()
+    {
+    }
+
+    public function check(string $interface): array
+    {
+        $this->checks++;
+        return ['available' => $this->checks > 1, 'last_handshake' => $this->checks > 1 ? 1000 : 0];
+    }
+}
+
+class UnavailableReconcilerHealth extends TunnelHealth
+{
+    public function __construct()
+    {
+    }
+
+    public function check(string $interface): array
+    {
+        return ['available' => false, 'last_handshake' => 0];
+    }
+}
+
+class FailOnceReconcilerRunner extends SystemRunner
+{
+    public int $firewallCalls = 0;
+
+    public function applyFirewall(): void
+    {
+        $this->firewallCalls++;
+        if ($this->firewallCalls === 1) {
+            throw new RuntimeException('Synthetic firewall failure.');
+        }
+    }
+}
+
+class RepairReconcilerRunner extends SystemRunner
+{
+    public int $routingCalls = 0;
+    public int $firewallCalls = 0;
+
+    public function applyRouting(): void
+    {
+        $this->routingCalls++;
+    }
+
+    public function applyFirewall(): void
+    {
+        $this->firewallCalls++;
+    }
+}
+
+class FailoverReconcilerRunner extends SystemRunner
+{
+    public int $wireGuardCalls = 0;
+    public int $routingCalls = 0;
+    public int $firewallCalls = 0;
+
+    public function applyWireGuard(): void
+    {
+        $this->wireGuardCalls++;
+    }
+
+    public function applyRouting(): void
+    {
+        $this->routingCalls++;
+    }
+
+    public function applyFirewall(): void
+    {
+        $this->firewallCalls++;
+    }
+}
+
+final class ReconcilerTest extends TestCase
+{
+    private string $key;
+
+    protected function setUp(): void
+    {
+        $this->key = base64_encode(str_repeat("\0", 32));
+    }
+
+    public function testReconciliationAppliesGatewayInterfaceRepair(): void
+    {
+        $path = $this->temporaryStatusPath();
+        $runner = new RepairReconcilerRunner();
+
+        (new Reconciler(
+            new NordApi(static fn() => []),
+            new RepairReconcilerConfiguration(),
+            new AvailableReconcilerHealth(),
+            new StatusStore($path),
+            $runner
+        ))->run();
+
+        $this->assertSame(1, $runner->routingCalls);
+        $this->assertSame(2, $runner->firewallCalls);
+        unlink($path);
+    }
+
+    public function testServerFailoverRepairsGatewayInterface(): void
+    {
+        $path = $this->temporaryStatusPath();
+        $configuration = new FailoverReconcilerConfiguration();
+        $runner = new FailoverReconcilerRunner();
+
+        (new Reconciler(
+            new NordApi(fn() => [$this->candidate('nl2.example.invalid', '198.51.100.2')]),
+            $configuration,
+            new RecoveringReconcilerHealth(),
+            new StatusStore($path),
+            $runner,
+            static fn($seconds) => null
+        ))->run();
+
+        $this->assertSame(1, $runner->wireGuardCalls);
+        $this->assertSame(1, $configuration->repairCalls);
+        $this->assertSame(1, $runner->routingCalls);
+        $this->assertSame(1, $runner->firewallCalls);
+        unlink($path);
+    }
+
+    public function testFailedFirewallApplyIsRetried(): void
+    {
+        $path = $this->temporaryStatusPath();
+        $status = new StatusStore($path);
+        $runner = new FailOnceReconcilerRunner();
+        $reconciler = new Reconciler(
+            new NordApi(static fn() => []),
+            new ReconcilerTestConfiguration(),
+            new AvailableReconcilerHealth(),
+            $status,
+            $runner
+        );
+
+        try {
+            $reconciler->run();
+            $this->fail('The first firewall failure was not reported.');
+        } catch (RuntimeException $exception) {
+            $this->assertSame('Synthetic firewall failure.', $exception->getMessage());
+        }
+
+        $this->assertTrue($status->read()['firewall_pending']);
+        $reconciler->run();
+        $this->assertSame(2, $runner->firewallCalls);
+        $this->assertFalse($status->read()['firewall_pending']);
+        unlink($path);
+    }
+
+    public function testUnavailableStatusNamesLastAttemptedServer(): void
+    {
+        $path = $this->temporaryStatusPath();
+        $configuration = new FailoverReconcilerConfiguration();
+        $result = (new Reconciler(
+            new NordApi(fn() => [
+                $this->candidate('nl2.example.invalid', '198.51.100.2'),
+                $this->candidate('nl3.example.invalid', '198.51.100.3'),
+            ]),
+            $configuration,
+            new UnavailableReconcilerHealth(),
+            new StatusStore($path),
+            new FailoverReconcilerRunner(),
+            static fn($seconds) => null
+        ))->run();
+
+        $this->assertSame('nl3.example.invalid', $result['server_hostname']);
+        $this->assertSame('NL', $result['server_country']);
+        unlink($path);
+    }
+
+    private function temporaryStatusPath(): string
+    {
+        return sys_get_temp_dir() . '/nordvpn-reconciler-' . bin2hex(random_bytes(8)) . '.json';
+    }
+
+    private function candidate(string $hostname, string $address): array
+    {
+        return [
+            'hostname' => $hostname,
+            'station' => $address,
+            'locations' => [['country' => ['code' => 'NL']]],
+            'technologies' => [[
+                'id' => 35,
+                'pivot' => ['status' => 'online'],
+                'metadata' => [['name' => 'public_key', 'value' => $this->key]],
+            ]],
+        ];
+    }
+}