From 37dd9da34781a9d97451da30bb9367ddc40e05b5 Mon Sep 17 00:00:00 2001
From: jeanluc <lkubb@protonmail.com>
Date: Tue, 23 Apr 2024 17:46:49 +0200
Subject: [PATCH 1/5] Minor optimizations

---
 src/saltext/vault/utils/vault/cache.py   |  6 ++---
 src/saltext/vault/utils/vault/client.py  |  2 +-
 src/saltext/vault/utils/vault/factory.py | 31 ++++++++++++------------
 src/saltext/vault/utils/vault/helpers.py |  2 +-
 src/saltext/vault/utils/vault/kv.py      |  4 +--
 src/saltext/vault/utils/vault/leases.py  |  4 +--
 tests/functional/modules/test_vault.py   |  2 +-
 tests/integration/modules/test_vault.py  |  2 +-
 tests/integration/runners/test_vault.py  |  6 ++---
 tests/unit/utils/vault/test_factory.py   |  4 +--
 10 files changed, 32 insertions(+), 31 deletions(-)

diff --git a/src/saltext/vault/utils/vault/cache.py b/src/saltext/vault/utils/vault/cache.py
index d2a646a6..ef9a8a8a 100644
--- a/src/saltext/vault/utils/vault/cache.py
+++ b/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:
diff --git a/src/saltext/vault/utils/vault/client.py b/src/saltext/vault/utils/vault/client.py
index 64b4047b..3e70a71f 100644
--- a/src/saltext/vault/utils/vault/client.py
+++ b/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)
diff --git a/src/saltext/vault/utils/vault/factory.py b/src/saltext/vault/utils/vault/factory.py
index ee9fb07a..5e5b6ff4 100644
--- a/src/saltext/vault/utils/vault/factory.py
+++ b/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:
diff --git a/src/saltext/vault/utils/vault/helpers.py b/src/saltext/vault/utils/vault/helpers.py
index 40bb7ad1..9a1f14e7 100644
--- a/src/saltext/vault/utils/vault/helpers.py
+++ b/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"
         )
diff --git a/src/saltext/vault/utils/vault/kv.py b/src/saltext/vault/utils/vault/kv.py
index 86636ce4..834f464d 100644
--- a/src/saltext/vault/utils/vault/kv.py
+++ b/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?"
diff --git a/src/saltext/vault/utils/vault/leases.py b/src/saltext/vault/utils/vault/leases.py
index b357a7ca..9ff4560b 100644
--- a/src/saltext/vault/utils/vault/leases.py
+++ b/src/saltext/vault/utils/vault/leases.py
@@ -165,7 +165,7 @@ def to_dict(self):
         """
         Return a dict of all contained attributes.
         """
-        return self.__dict__
+        return copy.deepcopy(self.__dict__)
 
 
 class VaultLease(BaseLease):
@@ -492,7 +492,7 @@ def renew(self, lease, increment=None, raise_all_errors=True, _store=True):
             return new_lease
         return ret
 
-    def renew_cached(self, match, increment=None):
+    def renew_cached(self, match="*", increment=None):
         """
         Renew cached leases.
 
diff --git a/tests/functional/modules/test_vault.py b/tests/functional/modules/test_vault.py
index 6cf96f55..af2c2b4a 100644
--- a/tests/functional/modules/test_vault.py
+++ b/tests/functional/modules/test_vault.py
@@ -52,7 +52,7 @@ def vault(modules, vault_container_version):  # pylint: disable=unused-argument
             for secret in vault_list_secrets(secret_path):
                 vault_delete_secret(f"{secret_path}/{secret}", metadata=True)
         policies = vault_list_policies()
-        for policy in ["functional_test_policy", "policy_write_test"]:
+        for policy in ("functional_test_policy", "policy_write_test"):
             if policy in policies:
                 vault_delete_policy(policy)
 
diff --git a/tests/integration/modules/test_vault.py b/tests/integration/modules/test_vault.py
index aa3f85ab..d359b2f0 100644
--- a/tests/integration/modules/test_vault.py
+++ b/tests/integration/modules/test_vault.py
@@ -105,7 +105,7 @@ def vault_testing_data(vault_container_version):  # pylint: disable=unused-argum
     finally:
         vault_delete_secret("secret/test/jvmdump/ssh_key")
         vault_delete_secret("secret/test/jenkins/master/ssh_key")
-        for x in ["deleteme", "write"]:
+        for x in ("deleteme", "write"):
             if x in vault_list_secrets("secret/test"):
                 vault_delete_secret(f"secret/test/{x}")
 
diff --git a/tests/integration/runners/test_vault.py b/tests/integration/runners/test_vault.py
index 3caf6064..32ebf143 100644
--- a/tests/integration/runners/test_vault.py
+++ b/tests/integration/runners/test_vault.py
@@ -547,7 +547,7 @@ def minion_conn_cachedir(vault_salt_call_cli):
 def missing_auth_cache(minion_conn_cachedir):
     token_cachefile = minion_conn_cachedir / "session" / "__token.p"
     secret_id_cachefile = minion_conn_cachedir / "secret_id.p"
-    for file in [secret_id_cachefile, token_cachefile]:
+    for file in (secret_id_cachefile, token_cachefile):
         if file.exists():
             file.unlink()
     yield
@@ -842,12 +842,12 @@ def test_issue_param_overrides_work(
         ret = vault_salt_run_cli.run("vault.show_approle", overriding_vault_salt_minion.id)
         assert ret.returncode == 0
         assert ret.data
-        for val in [
+        for val in (
             "token_explicit_max_ttl",
             "token_num_uses",
             "secret_id_num_uses",
             "secret_id_ttl",
-        ]:
+        ):
             assert ret.data[val] == issue_overrides[val]
 
     def test_impersonating_master_does_not_override_issue_param_overrides(
diff --git a/tests/unit/utils/vault/test_factory.py b/tests/unit/utils/vault/test_factory.py
index a50bdd4c..074ca755 100644
--- a/tests/unit/utils/vault/test_factory.py
+++ b/tests/unit/utils/vault/test_factory.py
@@ -315,9 +315,9 @@ def _cache(context, cbank, ckey, *args, **kwargs):  # pylint: disable=unused-arg
             token.get.return_value = None
             approle = Mock(spec=vauth_cache)
             approle.get.return_value = None
-            if cached_what in ["token", "both"]:
+            if cached_what in ("token", "both"):
                 token.get.return_value = vault.VaultToken(**token_auth["auth"])
-            if cached_what in ["secret_id", "both"]:
+            if cached_what in ("secret_id", "both"):
                 approle.get.return_value = vault.VaultSecretId(**secret_id_response["data"])
             return token if ckey == vfactory.TOKEN_CKEY else approle
 

From b1979c06f727b09269027cca59147d778df6bd2d Mon Sep 17 00:00:00 2001
From: jeanluc <lkubb@protonmail.com>
Date: Tue, 23 Apr 2024 17:58:21 +0200
Subject: [PATCH 2/5] Allow to list cached leases in LeaseStore

---
 src/saltext/vault/utils/vault/leases.py | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/src/saltext/vault/utils/vault/leases.py b/src/saltext/vault/utils/vault/leases.py
index 9ff4560b..e564068f 100644
--- a/src/saltext/vault/utils/vault/leases.py
+++ b/src/saltext/vault/utils/vault/leases.py
@@ -436,6 +436,24 @@ def list(self):
         """
         return self.cache.list()
 
