Expansion of user names fails for strings containing a JSON-like string

enolfc commented 1 year ago

Bug description

Setting environment variables in the single-user server with a JSON value makes kubespawner fail as the python string formatter will raise a key error.

Expected behaviour

The environment variable gets set in the single user environment

Actual behaviour

Spawn fails with a 500 error

How to reproduce

Set a variable with a JSON-string, e.g. using the jupyterhub helm:
```
singleuser:
 extraEnv:
   TEST: '{"foo": "bar"}'
```
Try to spawn the server
See error: Error: HTTP 500: Internal Server Error (Unhandled error starting server enol.fernandez)

Your personal set up

JupyterHub deployed via helm kubespawner version: v6.0.0

Logs

``` [E 2023-06-07 13:23:36.916 JupyterHub user:884] Unhandled error starting enol.fernandez's server: '"foo"' Traceback (most recent call last): File "/usr/local/lib/python3.9/site-packages/jupyterhub/user.py", line 798, in spawn url = await gen.with_timeout(timedelta(seconds=spawner.start_timeout), f) File "/usr/local/lib/python3.9/site-packages/kubespawner/spawner.py", line 2679, in _start pod = await self.get_pod_manifest() File "/usr/local/lib/python3.9/site-packages/kubespawner/spawner.py", line 1996, in get_pod_manifest env=self._expand_all(self.get_env()), File "/usr/local/lib/python3.9/site-packages/kubespawner/spawner.py", line 1829, in _expand_all return {k: self._expand_all(v) for k, v in src.items()} File "/usr/local/lib/python3.9/site-packages/kubespawner/spawner.py", line 1829, in return {k: self._expand_all(v) for k, v in src.items()} File "/usr/local/lib/python3.9/site-packages/kubespawner/spawner.py", line 1831, in _expand_all return self._expand_user_properties(src) File "/usr/local/lib/python3.9/site-packages/kubespawner/spawner.py", line 1812, in _expand_user_properties rendered = template.format( KeyError: '"foo"' ```

enolfc commented 1 year ago

Escaping the { in the string works, {{"foo": "bar"}}

However this would fail for variables that are not set by in the environment but automatically from JupyterHub, namely tornado_settings.cookie_options, config:

c.JupyterHub.tornado_settings = {"cookie_options": {"samesite": "None", "secure": True}}

Error in the log:

[E 2023-06-07 13:48:50.871 JupyterHub user:884] Unhandled error starting enol.fernandez's server: '"samesite"'
    Traceback (most recent call last):
      File "/usr/local/lib/python3.9/site-packages/jupyterhub/user.py", line 798, in spawn
        url = await gen.with_timeout(timedelta(seconds=spawner.start_timeout), f)
      File "/usr/local/lib/python3.9/site-packages/kubespawner/spawner.py", line 2679, in _start
        pod = await self.get_pod_manifest()
      File "/usr/local/lib/python3.9/site-packages/kubespawner/spawner.py", line 1996, in get_pod_manifest
        env=self._expand_all(self.get_env()),
      File "/usr/local/lib/python3.9/site-packages/kubespawner/spawner.py", line 1829, in _expand_all
        return {k: self._expand_all(v) for k, v in src.items()}
      File "/usr/local/lib/python3.9/site-packages/kubespawner/spawner.py", line 1829, in <dictcomp>
        return {k: self._expand_all(v) for k, v in src.items()}
      File "/usr/local/lib/python3.9/site-packages/kubespawner/spawner.py", line 1831, in _expand_all
        return self._expand_user_properties(src)
      File "/usr/local/lib/python3.9/site-packages/kubespawner/spawner.py", line 1812, in _expand_user_properties
        rendered = template.format(
    KeyError: '"samesite"'

yuvipanda commented 1 year ago

This was a breaking change intentionally introduced in https://github.com/jupyterhub/kubespawner/pull/642.

@minrk thoughts on how to handle the JupyterHub case?

minrk commented 1 year ago

Templating should probably only be appplied to self.environment, not the result of get_env().

Probably the easiest is overriding get_env like:

def get_env(self):
    env = self.get_env()
    env.update(self._expand_all(self.environment))
    return env

The only inefficiency would be that the un-templated self.environment values would be in env briefly before being immediately replaced by the templated values. But that's a few dict assignments, not exactly a big cost.

yuvipanda commented 12 months ago

jupyterhub / kubespawner