mirror of
https://github.com/coollabsio/coolify.git
synced 2026-11-06 13:10:37 -04:00
fix(compose): allow variables in external volume names
Accept Compose variables ($NAME, ${NAME}, ${NAME-default},
${NAME:-default}) in the `name:` of an external volume, and in the old
`external: {name: x}` syntax. Docker Compose resolves them from the
deployment .env. Coolify never removes external volumes, so it does not
need the resolved name.
The volume key must still be a literal Docker volume name, because
Compose does not resolve variables in keys. Names that are unsafe or use
unsupported syntax are still rejected, for example command substitution,
spaces, escaped dollars, and ${VAR:?err}.
Add tests for application and service parsing, legacy volume rows, the
StackForm save, legacy Compose parsing, and the rejected cases.
This commit is contained in:
@@ -549,19 +549,25 @@ function composeExternalVolumeMounts(?string $compose): array
|
||||
}
|
||||
|
||||
/**
|
||||
* The key and the name of an external volume must be literal Docker volume names. Docker Compose
|
||||
* can resolve variables in `name:`, but then only the deployment `.env` sets which existing volume
|
||||
* the resource mounts, and Coolify cannot show or check that volume. Thus variables are rejected.
|
||||
* The key of an external volume must be a literal Docker volume name, because Docker Compose does not
|
||||
* resolve variables in keys. The name (`name:` or the old `external: {name: x}`) may also contain
|
||||
* variables (see isComposeVolumeNameWithVariables()): Docker Compose resolves them from the deployment
|
||||
* `.env` when it starts the resource. Coolify never removes an external volume, because it gets no
|
||||
* storage entry, so Coolify does not need the resolved name.
|
||||
*
|
||||
* @param array<string, mixed> $declaration
|
||||
*
|
||||
* @throws Exception If the key or the name is not a literal, valid Docker volume name
|
||||
* @throws Exception If the key or the name is not a valid Docker volume name
|
||||
*/
|
||||
function validateComposeExternalVolume(int|string $key, array $declaration): void
|
||||
{
|
||||
$key = (string) $key;
|
||||
$label = preg_match(ValidationPatterns::VOLUME_NAME_PATTERN, $key) === 1 ? "volume {$key}" : 'volume';
|
||||
$names = [$key];
|
||||
if (preg_match(ValidationPatterns::VOLUME_NAME_PATTERN, $key) !== 1) {
|
||||
throw new Exception(
|
||||
'Invalid external Docker Compose volume. Volume names must start with an alphanumeric character and contain only alphanumeric characters, dots, hyphens, and underscores.'
|
||||
);
|
||||
}
|
||||
$names = [];
|
||||
if (array_key_exists('name', $declaration)) {
|
||||
$names[] = $declaration['name'];
|
||||
}
|
||||
@@ -570,19 +576,25 @@ function validateComposeExternalVolume(int|string $key, array $declaration): voi
|
||||
}
|
||||
|
||||
foreach ($names as $name) {
|
||||
if (is_string($name) && str_contains($name, '$')) {
|
||||
if (! is_string($name) || (preg_match(ValidationPatterns::VOLUME_NAME_PATTERN, $name) !== 1 && ! isComposeVolumeNameWithVariables($name))) {
|
||||
throw new Exception(
|
||||
"Invalid external Docker Compose {$label}: Coolify does not resolve variables in the name of an external volume. Use the literal name of the existing Docker volume."
|
||||
);
|
||||
}
|
||||
if (! is_string($name) || preg_match(ValidationPatterns::VOLUME_NAME_PATTERN, $name) !== 1) {
|
||||
throw new Exception(
|
||||
"Invalid external Docker Compose {$label}. Volume names must start with an alphanumeric character and contain only alphanumeric characters, dots, hyphens, and underscores."
|
||||
"Invalid external Docker Compose volume {$key}. The name must be a Docker volume name (alphanumeric characters, dots, hyphens, and underscores) and can contain variables such as \${NAME} or \${NAME:-default}."
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Tells if a volume name contains Compose variables and otherwise only Docker volume name characters.
|
||||
* The variables are `$NAME`, `${NAME}`, `${NAME-default}` and `${NAME:-default}`, with a default
|
||||
* that contains only Docker volume name characters.
|
||||
*/
|
||||
function isComposeVolumeNameWithVariables(string $name): bool
|
||||
{
|
||||
return str_contains($name, '$')
|
||||
&& preg_match('/^(?:[A-Za-z0-9._-]|\$[A-Za-z_][A-Za-z0-9_]*|\$\{[A-Za-z_][A-Za-z0-9_]*(?::?-[A-Za-z0-9._-]*)?\})+$/', $name) === 1;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validates a Docker volume string (format: "source:target" or "source:target:mode")
|
||||
*
|
||||
|
||||
@@ -151,6 +151,15 @@ test('loading a safe Compose file from Git saves it', function () {
|
||||
expect($this->application->refresh()->docker_compose_raw)->toBe(trim($compose));
|
||||
});
|
||||
|
||||
test('loading a Compose file with a variable external volume name saves it', function () {
|
||||
$compose = "services:\n web:\n image: nginx\n volumes:\n - 'shared-data:/data'\nvolumes:\n shared-data:\n external: true\n name: \${SHARED_VOLUME}\n";
|
||||
fakeRepositoryCompose($compose);
|
||||
|
||||
$this->application->loadComposeFile();
|
||||
|
||||
expect($this->application->fresh()->docker_compose_raw)->toBe(trim($compose));
|
||||
});
|
||||
|
||||
test('the queued LoadComposeFile action rejects injection', function () {
|
||||
fakeRepositoryCompose(composeInjectionPayloads()['service name command substitution'][0]);
|
||||
|
||||
|
||||
@@ -4,6 +4,7 @@ use App\Actions\Service\DeployServiceApplication;
|
||||
use App\Actions\Service\StartService;
|
||||
use App\Jobs\ApplicationDeploymentJob;
|
||||
use App\Jobs\DeleteResourceJob;
|
||||
use App\Livewire\Project\Service\StackForm;
|
||||
use App\Livewire\Project\Service\Storage as StoragePage;
|
||||
use App\Livewire\Project\Shared\Storages\All;
|
||||
use App\Models\Application;
|
||||
@@ -609,24 +610,112 @@ describe('legacy parsers', function () {
|
||||
});
|
||||
});
|
||||
|
||||
function variableExternalVolumeCompose(string $name, bool $oldSyntax = false): string
|
||||
{
|
||||
$declaration = $oldSyntax ? " external:\n name: '{$name}'\n" : " external: true\n name: '{$name}'\n";
|
||||
|
||||
return "services:\n web:\n image: nginx\n volumes:\n - 'shared-data:/data'\nvolumes:\n shared-data:\n{$declaration}";
|
||||
}
|
||||
|
||||
dataset('external volume names with a variable', [
|
||||
'variable' => ['${SHARED_VOLUME}', false],
|
||||
'variable with a default' => ['${SHARED_VOLUME:-shared}', false],
|
||||
'variable with text around it' => ['stack-${STACK}_data', false],
|
||||
'two variables' => ['${PROJECT}-${ENVIRONMENT}', false],
|
||||
'old syntax' => ['$SHARED_VOLUME', true],
|
||||
]);
|
||||
|
||||
describe('validation', function () {
|
||||
it('rejects an external volume name with a variable', function (string $compose) {
|
||||
expect(fn () => validateDockerComposeForInjection($compose))
|
||||
->toThrow(Exception::class, 'Coolify does not resolve variables in the name of an external volume');
|
||||
it('accepts an external volume name with a variable and leaves it to Docker Compose', function (string $name, bool $oldSyntax) {
|
||||
$compose = variableExternalVolumeCompose($name, $oldSyntax);
|
||||
$declaration = $oldSyntax ? ['external' => ['name' => $name]] : ['external' => true, 'name' => $name];
|
||||
validateDockerComposeForInjection($compose);
|
||||
|
||||
$application = externalVolumeApplication($compose);
|
||||
expect(fn () => applicationParser($application))
|
||||
->toThrow(Exception::class, 'Coolify does not resolve variables in the name of an external volume');
|
||||
$application = applicationParser(externalVolumeApplication($compose))->toArray();
|
||||
$service = serviceParser(externalVolumeService($compose))->toArray();
|
||||
|
||||
expect($application['services']['web']['volumes'])->toBe(['shared-data:/data'])
|
||||
->and($application['volumes'])->toBe(['shared-data' => $declaration])
|
||||
->and($service['services']['web']['volumes'])->toBe(['shared-data:/data'])
|
||||
->and($service['volumes'])->toBe(['shared-data' => $declaration])
|
||||
->and(persistentVolumeNames())->toBe([]);
|
||||
})->with('external volume names with a variable');
|
||||
|
||||
it('keeps the old volume of an existing application whose external volume name has a variable', function (string $name, bool $oldSyntax) {
|
||||
$application = externalVolumeApplication(variableExternalVolumeCompose($name, $oldSyntax));
|
||||
$uuid = $application->uuid;
|
||||
legacyExternalVolumeRow($application, "{$uuid}_shared-data");
|
||||
|
||||
$parsed = applicationParser($application)->toArray();
|
||||
|
||||
expect($parsed['services']['web']['volumes'])->toBe(["{$uuid}_shared-data:/data"])
|
||||
->and($parsed['volumes']["{$uuid}_shared-data"])->toBe(['name' => "{$uuid}_shared-data"])
|
||||
->and($application->composeVolumeWarnings())->toBe([legacyExternalVolumeWarning("{$uuid}_shared-data", $name)]);
|
||||
})->with('external volume names with a variable');
|
||||
|
||||
it('keeps the old volume of an existing service whose external volume name has a variable', function (string $name, bool $oldSyntax) {
|
||||
$compose = variableExternalVolumeCompose($name, $oldSyntax);
|
||||
$service = externalVolumeService($compose);
|
||||
expect(fn () => serviceParser($service))
|
||||
->toThrow(Exception::class, 'Coolify does not resolve variables in the name of an external volume');
|
||||
$uuid = $service->uuid;
|
||||
legacyExternalVolumeRow(legacyExternalVolumeServiceResource($service, 'web', $compose), "{$uuid}_shared-data");
|
||||
|
||||
$parsed = serviceParser($service)->toArray();
|
||||
|
||||
expect($parsed['services']['web']['volumes'])->toBe(["{$uuid}_shared-data:/data"])
|
||||
->and($parsed['volumes']["{$uuid}_shared-data"])->toBe(['name' => "{$uuid}_shared-data"])
|
||||
->and($service->composeVolumeWarnings())->toBe([legacyExternalVolumeWarning("{$uuid}_shared-data", $name)]);
|
||||
})->with('external volume names with a variable');
|
||||
|
||||
it('saves a service Compose file whose external volume name has a variable', function () {
|
||||
$compose = variableExternalVolumeCompose('${SHARED_VOLUME}');
|
||||
$service = externalVolumeService(EXTERNAL_VOLUME_SHORT_COMPOSE);
|
||||
$owner = User::factory()->create();
|
||||
$team = $service->environment->project->team;
|
||||
$team->members()->attach($owner, ['role' => 'owner']);
|
||||
$this->actingAs($owner);
|
||||
session(['currentTeam' => $team]);
|
||||
|
||||
Livewire::test(StackForm::class, ['service' => $service])
|
||||
->set('dockerComposeRaw', $compose)
|
||||
->call('submit')
|
||||
->assertDispatched('success');
|
||||
|
||||
expect($service->fresh()->docker_compose_raw)->toBe($compose);
|
||||
});
|
||||
|
||||
it('keeps the old behavior of a legacy Compose application whose external volume name has a variable', function (string $name, bool $oldSyntax) {
|
||||
$application = externalVolumeApplication(variableExternalVolumeCompose($name, $oldSyntax), '2');
|
||||
|
||||
$parsed = parseDockerComposeFile($application)->toArray();
|
||||
|
||||
expect($parsed['services']['web']['volumes'])->toBe(["{$application->uuid}-shared-data:/data"]);
|
||||
})->with('external volume names with a variable');
|
||||
|
||||
it('rejects an external volume name with an unsafe or unsupported variable', function (string $name) {
|
||||
$compose = variableExternalVolumeCompose($name);
|
||||
$application = externalVolumeApplication($compose);
|
||||
|
||||
expect(fn () => validateDockerComposeForInjection($compose))
|
||||
->toThrow(Exception::class, 'Invalid external Docker Compose volume shared-data')
|
||||
->and(fn () => applicationParser($application))
|
||||
->toThrow(Exception::class, 'Invalid external Docker Compose volume shared-data');
|
||||
})->with([
|
||||
'name field' => ["services:\n web:\n image: nginx\n volumes:\n - 'shared-data:/data'\nvolumes:\n shared-data:\n external: true\n name: \${SHARED_VOLUME}\n"],
|
||||
'name field with default' => ["services:\n web:\n image: nginx\n volumes:\n - 'shared-data:/data'\nvolumes:\n shared-data:\n external: true\n name: \${SHARED_VOLUME:-shared}\n"],
|
||||
'old syntax' => ["services:\n web:\n image: nginx\n volumes:\n - 'shared-data:/data'\nvolumes:\n shared-data:\n external:\n name: \$SHARED_VOLUME\n"],
|
||||
'command after a variable' => ['${SHARED_VOLUME}; rm -rf /'],
|
||||
'command substitution' => ['$(id)'],
|
||||
'space' => ['shared ${VOLUME}'],
|
||||
'dollar sign alone' => ['shared-$'],
|
||||
'escaped dollar sign' => ['$$SHARED_VOLUME'],
|
||||
'required variable syntax' => ['${SHARED_VOLUME:?required}'],
|
||||
'default with a space' => ['${SHARED_VOLUME:-a b}'],
|
||||
]);
|
||||
|
||||
it('rejects a variable in the key of an external volume', function () {
|
||||
$compose = "services:\n web:\n image: nginx\nvolumes:\n \${SHARED_VOLUME}:\n external: true\n";
|
||||
|
||||
expect(fn () => validateDockerComposeForInjection($compose))
|
||||
->toThrow(Exception::class, 'Invalid external Docker Compose volume. Volume names must start with an alphanumeric character');
|
||||
});
|
||||
|
||||
it('rejects an external volume name that is not a valid Docker volume name', function () {
|
||||
$compose = "services:\n web:\n image: nginx\nvolumes:\n shared-data:\n external: true\n name: 'shared data;rm -rf /'\n";
|
||||
|
||||
|
||||
Reference in New Issue
Block a user