Răsfoiți Sursa

keep previous repo location only in security dir

removed some code borg had for backwards compatibility with
old borg versions (that had previous_location only in the
cache).

now the repo location is only checked against the location
file in the security dir, simplifying the code and also
fixing a related test failure with NewCache.

also improved test_repository_move to test for aborting in
case the repo location changed unexpectedly.
Thomas Waldmann 1 an în urmă
părinte
comite
cf8c3a3ae7
3 a modificat fișierele cu 15 adăugiri și 58 ștergeri
  1. 2 6
      src/borg/archiver/config_cmd.py
  2. 3 39
      src/borg/cache.py
  3. 10 13
      src/borg/testsuite/archiver/checks.py

+ 2 - 6
src/borg/archiver/config_cmd.py

@@ -5,7 +5,6 @@ from ._common import with_repository
 from ..cache import Cache, assert_secure
 from ..cache import Cache, assert_secure
 from ..constants import *  # NOQA
 from ..constants import *  # NOQA
 from ..helpers import Error, CommandError
 from ..helpers import Error, CommandError
-from ..helpers import Location
 from ..helpers import parse_file_size, hex_to_bin
 from ..helpers import parse_file_size, hex_to_bin
 from ..manifest import Manifest
 from ..manifest import Manifest
 
 
@@ -52,11 +51,8 @@ class ConfigMixIn:
         def cache_validate(section, name, value=None, check_value=True):
         def cache_validate(section, name, value=None, check_value=True):
             if section not in ["cache"]:
             if section not in ["cache"]:
                 raise ValueError("Invalid section")
                 raise ValueError("Invalid section")
-            if name in ["previous_location"]:
-                if check_value:
-                    Location(value)
-            else:
-                raise ValueError("Invalid name")
+            # currently, we do not support setting anything in the cache via borg config.
+            raise ValueError("Invalid name")
 
 
         def list_config(config):
         def list_config(config):
             default_values = {
             default_values = {

+ 3 - 39
src/borg/cache.py

@@ -13,7 +13,6 @@ files_cache_logger = create_logger("borg.debug.files_cache")
 
 
 from .constants import CACHE_README, FILES_CACHE_MODE_DISABLED, ROBJ_FILE_STREAM
 from .constants import CACHE_README, FILES_CACHE_MODE_DISABLED, ROBJ_FILE_STREAM
 from .hashindex import ChunkIndex, ChunkIndexEntry, CacheSynchronizer
 from .hashindex import ChunkIndex, ChunkIndexEntry, CacheSynchronizer
-from .helpers import Location
 from .helpers import Error
 from .helpers import Error
 from .helpers import get_cache_dir, get_security_dir
 from .helpers import get_cache_dir, get_security_dir
 from .helpers import bin_to_hex, hex_to_bin, parse_stringified_list
 from .helpers import bin_to_hex, hex_to_bin, parse_stringified_list
@@ -100,7 +99,7 @@ class SecurityManager:
         with SaveFile(self.manifest_ts_file) as fd:
         with SaveFile(self.manifest_ts_file) as fd:
             fd.write(manifest.timestamp)
             fd.write(manifest.timestamp)
 
 
-    def assert_location_matches(self, cache_config=None):
+    def assert_location_matches(self):
         # Warn user before sending data to a relocated repository
         # Warn user before sending data to a relocated repository
         try:
         try:
             with open(self.location_file) as fd:
             with open(self.location_file) as fd:
@@ -112,10 +111,6 @@ class SecurityManager:
         except OSError as exc:
         except OSError as exc:
             logger.warning("Could not read previous location file: %s", exc)
             logger.warning("Could not read previous location file: %s", exc)
             previous_location = None
             previous_location = None
-        if cache_config and cache_config.previous_location and previous_location != cache_config.previous_location:
-            # Reconcile cache and security dir; we take the cache location.
-            previous_location = cache_config.previous_location
-            logger.debug("security: using previous_location of cache: %r", previous_location)
 
 
         repository_location = self.repository._location.canonical_path()
         repository_location = self.repository._location.canonical_path()
         if previous_location and previous_location != repository_location:
         if previous_location and previous_location != repository_location:
@@ -134,11 +129,9 @@ class SecurityManager:
             ):
             ):
                 raise Cache.RepositoryAccessAborted()
                 raise Cache.RepositoryAccessAborted()
             # adapt on-disk config immediately if the new location was accepted
             # adapt on-disk config immediately if the new location was accepted
-            logger.debug("security: updating location stored in cache and security dir")
+            logger.debug("security: updating location stored in security dir")
             with SaveFile(self.location_file) as fd:
             with SaveFile(self.location_file) as fd:
                 fd.write(repository_location)
                 fd.write(repository_location)
-            if cache_config:
-                cache_config.save()
 
 
     def assert_no_manifest_replay(self, manifest, key, cache_config=None):
     def assert_no_manifest_replay(self, manifest, key, cache_config=None):
         try:
         try:
@@ -184,7 +177,7 @@ class SecurityManager:
         logger.debug("security: repository checks ok, allowing access")
         logger.debug("security: repository checks ok, allowing access")
 
 
     def _assert_secure(self, manifest, key, cache_config=None):
     def _assert_secure(self, manifest, key, cache_config=None):
-        self.assert_location_matches(cache_config)
+        self.assert_location_matches()
         self.assert_key_type(key, cache_config)
         self.assert_key_type(key, cache_config)
         self.assert_no_manifest_replay(manifest, key, cache_config)
         self.assert_no_manifest_replay(manifest, key, cache_config)
         if not self.known():
         if not self.known():
