fix: correct apt.key removal, idempotency and test fixtures

- Rename _sanitize_apt_keyring_name → _sanitize_keyring_part (cleaner name)
- Remove dead 'dest' param from _derive_dest_from_src_and_keyids
- Expose APT_KEYRING_DIRS constant at module level
- Convert apt.key docstring to pyinfra '+' format
- Fix present=False + keyid without dest: delegate directly to
  gpg.key._inner(working_dirs=APT_KEYRING_DIRS) instead of resolving a
  wrong default dest
- Fix AptKeys.command() signature: directories=None with APT defaults as
  fallback, keeping LSP-compatible interface
- Add collision comment in AptKeys.process() (last keyring wins on same ID)
- Remove dead apt.AptKeys/gpg.GpgKey facts from test fixtures (apt.key
  delegates to gpg.key._inner, never calls those facts directly)
- Fix _tempfile_ path placeholder in keyserver fixtures
- Rewrite add_exists/add_keyserver_exists as true idempotent fixtures
  (file/keys already present → commands: [])
- Delete add_no_gpg and add_keyserver_multiple_partial (untestable or
  superseded)
- Add remove_file, remove_file_missing, remove_by_keyid fixtures

Author: maisim

src/pyinfra/facts/apt.py

@@ -382,28 +382,31 @@ class AptKeys(GpgKeyrings):
     .. code:: python
 
         {
-            "KEY-ID": {
+            "3B4FE6ACC0B21F32": {
+                "validity": "-",
                 "length": 4096,
-                "uid": "Oxygem <hello@oxygem.com>"
+                "subkeys": {},
+                "fingerprint": "790BC7277767219C42C86F933B4FE6ACC0B21F32",
+                "uid_hash": "B7A02867A0C1D32B594B36C00E20C8C57E397748",
+                "uid": "Ubuntu Archive Automatic Signing Key (2012) <ftpmaster@ubuntu.com>"
             },
         }
     """
 
     @override
-    def command(self, directories=None) -> str:
-        # Default to APT-specific directories if none specified
-        if directories is None:
-            directories = ["/etc/apt/trusted.gpg.d", "/etc/apt/keyrings", "/usr/share/keyrings"]
-
-        return super().command(directories)
+    def command(self, directories: list[str] | None = None) -> str:
+        return super().command(
+            directories or ["/etc/apt/trusted.gpg.d", "/etc/apt/keyrings", "/usr/share/keyrings"]
+        )
 
     @override
     def process(self, output):
         # Get the full keyring structure from parent
         keyrings_data = super().process(output)
 
-        # Flatten to match traditional AptKeys format (just key_id -> key_details)
-        flattened_keys = {}
+        # Flatten to match the traditional AptKeys format: {key_id: key_details}
+        # Note: if the same key ID appears in multiple keyring files, the last one wins.
+        flattened_keys: dict = {}
         for keyring_path, keyring_info in keyrings_data.items():
             if "keys" in keyring_info:
                 flattened_keys.update(keyring_info["keys"])

src/pyinfra/operations/apt.py

@@ -10,23 +10,21 @@
 
 from pyinfra import host
 from pyinfra.api import operation
-from pyinfra.api.exceptions import OperationError
 from pyinfra.facts.apt import (
-    AptKeys,
     AptSources,
     SimulateOperationWillChange,
     noninteractive_apt,
     parse_apt_repo,
 )
 from pyinfra.facts.deb import DebPackage, DebPackages
 from pyinfra.facts.files import File
-from pyinfra.facts.gpg import GpgKey, GpgKeyrings
 from pyinfra.facts.server import Date
 from pyinfra.operations import files, gpg
 
 from .util.packaging import ensure_packages
 
 APT_UPDATE_FILENAME = "/var/lib/apt/periodic/update-success-stamp"
+APT_KEYRING_DIRS = ["/etc/apt/trusted.gpg.d", "/etc/apt/keyrings", "/usr/share/keyrings"]
 
 
 def _simulate_then_perform(command: str):
@@ -47,82 +45,47 @@ def _simulate_then_perform(command: str):
         yield noninteractive_apt(command)
 
 
-def _sanitize_apt_keyring_name(name: str) -> str:
+def _sanitize_keyring_part(name: str) -> str:
     """
-    Produce a filesystem-friendly name from an URL host/basename or a local filename.
+    Produce a filesystem-friendly segment from a URL host, basename, or key ID.
     """
     name = name.strip().lower()
     name = re.sub(r"[^\w.-]+", "_", name)
     name = re.sub(r"_+", "_", name).strip("_.")
     return name or "apt-keyring"
 
 
-def _derive_dest_from_src_and_keyids(
-    src: str | None, keyids: list[str] | None, dest: str | None
-) -> str:
+def _derive_dest_from_src_and_keyids(src: str | None, keyids: list[str] | None) -> str:
     """
-    Compute a stable destination path in /etc/apt/keyrings/.
+    Compute a stable destination path in /etc/apt/keyrings/ from a source or key IDs.
+
     Priority:
-      1) explicit dest if provided
-      2) from src (URL host + basename, or local basename)
-      3) from keyids (joined)
-      4) fallback "apt-keyring.gpg"
+      1) from src (URL domain + basename, or local basename)
+      2) from keyids (joined)
+      3) fallback "apt-keyring.gpg"
     """
-    if dest:
-        # Ensure it ends with .gpg and is absolute under /etc/apt/keyrings
-        if not dest.endswith(".gpg"):
-            dest += ".gpg"
-        if not dest.startswith("/"):
-            dest = f"/etc/apt/keyrings/{dest}"
-        return dest
-
     base = None
     if src:
         parsed = urlparse(src)
         if parsed.scheme and parsed.netloc:
-            host_name = _sanitize_apt_keyring_name(parsed.netloc.replace(":", "_"))
-            bn = _sanitize_apt_keyring_name(
+            domain_part = _sanitize_keyring_part(parsed.netloc.replace(":", "_"))
+            bn = _sanitize_keyring_part(
                 (parsed.path.rsplit("/", 1)[-1] or "key").replace(".asc", "").replace(".gpg", "")
             )
-            base = f"{host_name}-{bn}"
+            base = f"{domain_part}-{bn}"
         else:
-            bn = _sanitize_apt_keyring_name(
+            bn = _sanitize_keyring_part(
                 src.rsplit("/", 1)[-1].replace(".asc", "").replace(".gpg", "")
             )
             base = bn or "key"
     elif keyids:
-        base = "keyserver-" + _sanitize_apt_keyring_name("-".join(keyids))
+        base = "keyserver-" + _sanitize_keyring_part("-".join(keyids))
     else:
         base = "apt-keyring"
 
     return f"/etc/apt/keyrings/{base}.gpg"
 
 
-def _get_apt_keys_comprehensive() -> dict[str, str]:
-    """
-    Get all GPG keys available in APT directories using the GpgKeyrings fact.
-    This provides more comprehensive coverage than AptKeys fact.
-    Falls back gracefully if GpgKeyrings data is not available.
-
-    Returns:
-        dict: Key ID -> keyring file path mapping
-    """
-    try:
-        apt_directories = ["/etc/apt/trusted.gpg.d", "/etc/apt/keyrings", "/usr/share/keyrings"]
-        keyrings_info = host.get_fact(GpgKeyrings, directories=apt_directories)
-
-        all_keys = {}
-        for keyring_path, keyring_data in keyrings_info.items():
-            keys = keyring_data.get("keys", {})
-            for key_id in keys.keys():
-                all_keys[key_id] = keyring_path
-
-        return all_keys
-    except (KeyError, AttributeError):
-        # Fallback to empty dict if GpgKeyrings fact is not available (e.g., in tests)
-        return {}
-
-
 @operation()
 def key(
     src: str | None = None,
@@ -132,26 +95,27 @@ def key(
     present: bool = True,
 ):
     """
