Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
enolfc opened this issue Jun 7, 2023 · 4 comments · Fixed by #759
Closed

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

enolfc opened this issue Jun 7, 2023 · 4 comments · Fixed by #759
Labels

Comments

@enolfc
Copy link

enolfc commented Jun 7, 2023

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

  1. Set a variable with a JSON-string, e.g. using the jupyterhub helm:
    singleuser:
      extraEnv:
        TEST: '{"foo": "bar"}'
    
  2. Try to spawn the server
  3. 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 <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: '"foo"'
@enolfc enolfc added the bug label Jun 7, 2023
@enolfc
Copy link
Author

enolfc commented Jun 7, 2023

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
Copy link
Collaborator

This was a breaking change intentionally introduced in #642.

@minrk thoughts on how to handle the JupyterHub case?

@minrk
Copy link
Member

minrk commented Jun 29, 2023

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 added a commit to yuvipanda/kubespawner that referenced this issue Jul 26, 2023
Ignores env variables set by JupyterHub itself, which may
contain JSON-like strings.

Fixes jupyterhub#743
@yuvipanda
Copy link
Collaborator

Suggested fix in #759

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants