Update GID and group handling for Gafaelfawr changes

Skip groups for both templating and supplemental groups for the
spawned pod if the group doesn't have a GID, since GIDs will become
optional for groups.  Pass the primary GID into the template if
available, rather than only using it in the pod spawner.

src/nublado2/hooks.py

@@ -35,8 +35,13 @@ async def pre_spawn(self, spawner: Spawner) -> None:
             spawner.gid = auth_state["gid"]
         else:
             spawner.gid = spawner.uid
+
+        # Not all groups may have GIDs.  Only those that do contribute to the
+        # supplemental GIDs.
         spawner.supplemental_gids = [
-            g["id"] for g in auth_state["groups"] if g["id"] != spawner.gid
+            g["id"]
+            for g in auth_state["groups"]
+            if "id" in g and g["id"] != spawner.gid
         ]
 
         # Since we will create a serviceaccount in the user resources,

src/nublado2/resourcemgr.py

@@ -194,15 +194,21 @@ async def _build_template_values(
         auth_state = await spawner.user.get_auth_state()
         groups = auth_state["groups"]
 
-        # Build a comma separated list of group:gid
-        # ex: group1:1000,group2:1001,group3:1002
-        external_groups = ",".join([f'{g["name"]}:{g["id"]}' for g in groups])
+        # Build a comma-separated list of groups used to populate the
+        # /etc/groups file.  Only groups with GIDs can be added, so skip
+        # groups without GIDs.
+        #
+        # Example: group1:1000,group2:1001,group3:1002
+        external_groups = ",".join(
+            [f'{g["name"]}:{g["id"]}' for g in groups if "id" in g]
+        )
 
         # Define the template variables.
         template_values = {
             "user_namespace": spawner.namespace,
             "user": spawner.user.name,
             "uid": auth_state["uid"],
+            "gid": auth_state.get("gid"),
             "token": auth_state["token"],
             "groups": groups,
             "external_groups": external_groups,

tests/resourcemgr_test.py

@@ -34,8 +34,9 @@
     namespace: "{{ user_namespace }}"
   data:
     group: |
-      {{user}}:x:{{uid}}:{% for group in groups %}
-      {{ group.name }}:x:{{ group.id }}:{{ user }}{% endfor %}
+      {{user}}:x:{{gid if gid else uid}}:{% for group in groups %}\
+{% if "id" in group %}
+      {{ group.name }}:x:{{ group.id }}:{{ user }}{% endif %}{% endfor %}
 - apiVersion: v1
   kind: ConfigMap
   metadata:
@@ -104,6 +105,7 @@ def config_mock() -> Iterator[None]:
             "FIREFLY_ROUTE": "/portal/app",
             "HUB_ROUTE": "{{ nublado_base_url }}",
             "EXTERNAL_GROUPS": "{{ external_groups }}",
+            "EXTERNAL_GID": "{{ gid }}",
             "EXTERNAL_UID": "{{ uid }}",
             "ACCESS_TOKEN": "{{ token }}",
             "IMAGE_DIGEST": "{{ options.image_info.digest }}",
@@ -151,7 +153,12 @@ async def test_create_kubernetes_resources(
     auth_state = {
         "token": "user-token",
         "uid": 1234,
-        "groups": [{"name": "foo", "id": 1235}, {"name": "bar", "id": 4567}],
+        "gid": 1551,
+        "groups": [
+            {"name": "foo", "id": 1235},
+            {"name": "bar", "id": 4567},
+            {"name": "baz"},
+        ],
     }
     pod_manifest = V1Pod(
         api_version="v1",
@@ -236,7 +243,7 @@ async def test_create_kubernetes_resources(
             },
             "data": {
                 "group": (
-                    "someuser:x:1234:\n"
+                    "someuser:x:1551:\n"
                     "foo:x:1235:someuser\n"
                     "bar:x:4567:someuser\n"
                 )
@@ -255,6 +262,7 @@ async def test_create_kubernetes_resources(
                 "EXTERNAL_INSTANCE_URL": "https://data.example.com/",
                 "FIREFLY_ROUTE": "/portal/app",
                 "HUB_ROUTE": "/nb/hub/",
+                "EXTERNAL_GID": "1551",
                 "EXTERNAL_GROUPS": "foo:1235,bar:4567",
                 "EXTERNAL_UID": "1234",
                 "ACCESS_TOKEN": "user-token",