From c6306631e68e8d6b1fb59cd9b0a45494aa6171ca Mon Sep 17 00:00:00 2001 From: Evgenii Alekseev Date: Thu, 10 Jul 2025 18:41:23 +0300 Subject: [PATCH] fix: careful handling of file permissions during initialization It has been found that during cold start (e.g. in docker container), some permissions are invalid. In order to handle that, some operations are not guarded with RepositoryPaths.preserve_root guard In addition, it has been also found that in some cases (e.g. web server start) migrations are performed on empty repository identifier which may lead to wrong data (see also 435375721d419469b7283e509f98458cbe85a1b4), as well as some unexpected results during database operations. In order to handle that, now all watcher instances have their own databases (and configurations) --- recipes/check/compose.yml | 2 +- recipes/distributed-manual/compose.yml | 2 +- recipes/distributed/compose.yml | 4 +- recipes/i686/compose.yml | 2 +- recipes/multirepo/compose.yml | 4 +- recipes/oauth/compose.yml | 2 +- recipes/sign/compose.yml | 2 +- recipes/web/compose.yml | 2 +- src/ahriman/application/handlers/setup.py | 20 ++--- src/ahriman/core/alpm/pacman.py | 4 +- .../migrations/m011_repository_name.py | 2 - src/ahriman/core/database/sqlite.py | 8 +- src/ahriman/models/repository_paths.py | 88 ++++++++++++++----- src/ahriman/web/web.py | 10 ++- .../handlers/test_handler_setup.py | 4 +- tests/ahriman/core/alpm/test_pacman.py | 4 +- .../migrations/test_m011_repository_name.py | 11 --- tests/ahriman/core/database/test_sqlite.py | 11 +++ tests/ahriman/models/test_repository_paths.py | 60 +++++++++---- 19 files changed, 162 insertions(+), 80 deletions(-) diff --git a/recipes/check/compose.yml b/recipes/check/compose.yml index 0a680310..25b127c7 100644 --- a/recipes/check/compose.yml +++ b/recipes/check/compose.yml @@ -8,7 +8,7 @@ services: AHRIMAN_OUTPUT: console AHRIMAN_PASSWORD: ${AHRIMAN_PASSWORD} AHRIMAN_PORT: 8080 - AHRIMAN_PRESETUP_COMMAND: (cat /run/secrets/password; echo; cat /run/secrets/password) | sudo -u ahriman ahriman user-add demo -R full + AHRIMAN_POSTSETUP_COMMAND: (cat /run/secrets/password; echo; cat /run/secrets/password) | sudo -u ahriman ahriman user-add demo -R full AHRIMAN_REPOSITORY: ahriman-demo AHRIMAN_UNIX_SOCKET: /var/lib/ahriman/ahriman/ahriman.sock diff --git a/recipes/distributed-manual/compose.yml b/recipes/distributed-manual/compose.yml index f6d82e58..0e29d267 100644 --- a/recipes/distributed-manual/compose.yml +++ b/recipes/distributed-manual/compose.yml @@ -8,7 +8,7 @@ services: AHRIMAN_OUTPUT: console AHRIMAN_PASSWORD: ${AHRIMAN_PASSWORD} AHRIMAN_PORT: 8080 - AHRIMAN_PRESETUP_COMMAND: (cat /run/secrets/password; echo; cat /run/secrets/password) | sudo -u ahriman ahriman user-add demo -R full + AHRIMAN_POSTSETUP_COMMAND: (cat /run/secrets/password; echo; cat /run/secrets/password) | sudo -u ahriman ahriman user-add demo -R full AHRIMAN_REPOSITORY: ahriman-demo AHRIMAN_UNIX_SOCKET: /var/lib/ahriman/ahriman/ahriman.sock diff --git a/recipes/distributed/compose.yml b/recipes/distributed/compose.yml index e510f023..fa5cda03 100644 --- a/recipes/distributed/compose.yml +++ b/recipes/distributed/compose.yml @@ -8,7 +8,7 @@ services: AHRIMAN_OUTPUT: console AHRIMAN_PASSWORD: ${AHRIMAN_PASSWORD} AHRIMAN_PORT: 8080 - AHRIMAN_PRESETUP_COMMAND: (cat /run/secrets/password; echo; cat /run/secrets/password) | sudo -u ahriman ahriman user-add demo -R full + AHRIMAN_POSTSETUP_COMMAND: (cat /run/secrets/password; echo; cat /run/secrets/password) | sudo -u ahriman ahriman user-add demo -R full AHRIMAN_REPOSITORY: ahriman-demo AHRIMAN_UNIX_SOCKET: /var/lib/ahriman/ahriman/ahriman.sock @@ -62,7 +62,7 @@ services: AHRIMAN_OUTPUT: console AHRIMAN_PASSWORD: ${AHRIMAN_PASSWORD} AHRIMAN_PORT: 8080 - AHRIMAN_PRESETUP_COMMAND: (cat /run/secrets/password; echo; cat /run/secrets/password) | sudo -u ahriman ahriman user-add demo -R full + AHRIMAN_POSTSETUP_COMMAND: (cat /run/secrets/password; echo; cat /run/secrets/password) | sudo -u ahriman ahriman user-add demo -R full AHRIMAN_REPOSITORY: ahriman-demo AHRIMAN_REPOSITORY_SERVER: http://frontend/repo/$$repo/$$arch diff --git a/recipes/i686/compose.yml b/recipes/i686/compose.yml index ed3abadf..f457cf02 100644 --- a/recipes/i686/compose.yml +++ b/recipes/i686/compose.yml @@ -12,7 +12,7 @@ services: AHRIMAN_PACMAN_MIRROR: https://de.mirror.archlinux32.org/$$arch/$$repo AHRIMAN_PASSWORD: ${AHRIMAN_PASSWORD} AHRIMAN_PORT: 8080 - AHRIMAN_PRESETUP_COMMAND: (cat /run/secrets/password; echo; cat /run/secrets/password) | sudo -u ahriman ahriman user-add demo -R full + AHRIMAN_POSTSETUP_COMMAND: (cat /run/secrets/password; echo; cat /run/secrets/password) | sudo -u ahriman ahriman user-add demo -R full AHRIMAN_REPOSITORY: ahriman-demo AHRIMAN_UNIX_SOCKET: /var/lib/ahriman/ahriman/ahriman.sock diff --git a/recipes/multirepo/compose.yml b/recipes/multirepo/compose.yml index 77003170..1807b740 100644 --- a/recipes/multirepo/compose.yml +++ b/recipes/multirepo/compose.yml @@ -8,8 +8,8 @@ services: AHRIMAN_OUTPUT: console AHRIMAN_PASSWORD: ${AHRIMAN_PASSWORD} AHRIMAN_PORT: 8080 - AHRIMAN_POSTSETUP_COMMAND: ahriman --architecture x86_64 --repository another-demo service-setup --build-as-user ahriman --packager 'ahriman bot ' - AHRIMAN_PRESETUP_COMMAND: (cat /run/secrets/password; echo; cat /run/secrets/password) | sudo -u ahriman ahriman user-add demo -R full + AHRIMAN_POSTSETUP_COMMAND: (cat /run/secrets/password; echo; cat /run/secrets/password) | sudo -u ahriman ahriman user-add demo -R full + AHRIMAN_PRESETUP_COMMAND: ahriman --architecture x86_64 --repository another-demo service-setup --build-as-user ahriman --packager 'ahriman bot ' AHRIMAN_REPOSITORY: ahriman-demo AHRIMAN_UNIX_SOCKET: /var/lib/ahriman/ahriman/ahriman.sock diff --git a/recipes/oauth/compose.yml b/recipes/oauth/compose.yml index d62a37ea..328706ec 100644 --- a/recipes/oauth/compose.yml +++ b/recipes/oauth/compose.yml @@ -9,7 +9,7 @@ services: AHRIMAN_OAUTH_CLIENT_SECRET: ${AHRIMAN_OAUTH_CLIENT_SECRET} AHRIMAN_OUTPUT: console AHRIMAN_PORT: 8080 - AHRIMAN_PRESETUP_COMMAND: sudo -u ahriman ahriman user-add ${AHRIMAN_OAUTH_USER} -R full -p "" + AHRIMAN_POSTSETUP_COMMAND: sudo -u ahriman ahriman user-add ${AHRIMAN_OAUTH_USER} -R full -p "" AHRIMAN_REPOSITORY: ahriman-demo AHRIMAN_UNIX_SOCKET: /var/lib/ahriman/ahriman/ahriman.sock diff --git a/recipes/sign/compose.yml b/recipes/sign/compose.yml index c93979cd..318a07e1 100644 --- a/recipes/sign/compose.yml +++ b/recipes/sign/compose.yml @@ -6,7 +6,7 @@ services: environment: AHRIMAN_DEBUG: yes AHRIMAN_OUTPUT: console - AHRIMAN_PRESETUP_COMMAND: sudo -u ahriman gpg --import /run/secrets/key + AHRIMAN_POSTSETUP_COMMAND: sudo -u ahriman gpg --import /run/secrets/key AHRIMAN_REPOSITORY: ahriman-demo configs: diff --git a/recipes/web/compose.yml b/recipes/web/compose.yml index 2030617c..45247968 100644 --- a/recipes/web/compose.yml +++ b/recipes/web/compose.yml @@ -8,7 +8,7 @@ services: AHRIMAN_OUTPUT: console AHRIMAN_PASSWORD: ${AHRIMAN_PASSWORD} AHRIMAN_PORT: 8080 - AHRIMAN_PRESETUP_COMMAND: (cat /run/secrets/password; echo; cat /run/secrets/password) | sudo -u ahriman ahriman user-add demo -R full + AHRIMAN_POSTSETUP_COMMAND: (cat /run/secrets/password; echo; cat /run/secrets/password) | sudo -u ahriman ahriman user-add demo -R full AHRIMAN_REPOSITORY: ahriman-demo AHRIMAN_UNIX_SOCKET: /var/lib/ahriman/ahriman/ahriman.sock diff --git a/src/ahriman/application/handlers/setup.py b/src/ahriman/application/handlers/setup.py index a00114fd..a7aa9129 100644 --- a/src/ahriman/application/handlers/setup.py +++ b/src/ahriman/application/handlers/setup.py @@ -72,16 +72,17 @@ class Setup(Handler): application = Application(repository_id, configuration, report=report) - Setup.configuration_create_makepkg(args.packager, args.makeflags_jobs, application.repository.paths) - Setup.executable_create(application.repository.paths, repository_id) - repository_server = f"file://{application.repository.paths.repository}" if args.server is None else args.server - Setup.configuration_create_devtools( - repository_id, args.from_configuration, args.mirror, args.multilib, repository_server) - Setup.configuration_create_sudo(application.repository.paths, repository_id) + with application.repository.paths.preserve_owner(): + Setup.configuration_create_makepkg(args.packager, args.makeflags_jobs, application.repository.paths) + Setup.executable_create(application.repository.paths, repository_id) + repository_server = f"file://{application.repository.paths.repository}" if args.server is None else args.server + Setup.configuration_create_devtools( + repository_id, args.from_configuration, args.mirror, args.multilib, repository_server) + Setup.configuration_create_sudo(application.repository.paths, repository_id) - application.repository.repo.init() - # lazy database sync - application.repository.pacman.handle # pylint: disable=pointless-statement + application.repository.repo.init() + # lazy database sync + application.repository.pacman.handle # pylint: disable=pointless-statement @staticmethod def _set_service_setup_parser(root: SubParserAction) -> argparse.ArgumentParser: @@ -280,6 +281,5 @@ class Setup(Handler): command = Setup.build_command(paths.root, repository_id) command.unlink(missing_ok=True) command.symlink_to(Setup.ARCHBUILD_COMMAND_PATH) - paths.chown(command) # we would like to keep owner inside ahriman's home arguments = [_set_service_setup_parser] diff --git a/src/ahriman/core/alpm/pacman.py b/src/ahriman/core/alpm/pacman.py index e7f6cd0a..d5ab1753 100644 --- a/src/ahriman/core/alpm/pacman.py +++ b/src/ahriman/core/alpm/pacman.py @@ -130,8 +130,8 @@ class Pacman(LazyLogging): return # database for some reason deos not exist self.logger.info("copy pacman database %s from operating system root to ahriman's home %s", src, dst) - shutil.copy(src, dst) - self.repository_paths.chown(dst) + with self.repository_paths.preserve_owner(dst.parent): + shutil.copy(src, dst) def database_init(self, handle: Handle, repository: str, architecture: str) -> DB: """ diff --git a/src/ahriman/core/database/migrations/m011_repository_name.py b/src/ahriman/core/database/migrations/m011_repository_name.py index cedb077c..a567fe56 100644 --- a/src/ahriman/core/database/migrations/m011_repository_name.py +++ b/src/ahriman/core/database/migrations/m011_repository_name.py @@ -203,8 +203,6 @@ def migrate_package_repository(connection: Connection, configuration: Configurat configuration(Configuration): configuration instance """ _, repository_id = configuration.check_loaded() - if repository_id.is_empty: - return # no repository available yet connection.execute("""update build_queue set repository = :repository""", {"repository": repository_id.id}) connection.execute("""update package_bases set repository = :repository""", {"repository": repository_id.id}) diff --git a/src/ahriman/core/database/sqlite.py b/src/ahriman/core/database/sqlite.py index e9093625..46759679 100644 --- a/src/ahriman/core/database/sqlite.py +++ b/src/ahriman/core/database/sqlite.py @@ -94,9 +94,13 @@ class SQLite( sqlite3.register_adapter(list, json.dumps) sqlite3.register_converter("json", json.loads) - if self._configuration.getboolean("settings", "apply_migrations", fallback=True): + if not self._configuration.getboolean("settings", "apply_migrations", fallback=True): + return + if self._repository_id.is_empty: + return # do not perform migration on empty repository identifier (e.g. multirepo command) + + with self._repository_paths.preserve_owner(): self.with_connection(lambda connection: Migrations.migrate(connection, self._configuration)) - self._repository_paths.chown(self.path) def package_clear(self, package_base: str, repository_id: RepositoryId | None = None) -> None: """ diff --git a/src/ahriman/models/repository_paths.py b/src/ahriman/models/repository_paths.py index d20dff70..7e8ae558 100644 --- a/src/ahriman/models/repository_paths.py +++ b/src/ahriman/models/repository_paths.py @@ -17,6 +17,7 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . # +import contextlib import os import shutil @@ -221,22 +222,14 @@ class RepositoryPaths(LazyLogging): stat = path.stat() return stat.st_uid, stat.st_gid - def cache_for(self, package_base: str) -> Path: - """ - get path to cached PKGBUILD and package sources for the package base - - Args: - package_base(str): package base name - - Returns: - Path: full path to directory for specified package base cache - """ - return self.cache / package_base - - def chown(self, path: Path) -> None: + def _chown(self, path: Path) -> None: """ set owner of path recursively (from root) to root owner + Notes: + More likely you don't want to call this method explicitly, consider using :func:`preserve_owner` + as context manager instead + Args: path(Path): path to be chown @@ -256,6 +249,56 @@ class RepositoryPaths(LazyLogging): set_owner(path) path = path.parent + def cache_for(self, package_base: str) -> Path: + """ + get path to cached PKGBUILD and package sources for the package base + + Args: + package_base(str): package base name + + Returns: + Path: full path to directory for specified package base cache + """ + return self.cache / package_base + + @contextlib.contextmanager + def preserve_owner(self, path: Path | None = None) -> Generator[None, None, None]: + """ + perform any action preserving owner for any newly created file or directory + + Args: + path(Path | None, optional): use this path as root instead of repository root (Default value = None) + + Examples: + This method is designed to use as context manager when you are going to perform operations which might + change filesystem, especially if you are doing it under unsafe flag, e.g.:: + + >>> with paths.preserve_owner(): + >>> paths.tree_create() + + Note, however, that this method doesn't handle any exceptions and will eventually interrupt + if there will be any. + """ + path = path or self.root + + def walk(root: Path) -> Generator[Path, None, None]: + # basically walk, but skipping some content + for child in root.iterdir(): + yield child + if child in (self.chroot.parent,): + yield from child.iterdir() # we only yield top-level in chroot directory + elif child.is_dir(): + yield from walk(child) + + # get current filesystem and run action + previous_snapshot = set(walk(path)) + yield + + # get newly created files and directories and chown them + new_entries = set(walk(path)).difference(previous_snapshot) + for entry in new_entries: + self._chown(entry) + def tree_clear(self, package_base: str) -> None: """ clear package specific files @@ -274,12 +317,13 @@ class RepositoryPaths(LazyLogging): """ if self.repository_id.is_empty: return # do not even try to create tree in case if no repository id set - for directory in ( - self.cache, - self.chroot, - self.packages, - self.pacman, - self.repository, - ): - directory.mkdir(mode=0o755, parents=True, exist_ok=True) - self.chown(directory) + + with self.preserve_owner(): + for directory in ( + self.cache, + self.chroot, + self.packages, + self.pacman, + self.repository, + ): + directory.mkdir(mode=0o755, parents=True, exist_ok=True) diff --git a/src/ahriman/web/web.py b/src/ahriman/web/web.py index 547023d3..f062cb41 100644 --- a/src/ahriman/web/web.py +++ b/src/ahriman/web/web.py @@ -166,11 +166,16 @@ def setup_server(configuration: Configuration, spawner: Spawn, repositories: lis # package cache if not repositories: raise InitializeError("No repositories configured, exiting") - database = SQLite.load(configuration) watchers: dict[RepositoryId, Watcher] = {} + configuration_path, _ = configuration.check_loaded() for repository_id in repositories: application.logger.info("load repository %s", repository_id) - client = Client.load(repository_id, configuration, database, report=False) # explicitly load local client + # load settings explicitly for architecture if any + repository_configuration = Configuration.from_path(configuration_path, repository_id) + # load database instance, because it holds identifier + database = SQLite.load(repository_configuration) + # explicitly load local client + client = Client.load(repository_id, repository_configuration, database, report=False) watchers[repository_id] = Watcher(client) application[WatcherKey] = watchers # workers cache @@ -179,6 +184,7 @@ def setup_server(configuration: Configuration, spawner: Spawn, repositories: lis application[SpawnKey] = spawner application.logger.info("setup authorization") + database = SQLite.load(configuration) validator = application[AuthKey] = Auth.load(configuration, database) if validator.enabled: from ahriman.web.middlewares.auth_handler import setup_auth diff --git a/tests/ahriman/application/handlers/test_handler_setup.py b/tests/ahriman/application/handlers/test_handler_setup.py index ec4aedd7..17f14661 100644 --- a/tests/ahriman/application/handlers/test_handler_setup.py +++ b/tests/ahriman/application/handlers/test_handler_setup.py @@ -58,9 +58,11 @@ def test_run(args: argparse.Namespace, configuration: Configuration, repository: sudo_configuration_mock = mocker.patch("ahriman.application.handlers.setup.Setup.configuration_create_sudo") executable_mock = mocker.patch("ahriman.application.handlers.setup.Setup.executable_create") init_mock = mocker.patch("ahriman.core.alpm.repo.Repo.init") + owner_guard_mock = mocker.patch("ahriman.models.repository_paths.RepositoryPaths.preserve_owner") _, repository_id = configuration.check_loaded() Setup.run(args, repository_id, configuration, report=False) + owner_guard_mock.assert_called_once_with() ahriman_configuration_mock.assert_called_once_with(args, repository_id, configuration) devtools_configuration_mock.assert_called_once_with( repository_id, args.from_configuration, args.mirror, args.multilib, f"file://{repository_paths.repository}") @@ -268,13 +270,11 @@ def test_executable_create(configuration: Configuration, repository_paths: Repos """ must create executable """ - chown_mock = mocker.patch("ahriman.models.repository_paths.RepositoryPaths.chown") symlink_mock = mocker.patch("pathlib.Path.symlink_to") unlink_mock = mocker.patch("pathlib.Path.unlink") _, repository_id = configuration.check_loaded() Setup.executable_create(repository_paths, repository_id) - chown_mock.assert_called_once_with(Setup.build_command(repository_paths.root, repository_id)) symlink_mock.assert_called_once_with(Setup.ARCHBUILD_COMMAND_PATH) unlink_mock.assert_called_once_with(missing_ok=True) diff --git a/tests/ahriman/core/alpm/test_pacman.py b/tests/ahriman/core/alpm/test_pacman.py index 76c6db49..b14c4881 100644 --- a/tests/ahriman/core/alpm/test_pacman.py +++ b/tests/ahriman/core/alpm/test_pacman.py @@ -62,12 +62,12 @@ def test_database_copy(pacman: Pacman, mocker: MockerFixture) -> None: mocker.patch("pathlib.Path.is_file", autospec=True, side_effect=lambda p: p.is_relative_to(path)) mkdir_mock = mocker.patch("pathlib.Path.mkdir") copy_mock = mocker.patch("shutil.copy") - chown_mock = mocker.patch("ahriman.models.repository_paths.RepositoryPaths.chown") + owner_guard_mock = mocker.patch("ahriman.models.repository_paths.RepositoryPaths.preserve_owner") pacman.database_copy(pacman.handle, database, path, use_ahriman_cache=True) mkdir_mock.assert_called_once_with(mode=0o755, exist_ok=True) copy_mock.assert_called_once_with(path / "sync" / "core.db", dst_path) - chown_mock.assert_called_once_with(dst_path) + owner_guard_mock.assert_called_once_with(dst_path.parent) def test_database_copy_skip(pacman: Pacman, mocker: MockerFixture) -> None: diff --git a/tests/ahriman/core/database/migrations/test_m011_repository_name.py b/tests/ahriman/core/database/migrations/test_m011_repository_name.py index e7053a7b..970d482e 100644 --- a/tests/ahriman/core/database/migrations/test_m011_repository_name.py +++ b/tests/ahriman/core/database/migrations/test_m011_repository_name.py @@ -6,7 +6,6 @@ from unittest.mock import call as MockCall from ahriman.core.configuration import Configuration from ahriman.core.database.migrations.m011_repository_name import migrate_data, migrate_package_repository, steps -from ahriman.models.repository_id import RepositoryId def test_migration_repository_name() -> None: @@ -38,13 +37,3 @@ def test_migrate_package_repository(connection: Connection, configuration: Confi MockCall(pytest.helpers.anyvar(str, strict=True), {"repository": configuration.repository_id.id}), MockCall(pytest.helpers.anyvar(str, strict=True), {"repository": configuration.repository_id.id}), ]) - - -def test_migrate_package_repository_empty_id(connection: Connection, configuration: Configuration, - mocker: MockerFixture) -> None: - """ - must do nothing on empty repository id - """ - mocker.patch("ahriman.core.configuration.Configuration.check_loaded", return_value=("", RepositoryId("", ""))) - migrate_package_repository(connection, configuration) - connection.execute.assert_not_called() diff --git a/tests/ahriman/core/database/test_sqlite.py b/tests/ahriman/core/database/test_sqlite.py index 60581334..91aa1bc8 100644 --- a/tests/ahriman/core/database/test_sqlite.py +++ b/tests/ahriman/core/database/test_sqlite.py @@ -36,6 +36,17 @@ def test_init_skip_migration(database: SQLite, mocker: MockerFixture) -> None: migrate_schema_mock.assert_not_called() +def test_init_skip_empty_repository(database: SQLite, mocker: MockerFixture) -> None: + """ + must skip migrations if repository identifier is not set + """ + database._repository_id = RepositoryId("", "") + migrate_schema_mock = mocker.patch("ahriman.core.database.migrations.Migrations.migrate") + + database.init() + migrate_schema_mock.assert_not_called() + + def test_package_clear(database: SQLite, repository_id: RepositoryId, mocker: MockerFixture) -> None: """ must clear package data diff --git a/tests/ahriman/models/test_repository_paths.py b/tests/ahriman/models/test_repository_paths.py index 4454ea79..4100d720 100644 --- a/tests/ahriman/models/test_repository_paths.py +++ b/tests/ahriman/models/test_repository_paths.py @@ -198,15 +198,6 @@ def test_owner(repository_paths: RepositoryPaths, mocker: MockerFixture) -> None assert RepositoryPaths.owner(repository_paths.root) == (42, 142) -def test_cache_for(repository_paths: RepositoryPaths, package_ahriman: Package) -> None: - """ - must return correct path for cache directory - """ - path = repository_paths.cache_for(package_ahriman.base) - assert path.name == package_ahriman.base - assert path.parent == repository_paths.cache - - def test_chown(repository_paths: RepositoryPaths, mocker: MockerFixture) -> None: """ must correctly set owner for the directory @@ -216,7 +207,7 @@ def test_chown(repository_paths: RepositoryPaths, mocker: MockerFixture) -> None chown_mock = mocker.patch("os.chown") path = repository_paths.root / "path" - repository_paths.chown(path) + repository_paths._chown(path) chown_mock.assert_called_once_with(path, 42, 42, follow_symlinks=False) @@ -229,7 +220,7 @@ def test_chown_parent(repository_paths: RepositoryPaths, mocker: MockerFixture) chown_mock = mocker.patch("os.chown") path = repository_paths.root / "parent" / "path" - repository_paths.chown(path) + repository_paths._chown(path) chown_mock.assert_has_calls([ MockCall(path, 42, 42, follow_symlinks=False), MockCall(path.parent, 42, 42, follow_symlinks=False) @@ -245,7 +236,7 @@ def test_chown_skip(repository_paths: RepositoryPaths, mocker: MockerFixture) -> chown_mock = mocker.patch("os.chown") path = repository_paths.root / "path" - repository_paths.chown(path) + repository_paths._chown(path) chown_mock.assert_not_called() @@ -254,7 +245,46 @@ def test_chown_invalid_path(repository_paths: RepositoryPaths) -> None: must raise invalid path exception in case if directory outside the root supplied """ with pytest.raises(PathError): - repository_paths.chown(repository_paths.root.parent) + repository_paths._chown(repository_paths.root.parent) + + +def test_cache_for(repository_paths: RepositoryPaths, package_ahriman: Package) -> None: + """ + must return correct path for cache directory + """ + path = repository_paths.cache_for(package_ahriman.base) + assert path.name == package_ahriman.base + assert path.parent == repository_paths.cache + + +def test_preserve_owner(tmp_path: Path, repository_id: RepositoryId, mocker: MockerFixture) -> None: + """ + must preserve file owner during operations + """ + repository_paths = RepositoryPaths(tmp_path, repository_id) + repository_paths.tree_create() + chown_mock = mocker.patch("ahriman.models.repository_paths.RepositoryPaths._chown") + + with repository_paths.preserve_owner(): + (repository_paths.root / "created1").touch() + (repository_paths.chroot / "created2").touch() + chown_mock.assert_has_calls([MockCall(repository_paths.root / "created1")]) + + +def test_preserve_owner_specific(tmp_path: Path, repository_id: RepositoryId, mocker: MockerFixture) -> None: + """ + must preserve file owner during operations only in specific directory + """ + repository_paths = RepositoryPaths(tmp_path, repository_id) + repository_paths.tree_create() + (repository_paths.root / "content").mkdir() + chown_mock = mocker.patch("ahriman.models.repository_paths.RepositoryPaths._chown") + + with repository_paths.preserve_owner(repository_paths.root / "content"): + (repository_paths.root / "created1").touch() + (repository_paths.root / "content" / "created2").touch() + (repository_paths.chroot / "created3").touch() + chown_mock.assert_has_calls([MockCall(repository_paths.root / "content" / "created2")]) def test_tree_clear(repository_paths: RepositoryPaths, package_ahriman: Package, mocker: MockerFixture) -> None: @@ -293,11 +323,11 @@ def test_tree_create(repository_paths: RepositoryPaths, mocker: MockerFixture) - and not callable(getattr(repository_paths, prop)) } mkdir_mock = mocker.patch("pathlib.Path.mkdir") - chown_mock = mocker.patch("ahriman.models.repository_paths.RepositoryPaths.chown") + owner_guard_mock = mocker.patch("ahriman.models.repository_paths.RepositoryPaths.preserve_owner") repository_paths.tree_create() mkdir_mock.assert_has_calls([MockCall(mode=0o755, parents=True, exist_ok=True) for _ in paths], any_order=True) - chown_mock.assert_has_calls([MockCall(pytest.helpers.anyvar(int)) for _ in paths], any_order=True) + owner_guard_mock.assert_called_once_with() def test_tree_create_skip(mocker: MockerFixture) -> None: