fix(service): save switches without saving pending Compose edits (#11985)

This commit is contained in:
Andras Bacsai
2026-09-24 13:43:37 +02:00
committed by GitHub
parent 803cea718b
commit 3f47bae0b6
12 changed files with 359 additions and 18 deletions
@@ -696,8 +696,7 @@ class ServicesController extends Controller
],
], 422);
}
$dockerCompose = base64_decode($request->docker_compose_raw);
$dockerComposeRaw = Yaml::dump(Yaml::parse($dockerCompose), 10, 2, Yaml::DUMP_MULTI_LINE_LITERAL_BLOCK);
Yaml::parse($dockerComposeRaw);
// Validate for command injection BEFORE saving to database
try {
@@ -1243,8 +1242,7 @@ class ServicesController extends Controller
],
], 422);
}
$dockerCompose = base64_decode($request->docker_compose_raw);
$dockerComposeRaw = Yaml::dump(Yaml::parse($dockerCompose), 10, 2, Yaml::DUMP_MULTI_LINE_LITERAL_BLOCK);
Yaml::parse($dockerComposeRaw);
// Validate for command injection BEFORE saving to database
try {
+1 -1
View File
@@ -38,7 +38,7 @@ class DockerCompose extends Component
$this->validate([
'dockerComposeRaw' => 'required',
]);
$this->dockerComposeRaw = Yaml::dump(Yaml::parse($this->dockerComposeRaw), 10, 2, Yaml::DUMP_MULTI_LINE_LITERAL_BLOCK);
Yaml::parse($this->dockerComposeRaw);
// Validate for command injection BEFORE saving to database
validateDockerComposeForInjection($this->dockerComposeRaw);
+3 -2
View File
@@ -91,8 +91,9 @@ class EditCompose extends Component
$this->validate([
'isContainerLabelEscapeEnabled' => 'required',
]);
$this->syncData(true);
$this->service->save(['is_container_label_escape_enabled' => $this->isContainerLabelEscapeEnabled]);
$this->service->refresh()->update([
'is_container_label_escape_enabled' => $this->isContainerLabelEscapeEnabled,
]);
$this->dispatch('success', 'Service updated successfully');
} catch (\Throwable $e) {
return handleError($e, $this);
+3 -2
View File
@@ -149,8 +149,9 @@ class StackForm extends Component
{
try {
$this->authorize('update', $this->service);
$this->syncData(true);
$this->service->save();
$this->service->refresh()->update([
'connect_to_docker_network' => $this->connectToDockerNetwork,
]);
$this->dispatch('success', 'Service settings saved.');
} catch (\Throwable $e) {
return handleError($e, $this);
+91 -2
View File
@@ -408,6 +408,89 @@ function addTraefikDockerNetworkLabel(Collection $labels, string $network): Coll
return $labels;
}
/**
* Remove one-time fields from long-form volume entries without reformatting the rest of the source.
* Fall back to a YAML dump when the source uses a form that the line edit cannot handle safely.
*
* @param array<string, mixed> $cleanedYaml
* @param array<int, string> $fields
*/
function removeComposeVolumeFieldsPreservingComments(string $source, array $cleanedYaml, array $fields): string
{
$context = [];
$removeIndent = null;
$blockIndent = null;
$result = [];
foreach (preg_split('/(?<=\n)/', $source) as $line) {
$text = rtrim($line, "\r\n");
$indent = strspn($text, ' ');
if ($removeIndent !== null) {
if (trim($text) === '' || $indent > $removeIndent) {
continue;
}
$removeIndent = null;
}
if ($blockIndent !== null) {
if (trim($text) === '' || $indent > $blockIndent) {
$result[] = $line;
continue;
}
$blockIndent = null;
}
if (trim($text) === '' || str_starts_with(ltrim($text), '#')) {
$result[] = $line;
continue;
}
while ($context && end($context)['indent'] >= $indent) {
array_pop($context);
}
$body = substr($text, $indent);
$isListItem = preg_match('/^-\s+/', $body) === 1;
if ($isListItem) {
$context[] = ['indent' => $indent, 'key' => '[]'];
$body = preg_replace('/^-\s+/', '', $body);
}
if (preg_match('/^([\w.-]+|"[^"]+"|\x27[^\x27]+\x27)\s*:(.*)$/', $body, $matches)) {
$key = trim($matches[1], "\"'");
$path = array_column($context, 'key');
if (! $isListItem && count($path) === 4 && $path[0] === 'services' && $path[2] === 'volumes' && $path[3] === '[]' && in_array($key, $fields, true)) {
$removeIndent = $indent;
continue;
}
$value = trim($matches[2]);
if ($value === '' || str_starts_with($value, '#')) {
$context[] = ['indent' => $indent, 'key' => $key];
} elseif (preg_match('/^[|>][+-]?(?:\s+#.*)?$/', $value)) {
$blockIndent = $indent;
}
}
$result[] = $line;
}
$candidate = implode('', $result);
try {
if (Yaml::parse($candidate) === $cleanedYaml) {
return $candidate;
}
} catch (Exception) {
// Use the validated parsed result if the line edit is not valid YAML.
}
return Yaml::dump($cleanedYaml, 10, 2);
}
function applicationParser(Application $resource, int $pull_request_id = 0, ?int $preview_id = null, ?string $commit = null): Collection
{
$uuid = data_get($resource, 'uuid');
@@ -1558,6 +1641,7 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int
// Parse the original compose again to create a clean version without Coolify additions
try {
$originalYaml = Yaml::parse($originalCompose);
$originalYamlBeforeCleanup = $originalYaml;
// Remove content, isDirectory, and is_directory from all volume definitions
if (isset($originalYaml['services'])) {
foreach ($originalYaml['services'] as $serviceName => &$service) {
@@ -1572,7 +1656,9 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int
}
}
}
$resource->docker_compose_raw = Yaml::dump($originalYaml, 10, 2);
if ($originalYaml !== $originalYamlBeforeCleanup) {
$resource->docker_compose_raw = removeComposeVolumeFieldsPreservingComments($originalCompose, $originalYaml, ['content', 'isDirectory', 'is_directory']);
}
} catch (Exception) {
// If parsing fails, keep the original docker_compose_raw unchanged
}
@@ -2797,6 +2883,7 @@ function serviceParser(Service $resource): Collection
// Parse the original compose again to create a clean version without Coolify additions
try {
$originalYaml = Yaml::parse($originalCompose);
$originalYamlBeforeCleanup = $originalYaml;
// Remove content, isDirectory, and is_directory from all volume definitions
if (isset($originalYaml['services'])) {
foreach ($originalYaml['services'] as $serviceName => &$service) {
@@ -2811,7 +2898,9 @@ function serviceParser(Service $resource): Collection
}
}
}
$resource->docker_compose_raw = Yaml::dump($originalYaml, 10, 2);
if ($originalYaml !== $originalYamlBeforeCleanup) {
$resource->docker_compose_raw = removeComposeVolumeFieldsPreservingComments($originalCompose, $originalYaml, ['content', 'isDirectory', 'is_directory']);
}
} catch (Exception $e) {
// If parsing fails, keep the original docker_compose_raw unchanged
}
+7 -3
View File
@@ -1617,7 +1617,7 @@ function sanitizeLogsForExport(string $text): string
return remove_iip($text);
}
function getTopLevelNetworks(Service|Application $resource)
function getTopLevelNetworks(Service|Application $resource): Collection
{
if ($resource->getMorphClass() === Service::class) {
if ($resource->docker_compose_raw) {
@@ -1743,6 +1743,8 @@ function getTopLevelNetworks(Service|Application $resource)
return $topLevelNetworks->keys();
}
return collect();
}
function sourceIsLocal(Stringable $source)
{
@@ -3312,8 +3314,11 @@ function parseDockerComposeFile(Service|Application $resource, bool $isNew = fal
'configs' => $topLevelConfigs->toArray(),
'secrets' => $topLevelSecrets->toArray(),
];
$originalYaml = $yaml;
$yaml = data_forget($yaml, 'services.*.volumes.*.content');
$resource->docker_compose_raw = Yaml::dump($yaml, 10, 2);
if ($yaml !== $originalYaml) {
$resource->docker_compose_raw = removeComposeVolumeFieldsPreservingComments($resource->docker_compose_raw, $yaml, ['content']);
}
$resource->docker_compose = Yaml::dump($finalServices, 10, 2);
$resource->save();
@@ -4088,7 +4093,6 @@ function parseDockerComposeFile(Service|Application $resource, bool $isNew = fal
'configs' => $topLevelConfigs->toArray(),
'secrets' => $topLevelSecrets->toArray(),
];
$resource->docker_compose_raw = Yaml::dump($yaml, 10, 2);
$resource->docker_compose = Yaml::dump($finalServices, 10, 2);
data_forget($resource, 'environment_variables');
data_forget($resource, 'environment_variables_preview');
@@ -12,6 +12,7 @@ use App\Models\StandaloneDocker;
use App\Models\Team;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Illuminate\Support\Facades\Bus;
use Symfony\Component\Yaml\Yaml;
uses(RefreshDatabase::class);
@@ -89,6 +90,55 @@ function seedFileVolume($resource, string $baseDir, string $fileName, string $mo
]);
}
it('preserves comments in a service source Compose when parsing', function () {
$source = "# Service notes\nservices:\n app:\n # Keep this image note\n image: nginx:latest # pinned by operator\n";
[$service] = makeComposeService($source);
serviceParser($service);
expect($service->fresh()->docker_compose_raw)->toBe($source)
->and($service->fresh()->docker_compose)->toContain('services:');
});
it('preserves comments in an application source Compose when parsing', function () {
$source = "# Application notes\nservices:\n app:\n # Keep this image note\n image: nginx:latest # pinned by operator\n";
$application = makeComposeApplication($source);
applicationParser($application);
expect($application->fresh()->docker_compose_raw)->toBe($source)
->and($application->fresh()->docker_compose)->toContain('services:');
});
it('removes one-time volume fields without losing service source comments', function () {
$source = "# Service note\nservices:\n app:\n image: nginx:latest # Image note\n command: |\n volumes:\n - type: bind\n content: keep-this-command\n volumes:\n # Volume note\n - type: bind\n source: ./config.txt\n target: /app/config.txt\n content: |\n first line\n second line\n isDirectory: false\n # After content\n";
[$service] = makeComposeService($source);
serviceParser($service);
expect($service->fresh()->docker_compose_raw)->toBe("# Service note\nservices:\n app:\n image: nginx:latest # Image note\n command: |\n volumes:\n - type: bind\n content: keep-this-command\n volumes:\n # Volume note\n - type: bind\n source: ./config.txt\n target: /app/config.txt\n # After content\n");
});
it('removes one-time volume fields without losing application source comments', function () {
$source = "# Application note\nservices:\n app:\n image: nginx:latest # Image note\n volumes:\n - type: bind\n source: ./config.txt\n target: /app/config.txt\n content: initial\n is_directory: false\n # After content\n";
$application = makeComposeApplication($source);
applicationParser($application);
expect($application->fresh()->docker_compose_raw)->toBe("# Application note\nservices:\n app:\n image: nginx:latest # Image note\n volumes:\n - type: bind\n source: ./config.txt\n target: /app/config.txt\n # After content\n");
});
it('keeps valid Compose when one-time fields use flow syntax', function () {
$source = "# Flow-style volume\nservices:\n app:\n image: nginx:latest\n volumes: [{type: bind, source: ./config.txt, target: /app/config.txt, content: initial}]\n";
$cleanedYaml = Yaml::parse($source);
unset($cleanedYaml['services']['app']['volumes'][0]['content']);
$cleanedSource = removeComposeVolumeFieldsPreservingComments($source, $cleanedYaml, ['content']);
expect(Yaml::parse($cleanedSource))->toBe($cleanedYaml)
->and($cleanedSource)->not->toContain('content: initial');
});
it('preserves existing application file volume content when reparsing compose bind mounts', function () {
$application = makeComposeApplication(TWO_FILE_COMPOSE);
$baseDir = application_configuration_dir()."/{$application->uuid}";
@@ -0,0 +1,34 @@
<?php
use App\Models\Project;
use App\Models\Server;
use App\Models\Service;
use App\Models\Team;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Illuminate\Support\Collection;
uses(RefreshDatabase::class);
test('an empty service compose contributes no networks and does not block proxy network setup', function () {
$team = Team::factory()->create();
$server = Server::factory()->create(['team_id' => $team->id]);
$destination = $server->standaloneDockers()->firstOrFail();
$project = Project::factory()->create(['team_id' => $team->id]);
$service = Service::factory()->create([
'docker_compose_raw' => '',
'docker_compose' => "services:\n app:\n image: nginx:alpine\n",
'server_id' => $server->id,
'destination_id' => $destination->id,
'destination_type' => $destination->getMorphClass(),
'environment_id' => $project->environments()->firstOrFail()->id,
]);
expect($service->networks())->toBeInstanceOf(Collection::class)->toBeEmpty();
$commands = ensureProxyNetworksExist($server)->implode("\n");
expect($commands)
->toContain("docker network inspect 'coolify'")
->not->toContain("docker network inspect ''");
});
@@ -12,7 +12,7 @@ use Illuminate\Foundation\Testing\RefreshDatabase;
uses(RefreshDatabase::class);
beforeEach(function () {
InstanceSettings::create(['id' => 0, 'is_api_enabled' => true]);
InstanceSettings::unguarded(fn () => InstanceSettings::create(['id' => 0, 'is_api_enabled' => true]));
$this->team = Team::factory()->create();
$this->user = User::factory()->create();
@@ -37,7 +37,41 @@ function serviceContainerLabelAuthHeaders($bearerToken): array
];
}
test('service API creation preserves source Compose comments', function () {
$source = "# Operator note\nservices:\n app:\n image: nginx:alpine # Keep this note\n";
$response = $this->withHeaders(serviceContainerLabelAuthHeaders($this->bearerToken))
->postJson('/api/v1/services', [
'project_uuid' => $this->project->uuid,
'environment_uuid' => $this->environment->uuid,
'server_uuid' => $this->server->uuid,
'docker_compose_raw' => base64_encode($source),
]);
$response->assertSuccessful();
expect(Service::whereUuid($response->json('uuid'))->firstOrFail()->docker_compose_raw)->toBe($source);
});
describe('PATCH /api/v1/services/{uuid}', function () {
test('preserves source Compose comments when updating a service', function () {
$service = Service::factory()->create([
'server_id' => $this->server->id,
'destination_id' => $this->destination->id,
'destination_type' => $this->destination->getMorphClass(),
'environment_id' => $this->environment->id,
]);
$source = "# Operator note\nservices:\n app:\n image: nginx:alpine # Keep this note\n";
$response = $this->withHeaders(serviceContainerLabelAuthHeaders($this->bearerToken))
->patchJson("/api/v1/services/{$service->uuid}", [
'docker_compose_raw' => base64_encode($source),
]);
$response->assertSuccessful();
expect($service->fresh()->docker_compose_raw)->toBe($source);
});
test('accepts is_container_label_escape_enabled field', function () {
$service = Service::factory()->create([
'server_id' => $this->server->id,
@@ -54,7 +88,7 @@ describe('PATCH /api/v1/services/{uuid}', function () {
$response->assertStatus(200);
$service->refresh();
expect($service->is_container_label_escape_enabled)->toBeFalse();
expect($service->is_container_label_escape_enabled)->toBeFalsy();
});
test('rejects invalid is_container_label_escape_enabled value', function () {
@@ -0,0 +1,99 @@
<?php
use App\Livewire\Project\Service\EditCompose;
use App\Livewire\Project\Service\StackForm;
use App\Models\InstanceSettings;
use App\Models\Project;
use App\Models\Server;
use App\Models\Service;
use App\Models\Team;
use App\Models\User;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Livewire\Livewire;
uses(RefreshDatabase::class);
beforeEach(function () {
$this->withoutVite();
InstanceSettings::create(['id' => 0]);
$team = Team::factory()->create();
$this->team = $team;
$user = User::factory()->create();
$team->members()->attach($user, ['role' => 'owner']);
$this->actingAs($user);
session(['currentTeam' => $team]);
$server = Server::factory()->create(['team_id' => $team->id]);
$destination = $server->standaloneDockers()->firstOrFail();
$project = Project::factory()->create(['team_id' => $team->id]);
$this->service = Service::factory()->create([
'name' => 'original-service',
'description' => 'Original description',
'docker_compose_raw' => "services:\n app:\n image: nginx:alpine\n",
'docker_compose' => "services:\n app:\n image: nginx:alpine\n",
'connect_to_docker_network' => false,
'is_container_label_escape_enabled' => false,
'server_id' => $server->id,
'destination_id' => $destination->id,
'destination_type' => $destination->getMorphClass(),
'environment_id' => $project->environments()->firstOrFail()->id,
]);
});
test('network instant save does not persist pending compose or service details', function () {
Livewire::test(StackForm::class, ['service' => $this->service])
->set('dockerComposeRaw', 'services: [invalid')
->set('dockerCompose', 'invalid generated compose')
->set('name', 'pending-name')
->set('description', 'Pending description')
->set('connectToDockerNetwork', true)
->call('instantSave')
->assertHasNoErrors()
->assertDispatched('success');
$stored = $this->service->fresh();
expect($stored->connect_to_docker_network)->toBeTruthy()
->and($stored->docker_compose_raw)->toBe("services:\n app:\n image: nginx:alpine\n")
->and($stored->docker_compose)->toBe("services:\n app:\n image: nginx:alpine\n")
->and($stored->name)->toBe('original-service')
->and($stored->description)->toBe('Original description');
});
test('label escape instant save does not persist pending compose', function () {
Livewire::test(EditCompose::class, ['serviceId' => $this->service->id])
->set('dockerComposeRaw', 'services: [invalid')
->set('dockerCompose', 'invalid generated compose')
->set('isContainerLabelEscapeEnabled', true)
->call('instantSave')
->assertHasNoErrors()
->assertDispatched('success');
$stored = $this->service->fresh();
expect($stored->is_container_label_escape_enabled)->toBeTruthy()
->and($stored->docker_compose_raw)->toBe("services:\n app:\n image: nginx:alpine\n")
->and($stored->docker_compose)->toBe("services:\n app:\n image: nginx:alpine\n");
});
test('members cannot instant save service switches', function (string $component, string $property) {
$member = User::factory()->create();
$this->team->members()->attach($member, ['role' => 'member']);
$this->actingAs($member);
$parameters = $component === StackForm::class
? ['service' => $this->service]
: ['serviceId' => $this->service->id];
Livewire::test($component, $parameters)
->set($property, true)
->call('instantSave')
->assertNotDispatched('success');
$stored = $this->service->fresh();
expect($stored->connect_to_docker_network)->toBeFalsy()
->and($stored->is_container_label_escape_enabled)->toBeFalsy();
})->with([
'network attachment' => [StackForm::class, 'connectToDockerNetwork'],
'label escaping' => [EditCompose::class, 'isContainerLabelEscapeEnabled'],
]);
@@ -142,6 +142,27 @@ describe('DockerImage destination team scope', function () {
});
describe('DockerCompose destination + server_id team scope', function () {
test('service creation preserves source Compose comments', function () {
$source = "# Operator note\nservices:\n app:\n image: nginx:alpine # Keep this note\n";
$routeParams = [
'project_uuid' => $this->projectA->uuid,
'environment_uuid' => $this->environmentA->uuid,
];
Livewire::withUrlParams([
'destination' => $this->destinationA->uuid,
'server_id' => $this->serverA->id,
])
->test(DockerCompose::class, $routeParams)
->set('parameters', $routeParams)
->set('query', ['destination' => $this->destinationA->uuid])
->set('dockerComposeRaw', $source)
->call('submit')
->assertHasNoErrors();
expect(Service::where('environment_id', $this->environmentA->id)->latest('id')->firstOrFail()->docker_compose_raw)->toBe($source);
});
test('submit with other team destination throws and creates no service', function () {
$routeParams = [
'project_uuid' => $this->projectA->uuid,
@@ -38,7 +38,7 @@ it('ensures applicationParser updates docker_compose_raw from original compose,
// Check that docker_compose_raw is set from originalCompose, not cleanedCompose
expect($parsersFile)
->toContain('$originalYaml = Yaml::parse($originalCompose);')
->toContain('$resource->docker_compose_raw = Yaml::dump($originalYaml, 10, 2);')
->toContain("\$resource->docker_compose_raw = removeComposeVolumeFieldsPreservingComments(\$originalCompose, \$originalYaml, ['content', 'isDirectory', 'is_directory']);")
->not->toContain('$resource->docker_compose_raw = $cleanedCompose;');
});
@@ -53,7 +53,7 @@ it('ensures serviceParser updates docker_compose_raw from original compose, not
// Check that docker_compose_raw is set from originalCompose within serviceParser
expect($serviceParserContent)
->toContain('$originalYaml = Yaml::parse($originalCompose);')
->toContain('$resource->docker_compose_raw = Yaml::dump($originalYaml, 10, 2);')
->toContain("\$resource->docker_compose_raw = removeComposeVolumeFieldsPreservingComments(\$originalCompose, \$originalYaml, ['content', 'isDirectory', 'is_directory']);")
->not->toContain('$resource->docker_compose_raw = $cleanedCompose;');
});
@@ -98,3 +98,13 @@ it('ensures docker_compose_raw update is wrapped in try-catch for error handling
->toContain('} catch (Exception) {')
->toContain('// If parsing fails, keep the original docker_compose_raw unchanged');
});
it('does not reformat legacy raw Compose when no content is removed', function () {
$source = file_get_contents(__DIR__.'/../../bootstrap/helpers/shared.php');
$parser = substr($source, strpos($source, 'function parseDockerComposeFile('), strpos($source, 'function generate_fluentd_configuration(') - strpos($source, 'function parseDockerComposeFile('));
expect($parser)
->toContain('if ($yaml !== $originalYaml) {')
->toContain("data_forget(\$yaml, 'services.*.volumes.*.content')");
expect(substr_count($parser, "\$resource->docker_compose_raw = removeComposeVolumeFieldsPreservingComments(\$resource->docker_compose_raw, \$yaml, ['content']);"))->toBe(1);
});