Skip to content

Commit

Permalink
Fix/get user timeout (#665)
Browse files Browse the repository at this point in the history
* feat: install pandas

* fix: add error handling for list groups with members

* feat: add tolerate errors support

* feat: specify arguments instead of kwargs
  • Loading branch information
gcharest authored Oct 1, 2024
1 parent 465436f commit f69c6a5
Show file tree
Hide file tree
Showing 4 changed files with 142 additions and 37 deletions.
32 changes: 25 additions & 7 deletions app/integrations/google_workspace/google_directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ def list_groups_with_members(
members_details: bool = True,
groups_filters: list = [],
query: str | None = None,
tolerate_errors: bool = False,
):
"""List all groups in the Google Workspace domain with their members.
Expand All @@ -183,6 +184,7 @@ def list_groups_with_members(
members_details (bool): Include the members details in the response.
groups_filters (list): List of filters to apply to the groups.
query (str): The query to search for groups.
tolerate_errors (bool): Whether to include groups that encountered errors during member detail retrieval.
Returns:
list: A list of group objects with members. Any group without members will not be included.
Expand All @@ -203,16 +205,32 @@ def list_groups_with_members(
groups_with_members = []
for group in groups:
logger.info(f"Getting members for group: {group['email']}")
members = list_group_members(
group["email"], fields="members(email, role, type, status)"
)
try:
members = list_group_members(
group["email"], fields="members(email, role, type, status)"
)
except Exception as e:
logger.warning(f"Error getting members for group {group['email']}: {e}")
continue

if members and members_details:
detailed_members = []
error_occurred = False
for member in members:
logger.info(f"Getting user details for member: {member['email']}")
detailed_members.append(
get_user(member["email"], fields="name, primaryEmail")
)
try:
logger.info(f"Getting user details for member: {member['email']}")
detailed_members.append(
get_user(member["email"], fields="name, primaryEmail")
)
except Exception as e:
logger.warning(
f"Error getting user details for member {member['email']}: {e}"
)
error_occurred = True
if not tolerate_errors:
break
if error_occurred and not tolerate_errors:
continue
group["members"] = detailed_members
groups_with_members.append(group)
return groups_with_members
62 changes: 32 additions & 30 deletions app/modules/aws/identity_center.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,30 @@
DRY_RUN = True


def synchronize(**kwargs):
def synchronize(
enable_users_sync: bool = True,
enable_user_create: bool = True,
enable_user_delete: bool = False,
enable_groups_sync: bool = True,
enable_membership_create: bool = True,
enable_membership_delete: bool = False,
query: str = "email:aws-*",
pre_processing_filters: list = [],
):
"""Sync the AWS Identity Center with the Google Workspace.
Args:
enable_users_sync (bool): Toggle to sync users.
enable_groups_sync (bool): Toggle to sync groups.
query (str): The query to filter the Google Groups.
enable_users_sync (bool): Enable the synchronization of users. Default is True.
enable_user_create (bool): Enable the creation of users. Default is True.
enable_user_delete (bool): Enable the deletion of users. Default is False.
enable_groups_sync (bool): Enable the synchronization of groups. Default is True.
enable_membership_create (bool): Enable the creation of group memberships. Default is True.
enable_membership_delete (bool): Enable the deletion of group memberships. Default is False.
query (str): The query to search for groups.
pre_processing_filters (list): List of filters to apply to the groups before processing the members.
Returns:
tuple: A tuple containing the users sync status and groups sync status.
"""
enable_users_sync = kwargs.pop("enable_users_sync", True)
enable_user_create = kwargs.pop("enable_user_create", True)
enable_user_delete = kwargs.pop("enable_user_delete", False)
enable_groups_sync = kwargs.pop("enable_groups_sync", True)
enable_membership_create = kwargs.pop("enable_membership_create", True)
enable_membership_delete = kwargs.pop("enable_membership_delete", False)
query = kwargs.pop("query", "email:aws-*")
pre_processing_filters = kwargs.pop("pre_processing_filters", [])

users_sync_status = None
groups_sync_status = None
Expand Down Expand Up @@ -56,7 +61,7 @@ def synchronize(**kwargs):

if enable_users_sync:
users_sync_status = sync_users(
source_users, target_users, enable_user_create, enable_user_delete, **kwargs
source_users, target_users, enable_user_create, enable_user_delete
)
target_users = identity_store.list_users()

Expand All @@ -67,7 +72,6 @@ def synchronize(**kwargs):
target_users,
enable_membership_create,
enable_membership_delete,
**kwargs,
)
logger.info("synchronize:Sync Completed")

Expand All @@ -78,26 +82,25 @@ def synchronize(**kwargs):


def sync_users(
source_users,
target_users,
enable_user_create=True,
enable_user_delete=False,
**kwargs,
source_users: list,
target_users: list,
enable_user_create: bool = True,
enable_user_delete: bool = False,
delete_target_all: bool = False,
):
"""Sync the users in the identity store.
Args:
source_users (list): A list of users from the source system.
target_users (list): A list of users in the identity store.
enable_user_create (bool): Enable creation of users.
enable_user_delete (bool): Enable deletion of users.
delete_target_all (bool): Mark all target users for deletion.
enable_user_create (bool): Enable creation of users. Default is True.
enable_user_delete (bool): Enable deletion of users. Default is False.
delete_target_all (bool): Mark all target users for deletion. Default is False.
Returns:
tuple: A tuple containing the users created and deleted.
"""
delete_target_all = kwargs.get("delete_target_all", False)

if delete_target_all:
users_to_delete = target_users
Expand Down Expand Up @@ -150,12 +153,11 @@ def sync_users(


def sync_groups(
source_groups,
target_groups,
target_users,
enable_membership_create=True,
enable_membership_delete=False,
**kwargs,
source_groups: list,
target_groups: list,
target_users: list,
enable_membership_create: bool = True,
enable_membership_delete: bool = False,
):
"""Sync the groups in the identity store.
Expand Down
1 change: 1 addition & 0 deletions app/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ google-api-core==2.19.2
google-auth==2.33.0
httpx==0.27.2
itsdangerous==2.2.0
pandas==2.2.3
PyJWT==2.9.0
PyYAML!=6.0.0,!=5.4.0,!=5.4.1
python-dotenv==0.21.1
Expand Down
84 changes: 84 additions & 0 deletions app/tests/integrations/google_workspace/test_google_directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,33 @@ def test_list_groups_with_members_filtered(
assert mock_get_user.call_count == 2


@patch("integrations.google_workspace.google_directory.list_groups")
@patch("integrations.google_workspace.google_directory.list_group_members")
@patch("integrations.google_workspace.google_directory.get_user")
def test_list_groups_with_members_error_in_list_group_members(
mock_get_user,
mock_list_group_members,
mock_list_groups,
google_groups,
google_group_members,
google_users,
google_groups_w_users,
):
groups = google_groups(2)
group_members = [Exception("Error fetching group members"), google_group_members(2)]
users = google_users(2)

mock_list_groups.return_value = groups
mock_list_group_members.side_effect = group_members
mock_get_user.side_effect = users

# Only the second group should be processed
expected_groups_with_users = [groups[1]]
expected_groups_with_users[0]["members"] = users

assert google_directory.list_groups_with_members() == expected_groups_with_users


@patch("integrations.google_workspace.google_directory.list_groups")
@patch("integrations.google_workspace.google_directory.list_group_members")
@patch("integrations.google_workspace.google_directory.get_user")
Expand Down Expand Up @@ -437,6 +464,63 @@ def test_list_groups_with_members_without_members_enabled(
mock_get_user.assert_not_called()


@patch("integrations.google_workspace.google_directory.list_groups")
@patch("integrations.google_workspace.google_directory.list_group_members")
@patch("integrations.google_workspace.google_directory.get_user")
def test_list_groups_with_members_error_in_get_user(
mock_get_user,
mock_list_group_members,
mock_list_groups,
google_groups,
google_group_members,
google_users,
):
groups = google_groups(2)
group_members = [google_group_members(2), google_group_members(2)]
users = [Exception("Error fetching user details"), google_users(1)[0]]

mock_list_groups.return_value = groups
mock_list_group_members.side_effect = group_members
mock_get_user.side_effect = users

# No groups should be processed due to error in get_user
assert google_directory.list_groups_with_members() == []


@patch("integrations.google_workspace.google_directory.list_groups")
@patch("integrations.google_workspace.google_directory.list_group_members")
@patch("integrations.google_workspace.google_directory.get_user")
def test_list_groups_with_members_tolerate_errors(
mock_get_user,
mock_list_group_members,
mock_list_groups,
google_groups,
google_group_members,
google_users,
):
groups = google_groups(2)
group_members = [google_group_members(2), google_group_members(2)]
users = [
Exception("Error fetching user details"),
google_users(1)[0],
Exception("Error fetching user details"),
google_users(1)[0],
]

mock_list_groups.return_value = groups
mock_list_group_members.side_effect = group_members
mock_get_user.side_effect = users

# Expected result should include both groups, with the second group having one member
expected_groups_with_users = [groups[0], groups[1]]
expected_groups_with_users[0]["members"] = []
expected_groups_with_users[1]["members"] = [google_users(1)[0]]

result = google_directory.list_groups_with_members(tolerate_errors=True)

assert result == expected_groups_with_users


@patch("integrations.google_workspace.google_directory.list_groups")
def test_list_groups_with_members_skips_when_no_groups(mock_list_groups):
mock_list_groups.return_value = []
Expand Down

0 comments on commit f69c6a5

Please sign in to comment.