Merge pull request #161 from openlawlibrary/renatav/updater-fixes

Various updater fixes and enhancements

CHANGELOG.md

@@ -9,10 +9,21 @@ and this project adheres to [Semantic Versioning][semver].
 
 ### Added
 
+- Raise an error if there are additional commits newer than the last authenticated commit if the updater is called with the check-authenticated flag ([161])
+- Added initial worktrees support to the updater ([161])
+- Added support for specifying location of the conf directory ([161])
+- Added a function for disabling fie logging ([161])
 ### Changed
 
+- Replaced authenticate-test-repo flag with an enum ([161])
+
 ### Fixed
 
+- Minor validation command fix ([161])
+
+
+[161]: https://github.com/openlawlibrary/taf/pull/161
+
 
 ## [0.8.0] - 02/09/2020
 

taf/auth_repo.py

@@ -18,6 +18,8 @@ class AuthRepoMixin(TAFRepository):
     LAST_VALIDATED_FILENAME = "last_validated_commit"
     TEST_REPO_FLAG_FILE = "test-auth-repo"
 
+    _conf_dir = None
+
     @property
     def conf_dir(self):
         """
@@ -27,10 +29,12 @@ def conf_dir(self):
         """
         # the repository's name consists of the namespace and name (namespace/name)
         # the configuration directory should be _name
-        last_dir = os.path.basename(os.path.normpath(self.path))
-        conf_path = Path(self.path).parent / f"_{last_dir}"
-        conf_path.mkdir(parents=True, exist_ok=True)
-        return str(conf_path)
+        if self._conf_dir is None:
+            last_dir = os.path.basename(os.path.normpath(self.path))
+            conf_path = Path(self.conf_directory_root, f"_{last_dir}")
+            conf_path.mkdir(parents=True, exist_ok=True)
+            self._conf_dir = str(conf_path)
+        return self._conf_dir
 
     @property
     def certs_dir(self):
@@ -207,9 +211,13 @@ def __init__(
         repo_urls=None,
         additional_info=None,
         default_branch="master",
+        conf_directory_root=None,
         *args,
         **kwargs,
     ):
+        if conf_directory_root is None:
+            conf_directory_root = str(Path(path).parent)
+        self.conf_directory_root = conf_directory_root
         super().__init__(path, repo_urls, additional_info, default_branch)
 
 
@@ -221,13 +229,18 @@ def __init__(
         repo_urls=None,
         additional_info=None,
         default_branch="master",
+        conf_directory_root=None,
         *args,
         **kwargs,
     ):
+        if conf_directory_root is None:
+            conf_directory_root = str(Path(root_dir, repo_name).parent)
+        self.conf_directory_root = conf_directory_root
         super().__init__(
             root_dir=root_dir,
             repo_name=repo_name,
             repo_urls=repo_urls,
             additional_info=additional_info,
             default_branch=default_branch,
+            conf_directory_root=conf_directory_root,
         )

taf/exceptions.py

@@ -129,5 +129,20 @@ class UpdateFailedError(TAFError):
     pass
 
 
+class UpdaterAdditionalCommitsError(TAFError):
+    def __init__(self, additional_commits_per_repo, message=None):
+        self.additional_commits_per_repo = additional_commits_per_repo
+        if message is None:
+            message = ""
+            for repo, branch_commits in additional_commits_per_repo.items():
+                for branch, commits in branch_commits.items():
+                    message += f"Repository {repo}: branch {branch} contains {len(commits)} additional"
+                    if len(commits) > 1:
+                        message += "commits\n"
+                    else:
+                        message += "commit\n"
+        self.message = message
+
+
 class YubikeyError(Exception):
     pass

taf/git.py

@@ -609,6 +609,14 @@ def fetch(self, fetch_all=False, branch=None, remote="origin"):
                 branch = ""
             self._git("fetch {} {}", remote, branch, log_error=True)
 
+    def find_worktree_path_by_branch(self, branch_name):
+        """Returns path of the workree where the branch is checked out, or None if not checked out in any worktree"""
+        worktrees = self.list_worktrees()
+        for path, _, _branch_name in worktrees.values():
+            if _branch_name == branch_name:
+                return path
+        return None
+
     def get_current_branch(self):
         """Return current branch."""
         return self._git("rev-parse --abbrev-ref HEAD").strip()
@@ -722,6 +730,22 @@ def list_untracked_files(self, path=None):
             untracked_file for untracked_file in untracked_files if len(untracked_file)
         ]
 
+    def list_worktrees(self):
+        """
+        Returns a dictionary containing information about repository's worktrees:
+        {
+            "worktree1_path: (worktree1_path, worktree1_commit, worktree1_branch),
+            "worktree2_path: (worktree2_path, worktree2_commit, worktree2_branch),
+            ...
+        }
+        """
+        worktrees_list = self._git("worktree list")
+        worktrees = [w.split() for w in worktrees_list.splitlines() if w]
+        return {
+            Path(wt[0]): (Path(wt[0]), wt[1], wt[2].replace("[", "").replace("]", ""))
+            for wt in worktrees
+        }
+
     def merge_commit(self, commit):
         self._git("merge {}", commit, log_error=True)
 

