From 37869839cfb4d3c3120ca54a4723d09d963e5d68 Mon Sep 17 00:00:00 2001
From: Louis Opter <louis@opter.org>
Date: Mon, 10 Mar 2025 22:10:09 +0000
Subject: [PATCH 1/8] clan-cli: `tests/age_keys.py` add notes, move function to
 check sops recipients

This supports the new integration test for `clan machines delete`.
---
 pkgs/clan-cli/tests/age_keys.py         | 51 +++++++++++++++++++++----
 pkgs/clan-cli/tests/test_secrets_cli.py | 39 ++++---------------
 2 files changed, 52 insertions(+), 38 deletions(-)

diff --git a/pkgs/clan-cli/tests/age_keys.py b/pkgs/clan-cli/tests/age_keys.py
index 41c583b4a..7c356620f 100644
--- a/pkgs/clan-cli/tests/age_keys.py
+++ b/pkgs/clan-cli/tests/age_keys.py
@@ -1,7 +1,9 @@
+import json
 import os
 from pathlib import Path
 
 import pytest
+from clan_cli.secrets.folders import sops_secrets_folder
 from helpers import cli
 
 
@@ -12,13 +14,26 @@ class KeyPair:
 
 
 class SopsSetup:
+    """Hold a list of three key pairs and create an "admin" user in the clan.
+
+    The first key in the list is used as the admin key and
+    the private part of the key is exposed in the
+    `SOPS_AGE_KEY` environment variable, the two others can
+    be used to add machines or other users.
+    """
+
     def __init__(self, keys: list[KeyPair]) -> None:
         self.keys = keys
+        self.user = os.environ.get("USER", "admin")
 
+    # louis@(2025-03-10): It is odd to have to call an init function on a
+    # fixture: the fixture should already be initialized when it is received in
+    # the test function. Maybe we can arrange for the `flake` fixtures, to take
+    # the `sops_setup` fixture as input and call its `init` function on the
+    # correct path.
     def init(self, flake_path: Path | None = None) -> None:
         if flake_path is None:
             flake_path = Path.cwd()
