From bbb0013138bc8d6c5769f6211da5dd9773a3cf82 Mon Sep 17 00:00:00 2001 From: twangboy Date: Fri, 6 Sep 2024 13:54:05 -0600 Subject: [PATCH] Make sure symlinks are done last --- salt/fileserver/roots.py | 2 +- salt/states/file.py | 23 +++++-- .../functional/states/file/test_recurse.py | 65 +++++++++++++++---- 3 files changed, 72 insertions(+), 18 deletions(-) diff --git a/salt/fileserver/roots.py b/salt/fileserver/roots.py index e81f37dcf02..cb27396b979 100644 --- a/salt/fileserver/roots.py +++ b/salt/fileserver/roots.py @@ -325,7 +325,7 @@ def file_hash(load, fnd): def _file_lists(load, form): """ - Return a dict containing the file lists for files, dirs, emtydirs and symlinks + Return a dict containing the file lists for files, dirs, empty dirs and symlinks """ if "env" in load: # "env" is not supported; Use "saltenv". diff --git a/salt/states/file.py b/salt/states/file.py index 04c490e9710..2e943f0797c 100644 --- a/salt/states/file.py +++ b/salt/states/file.py @@ -557,7 +557,18 @@ def _gen_recurse_managed_files( managed_directories.add(mdest) keep.add(mdest) - return managed_files, managed_directories, managed_symlinks, keep + # Sets are randomly ordered. We need to make sure symlinks are always at the + # end, so we need to use a list + new_managed_files = list(managed_files) + # Now let's move all the symlinks to the end + for symlink in managed_symlinks: + for file in managed_files: + if file[0].endswith(symlink[0]): + new_managed_files.append( + new_managed_files.pop(new_managed_files.index(file)) + ) + + return new_managed_files, managed_directories, managed_symlinks, keep def _gen_keep_files(name, require, walk_d=None): @@ -4668,16 +4679,12 @@ def recurse( name, source, keep_symlinks, include_pat, exclude_pat, maxdepth, include_empty ) - for dirname in mng_dirs: - manage_directory(dirname) - for dest, src in mng_files: - manage_file(dest, src, replace) for srelpath, ltarget in mng_symlinks: _ret = symlink( os.path.join(name, srelpath), ltarget, makedirs=True, - force=force_symlinks or keep_symlinks, + force=force_symlinks, user=user, group=group, mode=sym_mode, @@ -4685,6 +4692,10 @@ def recurse( if not _ret: continue merge_ret(os.path.join(name, srelpath), _ret) + for dirname in mng_dirs: + manage_directory(dirname) + for dest, src in mng_files: + manage_file(dest, src, replace) if clean: # TODO: Use directory(clean=True) instead diff --git a/tests/pytests/functional/states/file/test_recurse.py b/tests/pytests/functional/states/file/test_recurse.py index 53583b26b2d..04370c046af 100644 --- a/tests/pytests/functional/states/file/test_recurse.py +++ b/tests/pytests/functional/states/file/test_recurse.py @@ -8,16 +8,35 @@ pytestmark = [ @pytest.fixture(scope="module") -def symlink(state_tree): +def symlink_scenario_1(state_tree): # Create directory structure - source_dir = state_tree / "test_symlink" + dir_name = "symlink_scenario_1" + source_dir = state_tree / dir_name if not source_dir.is_dir(): source_dir.mkdir() source_file = source_dir / "source_file.txt" source_file.write_text("This is the source file...") symlink_file = source_dir / "symlink" symlink_file.symlink_to(source_file) - yield + yield dir_name + + +@pytest.fixture(scope="module") +def symlink_scenario_2(state_tree): + # Create directory structure + dir_name = "symlink_scenario_2" + source_dir = state_tree / dir_name / "test" + if not source_dir.is_dir(): + source_dir.mkdir(parents=True) + test1 = source_dir / "test1" + test2 = source_dir / "test2" + test3 = source_dir / "test3" + test_link = source_dir / "test" + test1.touch() + test2.touch() + test3.touch() + test_link.symlink_to(test3) + yield dir_name @pytest.mark.parametrize("test", (False, True)) @@ -264,11 +283,11 @@ def test_issue_2726_mode_kwarg(modules, tmp_path, state_tree): assert state_run.result is True -def test_issue_64630_keep_symlinks_true(file, symlink, tmp_path): +def test_issue_64630_keep_symlinks_true(file, symlink_scenario_1, tmp_path): """ Make sure that symlinks are created and that there isn't an error """ - target_dir = tmp_path / "test_symlink" # Target for the file.recurse state + target_dir = tmp_path / symlink_scenario_1 # Target for the file.recurse state target_file = target_dir / "source_file.txt" target_symlink = target_dir / "symlink" @@ -282,11 +301,11 @@ def test_issue_64630_keep_symlinks_true(file, symlink, tmp_path): assert target_symlink.is_symlink() -def test_issue_64630_keep_symlinks_false(file, symlink, tmp_path): +def test_issue_64630_keep_symlinks_false(file, symlink_scenario_1, tmp_path): """ Make sure that symlinks are created and that there isn't an error """ - target_dir = tmp_path / "test_symlink" # Target for the file.recurse state + target_dir = tmp_path / symlink_scenario_1 # Target for the file.recurse state target_file = target_dir / "source_file.txt" target_symlink = target_dir / "symlink" @@ -301,11 +320,11 @@ def test_issue_64630_keep_symlinks_false(file, symlink, tmp_path): assert target_file.read_text() == target_symlink.read_text() -def test_issue_64630_force_symlinks_true(file, symlink, tmp_path): +def test_issue_64630_force_symlinks_true(file, symlink_scenario_1, tmp_path): """ Make sure that symlinks are created and that there isn't an error """ - target_dir = tmp_path / "test_symlink" # Target for the file.recurse state + target_dir = tmp_path / symlink_scenario_1 # Target for the file.recurse state target_file = target_dir / "source_file.txt" target_symlink = target_dir / "symlink" @@ -319,11 +338,13 @@ def test_issue_64630_force_symlinks_true(file, symlink, tmp_path): assert target_symlink.is_file() -def test_issue_64630_force_symlinks_keep_symlinks_true(file, symlink, tmp_path): +def test_issue_64630_force_symlinks_keep_symlinks_true( + file, symlink_scenario_1, tmp_path +): """ Make sure that symlinks are created and that there isn't an error """ - target_dir = tmp_path / "test_symlink" # Target for the file.recurse state + target_dir = tmp_path / symlink_scenario_1 # Target for the file.recurse state target_file = target_dir / "source_file.txt" target_symlink = target_dir / "symlink" @@ -338,3 +359,25 @@ def test_issue_64630_force_symlinks_keep_symlinks_true(file, symlink, tmp_path): assert target_dir.exists() assert target_file.is_file() assert target_symlink.is_symlink() + + +def test_issue_62117(file, symlink_scenario_2, tmp_path): + target_dir = tmp_path / symlink_scenario_2 / "test" + target_file_1 = target_dir / "test1" + target_file_2 = target_dir / "test2" + target_file_3 = target_dir / "test3" + target_symlink = target_dir / "test" + + ret = file.recurse( + name=str(target_dir), + source=f"salt://{target_dir.parent.name}/test", + clean=True, + keep_symlinks=True, + ) + assert ret.result is True + + assert target_dir.exists() + assert target_file_1.is_file() + assert target_file_2.is_file() + assert target_file_3.is_file() + assert target_symlink.is_symlink()