jupyterhub / kubespawner

Kubernetes spawner for JupyterHub
https://jupyterhub-kubespawner.readthedocs.io
BSD 3-Clause "New" or "Revised" License
536 stars 301 forks source link

extraEnv value breaks template formatting in _expand_user_properties #763

Closed ryanlovett closed 11 months ago

ryanlovett commented 11 months ago

Bug description

We had been setting the value of a singleuser.extraEnv variable to a json string. When we updated to 3.0.1 helm chart, kubespawner started to fail in _expand_user_properties when invoking template.format and user servers would not spawn.

Expected behaviour

extraEnv values would not affect spawning.

Actual behaviour

User servers would not spawn.

How to reproduce

Set an singleuser.ExtraEnv var, e.g.

    singleuser:
        extraEnv:
            DEBUG_EXTRA_ENV: '{ "foo": "bar" }'

For user user1, the hub raises:

[E 2023-08-15 17:51:51.491 JupyterHub user:884] Unhandled error starting user1's server: ' "foo"'
    Traceback (most recent call last):
      File "/usr/local/lib/python3.11/site-packages/jupyterhub/user.py", line 798, in spawn
        url = await gen.with_timeout(timedelta(seconds=spawner.start_timeout), f)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "<string>", line 124, in start
      File "/usr/local/lib/python3.11/site-packages/kubespawner/spawner.py", line 2687, in _start
        pod = await self.get_pod_manifest()
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/usr/local/lib/python3.11/site-packages/kubespawner/spawner.py", line 2004, in get_pod_manifest
        env=self._expand_all(self.get_env()),
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/usr/local/lib/python3.11/site-packages/kubespawner/spawner.py", line 1837, in _expand_all
        return {k: self._expand_all(v) for k, v in src.items()}
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/usr/local/lib/python3.11/site-packages/kubespawner/spawner.py", line 1837, in <dictcomp>
        return {k: self._expand_all(v) for k, v in src.items()}
                   ^^^^^^^^^^^^^^^^^^^
      File "/usr/local/lib/python3.11/site-packages/kubespawner/spawner.py", line 1839, in _expand_all
        return self._expand_user_properties(src)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/usr/local/lib/python3.11/site-packages/kubespawner/spawner.py", line 1820, in _expand_user_properties
        rendered = template.format(
                   ^^^^^^^^^^^^^^^^
    KeyError: ' "foo"'

In investing this, I added some debugging to kubespawner, and logged the template value:

[I 2023-08-15 17:51:51.488 JupyterHub spawner:1812] template='{ "foo": "bar" }'

Initially the variable value was more complex, but I reduced it to this simpler reproducible string for debugging. We had been setting something like:

    singleuser:
        extraEnv:
            IMPORTANT_VAR: |-
              {
                "type": "service_account",
                ...
              }

Simple values like DEBUG_EXTRA_ENV: FOO work fine.

Your personal set up

We just updated to 3.0.1 helm chart from 2.0.1-0.dev.git.6085.h59225bad. Our deployment is at https://github.com/berkeley-dsep-infra/datahub. In this case the extraEnv setting is in a sops-encrypted file.

consideRatio commented 11 months ago

Writing from my mobile, its a breaking change introduced in kubespawner 5.0.0 i think. Im writing from a mobile, but it should be announced in the changelog.

ryanlovett commented 11 months ago

@consideRatio I see it now, https://github.com/jupyterhub/kubespawner/pull/642, thanks! Testing...

ryanlovett commented 11 months ago

@consideRatio Double braces worked perfectly. I'm going to note in our local docs to check upstream changelogs when we bump major/minor versions!

Sorry for the noise, and thanks again!