From 952e8e5d2f6c8d7e0490895760b1218c0c9bcb93 Mon Sep 17 00:00:00 2001
From: nexxo <nexxo@clupilot.local>
Date: Sat, 25 Jul 2026 23:59:58 +0200
Subject: [PATCH] fix(hosts): number DNS names by the normalised label, not the
 raw code
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two legacy codes can normalise to the same label (eu_west and eu-west), and
separate counters then handed both of them eu-west-01 — a unique-constraint
failure inside the reservation, which blocks onboarding rather than being a DNS
problem the step can shrug off. Lock, counter and the in-use check all key off
the label now.

(The helper was also called label(), which HostStep already declares as the
step's display name — renamed.)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 .../Steps/Host/RegisterHostDns.php            | 34 +++++++++++++------
 tests/Feature/Provisioning/HostStepsTest.php  | 20 +++++++++++
 2 files changed, 44 insertions(+), 10 deletions(-)

diff --git a/app/Provisioning/Steps/Host/RegisterHostDns.php b/app/Provisioning/Steps/Host/RegisterHostDns.php
index c7fd23e..665da3b 100644
--- a/app/Provisioning/Steps/Host/RegisterHostDns.php
+++ b/app/Provisioning/Steps/Host/RegisterHostDns.php
@@ -70,25 +70,27 @@ class RegisterHostDns extends HostStep
      */
     private function reserveName(Host $host): string
     {
-        return Cache::lock('host-dns-name:'.$host->datacenter, 30)->block(10, function () use ($host) {
-            $key = 'dns.sequence.'.$host->datacenter;
+        // Everything keys off the LABEL, not the raw code: two legacy codes can
+        // normalise to the same label (eu_west and eu-west), and separate
+        // counters would then hand both of them eu-west-01 — a unique-constraint
+        // failure that blocks onboarding rather than a DNS problem we can shrug off.
+        $label = $this->dnsLabel($host->datacenter);
+
+        return Cache::lock('host-dns-name:'.$label, 30)->block(10, function () use ($host, $label) {
+            $key = 'dns.sequence.'.$label;
             $next = ((int) Settings::get($key, 0)) + 1;
 
             // Never below what is already in use — a settings store that was
-            // wiped must not start handing out live names again.
+            // wiped must not start handing out live names again. Matched on the
+            // name itself, so it covers every code sharing this label.
             $inUse = Host::query()
-                ->where('datacenter', $host->datacenter)
-                ->whereNotNull('dns_name')
+                ->where('dns_name', 'like', $label.'-%')
                 ->pluck('dns_name')
                 ->map(fn (string $name) => (int) substr($name, strrpos($name, '-') + 1))
                 ->max() ?? 0;
             $next = max($next, $inUse + 1);
 
-            // Datacenter codes created before the rule was tightened can still
-            // hold characters a DNS label cannot; a name that the provider
-            // rejects would leave the host nameless forever.
-            $label = trim(preg_replace('/[^a-z0-9-]+/', '-', strtolower($host->datacenter)) ?? '', '-');
-            $name = sprintf('%s-%02d', $label !== '' ? $label : 'node', $next);
+            $name = sprintf('%s-%02d', $label, $next);
 
             Settings::set($key, $next);
             $host->update(['dns_name' => $name]);   // reserved while still locked
@@ -96,4 +98,16 @@ class RegisterHostDns extends HostStep
             return $name;
         });
     }
+
+    /**
+     * Datacenter codes created before the rule was tightened can hold characters
+     * a DNS label cannot; a name the provider rejects would leave the host
+     * nameless forever.
+     */
+    private function dnsLabel(string $datacenter): string
+    {
+        $label = trim(preg_replace('/[^a-z0-9-]+/', '-', strtolower($datacenter)) ?? '', '-');
+
+        return $label !== '' ? $label : 'node';
+    }
 }
diff --git a/tests/Feature/Provisioning/HostStepsTest.php b/tests/Feature/Provisioning/HostStepsTest.php
index faa8c47..8f94c79 100644
--- a/tests/Feature/Provisioning/HostStepsTest.php
+++ b/tests/Feature/Provisioning/HostStepsTest.php
@@ -361,3 +361,23 @@ it('builds a valid DNS label even from an awkward datacenter code', function ()
     expect($name)->toBe('eu-west-01')
         ->and($name)->toMatch('/^[a-z0-9]([a-z0-9-]*[a-z0-9])?$/');
 });
+
+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']);
+
+    $names = [];
+    foreach (['eu_west', 'eu-west'] as $i => $dc) {
+        $host = App\Models\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);
+        $names[] = $host->fresh()->dns_name;
+    }
+
+    // Separate counters per raw code would have produced eu-west-01 twice and
+    // failed the unique constraint mid-onboarding.
+    expect($names)->toBe(['eu-west-01', 'eu-west-02']);
+});