-
Notifications
You must be signed in to change notification settings - Fork 39
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
Apply spawner overrides from group properties. #4715
base: staging
Are you sure you want to change the base?
Apply spawner overrides from group properties. #4715
Conversation
An admin must set group properties of the form kubespawner_override.{property}, such as kubespawner_override.mem_limit. This can be performed in /hub/admin > Manage Groups. It will be important to make sure course staff have sufficient privileges to do what they need (stop/start servers, view server lists, etc.) while at the same time ensuring the cluster does not scale more than it needs to. In other words, we may want to create a role for course staff with fewer privileges than `admin`.
this is great, thanks @ryanlovett ! i also agree that we'll need another level of let's chat about this next week! should be pretty quick to come up w/a name and unravel perms (i hope). :) |
@balajialg shared in slack:
I reviewed the responses. Course staff said they used admin access for:
Interestingly, one course staff requested:
So it makes sense to set up something like the following: role for staff for a course with a given bcourse ID: {
"name": f"course-staff-{bcourse_id}",
"scopes": [
"admin-ui",
f"list:users!group=course::{bcourse_id}",
f"admin:servers!group=course::{bcourse_id}",
f"access:servers!group=course::{bcourse_id}",
],
"groups": [f"course::{bcourse_id}::enrollment_type::teacher", f"course::{bcourse_id}::enrollment_type::tas"]
} See available scopes, which interestingly contains nearly this exact example for some random course named "data8". (go figure; authored by @minrk!) The role above gives course staff the ability to access other course staff servers in the same way they do student servers. We could limit it to student servers only, but I feel like course staff don't necessarily want to hide things from other course staff, and may in fact benefit from being able to test things, e.g. starting another course staff's server. So the idea would be that for each course that opts into elevated privileges, we'd add them to a config list that would then be passed in |
If we wanted to automatically create course staff roles for every course, we would need to invoke c.JupyterHub.load_roles as above. If we only want to create such roles for specific courses that ask for "admin privileges", we can use the z2jh hub configuration for this. For each course we would manually add: # `123456` is an imaginary canvas course ID
hub:
loadRoles:
course-staff-123456:
description: Enable course staff to view and access servers.
# this role provides permissions to...
scopes:
- admin-ui
- list:users!group=course::123456
- admin:servers!group=course::123456
- access:servers!group=course::123456
# this role will be assigned to...
groups:
- course::123456::enrollment_type::teacher
- course::123456::enrollment_type::tas This configuration would obviate the need for In the long term, I think it'd be better to augment hub/values.yaml so that it dynamically loads roles for all courses on the hub. This would eliminate the need for course staff to ask for it, and is a more natural fit for instructional software where course staff automatically have the privileges they need. |
this is GREAT! thanks @ryanlovett for writing this up. i honestly believe that we should get this sorted for a course or two asap, and then once we're happy w/how it all works move the config to |
@shaneknapp I'll swap out the |
I tested |
@ryanlovett Just read the scopes document in detail. This is awesome hacking and excited to see the custom scopes live in Data 8 & 100 setting. I am assuming that course staff will still interpret the customized scopes as admin access and as a result, our instructor-facing documentation + issue template will not undergo any change (except for the previous template change to update bcourses is?). |
@balajialg Course staff should still be able to do what they have been doing, i.e. starting, stopping, accessing servers, etc. Their scope will no longer be what jupyterhub considers "admin" which is everything, but they should be able to function the same way. If not, we should identify what scopes they need and add them. At some point we should adjust the documentation to be more precise. For example we should change the guide so that it uses "Course Staff Privileges" instead of "Hub Admin Privileges", and replace the word admin where appropriate. We would also be able to remove the caveat on that page about shared admin access on the same hub since the new roles will be safer. Similarly we should eventually change the wording on the issue template to be "Course Staff Access Request" instead of "Admin Access Request". If we ultimately go with the more dynamic configuration, where we automatically give course staff expanded roles, then we can eliminate the issue template, and change the docs so that it just mentions how to make use of the access rather than also include how to acquire the privileges. |
@ryanlovett I am aligned with replacing the word "admin" with "course staff" related verbiage in the curriculum guide and issue template. I will do that once our first few requests are resolved successfully. Re: Dynamic configuration - Sharing couple of hypotheses in my mind,
|
I can confirm that the static configuration stanzas only serve that semester's course, so next semester's Data 8/100 won't have elevated privs automatically. (bcourse IDs are only use during a single term) We don't make use of the bcourse course end date, so if someone was a member of a course, the hub still considers them to be a member. (just like how bcourses shows you all of your expired courses) This means that if you were once course staff for Data 100, and several semesters on if that static configuration remains, you will still have elevated privileges. At the end of every semester, we should remove the loadRoles configs that grant elevated privileges. Alternatively, we could augment the CanvasOAuthenticator to pay attention to the end data of the course, and then not elevate privs for course staff for courses that have expired. This is not a great option because sometimes courses are still active for a little while after the term. We also need to reset the
Yes, if we knew the bcourse IDs in advance, we could elevate privileges for course staff and eliminate admin requests. |
Thanks for the clarity about the validity of bcourses id and the complication with identifying the end date of the course. Claring sqlite db at the end of semester makes sense. I am curious if I can write a python program that queries bcourses APIs to fetch the bcourses id for all the courses listed in this sheet? I remember doing that sometime ago but not able to exactly identify the context Edit: I did the reverse in this notebook/ Queried the name of the course from the bcourses API by sharing the bcourses id. |
There is an API that lets you search for all courses, https://canvas.instructure.com/doc/api/search.html#method.search.all_courses. If that output included course names, you'd be able to match it with courses in your spreadsheet and then extract the IDs. |
pinging @ryanlovett -- i recall that we spoke about this at some length last summer and decided that 'security through obscurity' for some decidedly powerful abilities wasn't something that we wanted to deploy. if so, we can probably close this. if not, we could potentially discuss more. |
@shaneknapp There used to be a security through obscurity issue, however we no longer give admin access to course staff.
We do now assign course staff a role other than full admin. Changes made through the jupyterhub UI can't be easily communicated back to the deployment -- they would only exist in the jupyterhub ORM. Should the database file get deleted, any changes made from the web UI and not also set in our deployment configuration would be lost. This is probably not a big deal though. The databases are not intentionally deleted until between terms, and even then it has not been completely regular. I think a database was only ever accidentally corrupted once by someone who was editing it with Lastly, and perhaps more importantly, there isn't a great story around oversight. While it'd be great to give course staff access to temporarily bump resources, reducing the involvement of DataHub staff and the time it takes for changes to go through CI, course staff would be able to change singleuser params like memory allocations without fully understanding costs issues, node resources, etc. There is no feature which could help us set memory ranges or limit time periods, or just straight up communicates the costs involved. Perhaps if/when the DataHub financial model changes this could be re-evaluated. All that being said, if there was a singleuser server param where it'd be convenient for course staff to be able to change directly that would not also impact compute/financial resources (like env vars for gh scoped creds 😄 ), we could enable the feature but filter for the safe params. |
An admin must set group properties of the form kubespawner_override.{property}, such as kubespawner_override.mem_limit. This can be performed in /hub/admin > Manage Groups.
It will be important to make sure course staff have sufficient privileges to do what they need (stop/start servers, view server lists, etc.) while at the same time ensuring the cluster does not scale more than it needs to. In other words, we may want to create a role for course staff with fewer privileges than
admin
.