@@ -221,29 +214,6 @@ def assert_secure(repository, manifest, lock_wait):
     sm.assert_secure(manifest, manifest.key, lock_wait=lock_wait)
     sm.assert_secure(manifest, manifest.key, lock_wait=lock_wait)
 
 
 
 
-def recanonicalize_relative_location(cache_location, repository):
-    # borg < 1.0.8rc1 had different canonicalization for the repo location (see #1655 and #1741).
-    repo_location = repository._location.canonical_path()
-    rl = Location(repo_location)
-    cl = Location(cache_location)
-    if (
-        cl.proto == rl.proto
-        and cl.user == rl.user
-        and cl.host == rl.host
-        and cl.port == rl.port
-        and cl.path
-        and rl.path
-        and cl.path.startswith("/~/")
-        and rl.path.startswith("/./")
-        and cl.path[3:] == rl.path[3:]
-    ):
-        # everything is same except the expected change in relative path canonicalization,
-        # update previous_location to avoid warning / user query about changed location:
-        return repo_location
-    else:
-        return cache_location
-
-
 def cache_dir(repository, path=None):
 def cache_dir(repository, path=None):
     return path or os.path.join(get_cache_dir(), repository.id_str)
     return path or os.path.join(get_cache_dir(), repository.id_str)
 
 
@@ -310,12 +280,6 @@ class CacheConfig:
         except configparser.NoSectionError:
         except configparser.NoSectionError:
             logger.debug("Cache integrity: No integrity data found (files, chunks). Cache is from old version.")
             logger.debug("Cache integrity: No integrity data found (files, chunks). Cache is from old version.")
             self.integrity = {}
             self.integrity = {}
-        previous_location = self._config.get("cache", "previous_location", fallback=None)
-        if previous_location:
-            self.previous_location = recanonicalize_relative_location(previous_location, self.repository)
-        else:
-            self.previous_location = None
-        self._config.set("cache", "previous_location", self.repository._location.canonical_path())
 
 
     def save(self, manifest=None, key=None):
     def save(self, manifest=None, key=None):
         if manifest:
         if manifest:

+ 10 - 13
src/borg/testsuite/archiver/checks.py

@@ -153,32 +153,29 @@ def test_repository_move(archivers, request, monkeypatch):
     security_dir = get_security_directory(archiver.repository_path)
     security_dir = get_security_directory(archiver.repository_path)
     os.replace(archiver.repository_path, archiver.repository_path + "_new")
     os.replace(archiver.repository_path, archiver.repository_path + "_new")
     archiver.repository_location += "_new"
     archiver.repository_location += "_new"
+    # borg should notice that the repository location changed and abort.
+    if archiver.FORK_DEFAULT:
+        cmd(archiver, "rinfo", exit_code=EXIT_ERROR)
+    else:
+        with pytest.raises(Cache.RepositoryAccessAborted):
+            cmd(archiver, "rinfo")
+    # if we explicitly allow relocated repos, it should work fine.
     monkeypatch.setenv("BORG_RELOCATED_REPO_ACCESS_IS_OK", "yes")
     monkeypatch.setenv("BORG_RELOCATED_REPO_ACCESS_IS_OK", "yes")
     cmd(archiver, "rinfo")
     cmd(archiver, "rinfo")
     monkeypatch.delenv("BORG_RELOCATED_REPO_ACCESS_IS_OK")
     monkeypatch.delenv("BORG_RELOCATED_REPO_ACCESS_IS_OK")
     with open(os.path.join(security_dir, "location")) as fd:
     with open(os.path.join(security_dir, "location")) as fd:
         location = fd.read()
         location = fd.read()
         assert location == Location(archiver.repository_location).canonical_path()
         assert location == Location(archiver.repository_location).canonical_path()
-    # Needs no confirmation anymore
-    cmd(archiver, "rinfo")
-    shutil.rmtree(archiver.cache_path)
+    # after new repo location was confirmed once, it needs no further confirmation anymore.
     cmd(archiver, "rinfo")
     cmd(archiver, "rinfo")
     shutil.rmtree(security_dir)
     shutil.rmtree(security_dir)
+    # it also needs no confirmation if we have no knowledge about the previous location.
     cmd(archiver, "rinfo")
     cmd(archiver, "rinfo")
+    # it will re-create security-related infos in the security dir:
     for file in ("location", "key-type", "manifest-timestamp"):
     for file in ("location", "key-type", "manifest-timestamp"):
         assert os.path.exists(os.path.join(security_dir, file))
         assert os.path.exists(os.path.join(security_dir, file))
 
 
 
 
-def test_security_dir_compat(archivers, request):
-    archiver = request.getfixturevalue(archivers)
-    cmd(archiver, "rcreate", RK_ENCRYPTION)
-    with open(os.path.join(get_security_directory(archiver.repository_path), "location"), "w") as fd:
-        fd.write("something outdated")
-    # This is fine, because the cache still has the correct information. security_dir and cache can disagree
-    # if older versions are used to confirm a renamed repository.
-    cmd(archiver, "rinfo")
-
-
 def test_unknown_unencrypted(archivers, request, monkeypatch):
 def test_unknown_unencrypted(archivers, request, monkeypatch):
     archiver = request.getfixturevalue(archivers)
     archiver = request.getfixturevalue(archivers)
     cmd(archiver, "rcreate", "--encryption=none")
     cmd(archiver, "rcreate", "--encryption=none")