From 53ea429324e30c2a47250f88ecd7499f67e23adc Mon Sep 17 00:00:00 2001
From: Rory Geoghegan <rory.geoghegan@behavox.com>
Date: Mon, 12 Sep 2022 16:39:33 -0400
Subject: [PATCH] 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                  |  1 +
 salt/utils/cache.py                    | 19 +++++++++++----
 tests/pytests/unit/utils/test_cache.py | 32 +++++++++++++++++++++-----
 3 files changed, 42 insertions(+), 10 deletions(-)
 create mode 100644 changelog/62527.fixed

diff --git a/changelog/62527.fixed b/changelog/62527.fixed
new file mode 100644
index 00000000000..3c9b76ebc33
--- /dev/null
+++ b/changelog/62527.fixed
@@ -0,0 +1 @@
+Fixes pillar where a corrupted CacheDisk file forces the pillar to be rebuilt
diff --git a/salt/utils/cache.py b/salt/utils/cache.py
index dc59000f9bd..2d9859f1a66 100644
--- a/salt/utils/cache.py
+++ b/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
-        with salt.utils.files.fopen(self._path, "rb") as fp_:
-            cache = salt.utils.msgpack.load(
-                fp_, encoding=__salt_system_encoding__, raw=False
-            )
+
+        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"]
diff --git a/tests/pytests/unit/utils/test_cache.py b/tests/pytests/unit/utils/test_cache.py
index 7910aa37a95..b46a4a6d441 100644
--- a/tests/pytests/unit/utils/test_cache.py
+++ b/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"]