fix: preserve empty strings and remove empty sections in docker-compose
- Preserve empty string environment variables instead of converting to null Empty strings and null have different semantics in Docker Compose: * Empty string (VAR: ""): Variable is set to "" in container (e.g., HTTP_PROXY="" means "no proxy") * Null (VAR: null): Variable is unset/removed from container environment - Remove empty top-level sections (volumes, configs, secrets) from generated compose files These sections now only appear when they contain actual content, following Docker Compose best practices - Add safety check for missing volumes in validateComposeFile to prevent iteration errors - Add comprehensive unit tests for both fixes Fixes #7126
This commit is contained in:
Andras Bacsai
parent
395d225f90
commit
1ab5dbca20
4 changed files with 427 additions and 10 deletions
|
|
@ -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']);
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
|
||||
|
|
|
|||
188
tests/Unit/DockerComposeEmptyStringPreservationTest.php
Normal file
188
tests/Unit/DockerComposeEmptyStringPreservationTest.php
Normal file
|
|
@ -0,0 +1,188 @@
|
|||
<?php
|
||||
|
||||
use Symfony\Component\Yaml\Yaml;
|
||||
|
||||
/**
|
||||
* Unit tests to verify that environment variables with empty strings
|
||||
* in Docker Compose files are preserved as empty strings, not converted to null.
|
||||
*
|
||||
* 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
|
||||
*
|
||||
* See: https://github.com/coollabsio/coolify/issues/7126
|
||||
*/
|
||||
it('ensures parsers.php preserves empty strings in application parser', function () {
|
||||
$parsersFile = file_get_contents(__DIR__.'/../../bootstrap/helpers/parsers.php');
|
||||
|
||||
// Find the applicationParser function's environment mapping logic
|
||||
$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
|
||||
|
||||
// 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();
|
||||
});
|
||||
194
tests/Unit/DockerComposeEmptyTopLevelSectionsTest.php
Normal file
194
tests/Unit/DockerComposeEmptyTopLevelSectionsTest.php
Normal file
|
|
@ -0,0 +1,194 @@
|
|||
<?php
|
||||
|
||||
use Symfony\Component\Yaml\Yaml;
|
||||
|
||||
/**
|
||||
* Unit tests to verify that empty top-level sections (volumes, configs, secrets)
|
||||
* are removed from generated Docker Compose files.
|
||||
*
|
||||
* Empty sections like "volumes: { }" are not valid/clean YAML and should be omitted
|
||||
* when they contain no actual content.
|
||||
*/
|
||||
it('ensures parsers.php filters empty top-level sections', function () {
|
||||
$parsersFile = file_get_contents(__DIR__.'/../../bootstrap/helpers/parsers.php');
|
||||
|
||||
// Check that filtering logic exists
|
||||
expect($parsersFile)
|
||||
->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);
|
||||
});
|
||||
Loading…
Reference in a new issue