From 938730c0760e84139baeda0656ab3488ff33d45e Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 22 Feb 2017 18:34:05 -0500 Subject: [PATCH] Move sync team into its own module and add tests --- data/model/team.py | 9 ++ data/users/teamsync.py | 119 +++++++++++++++++ data/users/test/test_teamsync.py | 220 +++++++++++++++++++++++++++++++ workers/teamsyncworker.py | 100 +------------- 4 files changed, 350 insertions(+), 98 deletions(-) create mode 100644 data/users/teamsync.py create mode 100644 data/users/test/test_teamsync.py diff --git a/data/model/team.py b/data/model/team.py index 9def9fdb4..0ca4cf328 100644 --- a/data/model/team.py +++ b/data/model/team.py @@ -410,6 +410,15 @@ def list_team_users(team): .where(Team.id == team, User.robot == False)) +def list_team_robots(team): + """ Returns an iterator of all the *robots* found in a team. Does not include users. """ + return (User + .select() + .join(TeamMember) + .join(Team) + .where(Team.id == team, User.robot == True)) + + def set_team_syncing(team, login_service_name, config): """ Sets the given team to sync to the given service using the given config. """ login_service = LoginService.get(name=login_service_name) diff --git a/data/users/teamsync.py b/data/users/teamsync.py new file mode 100644 index 000000000..b44d600e8 --- /dev/null +++ b/data/users/teamsync.py @@ -0,0 +1,119 @@ +import logging +import json + +from data import model + +logger = logging.getLogger(__name__) + + +def sync_teams_to_groups(authentication, stale_cutoff): + """ Performs team syncing by looking up any stale team(s) found, and performing the sync + operation on them. + """ + logger.debug('Looking up teams to sync to groups') + + sync_team_tried = set() + while True: + # Find a stale team. + stale_team_sync = model.team.get_stale_team(stale_cutoff) + if not stale_team_sync: + logger.debug('No additional stale team found; sleeping') + return + + # Make sure we don't try to reprocess a team on this iteration. + if stale_team_sync.id in sync_team_tried: + break + + sync_team_tried.add(stale_team_sync.id) + + # Sync the team. + if not sync_team(authentication, stale_team_sync): + return + + +def sync_team(authentication, stale_team_sync): + """ Performs synchronization of a team (as referenced by the TeamSync stale_team_sync). + Returns True on success and False otherwise. + """ + sync_config = json.loads(stale_team_sync.config) + logger.info('Syncing team `%s` under organization %s via %s (#%s)', stale_team_sync.team.name, + stale_team_sync.team.organization.username, sync_config, stale_team_sync.team_id) + + # Load all the existing members of the team in Quay that are bound to the auth service. + existing_users = model.team.list_federated_team_members(stale_team_sync.team, + authentication.federated_service) + + logger.debug('Existing membership of %s for team `%s` under organization %s via %s (#%s)', + len(existing_users), stale_team_sync.team.name, + stale_team_sync.team.organization.username, sync_config, stale_team_sync.team_id) + + # Load all the members of the team from the authenication system. + (member_iterator, err) = authentication.iterate_group_members(sync_config) + if err is not None: + logger.error('Got error when trying to iterate group members with config %s: %s', + sync_config, err) + return False + + # Collect all the members currently found in the group, adding them to the team as we go + # along. + group_membership = set() + for (member_info, err) in member_iterator: + if err is not None: + logger.error('Got error when trying to construct a member: %s', err) + continue + + # If the member is already in the team, nothing more to do. + if member_info.username in existing_users: + logger.debug('Member %s already in team `%s` under organization %s via %s (#%s)', + member_info.username, stale_team_sync.team.name, + stale_team_sync.team.organization.username, sync_config, + stale_team_sync.team_id) + + group_membership.add(existing_users[member_info.username]) + continue + + # Retrieve the Quay user associated with the member info. + (quay_user, err) = authentication.get_federated_user(member_info) + if err is not None: + logger.error('Could not link external user %s to an internal user: %s', + member_info.username, err) + continue + + # Add the user to the membership set. + group_membership.add(quay_user.id) + + # Add the user to the team. + try: + logger.info('Adding member %s to team `%s` under organization %s via %s (#%s)', + quay_user.username, stale_team_sync.team.name, + stale_team_sync.team.organization.username, sync_config, + stale_team_sync.team_id) + + model.team.add_user_to_team(quay_user, stale_team_sync.team) + except model.UserAlreadyInTeam: + # If the user is already present, nothing more to do for them. + pass + + # Update the transaction and last_updated time of the team sync. Only if it matches + # the current value will we then perform the deletion step. + result = model.team.update_sync_status(stale_team_sync) + if not result: + # Another worker updated this team. Nothing more to do. + logger.debug('Another worker synced team `%s` under organization %s via %s (#%s)', + stale_team_sync.team.name, + stale_team_sync.team.organization.username, sync_config, + stale_team_sync.team_id) + return True + + # Delete any team members not found in the backing auth system. + logger.debug('Deleting stale members for team `%s` under organization %s via %s (#%s)', + stale_team_sync.team.name, stale_team_sync.team.organization.username, + sync_config, stale_team_sync.team_id) + + deleted = model.team.delete_members_not_present(stale_team_sync.team, group_membership) + + # Done! + logger.info('Finishing sync for team `%s` under organization %s via %s (#%s): %s deleted', + stale_team_sync.team.name, stale_team_sync.team.organization.username, + sync_config, stale_team_sync.team_id, deleted) + return True diff --git a/data/users/test/test_teamsync.py b/data/users/test/test_teamsync.py new file mode 100644 index 000000000..d51739d15 --- /dev/null +++ b/data/users/test/test_teamsync.py @@ -0,0 +1,220 @@ +import pytest + +from datetime import timedelta +from mock import patch + +from data import model, database +from data.users.federated import FederatedUsers, UserInformation +from data.users.teamsync import sync_team, sync_teams_to_groups +from endpoints.test.fixtures import app, appconfig, database_uri, init_db_path, sqlitedb_file +from util.names import parse_robot_username + +_FAKE_AUTH = 'fake' + +class FakeUsers(FederatedUsers): + def __init__(self, group_members): + super(FakeUsers, self).__init__(_FAKE_AUTH, False) + self.group_tuples = [(m, None) for m in group_members] + + def iterate_group_members(self, group_lookup_args, page_size=None, disable_pagination=False): + return (self.group_tuples, None) + + +@pytest.mark.parametrize('starting_membership,group_membership,expected_membership', [ + # Empty team + single member in group => Single member in team. + ([], + [ + UserInformation('someuser', 'someuser', 'someuser@devtable.com'), + ], + ['someuser']), + + # Team with a Quay user + empty group => empty team. + ([('someuser', None)], + [], + []), + + # Team with an existing external user + user is in the group => no changes. + ([ + ('someuser', 'someuser'), + ], + [ + UserInformation('someuser', 'someuser', 'someuser@devtable.com'), + ], + ['someuser']), + + # Team with an existing external user (with a different Quay username) + user is in the group. + # => no changes + ([ + ('anotherquayname', 'someuser'), + ], + [ + UserInformation('someuser', 'someuser', 'someuser@devtable.com'), + ], + ['someuser']), + + # Team missing a few members that are in the group => members added. + ([('someuser', 'someuser')], + [ + UserInformation('anotheruser', 'anotheruser', 'anotheruser@devtable.com'), + UserInformation('someuser', 'someuser', 'someuser@devtable.com'), + UserInformation('thirduser', 'thirduser', 'thirduser@devtable.com'), + ], + ['anotheruser', 'someuser', 'thirduser']), + + # Team has a few extra members no longer in the group => members removed. + ([ + ('anotheruser', 'anotheruser'), + ('someuser', 'someuser'), + ('thirduser', 'thirduser'), + ('nontestuser', None), + ], + [ + UserInformation('thirduser', 'thirduser', 'thirduser@devtable.com'), + ], + ['thirduser']), + + # Team has different membership than the group => members added and removed. + ([ + ('anotheruser', 'anotheruser'), + ('someuser', 'someuser'), + ('nontestuser', None), + ], + [ + UserInformation('anotheruser', 'anotheruser', 'anotheruser@devtable.com'), + UserInformation('missinguser', 'missinguser', 'missinguser@devtable.com'), + ], + ['anotheruser', 'missinguser']), + + # Team has same membership but some robots => robots remain and no other changes. + ([ + ('someuser', 'someuser'), + ('buynlarge+anotherbot', None), + ('buynlarge+somerobot', None), + ], + [ + UserInformation('someuser', 'someuser', 'someuser@devtable.com'), + ], + ['someuser', 'buynlarge+somerobot', 'buynlarge+anotherbot']), + + # Team has an extra member and some robots => member removed and robots remain. + ([ + ('someuser', 'someuser'), + ('buynlarge+anotherbot', None), + ('buynlarge+somerobot', None), + ], + [ + # No members. + ], + ['buynlarge+somerobot', 'buynlarge+anotherbot']), + + # Team has a different member and some robots => member changed and robots remain. + ([ + ('someuser', 'someuser'), + ('buynlarge+anotherbot', None), + ('buynlarge+somerobot', None), + ], + [ + UserInformation('anotheruser', 'anotheruser', 'anotheruser@devtable.com'), + ], + ['anotheruser', 'buynlarge+somerobot', 'buynlarge+anotherbot']), + + # Team with an existing external user (with a different Quay username) + user is in the group. + # => no changes and robots remain. + ([ + ('anotherquayname', 'someuser'), + ('buynlarge+anotherbot', None), + ], + [ + UserInformation('someuser', 'someuser', 'someuser@devtable.com'), + ], + ['someuser', 'buynlarge+anotherbot']), +]) +def test_syncing(starting_membership, group_membership, expected_membership, app): + org = model.organization.get_organization('buynlarge') + + # Necessary for the fake auth entries to be created in FederatedLogin. + database.LoginService.create(name=_FAKE_AUTH) + + # Assert the team is empty, so we have a clean slate. + sync_team_info = model.team.get_team_sync_information('buynlarge', 'synced') + assert len(list(model.team.list_team_users(sync_team_info.team))) == 0 + + # Add the existing starting members to the team. + for starting_member in starting_membership: + (quay_username, fakeauth_username) = starting_member + if '+' in quay_username: + # Add a robot. + (_, shortname) = parse_robot_username(quay_username) + robot, _ = model.user.create_robot(shortname, org) + model.team.add_user_to_team(robot, sync_team_info.team) + else: + email = quay_username + '@devtable.com' + + if fakeauth_username is None: + quay_user = model.user.create_user_noverify(quay_username, email) + else: + quay_user = model.user.create_federated_user(quay_username, email, _FAKE_AUTH, + fakeauth_username, False) + + model.team.add_user_to_team(quay_user, sync_team_info.team) + + # Call syncing on the team. + fake_auth = FakeUsers(group_membership) + assert sync_team(fake_auth, sync_team_info) + + # Ensure the last updated time and transaction_id's have changed. + updated_sync_info = model.team.get_team_sync_information('buynlarge', 'synced') + assert updated_sync_info.last_updated is not None + assert updated_sync_info.transaction_id != sync_team_info.transaction_id + + users_expected = set([name for name in expected_membership if '+' not in name]) + robots_expected = set([name for name in expected_membership if '+' in name]) + assert len(users_expected) + len(robots_expected) == len(expected_membership) + + # Check that the team's users match those expected. + service_user_map = model.team.list_federated_team_members(sync_team_info.team, _FAKE_AUTH) + assert set(service_user_map.keys()) == users_expected + + quay_users = model.team.list_team_users(sync_team_info.team) + assert len(quay_users) == len(users_expected) + + for quay_user in quay_users: + fakeauth_record = model.user.lookup_federated_login(quay_user, _FAKE_AUTH) + assert fakeauth_record is not None + assert fakeauth_record.service_ident in users_expected + assert service_user_map[fakeauth_record.service_ident] == quay_user.id + + # Check that the team's robots match those expected. + robots_found = set([r.username for r in model.team.list_team_robots(sync_team_info.team)]) + assert robots_expected == robots_found + + +def test_sync_teams_to_groups(app): + # Necessary for the fake auth entries to be created in FederatedLogin. + database.LoginService.create(name=_FAKE_AUTH) + + # Assert the team has not yet been updated. + sync_team_info = model.team.get_team_sync_information('buynlarge', 'synced') + assert sync_team_info.last_updated is None + + # Call to sync all teams. + fake_auth = FakeUsers([]) + sync_teams_to_groups(fake_auth, timedelta(seconds=1)) + + # Ensure the team was synced. + updated_sync_info = model.team.get_team_sync_information('buynlarge', 'synced') + assert updated_sync_info.last_updated is not None + assert updated_sync_info.transaction_id != sync_team_info.transaction_id + + # Set the stale threshold to a high amount and ensure the team is not resynced. + sync_teams_to_groups(fake_auth, timedelta(seconds=120)) + + third_sync_info = model.team.get_team_sync_information('buynlarge', 'synced') + assert third_sync_info.last_updated == updated_sync_info.last_updated + assert third_sync_info.transaction_id == updated_sync_info.transaction_id + + # Set the stale threshold to -1 seconds, and ensure the team is resynced. + sync_teams_to_groups(fake_auth, timedelta(seconds=-1)) + + fourth_sync_info = model.team.get_team_sync_information('buynlarge', 'synced') + assert fourth_sync_info.transaction_id != updated_sync_info.transaction_id diff --git a/workers/teamsyncworker.py b/workers/teamsyncworker.py index d658bd64f..225118834 100644 --- a/workers/teamsyncworker.py +++ b/workers/teamsyncworker.py @@ -1,9 +1,8 @@ import logging import time -import json from app import app, authentication -from data import model +from data.users.teamsync import sync_teams_to_groups from workers.worker import Worker from util.timedeltastring import convert_to_timedelta @@ -20,103 +19,8 @@ class TeamSynchronizationWorker(Worker): self.add_operation(self._sync_teams_to_groups, WORKER_FREQUENCY) def _sync_teams_to_groups(self): - logger.debug('Looking up teams to sync to groups') + sync_teams_to_groups(authentication, STALE_CUTOFF) - sync_team_tried = set() - while True: - # Find a stale team. - stale_team_sync = model.team.get_stale_team(STALE_CUTOFF) - if not stale_team_sync: - logger.debug('No additional stale team found; sleeping') - return - - # Make sure we don't try to reprocess a team on this iteration. - if stale_team_sync.id in sync_team_tried: - continue - - sync_team_tried.add(stale_team_sync.id) - - sync_config = json.loads(stale_team_sync.config) - logger.info('Syncing team `%s` under organization %s via %s (#%s)', stale_team_sync.team.name, - stale_team_sync.team.organization.username, sync_config, stale_team_sync.team_id) - - # Load all the existing members of the team in Quay that are bound to the auth service. - existing_users = model.team.list_federated_team_members(stale_team_sync.team, - authentication.federated_service) - - logger.debug('Existing membership of %s for team `%s` under organization %s via %s (#%s)', - len(existing_users), stale_team_sync.team.name, - stale_team_sync.team.organization.username, sync_config, stale_team_sync.team_id) - - # Load all the members of the team from the authenication system. - (member_iterator, err) = authentication.iterate_group_members(sync_config) - if err is not None: - logger.error('Got error when trying to iterate group members with config %s: %s', - sync_config, err) - continue - - # Collect all the members currently found in the group, adding them to the team as we go - # along. - group_membership = set() - for (member_info, err) in member_iterator: - if err is not None: - logger.error('Got error when trying to construct a member: %s', err) - continue - - # If the member is already in the team, nothing more to do. - if member_info.username in existing_users: - logger.debug('Member %s already in team `%s` under organization %s via %s (#%s)', - member_info.username, stale_team_sync.team.name, - stale_team_sync.team.organization.username, sync_config, - stale_team_sync.team_id) - - group_membership.add(existing_users[member_info.username]) - continue - - # Retrieve the Quay user associated with the member info. - (quay_user, err) = authentication.get_federated_user(member_info) - if err is not None: - logger.error('Could not link external user %s to an internal user: %s', - member_info.username, err) - continue - - # Add the user to the membership set. - group_membership.add(quay_user.id) - - # Add the user to the team. - try: - logger.info('Adding member %s to team `%s` under organization %s via %s (#%s)', - quay_user.username, stale_team_sync.team.name, - stale_team_sync.team.organization.username, sync_config, - stale_team_sync.team_id) - - model.team.add_user_to_team(quay_user, stale_team_sync.team) - except model.UserAlreadyInTeam: - # If the user is already present, nothing more to do for them. - pass - - # Update the transaction and last_updated time of the team sync. Only if it matches - # the current value will we then perform the deletion step. - result = model.team.update_sync_status(stale_team_sync) - if not result: - # Another worker updated this team. Nothing more to do. - logger.debug('Another worker synced team `%s` under organization %s via %s (#%s)', - stale_team_sync.team.name, - stale_team_sync.team.organization.username, sync_config, - stale_team_sync.team_id) - continue - - # Delete any team members not found in the backing auth system. - logger.debug('Deleting stale members for team `%s` under organization %s via %s (#%s)', - stale_team_sync.team.name, stale_team_sync.team.organization.username, - sync_config, stale_team_sync.team_id) - - deleted = model.team.delete_members_not_present(stale_team_sync.team, group_membership) - - # Done! - logger.info('Finishing sync for team `%s` under organization %s via %s (#%s): %s deleted', - stale_team_sync.team.name, stale_team_sync.team.organization.username, - sync_config, stale_team_sync.team_id, deleted) def main(): logging.config.fileConfig('conf/logging_debug.conf', disable_existing_loggers=False)