diff --git a/app/Actions/Proxy/StartProxy.php b/app/Actions/Proxy/StartProxy.php index c546a4252e..3508a46302 100644 --- a/app/Actions/Proxy/StartProxy.php +++ b/app/Actions/Proxy/StartProxy.php @@ -2,6 +2,7 @@ namespace App\Actions\Proxy; +use App\Enums\ProxyTypes; use App\Events\ProxyStatusChanged; use App\Events\ProxyStatusChangedUI; use App\Models\Server; @@ -71,6 +72,10 @@ class StartProxy " echo 'Successfully stopped and removed existing coolify-proxy.'", 'fi', ]); + if ($proxyType !== ProxyTypes::TRAEFIK->value) { + // The sidecar belongs to the Traefik compose project, so --remove-orphans of another proxy keeps it. + $commands->push('docker rm -f '.TRAEFIK_LOGROTATE_CONTAINER.' 2>/dev/null || true'); + } // Ensure required networks exist BEFORE docker compose up (networks are declared as external) $commands = $commands->merge(ensureProxyNetworksExist($server)); $commands = $commands->merge([ diff --git a/app/Actions/Proxy/StopProxy.php b/app/Actions/Proxy/StopProxy.php index b9029d6555..d3999a6e30 100644 --- a/app/Actions/Proxy/StopProxy.php +++ b/app/Actions/Proxy/StopProxy.php @@ -26,6 +26,8 @@ class StopProxy instant_remote_process(command: [ dockerStopCommand($timeout, $containerName, $server).' 2>/dev/null || true', "docker rm -f $containerName 2>/dev/null || true", + // The Traefik traffic analytics log rotation sidecar must not outlive the proxy. + 'docker rm -f '.TRAEFIK_LOGROTATE_CONTAINER.' 2>/dev/null || true', '# Wait for container to be fully removed', 'for i in {1..10}; do', " if ! docker ps -a --format \"{{.Names}}\" | grep -q \"^$containerName$\"; then", diff --git a/bootstrap/helpers/proxy.php b/bootstrap/helpers/proxy.php index fed5c27544..90344e2466 100644 --- a/bootstrap/helpers/proxy.php +++ b/bootstrap/helpers/proxy.php @@ -8,6 +8,7 @@ use App\Models\Server; use App\Support\ValidationPatterns; use Illuminate\Support\Collection; use Illuminate\Support\Facades\Log; +use Symfony\Component\Yaml\Tag\TaggedValue; use Symfony\Component\Yaml\Yaml; function traefikAccessLogCommands(bool $enabled): array @@ -35,7 +36,7 @@ function traefikAccessLogCommands(bool $enabled): array function applyTrafficAnalyticsToProxyConfiguration(Server $server, string $configuration): string { - $config = Yaml::parse($configuration); + $config = parseProxyComposeYaml($configuration); if (! is_array($config)) { throw new RuntimeException('Proxy configuration must be a YAML mapping.'); @@ -43,7 +44,75 @@ function applyTrafficAnalyticsToProxyConfiguration(Server $server, string $confi $config = applyTrafficAnalyticsToProxyConfigArray($server, $config); - return Yaml::dump($config, 12, 2); + return Yaml::dump($config, 12, 2, Yaml::DUMP_OBJECT_AS_MAP | Yaml::DUMP_EMPTY_ARRAY_AS_SEQUENCE); +} + +/** + * Parses a proxy Compose file so that it can be written back with Yaml::DUMP_OBJECT_AS_MAP and + * Yaml::DUMP_EMPTY_ARRAY_AS_SEQUENCE without changes: Docker Compose merge tags such as `!reset` + * and `!override` stay TaggedValue objects, and an empty map stays a stdClass so that `{}` and `[]` + * keep their type (Compose rejects `!override {}` for a list such as `volumes`). + */ +function parseProxyComposeYaml(string $configuration): mixed +{ + return proxyComposeMapsToArrays(Yaml::parse($configuration, Yaml::PARSE_CUSTOM_TAGS | Yaml::PARSE_OBJECT_FOR_MAP)); +} + +function proxyComposeMapsToArrays(mixed $value): mixed +{ + if ($value instanceof TaggedValue) { + return new TaggedValue($value->getTag(), proxyComposeMapsToArrays($value->getValue())); + } + + if ($value instanceof stdClass) { + $value = get_object_vars($value); + if ($value === []) { + return new stdClass; + } + } + + return is_array($value) ? array_map(proxyComposeMapsToArrays(...), $value) : $value; +} + +/** + * The value of a proxy Compose file without Docker Compose merge tags, for reading only. + */ +function withoutProxyComposeTags(mixed $value): mixed +{ + if ($value instanceof TaggedValue) { + return withoutProxyComposeTags($value->getValue()); + } + + return is_array($value) ? array_map(withoutProxyComposeTags(...), $value) : $value; +} + +/** + * Changes a Compose list that may have a merge tag. The tag is kept when the list is unchanged or + * the tag is `!override`. Other tags such as `!reset` make Compose ignore the value, so a changed + * list is written without its tag. + */ +function updateTaggedProxyComposeList(mixed $value, Closure $update, string $error): mixed +{ + $list = $value instanceof TaggedValue ? $value->getValue() : $value; + if ($list instanceof stdClass && get_object_vars($list) === []) { + $list = []; + } + + if (! is_array($list)) { + throw new RuntimeException($error); + } + + $updated = $update($list); + + if (! $value instanceof TaggedValue) { + return $updated; + } + + if ($updated === $list) { + return $value; + } + + return $value->getTag() === 'override' ? new TaggedValue('override', $updated) : $updated; } /** @@ -150,24 +219,27 @@ function applyTraefikAccessLogCommands(Server $server, array $commands, bool $en return array_values($commands); } +/** + * Container of the Traefik access log rotation sidecar that traffic analytics adds to the proxy. + */ +const TRAEFIK_LOGROTATE_CONTAINER = 'coolify-proxy-logrotate'; + function applyTrafficAnalyticsToProxyConfigArray(Server $server, array $config): array { $enabled = $server->isTrafficAnalyticsEnabled(); if ($server->proxyType() === ProxyTypes::TRAEFIK->value) { - $commands = data_get($config, 'services.traefik.command', []); - - if (! is_array($commands)) { - throw new RuntimeException('Traefik commands must be a YAML list.'); - } - - data_set($config, 'services.traefik.command', applyTraefikAccessLogCommands($server, $commands, $enabled)); + data_set($config, 'services.traefik.command', updateTaggedProxyComposeList( + data_get($config, 'services.traefik.command', []), + fn (array $commands): array => applyTraefikAccessLogCommands($server, $commands, $enabled), + 'Traefik commands must be a YAML list.', + )); unset($config['services']['traefik-logrotate']); if ($enabled && ! $server->isSwarm()) { $proxyPath = devHostDockerPath($server, $server->proxyPath()); $config['services']['traefik-logrotate'] = [ - 'container_name' => 'coolify-proxy-logrotate', + 'container_name' => TRAEFIK_LOGROTATE_CONTAINER, 'image' => 'alpine:3.24', 'restart' => RESTART_MODE, 'network_mode' => 'none', @@ -184,19 +256,19 @@ function applyTrafficAnalyticsToProxyConfigArray(Server $server, array $config): } elseif ($server->proxyType() === ProxyTypes::CADDY->value) { // Caddy writes /traffic/access.log and Sentinel reads /access.log, so both use one path. $trafficVolume = StartSentinel::trafficLogDirectory($server).':/traffic'; - $volumes = data_get($config, 'services.caddy.volumes', []); + data_set($config, 'services.caddy.volumes', updateTaggedProxyComposeList( + data_get($config, 'services.caddy.volumes', []), + function (array $volumes) use ($enabled, $trafficVolume): array { + // Coolify owns /traffic: replace an older mount with a different source path. + $volumes = array_values(array_filter($volumes, fn (mixed $volume): bool => ! isCaddyTrafficVolume($volume))); + if ($enabled) { + $volumes[] = $trafficVolume; + } - if (! is_array($volumes)) { - throw new RuntimeException('Caddy volumes must be a YAML list.'); - } - - // Coolify owns /traffic: replace an older mount with a different source path. - $volumes = array_values(array_filter($volumes, fn (mixed $volume): bool => ! isCaddyTrafficVolume($volume))); - if ($enabled) { - $volumes[] = $trafficVolume; - } - - data_set($config, 'services.caddy.volumes', $volumes); + return $volumes; + }, + 'Caddy volumes must be a YAML list.', + )); } return $config; @@ -397,10 +469,10 @@ function extractCustomProxyCommands(Server $server, string $existing_config): ar } try { - $yaml = Yaml::parse($existing_config); + $yaml = withoutProxyComposeTags(Yaml::parse($existing_config, Yaml::PARSE_CUSTOM_TAGS)); $existing_commands = data_get($yaml, 'services.traefik.command', []); - if (empty($existing_commands)) { + if (empty($existing_commands) || ! is_array($existing_commands)) { return $custom_commands; } @@ -459,7 +531,8 @@ function removeLegacyTraefikDashboardLabels(string $configuration): string ]; try { - $yaml = Yaml::parse($configuration); + // Merge tags such as `!override` are kept in the text; the comparison below ignores them. + $yaml = withoutProxyComposeTags(Yaml::parse($configuration, Yaml::PARSE_CUSTOM_TAGS)); } catch (Throwable) { return $configuration; } @@ -500,7 +573,7 @@ function removeLegacyTraefikDashboardLabels(string $configuration): string // Keep the original when the line edit also changed other parts of the file. try { - return Yaml::parse($fixed) === $expected ? $fixed : $configuration; + return withoutProxyComposeTags(Yaml::parse($fixed, Yaml::PARSE_CUSTOM_TAGS)) === $expected ? $fixed : $configuration; } catch (Throwable) { return $configuration; } diff --git a/tests/Feature/Proxy/ProxyComposeMergeTagsTest.php b/tests/Feature/Proxy/ProxyComposeMergeTagsTest.php new file mode 100644 index 0000000000..d5750c00dd --- /dev/null +++ b/tests/Feature/Proxy/ProxyComposeMergeTagsTest.php @@ -0,0 +1,131 @@ +create(); + $server = Server::factory()->create([ + 'team_id' => $team->id, + 'private_key_id' => PrivateKey::factory()->create(['team_id' => $team->id])->id, + ]); + $server->proxy->set('type', $proxyType->value); + $server->save(); + $server->settings->update(['is_traffic_analytics_enabled' => $trafficAnalytics]); + + return $server->fresh(); +} + +it('enables traffic analytics on a Traefik configuration with Compose merge tags and keeps the tags', function () { + $server = proxyComposeMergeTagsServer(ProxyTypes::TRAEFIK, trafficAnalytics: true); + $configuration = <<<'YAML' +services: + traefik: + image: 'traefik:v3.6' + ports: !override + - '8080:80' + - '8443:443' + expose: !reset [] + environment: !reset {} + dns: !reset null + healthcheck: {} + command: !override + - '--ping=true' + - '--providers.docker=true' +YAML; + + $result = applyTrafficAnalyticsToProxyConfiguration($server, $configuration); + $traefik = Yaml::parse($result, Yaml::PARSE_CUSTOM_TAGS | Yaml::PARSE_OBJECT_FOR_MAP)->services->traefik; + + expect($traefik->ports)->toBeInstanceOf(TaggedValue::class) + ->and($traefik->ports->getTag())->toBe('override') + ->and($traefik->ports->getValue())->toBe(['8080:80', '8443:443']) + ->and($traefik->expose->getTag())->toBe('reset') + ->and($traefik->expose->getValue())->toBe([]) + ->and($traefik->environment->getTag())->toBe('reset') + ->and($traefik->environment->getValue())->toEqual(new stdClass) + ->and($traefik->dns->getTag())->toBe('reset') + ->and($traefik->dns->getValue())->toBeNull() + ->and($traefik->healthcheck)->toEqual(new stdClass) + ->and($traefik->command->getTag())->toBe('override') + ->and($traefik->command->getValue())->toBe(['--ping=true', '--providers.docker=true', ...traefikAccessLogCommands(true)]); +}); + +it('keeps a Traefik command list with a reset tag when traffic analytics does not change it', function () { + $server = proxyComposeMergeTagsServer(ProxyTypes::TRAEFIK, trafficAnalytics: false); + $configuration = "services:\n traefik:\n image: 'traefik:v3.6'\n command: !reset []\n"; + + $traefik = Yaml::parse(applyTrafficAnalyticsToProxyConfiguration($server, $configuration), Yaml::PARSE_CUSTOM_TAGS)['services']['traefik']; + + expect($traefik['command'])->toEqual(new TaggedValue('reset', [])); +}); + +it('adds the access log flags without the reset tag, which would make Compose drop them', function () { + $server = proxyComposeMergeTagsServer(ProxyTypes::TRAEFIK, trafficAnalytics: true); + $configuration = "services:\n traefik:\n image: 'traefik:v3.6'\n command: !reset []\n"; + + $traefik = Yaml::parse(applyTrafficAnalyticsToProxyConfiguration($server, $configuration), Yaml::PARSE_CUSTOM_TAGS)['services']['traefik']; + + expect($traefik['command'])->toBe(traefikAccessLogCommands(true)); +}); + +it('adds the traffic volume to a Caddy volume list with an override tag', function () { + $server = proxyComposeMergeTagsServer(ProxyTypes::CADDY, trafficAnalytics: true); + $configuration = "services:\n caddy:\n image: 'lucaslorentz/caddy-docker-proxy:2.8-alpine'\n ports: !override\n - '80:80'\n volumes: !override\n - '/var/run/docker.sock:/var/run/docker.sock:ro'\n"; + + $caddy = Yaml::parse(applyTrafficAnalyticsToProxyConfiguration($server, $configuration), Yaml::PARSE_CUSTOM_TAGS)['services']['caddy']; + + expect($caddy['ports'])->toEqual(new TaggedValue('override', ['80:80'])) + ->and($caddy['volumes']->getTag())->toBe('override') + ->and($caddy['volumes']->getValue())->toContain('/var/run/docker.sock:/var/run/docker.sock:ro') + ->toHaveCount(2); +}); + +it('removes the legacy dashboard labels from a configuration with Compose merge tags', function () { + $configuration = <<<'YAML' +# my custom comment +services: + traefik: + container_name: coolify-proxy + image: 'traefik:v3.6' + ports: !override + - '80:80' + - '443:443' + expose: !reset [] + labels: + - traefik.enable=true + - traefik.http.routers.traefik.entrypoints=http + - traefik.http.routers.traefik.service=api@internal + - traefik.http.services.traefik.loadbalancer.server.port=8080 + - coolify.managed=true +YAML; + + $fixed = removeLegacyTraefikDashboardLabels($configuration); + $traefik = Yaml::parse($fixed, Yaml::PARSE_CUSTOM_TAGS)['services']['traefik']; + + expect($traefik['labels'])->toBe(['traefik.enable=false', 'coolify.managed=true']) + ->and($traefik['ports'])->toEqual(new TaggedValue('override', ['80:80', '443:443'])) + ->and($fixed)->toContain('# my custom comment') + ->toContain('expose: !reset []'); +}); + +it('keeps custom Traefik commands of a configuration with Compose merge tags', function () { + $server = proxyComposeMergeTagsServer(ProxyTypes::TRAEFIK, trafficAnalytics: false); + $configuration = "services:\n traefik:\n ports: !override\n - '80:80'\n command: !override\n - '--ping=true'\n - '--entrypoints.http.forwardedHeaders.trustedIPs=10.0.0.0/8'\n"; + + expect(extractCustomProxyCommands($server, $configuration))->toBe(['--entrypoints.http.forwardedHeaders.trustedIPs=10.0.0.0/8']); +}); diff --git a/tests/Feature/Proxy/ProxyLogrotateSidecarCleanupTest.php b/tests/Feature/Proxy/ProxyLogrotateSidecarCleanupTest.php new file mode 100644 index 0000000000..d1639e1c30 --- /dev/null +++ b/tests/Feature/Proxy/ProxyLogrotateSidecarCleanupTest.php @@ -0,0 +1,80 @@ + 0]); + config(['constants.ssh.mux_enabled' => false]); + Process::fake(); + + $team = Team::factory()->create(); + $this->server = Server::factory()->create([ + 'team_id' => $team->id, + 'user' => 'cooluser', + 'private_key_id' => PrivateKey::factory()->create(['team_id' => $team->id])->id, + ]); + $this->server->settings->update(['is_reachable' => true, 'is_usable' => true, 'force_disabled' => false]); + $this->server->proxy->type = ProxyTypes::TRAEFIK->value; + $this->server->proxy->status = 'running'; + $this->server->save(); + $this->server->refresh(); +}); + +/** + * @return array + */ +function logrotateCleanupScriptLines(): array +{ + $lines = []; + Process::assertRan(function ($process) use (&$lines) { + array_push($lines, ...explode("\n", $process->command)); + + return true; + }); + + return $lines; +} + +it('removes the logrotate sidecar when the proxy is stopped', function (string $user, string $expected) { + $this->server->update(['user' => $user]); + + StopProxy::run($this->server); + + expect(logrotateCleanupScriptLines())->toContain($expected); +})->with([ + 'non-root' => ['cooluser', 'sudo docker rm -f coolify-proxy-logrotate 2>/dev/null || sudo true'], + 'root' => ['root', 'docker rm -f coolify-proxy-logrotate 2>/dev/null || true'], +]); + +it('removes the logrotate sidecar before Caddy starts after a switch from Traefik', function () { + $this->server->proxy->status = 'exited'; + $this->server->save(); + + $this->server->changeProxy(ProxyTypes::CADDY->value, async: false); + + $lines = logrotateCleanupScriptLines(); + $removal = array_search('sudo docker rm -f coolify-proxy-logrotate 2>/dev/null || sudo true', $lines, true); + $start = array_search('sudo docker compose -f /data/coolify/proxy/caddy/docker-compose.yml up -d --wait --remove-orphans', $lines, true); + + expect($removal)->toBeInt() + ->and($start)->toBeInt() + ->and($removal)->toBeLessThan($start); +});