secrets: ensure all added/deleted files get committed

checks/installation/flake-module.nix

@@ -63,7 +63,9 @@
             };
           };
           nodes.client = {
-            environment.systemPackages = [ self.packages.${pkgs.system}.clan-cli ];
+            environment.systemPackages = [
+              self.packages.${pkgs.system}.clan-cli
+            ] ++ self.packages.${pkgs.system}.clan-cli.runtimeDependencies;
             environment.etc."install-closure".source = "${closureInfo}/store-paths";
             virtualisation.memorySize = 2048;
             nix.settings = {

pkgs/clan-cli/clan_cli/facts/secret_modules/sops.py

@@ -16,6 +16,7 @@ class SecretStore(SecretStoreBase):
         # no need to generate keys if we don't manage secrets
         if not hasattr(self.machine, "facts_data"):
             return
+
         if not self.machine.facts_data:
             return
 

pkgs/clan-cli/clan_cli/git.py

@@ -29,6 +29,8 @@ def commit_files(
     repo_dir: Path,
     commit_message: str | None = None,
 ) -> None:
+    if not file_paths:
+        return
     # check that the file is in the git repository
     for file_path in file_paths:
         if not Path(file_path).resolve().is_relative_to(repo_dir.resolve()):

pkgs/clan-cli/clan_cli/secrets/folders.py

@@ -33,10 +33,13 @@ def list_objects(path: Path, is_valid: Callable[[str], bool]) -> list[str]:
     return objs
 
 
-def remove_object(path: Path, name: str) -> None:
+def remove_object(path: Path, name: str) -> list[Path]:
+    paths_to_commit = []
     try:
         shutil.rmtree(path / name)
+        paths_to_commit.append(path / name)
     except FileNotFoundError:
         raise ClanError(f"{name} not found in {path}")
     if not os.listdir(path):
         os.rmdir(path)
+    return paths_to_commit

pkgs/clan-cli/clan_cli/secrets/groups.py

@@ -2,6 +2,8 @@ import argparse
 import os
 from pathlib import Path
 
+from clan_cli.git import commit_files
+
 from ..errors import ClanError
 from ..machines.types import machine_name_type, validate_hostname
 from . import secrets
@@ -87,19 +89,21 @@ def list_directory(directory: Path) -> str:
     return msg
 
 
-def update_group_keys(flake_dir: Path, group: str) -> None:
+def update_group_keys(flake_dir: Path, group: str) -> list[Path]:
+    updated_paths = []
     for secret_ in secrets.list_secrets(flake_dir):
         secret = sops_secrets_folder(flake_dir) / secret_
         if (secret / "groups" / group).is_symlink():
-            update_keys(
+            updated_paths += update_keys(
                 secret,
                 list(sorted(secrets.collect_keys_for_path(secret))),
             )
+    return updated_paths
 
 
 def add_member(
     flake_dir: Path, group_folder: Path, source_folder: Path, name: str
-) -> None:
+) -> list[Path]:
     source = source_folder / name
     if not source.exists():
         msg = f"{name} does not exist in {source_folder}: "
@@ -114,7 +118,7 @@ def add_member(
             )
         os.remove(user_target)
     user_target.symlink_to(os.path.relpath(source, user_target.parent))
-    update_group_keys(flake_dir, group_folder.parent.name)
+    return update_group_keys(flake_dir, group_folder.parent.name)
 
 
 def remove_member(flake_dir: Path, group_folder: Path, name: str) -> None:
@@ -136,9 +140,14 @@ def remove_member(flake_dir: Path, group_folder: Path, name: str) -> None:
 
 
 def add_user(flake_dir: Path, group: str, name: str) -> None:
-    add_member(
+    updated_files = add_member(
         flake_dir, users_folder(flake_dir, group), sops_users_folder(flake_dir), name
     )
+    commit_files(
+        updated_files,
+        flake_dir,
+        f"Add user {name} to group {group}",
+    )
 
 
 def add_user_command(args: argparse.Namespace) -> None:
@@ -154,12 +163,17 @@ def remove_user_command(args: argparse.Namespace) -> None:
 
 
 def add_machine(flake_dir: Path, group: str, name: str) -> None:
-    add_member(
+    updated_files = add_member(
         flake_dir,
         machines_folder(flake_dir, group),
         sops_machines_folder(flake_dir),
         name,
     )
+    commit_files(
+        updated_files,
+        flake_dir,
+        f"Add machine {name} to group {group}",
+    )
 
 
 def add_machine_command(args: argparse.Namespace) -> None:
@@ -189,7 +203,14 @@ def add_secret_command(args: argparse.Namespace) -> None:
 
 
 def remove_secret(flake_dir: Path, group: str, name: str) -> None:
-    secrets.disallow_member(secrets.groups_folder(flake_dir, name), group)
+    updated_paths = secrets.disallow_member(
+        secrets.groups_folder(flake_dir, name), group
+    )
+    commit_files(
+        updated_paths,
+        flake_dir,
+        f"Remove group {group} from secret {name}",
+    )
 
 
 def remove_secret_command(args: argparse.Namespace) -> None:

pkgs/clan-cli/clan_cli/secrets/key.py

@@ -1,6 +1,8 @@
 import argparse
 from pathlib import Path
 
+from clan_cli.git import commit_files
+
 from .. import tty
 from ..errors import ClanError
 from .secrets import update_secrets
@@ -11,9 +13,7 @@ def generate_key() -> str:
     path = default_sops_key_path()
     if path.exists():
         raise ClanError(f"Key already exists at {path}")
-    priv_key, pub_key = generate_private_key()
-    path.parent.mkdir(parents=True, exist_ok=True)
-    path.write_text(priv_key)
+    priv_key, pub_key = generate_private_key(out_file=path)
     return pub_key
 
 
@@ -38,7 +38,7 @@ def show_command(args: argparse.Namespace) -> None:
 
 def update_command(args: argparse.Namespace) -> None:
     flake_dir = Path(args.flake)
-    update_secrets(flake_dir)
+    commit_files(update_secrets(flake_dir), flake_dir, "Updated secrets with new keys.")
 
 
 def register_key_parser(parser: argparse.ArgumentParser) -> None:

pkgs/clan-cli/clan_cli/secrets/machines.py

@@ -21,14 +21,19 @@ def add_machine(flake_dir: Path, name: str, key: str, force: bool) -> None:
 
     paths.extend(update_secrets(flake_dir, filter_secrets=filter_machine_secrets))
     commit_files(
-        [path],
+        paths,
         flake_dir,
         f"Add machine {name} to secrets",
     )
 
 
 def remove_machine(flake_dir: Path, name: str) -> None:
-    remove_object(sops_machines_folder(flake_dir), name)
+    removed_paths = remove_object(sops_machines_folder(flake_dir), name)
+    commit_files(
+        removed_paths,
+        flake_dir,
+        f"Remove machine {name}",
+    )
 
 
 def get_machine(flake_dir: Path, name: str) -> str:
@@ -49,20 +54,27 @@ def list_machines(flake_dir: Path) -> list[str]:
 
 
 def add_secret(flake_dir: Path, machine: str, secret: str) -> None:
-    path = secrets.allow_member(
+    paths = secrets.allow_member(
         secrets.machines_folder(flake_dir, secret),
         sops_machines_folder(flake_dir),
         machine,
     )
     commit_files(
-        [path],
+        paths,
         flake_dir,
         f"Add {machine} to secret",
     )
 
 
 def remove_secret(flake_dir: Path, machine: str, secret: str) -> None:
-    secrets.disallow_member(secrets.machines_folder(flake_dir, secret), machine)
+    updated_paths = secrets.disallow_member(
+        secrets.machines_folder(flake_dir, secret), machine
+    )
+    commit_files(
+        updated_paths,
+        flake_dir,
+        f"Remove {machine} from secret {secret}",
+    )
 
 
 def list_command(args: argparse.Namespace) -> None:

pkgs/clan-cli/clan_cli/secrets/secrets.py

@@ -85,7 +85,7 @@ def encrypt_secret(
 
     files_to_commit = []
     for user in add_users:
-        files_to_commit.append(
+        files_to_commit.extend(
             allow_member(
                 users_folder(flake_dir, secret.name),
                 sops_users_folder(flake_dir),
@@ -95,7 +95,7 @@ def encrypt_secret(
         )
 
     for machine in add_machines:
-        files_to_commit.append(
+        files_to_commit.extend(
             allow_member(
                 machines_folder(flake_dir, secret.name),
                 sops_machines_folder(flake_dir),
@@ -105,7 +105,7 @@ def encrypt_secret(
         )
 
     for group in add_groups:
-        files_to_commit.append(
+        files_to_commit.extend(
             allow_member(
                 groups_folder(flake_dir, secret.name),
                 sops_groups_folder(flake_dir),
@@ -118,7 +118,7 @@ def encrypt_secret(
 
     if key.pubkey not in keys:
         keys.add(key.pubkey)
-        files_to_commit.append(
+        files_to_commit.extend(
             allow_member(
                 users_folder(flake_dir, secret.name),
                 sops_users_folder(flake_dir),
@@ -180,7 +180,7 @@ def list_directory(directory: Path) -> str:
 
 def allow_member(
     group_folder: Path, source_folder: Path, name: str, do_update_keys: bool = True
-) -> Path:
+) -> list[Path]:
     source = source_folder / name
     if not source.exists():
         msg = f"Cannot encrypt {group_folder.parent.name} for '{name}' group. '{name}' group does not exist in {source_folder}: "
@@ -196,15 +196,18 @@ def allow_member(
         os.remove(user_target)
 
     user_target.symlink_to(os.path.relpath(source, user_target.parent))
+    changed = [user_target]
     if do_update_keys:
-        update_keys(
-            group_folder.parent,
-            list(sorted(collect_keys_for_path(group_folder.parent))),
+        changed.extend(
+            update_keys(
+                group_folder.parent,
+                list(sorted(collect_keys_for_path(group_folder.parent))),
+            )
         )
-    return user_target
+    return changed
 
 
-def disallow_member(group_folder: Path, name: str) -> None:
+def disallow_member(group_folder: Path, name: str) -> list[Path]:
     target = group_folder / name
     if not target.exists():
         msg = f"{name} does not exist in group in {group_folder}: "
@@ -225,7 +228,7 @@ def disallow_member(group_folder: Path, name: str) -> None:
     if len(os.listdir(group_folder.parent)) == 0:
         os.rmdir(group_folder.parent)
 
-    update_keys(
+    return update_keys(
         target.parent.parent, list(sorted(collect_keys_for_path(group_folder.parent)))
     )
 

pkgs/clan-cli/clan_cli/secrets/sops.py

@@ -34,7 +34,7 @@ def get_public_key(privkey: str) -> str:
     return res.stdout.strip()
 
 
-def generate_private_key() -> tuple[str, str]:
+def generate_private_key(out_file: Path | None = None) -> tuple[str, str]:
     cmd = nix_shell(["nixpkgs#age"], ["age-keygen"])
     try:
         proc = run(cmd)
@@ -50,6 +50,9 @@ def generate_private_key() -> tuple[str, str]:
             raise ClanError("Could not find public key in age-keygen output")
         if not private_key:
             raise ClanError("Could not find private key in age-keygen output")
+        if out_file:
+            out_file.parent.mkdir(parents=True, exist_ok=True)
+            out_file.write_text(res)
         return private_key, pubkey
     except subprocess.CalledProcessError as e:
         raise ClanError("Failed to generate private sops key") from e

pkgs/clan-cli/clan_cli/secrets/users.py

@@ -32,7 +32,12 @@ def add_user(flake_dir: Path, name: str, key: str, force: bool) -> None:
 
 
 def remove_user(flake_dir: Path, name: str) -> None:
-    remove_object(sops_users_folder(flake_dir), name)
+    removed_paths = remove_object(sops_users_folder(flake_dir), name)
+    commit_files(
+        removed_paths,
+        flake_dir,
+        f"Remove user {name}",
+    )
 
 
 def get_user(flake_dir: Path, name: str) -> str:
@@ -52,13 +57,25 @@ def list_users(flake_dir: Path) -> list[str]:
 
 
 def add_secret(flake_dir: Path, user: str, secret: str) -> None:
-    secrets.allow_member(
+    updated_paths = secrets.allow_member(
         secrets.users_folder(flake_dir, secret), sops_users_folder(flake_dir), user
     )
+    commit_files(
+        updated_paths,
+        flake_dir,
+        f"Add {user} to secret",
+    )
 
 
 def remove_secret(flake_dir: Path, user: str, secret: str) -> None:
-    secrets.disallow_member(secrets.users_folder(flake_dir, secret), user)
+    updated_paths = secrets.disallow_member(
+        secrets.users_folder(flake_dir, secret), user
+    )
+    commit_files(
+        updated_paths,
+        flake_dir,
+        f"Remove {user} from secret",
+    )
 
 
 def list_command(args: argparse.Namespace) -> None:

pkgs/clan-cli/default.nix

@@ -127,7 +127,7 @@ python3.pkgs.buildPythonApplication {
   # Define and expose the tests and checks to run in CI
   passthru.tests =
     (lib.mapAttrs' (n: lib.nameValuePair "clan-dep-${n}") runtimeDependenciesAsSet)
-    // rec {
+    // {
       clan-pytest-without-core =
         runCommand "clan-pytest-without-core"
           { nativeBuildInputs = [ pythonWithTestDeps ] ++ testDependencies; }

pkgs/clan-cli/tests/fixtures_flakes.py

@@ -175,6 +175,18 @@ def test_flake(
     monkeypatch: pytest.MonkeyPatch, temporary_home: Path
 ) -> Iterator[FlakeForTest]:
     yield from create_flake(monkeypatch, temporary_home, "test_flake")
+    # check that git diff on ./sops is empty
+    if (temporary_home / "test_flake" / "sops").exists():
+        git_proc = sp.run(
+            ["git", "diff", "--exit-code", "./sops"],
+            cwd=temporary_home / "test_flake",
+            stderr=sp.PIPE,
+        )
+        if git_proc.returncode != 0:
+            log.error(git_proc.stderr.decode())
+            raise Exception(
+                "git diff on ./sops is not empty. This should not happen as all changes should be committed"
+            )
 
 
 @pytest.fixture