fix(backups): sign mc requests with S3v4 and retry failed S3 uploads

app/Actions/Database/StartDatabaseImport.php

@@ -318,7 +318,7 @@ class StartDatabaseImport
     private function setMinioClientAlias(S3Storage $storage, Server $server, string $helper): void
     {
         $remoteCommand = ($server->isNonRoot() ? 'sudo ' : '').'docker exec -i '.escapeshellarg($helper)
-            .' mc alias set'.$storage->minioClientPathOption().' s3temp '.escapeshellarg((string) $storage->endpoint).' >/dev/null';
+            .' mc alias set'.$storage->minioClientAliasOptions().' s3temp '.escapeshellarg((string) $storage->endpoint).' >/dev/null';
         $timeout = (int) config('constants.ssh.command_timeout');
 
         SshRetryHandler::retry(

app/Jobs/DatabaseBackupJob.php

@@ -949,10 +949,10 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
 
             instant_remote_process($commands, $this->server, true, false, $this->commandTimeout(), disableMultiplexing: true);
             $this->setMinioClientAlias($this->server, "backup-of-{$this->backup_log_uuid}", $s3, $resolveOptions);
-            instant_remote_process(
+            S3Storage::retryMinioClientUpload(fn () => instant_remote_process(
                 ["docker exec backup-of-{$this->backup_log_uuid} mc cp{$resolveOptions} {$escapedBackupLocation} {$escapedS3Destination}"],
                 $this->server, true, false, $this->commandTimeout(), disableMultiplexing: true,
-            );
+            ));
 
         } catch (Throwable $e) {
             $this->add_to_error_output($e->getMessage());
@@ -970,7 +970,7 @@ class DatabaseBackupJob implements ShouldBeEncrypted, ShouldQueue
     private function setMinioClientAlias(Server $server, string $container, S3Storage $s3, string $resolveOptions): void
     {
         $remoteCommand = ($server->isNonRoot() ? 'sudo ' : '').'docker exec -i '.escapeshellarg($container)
-            .' mc alias set'.$resolveOptions.$s3->minioClientPathOption().' temporary '.escapeshellarg($s3->endpoint).' >/dev/null';
+            .' mc alias set'.$resolveOptions.$s3->minioClientAliasOptions().' temporary '.escapeshellarg($s3->endpoint).' >/dev/null';
         $timeout = (int) config('constants.ssh.command_timeout');
 
         SshRetryHandler::retry(

app/Jobs/VolumeBackupJob.php

@@ -449,10 +449,10 @@ class VolumeBackupJob implements ShouldBeEncrypted, ShouldQueue
                     .escapeshellarg($backupLocation.':'.$backupLocation.':ro').' '.escapeshellarg($image),
             ], $server, timeout: $this->timeout, disableMultiplexing: true);
             $this->setMinioClientAlias($server, $containerName, $s3, $resolveOptions);
-            instant_remote_process([
+            S3Storage::retryMinioClientUpload(fn () => instant_remote_process([
                 'docker exec '.escapeshellarg($containerName).' mc cp'.$resolveOptions.' '.escapeshellarg($backupLocation).' '
                     .escapeshellarg('temporary/'.$s3->bucket.$backupDirectory.'/'),
-            ], $server, timeout: $this->timeout, disableMultiplexing: true);
+            ], $server, timeout: $this->timeout, disableMultiplexing: true));
         } finally {
             instant_remote_process(
                 ['docker rm -f '.escapeshellarg($containerName)],
@@ -487,7 +487,7 @@ class VolumeBackupJob implements ShouldBeEncrypted, ShouldQueue
     private function setMinioClientAlias(Server $server, string $container, S3Storage $s3, string $resolveOptions): void
     {
         $remoteCommand = ($server->isNonRoot() ? 'sudo ' : '').'docker exec -i '.escapeshellarg($container)
-            .' mc alias set'.$resolveOptions.$s3->minioClientPathOption().' temporary '.escapeshellarg($s3->endpoint).' >/dev/null';
+            .' mc alias set'.$resolveOptions.$s3->minioClientAliasOptions().' temporary '.escapeshellarg($s3->endpoint).' >/dev/null';
         $timeout = (int) config('constants.ssh.command_timeout');
 
         SshRetryHandler::retry(

app/Models/S3Storage.php

@@ -34,6 +34,16 @@ class S3Storage extends BaseModel
      */
     private const PATH_STYLE_REJECTION_ERRORS = ['secondleveldomainforbidden', 'pathstyledomainforbidden', 'virtual hosted style', 'virtual-hosted style', 'virtual-styled'];
 
+    /**
+     * Waits before the second and the third attempt of an upload with the MinIO client.
+     */
+    private const MINIO_CLIENT_UPLOAD_RETRY_DELAYS_MS = [10_000, 30_000];
+
+    /**
+     * The exception code of runSshProcess() when a remote command times out.
+     */
+    private const REMOTE_COMMAND_TIMEOUT_EXIT_CODE = 124;
+
     protected $fillable = [
         'team_id',
         'name',
@@ -349,12 +359,27 @@ class S3Storage extends BaseModel
     }
 
     /**
-     * The `mc alias set` option that makes the MinIO client use virtual-hosted-style URLs, as filesystem() does.
-     * With path-style URLs, mc keeps its own `auto` lookup, as before.
+     * The `mc alias set` options. `--api S3v4` signs as filesystem() does. Without it, mc probes the signature
+     * version on every run, and a network error during the probe makes it try S3v2, which most providers reject.
+     * `--path off` uses virtual-hosted-style URLs, as filesystem() does. With path-style URLs, mc keeps its own
+     * `auto` lookup, as before.
      */
-    public function minioClientPathOption(): string
+    public function minioClientAliasOptions(): string
     {
-        return $this->usesPathStyleEndpoint() ? '' : ' --path off';
+        return ' --api S3v4'.($this->usesPathStyleEndpoint() ? '' : ' --path off');
+    }
+
+    /**
+     * Runs an upload of a local file with the MinIO client again when it fails, for example after a short network
+     * outage. A timeout is not retried, because another full attempt can pass the timeout of the backup job.
+     */
+    public static function retryMinioClientUpload(callable $upload): mixed
+    {
+        return retry(
+            self::MINIO_CLIENT_UPLOAD_RETRY_DELAYS_MS,
+            $upload,
+            when: fn (\Throwable $exception): bool => $exception->getCode() !== self::REMOTE_COMMAND_TIMEOUT_EXIT_CODE,
+        );
     }
 
     /**

tests/Feature/BackupS3CredentialsArgvTest.php

@@ -201,8 +201,8 @@ it('makes mc use virtual-hosted-style URLs when the storage turns path-style URL
     $aliasProcess = $processes->sole(fn (array $process) => str_contains($process['command'], 'mc alias set'));
     expect(s3ArgvTestRunAliasProcess($aliasProcess)['argv'])->toContain($expectedAlias);
 })->with([
-    'path-style' => [true, 'mc alias set temporary https://8.8.8.8'],
-    'virtual-hosted-style' => [false, 'mc alias set --path off temporary https://8.8.8.8'],
+    'path-style' => [true, 'mc alias set --api S3v4 temporary https://8.8.8.8'],
+    'virtual-hosted-style' => [false, 'mc alias set --api S3v4 --path off temporary https://8.8.8.8'],
 ]);
 
 it('passes the S3 keys of a volume backup to mc on stdin, not in a command line', function (string $sshUser, bool $streamToS3) {

tests/Feature/BackupS3DestinationRedactionTest.php

@@ -24,6 +24,7 @@ use Illuminate\Support\Facades\Event;
 use Illuminate\Support\Facades\Notification;
 use Illuminate\Support\Facades\Process;
 use Illuminate\Support\Facades\Storage;
+use Illuminate\Support\Sleep;
 
 uses(RefreshDatabase::class);
 
@@ -41,6 +42,7 @@ beforeEach(function () {
     defined('CURLOPT_RESOLVE') || define('CURLOPT_RESOLVE', 10203);
     Event::fake([BackupCreated::class]);
     Notification::fake();
+    Sleep::fake();
     InstanceSettings::unguarded(fn () => InstanceSettings::query()->firstOrCreate(['id' => 0]));
 
     $this->team = Team::factory()->create();

tests/Feature/StartDatabaseImportS3CredentialsTest.php

@@ -300,7 +300,7 @@ it('passes the S3 keys of an import to mc on stdin, not in a command line', func
     $run = importS3ArgvTestRunAliasProcess($aliasProcess);
 
     expect($run['mcStdin'])->toBe(IMPORT_S3_ARGV_KEY."\n".IMPORT_S3_ARGV_SECRET."\n")
-        ->and($run['argv'])->toContain('mc alias set s3temp https://8.8.8.8')
+        ->and($run['argv'])->toContain('mc alias set --api S3v4 s3temp https://8.8.8.8')
         ->and($run['argv'])->not->toContain(IMPORT_S3_ARGV_SECRET)
         ->and($run['argv'])->not->toContain(IMPORT_S3_ARGV_KEY)
         ->and($processes->contains(fn (array $process) => str_contains($process['command']."\n".$process['input'], "-e 'MC_REGION=ru-central-1'")))->toBeTrue()

tests/Feature/VolumeBackupMultipleS3DestinationsTest.php

@@ -22,12 +22,14 @@ use Illuminate\Support\Facades\Event;
 use Illuminate\Support\Facades\Notification;
 use Illuminate\Support\Facades\Process;
 use Illuminate\Support\Facades\Storage;
+use Illuminate\Support\Sleep;
 
 uses(RefreshDatabase::class);
 
 beforeEach(function () {
     Server::flushIdentityMap();
     Notification::fake();
+    Sleep::fake();
     config(['broadcasting.default' => 'null']);
     defined('CURLOPT_RESOLVE') || define('CURLOPT_RESOLVE', 10203);
 });
@@ -169,6 +171,28 @@ it('keeps the local archive and cleans the partial upload when one destination f
         ->and($secondReplica->message)->not->toBeEmpty();
 });
 
+it('uploads again when the first upload to a destination fails', function () {
+    [$backup] = createMultiDestinationVolumeBackup();
+    fakeMultiDestinationS3Disks();
+    $firstBucketUploads = 0;
+    Process::fake(function ($process) use (&$firstBucketUploads) {
+        $script = remoteProcessScript($process);
+        if (str_contains($script, 'mc cp') && str_contains($script, 'temporary/first-bucket/') && ++$firstBucketUploads === 1) {
+            return Process::result(errorOutput: 'dial tcp: connect: network is unreachable', exitCode: 1);
+        }
+
+        return Process::result(output: str_contains($script, 'du -b') ? '128' : '');
+    });
+
+    (new VolumeBackupJob($backup))->handle();
+
+    $execution = ScheduledVolumeBackupExecution::query()->sole();
+    expect($firstBucketUploads)->toBe(2)
+        ->and($execution->status)->toBe('success')
+        ->and($execution->message)->toBeNull()
+        ->and($execution->s3_uploaded)->toBeTrue();
+});
+
 it('applies S3 retention to each destination separately', function () {
     [$backup, $first, $second, $server] = createMultiDestinationVolumeBackup([
         'retention_amount_locally' => 0,

tests/Unit/S3StorageTest.php

@@ -3,6 +3,7 @@
 use App\Models\S3Storage;
 use Illuminate\Filesystem\FilesystemAdapter;
 use Illuminate\Support\Facades\Storage;
+use Illuminate\Support\Sleep;
 use Tests\TestCase;
 
 uses(TestCase::class);
@@ -114,11 +115,11 @@ test('S3Storage uses path-style URLs unless the storage turns them off', functio
     ]);
 
     expect($s3Storage->filesystem())->toBe($disk)
-        ->and($s3Storage->minioClientPathOption())->toBe($mcOption);
+        ->and($s3Storage->minioClientAliasOptions())->toBe($mcOption);
 })->with([
-    'not saved yet' => [null, true, ''],
-    'path-style' => [true, true, ''],
-    'virtual-hosted-style' => [false, false, ' --path off'],
+    'not saved yet' => [null, true, ' --api S3v4'],
+    'path-style' => [true, true, ' --api S3v4'],
+    'virtual-hosted-style' => [false, false, ' --api S3v4 --path off'],
 ]);
 
 test('S3Storage detects the URL style from the endpoint host', function (?string $endpoint, bool $usesPathStyle) {
@@ -227,3 +228,48 @@ test('S3Storage testConnection rejects invalid bucket before building client', f
     'newline' => ["lab\nid"],
     'underscore' => ['lab_bucket'],
 ]);
+
+test('S3Storage retries a failed MinIO client upload and waits longer before each attempt', function () {
+    Sleep::fake();
+    $attempts = 0;
+
+    $result = S3Storage::retryMinioClientUpload(function () use (&$attempts) {
+        if (++$attempts < 3) {
+            throw new RuntimeException('dial tcp: connect: network is unreachable', 1);
+        }
+
+        return 'uploaded';
+    });
+
+    expect($result)->toBe('uploaded')->and($attempts)->toBe(3);
+    Sleep::assertSequence([Sleep::for(10)->seconds(), Sleep::for(30)->seconds()]);
+});
+
+test('S3Storage stops retrying a MinIO client upload after three attempts', function () {
+    Sleep::fake();
+    $attempts = 0;
+
+    $upload = function () use (&$attempts) {
+        $attempts++;
+
+        throw new RuntimeException('Access Denied.', 1);
+    };
+
+    expect(fn () => S3Storage::retryMinioClientUpload($upload))->toThrow(RuntimeException::class, 'Access Denied.');
+    expect($attempts)->toBe(3);
+});
+
+test('S3Storage does not retry a MinIO client upload that timed out', function () {
+    Sleep::fake();
+    $attempts = 0;
+
+    $upload = function () use (&$attempts) {
+        $attempts++;
+
+        throw new RuntimeException('The command on the server timed out after 3600 seconds.', 124);
+    };
+
+    expect(fn () => S3Storage::retryMinioClientUpload($upload))->toThrow(RuntimeException::class, 'timed out');
+    expect($attempts)->toBe(1);
+    Sleep::assertNeverSlept();
+});