+    def list_info(self, match="*"):
+        """
+        List cached leases.
+        match
+            Only list cached leases whose ckey matches this glob pattern.
+            Defaults to ``*``.
+        """
+        ret = {}
+        for ckey in self.list():
+            if not fnmatch.fnmatch(ckey, match):
+                continue
+            lease = self.cache.get(ckey, flush=False)
+            info = lease.to_dict()
+            # do not leak auth data
+            info.pop("data", None)
+            ret[ckey] = info
+        return ret
+
     def lookup(self, lease):
         """
         Lookup lease meta information.

From 9ce9eb71e6d401d749a93a27eecd8ac27184533c Mon Sep 17 00:00:00 2001
From: jeanluc <lkubb@protonmail.com>
Date: Wed, 24 Apr 2024 01:25:01 +0200
Subject: [PATCH 3/5] Allow leases to have default handling values

Also add a lot more tests for the LeaseStore, it was the least well
tested part of the repo.
---
 src/saltext/vault/utils/vault/leases.py | 178 +++++++---
 tests/unit/utils/vault/conftest.py      |  32 +-
 tests/unit/utils/vault/test_leases.py   | 429 +++++++++++++++++++++++-
 3 files changed, 584 insertions(+), 55 deletions(-)

diff --git a/src/saltext/vault/utils/vault/leases.py b/src/saltext/vault/utils/vault/leases.py
index e564068f..b048d323 100644
--- a/src/saltext/vault/utils/vault/leases.py
+++ b/src/saltext/vault/utils/vault/leases.py
@@ -173,11 +173,46 @@ class VaultLease(BaseLease):
     Data object representing a Vault lease.
     """
 
-    def __init__(self, lease_id, data, **kwargs):
+    def __init__(
+        self,
+        lease_id,
+        data,
+        min_ttl=None,
+        renew_increment=None,
+        revoke_delay=None,
+        meta=None,
+        **kwargs,
+    ):
         # save lease-associated data
         self.data = data
+        # save metadata used by the engine and beacon modules
+        self.min_ttl = min_ttl
+        self.renew_increment = renew_increment
+        self.revoke_delay = revoke_delay
+        self.meta = meta
         super().__init__(lease_id, **kwargs)
 
+    def is_valid_for(self, valid_for=None, blur=0):
+        """
+        Checks whether the lease is valid.
+
+        valid_for
+            Check whether the entity will still be valid in the future.
+            This can be an integer, which will be interpreted as seconds, or a
+            time string using the same format as Vault does:
+            Suffix ``s`` for seconds, ``m`` for minutes, ``h`` for hours, ``d`` for days.
+            Defaults to the minimum TTL that was set on the lease
+            when creating it or 0.
+
+        blur
+            Allow undercutting ``valid_for`` for this amount of seconds.
+            Defaults to 0.
+        """
+        return super().is_valid_for(
+            valid_for=valid_for if valid_for is not None else (self.min_ttl or 0),
+            blur=blur,
+        )
+
 
 class VaultToken(UseCountMixin, AccessorMixin, BaseLease):
     """
@@ -340,11 +375,12 @@ def __init__(self, client, cache, expire_events=None):
     def get(
         self,
         ckey,
-        valid_for=0,
+        valid_for=None,
         renew=True,
         renew_increment=None,
         renew_blur=2,
-        revoke=60,
+        revoke=None,
+        check_server=False,
     ):
         """
         Return cached lease or None.
