From 296b693722a1bb9af00af1156d3e3cf9efc3612c Mon Sep 17 00:00:00 2001 From: Harm Kroon Date: Thu, 15 Oct 2026 18:23:35 +0200 Subject: [PATCH] Fix deb sync for missing indices and optimize enabled When running a sync with optimize=true on a existing deb remote, pulp compares the new package index with the previous version. When there is no previous version to compare to, the sync fails. This can happen when a actual remote only came into existance after a initial sync already has happened When a Release file had not changed, the sync added back the previous indices by matching the distribution name against the index path. For flat repos the stored name "flat-repo" never matches, so the index was dropped and the next upstream change hit the error above. A name that is a prefix of another distribution, such as "jammy" and "jammy-updates", matched the wrong indices and failed with "Paths are duplicated". The indices are now matched by the directory of the Release file. Closes: #719 Assisted-by: Claude Opus 5.5 --- CHANGES/719.bugfix | 1 + pulp_deb/app/tasks/synchronizing.py | 27 ++++++---- pulp_deb/tests/functional/api/test_sync.py | 16 ++++++ pulp_deb/tests/unit/test_sync_optimize.py | 59 ++++++++++++++++++++++ 4 files changed, 94 insertions(+), 9 deletions(-) create mode 100644 CHANGES/719.bugfix create mode 100644 pulp_deb/tests/unit/test_sync_optimize.py diff --git a/CHANGES/719.bugfix b/CHANGES/719.bugfix new file mode 100644 index 000000000..5926a020c --- /dev/null +++ b/CHANGES/719.bugfix @@ -0,0 +1 @@ +Fixed optimized syncs failing with "'NoneType' object has no attribute 'artifact_set_sha256'" when a package index is new upstream. Optimized syncs of flat repositories no longer drop their package index when the Release file has not changed, and a distribution whose name is a prefix of another (for example "jammy" and "jammy-updates") no longer causes "Paths are duplicated" errors. diff --git a/pulp_deb/app/tasks/synchronizing.py b/pulp_deb/app/tasks/synchronizing.py index 25d20c1e1..574610e17 100644 --- a/pulp_deb/app/tasks/synchronizing.py +++ b/pulp_deb/app/tasks/synchronizing.py @@ -591,7 +591,7 @@ async def _handle_distribution(self, distribution): ) if previous_release_file.artifact_set_sha256 == release_file.artifact_set_sha256: await _readd_previous_package_indices( - self.previous_repo_version, self.new_version, stored_distribution + self.previous_repo_version, self.new_version, release_file ) message = 'ReleaseFile has not changed for distribution="{}". Skipping.' log.info(_(message).format(distribution)) @@ -921,7 +921,12 @@ async def _handle_package_index( previous_package_index = await _get_previous_package_index( self.previous_repo_version, relative_path ) - if previous_package_index.artifact_set_sha256 == package_index.artifact_set_sha256: + # The previous version may lack this PackageIndex, for example if the index is new + # upstream. Then there is nothing to compare against. + if ( + previous_package_index + and previous_package_index.artifact_set_sha256 == package_index.artifact_set_sha256 + ): message = 'PackageIndex has not changed for relative_path="{}". Skipped.' log.info(_(message).format(relative_path)) async with ProgressReport( @@ -1249,16 +1254,20 @@ def _get_content_artifact_file(content_artifact): @sync_to_async -def _readd_previous_package_indices(previous_version, new_version, distribution): +def _readd_previous_package_indices(previous_version, new_version, release_file): + # Match on the directory of the Release file, not on the distribution name: a flat repo is + # stored as "flat-repo", which is not part of its index paths, and a name like "jammy" is also + # a substring of "jammy-updates". + release_base_path = os.path.dirname(release_file.relative_path) + if release_base_path: + path_filter = {"relative_path__startswith": f"{release_base_path}/"} + else: + path_filter = {"relative_path__regex": r"^[^/]+$"} new_version.add_content( - previous_version.get_content( - PackageIndex.objects.filter(relative_path__contains=distribution) - ) + previous_version.get_content(PackageIndex.objects.filter(**path_filter)) ) new_version.add_content( - previous_version.get_content( - InstallerFileIndex.objects.filter(relative_path__contains=distribution) - ) + previous_version.get_content(InstallerFileIndex.objects.filter(**path_filter)) ) diff --git a/pulp_deb/tests/functional/api/test_sync.py b/pulp_deb/tests/functional/api/test_sync.py index 8e8540561..a179a50ac 100644 --- a/pulp_deb/tests/functional/api/test_sync.py +++ b/pulp_deb/tests/functional/api/test_sync.py @@ -10,6 +10,7 @@ DEB_FIXTURE_ARCH_UPDATE, DEB_FIXTURE_COMPONENT, DEB_FIXTURE_COMPONENT_UPDATE, + DEB_FIXTURE_FLAT_REPOSITORY_NAME, DEB_FIXTURE_INVALID_REPOSITORY_NAME, DEB_FIXTURE_METADATA_UPDATE_REPOSITORY_NAME, DEB_FIXTURE_SINGLE_DIST, @@ -466,6 +467,21 @@ def test_sync_optimize_with_mirror_enabled(deb_init_and_sync): assert is_sync_skipped(task, DEB_REPORT_CODE_SKIP_COMPLETE) +@pytest.mark.parallel +def test_sync_optimize_flat_repo_keeps_package_index(deb_init_and_sync, apt_package_indices_api): + """Test that an unchanged optimized resync of a flat repo keeps its package index.""" + remote_args = {"distributions": "/"} + repo, remote = deb_init_and_sync(url=DEB_FIXTURE_FLAT_REPOSITORY_NAME, remote_args=remote_args) + repo_v1_href = repo.latest_version_href + assert apt_package_indices_api.list(repository_version=repo_v1_href).count > 0 + + repo, _, task = deb_init_and_sync(repository=repo, remote=remote, return_task=True) + + # Before the fix, the resync created a new version without the package index. + assert is_sync_skipped(task, DEB_REPORT_CODE_SKIP_RELEASE) + assert repo.latest_version_href == repo_v1_href + + @pytest.mark.skipif(settings.DOMAIN_ENABLED, reason="Domain produces different results.") def test_sync_orphan_cleanup_fail( deb_init_and_sync, diff --git a/pulp_deb/tests/unit/test_sync_optimize.py b/pulp_deb/tests/unit/test_sync_optimize.py new file mode 100644 index 000000000..f4d9d9cab --- /dev/null +++ b/pulp_deb/tests/unit/test_sync_optimize.py @@ -0,0 +1,59 @@ +"""Tests for optimized syncs against a previous version that lacks a package index.""" + +import io +from unittest import IsolatedAsyncioTestCase, mock + +from pulp_deb.app.tasks.synchronizing import DebFirstStage, gen_remote_options + +SYNCHRONIZING = "pulp_deb.app.tasks.synchronizing" + + +class TestOptimizeWithoutPreviousPackageIndex(IsolatedAsyncioTestCase): + """ + An index that is new upstream, for example a new component or architecture, has no counterpart + in the previous repository version. See https://github.com/pulp/pulp_deb/issues/719. + """ + + def setUp(self): + remote = mock.Mock( + url="http://example.com/debian/", + distributions="stable", + components="main", + architectures="amd64", + policy="immediate", + sync_sources=False, + sync_udebs=False, + sync_installer=False, + gpgkey=None, + ignore_missing_package_indices=False, + ) + previous_version = mock.Mock( + info={ + "remote_options": gen_remote_options(remote), + "sync_options": {"optimize": True, "mirror": False}, + } + ) + self.stage = DebFirstStage(remote, True, False, previous_version) + self.stage._create_unit = mock.AsyncMock( + return_value=mock.Mock(artifact_set_sha256="new-index") + ) + + async def test_new_package_index_is_processed(self): + """The new index must be parsed, not compared with a previous index that is not there.""" + self.assertTrue(self.stage.optimize) + main_artifact = mock.AsyncMock(return_value=mock.Mock(file=io.BytesIO(b""))) + with ( + mock.patch( + f"{SYNCHRONIZING}._get_previous_package_index", + mock.AsyncMock(return_value=None), + ), + mock.patch(f"{SYNCHRONIZING}._get_main_artifact_blocking", main_artifact), + ): + await self.stage._handle_package_index( + release_file=mock.Mock(relative_path="dists/stable/Release", architectures="amd64"), + release_component=mock.Mock(plain_component="main", component="main"), + base_architecture="amd64", + index_architecture="amd64", + file_references={"main/binary-amd64/Packages": {"SHA256": "0" * 64, "Size": "0"}}, + ) + main_artifact.assert_awaited_once()