From e09658c8172ce472d3c9f7b1fa0330109e8c010b Mon Sep 17 00:00:00 2001 From: DavHau Date: Fri, 19 Apr 2024 22:02:02 +0700 Subject: [PATCH] secrets: ensure all added/deleted files get committed --- checks/installation/flake-module.nix | 4 ++- .../clan_cli/facts/secret_modules/sops.py | 1 + pkgs/clan-cli/clan_cli/git.py | 2 ++ pkgs/clan-cli/clan_cli/secrets/folders.py | 5 ++- pkgs/clan-cli/clan_cli/secrets/groups.py | 35 +++++++++++++++---- pkgs/clan-cli/clan_cli/secrets/key.py | 8 ++--- pkgs/clan-cli/clan_cli/secrets/machines.py | 22 +++++++++--- pkgs/clan-cli/clan_cli/secrets/secrets.py | 25 +++++++------ pkgs/clan-cli/clan_cli/secrets/sops.py | 5 ++- pkgs/clan-cli/clan_cli/secrets/users.py | 23 ++++++++++-- pkgs/clan-cli/default.nix | 2 +- pkgs/clan-cli/tests/fixtures_flakes.py | 12 +++++++ 12 files changed, 110 insertions(+), 34 deletions(-) diff --git a/checks/installation/flake-module.nix b/checks/installation/flake-module.nix index 49e8ac302..b5f87ef12 100644 --- a/checks/installation/flake-module.nix +++ b/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 = { diff --git a/pkgs/clan-cli/clan_cli/facts/secret_modules/sops.py b/pkgs/clan-cli/clan_cli/facts/secret_modules/sops.py index 90ae3422b..9b6733cf6 100644 --- a/pkgs/clan-cli/clan_cli/facts/secret_modules/sops.py +++ b/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 diff --git a/pkgs/clan-cli/clan_cli/git.py b/pkgs/clan-cli/clan_cli/git.py index e8f4323e5..b812004ff 100644 --- a/pkgs/clan-cli/clan_cli/git.py +++ b/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()): diff --git a/pkgs/clan-cli/clan_cli/secrets/folders.py b/pkgs/clan-cli/clan_cli/secrets/folders.py index 0ccc4d46f..11b408d47 100644 --- a/pkgs/clan-cli/clan_cli/secrets/folders.py +++ b/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 diff --git a/pkgs/clan-cli/clan_cli/secrets/groups.py b/pkgs/clan-cli/clan_cli/secrets/groups.py index 9b71a39ba..558919797 100644 --- a/pkgs/clan-cli/clan_cli/secrets/groups.py +++ b/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: diff --git a/pkgs/clan-cli/clan_cli/secrets/key.py b/pkgs/clan-cli/clan_cli/secrets/key.py index c515be42c..8d13714e7 100644 --- a/pkgs/clan-cli/clan_cli/secrets/key.py +++ b/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: diff --git a/pkgs/clan-cli/clan_cli/secrets/machines.py b/pkgs/clan-cli/clan_cli/secrets/machines.py index e5e026b51..f84b89a8a 100644 --- a/pkgs/clan-cli/clan_cli/secrets/machines.py +++ b/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: diff --git a/pkgs/clan-cli/clan_cli/secrets/secrets.py b/pkgs/clan-cli/clan_cli/secrets/secrets.py index 7eace38fa..7ee2e8529 100644 --- a/pkgs/clan-cli/clan_cli/secrets/secrets.py +++ b/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))) ) diff --git a/pkgs/clan-cli/clan_cli/secrets/sops.py b/pkgs/clan-cli/clan_cli/secrets/sops.py index 3ec9d19b3..5fce40bde 100644 --- a/pkgs/clan-cli/clan_cli/secrets/sops.py +++ b/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 diff --git a/pkgs/clan-cli/clan_cli/secrets/users.py b/pkgs/clan-cli/clan_cli/secrets/users.py index 089221ed3..8f08eeb06 100644 --- a/pkgs/clan-cli/clan_cli/secrets/users.py +++ b/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: diff --git a/pkgs/clan-cli/default.nix b/pkgs/clan-cli/default.nix index 5fd929d02..c24521602 100644 --- a/pkgs/clan-cli/default.nix +++ b/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; } diff --git a/pkgs/clan-cli/tests/fixtures_flakes.py b/pkgs/clan-cli/tests/fixtures_flakes.py index 241fc30e5..ee6e7c916 100644 --- a/pkgs/clan-cli/tests/fixtures_flakes.py +++ b/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