From 59c5e58549b6b03d112f88760df0e3ec8d03a2a7 Mon Sep 17 00:00:00 2001 From: Sunil Mohan Adapa Date: Sat, 5 Feb 2022 17:29:42 -0800 Subject: [PATCH] backups: Implement backup/restore of key/value settings - Implemented within the backup component. Scope for implementing database backup/restore in similar way. - Add new 'settings' key in the backup manifest to allow keys to backed up and restored. - Implement by dumping/loading settings from DB into the file. Tests: - Unit tests. - Backup/restore tests for dynamicdns workss. Signed-off-by: Sunil Mohan Adapa Reviewed-by: James Valleroy --- actions/backups | 39 ++++++++++ plinth/modules/backups/components.py | 78 ++++++++++++++++++- .../modules/backups/tests/test_components.py | 68 +++++++++++++++- plinth/modules/gitweb/__init__.py | 1 + plinth/modules/samba/__init__.py | 2 + plinth/modules/ttrss/__init__.py | 2 + plinth/modules/wordpress/__init__.py | 2 + plinth/modules/zoph/__init__.py | 2 + 8 files changed, 188 insertions(+), 6 deletions(-) diff --git a/actions/backups b/actions/backups index a8d13118c..e83fa8b85 100755 --- a/actions/backups +++ b/actions/backups @@ -8,6 +8,8 @@ Wrapper to handle backups using borg-backups. import argparse import json import os +import pathlib +import re import subprocess import sys import tarfile @@ -16,6 +18,7 @@ from plinth.modules.backups import MANIFESTS_FOLDER from plinth.utils import Version TIMEOUT = 30 +BACKUPS_DATA_PATH = pathlib.Path('/var/lib/plinth/backups-data/') def parse_arguments(): @@ -78,6 +81,16 @@ def parse_arguments(): restore_exported_archive.add_argument('--path', help='Tarball file path', required=True) + dump_settings = subparsers.add_parser('dump-settings', + help='Dump JSON settings to a file') + dump_settings.add_argument('--app-id', + help='ID of the app to dump settings for') + + load_settings = subparsers.add_parser( + 'load-settings', help='Load JSON settings from a file') + load_settings.add_argument('--app-id', + help='ID of the app to load settings for') + subparsers.required = True return parser.parse_args() @@ -267,6 +280,32 @@ def subcommand_restore_exported_archive(arguments): break +def _assert_app_id(app_id): + """Check that app ID is correct.""" + if not re.fullmatch(r'[a-z0-9_]+', app_id): + raise Exception('Invalid App ID') + + +def subcommand_dump_settings(arguments): + """Dump an app's settings to a JSON file.""" + app_id = arguments.app_id + _assert_app_id(app_id) + BACKUPS_DATA_PATH.mkdir(exist_ok=True) + settings_path = BACKUPS_DATA_PATH / f'{app_id}-settings.json' + settings_path.write_text(arguments.stdin) + + +def subcommand_load_settings(arguments): + """Load an app's settings from a JSON file.""" + app_id = arguments.app_id + _assert_app_id(app_id) + settings_path = BACKUPS_DATA_PATH / f'{app_id}-settings.json' + try: + print(settings_path.read_text()) + except FileNotFoundError: + print('{}') + + def _read_encryption_passphrase(arguments): """Read encryption passphrase from stdin.""" if arguments.stdin: diff --git a/plinth/modules/backups/components.py b/plinth/modules/backups/components.py index 3fadf8e82..45165e856 100644 --- a/plinth/modules/backups/components.py +++ b/plinth/modules/backups/components.py @@ -3,7 +3,10 @@ App component for other apps to use backup/restore functionality. """ -from plinth import app +import copy +import json + +from plinth import actions, app def _validate_directories_and_files(section): @@ -45,29 +48,52 @@ def _validate_service(service): assert service['kind'] in ('config', 'site', 'module') +def _validate_settings(settings): + """Validate settings stored by an in kvstore.""" + if not settings: + return + + assert isinstance(settings, list) + for setting in settings: + assert isinstance(setting, str) + + class BackupRestore(app.FollowerComponent): """Component to backup/restore an app.""" def __init__(self, component_id, config=None, data=None, secrets=None, - services=None): + services=None, settings=None): """Initialize the backup/restore component.""" super().__init__(component_id) _validate_directories_and_files(config) self.config = config or {} _validate_directories_and_files(data) - self.data = data or {} + self._data = data or {} _validate_directories_and_files(secrets) self.secrets = secrets or {} _validate_services(services) self.services = services or [] + _validate_settings(settings) + self.settings = settings or [] - self.has_data = bool(config) or bool(data) or bool(secrets) + self.has_data = (bool(config) or bool(data) or bool(secrets) + or bool(settings)) def __eq__(self, other): """Check if this component is same as another.""" return self.component_id == other.component_id + @property + def data(self): + """Add additional files to data files list.""" + data = copy.deepcopy(self._data) + settings_file = self._get_settings_file() + if settings_file: + data.setdefault('files', []).append(settings_file) + + return data + @property def manifest(self): """Return the backup details as a dictionary.""" @@ -84,10 +110,14 @@ class BackupRestore(app.FollowerComponent): if self.services: manifest['services'] = self.services + if self.settings: + manifest['settings'] = self.settings + return manifest def backup_pre(self, packet): """Perform any special operations before backup.""" + self._settings_backup_pre() def backup_post(self, packet): """Perform any special operations after backup.""" @@ -97,3 +127,43 @@ class BackupRestore(app.FollowerComponent): def restore_post(self, packet): """Perform any special operations after restore.""" + self._settings_restore_post() + + def _get_settings_file(self): + """Return the settings file path to list of files to backup.""" + if not self.settings or not self.app_id: + return None + + data_path = '/var/lib/plinth/backups-data/' + return data_path + f'{self.app_id}-settings.json' + + def _settings_backup_pre(self): + """Read keys from kvstore and store them in a file to backup.""" + if not self.settings: + return + + from plinth import kvstore + data = {} + for key in self.settings: + try: + data[key] = kvstore.get(key) + except Exception: + pass + + input_ = json.dumps(data).encode() + actions.superuser_run('backups', + ['dump-settings', '--app-id', self.app_id], + input=input_) + + def _settings_restore_post(self): + """Read from a file and restore keys to kvstore.""" + if not self.settings: + return + + output = actions.superuser_run( + 'backups', ['load-settings', '--app-id', self.app_id]) + data = json.loads(output) + + from plinth import kvstore + for key, value in data.items(): + kvstore.set(key, value) diff --git a/plinth/modules/backups/tests/test_components.py b/plinth/modules/backups/tests/test_components.py index d1a0df774..29a88d91d 100644 --- a/plinth/modules/backups/tests/test_components.py +++ b/plinth/modules/backups/tests/test_components.py @@ -3,8 +3,13 @@ Test the App components provides by backups app. """ +import json +from unittest.mock import call, patch + import pytest +from plinth import kvstore + from .. import components from ..components import BackupRestore @@ -16,8 +21,9 @@ def fixture_backup_restore(): """Fixture to create a domain type after clearing all existing ones.""" value = {'files': ['a', 'b'], 'directories': ['a', 'b']} services = ['service-1', {'type': 'system', 'name': 'service-2'}] + settings = ['setting-1', 'setting-2'] return BackupRestore('test-backup-restore', config=value, data=value, - secrets=value, services=services) + secrets=value, services=services, settings=settings) @pytest.mark.parametrize('section', [ @@ -159,6 +165,7 @@ def test_backup_restore_init_default_arguments(): assert component.data == {} assert component.secrets == {} assert component.services == [] + assert component.settings == [] assert not component.has_data @@ -185,6 +192,22 @@ def test_backup_restore_init_services(): assert not component.has_data +def test_backup_restore_init_settings(): + """Test initialization of the backup restore object.""" + with pytest.raises(AssertionError): + BackupRestore('test-backup-restore', settings='invalid-value') + + settings = ['setting1', 'setting2'] + component = BackupRestore('test-backup-restore', settings=settings) + assert component.settings == settings + assert component.has_data + assert component.data == {} + + component.app_id = 'testapp' + settings_file = '/var/lib/plinth/backups-data/testapp-settings.json' + assert component.data == {'files': [settings_file]} + + def test_backup_restore_equal(backup_restore): """Test equality operator on the backup restore object.""" assert backup_restore == BackupRestore('test-backup-restore') @@ -199,6 +222,7 @@ def test_backup_restore_manifest(backup_restore): assert manifest['data'] == backup_restore.data assert manifest['secrets'] == backup_restore.secrets assert manifest['services'] == backup_restore.services + assert manifest['settings'] == backup_restore.settings assert BackupRestore('test-backup-restore').manifest == {} @@ -206,7 +230,47 @@ def test_backup_restore_manifest(backup_restore): def test_backup_restore_hooks(backup_restore): """Test running hooks on backup restore object.""" packet = None - backup_restore.backup_pre(packet) backup_restore.backup_post(packet) backup_restore.restore_pre(packet) + + +@pytest.mark.django_db +@patch('plinth.actions.superuser_run') +def test_backup_restore_backup_pre(run, backup_restore): + """Test running backup-pre hook.""" + packet = None + kvstore.set('setting-1', 'value-1') + backup_restore.app_id = 'testapp' + + component = BackupRestore('test-backup-restore') + component.backup_pre(packet) + run.assert_has_calls([]) + + backup_restore.backup_pre(packet) + input_ = {'setting-1': 'value-1'} + run.assert_has_calls([ + call('backups', ['dump-settings', '--app-id', 'testapp'], + input=json.dumps(input_).encode()) + ]) + + +@pytest.mark.django_db +@patch('plinth.actions.superuser_run') +def test_backup_restore_restore_post(run, backup_restore): + """Test running restore-post hook.""" + packet = None + backup_restore.app_id = 'testapp' + + component = BackupRestore('test-backup-restore') + component.restore_post(packet) + run.assert_has_calls([]) + + output = {'setting-1': 'value-1'} + run.return_value = json.dumps(output) backup_restore.restore_post(packet) + run.assert_has_calls( + [call('backups', ['load-settings', '--app-id', 'testapp'])]) + + assert kvstore.get('setting-1') == 'value-1' + with pytest.raises(Exception): + kvstore.get('setting-2') diff --git a/plinth/modules/gitweb/__init__.py b/plinth/modules/gitweb/__init__.py index bd0f6bf4a..e513414a0 100644 --- a/plinth/modules/gitweb/__init__.py +++ b/plinth/modules/gitweb/__init__.py @@ -153,6 +153,7 @@ class GitwebBackupRestore(BackupRestore): def restore_post(self, packet): """Update access after restoration of backups.""" + super().restore_post(packet) app.update_service_access() diff --git a/plinth/modules/samba/__init__.py b/plinth/modules/samba/__init__.py index b0baee393..c40c984ab 100644 --- a/plinth/modules/samba/__init__.py +++ b/plinth/modules/samba/__init__.py @@ -107,10 +107,12 @@ class SambaBackupRestore(BackupRestore): def backup_pre(self, packet): """Save registry share configuration.""" + super().backup_pre(packet) actions.superuser_run('samba', ['dump-shares']) def restore_post(self, packet): """Restore configuration.""" + super().restore_post(packet) actions.superuser_run('samba', ['setup']) actions.superuser_run('samba', ['restore-shares']) diff --git a/plinth/modules/ttrss/__init__.py b/plinth/modules/ttrss/__init__.py index fb90ad392..be2926be4 100644 --- a/plinth/modules/ttrss/__init__.py +++ b/plinth/modules/ttrss/__init__.py @@ -112,10 +112,12 @@ class TTRSSBackupRestore(BackupRestore): def backup_pre(self, packet): """Save database contents.""" + super().backup_pre(packet) actions.superuser_run('ttrss', ['dump-database']) def restore_post(self, packet): """Restore database contents.""" + super().restore_post(packet) actions.superuser_run('ttrss', ['restore-database']) diff --git a/plinth/modules/wordpress/__init__.py b/plinth/modules/wordpress/__init__.py index f4061acc8..54571145a 100644 --- a/plinth/modules/wordpress/__init__.py +++ b/plinth/modules/wordpress/__init__.py @@ -113,10 +113,12 @@ class WordPressBackupRestore(BackupRestore): def backup_pre(self, packet): """Save database contents.""" + super().backup_pre(packet) actions.superuser_run('wordpress', ['dump-database']) def restore_post(self, packet): """Restore database contents.""" + super().restore_post(packet) actions.superuser_run('wordpress', ['restore-database']) diff --git a/plinth/modules/zoph/__init__.py b/plinth/modules/zoph/__init__.py index c1486b05e..45991df18 100644 --- a/plinth/modules/zoph/__init__.py +++ b/plinth/modules/zoph/__init__.py @@ -126,8 +126,10 @@ class ZophBackupRestore(BackupRestore): def backup_pre(self, packet): """Save database contents.""" + super().backup_pre(packet) actions.superuser_run('zoph', ['dump-database']) def restore_post(self, packet): """Restore database contents.""" + super().restore_post(packet) actions.superuser_run('zoph', ['restore-database'])