Unbreak the gpg state module

changelog/63144.fixed.md

@@ -0,0 +1 @@
+Fixed gpg.receive_keys returns success on failed import

changelog/63153.fixed.md

@@ -0,0 +1 @@
+Fixed GPG state module always reports success without changes

changelog/63156.fixed.md

@@ -0,0 +1 @@
+Fixed GPG state module does not respect test mode

changelog/63159.fixed.md

@@ -0,0 +1 @@
+Fixed gpg.absent with gnupghome/user, fixed gpg.delete_key with gnupghome

changelog/65169.fixed.md

@@ -0,0 +1 @@
+Fixed `gpg.present` succeeds when the keyserver is unreachable

salt/modules/gpg.py

@@ -122,7 +122,7 @@ def _get_user_info(user=None):
             # if it doesn't exist then fall back to user Salt running as
             userinfo = _get_user_info()
         else:
-            raise SaltInvocationError("User {} does not exist".format(user))
+            raise SaltInvocationError(f"User {user} does not exist")
 
     return userinfo
 
@@ -556,15 +556,13 @@ def delete_key(
         return ret
 
     gpg = _create_gpg(user, gnupghome)
-    key = get_key(keyid, fingerprint, user)
+    key = get_key(keyid=keyid, fingerprint=fingerprint, user=user, gnupghome=gnupghome)
 
     def __delete_key(fingerprint, secret, use_passphrase):
-        if use_passphrase:
+        if secret and use_passphrase:
             gpg_passphrase = __salt__["pillar.get"]("gpg_passphrase")
             if not gpg_passphrase:
-                ret["res"] = False
-                ret["message"] = "gpg_passphrase not available in pillar."
-                return ret
+                return "gpg_passphrase not available in pillar."
             else:
                 out = gpg.delete_keys(fingerprint, secret, passphrase=gpg_passphrase)
         else:
@@ -573,7 +571,7 @@ def __delete_key(fingerprint, secret, use_passphrase):
 
     if key:
         fingerprint = key["fingerprint"]
-        skey = get_secret_key(keyid, fingerprint, user)
+        skey = get_secret_key(keyid, fingerprint, user, gnupghome=gnupghome)
         if skey:
             if not delete_secret:
                 ret["res"] = False
@@ -582,19 +580,29 @@ def __delete_key(fingerprint, secret, use_passphrase):
                 ] = "Secret key exists, delete first or pass delete_secret=True."
                 return ret
             else:
-                if str(__delete_key(fingerprint, True, use_passphrase)) == "ok":
+                out = __delete_key(fingerprint, True, use_passphrase)
+                if str(out) == "ok":
                     # Delete the secret key
-                    ret["message"] = "Secret key for {} deleted\n".format(fingerprint)
+                    ret["message"] = f"Secret key for {fingerprint} deleted\n"
+                else:
+                    ret["res"] = False
+                    ret[
+                        "message"
+                    ] = f"Failed to delete secret key for {fingerprint}: {out}"
+                    return ret
 
         # Delete the public key
-        if str(__delete_key(fingerprint, False, use_passphrase)) == "ok":
-            ret["message"] += "Public key for {} deleted".format(fingerprint)
-        ret["res"] = True
-        return ret
+        out = __delete_key(fingerprint, False, use_passphrase)
+        if str(out) == "ok":
+            ret["res"] = True
+            ret["message"] += f"Public key for {fingerprint} deleted"
+        else:
+            ret["res"] = False
+            ret["message"] += f"Failed to delete public key for {fingerprint}: {out}"
     else:
         ret["res"] = False
         ret["message"] = "Key not available in keychain."
-        return ret
+    return ret
 
 
 def get_key(keyid=None, fingerprint=None, user=None, gnupghome=None):
