From 07feb8aee3fdbaf18fc5c1e214f27e180bc82041 Mon Sep 17 00:00:00 2001 From: jeanluc Date: Wed, 27 Nov 2024 10:31:22 +0100 Subject: [PATCH] Allow `file.recurse` to merge all existing `source` dirs --- changelog/67072.added.md | 1 + salt/states/file.py | 371 +++++++++++------- .../functional/states/file/test_recurse.py | 364 +++++++++++++++++ 3 files changed, 602 insertions(+), 134 deletions(-) create mode 100644 changelog/67072.added.md diff --git a/changelog/67072.added.md b/changelog/67072.added.md new file mode 100644 index 000000000000..3dea814bf198 --- /dev/null +++ b/changelog/67072.added.md @@ -0,0 +1 @@ +Added a `merge` option to `file.recurse`, which merges subpaths from all existing `source`s before managing the directory. Handy when using different saltenvs or the TOFS pattern. diff --git a/salt/states/file.py b/salt/states/file.py index 10b16cd53173..368b24809ba9 100644 --- a/salt/states/file.py +++ b/salt/states/file.py @@ -294,6 +294,7 @@ def run(): from collections.abc import Iterable, Mapping from datetime import date, datetime # python3 problem in the making? from itertools import zip_longest +from pathlib import Path import salt.loader import salt.payload @@ -430,134 +431,226 @@ def _salt_to_os_path(path): return os.path.normpath(path.replace(posixpath.sep, os.path.sep)) +def _filter_recurse_sources(sources): + """ + Adaptation of ``file.source_list`` for file.recurse. Returns a list + of all ``salt://`` scheme sources that represent exisiting directories. + """ + source_list = _validate_str_list(sources) + + for idx, val in enumerate(source_list): + source_list[idx] = val.rstrip("/") + + for precheck in source_list: + if not precheck.startswith("salt://"): + raise CommandExecutionError( + f"Invalid source '{precheck}' (must be a salt:// URI)" + ) + + contextkey = f"recurse__|-{source_list}__|-{__env__}" + if contextkey in __context__: + return __context__[contextkey] + mdirs = {} + ret = [] + for single in source_list: + path, senv = salt.utils.url.parse(single) + if not senv: + senv = __env__ + if senv not in mdirs: + mdirs[senv] = __salt__["cp.list_master_dirs"](senv) + if path in mdirs[senv]: + ret.append(single) + if not ret: + raise CommandExecutionError("none of the specified sources were found") + __context__[contextkey] = ret + return __context__[contextkey] + + def _gen_recurse_managed_files( name, - source, + sources, keep_symlinks=False, include_pat=None, exclude_pat=None, maxdepth=None, include_empty=False, + merge=False, **kwargs, ): """ Generate the list of files managed by a recurse state """ + managed_files = {} + managed_directories = set() + managed_symlinks = {} + keep = set() + all_directories = set() + ignored_directories = set() + + if not merge: + try: + sources = [sources[0]] + except KeyError: + sources = [] # Convert a relative path generated from salt master paths to an OS path # using "name" as the base directory def full_path(master_relpath): return os.path.join(name, _salt_to_os_path(master_relpath)) + def check_relative_path(fileserver_path, fs_recurse_base_dir): + if not fileserver_path.strip(): + return False + # Render the file path relative to the source (== target) dir + relative_path = salt.utils.data.decode( + posixpath.relpath(fileserver_path, fs_recurse_base_dir) + ) + if not _is_valid_relpath(relative_path, maxdepth=maxdepth): + return False + + # Check if it is to be excluded. Match only part of the path + # relative to the target directory. + if not salt.utils.stringutils.check_include_exclude( + relative_path, include_pat, exclude_pat + ): + return False + return relative_path + + # Check whether an absolute destination path is already occupied. + def is_occupied(dest): + dpath = Path(dest) + return ( + any( + path in aggr + for path in [str(dest), *(str(parent) for parent in dpath.parents)] + for aggr in (managed_files, managed_symlinks) + ) + or str(dest) in all_directories + ) + # Process symlinks and return the updated filenames list - def process_symlinks(filenames, symlinks): + def process_symlinks(filenames): + symlinks = __salt__["cp.list_master_symlinks"](senv, recurse_root) for lname, ltarget in symlinks.items(): - srelpath = posixpath.relpath(lname, srcpath) - if not _is_valid_relpath(srelpath, maxdepth=maxdepth): + relative_path = check_relative_path(lname, recurse_root) + if relative_path is False: continue - if not salt.utils.stringutils.check_include_exclude( - srelpath, include_pat, exclude_pat - ): - continue - # Check for all paths that begin with the symlink - # and axe it leaving only the dirs/files below it. - # This needs to use list() otherwise they reference - # the same list. - _filenames = list(filenames) - for filename in _filenames: - if filename.startswith(lname + os.sep): - log.debug( - "** skipping file ** %s, it intersects a symlink", filename - ) + + # The fileclient (usually) follows symlinks when listing paths. + # We need to remove the symlink and (if it points to a directory) + # paths below it from the regular files list. + target_was_dir = False + for filename in list(filenames): + if filename == lname or filename.startswith(lname + os.sep): filenames.remove(filename) - # Create the symlink along with the necessary dirs. - # The dir perms/ownership will be adjusted later - # if needed - managed_symlinks.add((srelpath, ltarget)) + if filename != lname: + log.debug( + "** skipping file ** %s, it intersects a symlink", filename + ) + target_was_dir = True + dest = full_path(relative_path) + if is_occupied(dest): + continue + tdest = full_path(ltarget) + # Ensure symlink targets don't change type because of merging, + # otherwise drop the symlink. + if target_was_dir and tdest in managed_files: + # We need to ignore it explicitly (when include_empty is true), + # otherwise the directory would be detected as an empty one. + ignored_directories.add(dest) + continue + if not target_was_dir and tdest in managed_directories: + continue + managed_symlinks[dest] = ltarget # Add the path to the keep set in case clean is set to True - keep.add(full_path(srelpath)) - vdir.update(keep) + keep.add(dest) return filenames - managed_files = set() - managed_directories = set() - managed_symlinks = set() - keep = set() - vdir = set() - - srcpath, senv = salt.utils.url.parse(source) - if senv is None: - senv = __env__ - if not srcpath.endswith(posixpath.sep): - # we're searching for things that start with this *directory*. - srcpath = srcpath + posixpath.sep - fns_ = __salt__["cp.list_master"](senv, srcpath) - - # If we are instructed to keep symlinks, then process them. - if keep_symlinks: - # Make this global so that emptydirs can use it if needed. - symlinks = __salt__["cp.list_master_symlinks"](senv, srcpath) - fns_ = process_symlinks(fns_, symlinks) - - for fn_ in fns_: - if not fn_.strip(): - continue - - # fn_ here is the absolute (from file_roots) source path of - # the file to copy from; it is either a normal file or an - # empty dir(if include_empty==true). - - relname = salt.utils.data.decode(posixpath.relpath(fn_, srcpath)) - if not _is_valid_relpath(relname, maxdepth=maxdepth): - continue - - # Check if it is to be excluded. Match only part of the path - # relative to the target directory - if not salt.utils.stringutils.check_include_exclude( - relname, include_pat, exclude_pat - ): - continue - dest = full_path(relname) - dirname = os.path.dirname(dest) - keep.add(dest) - - if dirname not in vdir: - # verify the directory perms if they are set - managed_directories.add(dirname) - vdir.add(dirname) - - src = salt.utils.url.create(fn_, saltenv=senv) - managed_files.add((dest, src)) - - if include_empty: - mdirs = __salt__["cp.list_master_dirs"](senv, srcpath) - for mdir in mdirs: - relname = posixpath.relpath(mdir, srcpath) - if not _is_valid_relpath(relname, maxdepth=maxdepth): + for source in sources: + recurse_root, senv = salt.utils.url.parse(source) + if senv is None: + senv = __env__ + if not recurse_root.endswith(posixpath.sep): + # we're searching for things that start with this *directory*. + recurse_root += posixpath.sep + + # Fetch a list of all files in the current source. + # Includes resolved symlinks. + fns_ = __salt__["cp.list_master"](senv, recurse_root) + + # If we are instructed to keep symlinks, then process them. + if keep_symlinks: + fns_ = process_symlinks(fns_) + + for fn_ in fns_: + relative_path = check_relative_path(fn_, recurse_root) + if relative_path is False: continue - if not salt.utils.stringutils.check_include_exclude( - relname, include_pat, exclude_pat - ): + + dest = full_path(relative_path) + dpath = Path(dest) + if is_occupied(dpath): + # We're merging multiple sources and a higher-priority one + # provides the same path or a parent one that's not a directory. continue - mdest = full_path(relname) - # Check for symlinks that happen to point to an empty dir. - if keep_symlinks: - islink = False - for link in symlinks: - if mdir.startswith(link + os.sep, 0): - log.debug( - "** skipping empty dir ** %s, it intersects a symlink", mdir - ) - islink = True - break - if islink: - continue + keep.add(dest) + + managed_directories.add(str(dpath.parent)) + all_directories.update(str(par) for par in dpath.parents) - managed_directories.add(mdest) - keep.add(mdest) + salt_uri_with_senv = salt.utils.url.create(fn_, saltenv=senv) + managed_files[dest] = salt_uri_with_senv - return managed_files, managed_directories, managed_symlinks, keep + if include_empty: + mdirs = __salt__["cp.list_master_dirs"](senv, recurse_root) + for mdir in mdirs: + relative_path = check_relative_path(mdir, recurse_root) + if relative_path is False: + continue + dest = full_path(relative_path) + if dest in all_directories: + # Early check to avoid some unnecessary work. + continue + if dest in ignored_directories or is_occupied(dest): + # In addition to the usual checks, don't try to create + # directories that served as symlink targets in the current + # source layer and were overridden with a file in a higher + # priority one. + continue + managed_directories.add(dest) + all_directories.add(dest) + keep.add(dest) + + elif keep_symlinks: + # Filter invalid symlinks caused by pointing to an empty directory + # and include_empty being false. This needs to happen recursively + # because symlinks can point to other symlinks. + removed_links = None + mdirs = __salt__["cp.list_master_dirs"](senv, recurse_root) + while removed_links is None or removed_links: + removed_links = 0 + for lnk in list(managed_symlinks): + relative_target = managed_symlinks[lnk] + if recurse_root + relative_target not in mdirs: + # The symlink never pointed to a directory in the first place + continue + full_tgt = full_path(relative_target) + if any( + full_tgt in aggr + for aggr in (managed_files, all_directories, managed_symlinks) + ): + # The target exists in some way + continue + managed_symlinks.pop(lnk) + removed_links += 1 + + return ( + set(managed_files.items()), + managed_directories, + set(managed_symlinks.items()), + keep, + ) def _gen_keep_files(name, require, walk_d=None): @@ -623,9 +716,21 @@ def _process(name): if os.path.isdir(fn): if _is_child(fn, name): if fun == "recurse": - fkeep = _gen_recurse_managed_files(**low)[3] - log.debug("Keep from %s: %s", fn, fkeep) - keep.update(fkeep) + try: + sources = _filter_recurse_sources( + low.get("source", []) + ) + except CommandExecutionError: + pass + else: + recurse_kwargs = low.copy() + recurse_kwargs.pop("source", None) + recurse_kwargs["sources"] = sources + fkeep = _gen_recurse_managed_files( + **recurse_kwargs + )[3] + log.debug("Keep from %s: %s", fn, fkeep) + keep.update(fkeep) elif walk_d: walk_ret = set() _process_by_walk_d(fn, walk_ret) @@ -4474,6 +4579,7 @@ def recurse( win_perms=None, win_deny_perms=None, win_inheritance=True, + merge=False, **kwargs, ): """ @@ -4680,6 +4786,23 @@ def recurse( .. versionadded:: 2017.7.7 + merge + When ``source`` is a list, instead of choosing the first existing + source as the single source directory, merge paths in all existing + sources. When the same path exists in several sources, the earliest + source has priority. Defaults to false. + + .. versionadded:: 3008.0 + + .. note:: + When ``keep_symlinks`` is true, symlinks are treated like files, + i.e. override lower priority files, directories and symlinks completely. + When a higher priority source overrides the symlink target with the + same type (file -> file/dir -> dir), they are kept as expected. + Special handling is given to symlinks whose target type changes + because of the override (file -> dir/dir -> file), where the + (lower priority) symlink is dropped, even if it is not overridden + by a higher priority source. """ if "env" in kwargs: # "env" is not supported; Use "saltenv". @@ -4737,41 +4860,14 @@ def recurse( if not os.path.isabs(name): return _error(ret, f"Specified file {name} is not an absolute path") - # expand source into source_list - source_list = _validate_str_list(source) - - for idx, val in enumerate(source_list): - source_list[idx] = val.rstrip("/") - - for precheck in source_list: - if not precheck.startswith("salt://"): - return _error( - ret, - f"Invalid source '{precheck}' (must be a salt:// URI)", - ) - - # Select the first source in source_list that exists + # Validate the sources and ensure that at least one exists. try: - source, source_hash = __salt__["file.source_list"](source_list, "", __env__) + sources = _filter_recurse_sources(source) except CommandExecutionError as exc: ret["result"] = False ret["comment"] = f"Recurse failed: {exc}" return ret - # Check source path relative to fileserver root, make sure it is a - # directory - srcpath, senv = salt.utils.url.parse(source) - if senv is None: - senv = __env__ - master_dirs = __salt__["cp.list_master_dirs"](saltenv=senv, prefix=srcpath + "/") - if srcpath not in master_dirs: - ret["result"] = False - ret["comment"] = ( - "The directory '{}' does not exist on the salt fileserver " - "in saltenv '{}'".format(srcpath, senv) - ) - return ret - # Verify the target directory if not os.path.isdir(name): if os.path.exists(name): @@ -4878,12 +4974,19 @@ def manage_directory(path): merge_ret(path, _ret) mng_files, mng_dirs, mng_symlinks, keep = _gen_recurse_managed_files( - name, source, keep_symlinks, include_pat, exclude_pat, maxdepth, include_empty + name, + sources, + keep_symlinks=keep_symlinks, + include_pat=include_pat, + exclude_pat=exclude_pat, + maxdepth=maxdepth, + include_empty=include_empty, + merge=merge, ) - for srelpath, ltarget in mng_symlinks: + for sdest, ltarget in mng_symlinks: _ret = symlink( - os.path.join(name, srelpath), + sdest, ltarget, makedirs=True, force=force_symlinks, @@ -4893,7 +4996,7 @@ def manage_directory(path): ) if not _ret: continue - merge_ret(os.path.join(name, srelpath), _ret) + merge_ret(sdest, _ret) for dirname in mng_dirs: manage_directory(dirname) for dest, src in mng_files: diff --git a/tests/pytests/functional/states/file/test_recurse.py b/tests/pytests/functional/states/file/test_recurse.py index c735d5128dac..f469b288221a 100644 --- a/tests/pytests/functional/states/file/test_recurse.py +++ b/tests/pytests/functional/states/file/test_recurse.py @@ -1,3 +1,6 @@ +import contextlib +import copy +import shutil import stat import pytest @@ -154,6 +157,367 @@ def test_recurse_clean_specific_env(file, tmp_path): assert scene_32_dst.joinpath("scene").is_file() is True +@contextlib.contextmanager +def create_file_tree(base, tree): + """ + Helper to quickly create trees of directories, files and symlinks + inside a base directory. + """ + temp_files = [] + symlinks = {} + empty_dirs = [] + + def _create_tmp_files(tree, prefix): + for name, val in tree.items(): + if isinstance(val, dict): + if not val: + empty_dirs.append(base / prefix / name) + continue + _create_tmp_files(val, prefix + f"/{name}") + continue + if isinstance(val, tuple): + symlinks[base / prefix / name] = val[0] + continue + temp_files.append(pytest.helpers.temp_file(name, val, base / prefix)) + + for first_level_name, first_level_contents in tree.items(): + _create_tmp_files(first_level_contents, first_level_name) + + with contextlib.ExitStack() as stack: + for file in temp_files: + stack.enter_context(file) + try: + created_dirs = [] + for symlink, target in symlinks.items(): + for par in symlink.parents[::-1]: + if not par.exists(): + created_dirs.append(par) + break + symlink.parent.mkdir(parents=True, exist_ok=True) + symlink.symlink_to(target) + for empty_dir in empty_dirs: + for par in empty_dir.parents[::-1]: + if not par.exists(): + created_dirs.append(par) + break + empty_dir.mkdir(parents=True) + yield + finally: + for symlink in symlinks: + symlink.unlink(missing_ok=True) + for created_dir in created_dirs + empty_dirs: + shutil.rmtree(created_dir, ignore_errors=True) + + +@pytest.fixture +def _recurse_merge_dirs(state_tree, state_tree_prod): + tree_base = { + "file_from_base": "base", + "overridden_file": "foo", + "common": { + "file_from_base": "base", + "overridden_file": "foo", + }, + "base": { + "present": "", + }, + "empty": { + "in_base": {}, + "common": {}, + "override": {}, + "override_file": "", + "nested_in_base_only": { + "worked": {}, + "nonempty": { + "file": "", + }, + }, + }, + "deeply": "not_rendered", + "file_overridden_by_dir": "file", + "dir_overridden_by_file": { + "file_in_dir": "", + }, + "symlinked_dir": ("symlink_target_dir",), + "symlinked_file": ("symlink_target_file",), + "symlink_target_dir": { + "file_in_symlink_target": "", + }, + "symlink_target_file": "", + "symlink_dir_overridden_by_file": ("symlink_target_dir",), + "symlink_dir_overridden_by_dir": ("symlink_target_dir",), + "symlink_file_overridden_by_dir": ("symlink_target_file",), + "symlink_file_overridden_by_file": ("symlink_target_file",), + "dir_overridden_by_symlink_dir": { + "file_in_dir_overridden_by_symlink_dir": "", + }, + "dir_overridden_by_symlink_file": { + "file_in_dir_overridden_by_symlink_file": "", + }, + "file_overridden_by_symlink_dir": "", + "file_overridden_by_symlink_file": "", + "symlink_file_overridden_by_symlink_file": ("symlink_target_file",), + "symlink_file_overridden_by_symlink_dir": ("symlink_target_file",), + "symlink_dir_overridden_by_symlink_file": ("symlink_target_dir",), + "symlink_dir_overridden_by_symlink_dir": ("symlink_target_dir",), + "symlink_target_dir_overridden_by_file": ("starget_dir_overridden_by_file",), + "starget_dir_overridden_by_file": { + "file_in_starget_dir_overridden_by_file": "", + }, + "symlink_target_dir_overridden_by_dir": ("starget_dir_overridden_by_dir",), + "starget_dir_overridden_by_dir": { + "file_in_starget_dir_overridden_by_dir": "", + }, + "symlink_target_file_overridden_by_dir": ("starget_file_overridden_by_dir",), + "starget_file_overridden_by_dir": "", + "symlink_target_file_overridden_by_file": ("starget_file_overridden_by_file",), + "starget_file_overridden_by_file": "", + } + tree_base["nested"] = copy.deepcopy(tree_base) + tree_override = { + "file_from_override": "override", + "overridden_file": "quux", + "common": { + "file_from_override": "override", + "overridden_file": "quux", + }, + "empty": { + "in_override": {}, + "common": {}, + "override_dir": "", + "override_file": {}, + "nested_in_base_only": {}, + "symlinked": { + "dir": {}, + }, + }, + "empty_symlinked_dir": ("empty/symlinked",), + "empty_symlinked_dir_2": ("empty_symlinked_dir",), + "deeply": { + "nested": { + "path": { + "overrides_file": "success", + }, + }, + }, + "override": { + "present": "", + }, + "file_overridden_by_dir": { + "file_in_overriding_dir": "success", + }, + "dir_overridden_by_file": "success", + "symlink_dir_overridden_by_dir": { + "file_in_dir_overriding_symlink_dir": "success", + }, + "symlink_dir_overridden_by_file": "success", + "symlink_file_overridden_by_file": "success", + "symlink_file_overridden_by_dir": { + "file_in_dir_overriding_symlink_file": "success", + }, + "other_symlink_target_dir": { + "other_file_in_symlink_target": "success", + }, + "other_symlink_target_file": "success", + "dir_overridden_by_symlink_dir": ("other_symlink_target_dir",), + "dir_overridden_by_symlink_file": ("other_symlink_target_file",), + "file_overridden_by_symlink_dir": ("other_symlink_target_dir",), + "file_overridden_by_symlink_file": ("other_symlink_target_file",), + "symlink_file_overridden_by_symlink_file": ("other_symlink_target_file",), + "symlink_file_overridden_by_symlink_dir": ("other_symlink_target_dir",), + "symlink_dir_overridden_by_symlink_file": ("other_symlink_target_file",), + "symlink_dir_overridden_by_symlink_dir": ("other_symlink_target_dir",), + "starget_dir_overridden_by_file": "success", + "starget_dir_overridden_by_dir": { + "other_file_in_starget_dir_overridden_by_dir": "", + }, + "starget_file_overridden_by_dir": { + "file_in_starget_file_overridden_by_dir": "success", + }, + "starget_file_overridden_by_file": "success", + } + tree_override["nested"] = copy.deepcopy(tree_override) + + with create_file_tree(state_tree, {"base": tree_base}): + with create_file_tree(state_tree_prod, {"override": tree_override}): + yield + + +@pytest.mark.usefixtures("_recurse_merge_dirs") +@pytest.mark.parametrize("keep_symlinks", (False, True)) +@pytest.mark.parametrize("include_empty", (False, True)) +def test_recurse_merge(file, tmp_path, keep_symlinks, include_empty): + tgt = tmp_path / "target" + dirs = ("override", "base") + sources = [ + f"salt://{src}" + ("?saltenv=prod" if src == "override" else "") for src in dirs + ] + ret = file.recurse( + name=str(tgt), + source=sources, + merge=True, + keep_symlinks=keep_symlinks, + include_empty=include_empty, + ) + assert ret.result is True + assert tgt.is_dir() + + # Run the same assertions for both the root path and a nested dir + for base in (tgt, tgt / "nested"): + # Ensure unique dirs are all merged + for tgt_dir in dirs: + assert (base / tgt_dir / "present").exists() + for shared_dir in (base, base / "common"): + # Ensure unique files in shared dir are all merged + for suffix in dirs: + assert (shared_dir / f"file_from_{suffix}").is_file() + assert (shared_dir / f"file_from_{suffix}").read_text() == suffix + # Ensure shared files are overridden by first source + assert (shared_dir / "overridden_file").is_file() + assert (shared_dir / "overridden_file").read_text() == "quux" + # Ensure dir <-> file overrides work + for path in ( + ("file_overridden_by_dir", "file_in_overriding_dir"), + ("dir_overridden_by_file",), + ): + assert base.joinpath(*path).is_file() + assert base.joinpath(*path).read_text() == "success" + assert (base / "deeply" / "nested" / "path" / "overrides_file").exists() + + # Sanity check include_empty + for empty_dir in ("in_base", "in_override", "common"): + assert ((base / "empty" / empty_dir).is_dir()) is include_empty + # Ensure empty dirs can be overridden by files + assert (base / "empty" / "override_dir").is_file() + # Ensure empty dirs can override files when include_empty is set + assert ((base / "empty" / "override_file").is_dir()) is include_empty + # Ensure empty dirs in overrides don't block nested dirs/files + assert ( + (base / "empty" / "nested_in_base_only" / "worked").is_dir() + ) is include_empty + assert (base / "empty" / "nested_in_base_only" / "nonempty" / "file").is_file() + + # Sanity check symlinks + assert ((base / "symlinked_dir").is_symlink()) is keep_symlinks + assert (base / "symlinked_dir").is_dir() + assert (base / "symlink_target_dir").is_dir() + for empty_symlink in ("empty_symlinked_dir", "empty_symlinked_dir_2"): + assert ((base / empty_symlink).exists()) is include_empty + assert ((base / empty_symlink).is_dir()) is include_empty + assert ((base / empty_symlink).is_symlink()) is ( + include_empty and keep_symlinks + ) + + # Ensure symlinks can be overridden + for symlink_overridden in ( + "symlink_dir_overridden_by_file", + "symlink_dir_overridden_by_dir", + "symlink_file_overridden_by_file", + "symlink_file_overridden_by_dir", + ): + assert not (base / symlink_overridden).is_symlink() + if symlink_overridden.endswith("file"): + assert (base / symlink_overridden).read_text() == "success" + else: + assert ( + base / symlink_overridden / "file_in_dir_overriding_" + + symlink_overridden.split("_overridden", maxsplit=1)[0] + ).read_text() == "success" + + # Ensure symlinks can override files/directories and other symlinks + for symlink in ( + "dir_overridden_by_symlink_dir", + "dir_overridden_by_symlink_file", + "file_overridden_by_symlink_dir", + "file_overridden_by_symlink_file", + "symlink_dir_overridden_by_symlink_dir", + "symlink_dir_overridden_by_symlink_file", + "symlink_file_overridden_by_symlink_dir", + "symlink_file_overridden_by_symlink_file", + ): + assert ((base / symlink).is_symlink()) is keep_symlinks + if symlink.endswith("file"): + assert (base / symlink).read_text() == "success" + else: + assert ( + base / symlink / "other_file_in_symlink_target" + ).read_text() == "success" + + # Ensure symlinked dirs are merged when not preserving symlinks + assert ( + base / "symlink_dir_overridden_by_dir" / "file_in_symlink_target" + ).exists() is not keep_symlinks + assert ( + base / "symlink_dir_overridden_by_symlink_dir" / "file_in_symlink_target" + ).exists() is not keep_symlinks + assert ( + base + / "dir_overridden_by_symlink_dir" + / "file_in_dir_overridden_by_symlink_dir" + ).exists() is not keep_symlinks + + # Ensure overridden symlink targets don't cause surprises + assert (base / "symlink_target_file_overridden_by_file").exists() + assert ( + (base / "symlink_target_file_overridden_by_file").is_symlink() + ) is keep_symlinks + assert ( + (base / "symlink_target_file_overridden_by_file").read_text() == "success" + ) is keep_symlinks + assert ( + (base / "symlink_target_dir_overridden_by_dir").is_symlink() + ) is keep_symlinks + assert (base / "symlink_target_dir_overridden_by_dir").is_dir() + + assert not (base / "symlink_target_dir_overridden_by_file").is_symlink() + assert ( + (base / "symlink_target_dir_overridden_by_file").exists() + ) is not keep_symlinks + assert ( + (base / "symlink_target_dir_overridden_by_file").is_dir() + ) is not keep_symlinks + + assert not (base / "symlink_target_file_overridden_by_dir").is_symlink() + assert ( + (base / "symlink_target_file_overridden_by_dir").exists() + ) is not keep_symlinks + assert ( + (base / "symlink_target_file_overridden_by_dir").is_file() + ) is not keep_symlinks + + +@pytest.mark.parametrize("include_empty", (False, True)) +def test_recurse_merge_keep_symlinks_broken_symlinks_are_kept( + file, state_tree, tmp_path, include_empty +): + """ + When ``keep_symlinks`` is set, broken symlinks should be kept. + They cause an exception when keep_symlinks is false + (and the fileserver follows them, ``fileserver_followsymlinks``). + + This is not specific to ``merge=True``, but should be tested regardless. + """ + tree = { + "base": {"foo": {"random_file": "", "broken_link": ("../nonexistent",)}}, + "override": {"other_broken_link": ("42",)}, + } + with create_file_tree(state_tree, tree): + tgt = tmp_path / "target" + dirs = ("override", "base") + sources = [f"salt://{src}" for src in dirs] + ret = file.recurse( + name=str(tgt), + source=sources, + merge=True, + keep_symlinks=True, + include_empty=include_empty, + ) + assert ret.result is True + assert (tgt / "foo" / "broken_link").is_symlink() + assert (tgt / "foo" / "random_file").is_file() + assert (tgt / "other_broken_link").is_symlink() + + @pytest.mark.skip_on_windows(reason="'dir_mode' is not supported on Windows") def test_recurse_issue_34945(file, tmp_path, state_tree): """