File fixed-gitfs-cachedir_basename-to-avoid-hash-col... of Package salt

Overview Repositories Revisions Requests Users Attributes Meta

File fixed-gitfs-cachedir_basename-to-avoid-hash-collisio.patch of Package salt (Revision 6358268e61cc18a52d9eb63945f6f139)

Currently displaying revision 6358268e61cc18a52d9eb63945f6f139 , Show latest

From 7051f86bb48dbd618a7422d469f3aae4c6f18008 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pablo=20Su=C3=A1rez=20Hern=C3=A1ndez?=
 <psuarezhernandez@suse.com>
Date: Thu, 31 Aug 2023 10:41:53 +0100
Subject: [PATCH] Fixed gitfs cachedir_basename to avoid hash collisions
 (#599)

(bsc#1193948, bsc#1214797, CVE-2023-20898)

Fix gitfs tests

It's `gitfs` not `gtfs`, plus some code fixes and cleanup

Signed-off-by: Pedro Algarvio <palgarvio@vmware.com>

fix doc

wrap sha in base64

clean up cache name

stop branch collision

run pre

Co-authored-by: cmcmarrow <charles.mcmarrow.4@gmail.com>
---
 changelog/cve-2023-20898.security.md   |   1 +
 salt/utils/gitfs.py                    |  83 ++++++-
 tests/pytests/unit/utils/test_gitfs.py | 255 +++++++++++++++++++++
 tests/unit/utils/test_gitfs.py         | 305 ++++++-------------------
 4 files changed, 403 insertions(+), 241 deletions(-)
 create mode 100644 changelog/cve-2023-20898.security.md
 create mode 100644 tests/pytests/unit/utils/test_gitfs.py

diff --git a/changelog/cve-2023-20898.security.md b/changelog/cve-2023-20898.security.md
new file mode 100644
index 0000000000..44f1729192
--- /dev/null
+++ b/changelog/cve-2023-20898.security.md
@@ -0,0 +1 @@
+Fixed gitfs cachedir_basename to avoid hash collisions. Added MP Lock to gitfs. These changes should stop race conditions.
diff --git a/salt/utils/gitfs.py b/salt/utils/gitfs.py
index 38e84f38aa..af61aa0dda 100644
--- a/salt/utils/gitfs.py
+++ b/salt/utils/gitfs.py
@@ -3,6 +3,7 @@ Classes which provide the shared base for GitFS, git_pillar, and winrepo
 """
 
 
+import base64
 import contextlib
 import copy
 import errno
@@ -11,10 +12,12 @@ import glob
 import hashlib
 import io
 import logging
+import multiprocessing
 import os
 import shlex
 import shutil
 import stat
+import string
 import subprocess
 import time
 import weakref
@@ -22,6 +25,7 @@ from datetime import datetime
 
 import salt.ext.tornado.ioloop
 import salt.fileserver
+import salt.syspaths
 import salt.utils.configparser
 import salt.utils.data
 import salt.utils.files
@@ -34,7 +38,6 @@ import salt.utils.stringutils
 import salt.utils.url
 import salt.utils.user
 import salt.utils.versions
-import salt.syspaths
 from salt.config import DEFAULT_MASTER_OPTS as _DEFAULT_MASTER_OPTS
 from salt.exceptions import FileserverConfigError, GitLockError, get_error_message
 from salt.utils.event import tagify
@@ -226,6 +229,10 @@ class GitProvider:
     invoking the parent class' __init__.
     """
 
+    # master lock should only be locked for very short periods of times "seconds"
+    # the master lock should be used when ever git provider reads or writes to one if it locks
+    _master_lock = multiprocessing.Lock()
+
     def __init__(
         self,
         opts,
@@ -452,13 +459,44 @@ class GitProvider:
             failhard(self.role)
 
         hash_type = getattr(hashlib, self.opts.get("hash_type", "md5"))
+        # Generate full id.
+        # Full id helps decrease the chances of collections in the gitfs cache.
+        try:
+            target = str(self.get_checkout_target())
+        except AttributeError:
+            target = ""
+        self._full_id = "-".join(
+            [
+                getattr(self, "name", ""),
+                self.id,
+                getattr(self, "env", ""),
+                getattr(self, "_root", ""),
+                self.role,
+                getattr(self, "base", ""),
+                getattr(self, "branch", ""),
+                target,
+            ]
+        )
         # We loaded this data from yaml configuration files, so, its safe
         # to use UTF-8
-        self.hash = hash_type(self.id.encode("utf-8")).hexdigest()
-        self.cachedir_basename = getattr(self, "name", self.hash)
+        base64_hash = str(
+            base64.b64encode(hash_type(self._full_id.encode("utf-8")).digest()),
+            encoding="ascii",  # base64 only outputs ascii
+        ).replace(
+            "/", "_"
+        )  # replace "/" with "_" to not cause trouble with file system
+
+        # limit name length to 19, so we don't eat up all the path length for windows
+        # this is due to pygit2 limitations
+        # replace any unknown char with "_" to not cause trouble with file system
+        name_chars = string.ascii_letters + string.digits + "-"
+        cache_name = "".join(
+            c if c in name_chars else "_" for c in getattr(self, "name", "")[:19]
+        )
+
+        self.cachedir_basename = f"{cache_name}-{base64_hash}"
         self.cachedir = salt.utils.path.join(cache_root, self.cachedir_basename)
         self.linkdir = salt.utils.path.join(cache_root, "links", self.cachedir_basename)
-
         if not os.path.isdir(self.cachedir):
             os.makedirs(self.cachedir)
 
@@ -473,6 +511,12 @@ class GitProvider:
             log.critical(msg, exc_info=True)
             failhard(self.role)
 
+    def full_id(self):
+        return self._full_id
+
+    def get_cachedir_basename(self):
+        return self.cachedir_basename
+
     def _get_envs_from_ref_paths(self, refs):
         """
         Return the names of remote refs (stripped of the remote name) and tags
@@ -663,6 +707,19 @@ class GitProvider:
         """
         Clear update.lk
         """
+        if self.__class__._master_lock.acquire(timeout=60) is False:
+            # if gitfs works right we should never see this timeout error.
+            log.error("gitfs master lock timeout!")
+            raise TimeoutError("gitfs master lock timeout!")
+        try:
+            return self._clear_lock(lock_type)
+        finally:
+            self.__class__._master_lock.release()
+
+    def _clear_lock(self, lock_type="update"):
+        """
+        Clear update.lk without MultiProcessing locks
+        """
         lock_file = self._get_lock_file(lock_type=lock_type)
 
         def _add_error(errlist, exc):
@@ -838,6 +895,20 @@ class GitProvider:
         """
         Place a lock file if (and only if) it does not already exist.
         """
+        if self.__class__._master_lock.acquire(timeout=60) is False:
+            # if gitfs works right we should never see this timeout error.
+            log.error("gitfs master lock timeout!")
+            raise TimeoutError("gitfs master lock timeout!")
+        try:
+            return self.__lock(lock_type, failhard)
+        finally:
+            self.__class__._master_lock.release()
+
+    def __lock(self, lock_type="update", failhard=False):
+        """
+        Place a lock file if (and only if) it does not already exist.
+        Without MultiProcessing locks.
+        """
         try:
             fh_ = os.open(
                 self._get_lock_file(lock_type), os.O_CREAT | os.O_EXCL | os.O_WRONLY
@@ -904,9 +975,9 @@ class GitProvider:
                             lock_type,
                             lock_file,
                         )
-                    success, fail = self.clear_lock()
+                    success, fail = self._clear_lock()
                     if success:
-                        return self._lock(lock_type="update", failhard=failhard)
+                        return self.__lock(lock_type="update", failhard=failhard)
                     elif failhard:
                         raise
                     return
diff --git a/tests/pytests/unit/utils/test_gitfs.py b/tests/pytests/unit/utils/test_gitfs.py
new file mode 100644
index 0000000000..e9915de412
--- /dev/null
+++ b/tests/pytests/unit/utils/test_gitfs.py
@@ -0,0 +1,255 @@
+import os
+import string
+import time
+
+import pytest
+
+import salt.fileserver.gitfs
+import salt.utils.gitfs
+from salt.exceptions import FileserverConfigError
+from tests.support.helpers import patched_environ
+from tests.support.mock import MagicMock, patch
+
+try:
+    HAS_PYGIT2 = (
+        salt.utils.gitfs.PYGIT2_VERSION
+        and salt.utils.gitfs.PYGIT2_VERSION >= salt.utils.gitfs.PYGIT2_MINVER
+        and salt.utils.gitfs.LIBGIT2_VERSION
+        and salt.utils.gitfs.LIBGIT2_VERSION >= salt.utils.gitfs.LIBGIT2_MINVER
+    )
+except AttributeError:
+    HAS_PYGIT2 = False
+
+
+if HAS_PYGIT2:
+    import pygit2
+
+
+@pytest.mark.parametrize(
+    "role_name,role_class",
+    (
+        ("gitfs", salt.utils.gitfs.GitFS),
+        ("git_pillar", salt.utils.gitfs.GitPillar),
+        ("winrepo", salt.utils.gitfs.WinRepo),
+    ),
+)
+def test_provider_case_insensitive_gitfs_provider(minion_opts, role_name, role_class):
+    """
+    Ensure that both lowercase and non-lowercase values are supported
+    """
+    provider = "GitPython"
+    key = "{}_provider".format(role_name)
+    with patch.object(role_class, "verify_gitpython", MagicMock(return_value=True)):
+        with patch.object(role_class, "verify_pygit2", MagicMock(return_value=False)):
+            args = [minion_opts, {}]
+            kwargs = {"init_remotes": False}
+            if role_name == "winrepo":
+                kwargs["cache_root"] = "/tmp/winrepo-dir"
+            with patch.dict(minion_opts, {key: provider}):
+                # Try to create an instance with uppercase letters in
+                # provider name. If it fails then a
+                # FileserverConfigError will be raised, so no assert is
+                # necessary.
+                role_class(*args, **kwargs)
+            # Now try to instantiate an instance with all lowercase
+            # letters. Again, no need for an assert here.
+            role_class(*args, **kwargs)
+
+
+@pytest.mark.parametrize(
+    "role_name,role_class",
+    (
+        ("gitfs", salt.utils.gitfs.GitFS),
+        ("git_pillar", salt.utils.gitfs.GitPillar),
+        ("winrepo", salt.utils.gitfs.WinRepo),
+    ),
+)
+def test_valid_provider_gitfs_provider(minion_opts, role_name, role_class):
+    """
+    Ensure that an invalid provider is not accepted, raising a
+    FileserverConfigError.
+    """
+
+    def _get_mock(verify, provider):
+        """
+        Return a MagicMock with the desired return value
+        """
+        return MagicMock(return_value=verify.endswith(provider))
+
+    key = "{}_provider".format(role_name)
+    for provider in salt.utils.gitfs.GIT_PROVIDERS:
+        verify = "verify_gitpython"
+        mock1 = _get_mock(verify, provider)
+        with patch.object(role_class, verify, mock1):
+            verify = "verify_pygit2"
+            mock2 = _get_mock(verify, provider)
+            with patch.object(role_class, verify, mock2):
+                args = [minion_opts, {}]
+                kwargs = {"init_remotes": False}
+                if role_name == "winrepo":
+                    kwargs["cache_root"] = "/tmp/winrepo-dir"
+                with patch.dict(minion_opts, {key: provider}):
+                    role_class(*args, **kwargs)
+                with patch.dict(minion_opts, {key: "foo"}):
+                    # Set the provider name to a known invalid provider
+                    # and make sure it raises an exception.
+                    with pytest.raises(FileserverConfigError):
+                        role_class(*args, **kwargs)
+
+
+@pytest.fixture
+def _prepare_remote_repository_pygit2(tmp_path):
+    remote = os.path.join(tmp_path, "pygit2-repo")
+    filecontent = "This is an empty README file"
+    filename = "README"
+    signature = pygit2.Signature(
+        "Dummy Commiter", "dummy@dummy.com", int(time.time()), 0
+    )
+    repository = pygit2.init_repository(remote, False)
+    builder = repository.TreeBuilder()
+    tree = builder.write()
+    commit = repository.create_commit(
+        "HEAD", signature, signature, "Create master branch", tree, []
+    )
+    repository.create_reference("refs/tags/simple_tag", commit)
+    with salt.utils.files.fopen(
+        os.path.join(repository.workdir, filename), "w"
+    ) as file:
+        file.write(filecontent)
+    blob = repository.create_blob_fromworkdir(filename)
+    builder = repository.TreeBuilder()
+    builder.insert(filename, blob, pygit2.GIT_FILEMODE_BLOB)
+    tree = builder.write()
+    repository.index.read()
+    repository.index.add(filename)
+    repository.index.write()
+    commit = repository.create_commit(
+        "HEAD",
+        signature,
+        signature,
+        "Added a README",
+        tree,
+        [repository.head.target],
+    )
+    repository.create_tag(
+        "annotated_tag", commit, pygit2.GIT_OBJ_COMMIT, signature, "some message"
+    )
+    return remote
+
+
+@pytest.fixture
+def _prepare_provider(tmp_path, minion_opts, _prepare_remote_repository_pygit2):
+    cache = tmp_path / "pygit2-repo-cache"
+    minion_opts.update(
+        {
+            "cachedir": str(cache),
+            "gitfs_disable_saltenv_mapping": False,
+            "gitfs_base": "master",
+            "gitfs_insecure_auth": False,
+            "gitfs_mountpoint": "",
+            "gitfs_passphrase": "",
+            "gitfs_password": "",
+            "gitfs_privkey": "",
+            "gitfs_provider": "pygit2",
+            "gitfs_pubkey": "",
+            "gitfs_ref_types": ["branch", "tag", "sha"],
+            "gitfs_refspecs": [
+                "+refs/heads/*:refs/remotes/origin/*",
+                "+refs/tags/*:refs/tags/*",
+            ],
+            "gitfs_root": "",
+            "gitfs_saltenv_blacklist": [],
+            "gitfs_saltenv_whitelist": [],
+            "gitfs_ssl_verify": True,
+            "gitfs_update_interval": 3,
+            "gitfs_user": "",
+            "verified_gitfs_provider": "pygit2",
+        }
+    )
+    per_remote_defaults = {
+        "base": "master",
+        "disable_saltenv_mapping": False,
+        "insecure_auth": False,
+        "ref_types": ["branch", "tag", "sha"],
+        "passphrase": "",
+        "mountpoint": "",
+        "password": "",
+        "privkey": "",
+        "pubkey": "",
+        "refspecs": [
+            "+refs/heads/*:refs/remotes/origin/*",
+            "+refs/tags/*:refs/tags/*",
+        ],
+        "root": "",
+        "saltenv_blacklist": [],
+        "saltenv_whitelist": [],
+        "ssl_verify": True,
+        "update_interval": 60,
+        "user": "",
+    }
+    per_remote_only = ("all_saltenvs", "name", "saltenv")
+    override_params = tuple(per_remote_defaults)
+    cache_root = cache / "gitfs"
+    role = "gitfs"
+    provider = salt.utils.gitfs.Pygit2(
+        minion_opts,
+        _prepare_remote_repository_pygit2,
+        per_remote_defaults,
+        per_remote_only,
+        override_params,
+        str(cache_root),
+        role,
+    )
+    return provider
+
+
+@pytest.mark.skipif(not HAS_PYGIT2, reason="This host lacks proper pygit2 support")
+@pytest.mark.skip_on_windows(
+    reason="Skip Pygit2 on windows, due to pygit2 access error on windows"
+)
+def test_checkout_pygit2(_prepare_provider):
+    provider = _prepare_provider
+    provider.remotecallbacks = None
+    provider.credentials = None
+    provider.init_remote()
+    provider.fetch()
+    provider.branch = "master"
+    assert provider.cachedir in provider.checkout()
+    provider.branch = "simple_tag"
+    assert provider.cachedir in provider.checkout()
+    provider.branch = "annotated_tag"
+    assert provider.cachedir in provider.checkout()
+    provider.branch = "does_not_exist"
+    assert provider.checkout() is None
+
+
+@pytest.mark.skipif(not HAS_PYGIT2, reason="This host lacks proper pygit2 support")
+@pytest.mark.skip_on_windows(
+    reason="Skip Pygit2 on windows, due to pygit2 access error on windows"
+)
+def test_checkout_pygit2_with_home_env_unset(_prepare_provider):
+    provider = _prepare_provider
+    provider.remotecallbacks = None
+    provider.credentials = None
+    with patched_environ(__cleanup__=["HOME"]):
+        assert "HOME" not in os.environ
+        provider.init_remote()
+        provider.fetch()
+        assert "HOME" in os.environ
+
+
+def test_full_id_pygit2(_prepare_provider):
+    assert _prepare_provider.full_id().startswith("-")
+    assert _prepare_provider.full_id().endswith("/pygit2-repo---gitfs-master--")
+
+
+@pytest.mark.skipif(not HAS_PYGIT2, reason="This host lacks proper pygit2 support")
+@pytest.mark.skip_on_windows(
+    reason="Skip Pygit2 on windows, due to pygit2 access error on windows"
+)
+def test_get_cachedir_basename_pygit2(_prepare_provider):
+    basename = _prepare_provider.get_cachedir_basename()
+    assert len(basename) == 45
+    assert basename[0] == "-"
+    # check that a valid base64 is given '/' -> '_'
+    assert all(c in string.ascii_letters + string.digits + "+_=" for c in basename[1:])
diff --git a/tests/unit/utils/test_gitfs.py b/tests/unit/utils/test_gitfs.py
index 7c400b69af..6d8e97a239 100644
--- a/tests/unit/utils/test_gitfs.py
+++ b/tests/unit/utils/test_gitfs.py
@@ -2,37 +2,20 @@
 These only test the provider selection and verification logic, they do not init
 any remotes.
 """
-import os
-import shutil
-from time import time
+
+import tempfile
 
 import pytest
 
+import salt.ext.tornado.ioloop
 import salt.fileserver.gitfs
 import salt.utils.files
 import salt.utils.gitfs
+import salt.utils.path
 import salt.utils.platform
-import tests.support.paths
-from salt.exceptions import FileserverConfigError
-from tests.support.helpers import patched_environ
 from tests.support.mixins import AdaptedConfigurationTestCaseMixin
-from tests.support.mock import MagicMock, patch
 from tests.support.unit import TestCase
 
-try:
-    HAS_PYGIT2 = (
-        salt.utils.gitfs.PYGIT2_VERSION
-        and salt.utils.gitfs.PYGIT2_VERSION >= salt.utils.gitfs.PYGIT2_MINVER
-        and salt.utils.gitfs.LIBGIT2_VERSION
-        and salt.utils.gitfs.LIBGIT2_VERSION >= salt.utils.gitfs.LIBGIT2_MINVER
-    )
-except AttributeError:
-    HAS_PYGIT2 = False
-
-
-if HAS_PYGIT2:
-    import pygit2
-
 
 def _clear_instance_map():
     try:
@@ -45,6 +28,9 @@ def _clear_instance_map():
 
 class TestGitBase(TestCase, AdaptedConfigurationTestCaseMixin):
     def setUp(self):
+        self._tmp_dir = tempfile.TemporaryDirectory()
+        tmp_name = self._tmp_dir.name
+
         class MockedProvider(
             salt.utils.gitfs.GitProvider
         ):  # pylint: disable=abstract-method
@@ -71,6 +57,7 @@ class TestGitBase(TestCase, AdaptedConfigurationTestCaseMixin):
                 )
 
             def init_remote(self):
+                self.gitdir = salt.utils.path.join(tmp_name, ".git")
                 self.repo = True
                 new = False
                 return new
@@ -107,6 +94,7 @@ class TestGitBase(TestCase, AdaptedConfigurationTestCaseMixin):
         for remote in self.main_class.remotes:
             remote.fetched = False
         del self.main_class
+        self._tmp_dir.cleanup()
 
     def test_update_all(self):
         self.main_class.update()
@@ -126,226 +114,73 @@ class TestGitBase(TestCase, AdaptedConfigurationTestCaseMixin):
         self.assertTrue(self.main_class.remotes[0].fetched)
         self.assertFalse(self.main_class.remotes[1].fetched)
 
-
-class TestGitFSProvider(TestCase):
-    def setUp(self):
-        self.opts = {"cachedir": "/tmp/gitfs-test-cache"}
-
-    def tearDown(self):
-        self.opts = None
-
-    def test_provider_case_insensitive(self):
-        """
-        Ensure that both lowercase and non-lowercase values are supported
-        """
-        provider = "GitPython"
-        for role_name, role_class in (
-            ("gitfs", salt.utils.gitfs.GitFS),
-            ("git_pillar", salt.utils.gitfs.GitPillar),
-            ("winrepo", salt.utils.gitfs.WinRepo),
-        ):
-
-            key = "{}_provider".format(role_name)
-            with patch.object(
-                role_class, "verify_gitpython", MagicMock(return_value=True)
-            ):
-                with patch.object(
-                    role_class, "verify_pygit2", MagicMock(return_value=False)
-                ):
-                    args = [self.opts, {}]
-                    kwargs = {"init_remotes": False}
-                    if role_name == "winrepo":
-                        kwargs["cache_root"] = "/tmp/winrepo-dir"
-                    with patch.dict(self.opts, {key: provider}):
-                        # Try to create an instance with uppercase letters in
-                        # provider name. If it fails then a
-                        # FileserverConfigError will be raised, so no assert is
-                        # necessary.
-                        role_class(*args, **kwargs)
-                    # Now try to instantiate an instance with all lowercase
-                    # letters. Again, no need for an assert here.
-                    role_class(*args, **kwargs)
-
-    def test_valid_provider(self):
-        """
-        Ensure that an invalid provider is not accepted, raising a
-        FileserverConfigError.
-        """
-
-        def _get_mock(verify, provider):
-            """
-            Return a MagicMock with the desired return value
-            """
-            return MagicMock(return_value=verify.endswith(provider))
-
-        for role_name, role_class in (
-            ("gitfs", salt.utils.gitfs.GitFS),
-            ("git_pillar", salt.utils.gitfs.GitPillar),
-            ("winrepo", salt.utils.gitfs.WinRepo),
-        ):
-            key = "{}_provider".format(role_name)
-            for provider in salt.utils.gitfs.GIT_PROVIDERS:
-                verify = "verify_gitpython"
-                mock1 = _get_mock(verify, provider)
-                with patch.object(role_class, verify, mock1):
-                    verify = "verify_pygit2"
-                    mock2 = _get_mock(verify, provider)
-                    with patch.object(role_class, verify, mock2):
-                        args = [self.opts, {}]
-                        kwargs = {"init_remotes": False}
-                        if role_name == "winrepo":
-                            kwargs["cache_root"] = "/tmp/winrepo-dir"
-
-                        with patch.dict(self.opts, {key: provider}):
-                            role_class(*args, **kwargs)
-
-                        with patch.dict(self.opts, {key: "foo"}):
-                            # Set the provider name to a known invalid provider
-                            # and make sure it raises an exception.
-                            self.assertRaises(
-                                FileserverConfigError, role_class, *args, **kwargs
-                            )
-
-
-@pytest.mark.skipif(not HAS_PYGIT2, reason="This host lacks proper pygit2 support")
-@pytest.mark.skip_on_windows(
-    reason="Skip Pygit2 on windows, due to pygit2 access error on windows"
-)
-class TestPygit2(TestCase):
-    def _prepare_remote_repository(self, path):
-        shutil.rmtree(path, ignore_errors=True)
-
-        filecontent = "This is an empty README file"
-        filename = "README"
-
-        signature = pygit2.Signature(
-            "Dummy Commiter", "dummy@dummy.com", int(time()), 0
+    def test_full_id(self):
+        self.assertEqual(
+            self.main_class.remotes[0].full_id(), "-file://repo1.git---gitfs-master--"
         )
 
-        repository = pygit2.init_repository(path, False)
-        builder = repository.TreeBuilder()
-        tree = builder.write()
-        commit = repository.create_commit(
-            "HEAD", signature, signature, "Create master branch", tree, []
+    def test_full_id_with_name(self):
+        self.assertEqual(
+            self.main_class.remotes[1].full_id(),
+            "repo2-file://repo2.git---gitfs-master--",
         )
-        repository.create_reference("refs/tags/simple_tag", commit)
 
-        with salt.utils.files.fopen(
-            os.path.join(repository.workdir, filename), "w"
-        ) as file:
-            file.write(filecontent)
-
-        blob = repository.create_blob_fromworkdir(filename)
-        builder = repository.TreeBuilder()
-        builder.insert(filename, blob, pygit2.GIT_FILEMODE_BLOB)
-        tree = builder.write()
-
-        repository.index.read()
-        repository.index.add(filename)
-        repository.index.write()
-
-        commit = repository.create_commit(
-            "HEAD",
-            signature,
-            signature,
-            "Added a README",
-            tree,
-            [repository.head.target],
-        )
-        repository.create_tag(
-            "annotated_tag", commit, pygit2.GIT_OBJ_COMMIT, signature, "some message"
+    def test_get_cachedir_basename(self):
+        self.assertEqual(
+            self.main_class.remotes[0].get_cachedir_basename(),
+            "-jXhnbGDemchtZwTwaD2s6VOaVvs98a7w+AtiYlmOVb0=",
         )
 
-    def _prepare_cache_repository(self, remote, cache):
-        opts = {
-            "cachedir": cache,
-            "__role": "minion",
-            "gitfs_disable_saltenv_mapping": False,
-            "gitfs_base": "master",
-            "gitfs_insecure_auth": False,
-            "gitfs_mountpoint": "",
-            "gitfs_passphrase": "",
-            "gitfs_password": "",
-            "gitfs_privkey": "",
-            "gitfs_provider": "pygit2",
-            "gitfs_pubkey": "",
-            "gitfs_ref_types": ["branch", "tag", "sha"],
-            "gitfs_refspecs": [
-                "+refs/heads/*:refs/remotes/origin/*",
-                "+refs/tags/*:refs/tags/*",
-            ],
-            "gitfs_root": "",
-            "gitfs_saltenv_blacklist": [],
-            "gitfs_saltenv_whitelist": [],
-            "gitfs_ssl_verify": True,
-            "gitfs_update_interval": 3,
-            "gitfs_user": "",
-            "verified_gitfs_provider": "pygit2",
-        }
-        per_remote_defaults = {
-            "base": "master",
-            "disable_saltenv_mapping": False,
-            "insecure_auth": False,
-            "ref_types": ["branch", "tag", "sha"],
-            "passphrase": "",
-            "mountpoint": "",
-            "password": "",
-            "privkey": "",
-            "pubkey": "",
-            "refspecs": [
-                "+refs/heads/*:refs/remotes/origin/*",
-                "+refs/tags/*:refs/tags/*",
-            ],
-            "root": "",
-            "saltenv_blacklist": [],
-            "saltenv_whitelist": [],
-            "ssl_verify": True,
-            "update_interval": 60,
-            "user": "",
-        }
-        per_remote_only = ("all_saltenvs", "name", "saltenv")
-        override_params = tuple(per_remote_defaults.keys())
-        cache_root = os.path.join(cache, "gitfs")
-        role = "gitfs"
-        shutil.rmtree(cache_root, ignore_errors=True)
-        provider = salt.utils.gitfs.Pygit2(
-            opts,
-            remote,
-            per_remote_defaults,
-            per_remote_only,
-            override_params,
-            cache_root,
-            role,
+    def test_get_cachedir_base_with_name(self):
+        self.assertEqual(
+            self.main_class.remotes[1].get_cachedir_basename(),
+            "repo2-nuezpiDtjQRFC0ZJDByvi+F6Vb8ZhfoH41n_KFxTGsU=",
         )
-        return provider
 
-    def test_checkout(self):
-        remote = os.path.join(tests.support.paths.TMP, "pygit2-repo")
-        cache = os.path.join(tests.support.paths.TMP, "pygit2-repo-cache")
-        self._prepare_remote_repository(remote)
-        provider = self._prepare_cache_repository(remote, cache)
-        provider.remotecallbacks = None
-        provider.credentials = None
-        provider.init_remote()
-        provider.fetch()
-        provider.branch = "master"
-        self.assertIn(provider.cachedir, provider.checkout())
-        provider.branch = "simple_tag"
-        self.assertIn(provider.cachedir, provider.checkout())
-        provider.branch = "annotated_tag"
-        self.assertIn(provider.cachedir, provider.checkout())
-        provider.branch = "does_not_exist"
-        self.assertIsNone(provider.checkout())
+    def test_git_provider_mp_lock(self):
+        """
+        Check that lock is released after provider.lock()
+        """
+        provider = self.main_class.remotes[0]
+        provider.lock()
+        # check that lock has been released
+        self.assertTrue(provider._master_lock.acquire(timeout=5))
+        provider._master_lock.release()
 
-    def test_checkout_with_home_env_unset(self):
-        remote = os.path.join(tests.support.paths.TMP, "pygit2-repo")
-        cache = os.path.join(tests.support.paths.TMP, "pygit2-repo-cache")
-        self._prepare_remote_repository(remote)
-        provider = self._prepare_cache_repository(remote, cache)
-        provider.remotecallbacks = None
-        provider.credentials = None
-        with patched_environ(__cleanup__=["HOME"]):
-            self.assertTrue("HOME" not in os.environ)
-            provider.init_remote()
-            provider.fetch()
-            self.assertTrue("HOME" in os.environ)
+    def test_git_provider_mp_clear_lock(self):
+        """
+        Check that lock is released after provider.clear_lock()
+        """
+        provider = self.main_class.remotes[0]
+        provider.clear_lock()
+        # check that lock has been released
+        self.assertTrue(provider._master_lock.acquire(timeout=5))
+        provider._master_lock.release()
+
+    @pytest.mark.slow_test
+    def test_git_provider_mp_lock_timeout(self):
+        """
+        Check that lock will time out if master lock is locked.
+        """
+        provider = self.main_class.remotes[0]
+        # Hijack the lock so git provider is fooled into thinking another instance is doing somthing.
+        self.assertTrue(provider._master_lock.acquire(timeout=5))
+        try:
+            # git provider should raise timeout error to avoid lock race conditions
+            self.assertRaises(TimeoutError, provider.lock)
+        finally:
+            provider._master_lock.release()
+
+    @pytest.mark.slow_test
+    def test_git_provider_mp_clear_lock_timeout(self):
+        """
+        Check that clear lock will time out if master lock is locked.
+        """
+        provider = self.main_class.remotes[0]
+        # Hijack the lock so git provider is fooled into thinking another instance is doing somthing.
+        self.assertTrue(provider._master_lock.acquire(timeout=5))
+        try:
+            # git provider should raise timeout error to avoid lock race conditions
+            self.assertRaises(TimeoutError, provider.clear_lock)
+        finally:
+            provider._master_lock.release()
-- 
2.41.0

Places

File fixed-gitfs-cachedir_basename-to-avoid-hash-collisio.patch of Package salt (Revision 6358268e61cc18a52d9eb63945f6f139)

Places