diff --git a/bootstrap/helpers/parsers.php b/bootstrap/helpers/parsers.php index bd3a6e9e1..f6cb0b64b 100644 --- a/bootstrap/helpers/parsers.php +++ b/bootstrap/helpers/parsers.php @@ -453,13 +453,9 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int // for example SERVICE_FQDN_APP_3000 (without a value) if ($key->startsWith('SERVICE_FQDN_')) { // SERVICE_FQDN_APP or SERVICE_FQDN_APP_3000 - if (substr_count(str($key)->value(), '_') === 3) { - $fqdnFor = $key->after('SERVICE_FQDN_')->beforeLast('_')->lower()->value(); - $port = $key->afterLast('_')->value(); - } else { - $fqdnFor = $key->after('SERVICE_FQDN_')->lower()->value(); - $port = null; - } + $parsed = parseServiceEnvironmentVariable($key->value()); + $fqdnFor = $parsed['service_name']; + $port = $parsed['port']; $fqdn = $resource->fqdn; if (blank($resource->fqdn)) { $fqdn = generateFqdn(server: $server, random: "$uuid", parserVersion: $resource->compose_parsing_version); @@ -482,7 +478,7 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int $resource->save(); } - if (substr_count(str($key)->value(), '_') === 2) { + if (! $parsed['has_port']) { $resource->environment_variables()->updateOrCreate([ 'key' => $key->value(), 'resourceable_type' => get_class($resource), @@ -492,7 +488,7 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int 'is_preview' => false, ]); } - if (substr_count(str($key)->value(), '_') === 3) { + if ($parsed['has_port']) { $newKey = str($key)->beforeLast('_'); $resource->environment_variables()->updateOrCreate([ @@ -565,13 +561,9 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int } elseif ($command->value() === 'URL') { // SERVICE_URL_APP or SERVICE_URL_APP_3000 // Detect if there's a port suffix - if (substr_count(str($key)->value(), '_') === 3) { - $urlFor = $key->after('SERVICE_URL_')->beforeLast('_')->lower()->value(); - $port = $key->afterLast('_')->value(); - } else { - $urlFor = $key->after('SERVICE_URL_')->lower()->value(); - $port = null; - } + $parsed = parseServiceEnvironmentVariable($key->value()); + $urlFor = $parsed['service_name']; + $port = $parsed['port']; $originalUrlFor = str($urlFor)->replace('_', '-'); if (str($urlFor)->contains('-')) { $urlFor = str($urlFor)->replace('-', '_')->replace('.', '_'); @@ -1538,27 +1530,16 @@ function serviceParser(Service $resource): Collection // Get magic environments where we need to preset the FQDN / URL if ($key->startsWith('SERVICE_FQDN_') || $key->startsWith('SERVICE_URL_')) { // SERVICE_FQDN_APP or SERVICE_FQDN_APP_3000 - if (substr_count(str($key)->value(), '_') === 3) { - if ($key->startsWith('SERVICE_FQDN_')) { - $urlFor = null; - $fqdnFor = $key->after('SERVICE_FQDN_')->beforeLast('_')->lower()->value(); - } - if ($key->startsWith('SERVICE_URL_')) { - $fqdnFor = null; - $urlFor = $key->after('SERVICE_URL_')->beforeLast('_')->lower()->value(); - } - $port = $key->afterLast('_')->value(); - } else { - if ($key->startsWith('SERVICE_FQDN_')) { - $urlFor = null; - $fqdnFor = $key->after('SERVICE_FQDN_')->lower()->value(); - } - if ($key->startsWith('SERVICE_URL_')) { - $fqdnFor = null; - $urlFor = $key->after('SERVICE_URL_')->lower()->value(); - } - $port = null; + $parsed = parseServiceEnvironmentVariable($key->value()); + if ($key->startsWith('SERVICE_FQDN_')) { + $urlFor = null; + $fqdnFor = $parsed['service_name']; } + if ($key->startsWith('SERVICE_URL_')) { + $fqdnFor = null; + $urlFor = $parsed['service_name']; + } + $port = $parsed['port']; if (blank($savedService->fqdn)) { if ($fqdnFor) { $fqdn = generateFqdn(server: $server, random: "$fqdnFor-$uuid", parserVersion: $resource->compose_parsing_version); @@ -1603,7 +1584,7 @@ function serviceParser(Service $resource): Collection } $savedService->save(); } - if (substr_count(str($key)->value(), '_') === 2) { + if (! $parsed['has_port']) { $resource->environment_variables()->updateOrCreate([ 'key' => $key->value(), 'resourceable_type' => get_class($resource), @@ -1621,7 +1602,7 @@ function serviceParser(Service $resource): Collection 'is_preview' => false, ]); } - if (substr_count(str($key)->value(), '_') === 3) { + if ($parsed['has_port']) { // For port-specific variables (e.g., SERVICE_FQDN_UMAMI_3000), // keep the port suffix in the key and use the URL with port $resource->environment_variables()->updateOrCreate([ diff --git a/bootstrap/helpers/services.php b/bootstrap/helpers/services.php index a124272a2..a6d427a6b 100644 --- a/bootstrap/helpers/services.php +++ b/bootstrap/helpers/services.php @@ -184,3 +184,53 @@ function serviceKeys() { return get_service_templates()->keys(); } + +/** + * Parse a SERVICE_URL_* or SERVICE_FQDN_* variable to extract the service name and port. + * + * This function detects if a service environment variable has a port suffix by checking + * if the last segment after the underscore is numeric. + * + * Examples: + * - SERVICE_URL_APP_3000 → ['service_name' => 'app', 'port' => '3000', 'has_port' => true] + * - SERVICE_URL_MY_API_8080 → ['service_name' => 'my_api', 'port' => '8080', 'has_port' => true] + * - SERVICE_URL_MY_APP → ['service_name' => 'my_app', 'port' => null, 'has_port' => false] + * - SERVICE_FQDN_REDIS_CACHE_6379 → ['service_name' => 'redis_cache', 'port' => '6379', 'has_port' => true] + * + * @param string $key The environment variable key (e.g., SERVICE_URL_APP_3000) + * @return array{service_name: string, port: string|null, has_port: bool} Parsed service information + */ +function parseServiceEnvironmentVariable(string $key): array +{ + $strKey = str($key); + $lastSegment = $strKey->afterLast('_')->value(); + $hasPort = is_numeric($lastSegment) && ctype_digit($lastSegment); + + if ($hasPort) { + // Port-specific variable (e.g., SERVICE_URL_APP_3000) + if ($strKey->startsWith('SERVICE_URL_')) { + $serviceName = $strKey->after('SERVICE_URL_')->beforeLast('_')->lower()->value(); + } elseif ($strKey->startsWith('SERVICE_FQDN_')) { + $serviceName = $strKey->after('SERVICE_FQDN_')->beforeLast('_')->lower()->value(); + } else { + $serviceName = ''; + } + $port = $lastSegment; + } else { + // Base variable without port (e.g., SERVICE_URL_APP) + if ($strKey->startsWith('SERVICE_URL_')) { + $serviceName = $strKey->after('SERVICE_URL_')->lower()->value(); + } elseif ($strKey->startsWith('SERVICE_FQDN_')) { + $serviceName = $strKey->after('SERVICE_FQDN_')->lower()->value(); + } else { + $serviceName = ''; + } + $port = null; + } + + return [ + 'service_name' => $serviceName, + 'port' => $port, + 'has_port' => $hasPort, + ]; +} diff --git a/tests/Unit/ServiceParserPortDetectionLogicTest.php b/tests/Unit/ServiceParserPortDetectionLogicTest.php new file mode 100644 index 000000000..d677039af --- /dev/null +++ b/tests/Unit/ServiceParserPortDetectionLogicTest.php @@ -0,0 +1,158 @@ +toBe($expectedService, "Service name mismatch for $varName"); + expect($parsed['port'])->toBe($expectedPort, "Port mismatch for $varName"); + expect($parsed['has_port'])->toBe($isPortSpecific, "Port detection mismatch for $varName"); + } +}); + +it('shows current underscore-counting logic fails for some patterns', function () { + // This demonstrates the CURRENT BROKEN logic: substr_count === 3 + + $testCases = [ + // [variable_name, underscore_count, should_detect_port] + + // Works correctly with current logic (3 underscores total) + ['SERVICE_URL_APP_3000', 3, true], // 3 underscores ✓ + ['SERVICE_URL_API_8080', 3, true], // 3 underscores ✓ + + // FAILS: 4 underscores (two-word service + port) - current logic says no port + ['SERVICE_URL_MY_API_8080', 4, true], // 4 underscores ✗ + ['SERVICE_URL_WEB_APP_3000', 4, true], // 4 underscores ✗ + + // FAILS: 5+ underscores (three-word service + port) - current logic says no port + ['SERVICE_URL_REDIS_CACHE_SERVER_6379', 5, true], // 5 underscores ✗ + ['SERVICE_URL_MY_LONG_APP_8080', 5, true], // 5 underscores ✗ + + // Works correctly (no port, not 3 underscores) + ['SERVICE_URL_MY_APP', 3, false], // 3 underscores but non-numeric ✓ + ['SERVICE_URL_APP', 2, false], // 2 underscores ✓ + ]; + + foreach ($testCases as [$varName, $expectedUnderscoreCount, $shouldDetectPort]) { + $key = str($varName); + + // Current logic: count underscores + $underscoreCount = substr_count($key->value(), '_'); + expect($underscoreCount)->toBe($expectedUnderscoreCount, "Underscore count for $varName"); + + $currentLogicDetectsPort = ($underscoreCount === 3); + + // Correct logic: check if numeric + $lastSegment = $key->afterLast('_')->value(); + $correctLogicDetectsPort = is_numeric($lastSegment); + + expect($correctLogicDetectsPort)->toBe($shouldDetectPort, "Correct logic should detect port for $varName"); + + // Show the discrepancy where current logic fails + if ($currentLogicDetectsPort !== $correctLogicDetectsPort) { + // This is a known bug - current logic is wrong + expect($currentLogicDetectsPort)->not->toBe($correctLogicDetectsPort, "Bug confirmed: current logic wrong for $varName"); + } + } +}); + +it('generates correct URL with port suffix', function () { + // Test that URLs are correctly formatted with port appended + + $testCases = [ + ['http://umami-abc123.domain.com', '3000', 'http://umami-abc123.domain.com:3000'], + ['http://api-xyz789.domain.com', '8080', 'http://api-xyz789.domain.com:8080'], + ['https://db-server.example.com', '5432', 'https://db-server.example.com:5432'], + ['http://app.local', '80', 'http://app.local:80'], + ]; + + foreach ($testCases as [$baseUrl, $port, $expectedUrlWithPort]) { + $urlWithPort = "$baseUrl:$port"; + expect($urlWithPort)->toBe($expectedUrlWithPort); + } +}); + +it('generates correct FQDN with port suffix', function () { + // Test that FQDNs are correctly formatted with port appended + + $testCases = [ + ['umami-abc123.domain.com', '3000', 'umami-abc123.domain.com:3000'], + ['postgres-xyz789.domain.com', '5432', 'postgres-xyz789.domain.com:5432'], + ['redis-cache.example.com', '6379', 'redis-cache.example.com:6379'], + ]; + + foreach ($testCases as [$baseFqdn, $port, $expectedFqdnWithPort]) { + $fqdnWithPort = "$baseFqdn:$port"; + expect($fqdnWithPort)->toBe($expectedFqdnWithPort); + } +}); + +it('correctly identifies service name with various patterns', function () { + // Test service name extraction with different patterns + + $testCases = [ + // After parsing, service names should preserve underscores + ['SERVICE_URL_MY_API_8080', 'my_api'], + ['SERVICE_URL_REDIS_CACHE_6379', 'redis_cache'], + ['SERVICE_URL_NEW_API_3000', 'new_api'], + ['SERVICE_FQDN_DB_SERVER_5432', 'db_server'], + + // Single-word services + ['SERVICE_URL_UMAMI_3000', 'umami'], + ['SERVICE_URL_MYAPP_8080', 'myapp'], + + // Without port + ['SERVICE_URL_MY_APP', 'my_app'], + ['SERVICE_URL_REDIS_PRIMARY', 'redis_primary'], + ]; + + foreach ($testCases as [$varName, $expectedServiceName]) { + // Use the actual helper function from bootstrap/helpers/services.php + $parsed = parseServiceEnvironmentVariable($varName); + + expect($parsed['service_name'])->toBe($expectedServiceName, "Service name extraction for $varName"); + } +}); diff --git a/tests/Unit/ServicePortSpecificVariablesTest.php b/tests/Unit/ServicePortSpecificVariablesTest.php index 9c9966aaf..16aa74486 100644 --- a/tests/Unit/ServicePortSpecificVariablesTest.php +++ b/tests/Unit/ServicePortSpecificVariablesTest.php @@ -172,3 +172,50 @@ expect($extractedPort)->toBe((string) $port, "Port extraction failed for $description"); } }); + +it('detects port-specific variables with numeric suffix', function () { + // Test that variables ending with a numeric port are detected correctly + // This tests the logic: if last segment after _ is numeric, it's a port + + $tests = [ + // 2-underscore pattern: single-word service name + port + 'SERVICE_URL_MYAPP_3000' => ['service' => 'myapp', 'port' => '3000', 'hasPort' => true], + 'SERVICE_URL_REDIS_6379' => ['service' => 'redis', 'port' => '6379', 'hasPort' => true], + 'SERVICE_FQDN_NGINX_80' => ['service' => 'nginx', 'port' => '80', 'hasPort' => true], + + // 3-underscore pattern: two-word service name + port + 'SERVICE_URL_MY_API_8080' => ['service' => 'my_api', 'port' => '8080', 'hasPort' => true], + 'SERVICE_URL_WEB_APP_3000' => ['service' => 'web_app', 'port' => '3000', 'hasPort' => true], + 'SERVICE_FQDN_DB_SERVER_5432' => ['service' => 'db_server', 'port' => '5432', 'hasPort' => true], + + // 4-underscore pattern: three-word service name + port + 'SERVICE_URL_REDIS_CACHE_SERVER_6379' => ['service' => 'redis_cache_server', 'port' => '6379', 'hasPort' => true], + 'SERVICE_URL_MY_LONG_APP_8080' => ['service' => 'my_long_app', 'port' => '8080', 'hasPort' => true], + 'SERVICE_FQDN_POSTGRES_PRIMARY_DB_5432' => ['service' => 'postgres_primary_db', 'port' => '5432', 'hasPort' => true], + + // Non-numeric suffix: should NOT be treated as port-specific + 'SERVICE_URL_MY_APP' => ['service' => 'my_app', 'port' => null, 'hasPort' => false], + 'SERVICE_URL_REDIS_PRIMARY' => ['service' => 'redis_primary', 'port' => null, 'hasPort' => false], + 'SERVICE_FQDN_WEB_SERVER' => ['service' => 'web_server', 'port' => null, 'hasPort' => false], + 'SERVICE_URL_APP_CACHE_REDIS' => ['service' => 'app_cache_redis', 'port' => null, 'hasPort' => false], + + // Edge numeric cases + 'SERVICE_URL_APP_0' => ['service' => 'app', 'port' => '0', 'hasPort' => true], // Port 0 + 'SERVICE_URL_APP_99999' => ['service' => 'app', 'port' => '99999', 'hasPort' => true], // Port out of range + 'SERVICE_URL_APP_3.14' => ['service' => 'app_3.14', 'port' => null, 'hasPort' => false], // Float (should not be port) + 'SERVICE_URL_APP_1e5' => ['service' => 'app_1e5', 'port' => null, 'hasPort' => false], // Scientific notation + + // Edge cases + 'SERVICE_URL_APP' => ['service' => 'app', 'port' => null, 'hasPort' => false], + 'SERVICE_FQDN_DB' => ['service' => 'db', 'port' => null, 'hasPort' => false], + ]; + + foreach ($tests as $varName => $expected) { + // Use the actual helper function from bootstrap/helpers/services.php + $parsed = parseServiceEnvironmentVariable($varName); + + expect($parsed['service_name'])->toBe($expected['service'], "Service name mismatch for $varName"); + expect($parsed['port'])->toBe($expected['port'], "Port mismatch for $varName"); + expect($parsed['has_port'])->toBe($expected['hasPort'], "Port detection mismatch for $varName"); + } +});