fix(hub): tolerate missing GitHub teams during sync

Author: MioYuuIH

runtime/hub/core/groups.py

@@ -48,6 +48,8 @@
 _GITHUB_TEAM_SYNC_LOCKS: dict[str, asyncio.Lock] = {}
 _GITHUB_TEAM_MEMBERS_CACHE: dict[tuple[str, str, str, tuple[str, ...]], tuple[float, dict[str, list[str]]]] = {}
 _GITHUB_TEAM_MEMBERS_LOCK = asyncio.Lock()
+_GITHUB_APP_INSTALLATION_ID_CACHE: dict[tuple[str, str], str] = {}
+_GITHUB_APP_INSTALLATION_ID_LOCK = asyncio.Lock()
 _GITHUB_APP_INSTALLATION_TOKEN: dict[tuple[str, str], tuple[str, float]] = {}
 _GITHUB_APP_INSTALLATION_TOKEN_LOCK = asyncio.Lock()
 
@@ -72,16 +74,32 @@ def _github_app_private_key(private_key: str = "", private_key_file: str = "") -
     return private_key.replace("\\n", "\n").strip()
 
 
+def _github_team_api_slug(team_key: str) -> str:
+    """Convert a configured group/team key to the GitHub team slug API form."""
+    return team_key.strip().lower().replace(" ", "-")
+
+
 async def get_github_app_installation_token(
     app_id: str,
-    installation_id: str,
+    installation_id: str = "",
+    org_name: str = "",
     *,
     private_key: str = "",
     private_key_file: str = "",
 ) -> str | None:
     """Create or reuse a GitHub App installation access token for group sync."""
+    resolved_installation_id = await _resolve_github_app_installation_id(
+        app_id,
+        installation_id,
+        org_name,
+        private_key=private_key,
+        private_key_file=private_key_file,
+    )
+    if not resolved_installation_id:
+        return None
+
     now = time.time()
-    cache_key = (app_id, installation_id)
+    cache_key = (app_id, resolved_installation_id)
     cached_token = _GITHUB_APP_INSTALLATION_TOKEN.get(cache_key)
     if cached_token and now < cached_token[1] - 300:
         return cached_token[0]
