Move sync team into its own module and add tests

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)

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

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

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)