taf/log.py

@@ -22,6 +22,11 @@ def disable_console_logging():
     disable_tuf_console_logging()
 
 
+def disable_file_logging():
+    taf_logger.remove(file_loggers["log"])
+    disable_tuf_console_logging()
+
+
 def disable_tuf_console_logging():
     try:
         tuf.log.set_console_log_level(logging.CRITICAL)
@@ -31,7 +36,7 @@ def disable_tuf_console_logging():
 
 def disable_tuf_file_logging():
     if tuf.log.file_handler is not None:
-        tuf.log.disable_tuf_file_logging()
+        tuf.log.disable_file_logging()
     else:
         logging.getLogger("tuf").setLevel(logging.CRITICAL)
     logging.getLogger("securesystemslib_keys").setLevel(logging.CRITICAL)

taf/repository_tool.py

@@ -65,6 +65,15 @@ def is_delegated_role(role):
     return role not in ("root", "targets", "snapshot", "timestamp")
 
 
+def is_auth_repo(repo_path):
+    """Check if the given path contains a valid TUF repository"""
+    try:
+        Repository(repo_path)._repository
+        return True
+    except Exception:
+        return False
+
+
 def load_role_key(keystore, role, password=None, scheme=DEFAULT_RSA_SIGNATURE_SCHEME):
     """Loads the specified role's key from a keystore file.
     The keystore file can, but doesn't have to be password protected.
@@ -272,11 +281,15 @@ def _load_tuf_repository(self, path):
         """
         # before attempting to tuf repository, create empty targets directory if it does not exist
         # to avoid errors raised by tuf
+        targets_existed = True
         if not self.targets_path.is_dir():
+            targets_existed = False
             self.targets_path.mkdir(parents=True, exist_ok=True)
         try:
             self._tuf_repository = load_repository(path, self.name)
         except RepositoryError:
+            if not targets_existed:
+                self.targets_path.rmdir()
             raise InvalidRepositoryError(f"{self.name} is not a valid TUF repository!")
 
     def reload_tuf_repository(self):

taf/settings.py

@@ -6,6 +6,8 @@
 # unusable.
 temporary_directory = None
 
+conf_directory_root = None
+
 update_from_filesystem = False
 
 validate_repo_name = True

taf/tests/test_updater.py

@@ -58,7 +58,7 @@
 from taf.auth_repo import AuthenticationRepo
 from taf.exceptions import UpdateFailedError
 from taf.git import GitRepository
-from taf.updater.updater import update_repository
+from taf.updater.updater import update_repository, UpdateType
 from taf.utils import on_rm_error
 
 AUTH_REPO_REL_PATH = "organization/auth_repo"
