File fix-regression-multiple-values-for-keyword-argument-.patch of Package venv-salt-minion
From c25c8081ded775f3574b0bc999d809ce14701ba5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pablo=20Su=C3=A1rez=20Hern=C3=A1ndez?=
<psuarezhernandez@suse.com>
Date: Thu, 3 Aug 2023 10:07:28 +0100
Subject: [PATCH] Fix regression: multiple values for keyword argument
'saltenv' (bsc#1212844) (#590)
* fix passing wrong keyword arguments to cp.cache_file in pkg.installed with sources
* Drop `**kwargs` usage and be explicit about the supported keyword arguments.
Signed-off-by: Pedro Algarvio <palgarvio@vmware.com>
* Add regression test for https://github.com/saltstack/salt/issues/64118
Signed-off-by: Pedro Algarvio <palgarvio@vmware.com>
* Add changelog file
Signed-off-by: Pedro Algarvio <palgarvio@vmware.com>
---------
Signed-off-by: Pedro Algarvio <palgarvio@vmware.com>
Co-authored-by: Massimiliano Torromeo <massimiliano.torromeo@gmail.com>
Co-authored-by: Pedro Algarvio <palgarvio@vmware.com>
---
changelog/64118.fixed.md | 1 +
salt/modules/win_pkg.py | 25 +++++++-----
salt/states/pkg.py | 4 +-
tests/pytests/unit/modules/test_win_pkg.py | 2 +-
tests/pytests/unit/states/test_pkg.py | 46 +++++++++++++++++++---
5 files changed, 62 insertions(+), 16 deletions(-)
create mode 100644 changelog/64118.fixed.md
diff --git a/changelog/64118.fixed.md b/changelog/64118.fixed.md
new file mode 100644
index 0000000000..e7251827e9
--- /dev/null
+++ b/changelog/64118.fixed.md
@@ -0,0 +1 @@
+Stop passing `**kwargs` and be explicit about the keyword arguments to pass, namely, to `cp.cache_file` call in `salt.states.pkg`
diff --git a/salt/modules/win_pkg.py b/salt/modules/win_pkg.py
index 3aa7c7919a..e80dd19322 100644
--- a/salt/modules/win_pkg.py
+++ b/salt/modules/win_pkg.py
@@ -1298,7 +1298,7 @@ def _repo_process_pkg_sls(filename, short_path_name, ret, successful_verbose):
successful_verbose[short_path_name] = []
-def _get_source_sum(source_hash, file_path, saltenv, **kwargs):
+def _get_source_sum(source_hash, file_path, saltenv, verify_ssl=True):
"""
Extract the hash sum, whether it is in a remote hash file, or just a string.
"""
@@ -1315,7 +1315,7 @@ def _get_source_sum(source_hash, file_path, saltenv, **kwargs):
# The source_hash is a file on a server
try:
cached_hash_file = __salt__["cp.cache_file"](
- source_hash, saltenv, verify_ssl=kwargs.get("verify_ssl", True)
+ source_hash, saltenv=saltenv, verify_ssl=verify_ssl
)
except MinionError as exc:
log.exception("Failed to cache %s", source_hash, exc_info=exc)
@@ -1671,7 +1671,7 @@ def install(name=None, refresh=False, pkgs=None, **kwargs):
try:
cached_file = __salt__["cp.cache_file"](
cache_file,
- saltenv,
+ saltenv=saltenv,
verify_ssl=kwargs.get("verify_ssl", True),
)
except MinionError as exc:
@@ -1686,7 +1686,7 @@ def install(name=None, refresh=False, pkgs=None, **kwargs):
try:
cached_file = __salt__["cp.cache_file"](
cache_file,
- saltenv,
+ saltenv=saltenv,
verify_ssl=kwargs.get("verify_ssl", True),
)
except MinionError as exc:
@@ -1706,7 +1706,9 @@ def install(name=None, refresh=False, pkgs=None, **kwargs):
# It's not cached. Cache it, mate.
try:
cached_pkg = __salt__["cp.cache_file"](
- installer, saltenv, verify_ssl=kwargs.get("verify_ssl", True)
+ installer,
+ saltenv=saltenv,
+ verify_ssl=kwargs.get("verify_ssl", True),
)
except MinionError as exc:
msg = "Failed to cache {}".format(installer)
@@ -1730,7 +1732,7 @@ def install(name=None, refresh=False, pkgs=None, **kwargs):
try:
cached_pkg = __salt__["cp.cache_file"](
installer,
- saltenv,
+ saltenv=saltenv,
verify_ssl=kwargs.get("verify_ssl", True),
)
except MinionError as exc:
@@ -1754,7 +1756,12 @@ def install(name=None, refresh=False, pkgs=None, **kwargs):
# Compare the hash sums
source_hash = pkginfo[version_num].get("source_hash", False)
if source_hash:
- source_sum = _get_source_sum(source_hash, cached_pkg, saltenv, **kwargs)
+ source_sum = _get_source_sum(
+ source_hash,
+ cached_pkg,
+ saltenv=saltenv,
+ verify_ssl=kwargs.get("verify_ssl", True),
+ )
log.debug(
"pkg.install: Source %s hash: %s",
source_sum["hash_type"],
@@ -2126,7 +2133,7 @@ def remove(name=None, pkgs=None, **kwargs):
try:
cached_pkg = __salt__["cp.cache_file"](
uninstaller,
- saltenv,
+ saltenv=saltenv,
verify_ssl=kwargs.get("verify_ssl", True),
)
except MinionError as exc:
@@ -2150,7 +2157,7 @@ def remove(name=None, pkgs=None, **kwargs):
try:
cached_pkg = __salt__["cp.cache_file"](
uninstaller,
- saltenv,
+ saltenv=saltenv,
verify_ssl=kwargs.get("verify_ssl", True),
)
except MinionError as exc:
diff --git a/salt/states/pkg.py b/salt/states/pkg.py
index 12fbc87a1a..a605b23107 100644
--- a/salt/states/pkg.py
+++ b/salt/states/pkg.py
@@ -760,7 +760,9 @@ def _find_install_targets(
err = "Unable to cache {0}: {1}"
try:
cached_path = __salt__["cp.cache_file"](
- version_string, saltenv=kwargs["saltenv"], **kwargs
+ version_string,
+ saltenv=kwargs["saltenv"],
+ verify_ssl=kwargs.get("verify_ssl", True),
)
except CommandExecutionError as exc:
problems.append(err.format(version_string, exc))
diff --git a/tests/pytests/unit/modules/test_win_pkg.py b/tests/pytests/unit/modules/test_win_pkg.py
index 76234fb77e..6d435f00a5 100644
--- a/tests/pytests/unit/modules/test_win_pkg.py
+++ b/tests/pytests/unit/modules/test_win_pkg.py
@@ -262,7 +262,7 @@ def test_pkg_install_verify_ssl_false():
result = win_pkg.install(name="nsis", version="3.02", verify_ssl=False)
mock_cp.assert_called_once_with(
"http://download.sourceforge.net/project/nsis/NSIS%203/3.02/nsis-3.02-setup.exe",
- "base",
+ saltenv="base",
verify_ssl=False,
)
assert expected == result
diff --git a/tests/pytests/unit/states/test_pkg.py b/tests/pytests/unit/states/test_pkg.py
index b852f27b00..f58be11011 100644
--- a/tests/pytests/unit/states/test_pkg.py
+++ b/tests/pytests/unit/states/test_pkg.py
@@ -3,6 +3,7 @@ import logging
import pytest
import salt.modules.beacons as beaconmod
+import salt.modules.cp as cp
import salt.modules.pkg_resource as pkg_resource
import salt.modules.yumpkg as yumpkg
import salt.states.beacon as beaconstate
@@ -15,19 +16,28 @@ log = logging.getLogger(__name__)
@pytest.fixture
-def configure_loader_modules():
+def configure_loader_modules(minion_opts):
return {
+ cp: {
+ "__opts__": minion_opts,
+ },
pkg: {
"__env__": "base",
"__salt__": {},
"__grains__": {"os": "CentOS", "os_family": "RedHat"},
- "__opts__": {"test": False, "cachedir": ""},
+ "__opts__": minion_opts,
"__instance_id__": "",
"__low__": {},
"__utils__": {"state.gen_tag": state_utils.gen_tag},
},
- beaconstate: {"__salt__": {}, "__opts__": {}},
- beaconmod: {"__salt__": {}, "__opts__": {}},
+ beaconstate: {
+ "__salt__": {},
+ "__opts__": minion_opts,
+ },
+ beaconmod: {
+ "__salt__": {},
+ "__opts__": minion_opts,
+ },
pkg_resource: {
"__salt__": {},
"__grains__": {"os": "CentOS", "os_family": "RedHat"},
@@ -35,7 +45,7 @@ def configure_loader_modules():
yumpkg: {
"__salt__": {},
"__grains__": {"osarch": "x86_64", "osmajorrelease": 7},
- "__opts__": {},
+ "__opts__": minion_opts,
},
}
@@ -563,6 +573,32 @@ def test_installed_with_changes_test_true(list_pkgs):
assert ret["changes"] == expected
+def test_installed_with_sources(list_pkgs, tmp_path):
+ """
+ Test pkg.installed with passing `sources`
+ """
+
+ list_pkgs = MagicMock(return_value=list_pkgs)
+ pkg_source = tmp_path / "pkga-package-0.3.0.deb"
+
+ with patch.dict(
+ pkg.__salt__,
+ {
+ "cp.cache_file": cp.cache_file,
+ "pkg.list_pkgs": list_pkgs,
+ "pkg_resource.pack_sources": pkg_resource.pack_sources,
+ "lowpkg.bin_pkg_info": MagicMock(),
+ },
+ ), patch("salt.fileclient.get_file_client", return_value=MagicMock()):
+ try:
+ ret = pkg.installed("install-pkgd", sources=[{"pkga": str(pkg_source)}])
+ assert ret["result"] is False
+ except TypeError as exc:
+ if "got multiple values for keyword argument 'saltenv'" in str(exc):
+ pytest.fail(f"TypeError should have not been raised: {exc}")
+ raise exc from None
+
+
@pytest.mark.parametrize("action", ["removed", "purged"])
def test_removed_purged_with_changes_test_true(list_pkgs, action):
"""
--
2.41.0
