From 5cf1964d7d6e65012ccbbc4d78c3ce4729dfb0f0 Mon Sep 17 00:00:00 2001 From: nexxo Date: Sat, 25 Jul 2026 12:39:15 +0200 Subject: [PATCH] fix(engine-b): default-deny firewall, disk-based capacity, synchronous credential mail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - applyFirewall sets policy_in=DROP so only 80/443 are exposed. - Capacity/placement account for disk_gb (the real VM allocation), not the smaller Nextcloud user quota — no systematic overcommit. - CloudReady sent synchronously again; the step retries on mail failure with the password preserved (no lost credential in a failed queue job). Co-Authored-By: Claude Opus 4.8 --- app/Models/Host.php | 9 +++++---- app/Notifications/CloudReady.php | 17 ++++++++--------- .../Steps/Customer/ReserveResources.php | 2 +- app/Services/Proxmox/HttpProxmoxClient.php | 7 ++++++- .../Provisioning/CustomerDataModelTest.php | 14 +++++++------- 5 files changed, 27 insertions(+), 22 deletions(-) diff --git a/app/Models/Host.php b/app/Models/Host.php index 983706a..e5eb059 100644 --- a/app/Models/Host.php +++ b/app/Models/Host.php @@ -63,15 +63,16 @@ class Host extends Model implements ProvisioningSubject } /** - * Storage already committed. A failed instance still counts while its VM - * exists (has a vmid) — the disk is really consumed until cleanup; a failure - * before any VM was created (no vmid) releases its quota. + * Host storage already committed, counted as the VM disk allocation + * (disk_gb) since that is what is actually placed on the host — not the + * (smaller) Nextcloud user quota. A failed instance still counts while its VM + * exists (has a vmid); a failure before any VM was created releases it. */ public function committedGb(): int { return (int) $this->instances() ->where(fn ($q) => $q->where('status', '!=', 'failed')->orWhereNotNull('vmid')) - ->sum('quota_gb'); + ->sum('disk_gb'); } /** Storage still available for new instances. */ diff --git a/app/Notifications/CloudReady.php b/app/Notifications/CloudReady.php index 3d48e24..bd1273d 100644 --- a/app/Notifications/CloudReady.php +++ b/app/Notifications/CloudReady.php @@ -4,22 +4,21 @@ namespace App\Notifications; use App\Models\Instance; use Illuminate\Bus\Queueable; -use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Notifications\Messages\MailMessage; use Illuminate\Notifications\Notification; use Illuminate\Support\Facades\Crypt; /** - * Delivers the initial admin credentials once provisioning completes. Queued for - * durable delivery; the password is passed transiently and never persisted in - * plaintext. + * Delivers the initial admin credentials once provisioning completes. * - * Delivery is at-least-once: a worker crash in the tiny window between sending - * and recording the `credentials_sent` breadcrumb can re-send (a duplicate - * welcome email). Exactly-once would need a transactional outbox (v1.1); a - * duplicate is preferred over locking the customer out with a lost credential. + * Sent SYNCHRONOUSLY from CompleteProvisioning (not queued): the step only + * records the `credentials_sent` breadcrumb and scrubs the password AFTER the + * send returns, so a mail failure re-runs the step (password preserved) instead + * of losing the credential in a failed queue job. Delivery is at-least-once — a + * crash in the tiny window after send but before the breadcrumb can re-send a + * duplicate welcome email, which is preferable to a lost credential. */ -class CloudReady extends Notification implements ShouldQueue +class CloudReady extends Notification { use Queueable; diff --git a/app/Provisioning/Steps/Customer/ReserveResources.php b/app/Provisioning/Steps/Customer/ReserveResources.php index a6d0964..43039d1 100644 --- a/app/Provisioning/Steps/Customer/ReserveResources.php +++ b/app/Provisioning/Steps/Customer/ReserveResources.php @@ -39,7 +39,7 @@ class ReserveResources extends CustomerStep // Place + create under a per-datacenter lock so concurrent reservations // can't both pick the same host and overcommit its capacity. $instance = Cache::lock('placement:'.$order->datacenter, 30)->block(10, function () use ($order, $plan) { - $host = Host::placeableIn($order->datacenter, (int) $plan['quota_gb']); + $host = Host::placeableIn($order->datacenter, (int) $plan['disk_gb']); if ($host === null) { return null; } diff --git a/app/Services/Proxmox/HttpProxmoxClient.php b/app/Services/Proxmox/HttpProxmoxClient.php index 280441e..81e6623 100644 --- a/app/Services/Proxmox/HttpProxmoxClient.php +++ b/app/Services/Proxmox/HttpProxmoxClient.php @@ -131,7 +131,12 @@ class HttpProxmoxClient implements ProxmoxClient foreach ($rules as $rule) { $this->http()->asForm()->post("/nodes/{$node}/qemu/{$vmid}/firewall/rules", $rule)->throw(); } - $this->http()->asForm()->put("/nodes/{$node}/qemu/{$vmid}/firewall/options", ['enable' => 1])->throw(); + // Default-deny inbound so only the explicit ACCEPT rules (80/443) are open. + $this->http()->asForm()->put("/nodes/{$node}/qemu/{$vmid}/firewall/options", [ + 'enable' => 1, + 'policy_in' => 'DROP', + 'policy_out' => 'ACCEPT', + ])->throw(); } public function createBackupJob(string $node, int $vmid, string $schedule): string diff --git a/tests/Feature/Provisioning/CustomerDataModelTest.php b/tests/Feature/Provisioning/CustomerDataModelTest.php index df6ad47..43f8711 100644 --- a/tests/Feature/Provisioning/CustomerDataModelTest.php +++ b/tests/Feature/Provisioning/CustomerDataModelTest.php @@ -53,18 +53,18 @@ it('encrypts nc_admin_ref at rest', function () { it('computes host capacity from committed instance quotas', function () { $host = Host::factory()->active()->create(['total_gb' => 1000, 'reserve_pct' => 15]); // freeGb = 850 - Instance::factory()->create(['host_id' => $host->id, 'quota_gb' => 100, 'status' => 'active']); - Instance::factory()->create(['host_id' => $host->id, 'quota_gb' => 200, 'status' => 'provisioning']); - Instance::factory()->create(['host_id' => $host->id, 'quota_gb' => 500, 'status' => 'failed', 'vmid' => null]); // released (no VM) - Instance::factory()->create(['host_id' => $host->id, 'quota_gb' => 150, 'status' => 'failed', 'vmid' => 900]); // VM still exists → counts + Instance::factory()->create(['host_id' => $host->id, 'disk_gb' => 120, 'status' => 'active']); + Instance::factory()->create(['host_id' => $host->id, 'disk_gb' => 220, 'status' => 'provisioning']); + Instance::factory()->create(['host_id' => $host->id, 'disk_gb' => 500, 'status' => 'failed', 'vmid' => null]); // released (no VM) + Instance::factory()->create(['host_id' => $host->id, 'disk_gb' => 160, 'status' => 'failed', 'vmid' => 900]); // VM still exists → counts - expect($host->committedGb())->toBe(450) // 100 + 200 + 150 - ->and($host->availableGb())->toBe(400); // 850 - 450 + expect($host->committedGb())->toBe(500) // 120 + 220 + 160 (disk allocations) + ->and($host->availableGb())->toBe(350); // 850 - 500 }); it('places an instance on an active host in the datacenter with room', function () { $full = Host::factory()->active()->create(['datacenter' => 'fsn', 'name' => 'pve-fsn-a', 'total_gb' => 100, 'reserve_pct' => 15]); - Instance::factory()->create(['host_id' => $full->id, 'quota_gb' => 80, 'status' => 'active']); + Instance::factory()->create(['host_id' => $full->id, 'disk_gb' => 80, 'status' => 'active']); // free 85, committed 80 → 5 left $room = Host::factory()->active()->create(['datacenter' => 'fsn', 'name' => 'pve-fsn-b', 'total_gb' => 1000, 'reserve_pct' => 15]); Host::factory()->active()->create(['datacenter' => 'hel', 'name' => 'pve-hel-a', 'total_gb' => 1000]);