fix(team-provisioner): skip manual groups in registry group sync

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.

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 []

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 [])
@@ -106,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 [])
@@ -187,7 +195,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)}")

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 = {

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):

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))

scripts/tests/test_manual_groups.py

@@ -0,0 +1,128 @@
+"""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
+
+    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):
+        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", "javazone"]}), "other")
+
+
+if __name__ == "__main__":
+    unittest.main()

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"])

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()