feat: Enabling S3 bucket share - Addressing comments on PR

data-dot-all · Nov 15, 2023 · 4516f4d · 4516f4d
1 parent f43003c
commit 4516f4d
Show file tree

Hide file tree

Showing 7 changed files with 37 additions and 31 deletions.
diff --git a/.../dataall/modules/dataset_sharing/services/share_managers/s3_access_point_share_manager.py b/.../dataall/modules/dataset_sharing/services/share_managers/s3_access_point_share_manager.py
@@ -178,12 +178,22 @@ def grant_target_role_access_policy(self):
                 kms_target_resources = [
                     f"arn:aws:kms:{self.dataset_region}:{self.dataset_account_id}:key/{kms_key_id}"
                 ]
-                share_manager.add_missing_resources_to_policy_statement(
-                    kms_key_id,
-                    kms_target_resources,
-                    existing_policy["Statement"][1],
-                    IAM_ACCESS_POINT_ROLE_POLICY
-                )
+                if len(existing_policy["Statement"]) > 1:
+                    share_manager.add_missing_resources_to_policy_statement(
+                        kms_key_id,
+                        kms_target_resources,
+                        existing_policy["Statement"][1],
+                        IAM_ACCESS_POINT_ROLE_POLICY
+                    )
+                else:
+                    additional_policy = {
+                        "Effect": "Allow",
+                        "Action": [
+                            "kms:*"
+                        ],
+                        "Resource": kms_target_resources
+                    }
+                    existing_policy["Statement"].append(additional_policy)
 
             policy = existing_policy
         else:

diff --git a/backend/dataall/modules/dataset_sharing/services/share_managers/s3_bucket_share_manager.py b/backend/dataall/modules/dataset_sharing/services/share_managers/s3_bucket_share_manager.py
@@ -105,13 +105,22 @@ def grant_s3_iam_access(self):
                 kms_target_resources = [
                     f"arn:aws:kms:{self.bucket_region}:{self.source_account_id}:key/{kms_key_id}"
                 ]
-
-                share_manager.add_missing_resources_to_policy_statement(
-                    resource_type=kms_key_id,
-                    target_resources=kms_target_resources,
-                    existing_policy_statement=existing_policy["Statement"][1],
-                    iam_role_policy_name=IAM_S3BUCKET_ROLE_POLICY
-                )
+                if len(existing_policy["Statement"]) > 1:
+                    share_manager.add_missing_resources_to_policy_statement(
+                        resource_type=kms_key_id,
+                        target_resources=kms_target_resources,
+                        existing_policy_statement=existing_policy["Statement"][1],
+                        iam_role_policy_name=IAM_S3BUCKET_ROLE_POLICY
+                    )
+                else:
+                    additional_policy = {
+                        "Effect": "Allow",
+                        "Action": [
+                            "kms:*"
+                        ],
+                        "Resource": kms_target_resources
+                    }
+                    existing_policy["Statement"].append(additional_policy)
 
             policy = existing_policy
         else:

diff --git a/...ataall/modules/dataset_sharing/services/share_processors/s3_access_point_process_share.py b/...ataall/modules/dataset_sharing/services/share_processors/s3_access_point_process_share.py
@@ -21,6 +21,8 @@ def __init__(
         target_environment: Environment,
         source_env_group: EnvironmentGroup,
         env_group: EnvironmentGroup,
+        existing_shared_buckets: bool = False
+
     ):
 
         super().__init__(

diff --git a/backend/migrations/versions/8c79fb896983_add_table_for_buckets.py b/backend/migrations/versions/8c79fb896983_add_table_for_buckets.py
@@ -171,21 +171,6 @@ def upgrade():
                 )
                 session.add(dataset_bucket)
             session.flush()  # flush to get the bucketUri
-
-        for dataset in datasets:
-            shared_bucket_object: ShareObjectItem = session.query(ShareObjectItem).filter(
-                and_(
-                    ShareObjectItem.itemType == ShareableType.S3Bucket.value,
-                    ShareObjectItem.itemUri == dataset.datasetUri,
-                )
-            ).first()
-            dataset_bucket: DatasetBucket = session.query(DatasetBucket).filter(
-                DatasetBucket.datasetUri == dataset.datasetUri
-            ).first()
-            if shared_bucket_object is not None:
-                shared_bucket_object.itemUri = dataset_bucket.bucketUri
-                shared_bucket_object.itemName = dataset_bucket.S3BucketName
-
         session.commit()
 
     except Exception as exception:

diff --git a/frontend/src/modules/Shares/components/AddShareItemModal.js b/frontend/src/modules/Shares/components/AddShareItemModal.js
@@ -20,7 +20,7 @@ import { Defaults, Pager, Scrollbar } from 'design';
 import { SET_ERROR, useDispatch } from 'globalErrors';
 import { useClient } from 'services';
 import { addSharedItem, getShareObject } from '../services';
-import { generateShareItemLabel } from '../../../utils';
+import { generateShareItemLabel } from 'utils';
 
 export const AddShareItemModal = (props) => {
   const client = useClient();

diff --git a/frontend/src/modules/Shares/components/RevokeShareItemsModal.js b/frontend/src/modules/Shares/components/RevokeShareItemsModal.js
@@ -10,7 +10,7 @@ import { Defaults } from 'design';
 import { SET_ERROR, useDispatch } from 'globalErrors';
 import { useClient } from 'services';
 import { getShareObject, revokeItemsShareObject } from '../services';
-import { generateShareItemLabel } from '../../../utils';
+import { generateShareItemLabel } from 'utils';
 
 export const RevokeShareItemsModal = (props) => {
   const client = useClient();

diff --git a/frontend/src/modules/Shares/views/ShareView.js b/frontend/src/modules/Shares/views/ShareView.js
@@ -65,7 +65,7 @@ import {
   UpdateRejectReason,
   UpdateRequestReason
 } from '../components';
-import { generateShareItemLabel } from '../../../utils';
+import { generateShareItemLabel } from 'utils';
 
 function ShareViewHeader(props) {
   const {