From 5639c494a95c37df6e78c0ae602a532ecf2ca2aa Mon Sep 17 00:00:00 2001 From: bugclerk <40872210+bugclerk@users.noreply.github.com> Date: Thu, 26 Sep 2024 07:28:09 -0700 Subject: [PATCH] NAS-131330 / 24.10.0 / Better error handling for bad storj bucket name (by creatorcary) (#14582) --- .../middlewared/plugins/cloud_sync.py | 8 +++- .../middlewared/rclone/remote/storjix.py | 13 +++++ tests/api2/test_cloud_sync_storj.py | 47 ++++++++++--------- 3 files changed, 44 insertions(+), 24 deletions(-) diff --git a/src/middlewared/middlewared/plugins/cloud_sync.py b/src/middlewared/middlewared/plugins/cloud_sync.py index acb98edd9d59..2c1e6aaf7262 100644 --- a/src/middlewared/middlewared/plugins/cloud_sync.py +++ b/src/middlewared/middlewared/plugins/cloud_sync.py @@ -10,9 +10,10 @@ from middlewared.plugins.cloud.model import CloudTaskModelMixin, cloud_task_schema from middlewared.plugins.cloud.path import get_remote_path, check_local_path from middlewared.plugins.cloud.remotes import REMOTES, remote_classes +from middlewared.rclone.remote.storjix import StorjIxError from middlewared.schema import accepts, Bool, Cron, Dict, Int, Password, Patch, Str from middlewared.service import ( - CallError, CRUDService, ValidationErrors, item_method, job, pass_app, private, TaskPathService, + CallError, CRUDService, ValidationError, ValidationErrors, item_method, job, pass_app, private, TaskPathService, ) import middlewared.sqlalchemy as sa from middlewared.utils import Popen, run @@ -915,7 +916,10 @@ async def create_bucket(self, credentials_id, name): if not provider.can_create_bucket: raise CallError("This provider can't create buckets") - await provider.create_bucket(credentials, name) + try: + await provider.create_bucket(credentials, name) + except StorjIxError as e: + raise ValidationError("cloudsync.create_bucket", e.errmsg, e.errno) @accepts(Int("credentials_id"), roles=["CLOUD_SYNC_WRITE"]) async def list_buckets(self, credentials_id): diff --git a/src/middlewared/middlewared/rclone/remote/storjix.py b/src/middlewared/middlewared/rclone/remote/storjix.py index 32bc3487f0aa..5ea355f8bd8b 100644 --- a/src/middlewared/middlewared/rclone/remote/storjix.py +++ b/src/middlewared/middlewared/rclone/remote/storjix.py @@ -13,6 +13,10 @@ from middlewared.utils.network import INTERNET_TIMEOUT +class StorjIxError(CallError): + pass + + class StorjIxRcloneRemote(BaseRcloneRemote): name = "STORJ_IX" title = "Storj iX" @@ -41,10 +45,19 @@ def create_bucket_sync(): aws_access_key_id=credentials["attributes"]["access_key_id"], aws_secret_access_key=credentials["attributes"]["secret_access_key"], ) + # s3 bucket naming rules: https://docs.aws.amazon.com/AmazonS3/latest/userguide/bucketnamingrules.html try: s3_client.create_bucket(Bucket=name) except s3_client.exceptions.BucketAlreadyExists as e: raise CallError(str(e), errno=errno.EEXIST) + except botocore.exceptions.ParamValidationError as e: + raise StorjIxError("The bucket name can only contain lowercase letters, numbers, and hyphens.", + errno.EINVAL, str(e)) + except botocore.exceptions.ClientError as e: + if "InvalidBucketName" in e.args[0]: + raise StorjIxError("The bucket name must be between 3-63 characters in length and cannot contain " + "uppercase.", errno.EINVAL, str(e)) + raise return await self.middleware.run_in_thread(create_bucket_sync) diff --git a/tests/api2/test_cloud_sync_storj.py b/tests/api2/test_cloud_sync_storj.py index ae4fac02158c..5c45be4b264a 100644 --- a/tests/api2/test_cloud_sync_storj.py +++ b/tests/api2/test_cloud_sync_storj.py @@ -1,27 +1,14 @@ -import os -import sys - import pytest +from config import ( + STORJ_IX_AWS_ACCESS_KEY_ID, + STORJ_IX_AWS_SECRET_ACCESS_KEY, + STORJ_IX_BUCKET, +) from middlewared.test.integration.utils import call, ssh from middlewared.test.integration.assets.cloud_sync import credential, task, run_task from middlewared.test.integration.assets.pool import dataset -apifolder = os.getcwd() -sys.path.append(apifolder) - -pytestmark = pytest.mark.skip(reason='See IT ticket IT-9829') -try: - from config import ( - STORJ_IX_AWS_ACCESS_KEY_ID, - STORJ_IX_AWS_SECRET_ACCESS_KEY, - STORJ_IX_BUCKET, - ) -except ImportError: - pytestmark = pytest.mark.skip(reason='Storj credential are missing in config.py') - STORJ_IX_AWS_ACCESS_KEY_ID = None - STORJ_IX_AWS_SECRET_ACCESS_KEY = None - STORJ_IX_BUCKET = None CREDENTIAL = { "provider": "STORJ_IX", @@ -34,6 +21,7 @@ "bucket": STORJ_IX_BUCKET, "folder": "", } +FILENAME = "a" def test_storj_verify(): @@ -58,16 +46,31 @@ def test_storj_list_buckets(storj_credential): assert any(item["Name"] == STORJ_IX_BUCKET for item in call("cloudsync.list_buckets", storj_credential["id"])) -def test_storj_list_directory(storj_credential): +@pytest.fixture(scope="module") +def storj_sync(storj_credential): + """Reset the remote bucket to only contain a single empty file.""" + with dataset("test_storj_sync") as ds: + ssh(f"touch /mnt/{ds}/{FILENAME}") + with task({ + "direction": "PUSH", + "transfer_mode": "SYNC", + "path": f"/mnt/{ds}", + "credentials": storj_credential["id"], + "attributes": TASK_ATTRIBUTES, + }) as t: + run_task(t) + + +def test_storj_list_directory(storj_credential, storj_sync): result = call("cloudsync.list_directory", { "credentials": storj_credential["id"], "attributes": TASK_ATTRIBUTES, }) assert len(result) == 1 - assert result[0]["Name"] == "a" + assert result[0]["Name"] == FILENAME -def test_storj_sync(storj_credential): +def test_storj_pull(storj_credential, storj_sync): with dataset("test_storj_sync") as ds: with task({ "direction": "PULL", @@ -78,4 +81,4 @@ def test_storj_sync(storj_credential): }) as t: run_task(t) - assert ssh(f"ls /mnt/{ds}") == "a\n" + assert ssh(f"ls /mnt/{ds}") == FILENAME + "\n"