@@ -357,7 +393,7 @@ def get(
             This can be an integer, which will be interpreted as seconds, or a
             time string using the same format as Vault does:
             Suffix ``s`` for seconds, ``m`` for minutes, ``h`` for hours, ``d`` for days.
-            Defaults to 0.
+            Defaults to the minimum TTL that was set on the lease when creating it or 0.
 
             .. note::
 
@@ -384,7 +420,12 @@ def get(
         revoke
             If the lease is not valid for ``valid_for`` and renewals
             are disabled or impossible, attempt to have Vault revoke the lease
-            after this amount of time and flush the cache. Defaults to 60s.
+            after this amount of time and flush the cache. Defaults to the
+            revocation delay that was set on the lease when creating it or 60s.
+
+        check_server
+            Check on the Vault server whether the lease is still active and was not
+            revoked early. Defaults to false.
         """
         if renew_increment is not None and timestring_map(valid_for) > timestring_map(
             renew_increment
@@ -396,18 +437,45 @@ def get(
         def check_revoke(lease):
             if self.expire_events is not None:
                 self.expire_events(
-                    tag=f"vault/lease/{ckey}/expire", data={"valid_for_less": valid_for}
+                    tag=f"vault/lease/{ckey}/expire",
+                    data={
+                        "valid_for_less": valid_for
+                        if valid_for is not None
+                        else (lease.min_ttl or 0),
+                        "meta": lease.meta,
+                    },
                 )
-            if revoke:
+            if revoke is None or revoke:
                 self.revoke(lease, delta=revoke)
             return None
 
         # Since we can renew leases, do not check for future validity in cache
         lease = self.cache.get(ckey, flush=bool(revoke))
-        if lease is not None:
-            self.lease_id_ckey_cache[str(lease)] = ckey
-        if lease is None or lease.is_valid_for(valid_for):
+        if lease is None:
+            return lease
+        self.lease_id_ckey_cache[str(lease)] = ckey
+        # Leases can have an associated min_ttl, which should be taken into
+        # account here. It is not done on the lease class to not break internal
+        # expectations.
+        effective_min_validity = max(
+            timestring_map(valid_for) or 0, timestring_map(lease.min_ttl) or 0
+        )
+        if renew_increment is not None and effective_min_validity > timestring_map(renew_increment):
+            log.warning(
+                f"renew_increment is set to '{renew_increment}', which is lower than "
+                f"the minimum TTL of '{lease.min_ttl}' on lease '{ckey}'. "
+                f"Dropping requested renew_increment for lease '{ckey}'."
+            )
+            renew_increment = None
+        if lease.is_valid_for(effective_min_validity):
+            if check_server:
+                try:
+                    # TODO: Save the updated info?
+                    self.lookup(lease)
+                except VaultNotFoundError:
+                    return check_revoke(lease)
             return lease
+
         if not renew:
             return check_revoke(lease)
         try:
@@ -415,39 +483,52 @@ def check_revoke(lease):
         except VaultNotFoundError:
             # The cached lease was already revoked
             return check_revoke(lease)
-        if not lease.is_valid_for(valid_for, blur=renew_blur):
+        if not lease.is_valid_for(effective_min_validity, blur=renew_blur):
             if renew_increment is not None:
                 # valid_for cannot possibly be respected
                 return check_revoke(lease)
             # Maybe valid_for is greater than the default validity period, so check if
             # the lease can be renewed by valid_for
             try:
-                lease = self.renew(lease, increment=valid_for, raise_all_errors=False)
+                lease = self.renew(lease, increment=effective_min_validity, raise_all_errors=False)
             except VaultNotFoundError:
                 # The cached lease was already revoked
                 return check_revoke(lease)
-            if not lease.is_valid_for(valid_for, blur=renew_blur):
+            if not lease.is_valid_for(effective_min_validity, blur=renew_blur):
                 return check_revoke(lease)
         return lease
 
     def list(self):
         """
-        List all cached leases.
+        List all known cache keys of cached leases.
         """
         return self.cache.list()
 
+    def _list_cached_leases(self, match="*", flush=False):
+        """
+        Helper for functions that operate on the cached leases.
+        """
+        leases = []
+        for ckey in self.list():
+            if not fnmatch.fnmatch(ckey, match):
+                continue
+            lease = self.cache.get(ckey, flush=flush)
+            if lease is None:
+                continue
+            self.lease_id_ckey_cache[str(lease)] = ckey
+            leases.append((ckey, lease))
+        return leases
+
     def list_info(self, match="*"):
         """
         List cached leases.
+
         match
             Only list cached leases whose ckey matches this glob pattern.
             Defaults to ``*``.
         """
         ret = {}
-        for ckey in self.list():
-            if not fnmatch.fnmatch(ckey, match):
-                continue
-            lease = self.cache.get(ckey, flush=False)
+        for ckey, lease in self._list_cached_leases(match=match, flush=False):
             info = lease.to_dict()
             # do not leak auth data
             info.pop("data", None)
@@ -463,7 +544,12 @@ def lookup(self, lease):
         """
         endpoint = "sys/leases/lookup"
         payload = {"lease_id": str(lease)}
-        return self.client.post(endpoint, payload=payload)
+        try:
+            return self.client.post(endpoint, payload=payload)
+        except VaultInvocationError as err:
+            if "invalid lease" not in str(err):
+                raise
+            raise VaultNotFoundError(str(err)) from err
 
     def renew(self, lease, increment=None, raise_all_errors=True, _store=True):
         """
@@ -486,19 +572,25 @@ def renew(self, lease, increment=None, raise_all_errors=True, _store=True):
         """
         endpoint = "sys/leases/renew"
         payload = {"lease_id": str(lease)}
-        if increment is not None:
-            payload["increment"] = int(timestring_map(increment))
         if not isinstance(lease, VaultLease) and lease in self.lease_id_ckey_cache:
             lease = self.cache.get(self.lease_id_ckey_cache[lease], flush=False)
             if lease is None:
                 raise VaultNotFoundError("Lease is already expired")
+        if increment is not None:
+            payload["increment"] = int(timestring_map(increment))
+        if isinstance(lease, VaultLease) and lease.renew_increment is not None:
+            payload["increment"] = max(
+                int(timestring_map(lease.renew_increment)), payload.get("increment", 0)
+            )
         try:
             ret = self.client.post(endpoint, payload=payload)
         except VaultException as err:
             if raise_all_errors or not isinstance(lease, VaultLease):
                 raise
-            if isinstance(err, VaultNotFoundError):
-                raise
+            if isinstance(err, VaultInvocationError):
+                if "lease not found" not in str(err):
+                    raise
+                raise VaultNotFoundError(str(err)) from err
             return lease
 
         if _store and isinstance(lease, VaultLease):
@@ -523,15 +615,11 @@ def renew_cached(self, match="*", increment=None):
             point of time onwards. Can also be used to reduce the validity period.
             The server might not honor this increment.
             Can be an integer (seconds) or a time string like ``1h``. Optional.
+            If unset, defaults to the renewal increment that was set when creating
+            the lease.
         """
         failed = []
-        for ckey in self.list():
-            if not fnmatch.fnmatch(ckey, match):
-                continue
-            lease = self.cache.get(ckey, flush=True)
-            if lease is None:
-                continue
-            self.lease_id_ckey_cache[str(lease)] = ckey
+        for ckey, lease in self._list_cached_leases(match=match, flush=True):
             try:
                 self.renew(lease, increment=increment)
             except (VaultPermissionDeniedError, VaultNotFoundError) as err:
@@ -542,7 +630,7 @@ def renew_cached(self, match="*", increment=None):
             raise VaultException(f"Failed renewing some leases: {list(failed)}")
         return True
 
-    def revoke(self, lease, delta=60):
+    def revoke(self, lease, delta=None):
         """
         Revoke a lease. Will also remove the cached lease,
         if it has been requested from this LeaseStore before.
@@ -553,13 +641,20 @@ def revoke(self, lease, delta=60):
         delta
             Time after which the lease should be requested
             to be revoked by Vault.
-            Defaults to 60s.
-        """
+            Defaults to the revocation delay that was set when creating
+            the lease or 60s.
+        """
+        if delta is None:
+            if isinstance(lease, VaultLease) and lease.revoke_delay is not None:
+                delta = lease.revoke_delay
+            else:
+                delta = 60
         try:
             # 0 would attempt a complete renewal
             self.renew(lease, increment=delta or 1, _store=False)
-        except VaultNotFoundError:
-            pass
+        except VaultInvocationError as err:
+            if "lease not found" not in str(err):
+                raise
 
         if str(lease) in self.lease_id_ckey_cache:
             self.cache.flush(self.lease_id_ckey_cache.pop(str(lease)))
@@ -568,7 +663,7 @@ def revoke(self, lease, delta=60):
     def revoke_cached(
         self,
         match="*",
-        delta=60,
+        delta=None,
         flush_on_failure=True,
     ):
         """
@@ -580,20 +675,15 @@ def revoke_cached(
 
         delta
             Time after which the leases should be revoked by Vault.
-            Defaults to 60s.
+            Defaults to the revocation delay that was set when creating
+            the lease(s) or 60s.
 
         flush_on_failure
             If a revocation fails, remove the lease from cache anyways.
             Defaults to true.
         """
         failed = []
-        for ckey in self.list():
-            if not fnmatch.fnmatch(ckey, match):
-                continue
-            lease = self.cache.get(ckey, flush=True)
-            if lease is None:
-                continue
-            self.lease_id_ckey_cache[str(lease)] = ckey
+        for ckey, lease in self._list_cached_leases(match=match, flush=True):
             try:
                 self.revoke(lease, delta=delta)
             except VaultPermissionDeniedError:
diff --git a/tests/unit/utils/vault/conftest.py b/tests/unit/utils/vault/conftest.py
index b31d3901..93b24c70 100644
--- a/tests/unit/utils/vault/conftest.py
+++ b/tests/unit/utils/vault/conftest.py
@@ -415,8 +415,9 @@ def lease_response():
 
 
 @pytest.fixture
-def lease():
-    return {
+def lease(request):
+    overrides = getattr(request, "param", {})
+    base = {
         "id": "database/creds/testrole/abcd",
         "lease_id": "database/creds/testrole/abcd",
         "renewable": True,
@@ -427,6 +428,33 @@ def lease():
             "username": "test",
             "password": "test",
         },
+        "meta": None,
+        "min_ttl": None,
+        "revoke_delay": None,
+        "renew_increment": None,
+    }
+    base.update(overrides)
+    return base
+
+
+@pytest.fixture
+def lease_lookup_response():
+    return {
+        "request_id": "852e6e12-7bf5-0f38-896d-4ced16c8c8f0",
+        "lease_id": "",
+        "renewable": False,
+        "lease_duration": 0,
+        "data": {
+            "expire_time": 2000,
+            "id": "database/creds/testrole/abcd",
+            "issue_time": 0,
+            "last_renewal": 0,
+            "renewable": True,
+            "ttl": 2000,
+        },
+        "wrap_info": None,
+        "warnings": None,
+        "auth": None,
     }
 
 
diff --git a/tests/unit/utils/vault/test_leases.py b/tests/unit/utils/vault/test_leases.py
index 4239f2ec..3d7fa85c 100644
--- a/tests/unit/utils/vault/test_leases.py
+++ b/tests/unit/utils/vault/test_leases.py
@@ -1,3 +1,4 @@
+import logging
 from unittest.mock import call
 from unittest.mock import Mock
 from unittest.mock import patch
@@ -46,6 +47,19 @@ def store_valid(store, lease, lease_renewed_response):
     return store
 
 
+@pytest.fixture
+def store_multi(store, lease, lease_renewed_response):
+    lease = vleases.VaultLease(**lease)
+    lease_2 = lease.with_renewed(id="foobar", lease_id="foobar")
+    lease_3 = lease.with_renewed(id="barbaz", lease_id="barbaz")
+    leases = {"test_1": lease, "test_12": lease_2, "test_3": lease_3}
+    store.cache.exists.side_effect = lambda x, **y: x in leases
+    store.cache.get.side_effect = lambda x, **y: leases[x]
+    store.cache.list.return_value = list(leases)
+    store.client.post.return_value = lease_renewed_response
+    return store
+
+
 @pytest.mark.parametrize(
     "creation_time",
     [
@@ -175,7 +189,7 @@ def test_vault_approle_secret_id_is_valid_accounts_for_time(duration, offset, ex
         "secret_id": "test-secret-id",
         "renewable": False,
         "creation_time": 0,
-        "expire_time": duration,
+        "expiration_time": duration,
         "secret_id_num_uses": 0,
         "secret_id_ttl": duration,
     }
@@ -236,7 +250,7 @@ def test_get_cached_valid(self, store_valid, lease):
     @pytest.mark.parametrize("valid_for", [2000, pytest.param(2002, id="2002_renewal_leeway")])
     def test_get_valid_renew_default_period(self, store_valid, lease, valid_for):
         """
-        Ensure renewals are attempted by default, cache is updated accordingly
+        Ensure renewals are attempted by default, the cache is updated accordingly
         and validity checks after renewal allow for a little leeway to account
         for latency.
         """
@@ -250,15 +264,85 @@ def test_get_valid_renew_default_period(self, store_valid, lease, valid_for):
         store_valid.cache.store.assert_called_once_with("test", ret)
         store_valid.expire_events.assert_not_called()
 
+    @pytest.mark.parametrize("lease", ({"min_ttl": 3000},), indirect=True)
+    def test_get_valid_renew_min_ttl_on_lease(
+        self, store_valid, lease, lease_renewed_extended_response
+    ):
+        """
+        Ensure that even if the cached lease fulfills the requested valid_for,
+        that a renewal is attempted if min_ttl is higher.
+        """
+        store_valid.client.post.return_value = lease_renewed_extended_response
+        ret = store_valid.get("test", valid_for=1200)
+        lease["duration"] = lease["expire_time"] = 3000
+        assert ret == lease
+        store_valid.client.post.assert_called_once_with(
+            "sys/leases/renew", payload={"lease_id": lease["id"]}
+        )
+        store_valid.cache.flush.assert_not_called()
+        store_valid.cache.store.assert_called_once_with("test", ret)
+        store_valid.expire_events.assert_not_called()
+
+    @pytest.mark.parametrize("lease", ({"min_ttl": 3000},), indirect=True)
+    def test_get_valid_renew_min_ttl_on_lease_undercut(
+        self, store_valid, lease, lease_renewed_response
+    ):
+        """
+        Ensure that even if the cached lease fulfills the requested valid_for
+        and a renewal still undercuts it, nothing is returned
+        """
+        store_valid.client.post.return_value = lease_renewed_response
+        ret = store_valid.get("test", valid_for=1200)
+        assert ret is None
+        store_valid.client.post.assert_has_calls(
+            (
+                call(
+                    "sys/leases/renew",
+                    payload={"lease_id": lease["id"]},
+                ),
+                call(
+                    "sys/leases/renew",
+                    payload={"lease_id": lease["id"], "increment": 3000},
+                ),
+            )
+        )
+        store_valid.cache.flush.assert_called_once_with("test")
+        store_valid.expire_events.assert_called_once()
+
+    @pytest.mark.parametrize("lease", ({"renew_increment": 2005},), indirect=True)
+    def test_get_valid_renew_lease_default_period(self, store_valid, lease):
+        """
+        Ensure that renewals without increment override respect the value
+        set on the lease during creation.
+        """
+        ret = store_valid.get("test", valid_for=2000)
+        lease["duration"] = lease["expire_time"] = 2000
+        assert ret == lease
+        store_valid.client.post.assert_called_once_with(
+            "sys/leases/renew",
+            payload={"lease_id": lease["id"], "increment": lease["renew_increment"]},
+        )
+        store_valid.cache.flush.assert_not_called()
+        store_valid.cache.store.assert_called_once_with("test", ret)
+        store_valid.expire_events.assert_not_called()
+
+    @pytest.mark.parametrize(
+        "lease", ({}, {"renew_increment": 1999}, {"renew_increment": 2001}), indirect=True
+    )
     def test_get_valid_renew_increment(self, store_valid, lease):
         """
-        Ensure renew_increment is honored when renewing.
+        Ensure renew_increment is honored when renewing and overrides the one
+        set on the lease only if it does not undercut it.
         """
         ret = store_valid.get("test", valid_for=1400, renew_increment=2000)
         lease["duration"] = lease["expire_time"] = 2000
         assert ret == lease
         store_valid.client.post.assert_called_once_with(
-            "sys/leases/renew", payload={"lease_id": lease["id"], "increment": 2000}
+            "sys/leases/renew",
+            payload={
+                "lease_id": lease["id"],
+                "increment": max(lease["renew_increment"] or 0, 2000),
+            },
         )
         store_valid.cache.flush.assert_not_called()
         store_valid.cache.store.assert_called_once_with("test", ret)
@@ -266,7 +350,7 @@ def test_get_valid_renew_increment(self, store_valid, lease):
 
     def test_get_valid_renew_increment_insufficient(self, store_valid, lease):
         """
-        Ensure that when renewal_increment is set, valid_for is respected and that
+        Ensure that when renew_increment is set, valid_for is respected and that
         a second renewal using valid_for as increment is not attempted when the
         Vault server does not allow renewals for at least valid_for.
         If an event factory was passed, an event should be sent.
@@ -287,8 +371,42 @@ def test_get_valid_renew_increment_insufficient(self, store_valid, lease):
         )
         store_valid.cache.flush.assert_called_once_with("test")
         store_valid.expire_events.assert_called_once_with(
-            tag="vault/lease/test/expire", data={"valid_for_less": 2100}
+            tag="vault/lease/test/expire", data={"valid_for_less": 2100, "meta": None}
+        )
+
+    @pytest.mark.parametrize("lease", ({"min_ttl": 2100},), indirect=True)
+    def test_get_valid_renew_increment_undercuts_min_ttl(
+        self, store_valid, lease, caplog, lease_renewed_response, lease_renewed_extended_response
+    ):
+        """
+        Ensure that when a renew_increment undercuts the lease's default
+        min_ttl, the renewal is first attempted without a renew_increment
+        and if it still undercuts the min_ttl, a second one is attempted
+        with the min_ttl as the increment.
+        The user should be warned about the invalid request.
+        """
+        store_valid.client.post.side_effect = (
+            lease_renewed_response,
+            lease_renewed_extended_response,
+        )
+        with caplog.at_level(logging.WARNING):
+            ret = store_valid.get("test", valid_for=1500, renew_increment=1600)
+            assert "Dropping requested renew_increment" in caplog.text
+        lease["duration"] = lease["expire_time"] = 3000
+        assert ret == lease
+
+        store_valid.client.post.assert_has_calls(
+            (
+                call("sys/leases/renew", payload={"lease_id": lease["id"]}),
+                call(
+                    "sys/leases/renew",
+                    payload={"lease_id": lease["id"], "increment": lease["min_ttl"]},
+                ),
+            )
         )
+        store_valid.cache.flush.assert_not_called()
+        store_valid.cache.store.assert_called_with("test", ret)
+        store_valid.expire_events.assert_not_called()
 
     @pytest.mark.parametrize("valid_for", [3000, pytest.param(3002, id="3002_renewal_leeway")])
     def test_get_valid_renew_valid_for(
@@ -327,7 +445,7 @@ def test_get_valid_renew_valid_for(
     def test_get_valid_not_renew(self, store_valid, lease):
         """
         Currently valid leases should not be returned if they undercut
-        valid_for. By default, revocation should be attempted and cache
+        valid_for. By default, revocation should be attempted and the cache
         should be flushed. If an event factory was passed, an event should be sent.
         """
         ret = store_valid.get("test", valid_for=2000, renew=False)
@@ -338,8 +456,27 @@ def test_get_valid_not_renew(self, store_valid, lease):
         )
         store_valid.cache.flush.assert_called_once_with("test")
         store_valid.expire_events.assert_called_once_with(
-            tag="vault/lease/test/expire", data={"valid_for_less": 2000}
+            tag="vault/lease/test/expire", data={"valid_for_less": 2000, "meta": None}
+        )
+
+    @pytest.mark.parametrize(
+        "lease", ({}, {"revoke_delay": 1200}, {"revoke_delay": 1400}), indirect=True
+    )
+    @pytest.mark.parametrize("revoke", (None, 1100, 1500))
+    def test_get_valid_revoke(self, store_valid, lease, revoke):
+        """
+        Ensure revoke is honored when revoking and overrides the revoke_delay
+        set on the lease. Also ensure the revoke_delay on the lease is
+        the default.
+        """
+        ret = store_valid.get("test", valid_for=3500, revoke=revoke)
+        assert ret is None
+        store_valid.client.post.assert_called_with(
+            "sys/leases/renew",
+            payload={"lease_id": lease["id"], "increment": revoke or lease["revoke_delay"] or 60},
         )
+        store_valid.cache.flush.assert_called_once_with("test")
+        store_valid.expire_events.assert_called()
 
     def test_get_valid_not_flush(self, store_valid):
         """
@@ -353,5 +490,279 @@ def test_get_valid_not_flush(self, store_valid):
         store_valid.client.post.assert_not_called()
         store_valid.cache.store.assert_not_called()
         store_valid.expire_events.assert_called_once_with(
-            tag="vault/lease/test/expire", data={"valid_for_less": 2000}
+            tag="vault/lease/test/expire", data={"valid_for_less": 2000, "meta": None}
+        )
+
+    @pytest.mark.parametrize("check_server", (False, True))
+    @pytest.mark.parametrize("expected", (False, True))
+    def test_get_check_server(
+        self,
+        store_valid,
+        lease,
+        expected,
+        check_server,
+        lease_lookup_response,
+        lease_renewed_response,
+    ):
+        """
+        Ensure that "valid" leases are validated with the server only if requested.
+        """
+        if expected:
+            store_valid.client.post.return_value = lease_lookup_response
+        else:
+            store_valid.client.post.side_effect = (
+                vault.VaultInvocationError("invalid lease"),
+                lease_renewed_response,
+            )
+        ret = store_valid.get("test", valid_for=1300, check_server=check_server)
+        assert (ret == lease) is (expected or not check_server)
+        if check_server:
+            if expected:
+                store_valid.client.post.assert_called_once_with(
+                    "sys/leases/lookup", payload={"lease_id": lease["id"]}
+                )
+                store_valid.cache.flush.assert_not_called()
+                store_valid.expire_events.assert_not_called()
+            else:
+                store_valid.client.post.assert_has_calls(
+                    (
+                        call("sys/leases/lookup", payload={"lease_id": lease["id"]}),
+                        call(
+                            "sys/leases/renew",
+                            payload={"lease_id": lease["id"], "increment": 60},
+                        ),
+                    )
+                )
+                store_valid.cache.flush.assert_called_once_with("test")
+                store_valid.expire_events.assert_called()
+        else:
+            store_valid.client.post.assert_not_called()
+            store_valid.expire_events.assert_not_called()
+        store_valid.cache.store.assert_not_called()
+
+    @pytest.mark.parametrize("on_call", (1, 2))
+    def test_get_renew_already_revoked(self, store_valid, on_call, lease_renewed_response):
+        """
+        Ensure there's no crash when a renewal is attempted on a revoked lease
+        and that the cache is flushed.
+        """
+        store_valid.client.post.side_effect = (
+            (on_call - 1) * [lease_renewed_response]
+            + [vault.VaultInvocationError("lease not found")]
+            + [lease_renewed_response]
         )
+        ret = store_valid.get("test", valid_for=3000)
+        assert ret is None
+        store_valid.cache.flush.assert_called_once_with("test")
+        store_valid.expire_events.assert_called()
+
+    @pytest.mark.parametrize("as_str", (False, True))
+    def test_revoke_already_revoked(self, store_valid, lease, as_str):
+        """
+        A notfound exception should not matter for revoking.
+        """
+        store_valid.client.post.side_effect = vault.VaultInvocationError("lease not found")
+        lease = vault.VaultLease(**lease)
+        if as_str:
+            lease = str(lease)
+        assert store_valid.revoke(lease) is True
+
+    def test_revoke_delay_should_have_a_minimum_of_1(self, store_valid, lease):
+        """
+        Since revocation internally uses renewals by setting the lease
+        validity to a low value, a minimum value of 1 must be enforced,
+        otherwise the lease is renewed by its default TTL.
+        """
+        assert store_valid.revoke(lease["id"], delta=0) is True
+        store_valid.client.post.assert_called_once_with(
+            "sys/leases/renew", payload={"lease_id": lease["id"], "increment": 1}
+        )
+
+    def test_list_info(self, store_multi, lease):
+        """
+        Test listing cached leases.
+        """
+        ret = store_multi.list_info()
+        assert set(ret) == {"test_1", "test_12", "test_3"}
+        lease.pop("data")
+        assert ret["test_1"] == lease
+        assert ret["test_12"]["lease_id"] == "foobar"
+        assert ret["test_3"]["lease_id"] == "barbaz"
+
+    def test_list_info_match(self, store_multi, lease):
+        """
+        Test listing cached leases with a glob.
+        """
+        ret = store_multi.list_info(match="test_1*")
+        assert set(ret) == {"test_1", "test_12"}
+        lease.pop("data")
+        assert ret["test_1"] == lease
+        assert ret["test_12"]["lease_id"] == "foobar"
+
+    def test_list_info_expired(self, store_multi, lease):
+        """
+        The list should not contain expired leases, but they should
+        not be flushed from cache during the call.
+        """
+        prev = store_multi.cache.get.side_effect
+        store_multi.cache.get.side_effect = lambda x, **y: prev(x, **y) if x != "test_12" else None
+        ret = store_multi.list_info(match="test_1*")
+        assert set(ret) == {"test_1"}
+        lease.pop("data")
+        assert ret["test_1"] == lease
+        store_multi.cache.get.assert_called_with("test_12", flush=False)
+
+    def test_renew_cached(self, store_multi, lease):
+        """
+        Test renewing all cached leases.
+        """
+        assert store_multi.renew_cached() is True
+        store_multi.client.post.assert_has_calls(
+            (
+                call(
+                    "sys/leases/renew",
+                    payload={"lease_id": lease["id"]},
+                ),
+                call(
+                    "sys/leases/renew",
+                    payload={"lease_id": "foobar"},
+                ),
+                call(
+                    "sys/leases/renew",
+                    payload={"lease_id": "barbaz"},
+                ),
+            )
+        )
+
+    def test_renew_cached_match(self, store_multi, lease):
+        """
+        Test renewing all cached leases that match a glob.
+        """
+        assert store_multi.renew_cached(match="test_1*") is True
+        store_multi.client.post.assert_has_calls(
+            (
+                call(
+                    "sys/leases/renew",
+                    payload={"lease_id": lease["id"]},
+                ),
+                call(
+                    "sys/leases/renew",
+                    payload={"lease_id": "foobar"},
+                ),
+            )
+        )
+
+    def test_renew_cached_expired(self, store_multi, lease):
+        """
+        Expired leases should be skipped and they should be flushed
+        from cache during the call.
+        """
+        prev = store_multi.cache.get.side_effect
+        store_multi.cache.get.side_effect = lambda x, **y: prev(x, **y) if x != "test_12" else None
+        assert store_multi.renew_cached(match="test_1*") is True
+        store_multi.client.post.assert_called_once_with(
+            "sys/leases/renew", payload={"lease_id": lease["id"]}
+        )
+        store_multi.cache.get.assert_called_with("test_12", flush=True)
+
+    @pytest.mark.parametrize("exc", (vault.VaultNotFoundError, vault.VaultPermissionDeniedError))
+    def test_renew_cached_exception(self, store_multi, exc, lease_renewed_response, lease):
+        """
+        Ensure that exceptions are reraised after everything has been processed.
+        """
+        store_multi.client.post.side_effect = [lease_renewed_response, exc, lease_renewed_response]
+        with pytest.raises(
+            vault.VaultException, match=r"Failed renewing some leases: \['test_12'\]"
+        ):
+            store_multi.renew_cached()
+        store_multi.client.post.assert_has_calls(
+            (
+                call(
+                    "sys/leases/renew",
+                    payload={"lease_id": lease["id"]},
+                ),
+                call(
+                    "sys/leases/renew",
+                    payload={"lease_id": "foobar"},
+                ),
+                call(
+                    "sys/leases/renew",
+                    payload={"lease_id": "barbaz"},
+                ),
+            )
+        )
+
+    def test_revoke_cached(self, store_multi, lease):
+        """
+        Test revoking all cached leases.
+        """
+        assert store_multi.revoke_cached() is True
+        store_multi.client.post.assert_has_calls(
+            (
+                call(
+                    "sys/leases/renew",
+                    payload={"lease_id": lease["id"], "increment": 60},
+                ),
+                call(
+                    "sys/leases/renew",
+                    payload={"lease_id": "foobar", "increment": 60},
+                ),
+                call(
+                    "sys/leases/renew",
+                    payload={"lease_id": "barbaz", "increment": 60},
+                ),
+            )
+        )
+
+    def test_revoke_cached_match(self, store_multi, lease):
+        """
+        Test revoking all cached leases that match a glob.
+        """
+        assert store_multi.revoke_cached(match="test_1*", delta=120) is True
+        store_multi.client.post.assert_has_calls(
+            (
+                call(
+                    "sys/leases/renew",
+                    payload={"lease_id": lease["id"], "increment": 120},
+                ),
+                call(
+                    "sys/leases/renew",
+                    payload={"lease_id": "foobar", "increment": 120},
+                ),
+            )
+        )
+
+    def test_revoke_cached_expired(self, store_multi, lease):
+        """
+        Expired leases should be skipped and they should be flushed
+        from cache during the call.
+        """
+        prev = store_multi.cache.get.side_effect
+        store_multi.cache.get.side_effect = lambda x, **y: prev(x, **y) if x != "test_12" else None
+        assert store_multi.revoke_cached(match="test_1*") is True
+        store_multi.client.post.assert_called_once_with(
+            "sys/leases/renew", payload={"lease_id": lease["id"], "increment": 60}
+        )
+        store_multi.cache.get.assert_called_with("test_12", flush=True)
+
+    @pytest.mark.parametrize("flush_on_failure", (False, True))
+    def test_revoke_cached_exception(self, store_multi, lease_renewed_response, flush_on_failure):
+        """
+        Ensure that exceptions are reraised after everything has been processed.
+        The cached leases should only be flushed from cache if it was requested.
+        """
+        store_multi.client.post.side_effect = [
+            lease_renewed_response,
+            vault.VaultPermissionDeniedError,
+            lease_renewed_response,
+        ]
+        with pytest.raises(
+            vault.VaultException, match=r"Failed revoking some leases: \['test_12'\]"
+        ):
+            store_multi.revoke_cached(flush_on_failure=flush_on_failure)
+        if flush_on_failure:
+            store_multi.cache.flush.assert_has_calls(
+                (call("test_1"), call("test_12"), call("test_3"))
+            )
+        else:
+            store_multi.cache.flush.assert_has_calls((call("test_1"), call("test_3")))

From 7c1922e081e7d368bb11f6ef2083253989951fd0 Mon Sep 17 00:00:00 2001
From: jeanluc <lkubb@protonmail.com>
Date: Wed, 24 Apr 2024 12:12:13 +0200
Subject: [PATCH 4/5] Add test for lease metadata in expiry event, changelog

---
 changelog/45.fixed.md                       |  1 +
 changelog/46.added.md                       |  1 +
 changelog/47.added.md                       |  1 +
 changelog/48.added.md                       |  1 +
 changelog/49.added.md                       |  1 +
 src/saltext/vault/utils/vault/cache.py      |  9 ++--
 src/saltext/vault/utils/vault/exceptions.py |  4 ++
 src/saltext/vault/utils/vault/leases.py     | 56 ++++++++++++++-------
 tests/unit/utils/vault/test_cache.py        |  9 +++-
 tests/unit/utils/vault/test_leases.py       | 30 ++++++++---
 10 files changed, 83 insertions(+), 30 deletions(-)
 create mode 100644 changelog/45.fixed.md
 create mode 100644 changelog/46.added.md
 create mode 100644 changelog/47.added.md
 create mode 100644 changelog/48.added.md
 create mode 100644 changelog/49.added.md

diff --git a/changelog/45.fixed.md b/changelog/45.fixed.md
new file mode 100644
index 00000000..363cfb87
--- /dev/null
+++ b/changelog/45.fixed.md
@@ -0,0 +1 @@
+Fixed a crash when renewing/revoking leases that have been revoked on the Vault server early
diff --git a/changelog/46.added.md b/changelog/46.added.md
new file mode 100644
index 00000000..8f8b298b
--- /dev/null
+++ b/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
diff --git a/changelog/47.added.md b/changelog/47.added.md
new file mode 100644
index 00000000..48637147
--- /dev/null
+++ b/changelog/47.added.md
@@ -0,0 +1 @@
+Implemented setting per-lease defaults of lifecycle parameters
diff --git a/changelog/48.added.md b/changelog/48.added.md
new file mode 100644
index 00000000..1452dc0d
--- /dev/null
+++ b/changelog/48.added.md
@@ -0,0 +1 @@
+Implemented caching arbitrary metadata together with a lease and included it in expiry events
diff --git a/changelog/49.added.md b/changelog/49.added.md
new file mode 100644
index 00000000..9b8eb643
--- /dev/null
+++ b/changelog/49.added.md
@@ -0,0 +1 @@
+Added a LeaseStore method for listing cached lease information
diff --git a/src/saltext/vault/utils/vault/cache.py b/src/saltext/vault/utils/vault/cache.py
index ef9a8a8a..767cc29a 100644
--- a/src/saltext/vault/utils/vault/cache.py
+++ b/src/saltext/vault/utils/vault/cache.py
@@ -286,7 +286,7 @@ 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
 
 
@@ -294,6 +294,7 @@ 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
diff --git a/src/saltext/vault/utils/vault/exceptions.py b/src/saltext/vault/utils/vault/exceptions.py
index 0517e6eb..b439f7a2 100644
--- a/src/saltext/vault/utils/vault/exceptions.py
+++ b/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):
     """
diff --git a/src/saltext/vault/utils/vault/leases.py b/src/saltext/vault/utils/vault/leases.py
index b048d323..11d34bac 100644
--- a/src/saltext/vault/utils/vault/leases.py
+++ b/src/saltext/vault/utils/vault/leases.py
@@ -76,6 +76,10 @@ def is_valid_for(self, valid_for=0, blur=0):
             return True
         return abs(delta) <= blur
 
+    @property
+    def ttl_left(self):
+        return max(self.expire_time - round(time.time()), 0)
+
 
 class UseCountMixin:
     """
@@ -171,6 +175,26 @@ def to_dict(self):
 class VaultLease(BaseLease):
     """
     Data object representing a Vault lease.
+
+    Optional parameters in addition to the required``lease_id`` and ``data``:
+
+    min_ttl
+        When requesting this lease from the LeaseStore, ensure it is
+        valid for at least this amount of time, even if the
+        passed ``valid_for`` parameter is less.
+
+    renew_increment
+        When renewing this lease, instead of the lease's default TTL,
+        default to this increment.
+
+    revoke_delay
+        When revoking this lease, instead of the default value of 60,
+        default to this amount of time before having the Vault server
+        revoke it.
+
+    meta
+        Cache arbitrary metadata together with the lease. It will
+        be included in expiry events.
     """
 
     def __init__(
@@ -185,10 +209,11 @@ def __init__(
     ):
         # save lease-associated data
         self.data = data
-        # save metadata used by the engine and beacon modules
+        # lifecycle default parameters
         self.min_ttl = min_ttl
         self.renew_increment = renew_increment
         self.revoke_delay = revoke_delay
+        # metadata that is included in expiry events
         self.meta = meta
         super().__init__(lease_id, **kwargs)
 
@@ -434,17 +459,14 @@ def get(
                 "When renew_increment is set, it must be at least valid_for to make sense"
             )
 
-        def check_revoke(lease):
+        def check_revoke(lease, min_valid, validity_override=None):
             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 (lease.min_ttl or 0),
-                        "meta": lease.meta,
-                    },
-                )
+                event_data = {
+                    "valid_for_less": round(min_valid),
+                    "ttl": validity_override if validity_override is not None else lease.ttl_left,
+                    "meta": lease.meta,
+                }
+                self.expire_events(tag=f"vault/lease/{ckey}/expire", data=event_data)
             if revoke is None or revoke:
                 self.revoke(lease, delta=revoke)
             return None
@@ -473,29 +495,29 @@ def check_revoke(lease):
                     # TODO: Save the updated info?
                     self.lookup(lease)
                 except VaultNotFoundError:
-                    return check_revoke(lease)
+                    return check_revoke(lease, effective_min_validity, 0)
             return lease
 
         if not renew:
-            return check_revoke(lease)
+            return check_revoke(lease, effective_min_validity)
         try:
             lease = self.renew(lease, increment=renew_increment, raise_all_errors=False)
         except VaultNotFoundError:
             # The cached lease was already revoked
-            return check_revoke(lease)
+            return check_revoke(lease, effective_min_validity, 0)
         if not lease.is_valid_for(effective_min_validity, blur=renew_blur):
             if renew_increment is not None:
                 # valid_for cannot possibly be respected
-                return check_revoke(lease)
+                return check_revoke(lease, effective_min_validity)
             # Maybe valid_for is greater than the default validity period, so check if
             # the lease can be renewed by valid_for
             try:
                 lease = self.renew(lease, increment=effective_min_validity, raise_all_errors=False)
             except VaultNotFoundError:
                 # The cached lease was already revoked
-                return check_revoke(lease)
+                return check_revoke(lease, effective_min_validity, 0)
             if not lease.is_valid_for(effective_min_validity, blur=renew_blur):
-                return check_revoke(lease)
+                return check_revoke(lease, effective_min_validity)
         return lease
 
     def list(self):
diff --git a/tests/unit/utils/vault/test_cache.py b/tests/unit/utils/vault/test_cache.py
index f7937c44..988e74d8 100644
--- a/tests/unit/utils/vault/test_cache.py
+++ b/tests/unit/utils/vault/test_cache.py
@@ -614,8 +614,10 @@ def test_store(self, lease):
             cache.store("ckey", lease_)
             store.assert_called_once_with("ckey", lease_.to_dict())
 
+    @pytest.mark.parametrize("lease", ({}, {"meta": {"foo": "bar"}}), indirect=True)
+    @pytest.mark.usefixtures("cached_outdated")
     def test_expire_events_with_get(
-        self, events, cached_outdated, cbank, ckey, lease
+        self, events, lease
     ):  # pylint: disable=unused-argument, disable-msg=too-many-arguments
         """
         Ensure internal flushing is disabled when the object is initialized
@@ -624,4 +626,7 @@ def test_expire_events_with_get(
         cache = vcache.VaultLeaseCache({}, "cbank", expire_events=events)
         ret = cache.get("ckey", 10)
         assert ret is None
-        events.assert_called_once_with(tag="vault/lease/ckey/expire", data={"valid_for_less": 10})
+        events.assert_called_once_with(
+            tag="vault/lease/ckey/expire",
+            data={"valid_for_less": 10, "ttl_left": 6, "meta": lease["meta"]},
+        )
diff --git a/tests/unit/utils/vault/test_leases.py b/tests/unit/utils/vault/test_leases.py
index 3d7fa85c..c988eac6 100644
--- a/tests/unit/utils/vault/test_leases.py
+++ b/tests/unit/utils/vault/test_leases.py
@@ -307,7 +307,10 @@ def test_get_valid_renew_min_ttl_on_lease_undercut(
             )
         )
         store_valid.cache.flush.assert_called_once_with("test")
-        store_valid.expire_events.assert_called_once()
+        store_valid.expire_events.assert_called_once_with(
+            tag="vault/lease/test/expire",
+            data={"valid_for_less": 3000, "ttl": 2000, "meta": lease["meta"]},
+        )
 
     @pytest.mark.parametrize("lease", ({"renew_increment": 2005},), indirect=True)
     def test_get_valid_renew_lease_default_period(self, store_valid, lease):
@@ -371,7 +374,7 @@ def test_get_valid_renew_increment_insufficient(self, store_valid, lease):
         )
         store_valid.cache.flush.assert_called_once_with("test")
         store_valid.expire_events.assert_called_once_with(
-            tag="vault/lease/test/expire", data={"valid_for_less": 2100, "meta": None}
+            tag="vault/lease/test/expire", data={"valid_for_less": 2100, "ttl": 2000, "meta": None}
         )
 
     @pytest.mark.parametrize("lease", ({"min_ttl": 2100},), indirect=True)
@@ -442,11 +445,13 @@ def test_get_valid_renew_valid_for(
         store_valid.cache.store.assert_called_with("test", ret)
         store_valid.expire_events.assert_not_called()
 
+    @pytest.mark.parametrize("lease", ({}, {"meta": {"foo": "bar"}}), indirect=True)
     def test_get_valid_not_renew(self, store_valid, lease):
         """
         Currently valid leases should not be returned if they undercut
         valid_for. By default, revocation should be attempted and the cache
-        should be flushed. If an event factory was passed, an event should be sent.
+        should be flushed. If an event factory was passed, an event should be sent
+        which includes the per-lease metadata.
         """
         ret = store_valid.get("test", valid_for=2000, renew=False)
         assert ret is None
@@ -456,7 +461,8 @@ def test_get_valid_not_renew(self, store_valid, lease):
         )
         store_valid.cache.flush.assert_called_once_with("test")
         store_valid.expire_events.assert_called_once_with(
-            tag="vault/lease/test/expire", data={"valid_for_less": 2000, "meta": None}
+            tag="vault/lease/test/expire",
+            data={"valid_for_less": 2000, "ttl": 1337, "meta": lease["meta"]},
         )
 
     @pytest.mark.parametrize(
@@ -476,7 +482,10 @@ def test_get_valid_revoke(self, store_valid, lease, revoke):
             payload={"lease_id": lease["id"], "increment": revoke or lease["revoke_delay"] or 60},
         )
         store_valid.cache.flush.assert_called_once_with("test")
-        store_valid.expire_events.assert_called()
+        store_valid.expire_events.assert_called_once_with(
+            tag="vault/lease/test/expire",
+            data={"valid_for_less": 3500, "ttl": 2000, "meta": lease["meta"]},
+        )
 
     def test_get_valid_not_flush(self, store_valid):
         """
@@ -490,7 +499,7 @@ def test_get_valid_not_flush(self, store_valid):
         store_valid.client.post.assert_not_called()
         store_valid.cache.store.assert_not_called()
         store_valid.expire_events.assert_called_once_with(
-            tag="vault/lease/test/expire", data={"valid_for_less": 2000, "meta": None}
+            tag="vault/lease/test/expire", data={"valid_for_less": 2000, "ttl": 1337, "meta": None}
         )
 
     @pytest.mark.parametrize("check_server", (False, True))
@@ -534,7 +543,10 @@ def test_get_check_server(
                     )
                 )
                 store_valid.cache.flush.assert_called_once_with("test")
-                store_valid.expire_events.assert_called()
+                store_valid.expire_events.assert_called_once_with(
+                    tag="vault/lease/test/expire",
+                    data={"valid_for_less": 1300, "ttl": 0, "meta": lease["meta"]},
+                )
         else:
             store_valid.client.post.assert_not_called()
             store_valid.expire_events.assert_not_called()
@@ -554,7 +566,9 @@ def test_get_renew_already_revoked(self, store_valid, on_call, lease_renewed_res
         ret = store_valid.get("test", valid_for=3000)
         assert ret is None
         store_valid.cache.flush.assert_called_once_with("test")
-        store_valid.expire_events.assert_called()
+        store_valid.expire_events.assert_called_once_with(
+            tag="vault/lease/test/expire", data={"valid_for_less": 3000, "ttl": 0, "meta": None}
+        )
 
     @pytest.mark.parametrize("as_str", (False, True))
     def test_revoke_already_revoked(self, store_valid, lease, as_str):

From 605e1eda05f845f9b6ae017bdfc0c44c99f1c9fb Mon Sep 17 00:00:00 2001
From: jeanluc <lkubb@protonmail.com>
Date: Wed, 24 Apr 2024 15:46:04 +0200
Subject: [PATCH 5/5] Add versionadded tags to new funcs/params

---
 src/saltext/vault/utils/vault/leases.py | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/src/saltext/vault/utils/vault/leases.py b/src/saltext/vault/utils/vault/leases.py
index 11d34bac..74f6e1f7 100644
--- a/src/saltext/vault/utils/vault/leases.py
+++ b/src/saltext/vault/utils/vault/leases.py
@@ -78,6 +78,11 @@ def is_valid_for(self, valid_for=0, blur=0):
 
     @property
     def ttl_left(self):
+        """
+        .. versionadded:: 1.1.0
+
+        Return the time in seconds until the lease expires.
+        """
         return max(self.expire_time - round(time.time()), 0)
 
 
@@ -183,18 +188,26 @@ class VaultLease(BaseLease):
         valid for at least this amount of time, even if the
         passed ``valid_for`` parameter is less.
 
+        .. versionadded:: 1.1.0
+
     renew_increment
         When renewing this lease, instead of the lease's default TTL,
         default to this increment.
 
+        .. versionadded:: 1.1.0
+
     revoke_delay
         When revoking this lease, instead of the default value of 60,
         default to this amount of time before having the Vault server
         revoke it.
 
+        .. versionadded:: 1.1.0
+
     meta
         Cache arbitrary metadata together with the lease. It will
         be included in expiry events.
+
+        .. versionadded:: 1.1.0
     """
 
     def __init__(
@@ -451,6 +464,8 @@ def get(
         check_server
             Check on the Vault server whether the lease is still active and was not
             revoked early. Defaults to false.
+
+            .. versionadded:: 1.1.0
         """
         if renew_increment is not None and timestring_map(valid_for) > timestring_map(
             renew_increment
@@ -543,6 +558,8 @@ def _list_cached_leases(self, match="*", flush=False):
 
     def list_info(self, match="*"):
         """
+        .. versionadded:: 1.1.0
+
         List cached leases.
 
         match