NAS-131330 / 24.10.0 / Better error handling for bad storj bucket nam…

…e (by creatorcary) (#14582)

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):

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)
 

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"