fix(compose): normalize service-name keys for domains and env vars (#11040)

This commit is contained in:
Andras Bacsai authored and GitHub committed 2026-08-03 23:08:56 +02:00
1 parent 7b18777f06
commit 0b843bb07c
18 files changed
+1443 -220

No files matched your search

+10 -5
View File
@@ -1,5 +1,7 @@
<?php
use Illuminate\Support\Stringable;
/**
* Unit tests to verify that the applicationParser function in parsers.php
* properly converts Stringable objects to plain strings to fix strict
@@ -26,8 +28,8 @@ it('ensures service name normalization returns plain strings not Stringable obje
// Verify both are plain strings, not Stringable objects
expect(is_string($originalServiceName))->toBeTrue('$originalServiceName should be a plain string');
expect(is_string($serviceName))->toBeTrue('$serviceName should be a plain string');
expect($originalServiceName)->not->toBeInstanceOf(\Illuminate\Support\Stringable::class);
expect($serviceName)->not->toBeInstanceOf(\Illuminate\Support\Stringable::class);
expect($originalServiceName)->not->toBeInstanceOf(Stringable::class);
expect($serviceName)->not->toBeInstanceOf(Stringable::class);
// Verify the transformations work correctly
expect($originalServiceName)->toBe('my-service');
@@ -135,7 +137,7 @@ it('ensures originalServiceName conversion works for FQDN generation', function
$originalServiceName = str($serviceName)->replace('_', '-')->value();
expect(is_string($originalServiceName))->toBeTrue();
expect($originalServiceName)->not->toBeInstanceOf(\Illuminate\Support\Stringable::class);
expect($originalServiceName)->not->toBeInstanceOf(Stringable::class);
expect($originalServiceName)->toBe('my-service');
// Verify it can be used in string interpolation (line 544)
@@ -177,6 +179,9 @@ it('verifies parsers.php has the ->value() calls', function () {
// Line 539: Check originalServiceName conversion
expect($parsersFile)->toContain("str(\$serviceName)->replace('_', '-')->value()");
// Line 541: Check serviceName normalization
expect($parsersFile)->toContain("str(\$serviceName)->replace('-', '_')->replace('.', '_')->value()");
// Service name normalization for env keys / domain map (helper-based)
expect($parsersFile)->toContain('normalizeComposeServiceName((string) $serviceName)')
->and($parsersFile)->toContain('getComposeServiceDomainString')
->and($parsersFile)->toContain('putComposeServiceDomain')
->and($parsersFile)->toContain('findServiceApplicationForEnvName');
});
@@ -0,0 +1,224 @@
<?php
use App\Livewire\Project\Application\General;
use App\Models\Application;
use Illuminate\Support\Collection;
test('normalizeComposeServiceName replaces hyphens and dots', function () {
expect(normalizeComposeServiceName('another-service'))->toBe('another_service')
->and(normalizeComposeServiceName('api.test'))->toBe('api_test')
->and(normalizeComposeServiceName('api'))->toBe('api');
});
test('findComposeServiceName resolves legacy underscore keys to original names', function () {
$services = ['another-service', 'api.test', 'api'];
expect(findComposeServiceName('another-service', $services))->toBe('another-service')
->and(findComposeServiceName('another_service', $services))->toBe('another-service')
->and(findComposeServiceName('api.test', $services))->toBe('api.test')
->and(findComposeServiceName('api_test', $services))->toBe('api.test')
->and(findComposeServiceName('missing', $services))->toBeNull();
});
test('getComposeServiceDomainString reads original and legacy keys without dotted data_get bugs', function () {
$domains = [
'api.test' => ['domain' => 'https://dotted.example.com'],
'another_service' => ['domain' => 'https://legacy.example.com'],
];
expect(getComposeServiceDomainString($domains, 'api.test'))->toBe('https://dotted.example.com')
->and(getComposeServiceDomainString($domains, 'another-service'))->toBe('https://legacy.example.com')
->and(getComposeServiceDomainString($domains, 'missing'))->toBeNull();
});
test('putComposeServiceDomain writes original service keys and drops twin underscore keys', function () {
$domains = putComposeServiceDomain(
['another_service' => ['domain' => 'https://old.example.com']],
'another-service',
'https://new.example.com',
['another-service', 'analytics'],
);
expect($domains)->toHaveKey('another-service')
->and($domains)->not->toHaveKey('another_service')
->and($domains['another-service']['domain'])->toBe('https://new.example.com');
});
test('rekeyComposeDomainsToServiceNames migrates underscore keys to original compose names', function () {
$rekeyed = rekeyComposeDomainsToServiceNames(
[
'another_service' => ['domain' => 'https://hyphen.example.com'],
'api_test' => ['domain' => 'https://dotted.example.com'],
'orphan' => ['domain' => 'https://orphan.example.com'],
],
['another-service', 'api.test'],
);
expect($rekeyed)->toHaveKey('another-service')
->and($rekeyed)->not->toHaveKey('another_service')
->and($rekeyed['another-service']['domain'])->toBe('https://hyphen.example.com')
->and($rekeyed)->toHaveKey('api.test')
->and($rekeyed['api.test']['domain'])->toBe('https://dotted.example.com')
->and($rekeyed)->toHaveKey('orphan');
});
test('rekeyComposeDomainsToServiceNames prefers the canonical domain regardless of twin key order', function (array $domains) {
$rekeyed = rekeyComposeDomainsToServiceNames($domains, ['another-service']);
expect($rekeyed['another-service']['domain'])->toBe('https://canonical.example.com');
})->with([
'legacy key first' => [[
'another_service' => ['domain' => 'https://legacy.example.com'],
'another-service' => ['domain' => 'https://canonical.example.com'],
]],
'canonical key first' => [[
'another-service' => ['domain' => 'https://canonical.example.com'],
'another_service' => ['domain' => 'https://legacy.example.com'],
]],
]);
test('rekeyComposeDomainsToServiceNames never lets empty canonical wipe filled legacy', function (array $domains) {
$rekeyed = rekeyComposeDomainsToServiceNames($domains, ['another-service']);
expect($rekeyed)->toHaveKey('another-service')
->and($rekeyed)->not->toHaveKey('another_service')
->and($rekeyed['another-service']['domain'])->toBe('https://legacy.example.com');
})->with([
'empty canonical first' => [[
'another-service' => ['domain' => ''],
'another_service' => ['domain' => 'https://legacy.example.com'],
]],
'filled legacy first' => [[
'another_service' => ['domain' => 'https://legacy.example.com'],
'another-service' => ['domain' => ''],
]],
]);
test('getComposeServiceDomainString prefers filled twin over blank original key', function () {
$domains = [
'another-service' => ['domain' => ''],
'another_service' => ['domain' => 'https://legacy.example.com'],
];
expect(getComposeServiceDomainString($domains, 'another-service'))->toBe('https://legacy.example.com')
->and(getComposeServiceDomainString($domains, 'another_service'))->toBe('https://legacy.example.com');
});
test('preferredComposeServiceNamesFromDomainKeys collapses underscore twins', function () {
expect(preferredComposeServiceNamesFromDomainKeys(['web_api', 'web-api', 'api']))->toEqualCanonicalizing(['web-api', 'api']);
});
test('legacy underscore domain keys still resolve for hyphenated compose services', function () {
// Existing production shape: domains stored under underscore keys only.
$legacy = [
'another_service' => ['domain' => 'https://legacy.example.com'],
'web' => ['domain' => 'https://web.example.com'],
];
expect(getComposeServiceDomainString($legacy, 'another-service'))->toBe('https://legacy.example.com')
->and(getComposeServiceDomainString($legacy, 'another_service'))->toBe('https://legacy.example.com')
->and(getComposeServiceDomainString($legacy, 'web'))->toBe('https://web.example.com');
});
test('SERVICE env keys stay underscore-normalized for both storage shapes', function () {
// Mirrors ApplicationDeploymentJob: domain map key may be original or legacy.
foreach (['another-service', 'another_service'] as $storageKey) {
$envKey = str(normalizeComposeServiceName($storageKey))->upper()->toString();
expect($envKey)->toBe('ANOTHER_SERVICE')
->and('SERVICE_URL_'.$envKey)->toBe('SERVICE_URL_ANOTHER_SERVICE');
}
});
test('findComposeServiceName maps SERVICE env fragments used by serviceParser magic path', function () {
// serviceParser historically only did underscore→hyphen, so API_TEST became name "api-test"
// and missed compose service "api.test". normalize-based lookup finds the real name.
$services = ['another-service', 'api.test', 'web'];
expect(findComposeServiceName('another_service', $services))->toBe('another-service')
->and(findComposeServiceName('api_test', $services))->toBe('api.test')
->and(findComposeServiceName('api-test', $services))->toBe('api.test') // same normalized form
->and(findComposeServiceName('web', $services))->toBe('web');
// Exact DB lookup the old serviceParser used would miss dotted names:
expect(in_array('api-test', $services, true))->toBeFalse()
->and(in_array('api.test', $services, true))->toBeTrue();
});
test('normalized service name collisions only resolve exact matches', function () {
$services = ['api-test', 'api.test'];
expect(findComposeServiceName('api-test', $services))->toBe('api-test')
->and(findComposeServiceName('api.test', $services))->toBe('api.test')
->and(findComposeServiceName('api_test', $services))->toBeNull();
});
test('rekeyComposeDomainsToServiceNames keeps ambiguous normalized keys separate', function () {
$rekeyed = rekeyComposeDomainsToServiceNames(
[
'api-test' => ['domain' => 'https://hyphen.example.com'],
'api.test' => ['domain' => 'https://dot.example.com'],
'api_test' => ['domain' => 'https://legacy.example.com'],
],
['api-test', 'api.test'],
);
expect($rekeyed['api-test']['domain'])->toBe('https://hyphen.example.com')
->and($rekeyed['api.test']['domain'])->toBe('https://dot.example.com')
->and($rekeyed['api_test']['domain'])->toBe('https://legacy.example.com');
});
test('empty parsed compose services do not wipe existing domains', function () {
$application = new Application;
$application->docker_compose_domains = json_encode([
'web' => ['domain' => 'https://web.example.com'],
]);
$method = new ReflectionMethod($application, 'reconcileDockerComposeDomains');
$method->invoke($application, collect(['services' => []]));
expect($application->docker_compose_domains)->toBe(json_encode([
'web' => ['domain' => 'https://web.example.com'],
]));
});
test('normalized service collisions retain domains under their exact compose keys', function () {
$domains = collect([
'api-test' => ['domain' => 'https://hyphen.example.com'],
'api.test' => ['domain' => 'https://dot.example.com'],
]);
$rekeyed = rekeyComposeDomainsToServiceNames($domains, collect(['api-test', 'api.test']));
expect($rekeyed)->toBe([
'api-test' => ['domain' => 'https://hyphen.example.com'],
'api.test' => ['domain' => 'https://dot.example.com'],
]);
});
test('General rekeys form domains to original compose service names', function (array|Collection $domains) {
$component = new General;
$component->parsedServices = collect([
'services' => collect([
'web-api' => [],
'metrics.internal' => [],
]),
]);
$component->parsedServiceDomains = $domains;
$method = new ReflectionMethod($component, 'composeDomainsForStorage');
$stored = $method->invoke($component);
expect($stored)->toBe([
'web-api' => ['domain' => 'https://api.example.com'],
'metrics.internal' => ['domain' => 'https://metrics.example.com'],
]);
})->with([
'array' => [[
'web_api' => ['domain' => 'https://api.example.com'],
'metrics_internal' => ['domain' => 'https://metrics.example.com'],
]],
'collection' => [collect([
'web_api' => ['domain' => 'https://api.example.com'],
'metrics_internal' => ['domain' => 'https://metrics.example.com'],
])],
]);
+93
View File
@@ -0,0 +1,93 @@
<?php
/**
* Shared domain URL helpers used by application/service parsers and updateCompose.
*
* TDD targets:
* - getHostWithoutPort(): COOLIFY_FQDN / SERVICE_FQDN host-only form (no scheme, no port)
* - firstDomainFromList(): comma-separated FQDN lists (same as updateCompose / serviceParser)
* - getFqdnWithoutPort() remains the scheme+host form for COOLIFY_URL / SERVICE_URL
*/
// ---------------------------------------------------------------------------
// getHostWithoutPort — expected behavior (fails until helper exists / is correct)
// ---------------------------------------------------------------------------
test('getHostWithoutPort strips scheme and port from simple urls', function () {
expect(getHostWithoutPort('http://git.example.com:80'))->toBe('git.example.com')
->and(getHostWithoutPort('https://n8n.example.com:5678'))->toBe('n8n.example.com')
->and(getHostWithoutPort('http://git.example.com'))->toBe('git.example.com');
});
test('getHostWithoutPort preserves path segments', function () {
expect(getHostWithoutPort('http://git.example.com:80/v1/realtime'))
->toBe('git.example.com/v1/realtime');
});
test('getHostWithoutPort keeps real host when credentials are present', function () {
// Legacy COOLIFY_FQDN strip: replace scheme then before(':') → "user"
expect(getHostWithoutPort('http://user:secret@git.example.com:80'))
->toBe('git.example.com');
});
test('getHostWithoutPort handles scheme-less host:port', function () {
expect(getHostWithoutPort('git.example.com:80'))->toBe('git.example.com');
});
test('getHostWithoutPort returns original when host cannot be parsed', function () {
expect(getHostWithoutPort('not-a-url'))->toBe('not-a-url');
});
test('legacy COOLIFY_FQDN strip corrupts credential urls (documents why we refactor)', function () {
$fqdn = 'http://user:secret@git.example.com:80';
$legacy = (string) str($fqdn)
->replace('http://', '')
->replace('https://', '')
->before(':');
expect($legacy)->toBe('user')
->and($legacy)->not->toBe(getHostWithoutPort($fqdn));
});
// ---------------------------------------------------------------------------
// firstDomainFromList
// ---------------------------------------------------------------------------
test('firstDomainFromList returns the first comma-separated domain trimmed', function () {
expect(firstDomainFromList('http://a.example.com:80,http://b.example.com:80'))
->toBe('http://a.example.com:80')
->and(firstDomainFromList(' https://only.example.com '))
->toBe('https://only.example.com')
->and(firstDomainFromList(null))
->toBe('')
->and(firstDomainFromList(''))
->toBe('');
});
// ---------------------------------------------------------------------------
// updateCompose / parser pairing: URL + FQDN from the same helpers
// ---------------------------------------------------------------------------
test('url and host helpers stay paired for ported service domains', function () {
$fqdn = 'http://git.example.com:80';
$urlValue = getFqdnWithoutPort($fqdn);
$fqdnValue = getHostWithoutPort($fqdn);
$port = '80';
expect($urlValue)->toBe('http://git.example.com')
->and($fqdnValue)->toBe('git.example.com')
->and($urlValue.':'.$port)->toBe('http://git.example.com:80')
->and($fqdnValue.':'.$port)->toBe('git.example.com:80')
->and($urlValue.':'.$port)->not->toContain('/:');
});
test('first domain then helpers match multi-domain service FQDN handling', function () {
$saved = 'http://git.example.com:80,http://git-alt.example.com:80';
$first = firstDomainFromList($saved);
expect($first)->toBe('http://git.example.com:80')
->and(getFqdnWithoutPort($first))->toBe('http://git.example.com')
->and(getHostWithoutPort($first))->toBe('git.example.com');
});
+51
View File
@@ -0,0 +1,51 @@
<?php
use Spatie\Url\Url;
/**
* Regression tests for service domain FQDN port handling (#8798 / #8980).
*
* Fragile str()->after('://')->before(':') stripping corrupts URLs that contain
* credentials, IPv6, or other non-trivial structure. getFqdnWithoutPort() must
* strip only the port and keep a usable base URL for COOLIFY_URL / SERVICE_URL.
*/
test('getFqdnWithoutPort strips port from simple http urls without trailing slash', function () {
expect(getFqdnWithoutPort('http://git.example.com:80'))->toBe('http://git.example.com')
->and(getFqdnWithoutPort('https://n8n.example.com:5678'))->toBe('https://n8n.example.com')
->and(getFqdnWithoutPort('http://git.example.com'))->toBe('http://git.example.com');
});
test('getFqdnWithoutPort preserves path segments when stripping port', function () {
expect(getFqdnWithoutPort('http://git.example.com:80/v1/realtime'))
->toBe('http://git.example.com/v1/realtime');
});
test('getFqdnWithoutPort keeps host when credentials are present', function () {
// Fragile strip would turn this into "http://user" — helper must not.
expect(getFqdnWithoutPort('http://user:secret@git.example.com:80'))
->toBe('http://git.example.com');
});
test('getFqdnWithoutPort returns original string when input is not a valid url', function () {
expect(getFqdnWithoutPort('not-a-url'))->toBe('not-a-url');
});
test('fragile after/before strip corrupts credential urls (documents #8798 class of bugs)', function () {
$fqdn = 'http://user:secret@git.example.com:80';
$fragile = (string) str($fqdn)
->after('://')
->before(':')
->prepend(str($fqdn)->before('://')->append('://'));
expect($fragile)->toBe('http://user')
->and($fragile)->not->toBe(getFqdnWithoutPort($fqdn));
});
test('port comparisons must cast so string template ports match int url ports', function () {
$envPort = Url::fromString('http://git.example.com:80')->getPort();
$predefinedPort = '80'; // yaml / data_get style
expect($envPort !== $predefinedPort)->toBeTrue('strict compare wrongly treats matching ports as different')
->and((int) $envPort !== (int) $predefinedPort)->toBeFalse();
});
@@ -0,0 +1,97 @@
<?php
test('traefikServiceNameHash is stable and length 4', function () {
expect(traefikServiceNameHash('api.test'))->toBe(substr(md5('api.test'), 0, 4))
->and(traefikServiceNameHash('api.test'))->toHaveLength(4)
->and(traefikServiceNameHash('api.test'))->not->toBe(traefikServiceNameHash('api-test'));
});
test('traefikSafeServiceNameSegment replaces dots and always appends hash', function () {
$segment = traefikSafeServiceNameSegment('api.test');
$hash = traefikServiceNameHash('api.test');
expect($segment)->toBe("api-test-{$hash}")
->and($segment)->not->toContain('.')
->and(str_ends_with($segment, "-{$hash}"))->toBeTrue();
});
test('traefikSafeServiceNameSegment keeps api.test and api-test distinct', function () {
$dot = traefikSafeServiceNameSegment('api.test');
$hyphen = traefikSafeServiceNameSegment('api-test');
expect($dot)->not->toBe($hyphen)
->and($dot)->toStartWith('api-test-')
->and($hyphen)->toStartWith('api-test-');
});
test('traefikSafeServiceNameSegment normalizes underscores and odd characters', function () {
$segment = traefikSafeServiceNameSegment('web_api.v2');
$hash = traefikServiceNameHash('web_api.v2');
expect($segment)->toBe("web-api-v2-{$hash}")
->and($segment)->not->toContain('.')
->and($segment)->not->toContain('_');
});
test('fqdnLabelsForTraefik embeds safe hashed service segment without dotted router names', function () {
$uuid = 'testuuid1234';
$hashDot = traefikServiceNameHash('api.test');
$hashHyphen = traefikServiceNameHash('api-test');
$dotLabels = fqdnLabelsForTraefik(
uuid: $uuid,
domains: collect(['https://dot.example.com']),
is_force_https_enabled: true,
service_name: 'api.test',
image: 'nginx:alpine',
);
$hyphenLabels = fqdnLabelsForTraefik(
uuid: $uuid,
domains: collect(['https://hyphen.example.com']),
is_force_https_enabled: true,
service_name: 'api-test',
image: 'nginx:alpine',
);
$dotHttps = $dotLabels->first(
fn (string $line) => str_contains($line, "traefik.http.routers.https-0-{$uuid}-api-test-{$hashDot}.rule=")
);
$hyphenHttps = $hyphenLabels->first(
fn (string $line) => str_contains($line, "traefik.http.routers.https-0-{$uuid}-api-test-{$hashHyphen}.rule=")
);
expect($dotHttps)->not->toBeNull()
->and($hyphenHttps)->not->toBeNull()
->and($dotHttps)->not->toContain('api.test')
->and($dotHttps)->not->toBe($hyphenHttps)
->and($hashDot)->not->toBe($hashHyphen);
$ruleLines = $dotLabels->merge($hyphenLabels)->filter(
fn (string $line) => str_contains($line, 'traefik.http.routers.') && str_contains($line, '.rule=')
);
// Traefik label path must stay at traefik.http.routers.{name}.rule (5 segments).
foreach ($ruleLines as $line) {
$key = str($line)->before('=')->toString();
expect(explode('.', $key))->toHaveCount(5);
}
});
test('fqdnLabelsForTraefik hyphenated services also receive a hash suffix', function () {
$uuid = 'appuuid';
$hash = traefikServiceNameHash('another-service');
$labels = fqdnLabelsForTraefik(
uuid: $uuid,
domains: collect(['https://another.example.com']),
service_name: 'another-service',
image: 'nginx:alpine',
);
$routerLine = $labels->first(
fn (string $line) => str_contains($line, 'traefik.http.routers.https-0-') && str_contains($line, '.rule=')
);
expect($routerLine)->toContain("https-0-{$uuid}-another-service-{$hash}.rule=");
});