From 3930cf12ae11478ec5d47b9c02ce8e3ae99f38e3 Mon Sep 17 00:00:00 2001 From: nexxo Date: Mon, 3 Aug 2026 20:06:34 +0200 Subject: [PATCH] Mailversand darf eine bezahlte Bereitstellung nie aufhalten MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ConfigureInstanceMail stand als Pflichtschritt in der customer-Pipeline und liess den ganzen Lauf scheitern, wenn Server oder Postfach fehlten. Der Mailserver dieses Produkts existiert noch nicht — er wird gerade erst aufgesetzt. Jede neue bezahlte Bestellung waere damit an einer Nebenfunktion haengengeblieben: der Kunde zahlt, die Cloud kommt nicht. Denselben Fall hat dieses Projekt bei RegisterMonitoring schon richtig entschieden: eine nicht erreichbare Ueberwachung blockiert die Bereitstellung nie. Mailversand bekommt jetzt dieselbe Behandlung. Fehlt Server oder Postfach, geht der Schritt mit advance() weiter und protokolliert den Grund per Log::warning (Instanz-UUID + Grund) fuer den Betreiber; nachgeholt wird es ueber clupilot:configure-instance-mail (Aufgabe 4), sobald beides steht. Nicht eingerichtet bleibt etwas anderes als kaputt: sobald Server und Postfach da sind, wird geschrieben wie zuvor, und ein echter Schreibfehler (occ-Fehlercode, Gast antwortet nicht) bleibt weiterhin ein Fehlschlag ueber CustomerStep::guest(). Der bestehende Test "schreibt GAR NICHTS, wenn der Mailserver fehlt" prueft jetzt StepResult::ADVANCE statt FAIL und zusaetzlich, dass der Grund geloggt wird. Ein neuer End-to-End-Test in CustomerProvisioningEndToEndTest faehrt eine bezahlte Bestellung ohne jede Mail-Fixture vollstaendig bis "completed" — die Zusicherung, um die es hier eigentlich geht. Co-Authored-By: Claude Opus 5 --- .../Steps/Customer/ConfigureInstanceMail.php | 34 ++++++++-- .../ConfigureInstanceMailTest.php | 17 ++++- .../CustomerProvisioningEndToEndTest.php | 65 +++++++++++++++++-- 3 files changed, 102 insertions(+), 14 deletions(-) diff --git a/app/Provisioning/Steps/Customer/ConfigureInstanceMail.php b/app/Provisioning/Steps/Customer/ConfigureInstanceMail.php index d6f3aa9..75ac205 100644 --- a/app/Provisioning/Steps/Customer/ConfigureInstanceMail.php +++ b/app/Provisioning/Steps/Customer/ConfigureInstanceMail.php @@ -7,6 +7,7 @@ use App\Provisioning\StepResult; use App\Services\Mail\GuestMailConfig; use App\Services\Proxmox\ProxmoxClient; use App\Support\NextcloudOcc; +use Illuminate\Support\Facades\Log; /** * Bringt der Nextcloud eines Kunden bei, Mail zu verschicken. @@ -20,9 +21,26 @@ use App\Support\NextcloudOcc; * einen Mitarbeiter verschickt NEXTCLOUD, nicht CluPilot — nur so entsteht * das Passwort dort, wo niemand sonst es zu sehen bekommt. * - * Ohne Server oder Absenderpostfach wird NICHTS geschrieben und der Schritt - * scheitert mit einem Grund. Eine halb eingetragene Mailkonfiguration waere - * schlimmer als keine. + * Ohne Server oder Absenderpostfach wird NICHTS geschrieben. Eine halb + * eingetragene Mailkonfiguration waere schlimmer als keine. + * + * Das ist aber KEIN Grund, den Lauf scheitern zu lassen: dieser Schritt steht + * in der PFLICHT-Pipeline `customer`, und ein Kunde, der bezahlt hat, bekommt + * seine Cloud auch dann, wenn der Mailversand noch nicht eingerichtet ist — + * genau wie RegisterMonitoring die Bereitstellung nie an einer nicht + * erreichbaren Ueberwachung scheitern laesst (siehe deren Kopfkommentar). + * Mailversand ist hier dieselbe Nebenfunktion: fehlt er, funktioniert die + * Cloud vollstaendig, nur Einladungen und Freigabe-Benachrichtigungen gehen + * nicht. Der Schritt geht in diesem Fall mit `advance()` weiter und + * protokolliert den Grund per `Log::warning`, damit der Betreiber es findet; + * nachgeholt wird es ueber `clupilot:configure-instance-mail` (Aufgabe 4), + * sobald Server und Postfach stehen. + * + * Nicht eingerichtet ist etwas anderes als kaputt: sobald Server und Postfach + * da sind, wird geschrieben, und ein Fehler beim Schreiben selbst (occ liefert + * einen Fehlercode, der Gast antwortet nicht) bleibt ein Fehlschlag wie jeder + * andere Gastbefehl in dieser Pipeline — CustomerStep::guest() wirft dafuer, + * und der Runner macht daraus einen Retry. * * WO DAS SMTP-PASSWORT UEBERALL SICHTBAR IST — ehrlich aufgeschrieben, weil * ein erster Entwurf es zu verstecken versuchte und das Verstecken nichts @@ -71,7 +89,15 @@ class ConfigureInstanceMail extends CustomerStep $config = GuestMailConfig::for($instance); if (! $config->available()) { - return StepResult::fail($config->problem() ?? 'mail_unavailable'); + // Nicht eingerichtet, nicht kaputt — siehe Kopfkommentar. Die Cloud + // wird trotzdem ausgeliefert; der Grund geht ins Log, nicht in + // einen gescheiterten Lauf. + Log::warning('Mailversand fuer Instanz uebersprungen (nicht eingerichtet)', [ + 'instance' => $instance->uuid, + 'reason' => $config->problem() ?? 'mail_unavailable', + ]); + + return StepResult::advance(); } $pve = $this->pve->forHost($instance->host); diff --git a/tests/Feature/Provisioning/ConfigureInstanceMailTest.php b/tests/Feature/Provisioning/ConfigureInstanceMailTest.php index 6825eb9..3caa871 100644 --- a/tests/Feature/Provisioning/ConfigureInstanceMailTest.php +++ b/tests/Feature/Provisioning/ConfigureInstanceMailTest.php @@ -8,6 +8,7 @@ use App\Provisioning\Steps\Customer\ConfigureInstanceMail; use App\Services\Proxmox\FakeProxmoxClient; use App\Services\Proxmox\ProxmoxClient; use App\Support\Settings; +use Illuminate\Support\Facades\Log; function laufMitInstanz(Instance $instance): ProvisioningRun { @@ -89,15 +90,25 @@ it('maskiert das Passwort, sodass es keinen zweiten Befehl starten kann', functi }); it('schreibt GAR NICHTS, wenn der Mailserver fehlt', function () { + // Nicht eingerichtet ist etwas anderes als kaputt: dieser Schritt sitzt in + // der PFLICHT-Pipeline `customer`, und ein bezahlter Kunde bekommt seine + // Cloud auch dann, wenn der Mailversand noch nicht steht — derselbe + // Grundsatz wie bei RegisterMonitoring. Der Lauf geht deshalb WEITER + // (advance), nicht in einen Fehlschlag; nur in den Gast wird nichts + // geschrieben, und der Grund landet im Protokoll fuer den Betreiber. + Log::spy(); Settings::set('mail.host', ''); $pve = new FakeProxmoxClient; app()->instance(ProxmoxClient::class, $pve); + $instance = Instance::factory()->create(); - $ergebnis = app(ConfigureInstanceMail::class)->execute(laufMitInstanz(Instance::factory()->create())); + $ergebnis = app(ConfigureInstanceMail::class)->execute(laufMitInstanz($instance)); expect($pve->guestCommands)->toBe([]) - ->and($ergebnis->type)->toBe(\App\Provisioning\StepResult::FAIL) - ->and($ergebnis->reason)->toBe('no_server'); + ->and($ergebnis->type)->toBe(\App\Provisioning\StepResult::ADVANCE); + Log::shouldHaveReceived('warning')->once()->withArgs( + fn ($message, $context) => $context['instance'] === $instance->uuid && $context['reason'] === 'no_server' + ); }); it('schreibt beim zweiten Lauf erneut, statt sich mit einem Merker zu sperren', function () { diff --git a/tests/Feature/Provisioning/CustomerProvisioningEndToEndTest.php b/tests/Feature/Provisioning/CustomerProvisioningEndToEndTest.php index 2232296..e32aa83 100644 --- a/tests/Feature/Provisioning/CustomerProvisioningEndToEndTest.php +++ b/tests/Feature/Provisioning/CustomerProvisioningEndToEndTest.php @@ -22,12 +22,12 @@ it('provisions a paid order all the way to active (mocked)', function () { $s['pve']->guestScript('occ status', 0, '{"installed":true,"maintenance":false}'); // deploy health + acceptance probe // Fake defaults: tasks stopped/OK, guest agent up, cert reachable. - // ConfigureInstanceMail faellt jetzt in dieser Pipeline: ohne Server und - // Absenderpostfach faellt der ganze Lauf (Absicht, siehe GuestMailConfig - // und ConfigureInstanceMail selbst). Ein frischer Betrieb braucht diese - // Einrichtung einmalig, genau wie den Hetzner-DNS-Token oder den - // Stripe-Schluessel — hier nachgestellt, damit dieser Lauf ueberhaupt bis - // zum Ende kommt. + // ConfigureInstanceMail steht jetzt in dieser Pipeline. Mailversand ist + // eingerichtet, damit DIESER Lauf den Schreib-Pfad beweist (siehe + // Zusicherung weiter unten, dass der Gast die Werte tatsaechlich bekam). + // Der Fall OHNE Einrichtung — ein bezahlter Kunde bekommt seine Cloud + // trotzdem — hat einen eigenen Test weiter unten, weil er das genaue + // Gegenteil dieser Fixture braucht. Settings::set('mail.host', 'mail.clupilot.cloud'); Settings::set('mail.port', 587); Mailbox::factory()->create([ @@ -72,7 +72,11 @@ it('provisions a paid order all the way to active (mocked)', function () { // enforced allowance and a number in our database. ->and($instance->quota_applied_gb)->toBe($instance->quota_gb) ->and($instance->quota_applied_gb)->toBeGreaterThan(0) - ->and($s['pve']->guestRan('config:app:set files default_quota'))->toBeTrue(); + ->and($s['pve']->guestRan('config:app:set files default_quota'))->toBeTrue() + // Mail war eingerichtet (siehe Fixture oben) — der Schreib-Pfad von + // ConfigureInstanceMail lief also tatsaechlich, nicht nur der + // Ueberspringen-Pfad, den der Test weiter unten prueft. + ->and($s['pve']->guestRan('config:system:set mail_smtphost'))->toBeTrue(); // Every external resource created exactly once. foreach (['instance_id', 'vmid', 'dns_record_id', 'nc_admin', 'backup_job_id', 'monitoring_target_id'] as $kind) { @@ -89,6 +93,53 @@ it('provisions a paid order all the way to active (mocked)', function () { Notification::assertSentOnDemand(CloudReady::class); }); +it('completes a paid order even when the mail server does not exist yet', function () { + // Der Mailserver dieses Produkts wird gerade erst aufgesetzt — es gibt + // ihn zum Zeitpunkt dieses Laufs noch nicht. Ein bezahlter Kunde bekommt + // seine Cloud trotzdem: ConfigureInstanceMail ist eine Nebenfunktion in + // einer PFLICHT-Pipeline und darf sie nicht aufhalten, genau wie + // RegisterMonitoring die Bereitstellung nie an einer nicht erreichbaren + // Ueberwachung scheitern laesst. Bewusst KEINE mail.host/mail.port- oder + // Mailbox-Fixture hier — das ist der ganze Punkt dieses Tests. + Notification::fake(); + Queue::fake(); + $s = fakeServices(); + $s['pve']->guestScript('status.php', 0, 'ok'); + $s['pve']->guestScript('hostname -I', 0, '10.20.0.9'); + $s['pve']->guestScript('occ status', 0, '{"installed":true,"maintenance":false}'); + + Host::factory()->active()->create(['datacenter' => 'fsn', 'node' => 'pve', 'total_gb' => 1000]); + $order = Order::factory()->withSubscription()->create(['status' => 'paid', 'plan' => 'start', 'datacenter' => 'fsn']); + $run = ProvisioningRun::factory()->create([ + 'subject_type' => Order::class, 'subject_id' => $order->id, 'pipeline' => 'customer', 'context' => [], + ]); + + $runner = app(RunRunner::class); + for ($i = 0; $i < 60; $i++) { + $run->refresh(); + if (in_array($run->status, ['completed', 'failed'], true)) { + break; + } + if ($run->next_attempt_at?->isFuture()) { + $run->update(['next_attempt_at' => now()]); + } + $runner->advance($run); + } + + $run->refresh(); + $instance = Instance::query()->where('order_id', $order->id)->firstOrFail(); + + expect($run->status)->toBe('completed') + ->and($run->error)->toBeNull() + ->and($order->fresh()->status)->toBe('active') + ->and($instance->status)->toBe('active') + // Das Gegenstueck zur Zusicherung im ersten Test: ohne Einrichtung + // ging kein einziger Mailbefehl an den Gast, weder die Werte noch das + // Passwort. + ->and($s['pve']->guestRan('config:system:set mail_smtphost'))->toBeFalse() + ->and($s['pve']->guestRan('mail_smtppassword'))->toBeFalse(); +}); + it('does not duplicate external resources when a step re-runs after a crash', function () { Notification::fake(); Queue::fake();