diff --git a/backend/dataall/modules/s3_datasets_shares/__init__.py b/backend/dataall/modules/s3_datasets_shares/__init__.py index 9568aa694..fcf520992 100644 --- a/backend/dataall/modules/s3_datasets_shares/__init__.py +++ b/backend/dataall/modules/s3_datasets_shares/__init__.py @@ -23,11 +23,11 @@ def depends_on() -> List[Type['ModuleInterface']]: def __init__(self): from dataall.core.environment.services.environment_resource_manager import EnvironmentResourceManager from dataall.modules.s3_datasets_shares import api - from dataall.modules.s3_datasets_shares.services.managed_share_policy_service import SharePolicyService + from dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service import S3SharePolicyService from dataall.modules.s3_datasets.services.dataset_service import DatasetService from dataall.modules.datasets_base.services.dataset_list_service import DatasetListService - from dataall.modules.s3_datasets_shares.services.dataset_sharing_service import DatasetSharingService - from dataall.modules.s3_datasets_shares.db.share_object_repositories import ShareEnvironmentResource + from dataall.modules.s3_datasets_shares.services.s3_share_dataset_service import S3ShareDatasetService + from dataall.modules.s3_datasets_shares.db.s3_share_object_repositories import S3ShareEnvironmentResource from dataall.modules.shares_base.services.share_processor_manager import ( ShareProcessorManager, ShareProcessorDefinition, @@ -35,9 +35,9 @@ def __init__(self): from dataall.modules.shares_base.services.shares_enums import ShareableType from dataall.modules.s3_datasets.db.dataset_models import DatasetTable, DatasetBucket, DatasetStorageLocation - EnvironmentResourceManager.register(ShareEnvironmentResource()) - DatasetService.register(DatasetSharingService()) - DatasetListService.register(DatasetSharingService()) + EnvironmentResourceManager.register(S3ShareEnvironmentResource()) + DatasetService.register(S3ShareDatasetService()) + DatasetListService.register(S3ShareDatasetService()) ShareProcessorManager.register_processor( ShareProcessorDefinition(ShareableType.Table, None, DatasetTable, DatasetTable.tableUri) @@ -77,7 +77,7 @@ def depends_on() -> List[Type['ModuleInterface']]: ] def __init__(self): - log.info('S3 Sharing handlers have been imported') + log.info('s3_datasets_shares handlers have been imported') class S3DatasetsSharesCdkModuleInterface(ModuleInterface): @@ -89,9 +89,9 @@ def is_supported(modes): def __init__(self): import dataall.modules.s3_datasets_shares.cdk - from dataall.modules.s3_datasets_shares.services.managed_share_policy_service import SharePolicyService + from dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service import S3SharePolicyService - log.info('CDK module data_sharing has been imported') + log.info('CDK module s3_datasets_shares has been imported') class S3DatasetsSharesECSShareModuleInterface(ModuleInterface): @@ -144,4 +144,4 @@ def __init__(self): ) ) - log.info('ECS Share module s3_data_sharing has been imported') + log.info('ECS Share module s3_datasets_shares has been imported') diff --git a/backend/dataall/modules/s3_datasets_shares/api/resolvers.py b/backend/dataall/modules/s3_datasets_shares/api/resolvers.py index 5b07c159b..c8e73c022 100644 --- a/backend/dataall/modules/s3_datasets_shares/api/resolvers.py +++ b/backend/dataall/modules/s3_datasets_shares/api/resolvers.py @@ -3,7 +3,7 @@ from dataall.base.api.context import Context from dataall.base.db.exceptions import RequiredParameter from dataall.base.feature_toggle_checker import is_feature_enabled -from dataall.modules.s3_datasets_shares.services.dataset_sharing_service import DatasetSharingService +from dataall.modules.s3_datasets_shares.services.s3_share_service import S3ShareService log = logging.getLogger(__name__) @@ -41,32 +41,30 @@ def validate_dataset_share_selector_input(data): def list_shared_tables_by_env_dataset(context: Context, source, datasetUri: str, envUri: str): - return DatasetSharingService.list_shared_tables_by_env_dataset(datasetUri, envUri) + return S3ShareService.list_shared_tables_by_env_dataset(datasetUri, envUri) @is_feature_enabled('modules.s3_datasets.features.aws_actions') def get_dataset_shared_assume_role_url(context: Context, source, datasetUri: str = None): - return DatasetSharingService.get_dataset_shared_assume_role_url(uri=datasetUri) + return S3ShareService.get_dataset_shared_assume_role_url(uri=datasetUri) def verify_dataset_share_objects(context: Context, source, input): RequestValidator.validate_dataset_share_selector_input(input) dataset_uri = input.get('datasetUri') verify_share_uris = input.get('shareUris') - return DatasetSharingService.verify_dataset_share_objects(uri=dataset_uri, share_uris=verify_share_uris) + return S3ShareService.verify_dataset_share_objects(uri=dataset_uri, share_uris=verify_share_uris) def get_s3_consumption_data(context: Context, source, shareUri: str): - return DatasetSharingService.get_s3_consumption_data(uri=shareUri) + return S3ShareService.get_s3_consumption_data(uri=shareUri) def list_shared_databases_tables_with_env_group(context: Context, source, environmentUri: str, groupUri: str): - return DatasetSharingService.list_shared_databases_tables_with_env_group( - environmentUri=environmentUri, groupUri=groupUri - ) + return S3ShareService.list_shared_databases_tables_with_env_group(environmentUri=environmentUri, groupUri=groupUri) def resolve_shared_db_name(context: Context, source, **kwargs): - return DatasetSharingService.resolve_shared_db_name( + return S3ShareService.resolve_shared_db_name( source.GlueDatabaseName, source.shareUri, source.targetEnvAwsAccountId, source.targetEnvRegion ) diff --git a/backend/dataall/modules/s3_datasets_shares/aws/glue_client.py b/backend/dataall/modules/s3_datasets_shares/aws/glue_client.py index 61795b199..ef694d8c0 100644 --- a/backend/dataall/modules/s3_datasets_shares/aws/glue_client.py +++ b/backend/dataall/modules/s3_datasets_shares/aws/glue_client.py @@ -175,6 +175,18 @@ def get_source_catalog(self): raise e return None + def get_glue_database_from_catalog(self): + # Check if a catalog account exists and return database accordingly + try: + catalog_dict = self.get_source_catalog() + + if catalog_dict is not None: + return catalog_dict.get('database_name') + else: + return self._database + except Exception as e: + raise e + def get_database_tags(self): # Get tags from the glue database account_id = self._account_id diff --git a/backend/dataall/modules/s3_datasets_shares/db/share_object_repositories.py b/backend/dataall/modules/s3_datasets_shares/db/s3_share_object_repositories.py similarity index 74% rename from backend/dataall/modules/s3_datasets_shares/db/share_object_repositories.py rename to backend/dataall/modules/s3_datasets_shares/db/s3_share_object_repositories.py index c21e04a3d..dac3ae8f0 100644 --- a/backend/dataall/modules/s3_datasets_shares/db/share_object_repositories.py +++ b/backend/dataall/modules/s3_datasets_shares/db/s3_share_object_repositories.py @@ -2,16 +2,13 @@ from warnings import warn from typing import List -from sqlalchemy import and_, or_, func, case +from sqlalchemy import and_, or_ from sqlalchemy.orm import Query -from dataall.core.environment.db.environment_models import Environment, EnvironmentGroup +from dataall.core.environment.db.environment_models import Environment from dataall.core.environment.services.environment_resource_manager import EnvironmentResource -from dataall.base.db import exceptions, paginate from dataall.modules.shares_base.services.shares_enums import ( - ShareItemHealthStatus, ShareObjectStatus, - ShareItemStatus, ShareableType, PrincipalType, ) @@ -19,13 +16,13 @@ from dataall.modules.shares_base.db.share_object_models import ShareObjectItem, ShareObject from dataall.modules.shares_base.db.share_object_repositories import ShareObjectRepository from dataall.modules.s3_datasets.db.dataset_repositories import DatasetRepository -from dataall.modules.s3_datasets.db.dataset_models import DatasetStorageLocation, DatasetTable, S3Dataset, DatasetBucket +from dataall.modules.s3_datasets.db.dataset_models import DatasetTable, S3Dataset from dataall.modules.datasets_base.db.dataset_models import DatasetBase logger = logging.getLogger(__name__) -class ShareEnvironmentResource(EnvironmentResource): +class S3ShareEnvironmentResource(EnvironmentResource): @staticmethod def count_resources(session, environment, group_uri) -> int: return S3ShareObjectRepository.count_S3_principal_shares( @@ -395,124 +392,6 @@ def list_s3_dataset_shares_with_existing_shared_items( query = query.filter(ShareObjectItem.itemType == item_type) return query.all() - @staticmethod # TODO!!! - def list_shareable_items(session, share, states, data): # TODO - # All tables from dataset with a column isShared - # marking the table as part of the shareObject - tables = ( - session.query( - DatasetTable.tableUri.label('itemUri'), - func.coalesce('DatasetTable').label('itemType'), - DatasetTable.GlueTableName.label('itemName'), - DatasetTable.description.label('description'), - ShareObjectItem.shareItemUri.label('shareItemUri'), - ShareObjectItem.status.label('status'), - ShareObjectItem.healthStatus.label('healthStatus'), - ShareObjectItem.healthMessage.label('healthMessage'), - ShareObjectItem.lastVerificationTime.label('lastVerificationTime'), - case( - [(ShareObjectItem.shareItemUri.isnot(None), True)], - else_=False, - ).label('isShared'), - ) - .outerjoin( - ShareObjectItem, - and_( - ShareObjectItem.shareUri == share.shareUri, - DatasetTable.tableUri == ShareObjectItem.itemUri, - ), - ) - .filter(DatasetTable.datasetUri == share.datasetUri) - ) - if states: - tables = tables.filter(ShareObjectItem.status.in_(states)) - - # All folders from the dataset with a column isShared - # marking the folder as part of the shareObject - locations = ( - session.query( - DatasetStorageLocation.locationUri.label('itemUri'), - func.coalesce('DatasetStorageLocation').label('itemType'), - DatasetStorageLocation.S3Prefix.label('itemName'), - DatasetStorageLocation.description.label('description'), - ShareObjectItem.shareItemUri.label('shareItemUri'), - ShareObjectItem.status.label('status'), - ShareObjectItem.healthStatus.label('healthStatus'), - ShareObjectItem.healthMessage.label('healthMessage'), - ShareObjectItem.lastVerificationTime.label('lastVerificationTime'), - case( - [(ShareObjectItem.shareItemUri.isnot(None), True)], - else_=False, - ).label('isShared'), - ) - .outerjoin( - ShareObjectItem, - and_( - ShareObjectItem.shareUri == share.shareUri, - DatasetStorageLocation.locationUri == ShareObjectItem.itemUri, - ), - ) - .filter(DatasetStorageLocation.datasetUri == share.datasetUri) - ) - if states: - locations = locations.filter(ShareObjectItem.status.in_(states)) - - s3_buckets = ( - session.query( - DatasetBucket.bucketUri.label('itemUri'), - func.coalesce('S3Bucket').label('itemType'), - DatasetBucket.S3BucketName.label('itemName'), - DatasetBucket.description.label('description'), - ShareObjectItem.shareItemUri.label('shareItemUri'), - ShareObjectItem.status.label('status'), - ShareObjectItem.healthStatus.label('healthStatus'), - ShareObjectItem.healthMessage.label('healthMessage'), - ShareObjectItem.lastVerificationTime.label('lastVerificationTime'), - case( - [(ShareObjectItem.shareItemUri.isnot(None), True)], - else_=False, - ).label('isShared'), - ) - .outerjoin( - ShareObjectItem, - and_( - ShareObjectItem.shareUri == share.shareUri, - DatasetBucket.bucketUri == ShareObjectItem.itemUri, - ), - ) - .filter(DatasetBucket.datasetUri == share.datasetUri) - ) - if states: - s3_buckets = s3_buckets.filter(ShareObjectItem.status.in_(states)) - - shareable_objects = tables.union(locations, s3_buckets).subquery('shareable_objects') - query = session.query(shareable_objects) - - if data: - if data.get('term'): - term = data.get('term') - query = query.filter( - or_( - shareable_objects.c.itemName.ilike(term + '%'), - shareable_objects.c.description.ilike(term + '%'), - ) - ) - if 'isShared' in data: - is_shared = data.get('isShared') - query = query.filter(shareable_objects.c.isShared == is_shared) - - if 'isHealthy' in data: - # healthy_status = ShareItemHealthStatus.Healthy.value - query = ( - query.filter(shareable_objects.c.healthStatus == ShareItemHealthStatus.Healthy.value) - if data.get('isHealthy') - else query.filter(shareable_objects.c.healthStatus != ShareItemHealthStatus.Healthy.value) - ) - - return paginate( - query.order_by(shareable_objects.c.itemName).distinct(), data.get('page', 1), data.get('pageSize', 10) - ).to_dict() - # the next 2 methods are used in subscription task @staticmethod def find_share_items_by_item_uri(session, item_uri): diff --git a/backend/dataall/modules/s3_datasets_shares/services/dataset_sharing_alarm_service.py b/backend/dataall/modules/s3_datasets_shares/services/s3_share_alarm_service.py similarity index 99% rename from backend/dataall/modules/s3_datasets_shares/services/dataset_sharing_alarm_service.py rename to backend/dataall/modules/s3_datasets_shares/services/s3_share_alarm_service.py index 5a46fea2e..e9c6a5e2f 100644 --- a/backend/dataall/modules/s3_datasets_shares/services/dataset_sharing_alarm_service.py +++ b/backend/dataall/modules/s3_datasets_shares/services/s3_share_alarm_service.py @@ -9,7 +9,7 @@ log = logging.getLogger(__name__) -class DatasetSharingAlarmService(AlarmService): +class S3ShareAlarmService(AlarmService): """Contains set of alarms for datasets""" def trigger_table_sharing_failure_alarm( diff --git a/backend/dataall/modules/s3_datasets_shares/services/s3_share_dataset_service.py b/backend/dataall/modules/s3_datasets_shares/services/s3_share_dataset_service.py new file mode 100644 index 000000000..435ed4e8e --- /dev/null +++ b/backend/dataall/modules/s3_datasets_shares/services/s3_share_dataset_service.py @@ -0,0 +1,105 @@ +from dataall.core.permissions.services.resource_policy_service import ResourcePolicyService +from dataall.base.db import exceptions +from dataall.modules.shares_base.db.share_object_models import ShareObject +from dataall.modules.s3_datasets_shares.db.s3_share_object_repositories import S3ShareObjectRepository +from dataall.modules.shares_base.services.share_permissions import SHARE_OBJECT_APPROVER +from dataall.modules.s3_datasets.services.dataset_permissions import ( + DELETE_DATASET, + DELETE_DATASET_TABLE, + DELETE_DATASET_FOLDER, +) +from dataall.modules.datasets_base.services.datasets_enums import DatasetRole, DatasetTypes +from dataall.modules.datasets_base.services.dataset_service_interface import DatasetServiceInterface + + +import logging + +log = logging.getLogger(__name__) + + +class S3ShareDatasetService(DatasetServiceInterface): + @property + def dataset_type(self): + return DatasetTypes.S3 + + @staticmethod + def resolve_additional_dataset_user_role(session, uri, username, groups): + """Implemented as part of the DatasetServiceInterface""" + share = S3ShareObjectRepository.get_share_by_dataset_attributes(session, uri, username, groups) + if share is not None: + return DatasetRole.Shared.value + return None + + @staticmethod + def check_before_delete(session, uri, **kwargs): + """Implemented as part of the DatasetServiceInterface""" + action = kwargs.get('action') + if action in [DELETE_DATASET_FOLDER, DELETE_DATASET_TABLE]: + existing_s3_shared_items = S3ShareObjectRepository.check_existing_s3_shared_items(session, uri) + if existing_s3_shared_items: + raise exceptions.ResourceShared( + action=action, + message='Revoke all shares for this item before deletion', + ) + elif action in [DELETE_DATASET]: + shares = S3ShareObjectRepository.list_s3_dataset_shares_with_existing_shared_items( + session=session, dataset_uri=uri + ) + if shares: + raise exceptions.ResourceShared( + action=DELETE_DATASET, + message='Revoke all dataset shares before deletion.', + ) + else: + raise exceptions.RequiredParameter('Delete action') + return True + + @staticmethod + def execute_on_delete(session, uri, **kwargs): + """Implemented as part of the DatasetServiceInterface""" + action = kwargs.get('action') + if action in [DELETE_DATASET_FOLDER, DELETE_DATASET_TABLE]: + S3ShareObjectRepository.delete_s3_share_item(session, uri) + elif action in [DELETE_DATASET]: + S3ShareObjectRepository.delete_s3_shares_with_no_shared_items(session, uri) + else: + raise exceptions.RequiredParameter('Delete action') + return True + + @staticmethod + def append_to_list_user_datasets(session, username, groups): + """Implemented as part of the DatasetServiceInterface""" + return S3ShareObjectRepository.list_user_s3_shared_datasets(session, username, groups) + + @staticmethod + def extend_attach_steward_permissions(session, dataset, new_stewards, **kwargs): + """Implemented as part of the DatasetServiceInterface""" + dataset_shares = S3ShareObjectRepository.find_s3_dataset_shares(session, dataset.datasetUri) + if dataset_shares: + for share in dataset_shares: + ResourcePolicyService.attach_resource_policy( + session=session, + group=new_stewards, + permissions=SHARE_OBJECT_APPROVER, + resource_uri=share.shareUri, + resource_type=ShareObject.__name__, + ) + if dataset.stewards != dataset.SamlAdminGroupName: + ResourcePolicyService.delete_resource_policy( + session=session, + group=dataset.stewards, + resource_uri=share.shareUri, + ) + + @staticmethod + def extend_delete_steward_permissions(session, dataset, **kwargs): + """Implemented as part of the DatasetServiceInterface""" + dataset_shares = S3ShareObjectRepository.find_s3_dataset_shares(session, dataset.datasetUri) + if dataset_shares: + for share in dataset_shares: + if dataset.stewards != dataset.SamlAdminGroupName: + ResourcePolicyService.delete_resource_policy( + session=session, + group=dataset.stewards, + resource_uri=share.shareUri, + ) diff --git a/backend/dataall/modules/s3_datasets_shares/services/managed_share_policy_service.py b/backend/dataall/modules/s3_datasets_shares/services/s3_share_managed_policy_service.py similarity index 98% rename from backend/dataall/modules/s3_datasets_shares/services/managed_share_policy_service.py rename to backend/dataall/modules/s3_datasets_shares/services/s3_share_managed_policy_service.py index e53a94944..2738e8b38 100644 --- a/backend/dataall/modules/s3_datasets_shares/services/managed_share_policy_service.py +++ b/backend/dataall/modules/s3_datasets_shares/services/s3_share_managed_policy_service.py @@ -18,7 +18,7 @@ EMPTY_STATEMENT_SID = 'EmptyStatement' -class SharePolicyService(ManagedPolicy): +class S3SharePolicyService(ManagedPolicy): def __init__(self, role_name, account, region, environmentUri, resource_prefix): self.role_name = role_name self.account = account @@ -48,7 +48,7 @@ def generate_empty_policy(self) -> dict: @staticmethod def remove_empty_statement(policy_doc: dict, statement_sid: str) -> dict: - statement_index = SharePolicyService._get_statement_by_sid(policy_doc, statement_sid) + statement_index = S3SharePolicyService._get_statement_by_sid(policy_doc, statement_sid) if statement_index is not None: policy_doc['Statement'].pop(statement_index) return policy_doc diff --git a/backend/dataall/modules/s3_datasets_shares/services/dataset_sharing_service.py b/backend/dataall/modules/s3_datasets_shares/services/s3_share_service.py similarity index 57% rename from backend/dataall/modules/s3_datasets_shares/services/dataset_sharing_service.py rename to backend/dataall/modules/s3_datasets_shares/services/s3_share_service.py index 207ab1c65..de9906493 100644 --- a/backend/dataall/modules/s3_datasets_shares/services/dataset_sharing_service.py +++ b/backend/dataall/modules/s3_datasets_shares/services/s3_share_service.py @@ -1,123 +1,139 @@ +import logging from warnings import warn + from dataall.base.db import utils +from dataall.base.context import get_context +from dataall.base.aws.sts import SessionHelper from dataall.core.permissions.services.resource_policy_service import ResourcePolicyService from dataall.core.permissions.services.tenant_policy_service import TenantPolicyService from dataall.core.environment.services.environment_service import EnvironmentService -from dataall.base.context import get_context -from dataall.base.db import exceptions -from dataall.base.aws.sts import SessionHelper -from dataall.modules.shares_base.db.share_object_models import ShareObject -from dataall.modules.s3_datasets_shares.db.share_object_repositories import S3ShareObjectRepository from dataall.modules.shares_base.db.share_object_repositories import ShareObjectRepository from dataall.modules.shares_base.db.share_state_machines_repositories import ShareStatusRepository -from dataall.modules.shares_base.services.share_permissions import SHARE_OBJECT_APPROVER, GET_SHARE_OBJECT -from dataall.modules.s3_datasets_shares.services.share_item_service import S3ShareItemService from dataall.modules.shares_base.services.share_item_service import ShareItemService +from dataall.modules.shares_base.services.share_permissions import GET_SHARE_OBJECT +from dataall.modules.shares_base.services.shares_enums import ( + ShareableType, + ShareItemStatus, +) +from dataall.modules.s3_datasets.db.dataset_models import DatasetTable, DatasetStorageLocation from dataall.modules.s3_datasets.db.dataset_repositories import DatasetRepository from dataall.modules.s3_datasets.services.dataset_permissions import ( MANAGE_DATASETS, UPDATE_DATASET, - DELETE_DATASET, - DELETE_DATASET_TABLE, - DELETE_DATASET_FOLDER, CREDENTIALS_DATASET, + DATASET_TABLE_READ, + DATASET_FOLDER_READ, ) -from dataall.modules.datasets_base.services.datasets_enums import DatasetRole, DatasetTypes -from dataall.modules.datasets_base.services.dataset_service_interface import DatasetServiceInterface +from dataall.modules.s3_datasets_shares.db.s3_share_object_repositories import S3ShareObjectRepository from dataall.modules.s3_datasets_shares.aws.glue_client import GlueClient -import logging - log = logging.getLogger(__name__) -class DatasetSharingService(DatasetServiceInterface): - @property - def dataset_type(self): - return DatasetTypes.S3 - +class S3ShareService: @staticmethod - def resolve_additional_dataset_user_role(session, uri, username, groups): - """Implemented as part of the DatasetServiceInterface""" - share = S3ShareObjectRepository.get_share_by_dataset_attributes(session, uri, username, groups) - if share is not None: - return DatasetRole.Shared.value - return None + def delete_dataset_table_read_permission(session, share, tableUri): + """ + Delete Table permissions to share groups + """ + other_shares = S3ShareObjectRepository.find_all_other_share_items( + session, + not_this_share_uri=share.shareUri, + item_uri=tableUri, + share_type=ShareableType.Table.value, + principal_type='GROUP', + principal_uri=share.groupUri, + item_status=[ShareItemStatus.Share_Succeeded.value], + ) + log.info(f'Table {tableUri} has been shared with group {share.groupUri} in {len(other_shares)} more shares') + if len(other_shares) == 0: + log.info('Delete permissions...') + ResourcePolicyService.delete_resource_policy(session=session, group=share.groupUri, resource_uri=tableUri) @staticmethod - def check_before_delete(session, uri, **kwargs): - """Implemented as part of the DatasetServiceInterface""" - action = kwargs.get('action') - if action in [DELETE_DATASET_FOLDER, DELETE_DATASET_TABLE]: - existing_s3_shared_items = S3ShareObjectRepository.check_existing_s3_shared_items(session, uri) - if existing_s3_shared_items: - raise exceptions.ResourceShared( - action=action, - message='Revoke all shares for this item before deletion', - ) - elif action in [DELETE_DATASET]: - shares = S3ShareObjectRepository.list_s3_dataset_shares_with_existing_shared_items( - session=session, dataset_uri=uri + def delete_dataset_folder_read_permission(session, share, locationUri): + """ + Delete Folder permissions to share groups + """ + other_shares = S3ShareObjectRepository.find_all_other_share_items( + session, + not_this_share_uri=share.shareUri, + item_uri=locationUri, + share_type=ShareableType.StorageLocation.value, + principal_type='GROUP', + principal_uri=share.groupUri, + item_status=[ShareItemStatus.Share_Succeeded.value], + ) + log.info( + f'Location {locationUri} has been shared with group {share.groupUri} in {len(other_shares)} more shares' + ) + if len(other_shares) == 0: + log.info('Delete permissions...') + ResourcePolicyService.delete_resource_policy( + session=session, + group=share.groupUri, + resource_uri=locationUri, ) - if shares: - raise exceptions.ResourceShared( - action=DELETE_DATASET, - message='Revoke all dataset shares before deletion.', - ) - else: - raise exceptions.RequiredParameter('Delete action') - return True @staticmethod - def execute_on_delete(session, uri, **kwargs): - """Implemented as part of the DatasetServiceInterface""" - action = kwargs.get('action') - if action in [DELETE_DATASET_FOLDER, DELETE_DATASET_TABLE]: - S3ShareObjectRepository.delete_s3_share_item(session, uri) - elif action in [DELETE_DATASET]: - S3ShareObjectRepository.delete_s3_shares_with_no_shared_items(session, uri) + def attach_dataset_table_read_permission(session, share, tableUri): + """ + Attach Table permissions to share groups + """ + existing_policy = ResourcePolicyService.find_resource_policies( + session, + group=share.groupUri, + resource_uri=tableUri, + resource_type=DatasetTable.__name__, + permissions=DATASET_TABLE_READ, + ) + # toDo: separate policies from list DATASET_TABLE_READ, because in future only one of them can be granted (Now they are always granted together) + if len(existing_policy) == 0: + log.info( + f'Attaching new resource permission policy {DATASET_TABLE_READ} to table {tableUri} for group {share.groupUri}' + ) + ResourcePolicyService.attach_resource_policy( + session=session, + group=share.groupUri, + permissions=DATASET_TABLE_READ, + resource_uri=tableUri, + resource_type=DatasetTable.__name__, + ) else: - raise exceptions.RequiredParameter('Delete action') - return True - - @staticmethod - def append_to_list_user_datasets(session, username, groups): - """Implemented as part of the DatasetServiceInterface""" - return S3ShareObjectRepository.list_user_s3_shared_datasets(session, username, groups) + log.info( + f'Resource permission policy {DATASET_TABLE_READ} to table {tableUri} for group {share.groupUri} already exists. Skip... ' + ) @staticmethod - def extend_attach_steward_permissions(session, dataset, new_stewards, **kwargs): - """Implemented as part of the DatasetServiceInterface""" - dataset_shares = S3ShareObjectRepository.find_s3_dataset_shares(session, dataset.datasetUri) - if dataset_shares: - for share in dataset_shares: - ResourcePolicyService.attach_resource_policy( - session=session, - group=new_stewards, - permissions=SHARE_OBJECT_APPROVER, - resource_uri=share.shareUri, - resource_type=ShareObject.__name__, - ) - if dataset.stewards != dataset.SamlAdminGroupName: - ResourcePolicyService.delete_resource_policy( - session=session, - group=dataset.stewards, - resource_uri=share.shareUri, - ) + def attach_dataset_folder_read_permission(session, share, locationUri): + """ + Attach Folder permissions to share groups + """ + existing_policy = ResourcePolicyService.find_resource_policies( + session, + group=share.groupUri, + resource_uri=locationUri, + resource_type=DatasetStorageLocation.__name__, + permissions=DATASET_FOLDER_READ, + ) + # toDo: separate policies from list DATASET_TABLE_READ, because in future only one of them can be granted (Now they are always granted together) + if len(existing_policy) == 0: + log.info( + f'Attaching new resource permission policy {DATASET_FOLDER_READ} to folder {locationUri} for group {share.groupUri}' + ) - @staticmethod - def extend_delete_steward_permissions(session, dataset, **kwargs): - """Implemented as part of the DatasetServiceInterface""" - dataset_shares = S3ShareObjectRepository.find_s3_dataset_shares(session, dataset.datasetUri) - if dataset_shares: - for share in dataset_shares: - if dataset.stewards != dataset.SamlAdminGroupName: - ResourcePolicyService.delete_resource_policy( - session=session, - group=dataset.stewards, - resource_uri=share.shareUri, - ) + ResourcePolicyService.attach_resource_policy( + session=session, + group=share.groupUri, + permissions=DATASET_FOLDER_READ, + resource_uri=locationUri, + resource_type=DatasetStorageLocation.__name__, + ) + else: + log.info( + f'Resource permission policy {DATASET_FOLDER_READ} to table {locationUri} for group {share.groupUri} already exists. Skip... ' + ) @staticmethod @TenantPolicyService.has_tenant_permission(MANAGE_DATASETS) @@ -195,9 +211,10 @@ def get_s3_consumption_data(uri): separator='-', ) # Check if the share was made with a Glue Database - datasetGlueDatabase = S3ShareItemService.get_glue_database_for_share( - dataset.GlueDatabaseName, dataset.AwsAccountId, dataset.region - ) + datasetGlueDatabase = GlueClient( + account_id=dataset.AwsAccountId, region=dataset.region, database=dataset.GlueDatabaseName + ).get_glue_database_from_catalog() + old_shared_db_name = f'{datasetGlueDatabase}_shared_{uri}'[:254] database = GlueClient( account_id=environment.AwsAccountId, region=environment.region, database=old_shared_db_name diff --git a/backend/dataall/modules/s3_datasets_shares/services/share_item_service.py b/backend/dataall/modules/s3_datasets_shares/services/share_item_service.py deleted file mode 100644 index 695b9ccd4..000000000 --- a/backend/dataall/modules/s3_datasets_shares/services/share_item_service.py +++ /dev/null @@ -1,135 +0,0 @@ -import logging - -from dataall.core.permissions.services.resource_policy_service import ResourcePolicyService -from dataall.modules.shares_base.services.shares_enums import ( - ShareableType, - ShareItemStatus, -) -from dataall.modules.s3_datasets_shares.aws.glue_client import GlueClient -from dataall.modules.s3_datasets_shares.db.share_object_repositories import S3ShareObjectRepository -from dataall.modules.s3_datasets.db.dataset_models import DatasetTable, DatasetStorageLocation -from dataall.modules.s3_datasets.services.dataset_permissions import DATASET_TABLE_READ, DATASET_FOLDER_READ - -log = logging.getLogger(__name__) - - -class S3ShareItemService: - @staticmethod - def get_glue_database_for_share(glueDatabase, account_id, region): # TODO: IN S3_DATASETS_SHARES - # Check if a catalog account exists and return database accordingly - try: - catalog_dict = GlueClient( - account_id=account_id, - region=region, - database=glueDatabase, - ).get_source_catalog() - - if catalog_dict is not None: - return catalog_dict.get('database_name') - else: - return glueDatabase - except Exception as e: - raise e - - @staticmethod - def delete_dataset_table_read_permission(session, share, tableUri): - """ - Delete Table permissions to share groups - """ - other_shares = S3ShareObjectRepository.find_all_other_share_items( - session, - not_this_share_uri=share.shareUri, - item_uri=tableUri, - share_type=ShareableType.Table.value, - principal_type='GROUP', - principal_uri=share.groupUri, - item_status=[ShareItemStatus.Share_Succeeded.value], - ) - log.info(f'Table {tableUri} has been shared with group {share.groupUri} in {len(other_shares)} more shares') - if len(other_shares) == 0: - log.info('Delete permissions...') - ResourcePolicyService.delete_resource_policy(session=session, group=share.groupUri, resource_uri=tableUri) - - @staticmethod - def delete_dataset_folder_read_permission(session, share, locationUri): - """ - Delete Folder permissions to share groups - """ - other_shares = S3ShareObjectRepository.find_all_other_share_items( - session, - not_this_share_uri=share.shareUri, - item_uri=locationUri, - share_type=ShareableType.StorageLocation.value, - principal_type='GROUP', - principal_uri=share.groupUri, - item_status=[ShareItemStatus.Share_Succeeded.value], - ) - log.info( - f'Location {locationUri} has been shared with group {share.groupUri} in {len(other_shares)} more shares' - ) - if len(other_shares) == 0: - log.info('Delete permissions...') - ResourcePolicyService.delete_resource_policy( - session=session, - group=share.groupUri, - resource_uri=locationUri, - ) - - @staticmethod - def attach_dataset_table_read_permission(session, share, tableUri): - """ - Attach Table permissions to share groups - """ - existing_policy = ResourcePolicyService.find_resource_policies( - session, - group=share.groupUri, - resource_uri=tableUri, - resource_type=DatasetTable.__name__, - permissions=DATASET_TABLE_READ, - ) - # toDo: separate policies from list DATASET_TABLE_READ, because in future only one of them can be granted (Now they are always granted together) - if len(existing_policy) == 0: - log.info( - f'Attaching new resource permission policy {DATASET_TABLE_READ} to table {tableUri} for group {share.groupUri}' - ) - ResourcePolicyService.attach_resource_policy( - session=session, - group=share.groupUri, - permissions=DATASET_TABLE_READ, - resource_uri=tableUri, - resource_type=DatasetTable.__name__, - ) - else: - log.info( - f'Resource permission policy {DATASET_TABLE_READ} to table {tableUri} for group {share.groupUri} already exists. Skip... ' - ) - - @staticmethod - def attach_dataset_folder_read_permission(session, share, locationUri): - """ - Attach Folder permissions to share groups - """ - existing_policy = ResourcePolicyService.find_resource_policies( - session, - group=share.groupUri, - resource_uri=locationUri, - resource_type=DatasetStorageLocation.__name__, - permissions=DATASET_FOLDER_READ, - ) - # toDo: separate policies from list DATASET_TABLE_READ, because in future only one of them can be granted (Now they are always granted together) - if len(existing_policy) == 0: - log.info( - f'Attaching new resource permission policy {DATASET_FOLDER_READ} to folder {locationUri} for group {share.groupUri}' - ) - - ResourcePolicyService.attach_resource_policy( - session=session, - group=share.groupUri, - permissions=DATASET_FOLDER_READ, - resource_uri=locationUri, - resource_type=DatasetStorageLocation.__name__, - ) - else: - log.info( - f'Resource permission policy {DATASET_FOLDER_READ} to table {locationUri} for group {share.groupUri} already exists. Skip... ' - ) diff --git a/backend/dataall/modules/s3_datasets_shares/services/share_managers/lf_share_manager.py b/backend/dataall/modules/s3_datasets_shares/services/share_managers/lf_share_manager.py index 8d12fb40f..b2ddef07a 100644 --- a/backend/dataall/modules/s3_datasets_shares/services/share_managers/lf_share_manager.py +++ b/backend/dataall/modules/s3_datasets_shares/services/share_managers/lf_share_manager.py @@ -19,7 +19,7 @@ ShareItemHealthStatus, ) from dataall.modules.s3_datasets.db.dataset_models import DatasetTable -from dataall.modules.s3_datasets_shares.services.dataset_sharing_alarm_service import DatasetSharingAlarmService +from dataall.modules.s3_datasets_shares.services.s3_share_alarm_service import S3ShareAlarmService from dataall.modules.shares_base.db.share_object_models import ShareObjectItem from dataall.modules.s3_datasets_shares.services.share_managers.share_manager_utils import ShareErrorFormatter from dataall.modules.shares_base.services.sharing_service import ShareData @@ -569,7 +569,7 @@ def handle_share_failure( f'due to: {error}' ) - DatasetSharingAlarmService().trigger_table_sharing_failure_alarm(table, self.share, self.target_environment) + S3ShareAlarmService().trigger_table_sharing_failure_alarm(table, self.share, self.target_environment) return True def handle_revoke_failure( @@ -589,9 +589,7 @@ def handle_revoke_failure( f'with target account {self.target_environment.AwsAccountId}/{self.target_environment.region} ' f'due to: {error}' ) - DatasetSharingAlarmService().trigger_revoke_table_sharing_failure_alarm( - table, self.share, self.target_environment - ) + S3ShareAlarmService().trigger_revoke_table_sharing_failure_alarm(table, self.share, self.target_environment) return True def handle_share_failure_for_all_tables(self, tables, error, share_item_status, reapply=False): diff --git a/backend/dataall/modules/s3_datasets_shares/services/share_managers/s3_access_point_share_manager.py b/backend/dataall/modules/s3_datasets_shares/services/share_managers/s3_access_point_share_manager.py index e3663e3ee..8aaa9a7ea 100644 --- a/backend/dataall/modules/s3_datasets_shares/services/share_managers/s3_access_point_share_manager.py +++ b/backend/dataall/modules/s3_datasets_shares/services/share_managers/s3_access_point_share_manager.py @@ -18,12 +18,12 @@ DATAALL_KMS_PIVOT_ROLE_PERMISSIONS_SID, ) from dataall.base.aws.iam import IAM -from dataall.modules.s3_datasets_shares.services.dataset_sharing_alarm_service import DatasetSharingAlarmService +from dataall.modules.s3_datasets_shares.services.s3_share_alarm_service import S3ShareAlarmService from dataall.modules.shares_base.db.share_object_repositories import ShareObjectRepository from dataall.modules.shares_base.services.share_exceptions import PrincipalRoleNotFound from dataall.modules.s3_datasets_shares.services.share_managers.share_manager_utils import ShareErrorFormatter -from dataall.modules.s3_datasets_shares.services.managed_share_policy_service import ( - SharePolicyService, +from dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service import ( + S3SharePolicyService, IAM_S3_ACCESS_POINTS_STATEMENT_SID, EMPTY_STATEMENT_SID, ) @@ -159,7 +159,7 @@ def check_target_role_access_policy(self) -> None: key_alias = f'alias/{self.dataset.KmsAlias}' kms_client = KmsClient(self.dataset_account_id, self.source_environment.region) kms_key_id = kms_client.get_key_id(key_alias) - share_policy_service = SharePolicyService( + share_policy_service = S3SharePolicyService( environmentUri=self.target_environment.environmentUri, account=self.target_environment.AwsAccountId, region=self.target_environment.region, @@ -194,7 +194,7 @@ def check_target_role_access_policy(self) -> None: ) logger.info(f'Policy... {policy_document}') - s3_statement_index = SharePolicyService._get_statement_by_sid( + s3_statement_index = S3SharePolicyService._get_statement_by_sid( policy_document, f'{IAM_S3_ACCESS_POINTS_STATEMENT_SID}S3' ) @@ -228,7 +228,7 @@ def check_target_role_access_policy(self) -> None: ) if kms_key_id: - kms_statement_index = SharePolicyService._get_statement_by_sid( + kms_statement_index = S3SharePolicyService._get_statement_by_sid( policy_document, f'{IAM_S3_ACCESS_POINTS_STATEMENT_SID}KMS' ) kms_target_resources = [f'arn:aws:kms:{self.dataset_region}:{self.dataset_account_id}:key/{kms_key_id}'] @@ -268,7 +268,7 @@ def grant_target_role_access_policy(self): """ logger.info(f'Grant target role {self.target_requester_IAMRoleName} access policy') - share_policy_service = SharePolicyService( + share_policy_service = S3SharePolicyService( environmentUri=self.target_environment.environmentUri, account=self.target_environment.AwsAccountId, region=self.target_environment.region, @@ -619,7 +619,7 @@ def delete_access_point(self): def revoke_target_role_access_policy(self): logger.info('Deleting target role IAM statements...') - share_policy_service = SharePolicyService( + share_policy_service = S3SharePolicyService( environmentUri=self.target_environment.environmentUri, account=self.target_environment.AwsAccountId, region=self.target_environment.region, @@ -718,7 +718,7 @@ def handle_share_failure(self, error: Exception) -> None: f'with target account {self.target_environment.AwsAccountId}/{self.target_environment.region} ' f'due to: {error}' ) - DatasetSharingAlarmService().trigger_folder_sharing_failure_alarm( + S3ShareAlarmService().trigger_folder_sharing_failure_alarm( self.target_folder, self.share, self.target_environment ) @@ -735,7 +735,7 @@ def handle_revoke_failure(self, error: Exception) -> bool: f'with target account {self.target_environment.AwsAccountId}/{self.target_environment.region} ' f'due to: {error}' ) - DatasetSharingAlarmService().trigger_revoke_folder_sharing_failure_alarm( + S3ShareAlarmService().trigger_revoke_folder_sharing_failure_alarm( self.target_folder, self.share, self.target_environment ) return True diff --git a/backend/dataall/modules/s3_datasets_shares/services/share_managers/s3_bucket_share_manager.py b/backend/dataall/modules/s3_datasets_shares/services/share_managers/s3_bucket_share_manager.py index 609b50a7b..aafc932e7 100644 --- a/backend/dataall/modules/s3_datasets_shares/services/share_managers/s3_bucket_share_manager.py +++ b/backend/dataall/modules/s3_datasets_shares/services/share_managers/s3_bucket_share_manager.py @@ -15,9 +15,9 @@ from dataall.modules.shares_base.db.share_object_models import ShareObject from dataall.modules.shares_base.services.share_exceptions import PrincipalRoleNotFound from dataall.modules.s3_datasets_shares.services.share_managers.share_manager_utils import ShareErrorFormatter -from dataall.modules.s3_datasets_shares.services.dataset_sharing_alarm_service import DatasetSharingAlarmService -from dataall.modules.s3_datasets_shares.services.managed_share_policy_service import ( - SharePolicyService, +from dataall.modules.s3_datasets_shares.services.s3_share_alarm_service import S3ShareAlarmService +from dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service import ( + S3SharePolicyService, IAM_S3_BUCKETS_STATEMENT_SID, EMPTY_STATEMENT_SID, ) @@ -70,7 +70,7 @@ def check_s3_iam_access(self) -> None: kms_client = KmsClient(self.source_account_id, self.source_environment.region) kms_key_id = kms_client.get_key_id(key_alias) - share_policy_service = SharePolicyService( + share_policy_service = S3SharePolicyService( environmentUri=self.target_environment.environmentUri, account=self.target_environment.AwsAccountId, region=self.target_environment.region, @@ -98,7 +98,7 @@ def check_s3_iam_access(self) -> None: version_id, policy_document = IAM.get_managed_policy_default_version( self.target_environment.AwsAccountId, self.target_environment.region, share_resource_policy_name ) - s3_statement_index = SharePolicyService._get_statement_by_sid( + s3_statement_index = S3SharePolicyService._get_statement_by_sid( policy_document, f'{IAM_S3_BUCKETS_STATEMENT_SID}S3' ) @@ -131,7 +131,7 @@ def check_s3_iam_access(self) -> None: ) if kms_key_id: - kms_statement_index = SharePolicyService._get_statement_by_sid( + kms_statement_index = S3SharePolicyService._get_statement_by_sid( policy_document, f'{IAM_S3_BUCKETS_STATEMENT_SID}KMS' ) kms_target_resources = [f'arn:aws:kms:{self.bucket_region}:{self.source_account_id}:key/{kms_key_id}'] @@ -172,7 +172,7 @@ def grant_s3_iam_access(self): """ logger.info(f'Grant target role {self.target_requester_IAMRoleName} access policy') - share_policy_service = SharePolicyService( + share_policy_service = S3SharePolicyService( environmentUri=self.target_environment.environmentUri, account=self.target_environment.AwsAccountId, region=self.target_environment.region, @@ -484,7 +484,7 @@ def delete_target_role_access_policy( ): logger.info('Deleting target role IAM statements...') - share_policy_service = SharePolicyService( + share_policy_service = S3SharePolicyService( role_name=share.principalIAMRoleName, account=target_environment.AwsAccountId, region=self.target_environment.region, @@ -574,7 +574,7 @@ def handle_share_failure(self, error: Exception) -> bool: f'with target account {self.target_environment.AwsAccountId}/{self.target_environment.region} ' f'due to: {error}' ) - DatasetSharingAlarmService().trigger_s3_bucket_sharing_failure_alarm( + S3ShareAlarmService().trigger_s3_bucket_sharing_failure_alarm( self.target_bucket, self.share, self.target_environment ) return True @@ -592,7 +592,7 @@ def handle_revoke_failure(self, error: Exception) -> bool: f'with target account {self.target_environment.AwsAccountId}/{self.target_environment.region} ' f'due to: {error}' ) - DatasetSharingAlarmService().trigger_revoke_s3_bucket_sharing_failure_alarm( + S3ShareAlarmService().trigger_revoke_s3_bucket_sharing_failure_alarm( self.target_bucket, self.share, self.target_environment ) return True diff --git a/backend/dataall/modules/s3_datasets_shares/services/share_processors/glue_table_share_processor.py b/backend/dataall/modules/s3_datasets_shares/services/share_processors/glue_table_share_processor.py index 8c5bc0f42..3091f7041 100644 --- a/backend/dataall/modules/s3_datasets_shares/services/share_processors/glue_table_share_processor.py +++ b/backend/dataall/modules/s3_datasets_shares/services/share_processors/glue_table_share_processor.py @@ -16,10 +16,10 @@ from dataall.modules.s3_datasets_shares.services.share_managers import LFShareManager from dataall.modules.s3_datasets_shares.aws.ram_client import RamClient from dataall.modules.shares_base.services.share_object_service import ShareObjectService -from dataall.modules.s3_datasets_shares.services.share_item_service import S3ShareItemService +from dataall.modules.s3_datasets_shares.services.s3_share_service import S3ShareService from dataall.modules.shares_base.db.share_object_repositories import ShareObjectRepository from dataall.modules.shares_base.db.share_state_machines_repositories import ShareStatusRepository -from dataall.modules.s3_datasets_shares.db.share_object_repositories import S3ShareObjectRepository +from dataall.modules.s3_datasets_shares.db.s3_share_object_repositories import S3ShareObjectRepository from dataall.modules.shares_base.db.share_object_state_machines import ShareItemSM from dataall.modules.s3_datasets_shares.services.share_managers.share_manager_utils import ShareErrorFormatter @@ -154,7 +154,7 @@ def process_approved_shares(self) -> bool: manager.grant_principals_permissions_to_resource_link_table(table) log.info('Attaching TABLE READ permissions...') - S3ShareItemService.attach_dataset_table_read_permission( + S3ShareService.attach_dataset_table_read_permission( self.session, self.share_data.share, table.tableUri ) @@ -276,7 +276,7 @@ def process_revoked_shares(self) -> bool: and self.share_data.share.groupUri != self.share_data.dataset.stewards ): log.info('Deleting TABLE READ permissions...') - S3ShareItemService.delete_dataset_table_read_permission( + S3ShareService.delete_dataset_table_read_permission( self.session, self.share_data.share, table.tableUri ) diff --git a/backend/dataall/modules/s3_datasets_shares/services/share_processors/s3_access_point_share_processor.py b/backend/dataall/modules/s3_datasets_shares/services/share_processors/s3_access_point_share_processor.py index ab464f4dc..87ec4f6c0 100644 --- a/backend/dataall/modules/s3_datasets_shares/services/share_processors/s3_access_point_share_processor.py +++ b/backend/dataall/modules/s3_datasets_shares/services/share_processors/s3_access_point_share_processor.py @@ -5,7 +5,7 @@ from dataall.modules.shares_base.services.share_exceptions import PrincipalRoleNotFound from dataall.modules.s3_datasets_shares.services.share_managers import S3AccessPointShareManager from dataall.modules.shares_base.services.share_object_service import ShareObjectService -from dataall.modules.s3_datasets_shares.services.share_item_service import S3ShareItemService +from dataall.modules.s3_datasets_shares.services.s3_share_service import S3ShareService from dataall.modules.shares_base.services.shares_enums import ( ShareItemHealthStatus, ShareItemStatus, @@ -76,7 +76,7 @@ def process_approved_shares(self) -> bool: manager.update_dataset_bucket_key_policy() log.info('Attaching FOLDER READ permissions...') - S3ShareItemService.attach_dataset_folder_read_permission( + S3ShareService.attach_dataset_folder_read_permission( self.session, self.share_data.share, folder.locationUri ) @@ -145,7 +145,7 @@ def process_revoked_shares(self) -> bool: and self.share_data.share.groupUri != self.share_data.dataset.stewards ): log.info(f'Deleting FOLDER READ permissions from {folder.locationUri}...') - S3ShareItemService.delete_dataset_folder_read_permission( + S3ShareService.delete_dataset_folder_read_permission( self.session, manager.share, folder.locationUri ) diff --git a/backend/dataall/modules/s3_datasets_shares/tasks/dataset_subscription_task.py b/backend/dataall/modules/s3_datasets_shares/tasks/dataset_subscription_task.py index e382b05ef..e5a29c904 100644 --- a/backend/dataall/modules/s3_datasets_shares/tasks/dataset_subscription_task.py +++ b/backend/dataall/modules/s3_datasets_shares/tasks/dataset_subscription_task.py @@ -10,7 +10,7 @@ from dataall.core.environment.services.environment_service import EnvironmentService from dataall.base.db import get_engine from dataall.modules.shares_base.db.share_object_models import ShareObjectItem -from dataall.modules.s3_datasets_shares.db.share_object_repositories import S3ShareObjectRepository +from dataall.modules.s3_datasets_shares.db.s3_share_object_repositories import S3ShareObjectRepository from dataall.modules.shares_base.services.share_notification_service import ShareNotificationService from dataall.modules.s3_datasets.aws.sns_dataset_client import SnsDatasetClient from dataall.modules.s3_datasets.db.dataset_location_repositories import DatasetLocationRepository diff --git a/backend/migrations/versions/d05f9a5b215e_backfill_dataset_table_permissions.py b/backend/migrations/versions/d05f9a5b215e_backfill_dataset_table_permissions.py index 180483888..0474c86f4 100644 --- a/backend/migrations/versions/d05f9a5b215e_backfill_dataset_table_permissions.py +++ b/backend/migrations/versions/d05f9a5b215e_backfill_dataset_table_permissions.py @@ -24,7 +24,7 @@ ShareableType, ShareItemStatus, ) -from dataall.modules.s3_datasets_shares.db.share_object_repositories import ShareObjectRepository +from dataall.modules.shares_base.db.share_object_repositories import ShareObjectRepository from dataall.modules.s3_datasets.db.dataset_repositories import DatasetRepository from dataall.modules.s3_datasets.services.dataset_permissions import DATASET_TABLE_READ diff --git a/tests/modules/s3_datasets_shares/tasks/test_lf_share_manager.py b/tests/modules/s3_datasets_shares/tasks/test_lf_share_manager.py index 2f9c95b97..451eec950 100644 --- a/tests/modules/s3_datasets_shares/tasks/test_lf_share_manager.py +++ b/tests/modules/s3_datasets_shares/tasks/test_lf_share_manager.py @@ -17,7 +17,7 @@ from dataall.modules.shares_base.services.shares_enums import ShareItemStatus from dataall.modules.shares_base.db.share_object_models import ShareObject, ShareObjectItem from dataall.modules.s3_datasets.db.dataset_models import DatasetTable, S3Dataset -from dataall.modules.s3_datasets_shares.services.dataset_sharing_alarm_service import DatasetSharingAlarmService +from dataall.modules.s3_datasets_shares.services.s3_share_alarm_service import S3ShareAlarmService from dataall.modules.s3_datasets_shares.services.share_processors.glue_table_share_processor import ( ProcessLakeFormationShare, ) @@ -870,7 +870,7 @@ def test_check_catalog_account_exists_and_update_processor_with_catalog_doesnt_e def test_handle_share_failure(manager_with_mocked_clients, table1: DatasetTable, mocker): # Given - alarm_service_mock = mocker.patch.object(DatasetSharingAlarmService, 'trigger_table_sharing_failure_alarm') + alarm_service_mock = mocker.patch.object(S3ShareAlarmService, 'trigger_table_sharing_failure_alarm') error = Exception() manager, lf_client, glue_client, mock_glue_client = manager_with_mocked_clients @@ -887,7 +887,7 @@ def test_handle_revoke_failure( mocker, ): # Given - alarm_service_mock = mocker.patch.object(DatasetSharingAlarmService, 'trigger_revoke_table_sharing_failure_alarm') + alarm_service_mock = mocker.patch.object(S3ShareAlarmService, 'trigger_revoke_table_sharing_failure_alarm') error = Exception() manager, lf_client, glue_client, mock_glue_client = manager_with_mocked_clients diff --git a/tests/modules/s3_datasets_shares/tasks/test_s3_access_point_share_manager.py b/tests/modules/s3_datasets_shares/tasks/test_s3_access_point_share_manager.py index 7f8cd7ca2..b6ac04b86 100644 --- a/tests/modules/s3_datasets_shares/tasks/test_s3_access_point_share_manager.py +++ b/tests/modules/s3_datasets_shares/tasks/test_s3_access_point_share_manager.py @@ -10,7 +10,7 @@ from dataall.core.organizations.db.organization_models import Organization from dataall.modules.s3_datasets_shares.aws.s3_client import S3ControlClient from dataall.modules.shares_base.db.share_object_models import ShareObject, ShareObjectItem -from dataall.modules.s3_datasets_shares.services.managed_share_policy_service import SharePolicyService +from dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service import S3SharePolicyService from dataall.modules.s3_datasets_shares.services.share_managers import S3AccessPointShareManager from dataall.modules.s3_datasets.db.dataset_models import DatasetStorageLocation, S3Dataset from dataall.modules.shares_base.services.sharing_service import ShareData @@ -334,11 +334,11 @@ def test_grant_target_role_access_policy_test_empty_policy( return_value=None, ) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_exists', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_exists', return_value=True, ) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_attached', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_attached', return_value=True, ) @@ -373,7 +373,7 @@ def test_grant_target_role_access_policy_test_empty_policy( # When share_manager.grant_target_role_access_policy() - expected_policy_name = SharePolicyService( + expected_policy_name = S3SharePolicyService( environmentUri=target_environment.environmentUri, role_name=share1.principalIAMRoleName, account=target_environment.AwsAccountId, @@ -400,11 +400,11 @@ def test_grant_target_role_access_policy_existing_policy_bucket_not_included( mocker.patch('dataall.base.aws.iam.IAM.get_managed_policy_default_version', return_value=('v1', iam_policy)) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_exists', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_exists', return_value=True, ) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_attached', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_attached', return_value=True, ) @@ -424,10 +424,10 @@ def test_grant_target_role_access_policy_existing_policy_bucket_not_included( # Iam function is called with str from object so we transform back to object policy_object = iam_policy - s3_index = SharePolicyService._get_statement_by_sid( + s3_index = S3SharePolicyService._get_statement_by_sid( policy=policy_object, sid=f'{IAM_S3_ACCESS_POINTS_STATEMENT_SID}S3' ) - kms_index = SharePolicyService._get_statement_by_sid( + kms_index = S3SharePolicyService._get_statement_by_sid( policy=policy_object, sid=f'{IAM_S3_ACCESS_POINTS_STATEMENT_SID}KMS' ) @@ -440,7 +440,7 @@ def test_grant_target_role_access_policy_existing_policy_bucket_not_included( ) # Assert that statements for S3 bucket sharing are unaffected - s3_index = SharePolicyService._get_statement_by_sid(policy=policy_object, sid=f'{IAM_S3_BUCKETS_STATEMENT_SID}S3') + s3_index = S3SharePolicyService._get_statement_by_sid(policy=policy_object, sid=f'{IAM_S3_BUCKETS_STATEMENT_SID}S3') def test_grant_target_role_access_policy_existing_policy_bucket_included(mocker, share_manager): @@ -452,12 +452,12 @@ def test_grant_target_role_access_policy_existing_policy_bucket_included(mocker, mocker.patch('dataall.base.aws.iam.IAM.get_managed_policy_default_version', return_value=('v1', iam_policy)) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_exists', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_exists', return_value=True, ) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_attached', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_attached', return_value=True, ) @@ -903,12 +903,12 @@ def test_delete_target_role_access_policy_no_remaining_statement( 'dataall.base.aws.iam.IAM.get_managed_policy_default_version', return_value=('v1', existing_target_role_policy) ) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_exists', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_exists', return_value=True, ) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_attached', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_attached', return_value=True, ) @@ -928,7 +928,7 @@ def test_delete_target_role_access_policy_no_remaining_statement( # When we revoke IAM access to the target IAM role share_manager.revoke_target_role_access_policy() - expected_policy_name = SharePolicyService( + expected_policy_name = S3SharePolicyService( environmentUri=target_environment.environmentUri, role_name=share1.principalIAMRoleName, account=target_environment.AwsAccountId, @@ -1006,12 +1006,12 @@ def test_delete_target_role_access_policy_with_remaining_statement( ) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_exists', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_exists', return_value=True, ) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_attached', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_attached', return_value=True, ) @@ -1032,7 +1032,7 @@ def test_delete_target_role_access_policy_with_remaining_statement( share_manager.revoke_target_role_access_policy() # Then - expected_policy_name = SharePolicyService( + expected_policy_name = S3SharePolicyService( environmentUri=target_environment.environmentUri, role_name=share1.principalIAMRoleName, account=target_environment.AwsAccountId, @@ -1199,12 +1199,12 @@ def test_check_bucket_policy_missing_sid(mocker, base_bucket_policy, share_manag def test_check_target_role_access_policy(mocker, share_manager): # Given mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_exists', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_exists', return_value=True, ) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_attached', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_attached', return_value=True, ) @@ -1235,12 +1235,12 @@ def test_check_target_role_access_policy(mocker, share_manager): def test_check_target_role_access_policy_existing_policy_bucket_and_key_not_included(mocker, share_manager): # Given mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_exists', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_exists', return_value=True, ) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_attached', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_attached', return_value=True, ) @@ -1272,12 +1272,12 @@ def test_check_target_role_access_policy_existing_policy_bucket_and_key_not_incl def test_check_target_role_access_policy_test_no_policy(mocker, share_manager): # Given mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_exists', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_exists', return_value=False, ) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_attached', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_attached', return_value=True, ) @@ -1293,12 +1293,12 @@ def test_check_target_role_access_policy_test_no_policy(mocker, share_manager): def test_check_target_role_access_policy_test_policy_not_attached(mocker, share_manager): # Given mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_exists', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_exists', return_value=True, ) # Policy is not attached mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_attached', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_attached', return_value=False, ) diff --git a/tests/modules/s3_datasets_shares/tasks/test_s3_bucket_share_manager.py b/tests/modules/s3_datasets_shares/tasks/test_s3_bucket_share_manager.py index 3e560717a..840c20568 100644 --- a/tests/modules/s3_datasets_shares/tasks/test_s3_bucket_share_manager.py +++ b/tests/modules/s3_datasets_shares/tasks/test_s3_bucket_share_manager.py @@ -9,7 +9,7 @@ from dataall.core.organizations.db.organization_models import Organization from dataall.modules.shares_base.db.share_object_models import ShareObject from dataall.modules.s3_datasets_shares.services.share_managers import S3BucketShareManager -from dataall.modules.s3_datasets_shares.services.managed_share_policy_service import SharePolicyService +from dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service import S3SharePolicyService from dataall.modules.s3_datasets.db.dataset_models import S3Dataset, DatasetBucket from dataall.modules.shares_base.services.sharing_service import ShareData @@ -418,7 +418,7 @@ def test_grant_s3_iam_access_with_no_policy(mocker, dataset2, share2_manager): # Check if the get and update_role_policy func are called and policy statements are added mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_exists', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_exists', return_value=False, ) kms_client = mock_kms_client(mocker) @@ -429,15 +429,15 @@ def test_grant_s3_iam_access_with_no_policy(mocker, dataset2, share2_manager): 'Statement': [{'Sid': EMPTY_STATEMENT_SID, 'Effect': 'Allow', 'Action': 'none:null', 'Resource': '*'}], } share_policy_service_mock_1 = mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.create_managed_policy_from_inline_and_delete_inline', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.create_managed_policy_from_inline_and_delete_inline', return_value='arn:iam::someArn', ) share_policy_service_mock_2 = mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.attach_policy', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.attach_policy', return_value=True, ) share_policy_service_mock_3 = mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_attached', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_attached', return_value=False, ) iam_update_role_policy_mock_1 = mocker.patch( @@ -457,8 +457,8 @@ def test_grant_s3_iam_access_with_no_policy(mocker, dataset2, share2_manager): iam_policy = empty_policy_document - s3_index = SharePolicyService._get_statement_by_sid(policy=iam_policy, sid=f'{IAM_S3_BUCKETS_STATEMENT_SID}S3') - kms_index = SharePolicyService._get_statement_by_sid(policy=iam_policy, sid=f'{IAM_S3_BUCKETS_STATEMENT_SID}KMS') + s3_index = S3SharePolicyService._get_statement_by_sid(policy=iam_policy, sid=f'{IAM_S3_BUCKETS_STATEMENT_SID}S3') + kms_index = S3SharePolicyService._get_statement_by_sid(policy=iam_policy, sid=f'{IAM_S3_BUCKETS_STATEMENT_SID}KMS') # Assert if the IAM role policy with S3 and KMS permissions was created assert len(iam_policy['Statement']) == 2 @@ -484,11 +484,11 @@ def test_grant_s3_iam_access_with_empty_policy(mocker, dataset2, share2_manager) # Check if the get and update_role_policy func are called and policy statements are added mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_exists', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_exists', return_value=True, ) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_attached', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_attached', return_value=True, ) kms_client = mock_kms_client(mocker) @@ -513,8 +513,8 @@ def test_grant_s3_iam_access_with_empty_policy(mocker, dataset2, share2_manager) iam_policy = initial_policy_document - s3_index = SharePolicyService._get_statement_by_sid(policy=iam_policy, sid=f'{IAM_S3_BUCKETS_STATEMENT_SID}S3') - kms_index = SharePolicyService._get_statement_by_sid(policy=iam_policy, sid=f'{IAM_S3_BUCKETS_STATEMENT_SID}KMS') + s3_index = S3SharePolicyService._get_statement_by_sid(policy=iam_policy, sid=f'{IAM_S3_BUCKETS_STATEMENT_SID}S3') + kms_index = S3SharePolicyService._get_statement_by_sid(policy=iam_policy, sid=f'{IAM_S3_BUCKETS_STATEMENT_SID}KMS') # Assert if the IAM role policy with S3 and KMS permissions was created assert len(iam_policy['Statement']) == 2 @@ -558,15 +558,15 @@ def test_grant_s3_iam_access_with_policy_and_target_resources_not_present(mocker } mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_exists', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_exists', return_value=True, ) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_attached', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_attached', return_value=True, ) - s3_index = SharePolicyService._get_statement_by_sid(policy=policy, sid=f'{IAM_S3_BUCKETS_STATEMENT_SID}S3') - kms_index = SharePolicyService._get_statement_by_sid(policy=policy, sid=f'{IAM_S3_BUCKETS_STATEMENT_SID}KMS') + s3_index = S3SharePolicyService._get_statement_by_sid(policy=policy, sid=f'{IAM_S3_BUCKETS_STATEMENT_SID}S3') + kms_index = S3SharePolicyService._get_statement_by_sid(policy=policy, sid=f'{IAM_S3_BUCKETS_STATEMENT_SID}KMS') assert len(policy['Statement']) == 2 assert len(policy['Statement'][s3_index]['Resource']) == 2 @@ -630,11 +630,11 @@ def test_grant_s3_iam_access_with_complete_policy_present(mocker, dataset2, shar } mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_exists', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_exists', return_value=True, ) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_attached', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_attached', return_value=True, ) kms_client = mock_kms_client(mocker) @@ -654,8 +654,8 @@ def test_grant_s3_iam_access_with_complete_policy_present(mocker, dataset2, shar created_iam_policy = policy - s3_index = SharePolicyService._get_statement_by_sid(policy=policy, sid=f'{IAM_S3_BUCKETS_STATEMENT_SID}S3') - kms_index = SharePolicyService._get_statement_by_sid(policy=policy, sid=f'{IAM_S3_BUCKETS_STATEMENT_SID}KMS') + s3_index = S3SharePolicyService._get_statement_by_sid(policy=policy, sid=f'{IAM_S3_BUCKETS_STATEMENT_SID}S3') + kms_index = S3SharePolicyService._get_statement_by_sid(policy=policy, sid=f'{IAM_S3_BUCKETS_STATEMENT_SID}KMS') assert len(created_iam_policy['Statement']) == 2 assert ( @@ -920,7 +920,7 @@ def test_delete_target_role_access_no_policy_no_other_resources_shared( } mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_exists', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_exists', return_value=False, ) @@ -928,11 +928,11 @@ def test_delete_target_role_access_no_policy_no_other_resources_shared( kms_client().get_key_id.return_value = 'kms-key' share_policy_service_mock_1 = mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.create_managed_policy_from_inline_and_delete_inline', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.create_managed_policy_from_inline_and_delete_inline', return_value='arn:iam::someArn', ) share_policy_service_mock_2 = mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.attach_policy', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.attach_policy', return_value=True, ) @@ -955,10 +955,10 @@ def test_delete_target_role_access_no_policy_no_other_resources_shared( # Get the updated IAM policy and compare it with the existing one updated_iam_policy = policy_document - s3_index = SharePolicyService._get_statement_by_sid( + s3_index = S3SharePolicyService._get_statement_by_sid( policy=updated_iam_policy, sid=f'{IAM_S3_BUCKETS_STATEMENT_SID}S3' ) - kms_index = SharePolicyService._get_statement_by_sid( + kms_index = S3SharePolicyService._get_statement_by_sid( policy=updated_iam_policy, sid=f'{IAM_S3_BUCKETS_STATEMENT_SID}KMS' ) @@ -992,11 +992,11 @@ def test_delete_target_role_access_policy_no_resource_of_datasets_s3_bucket( } mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_exists', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_exists', return_value=True, ) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_attached', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_attached', return_value=True, ) @@ -1019,8 +1019,8 @@ def test_delete_target_role_access_policy_no_resource_of_datasets_s3_bucket( # Get the updated IAM policy and compare it with the existing one updated_iam_policy = iam_policy - s3_index = SharePolicyService._get_statement_by_sid(policy=iam_policy, sid=f'{IAM_S3_BUCKETS_STATEMENT_SID}S3') - kms_index = SharePolicyService._get_statement_by_sid(policy=iam_policy, sid=f'{IAM_S3_BUCKETS_STATEMENT_SID}KMS') + s3_index = S3SharePolicyService._get_statement_by_sid(policy=iam_policy, sid=f'{IAM_S3_BUCKETS_STATEMENT_SID}S3') + kms_index = S3SharePolicyService._get_statement_by_sid(policy=iam_policy, sid=f'{IAM_S3_BUCKETS_STATEMENT_SID}KMS') assert len(updated_iam_policy['Statement']) == 2 assert 'arn:aws:s3:::someOtherBucket,arn:aws:s3:::someOtherBucket/*' == ','.join( @@ -1065,11 +1065,11 @@ def test_delete_target_role_access_policy_with_multiple_s3_buckets_in_policy( } mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_exists', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_exists', return_value=True, ) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_attached', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_attached', return_value=True, ) @@ -1092,8 +1092,8 @@ def test_delete_target_role_access_policy_with_multiple_s3_buckets_in_policy( updated_iam_policy = iam_policy - s3_index = SharePolicyService._get_statement_by_sid(policy=iam_policy, sid=f'{IAM_S3_BUCKETS_STATEMENT_SID}S3') - kms_index = SharePolicyService._get_statement_by_sid(policy=iam_policy, sid=f'{IAM_S3_BUCKETS_STATEMENT_SID}KMS') + s3_index = S3SharePolicyService._get_statement_by_sid(policy=iam_policy, sid=f'{IAM_S3_BUCKETS_STATEMENT_SID}S3') + kms_index = S3SharePolicyService._get_statement_by_sid(policy=iam_policy, sid=f'{IAM_S3_BUCKETS_STATEMENT_SID}KMS') assert f'arn:aws:s3:::{dataset2.S3BucketName}' not in updated_iam_policy['Statement'][s3_index]['Resource'] assert f'arn:aws:s3:::{dataset2.S3BucketName}/*' not in updated_iam_policy['Statement'][s3_index]['Resource'] @@ -1139,11 +1139,11 @@ def test_delete_target_role_access_policy_with_one_s3_bucket_and_one_kms_resourc } mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_exists', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_exists', return_value=True, ) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_attached', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_attached', return_value=True, ) @@ -1397,11 +1397,11 @@ def test_check_s3_iam_access(mocker, dataset2, share2_manager): ], } mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_exists', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_exists', return_value=True, ) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_attached', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_attached', return_value=True, ) # Gets policy with S3 and KMS @@ -1425,7 +1425,7 @@ def test_check_s3_iam_access_no_policy(mocker, dataset2, share2_manager): # When policy does not exist iam_update_role_policy_mock_1 = mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_exists', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_exists', return_value=False, ) @@ -1446,11 +1446,11 @@ def test_check_s3_iam_access_policy_not_attached(mocker, dataset2, share2_manage # When policy does not exist iam_update_role_policy_mock_1 = mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_exists', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_exists', return_value=True, ) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_attached', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_attached', return_value=False, ) @@ -1481,11 +1481,11 @@ def test_check_s3_iam_access_missing_policy_statement(mocker, dataset2, share2_m ], } mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_exists', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_exists', return_value=True, ) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_attached', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_attached', return_value=True, ) # Gets policy with other S3 and KMS @@ -1529,11 +1529,11 @@ def test_check_s3_iam_access_missing_target_resource(mocker, dataset2, share2_ma ], } mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_exists', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_exists', return_value=True, ) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_attached', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_attached', return_value=True, ) # Gets policy with other S3 and KMS diff --git a/tests/modules/s3_datasets_shares/test_share.py b/tests/modules/s3_datasets_shares/test_share.py index 988b14f5e..68670a65b 100644 --- a/tests/modules/s3_datasets_shares/test_share.py +++ b/tests/modules/s3_datasets_shares/test_share.py @@ -25,8 +25,8 @@ @pytest.fixture(scope='function') def mock_glue_client(mocker): glue_client = MagicMock() - mocker.patch('dataall.modules.s3_datasets_shares.services.share_item_service.GlueClient', return_value=glue_client) - glue_client.get_source_catalog.return_value = None + mocker.patch('dataall.modules.s3_datasets_shares.aws.glue_client.GlueClient', return_value=glue_client) + glue_client.get_glue_database_from_catalog.return_value = None def random_table_name(): @@ -431,7 +431,7 @@ def create_share_object( } """ mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.create_managed_policy_from_inline_and_delete_inline', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.create_managed_policy_from_inline_and_delete_inline', return_value=True, ) @@ -864,11 +864,11 @@ def test_create_share_object_as_requester(mocker, client, user2, group2, env2gro # SharePolicy exists and is attached # When a user that belongs to environment and group creates request mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_exists', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_exists', return_value=True, ) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_attached', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_attached', return_value=True, ) create_share_object_response = create_share_object( @@ -893,11 +893,11 @@ def test_create_share_object_as_approver_and_requester(mocker, client, user, gro # SharePolicy exists and is attached # When a user that belongs to environment and group creates request mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_exists', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_exists', return_value=True, ) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_attached', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_attached', return_value=True, ) create_share_object_response = create_share_object( @@ -924,11 +924,11 @@ def test_create_share_object_with_item_authorized( # SharePolicy exists and is attached # When a user that belongs to environment and group creates request with table in the request mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_exists', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_exists', return_value=True, ) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_attached', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_attached', return_value=True, ) create_share_object_response = create_share_object( @@ -969,15 +969,15 @@ def test_create_share_object_share_policy_not_attached_attachMissingPolicies_ena # SharePolicy exists and is NOT attached, attachMissingPolicies=True # When a correct user creates request, data.all attaches the policy and the share creates successfully mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_exists', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_exists', return_value=True, ) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_attached', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_attached', return_value=False, ) attach_mocker = mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.attach_policy', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.attach_policy', return_value=True, ) create_share_object_response = create_share_object( @@ -1006,15 +1006,15 @@ def test_create_share_object_share_policy_not_attached_attachMissingPolicies_dis # SharePolicy exists and is NOT attached, attachMissingPolicies=True but principal=Group so managed=Trye # When a correct user creates request, data.all attaches the policy and the share creates successfully mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_exists', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_exists', return_value=True, ) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_attached', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_attached', return_value=False, ) attach_mocker = mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.attach_policy', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.attach_policy', return_value=True, ) create_share_object_response = create_share_object( @@ -1043,11 +1043,11 @@ def test_create_share_object_share_policy_not_attached_attachMissingPolicies_dis # SharePolicy exists and is NOT attached, attachMissingPolicies=True # When a correct user creates request, data.all attaches the policy and the share creates successfully mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_exists', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_exists', return_value=True, ) mocker.patch( - 'dataall.modules.s3_datasets_shares.services.managed_share_policy_service.SharePolicyService.check_if_policy_attached', + 'dataall.modules.s3_datasets_shares.services.s3_share_managed_policy_service.S3SharePolicyService.check_if_policy_attached', return_value=False, ) consumption_role = type('consumption_role', (object,), {})()