Skip to content

Commit

Permalink
use . to separate username and servername in object names
Browse files Browse the repository at this point in the history
separator must not be a safe character or the escape character to avoid collisions

'.' is not in the safe char list, but is a valid docker identifier

- servername is template variable is escaped, matching username
- add raw_servername for the raw value
- fix safe_username to be the safe version
  • Loading branch information
minrk committed Jul 20, 2020
1 parent 05f5433 commit b8c0ca4
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 26 deletions.
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ before_install:
- docker swarm init
- nvm install 10; nvm use 10
- npm install -g configurable-http-proxy
- pip install --upgrade setuptools pip
- pip install --upgrade -r dev-requirements.txt jupyterhub==${JUPYTERHUB:-0}.*

install:
Expand Down
20 changes: 14 additions & 6 deletions dockerspawner/dockerspawner.py
Original file line number Diff line number Diff line change
Expand Up @@ -326,16 +326,21 @@ def options_from_form(self, formdata):
It is important to include {servername} if JupyterHub's "named
servers" are enabled (JupyterHub.allow_named_servers = True).
If the server is named, the default name_template is
"{prefix}-{username}-{servername}". If it is unnamed, the default
"{prefix}-{username}.{servername}". If it is unnamed, the default
name_template is "{prefix}-{username}".
Note: when using named servers,
it is important that the separator between {username} and {servername}
is not a character that can occur in an escaped {username},
and also not the single escape character '_'.
"""
),
)

@default('name_template')
def _default_name_template(self):
if self.name:
return "{prefix}-{username}-{servername}"
return "{prefix}-{username}.{servername}"
else:
return "{prefix}-{username}"

Expand Down Expand Up @@ -728,17 +733,20 @@ def _escape(self, s):
def template_namespace(self):
escaped_image = self.image.replace("/", "_")
server_name = getattr(self, "name", "")
safe_server_name = self._escape(server_name.lower())
return {
"username": self.escaped_name,
"safe_username": self.user.name,
"safe_username": self.escaped_name,
"raw_username": self.user.name,
"imagename": escaped_image,
"servername": server_name,
"servername": safe_server_name,
"raw_servername": server_name,
"prefix": self.prefix,
}

@property
def object_name(self):
object_name = Unicode()
@default("object_name")
def _object_name_default(self):
"""Render the name of our container/service using name_template"""
return self.name_template.format(**self.template_namespace())

Expand Down
18 changes: 14 additions & 4 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,25 @@


@pytest.fixture
def dockerspawner_configured_app(app):
def named_servers(app):
with mock.patch.dict(
app.tornado_settings,
{"allow_named_servers": True, "named_server_limit_per_user": 2},
):
yield


@pytest.fixture
def dockerspawner_configured_app(app, named_servers):
"""Configure JupyterHub to use DockerSpawner"""
app.config.DockerSpawner.prefix = "dockerspawner-test"
# app.config.DockerSpawner.remove = True
with mock.patch.dict(app.tornado_settings, {"spawner_class": DockerSpawner}):
yield app


@pytest.fixture
def swarmspawner_configured_app(app):
def swarmspawner_configured_app(app, named_servers):
"""Configure JupyterHub to use DockerSpawner"""
app.config.SwarmSpawner.prefix = "dockerspawner-test"
with mock.patch.dict(
Expand All @@ -37,9 +47,9 @@ def swarmspawner_configured_app(app):


@pytest.fixture
def systemuserspawner_configured_app(app):
def systemuserspawner_configured_app(app, named_servers):
"""Configure JupyterHub to use DockerSpawner"""
app.config.SwarmSpawner.prefix = "dockerspawner-test"
app.config.DockerSpawner.prefix = "dockerspawner-test"
with mock.patch.dict(
app.tornado_settings, {"spawner_class": SystemUserSpawner}
):
Expand Down
30 changes: 24 additions & 6 deletions tests/test_dockerspawner.py
Original file line number Diff line number Diff line change
@@ -1,35 +1,53 @@
"""Tests for DockerSpawner class"""

import asyncio
import json

import asyncio
import docker
import pytest
from jupyterhub.tests.test_api import add_user, api_request
from jupyterhub.tests.mocking import public_url
from jupyterhub.utils import url_path_join
from tornado.httpclient import AsyncHTTPClient, HTTPRequest
from tornado.httpclient import AsyncHTTPClient

from dockerspawner import DockerSpawner

# Mark all tests in this file as asyncio
pytestmark = pytest.mark.asyncio

def test_name_collision(dockerspawner_configured_app):
app = dockerspawner_configured_app
has_hyphen = "user--foo"
add_user(app.db, app, name=has_hyphen)
user = app.users[has_hyphen]
spawner1 = user.spawners[""]
assert isinstance(spawner1, DockerSpawner)
assert spawner1.object_name == "{}-{}".format(spawner1.prefix, has_hyphen)

part1, part2 = ["user", "foo"]
add_user(app.db, app, name=part1)
user2 = app.users[part1]
spawner2 = user2.spawners[part2]
assert spawner1.object_name != spawner2.object_name


async def test_start_stop(dockerspawner_configured_app):
app = dockerspawner_configured_app
name = "has@"
add_user(app.db, app, name=name)
user = app.users[name]
assert isinstance(user.spawner, DockerSpawner)
server_name = 'also-has@'
spawner = user.spawners[server_name]
assert isinstance(spawner, DockerSpawner)
token = user.new_api_token()
# start the server
r = await api_request(app, "users", name, "server", method="post")
r = await api_request(app, "users", name, "servers", server_name, method="post")
while r.status_code == 202:
# request again
r = await api_request(app, "users", name, "server", method="post")
r = await api_request(app, "users", name, "servers", server_name, method="post")
assert r.status_code == 201, r.text

url = url_path_join(public_url(app, user), "api/status")
url = url_path_join(public_url(app, user), server_name, "api/status")
resp = await AsyncHTTPClient().fetch(url, headers={"Authorization": "token %s" % token})
assert resp.effective_url == url
resp.rethrow()
Expand Down
17 changes: 9 additions & 8 deletions tests/test_swarmspawner.py
Original file line number Diff line number Diff line change
@@ -1,34 +1,35 @@
"""Tests for SwarmSpawner"""

from unittest import mock

import pytest
from jupyterhub.tests.test_api import add_user, api_request
from jupyterhub.tests.mocking import public_url
from jupyterhub.utils import url_path_join
from tornado.httpclient import AsyncHTTPClient, HTTPRequest
from tornado.httpclient import AsyncHTTPClient

from dockerspawner import SwarmSpawner

# Mark all tests in this file as asyncio
pytestmark = pytest.mark.asyncio


async def test_start_stop(swarmspawner_configured_app):
app = swarmspawner_configured_app
name = "somebody"
add_user(app.db, app, name=name)
user = app.users[name]
assert isinstance(user.spawner, SwarmSpawner)
server_name = 'also-has@'
spawner = user.spawners[server_name]
assert isinstance(spawner, SwarmSpawner)
token = user.new_api_token()
# start the server
r = await api_request(app, "users", name, "server", method="post")
r = await api_request(app, "users", name, "servers", server_name, method="post")
while r.status_code == 202:
# request again
r = await api_request(app, "users", name, "server", method="post")
r = await api_request(app, "users", name, "servers", server_name, method="post")
assert r.status_code == 201, r.text

url = url_path_join(public_url(app, user), "api/status")
url = url_path_join(public_url(app, user), server_name, "api/status")
resp = await AsyncHTTPClient().fetch(url, headers={"Authorization": "token %s" % token})
assert resp.effective_url == url
resp.rethrow()
assert "kernels" in resp.body.decode("utf-8")
assert "kernels" in resp.body.decode("utf-8")
3 changes: 1 addition & 2 deletions tests/test_systemuserspawner.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
"""Tests for SwarmSpawner"""

from getpass import getuser
from unittest import mock

import pytest
from jupyterhub.tests.test_api import add_user, api_request
from jupyterhub.tests.mocking import public_url
from jupyterhub.utils import url_path_join
from tornado.httpclient import AsyncHTTPClient, HTTPRequest
from tornado.httpclient import AsyncHTTPClient

from dockerspawner import SystemUserSpawner

Expand Down

0 comments on commit b8c0ca4

Please sign in to comment.