From 455b22f26c334ceaad6efb1e7c23766200c92894 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Thu, 5 Nov 2026 09:49:35 +0100 Subject: [PATCH] fix(deploy): explain SSH exit 255 and handle non-map compose services - Add a hint to failed command messages when the exit code is 255, which explains that the SSH connection to the server closed before the command finished. - Return no services from applicationParser and serviceParser when the compose `services` key is empty or is not a map, so parsing does not fail. - Move Server::getIp() out of the sshEndpointInUse() docblock, where it was misplaced and broke the class. - Add tests for the SSH hint, for services keys that are not maps, and for saved compose variable values that must survive a redeploy (#6995). --- app/Models/Server.php | 8 +- app/Traits/ExecuteRemoteCommand.php | 14 +- bootstrap/helpers/parsers.php | 10 +- .../ApplicationDeploymentComposeParseTest.php | 2 + ...omposeVariableEditsSurviveRedeployTest.php | 120 ++++++++++++++++++ .../DeploymentSensitiveCommandFailureTest.php | 27 ++++ .../ServiceParserServicesSectionTest.php | 34 +++++ 7 files changed, 208 insertions(+), 7 deletions(-) create mode 100644 tests/Feature/ComposeVariableEditsSurviveRedeployTest.php create mode 100644 tests/Feature/ServiceParserServicesSectionTest.php diff --git a/app/Models/Server.php b/app/Models/Server.php index 31c851af99..75f2e21fe1 100644 --- a/app/Models/Server.php +++ b/app/Models/Server.php @@ -2055,10 +2055,6 @@ $siteAddress { return normalizeIpAddress(preg_replace('/[^0-9a-zA-Z.:%-]/', '', (string) $value)); } - public function getIp(): Attribute - { - return Attribute::make( - get: function () { /** * Whether a server of any team already uses this SSH endpoint. The port is part of the endpoint, * so servers behind one NAT address with different forwarded SSH ports are different servers. @@ -2072,6 +2068,10 @@ $siteAddress { ->exists(); } + public function getIp(): Attribute + { + return Attribute::make( + get: function () { if (isDev()) { return '127.0.0.1'; } diff --git a/app/Traits/ExecuteRemoteCommand.php b/app/Traits/ExecuteRemoteCommand.php index 928d5266b2..ff601972f1 100644 --- a/app/Traits/ExecuteRemoteCommand.php +++ b/app/Traits/ExecuteRemoteCommand.php @@ -254,7 +254,19 @@ trait ExecuteRemoteCommand $error = $this->redact_sensitive_info($error); - return "Command execution failed (exit code {$exitCode}): {$commandText}\nError: {$error}".$this->dockerNetworkGatewayHint($error); + return "Command execution failed (exit code {$exitCode}): {$commandText}\nError: {$error}".$this->dockerNetworkGatewayHint($error).$this->sshConnectionLostHint($exitCode); + } + + /** + * SSH exits with 255 when the connection closes, so the output of the remote command stops without an error. + */ + private function sshConnectionLostHint(int $exitCode): string + { + if ($exitCode !== 255) { + return ''; + } + + return "\nExit code 255 usually means that the SSH connection to the server closed before the command finished. Check the network between Coolify and the server, and the sshd logs and memory usage of the server, then deploy again."; } /** diff --git a/bootstrap/helpers/parsers.php b/bootstrap/helpers/parsers.php index 418756d927..f442fb764a 100644 --- a/bootstrap/helpers/parsers.php +++ b/bootstrap/helpers/parsers.php @@ -1161,7 +1161,10 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int } catch (Exception) { return collect([]); } - $services = data_get($yaml, 'services', collect([])); + $services = data_get($yaml, 'services', []); + if (! is_array($services)) { + return collect([]); + } $topLevel = collect([ 'volumes' => collect(data_get($yaml, 'volumes', [])), 'networks' => collect(data_get($yaml, 'networks', [])), @@ -2431,7 +2434,10 @@ function serviceParser(Service $resource): Collection } catch (Exception) { return collect([]); } - $services = data_get($yaml, 'services', collect([])); + $services = data_get($yaml, 'services', []); + if (! is_array($services)) { + return collect([]); + } // Clean up corrupted environment variables from previous parser bugs // (keys starting with $ or ending with } should not exist as env var names) diff --git a/tests/Feature/ApplicationDeploymentComposeParseTest.php b/tests/Feature/ApplicationDeploymentComposeParseTest.php index d6a7006be5..6671343bd9 100644 --- a/tests/Feature/ApplicationDeploymentComposeParseTest.php +++ b/tests/Feature/ApplicationDeploymentComposeParseTest.php @@ -89,6 +89,8 @@ it('stops the deployment when the Compose file gives no services, instead of wri ->implode(''), 'invalid YAML' => "services:\n web:\n image: [nginx\n", 'an empty services section' => "services: {}\n", + 'a services key with no value' => "services:\n", + 'a services key that is not a map' => "services: web\n", ]); it('deploys a Compose file that reuses YAML anchors more than 128 times', function () { diff --git a/tests/Feature/ComposeVariableEditsSurviveRedeployTest.php b/tests/Feature/ComposeVariableEditsSurviveRedeployTest.php new file mode 100644 index 0000000000..15115667f5 --- /dev/null +++ b/tests/Feature/ComposeVariableEditsSurviveRedeployTest.php @@ -0,0 +1,120 @@ +create(); + $server = Server::factory()->create(['team_id' => $team->id]); + + return [ + 'team' => $team, + 'server' => $server, + 'destination' => StandaloneDocker::query()->where('server_id', $server->id)->firstOrFail(), + 'environment' => Environment::factory()->create(['project_id' => Project::factory()->create(['team_id' => $team->id])->id]), + ]; +} + +function saveComposeVariable(Application|Service $resource, string $key, string $value): void +{ + $resource->environment_variables()->where('key', $key)->firstOrFail()->update(['value' => $value]); +} + +test('a service keeps saved variable values and uses the edited compose file when it is parsed again', function () { + $context = composeVariableEditsContext(); + $service = Service::factory()->create([ + 'server_id' => $context['server']->id, + 'environment_id' => $context['environment']->id, + 'destination_id' => $context['destination']->id, + 'destination_type' => $context['destination']->getMorphClass(), + 'docker_compose_raw' => composeWithEditableVariables('v1'), + ]); + + $service->parse(); + saveComposeVariable($service, 'SIMPLE', 'user-simple'); + saveComposeVariable($service, 'WITH_DEFAULT', 'user-default'); + saveComposeVariable($service, 'REQUIRED', 'user-required'); + + $service->refresh(); + $service->docker_compose_raw = composeWithEditableVariables('v2'); + $service->save(); + $service->parse(); + + $environment = data_get(Yaml::parse($service->fresh()->docker_compose), 'services.app.environment'); + + expect($service->environment_variables()->pluck('value', 'key')->only(['SIMPLE', 'WITH_DEFAULT', 'REQUIRED'])->all()) + ->toEqual(['SIMPLE' => 'user-simple', 'WITH_DEFAULT' => 'user-default', 'REQUIRED' => 'user-required']) + // The compose file keeps the references, so Docker Compose reads the saved values from .env. + ->and($environment)->toMatchArray([ + 'SIMPLE' => '${SIMPLE}', + 'WITH_DEFAULT' => '${WITH_DEFAULT:-from-compose}', + 'REQUIRED' => '${REQUIRED:?}', + 'HARDCODED' => 'v2', + ]); +}); + +test('a compose application keeps saved variable values and writes no stale values when it is parsed again', function () { + $context = composeVariableEditsContext(); + $application = Application::factory()->create([ + 'environment_id' => $context['environment']->id, + 'destination_id' => $context['destination']->id, + 'destination_type' => $context['destination']->getMorphClass(), + 'build_pack' => 'dockercompose', + 'docker_compose_raw' => composeWithEditableVariables('v1'), + ]); + + applicationParser($application); + saveComposeVariable($application, 'SIMPLE', 'first'); + saveComposeVariable($application, 'WITH_DEFAULT', 'first-default'); + saveComposeVariable($application, 'REQUIRED', 'first-required'); + applicationParser($application->fresh()); + + saveComposeVariable($application, 'SIMPLE', 'second'); + saveComposeVariable($application, 'WITH_DEFAULT', 'second-default'); + saveComposeVariable($application, 'REQUIRED', 'second-required'); + $application->refresh(); + $application->docker_compose_raw = composeWithEditableVariables('v2'); + $application->save(); + $parsedCompose = applicationParser($application->fresh()); + + expect($application->environment_variables()->where('is_preview', false)->pluck('value', 'key')->only(['SIMPLE', 'WITH_DEFAULT', 'REQUIRED'])->all()) + ->toEqual(['SIMPLE' => 'second', 'WITH_DEFAULT' => 'second-default', 'REQUIRED' => 'second-required']) + ->and(data_get($parsedCompose, 'services.app.environment'))->toMatchArray([ + 'SIMPLE' => '${SIMPLE}', + 'WITH_DEFAULT' => '${WITH_DEFAULT:-from-compose}', + 'REQUIRED' => '${REQUIRED:?}', + 'HARDCODED' => 'v2', + ]); +}); diff --git a/tests/Feature/DeploymentSensitiveCommandFailureTest.php b/tests/Feature/DeploymentSensitiveCommandFailureTest.php index 66a4b54606..0a3cd97b45 100644 --- a/tests/Feature/DeploymentSensitiveCommandFailureTest.php +++ b/tests/Feature/DeploymentSensitiveCommandFailureTest.php @@ -283,6 +283,33 @@ it('does not add the Docker network hint to other failures', function () { expect($exception->getMessage())->not->toContain('systemctl restart docker'); }); +it('reports the exit code and explains a lost SSH connection when a command ends with exit code 255', function () { + [$job] = makeSensitiveCommandFailureJob(); + + Process::fake(['*' => Process::result(errorOutput: '#26 exporting layers', exitCode: 255)]); + + $exception = captureDeploymentException(fn () => $job->execute_remote_command([ + 'command' => 'docker buildx build .', + ])); + + expect($exception->getCode())->toBe(255) + ->and($exception->getMessage()) + ->toContain('exit code 255') + ->toContain('SSH connection to the server closed'); +}); + +it('does not add the SSH connection hint to failures of the remote command', function () { + [$job] = makeSensitiveCommandFailureJob(); + + Process::fake(['*' => Process::result(errorOutput: 'build failed', exitCode: 1)]); + + $exception = captureDeploymentException(fn () => $job->execute_remote_command([ + 'command' => 'docker buildx build .', + ])); + + expect($exception->getMessage())->not->toContain('SSH connection'); +}); + it('redacts locked values from the error output of failed commands', function () { [$job] = makeSensitiveCommandFailureJob([ ['key' => 'LOCKED_SECRET', 'value' => SENSITIVE_FAILURE_SECRET, 'is_shown_once' => true], diff --git a/tests/Feature/ServiceParserServicesSectionTest.php b/tests/Feature/ServiceParserServicesSectionTest.php new file mode 100644 index 0000000000..960542cd29 --- /dev/null +++ b/tests/Feature/ServiceParserServicesSectionTest.php @@ -0,0 +1,34 @@ +create(); + $server = Server::factory()->create(['team_id' => $team->id]); + $destination = StandaloneDocker::where('server_id', $server->id)->firstOrFail(); + $project = Project::factory()->create(['team_id' => $team->id]); + $environment = Environment::factory()->create(['project_id' => $project->id]); + + $service = Service::factory()->create([ + 'docker_compose_raw' => $compose, + 'environment_id' => $environment->id, + 'server_id' => $server->id, + 'destination_id' => $destination->id, + 'destination_type' => $destination->getMorphClass(), + ]); + + expect(serviceParser($service))->toBeEmpty(); +})->with([ + 'no value' => "services:\n", + 'a string' => "services: web\n", +]);