diff --git a/pkgs/clan-cli/clan_cli/flash/flash.py b/pkgs/clan-cli/clan_cli/flash/flash.py index 686cee62d..2c8f19c67 100644 --- a/pkgs/clan-cli/clan_cli/flash/flash.py +++ b/pkgs/clan-cli/clan_cli/flash/flash.py @@ -54,9 +54,7 @@ def flash_machine( extra_args = [] system_config_nix: dict[str, Any] = {} - generate_vars_for_machine( - machine, generator_name=None, regenerate=False, fix=False - ) + generate_vars_for_machine(machine, generator_name=None, regenerate=False) generate_facts([machine]) if system_config.language: diff --git a/pkgs/clan-cli/clan_cli/vars/_types.py b/pkgs/clan-cli/clan_cli/vars/_types.py index fa022a2a3..cabefbf10 100644 --- a/pkgs/clan-cli/clan_cli/vars/_types.py +++ b/pkgs/clan-cli/clan_cli/vars/_types.py @@ -60,6 +60,20 @@ class StoreBase(ABC): def is_secret_store(self) -> bool: pass + def health_check( + self, + generator: "Generator | None" = None, + file_name: str | None = None, + ) -> str | None: + return None + + def fix( + self, + generator: "Generator | None" = None, + file_name: str | None = None, + ) -> None: + return None + def backend_collision_error(self, folder: Path) -> None: msg = ( f"Var folder {folder} exists but doesn't look like a {self.store_name} secret." diff --git a/pkgs/clan-cli/clan_cli/vars/check.py b/pkgs/clan-cli/clan_cli/vars/check.py index 2e93128a4..a60aac489 100644 --- a/pkgs/clan-cli/clan_cli/vars/check.py +++ b/pkgs/clan-cli/clan_cli/vars/check.py @@ -66,8 +66,11 @@ def vars_status(machine: Machine, generator_name: None | str = None) -> VarStatu ) missing_secret_vars.append(file) else: - needs_fix, msg = secret_vars_store.needs_fix(generator, file.name) - if needs_fix: + msg = secret_vars_store.health_check( + generator=generator, + file_name=file.name, + ) + if msg: log.info( f"Secret var '{file.name}' for service '{generator.name}' in machine {machine.name} needs update: {msg}" ) diff --git a/pkgs/clan-cli/clan_cli/vars/cli.py b/pkgs/clan-cli/clan_cli/vars/cli.py index 7177be71f..75d3ff562 100644 --- a/pkgs/clan-cli/clan_cli/vars/cli.py +++ b/pkgs/clan-cli/clan_cli/vars/cli.py @@ -4,6 +4,7 @@ import argparse from clan_cli.hyperlink import help_hyperlink from .check import register_check_parser +from .fix import register_fix_parser from .generate import register_generate_parser from .get import register_get_parser from .keygen import register_keygen_parser @@ -52,6 +53,23 @@ Examples: ) register_check_parser(check_parser) + fix_parser = subparser.add_parser( + "fix", + help="fix inconsistencies in the vars store", + epilog=( + """ +This subcommand allows fixing of inconsistencies in the vars store. + +Examples: + + $ clan vars fix [MACHINE] + Will fix vars for the specified machine. + """ + ), + formatter_class=argparse.RawTextHelpFormatter, + ) + register_fix_parser(fix_parser) + list_parser = subparser.add_parser( "list", help="list all vars", @@ -60,7 +78,7 @@ Examples: This subcommand allows listing all non-secret vars for a specific machine. The resulting list will be a json string with the name of the variable as its key -and the fact itself as it's value. +and the variable itself as it's value. This is how an example output might look like: ``` diff --git a/pkgs/clan-cli/clan_cli/vars/fix.py b/pkgs/clan-cli/clan_cli/vars/fix.py new file mode 100644 index 000000000..f194289f5 --- /dev/null +++ b/pkgs/clan-cli/clan_cli/vars/fix.py @@ -0,0 +1,57 @@ +import argparse +import importlib +import logging + +from clan_cli.completions import add_dynamic_completer, complete_machines +from clan_cli.errors import ClanError +from clan_cli.machines.machines import Machine +from clan_cli.vars.public_modules import FactStoreBase +from clan_cli.vars.secret_modules import SecretStoreBase + +log = logging.getLogger(__name__) + + +def fix_vars(machine: Machine, generator_name: None | str = None) -> None: + secret_vars_module = importlib.import_module(machine.secret_vars_module) + secret_vars_store: SecretStoreBase = secret_vars_module.SecretStore(machine=machine) + public_vars_module = importlib.import_module(machine.public_vars_module) + public_vars_store: FactStoreBase = public_vars_module.FactStore(machine=machine) + + generators = machine.vars_generators + if generator_name: + for generator in generators: + if generator_name == generator.name: + generators = [generator] + break + else: + err_msg = ( + f"Generator '{generator_name}' not found in machine {machine.name}" + ) + raise ClanError(err_msg) + + for generator in generators: + public_vars_store.fix(generator=generator) + secret_vars_store.fix(generator=generator) + + +def fix_command(args: argparse.Namespace) -> None: + machine = Machine( + name=args.machine, + flake=args.flake, + ) + fix_vars(machine, generator_name=args.generator) + + +def register_fix_parser(parser: argparse.ArgumentParser) -> None: + machines_parser = parser.add_argument( + "machine", + help="The machine to fix vars for", + ) + add_dynamic_completer(machines_parser, complete_machines) + + parser.add_argument( + "--generator", + "-g", + help="the generator to check", + ) + parser.set_defaults(func=fix_command) diff --git a/pkgs/clan-cli/clan_cli/vars/generate.py b/pkgs/clan-cli/clan_cli/vars/generate.py index 9afa7d431..8887207ce 100644 --- a/pkgs/clan-cli/clan_cli/vars/generate.py +++ b/pkgs/clan-cli/clan_cli/vars/generate.py @@ -392,58 +392,29 @@ def _check_can_migrate( ) -def ensure_consistent_state( - machine: "Machine", - generator_name: str | None, - fix: bool, -) -> None: - """ - Apply local updates to secrets like re-encrypting with missing keys - when new users were added. - """ - - if generator_name is None: - generators = machine.vars_generators - else: - for generator in machine.vars_generators: - if generator_name == generator.name: - generators = [generator] - break - else: - err_msg = ( - f"Could not find generator {generator_name} in machine {machine.name}" - ) - raise ClanError(err_msg) - outdated = [] - for generator in generators: - for file in generator.files: - if file.secret and machine.secret_vars_store.exists(generator, file.name): - if file.deploy: - machine.secret_vars_store.ensure_machine_has_access( - generator, file.name - ) - needs_update, msg = machine.secret_vars_store.needs_fix( - generator, file.name - ) - if needs_update: - outdated.append((generator_name, file.name, msg)) - if not fix and outdated: - msg = ( - "The local state of some secret vars is inconsistent and needs to be updated.\n" - "Rerun 'clan vars generate' passing '--fix' to apply the necessary changes." - "Problems to fix:\n" - "\n".join(o[2] for o in outdated if o[2]) - ) - raise ClanError(msg) - - def generate_vars_for_machine( machine: "Machine", generator_name: str | None, regenerate: bool, - fix: bool, ) -> bool: - ensure_consistent_state(machine, generator_name, fix) + _generator = None + if generator_name: + for generator in machine.vars_generators: + if generator.name == generator_name: + _generator = generator + break + + pub_healtcheck_msg = machine.public_vars_store.health_check(_generator) + sec_healtcheck_msg = machine.secret_vars_store.health_check(_generator) + + if pub_healtcheck_msg or sec_healtcheck_msg: + msg = f"Health check failed for machine {machine.name}:\n" + if pub_healtcheck_msg: + msg += f"Public vars store: {pub_healtcheck_msg}\n" + if sec_healtcheck_msg: + msg += f"Secret vars store: {sec_healtcheck_msg}" + raise ClanError(msg) + closure = get_closure(machine, generator_name, regenerate) if len(closure) == 0: return False @@ -467,14 +438,13 @@ def generate_vars( machines: list["Machine"], generator_name: str | None = None, regenerate: bool = False, - fix: bool = False, ) -> bool: was_regenerated = False for machine in machines: errors = [] try: was_regenerated |= generate_vars_for_machine( - machine, generator_name, regenerate, fix + machine, generator_name, regenerate ) machine.flush_caches() except Exception as exc: @@ -502,7 +472,7 @@ def generate_command(args: argparse.Namespace) -> None: machines = get_all_machines(args.flake, args.option) else: machines = get_selected_machines(args.flake, args.option, args.machines) - generate_vars(machines, args.generator, args.regenerate, args.fix) + generate_vars(machines, args.generator, args.regenerate) def register_generate_parser(parser: argparse.ArgumentParser) -> None: @@ -532,11 +502,4 @@ def register_generate_parser(parser: argparse.ArgumentParser) -> None: default=None, ) - parser.add_argument( - "--fix", - action=argparse.BooleanOptionalAction, - help="whether to fix local state inconsistencies, for example if a secret is not encrypted with the correct keys", - default=False, - ) - parser.set_defaults(func=generate_command) diff --git a/pkgs/clan-cli/clan_cli/vars/secret_modules/__init__.py b/pkgs/clan-cli/clan_cli/vars/secret_modules/__init__.py index e1d74eaf3..651d82a1b 100644 --- a/pkgs/clan-cli/clan_cli/vars/secret_modules/__init__.py +++ b/pkgs/clan-cli/clan_cli/vars/secret_modules/__init__.py @@ -5,7 +5,7 @@ from typing import TYPE_CHECKING from clan_cli.vars._types import StoreBase if TYPE_CHECKING: - from clan_cli.vars.generate import Generator + pass class SecretStoreBase(StoreBase): @@ -13,28 +13,6 @@ class SecretStoreBase(StoreBase): def is_secret_store(self) -> bool: return True - def ensure_machine_has_access(self, generator: "Generator", name: str) -> None: - pass - - def needs_fix( - self, - generator: "Generator", - name: str, - ) -> tuple[bool, str | None]: - """ - Check if local state needs updating, eg. secret needs to be re-encrypted with new keys - """ - return False, None - - def fix( - self, - generator: "Generator", - name: str, - ) -> None: - """ - Update local state, eg make sure secret is encrypted with correct keys - """ - @abstractmethod def populate_dir(self, output_dir: Path) -> None: pass 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 018ac2bb5..99c641305 100644 --- a/pkgs/clan-cli/clan_cli/vars/secret_modules/sops.py +++ b/pkgs/clan-cli/clan_cli/vars/secret_modules/sops.py @@ -87,6 +87,48 @@ class SecretStore(SecretStoreBase): def secret_path(self, generator: Generator, secret_name: str) -> Path: return self.directory(generator, secret_name) + @override + def health_check( + self, generator: Generator | None = None, file_name: str | None = None + ) -> str | None: + """ + Apply local updates to secrets like re-encrypting with missing keys + when new users were added. + """ + + if generator is None: + generators = self.machine.vars_generators + else: + generators = [generator] + file_found = False + outdated = [] + for generator in generators: + for file in generator.files: + # if we check only a single file, continue on all the other ones + if file_name: + if file.name == file_name: + file_found = True + else: + continue + if file.secret and self.exists(generator, file.name): + if file.deploy: + self.ensure_machine_has_access(generator, file.name) + needs_update, msg = self.needs_fix(generator, file.name) + if needs_update: + outdated.append((generator.name, file.name, msg)) + if file_name and not file_found: + msg = f"file {file_name} was not found" + raise ClanError(msg) + if outdated: + msg = ( + "The local state of some secret vars is inconsistent and needs to be updated.\n" + "Run 'clan vars fix' to apply the necessary changes." + "Problems to fix:\n" + "\n".join(o[2] for o in outdated if o[2]) + ) + return msg + return None + def _set( self, generator: Generator, @@ -183,11 +225,30 @@ class SecretStore(SecretStoreBase): return needs_update, msg @override - def fix(self, generator: Generator, name: str) -> None: + def fix( + self, generator: Generator | None = None, file_name: str | None = None + ) -> None: from clan_cli.secrets.secrets import update_keys - secret_path = self.secret_path(generator, name) - update_keys( - secret_path, - collect_keys_for_path(secret_path), - ) + if generator is None: + generators = self.machine.vars_generators + else: + generators = [generator] + file_found = False + for generator in generators: + for file in generator.files: + # if we check only a single file, continue on all the other ones + if file_name: + if file.name == file_name: + file_found = True + else: + continue + + secret_path = self.secret_path(generator, file.name) + update_keys( + secret_path, + collect_keys_for_path(secret_path), + ) + if file_name and not file_found: + msg = f"file {file_name} was not found" + raise ClanError(msg) diff --git a/pkgs/clan-cli/tests/test_vars.py b/pkgs/clan-cli/tests/test_vars.py index 98d973505..38de3435d 100644 --- a/pkgs/clan-cli/tests/test_vars.py +++ b/pkgs/clan-cli/tests/test_vars.py @@ -211,7 +211,7 @@ def test_generate_secret_var_sops_with_default_group( with pytest.raises(ClanError): cli.run(["vars", "generate", "--flake", str(flake.path), "my_machine"]) # apply fix - cli.run(["vars", "generate", "--flake", str(flake.path), "my_machine", "--fix"]) + cli.run(["vars", "fix", "--flake", str(flake.path), "my_machine"]) # check if new user can access the secret monkeypatch.setenv("USER", "uschi") assert sops_store.user_has_access( @@ -765,7 +765,6 @@ def test_stdout_of_generate( Machine(name="my_machine", flake=FlakeId(str(flake.path))), "my_generator", regenerate=False, - fix=False, ) assert "Updated var my_generator/my_value" in caplog.text @@ -779,7 +778,6 @@ def test_stdout_of_generate( Machine(name="my_machine", flake=FlakeId(str(flake.path))), "my_generator", regenerate=True, - fix=False, ) assert "Updated var my_generator/my_value" in caplog.text assert "old: world" in caplog.text @@ -791,7 +789,6 @@ def test_stdout_of_generate( Machine(name="my_machine", flake=FlakeId(str(flake.path))), "my_generator", regenerate=True, - fix=False, ) assert "Updated" not in caplog.text assert "hello" in caplog.text @@ -801,7 +798,6 @@ def test_stdout_of_generate( Machine(name="my_machine", flake=FlakeId(str(flake.path))), "my_secret_generator", regenerate=False, - fix=False, ) assert "Updated secret var my_secret_generator/my_secret" in caplog.text assert "hello" not in caplog.text @@ -817,7 +813,6 @@ def test_stdout_of_generate( Machine(name="my_machine", flake=FlakeId(str(flake.path))), "my_secret_generator", regenerate=True, - fix=False, ) assert "Updated secret var my_secret_generator/my_secret" in caplog.text assert "world" not in caplog.text @@ -919,7 +914,6 @@ def test_fails_when_files_are_left_from_other_backend( Machine(name="my_machine", flake=FlakeId(str(flake.path))), generator, regenerate=False, - fix=False, ) my_secret_generator["files"]["my_secret"]["secret"] = False my_value_generator["files"]["my_value"]["secret"] = True @@ -931,7 +925,6 @@ def test_fails_when_files_are_left_from_other_backend( Machine(name="my_machine", flake=FlakeId(str(flake.path))), generator, regenerate=False, - fix=False, )