Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prefer the 'apt_pkg' module over 'apt' (SC-1516) #2744

Merged
merged 11 commits into from
Oct 2, 2023
Merged
4 changes: 2 additions & 2 deletions .github/workflows/ci-base.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
- name: Install dependencies
run: |
sudo DEBIAN_FRONTEND=noninteractive apt-get -qy update
sudo DEBIAN_FRONTEND=noninteractive apt-get -qy install tox
sudo DEBIAN_FRONTEND=noninteractive apt-get -qy install tox libapt-pkg-dev
- name: Git checkout
uses: actions/checkout@v3
- name: Formatting
Expand All @@ -36,7 +36,7 @@ jobs:
- name: Install dependencies
run: |
sudo DEBIAN_FRONTEND=noninteractive apt-get -qy update
sudo DEBIAN_FRONTEND=noninteractive apt-get -qy install tox
sudo DEBIAN_FRONTEND=noninteractive apt-get -qy install tox libapt-pkg-dev
- name: Git checkout
uses: actions/checkout@v3
- name: Unit
Expand Down
1 change: 1 addition & 0 deletions debian/control
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ Build-Depends: bash-completion,
gettext,
git,
libapt-pkg-dev,
python3-apt,
libjson-c-dev,
libboost-test-dev,
po-debconf,
Expand Down
2 changes: 1 addition & 1 deletion features/unattached_commands.feature
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ Feature: Command behaviour when unattached
When I run `cat /var/log/ubuntu-advantage.log` with sudo
Then stdout matches regexp:
"""
Failed to fetch the ESM Apt Cache
Failed to fetch ESM Apt Cache item:
"""

Examples: ubuntu release
Expand Down
5 changes: 5 additions & 0 deletions test-requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,8 @@ pycodestyle
mock
pytest
pytest-cov