-    Add or remove apt GPG keys using modern keyring management.
+    Add or remove APT GPG keys using modern keyring management (no ``apt-key``).
 
-    This operation manages GPG keys for APT repos without using the deprecated apt-key command.
-    Keys are stored in /etc/apt/keyrings/ and can be referenced in source lists via signed-by=.
+    Keys are written to ``/etc/apt/keyrings/`` and can be referenced in source entries
+    via ``signed-by=``. The destination filename is derived automatically from the source
+    URL or key IDs when not specified explicitly.
 
-    Args:
-        src: filename or URL to a key (ASCII .asc or binary .gpg)
-        keyserver: keyserver URL for fetching keys by ID
-        keyid: key ID or list of key IDs (required with keyserver, optional for removal)
-        dest: optional keyring path ('.gpg' will be enforced, defaults under /etc/apt/keyrings)
-        present: whether the key should be present (True) or absent (False)
+    + src: filename or URL to a key (``.asc`` ASCII-armored or binary ``.gpg``)
+    + keyserver: keyserver URL for fetching keys by ID
+    + keyid: key ID or list of key IDs (required with ``keyserver``, optional for removal)
+    + dest: destination filename or absolute path — ``.gpg`` extension is enforced;
+      relative names are resolved under ``/etc/apt/keyrings/``
+    + present: whether the key should be present (default: ``True``) or removed
 
