Merge pull request #44 from lkubb/lease-handling

Fixes #45
Fixes #46
Fixes #47
Fixes #48
Fixes #49

changelog/45.fixed.md

@@ -0,0 +1 @@
+Fixed a crash when renewing/revoking leases that have been revoked on the Vault server early

changelog/46.added.md

@@ -0,0 +1 @@
+Added an optional switch for validating cached leases with the Vault server before returning them from the LeaseStore

changelog/47.added.md

@@ -0,0 +1 @@
+Implemented setting per-lease defaults of lifecycle parameters

changelog/48.added.md

@@ -0,0 +1 @@
+Implemented caching arbitrary metadata together with a lease and included it in expiry events

changelog/49.added.md

@@ -0,0 +1 @@
+Added a LeaseStore method for listing cached lease information

src/saltext/vault/utils/vault/cache.py

@@ -48,7 +48,7 @@ def _get_config_cache(opts, context, cbank, ckey="config"):
 def _get_cache_backend(config, opts):
     if config["cache"]["backend"] == "session":
         return None
-    if config["cache"]["backend"] in ["localfs", "disk", "file"]:
+    if config["cache"]["backend"] in ("localfs", "disk", "file"):
         # cache.Cache does not allow setting the type of cache by param
         local_opts = copy.copy(opts)
         local_opts["cache"] = "localfs"
@@ -62,10 +62,10 @@ def _get_cache_bank(opts, force_local=False, connection=True, session=False):
     minion_id = None
     # force_local is necessary because pillar compilation would otherwise
     # leak tokens between master and minions
-    if not force_local and hlp._get_salt_run_type(opts) in [
+    if not force_local and hlp._get_salt_run_type(opts) in (
         hlp.SALT_RUNTYPE_MASTER_IMPERSONATING,
         hlp.SALT_RUNTYPE_MASTER_PEER_RUN,
-    ]:
+    ):
         minion_id = opts["grains"]["id"]
     prefix = "vault" if minion_id is None else f"minions/{minion_id}/vault"
     if session:
@@ -286,14 +286,15 @@ def _check_validity(self, lease_data, valid_for=0):
                 log.debug("Using cached lease.")
                 return lease
         if self.expire_events is not None:
-            raise VaultLeaseExpired()
+            raise VaultLeaseExpired(lease)
         return None
 
 
 class VaultLeaseCache(LeaseCacheMixin, CommonCache):
     """
     Handles caching of Vault leases. Supports multiple cache keys.
     Checks whether cached leases are still valid before returning.
+    Does not enforce for per-lease ``min_ttl``.
     """
 
     def get(self, ckey, valid_for=0, flush=True):
@@ -306,14 +307,16 @@ def get(self, ckey, valid_for=0, flush=True):
             return data
         try:
             ret = self._check_validity(data, valid_for=valid_for)
-        except VaultLeaseExpired:
+        except VaultLeaseExpired as err:
             if self.expire_events is not None:
                 self.expire_events(
                     tag=f"vault/lease/{ckey}/expire",
                     data={
                         "valid_for_less": valid_for
                         if valid_for is not None
-                        else data.get("min_ttl") or 0,
+                        else err.lease.min_ttl or 0,
+                        "ttl_left": err.lease.ttl_left,
+                        "meta": err.lease.meta,
                     },
                 )
             ret = None

src/saltext/vault/utils/vault/client.py

@@ -326,7 +326,7 @@ def _raise_status(self, res):
             raise VaultUnsupportedOperationError(errors)
         if res.status_code == 412:
             raise VaultPreconditionFailedError(errors)
-        if res.status_code in [500, 502]:
+        if res.status_code in (500, 502):
             raise VaultServerError(errors)
         if res.status_code == 503:
             raise VaultUnavailableError(errors)

src/saltext/vault/utils/vault/exceptions.py

@@ -15,6 +15,10 @@ class VaultLeaseExpired(VaultException):
     Raised when a cached lease is reported to be expired locally.
     """
 
+    def __init__(self, lease):
+        super().__init__()
+        self.lease = lease
+
 
 class VaultAuthExpired(VaultException):
     """

src/saltext/vault/utils/vault/factory.py

