From aa109f1a15f2966b099201c978ba6c59916dabcc Mon Sep 17 00:00:00 2001 From: Alexanderamiri Date: Wed, 11 Nov 2026 18:25:48 +0100 Subject: [PATCH 1/2] fix(team-provisioner): skip manual groups in registry group sync javazone@java.no is the JavaZone coordination group's list. The registry gave `javazone` to ~36 heroes, so every groups/ merge added them to it (2026-10-23 re-added ~37 users that admin had removed on 2026-08-04). Only koordineringsgruppa may be in it, and its membership is kept by hand. A group with `manual: true` in the registry's groups.yaml is now left alone: - provision-groups.py sends it with no members - the Lambda skips it in sync_groups_and_heros and sync_groups - reconcile-groups.py plans no removals from it - sync-members.py maps the sheet's "JavaZone" choice to no group - sort-members.py drops the javazone section Co-Authored-By: Claude Opus 5.5 --- docs/lambda-functions.md | 9 ++ scripts/provision-groups.py | 14 ++- scripts/reconcile-groups.py | 3 +- scripts/sort-members.py | 4 +- scripts/sync-members.py | 12 +- scripts/tests/test_manual_groups.py | 108 ++++++++++++++++++ .../lambda-src/team_provisioner/handler.py | 11 +- .../test_team_provisioner_google_group.py | 31 +++++ 8 files changed, 178 insertions(+), 14 deletions(-) create mode 100644 scripts/tests/test_manual_groups.py diff --git a/docs/lambda-functions.md b/docs/lambda-functions.md index f0a4ad2..6ea23fc 100644 --- a/docs/lambda-functions.md +++ b/docs/lambda-functions.md @@ -86,6 +86,15 @@ Queries Security Hub for active findings at HIGH and CRITICAL severity, aggregat | `/javabin/platform/atlassian-org-id` | Atlassian organisation ID | | `/javabin/platform/atlassian-api-key` | Atlassian org admin API key (unscoped — full org admin) | +### Registry groups (`sync_groups_and_heros`) + +`scripts/provision-groups.py` resolves each group in the registry's `groups/groups.yaml` to the java.no +addresses of the members who list it, and the Lambda adds the missing ones (additive: members not in +the YAML are preserved). A group with `manual: true` is kept by hand in Google Admin: the script sends it +with no members and the Lambda skips it (no Google, Cognito or Identity Center sync), and +`scripts/reconcile-groups.py` plans no removals from it. `javazone@java.no`, the JavaZone coordination +group's list, is manual: only koordineringsgruppa may be in it. + ### Team permission set (`_ensure_team_permission_set`) `javabin-team-{team}` attaches the `javabin-team-{team}-guardrails` policy and an inline policy: `Action "*"` on resources tagged `team={team}` (or untagged), denies on platform-managed parameters, a few untagged reads, and `AllowExecOnPlatformCluster`: `ecs:ExecuteCommand` on the shared `javabin-platform` cluster. ECS Exec is authorized against both the cluster and the task, and the task is only allowed through its propagated `team` tag, so members can exec into their own team's tasks only (apps with `compute.exec: true`). Changes reach existing permission sets on the next registry sync, which reprovisions them. diff --git a/scripts/provision-groups.py b/scripts/provision-groups.py index c0e04ef..6f0df11 100644 --- a/scripts/provision-groups.py +++ b/scripts/provision-groups.py @@ -32,14 +32,19 @@ def load_yaml(path): def resolve_memberships(groups, members_data): - """For each group, determine which members belong to it.""" + """For each group, determine which members belong to it. + + Groups marked `manual: true` get no members: their membership is kept by + hand in Google Admin, and the provisioner leaves them alone. They stay in + the payload so access groups can still nest them. + """ members_list = members_data.get("members") or [] resolved_groups = [] for group in groups.get("groups", []): name = group["name"] - member_emails = [ + member_emails = [] if group.get("manual") else [ h["javabin_google_email"] for h in members_list if h.get("javabin_google_email") and name in (h.get("memberships") or []) @@ -187,7 +192,10 @@ def main(): total_members = sum(len(g["members"]) for g in resolved_groups) print(f"Resolved {len(resolved_groups)} groups with {total_members} total memberships") for g in resolved_groups: - print(f" {g['name']}: {len(g['members'])} member(s)") + if g.get("manual"): + print(f" {g['name']}: manual (not synced)") + else: + print(f" {g['name']}: {len(g['members'])} member(s)") if access_groups: print(f"\nAccess groups: {len(access_groups)}") diff --git a/scripts/reconcile-groups.py b/scripts/reconcile-groups.py index 0717277..b85c111 100644 --- a/scripts/reconcile-groups.py +++ b/scripts/reconcile-groups.py @@ -171,7 +171,8 @@ def generate_plan(token, members_data, groups_data, access_data): members_list = members_data.get("members") or [] for group in (groups_data.get("groups") or []): group_email = group.get("google", "") - if not group_email: + # manual: membership is kept by hand in Google Admin, never reconciled + if not group_email or group.get("manual"): continue desired_group_members = { diff --git a/scripts/sort-members.py b/scripts/sort-members.py index b5ec40a..dd219f3 100644 --- a/scripts/sort-members.py +++ b/scripts/sort-members.py @@ -9,7 +9,7 @@ sort-members.py groups/members.yaml groups/groups.yaml > sorted.yaml Priority order (highest first): - drift → styret → pkom → kodesmia → region → javazone → other + drift → styret → pkom → kodesmia → region → other """ import argparse @@ -18,7 +18,7 @@ import yaml -GROUP_PRIORITY = ["drift", "styret", "pkom", "kodesmia", "region", "javazone"] +GROUP_PRIORITY = ["drift", "styret", "pkom", "kodesmia", "region"] def primary_section(member): diff --git a/scripts/sync-members.py b/scripts/sync-members.py index 5692222..93ca71d 100644 --- a/scripts/sync-members.py +++ b/scripts/sync-members.py @@ -43,8 +43,11 @@ } # Maps Norwegian group names from the sheet to group IDs in groups.yaml. +# None = a known sheet choice that maps to no group. GROUP_MAP = { - "JavaZone": "javazone", + # javazone@java.no is the coordination group's list, kept by hand + # (`manual: true` in groups.yaml); JavaZone volunteers are not added to it. + "JavaZone": None, "Styret": "styret", "Kodesmia": "kodesmia", "Region": "region", @@ -197,11 +200,10 @@ def map_groups(groups_str): raw = [g.strip() for g in groups_str.split(",") if g.strip()] mapped = [] for g in raw: - group_id = GROUP_MAP.get(g) - if group_id: - mapped.append(group_id) - else: + if g not in GROUP_MAP: print(f" Warning: unknown group '{g}' — not in GROUP_MAP, skipping", file=sys.stderr) + elif GROUP_MAP[g]: + mapped.append(GROUP_MAP[g]) # Always include helter — all heroes are helter members mapped.append("helter") return sorted(set(mapped)) diff --git a/scripts/tests/test_manual_groups.py b/scripts/tests/test_manual_groups.py new file mode 100644 index 0000000..9964ded --- /dev/null +++ b/scripts/tests/test_manual_groups.py @@ -0,0 +1,108 @@ +"""Tests for `manual: true` groups in the registry sync scripts. + +A manual group (javazone@java.no) is kept by hand in Google Admin: no script may +add members to it or plan removals from it, and the hero sheet maps nothing into it. + +Run: python3 -m unittest discover -s scripts/tests +""" + +import contextlib +import importlib.util +import io +import os +import unittest +from unittest import mock + +HERE = os.path.dirname(os.path.abspath(__file__)) + + +def load(name, filename): + spec = importlib.util.spec_from_file_location(name, os.path.join(HERE, "..", filename)) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod + + +GROUPS = {"groups": [ + {"name": "helter", "google": "helter@java.no", "cognito": True}, + {"name": "javazone", "google": "javazone@java.no", "manual": True}, +]} + +# A stale members.yaml that still lists the manual group must not leak into it. +MEMBERS = {"members": [ + {"firstname": "Ola", "lastname": "Nordmann", "personal_email": "ola@example.com", + "javabin_google_email": "ola.nordmann@java.no", "memberships": ["helter", "javazone"]}, + {"firstname": "Kari", "lastname": "Nordmann", "personal_email": "kari@example.com", + "javabin_google_email": "kari.nordmann@java.no", "memberships": ["helter"]}, +]} + + +class ProvisionGroupsTest(unittest.TestCase): + def setUp(self): + try: + self.pg = load("provision_groups", "provision-groups.py") + except ModuleNotFoundError as e: # PyYAML is on the CI runner + self.skipTest(str(e)) + + def test_manual_group_resolves_to_no_members(self): + got = {g["name"]: g for g in self.pg.resolve_memberships(GROUPS, MEMBERS)} + self.assertEqual(got["helter"]["members"], ["ola.nordmann@java.no", "kari.nordmann@java.no"]) + self.assertEqual(got["javazone"]["members"], []) + self.assertIs(got["javazone"]["manual"], True) # the Lambda skips it too + + +class ReconcileGroupsTest(unittest.TestCase): + def setUp(self): + try: + self.rg = load("reconcile_groups", "reconcile-groups.py") + except ModuleNotFoundError as e: + self.skipTest(str(e)) + + def test_no_removals_planned_for_manual_group(self): + current = { + "helter@java.no": [{"email": "ola.nordmann@java.no", "type": "USER"}, + {"email": "kari.nordmann@java.no", "type": "USER"}], + "javazone@java.no": [{"email": "kari.nordmann@java.no", "type": "USER"}, + {"email": "jz-koordineringsgruppen@java.no", "type": "GROUP"}], + } + users = [{"primaryEmail": "ola.nordmann@java.no"}, {"primaryEmail": "kari.nordmann@java.no"}] + with mock.patch.object(self.rg, "list_workspace_users", return_value=users), \ + mock.patch.object(self.rg, "list_group_members", side_effect=lambda t, g: current[g]) as lgm, \ + contextlib.redirect_stdout(io.StringIO()): + plan = self.rg.generate_plan("token", MEMBERS, GROUPS, {}) + self.assertEqual(plan["actions"], []) + self.assertNotIn("javazone@java.no", [c.args[1] for c in lgm.call_args_list]) + + +class SyncMembersTest(unittest.TestCase): + def setUp(self): + self.sm = load("sync_members", "sync-members.py") + + def test_javazone_choice_maps_to_no_group_without_warning(self): + err = io.StringIO() + with contextlib.redirect_stderr(err): + got = self.sm.map_groups("JavaZone, Kodesmia") + self.assertEqual(got, ["helter", "kodesmia"]) + self.assertEqual(err.getvalue(), "") + + def test_unknown_choice_still_warns(self): + err = io.StringIO() + with contextlib.redirect_stderr(err): + self.assertEqual(self.sm.map_groups("Nope"), ["helter"]) + self.assertIn("unknown group 'Nope'", err.getvalue()) + + +class SortMembersTest(unittest.TestCase): + def setUp(self): + try: + self.sort = load("sort_members", "sort-members.py") + except ModuleNotFoundError as e: + self.skipTest(str(e)) + + def test_javazone_is_not_a_section(self): + self.assertNotIn("javazone", self.sort.GROUP_PRIORITY) + self.assertEqual(self.sort.primary_section({"memberships": ["helter"]}), "other") + + +if __name__ == "__main__": + unittest.main() diff --git a/terraform/lambda-src/team_provisioner/handler.py b/terraform/lambda-src/team_provisioner/handler.py index 34120bd..4e45b00 100644 --- a/terraform/lambda-src/team_provisioner/handler.py +++ b/terraform/lambda-src/team_provisioner/handler.py @@ -2657,9 +2657,14 @@ def handle_sync_groups_and_heros(event): "failed": aliases_failed, } - # Step 3 (+ 4, 5): Sync Google groups, Cognito, Identity Center per group (parallel) + # Step 3 (+ 4, 5): Sync Google groups, Cognito, Identity Center per group (parallel). + # Manual groups are kept by hand in Google Admin: never add members to them. + synced_groups = [g for g in groups if not g.get("manual")] + skipped = sorted(g.get("name") or "?" for g in groups if g.get("manual")) + if skipped: + logger.info("Skipping manual group(s): %s", ", ".join(skipped)) with ThreadPoolExecutor(max_workers=GROUP_SYNC_WORKERS) as executor: - futures = [executor.submit(_sync_one_group, group, access_token) for group in groups] + futures = [executor.submit(_sync_one_group, group, access_token) for group in synced_groups] for future in as_completed(futures): name, gr = future.result() if name: @@ -2838,7 +2843,7 @@ def handle_sync_groups(event): results = {} for group in groups: name = group.get("name") - if not name or not group.get("google"): + if not name or not group.get("google") or group.get("manual"): continue logger.info("Syncing group: %s (%s)", name, group["google"]) diff --git a/terraform/lambda-src/tests/test_team_provisioner_google_group.py b/terraform/lambda-src/tests/test_team_provisioner_google_group.py index e1588a4..223cf49 100644 --- a/terraform/lambda-src/tests/test_team_provisioner_google_group.py +++ b/terraform/lambda-src/tests/test_team_provisioner_google_group.py @@ -279,5 +279,36 @@ def test_changes_are_listed(self): self.assertIn("settings: isArchived", line) +class ManualGroupTest(unittest.TestCase): + """`manual: true` groups (javazone@java.no) are kept by hand: the sync adds nobody.""" + + def test_sync_groups_and_heros_skips_manual_group(self): + h = load() + directory = FakeDirectory({"jz-koordineringsgruppen@java.no": "MEMBER"}) + groups = [ + {"name": "helter", "google": "helter@java.no", "members": ["ola@java.no"]}, + # members a stale payload might still carry; none of them may be added + {"name": "javazone", "google": "javazone@java.no", "manual": True, "members": ["ola@java.no"]}, + ] + with mock.patch.object(h, "_get_google_access_token", return_value="tok"), \ + mock.patch.object(h, "_google_api", side_effect=directory), \ + mock.patch.object(h, "notify_slack"), \ + mock.patch.object(h, "_prune_memberships", return_value=[]), \ + mock.patch.object(h, "_explain_not_in_identity_center", return_value={}): + body = json.loads(h.handle_sync_groups_and_heros({"groups": groups, "heros": []})["body"]) + self.assertIn("helter", body["synced"]) + self.assertNotIn("javazone", body["synced"]) + self.assertFalse([c for c in directory.calls if "javazone%40java.no" in c[1]], directory.calls) + + def test_sync_groups_skips_manual_group(self): + h = load() + with mock.patch.object(h, "sync_group_to_cognito") as cognito, \ + mock.patch.object(h, "sync_group_to_identity_center") as ic, \ + mock.patch.object(h, "notify_slack"): + h.handle_sync_groups({"groups": [{"name": "javazone", "google": "javazone@java.no", "manual": True}]}) + cognito.assert_not_called() + ic.assert_not_called() + + if __name__ == "__main__": unittest.main() From fe597b3f5c4151f14b164a7258c6ab424d77667f Mon Sep 17 00:00:00 2001 From: Alexanderamiri Date: Wed, 11 Nov 2026 18:33:53 +0100 Subject: [PATCH 2/2] fix(scripts): leave manual groups out of removal warnings and plan diffs Co-Authored-By: Claude Opus 5.5 --- scripts/diff-groups.py | 8 +++++--- scripts/provision-groups.py | 5 ++++- scripts/tests/test_manual_groups.py | 22 +++++++++++++++++++++- 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/scripts/diff-groups.py b/scripts/diff-groups.py index a70dbba..6abb6ec 100644 --- a/scripts/diff-groups.py +++ b/scripts/diff-groups.py @@ -69,7 +69,7 @@ def diff_groups(base_data, pr_data): for name in sorted(base_groups.keys() & pr_groups.keys()): b, p = base_groups[name], pr_groups[name] diffs = [] - for field in ["google", "cognito", "identity_center", "permission_set"]: + for field in ["google", "cognito", "identity_center", "permission_set", "manual"]: bv, pv = b.get(field), p.get(field) if bv != pv: diffs.append(f"`{field}`: {bv!r} → {pv!r}") @@ -106,9 +106,11 @@ def diff_access(base_data, pr_data): def resolve_membership_changes(base_members_data, pr_members_data, base_groups_data, pr_groups_data): """Show per-group membership additions/removals.""" + # Manual groups are kept by hand in Google Admin: memberships never reach them. + manual = {g["name"] for g in (pr_groups_data.get("groups") or []) if g.get("manual")} all_group_names = sorted( - {g["name"] for g in (base_groups_data.get("groups") or [])} - | {g["name"] for g in (pr_groups_data.get("groups") or [])} + ({g["name"] for g in (base_groups_data.get("groups") or [])} + | {g["name"] for g in (pr_groups_data.get("groups") or [])}) - manual ) base_members = base_members_data.get("members") or [] diff --git a/scripts/provision-groups.py b/scripts/provision-groups.py index 6f0df11..0883b0b 100644 --- a/scripts/provision-groups.py +++ b/scripts/provision-groups.py @@ -111,7 +111,10 @@ def detect_removals(prev_members, curr_members, prev_groups, curr_groups): membership_removals = [] all_group_names = {g["name"] for g in (prev_groups.get("groups") or [])} | {g["name"] for g in (curr_groups.get("groups") or [])} - for gname in sorted(all_group_names): + # Manual groups are kept by hand in Google Admin: dropping one from + # memberships changes nothing there, and reconcile skips them. + manual = {g["name"] for g in (curr_groups.get("groups") or []) if g.get("manual")} + for gname in sorted(all_group_names - manual): prev_set = { m["javabin_google_email"] for m in (prev_members.get("members") or []) if m.get("javabin_google_email") and gname in (m.get("memberships") or []) diff --git a/scripts/tests/test_manual_groups.py b/scripts/tests/test_manual_groups.py index 9964ded..d5d86d1 100644 --- a/scripts/tests/test_manual_groups.py +++ b/scripts/tests/test_manual_groups.py @@ -50,6 +50,26 @@ def test_manual_group_resolves_to_no_members(self): self.assertEqual(got["javazone"]["members"], []) self.assertIs(got["javazone"]["manual"], True) # the Lambda skips it too + def test_no_removal_warning_for_manual_group(self): + now = {"members": [{**m, "memberships": ["helter"]} for m in MEMBERS["members"]]} + _, removals, _ = self.pg.detect_removals(MEMBERS, now, GROUPS, GROUPS) + self.assertEqual(removals, []) + + +class DiffGroupsTest(unittest.TestCase): + def setUp(self): + try: + self.dg = load("diff_groups", "diff-groups.py") + except ModuleNotFoundError as e: + self.skipTest(str(e)) + + def test_manual_flag_shown_and_manual_memberships_not_listed(self): + base = {"groups": [{k: v for k, v in g.items() if k != "manual"} for g in GROUPS["groups"]]} + _, _, changed = self.dg.diff_groups(base, GROUPS) + self.assertEqual(changed, [{"name": "javazone", "diffs": ["`manual`: None → True"]}]) + now = {"members": [{**m, "memberships": ["helter"]} for m in MEMBERS["members"]]} + self.assertEqual(self.dg.resolve_membership_changes(MEMBERS, now, base, GROUPS), []) + class ReconcileGroupsTest(unittest.TestCase): def setUp(self): @@ -101,7 +121,7 @@ def setUp(self): def test_javazone_is_not_a_section(self): self.assertNotIn("javazone", self.sort.GROUP_PRIORITY) - self.assertEqual(self.sort.primary_section({"memberships": ["helter"]}), "other") + self.assertEqual(self.sort.primary_section({"memberships": ["helter", "javazone"]}), "other") if __name__ == "__main__":