clan-cli: do not skip secrets.update_secrets when a group is removed

We need to remove all keys that were in the group from affected secrets.

With this change we now take `group_name` as an argument in
`{add,remove}_member`, which is a little bit more readable than
`group_folder.parent.name`, and helps DRY the code a bit.

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

@@ -1,5 +1,6 @@
 import argparse
 import os
+from collections.abc import Callable
 from pathlib import Path
 
 from clan_cli.completions import (
@@ -103,13 +104,19 @@ def update_group_keys(flake_dir: Path, group: str) -> list[Path]:
 
 
 def add_member(
-    flake_dir: Path, group_folder: Path, source_folder: Path, name: str
+    flake_dir: Path,
+    group_name: str,
+    get_group_folder: Callable[[Path, str], Path],
+    get_source_folder: Callable[[Path], Path],
+    name: str,
 ) -> list[Path]:
+    source_folder = get_source_folder(flake_dir)
     source = source_folder / name
     if not source.exists():
         msg = f"{name} does not exist in {source_folder}: "
         msg += list_directory(source_folder)
         raise ClanError(msg)
+    group_folder = get_group_folder(flake_dir, group_name)
     group_folder.mkdir(parents=True, exist_ok=True)
     user_target = group_folder / name
     if user_target.exists():
@@ -119,11 +126,18 @@ def add_member(
         user_target.unlink()
     user_target.symlink_to(os.path.relpath(source, user_target.parent))
     changed_files = [user_target]
-    changed_files.extend(update_group_keys(flake_dir, group_folder.parent.name))
+    group_name = group_folder.parent.name
+    changed_files.extend(update_group_keys(flake_dir, group_name))
     return changed_files
 
 
-def remove_member(flake_dir: Path, group_folder: Path, name: str) -> list[Path]:
+def remove_member(
+    flake_dir: Path,
+    group_name: str,
+    get_group_folder: Callable[[Path, str], Path],
+    name: str,
+) -> list[Path]:
+    group_folder = get_group_folder(flake_dir, group_name)
     target = group_folder / name
     if not target.exists():
         msg = f"{name} does not exist in group in {group_folder}: "
@@ -132,21 +146,24 @@ def remove_member(flake_dir: Path, group_folder: Path, name: str) -> list[Path]:
     target.unlink()
     updated_files = [target]
 
-    if len(os.listdir(group_folder)) > 0:
-        updated_files.extend(update_group_keys(flake_dir, group_folder.parent.name))
-
     if len(os.listdir(group_folder)) == 0:
         group_folder.rmdir()
 
     if len(os.listdir(group_folder.parent)) == 0:
         group_folder.parent.rmdir()
 
+    updated_files.extend(update_group_keys(flake_dir, group_name))
+
     return updated_files
 
 
 def add_user(flake_dir: Path, group: str, name: str) -> None:
     updated_files = add_member(
-        flake_dir, users_folder(flake_dir, group), sops_users_folder(flake_dir), name
+        flake_dir,
+        group,
+        users_folder,
+        sops_users_folder,
+        name,
     )
     commit_files(
         updated_files,
@@ -160,7 +177,12 @@ def add_user_command(args: argparse.Namespace) -> None:
 
 
 def remove_user(flake_dir: Path, group: str, name: str) -> None:
-    updated_files = remove_member(flake_dir, users_folder(flake_dir, group), name)
+    updated_files = remove_member(
+        flake_dir,
+        group,
+        users_folder,
+        name,
+    )
     commit_files(
         updated_files,
         flake_dir,
@@ -175,8 +197,9 @@ def remove_user_command(args: argparse.Namespace) -> None:
 def add_machine(flake_dir: Path, group: str, name: str) -> None:
     updated_files = add_member(
         flake_dir,
-        machines_folder(flake_dir, group),
-        sops_machines_folder(flake_dir),
+        group,
+        machines_folder,
+        sops_machines_folder,
         name,
     )
     commit_files(
@@ -191,7 +214,12 @@ def add_machine_command(args: argparse.Namespace) -> None:
 
 
 def remove_machine(flake_dir: Path, group: str, name: str) -> None:
-    updated_files = remove_member(flake_dir, machines_folder(flake_dir, group), name)
+    updated_files = remove_member(
+        flake_dir,
+        group,
+        machines_folder,
+        name,
+    )
     commit_files(
         updated_files,
         flake_dir,

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

@@ -69,7 +69,9 @@ def remove_user(flake_dir: Path, name: str) -> None:
             if not (memberships / name).exists():
                 continue
             log.info(f"Removing user {name} from group {group}")
-            updated_paths.extend(groups.remove_member(flake_dir, memberships, name))
+            updated_paths.extend(
+                groups.remove_member(flake_dir, group, groups.users_folder, name)
+            )
     # Remove the user's key:
     updated_paths.extend(remove_object(sops_users_folder(flake_dir), name))
     # Remove the user from any secret where it was used:

pkgs/clan-cli/tests/test_secrets_cli.py

@@ -409,7 +409,7 @@ def test_groups(
     groups = os.listdir(test_flake.path / "sops" / "groups")
     assert len(groups) == 0
 
-    # Check if the symlink to the group was removed our foo test secret:
+    # Check if the symlink to the group was removed from our foo test secret:
     group_symlink = test_flake.path / "sops/secrets/foo/groups/group1"
     err_msg = (
         "Symlink to group1's key in foo secret "