From faf61e645bea75078479019704d424fbd3b4087d Mon Sep 17 00:00:00 2001 From: adam Date: Thu, 18 Mar 2021 16:23:59 -0700 Subject: [PATCH 1/3] Address PR comments --- src/nublado2/resourcemgr.py | 67 +++++++++---------- .../static/moneypenny-jwt-template.json | 16 ++--- 2 files changed, 41 insertions(+), 42 deletions(-) diff --git a/src/nublado2/resourcemgr.py b/src/nublado2/resourcemgr.py index 7d077cd..9c354bf 100644 --- a/src/nublado2/resourcemgr.py +++ b/src/nublado2/resourcemgr.py @@ -1,15 +1,12 @@ -import asyncio import datetime import json -import math import os -from string import Template as strTemplate -from typing import Any, Dict, Optional import aiohttp import jwt from jinja2 import Template from jupyterhub.spawner import Spawner +from jupyterhub.utils import exponential_backoff from kubernetes import client, config from kubernetes.utils import create_from_dict from ruamel import yaml @@ -43,6 +40,7 @@ async def create_user_resources(self, spawner: Spawner) -> None: auth_state = await spawner.user.get_auth_state() self.log.debug(f"Auth state={auth_state}") + nc = NubladoConfig().get() groups = auth_state["groups"] # Build a comma separated list of group:gid @@ -58,12 +56,13 @@ async def create_user_resources(self, spawner: Spawner) -> None: "token": auth_state["token"], "groups": groups, "external_groups": external_groups, - "base_url": NubladoConfig().get().get("base_url"), + "no_sudo": nc.get("no_sudo"), + "base_url": nc.get("base_url"), "dask_yaml": await self._build_dask_template(spawner), } self.log.debug(f"Template values={template_values}") - resources = NubladoConfig().get().get("user_resources", []) + resources = nc.get("user_resources", []) for r in resources: t_yaml = yaml.dump(r, Dumper=RoundTripDumper) self.log.debug(f"Resource template:\n{t_yaml}") @@ -80,65 +79,64 @@ async def create_user_resources(self, spawner: Spawner) -> None: async def _request_homedir_provisioning(self, spawner: Spawner) -> None: """Submit a request for provisioning via Moneypenny.""" - nc = NubladoConfig() + nc = NubladoConfig().get() hc = self.http_client - base_url: str = nc.get().get("base_url") or "http://localhost:8080" + base_url = nc.get("base_url") + signing_key_path = nc.get("signing_key_path") + assert isinstance(base_url, str) and isinstance(signing_key_path, str) uname = spawner.user.name auth_state = await spawner.user.get_auth_state() - dossier = await self._make_dossier(uname, auth_state) - token = await self._mint_admin_token(base_url, None) - mp_ep = f"{base_url}/moneypenny" - endpt = f"{mp_ep}/commission" + dossier = { + "username": uname, + "uid": int(auth_state["uid"]), + "groups": auth_state["groups"], + } + token = await self._mint_admin_token(base_url, signing_key_path) + endpt = f"{base_url}/moneypenny/commission" auth = {"Authorization": f"Bearer {token}"} self.log.debug(f"Posting dossier {dossier} to {endpt}") resp = await hc.post(endpt, json=dossier, headers=auth) self.log.debug(f"POST got {resp.status}") resp.raise_for_status() - expiry = datetime.datetime.now() + datetime.timedelta(seconds=300) + route = f"{base_url}/moneypenny/{uname}" count = 0 - route = f"{mp_ep}/{uname}" - while datetime.datetime.now() < expiry: + + async def _check_moneypenny_completion() -> bool: + nonlocal count count += 1 self.log.debug(f"Checking Moneypenny status at {route}: #{count}") resp = await hc.get(f"{route}", headers=auth) status = resp.status self.log.debug(f"Moneypenny status: {status}") if status == 200 or 404: - return + return True if status != 202: raise RuntimeError( f"Unexpected status from Moneypenny: {status}" ) - await asyncio.sleep(int(math.log(count))) - raise RuntimeError("Moneypenny timed out") + # Moneypenny is still working. + return False - async def _make_dossier( - self, name: str, auth_state: Dict[str, Any] - ) -> Dict[str, Any]: - dossier = { - "username": name, - "uid": int(auth_state["uid"]), - "groups": auth_state["groups"], - } - self.log.debug(f"Dossier: {dossier}") - return dossier + await exponential_backoff( + _check_moneypenny_completion, + fail_message="Moneypenny did not complete.", + timeout=300, + ) async def _mint_admin_token( - self, base_url: str, signing_key_path: Optional[str] + self, base_url: str, signing_key_path: str ) -> str: """Allowing specification of the signing key makes testing easier.""" template_file = os.path.join( os.path.dirname(__file__), "static/moneypenny-jwt-template.json" ) - with open(template_file, "r") as f: - token_template = strTemplate(f.read()) - if not signing_key_path: - signing_key_path = "/etc/keys/signing_key.pem" with open(signing_key_path, "r") as f: signing_key = f.read() current_time = int( datetime.datetime.now(tz=datetime.timezone.utc).timestamp() ) + with open(template_file, "r") as f: + token_template = Template(f.read()) token_data = { "environment_url": base_url, "username": "moneypenny", @@ -146,7 +144,8 @@ async def _mint_admin_token( "issue_time": current_time, "expiration_time": current_time + 300, } - token_dict = json.loads(token_template.substitute(token_data)) + rendered_token = token_template.render(token_data) + token_dict = json.loads(rendered_token) token = jwt.encode( token_dict, key=signing_key, diff --git a/src/nublado2/static/moneypenny-jwt-template.json b/src/nublado2/static/moneypenny-jwt-template.json index 1bfe293..5564106 100644 --- a/src/nublado2/static/moneypenny-jwt-template.json +++ b/src/nublado2/static/moneypenny-jwt-template.json @@ -1,12 +1,12 @@ { - "iss": "${environment_url}", - "aud": "${environment_url}", - "auth_time": "${issue_time}", - "exp": ${expiration_time}, - "iat": ${issue_time}, - "name": "${username}", - "uid": "${username}", - "uidNumber": "${uidnumber}", + "iss": "{{ environment_url }}", + "aud": "{{ environment_url }}", + "auth_time": "{{ issue_time }}", + "exp": {{ expiration_time }}, + "iat": {{ issue_time }}, + "name": "{{ username }}", + "uid": "{{ username }}", + "uidNumber": "{{ uidnumber }}", "isMemberOf": [ { "name": "wheel", From 4886721ec975e06260f273bdb8aa7bcfb8427afc Mon Sep 17 00:00:00 2001 From: adam Date: Tue, 23 Mar 2021 09:57:10 -0700 Subject: [PATCH 2/3] Add supplemental groups --- src/nublado2/hooks.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/nublado2/hooks.py b/src/nublado2/hooks.py index 46ff9a4..7e706a2 100644 --- a/src/nublado2/hooks.py +++ b/src/nublado2/hooks.py @@ -54,6 +54,7 @@ async def pre_spawn(self, spawner: Spawner) -> None: else: spawner.uid = auth_state["uid"] spawner.gid = auth_state["uid"] + spawner.supplemental_gids = [g["id"] for g in auth_state["groups"]] # Since we will create a serviceaccount in the user resources, # make the pod use that. This will also automount the token, From 7ccc9e0649d2358204e692dcf5114c50613112ab Mon Sep 17 00:00:00 2001 From: adam Date: Tue, 23 Mar 2021 15:05:19 -0700 Subject: [PATCH 3/3] Remove pod_uid and further address PR comments --- dev-values.yaml | 1 - src/nublado2/hooks.py | 16 +++++----------- src/nublado2/nublado_config.py | 5 +---- src/nublado2/resourcemgr.py | 17 +++++++++-------- 4 files changed, 15 insertions(+), 24 deletions(-) diff --git a/dev-values.yaml b/dev-values.yaml index 6530b29..14385d1 100644 --- a/dev-values.yaml +++ b/dev-values.yaml @@ -57,7 +57,6 @@ config: - name: Large cpu: 4 ram: 12288M - pod_uid: 769 user_resources: - apiVersion: v1 kind: Namespace diff --git a/src/nublado2/hooks.py b/src/nublado2/hooks.py index 7e706a2..ff74024 100644 --- a/src/nublado2/hooks.py +++ b/src/nublado2/hooks.py @@ -44,17 +44,11 @@ async def pre_spawn(self, spawner: Spawner) -> None: auth_state = await spawner.user.get_auth_state() - # Should we spawn with the uid of the user (from the auth state) - # or the provisioner (769) which will then sudo and become the - # user? - pod_uid = nc.pod_uid() - if pod_uid: - spawner.uid = pod_uid - spawner.gid = pod_uid - else: - spawner.uid = auth_state["uid"] - spawner.gid = auth_state["uid"] - spawner.supplemental_gids = [g["id"] for g in auth_state["groups"]] + # We now always spawn as the target user; there is no way + # to do "provisionator" anymore + spawner.uid = auth_state["uid"] + spawner.gid = auth_state["uid"] + spawner.supplemental_gids = [g["id"] for g in auth_state["groups"]] # Since we will create a serviceaccount in the user resources, # make the pod use that. This will also automount the token, diff --git a/src/nublado2/nublado_config.py b/src/nublado2/nublado_config.py index bd2f880..7c8f576 100644 --- a/src/nublado2/nublado_config.py +++ b/src/nublado2/nublado_config.py @@ -2,7 +2,7 @@ __all__ = ["NubladoConfig"] -from typing import Any, Dict, Optional, Tuple +from typing import Any, Dict, Tuple from ruamel import yaml from ruamel.yaml import RoundTripLoader @@ -25,6 +25,3 @@ def lookup_size(self, name: str) -> Tuple[float, str]: return (float(s["cpu"]), s["ram"]) raise ValueError(f"Size {name} not found") - - def pod_uid(self) -> Optional[int]: - return self.get().get("pod_uid", None) diff --git a/src/nublado2/resourcemgr.py b/src/nublado2/resourcemgr.py index 9c354bf..cb52ddd 100644 --- a/src/nublado2/resourcemgr.py +++ b/src/nublado2/resourcemgr.py @@ -56,7 +56,6 @@ async def create_user_resources(self, spawner: Spawner) -> None: "token": auth_state["token"], "groups": groups, "external_groups": external_groups, - "no_sudo": nc.get("no_sudo"), "base_url": nc.get("base_url"), "dask_yaml": await self._build_dask_template(spawner), } @@ -82,8 +81,6 @@ async def _request_homedir_provisioning(self, spawner: Spawner) -> None: nc = NubladoConfig().get() hc = self.http_client base_url = nc.get("base_url") - signing_key_path = nc.get("signing_key_path") - assert isinstance(base_url, str) and isinstance(signing_key_path, str) uname = spawner.user.name auth_state = await spawner.user.get_auth_state() dossier = { @@ -91,7 +88,7 @@ async def _request_homedir_provisioning(self, spawner: Spawner) -> None: "uid": int(auth_state["uid"]), "groups": auth_state["groups"], } - token = await self._mint_admin_token(base_url, signing_key_path) + token = await self._mint_admin_token() endpt = f"{base_url}/moneypenny/commission" auth = {"Authorization": f"Bearer {token}"} self.log.debug(f"Posting dossier {dossier} to {endpt}") @@ -123,13 +120,17 @@ async def _check_moneypenny_completion() -> bool: timeout=300, ) - async def _mint_admin_token( - self, base_url: str, signing_key_path: str - ) -> str: - """Allowing specification of the signing key makes testing easier.""" + async def _mint_admin_token(self) -> str: + """Create a token with exec:admin scope, signed as if Gafaelfawr had + created it, in order to submit orders to Moneypenny. + """ + nc = NubladoConfig().get() template_file = os.path.join( os.path.dirname(__file__), "static/moneypenny-jwt-template.json" ) + base_url = nc.get("base_url") + signing_key_path = nc.get("signing_key_path") + assert isinstance(signing_key_path, str) with open(signing_key_path, "r") as f: signing_key = f.read() current_time = int(