fix: prevent SERVICE_FQDN/SERVICE_URL path duplication (#7370)

2026-04-17 17:21:04 +00:00 · 2025-11-27 10:59:39 +01:00
parent 7009cef8a4 0ecaa191db
commit 639a56be52
3 changed files with 380 additions and 3 deletions
--- a/bootstrap/helpers/parsers.php
+++ b/bootstrap/helpers/parsers.php
@@ -1644,9 +1644,16 @@ function serviceParser(Service $resource): Collection
                if ($value && get_class($value) === \Illuminate\Support\Stringable::class && $value->startsWith('/')) {
                    $path = $value->value();
                    if ($path !== '/') {
-                        $fqdn = "$fqdn$path";
-                        $url = "$url$path";
-                        $fqdnValueForEnv = "$fqdnValueForEnv$path";
+                        // Only add path if it's not already present (prevents duplication on subsequent parse() calls)
+                        if (! str($fqdn)->endsWith($path)) {
+                            $fqdn = "$fqdn$path";
+                        }
+                        if (! str($url)->endsWith($path)) {
+                            $url = "$url$path";
+                        }
+                        if (! str($fqdnValueForEnv)->endsWith($path)) {
+                            $fqdnValueForEnv = "$fqdnValueForEnv$path";
+                        }
                    }
                }

--- a/tests/Feature/ServiceFqdnUpdatePathTest.php
+++ b/tests/Feature/ServiceFqdnUpdatePathTest.php
@@ -0,0 +1,220 @@
+<?php
+
+/**
+ * Feature tests to verify that FQDN updates don't cause path duplication
+ * for services with path-based SERVICE_URL/SERVICE_FQDN template variables.
+ *
+ * This tests the fix for GitHub issue #7363 where Appwrite and MindsDB services
+ * had their paths duplicated (e.g., /v1/realtime/v1/realtime) after FQDN updates.
+ *
+ * IMPORTANT: These tests require database access and must be run inside Docker:
+ * docker exec coolify php artisan test --filter ServiceFqdnUpdatePathTest
+ */
+
+use App\Models\Server;
+use App\Models\Service;
+use App\Models\ServiceApplication;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+
+uses(RefreshDatabase::class);
+
+test('Appwrite realtime service does not duplicate path on FQDN update', function () {
+    // Create a server
+    $server = Server::factory()->create([
+        'name' => 'test-server',
+        'ip' => '127.0.0.1',
+    ]);
+
+    // Load Appwrite template
+    $appwriteTemplate = file_get_contents(base_path('templates/compose/appwrite.yaml'));
+
+    // Create Appwrite service
+    $service = Service::factory()->create([
+        'server_id' => $server->id,
+        'name' => 'appwrite-test',
+        'docker_compose_raw' => $appwriteTemplate,
+    ]);
+
+    // Create the appwrite-realtime service application
+    $serviceApp = ServiceApplication::factory()->create([
+        'service_id' => $service->id,
+        'name' => 'appwrite-realtime',
+        'fqdn' => 'https://test.abc/v1/realtime',
+    ]);
+
+    // Parse the service (simulates initial setup)
+    $service->parse();
+
+    // Get environment variable
+    $urlVar = $service->environment_variables()
+        ->where('key', 'SERVICE_URL_APPWRITE')
+        ->first();
+
+    // Initial setup should have path once
+    expect($urlVar)->not->toBeNull()
+        ->and($urlVar->value)->not->toContain('/v1/realtime/v1/realtime')
+        ->and($urlVar->value)->toContain('/v1/realtime');
+
+    // Simulate user updating FQDN
+    $serviceApp->fqdn = 'https://newdomain.com/v1/realtime';
+    $serviceApp->save();
+
+    // Call parse again (this is where the bug occurred)
+    $service->parse();
+
+    // Check that path is not duplicated
+    $urlVar = $service->environment_variables()
+        ->where('key', 'SERVICE_URL_APPWRITE')
+        ->first();
+
+    expect($urlVar)->not->toBeNull()
+        ->and($urlVar->value)->not->toContain('/v1/realtime/v1/realtime')
+        ->and($urlVar->value)->toContain('/v1/realtime');
+})->skip('Requires database and Appwrite template - run in Docker');
+
+test('Appwrite console service does not duplicate /console path', function () {
+    $server = Server::factory()->create();
+    $appwriteTemplate = file_get_contents(base_path('templates/compose/appwrite.yaml'));
+    $service = Service::factory()->create([
+        'server_id' => $server->id,
+        'docker_compose_raw' => $appwriteTemplate,
+    ]);
+
+    $serviceApp = ServiceApplication::factory()->create([
+        'service_id' => $service->id,
+        'name' => 'appwrite-console',
+        'fqdn' => 'https://test.abc/console',
+    ]);
+
+    // Parse service
+    $service->parse();
+
+    // Update FQDN
+    $serviceApp->fqdn = 'https://newdomain.com/console';
+    $serviceApp->save();
+
+    // Parse again
+    $service->parse();
+
+    // Verify no duplication
+    $urlVar = $service->environment_variables()
+        ->where('key', 'SERVICE_URL_APPWRITE')
+        ->first();
+
+    expect($urlVar)->not->toBeNull()
+        ->and($urlVar->value)->not->toContain('/console/console')
+        ->and($urlVar->value)->toContain('/console');
+})->skip('Requires database and Appwrite template - run in Docker');
+
+test('MindsDB service does not duplicate /api path', function () {
+    $server = Server::factory()->create();
+    $mindsdbTemplate = file_get_contents(base_path('templates/compose/mindsdb.yaml'));
+    $service = Service::factory()->create([
+        'server_id' => $server->id,
+        'docker_compose_raw' => $mindsdbTemplate,
+    ]);
+
+    $serviceApp = ServiceApplication::factory()->create([
+        'service_id' => $service->id,
+        'name' => 'mindsdb',
+        'fqdn' => 'https://test.abc/api',
+    ]);
+
+    // Parse service
+    $service->parse();
+
+    // Update FQDN multiple times
+    $serviceApp->fqdn = 'https://domain1.com/api';
+    $serviceApp->save();
+    $service->parse();
+
+    $serviceApp->fqdn = 'https://domain2.com/api';
+    $serviceApp->save();
+    $service->parse();
+
+    // Verify no duplication after multiple updates
+    $urlVar = $service->environment_variables()
+        ->where('key', 'SERVICE_URL_API')
+        ->orWhere('key', 'LIKE', 'SERVICE_URL_%')
+        ->first();
+
+    expect($urlVar)->not->toBeNull()
+        ->and($urlVar->value)->not->toContain('/api/api');
+})->skip('Requires database and MindsDB template - run in Docker');
+
+test('service without path declaration is not affected', function () {
+    $server = Server::factory()->create();
+
+    // Create a simple service without path in template
+    $simpleTemplate = <<<'YAML'
+services:
+  redis:
+    image: redis:7
+    environment:
+      - SERVICE_FQDN_REDIS
+YAML;
+
+    $service = Service::factory()->create([
+        'server_id' => $server->id,
+        'docker_compose_raw' => $simpleTemplate,
+    ]);
+
+    $serviceApp = ServiceApplication::factory()->create([
+        'service_id' => $service->id,
+        'name' => 'redis',
+        'fqdn' => 'https://redis.test.abc',
+    ]);
+
+    // Parse service
+    $service->parse();
+
+    $fqdnBefore = $service->environment_variables()
+        ->where('key', 'SERVICE_FQDN_REDIS')
+        ->first()?->value;
+
+    // Update FQDN
+    $serviceApp->fqdn = 'https://redis.newdomain.com';
+    $serviceApp->save();
+
+    // Parse again
+    $service->parse();
+
+    $fqdnAfter = $service->environment_variables()
+        ->where('key', 'SERVICE_FQDN_REDIS')
+        ->first()?->value;
+
+    // Should work normally without issues
+    expect($fqdnAfter)->toBe('redis.newdomain.com')
+        ->and($fqdnAfter)->not->toContain('//');
+})->skip('Requires database - run in Docker');
+
+test('multiple FQDN updates never cause path duplication', function () {
+    $server = Server::factory()->create();
+    $appwriteTemplate = file_get_contents(base_path('templates/compose/appwrite.yaml'));
+    $service = Service::factory()->create([
+        'server_id' => $server->id,
+        'docker_compose_raw' => $appwriteTemplate,
+    ]);
+
+    $serviceApp = ServiceApplication::factory()->create([
+        'service_id' => $service->id,
+        'name' => 'appwrite-realtime',
+        'fqdn' => 'https://test.abc/v1/realtime',
+    ]);
+
+    // Update FQDN 10 times and parse each time
+    for ($i = 1; $i <= 10; $i++) {
+        $serviceApp->fqdn = "https://domain{$i}.com/v1/realtime";
+        $serviceApp->save();
+        $service->parse();
+
+        // Check path is never duplicated
+        $urlVar = $service->environment_variables()
+            ->where('key', 'SERVICE_URL_APPWRITE')
+            ->first();
+
+        expect($urlVar)->not->toBeNull()
+            ->and($urlVar->value)->not->toContain('/v1/realtime/v1/realtime')
+            ->and($urlVar->value)->toContain('/v1/realtime');
+    }
+})->skip('Requires database and Appwrite template - run in Docker');
--- a/tests/Unit/ServiceParserPathDuplicationTest.php
+++ b/tests/Unit/ServiceParserPathDuplicationTest.php
@@ -0,0 +1,150 @@
+<?php
+
+/**
+ * Unit tests to verify that serviceParser() correctly handles path appending
+ * to prevent duplication when SERVICE_URL/SERVICE_FQDN variables have path values.
+ *
+ * This tests the fix for GitHub issue #7363 where paths like /v1/realtime
+ * were being duplicated on subsequent parse() calls after FQDN updates.
+ */
+test('path is added when FQDN does not already contain it', function () {
+    $fqdn = 'https://test.abc';
+    $path = '/v1/realtime';
+
+    // Simulate the logic in serviceParser()
+    if (! str($fqdn)->endsWith($path)) {
+        $fqdn = "$fqdn$path";
+    }
+
+    expect($fqdn)->toBe('https://test.abc/v1/realtime');
+});
+
+test('path is not added when FQDN already contains it', function () {
+    $fqdn = 'https://test.abc/v1/realtime';
+    $path = '/v1/realtime';
+
+    // Simulate the logic in serviceParser()
+    if (! str($fqdn)->endsWith($path)) {
+        $fqdn = "$fqdn$path";
+    }
+
+    expect($fqdn)->toBe('https://test.abc/v1/realtime');
+});
+
+test('multiple parse calls with same path do not cause duplication', function () {
+    $fqdn = 'https://test.abc';
+    $path = '/v1/realtime';
+
+    // First parse (initial creation)
+    if (! str($fqdn)->endsWith($path)) {
+        $fqdn = "$fqdn$path";
+    }
+    expect($fqdn)->toBe('https://test.abc/v1/realtime');
+
+    // Second parse (after FQDN update)
+    if (! str($fqdn)->endsWith($path)) {
+        $fqdn = "$fqdn$path";
+    }
+    expect($fqdn)->toBe('https://test.abc/v1/realtime');
+
+    // Third parse (another update)
+    if (! str($fqdn)->endsWith($path)) {
+        $fqdn = "$fqdn$path";
+    }
+    expect($fqdn)->toBe('https://test.abc/v1/realtime');
+});
+
+test('different paths for different services work correctly', function () {
+    // Appwrite main service (/)
+    $fqdn1 = 'https://test.abc';
+    $path1 = '/';
+    if ($path1 !== '/' && ! str($fqdn1)->endsWith($path1)) {
+        $fqdn1 = "$fqdn1$path1";
+    }
+    expect($fqdn1)->toBe('https://test.abc');
+
+    // Appwrite console (/console)
+    $fqdn2 = 'https://test.abc';
+    $path2 = '/console';
+    if ($path2 !== '/' && ! str($fqdn2)->endsWith($path2)) {
+        $fqdn2 = "$fqdn2$path2";
+    }
+    expect($fqdn2)->toBe('https://test.abc/console');
+
+    // Appwrite realtime (/v1/realtime)
+    $fqdn3 = 'https://test.abc';
+    $path3 = '/v1/realtime';
+    if ($path3 !== '/' && ! str($fqdn3)->endsWith($path3)) {
+        $fqdn3 = "$fqdn3$path3";
+    }
+    expect($fqdn3)->toBe('https://test.abc/v1/realtime');
+});
+
+test('nested paths are handled correctly', function () {
+    $fqdn = 'https://test.abc';
+    $path = '/api/v1/endpoint';
+
+    if (! str($fqdn)->endsWith($path)) {
+        $fqdn = "$fqdn$path";
+    }
+
+    expect($fqdn)->toBe('https://test.abc/api/v1/endpoint');
+
+    // Second call should not duplicate
+    if (! str($fqdn)->endsWith($path)) {
+        $fqdn = "$fqdn$path";
+    }
+
+    expect($fqdn)->toBe('https://test.abc/api/v1/endpoint');
+});
+
+test('MindsDB /api path is handled correctly', function () {
+    $fqdn = 'https://test.abc';
+    $path = '/api';
+
+    // First parse
+    if (! str($fqdn)->endsWith($path)) {
+        $fqdn = "$fqdn$path";
+    }
+    expect($fqdn)->toBe('https://test.abc/api');
+
+    // Second parse should not duplicate
+    if (! str($fqdn)->endsWith($path)) {
+        $fqdn = "$fqdn$path";
+    }
+    expect($fqdn)->toBe('https://test.abc/api');
+});
+
+test('fqdnValueForEnv path handling works correctly', function () {
+    $fqdnValueForEnv = 'test.abc';
+    $path = '/v1/realtime';
+
+    // First append
+    if (! str($fqdnValueForEnv)->endsWith($path)) {
+        $fqdnValueForEnv = "$fqdnValueForEnv$path";
+    }
+    expect($fqdnValueForEnv)->toBe('test.abc/v1/realtime');
+
+    // Second attempt should not duplicate
+    if (! str($fqdnValueForEnv)->endsWith($path)) {
+        $fqdnValueForEnv = "$fqdnValueForEnv$path";
+    }
+    expect($fqdnValueForEnv)->toBe('test.abc/v1/realtime');
+});
+
+test('url path handling works correctly', function () {
+    $url = 'https://test.abc';
+    $path = '/v1/realtime';
+
+    // First append
+    if (! str($url)->endsWith($path)) {
+        $url = "$url$path";
+    }
+    expect($url)->toBe('https://test.abc/v1/realtime');
+
+    // Second attempt should not duplicate
+    if (! str($url)->endsWith($path)) {
+        $url = "$url$path";
+    }
+    expect($url)->toBe('https://test.abc/v1/realtime');
+});