Merge remote-tracking branch 'origin/next' into next
This commit is contained in:
Andras Bacsai
commit
fc8f18a534
2 changed files with 95 additions and 16 deletions
|
|
@ -986,15 +986,17 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int
|
|||
continue;
|
||||
}
|
||||
if ($key->value() === $parsedValue->value()) {
|
||||
$value = null;
|
||||
$resource->environment_variables()->firstOrCreate([
|
||||
// Simple variable reference (e.g. DATABASE_URL: ${DATABASE_URL})
|
||||
// Use firstOrCreate to avoid overwriting user-saved values on redeploy
|
||||
$envVar = $resource->environment_variables()->firstOrCreate([
|
||||
'key' => $key,
|
||||
'resourceable_type' => get_class($resource),
|
||||
'resourceable_id' => $resource->id,
|
||||
], [
|
||||
'value' => $value,
|
||||
'is_preview' => false,
|
||||
]);
|
||||
// Add the variable to the environment using the saved DB value
|
||||
$environment[$key->value()] = $envVar->value;
|
||||
} else {
|
||||
if ($value->startsWith('$')) {
|
||||
$isRequired = false;
|
||||
|
|
@ -1074,7 +1076,7 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int
|
|||
} else {
|
||||
// Simple variable reference without default
|
||||
$parsedKeyValue = replaceVariables($value);
|
||||
$resource->environment_variables()->firstOrCreate([
|
||||
$envVar = $resource->environment_variables()->firstOrCreate([
|
||||
'key' => $content,
|
||||
'resourceable_type' => get_class($resource),
|
||||
'resourceable_id' => $resource->id,
|
||||
|
|
@ -1082,8 +1084,8 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int
|
|||
'is_preview' => false,
|
||||
'is_required' => $isRequired,
|
||||
]);
|
||||
// Add the variable to the environment
|
||||
$environment[$content] = $value;
|
||||
// Add the variable to the environment using the saved DB value
|
||||
$environment[$content] = $envVar->value;
|
||||
}
|
||||
} else {
|
||||
// Fallback to old behavior for malformed input (backward compatibility)
|
||||
|
|
@ -1109,7 +1111,7 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int
|
|||
if ($originalValue->value() === $value->value()) {
|
||||
// This means the variable does not have a default value
|
||||
$parsedKeyValue = replaceVariables($value);
|
||||
$resource->environment_variables()->firstOrCreate([
|
||||
$envVar = $resource->environment_variables()->firstOrCreate([
|
||||
'key' => $parsedKeyValue,
|
||||
'resourceable_type' => get_class($resource),
|
||||
'resourceable_id' => $resource->id,
|
||||
|
|
@ -1117,7 +1119,8 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int
|
|||
'is_preview' => false,
|
||||
'is_required' => $isRequired,
|
||||
]);
|
||||
$environment[$parsedKeyValue->value()] = $value;
|
||||
// Add the variable to the environment using the saved DB value
|
||||
$environment[$parsedKeyValue->value()] = $envVar->value;
|
||||
|
||||
continue;
|
||||
}
|
||||
|
|
@ -2325,16 +2328,18 @@ function serviceParser(Service $resource): Collection
|
|||
continue;
|
||||
}
|
||||
if ($key->value() === $parsedValue->value()) {
|
||||
$value = null;
|
||||
$resource->environment_variables()->updateOrCreate([
|
||||
// Simple variable reference (e.g. DATABASE_URL: ${DATABASE_URL})
|
||||
// Use firstOrCreate to avoid overwriting user-saved values on redeploy
|
||||
$envVar = $resource->environment_variables()->firstOrCreate([
|
||||
'key' => $key,
|
||||
'resourceable_type' => get_class($resource),
|
||||
'resourceable_id' => $resource->id,
|
||||
], [
|
||||
'value' => $value,
|
||||
'is_preview' => false,
|
||||
'comment' => $envComments[$originalKey] ?? null,
|
||||
]);
|
||||
// Add the variable to the environment using the saved DB value
|
||||
$environment[$key->value()] = $envVar->value;
|
||||
} else {
|
||||
if ($value->startsWith('$')) {
|
||||
$isRequired = false;
|
||||
|
|
@ -2421,7 +2426,8 @@ function serviceParser(Service $resource): Collection
|
|||
}
|
||||
} else {
|
||||
// Simple variable reference without default
|
||||
$resource->environment_variables()->updateOrCreate([
|
||||
// Use firstOrCreate to avoid overwriting user-saved values on redeploy
|
||||
$envVar = $resource->environment_variables()->firstOrCreate([
|
||||
'key' => $content,
|
||||
'resourceable_type' => get_class($resource),
|
||||
'resourceable_id' => $resource->id,
|
||||
|
|
@ -2430,6 +2436,8 @@ function serviceParser(Service $resource): Collection
|
|||
'is_required' => $isRequired,
|
||||
'comment' => $envComments[$originalKey] ?? null,
|
||||
]);
|
||||
// Add the variable to the environment using the saved DB value
|
||||
$environment[$content] = $envVar->value;
|
||||
}
|
||||
} else {
|
||||
// Fallback to old behavior for malformed input (backward compatibility)
|
||||
|
|
@ -2455,8 +2463,9 @@ function serviceParser(Service $resource): Collection
|
|||
|
||||
if ($originalValue->value() === $value->value()) {
|
||||
// This means the variable does not have a default value
|
||||
// Use firstOrCreate to avoid overwriting user-saved values on redeploy
|
||||
$parsedKeyValue = replaceVariables($value);
|
||||
$resource->environment_variables()->updateOrCreate([
|
||||
$envVar = $resource->environment_variables()->firstOrCreate([
|
||||
'key' => $parsedKeyValue,
|
||||
'resourceable_type' => get_class($resource),
|
||||
'resourceable_id' => $resource->id,
|
||||
|
|
@ -2465,12 +2474,13 @@ function serviceParser(Service $resource): Collection
|
|||
'is_required' => $isRequired,
|
||||
'comment' => $envComments[$originalKey] ?? null,
|
||||
]);
|
||||
// Add the variable to the environment so it will be shown in the deployable compose file
|
||||
$environment[$parsedKeyValue->value()] = $value;
|
||||
// Add the variable to the environment using the saved DB value
|
||||
$environment[$parsedKeyValue->value()] = $envVar->value;
|
||||
|
||||
continue;
|
||||
}
|
||||
$resource->environment_variables()->updateOrCreate([
|
||||
// Variable with a default value from compose — use firstOrCreate to preserve user edits
|
||||
$resource->environment_variables()->firstOrCreate([
|
||||
'key' => $key,
|
||||
'resourceable_type' => get_class($resource),
|
||||
'resourceable_id' => $resource->id,
|
||||
|
|
|
|||
69
tests/Unit/ServiceParserEnvVarPreservationTest.php
Normal file
69
tests/Unit/ServiceParserEnvVarPreservationTest.php
Normal file
|
|
@ -0,0 +1,69 @@
|
|||
<?php
|
||||
|
||||
/**
|
||||
* Unit tests to verify that Docker Compose environment variables
|
||||
* do not overwrite user-saved values on redeploy.
|
||||
*
|
||||
* Regression test for GitHub issue #8885.
|
||||
*/
|
||||
it('uses firstOrCreate for simple variable references in serviceParser to preserve user values', function () {
|
||||
$parsersFile = file_get_contents(__DIR__.'/../../bootstrap/helpers/parsers.php');
|
||||
|
||||
// The serviceParser function should use firstOrCreate (not updateOrCreate)
|
||||
// for simple variable references like DATABASE_URL: ${DATABASE_URL}
|
||||
// This is the key === parsedValue branch
|
||||
expect($parsersFile)->toContain(
|
||||
"// Simple variable reference (e.g. DATABASE_URL: \${DATABASE_URL})\n".
|
||||
" // Use firstOrCreate to avoid overwriting user-saved values on redeploy\n".
|
||||
' $envVar = $resource->environment_variables()->firstOrCreate('
|
||||
);
|
||||
});
|
||||
|
||||
it('does not set value to null for simple variable references in serviceParser', function () {
|
||||
$parsersFile = file_get_contents(__DIR__.'/../../bootstrap/helpers/parsers.php');
|
||||
|
||||
// The old bug: $value = null followed by updateOrCreate with 'value' => $value
|
||||
// This pattern should NOT exist for simple variable references
|
||||
expect($parsersFile)->not->toContain(
|
||||
"\$value = null;\n".
|
||||
' $resource->environment_variables()->updateOrCreate('
|
||||
);
|
||||
});
|
||||
|
||||
it('uses firstOrCreate for simple variable refs without default in serviceParser balanced brace path', function () {
|
||||
$parsersFile = file_get_contents(__DIR__.'/../../bootstrap/helpers/parsers.php');
|
||||
|
||||
// In the balanced brace extraction path, simple variable references without defaults
|
||||
// should use firstOrCreate to preserve user-saved values
|
||||
// This appears twice (applicationParser and serviceParser)
|
||||
$count = substr_count(
|
||||
$parsersFile,
|
||||
"// Simple variable reference without default\n".
|
||||
" // Use firstOrCreate to avoid overwriting user-saved values on redeploy\n".
|
||||
' $envVar = $resource->environment_variables()->firstOrCreate('
|
||||
);
|
||||
|
||||
expect($count)->toBe(1, 'serviceParser should use firstOrCreate for simple variable refs without default');
|
||||
});
|
||||
|
||||
it('populates environment array with saved DB value instead of raw compose variable', function () {
|
||||
$parsersFile = file_get_contents(__DIR__.'/../../bootstrap/helpers/parsers.php');
|
||||
|
||||
// After firstOrCreate, the environment should be populated with the DB value ($envVar->value)
|
||||
// not the raw compose variable reference (e.g., ${DATABASE_URL})
|
||||
// This pattern should appear in both parsers for all variable reference types
|
||||
expect($parsersFile)->toContain('// Add the variable to the environment using the saved DB value');
|
||||
expect($parsersFile)->toContain('$environment[$key->value()] = $envVar->value;');
|
||||
expect($parsersFile)->toContain('$environment[$content] = $envVar->value;');
|
||||
});
|
||||
|
||||
it('does not use updateOrCreate with value null for user-editable environment variables', function () {
|
||||
$parsersFile = file_get_contents(__DIR__.'/../../bootstrap/helpers/parsers.php');
|
||||
|
||||
// The specific bug pattern: setting $value = null then calling updateOrCreate with 'value' => $value
|
||||
// This overwrites user-saved values with null on every deploy
|
||||
expect($parsersFile)->not->toContain(
|
||||
"\$value = null;\n".
|
||||
' $resource->environment_variables()->updateOrCreate('
|
||||
);
|
||||
});
|
||||
Loading…
Reference in a new issue