From 3bda65baafbd0a9ea7618557c4180c405955d447 Mon Sep 17 00:00:00 2001 From: Stefano Borini Date: Wed, 29 Mar 2017 18:39:12 +0100 Subject: [PATCH 01/10] Fix for loss of registration of reverse proxying --- jupyterhub/jupyterhub_config.py | 1 + package.json | 2 +- remoteappmanager/application.py | 11 +++++-- remoteappmanager/environment_config.py | 2 +- remoteappmanager/handlers/api.py | 5 ++-- .../handlers/reregister_containers_handler.py | 30 +++++++++++++++++++ 6 files changed, 44 insertions(+), 7 deletions(-) create mode 100644 remoteappmanager/handlers/reregister_containers_handler.py diff --git a/jupyterhub/jupyterhub_config.py b/jupyterhub/jupyterhub_config.py index e488489c8..e17572162 100644 --- a/jupyterhub/jupyterhub_config.py +++ b/jupyterhub/jupyterhub_config.py @@ -8,6 +8,7 @@ c.JupyterHub.ssl_cert = 'test.crt' c.JupyterHub.hub_ip = public_ips()[0] +c.JupyterHub.proxy_auth_token = "hello" # Choose between system-user mode and virtual-user mode setting_mode = ('system_user', 'virtual_user')[1] 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..f94c01a7a 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, ReregisterContainersHandler) +from remoteappmanager.utils import url_path_join, with_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 [ + (with_end_slash( + url_path_join(base_urlpath, "containers", "(.*)") + ), ReregisterContainersHandler), (base_urlpath, HomeHandler), - (base_urlpath.rstrip('/'), - web.RedirectHandler, {"url": base_urlpath}), + (base_urlpath.rstrip('/'), web.RedirectHandler, { + "url": base_urlpath}), ] diff --git a/remoteappmanager/environment_config.py b/remoteappmanager/environment_config.py index c59063929..26cd382d8 100644 --- a/remoteappmanager/environment_config.py +++ b/remoteappmanager/environment_config.py @@ -20,8 +20,8 @@ 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, os.environ.get(envname, "")) + print(self.proxy_api_token) diff --git a/remoteappmanager/handlers/api.py b/remoteappmanager/handlers/api.py index cbaa9da65..c4fe7c6c9 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 .reregister_containers_handler import ReregisterContainersHandler # 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/reregister_containers_handler.py b/remoteappmanager/handlers/reregister_containers_handler.py new file mode 100644 index 000000000..780d1e1d3 --- /dev/null +++ b/remoteappmanager/handlers/reregister_containers_handler.py @@ -0,0 +1,30 @@ +from tornado import gen, web + +from remoteappmanager.handlers.base_handler import BaseHandler + + +class ReregisterContainersHandler(BaseHandler): + """This handler intercepts calls that filter through the reverse + proxy due e.g. to crash of the reverse proxy. + + It will extract the url_id from the request, lookup if the appropriate + container is actually running, and re-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.container_from_url_id( + url_id) + + if container is not None: + yield self.application.reverse_proxy.register( + container.urlpath, + container.host_url) + + self.redirect(self.request.uri) + + raise web.HTTPError(404) From 8a04d357d3051412b5284e69661d139df8cc77d2 Mon Sep 17 00:00:00 2001 From: Stefano Borini Date: Fri, 31 Mar 2017 11:53:49 +0100 Subject: [PATCH 02/10] Implementation using redirection. --- remoteappmanager/application.py | 10 +-- remoteappmanager/docker/container_manager.py | 76 +++++++++++++++---- remoteappmanager/handlers/api.py | 2 +- .../handlers/register_container_handler.py | 38 ++++++++++ .../handlers/reregister_containers_handler.py | 30 -------- remoteappmanager/utils.py | 11 +++ 6 files changed, 116 insertions(+), 51 deletions(-) create mode 100644 remoteappmanager/handlers/register_container_handler.py delete mode 100644 remoteappmanager/handlers/reregister_containers_handler.py diff --git a/remoteappmanager/application.py b/remoteappmanager/application.py index f94c01a7a..0e520ee01 100644 --- a/remoteappmanager/application.py +++ b/remoteappmanager/application.py @@ -2,8 +2,8 @@ from remoteappmanager.base_application import BaseApplication from remoteappmanager.handlers.api import ( - HomeHandler, ReregisterContainersHandler) -from remoteappmanager.utils import url_path_join, with_end_slash + HomeHandler, RegisterContainerHandler) +from remoteappmanager.utils import url_path_join, without_end_slash from remoteappmanager import webapi @@ -17,9 +17,9 @@ def _webapi_resources(self): def _web_handlers(self): base_urlpath = self.command_line_config.base_urlpath return [ - (with_end_slash( - url_path_join(base_urlpath, "containers", "(.*)") - ), ReregisterContainersHandler), + (without_end_slash( + url_path_join(base_urlpath, "containers", "([a-z0-9]*)") + )+"/?", RegisterContainerHandler), (base_urlpath, HomeHandler), (base_urlpath.rstrip('/'), web.RedirectHandler, { "url": base_urlpath}), diff --git a/remoteappmanager/docker/container_manager.py b/remoteappmanager/docker/container_manager.py index 95bb7c66c..aee9839e6 100644 --- a/remoteappmanager/docker/container_manager.py +++ b/remoteappmanager/docker/container_manager.py @@ -13,7 +13,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 ( @@ -37,6 +38,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 @@ -162,6 +168,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. @@ -177,30 +184,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 @@ -252,6 +251,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/handlers/api.py b/remoteappmanager/handlers/api.py index c4fe7c6c9..5a71a38f9 100644 --- a/remoteappmanager/handlers/api.py +++ b/remoteappmanager/handlers/api.py @@ -1,6 +1,6 @@ from .home_handler import HomeHandler # noqa from .base_handler import BaseHandler # noqa -from .reregister_containers_handler import ReregisterContainersHandler # 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 diff --git a/remoteappmanager/handlers/register_container_handler.py b/remoteappmanager/handlers/register_container_handler.py new file mode 100644 index 000000000..9d8692379 --- /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)) + + self.redirect(self.request.uri) + + raise web.HTTPError(404) diff --git a/remoteappmanager/handlers/reregister_containers_handler.py b/remoteappmanager/handlers/reregister_containers_handler.py deleted file mode 100644 index 780d1e1d3..000000000 --- a/remoteappmanager/handlers/reregister_containers_handler.py +++ /dev/null @@ -1,30 +0,0 @@ -from tornado import gen, web - -from remoteappmanager.handlers.base_handler import BaseHandler - - -class ReregisterContainersHandler(BaseHandler): - """This handler intercepts calls that filter through the reverse - proxy due e.g. to crash of the reverse proxy. - - It will extract the url_id from the request, lookup if the appropriate - container is actually running, and re-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.container_from_url_id( - url_id) - - if container is not None: - yield self.application.reverse_proxy.register( - container.urlpath, - container.host_url) - - self.redirect(self.request.uri) - - raise web.HTTPError(404) diff --git a/remoteappmanager/utils.py b/remoteappmanager/utils.py index dd3058e3d..02aafbffb 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,12 @@ def remove_quotes(s): return s[1:-1] return s + + +def deprecated(func): + @wraps(func) + def _deprecated(*args, **kwargs): + warnings.warn("Deprecation warning: {}".format(func.__name__)) + return func(*args, **kwargs) + + return _deprecated From 5c00ddf8f8fc42c0a943e0a4e085e37ce3a21905 Mon Sep 17 00:00:00 2001 From: Stefano Borini Date: Fri, 31 Mar 2017 11:58:58 +0100 Subject: [PATCH 03/10] Cleanup --- jupyterhub/jupyterhub_config.py | 1 - remoteappmanager/environment_config.py | 1 - remoteappmanager/utils.py | 1 + 3 files changed, 1 insertion(+), 2 deletions(-) diff --git a/jupyterhub/jupyterhub_config.py b/jupyterhub/jupyterhub_config.py index e17572162..e488489c8 100644 --- a/jupyterhub/jupyterhub_config.py +++ b/jupyterhub/jupyterhub_config.py @@ -8,7 +8,6 @@ c.JupyterHub.ssl_cert = 'test.crt' c.JupyterHub.hub_ip = public_ips()[0] -c.JupyterHub.proxy_auth_token = "hello" # Choose between system-user mode and virtual-user mode setting_mode = ('system_user', 'virtual_user')[1] diff --git a/remoteappmanager/environment_config.py b/remoteappmanager/environment_config.py index 26cd382d8..3cf49513e 100644 --- a/remoteappmanager/environment_config.py +++ b/remoteappmanager/environment_config.py @@ -24,4 +24,3 @@ def parse_config(self): envname = traitlet_name.upper() setattr(self, traitlet_name, os.environ.get(envname, "")) - print(self.proxy_api_token) diff --git a/remoteappmanager/utils.py b/remoteappmanager/utils.py index 02aafbffb..4d7d83215 100644 --- a/remoteappmanager/utils.py +++ b/remoteappmanager/utils.py @@ -104,6 +104,7 @@ def remove_quotes(s): def deprecated(func): + """Decorator. Marks a function/method as deprecated.""" @wraps(func) def _deprecated(*args, **kwargs): warnings.warn("Deprecation warning: {}".format(func.__name__)) From 35722200b42746e3ca0ba89d80c670ef2486117f Mon Sep 17 00:00:00 2001 From: Stefano Borini Date: Fri, 31 Mar 2017 13:28:19 +0100 Subject: [PATCH 04/10] Added tests --- remoteappmanager/application.py | 2 +- .../handlers/tests/test_home_handler.py | 36 -------------- .../tests/test_register_container_handler.py | 47 +++++++++++++++++++ .../tests/mocking/virtual/docker_client.py | 7 +-- 4 files changed, 52 insertions(+), 40 deletions(-) delete mode 100644 remoteappmanager/handlers/tests/test_home_handler.py create mode 100644 remoteappmanager/handlers/tests/test_register_container_handler.py diff --git a/remoteappmanager/application.py b/remoteappmanager/application.py index 0e520ee01..50bc36ab6 100644 --- a/remoteappmanager/application.py +++ b/remoteappmanager/application.py @@ -18,7 +18,7 @@ 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]*)") + url_path_join(base_urlpath, "containers", "([a-z0-9_]*)") )+"/?", RegisterContainerHandler), (base_urlpath, HomeHandler), (base_urlpath.rstrip('/'), web.RedirectHandler, { diff --git a/remoteappmanager/handlers/tests/test_home_handler.py b/remoteappmanager/handlers/tests/test_home_handler.py deleted file mode 100644 index c9f65bbd9..000000000 --- a/remoteappmanager/handlers/tests/test_home_handler.py +++ /dev/null @@ -1,36 +0,0 @@ -from remoteappmanager.tests import utils -from remoteappmanager.tests.mocking import dummy -from remoteappmanager.tests.temp_mixin import TempMixin - - -class TestHomeHandler(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_home(self): - res = self.fetch("/user/username/", - headers={ - "Cookie": "jupyter-hub-token-username=foo" - } - ) - - self.assertEqual(res.code, 200) - self.assertIn("applist", str(res.body)) - - def test_failed_auth(self): - self._app.hub.verify_token.return_value = {} - res = self.fetch("/user/username/", - headers={ - "Cookie": "jupyter-hub-token-username=foo" - } - ) - - self.assertGreaterEqual(res.code, 400) - self.assertIn(self._app.file_config.login_url, res.effective_url) - self.assertNotIn("applist", str(res.body)) 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..a907ac4ba --- /dev/null +++ b/remoteappmanager/handlers/tests/test_register_container_handler.py @@ -0,0 +1,47 @@ +from remoteappmanager.tests import utils +from remoteappmanager.tests.mocking import dummy +from remoteappmanager.tests.temp_mixin import TempMixin + + +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) diff --git a/remoteappmanager/tests/mocking/virtual/docker_client.py b/remoteappmanager/tests/mocking/virtual/docker_client.py index 021a04181..af8f78599 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.realm: 'remoteexec', 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)) @@ -105,6 +105,7 @@ def containers(**kwargs): if 'filters' in kwargs and 'label' in kwargs['filters']: label_filters = kwargs['filters']['label'] + print(label_filters) if not isinstance(label_filters, (list, tuple)): label_filters = [label_filters] From 4386fa1abd331ab5ffe626ce0c26dcb70c272eab Mon Sep 17 00:00:00 2001 From: Stefano Borini Date: Fri, 31 Mar 2017 13:29:01 +0100 Subject: [PATCH 05/10] Removed print statement --- remoteappmanager/tests/mocking/virtual/docker_client.py | 1 - 1 file changed, 1 deletion(-) diff --git a/remoteappmanager/tests/mocking/virtual/docker_client.py b/remoteappmanager/tests/mocking/virtual/docker_client.py index af8f78599..c8c41cd0b 100644 --- a/remoteappmanager/tests/mocking/virtual/docker_client.py +++ b/remoteappmanager/tests/mocking/virtual/docker_client.py @@ -105,7 +105,6 @@ def containers(**kwargs): if 'filters' in kwargs and 'label' in kwargs['filters']: label_filters = kwargs['filters']['label'] - print(label_filters) if not isinstance(label_filters, (list, tuple)): label_filters = [label_filters] From c15a0c9b8630c16a1b44680382b9eb9d7a5ad605 Mon Sep 17 00:00:00 2001 From: Stefano Borini Date: Fri, 31 Mar 2017 13:52:19 +0100 Subject: [PATCH 06/10] Restored realm --- remoteappmanager/tests/mocking/virtual/docker_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/remoteappmanager/tests/mocking/virtual/docker_client.py b/remoteappmanager/tests/mocking/virtual/docker_client.py index c8c41cd0b..463c4b91c 100644 --- a/remoteappmanager/tests/mocking/virtual/docker_client.py +++ b/remoteappmanager/tests/mocking/virtual/docker_client.py @@ -77,7 +77,7 @@ def get_fake_container_labels(num=3): 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: 'remoteexec', + SIMPHONY_NS_RUNINFO.realm: 'myrealm', SIMPHONY_NS_RUNINFO.urlpath: '/user/username/containers/url_id'}, # noqa {SIMPHONY_NS_RUNINFO.user: 'username'}, {})) From 4ee6f5bc68812869eee0ccff2e95884f9f509e72 Mon Sep 17 00:00:00 2001 From: Stefano Borini Date: Fri, 31 Mar 2017 13:57:51 +0100 Subject: [PATCH 07/10] Coverage --- remoteappmanager/docker/tests/test_container.py | 4 ++-- remoteappmanager/docker/tests/test_container_manager.py | 6 +++--- remoteappmanager/tests/mocking/dummy.py | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) 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 343656a24..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, 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() From ad4de747ed985a12c0f40c6f5c2a283cf997b8e3 Mon Sep 17 00:00:00 2001 From: Stefano Borini Date: Fri, 31 Mar 2017 14:15:49 +0100 Subject: [PATCH 08/10] Fixing test after merge --- remoteappmanager/docker/tests/test_container_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/remoteappmanager/docker/tests/test_container_manager.py b/remoteappmanager/docker/tests/test_container_manager.py index f4f521c77..bc4060065 100644 --- a/remoteappmanager/docker/tests/test_container_manager.py +++ b/remoteappmanager/docker/tests/test_container_manager.py @@ -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", From 4e91486548fa62720cb1237fbccaeabc1f87131e Mon Sep 17 00:00:00 2001 From: Stefano Borini Date: Fri, 31 Mar 2017 15:50:57 +0100 Subject: [PATCH 09/10] Coverage --- .../handlers/register_container_handler.py | 4 ++-- .../tests/test_register_container_handler.py | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/remoteappmanager/handlers/register_container_handler.py b/remoteappmanager/handlers/register_container_handler.py index 9d8692379..9dcd24ad1 100644 --- a/remoteappmanager/handlers/register_container_handler.py +++ b/remoteappmanager/handlers/register_container_handler.py @@ -32,7 +32,7 @@ def get(self, url_id): "Could not register reverse " "proxy for id {} in RegisterContainerHandler".format( url_id)) - - self.redirect(self.request.uri) + 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 index a907ac4ba..1eb92730b 100644 --- a/remoteappmanager/handlers/tests/test_register_container_handler.py +++ b/remoteappmanager/handlers/tests/test_register_container_handler.py @@ -1,6 +1,7 @@ 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): @@ -45,3 +46,16 @@ def test_failed_auth(self): # 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) From e89486c781f9b38422fbbd66de84d3aa02b829fb Mon Sep 17 00:00:00 2001 From: Stefano Borini Date: Mon, 3 Apr 2017 17:38:45 +0100 Subject: [PATCH 10/10] Restored missing test --- .../handlers/tests/test_home_handler.py | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 remoteappmanager/handlers/tests/test_home_handler.py diff --git a/remoteappmanager/handlers/tests/test_home_handler.py b/remoteappmanager/handlers/tests/test_home_handler.py new file mode 100644 index 000000000..c9f65bbd9 --- /dev/null +++ b/remoteappmanager/handlers/tests/test_home_handler.py @@ -0,0 +1,36 @@ +from remoteappmanager.tests import utils +from remoteappmanager.tests.mocking import dummy +from remoteappmanager.tests.temp_mixin import TempMixin + + +class TestHomeHandler(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_home(self): + res = self.fetch("/user/username/", + headers={ + "Cookie": "jupyter-hub-token-username=foo" + } + ) + + self.assertEqual(res.code, 200) + self.assertIn("applist", str(res.body)) + + def test_failed_auth(self): + self._app.hub.verify_token.return_value = {} + res = self.fetch("/user/username/", + headers={ + "Cookie": "jupyter-hub-token-username=foo" + } + ) + + self.assertGreaterEqual(res.code, 400) + self.assertIn(self._app.file_config.login_url, res.effective_url) + self.assertNotIn("applist", str(res.body))