From 113e5de656ece1f33b210215a3a0a7e0db3c7ac8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Su=C3=A1rez=20Hern=C3=A1ndez?= Date: Tue, 3 May 2022 10:46:46 +0100 Subject: [PATCH 1/4] Trigger SaltInvocationError when targeting duplicated package names --- salt/modules/pkg_resource.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/salt/modules/pkg_resource.py b/salt/modules/pkg_resource.py index d51b5948cea9..0476160a202c 100644 --- a/salt/modules/pkg_resource.py +++ b/salt/modules/pkg_resource.py @@ -12,7 +12,7 @@ import salt.utils.data import salt.utils.versions import salt.utils.yaml -from salt.exceptions import SaltInvocationError +from salt.exceptions import CommandExecutionError, SaltInvocationError log = logging.getLogger(__name__) __SUFFIX_NOT_NEEDED = ("x86_64", "noarch") @@ -27,11 +27,22 @@ def _repack_pkgs(pkgs, normalize=True): _normalize_name = __salt__["pkg.normalize_name"] else: _normalize_name = lambda pkgname: pkgname - return { + + repacked_pkgs = { _normalize_name(str(x)): str(y) if y is not None else y for x, y in salt.utils.data.repack_dictlist(pkgs).items() } + # Check if there were collisions in names + if len(pkgs) != len(repacked_pkgs): + raise CommandExecutionError( + "You are passing a list of packages that contains duplicated packages names: {}. This cannot be processed. In case you are targeting different versions of the same package, please target them individually".format( + pkgs + ) + ) + + return repacked_pkgs + def pack_sources(sources, normalize=True): """ From 8001a568e0e85d2c0ce800e7af6ce5cf0259948d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Su=C3=A1rez=20Hern=C3=A1ndez?= Date: Tue, 3 May 2022 11:55:55 +0100 Subject: [PATCH 2/4] Add unit test for repack_pkgs function --- tests/pytests/unit/modules/test_pkg_resource.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/pytests/unit/modules/test_pkg_resource.py b/tests/pytests/unit/modules/test_pkg_resource.py index 8ca28e27840a..5d87773817ea 100644 --- a/tests/pytests/unit/modules/test_pkg_resource.py +++ b/tests/pytests/unit/modules/test_pkg_resource.py @@ -7,6 +7,7 @@ import salt.utils.data import salt.utils.yaml import yaml +from salt.exceptions import CommandExecutionError from tests.support.mock import MagicMock, patch @@ -223,6 +224,17 @@ def test_format_pkg_list_with_attr(): assert sorted(pkgs) == sorted(expected_pkg_list) +def test_repack_pkgs(): + """ + Test to check that repack function is raising error in case of + package name collisions + """ + assert pkg_resource._repack_pkgs([{"A": "a"}]) + assert pkg_resource._repack_pkgs([{"A": "a"}, {"B": "b"}]) + with pytest.raises(CommandExecutionError): + assert pkg_resource._repack_pkgs([{"A": "a"}, {"A": "c"}]) + + def test_stringify(): """ Test to takes a dict of package name/version information From 50ed402ea2b76895dded6a0b8ca3a356ba401b03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Su=C3=A1rez=20Hern=C3=A1ndez?= Date: Tue, 3 May 2022 12:50:30 +0100 Subject: [PATCH 3/4] Add changelog file --- changelog/62019.fixed | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/62019.fixed diff --git a/changelog/62019.fixed b/changelog/62019.fixed new file mode 100644 index 000000000000..8a772963e47d --- /dev/null +++ b/changelog/62019.fixed @@ -0,0 +1 @@ +Make Salt to return an error on "pkg" modules and states when targeting duplicated package names From 14f0cddb2d357cbd091db58ce305ff684725f8bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Su=C3=A1rez=20Hern=C3=A1ndez?= Date: Mon, 17 Oct 2022 16:34:10 +0100 Subject: [PATCH 4/4] Use SaltInvocationError as exception is pointing to invalid input --- salt/modules/pkg_resource.py | 4 ++-- tests/pytests/unit/modules/test_pkg_resource.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/salt/modules/pkg_resource.py b/salt/modules/pkg_resource.py index 0476160a202c..e91afbc20497 100644 --- a/salt/modules/pkg_resource.py +++ b/salt/modules/pkg_resource.py @@ -12,7 +12,7 @@ import salt.utils.data import salt.utils.versions import salt.utils.yaml -from salt.exceptions import CommandExecutionError, SaltInvocationError +from salt.exceptions import SaltInvocationError log = logging.getLogger(__name__) __SUFFIX_NOT_NEEDED = ("x86_64", "noarch") @@ -35,7 +35,7 @@ def _repack_pkgs(pkgs, normalize=True): # Check if there were collisions in names if len(pkgs) != len(repacked_pkgs): - raise CommandExecutionError( + raise SaltInvocationError( "You are passing a list of packages that contains duplicated packages names: {}. This cannot be processed. In case you are targeting different versions of the same package, please target them individually".format( pkgs ) diff --git a/tests/pytests/unit/modules/test_pkg_resource.py b/tests/pytests/unit/modules/test_pkg_resource.py index 5d87773817ea..de739bfc3e37 100644 --- a/tests/pytests/unit/modules/test_pkg_resource.py +++ b/tests/pytests/unit/modules/test_pkg_resource.py @@ -7,7 +7,7 @@ import salt.utils.data import salt.utils.yaml import yaml -from salt.exceptions import CommandExecutionError +from salt.exceptions import SaltInvocationError from tests.support.mock import MagicMock, patch @@ -231,7 +231,7 @@ def test_repack_pkgs(): """ assert pkg_resource._repack_pkgs([{"A": "a"}]) assert pkg_resource._repack_pkgs([{"A": "a"}, {"B": "b"}]) - with pytest.raises(CommandExecutionError): + with pytest.raises(SaltInvocationError): assert pkg_resource._repack_pkgs([{"A": "a"}, {"A": "c"}])