SHA256
1
0
forked from pool/salt
salt/fix-salt.states.file.managed-for-follow_symlinks-tru.patch

309 lines
8.7 KiB
Diff

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