-
Notifications
You must be signed in to change notification settings - Fork 97
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
support updated kubespawner #2938
base: main
Are you sure you want to change the base?
Changes from all commits
e709f09
b7e19fc
85c6321
1d9821c
5829038
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,6 @@ | |
from pathlib import Path | ||
|
||
import z2jh | ||
from tornado import gen | ||
|
||
|
||
def base_profile_home_mounts(username): | ||
|
@@ -512,7 +511,6 @@ def render_profile( | |
return None | ||
|
||
profile = copy.copy(profile) | ||
profile_kubespawner_override = profile.get("kubespawner_override") | ||
profile["kubespawner_override"] = functools.reduce( | ||
deep_merge, | ||
[ | ||
|
@@ -522,38 +520,27 @@ def render_profile( | |
base_profile_extra_mounts(), | ||
configure_user(username, groups), | ||
configure_user_provisioned_repositories(username), | ||
profile_kubespawner_override, | ||
profile.get("kubespawner_override"), | ||
], | ||
{}, | ||
) | ||
|
||
# We need to merge any env vars from the spawner with any overrides from the profile | ||
# This is mainly to ensure JUPYTERHUB_ANYONE/GROUP is passed through from the spawner | ||
# to control dashboard access. | ||
envvars_fixed = {**(profile["kubespawner_override"].get("environment", {}))} | ||
|
||
def preserve_envvars(spawner): | ||
# This adds in JUPYTERHUB_ANYONE/GROUP rather than overwrite all env vars, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The behavior of Kubespawner changed in v5 so kubespawner_override's default behavior is merge instead of replace now so I'm now simplifing this part. |
||
# if set in the spawner for a dashboard to control access. | ||
return { | ||
**envvars_fixed, | ||
**spawner.environment, | ||
profile["kubespawner_override"]["environment"].update( | ||
{ | ||
**profile_argo_token(groups), | ||
**profile_conda_store_viewer_token(), | ||
} | ||
|
||
profile["kubespawner_override"]["environment"] = preserve_envvars | ||
) | ||
|
||
return profile | ||
|
||
|
||
@gen.coroutine | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. flyby: tornado coroutine to native coroutine |
||
def render_profiles(spawner): | ||
async def render_profiles(spawner): | ||
# jupyterhub does not yet manage groups but it will soon | ||
# so for now we rely on auth_state from the keycloak | ||
# userinfo request to have the groups in the key | ||
# "auth_state.oauth_user.groups" | ||
auth_state = yield spawner.user.get_auth_state() | ||
auth_state = await spawner.user.get_auth_state() | ||
|
||
username = auth_state["oauth_user"]["preferred_username"] | ||
|
||
|
@@ -570,7 +557,7 @@ def render_profiles(spawner): | |
|
||
# fetch available profiles and render additional attributes | ||
profile_list = z2jh.get_config("custom.profiles") | ||
return list( | ||
rendered_profiles = list( | ||
filter( | ||
None, | ||
[ | ||
|
@@ -585,6 +572,7 @@ def render_profiles(spawner): | |
], | ||
) | ||
) | ||
return rendered_profiles | ||
|
||
|
||
c.KubeSpawner.args = ["--debug"] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Adam-D-Lewis , I think this was not suposed to be committed as part of this PR, is that right?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NVM, just saw your comment above.