diff --git a/bootstrap/helpers/docker.php b/bootstrap/helpers/docker.php index d6c9b5bdf..5bccb50f1 100644 --- a/bootstrap/helpers/docker.php +++ b/bootstrap/helpers/docker.php @@ -1073,6 +1073,9 @@ function validateComposeFile(string $compose, int $server_id): string|Throwable } $yaml_compose = Yaml::parse($compose); foreach ($yaml_compose['services'] as $service_name => $service) { + if (! isset($service['volumes'])) { + continue; + } foreach ($service['volumes'] as $volume_name => $volume) { if (data_get($volume, 'type') === 'bind' && data_get($volume, 'content')) { unset($yaml_compose['services'][$service_name]['volumes'][$volume_name]['content']); diff --git a/bootstrap/helpers/parsers.php b/bootstrap/helpers/parsers.php index 01ae50f6b..beb643d7d 100644 --- a/bootstrap/helpers/parsers.php +++ b/bootstrap/helpers/parsers.php @@ -1164,13 +1164,17 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int $environment = $environment->filter(function ($value, $key) { return ! str($key)->startsWith('SERVICE_FQDN_'); })->map(function ($value, $key) use ($resource) { - // if value is empty, set it to null so if you set the environment variable in the .env file (Coolify's UI), it will used + // Preserve empty strings; only override if database value exists and is non-empty + // This is important because empty strings and null have different semantics in Docker: + // - Empty string: Variable is set to "" (e.g., HTTP_PROXY="" means "no proxy") + // - Null: Variable is unset/removed from container environment if (str($value)->isEmpty()) { - if ($resource->environment_variables()->where('key', $key)->exists()) { - $value = $resource->environment_variables()->where('key', $key)->first()->value; - } else { - $value = null; + $dbEnv = $resource->environment_variables()->where('key', $key)->first(); + // Only use database override if it exists AND has a non-empty value + if ($dbEnv && str($dbEnv->value)->isNotEmpty()) { + $value = $dbEnv->value; } + // Keep empty string as-is (don't convert to null) } return $value; @@ -1299,6 +1303,18 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int return array_search($key, $customOrder); }); + // Remove empty top-level sections (volumes, networks, configs, secrets) + // Keep only non-empty sections to match Docker Compose best practices + $topLevel = $topLevel->filter(function ($value, $key) { + // Always keep 'services' section + if ($key === 'services') { + return true; + } + + // Keep section only if it has content + return $value instanceof Collection ? $value->isNotEmpty() : ! empty($value); + }); + $cleanedCompose = Yaml::dump(convertToArray($topLevel), 10, 2); $resource->docker_compose = $cleanedCompose; @@ -2122,13 +2138,17 @@ function serviceParser(Service $resource): Collection $environment = $environment->filter(function ($value, $key) { return ! str($key)->startsWith('SERVICE_FQDN_'); })->map(function ($value, $key) use ($resource) { - // if value is empty, set it to null so if you set the environment variable in the .env file (Coolify's UI), it will used + // Preserve empty strings; only override if database value exists and is non-empty + // This is important because empty strings and null have different semantics in Docker: + // - Empty string: Variable is set to "" (e.g., HTTP_PROXY="" means "no proxy") + // - Null: Variable is unset/removed from container environment if (str($value)->isEmpty()) { - if ($resource->environment_variables()->where('key', $key)->exists()) { - $value = $resource->environment_variables()->where('key', $key)->first()->value; - } else { - $value = null; + $dbEnv = $resource->environment_variables()->where('key', $key)->first(); + // Only use database override if it exists AND has a non-empty value + if ($dbEnv && str($dbEnv->value)->isNotEmpty()) { + $value = $dbEnv->value; } + // Keep empty string as-is (don't convert to null) } return $value; @@ -2251,6 +2271,18 @@ function serviceParser(Service $resource): Collection return array_search($key, $customOrder); }); + // Remove empty top-level sections (volumes, networks, configs, secrets) + // Keep only non-empty sections to match Docker Compose best practices + $topLevel = $topLevel->filter(function ($value, $key) { + // Always keep 'services' section + if ($key === 'services') { + return true; + } + + // Keep section only if it has content + return $value instanceof Collection ? $value->isNotEmpty() : ! empty($value); + }); + $cleanedCompose = Yaml::dump(convertToArray($topLevel), 10, 2); $resource->docker_compose = $cleanedCompose; diff --git a/tests/Unit/DockerComposeEmptyStringPreservationTest.php b/tests/Unit/DockerComposeEmptyStringPreservationTest.php new file mode 100644 index 000000000..71f59ce81 --- /dev/null +++ b/tests/Unit/DockerComposeEmptyStringPreservationTest.php @@ -0,0 +1,188 @@ +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 + + // 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'); +}); + +it('ensures parsers.php preserves empty strings in service parser', function () { + $parsersFile = file_get_contents(__DIR__.'/../../bootstrap/helpers/parsers.php'); + + // Find the serviceParser function's environment mapping logic + $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'); +}); + +it('verifies YAML parsing preserves empty strings correctly', function () { + // Test that Symfony YAML parser handles empty strings as we expect + $yamlWithEmptyString = <<<'YAML' +environment: + HTTP_PROXY: "" + HTTPS_PROXY: '' + NO_PROXY: "localhost" +YAML; + + $parsed = Yaml::parse($yamlWithEmptyString); + + // Empty strings should remain as empty strings, not null + expect($parsed['environment']['HTTP_PROXY'])->toBe(''); + expect($parsed['environment']['HTTPS_PROXY'])->toBe(''); + expect($parsed['environment']['NO_PROXY'])->toBe('localhost'); +}); + +it('verifies YAML parsing handles null values correctly', function () { + // Test that null values are preserved as null + $yamlWithNull = <<<'YAML' +environment: + HTTP_PROXY: null + HTTPS_PROXY: + NO_PROXY: "localhost" +YAML; + + $parsed = Yaml::parse($yamlWithNull); + + // Null should remain null + expect($parsed['environment']['HTTP_PROXY'])->toBeNull(); + expect($parsed['environment']['HTTPS_PROXY'])->toBeNull(); + expect($parsed['environment']['NO_PROXY'])->toBe('localhost'); +}); + +it('verifies YAML serialization preserves empty strings', function () { + // Test that empty strings serialize back correctly + $data = [ + 'environment' => [ + 'HTTP_PROXY' => '', + 'HTTPS_PROXY' => '', + 'NO_PROXY' => 'localhost', + ], + ]; + + $yaml = Yaml::dump($data, 10, 2); + + // Empty strings should be serialized with quotes + expect($yaml)->toContain("HTTP_PROXY: ''"); + expect($yaml)->toContain("HTTPS_PROXY: ''"); + expect($yaml)->toContain('NO_PROXY: localhost'); + + // Should NOT contain "null" + expect($yaml)->not->toContain('HTTP_PROXY: null'); +}); + +it('verifies YAML serialization handles null values', function () { + // Test that null values serialize as null + $data = [ + 'environment' => [ + 'HTTP_PROXY' => null, + 'HTTPS_PROXY' => null, + 'NO_PROXY' => 'localhost', + ], + ]; + + $yaml = Yaml::dump($data, 10, 2); + + // Null should be serialized as "null" + expect($yaml)->toContain('HTTP_PROXY: null'); + expect($yaml)->toContain('HTTPS_PROXY: null'); + expect($yaml)->toContain('NO_PROXY: localhost'); + + // Should NOT contain empty quotes for null values + expect($yaml)->not->toContain("HTTP_PROXY: ''"); +}); + +it('verifies empty string round-trip through YAML', function () { + // Test full round-trip: empty string -> YAML -> parse -> serialize -> parse + $original = [ + 'environment' => [ + 'HTTP_PROXY' => '', + 'NO_PROXY' => 'localhost', + ], + ]; + + // Serialize to YAML + $yaml1 = Yaml::dump($original, 10, 2); + + // Parse back + $parsed1 = Yaml::parse($yaml1); + + // Verify empty string is preserved + expect($parsed1['environment']['HTTP_PROXY'])->toBe(''); + expect($parsed1['environment']['NO_PROXY'])->toBe('localhost'); + + // Serialize again + $yaml2 = Yaml::dump($parsed1, 10, 2); + + // Parse again + $parsed2 = Yaml::parse($yaml2); + + // Should still be empty string, not null + expect($parsed2['environment']['HTTP_PROXY'])->toBe(''); + expect($parsed2['environment']['NO_PROXY'])->toBe('localhost'); + + // Both YAML representations should be equivalent + expect($yaml1)->toBe($yaml2); +}); + +it('verifies str()->isEmpty() behavior with empty strings and null', function () { + // Test Laravel's str()->isEmpty() helper behavior + + // Empty string should be considered empty + expect(str('')->isEmpty())->toBeTrue(); + + // Null should be considered empty + expect(str(null)->isEmpty())->toBeTrue(); + + // String with content should not be empty + expect(str('value')->isEmpty())->toBeFalse(); + + // This confirms that we need additional logic to distinguish + // between empty string ('') and null, since both are "isEmpty" +}); + +it('verifies the distinction between empty string and null in PHP', function () { + // Document PHP's behavior for empty strings vs null + + $emptyString = ''; + $nullValue = null; + + // They are different values + expect($emptyString === $nullValue)->toBeFalse(); + + // Empty string is not null + expect($emptyString === '')->toBeTrue(); + expect($nullValue === null)->toBeTrue(); + + // isset() treats them differently + $arrayWithEmpty = ['key' => '']; + $arrayWithNull = ['key' => null]; + + expect(isset($arrayWithEmpty['key']))->toBeTrue(); + expect(isset($arrayWithNull['key']))->toBeFalse(); +}); diff --git a/tests/Unit/DockerComposeEmptyTopLevelSectionsTest.php b/tests/Unit/DockerComposeEmptyTopLevelSectionsTest.php new file mode 100644 index 000000000..bfd674053 --- /dev/null +++ b/tests/Unit/DockerComposeEmptyTopLevelSectionsTest.php @@ -0,0 +1,194 @@ +toContain('Remove empty top-level sections') + ->toContain('->filter(function ($value, $key)'); +}); + +it('verifies YAML dump produces empty objects for empty arrays', function () { + // Demonstrate the problem: empty arrays serialize as empty objects + $data = [ + 'services' => ['web' => ['image' => 'nginx']], + 'volumes' => [], + 'configs' => [], + 'secrets' => [], + ]; + + $yaml = Yaml::dump($data, 10, 2); + + // Empty arrays become empty objects in YAML + expect($yaml)->toContain('volumes: { }'); + expect($yaml)->toContain('configs: { }'); + expect($yaml)->toContain('secrets: { }'); +}); + +it('verifies YAML dump omits keys that are not present', function () { + // Demonstrate the solution: omit empty keys entirely + $data = [ + 'services' => ['web' => ['image' => 'nginx']], + // Don't include volumes, configs, secrets at all + ]; + + $yaml = Yaml::dump($data, 10, 2); + + // Keys that don't exist are not in the output + expect($yaml)->not->toContain('volumes:'); + expect($yaml)->not->toContain('configs:'); + expect($yaml)->not->toContain('secrets:'); + expect($yaml)->toContain('services:'); +}); + +it('verifies collection filter removes empty items', function () { + // Test Laravel Collection filter behavior + $collection = collect([ + 'services' => collect(['web' => ['image' => 'nginx']]), + 'volumes' => collect([]), + 'networks' => collect(['coolify' => ['external' => true]]), + 'configs' => collect([]), + 'secrets' => collect([]), + ]); + + $filtered = $collection->filter(function ($value, $key) { + // Always keep services + if ($key === 'services') { + return true; + } + + // Keep only non-empty collections + return $value->isNotEmpty(); + }); + + // Should have services and networks (non-empty) + expect($filtered)->toHaveKey('services'); + expect($filtered)->toHaveKey('networks'); + + // Should NOT have volumes, configs, secrets (empty) + expect($filtered)->not->toHaveKey('volumes'); + expect($filtered)->not->toHaveKey('configs'); + expect($filtered)->not->toHaveKey('secrets'); +}); + +it('verifies filtered collections serialize cleanly to YAML', function () { + // Full test: filter then serialize + $collection = collect([ + 'services' => collect(['web' => ['image' => 'nginx']]), + 'volumes' => collect([]), + 'networks' => collect(['coolify' => ['external' => true]]), + 'configs' => collect([]), + 'secrets' => collect([]), + ]); + + $filtered = $collection->filter(function ($value, $key) { + if ($key === 'services') { + return true; + } + + return $value->isNotEmpty(); + }); + + $yaml = Yaml::dump($filtered->toArray(), 10, 2); + + // Should have services and networks + expect($yaml)->toContain('services:'); + expect($yaml)->toContain('networks:'); + + // Should NOT have empty sections + expect($yaml)->not->toContain('volumes:'); + expect($yaml)->not->toContain('configs:'); + expect($yaml)->not->toContain('secrets:'); +}); + +it('ensures services section is always kept even if empty', function () { + // Services should never be filtered out + $collection = collect([ + 'services' => collect([]), + 'volumes' => collect([]), + ]); + + $filtered = $collection->filter(function ($value, $key) { + if ($key === 'services') { + return true; // Always keep + } + + return $value->isNotEmpty(); + }); + + // Services should be present + expect($filtered)->toHaveKey('services'); + + // Volumes should be removed + expect($filtered)->not->toHaveKey('volumes'); +}); + +it('verifies non-empty sections are preserved', function () { + // Non-empty sections should remain + $collection = collect([ + 'services' => collect(['web' => ['image' => 'nginx']]), + 'volumes' => collect(['data' => ['driver' => 'local']]), + 'networks' => collect(['coolify' => ['external' => true]]), + 'configs' => collect(['app_config' => ['file' => './config']]), + 'secrets' => collect(['db_password' => ['file' => './secret']]), + ]); + + $filtered = $collection->filter(function ($value, $key) { + if ($key === 'services') { + return true; + } + + return $value->isNotEmpty(); + }); + + // All sections should be present (none are empty) + expect($filtered)->toHaveKey('services'); + expect($filtered)->toHaveKey('volumes'); + expect($filtered)->toHaveKey('networks'); + expect($filtered)->toHaveKey('configs'); + expect($filtered)->toHaveKey('secrets'); + + // Count should be 5 (all original keys) + expect($filtered->count())->toBe(5); +}); + +it('verifies mixed empty and non-empty sections', function () { + // Mixed scenario: some empty, some not + $collection = collect([ + 'services' => collect(['web' => ['image' => 'nginx']]), + 'volumes' => collect([]), // Empty + 'networks' => collect(['coolify' => ['external' => true]]), // Not empty + 'configs' => collect([]), // Empty + 'secrets' => collect(['db_password' => ['file' => './secret']]), // Not empty + ]); + + $filtered = $collection->filter(function ($value, $key) { + if ($key === 'services') { + return true; + } + + return $value->isNotEmpty(); + }); + + // Should have: services, networks, secrets + expect($filtered)->toHaveKey('services'); + expect($filtered)->toHaveKey('networks'); + expect($filtered)->toHaveKey('secrets'); + + // Should NOT have: volumes, configs + expect($filtered)->not->toHaveKey('volumes'); + expect($filtered)->not->toHaveKey('configs'); + + // Count should be 3 + expect($filtered->count())->toBe(3); +});