From ec76a68421c5f68014811ff5ea72d597e2a9d268 Mon Sep 17 00:00:00 2001 From: Rollin Thomas Date: Wed, 18 May 2022 19:17:44 -0700 Subject: [PATCH 01/12] Move out encoder, ssl, and queue --- jupyterhub_announcement/announcement.py | 139 +----------------------- 1 file changed, 6 insertions(+), 133 deletions(-) diff --git a/jupyterhub_announcement/announcement.py b/jupyterhub_announcement/announcement.py index 7569f22..d599282 100644 --- a/jupyterhub_announcement/announcement.py +++ b/jupyterhub_announcement/announcement.py @@ -1,124 +1,21 @@ import binascii -import datetime import json import os import sys -import aiofiles from html_sanitizer import Sanitizer from jinja2 import ChoiceLoader, Environment, FileSystemLoader, PrefixLoader from jupyterhub._data import DATA_FILES_PATH from jupyterhub.handlers.static import LogoHandler from jupyterhub.services.auth import HubOAuthCallbackHandler, HubOAuthenticated -from jupyterhub.utils import make_ssl_context, url_path_join +from jupyterhub.utils import url_path_join from tornado import escape, gen, ioloop, web -from traitlets import Any, Bool, Dict, Float, Integer, List, Unicode, default -from traitlets.config import Application, Configurable, LoggingConfigurable +from traitlets import Any, Bool, Dict, Integer, List, Unicode, default +from traitlets.config import Application - -class _JSONEncoder(json.JSONEncoder): - def default(self, obj): - if isinstance(obj, datetime.datetime): - return obj.isoformat() - return json.JSONEncoder.default(self, obj) - - -def _datetime_hook(json_dict): - for (key, value) in json_dict.items(): - try: - json_dict[key] = datetime.datetime.fromisoformat(value) - except Exception: - pass - return json_dict - - -class AnnouncementQueue(LoggingConfigurable): - - announcements = List() - - persist_path = Unicode( - "", - help="""File path where announcements persist as JSON. - - For a persistent announcement queue, this parameter must be set to - a non-empty value and correspond to a read+write-accessible path. - The announcement queue is stored as a list of JSON objects. If this - parameter is set to a non-empty value: - - * The persistence file is used to initialize the announcement queue - at start-up. This is the only time the persistence file is read. - * If the persistence file does not exist at start-up, it is - created when an announcement is added to the queue. - * The persistence file is over-written with the contents of the - announcement queue each time a new announcement is added. - - If this parameter is set to an empty value (the default) then the - queue is just empty at initialization and the queue is ephemeral; - announcements will not be persisted on updates to the queue.""", - ).tag(config=True) - - lifetime_days = Float( - 7.0, - help="""Number of days to retain announcements. - - Announcements that have been in the queue for this many days are - purged from the queue.""", - ).tag(config=True) - - def __init__(self, **kwargs): - super().__init__(**kwargs) - - if self.persist_path: - self.log.info(f"restoring queue from {self.persist_path}") - self._handle_restore() - else: - self.log.info("ephemeral queue, persist_path not set") - self.log.info(f"queue has {len(self.announcements)} announcements") - - def _handle_restore(self): - try: - self._restore() - except FileNotFoundError: - self.log.info(f"persist_path not found ({self.persist_path})") - except Exception as err: - self.log.error(f"failed to restore queue ({err})") - - def _restore(self): - with open(self.persist_path) as stream: - self.announcements = json.load(stream, object_hook=_datetime_hook) - - async def update(self, user, announcement=""): - self.announcements.append( - dict( - user=user, announcement=announcement, timestamp=datetime.datetime.now() - ) - ) - if self.persist_path: - self.log.info(f"persisting queue to {self.persist_path}") - await self._handle_persist() - - async def _handle_persist(self): - try: - await self._persist() - except Exception as err: - self.log.error(f"failed to persist queue ({err})") - - async def _persist(self): - async with aiofiles.open(self.persist_path, "w") as stream: - await stream.write( - json.dumps(self.announcements, cls=_JSONEncoder, indent=2) - ) - - async def purge(self): - max_age = datetime.timedelta(days=self.lifetime_days) - now = datetime.datetime.now() - old_count = len(self.announcements) - self.announcements = [ - a for a in self.announcements if now - a["timestamp"] < max_age - ] - if self.persist_path and len(self.announcements) < old_count: - self.log.info(f"persisting queue to {self.persist_path}") - await self._handle_persist() +from jupyterhub_announcement.encoder import _JSONEncoder +from jupyterhub_announcement.queue import AnnouncementQueue +from jupyterhub_announcement.ssl import SSLContext class AnnouncementHandler(HubOAuthenticated, web.RequestHandler): @@ -191,30 +88,6 @@ async def post(self): self.redirect(self.application.reverse_url("view")) -class SSLContext(Configurable): - - keyfile = Unicode( - os.getenv("JUPYTERHUB_SSL_KEYFILE", ""), help="SSL key, use with certfile" - ).tag(config=True) - - certfile = Unicode( - os.getenv("JUPYTERHUB_SSL_CERTFILE", ""), help="SSL cert, use with keyfile" - ).tag(config=True) - - cafile = Unicode( - os.getenv("JUPYTERHUB_SSL_CLIENT_CA", ""), - help="SSL CA, use with keyfile and certfile", - ).tag(config=True) - - def ssl_context(self): - if self.keyfile and self.certfile and self.cafile: - return make_ssl_context( - self.keyfile, self.certfile, cafile=self.cafile, check_hostname=False - ) - else: - return None - - class AnnouncementService(Application): classes = [AnnouncementQueue, SSLContext] From b59b3d3691f69b894be0d75589bb84b93a5d58c0 Mon Sep 17 00:00:00 2001 From: Rollin Thomas Date: Wed, 18 May 2022 19:19:43 -0700 Subject: [PATCH 02/12] Separate encoder, queue, ssl into their own modules --- jupyterhub_announcement/encoder.py | 9 +++ jupyterhub_announcement/queue.py | 109 +++++++++++++++++++++++++++++ jupyterhub_announcement/ssl.py | 29 ++++++++ 3 files changed, 147 insertions(+) create mode 100644 jupyterhub_announcement/encoder.py create mode 100644 jupyterhub_announcement/queue.py create mode 100644 jupyterhub_announcement/ssl.py diff --git a/jupyterhub_announcement/encoder.py b/jupyterhub_announcement/encoder.py new file mode 100644 index 0000000..ec80d37 --- /dev/null +++ b/jupyterhub_announcement/encoder.py @@ -0,0 +1,9 @@ +import datetime +import json + + +class _JSONEncoder(json.JSONEncoder): + def default(self, obj): + if isinstance(obj, datetime.datetime): + return obj.isoformat() + return json.JSONEncoder.default(self, obj) diff --git a/jupyterhub_announcement/queue.py b/jupyterhub_announcement/queue.py new file mode 100644 index 0000000..e407a87 --- /dev/null +++ b/jupyterhub_announcement/queue.py @@ -0,0 +1,109 @@ +import datetime +import json + +import aiofiles +from traitlets import Float, List, Unicode +from traitlets.config import LoggingConfigurable + +from jupyterhub_announcement.encoder import _JSONEncoder + + +def _datetime_hook(json_dict): + for (key, value) in json_dict.items(): + try: + json_dict[key] = datetime.datetime.fromisoformat(value) + except Exception: + pass + return json_dict + + +class AnnouncementQueue(LoggingConfigurable): + + announcements = List() + + persist_path = Unicode( + "", + help="""File path where announcements persist as JSON. + + For a persistent announcement queue, this parameter must be set to + a non-empty value and correspond to a read+write-accessible path. + The announcement queue is stored as a list of JSON objects. If this + parameter is set to a non-empty value: + + * The persistence file is used to initialize the announcement queue + at start-up. This is the only time the persistence file is read. + * If the persistence file does not exist at start-up, it is + created when an announcement is added to the queue. + * The persistence file is over-written with the contents of the + announcement queue each time a new announcement is added. + + If this parameter is set to an empty value (the default) then the + queue is just empty at initialization and the queue is ephemeral; + announcements will not be persisted on updates to the queue.""", + ).tag(config=True) + + lifetime_days = Float( + 7.0, + help="""Number of days to retain announcements. + + Announcements that have been in the queue for this many days are + purged from the queue.""", + ).tag(config=True) + + def __init__(self, **kwargs): + super().__init__(**kwargs) + + if self.persist_path: + self.log.info(f"restoring queue from {self.persist_path}") + self._handle_restore() + else: + self.log.info("ephemeral queue, persist_path not set") + self.log.info(f"queue has {len(self.announcements)} announcements") + + def __len__(self): + return len(self.announcements) + + def _handle_restore(self): + try: + self._restore() + except FileNotFoundError: + self.log.info(f"persist_path not found ({self.persist_path})") + except Exception as err: + self.log.error(f"failed to restore queue ({err})") + + def _restore(self): + with open(self.persist_path) as stream: + self.announcements = json.load(stream, object_hook=_datetime_hook) + + async def update(self, user, announcement=""): + self.announcements.append( + dict( + user=user, announcement=announcement, timestamp=datetime.datetime.now() + ) + ) + if self.persist_path: + self.log.info(f"persisting queue to {self.persist_path}") + await self._handle_persist() + + async def _handle_persist(self): + try: + await self._persist() + except Exception as err: + self.log.error(f"failed to persist queue ({err})") + + async def _persist(self): + async with aiofiles.open(self.persist_path, "w") as stream: + await stream.write( + json.dumps(self.announcements, cls=_JSONEncoder, indent=2) + ) + + async def purge(self): + max_age = datetime.timedelta(days=self.lifetime_days) + now = datetime.datetime.now() + old_count = len(self.announcements) + self.announcements = [ + a for a in self.announcements if now - a["timestamp"] < max_age + ] + if self.persist_path and len(self.announcements) < old_count: + self.log.info(f"persisting queue to {self.persist_path}") + await self._handle_persist() diff --git a/jupyterhub_announcement/ssl.py b/jupyterhub_announcement/ssl.py new file mode 100644 index 0000000..9b47517 --- /dev/null +++ b/jupyterhub_announcement/ssl.py @@ -0,0 +1,29 @@ +import os + +from jupyterhub.utils import make_ssl_context +from traitlets import Unicode +from traitlets.config import Configurable + + +class SSLContext(Configurable): + + keyfile = Unicode( + os.getenv("JUPYTERHUB_SSL_KEYFILE", ""), help="SSL key, use with certfile" + ).tag(config=True) + + certfile = Unicode( + os.getenv("JUPYTERHUB_SSL_CERTFILE", ""), help="SSL cert, use with keyfile" + ).tag(config=True) + + cafile = Unicode( + os.getenv("JUPYTERHUB_SSL_CLIENT_CA", ""), + help="SSL CA, use with keyfile and certfile", + ).tag(config=True) + + def ssl_context(self): + if self.keyfile and self.certfile and self.cafile: + return make_ssl_context( + self.keyfile, self.certfile, cafile=self.cafile, check_hostname=False + ) + else: + return None From d133341dcbb4b180ba31bee0c36b805c8f28d9f9 Mon Sep 17 00:00:00 2001 From: Rollin Thomas Date: Wed, 18 May 2022 19:22:03 -0700 Subject: [PATCH 03/12] Add some tests --- tests/__init__.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 tests/__init__.py diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..e69de29 From 9820b20f97ee0a5c75609dfd05f487964ad29543 Mon Sep 17 00:00:00 2001 From: Rollin Thomas Date: Wed, 18 May 2022 19:22:35 -0700 Subject: [PATCH 04/12] Add test for ssl context --- tests/test_ssl.py | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 tests/test_ssl.py diff --git a/tests/test_ssl.py b/tests/test_ssl.py new file mode 100644 index 0000000..705a5bd --- /dev/null +++ b/tests/test_ssl.py @@ -0,0 +1,36 @@ +import ssl + +import pytest +from certipy import Certipy + +from jupyterhub_announcement.ssl import SSLContext + + +@pytest.fixture +def ca(): + yield "foo" + + +@pytest.fixture +def keypair(): + yield "bar" + + +@pytest.fixture +def certs_record(tmp_path, ca, keypair): + certipy = Certipy(store_dir=tmp_path) + certipy.create_ca(ca) + yield certipy.create_signed_pair(keypair, ca) + + +def test_none(): + context = SSLContext() + assert context.ssl_context() is None + + +def test_whatever(certs_record): + files = certs_record["files"] + context = SSLContext( + keyfile=files["key"], certfile=files["cert"], cafile=files["ca"] + ) + assert type(context.ssl_context()) == ssl.SSLContext From 9ced0bdbc8971439bc18934d6482af781802f99a Mon Sep 17 00:00:00 2001 From: Rollin Thomas Date: Wed, 18 May 2022 19:24:21 -0700 Subject: [PATCH 05/12] Tests for JSON encoder --- tests/test_encoder.py | 50 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 tests/test_encoder.py diff --git a/tests/test_encoder.py b/tests/test_encoder.py new file mode 100644 index 0000000..237f1d5 --- /dev/null +++ b/tests/test_encoder.py @@ -0,0 +1,50 @@ +import datetime +import json + +import pytest + +from jupyterhub_announcement.encoder import _JSONEncoder + + +@pytest.fixture +def timestamp(): + yield datetime.datetime(2022, 5, 17, 16, 59, 23, 123456) + + +@pytest.fixture +def good_document(timestamp): + yield { + "user": "falken", + "announcement": "Wouldn't you like to play a nice game of chess?", + "timestamp": timestamp, + } + + +@pytest.fixture +def bad_document(timestamp): + class Whatever: + pass + + yield { + "user": "falken", + "announcement": "Wouldn't you like to play a nice game of chess?", + "timestamp": timestamp, + "other": Whatever(), + } + + +def test_json_encoder_good(good_document, timestamp): + string = json.dumps(good_document, cls=_JSONEncoder) + parsed = json.loads(string) + + assert not (set(parsed.keys()) - set(good_document.keys())) + for key in good_document: + if key == "timestamp": + assert parsed[key] == timestamp.isoformat() + else: + assert parsed[key] == good_document[key] + + +def test_json_encoder_bad(bad_document, timestamp): + with pytest.raises(TypeError): + json.dumps(bad_document, cls=_JSONEncoder) From 3a2c96f4d02c31acb1c4df2d1a38318abb9b15a9 Mon Sep 17 00:00:00 2001 From: Rollin Thomas Date: Wed, 18 May 2022 19:24:42 -0700 Subject: [PATCH 06/12] Tests for queue --- tests/test_queue.py | 119 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 119 insertions(+) create mode 100644 tests/test_queue.py diff --git a/tests/test_queue.py b/tests/test_queue.py new file mode 100644 index 0000000..ecc4759 --- /dev/null +++ b/tests/test_queue.py @@ -0,0 +1,119 @@ +import time + +import pytest + +from jupyterhub_announcement.queue import AnnouncementQueue + + +@pytest.fixture +def announcement(): + yield ("user1", "hello world") + + +@pytest.fixture +def lifetime_days_2s(): + yield 2.0 / 86400.0 + + +@pytest.mark.asyncio +async def test_queue_default(announcement): + queue = AnnouncementQueue() + + # Defaults are set + + assert len(queue) == 0 + assert queue.persist_path == "" + assert queue.lifetime_days == pytest.approx(7.0) + + # Updating adds announcement + + await queue.update(*announcement) + assert len(queue) == 1 + assert queue.announcements[0]["user"] == announcement[0] + assert queue.announcements[0]["announcement"] == announcement[1] + + # Updating again adds yet another with a later timestamp + + await queue.update(*announcement) + assert len(queue) == 2 + assert queue.announcements[1]["timestamp"] > queue.announcements[0]["timestamp"] + + # Unless the test takes a log time, purge should have no effect here + + await queue.purge() + assert len(queue) == 2 + + +@pytest.mark.asyncio +async def test_queue_persistent(announcement, tmp_path): + persist_path = str(tmp_path / "announcements.json") + + queue = AnnouncementQueue(persist_path=persist_path) + + # Persist path is set + + assert queue.persist_path == persist_path + + # Update queue + + await queue.update(*announcement) + + # Drop the queue, make new one and verify the announcement is there + + del queue + new_queue = AnnouncementQueue(persist_path=persist_path) + assert len(new_queue) == 1 + assert new_queue.announcements[0]["user"] == announcement[0] + assert new_queue.announcements[0]["announcement"] == announcement[1] + + +@pytest.mark.asyncio +async def test_queue_purge(announcement, tmp_path, lifetime_days_2s): + persist_path = str(tmp_path / "announcements.json") + + queue = AnnouncementQueue(persist_path=persist_path, lifetime_days=lifetime_days_2s) + + # Update queue and purge immediately without effect + + await queue.update(*announcement) + await queue.purge() + assert len(queue) == 1 + + # Wait until after purge and verify old message disappeared + + time.sleep(5) + await queue.purge() + assert len(queue) == 0 + + # Drop the queue, make new one, should be no message + + del queue + new_queue = AnnouncementQueue(persist_path=persist_path) + assert len(new_queue) == 0 + + +def test_queue_restore_fail(tmp_path): + + # Failure to restore isn't fatal, the queue is just empty + + persist_path = tmp_path / "announcements.json" + persist_path.write_text("not json") + queue = AnnouncementQueue(persist_path=str(persist_path)) + assert len(queue) == 0 + + +@pytest.mark.asyncio +async def test_queue_persist_fail(tmp_path, announcement): + class Whatever: + pass + + # Failure to persist isn't fatal just keep going + + persist_path = str(tmp_path / "announcements.json") + queue = AnnouncementQueue(persist_path=persist_path) + await queue.update(*announcement) + queue.announcements[0]["other"] = Whatever() + try: + await queue._handle_persist() + except Exception as err: + assert False, f"'_handle_persist' raised exception {err}" From 0044a5b1ecb57d9efd4fb4094383471f813f2c0b Mon Sep 17 00:00:00 2001 From: Rollin Thomas Date: Wed, 18 May 2022 19:55:58 -0700 Subject: [PATCH 07/12] Run on any --- .github/workflows/python-package.yml | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index 34118f5..810f051 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -1,13 +1,9 @@ # This workflow will install Python dependencies, run tests and lint with a variety of Python versions # For more information see: https://help.github.com/actions/language-and-framework-guides/using-python-with-github-actions -name: Python package +name: Test -on: - push: - branches: [ main ] - pull_request: - branches: [ main ] +on: [push, pull_request] jobs: build: From 4166658b2ea51c213c4945a8d6c9a2d9d0708389 Mon Sep 17 00:00:00 2001 From: Rollin Thomas Date: Wed, 18 May 2022 19:57:44 -0700 Subject: [PATCH 08/12] Fix test --- .github/workflows/python-package.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index 810f051..cca2097 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -33,4 +33,4 @@ jobs: flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics - name: Test with pytest run: | - pytest jupyterhub_announcement + python -m pytest -v --cov=jupyterhub_announcement --cov-report=term-missing tests From b9ff470afab9e740a78ca4526e9174da2863328f Mon Sep 17 00:00:00 2001 From: Rollin Thomas Date: Wed, 18 May 2022 19:59:07 -0700 Subject: [PATCH 09/12] Add pytest-cov and pytest-asyncio --- .github/workflows/python-package.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index cca2097..1b5bbf4 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -23,7 +23,7 @@ jobs: - name: Install dependencies run: | python -m pip install --upgrade pip - python -m pip install flake8 pytest + python -m pip install flake8 pytest pytest-asyncio pytest-cov if [ -f requirements.txt ]; then pip install -r requirements.txt; fi - name: Lint with flake8 run: | From 88effe0681be797bdc74716569d552e07afa0091 Mon Sep 17 00:00:00 2001 From: Rollin Thomas Date: Wed, 18 May 2022 20:08:31 -0700 Subject: [PATCH 10/12] Only dev requirements not pre-commit --- requirements-dev.txt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/requirements-dev.txt b/requirements-dev.txt index 416634f..50ef6d5 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1 +1,4 @@ -pre-commit +flake8 +pytest +pytest-asyncio +pytest-cov From 54c83eedb0db7345c1eb15788a3b3bf8f18f5d79 Mon Sep 17 00:00:00 2001 From: Rollin Thomas Date: Wed, 18 May 2022 20:09:11 -0700 Subject: [PATCH 11/12] Use requirements files --- .github/workflows/python-package.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index 1b5bbf4..44ad2b1 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -23,8 +23,8 @@ jobs: - name: Install dependencies run: | python -m pip install --upgrade pip - python -m pip install flake8 pytest pytest-asyncio pytest-cov if [ -f requirements.txt ]; then pip install -r requirements.txt; fi + if [ -f requirements-dev.txt ]; then pip install -r requirements-dev.txt; fi - name: Lint with flake8 run: | # stop the build if there are Python syntax errors or undefined names From 8db3d55b0a9ecd6473ecbb204506b54167df526a Mon Sep 17 00:00:00 2001 From: Rollin Thomas Date: Wed, 18 May 2022 20:09:25 -0700 Subject: [PATCH 12/12] Use requirements.txt --- requirements.txt | 3 +++ setup.py | 6 +----- 2 files changed, 4 insertions(+), 5 deletions(-) create mode 100644 requirements.txt diff --git a/requirements.txt b/requirements.txt new file mode 100644 index 0000000..e1efed0 --- /dev/null +++ b/requirements.txt @@ -0,0 +1,3 @@ +aiofiles +html-sanitizer +jupyterhub diff --git a/setup.py b/setup.py index 3a73b36..ede8f46 100644 --- a/setup.py +++ b/setup.py @@ -5,11 +5,7 @@ author_email="rcthomas@lbl.gov", data_files=[("share/jupyterhub/announcement/templates", ["templates/index.html"])], description="JupyterHub Announcement Service", - install_requires=[ - "aiofiles", - "html-sanitizer", - "jupyterhub", - ], + install_requires=open("requirements.txt").read().splitlines(), name="jupyterhub-announcement", packages=["jupyterhub_announcement"], version="0.8.0.dev",