From 7021d7e7583edc952d85dd444dd8758c9e25a199 Mon Sep 17 00:00:00 2001 From: "tobias.goergens" Date: Wed, 15 Apr 2026 13:46:04 +0200 Subject: [PATCH] Preserve multiarch apt package names in pkg.installed Honor split_arch in aptpkg target parsing, keep arch-qualified package names through the pkg.installed preflight path, and normalize installed package lookups during final verification so held multiarch packages can be updated correctly with update_holds=True. Add regression tests and document the provider-specific naming caveat for held/unheld packages. --- changelog/68932.fixed.md | 1 + salt/modules/aptpkg.py | 32 ++++++- salt/states/pkg.py | 26 +++-- tests/pytests/unit/modules/test_aptpkg.py | 34 +++++++ tests/pytests/unit/states/test_pkg.py | 111 +++++++++++++++++++++- 5 files changed, 195 insertions(+), 9 deletions(-) create mode 100644 changelog/68932.fixed.md diff --git a/changelog/68932.fixed.md b/changelog/68932.fixed.md new file mode 100644 index 000000000000..773870ebe56b --- /dev/null +++ b/changelog/68932.fixed.md @@ -0,0 +1 @@ +Fixed pkg.installed(update_holds=True) for APT multiarch packages by preserving arch-qualified package names through install target parsing and verification. diff --git a/salt/modules/aptpkg.py b/salt/modules/aptpkg.py index 15755e8b53cd..deae0882eab6 100644 --- a/salt/modules/aptpkg.py +++ b/salt/modules/aptpkg.py @@ -646,6 +646,7 @@ def install( reinstall=False, downloadonly=False, ignore_epoch=False, + normalize=True, **kwargs, ): """ @@ -737,12 +738,21 @@ def install( .. versionadded:: 2018.3.0 + normalize : True + Normalize the package name by removing the architecture. Set this to + ``False`` to preserve explicitly arch-qualified package names such as + ``name:amd64``. + Multiple Package Installation Options: pkgs A list of packages to install from a software repository. Must be passed as a python list. + For multiarch packages, use the arch-qualified package name (for + example ``name:amd64``) when you need Salt to target that exact APT + package name. + CLI Example: .. code-block:: bash @@ -828,7 +838,11 @@ def install( try: pkg_params, pkg_type = __salt__["pkg_resource.parse_targets"]( - name, pkgs, sources, **kwargs + name, + pkgs, + sources, + normalize=normalize and kwargs.get("split_arch", True), + **kwargs, ) except MinionError as exc: raise CommandExecutionError(exc) @@ -1362,6 +1376,10 @@ def hold(name=None, pkgs=None, sources=None, **kwargs): # pylint: disable=W0613 name The name of the package, e.g., 'tmux' + For multiarch packages, dpkg may record the held package name with an + architecture suffix such as ``:amd64``. In those cases, the + arch-qualified package name may need to be used. + CLI Example: .. code-block:: bash @@ -1371,6 +1389,10 @@ def hold(name=None, pkgs=None, sources=None, **kwargs): # pylint: disable=W0613 pkgs A list of packages to hold. Must be passed as a python list. + For multiarch packages, dpkg may record the held package name with an + architecture suffix such as ``:amd64``. In those cases, the + arch-qualified package name may need to be used. + CLI Example: .. code-block:: bash @@ -1427,6 +1449,10 @@ def unhold(name=None, pkgs=None, sources=None, **kwargs): # pylint: disable=W06 name The name of the package, e.g., 'tmux' + For multiarch packages, dpkg may record the held package name with an + architecture suffix such as ``:amd64``. In those cases, the + arch-qualified package name may need to be used. + CLI Example: .. code-block:: bash @@ -1436,6 +1462,10 @@ def unhold(name=None, pkgs=None, sources=None, **kwargs): # pylint: disable=W06 pkgs A list of packages to unhold. Must be passed as a python list. + For multiarch packages, dpkg may record the held package name with an + architecture suffix such as ``:amd64``. In those cases, the + arch-qualified package name may need to be used. + CLI Example: .. code-block:: bash diff --git a/salt/states/pkg.py b/salt/states/pkg.py index 7a0e78afcafa..3fe1598bf2ca 100644 --- a/salt/states/pkg.py +++ b/salt/states/pkg.py @@ -583,7 +583,9 @@ def _find_install_targets( if any((pkgs, sources)): if pkgs: # pylint: disable=not-callable - desired = _repack_pkgs(pkgs, normalize=normalize) + desired = _repack_pkgs( + pkgs, normalize=normalize and kwargs.get("split_arch", True) + ) # pylint: enable=not-callable elif sources: desired = __salt__["pkg_resource.pack_sources"]( @@ -904,12 +906,15 @@ def _verify_install(desired, new_pkgs, ignore_epoch=None, new_caps=None): cver = new_pkgs.get(pkgname, new_pkgs.get(pkgname.split("/")[-1])) elif __grains__["os"] == "OpenBSD": cver = new_pkgs.get(pkgname.split("%")[0]) - elif __grains__["os_family"] == "Debian": - cver = new_pkgs.get(pkgname.split("=")[0]) else: - cver = new_pkgs.get(pkgname) - if not cver and pkgname in new_caps: - cver = new_pkgs.get(new_caps.get(pkgname)[0]) + lookup_name = pkgname.split("=")[0] + cver = new_pkgs.get(lookup_name) + if not cver and "pkg.normalize_name" in __salt__: + normalized_name = __salt__["pkg.normalize_name"](lookup_name) + if normalized_name != lookup_name: + cver = new_pkgs.get(normalized_name) + if not cver and lookup_name in new_caps: + cver = new_pkgs.get(new_caps.get(lookup_name)[0]) if not cver: failed.append(pkgname) @@ -1481,6 +1486,10 @@ def installed( package, the held package(s) will be skipped and the state will fail. By default, this parameter is set to ``False``. + Package naming rules for held packages may vary by package manager. + See the documentation for your platform's ``pkg`` module for any + provider-specific requirements. + Supported on YUM/DNF & APT based systems. .. versionadded:: 2016.11.0 @@ -1712,6 +1721,7 @@ def installed( ignore_epoch=ignore_epoch, reinstall=reinstall, refresh=refresh, + split_arch=False, **kwargs, ) @@ -3867,6 +3877,10 @@ def unheld(name, version=None, pkgs=None, all=False, **kwargs): For ``unheld`` there is no need to specify the exact version to be unheld. + Package naming rules for held packages may vary by package manager. + See the documentation for your platform's ``pkg`` module for any + provider-specific requirements. + :param bool all: Force removing of all existings locks. By default, this parameter is set to ``False``. diff --git a/tests/pytests/unit/modules/test_aptpkg.py b/tests/pytests/unit/modules/test_aptpkg.py index 15a3c620157d..0b041649159f 100644 --- a/tests/pytests/unit/modules/test_aptpkg.py +++ b/tests/pytests/unit/modules/test_aptpkg.py @@ -649,6 +649,40 @@ def test_install(install_var): assert expected_call in mock_call_apt.mock_calls +def test_install_preserves_multiarch_pkg_names_when_split_arch_is_false(): + """ + Test aptpkg.install preserves explicit multiarch package names. + """ + patch_kwargs = { + "__salt__": { + "pkg_resource.parse_targets": MagicMock( + return_value=({"libnvidia-cfg1-570-server:amd64": None}, "repository") + ), + "pkg_resource.sort_pkglist": MagicMock(), + "pkg_resource.stringify": MagicMock(), + "cmd.run_stdout": MagicMock(return_value=""), + } + } + mock_call_apt = MagicMock(return_value={"retcode": 0, "stdout": "", "stderr": ""}) + mock_parse_targets = patch_kwargs["__salt__"]["pkg_resource.parse_targets"] + + with patch.multiple( + aptpkg, list_pkgs=MagicMock(side_effect=[{}, {}]), **patch_kwargs + ): + with patch( + "salt.modules.aptpkg.get_selections", MagicMock(return_value={"hold": []}) + ): + with patch("salt.modules.aptpkg._call_apt", mock_call_apt): + aptpkg.install( + pkgs=["libnvidia-cfg1-570-server:amd64"], + refresh=False, + split_arch=False, + ) + + assert mock_parse_targets.call_args.kwargs["normalize"] is False + assert "libnvidia-cfg1-570-server:amd64" in mock_call_apt.mock_calls[0].args[0] + + def test_remove(uninstall_var): """ Test - Remove packages. diff --git a/tests/pytests/unit/states/test_pkg.py b/tests/pytests/unit/states/test_pkg.py index f9c566524df7..ba271cbd7ef3 100644 --- a/tests/pytests/unit/states/test_pkg.py +++ b/tests/pytests/unit/states/test_pkg.py @@ -828,6 +828,7 @@ def test_installed_with_single_normalize(): "pkg.install": yumpkg.install, "pkg.list_pkgs": list_pkgs, "pkg.normalize_name": yumpkg.normalize_name, + "pkg_resource.check_extra_requirements": MagicMock(return_value=True), "pkg_resource.version_clean": pkg_resource.version_clean, "pkg_resource.parse_targets": pkg_resource.parse_targets, } @@ -859,7 +860,7 @@ def test_installed_with_single_normalize(): ) call_yum_mock.assert_called_once() assert ( - "weird-name-1.2.3-1234.5.6.test7tst.x86_64-20220214-2.1" + "weird-name-1.2.3-1234.5.6.test7tst.x86_64.noarch-20220214-2.1" in call_yum_mock.mock_calls[0].args[0] ) assert ret["result"] @@ -873,13 +874,119 @@ def test_installed_with_single_normalize(): ) call_yum_mock.assert_called_once() assert ( - "weird-name-1.2.3-1234.5.6.test7tst.x86_64" + "weird-name-1.2.3-1234.5.6.test7tst.x86_64.noarch" in call_yum_mock.mock_calls[0].args[0] ) assert ret["result"] assert ret["changes"] == expected +def test_installed_preserves_apt_multiarch_pkg_names_for_update_holds(): + """ + Test pkg.installed keeps explicit APT multiarch package names intact. + """ + pkg_name = "zlib1g:amd64" + pkg_version = "1:1.3.dfsg-3.1ubuntu2.1" + install_mock = MagicMock( + return_value={pkg_name: {"old": "1:1.3.dfsg-3.1ubuntu2", "new": pkg_version}} + ) + list_pkgs_mock = MagicMock( + side_effect=[ + {pkg_name: ["1:1.3.dfsg-3.1ubuntu2"]}, + {"zlib1g": [pkg_version]}, + {"zlib1g": [pkg_version]}, + ] + ) + + salt_dict = { + "pkg.install": install_mock, + "pkg.list_pkgs": list_pkgs_mock, + "pkg.normalize_name": lambda pkg: ( + pkg.rsplit(":", 1)[0] if pkg.endswith(":amd64") else pkg + ), + "lowpkg.unpurge": MagicMock(return_value={}), + "pkg_resource.check_extra_requirements": MagicMock(return_value=True), + "pkg_resource.version_clean": pkg_resource.version_clean, + } + + with patch.dict(pkg.__salt__, salt_dict), patch.dict( + pkg_resource.__salt__, salt_dict + ), patch.dict( + pkg.__grains__, {"os": "Ubuntu", "os_family": "Debian", "osarch": "amd64"} + ), patch.dict( + pkg_resource.__grains__, {"os": "Ubuntu", "os_family": "Debian"} + ): + ret = pkg.installed( + "test_install", + pkgs=[{pkg_name: pkg_version}], + skip_suggestions=True, + update_holds=True, + ) + + install_mock.assert_called_once_with( + name=None, + refresh=False, + version=None, + fromrepo=None, + skip_verify=False, + pkgs=[{pkg_name: pkg_version}], + sources=None, + reinstall=False, + normalize=True, + update_holds=True, + ignore_epoch=None, + split_arch=False, + allow_updates=False, + saltenv="base", + ) + assert ret["result"] + assert ret["changes"] == { + pkg_name: {"old": "1:1.3.dfsg-3.1ubuntu2", "new": pkg_version}, + } + + +def test_verify_install_normalizes_debian_multiarch_names(): + """ + Test _verify_install matches Debian native-arch package names correctly. + """ + desired = {"zlib1g:amd64": "1:1.3.dfsg-3.1ubuntu2.1"} + new_pkgs = {"zlib1g": ["1:1.3.dfsg-3.1ubuntu2.1"]} + + with patch.dict( + pkg.__salt__, + { + "pkg.normalize_name": lambda name: ( + name.rsplit(":", 1)[0] if name.endswith(":amd64") else name + ), + "pkg_resource.version_clean": pkg_resource.version_clean, + }, + ), patch.dict(pkg.__grains__, {"os": "Ubuntu", "os_family": "Debian"}): + ok, failed = pkg._verify_install(desired, new_pkgs) + + assert ok == ["zlib1g:amd64"] + assert failed == [] + + +def test_verify_install_normalizes_yum_arch_names(): + """ + Test _verify_install matches YUM package names using normalized names. + """ + desired = {"weird-name-1.2.3-1234.5.6.test7tst.x86_64.noarch": "20220214-2.1"} + new_pkgs = {"weird-name-1.2.3-1234.5.6.test7tst.x86_64": ["20220214-2.1"]} + + with patch.dict( + pkg.__salt__, + { + "pkg.normalize_name": yumpkg.normalize_name, + "pkg_resource.version_clean": pkg_resource.version_clean, + }, + ), patch.dict(pkg.__grains__, {"os": "CentOS", "os_family": "RedHat"}): + ok, failed = pkg._verify_install(desired, new_pkgs) + + assert ok == ["weird-name-1.2.3-1234.5.6.test7tst.x86_64.noarch"] + assert failed == [] + + def test_removed_with_single_normalize(): """ Test pkg.removed with preventing multiple package name normalisation