diff --git a/.env.example b/.env.example index 46a31d1..7ac9a47 100644 --- a/.env.example +++ b/.env.example @@ -105,6 +105,12 @@ STRIPE_SECRET= HETZNER_DNS_TOKEN= CLUPILOT_DNS_ZONE=clupilot.com +# Host names (fsn-01.node.…) are NOT published here — that would leak the +# internal WireGuard subnet. They go into this directory instead, which the +# vpn-dns container reads (dnsmasq --hostsdir) over the dns-hosts volume; see +# docker-compose.yml. Only change if that volume is mounted somewhere else. +CLUPILOT_DNS_HOSTS_DIR=/etc/clupilot/dns-hosts + # ── Traefik (reverse proxy + TLS) ──────────────────────────────────────── # Directory ON THE PROXMOX HOST where CluPilot writes dynamic route files. TRAEFIK_DYNAMIC_PATH=/etc/traefik/dynamic diff --git a/app/Console/Commands/PruneHostDns.php b/app/Console/Commands/PruneHostDns.php new file mode 100644 index 0000000..325d68d --- /dev/null +++ b/app/Console/Commands/PruneHostDns.php @@ -0,0 +1,72 @@ +whereNotNull('dns_record_id')->orderBy('name')->get(); + + if ($hosts->isEmpty()) { + $this->info('No host carries a public dns_record_id. Nothing to do.'); + + return self::SUCCESS; + } + + $force = (bool) $this->option('force'); + $failed = 0; + + foreach ($hosts as $host) { + $label = $host->name !== '' && $host->name !== null ? $host->name : $host->uuid; + $recordId = $host->dns_record_id; + + if (! $force) { + $this->line(" would delete {$label} record={$recordId}"); + + continue; + } + + try { + $dns->deleteRecord($recordId); + $host->update(['dns_record_id' => null]); + $this->info(" deleted {$label} record={$recordId}"); + } catch (Throwable $e) { + $failed++; + $this->error(" failed {$label} record={$recordId}: {$e->getMessage()}"); + } + } + + if (! $force) { + $this->newLine(); + $this->comment(sprintf('Dry run — %d record(s) found, nothing changed. Re-run with --force to delete.', $hosts->count())); + + return self::SUCCESS; + } + + return $failed === 0 ? self::SUCCESS : self::FAILURE; + } +} diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index d293e8c..7ad2c6b 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -2,31 +2,33 @@ namespace App\Providers; -use App\Listeners\RecordSignInDevice; -use App\Provisioning\PipelineRegistry; -use App\Services\Dns\HetznerDnsClient; -use App\Services\Dns\HttpHetznerDnsClient; -use App\Services\Monitoring\HttpMonitoringClient; -use App\Services\Monitoring\MonitoringClient; -use App\Services\Proxmox\HttpProxmoxClient; -use App\Services\Proxmox\ProxmoxClient; -use App\Services\Stripe\HttpStripeClient; -use App\Services\Stripe\StripeClient; -use App\Services\Ssh\PhpseclibRemoteShell; -use App\Services\Ssh\RemoteShell; -use App\Services\Traefik\SshTraefikWriter; -use App\Services\Traefik\TraefikWriter; -use App\Services\Wireguard\LocalWireguardHub; -use App\Services\Wireguard\WireguardHub; use App\Http\Middleware\EnsureAdmin; use App\Http\Middleware\EnsureCustomerActive; use App\Http\Middleware\RestrictAdminHost; use App\Http\Middleware\RestrictConsoleNetwork; +use App\Listeners\RecordSignInDevice; use App\Mail\MaintenanceCancelledMail; -use App\Models\MaintenanceNotification; -use App\Services\Maintenance\MaintenanceNotifier; -use Carbon\CarbonImmutable; use App\Mail\Transport\MailboxTransport; +use App\Models\MaintenanceNotification; +use App\Provisioning\PipelineRegistry; +use App\Services\Dns\FileHostDnsDirectory; +use App\Services\Dns\HetznerDnsClient; +use App\Services\Dns\HostDnsDirectory; +use App\Services\Dns\HttpHetznerDnsClient; +use App\Services\Maintenance\MaintenanceNotifier; +use App\Services\Monitoring\HttpMonitoringClient; +use App\Services\Monitoring\MonitoringClient; +use App\Services\Proxmox\HttpProxmoxClient; +use App\Services\Proxmox\ProxmoxClient; +use App\Services\Ssh\PhpseclibRemoteShell; +use App\Services\Ssh\RemoteShell; +use App\Services\Stripe\HttpStripeClient; +use App\Services\Stripe\StripeClient; +use App\Services\Traefik\SshTraefikWriter; +use App\Services\Traefik\TraefikWriter; +use App\Services\Wireguard\LocalWireguardHub; +use App\Services\Wireguard\WireguardHub; +use Carbon\CarbonImmutable; use Illuminate\Auth\Events\Login; use Illuminate\Mail\Events\MessageSending; use Illuminate\Mail\Events\MessageSent; @@ -52,6 +54,7 @@ class AppServiceProvider extends ServiceProvider $this->app->bind(WireguardHub::class, LocalWireguardHub::class); $this->app->bind(ProxmoxClient::class, HttpProxmoxClient::class); $this->app->bind(HetznerDnsClient::class, HttpHetznerDnsClient::class); + $this->app->bind(HostDnsDirectory::class, FileHostDnsDirectory::class); $this->app->bind(TraefikWriter::class, SshTraefikWriter::class); $this->app->bind(MonitoringClient::class, HttpMonitoringClient::class); $this->app->bind(StripeClient::class, HttpStripeClient::class); diff --git a/app/Provisioning/Jobs/PurgeHost.php b/app/Provisioning/Jobs/PurgeHost.php index 3d395fb..af865f4 100644 --- a/app/Provisioning/Jobs/PurgeHost.php +++ b/app/Provisioning/Jobs/PurgeHost.php @@ -3,8 +3,9 @@ namespace App\Provisioning\Jobs; use App\Models\Host; -use App\Services\Dns\HetznerDnsClient; use App\Models\VpnPeer; +use App\Services\Dns\HetznerDnsClient; +use App\Services\Dns\HostDnsDirectory; use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Bus\Dispatchable; @@ -60,6 +61,12 @@ class PurgeHost implements ShouldQueue // delete the row that held the id. $host->refresh(); + // Legacy: a host onboarded before RegisterHostDns moved off the public + // Hetzner zone may still carry a public record id here — remove it so + // a deleted host does not leave its (already-published) management + // address behind. Hosts onboarded since never set this column; see + // clupilot:prune-host-dns for the ones already published elsewhere. + // // The record identifier lives on the row about to be deleted, so this // has to happen first — otherwise the host's management address stays // published with nothing left to clean it up. @@ -73,6 +80,13 @@ class PurgeHost implements ShouldQueue app(HetznerDnsClient::class)->deleteRecord($host->dns_record_id); } + // The internal hostsdir entry (vpn-dns) — same reasoning as above, + // for the name every host has actually been getting since. The name + // lives on the row about to be deleted, so it has to come out first. + if (filled($host->dns_name)) { + app(HostDnsDirectory::class)->remove($host->dns_name); + } + if (filled($host->wg_pubkey)) { // Tombstone the console's view of this peer as well. The FK only // nulls host_id, which would leave an enabled-but-absent row behind: diff --git a/app/Provisioning/Steps/Host/RegisterHostDns.php b/app/Provisioning/Steps/Host/RegisterHostDns.php index 50a82b3..1b844c2 100644 --- a/app/Provisioning/Steps/Host/RegisterHostDns.php +++ b/app/Provisioning/Steps/Host/RegisterHostDns.php @@ -5,25 +5,28 @@ namespace App\Provisioning\Steps\Host; use App\Models\Host; use App\Models\ProvisioningRun; use App\Provisioning\StepResult; -use App\Services\Dns\HetznerDnsClient; +use App\Services\Dns\HostDnsDirectory; use App\Support\Settings; use Illuminate\Support\Facades\Cache; use Throwable; /** - * Gives the host a name under : fsn-01.node.clupilot.com. + * Gives the host a name: fsn-01.node.clupilot.com. * - * The record points at the host's WireGuard address, NOT its public IP. The - * name is for us — so an operator can ssh fsn-01.node.… instead of memorising - * addresses — and publishing a Proxmox host's public address would hand every - * scanner a target, which is the one thing the whole network design avoids. + * The entry points at the host's WireGuard address, NOT its public IP, and it + * is written to the vpn-dns container's --hostsdir — resolvable only inside + * the tunnel — rather than the public Hetzner zone. The name is for us, so an + * operator can ssh fsn-01.node.… instead of memorising addresses; publishing + * it in PUBLIC DNS would hand every scanner the internal subnet and roughly + * how many hosts sit behind it, which is the one thing the whole network + * design avoids. See App\Services\Dns\HostDnsDirectory. * * DNS is convenience, not a prerequisite for a working host: a failure here * logs and moves on rather than stranding an otherwise finished onboarding. */ class RegisterHostDns extends HostStep { - public function __construct(private HetznerDnsClient $dns) {} + public function __construct(private HostDnsDirectory $dns) {} public function key(): string { @@ -45,12 +48,12 @@ class RegisterHostDns extends HostStep $fqdn = $name.'.node.'.config('provisioning.dns.zone', 'clupilot.com'); try { - $recordId = $this->dns->upsertRecord($fqdn, 'A', $host->wg_ip); + $this->dns->write($name, $fqdn, $host->wg_ip); } catch (Throwable $e) { - // Retried first, because the failure may be a lost response to a - // request that DID create the record: advancing then leaves a - // published address with no id, so PurgeHost could never remove it. - // The upsert is idempotent, so a retry recovers the id. + // Retried first: the write is idempotent (overwrite-by-name), so a + // retry after a transient failure (e.g. the shared volume briefly + // unavailable) just rewrites the same entry rather than risking a + // half-written file. $allowed = min(3, (int) $run->max_attempts); if ($run->attempt + 1 < $allowed) { return StepResult::retry(30, 'DNS unavailable: '.$e->getMessage()); @@ -67,8 +70,6 @@ class RegisterHostDns extends HostStep return StepResult::advance(); } - $host->update(['dns_record_id' => $recordId]); - return StepResult::advance(); } diff --git a/app/Services/Dns/FakeHostDnsDirectory.php b/app/Services/Dns/FakeHostDnsDirectory.php new file mode 100644 index 0000000..4bef50e --- /dev/null +++ b/app/Services/Dns/FakeHostDnsDirectory.php @@ -0,0 +1,37 @@ + name => ip */ + public array $ips = []; + + /** @var array name => fqdn */ + public array $fqdns = []; + + public bool $failWrite = false; + + public bool $failRemove = false; + + public function write(string $name, string $fqdn, string $ip): void + { + if ($this->failWrite) { + throw new RuntimeException('dns-hosts volume unavailable'); + } + + $this->ips[$name] = $ip; + $this->fqdns[$name] = $fqdn; + } + + public function remove(string $name): void + { + if ($this->failRemove) { + throw new RuntimeException('dns-hosts volume unavailable'); + } + + unset($this->ips[$name], $this->fqdns[$name]); + } +} diff --git a/app/Services/Dns/FileHostDnsDirectory.php b/app/Services/Dns/FileHostDnsDirectory.php new file mode 100644 index 0000000..3105df1 --- /dev/null +++ b/app/Services/Dns/FileHostDnsDirectory.php @@ -0,0 +1,69 @@ + ") per entry, in the directory dnsmasq's + * --hostsdir watches via inotify (see vpn-dns in docker-compose.yml). No + * restart, no reload call: the file landing on disk IS the publish. + * + * The directory is shared with the app's own containers over a plain Docker + * volume, not SSH — unlike SshTraefikWriter, everything that touches it runs + * on the same Docker host. Left world-writable/readable (0775/0664) because + * whichever container touches it first (queue-provisioning as root in + * production, www-data under `app`) must not lock the other out. + */ +class FileHostDnsDirectory implements HostDnsDirectory +{ + public function write(string $name, string $fqdn, string $ip): void + { + $this->ensureDir(); + + $path = $this->path($name); + + if (@file_put_contents($path, "{$ip} {$fqdn}\n") === false) { + throw new RuntimeException("Could not write DNS hosts entry: {$path}"); + } + + @chmod($path, 0664); + } + + public function remove(string $name): void + { + $path = $this->path($name); + + // Nothing to do is not a failure — a host whose name was never + // written (or already removed) still has to purge cleanly. + if (is_file($path) && ! @unlink($path)) { + throw new RuntimeException("Could not remove DNS hosts entry: {$path}"); + } + } + + private function ensureDir(): void + { + $dir = $this->dir(); + + if (is_dir($dir)) { + return; + } + + if (! @mkdir($dir, 0775, true) && ! is_dir($dir)) { + throw new RuntimeException("Could not create DNS hosts directory: {$dir}"); + } + + @chmod($dir, 0775); + } + + private function path(string $name): string + { + return rtrim($this->dir(), '/')."/{$name}.hosts"; + } + + private function dir(): string + { + return (string) config('provisioning.dns.hosts_dir'); + } +} diff --git a/app/Services/Dns/HostDnsDirectory.php b/app/Services/Dns/HostDnsDirectory.php new file mode 100644 index 0000000..53664d6 --- /dev/null +++ b/app/Services/Dns/HostDnsDirectory.php @@ -0,0 +1,17 @@ + 'hetzner', 'token' => env('HETZNER_DNS_TOKEN', ''), 'zone' => env('CLUPILOT_DNS_ZONE', 'clupilot.com'), + + // Internal, WireGuard-only host names (fsn-01.node.…) — the vpn-dns + // container's dnsmasq --hostsdir. NOT the public Hetzner zone above: + // a host's tunnel address must never be resolvable outside the tunnel, + // see RegisterHostDns. Shared with vpn-dns via the dns-hosts volume. + 'hosts_dir' => env('CLUPILOT_DNS_HOSTS_DIR', '/etc/clupilot/dns-hosts'), ], 'traefik' => [ diff --git a/docker-compose.yml b/docker-compose.yml index 83f3c52..37fecc3 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -14,6 +14,10 @@ services: restart: unless-stopped volumes: - .:/var/www/html + # RegisterHostDns/PurgeHost write here (see queue-provisioning below, + # which is where those actually run); mounted here too so an ad hoc + # `artisan` in this container sees and can touch the same entries. + - dns-hosts:/etc/clupilot/dns-hosts ports: # Loopback by DEFAULT. Docker publishes ports ahead of UFW, so a backend # published on 0.0.0.0 is reachable from the internet even with the @@ -81,6 +85,11 @@ services: volumes: - .:/var/www/html - wireguard:/etc/wireguard + # RegisterHostDns writes one file per host here (name -> wg_ip); + # PurgeHost removes it on host deletion. vpn-dns mounts the same volume + # read-only and watches it (dnsmasq --hostsdir) — this is HOW a host + # gets a name now, since it can no longer go in public DNS. + - dns-hosts:/etc/clupilot/dns-hosts depends_on: - app - redis @@ -106,11 +115,17 @@ services: network_mode: "service:queue-provisioning" cap_add: - NET_ADMIN + volumes: + # Host names (fsn-01.node.… -> wg_ip), written by RegisterHostDns onto + # the same volume — see queue-provisioning above. Read-only here: + # dnsmasq only ever reads this directory, never writes it. + - dns-hosts:/etc/clupilot/dns-hosts:ro command: >- --keep-in-foreground --log-facility=- --listen-address=${CLUPILOT_WG_HUB_ADDRESS:-10.66.0.1} --bind-interfaces --no-resolv --server=1.1.1.1 --server=9.9.9.9 --address=/${VPN_INTERNAL_HOST:-admin.invalid}/${CLUPILOT_WG_HUB_ADDRESS:-10.66.0.1} + --hostsdir=/etc/clupilot/dns-hosts depends_on: - queue-provisioning @@ -219,3 +234,4 @@ volumes: runner-data: redis-data: wireguard: + dns-hosts: diff --git a/tests/Feature/Provisioning/HostStepsTest.php b/tests/Feature/Provisioning/HostStepsTest.php index d5d1a6a..025c164 100644 --- a/tests/Feature/Provisioning/HostStepsTest.php +++ b/tests/Feature/Provisioning/HostStepsTest.php @@ -1,8 +1,10 @@ active()->create([ + $host = Host::factory()->active()->create([ 'datacenter' => 'fsn', 'wg_ip' => '10.66.0.11', 'public_ip' => '203.0.113.11', 'dns_name' => null, ]); - $run = App\Models\ProvisioningRun::factory()->forHost($host)->create(["pipeline" => "host"]); + $run = ProvisioningRun::factory()->forHost($host)->create(['pipeline' => 'host']); - $result = (new App\Provisioning\Steps\Host\RegisterHostDns($s['dns']))->execute($run); + $result = (new RegisterHostDns($s['hostDns']))->execute($run); - expect($result->type)->toBe("advance") + expect($result->type)->toBe('advance') ->and($host->fresh()->dns_name)->toBe('fsn-01'); // The management address: publishing a Proxmox host's public IP would hand - // every scanner a target. - expect($s['dns']->values['fsn-01.node.clupilot.com'])->toBe('10.66.0.11') - ->and($s['dns']->values['fsn-01.node.clupilot.com'])->not->toBe('203.0.113.11'); + // every scanner a target. Written to the internal hostsdir, not upserted + // anywhere via the (public) Hetzner client — see the mutation test below. + expect($s['hostDns']->fqdns['fsn-01'])->toBe('fsn-01.node.clupilot.com') + ->and($s['hostDns']->ips['fsn-01'])->toBe('10.66.0.11') + ->and($s['hostDns']->ips['fsn-01'])->not->toBe('203.0.113.11'); +}); + +it('never reaches the public Hetzner DNS client for a host name', function () { + // Mutation boundary: if RegisterHostDns is ever changed to also (or + // instead) call the public client, this must fail. Verified by hand — + // temporarily adding a `$this->publicDns->upsertRecord(...)` call to the + // step and confirming this test goes red before reverting it. + $s = fakeServices(); + $host = Host::factory()->active()->create([ + 'datacenter' => 'fsn', 'wg_ip' => '10.66.0.12', 'dns_name' => null, + ]); + $run = ProvisioningRun::factory()->forHost($host)->create(['pipeline' => 'host']); + + (new RegisterHostDns($s['hostDns']))->execute($run); + + expect($s['dns']->records)->toBe([]) + ->and($s['dns']->values)->toBe([]); }); it('numbers hosts per datacenter and never reuses a number', function () { @@ -266,11 +288,11 @@ it('numbers hosts per datacenter and never reuses a number', function () { $names = []; foreach (['fsn', 'fsn', 'hel'] as $i => $dc) { - $host = App\Models\Host::factory()->active()->create([ + $host = Host::factory()->active()->create([ 'datacenter' => $dc, 'wg_ip' => '10.66.0.'.(20 + $i), 'dns_name' => null, ]); - $run = App\Models\ProvisioningRun::factory()->forHost($host)->create(["pipeline" => "host"]); - (new App\Provisioning\Steps\Host\RegisterHostDns($s['dns']))->execute($run); + $run = ProvisioningRun::factory()->forHost($host)->create(['pipeline' => 'host']); + (new RegisterHostDns($s['hostDns']))->execute($run); $names[] = $host->fresh()->dns_name; } @@ -279,21 +301,21 @@ it('numbers hosts per datacenter and never reuses a number', function () { // Removing the HIGHEST one is the dangerous case: deriving the number from // live hosts would hand fsn-02 straight back out, and a cached name would // then point at a different machine. - App\Models\Host::query()->where('dns_name', 'fsn-02')->delete(); - $host = App\Models\Host::factory()->active()->create(['datacenter' => 'fsn', 'wg_ip' => '10.66.0.30', 'dns_name' => null]); - $run = App\Models\ProvisioningRun::factory()->forHost($host)->create(["pipeline" => "host"]); - (new App\Provisioning\Steps\Host\RegisterHostDns($s['dns']))->execute($run); + Host::query()->where('dns_name', 'fsn-02')->delete(); + $host = Host::factory()->active()->create(['datacenter' => 'fsn', 'wg_ip' => '10.66.0.30', 'dns_name' => null]); + $run = ProvisioningRun::factory()->forHost($host)->create(['pipeline' => 'host']); + (new RegisterHostDns($s['hostDns']))->execute($run); expect($host->fresh()->dns_name)->toBe('fsn-03'); }); it('retries a failed DNS registration before giving up on the name', function () { $s = fakeServices(); - $s['dns']->failUpsert = true; - $host = App\Models\Host::factory()->active()->create(['datacenter' => 'fsn', 'wg_ip' => '10.66.0.40', 'dns_name' => null]); - $run = App\Models\ProvisioningRun::factory()->forHost($host)->create(['pipeline' => 'host', 'attempt' => 0]); + $s['hostDns']->failWrite = true; + $host = Host::factory()->active()->create(['datacenter' => 'fsn', 'wg_ip' => '10.66.0.40', 'dns_name' => null]); + $run = ProvisioningRun::factory()->forHost($host)->create(['pipeline' => 'host', 'attempt' => 0]); - $step = new App\Provisioning\Steps\Host\RegisterHostDns($s['dns']); + $step = new RegisterHostDns($s['hostDns']); // The failure may be a lost response to a request that did create the // record — advancing straight away would orphan it. @@ -305,64 +327,82 @@ it('retries a failed DNS registration before giving up on the name', function () ->and($run->events()->where('step', 'register_host_dns')->exists())->toBeTrue(); }); -it('takes the host DNS record with it when the host is purged', function () { +it("takes the host's internal DNS entry with it when the host is purged", function () { $s = fakeServices(); - app()->instance(App\Services\Dns\HetznerDnsClient::class, $s['dns']); - $host = App\Models\Host::factory()->active()->create([ + $host = Host::factory()->active()->create([ 'datacenter' => 'fsn', 'wg_ip' => '10.66.0.50', 'dns_name' => null, ]); - $run = App\Models\ProvisioningRun::factory()->forHost($host)->create(['pipeline' => 'host']); - (new App\Provisioning\Steps\Host\RegisterHostDns($s['dns']))->execute($run); + $run = ProvisioningRun::factory()->forHost($host)->create(['pipeline' => 'host']); + (new RegisterHostDns($s['hostDns']))->execute($run); - $fqdn = $host->fresh()->dns_name.'.node.clupilot.com'; - expect($s['dns']->records)->toHaveKey($fqdn); + $name = $host->fresh()->dns_name; + expect($s['hostDns']->ips)->toHaveKey($name); - (new App\Provisioning\Jobs\PurgeHost($host->uuid))->handle(); + (new PurgeHost($host->uuid))->handle(); - // Otherwise the machine's management address stays published for good, with - // the identifier needed to remove it deleted along with the row. - expect($s['dns']->records)->not->toHaveKey($fqdn); + // Otherwise the machine's management address stays in the hostsdir for + // good, with the name needed to remove it deleted along with the row. + expect($s['hostDns']->ips)->not->toHaveKey($name); }); -it('keeps the host until its DNS record is really gone', function () { +it('keeps the host until its internal DNS entry is really gone', function () { $s = fakeServices(); - app()->instance(App\Services\Dns\HetznerDnsClient::class, $s['dns']); - $host = App\Models\Host::factory()->active()->create([ + $host = Host::factory()->active()->create([ 'datacenter' => 'hel', 'wg_ip' => '10.66.0.60', 'dns_name' => null, ]); - $run = App\Models\ProvisioningRun::factory()->forHost($host)->create(['pipeline' => 'host']); - (new App\Provisioning\Steps\Host\RegisterHostDns($s['dns']))->execute($run); + $run = ProvisioningRun::factory()->forHost($host)->create(['pipeline' => 'host']); + (new RegisterHostDns($s['hostDns']))->execute($run); - $s['dns']->failDelete = true; + $s['hostDns']->failRemove = true; - // The row carries the only reference to that record — deleting it anyway - // would publish the machine's management address for good. - expect(fn () => (new App\Provisioning\Jobs\PurgeHost($host->uuid))->handle()) + // The row carries the only name that maps to that entry — deleting it + // anyway would leave the machine's management address in the hostsdir + // for good. + expect(fn () => (new PurgeHost($host->uuid))->handle()) ->toThrow(RuntimeException::class); - expect(App\Models\Host::query()->whereKey($host->id)->exists())->toBeTrue(); + expect(Host::query()->whereKey($host->id)->exists())->toBeTrue(); - // Once DNS is back, the purge completes. - $s['dns']->failDelete = false; - (new App\Provisioning\Jobs\PurgeHost($host->uuid))->handle(); + // Once the hostsdir write is back, the purge completes. + $s['hostDns']->failRemove = false; + (new PurgeHost($host->uuid))->handle(); - expect(App\Models\Host::query()->whereKey($host->id)->exists())->toBeFalse() - ->and($s['dns']->records)->toBe([]); + expect(Host::query()->whereKey($host->id)->exists())->toBeFalse() + ->and($s['hostDns']->ips)->toBe([]); +}); + +it('still removes a legacy public Hetzner record when a pre-fix host is purged', function () { + // Hosts onboarded before this change may still carry a public record id. + // PurgeHost has always cleaned that up on removal — this keeps working + // for those, independent of clupilot:prune-host-dns, which only handles + // hosts that are NOT being deleted. + $s = fakeServices(); + $recordId = $s['dns']->upsertRecord('fsn-legacy.node.clupilot.com', 'A', '10.66.0.55'); + $host = Host::factory()->active()->create([ + 'datacenter' => 'fsn', 'wg_ip' => '10.66.0.55', + 'dns_name' => 'fsn-legacy', 'dns_record_id' => $recordId, + ]); + + expect($s['dns']->records)->toHaveKey('fsn-legacy.node.clupilot.com'); + + (new PurgeHost($host->uuid))->handle(); + + expect($s['dns']->records)->not->toHaveKey('fsn-legacy.node.clupilot.com'); }); it('builds a valid DNS label even from an awkward datacenter code', function () { $s = fakeServices(); // Inserted directly: the console now rejects this shape, but rows created // before the rule was tightened still exist. - App\Models\Datacenter::factory()->create(['code' => 'eu_west', 'name' => 'EU West']); - $host = App\Models\Host::factory()->active()->create([ + Datacenter::factory()->create(['code' => 'eu_west', 'name' => 'EU West']); + $host = Host::factory()->active()->create([ 'datacenter' => 'eu_west', 'wg_ip' => '10.66.0.70', 'dns_name' => null, ]); - $run = App\Models\ProvisioningRun::factory()->forHost($host)->create(['pipeline' => 'host']); + $run = ProvisioningRun::factory()->forHost($host)->create(['pipeline' => 'host']); - (new App\Provisioning\Steps\Host\RegisterHostDns($s['dns']))->execute($run); + (new RegisterHostDns($s['hostDns']))->execute($run); $name = $host->fresh()->dns_name; expect($name)->toBe('eu-west-01') @@ -371,16 +411,16 @@ it('builds a valid DNS label even from an awkward datacenter code', function () it('does not hand the same name to two datacenter codes that look alike', function () { $s = fakeServices(); - App\Models\Datacenter::factory()->create(['code' => 'eu_west', 'name' => 'EU West (alt)']); - App\Models\Datacenter::factory()->create(['code' => 'eu-west', 'name' => 'EU West']); + Datacenter::factory()->create(['code' => 'eu_west', 'name' => 'EU West (alt)']); + Datacenter::factory()->create(['code' => 'eu-west', 'name' => 'EU West']); $names = []; foreach (['eu_west', 'eu-west'] as $i => $dc) { - $host = App\Models\Host::factory()->active()->create([ + $host = Host::factory()->active()->create([ 'datacenter' => $dc, 'wg_ip' => '10.66.0.'.(80 + $i), 'dns_name' => null, ]); - $run = App\Models\ProvisioningRun::factory()->forHost($host)->create(['pipeline' => 'host']); - (new App\Provisioning\Steps\Host\RegisterHostDns($s['dns']))->execute($run); + $run = ProvisioningRun::factory()->forHost($host)->create(['pipeline' => 'host']); + (new RegisterHostDns($s['hostDns']))->execute($run); $names[] = $host->fresh()->dns_name; } @@ -389,23 +429,22 @@ it('does not hand the same name to two datacenter codes that look alike', functi expect($names)->toBe(['eu-west-01', 'eu-west-02']); }); -it('sees a DNS record registered while the purge was waiting', function () { +it('sees an internal DNS entry registered while the purge was waiting', function () { $s = fakeServices(); - app()->instance(App\Services\Dns\HetznerDnsClient::class, $s['dns']); - $host = App\Models\Host::factory()->active()->create([ + $host = Host::factory()->active()->create([ 'datacenter' => 'fsn', 'wg_ip' => '10.66.0.90', 'dns_name' => null, ]); - $purge = new App\Provisioning\Jobs\PurgeHost($host->uuid); + $purge = new PurgeHost($host->uuid); // Registration finishes while the purge is blocked on the run lock: the - // copy it loaded first still says there is no record to delete. - $run = App\Models\ProvisioningRun::factory()->forHost($host)->create(['pipeline' => 'host']); - (new App\Provisioning\Steps\Host\RegisterHostDns($s['dns']))->execute($run); - $fqdn = $host->fresh()->dns_name.'.node.clupilot.com'; + // copy it loaded first still says there is no entry to remove. + $run = ProvisioningRun::factory()->forHost($host)->create(['pipeline' => 'host']); + (new RegisterHostDns($s['hostDns']))->execute($run); + $name = $host->fresh()->dns_name; $purge->handle(); - expect($s['dns']->records)->not->toHaveKey($fqdn) - ->and(App\Models\Host::query()->whereKey($host->id)->exists())->toBeFalse(); + expect($s['hostDns']->ips)->not->toHaveKey($name) + ->and(Host::query()->whereKey($host->id)->exists())->toBeFalse(); }); diff --git a/tests/Feature/Provisioning/PruneHostDnsTest.php b/tests/Feature/Provisioning/PruneHostDnsTest.php new file mode 100644 index 0000000..8d1238d --- /dev/null +++ b/tests/Feature/Provisioning/PruneHostDnsTest.php @@ -0,0 +1,47 @@ +active()->create(['dns_record_id' => null]); + + $this->artisan('clupilot:prune-host-dns') + ->assertSuccessful() + ->expectsOutputToContain('Nothing to do'); +}); + +it('without --force only reports what it would delete, and changes nothing', function () { + $s = fakeServices(); + $recordId = $s['dns']->upsertRecord('fsn-legacy.node.clupilot.com', 'A', '10.66.0.55'); + $host = Host::factory()->active()->create(['dns_record_id' => $recordId]); + + $this->artisan('clupilot:prune-host-dns')->assertSuccessful(); + + expect($host->fresh()->dns_record_id)->toBe($recordId) + ->and($s['dns']->records)->toHaveKey('fsn-legacy.node.clupilot.com'); +}); + +it('with --force deletes the record and clears the column', function () { + $s = fakeServices(); + $recordId = $s['dns']->upsertRecord('fsn-legacy.node.clupilot.com', 'A', '10.66.0.55'); + $host = Host::factory()->active()->create(['dns_record_id' => $recordId]); + + $this->artisan('clupilot:prune-host-dns --force')->assertSuccessful(); + + expect($host->fresh()->dns_record_id)->toBeNull() + ->and($s['dns']->records)->not->toHaveKey('fsn-legacy.node.clupilot.com'); +}); + +it('with --force leaves the id in place when the provider refuses the deletion', function () { + $s = fakeServices(); + $recordId = $s['dns']->upsertRecord('fsn-legacy.node.clupilot.com', 'A', '10.66.0.55'); + $host = Host::factory()->active()->create(['dns_record_id' => $recordId]); + $s['dns']->failDelete = true; + + // A failed provider call must not silently forget which record still + // needs cleaning up — nulling the column here would do exactly that. + $this->artisan('clupilot:prune-host-dns --force')->assertFailed(); + + expect($host->fresh()->dns_record_id)->toBe($recordId); +}); diff --git a/tests/Feature/Provisioning/ServicesTest.php b/tests/Feature/Provisioning/ServicesTest.php index 70d8883..dc18551 100644 --- a/tests/Feature/Provisioning/ServicesTest.php +++ b/tests/Feature/Provisioning/ServicesTest.php @@ -1,11 +1,16 @@ addPeer('PK', '10.66.0.9'); - (new App\Provisioning\Jobs\RemoveWireguardPeer('PK'))->handle($hub); + (new RemoveWireguardPeer('PK'))->handle($hub); expect($hub->peerIps())->toBe([]); }); @@ -105,11 +110,44 @@ it('reports node capacity for a host (FakeProxmoxClient)', function () { }); it('treats an already-deleted DNS record as done', function () { - Illuminate\Support\Facades\Http::fake([ - '*' => Illuminate\Support\Facades\Http::response(['error' => 'not found'], 404), + Http::fake([ + '*' => Http::response(['error' => 'not found'], 404), ]); // A lost response, or a retry after a later step failed, must not make the // record impossible to clean up — and its host impossible to purge. - (new App\Services\Dns\HttpHetznerDnsClient)->deleteRecord('rec-123'); + (new HttpHetznerDnsClient)->deleteRecord('rec-123'); +})->throwsNoExceptions(); + +it('writes and removes a real hostsdir entry on disk (FileHostDnsDirectory)', function () { + $dir = sys_get_temp_dir().'/clupilot-dns-hosts-test-'.uniqid(); + config()->set('provisioning.dns.hosts_dir', $dir); + $writer = new FileHostDnsDirectory; + + // The directory itself is created on first write — dnsmasq --hostsdir + // only needs it to exist, not to be pre-provisioned by anything else. + $writer->write('fsn-01', 'fsn-01.node.clupilot.com', '10.66.0.11'); + $path = $dir.'/fsn-01.hosts'; + + expect(is_file($path))->toBeTrue() + ->and(trim(file_get_contents($path)))->toBe('10.66.0.11 fsn-01.node.clupilot.com'); + + // Overwrite is the update path — RegisterHostDns calls write() again on + // retry with the same name, and that must replace, not append. + $writer->write('fsn-01', 'fsn-01.node.clupilot.com', '10.66.0.99'); + expect(trim(file_get_contents($path)))->toBe('10.66.0.99 fsn-01.node.clupilot.com'); + + $writer->remove('fsn-01'); + expect(is_file($path))->toBeFalse(); + + File::deleteDirectory($dir); +}); + +it('treats removing a name that was never written as done (FileHostDnsDirectory)', function () { + $dir = sys_get_temp_dir().'/clupilot-dns-hosts-test-'.uniqid(); + config()->set('provisioning.dns.hosts_dir', $dir); + + // Mirrors PurgeHost's guard (filled($host->dns_name)) not always holding + // — a host that never got a name still has to purge cleanly. + (new FileHostDnsDirectory)->remove('never-written'); })->throwsNoExceptions(); diff --git a/tests/Pest.php b/tests/Pest.php index 6cc5dd3..7db9704 100644 --- a/tests/Pest.php +++ b/tests/Pest.php @@ -3,7 +3,9 @@ use App\Models\Mailbox; use App\Models\Operator; use App\Services\Dns\FakeHetznerDnsClient; +use App\Services\Dns\FakeHostDnsDirectory; use App\Services\Dns\HetznerDnsClient; +use App\Services\Dns\HostDnsDirectory; use App\Services\Mail\MailPurpose; use App\Services\Monitoring\FakeMonitoringClient; use App\Services\Monitoring\MonitoringClient; @@ -69,7 +71,7 @@ function something() * Bind fake provisioning services into the container and return them so a test * can script/inspect SSH, WireGuard, Proxmox, DNS and Traefik interactions. * - * @return array{shell: FakeRemoteShell, hub: FakeWireguardHub, pve: FakeProxmoxClient, dns: FakeHetznerDnsClient, traefik: FakeTraefikWriter} + * @return array{shell: FakeRemoteShell, hub: FakeWireguardHub, pve: FakeProxmoxClient, dns: FakeHetznerDnsClient, hostDns: FakeHostDnsDirectory, traefik: FakeTraefikWriter} */ function fakeServices(): array { @@ -77,6 +79,7 @@ function fakeServices(): array $hub = new FakeWireguardHub; $pve = new FakeProxmoxClient; $dns = new FakeHetznerDnsClient; + $hostDns = new FakeHostDnsDirectory; $traefik = new FakeTraefikWriter; $monitoring = new FakeMonitoringClient; @@ -84,10 +87,11 @@ function fakeServices(): array app()->instance(WireguardHub::class, $hub); app()->instance(ProxmoxClient::class, $pve); app()->instance(HetznerDnsClient::class, $dns); + app()->instance(HostDnsDirectory::class, $hostDns); app()->instance(TraefikWriter::class, $traefik); app()->instance(MonitoringClient::class, $monitoring); - return ['shell' => $shell, 'hub' => $hub, 'pve' => $pve, 'dns' => $dns, 'traefik' => $traefik, 'monitoring' => $monitoring]; + return ['shell' => $shell, 'hub' => $hub, 'pve' => $pve, 'dns' => $dns, 'hostDns' => $hostDns, 'traefik' => $traefik, 'monitoring' => $monitoring]; } /*