fix(parser): preserve user-saved env vars on Docker Compose redeploy (#8894)
This commit is contained in:
commit
550db87724
2 changed files with 95 additions and 16 deletions
|
|
@ -986,15 +986,17 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
if ($key->value() === $parsedValue->value()) {
|
if ($key->value() === $parsedValue->value()) {
|
||||||
$value = null;
|
// Simple variable reference (e.g. DATABASE_URL: ${DATABASE_URL})
|
||||||
$resource->environment_variables()->firstOrCreate([
|
// Use firstOrCreate to avoid overwriting user-saved values on redeploy
|
||||||
|
$envVar = $resource->environment_variables()->firstOrCreate([
|
||||||
'key' => $key,
|
'key' => $key,
|
||||||
'resourceable_type' => get_class($resource),
|
'resourceable_type' => get_class($resource),
|
||||||
'resourceable_id' => $resource->id,
|
'resourceable_id' => $resource->id,
|
||||||
], [
|
], [
|
||||||
'value' => $value,
|
|
||||||
'is_preview' => false,
|
'is_preview' => false,
|
||||||
]);
|
]);
|
||||||
|
// Add the variable to the environment using the saved DB value
|
||||||
|
$environment[$key->value()] = $envVar->value;
|
||||||
} else {
|
} else {
|
||||||
if ($value->startsWith('$')) {
|
if ($value->startsWith('$')) {
|
||||||
$isRequired = false;
|
$isRequired = false;
|
||||||
|
|
@ -1074,7 +1076,7 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int
|
||||||
} else {
|
} else {
|
||||||
// Simple variable reference without default
|
// Simple variable reference without default
|
||||||
$parsedKeyValue = replaceVariables($value);
|
$parsedKeyValue = replaceVariables($value);
|
||||||
$resource->environment_variables()->firstOrCreate([
|
$envVar = $resource->environment_variables()->firstOrCreate([
|
||||||
'key' => $content,
|
'key' => $content,
|
||||||
'resourceable_type' => get_class($resource),
|
'resourceable_type' => get_class($resource),
|
||||||
'resourceable_id' => $resource->id,
|
'resourceable_id' => $resource->id,
|
||||||
|
|
@ -1082,8 +1084,8 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int
|
||||||
'is_preview' => false,
|
'is_preview' => false,
|
||||||
'is_required' => $isRequired,
|
'is_required' => $isRequired,
|
||||||
]);
|
]);
|
||||||
// Add the variable to the environment
|
// Add the variable to the environment using the saved DB value
|
||||||
$environment[$content] = $value;
|
$environment[$content] = $envVar->value;
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
// Fallback to old behavior for malformed input (backward compatibility)
|
// 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()) {
|
if ($originalValue->value() === $value->value()) {
|
||||||
// This means the variable does not have a default value
|
// This means the variable does not have a default value
|
||||||
$parsedKeyValue = replaceVariables($value);
|
$parsedKeyValue = replaceVariables($value);
|
||||||
$resource->environment_variables()->firstOrCreate([
|
$envVar = $resource->environment_variables()->firstOrCreate([
|
||||||
'key' => $parsedKeyValue,
|
'key' => $parsedKeyValue,
|
||||||
'resourceable_type' => get_class($resource),
|
'resourceable_type' => get_class($resource),
|
||||||
'resourceable_id' => $resource->id,
|
'resourceable_id' => $resource->id,
|
||||||
|
|
@ -1117,7 +1119,8 @@ function applicationParser(Application $resource, int $pull_request_id = 0, ?int
|
||||||
'is_preview' => false,
|
'is_preview' => false,
|
||||||
'is_required' => $isRequired,
|
'is_required' => $isRequired,
|
||||||
]);
|
]);
|
||||||
$environment[$parsedKeyValue->value()] = $value;
|
// Add the variable to the environment using the saved DB value
|
||||||
|
$environment[$parsedKeyValue->value()] = $envVar->value;
|
||||||
|
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
@ -2325,16 +2328,18 @@ function serviceParser(Service $resource): Collection
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
if ($key->value() === $parsedValue->value()) {
|
if ($key->value() === $parsedValue->value()) {
|
||||||
$value = null;
|
// Simple variable reference (e.g. DATABASE_URL: ${DATABASE_URL})
|
||||||
$resource->environment_variables()->updateOrCreate([
|
// Use firstOrCreate to avoid overwriting user-saved values on redeploy
|
||||||
|
$envVar = $resource->environment_variables()->firstOrCreate([
|
||||||
'key' => $key,
|
'key' => $key,
|
||||||
'resourceable_type' => get_class($resource),
|
'resourceable_type' => get_class($resource),
|
||||||
'resourceable_id' => $resource->id,
|
'resourceable_id' => $resource->id,
|
||||||
], [
|
], [
|
||||||
'value' => $value,
|
|
||||||
'is_preview' => false,
|
'is_preview' => false,
|
||||||
'comment' => $envComments[$originalKey] ?? null,
|
'comment' => $envComments[$originalKey] ?? null,
|
||||||
]);
|
]);
|
||||||
|
// Add the variable to the environment using the saved DB value
|
||||||
|
$environment[$key->value()] = $envVar->value;
|
||||||
} else {
|
} else {
|
||||||
if ($value->startsWith('$')) {
|
if ($value->startsWith('$')) {
|
||||||
$isRequired = false;
|
$isRequired = false;
|
||||||
|
|
@ -2421,7 +2426,8 @@ function serviceParser(Service $resource): Collection
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
// Simple variable reference without default
|
// 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,
|
'key' => $content,
|
||||||
'resourceable_type' => get_class($resource),
|
'resourceable_type' => get_class($resource),
|
||||||
'resourceable_id' => $resource->id,
|
'resourceable_id' => $resource->id,
|
||||||
|
|
@ -2430,6 +2436,8 @@ function serviceParser(Service $resource): Collection
|
||||||
'is_required' => $isRequired,
|
'is_required' => $isRequired,
|
||||||
'comment' => $envComments[$originalKey] ?? null,
|
'comment' => $envComments[$originalKey] ?? null,
|
||||||
]);
|
]);
|
||||||
|
// Add the variable to the environment using the saved DB value
|
||||||
|
$environment[$content] = $envVar->value;
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
// Fallback to old behavior for malformed input (backward compatibility)
|
// Fallback to old behavior for malformed input (backward compatibility)
|
||||||
|
|
@ -2455,8 +2463,9 @@ function serviceParser(Service $resource): Collection
|
||||||
|
|
||||||
if ($originalValue->value() === $value->value()) {
|
if ($originalValue->value() === $value->value()) {
|
||||||
// This means the variable does not have a default value
|
// This means the variable does not have a default value
|
||||||
|
// Use firstOrCreate to avoid overwriting user-saved values on redeploy
|
||||||
$parsedKeyValue = replaceVariables($value);
|
$parsedKeyValue = replaceVariables($value);
|
||||||
$resource->environment_variables()->updateOrCreate([
|
$envVar = $resource->environment_variables()->firstOrCreate([
|
||||||
'key' => $parsedKeyValue,
|
'key' => $parsedKeyValue,
|
||||||
'resourceable_type' => get_class($resource),
|
'resourceable_type' => get_class($resource),
|
||||||
'resourceable_id' => $resource->id,
|
'resourceable_id' => $resource->id,
|
||||||
|
|
@ -2465,12 +2474,13 @@ function serviceParser(Service $resource): Collection
|
||||||
'is_required' => $isRequired,
|
'is_required' => $isRequired,
|
||||||
'comment' => $envComments[$originalKey] ?? null,
|
'comment' => $envComments[$originalKey] ?? null,
|
||||||
]);
|
]);
|
||||||
// Add the variable to the environment so it will be shown in the deployable compose file
|
// Add the variable to the environment using the saved DB value
|
||||||
$environment[$parsedKeyValue->value()] = $value;
|
$environment[$parsedKeyValue->value()] = $envVar->value;
|
||||||
|
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
$resource->environment_variables()->updateOrCreate([
|
// Variable with a default value from compose — use firstOrCreate to preserve user edits
|
||||||
|
$resource->environment_variables()->firstOrCreate([
|
||||||
'key' => $key,
|
'key' => $key,
|
||||||
'resourceable_type' => get_class($resource),
|
'resourceable_type' => get_class($resource),
|
||||||
'resourceable_id' => $resource->id,
|
'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