mirror of
https://github.com/coollabsio/coolify.git
synced 2026-11-06 13:10:37 -04:00
fix(services): keep SERVICE_URL_* on the named service domain
When a compose service copies another service's SERVICE_URL_/SERVICE_FQDN_ declaration (old Twenty template: worker repeats SERVICE_URL_TWENTY_3000), the parser and updateCompose wrote the variable from the copier's generated domain, so a custom domain on the named service was lost on every deploy. Skip exact copies; declarations with another path or port are unchanged. Closes #11924
This commit is contained in:
@@ -3,6 +3,7 @@
|
||||
use App\Models\Application;
|
||||
use App\Models\Service;
|
||||
use App\Models\ServiceApplication;
|
||||
use App\Models\ServiceDatabase;
|
||||
use Illuminate\Support\Collection;
|
||||
|
||||
/**
|
||||
@@ -591,6 +592,37 @@ function findServiceApplicationForEnvName(Service $resource, string $envServiceN
|
||||
return $resource->applications()->where('name', $resolved)->first();
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if a service copies a SERVICE_FQDN_ / SERVICE_URL_ declaration of the application named in it.
|
||||
* The copy (same key, same value) must not generate a domain or overwrite the variable (#11924).
|
||||
* Declarations with another port or path share the host on purpose (for example Appwrite).
|
||||
*
|
||||
* @param array<string, mixed>|Collection<string, mixed> $services Parsed compose services
|
||||
*/
|
||||
function isCopiedServiceUrlDeclaration(
|
||||
Service $resource,
|
||||
array|Collection $services,
|
||||
ServiceApplication|ServiceDatabase $current,
|
||||
string $key,
|
||||
): bool {
|
||||
$owner = findServiceApplicationForEnvName($resource, parseServiceEnvironmentVariable($key)['service_name']);
|
||||
if (! $owner || $owner->name === $current->name) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Same declaration sources as serviceParser: environment and build arguments.
|
||||
$declarationsOf = fn (string $serviceName) => convertToKeyValueCollection(
|
||||
collect(data_get($services, [$serviceName, 'environment'], []))
|
||||
->merge(data_get($services, [$serviceName, 'build', 'args'], []))
|
||||
);
|
||||
$ownerDeclarations = $declarationsOf($owner->name);
|
||||
$currentDeclarations = $declarationsOf($current->name);
|
||||
|
||||
return $ownerDeclarations->has($key)
|
||||
&& $currentDeclarations->has($key)
|
||||
&& (string) $ownerDeclarations->get($key) === (string) $currentDeclarations->get($key);
|
||||
}
|
||||
|
||||
/**
|
||||
* Put/update a service domain entry under the original compose service name,
|
||||
* removing legacy twin keys that resolve unambiguously to the same name.
|
||||
|
||||
@@ -2547,6 +2547,10 @@ function serviceParser(Service $resource): Collection
|
||||
$port = $parsed['port'];
|
||||
$fqdnFor = $parsed['service_name'];
|
||||
|
||||
if (isCopiedServiceUrlDeclaration($resource, $services, $savedService, $key->value())) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Only ServiceApplication has fqdn column, ServiceDatabase does not
|
||||
$isServiceApplication = $savedService instanceof ServiceApplication;
|
||||
|
||||
|
||||
@@ -313,8 +313,16 @@ function updateCompose(ServiceApplication|ServiceDatabase $resource)
|
||||
// Those belong to other services and will be updated when THOSE services are updated
|
||||
}
|
||||
|
||||
// Remove duplicates
|
||||
$templateVariableNames = array_unique($templateVariableNames);
|
||||
// Remove duplicates and copies of another application's declaration
|
||||
$templateVariableNames = array_filter(
|
||||
array_unique($templateVariableNames),
|
||||
fn ($variableName) => ! isCopiedServiceUrlDeclaration(
|
||||
$resource->service,
|
||||
collect(data_get($dockerCompose, 'services', [])),
|
||||
$resource,
|
||||
$variableName,
|
||||
),
|
||||
);
|
||||
|
||||
// Extract unique service names to process (preserving the original case from template)
|
||||
// This allows us to create both URL and FQDN pairs regardless of which one is in the template
|
||||
|
||||
@@ -0,0 +1,156 @@
|
||||
<?php
|
||||
|
||||
/**
|
||||
* Regression for #11924: when two compose services declare the same
|
||||
* SERVICE_URL_<NAME>_<PORT> (old Twenty template: twenty + worker), the
|
||||
* variable must follow the domain of the <NAME> application, not the domain
|
||||
* of whichever service the parser processes last.
|
||||
*/
|
||||
|
||||
use App\Models\Environment;
|
||||
use App\Models\InstanceSettings;
|
||||
use App\Models\Project;
|
||||
use App\Models\Server;
|
||||
use App\Models\Service;
|
||||
use App\Models\ServiceApplication;
|
||||
use App\Models\StandaloneDocker;
|
||||
use App\Models\Team;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
|
||||
uses(RefreshDatabase::class);
|
||||
|
||||
beforeEach(function () {
|
||||
InstanceSettings::forceCreate(['id' => 0]);
|
||||
$team = Team::factory()->create();
|
||||
$server = Server::factory()->create(['team_id' => $team->id]);
|
||||
$destination = StandaloneDocker::where('server_id', $server->id)->first()
|
||||
?? StandaloneDocker::factory()->create(['server_id' => $server->id]);
|
||||
$project = Project::factory()->create(['team_id' => $team->id]);
|
||||
$environment = Environment::factory()->create(['project_id' => $project->id]);
|
||||
|
||||
$this->service = Service::factory()->create([
|
||||
'environment_id' => $environment->id,
|
||||
'server_id' => $server->id,
|
||||
'destination_id' => $destination->id,
|
||||
'destination_type' => $destination->getMorphClass(),
|
||||
'compose_parsing_version' => '5',
|
||||
'docker_compose_raw' => <<<'YAML'
|
||||
services:
|
||||
twenty:
|
||||
image: twentycrm/twenty:v1.15
|
||||
environment:
|
||||
- SERVICE_URL_TWENTY_3000
|
||||
worker:
|
||||
image: twentycrm/twenty:v1.15
|
||||
environment:
|
||||
- SERVICE_URL_TWENTY_3000
|
||||
YAML,
|
||||
]);
|
||||
});
|
||||
|
||||
it('keeps SERVICE_URL_TWENTY on the twenty domain when worker declares the same variable', function () {
|
||||
$this->service->parse();
|
||||
|
||||
$twenty = $this->service->applications()->where('name', 'twenty')->first();
|
||||
$twenty->update(['fqdn' => 'https://crm.example.com']);
|
||||
|
||||
$this->service->fresh()->parse();
|
||||
|
||||
$env = fn (string $key) => $this->service->environment_variables()->where('key', $key)->value('value');
|
||||
|
||||
expect($env('SERVICE_URL_TWENTY'))->toBe('https://crm.example.com')
|
||||
->and($env('SERVICE_FQDN_TWENTY'))->toBe('crm.example.com')
|
||||
->and($env('SERVICE_URL_TWENTY_3000'))->toBe('https://crm.example.com:3000')
|
||||
->and($this->service->applications()->where('name', 'worker')->value('fqdn'))->toBeNull();
|
||||
});
|
||||
|
||||
it('does not overwrite SERVICE_URL_TWENTY when the worker domain is saved', function () {
|
||||
$this->service->parse();
|
||||
$this->service->applications()->where('name', 'twenty')->first()->update(['fqdn' => 'https://crm.example.com']);
|
||||
$this->service->fresh()->parse();
|
||||
|
||||
// Installs parsed before the fix keep a generated domain on worker.
|
||||
$worker = $this->service->applications()->where('name', 'worker')->first();
|
||||
$worker->update(['fqdn' => 'http://twenty-stale.10.0.1.1.sslip.io']);
|
||||
|
||||
updateCompose($worker->fresh());
|
||||
|
||||
expect($this->service->environment_variables()->where('key', 'SERVICE_URL_TWENTY')->value('value'))
|
||||
->toBe('https://crm.example.com');
|
||||
});
|
||||
|
||||
it('keeps path routing when services declare the same variable with another path or port', function () {
|
||||
$this->service->update(['docker_compose_raw' => <<<'YAML'
|
||||
services:
|
||||
appwrite:
|
||||
image: appwrite/appwrite:1.7
|
||||
environment:
|
||||
- SERVICE_URL_APPWRITE=/v1
|
||||
appwrite-console:
|
||||
image: appwrite/console:6
|
||||
environment:
|
||||
- SERVICE_URL_APPWRITE_3000=/
|
||||
appwrite-realtime:
|
||||
image: appwrite/appwrite:1.7
|
||||
environment:
|
||||
- SERVICE_URL_APPWRITE=/v1/realtime
|
||||
YAML]);
|
||||
|
||||
$this->service->parse();
|
||||
|
||||
$domains = $this->service->applications()->pluck('fqdn', 'name');
|
||||
$host = str($domains['appwrite'])->before('/v1')->value();
|
||||
|
||||
expect($domains['appwrite'])->toEndWith('.sslip.io/v1')
|
||||
->and($domains['appwrite-console'])->toBe($host)
|
||||
->and($domains['appwrite-realtime'])->toBe("{$host}/v1/realtime");
|
||||
});
|
||||
|
||||
it('does not treat a build argument as a copy of the named application declaration', function () {
|
||||
$this->service->update(['docker_compose_raw' => <<<'YAML'
|
||||
services:
|
||||
twenty:
|
||||
image: twentycrm/twenty:v1.15
|
||||
environment:
|
||||
- SERVICE_URL_TWENTY_3000
|
||||
web:
|
||||
image: nginx:alpine
|
||||
build:
|
||||
context: .
|
||||
args:
|
||||
- SERVICE_URL_TWENTY_3000=/admin
|
||||
YAML]);
|
||||
|
||||
$this->service->parse();
|
||||
|
||||
expect($this->service->applications()->where('name', 'web')->value('fqdn'))->toEndWith('/admin');
|
||||
});
|
||||
|
||||
it('does not treat a service as a copy of a duplicate row with the same name', function () {
|
||||
$this->service->update(['docker_compose_raw' => <<<'YAML'
|
||||
services:
|
||||
app:
|
||||
image: nginx:alpine
|
||||
environment:
|
||||
- SERVICE_URL_APP_80
|
||||
YAML]);
|
||||
ServiceApplication::create(['name' => 'app', 'service_id' => $this->service->id, 'image' => 'nginx:alpine']);
|
||||
ServiceApplication::create(['name' => 'app', 'service_id' => $this->service->id, 'image' => 'nginx:alpine', 'is_migrated' => true, 'fqdn' => 'https://app.example.com']);
|
||||
|
||||
$this->service->parse();
|
||||
|
||||
expect($this->service->environment_variables()->where('key', 'SERVICE_URL_APP')->value('value'))
|
||||
->toBe('https://app.example.com');
|
||||
});
|
||||
|
||||
it('fixes SERVICE_URL_TWENTY for installs where worker already has a generated domain', function () {
|
||||
$this->service->parse();
|
||||
$this->service->applications()->where('name', 'twenty')->update(['fqdn' => 'https://crm.example.com']);
|
||||
$this->service->applications()->where('name', 'worker')->update(['fqdn' => 'http://twenty-stale.10.0.1.1.sslip.io']);
|
||||
$this->service->environment_variables()->where('key', 'SERVICE_URL_TWENTY')->first()->update(['value' => 'http://twenty-stale.10.0.1.1.sslip.io']);
|
||||
|
||||
$this->service->fresh()->parse();
|
||||
|
||||
expect($this->service->environment_variables()->where('key', 'SERVICE_URL_TWENTY')->value('value'))->toBe('https://crm.example.com')
|
||||
->and($this->service->applications()->where('name', 'worker')->value('fqdn'))->toBe('http://twenty-stale.10.0.1.1.sslip.io');
|
||||
});
|
||||
Reference in New Issue
Block a user