@@ -909,7 +917,7 @@ def receive_keys(keyserver=None, keys=None, user=None, gnupghome=None):
         salt '*' gpg.receive_keys keys=3FAD9F1E user=username
 
     """
-    ret = {"res": True, "changes": {}, "message": []}
+    ret = {"res": True, "message": []}
 
     gpg = _create_gpg(user, gnupghome)
 
@@ -920,18 +928,30 @@ def receive_keys(keyserver=None, keys=None, user=None, gnupghome=None):
         keys = keys.split(",")
 
     recv_data = gpg.recv_keys(keyserver, *keys)
-    for result in recv_data.results:
-        if "ok" in result:
-            if result["ok"] == "1":
-                ret["message"].append(
-                    "Key {} added to keychain".format(result["fingerprint"])
-                )
-            elif result["ok"] == "0":
-                ret["message"].append(
-                    "Key {} already exists in keychain".format(result["fingerprint"])
-                )
-        elif "problem" in result:
-            ret["message"].append("Unable to add key to keychain")
+    try:
+        if recv_data.results:
+            for result in recv_data.results:
+                if "ok" in result:
+                    if result["ok"] == "1":
+                        ret["message"].append(
+                            f"Key {result['fingerprint']} added to keychain"
+                        )
+                    elif result["ok"] == "0":
+                        ret["message"].append(
+                            f"Key {result['fingerprint']} already exists in keychain"
+                        )
+                elif "problem" in result:
+                    ret["message"].append(
+                        f"Unable to add key to keychain: {result.get('text', 'No further description')}"
+                    )
+
+        if not recv_data:
+            ret["res"] = False
+            ret["message"].append(f"GPG reported failure: {recv_data.stderr}")
+    except AttributeError:
+        ret["res"] = False
+        ret["message"] = ["Invalid return from python-gpg"]
+
     return ret
 
 
@@ -986,12 +1006,12 @@ def trust_key(keyid=None, fingerprint=None, trust_level=None, user=None):
             if key:
                 if "fingerprint" not in key:
                     ret["res"] = False
-                    ret["message"] = "Fingerprint not found for keyid {}".format(keyid)
+                    ret["message"] = f"Fingerprint not found for keyid {keyid}"
                     return ret
                 fingerprint = key["fingerprint"]
             else:
                 ret["res"] = False
-                ret["message"] = "KeyID {} not in GPG keychain".format(keyid)
+                ret["message"] = f"KeyID {keyid} not in GPG keychain"
                 return ret
         else:
             ret["res"] = False
@@ -1001,7 +1021,7 @@ def trust_key(keyid=None, fingerprint=None, trust_level=None, user=None):
     if trust_level not in _VALID_TRUST_LEVELS:
         return "ERROR: Valid trust levels - {}".format(",".join(_VALID_TRUST_LEVELS))
 
-    stdin = "{}:{}\n".format(fingerprint, NUM_TRUST_DICT[trust_level])
+    stdin = f"{fingerprint}:{NUM_TRUST_DICT[trust_level]}\n"
     cmd = [_gpg(), "--import-ownertrust"]
     _user = user
 
@@ -1397,7 +1417,7 @@ def encrypt(
     if result.ok:
         if not bare:
             if output:
-                ret["comment"] = "Encrypted data has been written to {}".format(output)
+                ret["comment"] = f"Encrypted data has been written to {output}"
             else:
                 ret["comment"] = result.data
         else:
@@ -1485,7 +1505,7 @@ def decrypt(
     if result.ok:
         if not bare:
             if output:
-                ret["comment"] = "Decrypted data has been written to {}".format(output)
+                ret["comment"] = f"Decrypted data has been written to {output}"
             else:
                 ret["comment"] = result.data
         else:

salt/states/gpg.py

@@ -8,6 +8,8 @@
 
 import logging
 
+import salt.utils.dictupdate
+
 log = logging.getLogger(__name__)
 
 _VALID_TRUST_VALUES = [
@@ -33,30 +35,28 @@ def present(
     name, keys=None, user=None, keyserver=None, gnupghome=None, trust=None, **kwargs
 ):
     """
-    Ensure GPG public key is present in keychain
+    Ensure a GPG public key is present in the GPG keychain.
 
     name
-        The unique name or keyid for the GPG public key.
+        The key ID of the GPG public key.
 
     keys
-        The keyId or keyIds to add to the GPG keychain.
+        The key ID or key IDs to add to the GPG keychain.
 
     user
-        Add GPG keys to the specified user's keychain
+        Add GPG keys to the specified user's keychain.
 
     keyserver
         The keyserver to retrieve the keys from.
 
     gnupghome
-        Override GNUPG Home directory
+        Override GnuPG home directory.
 
     trust
         Trust level for the key in the keychain,
-        ignored by default.  Valid trust levels:
+        ignored by default. Valid trust levels:
         expired, unknown, not_trusted, marginally,
         fully, ultimately
