Mirrors/coolify
1
0
Fork 0
mirror of https://github.com/coollabsio/coolify.git synced 2025-12-28 05:34:50 +00:00
Code Issues Actions 11 Packages Projects Releases Wiki Activity

feat: implement service environment variable parsing and add unit tests for port detection logic

Browse Source
This commit is contained in:
Andras Bacsai 2025-11-11 11:19:33 +01:00
parent 6d6ebe92ff
commit 7fc4a2f7f6
4 changed files with 274 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
bootstrap/helpers/parsers.php
View File

@ -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([

50
bootstrap/helpers/services.php
View File

@ -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,
];
}

158
tests/Unit/ServiceParserPortDetectionLogicTest.php Normal file
View File

@ -0,0 +1,158 @@
<?php
/**
* Unit tests to verify the parser logic for detecting port-specific SERVICE variables.
* These tests simulate the logic used in bootstrap/helpers/parsers.php without database operations.
*
* The parser should detect when a SERVICE_URL_* or SERVICE_FQDN_* variable has a numeric
* port suffix and extract both the service name and port correctly.
*/
it('detects port suffix using numeric check (correct logic)', function () {
// This tests the CORRECT logic: check if last segment is numeric
$testCases = [
// [variable_name, expected_service_name, expected_port, is_port_specific]
// 2-underscore pattern: SERVICE_URL_{SERVICE}_{PORT}
['SERVICE_URL_MYAPP_3000', 'myapp', '3000', true],
['SERVICE_URL_REDIS_6379', 'redis', '6379', true],
['SERVICE_FQDN_NGINX_80', 'nginx', '80', true],
// 3-underscore pattern: SERVICE_URL_{SERVICE}_{NAME}_{PORT}
['SERVICE_URL_MY_API_8080', 'my_api', '8080', true],
['SERVICE_URL_WEB_APP_3000', 'web_app', '3000', true],
['SERVICE_FQDN_DB_SERVER_5432', 'db_server', '5432', true],
// 4-underscore pattern: SERVICE_URL_{SERVICE}_{NAME}_{OTHER}_{PORT}
['SERVICE_URL_REDIS_CACHE_SERVER_6379', 'redis_cache_server', '6379', true],
['SERVICE_URL_MY_LONG_APP_8080', 'my_long_app', '8080', true],
['SERVICE_FQDN_POSTGRES_PRIMARY_DB_5432', 'postgres_primary_db', '5432', true],
// Non-numeric suffix: should NOT be port-specific
['SERVICE_URL_MY_APP', 'my_app', null, false],
['SERVICE_URL_REDIS_PRIMARY', 'redis_primary', null, false],
['SERVICE_FQDN_WEB_SERVER', 'web_server', null, false],
['SERVICE_URL_APP_CACHE_REDIS', 'app_cache_redis', null, false],
// Single word without port
['SERVICE_URL_APP', 'app', null, false],
['SERVICE_FQDN_DB', 'db', null, false],
// Edge cases with numbers in service name
['SERVICE_URL_REDIS2_MASTER', 'redis2_master', null, false],
['SERVICE_URL_WEB3_APP', 'web3_app', null, false],
];
foreach ($testCases as [$varName, $expectedService, $expectedPort, $isPortSpecific]) {
// Use the actual helper function from bootstrap/helpers/services.php
$parsed = parseServiceEnvironmentVariable($varName);
// Assertions
expect($parsed['service_name'])->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");
}
});

47
tests/Unit/ServicePortSpecificVariablesTest.php
View File

@ -172,3 +172,50 @@ it('verifies common port numbers are handled correctly', function () {
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");
}
});