From 7a8d9ebaf5710fc4097d4b8b6c4e7831f65bfda3 Mon Sep 17 00:00:00 2001 From: Tiago Nobrega Date: Thu, 5 Dec 2024 19:04:30 -0300 Subject: [PATCH] fix: disregard scheme when looking for existing sources (#140) Apt does not take the scheme (e.g. http vs https) into account when parsing sources, which means that, for the purposes of finding existing keys, 'https://archive.ubuntu.com' and 'http://archive.ubuntu.com' should be considered the same. Fixes #139 --- craft_archives/repo/apt_sources_manager.py | 16 +++-- docs/changelog.rst | 7 +++ .../repo/test_apt_sources_manager.py | 60 +++++++++++++++---- tests/unit/repo/test_apt_sources_manager.py | 2 +- 4 files changed, 69 insertions(+), 16 deletions(-) diff --git a/craft_archives/repo/apt_sources_manager.py b/craft_archives/repo/apt_sources_manager.py index c92f430..4fc2119 100644 --- a/craft_archives/repo/apt_sources_manager.py +++ b/craft_archives/repo/apt_sources_manager.py @@ -22,6 +22,7 @@ import subprocess from pathlib import Path from typing import List, Optional, Sequence, cast +from urllib.parse import urlparse import distro from debian.deb822 import Deb822 @@ -367,8 +368,12 @@ def _get_suites(pocket: PocketEnum, series: str) -> List[str]: return suites -def _dirify_url(url: str) -> str: - # Apt urls are always directories +def _normalize_archive_url(url: str) -> str: + parsed = urlparse(url) + # Disregard the scheme: Apt considers both http and https the same for + # resolving the archive. + url = parsed.netloc + parsed.path + # Apt urls are always directories. if not url.endswith("/"): return url + "/" return url @@ -397,7 +402,8 @@ def _get_existing_keyring_for( if not sources_file.is_file(): return None - url = _dirify_url(url) + original_url = url + url = _normalize_archive_url(url) logger.debug("Reading sources in '%s' looking for '%s'", sources_file, url) @@ -405,7 +411,7 @@ def _get_existing_keyring_for( sequence=sources_file.read_text(), fields=["URIs", "Suites", "Signed-By"] ): try: - source_url = _dirify_url(source_dict["URIs"]) + source_url = _normalize_archive_url(source_dict["URIs"]) source_suites = set(source_dict.get("Suites", "").split()) source_signed = source_dict["Signed-By"] except KeyError: @@ -432,7 +438,7 @@ def _get_existing_keyring_for( # the moment. raise errors.SourcesKeyConflictError( requested_key_id=key_id, - requested_url=url, + requested_url=original_url, conflict_keyring=source_signed, conflicting_source=sources_file, ) diff --git a/docs/changelog.rst b/docs/changelog.rst index 5325854..c065276 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -5,6 +5,13 @@ Changelog See the `Releases page`_ on Github for a complete list of commits that are included in each version. +2.0.2 (2024-Dec-04) +------------------- + +* Fix an issue where declaring a package-repository to an Ubuntu archive + using the "https" scheme would cause an error in a later ``apt update`` + when in Noble. + 2.0.1 (2024-Oct-21) ------------------- diff --git a/tests/integration/repo/test_apt_sources_manager.py b/tests/integration/repo/test_apt_sources_manager.py index 0c8822c..56e6355 100644 --- a/tests/integration/repo/test_apt_sources_manager.py +++ b/tests/integration/repo/test_apt_sources_manager.py @@ -38,14 +38,57 @@ @pytest.mark.parametrize( - ["source_url", "source_arch"], + ["source_url", "repo_url", "repo_arch", "expected_url"], [ - ("http://archive.ubuntu.com/ubuntu/", "i386"), - ("http://ports.ubuntu.com/ubuntu-ports/", "armhf"), + # Exact same url + pytest.param( + "http://archive.ubuntu.com/ubuntu/", + "http://archive.ubuntu.com/ubuntu/", + "i386", + "archive.ubuntu.com/ubuntu/", + id="archive-same-url", + ), + pytest.param( + "http://ports.ubuntu.com/ubuntu-ports/", + "http://ports.ubuntu.com/ubuntu-ports/", + "armhf", + "ports.ubuntu.com/ubuntu-ports/", + id="ports-same-url", + ), + # Different url (no ending /) + pytest.param( + "http://archive.ubuntu.com/ubuntu/", + "http://archive.ubuntu.com/ubuntu", + "i386", + "archive.ubuntu.com/ubuntu/", + id="archive-different-ending", + ), + pytest.param( + "http://ports.ubuntu.com/ubuntu-ports/", + "http://ports.ubuntu.com/ubuntu-ports", + "armhf", + "ports.ubuntu.com/ubuntu-ports/", + id="ports-different-ending", + ), + # Different scheme (http vs https) + pytest.param( + "http://archive.ubuntu.com/ubuntu/", + "https://archive.ubuntu.com/ubuntu", + "i386", + "archive.ubuntu.com/ubuntu/", + id="archive-different-scheme", + ), + pytest.param( + "http://ports.ubuntu.com/ubuntu-ports/", + "https://ports.ubuntu.com/ubuntu-ports", + "armhf", + "ports.ubuntu.com/ubuntu-ports/", + id="ports-different-scheme", + ), ], ) def test_install_sources_conflicting_keys( - tmp_path, test_data_dir, caplog, source_url, source_arch + tmp_path, test_data_dir, caplog, source_url, repo_url, repo_arch, expected_url ): caplog.set_level(logging.DEBUG) fake_system = tmp_path @@ -72,15 +115,12 @@ def test_install_sources_conflicting_keys( ) ) - # Note that the `url` string is different from the URIs in DEFAULT_SOURCE, - # but they mean the same URL. - repo_url = source_url[:-1] repository = PackageRepositoryApt( type="apt", url=repo_url, suites=["noble"], components=["main", "universe"], - architectures=[source_arch], + architectures=[repo_arch], key_id="78E1918602959B9C59103100F1831DDAFC42E99D", ) sources_manager = AptSourcesManager( @@ -96,8 +136,8 @@ def test_install_sources_conflicting_keys( expected_log = textwrap.dedent( f""" - Looking for existing sources files for url '{repo_url}' and suites ['noble'] - Reading sources in '{ubuntu_sources}' looking for '{source_url}' + Looking for existing sources files for url '{repository.url}' and suites ['noble'] + Reading sources in '{ubuntu_sources}' looking for '{expected_url}' Source has these suites: ['noble', 'noble-backports', 'noble-updates'] Suites match - Signed-By is '/usr/share/keyrings/FC42E99D.gpg' """ diff --git a/tests/unit/repo/test_apt_sources_manager.py b/tests/unit/repo/test_apt_sources_manager.py index a58267b..6273d1b 100644 --- a/tests/unit/repo/test_apt_sources_manager.py +++ b/tests/unit/repo/test_apt_sources_manager.py @@ -440,7 +440,7 @@ def test_existing_key_incompatible(apt_sources_mgr, tmp_path, mocker): expected_message = re.escape( "The key '78E1918602959B9C59103100F1831DDAFC42E99D' for " - "the repository with url 'http://archive.ubuntu.com/ubuntu/' conflicts " + "the repository with url 'http://archive.ubuntu.com/ubuntu' conflicts " f"with a source in '{ubuntu_sources}', " "which is signed by '/usr/share/keyrings/0264B26D.gpg'" )