diff --git a/bootstrap/helpers/parsers.php b/bootstrap/helpers/parsers.php index 99ce9185a..85ae5ad3e 100644 --- a/bootstrap/helpers/parsers.php +++ b/bootstrap/helpers/parsers.php @@ -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, diff --git a/tests/Unit/ServiceParserEnvVarPreservationTest.php b/tests/Unit/ServiceParserEnvVarPreservationTest.php new file mode 100644 index 000000000..3f56447dc --- /dev/null +++ b/tests/Unit/ServiceParserEnvVarPreservationTest.php @@ -0,0 +1,69 @@ +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(' + ); +});