jupyter-on-openshift / jupyterhub-quickstart

OpenShift compatible version of the JupyterHub application.
Apache License 2.0
101 stars 107 forks source link

Upgrade JupyterHub to 1.0.0 #20

Open zkatona opened 5 years ago

zkatona commented 5 years ago

Hi!

Is there a possibility to upgrade JupyterHub from 0.9.4 to 1.0.0? Reason for the request: It has features like:

These could be very well used when creating a custom authenticator. I already have a custom OpenShiftOAuthenticator class, that can restrict access to only those OpenShift users, who have any role in the namespace on OpenShift, where JupyterHub is running. This is working perfectly fine, however, if a user is deleted from the membership of the namespace, and the user is already logged in, I want the user to be forcefully logged out from JupyterHub. I think I could implement this using the latest version of JupyterHub using the above mentioned new features.

Thank you for your help in advance!

Cheers,

Zoltán

GrahamDumpleton commented 5 years ago

I have to be careful when I move to JupyterHub 1.0.0 as that version removes support for configuration settings under some names, forcing you to use newer names. Any examples or applications which use this repo will need to be updated to use newer configuration names otherwise they will stop working. Documentation also needs to be updated and warnings added about the changes. I don't expect to have time to do that for another couple of weeks due to travel.

BTW, is the code for the custom OpenShift OAuth class public somewhere. If it is of general use, it would be better to add such support into the upstream oauthenticator package and make it configurable so the feature is optional. Based on what you have said, would suggest you would be better off being able to configure that a user is in a specific named role by virtue of a role binding, rather than any role. One could even create a dummy rolebinding for doing that, which doesn't actually grant the user any other rights in the project, or in other words just use it to track membership.

GrahamDumpleton commented 4 years ago

JupyterHub 1.0.0 has broken the ability to configure from the jupyterhub_config.py file any authenticators supported by the oauthenticator package which need environment variables to be set. Specifically, the environment variables cannot be set in jupyterhub_config.py prior to setting up the authenticator. This is because JupyterHub now force preloads the authenticator modules, meaning it is too late to set the environment variables. JupyterHub is in effect forcing you to set the environment variables before JupyterHub is even run. This makes it somewhat inconvenient to configure as it can't be all in the jupyterhub_config.py file. Upgrading to JupyerHub 1.0.0 will therefore for example break any configuration using OpenShift authentication, or KeyCloak via generic OAuth authenticator. Right now there doesn't seem to be an easy solution to this.

GrahamDumpleton commented 4 years ago

The problem is more caused by oauthenticator. In 0.9.0 of that package they started registry entrypoints for all the authenticators, which JupyterHub force preloads. Could temporarily use older commit of oauthenticator. Can't use prior tagged version though, as need a change for OCP 4 that only ended up being released in 0.9.0. Up to now had been using fork with the OCP 4 change.

GrahamDumpleton commented 4 years ago

Issue logged against oauthenticator.

guimou commented 4 years ago

Hi Graham, I managed to find a different solution than deleting the module as you explain in the oauthenticator issue. It's done by overriding some parameters in the jupyterhub_config.py: c.JupyterHub.authenticator_class.login_handler._OAUTH_AUTHORIZE_URL=

It's pretty dirty IMHO, but it works (ref: https://github.com/jupyterhub/oauthenticator/issues/271#issuecomment-510965987)

Our full implementation: https://github.com/ulaval/valeria-jupyterhub

GrahamDumpleton commented 4 years ago

Patching into the already loaded authenticator module may require more than just that. Take for example the OpenShift one. Because of:

you also need to patch the cached environment variable at:

So this change to preload the authenticators is a huge pain. The cleanest way which is probably still safe, is to delete the module an reimport it.

Because I don't know what authenticator from oauthenticator may be used, I will probably need to iterate over the list of entrypoints and delete the modules for all of them. Is only way I can preserve ability to configure them from jupyterhub_config.py. It is really inconvenient to require a separate config file, perhaps needing to be a shell script, to set environment variables before JupyterHub starts.

GrahamDumpleton commented 4 years ago

BTW, it was never intended that you should need to fork jupyterhub-quickstart. It was designed in a way to be extendable through S2I. It is unfortunate that you forked it. Would have been better to have made a copy (not fork) of:

and use that as a starting point. That way not modifying jupyterhub-quickstart as the core and just extending the config and adding your own custom templates for deployment.

guimou commented 4 years ago

Yes, I much prefer the deleting approach, I will give it a try. And yes, our original intent was to build upon jupyterhub-quickstart without forking it, as we discussed a few months ago. That's the way we started effectively, but we had to do it at some point for various reasons: first the upgrade to JH 1.0.0 (with other libraries as well), as the building process involved uninstalling libraries from base JH to reinstall over the new versions, which was not very effective. Second (and more important) were the requirements to stay "independant", in case you decided to just wipe out all the repos. Nothing personal of course :-), and we always could have kept a copy somewhere, but that was per organization policy for this kind of thing. And last, as you may have seen, we have reorganized things a little bit to make it clearer for people who will maintain it in production (not that it was not, but again our version is tailored for our specific needs). I am right now finishing the README to give more details. But again, that was part of a development process, and as I would have much liked to stay with the original intent (and now that we know more precisely what we need), maybe I'll try to repackage it with jupyterhub-quickstart as the core (or they will do it at ULaval now that I'm not there anymore).