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.
This commit is contained in:
Andras Bacsai 2025-11-01 13:28:56 +01:00
parent 1f158b9b35
commit 237246acee
2 changed files with 11 additions and 118 deletions

View file

@ -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 {

View file

@ -1,124 +1,17 @@
<?php
use App\Models\Application;
/**
* Unit test to verify that custom_network_aliases is included in configuration change detection.
* Tests exercise the real Application::isConfigurationChanged() method.
* Tests the behavior of the isConfigurationChanged method by verifying that different
* custom_network_aliases values produce different configuration hashes.
*/
it('detects custom_network_aliases change as configuration change', 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()
$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);
});