mirror of
https://github.com/coollabsio/coolify.git
synced 2026-11-06 21:18:11 -04:00
fix(services): do not warn about the port when a domain has no port
An empty port routes to the required port, so it needs no confirmation. The edit dialog shows the effective port, the warning buttons name the ports, and domain conflicts ignore the container port. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
b1a20fabd4
commit
53e21abe97
@@ -112,6 +112,8 @@ class Domains extends Component
|
||||
|
||||
public ?int $requiredPort = null;
|
||||
|
||||
public ?int $requestedPort = null;
|
||||
|
||||
public bool $isCheckingDns = false;
|
||||
|
||||
#[Locked]
|
||||
@@ -1603,10 +1605,10 @@ class Domains extends Component
|
||||
if ($checkPorts && ! $this->forceRemovePort) {
|
||||
$requiredPort = $app->getRequiredPort();
|
||||
if ($requiredPort !== null && $domainString) {
|
||||
$previousFqdn = $app->getOriginal('fqdn');
|
||||
foreach ($this->splitDomains($domainString) as $fqdn) {
|
||||
if ($app->portRequiresConfirmation($fqdn, $requiredPort, is_string($previousFqdn) ? $previousFqdn : null)) {
|
||||
if ($app->portRequiresConfirmation($fqdn, $requiredPort)) {
|
||||
$this->requiredPort = $requiredPort;
|
||||
$this->requestedPort = ServiceApplication::extractPortFromUrl($fqdn);
|
||||
$this->showPortWarningModal = true;
|
||||
$app->refresh();
|
||||
|
||||
|
||||
@@ -84,6 +84,8 @@ class Index extends Component
|
||||
|
||||
public $requiredPort = null;
|
||||
|
||||
public ?int $requestedPort = null;
|
||||
|
||||
public ?string $fqdn = null;
|
||||
|
||||
public bool $isGzipEnabled = false;
|
||||
@@ -630,7 +632,6 @@ class Index extends Component
|
||||
$persistedApplication = $this->serviceApplication->fresh();
|
||||
$previousDnsHostnames = app(ManagedDnsRecordCleanup::class)->hostnamesOf($persistedApplication);
|
||||
$previousServiceDnsHostnames = array_keys(app(ResourceDnsRecords::class)->hostnameUsers($this->service));
|
||||
$previousEditableUrls = $persistedApplication->url;
|
||||
$previousFqdn = $persistedApplication->fqdn;
|
||||
$previousPortOverrides = $persistedApplication->domain_port_overrides;
|
||||
$this->authorize('update', $this->serviceApplication);
|
||||
@@ -669,8 +670,9 @@ class Index extends Component
|
||||
continue;
|
||||
}
|
||||
|
||||
if ($this->serviceApplication->portRequiresConfirmation($fqdn, $requiredPort, $previousEditableUrls)) {
|
||||
if ($this->serviceApplication->portRequiresConfirmation($fqdn, $requiredPort)) {
|
||||
$this->requiredPort = $requiredPort;
|
||||
$this->requestedPort = ServiceApplication::extractPortFromUrl($fqdn);
|
||||
$this->showPortWarningModal = true;
|
||||
$this->serviceApplication->fqdn = $previousFqdn;
|
||||
$this->serviceApplication->domain_port_overrides = $previousPortOverrides;
|
||||
|
||||
@@ -275,7 +275,7 @@ class ServiceApplication extends BaseModel
|
||||
/**
|
||||
* True when saving this URL should confirm that it does not use the required template port.
|
||||
*/
|
||||
public function portRequiresConfirmation(string $fqdn, ?int $requiredPort, ?string $previousFqdn = null): bool
|
||||
public function portRequiresConfirmation(string $fqdn, ?int $requiredPort): bool
|
||||
{
|
||||
if ($requiredPort === null) {
|
||||
return false;
|
||||
@@ -286,28 +286,14 @@ class ServiceApplication extends BaseModel
|
||||
return false;
|
||||
}
|
||||
|
||||
$canonical = DomainPortOverrides::withoutPort($fqdn);
|
||||
$explicit = self::extractPortFromUrl($fqdn);
|
||||
|
||||
if ($explicit === $requiredPort) {
|
||||
// A domain without a port routes to the required port.
|
||||
if ($explicit === null || $explicit === $requiredPort) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if ($explicit === null) {
|
||||
$previous = collect(explode(',', (string) $previousFqdn))
|
||||
->filter();
|
||||
$previousUrl = $previous->first(
|
||||
fn (string $url): bool => DomainPortOverrides::withoutPort(trim($url)) === $canonical
|
||||
);
|
||||
|
||||
if (is_string($previousUrl) && self::extractPortFromUrl($previousUrl) !== null) {
|
||||
return true;
|
||||
}
|
||||
|
||||
return $previousUrl === null;
|
||||
}
|
||||
|
||||
$existingOverride = $this->domain_port_overrides[$canonical] ?? null;
|
||||
$existingOverride = $this->domain_port_overrides[DomainPortOverrides::withoutPort($fqdn)] ?? null;
|
||||
|
||||
return (int) $existingOverride !== $explicit;
|
||||
}
|
||||
|
||||
@@ -79,7 +79,10 @@ function domainConflictKey(string $domain): string
|
||||
? str($domain)->beforeLast('/')->toString()
|
||||
: $domain;
|
||||
|
||||
return preg_replace('#^https?://#i', '', $domain) ?? $domain;
|
||||
$domain = preg_replace('#^https?://#i', '', $domain) ?? $domain;
|
||||
|
||||
// The URL port is the container port. The public address does not include it.
|
||||
return preg_replace('#^([^/:\[]+|\[[^\]]+\]):\d+#', '$1', $domain) ?? $domain;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -336,18 +336,18 @@
|
||||
<div class="application-settings-section-body">
|
||||
<x-callout type="warning" title="Port requirement" class="mb-4">
|
||||
This service requires port <strong>{{ $requiredPort }}</strong> to function correctly.
|
||||
One or more of your domains use a different port, or none.
|
||||
One or more of your domains use port <strong>{{ $requestedPort }}</strong>.
|
||||
</x-callout>
|
||||
|
||||
<div class="mt-4 flex flex-wrap justify-end gap-2 border-t border-neutral-200 pt-4 dark:border-white/[0.08]">
|
||||
<x-forms.button type="button" wire:click="confirmRemovePort"
|
||||
@click="modalOpen = false" isError>
|
||||
Use port {{ $requestedPort }} anyway
|
||||
</x-forms.button>
|
||||
<x-forms.button type="button"
|
||||
wire:click="cancelRemovePort"
|
||||
@click="modalOpen = false">
|
||||
Keep required port
|
||||
</x-forms.button>
|
||||
<x-forms.button type="button" wire:click="confirmRemovePort"
|
||||
@click="modalOpen = false" isError>
|
||||
Use this port anyway
|
||||
Keep port {{ $requiredPort }}
|
||||
</x-forms.button>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
@@ -141,7 +141,7 @@
|
||||
<div class="application-settings-section-body">
|
||||
<x-callout type="warning" title="Port requirement" class="mb-4">
|
||||
This service requires port <strong>{{ $requiredPort }}</strong> to function correctly.
|
||||
One or more of your domains use a different port, or none.
|
||||
One or more of your domains use port <strong>{{ $requestedPort }}</strong>.
|
||||
</x-callout>
|
||||
|
||||
<x-callout type="danger" title="What will happen if you continue?" class="mb-4">
|
||||
@@ -154,13 +154,13 @@
|
||||
</x-callout>
|
||||
|
||||
<div class="mt-4 flex flex-wrap justify-end gap-2 border-t border-neutral-200 pt-4 dark:border-white/[0.08]">
|
||||
<x-forms.button @click="modalOpen = false; $wire.call('cancelRemovePort')"
|
||||
class="w-auto">
|
||||
Keep required port
|
||||
</x-forms.button>
|
||||
<x-forms.button wire:click="confirmRemovePort" @click="modalOpen = false" class="w-auto"
|
||||
isError>
|
||||
Use this port anyway
|
||||
Use port {{ $requestedPort }} anyway
|
||||
</x-forms.button>
|
||||
<x-forms.button @click="modalOpen = false; $wire.call('cancelRemovePort')"
|
||||
class="w-auto">
|
||||
Keep port {{ $requiredPort }}
|
||||
</x-forms.button>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
@@ -56,7 +56,7 @@
|
||||
$faviconUrl = $isSuggested || $serviceApplication?->hasProxyBasicAuth() ? null : \App\Support\DomainFavicon::url($publicUrl, $row['dns_status'] ?? null, $row['expected_ip'] ?? null);
|
||||
$domainKey = hash('sha256', $row['url'].'|'.($row['service_application_id'] ?? ''));
|
||||
$editingParts = \App\Support\DomainUrlParts::split($row['url']);
|
||||
if ($row['has_port_override'] ?? false) {
|
||||
if (filled($row['internal_port'] ?? null)) {
|
||||
$editingParts['port'] = (string) $row['internal_port'];
|
||||
}
|
||||
@endphp
|
||||
|
||||
@@ -755,7 +755,7 @@ YAML,
|
||||
->assertDispatched('success');
|
||||
});
|
||||
|
||||
it('saves after confirming both a domain conflict and a missing required port', function () {
|
||||
it('saves after confirming both a domain conflict and a different port', function () {
|
||||
$this->service->update([
|
||||
'docker_compose_raw' => <<<'YAML'
|
||||
services:
|
||||
@@ -770,7 +770,9 @@ YAML,
|
||||
|
||||
$component = Livewire::test(Domains::class, ['service' => $this->service->fresh(['applications', 'server'])])
|
||||
->set('newServiceApplicationId', $this->webApp->id)
|
||||
->set('newDomain', 'https://api.example.com')
|
||||
->set('newDomainParts.host', 'api.example.com')
|
||||
->set('newDomainParts.port', '3000')
|
||||
->set('newDomainPartsChanged', true)
|
||||
->call('addDomain')
|
||||
->assertSet('showDomainConflictModal', true)
|
||||
->call('confirmDomainUsage')
|
||||
@@ -778,8 +780,8 @@ YAML,
|
||||
->assertSet('showPortWarningModal', true)
|
||||
->assertSet('forceSaveDomains', true)
|
||||
->assertSee('Use a different port?')
|
||||
->assertSee('Keep required port')
|
||||
->assertSee('Use this port anyway');
|
||||
->assertSee('Keep port 8000')
|
||||
->assertSee('Use port 3000 anyway');
|
||||
|
||||
$component
|
||||
->call('confirmRemovePort')
|
||||
@@ -790,7 +792,28 @@ YAML,
|
||||
->assertDispatched('success');
|
||||
|
||||
expect(explode(',', (string) $this->webApp->fresh()->fqdn))
|
||||
->toBe(['https://api.example.com']);
|
||||
->toBe(['https://api.example.com'])
|
||||
->and($this->webApp->fresh()->domain_port_overrides)->toBe(['https://api.example.com' => 3000]);
|
||||
});
|
||||
|
||||
it('does not warn about the port when a domain with an empty port is renamed', function () {
|
||||
$this->service->update([
|
||||
'docker_compose_raw' => "services:\n api:\n image: node:alpine\n environment:\n - SERVICE_URL_API_8080\n web:\n image: nginx:alpine\n",
|
||||
]);
|
||||
$this->apiApp->update(['fqdn' => 'https://api.example.com']);
|
||||
|
||||
Livewire::test(Domains::class, ['service' => $this->service->fresh(['applications', 'server'])])
|
||||
->call('startEdit', 0)
|
||||
->set('editingDomainParts.host', 'renamed.example.com')
|
||||
->set('editingDomainParts.port', '')
|
||||
->set('editingDomainPartsChanged', true)
|
||||
->call('updateDomain')
|
||||
->assertSet('showPortWarningModal', false)
|
||||
->assertDispatched('success');
|
||||
|
||||
$this->apiApp->refresh();
|
||||
expect($this->apiApp->fqdn)->toBe('https://renamed.example.com')
|
||||
->and($this->apiApp->domain_port_overrides)->toBeNull();
|
||||
});
|
||||
|
||||
it('loads persisted dns status for service applications', function () {
|
||||
|
||||
Reference in New Issue
Block a user