-    Behavior:
-        - Installation: Idempotent via AptKeys - if key IDs are already present, nothing changes
-        - Removal: Uses GpgKeyrings fact to find and remove keys from APT directories
-        - If src is ASCII (.asc), it will be dearmored; if binary (.gpg), it's copied as-is
-        - Keyserver flow uses temporary GNUPGHOME, then exports to destination keyring
+    .. note::
+        ASCII-armored keys (``.asc``) are automatically dearmored on installation.
+        Keyserver fetches use a temporary ``GNUPGHOME`` and export binary keyrings.
+        Removal without ``keyid`` deletes the whole keyring file; with ``keyid`` it
+        removes individual keys and prunes empty files.
 
     .. warning::
-        ``apt-key`` is deprecated in Debian. This operation follows modern keyring management:
+        ``apt-key`` is deprecated in Debian. This operation follows the modern approach:
         https://wiki.debian.org/DebianRepository/UseThirdParty
 
     **Examples:**
@@ -187,64 +151,34 @@ def key(
         )
     """
 
-    # Handle removal operations using the GPG infrastructure
-    if present is False:
-        # Use the GPG operation for removal, but restrict to APT directories
-        apt_working_dirs = ["/etc/apt/trusted.gpg.d", "/etc/apt/keyrings", "/usr/share/keyrings"]
+    # Special case: remove by key ID without explicit destination → search all APT keyring dirs
+    if not present and keyid and not dest and not src and not keyserver:
         yield from gpg.key._inner(
-            dest=dest,
             keyid=keyid,
             present=False,
-            working_dirs=apt_working_dirs,
+            working_dirs=APT_KEYRING_DIRS,
         )
         return
 
-    # Installation logic (existing code)
-    # Get comprehensive view of all keys in APT directories
-    existing_keys_comprehensive = _get_apt_keys_comprehensive()
-    # Also get the legacy AptKeys fact for compatibility
-    existing_keys = host.get_fact(AptKeys)
-
-    # Combine both sources of key information for complete coverage
-    all_available_keys = set(existing_keys_comprehensive.keys()) | set(existing_keys.keys())
-
-    # Check idempotency for src branch
-    if src:
-        key_data = host.get_fact(GpgKey, src=src)  # Parses the key(s) from src to extract key IDs
-        keyids_from_src = list(key_data.keys()) if key_data else []
-
-        # If we don't know the IDs (eg. unreachable URL), we cannot determine idempotency
-        # -> try to install.
-        # Otherwise, skip if all key IDs are already present.
-        if keyids_from_src and all(kid in all_available_keys for kid in keyids_from_src):
-            host.noop(f"All keys from {src} are already available in the apt keychain")
-            return
-
-        dest_path = _derive_dest_from_src_and_keyids(src, keyids_from_src or None, dest)
-
-    # Check idempotency for keyserver branch
-    elif keyserver:
-        if not keyid:
-            raise OperationError("`keyid` must be provided with `keyserver`")
-
-        if isinstance(keyid, str):
-            keyid = [keyid]
-
-        needed_keys = sorted(set(keyid) - all_available_keys)
-        if not needed_keys:
-            host.noop(f"Keys {', '.join(keyid)} are already available in the apt keychain")
-            return
-
-        dest_path = _derive_dest_from_src_and_keyids(None, needed_keys, dest)
-        # Only install the needed keys
-        keyid = needed_keys
+    # Resolve destination path under /etc/apt/keyrings/
+    if dest and not dest.startswith("/"):
+        dest = f"/etc/apt/keyrings/{dest}"
+    elif not dest:
+        if src:
+            dest = _derive_dest_from_src_and_keyids(src, None)
+        elif keyserver and keyid:
+            keyid_list = [keyid] if isinstance(keyid, str) else keyid
+            dest = _derive_dest_from_src_and_keyids(None, keyid_list)
+        else:
+            dest = "/etc/apt/keyrings/apt-key.gpg"
 
-    # Use the generic GPG operation to install the key
+    # Delegate everything to gpg.key with APT-specific defaults
     yield from gpg.key._inner(
         src=src,
-        dest=dest_path,
+        dest=dest,
         keyserver=keyserver,
         keyid=keyid,
+        present=present,
         dearmor=True,
         mode="0644",
     )

tests/facts/apt.AptKeys/keys.json

@@ -1,5 +1,5 @@
 {
-    "command": "for keyring in $(find \"/etc/apt/trusted.gpg.d\" \"/etc/apt/keyrings\" \"/usr/share/keyrings\" -type f \\( -name '*.gpg' -o -name '*.asc' -o -name '*.kbx' \\) 2>/dev/null); do echo \"KEYRING:$keyring\"; if [[ \"$keyring\" == *.asc ]]; then gpg --with-colons \"$keyring\" 2>/dev/null || true; else gpg --list-keys --with-colons --keyring \"$keyring\" --no-default-keyring 2>/dev/null || true; fi; done",
+    "command": "find \"/etc/apt/trusted.gpg.d\" \"/etc/apt/keyrings\" \"/usr/share/keyrings\" -type f \\( -name '*.gpg' -o -name '*.kbx' \\) 2>/dev/null -exec sh -c 'echo \"KEYRING:$1\"; gpg --list-keys --with-colons --keyring \"$1\" --no-default-keyring 2>/dev/null || true' _ {} \\; ; find \"/etc/apt/trusted.gpg.d\" \"/etc/apt/keyrings\" \"/usr/share/keyrings\" -type f -name '*.asc' 2>/dev/null -exec sh -c 'echo \"KEYRING:$1\"; gpg --with-colons \"$1\" 2>/dev/null || true' _ {} \\;",
     "requires_command": "gpg",
     "output": [
         "KEYRING:/etc/apt/trusted.gpg.d/ubuntu-keyring.gpg",

tests/operations/apt.key/add.json

@@ -1,12 +1,6 @@
 {
     "args": ["mykey"],
     "facts": {
-        "apt.AptKeys": {},
-        "gpg.GpgKey": {
-            "src=mykey": {
-                "abc": {}
-            }
-        },
         "files.Directory": {
             "path=/etc/apt/keyrings": null
         },

tests/operations/apt.key/add_exists.json

@@ -1,18 +1,10 @@
 {
     "args": ["mykey"],
     "facts": {
-        "apt.AptKeys": {
-            "abc": {}
-        },
-        "gpg.GpgKey": {
-            "src=mykey": {
-                "abc": {}
-            }
-        },
-        "files.Directory": {
-            "path=/etc/apt/keyrings": null
+        "files.File": {
+            "path=/etc/apt/keyrings/mykey.gpg": {"mode": 644, "user": "root", "group": "root"}
         }
     },
     "commands": [],
-    "noop_description": "All keys from mykey are already available in the apt keychain"
+    "noop_description": "GPG keyring /etc/apt/keyrings/mykey.gpg already exists"
 }

tests/operations/apt.key/add_keyserver.json

@@ -4,10 +4,9 @@
         "keyid": "abc"
     },
     "facts": {
-        "apt.AptKeys": {},
         "files.Directory": {
             "path=/etc/apt/keyrings": null,
-            "path=/tmp/pyinfra-gpg-empfile_": null
+            "path=_tempfile_": null
         },
         "files.File": {
             "path=/etc/apt/keyrings/keyserver-abc.gpg": null
@@ -16,10 +15,10 @@
     "commands": [
         "mkdir -p /etc/apt/keyrings",
         "chmod 755 /etc/apt/keyrings",
-        "mkdir -p /tmp/pyinfra-gpg-empfile_",
-        "chmod 700 /tmp/pyinfra-gpg-empfile_",
-        "export GNUPGHOME=\"/tmp/pyinfra-gpg-empfile_\" && gpg --batch --keyserver \"key-server.net\" --recv-keys abc",
-        "export GNUPGHOME=\"/tmp/pyinfra-gpg-empfile_\" && gpg --batch --export abc > \"/etc/apt/keyrings/keyserver-abc.gpg\"",
+        "mkdir -p _tempfile_",
+        "chmod 700 _tempfile_",
+        "export GNUPGHOME=\"_tempfile_\" && gpg --batch --keyserver \"key-server.net\" --recv-keys abc",
+        "export GNUPGHOME=\"_tempfile_\" && gpg --batch --export abc > \"/etc/apt/keyrings/keyserver-abc.gpg\"",
         "mkdir -p /etc/apt/keyrings",
         "touch /etc/apt/keyrings/keyserver-abc.gpg",
         "chmod 644 /etc/apt/keyrings/keyserver-abc.gpg"

tests/operations/apt.key/add_keyserver_exists.json

@@ -4,14 +4,35 @@
         "keyid": ["abc", "def"]
     },
     "facts": {
-        "apt.AptKeys": {
-            "abc": {},
-            "def": {}
+        "gpg.GpgKeyrings": {
+            "directories=['/etc/apt/keyrings']": {
+                "/etc/apt/keyrings/keyserver-abc-def.gpg": {
+                    "format": "gpg",
+                    "keys": {
+                        "ABC": {
+                            "validity": "-",
+                            "length": 4096,
+                            "subkeys": {},
+                            "fingerprint": "ABCABC1234567890ABCABC1234567890ABCABC12",
+                            "uid_hash": "AAABBBCCC",
+                            "uid": "Test Vendor A <a@vendor.com>"
+                        },
+                        "DEF": {
+                            "validity": "-",
+                            "length": 4096,
+                            "subkeys": {},
+                            "fingerprint": "DEFDEF1234567890DEFDEF1234567890DEFDEF12",
+                            "uid_hash": "DDDEEEFFF",
+                            "uid": "Test Vendor B <b@vendor.com>"
+                        }
+                    }
+                }
+            }
         },
-        "files.Directory": {
-            "path=/etc/apt/keyrings": null
+        "files.File": {
+            "path=/etc/apt/keyrings/keyserver-abc-def.gpg": {"mode": 644, "user": "root", "group": "root"}
         }
     },
     "commands": [],
-    "noop_description": "Keys abc, def are already available in the apt keychain"
+    "noop_description": "GPG keys ['abc', 'def'] already exist in /etc/apt/keyrings/keyserver-abc-def.gpg"
 }