From 2019da1e10bb693b4cacc71734b71c10bbb558b5 Mon Sep 17 00:00:00 2001 From: Inex Code Date: Mon, 12 Feb 2024 18:17:18 +0300 Subject: [PATCH 01/35] feat: Track the status of the nixos rebuild systemd unit --- selfprivacy_api/actions/system.py | 26 +++- .../graphql/mutations/system_mutations.py | 26 ++-- selfprivacy_api/jobs/upgrade_system.py | 120 ++++++++++++++++++ selfprivacy_api/migrations/__init__.py | 4 + .../check_for_system_rebuild_jobs.py | 47 +++++++ 5 files changed, 206 insertions(+), 17 deletions(-) create mode 100644 selfprivacy_api/jobs/upgrade_system.py create mode 100644 selfprivacy_api/migrations/check_for_system_rebuild_jobs.py diff --git a/selfprivacy_api/actions/system.py b/selfprivacy_api/actions/system.py index 13c3708..9b52497 100644 --- a/selfprivacy_api/actions/system.py +++ b/selfprivacy_api/actions/system.py @@ -4,6 +4,8 @@ import subprocess import pytz from typing import Optional, List from pydantic import BaseModel +from selfprivacy_api.jobs import Job, JobStatus, Jobs +from selfprivacy_api.jobs.upgrade_system import rebuild_system_task from selfprivacy_api.utils import WriteUserData, ReadUserData @@ -87,10 +89,16 @@ def run_blocking(cmd: List[str], new_session: bool = False) -> str: return stdout -def rebuild_system() -> int: +def rebuild_system() -> Job: """Rebuild the system""" - run_blocking(["systemctl", "start", "sp-nixos-rebuild.service"], new_session=True) - return 0 + job = Jobs.add( + type_id="system.nixos.rebuild", + name="Rebuild system", + description="Applying the new system configuration by building the new NixOS generation.", + status=JobStatus.CREATED, + ) + rebuild_system_task(job) + return job def rollback_system() -> int: @@ -99,10 +107,16 @@ def rollback_system() -> int: return 0 -def upgrade_system() -> int: +def upgrade_system() -> Job: """Upgrade the system""" - run_blocking(["systemctl", "start", "sp-nixos-upgrade.service"], new_session=True) - return 0 + job = Jobs.add( + type_id="system.nixos.upgrade", + name="Upgrade system", + description="Upgrading the system to the latest version.", + status=JobStatus.CREATED, + ) + rebuild_system_task(job, upgrade=True) + return job def reboot_system() -> None: diff --git a/selfprivacy_api/graphql/mutations/system_mutations.py b/selfprivacy_api/graphql/mutations/system_mutations.py index 13ac16b..5740a0d 100644 --- a/selfprivacy_api/graphql/mutations/system_mutations.py +++ b/selfprivacy_api/graphql/mutations/system_mutations.py @@ -3,7 +3,9 @@ import typing import strawberry from selfprivacy_api.graphql import IsAuthenticated +from selfprivacy_api.graphql.common_types.jobs import job_to_api_job from selfprivacy_api.graphql.mutations.mutation_interface import ( + GenericJobMutationReturn, GenericMutationReturn, MutationReturnInterface, ) @@ -114,16 +116,17 @@ class SystemMutations: ) @strawberry.mutation(permission_classes=[IsAuthenticated]) - def run_system_rebuild(self) -> GenericMutationReturn: + def run_system_rebuild(self) -> GenericJobMutationReturn: try: - system_actions.rebuild_system() - return GenericMutationReturn( + job = system_actions.rebuild_system() + return GenericJobMutationReturn( success=True, - message="Starting rebuild system", + message="Starting system rebuild", code=200, + job=job_to_api_job(job), ) except system_actions.ShellException as e: - return GenericMutationReturn( + return GenericJobMutationReturn( success=False, message=str(e), code=500, @@ -135,7 +138,7 @@ class SystemMutations: try: return GenericMutationReturn( success=True, - message="Starting rebuild system", + message="Starting system rollback", code=200, ) except system_actions.ShellException as e: @@ -146,16 +149,17 @@ class SystemMutations: ) @strawberry.mutation(permission_classes=[IsAuthenticated]) - def run_system_upgrade(self) -> GenericMutationReturn: - system_actions.upgrade_system() + def run_system_upgrade(self) -> GenericJobMutationReturn: try: - return GenericMutationReturn( + job = system_actions.upgrade_system() + return GenericJobMutationReturn( success=True, - message="Starting rebuild system", + message="Starting system upgrade", code=200, + job=job_to_api_job(job), ) except system_actions.ShellException as e: - return GenericMutationReturn( + return GenericJobMutationReturn( success=False, message=str(e), code=500, diff --git a/selfprivacy_api/jobs/upgrade_system.py b/selfprivacy_api/jobs/upgrade_system.py new file mode 100644 index 0000000..2d645b8 --- /dev/null +++ b/selfprivacy_api/jobs/upgrade_system.py @@ -0,0 +1,120 @@ +""" +A task to start the system upgrade or rebuild by starting a systemd unit. +After starting, track the status of the systemd unit and update the Job +status accordingly. +""" +import subprocess +from selfprivacy_api.utils.huey import huey +from selfprivacy_api.jobs import JobStatus, Jobs, Job +import time + + +@huey.task() +def rebuild_system_task(job: Job, upgrade: bool = False): + """Rebuild the system""" + try: + if upgrade: + command = ["systemctl", "start", "sp-nixos-upgrade.service"] + else: + command = ["systemctl", "start", "sp-nixos-rebuild.service"] + subprocess.run( + command, + check=True, + start_new_session=True, + shell=False, + ) + Jobs.update( + job=job, + status=JobStatus.RUNNING, + status_text="Rebuilding the system...", + ) + # Get current time to handle timeout + start_time = time.time() + # Wait for the systemd unit to start + while True: + try: + status = subprocess.run( + ["systemctl", "is-active", "selfprivacy-upgrade"], + check=True, + capture_output=True, + text=True, + ) + if status.stdout.strip() == "active": + log_line = subprocess.run( + [ + "journalctl", + "-u", + "selfprivacy-upgrade", + "-n", + "1", + "-o", + "cat", + ], + check=True, + capture_output=True, + text=True, + ).stdout.strip() + Jobs.update( + job=job, + status=JobStatus.RUNNING, + status_text=f"Rebuilding the system... Latest log line: {log_line}", + ) + break + # Timeount after 5 minutes + if time.time() - start_time > 300: + Jobs.update( + job=job, + status=JobStatus.ERROR, + error="System rebuild timed out.", + ) + return + time.sleep(1) + except subprocess.CalledProcessError: + pass + Jobs.update( + job=job, + status=JobStatus.RUNNING, + status_text="Rebuilding the system...", + ) + # Wait for the systemd unit to finish + while True: + try: + status = subprocess.run( + ["systemctl", "is-active", "selfprivacy-upgrade"], + check=True, + capture_output=True, + text=True, + ) + if status.stdout.strip() == "inactive": + Jobs.update( + job=job, + status=JobStatus.FINISHED, + result="System rebuilt.", + progress=100, + ) + elif status.stdout.strip() == "failed": + Jobs.update( + job=job, + status=JobStatus.ERROR, + error="System rebuild failed.", + ) + break + # Timeout of 60 minutes + if time.time() - start_time > 3600: + Jobs.update( + job=job, + status=JobStatus.ERROR, + error="System rebuild timed out.", + ) + break + except subprocess.CalledProcessError: + pass + + time.sleep(5) + + except subprocess.CalledProcessError as e: + Jobs.update( + job=job, + status=JobStatus.ERROR, + status_text=str(e), + ) diff --git a/selfprivacy_api/migrations/__init__.py b/selfprivacy_api/migrations/__init__.py index 5e05b2d..2a2cbaa 100644 --- a/selfprivacy_api/migrations/__init__.py +++ b/selfprivacy_api/migrations/__init__.py @@ -11,9 +11,13 @@ Adding DISABLE_ALL to that array disables the migrations module entirely. from selfprivacy_api.utils import ReadUserData, UserDataFiles from selfprivacy_api.migrations.write_token_to_redis import WriteTokenToRedis +from selfprivacy_api.migrations.check_for_system_rebuild_jobs import ( + CheckForSystemRebuildJobs, +) migrations = [ WriteTokenToRedis(), + CheckForSystemRebuildJobs(), ] diff --git a/selfprivacy_api/migrations/check_for_system_rebuild_jobs.py b/selfprivacy_api/migrations/check_for_system_rebuild_jobs.py new file mode 100644 index 0000000..9bbac8a --- /dev/null +++ b/selfprivacy_api/migrations/check_for_system_rebuild_jobs.py @@ -0,0 +1,47 @@ +from selfprivacy_api.migrations.migration import Migration +from selfprivacy_api.jobs import JobStatus, Jobs + + +class CheckForSystemRebuildJobs(Migration): + """Check if there are unfinished system rebuild jobs and finish them""" + + def get_migration_name(self): + return "check_for_system_rebuild_jobs" + + def get_migration_description(self): + return "Check if there are unfinished system rebuild jobs and finish them" + + def is_migration_needed(self): + # Check if there are any unfinished system rebuild jobs + for job in Jobs.get_jobs(): + if ( + job.type_id + in [ + "system.nixos.rebuild", + "system.nixos.upgrade", + ] + ) and job.status in [ + JobStatus.CREATED, + JobStatus.RUNNING, + ]: + return True + + def migrate(self): + # As the API is restarted, we assume that the jobs are finished + for job in Jobs.get_jobs(): + if ( + job.type_id + in [ + "system.nixos.rebuild", + "system.nixos.upgrade", + ] + ) and job.status in [ + JobStatus.CREATED, + JobStatus.RUNNING, + ]: + Jobs.update( + job=job, + status=JobStatus.FINISHED, + result="System rebuilt.", + progress=100, + ) From 56de00226a260a89d892f25b403be3a73242f2a1 Mon Sep 17 00:00:00 2001 From: Inex Code Date: Mon, 12 Feb 2024 18:21:09 +0300 Subject: [PATCH 02/35] chore: Testing env --- nixos/module.nix | 2 +- selfprivacy_api/dependencies.py | 2 +- setup.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/nixos/module.nix b/nixos/module.nix index 7790e18..ba2db08 100644 --- a/nixos/module.nix +++ b/nixos/module.nix @@ -129,7 +129,7 @@ in # TODO get URL from systemd template parameter? ExecStartPre = '' ${nix} flake update \ - --override-input selfprivacy-nixos-config git+https://git.selfprivacy.org/SelfPrivacy/selfprivacy-nixos-config.git?ref=flakes + --override-input selfprivacy-nixos-config git+https://git.selfprivacy.org/SelfPrivacy/selfprivacy-nixos-config.git?ref=inex/test-systemd-rebuild ''; ExecStart = '' ${nixos-rebuild} switch --flake .#${config-id} diff --git a/selfprivacy_api/dependencies.py b/selfprivacy_api/dependencies.py index 1dfc0a9..a424fc8 100644 --- a/selfprivacy_api/dependencies.py +++ b/selfprivacy_api/dependencies.py @@ -27,4 +27,4 @@ async def get_token_header( def get_api_version() -> str: """Get API version""" - return "3.0.0" + return "3.0.0-inex" diff --git a/setup.py b/setup.py index 36aa68e..7eb2d72 100755 --- a/setup.py +++ b/setup.py @@ -2,7 +2,7 @@ from setuptools import setup, find_packages setup( name="selfprivacy_api", - version="3.0.0", + version="3.0.0-inex", packages=find_packages(), scripts=[ "selfprivacy_api/app.py", From 00bcca0f994453822be73103c5958d5b89e9b9d8 Mon Sep 17 00:00:00 2001 From: Inex Code Date: Mon, 12 Feb 2024 18:24:54 +0300 Subject: [PATCH 03/35] fix: invalid setuptools version --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 7eb2d72..36aa68e 100755 --- a/setup.py +++ b/setup.py @@ -2,7 +2,7 @@ from setuptools import setup, find_packages setup( name="selfprivacy_api", - version="3.0.0-inex", + version="3.0.0", packages=find_packages(), scripts=[ "selfprivacy_api/app.py", From ab1ca6e59c8f298604973d7149327807586496d9 Mon Sep 17 00:00:00 2001 From: Inex Code Date: Mon, 12 Feb 2024 18:27:32 +0300 Subject: [PATCH 04/35] fix: register huey task --- selfprivacy_api/task_registry.py | 1 + 1 file changed, 1 insertion(+) diff --git a/selfprivacy_api/task_registry.py b/selfprivacy_api/task_registry.py index dfd329c..92e19fe 100644 --- a/selfprivacy_api/task_registry.py +++ b/selfprivacy_api/task_registry.py @@ -2,3 +2,4 @@ from selfprivacy_api.utils.huey import huey from selfprivacy_api.jobs.test import test_job from selfprivacy_api.backup.tasks import * from selfprivacy_api.services.generic_service_mover import move_service +from selfprivacy_api.jobs.upgrade_system import rebuild_system_task From 94456af7d433c112ec2b7d2bba13fe315104c7ce Mon Sep 17 00:00:00 2001 From: Inex Code Date: Mon, 12 Feb 2024 18:34:55 +0300 Subject: [PATCH 05/35] fix: debugging --- selfprivacy_api/jobs/upgrade_system.py | 49 ++++++++++++++------------ 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/selfprivacy_api/jobs/upgrade_system.py b/selfprivacy_api/jobs/upgrade_system.py index 2d645b8..7a8d334 100644 --- a/selfprivacy_api/jobs/upgrade_system.py +++ b/selfprivacy_api/jobs/upgrade_system.py @@ -26,7 +26,7 @@ def rebuild_system_task(job: Job, upgrade: bool = False): Jobs.update( job=job, status=JobStatus.RUNNING, - status_text="Rebuilding the system...", + status_text="Starting the system rebuild...", ) # Get current time to handle timeout start_time = time.time() @@ -39,26 +39,8 @@ def rebuild_system_task(job: Job, upgrade: bool = False): capture_output=True, text=True, ) + print(status.stdout.strip()) if status.stdout.strip() == "active": - log_line = subprocess.run( - [ - "journalctl", - "-u", - "selfprivacy-upgrade", - "-n", - "1", - "-o", - "cat", - ], - check=True, - capture_output=True, - text=True, - ).stdout.strip() - Jobs.update( - job=job, - status=JobStatus.RUNNING, - status_text=f"Rebuilding the system... Latest log line: {log_line}", - ) break # Timeount after 5 minutes if time.time() - start_time > 300: @@ -76,6 +58,7 @@ def rebuild_system_task(job: Job, upgrade: bool = False): status=JobStatus.RUNNING, status_text="Rebuilding the system...", ) + print("Rebuilding the system...") # Wait for the systemd unit to finish while True: try: @@ -85,6 +68,7 @@ def rebuild_system_task(job: Job, upgrade: bool = False): capture_output=True, text=True, ) + print(status.stdout.strip()) if status.stdout.strip() == "inactive": Jobs.update( job=job, @@ -99,6 +83,28 @@ def rebuild_system_task(job: Job, upgrade: bool = False): error="System rebuild failed.", ) break + elif status.stdout.strip() == "active": + print("Geting a log line") + log_line = subprocess.run( + [ + "journalctl", + "-u", + "selfprivacy-upgrade", + "-n", + "1", + "-o", + "cat", + ], + check=True, + capture_output=True, + text=True, + ).stdout.strip() + print(log_line) + Jobs.update( + job=job, + status=JobStatus.RUNNING, + status_text=f"Rebuilding the system... Latest log line: {log_line}", + ) # Timeout of 60 minutes if time.time() - start_time > 3600: Jobs.update( @@ -107,11 +113,10 @@ def rebuild_system_task(job: Job, upgrade: bool = False): error="System rebuild timed out.", ) break + time.sleep(5) except subprocess.CalledProcessError: pass - time.sleep(5) - except subprocess.CalledProcessError as e: Jobs.update( job=job, From b98c020f239f8a677ff69ed173988d727e0e8fe4 Mon Sep 17 00:00:00 2001 From: Inex Code Date: Mon, 12 Feb 2024 18:41:24 +0300 Subject: [PATCH 06/35] fix: wrong systemd unit used --- selfprivacy_api/jobs/upgrade_system.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/selfprivacy_api/jobs/upgrade_system.py b/selfprivacy_api/jobs/upgrade_system.py index 7a8d334..4aa28da 100644 --- a/selfprivacy_api/jobs/upgrade_system.py +++ b/selfprivacy_api/jobs/upgrade_system.py @@ -12,11 +12,9 @@ import time @huey.task() def rebuild_system_task(job: Job, upgrade: bool = False): """Rebuild the system""" + unit_name = "sp-nixos-upgrade.service" if upgrade else "sp-nixos-rebuild.service" try: - if upgrade: - command = ["systemctl", "start", "sp-nixos-upgrade.service"] - else: - command = ["systemctl", "start", "sp-nixos-rebuild.service"] + command = ["systemctl", "start", unit_name] subprocess.run( command, check=True, @@ -34,7 +32,7 @@ def rebuild_system_task(job: Job, upgrade: bool = False): while True: try: status = subprocess.run( - ["systemctl", "is-active", "selfprivacy-upgrade"], + ["systemctl", "is-active", unit_name], check=True, capture_output=True, text=True, @@ -63,7 +61,7 @@ def rebuild_system_task(job: Job, upgrade: bool = False): while True: try: status = subprocess.run( - ["systemctl", "is-active", "selfprivacy-upgrade"], + ["systemctl", "is-active", unit_name], check=True, capture_output=True, text=True, From ad069a2ad213b9bb25daa572b8df74a62724a254 Mon Sep 17 00:00:00 2001 From: Inex Code Date: Mon, 12 Feb 2024 18:47:37 +0300 Subject: [PATCH 07/35] fix: wrong unit name again --- selfprivacy_api/jobs/upgrade_system.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/selfprivacy_api/jobs/upgrade_system.py b/selfprivacy_api/jobs/upgrade_system.py index 4aa28da..13e6fb9 100644 --- a/selfprivacy_api/jobs/upgrade_system.py +++ b/selfprivacy_api/jobs/upgrade_system.py @@ -74,6 +74,7 @@ def rebuild_system_task(job: Job, upgrade: bool = False): result="System rebuilt.", progress=100, ) + break elif status.stdout.strip() == "failed": Jobs.update( job=job, @@ -87,7 +88,7 @@ def rebuild_system_task(job: Job, upgrade: bool = False): [ "journalctl", "-u", - "selfprivacy-upgrade", + unit_name, "-n", "1", "-o", From c851c3d193b3a3afcdda48c2ec3ba61b001eaed3 Mon Sep 17 00:00:00 2001 From: Inex Code Date: Mon, 12 Feb 2024 18:53:14 +0300 Subject: [PATCH 08/35] chore: more debugging outuput --- selfprivacy_api/jobs/upgrade_system.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/selfprivacy_api/jobs/upgrade_system.py b/selfprivacy_api/jobs/upgrade_system.py index 13e6fb9..8a1e845 100644 --- a/selfprivacy_api/jobs/upgrade_system.py +++ b/selfprivacy_api/jobs/upgrade_system.py @@ -66,8 +66,9 @@ def rebuild_system_task(job: Job, upgrade: bool = False): capture_output=True, text=True, ) - print(status.stdout.strip()) + print(f"Unit status: {status.stdout.strip()}") if status.stdout.strip() == "inactive": + print("System rebuilt.") Jobs.update( job=job, status=JobStatus.FINISHED, @@ -76,6 +77,7 @@ def rebuild_system_task(job: Job, upgrade: bool = False): ) break elif status.stdout.strip() == "failed": + print("System rebuild failed.") Jobs.update( job=job, status=JobStatus.ERROR, @@ -106,6 +108,7 @@ def rebuild_system_task(job: Job, upgrade: bool = False): ) # Timeout of 60 minutes if time.time() - start_time > 3600: + print("System rebuild timed out.") Jobs.update( job=job, status=JobStatus.ERROR, @@ -114,6 +117,7 @@ def rebuild_system_task(job: Job, upgrade: bool = False): break time.sleep(5) except subprocess.CalledProcessError: + print("subprocess.CalledProcessError") pass except subprocess.CalledProcessError as e: From 1a34558e23c8f3ba0835cc88791a176b70fe0b9a Mon Sep 17 00:00:00 2001 From: Inex Code Date: Mon, 12 Feb 2024 18:54:32 +0300 Subject: [PATCH 09/35] chore: Shorten the output on status_text --- selfprivacy_api/jobs/upgrade_system.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/selfprivacy_api/jobs/upgrade_system.py b/selfprivacy_api/jobs/upgrade_system.py index 8a1e845..58e37f8 100644 --- a/selfprivacy_api/jobs/upgrade_system.py +++ b/selfprivacy_api/jobs/upgrade_system.py @@ -104,7 +104,7 @@ def rebuild_system_task(job: Job, upgrade: bool = False): Jobs.update( job=job, status=JobStatus.RUNNING, - status_text=f"Rebuilding the system... Latest log line: {log_line}", + status_text=f"{log_line}", ) # Timeout of 60 minutes if time.time() - start_time > 3600: From 25c691104f323655c5e8ff4cf96fa2cdaa87193c Mon Sep 17 00:00:00 2001 From: Inex Code Date: Mon, 12 Feb 2024 18:58:27 +0300 Subject: [PATCH 10/35] fix: non-0 exit status of is-active --- selfprivacy_api/jobs/upgrade_system.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/selfprivacy_api/jobs/upgrade_system.py b/selfprivacy_api/jobs/upgrade_system.py index 58e37f8..42eb4ce 100644 --- a/selfprivacy_api/jobs/upgrade_system.py +++ b/selfprivacy_api/jobs/upgrade_system.py @@ -62,7 +62,7 @@ def rebuild_system_task(job: Job, upgrade: bool = False): try: status = subprocess.run( ["systemctl", "is-active", unit_name], - check=True, + check=False, capture_output=True, text=True, ) @@ -96,7 +96,7 @@ def rebuild_system_task(job: Job, upgrade: bool = False): "-o", "cat", ], - check=True, + check=False, capture_output=True, text=True, ).stdout.strip() From c63552241c6c3274cb08c8245de8b6581549085b Mon Sep 17 00:00:00 2001 From: Inex Code Date: Mon, 26 Feb 2024 22:49:32 +0300 Subject: [PATCH 11/35] tests: Cover upgrade and rebuild task --- flake.nix | 1 + selfprivacy_api/jobs/upgrade_system.py | 41 ++-- tests/test_graphql/test_system_nixos_tasks.py | 196 +++++++++++++----- 3 files changed, 164 insertions(+), 74 deletions(-) diff --git a/flake.nix b/flake.nix index f82fcf5..7271259 100644 --- a/flake.nix +++ b/flake.nix @@ -19,6 +19,7 @@ pytest pytest-datadir pytest-mock + pytest-subprocess black mypy pylsp-mypy diff --git a/selfprivacy_api/jobs/upgrade_system.py b/selfprivacy_api/jobs/upgrade_system.py index 42eb4ce..afb3eb1 100644 --- a/selfprivacy_api/jobs/upgrade_system.py +++ b/selfprivacy_api/jobs/upgrade_system.py @@ -4,9 +4,15 @@ After starting, track the status of the systemd unit and update the Job status accordingly. """ import subprocess +import time from selfprivacy_api.utils.huey import huey from selfprivacy_api.jobs import JobStatus, Jobs, Job -import time +from datetime import datetime + +START_TIMEOUT = 60 * 5 +START_INTERVAL = 1 +RUN_TIMEOUT = 60 * 60 +RUN_INTERVAL = 5 @huey.task() @@ -27,7 +33,7 @@ def rebuild_system_task(job: Job, upgrade: bool = False): status_text="Starting the system rebuild...", ) # Get current time to handle timeout - start_time = time.time() + start_time = datetime.now() # Wait for the systemd unit to start while True: try: @@ -37,18 +43,16 @@ def rebuild_system_task(job: Job, upgrade: bool = False): capture_output=True, text=True, ) - print(status.stdout.strip()) if status.stdout.strip() == "active": break - # Timeount after 5 minutes - if time.time() - start_time > 300: + if (datetime.now() - start_time).total_seconds() > START_TIMEOUT: Jobs.update( job=job, status=JobStatus.ERROR, error="System rebuild timed out.", ) return - time.sleep(1) + time.sleep(START_INTERVAL) except subprocess.CalledProcessError: pass Jobs.update( @@ -56,7 +60,6 @@ def rebuild_system_task(job: Job, upgrade: bool = False): status=JobStatus.RUNNING, status_text="Rebuilding the system...", ) - print("Rebuilding the system...") # Wait for the systemd unit to finish while True: try: @@ -66,9 +69,7 @@ def rebuild_system_task(job: Job, upgrade: bool = False): capture_output=True, text=True, ) - print(f"Unit status: {status.stdout.strip()}") if status.stdout.strip() == "inactive": - print("System rebuilt.") Jobs.update( job=job, status=JobStatus.FINISHED, @@ -77,7 +78,6 @@ def rebuild_system_task(job: Job, upgrade: bool = False): ) break elif status.stdout.strip() == "failed": - print("System rebuild failed.") Jobs.update( job=job, status=JobStatus.ERROR, @@ -85,7 +85,6 @@ def rebuild_system_task(job: Job, upgrade: bool = False): ) break elif status.stdout.strip() == "active": - print("Geting a log line") log_line = subprocess.run( [ "journalctl", @@ -100,25 +99,21 @@ def rebuild_system_task(job: Job, upgrade: bool = False): capture_output=True, text=True, ).stdout.strip() - print(log_line) Jobs.update( job=job, status=JobStatus.RUNNING, status_text=f"{log_line}", ) - # Timeout of 60 minutes - if time.time() - start_time > 3600: - print("System rebuild timed out.") - Jobs.update( - job=job, - status=JobStatus.ERROR, - error="System rebuild timed out.", - ) - break - time.sleep(5) except subprocess.CalledProcessError: - print("subprocess.CalledProcessError") pass + if (datetime.now() - start_time).total_seconds() > RUN_TIMEOUT: + Jobs.update( + job=job, + status=JobStatus.ERROR, + error="System rebuild timed out.", + ) + break + time.sleep(RUN_INTERVAL) except subprocess.CalledProcessError as e: Jobs.update( diff --git a/tests/test_graphql/test_system_nixos_tasks.py b/tests/test_graphql/test_system_nixos_tasks.py index 4a750c4..3f47ad6 100644 --- a/tests/test_graphql/test_system_nixos_tasks.py +++ b/tests/test_graphql/test_system_nixos_tasks.py @@ -3,6 +3,8 @@ # pylint: disable=missing-function-docstring import pytest +from selfprivacy_api.jobs import JobStatus, Jobs + class ProcessMock: """Mock subprocess.Popen""" @@ -37,6 +39,13 @@ def mock_subprocess_check_output(mocker): return mock +@pytest.fixture +def mock_sleep_intervals(mocker): + mock_start = mocker.patch("selfprivacy_api.jobs.upgrade_system.START_INTERVAL", 0) + mock_run = mocker.patch("selfprivacy_api.jobs.upgrade_system.RUN_INTERVAL", 0) + return (mock_start, mock_run) + + API_REBUILD_SYSTEM_MUTATION = """ mutation rebuildSystem { system { @@ -44,46 +53,14 @@ mutation rebuildSystem { success message code + job { + uid + } } } } """ - -def test_graphql_system_rebuild_unauthorized(client, mock_subprocess_popen): - """Test system rebuild without authorization""" - response = client.post( - "/graphql", - json={ - "query": API_REBUILD_SYSTEM_MUTATION, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is None - assert mock_subprocess_popen.call_count == 0 - - -def test_graphql_system_rebuild(authorized_client, mock_subprocess_popen): - """Test system rebuild""" - response = authorized_client.post( - "/graphql", - json={ - "query": API_REBUILD_SYSTEM_MUTATION, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - assert response.json()["data"]["system"]["runSystemRebuild"]["success"] is True - assert response.json()["data"]["system"]["runSystemRebuild"]["message"] is not None - assert response.json()["data"]["system"]["runSystemRebuild"]["code"] == 200 - assert mock_subprocess_popen.call_count == 1 - assert mock_subprocess_popen.call_args[0][0] == [ - "systemctl", - "start", - "sp-nixos-rebuild.service", - ] - - API_UPGRADE_SYSTEM_MUTATION = """ mutation upgradeSystem { system { @@ -91,44 +68,161 @@ mutation upgradeSystem { success message code + job { + uid + } } } } """ -def test_graphql_system_upgrade_unauthorized(client, mock_subprocess_popen): - """Test system upgrade without authorization""" +@pytest.mark.parametrize("action", ["rebuild", "upgrade"]) +def test_graphql_system_rebuild_unauthorized(client, fp, action): + """Test system rebuild without authorization""" + query = ( + API_REBUILD_SYSTEM_MUTATION + if action == "rebuild" + else API_UPGRADE_SYSTEM_MUTATION + ) + response = client.post( "/graphql", json={ - "query": API_UPGRADE_SYSTEM_MUTATION, + "query": query, }, ) assert response.status_code == 200 assert response.json().get("data") is None - assert mock_subprocess_popen.call_count == 0 + assert fp.call_count([fp.any()]) == 0 -def test_graphql_system_upgrade(authorized_client, mock_subprocess_popen): - """Test system upgrade""" +@pytest.mark.parametrize("action", ["rebuild", "upgrade"]) +def test_graphql_system_rebuild(authorized_client, fp, action, mock_sleep_intervals): + """Test system rebuild""" + unit_name = f"sp-nixos-{action}.service" + query = ( + API_REBUILD_SYSTEM_MUTATION + if action == "rebuild" + else API_UPGRADE_SYSTEM_MUTATION + ) + + # Start the unit + fp.register(["systemctl", "start", unit_name]) + + # Wait for it to start + fp.register(["systemctl", "is-active", unit_name], stdout="inactive") + fp.register(["systemctl", "is-active", unit_name], stdout="inactive") + fp.register(["systemctl", "is-active", unit_name], stdout="active") + + # Check its exectution + fp.register(["systemctl", "is-active", unit_name], stdout="active") + fp.register( + ["journalctl", "-u", unit_name, "-n", "1", "-o", "cat"], + stdout="Starting rebuild...", + ) + + fp.register(["systemctl", "is-active", unit_name], stdout="active") + fp.register( + ["journalctl", "-u", unit_name, "-n", "1", "-o", "cat"], stdout="Rebuilding..." + ) + + fp.register(["systemctl", "is-active", unit_name], stdout="inactive") + response = authorized_client.post( "/graphql", json={ - "query": API_UPGRADE_SYSTEM_MUTATION, + "query": query, }, ) assert response.status_code == 200 assert response.json().get("data") is not None - assert response.json()["data"]["system"]["runSystemUpgrade"]["success"] is True - assert response.json()["data"]["system"]["runSystemUpgrade"]["message"] is not None - assert response.json()["data"]["system"]["runSystemUpgrade"]["code"] == 200 - assert mock_subprocess_popen.call_count == 1 - assert mock_subprocess_popen.call_args[0][0] == [ - "systemctl", - "start", - "sp-nixos-upgrade.service", - ] + assert ( + response.json()["data"]["system"][f"runSystem{action.capitalize()}"]["success"] + is True + ) + assert ( + response.json()["data"]["system"][f"runSystem{action.capitalize()}"]["message"] + is not None + ) + assert ( + response.json()["data"]["system"][f"runSystem{action.capitalize()}"]["code"] + == 200 + ) + + assert fp.call_count(["systemctl", "start", unit_name]) == 1 + assert fp.call_count(["systemctl", "is-active", unit_name]) == 6 + + job_id = response.json()["data"]["system"][f"runSystem{action.capitalize()}"][ + "job" + ]["uid"] + assert Jobs.get_job(job_id).status == JobStatus.FINISHED + assert Jobs.get_job(job_id).type_id == f"system.nixos.{action}" + + +@pytest.mark.parametrize("action", ["rebuild", "upgrade"]) +def test_graphql_system_rebuild_failed( + authorized_client, fp, action, mock_sleep_intervals +): + """Test system rebuild""" + unit_name = f"sp-nixos-{action}.service" + query = ( + API_REBUILD_SYSTEM_MUTATION + if action == "rebuild" + else API_UPGRADE_SYSTEM_MUTATION + ) + + # Start the unit + fp.register(["systemctl", "start", unit_name]) + + # Wait for it to start + fp.register(["systemctl", "is-active", unit_name], stdout="inactive") + fp.register(["systemctl", "is-active", unit_name], stdout="inactive") + fp.register(["systemctl", "is-active", unit_name], stdout="active") + + # Check its exectution + fp.register(["systemctl", "is-active", unit_name], stdout="active") + fp.register( + ["journalctl", "-u", unit_name, "-n", "1", "-o", "cat"], + stdout="Starting rebuild...", + ) + + fp.register(["systemctl", "is-active", unit_name], stdout="active") + fp.register( + ["journalctl", "-u", unit_name, "-n", "1", "-o", "cat"], stdout="Rebuilding..." + ) + + fp.register(["systemctl", "is-active", unit_name], stdout="failed") + + response = authorized_client.post( + "/graphql", + json={ + "query": query, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + assert ( + response.json()["data"]["system"][f"runSystem{action.capitalize()}"]["success"] + is True + ) + assert ( + response.json()["data"]["system"][f"runSystem{action.capitalize()}"]["message"] + is not None + ) + assert ( + response.json()["data"]["system"][f"runSystem{action.capitalize()}"]["code"] + == 200 + ) + + assert fp.call_count(["systemctl", "start", unit_name]) == 1 + assert fp.call_count(["systemctl", "is-active", unit_name]) == 6 + + job_id = response.json()["data"]["system"][f"runSystem{action.capitalize()}"][ + "job" + ]["uid"] + assert Jobs.get_job(job_id).status == JobStatus.ERROR + assert Jobs.get_job(job_id).type_id == f"system.nixos.{action}" API_ROLLBACK_SYSTEM_MUTATION = """ From 2443ae0144d266423eff7b2c93e4d6cbe4732a1c Mon Sep 17 00:00:00 2001 From: Inex Code Date: Mon, 26 Feb 2024 22:51:31 +0300 Subject: [PATCH 12/35] chore: Remove version flavor --- selfprivacy_api/dependencies.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/selfprivacy_api/dependencies.py b/selfprivacy_api/dependencies.py index a424fc8..1dfc0a9 100644 --- a/selfprivacy_api/dependencies.py +++ b/selfprivacy_api/dependencies.py @@ -27,4 +27,4 @@ async def get_token_header( def get_api_version() -> str: """Get API version""" - return "3.0.0-inex" + return "3.0.0" From 8cb812be5684b59d0a2ff718cb26c6b67ebb2425 Mon Sep 17 00:00:00 2001 From: Inex Code Date: Sun, 3 Mar 2024 12:00:07 +0300 Subject: [PATCH 13/35] chore: Remove debug leftover --- nixos/module.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nixos/module.nix b/nixos/module.nix index ba2db08..7790e18 100644 --- a/nixos/module.nix +++ b/nixos/module.nix @@ -129,7 +129,7 @@ in # TODO get URL from systemd template parameter? ExecStartPre = '' ${nix} flake update \ - --override-input selfprivacy-nixos-config git+https://git.selfprivacy.org/SelfPrivacy/selfprivacy-nixos-config.git?ref=inex/test-systemd-rebuild + --override-input selfprivacy-nixos-config git+https://git.selfprivacy.org/SelfPrivacy/selfprivacy-nixos-config.git?ref=flakes ''; ExecStart = '' ${nixos-rebuild} switch --flake .#${config-id} From 71433da424044822552ea7ea19d8e34e8e5b9711 Mon Sep 17 00:00:00 2001 From: Inex Code Date: Tue, 5 Mar 2024 11:55:52 +0300 Subject: [PATCH 14/35] refactor: move systemd functions to utils --- selfprivacy_api/jobs/upgrade_system.py | 152 +++++++++--------- .../services/bitwarden/__init__.py | 4 +- selfprivacy_api/services/gitea/__init__.py | 4 +- .../services/jitsimeet/__init__.py | 4 +- .../services/mailserver/__init__.py | 2 +- .../services/nextcloud/__init__.py | 4 +- selfprivacy_api/services/ocserv/__init__.py | 3 +- selfprivacy_api/services/pleroma/__init__.py | 4 +- .../systemd.py} | 22 +++ tests/test_graphql/test_system_nixos_tasks.py | 64 +++----- 10 files changed, 129 insertions(+), 134 deletions(-) rename selfprivacy_api/{services/generic_status_getter.py => utils/systemd.py} (78%) diff --git a/selfprivacy_api/jobs/upgrade_system.py b/selfprivacy_api/jobs/upgrade_system.py index afb3eb1..f766f7e 100644 --- a/selfprivacy_api/jobs/upgrade_system.py +++ b/selfprivacy_api/jobs/upgrade_system.py @@ -4,10 +4,14 @@ After starting, track the status of the systemd unit and update the Job status accordingly. """ import subprocess -import time from selfprivacy_api.utils.huey import huey from selfprivacy_api.jobs import JobStatus, Jobs, Job -from datetime import datetime +from selfprivacy_api.utils.waitloop import wait_until_true +from selfprivacy_api.utils.systemd import ( + get_service_status, + get_last_log_lines, + ServiceStatus, +) START_TIMEOUT = 60 * 5 START_INTERVAL = 1 @@ -15,6 +19,49 @@ RUN_TIMEOUT = 60 * 60 RUN_INTERVAL = 5 +def check_if_started(unit_name: str): + """Check if the systemd unit has started""" + try: + status = get_service_status(unit_name) + if status == ServiceStatus.ACTIVE: + return True + return False + except subprocess.CalledProcessError: + return False + + +def check_running_status(job: Job, unit_name: str): + """Check if the systemd unit is running""" + try: + status = get_service_status(unit_name) + if status == ServiceStatus.INACTIVE: + Jobs.update( + job=job, + status=JobStatus.FINISHED, + result="System rebuilt.", + progress=100, + ) + return True + if status == ServiceStatus.FAILED: + Jobs.update( + job=job, + status=JobStatus.ERROR, + error="System rebuild failed.", + ) + return True + if status == ServiceStatus.ACTIVE: + log_lines = get_last_log_lines(unit_name, 1) + Jobs.update( + job=job, + status=JobStatus.RUNNING, + status_text=log_lines[0] if len(log_lines) > 0 else "", + ) + return False + return False + except subprocess.CalledProcessError: + return False + + @huey.task() def rebuild_system_task(job: Job, upgrade: bool = False): """Rebuild the system""" @@ -32,88 +79,39 @@ def rebuild_system_task(job: Job, upgrade: bool = False): status=JobStatus.RUNNING, status_text="Starting the system rebuild...", ) - # Get current time to handle timeout - start_time = datetime.now() # Wait for the systemd unit to start - while True: - try: - status = subprocess.run( - ["systemctl", "is-active", unit_name], - check=True, - capture_output=True, - text=True, - ) - if status.stdout.strip() == "active": - break - if (datetime.now() - start_time).total_seconds() > START_TIMEOUT: - Jobs.update( - job=job, - status=JobStatus.ERROR, - error="System rebuild timed out.", - ) - return - time.sleep(START_INTERVAL) - except subprocess.CalledProcessError: - pass + try: + wait_until_true( + lambda: check_if_started(unit_name), + timeout_sec=START_TIMEOUT, + interval=START_INTERVAL, + ) + except TimeoutError: + Jobs.update( + job=job, + status=JobStatus.ERROR, + error="System rebuild timed out.", + ) + return Jobs.update( job=job, status=JobStatus.RUNNING, status_text="Rebuilding the system...", ) # Wait for the systemd unit to finish - while True: - try: - status = subprocess.run( - ["systemctl", "is-active", unit_name], - check=False, - capture_output=True, - text=True, - ) - if status.stdout.strip() == "inactive": - Jobs.update( - job=job, - status=JobStatus.FINISHED, - result="System rebuilt.", - progress=100, - ) - break - elif status.stdout.strip() == "failed": - Jobs.update( - job=job, - status=JobStatus.ERROR, - error="System rebuild failed.", - ) - break - elif status.stdout.strip() == "active": - log_line = subprocess.run( - [ - "journalctl", - "-u", - unit_name, - "-n", - "1", - "-o", - "cat", - ], - check=False, - capture_output=True, - text=True, - ).stdout.strip() - Jobs.update( - job=job, - status=JobStatus.RUNNING, - status_text=f"{log_line}", - ) - except subprocess.CalledProcessError: - pass - if (datetime.now() - start_time).total_seconds() > RUN_TIMEOUT: - Jobs.update( - job=job, - status=JobStatus.ERROR, - error="System rebuild timed out.", - ) - break - time.sleep(RUN_INTERVAL) + try: + wait_until_true( + lambda: check_running_status(job, unit_name), + timeout_sec=RUN_TIMEOUT, + interval=RUN_INTERVAL, + ) + except TimeoutError: + Jobs.update( + job=job, + status=JobStatus.ERROR, + error="System rebuild timed out.", + ) + return except subprocess.CalledProcessError as e: Jobs.update( diff --git a/selfprivacy_api/services/bitwarden/__init__.py b/selfprivacy_api/services/bitwarden/__init__.py index 1590729..1ad44d3 100644 --- a/selfprivacy_api/services/bitwarden/__init__.py +++ b/selfprivacy_api/services/bitwarden/__init__.py @@ -5,9 +5,9 @@ import typing from selfprivacy_api.jobs import Job, Jobs from selfprivacy_api.services.generic_service_mover import FolderMoveNames, move_service -from selfprivacy_api.services.generic_status_getter import get_service_status +from selfprivacy_api.utils.systemd import get_service_status from selfprivacy_api.services.service import Service, ServiceDnsRecord, ServiceStatus -from selfprivacy_api.utils import ReadUserData, WriteUserData, get_domain +from selfprivacy_api.utils import get_domain from selfprivacy_api.utils.block_devices import BlockDevice import selfprivacy_api.utils.network as network_utils from selfprivacy_api.services.bitwarden.icon import BITWARDEN_ICON diff --git a/selfprivacy_api/services/gitea/__init__.py b/selfprivacy_api/services/gitea/__init__.py index 9b6f80f..f4fb559 100644 --- a/selfprivacy_api/services/gitea/__init__.py +++ b/selfprivacy_api/services/gitea/__init__.py @@ -5,9 +5,9 @@ import typing from selfprivacy_api.jobs import Job, Jobs from selfprivacy_api.services.generic_service_mover import FolderMoveNames, move_service -from selfprivacy_api.services.generic_status_getter import get_service_status +from selfprivacy_api.utils.systemd import get_service_status from selfprivacy_api.services.service import Service, ServiceDnsRecord, ServiceStatus -from selfprivacy_api.utils import ReadUserData, WriteUserData, get_domain +from selfprivacy_api.utils import get_domain from selfprivacy_api.utils.block_devices import BlockDevice import selfprivacy_api.utils.network as network_utils from selfprivacy_api.services.gitea.icon import GITEA_ICON diff --git a/selfprivacy_api/services/jitsimeet/__init__.py b/selfprivacy_api/services/jitsimeet/__init__.py index 30663f9..f19cc55 100644 --- a/selfprivacy_api/services/jitsimeet/__init__.py +++ b/selfprivacy_api/services/jitsimeet/__init__.py @@ -4,11 +4,11 @@ import subprocess import typing from selfprivacy_api.jobs import Job -from selfprivacy_api.services.generic_status_getter import ( +from selfprivacy_api.utils.systemd import ( get_service_status_from_several_units, ) from selfprivacy_api.services.service import Service, ServiceDnsRecord, ServiceStatus -from selfprivacy_api.utils import ReadUserData, WriteUserData, get_domain +from selfprivacy_api.utils import get_domain from selfprivacy_api.utils.block_devices import BlockDevice import selfprivacy_api.utils.network as network_utils from selfprivacy_api.services.jitsimeet.icon import JITSI_ICON diff --git a/selfprivacy_api/services/mailserver/__init__.py b/selfprivacy_api/services/mailserver/__init__.py index 536b444..80feb68 100644 --- a/selfprivacy_api/services/mailserver/__init__.py +++ b/selfprivacy_api/services/mailserver/__init__.py @@ -6,7 +6,7 @@ import typing from selfprivacy_api.jobs import Job, Jobs from selfprivacy_api.services.generic_service_mover import FolderMoveNames, move_service -from selfprivacy_api.services.generic_status_getter import ( +from selfprivacy_api.utils.systemd import ( get_service_status_from_several_units, ) from selfprivacy_api.services.service import Service, ServiceDnsRecord, ServiceStatus diff --git a/selfprivacy_api/services/nextcloud/__init__.py b/selfprivacy_api/services/nextcloud/__init__.py index 0da6dd9..f44d0f3 100644 --- a/selfprivacy_api/services/nextcloud/__init__.py +++ b/selfprivacy_api/services/nextcloud/__init__.py @@ -4,9 +4,9 @@ import subprocess import typing from selfprivacy_api.jobs import Job, Jobs from selfprivacy_api.services.generic_service_mover import FolderMoveNames, move_service -from selfprivacy_api.services.generic_status_getter import get_service_status +from selfprivacy_api.utils.systemd import get_service_status from selfprivacy_api.services.service import Service, ServiceDnsRecord, ServiceStatus -from selfprivacy_api.utils import ReadUserData, WriteUserData, get_domain +from selfprivacy_api.utils import get_domain from selfprivacy_api.utils.block_devices import BlockDevice import selfprivacy_api.utils.network as network_utils from selfprivacy_api.services.nextcloud.icon import NEXTCLOUD_ICON diff --git a/selfprivacy_api/services/ocserv/__init__.py b/selfprivacy_api/services/ocserv/__init__.py index a28358d..e680549 100644 --- a/selfprivacy_api/services/ocserv/__init__.py +++ b/selfprivacy_api/services/ocserv/__init__.py @@ -3,9 +3,8 @@ import base64 import subprocess import typing from selfprivacy_api.jobs import Job -from selfprivacy_api.services.generic_status_getter import get_service_status +from selfprivacy_api.utils.systemd import get_service_status from selfprivacy_api.services.service import Service, ServiceDnsRecord, ServiceStatus -from selfprivacy_api.utils import ReadUserData, WriteUserData from selfprivacy_api.utils.block_devices import BlockDevice from selfprivacy_api.services.ocserv.icon import OCSERV_ICON import selfprivacy_api.utils.network as network_utils diff --git a/selfprivacy_api/services/pleroma/__init__.py b/selfprivacy_api/services/pleroma/__init__.py index 1aae50e..be782f2 100644 --- a/selfprivacy_api/services/pleroma/__init__.py +++ b/selfprivacy_api/services/pleroma/__init__.py @@ -4,10 +4,10 @@ import subprocess import typing from selfprivacy_api.jobs import Job, Jobs from selfprivacy_api.services.generic_service_mover import FolderMoveNames, move_service -from selfprivacy_api.services.generic_status_getter import get_service_status +from selfprivacy_api.utils.systemd import get_service_status from selfprivacy_api.services.service import Service, ServiceDnsRecord, ServiceStatus from selfprivacy_api.services.owned_path import OwnedPath -from selfprivacy_api.utils import ReadUserData, WriteUserData, get_domain +from selfprivacy_api.utils import get_domain from selfprivacy_api.utils.block_devices import BlockDevice import selfprivacy_api.utils.network as network_utils from selfprivacy_api.services.pleroma.icon import PLEROMA_ICON diff --git a/selfprivacy_api/services/generic_status_getter.py b/selfprivacy_api/utils/systemd.py similarity index 78% rename from selfprivacy_api/services/generic_status_getter.py rename to selfprivacy_api/utils/systemd.py index 46720af..f8b6244 100644 --- a/selfprivacy_api/services/generic_status_getter.py +++ b/selfprivacy_api/utils/systemd.py @@ -1,5 +1,6 @@ """Generic service status fetcher using systemctl""" import subprocess +from typing import List from selfprivacy_api.services.service import ServiceStatus @@ -58,3 +59,24 @@ def get_service_status_from_several_units(services: list[str]) -> ServiceStatus: if ServiceStatus.ACTIVE in service_statuses: return ServiceStatus.ACTIVE return ServiceStatus.OFF + + +def get_last_log_lines(service: str, lines_count: int) -> List[str]: + if lines_count < 1: + raise ValueError("lines_count must be greater than 0") + try: + logs = subprocess.check_output( + [ + "journalctl", + "-u", + service, + "-n", + str(lines_count), + "-o", + "cat", + ], + shell=False, + ).decode("utf-8") + return logs.splitlines() + except subprocess.CalledProcessError: + return [] diff --git a/tests/test_graphql/test_system_nixos_tasks.py b/tests/test_graphql/test_system_nixos_tasks.py index 3f47ad6..a9d2380 100644 --- a/tests/test_graphql/test_system_nixos_tasks.py +++ b/tests/test_graphql/test_system_nixos_tasks.py @@ -4,6 +4,7 @@ import pytest from selfprivacy_api.jobs import JobStatus, Jobs +from tests.test_graphql.common import assert_empty, assert_ok, get_data class ProcessMock: @@ -92,8 +93,7 @@ def test_graphql_system_rebuild_unauthorized(client, fp, action): "query": query, }, ) - assert response.status_code == 200 - assert response.json().get("data") is None + assert_empty(response) assert fp.call_count([fp.any()]) == 0 @@ -111,23 +111,23 @@ def test_graphql_system_rebuild(authorized_client, fp, action, mock_sleep_interv fp.register(["systemctl", "start", unit_name]) # Wait for it to start - fp.register(["systemctl", "is-active", unit_name], stdout="inactive") - fp.register(["systemctl", "is-active", unit_name], stdout="inactive") - fp.register(["systemctl", "is-active", unit_name], stdout="active") + fp.register(["systemctl", "show", unit_name], stdout="ActiveState=inactive") + fp.register(["systemctl", "show", unit_name], stdout="ActiveState=inactive") + fp.register(["systemctl", "show", unit_name], stdout="ActiveState=active") # Check its exectution - fp.register(["systemctl", "is-active", unit_name], stdout="active") + fp.register(["systemctl", "show", unit_name], stdout="ActiveState=active") fp.register( ["journalctl", "-u", unit_name, "-n", "1", "-o", "cat"], stdout="Starting rebuild...", ) - fp.register(["systemctl", "is-active", unit_name], stdout="active") + fp.register(["systemctl", "show", unit_name], stdout="ActiveState=active") fp.register( ["journalctl", "-u", unit_name, "-n", "1", "-o", "cat"], stdout="Rebuilding..." ) - fp.register(["systemctl", "is-active", unit_name], stdout="inactive") + fp.register(["systemctl", "show", unit_name], stdout="ActiveState=inactive") response = authorized_client.post( "/graphql", @@ -135,23 +135,11 @@ def test_graphql_system_rebuild(authorized_client, fp, action, mock_sleep_interv "query": query, }, ) - assert response.status_code == 200 - assert response.json().get("data") is not None - assert ( - response.json()["data"]["system"][f"runSystem{action.capitalize()}"]["success"] - is True - ) - assert ( - response.json()["data"]["system"][f"runSystem{action.capitalize()}"]["message"] - is not None - ) - assert ( - response.json()["data"]["system"][f"runSystem{action.capitalize()}"]["code"] - == 200 - ) + data = get_data(response)["system"][f"runSystem{action.capitalize()}"] + assert_ok(data) assert fp.call_count(["systemctl", "start", unit_name]) == 1 - assert fp.call_count(["systemctl", "is-active", unit_name]) == 6 + assert fp.call_count(["systemctl", "show", unit_name]) == 6 job_id = response.json()["data"]["system"][f"runSystem{action.capitalize()}"][ "job" @@ -176,23 +164,23 @@ def test_graphql_system_rebuild_failed( fp.register(["systemctl", "start", unit_name]) # Wait for it to start - fp.register(["systemctl", "is-active", unit_name], stdout="inactive") - fp.register(["systemctl", "is-active", unit_name], stdout="inactive") - fp.register(["systemctl", "is-active", unit_name], stdout="active") + fp.register(["systemctl", "show", unit_name], stdout="ActiveState=inactive") + fp.register(["systemctl", "show", unit_name], stdout="ActiveState=inactive") + fp.register(["systemctl", "show", unit_name], stdout="ActiveState=active") # Check its exectution - fp.register(["systemctl", "is-active", unit_name], stdout="active") + fp.register(["systemctl", "show", unit_name], stdout="ActiveState=active") fp.register( ["journalctl", "-u", unit_name, "-n", "1", "-o", "cat"], stdout="Starting rebuild...", ) - fp.register(["systemctl", "is-active", unit_name], stdout="active") + fp.register(["systemctl", "show", unit_name], stdout="ActiveState=active") fp.register( ["journalctl", "-u", unit_name, "-n", "1", "-o", "cat"], stdout="Rebuilding..." ) - fp.register(["systemctl", "is-active", unit_name], stdout="failed") + fp.register(["systemctl", "show", unit_name], stdout="ActiveState=failed") response = authorized_client.post( "/graphql", @@ -200,23 +188,11 @@ def test_graphql_system_rebuild_failed( "query": query, }, ) - assert response.status_code == 200 - assert response.json().get("data") is not None - assert ( - response.json()["data"]["system"][f"runSystem{action.capitalize()}"]["success"] - is True - ) - assert ( - response.json()["data"]["system"][f"runSystem{action.capitalize()}"]["message"] - is not None - ) - assert ( - response.json()["data"]["system"][f"runSystem{action.capitalize()}"]["code"] - == 200 - ) + data = get_data(response)["system"][f"runSystem{action.capitalize()}"] + assert_ok(data) assert fp.call_count(["systemctl", "start", unit_name]) == 1 - assert fp.call_count(["systemctl", "is-active", unit_name]) == 6 + assert fp.call_count(["systemctl", "show", unit_name]) == 6 job_id = response.json()["data"]["system"][f"runSystem{action.capitalize()}"][ "job" From f895f2a38b67458257888412712a18bd849e4b58 Mon Sep 17 00:00:00 2001 From: Inex Code Date: Wed, 6 Mar 2024 18:33:55 +0300 Subject: [PATCH 15/35] refactor: Return last 10 log lines when system rebuild failed --- selfprivacy_api/jobs/upgrade_system.py | 11 ++++++++--- tests/test_graphql/test_system_nixos_tasks.py | 4 ++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/selfprivacy_api/jobs/upgrade_system.py b/selfprivacy_api/jobs/upgrade_system.py index f766f7e..940efdb 100644 --- a/selfprivacy_api/jobs/upgrade_system.py +++ b/selfprivacy_api/jobs/upgrade_system.py @@ -43,10 +43,11 @@ def check_running_status(job: Job, unit_name: str): ) return True if status == ServiceStatus.FAILED: + log_lines = get_last_log_lines(unit_name, 10) Jobs.update( job=job, status=JobStatus.ERROR, - error="System rebuild failed.", + error="System rebuild failed. Last log lines:\n" + "\n".join(log_lines), ) return True if status == ServiceStatus.ACTIVE: @@ -87,10 +88,12 @@ def rebuild_system_task(job: Job, upgrade: bool = False): interval=START_INTERVAL, ) except TimeoutError: + log_lines = get_last_log_lines(unit_name, 10) Jobs.update( job=job, status=JobStatus.ERROR, - error="System rebuild timed out.", + error="System rebuild timed out. Last log lines:\n" + + "\n".join(log_lines), ) return Jobs.update( @@ -106,10 +109,12 @@ def rebuild_system_task(job: Job, upgrade: bool = False): interval=RUN_INTERVAL, ) except TimeoutError: + log_lines = get_last_log_lines(unit_name, 10) Jobs.update( job=job, status=JobStatus.ERROR, - error="System rebuild timed out.", + error="System rebuild timed out. Last log lines:\n" + + "\n".join(log_lines), ) return diff --git a/tests/test_graphql/test_system_nixos_tasks.py b/tests/test_graphql/test_system_nixos_tasks.py index a9d2380..b50223e 100644 --- a/tests/test_graphql/test_system_nixos_tasks.py +++ b/tests/test_graphql/test_system_nixos_tasks.py @@ -182,6 +182,10 @@ def test_graphql_system_rebuild_failed( fp.register(["systemctl", "show", unit_name], stdout="ActiveState=failed") + fp.register( + ["journalctl", "-u", unit_name, "-n", "10", "-o", "cat"], stdout="Some error" + ) + response = authorized_client.post( "/graphql", json={ From 5cd1e28632f3c196093d675bd81d5640e3d6ccdf Mon Sep 17 00:00:00 2001 From: def Date: Sat, 15 Oct 2022 18:38:25 +0300 Subject: [PATCH 16/35] add storage tests --- tests/test_graphql/test_api_storage.py | 342 +++++++++++++++++++++++++ 1 file changed, 342 insertions(+) create mode 100644 tests/test_graphql/test_api_storage.py diff --git a/tests/test_graphql/test_api_storage.py b/tests/test_graphql/test_api_storage.py new file mode 100644 index 0000000..98c75f0 --- /dev/null +++ b/tests/test_graphql/test_api_storage.py @@ -0,0 +1,342 @@ +import pytest + + +class BlockDeviceMockReturnNone: + """Mock BlockDevices""" + + def __init__(self, *args, **kwargs): + self.args = args + self.kwargs = kwargs + + def mount(self): + return None + + def unmount(self): + return None + + def resize(self): + return None + + returncode = 0 + + +class BlockDeviceMockReturnTrue: + """Mock BlockDevices""" + + def __init__(self, *args, **kwargs): + self.args = args + self.kwargs = kwargs + + def mount(self): + return True + + def unmount(self): + return True + + def resize(self): + return True + + returncode = 0 + + +class BlockDeviceMockReturnFalse: + """Mock BlockDevices""" + + def __init__(self, *args, **kwargs): + self.args = args + self.kwargs = kwargs + + def mount(self): + return False + + def unmount(self): + return False + + def resize(self): + return False + + returncode = 0 + + +class BlockDevicesMockReturnTrue: + def get_block_device(name: str): # type: ignore + return BlockDeviceMockReturnTrue() + + def __new__(cls, *args, **kwargs): + pass + + def __init__(self): + pass + + +class BlockDevicesMockReturnNone: + def get_block_device(name: str): # type: ignore + return None + + def __new__(cls, *args, **kwargs): + pass + + def __init__(self): + pass + + +@pytest.fixture +def mock_block_devices_return_true(mocker): + mock = mocker.patch( + "selfprivacy_api.graphql.mutations.storage_mutations.BlockDevices", + # "selfprivacy_api.utils.block_devices.BlockDevices", + autospec=True, + return_value=BlockDevicesMockReturnTrue, + ) + return mock + + +@pytest.fixture +def mock_block_devices_return_none(mocker): + mock = mocker.patch( + "selfprivacy_api.utils.block_devices.BlockDevices", + autospec=True, + return_value=BlockDevicesMockReturnNone, + ) + return mock + + +@pytest.fixture +def mock_block_device_return_none(mocker): + mock = mocker.patch( + "selfprivacy_api.utils.block_devices.BlockDevice", + autospec=True, + return_value=BlockDeviceMockReturnNone, + ) + return mock + + +@pytest.fixture +def mock_block_device_return_true(mocker): + mock = mocker.patch( + "selfprivacy_api.utils.block_devices.BlockDevice", + autospec=True, + return_value=BlockDeviceMockReturnTrue, + ) + return mock + + +@pytest.fixture +def mock_block_device_return_false(mocker): + mock = mocker.patch( + "selfprivacy_api.utils.block_devices.BlockDevice", + autospec=True, + return_value=BlockDeviceMockReturnFalse, + ) + return mock + + +API_RESIZE_VOLUME_MUTATION = """ +mutation resizeVolume($name: String!) { + resizeVolume(name: $name) { + success + message + code + } +} +""" + + +def test_graphql_resize_volumea_unathorized_client( + client, mock_block_devices_return_true +): + response = client.post( + "/graphql", + json={ + "query": API_RESIZE_VOLUME_MUTATION, + "variables": {"name": "sdx"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is None + + +def test_graphql_resize_volume_nonexistent_block_device( + authorized_client, mock_block_devices_return_none +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_RESIZE_VOLUME_MUTATION, + "variables": {"name": "sdx"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["resizeVolume"]["code"] == 404 + assert response.json()["data"]["resizeVolume"]["message"] is not None + assert response.json()["data"]["resizeVolume"]["success"] is False + + +def test_graphql_resize_volume(authorized_client, mock_block_devices_return_true): + response = authorized_client.post( + "/graphql", + json={ + "query": API_RESIZE_VOLUME_MUTATION, + "variables": {"name": "sdx"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["resizeVolume"]["code"] == 200 + assert response.json()["data"]["resizeVolume"]["message"] is not None + assert response.json()["data"]["resizeVolume"]["success"] is True + + +API_MOUNT_VOLUME_MUTATION = """ +mutation mountVolume($name: String!) { + mountVolume(name: $name) { + success + message + code + } +} +""" + + +def test_graphql_mount_volume_unathorized_client(client, mock_block_device_return_true): + response = client.post( + "/graphql", + json={ + "query": API_MOUNT_VOLUME_MUTATION, + "variables": {"name": "sdx"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is None + + +def test_graphql_mount_already_mounted_volume( + authorized_client, mock_block_devices_return_none +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_MOUNT_VOLUME_MUTATION, + "variables": {"name": "sdx"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["mountVolume"]["code"] == 404 + assert response.json()["data"]["mountVolume"]["message"] is not None + assert response.json()["data"]["mountVolume"]["success"] is False + + +def test_graphql_mount_not_found_volume( + authorized_client, mock_block_devices_return_none +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_MOUNT_VOLUME_MUTATION, + "variables": {"name": "sdx"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["mountVolume"]["code"] == 404 + assert response.json()["data"]["mountVolume"]["message"] is not None + assert response.json()["data"]["mountVolume"]["success"] is False + + +def test_graphql_mount_volume(authorized_client, mock_block_devices_return_true): + response = authorized_client.post( + "/graphql", + json={ + "query": API_MOUNT_VOLUME_MUTATION, + "variables": {"name": "sdx"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["mountVolume"]["code"] == 200 + assert response.json()["data"]["mountVolume"]["message"] is not None + assert response.json()["data"]["mountVolume"]["success"] is True + + +API_UNMOUNT_VOLUME_MUTATION = """ +mutation unmountVolume($name: String!) { + unmountVolume(name: $name) { + success + message + code + } +} +""" + + +def test_graphql_unmount_volume_unathorized_client( + client, mock_block_devices_return_true +): + response = client.post( + "/graphql", + json={ + "query": API_UNMOUNT_VOLUME_MUTATION, + "variables": {"name": "sdx"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is None + + +def test_graphql_unmount_not_fount_volume( + authorized_client, mock_block_devices_return_true +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_UNMOUNT_VOLUME_MUTATION, + "variables": {"name": "sdx"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["unmountVolume"]["code"] == 404 + assert response.json()["data"]["unmountVolume"]["message"] is not None + assert response.json()["data"]["unmountVolume"]["success"] is False + + +def test_graphql_unmount_volume_false( + authorized_client, mock_block_devices_return_true +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_UNMOUNT_VOLUME_MUTATION, + "variables": {"name": "sdx"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["unmountVolume"]["code"] == 404 + assert response.json()["data"]["unmountVolume"]["message"] is not None + assert response.json()["data"]["unmountVolume"]["success"] is False + + +def test_graphql_unmount_volume(authorized_client, mock_block_devices_return_true): + response = authorized_client.post( + "/graphql", + json={ + "query": API_UNMOUNT_VOLUME_MUTATION, + "variables": {"name": "sdx"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["unmountVolume"]["code"] == 200 + assert response.json()["data"]["unmountVolume"]["message"] is not None + assert response.json()["data"]["unmountVolume"]["success"] is True From e01b8ed8f0ed2fb0962aa077a47a07c1cb644737 Mon Sep 17 00:00:00 2001 From: def Date: Sat, 15 Oct 2022 18:43:05 +0300 Subject: [PATCH 17/35] add test_api_services.py --- tests/test_graphql/test_api_services.py | 598 ++++++++++++++++++++++++ 1 file changed, 598 insertions(+) create mode 100644 tests/test_graphql/test_api_services.py diff --git a/tests/test_graphql/test_api_services.py b/tests/test_graphql/test_api_services.py new file mode 100644 index 0000000..47e6739 --- /dev/null +++ b/tests/test_graphql/test_api_services.py @@ -0,0 +1,598 @@ +import pytest + + +def get_service_by_id_return_none_mock(): + return None + + +def get_service_by_id_mock(): + return "nextcloud" + + +def service_to_graphql_service_mock(): + pass + + +class BlockDevicesMock: + def get_block_device(self, name: str): + pass + + +class BlockDevicesReturnNoneMock: + def get_block_device(self, name: str): + return None + + +class NextcloudMock: + def __init__(self, args, **kwargs): + self.args = args + self.kwargs = kwargs + + def enable(self): + pass + + def disable(self): + pass + + def stop(self): + pass + + def is_movable(self): + return True + + def move_to_volume(self): + pass + + returncode = 0 + + +class NextcloudReturnFalseMock: + def __init__(self, args, **kwargs): + self.args = args + self.kwargs = kwargs + + def enable(self): + pass + + def disable(self): + pass + + def stop(self): + pass + + def is_movable(self): + return False + + def move_to_volume(self): + pass + + returncode = 0 + + +@pytest.fixture +def mock_service_to_graphql_service(mocker): + mock = mocker.patch( + "selfprivacy_api.graphql.common_types.service.service_to_graphql_service", + autospec=True, + return_value=service_to_graphql_service_mock, + ) + return mock + + +@pytest.fixture +def mock_nextcloud(mocker): + mock = mocker.patch( + "selfprivacy_api.services.nextcloud.__init__.Nextcloud", + autospec=True, + return_value=NextcloudMock, + ) + return mock + + +@pytest.fixture +def mock_block_devices_return_none(mocker): + mock = mocker.patch( + "selfprivacy_api.utils.block_devices.BlockDevices", + autospec=True, + return_value=BlockDevicesReturnNoneMock, + ) + return mock + + +@pytest.fixture +def mock_block_devices(mocker): + mock = mocker.patch( + "selfprivacy_api.utils.block_devices.BlockDevices", + autospec=True, + return_value=BlockDevicesMock, + ) + return mock + + +@pytest.fixture +def mock_nextcloud_return_false(mocker): + mock = mocker.patch( + "selfprivacy_api.services.nextcloud.__init__.Nextcloud", + autospec=True, + return_value=NextcloudReturnFalseMock, + ) + return mock + + +@pytest.fixture +def mock_get_service_by_id_return_none(mocker): + mock = mocker.patch( + "selfprivacy_api.services.__init__.get_service_by_id", + autospec=True, + return_value=mock_get_service_by_id_return_none, + ) + return mock + + +@pytest.fixture +def mock_get_service_by_id(mocker): + mock = mocker.patch( + "selfprivacy_api.services.__init__.get_service_by_id", + autospec=True, + return_value=mock_get_service_by_id, + ) + return mock + + +#################################################################### + + +API_ENABLE_SERVICE_MUTATION = """ +mutation enableService($service_id: String!) { + enableService(service_id: $service_id) { + success + message + code + } +} +""" + + +def test_graphql_enable_service_unathorized_client( + client, mock_get_service_by_id_return_none, mock_nextcloud +): + response = client.post( + "/graphql", + json={ + "query": API_ENABLE_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is None + + +def test_graphql_enable_not_found_service( + authorized_client, mock_get_service_by_id_return_none, mock_nextcloud +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_ENABLE_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["enableService"]["code"] == 404 + assert response.json()["data"]["enableService"]["message"] is not None + assert response.json()["data"]["enableService"]["success"] is False + + +def test_graphql_enable_service( + authorized_client, mock_get_service_by_id, mock_nextcloud +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_ENABLE_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["enableService"]["code"] == 200 + assert response.json()["data"]["enableService"]["message"] is not None + assert response.json()["data"]["enableService"]["success"] is True + + +API_DISABLE_SERVICE_MUTATION = """ +mutation disableService($service_id: String!) { + disableService(service_id: $service_id) { + success + message + code + } +} +""" + + +def test_graphql_disable_service_unathorized_client( + client, mock_get_service_by_id_return_none, mock_nextcloud +): + response = client.post( + "/graphql", + json={ + "query": API_DISABLE_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is None + + +def test_graphql_disable_not_found_service( + authorized_client, mock_get_service_by_id_return_none, mock_nextcloud +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_DISABLE_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["disableService"]["code"] == 404 + assert response.json()["data"]["disableService"]["message"] is not None + assert response.json()["data"]["disableService"]["success"] is False + + +def test_graphql_disable_services( + authorized_client, mock_get_service_by_id, mock_nextcloud +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_DISABLE_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["disableService"]["code"] == 200 + assert response.json()["data"]["disableService"]["message"] is not None + assert response.json()["data"]["disableService"]["success"] is True + + +API_STOP_SERVICE_MUTATION = """ +mutation stopService($service_id: String!) { + stopService(service_id: $service_id) { + success + message + code + } +} +""" + + +def test_graphql_stop_service_unathorized_client( + client, + mock_get_service_by_id_return_none, + mock_nextcloud, + mock_service_to_graphql_service, +): + response = client.post( + "/graphql", + json={ + "query": API_STOP_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is None + + +def test_graphql_stop_not_found_service( + authorized_client, + mock_get_service_by_id_return_none, + mock_nextcloud, + mock_service_to_graphql_service, +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_STOP_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["stopService"]["code"] == 404 + assert response.json()["data"]["stopService"]["message"] is not None + assert response.json()["data"]["stopService"]["success"] is False + + +def test_graphql_stop_services( + authorized_client, + mock_get_service_by_id, + mock_nextcloud, + mock_service_to_graphql_service, +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_STOP_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["stopService"]["code"] == 200 + assert response.json()["data"]["stopService"]["message"] is not None + assert response.json()["data"]["stopService"]["success"] is True + + +API_START_SERVICE_MUTATION = """ +mutation startService($service_id: String!) { + startService(service_id: $service_id) { + success + message + code + } +} +""" + + +def test_graphql_start_service_unathorized_client( + client, + mock_get_service_by_id_return_none, + mock_nextcloud, + mock_service_to_graphql_service, +): + response = client.post( + "/graphql", + json={ + "query": API_START_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is None + + +def test_graphql_start_not_found_service( + authorized_client, + mock_get_service_by_id_return_none, + mock_nextcloud, + mock_service_to_graphql_service, +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_START_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["startService"]["code"] == 404 + assert response.json()["data"]["startService"]["message"] is not None + assert response.json()["data"]["startService"]["success"] is False + + +def test_graphql_start_services( + authorized_client, + mock_get_service_by_id, + mock_nextcloud, + mock_service_to_graphql_service, +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_START_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["startService"]["code"] == 200 + assert response.json()["data"]["startService"]["message"] is not None + assert response.json()["data"]["startService"]["success"] is True + + +API_RESTART_SERVICE_MUTATION = """ +mutation restartService($service_id: String!) { + restartService(service_id: $service_id) { + success + message + code + } +} +""" + + +def test_graphql_restart_service_unathorized_client( + client, + mock_get_service_by_id_return_none, + mock_nextcloud, + mock_service_to_graphql_service, +): + response = client.post( + "/graphql", + json={ + "query": API_RESTART_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is None + + +def test_graphql_restart_not_found_service( + authorized_client, + mock_get_service_by_id_return_none, + mock_nextcloud, + mock_service_to_graphql_service, +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_RESTART_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["restartService"]["code"] == 404 + assert response.json()["data"]["restartService"]["message"] is not None + assert response.json()["data"]["restartService"]["success"] is False + + +def test_graphql_restart_service( + authorized_client, + mock_get_service_by_id, + mock_nextcloud, + mock_service_to_graphql_service, +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_RESTART_SERVICE_MUTATION, + "variables": {"service_id": "nextcloud"}, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["restartService"]["code"] == 200 + assert response.json()["data"]["restartService"]["message"] is not None + assert response.json()["data"]["restartService"]["success"] is True + + +API_MOVE_SERVICE_MUTATION = """ +mutation moveService($input: MoveServiceInput!) { + moveService(input: $input) { + success + message + code + } +} +""" + + +def test_graphql_move_service_unathorized_client( + client, + mock_get_service_by_id_return_none, + mock_nextcloud, + mock_service_to_graphql_service, +): + response = client.post( + "/graphql", + json={ + "query": API_MOVE_SERVICE_MUTATION, + "variables": { + "input": {"service_id": "nextcloud", "location": "sdx"}, + }, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is None + + +def test_graphql_move_not_found_service( + authorized_client, + mock_get_service_by_id_return_none, + mock_nextcloud, + mock_service_to_graphql_service, +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_MOVE_SERVICE_MUTATION, + "variables": { + "input": {"service_id": "nextcloud", "location": "sdx"}, + }, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["moveService"]["code"] == 404 + assert response.json()["data"]["moveService"]["message"] is not None + assert response.json()["data"]["moveService"]["success"] is False + + +def test_graphql_move_not_moveble_service( + authorized_client, + mock_get_service_by_id, + mock_nextcloud_return_false, + mock_service_to_graphql_service, +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_MOVE_SERVICE_MUTATION, + "variables": { + "input": {"service_id": "nextcloud", "location": "sdx"}, + }, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["moveService"]["code"] == 400 + assert response.json()["data"]["moveService"]["message"] is not None + assert response.json()["data"]["moveService"]["success"] is False + + +def test_graphql_move_service_volume_not_found( + authorized_client, + mock_get_service_by_id, + mock_nextcloud, + mock_service_to_graphql_service, + mock_block_devices_return_none, +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_MOVE_SERVICE_MUTATION, + "variables": { + "input": {"service_id": "nextcloud", "location": "sdx"}, + }, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["moveService"]["code"] == 400 + assert response.json()["data"]["moveService"]["message"] is not None + assert response.json()["data"]["moveService"]["success"] is False + + +def test_graphql_move_service( + authorized_client, + mock_get_service_by_id, + mock_nextcloud, + mock_service_to_graphql_service, + mock_block_devices, +): + response = authorized_client.post( + "/graphql", + json={ + "query": API_MOVE_SERVICE_MUTATION, + "variables": { + "input": {"service_id": "nextcloud", "location": "sdx"}, + }, + }, + ) + assert response.status_code == 200 + assert response.json().get("data") is not None + + assert response.json()["data"]["moveService"]["code"] == 200 + assert response.json()["data"]["moveService"]["message"] is not None + assert response.json()["data"]["moveService"]["success"] is True From b5183948af386ec7534a3d7642e0bbade6fa5215 Mon Sep 17 00:00:00 2001 From: def Date: Fri, 21 Oct 2022 20:37:32 +0400 Subject: [PATCH 18/35] fix: service tests --- tests/test_graphql/test_api_services.py | 265 ++++++++++-------- .../test_api_services/one_user.json | 61 ++++ tests/test_graphql/test_api_storage.py | 4 +- 3 files changed, 208 insertions(+), 122 deletions(-) create mode 100644 tests/test_graphql/test_api_services/one_user.json diff --git a/tests/test_graphql/test_api_services.py b/tests/test_graphql/test_api_services.py index 47e6739..cc33070 100644 --- a/tests/test_graphql/test_api_services.py +++ b/tests/test_graphql/test_api_services.py @@ -1,100 +1,99 @@ +# pylint: disable=redefined-outer-name +# pylint: disable=unused-argument import pytest - -def get_service_by_id_return_none_mock(): - return None +from tests.common import read_json -def get_service_by_id_mock(): - return "nextcloud" - - -def service_to_graphql_service_mock(): - pass - - -class BlockDevicesMock: - def get_block_device(self, name: str): - pass - - -class BlockDevicesReturnNoneMock: - def get_block_device(self, name: str): - return None - - -class NextcloudMock: +class NextcloudMockReturnTrue: def __init__(self, args, **kwargs): self.args = args self.kwargs = kwargs - def enable(self): + def enable(): pass - def disable(self): + def disable(): pass - def stop(self): + def stop(): pass - def is_movable(self): + def is_movable(): return True - def move_to_volume(self): + def move_to_volume(what): + return None + + def start(): + pass + + def restart(): pass returncode = 0 -class NextcloudReturnFalseMock: +class BlockDevices: + def get_block_device(location): + return True + +class ProcessMock: + """Mock subprocess.Popen""" + def __init__(self, args, **kwargs): self.args = args self.kwargs = kwargs - def enable(self): - pass - - def disable(self): - pass - - def stop(self): - pass - - def is_movable(self): - return False - - def move_to_volume(self): - pass + def communicate(): # pylint: disable=no-method-argument + return (b"", None) returncode = 0 +@pytest.fixture +def mock_subprocess_popen(mocker): + mock = mocker.patch("subprocess.Popen", autospec=True, return_value=ProcessMock) + return mock + + +@pytest.fixture +def one_user(mocker, datadir): + mocker.patch("selfprivacy_api.utils.USERDATA_FILE", new=datadir / "one_user.json") + assert read_json(datadir / "one_user.json")["users"] == [ + { + "username": "user1", + "hashedPassword": "HASHED_PASSWORD_1", + "sshKeys": ["ssh-rsa KEY user1@pc"], + } + ] + return datadir + + @pytest.fixture def mock_service_to_graphql_service(mocker): mock = mocker.patch( - "selfprivacy_api.graphql.common_types.service.service_to_graphql_service", + "selfprivacy_api.graphql.mutations.services_mutations.service_to_graphql_service", autospec=True, - return_value=service_to_graphql_service_mock, + return_value=None, ) return mock - @pytest.fixture -def mock_nextcloud(mocker): +def mock_job_to_api_job(mocker): mock = mocker.patch( - "selfprivacy_api.services.nextcloud.__init__.Nextcloud", + "selfprivacy_api.graphql.mutations.services_mutations.job_to_api_job", autospec=True, - return_value=NextcloudMock, + return_value=None, ) return mock - @pytest.fixture def mock_block_devices_return_none(mocker): mock = mocker.patch( "selfprivacy_api.utils.block_devices.BlockDevices", autospec=True, - return_value=BlockDevicesReturnNoneMock, + return_value=None, ) return mock @@ -102,19 +101,9 @@ def mock_block_devices_return_none(mocker): @pytest.fixture def mock_block_devices(mocker): mock = mocker.patch( - "selfprivacy_api.utils.block_devices.BlockDevices", + "selfprivacy_api.graphql.mutations.services_mutations.BlockDevices", autospec=True, - return_value=BlockDevicesMock, - ) - return mock - - -@pytest.fixture -def mock_nextcloud_return_false(mocker): - mock = mocker.patch( - "selfprivacy_api.services.nextcloud.__init__.Nextcloud", - autospec=True, - return_value=NextcloudReturnFalseMock, + return_value=BlockDevices, ) return mock @@ -122,9 +111,9 @@ def mock_nextcloud_return_false(mocker): @pytest.fixture def mock_get_service_by_id_return_none(mocker): mock = mocker.patch( - "selfprivacy_api.services.__init__.get_service_by_id", + "selfprivacy_api.graphql.mutations.services_mutations.get_service_by_id", autospec=True, - return_value=mock_get_service_by_id_return_none, + return_value=None, ) return mock @@ -132,9 +121,9 @@ def mock_get_service_by_id_return_none(mocker): @pytest.fixture def mock_get_service_by_id(mocker): mock = mocker.patch( - "selfprivacy_api.services.__init__.get_service_by_id", + "selfprivacy_api.graphql.mutations.services_mutations.get_service_by_id", autospec=True, - return_value=mock_get_service_by_id, + return_value=NextcloudMockReturnTrue, ) return mock @@ -143,8 +132,8 @@ def mock_get_service_by_id(mocker): API_ENABLE_SERVICE_MUTATION = """ -mutation enableService($service_id: String!) { - enableService(service_id: $service_id) { +mutation enableService($serviceId: String!) { + enableService(serviceId: $serviceId) { success message code @@ -154,13 +143,13 @@ mutation enableService($service_id: String!) { def test_graphql_enable_service_unathorized_client( - client, mock_get_service_by_id_return_none, mock_nextcloud + client, mock_get_service_by_id_return_none, mock_subprocess_popen ): response = client.post( "/graphql", json={ "query": API_ENABLE_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -168,13 +157,17 @@ def test_graphql_enable_service_unathorized_client( def test_graphql_enable_not_found_service( - authorized_client, mock_get_service_by_id_return_none, mock_nextcloud + authorized_client, + mock_get_service_by_id_return_none, + mock_subprocess_popen, + one_user, + mock_service_to_graphql_service, ): response = authorized_client.post( "/graphql", json={ "query": API_ENABLE_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -186,13 +179,17 @@ def test_graphql_enable_not_found_service( def test_graphql_enable_service( - authorized_client, mock_get_service_by_id, mock_nextcloud + authorized_client, + mock_get_service_by_id, + mock_subprocess_popen, + one_user, + mock_service_to_graphql_service, ): response = authorized_client.post( "/graphql", json={ "query": API_ENABLE_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -204,8 +201,8 @@ def test_graphql_enable_service( API_DISABLE_SERVICE_MUTATION = """ -mutation disableService($service_id: String!) { - disableService(service_id: $service_id) { +mutation disableService($serviceId: String!) { + disableService(serviceId: $serviceId) { success message code @@ -215,13 +212,17 @@ mutation disableService($service_id: String!) { def test_graphql_disable_service_unathorized_client( - client, mock_get_service_by_id_return_none, mock_nextcloud + client, + mock_get_service_by_id_return_none, + mock_subprocess_popen, + one_user, + mock_service_to_graphql_service, ): response = client.post( "/graphql", json={ "query": API_DISABLE_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -229,13 +230,17 @@ def test_graphql_disable_service_unathorized_client( def test_graphql_disable_not_found_service( - authorized_client, mock_get_service_by_id_return_none, mock_nextcloud + authorized_client, + mock_get_service_by_id_return_none, + mock_subprocess_popen, + one_user, + mock_service_to_graphql_service, ): response = authorized_client.post( "/graphql", json={ "query": API_DISABLE_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -247,13 +252,17 @@ def test_graphql_disable_not_found_service( def test_graphql_disable_services( - authorized_client, mock_get_service_by_id, mock_nextcloud + authorized_client, + mock_get_service_by_id, + mock_subprocess_popen, + one_user, + mock_service_to_graphql_service, ): response = authorized_client.post( "/graphql", json={ "query": API_DISABLE_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -265,8 +274,8 @@ def test_graphql_disable_services( API_STOP_SERVICE_MUTATION = """ -mutation stopService($service_id: String!) { - stopService(service_id: $service_id) { +mutation stopService($serviceId: String!) { + stopService(serviceId: $serviceId) { success message code @@ -278,14 +287,15 @@ mutation stopService($service_id: String!) { def test_graphql_stop_service_unathorized_client( client, mock_get_service_by_id_return_none, - mock_nextcloud, mock_service_to_graphql_service, + mock_subprocess_popen, + one_user, ): response = client.post( "/graphql", json={ "query": API_STOP_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -295,14 +305,15 @@ def test_graphql_stop_service_unathorized_client( def test_graphql_stop_not_found_service( authorized_client, mock_get_service_by_id_return_none, - mock_nextcloud, mock_service_to_graphql_service, + mock_subprocess_popen, + one_user, ): response = authorized_client.post( "/graphql", json={ "query": API_STOP_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -316,14 +327,15 @@ def test_graphql_stop_not_found_service( def test_graphql_stop_services( authorized_client, mock_get_service_by_id, - mock_nextcloud, mock_service_to_graphql_service, + mock_subprocess_popen, + one_user, ): response = authorized_client.post( "/graphql", json={ "query": API_STOP_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -335,8 +347,8 @@ def test_graphql_stop_services( API_START_SERVICE_MUTATION = """ -mutation startService($service_id: String!) { - startService(service_id: $service_id) { +mutation startService($serviceId: String!) { + startService(serviceId: $serviceId) { success message code @@ -348,14 +360,15 @@ mutation startService($service_id: String!) { def test_graphql_start_service_unathorized_client( client, mock_get_service_by_id_return_none, - mock_nextcloud, mock_service_to_graphql_service, + mock_subprocess_popen, + one_user, ): response = client.post( "/graphql", json={ "query": API_START_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -365,14 +378,15 @@ def test_graphql_start_service_unathorized_client( def test_graphql_start_not_found_service( authorized_client, mock_get_service_by_id_return_none, - mock_nextcloud, mock_service_to_graphql_service, + mock_subprocess_popen, + one_user, ): response = authorized_client.post( "/graphql", json={ "query": API_START_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -386,14 +400,15 @@ def test_graphql_start_not_found_service( def test_graphql_start_services( authorized_client, mock_get_service_by_id, - mock_nextcloud, mock_service_to_graphql_service, + mock_subprocess_popen, + one_user, ): response = authorized_client.post( "/graphql", json={ "query": API_START_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -405,8 +420,8 @@ def test_graphql_start_services( API_RESTART_SERVICE_MUTATION = """ -mutation restartService($service_id: String!) { - restartService(service_id: $service_id) { +mutation restartService($serviceId: String!) { + restartService(serviceId: $serviceId) { success message code @@ -418,14 +433,15 @@ mutation restartService($service_id: String!) { def test_graphql_restart_service_unathorized_client( client, mock_get_service_by_id_return_none, - mock_nextcloud, mock_service_to_graphql_service, + mock_subprocess_popen, + one_user, ): response = client.post( "/graphql", json={ "query": API_RESTART_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -435,14 +451,15 @@ def test_graphql_restart_service_unathorized_client( def test_graphql_restart_not_found_service( authorized_client, mock_get_service_by_id_return_none, - mock_nextcloud, mock_service_to_graphql_service, + mock_subprocess_popen, + one_user, ): response = authorized_client.post( "/graphql", json={ "query": API_RESTART_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -456,14 +473,15 @@ def test_graphql_restart_not_found_service( def test_graphql_restart_service( authorized_client, mock_get_service_by_id, - mock_nextcloud, mock_service_to_graphql_service, + mock_subprocess_popen, + one_user, ): response = authorized_client.post( "/graphql", json={ "query": API_RESTART_SERVICE_MUTATION, - "variables": {"service_id": "nextcloud"}, + "variables": {"serviceId": "nextcloud"}, }, ) assert response.status_code == 200 @@ -488,15 +506,16 @@ mutation moveService($input: MoveServiceInput!) { def test_graphql_move_service_unathorized_client( client, mock_get_service_by_id_return_none, - mock_nextcloud, mock_service_to_graphql_service, + mock_subprocess_popen, + one_user, ): response = client.post( "/graphql", json={ "query": API_MOVE_SERVICE_MUTATION, "variables": { - "input": {"service_id": "nextcloud", "location": "sdx"}, + "input": {"serviceId": "nextcloud", "location": "sdx"}, }, }, ) @@ -507,15 +526,16 @@ def test_graphql_move_service_unathorized_client( def test_graphql_move_not_found_service( authorized_client, mock_get_service_by_id_return_none, - mock_nextcloud, mock_service_to_graphql_service, + mock_subprocess_popen, + one_user, ): response = authorized_client.post( "/graphql", json={ "query": API_MOVE_SERVICE_MUTATION, "variables": { - "input": {"service_id": "nextcloud", "location": "sdx"}, + "input": {"serviceId": "nextcloud", "location": "sdx"}, }, }, ) @@ -529,47 +549,50 @@ def test_graphql_move_not_found_service( def test_graphql_move_not_moveble_service( authorized_client, - mock_get_service_by_id, - mock_nextcloud_return_false, + mock_get_service_by_id_return_none, mock_service_to_graphql_service, + mock_subprocess_popen, + one_user, ): response = authorized_client.post( "/graphql", json={ "query": API_MOVE_SERVICE_MUTATION, "variables": { - "input": {"service_id": "nextcloud", "location": "sdx"}, + "input": {"serviceId": "nextcloud", "location": "sdx"}, }, }, ) assert response.status_code == 200 assert response.json().get("data") is not None - assert response.json()["data"]["moveService"]["code"] == 400 + assert response.json()["data"]["moveService"]["code"] == 404 assert response.json()["data"]["moveService"]["message"] is not None assert response.json()["data"]["moveService"]["success"] is False def test_graphql_move_service_volume_not_found( authorized_client, - mock_get_service_by_id, - mock_nextcloud, + mock_get_service_by_id_return_none, mock_service_to_graphql_service, mock_block_devices_return_none, + mock_subprocess_popen, + one_user, ): response = authorized_client.post( "/graphql", json={ "query": API_MOVE_SERVICE_MUTATION, "variables": { - "input": {"service_id": "nextcloud", "location": "sdx"}, + "input": {"serviceId": "nextcloud", "location": "sdx"}, }, }, ) + assert response.status_code == 200 assert response.json().get("data") is not None - assert response.json()["data"]["moveService"]["code"] == 400 + assert response.json()["data"]["moveService"]["code"] == 404 assert response.json()["data"]["moveService"]["message"] is not None assert response.json()["data"]["moveService"]["success"] is False @@ -577,16 +600,18 @@ def test_graphql_move_service_volume_not_found( def test_graphql_move_service( authorized_client, mock_get_service_by_id, - mock_nextcloud, mock_service_to_graphql_service, mock_block_devices, + mock_subprocess_popen, + one_user, + mock_job_to_api_job, ): response = authorized_client.post( "/graphql", json={ "query": API_MOVE_SERVICE_MUTATION, "variables": { - "input": {"service_id": "nextcloud", "location": "sdx"}, + "input": {"serviceId": "nextcloud", "location": "sdx"}, }, }, ) diff --git a/tests/test_graphql/test_api_services/one_user.json b/tests/test_graphql/test_api_services/one_user.json new file mode 100644 index 0000000..5df2108 --- /dev/null +++ b/tests/test_graphql/test_api_services/one_user.json @@ -0,0 +1,61 @@ +{ + "backblaze": { + "accountId": "ID", + "accountKey": "KEY", + "bucket": "selfprivacy" + }, + "api": { + "token": "TEST_TOKEN", + "enableSwagger": false + }, + "bitwarden": { + "enable": false + }, + "cloudflare": { + "apiKey": "TOKEN" + }, + "databasePassword": "PASSWORD", + "domain": "test.tld", + "hashedMasterPassword": "HASHED_PASSWORD", + "hostname": "test-instance", + "nextcloud": { + "adminPassword": "ADMIN", + "databasePassword": "ADMIN", + "enable": true + }, + "resticPassword": "PASS", + "ssh": { + "enable": true, + "passwordAuthentication": true, + "rootKeys": [ + "ssh-ed25519 KEY test@pc" + ] + }, + "username": "tester", + "gitea": { + "enable": false + }, + "ocserv": { + "enable": true + }, + "pleroma": { + "enable": true + }, + "autoUpgrade": { + "enable": true, + "allowReboot": true + }, + "timezone": "Europe/Moscow", + "sshKeys": [ + "ssh-rsa KEY test@pc" + ], + "users": [ + { + "username": "user1", + "hashedPassword": "HASHED_PASSWORD_1", + "sshKeys": [ + "ssh-rsa KEY user1@pc" + ] + } + ] +} \ No newline at end of file diff --git a/tests/test_graphql/test_api_storage.py b/tests/test_graphql/test_api_storage.py index 98c75f0..1fa2f78 100644 --- a/tests/test_graphql/test_api_storage.py +++ b/tests/test_graphql/test_api_storage.py @@ -291,7 +291,7 @@ def test_graphql_unmount_volume_unathorized_client( def test_graphql_unmount_not_fount_volume( - authorized_client, mock_block_devices_return_true + authorized_client, mock_block_devices_return_none ): response = authorized_client.post( "/graphql", @@ -309,7 +309,7 @@ def test_graphql_unmount_not_fount_volume( def test_graphql_unmount_volume_false( - authorized_client, mock_block_devices_return_true + authorized_client, mock_block_devices_return_none ): response = authorized_client.post( "/graphql", From 18327ffa851ee4501d9026ac3c0bd41fa6547490 Mon Sep 17 00:00:00 2001 From: def Date: Sun, 23 Oct 2022 21:44:24 +0400 Subject: [PATCH 19/35] test: remove unused mocks, fix tests naming --- tests/test_graphql/test_api_services.py | 21 +++--- tests/test_graphql/test_api_storage.py | 86 ++----------------------- 2 files changed, 18 insertions(+), 89 deletions(-) diff --git a/tests/test_graphql/test_api_services.py b/tests/test_graphql/test_api_services.py index cc33070..ac6d217 100644 --- a/tests/test_graphql/test_api_services.py +++ b/tests/test_graphql/test_api_services.py @@ -38,6 +38,7 @@ class BlockDevices: def get_block_device(location): return True + class ProcessMock: """Mock subprocess.Popen""" @@ -79,6 +80,7 @@ def mock_service_to_graphql_service(mocker): ) return mock + @pytest.fixture def mock_job_to_api_job(mocker): mock = mocker.patch( @@ -88,6 +90,7 @@ def mock_job_to_api_job(mocker): ) return mock + @pytest.fixture def mock_block_devices_return_none(mocker): mock = mocker.patch( @@ -142,7 +145,7 @@ mutation enableService($serviceId: String!) { """ -def test_graphql_enable_service_unathorized_client( +def test_graphql_enable_service_unauthorized_client( client, mock_get_service_by_id_return_none, mock_subprocess_popen ): response = client.post( @@ -211,7 +214,7 @@ mutation disableService($serviceId: String!) { """ -def test_graphql_disable_service_unathorized_client( +def test_graphql_disable_service_unauthorized_client( client, mock_get_service_by_id_return_none, mock_subprocess_popen, @@ -284,7 +287,7 @@ mutation stopService($serviceId: String!) { """ -def test_graphql_stop_service_unathorized_client( +def test_graphql_stop_service_unauthorized_client( client, mock_get_service_by_id_return_none, mock_service_to_graphql_service, @@ -324,7 +327,7 @@ def test_graphql_stop_not_found_service( assert response.json()["data"]["stopService"]["success"] is False -def test_graphql_stop_services( +def test_graphql_stop_service( authorized_client, mock_get_service_by_id, mock_service_to_graphql_service, @@ -357,7 +360,7 @@ mutation startService($serviceId: String!) { """ -def test_graphql_start_service_unathorized_client( +def test_graphql_start_service_unauthorized_client( client, mock_get_service_by_id_return_none, mock_service_to_graphql_service, @@ -397,7 +400,7 @@ def test_graphql_start_not_found_service( assert response.json()["data"]["startService"]["success"] is False -def test_graphql_start_services( +def test_graphql_start_service( authorized_client, mock_get_service_by_id, mock_service_to_graphql_service, @@ -430,7 +433,7 @@ mutation restartService($serviceId: String!) { """ -def test_graphql_restart_service_unathorized_client( +def test_graphql_restart_service_unauthorized_client( client, mock_get_service_by_id_return_none, mock_service_to_graphql_service, @@ -503,7 +506,7 @@ mutation moveService($input: MoveServiceInput!) { """ -def test_graphql_move_service_unathorized_client( +def test_graphql_move_service_unauthorized_client( client, mock_get_service_by_id_return_none, mock_service_to_graphql_service, @@ -547,7 +550,7 @@ def test_graphql_move_not_found_service( assert response.json()["data"]["moveService"]["success"] is False -def test_graphql_move_not_moveble_service( +def test_graphql_move_not_movable_service( authorized_client, mock_get_service_by_id_return_none, mock_service_to_graphql_service, diff --git a/tests/test_graphql/test_api_storage.py b/tests/test_graphql/test_api_storage.py index 1fa2f78..f4acdaa 100644 --- a/tests/test_graphql/test_api_storage.py +++ b/tests/test_graphql/test_api_storage.py @@ -1,25 +1,6 @@ import pytest -class BlockDeviceMockReturnNone: - """Mock BlockDevices""" - - def __init__(self, *args, **kwargs): - self.args = args - self.kwargs = kwargs - - def mount(self): - return None - - def unmount(self): - return None - - def resize(self): - return None - - returncode = 0 - - class BlockDeviceMockReturnTrue: """Mock BlockDevices""" @@ -39,25 +20,6 @@ class BlockDeviceMockReturnTrue: returncode = 0 -class BlockDeviceMockReturnFalse: - """Mock BlockDevices""" - - def __init__(self, *args, **kwargs): - self.args = args - self.kwargs = kwargs - - def mount(self): - return False - - def unmount(self): - return False - - def resize(self): - return False - - returncode = 0 - - class BlockDevicesMockReturnTrue: def get_block_device(name: str): # type: ignore return BlockDeviceMockReturnTrue() @@ -101,16 +63,6 @@ def mock_block_devices_return_none(mocker): return mock -@pytest.fixture -def mock_block_device_return_none(mocker): - mock = mocker.patch( - "selfprivacy_api.utils.block_devices.BlockDevice", - autospec=True, - return_value=BlockDeviceMockReturnNone, - ) - return mock - - @pytest.fixture def mock_block_device_return_true(mocker): mock = mocker.patch( @@ -121,16 +73,6 @@ def mock_block_device_return_true(mocker): return mock -@pytest.fixture -def mock_block_device_return_false(mocker): - mock = mocker.patch( - "selfprivacy_api.utils.block_devices.BlockDevice", - autospec=True, - return_value=BlockDeviceMockReturnFalse, - ) - return mock - - API_RESIZE_VOLUME_MUTATION = """ mutation resizeVolume($name: String!) { resizeVolume(name: $name) { @@ -142,7 +84,7 @@ mutation resizeVolume($name: String!) { """ -def test_graphql_resize_volumea_unathorized_client( +def test_graphql_resize_volume_unauthorized_client( client, mock_block_devices_return_true ): response = client.post( @@ -201,7 +143,9 @@ mutation mountVolume($name: String!) { """ -def test_graphql_mount_volume_unathorized_client(client, mock_block_device_return_true): +def test_graphql_mount_volume_unauthorized_client( + client, mock_block_device_return_true +): response = client.post( "/graphql", json={ @@ -276,7 +220,7 @@ mutation unmountVolume($name: String!) { """ -def test_graphql_unmount_volume_unathorized_client( +def test_graphql_unmount_volume_unauthorized_client( client, mock_block_devices_return_true ): response = client.post( @@ -290,25 +234,7 @@ def test_graphql_unmount_volume_unathorized_client( assert response.json().get("data") is None -def test_graphql_unmount_not_fount_volume( - authorized_client, mock_block_devices_return_none -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_UNMOUNT_VOLUME_MUTATION, - "variables": {"name": "sdx"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["unmountVolume"]["code"] == 404 - assert response.json()["data"]["unmountVolume"]["message"] is not None - assert response.json()["data"]["unmountVolume"]["success"] is False - - -def test_graphql_unmount_volume_false( +def test_graphql_unmount_not_found_volume( authorized_client, mock_block_devices_return_none ): response = authorized_client.post( From 28fdf8fb49c8db792db5cb2f198b2fa190bfe559 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 29 Jan 2024 16:51:35 +0000 Subject: [PATCH 20/35] refactor(service_mover): decompose the giant move_service --- .../services/generic_service_mover.py | 263 ++++++++---------- 1 file changed, 115 insertions(+), 148 deletions(-) diff --git a/selfprivacy_api/services/generic_service_mover.py b/selfprivacy_api/services/generic_service_mover.py index 819b48e..20c717b 100644 --- a/selfprivacy_api/services/generic_service_mover.py +++ b/selfprivacy_api/services/generic_service_mover.py @@ -2,18 +2,24 @@ from __future__ import annotations import subprocess -import time import pathlib import shutil +from typing import List from pydantic import BaseModel from selfprivacy_api.jobs import Job, JobStatus, Jobs from selfprivacy_api.utils.huey import huey from selfprivacy_api.utils.block_devices import BlockDevice from selfprivacy_api.utils import ReadUserData, WriteUserData -from selfprivacy_api.services.service import Service, ServiceStatus +from selfprivacy_api.services.service import Service from selfprivacy_api.services.owned_path import OwnedPath +from selfprivacy_api.services.service import StoppedService + + +class MoveError(Exception): + """Move failed""" + class FolderMoveNames(BaseModel): name: str @@ -45,110 +51,94 @@ class FolderMoveNames(BaseModel): @huey.task() def move_service( service: Service, - volume: BlockDevice, + new_volume: BlockDevice, job: Job, - folder_names: list[FolderMoveNames], - userdata_location: str, + folder_names: List[FolderMoveNames], + userdata_location: str = None, # deprecated, not used ): - """Move a service to another volume.""" - job = Jobs.update( - job=job, - status_text="Performing pre-move checks...", - status=JobStatus.RUNNING, - ) + """ + Move a service to another volume. + Is not allowed to raise errors because it is a task. + """ service_name = service.get_display_name() - with ReadUserData() as user_data: - if not user_data.get("useBinds", False): + old_volume = service.get_drive() + report_progress(0, job, "Performing pre-move checks...") + + try: + with ReadUserData() as user_data: + if not user_data.get("useBinds", False): + raise MoveError("Server is not using binds.") + + check_volume(new_volume, service) + check_folders(old_volume, folder_names) + + report_progress(5, job, f"Stopping {service_name}...") + + with StoppedService(service): + report_progress(10, job, "Unmounting folders from old volume...") + unmount_old_volume(folder_names) + + report_progress(20, job, "Moving data to new volume...") + move_folders_to_volume(folder_names, old_volume, new_volume, job) + + report_progress(70, job, f"Making sure {service_name} owns its files...") + chown_folders(folder_names, new_volume, job, service) + + report_progress(90, job, f"Mounting {service_name} data...") + mount_folders(folder_names, new_volume) + + report_progress(95, job, f"Finishing moving {service_name}...") + update_volume_in_userdata(service, new_volume) + Jobs.update( job=job, - status=JobStatus.ERROR, - error="Server is not using binds.", + status=JobStatus.FINISHED, + result=f"{service_name} moved successfully.", + status_text=f"Starting {service_name}...", + progress=100, ) - return + except Exception as e: + Jobs.update( + job=job, + status=JobStatus.ERROR, + error=type(e).__name__ + " " + str(e), + ) + + +def check_volume(new_volume: BlockDevice, service: Service) -> bool: + service_name = service.get_display_name() + old_volume_name: str = service.get_drive() + # Check if we are on the same volume - old_volume = service.get_drive() - if old_volume == volume.name: - Jobs.update( - job=job, - status=JobStatus.ERROR, - error=f"{service_name} is already on this volume.", - ) - return + if old_volume_name == new_volume.name: + raise MoveError(f"{service_name} is already on volume {new_volume}") + # Check if there is enough space on the new volume - if int(volume.fsavail) < service.get_storage_usage(): - Jobs.update( - job=job, - status=JobStatus.ERROR, - error="Not enough space on the new volume.", - ) - return + if int(new_volume.fsavail) < service.get_storage_usage(): + raise MoveError("Not enough space on the new volume.") + # Make sure the volume is mounted - if not volume.is_root() and f"/volumes/{volume.name}" not in volume.mountpoints: - Jobs.update( - job=job, - status=JobStatus.ERROR, - error="Volume is not mounted.", - ) - return + if ( + not new_volume.is_root() + and f"/volumes/{new_volume.name}" not in new_volume.mountpoints + ): + raise MoveError("Volume is not mounted.") + + +def check_folders(old_volume: BlockDevice, folder_names: List[FolderMoveNames]) -> None: # Make sure current actual directory exists and if its user and group are correct for folder in folder_names: - if not pathlib.Path(f"/volumes/{old_volume}/{folder.name}").exists(): - Jobs.update( - job=job, - status=JobStatus.ERROR, - error=f"{service_name} is not found.", - ) - return - if not pathlib.Path(f"/volumes/{old_volume}/{folder.name}").is_dir(): - Jobs.update( - job=job, - status=JobStatus.ERROR, - error=f"{service_name} is not a directory.", - ) - return - if ( - not pathlib.Path(f"/volumes/{old_volume}/{folder.name}").owner() - == folder.owner - ): - Jobs.update( - job=job, - status=JobStatus.ERROR, - error=f"{service_name} owner is not {folder.owner}.", - ) - return + path = pathlib.Path(f"/volumes/{old_volume}/{folder.name}") - # Stop service - Jobs.update( - job=job, - status=JobStatus.RUNNING, - status_text=f"Stopping {service_name}...", - progress=5, - ) - service.stop() - # Wait for the service to stop, check every second - # If it does not stop in 30 seconds, abort - for _ in range(30): - if service.get_status() not in ( - ServiceStatus.ACTIVATING, - ServiceStatus.DEACTIVATING, - ): - break - time.sleep(1) - else: - Jobs.update( - job=job, - status=JobStatus.ERROR, - error=f"{service_name} did not stop in 30 seconds.", - ) - return + if not path.exists(): + raise MoveError(f"{path} is not found.") + if not path.is_dir(): + raise MoveError(f"{path} is not a directory.") + if path.owner() != folder.owner: + raise MoveError(f"{path} owner is not {folder.owner}.") - # Unmount old volume - Jobs.update( - job=job, - status_text="Unmounting old folder...", - status=JobStatus.RUNNING, - progress=10, - ) + +def unmount_old_volume(folder_names: List[FolderMoveNames]) -> None: for folder in folder_names: try: subprocess.run( @@ -156,39 +146,31 @@ def move_service( check=True, ) except subprocess.CalledProcessError: - Jobs.update( - job=job, - status=JobStatus.ERROR, - error="Unable to unmount old volume.", - ) - return + raise MoveError("Unable to unmount old volume.") + + +def move_folders_to_volume( + folder_names: List[FolderMoveNames], + old_volume: BlockDevice, + new_volume: BlockDevice, + job: Job, +) -> None: # Move data to new volume and set correct permissions - Jobs.update( - job=job, - status_text="Moving data to new volume...", - status=JobStatus.RUNNING, - progress=20, - ) - current_progress = 20 + current_progress = job.progress folder_percentage = 50 // len(folder_names) for folder in folder_names: shutil.move( f"/volumes/{old_volume}/{folder.name}", - f"/volumes/{volume.name}/{folder.name}", - ) - Jobs.update( - job=job, - status_text="Moving data to new volume...", - status=JobStatus.RUNNING, - progress=current_progress + folder_percentage, + f"/volumes/{new_volume.name}/{folder.name}", ) + progress = current_progress + folder_percentage + report_progress(progress, job, "Moving data to new volume...") - Jobs.update( - job=job, - status_text=f"Making sure {service_name} owns its files...", - status=JobStatus.RUNNING, - progress=70, - ) + +def chown_folders( + folder_names: List[FolderMoveNames], volume: BlockDevice, job: Job, service: Service +) -> None: + service_name = service.get_display_name() for folder in folder_names: try: subprocess.run( @@ -208,14 +190,8 @@ def move_service( error=f"Unable to set ownership of new volume. {service_name} may not be able to access its files. Continuing anyway.", ) - # Mount new volume - Jobs.update( - job=job, - status_text=f"Mounting {service_name} data...", - status=JobStatus.RUNNING, - progress=90, - ) +def mount_folders(folder_names: List[FolderMoveNames], volume: BlockDevice) -> None: for folder in folder_names: try: subprocess.run( @@ -229,32 +205,23 @@ def move_service( ) except subprocess.CalledProcessError as error: print(error.output) - Jobs.update( - job=job, - status=JobStatus.ERROR, - error="Unable to mount new volume.", - ) - return + raise MoveError(f"Unable to mount new volume:{error.output}") - # Update userdata - Jobs.update( - job=job, - status_text="Finishing move...", - status=JobStatus.RUNNING, - progress=95, - ) + +def update_volume_in_userdata(service: Service, volume: BlockDevice): with WriteUserData() as user_data: + service_id = service.get_id() if "modules" not in user_data: user_data["modules"] = {} - if userdata_location not in user_data["modules"]: - user_data["modules"][userdata_location] = {} - user_data["modules"][userdata_location]["location"] = volume.name - # Start service - service.start() + if service_id not in user_data["modules"]: + user_data["modules"][service_id] = {} + user_data["modules"][service_id]["location"] = volume.name + + +def report_progress(progress: int, job: Job, status_text: str) -> None: Jobs.update( job=job, - status=JobStatus.FINISHED, - result=f"{service_name} moved successfully.", - status_text=f"Starting {service_name}...", - progress=100, + status=JobStatus.RUNNING, + status_text=status_text, + progress=progress, ) From d34db3d6611e00007478d86c65403be13fefc7b2 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 29 Jan 2024 16:53:32 +0000 Subject: [PATCH 21/35] fix(services): report moving errors fully --- selfprivacy_api/graphql/mutations/services_mutations.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/selfprivacy_api/graphql/mutations/services_mutations.py b/selfprivacy_api/graphql/mutations/services_mutations.py index 9bacf66..e2aea34 100644 --- a/selfprivacy_api/graphql/mutations/services_mutations.py +++ b/selfprivacy_api/graphql/mutations/services_mutations.py @@ -157,7 +157,7 @@ class ServicesMutations: if service is None: return ServiceJobMutationReturn( success=False, - message="Service not found.", + message=f"Service not found: {input.service_id}", code=404, ) # TODO: make serviceImmovable and BlockdeviceNotFound exceptions @@ -165,7 +165,7 @@ class ServicesMutations: if not service.is_movable(): return ServiceJobMutationReturn( success=False, - message="Service is not movable.", + message=f"Service is not movable: {service.get_display_name()}", code=400, service=service_to_graphql_service(service), ) @@ -173,7 +173,7 @@ class ServicesMutations: if volume is None: return ServiceJobMutationReturn( success=False, - message="Volume not found.", + message=f"Volume not found: {input.location}", code=404, service=service_to_graphql_service(service), ) @@ -197,7 +197,7 @@ class ServicesMutations: else: return ServiceJobMutationReturn( success=False, - message=f"Service move failure: {job.status_text}", + message=f"Service move failure: {job.status_text}: {job.error}", code=400, service=service_to_graphql_service(service), job=job_to_api_job(job), From 2519a50aac38aaa5f7e7ccf283af45af6ded2441 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 29 Jan 2024 16:54:31 +0000 Subject: [PATCH 22/35] test(services): merge def and current service tests --- tests/test_graphql/test_api_services.py | 626 ------------------------ tests/test_graphql/test_services.py | 81 +++ 2 files changed, 81 insertions(+), 626 deletions(-) delete mode 100644 tests/test_graphql/test_api_services.py diff --git a/tests/test_graphql/test_api_services.py b/tests/test_graphql/test_api_services.py deleted file mode 100644 index ac6d217..0000000 --- a/tests/test_graphql/test_api_services.py +++ /dev/null @@ -1,626 +0,0 @@ -# pylint: disable=redefined-outer-name -# pylint: disable=unused-argument -import pytest - -from tests.common import read_json - - -class NextcloudMockReturnTrue: - def __init__(self, args, **kwargs): - self.args = args - self.kwargs = kwargs - - def enable(): - pass - - def disable(): - pass - - def stop(): - pass - - def is_movable(): - return True - - def move_to_volume(what): - return None - - def start(): - pass - - def restart(): - pass - - returncode = 0 - - -class BlockDevices: - def get_block_device(location): - return True - - -class ProcessMock: - """Mock subprocess.Popen""" - - def __init__(self, args, **kwargs): - self.args = args - self.kwargs = kwargs - - def communicate(): # pylint: disable=no-method-argument - return (b"", None) - - returncode = 0 - - -@pytest.fixture -def mock_subprocess_popen(mocker): - mock = mocker.patch("subprocess.Popen", autospec=True, return_value=ProcessMock) - return mock - - -@pytest.fixture -def one_user(mocker, datadir): - mocker.patch("selfprivacy_api.utils.USERDATA_FILE", new=datadir / "one_user.json") - assert read_json(datadir / "one_user.json")["users"] == [ - { - "username": "user1", - "hashedPassword": "HASHED_PASSWORD_1", - "sshKeys": ["ssh-rsa KEY user1@pc"], - } - ] - return datadir - - -@pytest.fixture -def mock_service_to_graphql_service(mocker): - mock = mocker.patch( - "selfprivacy_api.graphql.mutations.services_mutations.service_to_graphql_service", - autospec=True, - return_value=None, - ) - return mock - - -@pytest.fixture -def mock_job_to_api_job(mocker): - mock = mocker.patch( - "selfprivacy_api.graphql.mutations.services_mutations.job_to_api_job", - autospec=True, - return_value=None, - ) - return mock - - -@pytest.fixture -def mock_block_devices_return_none(mocker): - mock = mocker.patch( - "selfprivacy_api.utils.block_devices.BlockDevices", - autospec=True, - return_value=None, - ) - return mock - - -@pytest.fixture -def mock_block_devices(mocker): - mock = mocker.patch( - "selfprivacy_api.graphql.mutations.services_mutations.BlockDevices", - autospec=True, - return_value=BlockDevices, - ) - return mock - - -@pytest.fixture -def mock_get_service_by_id_return_none(mocker): - mock = mocker.patch( - "selfprivacy_api.graphql.mutations.services_mutations.get_service_by_id", - autospec=True, - return_value=None, - ) - return mock - - -@pytest.fixture -def mock_get_service_by_id(mocker): - mock = mocker.patch( - "selfprivacy_api.graphql.mutations.services_mutations.get_service_by_id", - autospec=True, - return_value=NextcloudMockReturnTrue, - ) - return mock - - -#################################################################### - - -API_ENABLE_SERVICE_MUTATION = """ -mutation enableService($serviceId: String!) { - enableService(serviceId: $serviceId) { - success - message - code - } -} -""" - - -def test_graphql_enable_service_unauthorized_client( - client, mock_get_service_by_id_return_none, mock_subprocess_popen -): - response = client.post( - "/graphql", - json={ - "query": API_ENABLE_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is None - - -def test_graphql_enable_not_found_service( - authorized_client, - mock_get_service_by_id_return_none, - mock_subprocess_popen, - one_user, - mock_service_to_graphql_service, -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_ENABLE_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["enableService"]["code"] == 404 - assert response.json()["data"]["enableService"]["message"] is not None - assert response.json()["data"]["enableService"]["success"] is False - - -def test_graphql_enable_service( - authorized_client, - mock_get_service_by_id, - mock_subprocess_popen, - one_user, - mock_service_to_graphql_service, -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_ENABLE_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["enableService"]["code"] == 200 - assert response.json()["data"]["enableService"]["message"] is not None - assert response.json()["data"]["enableService"]["success"] is True - - -API_DISABLE_SERVICE_MUTATION = """ -mutation disableService($serviceId: String!) { - disableService(serviceId: $serviceId) { - success - message - code - } -} -""" - - -def test_graphql_disable_service_unauthorized_client( - client, - mock_get_service_by_id_return_none, - mock_subprocess_popen, - one_user, - mock_service_to_graphql_service, -): - response = client.post( - "/graphql", - json={ - "query": API_DISABLE_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is None - - -def test_graphql_disable_not_found_service( - authorized_client, - mock_get_service_by_id_return_none, - mock_subprocess_popen, - one_user, - mock_service_to_graphql_service, -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_DISABLE_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["disableService"]["code"] == 404 - assert response.json()["data"]["disableService"]["message"] is not None - assert response.json()["data"]["disableService"]["success"] is False - - -def test_graphql_disable_services( - authorized_client, - mock_get_service_by_id, - mock_subprocess_popen, - one_user, - mock_service_to_graphql_service, -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_DISABLE_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["disableService"]["code"] == 200 - assert response.json()["data"]["disableService"]["message"] is not None - assert response.json()["data"]["disableService"]["success"] is True - - -API_STOP_SERVICE_MUTATION = """ -mutation stopService($serviceId: String!) { - stopService(serviceId: $serviceId) { - success - message - code - } -} -""" - - -def test_graphql_stop_service_unauthorized_client( - client, - mock_get_service_by_id_return_none, - mock_service_to_graphql_service, - mock_subprocess_popen, - one_user, -): - response = client.post( - "/graphql", - json={ - "query": API_STOP_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is None - - -def test_graphql_stop_not_found_service( - authorized_client, - mock_get_service_by_id_return_none, - mock_service_to_graphql_service, - mock_subprocess_popen, - one_user, -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_STOP_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["stopService"]["code"] == 404 - assert response.json()["data"]["stopService"]["message"] is not None - assert response.json()["data"]["stopService"]["success"] is False - - -def test_graphql_stop_service( - authorized_client, - mock_get_service_by_id, - mock_service_to_graphql_service, - mock_subprocess_popen, - one_user, -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_STOP_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["stopService"]["code"] == 200 - assert response.json()["data"]["stopService"]["message"] is not None - assert response.json()["data"]["stopService"]["success"] is True - - -API_START_SERVICE_MUTATION = """ -mutation startService($serviceId: String!) { - startService(serviceId: $serviceId) { - success - message - code - } -} -""" - - -def test_graphql_start_service_unauthorized_client( - client, - mock_get_service_by_id_return_none, - mock_service_to_graphql_service, - mock_subprocess_popen, - one_user, -): - response = client.post( - "/graphql", - json={ - "query": API_START_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is None - - -def test_graphql_start_not_found_service( - authorized_client, - mock_get_service_by_id_return_none, - mock_service_to_graphql_service, - mock_subprocess_popen, - one_user, -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_START_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["startService"]["code"] == 404 - assert response.json()["data"]["startService"]["message"] is not None - assert response.json()["data"]["startService"]["success"] is False - - -def test_graphql_start_service( - authorized_client, - mock_get_service_by_id, - mock_service_to_graphql_service, - mock_subprocess_popen, - one_user, -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_START_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["startService"]["code"] == 200 - assert response.json()["data"]["startService"]["message"] is not None - assert response.json()["data"]["startService"]["success"] is True - - -API_RESTART_SERVICE_MUTATION = """ -mutation restartService($serviceId: String!) { - restartService(serviceId: $serviceId) { - success - message - code - } -} -""" - - -def test_graphql_restart_service_unauthorized_client( - client, - mock_get_service_by_id_return_none, - mock_service_to_graphql_service, - mock_subprocess_popen, - one_user, -): - response = client.post( - "/graphql", - json={ - "query": API_RESTART_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is None - - -def test_graphql_restart_not_found_service( - authorized_client, - mock_get_service_by_id_return_none, - mock_service_to_graphql_service, - mock_subprocess_popen, - one_user, -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_RESTART_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["restartService"]["code"] == 404 - assert response.json()["data"]["restartService"]["message"] is not None - assert response.json()["data"]["restartService"]["success"] is False - - -def test_graphql_restart_service( - authorized_client, - mock_get_service_by_id, - mock_service_to_graphql_service, - mock_subprocess_popen, - one_user, -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_RESTART_SERVICE_MUTATION, - "variables": {"serviceId": "nextcloud"}, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["restartService"]["code"] == 200 - assert response.json()["data"]["restartService"]["message"] is not None - assert response.json()["data"]["restartService"]["success"] is True - - -API_MOVE_SERVICE_MUTATION = """ -mutation moveService($input: MoveServiceInput!) { - moveService(input: $input) { - success - message - code - } -} -""" - - -def test_graphql_move_service_unauthorized_client( - client, - mock_get_service_by_id_return_none, - mock_service_to_graphql_service, - mock_subprocess_popen, - one_user, -): - response = client.post( - "/graphql", - json={ - "query": API_MOVE_SERVICE_MUTATION, - "variables": { - "input": {"serviceId": "nextcloud", "location": "sdx"}, - }, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is None - - -def test_graphql_move_not_found_service( - authorized_client, - mock_get_service_by_id_return_none, - mock_service_to_graphql_service, - mock_subprocess_popen, - one_user, -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_MOVE_SERVICE_MUTATION, - "variables": { - "input": {"serviceId": "nextcloud", "location": "sdx"}, - }, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["moveService"]["code"] == 404 - assert response.json()["data"]["moveService"]["message"] is not None - assert response.json()["data"]["moveService"]["success"] is False - - -def test_graphql_move_not_movable_service( - authorized_client, - mock_get_service_by_id_return_none, - mock_service_to_graphql_service, - mock_subprocess_popen, - one_user, -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_MOVE_SERVICE_MUTATION, - "variables": { - "input": {"serviceId": "nextcloud", "location": "sdx"}, - }, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["moveService"]["code"] == 404 - assert response.json()["data"]["moveService"]["message"] is not None - assert response.json()["data"]["moveService"]["success"] is False - - -def test_graphql_move_service_volume_not_found( - authorized_client, - mock_get_service_by_id_return_none, - mock_service_to_graphql_service, - mock_block_devices_return_none, - mock_subprocess_popen, - one_user, -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_MOVE_SERVICE_MUTATION, - "variables": { - "input": {"serviceId": "nextcloud", "location": "sdx"}, - }, - }, - ) - - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["moveService"]["code"] == 404 - assert response.json()["data"]["moveService"]["message"] is not None - assert response.json()["data"]["moveService"]["success"] is False - - -def test_graphql_move_service( - authorized_client, - mock_get_service_by_id, - mock_service_to_graphql_service, - mock_block_devices, - mock_subprocess_popen, - one_user, - mock_job_to_api_job, -): - response = authorized_client.post( - "/graphql", - json={ - "query": API_MOVE_SERVICE_MUTATION, - "variables": { - "input": {"serviceId": "nextcloud", "location": "sdx"}, - }, - }, - ) - assert response.status_code == 200 - assert response.json().get("data") is not None - - assert response.json()["data"]["moveService"]["code"] == 200 - assert response.json()["data"]["moveService"]["message"] is not None - assert response.json()["data"]["moveService"]["success"] is True diff --git a/tests/test_graphql/test_services.py b/tests/test_graphql/test_services.py index 3983b56..c6784ee 100644 --- a/tests/test_graphql/test_services.py +++ b/tests/test_graphql/test_services.py @@ -10,6 +10,9 @@ from selfprivacy_api.services.test_service import DummyService from tests.common import generate_service_query from tests.test_graphql.common import assert_empty, assert_ok, get_data +from tests.test_block_device_utils import lsblk_singular_mock + +from subprocess import CompletedProcess @pytest.fixture() @@ -23,6 +26,33 @@ def only_dummy_service(dummy_service) -> Generator[DummyService, None, None]: service_module.services.extend(back_copy) +MOVER_MOCK_PROCESS = CompletedProcess(["ls"], returncode=0) + + +@pytest.fixture() +def mock_check_service_mover_folders(mocker): + mock = mocker.patch( + "selfprivacy_api.services.generic_service_mover.check_folders", + autospec=True, + return_value=None, + ) + return mock + + +@pytest.fixture() +def mock_subprocess_run(mocker): + mock = mocker.patch( + "subprocess.run", autospec=True, return_value=MOVER_MOCK_PROCESS + ) + return mock + + +@pytest.fixture() +def mock_shutil_move(mocker): + mock = mocker.patch("shutil.move", autospec=True, return_value=None) + return mock + + API_START_MUTATION = """ mutation TestStartService($service_id: String!) { services { @@ -474,6 +504,15 @@ def test_move_immovable(authorized_client, only_dummy_service): assert data["job"] is None +def test_move_no_such_service(authorized_client, only_dummy_service): + mutation_response = api_move_by_name(authorized_client, "bogus_service", "sda1") + data = get_data(mutation_response)["services"]["moveService"] + assert_errorcode(data, 404) + + assert data["service"] is None + assert data["job"] is None + + def test_move_no_such_volume(authorized_client, only_dummy_service): dummy_service = only_dummy_service mutation_response = api_move(authorized_client, dummy_service, "bogus_volume") @@ -502,6 +541,48 @@ def test_move_same_volume(authorized_client, dummy_service): assert data["job"] is not None +def test_graphql_move_service_without_folders_on_old_volume( + authorized_client, + generic_userdata, + lsblk_singular_mock, + dummy_service: DummyService, +): + target = "sda1" + BlockDevices().update() + assert BlockDevices().get_block_device(target) is not None + + dummy_service.set_simulated_moves(False) + dummy_service.set_drive("sda2") + mutation_response = api_move(authorized_client, dummy_service, target) + + data = get_data(mutation_response)["services"]["moveService"] + assert_errorcode(data, 400) + + +def test_graphql_move_service( + authorized_client, + generic_userdata, + mock_check_service_mover_folders, + lsblk_singular_mock, + dummy_service: DummyService, + mock_subprocess_run, + mock_shutil_move, +): + # Does not check real moving, + # but tests the finished job propagation through API + + target = "sda1" + BlockDevices().update() + assert BlockDevices().get_block_device(target) is not None + + dummy_service.set_simulated_moves(False) + dummy_service.set_drive("sda2") + mutation_response = api_move(authorized_client, dummy_service, target) + + data = get_data(mutation_response)["services"]["moveService"] + assert_ok(data) + + def test_mailservice_cannot_enable_disable(authorized_client): mailservice = get_service_by_id("simple-nixos-mailserver") From b054235d96ab7a47cdb2004c33edf6697365bfea Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 29 Jan 2024 17:06:29 +0000 Subject: [PATCH 23/35] test(services): remove unused json --- .../test_api_services/one_user.json | 61 ------------------- 1 file changed, 61 deletions(-) delete mode 100644 tests/test_graphql/test_api_services/one_user.json diff --git a/tests/test_graphql/test_api_services/one_user.json b/tests/test_graphql/test_api_services/one_user.json deleted file mode 100644 index 5df2108..0000000 --- a/tests/test_graphql/test_api_services/one_user.json +++ /dev/null @@ -1,61 +0,0 @@ -{ - "backblaze": { - "accountId": "ID", - "accountKey": "KEY", - "bucket": "selfprivacy" - }, - "api": { - "token": "TEST_TOKEN", - "enableSwagger": false - }, - "bitwarden": { - "enable": false - }, - "cloudflare": { - "apiKey": "TOKEN" - }, - "databasePassword": "PASSWORD", - "domain": "test.tld", - "hashedMasterPassword": "HASHED_PASSWORD", - "hostname": "test-instance", - "nextcloud": { - "adminPassword": "ADMIN", - "databasePassword": "ADMIN", - "enable": true - }, - "resticPassword": "PASS", - "ssh": { - "enable": true, - "passwordAuthentication": true, - "rootKeys": [ - "ssh-ed25519 KEY test@pc" - ] - }, - "username": "tester", - "gitea": { - "enable": false - }, - "ocserv": { - "enable": true - }, - "pleroma": { - "enable": true - }, - "autoUpgrade": { - "enable": true, - "allowReboot": true - }, - "timezone": "Europe/Moscow", - "sshKeys": [ - "ssh-rsa KEY test@pc" - ], - "users": [ - { - "username": "user1", - "hashedPassword": "HASHED_PASSWORD_1", - "sshKeys": [ - "ssh-rsa KEY user1@pc" - ] - } - ] -} \ No newline at end of file From 7fd09982a44e3beada3a44df60ac16ff6659e79c Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Wed, 31 Jan 2024 10:51:01 +0000 Subject: [PATCH 24/35] fix(services): a better error message --- selfprivacy_api/graphql/mutations/services_mutations.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/selfprivacy_api/graphql/mutations/services_mutations.py b/selfprivacy_api/graphql/mutations/services_mutations.py index e2aea34..e8edbcf 100644 --- a/selfprivacy_api/graphql/mutations/services_mutations.py +++ b/selfprivacy_api/graphql/mutations/services_mutations.py @@ -197,7 +197,7 @@ class ServicesMutations: else: return ServiceJobMutationReturn( success=False, - message=f"Service move failure: {job.status_text}: {job.error}", + message=f"While moving service and performing the step '{job.status_text}', error occured: {job.error}", code=400, service=service_to_graphql_service(service), job=job_to_api_job(job), From d7ef2ed09a08ec5feae00266184564842bb55015 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 12 Feb 2024 14:41:15 +0000 Subject: [PATCH 25/35] refactor(services): make moving a part of generic service functionality --- selfprivacy_api/jobs/__init__.py | 12 + .../services/generic_service_mover.py | 227 ------------------ selfprivacy_api/services/moving.py | 114 +++++++++ selfprivacy_api/services/service.py | 97 +++++++- selfprivacy_api/services/tasks.py | 11 + 5 files changed, 233 insertions(+), 228 deletions(-) delete mode 100644 selfprivacy_api/services/generic_service_mover.py create mode 100644 selfprivacy_api/services/moving.py create mode 100644 selfprivacy_api/services/tasks.py diff --git a/selfprivacy_api/jobs/__init__.py b/selfprivacy_api/jobs/__init__.py index 7310016..7f46e9d 100644 --- a/selfprivacy_api/jobs/__init__.py +++ b/selfprivacy_api/jobs/__init__.py @@ -268,6 +268,18 @@ class Jobs: return False +# A terse way to call a common operation, for readability +# job.report_progress() would be even better +# but it would go against how this file is written +def report_progress(progress: int, job: Job, status_text: str) -> None: + Jobs.update( + job=job, + status=JobStatus.RUNNING, + status_text=status_text, + progress=progress, + ) + + def _redis_key_from_uuid(uuid_string) -> str: return "jobs:" + str(uuid_string) diff --git a/selfprivacy_api/services/generic_service_mover.py b/selfprivacy_api/services/generic_service_mover.py deleted file mode 100644 index 20c717b..0000000 --- a/selfprivacy_api/services/generic_service_mover.py +++ /dev/null @@ -1,227 +0,0 @@ -"""Generic handler for moving services""" - -from __future__ import annotations -import subprocess -import pathlib -import shutil -from typing import List - -from pydantic import BaseModel -from selfprivacy_api.jobs import Job, JobStatus, Jobs -from selfprivacy_api.utils.huey import huey -from selfprivacy_api.utils.block_devices import BlockDevice -from selfprivacy_api.utils import ReadUserData, WriteUserData -from selfprivacy_api.services.service import Service -from selfprivacy_api.services.owned_path import OwnedPath - -from selfprivacy_api.services.service import StoppedService - - -class MoveError(Exception): - """Move failed""" - - -class FolderMoveNames(BaseModel): - name: str - bind_location: str - owner: str - group: str - - @staticmethod - def from_owned_path(path: OwnedPath) -> FolderMoveNames: - return FolderMoveNames( - name=FolderMoveNames.get_foldername(path.path), - bind_location=path.path, - owner=path.owner, - group=path.group, - ) - - @staticmethod - def get_foldername(path: str) -> str: - return path.split("/")[-1] - - @staticmethod - def default_foldermoves(service: Service) -> list[FolderMoveNames]: - return [ - FolderMoveNames.from_owned_path(folder) - for folder in service.get_owned_folders() - ] - - -@huey.task() -def move_service( - service: Service, - new_volume: BlockDevice, - job: Job, - folder_names: List[FolderMoveNames], - userdata_location: str = None, # deprecated, not used -): - """ - Move a service to another volume. - Is not allowed to raise errors because it is a task. - """ - service_name = service.get_display_name() - old_volume = service.get_drive() - report_progress(0, job, "Performing pre-move checks...") - - try: - with ReadUserData() as user_data: - if not user_data.get("useBinds", False): - raise MoveError("Server is not using binds.") - - check_volume(new_volume, service) - check_folders(old_volume, folder_names) - - report_progress(5, job, f"Stopping {service_name}...") - - with StoppedService(service): - report_progress(10, job, "Unmounting folders from old volume...") - unmount_old_volume(folder_names) - - report_progress(20, job, "Moving data to new volume...") - move_folders_to_volume(folder_names, old_volume, new_volume, job) - - report_progress(70, job, f"Making sure {service_name} owns its files...") - chown_folders(folder_names, new_volume, job, service) - - report_progress(90, job, f"Mounting {service_name} data...") - mount_folders(folder_names, new_volume) - - report_progress(95, job, f"Finishing moving {service_name}...") - update_volume_in_userdata(service, new_volume) - - Jobs.update( - job=job, - status=JobStatus.FINISHED, - result=f"{service_name} moved successfully.", - status_text=f"Starting {service_name}...", - progress=100, - ) - except Exception as e: - Jobs.update( - job=job, - status=JobStatus.ERROR, - error=type(e).__name__ + " " + str(e), - ) - - -def check_volume(new_volume: BlockDevice, service: Service) -> bool: - service_name = service.get_display_name() - old_volume_name: str = service.get_drive() - - # Check if we are on the same volume - if old_volume_name == new_volume.name: - raise MoveError(f"{service_name} is already on volume {new_volume}") - - # Check if there is enough space on the new volume - if int(new_volume.fsavail) < service.get_storage_usage(): - raise MoveError("Not enough space on the new volume.") - - # Make sure the volume is mounted - if ( - not new_volume.is_root() - and f"/volumes/{new_volume.name}" not in new_volume.mountpoints - ): - raise MoveError("Volume is not mounted.") - - -def check_folders(old_volume: BlockDevice, folder_names: List[FolderMoveNames]) -> None: - # Make sure current actual directory exists and if its user and group are correct - for folder in folder_names: - path = pathlib.Path(f"/volumes/{old_volume}/{folder.name}") - - if not path.exists(): - raise MoveError(f"{path} is not found.") - if not path.is_dir(): - raise MoveError(f"{path} is not a directory.") - if path.owner() != folder.owner: - raise MoveError(f"{path} owner is not {folder.owner}.") - - -def unmount_old_volume(folder_names: List[FolderMoveNames]) -> None: - for folder in folder_names: - try: - subprocess.run( - ["umount", folder.bind_location], - check=True, - ) - except subprocess.CalledProcessError: - raise MoveError("Unable to unmount old volume.") - - -def move_folders_to_volume( - folder_names: List[FolderMoveNames], - old_volume: BlockDevice, - new_volume: BlockDevice, - job: Job, -) -> None: - # Move data to new volume and set correct permissions - current_progress = job.progress - folder_percentage = 50 // len(folder_names) - for folder in folder_names: - shutil.move( - f"/volumes/{old_volume}/{folder.name}", - f"/volumes/{new_volume.name}/{folder.name}", - ) - progress = current_progress + folder_percentage - report_progress(progress, job, "Moving data to new volume...") - - -def chown_folders( - folder_names: List[FolderMoveNames], volume: BlockDevice, job: Job, service: Service -) -> None: - service_name = service.get_display_name() - for folder in folder_names: - try: - subprocess.run( - [ - "chown", - "-R", - f"{folder.owner}:{folder.group}", - f"/volumes/{volume.name}/{folder.name}", - ], - check=True, - ) - except subprocess.CalledProcessError as error: - print(error.output) - Jobs.update( - job=job, - status=JobStatus.RUNNING, - error=f"Unable to set ownership of new volume. {service_name} may not be able to access its files. Continuing anyway.", - ) - - -def mount_folders(folder_names: List[FolderMoveNames], volume: BlockDevice) -> None: - for folder in folder_names: - try: - subprocess.run( - [ - "mount", - "--bind", - f"/volumes/{volume.name}/{folder.name}", - folder.bind_location, - ], - check=True, - ) - except subprocess.CalledProcessError as error: - print(error.output) - raise MoveError(f"Unable to mount new volume:{error.output}") - - -def update_volume_in_userdata(service: Service, volume: BlockDevice): - with WriteUserData() as user_data: - service_id = service.get_id() - if "modules" not in user_data: - user_data["modules"] = {} - if service_id not in user_data["modules"]: - user_data["modules"][service_id] = {} - user_data["modules"][service_id]["location"] = volume.name - - -def report_progress(progress: int, job: Job, status_text: str) -> None: - Jobs.update( - job=job, - status=JobStatus.RUNNING, - status_text=status_text, - progress=progress, - ) diff --git a/selfprivacy_api/services/moving.py b/selfprivacy_api/services/moving.py new file mode 100644 index 0000000..d667935 --- /dev/null +++ b/selfprivacy_api/services/moving.py @@ -0,0 +1,114 @@ +"""Generic handler for moving services""" + +from __future__ import annotations +import subprocess +import pathlib +import shutil +from typing import List + +from selfprivacy_api.jobs import Job, report_progress +from selfprivacy_api.utils.block_devices import BlockDevice +from selfprivacy_api.services.owned_path import OwnedPath + + +class MoveError(Exception): + """Move failed""" + +def get_foldername(path: str) -> str: + return path.split("/")[-1] + + + + +def check_volume(volume: BlockDevice, space_needed: int) -> bool: + # Check if there is enough space on the new volume + if int(volume.fsavail) < space_needed: + raise MoveError("Not enough space on the new volume.") + + # Make sure the volume is mounted + if ( + not volume.is_root() + and f"/volumes/{volume.name}" not in volume.mountpoints + ): + raise MoveError("Volume is not mounted.") + + +def check_folders(current_volume: BlockDevice, folders: List[OwnedPath]) -> None: + # Make sure current actual directory exists and if its user and group are correct + for folder in folders: + path = pathlib.Path(f"/volumes/{current_volume}/{get_foldername(folder)}") + + if not path.exists(): + raise MoveError(f"{path} is not found.") + if not path.is_dir(): + raise MoveError(f"{path} is not a directory.") + if path.owner() != folder.owner: + raise MoveError(f"{path} owner is not {folder.owner}.") + + +def unbind_folders(owned_folders: List[OwnedPath]) -> None: + for folder in owned_folders: + try: + subprocess.run( + ["umount", folder.path], + check=True, + ) + except subprocess.CalledProcessError: + raise MoveError(f"Unable to unmount folder {folder.path}.") + + +def move_folders_to_volume( + folders: List[OwnedPath], + old_volume: BlockDevice, + new_volume: BlockDevice, + job: Job, +) -> None: + current_progress = job.progress + folder_percentage = 50 // len(folders) + for folder in folders: + folder_name = get_foldername(folder.path) + shutil.move( + f"/volumes/{old_volume}/{folder_name}", + f"/volumes/{new_volume.name}/{folder_name}", + ) + progress = current_progress + folder_percentage + report_progress(progress, job, "Moving data to new volume...") + + +def ensure_folder_ownership( + folders: List[OwnedPath], volume: BlockDevice +) -> None: + for folder in folders: + true_location = f"/volumes/{volume.name}/{get_foldername(folder.path)}" + try: + subprocess.run( + [ + "chown", + "-R", + f"{folder.owner}:{folder.group}", + # Could we just chown the binded location instead? + true_location + ], + check=True, + ) + except subprocess.CalledProcessError as error: + error_message = f"Unable to set ownership of {true_location} :{error.output}" + print(error.output) + raise MoveError(error_message) + + +def bind_folders(folders: List[OwnedPath], volume: BlockDevice) -> None: + for folder in folders: + try: + subprocess.run( + [ + "mount", + "--bind", + f"/volumes/{volume.name}/{get_foldername(folder.path)}", + folder.path, + ], + check=True, + ) + except subprocess.CalledProcessError as error: + print(error.output) + raise MoveError(f"Unable to mount new volume:{error.output}") diff --git a/selfprivacy_api/services/service.py b/selfprivacy_api/services/service.py index 0cca38a..6255f20 100644 --- a/selfprivacy_api/services/service.py +++ b/selfprivacy_api/services/service.py @@ -4,12 +4,14 @@ from enum import Enum from typing import List, Optional from pydantic import BaseModel -from selfprivacy_api.jobs import Job +from selfprivacy_api.jobs import Job, Jobs, JobStatus, report_progress from selfprivacy_api.utils.block_devices import BlockDevice, BlockDevices from selfprivacy_api.services.generic_size_counter import get_storage_usage from selfprivacy_api.services.owned_path import OwnedPath +from selfprivacy_api.services.moving import check_folders, check_volume, unbind_folders, bind_folders, ensure_folder_ownership, MoveError, move_folders_to_volume + from selfprivacy_api import utils from selfprivacy_api.utils.waitloop import wait_until_true from selfprivacy_api.utils import ReadUserData, WriteUserData @@ -294,6 +296,99 @@ class Service(ABC): def get_foldername(path: str) -> str: return path.split("/")[-1] + # TODO: with better json utils, it can be one line, and not a separate function + @classmethod + def set_location(cls, volume: BlockDevice): + """ + Only changes userdata + """ + + with WriteUserData() as user_data: + service_id = cls.get_id() + if "modules" not in user_data: + user_data["modules"] = {} + if service_id not in user_data["modules"]: + user_data["modules"][service_id] = {} + user_data["modules"][service_id]["location"] = volume.name + + def assert_can_move(self, new_volume): + """ + Checks if the service can be moved to new volume + Raises errors if it cannot + """ + with ReadUserData() as user_data: + if not user_data.get("useBinds", False): + raise MoveError("Server is not using binds.") + + current_volume_name = self.get_drive() + service_name = self.get_display_name() + if current_volume_name == new_volume.name: + raise MoveError(f"{service_name} is already on volume {new_volume}") + + check_volume(new_volume, space_needed=self.get_storage_usage()) + + owned_folders = self.get_owned_folders() + if owned_folders == []: + raise MoveError("nothing to move") + + check_folders(current_volume_name, owned_folders) + + + def do_move_to_volume( + self, + new_volume: BlockDevice, + job: Job, + ): + """ + Move a service to another volume. + Is not allowed to raise errors because it is a task. + """ + service_name = self.get_display_name() + old_volume_name = self.get_drive() + owned_folders = self.get_owned_folders() + + report_progress(0, job, "Performing pre-move checks...") + + # TODO: move trying to the task + try: + report_progress(5, job, f"Stopping {service_name}...") + + with StoppedService(self): + report_progress(10, job, "Unmounting folders from old volume...") + unbind_folders(owned_folders) + + report_progress(20, job, "Moving data to new volume...") + move_folders_to_volume(owned_folders, old_volume_name, new_volume, job) + + report_progress(70, job, f"Making sure {service_name} owns its files...") + try: + ensure_folder_ownership(owned_folders, new_volume, job, self) + except Exception as error: + # We have logged it via print and we additionally log it here in the error field + # We are continuing anyway but Job has no warning field + Jobs.update(job, JobStatus.RUNNING, error=f"Service {service_name} will not be able to write files: " + str(error)) + + report_progress(90, job, f"Mounting {service_name} data...") + bind_folders(owned_folders, new_volume) + + report_progress(95, job, f"Finishing moving {service_name}...") + self.set_location(self, new_volume) + + Jobs.update( + job=job, + status=JobStatus.FINISHED, + result=f"{service_name} moved successfully.", + status_text=f"Starting {service_name}...", + progress=100, + ) + except Exception as e: + Jobs.update( + job=job, + status=JobStatus.ERROR, + error=type(e).__name__ + " " + str(e), + ) + + @abstractmethod def move_to_volume(self, volume: BlockDevice) -> Job: """Cannot raise errors. diff --git a/selfprivacy_api/services/tasks.py b/selfprivacy_api/services/tasks.py new file mode 100644 index 0000000..2cc52ad --- /dev/null +++ b/selfprivacy_api/services/tasks.py @@ -0,0 +1,11 @@ +from selfprivacy_api.services import Service +from selfprivacy_api.utils.block_devices import BlockDevice +from selfprivacy_api.utils.huey import huey + + +@huey.task() +def move_service( + service: Service, + new_volume: BlockDevice, +): + service.move_to_volume(new_volume) From 17a1e34c0dbc4f9fd94ea0749674ba718350269f Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Sun, 18 Feb 2024 23:58:00 +0000 Subject: [PATCH 26/35] feature(services): check before moving task and before move itself --- selfprivacy_api/actions/services.py | 36 ++++++ .../graphql/mutations/services_mutations.py | 51 +++++---- .../services/bitwarden/__init__.py | 23 +--- selfprivacy_api/services/gitea/__init__.py | 23 +--- .../services/mailserver/__init__.py | 20 ---- selfprivacy_api/services/moving.py | 30 +++-- .../services/nextcloud/__init__.py | 22 +--- selfprivacy_api/services/pleroma/__init__.py | 27 +---- selfprivacy_api/services/service.py | 104 ++++++++++-------- selfprivacy_api/services/tasks.py | 21 +++- .../services/test_service/__init__.py | 22 +--- tests/test_graphql/test_services.py | 17 ++- tests/test_services.py | 25 ++--- 13 files changed, 192 insertions(+), 229 deletions(-) create mode 100644 selfprivacy_api/actions/services.py diff --git a/selfprivacy_api/actions/services.py b/selfprivacy_api/actions/services.py new file mode 100644 index 0000000..56d35e9 --- /dev/null +++ b/selfprivacy_api/actions/services.py @@ -0,0 +1,36 @@ +from selfprivacy_api.utils.block_devices import BlockDevices +from selfprivacy_api.jobs import Jobs, Job + +from selfprivacy_api.services import get_service_by_id +from selfprivacy_api.services.tasks import move_service as move_service_task + + +class ServiceNotFoundError(Exception): + pass + + +class VolumeNotFoundError(Exception): + pass + + +def move_service(service_id: str, volume_name: str) -> Job: + service = get_service_by_id(service_id) + if service is None: + raise ServiceNotFoundError(f"No such service:{service_id}") + + volume = BlockDevices().get_block_device(volume_name) + if volume is None: + raise VolumeNotFoundError(f"No such volume:{volume_name}") + + service.assert_can_move(volume) + + job = Jobs.add( + type_id=f"services.{service.get_id()}.move", + name=f"Move {service.get_display_name()}", + description=f"Moving {service.get_display_name()} data to {volume.name}", + ) + + handle = move_service_task(service, volume, job) + # Nonblocking + handle() + return job diff --git a/selfprivacy_api/graphql/mutations/services_mutations.py b/selfprivacy_api/graphql/mutations/services_mutations.py index e8edbcf..911ad26 100644 --- a/selfprivacy_api/graphql/mutations/services_mutations.py +++ b/selfprivacy_api/graphql/mutations/services_mutations.py @@ -5,18 +5,25 @@ import strawberry from selfprivacy_api.graphql import IsAuthenticated from selfprivacy_api.graphql.common_types.jobs import job_to_api_job from selfprivacy_api.jobs import JobStatus +from selfprivacy_api.utils.block_devices import BlockDevices -from selfprivacy_api.graphql.common_types.service import ( - Service, - service_to_graphql_service, -) from selfprivacy_api.graphql.mutations.mutation_interface import ( GenericJobMutationReturn, GenericMutationReturn, ) +from selfprivacy_api.graphql.common_types.service import ( + Service, + service_to_graphql_service, +) + +from selfprivacy_api.actions.services import ( + move_service, + ServiceNotFoundError, + VolumeNotFoundError, +) +from selfprivacy_api.services.moving import MoveError from selfprivacy_api.services import get_service_by_id -from selfprivacy_api.utils.block_devices import BlockDevices @strawberry.type @@ -60,7 +67,7 @@ class ServicesMutations: except Exception as e: return ServiceMutationReturn( success=False, - message=format_error(e), + message=pretty_error(e), code=400, ) @@ -86,7 +93,7 @@ class ServicesMutations: except Exception as e: return ServiceMutationReturn( success=False, - message=format_error(e), + message=pretty_error(e), code=400, ) return ServiceMutationReturn( @@ -153,31 +160,31 @@ class ServicesMutations: @strawberry.mutation(permission_classes=[IsAuthenticated]) def move_service(self, input: MoveServiceInput) -> ServiceJobMutationReturn: """Move service.""" + # We need a service instance for a reply later service = get_service_by_id(input.service_id) if service is None: return ServiceJobMutationReturn( success=False, - message=f"Service not found: {input.service_id}", + message=f"Service does not exist: {input.service_id}", code=404, ) - # TODO: make serviceImmovable and BlockdeviceNotFound exceptions - # in the move_to_volume() function and handle them here - if not service.is_movable(): + + try: + job = move_service(input.service_id, input.location) + except (ServiceNotFoundError, VolumeNotFoundError) as e: return ServiceJobMutationReturn( success=False, - message=f"Service is not movable: {service.get_display_name()}", + message=pretty_error(e), + code=404, + ) + except Exception as e: + return ServiceJobMutationReturn( + success=False, + message=pretty_error(e), code=400, service=service_to_graphql_service(service), ) - volume = BlockDevices().get_block_device(input.location) - if volume is None: - return ServiceJobMutationReturn( - success=False, - message=f"Volume not found: {input.location}", - code=404, - service=service_to_graphql_service(service), - ) - job = service.move_to_volume(volume) + if job.status in [JobStatus.CREATED, JobStatus.RUNNING]: return ServiceJobMutationReturn( success=True, @@ -204,5 +211,5 @@ class ServicesMutations: ) -def format_error(e: Exception) -> str: +def pretty_error(e: Exception) -> str: return type(e).__name__ + ": " + str(e) diff --git a/selfprivacy_api/services/bitwarden/__init__.py b/selfprivacy_api/services/bitwarden/__init__.py index d1910ea..52f1466 100644 --- a/selfprivacy_api/services/bitwarden/__init__.py +++ b/selfprivacy_api/services/bitwarden/__init__.py @@ -3,12 +3,10 @@ import base64 import subprocess from typing import Optional, List -from selfprivacy_api.jobs import Job, Jobs -from selfprivacy_api.services.generic_service_mover import FolderMoveNames, move_service +from selfprivacy_api.utils import get_domain + from selfprivacy_api.utils.systemd import get_service_status from selfprivacy_api.services.service import Service, ServiceStatus -from selfprivacy_api.utils import get_domain -from selfprivacy_api.utils.block_devices import BlockDevice from selfprivacy_api.services.bitwarden.icon import BITWARDEN_ICON @@ -101,20 +99,3 @@ class Bitwarden(Service): @staticmethod def get_folders() -> List[str]: return ["/var/lib/bitwarden", "/var/lib/bitwarden_rs"] - - def move_to_volume(self, volume: BlockDevice) -> Job: - job = Jobs.add( - type_id="services.bitwarden.move", - name="Move Bitwarden", - description=f"Moving Bitwarden data to {volume.name}", - ) - - move_service( - self, - volume, - job, - FolderMoveNames.default_foldermoves(self), - "bitwarden", - ) - - return job diff --git a/selfprivacy_api/services/gitea/__init__.py b/selfprivacy_api/services/gitea/__init__.py index 31aa743..311d59e 100644 --- a/selfprivacy_api/services/gitea/__init__.py +++ b/selfprivacy_api/services/gitea/__init__.py @@ -3,12 +3,10 @@ import base64 import subprocess from typing import Optional, List -from selfprivacy_api.jobs import Job, Jobs -from selfprivacy_api.services.generic_service_mover import FolderMoveNames, move_service +from selfprivacy_api.utils import get_domain + from selfprivacy_api.utils.systemd import get_service_status from selfprivacy_api.services.service import Service, ServiceStatus -from selfprivacy_api.utils import get_domain -from selfprivacy_api.utils.block_devices import BlockDevice from selfprivacy_api.services.gitea.icon import GITEA_ICON @@ -96,20 +94,3 @@ class Gitea(Service): @staticmethod def get_folders() -> List[str]: return ["/var/lib/gitea"] - - def move_to_volume(self, volume: BlockDevice) -> Job: - job = Jobs.add( - type_id="services.gitea.move", - name="Move Gitea", - description=f"Moving Gitea data to {volume.name}", - ) - - move_service( - self, - volume, - job, - FolderMoveNames.default_foldermoves(self), - "gitea", - ) - - return job diff --git a/selfprivacy_api/services/mailserver/__init__.py b/selfprivacy_api/services/mailserver/__init__.py index 2f2f7f7..d2e9b5d 100644 --- a/selfprivacy_api/services/mailserver/__init__.py +++ b/selfprivacy_api/services/mailserver/__init__.py @@ -4,14 +4,11 @@ import base64 import subprocess from typing import Optional, List -from selfprivacy_api.jobs import Job, Jobs -from selfprivacy_api.services.generic_service_mover import FolderMoveNames, move_service from selfprivacy_api.utils.systemd import ( get_service_status_from_several_units, ) from selfprivacy_api.services.service import Service, ServiceDnsRecord, ServiceStatus from selfprivacy_api import utils -from selfprivacy_api.utils.block_devices import BlockDevice from selfprivacy_api.services.mailserver.icon import MAILSERVER_ICON @@ -166,20 +163,3 @@ class MailServer(Service): ), ) return dns_records - - def move_to_volume(self, volume: BlockDevice) -> Job: - job = Jobs.add( - type_id="services.email.move", - name="Move Mail Server", - description=f"Moving mailserver data to {volume.name}", - ) - - move_service( - self, - volume, - job, - FolderMoveNames.default_foldermoves(self), - "simple-nixos-mailserver", - ) - - return job diff --git a/selfprivacy_api/services/moving.py b/selfprivacy_api/services/moving.py index d667935..ecc505b 100644 --- a/selfprivacy_api/services/moving.py +++ b/selfprivacy_api/services/moving.py @@ -14,10 +14,9 @@ from selfprivacy_api.services.owned_path import OwnedPath class MoveError(Exception): """Move failed""" -def get_foldername(path: str) -> str: - return path.split("/")[-1] - +def get_foldername(p: OwnedPath) -> str: + return p.path.split("/")[-1] def check_volume(volume: BlockDevice, space_needed: int) -> bool: @@ -26,10 +25,7 @@ def check_volume(volume: BlockDevice, space_needed: int) -> bool: raise MoveError("Not enough space on the new volume.") # Make sure the volume is mounted - if ( - not volume.is_root() - and f"/volumes/{volume.name}" not in volume.mountpoints - ): + if not volume.is_root() and f"/volumes/{volume.name}" not in volume.mountpoints: raise MoveError("Volume is not mounted.") @@ -39,11 +35,11 @@ def check_folders(current_volume: BlockDevice, folders: List[OwnedPath]) -> None path = pathlib.Path(f"/volumes/{current_volume}/{get_foldername(folder)}") if not path.exists(): - raise MoveError(f"{path} is not found.") + raise MoveError(f"directory {path} is not found.") if not path.is_dir(): raise MoveError(f"{path} is not a directory.") if path.owner() != folder.owner: - raise MoveError(f"{path} owner is not {folder.owner}.") + raise MoveError(f"{path} is not owned by {folder.owner}.") def unbind_folders(owned_folders: List[OwnedPath]) -> None: @@ -66,7 +62,7 @@ def move_folders_to_volume( current_progress = job.progress folder_percentage = 50 // len(folders) for folder in folders: - folder_name = get_foldername(folder.path) + folder_name = get_foldername(folder) shutil.move( f"/volumes/{old_volume}/{folder_name}", f"/volumes/{new_volume.name}/{folder_name}", @@ -75,11 +71,9 @@ def move_folders_to_volume( report_progress(progress, job, "Moving data to new volume...") -def ensure_folder_ownership( - folders: List[OwnedPath], volume: BlockDevice -) -> None: +def ensure_folder_ownership(folders: List[OwnedPath], volume: BlockDevice) -> None: for folder in folders: - true_location = f"/volumes/{volume.name}/{get_foldername(folder.path)}" + true_location = f"/volumes/{volume.name}/{get_foldername(folder)}" try: subprocess.run( [ @@ -87,12 +81,14 @@ def ensure_folder_ownership( "-R", f"{folder.owner}:{folder.group}", # Could we just chown the binded location instead? - true_location + true_location, ], check=True, ) except subprocess.CalledProcessError as error: - error_message = f"Unable to set ownership of {true_location} :{error.output}" + error_message = ( + f"Unable to set ownership of {true_location} :{error.output}" + ) print(error.output) raise MoveError(error_message) @@ -104,7 +100,7 @@ def bind_folders(folders: List[OwnedPath], volume: BlockDevice) -> None: [ "mount", "--bind", - f"/volumes/{volume.name}/{get_foldername(folder.path)}", + f"/volumes/{volume.name}/{get_foldername(folder)}", folder.path, ], check=True, diff --git a/selfprivacy_api/services/nextcloud/__init__.py b/selfprivacy_api/services/nextcloud/__init__.py index 87c47af..3e5b8d3 100644 --- a/selfprivacy_api/services/nextcloud/__init__.py +++ b/selfprivacy_api/services/nextcloud/__init__.py @@ -2,12 +2,13 @@ import base64 import subprocess from typing import Optional, List + +from selfprivacy_api.utils import get_domain from selfprivacy_api.jobs import Job, Jobs -from selfprivacy_api.services.generic_service_mover import FolderMoveNames, move_service + from selfprivacy_api.utils.systemd import get_service_status from selfprivacy_api.services.service import Service, ServiceStatus -from selfprivacy_api.utils import get_domain -from selfprivacy_api.utils.block_devices import BlockDevice + from selfprivacy_api.services.nextcloud.icon import NEXTCLOUD_ICON @@ -101,18 +102,3 @@ class Nextcloud(Service): @staticmethod def get_folders() -> List[str]: return ["/var/lib/nextcloud"] - - def move_to_volume(self, volume: BlockDevice) -> Job: - job = Jobs.add( - type_id="services.nextcloud.move", - name="Move Nextcloud", - description=f"Moving Nextcloud to volume {volume.name}", - ) - move_service( - self, - volume, - job, - FolderMoveNames.default_foldermoves(self), - "nextcloud", - ) - return job diff --git a/selfprivacy_api/services/pleroma/__init__.py b/selfprivacy_api/services/pleroma/__init__.py index 07e6996..44a9be8 100644 --- a/selfprivacy_api/services/pleroma/__init__.py +++ b/selfprivacy_api/services/pleroma/__init__.py @@ -2,13 +2,13 @@ import base64 import subprocess from typing import Optional, List -from selfprivacy_api.jobs import Job, Jobs -from selfprivacy_api.services.generic_service_mover import FolderMoveNames, move_service + +from selfprivacy_api.utils import get_domain + +from selfprivacy_api.services.owned_path import OwnedPath from selfprivacy_api.utils.systemd import get_service_status from selfprivacy_api.services.service import Service, ServiceStatus -from selfprivacy_api.services.owned_path import OwnedPath -from selfprivacy_api.utils import get_domain -from selfprivacy_api.utils.block_devices import BlockDevice + from selfprivacy_api.services.pleroma.icon import PLEROMA_ICON @@ -88,7 +88,7 @@ class Pleroma(Service): def get_owned_folders() -> List[OwnedPath]: """ Get a list of occupied directories with ownership info - pleroma has folders that are owned by different users + Pleroma has folders that are owned by different users """ return [ OwnedPath( @@ -102,18 +102,3 @@ class Pleroma(Service): group="postgres", ), ] - - def move_to_volume(self, volume: BlockDevice) -> Job: - job = Jobs.add( - type_id="services.pleroma.move", - name="Move Pleroma", - description=f"Moving Pleroma to volume {volume.name}", - ) - move_service( - self, - volume, - job, - FolderMoveNames.default_foldermoves(self), - "pleroma", - ) - return job diff --git a/selfprivacy_api/services/service.py b/selfprivacy_api/services/service.py index 6255f20..224fde6 100644 --- a/selfprivacy_api/services/service.py +++ b/selfprivacy_api/services/service.py @@ -10,7 +10,15 @@ from selfprivacy_api.utils.block_devices import BlockDevice, BlockDevices from selfprivacy_api.services.generic_size_counter import get_storage_usage from selfprivacy_api.services.owned_path import OwnedPath -from selfprivacy_api.services.moving import check_folders, check_volume, unbind_folders, bind_folders, ensure_folder_ownership, MoveError, move_folders_to_volume +from selfprivacy_api.services.moving import ( + check_folders, + check_volume, + unbind_folders, + bind_folders, + ensure_folder_ownership, + MoveError, + move_folders_to_volume, +) from selfprivacy_api import utils from selfprivacy_api.utils.waitloop import wait_until_true @@ -300,7 +308,7 @@ class Service(ABC): @classmethod def set_location(cls, volume: BlockDevice): """ - Only changes userdata + Only changes userdata """ with WriteUserData() as user_data: @@ -313,15 +321,18 @@ class Service(ABC): def assert_can_move(self, new_volume): """ - Checks if the service can be moved to new volume - Raises errors if it cannot + Checks if the service can be moved to new volume + Raises errors if it cannot """ + service_name = self.get_display_name() + if not self.is_movable(): + raise MoveError(f"{service_name} is not movable") + with ReadUserData() as user_data: if not user_data.get("useBinds", False): raise MoveError("Server is not using binds.") current_volume_name = self.get_drive() - service_name = self.get_display_name() if current_volume_name == new_volume.name: raise MoveError(f"{service_name} is already on volume {new_volume}") @@ -333,7 +344,6 @@ class Service(ABC): check_folders(current_volume_name, owned_folders) - def do_move_to_volume( self, new_volume: BlockDevice, @@ -341,59 +351,57 @@ class Service(ABC): ): """ Move a service to another volume. - Is not allowed to raise errors because it is a task. """ service_name = self.get_display_name() old_volume_name = self.get_drive() owned_folders = self.get_owned_folders() - report_progress(0, job, "Performing pre-move checks...") + report_progress(10, job, "Unmounting folders from old volume...") + unbind_folders(owned_folders) - # TODO: move trying to the task + report_progress(20, job, "Moving data to new volume...") + move_folders_to_volume(owned_folders, old_volume_name, new_volume, job) + + report_progress(70, job, f"Making sure {service_name} owns its files...") try: - report_progress(5, job, f"Stopping {service_name}...") - - with StoppedService(self): - report_progress(10, job, "Unmounting folders from old volume...") - unbind_folders(owned_folders) - - report_progress(20, job, "Moving data to new volume...") - move_folders_to_volume(owned_folders, old_volume_name, new_volume, job) - - report_progress(70, job, f"Making sure {service_name} owns its files...") - try: - ensure_folder_ownership(owned_folders, new_volume, job, self) - except Exception as error: - # We have logged it via print and we additionally log it here in the error field - # We are continuing anyway but Job has no warning field - Jobs.update(job, JobStatus.RUNNING, error=f"Service {service_name} will not be able to write files: " + str(error)) - - report_progress(90, job, f"Mounting {service_name} data...") - bind_folders(owned_folders, new_volume) - - report_progress(95, job, f"Finishing moving {service_name}...") - self.set_location(self, new_volume) - - Jobs.update( - job=job, - status=JobStatus.FINISHED, - result=f"{service_name} moved successfully.", - status_text=f"Starting {service_name}...", - progress=100, - ) - except Exception as e: + ensure_folder_ownership(owned_folders, new_volume, job, self) + except Exception as error: + # We have logged it via print and we additionally log it here in the error field + # We are continuing anyway but Job has no warning field Jobs.update( - job=job, - status=JobStatus.ERROR, - error=type(e).__name__ + " " + str(e), + job, + JobStatus.RUNNING, + error=f"Service {service_name} will not be able to write files: " + + str(error), ) + report_progress(90, job, f"Mounting {service_name} data...") + bind_folders(owned_folders, new_volume) - @abstractmethod - def move_to_volume(self, volume: BlockDevice) -> Job: - """Cannot raise errors. - Returns errors as an errored out Job instead.""" - pass + report_progress(95, job, f"Finishing moving {service_name}...") + self.set_location(new_volume) + + Jobs.update( + job=job, + status=JobStatus.FINISHED, + result=f"{service_name} moved successfully.", + status_text=f"Starting {service_name}...", + progress=100, + ) + + def move_to_volume(self, volume: BlockDevice, job: Job) -> Job: + service_name = self.get_display_name() + + report_progress(0, job, "Performing pre-move checks...") + self.assert_can_move(volume) + + report_progress(5, job, f"Stopping {service_name}...") + assert self is not None + with StoppedService(self): + report_progress(9, job, f"Stopped server, starting the move...") + self.do_move_to_volume(volume, job) + + return job @classmethod def owned_path(cls, path: str): diff --git a/selfprivacy_api/services/tasks.py b/selfprivacy_api/services/tasks.py index 2cc52ad..ec44e37 100644 --- a/selfprivacy_api/services/tasks.py +++ b/selfprivacy_api/services/tasks.py @@ -1,11 +1,22 @@ from selfprivacy_api.services import Service from selfprivacy_api.utils.block_devices import BlockDevice from selfprivacy_api.utils.huey import huey +from selfprivacy_api.jobs import Job, Jobs, JobStatus @huey.task() -def move_service( - service: Service, - new_volume: BlockDevice, -): - service.move_to_volume(new_volume) +def move_service(service: Service, new_volume: BlockDevice, job: Job) -> bool: + """ + Move service's folders to new physical volume + Does not raise exceptions (we cannot handle exceptions from tasks). + Reports all errors via job. + """ + try: + service.move_to_volume(new_volume, job) + except Exception as e: + Jobs.update( + job=job, + status=JobStatus.ERROR, + error=type(e).__name__ + " " + str(e), + ) + return True diff --git a/selfprivacy_api/services/test_service/__init__.py b/selfprivacy_api/services/test_service/__init__.py index 803896b..f869bb3 100644 --- a/selfprivacy_api/services/test_service/__init__.py +++ b/selfprivacy_api/services/test_service/__init__.py @@ -11,7 +11,6 @@ from os import path from selfprivacy_api.jobs import Job, Jobs, JobStatus from selfprivacy_api.services.service import Service, ServiceDnsRecord, ServiceStatus from selfprivacy_api.utils.block_devices import BlockDevice -from selfprivacy_api.services.generic_service_mover import move_service, FolderMoveNames import selfprivacy_api.utils.network as network_utils from selfprivacy_api.services.test_service.icon import BITWARDEN_ICON @@ -189,23 +188,10 @@ class DummyService(Service): def get_folders(cls) -> List[str]: return cls.folders - def move_to_volume(self, volume: BlockDevice) -> Job: - job = Jobs.add( - type_id=f"services.{self.get_id()}.move", - name=f"Move {self.get_display_name()}", - description=f"Moving {self.get_display_name()} data to {volume.name}", - ) + def do_move_to_volume(self, volume: BlockDevice, job: Job) -> Job: if self.simulate_moving is False: - # completely generic code, TODO: make it the default impl. - move_service( - self, - volume, - job, - FolderMoveNames.default_foldermoves(self), - self.get_id(), - ) + return super(DummyService, self).do_move_to_volume(volume, job) else: Jobs.update(job, status=JobStatus.FINISHED) - - self.set_drive(volume.name) - return job + self.set_drive(volume.name) + return job diff --git a/tests/test_graphql/test_services.py b/tests/test_graphql/test_services.py index c6784ee..9208371 100644 --- a/tests/test_graphql/test_services.py +++ b/tests/test_graphql/test_services.py @@ -8,6 +8,8 @@ from selfprivacy_api.services import get_service_by_id from selfprivacy_api.services.service import Service, ServiceStatus from selfprivacy_api.services.test_service import DummyService +# from selfprivacy_api.services.moving import check_folders + from tests.common import generate_service_query from tests.test_graphql.common import assert_empty, assert_ok, get_data from tests.test_block_device_utils import lsblk_singular_mock @@ -32,7 +34,7 @@ MOVER_MOCK_PROCESS = CompletedProcess(["ls"], returncode=0) @pytest.fixture() def mock_check_service_mover_folders(mocker): mock = mocker.patch( - "selfprivacy_api.services.generic_service_mover.check_folders", + "selfprivacy_api.services.service.check_folders", autospec=True, return_value=None, ) @@ -495,9 +497,14 @@ def test_disable_enable(authorized_client, only_dummy_service): def test_move_immovable(authorized_client, only_dummy_service): dummy_service = only_dummy_service dummy_service.set_movable(False) - mutation_response = api_move(authorized_client, dummy_service, "sda1") + root = BlockDevices().get_root_block_device() + mutation_response = api_move(authorized_client, dummy_service, root.name) data = get_data(mutation_response)["services"]["moveService"] assert_errorcode(data, 400) + try: + assert "not movable" in data["message"] + except AssertionError: + raise ValueError("wrong type of error?: ", data["message"]) # is there a meaning in returning the service in this? assert data["service"] is not None @@ -519,8 +526,7 @@ def test_move_no_such_volume(authorized_client, only_dummy_service): data = get_data(mutation_response)["services"]["moveService"] assert_notfound(data) - # is there a meaning in returning the service in this? - assert data["service"] is not None + assert data["service"] is None assert data["job"] is None @@ -538,7 +544,8 @@ def test_move_same_volume(authorized_client, dummy_service): # is there a meaning in returning the service in this? assert data["service"] is not None - assert data["job"] is not None + # We do not create a job if task is not created + assert data["job"] is None def test_graphql_move_service_without_folders_on_old_volume( diff --git a/tests/test_services.py b/tests/test_services.py index de3665a..98cfa4e 100644 --- a/tests/test_services.py +++ b/tests/test_services.py @@ -13,7 +13,6 @@ from selfprivacy_api.services.bitwarden import Bitwarden from selfprivacy_api.services.pleroma import Pleroma from selfprivacy_api.services.mailserver import MailServer from selfprivacy_api.services.owned_path import OwnedPath -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 @@ -81,19 +80,19 @@ def test_paths_from_owned_paths(): ] -def test_foldermoves_from_ownedpaths(): - owned = OwnedPath( - path="var/lib/bitwarden", - group="vaultwarden", - owner="vaultwarden", - ) +# def test_foldermoves_from_ownedpaths(): +# owned = OwnedPath( +# path="var/lib/bitwarden", +# group="vaultwarden", +# owner="vaultwarden", +# ) - assert FolderMoveNames.from_owned_path(owned) == FolderMoveNames( - name="bitwarden", - bind_location="var/lib/bitwarden", - group="vaultwarden", - owner="vaultwarden", - ) +# assert FolderMoveNames.from_owned_path(owned) == FolderMoveNames( +# name="bitwarden", +# bind_location="var/lib/bitwarden", +# group="vaultwarden", +# owner="vaultwarden", +# ) def test_enabling_disabling_reads_json(dummy_service: DummyService): From c22802f69354c5f625c56a21fa2b84c9eef01f57 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 19 Feb 2024 00:10:13 +0000 Subject: [PATCH 27/35] fix(services): check for possible None progress when moving folders --- selfprivacy_api/services/moving.py | 3 +++ selfprivacy_api/services/service.py | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/selfprivacy_api/services/moving.py b/selfprivacy_api/services/moving.py index ecc505b..ce13b30 100644 --- a/selfprivacy_api/services/moving.py +++ b/selfprivacy_api/services/moving.py @@ -60,6 +60,9 @@ def move_folders_to_volume( job: Job, ) -> None: current_progress = job.progress + if current_progress is None: + current_progress = 0 + folder_percentage = 50 // len(folders) for folder in folders: folder_name = get_foldername(folder) diff --git a/selfprivacy_api/services/service.py b/selfprivacy_api/services/service.py index 224fde6..233e6e5 100644 --- a/selfprivacy_api/services/service.py +++ b/selfprivacy_api/services/service.py @@ -311,8 +311,8 @@ class Service(ABC): Only changes userdata """ + service_id = cls.get_id() with WriteUserData() as user_data: - service_id = cls.get_id() if "modules" not in user_data: user_data["modules"] = {} if service_id not in user_data["modules"]: From ddca1b0cdef0db00886e66249b031acd6017e713 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 19 Feb 2024 00:24:32 +0000 Subject: [PATCH 28/35] refactor(services): fix type annotation --- selfprivacy_api/services/moving.py | 6 +++--- selfprivacy_api/services/service.py | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/selfprivacy_api/services/moving.py b/selfprivacy_api/services/moving.py index ce13b30..e311c4f 100644 --- a/selfprivacy_api/services/moving.py +++ b/selfprivacy_api/services/moving.py @@ -19,7 +19,7 @@ def get_foldername(p: OwnedPath) -> str: return p.path.split("/")[-1] -def check_volume(volume: BlockDevice, space_needed: int) -> bool: +def check_volume(volume: BlockDevice, space_needed: int) -> None: # Check if there is enough space on the new volume if int(volume.fsavail) < space_needed: raise MoveError("Not enough space on the new volume.") @@ -55,7 +55,7 @@ def unbind_folders(owned_folders: List[OwnedPath]) -> None: def move_folders_to_volume( folders: List[OwnedPath], - old_volume: BlockDevice, + old_volume_name: str, # TODO: pass an actual validated block device new_volume: BlockDevice, job: Job, ) -> None: @@ -67,7 +67,7 @@ def move_folders_to_volume( for folder in folders: folder_name = get_foldername(folder) shutil.move( - f"/volumes/{old_volume}/{folder_name}", + f"/volumes/{old_volume_name}/{folder_name}", f"/volumes/{new_volume.name}/{folder_name}", ) progress = current_progress + folder_percentage diff --git a/selfprivacy_api/services/service.py b/selfprivacy_api/services/service.py index 233e6e5..da3f5ca 100644 --- a/selfprivacy_api/services/service.py +++ b/selfprivacy_api/services/service.py @@ -353,6 +353,7 @@ class Service(ABC): Move a service to another volume. """ service_name = self.get_display_name() + # TODO: validate that this volume exists old_volume_name = self.get_drive() owned_folders = self.get_owned_folders() From 235c59b5563b89f6f0d7b6c0d54144a6259f4348 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 19 Feb 2024 18:37:00 +0000 Subject: [PATCH 29/35] refactor(services): break out location construction when moving --- selfprivacy_api/services/moving.py | 25 ++++++++++++++----------- selfprivacy_api/services/service.py | 6 +++--- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/selfprivacy_api/services/moving.py b/selfprivacy_api/services/moving.py index e311c4f..ba9b2c9 100644 --- a/selfprivacy_api/services/moving.py +++ b/selfprivacy_api/services/moving.py @@ -19,6 +19,10 @@ def get_foldername(p: OwnedPath) -> str: return p.path.split("/")[-1] +def location_at_volume(binding_path: OwnedPath, volume_name: str): + return f"/volumes/{volume_name}/{get_foldername(binding_path)}" + + def check_volume(volume: BlockDevice, space_needed: int) -> None: # Check if there is enough space on the new volume if int(volume.fsavail) < space_needed: @@ -29,10 +33,10 @@ def check_volume(volume: BlockDevice, space_needed: int) -> None: raise MoveError("Volume is not mounted.") -def check_folders(current_volume: BlockDevice, folders: List[OwnedPath]) -> None: +def check_folders(volume_name: str, folders: List[OwnedPath]) -> None: # Make sure current actual directory exists and if its user and group are correct for folder in folders: - path = pathlib.Path(f"/volumes/{current_volume}/{get_foldername(folder)}") + path = pathlib.Path(location_at_volume(folder, volume_name)) if not path.exists(): raise MoveError(f"directory {path} is not found.") @@ -55,7 +59,7 @@ def unbind_folders(owned_folders: List[OwnedPath]) -> None: def move_folders_to_volume( folders: List[OwnedPath], - old_volume_name: str, # TODO: pass an actual validated block device + old_volume_name: str, # TODO: pass an actual validated block device new_volume: BlockDevice, job: Job, ) -> None: @@ -63,20 +67,19 @@ def move_folders_to_volume( if current_progress is None: current_progress = 0 - folder_percentage = 50 // len(folders) + progress_per_folder = 50 // len(folders) for folder in folders: - folder_name = get_foldername(folder) shutil.move( - f"/volumes/{old_volume_name}/{folder_name}", - f"/volumes/{new_volume.name}/{folder_name}", + location_at_volume(folder, old_volume_name), + location_at_volume(folder, new_volume.name), ) - progress = current_progress + folder_percentage + progress = current_progress + progress_per_folder report_progress(progress, job, "Moving data to new volume...") def ensure_folder_ownership(folders: List[OwnedPath], volume: BlockDevice) -> None: for folder in folders: - true_location = f"/volumes/{volume.name}/{get_foldername(folder)}" + true_location = location_at_volume(folder, volume.name) try: subprocess.run( [ @@ -89,10 +92,10 @@ def ensure_folder_ownership(folders: List[OwnedPath], volume: BlockDevice) -> No check=True, ) except subprocess.CalledProcessError as error: + print(error.output) error_message = ( f"Unable to set ownership of {true_location} :{error.output}" ) - print(error.output) raise MoveError(error_message) @@ -103,7 +106,7 @@ def bind_folders(folders: List[OwnedPath], volume: BlockDevice) -> None: [ "mount", "--bind", - f"/volumes/{volume.name}/{get_foldername(folder)}", + location_at_volume(folder, volume.name), folder.path, ], check=True, diff --git a/selfprivacy_api/services/service.py b/selfprivacy_api/services/service.py index da3f5ca..e60cf8a 100644 --- a/selfprivacy_api/services/service.py +++ b/selfprivacy_api/services/service.py @@ -353,7 +353,7 @@ class Service(ABC): Move a service to another volume. """ service_name = self.get_display_name() - # TODO: validate that this volume exists + # TODO : Make sure device exists old_volume_name = self.get_drive() owned_folders = self.get_owned_folders() @@ -365,7 +365,7 @@ class Service(ABC): report_progress(70, job, f"Making sure {service_name} owns its files...") try: - ensure_folder_ownership(owned_folders, new_volume, job, self) + ensure_folder_ownership(owned_folders, new_volume) except Exception as error: # We have logged it via print and we additionally log it here in the error field # We are continuing anyway but Job has no warning field @@ -399,7 +399,7 @@ class Service(ABC): report_progress(5, job, f"Stopping {service_name}...") assert self is not None with StoppedService(self): - report_progress(9, job, f"Stopped server, starting the move...") + report_progress(9, job, "Stopped service, starting the move...") self.do_move_to_volume(volume, job) return job From 1e51f51844059a438a6cf3f51598ff84721ec322 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 26 Feb 2024 15:01:07 +0000 Subject: [PATCH 30/35] feature(backups): intermittent commit for binds, to be replaced --- .../graphql/mutations/services_mutations.py | 6 +- selfprivacy_api/services/moving.py | 102 +++++------------- selfprivacy_api/services/owned_path.py | 96 +++++++++++++++++ selfprivacy_api/services/service.py | 33 +++--- selfprivacy_api/utils/block_devices.py | 2 + tests/test_graphql/test_services.py | 6 +- 6 files changed, 154 insertions(+), 91 deletions(-) diff --git a/selfprivacy_api/graphql/mutations/services_mutations.py b/selfprivacy_api/graphql/mutations/services_mutations.py index 911ad26..97eb4d9 100644 --- a/selfprivacy_api/graphql/mutations/services_mutations.py +++ b/selfprivacy_api/graphql/mutations/services_mutations.py @@ -7,6 +7,8 @@ from selfprivacy_api.graphql.common_types.jobs import job_to_api_job from selfprivacy_api.jobs import JobStatus from selfprivacy_api.utils.block_devices import BlockDevices +from traceback import format_tb as format_traceback + from selfprivacy_api.graphql.mutations.mutation_interface import ( GenericJobMutationReturn, GenericMutationReturn, @@ -171,6 +173,7 @@ class ServicesMutations: try: job = move_service(input.service_id, input.location) + except (ServiceNotFoundError, VolumeNotFoundError) as e: return ServiceJobMutationReturn( success=False, @@ -212,4 +215,5 @@ class ServicesMutations: def pretty_error(e: Exception) -> str: - return type(e).__name__ + ": " + str(e) + traceback = "/r".join(format_traceback(e.__traceback__)) + return type(e).__name__ + ": " + str(e) + ": " + traceback diff --git a/selfprivacy_api/services/moving.py b/selfprivacy_api/services/moving.py index ba9b2c9..09af765 100644 --- a/selfprivacy_api/services/moving.py +++ b/selfprivacy_api/services/moving.py @@ -1,26 +1,16 @@ """Generic handler for moving services""" from __future__ import annotations -import subprocess -import pathlib import shutil from typing import List from selfprivacy_api.jobs import Job, report_progress from selfprivacy_api.utils.block_devices import BlockDevice -from selfprivacy_api.services.owned_path import OwnedPath +from selfprivacy_api.services.owned_path import Bind class MoveError(Exception): - """Move failed""" - - -def get_foldername(p: OwnedPath) -> str: - return p.path.split("/")[-1] - - -def location_at_volume(binding_path: OwnedPath, volume_name: str): - return f"/volumes/{volume_name}/{get_foldername(binding_path)}" + """Move of the data has failed""" def check_volume(volume: BlockDevice, space_needed: int) -> None: @@ -33,84 +23,50 @@ def check_volume(volume: BlockDevice, space_needed: int) -> None: raise MoveError("Volume is not mounted.") -def check_folders(volume_name: str, folders: List[OwnedPath]) -> None: +def check_binds(volume_name: str, binds: List[Bind]) -> None: # Make sure current actual directory exists and if its user and group are correct - for folder in folders: - path = pathlib.Path(location_at_volume(folder, volume_name)) - - if not path.exists(): - raise MoveError(f"directory {path} is not found.") - if not path.is_dir(): - raise MoveError(f"{path} is not a directory.") - if path.owner() != folder.owner: - raise MoveError(f"{path} is not owned by {folder.owner}.") + for bind in binds: + bind.validate() -def unbind_folders(owned_folders: List[OwnedPath]) -> None: +def unbind_folders(owned_folders: List[Bind]) -> None: for folder in owned_folders: - try: - subprocess.run( - ["umount", folder.path], - check=True, - ) - except subprocess.CalledProcessError: - raise MoveError(f"Unable to unmount folder {folder.path}.") + folder.unbind() -def move_folders_to_volume( - folders: List[OwnedPath], - old_volume_name: str, # TODO: pass an actual validated block device +# May be moved into Bind +def move_data_to_volume( + binds: List[Bind], new_volume: BlockDevice, job: Job, -) -> None: +) -> List[Bind]: current_progress = job.progress if current_progress is None: current_progress = 0 - progress_per_folder = 50 // len(folders) - for folder in folders: - shutil.move( - location_at_volume(folder, old_volume_name), - location_at_volume(folder, new_volume.name), - ) + progress_per_folder = 50 // len(binds) + for bind in binds: + old_location = bind.location_at_volume() + bind.drive = new_volume + new_location = bind.location_at_volume() + + try: + shutil.move(old_location, new_location) + except Exception as error: + raise MoveError( + f"could not move {old_location} to {new_location} : {str(error)}" + ) from error + progress = current_progress + progress_per_folder report_progress(progress, job, "Moving data to new volume...") + return binds -def ensure_folder_ownership(folders: List[OwnedPath], volume: BlockDevice) -> None: +def ensure_folder_ownership(folders: List[Bind]) -> None: for folder in folders: - true_location = location_at_volume(folder, volume.name) - try: - subprocess.run( - [ - "chown", - "-R", - f"{folder.owner}:{folder.group}", - # Could we just chown the binded location instead? - true_location, - ], - check=True, - ) - except subprocess.CalledProcessError as error: - print(error.output) - error_message = ( - f"Unable to set ownership of {true_location} :{error.output}" - ) - raise MoveError(error_message) + folder.ensure_ownership() -def bind_folders(folders: List[OwnedPath], volume: BlockDevice) -> None: +def bind_folders(folders: List[Bind], volume: BlockDevice): for folder in folders: - try: - subprocess.run( - [ - "mount", - "--bind", - location_at_volume(folder, volume.name), - folder.path, - ], - check=True, - ) - except subprocess.CalledProcessError as error: - print(error.output) - raise MoveError(f"Unable to mount new volume:{error.output}") + folder.bind() diff --git a/selfprivacy_api/services/owned_path.py b/selfprivacy_api/services/owned_path.py index 23542dc..da40510 100644 --- a/selfprivacy_api/services/owned_path.py +++ b/selfprivacy_api/services/owned_path.py @@ -1,7 +1,103 @@ +from __future__ import annotations +import subprocess +import pathlib from pydantic import BaseModel +from selfprivacy_api.utils.block_devices import BlockDevice, BlockDevices + +class BindError(Exception): + pass + + +# May be deprecated because of Binds class OwnedPath(BaseModel): path: str owner: str group: str + + +class Bind: + """ + A directory that resides on some volume but we mount it into fs + where we need it. + Used for service data. + """ + + def __init__(self, binding_path: str, owner: str, group: str, drive: BlockDevice): + self.binding_path = binding_path + self.owner = owner + self.group = group + self.drive = drive + + # TODO: make Service return a list of binds instead of owned paths + @staticmethod + def from_owned_path(path: OwnedPath, drive_name: str) -> Bind: + drive = BlockDevices().get_block_device(drive_name) + if drive is None: + raise BindError(f"No such drive: {drive_name}") + + return Bind( + binding_path=path.path, owner=path.owner, group=path.group, drive=drive + ) + + def bind_foldername(self) -> str: + return self.binding_path.split("/")[-1] + + def location_at_volume(self) -> str: + return f"/volumes/{self.drive.name}/{self.bind_foldername()}" + + def validate(self) -> str: + path = pathlib.Path(self.location_at_volume()) + + if not path.exists(): + raise BindError(f"directory {path} is not found.") + if not path.is_dir(): + raise BindError(f"{path} is not a directory.") + if path.owner() != self.owner: + raise BindError(f"{path} is not owned by {self.owner}.") + + def bind(self) -> None: + try: + subprocess.run( + [ + "mount", + "--bind", + self.location_at_volume(), + self.binding_path, + ], + check=True, + ) + except subprocess.CalledProcessError as error: + print(error.output) + raise BindError(f"Unable to mount new volume:{error.output}") + + def unbind(self) -> None: + try: + subprocess.run( + ["umount", self.binding_path], + check=True, + ) + except subprocess.CalledProcessError: + raise BindError(f"Unable to unmount folder {self.binding_path}.") + pass + + def ensure_ownership(self) -> None: + true_location = self.location_at_volume() + try: + subprocess.run( + [ + "chown", + "-R", + f"{self.owner}:{self.group}", + # Could we just chown the binded location instead? + true_location, + ], + check=True, + ) + except subprocess.CalledProcessError as error: + print(error.output) + error_message = ( + f"Unable to set ownership of {true_location} :{error.output}" + ) + raise BindError(error_message) diff --git a/selfprivacy_api/services/service.py b/selfprivacy_api/services/service.py index e60cf8a..19e395e 100644 --- a/selfprivacy_api/services/service.py +++ b/selfprivacy_api/services/service.py @@ -9,15 +9,15 @@ from selfprivacy_api.jobs import Job, Jobs, JobStatus, report_progress from selfprivacy_api.utils.block_devices import BlockDevice, BlockDevices from selfprivacy_api.services.generic_size_counter import get_storage_usage -from selfprivacy_api.services.owned_path import OwnedPath +from selfprivacy_api.services.owned_path import OwnedPath, Bind from selfprivacy_api.services.moving import ( - check_folders, + check_binds, check_volume, unbind_folders, bind_folders, ensure_folder_ownership, MoveError, - move_folders_to_volume, + move_data_to_volume, ) from selfprivacy_api import utils @@ -319,6 +319,13 @@ class Service(ABC): user_data["modules"][service_id] = {} user_data["modules"][service_id]["location"] = volume.name + def binds(self) -> typing.List[Bind]: + owned_folders = self.get_owned_folders() + + return [ + Bind.from_owned_path(folder, self.get_drive()) for folder in owned_folders + ] + def assert_can_move(self, new_volume): """ Checks if the service can be moved to new volume @@ -338,11 +345,10 @@ class Service(ABC): check_volume(new_volume, space_needed=self.get_storage_usage()) - owned_folders = self.get_owned_folders() - if owned_folders == []: + binds = self.binds() + if binds == []: raise MoveError("nothing to move") - - check_folders(current_volume_name, owned_folders) + check_binds(current_volume_name, binds) def do_move_to_volume( self, @@ -351,21 +357,20 @@ class Service(ABC): ): """ Move a service to another volume. + Note: It may be much simpler to write it per bind, but a bit less safe? """ service_name = self.get_display_name() - # TODO : Make sure device exists - old_volume_name = self.get_drive() - owned_folders = self.get_owned_folders() + binds = self.binds() report_progress(10, job, "Unmounting folders from old volume...") - unbind_folders(owned_folders) + unbind_folders(binds) report_progress(20, job, "Moving data to new volume...") - move_folders_to_volume(owned_folders, old_volume_name, new_volume, job) + binds = move_data_to_volume(binds, new_volume, job) report_progress(70, job, f"Making sure {service_name} owns its files...") try: - ensure_folder_ownership(owned_folders, new_volume) + ensure_folder_ownership(binds) except Exception as error: # We have logged it via print and we additionally log it here in the error field # We are continuing anyway but Job has no warning field @@ -377,7 +382,7 @@ class Service(ABC): ) report_progress(90, job, f"Mounting {service_name} data...") - bind_folders(owned_folders, new_volume) + bind_folders(binds) report_progress(95, job, f"Finishing moving {service_name}...") self.set_location(new_volume) diff --git a/selfprivacy_api/utils/block_devices.py b/selfprivacy_api/utils/block_devices.py index ab3794d..f1a4149 100644 --- a/selfprivacy_api/utils/block_devices.py +++ b/selfprivacy_api/utils/block_devices.py @@ -4,6 +4,8 @@ import subprocess import json import typing +from pydantic import BaseModel + from selfprivacy_api.utils import WriteUserData from selfprivacy_api.utils.singleton_metaclass import SingletonMetaclass diff --git a/tests/test_graphql/test_services.py b/tests/test_graphql/test_services.py index 9208371..d509a6f 100644 --- a/tests/test_graphql/test_services.py +++ b/tests/test_graphql/test_services.py @@ -32,9 +32,9 @@ MOVER_MOCK_PROCESS = CompletedProcess(["ls"], returncode=0) @pytest.fixture() -def mock_check_service_mover_folders(mocker): +def mock_check_service_mover_binds(mocker): mock = mocker.patch( - "selfprivacy_api.services.service.check_folders", + "selfprivacy_api.services.service.check_binds", autospec=True, return_value=None, ) @@ -569,7 +569,7 @@ def test_graphql_move_service_without_folders_on_old_volume( def test_graphql_move_service( authorized_client, generic_userdata, - mock_check_service_mover_folders, + mock_check_service_mover_binds, lsblk_singular_mock, dummy_service: DummyService, mock_subprocess_run, From 305e5cc2c3c02d9fced918c87e6259cc6e42b9ef Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Thu, 29 Feb 2024 00:54:39 +0000 Subject: [PATCH 31/35] refactor(services): introduce Bind class and test moving deeper --- selfprivacy_api/services/moving.py | 2 +- selfprivacy_api/services/owned_path.py | 42 +++++---- selfprivacy_api/utils/block_devices.py | 3 + tests/conftest.py | 40 +++++++++ tests/test_binds.py | 92 +++++++++++++++++++ tests/test_block_device_utils.py | 1 + tests/test_graphql/test_services.py | 118 +++++++++++++++++++------ 7 files changed, 253 insertions(+), 45 deletions(-) create mode 100644 tests/test_binds.py diff --git a/selfprivacy_api/services/moving.py b/selfprivacy_api/services/moving.py index 09af765..8b6d3b1 100644 --- a/selfprivacy_api/services/moving.py +++ b/selfprivacy_api/services/moving.py @@ -67,6 +67,6 @@ def ensure_folder_ownership(folders: List[Bind]) -> None: folder.ensure_ownership() -def bind_folders(folders: List[Bind], volume: BlockDevice): +def bind_folders(folders: List[Bind]): for folder in folders: folder.bind() diff --git a/selfprivacy_api/services/owned_path.py b/selfprivacy_api/services/owned_path.py index da40510..6e85fb0 100644 --- a/selfprivacy_api/services/owned_path.py +++ b/selfprivacy_api/services/owned_path.py @@ -2,9 +2,13 @@ from __future__ import annotations import subprocess import pathlib from pydantic import BaseModel +from os.path import exists from selfprivacy_api.utils.block_devices import BlockDevice, BlockDevices +# tests override it to a tmpdir +VOLUMES_PATH = "/volumes" + class BindError(Exception): pass @@ -19,9 +23,8 @@ class OwnedPath(BaseModel): class Bind: """ - A directory that resides on some volume but we mount it into fs - where we need it. - Used for service data. + A directory that resides on some volume but we mount it into fs where we need it. + Used for storing service data. """ def __init__(self, binding_path: str, owner: str, group: str, drive: BlockDevice): @@ -30,7 +33,7 @@ class Bind: self.group = group self.drive = drive - # TODO: make Service return a list of binds instead of owned paths + # TODO: delete owned path interface from Service @staticmethod def from_owned_path(path: OwnedPath, drive_name: str) -> Bind: drive = BlockDevices().get_block_device(drive_name) @@ -45,9 +48,9 @@ class Bind: return self.binding_path.split("/")[-1] def location_at_volume(self) -> str: - return f"/volumes/{self.drive.name}/{self.bind_foldername()}" + return f"{VOLUMES_PATH}/{self.drive.name}/{self.bind_foldername()}" - def validate(self) -> str: + def validate(self) -> None: path = pathlib.Path(self.location_at_volume()) if not path.exists(): @@ -58,23 +61,29 @@ class Bind: raise BindError(f"{path} is not owned by {self.owner}.") def bind(self) -> None: + if not exists(self.binding_path): + raise BindError(f"cannot bind to a non-existing path: {self.binding_path}") + + source = self.location_at_volume() + target = self.binding_path + try: subprocess.run( - [ - "mount", - "--bind", - self.location_at_volume(), - self.binding_path, - ], + ["mount", "--bind", source, target], + stderr=subprocess.PIPE, check=True, ) except subprocess.CalledProcessError as error: - print(error.output) - raise BindError(f"Unable to mount new volume:{error.output}") + print(error.stderr) + raise BindError(f"Unable to bind {source} to {target} :{error.stderr}") def unbind(self) -> None: + if not exists(self.binding_path): + raise BindError(f"cannot unbind a non-existing path: {self.binding_path}") + try: subprocess.run( + # umount -l ? ["umount", self.binding_path], check=True, ) @@ -94,10 +103,11 @@ class Bind: true_location, ], check=True, + stderr=subprocess.PIPE, ) except subprocess.CalledProcessError as error: - print(error.output) + print(error.stderr) error_message = ( - f"Unable to set ownership of {true_location} :{error.output}" + f"Unable to set ownership of {true_location} :{error.stderr}" ) raise BindError(error_message) diff --git a/selfprivacy_api/utils/block_devices.py b/selfprivacy_api/utils/block_devices.py index f1a4149..4de5b75 100644 --- a/selfprivacy_api/utils/block_devices.py +++ b/selfprivacy_api/utils/block_devices.py @@ -171,6 +171,9 @@ class BlockDevice: return False +# TODO: SingletonMetaclass messes with tests and is able to persist state +# between them. If you have very weird test crosstalk that's probably why +# I am not sure it NEEDS to be SingletonMetaclass class BlockDevices(metaclass=SingletonMetaclass): """Singleton holding all Block devices""" diff --git a/tests/conftest.py b/tests/conftest.py index e651c08..0cd1493 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -4,6 +4,7 @@ import os import pytest import datetime +import subprocess from os import path from os import makedirs @@ -135,6 +136,18 @@ def wrong_auth_client(huey_database, redis_repo_with_tokens): return client +@pytest.fixture() +def volume_folders(tmpdir, mocker): + volumes_dir = path.join(tmpdir, "volumes") + + makedirs(volumes_dir) + volumenames = ["sda1", "sda2"] + for d in volumenames: + service_dir = path.join(volumes_dir, d) + makedirs(service_dir) + mock = mocker.patch("selfprivacy_api.services.owned_path.VOLUMES_PATH", volumes_dir) + + @pytest.fixture() def raw_dummy_service(tmpdir): dirnames = ["test_service", "also_test_service"] @@ -161,11 +174,38 @@ def raw_dummy_service(tmpdir): return service +def ensure_user_exists(user: str): + try: + output = subprocess.check_output( + ["useradd", "-U", user], stderr=subprocess.PIPE, shell=False + ) + except subprocess.CalledProcessError as error: + if b"already exists" not in error.stderr: + raise error + + try: + output = subprocess.check_output( + ["useradd", user], stderr=subprocess.PIPE, shell=False + ) + except subprocess.CalledProcessError as error: + assert b"already exists" in error.stderr + return + + raise ValueError("could not create user", user) + + @pytest.fixture() def dummy_service( tmpdir, raw_dummy_service, generic_userdata ) -> Generator[Service, None, None]: service = raw_dummy_service + user = service.get_user() + + # TODO: use create_user from users actions. But it will need NIXOS to be there + # and react to our changes to files. + # from selfprivacy_api.actions.users import create_user + # create_user(user, "yay, it is me") + ensure_user_exists(user) # register our service services.services.append(service) diff --git a/tests/test_binds.py b/tests/test_binds.py new file mode 100644 index 0000000..ef9a0d5 --- /dev/null +++ b/tests/test_binds.py @@ -0,0 +1,92 @@ +import pytest +from os import mkdir, rmdir +from os.path import join, exists + + +from tests.conftest import ensure_user_exists +from tests.test_graphql.test_services import mock_lsblk_devices + +from selfprivacy_api.services.owned_path import Bind, BindError +from selfprivacy_api.utils.block_devices import BlockDevices +from selfprivacy_api.utils.waitloop import wait_until_true + + +BINDTESTS_USER = "binduser" +TESTFILE_CONTENTS = "testissimo" +TESTFILE_NAME = "testfile" + + +@pytest.fixture() +def bind_user(): + ensure_user_exists(BINDTESTS_USER) + return BINDTESTS_USER + + +def prepare_test_bind(tmpdir, bind_user) -> Bind: + test_binding_name = "bindy_dir" + binding_path = join(tmpdir, test_binding_name) + drive = BlockDevices().get_block_device("sda2") + assert drive is not None + + bind = Bind( + binding_path=binding_path, owner=bind_user, group=bind_user, drive=drive + ) + + source_dir = bind.location_at_volume() + mkdir(source_dir) + mkdir(binding_path) + + testfile_path = join(source_dir, TESTFILE_NAME) + with open(testfile_path, "w") as file: + file.write(TESTFILE_CONTENTS) + + return bind + + +def test_bind_unbind(volume_folders, tmpdir, bind_user, mock_lsblk_devices): + bind = prepare_test_bind(tmpdir, bind_user) + bind.ensure_ownership() + bind.validate() + + testfile_path = join(bind.location_at_volume(), TESTFILE_NAME) + assert exists(testfile_path) + with open(testfile_path, "r") as file: + assert file.read() == TESTFILE_CONTENTS + + bind.bind() + + testfile_binding_path = join(bind.binding_path, TESTFILE_NAME) + assert exists(testfile_path) + with open(testfile_path, "r") as file: + assert file.read() == TESTFILE_CONTENTS + + bind.unbind() + # wait_until_true(lambda : not exists(testfile_binding_path), timeout_sec=2) + assert not exists(testfile_binding_path) + assert exists(bind.binding_path) + + +def test_bind_nonexistent_target(volume_folders, tmpdir, bind_user, mock_lsblk_devices): + bind = prepare_test_bind(tmpdir, bind_user) + + bind.ensure_ownership() + bind.validate() + rmdir(bind.binding_path) + + with pytest.raises(BindError): + bind.bind() + + +def test_unbind_nonexistent_target( + volume_folders, tmpdir, bind_user, mock_lsblk_devices +): + bind = prepare_test_bind(tmpdir, bind_user) + + bind.ensure_ownership() + bind.validate() + bind.bind() + + bind.binding_path = "/bogus" + + with pytest.raises(BindError): + bind.unbind() diff --git a/tests/test_block_device_utils.py b/tests/test_block_device_utils.py index 41c30c8..2162c4d 100644 --- a/tests/test_block_device_utils.py +++ b/tests/test_block_device_utils.py @@ -410,6 +410,7 @@ def lsblk_full_mock(mocker): mock = mocker.patch( "subprocess.check_output", autospec=True, return_value=FULL_LSBLK_OUTPUT ) + BlockDevices().update() return mock diff --git a/tests/test_graphql/test_services.py b/tests/test_graphql/test_services.py index d509a6f..6d841d6 100644 --- a/tests/test_graphql/test_services.py +++ b/tests/test_graphql/test_services.py @@ -1,5 +1,8 @@ import pytest +import shutil + from typing import Generator +from os import mkdir from selfprivacy_api.utils.block_devices import BlockDevices @@ -8,13 +11,77 @@ from selfprivacy_api.services import get_service_by_id from selfprivacy_api.services.service import Service, ServiceStatus from selfprivacy_api.services.test_service import DummyService -# from selfprivacy_api.services.moving import check_folders - from tests.common import generate_service_query from tests.test_graphql.common import assert_empty, assert_ok, get_data from tests.test_block_device_utils import lsblk_singular_mock -from subprocess import CompletedProcess + +LSBLK_BLOCKDEVICES_DICTS = [ + { + "name": "sda1", + "path": "/dev/sda1", + "fsavail": "4614107136", + "fssize": "19814920192", + "fstype": "ext4", + "fsused": "14345314304", + "mountpoints": ["/nix/store", "/"], + "label": None, + "uuid": "ec80c004-baec-4a2c-851d-0e1807135511", + "size": 20210236928, + "model": None, + "serial": None, + "type": "part", + }, + { + "name": "sda2", + "path": "/dev/sda2", + "fsavail": "4614107136", + "fssize": "19814920192", + "fstype": "ext4", + "fsused": "14345314304", + "mountpoints": ["/home"], + "label": None, + "uuid": "deadbeef-baec-4a2c-851d-0e1807135511", + "size": 20210236928, + "model": None, + "serial": None, + "type": "part", + }, +] + + +@pytest.fixture() +def mock_lsblk_devices(mocker): + mock = mocker.patch( + "selfprivacy_api.utils.block_devices.BlockDevices.lsblk_device_dicts", + autospec=True, + return_value=LSBLK_BLOCKDEVICES_DICTS, + ) + BlockDevices().update() + assert BlockDevices().lsblk_device_dicts() == LSBLK_BLOCKDEVICES_DICTS + devices = BlockDevices().get_block_devices() + + assert len(devices) == 2 + + names = [device.name for device in devices] + assert "sda1" in names + assert "sda2" in names + return mock + + +@pytest.fixture() +def dummy_service_with_binds(dummy_service, mock_lsblk_devices, volume_folders): + binds = dummy_service.binds() + for bind in binds: + path = bind.binding_path + shutil.move(bind.binding_path, bind.location_at_volume()) + mkdir(bind.binding_path) + + bind.ensure_ownership() + bind.validate() + + bind.bind() + return dummy_service @pytest.fixture() @@ -28,9 +95,17 @@ def only_dummy_service(dummy_service) -> Generator[DummyService, None, None]: service_module.services.extend(back_copy) -MOVER_MOCK_PROCESS = CompletedProcess(["ls"], returncode=0) +@pytest.fixture() +def mock_check_volume(mocker): + mock = mocker.patch( + "selfprivacy_api.services.service.check_volume", + autospec=True, + return_value=None, + ) + return mock +# TODO: remove @pytest.fixture() def mock_check_service_mover_binds(mocker): mock = mocker.patch( @@ -41,20 +116,6 @@ def mock_check_service_mover_binds(mocker): return mock -@pytest.fixture() -def mock_subprocess_run(mocker): - mock = mocker.patch( - "subprocess.run", autospec=True, return_value=MOVER_MOCK_PROCESS - ) - return mock - - -@pytest.fixture() -def mock_shutil_move(mocker): - mock = mocker.patch("shutil.move", autospec=True, return_value=None) - return mock - - API_START_MUTATION = """ mutation TestStartService($service_id: String!) { services { @@ -551,7 +612,7 @@ def test_move_same_volume(authorized_client, dummy_service): def test_graphql_move_service_without_folders_on_old_volume( authorized_client, generic_userdata, - lsblk_singular_mock, + mock_lsblk_devices, dummy_service: DummyService, ): target = "sda1" @@ -564,26 +625,27 @@ def test_graphql_move_service_without_folders_on_old_volume( data = get_data(mutation_response)["services"]["moveService"] assert_errorcode(data, 400) + assert "sda2/test_service is not found" in data["message"] def test_graphql_move_service( authorized_client, generic_userdata, + # TODO: substitute with a weaker mock or delete altogether mock_check_service_mover_binds, - lsblk_singular_mock, - dummy_service: DummyService, - mock_subprocess_run, - mock_shutil_move, + mock_check_volume, + dummy_service_with_binds, ): - # Does not check real moving, - # but tests the finished job propagation through API + dummy_service = dummy_service_with_binds - target = "sda1" - BlockDevices().update() + origin = "sda1" + target = "sda2" assert BlockDevices().get_block_device(target) is not None + assert BlockDevices().get_block_device(origin) is not None + dummy_service.set_drive(origin) dummy_service.set_simulated_moves(False) - dummy_service.set_drive("sda2") + mutation_response = api_move(authorized_client, dummy_service, target) data = get_data(mutation_response)["services"]["moveService"] From 3f9d2b2481c54e97c24199f1c4cb5ad08dde3754 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Fri, 1 Mar 2024 14:30:54 +0000 Subject: [PATCH 32/35] refactor(services): remove too many imports and cleanup --- selfprivacy_api/actions/services.py | 4 +--- selfprivacy_api/graphql/mutations/services_mutations.py | 2 -- selfprivacy_api/jobs/__init__.py | 8 +++++--- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/selfprivacy_api/actions/services.py b/selfprivacy_api/actions/services.py index 56d35e9..ebb0917 100644 --- a/selfprivacy_api/actions/services.py +++ b/selfprivacy_api/actions/services.py @@ -30,7 +30,5 @@ def move_service(service_id: str, volume_name: str) -> Job: description=f"Moving {service.get_display_name()} data to {volume.name}", ) - handle = move_service_task(service, volume, job) - # Nonblocking - handle() + move_service_task(service, volume, job) return job diff --git a/selfprivacy_api/graphql/mutations/services_mutations.py b/selfprivacy_api/graphql/mutations/services_mutations.py index 97eb4d9..be0cb77 100644 --- a/selfprivacy_api/graphql/mutations/services_mutations.py +++ b/selfprivacy_api/graphql/mutations/services_mutations.py @@ -5,7 +5,6 @@ import strawberry from selfprivacy_api.graphql import IsAuthenticated from selfprivacy_api.graphql.common_types.jobs import job_to_api_job from selfprivacy_api.jobs import JobStatus -from selfprivacy_api.utils.block_devices import BlockDevices from traceback import format_tb as format_traceback @@ -23,7 +22,6 @@ from selfprivacy_api.actions.services import ( ServiceNotFoundError, VolumeNotFoundError, ) -from selfprivacy_api.services.moving import MoveError from selfprivacy_api.services import get_service_by_id diff --git a/selfprivacy_api/jobs/__init__.py b/selfprivacy_api/jobs/__init__.py index 7f46e9d..4649bb0 100644 --- a/selfprivacy_api/jobs/__init__.py +++ b/selfprivacy_api/jobs/__init__.py @@ -268,10 +268,12 @@ class Jobs: return False -# A terse way to call a common operation, for readability -# job.report_progress() would be even better -# but it would go against how this file is written def report_progress(progress: int, job: Job, status_text: str) -> None: + """ + A terse way to call a common operation, for readability + job.report_progress() would be even better + but it would go against how this file is written + """ Jobs.update( job=job, status=JobStatus.RUNNING, From eeef2891c9cb74cda797340704fbd6178baeee3e Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 4 Mar 2024 14:26:26 +0000 Subject: [PATCH 33/35] fix(services): fix merge bug --- selfprivacy_api/services/service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/selfprivacy_api/services/service.py b/selfprivacy_api/services/service.py index 19e395e..576877c 100644 --- a/selfprivacy_api/services/service.py +++ b/selfprivacy_api/services/service.py @@ -319,7 +319,7 @@ class Service(ABC): user_data["modules"][service_id] = {} user_data["modules"][service_id]["location"] = volume.name - def binds(self) -> typing.List[Bind]: + def binds(self) -> List[Bind]: owned_folders = self.get_owned_folders() return [ From fd43a6ccf18df94d08d81b39d07c255a5b2ee152 Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 4 Mar 2024 17:16:08 +0000 Subject: [PATCH 34/35] doc(services): explain the Owned Path reason d'etre after trying to remove it --- selfprivacy_api/services/owned_path.py | 15 ++++++++++++++- selfprivacy_api/services/service.py | 22 +++++++++++++++++++--- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/selfprivacy_api/services/owned_path.py b/selfprivacy_api/services/owned_path.py index 6e85fb0..aa6e92e 100644 --- a/selfprivacy_api/services/owned_path.py +++ b/selfprivacy_api/services/owned_path.py @@ -14,8 +14,21 @@ class BindError(Exception): pass -# May be deprecated because of Binds class OwnedPath(BaseModel): + """ + A convenient interface for explicitly defining ownership of service folders. + One overrides Service.get_owned_paths() for this. + + Why this exists?: + One could use Bind to define ownership but then one would need to handle drive which + is unnecessary and produces code duplication. + + It is also somewhat semantically wrong to include Owned Path into Bind + instead of user and group. Because owner and group in Bind are applied to + the original folder on the drive, not to the binding path. But maybe it is + ok since they are technically both owned. Idk yet. + """ + path: str owner: str group: str diff --git a/selfprivacy_api/services/service.py b/selfprivacy_api/services/service.py index 576877c..9add2dc 100644 --- a/selfprivacy_api/services/service.py +++ b/selfprivacy_api/services/service.py @@ -411,11 +411,27 @@ class Service(ABC): @classmethod def owned_path(cls, path: str): - """A default guess on folder ownership""" + """Default folder ownership""" + service_name = cls.get_display_name() + + try: + owner = cls.get_user() + if owner is None: + # TODO: assume root? + # (if we do not want to do assumptions, maybe not declare user optional?) + raise LookupError(f"no user for service: {service_name}") + group = cls.get_group() + if group is None: + raise LookupError(f"no group for service: {service_name}") + except Exception as error: + raise LookupError( + f"when deciding a bind for folder {path} of service {service_name}, error: {str(error)}" + ) + return OwnedPath( path=path, - owner=cls.get_user(), - group=cls.get_group(), + owner=owner, + group=group, ) def pre_backup(self): From 7dae81530e990233af0659b2085abc999a85341a Mon Sep 17 00:00:00 2001 From: Houkime <> Date: Mon, 4 Mar 2024 17:37:26 +0000 Subject: [PATCH 35/35] test(services): clean up tests --- tests/conftest.py | 2 +- tests/test_graphql/test_services.py | 13 ------------- tests/test_services.py | 15 --------------- 3 files changed, 1 insertion(+), 29 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 0cd1493..dceac72 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -149,7 +149,7 @@ def volume_folders(tmpdir, mocker): @pytest.fixture() -def raw_dummy_service(tmpdir): +def raw_dummy_service(tmpdir) -> DummyService: dirnames = ["test_service", "also_test_service"] service_dirs = [] for d in dirnames: diff --git a/tests/test_graphql/test_services.py b/tests/test_graphql/test_services.py index 6d841d6..06ef3a1 100644 --- a/tests/test_graphql/test_services.py +++ b/tests/test_graphql/test_services.py @@ -105,17 +105,6 @@ def mock_check_volume(mocker): return mock -# TODO: remove -@pytest.fixture() -def mock_check_service_mover_binds(mocker): - mock = mocker.patch( - "selfprivacy_api.services.service.check_binds", - autospec=True, - return_value=None, - ) - return mock - - API_START_MUTATION = """ mutation TestStartService($service_id: String!) { services { @@ -631,8 +620,6 @@ def test_graphql_move_service_without_folders_on_old_volume( def test_graphql_move_service( authorized_client, generic_userdata, - # TODO: substitute with a weaker mock or delete altogether - mock_check_service_mover_binds, mock_check_volume, dummy_service_with_binds, ): diff --git a/tests/test_services.py b/tests/test_services.py index 98cfa4e..de828d8 100644 --- a/tests/test_services.py +++ b/tests/test_services.py @@ -80,21 +80,6 @@ def test_paths_from_owned_paths(): ] -# def test_foldermoves_from_ownedpaths(): -# owned = OwnedPath( -# path="var/lib/bitwarden", -# group="vaultwarden", -# owner="vaultwarden", -# ) - -# assert FolderMoveNames.from_owned_path(owned) == FolderMoveNames( -# name="bitwarden", -# bind_location="var/lib/bitwarden", -# group="vaultwarden", -# owner="vaultwarden", -# ) - - def test_enabling_disabling_reads_json(dummy_service: DummyService): with WriteUserData() as data: data["modules"][dummy_service.get_id()]["enable"] = False