@@ -171,7 +171,6 @@ def clear_cache(opts, context, ckey=None, connection=True, session=False, force_
     cbank = vcache._get_cache_bank(
         opts, connection=connection, session=session, force_local=force_local
     )
-    client, config = _build_revocation_client(opts, context, force_local=force_local)
     if (
         not ckey
         or (not (connection or session) and ckey == "connection")
@@ -197,10 +196,10 @@ def clear_cache(opts, context, ckey=None, connection=True, session=False, force_
                         config["cache"]["expire_events"]
                         and not force_local
                         and hlp._get_salt_run_type(opts)
-                        not in [
+                        not in (
                             hlp.SALT_RUNTYPE_MASTER_IMPERSONATING,
                             hlp.SALT_RUNTYPE_MASTER_PEER_RUN,
-                        ]
+                        )
                     ):
                         scope = cbank.split("/")[-1]
                         _get_event(opts)(data={"scope": scope}, tag=f"vault/cache/{scope}/clear")
@@ -254,10 +253,10 @@ def update_config(opts, context, keep_session=False):
         significantly.
         Defaults to False.
     """
-    if hlp._get_salt_run_type(opts) in [
+    if hlp._get_salt_run_type(opts) in (
         hlp.SALT_RUNTYPE_MASTER,
         hlp.SALT_RUNTYPE_MINION_LOCAL,
-    ]:
+    ):
         # local configuration is not cached
         return True
     connection_cbank = vcache._get_cache_bank(opts)
@@ -322,7 +321,7 @@ def _build_authd_client(opts, context, force_local=False):
                 # For remote sources, we would needlessly request one, so don't.
                 if (
                     hlp._get_salt_run_type(opts)
-                    in [hlp.SALT_RUNTYPE_MASTER, hlp.SALT_RUNTYPE_MINION_LOCAL]
+                    in (hlp.SALT_RUNTYPE_MASTER, hlp.SALT_RUNTYPE_MINION_LOCAL)
                     or force_local
                 ):
                     secret_id = _fetch_secret_id(
@@ -346,7 +345,7 @@ def _build_authd_client(opts, context, force_local=False):
         client = vclient.AuthenticatedVaultClient(
             auth, session=unauthd_client.session, **config["server"]
         )
-    elif config["auth"]["method"] in ["token", "wrapped_token"]:
+    elif config["auth"]["method"] in ("token", "wrapped_token"):
         token = _fetch_token(
             config,
             opts,
@@ -374,7 +373,7 @@ def _build_revocation_client(opts, context, force_local=False):
     # Disregard a possibly returned locally configured token since
     # it is cached with metadata if it has been used. Also, we do not want
     # to revoke statically configured tokens anyways.
-    config, _, _ = _get_connection_config(
+    config, _, unauthd_client = _get_connection_config(
         connection_cbank, opts, context, force_local=force_local, pre_flush=True
     )
     if config is None:
@@ -395,13 +394,15 @@ def _build_revocation_client(opts, context, force_local=False):
     if token is None:
         return None, None
     auth = vauth.VaultTokenAuth(token=token, cache=token_cache)
-    client = vclient.AuthenticatedVaultClient(auth, **config["server"])
+    client = vclient.AuthenticatedVaultClient(
+        auth, session=unauthd_client.session, **config["server"]
+    )
     return client, config
 
 
 def _get_connection_config(cbank, opts, context, force_local=False, pre_flush=False, update=False):
     if (
-        hlp._get_salt_run_type(opts) in [hlp.SALT_RUNTYPE_MASTER, hlp.SALT_RUNTYPE_MINION_LOCAL]
+        hlp._get_salt_run_type(opts) in (hlp.SALT_RUNTYPE_MASTER, hlp.SALT_RUNTYPE_MINION_LOCAL)
         or force_local
     ):
         # only cache config fetched from remote
@@ -516,7 +517,7 @@ def cache_or_fetch(config, opts, secret_id_cache, unwrap_client):
         return secret_id
 
     if (
-        hlp._get_salt_run_type(opts) in [hlp.SALT_RUNTYPE_MASTER, hlp.SALT_RUNTYPE_MINION_LOCAL]
+        hlp._get_salt_run_type(opts) in (hlp.SALT_RUNTYPE_MASTER, hlp.SALT_RUNTYPE_MINION_LOCAL)
         or force_local
     ):
         secret_id = config["auth"]["secret_id"]
@@ -571,7 +572,7 @@ def cache_or_fetch(config, opts, token_cache, unwrap_client, embedded_token):
         return token
 
     if (
-        hlp._get_salt_run_type(opts) in [hlp.SALT_RUNTYPE_MASTER, hlp.SALT_RUNTYPE_MINION_LOCAL]
+        hlp._get_salt_run_type(opts) in (hlp.SALT_RUNTYPE_MASTER, hlp.SALT_RUNTYPE_MINION_LOCAL)
         or force_local
     ):
         token = None
@@ -936,17 +937,17 @@ def parse_config(config, validate=True, opts=None, require_token=True):
     )
     # ttl, uses were used as configuration for issuance and minion overrides as well
     # as token meta information. The new configuration splits those semantics.
-    for old_token_conf, new_token_conf in [
+    for old_token_conf, new_token_conf in (
         ("ttl", "explicit_max_ttl"),
         ("uses", "num_uses"),
-    ]:
+    ):
         if old_token_conf in merged["auth"]:
             merged["issue"]["token"]["params"][new_token_conf] = merged["issue_params"][
                 new_token_conf
             ] = merged["auth"].pop(old_token_conf)
     # Those were found in the root namespace, but grouping them together
     # makes semantic and practical sense.
-    for old_server_conf in ["namespace", "url", "verify"]:
+    for old_server_conf in ("namespace", "url", "verify"):
         if old_server_conf in merged:
             merged["server"][old_server_conf] = merged.pop(old_server_conf)
     if "role_name" in merged:

src/saltext/vault/utils/vault/helpers.py

@@ -24,7 +24,7 @@ def _get_salt_run_type(opts):
         return SALT_RUNTYPE_MASTER
 
     config_location = opts.get("vault", {}).get("config_location")
-    if config_location and config_location not in ["local", "master"]:
+    if config_location and config_location not in ("local", "master"):
         raise InvalidConfigError(
             "Invalid vault configuration: config_location must be either local or master"
         )

src/saltext/vault/utils/vault/kv.py

@@ -186,7 +186,7 @@ def is_v2(self, path):
         if (
             ret["type"] == "kv"
             and path_metadata["options"] is not None
-            and path_metadata.get("options", {}).get("version", "1") in ["2"]
+            and path_metadata.get("options", {}).get("version", "1") == "2"
         ):
             ret["v2"] = True
             ret["data"] = self._v2_the_path(path, path_metadata.get("path", path))
@@ -203,7 +203,7 @@ def _v2_the_path(self, path, pfilter, ptype="data"):
         Given a path, a filter, and a path type, properly inject
         'data' or 'metadata' into the path.
         """
-        possible_types = ["data", "metadata", "delete", "destroy"]
+        possible_types = ("data", "metadata", "delete", "destroy")
         if ptype not in possible_types:
             raise AssertionError()
         msg = f"Path {path} already contains {ptype} in the right place - saltstack duct tape?"