From ee62f64addf4fa652360a9efa225ec56779ae5d6 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Sun, 27 Sep 2026 16:53:54 +0200 Subject: [PATCH] fix(compose): use external volumes as declared Compose volumes declared with external: true (or external: {name: x}) were renamed to _, so containers got a new empty volume instead of the existing one. All Compose parsers (applications, previews, services, and the legacy parsers) now use them as written: no uuid prefix, no preview suffix, the top-level declaration kept, and no tracking row, so Coolify never removes a shared external volume. External volume names must be literal; ${VAR} names are rejected. Existing _ volumes and their rows are kept on redeploy. Co-Authored-By: Claude Opus 5.5 --- app/Models/ApplicationPreview.php | 2 +- bootstrap/helpers/parsers.php | 94 ++++ bootstrap/helpers/shared.php | 24 +- .../ApplicationPreviewComposeCleanupTest.php | 2 +- tests/Feature/ComposeExternalVolumeTest.php | 434 ++++++++++++++++++ tests/Unit/PreviewDeploymentBindMountTest.php | 3 +- 6 files changed, 552 insertions(+), 7 deletions(-) create mode 100644 tests/Feature/ComposeExternalVolumeTest.php diff --git a/app/Models/ApplicationPreview.php b/app/Models/ApplicationPreview.php index 0d862b2829..f67497bf34 100644 --- a/app/Models/ApplicationPreview.php +++ b/app/Models/ApplicationPreview.php @@ -112,7 +112,7 @@ class ApplicationPreview extends BaseModel return $volumes ->map(function (mixed $volume, int|string $key) use ($prefix, $suffix): ?string { $key = (string) $key; - if (filter_var(data_get($volume, 'external', false), FILTER_VALIDATE_BOOLEAN)) { + if (isComposeExternalVolume($volume)) { return null; } if (data_get($volume, 'name', $key) !== $key) { diff --git a/bootstrap/helpers/parsers.php b/bootstrap/helpers/parsers.php index 8a203a3d0d..3cb7cd6fba 100644 --- a/bootstrap/helpers/parsers.php +++ b/bootstrap/helpers/parsers.php @@ -105,6 +105,14 @@ function validateDockerComposeForInjection(string $composeYaml, ?string $resourc } } } + + if (isset($parsed['volumes']) && is_array($parsed['volumes'])) { + foreach ($parsed['volumes'] as $volumeName => $volumeConfig) { + if (isComposeExternalVolume($volumeConfig)) { + validateComposeExternalVolume($volumeName, $volumeConfig); + } + } + } } /** @@ -302,6 +310,80 @@ function validateComposeNetworkName(string $networkName, string $context = 'netw } } +/** + * Tells if a top-level Compose volume declaration is external: `external: true` or the old + * `external: {name: x}` syntax. Docker Compose does not create or remove an external volume. + */ +function isComposeExternalVolume(mixed $declaration): bool +{ + if (! is_array($declaration) || ! array_key_exists('external', $declaration)) { + return false; + } + $external = $declaration['external']; + if (is_array($external)) { + return true; + } + + return filter_var($external, FILTER_VALIDATE_BOOLEAN); +} + +/** + * Returns the top-level declaration of a volume source when the Compose file declares it as + * external, and null for all other sources. The parsers use an external volume as written: no + * "{uuid}_" prefix, no preview suffix and no LocalPersistentVolume row, so that Coolify never + * removes a volume that it does not own. + * + * @param iterable $topLevelVolumes + * @return array|null + * + * @throws Exception If the external volume has a name that is not safe (see validateComposeExternalVolume()) + */ +function composeExternalVolumeDeclaration(iterable $topLevelVolumes, string $source): ?array +{ + $declaration = collect($topLevelVolumes)->get($source); + if (! isComposeExternalVolume($declaration)) { + return null; + } + validateComposeExternalVolume($source, $declaration); + + return $declaration; +} + +/** + * The key and the name of an external volume must be literal Docker volume names. Docker Compose + * can resolve variables in `name:`, but then only the deployment `.env` sets which existing volume + * the resource mounts, and Coolify cannot show or check that volume. Thus variables are rejected. + * + * @param array $declaration + * + * @throws Exception If the key or the name is not a literal, valid Docker volume name + */ +function validateComposeExternalVolume(int|string $key, array $declaration): void +{ + $key = (string) $key; + $label = preg_match(ValidationPatterns::VOLUME_NAME_PATTERN, $key) === 1 ? "volume {$key}" : 'volume'; + $names = [$key]; + if (array_key_exists('name', $declaration)) { + $names[] = $declaration['name']; + } + if (is_array($declaration['external'] ?? null) && array_key_exists('name', $declaration['external'])) { + $names[] = $declaration['external']['name']; + } + + foreach ($names as $name) { + if (is_string($name) && str_contains($name, '$')) { + throw new Exception( + "Invalid external Docker Compose {$label}: Coolify does not resolve variables in the name of an external volume. Use the literal name of the existing Docker volume." + ); + } + if (! is_string($name) || preg_match(ValidationPatterns::VOLUME_NAME_PATTERN, $name) !== 1) { + throw new Exception( + "Invalid external Docker Compose {$label}. Volume names must start with an alphanumeric character and contain only alphanumeric characters, dots, hyphens, and underscores." + ); + } + } +} + /** * Validates a Docker volume string (format: "source:target" or "source:target:mode") * @@ -1119,6 +1201,12 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int } } } elseif ($type->value() === 'volume') { + if (composeExternalVolumeDeclaration($topLevel->get('volumes'), $source->value()) !== null) { + // Previews share the external volume. It gets no row, so Coolify never removes it. + $volumesParsed->put($index, $volume); + + continue; + } if ($topLevel->get('volumes')->has($source->value())) { $temp = $topLevel->get('volumes')->get($source->value()); if (data_get($temp, 'driver_opts.type') === 'cifs') { @@ -2468,6 +2556,12 @@ function serviceParser(Service $resource): Collection } } } elseif ($type->value() === 'volume') { + if (composeExternalVolumeDeclaration($topLevel->get('volumes'), $source->value()) !== null) { + // The external volume gets no row, so Coolify never removes it. + $volumesParsed->put($index, $volume); + + continue; + } if ($topLevel->get('volumes')->has($source->value())) { $temp = $topLevel->get('volumes')->get($source->value()); if (data_get($temp, 'driver_opts.type') === 'cifs') { diff --git a/bootstrap/helpers/shared.php b/bootstrap/helpers/shared.php index affdc1faaf..afc15e20e5 100644 --- a/bootstrap/helpers/shared.php +++ b/bootstrap/helpers/shared.php @@ -2867,6 +2867,10 @@ function parseDockerComposeFile(Service|Application $resource, bool $isNew = fal ] ); } elseif ($type->value() === 'volume') { + if (composeExternalVolumeDeclaration($topLevelVolumes, $source->value()) !== null) { + // The external volume gets no row, so Coolify never removes it. + return $volume; + } if ($topLevelVolumes->has($source->value())) { $v = $topLevelVolumes->get($source->value()); if (data_get($v, 'driver_opts.type') === 'cifs') { @@ -3386,8 +3390,10 @@ function parseDockerComposeFile(Service|Application $resource, bool $isNew = fal } $server = $resource->destination->server; $topLevelVolumes = collect(data_get($yaml, 'volumes', [])); + // External volumes are used as written, also by previews (no prefix and no preview suffix). + $externalTopLevelVolumes = $topLevelVolumes->filter(fn (mixed $volume): bool => isComposeExternalVolume($volume)); if ($pull_request_id !== 0) { - $topLevelVolumes = collect([]); + $topLevelVolumes = collect($externalTopLevelVolumes->all()); } if ($topLevelVolumes->count() > 0) { @@ -3418,7 +3424,7 @@ function parseDockerComposeFile(Service|Application $resource, bool $isNew = fal if ($pull_request_id !== 0) { $definedNetwork = collect(["{$resource->uuid}-$pull_request_id"]); } - $services = collect($services)->map(function ($service, $serviceName) use ($topLevelVolumes, $topLevelNetworks, $definedNetwork, $isNew, $generatedServiceFQDNS, $resource, $server, $pull_request_id, $preview_id) { + $services = collect($services)->map(function ($service, $serviceName) use ($topLevelVolumes, $externalTopLevelVolumes, $topLevelNetworks, $definedNetwork, $isNew, $generatedServiceFQDNS, $resource, $server, $pull_request_id, $preview_id) { $serviceVolumes = collect(data_get($service, 'volumes', [])); $servicePorts = collect(data_get($service, 'ports', [])); $serviceNetworks = collect(data_get($service, 'networks', [])); @@ -3457,7 +3463,7 @@ function parseDockerComposeFile(Service|Application $resource, bool $isNew = fal $containerName = "$serviceName-$baseName"; if ($resource->compose_parsing_version === '1') { if (count($serviceVolumes) > 0) { - $serviceVolumes = $serviceVolumes->map(function ($volume) use ($resource, $topLevelVolumes, $pull_request_id) { + $serviceVolumes = $serviceVolumes->map(function ($volume) use ($resource, $topLevelVolumes, $externalTopLevelVolumes, $pull_request_id) { if (is_string($volume)) { $volume = str($volume); if ($volume->contains(':') && ! $volume->startsWith('/')) { @@ -3475,6 +3481,8 @@ function parseDockerComposeFile(Service|Application $resource, bool $isNew = fal $name = addPreviewDeploymentSuffix($name, $pull_request_id); } $volume = str("$name:$mount"); + } elseif (composeExternalVolumeDeclaration($externalTopLevelVolumes, $name->value()) !== null) { + // An external volume is used as written. } else { if ($pull_request_id !== 0) { $name = addPreviewDeploymentSuffix($name, $pull_request_id); @@ -3542,6 +3550,9 @@ function parseDockerComposeFile(Service|Application $resource, bool $isNew = fal } else { data_set($volume, 'source', $source.':'.$target); } + } elseif (composeExternalVolumeDeclaration($externalTopLevelVolumes, (string) $source) !== null) { + // An external volume is used as written. + data_set($volume, 'source', $source.':'.$target.($read_only ? ':ro' : '')); } else { if ($pull_request_id !== 0) { $source = addPreviewDeploymentSuffix($source, $pull_request_id); @@ -3581,7 +3592,7 @@ function parseDockerComposeFile(Service|Application $resource, bool $isNew = fal } } elseif ($resource->compose_parsing_version === '2') { if (count($serviceVolumes) > 0) { - $serviceVolumes = $serviceVolumes->map(function ($volume) use ($resource, $topLevelVolumes, $pull_request_id) { + $serviceVolumes = $serviceVolumes->map(function ($volume) use ($resource, $topLevelVolumes, $externalTopLevelVolumes, $pull_request_id) { if (is_string($volume)) { $volume = str($volume); if ($volume->contains(':') && ! $volume->startsWith('/')) { @@ -3599,6 +3610,8 @@ function parseDockerComposeFile(Service|Application $resource, bool $isNew = fal $name = addPreviewDeploymentSuffix($name, $pull_request_id); } $volume = str("$name:$mount"); + } elseif (composeExternalVolumeDeclaration($externalTopLevelVolumes, $name->value()) !== null) { + // An external volume is used as written. } else { if ($pull_request_id !== 0) { $uuid = $resource->uuid; @@ -3668,6 +3681,9 @@ function parseDockerComposeFile(Service|Application $resource, bool $isNew = fal } else { data_set($volume, 'source', $source.':'.$target); } + } elseif (composeExternalVolumeDeclaration($externalTopLevelVolumes, (string) $source) !== null) { + // An external volume is used as written. + data_set($volume, 'source', $source.':'.$target.($read_only ? ':ro' : '')); } else { if ($pull_request_id === 0) { $source = $uuid."-$source"; diff --git a/tests/Feature/ApplicationPreviewComposeCleanupTest.php b/tests/Feature/ApplicationPreviewComposeCleanupTest.php index c055631165..41c904d35c 100644 --- a/tests/Feature/ApplicationPreviewComposeCleanupTest.php +++ b/tests/Feature/ApplicationPreviewComposeCleanupTest.php @@ -131,7 +131,7 @@ it('removes only the network and volumes that Coolify generated for the preview' expect($resourceCommands) ->toContain("sudo docker volume rm -f '{$this->uuid}_app-data-pr-42'") - ->toContain("sudo docker volume rm -f '{$this->uuid}_shared-data-pr-42'") + ->not->toContain('shared-data-pr-42') ->toContain("sudo docker network disconnect '{$this->previewNetwork}' coolify-proxy") ->toContain("sudo docker network rm '{$this->previewNetwork}'") ->not->toContain("'app-data'") diff --git a/tests/Feature/ComposeExternalVolumeTest.php b/tests/Feature/ComposeExternalVolumeTest.php new file mode 100644 index 0000000000..7727d6f119 --- /dev/null +++ b/tests/Feature/ComposeExternalVolumeTest.php @@ -0,0 +1,434 @@ + InstanceSettings::firstOrCreate(['id' => 0])); + config(['constants.ssh.mux_enabled' => false]); + Bus::fake(); + Queue::fake(); + Process::fake(['*' => Process::result(output: '')]); + + $team = Team::factory()->create(); + $privateKey = PrivateKey::factory()->create(['team_id' => $team->id]); + $server = Server::factory()->create([ + 'team_id' => $team->id, + 'private_key_id' => $privateKey->id, + ]); + $server->settings()->update(['is_reachable' => true, 'is_usable' => true]); + $this->destination = StandaloneDocker::where('server_id', $server->id)->firstOrFail(); + $project = Project::factory()->create(['team_id' => $team->id]); + $this->environment = $project->environments()->first() + ?? Environment::factory()->create(['project_id' => $project->id]); +}); + +function externalVolumeApplication(string $compose, ?string $parsingVersion = null): Application +{ + $application = Application::factory()->create([ + 'build_pack' => 'dockercompose', + 'docker_compose_raw' => $compose, + 'environment_id' => test()->environment->id, + 'destination_id' => test()->destination->id, + 'destination_type' => test()->destination->getMorphClass(), + ]); + if ($parsingVersion !== null) { + $application->update(['compose_parsing_version' => $parsingVersion]); + } + + return $application->fresh(); +} + +function externalVolumeService(string $compose, ?string $parsingVersion = null): Service +{ + $service = Service::factory()->create([ + 'docker_compose_raw' => $compose, + 'environment_id' => test()->environment->id, + 'server_id' => test()->destination->server_id, + 'destination_id' => test()->destination->id, + 'destination_type' => test()->destination->getMorphClass(), + ]); + if ($parsingVersion !== null) { + $service->update(['compose_parsing_version' => $parsingVersion]); + } + + return $service->fresh(); +} + +function externalVolumePreview(Application $application, int $pullRequestId = 42): ApplicationPreview +{ + return ApplicationPreview::create([ + 'uuid' => 'external-volume-preview-'.$pullRequestId, + 'application_id' => $application->id, + 'pull_request_id' => $pullRequestId, + 'pull_request_html_url' => 'https://github.com/example/repository/pull/'.$pullRequestId, + ]); +} + +/** + * @return list + */ +function persistentVolumeNames(): array +{ + return LocalPersistentVolume::query()->pluck('name')->sort()->values()->all(); +} + +/** + * @param array $commands + */ +function fakeExternalVolumeServer(array &$commands): void +{ + Process::fake(function ($process) use (&$commands) { + $commands[] = is_array($process->command) ? implode(' ', $process->command) : $process->command; + + return Process::result(output: ''); + }); +} + +describe('applicationParser', function () { + it('keeps an external volume in short syntax and renames only the other volumes', function () { + $application = externalVolumeApplication(EXTERNAL_VOLUME_SHORT_COMPOSE); + $uuid = $application->uuid; + + $compose = applicationParser($application)->toArray(); + + expect($compose['services']['web']['volumes'])->toContain('shared-data:/data:ro') + ->toContain("{$uuid}_app-data:/app") + ->and($compose['volumes']['shared-data'])->toBe(['external' => true]) + ->and($compose['volumes'])->toHaveKey("{$uuid}_app-data") + ->not->toHaveKey("{$uuid}_shared-data") + ->and(persistentVolumeNames())->toBe(["{$uuid}_app-data"]); + }); + + it('keeps an external volume in long syntax with its name', function () { + $application = externalVolumeApplication(EXTERNAL_VOLUME_LONG_COMPOSE); + $uuid = $application->uuid; + + $compose = applicationParser($application)->toArray(); + $sources = collect($compose['services']['web']['volumes'])->pluck('source')->all(); + + expect($sources)->toBe(['shared-data', "{$uuid}_app-data"]) + ->and($compose['volumes']['shared-data'])->toBe(['external' => true, 'name' => 'existing-shared-volume']) + ->and($compose['volumes'])->not->toHaveKey("{$uuid}_shared-data") + ->and(persistentVolumeNames())->toBe(["{$uuid}_app-data"]); + }); + + it('keeps an external volume in the old external name syntax', function () { + $application = externalVolumeApplication(EXTERNAL_VOLUME_OLD_SYNTAX_COMPOSE); + + $compose = applicationParser($application)->toArray(); + + expect($compose['services']['web']['volumes'])->toBe(['shared-data:/data']) + ->and($compose['volumes'])->toBe(['shared-data' => ['external' => ['name' => 'legacy-shared-volume']]]) + ->and(persistentVolumeNames())->toBe([]); + }); + + it('shares the external volume with a preview deployment', function () { + $application = externalVolumeApplication(EXTERNAL_VOLUME_SHORT_COMPOSE); + $uuid = $application->uuid; + $preview = externalVolumePreview($application); + + $compose = applicationParser($application, 42, $preview->id)->toArray(); + + expect($compose['services']['web-pr-42']['volumes'])->toContain('shared-data:/data:ro') + ->toContain("{$uuid}_app-data-pr-42:/app") + ->and($compose['volumes']['shared-data'])->toBe(['external' => true]) + ->and($compose['volumes'])->not->toHaveKey("{$uuid}_shared-data-pr-42") + ->and(persistentVolumeNames())->toBe(["{$uuid}_app-data-pr-42"]); + }); + + it('keeps an existing row of a volume that an earlier parse prefixed', function () { + $application = externalVolumeApplication(EXTERNAL_VOLUME_SHORT_COMPOSE); + $uuid = $application->uuid; + $oldVolume = $application->persistentStorages()->create([ + 'name' => "{$uuid}_shared-data", + 'mount_path' => '/data', + ]); + + applicationParser($application); + + expect(LocalPersistentVolume::find($oldVolume->id))->not->toBeNull() + ->and(LocalPersistentVolume::find($oldVolume->id)->name)->toBe("{$uuid}_shared-data") + ->and(persistentVolumeNames())->toBe(["{$uuid}_app-data", "{$uuid}_shared-data"]); + }); + + it('does not change nfs volumes', function () { + $application = externalVolumeApplication(EXTERNAL_VOLUME_NFS_COMPOSE); + + $compose = applicationParser($application)->toArray(); + + expect($compose['services']['web']['volumes'])->toBe(['nfs-data:/data']) + ->and($compose['volumes']['nfs-data']['driver_opts']['type'])->toBe('nfs') + ->and(persistentVolumeNames())->toBe([]); + }); +}); + +describe('serviceParser', function () { + it('keeps an external volume in short syntax', function () { + $service = externalVolumeService(EXTERNAL_VOLUME_SHORT_COMPOSE); + $uuid = $service->uuid; + + $compose = serviceParser($service)->toArray(); + + expect($compose['services']['web']['volumes'])->toContain('shared-data:/data:ro') + ->toContain("{$uuid}_app-data:/app") + ->and($compose['volumes']['shared-data'])->toBe(['external' => true]) + ->and($compose['volumes'])->not->toHaveKey("{$uuid}_shared-data") + ->and(persistentVolumeNames())->toBe(["{$uuid}_app-data"]); + }); + + it('keeps an external volume in long syntax with its name', function () { + $service = externalVolumeService(EXTERNAL_VOLUME_LONG_COMPOSE); + $uuid = $service->uuid; + + $compose = serviceParser($service)->toArray(); + $sources = collect($compose['services']['web']['volumes'])->pluck('source')->all(); + + expect($sources)->toBe(['shared-data', "{$uuid}_app-data"]) + ->and($compose['volumes']['shared-data'])->toBe(['external' => true, 'name' => 'existing-shared-volume']) + ->and(persistentVolumeNames())->toBe(["{$uuid}_app-data"]); + }); + + it('keeps an existing row of a volume that an earlier parse prefixed', function () { + $service = externalVolumeService(EXTERNAL_VOLUME_SHORT_COMPOSE); + $uuid = $service->uuid; + $serviceApplication = ServiceApplication::create(['name' => 'web', 'service_id' => $service->id, 'image' => 'nginx:alpine']); + $oldVolume = $serviceApplication->persistentStorages()->create([ + 'name' => "{$uuid}_shared-data", + 'mount_path' => '/data', + ]); + + serviceParser($service); + + expect(LocalPersistentVolume::find($oldVolume->id)?->name)->toBe("{$uuid}_shared-data") + ->and(persistentVolumeNames())->toBe(["{$uuid}_app-data", "{$uuid}_shared-data"]); + }); + + it('does not change nfs volumes', function () { + $service = externalVolumeService(EXTERNAL_VOLUME_NFS_COMPOSE); + + $compose = serviceParser($service)->toArray(); + + expect($compose['volumes']['nfs-data']['driver_opts']['type'])->toBe('nfs') + ->and(persistentVolumeNames())->toBe([]); + }); +}); + +describe('legacy parsers', function () { + it('keeps an external volume in a legacy service', function (string $compose, string $expectedVolume) { + $service = externalVolumeService($compose, '2'); + $uuid = $service->uuid; + + $parsed = parseDockerComposeFile($service)->toArray(); + $volumes = collect($parsed['services']['web']['volumes']) + ->map(fn (mixed $volume): string => is_array($volume) ? $volume['source'] : $volume) + ->all(); + + expect($volumes)->toContain($expectedVolume) + ->toContain(str_ends_with($expectedVolume, ':ro') ? "{$uuid}_app-data:/app" : "{$uuid}_app-data") + ->and($parsed['volumes']['shared-data']['external'])->toBeTrue() + ->and($parsed['volumes'])->not->toHaveKey("{$uuid}_shared-data") + ->and(persistentVolumeNames())->toBe(["{$uuid}_app-data"]); + })->with([ + 'short syntax' => [EXTERNAL_VOLUME_SHORT_COMPOSE, 'shared-data:/data:ro'], + 'long syntax' => [EXTERNAL_VOLUME_LONG_COMPOSE, 'shared-data'], + ]); + + it('keeps an existing row of a volume that an earlier legacy service parse prefixed', function () { + $service = externalVolumeService(EXTERNAL_VOLUME_SHORT_COMPOSE, '2'); + $uuid = $service->uuid; + $serviceApplication = ServiceApplication::create(['name' => 'web', 'service_id' => $service->id, 'image' => 'nginx:alpine']); + $oldVolume = $serviceApplication->persistentStorages()->create([ + 'name' => "{$uuid}_shared-data", + 'mount_path' => '/data', + ]); + + parseDockerComposeFile($service); + + expect(LocalPersistentVolume::find($oldVolume->id)?->name)->toBe("{$uuid}_shared-data"); + }); + + it('keeps an external volume in a legacy application', function (string $parsingVersion, int $pullRequestId, string $compose, string $expectedExternal, string $expectedOther) { + $application = externalVolumeApplication($compose, $parsingVersion); + $uuid = $application->uuid; + $previewId = $pullRequestId === 0 ? null : externalVolumePreview($application, $pullRequestId)->id; + + $parsed = parseDockerComposeFile($application, pull_request_id: $pullRequestId, preview_id: $previewId)->toArray(); + $serviceName = $pullRequestId === 0 ? 'web' : "web-pr-{$pullRequestId}"; + $expectedOther = str_replace('{uuid}', $uuid, $expectedOther); + + expect($parsed['services'][$serviceName]['volumes'])->toContain($expectedExternal) + ->toContain($expectedOther) + ->and($parsed['volumes']['shared-data']['external'])->toBeTrue() + ->and(array_keys($parsed['volumes']))->not->toContain('shared-data-pr-42') + ->not->toContain("{$uuid}-shared-data") + ->not->toContain("{$uuid}-shared-data-pr-42"); + })->with([ + 'v1 short' => ['1', 0, EXTERNAL_VOLUME_SHORT_COMPOSE, 'shared-data:/data:ro', 'app-data:/app'], + 'v1 short preview' => ['1', 42, EXTERNAL_VOLUME_SHORT_COMPOSE, 'shared-data:/data:ro', 'app-data-pr-42:/app'], + 'v1 long preview' => ['1', 42, EXTERNAL_VOLUME_LONG_COMPOSE, 'shared-data:/data', 'app-data-pr-42:/app'], + 'v2 short' => ['2', 0, EXTERNAL_VOLUME_SHORT_COMPOSE, 'shared-data:/data:ro', '{uuid}-app-data:/app'], + 'v2 short preview' => ['2', 42, EXTERNAL_VOLUME_SHORT_COMPOSE, 'shared-data:/data:ro', '{uuid}-app-data-pr-42:/app'], + 'v2 long' => ['2', 0, EXTERNAL_VOLUME_LONG_COMPOSE, 'shared-data:/data', '{uuid}-app-data:/app'], + 'v2 long preview' => ['2', 42, EXTERNAL_VOLUME_LONG_COMPOSE, 'shared-data:/data', '{uuid}-app-data-pr-42:/app'], + ]); +}); + +describe('validation', function () { + it('rejects an external volume name with a variable', function (string $compose) { + expect(fn () => validateDockerComposeForInjection($compose)) + ->toThrow(Exception::class, 'Coolify does not resolve variables in the name of an external volume'); + + $application = externalVolumeApplication($compose); + expect(fn () => applicationParser($application)) + ->toThrow(Exception::class, 'Coolify does not resolve variables in the name of an external volume'); + + $service = externalVolumeService($compose); + expect(fn () => serviceParser($service)) + ->toThrow(Exception::class, 'Coolify does not resolve variables in the name of an external volume'); + })->with([ + 'name field' => ["services:\n web:\n image: nginx\n volumes:\n - 'shared-data:/data'\nvolumes:\n shared-data:\n external: true\n name: \${SHARED_VOLUME}\n"], + 'name field with default' => ["services:\n web:\n image: nginx\n volumes:\n - 'shared-data:/data'\nvolumes:\n shared-data:\n external: true\n name: \${SHARED_VOLUME:-shared}\n"], + 'old syntax' => ["services:\n web:\n image: nginx\n volumes:\n - 'shared-data:/data'\nvolumes:\n shared-data:\n external:\n name: \$SHARED_VOLUME\n"], + ]); + + it('rejects an external volume name that is not a valid Docker volume name', function () { + $compose = "services:\n web:\n image: nginx\nvolumes:\n shared-data:\n external: true\n name: 'shared data;rm -rf /'\n"; + + expect(fn () => validateDockerComposeForInjection($compose)) + ->toThrow(Exception::class, 'Invalid external Docker Compose volume shared-data'); + }); + + it('accepts external volumes with a literal name', function (string $compose) { + validateDockerComposeForInjection($compose); + + expect(true)->toBeTrue(); + })->with([ + 'short' => [EXTERNAL_VOLUME_SHORT_COMPOSE], + 'long' => [EXTERNAL_VOLUME_LONG_COMPOSE], + 'old syntax' => [EXTERNAL_VOLUME_OLD_SYNTAX_COMPOSE], + ]); +}); + +describe('delete', function () { + it('never removes the external volume when a service is deleted with its volumes', function () { + $service = externalVolumeService(EXTERNAL_VOLUME_SHORT_COMPOSE); + $uuid = $service->uuid; + serviceParser($service); + $service->delete(); + $commands = []; + fakeExternalVolumeServer($commands); + + (new DeleteResourceJob($service))->handle(); + + $all = implode("\n", $commands); + expect($all)->toContain("docker volume rm -f '{$uuid}_app-data'") + ->not->toContain("'shared-data'") + ->not->toContain("{$uuid}_shared-data"); + }); + + it('lets Docker Compose keep the external volume when a Compose application is deleted with its volumes', function () { + $application = externalVolumeApplication(EXTERNAL_VOLUME_SHORT_COMPOSE); + applicationParser($application); + $application->delete(); + $commands = []; + fakeExternalVolumeServer($commands); + + (new DeleteResourceJob($application))->handle(); + + $all = implode("\n", $commands); + expect($all)->toContain('docker compose down -v') + ->not->toContain('docker volume rm') + ->and($application->docker_compose)->toContain("shared-data:\n external: true"); + }); + + it('does not remove the external volume when a preview is deleted', function () { + $application = externalVolumeApplication(EXTERNAL_VOLUME_SHORT_COMPOSE); + $uuid = $application->uuid; + $preview = externalVolumePreview($application); + $commands = []; + fakeExternalVolumeServer($commands); + + $preview->forceDelete(); + + $volumeCommands = collect($commands)->filter(fn (string $command): bool => str_contains($command, 'docker volume'))->implode("\n"); + expect($volumeCommands)->toContain("docker volume rm -f '{$uuid}_app-data-pr-42'") + ->not->toContain('shared-data'); + }); +}); diff --git a/tests/Unit/PreviewDeploymentBindMountTest.php b/tests/Unit/PreviewDeploymentBindMountTest.php index 0bf23e4e33..27ef0606ce 100644 --- a/tests/Unit/PreviewDeploymentBindMountTest.php +++ b/tests/Unit/PreviewDeploymentBindMountTest.php @@ -115,7 +115,8 @@ describe('parser structure: bind mount handling', function () { $parsersFile = file_get_contents(__DIR__.'/../../bootstrap/helpers/parsers.php'); $volumeBlockStart = strpos($parsersFile, "} elseif (\$type->value() === 'volume')"); - $volumeBlock = substr($parsersFile, $volumeBlockStart, 1000); + $volumeBlockEnd = strpos($parsersFile, 'LocalPersistentVolume::updateOrCreate', $volumeBlockStart); + $volumeBlock = substr($parsersFile, $volumeBlockStart, $volumeBlockEnd - $volumeBlockStart); expect($volumeBlock)->toContain('addPreviewDeploymentSuffix'); });