From 12f926088037aa34637412fb1bae186a72307240 Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Sun, 20 Aug 2023 23:33:37 -0500 Subject: [PATCH 01/31] wip fix env --- salt/utils/gitfs.py | 125 ++++++++++++++++++++++++-------------------- 1 file changed, 69 insertions(+), 56 deletions(-) diff --git a/salt/utils/gitfs.py b/salt/utils/gitfs.py index d7774bacd3bb..b644d854f885 100644 --- a/salt/utils/gitfs.py +++ b/salt/utils/gitfs.py @@ -17,7 +17,6 @@ import shlex import shutil import stat -import string import subprocess import time import weakref @@ -252,7 +251,6 @@ def __init__( val_cb=lambda x, y: str(y), ) self.conf = copy.deepcopy(per_remote_defaults) - # Remove the 'salt://' from the beginning of any globally-defined # per-saltenv mountpoints for saltenv, saltenv_conf in self.global_saltenv.items(): @@ -458,46 +456,28 @@ def __init__( 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 - base64_hash = str( - base64.b64encode(hash_type(self._full_id.encode("utf-8")).digest()), + self._cache_basehash = str( + base64.b64encode(hash_type(self.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._cache_hash = salt.utils.path.join(cache_root, self._cache_basehash) + try: + self._cache_basename = self.get_checkout_target() + except AttributeError: + self._cache_basename = "_" + self._cache_full_basename = salt.utils.path.join( + self._cache_basehash, self._cache_basename ) - - 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) + self._cachedir = salt.utils.path.join(self._cache_hash, self._cache_basename) + self._linkdir = salt.utils.path.join( + cache_root, "links", self._cache_full_basename + ) + if not os.path.isdir(self._cachedir): + os.makedirs(self._cachedir) try: self.new = self.init_remote() @@ -510,11 +490,23 @@ def __init__( log.critical(msg, exc_info=True) failhard(self.role) - def full_id(self): - return self._full_id + def get_cache_basehash(self): + return self._cache_basehash + + def get_cache_hash(self): + return self._cache_hash + + def get_cache_basename(self): + return self._cache_basename + + def get_cache_full_basename(self): + return self._cache_full_basename - def get_cachedir_basename(self): - return self.cachedir_basename + def get_cachedir(self): + return self._cachedir + + def get_linkdir(self): + return self._linkdir def _get_envs_from_ref_paths(self, refs): """ @@ -643,7 +635,7 @@ def check_root(self): # No need to pass an environment to self.root() here since per-saltenv # configuration is a gitfs-only feature and check_root() is not used # for gitfs. - root_dir = salt.utils.path.join(self.cachedir, self.root()).rstrip(os.sep) + root_dir = salt.utils.path.join(self._cachedir, self.root()).rstrip(os.sep) if os.path.isdir(root_dir): return root_dir log.error( @@ -1068,7 +1060,7 @@ def init_remote(self): """ raise NotImplementedError() - def checkout(self): + def checkout(self, fetch_on_fail=True): """ This function must be overridden in a sub-class """ @@ -1274,7 +1266,7 @@ def __init__( role, ) - def checkout(self): + def checkout(self, fetch_on_fail=True): """ Checkout the configured branch/tag. We catch an "Exception" class here instead of a specific exception class because the exceptions raised by @@ -1344,6 +1336,15 @@ def checkout(self): except Exception: # pylint: disable=broad-except continue return self.check_root() + if fetch_on_fail: + log.debug( + "Failed to checkout %s from %s remote '%s': fetch and try again", + tgt_ref, + self.role, + self.id, + ) + self.fetch() + return self.checkout(fetch_on_fail=False) log.error( "Failed to checkout %s from %s remote '%s': remote ref does not exist", tgt_ref, @@ -1359,16 +1360,16 @@ def init_remote(self): initialized by this function. """ new = False - if not os.listdir(self.cachedir): + if not os.listdir(self._cachedir): # Repo cachedir is empty, initialize a new repo there - self.repo = git.Repo.init(self.cachedir) + self.repo = git.Repo.init(self._cachedir) new = True else: # Repo cachedir exists, try to attach try: - self.repo = git.Repo(self.cachedir) + self.repo = git.Repo(self._cachedir) except git.exc.InvalidGitRepositoryError: - log.error(_INVALID_REPO, self.cachedir, self.url, self.role) + log.error(_INVALID_REPO, self._cachedir, self.url, self.role) return new self.gitdir = salt.utils.path.join(self.repo.working_dir, ".git") @@ -1602,7 +1603,7 @@ def peel(self, obj): except AttributeError: return obj.get_object() - def checkout(self): + def checkout(self, fetch_on_fail=True): """ Checkout the configured branch/tag """ @@ -1795,6 +1796,15 @@ def _perform_checkout(checkout_ref, branch=True): exc_info=True, ) return None + if fetch_on_fail: + log.debug( + "Failed to checkout %s from %s remote '%s': fetch and try again", + tgt_ref, + self.role, + self.id, + ) + self.fetch() + return self.checkout(fetch_on_fail=False) log.error( "Failed to checkout %s from %s remote '%s': remote ref does not exist", tgt_ref, @@ -1836,16 +1846,16 @@ def init_remote(self): home = os.path.expanduser("~") pygit2.settings.search_path[pygit2.GIT_CONFIG_LEVEL_GLOBAL] = home new = False - if not os.listdir(self.cachedir): + if not os.listdir(self._cachedir): # Repo cachedir is empty, initialize a new repo there - self.repo = pygit2.init_repository(self.cachedir) + self.repo = pygit2.init_repository(self._cachedir) new = True else: # Repo cachedir exists, try to attach try: - self.repo = pygit2.Repository(self.cachedir) + self.repo = pygit2.Repository(self._cachedir) except KeyError: - log.error(_INVALID_REPO, self.cachedir, self.url, self.role) + log.error(_INVALID_REPO, self._cachedir, self.url, self.role) return new self.gitdir = salt.utils.path.join(self.repo.workdir, ".git") @@ -2487,7 +2497,7 @@ def init_remotes( # Don't allow collisions in cachedir naming cachedir_map = {} for repo in self.remotes: - cachedir_map.setdefault(repo.cachedir, []).append(repo.id) + cachedir_map.setdefault(repo.get_cachedir(), []).append(repo.id) collisions = [x for x in cachedir_map if len(cachedir_map[x]) > 1] if collisions: @@ -2512,10 +2522,11 @@ def clear_old_remotes(self): cachedir_ls = os.listdir(self.cache_root) except OSError: cachedir_ls = [] + log.critical(cachedir_ls) # Remove actively-used remotes from list for repo in self.remotes: try: - cachedir_ls.remove(repo.cachedir_basename) + cachedir_ls.remove(repo.get_cache_basehash()) except ValueError: pass to_remove = [] @@ -2858,7 +2869,7 @@ def write_remote_map(self): for repo in self.remotes: fp_.write( salt.utils.stringutils.to_str( - "{} = {}\n".format(repo.cachedir_basename, repo.id) + "{} = {}\n".format(repo.get_cache_basehash(), repo.id) ) ) except OSError: @@ -3303,8 +3314,10 @@ def link_mountpoint(self, repo): Ensure that the mountpoint is present in the correct location and points at the correct path """ - lcachelink = salt.utils.path.join(repo.linkdir, repo._mountpoint) - lcachedest = salt.utils.path.join(repo.cachedir, repo.root()).rstrip(os.sep) + lcachelink = salt.utils.path.join(repo.get_linkdir(), repo._mountpoint) + lcachedest = salt.utils.path.join(repo.get_cachedir(), repo.root()).rstrip( + os.sep + ) wipe_linkdir = False create_link = False try: From 3dd4f6437c29d509f52d3d33558008eee81823a1 Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Mon, 21 Aug 2023 20:46:05 -0500 Subject: [PATCH 02/31] clear cache after upgrade or downgrade --- salt/master.py | 4 ++++ salt/minion.py | 2 ++ salt/utils/cache.py | 31 +++++++++++++++++++++++++++++++ 3 files changed, 37 insertions(+) diff --git a/salt/master.py b/salt/master.py index 9d2239bffbe2..56deff1f28b2 100644 --- a/salt/master.py +++ b/salt/master.py @@ -604,6 +604,10 @@ def _pre_flight(self): errors = [] critical_errors = [] + import salt.utils.cache + + salt.utils.cache.verify_cache_version(self.opts["cachedir"]) + try: os.chdir("/") except OSError as err: diff --git a/salt/minion.py b/salt/minion.py index d85f5341cba2..9fc7fe81ea52 100644 --- a/salt/minion.py +++ b/salt/minion.py @@ -37,6 +37,7 @@ import salt.syspaths import salt.transport import salt.utils.args +import salt.utils.cache import salt.utils.context import salt.utils.crypt import salt.utils.data @@ -1248,6 +1249,7 @@ def __init__( self.jid_queue = [] if jid_queue is None else jid_queue self.periodic_callbacks = {} + salt.utils.cache.verify_cache_version(self.opts["cachedir"]) if io_loop is None: self.io_loop = salt.ext.tornado.ioloop.IOLoop.current() else: diff --git a/salt/utils/cache.py b/salt/utils/cache.py index a78a1f70fc91..0f7c3166471f 100644 --- a/salt/utils/cache.py +++ b/salt/utils/cache.py @@ -6,6 +6,7 @@ import logging import os import re +import shutil import time import salt.config @@ -15,6 +16,8 @@ import salt.utils.dictupdate import salt.utils.files import salt.utils.msgpack +import salt.utils.path +import salt.version from salt.utils.zeromq import zmq log = logging.getLogger(__name__) @@ -345,3 +348,31 @@ def context_cache_wrap(*args, **kwargs): return func(*args, **kwargs) return context_cache_wrap + + +def verify_cache_version(cache): + """ + Check that cache version matches salt version. + If cache version deos not matches salt version wipe the cache. + + :return: True if cache version matched. False if cache version did not match. + """ + with salt.utils.files.fopen( + salt.utils.path.join(cache, "cache_version"), "a+" + ) as file: + file.seek(0) + data = "\n".join(file.readlines()) + if data != salt.version.__version__: + log.warning(f"Cache version mismatch clearing: {repr(cache)}") + file.truncate(0) + file.write(salt.version.__version__) + for item in os.listdir(cache): + if item != "cache_version": + log.critical(item) + item_path = salt.utils.path.join(cache, item) + if os.path.isfile(item_path): + os.remove(item_path) + else: + shutil.rmtree(item_path) + return False + return True From b8a5590fe45ee26cb77b14072cbdc5afdf5dd672 Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Tue, 22 Aug 2023 19:00:39 -0500 Subject: [PATCH 03/31] clean up clean old remotes --- changelog/65002.fixed.md | 1 + salt/utils/cache.py | 1 - salt/utils/gitfs.py | 73 +++++++++++++++++++--------------------- 3 files changed, 36 insertions(+), 39 deletions(-) create mode 100644 changelog/65002.fixed.md diff --git a/changelog/65002.fixed.md b/changelog/65002.fixed.md new file mode 100644 index 000000000000..86ed2d4bccd8 --- /dev/null +++ b/changelog/65002.fixed.md @@ -0,0 +1 @@ +Fix __env__ and improve cache cleaning see more info at pull #65017. diff --git a/salt/utils/cache.py b/salt/utils/cache.py index 0f7c3166471f..b54f0d400dce 100644 --- a/salt/utils/cache.py +++ b/salt/utils/cache.py @@ -368,7 +368,6 @@ def verify_cache_version(cache): file.write(salt.version.__version__) for item in os.listdir(cache): if item != "cache_version": - log.critical(item) item_path = salt.utils.path.join(cache, item) if os.path.isfile(item_path): os.remove(item_path) diff --git a/salt/utils/gitfs.py b/salt/utils/gitfs.py index b644d854f885..9fe5ea3e760b 100644 --- a/salt/utils/gitfs.py +++ b/salt/utils/gitfs.py @@ -473,6 +473,7 @@ def __init__( self._cache_basehash, self._cache_basename ) self._cachedir = salt.utils.path.join(self._cache_hash, self._cache_basename) + self._salt_working_dir = salt.utils.path.join(self._cachedir, ".git", ".salt") self._linkdir = salt.utils.path.join( cache_root, "links", self._cache_full_basename ) @@ -508,6 +509,9 @@ def get_cachedir(self): def get_linkdir(self): return self._linkdir + def get_salt_working_dir(self): + return self._salt_working_dir + def _get_envs_from_ref_paths(self, refs): """ Return the names of remote refs (stripped of the remote name) and tags @@ -2514,49 +2518,42 @@ def init_remotes( if any(x.new for x in self.remotes): self.write_remote_map() + def _remove_cache_dir(self, cache_dir): + try: + shutil.rmtree(cache_dir) + except OSError as exc: + log.error( + "Unable to remove old %s remote cachedir %s: %s", + self.role, + cache_dir, + exc, + ) + return False + log.debug("%s removed old cachedir %s", self.role, cache_dir) + return True + + def _iter_remote_hashs(self): + for item in os.listdir(self.cache_root): + if item in ("hash", "refs", "links"): + continue + if os.path.isdir(salt.utils.path.join(self.cache_root, item)): + yield item + def clear_old_remotes(self): """ Remove cache directories for remotes no longer configured """ - try: - cachedir_ls = os.listdir(self.cache_root) - except OSError: - cachedir_ls = [] - log.critical(cachedir_ls) - # Remove actively-used remotes from list - for repo in self.remotes: - try: - cachedir_ls.remove(repo.get_cache_basehash()) - except ValueError: - pass - to_remove = [] - for item in cachedir_ls: - if item in ("hash", "refs"): - continue - path = salt.utils.path.join(self.cache_root, item) - if os.path.isdir(path): - to_remove.append(path) - failed = [] - if to_remove: - for rdir in to_remove: - try: - shutil.rmtree(rdir) - except OSError as exc: - log.error( - "Unable to remove old %s remote cachedir %s: %s", - self.role, - rdir, - exc, - ) - failed.append(rdir) - else: - log.debug("%s removed old cachedir %s", self.role, rdir) - for fdir in failed: - to_remove.remove(fdir) - ret = bool(to_remove) - if ret: + change = False + # Remove all hash dirs not part of this group + remote_set = {r.get_cache_basehash() for r in self.remotes} + for item in self._iter_remote_hashs(): + if item not in remote_set: + change = change or self._remove_cache_dir( + salt.utils.path.join(self.cache_root, item) + ) + if not change: self.write_remote_map() - return ret + return change def clear_cache(self): """ From 720edbf0fc3763bcbc924604e9346ba621026b61 Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Tue, 22 Aug 2023 22:11:43 -0500 Subject: [PATCH 04/31] place fetch request file on fetch --- salt/utils/gitfs.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/salt/utils/gitfs.py b/salt/utils/gitfs.py index 9fe5ea3e760b..8eaae1f8f119 100644 --- a/salt/utils/gitfs.py +++ b/salt/utils/gitfs.py @@ -491,6 +491,9 @@ def __init__( log.critical(msg, exc_info=True) failhard(self.role) + if not os.path.isdir(self._salt_working_dir): + os.makedirs(self._salt_working_dir) + def get_cache_basehash(self): return self._cache_basehash @@ -2612,6 +2615,29 @@ def fetch_remotes(self, remotes=None): name = getattr(repo, "name", None) if not remotes or (repo.id, name) in remotes or name in remotes: try: + # Find and place fetch_request file for all the other branches for this repo + for branch in os.listdir(repo.get_cache_hash()): + # Don't place fetch request in current branch being updated + if branch == repo.get_cache_basename(): + continue + branch_salt_dir = salt.utils.path.join( + repo.get_cache_hash(), branch, ".git", ".salt" + ) + fetch_path = salt.utils.path.join( + branch_salt_dir, "fetch_request" + ) + if os.path.isdir(branch_salt_dir): + try: + with salt.utils.files.fopen(fetch_path, "w"): + pass + except Exception as exc: # pylint: disable=broad-except + log.error( + f"Failed to place fetch request: {fetch_path}", + exc, + exc_info=True, + ) + else: + log.error(f"Failed to place fetch request: {fetch_path}") if repo.fetch(): # We can't just use the return value from repo.fetch() # because the data could still have changed if old From 9366833c99118e14166cf46a23d1156d549591e4 Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Wed, 23 Aug 2023 00:08:19 -0500 Subject: [PATCH 05/31] fetch_request_check --- salt/utils/gitfs.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/salt/utils/gitfs.py b/salt/utils/gitfs.py index 8eaae1f8f119..83c4b6e0f79d 100644 --- a/salt/utils/gitfs.py +++ b/salt/utils/gitfs.py @@ -1190,6 +1190,18 @@ def get_url(self): else: self.url = self.id + def fetch_request_check(self): + fetch_request = salt.utils.path.join(self._salt_working_dir, "fetch_request") + if os.path.isfile(fetch_request): + log.debug(f"Fetch request: {self._salt_working_dir}") + try: + os.remove(fetch_request) + except Exception: # pylint: disable=broad-except + pass + self.fetch() + return True + return False + @property def linkdir_walk(self): """ @@ -1280,6 +1292,7 @@ def checkout(self, fetch_on_fail=True): GitPython when running these functions vary in different versions of GitPython. """ + self.fetch_request_check() tgt_ref = self.get_checkout_target() try: head_sha = self.repo.rev_parse("HEAD").hexsha @@ -1614,6 +1627,7 @@ def checkout(self, fetch_on_fail=True): """ Checkout the configured branch/tag """ + self.fetch_request_check() tgt_ref = self.get_checkout_target() local_ref = "refs/heads/" + tgt_ref remote_ref = "refs/remotes/origin/" + tgt_ref From d8061b4c3413c15c03acd2a7ec484168c0bae4c3 Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Wed, 23 Aug 2023 10:57:35 -0500 Subject: [PATCH 06/31] fire fetch request --- salt/utils/gitfs.py | 1 + 1 file changed, 1 insertion(+) diff --git a/salt/utils/gitfs.py b/salt/utils/gitfs.py index 83c4b6e0f79d..8bdb9b7eea2c 100644 --- a/salt/utils/gitfs.py +++ b/salt/utils/gitfs.py @@ -493,6 +493,7 @@ def __init__( if not os.path.isdir(self._salt_working_dir): os.makedirs(self._salt_working_dir) + self.fetch_request_check() def get_cache_basehash(self): return self._cache_basehash From 0d27aa0d95c277481fdc3cecf06078d1ba2de46b Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Wed, 23 Aug 2023 23:48:50 -0500 Subject: [PATCH 07/31] update some unit tests --- salt/utils/gitfs.py | 15 +++++++------ tests/pytests/unit/test_minion.py | 29 ++++++++++---------------- tests/pytests/unit/utils/test_gitfs.py | 20 ++++-------------- tests/unit/utils/test_gitfs.py | 21 ++++--------------- 4 files changed, 28 insertions(+), 57 deletions(-) diff --git a/salt/utils/gitfs.py b/salt/utils/gitfs.py index 8bdb9b7eea2c..ca0fa3c6b6a0 100644 --- a/salt/utils/gitfs.py +++ b/salt/utils/gitfs.py @@ -465,10 +465,13 @@ def __init__( "/", "_" ) # replace "/" with "_" to not cause trouble with file system self._cache_hash = salt.utils.path.join(cache_root, self._cache_basehash) - try: - self._cache_basename = self.get_checkout_target() - except AttributeError: - self._cache_basename = "_" + self._cache_basename = "_" + if self.id.startswith("__env__"): + try: + self._cache_basename = self.get_checkout_target() + except AttributeError: + log.critical(f"__env__ cant generate basename: {self.role} {self.id}") + failhard(self.role) self._cache_full_basename = salt.utils.path.join( self._cache_basehash, self._cache_basename ) @@ -2566,8 +2569,8 @@ def clear_old_remotes(self): remote_set = {r.get_cache_basehash() for r in self.remotes} for item in self._iter_remote_hashs(): if item not in remote_set: - change = change or self._remove_cache_dir( - salt.utils.path.join(self.cache_root, item) + change = self._remove_cache_dir( + salt.utils.path.join(self.cache_root, item) or change ) if not change: self.write_remote_map() diff --git a/tests/pytests/unit/test_minion.py b/tests/pytests/unit/test_minion.py index 1cee025a485b..90b9b8a821a4 100644 --- a/tests/pytests/unit/test_minion.py +++ b/tests/pytests/unit/test_minion.py @@ -21,35 +21,33 @@ log = logging.getLogger(__name__) -def test_minion_load_grains_false(): +def test_minion_load_grains_false(minion_opts): """ Minion does not generate grains when load_grains is False """ - opts = {"random_startup_delay": 0, "grains": {"foo": "bar"}} + minion_opts["grains"] = {"foo": "bar"} with patch("salt.loader.grains") as grainsfunc: - minion = salt.minion.Minion(opts, load_grains=False) - assert minion.opts["grains"] == opts["grains"] + minion = salt.minion.Minion(minion_opts, load_grains=False) + assert minion.opts["grains"] == minion_opts["grains"] grainsfunc.assert_not_called() -def test_minion_load_grains_true(): +def test_minion_load_grains_true(minion_opts): """ Minion generates grains when load_grains is True """ - opts = {"random_startup_delay": 0, "grains": {}} with patch("salt.loader.grains") as grainsfunc: - minion = salt.minion.Minion(opts, load_grains=True) + minion = salt.minion.Minion(minion_opts, load_grains=True) assert minion.opts["grains"] != {} grainsfunc.assert_called() -def test_minion_load_grains_default(): +def test_minion_load_grains_default(minion_opts): """ Minion load_grains defaults to True """ - opts = {"random_startup_delay": 0, "grains": {}} with patch("salt.loader.grains") as grainsfunc: - minion = salt.minion.Minion(opts) + minion = salt.minion.Minion(minion_opts) assert minion.opts["grains"] != {} grainsfunc.assert_called() @@ -140,22 +138,17 @@ async def test_send_req_async_regression_62453(minion_opts): assert rtn is False -def test_mine_send_tries(): +def test_mine_send_tries(minion_opts): channel_enter = MagicMock() channel_enter.send.side_effect = lambda load, timeout, tries: tries channel = MagicMock() channel.__enter__.return_value = channel_enter - opts = { - "random_startup_delay": 0, - "grains": {}, - "return_retry_tries": 20, - "minion_sign_messages": False, - } + minion_opts["return_retry_tries"] = 20 with patch("salt.channel.client.ReqChannel.factory", return_value=channel), patch( "salt.loader.grains" ): - minion = salt.minion.Minion(opts) + minion = salt.minion.Minion(minion_opts) minion.tok = "token" data = {} diff --git a/tests/pytests/unit/utils/test_gitfs.py b/tests/pytests/unit/utils/test_gitfs.py index 76c9409a1afb..de630cd867e5 100644 --- a/tests/pytests/unit/utils/test_gitfs.py +++ b/tests/pytests/unit/utils/test_gitfs.py @@ -1,5 +1,4 @@ import os -import string import time import pytest @@ -231,11 +230,11 @@ def test_checkout_pygit2(_prepare_provider): provider.init_remote() provider.fetch() provider.branch = "master" - assert provider.cachedir in provider.checkout() + assert provider.get_cachedir() in provider.checkout() provider.branch = "simple_tag" - assert provider.cachedir in provider.checkout() + assert provider.get_cachedir() in provider.checkout() provider.branch = "annotated_tag" - assert provider.cachedir in provider.checkout() + assert provider.get_cachedir() in provider.checkout() provider.branch = "does_not_exist" assert provider.checkout() is None @@ -244,20 +243,9 @@ def test_checkout_pygit2(_prepare_provider): @pytest.mark.skip_on_windows( reason="Skip Pygit2 on windows, due to pygit2 access error on windows" ) -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() - # Note: if you are changing the length of basename - # keep in mind that pygit2 will error out on large file paths on Windows - 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:]) + assert "_" == _prepare_provider.get_cache_basename() diff --git a/tests/unit/utils/test_gitfs.py b/tests/unit/utils/test_gitfs.py index 6d8e97a239e7..259ea056fcd4 100644 --- a/tests/unit/utils/test_gitfs.py +++ b/tests/unit/utils/test_gitfs.py @@ -114,27 +114,14 @@ def test_update_by_id_and_name(self): self.assertTrue(self.main_class.remotes[0].fetched) self.assertFalse(self.main_class.remotes[1].fetched) - def test_full_id(self): - self.assertEqual( - self.main_class.remotes[0].full_id(), "-file://repo1.git---gitfs-master--" - ) - - def test_full_id_with_name(self): - self.assertEqual( - self.main_class.remotes[1].full_id(), - "repo2-file://repo2.git---gitfs-master--", - ) - def test_get_cachedir_basename(self): self.assertEqual( - self.main_class.remotes[0].get_cachedir_basename(), - "-jXhnbGDemchtZwTwaD2s6VOaVvs98a7w+AtiYlmOVb0=", + self.main_class.remotes[0].get_cache_basename(), + "_", ) - - def test_get_cachedir_base_with_name(self): self.assertEqual( - self.main_class.remotes[1].get_cachedir_basename(), - "repo2-nuezpiDtjQRFC0ZJDByvi+F6Vb8ZhfoH41n_KFxTGsU=", + self.main_class.remotes[1].get_cache_basename(), + "_", ) def test_git_provider_mp_lock(self): From 73397cdbb269845f29917270a3ccd9ddebe0185d Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Thu, 24 Aug 2023 00:45:45 -0500 Subject: [PATCH 08/31] fix test minion start --- salt/utils/cache.py | 12 +++++++----- salt/utils/gitfs.py | 7 +++---- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/salt/utils/cache.py b/salt/utils/cache.py index b54f0d400dce..624653c33f8e 100644 --- a/salt/utils/cache.py +++ b/salt/utils/cache.py @@ -350,25 +350,27 @@ def context_cache_wrap(*args, **kwargs): return context_cache_wrap -def verify_cache_version(cache): +def verify_cache_version(cache_path): """ Check that cache version matches salt version. If cache version deos not matches salt version wipe the cache. :return: True if cache version matched. False if cache version did not match. """ + if not os.path.isdir(cache_path): + os.mkdir(cache_path) with salt.utils.files.fopen( - salt.utils.path.join(cache, "cache_version"), "a+" + salt.utils.path.join(cache_path, "cache_version"), "a+" ) as file: file.seek(0) data = "\n".join(file.readlines()) if data != salt.version.__version__: - log.warning(f"Cache version mismatch clearing: {repr(cache)}") + log.warning(f"Cache version mismatch clearing: {repr(cache_path)}") file.truncate(0) file.write(salt.version.__version__) - for item in os.listdir(cache): + for item in os.listdir(cache_path): if item != "cache_version": - item_path = salt.utils.path.join(cache, item) + item_path = salt.utils.path.join(cache_path, item) if os.path.isfile(item_path): os.remove(item_path) else: diff --git a/salt/utils/gitfs.py b/salt/utils/gitfs.py index ca0fa3c6b6a0..a5a2ecf968ad 100644 --- a/salt/utils/gitfs.py +++ b/salt/utils/gitfs.py @@ -2553,7 +2553,7 @@ def _remove_cache_dir(self, cache_dir): log.debug("%s removed old cachedir %s", self.role, cache_dir) return True - def _iter_remote_hashs(self): + def _iter_remote_hashes(self): for item in os.listdir(self.cache_root): if item in ("hash", "refs", "links"): continue @@ -2567,7 +2567,7 @@ def clear_old_remotes(self): change = False # Remove all hash dirs not part of this group remote_set = {r.get_cache_basehash() for r in self.remotes} - for item in self._iter_remote_hashs(): + for item in self._iter_remote_hashes(): if item not in remote_set: change = self._remove_cache_dir( salt.utils.path.join(self.cache_root, item) or change @@ -2650,8 +2650,7 @@ def fetch_remotes(self, remotes=None): pass except Exception as exc: # pylint: disable=broad-except log.error( - f"Failed to place fetch request: {fetch_path}", - exc, + f"Failed to place fetch request: {fetch_path} {exc}", exc_info=True, ) else: From 35a931c9df22f7cc7afcda958e19d13b9efe751c Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Fri, 25 Aug 2023 13:45:08 -0500 Subject: [PATCH 09/31] only clear cache in gtfs dirs --- salt/master.py | 4 ---- salt/minion.py | 2 -- salt/utils/gitfs.py | 2 ++ 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/salt/master.py b/salt/master.py index cc1cba378a57..5a317bf93b72 100644 --- a/salt/master.py +++ b/salt/master.py @@ -604,10 +604,6 @@ def _pre_flight(self): errors = [] critical_errors = [] - import salt.utils.cache - - salt.utils.cache.verify_cache_version(self.opts["cachedir"]) - try: os.chdir("/") except OSError as err: diff --git a/salt/minion.py b/salt/minion.py index 1147bef40777..a8d936701cb3 100644 --- a/salt/minion.py +++ b/salt/minion.py @@ -37,7 +37,6 @@ import salt.syspaths import salt.transport import salt.utils.args -import salt.utils.cache import salt.utils.context import salt.utils.crypt import salt.utils.data @@ -1249,7 +1248,6 @@ def __init__( self.jid_queue = [] if jid_queue is None else jid_queue self.periodic_callbacks = {} - salt.utils.cache.verify_cache_version(self.opts["cachedir"]) if io_loop is None: self.io_loop = salt.ext.tornado.ioloop.IOLoop.current() else: diff --git a/salt/utils/gitfs.py b/salt/utils/gitfs.py index a5a2ecf968ad..4a29d405a722 100644 --- a/salt/utils/gitfs.py +++ b/salt/utils/gitfs.py @@ -24,6 +24,7 @@ import salt.ext.tornado.ioloop import salt.fileserver +import salt.utils.cache import salt.utils.configparser import salt.utils.data import salt.utils.files @@ -2400,6 +2401,7 @@ def fetch_remotes(self): self.file_list_cachedir = salt.utils.path.join( self.opts["cachedir"], "file_lists", self.role ) + salt.utils.cache.verify_cache_version(self.cache_root) if init_remotes: self.init_remotes( remotes if remotes is not None else [], From 61da06ea58d26b730fae724e736e26e8b12243d9 Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Fri, 25 Aug 2023 15:29:18 -0500 Subject: [PATCH 10/31] add cache tests --- salt/utils/cache.py | 2 +- tests/pytests/functional/utils/test_cache.py | 70 ++++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 tests/pytests/functional/utils/test_cache.py diff --git a/salt/utils/cache.py b/salt/utils/cache.py index 624653c33f8e..d3a8084251d8 100644 --- a/salt/utils/cache.py +++ b/salt/utils/cache.py @@ -358,7 +358,7 @@ def verify_cache_version(cache_path): :return: True if cache version matched. False if cache version did not match. """ if not os.path.isdir(cache_path): - os.mkdir(cache_path) + os.makedirs(cache_path) with salt.utils.files.fopen( salt.utils.path.join(cache_path, "cache_version"), "a+" ) as file: diff --git a/tests/pytests/functional/utils/test_cache.py b/tests/pytests/functional/utils/test_cache.py new file mode 100644 index 000000000000..c2037f84c435 --- /dev/null +++ b/tests/pytests/functional/utils/test_cache.py @@ -0,0 +1,70 @@ +import os + +import salt.utils.cache +import salt.utils.files +import salt.utils.path + +_ROOT_DIR = ( + "data.txt", + "foo.t2", + "bar.t3", + "nested/test", + "nested/cache.txt", + "n/n1/n2/n3/n4/n5", +) + + +def _make_dummy_files(tmp_path): + for full_path in _ROOT_DIR: + full_path = salt.utils.path.join(tmp_path, full_path) + path, _ = os.path.split(full_path) + if not os.path.isdir(path): + os.makedirs(path) + with salt.utils.files.fopen(full_path, "w") as file: + file.write("data") + + +def _dummy_files_exists(tmp_path): + """ + True if all files exists + False if all files are missing + None if some files exists and others are missing + """ + ret = None + for full_path in _ROOT_DIR: + full_path = salt.utils.path.join(tmp_path, full_path) + path, _ = os.path.split(full_path) + is_file = os.path.isdir(path) and os.path.isfile(full_path) + if ret is None: + ret = is_file + elif ret is not is_file: + return None # Some files are found and others are missing + return ret + + +def test_verify_cache_version(tmp_path): + tmp_path = str(tmp_path) + cache_version = salt.utils.path.join(tmp_path, "cache_version") + + # check that cache clears when no cache_version is present + _make_dummy_files(tmp_path) + assert salt.utils.cache.verify_cache_version(tmp_path) is False + assert _dummy_files_exists(tmp_path) is False + + # check that cache does not get clear when check is called multiple times + _make_dummy_files(tmp_path) + for _ in range(3): + assert salt.utils.cache.verify_cache_version(tmp_path) is True + assert _dummy_files_exists(tmp_path) is True + + # check that cache clears when a different version is present + with salt.utils.files.fopen(cache_version, "w") as file: + file.write("-1") + assert salt.utils.cache.verify_cache_version(tmp_path) is False + assert _dummy_files_exists(tmp_path) is False + + # check that cache does not get clear when check is called multiple times + _make_dummy_files(tmp_path) + for _ in range(3): + assert salt.utils.cache.verify_cache_version(tmp_path) is True + assert _dummy_files_exists(tmp_path) is True From 03dae694f719583035bc58eea9d39f7706905980 Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Fri, 25 Aug 2023 16:55:24 -0500 Subject: [PATCH 11/31] doc and exception update --- salt/utils/cache.py | 6 +++--- salt/utils/gitfs.py | 19 ++++++++++++++----- tests/pytests/functional/utils/test_cache.py | 2 +- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/salt/utils/cache.py b/salt/utils/cache.py index d3a8084251d8..88e7fa24000e 100644 --- a/salt/utils/cache.py +++ b/salt/utils/cache.py @@ -352,10 +352,10 @@ def context_cache_wrap(*args, **kwargs): def verify_cache_version(cache_path): """ - Check that cache version matches salt version. - If cache version deos not matches salt version wipe the cache. + Check that the cached version matches the Salt version. + If the cached version does not match the Salt version, wipe the cache. - :return: True if cache version matched. False if cache version did not match. + :return: ``True`` if cache version matches, otherwise ``False`` """ if not os.path.isdir(cache_path): os.makedirs(cache_path) diff --git a/salt/utils/gitfs.py b/salt/utils/gitfs.py index 4a29d405a722..692fe410ada3 100644 --- a/salt/utils/gitfs.py +++ b/salt/utils/gitfs.py @@ -1201,8 +1201,11 @@ def fetch_request_check(self): log.debug(f"Fetch request: {self._salt_working_dir}") try: os.remove(fetch_request) - except Exception: # pylint: disable=broad-except - pass + except OSError as exc: + log.error( + f"Failed to remove Fetch request: {self._salt_working_dir} {exc}", + exc_info=True, + ) self.fetch() return True return False @@ -1296,6 +1299,9 @@ def checkout(self, fetch_on_fail=True): instead of a specific exception class because the exceptions raised by GitPython when running these functions vary in different versions of GitPython. + + fetch_on_fail + If checkout fails perform a fetch then try to checkout again. """ self.fetch_request_check() tgt_ref = self.get_checkout_target() @@ -1631,6 +1637,9 @@ def peel(self, obj): def checkout(self, fetch_on_fail=True): """ Checkout the configured branch/tag + + fetch_on_fail + If checkout fails perform a fetch then try to checkout again. """ self.fetch_request_check() tgt_ref = self.get_checkout_target() @@ -2650,13 +2659,13 @@ def fetch_remotes(self, remotes=None): try: with salt.utils.files.fopen(fetch_path, "w"): pass - except Exception as exc: # pylint: disable=broad-except + except OSError as exc: # pylint: disable=broad-except log.error( - f"Failed to place fetch request: {fetch_path} {exc}", + f"Failed to make fetch request: {fetch_path} {exc}", exc_info=True, ) else: - log.error(f"Failed to place fetch request: {fetch_path}") + log.error(f"Failed to make fetch request: {fetch_path}") if repo.fetch(): # We can't just use the return value from repo.fetch() # because the data could still have changed if old diff --git a/tests/pytests/functional/utils/test_cache.py b/tests/pytests/functional/utils/test_cache.py index c2037f84c435..0527a96b880d 100644 --- a/tests/pytests/functional/utils/test_cache.py +++ b/tests/pytests/functional/utils/test_cache.py @@ -34,7 +34,7 @@ def _dummy_files_exists(tmp_path): for full_path in _ROOT_DIR: full_path = salt.utils.path.join(tmp_path, full_path) path, _ = os.path.split(full_path) - is_file = os.path.isdir(path) and os.path.isfile(full_path) + is_file = os.path.isfile(full_path) if ret is None: ret = is_file elif ret is not is_file: From 60485967681530e1e2becb3230d6248f160e68eb Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Mon, 24 Jul 2023 14:31:21 -0500 Subject: [PATCH 12/31] linkdir -> _linkdir --- salt/utils/gitfs.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/salt/utils/gitfs.py b/salt/utils/gitfs.py index 692fe410ada3..1ec5dcf739ad 100644 --- a/salt/utils/gitfs.py +++ b/salt/utils/gitfs.py @@ -1236,14 +1236,14 @@ def linkdir_walk(self): dirs = [] self._linkdir_walk.append( ( - salt.utils.path.join(self.linkdir, *parts[: idx + 1]), + salt.utils.path.join(self._linkdir, *parts[: idx + 1]), dirs, [], ) ) try: # The linkdir itself goes at the beginning - self._linkdir_walk.insert(0, (self.linkdir, [parts[0]], [])) + self._linkdir_walk.insert(0, (self._linkdirlinkdir, [parts[0]], [])) except IndexError: pass return self._linkdir_walk @@ -3355,8 +3355,8 @@ def checkout(self): env = "base" if tgt == repo.base else tgt if repo._mountpoint: if self.link_mountpoint(repo): - self.pillar_dirs[repo.linkdir] = env - self.pillar_linked_dirs.append(repo.linkdir) + self.pillar_dirs[repo.get_linkdir()] = env + self.pillar_linked_dirs.append(repo.get_linkdir()) else: self.pillar_dirs[cachedir] = env @@ -3373,11 +3373,11 @@ def link_mountpoint(self, repo): create_link = False try: with repo.gen_lock(lock_type="mountpoint", timeout=10): - walk_results = list(os.walk(repo.linkdir, followlinks=False)) + walk_results = list(os.walk(repo.get_linkdir(), followlinks=False)) if walk_results != repo.linkdir_walk: log.debug( "Results of walking %s differ from expected results", - repo.linkdir, + repo.get_linkdir(), ) log.debug("Walk results: %s", walk_results) log.debug("Expected results: %s", repo.linkdir_walk) @@ -3438,7 +3438,7 @@ def link_mountpoint(self, repo): # Wiping implies that we need to create the link create_link = True try: - shutil.rmtree(repo.linkdir) + shutil.rmtree(repo.get_linkdir()) except OSError: pass try: From b5437ced24443d01b869cf450ed2659a3508033f Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Fri, 25 Aug 2023 20:39:05 -0500 Subject: [PATCH 13/31] bad copy --- salt/utils/gitfs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/utils/gitfs.py b/salt/utils/gitfs.py index 1ec5dcf739ad..ecd67d977350 100644 --- a/salt/utils/gitfs.py +++ b/salt/utils/gitfs.py @@ -1243,7 +1243,7 @@ def linkdir_walk(self): ) try: # The linkdir itself goes at the beginning - self._linkdir_walk.insert(0, (self._linkdirlinkdir, [parts[0]], [])) + self._linkdir_walk.insert(0, (self._linkdir, [parts[0]], [])) except IndexError: pass return self._linkdir_walk From fea95bb91e1071b133044c37a97cc631fc4c81d7 Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Sat, 26 Aug 2023 20:18:35 -0500 Subject: [PATCH 14/31] fix pygit2 --- salt/utils/gitfs.py | 16 ++++++++-------- tests/pytests/functional/utils/test_cache.py | 7 +++---- tests/pytests/unit/pillar/test_git_pillar.py | 0 3 files changed, 11 insertions(+), 12 deletions(-) create mode 100644 tests/pytests/unit/pillar/test_git_pillar.py diff --git a/salt/utils/gitfs.py b/salt/utils/gitfs.py index ecd67d977350..bb8bd3266e02 100644 --- a/salt/utils/gitfs.py +++ b/salt/utils/gitfs.py @@ -477,7 +477,7 @@ def __init__( self._cache_basehash, self._cache_basename ) self._cachedir = salt.utils.path.join(self._cache_hash, self._cache_basename) - self._salt_working_dir = salt.utils.path.join(self._cachedir, ".git", ".salt") + self._salt_working_dir = salt.utils.path.join(cache_root, "work", self._cache_full_basename) self._linkdir = salt.utils.path.join( cache_root, "links", self._cache_full_basename ) @@ -494,6 +494,9 @@ def __init__( msg += " Perhaps git is not available." log.critical(msg, exc_info=True) failhard(self.role) + else: + self.verify_auth() + self.setup_callbacks() if not os.path.isdir(self._salt_working_dir): os.makedirs(self._salt_working_dir) @@ -2483,8 +2486,6 @@ def init_remotes( ) if hasattr(repo_obj, "repo"): # Sanity check and assign the credential parameter - repo_obj.verify_auth() - repo_obj.setup_callbacks() if self.opts["__role"] == "minion" and repo_obj.new: # Perform initial fetch on masterless minion repo_obj.fetch() @@ -2566,7 +2567,7 @@ def _remove_cache_dir(self, cache_dir): def _iter_remote_hashes(self): for item in os.listdir(self.cache_root): - if item in ("hash", "refs", "links"): + if item in ("hash", "refs", "links", "work"): continue if os.path.isdir(salt.utils.path.join(self.cache_root, item)): yield item @@ -2645,13 +2646,12 @@ def fetch_remotes(self, remotes=None): if not remotes or (repo.id, name) in remotes or name in remotes: try: # Find and place fetch_request file for all the other branches for this repo - for branch in os.listdir(repo.get_cache_hash()): + repo_work_hash = os.path.split(repo.get_salt_working_dir())[0] + for branch in os.listdir(repo_work_hash): # Don't place fetch request in current branch being updated if branch == repo.get_cache_basename(): continue - branch_salt_dir = salt.utils.path.join( - repo.get_cache_hash(), branch, ".git", ".salt" - ) + branch_salt_dir = salt.utils.path.join(repo_work_hash, branch) fetch_path = salt.utils.path.join( branch_salt_dir, "fetch_request" ) diff --git a/tests/pytests/functional/utils/test_cache.py b/tests/pytests/functional/utils/test_cache.py index 0527a96b880d..98c6946b653b 100644 --- a/tests/pytests/functional/utils/test_cache.py +++ b/tests/pytests/functional/utils/test_cache.py @@ -4,7 +4,7 @@ import salt.utils.files import salt.utils.path -_ROOT_DIR = ( +_DUMMY_FILES = ( "data.txt", "foo.t2", "bar.t3", @@ -15,7 +15,7 @@ def _make_dummy_files(tmp_path): - for full_path in _ROOT_DIR: + for full_path in _DUMMY_FILES: full_path = salt.utils.path.join(tmp_path, full_path) path, _ = os.path.split(full_path) if not os.path.isdir(path): @@ -31,9 +31,8 @@ def _dummy_files_exists(tmp_path): None if some files exists and others are missing """ ret = None - for full_path in _ROOT_DIR: + for full_path in _DUMMY_FILES: full_path = salt.utils.path.join(tmp_path, full_path) - path, _ = os.path.split(full_path) is_file = os.path.isfile(full_path) if ret is None: ret = is_file diff --git a/tests/pytests/unit/pillar/test_git_pillar.py b/tests/pytests/unit/pillar/test_git_pillar.py new file mode 100644 index 000000000000..e69de29bb2d1 From 0e675a2469fc68b5134bd2da1963110bee1aed86 Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Sat, 26 Aug 2023 20:25:01 -0500 Subject: [PATCH 15/31] move fetch_request to try else --- salt/utils/gitfs.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/salt/utils/gitfs.py b/salt/utils/gitfs.py index bb8bd3266e02..24af437825a6 100644 --- a/salt/utils/gitfs.py +++ b/salt/utils/gitfs.py @@ -497,10 +497,9 @@ def __init__( else: self.verify_auth() self.setup_callbacks() - - if not os.path.isdir(self._salt_working_dir): - os.makedirs(self._salt_working_dir) - self.fetch_request_check() + if not os.path.isdir(self._salt_working_dir): + os.makedirs(self._salt_working_dir) + self.fetch_request_check() def get_cache_basehash(self): return self._cache_basehash From 8cd578fe2356b64adbccf8e7831c5dcd75502a6f Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Sun, 27 Aug 2023 20:02:45 -0500 Subject: [PATCH 16/31] tests --- salt/utils/gitfs.py | 4 +- .../functional/pillar/test_git_pillar.py | 191 ++++++++++++++++++ tests/pytests/unit/pillar/test_git_pillar.py | 0 3 files changed, 194 insertions(+), 1 deletion(-) create mode 100644 tests/pytests/functional/pillar/test_git_pillar.py delete mode 100644 tests/pytests/unit/pillar/test_git_pillar.py diff --git a/salt/utils/gitfs.py b/salt/utils/gitfs.py index 24af437825a6..eb2676c75b9e 100644 --- a/salt/utils/gitfs.py +++ b/salt/utils/gitfs.py @@ -477,7 +477,9 @@ def __init__( self._cache_basehash, self._cache_basename ) self._cachedir = salt.utils.path.join(self._cache_hash, self._cache_basename) - self._salt_working_dir = salt.utils.path.join(cache_root, "work", self._cache_full_basename) + self._salt_working_dir = salt.utils.path.join( + cache_root, "work", self._cache_full_basename + ) self._linkdir = salt.utils.path.join( cache_root, "links", self._cache_full_basename ) diff --git a/tests/pytests/functional/pillar/test_git_pillar.py b/tests/pytests/functional/pillar/test_git_pillar.py new file mode 100644 index 000000000000..28bf638ed8f6 --- /dev/null +++ b/tests/pytests/functional/pillar/test_git_pillar.py @@ -0,0 +1,191 @@ +import copy + +import pytest + +from salt.pillar.git_pillar import ext_pillar +from tests.support.mock import patch + +pytestmark = [ + pytest.mark.slow_test, +] + + +try: + import git # pylint: disable=unused-import + + HAS_GITPYTHON = True +except ImportError: + HAS_GITPYTHON = False + + +try: + import pygit2 # pylint: disable=unused-import + + HAS_PYGIT2 = True +except ImportError: + HAS_PYGIT2 = False + + +skipif_no_gitpython = pytest.mark.skipif(not HAS_GITPYTHON, reason="Missing gitpython") +skipif_no_pygit2 = pytest.mark.skipif(not HAS_PYGIT2, reason="Missing pygit2") + + +@pytest.fixture +def git_pillar_opts(salt_master, tmp_path): + opts = dict(copy.deepcopy(salt_master.config)) + opts["cachedir"] = str(tmp_path) + return opts + + +@pytest.fixture +def gitpython_pillar_opts(git_pillar_opts): + git_pillar_opts["verified_git_pillar_provider"] = "gitpython" + return git_pillar_opts + + +@pytest.fixture +def pygit2_pillar_opts(git_pillar_opts): + git_pillar_opts["verified_git_pillar_provider"] = "pygit2" + return git_pillar_opts + + +def _get_ext_pillar(minion, pillar_opts, grains, *repos): + with patch("salt.pillar.git_pillar.__opts__", pillar_opts, create=True): + with patch("salt.pillar.git_pillar.__grains__", grains, create=True): + return ext_pillar(minion, None, *repos) + + +def _test_simple(pillar_opts, grains): + data = _get_ext_pillar( + "minion", + pillar_opts, + grains, + "https://github.com/saltstack/salt-test-pillar-gitfs.git", + ) + assert data == {"key": "value"} + + +@skipif_no_gitpython +def test_gitpython_simple(gitpython_pillar_opts, grains): + _test_simple(gitpython_pillar_opts, grains) + + +@skipif_no_pygit2 +def test_pygit2_simple(pygit2_pillar_opts, grains): + _test_simple(pygit2_pillar_opts, grains) + + +def _test_missing_env(pillar_opts, grains): + data = _get_ext_pillar( + "minion", + pillar_opts, + grains, + { + "https://github.com/saltstack/salt-test-pillar-gitfs.git": [ + {"env": "misssing"} + ] + }, + ) + assert data == {} + + +@skipif_no_gitpython +def test_gitpython_missing_env(gitpython_pillar_opts, grains): + _test_missing_env(gitpython_pillar_opts, grains) + + +@skipif_no_pygit2 +def test_pygit2_missing_env(pygit2_pillar_opts, grains): + _test_missing_env(pygit2_pillar_opts, grains) + + +def _test_env(pillar_opts, grains): + data = _get_ext_pillar( + "minion", + pillar_opts, + grains, + { + "https://github.com/saltstack/salt-test-pillar-gitfs-2.git": [ + {"env": "other_env"} + ] + }, + ) + assert data == {"other": "env"} + + +@skipif_no_gitpython +def test_gitpython_env(gitpython_pillar_opts, grains): + _test_env(gitpython_pillar_opts, grains) + + +@skipif_no_pygit2 +def test_pygit2_env(pygit2_pillar_opts, grains): + _test_env(pygit2_pillar_opts, grains) + + +def _test_simple_dynamic(pillar_opts, grains): + data = _get_ext_pillar( + "minion", + pillar_opts, + grains, + "__env__ https://github.com/saltstack/salt-test-pillar-gitfs.git", + ) + assert data == {"key": "value"} + + +@skipif_no_gitpython +def test_gitpython_simple_dynamic(gitpython_pillar_opts, grains): + _test_simple_dynamic(gitpython_pillar_opts, grains) + + +@skipif_no_pygit2 +def test_pygit2_simple_dynamic(pygit2_pillar_opts, grains): + _test_simple_dynamic(pygit2_pillar_opts, grains) + + +def _test_missing_env_dynamic(pillar_opts, grains): + data = _get_ext_pillar( + "minion", + pillar_opts, + grains, + { + "__env__ https://github.com/saltstack/salt-test-pillar-gitfs.git": [ + {"env": "misssing"} + ] + }, + ) + assert data == {} + + +@skipif_no_gitpython +def test_gitpython_missing_env_dynamic(gitpython_pillar_opts, grains): + _test_missing_env_dynamic(gitpython_pillar_opts, grains) + + +@skipif_no_pygit2 +def test_pygit2_missing_env_dynamic(pygit2_pillar_opts, grains): + _test_missing_env_dynamic(pygit2_pillar_opts, grains) + + +def _test_env_dynamic(pillar_opts, grains): + data = _get_ext_pillar( + "minion", + pillar_opts, + grains, + { + "__env__ https://github.com/saltstack/salt-test-pillar-gitfs-2.git": [ + {"env": "other_env"} + ] + }, + ) + assert data == {"other": "env"} + + +@skipif_no_gitpython +def test_gitpython_env_dynamic(gitpython_pillar_opts, grains): + _test_env_dynamic(gitpython_pillar_opts, grains) + + +@skipif_no_pygit2 +def test_pygit2_env_dynamic(pygit2_pillar_opts, grains): + _test_env_dynamic(pygit2_pillar_opts, grains) diff --git a/tests/pytests/unit/pillar/test_git_pillar.py b/tests/pytests/unit/pillar/test_git_pillar.py deleted file mode 100644 index e69de29bb2d1..000000000000 From 0b1d2b0965eb39468b2b8b682fd1332813cd631b Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Mon, 28 Aug 2023 08:21:54 -0500 Subject: [PATCH 17/31] pillarenv dynamic --- .../functional/pillar/test_git_pillar.py | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/tests/pytests/functional/pillar/test_git_pillar.py b/tests/pytests/functional/pillar/test_git_pillar.py index 28bf638ed8f6..2f01e3110654 100644 --- a/tests/pytests/functional/pillar/test_git_pillar.py +++ b/tests/pytests/functional/pillar/test_git_pillar.py @@ -123,6 +123,26 @@ def test_pygit2_env(pygit2_pillar_opts, grains): _test_env(pygit2_pillar_opts, grains) +def _test_branch(pillar_opts, grains): + data = _get_ext_pillar( + "minion", + pillar_opts, + grains, + "branch https://github.com/saltstack/salt-test-pillar-gitfs.git", + ) + assert data == {"key": "data"} + + +@skipif_no_gitpython +def test_gitpython_branch(gitpython_pillar_opts, grains): + _test_branch(gitpython_pillar_opts, grains) + + +@skipif_no_pygit2 +def test_pygit2_branch(pygit2_pillar_opts, grains): + _test_branch(pygit2_pillar_opts, grains) + + def _test_simple_dynamic(pillar_opts, grains): data = _get_ext_pillar( "minion", @@ -189,3 +209,24 @@ def test_gitpython_env_dynamic(gitpython_pillar_opts, grains): @skipif_no_pygit2 def test_pygit2_env_dynamic(pygit2_pillar_opts, grains): _test_env_dynamic(pygit2_pillar_opts, grains) + + +def _test_pillarenv_dynamic(pillar_opts, grains): + pillar_opts["pillarenv"] = "branch" + data = _get_ext_pillar( + "minion", + pillar_opts, + grains, + "__env__ https://github.com/saltstack/salt-test-pillar-gitfs.git", + ) + assert data == {"key": "data"} + + +@skipif_no_gitpython +def test_gitpython_pillarenv_dynamic(gitpython_pillar_opts, grains): + _test_pillarenv_dynamic(gitpython_pillar_opts, grains) + + +@skipif_no_pygit2 +def test_pygit2_pillarenv_dynamic(pygit2_pillar_opts, grains): + _test_pillarenv_dynamic(pygit2_pillar_opts, grains) From 5a5adfe2f7ab1a615e36bf6d508744721d9b5d96 Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Mon, 28 Aug 2023 09:53:14 -0500 Subject: [PATCH 18/31] fix some tests --- .../functional/pillar/test_git_pillar.py | 32 +++++++++---------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/tests/pytests/functional/pillar/test_git_pillar.py b/tests/pytests/functional/pillar/test_git_pillar.py index 2f01e3110654..f7c59558634c 100644 --- a/tests/pytests/functional/pillar/test_git_pillar.py +++ b/tests/pytests/functional/pillar/test_git_pillar.py @@ -105,7 +105,7 @@ def _test_env(pillar_opts, grains): pillar_opts, grains, { - "https://github.com/saltstack/salt-test-pillar-gitfs-2.git": [ + "other https://github.com/saltstack/salt-test-pillar-gitfs-2.git": [ {"env": "other_env"} ] }, @@ -187,46 +187,44 @@ def test_pygit2_missing_env_dynamic(pygit2_pillar_opts, grains): _test_missing_env_dynamic(pygit2_pillar_opts, grains) -def _test_env_dynamic(pillar_opts, grains): +def _test_pillarenv_dynamic(pillar_opts, grains): + pillar_opts["pillarenv"] = "branch" data = _get_ext_pillar( "minion", pillar_opts, grains, - { - "__env__ https://github.com/saltstack/salt-test-pillar-gitfs-2.git": [ - {"env": "other_env"} - ] - }, + "__env__ https://github.com/saltstack/salt-test-pillar-gitfs.git", ) - assert data == {"other": "env"} + assert data == {"key": "data"} @skipif_no_gitpython -def test_gitpython_env_dynamic(gitpython_pillar_opts, grains): - _test_env_dynamic(gitpython_pillar_opts, grains) +def test_gitpython_pillarenv_dynamic(gitpython_pillar_opts, grains): + _test_pillarenv_dynamic(gitpython_pillar_opts, grains) @skipif_no_pygit2 -def test_pygit2_env_dynamic(pygit2_pillar_opts, grains): - _test_env_dynamic(pygit2_pillar_opts, grains) +def test_pygit2_pillarenv_dynamic(pygit2_pillar_opts, grains): + _test_pillarenv_dynamic(pygit2_pillar_opts, grains) -def _test_pillarenv_dynamic(pillar_opts, grains): +def _test_multiple(pillar_opts, grains): pillar_opts["pillarenv"] = "branch" data = _get_ext_pillar( "minion", pillar_opts, grains, "__env__ https://github.com/saltstack/salt-test-pillar-gitfs.git", + "other https://github.com/saltstack/salt-test-pillar-gitfs-2.git", ) assert data == {"key": "data"} @skipif_no_gitpython -def test_gitpython_pillarenv_dynamic(gitpython_pillar_opts, grains): - _test_pillarenv_dynamic(gitpython_pillar_opts, grains) +def test_gitpython_multiple(gitpython_pillar_opts, grains): + _test_multiple(gitpython_pillar_opts, grains) @skipif_no_pygit2 -def test_pygit2_pillarenv_dynamic(pygit2_pillar_opts, grains): - _test_pillarenv_dynamic(pygit2_pillar_opts, grains) +def test_pygit2_multiple(pygit2_pillar_opts, grains): + _test_multiple(pygit2_pillar_opts, grains) From e2d43b083f89a2a39ff5523a4526fc685ea88d8e Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Tue, 29 Aug 2023 13:12:57 -0500 Subject: [PATCH 19/31] m --- .../functional/pillar/test_git_pillar.py | 32 +++++++++- tests/pytests/functional/utils/test_gifts.py | 62 +++++++++++++++++++ 2 files changed, 91 insertions(+), 3 deletions(-) create mode 100644 tests/pytests/functional/utils/test_gifts.py diff --git a/tests/pytests/functional/pillar/test_git_pillar.py b/tests/pytests/functional/pillar/test_git_pillar.py index f7c59558634c..42e0e67b94d0 100644 --- a/tests/pytests/functional/pillar/test_git_pillar.py +++ b/tests/pytests/functional/pillar/test_git_pillar.py @@ -1,5 +1,3 @@ -import copy - import pytest from salt.pillar.git_pillar import ext_pillar @@ -32,7 +30,7 @@ @pytest.fixture def git_pillar_opts(salt_master, tmp_path): - opts = dict(copy.deepcopy(salt_master.config)) + opts = dict(salt_master.config) opts["cachedir"] = str(tmp_path) return opts @@ -228,3 +226,31 @@ def test_gitpython_multiple(gitpython_pillar_opts, grains): @skipif_no_pygit2 def test_pygit2_multiple(pygit2_pillar_opts, grains): _test_multiple(pygit2_pillar_opts, grains) + + +def _test_multiple_2(pillar_opts, grains): + data = _get_ext_pillar( + "minion", + pillar_opts, + grains, + "https://github.com/saltstack/salt-test-pillar-gitfs.git", + "https://github.com/saltstack/salt-test-pillar-gitfs-2.git", + ) + assert data == { + "key": "value", + "key1": "value1", + "key2": "value2", + "key4": "value4", + "data1": "d", + "data2": "d2", + } + + +@skipif_no_gitpython +def test_gitpython_multiple_2(gitpython_pillar_opts, grains): + _test_multiple_2(gitpython_pillar_opts, grains) + + +@skipif_no_pygit2 +def test_pygit2_multiple_2(pygit2_pillar_opts, grains): + _test_multiple_2(pygit2_pillar_opts, grains) diff --git a/tests/pytests/functional/utils/test_gifts.py b/tests/pytests/functional/utils/test_gifts.py new file mode 100644 index 000000000000..70c4ee4c6543 --- /dev/null +++ b/tests/pytests/functional/utils/test_gifts.py @@ -0,0 +1,62 @@ +import pytest + +from salt.utils.gitfs import GitFS + +pytestmark = [ + pytest.mark.slow_test, +] + + +try: + import git # pylint: disable=unused-import + + HAS_GITPYTHON = True +except ImportError: + HAS_GITPYTHON = False + + +try: + import pygit2 # pylint: disable=unused-import + + HAS_PYGIT2 = True +except ImportError: + HAS_PYGIT2 = False + + +skipif_no_gitpython = pytest.mark.skipif(not HAS_GITPYTHON, reason="Missing gitpython") +skipif_no_pygit2 = pytest.mark.skipif(not HAS_PYGIT2, reason="Missing pygit2") + + +@pytest.fixture +def gitfs_opts(salt_factories, tmp_path): + config_defaults = {"cachedir": str(tmp_path)} + factory = salt_factories.salt_master_daemon( + "gitfs-functional-master", defaults=config_defaults + ) + return dict(factory.config) + + +@pytest.fixture +def gitpython_gifts_opts(gitfs_opts): + gitfs_opts["verified_gifts_provider"] = "gitpython" + return gitfs_opts + + +@pytest.fixture +def pygit2_gifts_opts(gitfs_opts): + gitfs_opts["verified_gifts_provider"] = "pygit2" + return gitfs_opts + + +def _test_gitfs_simple(gitfs_opts): + g = GitFS(gitfs_opts, ["https://github.com/saltstack/salt-test-pillar-gitfs.git"]) + + +# @skipif_no_gitpython +# def test_gitpython_gitfs_simple(gitpython_gifts_opts): +# _test_gitfs_simple(gitpython_gifts_opts) + + +# @skipif_no_pygit2 +# def test_pygit2_gitfs_simple(pygit2_gifts_opts): +# _test_gitfs_simple(pygit2_gifts_opts) From da149d753b24e01c25faefa8803ca5a76aed78c8 Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Wed, 30 Aug 2023 16:48:38 -0500 Subject: [PATCH 20/31] fix gtfs ssl --- salt/utils/gitfs.py | 4 +++- tests/pytests/functional/utils/test_gifts.py | 12 ++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/salt/utils/gitfs.py b/salt/utils/gitfs.py index eb2676c75b9e..40f3be83df39 100644 --- a/salt/utils/gitfs.py +++ b/salt/utils/gitfs.py @@ -251,6 +251,7 @@ def __init__( key_cb=str, val_cb=lambda x, y: str(y), ) + self.ssl_verify = self.opts.get(f"{self.role}_ssl_verify", None) self.conf = copy.deepcopy(per_remote_defaults) # Remove the 'salt://' from the beginning of any globally-defined # per-saltenv mountpoints @@ -823,7 +824,7 @@ def enforce_git_config(self): desired_refspecs, ) if refspecs != desired_refspecs: - conf.set_multivar(remote_section, "fetch", self.refspecs) + conf.set_multivar(remote_section, "fetch", desired_refspecs) log.debug( "Refspecs for %s remote '%s' set to %s", self.role, @@ -856,6 +857,7 @@ def enforce_git_config(self): self.id, desired_ssl_verify, ) + self._ssl_verfiy = self.opts.get(f"{self.role}_ssl_verify", None) conf_changed = True # Write changes, if necessary diff --git a/tests/pytests/functional/utils/test_gifts.py b/tests/pytests/functional/utils/test_gifts.py index 70c4ee4c6543..60ab6e72ec60 100644 --- a/tests/pytests/functional/utils/test_gifts.py +++ b/tests/pytests/functional/utils/test_gifts.py @@ -52,11 +52,11 @@ def _test_gitfs_simple(gitfs_opts): g = GitFS(gitfs_opts, ["https://github.com/saltstack/salt-test-pillar-gitfs.git"]) -# @skipif_no_gitpython -# def test_gitpython_gitfs_simple(gitpython_gifts_opts): -# _test_gitfs_simple(gitpython_gifts_opts) +@skipif_no_gitpython +def test_gitpython_gitfs_simple(gitpython_gifts_opts): + _test_gitfs_simple(gitpython_gifts_opts) -# @skipif_no_pygit2 -# def test_pygit2_gitfs_simple(pygit2_gifts_opts): -# _test_gitfs_simple(pygit2_gifts_opts) +@skipif_no_pygit2 +def test_pygit2_gitfs_simple(pygit2_gifts_opts): + _test_gitfs_simple(pygit2_gifts_opts) From e9e88325303460d5beed62c7e6f9f40ddf656d7e Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Thu, 31 Aug 2023 10:50:29 -0500 Subject: [PATCH 21/31] fire up gitfs --- .../functional/pillar/test_git_pillar.py | 6 ++++++ tests/pytests/functional/utils/test_gifts.py | 20 +++++++++++++++++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/tests/pytests/functional/pillar/test_git_pillar.py b/tests/pytests/functional/pillar/test_git_pillar.py index 42e0e67b94d0..6fd3dee431b1 100644 --- a/tests/pytests/functional/pillar/test_git_pillar.py +++ b/tests/pytests/functional/pillar/test_git_pillar.py @@ -1,6 +1,7 @@ import pytest from salt.pillar.git_pillar import ext_pillar +from salt.utils.immutabletypes import ImmutableDict, ImmutableList from tests.support.mock import patch pytestmark = [ @@ -32,6 +33,11 @@ def git_pillar_opts(salt_master, tmp_path): opts = dict(salt_master.config) opts["cachedir"] = str(tmp_path) + for key, item in opts.items(): + if isinstance(item, ImmutableDict): + opts[key] = dict(item) + elif isinstance(item, ImmutableList): + opts[key] = list(item) return opts diff --git a/tests/pytests/functional/utils/test_gifts.py b/tests/pytests/functional/utils/test_gifts.py index 60ab6e72ec60..d52eea62fb62 100644 --- a/tests/pytests/functional/utils/test_gifts.py +++ b/tests/pytests/functional/utils/test_gifts.py @@ -1,6 +1,8 @@ import pytest +from salt.fileserver.gitfs import PER_REMOTE_ONLY, PER_REMOTE_OVERRIDES from salt.utils.gitfs import GitFS +from salt.utils.immutabletypes import ImmutableDict, ImmutableList pytestmark = [ pytest.mark.slow_test, @@ -33,7 +35,13 @@ def gitfs_opts(salt_factories, tmp_path): factory = salt_factories.salt_master_daemon( "gitfs-functional-master", defaults=config_defaults ) - return dict(factory.config) + config_defaults = dict(factory.config) + for key, item in config_defaults.items(): + if isinstance(item, ImmutableDict): + config_defaults[key] = dict(item) + elif isinstance(item, ImmutableList): + config_defaults[key] = list(item) + return config_defaults @pytest.fixture @@ -49,7 +57,15 @@ def pygit2_gifts_opts(gitfs_opts): def _test_gitfs_simple(gitfs_opts): - g = GitFS(gitfs_opts, ["https://github.com/saltstack/salt-test-pillar-gitfs.git"]) + g = GitFS( + gitfs_opts, + ["https://github.com/saltstack/salt-test-pillar-gitfs.git"], + per_remote_overrides=PER_REMOTE_OVERRIDES, + per_remote_only=PER_REMOTE_ONLY, + ) + g.fetch_remotes() + assert len(g.remotes) == 1 + assert g.file_list({"saltenv": "main"}) == [".gitignore", "README.md"] @skipif_no_gitpython From 6d76addca32f2aee4a75a23d5c881a3342c91244 Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Thu, 31 Aug 2023 22:59:36 -0500 Subject: [PATCH 22/31] test __env__ cache --- tests/pytests/functional/utils/test_gifts.py | 75 +++++++++-- tests/pytests/functional/utils/test_pillar.py | 126 ++++++++++++++++++ 2 files changed, 191 insertions(+), 10 deletions(-) create mode 100644 tests/pytests/functional/utils/test_pillar.py diff --git a/tests/pytests/functional/utils/test_gifts.py b/tests/pytests/functional/utils/test_gifts.py index d52eea62fb62..0ba5333b2ca6 100644 --- a/tests/pytests/functional/utils/test_gifts.py +++ b/tests/pytests/functional/utils/test_gifts.py @@ -1,7 +1,7 @@ import pytest from salt.fileserver.gitfs import PER_REMOTE_ONLY, PER_REMOTE_OVERRIDES -from salt.utils.gitfs import GitFS +from salt.utils.gitfs import GitFS, GitPython, Pygit2 from salt.utils.immutabletypes import ImmutableDict, ImmutableList pytestmark = [ @@ -45,14 +45,16 @@ def gitfs_opts(salt_factories, tmp_path): @pytest.fixture -def gitpython_gifts_opts(gitfs_opts): - gitfs_opts["verified_gifts_provider"] = "gitpython" +def gitpython_gitfs_opts(gitfs_opts): + gitfs_opts["verified_gitfs_provider"] = "gitpython" + GitFS.instance_map.clear() return gitfs_opts @pytest.fixture -def pygit2_gifts_opts(gitfs_opts): - gitfs_opts["verified_gifts_provider"] = "pygit2" +def pygit2_gitfs_opts(gitfs_opts): + gitfs_opts["verified_gitfs_provider"] = "pygit2" + GitFS.instance_map.clear() return gitfs_opts @@ -65,14 +67,67 @@ def _test_gitfs_simple(gitfs_opts): ) g.fetch_remotes() assert len(g.remotes) == 1 - assert g.file_list({"saltenv": "main"}) == [".gitignore", "README.md"] + assert set(g.file_list({"saltenv": "main"})) == {".gitignore", "README.md"} @skipif_no_gitpython -def test_gitpython_gitfs_simple(gitpython_gifts_opts): - _test_gitfs_simple(gitpython_gifts_opts) +def test_gitpython_gitfs_simple(gitpython_gitfs_opts): + _test_gitfs_simple(gitpython_gitfs_opts) @skipif_no_pygit2 -def test_pygit2_gitfs_simple(pygit2_gifts_opts): - _test_gitfs_simple(pygit2_gifts_opts) +def test_pygit2_gitfs_simple(pygit2_gitfs_opts): + _test_gitfs_simple(pygit2_gitfs_opts) + + +def _test_gitfs_simple_base(gitfs_opts): + g = GitFS( + gitfs_opts, + ["https://github.com/saltstack/salt-test-pillar-gitfs.git"], + per_remote_overrides=PER_REMOTE_OVERRIDES, + per_remote_only=PER_REMOTE_ONLY, + ) + g.fetch_remotes() + assert len(g.remotes) == 1 + assert set(g.file_list({"saltenv": "base"})) == { + ".gitignore", + "README.md", + "file.sls", + "top.sls", + } + + +@skipif_no_gitpython +def test_gitpython_gitfs_simple_base(gitpython_gitfs_opts): + _test_gitfs_simple_base(gitpython_gitfs_opts) + + +@skipif_no_pygit2 +def test_pygit2_gitfs_simple_base(pygit2_gitfs_opts): + _test_gitfs_simple_base(pygit2_gitfs_opts) + + +@skipif_no_gitpython +def test_gitpython_gitfs_provider(gitpython_gitfs_opts): + g = GitFS( + gitpython_gitfs_opts, + ["https://github.com/saltstack/salt-test-pillar-gitfs.git"], + per_remote_overrides=PER_REMOTE_OVERRIDES, + per_remote_only=PER_REMOTE_ONLY, + ) + assert len(g.remotes) == 1 + assert g.provider == "gitpython" + assert isinstance(g.remotes[0], GitPython) + + +@skipif_no_pygit2 +def test_pygit2_gitfs_provider(pygit2_gitfs_opts): + g = GitFS( + pygit2_gitfs_opts, + ["https://github.com/saltstack/salt-test-pillar-gitfs.git"], + per_remote_overrides=PER_REMOTE_OVERRIDES, + per_remote_only=PER_REMOTE_ONLY, + ) + assert len(g.remotes) == 1 + assert g.provider == "pygit2" + assert isinstance(g.remotes[0], Pygit2) diff --git a/tests/pytests/functional/utils/test_pillar.py b/tests/pytests/functional/utils/test_pillar.py new file mode 100644 index 000000000000..3ad1c071267a --- /dev/null +++ b/tests/pytests/functional/utils/test_pillar.py @@ -0,0 +1,126 @@ +import os + +import pytest + +from salt.pillar.git_pillar import GLOBAL_ONLY, PER_REMOTE_ONLY, PER_REMOTE_OVERRIDES +from salt.utils.gitfs import GitPillar, GitPython, Pygit2 +from salt.utils.immutabletypes import ImmutableDict, ImmutableList + +pytestmark = [ + pytest.mark.slow_test, +] + + +try: + import git # pylint: disable=unused-import + + HAS_GITPYTHON = True +except ImportError: + HAS_GITPYTHON = False + + +try: + import pygit2 # pylint: disable=unused-import + + HAS_PYGIT2 = True +except ImportError: + HAS_PYGIT2 = False + + +skipif_no_gitpython = pytest.mark.skipif(not HAS_GITPYTHON, reason="Missing gitpython") +skipif_no_pygit2 = pytest.mark.skipif(not HAS_PYGIT2, reason="Missing pygit2") + + +@pytest.fixture +def pillar_opts(salt_factories, tmp_path): + config_defaults = {"cachedir": str(tmp_path)} + factory = salt_factories.salt_master_daemon( + "pillar-functional-master", defaults=config_defaults + ) + config_defaults = dict(factory.config) + for key, item in config_defaults.items(): + if isinstance(item, ImmutableDict): + config_defaults[key] = dict(item) + elif isinstance(item, ImmutableList): + config_defaults[key] = list(item) + return config_defaults + + +@pytest.fixture +def gitpython_pillar_opts(pillar_opts): + pillar_opts["verified_git_pillar_provider"] = "gitpython" + return pillar_opts + + +@pytest.fixture +def pygit2_pillar_opts(pillar_opts): + pillar_opts["verified_git_pillar_provider"] = "pygit2" + return pillar_opts + + +def _get_pillar(opts, *remotes): + return GitPillar( + opts, + remotes, + per_remote_overrides=PER_REMOTE_OVERRIDES, + per_remote_only=PER_REMOTE_ONLY, + global_only=GLOBAL_ONLY, + ) + + +@skipif_no_gitpython +def test_gitpython_pillar_provider(gitpython_pillar_opts): + p = _get_pillar( + gitpython_pillar_opts, "https://github.com/saltstack/salt-test-pillar-gitfs.git" + ) + assert len(p.remotes) == 1 + assert p.provider == "gitpython" + assert isinstance(p.remotes[0], GitPython) + + +@skipif_no_pygit2 +def test_pygit2_pillar_provider(pygit2_pillar_opts): + p = _get_pillar( + pygit2_pillar_opts, "https://github.com/saltstack/salt-test-pillar-gitfs.git" + ) + assert len(p.remotes) == 1 + assert p.provider == "pygit2" + assert isinstance(p.remotes[0], Pygit2) + + +def _test_env(opts): + p = _get_pillar( + opts, "__env__ https://github.com/saltstack/salt-test-pillar-gitfs.git" + ) + assert len(p.remotes) == 1 + p.checkout() + repo = p.remotes[0] + files = set(os.listdir(repo.get_cachedir())) + for f in (".gitignore", "README.md", "file.sls", "top.sls"): + assert f in files + opts["pillarenv"] = "main" + p2 = _get_pillar( + opts, "__env__ https://github.com/saltstack/salt-test-pillar-gitfs.git" + ) + assert len(p.remotes) == 1 + p2.checkout() + repo2 = p2.remotes[0] + files = set(os.listdir(repo2.get_cachedir())) + for f in (".gitignore", "README.md"): + assert f in files + for f in ("file.sls", "top.sls", "back.sls", "rooms.sls"): + assert f not in files + assert repo.get_cachedir() != repo2.get_cachedir() + files = set(os.listdir(repo.get_cachedir())) + for f in (".gitignore", "README.md", "file.sls", "top.sls"): + assert f in files + + +@skipif_no_gitpython +def test_gitpython_env(gitpython_pillar_opts): + _test_env(gitpython_pillar_opts) + + +@skipif_no_pygit2 +def test_pygit2_env(pygit2_pillar_opts): + _test_env(pygit2_pillar_opts) From d45fcac799f2128a3a95f0424a12a9aed88e7ced Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Thu, 31 Aug 2023 23:01:23 -0500 Subject: [PATCH 23/31] fix file name typo --- tests/pytests/functional/utils/{test_gifts.py => test_gitfs.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/pytests/functional/utils/{test_gifts.py => test_gitfs.py} (100%) diff --git a/tests/pytests/functional/utils/test_gifts.py b/tests/pytests/functional/utils/test_gitfs.py similarity index 100% rename from tests/pytests/functional/utils/test_gifts.py rename to tests/pytests/functional/utils/test_gitfs.py From e68115f3eec654634577b769c072138c920391c4 Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Fri, 1 Sep 2023 00:04:24 -0500 Subject: [PATCH 24/31] fetch_on_fail --- salt/utils/gitfs.py | 21 +++++++--- tests/pytests/functional/utils/test_cache.py | 16 +++++++- tests/pytests/functional/utils/test_gitfs.py | 39 +++++++++---------- tests/pytests/functional/utils/test_pillar.py | 34 ++++++++++++++++ 4 files changed, 82 insertions(+), 28 deletions(-) diff --git a/salt/utils/gitfs.py b/salt/utils/gitfs.py index 40f3be83df39..f7577a5640df 100644 --- a/salt/utils/gitfs.py +++ b/salt/utils/gitfs.py @@ -2931,15 +2931,18 @@ def write_remote_map(self): else: log.info("Wrote new %s remote map to %s", self.role, remote_map) - def do_checkout(self, repo): + def do_checkout(self, repo, fetch_on_fail=True): """ Common code for git_pillar/winrepo to handle locking and checking out of a repo. + + fetch_on_fail + If checkout fails perform a fetch then try to checkout again. """ time_start = time.time() while time.time() - time_start <= 5: try: - return repo.checkout() + return repo.checkout(fetch_on_fail=fetch_on_fail) except GitLockError as exc: if exc.errno == errno.EEXIST: time.sleep(0.1) @@ -3334,14 +3337,17 @@ class GitPillar(GitBase): role = "git_pillar" - def checkout(self): + def checkout(self, fetch_on_fail=True): """ Checkout the targeted branches/tags from the git_pillar remotes + + fetch_on_fail + If checkout fails perform a fetch then try to checkout again. """ self.pillar_dirs = OrderedDict() self.pillar_linked_dirs = [] for repo in self.remotes: - cachedir = self.do_checkout(repo) + cachedir = self.do_checkout(repo, fetch_on_fail=fetch_on_fail) if cachedir is not None: # Figure out which environment this remote should be assigned if repo.branch == "__env__" and hasattr(repo, "all_saltenvs"): @@ -3493,6 +3499,9 @@ def link_mountpoint(self, repo): class WinRepo(GitBase): """ Functionality specific to the winrepo runner + + fetch_on_fail + If checkout fails perform a fetch then try to checkout again. """ role = "winrepo" @@ -3500,12 +3509,12 @@ class WinRepo(GitBase): # out the repos. winrepo_dirs = {} - def checkout(self): + def checkout(self, fetch_on_fail=True): """ Checkout the targeted branches/tags from the winrepo remotes """ self.winrepo_dirs = {} for repo in self.remotes: - cachedir = self.do_checkout(repo) + cachedir = self.do_checkout(repo, fetch_on_fail=fetch_on_fail) if cachedir is not None: self.winrepo_dirs[repo.id] = cachedir diff --git a/tests/pytests/functional/utils/test_cache.py b/tests/pytests/functional/utils/test_cache.py index 98c6946b653b..d405b8246fc0 100644 --- a/tests/pytests/functional/utils/test_cache.py +++ b/tests/pytests/functional/utils/test_cache.py @@ -1,8 +1,11 @@ import os +import pytest + import salt.utils.cache import salt.utils.files import salt.utils.path +import salt.version _DUMMY_FILES = ( "data.txt", @@ -41,8 +44,15 @@ def _dummy_files_exists(tmp_path): return ret +def test_verify_cache_version_bad_path(): + with pytest.raises(ValueError): + # cache version should fail if given bad file python + salt.utils.cache.verify_cache_version("\0/bad/path") + + def test_verify_cache_version(tmp_path): - tmp_path = str(tmp_path) + # cache version should make dir if it does not exist + tmp_path = str(salt.utils.path.join(str(tmp_path), "work", "salt")) cache_version = salt.utils.path.join(tmp_path, "cache_version") # check that cache clears when no cache_version is present @@ -50,6 +60,10 @@ def test_verify_cache_version(tmp_path): assert salt.utils.cache.verify_cache_version(tmp_path) is False assert _dummy_files_exists(tmp_path) is False + # check that cache_version has correct salt version + with salt.utils.files.fopen(cache_version, "r") as file: + assert "\n".join(file.readlines()) == salt.version.__version__ + # check that cache does not get clear when check is called multiple times _make_dummy_files(tmp_path) for _ in range(3): diff --git a/tests/pytests/functional/utils/test_gitfs.py b/tests/pytests/functional/utils/test_gitfs.py index 0ba5333b2ca6..51be700ec7b3 100644 --- a/tests/pytests/functional/utils/test_gitfs.py +++ b/tests/pytests/functional/utils/test_gitfs.py @@ -47,24 +47,30 @@ def gitfs_opts(salt_factories, tmp_path): @pytest.fixture def gitpython_gitfs_opts(gitfs_opts): gitfs_opts["verified_gitfs_provider"] = "gitpython" - GitFS.instance_map.clear() + GitFS.instance_map.clear() # wipe instance_map object map for clean run return gitfs_opts @pytest.fixture def pygit2_gitfs_opts(gitfs_opts): gitfs_opts["verified_gitfs_provider"] = "pygit2" - GitFS.instance_map.clear() + GitFS.instance_map.clear() # wipe instance_map object map for clean run return gitfs_opts -def _test_gitfs_simple(gitfs_opts): - g = GitFS( - gitfs_opts, - ["https://github.com/saltstack/salt-test-pillar-gitfs.git"], +def _get_gitfs(opts, *remotes): + return GitFS( + opts, + remotes, per_remote_overrides=PER_REMOTE_OVERRIDES, per_remote_only=PER_REMOTE_ONLY, ) + + +def _test_gitfs_simple(gitfs_opts): + g = _get_gitfs( + gitfs_opts, "https://github.com/saltstack/salt-test-pillar-gitfs.git" + ) g.fetch_remotes() assert len(g.remotes) == 1 assert set(g.file_list({"saltenv": "main"})) == {".gitignore", "README.md"} @@ -81,11 +87,8 @@ def test_pygit2_gitfs_simple(pygit2_gitfs_opts): def _test_gitfs_simple_base(gitfs_opts): - g = GitFS( - gitfs_opts, - ["https://github.com/saltstack/salt-test-pillar-gitfs.git"], - per_remote_overrides=PER_REMOTE_OVERRIDES, - per_remote_only=PER_REMOTE_ONLY, + g = _get_gitfs( + gitfs_opts, "https://github.com/saltstack/salt-test-pillar-gitfs.git" ) g.fetch_remotes() assert len(g.remotes) == 1 @@ -109,11 +112,8 @@ def test_pygit2_gitfs_simple_base(pygit2_gitfs_opts): @skipif_no_gitpython def test_gitpython_gitfs_provider(gitpython_gitfs_opts): - g = GitFS( - gitpython_gitfs_opts, - ["https://github.com/saltstack/salt-test-pillar-gitfs.git"], - per_remote_overrides=PER_REMOTE_OVERRIDES, - per_remote_only=PER_REMOTE_ONLY, + g = _get_gitfs( + gitpython_gitfs_opts, "https://github.com/saltstack/salt-test-pillar-gitfs.git" ) assert len(g.remotes) == 1 assert g.provider == "gitpython" @@ -122,11 +122,8 @@ def test_gitpython_gitfs_provider(gitpython_gitfs_opts): @skipif_no_pygit2 def test_pygit2_gitfs_provider(pygit2_gitfs_opts): - g = GitFS( - pygit2_gitfs_opts, - ["https://github.com/saltstack/salt-test-pillar-gitfs.git"], - per_remote_overrides=PER_REMOTE_OVERRIDES, - per_remote_only=PER_REMOTE_ONLY, + g = _get_gitfs( + pygit2_gitfs_opts, "https://github.com/saltstack/salt-test-pillar-gitfs.git" ) assert len(g.remotes) == 1 assert g.provider == "pygit2" diff --git a/tests/pytests/functional/utils/test_pillar.py b/tests/pytests/functional/utils/test_pillar.py index 3ad1c071267a..3b225e8fbd6d 100644 --- a/tests/pytests/functional/utils/test_pillar.py +++ b/tests/pytests/functional/utils/test_pillar.py @@ -95,6 +95,7 @@ def _test_env(opts): assert len(p.remotes) == 1 p.checkout() repo = p.remotes[0] + # test that two different pillarenvs can exist at the same time files = set(os.listdir(repo.get_cachedir())) for f in (".gitignore", "README.md", "file.sls", "top.sls"): assert f in files @@ -115,6 +116,24 @@ def _test_env(opts): for f in (".gitignore", "README.md", "file.sls", "top.sls"): assert f in files + # double check cache paths + assert ( + repo.get_cache_hash() == repo2.get_cache_hash() + ) # __env__ repos share same hash + assert repo.get_cache_basename() != repo2.get_cache_basename() + assert repo.get_linkdir() != repo2.get_linkdir() + assert repo.get_salt_working_dir() != repo2.get_salt_working_dir() + assert repo.get_cache_basename() == "master" + assert repo2.get_cache_basename() == "main" + + assert repo.get_cache_basename() in repo.get_cachedir() + assert ( + os.path.join(repo.get_cache_basehash(), repo.get_cache_basename()) + == repo.get_cache_full_basename() + ) + assert repo.get_linkdir() not in repo.get_cachedir() + assert repo.get_salt_working_dir() not in repo.get_cachedir() + @skipif_no_gitpython def test_gitpython_env(gitpython_pillar_opts): @@ -124,3 +143,18 @@ def test_gitpython_env(gitpython_pillar_opts): @skipif_no_pygit2 def test_pygit2_env(pygit2_pillar_opts): _test_env(pygit2_pillar_opts) + + +def _test_checkout_fetch_on_fail(opts): + p = _get_pillar(opts, "https://github.com/saltstack/salt-test-pillar-gitfs.git") + p.checkout(fetch_on_fail=False) # TODO write me + + +@skipif_no_gitpython +def test_gitpython_checkout_fetch_on_fail(gitpython_pillar_opts): + _test_checkout_fetch_on_fail(gitpython_pillar_opts) + + +@skipif_no_pygit2 +def test_pygit2_checkout_fetch_on_fail(pygit2_pillar_opts): + _test_checkout_fetch_on_fail(pygit2_pillar_opts) From f33ff212959ceabe259ebf6ca962d99b75f10c71 Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Fri, 1 Sep 2023 11:27:20 -0500 Subject: [PATCH 25/31] winrepo tests --- tests/pytests/functional/utils/test_gitfs.py | 26 +++++ .../pytests/functional/utils/test_winrepo.py | 109 ++++++++++++++++++ 2 files changed, 135 insertions(+) create mode 100644 tests/pytests/functional/utils/test_winrepo.py diff --git a/tests/pytests/functional/utils/test_gitfs.py b/tests/pytests/functional/utils/test_gitfs.py index 51be700ec7b3..1ffcaf083882 100644 --- a/tests/pytests/functional/utils/test_gitfs.py +++ b/tests/pytests/functional/utils/test_gitfs.py @@ -128,3 +128,29 @@ def test_pygit2_gitfs_provider(pygit2_gitfs_opts): assert len(g.remotes) == 1 assert g.provider == "pygit2" assert isinstance(g.remotes[0], Pygit2) + + +def _test_gitfs_minion(gitfs_opts): + gitfs_opts["__role"] = "minion" + g = _get_gitfs( + gitfs_opts, "https://github.com/saltstack/salt-test-pillar-gitfs.git" + ) + g.fetch_remotes() + assert len(g.remotes) == 1 + assert set(g.file_list({"saltenv": "base"})) == { + ".gitignore", + "README.md", + "file.sls", + "top.sls", + } + assert set(g.file_list({"saltenv": "main"})) == {".gitignore", "README.md"} + + +@skipif_no_gitpython +def test_gitpython_gitfs_minion(gitpython_gitfs_opts): + _test_gitfs_minion(gitpython_gitfs_opts) + + +@skipif_no_pygit2 +def test_pygit2_gitfs_minion(pygit2_gitfs_opts): + _test_gitfs_minion(pygit2_gitfs_opts) diff --git a/tests/pytests/functional/utils/test_winrepo.py b/tests/pytests/functional/utils/test_winrepo.py new file mode 100644 index 000000000000..1ec730cbb9d5 --- /dev/null +++ b/tests/pytests/functional/utils/test_winrepo.py @@ -0,0 +1,109 @@ +import os + +import pytest + +from salt.runners.winrepo import GLOBAL_ONLY, PER_REMOTE_ONLY, PER_REMOTE_OVERRIDES +from salt.utils.gitfs import GitPython, Pygit2, WinRepo +from salt.utils.immutabletypes import ImmutableDict, ImmutableList + +pytestmark = [ + pytest.mark.slow_test, +] + + +try: + import git # pylint: disable=unused-import + + HAS_GITPYTHON = True +except ImportError: + HAS_GITPYTHON = False + + +try: + import pygit2 # pylint: disable=unused-import + + HAS_PYGIT2 = True +except ImportError: + HAS_PYGIT2 = False + + +skipif_no_gitpython = pytest.mark.skipif(not HAS_GITPYTHON, reason="Missing gitpython") +skipif_no_pygit2 = pytest.mark.skipif(not HAS_PYGIT2, reason="Missing pygit2") + + +@pytest.fixture +def winrepo_opts(salt_factories, tmp_path): + config_defaults = {"cachedir": str(tmp_path)} + factory = salt_factories.salt_master_daemon( + "winrepo-functional-master", defaults=config_defaults + ) + config_defaults = dict(factory.config) + for key, item in config_defaults.items(): + if isinstance(item, ImmutableDict): + config_defaults[key] = dict(item) + elif isinstance(item, ImmutableList): + config_defaults[key] = list(item) + return config_defaults + + +@pytest.fixture +def gitpython_winrepo_opts(winrepo_opts): + winrepo_opts["verified_winrepo_provider"] = "gitpython" + return winrepo_opts + + +@pytest.fixture +def pygit2_winrepo_opts(winrepo_opts): + winrepo_opts["verified_winrepo_provider"] = "pygit2" + return winrepo_opts + + +def _get_winrepo(opts, *remotes): + return WinRepo( + opts, + remotes, + per_remote_overrides=PER_REMOTE_OVERRIDES, + per_remote_only=PER_REMOTE_ONLY, + global_only=GLOBAL_ONLY, + ) + + +@skipif_no_gitpython +def test_gitpython_winrepo_provider(gitpython_winrepo_opts): + w = _get_winrepo( + gitpython_winrepo_opts, + "https://github.com/saltstack/salt-test-pillar-gitfs.git", + ) + assert len(w.remotes) == 1 + assert w.provider == "gitpython" + assert isinstance(w.remotes[0], GitPython) + + +@skipif_no_pygit2 +def test_pygit2_winrepo_provider(pygit2_winrepo_opts): + w = _get_winrepo( + pygit2_winrepo_opts, "https://github.com/saltstack/salt-test-pillar-gitfs.git" + ) + assert len(w.remotes) == 1 + assert w.provider == "pygit2" + assert isinstance(w.remotes[0], Pygit2) + + +def _test_winrepo_simple(opts): + w = _get_winrepo(opts, "https://github.com/saltstack/salt-test-pillar-gitfs.git") + assert len(w.remotes) == 1 + w.checkout() + repo = w.remotes[0] + files = set(os.listdir(repo.get_cachedir())) + for f in (".gitignore", "README.md", "file.sls", "top.sls"): + assert f in files + + +@skipif_no_gitpython +def test_gitpython_winrepo_simple(gitpython_winrepo_opts): + _test_winrepo_simple(gitpython_winrepo_opts) + + +@skipif_no_pygit2 +def test_pygit2_winrepo_simple(pygit2_winrepo_opts): + _test_winrepo_simple(pygit2_winrepo_opts) From 3271614b4be074615a21649ab7ebb08f91848a07 Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Fri, 1 Sep 2023 13:23:44 -0500 Subject: [PATCH 26/31] multi tests --- tests/pytests/functional/utils/test_pillar.py | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/tests/pytests/functional/utils/test_pillar.py b/tests/pytests/functional/utils/test_pillar.py index 3b225e8fbd6d..d1a26bb042d4 100644 --- a/tests/pytests/functional/utils/test_pillar.py +++ b/tests/pytests/functional/utils/test_pillar.py @@ -158,3 +158,65 @@ def test_gitpython_checkout_fetch_on_fail(gitpython_pillar_opts): @skipif_no_pygit2 def test_pygit2_checkout_fetch_on_fail(pygit2_pillar_opts): _test_checkout_fetch_on_fail(pygit2_pillar_opts) + + +def _test_multiple_repos(opts): + p = _get_pillar( + opts, + "__env__ https://github.com/saltstack/salt-test-pillar-gitfs.git", + "main https://github.com/saltstack/salt-test-pillar-gitfs.git", + "branch https://github.com/saltstack/salt-test-pillar-gitfs.git", + "__env__ https://github.com/saltstack/salt-test-pillar-gitfs-2.git", + "other https://github.com/saltstack/salt-test-pillar-gitfs-2.git", + ) + p.checkout() + assert len(p.remotes) == 5 + # make sure all repos dont share cache and working dir + assert len({r.get_cachedir() for r in p.remotes}) == 5 + assert len({r.get_salt_working_dir() for r in p.remotes}) == 5 + + p2 = _get_pillar( + opts, + "__env__ https://github.com/saltstack/salt-test-pillar-gitfs.git", + "main https://github.com/saltstack/salt-test-pillar-gitfs.git", + "branch https://github.com/saltstack/salt-test-pillar-gitfs.git", + "__env__ https://github.com/saltstack/salt-test-pillar-gitfs-2.git", + "other https://github.com/saltstack/salt-test-pillar-gitfs-2.git", + ) + p2.checkout() + assert len(p2.remotes) == 5 + # make sure that repos are given same cache dir + for repo, repo2 in zip(p.remotes, p2.remotes): + assert repo.get_cachedir() == repo2.get_cachedir() + assert repo.get_salt_working_dir() == repo2.get_salt_working_dir() + opts["pillarenv"] = "main" + p3 = _get_pillar( + opts, + "__env__ https://github.com/saltstack/salt-test-pillar-gitfs.git", + "main https://github.com/saltstack/salt-test-pillar-gitfs.git", + "branch https://github.com/saltstack/salt-test-pillar-gitfs.git", + "__env__ https://github.com/saltstack/salt-test-pillar-gitfs-2.git", + "other https://github.com/saltstack/salt-test-pillar-gitfs-2.git", + ) + p3.checkout() + # check that __env__ has different cache with different pillarenv + assert p.remotes[0].get_cachedir() != p3.remotes[0].get_cachedir() + assert p.remotes[1].get_cachedir() == p3.remotes[1].get_cachedir() + assert p.remotes[2].get_cachedir() == p3.remotes[2].get_cachedir() + assert p.remotes[3].get_cachedir() != p3.remotes[3].get_cachedir() + assert p.remotes[4].get_cachedir() == p3.remotes[4].get_cachedir() + + # check that other branch data is in cache + files = set(os.listdir(p.remotes[4].get_cachedir())) + for f in (".gitignore", "README.md", "file.sls", "top.sls", "other_env.sls"): + assert f in files + + +@skipif_no_gitpython +def test_gitpython_multiple_repos(gitpython_pillar_opts): + _test_multiple_repos(gitpython_pillar_opts) + + +@skipif_no_pygit2 +def test_pygit2_multiple_repos(pygit2_pillar_opts): + _test_multiple_repos(pygit2_pillar_opts) From 71d440b46b6b0ab88971f724883f18e3102da13b Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Fri, 1 Sep 2023 14:30:30 -0500 Subject: [PATCH 27/31] fetch_request & clear_old_remote tests --- tests/pytests/functional/utils/test_pillar.py | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/tests/pytests/functional/utils/test_pillar.py b/tests/pytests/functional/utils/test_pillar.py index d1a26bb042d4..896acf751a26 100644 --- a/tests/pytests/functional/utils/test_pillar.py +++ b/tests/pytests/functional/utils/test_pillar.py @@ -220,3 +220,89 @@ def test_gitpython_multiple_repos(gitpython_pillar_opts): @skipif_no_pygit2 def test_pygit2_multiple_repos(pygit2_pillar_opts): _test_multiple_repos(pygit2_pillar_opts) + + +def _test_fetch_request(opts): + p = _get_pillar( + opts, + "__env__ https://github.com/saltstack/salt-test-pillar-gitfs.git", + "other https://github.com/saltstack/salt-test-pillar-gitfs-2.git", + ) + frequest = os.path.join(p.remotes[0].get_salt_working_dir(), "fetch_request") + frequest_other = os.path.join(p.remotes[1].get_salt_working_dir(), "fetch_request") + opts["pillarenv"] = "main" + p2 = _get_pillar( + opts, "__env__ https://github.com/saltstack/salt-test-pillar-gitfs.git" + ) + frequest2 = os.path.join(p2.remotes[0].get_salt_working_dir(), "fetch_request") + assert frequest != frequest2 + assert os.path.isfile(frequest) is False + assert os.path.isfile(frequest2) is False + assert os.path.isfile(frequest_other) is False + p.fetch_remotes() + assert os.path.isfile(frequest) is False + # fetch request was placed + assert os.path.isfile(frequest2) is True + p2.checkout() + # fetch request was found + assert os.path.isfile(frequest2) is False + p2.fetch_remotes() + assert os.path.isfile(frequest) is True + assert os.path.isfile(frequest2) is False + assert os.path.isfile(frequest_other) is False + for _ in range(3): + p2.fetch_remotes() + assert os.path.isfile(frequest) is True + assert os.path.isfile(frequest2) is False + assert os.path.isfile(frequest_other) is False + # fetch request should still be processed even on fetch_on_fail=False + p.checkout(fetch_on_fail=False) + assert os.path.isfile(frequest) is False + assert os.path.isfile(frequest2) is False + assert os.path.isfile(frequest_other) is False + + +@skipif_no_gitpython +def test_gitpython_fetch_request(gitpython_pillar_opts): + _test_fetch_request(gitpython_pillar_opts) + + +@skipif_no_pygit2 +def test_pygit2_fetch_request(pygit2_pillar_opts): + _test_fetch_request(pygit2_pillar_opts) + + +def _test_clear_old_remotes(opts): + p = _get_pillar( + opts, + "__env__ https://github.com/saltstack/salt-test-pillar-gitfs.git", + "other https://github.com/saltstack/salt-test-pillar-gitfs-2.git", + ) + repo = p.remotes[0] + repo2 = p.remotes[1] + opts["pillarenv"] = "main" + p2 = _get_pillar( + opts, "__env__ https://github.com/saltstack/salt-test-pillar-gitfs.git" + ) + repo3 = p2.remotes[0] + assert os.path.isdir(repo.get_cachedir()) is True + assert os.path.isdir(repo2.get_cachedir()) is True + assert os.path.isdir(repo3.get_cachedir()) is True + p.clear_old_remotes() + assert os.path.isdir(repo.get_cachedir()) is True + assert os.path.isdir(repo2.get_cachedir()) is True + assert os.path.isdir(repo3.get_cachedir()) is True + p2.clear_old_remotes() + assert os.path.isdir(repo.get_cachedir()) is True + assert os.path.isdir(repo2.get_cachedir()) is False + assert os.path.isdir(repo3.get_cachedir()) is True + + +@skipif_no_gitpython +def test_gitpython_clear_old_remotes(gitpython_pillar_opts): + _test_clear_old_remotes(gitpython_pillar_opts) + + +@skipif_no_pygit2 +def test_pygit2_clear_old_remotes(pygit2_pillar_opts): + _test_clear_old_remotes(pygit2_pillar_opts) From 36a78d2c43fd03242d330b59507fd95524174753 Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Mon, 4 Sep 2023 16:25:23 -0500 Subject: [PATCH 28/31] revert ssl --- salt/utils/gitfs.py | 1 - 1 file changed, 1 deletion(-) diff --git a/salt/utils/gitfs.py b/salt/utils/gitfs.py index f7577a5640df..da25add4fad3 100644 --- a/salt/utils/gitfs.py +++ b/salt/utils/gitfs.py @@ -251,7 +251,6 @@ def __init__( key_cb=str, val_cb=lambda x, y: str(y), ) - self.ssl_verify = self.opts.get(f"{self.role}_ssl_verify", None) self.conf = copy.deepcopy(per_remote_defaults) # Remove the 'salt://' from the beginning of any globally-defined # per-saltenv mountpoints From efaa2cd0bf0861cabcba606d8268b3611c5c692f Mon Sep 17 00:00:00 2001 From: Charles McMarrow Date: Mon, 4 Sep 2023 19:48:30 -0500 Subject: [PATCH 29/31] Add Murphy tests --- tests/pytests/functional/utils/test_pillar.py | 121 ++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/tests/pytests/functional/utils/test_pillar.py b/tests/pytests/functional/utils/test_pillar.py index 896acf751a26..9091bc1c2bc6 100644 --- a/tests/pytests/functional/utils/test_pillar.py +++ b/tests/pytests/functional/utils/test_pillar.py @@ -306,3 +306,124 @@ def test_gitpython_clear_old_remotes(gitpython_pillar_opts): @skipif_no_pygit2 def test_pygit2_clear_old_remotes(pygit2_pillar_opts): _test_clear_old_remotes(pygit2_pillar_opts) + +@skipif_no_pygit2 +def test_pygit2_fetch_request_with_mountpoint(pygit2_pillar_opts): + opts = pygit2_pillar_opts + mpoint = [{"mountpoint": "test_dir1/test_dir2"}] + p = _get_pillar( + opts, + {"__env__ https://github.com/saltstack/salt-test-pillar-gitfs.git": mpoint}, + {"testmount https://github.com/saltstack/salt-test-pillar-gitfs.git": mpoint}, + ) + log.warning(f"DGM test_pygit2_fetch_request_with_mountpoint p '{p}', p.remote[0] '{p.remotes[0]}', working_dir op '{p.remotes[0].get_salt_working_dir()}'") + frequest = os.path.join(p.remotes[0].get_salt_working_dir(), "fetch_request") + frequest_testmount = os.path.join(p.remotes[1].get_salt_working_dir(), "fetch_request") + log.warning(f"DGM test_pygit2_fetch_request_with_mountpoint frequest '{frequest}'") + log.warning(f"DGM test_pygit2_fetch_request_with_mountpoint frequest_testmount '{frequest_testmount}'") + opts["pillarenv"] = "main" + p2 = _get_pillar( + opts, {"__env__ https://github.com/saltstack/salt-test-pillar-gitfs.git": mpoint} + ) + log.warning(f"DGM test_pygit2_fetch_request_with_mountpoint p2 '{p2}', p.remote[0] '{p.remotes[0]}', working_dir op '{p2.remotes[0].get_salt_working_dir()}'") + frequest2 = os.path.join(p2.remotes[0].get_salt_working_dir(), "fetch_request") + log.warning(f"DGM test_pygit2_fetch_request_with_mountpoint frequest2 '{frequest2}'") + assert frequest != frequest2 + assert os.path.isfile(frequest) is False + assert os.path.isfile(frequest2) is False + assert os.path.isfile(frequest_testmount) is False + p.fetch_remotes() + assert os.path.isfile(frequest) is False + # fetch request was placed + assert os.path.isfile(frequest2) is True + p2.checkout() + # fetch request was found + assert os.path.isfile(frequest2) is False + p2.fetch_remotes() + assert os.path.isfile(frequest) is True + assert os.path.isfile(frequest2) is False + assert os.path.isfile(frequest_testmount) is False + for _ in range(3): + p2.fetch_remotes() + assert os.path.isfile(frequest) is True + assert os.path.isfile(frequest2) is False + assert os.path.isfile(frequest_testmount) is False + # fetch request should still be processed even on fetch_on_fail=False + p.checkout(fetch_on_fail=False) + assert os.path.isfile(frequest) is False + assert os.path.isfile(frequest2) is False + assert os.path.isfile(frequest_testmount) is False + + + +@skipif_no_pygit2 +def test_pygit2_multiple_repos_with_mountpoint(pygit2_pillar_opts): + ## _test_multiple_repos(pygit2_pillar_opts) + opts = pygit2_pillar_opts + log.warning(f"DGM test_pygit2_multiple_repos_with_mountpoint opts '{opts}'") + mpoint = [{"mountpoint": "test_dir1/test_dir2"}] + p = _get_pillar( + opts, + {"__env__ https://github.com/saltstack/salt-test-pillar-gitfs.git": mpoint}, + {"testmount https://github.com/saltstack/salt-test-pillar-gitfs.git": mpoint}, + ) + p.checkout() + assert len(p.remotes) == 2 + # make sure all repos dont share cache and working dir + assert len({r.get_cachedir() for r in p.remotes}) == 2 + assert len({r.get_salt_working_dir() for r in p.remotes}) == 2 + + p2 = _get_pillar( + opts, + {"__env__ https://github.com/saltstack/salt-test-pillar-gitfs.git": mpoint}, + {"testmount https://github.com/saltstack/salt-test-pillar-gitfs.git": mpoint}, + ) + p2.checkout() + assert len(p2.remotes) == 2 + # make sure that repos are given same cache dir + for repo, repo2 in zip(p.remotes, p2.remotes): + assert repo.get_cachedir() == repo2.get_cachedir() + assert repo.get_salt_working_dir() == repo2.get_salt_working_dir() + opts["pillarenv"] = "main" + p3 = _get_pillar( + opts, + {"__env__ https://github.com/saltstack/salt-test-pillar-gitfs.git": mpoint}, + {"testmount https://github.com/saltstack/salt-test-pillar-gitfs.git": mpoint}, + ) + p3.checkout() + # check that __env__ has different cache with different pillarenv + assert p.remotes[0].get_cachedir() != p3.remotes[0].get_cachedir() + assert p.remotes[1].get_cachedir() == p3.remotes[1].get_cachedir() + + # check that __env__ branch data is in cache + files = set(os.listdir(p.remotes[0].get_cachedir())) + log.warning(f"DGM test_pygit2_multiple_repos_with_mountpoint p 0 files '{files}'") + for f in (".gitignore", "README.md", "file.sls", "top.sls"): + assert f in files + for f in ("test_dir1"): + assert f not in files + + # check that testmount branch data is in cache + files = set(os.listdir(p.remotes[1].get_cachedir())) + log.warning(f"DGM test_pygit2_multiple_repos_with_mountpoint p 1 files '{files}'") + for f in (".gitignore", "README.md", ".git", "test_dir1"): + assert f in files + + filesp30link = set(os.listdir(p3.remotes[0].get_linkdir())) + log.warning(f"DGM test_pygit2_multiple_repos_with_mountpoint files p3 0 linkdir '{filesp30link}'") + assert filesp30link == {"test_dir1"} + + filesp31link = set(os.listdir(p3.remotes[1].get_linkdir())) + log.warning(f"DGM test_pygit2_multiple_repos_with_mountpoint files p3 1 linkdir '{filesp31link}'") + assert filesp31link == {"test_dir1"} + + ## filesx = set(p3.remotes[1].file_list({"saltenv": "testmount"})) + ## pathfilesx = set(os.listdir(p3.remotes[1].get_cachedir())) + pathfilesx = os.path.join(p3.remotes[1].get_cachedir(), "test_dir1") + ## filesx = dir(p3.remotes[1]) + log.warning(f"DGM test_pygit2_multiple_repos_with_mountpoint pathfilesx p3 1 '{pathfilesx}'") + filesx = list(os.walk(pathfilesx)) + log.warning(f"DGM test_pygit2_multiple_repos_with_mountpoint filesx '{filesx}'") + for f in ("test_dir1/test_dir2/testfile1", "test_dir1/test_dir2/testfile2", "test_dir1/testfile3"): + log.warning(f"DGM test_pygit2_multiple_repos_with_mountpoint filesx file f '{f}'") + assert f in filesx From 534e588202e040e1fc3af3202799fc3e818030c4 Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Mon, 4 Sep 2023 21:02:54 -0500 Subject: [PATCH 30/31] clean up murphy --- tests/pytests/functional/utils/test_gitfs.py | 32 ++++- tests/pytests/functional/utils/test_pillar.py | 121 ------------------ 2 files changed, 31 insertions(+), 122 deletions(-) diff --git a/tests/pytests/functional/utils/test_gitfs.py b/tests/pytests/functional/utils/test_gitfs.py index 1ffcaf083882..e44f4aea34aa 100644 --- a/tests/pytests/functional/utils/test_gitfs.py +++ b/tests/pytests/functional/utils/test_gitfs.py @@ -69,7 +69,8 @@ def _get_gitfs(opts, *remotes): def _test_gitfs_simple(gitfs_opts): g = _get_gitfs( - gitfs_opts, "https://github.com/saltstack/salt-test-pillar-gitfs.git" + gitfs_opts, + {"https://github.com/saltstack/salt-test-pillar-gitfs.git": [{"name": "bob"}]}, ) g.fetch_remotes() assert len(g.remotes) == 1 @@ -154,3 +155,32 @@ def test_gitpython_gitfs_minion(gitpython_gitfs_opts): @skipif_no_pygit2 def test_pygit2_gitfs_minion(pygit2_gitfs_opts): _test_gitfs_minion(pygit2_gitfs_opts) + + +def _test_fetch_request_with_mountpoint(opts): + mpoint = [{"mountpoint": "salt/m"}] + p = _get_gitfs( + opts, + {"https://github.com/saltstack/salt-test-pillar-gitfs.git": mpoint}, + ) + p.fetch_remotes() + assert len(p.remotes) == 1 + repo = p.remotes[0] + assert repo.mountpoint("testmount") == "salt/m" + assert set(p.file_list({"saltenv": "testmount"})) == { + "salt/m/test_dir1/testfile3", + "salt/m/test_dir1/test_dir2/testfile2", + "salt/m/.gitignore", + "salt/m/README.md", + "salt/m/test_dir1/test_dir2/testfile1", + } + + +@skipif_no_gitpython +def test_gitpython_fetch_request_with_mountpoint(gitpython_gitfs_opts): + _test_fetch_request_with_mountpoint(gitpython_gitfs_opts) + + +@skipif_no_pygit2 +def test_pygit2_fetch_request_with_mountpoint(pygit2_gitfs_opts): + _test_fetch_request_with_mountpoint(pygit2_gitfs_opts) diff --git a/tests/pytests/functional/utils/test_pillar.py b/tests/pytests/functional/utils/test_pillar.py index 9091bc1c2bc6..896acf751a26 100644 --- a/tests/pytests/functional/utils/test_pillar.py +++ b/tests/pytests/functional/utils/test_pillar.py @@ -306,124 +306,3 @@ def test_gitpython_clear_old_remotes(gitpython_pillar_opts): @skipif_no_pygit2 def test_pygit2_clear_old_remotes(pygit2_pillar_opts): _test_clear_old_remotes(pygit2_pillar_opts) - -@skipif_no_pygit2 -def test_pygit2_fetch_request_with_mountpoint(pygit2_pillar_opts): - opts = pygit2_pillar_opts - mpoint = [{"mountpoint": "test_dir1/test_dir2"}] - p = _get_pillar( - opts, - {"__env__ https://github.com/saltstack/salt-test-pillar-gitfs.git": mpoint}, - {"testmount https://github.com/saltstack/salt-test-pillar-gitfs.git": mpoint}, - ) - log.warning(f"DGM test_pygit2_fetch_request_with_mountpoint p '{p}', p.remote[0] '{p.remotes[0]}', working_dir op '{p.remotes[0].get_salt_working_dir()}'") - frequest = os.path.join(p.remotes[0].get_salt_working_dir(), "fetch_request") - frequest_testmount = os.path.join(p.remotes[1].get_salt_working_dir(), "fetch_request") - log.warning(f"DGM test_pygit2_fetch_request_with_mountpoint frequest '{frequest}'") - log.warning(f"DGM test_pygit2_fetch_request_with_mountpoint frequest_testmount '{frequest_testmount}'") - opts["pillarenv"] = "main" - p2 = _get_pillar( - opts, {"__env__ https://github.com/saltstack/salt-test-pillar-gitfs.git": mpoint} - ) - log.warning(f"DGM test_pygit2_fetch_request_with_mountpoint p2 '{p2}', p.remote[0] '{p.remotes[0]}', working_dir op '{p2.remotes[0].get_salt_working_dir()}'") - frequest2 = os.path.join(p2.remotes[0].get_salt_working_dir(), "fetch_request") - log.warning(f"DGM test_pygit2_fetch_request_with_mountpoint frequest2 '{frequest2}'") - assert frequest != frequest2 - assert os.path.isfile(frequest) is False - assert os.path.isfile(frequest2) is False - assert os.path.isfile(frequest_testmount) is False - p.fetch_remotes() - assert os.path.isfile(frequest) is False - # fetch request was placed - assert os.path.isfile(frequest2) is True - p2.checkout() - # fetch request was found - assert os.path.isfile(frequest2) is False - p2.fetch_remotes() - assert os.path.isfile(frequest) is True - assert os.path.isfile(frequest2) is False - assert os.path.isfile(frequest_testmount) is False - for _ in range(3): - p2.fetch_remotes() - assert os.path.isfile(frequest) is True - assert os.path.isfile(frequest2) is False - assert os.path.isfile(frequest_testmount) is False - # fetch request should still be processed even on fetch_on_fail=False - p.checkout(fetch_on_fail=False) - assert os.path.isfile(frequest) is False - assert os.path.isfile(frequest2) is False - assert os.path.isfile(frequest_testmount) is False - - - -@skipif_no_pygit2 -def test_pygit2_multiple_repos_with_mountpoint(pygit2_pillar_opts): - ## _test_multiple_repos(pygit2_pillar_opts) - opts = pygit2_pillar_opts - log.warning(f"DGM test_pygit2_multiple_repos_with_mountpoint opts '{opts}'") - mpoint = [{"mountpoint": "test_dir1/test_dir2"}] - p = _get_pillar( - opts, - {"__env__ https://github.com/saltstack/salt-test-pillar-gitfs.git": mpoint}, - {"testmount https://github.com/saltstack/salt-test-pillar-gitfs.git": mpoint}, - ) - p.checkout() - assert len(p.remotes) == 2 - # make sure all repos dont share cache and working dir - assert len({r.get_cachedir() for r in p.remotes}) == 2 - assert len({r.get_salt_working_dir() for r in p.remotes}) == 2 - - p2 = _get_pillar( - opts, - {"__env__ https://github.com/saltstack/salt-test-pillar-gitfs.git": mpoint}, - {"testmount https://github.com/saltstack/salt-test-pillar-gitfs.git": mpoint}, - ) - p2.checkout() - assert len(p2.remotes) == 2 - # make sure that repos are given same cache dir - for repo, repo2 in zip(p.remotes, p2.remotes): - assert repo.get_cachedir() == repo2.get_cachedir() - assert repo.get_salt_working_dir() == repo2.get_salt_working_dir() - opts["pillarenv"] = "main" - p3 = _get_pillar( - opts, - {"__env__ https://github.com/saltstack/salt-test-pillar-gitfs.git": mpoint}, - {"testmount https://github.com/saltstack/salt-test-pillar-gitfs.git": mpoint}, - ) - p3.checkout() - # check that __env__ has different cache with different pillarenv - assert p.remotes[0].get_cachedir() != p3.remotes[0].get_cachedir() - assert p.remotes[1].get_cachedir() == p3.remotes[1].get_cachedir() - - # check that __env__ branch data is in cache - files = set(os.listdir(p.remotes[0].get_cachedir())) - log.warning(f"DGM test_pygit2_multiple_repos_with_mountpoint p 0 files '{files}'") - for f in (".gitignore", "README.md", "file.sls", "top.sls"): - assert f in files - for f in ("test_dir1"): - assert f not in files - - # check that testmount branch data is in cache - files = set(os.listdir(p.remotes[1].get_cachedir())) - log.warning(f"DGM test_pygit2_multiple_repos_with_mountpoint p 1 files '{files}'") - for f in (".gitignore", "README.md", ".git", "test_dir1"): - assert f in files - - filesp30link = set(os.listdir(p3.remotes[0].get_linkdir())) - log.warning(f"DGM test_pygit2_multiple_repos_with_mountpoint files p3 0 linkdir '{filesp30link}'") - assert filesp30link == {"test_dir1"} - - filesp31link = set(os.listdir(p3.remotes[1].get_linkdir())) - log.warning(f"DGM test_pygit2_multiple_repos_with_mountpoint files p3 1 linkdir '{filesp31link}'") - assert filesp31link == {"test_dir1"} - - ## filesx = set(p3.remotes[1].file_list({"saltenv": "testmount"})) - ## pathfilesx = set(os.listdir(p3.remotes[1].get_cachedir())) - pathfilesx = os.path.join(p3.remotes[1].get_cachedir(), "test_dir1") - ## filesx = dir(p3.remotes[1]) - log.warning(f"DGM test_pygit2_multiple_repos_with_mountpoint pathfilesx p3 1 '{pathfilesx}'") - filesx = list(os.walk(pathfilesx)) - log.warning(f"DGM test_pygit2_multiple_repos_with_mountpoint filesx '{filesx}'") - for f in ("test_dir1/test_dir2/testfile1", "test_dir1/test_dir2/testfile2", "test_dir1/testfile3"): - log.warning(f"DGM test_pygit2_multiple_repos_with_mountpoint filesx file f '{f}'") - assert f in filesx From 254c2ec4da4dfc6a9435c95d3667eeaef81a2b0d Mon Sep 17 00:00:00 2001 From: cmcmarrow Date: Tue, 5 Sep 2023 15:16:15 -0500 Subject: [PATCH 31/31] fix name --- salt/utils/gitfs.py | 33 ++++++------ tests/pytests/functional/utils/test_gitfs.py | 54 +++++++++++++++++++ tests/pytests/functional/utils/test_pillar.py | 22 ++++++++ .../pytests/functional/utils/test_winrepo.py | 20 +++++++ 4 files changed, 113 insertions(+), 16 deletions(-) diff --git a/salt/utils/gitfs.py b/salt/utils/gitfs.py index da25add4fad3..96a4185704c9 100644 --- a/salt/utils/gitfs.py +++ b/salt/utils/gitfs.py @@ -455,16 +455,18 @@ def __init__( self.id, ) failhard(self.role) - - hash_type = getattr(hashlib, self.opts.get("hash_type", "md5")) - # We loaded this data from yaml configuration files, so, its safe - # to use UTF-8 - self._cache_basehash = str( - base64.b64encode(hash_type(self.id.encode("utf-8")).digest()), - encoding="ascii", # base64 only outputs ascii - ).replace( - "/", "_" - ) # replace "/" with "_" to not cause trouble with file system + if hasattr(self, "name"): + self._cache_basehash = self.name + else: + hash_type = getattr(hashlib, self.opts.get("hash_type", "md5")) + # We loaded this data from yaml configuration files, so, its safe + # to use UTF-8 + self._cache_basehash = str( + base64.b64encode(hash_type(self.id.encode("utf-8")).digest()), + encoding="ascii", # base64 only outputs ascii + ).replace( + "/", "_" + ) # replace "/" with "_" to not cause trouble with file system self._cache_hash = salt.utils.path.join(cache_root, self._cache_basehash) self._cache_basename = "_" if self.id.startswith("__env__"): @@ -496,12 +498,11 @@ def __init__( msg += " Perhaps git is not available." log.critical(msg, exc_info=True) failhard(self.role) - else: - self.verify_auth() - self.setup_callbacks() - if not os.path.isdir(self._salt_working_dir): - os.makedirs(self._salt_working_dir) - self.fetch_request_check() + self.verify_auth() + self.setup_callbacks() + if not os.path.isdir(self._salt_working_dir): + os.makedirs(self._salt_working_dir) + self.fetch_request_check() def get_cache_basehash(self): return self._cache_basehash diff --git a/tests/pytests/functional/utils/test_gitfs.py b/tests/pytests/functional/utils/test_gitfs.py index e44f4aea34aa..fba68a17161e 100644 --- a/tests/pytests/functional/utils/test_gitfs.py +++ b/tests/pytests/functional/utils/test_gitfs.py @@ -1,3 +1,5 @@ +import os.path + import pytest from salt.fileserver.gitfs import PER_REMOTE_ONLY, PER_REMOTE_OVERRIDES @@ -184,3 +186,55 @@ def test_gitpython_fetch_request_with_mountpoint(gitpython_gitfs_opts): @skipif_no_pygit2 def test_pygit2_fetch_request_with_mountpoint(pygit2_gitfs_opts): _test_fetch_request_with_mountpoint(pygit2_gitfs_opts) + + +def _test_name(opts): + p = _get_gitfs( + opts, + { + "https://github.com/saltstack/salt-test-pillar-gitfs.git": [ + {"name": "name1"} + ] + }, + { + "https://github.com/saltstack/salt-test-pillar-gitfs.git": [ + {"name": "name2"} + ] + }, + ) + p.fetch_remotes() + assert len(p.remotes) == 2 + repo = p.remotes[0] + repo2 = p.remotes[1] + assert repo.get_cache_basehash() == "name1" + assert repo2.get_cache_basehash() == "name2" + + +@skipif_no_gitpython +def test_gitpython_name(gitpython_gitfs_opts): + _test_name(gitpython_gitfs_opts) + + +@skipif_no_pygit2 +def test_pygit2_name(pygit2_gitfs_opts): + _test_name(pygit2_gitfs_opts) + + +def _test_remote_map(opts): + p = _get_gitfs( + opts, + "https://github.com/saltstack/salt-test-pillar-gitfs.git", + ) + p.fetch_remotes() + assert len(p.remotes) == 1 + assert os.path.isfile(os.path.join(opts["cachedir"], "gitfs", "remote_map.txt")) + + +@skipif_no_gitpython +def test_gitpython_remote_map(gitpython_gitfs_opts): + _test_remote_map(gitpython_gitfs_opts) + + +@skipif_no_pygit2 +def test_pygit2_remote_map(pygit2_gitfs_opts): + _test_remote_map(pygit2_gitfs_opts) diff --git a/tests/pytests/functional/utils/test_pillar.py b/tests/pytests/functional/utils/test_pillar.py index 896acf751a26..d274ff6db5b7 100644 --- a/tests/pytests/functional/utils/test_pillar.py +++ b/tests/pytests/functional/utils/test_pillar.py @@ -306,3 +306,25 @@ def test_gitpython_clear_old_remotes(gitpython_pillar_opts): @skipif_no_pygit2 def test_pygit2_clear_old_remotes(pygit2_pillar_opts): _test_clear_old_remotes(pygit2_pillar_opts) + + +def _test_remote_map(opts): + p = _get_pillar( + opts, + "https://github.com/saltstack/salt-test-pillar-gitfs.git", + ) + p.fetch_remotes() + assert len(p.remotes) == 1 + assert os.path.isfile( + os.path.join(opts["cachedir"], "git_pillar", "remote_map.txt") + ) + + +@skipif_no_gitpython +def test_gitpython_remote_map(gitpython_pillar_opts): + _test_remote_map(gitpython_pillar_opts) + + +@skipif_no_pygit2 +def test_pygit2_remote_map(pygit2_pillar_opts): + _test_remote_map(pygit2_pillar_opts) diff --git a/tests/pytests/functional/utils/test_winrepo.py b/tests/pytests/functional/utils/test_winrepo.py index 1ec730cbb9d5..33897ca4b350 100644 --- a/tests/pytests/functional/utils/test_winrepo.py +++ b/tests/pytests/functional/utils/test_winrepo.py @@ -107,3 +107,23 @@ def test_gitpython_winrepo_simple(gitpython_winrepo_opts): @skipif_no_pygit2 def test_pygit2_winrepo_simple(pygit2_winrepo_opts): _test_winrepo_simple(pygit2_winrepo_opts) + + +def _test_remote_map(opts): + p = _get_winrepo( + opts, + "https://github.com/saltstack/salt-test-pillar-gitfs.git", + ) + p.fetch_remotes() + assert len(p.remotes) == 1 + assert os.path.isfile(os.path.join(opts["cachedir"], "winrepo", "remote_map.txt")) + + +@skipif_no_gitpython +def test_gitpython_remote_map(gitpython_winrepo_opts): + _test_remote_map(gitpython_winrepo_opts) + + +@skipif_no_pygit2 +def test_pygit2_remote_map(pygit2_winrepo_opts): + _test_remote_map(pygit2_winrepo_opts)