diff --git a/package.json b/package.json index ec892c057..906e47789 100644 --- a/package.json +++ b/package.json @@ -10,7 +10,7 @@ }, "dependencies": { "bower": "*", - "configurable-http-proxy": "git://github.com/jupyterhub/configurable-http-proxy.git#f54c6a46a235f17cb6c36046a913d37fa45ec95b" + "configurable-http-proxy": "git://github.com/jupyterhub/configurable-http-proxy.git#e52764b9e742904c0982739b1479f3bf88dad3f4" }, "devDependencies": { "jshint": "*", diff --git a/remoteappmanager/application.py b/remoteappmanager/application.py index 8ffe1225f..50bc36ab6 100644 --- a/remoteappmanager/application.py +++ b/remoteappmanager/application.py @@ -1,7 +1,9 @@ from tornado import web from remoteappmanager.base_application import BaseApplication -from remoteappmanager.handlers.api import HomeHandler +from remoteappmanager.handlers.api import ( + HomeHandler, RegisterContainerHandler) +from remoteappmanager.utils import url_path_join, without_end_slash from remoteappmanager import webapi @@ -15,7 +17,10 @@ def _webapi_resources(self): def _web_handlers(self): base_urlpath = self.command_line_config.base_urlpath return [ + (without_end_slash( + url_path_join(base_urlpath, "containers", "([a-z0-9_]*)") + )+"/?", RegisterContainerHandler), (base_urlpath, HomeHandler), - (base_urlpath.rstrip('/'), - web.RedirectHandler, {"url": base_urlpath}), + (base_urlpath.rstrip('/'), web.RedirectHandler, { + "url": base_urlpath}), ] diff --git a/remoteappmanager/docker/container_manager.py b/remoteappmanager/docker/container_manager.py index 87a358f44..3885ef0a9 100644 --- a/remoteappmanager/docker/container_manager.py +++ b/remoteappmanager/docker/container_manager.py @@ -14,7 +14,8 @@ from remoteappmanager.logging.logging_mixin import LoggingMixin from remoteappmanager.utils import ( url_path_join, - without_end_slash) + without_end_slash, + deprecated) from tornado import gen from traitlets import ( @@ -38,6 +39,11 @@ class OperationInProgress(Exception): pass +class MultipleResultsFound(Exception): + """Raised when we are asking for a specific container, but more than + one result is found.""" + + class ContainerManager(LoggingMixin): #: The container (not host) port. We decided it's 8888 by default. It will @@ -163,6 +169,7 @@ def stop_and_remove_container(self, container_id): self._stop_pending.remove(container_id) @gen.coroutine + @deprecated def containers_from_mapping_id(self, user_name, mapping_id): """Returns the currently running containers for a given user and mapping_id. @@ -178,30 +185,22 @@ def containers_from_mapping_id(self, user_name, mapping_id): ------ A list of Container objects, or an empty list if nothing is found. """ - containers = yield self.containers_with_labels({ - SIMPHONY_NS_RUNINFO.user: user_name, - SIMPHONY_NS_RUNINFO.mapping_id: mapping_id, - SIMPHONY_NS_RUNINFO.realm: self.realm - }) - return containers + return (yield self.find_containers(user_name=user_name, + mapping_id=mapping_id)) @gen.coroutine + @deprecated def container_from_url_id(self, url_id): """Retrieves and returns the container by its url_id, if present. If not present, returns None. """ - containers = yield self.containers_with_labels({ - SIMPHONY_NS_RUNINFO.url_id: url_id, - SIMPHONY_NS_RUNINFO.realm: self.realm - }) - return containers[0] if len(containers) else None + return (yield self.find_container(url_id=url_id)) @gen.coroutine + @deprecated def running_containers(self): """Returns all the running containers""" - containers = yield self.containers_with_labels({ - SIMPHONY_NS_RUNINFO.realm: self.realm - }) + containers = yield self.find_containers() return containers @@ -253,6 +252,53 @@ def containers_from_filters(self, filters): return containers + @gen.coroutine + def find_containers(self, + *, + url_id=None, + mapping_id=None, + user_name=None): + """Finds and returns containers matching all the specified arguments. + """ + labels = { + SIMPHONY_NS_RUNINFO.realm: self.realm + } + + if url_id is not None: + labels[SIMPHONY_NS_RUNINFO.url_id] = url_id + + if mapping_id is not None: + labels[SIMPHONY_NS_RUNINFO.mapping_id] = mapping_id + + if user_name is not None: + labels[SIMPHONY_NS_RUNINFO.user] = user_name + + containers = yield self.containers_with_labels(labels) + + return containers + + @gen.coroutine + def find_container(self, + *, + url_id=None, + mapping_id=None, + user_name=None): + """Find and returns a container matching the specified + arguments. + + Returns the found container or None. If multiple containers + match the query, it will raise MultipleResultsFound""" + containers = yield self.find_containers(url_id=url_id, + mapping_id=mapping_id, + user_name=user_name) + if len(containers) > 1: + raise MultipleResultsFound() + + if len(containers) == 1: + return containers[0] + + return None + @gen.coroutine def image(self, image_id_or_name): """Returns the Image object associated to a given id diff --git a/remoteappmanager/docker/tests/test_container.py b/remoteappmanager/docker/tests/test_container.py index 8db057b2e..43ef69d06 100644 --- a/remoteappmanager/docker/tests/test_container.py +++ b/remoteappmanager/docker/tests/test_container.py @@ -92,7 +92,7 @@ def test_from_docker_dict_without_public_port(self): name='/container_name1', image_name='image_name1', image_id='image_id1', - user="user_name", + user="username", ip='0.0.0.0', port=80, url_id="url_id", @@ -113,7 +113,7 @@ def test_from_docker_dict_inspect_container(self): name='/myrealm-username-mapping_5Fid', image_name='image_name1', image_id='image_id1', - user="user_name", + user="username", ip='0.0.0.0', port=666, url_id="url_id", diff --git a/remoteappmanager/docker/tests/test_container_manager.py b/remoteappmanager/docker/tests/test_container_manager.py index 732395fe7..bc4060065 100644 --- a/remoteappmanager/docker/tests/test_container_manager.py +++ b/remoteappmanager/docker/tests/test_container_manager.py @@ -58,13 +58,13 @@ def test_start_stop(self): @gen_test def test_containers_from_mapping_id(self): """ Test containers_for_mapping_id returns a list of Container """ - result = yield self.manager.containers_from_mapping_id("user_name", + result = yield self.manager.containers_from_mapping_id("username", "mapping_id") expected = Container(docker_id='container_id1', mapping_id="mapping_id", name='/myrealm-username-mapping_5Fid', image_name='image_name1', # noqa - user="user_name", + user="username", image_id='image_id1', ip='127.0.0.1', port=666, @@ -83,7 +83,7 @@ def test_containers_from_url_id(self): mapping_id="mapping_id", name='/myrealm-username-mapping_5Fid', image_name='image_name1', # noqa - user="user_name", + user="username", image_id='image_id1', ip='127.0.0.1', port=666, @@ -162,7 +162,7 @@ def test_start_already_present_container(self): mock_client = self.mock_docker_client result = yield self.manager.start_container( - "user_name", + "username", "image_name1", "mapping_id", "/base/url", diff --git a/remoteappmanager/environment_config.py b/remoteappmanager/environment_config.py index c59063929..3cf49513e 100644 --- a/remoteappmanager/environment_config.py +++ b/remoteappmanager/environment_config.py @@ -20,7 +20,6 @@ def parse_config(self): """Parses the environment variables, and assign their values to our local traits. """ - for traitlet_name in self.traits().keys(): envname = traitlet_name.upper() setattr(self, traitlet_name, diff --git a/remoteappmanager/handlers/api.py b/remoteappmanager/handlers/api.py index cbaa9da65..5a71a38f9 100644 --- a/remoteappmanager/handlers/api.py +++ b/remoteappmanager/handlers/api.py @@ -1,7 +1,8 @@ from .home_handler import HomeHandler # noqa from .base_handler import BaseHandler # noqa +from .register_container_handler import RegisterContainerHandler # noqa from .admin.admin_home_handler import AdminHomeHandler # noqa -from .admin.containers_handler import ContainersHandler # noqa -from .admin.users_handler import UsersHandler # noqa +from .admin.containers_handler import ContainersHandler # noqa +from .admin.users_handler import UsersHandler # noqa from .admin.applications_handler import ApplicationsHandler # noqa from .admin.accounting_handler import AccountingHandler # noqa diff --git a/remoteappmanager/handlers/register_container_handler.py b/remoteappmanager/handlers/register_container_handler.py new file mode 100644 index 000000000..9dcd24ad1 --- /dev/null +++ b/remoteappmanager/handlers/register_container_handler.py @@ -0,0 +1,38 @@ +from tornado import gen, web + +from remoteappmanager.handlers.base_handler import BaseHandler + + +class RegisterContainerHandler(BaseHandler): + """This handler intercepts calls that filter through the reverse + proxy and sets up the redirection if there's a receiving container + accepting the connection. + + It will extract the url_id from the request, lookup if the appropriate + container is actually running, and seed the reverse proxy with the + appropriate data. + """ + + @web.authenticated + @gen.coroutine + def get(self, url_id): + container_manager = self.application.container_manager + + container = yield container_manager.find_container( + user_name=self.current_user.name, + url_id=url_id) + + if container is not None: + try: + yield self.application.reverse_proxy.register( + container.urlpath, + container.host_url) + except Exception: + self.log.exception( + "Could not register reverse " + "proxy for id {} in RegisterContainerHandler".format( + url_id)) + else: + self.redirect(self.request.uri) + + raise web.HTTPError(404) diff --git a/remoteappmanager/handlers/tests/test_register_container_handler.py b/remoteappmanager/handlers/tests/test_register_container_handler.py new file mode 100644 index 000000000..1eb92730b --- /dev/null +++ b/remoteappmanager/handlers/tests/test_register_container_handler.py @@ -0,0 +1,61 @@ +from remoteappmanager.tests import utils +from remoteappmanager.tests.mocking import dummy +from remoteappmanager.tests.temp_mixin import TempMixin +from remoteappmanager.tests.utils import mock_coro_factory + + +class TestRegisterContainerHandler(TempMixin, utils.AsyncHTTPTestCase): + def get_app(self): + app = dummy.create_application() + app.hub.verify_token.return_value = { + 'pending': None, + 'name': app.settings['user'], + 'admin': False, + 'server': app.settings['base_urlpath']} + return app + + def test_absent_url_id(self): + res = self.fetch("/user/username/containers/99999/", + headers={ + "Cookie": "jupyter-hub-token-username=foo" + } + ) + + self.assertEqual(res.code, 404) + + def test_present_url_id(self): + res = self.fetch("/user/username/containers/url_id", + headers={ + "Cookie": "jupyter-hub-token-username=foo" + }, + follow_redirects=False + ) + + self.assertEqual(res.code, 302) + self.assertTrue(self._app.reverse_proxy.register.called) + + def test_failed_auth(self): + self._app.hub.verify_token.return_value = {} + res = self.fetch("/user/username/containers/url_id", + headers={ + "Cookie": "jupyter-hub-token-username=foo" + }, + follow_redirects=False + ) + + # It will be sent to the login instead + self.assertFalse(self._app.reverse_proxy.register.called) + self.assertEqual(res.code, 302) + + def test_failure_of_reverse_proxy(self): + self._app.reverse_proxy.register = mock_coro_factory( + side_effect=Exception("BOOM")) + + res = self.fetch("/user/username/containers/url_id", + headers={ + "Cookie": "jupyter-hub-token-username=foo" + }, + follow_redirects=False + ) + + self.assertEqual(res.code, 404) diff --git a/remoteappmanager/tests/mocking/dummy.py b/remoteappmanager/tests/mocking/dummy.py index 1c9c6b68f..27bc5b971 100644 --- a/remoteappmanager/tests/mocking/dummy.py +++ b/remoteappmanager/tests/mocking/dummy.py @@ -221,7 +221,7 @@ def create_container_manager(params=None): manager : remoteappmanager.docker.container_manager.ContainerManager ''' if params is None: - params = {'docker_config': {}} + params = {'docker_config': {}, "realm": "myrealm"} manager = ContainerManager(**params) manager._docker_client._sync_client = create_docker_client() diff --git a/remoteappmanager/tests/mocking/virtual/docker_client.py b/remoteappmanager/tests/mocking/virtual/docker_client.py index 021a04181..463c4b91c 100644 --- a/remoteappmanager/tests/mocking/virtual/docker_client.py +++ b/remoteappmanager/tests/mocking/virtual/docker_client.py @@ -74,12 +74,12 @@ def get_fake_container_states(num=3): def get_fake_container_labels(num=3): - samples = cycle(({SIMPHONY_NS_RUNINFO.user: 'user_name', + samples = cycle(({SIMPHONY_NS_RUNINFO.user: 'username', SIMPHONY_NS_RUNINFO.mapping_id: 'mapping_id', SIMPHONY_NS_RUNINFO.url_id: 'url_id', SIMPHONY_NS_RUNINFO.realm: 'myrealm', SIMPHONY_NS_RUNINFO.urlpath: '/user/username/containers/url_id'}, # noqa - {SIMPHONY_NS_RUNINFO.user: 'user_name'}, + {SIMPHONY_NS_RUNINFO.user: 'username'}, {})) return tuple(next(samples) for _ in range(num)) diff --git a/remoteappmanager/utils.py b/remoteappmanager/utils.py index dd3058e3d..4d7d83215 100644 --- a/remoteappmanager/utils.py +++ b/remoteappmanager/utils.py @@ -1,4 +1,6 @@ import inspect +from functools import wraps +import warnings def url_path_join(*pieces): @@ -99,3 +101,13 @@ def remove_quotes(s): return s[1:-1] return s + + +def deprecated(func): + """Decorator. Marks a function/method as deprecated.""" + @wraps(func) + def _deprecated(*args, **kwargs): + warnings.warn("Deprecation warning: {}".format(func.__name__)) + return func(*args, **kwargs) + + return _deprecated