Add test for lease metadata in expiry event, changelog

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

@@ -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/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/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):

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"]},
+        )

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):