FAQ | This is a LIVE service | Changelog

Commit 344f0884 authored by Dave Hart's avatar Dave Hart 🍕
Browse files

Merge branch 'create-groups' into 'master'

Create groups before adding members

Closes #1

See merge request !2
parents 051d1bff 1c59156e
Pipeline #192989 passed with stages
in 3 minutes and 9 seconds
......@@ -3,8 +3,8 @@
Tool to query Lookup for CHRIS and CamSIS institutional membership.
> This tool is currently only has the one operation `student-inst-members` that is able to compare
> CamSIS student affiliations to Lookup group membership and update as appropriate. It is not able
> to create Lookup groups.
> CamSIS student affiliations to Lookup group membership and update as appropriate. If the
> appropriate lookup group doesn't exist then it is created first.
>
> Additionally, an operation to do the same for CHRIS institution membership is yet to be
> implemented.
......
......@@ -6,7 +6,7 @@ Usage:
lookupsync student-inst-members --gateway-client-id=CLIENT_ID --lookup-username=USERNAME
( --gateway-client-secret=CLIENT_SECRET | --gateway-client-secret-from=PATH )
( --lookup-password=PASSWORD | --lookup-password-from=PATH )
[--quiet] [--debug] [--lookup-test] [--really-do-this]
[--quiet] [--debug] [--lookup-test | --lookup-local] [--really-do-this]
Options:
-h, --help Show a brief usage summary.
......@@ -29,6 +29,7 @@ Options:
to passing secrets on the command line.
--lookup-test Use Lookup test instance instead of production
--lookup-local Use local instance of Lookup (developers only)
--really-do-this Actually attempt to update lookup group memberships, otherwise
just output what would have been changed.
......@@ -47,7 +48,9 @@ import ibisclient
from .api_gateway import create_api_gateway_session
from .inst_mapping import fetch_inst_mapping
from .student_api import get_students_by_group
from .lookup import create_lookup_connection, compare_with_lookup_groups, update_lookup_groups
from .lookup import (
create_lookup_connection, compare_with_lookup_groups,
update_lookup_groups, create_lookup_groups, strip_groups_missing_insts)
STUDENT_API_ROOT = 'https://api.apps.cam.ac.uk/university-student/v1alpha2/'
INST_MAPPING_API_ROOT = 'https://api.apps.cam.ac.uk/institutions/mapping/v1/'
......@@ -124,10 +127,17 @@ def _student_inst_members(opts: dict, dry_run: bool):
ibis_group_methods, students_by_group)
if missing_groups:
# Shame we cannot automatically do this through API
LOG.info('Groups that need creating:')
for group in sorted(missing_groups):
LOG.info(f'- {group}')
# Create groups that couldn't be found
ibis_inst_methods = ibisclient.InstitutionMethods(ibis_conn)
missing_insts = create_lookup_groups(ibis_inst_methods, missing_groups, dry_run)
if missing_insts:
LOG.info('%s institution(s) not found', len(missing_insts))
# Strip group changes for institutions that couldn't be found
previous_count = len(group_changes)
group_changes = strip_groups_missing_insts(group_changes, missing_insts)
if previous_count != len(group_changes):
LOG.info('%s groups(s) ignored as no matching institution found',
previous_count - len(group_changes))
# Make changes to Lookup groups
update_lookup_groups(ibis_group_methods, group_changes, dry_run)
......@@ -9,6 +9,19 @@ LOG = logging.getLogger(os.path.basename(sys.argv[0]))
# Convenient type definition for group changes dict
GroupChanges = Dict[str, Dict[str, Set[str]]]
# Make group 'career' suffix to group title suffix
GROUP_TITLE_MAPPING = {
'ug': 'Undergraduates',
'pg': 'Postgraduates',
}
# Fixed group description of created groups
GROUP_DESCRIPTION = (
"Group synchronised with student affiliations in the University's Student Information System."
)
# Transaction comments for group creation and update
TRANSACTION_COMMENT = "SIS Synchronisation"
def create_lookup_connection(opts: dict) -> ibisclient.IbisClientConnection:
"""
......@@ -23,7 +36,10 @@ def create_lookup_connection(opts: dict) -> ibisclient.IbisClientConnection:
with open(password_from) as fobj:
password = fobj.read().strip()
if opts['--lookup-test']:
if opts['--lookup-local']:
LOG.info('Using local dev instance of Lookup')
ibis_conn = ibisclient.createLocalConnection()
elif opts['--lookup-test']:
LOG.info('Using test instance of Lookup')
ibis_conn = ibisclient.createTestConnection()
else:
......@@ -63,14 +79,19 @@ def compare_with_lookup_groups(
# Check that group exists
if ibis_group_methods.getGroup(group) is None:
missing_groups.add(group)
# Will want to add everyone after creating the group
group_changes[group] = {'add': students, 'remove': set()}
LOG.info('Group "%s" needs creating with %s students', group, len(students))
continue
LOG.info('Group "%s" should have %s student(s):', group, len(students))
# Get lookup direct membership
members: List[ibisclient.IbisPerson] = ibis_group_methods.getDirectMembers(
group, 'all_identifiers')
LOG.info('- Lookup has %s member(s)', len(members))
# Get set of USNs from membership
group_usns = {
id.value
for person in members if person.identifiers is not None
......@@ -81,6 +102,7 @@ def compare_with_lookup_groups(
' - mismatch with membership' if len(group_usns) != len(members) else ''
)
# Determine who to add and/or remove
to_add = students - group_usns
LOG.info('- %s need adding', len(to_add))
to_remove = group_usns - students
......@@ -89,13 +111,15 @@ def compare_with_lookup_groups(
if to_add | to_remove:
group_changes[group] = {'add': to_add, 'remove': to_remove}
LOG.info('%s group(s) need creating', len(missing_groups))
LOG.info('%s group(s) need changes', len(group_changes))
return (missing_groups, group_changes)
def update_lookup_groups(
ibis_group_methods: ibisclient.GroupMethods,
group_changes: GroupChanges, dry_run: bool = True):
group_changes: GroupChanges,
dry_run: bool = True):
"""
Log and update (if not a dry-run) group memberships
......@@ -113,5 +137,72 @@ def update_lookup_groups(
group,
[f'usn/{usn}' for usn in changes['add']],
[f'usn/{usn}' for usn in changes['remove']],
'SIS Synchronisation',
TRANSACTION_COMMENT,
)
def create_lookup_groups(
ibis_inst_methods: ibisclient.InstitutionMethods,
groups_to_create: Set[str],
dry_run: bool = True) -> Set[str]:
"""
Log and create (if not a dry-run) lookup groups with names, titles, descriptions
and the current lookup account as the only manager.
Return a set of institution ids for institutions that couldn't be found.
"""
# Get current authenticated lookup account
managed_by = ibis_inst_methods.conn.username
# Cache institution details so we can name, title and describe groups appropriately
institutions: Dict[str, ibisclient.IbisInstitution] = {}
# Compile a set of institutions we couldn't find
missing_insts = set()
for group in sorted(groups_to_create):
# split group name into instid and career (ug or pg)
(instid, _, career) = group.split('-')
instid = instid.upper()
if instid not in institutions:
# Make sure institution exists
inst = ibis_inst_methods.getInst(instid)
if inst is None:
missing_insts.add(instid)
LOG.warning('Institution "%s" not found for group "%s"', instid, group)
continue
institutions[instid] = inst
inst_name = institutions[instid].name
group_title = f'{inst_name} - SIS - {GROUP_TITLE_MAPPING[career]}'
LOG.info('Creating group "%s"', group)
LOG.info('- in institution "%s"', instid)
LOG.info('- with title "%s"', group_title)
if dry_run:
LOG.info('- skipping creation in dry-run mode')
else:
ibis_inst_methods.createGroup(
instid, group, group_title,
GROUP_DESCRIPTION, managed_by, TRANSACTION_COMMENT)
return missing_insts
def strip_groups_missing_insts(
group_changes: GroupChanges, missing_insts: Set[str]) -> GroupChanges:
"""
Remove group changes that would belong to institutions that couldn't be found.
>>> strip_groups_missing_insts(
... {
... 'abc-sis-pg': {'add': {'123'}, 'remove': {'456'}},
... 'def-sis-ug': {'add': {'234'}, 'remove': {'567'}},
... },
... {'DEF', 'GHI'}
... )
{'abc-sis-pg': {'add': {'123'}, 'remove': {'456'}}}
"""
return {
group: changes for group, changes in group_changes.items()
if group.split('-')[0].upper() not in missing_insts
}
......@@ -6,7 +6,9 @@ import ibisclient
from . import temp_password_file
from lookupsync.lookup import (
create_lookup_connection, compare_with_lookup_groups, update_lookup_groups)
create_lookup_connection, compare_with_lookup_groups,
create_lookup_groups, update_lookup_groups,
GROUP_DESCRIPTION, TRANSACTION_COMMENT)
@patch('ibisclient.createConnection')
......@@ -20,6 +22,7 @@ def test_create_lookup_connection_var(ibis_connect_mock: MagicMock):
'--lookup-username': 'TEST_USER',
'--lookup-password': 'TEST_PASSWORD',
'--lookup-test': False,
'--lookup-local': False,
})
# createConnection called once
ibis_connect_mock.assert_called_once()
......@@ -42,6 +45,7 @@ def test_create_lookup_connection_file(ibis_connect_mock: MagicMock):
'--lookup-username': 'TEST_USER',
'--lookup-password-from': password_file,
'--lookup-test': False,
'--lookup-local': False,
})
# createConnection called once
ibis_connect_mock.assert_called_once()
......@@ -65,6 +69,7 @@ def test_create_lookup_test_connection(
'--lookup-username': 'TEST_USER',
'--lookup-password': 'TEST_PASSWORD',
'--lookup-test': True,
'--lookup-local': False,
})
# createConnection is not called
ibis_connect_mock.assert_not_called()
......@@ -160,17 +165,18 @@ def test_compare_with_lookup_groups_missing_group(
"""
# Add a group in mock_student_by_groups that is not in mock_lookup_group_members
NEW_GROUP = 'baz-sis-pg'
USN_TO_ADD = '999999999'
assert mock_lookup_group_members.get(NEW_GROUP) is None
mock_students_by_group[NEW_GROUP] = {'999999999'}
mock_students_by_group[NEW_GROUP] = {USN_TO_ADD}
# Perform comparison
mock_group_methods = MockGroupMethods(mock_lookup_group_members)
missing_groups, group_changes = compare_with_lookup_groups(
mock_group_methods, mock_students_by_group)
# Just new group reported missing
# Just new group reported missing and all members appear in changes to be added
assert missing_groups == {NEW_GROUP}
assert group_changes == dict()
assert group_changes == {NEW_GROUP: {'add': {USN_TO_ADD}, 'remove': set()}}
def test_compare_with_lookup_groups_changes(
......@@ -256,3 +262,115 @@ def test_update_lookup_groups():
# however have matching contents
assert expected_to_add == set(mock_group_methods.updates[group]['to_add'])
assert expected_to_remove == set(mock_group_methods.updates[group]['to_remove'])
# Mock ibisclient.InstitutionMethods
class MockInstitutionMethods:
def __init__(self, insts: List[Dict[str, ibisclient.IbisInstitution]] = list()) -> None:
self.insts = insts
self.group_creations = {}
self.conn = MagicMock()
self.conn.username = 'TEST_USER'
def getInst(self, inst):
return self.insts.get(inst)
def createGroup(self, instid, name, title, description,
managedBy=None, commitComment=None):
# Should only be called with known institution
assert instid in self.insts
# Record all parts of the creation
self.group_creations.setdefault(instid, []).append({
'name': name,
'title': title,
'description': description,
'managedBy': managedBy,
'commitComment': commitComment,
})
@pytest.fixture
def mock_instids_to_names():
"""
An example set of Lookup institution instids
"""
return {
'FOO': 'Department of Foo',
'BAR': 'Faculty of Bar',
'MISC': 'School of Miscellaneous'
}
@pytest.fixture
def mock_lookup_institutions(mock_instids_to_names):
"""
An example dict of Lookup instids to ibisclient.IbisInstitution records matching those
in mock_instids fixture
"""
def _make_ibis_inst(instid: str, name: str):
inst = ibisclient.IbisInstitution({'instid': instid})
setattr(inst, 'name', name)
return inst
return {
instid: _make_ibis_inst(instid, name)
for instid, name in mock_instids_to_names.items()
}
def test_create_lookup_groups(mock_lookup_institutions, mock_instids_to_names):
"""
Groups in known institution get created. Those in unknown institutions don't
get created and the institution is included in the returned set.
"""
# New groups (2 for 1 known inst, 1 for another known inst, 1 for unknown inst)
NEW_GROUPS = {'foo-sis-ug', 'foo-sis-pg', 'bar-sis-ug', 'bad-sis-pg'}
EXPECTED_MISSING_INSTS = {'BAD'}
EXPECTED_CREATED_GROUPS_BY_INST = {
'FOO': {'foo-sis-ug', 'foo-sis-pg'},
'BAR': {'bar-sis-ug'},
}
mock_inst_methods = MockInstitutionMethods(mock_lookup_institutions)
# Make create calls (dry-run)
create_lookup_groups(mock_inst_methods, NEW_GROUPS, True)
# No createGroup calls made
assert mock_inst_methods.group_creations == {}
# Make create calls (not dry-run)
missing_insts = create_lookup_groups(mock_inst_methods, NEW_GROUPS, False)
# Set of missing institutions returned
assert EXPECTED_MISSING_INSTS == missing_insts
# Only groups with a known institution are created
assert (
set(EXPECTED_CREATED_GROUPS_BY_INST.keys()) ==
set(mock_inst_methods.group_creations.keys())
)
for instid, groups in mock_inst_methods.group_creations.items():
# Expected groups created for each institution
assert (
set(EXPECTED_CREATED_GROUPS_BY_INST[instid]) ==
{group['name'] for group in groups}
)
inst_name = mock_instids_to_names[instid]
for group in groups:
# Each group would be created with:
# ... a title containing institution name and (Under|Post)graduates
assert inst_name in group['title']
if group['name'][-2:] == 'ug':
assert 'Undergraduates' in group['title']
else:
assert 'Postgraduates' in group['title']
# ... a fixed description and commit message
assert group['description'] == GROUP_DESCRIPTION
assert group['commitComment'] == TRANSACTION_COMMENT
# ... and be managed by the connection's user
assert group['managedBy'] == 'TEST_USER'
......@@ -3,7 +3,4 @@ requests-oauthlib>=1.3.1,<2.0
requests>=2.21.1,<3.0
pydantic>=1.9.0,<2.0
ucam-identitylib~=1.0.4
# Until we are able to use group authentication through API Gateway, we need to
# use Lookup API directly
ibisclient~=1.3.0
ibisclient~=1.3.2
......@@ -17,7 +17,7 @@ def load_requirements():
setup(
name='lookupsync',
version='0.2.0',
version='1.0.0',
packages=find_packages(),
install_requires=load_requirements(),
entry_points={
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment