Add tests

ci/init-db.sh

@@ -21,7 +21,7 @@ fi
 # Configure a set of databases in the database server for upgrade tests
 # this list must be in sync with versions in test_db.py:test_upgrade
 set -x
-for SUFFIX in '' _upgrade_110 _upgrade_122 _upgrade_130 _upgrade_150 _upgrade_211; do
+for SUFFIX in '' _upgrade_110 _upgrade_122 _upgrade_130 _upgrade_150 _upgrade_211 _upgrade_311; do
     $SQL_CLIENT "DROP DATABASE jupyterhub${SUFFIX};" 2>/dev/null || true
     $SQL_CLIENT "CREATE DATABASE jupyterhub${SUFFIX} ${EXTRA_CREATE_DATABASE_ARGS:-};"
 done

jupyterhub/apihandlers/services.py

@@ -49,10 +49,16 @@ class ServiceAPIHandler(APIHandler):
 
         self._check_service_model(spec)
         service_name = spec["name"]
-        new_service = self.service_from_spec(spec)
+        try:
+            new_service = self.service_from_spec(spec)
+        except Exception:
+            msg = f"Failed to create service {service_name}"
+            self.log.error(msg, exc_info=True)
+            raise web.HTTPError(400, msg)
 
         if new_service is None:
             raise web.HTTPError(400, f"Failed to create service {service_name}")