# python3-apt
# We know it is in the distro, but for testing in venvs we need to install it
# And well, there is no pypi package
git+https://salsa.debian.org/apt-team/python-apt@main
2 changes: 1 addition & 1 deletion uaclient/api/tests/test_api_u_pro_packages_updates.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
class TestPackagesUpdatesV1:
@mock.patch(M_PATH + "get_ua_info")
@mock.patch(M_PATH + "get_installed_packages_by_origin")
@mock.patch(M_PATH + "filter_security_updates")
@mock.patch(M_PATH + "filter_updates")
@mock.patch(M_PATH + "create_updates_list")
def test_package_updates(
self, m_updates, m_filter, _m_packages, _m_ua_info, FakeConfig
Expand Down
36 changes: 0 additions & 36 deletions uaclient/api/tests/test_fix.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,6 @@ def test_fix_plan_for_no_affected_packages(
@mock.patch(M_PATH + "merge_usn_released_binary_package_versions")
@mock.patch(M_PATH + "get_cve_affected_source_packages_status")
@mock.patch("uaclient.apt.get_pkg_candidate_version")
@mock.patch("uaclient.apt.compare_versions")
@mock.patch(M_PATH + "_get_cve_data")
@mock.patch(M_PATH + "query_installed_source_pkg_versions")
@mock.patch(M_PATH + "_check_cve_fixed_by_livepatch")
Expand All @@ -173,7 +172,6 @@ def test_fix_plan_for_cve(
m_check_cve_fixed_by_livepatch,
m_query_installed_pkgs,
m_get_cve_data,
m_apt_compare_versions,
m_get_pkg_candidate_version,
m_get_cve_affected_pkgs,
m_merge_usn_pkgs,
Expand Down Expand Up @@ -220,7 +218,6 @@ def test_fix_plan_for_cve(
},
}
}
m_apt_compare_versions.side_effect = [False, False, True, True]
m_get_pkg_candidate_version.side_effect = ["1.1", "1.2"]

expected_plan = FixPlanResult(
Expand Down Expand Up @@ -253,7 +250,6 @@ def test_fix_plan_for_cve(
@mock.patch(M_PATH + "_is_attached")
@mock.patch(M_PATH + "get_contract_expiry_status")
@mock.patch("uaclient.apt.get_pkg_candidate_version")
@mock.patch("uaclient.apt.compare_versions")
@mock.patch(M_PATH + "_get_cve_data")
@mock.patch(M_PATH + "query_installed_source_pkg_versions")
@mock.patch(M_PATH + "_check_cve_fixed_by_livepatch")
Expand All @@ -262,7 +258,6 @@ def test_fix_plan_for_cve_that_requires_pro_services(
m_check_cve_fixed_by_livepatch,
m_query_installed_pkgs,
m_get_cve_data,
m_apt_compare_versions,
m_get_pkg_candidate_version,
m_get_contract_expiry_status,
m_is_attached,
Expand Down Expand Up @@ -356,16 +351,6 @@ def test_fix_plan_for_cve_that_requires_pro_services(
},
},
}
m_apt_compare_versions.side_effect = [
False,
False,
False,
False,
True,
True,
True,
True,
]
m_get_pkg_candidate_version.side_effect = [
"1.1",
"1.2",
Expand Down Expand Up @@ -454,7 +439,6 @@ def test_fix_plan_for_cve_that_requires_pro_services(
@mock.patch(M_PATH + "merge_usn_released_binary_package_versions")
@mock.patch(M_PATH + "get_cve_affected_source_packages_status")
@mock.patch("uaclient.apt.get_pkg_candidate_version")
@mock.patch("uaclient.apt.compare_versions")
@mock.patch("uaclient.api.u.pro.security.fix._get_cve_data")
@mock.patch(M_PATH + "query_installed_source_pkg_versions")
@mock.patch(M_PATH + "_check_cve_fixed_by_livepatch")
Expand All @@ -463,7 +447,6 @@ def test_fix_plan_for_cve_when_package_cannot_be_installed(
m_check_cve_fixed_by_livepatch,
m_query_installed_pkgs,
m_get_cve_data,
m_apt_compare_versions,
m_get_pkg_candidate_version,
m_get_cve_affected_pkgs,
m_merge_usn_pkgs,
Expand Down Expand Up @@ -510,7 +493,6 @@ def test_fix_plan_for_cve_when_package_cannot_be_installed(
},
},
}
m_apt_compare_versions.side_effect = [False, False, True, False]
m_get_pkg_candidate_version.side_effect = ["1.1", "1.1"]

expected_plan = FixPlanResult(
Expand Down Expand Up @@ -550,7 +532,6 @@ def test_fix_plan_for_cve_when_package_cannot_be_installed(
@mock.patch(M_PATH + "merge_usn_released_binary_package_versions")
@mock.patch(M_PATH + "get_cve_affected_source_packages_status")
@mock.patch("uaclient.apt.get_pkg_candidate_version")
@mock.patch("uaclient.apt.compare_versions")
@mock.patch("uaclient.api.u.pro.security.fix._get_cve_data")
@mock.patch(M_PATH + "query_installed_source_pkg_versions")
@mock.patch(M_PATH + "_check_cve_fixed_by_livepatch")
Expand All @@ -559,7 +540,6 @@ def test_fix_plan_for_cve_with_not_released_status(
m_check_cve_fixed_by_livepatch,
m_query_installed_pkgs,
m_get_cve_data,
m_apt_compare_versions,
m_get_pkg_candidate_version,
m_get_cve_affected_pkgs,
m_merge_usn_pkgs,
Expand Down Expand Up @@ -615,7 +595,6 @@ def test_fix_plan_for_cve_with_not_released_status(
},
}
}
m_apt_compare_versions.side_effect = [False, False, True, True]
m_get_pkg_candidate_version.side_effect = ["1.1", "1.2"]

expected_plan = FixPlanResult(
Expand Down Expand Up @@ -651,7 +630,6 @@ def test_fix_plan_for_cve_with_not_released_status(
)

@mock.patch("uaclient.apt.get_pkg_candidate_version")
@mock.patch("uaclient.apt.compare_versions")
@mock.patch(M_PATH + "merge_usn_released_binary_package_versions")
@mock.patch(M_PATH + "get_affected_packages_from_usn")
@mock.patch(M_PATH + "_get_usn_data")
Expand All @@ -662,7 +640,6 @@ def test_fix_plan_for_usn(
m_get_usn_data,
m_get_affected_packages_from_usn,
m_merge_usn_released_binary_package,
m_apt_compare_versions,
m_get_pkg_candidate_version,
):
m_query_installed_pkgs.return_value = {
Expand Down Expand Up @@ -773,16 +750,6 @@ def test_fix_plan_for_usn(
}
},
]
m_apt_compare_versions.side_effect = [
False,
False,
True,
True,
False,
True,
False,
True,
]
m_get_pkg_candidate_version.side_effect = ["1.1", "1.2", "1.3", "1.4"]

expected_plan = FixPlanUSNResult(
Expand Down Expand Up @@ -865,7 +832,6 @@ def test_fix_plan_for_usn(
@mock.patch(M_PATH + "merge_usn_released_binary_package_versions")
@mock.patch(M_PATH + "get_cve_affected_source_packages_status")
@mock.patch("uaclient.apt.get_pkg_candidate_version")
@mock.patch("uaclient.apt.compare_versions")
@mock.patch(M_PATH + "_get_cve_data")
@mock.patch(M_PATH + "query_installed_source_pkg_versions")
@mock.patch(M_PATH + "_check_cve_fixed_by_livepatch")
Expand All @@ -874,7 +840,6 @@ def test_fix_plan_for_cve_when_package_already_installed(
m_check_cve_fixed_by_livepatch,
m_query_installed_pkgs,
m_get_cve_data,
m_apt_compare_versions,
m_get_pkg_candidate_version,
m_get_cve_affected_pkgs,
m_merge_usn_pkgs,
Expand Down Expand Up @@ -921,7 +886,6 @@ def test_fix_plan_for_cve_when_package_already_installed(
},
},
}
m_apt_compare_versions.side_effect = [True, True]
m_get_pkg_candidate_version.side_effect = ["1.1", "1.1"]

expected_plan = FixPlanResult(
Expand Down
4 changes: 2 additions & 2 deletions uaclient/api/u/pro/packages/updates/v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
)
from uaclient.security_status import (
create_updates_list,
filter_security_updates,
filter_updates,
get_installed_packages_by_origin,
get_ua_info,
)
Expand Down Expand Up @@ -87,7 +87,7 @@ def updates() -> PackageUpdatesResult:
def _updates(cfg: UAConfig) -> PackageUpdatesResult:
ua_info = get_ua_info(cfg)
packages = get_installed_packages_by_origin()
upgradable_versions = filter_security_updates(packages["all"])
upgradable_versions = filter_updates(packages["all"])
update_list = create_updates_list(upgradable_versions, ua_info)

num_esm_apps_updates = len(upgradable_versions["esm-apps"])
Expand Down
10 changes: 7 additions & 3 deletions uaclient/api/u/pro/security/fix/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -576,8 +576,12 @@ def _get_upgradable_pkgs(
candidate_version = apt.get_pkg_candidate_version(
binary_pkg.binary_pkg, check_esm_cache=check_esm_cache
)
if candidate_version and apt.compare_versions(
binary_pkg.fixed_version, candidate_version, "le"
if (
candidate_version
and apt.version_compare(
binary_pkg.fixed_version, candidate_version
)
<= 0
):
upgrade_pkgs.append(binary_pkg.binary_pkg)
else:
Expand Down Expand Up @@ -610,7 +614,7 @@ def _get_upgradable_package_candidates_by_pocket(
"version", ""
)

if not apt.compare_versions(fixed_version, version, "le"):
if apt.version_compare(fixed_version, version) > 0:
binary_pocket_pkgs[pkg_status.pocket_source].append(
BinaryPackageFix(
source_pkg=src_pkg,
Expand Down
Loading
Loading