Resolve some reviews

uc-cdis · Dec 1, 2023 · 4c65be6 · 4c65be6
1 parent 4ba4233
commit 4c65be6
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 13 deletions.
diff --git a/fence/blueprints/data/blueprint.py b/fence/blueprints/data/blueprint.py
@@ -288,7 +288,11 @@ def complete_multipart_upload():
         raise UserError("missing required arguments: {}".format(list(missing)))
 
     default_expires_in = flask.current_app.config.get("MAX_PRESIGNED_URL_TTL", 3600)
+
     bucket = params.get("bucket")
+    if bucket:
+        verify_data_upload_bucket_configuration(bucket)
+
     expires_in = get_valid_expiration(
         params.get("expires_in"),
         max_limit=default_expires_in,

diff --git a/fence/blueprints/data/indexd.py b/fence/blueprints/data/indexd.py
@@ -317,13 +317,12 @@ def init_multipart_upload(key, expires_in=None, bucket=None):
         Returns:
             uploadId(str)
         """
+        bucket = bucket or flask.current_app.config["DATA_UPLOAD_BUCKET"]
         if not bucket:
-            try:
-                bucket = flask.current_app.config["DATA_UPLOAD_BUCKET"]
-            except KeyError:
-                raise InternalError(
-                    "fence not configured with data upload bucket; can't create signed URL"
-                )
+            raise InternalError(
+                "fence not configured with data upload bucket; can't create signed URL"
+            )
+
         s3_url = "s3://{}/{}".format(bucket, key)
         return S3IndexedFileLocation(s3_url).init_multipart_upload(expires_in)
 
@@ -342,11 +341,6 @@ def complete_multipart_upload(key, uploadId, parts, expires_in=None, bucket=None
             None if success otherwise an exception
         """
         if bucket:
-            s3_buckets = get_value(
-                flask.current_app.config,
-                "ALLOWED_DATA_UPLOAD_BUCKETS",
-                InternalError("ALLOWED_DATA_UPLOAD_BUCKETS not configured"),
-            )
             verify_data_upload_bucket_configuration(bucket)
         else:
             try:
@@ -1057,7 +1051,7 @@ def init_multipart_upload(self, expires_in):
             self.bucket_name(), aws_creds, expires_in
         )
 
-        return multipart_upload.initilize_multipart_upload(
+        return multipart_upload.initialize_multipart_upload(
             self.parsed_url.netloc, self.parsed_url.path.strip("/"), credentials
         )
 
@@ -1631,6 +1625,17 @@ def filter_auth_ids(action, list_auth_ids):
 
 
 def verify_data_upload_bucket_configuration(bucket):
+    """
+    Verify that the bucket is configured in Fence as an uploadable bucket
+
+    Args:
+        bucket(str): bucket name
+    """
+    s3_buckets = flask.current_app.config["ALLOWED_DATA_UPLOAD_BUCKETS"]
+
+    if not s3_buckets:
+        raise InternalError("ALLOWED_DATA_UPLOAD_BUCKETS not configured")
+
     s3_buckets = get_value(
         flask.current_app.config,
         "ALLOWED_DATA_UPLOAD_BUCKETS",

diff --git a/fence/blueprints/data/multipart_upload.py b/fence/blueprints/data/multipart_upload.py
@@ -13,7 +13,7 @@
 logger = get_logger(__name__)
 
 
-def initilize_multipart_upload(bucket_name, key, credentials):
+def initialize_multipart_upload(bucket_name, key, credentials):
     """
     Initialize multipart upload