mirror of
https://github.com/coollabsio/coolify.git
synced 2025-12-28 13:41:51 +00:00
refactor: move buildpack cleanup logic to model lifecycle hooks
Move buildpack switching cleanup from Livewire component to Application model's boot lifecycle. This improves separation of concerns and ensures cleanup happens consistently regardless of how the buildpack change is triggered. Also clears Dockerfile-specific data when switching away from dockerfile buildpack. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
parent
fb19364a55
commit
36f8a58c28
@ -622,6 +622,7 @@ class General extends Component
|
||||
|
||||
public function updatedBuildPack()
|
||||
{
|
||||
$originalBuildPack = $this->application->getOriginal('build_pack');
|
||||
// Check if user has permission to update
|
||||
try {
|
||||
$this->authorize('update', $this->application);
|
||||
@ -641,8 +642,6 @@ class General extends Component
|
||||
$this->application->settings->is_static = false;
|
||||
$this->application->settings->save();
|
||||
} else {
|
||||
$this->portsExposes = '3000';
|
||||
$this->application->ports_exposes = '3000';
|
||||
$this->resetDefaultLabels(false);
|
||||
}
|
||||
if ($this->buildPack === 'dockercompose') {
|
||||
@ -655,18 +654,6 @@ class General extends Component
|
||||
} catch (\Illuminate\Auth\Access\AuthorizationException $e) {
|
||||
// User doesn't have update permission, just continue without saving
|
||||
}
|
||||
} else {
|
||||
// Clear Docker Compose specific data when switching away from dockercompose
|
||||
if ($this->application->getOriginal('build_pack') === 'dockercompose') {
|
||||
$this->application->docker_compose_domains = null;
|
||||
$this->application->docker_compose_raw = null;
|
||||
|
||||
// Remove SERVICE_FQDN_* and SERVICE_URL_* environment variables
|
||||
$this->application->environment_variables()->where('key', 'LIKE', 'SERVICE_FQDN_%')->delete();
|
||||
$this->application->environment_variables()->where('key', 'LIKE', 'SERVICE_URL_%')->delete();
|
||||
$this->application->environment_variables_preview()->where('key', 'LIKE', 'SERVICE_FQDN_%')->delete();
|
||||
$this->application->environment_variables_preview()->where('key', 'LIKE', 'SERVICE_URL_%')->delete();
|
||||
}
|
||||
}
|
||||
if ($this->buildPack === 'static') {
|
||||
$this->portsExposes = '80';
|
||||
|
||||
@ -176,6 +176,36 @@ class Application extends BaseModel
|
||||
if (count($payload) > 0) {
|
||||
$application->forceFill($payload);
|
||||
}
|
||||
|
||||
// Buildpack switching cleanup logic
|
||||
if ($application->isDirty('build_pack')) {
|
||||
$originalBuildPack = $application->getOriginal('build_pack');
|
||||
|
||||
// Clear Docker Compose specific data when switching away from dockercompose
|
||||
if ($originalBuildPack === 'dockercompose') {
|
||||
$application->docker_compose_domains = null;
|
||||
$application->docker_compose_raw = null;
|
||||
|
||||
// Remove SERVICE_FQDN_* and SERVICE_URL_* environment variables
|
||||
$application->environment_variables()
|
||||
->where('key', 'LIKE', 'SERVICE_FQDN_%')
|
||||
->orWhere('key', 'LIKE', 'SERVICE_URL_%')
|
||||
->delete();
|
||||
|
||||
$application->environment_variables_preview()
|
||||
->where('key', 'LIKE', 'SERVICE_FQDN_%')
|
||||
->orWhere('key', 'LIKE', 'SERVICE_URL_%')
|
||||
->delete();
|
||||
}
|
||||
|
||||
// Clear Dockerfile specific data when switching away from dockerfile
|
||||
if ($originalBuildPack === 'dockerfile') {
|
||||
$application->dockerfile = null;
|
||||
$application->dockerfile_location = null;
|
||||
$application->dockerfile_target_build = null;
|
||||
$application->custom_healthcheck_found = false;
|
||||
}
|
||||
}
|
||||
});
|
||||
static::created(function ($application) {
|
||||
ApplicationSetting::create([
|
||||
|
||||
@ -0,0 +1,31 @@
|
||||
<?php
|
||||
|
||||
use Illuminate\Database\Migrations\Migration;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
|
||||
return new class extends Migration
|
||||
{
|
||||
/**
|
||||
* Run the migrations.
|
||||
*/
|
||||
public function up(): void
|
||||
{
|
||||
// Clear dockerfile fields for applications not using dockerfile buildpack
|
||||
DB::table('applications')
|
||||
->where('build_pack', '!=', 'dockerfile')
|
||||
->update([
|
||||
'dockerfile' => null,
|
||||
'dockerfile_location' => null,
|
||||
'dockerfile_target_build' => null,
|
||||
'custom_healthcheck_found' => false,
|
||||
]);
|
||||
}
|
||||
|
||||
/**
|
||||
* Reverse the migrations.
|
||||
*/
|
||||
public function down(): void
|
||||
{
|
||||
// No rollback needed - we're cleaning up corrupt data
|
||||
}
|
||||
};
|
||||
183
tests/Feature/ApplicationBuildpackCleanupTest.php
Normal file
183
tests/Feature/ApplicationBuildpackCleanupTest.php
Normal file
@ -0,0 +1,183 @@
|
||||
<?php
|
||||
|
||||
use App\Models\Application;
|
||||
use App\Models\Environment;
|
||||
use App\Models\EnvironmentVariable;
|
||||
use App\Models\Project;
|
||||
use App\Models\Team;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
|
||||
uses(RefreshDatabase::class);
|
||||
|
||||
describe('Application Model Buildpack Cleanup', function () {
|
||||
test('model clears dockerfile fields when build_pack changes from dockerfile to nixpacks', function () {
|
||||
$team = Team::factory()->create();
|
||||
$project = Project::factory()->create(['team_id' => $team->id]);
|
||||
$environment = Environment::factory()->create(['project_id' => $project->id]);
|
||||
|
||||
$application = Application::factory()->create([
|
||||
'environment_id' => $environment->id,
|
||||
'build_pack' => 'dockerfile',
|
||||
'dockerfile' => 'FROM node:18\nHEALTHCHECK CMD curl -f http://localhost/ || exit 1',
|
||||
'dockerfile_location' => '/Dockerfile',
|
||||
'dockerfile_target_build' => 'production',
|
||||
'custom_healthcheck_found' => true,
|
||||
]);
|
||||
|
||||
// Change buildpack to nixpacks
|
||||
$application->build_pack = 'nixpacks';
|
||||
$application->save();
|
||||
|
||||
// Reload from database
|
||||
$application->refresh();
|
||||
|
||||
// Verify dockerfile fields were cleared
|
||||
expect($application->build_pack)->toBe('nixpacks');
|
||||
expect($application->dockerfile)->toBeNull();
|
||||
expect($application->dockerfile_location)->toBeNull();
|
||||
expect($application->dockerfile_target_build)->toBeNull();
|
||||
expect($application->custom_healthcheck_found)->toBeFalse();
|
||||
});
|
||||
|
||||
test('model clears dockerfile fields when build_pack changes from dockerfile to static', function () {
|
||||
$team = Team::factory()->create();
|
||||
$project = Project::factory()->create(['team_id' => $team->id]);
|
||||
$environment = Environment::factory()->create(['project_id' => $project->id]);
|
||||
|
||||
$application = Application::factory()->create([
|
||||
'environment_id' => $environment->id,
|
||||
'build_pack' => 'dockerfile',
|
||||
'dockerfile' => 'FROM nginx:alpine',
|
||||
'dockerfile_location' => '/custom.Dockerfile',
|
||||
'dockerfile_target_build' => 'prod',
|
||||
'custom_healthcheck_found' => true,
|
||||
]);
|
||||
|
||||
$application->build_pack = 'static';
|
||||
$application->save();
|
||||
$application->refresh();
|
||||
|
||||
expect($application->build_pack)->toBe('static');
|
||||
expect($application->dockerfile)->toBeNull();
|
||||
expect($application->dockerfile_location)->toBeNull();
|
||||
expect($application->dockerfile_target_build)->toBeNull();
|
||||
expect($application->custom_healthcheck_found)->toBeFalse();
|
||||
});
|
||||
|
||||
test('model clears dockercompose fields when build_pack changes from dockercompose to nixpacks', function () {
|
||||
$team = Team::factory()->create();
|
||||
$project = Project::factory()->create(['team_id' => $team->id]);
|
||||
$environment = Environment::factory()->create(['project_id' => $project->id]);
|
||||
|
||||
$application = Application::factory()->create([
|
||||
'environment_id' => $environment->id,
|
||||
'build_pack' => 'dockercompose',
|
||||
'docker_compose_domains' => '{"app": "example.com"}',
|
||||
'docker_compose_raw' => 'version: "3.8"\nservices:\n app:\n image: nginx',
|
||||
]);
|
||||
|
||||
// Add environment variables that should be deleted
|
||||
EnvironmentVariable::create([
|
||||
'application_id' => $application->id,
|
||||
'key' => 'SERVICE_FQDN_APP',
|
||||
'value' => 'app.example.com',
|
||||
'is_build_time' => false,
|
||||
'is_preview' => false,
|
||||
]);
|
||||
|
||||
EnvironmentVariable::create([
|
||||
'application_id' => $application->id,
|
||||
'key' => 'SERVICE_URL_APP',
|
||||
'value' => 'http://app.example.com',
|
||||
'is_build_time' => false,
|
||||
'is_preview' => false,
|
||||
]);
|
||||
|
||||
EnvironmentVariable::create([
|
||||
'application_id' => $application->id,
|
||||
'key' => 'REGULAR_VAR',
|
||||
'value' => 'should_remain',
|
||||
'is_build_time' => false,
|
||||
'is_preview' => false,
|
||||
]);
|
||||
|
||||
$application->build_pack = 'nixpacks';
|
||||
$application->save();
|
||||
$application->refresh();
|
||||
|
||||
expect($application->build_pack)->toBe('nixpacks');
|
||||
expect($application->docker_compose_domains)->toBeNull();
|
||||
expect($application->docker_compose_raw)->toBeNull();
|
||||
|
||||
// Verify SERVICE_FQDN_* and SERVICE_URL_* were deleted
|
||||
expect($application->environment_variables()->where('key', 'SERVICE_FQDN_APP')->count())->toBe(0);
|
||||
expect($application->environment_variables()->where('key', 'SERVICE_URL_APP')->count())->toBe(0);
|
||||
|
||||
// Verify regular variables remain
|
||||
expect($application->environment_variables()->where('key', 'REGULAR_VAR')->count())->toBe(1);
|
||||
});
|
||||
|
||||
test('model does not clear dockerfile fields when switching to dockerfile', function () {
|
||||
$team = Team::factory()->create();
|
||||
$project = Project::factory()->create(['team_id' => $team->id]);
|
||||
$environment = Environment::factory()->create(['project_id' => $project->id]);
|
||||
|
||||
$application = Application::factory()->create([
|
||||
'environment_id' => $environment->id,
|
||||
'build_pack' => 'nixpacks',
|
||||
'dockerfile' => null,
|
||||
]);
|
||||
|
||||
$application->build_pack = 'dockerfile';
|
||||
$application->save();
|
||||
$application->refresh();
|
||||
|
||||
// When switching TO dockerfile, no cleanup should happen
|
||||
expect($application->build_pack)->toBe('dockerfile');
|
||||
});
|
||||
|
||||
test('model does not clear fields when switching between non-dockerfile buildpacks', function () {
|
||||
$team = Team::factory()->create();
|
||||
$project = Project::factory()->create(['team_id' => $team->id]);
|
||||
$environment = Environment::factory()->create(['project_id' => $project->id]);
|
||||
|
||||
$application = Application::factory()->create([
|
||||
'environment_id' => $environment->id,
|
||||
'build_pack' => 'nixpacks',
|
||||
'dockerfile' => null,
|
||||
'dockerfile_location' => null,
|
||||
]);
|
||||
|
||||
$application->build_pack = 'static';
|
||||
$application->save();
|
||||
$application->refresh();
|
||||
|
||||
expect($application->build_pack)->toBe('static');
|
||||
expect($application->dockerfile)->toBeNull();
|
||||
});
|
||||
|
||||
test('model does not trigger cleanup when build_pack is not changed', function () {
|
||||
$team = Team::factory()->create();
|
||||
$project = Project::factory()->create(['team_id' => $team->id]);
|
||||
$environment = Environment::factory()->create(['project_id' => $project->id]);
|
||||
|
||||
$application = Application::factory()->create([
|
||||
'environment_id' => $environment->id,
|
||||
'build_pack' => 'dockerfile',
|
||||
'dockerfile' => 'FROM alpine:latest',
|
||||
'dockerfile_location' => '/Dockerfile',
|
||||
'custom_healthcheck_found' => true,
|
||||
]);
|
||||
|
||||
// Update another field without changing build_pack
|
||||
$application->name = 'Updated Name';
|
||||
$application->save();
|
||||
$application->refresh();
|
||||
|
||||
// Dockerfile fields should remain unchanged
|
||||
expect($application->build_pack)->toBe('dockerfile');
|
||||
expect($application->dockerfile)->toBe('FROM alpine:latest');
|
||||
expect($application->dockerfile_location)->toBe('/Dockerfile');
|
||||
expect($application->custom_healthcheck_found)->toBeTrue();
|
||||
});
|
||||
});
|
||||
134
tests/Feature/BuildpackSwitchCleanupTest.php
Normal file
134
tests/Feature/BuildpackSwitchCleanupTest.php
Normal file
@ -0,0 +1,134 @@
|
||||
<?php
|
||||
|
||||
use App\Livewire\Project\Application\General;
|
||||
use App\Models\Application;
|
||||
use App\Models\Environment;
|
||||
use App\Models\Project;
|
||||
use App\Models\Team;
|
||||
use App\Models\User;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
use Livewire\Livewire;
|
||||
|
||||
uses(RefreshDatabase::class);
|
||||
|
||||
beforeEach(function () {
|
||||
// Create a team with owner
|
||||
$this->team = Team::factory()->create();
|
||||
$this->user = User::factory()->create();
|
||||
$this->team->members()->attach($this->user->id, ['role' => 'owner']);
|
||||
|
||||
// Set current team
|
||||
$this->actingAs($this->user);
|
||||
session(['currentTeam' => $this->team]);
|
||||
|
||||
// Create project and environment
|
||||
$this->project = Project::factory()->create(['team_id' => $this->team->id]);
|
||||
$this->environment = Environment::factory()->create(['project_id' => $this->project->id]);
|
||||
});
|
||||
|
||||
describe('Buildpack Switching Cleanup', function () {
|
||||
test('clears dockerfile fields when switching from dockerfile to nixpacks', function () {
|
||||
// Create an application with dockerfile buildpack and dockerfile content
|
||||
$application = Application::factory()->create([
|
||||
'environment_id' => $this->environment->id,
|
||||
'build_pack' => 'dockerfile',
|
||||
'dockerfile' => 'FROM node:18\nHEALTHCHECK CMD curl -f http://localhost/ || exit 1',
|
||||
'dockerfile_location' => '/Dockerfile',
|
||||
'dockerfile_target_build' => 'production',
|
||||
'custom_healthcheck_found' => true,
|
||||
]);
|
||||
|
||||
// Switch to nixpacks buildpack
|
||||
Livewire::test(General::class, ['application' => $application])
|
||||
->assertSuccessful()
|
||||
->set('buildPack', 'nixpacks')
|
||||
->call('updatedBuildPack');
|
||||
|
||||
// Verify dockerfile fields were cleared
|
||||
$application->refresh();
|
||||
expect($application->build_pack)->toBe('nixpacks');
|
||||
expect($application->dockerfile)->toBeNull();
|
||||
expect($application->dockerfile_location)->toBeNull();
|
||||
expect($application->dockerfile_target_build)->toBeNull();
|
||||
expect($application->custom_healthcheck_found)->toBeFalse();
|
||||
});
|
||||
|
||||
test('clears dockerfile fields when switching from dockerfile to static', function () {
|
||||
$application = Application::factory()->create([
|
||||
'environment_id' => $this->environment->id,
|
||||
'build_pack' => 'dockerfile',
|
||||
'dockerfile' => 'FROM nginx:alpine',
|
||||
'dockerfile_location' => '/custom.Dockerfile',
|
||||
'dockerfile_target_build' => 'prod',
|
||||
'custom_healthcheck_found' => true,
|
||||
]);
|
||||
|
||||
Livewire::test(General::class, ['application' => $application])
|
||||
->assertSuccessful()
|
||||
->set('buildPack', 'static')
|
||||
->call('updatedBuildPack');
|
||||
|
||||
$application->refresh();
|
||||
expect($application->build_pack)->toBe('static');
|
||||
expect($application->dockerfile)->toBeNull();
|
||||
expect($application->dockerfile_location)->toBeNull();
|
||||
expect($application->dockerfile_target_build)->toBeNull();
|
||||
expect($application->custom_healthcheck_found)->toBeFalse();
|
||||
});
|
||||
|
||||
test('does not clear dockerfile fields when switching to dockerfile', function () {
|
||||
$application = Application::factory()->create([
|
||||
'environment_id' => $this->environment->id,
|
||||
'build_pack' => 'nixpacks',
|
||||
'dockerfile' => null,
|
||||
]);
|
||||
|
||||
Livewire::test(General::class, ['application' => $application])
|
||||
->assertSuccessful()
|
||||
->set('buildPack', 'dockerfile')
|
||||
->call('updatedBuildPack');
|
||||
|
||||
// When switching TO dockerfile, fields remain as they were
|
||||
$application->refresh();
|
||||
expect($application->build_pack)->toBe('dockerfile');
|
||||
});
|
||||
|
||||
test('does not affect fields when switching between non-dockerfile buildpacks', function () {
|
||||
$application = Application::factory()->create([
|
||||
'environment_id' => $this->environment->id,
|
||||
'build_pack' => 'nixpacks',
|
||||
'dockerfile' => null,
|
||||
'dockerfile_location' => null,
|
||||
]);
|
||||
|
||||
Livewire::test(General::class, ['application' => $application])
|
||||
->assertSuccessful()
|
||||
->set('buildPack', 'static')
|
||||
->call('updatedBuildPack');
|
||||
|
||||
$application->refresh();
|
||||
expect($application->build_pack)->toBe('static');
|
||||
expect($application->dockerfile)->toBeNull();
|
||||
});
|
||||
|
||||
test('clears dockerfile fields when switching from dockerfile to dockercompose', function () {
|
||||
$application = Application::factory()->create([
|
||||
'environment_id' => $this->environment->id,
|
||||
'build_pack' => 'dockerfile',
|
||||
'dockerfile' => 'FROM alpine:latest',
|
||||
'dockerfile_location' => '/docker/Dockerfile',
|
||||
'custom_healthcheck_found' => true,
|
||||
]);
|
||||
|
||||
Livewire::test(General::class, ['application' => $application])
|
||||
->assertSuccessful()
|
||||
->set('buildPack', 'dockercompose')
|
||||
->call('updatedBuildPack');
|
||||
|
||||
$application->refresh();
|
||||
expect($application->build_pack)->toBe('dockercompose');
|
||||
expect($application->dockerfile)->toBeNull();
|
||||
expect($application->dockerfile_location)->toBeNull();
|
||||
expect($application->custom_healthcheck_found)->toBeFalse();
|
||||
});
|
||||
});
|
||||
Loading…
Reference in New Issue
Block a user