diff --git a/ch_backup/backup/layout.py b/ch_backup/backup/layout.py index c8a0dc62..03a19439 100644 --- a/ch_backup/backup/layout.py +++ b/ch_backup/backup/layout.py @@ -120,7 +120,7 @@ def upload_access_control_file(self, backup_name: str, file_name: str) -> None: raise StorageError(msg) from e def upload_access_control_files( - self, path: str, backup_name: str, local_path: str, file_names: List[str] + self, local_path: str, backup_name: str, file_names: List[str] ) -> None: """ Upload access control list. @@ -131,7 +131,7 @@ def upload_access_control_files( try: logging.debug('Uploading access control data "{}"', local_path) self._storage_loader.upload_files_tarball( - path, + local_path, remote_path, files=file_names, encryption=True, diff --git a/ch_backup/logic/access.py b/ch_backup/logic/access.py index 3465f2ed..9e0e9d36 100644 --- a/ch_backup/logic/access.py +++ b/ch_backup/logic/access.py @@ -4,6 +4,7 @@ import os import re +import shutil from typing import Any, Dict, List, Sequence, Union from kazoo.client import KazooClient @@ -15,9 +16,7 @@ from ch_backup.logic.backup_manager import BackupManager from ch_backup.util import ( chown_dir_contents, - chown_path, copy_directory_content, - create_dir, temporary_directory, ) @@ -41,7 +40,9 @@ def backup(self, context: BackupContext) -> None: user = context.ch_ctl_conf["user"] group = context.ch_ctl_conf["group"] - create_dir(clickhouse_access_path, user, group) + os.makedirs(clickhouse_access_path, exist_ok=True) + shutil.chown(clickhouse_access_path, user, group) + with temporary_directory(backup_tmp_path, user, group): objects = context.ch_ctl.get_access_control_objects() context.backup_meta.set_access_control(objects) @@ -63,7 +64,6 @@ def backup(self, context: BackupContext) -> None: context.backup_layout.upload_access_control_files( backup_tmp_path, context.backup_meta.name, - backup_tmp_path, acl_file_names, ) @@ -87,20 +87,14 @@ def restore(self, context: BackupContext) -> None: user = context.ch_ctl_conf["user"] group = context.ch_ctl_conf["group"] - create_dir(clickhouse_access_path, user, group) + if os.path.exists(clickhouse_access_path): + shutil.rmtree(clickhouse_access_path) + + os.makedirs(clickhouse_access_path) + shutil.chown(clickhouse_access_path, user, group) + with temporary_directory(restore_tmp_path, user, group): - if ( - context.backup_meta.access_control.backup_format - == BackupStorageFormat.TAR - ): - context.backup_layout.download_access_control( - restore_tmp_path, context.backup_meta.name - ) - else: - for name in _get_access_control_files(acl_ids): - context.backup_layout.download_access_control_file( - restore_tmp_path, context.backup_meta.name, name - ) + self._download_access_control_list(context, restore_tmp_path, acl_ids) if has_replicated_access: self._restore_replicated(restore_tmp_path, acl_ids, acl_meta, context) @@ -178,6 +172,19 @@ def fix_admin_user(self, context: BackupContext, dry_run: bool = True) -> None: except FileNotFoundError: logging.debug(f"File {file_path} not found.") + def _download_access_control_list( + self, context: BackupContext, restore_tmp_path: str, acl_ids: List[str] + ) -> None: + if context.backup_meta.access_control.backup_format == BackupStorageFormat.TAR: + context.backup_layout.download_access_control( + restore_tmp_path, context.backup_meta.name + ) + else: + for name in _get_access_control_files(acl_ids): + context.backup_layout.download_access_control_file( + restore_tmp_path, context.backup_meta.name, name + ) + def _clean_user_uuid(self, raw_str: str) -> str: return re.sub(r"EXCEPT ID\('(.+)'\)", "", raw_str) @@ -263,7 +270,7 @@ def _mark_to_rebuild( mark_file = os.path.join(clickhouse_access_path, CH_MARK_FILE) with open(mark_file, "a", encoding="utf-8"): pass - chown_path(mark_file, user, group) + shutil.chown(mark_file, user, group) def _has_replicated_access(self, context: BackupContext) -> bool: return ( diff --git a/ch_backup/util.py b/ch_backup/util.py index d1026fe4..b187191c 100644 --- a/ch_backup/util.py +++ b/ch_backup/util.py @@ -466,6 +466,7 @@ def __eq__(self, other): if not getattr(self, slot) == getattr(other, slot): return False return True + def chown_path(path: str, user: str, group: str) -> None: """ Interface for chown @@ -480,18 +481,14 @@ def copy_directory_content(from_path_dir: str, to_path_dir: str) -> None: if to_path_dir[-1] != "/": to_path_dir += "/" for subpath in os.listdir(from_path_dir): - subpath = os.path.join(from_path_dir, subpath) - shutil.copy(subpath, to_path_dir) - - -def create_dir(path: str, user: str, group: str) -> None: - """ - Create dir and chown it. - """ - os.makedirs(path, exist_ok=True) - chown_path(path, user, group) + subpath_from = os.path.join(from_path_dir, subpath) + subpath_to = os.path.join(to_path_dir, subpath) + if not os.path.exists(subpath_to): + shutil.copy(subpath_from, to_path_dir) +# tempfile.TemporaryDirectory : https://docs.python.org/3.11/library/tempfile.html#tempfile.TemporaryDirectory +# It is not possible to keep directory content only when exception occurs. class temporary_directory: """ Class to automatically create and remove temporary directory. @@ -505,10 +502,19 @@ def __init__(self, path: str, user: str, group: str): self._group = group def __enter__(self) -> None: - create_dir(self._path, self._user, self._group) + if os.path.exists(self._path): + shutil.rmtree(self._path) + + os.makedirs(self._path) + shutil.chown(self._path, self._user, self._group) def __exit__(self, exc_type, exc_value, traceback): if exc_type is not None: - logging.warning("Dont remove tmp dir {} due to exception.", self._path) + logging.warning( + "Dont remove tmp dir {} due to exception. {}: {}", + self._path, + exc_type.__name__, + exc_value, + ) return shutil.rmtree(self._path)