From 0834a79fb26993d3653ba2cc41933f89146efdf3 Mon Sep 17 00:00:00 2001 From: Mahad Kalam Date: Wed, 10 Dec 2025 17:35:19 +0000 Subject: [PATCH] coderabbit fixes, batch 2 --- app/Actions/Database/PgBackrestRestore.php | 1 + app/Jobs/PgBackrestRestoreJob.php | 45 +++++++++---------- app/Livewire/Project/Database/BackupEdit.php | 3 +- ...25_12_09_231049_add_pgbackrest_support.php | 4 ++ .../database/backup-executions.blade.php | 2 + 5 files changed, 30 insertions(+), 25 deletions(-) diff --git a/app/Actions/Database/PgBackrestRestore.php b/app/Actions/Database/PgBackrestRestore.php index e14aea97f..6081d6064 100644 --- a/app/Actions/Database/PgBackrestRestore.php +++ b/app/Actions/Database/PgBackrestRestore.php @@ -38,6 +38,7 @@ class PgBackrestRestore { return [ 'database' => ['required'], + 'targetTime' => ['nullable', 'date'], ]; } } diff --git a/app/Jobs/PgBackrestRestoreJob.php b/app/Jobs/PgBackrestRestoreJob.php index c6ce39b01..d43a32969 100644 --- a/app/Jobs/PgBackrestRestoreJob.php +++ b/app/Jobs/PgBackrestRestoreJob.php @@ -17,6 +17,7 @@ use Illuminate\Foundation\Bus\Dispatchable; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\SerializesModels; use Illuminate\Support\Facades\Log; +use RuntimeException; use Throwable; class PgBackrestRestoreJob implements ShouldBeEncrypted, ShouldQueue @@ -38,8 +39,6 @@ class PgBackrestRestoreJob implements ShouldBeEncrypted, ShouldQueue public function handle(): void { - $server = $this->database->destination->server; - $containerName = $this->database->uuid; $stanza = PgBackrestService::getStanzaName($this->database); $backupTimestamp = time(); @@ -124,54 +123,54 @@ class PgBackrestRestoreJob implements ShouldBeEncrypted, ShouldQueue $this->restore->appendLog('Running pre-flight validation.'); if (! $this->database->hasPgBackrestBackups()) { - throw new \RuntimeException('No PgBackRest backup configuration found for this database.'); + throw new RuntimeException('No PgBackRest backup configuration found for this database.'); } $backup = $this->database->pgbackrestBackups()->where('enabled', true)->first(); if (! $backup) { - throw new \RuntimeException('No enabled PgBackRest backup configuration found.'); + throw new RuntimeException('No enabled PgBackRest backup configuration found.'); } $s3Repos = $backup->pgbackrestRepos()->where('type', 's3')->where('enabled', true)->get(); foreach ($s3Repos as $s3Repo) { $s3 = $s3Repo->s3Storage; if (! $s3) { - throw new \RuntimeException("S3 storage configuration not found for repo {$s3Repo->repo_number}."); + throw new RuntimeException("S3 storage configuration not found for repo {$s3Repo->repo_number}."); } try { $s3->testConnection(shouldSave: true); } catch (Throwable $e) { - throw new \RuntimeException("S3 connection test failed for repo {$s3Repo->repo_number}: ".$e->getMessage()); + throw new RuntimeException("S3 connection test failed for repo {$s3Repo->repo_number}: ".$e->getMessage()); } } $pgdataVolume = $this->database->pgdataVolume(); if (! $pgdataVolume) { - throw new \RuntimeException('PGDATA volume not found.'); + throw new RuntimeException('PGDATA volume not found.'); } $repoVolume = $this->database->pgbackrestRepoVolume(); $hasLocalRepo = $backup->hasLocalRepo(); if (! $repoVolume && $hasLocalRepo) { - throw new \RuntimeException('PgBackRest repository volume not found.'); + throw new RuntimeException('PgBackRest repository volume not found.'); } $this->restore->appendLog('Verifying backup exists in repository via sidecar container.'); $info = $this->runInfoSidecar($stanza, $backup); if (! PgBackrestService::stanzaExists($info)) { - throw new \RuntimeException('PgBackRest stanza does not exist or is not healthy.'); + throw new RuntimeException('PgBackRest stanza does not exist or is not healthy.'); } if (! PgBackrestService::hasBackups($info)) { - throw new \RuntimeException('No backups found in PgBackRest repository.'); + throw new RuntimeException('No backups found in PgBackRest repository.'); } if ($this->execution && $this->execution->pgbackrest_label) { $targetBackup = PgBackrestService::findBackupByLabel($info, $this->execution->pgbackrest_label); if (! $targetBackup) { - throw new \RuntimeException("Backup with label '{$this->execution->pgbackrest_label}' not found in repository."); + throw new RuntimeException("Backup with label '{$this->execution->pgbackrest_label}' not found in repository."); } $this->restore->appendLog("Target backup verified: {$this->execution->pgbackrest_label}"); } else { @@ -203,8 +202,7 @@ class PgBackrestRestoreJob implements ShouldBeEncrypted, ShouldQueue $s3EnvVars = PgBackrestService::buildS3EnvVars($backup); foreach ($s3EnvVars as $key => $value) { - $escapedValue = addslashes($value); - $envPieces[] = "-e {$key}=\"{$escapedValue}\""; + $envPieces[] = '-e '.$key.'='.escapeshellarg($value); } $infoCmd = PgBackrestService::buildInfoCommand($stanza, true); @@ -223,7 +221,7 @@ class PgBackrestRestoreJob implements ShouldBeEncrypted, ShouldQueue $output = instant_remote_process([$cmd], $server, false, false, 120, disableMultiplexing: true); if ($output === null || $output === '') { - throw new \RuntimeException('Failed to get PgBackRest info - command returned no output. Command: '.$cmd); + throw new RuntimeException('Failed to get PgBackRest info - command returned no output. Command: '.$cmd); } $jsonStart = strpos($output, '['); @@ -233,7 +231,7 @@ class PgBackrestRestoreJob implements ShouldBeEncrypted, ShouldQueue $info = PgBackrestService::parseInfoJson($output); if ($info === null) { - throw new \RuntimeException('Failed to parse PgBackRest info output: '.$output); + throw new RuntimeException('Failed to parse PgBackRest info output: '.$output); } return $info; @@ -245,7 +243,7 @@ class PgBackrestRestoreJob implements ShouldBeEncrypted, ShouldQueue $pgdataVolume = $this->database->pgdataVolume(); if (! $pgdataVolume) { - throw new \RuntimeException('PGDATA volume not found.'); + throw new RuntimeException('PGDATA volume not found.'); } $mount = $pgdataVolume->host_path ?: $pgdataVolume->name; @@ -268,7 +266,7 @@ class PgBackrestRestoreJob implements ShouldBeEncrypted, ShouldQueue $result = instant_remote_process([$verifyCmd], $server, false, false, 30, disableMultiplexing: true); if (trim($result) !== 'OK') { - throw new \RuntimeException('PGDATA backup verification failed: backup directory is empty or inaccessible.'); + throw new RuntimeException('PGDATA backup verification failed: backup directory is empty or inaccessible.'); } $this->restore->appendLog("PGDATA backed up to temporary location: {$backupPath}"); @@ -320,7 +318,7 @@ class PgBackrestRestoreJob implements ShouldBeEncrypted, ShouldQueue $pgdataVolume = $this->database->pgdataVolume(); if (! $pgdataVolume) { - throw new \RuntimeException('PGDATA volume not found.'); + throw new RuntimeException('PGDATA volume not found.'); } $mount = $pgdataVolume->host_path ?: $pgdataVolume->name; @@ -336,7 +334,7 @@ class PgBackrestRestoreJob implements ShouldBeEncrypted, ShouldQueue try { $pgdataVolume = $this->database->pgdataVolume(); if (! $pgdataVolume) { - throw new \RuntimeException('PGDATA volume not found.'); + throw new RuntimeException('PGDATA volume not found.'); } $server = $this->database->destination->server; @@ -347,12 +345,12 @@ class PgBackrestRestoreJob implements ShouldBeEncrypted, ShouldQueue $result = instant_remote_process([$checkCmd], $server, false, false, 30, disableMultiplexing: true); if (trim($result) !== 'OK') { - throw new \RuntimeException('Restored PGDATA does not contain valid PostgreSQL data (PG_VERSION not found).'); + throw new RuntimeException('Restored PGDATA does not contain valid PostgreSQL data (PG_VERSION not found).'); } $this->restore->appendLog('Restored database verified successfully.'); } catch (Throwable $e) { - throw new \RuntimeException('Database verification failed: '.$e->getMessage()); + throw new RuntimeException('Database verification failed: '.$e->getMessage()); } } @@ -364,7 +362,7 @@ class PgBackrestRestoreJob implements ShouldBeEncrypted, ShouldQueue $backup = $this->database->pgbackrestBackups()->where('enabled', true)->first(); if (! $backup) { - throw new \RuntimeException('No enabled PgBackRest backup configuration found.'); + throw new RuntimeException('No enabled PgBackRest backup configuration found.'); } $pgdataVolume = $this->database->pgdataVolume(); @@ -386,8 +384,7 @@ class PgBackrestRestoreJob implements ShouldBeEncrypted, ShouldQueue $s3EnvVars = PgBackrestService::buildS3EnvVars($backup); foreach ($s3EnvVars as $key => $value) { - $escapedValue = addslashes($value); - $envPieces[] = "-e {$key}=\"{$escapedValue}\""; + $envPieces[] = '-e '.$key.'='.escapeshellarg($value); } $restoreCmd = PgBackrestService::buildRestoreCommand( diff --git a/app/Livewire/Project/Database/BackupEdit.php b/app/Livewire/Project/Database/BackupEdit.php index 775c09736..275be6547 100644 --- a/app/Livewire/Project/Database/BackupEdit.php +++ b/app/Livewire/Project/Database/BackupEdit.php @@ -5,6 +5,7 @@ namespace App\Livewire\Project\Database; use App\Models\InstanceSettings; use App\Models\PgbackrestRepo; use App\Models\ScheduledDatabaseBackup; +use App\Models\StandalonePostgresql; use Exception; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; use Illuminate\Support\Facades\Auth; @@ -417,7 +418,7 @@ class BackupEdit extends Component public function isPostgresql(): bool { - return $this->backup->database_type === 'App\Models\StandalonePostgresql'; + return $this->backup->database_type === StandalonePostgresql::class; } public function render() diff --git a/database/migrations/2025_12_09_231049_add_pgbackrest_support.php b/database/migrations/2025_12_09_231049_add_pgbackrest_support.php index 615eeee6f..78be380ff 100644 --- a/database/migrations/2025_12_09_231049_add_pgbackrest_support.php +++ b/database/migrations/2025_12_09_231049_add_pgbackrest_support.php @@ -69,6 +69,7 @@ return new class extends Migration $table->timestamp('target_time')->nullable(); $table->string('status')->default('pending'); + $table->index('status'); $table->longText('message')->nullable(); $table->longText('log')->nullable(); @@ -79,6 +80,9 @@ return new class extends Migration public function down(): void { + Schema::table('database_restores', function (Blueprint $table) { + $table->dropIndex(['status']); + }); Schema::dropIfExists('database_restores'); Schema::dropIfExists('pgbackrest_repos'); diff --git a/resources/views/livewire/project/database/backup-executions.blade.php b/resources/views/livewire/project/database/backup-executions.blade.php index 6e110a026..534da1790 100644 --- a/resources/views/livewire/project/database/backup-executions.blade.php +++ b/resources/views/livewire/project/database/backup-executions.blade.php @@ -249,6 +249,7 @@ {{-- Restore Confirmation Modal --}} + @can('manage', $database) @if ($showRestoreModal && $restoreExecutionId) @php $restoreExecution = $executions->firstWhere('id', $restoreExecutionId); @@ -319,6 +320,7 @@ @endif + @endcan {{-- Restore Progress Modal --}} @if ($showRestoreProgressModal && $currentRestore)