mirror of
https://github.com/coollabsio/coolify.git
synced 2025-12-28 13:41:51 +00:00
refactor: Improve handling of custom network aliases
The custom_network_aliases attribute in the Application model was being cast to an array directly. This commit refactors the attribute to provide both a string representation (for compatibility with older configurations and hashing) and an array representation for internal use. This ensures that network aliases are correctly parsed and utilized, preventing potential issues during deployment and configuration updates.
This commit is contained in:
parent
c34e5c803b
commit
9a664865ee
@ -2322,8 +2322,8 @@ class ApplicationDeploymentJob implements ShouldBeEncrypted, ShouldQueue
|
||||
$this->application->parseHealthcheckFromDockerfile($this->saved_outputs->get('dockerfile_from_repo'));
|
||||
}
|
||||
$custom_network_aliases = [];
|
||||
if (is_array($this->application->custom_network_aliases) && count($this->application->custom_network_aliases) > 0) {
|
||||
$custom_network_aliases = $this->application->custom_network_aliases;
|
||||
if (is_array($this->application->custom_network_aliases_array) && count($this->application->custom_network_aliases_array) > 0) {
|
||||
$custom_network_aliases = $this->application->custom_network_aliases_array;
|
||||
}
|
||||
$docker_compose = [
|
||||
'services' => [
|
||||
|
||||
@ -120,7 +120,6 @@ class Application extends BaseModel
|
||||
protected $appends = ['server_status'];
|
||||
|
||||
protected $casts = [
|
||||
'custom_network_aliases' => 'array',
|
||||
'http_basic_auth_password' => 'encrypted',
|
||||
];
|
||||
|
||||
@ -253,6 +252,30 @@ class Application extends BaseModel
|
||||
return null;
|
||||
}
|
||||
|
||||
if (is_string($value) && $this->isJson($value)) {
|
||||
$decoded = json_decode($value, true);
|
||||
|
||||
// Return as comma-separated string, not array
|
||||
return is_array($decoded) ? implode(',', $decoded) : $value;
|
||||
}
|
||||
|
||||
return $value;
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Get custom_network_aliases as an array
|
||||
*/
|
||||
public function customNetworkAliasesArray(): Attribute
|
||||
{
|
||||
return Attribute::make(
|
||||
get: function () {
|
||||
$value = $this->getRawOriginal('custom_network_aliases');
|
||||
if (is_null($value)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
if (is_string($value) && $this->isJson($value)) {
|
||||
return json_decode($value, true);
|
||||
}
|
||||
@ -957,7 +980,7 @@ class Application extends BaseModel
|
||||
|
||||
public function isConfigurationChanged(bool $save = false)
|
||||
{
|
||||
$newConfigHash = base64_encode($this->fqdn.$this->git_repository.$this->git_branch.$this->git_commit_sha.$this->build_pack.$this->static_image.$this->install_command.$this->build_command.$this->start_command.$this->ports_exposes.$this->ports_mappings.$this->base_directory.$this->publish_directory.$this->dockerfile.$this->dockerfile_location.$this->custom_labels.$this->custom_docker_run_options.$this->dockerfile_target_build.$this->redirect.$this->custom_nginx_configuration.$this->custom_labels.$this->settings->use_build_secrets);
|
||||
$newConfigHash = base64_encode($this->fqdn.$this->git_repository.$this->git_branch.$this->git_commit_sha.$this->build_pack.$this->static_image.$this->install_command.$this->build_command.$this->start_command.$this->ports_exposes.$this->ports_mappings.$this->custom_network_aliases.$this->base_directory.$this->publish_directory.$this->dockerfile.$this->dockerfile_location.$this->custom_labels.$this->custom_docker_run_options.$this->dockerfile_target_build.$this->redirect.$this->custom_nginx_configuration.$this->custom_labels.$this->settings->use_build_secrets);
|
||||
if ($this->pull_request_id === 0 || $this->pull_request_id === null) {
|
||||
$newConfigHash .= json_encode($this->environment_variables()->get(['value', 'is_multiline', 'is_literal', 'is_buildtime', 'is_runtime'])->sort());
|
||||
} else {
|
||||
|
||||
@ -97,6 +97,7 @@ function sharedDataApplications()
|
||||
'start_command' => 'string|nullable',
|
||||
'ports_exposes' => 'string|regex:/^(\d+)(,\d+)*$/',
|
||||
'ports_mappings' => 'string|regex:/^(\d+:\d+)(,\d+:\d+)*$/|nullable',
|
||||
'custom_network_aliases' => 'string|nullable',
|
||||
'base_directory' => 'string|nullable',
|
||||
'publish_directory' => 'string|nullable',
|
||||
'health_check_enabled' => 'boolean',
|
||||
|
||||
64
tests/Unit/ApplicationConfigurationChangeTest.php
Normal file
64
tests/Unit/ApplicationConfigurationChangeTest.php
Normal file
@ -0,0 +1,64 @@
|
||||
<?php
|
||||
|
||||
use App\Models\Application;
|
||||
|
||||
/**
|
||||
* Unit test to verify that custom_network_aliases is included in configuration change detection.
|
||||
* These tests verify the hash calculation includes the field by checking the behavior.
|
||||
*/
|
||||
it('custom_network_aliases affects configuration hash', function () {
|
||||
// Test helper to calculate hash like isConfigurationChanged does
|
||||
$calculateHash = function ($customNetworkAliases) {
|
||||
return md5(base64_encode(
|
||||
'example.com'. // fqdn
|
||||
'https://github.com/example/repo'. // git_repository
|
||||
'main'. // git_branch
|
||||
'abc123'. // git_commit_sha
|
||||
'nixpacks'. // build_pack
|
||||
null. // static_image
|
||||
'npm install'. // install_command
|
||||
'npm run build'. // build_command
|
||||
'npm start'. // start_command
|
||||
'3000'. // ports_exposes
|
||||
null. // ports_mappings
|
||||
$customNetworkAliases. // custom_network_aliases (THIS IS THE KEY LINE)
|
||||
'/'. // base_directory
|
||||
null. // publish_directory
|
||||
null. // dockerfile
|
||||
'Dockerfile'. // dockerfile_location
|
||||
null. // custom_labels
|
||||
null. // custom_docker_run_options
|
||||
null. // dockerfile_target_build
|
||||
null. // redirect
|
||||
null. // custom_nginx_configuration
|
||||
null. // custom_labels (duplicate)
|
||||
false // use_build_secrets
|
||||
));
|
||||
};
|
||||
|
||||
// Different custom_network_aliases should produce different hashes
|
||||
$hash1 = $calculateHash('api.internal,api.local');
|
||||
$hash2 = $calculateHash('api.internal,api.local,api.staging');
|
||||
$hash3 = $calculateHash(null);
|
||||
|
||||
expect($hash1)->not->toBe($hash2)
|
||||
->and($hash1)->not->toBe($hash3)
|
||||
->and($hash2)->not->toBe($hash3);
|
||||
});
|
||||
|
||||
it('custom_network_aliases is in the configuration hash fields', function () {
|
||||
// This test verifies the field is in the isConfigurationChanged method by reading the source
|
||||
$reflection = new ReflectionClass(Application::class);
|
||||
$method = $reflection->getMethod('isConfigurationChanged');
|
||||
$source = file_get_contents($method->getFileName());
|
||||
|
||||
// Extract the method source
|
||||
$lines = explode("\n", $source);
|
||||
$methodStartLine = $method->getStartLine() - 1;
|
||||
$methodEndLine = $method->getEndLine();
|
||||
$methodSource = implode("\n", array_slice($lines, $methodStartLine, $methodEndLine - $methodStartLine));
|
||||
|
||||
// Verify custom_network_aliases is in the hash calculation
|
||||
expect($methodSource)->toContain('$this->custom_network_aliases')
|
||||
->and($methodSource)->toContain('ports_mappings');
|
||||
});
|
||||
50
tests/Unit/ApplicationNetworkAliasesSyncTest.php
Normal file
50
tests/Unit/ApplicationNetworkAliasesSyncTest.php
Normal file
@ -0,0 +1,50 @@
|
||||
<?php
|
||||
|
||||
use App\Models\Application;
|
||||
|
||||
/**
|
||||
* Unit test to verify custom_network_aliases conversion from array to string.
|
||||
*
|
||||
* The issue: Application model's accessor returns an array, but the Livewire
|
||||
* component property is typed as ?string for the text input field.
|
||||
* The conversion happens in mount() after syncFromModel().
|
||||
*/
|
||||
it('converts array aliases to comma-separated string', function () {
|
||||
// Test that an array is correctly converted to a string
|
||||
$aliases = ['api.internal', 'api.local'];
|
||||
$result = implode(',', $aliases);
|
||||
|
||||
expect($result)->toBe('api.internal,api.local')
|
||||
->and($result)->toBeString();
|
||||
});
|
||||
|
||||
it('handles null aliases', function () {
|
||||
// Test that null remains null
|
||||
$aliases = null;
|
||||
|
||||
if (is_array($aliases)) {
|
||||
$result = implode(',', $aliases);
|
||||
} else {
|
||||
$result = $aliases;
|
||||
}
|
||||
|
||||
expect($result)->toBeNull();
|
||||
});
|
||||
|
||||
it('handles empty array aliases', function () {
|
||||
// Test that empty array becomes empty string
|
||||
$aliases = [];
|
||||
$result = implode(',', $aliases);
|
||||
|
||||
expect($result)->toBe('')
|
||||
->and($result)->toBeString();
|
||||
});
|
||||
|
||||
it('handles single alias', function () {
|
||||
// Test that single-element array is converted correctly
|
||||
$aliases = ['api.internal'];
|
||||
$result = implode(',', $aliases);
|
||||
|
||||
expect($result)->toBe('api.internal')
|
||||
->and($result)->toBeString();
|
||||
});
|
||||
Loading…
Reference in New Issue
Block a user