Fix salt.states.file.managed() for follow_symlinks=True and test=True

When managing file /etc/test as follows:
> file /etc/test:
>   file.managed:
>     - name: /etc/test
>     - source: salt://config/test
>     - mode: 644
>     - follow_symlinks: True

and with /etc/test being a symlink to a different file, an invocation of
"salt-call '*' state.apply test=True" can report that the file should be
updated even when a subsequent run of the same command without the test
parameter makes no changes.

The problem is that the test code path doesn't take correctly into
account the follow_symlinks=True setting and ends up comparing
permissions of the symlink instead of its target file.

The patch addresses the problem by extending functions
salt.modules.file.check_managed(), check_managed_changes() and
check_file_meta() to have the follow_symlinks parameter which gets
propagated to the salt.modules.file.stats() call and by updating
salt.states.file.managed() to forward the same parameter to
salt.modules.file.check_managed_changes().

Fixes #62066.

changelog/62066.fixed

@@ -0,0 +1 @@
+Fixed salt.states.file.managed() for follow_symlinks=True and test=True

salt/modules/file.py

@@ -5335,11 +5335,18 @@ def check_managed(
     serole=None,
     setype=None,
     serange=None,
+    follow_symlinks=False,
     **kwargs
 ):
     """
     Check to see what changes need to be made for a file
 
+    follow_symlinks
+        If the desired path is a symlink, follow it and check the permissions
+        of the file to which the symlink points.
+
+        .. versionadded:: 3005
+
     CLI Example:
 
     .. code-block:: bash
@@ -5390,6 +5397,7 @@ def check_managed(
         serole=serole,
         setype=setype,
         serange=serange,
+        follow_symlinks=follow_symlinks,
     )
     # Ignore permission for files written temporary directories
     # Files in any path will still be set correctly using get_managed()
@@ -5426,6 +5434,7 @@ def check_managed_changes(
     setype=None,
     serange=None,
     verify_ssl=True,
+    follow_symlinks=False,
     **kwargs
 ):
     """
