mirror of
https://github.com/coollabsio/coolify.git
synced 2026-11-04 10:42:23 +00:00
fix(server): allow servers with the same IP and different SSH ports
Closes #7573 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
21d1ad4874
commit
ecb14e6cd5
12 files changed
+186
-34
No files matched your search
@@ -249,7 +249,7 @@ class ServerTransferController extends Controller
|
||||
'message' => $schemaVersion === ServerTransferBundle::SCHEMA_VERSION ? null
|
||||
: 'This instance reads transfer bundle version '.ServerTransferBundle::SCHEMA_VERSION.", but the source sends version {$schemaVersion}. Upgrade both instances to the same Coolify version.",
|
||||
],
|
||||
...collect($this->importer->importChecks((string) $request->query('server_uuid'), $ip, $teamId))
|
||||
...collect($this->importer->importChecks((string) $request->query('server_uuid'), $ip, $teamId, serverPort: $port))
|
||||
->map(fn (array $check) => collect($check)->only(['label', 'ok', 'message'])->all())
|
||||
->all(),
|
||||
[
|
||||
|
||||
@@ -620,9 +620,8 @@ class ServersController extends Controller
|
||||
if (! $privateKey) {
|
||||
return response()->json(['message' => 'Private key not found.'], 404);
|
||||
}
|
||||
$foundServer = ModelsServer::whereIp(ModelsServer::normalizeIp($request->ip))->first();
|
||||
if ($foundServer) {
|
||||
return response()->json(['message' => 'A server with this IP/Domain is already in use.'], 400);
|
||||
if (ModelsServer::sshEndpointInUse($request->ip, $request->port)) {
|
||||
return response()->json(['message' => 'A server with this IP/Domain and SSH port is already in use.'], 400);
|
||||
}
|
||||
if ($request->instant_validate) {
|
||||
ModelsServer::throttleValidation($teamId, $request->ip);
|
||||
@@ -812,6 +811,13 @@ class ServersController extends Controller
|
||||
'errors' => $connectionErrors,
|
||||
], 422);
|
||||
}
|
||||
$newIp = $request->filled('ip') ? $request->input('ip') : $server->getRawOriginal('ip');
|
||||
$newPort = $request->filled('port') ? (int) $request->input('port') : (int) $server->port;
|
||||
$endpointChanged = ModelsServer::normalizeIp($newIp) !== ModelsServer::normalizeIp($server->getRawOriginal('ip'))
|
||||
|| $newPort !== (int) $server->port;
|
||||
if ($endpointChanged && ModelsServer::sshEndpointInUse($newIp, $newPort, $server->id)) {
|
||||
return response()->json(['message' => 'A server with this IP/Domain and SSH port is already in use.'], 400);
|
||||
}
|
||||
if ($request->instant_validate) {
|
||||
ModelsServer::throttleValidation($teamId, $request->ip ?? $server->ip);
|
||||
}
|
||||
|
||||
@@ -302,9 +302,8 @@ class Index extends Component
|
||||
|
||||
$this->validate();
|
||||
|
||||
$foundServer = Server::whereIp(Server::normalizeIp($this->remoteServerHost))->first();
|
||||
if ($foundServer) {
|
||||
return $this->dispatch('error', 'A server with this IP/Domain already exists.');
|
||||
if (Server::sshEndpointInUse($this->remoteServerHost, $this->remoteServerPort)) {
|
||||
return $this->dispatch('error', 'A server with this IP/Domain and SSH port already exists.');
|
||||
}
|
||||
$privateKeyId = $this->createdPrivateKey?->id ?? $this->selectedExistingPrivateKey;
|
||||
$this->createdPrivateKey = PrivateKey::ownedByCurrentTeam(['team_id'])->findOrFail($privateKeyId);
|
||||
|
||||
@@ -144,9 +144,8 @@ class ByIp extends Component
|
||||
try {
|
||||
$this->authorize('create', Server::class);
|
||||
$this->ip = Server::normalizeIp($this->ip);
|
||||
$foundServer = Server::whereIp($this->ip)->first();
|
||||
if ($foundServer) {
|
||||
return $this->dispatch('error', 'A server with this IP/Domain already exists.');
|
||||
if (Server::sshEndpointInUse($this->ip, $this->port)) {
|
||||
return $this->dispatch('error', 'A server with this IP/Domain and SSH port already exists.');
|
||||
}
|
||||
|
||||
if (is_null($this->private_key_id)) {
|
||||
|
||||
@@ -250,14 +250,13 @@ class Show extends Component
|
||||
$this->port = $this->server->port;
|
||||
throw new \Exception('The IP address, SSH user, and SSH port of a transferred server cannot be changed.');
|
||||
}
|
||||
// Check only a changed IP, so that an old duplicate does not block saving other fields.
|
||||
$ipChanged = $this->ip !== Server::normalizeIp($this->server->getRawOriginal('ip'));
|
||||
$foundServer = $ipChanged ? Server::where('ip', $this->ip)
|
||||
->where('id', '!=', $this->server->id)
|
||||
->first() : null;
|
||||
if ($foundServer) {
|
||||
// Check only a changed IP or port, so that an old duplicate does not block saving other fields.
|
||||
$endpointChanged = $this->ip !== Server::normalizeIp($this->server->getRawOriginal('ip'))
|
||||
|| (int) $this->port !== (int) $this->server->port;
|
||||
if ($endpointChanged && Server::sshEndpointInUse($this->ip, $this->port, $this->server->id)) {
|
||||
$this->ip = $this->server->ip;
|
||||
throw new \Exception('A server with this IP/Domain already exists.');
|
||||
$this->port = $this->server->port;
|
||||
throw new \Exception('A server with this IP/Domain and SSH port already exists.');
|
||||
}
|
||||
|
||||
$this->server->name = $this->name;
|
||||
|
||||
@@ -2055,6 +2055,19 @@ $siteAddress {
|
||||
{
|
||||
return Attribute::make(
|
||||
get: function () {
|
||||
/**
|
||||
* Whether a server of any team already uses this SSH endpoint. The port is part of the endpoint,
|
||||
* so servers behind one NAT address with different forwarded SSH ports are different servers.
|
||||
*/
|
||||
public static function sshEndpointInUse(?string $ip, int|string|null $port, ?int $exceptServerId = null): bool
|
||||
{
|
||||
return self::query()
|
||||
->where('ip', self::normalizeIp($ip))
|
||||
->where('port', (int) ($port ?? 22))
|
||||
->when($exceptServerId !== null, fn ($query) => $query->whereKeyNot($exceptServerId))
|
||||
->exists();
|
||||
}
|
||||
|
||||
if (isDev()) {
|
||||
return '127.0.0.1';
|
||||
}
|
||||
|
||||
@@ -201,14 +201,15 @@ class ServerTransferImporter
|
||||
|
||||
$serverUuid = (string) data_get($bundle, 'server.uuid');
|
||||
$serverIp = (string) data_get($bundle, 'server.ip');
|
||||
$serverPort = (int) data_get($bundle, 'server.port', 22);
|
||||
|
||||
// A server that comes back replaces the archived copy that this instance kept from the first transfer.
|
||||
$archive = $preserveUuids ? $this->replaceableArchive($serverUuid, $teamId) : null;
|
||||
$uuidConflict = $archive === null && $preserveUuids && Server::withTrashed()->where('uuid', $serverUuid)->exists();
|
||||
$ipConflict = Server::where('ip', Server::normalizeIp($serverIp))->when($archive, fn ($query) => $query->whereKeyNot($archive->id))->exists();
|
||||
$ipConflict = Server::sshEndpointInUse($serverIp, $serverPort, $archive?->id);
|
||||
|
||||
if (! $dryRun) {
|
||||
$this->assertCanImport($serverUuid, $serverIp, $teamId, $preserveUuids);
|
||||
$this->assertCanImport($serverUuid, $serverIp, $teamId, $preserveUuids, $serverPort);
|
||||
// The import validates the server over SSH, so it uses the same Cloud limit as a new server.
|
||||
Server::throttleValidation($teamId, $serverIp);
|
||||
}
|
||||
@@ -217,7 +218,7 @@ class ServerTransferImporter
|
||||
$warnings[] = "A server with UUID {$serverUuid} already exists on this instance.";
|
||||
}
|
||||
if ($ipConflict) {
|
||||
$warnings[] = "A server with IP/domain {$serverIp} already exists on this instance.";
|
||||
$warnings[] = "A server with IP/domain {$serverIp} and SSH port {$serverPort} already exists on this instance.";
|
||||
}
|
||||
|
||||
$created = [
|
||||
@@ -503,7 +504,7 @@ class ServerTransferImporter
|
||||
*
|
||||
* @return list<array{field: string, label: string, ok: bool, message: string|null}>
|
||||
*/
|
||||
public function importChecks(string $serverUuid, string $serverIp, int $teamId, bool $preserveUuids = true): array
|
||||
public function importChecks(string $serverUuid, string $serverIp, int $teamId, bool $preserveUuids = true, int $serverPort = 22): array
|
||||
{
|
||||
$team = Team::find($teamId);
|
||||
$archive = $preserveUuids ? $this->replaceableArchive($serverUuid, $teamId) : null;
|
||||
@@ -518,7 +519,7 @@ class ServerTransferImporter
|
||||
default => "A server with UUID {$serverUuid} already exists on this instance. Delete/transfer it first, or import with preserve_uuids=false.",
|
||||
};
|
||||
|
||||
$ipExists = Server::where('ip', Server::normalizeIp($serverIp))->when($archive, fn ($query) => $query->whereKeyNot($archive->id))->exists();
|
||||
$ipExists = Server::sshEndpointInUse($serverIp, $serverPort, $archive?->id);
|
||||
|
||||
return [
|
||||
[
|
||||
@@ -535,9 +536,9 @@ class ServerTransferImporter
|
||||
],
|
||||
[
|
||||
'field' => 'server.ip',
|
||||
'label' => 'no server with the same IP exists',
|
||||
'label' => 'no server with the same IP and SSH port exists',
|
||||
'ok' => ! $ipExists,
|
||||
'message' => $ipExists ? "A server with IP/domain {$serverIp} already exists on this instance. Remove it first." : null,
|
||||
'message' => $ipExists ? "A server with IP/domain {$serverIp} and SSH port {$serverPort} already exists on this instance. Remove it first." : null,
|
||||
],
|
||||
];
|
||||
}
|
||||
@@ -572,9 +573,9 @@ class ServerTransferImporter
|
||||
/**
|
||||
* @throws ValidationException with the first failed import check
|
||||
*/
|
||||
public function assertCanImport(string $serverUuid, string $serverIp, int $teamId, bool $preserveUuids = true): void
|
||||
public function assertCanImport(string $serverUuid, string $serverIp, int $teamId, bool $preserveUuids = true, int $serverPort = 22): void
|
||||
{
|
||||
foreach ($this->importChecks($serverUuid, $serverIp, $teamId, $preserveUuids) as $check) {
|
||||
foreach ($this->importChecks($serverUuid, $serverIp, $teamId, $preserveUuids, $serverPort) as $check) {
|
||||
if (! $check['ok']) {
|
||||
throw ValidationException::withMessages([$check['field'] => [$check['message']]]);
|
||||
}
|
||||
|
||||
@@ -634,7 +634,7 @@ describe('GET /api/v1/servers/import/check', function () {
|
||||
test('lists every check with its result when a server already exists on this instance', function () {
|
||||
$listener = stream_socket_server('tcp://127.0.0.1:0');
|
||||
$port = (int) substr(strrchr(stream_socket_get_name($listener, false), ':'), 1);
|
||||
$this->server->forceFill(['ip' => '127.0.0.1'])->save();
|
||||
$this->server->forceFill(['ip' => '127.0.0.1', 'port' => $port])->save();
|
||||
|
||||
$checks = $this->withHeaders(transferHeaders($this->importToken))
|
||||
->getJson("/api/v1/servers/import/check?server_uuid=new-server-uuid&ip=127.0.0.1&port={$port}")
|
||||
@@ -645,12 +645,26 @@ describe('GET /api/v1/servers/import/check', function () {
|
||||
expect(collect($checks)->pluck('ok', 'label')->all())->toMatchArray([
|
||||
'the transfer bundle version is supported' => true,
|
||||
'no server with the same UUID exists' => true,
|
||||
'no server with the same IP exists' => false,
|
||||
'no server with the same IP and SSH port exists' => false,
|
||||
"the target can reach the SSH port of the server (127.0.0.1:{$port})" => true,
|
||||
]);
|
||||
fclose($listener);
|
||||
});
|
||||
|
||||
test('accepts a server with the IP of an existing server and a different SSH port', function () {
|
||||
$listener = stream_socket_server('tcp://127.0.0.1:0');
|
||||
$port = (int) substr(strrchr(stream_socket_get_name($listener, false), ':'), 1);
|
||||
$this->server->forceFill(['ip' => '127.0.0.1', 'port' => 22])->save();
|
||||
|
||||
$checks = $this->withHeaders(transferHeaders($this->importToken))
|
||||
->getJson("/api/v1/servers/import/check?server_uuid=new-server-uuid&ip=127.0.0.1&port={$port}")
|
||||
->assertOk()
|
||||
->json('checks');
|
||||
|
||||
expect(collect($checks)->pluck('ok', 'label')->get('no server with the same IP and SSH port exists'))->toBeTrue();
|
||||
fclose($listener);
|
||||
});
|
||||
|
||||
test('on Cloud, does not probe private, loopback, or link-local addresses', function (string $ip) {
|
||||
config()->set('constants.coolify.self_hosted', false);
|
||||
|
||||
|
||||
@@ -68,7 +68,7 @@ test('server creation returns a consistent duplicate address response', function
|
||||
$ownResponse->assertBadRequest();
|
||||
$otherResponse->assertBadRequest();
|
||||
expect($ownResponse->json('message'))
|
||||
->toBe('A server with this IP/Domain is already in use.')
|
||||
->toBe('A server with this IP/Domain and SSH port is already in use.')
|
||||
->toBe($otherResponse->json('message'));
|
||||
});
|
||||
|
||||
|
||||
@@ -1,18 +1,22 @@
|
||||
<?php
|
||||
|
||||
use App\Livewire\Server\New\ByIp;
|
||||
use App\Livewire\Server\Show;
|
||||
use App\Models\InstanceSettings;
|
||||
use App\Models\PrivateKey;
|
||||
use App\Models\Server;
|
||||
use App\Models\Team;
|
||||
use App\Models\User;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
use Illuminate\Support\Facades\Auth;
|
||||
use Illuminate\Support\Facades\Process;
|
||||
use Livewire\Livewire;
|
||||
|
||||
uses(RefreshDatabase::class);
|
||||
|
||||
beforeEach(function () {
|
||||
InstanceSettings::forceCreate(['id' => 0]);
|
||||
Process::fake();
|
||||
InstanceSettings::forceCreate(['id' => 0, 'is_api_enabled' => true]);
|
||||
|
||||
$this->user = User::factory()->create();
|
||||
$this->team = Team::factory()->create();
|
||||
@@ -23,6 +27,10 @@ beforeEach(function () {
|
||||
$this->privateKey = PrivateKey::factory()->create([
|
||||
'team_id' => $this->team->id,
|
||||
]);
|
||||
|
||||
$token = $this->user->createToken('write-token', ['write']);
|
||||
$token->accessToken->forceFill(['team_id' => $this->team->id])->save();
|
||||
$this->token = $token->plainTextToken;
|
||||
});
|
||||
|
||||
it('detects duplicate ip within the same team', function () {
|
||||
@@ -66,7 +74,7 @@ it('rejects a duplicate ip from the same team when adding a server', function ()
|
||||
->set('ip', '1.2.3.4')
|
||||
->set('private_key_id', $this->privateKey->id)
|
||||
->call('submit')
|
||||
->assertDispatched('error', 'A server with this IP/Domain already exists.');
|
||||
->assertDispatched('error', 'A server with this IP/Domain and SSH port already exists.');
|
||||
|
||||
expect(Server::where('ip', '1.2.3.4')->count())->toBe(1);
|
||||
});
|
||||
@@ -86,7 +94,7 @@ it('rejects a duplicate ip from another team without revealing the owner team',
|
||||
->set('ip', '5.6.7.8')
|
||||
->set('private_key_id', $this->privateKey->id)
|
||||
->call('submit')
|
||||
->assertDispatched('error', 'A server with this IP/Domain already exists.')
|
||||
->assertDispatched('error', 'A server with this IP/Domain and SSH port already exists.')
|
||||
->assertNotDispatched('error', 'A server with this IP/Domain is already in use by another team.');
|
||||
|
||||
expect(Server::where('ip', '5.6.7.8')->count())->toBe(1);
|
||||
@@ -116,3 +124,116 @@ it('enforces global uniqueness not just team-scoped', function () {
|
||||
->first();
|
||||
expect($teamScopedServer)->toBeNull();
|
||||
});
|
||||
|
||||
it('adds a server with the IP of another server and a different SSH port', function () {
|
||||
Server::factory()->create([
|
||||
'ip' => '1.2.3.4',
|
||||
'port' => 22,
|
||||
'team_id' => $this->team->id,
|
||||
'private_key_id' => $this->privateKey->id,
|
||||
]);
|
||||
|
||||
Livewire::test(ByIp::class, [
|
||||
'private_keys' => collect([$this->privateKey]),
|
||||
'limit_reached' => false,
|
||||
])
|
||||
->set('name', 'nat-vm-2')
|
||||
->set('ip', '1.2.3.4')
|
||||
->set('port', 2222)
|
||||
->set('private_key_id', $this->privateKey->id)
|
||||
->call('submit')
|
||||
->assertNotDispatched('error');
|
||||
|
||||
expect(Server::where('ip', '1.2.3.4')->orderBy('port')->pluck('port')->all())->toBe([22, 2222]);
|
||||
});
|
||||
|
||||
it('rejects a server with the IP and SSH port of a server of another team', function () {
|
||||
Server::factory()->create([
|
||||
'ip' => '5.6.7.8',
|
||||
'port' => 2222,
|
||||
'team_id' => Team::factory()->create()->id,
|
||||
]);
|
||||
|
||||
Livewire::test(ByIp::class, [
|
||||
'private_keys' => collect([$this->privateKey]),
|
||||
'limit_reached' => false,
|
||||
])
|
||||
->set('ip', '5.6.7.8')
|
||||
->set('port', 2222)
|
||||
->set('private_key_id', $this->privateKey->id)
|
||||
->call('submit')
|
||||
->assertDispatched('error', 'A server with this IP/Domain and SSH port already exists.');
|
||||
|
||||
expect(Server::where('ip', '5.6.7.8')->count())->toBe(1);
|
||||
});
|
||||
|
||||
it('rejects changing the SSH port of a server to the endpoint of another server', function () {
|
||||
Server::factory()->create([
|
||||
'ip' => '1.2.3.4',
|
||||
'port' => 2222,
|
||||
'team_id' => $this->team->id,
|
||||
'private_key_id' => $this->privateKey->id,
|
||||
]);
|
||||
$server = Server::factory()->create([
|
||||
'ip' => '1.2.3.4',
|
||||
'port' => 22,
|
||||
'team_id' => $this->team->id,
|
||||
'private_key_id' => $this->privateKey->id,
|
||||
]);
|
||||
|
||||
Livewire::test(Show::class, ['server_uuid' => $server->uuid])
|
||||
->set('port', '2222')
|
||||
->call('submit')
|
||||
->assertDispatched('error')
|
||||
->assertNotDispatched('success');
|
||||
|
||||
expect($server->fresh()->port)->toBe(22);
|
||||
});
|
||||
|
||||
it('creates a server through the API with the IP of another server and a different SSH port', function () {
|
||||
Auth::forgetGuards();
|
||||
Server::factory()->create([
|
||||
'ip' => '1.2.3.4',
|
||||
'port' => 22,
|
||||
'team_id' => $this->team->id,
|
||||
'private_key_id' => $this->privateKey->id,
|
||||
]);
|
||||
|
||||
$payload = [
|
||||
'name' => 'nat-vm',
|
||||
'ip' => '1.2.3.4',
|
||||
'private_key_uuid' => $this->privateKey->uuid,
|
||||
];
|
||||
|
||||
$this->withToken($this->token)->postJson('/api/v1/servers', $payload)
|
||||
->assertBadRequest()
|
||||
->assertJsonPath('message', 'A server with this IP/Domain and SSH port is already in use.');
|
||||
$this->withToken($this->token)->postJson('/api/v1/servers', [...$payload, 'port' => 2222])
|
||||
->assertCreated();
|
||||
|
||||
expect(Server::where('ip', '1.2.3.4')->orderBy('port')->pluck('port')->all())->toBe([22, 2222]);
|
||||
});
|
||||
|
||||
it('rejects an API update to the IP and SSH port of another server', function () {
|
||||
Auth::forgetGuards();
|
||||
Server::factory()->create([
|
||||
'ip' => '1.2.3.4',
|
||||
'port' => 2222,
|
||||
'team_id' => Team::factory()->create()->id,
|
||||
]);
|
||||
$server = Server::factory()->create([
|
||||
'ip' => '9.9.9.9',
|
||||
'port' => 22,
|
||||
'team_id' => $this->team->id,
|
||||
'private_key_id' => $this->privateKey->id,
|
||||
]);
|
||||
|
||||
$this->withToken($this->token)->patchJson('/api/v1/servers/'.$server->uuid, ['ip' => '1.2.3.4', 'port' => 2222])
|
||||
->assertBadRequest()
|
||||
->assertJsonPath('message', 'A server with this IP/Domain and SSH port is already in use.');
|
||||
expect($server->fresh()->only(['ip', 'port']))->toBe(['ip' => '9.9.9.9', 'port' => 22]);
|
||||
|
||||
$this->withToken($this->token)->patchJson('/api/v1/servers/'.$server->uuid, ['ip' => '1.2.3.4'])
|
||||
->assertCreated();
|
||||
expect($server->fresh()->ip)->toBe('1.2.3.4');
|
||||
});
|
||||
@@ -89,7 +89,7 @@ it('treats IPv6 notations as the same IP when adding a server', function () {
|
||||
->set('ip', '2001:0DB8::1')
|
||||
->set('private_key_id', $this->privateKey->id)
|
||||
->call('submit')
|
||||
->assertDispatched('error', 'A server with this IP/Domain already exists.');
|
||||
->assertDispatched('error', 'A server with this IP/Domain and SSH port already exists.');
|
||||
|
||||
expect(Server::query()->where('ip', '2001:db8::1')->count())->toBe(1);
|
||||
});
|
||||
|
||||
@@ -191,6 +191,6 @@ it('does not disclose another team through the duplicate server IP message', fun
|
||||
Livewire::test(ByIp::class, ['private_keys' => collect([$this->privateKey])])
|
||||
->set('ip', $ip)
|
||||
->call('submit')
|
||||
->assertDispatched('error', 'A server with this IP/Domain already exists.');
|
||||
->assertDispatched('error', 'A server with this IP/Domain and SSH port already exists.');
|
||||
}
|
||||
});
|
||||
Reference in new issue
Block a user