rosslh/coolify
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

refactor(jobs): extract container resolution logic for deployment commands

Browse source 
Extract common container selection logic into resolveCommandContainer() method
that handles both single and multi-container app scenarios. This consolidates
duplicated code from run_pre_deployment_command() and run_post_deployment_command()
while improving error messaging and test coverage.
This commit is contained in:
Andras Bacsai 2026-03-25 14:09:07 +01:00
parent b8b49b9f42
commit 811ee5d327
2 changed files with 217 additions and 52 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

153
app/Jobs/ApplicationDeploymentJob.php
View file

@ -3989,6 +3989,51 @@ private function validateContainerName(string $value): string
return $value;
}
/**
* Resolve which container to execute a deployment command in.
*
* For single-container apps, returns the sole container.
* For multi-container apps, matches by the user-specified container name.
* If no container name is specified for multi-container apps, logs available containers and returns null.
*/
private function resolveCommandContainer(Collection $containers, ?string $specifiedContainerName, string $commandType): ?array
{
if ($containers->count() === 0) {
return null;
}
if ($containers->count() === 1) {
return $containers->first();
}
// Multi-container: require a container name to be specified
if (empty($specifiedContainerName)) {
$available = $containers->map(fn ($c) => data_get($c, 'Names'))->implode(', ');
$this->application_deployment_queue->addLogEntry(
"{$commandType} command: Multiple containers found but no container name specified. Available: {$available}"
);
return null;
}
// Multi-container: match by specified name prefix
$prefix = $specifiedContainerName.'-'.$this->application->uuid;
foreach ($containers as $container) {
$containerName = data_get($container, 'Names');
if (str_starts_with($containerName, $prefix)) {
return $container;
}
}
// No match found — log available containers to help the user debug
$available = $containers->map(fn ($c) => data_get($c, 'Names'))->implode(', ');
$this->application_deployment_queue->addLogEntry(
"{$commandType} command: Container '{$specifiedContainerName}' not found. Available: {$available}"
);
return null;
}
private function run_pre_deployment_command()
{
if (empty($this->application->pre_deployment_command)) {
@ -3996,36 +4041,36 @@ private function run_pre_deployment_command()
}
$containers = getCurrentApplicationContainerStatus($this->server, $this->application->id, $this->pull_request_id);
if ($containers->count() == 0) {
$this->application_deployment_queue->addLogEntry('Pre-deployment command: No running containers found. Skipping.');
return;
}
$this->application_deployment_queue->addLogEntry('Executing pre-deployment command (see debug log for output/errors).');
foreach ($containers as $container) {
$containerName = data_get($container, 'Names');
if ($containerName) {
$this->validateContainerName($containerName);
}
if ($containers->count() == 1 || str_starts_with($containerName, $this->application->pre_deployment_command_container.'-'.$this->application->uuid)) {
// Security: pre_deployment_command is intentionally treated as arbitrary shell input.
// Users (team members with deployment access) need full shell flexibility to run commands
// like "php artisan migrate", "npm run build", etc. inside their own application containers.
// The trust boundary is at the application/team ownership level — only authenticated team
// members can set these commands, and execution is scoped to the application's own container.
// The single-quote escaping here prevents breaking out of the sh -c wrapper, but does not
// restrict the command itself. Container names are validated separately via validateContainerName().
$cmd = "sh -c '".str_replace("'", "'\''", $this->application->pre_deployment_command)."'";
$exec = "docker exec {$containerName} {$cmd}";
$this->execute_remote_command(
[
'command' => $exec,
'hidden' => true,
],
);
return;
}
$container = $this->resolveCommandContainer($containers, $this->application->pre_deployment_command_container, 'Pre-deployment');
if ($container === null) {
throw new DeploymentException('Pre-deployment command: Could not find a valid container. Is the container name correct?');
}
throw new DeploymentException('Pre-deployment command: Could not find a valid container. Is the container name correct?');
$containerName = data_get($container, 'Names');
if ($containerName) {
$this->validateContainerName($containerName);
}
// Security: pre_deployment_command is intentionally treated as arbitrary shell input.
// Users (team members with deployment access) need full shell flexibility to run commands
// like "php artisan migrate", "npm run build", etc. inside their own application containers.
// The trust boundary is at the application/team ownership level — only authenticated team
// members can set these commands, and execution is scoped to the application's own container.
// The single-quote escaping here prevents breaking out of the sh -c wrapper, but does not
// restrict the command itself. Container names are validated separately via validateContainerName().
$cmd = "sh -c '".str_replace("'", "'\''", $this->application->pre_deployment_command)."'";
$exec = "docker exec {$containerName} {$cmd}";
$this->execute_remote_command(
[
'command' => $exec,
'hidden' => true,
],
);
}
private function run_post_deployment_command()
@ -4037,36 +4082,40 @@ private function run_post_deployment_command()
$this->application_deployment_queue->addLogEntry('Executing post-deployment command (see debug log for output).');
$containers = getCurrentApplicationContainerStatus($this->server, $this->application->id, $this->pull_request_id);
foreach ($containers as $container) {
$containerName = data_get($container, 'Names');
if ($containerName) {
$this->validateContainerName($containerName);
}
if ($containers->count() == 1 || str_starts_with($containerName, $this->application->post_deployment_command_container.'-'.$this->application->uuid)) {
// Security: post_deployment_command is intentionally treated as arbitrary shell input.
// See the equivalent comment in run_pre_deployment_command() for the full security rationale.
$cmd = "sh -c '".str_replace("'", "'\''", $this->application->post_deployment_command)."'";
$exec = "docker exec {$containerName} {$cmd}";
try {
$this->execute_remote_command(
[
'command' => $exec,
'hidden' => true,
'save' => 'post-deployment-command-output',
],
);
} catch (Exception $e) {
$post_deployment_command_output = $this->saved_outputs->get('post-deployment-command-output');
if ($post_deployment_command_output) {
$this->application_deployment_queue->addLogEntry('Post-deployment command failed.');
$this->application_deployment_queue->addLogEntry($post_deployment_command_output, 'stderr');
}
}
if ($containers->count() == 0) {
$this->application_deployment_queue->addLogEntry('Post-deployment command: No running containers found. Skipping.');
return;
return;
}
$container = $this->resolveCommandContainer($containers, $this->application->post_deployment_command_container, 'Post-deployment');
if ($container === null) {
throw new DeploymentException('Post-deployment command: Could not find a valid container. Is the container name correct?');
}
$containerName = data_get($container, 'Names');
if ($containerName) {
$this->validateContainerName($containerName);
}
// Security: post_deployment_command is intentionally treated as arbitrary shell input.
// See the equivalent comment in run_pre_deployment_command() for the full security rationale.
$cmd = "sh -c '".str_replace("'", "'\''", $this->application->post_deployment_command)."'";
$exec = "docker exec {$containerName} {$cmd}";
try {
$this->execute_remote_command(
[
'command' => $exec,
'hidden' => true,
'save' => 'post-deployment-command-output',
],
);
} catch (Exception $e) {
$post_deployment_command_output = $this->saved_outputs->get('post-deployment-command-output');
if ($post_deployment_command_output) {
$this->application_deployment_queue->addLogEntry('Post-deployment command failed.');
$this->application_deployment_queue->addLogEntry($post_deployment_command_output, 'stderr');
}
}
throw new DeploymentException('Post-deployment command: Could not find a valid container. Is the container name correct?');
}
/**

116
tests/Feature/DeploymentCommandContainerResolutionTest.php Normal file
View file

@ -0,0 +1,116 @@
<?php
use App\Jobs\ApplicationDeploymentJob;
use App\Models\Application;
use App\Models\ApplicationDeploymentQueue;
function createJobWithProperties(string $uuid): object
{
$ref = new ReflectionClass(ApplicationDeploymentJob::class);
$instance = $ref->newInstanceWithoutConstructor();
$app = Mockery::mock(Application::class)->makePartial();
$app->uuid = $uuid;
$queue = Mockery::mock(ApplicationDeploymentQueue::class)->makePartial();
$queue->shouldReceive('addLogEntry')->andReturnNull();
$appProp = $ref->getProperty('application');
$appProp->setAccessible(true);
$appProp->setValue($instance, $app);
$queueProp = $ref->getProperty('application_deployment_queue');
$queueProp->setAccessible(true);
$queueProp->setValue($instance, $queue);
return $instance;
}
function invokeResolve(object $instance, $containers, ?string $specifiedName, string $type): ?array
{
$ref = new ReflectionClass(ApplicationDeploymentJob::class);
$method = $ref->getMethod('resolveCommandContainer');
$method->setAccessible(true);
return $method->invoke($instance, $containers, $specifiedName, $type);
}
describe('resolveCommandContainer', function () {
test('returns null when no containers exist', function () {
$instance = createJobWithProperties('abc123');
$result = invokeResolve($instance, collect([]), 'web', 'Pre-deployment');
expect($result)->toBeNull();
});
test('returns the sole container when only one exists', function () {
$container = ['Names' => 'web-abc123', 'Labels' => ''];
$instance = createJobWithProperties('abc123');
$result = invokeResolve($instance, collect([$container]), null, 'Pre-deployment');
expect($result)->toBe($container);
});
test('returns the sole container regardless of specified name when only one exists', function () {
$container = ['Names' => 'web-abc123', 'Labels' => ''];
$instance = createJobWithProperties('abc123');
$result = invokeResolve($instance, collect([$container]), 'wrong-name', 'Pre-deployment');
expect($result)->toBe($container);
});
test('returns null when no container name specified for multi-container app', function () {
$containers = collect([
['Names' => 'web-abc123', 'Labels' => ''],
['Names' => 'worker-abc123', 'Labels' => ''],
]);
$instance = createJobWithProperties('abc123');
$result = invokeResolve($instance, $containers, null, 'Pre-deployment');
expect($result)->toBeNull();
});
test('returns null when empty string container name for multi-container app', function () {
$containers = collect([
['Names' => 'web-abc123', 'Labels' => ''],
['Names' => 'worker-abc123', 'Labels' => ''],
]);
$instance = createJobWithProperties('abc123');
$result = invokeResolve($instance, $containers, '', 'Pre-deployment');
expect($result)->toBeNull();
});
test('matches correct container by specified name in multi-container app', function () {
$containers = collect([
['Names' => 'web-abc123', 'Labels' => ''],
['Names' => 'worker-abc123', 'Labels' => ''],
]);
$instance = createJobWithProperties('abc123');
$result = invokeResolve($instance, $containers, 'worker', 'Pre-deployment');
expect($result)->toBe(['Names' => 'worker-abc123', 'Labels' => '']);
});
test('returns null when specified container name does not match any container', function () {
$containers = collect([
['Names' => 'web-abc123', 'Labels' => ''],
['Names' => 'worker-abc123', 'Labels' => ''],
]);
$instance = createJobWithProperties('abc123');
$result = invokeResolve($instance, $containers, 'nonexistent', 'Pre-deployment');
expect($result)->toBeNull();
});
test('matches container with PR suffix', function () {
$containers = collect([
['Names' => 'web-abc123-pr-42', 'Labels' => ''],
['Names' => 'worker-abc123-pr-42', 'Labels' => ''],
]);
$instance = createJobWithProperties('abc123');
$result = invokeResolve($instance, $containers, 'web', 'Pre-deployment');
expect($result)->toBe(['Names' => 'web-abc123-pr-42', 'Labels' => '']);
});
});