diff --git a/app/Actions/Service/DeployServiceApplication.php b/app/Actions/Service/DeployServiceApplication.php index 927d3be6cf..f09967d8bd 100644 --- a/app/Actions/Service/DeployServiceApplication.php +++ b/app/Actions/Service/DeployServiceApplication.php @@ -67,7 +67,7 @@ class DeployServiceApplication if ($connectToPredefinedNetwork) { // Connect to the predefined network before the container starts (see StartService). - $serviceConfig = data_get(parseDockerComposeYaml($service->docker_compose ?? ''), "services.{$composeServiceName}"); + $serviceConfig = data_get(parseDockerComposeYaml($service->docker_compose ?? ''), ['services', $composeServiceName]); if ($command = StartService::predefinedNetworkConnectCommand($service, $composeServiceName, $serviceConfig)) { $commands->push($command); } diff --git a/bootstrap/helpers/parsers.php b/bootstrap/helpers/parsers.php index f442fb764a..0bcb561714 100644 --- a/bootstrap/helpers/parsers.php +++ b/bootstrap/helpers/parsers.php @@ -63,6 +63,7 @@ function validateDockerComposeForInjection(string $composeYaml): void $e ); } + validateComposeServiceName((string) $serviceName); // Validate volumes in this service (both string and array formats) if (isset($serviceConfig['volumes']) && is_array($serviceConfig['volumes'])) { @@ -208,6 +209,24 @@ function forgetComposeBindStorage(Application|ServiceApplication|ServiceDatabase * * @throws Exception If the source is missing or contains shell metacharacters */ +/** + * Coolify names each container "-". Docker allows only container names + * that match [a-zA-Z0-9][a-zA-Z0-9_.-]+, and Docker Compose allows only service names that + * match [a-zA-Z0-9._-]+. Compose accepts a service name that starts with ".", "-", or "_", + * but Docker then refuses to create its container. + * + * @throws Exception If Docker cannot use the service name at the start of a container name + */ +function validateComposeServiceName(string $serviceName): void +{ + if (preg_match('/^[a-zA-Z0-9][a-zA-Z0-9_.-]*$/', $serviceName) !== 1) { + throw new Exception( + "Service name \"{$serviceName}\" must start with a letter or a digit and contain only letters, digits, \".\", \"-\", and \"_\". ". + 'Docker uses the same rule for container names, and Coolify uses the service name at the start of the container name.' + ); + } +} + /** * Coolify sets container_name on every Compose service, and Docker Compose does not start more * than one container of a service that has a fixed container_name. @@ -1208,6 +1227,7 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int ' Service names must not contain shell metacharacters.' ); } + validateComposeServiceName((string) $serviceName); validateComposeServiceScale((string) $serviceName, $service); $magicEnvironments = collect([]); @@ -2491,6 +2511,7 @@ function serviceParser(Service $resource): Collection ' Service names must not contain shell metacharacters.' ); } + validateComposeServiceName((string) $serviceName); validateComposeServiceScale((string) $serviceName, $service); $image = data_get_str($service, 'image'); diff --git a/bootstrap/helpers/services.php b/bootstrap/helpers/services.php index 50e0b7441d..0ba3dda8c4 100644 --- a/bootstrap/helpers/services.php +++ b/bootstrap/helpers/services.php @@ -395,9 +395,9 @@ function updateCompose(ServiceApplication|ServiceDatabase $resource) // Switch Image $updatedImage = data_get_str($resource, 'image'); - $currentImage = data_get_str($dockerCompose, "services.{$name}.image"); + $currentImage = data_get_str($dockerCompose, ['services', $name, 'image']); if ($currentImage !== $updatedImage) { - data_set($dockerCompose, "services.{$name}.image", $updatedImage->value()); + data_set($dockerCompose, ['services', $name, 'image'], $updatedImage->value()); $dockerComposeRaw = Yaml::dump($dockerCompose, 10, 2); $resource->service->docker_compose_raw = $dockerComposeRaw; $resource->service->save(); @@ -409,7 +409,7 @@ function updateCompose(ServiceApplication|ServiceDatabase $resource) // to ensure we use the exact names defined in the template (which may be abbreviated) // IMPORTANT: Only extract variables that are DIRECTLY DECLARED for this service, // not variables that are merely referenced from other services - $serviceConfig = data_get($dockerCompose, "services.{$name}"); + $serviceConfig = data_get($dockerCompose, ['services', $name]); $environment = data_get($serviceConfig, 'environment') ?? []; $templateVariableNames = []; diff --git a/bootstrap/helpers/shared.php b/bootstrap/helpers/shared.php index 4a22597ec6..215b2e2434 100644 --- a/bootstrap/helpers/shared.php +++ b/bootstrap/helpers/shared.php @@ -4461,7 +4461,7 @@ function parseDockerComposeFile(Service|Application $resource, bool $isNew = fal } $noindexDomains = $pull_request_id !== 0 ? $fqdns : $resource->noindexDomains(); $shouldGenerateLabelsExactly = $server->settings->generate_exact_labels; - $composeRedirect = data_get($domains, "$serviceName.redirect"); + $composeRedirect = data_get($domains, [$serviceName, 'redirect']); $redirectDirection = in_array($composeRedirect, ['www', 'non-www', 'both'], true) ? $composeRedirect : 'both'; diff --git a/tests/Feature/ComposeServiceNameStartTest.php b/tests/Feature/ComposeServiceNameStartTest.php new file mode 100644 index 0000000000..f9f0f60c5a --- /dev/null +++ b/tests/Feature/ComposeServiceNameStartTest.php @@ -0,0 +1,70 @@ +create(); + $server = Server::factory()->create(['team_id' => $team->id]); + $this->destination = StandaloneDocker::where('server_id', $server->id)->firstOrFail(); + $project = Project::factory()->create(['team_id' => $team->id]); + $this->environment = Environment::factory()->create(['project_id' => $project->id]); +}); + +function serviceNameStartCompose(string $serviceName): string +{ + return "services:\n '{$serviceName}':\n image: alpine\n"; +} + +function serviceNameStartApplication(string $compose): Application +{ + return 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(); +} + +function serviceNameStartService(string $compose): Service +{ + return 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(), + ]); +} + +it('rejects a service name that does not follow the Docker container name rule', function (string $serviceName) { + $compose = serviceNameStartCompose($serviceName); + $message = "Service name \"{$serviceName}\" must start with a letter or a digit and contain only letters, digits,"; + + expect(fn () => validateDockerComposeForInjection($compose))->toThrow(Exception::class, $message) + ->and(fn () => applicationParser(serviceNameStartApplication($compose)))->toThrow(Exception::class, $message) + ->and(fn () => serviceParser(serviceNameStartService($compose)))->toThrow(Exception::class, $message); +})->with(['.api', '-api', '_api', 'api@x', 'api:x', 'api+x', 'äpi', 'api x', 'api/x']); + +it('accepts service names that follow the Docker container name rule', function (string $serviceName) { + $compose = serviceNameStartCompose($serviceName); + validateDockerComposeForInjection($compose); + $service = serviceNameStartService($compose); + + $parsed = serviceParser($service); + + expect(data_get($parsed, ['services', $serviceName, 'container_name']))->toBe("{$serviceName}-{$service->uuid}") + ->and(data_get(applicationParser(serviceNameStartApplication($compose)), ['services', $serviceName, 'image']))->toBe('alpine'); +})->with(['api.test', 'API', '9api', '1', 'a..b', 'api_', 'api-']); diff --git a/tests/Feature/ServiceApplicationsApiTest.php b/tests/Feature/ServiceApplicationsApiTest.php index 59f3414376..fd515c760a 100644 --- a/tests/Feature/ServiceApplicationsApiTest.php +++ b/tests/Feature/ServiceApplicationsApiTest.php @@ -245,6 +245,43 @@ describe('PATCH /api/v1/services/{uuid}/applications/{app_uuid}', function () { ]); }); + test('updates only the image of a parsed service whose name contains a dot', function () { + $service = Service::factory()->create([ + 'environment_id' => $this->environment->id, + 'server_id' => $this->server->id, + 'destination_id' => $this->destination->id, + 'destination_type' => $this->destination->getMorphClass(), + 'docker_compose_raw' => "services:\n api.test:\n image: nginx:1.26\n environment:\n KEEP: original\n api:\n image: redis:7\n", + ]); + $service->parse(); + $serviceApplication = $service->applications()->where('name', 'api.test')->firstOrFail(); + + $this->withHeaders([ + 'Authorization' => 'Bearer '.$this->bearerToken, + ])->patchJson("/api/v1/services/{$service->uuid}/applications/{$serviceApplication->uuid}", [ + 'image' => 'nginx:1.27', + ])->assertSuccessful(); + + $service = $service->fresh(); + expect(parseDockerComposeYaml($service->docker_compose_raw)['services'])->toBe([ + 'api.test' => [ + 'image' => 'nginx:1.27', + 'environment' => ['KEEP' => 'original'], + ], + 'api' => ['image' => 'redis:7'], + ]); + + $service->parse(); + $parsed = parseDockerComposeYaml($service->fresh()->docker_compose)['services']; + + expect(array_keys($parsed))->toBe(['api.test', 'api']) + ->and($parsed['api.test']['image'])->toBe('nginx:1.27') + ->and($parsed['api.test']['environment']['KEEP'])->toBe('original') + ->and($parsed['api']['image'])->toBe('redis:7') + ->and($service->applications()->pluck('image', 'name')->all())->toBe(['api.test' => 'nginx:1.27']) + ->and($service->databases()->pluck('image', 'name')->all())->toBe(['api' => 'redis:7']); + }); + test('updates the HTTP to HTTPS redirect setting', function () { config(['app.maintenance.driver' => 'file']); $ctx = createServiceWithApplicationForApiTest($this);