From 3f1235158d56359dac5ad5c80e0d3891a85015d3 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Thu, 10 Sep 2026 14:53:01 +0200 Subject: [PATCH] fix(domains): detect Docker Compose domain conflicts Normalize schemes when comparing routing identities and include per-service Compose domains in conflict checks. Show the conflicting service name and align the service domains heading layout. --- .ai/lessons.md | 7 + bootstrap/helpers/domains.php | 135 ++++++++++-------- .../domain-conflict-modal.blade.php | 2 +- .../project/service/domains.blade.php | 4 +- tests/Feature/ApplicationDomainsTest.php | 118 +++++++++++++++ tests/Feature/ServiceDomainsTest.php | 10 ++ 6 files changed, 210 insertions(+), 66 deletions(-) diff --git a/.ai/lessons.md b/.ai/lessons.md index 3a2dbb947..28df883d9 100644 --- a/.ai/lessons.md +++ b/.ai/lessons.md @@ -41,3 +41,10 @@ ## Use one DNS progress pattern - All DNS check entry points must set the domain badge to the same `checking` state. - Do not use separate loading feedback on Check all or per-domain action buttons when the badge is the progress indicator. - Verify the rendered badge uses the spinner slot instead of the default status dot. + +## Confirm whether old reports still apply before changing code +- For an old issue, first test the current branch and inspect later fixes. Do not assume that the historical reproduction still needs a new code change. + +## Compare routing identity, not complete domain URLs +- Domain-conflict checks must treat `http://host` and `https://host` as the same routing identity. +- Reproduce reports with the exact stored schemes before stating that duplicate detection works. diff --git a/bootstrap/helpers/domains.php b/bootstrap/helpers/domains.php index 28ff41b3d..8be6d9a77 100644 --- a/bootstrap/helpers/domains.php +++ b/bootstrap/helpers/domains.php @@ -68,6 +68,52 @@ function isValidDomainUrl(string $url): bool return filter_var($urlToValidate, FILTER_VALIDATE_URL) !== false; } +function domainConflictKey(string $domain): string +{ + $domain = str($domain)->endsWith('/') + ? str($domain)->beforeLast('/')->toString() + : $domain; + + return preg_replace('#^https?://#i', '', $domain) ?? $domain; +} + +/** + * @return Collection + */ +function applicationDomainEntries(Application $application): Collection +{ + $entries = collect(explode(',', (string) $application->fqdn)) + ->filter(fn ($domain) => $domain !== '') + ->map(fn ($domain) => [ + 'domain' => str($domain)->finish('/')->beforeLast('/')->toString(), + 'service_name' => null, + ]); + + if ($application->build_pack !== 'dockercompose' || empty($application->docker_compose_domains)) { + return $entries->values(); + } + + $composeDomains = json_decode($application->docker_compose_domains, true); + if (! is_array($composeDomains)) { + return $entries->values(); + } + + foreach ($composeDomains as $serviceName => $domainConfig) { + foreach (explode(',', (string) data_get($domainConfig, 'domain')) as $domain) { + if ($domain === '') { + continue; + } + + $entries->push([ + 'domain' => str($domain)->finish('/')->beforeLast('/')->toString(), + 'service_name' => (string) $serviceName, + ]); + } + } + + return $entries->values(); +} + function checkDomainUsage(ServiceApplication|Application|null $resource = null, ?string $domain = null) { $conflicts = []; @@ -80,8 +126,7 @@ function checkDomainUsage(ServiceApplication|Application|null $resource = null, if ($resource) { if ($resource->getMorphClass() === Application::class && $resource->build_pack === 'dockercompose') { - $domains = data_get(json_decode($resource->docker_compose_domains, true), '*.domain'); - $domains = collect($domains); + $domains = applicationDomainEntries($resource)->pluck('domain'); } else { $domains = collect($resource->fqdns); } @@ -96,7 +141,7 @@ function checkDomainUsage(ServiceApplication|Application|null $resource = null, $domain = str($domain)->beforeLast('/'); } - return str($domain); + return domainConflictKey((string) $domain); }); // Filter applications by team if we have a current team @@ -108,13 +153,9 @@ function checkDomainUsage(ServiceApplication|Application|null $resource = null, } $apps = $appsQuery->get(); foreach ($apps as $app) { - $list_of_domains = collect(explode(',', $app->fqdn))->filter(fn ($fqdn) => $fqdn !== ''); - foreach ($list_of_domains as $domain) { - if (str($domain)->endsWith('/')) { - $domain = str($domain)->beforeLast('/'); - } - $naked_domain = str($domain)->value(); - if ($domains->contains($naked_domain)) { + foreach (applicationDomainEntries($app) as $domainEntry) { + $naked_domain = $domainEntry['domain']; + if ($domains->contains(domainConflictKey($naked_domain))) { if (data_get($resource, 'uuid')) { if ($resource->uuid !== $app->uuid) { $conflicts[] = [ @@ -122,16 +163,18 @@ function checkDomainUsage(ServiceApplication|Application|null $resource = null, 'resource_name' => $app->name, 'resource_link' => $app->link(), 'resource_type' => 'application', - 'message' => "Domain $naked_domain is already in use by application '{$app->name}'", + 'message' => "Domain $naked_domain is already in use by application '{$app->name}'".($domainEntry['service_name'] ? " (service: {$domainEntry['service_name']})" : ''), + ...($domainEntry['service_name'] ? ['service_name' => $domainEntry['service_name']] : []), ]; } - } elseif ($domain) { + } else { $conflicts[] = [ 'domain' => $naked_domain, 'resource_name' => $app->name, 'resource_link' => $app->link(), 'resource_type' => 'application', - 'message' => "Domain $naked_domain is already in use by application '{$app->name}'", + 'message' => "Domain $naked_domain is already in use by application '{$app->name}'".($domainEntry['service_name'] ? " (service: {$domainEntry['service_name']})" : ''), + ...($domainEntry['service_name'] ? ['service_name' => $domainEntry['service_name']] : []), ]; } } @@ -153,7 +196,7 @@ function checkDomainUsage(ServiceApplication|Application|null $resource = null, $domain = str($domain)->beforeLast('/'); } $naked_domain = str($domain)->value(); - if ($domains->contains($naked_domain)) { + if ($domains->contains(domainConflictKey($naked_domain))) { if (data_get($resource, 'uuid')) { if ($resource->uuid !== $app->uuid) { $conflicts[] = [ @@ -185,7 +228,7 @@ function checkDomainUsage(ServiceApplication|Application|null $resource = null, $domain = str($domain)->beforeLast('/'); } $naked_domain = str($domain)->value(); - if ($domains->contains($naked_domain)) { + if ($domains->contains(domainConflictKey($naked_domain))) { $conflicts[] = [ 'domain' => $naked_domain, 'resource_name' => 'Coolify Instance', @@ -219,7 +262,7 @@ function checkIfDomainIsAlreadyUsedViaAPI(Collection|array $domains, ?string $te $domain = str($domain)->beforeLast('/'); } - return str($domain); + return domainConflictKey((string) $domain); }); $applications = Application::ownedByCurrentTeamAPI($teamId)->get(['fqdn', 'uuid', 'name', 'id', 'docker_compose_domains', 'build_pack']); @@ -231,51 +274,17 @@ function checkIfDomainIsAlreadyUsedViaAPI(Collection|array $domains, ?string $te } foreach ($applications as $app) { - if (! is_null($app->fqdn)) { - $list_of_domains = collect(explode(',', $app->fqdn))->filter(fn ($fqdn) => $fqdn !== ''); - foreach ($list_of_domains as $domain) { - if (str($domain)->endsWith('/')) { - $domain = str($domain)->beforeLast('/'); - } - $naked_domain = str($domain)->value(); - if ($domains->contains($naked_domain)) { - $conflicts[] = [ - 'domain' => $naked_domain, - 'resource_name' => $app->name, - 'resource_uuid' => $app->uuid, - 'resource_type' => 'application', - 'message' => "Domain $naked_domain is already in use by application '{$app->name}'", - ]; - } - } - } - - if ($app->build_pack === 'dockercompose' && ! empty($app->docker_compose_domains)) { - $dockerComposeDomains = json_decode($app->docker_compose_domains, true); - if (is_array($dockerComposeDomains)) { - foreach ($dockerComposeDomains as $serviceName => $domainConfig) { - $domainValue = data_get($domainConfig, 'domain'); - if (empty($domainValue)) { - continue; - } - $list_of_domains = collect(explode(',', $domainValue))->filter(fn ($fqdn) => $fqdn !== ''); - foreach ($list_of_domains as $domain) { - if (str($domain)->endsWith('/')) { - $domain = str($domain)->beforeLast('/'); - } - $naked_domain = str($domain)->value(); - if ($domains->contains($naked_domain)) { - $conflicts[] = [ - 'domain' => $naked_domain, - 'resource_name' => $app->name, - 'resource_uuid' => $app->uuid, - 'resource_type' => 'application', - 'service_name' => $serviceName, - 'message' => "Domain $naked_domain is already in use by application '{$app->name}' (service: {$serviceName})", - ]; - } - } - } + foreach (applicationDomainEntries($app) as $domainEntry) { + $naked_domain = $domainEntry['domain']; + if ($domains->contains(domainConflictKey($naked_domain))) { + $conflicts[] = [ + 'domain' => $naked_domain, + 'resource_name' => $app->name, + 'resource_uuid' => $app->uuid, + 'resource_type' => 'application', + 'message' => "Domain $naked_domain is already in use by application '{$app->name}'".($domainEntry['service_name'] ? " (service: {$domainEntry['service_name']})" : ''), + ...($domainEntry['service_name'] ? ['service_name' => $domainEntry['service_name']] : []), + ]; } } } @@ -290,7 +299,7 @@ function checkIfDomainIsAlreadyUsedViaAPI(Collection|array $domains, ?string $te $domain = str($domain)->beforeLast('/'); } $naked_domain = str($domain)->value(); - if ($domains->contains($naked_domain)) { + if ($domains->contains(domainConflictKey($naked_domain))) { $conflicts[] = [ 'domain' => $naked_domain, 'resource_name' => $app->service->name ?? 'Unknown Service', @@ -310,7 +319,7 @@ function checkIfDomainIsAlreadyUsedViaAPI(Collection|array $domains, ?string $te $domain = str($domain)->beforeLast('/'); } $naked_domain = str($domain)->value(); - if ($domains->contains($naked_domain)) { + if ($domains->contains(domainConflictKey($naked_domain))) { $conflicts[] = [ 'domain' => $naked_domain, 'resource_name' => 'Coolify Instance', diff --git a/resources/views/components/domain-conflict-modal.blade.php b/resources/views/components/domain-conflict-modal.blade.php index 9976fb638..5ad53bc78 100644 --- a/resources/views/components/domain-conflict-modal.blade.php +++ b/resources/views/components/domain-conflict-modal.blade.php @@ -47,7 +47,7 @@ class="underline hover:text-red-400"> {{ $conflict['resource_name'] }} @endif - ({{ $conflict['resource_type'] }}) + ({{ $conflict['resource_type'] }}@if (filled($conflict['service_name'] ?? null)): {{ $conflict['service_name'] }}@endif) @endforeach diff --git a/resources/views/livewire/project/service/domains.blade.php b/resources/views/livewire/project/service/domains.blade.php index ff6fdf66f..c745b29b7 100644 --- a/resources/views/livewire/project/service/domains.blade.php +++ b/resources/views/livewire/project/service/domains.blade.php @@ -69,9 +69,9 @@ @endcannot {{-- Toolbar --}} -
+
-