+
         if new_service.api_token:
             # Add api token to database
             await self.app._add_tokens(
@@ -150,7 +156,7 @@ class ServiceAPIHandler(APIHandler):
 
         if service.oauth_no_confirm:
             oauth_no_confirm_list = self.settings.get('oauth_no_confirm_list')
-            oauth_no_confirm_list.remove(service.oauth_client_id)
+            oauth_no_confirm_list.discard(service.oauth_client_id)
 
         self.db.delete(orm_service)
         self.db.commit()

jupyterhub/orm.py

@@ -458,39 +458,54 @@ class Service(Base):
         """
         return db.query(cls).filter(cls.name == name).first()
 
-    def update_column(self, column_name: str, value: any) -> bool:
-        """_summary_
+    def _check_data_only_column(self, column_name: str) -> bool:
+        """Check if a column is not a ForeignKey or in a
+        relationship.
 
         Args:
-            column_name (str): _description_
-            value (any): _description_
+            column_name (str): ame of the column
 
         Returns:
-            bool: _description_
+            bool: returns `True` if the column is data-only,
+            returns `False` otherwise.
         """
-        if (
-            hasattr(self, column_name)
-            and not getattr(Service, column_name).foreign_keys
-        ):
+        if hasattr(self, column_name):
+            column = getattr(Service, column_name)
+            if hasattr(column, 'foreign_keys'):
+                return len(column.foreign_keys) == 0
+            else:
+                return False
+        else:
+            return False
+
+    def update_column(self, column_name: str, value: any) -> bool:
+        """Update the value of a non-ForeignKey column.
+
+        Args:
+            column_name (str): Name of the column
+            value (any): New value
+
+        Returns:
+            bool: returns `True` if the column is updated,
+            returns `False` otherwise.
+        """
+        if self._check_data_only_column(column_name):
             setattr(self, column_name, value)
             return True
         else:
             return False
 
     def get_column(self, column_name: str):
-        """_summary_
+        """Get non-ForeignKey, non-relationship column
 
         Args:
-            column_name (str): _description_
-            value (any): _description_
+            column_name (str): Name of the column
 
         Returns:
-            bool: _description_
+            The value of requested column, None if it is
+            a foreign key or it does not exist.
         """
-        if (
-            hasattr(self, column_name)
-            and not getattr(Service, column_name).foreign_keys
-        ):
+        if self._check_data_only_column(column_name):
             return getattr(self, column_name)
         else:
             return None

jupyterhub/tests/test_api.py

@@ -8,12 +8,13 @@ from datetime import datetime, timedelta
 from unittest import mock
 from urllib.parse import quote, urlparse
 
+import pytest
 from pytest import fixture, mark
 from tornado.httputil import url_concat
 
 import jupyterhub
 
-from .. import orm
+from .. import orm, roles
 from ..apihandlers.base import PAGINATION_MEDIA_TYPE
 from ..objects import Server
 from ..utils import url_path_join as ujoin
@@ -2091,6 +2092,155 @@ async def test_get_service(app, mockservice_url):
     assert r.status_code == 403
 
 
+@pytest.fixture
+def service_data():
+    return {
+        "oauth_client_id": "service-oauth-client-from-api",
+        "api_token": "api_token-from-api",
+        "oauth_redirect_uri": "http://127.0.0.1:5555/oauth_callback-from-api",
+        "oauth_no_confirm": True,
+        "info": {'foo': 'bar'},
+    }
+
+
+@pytest.fixture
+def service_admin_user(app):
+    user_name = 'admin_services'
+    service_role = {
+        'name': 'admin-services-role',
+        'description': '',
+        'users': [user_name],
+        'scopes': ['admin:services'],
+    }
+    roles.create_role(app.db, service_role)
+    user = add_user(app.db, name=user_name)
+    roles.update_roles(app.db, user, roles=['admin-services-role'])
+    return user
+
+
+@mark.services
+async def test_create_service(app, service_admin_user, service_data):
+    db = app.db
+    service_name = 'service-from-api'
+    r = await api_request(
+        app,
+        f'services/{service_name}',
+        headers=auth_header(db, service_admin_user.name),
+        data=json.dumps(service_data),
+        method='post',
+    )
+
+    r.raise_for_status()
+    assert r.status_code == 201
+    assert r.json()['name'] == service_name
+    orm_service = orm.Service.find(db, service_name)
+    assert orm_service is not None
+
+    oath_client = (
+        db.query(orm.OAuthClient)
+        .filter_by(identifier=service_data['oauth_client_id'])
+        .first()
+    )
+    assert oath_client.redirect_uri == service_data['oauth_redirect_uri']
+
+    assert service_name in app._service_map
+    assert (
+        app._service_map[service_name].oauth_no_confirm
+        == service_data['oauth_no_confirm']
+    )
+
+
+@mark.services
+async def test_create_service_no_role(app, service_data):
+    db = app.db
+    service_name = 'service-from-api'
+    r = await api_request(
+        app,
+        f'services/{service_name}',
+        headers=auth_header(db, 'user'),
+        data=json.dumps(service_data),
+        method='post',
+    )
+
+    assert r.status_code == 403
+
+
+@mark.services
+async def test_create_service_conflict(app, service_admin_user, service_data):
+    db = app.db
+    service_name = 'service-from-config'
+    app.services = [{'name': service_name}]
+    app.init_services()
+
+    r = await api_request(
+        app,
+        f'services/{service_name}',
+        headers=auth_header(db, service_admin_user.name),
+        data=json.dumps(service_data),
+        method='post',
+    )
+
+    assert r.status_code == 409
+
+
+@mark.services
+async def test_create_service_duplication(app, service_admin_user, service_data):
+    db = app.db
+    service_name = 'service-from-api'
+
+    r = await api_request(
+        app,
+        f'services/{service_name}',
+        headers=auth_header(db, service_admin_user.name),
+        data=json.dumps(service_data),
+        method='post',
+    )
+    assert r.status_code == 409
+
+
+@mark.services
+async def test_remove_service(app, service_admin_user, service_data):
+    db = app.db
+    service_name = 'service-from-api'
+
+    r = await api_request(
+        app,
+        f'services/{service_name}',
+        headers=auth_header(db, service_admin_user.name),
+        method='delete',
+    )
+    assert r.status_code == 200
+
+    orm_service = orm.Service.find(db, service_name)
+    assert orm_service is None
+
+    oath_client = (
+        db.query(orm.OAuthClient)
+        .filter_by(identifier=service_data['oauth_client_id'])
+        .first()
+    )
+    assert oath_client is None
+
+    assert service_name not in app._service_map
+
+
+@mark.services
+async def test_remove_service_from_config(app, service_admin_user):
+    db = app.db
+    service_name = 'service-from-config'
+    r = await api_request(
+        app,
+        f'services/{service_name}',
+        headers=auth_header(db, service_admin_user.name),
+        method='delete',
+    )
+    assert r.status_code == 405
+    assert (
+        r.json()['message']
+        == 'Service service-from-config is not modifiable at runtime'
+    )
+
+
 async def test_root_api(app):
     base_url = app.hub.url
     url = ujoin(base_url, 'api')

jupyterhub/tests/test_db.py

@@ -44,7 +44,9 @@ def generate_old_db(env_dir, hub_version, db_url):
 
 # changes to this version list must also be reflected
 # in ci/init-db.sh
-@pytest.mark.parametrize('hub_version', ["1.1.0", "1.2.2", "1.3.0", "1.5.0", "2.1.1"])
+@pytest.mark.parametrize(
+    'hub_version', ['1.1.0', '1.2.2', '1.3.0', '1.5.0', '2.1.1', '3.1.1']
+)
 async def test_upgrade(tmpdir, hub_version):
     db_url = os.getenv('JUPYTERHUB_TEST_DB_URL')
     if db_url:

jupyterhub/tests/test_orm.py

@@ -145,6 +145,33 @@ def test_token_expiry(db):
     assert orm_token not in user.api_tokens
 
 
+def test_service_check_data_only_column(db):
+    orm_service = orm.Service(name='check_data_only_column', from_config=True)
+    db.add(orm_service)
+    db.commit()
+    assert orm_service._check_data_only_column('from_config')
+    assert not orm_service._check_data_only_column('server')
+    assert not orm_service._check_data_only_column('oauth_client_id')
+
+
+def test_service_get_column(db):
+    orm_service = orm.Service(name='test_service_get_column', from_config=True)
+    db.add(orm_service)
+    db.commit()
+    orm_service.server = orm.Server()
+    assert orm_service.get_column('from_config')
+    assert orm_service.get_column('server') is None
+
+
+def test_service_update_column(db):
+    orm_service = orm.Service(name='test_service_set_column', from_config=True)
+    db.add(orm_service)
+    db.commit()
+
+    assert orm_service.update_column('from_config', False)
+    assert not orm_service.update_column('server', orm.Server())
+
+
 def test_service_tokens(db):
     service = orm.Service(name='secret')
     db.add(service)

jupyterhub/tests/test_roles.py

@@ -236,6 +236,16 @@ def test_orm_roles_delete_cascade(db):
             ['tokens!group=hobbits'],
             {'tokens!group=hobbits', 'read:tokens!group=hobbits'},
         ),
+        (
+            ['admin:services'],
+            {
+                'read:roles:services',
+                'read:services:name',
+                'admin:services',
+                'list:services',
+                'read:services',
+            },
+        ),
     ],
 )
 def test_get_expanded_scopes(db, scopes, expected_scopes):