From 1d7bee0b83f1592d847dfd426734c78d475771c8 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Sun, 11 Oct 2026 21:02:29 +0200 Subject: [PATCH] fix(compose): preserve long bind options and non-creating sources (#12135) --- bootstrap/helpers/parsers.php | 174 ++++++++++++------ tests/Feature/ComposeLongBindOptionsTest.php | 182 +++++++++++++++++++ tests/Feature/DevHostDockerPathTest.php | 14 +- 3 files changed, 314 insertions(+), 56 deletions(-) create mode 100644 tests/Feature/ComposeLongBindOptionsTest.php diff --git a/bootstrap/helpers/parsers.php b/bootstrap/helpers/parsers.php index ada1bb03ec..01a5b04a66 100644 --- a/bootstrap/helpers/parsers.php +++ b/bootstrap/helpers/parsers.php @@ -140,6 +140,56 @@ function composeResourceDirectory(Application|Service $resource): string return $resource->workdir(); } +/** + * Renders a long-syntax bind with its resolved source. It keeps the options that Docker Compose + * accepts, such as read_only and bind, and removes other keys, which Compose rejects. Without + * `bind.create_host_path`, a missing source is created, as with short syntax: Docker Compose + * before v2.30 does not do that for long syntax. + * + * @param array $volume + * @return array + */ +function composeLongBindVolume(array $volume, string $source): array +{ + $volume = array_filter( + $volume, + fn ($key) => in_array($key, ['type', 'source', 'target', 'read_only', 'consistency', 'bind'], true) || str_starts_with((string) $key, 'x-'), + ARRAY_FILTER_USE_KEY + ); + $volume['source'] = $source; + $volume['bind'] = is_array($volume['bind'] ?? null) ? $volume['bind'] : []; + $volume['bind']['create_host_path'] ??= true; + + return $volume; +} + +/** + * A long-syntax bind with `bind.create_host_path: false` must fail when its host path is missing. + * Coolify must not create that path, so it does not add a file storage for it. A volume with + * `content:` is still written by Coolify. + */ +function composeBindKeepsMissingHostPath(mixed $volume): bool +{ + return is_array($volume) + && data_get($volume, 'bind.create_host_path') === false + && ! array_key_exists('content', $volume); +} + +/** + * Removes the file storage that an earlier save created for a bind that now keeps its missing host + * path. Otherwise, the storage jobs still create that path. The files on the server stay. A storage + * with a backup schedule stays, because it cannot be deleted before its schedule. + */ +function forgetComposeBindStorage(Application|ServiceApplication|ServiceDatabase $resource, string $mountPath): void +{ + $resource->fileStorages() + ->where('mount_path', $mountPath) + ->where('is_host_file', false) + ->whereDoesntHave('scheduledBackups') + ->get() + ->each->delete(); +} + /** * Coolify writes the `content:` of a Compose bind volume to the source path on the host. Only * administrators can edit a Compose file, and they can mount any host path, so the source can be @@ -1451,15 +1501,14 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int } } if ($type->value() === 'bind') { - if ($source->value() === '/var/run/docker.sock') { - $volume = $source->value().':'.$target->value(); - if (isset($parsed['mode']) && $parsed['mode']) { - $volume .= ':'.$parsed['mode']->value(); - } - } elseif ($source->value() === '/tmp' || $source->value() === '/tmp/') { - $volume = $source->value().':'.$target->value(); - if (isset($parsed['mode']) && $parsed['mode']) { - $volume .= ':'.$parsed['mode']->value(); + if (in_array($source->value(), ['/var/run/docker.sock', '/tmp', '/tmp/'], true)) { + if (is_string($volume)) { + $volume = $source->value().':'.$target->value(); + if (isset($parsed['mode']) && $parsed['mode']) { + $volume .= ':'.$parsed['mode']->value(); + } + } else { + $volume = composeLongBindVolume($volume, $source->value()); } } else { if ((int) $resource->compose_parsing_version >= 4) { @@ -1474,26 +1523,35 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int if ($isPullRequest && $isPreviewSuffixEnabled) { $source = str(addPreviewDeploymentSuffix($source, $pull_request_id)); } - LocalFileVolume::updateOrCreate( - [ - 'mount_path' => $target, - 'resource_id' => $originalResource->id, - 'resource_type' => get_class($originalResource), - ], - [ - 'fs_path' => $source, - 'mount_path' => $target, - 'content' => $content, - 'is_directory' => $isDirectory, - 'resource_id' => $originalResource->id, - 'resource_type' => get_class($originalResource), - ] - ); + // With `bind.create_host_path: false`, the host path must exist already: Coolify does not create or manage it. + if (! composeBindKeepsMissingHostPath($volume)) { + LocalFileVolume::updateOrCreate( + [ + 'mount_path' => $target, + 'resource_id' => $originalResource->id, + 'resource_type' => get_class($originalResource), + ], + [ + 'fs_path' => $source, + 'mount_path' => $target, + 'content' => $content, + 'is_directory' => $isDirectory, + 'resource_id' => $originalResource->id, + 'resource_type' => get_class($originalResource), + ] + ); + } else { + forgetComposeBindStorage($originalResource, $target->value()); + } // The file storage keeps the path that Coolify writes; the Docker daemon may need another one. $source = $source->replace($mainDirectory, devHostDockerPath($server, $mainDirectory->value())); - $volume = "$source:$target"; - if (isset($parsed['mode']) && $parsed['mode']) { - $volume .= ':'.$parsed['mode']->value(); + if (is_array($volume)) { + $volume = composeLongBindVolume($volume, $source->value()); + } else { + $volume = "$source:$target"; + if (isset($parsed['mode']) && $parsed['mode']) { + $volume .= ':'.$parsed['mode']->value(); + } } } } elseif ($type->value() === 'volume') { @@ -2853,15 +2911,14 @@ function serviceParser(Service $resource): Collection } } if ($type->value() === 'bind') { - if ($source->value() === '/var/run/docker.sock') { - $volume = $source->value().':'.$target->value(); - if (isset($parsed['mode']) && $parsed['mode']) { - $volume .= ':'.$parsed['mode']->value(); - } - } elseif ($source->value() === '/tmp' || $source->value() === '/tmp/') { - $volume = $source->value().':'.$target->value(); - if (isset($parsed['mode']) && $parsed['mode']) { - $volume .= ':'.$parsed['mode']->value(); + if (in_array($source->value(), ['/var/run/docker.sock', '/tmp', '/tmp/'], true)) { + if (is_string($volume)) { + $volume = $source->value().':'.$target->value(); + if (isset($parsed['mode']) && $parsed['mode']) { + $volume .= ':'.$parsed['mode']->value(); + } + } else { + $volume = composeLongBindVolume($volume, $source->value()); } } else { if ((int) $resource->compose_parsing_version >= 4) { @@ -2870,26 +2927,35 @@ function serviceParser(Service $resource): Collection $mainDirectory = str(base_configuration_dir().'/applications/'.$uuid); } $source = resolveComposeBindSource($source, $mainDirectory, $foundConfig?->fs_path); - LocalFileVolume::updateOrCreate( - [ - 'mount_path' => $target, - 'resource_id' => $originalResource->id, - 'resource_type' => get_class($originalResource), - ], - [ - 'fs_path' => $source, - 'mount_path' => $target, - 'content' => $content, - 'is_directory' => $isDirectory, - 'resource_id' => $originalResource->id, - 'resource_type' => get_class($originalResource), - ] - ); + // With `bind.create_host_path: false`, the host path must exist already: Coolify does not create or manage it. + if (! composeBindKeepsMissingHostPath($volume)) { + LocalFileVolume::updateOrCreate( + [ + 'mount_path' => $target, + 'resource_id' => $originalResource->id, + 'resource_type' => get_class($originalResource), + ], + [ + 'fs_path' => $source, + 'mount_path' => $target, + 'content' => $content, + 'is_directory' => $isDirectory, + 'resource_id' => $originalResource->id, + 'resource_type' => get_class($originalResource), + ] + ); + } else { + forgetComposeBindStorage($originalResource, $target->value()); + } // The file storage keeps the path that Coolify writes; the Docker daemon may need another one. $source = $source->replace($mainDirectory, devHostDockerPath($server, $mainDirectory->value())); - $volume = "$source:$target"; - if (isset($parsed['mode']) && $parsed['mode']) { - $volume .= ':'.$parsed['mode']->value(); + if (is_array($volume)) { + $volume = composeLongBindVolume($volume, $source->value()); + } else { + $volume = "$source:$target"; + if (isset($parsed['mode']) && $parsed['mode']) { + $volume .= ':'.$parsed['mode']->value(); + } } } } elseif ($type->value() === 'volume') { diff --git a/tests/Feature/ComposeLongBindOptionsTest.php b/tests/Feature/ComposeLongBindOptionsTest.php new file mode 100644 index 0000000000..4275747275 --- /dev/null +++ b/tests/Feature/ComposeLongBindOptionsTest.php @@ -0,0 +1,182 @@ + 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 longBindCompose(string $source, string $options): string +{ + return <<, storages: Collection, directory: string} + */ +function parseLongBind(string $kind, string $compose): array +{ + if ($kind === 'service') { + $resource = 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(), + ])->fresh(); + serviceParser($resource); + $parsed = serviceParser($resource->fresh())->toArray(); + $owner = $resource->applications()->firstOrFail(); + $directory = base_configuration_dir().'/services/'.$resource->uuid; + } else { + $resource = 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(), + ])->fresh(); + applicationParser($resource); + $parsed = applicationParser($resource->fresh())->toArray(); + $owner = $resource; + $directory = base_configuration_dir().'/applications/'.$resource->uuid; + } + + return [ + 'volume' => $parsed['services']['probe']['volumes'][0], + 'volumes' => $parsed['services']['probe']['volumes'], + 'storages' => $owner->fileStorages()->where('mount_path', '/fixture')->get(), + 'directory' => $directory, + ]; +} + +it('keeps a non-creating long-form bind and does not manage its host path', function (string $kind) { + $result = parseLongBind($kind, longBindCompose('/opt/strict-bind-repro/missing-directory', LONG_BIND_STRICT_OPTIONS)); + + expect($result['volume'])->toBe([ + 'type' => 'bind', + 'source' => '/opt/strict-bind-repro/missing-directory', + 'target' => '/fixture', + 'read_only' => true, + 'bind' => ['create_host_path' => false], + ])->and($result['storages'])->toBeEmpty(); +})->with(['service', 'application']); + +it('resolves the relative source of a non-creating long-form bind', function (string $kind) { + $result = parseLongBind($kind, longBindCompose('./missing', LONG_BIND_STRICT_OPTIONS)); + + expect($result['volume']['source'])->toBe($result['directory'].'/missing') + ->and($result['volume']['bind'])->toBe(['create_host_path' => false]) + ->and($result['storages'])->toBeEmpty(); +})->with(['service', 'application']); + +it('keeps the options of a long-form bind that Coolify manages', function (string $kind) { + $result = parseLongBind($kind, longBindCompose('./data', LONG_BIND_READ_ONLY_OPTIONS)); + + expect($result['volume'])->toBe([ + 'type' => 'bind', + 'source' => $result['directory'].'/data', + 'target' => '/fixture', + 'read_only' => true, + // Short syntax always created a missing source; Docker Compose before v2.30 does not do that for long syntax. + 'bind' => ['create_host_path' => true], + ])->and($result['storages']->pluck('fs_path')->all())->toBe([$result['directory'].'/data']); +})->with(['service', 'application']); + +it('removes volume keys that Docker Compose rejects from a long-form bind', function (string $kind, string $source) { + $result = parseLongBind($kind, longBindCompose($source, <<<'YAML' + mode: "0755" +YAML)); + + expect($result['volume'])->not->toHaveKey('mode') + ->and($result['volume']['bind'])->toBe(['create_host_path' => true]); +})->with(['service', 'application'])->with(['./entrypoint.sh', '/var/run/docker.sock']); + +it('removes the storage that an earlier save created when a bind keeps its missing host path', function (string $kind) { + $first = parseLongBind($kind, longBindCompose('/opt/strict-bind-repro/missing-directory', LONG_BIND_READ_ONLY_OPTIONS)); + expect($first['storages'])->toHaveCount(1); + + $owner = $first['storages']->first()->resource; + $resource = $kind === 'service' ? $owner->service : $owner; + $resource->update(['docker_compose_raw' => longBindCompose('/opt/strict-bind-repro/missing-directory', LONG_BIND_STRICT_OPTIONS)]); + $kind === 'service' ? serviceParser($resource->fresh()) : applicationParser($resource->fresh()); + + expect($owner->fileStorages()->where('mount_path', '/fixture')->exists())->toBeFalse(); +})->with(['service', 'application']); + +it('does not give a long-form bind the mode of an earlier short-syntax volume', function (string $kind) { + $result = parseLongBind($kind, <<<'YAML' +services: + probe: + image: alpine:3.22 + volumes: + - 'database_data:/backup/data:ro' + - '/var/run/docker.sock:/var/run/docker.sock:ro' + - type: bind + source: /var/backups + target: /archive + read_only: false +YAML); + + expect($result['volumes'][1])->toBe('/var/run/docker.sock:/var/run/docker.sock:ro') + ->and($result['volumes'][2])->toBe([ + 'type' => 'bind', + 'source' => '/var/backups', + 'target' => '/archive', + 'read_only' => false, + 'bind' => ['create_host_path' => true], + ]); +})->with(['service', 'application']); diff --git a/tests/Feature/DevHostDockerPathTest.php b/tests/Feature/DevHostDockerPathTest.php index ef647f024d..484264c461 100644 --- a/tests/Feature/DevHostDockerPathTest.php +++ b/tests/Feature/DevHostDockerPathTest.php @@ -150,7 +150,12 @@ it('mounts application compose bind and content volumes from the server data pat $volumes = applicationParser($application)->get('services')->get('app')['volumes']; $directory = "{$base}/applications/{$application->uuid}"; - expect(collect($volumes)->all())->toContain("{$directory}/data:/app/data", "{$directory}/config/app.conf:/etc/app.conf") + expect(collect($volumes)->all())->toContain("{$directory}/data:/app/data", [ + 'type' => 'bind', + 'source' => "{$directory}/config/app.conf", + 'target' => '/etc/app.conf', + 'bind' => ['create_host_path' => true], + ]) // Coolify writes the files through SSH to the normal path. ->and($application->fileStorages()->pluck('fs_path')->all()) ->toContain("/data/coolify/applications/{$application->uuid}/config/app.conf"); @@ -171,7 +176,12 @@ it('mounts service compose bind and content volumes from the server data path', $volumes = serviceParser($service->fresh())->get('services')->get('app')['volumes']; $directory = "{$base}/services/{$service->uuid}"; - expect(collect($volumes)->all())->toContain("{$directory}/data:/app/data", "{$directory}/config/app.conf:/etc/app.conf"); + expect(collect($volumes)->all())->toContain("{$directory}/data:/app/data", [ + 'type' => 'bind', + 'source' => "{$directory}/config/app.conf", + 'target' => '/etc/app.conf', + 'bind' => ['create_host_path' => true], + ]); })->with(devPathKinds()); it('mounts the Traefik proxy and log rotation directory from the server data path', function (string $kind, string $base) {