From 1039368491e413edce76ce8ca704f91d42a46ac0 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 27 Oct 2026 15:53:16 +0100 Subject: [PATCH] fix(s3): support virtual-hosted-style URLs for S3 storages Add a "Use path-style URLs" setting to S3 storages. It is detected from the endpoint (Alibaba OSS, Tencent COS, Volcengine/BytePlus TOS), and the connection test switches to virtual-hosted style when the provider rejects path-style requests. Backups pass `--path off` to mc. Closes #10347 --- app/Actions/Database/StartDatabaseImport.php | 2 +- .../Controllers/Api/S3StoragesController.php | 11 ++- app/Jobs/DatabaseBackupJob.php | 2 +- app/Jobs/VolumeBackupJob.php | 2 +- app/Livewire/Project/Database/ImportForm.php | 2 +- app/Livewire/Storage/Create.php | 15 ++++ app/Livewire/Storage/Form.php | 28 ++++++- app/Models/S3Storage.php | 83 ++++++++++++++++++- app/Services/FileDownloadStorageService.php | 2 +- .../ServerTransfer/ServerTransferExporter.php | 1 + .../ServerTransfer/ServerTransferImporter.php | 1 + bootstrap/helpers/databases.php | 2 +- ...th_style_endpoint_to_s3_storages_table.php | 22 +++++ openapi.json | 14 ++++ openapi.yaml | 9 +- .../views/livewire/storage/create.blade.php | 11 ++- .../views/livewire/storage/form.blade.php | 10 ++- tests/Feature/Api/S3StoragesApiTest.php | 32 +++++++ tests/Feature/BackupS3CredentialsArgvTest.php | 30 +++++++ .../Feature/S3StorageCreateErrorLinkTest.php | 16 ++++ tests/Feature/S3StorageFormTest.php | 2 + tests/Unit/S3StorageTest.php | 83 +++++++++++++++++++ .../S3StoragePathStyleDetectionTest.php | 35 ++++++++ 23 files changed, 396 insertions(+), 19 deletions(-) create mode 100644 database/migrations/2026_10_27_142752_add_use_path_style_endpoint_to_s3_storages_table.php create mode 100644 tests/v4/Browser/S3StoragePathStyleDetectionTest.php diff --git a/app/Actions/Database/StartDatabaseImport.php b/app/Actions/Database/StartDatabaseImport.php index 9d8add6b8f..d9e491345c 100644 --- a/app/Actions/Database/StartDatabaseImport.php +++ b/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 s3temp '.escapeshellarg((string) $storage->endpoint).' >/dev/null'; + .' mc alias set'.$storage->minioClientPathOption().' s3temp '.escapeshellarg((string) $storage->endpoint).' >/dev/null'; $timeout = (int) config('constants.ssh.command_timeout'); SshRetryHandler::retry( diff --git a/app/Http/Controllers/Api/S3StoragesController.php b/app/Http/Controllers/Api/S3StoragesController.php index c2ad48c51d..3aefa4f297 100644 --- a/app/Http/Controllers/Api/S3StoragesController.php +++ b/app/Http/Controllers/Api/S3StoragesController.php @@ -97,6 +97,7 @@ class S3StoragesController extends Controller 'endpoint' => ['type' => 'string'], 'bucket' => ['type' => 'string'], 'region' => ['type' => 'string'], + 'use_path_style_endpoint' => ['type' => 'boolean'], 'is_usable' => ['type' => 'boolean'], 'team_id' => ['type' => 'integer'], 'created_at' => ['type' => 'string'], @@ -160,6 +161,7 @@ class S3StoragesController extends Controller 'endpoint' => ['type' => 'string'], 'bucket' => ['type' => 'string'], 'region' => ['type' => 'string'], + 'use_path_style_endpoint' => ['type' => 'boolean'], 'is_usable' => ['type' => 'boolean'], 'team_id' => ['type' => 'integer'], 'created_at' => ['type' => 'string'], @@ -220,6 +222,7 @@ class S3StoragesController extends Controller 'endpoint' => ['type' => 'string', 'example' => 'https://s3.us-east-1.amazonaws.com', 'description' => 'S3-compatible endpoint URL.'], 'bucket' => ['type' => 'string', 'example' => 'my-bucket', 'description' => 'S3 bucket name.'], 'region' => ['type' => 'string', 'example' => 'us-east-1', 'description' => 'S3 region.'], + 'use_path_style_endpoint' => ['type' => 'boolean', 'description' => 'Use path-style URLs (endpoint/bucket). Set to false for providers that only accept virtual-hosted-style URLs (bucket.endpoint). Default: false for Alibaba Cloud OSS, Tencent Cloud COS, and Volcengine TOS endpoints, true for all other endpoints.'], 'key' => ['type' => 'string', 'description' => 'Access key.'], 'secret' => ['type' => 'string', 'description' => 'Secret key.'], 'is_usable' => ['type' => 'boolean', 'description' => 'Whether the storage is marked usable.'], @@ -258,7 +261,7 @@ class S3StoragesController extends Controller )] public function store(Request $request) { - $allowedFields = ['name', 'description', 'endpoint', 'bucket', 'region', 'key', 'secret', 'is_usable']; + $allowedFields = ['name', 'description', 'endpoint', 'bucket', 'region', 'key', 'secret', 'use_path_style_endpoint', 'is_usable']; $teamId = getTeamIdFromToken(); if (is_null($teamId)) { @@ -281,6 +284,7 @@ class S3StoragesController extends Controller 'region' => 'required|string|max:255', 'key' => 'required|string|max:255', 'secret' => 'required|string|max:255', + 'use_path_style_endpoint' => 'sometimes|boolean', 'is_usable' => 'sometimes|boolean', ]); if ($validationError instanceof JsonResponse) { @@ -296,6 +300,7 @@ class S3StoragesController extends Controller 'region' => $body['region'], 'key' => $body['key'], 'secret' => $body['secret'], + 'use_path_style_endpoint' => $body['use_path_style_endpoint'] ?? S3Storage::detectPathStyleEndpoint($body['endpoint']), 'is_usable' => $body['is_usable'] ?? false, ]); @@ -335,6 +340,7 @@ class S3StoragesController extends Controller 'endpoint' => ['type' => 'string', 'description' => 'S3-compatible endpoint URL.'], 'bucket' => ['type' => 'string', 'description' => 'S3 bucket name.'], 'region' => ['type' => 'string', 'description' => 'S3 region.'], + 'use_path_style_endpoint' => ['type' => 'boolean', 'description' => 'Use path-style URLs (endpoint/bucket). Set to false for providers that only accept virtual-hosted-style URLs (bucket.endpoint), such as Alibaba Cloud OSS.'], 'key' => ['type' => 'string', 'description' => 'Access key.'], 'secret' => ['type' => 'string', 'description' => 'Secret key.'], 'is_usable' => ['type' => 'boolean', 'description' => 'Whether the storage is marked usable.'], @@ -373,7 +379,7 @@ class S3StoragesController extends Controller )] public function update(Request $request) { - $allowedFields = ['name', 'description', 'endpoint', 'bucket', 'region', 'key', 'secret', 'is_usable']; + $allowedFields = ['name', 'description', 'endpoint', 'bucket', 'region', 'key', 'secret', 'use_path_style_endpoint', 'is_usable']; $teamId = getTeamIdFromToken(); if (is_null($teamId)) { @@ -395,6 +401,7 @@ class S3StoragesController extends Controller 'region' => 'sometimes|string|max:255', 'key' => 'sometimes|string|max:255', 'secret' => 'sometimes|string|max:255', + 'use_path_style_endpoint' => 'sometimes|boolean', 'is_usable' => 'sometimes|boolean', ]); if ($validationError instanceof JsonResponse) { diff --git a/app/Jobs/DatabaseBackupJob.php b/app/Jobs/DatabaseBackupJob.php index a05d713165..07e2fd56e3 100644 --- a/app/Jobs/DatabaseBackupJob.php +++ b/app/Jobs/DatabaseBackupJob.php @@ -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.' temporary '.escapeshellarg($s3->endpoint).' >/dev/null'; + .' mc alias set'.$resolveOptions.$s3->minioClientPathOption().' temporary '.escapeshellarg($s3->endpoint).' >/dev/null'; $timeout = (int) config('constants.ssh.command_timeout'); SshRetryHandler::retry( diff --git a/app/Jobs/VolumeBackupJob.php b/app/Jobs/VolumeBackupJob.php index 344cefdccc..b7524c7012 100644 --- a/app/Jobs/VolumeBackupJob.php +++ b/app/Jobs/VolumeBackupJob.php @@ -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.' temporary '.escapeshellarg($s3->endpoint).' >/dev/null'; + .' mc alias set'.$resolveOptions.$s3->minioClientPathOption().' temporary '.escapeshellarg($s3->endpoint).' >/dev/null'; $timeout = (int) config('constants.ssh.command_timeout'); SshRetryHandler::retry( diff --git a/app/Livewire/Project/Database/ImportForm.php b/app/Livewire/Project/Database/ImportForm.php index 7fe8c8abef..a1a6783b8e 100644 --- a/app/Livewire/Project/Database/ImportForm.php +++ b/app/Livewire/Project/Database/ImportForm.php @@ -648,7 +648,7 @@ class ImportForm extends Component 'secret' => $s3Storage->secret, 'bucket' => $s3Storage->bucket, 'endpoint' => $s3Storage->endpoint, - 'use_path_style_endpoint' => true, + 'use_path_style_endpoint' => $s3Storage->usesPathStyleEndpoint(), 'http' => SafeWebhookUrl::httpClientOptions($s3Storage->endpoint), ]); diff --git a/app/Livewire/Storage/Create.php b/app/Livewire/Storage/Create.php index 9e22e5491e..10dfb17678 100644 --- a/app/Livewire/Storage/Create.php +++ b/app/Livewire/Storage/Create.php @@ -29,6 +29,10 @@ class Create extends Component public string $endpoint = ''; + public bool $usePathStyleEndpoint = true; + + public bool $usePathStyleEndpointChanged = false; + public array $endpointParts = ['scheme' => 'https', 'host' => '', 'port' => '', 'path' => '']; public bool $endpointPartsChanged = false; @@ -45,6 +49,7 @@ class Create extends Component 'secret' => 'required|max:255', 'bucket' => ['required', new ValidS3BucketName], 'endpoint' => ['required', 'max:255', new SafeWebhookUrl], + 'usePathStyleEndpoint' => 'boolean', ]; } @@ -74,6 +79,7 @@ class Create extends Component 'secret' => 'Secret', 'bucket' => 'Bucket', 'endpoint' => 'Endpoint', + 'usePathStyleEndpoint' => 'Use path-style URLs', ]; public function submit() @@ -93,6 +99,7 @@ class Create extends Component $this->storage->key = $this->key; $this->storage->secret = $this->secret; $this->storage->bucket = $this->bucket; + $this->storage->use_path_style_endpoint = $this->usePathStyleEndpoint; if (empty($this->endpoint)) { $this->storage->endpoint = "https://s3.{$this->region}.amazonaws.com"; } else { @@ -112,6 +119,14 @@ class Create extends Component public function updatedEndpointParts(): void { $this->endpointPartsChanged = true; + if (! $this->usePathStyleEndpointChanged) { + $this->usePathStyleEndpoint = S3Storage::detectPathStyleEndpoint(DomainUrlParts::compose(...$this->endpointParts)); + } + } + + public function updatedUsePathStyleEndpoint(): void + { + $this->usePathStyleEndpointChanged = true; } private function connectionErrorDescription(\Throwable $exception): string diff --git a/app/Livewire/Storage/Form.php b/app/Livewire/Storage/Form.php index 3a80c4348f..4f5b01b576 100644 --- a/app/Livewire/Storage/Form.php +++ b/app/Livewire/Storage/Form.php @@ -33,6 +33,10 @@ class Form extends Component public string $region; + public bool $usePathStyleEndpoint = true; + + public bool $usePathStyleEndpointChanged = false; + public string $key; public string $secret; @@ -52,6 +56,7 @@ class Form extends Component 'secret' => 'required|max:255', 'bucket' => ['required', new ValidS3BucketName], 'endpoint' => ['required', 'max:255', new SafeWebhookUrl], + 'usePathStyleEndpoint' => 'boolean', ]; } @@ -82,6 +87,7 @@ class Form extends Component 'secret' => 'Secret', 'bucket' => 'Bucket', 'endpoint' => 'Endpoint', + 'usePathStyleEndpoint' => 'Use path-style URLs', ]; /** @@ -98,6 +104,7 @@ class Form extends Component $this->storage->endpoint = $this->endpoint; $this->storage->bucket = $this->bucket; $this->storage->region = $this->region; + $this->storage->use_path_style_endpoint = $this->usePathStyleEndpoint; $this->storage->key = $this->key; $this->storage->secret = $this->secret; $this->storage->is_usable = $this->isUsable; @@ -110,6 +117,9 @@ class Form extends Component $this->endpointPartsChanged = false; $this->bucket = $this->storage->bucket; $this->region = $this->storage->region; + $this->usePathStyleEndpoint = $this->storage->usesPathStyleEndpoint(); + // A saved style that detection would not pick is a user choice, so keep it when the endpoint changes. + $this->usePathStyleEndpointChanged = $this->usePathStyleEndpoint !== S3Storage::detectPathStyleEndpoint($this->endpoint); $canSeeCredentials = auth()->user()?->can('update', $this->storage) ?? false; $this->key = $canSeeCredentials ? $this->storage->key : ''; $this->secret = $canSeeCredentials ? $this->storage->secret : ''; @@ -142,10 +152,13 @@ class Form extends Component $testedStorage->endpoint = $this->endpoint; $testedStorage->bucket = $this->bucket; $testedStorage->region = $this->region; + $testedStorage->use_path_style_endpoint = $this->usePathStyleEndpoint; $testedStorage->key = $this->key; $testedStorage->secret = $this->secret; $testedStorage->testConnection(); + $switchedToVirtualHostedStyle = $this->usePathStyleEndpoint && ! $testedStorage->usesPathStyleEndpoint(); + $this->usePathStyleEndpoint = $testedStorage->usesPathStyleEndpoint(); // Update component property to reflect the new validation status $this->isUsable = $testedStorage->is_usable; @@ -154,7 +167,9 @@ class Form extends Component $this->storage->save(); $this->dispatch('storage-status-changed', isUsable: $this->isUsable); - return $this->dispatch('success', 'Connection is working.', 'Tested with "ListObjectsV2" action.'); + return $this->dispatch('success', 'Connection is working.', $switchedToVirtualHostedStyle + ? 'The provider rejects path-style URLs, so "Use path-style URLs" is now off. Save to keep it.' + : 'Tested with "ListObjectsV2" action.'); } catch (\Throwable $e) { if ($testedStorage) { $this->isUsable = $testedStorage->is_usable; @@ -192,8 +207,9 @@ class Form extends Component $this->storage->unusable_email_sent = false; $this->storage->save(); - // Update local property to reflect success + // Update local properties to reflect success $this->isUsable = true; + $this->usePathStyleEndpoint = $this->storage->usesPathStyleEndpoint(); }); $this->dispatch('name-changed', uuid: $this->storage->uuid, name: $this->storage->name); @@ -210,5 +226,13 @@ class Form extends Component public function updatedEndpointParts(): void { $this->endpointPartsChanged = true; + if (! $this->usePathStyleEndpointChanged) { + $this->usePathStyleEndpoint = S3Storage::detectPathStyleEndpoint(DomainUrlParts::compose(...$this->endpointParts)); + } + } + + public function updatedUsePathStyleEndpoint(): void + { + $this->usePathStyleEndpointChanged = true; } } diff --git a/app/Models/S3Storage.php b/app/Models/S3Storage.php index 9857b4faf2..6bd9f88322 100644 --- a/app/Models/S3Storage.php +++ b/app/Models/S3Storage.php @@ -23,6 +23,17 @@ class S3Storage extends BaseModel private const REQUEST_TIMEOUT_SECONDS = 15; + /** + * Endpoint host suffixes of providers that reject path-style URLs: Alibaba Cloud OSS, Tencent Cloud COS + * (buckets created after 2024-01-01), and Volcengine / BytePlus TOS. + */ + private const VIRTUAL_HOSTED_STYLE_HOST_SUFFIXES = ['aliyuncs.com', 'myqcloud.com', 'volces.com', 'bytepluses.com']; + + /** + * Parts of the errors that these providers return for a path-style request. + */ + private const PATH_STYLE_REJECTION_ERRORS = ['secondleveldomainforbidden', 'pathstyledomainforbidden', 'virtual hosted style', 'virtual-hosted style', 'virtual-styled']; + protected $fillable = [ 'team_id', 'name', @@ -32,6 +43,7 @@ class S3Storage extends BaseModel 'secret', 'bucket', 'endpoint', + 'use_path_style_endpoint', 'is_usable', 'unusable_email_sent', ]; @@ -43,6 +55,7 @@ class S3Storage extends BaseModel protected $casts = [ 'is_usable' => 'boolean', + 'use_path_style_endpoint' => 'boolean', 'key' => 'encrypted', 'secret' => 'encrypted', ]; @@ -217,9 +230,7 @@ class S3Storage extends BaseModel throw new \RuntimeException('S3 bucket name is not allowed: '.$validator->errors()->first('bucket')); } - $disk = $this->filesystem(); - // Test the connection by listing files with ListObjectsV2 (S3) - $disk->files(); + $this->listFilesWithAddressingStyleFallback(); $this->unusable_email_sent = false; $this->is_usable = true; @@ -265,7 +276,7 @@ class S3Storage extends BaseModel 'secret' => $this['secret'], 'bucket' => $this['bucket'], 'endpoint' => $this['endpoint'], - 'use_path_style_endpoint' => true, + 'use_path_style_endpoint' => $this->usesPathStyleEndpoint(), 'http' => array_merge(SafeWebhookUrl::httpClientOptions($this['endpoint'], $this->trustedInternalHosts()), [ 'connect_timeout' => self::CONNECTION_TIMEOUT_SECONDS, 'timeout' => self::REQUEST_TIMEOUT_SECONDS, @@ -282,6 +293,70 @@ class S3Storage extends BaseModel return '-e '.escapeshellarg('MC_REGION='.$this->region); } + /** + * Path-style URLs (`endpoint/bucket`) are the default. Providers like Alibaba Cloud OSS only accept + * virtual-hosted-style URLs (`bucket.endpoint`). A storage that is not saved yet has no value, so it uses the default. + */ + public function usesPathStyleEndpoint(): bool + { + return $this->use_path_style_endpoint ?? true; + } + + /** + * The URL style for a new storage: virtual-hosted style for providers that reject path-style URLs, else path-style. + */ + public static function detectPathStyleEndpoint(?string $endpoint): bool + { + $host = strtolower((string) parse_url((string) $endpoint, PHP_URL_HOST)); + + return ! collect(self::VIRTUAL_HOSTED_STYLE_HOST_SUFFIXES) + ->contains(fn (string $suffix): bool => $host === $suffix || str_ends_with($host, '.'.$suffix)); + } + + /** + * Tests the connection with ListObjectsV2. When the provider rejects path-style URLs, it tests again with + * virtual-hosted-style URLs and keeps that style when it works. + */ + private function listFilesWithAddressingStyleFallback(): void + { + try { + $this->filesystem()->files(); + } catch (\Throwable $exception) { + if (! $this->usesPathStyleEndpoint() || ! self::isPathStyleRejection($exception)) { + throw $exception; + } + + $this->use_path_style_endpoint = false; + try { + $this->filesystem()->files(); + } catch (\Throwable) { + $this->use_path_style_endpoint = true; + + throw $exception; + } + } + } + + private static function isPathStyleRejection(\Throwable $exception): bool + { + for ($current = $exception; $current !== null; $current = $current->getPrevious()) { + if (str($current->getMessage())->lower()->contains(self::PATH_STYLE_REJECTION_ERRORS)) { + return true; + } + } + + return false; + } + + /** + * 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. + */ + public function minioClientPathOption(): string + { + return $this->usesPathStyleEndpoint() ? '' : ' --path off'; + } + /** * The bundled MinIO container is a trusted internal S3 target, not a user-supplied webhook destination. * diff --git a/app/Services/FileDownloadStorageService.php b/app/Services/FileDownloadStorageService.php index f342873c9c..578afb36ae 100644 --- a/app/Services/FileDownloadStorageService.php +++ b/app/Services/FileDownloadStorageService.php @@ -119,7 +119,7 @@ class FileDownloadStorageService 'version' => 'latest', 'region' => $storage->region, 'endpoint' => $storage->endpoint, - 'use_path_style_endpoint' => true, + 'use_path_style_endpoint' => $storage->usesPathStyleEndpoint(), 'credentials' => false, 'retries' => 0, 'http' => array_merge(SafeWebhookUrl::httpClientOptions($storage->endpoint, $storage->trustedInternalHosts()), [ diff --git a/app/Services/ServerTransfer/ServerTransferExporter.php b/app/Services/ServerTransfer/ServerTransferExporter.php index 04181d22d3..66254ae647 100644 --- a/app/Services/ServerTransfer/ServerTransferExporter.php +++ b/app/Services/ServerTransfer/ServerTransferExporter.php @@ -1488,6 +1488,7 @@ class ServerTransferExporter 'secret' => $storage->secret, 'bucket' => $storage->bucket, 'endpoint' => $storage->endpoint, + 'use_path_style_endpoint' => $storage->usesPathStyleEndpoint(), 'is_usable' => (bool) $storage->is_usable, ]; } diff --git a/app/Services/ServerTransfer/ServerTransferImporter.php b/app/Services/ServerTransfer/ServerTransferImporter.php index 9cdb066eee..7362160471 100644 --- a/app/Services/ServerTransfer/ServerTransferImporter.php +++ b/app/Services/ServerTransfer/ServerTransferImporter.php @@ -2007,6 +2007,7 @@ class ServerTransferImporter 'secret' => data_get($payload, 'secret'), 'bucket' => data_get($payload, 'bucket'), 'endpoint' => data_get($payload, 'endpoint'), + 'use_path_style_endpoint' => (bool) data_get($payload, 'use_path_style_endpoint', true), 'is_usable' => (bool) data_get($payload, 'is_usable', true), ]); $storage->uuid = $uuid; diff --git a/bootstrap/helpers/databases.php b/bootstrap/helpers/databases.php index a34fdde9a8..14b1047545 100644 --- a/bootstrap/helpers/databases.php +++ b/bootstrap/helpers/databases.php @@ -282,7 +282,7 @@ function deleteBackupsS3(string|array|null $filenames, S3Storage $s3): void 'region' => $s3->region, 'bucket' => $s3->bucket, 'endpoint' => $s3->endpoint, - 'use_path_style_endpoint' => true, + 'use_path_style_endpoint' => $s3->usesPathStyleEndpoint(), 'aws_url' => $s3->awsUrl(), 'throw' => true, ]); diff --git a/database/migrations/2026_10_27_142752_add_use_path_style_endpoint_to_s3_storages_table.php b/database/migrations/2026_10_27_142752_add_use_path_style_endpoint_to_s3_storages_table.php new file mode 100644 index 0000000000..1c66f52b18 --- /dev/null +++ b/database/migrations/2026_10_27_142752_add_use_path_style_endpoint_to_s3_storages_table.php @@ -0,0 +1,22 @@ +boolean('use_path_style_endpoint')->default(true); + }); + } + + public function down(): void + { + Schema::table('s3_storages', function (Blueprint $table) { + $table->dropColumn('use_path_style_endpoint'); + }); + } +}; diff --git a/openapi.json b/openapi.json index 999cdb2e2a..6b1ce726f5 100644 --- a/openapi.json +++ b/openapi.json @@ -14720,6 +14720,9 @@ "region": { "type": "string" }, + "use_path_style_endpoint": { + "type": "boolean" + }, "is_usable": { "type": "boolean" }, @@ -14799,6 +14802,10 @@ "example": "us-east-1", "description": "S3 region." }, + "use_path_style_endpoint": { + "type": "boolean", + "description": "Use path-style URLs (endpoint\/bucket). Set to false for providers that only accept virtual-hosted-style URLs (bucket.endpoint). Default: false for Alibaba Cloud OSS, Tencent Cloud COS, and Volcengine TOS endpoints, true for all other endpoints." + }, "key": { "type": "string", "description": "Access key." @@ -14897,6 +14904,9 @@ "region": { "type": "string" }, + "use_path_style_endpoint": { + "type": "boolean" + }, "is_usable": { "type": "boolean" }, @@ -15022,6 +15032,10 @@ "type": "string", "description": "S3 region." }, + "use_path_style_endpoint": { + "type": "boolean", + "description": "Use path-style URLs (endpoint\/bucket). Set to false for providers that only accept virtual-hosted-style URLs (bucket.endpoint), such as Alibaba Cloud OSS." + }, "key": { "type": "string", "description": "Access key." diff --git a/openapi.yaml b/openapi.yaml index c3872be12a..85f5c49369 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -9407,7 +9407,7 @@ paths: schema: type: array items: - properties: { uuid: { type: string }, name: { type: string }, description: { type: string, nullable: true }, endpoint: { type: string }, bucket: { type: string }, region: { type: string }, is_usable: { type: boolean }, team_id: { type: integer }, created_at: { type: string }, updated_at: { type: string } } + properties: { uuid: { type: string }, name: { type: string }, description: { type: string, nullable: true }, endpoint: { type: string }, bucket: { type: string }, region: { type: string }, use_path_style_endpoint: { type: boolean }, is_usable: { type: boolean }, team_id: { type: integer }, created_at: { type: string }, updated_at: { type: string } } type: object '401': $ref: '#/components/responses/401' @@ -9456,6 +9456,9 @@ paths: type: string example: us-east-1 description: 'S3 region.' + use_path_style_endpoint: + type: boolean + description: 'Use path-style URLs (endpoint/bucket). Set to false for providers that only accept virtual-hosted-style URLs (bucket.endpoint). Default: false for Alibaba Cloud OSS, Tencent Cloud COS, and Volcengine TOS endpoints, true for all other endpoints.' key: type: string description: 'Access key.' @@ -9512,6 +9515,7 @@ paths: endpoint: { type: string } bucket: { type: string } region: { type: string } + use_path_style_endpoint: { type: boolean } is_usable: { type: boolean } team_id: { type: integer } created_at: { type: string } @@ -9591,6 +9595,9 @@ paths: region: type: string description: 'S3 region.' + use_path_style_endpoint: + type: boolean + description: 'Use path-style URLs (endpoint/bucket). Set to false for providers that only accept virtual-hosted-style URLs (bucket.endpoint), such as Alibaba Cloud OSS.' key: type: string description: 'Access key.' diff --git a/resources/views/livewire/storage/create.blade.php b/resources/views/livewire/storage/create.blade.php index d583fcd268..7cab49ab88 100644 --- a/resources/views/livewire/storage/create.blade.php +++ b/resources/views/livewire/storage/create.blade.php @@ -7,8 +7,11 @@ - + {{-- Sends the endpoint right away, so the URL style below follows the provider. --}} +
+ +
+
+ +
Validate Connection & Continue diff --git a/resources/views/livewire/storage/form.blade.php b/resources/views/livewire/storage/form.blade.php index d13d632fbe..2161c4a5b8 100644 --- a/resources/views/livewire/storage/form.blade.php +++ b/resources/views/livewire/storage/form.blade.php @@ -17,8 +17,11 @@
@can('update', $storage) - + {{-- Sends the endpoint right away, so the URL style below follows the provider. --}} +
+ +
@else @endcan @@ -34,6 +37,9 @@ @endif +
diff --git a/tests/Feature/Api/S3StoragesApiTest.php b/tests/Feature/Api/S3StoragesApiTest.php index 5b0f9d6da9..86cdec4704 100644 --- a/tests/Feature/Api/S3StoragesApiTest.php +++ b/tests/Feature/Api/S3StoragesApiTest.php @@ -261,6 +261,25 @@ describe('POST /api/v1/s3-storages', function () { ]); }); + test('stores the given URL style, or detects it from the endpoint', function () { + $headers = ['Authorization' => 'Bearer '.$this->bearerToken, 'Content-Type' => 'application/json']; + + $this->withHeaders($headers)->postJson('/api/v1/s3-storages', validS3StoragePayload(['name' => 'Default']))->assertStatus(201); + $this->withHeaders($headers)->postJson('/api/v1/s3-storages', validS3StoragePayload([ + 'name' => 'Alibaba', + 'use_path_style_endpoint' => false, + ]))->assertStatus(201); + + $this->withHeaders($headers)->postJson('/api/v1/s3-storages', validS3StoragePayload([ + 'name' => 'Detected', + 'endpoint' => 'https://s3.oss-ap-southeast-1.aliyuncs.com', + ]))->assertStatus(201); + + expect(S3Storage::where('name', 'Default')->sole()->use_path_style_endpoint)->toBeTrue() + ->and(S3Storage::where('name', 'Alibaba')->sole()->use_path_style_endpoint)->toBeFalse() + ->and(S3Storage::where('name', 'Detected')->sole()->use_path_style_endpoint)->toBeFalse(); + }); + test('validates name is required', function () { $payload = validS3StoragePayload(); unset($payload['name']); @@ -377,6 +396,19 @@ describe('PATCH /api/v1/s3-storages/{uuid}', function () { ]); }); + test('updates the path-style setting', function () { + $storage = createS3StorageForTeam($this->team); + + $this->withHeaders([ + 'Authorization' => 'Bearer '.$this->bearerToken, + 'Content-Type' => 'application/json', + ])->patchJson("/api/v1/s3-storages/{$storage->uuid}", [ + 'use_path_style_endpoint' => false, + ])->assertStatus(200); + + expect($storage->fresh()->use_path_style_endpoint)->toBeFalse(); + }); + test('rejects empty body', function () { $storage = createS3StorageForTeam($this->team); diff --git a/tests/Feature/BackupS3CredentialsArgvTest.php b/tests/Feature/BackupS3CredentialsArgvTest.php index 0cdc63c214..5a2775751b 100644 --- a/tests/Feature/BackupS3CredentialsArgvTest.php +++ b/tests/Feature/BackupS3CredentialsArgvTest.php @@ -175,6 +175,36 @@ it('passes the S3 keys of a database backup upload to mc on stdin, not in a comm s3ArgvTestAssertSecretOnlyOnStdin($processes, $sshUser !== 'root'); })->with(['root', 'deploy']); +it('makes mc use virtual-hosted-style URLs when the storage turns path-style URLs off', function (bool $usePathStyleEndpoint, string $expectedAlias) { + [$team, $server, $storage] = s3ArgvTestSetup('root'); + $storage->update(['use_path_style_endpoint' => $usePathStyleEndpoint]); + $destination = StandaloneDocker::where('server_id', $server->id)->firstOrFail(); + $environment = Environment::factory()->create(['project_id' => Project::factory()->create(['team_id' => $team->id])->id]); + $database = create_standalone_postgresql($environment->id, $destination, ['status' => 'running:healthy']); + $backup = new ScheduledDatabaseBackup; + $backup->forceFill([ + 'enabled' => true, + 'save_s3' => true, + 'disable_local_backup' => true, + 's3_storage_id' => $storage->id, + 'frequency' => 'daily', + 'databases_to_backup' => 'app', + 'database_id' => $database->id, + 'database_type' => $database->getMorphClass(), + 'team_id' => $team->id, + ])->save(); + $backup->syncS3Storages([$storage->id]); + $processes = s3ArgvTestFakeSsh(); + + (new DatabaseBackupJob($backup->fresh()))->handle(); + + $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'], +]); + it('passes the S3 keys of a volume backup to mc on stdin, not in a command line', function (string $sshUser, bool $streamToS3) { [$team, $server, $storage] = s3ArgvTestSetup($sshUser); $destination = StandaloneDocker::where('server_id', $server->id)->firstOrFail(); diff --git a/tests/Feature/S3StorageCreateErrorLinkTest.php b/tests/Feature/S3StorageCreateErrorLinkTest.php index 2fceeb7e92..5644448bba 100644 --- a/tests/Feature/S3StorageCreateErrorLinkTest.php +++ b/tests/Feature/S3StorageCreateErrorLinkTest.php @@ -1,6 +1,7 @@ toContain('Set them here.') ->not->toContain('targets: '.$settingsUrl); }); + +it('turns path-style URLs off when the endpoint belongs to a provider that rejects them', function () { + Livewire::test(Create::class) + ->set('endpointParts.host', 's3.oss-ap-southeast-1.aliyuncs.com') + ->assertSet('usePathStyleEndpoint', false) + ->set('endpointParts.host', 'minio.internal') + ->assertSet('usePathStyleEndpoint', true); +}); + +it('keeps the URL style that the user selected when the endpoint changes', function () { + Livewire::test(Create::class) + ->set('usePathStyleEndpoint', true) + ->set('endpointParts.host', 's3.oss-ap-southeast-1.aliyuncs.com') + ->assertSet('usePathStyleEndpoint', true); +}); diff --git a/tests/Feature/S3StorageFormTest.php b/tests/Feature/S3StorageFormTest.php index c69cd293f1..f99db9775d 100644 --- a/tests/Feature/S3StorageFormTest.php +++ b/tests/Feature/S3StorageFormTest.php @@ -55,6 +55,7 @@ it('tests the S3 connection with the values currently entered in the form', func $form->region = 'eu-central-1'; $form->key = 'new-key'; $form->secret = 'new-secret'; + $form->usePathStyleEndpoint = false; $form->isUsable = false; $form->testConnection(); @@ -67,6 +68,7 @@ it('tests the S3 connection with the values currently entered in the form', func 'region' => 'eu-central-1', 'key' => 'new-key', 'secret' => 'new-secret', + 'use_path_style_endpoint' => false, ]) ->and($form->isUsable)->toBeTrue(); }); diff --git a/tests/Unit/S3StorageTest.php b/tests/Unit/S3StorageTest.php index 7a9d889ef3..2778d0a741 100644 --- a/tests/Unit/S3StorageTest.php +++ b/tests/Unit/S3StorageTest.php @@ -62,6 +62,7 @@ test('S3Storage model fillable attributes are configured correctly', function () 'secret', 'bucket', 'endpoint', + 'use_path_style_endpoint', 'is_usable', 'unusable_email_sent', ]); @@ -97,6 +98,88 @@ test('S3Storage connection validation uses short s3 client timeouts', function ( expect($s3Storage->is_usable)->toBeTrue(); }); +test('S3Storage uses path-style URLs unless the storage turns them off', function (?bool $setting, bool $expected, string $mcOption) { + $disk = Mockery::mock(FilesystemAdapter::class); + Storage::expects('build') + ->once() + ->with(Mockery::on(fn (array $config) => $config['use_path_style_endpoint'] === $expected)) + ->andReturn($disk); + + $s3Storage = new S3Storage; + $s3Storage->setRawAttributes([ + 'region' => 'oss-ap-southeast-1', + 'bucket' => 'backups', + 'endpoint' => 'https://s3.oss-ap-southeast-1.aliyuncs.com', + 'use_path_style_endpoint' => $setting, + ]); + + expect($s3Storage->filesystem())->toBe($disk) + ->and($s3Storage->minioClientPathOption())->toBe($mcOption); +})->with([ + 'not saved yet' => [null, true, ''], + 'path-style' => [true, true, ''], + 'virtual-hosted-style' => [false, false, ' --path off'], +]); + +test('S3Storage detects the URL style from the endpoint host', function (?string $endpoint, bool $usesPathStyle) { + expect(S3Storage::detectPathStyleEndpoint($endpoint))->toBe($usesPathStyle); +})->with([ + 'Alibaba Cloud OSS' => ['https://s3.oss-ap-southeast-1-internal.aliyuncs.com', false], + 'Tencent Cloud COS' => ['https://cos.ap-guangzhou.myqcloud.com', false], + 'Volcengine TOS' => ['https://tos-s3-cn-beijing.volces.com', false], + 'BytePlus TOS' => ['https://tos-s3-ap-southeast-1.bytepluses.com', false], + 'AWS' => ['https://s3.us-east-1.amazonaws.com', true], + 'look-alike host' => ['https://notaliyuncs.com', true], + 'MinIO by IP' => ['http://192.168.1.50:9000', true], + 'empty' => [null, true], +]); + +test('S3Storage connection validation switches to virtual-hosted-style URLs when the provider rejects path-style URLs', function () { + $pathStyleDisk = Mockery::mock(FilesystemAdapter::class); + $pathStyleDisk->expects('files')->once()->andThrow(new RuntimeException( + 'Unable to list contents', + previous: new RuntimeException('AWS HTTP error: SecondLevelDomainForbidden (client): Please use virtual hosted style to access.'), + )); + $virtualHostedDisk = Mockery::mock(FilesystemAdapter::class); + $virtualHostedDisk->expects('files')->once()->andReturn([]); + Storage::expects('build')->with(Mockery::on(fn (array $config) => $config['use_path_style_endpoint'] === true))->once()->andReturn($pathStyleDisk); + Storage::expects('build')->with(Mockery::on(fn (array $config) => $config['use_path_style_endpoint'] === false))->once()->andReturn($virtualHostedDisk); + + $s3Storage = new S3Storage; + $s3Storage->setRawAttributes(['region' => 'us-east-1', 'bucket' => 'backups', 'endpoint' => 'https://s3.amazonaws.com']); + + $s3Storage->testConnection(); + + expect($s3Storage->is_usable)->toBeTrue() + ->and($s3Storage->usesPathStyleEndpoint())->toBeFalse(); +}); + +test('S3Storage connection validation keeps path-style URLs when the virtual-hosted-style test also fails', function () { + $pathStyleError = new RuntimeException('AWS HTTP error: PathStyleDomainForbidden (client): use a virtual-styled domain'); + $disk = Mockery::mock(FilesystemAdapter::class); + $disk->expects('files')->twice()->andThrow($pathStyleError, new RuntimeException('AccessDenied')); + Storage::expects('build')->twice()->andReturn($disk); + + $s3Storage = new S3Storage; + $s3Storage->setRawAttributes(['region' => 'us-east-1', 'bucket' => 'backups', 'endpoint' => 'https://s3.amazonaws.com']); + + expect(fn () => $s3Storage->testConnection())->toThrow(RuntimeException::class, 'PathStyleDomainForbidden') + ->and($s3Storage->usesPathStyleEndpoint())->toBeTrue() + ->and($s3Storage->is_usable)->toBeFalse(); +}); + +test('S3Storage connection validation does not retry other errors', function () { + $disk = Mockery::mock(FilesystemAdapter::class); + $disk->expects('files')->once()->andThrow(new RuntimeException('AccessDenied')); + Storage::expects('build')->once()->andReturn($disk); + + $s3Storage = new S3Storage; + $s3Storage->setRawAttributes(['region' => 'us-east-1', 'bucket' => 'backups', 'endpoint' => 'https://s3.amazonaws.com']); + + expect(fn () => $s3Storage->testConnection())->toThrow(RuntimeException::class, 'AccessDenied') + ->and($s3Storage->usesPathStyleEndpoint())->toBeTrue(); +}); + test('S3Storage connection validation returns friendly timeout error', function () { $disk = Mockery::mock(FilesystemAdapter::class); $disk->expects('files') diff --git a/tests/v4/Browser/S3StoragePathStyleDetectionTest.php b/tests/v4/Browser/S3StoragePathStyleDetectionTest.php new file mode 100644 index 0000000000..f3a80d2469 --- /dev/null +++ b/tests/v4/Browser/S3StoragePathStyleDetectionTest.php @@ -0,0 +1,35 @@ +set('app.maintenance.store', 'array'); + seedBrowserResourceStack(); +}); + +it('turns path-style URLs off when the user enters the endpoint of a provider that rejects them', function () { + $storage = S3Storage::create([ + 'team_id' => 0, + 'name' => 'Backups', + 'region' => 'us-east-1', + 'key' => 'key', + 'secret' => 'secret', + 'bucket' => 'backups', + 'endpoint' => 'https://minio.example.com', + ]); + loginAndSkipBoarding(); + + $page = visit(route('storage.show', ['storage_uuid' => $storage->uuid])); + $pathStyleChecked = fn () => $page->script("document.querySelector('form.application-settings-form input[wire\\\\:model=\"usePathStyleEndpoint\"]').checked"); + expect($pathStyleChecked())->toBeTrue(); + + $page->fill('form.application-settings-form #endpointParts-host', 's3.oss-ap-southeast-1.aliyuncs.com') + ->click('form.application-settings-form label[for="endpointParts-host"]') + ->wait(1); + + expect($pathStyleChecked())->toBeFalse(); + $page->screenshot(filename: 's3-storage-path-style-detected'); +});