fix(hosts): number DNS names by the normalised label, not the raw code
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>feat/portal-design
parent
e96f4c1c1a
commit
952e8e5d2f
|
|
@ -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';
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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']);
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue