Skip to content

Commit

Permalink
fix: disregard scheme when looking for existing sources (#140)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
tigarmo authored Dec 5, 2024
1 parent adcc412 commit 7a8d9eb
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 16 deletions.
16 changes: 11 additions & 5 deletions craft_archives/repo/apt_sources_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -397,15 +402,16 @@ 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)

for source_dict in Deb822.iter_paragraphs(
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:
Expand All @@ -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,
)
Expand Down
7 changes: 7 additions & 0 deletions docs/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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)
-------------------

Expand Down
60 changes: 50 additions & 10 deletions tests/integration/repo/test_apt_sources_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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(
Expand All @@ -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'
"""
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/repo/test_apt_sources_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'"
)
Expand Down

0 comments on commit 7a8d9eb

Please sign in to comment.