Skip to content

Commit

Permalink
Merge pull request #459 from lsst-sqre:tickets/DM-48546
Browse files Browse the repository at this point in the history
DM-48546: Add support for blocking spawn with a quota flag
  • Loading branch information
rra authored Jan 23, 2025
2 parents 6572d1d + f37dfb3 commit c30f143
Show file tree
Hide file tree
Showing 11 changed files with 155 additions and 17 deletions.
3 changes: 3 additions & 0 deletions changelog.d/20250122_163137_rra.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
### New features

- Support the new `spawn` flag in the user's Gafaelfawr metadata. If set to false, tell the user that spawns aren't allowed rather than presenting a menu and reject spawns in the Nublado controller with a 403 error.
7 changes: 6 additions & 1 deletion controller/src/controller/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,12 @@ class NotConfiguredError(ClientRequestError):


class PermissionDeniedError(ClientRequestError):
"""Attempt to access a resource for another user."""
"""Attempt to access a restricted resource.
This exception may indicate an attempt to access a resource for another
user or an attempt to spawn a lab when the user's quota does not allow
lab spawning.
"""

error = "permission_denied"
status_code = status.HTTP_403_FORBIDDEN
Expand Down
5 changes: 5 additions & 0 deletions controller/src/controller/handlers/form.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ async def get_user_lab_form(
) -> Response:
if username != user.username:
raise PermissionDeniedError("Permission denied")
if user.quota and user.quota.notebook:
if not user.quota.notebook.spawn:
return templates.TemplateResponse(
context.request, "unavailable.html.jinja"
)
images = context.image_service.menu_images()

# Filter the list of configured lab sizes to exclude labs that are larger
Expand Down
6 changes: 6 additions & 0 deletions controller/src/controller/models/domain/gafaelfawr.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,12 @@ class NotebookQuota(BaseModel):
float, Field(title="Maximum memory use (GiB)", examples=[16.0])
]

spawn: bool = Field(
True,
title="Spawning allowed",
description="Whether the user is allowed to spawn a notebook",
)