@@ -5441,6 +5450,12 @@ def check_managed_changes(
 
         .. versionadded:: 3002
 
+    follow_symlinks
+        If the desired path is a symlink, follow it and check the permissions
+        of the file to which the symlink points.
+
+        .. versionadded:: 3005
+
     CLI Example:
 
     .. code-block:: bash
@@ -5510,6 +5525,7 @@ def check_managed_changes(
         serole=serole,
         setype=setype,
         serange=serange,
+        follow_symlinks=follow_symlinks,
     )
     __clean_tmp(sfn)
     return changes
@@ -5531,6 +5547,7 @@ def check_file_meta(
     setype=None,
     serange=None,
     verify_ssl=True,
+    follow_symlinks=False,
 ):
     """
     Check for the changes in the file metadata.
@@ -5607,6 +5624,12 @@ def check_file_meta(
         will not attempt to validate the servers certificate. Default is True.
 
         .. versionadded:: 3002
+
+    follow_symlinks
+        If the desired path is a symlink, follow it and check the permissions
+        of the file to which the symlink points.
+
+        .. versionadded:: 3005
     """
     changes = {}
     if not source_sum:
@@ -5614,7 +5637,9 @@ def check_file_meta(
 
     try:
         lstats = stats(
-            name, hash_type=source_sum.get("hash_type", None), follow_symlinks=False
+            name,
+            hash_type=source_sum.get("hash_type", None),
+            follow_symlinks=follow_symlinks,
         )
     except CommandExecutionError:
         lstats = {}

salt/states/file.py

@@ -3118,6 +3118,7 @@ def managed(
                     setype=setype,
                     serange=serange,
                     verify_ssl=verify_ssl,
+                    follow_symlinks=follow_symlinks,
                     **kwargs
                 )
 

tests/pytests/unit/modules/file/test_file_check.py

@@ -0,0 +1,144 @@
+import getpass
+import logging
+import os
+
+import pytest
+import salt.modules.file as filemod
+import salt.utils.files
+import salt.utils.platform
+
+log = logging.getLogger(__name__)
+
+
+@pytest.fixture
+def configure_loader_modules():
+    return {filemod: {"__context__": {}}}
+
+
+@pytest.fixture
+def tfile(tmp_path):
+    filename = str(tmp_path / "file-check-test-file")
+
+    with salt.utils.files.fopen(filename, "w") as fp:
+        fp.write("Hi hello! I am a file.")
+    os.chmod(filename, 0o644)
+
+    yield filename
+
+    os.remove(filename)
+
+
+@pytest.fixture
+def a_link(tmp_path, tfile):
+    linkname = str(tmp_path / "a_link")
+    os.symlink(tfile, linkname)
+
+    yield linkname
+
+    os.remove(linkname)
+
+
+def get_link_perms():
+    if salt.utils.platform.is_linux():
+        return "0777"
+    return "0755"
+
+
+@pytest.mark.skip_on_windows(reason="os.symlink is not available on Windows")
+def test_check_file_meta_follow_symlinks(a_link, tfile):
+    user = getpass.getuser()
+    lperms = get_link_perms()
+
+    # follow_symlinks=False (default)
+    ret = filemod.check_file_meta(
+        a_link, tfile, None, None, user, None, lperms, None, None
+    )
+    assert ret == {}
+
+    ret = filemod.check_file_meta(
+        a_link, tfile, None, None, user, None, "0644", None, None
+    )
+    assert ret == {"mode": "0644"}
+
+    # follow_symlinks=True
+    ret = filemod.check_file_meta(
+        a_link, tfile, None, None, user, None, "0644", None, None, follow_symlinks=True
+    )
+    assert ret == {}
+
+
+@pytest.mark.skip_on_windows(reason="os.symlink is not available on Windows")
+def test_check_managed_follow_symlinks(a_link, tfile):
+    user = getpass.getuser()
+    lperms = get_link_perms()
+
+    # Function check_managed() ignores mode changes for files in the temp directory.
+    # Trick it to not recognize a_link as such.
+    a_link = "/" + a_link
+
+    # follow_symlinks=False (default)
+    ret, comments = filemod.check_managed(
+        a_link, tfile, None, None, user, None, lperms, None, None, None, None, None
+    )
+    assert ret is True
+    assert comments == "The file {} is in the correct state".format(a_link)
+
+    ret, comments = filemod.check_managed(
+        a_link, tfile, None, None, user, None, "0644", None, None, None, None, None
+    )
+    assert ret is None
+    assert comments == "The following values are set to be changed:\nmode: 0644\n"
+
+    # follow_symlinks=True
+    ret, comments = filemod.check_managed(
+        a_link,
+        tfile,
+        None,
+        None,
+        user,
+        None,
+        "0644",
+        None,
+        None,
+        None,
+        None,
+        None,
+        follow_symlinks=True,
+    )
+    assert ret is True
+    assert comments == "The file {} is in the correct state".format(a_link)
+
+
+@pytest.mark.skip_on_windows(reason="os.symlink is not available on Windows")
+def test_check_managed_changes_follow_symlinks(a_link, tfile):
+    user = getpass.getuser()
+    lperms = get_link_perms()
+
+    # follow_symlinks=False (default)
+    ret = filemod.check_managed_changes(
+        a_link, tfile, None, None, user, None, lperms, None, None, None, None, None
+    )
+    assert ret == {}
+
+    ret = filemod.check_managed_changes(
+        a_link, tfile, None, None, user, None, "0644", None, None, None, None, None
+    )
+    assert ret == {"mode": "0644"}
+
+    # follow_symlinks=True
+    ret = filemod.check_managed_changes(
+        a_link,
+        tfile,
+        None,
+        None,
+        user,
+        None,
+        "0644",
+        None,
+        None,
+        None,
+        None,
+        None,
+        follow_symlinks=True,
+    )
+    assert ret == {}