Domains

+

Domains

{{ $configuredCount }} domain{{ $configuredCount === 1 ? '' : 's' }} across {{ $domainGroups->count() }} service{{ $domainGroups->count() === 1 ? '' : 's' }} @if ($suggestedCount > 0) diff --git a/tests/Feature/ApplicationDomainsTest.php b/tests/Feature/ApplicationDomainsTest.php index 51b2b0ae8..d7be33df7 100644 --- a/tests/Feature/ApplicationDomainsTest.php +++ b/tests/Feature/ApplicationDomainsTest.php @@ -15,6 +15,7 @@ use App\Models\User; use App\Support\ValidationPatterns; use Illuminate\Foundation\Testing\RefreshDatabase; +use Illuminate\Support\Facades\Blade; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Queue; use Illuminate\Support\Str; @@ -1787,6 +1788,123 @@ ]); }); +it('warns when adding a domain used by a docker compose application', function () { + Application::factory()->create([ + 'uuid' => (string) Str::uuid(), + 'name' => 'Compose Conflict App', + 'environment_id' => $this->environment->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + 'fqdn' => null, + 'build_pack' => 'dockercompose', + 'docker_compose_domains' => json_encode([ + 'web' => ['domain' => 'https://compose-taken.example.com', 'redirect' => 'both'], + ]), + ]); + + Livewire::test(Domains::class, ['application' => $this->application->fresh()]) + ->set('newDomain', 'https://compose-taken.example.com') + ->call('addDomain') + ->assertSet('showDomainConflictModal', true) + ->assertSet('domainConflicts.0.service_name', 'web') + ->assertSet('pendingAction', 'add'); + + expect($this->application->fresh()->fqdn)->toBeNull(); +}); + +it('warns when a compose domain uses a different scheme', function () { + Application::factory()->create([ + 'uuid' => (string) Str::uuid(), + 'name' => 'HTTP Compose Conflict App', + 'environment_id' => $this->environment->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + 'fqdn' => null, + 'build_pack' => 'dockercompose', + 'docker_compose_domains' => json_encode([ + 'web' => ['domain' => 'http://scheme-conflict.example.com', 'redirect' => 'both'], + ]), + ]); + + Livewire::test(Domains::class, ['application' => $this->application->fresh()]) + ->set('newDomain', 'https://scheme-conflict.example.com') + ->call('addDomain') + ->assertSet('showDomainConflictModal', true) + ->assertSet('domainConflicts.0.service_name', 'web'); + + expect($this->application->fresh()->fqdn)->toBeNull(); +}); + +it('shows the compose service name in the domain conflict modal', function () { + $html = Blade::render( + '', + ['conflicts' => [[ + 'domain' => 'https://compose-taken.example.com', + 'resource_name' => 'Compose Conflict App', + 'resource_link' => '#', + 'resource_type' => 'application', + 'service_name' => 'web', + ]]], + ); + + expect($html)->toContain('(application: web)'); +}); + +it('checks each domain configured for one docker compose service', function () { + Application::factory()->create([ + 'uuid' => (string) Str::uuid(), + 'name' => 'Conflicting App', + 'environment_id' => $this->environment->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + 'fqdn' => 'https://second-compose.example.com', + 'build_pack' => 'nixpacks', + ]); + + $this->application->update([ + 'build_pack' => 'dockercompose', + 'fqdn' => null, + 'docker_compose_domains' => json_encode([ + 'web' => [ + 'domain' => 'https://first-compose.example.com,https://second-compose.example.com', + 'redirect' => 'both', + ], + ]), + ]); + + $result = checkDomainUsage(resource: $this->application->fresh()); + + expect($result['hasConflicts'])->toBeTrue() + ->and($result['conflicts'])->toHaveCount(1) + ->and($result['conflicts'][0]['domain'])->toBe('https://second-compose.example.com'); +}); + +it('uses docker compose domains in API conflict checks', function () { + $composeApplication = Application::factory()->create([ + 'uuid' => (string) Str::uuid(), + 'name' => 'Compose API Conflict App', + 'environment_id' => $this->environment->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + 'fqdn' => null, + 'build_pack' => 'dockercompose', + 'docker_compose_domains' => json_encode([ + 'api' => ['domain' => 'https://compose-api.example.com', 'redirect' => 'both'], + ]), + ]); + + $result = checkIfDomainIsAlreadyUsedViaAPI( + ['https://compose-api.example.com'], + (string) $this->team->id, + $this->application->uuid, + ); + + expect($result['hasConflicts'])->toBeTrue() + ->and($result['conflicts'])->toHaveCount(1) + ->and($result['conflicts'][0]['resource_uuid'])->toBe($composeApplication->uuid) + ->and($result['conflicts'][0]['service_name'])->toBe('api'); +}); + it('saves after confirming a domain conflict on edit', function () { Application::factory()->create([ 'uuid' => (string) Str::uuid(), diff --git a/tests/Feature/ServiceDomainsTest.php b/tests/Feature/ServiceDomainsTest.php index 581972b86..19a87c99d 100644 --- a/tests/Feature/ServiceDomainsTest.php +++ b/tests/Feature/ServiceDomainsTest.php @@ -225,6 +225,16 @@ ->not->toContain('placeholder="https://app.example.com"'); }); +it('matches the application domains toolbar heading and top spacing', function () { + $view = file_get_contents(resource_path('views/livewire/project/service/domains.blade.php')); + + expect($view) + ->toContain('

') + ->toContain('

Domains

') + ->not->toContain('
') + ->not->toContain('

Domains

'); +}); + it('resets the add domain dns gate when segmented domain fields change', function () { Livewire::test(Domains::class, ['service' => $this->service->fresh(['applications', 'server'])]) ->set('addDomainDnsFailed', true)