mirror of
https://github.com/coollabsio/coolify.git
synced 2026-11-06 13:10:37 -04:00
fix(compose): keep dotted service names literal and validate service names
Updating the image of a service named like "api.test" wrote a nested "test" key into the "api" service, because data_get/data_set split the name on dots. Use array key paths instead. Also refuse service names that Docker cannot use as a container name prefix, with a clear error on save. Closes #12456 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
32541a32ce
commit
05e52fc41d
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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 "<service name>-<suffix>". 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');
|
||||
|
||||
@@ -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 = [];
|
||||
|
||||
|
||||
@@ -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';
|
||||
|
||||
@@ -0,0 +1,70 @@
|
||||
<?php
|
||||
|
||||
use App\Models\Application;
|
||||
use App\Models\Environment;
|
||||
use App\Models\Project;
|
||||
use App\Models\Server;
|
||||
use App\Models\Service;
|
||||
use App\Models\StandaloneDocker;
|
||||
use App\Models\Team;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
use Illuminate\Support\Facades\Bus;
|
||||
|
||||
uses(RefreshDatabase::class);
|
||||
|
||||
beforeEach(function () {
|
||||
Bus::fake();
|
||||
|
||||
$team = Team::factory()->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-']);
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user