From 49086c82a8bc4d4c30baf524abecb791d3378da7 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 29 Sep 2026 11:10:22 +0200 Subject: [PATCH] fix(multi-server): block additional servers with a mismatched proxy (#12053) --- .../Api/ApplicationsController.php | 2 +- app/Jobs/ApplicationDeploymentJob.php | 33 +++++ app/Livewire/Project/Shared/Destination.php | 2 +- app/Models/Application.php | 23 +++- tests/Feature/MultiServerApplicationTest.php | 129 ++++++++++++++++++ 5 files changed, 186 insertions(+), 3 deletions(-) diff --git a/app/Http/Controllers/Api/ApplicationsController.php b/app/Http/Controllers/Api/ApplicationsController.php index 679e7c2581..c45e03a281 100644 --- a/app/Http/Controllers/Api/ApplicationsController.php +++ b/app/Http/Controllers/Api/ApplicationsController.php @@ -6182,7 +6182,7 @@ class ApplicationsController extends Controller return response()->json(['message' => 'A destination on this server is already attached.'], 422); } - if ($reason = $application->additionalServersUnavailableReason()) { + if ($reason = $application->additionalServersUnavailableReason($destination->server)) { return response()->json(['message' => $reason], 422); } diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index 6ff601cd17..5b405036ba 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -318,6 +318,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue $this->validateDeploymentEnvironmentVariableKeys(); $this->ensureRegistryImageForMultipleServers(); $this->warnAboutVolumesOnMultipleServers(); + $this->warnAboutMixedProxiesOnMultipleServers(); } catch (Exception $e) { $this->fail($e); throw $e; @@ -2171,6 +2172,22 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue $this->application_deployment_queue->addLogEntry('Warning: This application uses multiple servers and has persistent storage. Volumes are not shared between servers, so each server has its own data.', 'stderr'); } + /** + * Servers that were added before the proxy check can still use another proxy than the primary server. + */ + private function warnAboutMixedProxiesOnMultipleServers(): void + { + if ($this->pull_request_id !== 0) { + return; + } + + foreach ($this->application->additional_servers as $server) { + if ($reason = $this->application->proxyMismatchReason($server)) { + $this->application_deployment_queue->addLogEntry("Warning: {$reason} The domains of this application will not work through the proxy on {$server->name}.", 'stderr'); + } + } + } + /** * Additional servers pull the image that the main server pushes. Without a registry image, each * server builds its own image, so the servers can run different code. @@ -2691,6 +2708,21 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue $this->prepare_builder_image(firstTry: false); } + /** + * Proxy labels are generated for the primary server. On an additional server whose + * network has another name, point Caddy to the network the container joins there. + */ + private function useDestinationNetworkInCaddyLabels(Collection $labels): Collection + { + $primaryNetwork = $this->application->destination->network; + $network = $this->destination->network; + if ($network === $primaryNetwork) { + return $labels; + } + + return $labels->map(fn ($label) => $label === "caddy_ingress_network={$primaryNetwork}" ? "caddy_ingress_network={$network}" : $label); + } + /** * @return int The number of deployments that Coolify queued on additional servers */ @@ -3754,6 +3786,7 @@ COPY ./nginx.conf /etc/nginx/conf.d/default.conf"); if ($this->pull_request_id !== 0) { $labels = collect(generateLabelsApplication($this->application, $this->preview)); } + $labels = $this->useDestinationNetworkInCaddyLabels($labels); if ($this->application->settings->is_container_label_escape_enabled) { $labels = $labels->map(function ($value, $key) { return escapeDollarSign($value); diff --git a/app/Livewire/Project/Shared/Destination.php b/app/Livewire/Project/Shared/Destination.php index 55d3f64e84..24c3b51090 100644 --- a/app/Livewire/Project/Shared/Destination.php +++ b/app/Livewire/Project/Shared/Destination.php @@ -162,7 +162,7 @@ class Destination extends Component $network = StandaloneDocker::ownedByCurrentTeam()->where('server_id', $server->id)->findOrFail($network_id); $this->authorize('update', $this->resource); $reason = $this->resource instanceof Application - ? $this->resource->additionalServersUnavailableReason() + ? $this->resource->additionalServersUnavailableReason($server) : 'Only applications can use multiple servers.'; if ($reason) { $this->dispatch('error', 'Failed to add server.', $reason); diff --git a/app/Models/Application.php b/app/Models/Application.php index 255967644b..76a11f3b3a 100644 --- a/app/Models/Application.php +++ b/app/Models/Application.php @@ -4,6 +4,7 @@ namespace App\Models; use App\Enums\ApplicationDeploymentStatus; use App\Enums\BuildPackTypes; +use App\Enums\ProxyTypes; use App\Exceptions\DeploymentException; use App\Services\ConfigurationGenerator; use App\Services\DeploymentConfiguration\ApplicationConfigurationSnapshot; @@ -599,8 +600,9 @@ class Application extends BaseModel /** * The reason why this application cannot deploy to additional servers, or null if it can. + * When a server is given, also check that the server can route the application like the primary server. */ - public function additionalServersUnavailableReason(): ?string + public function additionalServersUnavailableReason(?Server $server = null): ?string { if ($this->build_pack === 'dockercompose') { return 'Docker Compose applications cannot use multiple servers.'; @@ -608,10 +610,29 @@ class Application extends BaseModel if ($this->persistentStorages()->exists()) { return 'Applications with persistent storage cannot use multiple servers because volumes are not shared between servers.'; } + if ($server) { + return $this->proxyMismatchReason($server); + } return null; } + /** + * The reason why the server cannot route this application, or null if it can. + * Proxy labels are generated for the primary server, so Traefik labels do not work on a Caddy server and the reverse. + */ + public function proxyMismatchReason(Server $server): ?string + { + $routingProxies = [ProxyTypes::TRAEFIK->value, ProxyTypes::CADDY->value]; + $primaryProxy = $this->destination?->server?->proxyType(); + $serverProxy = $server->proxyType(); + if (! in_array($primaryProxy, $routingProxies, true) || ! in_array($serverProxy, $routingProxies, true) || $primaryProxy === $serverProxy) { + return null; + } + + return "The primary server uses {$primaryProxy} and {$server->name} uses {$serverProxy}. All servers of an application must use the same proxy, because they share the same proxy labels."; + } + public function is_public_repository(): bool { if (data_get($this, 'source.is_public')) { diff --git a/tests/Feature/MultiServerApplicationTest.php b/tests/Feature/MultiServerApplicationTest.php index fe6eef7b9e..13cac656f7 100644 --- a/tests/Feature/MultiServerApplicationTest.php +++ b/tests/Feature/MultiServerApplicationTest.php @@ -2,6 +2,7 @@ use App\Actions\Application\StopApplication; use App\Enums\ApplicationDeploymentStatus; +use App\Enums\ProxyTypes; use App\Exceptions\DeploymentException; use App\Jobs\ApplicationDeploymentJob; use App\Livewire\Project\Database\Sqlite\ConnectApplication; @@ -157,6 +158,37 @@ describe('volume warning', function () { }); }); +describe('mixed proxy warning', function () { + test('warns in the deployment log when an additional server uses another proxy', function () { + $this->mainServer->proxy->set('type', ProxyTypes::TRAEFIK->value); + $this->mainServer->save(); + $this->extraServer->proxy->set('type', ProxyTypes::CADDY->value); + $this->extraServer->save(); + $logEntries = []; + $job = makeVolumeWarningJob($this->application->fresh(), $logEntries); + + callMultiServerDeploymentJob($job, 'warnAboutMixedProxiesOnMultipleServers'); + + expect($logEntries)->toBe([[ + 'Warning: The primary server uses TRAEFIK and extra-server uses CADDY. All servers of an application must use the same proxy, because they share the same proxy labels. The domains of this application will not work through the proxy on extra-server.', + 'stderr', + ]]); + }); + + test('does not warn when all servers use the same proxy', function () { + $this->mainServer->proxy->set('type', ProxyTypes::CADDY->value); + $this->mainServer->save(); + $this->extraServer->proxy->set('type', ProxyTypes::CADDY->value); + $this->extraServer->save(); + $logEntries = []; + $job = makeVolumeWarningJob($this->application->fresh(), $logEntries); + + callMultiServerDeploymentJob($job, 'warnAboutMixedProxiesOnMultipleServers'); + + expect($logEntries)->toBe([]); + }); +}); + describe('combined deployment notification', function () { test('links queued additional deployments to the main deployment', function () { $parent = createMultiServerDeployment($this->application, $this->mainServer, ApplicationDeploymentStatus::FINISHED->value); @@ -364,6 +396,103 @@ describe('adding a server in the UI', function () { expect($this->application->fresh()->additional_networks)->toHaveCount(1); }); + + test('rejects a server that uses a different proxy than the primary server', function () { + $this->mainServer->proxy->set('type', ProxyTypes::TRAEFIK->value); + $this->mainServer->save(); + $this->extraServer->proxy->set('type', ProxyTypes::CADDY->value); + $this->extraServer->save(); + + Livewire::test(Destination::class, ['resource' => $this->application->fresh()]) + ->call('addServer', $this->extraDestination->id, $this->extraServer->id) + ->assertDispatched('error'); + + expect($this->application->fresh()->additional_networks)->toHaveCount(0); + }); + + test('adds a server without a proxy next to a primary server with a proxy', function () { + $this->mainServer->proxy->set('type', ProxyTypes::TRAEFIK->value); + $this->mainServer->save(); + $this->extraServer->proxy->set('type', ProxyTypes::NONE->value); + $this->extraServer->save(); + + Livewire::test(Destination::class, ['resource' => $this->application->fresh()]) + ->call('addServer', $this->extraDestination->id, $this->extraServer->id); + + expect($this->application->fresh()->additional_networks)->toHaveCount(1); + }); +}); + +describe('adding a server through the API', function () { + beforeEach(function () { + $this->application->additional_networks()->detach(); + config(['app.maintenance.store' => 'array']); + InstanceSettings::query()->whereKey(0)->update(['is_api_enabled' => true]); + session(['currentTeam' => $this->team]); + $this->token = $this->user->createToken('test-token', ['*']); + }); + + test('rejects a server that uses a different proxy than the primary server', function () { + $this->mainServer->proxy->set('type', ProxyTypes::CADDY->value); + $this->mainServer->save(); + $this->extraServer->proxy->set('type', ProxyTypes::TRAEFIK->value); + $this->extraServer->save(); + + $this->withHeader('Authorization', 'Bearer '.$this->token->plainTextToken) + ->postJson("/api/v1/applications/{$this->application->uuid}/destinations", [ + 'destination_uuid' => $this->extraDestination->uuid, + ]) + ->assertStatus(422); + + expect($this->application->fresh()->additional_networks)->toHaveCount(0); + }); + + test('adds a server that uses the same proxy as the primary server', function () { + $this->mainServer->proxy->set('type', ProxyTypes::CADDY->value); + $this->mainServer->save(); + $this->extraServer->proxy->set('type', ProxyTypes::CADDY->value); + $this->extraServer->save(); + + $this->withHeader('Authorization', 'Bearer '.$this->token->plainTextToken) + ->postJson("/api/v1/applications/{$this->application->uuid}/destinations", [ + 'destination_uuid' => $this->extraDestination->uuid, + ]) + ->assertStatus(201); + + expect($this->application->fresh()->additional_networks)->toHaveCount(1); + }); +}); + +describe('caddy network label on additional servers', function () { + test('uses the network of the server that is deployed', function () { + $this->extraDestination->update(['network' => 'extra-network']); + $labels = collect([ + "caddy_ingress_network={$this->mainDestination->network}", + 'caddy_0=app.example.com', + ]); + + $job = makeMultiServerDeploymentJob([ + 'application' => $this->application, + 'destination' => $this->extraDestination->fresh(), + ]); + + expect(callMultiServerDeploymentJob($job, 'useDestinationNetworkInCaddyLabels', $labels)->all())->toBe([ + 'caddy_ingress_network=extra-network', + 'caddy_0=app.example.com', + ]); + }); + + test('keeps the labels on the primary server', function () { + $labels = collect(["caddy_ingress_network={$this->mainDestination->network}"]); + + $job = makeMultiServerDeploymentJob([ + 'application' => $this->application, + 'destination' => $this->mainDestination, + ]); + + expect(callMultiServerDeploymentJob($job, 'useDestinationNetworkInCaddyLabels', $labels)->all()) + ->toBe(["caddy_ingress_network={$this->mainDestination->network}"]); + }); }); describe('adding a volume to an application with additional servers', function () {