From d143359a2d0e277a8baf7503405b75180d87eee8 Mon Sep 17 00:00:00 2001 From: DavHau Date: Sat, 5 Jul 2025 15:54:37 +0700 Subject: [PATCH] refactor: reduce coupling to Machine class in vars module - Change Generator class to store machine name as string instead of Machine reference - Update Generator.generators_from_flake() to only require machine name and flake - Refactor check_vars() to accept machine name and flake instead of Machine object - Create Machine instances only when needed for specific operations This continues the effort to reduce dependencies on the Machine class, making the codebase more modular and easier to refactor. --- pkgs/clan-cli/clan_cli/flash/flash.py | 4 +- pkgs/clan-cli/clan_cli/tests/test_modules.py | 4 +- pkgs/clan-cli/clan_cli/tests/test_vars.py | 14 +++--- pkgs/clan-cli/clan_cli/vars/check.py | 21 ++++----- pkgs/clan-cli/clan_cli/vars/fix.py | 2 +- pkgs/clan-cli/clan_cli/vars/generate.py | 44 +++++++++---------- pkgs/clan-cli/clan_cli/vars/list.py | 10 ++--- .../vars/secret_modules/password_store.py | 4 +- .../clan_cli/vars/secret_modules/sops.py | 8 ++-- 9 files changed, 50 insertions(+), 61 deletions(-) diff --git a/pkgs/clan-cli/clan_cli/flash/flash.py b/pkgs/clan-cli/clan_cli/flash/flash.py index 5b409bf3b..aa77db5f5 100644 --- a/pkgs/clan-cli/clan_cli/flash/flash.py +++ b/pkgs/clan-cli/clan_cli/flash/flash.py @@ -93,9 +93,7 @@ def flash_machine( from clan_cli.vars.generate import Generator - for generator in Generator.generators_from_flake( - machine.name, machine.flake, machine - ): + for generator in Generator.generators_from_flake(machine.name, machine.flake): for file in generator.files: if file.needed_for == "partitioning": msg = f"Partitioning time secrets are not supported with `clan flash write`: clan.core.vars.generators.{generator.name}.files.{file.name}" diff --git a/pkgs/clan-cli/clan_cli/tests/test_modules.py b/pkgs/clan-cli/clan_cli/tests/test_modules.py index 045fb8f97..94de8a815 100644 --- a/pkgs/clan-cli/clan_cli/tests/test_modules.py +++ b/pkgs/clan-cli/clan_cli/tests/test_modules.py @@ -118,9 +118,7 @@ def test_add_module_to_inventory( generator = None - generators = Generator.generators_from_flake( - machine.name, machine.flake, machine - ) + generators = Generator.generators_from_flake(machine.name, machine.flake) for gen in generators: if gen.name == "borgbackup": generator = gen diff --git a/pkgs/clan-cli/clan_cli/tests/test_vars.py b/pkgs/clan-cli/clan_cli/tests/test_vars.py index 306f61615..181b2a543 100644 --- a/pkgs/clan-cli/clan_cli/tests/test_vars.py +++ b/pkgs/clan-cli/clan_cli/tests/test_vars.py @@ -140,7 +140,7 @@ def test_generate_public_and_secret_vars( monkeypatch.chdir(flake.path) machine = Machine(name="my_machine", flake=Flake(str(flake.path))) - assert not check_vars(machine) + assert not check_vars(machine.name, machine.flake) vars_text = stringify_all_vars(machine) assert "my_generator/my_value: " in vars_text assert "my_generator/my_secret: " in vars_text @@ -158,7 +158,7 @@ def test_generate_public_and_secret_vars( assert json.loads(value_non_default) == "default_value" cli.run(["vars", "generate", "--flake", str(flake.path), "my_machine"]) - assert check_vars(machine) + assert check_vars(machine.name, machine.flake) # get last commit message commit_message = run( ["git", "log", "-3", "--pretty=%B"], @@ -351,10 +351,10 @@ def test_generated_shared_secret_sops( machine1 = Machine(name="machine1", flake=Flake(str(flake.path))) machine2 = Machine(name="machine2", flake=Flake(str(flake.path))) cli.run(["vars", "generate", "--flake", str(flake.path), "machine1"]) - assert check_vars(machine1) + assert check_vars(machine1.name, machine1.flake) cli.run(["vars", "generate", "--flake", str(flake.path), "machine2"]) - assert check_vars(machine2) - assert check_vars(machine2) + assert check_vars(machine2.name, machine2.flake) + assert check_vars(machine2.name, machine2.flake) m1_sops_store = sops.SecretStore(machine1) m2_sops_store = sops.SecretStore(machine2) assert m1_sops_store.exists( @@ -404,9 +404,9 @@ def test_generate_secret_var_password_store( monkeypatch.setenv("PASSWORD_STORE_DIR", str(password_store_dir)) machine = Machine(name="my_machine", flake=Flake(str(flake.path))) - assert not check_vars(machine) + assert not check_vars(machine.name, machine.flake) cli.run(["vars", "generate", "--flake", str(flake.path), "my_machine"]) - assert check_vars(machine) + assert check_vars(machine.name, machine.flake) store = password_store.SecretStore( Machine(name="my_machine", flake=Flake(str(flake.path))) ) diff --git a/pkgs/clan-cli/clan_cli/vars/check.py b/pkgs/clan-cli/clan_cli/vars/check.py index 417dad36d..0c3fe484b 100644 --- a/pkgs/clan-cli/clan_cli/vars/check.py +++ b/pkgs/clan-cli/clan_cli/vars/check.py @@ -4,6 +4,7 @@ import logging from clan_cli.completions import add_dynamic_completer, complete_machines from clan_lib.errors import ClanError from clan_lib.machines.machines import Machine +from clan_lib.flake import Flake from typing import TYPE_CHECKING if TYPE_CHECKING: @@ -26,7 +27,10 @@ class VarStatus: self.invalid_generators = invalid_generators -def vars_status(machine: Machine, generator_name: None | str = None) -> VarStatus: +def vars_status( + machine_name: str, flake: Flake, generator_name: None | str = None +) -> VarStatus: + machine = Machine(name=machine_name, flake=flake) missing_secret_vars = [] missing_public_vars = [] # signals if a var needs to be updated (eg. needs re-encryption due to new users added) @@ -34,7 +38,7 @@ def vars_status(machine: Machine, generator_name: None | str = None) -> VarStatu invalid_generators = [] from clan_cli.vars.generate import Generator - generators = Generator.generators_from_flake(machine.name, machine.flake, machine) + generators = Generator.generators_from_flake(machine.name, machine.flake) if generator_name: for generator in generators: if generator_name == generator.name: @@ -47,7 +51,6 @@ def vars_status(machine: Machine, generator_name: None | str = None) -> VarStatu raise ClanError(err_msg) for generator in generators: - generator.machine(machine) for file in generator.files: file.store( machine.secret_vars_store if file.secret else machine.public_vars_store @@ -93,8 +96,10 @@ def vars_status(machine: Machine, generator_name: None | str = None) -> VarStatu ) -def check_vars(machine: Machine, generator_name: None | str = None) -> bool: - status = vars_status(machine, generator_name=generator_name) +def check_vars( + machine_name: str, flake: Flake, generator_name: None | str = None +) -> bool: + status = vars_status(machine_name, flake, generator_name=generator_name) return not ( status.missing_secret_vars or status.missing_public_vars @@ -104,11 +109,7 @@ def check_vars(machine: Machine, generator_name: None | str = None) -> bool: def check_command(args: argparse.Namespace) -> None: - machine = Machine( - name=args.machine, - flake=args.flake, - ) - ok = check_vars(machine, generator_name=args.generator) + ok = check_vars(args.machine, args.flake, generator_name=args.generator) if not ok: raise SystemExit(1) diff --git a/pkgs/clan-cli/clan_cli/vars/fix.py b/pkgs/clan-cli/clan_cli/vars/fix.py index 9d5889fbd..6ad0e6ab2 100644 --- a/pkgs/clan-cli/clan_cli/vars/fix.py +++ b/pkgs/clan-cli/clan_cli/vars/fix.py @@ -11,7 +11,7 @@ log = logging.getLogger(__name__) def fix_vars(machine: Machine, generator_name: None | str = None) -> None: from clan_cli.vars.generate import Generator - generators = Generator.generators_from_flake(machine.name, machine.flake, machine) + generators = Generator.generators_from_flake(machine.name, machine.flake) if generator_name: for generator in generators: if generator_name == generator.name: diff --git a/pkgs/clan-cli/clan_cli/vars/generate.py b/pkgs/clan-cli/clan_cli/vars/generate.py index 18b6a37fa..925c97b1a 100644 --- a/pkgs/clan-cli/clan_cli/vars/generate.py +++ b/pkgs/clan-cli/clan_cli/vars/generate.py @@ -49,20 +49,17 @@ class Generator: migrate_fact: str | None = None - # TODO: remove this - _machine: "Machine | None" = None - - def machine(self, machine: "Machine") -> None: - self._machine = machine + machine: str | None = None + _flake: "Flake | None" = None @cached_property def exists(self) -> bool: - assert self._machine is not None - return check_vars(self._machine, generator_name=self.name) + assert self.machine is not None and self._flake is not None + return check_vars(self.machine, self._flake, generator_name=self.name) @classmethod def generators_from_flake( - cls: type["Generator"], machine_name: str, flake: "Flake", machine: "Machine" + cls: type["Generator"], machine_name: str, flake: "Flake" ) -> list["Generator"]: config = nix_config() system = config["system"] @@ -112,19 +109,21 @@ class Generator: dependencies=gen_data["dependencies"], migrate_fact=gen_data.get("migrateFact"), prompts=prompts, + machine=machine_name, + _flake=flake, ) - # Set the machine immediately - generator.machine(machine) generators.append(generator) return generators def final_script(self) -> Path: - assert self._machine is not None + assert self.machine is not None and self._flake is not None + from clan_lib.machines.machines import Machine from clan_lib.nix import nix_test_store + machine = Machine(name=self.machine, flake=self._flake) output = Path( - self._machine.select( + machine.select( f'config.clan.core.vars.generators."{self.name}".finalScript' ) ) @@ -133,8 +132,11 @@ class Generator: return output def validation(self) -> str | None: - assert self._machine is not None - return self._machine.select( + assert self.machine is not None and self._flake is not None + from clan_lib.machines.machines import Machine + + machine = Machine(name=self.machine, flake=self._flake) + return machine.select( f'config.clan.core.vars.generators."{self.name}".validationHash' ) @@ -187,9 +189,7 @@ def decrypt_dependencies( decrypted_dependencies: dict[str, Any] = {} for generator_name in set(generator.dependencies): decrypted_dependencies[generator_name] = {} - generators = Generator.generators_from_flake( - machine.name, machine.flake, machine - ) + generators = Generator.generators_from_flake(machine.name, machine.flake) for dep_generator in generators: if generator_name == dep_generator.name: break @@ -398,9 +398,7 @@ def get_closure( ) -> list[Generator]: from . import graph - vars_generators = Generator.generators_from_flake( - machine.name, machine.flake, machine - ) + vars_generators = Generator.generators_from_flake(machine.name, machine.flake) generators: dict[str, Generator] = { generator.name: generator for generator in vars_generators } @@ -477,7 +475,7 @@ def generate_vars_for_machine( generators_set = set(generators) generators_ = [ g - for g in Generator.generators_from_flake(machine_name, machine.flake, machine) + for g in Generator.generators_from_flake(machine_name, machine.flake) if g.name in generators_set ] @@ -498,9 +496,7 @@ def generate_vars_for_machine_interactive( ) -> bool: _generator = None if generator_name: - generators = Generator.generators_from_flake( - machine.name, machine.flake, machine - ) + generators = Generator.generators_from_flake(machine.name, machine.flake) for generator in generators: if generator.name == generator_name: _generator = generator diff --git a/pkgs/clan-cli/clan_cli/vars/list.py b/pkgs/clan-cli/clan_cli/vars/list.py index 75a655189..c880b6696 100644 --- a/pkgs/clan-cli/clan_cli/vars/list.py +++ b/pkgs/clan-cli/clan_cli/vars/list.py @@ -21,9 +21,7 @@ def get_vars(base_dir: str, machine_name: str) -> list[Var]: from clan_cli.vars.generate import Generator all_vars = [] - for generator in Generator.generators_from_flake( - machine_name, machine.flake, machine - ): + for generator in Generator.generators_from_flake(machine_name, machine.flake): for var in generator.files: if var.secret: var.store(sec_store) @@ -57,7 +55,7 @@ def get_generators(base_dir: str, machine_name: str) -> list[Generator]: machine = Machine(name=machine_name, flake=Flake(base_dir)) generators: list[Generator] = Generator.generators_from_flake( - machine_name, machine.flake, machine + machine_name, machine.flake ) for generator in generators: for prompt in generator.prompts: @@ -76,9 +74,7 @@ def set_prompts( machine = Machine(name=machine_name, flake=Flake(base_dir)) for update in updates: - generators = Generator.generators_from_flake( - machine_name, machine.flake, machine - ) + generators = Generator.generators_from_flake(machine_name, machine.flake) for generator in generators: if generator.name == update.generator: break diff --git a/pkgs/clan-cli/clan_cli/vars/secret_modules/password_store.py b/pkgs/clan-cli/clan_cli/vars/secret_modules/password_store.py index f4c99465d..98060a997 100644 --- a/pkgs/clan-cli/clan_cli/vars/secret_modules/password_store.py +++ b/pkgs/clan-cli/clan_cli/vars/secret_modules/password_store.py @@ -144,7 +144,7 @@ class SecretStore(StoreBase): manifest = [] generators = Generator.generators_from_flake( - self.machine.name, self.machine.flake, self.machine + self.machine.name, self.machine.flake ) for generator in generators: for file in generator.files: @@ -173,7 +173,7 @@ class SecretStore(StoreBase): from clan_cli.vars.generate import Generator vars_generators = Generator.generators_from_flake( - self.machine.name, self.machine.flake, self.machine + self.machine.name, self.machine.flake ) if "users" in phases: with tarfile.open( diff --git a/pkgs/clan-cli/clan_cli/vars/secret_modules/sops.py b/pkgs/clan-cli/clan_cli/vars/secret_modules/sops.py index 64f1f374e..5008d1b56 100644 --- a/pkgs/clan-cli/clan_cli/vars/secret_modules/sops.py +++ b/pkgs/clan-cli/clan_cli/vars/secret_modules/sops.py @@ -55,7 +55,7 @@ class SecretStore(StoreBase): from clan_cli.vars.generate import Generator vars_generators = Generator.generators_from_flake( - self.machine.name, self.machine.flake, self.machine + self.machine.name, self.machine.flake ) if not vars_generators: return @@ -118,7 +118,7 @@ class SecretStore(StoreBase): from clan_cli.vars.generate import Generator generators = Generator.generators_from_flake( - self.machine.name, self.machine.flake, self.machine + self.machine.name, self.machine.flake ) else: generators = [generator] @@ -195,7 +195,7 @@ class SecretStore(StoreBase): from clan_cli.vars.generate import Generator vars_generators = Generator.generators_from_flake( - self.machine.name, self.machine.flake, self.machine + self.machine.name, self.machine.flake ) if "users" in phases or "services" in phases: key_name = f"{self.machine.name}-age.key" @@ -310,7 +310,7 @@ class SecretStore(StoreBase): from clan_cli.vars.generate import Generator generators = Generator.generators_from_flake( - self.machine.name, self.machine.flake, self.machine + self.machine.name, self.machine.flake ) else: generators = [generator]