@@ -98,16 +98,16 @@ def run_around_tests(client_dir):
 @pytest.mark.parametrize(
     "test_name, test_repo",
     [
-        ("test-updater-valid", False),
-        ("test-updater-additional-target-commit", False),
-        ("test-updater-valid-with-updated-expiration-dates", False),
-        ("test-updater-allow-unauthenticated-commits", False),
-        ("test-updater-test-repo", True),
-        ("test-updater-multiple-branches", False),
-        ("test-updater-delegated-roles", False),
-        ("test-updater-updated-root", False),
-        ("test-updater-updated-root-old-snapshot", False),
-        ("test-updater-updated-root-version-skipped", False),
+        ("test-updater-valid", UpdateType.OFFICIAL),
+        ("test-updater-additional-target-commit", UpdateType.OFFICIAL),
+        ("test-updater-valid-with-updated-expiration-dates", UpdateType.OFFICIAL),
+        ("test-updater-allow-unauthenticated-commits", UpdateType.OFFICIAL),
+        ("test-updater-test-repo", UpdateType.TEST),
+        ("test-updater-multiple-branches", UpdateType.OFFICIAL),
+        ("test-updater-delegated-roles", UpdateType.OFFICIAL),
+        ("test-updater-updated-root", UpdateType.OFFICIAL),
+        ("test-updater-updated-root-old-snapshot", UpdateType.OFFICIAL),
+        ("test-updater-updated-root-version-skipped", UpdateType.OFFICIAL),
     ],
 )
 def test_valid_update_no_client_repo(
@@ -121,11 +121,11 @@ def test_valid_update_no_client_repo(
 @pytest.mark.parametrize(
     "test_name, test_repo",
     [
-        ("test-updater-valid", False),
-        ("test-updater-additional-target-commit", False),
-        ("test-updater-allow-unauthenticated-commits", False),
-        ("test-updater-multiple-branches", False),
-        ("test-updater-delegated-roles", False),
+        ("test-updater-valid", UpdateType.OFFICIAL),
+        ("test-updater-additional-target-commit", UpdateType.OFFICIAL),
+        ("test-updater-allow-unauthenticated-commits", UpdateType.OFFICIAL),
+        ("test-updater-multiple-branches", UpdateType.OFFICIAL),
+        ("test-updater-delegated-roles", UpdateType.OFFICIAL),
     ],
 )
 def test_valid_update_no_auth_repo_one_target_repo_exists(
@@ -170,11 +170,11 @@ def test_valid_update_existing_client_repos(
 @pytest.mark.parametrize(
     "test_name, test_repo",
     [
-        ("test-updater-valid", False),
-        ("test-updater-allow-unauthenticated-commits", False),
-        ("test-updater-test-repo", True),
-        ("test-updater-multiple-branches", False),
-        ("test-updater-delegated-roles", False),
+        ("test-updater-valid", UpdateType.OFFICIAL),
+        ("test-updater-allow-unauthenticated-commits", UpdateType.OFFICIAL),
+        ("test-updater-test-repo", UpdateType.TEST),
+        ("test-updater-multiple-branches", UpdateType.OFFICIAL),
+        ("test-updater-delegated-roles", UpdateType.OFFICIAL),
     ],
 )
 def test_no_update_necessary(
@@ -332,7 +332,7 @@ def test_update_test_repo_no_flag(updater_repositories, origin_dir, client_dir):
     origin_dir = origin_dir / "test-updater-test-repo"
     # try to update without setting the last validated commit
     _update_invalid_repos_and_check_if_repos_exist(
-        client_dir, repositories, IS_A_TEST_REPO
+        client_dir, repositories, IS_A_TEST_REPO, UpdateType.OFFICIAL
     )
 
 
@@ -341,7 +341,7 @@ def test_update_repo_wrong_flag(updater_repositories, origin_dir, client_dir):
     origin_dir = origin_dir / "test-updater-valid"
     # try to update without setting the last validated commit
     _update_invalid_repos_and_check_if_repos_exist(
-        client_dir, repositories, NOT_A_TEST_REPO, True
+        client_dir, repositories, NOT_A_TEST_REPO, UpdateType.TEST
     )
 
 
@@ -471,7 +471,11 @@ def _get_valid_update_time(origin_auth_repo_path):
 
 
 def _update_and_check_commit_shas(
-    client_repos, repositories, origin_dir, client_dir, authetnicate_test_repo=False
+    client_repos,
+    repositories,
+    origin_dir,
+    client_dir,
+    expected_repo_type=UpdateType.EITHER,
 ):
     start_head_shas = _get_head_commit_shas(client_repos)
     clients_auth_repo_path = client_dir / AUTH_REPO_REL_PATH
@@ -482,14 +486,18 @@ def _update_and_check_commit_shas(
             str(clients_auth_repo_path),
             str(client_dir),
             True,
-            authenticate_test_repo=authetnicate_test_repo,
+            expected_repo_type=expected_repo_type,
         )
     _check_if_commits_match(repositories, origin_dir, client_dir, start_head_shas)
     _check_last_validated_commit(clients_auth_repo_path)
 
 
 def _update_invalid_repos_and_check_if_remained_same(
-    client_repos, client_dir, repositories, expected_error, authenticate_test_repo=False
+    client_repos,
+    client_dir,
+    repositories,
+    expected_error,
+    expected_repo_type=UpdateType.EITHER,
 ):
 
     start_head_shas = _get_head_commit_shas(client_repos)
@@ -503,7 +511,7 @@ def _update_invalid_repos_and_check_if_remained_same(
                 str(clients_auth_repo_path),
                 str(client_dir),
                 True,
-                authenticate_test_repo=authenticate_test_repo,
+                expected_repo_type=expected_repo_type,
             )
 
     # all repositories should still have the same head commit
@@ -518,7 +526,7 @@ def _update_invalid_repos_and_check_if_repos_exist(
     client_dir,
     repositories,
     expected_error,
-    authenticate_test_repo=False,
+    expected_repo_type=UpdateType.EITHER,
     set_time=True,
 ):
 
@@ -530,21 +538,21 @@ def _update_invalid_repos_and_check_if_repos_exist(
         if (client_dir / repository_rel_path).exists()
     ]
 
-    def _update_expect_error(client_dir, authenticate_test_repo):
+    def _update_expect_error(client_dir, expected_repo_type):
         with pytest.raises(UpdateFailedError, match=expected_error):
             update_repository(
                 str(origin_auth_repo_path),
                 str(clients_auth_repo_path),
                 str(client_dir),
                 True,
-                authenticate_test_repo=authenticate_test_repo,
+                expected_repo_type=expected_repo_type,
             ),
 
     if set_time:
         with freeze_time(_get_valid_update_time(origin_auth_repo_path)):
-            _update_expect_error(client_dir, authenticate_test_repo)
+            _update_expect_error(client_dir, expected_repo_type)
     else:
-        _update_expect_error(client_dir, authenticate_test_repo)
+        _update_expect_error(client_dir, expected_repo_type)
 
     # the client repositories should not exits
     for repository_rel_path in repositories: