fix(previews): give each preview its own volume for external volumes

Previews of Docker Compose applications used external volumes as
written, so a preview mounted the production volume. Two database
containers on the same data can corrupt it.

- A preview uses {uuid}_<name>-pr-N with a name-only declaration, the
  same name as v4.3.23, so old preview volumes are used again
- Production still uses the external volume as written
- Preview cleanup removes the preview volume, never the external one
- Do not show the external volume warning for previews

app/Models/LocalPersistentVolume.php

@@ -227,8 +227,9 @@ class LocalPersistentVolume extends BaseModel
     /**
      * The external Compose volume that this storage entry still replaces, or null. Before Coolify
      * used external volumes as written, the parsers gave them a generated name, for example
-     * "{uuid}_{volume}" or "{uuid}_{volume}-pr-{id}". While this storage entry exists, the parsers
-     * keep that name so that the resource keeps its data (see useComposeExternalVolumeAsWritten()).
+     * "{uuid}_{volume}". While this storage entry exists, the parsers keep that name so that the
+     * resource keeps its data (see useComposeExternalVolumeAsWritten()). A preview volume
+     * ("{uuid}_{volume}-pr-{id}") replaces nothing: a preview never uses the external volume.
      */
     public function replacedExternalComposeVolume(): ?string
     {
@@ -256,8 +257,7 @@ class LocalPersistentVolume extends BaseModel
                     $resource instanceof Application => $resource->uuid.'_'.Str::slug($key, '-'),
                     default => data_get($resource, 'service.uuid').'_'.Str::slug($key, '-'),
                 };
-                $isLegacyName = $legacyName !== $key && $this->name === $legacyName;
-                if ($isLegacyName || preg_match('/^'.preg_quote($legacyName, '/').'-pr-\d+$/', $this->name) === 1) {
+                if ($legacyName !== $key && $this->name === $legacyName) {
                     return $key;
                 }
             }

bootstrap/helpers/parsers.php

@@ -342,8 +342,9 @@ function isComposeExternalVolume(mixed $declaration): bool
 /**
  * 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.
+ * "{uuid}_" prefix and no LocalPersistentVolume row, so that Coolify never removes a volume that it
+ * does not own. A preview deployment never uses an external volume; it gets its own volume
+ * "{uuid}_{volume}-pr-{id}" like any other named volume.
  *
  * @param  iterable<array-key, mixed>  $topLevelVolumes
  * @return array<string, mixed>|null
@@ -364,9 +365,11 @@ function composeExternalVolumeDeclaration(iterable $topLevelVolumes, string $sou
 /**
  * Tells if a parser uses the volume source as written because it is an external volume.
  *
+ * Only for production deployments: a preview deployment always gets its own volume.
+ *
  * Before Coolify used external volumes as written, the parsers renamed them like all other
- * volumes (the old name, for example "{uuid}_{volume}" or "{uuid}_{volume}-pr-{id}"), so the
- * resource wrote its data into the renamed volume. When the owner still has the storage entry
+ * volumes (the old name, for example "{uuid}_{volume}"), so the resource wrote its data into the
+ * renamed volume. When the owner still has the storage entry
  * with the old name, the parser must keep the old name, or the resource loses its data. Then
  * this function records a warning on the resource and returns false. When the user deletes that
  * storage entry, the next parse uses the external volume.
@@ -431,7 +434,8 @@ function legacyApplicationRenamedVolumeDeclaration(string $name): array
  * Records a warning when a legacy Compose application (parser version 1 or 2) does not use an
  * external volume as written. These parsers keep the old volume name (see
  * legacyApplicationComposeVolumeName()), so the resource keeps its data. The parser version 1 keeps
- * the name of a production volume, so it uses the external volume and gets no warning.
+ * the name of a production volume, so it uses the external volume and gets no warning. A preview
+ * always uses its own volume, so it gets no warning either.
  *
  * The volume names are not validated here, so that the legacy parsers keep their old behavior. The
  * warning shows the Docker volume name only when it is a literal, valid Docker volume name.
@@ -440,6 +444,9 @@ function legacyApplicationRenamedVolumeDeclaration(string $name): array
  */
 function warnLegacyApplicationComposeExternalVolume(Application $resource, iterable $topLevelVolumes, string $source, int $pull_request_id): void
 {
+    if ($pull_request_id !== 0) {
+        return;
+    }
     $declaration = collect($topLevelVolumes)->get($source);
     if (! isComposeExternalVolume($declaration)) {
         return;
@@ -1331,6 +1338,7 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int
     }
 
     // Parse the rest of the services
+    $previewOwnExternalVolumes = collect([]);
     foreach ($services as $serviceName => $service) {
         $image = data_get_str($service, 'image');
         $restart = data_get_str($service, 'restart', RESTART_MODE);
@@ -1481,16 +1489,21 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int
                         }
                     }
                 } elseif ($type->value() === 'volume') {
-                    $legacyName = "{$uuid}_".Str::slug($source, '-');
-                    if ($isPullRequest) {
-                        $legacyName = addPreviewDeploymentSuffix($legacyName, $pull_request_id);
-                    }
-                    if (useComposeExternalVolumeAsWritten($resource, $originalResource, $topLevel->get('volumes'), $source->value(), $legacyName)) {
-                        // Previews share the external volume. It gets no row, so Coolify never removes it.
+                    // A preview never mounts an external volume: two deployments that write into the same
+                    // data directory (for example two databases) can corrupt it. The preview gets its own
+                    // volume with the name that older Coolify versions gave it, so it keeps its data, and
+                    // the preview cleanup removes it.
+                    $isPreviewOwnExternalVolume = $isPullRequest
+                        && composeExternalVolumeDeclaration($topLevel->get('volumes'), $source->value()) !== null;
+                    if (! $isPullRequest && useComposeExternalVolumeAsWritten($resource, $originalResource, $topLevel->get('volumes'), $source->value(), "{$uuid}_".Str::slug($source, '-'))) {
+                        // The external volume gets no row, so Coolify never removes it.
                         $volumesParsed->put($index, $volume);
 
                         continue;
                     }
+                    if ($isPreviewOwnExternalVolume) {
+                        $previewOwnExternalVolumes->put($source->value(), true);
+                    }
                     $declaration = $topLevel->get('volumes')->get($source->value());
                     if (in_array(data_get($declaration, 'driver_opts.type'), ['cifs', 'nfs'], true)) {
                         // Network volumes are used as written.
@@ -1529,7 +1542,9 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int
                             'resource_type' => get_class($originalResource),
                         ]
                     );
-                    $topLevel->get('volumes')->put($name, composeRenamedVolumeDeclarationFor($declaration, $name, $persistentVolume, $isPullRequest));
+                    $topLevel->get('volumes')->put($name, $isPreviewOwnExternalVolume
+                        ? ['name' => $name]
+                        : composeRenamedVolumeDeclarationFor($declaration, $name, $persistentVolume, $isPullRequest));
                 }
                 dispatch(new ServerFilesFromServerJob($originalResource));
                 $volumesParsed->put($index, $volume);
