feature(services): introduce 'modules' field in userdata and group services settings there

selfprivacy_api/migrations/__init__.py

@@ -19,6 +19,7 @@ from selfprivacy_api.migrations.migrate_to_selfprivacy_channel import (
 )
 from selfprivacy_api.migrations.mount_volume import MountVolume
 from selfprivacy_api.migrations.providers import CreateProviderFields
+from selfprivacy_api.migrations.modules_in_json import CreateModulesField
 from selfprivacy_api.migrations.prepare_for_nixos_2211 import (
     MigrateToSelfprivacyChannelFrom2205,
 )
@@ -37,6 +38,7 @@ migrations = [
     MigrateToSelfprivacyChannelFrom2205(),
     MigrateToSelfprivacyChannelFrom2211(),
     LoadTokensToRedis(),
+    CreateModulesField(),
 ]
 
 

selfprivacy_api/migrations/modules_in_json.py

@@ -0,0 +1,50 @@
+from selfprivacy_api.migrations.migration import Migration
+from selfprivacy_api.utils import ReadUserData, WriteUserData
+from selfprivacy_api.services import get_all_services
+
+
+def migrate_services_to_modules():
+    with WriteUserData() as userdata:
+        if "modules" not in userdata.keys():
+            userdata["modules"] = {}
+
+        for service in get_all_services():
+            name = service.get_id()
+            if name in userdata.keys():
+                field_content = userdata[name]
+                userdata["modules"][name] = field_content
+                del userdata[name]
+
+
+# If you ever want to get rid of modules field you will need to get rid of this migration
+class CreateModulesField(Migration):
+    """introduce 'modules' (services) into userdata"""
+
+    def get_migration_name(self):
+        return "modules_in_json"
+
+    def get_migration_description(self):
+        return "Group service settings into a 'modules' field in userdata.json"
+
+    def is_migration_needed(self) -> bool:
+        try:
+            with ReadUserData() as userdata:
+                for service in get_all_services():
+                    if service.get_id() in userdata.keys():
+                        return True
+
+                if "modules" not in userdata.keys():
+                    return True
+                return False
+        except Exception as e:
+            print(e)
+            return False
+
+    def migrate(self):
+        # Write info about providers to userdata.json
+        try:
+            migrate_services_to_modules()
+            print("Done")
+        except Exception as e:
+            print(e)
+            print("Error migrating service fields")

selfprivacy_api/services/service.py