-        self.user = os.environ.get("USER", "user")
         cli.run(
             [
                 "vars",
@@ -49,9 +64,6 @@ KEYS = [
 
 @pytest.fixture
 def age_keys() -> list[KeyPair]:
-    """
-    Root directory of the tests
-    """
     return KEYS
 
 
@@ -59,8 +71,33 @@ def age_keys() -> list[KeyPair]:
 def sops_setup(
     monkeypatch: pytest.MonkeyPatch,
 ) -> SopsSetup:
-    """
-    Root directory of the tests
-    """
     monkeypatch.setenv("SOPS_AGE_KEY", KEYS[0].privkey)
     return SopsSetup(KEYS)
+
+
+# louis@(2025-03-10): right now this is specific to the `sops/secrets` folder,
+# but we could make it generic to any sops file if the need arises.
+def assert_secrets_file_recipients(
+    flake_path: Path,
+    secret_name: str,
+    expected_age_recipients_keypairs: list["KeyPair"],
+    err_msg: str | None = None,
+) -> None:
+    """Checks that the recipients of a secrets file matches expectations.
+
+    This looks up the `secret` file for `secret_name` in the `sops` directory
+    under `flake_path`.
+
+    :param err_msg: in case of failure, if you gave an error message then it
+       will be displayed, otherwise pytest will display the two different sets
+       of recipients.
+    """
+    sops_file = sops_secrets_folder(flake_path) / secret_name / "secret"
+    with sops_file.open("rb") as fp:
+        sops_data = json.load(fp)
+    age_recipients = {each["recipient"] for each in sops_data["sops"]["age"]}
+    expected_age_recipients = {pair.pubkey for pair in expected_age_recipients_keypairs}
+    if not err_msg:
+        assert age_recipients == expected_age_recipients
+        return
+    assert age_recipients == expected_age_recipients, err_msg
diff --git a/pkgs/clan-cli/tests/test_secrets_cli.py b/pkgs/clan-cli/tests/test_secrets_cli.py
index 3b5d6bdaa..b9cafb210 100644
--- a/pkgs/clan-cli/tests/test_secrets_cli.py
+++ b/pkgs/clan-cli/tests/test_secrets_cli.py
@@ -10,8 +10,8 @@ from pathlib import Path
 from typing import TYPE_CHECKING
 
 import pytest
+from age_keys import assert_secrets_file_recipients
 from clan_cli.errors import ClanError
-from clan_cli.secrets.folders import sops_secrets_folder
 from fixtures_flakes import FlakeForTest
 from helpers import cli
 from stdout import CaptureOutput
@@ -91,7 +91,7 @@ def _test_identities(
                 test_secret_name,
             ]
         )
-    assert_sops_file_recipients(
+    assert_secrets_file_recipients(
         test_flake.path,
         test_secret_name,
         expected_age_recipients_keypairs=[age_keys[0], admin_age_key],
@@ -111,7 +111,7 @@ def _test_identities(
                 age_keys[1].privkey,
             ]
         )
-    assert_sops_file_recipients(
+    assert_secrets_file_recipients(
         test_flake.path,
         test_secret_name,
         expected_age_recipients_keypairs=[age_keys[1], admin_age_key],
@@ -302,7 +302,7 @@ def test_groups(
             ]
         )
 
-    assert_sops_file_recipients(
+    assert_secrets_file_recipients(
         test_flake.path,
         secret_name,
         expected_age_recipients_keypairs=[
@@ -327,7 +327,7 @@ def test_groups(
             "user1",
         ]
     )
-    assert_sops_file_recipients(
+    assert_secrets_file_recipients(
         test_flake.path,
         secret_name,
         expected_age_recipients_keypairs=[machine1_age_key, admin_age_key],
@@ -349,7 +349,7 @@ def test_groups(
             "user1",
         ]
     )
-    assert_sops_file_recipients(
+    assert_secrets_file_recipients(
         test_flake.path,
         secret_name,
         expected_age_recipients_keypairs=[
@@ -370,7 +370,7 @@ def test_groups(
             "user1",
         ]
     )
-    assert_sops_file_recipients(
+    assert_secrets_file_recipients(
         test_flake.path,
         secret_name,
         expected_age_recipients_keypairs=[machine1_age_key, admin_age_key],
@@ -391,7 +391,7 @@ def test_groups(
             "machine1",
         ]
     )
-    assert_sops_file_recipients(
+    assert_secrets_file_recipients(
         test_flake.path,
         secret_name,
         expected_age_recipients_keypairs=[admin_age_key],
@@ -413,29 +413,6 @@ def test_groups(
     assert not group_symlink.exists(follow_symlinks=False), err_msg
 
 
-def assert_sops_file_recipients(
-    flake_path: Path,
-    secret_name: str,
-    expected_age_recipients_keypairs: list["KeyPair"],
-    err_msg: str | None = None,
-) -> None:
-    """Checks that the recipients of a SOPS file matches expectations.
-
-    :param err_msg: in case of failure, if you gave an error message then it
-       will be displayed, otherwise pytest will display the two different sets
-       of recipients.
-    """
-    sops_file = sops_secrets_folder(flake_path) / secret_name / "secret"
-    with sops_file.open("rb") as fp:
-        sops_data = json.load(fp)
-    age_recipients = {each["recipient"] for each in sops_data["sops"]["age"]}
-    expected_age_recipients = {pair.pubkey for pair in expected_age_recipients_keypairs}
-    if not err_msg:
-        assert age_recipients == expected_age_recipients
-        return
-    assert age_recipients == expected_age_recipients, err_msg
-
-
 @contextmanager
 def use_age_key(key: str, monkeypatch: pytest.MonkeyPatch) -> Iterator[None]:
     old_key = os.environ["SOPS_AGE_KEY_FILE"]

From 77b53a366eae939c46bad24bf2118139dc657b64 Mon Sep 17 00:00:00 2001
From: Louis Opter <louis@opter.org>
Date: Mon, 10 Mar 2025 22:13:02 +0000
Subject: [PATCH 2/8] clan-cli: add an integration test for `clan machines
 delete`

This tests the changes made to that command to clean-up vars and secrets
when a machine is deleted.
---
 pkgs/clan-cli/tests/test_machines_cli.py | 86 +++++++++++++++++++++++-
 1 file changed, 84 insertions(+), 2 deletions(-)

diff --git a/pkgs/clan-cli/tests/test_machines_cli.py b/pkgs/clan-cli/tests/test_machines_cli.py
index 9b31e5438..9e5977c1d 100644
--- a/pkgs/clan-cli/tests/test_machines_cli.py
+++ b/pkgs/clan-cli/tests/test_machines_cli.py
@@ -1,13 +1,15 @@
+import fixtures_flakes
 import pytest
+from age_keys import SopsSetup, assert_secrets_file_recipients
 from clan_cli.inventory import load_inventory_json
-from fixtures_flakes import FlakeForTest
+from clan_cli.secrets.folders import sops_machines_folder
 from helpers import cli
 from stdout import CaptureOutput
 
 
 @pytest.mark.impure
 def test_machine_subcommands(
-    test_flake_with_core: FlakeForTest,
+    test_flake_with_core: fixtures_flakes.FlakeForTest,
     capture_output: CaptureOutput,
 ) -> None:
     cli.run(
@@ -47,3 +49,83 @@ def test_machine_subcommands(
     assert "machine1" not in output.out
     assert "vm1" in output.out
     assert "vm2" in output.out
+
+
+# louis(2025-03-09):
+#
+# The `with_core` mark is cargo-culted from
+# `test_generate_public_and_secret_vars` which
+# I used as a starting point:
+@pytest.mark.with_core
+def test_machine_delete(
+    monkeypatch: pytest.MonkeyPatch,
+    flake: fixtures_flakes.ClanFlake,
+    sops_setup: SopsSetup,
+) -> None:
+    # create the admin user and set its key:
+    sops_setup.init(flake.path)
+
+    # admin_key, machine_key, machine2_key = age_keys
+    admin_key, machine_key, machine2_key = sops_setup.keys
+
+    # create a couple machines with their keys
+    for name, key in (("my-machine", machine_key), ("my-machine2", machine2_key)):
+        cli.run(["machines", "create", f"--flake={flake.path}", name])
+        add_machine_key = [
+            "secrets",
+            "machines",
+            "add",
+            f"--flake={flake.path}",
+            name,
+            key.pubkey,
+        ]
+        cli.run(add_machine_key)
+
+    # create a secret shared by both machines
+    shared_secret_name = "shared_secret"
+    with monkeypatch.context():
+        monkeypatch.setenv("SOPS_NIX_SECRET", "secret_value")
+        set_shared_secret = [
+            "secrets",
+            "set",
+            f"--flake={flake.path}",
+            "--machine=my-machine",
+            "--machine=my-machine2",
+            shared_secret_name,
+        ]
+        cli.run(set_shared_secret)
+
+    my_machine_sops_folder = sops_machines_folder(flake.path) / "my-machine"
+    assert my_machine_sops_folder.is_dir(), (
+        "A sops folder for `my-machine` should have been created with its public key"
+    )
+
+    # define some vars generator for `my-machine`:
+    config = flake.machines["my-machine"]
+    config["nixpkgs"]["hostPlatform"] = "x86_64-linux"
+    my_generator = config["clan"]["core"]["vars"]["generators"]["my_generator"]
+    my_generator["files"]["my_value"]["secret"] = False
+    my_generator["files"]["my_secret"]["secret"] = True
+    my_generator["script"] = (
+        "echo -n public > $out/my_value;"
+        "echo -n secret > $out/my_secret;"
+        "echo -n non-default > $out/value_with_default"
+    )
+    flake.refresh()  # saves "my_generator"
+    monkeypatch.chdir(flake.path)
+
+    cli.run(["vars", "generate", "--flake", str(flake.path), "my-machine"])
+    my_machine_vars_store = flake.path / "vars/per-machine" / "my-machine"
+    assert my_machine_vars_store.is_dir(), (
+        "A vars directory should have been created for `my-machine`"
+    )
+
+    cli.run(["machines", "delete", "--flake", str(flake.path), "my-machine"])
+    assert not my_machine_vars_store.exists(), (
+        "The vars directory for `my-machine` should have been deleted"
+    )
+    assert not my_machine_sops_folder.exists(), (
+        "The sops folder holding the public key for `my-machine` should have been deleted"
+    )
+    expected_recipients = [admin_key, machine2_key]
+    assert_secrets_file_recipients(flake.path, shared_secret_name, expected_recipients)

From f7bec766bc7a53b9b7ef3ae4c8b75320079625f6 Mon Sep 17 00:00:00 2001
From: Louis Opter <louis@opter.org>
Date: Fri, 7 Feb 2025 11:20:51 +0000
Subject: [PATCH 3/8] clan-cli: add `delete` and `delete_store` to `StoreBase`

- `delete` lets you delete a specific var under a specific generator;
- `delete_store` deletes an entire store.

The `delete` method could be useful to "garbage-collect" unused vars as
a machine's configuration changes.

The `delete_store` method can be used to delete all the vars for a
machine when the machine is deleted. The current behavior is to leave
everything behind.

Important point:

- `delete_store` needs to be idempotent because public and
  "private"/"secret" vars for a machine can share the same physical
  store (directory), and deleting either type of store (public or
  private) will delete both.
---
 pkgs/clan-cli/clan_cli/vars/_types.py         | 24 +++++++
 .../clan_cli/vars/public_modules/in_repo.py   | 24 ++++++-
 .../clan_cli/vars/public_modules/vm.py        | 17 +++++
 .../vars/secret_modules/password_store.py     | 66 +++++++++----------
 .../clan_cli/vars/secret_modules/sops.py      | 15 +++++
 .../clan_cli/vars/secret_modules/vm.py        | 16 +++++
 6 files changed, 126 insertions(+), 36 deletions(-)

diff --git a/pkgs/clan-cli/clan_cli/vars/_types.py b/pkgs/clan-cli/clan_cli/vars/_types.py
index 7269cd625..b058afe6c 100644
--- a/pkgs/clan-cli/clan_cli/vars/_types.py
+++ b/pkgs/clan-cli/clan_cli/vars/_types.py
@@ -1,5 +1,6 @@
 import logging
 from abc import ABC, abstractmethod
+from collections.abc import Iterable
 from dataclasses import dataclass
 from pathlib import Path
 from typing import TYPE_CHECKING
@@ -122,6 +123,29 @@ class StoreBase(ABC):
                 )
         return new_file
 
+    @abstractmethod
+    def delete(self, generator: "Generator", name: str) -> Iterable[Path]:
+        """Remove a var from the store.
+
+        :return: An iterable of affected paths in the git repository. This
+          may be empty if the store is outside of the repository.
+        """
+
+    @abstractmethod
+    def delete_store(self) -> Iterable[Path]:
+        """Delete the store (all vars) for this machine.
+
+        .. note::
+
+           This does not make the distinction between public and private vars.
+           Since the public and private store of a machine can be co-located
+           under the same directory, this method's implementation has to be
+           idempotent.
+
+        :return: An iterable of affected paths in the git repository. This
+          may be empty if the store was outside of the repository.
+        """
+
     def get_validation(self, generator: "Generator") -> str | None:
         """
         Return the invalidation hash that indicates if a generator needs to be re-run
diff --git a/pkgs/clan-cli/clan_cli/vars/public_modules/in_repo.py b/pkgs/clan-cli/clan_cli/vars/public_modules/in_repo.py
index 51219635a..566d84dec 100644
--- a/pkgs/clan-cli/clan_cli/vars/public_modules/in_repo.py
+++ b/pkgs/clan-cli/clan_cli/vars/public_modules/in_repo.py
@@ -1,4 +1,5 @@
 import shutil
+from collections.abc import Iterable
 from pathlib import Path
 
 from clan_cli.errors import ClanError
@@ -30,15 +31,14 @@ class FactStore(StoreBase):
             msg = f"in_flake fact storage is only supported for local flakes: {self.machine.flake}"
             raise ClanError(msg)
         folder = self.directory(generator, var.name)
+        file_path = folder / "value"
         if folder.exists():
-            if not (folder / "value").exists():
+            if not file_path.exists():
                 # another backend has used that folder before -> error out
                 self.backend_collision_error(folder)
             shutil.rmtree(folder)
         # re-create directory
-        file_path = folder / "value"
         folder.mkdir(parents=True, exist_ok=True)
-        file_path.parent.mkdir(parents=True, exist_ok=True)
         file_path.touch()
         file_path.write_bytes(value)
         return file_path
@@ -50,6 +50,24 @@ class FactStore(StoreBase):
     def exists(self, generator: Generator, name: str) -> bool:
         return (self.directory(generator, name) / "value").exists()
 
+    def delete(self, generator: Generator, name: str) -> Iterable[Path]:
+        fact_folder = self.directory(generator, name)
+        fact_file = fact_folder / "value"
+        fact_file.unlink()
+        empty = None
+        if next(fact_folder.iterdir(), empty) is not empty:
+            return [fact_file]
+        fact_folder.rmdir()
+        return [fact_folder]
+
+    def delete_store(self) -> Iterable[Path]:
+        flake_root = Path(self.machine.flake_dir)
+        store_folder = flake_root / "vars/per-machine" / self.machine.name
+        if not store_folder.exists():
+            return []
+        shutil.rmtree(store_folder)
+        return [store_folder]
+
     def populate_dir(self, output_dir: Path, phases: list[str]) -> None:
         msg = "populate_dir is not implemented for public vars stores"
         raise NotImplementedError(msg)
diff --git a/pkgs/clan-cli/clan_cli/vars/public_modules/vm.py b/pkgs/clan-cli/clan_cli/vars/public_modules/vm.py
index 315a4e9b6..a2719c8fb 100644
--- a/pkgs/clan-cli/clan_cli/vars/public_modules/vm.py
+++ b/pkgs/clan-cli/clan_cli/vars/public_modules/vm.py
@@ -1,4 +1,6 @@
 import logging
+import shutil
+from collections.abc import Iterable
 from pathlib import Path
 
 from clan_cli.dirs import vm_state_dir
@@ -48,6 +50,21 @@ class FactStore(StoreBase):
         msg = f"Fact {name} for service {generator.name} not found"
         raise ClanError(msg)
 
+    def delete(self, generator: Generator, name: str) -> Iterable[Path]:
+        fact_dir = self.dir / generator.name
+        fact_file = fact_dir / name
+        fact_file.unlink()
+        empty = None
+        if next(fact_dir.iterdir(), empty) is empty:
+            fact_dir.rmdir()
+        return [fact_file]
+
+    def delete_store(self) -> Iterable[Path]:
+        if not self.dir.exists():
+            return []
+        shutil.rmtree(self.dir)
+        return [self.dir]
+
     def populate_dir(self, output_dir: Path, phases: list[str]) -> None:
         msg = "populate_dir is not implemented for public vars stores"
         raise NotImplementedError(msg)
diff --git a/pkgs/clan-cli/clan_cli/vars/secret_modules/password_store.py b/pkgs/clan-cli/clan_cli/vars/secret_modules/password_store.py
index 61e4e699f..c82b59fa4 100644
--- a/pkgs/clan-cli/clan_cli/vars/secret_modules/password_store.py
+++ b/pkgs/clan-cli/clan_cli/vars/secret_modules/password_store.py
@@ -2,11 +2,12 @@ import io
 import logging
 import os
 import tarfile
+from collections.abc import Iterable
 from itertools import chain
 from pathlib import Path
 from tempfile import TemporaryDirectory
 
-from clan_cli.cmd import Log, RunOpts, run
+from clan_cli.cmd import CmdOut, Log, RunOpts, run
 from clan_cli.machines.machines import Machine
 from clan_cli.nix import nix_shell
 from clan_cli.ssh.upload import upload
@@ -35,52 +36,51 @@ class SecretStore(StoreBase):
         return backend
 
     @property
-    def _password_store_dir(self) -> str:
+    def _password_store_dir(self) -> Path:
         if self._store_backend == "passage":
-            return os.environ.get("PASSAGE_DIR", f"{os.environ['HOME']}/.passage/store")
-        return os.environ.get(
-            "PASSWORD_STORE_DIR", f"{os.environ['HOME']}/.password-store"
-        )
+            lookup = os.environ.get("PASSAGE_DIR")
+            default = Path.home() / ".passage/store"
+        else:
+            lookup = os.environ.get("PASSWORD_STORE_DIR")
+            default = Path.home() / ".password-store"
+        return Path(lookup) if lookup else default
 
     def entry_dir(self, generator: Generator, name: str) -> Path:
         return Path(self.entry_prefix) / self.rel_dir(generator, name)
 
+    def _run_pass(self, *args: str, options: RunOpts | None = None) -> CmdOut:
+        cmd = nix_shell(packages=["nixpkgs#pass"], cmd=[self._store_backend, *args])
+        return run(cmd, options)
+
     def _set(
         self,
         generator: Generator,
         var: Var,
         value: bytes,
     ) -> Path | None:
-        run(
-            nix_shell(
-                [f"nixpkgs#{self._store_backend}"],
-                [
-                    f"{self._store_backend}",
-                    "insert",
-                    "-m",
-                    str(self.entry_dir(generator, var.name)),
-                ],
-            ),
-            RunOpts(input=value, check=True),
-        )
+        pass_call = ["insert", "-m", str(self.entry_dir(generator, var.name))]
+        self._run_pass(*pass_call, options=RunOpts(input=value, check=True))
         return None  # we manage the files outside of the git repo
 
     def get(self, generator: Generator, name: str) -> bytes:
-        return run(
-            nix_shell(
-                [f"nixpkgs#{self._store_backend}"],
-                [
-                    f"{self._store_backend}",
-                    "show",
-                    str(self.entry_dir(generator, name)),
-                ],
-            ),
-        ).stdout.encode()
+        pass_name = str(self.entry_dir(generator, name))
+        return self._run_pass("show", pass_name).stdout.encode()
 
     def exists(self, generator: Generator, name: str) -> bool:
         extension = "age" if self._store_backend == "passage" else "gpg"
         filename = f"{self.entry_dir(generator, name)}.{extension}"
-        return (Path(self._password_store_dir) / filename).exists()
+        return (self._password_store_dir / filename).exists()
+
+    def delete(self, generator: Generator, name: str) -> Iterable[Path]:
+        pass_name = str(self.entry_dir(generator, name))
+        self._run_pass("rm", "--force", pass_name, options=RunOpts(check=True))
+        return []
+
+    def delete_store(self) -> Iterable[Path]:
+        machine_pass_dir = Path(self.entry_prefix) / "per-machine" / self.machine.name
+        pass_call = ["rm", "--force", "--recursive", str(machine_pass_dir)]
+        self._run_pass(*pass_call, options=RunOpts(check=True))
+        return []
 
     def generate_hash(self) -> bytes:
         hashes = []
@@ -91,7 +91,7 @@ class SecretStore(StoreBase):
                     [
                         "git",
                         "-C",
-                        self._password_store_dir,
+                        str(self._password_store_dir),
                         "log",
                         "-1",
                         "--format=%H",
@@ -103,9 +103,9 @@ class SecretStore(StoreBase):
             .stdout.strip()
             .encode()
         )
-        shared_dir = Path(self._password_store_dir) / self.entry_prefix / "shared"
+        shared_dir = self._password_store_dir / self.entry_prefix / "shared"
         machine_dir = (
-            Path(self._password_store_dir)
+            self._password_store_dir
             / self.entry_prefix
             / "per-machine"
             / self.machine.name
@@ -119,7 +119,7 @@ class SecretStore(StoreBase):
                             [
                                 "git",
                                 "-C",
-                                self._password_store_dir,
+                                str(self._password_store_dir),
                                 "log",
                                 "-1",
                                 "--format=%H",
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 f6dd1f437..3ef05ce2b 100644
--- a/pkgs/clan-cli/clan_cli/vars/secret_modules/sops.py
+++ b/pkgs/clan-cli/clan_cli/vars/secret_modules/sops.py
@@ -1,3 +1,5 @@
+import shutil
+from collections.abc import Iterable
 from dataclasses import dataclass
 from pathlib import Path
 from tempfile import TemporaryDirectory
@@ -162,6 +164,19 @@ class SecretStore(StoreBase):
             self.machine.flake_dir, self.secret_path(generator, name)
         ).encode("utf-8")
 
+    def delete(self, generator: "Generator", name: str) -> Iterable[Path]:
+        secret_dir = self.directory(generator, name)
+        shutil.rmtree(secret_dir)
+        return [secret_dir]
+
+    def delete_store(self) -> Iterable[Path]:
+        flake_root = Path(self.machine.flake_dir)
+        store_folder = flake_root / "vars/per-machine" / self.machine.name
+        if not store_folder.exists():
+            return []
+        shutil.rmtree(store_folder)
+        return [store_folder]
+
     def populate_dir(self, output_dir: Path, phases: list[str]) -> None:
         if "users" in phases or "services" in phases:
             key_name = f"{self.machine.name}-age.key"
diff --git a/pkgs/clan-cli/clan_cli/vars/secret_modules/vm.py b/pkgs/clan-cli/clan_cli/vars/secret_modules/vm.py
index 2fefcce5f..d8c1aa59d 100644
--- a/pkgs/clan-cli/clan_cli/vars/secret_modules/vm.py
+++ b/pkgs/clan-cli/clan_cli/vars/secret_modules/vm.py
@@ -1,4 +1,5 @@
 import shutil
+from collections.abc import Iterable
 from pathlib import Path
 
 from clan_cli.dirs import vm_state_dir
@@ -39,6 +40,21 @@ class SecretStore(StoreBase):
         secret_file = self.dir / generator.name / name
         return secret_file.read_bytes()
 
+    def delete(self, generator: Generator, name: str) -> Iterable[Path]:
+        secret_dir = self.dir / generator.name
+        secret_file = secret_dir / name
+        secret_file.unlink()
+        empty = None
+        if next(secret_dir.iterdir(), empty) is empty:
+            secret_dir.rmdir()
+        return [secret_file]
+
+    def delete_store(self) -> Iterable[Path]:
+        if not self.dir.exists():
+            return []
+        shutil.rmtree(self.dir)
+        return [self.dir]
+
     def populate_dir(self, output_dir: Path, phases: list[str]) -> None:
         if output_dir.exists():
             shutil.rmtree(output_dir)

From cd248b69db0a66e808844a6006424a42d28744f2 Mon Sep 17 00:00:00 2001
From: Louis Opter <louis@opter.org>
Date: Fri, 7 Feb 2025 11:31:38 +0000
Subject: [PATCH 4/8] clan-cli: machines delete: delete the machine's vars and
 secrets

When a machine is deleted with `clan machines delete`, remove its
vars and legacy secrets, and update any secrets that reference the
machine's key.

This command is a superset of `clan secrets machine delete`, and I am
wondering if we could remove the `clan secrets machine` subcommand,
unless there is an use case for having a machine defined without its
key, and any secrets/vars?

Note:

- This deletes the `ListSecretsOptions` dataclass, as it did not seem to
  bring any value, especially since `list_secrets` was receiving its
  individual members instead of the whole dataclass. We can always bring
  it back if complexity grows to demand it.
---
 pkgs/clan-cli/clan_cli/completions.py     |  9 ++----
 pkgs/clan-cli/clan_cli/machines/delete.py | 38 +++++++++++++++++++++--
 pkgs/clan-cli/clan_cli/secrets/secrets.py | 23 +++++---------
 3 files changed, 45 insertions(+), 25 deletions(-)

diff --git a/pkgs/clan-cli/clan_cli/completions.py b/pkgs/clan-cli/clan_cli/completions.py
index 2bb64aa83..30e329089 100644
--- a/pkgs/clan-cli/clan_cli/completions.py
+++ b/pkgs/clan-cli/clan_cli/completions.py
@@ -208,16 +208,11 @@ def complete_secrets(
     Provides completion functionality for clan secrets
     """
     from .clan_uri import Flake
-    from .secrets.secrets import ListSecretsOptions, list_secrets
+    from .secrets.secrets import list_secrets
 
     flake = clan_dir_result if (clan_dir_result := clan_dir(None)) is not None else "."
 
-    options = ListSecretsOptions(
-        flake=Flake(flake),
-        pattern=None,
-    )
-
-    secrets = list_secrets(options.flake.path, options.pattern)
+    secrets = list_secrets(Flake(flake).path)
 
     secrets_dict = dict.fromkeys(secrets, "secret")
     return secrets_dict
diff --git a/pkgs/clan-cli/clan_cli/machines/delete.py b/pkgs/clan-cli/clan_cli/machines/delete.py
index 18adc78c4..b24de1392 100644
--- a/pkgs/clan-cli/clan_cli/machines/delete.py
+++ b/pkgs/clan-cli/clan_cli/machines/delete.py
@@ -1,25 +1,57 @@
 import argparse
 import logging
 import shutil
+from pathlib import Path
 
+from clan_cli import inventory
 from clan_cli.api import API
 from clan_cli.clan_uri import Flake
 from clan_cli.completions import add_dynamic_completer, complete_machines
 from clan_cli.dirs import specific_machine_dir
-from clan_cli.inventory import delete
+from clan_cli.secrets.folders import sops_secrets_folder
+from clan_cli.secrets.machines import has_machine as secrets_has_machine
+from clan_cli.secrets.machines import remove_machine as secrets_machine_remove
+from clan_cli.secrets.secrets import (
+    list_secrets,
+)
+from clan_cli.vars.list import (
+    public_store as vars_public_store,
+)
+from clan_cli.vars.list import (
+    secret_store as vars_secret_store,
+)
+
+from .machines import Machine
 
 log = logging.getLogger(__name__)
 
 
 @API.register
 def delete_machine(flake: Flake, name: str) -> None:
-    delete(str(flake.path), {f"machines.{name}"})
+    inventory.delete(str(flake.path), {f"machines.{name}"})
+
+    changed_paths: list[Path] = []
 
-    # Remove the machine directory
     folder = specific_machine_dir(flake.path, name)
     if folder.exists():
+        changed_paths.append(folder)
         shutil.rmtree(folder)
 
+    # louis@(2025-02-04): clean-up legacy (pre-vars) secrets:
+    sops_folder = sops_secrets_folder(flake.path)
+    filter_fn = lambda secret_name: secret_name.startswith(f"{name}-")
+    for secret_name in list_secrets(flake.path, filter_fn):
+        secret_path = sops_folder / secret_name
+        changed_paths.append(secret_path)
+        shutil.rmtree(secret_path)
+
+    machine = Machine(name, flake)
+    changed_paths.extend(vars_public_store(machine).delete_store())
+    changed_paths.extend(vars_secret_store(machine).delete_store())
+    # Remove the machine's key, and update secrets & vars that referenced it:
+    if secrets_has_machine(flake.path, name):
+        secrets_machine_remove(flake.path, name)
+
 
 def delete_command(args: argparse.Namespace) -> None:
     delete_machine(args.flake, args.name)
diff --git a/pkgs/clan-cli/clan_cli/secrets/secrets.py b/pkgs/clan-cli/clan_cli/secrets/secrets.py
index e3af24955..ab9aa0713 100644
--- a/pkgs/clan-cli/clan_cli/secrets/secrets.py
+++ b/pkgs/clan-cli/clan_cli/secrets/secrets.py
@@ -6,7 +6,6 @@ import os
 import shutil
 import sys
 from collections.abc import Callable
-from dataclasses import dataclass
 from pathlib import Path
 from typing import IO
 
@@ -18,7 +17,6 @@ from clan_cli.completions import (
     complete_users,
 )
 from clan_cli.errors import ClanError
-from clan_cli.flake import Flake
 from clan_cli.git import commit_files
 
 from . import sops
@@ -325,31 +323,26 @@ def has_secret(secret_path: Path) -> bool:
     return (secret_path / "secret").exists()
 
 
-def list_secrets(flake_dir: Path, pattern: str | None = None) -> list[str]:
+def list_secrets(
+    flake_dir: Path, filter_fn: Callable[[str], bool] | None = None
+) -> list[str]:
     path = sops_secrets_folder(flake_dir)
 
     def validate(name: str) -> bool:
         return (
             VALID_SECRET_NAME.match(name) is not None
             and has_secret(sops_secrets_folder(flake_dir) / name)
-            and (pattern is None or pattern in name)
+            and (filter_fn is None or filter_fn(name) is True)
         )
 
     return list_objects(path, validate)
 
 
-@dataclass
-class ListSecretsOptions:
-    flake: Flake
-    pattern: str | None
-
-
 def list_command(args: argparse.Namespace) -> None:
-    options = ListSecretsOptions(
-        flake=args.flake,
-        pattern=args.pattern,
-    )
-    lst = list_secrets(options.flake.path, options.pattern)
+    def filter_fn(name: str) -> bool:
+        return args.pattern in name
+
+    lst = list_secrets(args.flake.path, filter_fn if args.pattern else None)
     if len(lst) > 0:
         print("\n".join(lst))
 

From 4120a891153cb7ea3d5b776f1cbce9cd05b6a523 Mon Sep 17 00:00:00 2001
From: Louis Opter <louis@opter.org>
Date: Mon, 10 Mar 2025 22:06:06 +0000
Subject: [PATCH 5/8] clan-cli: do not crash if a machine being deleted is
 missing from the inventory

We implement that by actually raising `KeyError` in `inventory.delete_by_path`
(as advertised in the docstring), since it makes more sense to catch a
`KeyError` than a generic `ClanError`.
---
 pkgs/clan-cli/clan_cli/inventory/__init__.py | 12 ++++++------
 pkgs/clan-cli/clan_cli/machines/delete.py    | 11 ++++++++++-
 pkgs/clan-cli/tests/test_patch_inventory.py  |  8 ++++----
 3 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/pkgs/clan-cli/clan_cli/inventory/__init__.py b/pkgs/clan-cli/clan_cli/inventory/__init__.py
index 7cb188c1c..64a791b6b 100644
--- a/pkgs/clan-cli/clan_cli/inventory/__init__.py
+++ b/pkgs/clan-cli/clan_cli/inventory/__init__.py
@@ -431,7 +431,7 @@ def delete_by_path(d: dict[str, Any], path: str) -> Any:
     """
     if not path:
         msg = "Cannot delete. Path is empty."
-        raise ClanError(msg)
+        raise KeyError(msg)
 
     keys = path.split(".")
     current = d
@@ -439,17 +439,17 @@ def delete_by_path(d: dict[str, Any], path: str) -> Any:
     # Navigate to the parent dictionary of the final key
     for key in keys[:-1]:
         if key not in current or not isinstance(current[key], dict):
-            msg = f"Cannot delete. Key '{path}' not found or not a dictionary '{d}'."
-            raise ClanError(msg)
+            msg = f"Cannot delete. Key '{path}' not found or not a dictionary '{d}'"
+            raise KeyError(msg)
         current = current[key]
 
     # Attempt to pop the final key
     last_key = keys[-1]
     try:
         value = current.pop(last_key)
-    except KeyError:
-        msg = f"Cannot delete. Path '{path}' not found in data '{d}'. "
-        raise ClanError(msg) from KeyError
+    except KeyError as exc:
+        msg = f"Cannot delete. Path '{path}' not found in data '{d}'"
+        raise KeyError(msg) from exc
     else:
         return {last_key: value}
 
diff --git a/pkgs/clan-cli/clan_cli/machines/delete.py b/pkgs/clan-cli/clan_cli/machines/delete.py
index b24de1392..1d973dd14 100644
--- a/pkgs/clan-cli/clan_cli/machines/delete.py
+++ b/pkgs/clan-cli/clan_cli/machines/delete.py
@@ -28,7 +28,16 @@ log = logging.getLogger(__name__)
 
 @API.register
 def delete_machine(flake: Flake, name: str) -> None:
-    inventory.delete(str(flake.path), {f"machines.{name}"})
+    try:
+        inventory.delete(str(flake.path), {f"machines.{name}"})
+    except KeyError as exc:
+        # louis@(2025-03-09): test infrastructure does not seem to set the
+        # inventory properly, but more importantly only one machine in my
+        # personal clan ended up in the inventory for some reason, so I think
+        # it makes sense to eat the exception here.
+        log.warning(
+            f"{name} was missing or already deleted from the machines inventory: {exc}"
+        )
 
     changed_paths: list[Path] = []
 
diff --git a/pkgs/clan-cli/tests/test_patch_inventory.py b/pkgs/clan-cli/tests/test_patch_inventory.py
index 356e6cfcf..376c21dad 100644
--- a/pkgs/clan-cli/tests/test_patch_inventory.py
+++ b/pkgs/clan-cli/tests/test_patch_inventory.py
@@ -557,7 +557,7 @@ def test_delete_top_level() -> None:
 def test_delete_key_not_found() -> None:
     data = {"foo": {"bar": 1}}
     # Trying to delete a non-existing key "foo.baz"
-    with pytest.raises(ClanError) as excinfo:
+    with pytest.raises(KeyError) as excinfo:
         delete_by_path(data, "foo.baz")
     assert "Cannot delete. Path 'foo.baz'" in str(excinfo.value)
     # Data should remain unchanged
@@ -567,7 +567,7 @@ def test_delete_key_not_found() -> None:
 def test_delete_intermediate_not_dict() -> None:
     data = {"foo": "not a dict"}
     # Trying to go deeper into a non-dict value
-    with pytest.raises(ClanError) as excinfo:
+    with pytest.raises(KeyError) as excinfo:
         delete_by_path(data, "foo.bar")
     assert "not found or not a dictionary" in str(excinfo.value)
     # Data should remain unchanged
@@ -577,7 +577,7 @@ def test_delete_intermediate_not_dict() -> None:
 def test_delete_empty_path() -> None:
     data = {"foo": {"bar": 1}}
     # Attempting to delete with an empty path
-    with pytest.raises(ClanError) as excinfo:
+    with pytest.raises(KeyError) as excinfo:
         delete_by_path(data, "")
     # Depending on how you handle empty paths, you might raise an error or handle it differently.
     # If you do raise an error, check the message.
@@ -588,7 +588,7 @@ def test_delete_empty_path() -> None:
 def test_delete_non_existent_path_deep() -> None:
     data = {"foo": {"bar": {"baz": 123}}}
     # non-existent deep path
-    with pytest.raises(ClanError) as excinfo:
+    with pytest.raises(KeyError) as excinfo:
         delete_by_path(data, "foo.bar.qux")
     assert "not found" in str(excinfo.value)
     # Data remains unchanged

From b52ec0549751f06e136d7913d9e0de361d7bdc48 Mon Sep 17 00:00:00 2001
From: Louis Opter <louis@opter.org>
Date: Tue, 18 Mar 2025 17:49:51 +0000
Subject: [PATCH 6/8] clan-cli: don't try to delete a dir that doesn't exist in
 the pass vars backend

Do not crash in `delete_store`, if the machine has no vars, or the store
has been deleted already.
---
 .../vars/secret_modules/password_store.py     |  8 +++--
 pkgs/clan-cli/tests/test_vars.py              | 33 +++++++++++++++----
 2 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/pkgs/clan-cli/clan_cli/vars/secret_modules/password_store.py b/pkgs/clan-cli/clan_cli/vars/secret_modules/password_store.py
index c82b59fa4..57524d8c4 100644
--- a/pkgs/clan-cli/clan_cli/vars/secret_modules/password_store.py
+++ b/pkgs/clan-cli/clan_cli/vars/secret_modules/password_store.py
@@ -77,8 +77,12 @@ class SecretStore(StoreBase):
         return []
 
     def delete_store(self) -> Iterable[Path]:
-        machine_pass_dir = Path(self.entry_prefix) / "per-machine" / self.machine.name
-        pass_call = ["rm", "--force", "--recursive", str(machine_pass_dir)]
+        machine_dir = Path(self.entry_prefix) / "per-machine" / self.machine.name
+        if not (self._password_store_dir / machine_dir).exists():
+            # The directory may not exist if the machine
+            # has no vars, or they have been deleted already.
+            return []
+        pass_call = ["rm", "--force", "--recursive", str(machine_dir)]
         self._run_pass(*pass_call, options=RunOpts(check=True))
         return []
 
diff --git a/pkgs/clan-cli/tests/test_vars.py b/pkgs/clan-cli/tests/test_vars.py
index 3062718d4..68163e474 100644
--- a/pkgs/clan-cli/tests/test_vars.py
+++ b/pkgs/clan-cli/tests/test_vars.py
@@ -366,13 +366,17 @@ def test_generate_secret_var_password_store(
 ) -> None:
     config = flake.machines["my_machine"]
     config["nixpkgs"]["hostPlatform"] = "x86_64-linux"
-    config["clan"]["core"]["vars"]["settings"]["secretStore"] = "password-store"
-    my_generator = config["clan"]["core"]["vars"]["generators"]["my_generator"]
+    clan_vars = config["clan"]["core"]["vars"]
+    clan_vars["settings"]["secretStore"] = "password-store"
+    # Create a second secret so that when we delete the first one,
+    # we still have the second one to test `delete_store`:
+    my_generator = clan_vars["generators"]["my_generator"]
     my_generator["files"]["my_secret"]["secret"] = True
     my_generator["script"] = "echo hello > $out/my_secret"
-    my_shared_generator = config["clan"]["core"]["vars"]["generators"][
-        "my_shared_generator"
-    ]
+    my_generator2 = clan_vars["generators"]["my_generator2"]
+    my_generator2["files"]["my_secret2"]["secret"] = True
+    my_generator2["script"] = "echo world > $out/my_secret2"
+    my_shared_generator = clan_vars["generators"]["my_shared_generator"]
     my_shared_generator["share"] = True
     my_shared_generator["files"]["my_shared_secret"]["secret"] = True
     my_shared_generator["script"] = "echo hello > $out/my_shared_secret"
@@ -384,7 +388,7 @@ def test_generate_secret_var_password_store(
 
     password_store_dir = flake.path / "pass"
     shutil.copytree(test_root / "data" / "password-store", password_store_dir)
-    monkeypatch.setenv("PASSWORD_STORE_DIR", str(flake.path / "pass"))
+    monkeypatch.setenv("PASSWORD_STORE_DIR", str(password_store_dir))
 
     machine = Machine(name="my_machine", flake=Flake(str(flake.path)))
     assert not check_vars(machine)
@@ -409,6 +413,23 @@ def test_generate_secret_var_password_store(
     vars_text = stringify_all_vars(machine)
     assert "my_generator/my_secret" in vars_text
 
+    my_generator = Generator("my_generator", share=False, files=[])
+    var_name = "my_secret"
+    store.delete(my_generator, var_name)
+    assert not store.exists(my_generator, var_name)
+
+    store.delete_store()
+    store.delete_store()  # check idempotency
+    my_generator2 = Generator("my_generator2", share=False, files=[])
+    var_name = "my_secret2"
+    assert not store.exists(my_generator2, var_name)
+
+    # The shared secret should still be there,
+    # not sure if we can delete those automatically:
+    my_shared_generator = Generator("my_shared_generator", share=True, files=[])
+    var_name = "my_shared_secret"
+    assert store.exists(my_shared_generator, var_name)
+
 
 @pytest.mark.with_core
 def test_generate_secret_for_multiple_machines(

From 546ed03a90c18123d2edb6c3c83871726e4833a8 Mon Sep 17 00:00:00 2001
From: Louis Opter <louis@opter.org>
Date: Tue, 18 Mar 2025 18:22:56 +0000
Subject: [PATCH 7/8] clan-cli: tests: call `SopsSetup.init` while setting up
 fixtures

We do this by introducing `flake_with_sops` fixture, that calls the
init method ahead of the test. We did not want to do this in the `flake`
fixture since not all tests using the `flake` fixture need to have sops
setup.
---
 pkgs/clan-cli/tests/age_keys.py          |  9 +---
 pkgs/clan-cli/tests/fixtures_flakes.py   | 10 ++++
 pkgs/clan-cli/tests/test_machines_cli.py | 11 +----
 pkgs/clan-cli/tests/test_vars.py         | 62 +++++++++++-------------
 4 files changed, 42 insertions(+), 50 deletions(-)

diff --git a/pkgs/clan-cli/tests/age_keys.py b/pkgs/clan-cli/tests/age_keys.py
index 7c356620f..41162a287 100644
--- a/pkgs/clan-cli/tests/age_keys.py
+++ b/pkgs/clan-cli/tests/age_keys.py
@@ -26,14 +26,7 @@ class SopsSetup:
         self.keys = keys
         self.user = os.environ.get("USER", "admin")
 
-    # louis@(2025-03-10): It is odd to have to call an init function on a
-    # fixture: the fixture should already be initialized when it is received in
-    # the test function. Maybe we can arrange for the `flake` fixtures, to take
-    # the `sops_setup` fixture as input and call its `init` function on the
-    # correct path.
-    def init(self, flake_path: Path | None = None) -> None:
-        if flake_path is None:
-            flake_path = Path.cwd()
+    def init(self, flake_path: Path) -> None:
         cli.run(
             [
                 "vars",
diff --git a/pkgs/clan-cli/tests/fixtures_flakes.py b/pkgs/clan-cli/tests/fixtures_flakes.py
index 114536718..17b32ec4a 100644
--- a/pkgs/clan-cli/tests/fixtures_flakes.py
+++ b/pkgs/clan-cli/tests/fixtures_flakes.py
@@ -9,6 +9,7 @@ from collections.abc import Callable, Iterator
 from pathlib import Path
 from typing import Any, NamedTuple
 
+import age_keys
 import pytest
 from clan_cli.dirs import TemplateType, clan_templates, nixpkgs_source
 from clan_cli.locked_open import locked_open
@@ -229,6 +230,15 @@ def flake(
     return minimal_flake_template.copy(temporary_home, monkeypatch)
 
 
+@pytest.fixture
+def flake_with_sops(
+    flake: ClanFlake,
+    sops_setup: age_keys.SopsSetup,
+) -> ClanFlake:
+    sops_setup.init(flake.path)
+    return flake
+
+
 def create_flake(
     temporary_home: Path,
     flake_template: str | Path,
diff --git a/pkgs/clan-cli/tests/test_machines_cli.py b/pkgs/clan-cli/tests/test_machines_cli.py
index 9e5977c1d..4c4db7173 100644
--- a/pkgs/clan-cli/tests/test_machines_cli.py
+++ b/pkgs/clan-cli/tests/test_machines_cli.py
@@ -51,21 +51,14 @@ def test_machine_subcommands(
     assert "vm2" in output.out
 
 
-# louis(2025-03-09):
-#
-# The `with_core` mark is cargo-culted from
-# `test_generate_public_and_secret_vars` which
-# I used as a starting point:
 @pytest.mark.with_core
 def test_machine_delete(
     monkeypatch: pytest.MonkeyPatch,
-    flake: fixtures_flakes.ClanFlake,
+    flake_with_sops: fixtures_flakes.ClanFlake,
     sops_setup: SopsSetup,
 ) -> None:
-    # create the admin user and set its key:
-    sops_setup.init(flake.path)
+    flake = flake_with_sops
 
-    # admin_key, machine_key, machine2_key = age_keys
     admin_key, machine_key, machine2_key = sops_setup.keys
 
     # create a couple machines with their keys
diff --git a/pkgs/clan-cli/tests/test_vars.py b/pkgs/clan-cli/tests/test_vars.py
index 68163e474..12123e8ab 100644
--- a/pkgs/clan-cli/tests/test_vars.py
+++ b/pkgs/clan-cli/tests/test_vars.py
@@ -2,7 +2,6 @@ import json
 import logging
 import shutil
 from pathlib import Path
-from typing import TYPE_CHECKING
 
 import pytest
 from age_keys import SopsSetup
@@ -21,9 +20,6 @@ from clan_cli.vars.set import set_var
 from fixtures_flakes import ClanFlake
 from helpers import cli
 
-if TYPE_CHECKING:
-    from age_keys import KeyPair
-
 
 def test_dependencies_as_files(temp_dir: Path) -> None:
     from clan_cli.vars.generate import dependencies_as_dir
@@ -100,9 +96,10 @@ def test_required_generators() -> None:
 @pytest.mark.with_core
 def test_generate_public_and_secret_vars(
     monkeypatch: pytest.MonkeyPatch,
-    flake: ClanFlake,
-    sops_setup: SopsSetup,
+    flake_with_sops: ClanFlake,
 ) -> None:
+    flake = flake_with_sops
+
     config = flake.machines["my_machine"]
     config["nixpkgs"]["hostPlatform"] = "x86_64-linux"
     my_generator = config["clan"]["core"]["vars"]["generators"]["my_generator"]
@@ -136,7 +133,6 @@ def test_generate_public_and_secret_vars(
 
     flake.refresh()
     monkeypatch.chdir(flake.path)
-    sops_setup.init()
 
     machine = Machine(name="my_machine", flake=Flake(str(flake.path)))
     assert not check_vars(machine)
@@ -227,10 +223,11 @@ def test_generate_public_and_secret_vars(
 @pytest.mark.with_core
 def test_generate_secret_var_sops_with_default_group(
     monkeypatch: pytest.MonkeyPatch,
-    flake: ClanFlake,
+    flake_with_sops: ClanFlake,
     sops_setup: SopsSetup,
-    age_keys: list["KeyPair"],
 ) -> None:
+    flake = flake_with_sops
+
     config = flake.machines["my_machine"]
     config["nixpkgs"]["hostPlatform"] = "x86_64-linux"
     config["clan"]["core"]["sops"]["defaultGroups"] = ["my_group"]
@@ -248,7 +245,6 @@ def test_generate_secret_var_sops_with_default_group(
     )
     flake.refresh()
     monkeypatch.chdir(flake.path)
-    sops_setup.init()
     cli.run(["secrets", "groups", "add-user", "my_group", sops_setup.user])
     cli.run(["vars", "generate", "--flake", str(flake.path), "my_machine"])
     in_repo_store = in_repo.FactStore(
@@ -268,7 +264,7 @@ def test_generate_secret_var_sops_with_default_group(
     )
 
     # add another user to the group and check if secret gets re-encrypted
-    pubkey_user2 = age_keys[1]
+    pubkey_user2 = sops_setup.keys[1]
     cli.run(
         [
             "secrets",
@@ -291,7 +287,7 @@ def test_generate_secret_var_sops_with_default_group(
     )
 
     # Rotate key of a user
-    pubkey_user3 = age_keys[2]
+    pubkey_user3 = sops_setup.keys[2]
     cli.run(
         [
             "secrets",
@@ -316,9 +312,10 @@ def test_generate_secret_var_sops_with_default_group(
 @pytest.mark.with_core
 def test_generated_shared_secret_sops(
     monkeypatch: pytest.MonkeyPatch,
-    flake: ClanFlake,
-    sops_setup: SopsSetup,
+    flake_with_sops: ClanFlake,
 ) -> None:
+    flake = flake_with_sops
+
     m1_config = flake.machines["machine1"]
     m1_config["nixpkgs"]["hostPlatform"] = "x86_64-linux"
     shared_generator = m1_config["clan"]["core"]["vars"]["generators"][
@@ -334,7 +331,6 @@ def test_generated_shared_secret_sops(
     )
     flake.refresh()
     monkeypatch.chdir(flake.path)
-    sops_setup.init()
     machine1 = Machine(name="machine1", flake=Flake(str(flake.path)))
     machine2 = Machine(name="machine2", flake=Flake(str(flake.path)))
     cli.run(["vars", "generate", "--flake", str(flake.path), "machine1"])
@@ -434,9 +430,10 @@ def test_generate_secret_var_password_store(
 @pytest.mark.with_core
 def test_generate_secret_for_multiple_machines(
     monkeypatch: pytest.MonkeyPatch,
-    flake: ClanFlake,
-    sops_setup: SopsSetup,
+    flake_with_sops: ClanFlake,
 ) -> None:
+    flake = flake_with_sops
+
     from clan_cli.nix import nix_config
 
     local_system = nix_config()["system"]
@@ -467,7 +464,6 @@ def test_generate_secret_for_multiple_machines(
     )
     flake.refresh()
     monkeypatch.chdir(flake.path)
-    sops_setup.init()
     cli.run(["vars", "generate", "--flake", str(flake.path)])
     # check if public vars have been created correctly
     in_repo_store1 = in_repo.FactStore(
@@ -506,9 +502,10 @@ def test_generate_secret_for_multiple_machines(
 @pytest.mark.with_core
 def test_prompt(
     monkeypatch: pytest.MonkeyPatch,
-    flake: ClanFlake,
-    sops_setup: SopsSetup,
+    flake_with_sops: ClanFlake,
 ) -> None:
+    flake = flake_with_sops
+
     config = flake.machines["my_machine"]
     config["nixpkgs"]["hostPlatform"] = "x86_64-linux"
     my_generator = config["clan"]["core"]["vars"]["generators"]["my_generator"]
@@ -530,7 +527,6 @@ def test_prompt(
     )
     flake.refresh()
     monkeypatch.chdir(flake.path)
-    sops_setup.init()
     monkeypatch.setattr(
         "clan_cli.vars.prompt.MOCK_PROMPT_RESPONSE",
         iter(["line input", "my\nmultiline\ninput\n", "prompt_persist"]),
@@ -565,8 +561,7 @@ def test_prompt(
 @pytest.mark.with_core
 def test_multi_machine_shared_vars(
     monkeypatch: pytest.MonkeyPatch,
-    flake: ClanFlake,
-    sops_setup: SopsSetup,
+    flake_with_sops: ClanFlake,
 ) -> None:
     """
     Ensure that shared vars are regenerated only when they should, and also can be
@@ -576,6 +571,8 @@ def test_multi_machine_shared_vars(
         - make sure shared wars are not regenerated when a second machines is added
         - make sure vars can still be accessed by all machines, after they are regenerated
     """
+    flake = flake_with_sops
+
     machine1_config = flake.machines["machine1"]
     machine1_config["nixpkgs"]["hostPlatform"] = "x86_64-linux"
     shared_generator = machine1_config["clan"]["core"]["vars"]["generators"][
@@ -591,7 +588,6 @@ def test_multi_machine_shared_vars(
     flake.machines["machine2"] = machine1_config
     flake.refresh()
     monkeypatch.chdir(flake.path)
-    sops_setup.init()
     machine1 = Machine(name="machine1", flake=Flake(str(flake.path)))
     machine2 = Machine(name="machine2", flake=Flake(str(flake.path)))
     sops_store_1 = sops.SecretStore(machine1)
@@ -680,10 +676,11 @@ def test_api_set_prompts(
 @pytest.mark.with_core
 def test_stdout_of_generate(
     monkeypatch: pytest.MonkeyPatch,
-    flake: ClanFlake,
-    sops_setup: SopsSetup,
+    flake_with_sops: ClanFlake,
     caplog: pytest.LogCaptureFixture,
 ) -> None:
+    flake = flake_with_sops
+
     config = flake.machines["my_machine"]
     config["nixpkgs"]["hostPlatform"] = "x86_64-linux"
     my_generator = config["clan"]["core"]["vars"]["generators"]["my_generator"]
@@ -696,7 +693,6 @@ def test_stdout_of_generate(
     my_secret_generator["script"] = "echo -n hello > $out/my_secret"
     flake.refresh()
     monkeypatch.chdir(flake.path)
-    sops_setup.init()
     from clan_cli.vars.generate import generate_vars_for_machine
 
     # with capture_output as output:
@@ -763,10 +759,11 @@ def test_stdout_of_generate(
 @pytest.mark.with_core
 def test_migration(
     monkeypatch: pytest.MonkeyPatch,
-    flake: ClanFlake,
-    sops_setup: SopsSetup,
+    flake_with_sops: ClanFlake,
     caplog: pytest.LogCaptureFixture,
 ) -> None:
+    flake = flake_with_sops
+
     config = flake.machines["my_machine"]
     config["nixpkgs"]["hostPlatform"] = "x86_64-linux"
     my_service = config["clan"]["core"]["facts"]["services"]["my_service"]
@@ -792,7 +789,6 @@ def test_migration(
 
     flake.refresh()
     monkeypatch.chdir(flake.path)
-    sops_setup.init()
     cli.run(["facts", "generate", "--flake", str(flake.path), "my_machine"])
     with caplog.at_level(logging.INFO):
         cli.run(["vars", "generate", "--flake", str(flake.path), "my_machine"])
@@ -819,9 +815,10 @@ def test_migration(
 @pytest.mark.with_core
 def test_fails_when_files_are_left_from_other_backend(
     monkeypatch: pytest.MonkeyPatch,
-    flake: ClanFlake,
-    sops_setup: SopsSetup,
+    flake_with_sops: ClanFlake,
 ) -> None:
+    flake = flake_with_sops
+
     config = flake.machines["my_machine"]
     config["nixpkgs"]["hostPlatform"] = "x86_64-linux"
     my_secret_generator = config["clan"]["core"]["vars"]["generators"][
@@ -836,7 +833,6 @@ def test_fails_when_files_are_left_from_other_backend(
     my_value_generator["script"] = "echo hello > $out/my_value"
     flake.refresh()
     monkeypatch.chdir(flake.path)
-    sops_setup.init()
     for generator in ["my_secret_generator", "my_value_generator"]:
         generate_vars_for_machine(
             Machine(name="my_machine", flake=Flake(str(flake.path))),

From daeeb2ee436845ee09029f0f7c333aa593ae2957 Mon Sep 17 00:00:00 2001
From: Louis Opter <louis@opter.org>
Date: Tue, 18 Mar 2025 18:27:15 +0000
Subject: [PATCH 8/8] clan-cli: add DavHau's explanation about the `with_core`
 pytest marker

See: https://git.clan.lol/clan/clan-core/pulls/2994#issuecomment-22542
---
 pkgs/clan-cli/pyproject.toml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/pkgs/clan-cli/pyproject.toml b/pkgs/clan-cli/pyproject.toml
index bec9da247..969f91a40 100644
--- a/pkgs/clan-cli/pyproject.toml
+++ b/pkgs/clan-cli/pyproject.toml
@@ -34,6 +34,11 @@ log_level = "DEBUG"
 log_format = "%(message)s"
 addopts = "--cov . --cov-report term --cov-report html:.reports/html --no-cov-on-fail --durations 5 --color=yes --new-first -W error -n auto" # Add --pdb for debugging
 norecursedirs = ["clan_cli/tests/helpers", "clan_cli/nixpkgs"]
+#  All tests which evaluate any nix library code from clan-core need to use the
+#  `with_core` marker, so basically all tests which evaluate a flake with
+#  machines. In the CI pipeline we run these tests in a separate derivation
+#  depending on clan-core. All other tests do not need to depend on clan-core
+#  and can be cached more effectively.
 markers = ["impure", "with_core"]
 filterwarnings = "default::ResourceWarning"