rpm/salt
SHA256
1
0
Fork 0
forked from pool/salt
Code Pull Requests Activity

Accepting request 1141015 from home:PSuarezHernandez:branches:systemsmanagement:saltstack

Browse Source 
- Prevent exceptions with fileserver.update when called via state (bsc#1218482)
- Added:
  * allow-kwargs-for-fileserver-roots-update-bsc-1218482.patch

- Improve pip target override condition with VENV_PIP_TARGET
  environment variable (bsc#1216850)
- Fixed KeyError in logs when running a state that fails
- Added:
  * improve-pip-target-override-condition-with-venv_pip_.patch
  * fixed-keyerror-in-logs-when-running-a-state-that-fai.patch

OBS-URL: https://build.opensuse.org/request/show/1141015
OBS-URL: https://build.opensuse.org/package/show/systemsmanagement:saltstack/salt?expand=0&rev=226
This commit is contained in:
Victor Zhestkov 2024-01-26 09:29:32 +00:00 committed by Git OBS Bridge
parent 323feae962
commit 1bf701f25a
6 changed files with 424 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
_lastrevision
View File

@ -1 +1 @@
ee71f172008f6c3aa9d80a26ee15f873ca17f471
d02a7d8eb7fdb302ed603f2c45525b41e812beb7

164
allow-kwargs-for-fileserver-roots-update-bsc-1218482.patch Normal file
View File

@ -0,0 +1,164 @@
From 8ae54e8a0e12193507f1936f363c3438b4a006ee Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Yeray=20Guti=C3=A9rrez=20Cedr=C3=A9s?=
<yeray.gutierrez@suse.com>
Date: Tue, 23 Jan 2024 15:33:28 +0000
Subject: [PATCH] Allow kwargs for fileserver roots update
(bsc#1218482) (#618)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* Allow kwargs for fileserver roots update (bsc#1218482)
* Prevent exceptions with fileserver.update when called via state
* Fix wrong logic and enhance tests around fileserver.update
* Remove test which is not longer valid
---------
Co-authored-by: Pablo Suárez Hernández <psuarezhernandez@suse.com>
---
changelog/65819.fixed.md | 1 +
salt/fileserver/roots.py | 8 ++--
salt/runners/fileserver.py | 6 +++
tests/integration/runners/test_fileserver.py | 40 ++++++++++++++++++--
tests/pytests/unit/fileserver/test_roots.py | 2 +-
5 files changed, 47 insertions(+), 10 deletions(-)
create mode 100644 changelog/65819.fixed.md
diff --git a/changelog/65819.fixed.md b/changelog/65819.fixed.md
new file mode 100644
index 0000000000..432f5c791c
--- /dev/null
+++ b/changelog/65819.fixed.md
@@ -0,0 +1 @@
+Prevent exceptions with fileserver.update when called via state
diff --git a/salt/fileserver/roots.py b/salt/fileserver/roots.py
index 4880cbab9b..a02b597c6f 100644
--- a/salt/fileserver/roots.py
+++ b/salt/fileserver/roots.py
@@ -193,9 +193,7 @@ def update():
os.makedirs(mtime_map_path_dir)
with salt.utils.files.fopen(mtime_map_path, "wb") as fp_:
for file_path, mtime in new_mtime_map.items():
- fp_.write(
- salt.utils.stringutils.to_bytes("{}:{}\n".format(file_path, mtime))
- )
+ fp_.write(salt.utils.stringutils.to_bytes(f"{file_path}:{mtime}\n"))
if __opts__.get("fileserver_events", False):
# if there is a change, fire an event
@@ -326,11 +324,11 @@ def _file_lists(load, form):
return []
list_cache = os.path.join(
list_cachedir,
- "{}.p".format(salt.utils.files.safe_filename_leaf(actual_saltenv)),
+ f"{salt.utils.files.safe_filename_leaf(actual_saltenv)}.p",
)
w_lock = os.path.join(
list_cachedir,
- ".{}.w".format(salt.utils.files.safe_filename_leaf(actual_saltenv)),
+ f".{salt.utils.files.safe_filename_leaf(actual_saltenv)}.w",
)
cache_match, refresh_cache, save_cache = salt.fileserver.check_file_list_cache(
__opts__, form, list_cache, w_lock
diff --git a/salt/runners/fileserver.py b/salt/runners/fileserver.py
index d75d7de0cf..1ed05b68ca 100644
--- a/salt/runners/fileserver.py
+++ b/salt/runners/fileserver.py
@@ -350,6 +350,12 @@ def update(backend=None, **kwargs):
salt-run fileserver.update backend=git remotes=myrepo,yourrepo
"""
fileserver = salt.fileserver.Fileserver(__opts__)
+
+ # Remove possible '__pub_user' in kwargs as it is not expected
+ # on "update" function for the different fileserver backends.
+ if "__pub_user" in kwargs:
+ del kwargs["__pub_user"]
+
fileserver.update(back=backend, **kwargs)
return True
diff --git a/tests/integration/runners/test_fileserver.py b/tests/integration/runners/test_fileserver.py
index ae8ab766aa..62f0da0c4a 100644
--- a/tests/integration/runners/test_fileserver.py
+++ b/tests/integration/runners/test_fileserver.py
@@ -202,15 +202,31 @@ class FileserverTest(ShellCase):
fileserver.update
"""
ret = self.run_run_plus(fun="fileserver.update")
- self.assertTrue(ret["return"])
+ self.assertTrue(ret["return"] is True)
# Backend submitted as a string
ret = self.run_run_plus(fun="fileserver.update", backend="roots")
- self.assertTrue(ret["return"])
+ self.assertTrue(ret["return"] is True)
# Backend submitted as a list
ret = self.run_run_plus(fun="fileserver.update", backend=["roots"])
- self.assertTrue(ret["return"])
+ self.assertTrue(ret["return"] is True)
+
+ # Possible '__pub_user' is removed from kwargs
+ ret = self.run_run_plus(
+ fun="fileserver.update", backend=["roots"], __pub_user="foo"
+ )
+ self.assertTrue(ret["return"] is True)
+
+ # Unknown arguments
+ ret = self.run_run_plus(
+ fun="fileserver.update", backend=["roots"], unknown_arg="foo"
+ )
+ self.assertIn(
+ "Passed invalid arguments: update() got an unexpected keyword argument"
+ " 'unknown_arg'",
+ ret["return"],
+ )
# Other arguments are passed to backend
def mock_gitfs_update(remotes=None):
@@ -225,7 +241,23 @@ class FileserverTest(ShellCase):
ret = self.run_run_plus(
fun="fileserver.update", backend="gitfs", remotes="myrepo,yourrepo"
)
- self.assertTrue(ret["return"])
+ self.assertTrue(ret["return"] is True)
+ mock_backend_func.assert_called_once_with(remotes="myrepo,yourrepo")
+
+ # Possible '__pub_user' arguments are removed from kwargs
+ mock_backend_func = create_autospec(mock_gitfs_update)
+ mock_return_value = {
+ "gitfs.envs": None, # This is needed to activate the backend
+ "gitfs.update": mock_backend_func,
+ }
+ with patch("salt.loader.fileserver", MagicMock(return_value=mock_return_value)):
+ ret = self.run_run_plus(
+ fun="fileserver.update",
+ backend="gitfs",
+ remotes="myrepo,yourrepo",
+ __pub_user="foo",
+ )
+ self.assertTrue(ret["return"] is True)
mock_backend_func.assert_called_once_with(remotes="myrepo,yourrepo")
# Unknown arguments are passed to backend
diff --git a/tests/pytests/unit/fileserver/test_roots.py b/tests/pytests/unit/fileserver/test_roots.py
index a8a80eea17..96bceb0fd3 100644
--- a/tests/pytests/unit/fileserver/test_roots.py
+++ b/tests/pytests/unit/fileserver/test_roots.py
@@ -236,7 +236,7 @@ def test_update_mtime_map():
# between Python releases.
lines_written = sorted(mtime_map_mock.write_calls())
expected = sorted(
- salt.utils.stringutils.to_bytes("{key}:{val}\n".format(key=key, val=val))
+ salt.utils.stringutils.to_bytes(f"{key}:{val}\n")
for key, val in new_mtime_map.items()
)
assert lines_written == expected, lines_written
--
2.43.0

121
fixed-keyerror-in-logs-when-running-a-state-that-fai.patch Normal file
View File

@ -0,0 +1,121 @@
From f41a8e2a142a8487e13af481990928e0afb5f15e Mon Sep 17 00:00:00 2001
From: Victor Zhestkov <vzhestkov@suse.com>
Date: Thu, 18 Jan 2024 17:02:03 +0100
Subject: [PATCH] Fixed KeyError in logs when running a state that
fails. (#615)
Co-authored-by: Megan Wilhite <mwilhite@vmware.com>
---
changelog/64231.fixed.md | 1 +
salt/master.py | 2 +-
salt/minion.py | 4 ++
salt/utils/event.py | 3 +-
.../integration/states/test_state_test.py | 38 +++++++++++++++++++
5 files changed, 46 insertions(+), 2 deletions(-)
create mode 100644 changelog/64231.fixed.md
create mode 100644 tests/pytests/integration/states/test_state_test.py
diff --git a/changelog/64231.fixed.md b/changelog/64231.fixed.md
new file mode 100644
index 0000000000..0991c5a8b9
--- /dev/null
+++ b/changelog/64231.fixed.md
@@ -0,0 +1 @@
+Fixed KeyError in logs when running a state that fails.
diff --git a/salt/master.py b/salt/master.py
index fc243ef674..3d2ba1e29d 100644
--- a/salt/master.py
+++ b/salt/master.py
@@ -1790,7 +1790,7 @@ class AESFuncs(TransportMethods):
def pub_ret(self, load):
"""
Request the return data from a specific jid, only allowed
- if the requesting minion also initialted the execution.
+ if the requesting minion also initiated the execution.
:param dict load: The minion payload
diff --git a/salt/minion.py b/salt/minion.py
index 4db0d31bd4..2ccd0cd5a9 100644
--- a/salt/minion.py
+++ b/salt/minion.py
@@ -2022,6 +2022,8 @@ class Minion(MinionBase):
ret["jid"] = data["jid"]
ret["fun"] = data["fun"]
ret["fun_args"] = data["arg"]
+ if "user" in data:
+ ret["user"] = data["user"]
if "master_id" in data:
ret["master_id"] = data["master_id"]
if "metadata" in data:
@@ -2141,6 +2143,8 @@ class Minion(MinionBase):
ret["jid"] = data["jid"]
ret["fun"] = data["fun"]
ret["fun_args"] = data["arg"]
+ if "user" in data:
+ ret["user"] = data["user"]
if "metadata" in data:
ret["metadata"] = data["metadata"]
if minion_instance.connected:
diff --git a/salt/utils/event.py b/salt/utils/event.py
index 869e12a140..e6d7b00520 100644
--- a/salt/utils/event.py
+++ b/salt/utils/event.py
@@ -902,7 +902,8 @@ class SaltEvent:
data["success"] = False
data["return"] = "Error: {}.{}".format(tags[0], tags[-1])
data["fun"] = fun
- data["user"] = load["user"]
+ if "user" in load:
+ data["user"] = load["user"]
self.fire_event(
data,
tagify([load["jid"], "sub", load["id"], "error", fun], "job"),
diff --git a/tests/pytests/integration/states/test_state_test.py b/tests/pytests/integration/states/test_state_test.py
new file mode 100644
index 0000000000..b2328a4c2b
--- /dev/null
+++ b/tests/pytests/integration/states/test_state_test.py
@@ -0,0 +1,38 @@
+def test_failing_sls(salt_master, salt_minion, salt_cli, caplog):
+ """
+ Test when running state.sls and the state fails.
+ When the master stores the job and attempts to send
+ an event a KeyError was previously being logged.
+ This test ensures we do not log an error when
+ attempting to send an event about a failing state.
+ """
+ statesls = """
+ test_state:
+ test.fail_without_changes:
+ - name: "bla"
+ """
+ with salt_master.state_tree.base.temp_file("test_failure.sls", statesls):
+ ret = salt_cli.run("state.sls", "test_failure", minion_tgt=salt_minion.id)
+ for message in caplog.messages:
+ assert "Event iteration failed with" not in message
+
+
+def test_failing_sls_compound(salt_master, salt_minion, salt_cli, caplog):
+ """
+ Test when running state.sls in a compound command and the state fails.
+ When the master stores the job and attempts to send
+ an event a KeyError was previously being logged.
+ This test ensures we do not log an error when
+ attempting to send an event about a failing state.
+ """
+ statesls = """
+ test_state:
+ test.fail_without_changes:
+ - name: "bla"
+ """
+ with salt_master.state_tree.base.temp_file("test_failure.sls", statesls):
+ ret = salt_cli.run(
+ "state.sls,cmd.run", "test_failure,ls", minion_tgt=salt_minion.id
+ )
+ for message in caplog.messages:
+ assert "Event iteration failed with" not in message
--
2.43.0

113
improve-pip-target-override-condition-with-venv_pip_.patch Normal file
View File

@ -0,0 +1,113 @@
From da938aa8a572138b5b9b1535c5c3d69326e5194e Mon Sep 17 00:00:00 2001
From: Victor Zhestkov <vzhestkov@suse.com>
Date: Thu, 18 Jan 2024 17:02:23 +0100
Subject: [PATCH] Improve pip target override condition with
VENV_PIP_TARGET environment variable (bsc#1216850) (#613)
* Improve pip target override condition
* Improve pip test with different condition of overriding the target
* Add changelog entry
---
changelog/65562.fixed.md | 1 +
salt/modules/pip.py | 6 ++--
tests/pytests/unit/modules/test_pip.py | 50 +++++++++++++++++---------
3 files changed, 38 insertions(+), 19 deletions(-)
create mode 100644 changelog/65562.fixed.md
diff --git a/changelog/65562.fixed.md b/changelog/65562.fixed.md
new file mode 100644
index 0000000000..ba483b4b77
--- /dev/null
+++ b/changelog/65562.fixed.md
@@ -0,0 +1 @@
+Improve the condition of overriding target for pip with VENV_PIP_TARGET environment variable.
diff --git a/salt/modules/pip.py b/salt/modules/pip.py
index a60bdca0bb..68a2a442a1 100644
--- a/salt/modules/pip.py
+++ b/salt/modules/pip.py
@@ -857,9 +857,11 @@ def install(
cmd.extend(["--build", build])
# Use VENV_PIP_TARGET environment variable value as target
- # if set and no target specified on the function call
+ # if set and no target specified on the function call.
+ # Do not set target if bin_env specified, use default
+ # for specified binary environment or expect explicit target specification.
target_env = os.environ.get("VENV_PIP_TARGET", None)
- if target is None and target_env is not None:
+ if target is None and target_env is not None and bin_env is None:
target = target_env
if target:
diff --git a/tests/pytests/unit/modules/test_pip.py b/tests/pytests/unit/modules/test_pip.py
index b7ad1ea3fd..c03e6ed292 100644
--- a/tests/pytests/unit/modules/test_pip.py
+++ b/tests/pytests/unit/modules/test_pip.py
@@ -1738,28 +1738,44 @@ def test_when_version_is_called_with_a_user_it_should_be_passed_to_undelying_run
)
-def test_install_target_from_VENV_PIP_TARGET_in_resulting_command(python_binary):
+@pytest.mark.parametrize(
+ "bin_env,target,target_env,expected_target",
+ [
+ (None, None, None, None),
+ (None, "/tmp/foo", None, "/tmp/foo"),
+ (None, None, "/tmp/bar", "/tmp/bar"),
+ (None, "/tmp/foo", "/tmp/bar", "/tmp/foo"),
+ ("/tmp/venv", "/tmp/foo", None, "/tmp/foo"),
+ ("/tmp/venv", None, "/tmp/bar", None),
+ ("/tmp/venv", "/tmp/foo", "/tmp/bar", "/tmp/foo"),
+ ],
+)
+def test_install_target_from_VENV_PIP_TARGET_in_resulting_command(
+ python_binary, bin_env, target, target_env, expected_target
+):
pkg = "pep8"
- target = "/tmp/foo"
- target_env = "/tmp/bar"
mock = MagicMock(return_value={"retcode": 0, "stdout": ""})
environment = os.environ.copy()
- environment["VENV_PIP_TARGET"] = target_env
+ real_get_pip_bin = pip._get_pip_bin
+
+ def mock_get_pip_bin(bin_env):
+ if not bin_env:
+ return real_get_pip_bin(bin_env)
+ return [f"{bin_env}/bin/pip"]
+
+ if target_env is not None:
+ environment["VENV_PIP_TARGET"] = target_env
with patch.dict(pip.__salt__, {"cmd.run_all": mock}), patch.object(
os, "environ", environment
- ):
- pip.install(pkg)
- expected = [*python_binary, "install", "--target", target_env, pkg]
- mock.assert_called_with(
- expected,
- saltenv="base",
- runas=None,
- use_vt=False,
- python_shell=False,
- )
- mock.reset_mock()
- pip.install(pkg, target=target)
- expected = [*python_binary, "install", "--target", target, pkg]
+ ), patch.object(pip, "_get_pip_bin", mock_get_pip_bin):
+ pip.install(pkg, bin_env=bin_env, target=target)
+ expected_binary = python_binary
+ if bin_env is not None:
+ expected_binary = [f"{bin_env}/bin/pip"]
+ if expected_target is not None:
+ expected = [*expected_binary, "install", "--target", expected_target, pkg]
+ else:
+ expected = [*expected_binary, "install", pkg]
mock.assert_called_with(
expected,
saltenv="base",
--
2.43.0

19
salt.changes
View File

@ -1,3 +1,22 @@
-------------------------------------------------------------------
Tue Jan 23 15:39:14 UTC 2024 - Pablo Suárez Hernández <pablo.suarezhernandez@suse.com>
- Prevent exceptions with fileserver.update when called via state (bsc#1218482)
- Added:
* allow-kwargs-for-fileserver-roots-update-bsc-1218482.patch
-------------------------------------------------------------------
Thu Jan 18 16:10:04 UTC 2024 - Pablo Suárez Hernández <pablo.suarezhernandez@suse.com>
- Improve pip target override condition with VENV_PIP_TARGET
environment variable (bsc#1216850)
- Fixed KeyError in logs when running a state that fails
- Added:
* improve-pip-target-override-condition-with-venv_pip_.patch
* fixed-keyerror-in-logs-when-running-a-state-that-fai.patch
-------------------------------------------------------------------
Thu Jan 18 13:06:21 UTC 2024 - Marek Czernek <marek.czernek@suse.com>

6
salt.spec
View File

@ -335,6 +335,12 @@ Patch93: prefer-unittest.mock-for-python-versions-that-are-su.patch
Patch94: fix-the-aptpkg.py-unit-test-failure.patch
# PATCH-FIX_UPSTREAM https://github.com/saltstack/salt/pull/65092
Patch95: update-__pillar__-during-pillar_refresh.patch
# PATCH-FIX_UPSTREAM https://github.com/saltstack/salt/pull/65009
Patch96: fixed-keyerror-in-logs-when-running-a-state-that-fai.patch
# PATCH-FIX_UPSTREAM https://github.com/saltstack/salt/pull/65562
Patch97: improve-pip-target-override-condition-with-venv_pip_.patch
# PATCH-FIX_UPSTREAM https://github.com/saltstack/salt/pull/65819
Patch98: allow-kwargs-for-fileserver-roots-update-bsc-1218482.patch
### IMPORTANT: The line below is used as a snippet marker. Do not touch it.