diff --git a/pkgs/clan-cli/clan_cli/tests/test_vars.py b/pkgs/clan-cli/clan_cli/tests/test_vars.py index 3a5793703..44b392f43 100644 --- a/pkgs/clan-cli/clan_cli/tests/test_vars.py +++ b/pkgs/clan-cli/clan_cli/tests/test_vars.py @@ -1121,6 +1121,89 @@ def test_invalidation( assert value2 == value2_new +@pytest.mark.with_core +def test_share_mode_switch_regenerates_secret( + monkeypatch: pytest.MonkeyPatch, + flake_with_sops: ClanFlake, +) -> None: + """ + Test that switching a generator from share=false to share=true + causes the secret to be regenerated with a new value. + """ + flake = flake_with_sops + + config = flake.machines["my_machine"] + config["nixpkgs"]["hostPlatform"] = "x86_64-linux" + + # Create a generator with share=false initially + my_generator = config["clan"]["core"]["vars"]["generators"]["my_generator"] + my_generator["share"] = False + my_generator["files"]["my_value"]["secret"] = False + my_generator["files"]["my_secret"]["secret"] = True + my_generator["script"] = ( + 'echo -n "public$RANDOM" > "$out"/my_value && echo -n "secret$RANDOM" > "$out"/my_secret' + ) + + flake.refresh() + monkeypatch.chdir(flake.path) + + # Generate with share=false + cli.run(["vars", "generate", "--flake", str(flake.path), "my_machine"]) + + # Read the initial values + flake_obj = Flake(str(flake.path)) + in_repo_store = in_repo.FactStore(flake=flake_obj) + sops_store = sops.SecretStore(flake=flake_obj) + + generator_not_shared = Generator( + "my_generator", share=False, machine="my_machine", _flake=flake_obj + ) + + initial_public = in_repo_store.get(generator_not_shared, "my_value").decode() + initial_secret = sops_store.get(generator_not_shared, "my_secret").decode() + + # Verify initial values exist and have expected format + assert initial_public.startswith("public") + assert initial_secret.startswith("secret") + + # Now switch to share=true + my_generator["share"] = True + flake.refresh() + + # Generate again with share=true + cli.run(["vars", "generate", "--flake", str(flake.path), "my_machine"]) + + # Read the new values with shared generator + generator_shared = Generator( + "my_generator", share=True, machine="my_machine", _flake=flake_obj + ) + + new_public = in_repo_store.get(generator_shared, "my_value").decode() + new_secret = sops_store.get(generator_shared, "my_secret").decode() + + # Verify that both values have changed (regenerated) + assert new_public != initial_public, ( + "Public value should be regenerated when switching to share=true" + ) + assert new_secret != initial_secret, ( + "Secret value should be regenerated when switching to share=true" + ) + + # Verify new values still have expected format + assert new_public.startswith("public") + assert new_secret.startswith("secret") + + # Verify the old machine-specific secret no longer exists + assert not sops_store.exists(generator_not_shared, "my_secret"), ( + "Machine-specific secret should be removed" + ) + + # Verify the new shared secret exists + assert sops_store.exists(generator_shared, "my_secret"), ( + "Shared secret should exist" + ) + + @pytest.mark.with_core def test_dynamic_invalidation( monkeypatch: pytest.MonkeyPatch, diff --git a/pkgs/clan-cli/clan_cli/vars/_types.py b/pkgs/clan-cli/clan_cli/vars/_types.py index 9d9bc7e2b..f0844d2a8 100644 --- a/pkgs/clan-cli/clan_cli/vars/_types.py +++ b/pkgs/clan-cli/clan_cli/vars/_types.py @@ -1,3 +1,4 @@ +import dataclasses import logging from abc import ABC, abstractmethod from collections.abc import Callable, Iterable @@ -139,9 +140,18 @@ class StoreBase(ABC): var: "Var", value: bytes, is_migration: bool = False, - ) -> Path | None: + ) -> list[Path]: from clan_lib.machines.machines import Machine + changed_files: list[Path] = [] + + # if generator was switched from shared to per-machine or vice versa, + # remove the old var first + if self.exists( + gen := dataclasses.replace(generator, share=not generator.share), var.name + ): + changed_files += self.delete(gen, var.name) + if self.exists(generator, var.name): if self.is_secret_store: old_val = None @@ -171,7 +181,8 @@ class StoreBase(ABC): log_info( f"Var {generator.name}/{var.name} remains unchanged: {old_val_str}", ) - return new_file + changed_files += [new_file] if new_file else [] + return changed_files @abstractmethod def delete(self, generator: "Generator", name: str) -> Iterable[Path]: diff --git a/pkgs/clan-cli/clan_cli/vars/generator.py b/pkgs/clan-cli/clan_cli/vars/generator.py index 9d67e8b69..e0298caa3 100644 --- a/pkgs/clan-cli/clan_cli/vars/generator.py +++ b/pkgs/clan-cli/clan_cli/vars/generator.py @@ -410,21 +410,20 @@ class Generator: msg += f" - {f.name}\n" raise ClanError(msg) if file.secret: - file_path = machine.secret_vars_store.set( + file_paths = machine.secret_vars_store.set( self, file, secret_file.read_bytes(), ) secret_changed = True else: - file_path = machine.public_vars_store.set( + file_paths = machine.public_vars_store.set( self, file, secret_file.read_bytes(), ) public_changed = True - if file_path: - files_to_commit.append(file_path) + files_to_commit.extend(file_paths) validation = self.validation() if validation is not None: diff --git a/pkgs/clan-cli/clan_cli/vars/migration.py b/pkgs/clan-cli/clan_cli/vars/migration.py index eff2db679..5057d3cb5 100644 --- a/pkgs/clan-cli/clan_cli/vars/migration.py +++ b/pkgs/clan-cli/clan_cli/vars/migration.py @@ -58,24 +58,16 @@ def _migrate_file( if file.secret: old_value = machine.secret_facts_store.get(service_name, fact_name) - maybe_path = machine.secret_vars_store.set( - generator, - file, - old_value, - is_migration=True, + paths_list = machine.secret_vars_store.set( + generator, file, old_value, is_migration=True ) - if maybe_path: - paths.append(maybe_path) + paths.extend(paths_list) else: old_value = machine.public_facts_store.get(service_name, fact_name) - maybe_path = machine.public_vars_store.set( - generator, - file, - old_value, - is_migration=True, + paths_list = machine.public_vars_store.set( + generator, file, old_value, is_migration=True ) - if maybe_path: - paths.append(maybe_path) + paths.extend(paths_list) return paths diff --git a/pkgs/clan-cli/clan_cli/vars/set.py b/pkgs/clan-cli/clan_cli/vars/set.py index 8f02ac520..04a0d7f85 100644 --- a/pkgs/clan-cli/clan_cli/vars/set.py +++ b/pkgs/clan-cli/clan_cli/vars/set.py @@ -28,10 +28,10 @@ def set_var(machine: str | Machine, var: str | Var, value: bytes, flake: Flake) _var = get_machine_var(_machine, var) else: _var = var - path = _var.set(value) - if path: + paths = _var.set(value) + if paths: commit_files( - [path], + paths, _machine.flake_dir, f"Update var {_var.id} for machine {_machine.name}", ) diff --git a/pkgs/clan-cli/clan_cli/vars/var.py b/pkgs/clan-cli/clan_cli/vars/var.py index c5a7b6835..6651920aa 100644 --- a/pkgs/clan-cli/clan_cli/vars/var.py +++ b/pkgs/clan-cli/clan_cli/vars/var.py @@ -46,7 +46,7 @@ class Var: except UnicodeDecodeError: return "" - def set(self, value: bytes) -> Path | None: + def set(self, value: bytes) -> list[Path]: assert self._store is not None assert self._generator is not None return self._store.set(self._generator, self, value)