-
-
     """
 
     ret = {"name": name, "result": True, "changes": {}, "comment": []}
@@ -80,42 +80,62 @@ def present(
             if trust:
                 if trust in _VALID_TRUST_VALUES:
                     if current_keys[key]["trust"] != TRUST_MAP[trust]:
+                        if __opts__["test"]:
+                            ret["result"] = None
+                            ret["comment"].append(
+                                f"Would have set trust level for {key} to {trust}"
+                            )
+                            salt.utils.dictupdate.set_dict_key_value(
+                                ret, f"changes:{key}:trust", trust
+                            )
+                            continue
                         # update trust level
                         result = __salt__["gpg.trust_key"](
                             keyid=key,
                             trust_level=trust,
                             user=user,
                         )
-                        if "result" in result and not result["result"]:
-                            ret["result"] = result["result"]
-                            ret["comment"].append(result["comment"])
+                        if result["res"] is False:
+                            ret["result"] = result["res"]
+                            ret["comment"].append(result["message"])
                         else:
+                            salt.utils.dictupdate.set_dict_key_value(
+                                ret, f"changes:{key}:trust", trust
+                            )
                             ret["comment"].append(
-                                "Set trust level for {} to {}".format(key, trust)
+                                f"Set trust level for {key} to {trust}"
                             )
                     else:
                         ret["comment"].append(
-                            "GPG Public Key {} already in correct trust state".format(
-                                key
-                            )
+                            f"GPG Public Key {key} already in correct trust state"
                         )
                 else:
-                    ret["comment"].append("Invalid trust level {}".format(trust))
+                    ret["comment"].append(f"Invalid trust level {trust}")
 
-            ret["comment"].append("GPG Public Key {} already in keychain ".format(key))
+            ret["comment"].append(f"GPG Public Key {key} already in keychain")
 
         else:
+            if __opts__["test"]:
+                ret["result"] = None
+                ret["comment"].append(f"Would have added {key} to GPG keychain")
+                salt.utils.dictupdate.set_dict_key_value(
+                    ret, f"changes:{key}:added", True
+                )
+                continue
             result = __salt__["gpg.receive_keys"](
                 keyserver,
                 key,
                 user,
                 gnupghome,
             )
-            if "result" in result and not result["result"]:
-                ret["result"] = result["result"]
-                ret["comment"].append(result["comment"])
+            if result["res"] is False:
+                ret["result"] = result["res"]
+                ret["comment"].extend(result["message"])
             else:
-                ret["comment"].append("Adding {} to GPG keychain".format(name))
+                ret["comment"].append(f"Added {key} to GPG keychain")
+                salt.utils.dictupdate.set_dict_key_value(
+                    ret, f"changes:{key}:added", True
+                )
 
             if trust:
                 if trust in _VALID_TRUST_VALUES:
@@ -124,41 +144,38 @@ def present(
                         trust_level=trust,
                         user=user,
                     )
-                    if "result" in result and not result["result"]:
-                        ret["result"] = result["result"]
-                        ret["comment"].append(result["comment"])
+                    if result["res"] is False:
+                        ret["result"] = result["res"]
+                        ret["comment"].append(result["message"])
                     else:
-                        ret["comment"].append(
-                            "Set trust level for {} to {}".format(key, trust)
-                        )
+                        ret["comment"].append(f"Set trust level for {key} to {trust}")
                 else:
-                    ret["comment"].append("Invalid trust level {}".format(trust))
+                    ret["comment"].append(f"Invalid trust level {trust}")
 
     ret["comment"] = "\n".join(ret["comment"])
     return ret
 
 
 def absent(name, keys=None, user=None, gnupghome=None, **kwargs):
     """
-    Ensure GPG public key is absent in keychain
+    Ensure a GPG public key is absent from the keychain.
 
     name
-        The unique name or keyid for the GPG public key.
+        The key ID of the GPG public key.
 
     keys
-        The keyId or keyIds to add to the GPG keychain.
+        The key ID or key IDs to remove from the GPG keychain.
 
     user
-        Remove GPG keys from the specified user's keychain
+        Remove GPG keys from the specified user's keychain.
 
     gnupghome
-        Override GNUPG Home directory
-
+        Override GnuPG home directory.
     """
 
     ret = {"name": name, "result": True, "changes": {}, "comment": []}
 
-    _current_keys = __salt__["gpg.list_keys"]()
+    _current_keys = __salt__["gpg.list_keys"](user=user, gnupghome=gnupghome)
 
     current_keys = []
     for key in _current_keys:
@@ -172,17 +189,23 @@ def absent(name, keys=None, user=None, gnupghome=None, **kwargs):
 
     for key in keys:
         if key in current_keys:
+            if __opts__["test"]:
+                ret["result"] = None
+                ret["comment"].append(f"Would have deleted {key} from GPG keychain")
+                salt.utils.dictupdate.append_dict_key_value(ret, "changes:deleted", key)
+                continue
             result = __salt__["gpg.delete_key"](
-                key,
-                user,
-                gnupghome,
+                keyid=key,
+                user=user,
+                gnupghome=gnupghome,
             )
-            if "result" in result and not result["result"]:
-                ret["result"] = result["result"]
-                ret["comment"].append(result["comment"])
+            if result["res"] is False:
+                ret["result"] = result["res"]
+                ret["comment"].append(result["message"])
             else:
-                ret["comment"].append("Deleting {} from GPG keychain".format(name))
+                ret["comment"].append(f"Deleted {key} from GPG keychain")
+                salt.utils.dictupdate.append_dict_key_value(ret, "changes:deleted", key)
         else:
-            ret["comment"].append("{} not found in GPG keychain".format(name))
+            ret["comment"].append(f"{key} not found in GPG keychain")
     ret["comment"] = "\n".join(ret["comment"])
     return ret