From 8d083deffb493ff5bdf3356ed8876a9aeb59ae6c Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 27 Oct 2026 12:04:37 +0100 Subject: [PATCH] fix(storage): stop syncing file storages on every compose parse The compose parser queued ServerFilesFromServerJob for every volume on each parse, and pages that only show data parse the Compose file. Each sync also sent two SSH commands for each file. - Queue the sync only for a compose deployment, and for each service resource with file storages when the service starts or deploys. - Read the compose service names without a parse on the Domains and scheduled task pages. - Check all file storages with one remote command, and stop when a symbolic link, a special file, or an unknown state is at a file path. - Keep the application path in a file storage when a pull request preview is parsed. Preview deployments of the current parser write their `-pr-` copies through LocalFileVolume::forPullRequest(). - Remove the unreachable file sync from the service resource page. --- .ai/lessons.md | 2 + .../Service/DeployServiceApplication.php | 1 + app/Actions/Service/StartService.php | 14 ++ app/Jobs/ApplicationDeploymentJob.php | 30 ++++- app/Jobs/ServerFilesFromServerJob.php | 3 +- app/Livewire/Project/Application/Domains.php | 10 +- app/Livewire/Project/Service/Index.php | 34 +---- .../Project/Shared/ScheduledTask/All.php | 4 +- app/Models/Application.php | 17 +++ app/Models/LocalFileVolume.php | 62 ++++++--- bootstrap/helpers/parsers.php | 7 +- bootstrap/helpers/services.php | 52 ++++---- tests/Feature/ApplicationDomainsTest.php | 8 +- .../ComposeContentVolumeConfinementTest.php | 3 +- .../ContentFilesBeforeComposeUpTest.php | 47 +++++-- tests/Feature/FileStorageSyncTriggersTest.php | 123 ++++++++++++++++++ .../LocalFileVolumeRelativePathTest.php | 2 + .../PersistentStoragePerformanceTest.php | 8 +- .../PreviewDomainPortOverridesTest.php | 9 +- tests/Feature/PreviewFileStoragePathTest.php | 94 +++++++++++++ .../Feature/ServerStorageSaveDispatchTest.php | 8 +- tests/Feature/StorageAndServiceAuditTest.php | 10 +- tests/Feature/StorageHostPathTest.php | 4 +- .../application-with-exact-caddy-labels.yaml | 12 +- ...application-with-exact-traefik-labels.yaml | 12 +- .../expected/application-with-settings.yaml | 12 +- .../Fixtures/parser/expected/application.yaml | 12 +- tests/Support/RemoteProcessTestHelpers.php | 8 ++ tests/Unit/LocalFileVolumeContentSizeTest.php | 21 +-- 29 files changed, 467 insertions(+), 162 deletions(-) create mode 100644 tests/Feature/FileStorageSyncTriggersTest.php create mode 100644 tests/Feature/PreviewFileStoragePathTest.php diff --git a/.ai/lessons.md b/.ai/lessons.md index d6db4e49b2..11ca067b78 100644 --- a/.ai/lessons.md +++ b/.ai/lessons.md @@ -34,6 +34,8 @@ - The root team (id 0) has `personal_team = true`, so `personal_team` does not identify a user's own team. To clean up test users, detach them from shared teams and then delete the user; never delete a team selected by a filter. A Team delete revokes the team's API tokens and deletes its private keys before any guard can stop it. - SQLite returns `0`/`1` for boolean columns without a `boolean` cast, while Postgres returns real booleans. A strict `=== false` check on such a column never matches in tests, so tests can pass on behavior that Postgres blocks. Add the cast or compare loosely. +- The Compose parsers (`parse()`, `applicationParser`, `serviceParser`) run on page mounts, refreshes, and deployments. Do not dispatch jobs or run server commands from them; let the caller that changes data start the work. Code that only needs the service names reads `docker_compose_raw` (`Application::composeServicesWithoutParse()`). + ## Verify the complete user flow - Do not use a passing unit test, a successful build, or a healthy process as proof for a reported UI failure. - Verify the exact live flow, persisted state, relevant logs, and queue state when they affect the result. diff --git a/app/Actions/Service/DeployServiceApplication.php b/app/Actions/Service/DeployServiceApplication.php index cc49010db6..abf66941a6 100644 --- a/app/Actions/Service/DeployServiceApplication.php +++ b/app/Actions/Service/DeployServiceApplication.php @@ -20,6 +20,7 @@ class DeployServiceApplication $composeServiceName = $serviceApplication->name; $service->parse(); + StartService::queueFileStorageSync($service); $service->saveComposeConfigs(); $service->isConfigurationChanged(save: true); diff --git a/app/Actions/Service/StartService.php b/app/Actions/Service/StartService.php index c7119bfefe..c9f0d6e079 100644 --- a/app/Actions/Service/StartService.php +++ b/app/Actions/Service/StartService.php @@ -3,6 +3,7 @@ namespace App\Actions\Service; use App\Actions\Shared\EnsureContentFilesOnServer; +use App\Jobs\ServerFilesFromServerJob; use App\Models\LocalFileVolume; use App\Models\Service; use App\Models\ServiceApplication; @@ -24,6 +25,7 @@ class StartService public function handle(Service $service, bool $pullLatestImages = false, bool $stopBeforeStart = false) { $service->parse(); + self::queueFileStorageSync($service); // Fetch remote secrets before stopping: if the secret manager fails, the service keeps running. // saveComposeConfigs() below reuses the fetched secrets. $service->ensureRemoteSecretsResolvable($service->environment_variables()->get()); @@ -103,6 +105,18 @@ class StartService return "{$connectedNetworks} | grep -qxF {$network} || docker network connect --alias {$containerName} {$network} {$containerName} || echo {$warning}"; } + /** + * Queues one sync of the file storages with the server for each resource of the service that has + * file storages. The sync marks each storage as a file or a directory, as it is on the server. + */ + public static function queueFileStorageSync(Service $service): void + { + $service->applications()->get() + ->concat($service->databases()->get()) + ->filter(fn (ServiceApplication|ServiceDatabase $resource): bool => $resource->fileStorages()->exists()) + ->each(fn (ServiceApplication|ServiceDatabase $resource) => ServerFilesFromServerJob::dispatch($resource)); + } + /** * Shows the volume warnings of the last parse (for example an external volume that the * service does not use yet) in the start log. diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index ea3ce9191c..0603b971ea 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -810,14 +810,14 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue $fileStorage->is_directory = true; $fileStorage->content = null; $fileStorage->save(); - $fileStorage->deleteStorageOnServer($this->mainServer); - $fileStorage->saveStorageOnServer($this->mainServer); + $fileStorage->forPullRequest($this->previewFileStoragePullRequestId())->deleteStorageOnServer($this->mainServer); + $fileStorage->forPullRequest($this->previewFileStoragePullRequestId())->saveStorageOnServer($this->mainServer); } elseif ($fileStat->value() === 'regular file' && $fileStorage->is_directory) { $fileStorage->is_directory = false; $fileStorage->is_based_on_git = true; $fileStorage->save(); - $fileStorage->deleteStorageOnServer($this->mainServer); - $fileStorage->saveStorageOnServer($this->mainServer); + $fileStorage->forPullRequest($this->previewFileStoragePullRequestId())->deleteStorageOnServer($this->mainServer); + $fileStorage->forPullRequest($this->previewFileStoragePullRequestId())->saveStorageOnServer($this->mainServer); } } } @@ -994,12 +994,18 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue /** * Parses the Compose file for this deployment and writes the volume warnings of the parser * (for example an external volume that the resource does not use yet) to the deployment log. + * A deployment of the application also queues one sync of the file storages with the server, which + * loads the content of the files that come from the repository. A preview uses its own copies of the + * files, so its deployment does not sync the storages of the application. * * @return Collection */ private function parseComposeFileForDeployment(): Collection { $composeFile = collect($this->application->parse(pull_request_id: $this->pull_request_id, preview_id: data_get($this->preview, 'id'), commit: $this->commit)); + if ($this->pull_request_id === 0) { + ServerFilesFromServerJob::dispatch($this->application); + } foreach ($this->application->composeVolumeWarnings() as $warning) { $this->application_deployment_queue->addLogEntry("Warning: {$warning}", 'stderr'); } @@ -1158,6 +1164,18 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue } } + /** + * The pull request whose file storage paths this deployment writes. Only the current Compose parser + * mounts the `-pr-` copies in a preview; other build packs and the legacy parser mount the + * paths of the application. + */ + private function previewFileStoragePullRequestId(): int + { + return $this->application->build_pack === 'dockercompose' && (int) $this->application->compose_parsing_version >= 3 + ? $this->pull_request_id + : 0; + } + /** * The queued ServerStorageSaveJob writes new content files, but it can run after the containers * start. Preserve-repository deployments write all file storages in write_deployment_configurations(). @@ -1171,7 +1189,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue } EnsureContentFilesOnServer::run( - $this->application->fileStorages()->get(), + $this->application->fileStorages()->get()->map->forPullRequest($this->previewFileStoragePullRequestId()), $this->mainServer, function (string $message, string $type): void { $this->application_deployment_queue->addLogEntry($message, $type); @@ -1334,7 +1352,7 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue } foreach ($this->application->fileStorages as $fileStorage) { if (! $fileStorage->is_host_file && ! $fileStorage->is_based_on_git && ! $fileStorage->is_directory) { - $fileStorage->saveStorageOnServer($this->mainServer); + $fileStorage->forPullRequest($this->previewFileStoragePullRequestId())->saveStorageOnServer($this->mainServer); } } if ($this->use_build_server) { diff --git a/app/Jobs/ServerFilesFromServerJob.php b/app/Jobs/ServerFilesFromServerJob.php index 73641131fe..3d0b06b2f7 100644 --- a/app/Jobs/ServerFilesFromServerJob.php +++ b/app/Jobs/ServerFilesFromServerJob.php @@ -18,8 +18,7 @@ class ServerFilesFromServerJob implements ShouldBeEncrypted, ShouldBeUniqueUntil use Dispatchable, InteractsWithQueue, Queueable, SerializesModels; /** - * Only one sync per resource can wait in the queue. The compose parser dispatches - * this job once per volume on every parse, so without this the queue fills with copies. + * Only one sync per resource can wait in the queue, also when deployments follow each other quickly. */ public int $uniqueFor = 600; diff --git a/app/Livewire/Project/Application/Domains.php b/app/Livewire/Project/Application/Domains.php index 118ba4461c..70918b6e54 100644 --- a/app/Livewire/Project/Application/Domains.php +++ b/app/Livewire/Project/Application/Domains.php @@ -321,16 +321,10 @@ class Domains extends Component if (! $this->isCompose) { $this->composeServices = []; } elseif ($parseCompose) { - try { - $parsed = $this->application->parse() ?? []; - } catch (\Throwable) { - $parsed = []; - } $this->composeServices = []; - $services = data_get($parsed, 'services', []); - foreach ($services as $serviceName => $service) { + foreach ($this->application->composeServicesWithoutParse() as $serviceName => $service) { if (! isDatabaseImage(data_get($service, 'image'))) { - $this->composeServices[] = $serviceName; + $this->composeServices[] = (string) $serviceName; } } } diff --git a/app/Livewire/Project/Service/Index.php b/app/Livewire/Project/Service/Index.php index 2c91c31c35..ed5025ab1b 100644 --- a/app/Livewire/Project/Service/Index.php +++ b/app/Livewire/Project/Service/Index.php @@ -168,8 +168,6 @@ class Index extends Component return; } $this->parameters = get_route_parameters(); - $this->query = request()->query(); - $this->currentRoute = request()->route()->getName(); $project = currentTeam() ->projects() ->select('id', 'uuid', 'team_id') @@ -181,32 +179,12 @@ class Index extends Component ->firstOrFail(); $this->service = $environment->services()->whereUuid($this->parameters['service_uuid'])->firstOrFail(); $this->authorize('view', $this->service); - if (in_array($this->currentRoute, ['project.service.index', 'project.service.index.advanced'], true)) { - return redirect()->route( - 'project.service.configuration', - collect($this->parameters)->except('stack_service_uuid')->all(), - ); - } - $service = $this->service->applications()->whereUuid($this->parameters['stack_service_uuid'])->first(); - if ($service) { - $this->serviceApplication = $service; - $this->resourceType = 'application'; - $this->serviceApplication->getFilesFromServer(); - $this->initializeApplicationProperties(); - } else { - $this->serviceDatabase = $this->service->databases()->whereUuid($this->parameters['stack_service_uuid'])->first(); - if (! $this->serviceDatabase) { - return redirect()->route('project.service.configuration', [ - 'project_uuid' => $this->parameters['project_uuid'], - 'environment_uuid' => $this->parameters['environment_uuid'], - 'service_uuid' => $this->parameters['service_uuid'], - ]); - } - $this->resourceType = 'database'; - $this->serviceDatabase->getFilesFromServer(); - $this->initializeDatabaseProperties(); - } - $this->s3s = $this->serviceTeamS3Storages(); + + // The settings of a stack service open in a modal on the configuration page. + return redirect()->route( + 'project.service.configuration', + collect($this->parameters)->except('stack_service_uuid')->all(), + ); } catch (\Throwable $e) { return handleError($e, $this); } diff --git a/app/Livewire/Project/Shared/ScheduledTask/All.php b/app/Livewire/Project/Shared/ScheduledTask/All.php index 124c907210..971554f6a1 100644 --- a/app/Livewire/Project/Shared/ScheduledTask/All.php +++ b/app/Livewire/Project/Shared/ScheduledTask/All.php @@ -44,9 +44,7 @@ class All extends Component $this->containerNames = $this->containerNames->merge($this->resource->databases()->pluck('name')); } elseif ($this->resource->type() === 'application') { if ($this->resource->build_pack === 'dockercompose') { - $parsed = $this->resource->parse(); - $containers = collect(data_get($parsed, 'services'))->keys(); - $this->containerNames = $containers; + $this->containerNames = collect($this->resource->composeServicesWithoutParse())->keys(); } else { $this->containerNames = collect([]); } diff --git a/app/Models/Application.php b/app/Models/Application.php index 1f474d4aa3..e729d77fcc 100644 --- a/app/Models/Application.php +++ b/app/Models/Application.php @@ -2206,6 +2206,23 @@ class Application extends BaseModel instant_remote_process($commands, $this->destination->server, false); } + /** + * The services of the Compose file, read without parse(). A parse saves volumes and variables, so + * code that only needs the service names uses this. + * + * @return array + */ + public function composeServicesWithoutParse(): array + { + try { + $services = data_get(parseDockerComposeYaml((string) $this->docker_compose_raw), 'services'); + } catch (\Throwable) { + return []; + } + + return is_array($services) ? $services : []; + } + public function parse(int $pull_request_id = 0, ?int $preview_id = null, ?string $commit = null) { if ((int) $this->compose_parsing_version >= 3) { diff --git a/app/Models/LocalFileVolume.php b/app/Models/LocalFileVolume.php index 50d5a51578..d96632a683 100644 --- a/app/Models/LocalFileVolume.php +++ b/app/Models/LocalFileVolume.php @@ -72,6 +72,11 @@ class LocalFileVolume extends BaseModel done SH; + /** + * The pull request whose preview this copy writes. Not saved: see forPullRequest(). + */ + protected int $previewPullRequestId = 0; + protected $casts = [ // 'fs_path' => 'encrypted', // 'mount_path' => 'encrypted', @@ -343,10 +348,17 @@ class LocalFileVolume extends BaseModel validateShellSafePath($path, 'storage path'); $escapedPath = escapeshellarg($path); - $isFile = instant_remote_process(["test -f {$escapedPath} && echo OK || echo NOK"], $server); - $isDir = instant_remote_process(["test -d {$escapedPath} && echo OK || echo NOK"], $server); + $state = self::remoteFileStates([(string) $path], $server)[0]; + if ($state === 'unknown') { + throw new \Exception("Coolify cannot check the file {$path} on the server."); + } + if ($state === 'other' && ! $this->is_directory) { + throw new \Exception("A symbolic link or a special file is at {$path} on the server. Coolify did not write the file."); + } + $isFile = $state === 'file'; + $isDir = in_array($state, ['directory', 'empty-directory'], true); $replacesEmptyDirectory = false; - if ($isFile === 'OK' && $this->is_directory) { + if ($isFile && $this->is_directory) { if ($this->remoteFileExceedsLimit($escapedPath, $server)) { $this->content = self::TOO_LARGE_PLACEHOLDER; } else { @@ -356,7 +368,7 @@ class LocalFileVolume extends BaseModel $this->save(); FileStorageChanged::dispatch(data_get($server, 'team_id')); throw new \Exception('The following file is a file on the server, but you are trying to mark it as a directory. Please delete the file on the server or mark it as directory.'); - } elseif ($isDir === 'OK' && ! $this->is_directory) { + } elseif ($isDir && ! $this->is_directory) { if ($path === '/' || $path === '.' || $path === '..' || $path === '' || str($path)->isEmpty() || is_null($path)) { $this->is_directory = true; $this->save(); @@ -364,12 +376,12 @@ class LocalFileVolume extends BaseModel } // Docker creates a missing bind source as an empty directory. Replace only an empty // directory inside the resource directory; never delete files that are on the server. - if ($isOutsideResourceDirectory || self::remoteFileStates([(string) $path], $server)[0] !== 'empty-directory') { + if ($isOutsideResourceDirectory || $state !== 'empty-directory') { throw new \Exception("The following file is a directory on the server, but you are trying to mark it as a file: {$path}

