From ef20b40faa5fa8f38ead50e44150e7c2649cfe4f Mon Sep 17 00:00:00 2001 From: David Rheinsberg Date: Fri, 2 Dec 2022 14:49:09 +0100 Subject: [PATCH] util/fscache: introduce versioning Add a new field to the cache-information called `version`, which is a simple integer that is incremented on any backward-incompatible change. The cache-implementation is modified to avoid any access to the cache except for `/staging/`. This means, changes to the staging area must be backwards compatible at all cost. Furthermore, it means we can always successfully run osbuild even on possibly incompatible caches, because we can always just ignore the cache and fully rely on the staging area being accessible. The `load()` method will always return cache-misses. The `store()` method simply discards the entry instead of storing it. Note that `store()` needs to provide a context to the caller, hence this implementation simply creates another staging-context to provide to the caller and then discard. This is non-optimal, but keeps the API simple and avoids raising an exception to the caller (but this can be changed if it turns out to be problematic or unwanted). Lastly, the `cache.info` field behaves as usual, since this is also the field used to read the cache-version. However, this file is never written to improve resiliency and allow blacklisting buggy versions from the past. Signed-off-by: David Rheinsberg --- osbuild/util/fscache.py | 41 ++++++++++++++++++++++++++++++++--- test/mod/test_util_fscache.py | 5 +++-- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/osbuild/util/fscache.py b/osbuild/util/fscache.py index d08da631..f2f1c4fc 100644 --- a/osbuild/util/fscache.py +++ b/osbuild/util/fscache.py @@ -35,10 +35,12 @@ class FsCacheInfo(NamedTuple): creation_boot_id - Hashed linux boot-id at the time of cache-creation maximum_size - Maximum cache size in bytes, or "unlimited" + version - version of the cache data structures """ creation_boot_id: Optional[str] = None maximum_size: MaximumSizeType = None + version: Optional[int] = None @classmethod def from_json(cls, data: Any) -> "FsCacheInfo": @@ -54,6 +56,7 @@ class FsCacheInfo(NamedTuple): creation_boot_id = None maximum_size: MaximumSizeType = None + version = None # parse "creation-boot-id" _creation_boot_id = data.get("creation-boot-id", None) @@ -67,10 +70,16 @@ class FsCacheInfo(NamedTuple): elif isinstance(_maximum_size, str) and _maximum_size == "unlimited": maximum_size = "unlimited" + # parse "version" + _version = data.get("version", None) + if isinstance(_version, int): + version = _version + # create immutable tuple return cls( creation_boot_id, maximum_size, + version, ) def to_json(self) -> Dict[str, Any]: @@ -86,6 +95,8 @@ class FsCacheInfo(NamedTuple): data["creation-boot-id"] = self.creation_boot_id if self.maximum_size is not None: data["maximum-size"] = self.maximum_size + if self.version is not None: + data["version"] = self.version return data @@ -195,6 +206,8 @@ class FsCache(contextlib.AbstractContextManager, os.PathLike): _filename_cache_size = "cache.size" _filename_object_info = "object.info" _filename_object_lock = "object.lock" + _version_current = 1 + _version_minimum = 1 # constant properties _appid: str @@ -551,7 +564,7 @@ class FsCache(contextlib.AbstractContextManager, os.PathLike): # Create the file-scaffolding of the cache. We fill in the default # information and ignore racing operations. with self._atomic_file(self._filename_cache_info, self._dirname_objects, ignore_exist=True) as f: - f.write("{}") + json.dump({"version": self._version_current}, f) with self._atomic_file(self._filename_cache_lock, self._dirname_objects, ignore_exist=True) as f: pass with self._atomic_file(self._filename_cache_size, self._dirname_objects, ignore_exist=True) as f: @@ -604,6 +617,11 @@ class FsCache(contextlib.AbstractContextManager, os.PathLike): # Internal helper to verify we are in an active context-manager. return self._active + def _is_compatible(self): + # Internal helper to verify the cache-version is supported. + return self._info.version is not None and \ + self._version_minimum <= self._info.version <= self._version_current + def __enter__(self): assert not self._active @@ -662,6 +680,7 @@ class FsCache(contextlib.AbstractContextManager, os.PathLike): """ assert self._is_active() + assert self._is_compatible() # Open the cache-size and lock it for writing. But instead of writing # directly to it, we replace it with a new file. This guarantees that @@ -754,6 +773,10 @@ class FsCache(contextlib.AbstractContextManager, os.PathLike): Hence, any cache management routine will discard it. """ + # We check for an active context, but we never check for + # version-compatibility, because there is no way we can run without + # a staging area. Hence, the staging-area has to be backwards + # compatible at all times. assert self._is_active() uuidname = None @@ -805,6 +828,15 @@ class FsCache(contextlib.AbstractContextManager, os.PathLike): if not name: raise ValueError() + # If the cache-version is incompatible to this implementation, we short + # this call into the staging-area (which is always compatible). This + # avoids raising an exception (at the cost of dealing with this in the + # caller), and instead just creates a temporary copy which we discard. + if not self._is_compatible(): + with self.stage() as p: + yield p + return + uuidname = None lockfd = None @@ -912,6 +944,8 @@ class FsCache(contextlib.AbstractContextManager, os.PathLike): if not name: raise ValueError() + if not self._is_compatible(): + raise self.MissError() with contextlib.ExitStack() as es: # Use an ExitStack so we can catch exceptions raised by the @@ -1005,7 +1039,8 @@ class FsCache(contextlib.AbstractContextManager, os.PathLike): info = FsCacheInfo.from_json(info_raw) # Replace the file with the new values. This releases the lock. - with self._atomic_file(self._filename_cache_info, self._dirname_objects, replace=True) as f: - json.dump(info_raw, f) + if self._is_compatible(): + with self._atomic_file(self._filename_cache_info, self._dirname_objects, replace=True) as f: + json.dump(info_raw, f) self._load_cache_info(info) diff --git a/test/mod/test_util_fscache.py b/test/mod/test_util_fscache.py index 1125abd9..334f4736 100644 --- a/test/mod/test_util_fscache.py +++ b/test/mod/test_util_fscache.py @@ -4,6 +4,7 @@ # pylint: disable=protected-access +import json import os import tempfile @@ -194,7 +195,7 @@ def test_scaffolding(tmpdir): assert len(list(os.scandir(os.path.join(tmpdir, cache._dirname_stage)))) == 0 with open(os.path.join(tmpdir, cache._filename_cache_info), "r", encoding="utf8") as f: - assert f.read() == "{}" + assert json.load(f) == {"version": 1} with open(os.path.join(tmpdir, cache._filename_cache_lock), "r", encoding="utf8") as f: assert f.read() == "" with open(os.path.join(tmpdir, cache._filename_cache_size), "r", encoding="utf8") as f: @@ -210,7 +211,7 @@ def test_cache_info(tmpdir): cache = fscache.FsCache("osbuild-test-appid", tmpdir) with cache: - assert cache._info == fscache.FsCacheInfo() + assert cache._info == fscache.FsCacheInfo(version=1) assert cache.info == cache._info assert cache.info.maximum_size is None