From eb4915eb67e5d346478adba91b43d1cc01abb3dc Mon Sep 17 00:00:00 2001 From: Sofia Sazonova Date: Wed, 11 Dec 2024 14:52:11 +0000 Subject: [PATCH] fix persistent_crossacc shares tests --- .../shares/s3_datasets_shares/conftest.py | 14 +++--- .../shared_test_functions.py | 13 ++--- .../test_new_crossacc_s3_share.py | 34 ++++++++----- .../test_persistent_crossacc_share.py | 48 +++++++++++-------- .../integration_tests/modules/shares/types.py | 5 -- 5 files changed, 65 insertions(+), 49 deletions(-) diff --git a/tests_new/integration_tests/modules/shares/s3_datasets_shares/conftest.py b/tests_new/integration_tests/modules/shares/s3_datasets_shares/conftest.py index 535480897..60c24f143 100644 --- a/tests_new/integration_tests/modules/shares/s3_datasets_shares/conftest.py +++ b/tests_new/integration_tests/modules/shares/s3_datasets_shares/conftest.py @@ -194,11 +194,11 @@ def principal1(request, group5, session_consumption_role_1): @pytest.fixture(params=['Group', 'ConsumptionRole']) -def share_params_main(request, session_share_1, session_share_consrole_1, session_s3_dataset1): +def share_params_main(request, session_share_1, session_cross_acc_env_1, session_share_consrole_1, session_s3_dataset1): if request.param == 'Group': - yield session_share_1, session_s3_dataset1 + yield session_share_1, session_s3_dataset1, session_cross_acc_env_1 else: - yield session_share_consrole_1, session_s3_dataset1 + yield session_share_consrole_1, session_s3_dataset1, session_cross_acc_env_1 @pytest.fixture(params=[(False, 'Group'), (True, 'Group'), (False, 'ConsumptionRole'), (True, 'ConsumptionRole')]) @@ -315,8 +315,10 @@ def persistent_role_share_1( @pytest.fixture(params=['Group', 'ConsumptionRole']) -def persistent_share_params_main(request, persistent_role_share_1, persistent_group_share_1): +def persistent_share_params_main( + request, persistent_cross_acc_env_1, persistent_role_share_1, persistent_group_share_1 +): if request.param == 'Group': - yield persistent_group_share_1 + yield persistent_group_share_1, persistent_cross_acc_env_1 else: - yield persistent_role_share_1 + yield persistent_role_share_1, persistent_cross_acc_env_1 diff --git a/tests_new/integration_tests/modules/shares/s3_datasets_shares/shared_test_functions.py b/tests_new/integration_tests/modules/shares/s3_datasets_shares/shared_test_functions.py index b28a5bdeb..b7e101ca3 100644 --- a/tests_new/integration_tests/modules/shares/s3_datasets_shares/shared_test_functions.py +++ b/tests_new/integration_tests/modules/shares/s3_datasets_shares/shared_test_functions.py @@ -124,19 +124,20 @@ def check_bucket_access(client, s3_client, bucket_name, should_have_access): def check_accesspoint_access(client, s3_client, access_point_arn, item_uri, should_have_access): + folder = get_folder(client, item_uri) if should_have_access: - folder = get_folder(client, item_uri) assert_that(s3_client.list_accesspoint_folder_objects(access_point_arn, folder.S3Prefix + '/')).is_not_none() else: - assert_that(get_folder).raises(Exception).when_called_with(client, item_uri).contains( - 'is not authorized to perform: GET_DATASET_FOLDER' - ) + assert_that(s3_client.list_accesspoint_folder_objects).raises(ClientError).when_called_with( + access_point_arn, folder.S3Prefix + '/' + ).contains('AccessDenied') def check_share_items_access( client, group, shareUri, + share_environment, consumption_role, env_client, ): @@ -144,7 +145,7 @@ def check_share_items_access( dataset = share.dataset principal_type = share.principal.principalType if principal_type == 'Group': - credentials_str = get_environment_access_token(client, share.environment.environmentUri, group) + credentials_str = get_environment_access_token(client, share_environment.environmentUri, group) credentials = json.loads(credentials_str) session = boto3.Session( aws_access_key_id=credentials['AccessKey'], @@ -169,7 +170,7 @@ def check_share_items_access( f'arn:aws:s3:{dataset.region}:{dataset.AwsAccountId}:accesspoint/{consumption_data.s3AccessPointName}' ) if principal_type == 'Group': - workgroup = athena_client.get_env_work_group(share.environment.label) + workgroup = athena_client.get_env_work_group(share_environment.label) athena_workgroup_output_location = None else: workgroup = 'primary' diff --git a/tests_new/integration_tests/modules/shares/s3_datasets_shares/test_new_crossacc_s3_share.py b/tests_new/integration_tests/modules/shares/s3_datasets_shares/test_new_crossacc_s3_share.py index e54e04db4..e0be2fa46 100644 --- a/tests_new/integration_tests/modules/shares/s3_datasets_shares/test_new_crossacc_s3_share.py +++ b/tests_new/integration_tests/modules/shares/s3_datasets_shares/test_new_crossacc_s3_share.py @@ -137,7 +137,7 @@ def test_reject_share(client1, client5, session_cross_acc_env_1, session_s3_data def test_change_share_purpose(client5, share_params_main): - share, dataset = share_params_main + share, dataset, _ = share_params_main change_request_purpose = update_share_request_reason(client5, share.shareUri, 'new purpose') assert_that(change_request_purpose).is_true() updated_share = get_share_object(client5, share.shareUri) @@ -153,19 +153,19 @@ def test_submit_object(client5, share_params_all): @pytest.mark.dependency(name='share_approved', depends=['share_submitted']) def test_approve_share(client1, share_params_main): - share, dataset = share_params_main + share, dataset, _ = share_params_main check_approve_share_object(client1, share.shareUri) @pytest.mark.dependency(name='share_succeeded', depends=['share_approved']) def test_share_succeeded(client1, share_params_main): - share, dataset = share_params_main + share, dataset, _ = share_params_main check_share_succeeded(client1, share.shareUri, check_contains_all_item_types=True) @pytest.mark.dependency(name='share_verified', depends=['share_succeeded']) def test_verify_share_items(client1, share_params_main): - share, dataset = share_params_main + share, dataset, _ = share_params_main check_verify_share_items(client1, share.shareUri) @@ -173,9 +173,14 @@ def test_verify_share_items(client1, share_params_main): def test_check_item_access( client5, session_cross_acc_env_1_aws_client, share_params_main, group5, session_consumption_role_1 ): - share, dataset = share_params_main + share, dataset, share_environment = share_params_main check_share_items_access( - client5, group5, share.shareUri, session_consumption_role_1, session_cross_acc_env_1_aws_client + client5, + group5, + share.shareUri, + share_environment, + session_consumption_role_1, + session_cross_acc_env_1_aws_client, ) @@ -183,7 +188,7 @@ def test_check_item_access( def test_unhealthy_items( client5, session_cross_acc_env_1_aws_client, session_cross_acc_env_1_integration_role_arn, share_params_main ): - share, _ = share_params_main + share, _, __ = share_params_main iam = session_cross_acc_env_1_aws_client.resource('iam') principal_role = iam.Role(share.principal.principalRoleName) # break s3 by removing policies @@ -209,7 +214,7 @@ def test_unhealthy_items( @pytest.mark.dependency(depends=['share_approved']) def test_reapply_unauthoried(client5, share_params_main): - share, _ = share_params_main + share, _, __ = share_params_main share_uri = share.shareUri share_object = get_share_object(client5, share_uri) item_uris = [item.shareItemUri for item in share_object['items'].nodes] @@ -220,7 +225,7 @@ def test_reapply_unauthoried(client5, share_params_main): @pytest.mark.dependency(depends=['share_approved']) def test_reapply(client1, share_params_main): - share, _ = share_params_main + share, _, __ = share_params_main share_uri = share.shareUri share_object = get_share_object(client1, share_uri) item_uris = [item.shareItemUri for item in share_object['items'].nodes] @@ -233,7 +238,7 @@ def test_reapply(client1, share_params_main): @pytest.mark.dependency(name='share_revoked', depends=['share_succeeded']) def test_revoke_share(client1, share_params_main): - share, dataset = share_params_main + share, dataset, _ = share_params_main check_share_ready(client1, share.shareUri) revoke_and_check_all_shared_items(client1, share.shareUri, check_contains_all_item_types=True) @@ -242,8 +247,13 @@ def test_revoke_share(client1, share_params_main): def test_revoke_succeeded( client1, client5, session_cross_acc_env_1_aws_client, share_params_main, group5, session_consumption_role_1 ): - share, dataset = share_params_main + share, dataset, share_environment = share_params_main check_all_items_revoke_job_succeeded(client1, share.shareUri, check_contains_all_item_types=True) check_share_items_access( - client5, group5, share.shareUri, session_consumption_role_1, session_cross_acc_env_1_aws_client + client5, + group5, + share.shareUri, + share_environment, + session_consumption_role_1, + session_cross_acc_env_1_aws_client, ) diff --git a/tests_new/integration_tests/modules/shares/s3_datasets_shares/test_persistent_crossacc_share.py b/tests_new/integration_tests/modules/shares/s3_datasets_shares/test_persistent_crossacc_share.py index a7567db82..ad5a09f4f 100644 --- a/tests_new/integration_tests/modules/shares/s3_datasets_shares/test_persistent_crossacc_share.py +++ b/tests_new/integration_tests/modules/shares/s3_datasets_shares/test_persistent_crossacc_share.py @@ -25,26 +25,28 @@ def test_verify_share_items(client5, persistent_share_params_main): - check_verify_share_items(client5, persistent_share_params_main.shareUri) + share, _ = persistent_share_params_main + check_verify_share_items(client5, share.shareUri) def test_check_share_items_access( client5, group5, persistent_share_params_main, persistent_consumption_role_1, persistent_cross_acc_env_1_aws_client ): + share, env = persistent_share_params_main check_share_items_access( client5, group5, - persistent_share_params_main.shareUri, + share.shareUri, + env, persistent_consumption_role_1, persistent_cross_acc_env_1_aws_client, ) def test_revoke_share(client1, persistent_share_params_main): - check_share_ready(client1, persistent_share_params_main.shareUri) - revoke_and_check_all_shared_items( - client1, persistent_share_params_main.shareUri, check_contains_all_item_types=True - ) + share, _ = persistent_share_params_main + check_share_ready(client1, share.shareUri) + revoke_and_check_all_shared_items(client1, share.shareUri, check_contains_all_item_types=True) def test_revoke_succeeded( @@ -55,45 +57,51 @@ def test_revoke_succeeded( persistent_consumption_role_1, persistent_cross_acc_env_1_aws_client, ): - check_all_items_revoke_job_succeeded( - client1, persistent_share_params_main.shareUri, check_contains_all_item_types=True - ) + share, env = persistent_share_params_main + check_all_items_revoke_job_succeeded(client1, share.shareUri, check_contains_all_item_types=True) check_share_items_access( client5, group5, - persistent_share_params_main.shareUri, + share.shareUri, + env, persistent_consumption_role_1, persistent_cross_acc_env_1_aws_client, ) def test_delete_all_nonshared_items(client5, persistent_share_params_main): - check_share_ready(client5, persistent_share_params_main.shareUri) - delete_all_non_shared_items(client5, persistent_share_params_main.shareUri) + share, _ = persistent_share_params_main + check_share_ready(client5, share.shareUri) + delete_all_non_shared_items(client5, share.shareUri) def test_add_items_back_to_share(client5, persistent_share_params_main): - check_share_ready(client5, persistent_share_params_main.shareUri) - add_all_items_to_share(client5, persistent_share_params_main.shareUri) + share, _ = persistent_share_params_main + check_share_ready(client5, share.shareUri) + add_all_items_to_share(client5, share.shareUri) def test_submit_share(client5, persistent_share_params_main, persistent_s3_dataset1): - check_submit_share_object(client5, persistent_share_params_main.shareUri, persistent_s3_dataset1) + share, _ = persistent_share_params_main + check_submit_share_object(client5, share.shareUri, persistent_s3_dataset1) def test_approve_share(client1, persistent_share_params_main): - check_approve_share_object(client1, persistent_share_params_main.shareUri) + share, _ = persistent_share_params_main + check_approve_share_object(client1, share.shareUri) def test_re_share_succeeded( client5, persistent_share_params_main, persistent_consumption_role_1, persistent_cross_acc_env_1_aws_client ): - check_share_succeeded(client5, persistent_share_params_main.shareUri, check_contains_all_item_types=True) - check_verify_share_items(client5, persistent_share_params_main.shareUri) + share, env = persistent_share_params_main + check_share_succeeded(client5, share.shareUri, check_contains_all_item_types=True) + check_verify_share_items(client5, share.shareUri) check_share_items_access( client5, - persistent_share_params_main.group, - persistent_share_params_main.shareUri, + share.group, + share.shareUri, + env, persistent_consumption_role_1, persistent_cross_acc_env_1_aws_client, ) diff --git a/tests_new/integration_tests/modules/shares/types.py b/tests_new/integration_tests/modules/shares/types.py index 844e8a3b7..099a4aad0 100644 --- a/tests_new/integration_tests/modules/shares/types.py +++ b/tests_new/integration_tests/modules/shares/types.py @@ -69,11 +69,6 @@ items(filter: $filter){{ {SharedItemSearchResult} }}, -environment{{ - environmentUri - label - region -}} canViewLogs, userRoleForShareObject, """