Please delete the directory on the server or mark it as directory."); } $replacesEmptyDirectory = true; } - if (($isDir === 'NOK' || $replacesEmptyDirectory) && ! $this->is_directory) { + if ((! $isDir || $replacesEmptyDirectory) && ! $this->is_directory) { if ($replacesEmptyDirectory) { $commands->push("rmdir {$escapedPath}"); } @@ -388,7 +400,7 @@ class LocalFileVolume extends BaseModel if ($chmod) { $commands->push('chmod -- '.escapeshellarg($chmod)." {$escapedPath}"); } - } elseif ($isDir === 'NOK' && $this->is_directory) { + } elseif (! $isDir && $this->is_directory) { $commands->push("mkdir -p {$escapedPath} > /dev/null 2>&1 || true"); } @@ -405,22 +417,24 @@ class LocalFileVolume extends BaseModel * front of it and never changes the script. See REMOTE_FILE_STATE_SCRIPT for the output. * * @param list $paths + * @param (Closure(string): string)|null $quote Makes a shell argument of a path. The default is escapeshellarg(). */ - public static function remoteFileStateCommand(array $paths): string + public static function remoteFileStateCommand(array $paths, ?Closure $quote = null): string { - return 'sh -c '.escapeshellarg(self::REMOTE_FILE_STATE_SCRIPT).' sh '.implode(' ', array_map('escapeshellarg', $paths)); + return 'sh -c '.escapeshellarg(self::REMOTE_FILE_STATE_SCRIPT).' sh '.implode(' ', array_map($quote ?? escapeshellarg(...), $paths)); } /** * Gets the state of each path with one server command. A path without a state in the output is `unknown`. * * @param list $paths + * @param (Closure(string): string)|null $quote Makes a shell argument of a path. The default is escapeshellarg(). * @return list */ - public static function remoteFileStates(array $paths, Server $server): array + public static function remoteFileStates(array $paths, Server $server, ?Closure $quote = null): array { $states = []; - $output = (string) instant_remote_process([self::remoteFileStateCommand($paths)], $server, false); + $output = (string) instant_remote_process([self::remoteFileStateCommand($paths, $quote)], $server, false); foreach (preg_split('/\R/', trim($output)) as $line) { if (preg_match('/^(\d+):([a-z-]+)$/', trim($line), $matches)) { $states[(int) $matches[1] - 1] = $matches[2]; @@ -560,16 +574,28 @@ class LocalFileVolume extends BaseModel public function resolvedFsPath(string $workdir): Stringable { $path = trim((string) $this->fs_path); - if (str_starts_with($path, '/') || str_starts_with($path, '~')) { - return str($path); - } - if (str_starts_with($path, '.')) { - $path = substr($path, 1); + if (! str_starts_with($path, '/') && ! str_starts_with($path, '~')) { + if (str_starts_with($path, '.')) { + $path = substr($path, 1); + } + $path = ltrim($path, '/'); + $path = rtrim($workdir, '/').($path === '' ? '' : '/'.$path); } - $path = ltrim($path, '/'); + return str(addPreviewDeploymentSuffix($path, $this->previewPullRequestId)); + } - return str(rtrim($workdir, '/').($path === '' ? '' : '/'.$path)); + /** + * A copy that uses the path of the preview of a pull request, as the Compose parser mounts it + * (`-pr-` unless the PR suffix is off for this storage). The row keeps the path of + * the application, so saving the copy never stores the preview path. + */ + public function forPullRequest(int $pullRequestId): static + { + $copy = clone $this; + $copy->previewPullRequestId = $pullRequestId !== 0 && ($this->is_preview_suffix_enabled ?? true) ? $pullRequestId : 0; + + return $copy; } /** diff --git a/bootstrap/helpers/parsers.php b/bootstrap/helpers/parsers.php index 8ae596a1e1..37ef8cc2fa 100644 --- a/bootstrap/helpers/parsers.php +++ b/bootstrap/helpers/parsers.php @@ -1,7 +1,6 @@ fs_path); + // The file storage keeps the path of the application; a preview deployment adds the suffix (LocalFileVolume::forPullRequest()). + $storedSource = $source; $isPreviewSuffixEnabled = $foundConfig ? (bool) data_get($foundConfig, 'is_preview_suffix_enabled', true) : true; @@ -1617,7 +1618,7 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int 'resource_type' => get_class($originalResource), ], [ - 'fs_path' => $source, + 'fs_path' => $storedSource, 'mount_path' => $target, 'content' => $content, 'is_directory' => $isDirectory, @@ -1698,7 +1699,6 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int ? ['name' => $name] : composeRenamedVolumeDeclarationFor($declaration, $name, $persistentVolume, $isPullRequest)); } - dispatch(new ServerFilesFromServerJob($originalResource)); $volumesParsed->put($index, $volume); } } @@ -3121,7 +3121,6 @@ function serviceParser(Service $resource): Collection ); $topLevel->get('volumes')->put($name, composeRenamedVolumeDeclarationFor($declaration, $name, $persistentVolume)); } - dispatch(new ServerFilesFromServerJob($originalResource)); $volumesParsed->put($index, $volume); } } diff --git a/bootstrap/helpers/services.php b/bootstrap/helpers/services.php index bfb5f22759..576089ba95 100644 --- a/bootstrap/helpers/services.php +++ b/bootstrap/helpers/services.php @@ -301,45 +301,53 @@ function getFilesystemVolumesFromServer(ServiceApplication|ServiceDatabase|Appli $server = $oneService->service->server; } $fileVolumes = $oneService->fileStorages()->get(); - $escapedWorkdir = escapeshellarg($workdir); - $commands = collect([ - "mkdir -p -- {$escapedWorkdir} > /dev/null 2>&1 || true", - ]); - instant_remote_process($commands, $server); - foreach ($fileVolumes as $fileVolume) { - $content = data_get($fileVolume, 'content'); - // A relative path is inside the resource directory, never in the SSH user's working - // directory. A persisted Compose expression (`${VAR:-/path}`) stays as it is. + if ($isInit) { + instant_remote_process(['mkdir -p -- '.escapeshellarg($workdir).' > /dev/null 2>&1 || true'], $server); + } + if ($fileVolumes->isEmpty()) { + return; + } + // A relative path is inside the resource directory, never in the SSH user's working + // directory. A persisted Compose expression (`${VAR:-/path}`) stays as it is. + $fileLocations = $fileVolumes->map(function (LocalFileVolume $fileVolume) use ($workdir): string { $fsPath = trim((string) $fileVolume->fs_path); - $fileLocation = str_starts_with($fsPath, '$') ? $fsPath : $fileVolume->resolvedFsPath($workdir)->value(); - $escapedFileLocation = filesystemVolumeShellArgument((string) $fileLocation); + + return str_starts_with($fsPath, '$') ? $fsPath : $fileVolume->resolvedFsPath($workdir)->value(); + })->values()->all(); + $states = LocalFileVolume::remoteFileStates($fileLocations, $server, filesystemVolumeShellArgument(...)); + foreach ($fileVolumes->values() as $index => $fileVolume) { + $content = data_get($fileVolume, 'content'); + $fileLocation = $fileLocations[$index]; + $escapedFileLocation = filesystemVolumeShellArgument($fileLocation); // A token with only the write permission can save a Compose file, so Coolify does not // write outside the resource directory here. A deployment or start writes those paths. $isOutsideResourceDirectory = fn (): bool => $fileVolume->isOutsideResourceDirectory(); - // Exists and is a file - $isFile = instant_remote_process(["test -f {$escapedFileLocation} && echo OK || echo NOK"], $server); - // Exists and is a directory - $isDir = instant_remote_process(["test -d {$escapedFileLocation} && echo OK || echo NOK"], $server); + $state = $states[$index]; + // A symbolic link to a directory, a special file, or a failed check: Coolify changes nothing. + if ($state === 'other' || $state === 'unknown') { + continue; + } + $isDir = in_array($state, ['directory', 'empty-directory'], true); - if ($isFile === 'OK') { + if ($state === 'file') { $fileVolume->is_directory = false; $fileVolume->save(); if ($fileVolume->is_based_on_git) { $fileVolume->loadStorageOnServer(); } - } elseif ($isDir === 'OK' && ! $fileVolume->is_directory && filled($content)) { + } elseif ($isDir && ! $fileVolume->is_directory && filled($content)) { // A configured file must not lose its content because a directory is at its path. // Docker leaves an empty directory when it starts before the file exists: replace it. // A directory with files stays; the start or deployment shows a warning for it. - if (! $isOutsideResourceDirectory() && LocalFileVolume::remoteFileStates([(string) $fileLocation], $server)[0] === 'empty-directory') { + if (! $isOutsideResourceDirectory() && $state === 'empty-directory') { $fileVolume->saveStorageOnServer(); } - } elseif ($isDir === 'OK') { + } elseif ($isDir) { // If its a directory & exists $fileVolume->content = null; $fileVolume->is_directory = true; $fileVolume->save(); - } elseif ($isFile === 'NOK' && $isDir === 'NOK' && ! $fileVolume->is_directory && $isInit && $content) { + } elseif ($state === 'missing' && ! $fileVolume->is_directory && $isInit && $content) { // Does not exists (no dir or file), not flagged as directory, is init, has content if ($isOutsideResourceDirectory()) { continue; @@ -352,7 +360,7 @@ function getFilesystemVolumesFromServer(ServiceApplication|ServiceDatabase|Appli 'mkdir -p -- "$(dirname -- '.$escapedFileLocation.')"', "echo '$content' | base64 -d | tee -- {$escapedFileLocation}", ], $server); - } elseif ($isFile === 'NOK' && $isDir === 'NOK' && $fileVolume->is_directory && $isInit) { + } elseif ($state === 'missing' && $fileVolume->is_directory && $isInit) { // Does not exists (no dir or file), flagged as directory, is init $fileVolume->content = null; $fileVolume->is_directory = true; @@ -360,7 +368,7 @@ function getFilesystemVolumesFromServer(ServiceApplication|ServiceDatabase|Appli if (! $isOutsideResourceDirectory()) { instant_remote_process(["mkdir -p -- {$escapedFileLocation}"], $server); } - } elseif ($isFile === 'NOK' && $isDir === 'NOK' && ! $fileVolume->is_directory && $isInit && is_null($content)) { + } elseif ($state === 'missing' && ! $fileVolume->is_directory && $isInit && is_null($content)) { // Does not exists (no dir or file), not flagged as directory, is init, has no content => create directory $fileVolume->content = null; $fileVolume->is_directory = true; diff --git a/tests/Feature/ApplicationDomainsTest.php b/tests/Feature/ApplicationDomainsTest.php index a7d18a9865..7050f5777b 100644 --- a/tests/Feature/ApplicationDomainsTest.php +++ b/tests/Feature/ApplicationDomainsTest.php @@ -3697,7 +3697,7 @@ it('offers dns provider record creation for a public server ip', function () { ->not->toContain('The server has no public IP address; add the DNS record manually.'); }); -it('does not queue file sync jobs when polling dns checks for compose applications', function () { +it('does not parse the compose file or queue file sync jobs when it shows compose application domains', function () { Queue::fake(); $this->application->update([ @@ -3710,12 +3710,10 @@ it('does not queue file sync jobs when polling dns checks for compose applicatio ]); $component = Livewire::test(Domains::class, ['application' => $this->application->fresh()]); - $queuedOnMount = Queue::pushed(ServerFilesFromServerJob::class)->count(); - $component->call('pollDnsChecks')->call('pollDnsChecks')->call('pollDnsChecks'); - expect($queuedOnMount)->toBe(1) - ->and(Queue::pushed(ServerFilesFromServerJob::class))->toHaveCount(1) + expect(Queue::pushed(ServerFilesFromServerJob::class))->toHaveCount(0) + ->and($this->application->persistentStorages()->count())->toBe(0) ->and($component->get('composeServices'))->toBe(['web', 'api']); }); diff --git a/tests/Feature/ComposeContentVolumeConfinementTest.php b/tests/Feature/ComposeContentVolumeConfinementTest.php index 7218de04b6..876065ded9 100644 --- a/tests/Feature/ComposeContentVolumeConfinementTest.php +++ b/tests/Feature/ComposeContentVolumeConfinementTest.php @@ -127,11 +127,12 @@ function contentHostPathUseServerUser(string $user): void } /** - * Fakes the server: `test -f`/`test -d` print NOK (the file is missing), the symlink check prints OK. + * Fakes the server: every checked path is missing, the symlink check prints OK. */ function contentHostPathFakeServer(): void { Process::fake(fn ($process) => Process::result(output: match (true) { + str_contains(remoteProcessScript($process), 'empty-directory') => remoteFileStatesOutput('missing'), str_contains(remoteProcessScript($process), 'readlink -f') => 'OK', str_contains(remoteProcessScript($process), 'test -') => 'NOK', default => '', diff --git a/tests/Feature/ContentFilesBeforeComposeUpTest.php b/tests/Feature/ContentFilesBeforeComposeUpTest.php index 054b812b17..20b9db56f4 100644 --- a/tests/Feature/ContentFilesBeforeComposeUpTest.php +++ b/tests/Feature/ContentFilesBeforeComposeUpTest.php @@ -125,6 +125,9 @@ function fakeContentFilesServer(array $states): void }); } +/** + * The file state checks: one for all content files, then one by each write of a missing file. + */ function contentFilesStateChecks(): array { return array_values(array_filter(ContentFilesUpRecorder::ssh(), fn (string $command) => str_contains($command, 'empty-directory'))); @@ -251,7 +254,7 @@ test('a Compose deployment checks all content files with one server command', fu runComposeStart($this); - expect(contentFilesStateChecks())->toHaveCount(1) + expect(contentFilesStateChecks())->toHaveCount(2) ->and(contentFilesStateChecks()[0])->toContain(escapeshellarg($first->fs_path)) ->toContain(escapeshellarg($second->fs_path)) ->toContain(escapeshellarg($third->fs_path)) @@ -322,14 +325,28 @@ test('a Compose deployment does not write storages without Coolify content', fun ]); test('a pull request deployment writes the preview content file path', function () { - $volume = contentFilesVolume($this->application, 'config/app.conf-pr-7', ['is_preview_suffix_enabled' => true]); + $volume = contentFilesVolume($this->application, 'config/app.conf'); + $previewPath = $volume->fs_path.'-pr-7'; + fakeContentFilesServer([$volume->fs_path => 'file', $previewPath => 'missing']); + + runComposeStart($this, ['pull_request_id' => 7]); + + $write = ContentFilesUpRecorder::indexOf('ssh', 'base64 -d | tee '.escapeshellarg($previewPath)); + expect($write)->not->toBeNull() + ->and($write)->toBeLessThan(ContentFilesUpRecorder::indexOf('deployment', ' up -d')) + ->and(ContentFilesUpRecorder::indexOf('ssh', 'tee '.escapeshellarg($volume->fs_path).' '))->toBeNull() + ->and($volume->fresh()->fs_path)->toBe($this->application->workdir().'/config/app.conf'); +}); + +test('a pull request deployment of the legacy compose parser writes the content file path of the application', function () { + $this->application->update(['compose_parsing_version' => '2']); + $volume = contentFilesVolume($this->application, 'config/app.conf'); fakeContentFilesServer([$volume->fs_path => 'missing']); runComposeStart($this, ['pull_request_id' => 7]); - $write = ContentFilesUpRecorder::indexOf('ssh', 'tee '.escapeshellarg($this->application->workdir().'/config/app.conf-pr-7')); - expect($write)->not->toBeNull() - ->and($write)->toBeLessThan(ContentFilesUpRecorder::indexOf('deployment', ' up -d')); + expect(ContentFilesUpRecorder::indexOf('ssh', 'base64 -d | tee '.escapeshellarg($volume->fs_path)))->not->toBeNull() + ->and(ContentFilesUpRecorder::indexOf('ssh', '-pr-7'))->toBeNull(); }); test('a preserve-repository deployment writes content files before docker compose up', function () { @@ -355,7 +372,7 @@ test('a deployment to an additional server writes missing content files on that runComposeStart($this, ['server' => $otherServer, 'mainServer' => $otherServer]); $write = ContentFilesUpRecorder::indexOf('ssh', 'base64 -d | tee '.escapeshellarg($volume->fs_path)); - expect(contentFilesStateChecks())->toHaveCount(1) + expect(contentFilesStateChecks())->toHaveCount(2) ->and(contentFilesStateChecks()[0])->toContain("@'{$otherServer->ip}'") ->and($write)->not->toBeNull() ->and(ContentFilesUpRecorder::ssh()[$write])->toContain("@'{$otherServer->ip}'") @@ -441,7 +458,7 @@ test('starting a service writes missing content files before docker compose up', $activity = StartService::run($service->fresh()); $command = RemoteProcessCommand::read($activity); - expect(contentFilesStateChecks())->toHaveCount(1) + expect(contentFilesStateChecks())->toHaveCount(2) ->and($command)->toContain("echo 'Writing 1 missing configuration file.'") ->and($command)->toContain("echo 'Warning: A directory that is not empty is at {$workerConf->fs_path} on the server.") ->and(strpos($command, 'Writing 1 missing configuration file.'))->toBeLessThan(strpos($command, ' up -d')) @@ -469,7 +486,7 @@ test('deploying one service application writes only its missing content files', $activity = DeployServiceApplication::run($service->applications()->where('name', 'app')->firstOrFail()); $command = RemoteProcessCommand::read($activity); - expect(contentFilesStateChecks())->toHaveCount(1) + expect(contentFilesStateChecks())->toHaveCount(2) ->and(contentFilesStateChecks()[0])->not->toContain(escapeshellarg($workerConf->fs_path)) ->and(strpos($command, 'Writing 1 missing configuration file.'))->toBeLessThan(strpos($command, ' up -d')); Process::assertRan(fn ($process) => str_contains(remoteProcessScript($process), 'tee '.escapeshellarg($appConf->fs_path))); @@ -577,6 +594,20 @@ test('the server file sync replaces an empty directory with the content file', f Process::assertRan(fn ($process) => str_contains(remoteProcessScript($process), 'base64 -d')); }); +test('the server file sync checks all file storages with one server command', function () { + $first = contentFilesVolume($this->application, 'conf/app.conf'); + $second = contentFilesVolume($this->application, 'conf/worker.conf'); + $directory = contentFilesVolume($this->application, 'data/cache', ['content' => null]); + fakeContentFilesServer([$first->fs_path => 'file', $second->fs_path => 'file', $directory->fs_path => 'directory']); + + getFilesystemVolumesFromServer($this->application->fresh()); + + expect(ContentFilesUpRecorder::ssh())->toHaveCount(1) + ->and(contentFilesStateChecks())->toHaveCount(1) + ->and($directory->fresh()->is_directory)->toBeTruthy() + ->and($first->fresh()->is_directory)->toBeFalsy(); +}); + test('the server file sync still marks a storage without content as a directory', function () { $volume = contentFilesVolume($this->application, 'data/cache', ['content' => null]); fakeContentFilesServer([$volume->fs_path => 'directory']); diff --git a/tests/Feature/FileStorageSyncTriggersTest.php b/tests/Feature/FileStorageSyncTriggersTest.php new file mode 100644 index 0000000000..4532d1859c --- /dev/null +++ b/tests/Feature/FileStorageSyncTriggersTest.php @@ -0,0 +1,123 @@ +create(); + $server = Server::factory()->create(['team_id' => $team->id]); + $this->destination = StandaloneDocker::where('server_id', $server->id)->first(); + $project = Project::factory()->create(['team_id' => $team->id]); + $this->environment = Environment::factory()->create(['project_id' => $project->id]); + + $this->application = Application::factory()->create([ + 'environment_id' => $this->environment->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + 'build_pack' => 'dockercompose', + 'docker_compose_raw' => SYNC_TRIGGERS_COMPOSE, + ]); +}); + +it('does not queue a server file sync when it parses a compose application', function () { + $this->application->parse(); + $this->application->parse(pull_request_id: 7); + + Bus::assertNotDispatched(ServerFilesFromServerJob::class); +}); + +it('does not queue a server file sync when it parses a service', function () { + $service = Service::factory()->create([ + 'environment_id' => $this->environment->id, + 'server_id' => $this->destination->server_id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + 'docker_compose_raw' => SYNC_TRIGGERS_COMPOSE, + ]); + ServiceApplication::create(['name' => 'app', 'service_id' => $service->id]); + + $service->parse(); + + Bus::assertNotDispatched(ServerFilesFromServerJob::class); +}); + +it('queues one server file sync for a compose deployment of the application only', function (int $pullRequestId, int $expectedSyncs) { + $job = (new ReflectionClass(ApplicationDeploymentJob::class))->newInstanceWithoutConstructor(); + $queue = Mockery::mock(ApplicationDeploymentQueue::class); + $queue->shouldReceive('addLogEntry')->andReturnNull(); + foreach ([ + 'application' => $this->application, + 'application_deployment_queue' => $queue, + 'pull_request_id' => $pullRequestId, + 'commit' => 'HEAD', + ] as $property => $value) { + (new ReflectionProperty(ApplicationDeploymentJob::class, $property))->setValue($job, $value); + } + + (new ReflectionMethod(ApplicationDeploymentJob::class, 'parseComposeFileForDeployment'))->invoke($job); + + Bus::assertDispatchedTimes(ServerFilesFromServerJob::class, $expectedSyncs); +})->with([ + 'application' => [0, 1], + 'pull request preview' => [7, 0], +]); + +it('reads the compose service names without a parse', function () { + $this->application->update(['docker_compose_raw' => "services:\n web:\n image: nginx:alpine\n db:\n image: postgres:16\n"]); + + expect(array_keys($this->application->composeServicesWithoutParse()))->toBe(['web', 'db']) + ->and($this->application->fileStorages()->count())->toBe(0) + ->and($this->application->persistentStorages()->count())->toBe(0); + + $this->application->update(['docker_compose_raw' => "services: [\n"]); + expect($this->application->composeServicesWithoutParse())->toBe([]); +}); + +it('queues one server file sync for each service resource with file storages when the service starts', function () { + $service = Service::factory()->create([ + 'environment_id' => $this->environment->id, + 'server_id' => $this->destination->server_id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + 'docker_compose_raw' => "services:\n app:\n image: nginx:alpine\n volumes:\n - ./data:/app/data\n cache:\n image: redis:7\n", + ]); + $app = ServiceApplication::create(['name' => 'app', 'service_id' => $service->id]); + ServiceApplication::create(['name' => 'cache', 'service_id' => $service->id]); + $service->parse(); + + StartService::queueFileStorageSync($service); + + Bus::assertDispatchedTimes(ServerFilesFromServerJob::class, 1); + Bus::assertDispatched(ServerFilesFromServerJob::class, fn (ServerFilesFromServerJob $job) => $job->resource->is($app)); +}); diff --git a/tests/Feature/LocalFileVolumeRelativePathTest.php b/tests/Feature/LocalFileVolumeRelativePathTest.php index 8fc83ca274..e568f1f3a7 100644 --- a/tests/Feature/LocalFileVolumeRelativePathTest.php +++ b/tests/Feature/LocalFileVolumeRelativePathTest.php @@ -43,6 +43,7 @@ beforeEach(function () { $this->serviceApplication = ServiceApplication::create(['name' => 'app', 'service_id' => $this->service->id]); Process::fake(fn ($process) => Process::result(output: match (true) { + str_contains(remoteProcessScript($process), 'empty-directory') => remoteFileStatesOutput('missing'), str_contains(remoteProcessScript($process), 'readlink -f') => 'OK', str_contains(remoteProcessScript($process), 'test -') => 'NOK', default => '', @@ -106,6 +107,7 @@ test('an absolute directory path keeps working unchanged', function () { test('deleting a relative path removes it from the resource directory', function () { Process::fake(fn ($process) => Process::result(output: match (true) { + str_contains(remoteProcessScript($process), 'empty-directory') => remoteFileStatesOutput('file'), str_contains(remoteProcessScript($process), 'readlink -f') => 'OK', str_contains(remoteProcessScript($process), 'test -f') => 'OK', default => 'NOK', diff --git a/tests/Feature/PersistentStoragePerformanceTest.php b/tests/Feature/PersistentStoragePerformanceTest.php index db0853a8bd..66960f03d2 100644 --- a/tests/Feature/PersistentStoragePerformanceTest.php +++ b/tests/Feature/PersistentStoragePerformanceTest.php @@ -24,9 +24,11 @@ uses(RefreshDatabase::class); beforeEach(function () { $this->withoutVite(); config(['app.maintenance.store' => 'array', 'cache.default' => 'array']); - Process::fake(fn ($process) => Process::result( - output: str_contains(remoteProcessScript($process), 'readlink -f') ? 'OK' : '' - )); + Process::fake(fn ($process) => Process::result(output: match (true) { + str_contains(remoteProcessScript($process), 'empty-directory') => remoteFileStatesOutput('missing'), + str_contains(remoteProcessScript($process), 'readlink -f') => 'OK', + default => '', + })); InstanceSettings::unguarded(fn () => InstanceSettings::updateOrCreate( ['id' => 0], ['id' => 0, 'is_dns_validation_enabled' => false] diff --git a/tests/Feature/PreviewDomainPortOverridesTest.php b/tests/Feature/PreviewDomainPortOverridesTest.php index 4c41f3234e..69e1df8df6 100644 --- a/tests/Feature/PreviewDomainPortOverridesTest.php +++ b/tests/Feature/PreviewDomainPortOverridesTest.php @@ -14,9 +14,7 @@ use App\Models\Server; use App\Models\StandaloneDocker; use App\Models\Team; use App\Models\User; -use Illuminate\Bus\UniqueLock; use Illuminate\Foundation\Testing\RefreshDatabase; -use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Queue; use Illuminate\Support\Str; @@ -1103,14 +1101,9 @@ it('does not queue file sync jobs when polling dns checks for compose previews', ]); $component = Livewire::test(PreviewDomains::class, ['preview' => $preview]); - $queuedOnMount = Queue::pushed(ServerFilesFromServerJob::class); - - // A worker that starts the queued job releases its unique lock, so the next parse queues a new one. - Cache::lock(UniqueLock::getKey($queuedOnMount->first()))->forceRelease(); $component->call('pollDnsChecks')->call('pollDnsChecks')->call('pollDnsChecks'); - expect($queuedOnMount)->toHaveCount(1) - ->and(Queue::pushed(ServerFilesFromServerJob::class))->toHaveCount(1) + expect(Queue::pushed(ServerFilesFromServerJob::class))->toHaveCount(0) ->and($component->get('composeServices'))->toBe(['web', 'api']); }); diff --git a/tests/Feature/PreviewFileStoragePathTest.php b/tests/Feature/PreviewFileStoragePathTest.php new file mode 100644 index 0000000000..2a864b768a --- /dev/null +++ b/tests/Feature/PreviewFileStoragePathTest.php @@ -0,0 +1,94 @@ +create(); + $server = Server::factory()->create(['team_id' => $team->id]); + $destination = StandaloneDocker::where('server_id', $server->id)->first(); + $project = Project::factory()->create(['team_id' => $team->id]); + $environment = Environment::factory()->create(['project_id' => $project->id]); + + $this->application = Application::factory()->create([ + 'environment_id' => $environment->id, + 'destination_id' => $destination->id, + 'destination_type' => $destination->getMorphClass(), + 'build_pack' => 'dockercompose', + 'docker_compose_raw' => PREVIEW_FILE_STORAGE_COMPOSE, + ]); + $this->workdir = $this->application->workdir(); +}); + +/** + * @return array mount path => stored host path + */ +function previewFileStoragePaths(Application $application): array +{ + return LocalFileVolume::query()->where('resource_id', $application->id)->orderBy('mount_path')->pluck('fs_path', 'mount_path')->all(); +} + +it('keeps the paths of the application file storages when it parses a pull request preview', function () { + $this->application->parse(); + + $preview = $this->application->parse(pull_request_id: 7); + + expect(previewFileStoragePaths($this->application))->toBe([ + '/app/data' => "{$this->workdir}/data", + '/etc/app.conf' => "{$this->workdir}/config/app.conf", + ]); + $volumes = collect(data_get($preview, 'services.app-pr-7.volumes')) + ->map(fn ($volume) => is_array($volume) ? $volume['source'].':'.$volume['target'] : $volume) + ->all(); + expect($volumes)->toContain("{$this->workdir}/data-pr-7:/app/data") + ->toContain("{$this->workdir}/config/app.conf-pr-7:/etc/app.conf"); +}); + +it('stores the application path when the first parse is a pull request preview', function () { + $this->application->parse(pull_request_id: 7); + + expect(previewFileStoragePaths($this->application))->toBe([ + '/app/data' => "{$this->workdir}/data", + '/etc/app.conf' => "{$this->workdir}/config/app.conf", + ]); +}); + +it('writes the file of a pull request preview to its own path without saving that path', function () { + $this->application->parse(); + $storage = LocalFileVolume::query()->where('mount_path', '/etc/app.conf')->firstOrFail(); + + $preview = $storage->forPullRequest(7); + $preview->is_directory = false; + $preview->save(); + + expect($preview->contentPathOnServer())->toBe("{$this->workdir}/config/app.conf-pr-7") + ->and($storage->contentPathOnServer())->toBe("{$this->workdir}/config/app.conf") + ->and($storage->forPullRequest(0)->contentPathOnServer())->toBe("{$this->workdir}/config/app.conf") + ->and($storage->fresh()->fs_path)->toBe("{$this->workdir}/config/app.conf"); + + $storage->update(['is_preview_suffix_enabled' => false]); + expect($storage->fresh()->forPullRequest(7)->contentPathOnServer())->toBe("{$this->workdir}/config/app.conf"); +}); diff --git a/tests/Feature/ServerStorageSaveDispatchTest.php b/tests/Feature/ServerStorageSaveDispatchTest.php index b179890d1b..90dc4d5a31 100644 --- a/tests/Feature/ServerStorageSaveDispatchTest.php +++ b/tests/Feature/ServerStorageSaveDispatchTest.php @@ -37,9 +37,11 @@ beforeEach(function () { 'destination_type' => $destination->getMorphClass(), ]); - Process::fake(fn ($process) => Process::result( - output: str_contains(remoteProcessScript($process), 'readlink -f') ? 'OK' : 'NOK' - )); + Process::fake(fn ($process) => Process::result(output: match (true) { + str_contains(remoteProcessScript($process), 'empty-directory') => remoteFileStatesOutput('missing'), + str_contains(remoteProcessScript($process), 'readlink -f') => 'OK', + default => 'NOK', + })); $this->processedSaves = 0; Event::listen(JobProcessed::class, function (JobProcessed $event): void { diff --git a/tests/Feature/StorageAndServiceAuditTest.php b/tests/Feature/StorageAndServiceAuditTest.php index 6b9a3974d3..63fbfbc086 100644 --- a/tests/Feature/StorageAndServiceAuditTest.php +++ b/tests/Feature/StorageAndServiceAuditTest.php @@ -45,10 +45,12 @@ beforeEach(function () { $this->withoutDefer(); config()->set('app.maintenance.store', 'array'); Bus::fake(); - // Remote `test -f` / `test -d` probes report a missing path; path confinement checks pass. - Process::fake(fn ($process) => Process::result( - output: str_contains(implode(' ', (array) remoteProcessScript($process)), '&& echo OK || echo NOK') ? 'NOK' : 'OK' - )); + // Remote file probes report a missing path; path confinement checks pass. + Process::fake(fn ($process) => Process::result(output: match (true) { + str_contains(remoteProcessScript($process), 'empty-directory') => remoteFileStatesOutput('missing'), + str_contains(remoteProcessScript($process), '&& echo OK || echo NOK') => 'NOK', + default => 'OK', + })); Server::flushIdentityMap(); InstanceSettings::unguarded(fn () => InstanceSettings::updateOrCreate( diff --git a/tests/Feature/StorageHostPathTest.php b/tests/Feature/StorageHostPathTest.php index 856037e55e..174e93293f 100644 --- a/tests/Feature/StorageHostPathTest.php +++ b/tests/Feature/StorageHostPathTest.php @@ -59,11 +59,12 @@ beforeEach(function () { }); /** - * `test -L` prints $symlink and `test -f`/`test -d` print $exists. + * `test -L` prints $symlink and `test -f`/`test -d` print $exists. The file state check sees a file when $exists is OK. */ function storageHostPathFakeServer(string $symlink = 'OK', string $exists = 'NOK'): void { Process::fake(fn ($process) => Process::result(output: match (true) { + str_contains(remoteProcessScript($process), 'empty-directory') => remoteFileStatesOutput($exists === 'OK' ? 'file' : 'missing'), str_contains(remoteProcessScript($process), 'test -L') => $symlink, str_contains(remoteProcessScript($process), 'readlink -f') => 'OK', str_contains(remoteProcessScript($process), 'test -') => $exists, @@ -333,6 +334,7 @@ test('the delete dialog offers deletion on the host only for mounts inside the r test('a file outside the resource directory never replaces a directory on the host', function () { Process::fake(fn ($process) => Process::result(output: match (true) { + str_contains(remoteProcessScript($process), 'empty-directory') => remoteFileStatesOutput('directory'), str_contains(remoteProcessScript($process), 'test -L') => 'OK', str_contains(remoteProcessScript($process), 'test -d') => 'OK', str_contains(remoteProcessScript($process), 'test -f') => 'NOK', diff --git a/tests/Fixtures/parser/expected/application-with-exact-caddy-labels.yaml b/tests/Fixtures/parser/expected/application-with-exact-caddy-labels.yaml index 63897e9c8f..4ab6157c40 100644 --- a/tests/Fixtures/parser/expected/application-with-exact-caddy-labels.yaml +++ b/tests/Fixtures/parser/expected/application-with-exact-caddy-labels.yaml @@ -1359,15 +1359,15 @@ - 'referenceapplication0001_web-data-pr-7:/app/data' files: - - fs_path: /data/coolify/applications/referenceapplication0001/config-pr-7 + fs_path: /data/coolify/applications/referenceapplication0001/config mount_path: /app/config is_directory: true - - fs_path: /data/coolify/applications/referenceapplication0001/selinux-pr-7 + fs_path: /data/coolify/applications/referenceapplication0001/selinux mount_path: /app/selinux is_directory: true - - fs_path: /data/coolify/applications/referenceapplication0001/web-init.sh-pr-7 + fs_path: /data/coolify/applications/referenceapplication0001/web-init.sh mount_path: /docker-entrypoint.d/init.sh is_directory: false content: | @@ -2060,15 +2060,15 @@ - 'referenceapplication0001_web-data-pr-8:/app/data' files: - - fs_path: /data/coolify/applications/referenceapplication0001/config-pr-8 + fs_path: /data/coolify/applications/referenceapplication0001/config mount_path: /app/config is_directory: true - - fs_path: /data/coolify/applications/referenceapplication0001/selinux-pr-8 + fs_path: /data/coolify/applications/referenceapplication0001/selinux mount_path: /app/selinux is_directory: true - - fs_path: /data/coolify/applications/referenceapplication0001/web-init.sh-pr-8 + fs_path: /data/coolify/applications/referenceapplication0001/web-init.sh mount_path: /docker-entrypoint.d/init.sh is_directory: false content: | diff --git a/tests/Fixtures/parser/expected/application-with-exact-traefik-labels.yaml b/tests/Fixtures/parser/expected/application-with-exact-traefik-labels.yaml index 83792ffb74..ce031aaebd 100644 --- a/tests/Fixtures/parser/expected/application-with-exact-traefik-labels.yaml +++ b/tests/Fixtures/parser/expected/application-with-exact-traefik-labels.yaml @@ -1425,15 +1425,15 @@ - 'referenceapplication0001_web-data-pr-7:/app/data' files: - - fs_path: /data/coolify/applications/referenceapplication0001/config-pr-7 + fs_path: /data/coolify/applications/referenceapplication0001/config mount_path: /app/config is_directory: true - - fs_path: /data/coolify/applications/referenceapplication0001/selinux-pr-7 + fs_path: /data/coolify/applications/referenceapplication0001/selinux mount_path: /app/selinux is_directory: true - - fs_path: /data/coolify/applications/referenceapplication0001/web-init.sh-pr-7 + fs_path: /data/coolify/applications/referenceapplication0001/web-init.sh mount_path: /docker-entrypoint.d/init.sh is_directory: false content: | @@ -2177,15 +2177,15 @@ - 'referenceapplication0001_web-data-pr-8:/app/data' files: - - fs_path: /data/coolify/applications/referenceapplication0001/config-pr-8 + fs_path: /data/coolify/applications/referenceapplication0001/config mount_path: /app/config is_directory: true - - fs_path: /data/coolify/applications/referenceapplication0001/selinux-pr-8 + fs_path: /data/coolify/applications/referenceapplication0001/selinux mount_path: /app/selinux is_directory: true - - fs_path: /data/coolify/applications/referenceapplication0001/web-init.sh-pr-8 + fs_path: /data/coolify/applications/referenceapplication0001/web-init.sh mount_path: /docker-entrypoint.d/init.sh is_directory: false content: | diff --git a/tests/Fixtures/parser/expected/application-with-settings.yaml b/tests/Fixtures/parser/expected/application-with-settings.yaml index 3363af1f60..5e2316d90b 100644 --- a/tests/Fixtures/parser/expected/application-with-settings.yaml +++ b/tests/Fixtures/parser/expected/application-with-settings.yaml @@ -1621,15 +1621,15 @@ - 'referenceapplication0001_web-data-pr-7:/app/data' files: - - fs_path: /data/coolify/applications/referenceapplication0001/config-pr-7 + fs_path: /data/coolify/applications/referenceapplication0001/config mount_path: /app/config is_directory: true - - fs_path: /data/coolify/applications/referenceapplication0001/selinux-pr-7 + fs_path: /data/coolify/applications/referenceapplication0001/selinux mount_path: /app/selinux is_directory: true - - fs_path: /data/coolify/applications/referenceapplication0001/web-init.sh-pr-7 + fs_path: /data/coolify/applications/referenceapplication0001/web-init.sh mount_path: /docker-entrypoint.d/init.sh is_directory: false content: | @@ -2483,15 +2483,15 @@ - 'referenceapplication0001_web-data-pr-8:/app/data' files: - - fs_path: /data/coolify/applications/referenceapplication0001/config-pr-8 + fs_path: /data/coolify/applications/referenceapplication0001/config mount_path: /app/config is_directory: true - - fs_path: /data/coolify/applications/referenceapplication0001/selinux-pr-8 + fs_path: /data/coolify/applications/referenceapplication0001/selinux mount_path: /app/selinux is_directory: true - - fs_path: /data/coolify/applications/referenceapplication0001/web-init.sh-pr-8 + fs_path: /data/coolify/applications/referenceapplication0001/web-init.sh mount_path: /docker-entrypoint.d/init.sh is_directory: false content: | diff --git a/tests/Fixtures/parser/expected/application.yaml b/tests/Fixtures/parser/expected/application.yaml index fc3380cdbf..0d196b8ec2 100644 --- a/tests/Fixtures/parser/expected/application.yaml +++ b/tests/Fixtures/parser/expected/application.yaml @@ -1503,15 +1503,15 @@ - 'referenceapplication0001_web-data-pr-7:/app/data' files: - - fs_path: /data/coolify/applications/referenceapplication0001/config-pr-7 + fs_path: /data/coolify/applications/referenceapplication0001/config mount_path: /app/config is_directory: true - - fs_path: /data/coolify/applications/referenceapplication0001/selinux-pr-7 + fs_path: /data/coolify/applications/referenceapplication0001/selinux mount_path: /app/selinux is_directory: true - - fs_path: /data/coolify/applications/referenceapplication0001/web-init.sh-pr-7 + fs_path: /data/coolify/applications/referenceapplication0001/web-init.sh mount_path: /docker-entrypoint.d/init.sh is_directory: false content: | @@ -2303,15 +2303,15 @@ - 'referenceapplication0001_web-data-pr-8:/app/data' files: - - fs_path: /data/coolify/applications/referenceapplication0001/config-pr-8 + fs_path: /data/coolify/applications/referenceapplication0001/config mount_path: /app/config is_directory: true - - fs_path: /data/coolify/applications/referenceapplication0001/selinux-pr-8 + fs_path: /data/coolify/applications/referenceapplication0001/selinux mount_path: /app/selinux is_directory: true - - fs_path: /data/coolify/applications/referenceapplication0001/web-init.sh-pr-8 + fs_path: /data/coolify/applications/referenceapplication0001/web-init.sh mount_path: /docker-entrypoint.d/init.sh is_directory: false content: | diff --git a/tests/Support/RemoteProcessTestHelpers.php b/tests/Support/RemoteProcessTestHelpers.php index fd28084079..64b602b896 100644 --- a/tests/Support/RemoteProcessTestHelpers.php +++ b/tests/Support/RemoteProcessTestHelpers.php @@ -34,3 +34,11 @@ function fakeRemoteProcesses(array $handlers): void return Process::result(); }); } + +/** + * The output of LocalFileVolume::remoteFileStateCommand() when every checked path has the same state. + */ +function remoteFileStatesOutput(string $state, int $paths = 20): string +{ + return implode("\n", array_map(fn (int $index) => "{$index}:{$state}", range(1, $paths))); +} diff --git a/tests/Unit/LocalFileVolumeContentSizeTest.php b/tests/Unit/LocalFileVolumeContentSizeTest.php index 5ee1e3775d..ea666aedef 100644 --- a/tests/Unit/LocalFileVolumeContentSizeTest.php +++ b/tests/Unit/LocalFileVolumeContentSizeTest.php @@ -89,22 +89,13 @@ it('does not read regular bind-mounted file contents while loading service setti $application->shouldReceive('fileStorages')->once()->andReturn($fileStorages); $application->setRelation('destination', (object) ['server' => $server]); - Process::fake(function ($process) { - if (str_contains(remoteProcessScript($process), "test -f '/data/large.bin'")) { - return Process::result(output: 'OK'); - } - - if (str_contains(remoteProcessScript($process), "test -d '/data/large.bin'")) { - return Process::result(output: 'NOK'); - } - - return Process::result(); - }); + Process::fake(fn ($process) => Process::result( + output: str_contains(remoteProcessScript($process), 'empty-directory') ? '1:file' : '' + )); getFilesystemVolumesFromServer($application); expect($volume->is_directory)->toBeFalse(); - Process::assertRan(fn ($process) => str_contains(remoteProcessScript($process), "test -f '/data/large.bin'")); Process::assertNotRan(fn ($process) => str_contains(remoteProcessScript($process), 'cat /data/large.bin') || str_contains(remoteProcessScript($process), 'head -c')); }); @@ -194,13 +185,15 @@ it('quotes literal file-storage paths and safely expands persisted expressions', $expression->setRelation('resource', $application); Process::fake(fn ($process) => Process::result(output: match (true) { + str_contains(remoteProcessScript($process), 'empty-directory') => remoteFileStatesOutput('missing'), str_contains(remoteProcessScript($process), 'readlink -f') => 'OK', - str_contains(remoteProcessScript($process), 'test -') => 'NOK', default => '', })); getFilesystemVolumesFromServer($application, true); - Process::assertRan(fn ($process) => str_contains(remoteProcessScript($process), "test -f '/data/application/my files/config.yaml'")); + Process::assertRan(fn ($process) => str_contains(remoteProcessScript($process), 'empty-directory') + && str_contains(remoteProcessScript($process), 'my files/config.yaml') + && str_contains(remoteProcessScript($process), '"${DATA_PATH:-/srv/app/config.yaml}"')); Process::assertRan(fn ($process) => str_contains(remoteProcessScript($process), "mkdir -p -- '/data/application/my files/config.yaml'")); Process::assertRan(fn ($process) => str_contains(remoteProcessScript($process), "dirname -- '/data/application/my files/settings.json'")); Process::assertRan(fn ($process) => str_contains(remoteProcessScript($process), "tee -- '/data/application/my files/settings.json'"));