feat: Phase 2 — remove builtins layer, ServiceRegistry is installed-only
Unit Tests / test (push) Successful in 11m31s

Builtins (email/calendar/files) are no longer baked into the API image.
ServiceRegistry now only knows about installed store services. When nothing
is installed, Caddy and DNS get no service routes — no hardcoded fallback.

Changes:
- service_registry.py: remove _BUILTINS_DIR, _builtin_ids, _builtin_manifest,
  _load_manifest; get() and list_all() now delegate entirely to installed services
- caddy_manager.py: remove _build_core_service_routes(); remove hardcoded
  fallback pairs from _http01_service_pairs(); empty registry → api block only
- network_manager.py: _get_service_subdomains() returns [] when no registry
- api/services/builtins/: deleted (email, calendar, files manifests)
- Tests updated throughout: removed builtin-dependent assertions, added
  installed-service fixtures, updated fallback expectations to api-only

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
2026-05-29 08:53:44 -04:00
parent 18b50d08c1
commit 0bfe95320b
12 changed files with 419 additions and 766 deletions
+233 -178
View File
@@ -1,164 +1,210 @@
"""
Unit tests for ServiceRegistry.
Tests load actual built-in manifests from api/services/builtins/ and verify
that the registry merges config correctly, returns expected routes/backup plans,
and handles missing manifests gracefully.
Tests verify that the registry merges config correctly from installed store
services, returns expected routes/backup plans, and handles missing or broken
records gracefully. There are no builtins — only installed store services.
"""
import json
import os
import sys
import tempfile
import unittest
from pathlib import Path
from unittest.mock import MagicMock, patch
from unittest.mock import MagicMock
sys.path.insert(0, str(Path(__file__).parent.parent / 'api'))
from service_registry import ServiceRegistry, _BUILTINS_DIR
from service_registry import ServiceRegistry
def _make_cm(configs: dict = None) -> MagicMock:
# ---------------------------------------------------------------------------
# Shared manifests used across multiple test classes
# ---------------------------------------------------------------------------
_CALENDAR_MANIFEST = {
'schema_version': 1,
'id': 'calendar',
'name': 'Calendar',
'kind': 'store',
'capabilities': {
'has_subdomain': True,
'has_accounts': False,
'has_admin_config': True,
'has_storage': True,
'has_egress': False,
'has_api_hooks': False,
},
'subdomain': 'calendar',
'backend': 'cell-radicale:5232',
'extra_subdomains': [],
'extra_backends': {},
'config_schema': {
'port': {'type': 'integer', 'default': 5232, 'label': 'Port'},
},
'peer_config_template': {
'caldav_url': 'https://calendar.{domain}/radicale/{peer.username}/',
'password': '{peer.service_credentials.calendar.password}',
},
'backup': {
'volumes': [
{'container': 'cell-radicale', 'path': '/data', 'name': 'radicale_data'}
]
},
}
_EMAIL_MANIFEST = {
'schema_version': 1,
'id': 'email',
'name': 'Email',
'kind': 'store',
'capabilities': {
'has_subdomain': True,
'has_accounts': True,
'has_admin_config': True,
'has_storage': True,
'has_egress': True,
'has_api_hooks': False,
},
'subdomain': 'mail',
'backend': 'cell-rainloop:8888',
'extra_subdomains': ['webmail'],
'extra_backends': {},
'config_schema': {
'smtp_port': {'type': 'integer', 'default': 587, 'label': 'SMTP Port'},
'imap_port': {'type': 'integer', 'default': 993, 'label': 'IMAP Port'},
},
'peer_config_template': {
'smtp_server': 'mail.{domain}',
'imap_server': 'mail.{domain}',
'password': '{peer.service_credentials.email.password}',
},
'backup': {
'volumes': [
{'container': 'cell-mail', 'path': '/var/mail', 'name': 'maildata'},
]
},
}
_FILES_MANIFEST = {
'schema_version': 1,
'id': 'files',
'name': 'Files',
'kind': 'store',
'capabilities': {
'has_subdomain': True,
'has_accounts': False,
'has_admin_config': False,
'has_storage': True,
'has_egress': False,
'has_api_hooks': False,
},
'subdomain': 'files',
'backend': 'cell-filegator:8080',
'extra_subdomains': ['webdav'],
'extra_backends': {'webdav': 'cell-webdav:80'},
'config_schema': {},
'peer_config_template': {
'files_url': 'https://files.{domain}/',
},
'backup': {
'volumes': [
{'container': 'cell-filegator', 'path': '/data', 'name': 'filegator'},
{'container': 'cell-webdav', 'path': '/data', 'name': 'files'},
]
},
}
def _make_cm(configs: dict = None, installed: dict = None) -> MagicMock:
cm = MagicMock()
cm.configs = configs or {}
cm.get_installed_services.return_value = {}
cm.get_installed_services.return_value = installed or {}
return cm
class TestBuiltinManifests(unittest.TestCase):
"""Verify the built-in manifest files are valid JSON with required fields."""
def _load(self, service_id: str) -> dict:
path = os.path.join(_BUILTINS_DIR, service_id, 'manifest.json')
self.assertTrue(os.path.exists(path), f'Missing manifest for {service_id}')
with open(path) as f:
return json.load(f)
def _assert_required(self, manifest: dict):
for field in ('schema_version', 'id', 'name', 'kind', 'capabilities'):
self.assertIn(field, manifest, f'Missing required field: {field}')
caps = manifest['capabilities']
for cap in ('has_subdomain', 'has_accounts', 'has_admin_config',
'has_storage', 'has_egress', 'has_api_hooks'):
self.assertIn(cap, caps, f'Missing capability flag: {cap}')
def test_email_manifest_valid(self):
m = self._load('email')
self._assert_required(m)
self.assertEqual(m['id'], 'email')
self.assertEqual(m['kind'], 'builtin')
self.assertIn('mail', [m.get('subdomain')] + (m.get('extra_subdomains') or []))
self.assertIn('webmail', m.get('extra_subdomains', []))
self.assertEqual(m['capabilities']['has_accounts'], True)
def test_calendar_manifest_valid(self):
m = self._load('calendar')
self._assert_required(m)
self.assertEqual(m['id'], 'calendar')
self.assertEqual(m['subdomain'], 'calendar')
def test_files_manifest_valid(self):
m = self._load('files')
self._assert_required(m)
self.assertEqual(m['id'], 'files')
self.assertIn('webdav', m.get('extra_subdomains', []))
def test_all_builtins_have_backup_volumes(self):
for svc_id in ('email', 'calendar', 'files'):
m = self._load(svc_id)
volumes = m.get('backup', {}).get('volumes')
self.assertTrue(volumes, f'{svc_id}: backup.volumes must not be empty')
for vol in volumes:
for field in ('container', 'path', 'name'):
self.assertIn(field, vol,
f'{svc_id}: backup volume entry missing {field!r}')
def test_all_builtins_have_peer_config_template(self):
for svc_id in ('email', 'calendar', 'files'):
m = self._load(svc_id)
self.assertTrue(m.get('peer_config_template'),
f'{svc_id}: peer_config_template must not be empty')
def test_config_schema_defaults_are_correct_types(self):
for svc_id in ('email', 'calendar', 'files'):
m = self._load(svc_id)
for field, spec in (m.get('config_schema') or {}).items():
if 'default' in spec:
if spec['type'] == 'integer':
self.assertIsInstance(
spec['default'], int,
f'{svc_id}.{field}: integer default must be int')
elif spec['type'] == 'string':
self.assertIsInstance(
spec['default'], str,
f'{svc_id}.{field}: string default must be str')
# ---------------------------------------------------------------------------
# TestServiceRegistryListAll
# ---------------------------------------------------------------------------
class TestServiceRegistryListAll(unittest.TestCase):
def setUp(self):
self.cm = _make_cm()
self.registry = ServiceRegistry(self.cm)
def test_list_all_empty_when_nothing_installed(self):
cm = _make_cm()
reg = ServiceRegistry(cm)
self.assertEqual(reg.list_all(), [])
def test_lists_three_builtins(self):
services = self.registry.list_all()
ids = [s['id'] for s in services]
self.assertIn('email', ids)
def test_list_all_returns_installed_services(self):
cm = _make_cm(installed={'calendar': {'manifest': _CALENDAR_MANIFEST}})
reg = ServiceRegistry(cm)
ids = [s['id'] for s in reg.list_all()]
self.assertIn('calendar', ids)
self.assertIn('files', ids)
def test_builtins_come_before_store_services(self):
self.cm.get_installed_services.return_value = {
'zstore': {'manifest': {
'id': 'zstore', 'name': 'Z Store', 'kind': 'store',
'capabilities': {}, 'config_schema': {}
}}
}
services = self.registry.list_all()
ids = [s['id'] for s in services]
# builtins (email, calendar, files) should all appear before zstore
for builtin_id in ('email', 'calendar', 'files'):
self.assertLess(ids.index(builtin_id), ids.index('zstore'))
def test_each_service_has_config_key(self):
for svc in self.registry.list_all():
cm = _make_cm(installed={
'calendar': {'manifest': _CALENDAR_MANIFEST},
'email': {'manifest': _EMAIL_MANIFEST},
})
reg = ServiceRegistry(cm)
for svc in reg.list_all():
self.assertIn('config', svc, f'{svc["id"]} missing config key')
def test_no_duplicate_ids(self):
services = self.registry.list_all()
ids = [s['id'] for s in services]
cm = _make_cm(installed={
'calendar': {'manifest': _CALENDAR_MANIFEST},
'email': {'manifest': _EMAIL_MANIFEST},
})
reg = ServiceRegistry(cm)
ids = [s['id'] for s in reg.list_all()]
self.assertEqual(len(ids), len(set(ids)))
# ---------------------------------------------------------------------------
# TestServiceRegistryConfigMerge
# ---------------------------------------------------------------------------
class TestServiceRegistryConfigMerge(unittest.TestCase):
def test_defaults_used_when_no_saved_config(self):
cm = _make_cm({'calendar': {}})
cm = _make_cm(
configs={'calendar': {}},
installed={'calendar': {'manifest': _CALENDAR_MANIFEST}},
)
reg = ServiceRegistry(cm)
svc = reg.get('calendar')
self.assertEqual(svc['config']['port'], 5232)
def test_saved_config_overrides_defaults(self):
cm = _make_cm({'calendar': {'port': 9999}})
cm = _make_cm(
configs={'calendar': {'port': 9999}},
installed={'calendar': {'manifest': _CALENDAR_MANIFEST}},
)
reg = ServiceRegistry(cm)
svc = reg.get('calendar')
self.assertEqual(svc['config']['port'], 9999)
def test_unknown_saved_keys_excluded(self):
cm = _make_cm({'calendar': {'port': 5232, 'unknown_field': 'x'}})
cm = _make_cm(
configs={'calendar': {'port': 5232, 'unknown_field': 'x'}},
installed={'calendar': {'manifest': _CALENDAR_MANIFEST}},
)
reg = ServiceRegistry(cm)
svc = reg.get('calendar')
self.assertNotIn('unknown_field', svc['config'])
def test_partial_override_keeps_other_defaults(self):
cm = _make_cm({'email': {'smtp_port': 2525}})
cm = _make_cm(
configs={'email': {'smtp_port': 2525}},
installed={'email': {'manifest': _EMAIL_MANIFEST}},
)
reg = ServiceRegistry(cm)
svc = reg.get('email')
self.assertEqual(svc['config']['smtp_port'], 2525)
self.assertEqual(svc['config']['imap_port'], 993)
# ---------------------------------------------------------------------------
# TestServiceRegistryGet
# ---------------------------------------------------------------------------
class TestServiceRegistryGet(unittest.TestCase):
def setUp(self):
@@ -168,11 +214,6 @@ class TestServiceRegistryGet(unittest.TestCase):
def test_returns_none_for_unknown_id(self):
self.assertIsNone(self.registry.get('nonexistent_service'))
def test_returns_builtin_by_id(self):
svc = self.registry.get('email')
self.assertIsNotNone(svc)
self.assertEqual(svc['id'], 'email')
def test_returns_store_service_from_installed(self):
self.cm.get_installed_services.return_value = {
'mywiki': {'manifest': {
@@ -184,6 +225,16 @@ class TestServiceRegistryGet(unittest.TestCase):
self.assertIsNotNone(svc)
self.assertEqual(svc['id'], 'mywiki')
def test_get_returns_none_when_installed_record_has_no_manifest(self):
self.cm.get_installed_services.return_value = {
'broken': {} # record exists but has no 'manifest' key
}
self.assertIsNone(self.registry.get('broken'))
# ---------------------------------------------------------------------------
# TestServiceRegistryGetCaddyRoutes
# ---------------------------------------------------------------------------
class TestServiceRegistryGetCaddyRoutes(unittest.TestCase):
@@ -191,22 +242,6 @@ class TestServiceRegistryGetCaddyRoutes(unittest.TestCase):
self.cm = _make_cm()
self.registry = ServiceRegistry(self.cm)
def test_all_builtins_appear_in_routes(self):
routes = self.registry.get_caddy_routes()
route_ids = [r['service_id'] for r in routes]
for svc_id in ('email', 'calendar', 'files'):
self.assertIn(svc_id, route_ids)
def test_email_route_has_webmail_extra_subdomain(self):
routes = self.registry.get_caddy_routes()
email_route = next(r for r in routes if r['service_id'] == 'email')
self.assertIn('webmail', email_route['extra_subdomains'])
def test_files_route_has_webdav_extra_subdomain(self):
routes = self.registry.get_caddy_routes()
files_route = next(r for r in routes if r['service_id'] == 'files')
self.assertIn('webdav', files_route['extra_subdomains'])
def test_services_without_subdomain_excluded(self):
self.cm.get_installed_services.return_value = {
'nosubdomain': {'manifest': {
@@ -218,6 +253,21 @@ class TestServiceRegistryGetCaddyRoutes(unittest.TestCase):
routes = self.registry.get_caddy_routes()
self.assertNotIn('nosubdomain', [r['service_id'] for r in routes])
def test_installed_service_with_subdomain_appears_in_routes(self):
self.cm.get_installed_services.return_value = {
'calendar': {'manifest': _CALENDAR_MANIFEST},
}
routes = self.registry.get_caddy_routes()
route_ids = [r['service_id'] for r in routes]
self.assertIn('calendar', route_ids)
cal_route = next(r for r in routes if r['service_id'] == 'calendar')
self.assertEqual(cal_route['subdomain'], 'calendar')
self.assertEqual(cal_route['backend'], 'cell-radicale:5232')
# ---------------------------------------------------------------------------
# TestServiceRegistryGetBackupPlan
# ---------------------------------------------------------------------------
class TestServiceRegistryGetBackupPlan(unittest.TestCase):
@@ -225,37 +275,6 @@ class TestServiceRegistryGetBackupPlan(unittest.TestCase):
self.cm = _make_cm()
self.registry = ServiceRegistry(self.cm)
def test_all_builtins_in_backup_plan(self):
plan = self.registry.get_backup_plan()
plan_ids = [p['service_id'] for p in plan]
for svc_id in ('email', 'calendar', 'files'):
self.assertIn(svc_id, plan_ids)
def test_email_backup_includes_maildata_volume(self):
plan = self.registry.get_backup_plan()
email_plan = next(p for p in plan if p['service_id'] == 'email')
names = [v['name'] for v in email_plan['volumes']]
self.assertIn('maildata', names)
vol = next(v for v in email_plan['volumes'] if v['name'] == 'maildata')
self.assertEqual(vol['container'], 'cell-mail')
self.assertEqual(vol['path'], '/var/mail')
def test_calendar_backup_includes_radicale_volume(self):
plan = self.registry.get_backup_plan()
cal_plan = next(p for p in plan if p['service_id'] == 'calendar')
names = [v['name'] for v in cal_plan['volumes']]
self.assertIn('radicale_data', names)
vol = next(v for v in cal_plan['volumes'] if v['name'] == 'radicale_data')
self.assertEqual(vol['container'], 'cell-radicale')
self.assertEqual(vol['path'], '/data')
def test_files_backup_includes_both_volumes(self):
plan = self.registry.get_backup_plan()
files_plan = next(p for p in plan if p['service_id'] == 'files')
names = {v['name'] for v in files_plan['volumes']}
self.assertIn('filegator', names)
self.assertIn('files', names)
def test_service_without_storage_excluded(self):
self.cm.get_installed_services.return_value = {
'nostorage': {'manifest': {
@@ -267,11 +286,29 @@ class TestServiceRegistryGetBackupPlan(unittest.TestCase):
plan = self.registry.get_backup_plan()
self.assertNotIn('nostorage', [p['service_id'] for p in plan])
def test_installed_service_with_storage_in_backup_plan(self):
self.cm.get_installed_services.return_value = {
'calendar': {'manifest': _CALENDAR_MANIFEST},
}
plan = self.registry.get_backup_plan()
plan_ids = [p['service_id'] for p in plan]
self.assertIn('calendar', plan_ids)
cal_plan = next(p for p in plan if p['service_id'] == 'calendar')
names = [v['name'] for v in cal_plan['volumes']]
self.assertIn('radicale_data', names)
# ---------------------------------------------------------------------------
# TestServiceRegistryGetPeerServiceInfo
# ---------------------------------------------------------------------------
class TestServiceRegistryGetPeerServiceInfo(unittest.TestCase):
def setUp(self):
self.cm = _make_cm({'calendar': {}})
self.cm = _make_cm(
configs={'calendar': {}},
installed={'calendar': {'manifest': _CALENDAR_MANIFEST}},
)
self.registry = ServiceRegistry(self.cm)
def test_fills_domain_placeholder(self):
@@ -306,41 +343,59 @@ class TestServiceRegistryGetPeerServiceInfo(unittest.TestCase):
self.assertIn('legit.example.com', info['caldav_url'])
# ---------------------------------------------------------------------------
# TestServiceRegistryConfigMergeTypeCoercion
# ---------------------------------------------------------------------------
class TestServiceRegistryConfigMergeTypeCoercion(unittest.TestCase):
def test_string_in_config_coerced_to_int(self):
cm = _make_cm({'calendar': {'port': '9999'}})
cm = _make_cm(
configs={'calendar': {'port': '9999'}},
installed={'calendar': {'manifest': _CALENDAR_MANIFEST}},
)
reg = ServiceRegistry(cm)
svc = reg.get('calendar')
self.assertIsInstance(svc['config']['port'], int)
self.assertEqual(svc['config']['port'], 9999)
def test_unconvertible_value_falls_back_to_default(self):
cm = _make_cm({'calendar': {'port': 'not_a_number'}})
cm = _make_cm(
configs={'calendar': {'port': 'not_a_number'}},
installed={'calendar': {'manifest': _CALENDAR_MANIFEST}},
)
reg = ServiceRegistry(cm)
svc = reg.get('calendar')
self.assertEqual(svc['config']['port'], 5232)
class TestServiceRegistryWithBrokenManifest(unittest.TestCase):
"""Registry must not crash when a manifest file is corrupt or missing."""
# ---------------------------------------------------------------------------
# TestServiceRegistryRobustness
# ---------------------------------------------------------------------------
def test_missing_builtins_dir_returns_empty(self):
with patch('service_registry._BUILTINS_DIR', '/nonexistent/path'):
reg = ServiceRegistry(_make_cm())
self.assertEqual(reg.list_all(), [])
class TestServiceRegistryRobustness(unittest.TestCase):
"""Registry must not crash when records are corrupt or missing."""
def test_malformed_json_manifest_skipped(self):
with tempfile.TemporaryDirectory() as tmpdir:
bad_dir = os.path.join(tmpdir, 'bad_svc')
os.makedirs(bad_dir)
with open(os.path.join(bad_dir, 'manifest.json'), 'w') as f:
f.write('this is not json {{{')
with patch('service_registry._BUILTINS_DIR', tmpdir):
reg = ServiceRegistry(_make_cm())
# Should not raise; just return empty list
result = reg.list_all()
self.assertEqual(result, [])
def test_installed_record_with_no_manifest_skipped(self):
cm = _make_cm(installed={'broken': {}})
reg = ServiceRegistry(cm)
self.assertIsNone(reg.get('broken'))
def test_list_all_skips_records_without_id(self):
cm = _make_cm(installed={
'noid': {'manifest': {
'name': 'No ID Service', 'kind': 'store',
'capabilities': {}, 'config_schema': {},
# 'id' key intentionally absent
}},
'calendar': {'manifest': _CALENDAR_MANIFEST},
})
reg = ServiceRegistry(cm)
result = reg.list_all()
ids = [s['id'] for s in result]
self.assertNotIn(None, ids)
self.assertIn('calendar', ids)
self.assertEqual(len(result), 1)
if __name__ == '__main__':