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"]