@@ -136,7 +136,7 @@ class Service(ABC):
         """
         name = cls.get_id()
         with ReadUserData() as user_data:
-            return user_data.get(name, {}).get("enable", False)
+            return user_data.get("modules", {}).get(name, {}).get("enable", False)
 
     @staticmethod
     @abstractmethod
@@ -144,24 +144,25 @@ class Service(ABC):
         """The status of the service, reported by systemd."""
         pass
 
-    # But they do not really enable?
+    @classmethod
+    def _set_enable(cls, enable: bool):
+        name = cls.get_id()
+        with WriteUserData() as user_data:
+            if "modules" not in user_data:
+                user_data["modules"] = {}
+            if name not in user_data["modules"]:
+                user_data["modules"][name] = {}
+            user_data["modules"][name]["enable"] = enable
+
     @classmethod
     def enable(cls):
         """Enable the service. Usually this means enabling systemd unit."""
-        name = cls.get_id()
+        cls._set_enable(True)
-        with WriteUserData() as user_data:
-            if name not in user_data:
-                user_data[name] = {}
-            user_data[name]["enable"] = True
 
     @classmethod
     def disable(cls):
         """Disable the service. Usually this means disabling systemd unit."""
-        name = cls.get_id()
+        cls._set_enable(False)
-        with WriteUserData() as user_data:
-            if name not in user_data:
-                user_data[name] = {}
-            user_data[name]["enable"] = False
 
     @staticmethod
     @abstractmethod

tests/data/turned_on.json

@@ -1,15 +1,9 @@
 {
     "api": {"token": "TEST_TOKEN", "enableSwagger": false},
-    "bitwarden": {"enable": true},
     "databasePassword": "PASSWORD",
     "domain": "test.tld",
     "hashedMasterPassword": "HASHED_PASSWORD",
     "hostname": "test-instance",
-    "nextcloud": {
-        "adminPassword": "ADMIN",
-        "databasePassword": "ADMIN",
-        "enable": true
-    },
     "resticPassword": "PASS",
     "ssh": {
         "enable": true,
@@ -17,16 +11,24 @@
         "rootKeys": ["ssh-ed25519 KEY test@pc"]
     },
     "username": "tester",
-    "gitea": {"enable": true},
-    "ocserv": {"enable": true},
-    "pleroma": {"enable": true},
-    "jitsi": {"enable": true},
     "autoUpgrade": {"enable": true, "allowReboot": true},
     "useBinds": true,
     "timezone": "Europe/Moscow",
     "sshKeys": ["ssh-rsa KEY test@pc"],
     "dns": {"provider": "CLOUDFLARE", "apiKey": "TOKEN"},
     "server": {"provider": "HETZNER"},
+    "modules": {
+        "bitwarden": {"enable": true},
+        "gitea": {"enable": true},
+        "ocserv": {"enable": true},
+        "pleroma": {"enable": true},
+        "jitsi": {"enable": true},
+        "nextcloud": {
+            "adminPassword": "ADMIN",
+            "databasePassword": "ADMIN",
+            "enable": true
+        }
+    },
     "backup": {
         "provider": "BACKBLAZE",
         "accountId": "ID",

tests/test_graphql/test_system/turned_on.json

@@ -3,18 +3,10 @@
         "token": "TEST_TOKEN",
         "enableSwagger": false
     },
-    "bitwarden": {
-        "enable": true
-    },
     "databasePassword": "PASSWORD",
     "domain": "test.tld",
     "hashedMasterPassword": "HASHED_PASSWORD",
     "hostname": "test-instance",
-    "nextcloud": {
-        "adminPassword": "ADMIN",
-        "databasePassword": "ADMIN",
-        "enable": true
-    },
     "resticPassword": "PASS",
     "ssh": {
         "enable": true,
@@ -24,6 +16,7 @@
         ]
     },
     "username": "tester",
+    "modules": {
         "gitea": {
             "enable": true
         },
@@ -36,6 +29,15 @@
         "jitsi": {
             "enable": true
         },
+        "nextcloud": {
+            "adminPassword": "ADMIN",
+            "databasePassword": "ADMIN",
+            "enable": true
+        },
+        "bitwarden": {
+            "enable": true
+        }
+    },
     "autoUpgrade": {
         "enable": true,
         "allowReboot": true

tests/test_migrations.py

@@ -0,0 +1,60 @@
+import pytest
+
+from selfprivacy_api.migrations.modules_in_json import CreateModulesField
+from selfprivacy_api.utils import ReadUserData, WriteUserData
+from selfprivacy_api.services import get_all_services
+
+
+@pytest.fixture()
+def stray_services(mocker, datadir):
+    mocker.patch("selfprivacy_api.utils.USERDATA_FILE", new=datadir / "strays.json")
+    return datadir
+
+
+@pytest.fixture()
+def empty_json(generic_userdata):
+    with WriteUserData() as data:
+        data.clear()
+
+    with ReadUserData() as data:
+        assert len(data.keys()) == 0
+
+    return
+
+
+def test_modules_empty_json(empty_json):
+    with ReadUserData() as data:
+        assert "modules" not in data.keys()
+
+    assert CreateModulesField().is_migration_needed()
+
+    CreateModulesField().migrate()
+    assert not CreateModulesField().is_migration_needed()
+
+    with ReadUserData() as data:
+        assert "modules" in data.keys()
+
+
+@pytest.mark.parametrize("modules_field", [True, False])
+def test_modules_stray_services(modules_field, stray_services):
+    if not modules_field:
+        with WriteUserData() as data:
+            del data["modules"]
+    assert CreateModulesField().is_migration_needed()
+
+    CreateModulesField().migrate()
+
+    for service in get_all_services():
+        # assumes we do not tolerate previous format
+        assert service.is_enabled()
+        if service.get_id() == "email":
+            continue
+        with ReadUserData() as data:
+            assert service.get_id() in data["modules"].keys()
+            assert service.get_id() not in data.keys()
+
+    assert not CreateModulesField().is_migration_needed()
+
+
+def test_modules_no_migration_on_generic_data(generic_userdata):
+    assert not CreateModulesField().is_migration_needed()

tests/test_migrations/strays.json

@@ -0,0 +1,23 @@
+{
+    "bitwarden": {
+        "enable": true
+    },
+    "nextcloud": {
+        "adminPassword": "ADMIN",
+        "databasePassword": "ADMIN",
+        "enable": true
+    },
+    "gitea": {
+        "enable": true
+    },
+    "ocserv": {
+        "enable": true
+    },
+    "pleroma": {
+        "enable": true
+    },
+    "jitsi": {
+        "enable": true
+    },
+    "modules": {}
+}

tests/test_services.py

@@ -7,6 +7,8 @@ from pytest import raises
 from selfprivacy_api.utils import ReadUserData, WriteUserData
 from selfprivacy_api.utils.waitloop import wait_until_true
 
+import selfprivacy_api.services as services_module
+
 from selfprivacy_api.services.bitwarden import Bitwarden
 from selfprivacy_api.services.pleroma import Pleroma
 from selfprivacy_api.services.mailserver import MailServer
@@ -15,6 +17,7 @@ from selfprivacy_api.services.generic_service_mover import FolderMoveNames
 
 from selfprivacy_api.services.test_service import DummyService
 from selfprivacy_api.services.service import Service, ServiceStatus, StoppedService
+from selfprivacy_api.services import get_enabled_services
 
 from tests.test_dkim import domain_file, dkim_file, no_dkim_file
 
@@ -95,35 +98,49 @@ def test_foldermoves_from_ownedpaths():
 
 def test_enabling_disabling_reads_json(dummy_service: DummyService):
     with WriteUserData() as data:
-        data[dummy_service.get_id()]["enable"] = False
+        data["modules"][dummy_service.get_id()]["enable"] = False
     assert dummy_service.is_enabled() is False
     with WriteUserData() as data:
-        data[dummy_service.get_id()]["enable"] = True
+        data["modules"][dummy_service.get_id()]["enable"] = True
     assert dummy_service.is_enabled() is True
 
 
-@pytest.fixture(params=["normally_enabled", "deleted_attribute", "service_not_in_json"])
+# A helper to test undefined states. Used in fixtures below
+def undefine_service_enabled_status(param, dummy_service):
+    if param == "deleted_attribute":
+        with WriteUserData() as data:
+            del data["modules"][dummy_service.get_id()]["enable"]
+    if param == "service_not_in_json":
+        with WriteUserData() as data:
+            del data["modules"][dummy_service.get_id()]
+    if param == "modules_not_in_json":
+        with WriteUserData() as data:
+            del data["modules"]
+
+
+# May be defined or not
+@pytest.fixture(
+    params=[
+        "normally_enabled",
+        "deleted_attribute",
+        "service_not_in_json",
+        "modules_not_in_json",
+    ]
+)
 def possibly_dubiously_enabled_service(
     dummy_service: DummyService, request
 ) -> DummyService:
-    if request.param == "deleted_attribute":
+    if request.param != "normally_enabled":
-        with WriteUserData() as data:
+        undefine_service_enabled_status(request.param, dummy_service)
-            del data[dummy_service.get_id()]["enable"]
-    if request.param == "service_not_in_json":
-        with WriteUserData() as data:
-            del data[dummy_service.get_id()]
     return dummy_service
 
 
-# Yeah, idk yet how to dry it.
+# Strictly UNdefined
-@pytest.fixture(params=["deleted_attribute", "service_not_in_json"])
+@pytest.fixture(
+    params=["deleted_attribute", "service_not_in_json", "modules_not_in_json"]
+)
 def undefined_enabledness_service(dummy_service: DummyService, request) -> DummyService:
-    if request.param == "deleted_attribute":
+    undefine_service_enabled_status(request.param, dummy_service)
-        with WriteUserData() as data:
-            del data[dummy_service.get_id()]["enable"]
-    if request.param == "service_not_in_json":
-        with WriteUserData() as data:
-            del data[dummy_service.get_id()]
     return dummy_service
 
 
@@ -141,13 +158,13 @@ def test_enabling_disabling_writes_json(
 
     dummy_service.disable()
     with ReadUserData() as data:
-        assert data[dummy_service.get_id()]["enable"] is False
+        assert data["modules"][dummy_service.get_id()]["enable"] is False
     dummy_service.enable()
     with ReadUserData() as data:
-        assert data[dummy_service.get_id()]["enable"] is True
+        assert data["modules"][dummy_service.get_id()]["enable"] is True
     dummy_service.disable()
     with ReadUserData() as data:
-        assert data[dummy_service.get_id()]["enable"] is False
+        assert data["modules"][dummy_service.get_id()]["enable"] is False
 
 
 # more detailed testing of this is in test_graphql/test_system.py
@@ -158,3 +175,7 @@ def test_mailserver_with_dkim_returns_some_dns(dkim_file):
 
 def test_mailserver_with_no_dkim_returns_no_dns(no_dkim_file):
     assert MailServer().get_dns_records() == []
+
+
+def test_services_enabled_by_default(generic_userdata):
+    assert set(get_enabled_services()) == set(services_module.services)