diff --git a/bootstrap/helpers/docker.php b/bootstrap/helpers/docker.php index 7b74392cf..ae9470934 100644 --- a/bootstrap/helpers/docker.php +++ b/bootstrap/helpers/docker.php @@ -11,6 +11,16 @@ use Spatie\Url\Url; use Symfony\Component\Yaml\Yaml; use Visus\Cuid2\Cuid2; +/** + * Generate a stable 4-character hash from a service name for uniqueness + * This is used to differentiate services like "api.test" and "api-test" + * which would otherwise collide when normalized. + */ +function serviceNameHash(string $serviceName): string +{ + return substr(md5($serviceName), 0, 4); +} + function getCurrentApplicationContainerStatus(Server $server, int $id, ?int $pullRequestId = null, ?bool $includePullrequests = false): Collection { $containers = collect([]); @@ -468,8 +478,13 @@ function fqdnLabelsForTraefik(string $uuid, Collection $domains, bool $is_force_ $http_label = "http-{$loop}-{$uuid}"; $https_label = "https-{$loop}-{$uuid}"; if ($service_name) { - $http_label = "http-{$loop}-{$uuid}-{$service_name}"; - $https_label = "https-{$loop}-{$uuid}-{$service_name}"; + // Normalize service name for Traefik labels by replacing dots with hyphens + // This prevents label parsing issues with service names like "api.test" + // Add a 4-char hash to ensure uniqueness for services like "api.test" vs "api-test" + $normalized_service_name = str($service_name)->replace('.', '-')->value(); + $hash = serviceNameHash($service_name); + $http_label = "http-{$loop}-{$uuid}-{$normalized_service_name}-{$hash}"; + $https_label = "https-{$loop}-{$uuid}-{$normalized_service_name}-{$hash}"; } if (str($image)->contains('ghost')) { $labels->push("traefik.http.middlewares.redir-ghost-{$uuid}.redirectregex.regex=^{$path}/(.*)"); diff --git a/bootstrap/helpers/parsers.php b/bootstrap/helpers/parsers.php index 99ce9185a..e5ebb7a80 100644 --- a/bootstrap/helpers/parsers.php +++ b/bootstrap/helpers/parsers.php @@ -601,24 +601,27 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int if ($resource->build_pack === 'dockercompose') { // Check if a service with this name actually exists $serviceExists = false; + $actualServiceName = null; foreach ($services as $serviceNameKey => $service) { $transformedServiceName = str($serviceNameKey)->replace('-', '_')->replace('.', '_')->value(); if ($transformedServiceName === $serviceName) { $serviceExists = true; + $actualServiceName = $serviceNameKey; // Store the ORIGINAL service name break; } } // Only add domain if the service exists - if ($serviceExists) { + if ($serviceExists && $actualServiceName) { $domains = collect(json_decode(data_get($resource, 'docker_compose_domains'))) ?? collect([]); - $domainExists = data_get($domains->get($serviceName), 'domain'); + // Use the ORIGINAL service name as the key to avoid collisions + $domainExists = data_get($domains->get($actualServiceName), 'domain'); // Update domain using URL with port if applicable $domainValue = $port ? $urlWithPort : $url; if (is_null($domainExists)) { - $domains->put($serviceName, [ + $domains->put($actualServiceName, [ 'domain' => $domainValue, ]); $resource->docker_compose_domains = $domains->toJson(); @@ -1163,8 +1166,8 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int if ($resource->build_pack !== 'dockercompose') { $domains = collect([]); } - $changedServiceName = str($serviceName)->replace('-', '_')->replace('.', '_')->value(); - $fqdns = data_get($domains, "$changedServiceName.domain"); + // Use the original service name for lookup (no transformation needed) + $fqdns = data_get($domains, "$serviceName.domain"); // Generate SERVICE_FQDN & SERVICE_URL for dockercompose if ($resource->build_pack === 'dockercompose') { foreach ($domains as $forServiceName => $domain) { diff --git a/database/migrations/2025_11_24_115518_migrate_docker_compose_domains_to_original_service_names.php b/database/migrations/2025_11_24_115518_migrate_docker_compose_domains_to_original_service_names.php new file mode 100644 index 000000000..8b5df731f --- /dev/null +++ b/database/migrations/2025_11_24_115518_migrate_docker_compose_domains_to_original_service_names.php @@ -0,0 +1,101 @@ +whereNotNull('docker_compose_domains') + ->chunk(100, function ($applications) { + foreach ($applications as $application) { + try { + $domains = collect(json_decode($application->docker_compose_domains, true)); + + if ($domains->isEmpty()) { + continue; + } + + // Parse the compose file to get original service names + $compose = $application->parseCompose(); + $services = data_get($compose, 'services', []); + + if (empty($services)) { + continue; + } + + // Create a mapping from transformed names to original names + $transformedToOriginal = []; + foreach ($services as $originalServiceName => $service) { + $transformedName = str($originalServiceName)->replace('-', '_')->replace('.', '_')->value(); + $transformedToOriginal[$transformedName] = $originalServiceName; + } + + // Migrate the domains to use original service names + $migratedDomains = collect(); + foreach ($domains as $key => $value) { + // If the key is a transformed name, use the original name + if (isset($transformedToOriginal[$key])) { + $migratedDomains->put($transformedToOriginal[$key], $value); + } else { + // If we can't find a mapping, keep the original key + // (it might already be using the original service name) + $migratedDomains->put($key, $value); + } + } + + // Update the application with migrated domains + $application->docker_compose_domains = $migratedDomains->toJson(); + $application->save(); + + } catch (\Exception $e) { + // Log the error but continue with other applications + logger()->error('Failed to migrate docker_compose_domains for application '.$application->id.': '.$e->getMessage()); + } + } + }); + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + // Reverse the migration by transforming service names back + Application::where('build_pack', 'dockercompose') + ->whereNotNull('docker_compose_domains') + ->chunk(100, function ($applications) { + foreach ($applications as $application) { + try { + $domains = collect(json_decode($application->docker_compose_domains, true)); + + if ($domains->isEmpty()) { + continue; + } + + // Transform all keys back to underscore format + $transformedDomains = collect(); + foreach ($domains as $key => $value) { + $transformedKey = str($key)->replace('-', '_')->replace('.', '_')->value(); + $transformedDomains->put($transformedKey, $value); + } + + $application->docker_compose_domains = $transformedDomains->toJson(); + $application->save(); + + } catch (\Exception $e) { + logger()->error('Failed to reverse migrate docker_compose_domains for application '.$application->id.': '.$e->getMessage()); + } + } + }); + } +}; diff --git a/tests/Unit/TraefikServiceNameNormalizationTest.php b/tests/Unit/TraefikServiceNameNormalizationTest.php new file mode 100644 index 000000000..4022fccc8 --- /dev/null +++ b/tests/Unit/TraefikServiceNameNormalizationTest.php @@ -0,0 +1,208 @@ +toContain('// Normalize service name for Traefik labels by replacing dots with hyphens') + ->toContain('$normalized_service_name = str($service_name)->replace(\'.\', \'-\')->value();'); +}); + +it('uses normalized service name in http label construction', function () { + // Read the fqdnLabelsForTraefik function from docker.php + $dockerFile = file_get_contents(__DIR__.'/../../bootstrap/helpers/docker.php'); + + // Check that normalized service name with hash is used in label construction + expect($dockerFile) + ->toContain('$http_label = "http-{$loop}-{$uuid}-{$normalized_service_name}-{$hash}";') + ->toContain('$https_label = "https-{$loop}-{$uuid}-{$normalized_service_name}-{$hash}";'); +}); + +it('generates valid traefik labels for service names with dots', function () { + $uuid = 'test-uuid-123'; + $domains = collect(['http://example.com']); + $serviceName = 'api.test'; + + // Call the function with a service name containing a dot + $labels = fqdnLabelsForTraefik( + uuid: $uuid, + domains: $domains, + service_name: $serviceName + ); + + // Convert collection to array for easier testing + $labelsArray = $labels->toArray(); + + // Check that labels contain normalized service name (api-test) not original (api.test) + $hasNormalizedLabel = collect($labelsArray)->contains(function ($label) use ($uuid) { + return str_contains($label, "http-0-{$uuid}-api-test"); + }); + + expect($hasNormalizedLabel)->toBeTrue( + 'Expected Traefik labels to contain normalized service name "api-test" instead of "api.test"' + ); + + // Verify no labels contain the original dotted service name in the label identifier + $hasInvalidLabel = collect($labelsArray)->contains(function ($label) use ($uuid) { + // Check if label identifier (before the = sign) contains the problematic pattern + if (str_contains($label, '=')) { + [$labelName, $labelValue] = explode('=', $label, 2); + + return str_contains($labelName, "{$uuid}-api.test"); + } + + return false; + }); + + expect($hasInvalidLabel)->toBeFalse( + 'Traefik labels should not contain service name with dots in label identifiers' + ); +}); + +it('generates valid traefik labels for service names without dots', function () { + $uuid = 'test-uuid-456'; + $domains = collect(['http://example.com']); + $serviceName = 'api-backend'; + + // Call the function with a service name without dots (should work as before) + $labels = fqdnLabelsForTraefik( + uuid: $uuid, + domains: $domains, + service_name: $serviceName + ); + + // Convert collection to array for easier testing + $labelsArray = $labels->toArray(); + + // Check that labels contain the service name unchanged + $hasLabel = collect($labelsArray)->contains(function ($label) use ($uuid) { + return str_contains($label, "http-0-{$uuid}-api-backend"); + }); + + expect($hasLabel)->toBeTrue( + 'Expected Traefik labels to contain service name "api-backend" for services without dots' + ); +}); + +it('handles multiple dots in service names', function () { + $uuid = 'test-uuid-789'; + $domains = collect(['http://example.com']); + $serviceName = 'api.v1.test'; + + // Call the function with a service name containing multiple dots + $labels = fqdnLabelsForTraefik( + uuid: $uuid, + domains: $domains, + service_name: $serviceName + ); + + // Convert collection to array for easier testing + $labelsArray = $labels->toArray(); + + // Check that labels contain fully normalized service name (all dots replaced) + $hasNormalizedLabel = collect($labelsArray)->contains(function ($label) use ($uuid) { + return str_contains($label, "http-0-{$uuid}-api-v1-test"); + }); + + expect($hasNormalizedLabel)->toBeTrue( + 'Expected Traefik labels to normalize all dots in service name "api.v1.test" to "api-v1-test"' + ); +}); + +it('generates unique hashes for different service names', function () { + // Test that the serviceNameHash function exists and generates consistent hashes + $hash1 = serviceNameHash('api.test'); + $hash2 = serviceNameHash('api-test'); + $hash3 = serviceNameHash('api.test'); // Same as hash1 + + // Hashes should be exactly 4 characters + expect(strlen($hash1))->toBe(4); + expect(strlen($hash2))->toBe(4); + + // Same input should generate same hash (stable) + expect($hash1)->toBe($hash3); + + // Different inputs should generate different hashes (unique) + expect($hash1)->not->toBe($hash2); +}); + +it('includes hash in traefik labels to prevent collisions', function () { + $uuid = 'test-uuid-collision'; + $domains = collect(['http://example.com']); + + // Test both "api.test" and "api-test" which would otherwise collide + $serviceName1 = 'api.test'; + $serviceName2 = 'api-test'; + + $labels1 = fqdnLabelsForTraefik( + uuid: $uuid, + domains: $domains, + service_name: $serviceName1 + ); + + $labels2 = fqdnLabelsForTraefik( + uuid: $uuid, + domains: $domains, + service_name: $serviceName2 + ); + + // Get the hashes for both service names + $hash1 = serviceNameHash($serviceName1); + $hash2 = serviceNameHash($serviceName2); + + // Check that labels include the hash + $labels1Array = $labels1->toArray(); + $labels2Array = $labels2->toArray(); + + $hasHash1 = collect($labels1Array)->contains(function ($label) use ($uuid, $hash1) { + return str_contains($label, "http-0-{$uuid}-api-test-{$hash1}"); + }); + + $hasHash2 = collect($labels2Array)->contains(function ($label) use ($uuid, $hash2) { + return str_contains($label, "http-0-{$uuid}-api-test-{$hash2}"); + }); + + expect($hasHash1)->toBeTrue( + 'Expected labels for "api.test" to include hash suffix' + ); + + expect($hasHash2)->toBeTrue( + 'Expected labels for "api-test" to include hash suffix' + ); + + // Verify that the labels are different (no collision) + $router1 = collect($labels1Array)->first(function ($label) { + return str_contains($label, 'traefik.http.routers.'); + }); + + $router2 = collect($labels2Array)->first(function ($label) { + return str_contains($label, 'traefik.http.routers.'); + }); + + expect($router1)->not->toBe($router2, + 'Traefik router labels should be unique for "api.test" and "api-test"' + ); +}); + +it('stores original service names in docker_compose_domains', function () { + // Test that the parsers.php file stores original service names + $parsersFile = file_get_contents(__DIR__.'/../../bootstrap/helpers/parsers.php'); + + // Check that we capture the original service name + expect($parsersFile) + ->toContain('$actualServiceName = $serviceNameKey; // Store the ORIGINAL service name') + ->toContain('// Use the ORIGINAL service name as the key to avoid collisions') + ->toContain('$domains->put($actualServiceName,'); +});