@@ -93,7 +111,7 @@ async def get_github_app_installation_token(
             return cached_token[0]
 
         private_key = _github_app_private_key(private_key=private_key, private_key_file=private_key_file)
-        if not app_id or not installation_id or not private_key:
+        if not app_id or not resolved_installation_id or not private_key:
             log.warning(
                 "GitHub App installation token is unavailable because app id, installation id, or private key is missing"
             )
@@ -114,7 +132,7 @@ async def get_github_app_installation_token(
             async with (
                 aiohttp.ClientSession() as session,
                 session.post(
-                    f"https://api.github.com/app/installations/{installation_id}/access_tokens",
+                    f"https://api.github.com/app/installations/{resolved_installation_id}/access_tokens",
                     headers=headers,
                 ) as resp,
             ):
@@ -143,6 +161,86 @@ async def get_github_app_installation_token(
         return token
 
 
+async def _resolve_github_app_installation_id(
+    app_id: str,
+    installation_id: str,
+    org_name: str,
+    *,
+    private_key: str = "",
+    private_key_file: str = "",
+) -> str | None:
+    installation_id = installation_id.strip()
+    if installation_id:
+        return installation_id
+
+    org_name = org_name.strip()
+    if not app_id or not org_name:
+        log.warning("GitHub App installation ID lookup is unavailable because app id or org name is missing")
+        return None
+
+    cache_key = (app_id, org_name)
+    cached_installation_id = _GITHUB_APP_INSTALLATION_ID_CACHE.get(cache_key)
+    if cached_installation_id:
+        return cached_installation_id
+
+    async with _GITHUB_APP_INSTALLATION_ID_LOCK:
+        cached_installation_id = _GITHUB_APP_INSTALLATION_ID_CACHE.get(cache_key)
+        if cached_installation_id:
+            return cached_installation_id
+
+        private_key = _github_app_private_key(private_key=private_key, private_key_file=private_key_file)
+        if not private_key:
+            log.warning("GitHub App installation ID lookup is unavailable because the private key is missing")
+            return None
+
+        now = time.time()
+        app_jwt = jwt.encode(
+            {"iat": int(now) - 60, "exp": int(now) + 540, "iss": app_id},
+            private_key,
+            algorithm="RS256",
+        )
+        headers = {
+            "Authorization": f"Bearer {app_jwt}",
+            "Accept": "application/vnd.github+json",
+            "X-GitHub-Api-Version": "2022-11-28",
+        }
+
+        try:
+            async with (
+                aiohttp.ClientSession() as session,
+                session.get(
+                    f"https://api.github.com/orgs/{org_name}/installation",
+                    headers=headers,
+                ) as resp,
+            ):
+                if resp.status == 404:
+                    log.warning("GitHub App installation lookup returned 404 for org %s", org_name)
+                    return None
+                if resp.status != 200:
+                    log.warning(
+                        "GitHub App installation lookup returned status %d for org %s",
+                        resp.status,
+                        org_name,
+                    )
+                    return None
+                data = await resp.json()
+        except Exception as e:
+            log.warning("Error resolving GitHub App installation for org %s: %s", org_name, e)
+            return None
+
+        resolved_installation_id = data.get("id")
+        if resolved_installation_id is None:
+            log.warning(
+                "GitHub App installation lookup for org %s did not include an installation id",
+                org_name,
+            )
+            return None
+
+        resolved_installation_id = str(resolved_installation_id)
+        _GITHUB_APP_INSTALLATION_ID_CACHE[cache_key] = resolved_installation_id
+        return resolved_installation_id
+
+
 async def fetch_github_team_members(access_token: str, org_name: str, team_slug: str) -> set[str] | None:
     """Fetch all GitHub usernames in one team using the configured platform token."""
     if not access_token or not org_name or not team_slug:
@@ -160,9 +258,19 @@ async def fetch_github_team_members(access_token: str, org_name: str, team_slug:
             while True:
                 url = f"https://api.github.com/orgs/{org_name}/teams/{team_slug}/members?per_page=100&page={page}"
                 async with session.get(url, headers=headers) as resp:
+                    if resp.status == 404:
+                        log.warning(
+                            "GitHub API returned status 404 when fetching members for team %s in org %s",
+                            team_slug,
+                            org_name,
+                        )
+                        return set()
                     if resp.status != 200:
                         log.warning(
-                            "GitHub API returned status %d when fetching members for team %s", resp.status, team_slug
+                            "GitHub API returned status %d when fetching members for team %s in org %s",
+                            resp.status,
+                            team_slug,
+                            org_name,
                         )
                         return None
                     data = await resp.json()
@@ -214,19 +322,20 @@ async def fetch_github_team_members_table(
         installation_token = await get_github_app_installation_token(
             app_id,
             installation_id,
+            org_name=org_name,
             private_key=private_key,
             private_key_file=private_key_file,
         )
         if not installation_token:
             return None
 
         teams_by_login: dict[str, list[str]] = {}
-        for team_slug in github_team_keys:
-            members = await fetch_github_team_members(installation_token, org_name, team_slug)
+        for team_key in github_team_keys:
+            members = await fetch_github_team_members(installation_token, org_name, _github_team_api_slug(team_key))
             if members is None:
                 return None
             for login in members:
-                teams_by_login.setdefault(login, []).append(team_slug)
+                teams_by_login.setdefault(login, []).append(team_key)
 
         _GITHUB_TEAM_MEMBERS_CACHE[cache_key] = (now, teams_by_login)
         return {login: list(teams) for login, teams in teams_by_login.items()}

runtime/hub/tests/test_groups.py

@@ -17,6 +17,7 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
 
+import asyncio
 import importlib.util
 import sys
 import types
@@ -62,6 +63,9 @@ def load_module(name: str, path: Path):
 
 groups = load_module("core.groups", CORE / "groups.py")
 resolve_resources_for_user = groups.resolve_resources_for_user
+fetch_github_team_members = groups.fetch_github_team_members
+get_github_app_installation_token = groups.get_github_app_installation_token
+fetch_github_team_members_table = groups.fetch_github_team_members_table
 sync_user_github_teams = groups.sync_user_github_teams
 
 
@@ -101,6 +105,61 @@ def commit(self):
         pass
 
 
+class DummyResponse:
+    def __init__(self, status, payload):
+        self.status = status
+        self._payload = payload
+
+    async def __aenter__(self):
+        return self
+
+    async def __aexit__(self, exc_type, exc, tb):
+        return False
+
+    async def json(self):
+        return self._payload
+
+
+class DummyClientSession:
+    created = 0
+    get_calls = 0
+    post_calls = 0
+
+    def __init__(self):
+        type(self).created += 1
+
+    async def __aenter__(self):
+        return self
+
+    async def __aexit__(self, exc_type, exc, tb):
+        return False
+
+    def get(self, url, headers=None):
+        type(self).get_calls += 1
+        if url.endswith("/orgs/test-org/installation"):
+            return DummyResponse(200, {"id": 12345})
+        if url.endswith("/orgs/test-org/teams/missing-team/members?per_page=100&page=1"):
+            return DummyResponse(404, {})
+        if url.endswith("/orgs/test-org/teams/aup/members?per_page=100&page=1"):
+            return DummyResponse(200, [{"login": "OctoUser"}])
+        return DummyResponse(200, {"repositories": []})
+
+    def post(self, url, headers=None):
+        type(self).post_calls += 1
+        if url.endswith("/app/installations/12345/access_tokens"):
+            return DummyResponse(201, {"token": "cached-token", "expires_at": "2099-01-01T00:00:00Z"})
+        return DummyResponse(500, {})
+
+
+def _reset_dummy_client_session():
+    DummyClientSession.created = 0
+    DummyClientSession.get_calls = 0
+    DummyClientSession.post_calls = 0
+    groups._GITHUB_APP_INSTALLATION_ID_CACHE.clear()
+    groups._GITHUB_APP_INSTALLATION_TOKEN.clear()
+    groups._GITHUB_TEAM_MEMBERS_CACHE.clear()
+
+
 def test_sync_user_github_teams_skips_removals_when_team_fetch_failed():
     existing_group = DummyGroup("team-a")
     user = DummyUser([existing_group])
@@ -110,6 +169,67 @@ def test_sync_user_github_teams_skips_removals_when_team_fetch_failed():
     assert user.orm_user.groups == [existing_group]
 
 
+def test_fetch_github_team_members_treats_missing_team_as_empty_set(monkeypatch, caplog):
+    _reset_dummy_client_session()
+    monkeypatch.setattr(groups.aiohttp, "ClientSession", DummyClientSession)
+
+    caplog.set_level("WARNING", logger="jupyterhub.groups")
+    members = asyncio.run(fetch_github_team_members("token", "test-org", "missing-team"))
+
+    assert members == set()
+    assert "team missing-team" in caplog.text
+    assert "test-org" in caplog.text
+
+
+def test_get_github_app_installation_token_discovers_installation_id(monkeypatch):
+    _reset_dummy_client_session()
+    monkeypatch.setattr(groups.aiohttp, "ClientSession", DummyClientSession)
+    monkeypatch.setattr(groups.jwt, "encode", lambda payload, private_key, algorithm: "jwt-token")
+
+    token = asyncio.run(
+        get_github_app_installation_token(
+            "app-123",
+            "",
+            org_name="test-org",
+            private_key="dummy-private-key",
+        )
+    )
+    cached_token = asyncio.run(
+        get_github_app_installation_token(
+            "app-123",
+            "",
+            org_name="test-org",
+            private_key="dummy-private-key",
+        )
+    )
+
+    assert token == "cached-token"
+    assert cached_token == "cached-token"
+    assert DummyClientSession.created == 2
+    assert DummyClientSession.get_calls == 1
+    assert DummyClientSession.post_calls == 1
+
+
+def test_fetch_github_team_members_table_uses_api_slug_and_preserves_group_key(monkeypatch):
+    _reset_dummy_client_session()
+    monkeypatch.setattr(groups.aiohttp, "ClientSession", DummyClientSession)
+    monkeypatch.setattr(groups.jwt, "encode", lambda payload, private_key, algorithm: "jwt-token")
+
+    teams_by_login = asyncio.run(
+        fetch_github_team_members_table(
+            "app-123",
+            "",
+            "dummy-private-key",
+            "",
+            "test-org",
+            {"AUP"},
+            force=True,
+        )
+    )
+
+    assert teams_by_login == {"octouser": ["AUP"]}
+
+
 def test_resolve_resources_for_user_uses_group_mapping():
     user = DummyUser([DummyGroup("team-a"), DummyGroup("team-b")])