From 556ced24b524ef67fcbd80cf8c860d307020a359 Mon Sep 17 00:00:00 2001 From: Mahad Kalam Date: Wed, 10 Dec 2025 19:38:32 +0000 Subject: [PATCH] Make sh*t work when WD isn't 'coolify' in dev + rename, not copy, old PGData Rename, not copy, old PGDATA Bit more reliability when working dir isn't called coolify! --- app/Jobs/DatabaseBackupJob.php | 92 ++++++++--------------- app/Jobs/PgBackrestRestoreJob.php | 40 +++++----- app/Services/Backup/PgBackrestService.php | 45 +++++++++++ bootstrap/helpers/shared.php | 35 ++++++++- 4 files changed, 131 insertions(+), 81 deletions(-) diff --git a/app/Jobs/DatabaseBackupJob.php b/app/Jobs/DatabaseBackupJob.php index f8817576c..0d6b348fe 100644 --- a/app/Jobs/DatabaseBackupJob.php +++ b/app/Jobs/DatabaseBackupJob.php @@ -471,7 +471,7 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue throw new \Exception('MongoDB credentials not found. Ensure MONGO_INITDB_ROOT_USERNAME and MONGO_INITDB_ROOT_PASSWORD environment variables are available in the container.'); } } - \Log::info('MongoDB backup URL configured', ['has_url' => filled($url), 'using_env_vars' => blank($this->database->internal_db_url)]); + Log::info('MongoDB backup URL configured', ['has_url' => filled($url), 'using_env_vars' => blank($this->database->internal_db_url)]); if ($databaseWithCollections === 'all') { $commands[] = 'mkdir -p '.$this->backup_dir; if (str($this->database->image)->startsWith('mongo:4')) { @@ -703,18 +703,14 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue $this->update_pgbackrest_config(); $backupCmd = PgBackrestService::buildBackupCommand($stanza, $backupType, 'info'); + $backupCmdWithWait = PgBackrestService::wrapWithLockWait($backupCmd); $s3EnvVars = PgBackrestService::buildS3EnvVars($this->backup); - if (! empty($s3EnvVars)) { - $envExport = ''; - foreach ($s3EnvVars as $key => $value) { - $escapedValue = addslashes($value); - $envExport .= "export {$key}=\"{$escapedValue}\"; "; - } - $backupFullCmd = "docker exec {$this->container_name} sh -c '{$envExport} {$backupCmd} 2>&1; echo \"EXIT_CODE:\$?\"'"; - } else { - $backupFullCmd = "docker exec {$this->container_name} sh -c '{$backupCmd} 2>&1; echo \"EXIT_CODE:\$?\"'"; - } + $dockerEnvArgs = PgBackrestService::buildDockerEnvArgs($s3EnvVars); + $fixPermsCmd = 'chown -R postgres:postgres /var/lib/pgbackrest /tmp/pgbackrest /var/log/pgbackrest 2>/dev/null || true'; + $escapedBackupCmd = escapeshellarg($backupCmdWithWait); + $containerName = escapeshellarg($this->container_name); + $backupFullCmd = "docker exec{$dockerEnvArgs} {$containerName} sh -c '{$fixPermsCmd}; su postgres -c {$escapedBackupCmd} 2>&1; echo \"EXIT_CODE:\$?\"'"; $rawOutput = instant_remote_process([$backupFullCmd], $this->server, false, false, $this->timeout, disableMultiplexing: true); $rawOutput = trim($rawOutput) ?: ''; @@ -733,31 +729,15 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue } $infoCmd = PgBackrestService::buildInfoCommand($stanza, true); - - if (! empty($s3EnvVars)) { - $envExport = ''; - foreach ($s3EnvVars as $key => $value) { - $escapedValue = addslashes($value); - $envExport .= "export {$key}=\"{$escapedValue}\"; "; - } - $infoJson = instant_remote_process( - ["docker exec {$this->container_name} sh -c '{$envExport} {$infoCmd}'"], - $this->server, - false, - false, - 120, - disableMultiplexing: true - ); - } else { - $infoJson = instant_remote_process( - ["docker exec {$this->container_name} {$infoCmd}"], - $this->server, - false, - false, - 120, - disableMultiplexing: true - ); - } + $escapedInfoCmd = escapeshellarg($infoCmd); + $infoJson = instant_remote_process( + ["docker exec{$dockerEnvArgs} {$containerName} su postgres -c {$escapedInfoCmd}"], + $this->server, + false, + false, + 120, + disableMultiplexing: true + ); $info = PgBackrestService::parseInfoJson($infoJson); $latestBackup = $info ? PgBackrestService::getLatestBackup($info) : null; @@ -812,35 +792,22 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue { try { $repos = $this->backup->enabledPgbackrestRepos()->get(); + $s3EnvVars = PgBackrestService::buildS3EnvVars($this->backup); + $dockerEnvArgs = PgBackrestService::buildDockerEnvArgs($s3EnvVars); + $containerName = escapeshellarg($this->container_name); foreach ($repos as $repo) { $expireCmd = PgBackrestService::buildExpireCommand($stanza, $repo->repo_number); + $escapedExpireCmd = escapeshellarg($expireCmd); - $s3EnvVars = PgBackrestService::buildS3EnvVars($this->backup); - if (! empty($s3EnvVars)) { - $envExport = ''; - foreach ($s3EnvVars as $key => $value) { - $escapedValue = addslashes($value); - $envExport .= "export {$key}=\"{$escapedValue}\"; "; - } - instant_remote_process( - ["docker exec {$this->container_name} sh -c '{$envExport} {$expireCmd}'"], - $this->server, - false, - false, - $this->timeout, - disableMultiplexing: true - ); - } else { - instant_remote_process( - ["docker exec {$this->container_name} {$expireCmd}"], - $this->server, - false, - false, - $this->timeout, - disableMultiplexing: true - ); - } + instant_remote_process( + ["docker exec{$dockerEnvArgs} {$containerName} sh -c {$escapedExpireCmd}"], + $this->server, + false, + false, + $this->timeout, + disableMultiplexing: true + ); } } catch (Throwable $e) { Log::warning('PgBackRest expire failed', [ @@ -860,9 +827,10 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue $configBase64 = base64_encode($config); $configPath = PgBackrestService::CONFIG_PATH; + $containerName = escapeshellarg($this->container_name); instant_remote_process([ - "docker exec {$this->container_name} sh -c 'echo {$configBase64} | base64 -d > {$configPath}/pgbackrest.conf'", + "docker exec {$containerName} sh -c 'echo {$configBase64} | base64 -d > {$configPath}/pgbackrest.conf'", ], $this->server, true, false, 60, disableMultiplexing: true); } diff --git a/app/Jobs/PgBackrestRestoreJob.php b/app/Jobs/PgBackrestRestoreJob.php index d43a32969..23683c3db 100644 --- a/app/Jobs/PgBackrestRestoreJob.php +++ b/app/Jobs/PgBackrestRestoreJob.php @@ -246,9 +246,10 @@ class PgBackrestRestoreJob implements ShouldBeEncrypted, ShouldQueue throw new RuntimeException('PGDATA volume not found.'); } - $mount = $pgdataVolume->host_path ?: $pgdataVolume->name; + $volumeName = $pgdataVolume->host_path ?: $pgdataVolume->name; + $backupVolumeName = "{$pgdataVolume->name}_backup_{$timestamp}"; - $checkCmd = "docker run --rm -v {$mount}:/data alpine sh -c 'test -n \"$(ls -A /data 2>/dev/null)\" && echo OK || echo EMPTY'"; + $checkCmd = "docker run --rm -v {$volumeName}:/data alpine sh -c 'test -n \"$(ls -A /data 2>/dev/null)\" && echo OK || echo EMPTY'"; $checkResult = instant_remote_process([$checkCmd], $server, false, false, 30, disableMultiplexing: true); if (trim($checkResult) === 'EMPTY') { @@ -257,24 +258,24 @@ class PgBackrestRestoreJob implements ShouldBeEncrypted, ShouldQueue return null; } - $backupPath = "{$mount}_backup_{$timestamp}"; + instant_remote_process(["docker volume create {$backupVolumeName}"], $server, false, false, 30, disableMultiplexing: true); - $backupCmd = "docker run --rm -v {$mount}:/data -v {$mount}_backup_{$timestamp}:/backup alpine sh -c 'cp -a /data/. /backup/'"; - instant_remote_process([$backupCmd], $server, true, false, 600, disableMultiplexing: true); + $copyCmd = "docker run --rm -v {$volumeName}:/source:ro -v {$backupVolumeName}:/backup alpine sh -c 'cp -a /source/. /backup/'"; + instant_remote_process([$copyCmd], $server, true, false, 300, disableMultiplexing: true); - $verifyCmd = "docker run --rm -v {$backupPath}:/backup alpine sh -c 'test -n \"$(ls -A /backup 2>/dev/null)\" && echo OK'"; + $verifyCmd = "docker run --rm -v {$backupVolumeName}:/backup alpine sh -c 'test -n \"$(ls -A /backup 2>/dev/null)\" && echo OK'"; $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 volume is empty or inaccessible.'); } - $this->restore->appendLog("PGDATA backed up to temporary location: {$backupPath}"); + $this->restore->appendLog("PGDATA backed up to volume: {$backupVolumeName}"); - return $backupPath; + return $backupVolumeName; } - private function recoverFromBackup(string $backupPath): void + private function recoverFromBackup(string $backupVolumeName): void { try { $server = $this->database->destination->server; @@ -286,11 +287,15 @@ class PgBackrestRestoreJob implements ShouldBeEncrypted, ShouldQueue return; } - $mount = $pgdataVolume->host_path ?: $pgdataVolume->name; + $volumeName = $pgdataVolume->host_path ?: $pgdataVolume->name; $this->restore->appendLog('Recovering PGDATA from backup...'); - $recoverCmd = "docker run --rm -v {$mount}:/data -v {$backupPath}:/backup alpine sh -c 'rm -rf /data/* /data/.[!.]* /data/..?* 2>/dev/null || true && cp -a /backup/* /data/ 2>/dev/null || true'"; - instant_remote_process([$recoverCmd], $server, false, false, 600, disableMultiplexing: true); + + $clearCmd = "docker run --rm -v {$volumeName}:/data alpine sh -c 'rm -rf /data/* /data/.[!.]* /data/..?* 2>/dev/null || true'"; + instant_remote_process([$clearCmd], $server, false, false, 60, disableMultiplexing: true); + + $copyCmd = "docker run --rm -v {$backupVolumeName}:/source:ro -v {$volumeName}:/data alpine sh -c 'cp -a /source/. /data/'"; + instant_remote_process([$copyCmd], $server, true, false, 300, disableMultiplexing: true); $this->restore->appendLog('PGDATA recovered from backup.'); } catch (Throwable $e) { @@ -298,17 +303,16 @@ class PgBackrestRestoreJob implements ShouldBeEncrypted, ShouldQueue } } - private function removePgDataBackup(string $backupPath): void + private function removePgDataBackup(string $backupVolumeName): void { try { $server = $this->database->destination->server; - $rmCmd = "docker run --rm -v {$backupPath}:/backup alpine sh -c 'rm -rf /backup/* /backup/.[!.]* /backup/..?* 2>/dev/null || true'"; - instant_remote_process([$rmCmd], $server, false, false, 300, disableMultiplexing: true); + instant_remote_process(["docker volume rm {$backupVolumeName} 2>/dev/null || true"], $server, false, false, 60, disableMultiplexing: true); - $this->restore->appendLog('Temporary backup removed.'); + $this->restore->appendLog('Temporary backup volume removed.'); } catch (Throwable $e) { - $this->restore->appendLog('Warning: Failed to remove temporary backup: '.$e->getMessage()); + $this->restore->appendLog('Warning: Failed to remove temporary backup volume: '.$e->getMessage()); } } diff --git a/app/Services/Backup/PgBackrestService.php b/app/Services/Backup/PgBackrestService.php index b940a2dd2..60630db84 100644 --- a/app/Services/Backup/PgBackrestService.php +++ b/app/Services/Backup/PgBackrestService.php @@ -192,6 +192,19 @@ BASH; ]; } + public static function buildDockerEnvArgs(array $envVars): string + { + $args = ''; + foreach ($envVars as $key => $value) { + if (! preg_match('/^[A-Z_][A-Z0-9_]*$/i', $key)) { + throw new \InvalidArgumentException("Invalid environment variable name: {$key}"); + } + $args .= ' -e '.escapeshellarg("{$key}={$value}"); + } + + return $args; + } + public static function buildBackupCommand( string $stanza, string $type = 'full', @@ -216,6 +229,38 @@ BASH; return $cmd; } + public static function wrapWithLockWait(string $command, int $maxWaitSeconds = 900, int $intervalSeconds = 10): string + { + if ($intervalSeconds <= 0) { + throw new \InvalidArgumentException('Interval seconds must be greater than 0'); + } + if ($maxWaitSeconds <= 0) { + throw new \InvalidArgumentException('Max wait seconds must be greater than 0'); + } + + $maxAttempts = (int) ceil($maxWaitSeconds / $intervalSeconds); + + return <<