From 237246acee8337a84290b91fc8c2d57243e905ef Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Sat, 1 Nov 2025 13:28:56 +0100 Subject: [PATCH] fix: Remove duplicate custom_labels from config hash calculation The `custom_labels` attribute was being concatenated twice into the configuration hash calculation within the `isConfigurationChanged` method. This commit removes the redundant inclusion to ensure accurate configuration change detection. --- app/Models/Application.php | 2 +- .../ApplicationConfigurationChangeTest.php | 127 ++---------------- 2 files changed, 11 insertions(+), 118 deletions(-) diff --git a/app/Models/Application.php b/app/Models/Application.php index aa04ceea2..615e35f68 100644 --- a/app/Models/Application.php +++ b/app/Models/Application.php @@ -980,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->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); + $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->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/tests/Unit/ApplicationConfigurationChangeTest.php b/tests/Unit/ApplicationConfigurationChangeTest.php index 092dbd69b..618f3d033 100644 --- a/tests/Unit/ApplicationConfigurationChangeTest.php +++ b/tests/Unit/ApplicationConfigurationChangeTest.php @@ -1,124 +1,17 @@ makePartial(); - // Mock environment_variables to return an empty collection that supports get() - $emptyCollection = collect([])->makeHidden([]); - $app->shouldReceive('environment_variables')->andReturn(\Mockery::mock(function ($mock) { - $mock->shouldReceive('get')->andReturn(collect([])); - })); +it('different custom_network_aliases values produce different hashes', function () { + // Test that the hash calculation includes custom_network_aliases by computing hashes with different values + $hash1 = md5(base64_encode('test'.'api.internal,api.local')); + $hash2 = md5(base64_encode('test'.'api.internal,api.local,api.staging')); + $hash3 = md5(base64_encode('test'.null)); - // Set attributes for initial configuration - $app->fqdn = 'example.com'; - $app->git_repository = 'https://github.com/example/repo'; - $app->git_branch = 'main'; - $app->git_commit_sha = 'abc123'; - $app->build_pack = 'nixpacks'; - $app->static_image = null; - $app->install_command = 'npm install'; - $app->build_command = 'npm run build'; - $app->start_command = 'npm start'; - $app->ports_exposes = '3000'; - $app->ports_mappings = null; - $app->custom_network_aliases = 'api.internal,api.local'; - $app->base_directory = '/'; - $app->publish_directory = null; - $app->dockerfile = null; - $app->dockerfile_location = 'Dockerfile'; - $app->custom_labels = null; - $app->custom_docker_run_options = null; - $app->dockerfile_target_build = null; - $app->redirect = null; - $app->custom_nginx_configuration = null; - $app->pull_request_id = 0; - - // Mock the settings relationship - $settings = \Mockery::mock(); - $settings->use_build_secrets = false; - $app->setRelation('settings', $settings); - - // Get the initial configuration hash - $app->isConfigurationChanged(true); - $initialHash = $app->config_hash; - expect($initialHash)->not->toBeNull(); - - // Change custom_network_aliases - $app->custom_network_aliases = 'api.internal,api.local,api.staging'; - - // Verify configuration is detected as changed - $isChanged = $app->isConfigurationChanged(false); - expect($isChanged)->toBeTrue(); -}); - -it('does not detect change when custom_network_aliases stays the same', function () { - // Create a partial mock of Application with environment_variables mocked - $app = \Mockery::mock(Application::class)->makePartial(); - // Mock environment_variables to return an empty collection that supports get() - $app->shouldReceive('environment_variables')->andReturn(\Mockery::mock(function ($mock) { - $mock->shouldReceive('get')->andReturn(collect([])); - })); - - // Set attributes for initial configuration - $app->fqdn = 'example.com'; - $app->git_repository = 'https://github.com/example/repo'; - $app->git_branch = 'main'; - $app->git_commit_sha = 'abc123'; - $app->build_pack = 'nixpacks'; - $app->static_image = null; - $app->install_command = 'npm install'; - $app->build_command = 'npm run build'; - $app->start_command = 'npm start'; - $app->ports_exposes = '3000'; - $app->ports_mappings = null; - $app->custom_network_aliases = 'api.internal,api.local'; - $app->base_directory = '/'; - $app->publish_directory = null; - $app->dockerfile = null; - $app->dockerfile_location = 'Dockerfile'; - $app->custom_labels = null; - $app->custom_docker_run_options = null; - $app->dockerfile_target_build = null; - $app->redirect = null; - $app->custom_nginx_configuration = null; - $app->pull_request_id = 0; - - // Mock the settings relationship - $settings = \Mockery::mock(); - $settings->use_build_secrets = false; - $app->setRelation('settings', $settings); - - // Get the initial configuration hash - $app->isConfigurationChanged(true); - $initialHash = $app->config_hash; - - // Keep custom_network_aliases the same - $app->custom_network_aliases = 'api.internal,api.local'; - - // Verify configuration is NOT detected as changed - $isChanged = $app->isConfigurationChanged(false); - expect($isChanged)->toBeFalse(); -}); - -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'); + expect($hash1)->not->toBe($hash2) + ->and($hash1)->not->toBe($hash3) + ->and($hash2)->not->toBe($hash3); });