From 9a664865ee2f261a3e6a9abc5dfc96c61d8dcdbd Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Sat, 1 Nov 2025 13:13:14 +0100 Subject: [PATCH] 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. --- app/Jobs/ApplicationDeploymentJob.php | 4 +- app/Models/Application.php | 27 +++++++- bootstrap/helpers/api.php | 1 + .../ApplicationConfigurationChangeTest.php | 64 +++++++++++++++++++ .../ApplicationNetworkAliasesSyncTest.php | 50 +++++++++++++++ 5 files changed, 142 insertions(+), 4 deletions(-) create mode 100644 tests/Unit/ApplicationConfigurationChangeTest.php create mode 100644 tests/Unit/ApplicationNetworkAliasesSyncTest.php diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index 971c1d806..f9c181a1c 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -2322,8 +2322,8 @@ private function generate_compose_file() $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' => [ diff --git a/app/Models/Application.php b/app/Models/Application.php index 32459f752..aa04ceea2 100644 --- a/app/Models/Application.php +++ b/app/Models/Application.php @@ -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 @@ public function customNetworkAliases(): Attribute 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 @@ public function isLogDrainEnabled() 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 { diff --git a/bootstrap/helpers/api.php b/bootstrap/helpers/api.php index 5d0f9a2a7..488653fb1 100644 --- a/bootstrap/helpers/api.php +++ b/bootstrap/helpers/api.php @@ -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', diff --git a/tests/Unit/ApplicationConfigurationChangeTest.php b/tests/Unit/ApplicationConfigurationChangeTest.php new file mode 100644 index 000000000..a9763ea34 --- /dev/null +++ b/tests/Unit/ApplicationConfigurationChangeTest.php @@ -0,0 +1,64 @@ +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'); +}); diff --git a/tests/Unit/ApplicationNetworkAliasesSyncTest.php b/tests/Unit/ApplicationNetworkAliasesSyncTest.php new file mode 100644 index 000000000..552ac854c --- /dev/null +++ b/tests/Unit/ApplicationNetworkAliasesSyncTest.php @@ -0,0 +1,50 @@ +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(); +});