@@ -2134,6 +2149,8 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int
 
         $parsedServices->put($serviceName, $payload);
     }
+    // The preview mounts its own volumes instead of these external volumes (see above), so it does not declare them.
+    $topLevel->put('volumes', $topLevel->get('volumes')->except($previewOwnExternalVolumes->keys()->all()));
     $topLevel->put('services', $parsedServices);
 
     $customOrder = ['services', 'volumes', 'networks', 'configs', 'secrets'];

tests/Feature/ApplicationPreviewComposeCleanupTest.php

@@ -131,7 +131,8 @@ 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'")
-        ->not->toContain('shared-data-pr-42')
+        // A preview gets its own volume instead of the external one.
+        ->toContain("sudo docker volume rm -f '{$this->uuid}_shared-data-pr-42'")
         ->toContain("sudo docker network disconnect '{$this->previewNetwork}' coolify-proxy")
         ->toContain("sudo docker network rm '{$this->previewNetwork}'")
         ->not->toContain("'app-data'")

tests/Feature/ComposeExternalVolumeTest.php

@@ -304,18 +304,34 @@ describe('applicationParser', function () {
             ->and(persistentVolumeNames())->toBe([]);
     });
 
-    it('shares the external volume with a preview deployment', function () {
+    it('gives a preview deployment its own volume instead of the external volume', 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')
+        expect($compose['services']['web-pr-42']['volumes'])->toContain("{$uuid}_shared-data-pr-42:/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"]);
+            ->not->toContain('shared-data:/data:ro')
+            ->and($compose['volumes']["{$uuid}_shared-data-pr-42"])->toBe(['name' => "{$uuid}_shared-data-pr-42"])
+            ->and($compose['volumes'])->not->toHaveKey('shared-data')
+            ->and(persistentVolumeNames())->toBe(["{$uuid}_app-data-pr-42", "{$uuid}_shared-data-pr-42"])
+            ->and($application->composeVolumeWarnings())->toBe([]);
+    });
+
+    it('gives a preview deployment its own volume instead of a named external volume in long syntax', function () {
+        $application = externalVolumeApplication(EXTERNAL_VOLUME_LONG_COMPOSE);
+        $uuid = $application->uuid;
+        $preview = externalVolumePreview($application);
+
+        $compose = applicationParser($application, 42, $preview->id);
+        $sources = collect($compose->toArray()['services']['web-pr-42']['volumes'])->pluck('source')->all();
+
+        expect($sources)->toBe(["{$uuid}_shared-data-pr-42", "{$uuid}_app-data-pr-42"])
+            ->and($compose->toArray()['volumes']["{$uuid}_shared-data-pr-42"])->toBe(['name' => "{$uuid}_shared-data-pr-42"])
+            ->and(Yaml::dump(convertToArray($compose), 10, 2))->not->toContain('existing-shared-volume')
+            ->and($application->composeVolumeWarnings())->toBe([]);
     });
 
     it('does not show a warning when the resource has no old storage entry', function () {
@@ -354,21 +370,22 @@ describe('applicationParser', function () {
             ->and($application->composeVolumeWarnings())->toBe([legacyExternalVolumeWarning("{$uuid}_shared-data", 'existing-shared-volume')]);
     });
 
-    it('keeps the old prefixed preview volume when the resource has its old storage entry', function () {
+    it('reuses the preview volume of an older Coolify version without a warning', function () {
         $application = externalVolumeApplication(EXTERNAL_VOLUME_SHORT_COMPOSE);
         $uuid = $application->uuid;
         $preview = externalVolumePreview($application);
-        legacyExternalVolumeRow($application, "{$uuid}_shared-data-pr-42");
+        $oldVolume = legacyExternalVolumeRow($application, "{$uuid}_shared-data-pr-42");
 
         $compose = applicationParser($application, 42, $preview->id)->toArray();
 
         expect($compose['services']['web-pr-42']['volumes'])->toContain("{$uuid}_shared-data-pr-42:/data:ro")
             ->and($compose['volumes']["{$uuid}_shared-data-pr-42"])->toBe(['name' => "{$uuid}_shared-data-pr-42"])
+            ->and(LocalPersistentVolume::find($oldVolume->id)?->name)->toBe("{$uuid}_shared-data-pr-42")
             ->and(persistentVolumeNames())->toBe(["{$uuid}_app-data-pr-42", "{$uuid}_shared-data-pr-42"])
-            ->and($application->composeVolumeWarnings())->toBe([legacyExternalVolumeWarning("{$uuid}_shared-data-pr-42")]);
+            ->and($application->composeVolumeWarnings())->toBe([]);
     });
 
-    it('uses the external volume in a preview when only the production volume has an old storage entry', function () {
+    it('gives a preview its own volume when only the production volume has an old storage entry', function () {
         $application = externalVolumeApplication(EXTERNAL_VOLUME_SHORT_COMPOSE);
         $uuid = $application->uuid;
         $preview = externalVolumePreview($application);
@@ -376,7 +393,9 @@ describe('applicationParser', function () {
 
         $compose = applicationParser($application, 42, $preview->id)->toArray();
 
-        expect($compose['services']['web-pr-42']['volumes'])->toContain('shared-data:/data:ro')
+        expect($compose['services']['web-pr-42']['volumes'])->toContain("{$uuid}_shared-data-pr-42:/data:ro")
+            ->not->toContain('shared-data:/data:ro')
+            ->not->toContain("{$uuid}_shared-data:/data:ro")
             ->and($application->composeVolumeWarnings())->toBe([]);
     });
 
@@ -532,7 +551,7 @@ describe('legacy parsers', function () {
             ->and($service->composeVolumeWarnings())->toBe([]);
     });
 
-    it('keeps the old volume names of a legacy application and shows a warning when the name is not the external name', function (string $parsingVersion, int $pullRequestId, string $compose, string $expectedVolume, ?string $dockerVolume) {
+    it('keeps the old volume names of a legacy application and shows a warning when production does not use the external name', function (string $parsingVersion, int $pullRequestId, string $compose, string $expectedVolume, ?string $dockerVolume) {
         // Legacy Compose applications (parser versions 1 and 2) never stored their volumes, so Coolify
         // cannot tell which external volume already holds data: they always keep the old names.
         $application = externalVolumeApplication($compose, $parsingVersion);
@@ -548,7 +567,8 @@ describe('legacy parsers', function () {
             ->and(array_keys($parsed['volumes'] ?? []))->toContain($expectedName)
             ->and($application->composeVolumeWarnings())->toBe(
                 // Parser version 1 keeps the name of a production volume, so it uses the external volume.
-                $expectedName === 'shared-data' ? [] : [legacyParserExternalVolumeWarning($expectedName, $dockerVolume)]
+                // A preview always uses its own volume, so it gets no warning.
+                $pullRequestId !== 0 || $expectedName === 'shared-data' ? [] : [legacyParserExternalVolumeWarning($expectedName, $dockerVolume)]
             );
     })->with([
         'v1 short' => ['1', 0, EXTERNAL_VOLUME_SHORT_COMPOSE, 'shared-data:/data:ro', null],
@@ -572,8 +592,20 @@ describe('legacy parsers', function () {
             ->and($logEntries)->toBe([['Warning: '.legacyParserExternalVolumeWarning($keptName), 'stderr']]);
     })->with([
         'v2 production' => ['2', 0, '{uuid}-shared-data'],
-        'v2 preview' => ['2', 42, '{uuid}-shared-data-pr-42'],
-        'v1 preview' => ['1', 42, 'shared-data-pr-42'],
+    ]);
+
+    it('writes no warning to the deployment log of a legacy application preview, which always uses its own volume', function (string $parsingVersion, string $keptName) {
+        $application = externalVolumeApplication(EXTERNAL_VOLUME_SHORT_COMPOSE, $parsingVersion);
+        $keptName = str_replace('{uuid}', $application->uuid, $keptName);
+        $preview = externalVolumePreview($application);
+
+        [$composeFile, $logEntries] = parseExternalVolumeDeployment($application, 42, $preview);
+
+        expect(data_get($composeFile, 'services.web-pr-42.volumes'))->toContain("{$keptName}:/data:ro")
+            ->and($logEntries)->toBe([]);
+    })->with([
+        'v2 preview' => ['2', '{uuid}-shared-data-pr-42'],
+        'v1 preview' => ['1', 'shared-data-pr-42'],
     ]);
 
     it('writes no warning to the deployment log for a production deployment of a parser version 1 application', function () {
@@ -842,8 +874,8 @@ describe('delete', function () {
         expect(LocalPersistentVolume::find($volume->id))->not->toBeNull();
     });
 
-    it('does not remove the external volume when a preview is deleted', function () {
-        $application = externalVolumeApplication(EXTERNAL_VOLUME_SHORT_COMPOSE);
+    it('removes the own volume of the preview, but not the external volume, when a preview is deleted', function (string $compose, string $externalVolume) {
+        $application = externalVolumeApplication($compose);
         $uuid = $application->uuid;
         $preview = externalVolumePreview($application);
         $commands = [];
@@ -853,7 +885,23 @@ describe('delete', function () {
 
         $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');
+            ->toContain("docker volume rm -f '{$uuid}_shared-data-pr-42'")
+            ->not->toContain("'{$externalVolume}'");
+    })->with([
+        'short syntax' => [EXTERNAL_VOLUME_SHORT_COMPOSE, 'shared-data'],
+        'named external volume' => [EXTERNAL_VOLUME_LONG_COMPOSE, 'existing-shared-volume'],
+    ]);
+
+    it('does not mark the own volume of a preview as an entry that replaces the external volume', function () {
+        $application = externalVolumeApplication(EXTERNAL_VOLUME_SHORT_COMPOSE);
+        $uuid = $application->uuid;
+        $preview = externalVolumePreview($application);
+        $productionEntry = legacyExternalVolumeRow($application, "{$uuid}_shared-data");
+        applicationParser($application, 42, $preview->id);
+        $previewEntry = LocalPersistentVolume::query()->where('name', "{$uuid}_shared-data-pr-42")->firstOrFail();
+
+        expect($previewEntry->replacedExternalComposeVolume())->toBeNull()
+            ->and($productionEntry->replacedExternalComposeVolume())->toBe('shared-data');
     });
 
     it('removes the old prefixed preview volume, but not the external volume, when a preview is deleted', function () {
@@ -886,9 +934,20 @@ describe('warnings', function () {
             ->and($logEntries)->toBe([['Warning: '.legacyExternalVolumeWarning(str_replace('{uuid}', $uuid, $oldName)), 'stderr']]);
     })->with([
         'production' => [0, '{uuid}_shared-data', '{uuid}_shared-data:/data:ro'],
-        'preview' => [42, '{uuid}_shared-data-pr-42', '{uuid}_shared-data-pr-42:/data:ro'],
     ]);
 
+    it('writes no warning to the deployment log of a preview, which always uses its own volume', function () {
+        $application = externalVolumeApplication(EXTERNAL_VOLUME_SHORT_COMPOSE);
+        $uuid = $application->uuid;
+        $preview = externalVolumePreview($application);
+        legacyExternalVolumeRow($application, "{$uuid}_shared-data-pr-42");
+
+        [$composeFile, $logEntries] = parseExternalVolumeDeployment($application, 42, $preview);
+
+        expect(data_get($composeFile, 'services.web-pr-42.volumes'))->toContain("{$uuid}_shared-data-pr-42:/data:ro")
+            ->and($logEntries)->toBe([]);
+    });
+
     it('writes no warning to the deployment log when the external volume is used as written', function () {
         $application = externalVolumeApplication(EXTERNAL_VOLUME_SHORT_COMPOSE);