ideonate / cdsdashboards

JupyterHub extension for ContainDS Dashboards
https://cdsdashboards.readthedocs.io/
Other
199 stars 38 forks source link

Environment variables "JUPYTERHUB_ANYONE" and "JUPYTERHUB_GROUP" not handed over to spawned server #77

Open xeliba opened 3 years ago

xeliba commented 3 years ago

In an environment that was configured according to Z2JH on Azure kubernetes, I have a problem with authentication. Each user is able to create new dashboards and spawn them, but only the creator is allowed to actually access the dashboards in the browser.

Looking deeper into it I realized that the spawned dashboard servers do not have the environment variables "JUPYTERHUB_ANYONE" and "JUPYTERHUB_GROUP" in their environment.

It seems to me that they are simply not included in the spawning process. Looking at the "get_env()" method which should provide all the environment variables for the spawner I see that only the special environment variables for the specific dashboard are included, but not the general ones (like JUPYTERHUB_ANYONE):

https://github.com/ideonate/cdsdashboards/blob/0a3f7de0c722f041e75cb12133f16c4701a8ccaf/cdsdashboards/hubextension/spawners/variablemixin.py#L297

Can you help to fix this? I could try to submit a PR, but maybe my solution would be more complicated than necessary.

danlester commented 3 years ago

I'm not sure why these env vars aren't ending up in your spawned pods, but they aren't supposed to be added in the code you referenced anyway.

They should be set earlier in the 'builder':

https://github.com/ideonate/cdsdashboards/blob/0a3f7de0c722f041e75cb12133f16c4701a8ccaf/cdsdashboards/builder/processbuilder.py#L85

Does that help you debug further?

What version of cdsdashboards and z2jh are you using? (and what z2jh image version if not the default)

xeliba commented 3 years ago

Our custom hub docker image is on jupyterhub==1.4.1 together with cdsdashboards==0.5.7.
We're using the new stable z2jh helm chart version 1.0.0. The spawner is configured as described in your documentation:

cds-kube: |
      c.JupyterHub.spawner_class = 'cdsdashboards.hubextension.spawners.variablekube.VariableKubeSpawner'
      c.CDSDashboardsConfig.builder_class = 'cdsdashboards.builder.kubebuilder.KubeBuilder'

In the logs of the hub pod we get messages about ignored user options:
Ignoring unrecognized KubeSpawner user_options: ... environment ...
Therefore, to me it seems that KubeSpawner (jupyterhub-kubespawner==1.0.0) does not recognize the user option environment and therefore does not ingest the environment variable JUPYTERHUB_ANYONE and .._USER in the spawned dashboard pod.

For me the logical solution would be to add a "get_env()" method to the class VariableKubeSpawner. This would add those additional environment variable to the list, so that finally jupyterhubs spawner does use them. Maybe it should look similar to this: JupyterHub Example Config

danlester commented 3 years ago

I'm really not sure why the standard setup isn't working for you. It shouldn't be necessary to worry about modifying get_env as the builder should take care of setting the env vars.

I have a setup with the following in a custom jupyterhub image and z2jh:

which are of course the same as yours.

Note I also get the Ignoring unrecognized KubeSpawner user_options: ... environment ... message in logs - it looks ominous but is misleading I believe.

But in my case the JUPYTERHUB_ANYONE etc vars are set on the pods.

Does this even go wrong for you on the first start of the dashboard, i.e. when you save the Dashboard edit page and it builds and redirects? (Rather than if you manually stop then start the dashboard server, for example.)

xeliba commented 3 years ago

Seems strange that it does work for you, but not for me. And no, it does not depend on whether the Dashboard pod was started after saving the edit page or if it was restarted later: I never see the env variable JUPYTERHUB_ANYONE in the Dashboard pod.

However, I do not really understand how the hand-over of the environment really works. Since some of the extra environment variables, like e.g. DASH_REQUESTS_PATHNAME_PREFIX for spawning Dashboards using plotlydash are indeed handed over to the server pod. But as far as I understand the code, this happens in VariableMixin.get_env(), or am I wrong?

So how does this work using the the "ProcessBuilder"? Is the KubeSpawner supposed to read those new_server_options and make environment variables from the dict new_server_options["environment"]? I cannot find the code where the jupyterhub KubeSpawner actually does that. But I see these warnings, that those user_options, likeenvironment` are ignored.

xeliba commented 3 years ago

Sorry for bothering you so much with this issue. If you do kubectl -n jhub describe <dashboard_pod_name>, do you then really see JUPYTERHUB_ANYONE and JUPYTERHUB_GROUP in the output in section "Containers -> Environment:"? Because for me they are not existing.

Anyway: A possible fix is to define JUPYTERHUB_ANYONE: "1" in the environment section of the jupyterhub config.yaml. That does not allow for restriction to groups, but at least it opens dashboard access among all the users.

GabeChurch commented 1 year ago

This was helpful for me. I had the exact same issue in my z2jh deployment, multi-user access works when adding that parameter.

For those who may need more help testing this issue themselves, in z2jh helm it can be configured under

singleuser: 
   profileList:
      - display_name: "your_jh_user_pod_config"
         environment: 
            #Dashboard access for cdsdashboards
            JUPYTERHUB_ANYONE: "1"

For reference I started with Hub Image

I thought maybe the jupyterhub-kubespawner was the source of my problems, so next step I decided to test by updating jupyterhub-hub image to danlester recommended hub image

Ultimately that had no impact, the only thing that worked for me so far is doing JUPYTERHUB_ANYONE: "1" (I haven't tried the patch provided via xeliba yet).

@danlester are you running z2jh deployments? I am also running z2jh like xeliba, and it makes sense that Variable mixin overrides are messing up the cdsdashboards builder env behavior based on my tests.