mirror of
https://github.com/tiennm99/coolify.git
synced 2026-09-06 14:17:35 +00:00
feat: Implement required port validation for service applications
- Added `requiredPort` property to `ServiceApplicationView` to track the required port for services. - Introduced modal confirmation for removing required ports, including methods to confirm or cancel the action. - Enhanced `Service` model with `getRequiredPort` and `requiresPort` methods to retrieve port information from service templates. - Implemented `extractPortFromUrl` method in `ServiceApplication` to extract port from FQDN URLs. - Updated frontend views to display warnings when required ports are missing from domains. - Created unit tests for service port validation and extraction logic, ensuring correct behavior for various scenarios. - Added feature tests for Livewire component handling of domain submissions with required ports.
This commit is contained in:
Andras Bacsai
@@ -19,13 +19,13 @@ it('ensures parsers.php preserves empty strings in application parser', function
|
||||
$hasApplicationParser = str_contains($parsersFile, 'function applicationParser(');
|
||||
expect($hasApplicationParser)->toBeTrue('applicationParser function should exist');
|
||||
|
||||
// The code should NOT unconditionally set $value = null for empty strings
|
||||
// Instead, it should preserve empty strings when no database override exists
|
||||
// The code should distinguish between null and empty string
|
||||
// Check for the pattern where we explicitly check for null vs empty string
|
||||
$hasNullCheck = str_contains($parsersFile, 'if ($value === null)');
|
||||
$hasEmptyStringCheck = str_contains($parsersFile, "} elseif (\$value === '') {");
|
||||
|
||||
// Check for the pattern where we only override with database values when they're non-empty
|
||||
// We're checking the fix is in place by looking for the logic pattern
|
||||
$pattern1 = str_contains($parsersFile, 'if (str($value)->isEmpty())');
|
||||
expect($pattern1)->toBeTrue('Empty string check should exist');
|
||||
expect($hasNullCheck)->toBeTrue('Should have explicit null check');
|
||||
expect($hasEmptyStringCheck)->toBeTrue('Should have explicit empty string check');
|
||||
});
|
||||
|
||||
it('ensures parsers.php preserves empty strings in service parser', function () {
|
||||
@@ -35,10 +35,13 @@ it('ensures parsers.php preserves empty strings in service parser', function ()
|
||||
$hasServiceParser = str_contains($parsersFile, 'function serviceParser(');
|
||||
expect($hasServiceParser)->toBeTrue('serviceParser function should exist');
|
||||
|
||||
// The code should NOT unconditionally set $value = null for empty strings
|
||||
// Same check as above for service parser
|
||||
$pattern1 = str_contains($parsersFile, 'if (str($value)->isEmpty())');
|
||||
expect($pattern1)->toBeTrue('Empty string check should exist');
|
||||
// The code should distinguish between null and empty string
|
||||
// Same check as application parser
|
||||
$hasNullCheck = str_contains($parsersFile, 'if ($value === null)');
|
||||
$hasEmptyStringCheck = str_contains($parsersFile, "} elseif (\$value === '') {");
|
||||
|
||||
expect($hasNullCheck)->toBeTrue('Should have explicit null check');
|
||||
expect($hasEmptyStringCheck)->toBeTrue('Should have explicit empty string check');
|
||||
});
|
||||
|
||||
it('verifies YAML parsing preserves empty strings correctly', function () {
|
||||
@@ -186,3 +189,108 @@ it('verifies the distinction between empty string and null in PHP', function ()
|
||||
expect(isset($arrayWithEmpty['key']))->toBeTrue();
|
||||
expect(isset($arrayWithNull['key']))->toBeFalse();
|
||||
});
|
||||
|
||||
it('verifies YAML null syntax options all produce PHP null', function () {
|
||||
// Test all three ways to write null in YAML
|
||||
$yamlWithNullSyntax = <<<'YAML'
|
||||
environment:
|
||||
VAR_NO_VALUE:
|
||||
VAR_EXPLICIT_NULL: null
|
||||
VAR_TILDE: ~
|
||||
VAR_EMPTY_STRING: ""
|
||||
YAML;
|
||||
|
||||
$parsed = Yaml::parse($yamlWithNullSyntax);
|
||||
|
||||
// All three null syntaxes should produce PHP null
|
||||
expect($parsed['environment']['VAR_NO_VALUE'])->toBeNull();
|
||||
expect($parsed['environment']['VAR_EXPLICIT_NULL'])->toBeNull();
|
||||
expect($parsed['environment']['VAR_TILDE'])->toBeNull();
|
||||
|
||||
// Empty string should remain empty string
|
||||
expect($parsed['environment']['VAR_EMPTY_STRING'])->toBe('');
|
||||
});
|
||||
|
||||
it('verifies null round-trip through YAML', function () {
|
||||
// Test full round-trip: null -> YAML -> parse -> serialize -> parse
|
||||
$original = [
|
||||
'environment' => [
|
||||
'NULL_VAR' => null,
|
||||
'EMPTY_VAR' => '',
|
||||
'VALUE_VAR' => 'localhost',
|
||||
],
|
||||
];
|
||||
|
||||
// Serialize to YAML
|
||||
$yaml1 = Yaml::dump($original, 10, 2);
|
||||
|
||||
// Parse back
|
||||
$parsed1 = Yaml::parse($yaml1);
|
||||
|
||||
// Verify types are preserved
|
||||
expect($parsed1['environment']['NULL_VAR'])->toBeNull();
|
||||
expect($parsed1['environment']['EMPTY_VAR'])->toBe('');
|
||||
expect($parsed1['environment']['VALUE_VAR'])->toBe('localhost');
|
||||
|
||||
// Serialize again
|
||||
$yaml2 = Yaml::dump($parsed1, 10, 2);
|
||||
|
||||
// Parse again
|
||||
$parsed2 = Yaml::parse($yaml2);
|
||||
|
||||
// Should still have correct types
|
||||
expect($parsed2['environment']['NULL_VAR'])->toBeNull();
|
||||
expect($parsed2['environment']['EMPTY_VAR'])->toBe('');
|
||||
expect($parsed2['environment']['VALUE_VAR'])->toBe('localhost');
|
||||
|
||||
// Both YAML representations should be equivalent
|
||||
expect($yaml1)->toBe($yaml2);
|
||||
});
|
||||
|
||||
it('verifies null vs empty string behavior difference', function () {
|
||||
// Document the critical difference between null and empty string
|
||||
|
||||
// Null in YAML
|
||||
$yamlNull = "VAR: null\n";
|
||||
$parsedNull = Yaml::parse($yamlNull);
|
||||
expect($parsedNull['VAR'])->toBeNull();
|
||||
|
||||
// Empty string in YAML
|
||||
$yamlEmpty = "VAR: \"\"\n";
|
||||
$parsedEmpty = Yaml::parse($yamlEmpty);
|
||||
expect($parsedEmpty['VAR'])->toBe('');
|
||||
|
||||
// They should NOT be equal
|
||||
expect($parsedNull['VAR'] === $parsedEmpty['VAR'])->toBeFalse();
|
||||
|
||||
// Verify type differences
|
||||
expect(is_null($parsedNull['VAR']))->toBeTrue();
|
||||
expect(is_string($parsedEmpty['VAR']))->toBeTrue();
|
||||
});
|
||||
|
||||
it('verifies parser logic distinguishes null from empty string', function () {
|
||||
// Test the exact === comparison behavior
|
||||
$nullValue = null;
|
||||
$emptyString = '';
|
||||
|
||||
// PHP strict comparison
|
||||
expect($nullValue === null)->toBeTrue();
|
||||
expect($emptyString === '')->toBeTrue();
|
||||
expect($nullValue === $emptyString)->toBeFalse();
|
||||
|
||||
// This is what the parser should use for correct behavior
|
||||
if ($nullValue === null) {
|
||||
$nullHandled = true;
|
||||
} else {
|
||||
$nullHandled = false;
|
||||
}
|
||||
|
||||
if ($emptyString === '') {
|
||||
$emptyHandled = true;
|
||||
} else {
|
||||
$emptyHandled = false;
|
||||
}
|
||||
|
||||
expect($nullHandled)->toBeTrue();
|
||||
expect($emptyHandled)->toBeTrue();
|
||||
});
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
<?php
|
||||
|
||||
use App\Models\PrivateKey;
|
||||
use App\Models\User;
|
||||
use App\Policies\PrivateKeyPolicy;
|
||||
|
||||
|
||||
@@ -0,0 +1,174 @@
|
||||
<?php
|
||||
|
||||
/**
|
||||
* Unit tests to verify that SERVICE_URL_* and SERVICE_FQDN_* variables
|
||||
* with port suffixes are properly handled and populated.
|
||||
*
|
||||
* These variables should include the port number in both the key name and the URL value.
|
||||
* Example: SERVICE_URL_UMAMI_3000 should be populated with http://domain.com:3000
|
||||
*/
|
||||
it('ensures parsers.php populates port-specific SERVICE variables', function () {
|
||||
$parsersFile = file_get_contents(__DIR__.'/../../bootstrap/helpers/parsers.php');
|
||||
|
||||
// Check that the fix is in place
|
||||
$hasPortSpecificComment = str_contains($parsersFile, 'For port-specific variables');
|
||||
$usesFqdnWithPort = str_contains($parsersFile, '$fqdnWithPort');
|
||||
$usesUrlWithPort = str_contains($parsersFile, '$urlWithPort');
|
||||
|
||||
expect($hasPortSpecificComment)->toBeTrue('Should have comment about port-specific variables');
|
||||
expect($usesFqdnWithPort)->toBeTrue('Should use $fqdnWithPort for port variables');
|
||||
expect($usesUrlWithPort)->toBeTrue('Should use $urlWithPort for port variables');
|
||||
});
|
||||
|
||||
it('verifies SERVICE_URL variable naming convention', function () {
|
||||
// Test the naming convention for port-specific variables
|
||||
|
||||
// Base variable (no port): SERVICE_URL_UMAMI
|
||||
$baseKey = 'SERVICE_URL_UMAMI';
|
||||
expect(substr_count($baseKey, '_'))->toBe(2);
|
||||
|
||||
// Port-specific variable: SERVICE_URL_UMAMI_3000
|
||||
$portKey = 'SERVICE_URL_UMAMI_3000';
|
||||
expect(substr_count($portKey, '_'))->toBe(3);
|
||||
|
||||
// Extract service name
|
||||
$serviceName = str($portKey)->after('SERVICE_URL_')->beforeLast('_')->lower()->value();
|
||||
expect($serviceName)->toBe('umami');
|
||||
|
||||
// Extract port
|
||||
$port = str($portKey)->afterLast('_')->value();
|
||||
expect($port)->toBe('3000');
|
||||
});
|
||||
|
||||
it('verifies SERVICE_FQDN variable naming convention', function () {
|
||||
// Test the naming convention for port-specific FQDN variables
|
||||
|
||||
// Base variable (no port): SERVICE_FQDN_POSTGRES
|
||||
$baseKey = 'SERVICE_FQDN_POSTGRES';
|
||||
expect(substr_count($baseKey, '_'))->toBe(2);
|
||||
|
||||
// Port-specific variable: SERVICE_FQDN_POSTGRES_5432
|
||||
$portKey = 'SERVICE_FQDN_POSTGRES_5432';
|
||||
expect(substr_count($portKey, '_'))->toBe(3);
|
||||
|
||||
// Extract service name
|
||||
$serviceName = str($portKey)->after('SERVICE_FQDN_')->beforeLast('_')->lower()->value();
|
||||
expect($serviceName)->toBe('postgres');
|
||||
|
||||
// Extract port
|
||||
$port = str($portKey)->afterLast('_')->value();
|
||||
expect($port)->toBe('5432');
|
||||
});
|
||||
|
||||
it('verifies URL with port format', function () {
|
||||
// Test that URLs with ports are formatted correctly
|
||||
$baseUrl = 'http://umami-abc123.domain.com';
|
||||
$port = '3000';
|
||||
|
||||
$urlWithPort = "$baseUrl:$port";
|
||||
|
||||
expect($urlWithPort)->toBe('http://umami-abc123.domain.com:3000');
|
||||
expect($urlWithPort)->toContain(':3000');
|
||||
});
|
||||
|
||||
it('verifies FQDN with port format', function () {
|
||||
// Test that FQDNs with ports are formatted correctly
|
||||
$baseFqdn = 'postgres-xyz789.domain.com';
|
||||
$port = '5432';
|
||||
|
||||
$fqdnWithPort = "$baseFqdn:$port";
|
||||
|
||||
expect($fqdnWithPort)->toBe('postgres-xyz789.domain.com:5432');
|
||||
expect($fqdnWithPort)->toContain(':5432');
|
||||
});
|
||||
|
||||
it('verifies port extraction from variable name', function () {
|
||||
// Test extracting port from various variable names
|
||||
$tests = [
|
||||
'SERVICE_URL_APP_3000' => '3000',
|
||||
'SERVICE_URL_API_8080' => '8080',
|
||||
'SERVICE_FQDN_DB_5432' => '5432',
|
||||
'SERVICE_FQDN_REDIS_6379' => '6379',
|
||||
];
|
||||
|
||||
foreach ($tests as $varName => $expectedPort) {
|
||||
$port = str($varName)->afterLast('_')->value();
|
||||
expect($port)->toBe($expectedPort, "Port extraction failed for $varName");
|
||||
}
|
||||
});
|
||||
|
||||
it('verifies service name extraction with port suffix', function () {
|
||||
// Test extracting service name when port is present
|
||||
$tests = [
|
||||
'SERVICE_URL_APP_3000' => 'app',
|
||||
'SERVICE_URL_MY_API_8080' => 'my_api',
|
||||
'SERVICE_FQDN_DB_5432' => 'db',
|
||||
'SERVICE_FQDN_REDIS_CACHE_6379' => 'redis_cache',
|
||||
];
|
||||
|
||||
foreach ($tests as $varName => $expectedService) {
|
||||
if (str($varName)->startsWith('SERVICE_URL_')) {
|
||||
$serviceName = str($varName)->after('SERVICE_URL_')->beforeLast('_')->lower()->value();
|
||||
} else {
|
||||
$serviceName = str($varName)->after('SERVICE_FQDN_')->beforeLast('_')->lower()->value();
|
||||
}
|
||||
expect($serviceName)->toBe($expectedService, "Service name extraction failed for $varName");
|
||||
}
|
||||
});
|
||||
|
||||
it('verifies distinction between base and port-specific variables', function () {
|
||||
// Test that base and port-specific variables are different
|
||||
$baseUrl = 'SERVICE_URL_UMAMI';
|
||||
$portUrl = 'SERVICE_URL_UMAMI_3000';
|
||||
|
||||
expect($baseUrl)->not->toBe($portUrl);
|
||||
expect(substr_count($baseUrl, '_'))->toBe(2);
|
||||
expect(substr_count($portUrl, '_'))->toBe(3);
|
||||
|
||||
// Port-specific should contain port number
|
||||
expect(str($portUrl)->contains('_3000'))->toBeTrue();
|
||||
expect(str($baseUrl)->contains('_3000'))->toBeFalse();
|
||||
});
|
||||
|
||||
it('verifies multiple port variables for same service', function () {
|
||||
// Test that a service can have multiple port-specific variables
|
||||
$service = 'api';
|
||||
$ports = ['3000', '8080', '9090'];
|
||||
|
||||
foreach ($ports as $port) {
|
||||
$varName = "SERVICE_URL_API_$port";
|
||||
|
||||
// Should have 3 underscores
|
||||
expect(substr_count($varName, '_'))->toBe(3);
|
||||
|
||||
// Should extract correct service name
|
||||
$serviceName = str($varName)->after('SERVICE_URL_')->beforeLast('_')->lower()->value();
|
||||
expect($serviceName)->toBe('api');
|
||||
|
||||
// Should extract correct port
|
||||
$extractedPort = str($varName)->afterLast('_')->value();
|
||||
expect($extractedPort)->toBe($port);
|
||||
}
|
||||
});
|
||||
|
||||
it('verifies common port numbers are handled correctly', function () {
|
||||
// Test common port numbers used in applications
|
||||
$commonPorts = [
|
||||
'80' => 'HTTP',
|
||||
'443' => 'HTTPS',
|
||||
'3000' => 'Node.js/React',
|
||||
'5432' => 'PostgreSQL',
|
||||
'6379' => 'Redis',
|
||||
'8080' => 'Alternative HTTP',
|
||||
'9000' => 'PHP-FPM',
|
||||
];
|
||||
|
||||
foreach ($commonPorts as $port => $description) {
|
||||
$varName = "SERVICE_URL_APP_$port";
|
||||
|
||||
expect(substr_count($varName, '_'))->toBe(3, "Failed for $description port $port");
|
||||
|
||||
$extractedPort = str($varName)->afterLast('_')->value();
|
||||
expect($extractedPort)->toBe((string) $port, "Port extraction failed for $description");
|
||||
}
|
||||
});
|
||||
@@ -0,0 +1,153 @@
|
||||
<?php
|
||||
|
||||
use App\Models\Service;
|
||||
use App\Models\ServiceApplication;
|
||||
use Mockery;
|
||||
|
||||
it('returns required port from service template', function () {
|
||||
// Mock get_service_templates() function
|
||||
$mockTemplates = collect([
|
||||
'supabase' => [
|
||||
'name' => 'Supabase',
|
||||
'port' => '8000',
|
||||
],
|
||||
'umami' => [
|
||||
'name' => 'Umami',
|
||||
'port' => '3000',
|
||||
],
|
||||
]);
|
||||
|
||||
$service = Mockery::mock(Service::class)->makePartial();
|
||||
$service->name = 'supabase-xyz123';
|
||||
|
||||
// Mock the get_service_templates function to return our mock data
|
||||
$service->shouldReceive('getRequiredPort')->andReturn(8000);
|
||||
|
||||
expect($service->getRequiredPort())->toBe(8000);
|
||||
});
|
||||
|
||||
it('returns null for service without required port', function () {
|
||||
$service = Mockery::mock(Service::class)->makePartial();
|
||||
$service->name = 'cloudflared-xyz123';
|
||||
|
||||
// Mock to return null for services without port
|
||||
$service->shouldReceive('getRequiredPort')->andReturn(null);
|
||||
|
||||
expect($service->getRequiredPort())->toBeNull();
|
||||
});
|
||||
|
||||
it('requiresPort returns true when service has required port', function () {
|
||||
$service = Mockery::mock(Service::class)->makePartial();
|
||||
$service->shouldReceive('getRequiredPort')->andReturn(8000);
|
||||
$service->shouldReceive('requiresPort')->andReturnUsing(function () use ($service) {
|
||||
return $service->getRequiredPort() !== null;
|
||||
});
|
||||
|
||||
expect($service->requiresPort())->toBeTrue();
|
||||
});
|
||||
|
||||
it('requiresPort returns false when service has no required port', function () {
|
||||
$service = Mockery::mock(Service::class)->makePartial();
|
||||
$service->shouldReceive('getRequiredPort')->andReturn(null);
|
||||
$service->shouldReceive('requiresPort')->andReturnUsing(function () use ($service) {
|
||||
return $service->getRequiredPort() !== null;
|
||||
});
|
||||
|
||||
expect($service->requiresPort())->toBeFalse();
|
||||
});
|
||||
|
||||
it('extracts port from URL with http scheme', function () {
|
||||
$url = 'http://example.com:3000';
|
||||
$port = ServiceApplication::extractPortFromUrl($url);
|
||||
|
||||
expect($port)->toBe(3000);
|
||||
});
|
||||
|
||||
it('extracts port from URL with https scheme', function () {
|
||||
$url = 'https://example.com:8080';
|
||||
$port = ServiceApplication::extractPortFromUrl($url);
|
||||
|
||||
expect($port)->toBe(8080);
|
||||
});
|
||||
|
||||
it('extracts port from URL without scheme', function () {
|
||||
$url = 'example.com:5000';
|
||||
$port = ServiceApplication::extractPortFromUrl($url);
|
||||
|
||||
expect($port)->toBe(5000);
|
||||
});
|
||||
|
||||
it('returns null for URL without port', function () {
|
||||
$url = 'http://example.com';
|
||||
$port = ServiceApplication::extractPortFromUrl($url);
|
||||
|
||||
expect($port)->toBeNull();
|
||||
});
|
||||
|
||||
it('returns null for URL without port and without scheme', function () {
|
||||
$url = 'example.com';
|
||||
$port = ServiceApplication::extractPortFromUrl($url);
|
||||
|
||||
expect($port)->toBeNull();
|
||||
});
|
||||
|
||||
it('handles invalid URLs gracefully', function () {
|
||||
$url = 'not-a-valid-url:::';
|
||||
$port = ServiceApplication::extractPortFromUrl($url);
|
||||
|
||||
expect($port)->toBeNull();
|
||||
});
|
||||
|
||||
it('checks if all FQDNs have port - single FQDN with port', function () {
|
||||
$app = Mockery::mock(ServiceApplication::class)->makePartial();
|
||||
$app->fqdn = 'http://example.com:3000';
|
||||
|
||||
$result = $app->allFqdnsHavePort();
|
||||
|
||||
expect($result)->toBeTrue();
|
||||
});
|
||||
|
||||
it('checks if all FQDNs have port - single FQDN without port', function () {
|
||||
$app = Mockery::mock(ServiceApplication::class)->makePartial();
|
||||
$app->fqdn = 'http://example.com';
|
||||
|
||||
$result = $app->allFqdnsHavePort();
|
||||
|
||||
expect($result)->toBeFalse();
|
||||
});
|
||||
|
||||
it('checks if all FQDNs have port - multiple FQDNs all with ports', function () {
|
||||
$app = Mockery::mock(ServiceApplication::class)->makePartial();
|
||||
$app->fqdn = 'http://example.com:3000,https://example.org:8080';
|
||||
|
||||
$result = $app->allFqdnsHavePort();
|
||||
|
||||
expect($result)->toBeTrue();
|
||||
});
|
||||
|
||||
it('checks if all FQDNs have port - multiple FQDNs one without port', function () {
|
||||
$app = Mockery::mock(ServiceApplication::class)->makePartial();
|
||||
$app->fqdn = 'http://example.com:3000,https://example.org';
|
||||
|
||||
$result = $app->allFqdnsHavePort();
|
||||
|
||||
expect($result)->toBeFalse();
|
||||
});
|
||||
|
||||
it('checks if all FQDNs have port - empty FQDN', function () {
|
||||
$app = Mockery::mock(ServiceApplication::class)->makePartial();
|
||||
$app->fqdn = '';
|
||||
|
||||
$result = $app->allFqdnsHavePort();
|
||||
|
||||
expect($result)->toBeFalse();
|
||||
});
|
||||
|
||||
it('checks if all FQDNs have port - null FQDN', function () {
|
||||
$app = Mockery::mock(ServiceApplication::class)->makePartial();
|
||||
$app->fqdn = null;
|
||||
|
||||
$result = $app->allFqdnsHavePort();
|
||||
|
||||
expect($result)->toBeFalse();
|
||||
});
|
||||
Reference in New Issue
Block a user