Proper error handling when the CacheDisk file is not readable

Instead of just failing with an exception, if the msgpack in the
diskcache is unreadable, then it is treated as an empty cache (and the
pillar is rebuilt).

changelog/62527.fixed

@@ -0,0 +1 @@
+Fixes pillar where a corrupted CacheDisk file forces the pillar to be rebuilt

salt/utils/cache.py

@@ -142,10 +142,21 @@ class CacheDisk(CacheDict):
         """
         if not salt.utils.msgpack.HAS_MSGPACK or not os.path.exists(self._path):
             return
+
+        try:
             with salt.utils.files.fopen(self._path, "rb") as fp_:
                 cache = salt.utils.msgpack.load(
                     fp_, encoding=__salt_system_encoding__, raw=False
                 )
+        except FileNotFoundError:
+            # File was deleted after os.path.exists call above, treat as empty cache
+            return
+        except (salt.utils.msgpack.exceptions.UnpackException, ValueError) as exc:
+            # File is unreadable, treat as empty cache
+            if log.isEnabledFor(logging.DEBUG):
+                log.debug("Error reading cache file at %r: %s", self._path, exc)
+            return
+
         if "CacheDisk_cachetime" in cache:  # new format
             self._dict = cache["CacheDisk_data"]
             self._key_cache_time = cache["CacheDisk_cachetime"]

tests/pytests/unit/utils/test_cache.py

@@ -193,11 +193,11 @@ def test_refill_cache(minion_config, cache_mods_path):
     assert ret == context_copy
 
 
-def test_everything(tmp_path):
+def test_everything(cache_dir):
     """
     Make sure you can instantiate, add, update, remove, expire
     """
-    path = str(tmp_path / "cachedir")
+    path = str(cache_dir / "minion")
 
     # test instantiation
     cd = cache.CacheDisk(0.3, path)
@@ -230,14 +230,12 @@ def test_everything(tmp_path):
         b"\xc3\x83\xc2\xa6\xc3\x83\xc2\xb8\xc3\x83\xc2\xa5",
     ],
 )
-def test_unicode_error(tmp_path, data):
+def test_unicode_error(cache_dir, data):
     """
     Test when the data in the cache raises a UnicodeDecodeError
     we do not raise an error.
     """
-    path = tmp_path / "cachedir"
-    path.mkdir()
-    path = path / "minion"
+    path = cache_dir / "minion"
     path.touch()
     cache_data = {
         "CacheDisk_data": {
@@ -254,3 +252,25 @@ def test_unicode_error(tmp_path, data):
     with patch.object(salt.utils.msgpack, "load", return_value=cache_data):
         cd = cache.CacheDisk(0.3, str(path))
         assert cd._dict == cache_data
+
+
+def test_cache_corruption(cache_dir):
+    """
+    Tests if the CacheDisk can survive a corrupted cache file.
+    """
+    # Write valid cache file
+    cache_file = cache_dir / "minion"
+    cd = cache.CacheDisk(0.3, str(cache_file))
+    cd["test-key"] = "test-value"
+    del cd
+
+    # Add random string to the data to make the msgpack structure un-decodable
+    with cache_file.open("a") as f:
+        f.write("I am data that should corrupt the msgpack file")
+
+    # Reopen cache, try to fetch key
+    cd = cache.CacheDisk(0.3, str(cache_file))
+    # If the cache is unreadable, we want it to act like an empty cache (as
+    # if the file did not exist in the first place), and should raise a KeyError
+    with pytest.raises(KeyError):
+        assert cd["test-key"]