refactor: move buildpack cleanup to model lifecycle hooks (#7268)

This commit is contained in:
Andras Bacsai 2025-11-18 10:16:41 +01:00 committed by GitHub
commit 108f3a3d61
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 381 additions and 14 deletions

View file

@ -641,8 +641,6 @@ public function updatedBuildPack()
$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 +653,6 @@ public function updatedBuildPack()
} 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';

View file

@ -176,6 +176,39 @@ protected static function booted()
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(function ($q) {
$q->where('key', 'LIKE', 'SERVICE_FQDN_%')
->orWhere('key', 'LIKE', 'SERVICE_URL_%');
})
->delete();
$application->environment_variables_preview()
->where(function ($q) {
$q->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([

View file

@ -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
}
};

View 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();
});
});

View 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();
});
});