From 32148d7a268113c9ed075f2bd980060c10997b3e Mon Sep 17 00:00:00 2001 From: Min RK Date: Sat, 10 Feb 2018 12:13:24 +0100 Subject: [PATCH 1/5] add image_whitelist and default options form if c.DockerSpawner.image_whitelist is specified, a default options form is constructed with a select dropdown for choosing the image to launch. image_whitelist is a dict of key: image, where the key is the value to be used in the form, while the values are the actual docker images. image_whitelist may be specified as a list, in which case it is cast to a dict of the form {image:image}, where the keys are the same as the values (users see actual image names, rather than abbreviations/descriptions). --- dockerspawner/dockerspawner.py | 101 ++++++++++++++++++++++++++++++--- 1 file changed, 93 insertions(+), 8 deletions(-) diff --git a/dockerspawner/dockerspawner.py b/dockerspawner/dockerspawner.py index 41894c7c..e02baadf 100644 --- a/dockerspawner/dockerspawner.py +++ b/dockerspawner/dockerspawner.py @@ -13,11 +13,22 @@ import docker from docker.errors import APIError from docker.utils import kwargs_from_env -from tornado import gen +from tornado import gen, web from escapism import escape from jupyterhub.spawner import Spawner -from traitlets import Dict, Unicode, Bool, Int, Any, default, observe +from traitlets import ( + Any, + Bool, + Dict, + List, + Int, + Unicode, + Union, + default, + observe, + validate, +) from .volumenamingstrategy import default_format_volume_name @@ -117,6 +128,7 @@ def _container_ip_deprecated(self, change): """, config=True, ) + @default('host_ip') def _default_host_ip(self): docker_host = os.getenv('DOCKER_HOST') @@ -177,6 +189,59 @@ def _container_image_changed(self, change): """, ) + image_whitelist = Union( + [Dict(), List()], + config=True, + help=""" + List or dict of images that users can run. + + If specified, users will be presented with a form + from which they can select an image to run. + """, + ) + + @validate('image_whitelist') + def _image_whitelist_dict(self, proposal): + """cast image_whitelist to a dict + + If passing a list, cast it to a {item:item} + dict where the keys and values are the same. + """ + whitelist = proposal.value + if not isinstance(whitelist, dict): + whitelist = {item: item for item in whitelist} + return whitelist + + @default('options_form') + def _default_options_form(self): + if len(self.image_whitelist) <= 1: + # default form only when there are images to choose from + return '' + # form derived from wrapspawner.ProfileSpawner + option_t = '' + options = [ + option_t.format( + image=image, selected='selected' if image == self.image else '' + ) + for image in self.image_whitelist + ] + return """ + + + """.format( + options=options + ) + + def options_from_form(self, formdata): + """Turn options formdata into user_options""" + options = {} + print(formdata) + if 'image' in formdata: + options['image'] = formdata['image'][0] + return options + container_prefix = Unicode(config=True, help="DEPRECATED in 0.10. Use prefix") container_name_template = Unicode( @@ -185,7 +250,7 @@ def _container_image_changed(self, change): @observe("container_name_template", "container_prefix") def _deprecate_container_alias(self, change): - new_name = change.name[len("container_"):] + new_name = change.name[len("container_") :] setattr(self, new_name, change.new) prefix = Unicode( @@ -259,9 +324,7 @@ def _deprecate_container_alias(self, change): Reusable implementations should go in dockerspawner.VolumeNamingStrategy, tests should go in ... """ - ).tag( - config=True - ) + ).tag(config=True) def default_format_volume_name(template, spawner): return template.format(username=spawner.user.name) @@ -599,8 +662,27 @@ def remove_object(self): @gen.coroutine def create_object(self): """Create the container/service object""" + # image priority: + # 1. explicit argument + # (this never happens when DockerSpawner is used directly, + # but can be used by subclasses) + # 2. user options (from spawn options form) + # 3. self.image from config + image = self.user_options.get('image') or self.image + if self.image_whitelist: + if image not in self.image_whitelist: + raise web.HTTPError( + 400, + "Image %s not in whitelist: %s" + % (image, ', '.join(self.image_whitelist)), + ) + # resolve image alias to actual image name + image = self.image_whitelist[image] + # save choice in self.image + self.image = image + create_kwargs = dict( - image=self.image, + image=image, environment=self.get_env(), volumes=self.volume_mount_points, name=self.container_name, @@ -791,7 +873,10 @@ def stop(self, now=False): Consider using pause/unpause when docker-py adds support """ self.log.info( - "Stopping %s %s (id: %s)", self.object_type, self.object_name, self.object_id[:7] + "Stopping %s %s (id: %s)", + self.object_type, + self.object_name, + self.object_id[:7], ) yield self.stop_object() From 369d99c37ef9eea1890b9b8532f1b636158dd3de Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 13 Jun 2018 16:28:28 +0200 Subject: [PATCH 2/5] allow image_whitelist to be a callable So that it can be derived from the user callable will be passed the Spawner as a single argument. User is accessible as Spawner.user. --- dockerspawner/dockerspawner.py | 71 +++++++++++++++++++++++----------- 1 file changed, 48 insertions(+), 23 deletions(-) diff --git a/dockerspawner/dockerspawner.py b/dockerspawner/dockerspawner.py index e02baadf..1af29277 100644 --- a/dockerspawner/dockerspawner.py +++ b/dockerspawner/dockerspawner.py @@ -190,13 +190,23 @@ def _container_image_changed(self, change): ) image_whitelist = Union( - [Dict(), List()], + [Any(), Dict(), List()], config=True, help=""" List or dict of images that users can run. If specified, users will be presented with a form from which they can select an image to run. + + If a dictionary, the keys will be the options presented to users + and the values the actual images that will be launched. + + If a list, will be cast to a dictionary where keys and values are the same + (i.e. a shortcut for presenting the actual images directly to users). + + If a callable, will be called with the Spawner instance as its only argument. + The user is accessible as spawner.user. + The callable should return a dict or list as above. """, ) @@ -208,13 +218,27 @@ def _image_whitelist_dict(self, proposal): dict where the keys and values are the same. """ whitelist = proposal.value - if not isinstance(whitelist, dict): + if isinstance(whitelist, list): whitelist = {item: item for item in whitelist} return whitelist + def _get_image_whitelist(self): + """Evaluate image_whitelist callable + + Or return the whitelist as-is if it's already a dict + """ + if callable(self.image_whitelist): + whitelist = self.image_whitelist(self) + if not isinstance(whitelist, dict): + # always return a dict + whitelist = {item: item for item in whitelist} + return whitelist + return self.image_whitelist + @default('options_form') def _default_options_form(self): - if len(self.image_whitelist) <= 1: + image_whitelist = self._get_image_whitelist() + if len(image_whitelist) <= 1: # default form only when there are images to choose from return '' # form derived from wrapspawner.ProfileSpawner @@ -223,7 +247,7 @@ def _default_options_form(self): option_t.format( image=image, selected='selected' if image == self.image else '' ) - for image in self.image_whitelist + for image in image_whitelist ] return """ @@ -237,7 +261,6 @@ def _default_options_form(self): def options_from_form(self, formdata): """Turn options formdata into user_options""" options = {} - print(formdata) if 'image' in formdata: options['image'] = formdata['image'][0] return options @@ -659,30 +682,32 @@ def remove_object(self): # remove the container, as well as any associated volumes yield self.docker("remove_" + self.object_type, self.object_id, v=True) + @gen.coroutine + def check_image_whitelist(self, image): + image_whitelist = self._get_image_whitelist() + if not image_whitelist: + return image + if image not in image_whitelist: + raise web.HTTPError( + 400, + "Image %s not in whitelist: %s" % (image, ', '.join(image_whitelist)), + ) + # resolve image alias to actual image name + return image_whitelist[image] + @gen.coroutine def create_object(self): """Create the container/service object""" # image priority: - # 1. explicit argument - # (this never happens when DockerSpawner is used directly, - # but can be used by subclasses) - # 2. user options (from spawn options form) - # 3. self.image from config - image = self.user_options.get('image') or self.image - if self.image_whitelist: - if image not in self.image_whitelist: - raise web.HTTPError( - 400, - "Image %s not in whitelist: %s" - % (image, ', '.join(self.image_whitelist)), - ) - # resolve image alias to actual image name - image = self.image_whitelist[image] - # save choice in self.image - self.image = image + # 1. user options (from spawn options form) + # 2. self.image from config + image_option = self.user_options.get('image') + if image_option: + # save choice in self.image + self.image = yield self.check_image_whitelist(image_option) create_kwargs = dict( - image=image, + image=self.image, environment=self.get_env(), volumes=self.volume_mount_points, name=self.container_name, From f57ef5baf6823fcf971e37f80c4b46cf84697a46 Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 13 Jun 2018 16:28:36 +0200 Subject: [PATCH 3/5] test image whitelist --- .travis.yml | 3 ++- tests/test_dockerspawner.py | 38 +++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 976ed0c6..47c3b850 100644 --- a/.travis.yml +++ b/.travis.yml @@ -20,5 +20,6 @@ install: - pip freeze script: - pyflakes dockerspawner - - docker pull jupyterhub/singleuser:$JUPYTERHUB + - docker pull jupyterhub/singleuser:0.8 + - docker pull jupyterhub/singleuser:0.9 - travis_retry py.test --cov dockerspawner tests -v diff --git a/tests/test_dockerspawner.py b/tests/test_dockerspawner.py index eb9795f8..30b7502c 100644 --- a/tests/test_dockerspawner.py +++ b/tests/test_dockerspawner.py @@ -1,6 +1,9 @@ """Tests for DockerSpawner class""" +import json + import pytest + from jupyterhub.tests.test_api import add_user, api_request from jupyterhub.tests.mocking import public_url from jupyterhub.tests.utils import async_requests @@ -30,3 +33,38 @@ def test_start_stop(app): r.raise_for_status() print(r.text) assert "kernels" in r.json() + + +@pytest.mark.gen_test +@pytest.mark.parametrize("image", ["0.8", "0.9", "nomatch"]) +def test_image_whitelist(app, image): + name = "checker" + add_user(app.db, app, name=name) + user = app.users[name] + assert isinstance(user.spawner, DockerSpawner) + user.spawner.remove_containers = True + user.spawner.image_whitelist = { + "0.9": "jupyterhub/singleuser:0.9", + "0.8": "jupyterhub/singleuser:0.8", + } + token = user.new_api_token() + # start the server + r = yield api_request( + app, "users", name, "server", method="post", data=json.dumps({"image": image}) + ) + if image not in user.spawner.image_whitelist: + with pytest.raises(Exception): + r.raise_for_status() + return + while r.status_code == 202: + # request again + r = yield api_request(app, "users", name, "server", method="post") + assert r.status_code == 201, r.text + url = url_path_join(public_url(app, user), "api/status") + r = yield async_requests.get(url, headers={"Authorization": "token %s" % token}) + r.raise_for_status() + assert r.headers['x-jupyterhub-version'].startswith(image) + r = yield api_request( + app, "users", name, "server", method="delete", + ) + r.raise_for_status() From 25273592b97edfdb6bafcb374f8db0fd520af732 Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 13 Jun 2018 16:28:49 +0200 Subject: [PATCH 4/5] don't cleanup services if they aren't available --- tests/conftest.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 99aef951..5282ab53 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3,6 +3,7 @@ from unittest import mock from docker import from_env as docker_from_env +from docker.errors import APIError import pytest from jupyterhub.tests.mocking import MockHub @@ -40,7 +41,12 @@ def docker(): if c.name.startswith("dockerspawner-test"): c.stop() c.remove() - - for c in d.services.list(): - if c.name.startswith("dockerspawner-test"): - c.remove() + try: + services = d.services.list() + except APIError: + # e.g. services not available + return + else: + for s in services: + if s.name.startswith("dockerspawner-test"): + s.remove() From b6cdf7a8e10e4062a888a58b20086de262699e94 Mon Sep 17 00:00:00 2001 From: Min RK Date: Wed, 13 Jun 2018 16:29:15 +0200 Subject: [PATCH 5/5] gitignore pytest cache --- .gitignore | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 73fc857e..7d132f67 100644 --- a/.gitignore +++ b/.gitignore @@ -38,6 +38,7 @@ htmlcov/ .tox/ .coverage .cache +.pytest_cache nosetests.xml coverage.xml @@ -63,4 +64,4 @@ jupyterhub.sqlite src # Pycharm -.idea \ No newline at end of file +.idea