Make sure symlinks are done last

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".

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

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()