@property
def memory_bytes(self) -> int:
"""Maximum memory use in bytes."""
Expand Down
24 changes: 23 additions & 1 deletion controller/src/controller/services/lab.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
MissingSecretError,
NoOperationError,
OperationConflictError,
PermissionDeniedError,
UnknownUserError,
)
from ..models.domain.docker import DockerReference
Expand Down Expand Up @@ -245,11 +246,14 @@ async def create_lab(
OperationConflictError
Raised if some other operation (either spawn or delete) is already
in progress on this user's lab.
PermissionDeniedError
Raised if the user's quota does not allow them to spawn labs.
"""
username = user.username
self._check_quota(user)

# If the user was not previously seen, set up their data structure and
# monitor class.
username = user.username
if username not in self._labs:
monitor = _LabMonitor(
username=username,
Expand Down Expand Up @@ -644,6 +648,24 @@ async def stop_monitor_tasks(self) -> None:
for state in labs.values():
await state.monitor.cancel()

def _check_quota(self, user: GafaelfawrUser) -> None:
"""Check if lab spawning is allowed by the user's quota.
Parameters
----------
user
User information for the user attempting to spawn a lab.
Raises
------
PermissionDeniedError
Raised if the user's quota does not allow them to spawn labs.
"""
if not user.quota or not user.quota.notebook:
return
if not user.quota.notebook.spawn:
raise PermissionDeniedError("Spawning a notebook is not permitted")

async def _delete_lab(
self,
username: str,
Expand Down
2 changes: 2 additions & 0 deletions controller/src/controller/templates/unavailable.html.jinja
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<h2>Notebooks unavailable</h2>
<p>Creating a new Nublado notebook is currently unavailable.</p>
76 changes: 61 additions & 15 deletions controller/tests/data/base/input/users.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,22 @@
"uid": 1101,
"gid": 1101,
"groups": [
{"name": "rachel", "id": 1101},
{"name": "lunatics", "id": 2028},
{"name": "mechanics", "id": 2001},
{"name": "storytellers", "id": 2021}
{
"name": "rachel",
"id": 1101
},
{
"name": "lunatics",
"id": 2028
},
{
"name": "mechanics",
"id": 2001
},
{
"name": "storytellers",
"id": 2021
}
],
"quota": {
"notebook": {
Expand All @@ -23,9 +35,18 @@
"uid": 1102,
"gid": 1102,
"groups": [
{"name": "wrench", "id": 1102},
{"name": "jovians", "id": 2010},
{"name": "mechanics", "id": 2001}
{
"name": "wrench",
"id": 1102
},
{
"name": "jovians",
"id": 2010
},
{
"name": "mechanics",
"id": 2001
}
]
},
"token-of-mystery": {
Expand All @@ -34,9 +55,18 @@
"uid": 1103,
"gid": 1103,
"groups": [
{"name": "violet", "id": 1103},
{"name": "saturnians", "id": 2011},
{"name": "pirates", "id": 2002}
{
"name": "violet",
"id": 1103
},
{
"name": "saturnians",
"id": 2011
},
{
"name": "pirates",
"id": 2002
}
]
},
"token-of-stories": {
Expand All @@ -45,9 +75,25 @@
"uid": 1104,
"gid": 1104,
"groups": [
{"name": "ribbon", "id": 1104},
{"name": "ferrymen", "id": 2023},
{"name": "ninjas", "id": 2003}
]
{
"name": "ribbon",
"id": 1104
},
{
"name": "ferrymen",
"id": 2023
},
{
"name": "ninjas",
"id": 2003
}
],
"quota": {
"notebook": {
"cpu": 4.0,
"memory": 16.0,
"spawn": false
}
}
}
}
}
2 changes: 2 additions & 0 deletions controller/tests/data/standard/output/lab-unavailable.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
<h2>Notebooks unavailable</h2>
<p>Creating a new Nublado notebook is currently unavailable.</p>
13 changes: 13 additions & 0 deletions controller/tests/handlers/form_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from controller.models.domain.gafaelfawr import GafaelfawrUser

from ..support.data import read_output_data
from ..support.gafaelfawr import get_no_spawn_user


@pytest.mark.asyncio
Expand All @@ -33,3 +34,15 @@ async def test_errors(client: AsyncClient, user: GafaelfawrUser) -> None:
assert r.json() == {
"detail": [{"msg": "Permission denied", "type": "permission_denied"}]
}


@pytest.mark.asyncio
async def test_quota_spawn(client: AsyncClient) -> None:
token, user = get_no_spawn_user()
r = await client.get(
f"/nublado/spawner/v1/lab-form/{user.username}",
headers=user.to_headers(),
)
assert r.status_code == 200
expected = read_output_data("standard", "lab-unavailable.html").strip()
assert r.text == expected
15 changes: 15 additions & 0 deletions controller/tests/handlers/labs_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
read_output_data,
read_output_json,
)
from ..support.gafaelfawr import get_no_spawn_user
from ..support.kubernetes import objects_to_dicts


Expand Down Expand Up @@ -915,3 +916,17 @@ async def test_extra_annotations(
pod = await mock_kubernetes.read_namespaced_pod(pod_name, namespace)
for key, value in config.lab.extra_annotations.items():
assert pod.metadata.annotations[key] == value


@pytest.mark.asyncio
async def test_quota_no_spawn(client: AsyncClient) -> None:
"""Check that spawning is denied for a user blocked by quota."""
lab = read_input_lab_specification_json("base", "lab-specification")
token, user = get_no_spawn_user()

r = await client.post(
f"/nublado/spawner/v1/labs/{user.username}/create",
json={"options": lab.options.model_dump(), "env": lab.env},
headers=user.to_headers(),
)
assert r.status_code == 403
19 changes: 19 additions & 0 deletions controller/tests/support/gafaelfawr.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
GafaelfawrUserInfo,
)

from .data import read_input_users_json

__all__ = ["MockGafaelfawr", "register_mock_gafaelfawr"]


Expand Down Expand Up @@ -80,3 +82,20 @@ def register_mock_gafaelfawr(
api_url = f"{base_url}/auth/api/v1/user-info"
respx_mock.get(api_url).mock(side_effect=mock.get_info)
return mock


def get_no_spawn_user() -> tuple[str, GafaelfawrUser]:
"""Find a user whose quota says they can't spawn labs.
Returns
-------
str, GafaelfawrUser
Token and corresponding user data structure for a user with a quota
set that forbids spawning labs.
"""
users = read_input_users_json("base", "users")
for token, user in users.items():
if user.quota and user.quota.notebook:
if not user.quota.notebook.spawn:
return token, GafaelfawrUser(token=token, **user.model_dump())
raise ValueError("No users found with a quota forbidding spawning")

0 comments on commit c30f143

Please sign in to comment.