File fix-salt.states.file.managed-for-follow_symlink... of Package salt

Overview Repositories Revisions Requests Users Attributes Meta

File fix-salt.states.file.managed-for-follow_symlinks-tru.patch of Package salt (Revision 83ab7d4e35f03aafd4bde7c8d3c292f4)

Currently displaying revision 83ab7d4e35f03aafd4bde7c8d3c292f4 , Show latest

From 10705d922a11e5f2654d26e83e9f302862fafb18 Mon Sep 17 00:00:00 2001
From: Petr Pavlu <31453820+petrpavlu@users.noreply.github.com>
Date: Fri, 8 Jul 2022 10:11:52 +0200
Subject: [PATCH] Fix salt.states.file.managed() for
 follow_symlinks=True and test=True (bsc#1199372) (#535)

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.

[Cherry-picked from upstream commit
95bfbe31a2dc54723af3f1783d40de152760fe1a.]
---
 changelog/62066.fixed                         |   1 +
 salt/modules/file.py                          |  27 +++-
 salt/states/file.py                           |   1 +
 .../unit/modules/file/test_file_check.py      | 144 ++++++++++++++++++
 4 files changed, 172 insertions(+), 1 deletion(-)
 create mode 100644 changelog/62066.fixed
 create mode 100644 tests/pytests/unit/modules/file/test_file_check.py

diff --git a/changelog/62066.fixed b/changelog/62066.fixed
new file mode 100644
index 0000000000..68216a03c1
--- /dev/null
+++ b/changelog/62066.fixed
@@ -0,0 +1 @@
+Fixed salt.states.file.managed() for follow_symlinks=True and test=True
diff --git a/salt/modules/file.py b/salt/modules/file.py
index 73619064ef..40c07455e3 100644
--- a/salt/modules/file.py
+++ b/salt/modules/file.py
@@ -5281,11 +5281,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
@@ -5336,6 +5343,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()
@@ -5372,6 +5380,7 @@ def check_managed_changes(
     setype=None,
     serange=None,
     verify_ssl=True,
+    follow_symlinks=False,
     **kwargs
 ):
     """
@@ -5387,6 +5396,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
@@ -5456,6 +5471,7 @@ def check_managed_changes(
         serole=serole,
         setype=setype,
         serange=serange,
+        follow_symlinks=follow_symlinks,
     )
     __clean_tmp(sfn)
     return changes
@@ -5477,6 +5493,7 @@ def check_file_meta(
     setype=None,
     serange=None,
     verify_ssl=True,
+    follow_symlinks=False,
 ):
     """
     Check for the changes in the file metadata.
@@ -5553,6 +5570,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:
@@ -5560,7 +5583,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 = {}
diff --git a/salt/states/file.py b/salt/states/file.py
index 54e7decf86..a6288025e5 100644
--- a/salt/states/file.py
+++ b/salt/states/file.py
@@ -3038,6 +3038,7 @@ def managed(
                     setype=setype,
                     serange=serange,
                     verify_ssl=verify_ssl,
+                    follow_symlinks=follow_symlinks,
                     **kwargs
                 )
 
diff --git a/tests/pytests/unit/modules/file/test_file_check.py b/tests/pytests/unit/modules/file/test_file_check.py
new file mode 100644
index 0000000000..bd0379ddae
--- /dev/null
+++ b/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 == {}
-- 
2.36.1

Places

File fix-salt.states.file.managed-for-follow_symlinks-tru.patch of Package salt (Revision 83ab7d4e35f03aafd4bde7c8d3c292f4)

Places