From ae7c8c25882bfb1d96f49b8f60489a147715299c Mon Sep 17 00:00:00 2001 From: Robert Bikar Date: Fri, 5 Jun 2026 10:26:22 +0200 Subject: [PATCH 1/3] Disallow RPM upload with the same NVR [RHELDST-37824] This change introduces a functionality that will raise a fatal error when upload of an RPM with identical `cdn_path` of already present unit in pulp is attempted. The RPMs in question differ only with their checksums (and signing keys), further association to live repositories and publish will cause breakage of repository. Duplicates check is by default disabled, and can be enabled by setting PUBTOOLS_PULP_ALLOW_DUPLICATE_UNITS env var. This should be a workaround until proper suport for such scenario is implemented. --- src/pubtools/_pulp/tasks/push/items/base.py | 4 + src/pubtools/_pulp/tasks/push/items/rpm.py | 20 +++++ .../_pulp/tasks/push/phase/constants.py | 5 ++ src/pubtools/_pulp/tasks/push/phase/upload.py | 9 +++ tests/push/test_push.py | 7 +- tests/push/test_push_rpm_duplicate.py | 79 +++++++++++++++++++ 6 files changed, 123 insertions(+), 1 deletion(-) create mode 100644 tests/push/test_push_rpm_duplicate.py diff --git a/src/pubtools/_pulp/tasks/push/items/base.py b/src/pubtools/_pulp/tasks/push/items/base.py index ae48d2bb..0b55b2a0 100644 --- a/src/pubtools/_pulp/tasks/push/items/base.py +++ b/src/pubtools/_pulp/tasks/push/items/base.py @@ -645,3 +645,7 @@ def upload_key(self): content types which can safely reuse uploads. """ return None + + def fail_if_duplicate(self, pulp_client): # pylint:disable=unused-argument + """Subclasses may override this method to check for duplicates and fail if found.""" + return None diff --git a/src/pubtools/_pulp/tasks/push/items/rpm.py b/src/pubtools/_pulp/tasks/push/items/rpm.py index 9fa69294..082ed484 100644 --- a/src/pubtools/_pulp/tasks/push/items/rpm.py +++ b/src/pubtools/_pulp/tasks/push/items/rpm.py @@ -144,3 +144,23 @@ def ensure_uploaded(self, ctx, repo_f=None): def upload_to_repo(self, repo): return repo.upload_rpm(self.pushsource_item.content(), cdn_path=self.cdn_path) + + def fail_if_duplicate(self, pulp_client): + """ + If there is already an RPM with identical CDN path of requested within push present in Pulp but has different checksum, raise an error. + Publishing such units would break integrity of repositories when published becuase of collision on origin path. + """ + crit = Criteria.and_( + Criteria.with_unit_type(RpmUnit, unit_fields=["filename", "cdn_path", "sha256sum"]), + Criteria.with_field("filename", self.pushsource_item.name), + ) + + results = pulp_client.search_content(crit) + for item in results: + if item.cdn_path and item.cdn_path == self.cdn_path and item.sha256sum != self.pushsource_item.sha256sum: + msg = "Fatal error: Duplicate RPM present in Pulp: %s, sha256: %s, cdn_path: %s" % ( + item.filename, + item.sha256sum, + item.cdn_path, + ) + raise RuntimeError(msg) diff --git a/src/pubtools/_pulp/tasks/push/phase/constants.py b/src/pubtools/_pulp/tasks/push/phase/constants.py index 8c713d51..36490049 100644 --- a/src/pubtools/_pulp/tasks/push/phase/constants.py +++ b/src/pubtools/_pulp/tasks/push/phase/constants.py @@ -112,3 +112,8 @@ def atom(name): OUT_MAX_FUTURES = int(os.getenv("PUBTOOLS_PULP_OUT_MAX_FUTURES") or "10") """Max number of pending futures in output buffer.""" + +# Special workaround for allowing duplicate units to be uploaded to Pulp. +ALLOW_DUPLICATE_UNITS = os.getenv( + "PUBTOOLS_PULP_ALLOW_DUPLICATE_UNITS", "True" +).strip().lower() in ("true", "1", "yes", "y") diff --git a/src/pubtools/_pulp/tasks/push/phase/upload.py b/src/pubtools/_pulp/tasks/push/phase/upload.py index 3be6772e..be65b314 100644 --- a/src/pubtools/_pulp/tasks/push/phase/upload.py +++ b/src/pubtools/_pulp/tasks/push/phase/upload.py @@ -1,6 +1,8 @@ import logging + from .base import Phase +from . import constants from ..items import State LOG = logging.getLogger("pubtools.pulp") @@ -48,6 +50,12 @@ def run(self): upload_context = {} for item in self.iter_input(): + # We don't want to allow any other operation on units with potentially duplicated origin path. + # Limited to units that would be associated to destination repos which would cause origin path collision. + if not constants.ALLOW_DUPLICATE_UNITS and \ + item.pulp_state in [State.MISSING, State.PARTIAL]: + item.fail_if_duplicate(self.pulp_client) + if item.pulp_state in [State.IN_REPOS, State.PARTIAL, State.NEEDS_UPDATE]: # This item is already in Pulp. uploaded += 1 @@ -59,6 +67,7 @@ def run(self): else: # This item is not in Pulp, or otherwise needs a reupload. item_type = type(item) + if item.MULTI_UPLOAD_CONTEXT: if item_type not in upload_context: upload_context[item_type] = {} diff --git a/tests/push/test_push.py b/tests/push/test_push.py index 2d27e0a0..f2ef7619 100644 --- a/tests/push/test_push.py +++ b/tests/push/test_push.py @@ -19,7 +19,7 @@ from pubtools.pluggy import pm from pubtools._pulp.tasks.push import entry_point -from pubtools._pulp.tasks.push.phase import context +from pubtools._pulp.tasks.push.phase import context, constants from .util import hide_unit_ids @@ -88,10 +88,15 @@ def test_typical_push( command_tester, hookspy, stub_collector, + monkeypatch, ): """Test a typical case of push using all sorts of content where the content is initially not present in Pulp. """ + # patch this constant to disallow duplicate units to be uploaded to Pulp. + # should be a no-op for non-RPM units, should pass for RPM units. + monkeypatch.setattr(constants, "ALLOW_DUPLICATE_UNITS", False) + # Sanity check that the Pulp server is, initially, empty. client = fake_controller.client assert list(client.search_content()) == [] diff --git a/tests/push/test_push_rpm_duplicate.py b/tests/push/test_push_rpm_duplicate.py new file mode 100644 index 00000000..5782ffd3 --- /dev/null +++ b/tests/push/test_push_rpm_duplicate.py @@ -0,0 +1,79 @@ +import functools + +import pytest + +from pushsource import Source, RpmPushItem +from pubtools.pulplib import RpmUnit + +from pubtools._pulp.tasks.push import entry_point +from pubtools._pulp.tasks.push.phase import constants + + +def test_push_rpm_duplicate_fail( + fake_controller, fake_push, command_tester, caplog, monkeypatch +): + """Test that push detects and fails in the case where a RPM with the same NVR but different checksum is pushed to Pulp in the destionation repository.""" + monkeypatch.setattr(constants, "ALLOW_DUPLICATE_UNITS", False) + + client = fake_controller.client + + rpm_dest = client.get_repository("dest1").result() + + # Make this file exist but not in all the desired repos. + existing_rpm = RpmUnit( + name="some-rpm", + version="1.0.0", + release="1", + arch="noarch", + sha256sum="db68c8a70f8383de71c107dca5fcfe53b1132186d1a6681d9ee3f4eea724fabb", + filename="some-rpm-1.0.0-1.noarch.rpm", + cdn_path="/content/origin/rpms/some-rpm/1.0.0/1/f21541eb/some-rpm-1.0.0-1.noarch.rpm", + signing_key="F21541EB", + ) + fake_controller.insert_units(rpm_dest, [existing_rpm]) + + # Unit is now in dest1 repository. + # Set up a pushsource backend which requests push of the RPM with the same NVR/signing key but different checksum + Source.register_backend( + "test", + lambda: [ + RpmPushItem( + name="some-rpm-1.0.0-1.noarch.rpm", + dest=["dest1", "dest2"], + # different checksum but same GPG key + sha256sum="e823456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", + md5sum=32 * "a", + src="fake/path", + signing_key="F21541EB", + ), + ], + ) + + args = [ + "", + "--source", + "test:", + "--pulp-url", + "https://pulp.example.com/", + "--allow-unsigned", + ] + + run = functools.partial(entry_point, cls=lambda: fake_push) + + # Ask it to push. + with pytest.raises(SystemExit) as excinfo: + command_tester.test( + run, + args, + # Can't guarantee a stable log order. + compare_plaintext=False, + compare_jsonl=False, + ) + + # It should have failed. + assert excinfo.value.code == 59 + + # It should tell us why it failed. + msg = "Duplicate RPM present in Pulp: some-rpm-1.0.0-1.noarch.rpm, sha256: db68c8a70f8383de71c107dca5fcfe53b1132186d1a6681d9ee3f4eea724fabb, cdn_path: /content/origin/rpms/some-rpm/1.0.0/1/f21541eb/some-rpm-1.0.0-1.noarch.rpm" + + assert msg in caplog.text From 8c702aba8ae8557697581b69ab8559d25141255f Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 12 Jun 2026 11:58:00 +0000 Subject: [PATCH 2/3] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/pubtools/_pulp/tasks/push/items/rpm.py | 23 +++++++++++++------ src/pubtools/_pulp/tasks/push/phase/upload.py | 6 +++-- tests/push/test_push_rpm_duplicate.py | 13 ++++++----- 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/src/pubtools/_pulp/tasks/push/items/rpm.py b/src/pubtools/_pulp/tasks/push/items/rpm.py index 082ed484..3f1c7d5a 100644 --- a/src/pubtools/_pulp/tasks/push/items/rpm.py +++ b/src/pubtools/_pulp/tasks/push/items/rpm.py @@ -151,16 +151,25 @@ def fail_if_duplicate(self, pulp_client): Publishing such units would break integrity of repositories when published becuase of collision on origin path. """ crit = Criteria.and_( - Criteria.with_unit_type(RpmUnit, unit_fields=["filename", "cdn_path", "sha256sum"]), + Criteria.with_unit_type( + RpmUnit, unit_fields=["filename", "cdn_path", "sha256sum"] + ), Criteria.with_field("filename", self.pushsource_item.name), ) results = pulp_client.search_content(crit) for item in results: - if item.cdn_path and item.cdn_path == self.cdn_path and item.sha256sum != self.pushsource_item.sha256sum: - msg = "Fatal error: Duplicate RPM present in Pulp: %s, sha256: %s, cdn_path: %s" % ( - item.filename, - item.sha256sum, - item.cdn_path, - ) + if ( + item.cdn_path + and item.cdn_path == self.cdn_path + and item.sha256sum != self.pushsource_item.sha256sum + ): + msg = ( + "Fatal error: Duplicate RPM present in Pulp: %s, sha256: %s, cdn_path: %s" + % ( + item.filename, + item.sha256sum, + item.cdn_path, + ) + ) raise RuntimeError(msg) diff --git a/src/pubtools/_pulp/tasks/push/phase/upload.py b/src/pubtools/_pulp/tasks/push/phase/upload.py index be65b314..99fa5b58 100644 --- a/src/pubtools/_pulp/tasks/push/phase/upload.py +++ b/src/pubtools/_pulp/tasks/push/phase/upload.py @@ -52,8 +52,10 @@ def run(self): for item in self.iter_input(): # We don't want to allow any other operation on units with potentially duplicated origin path. # Limited to units that would be associated to destination repos which would cause origin path collision. - if not constants.ALLOW_DUPLICATE_UNITS and \ - item.pulp_state in [State.MISSING, State.PARTIAL]: + if not constants.ALLOW_DUPLICATE_UNITS and item.pulp_state in [ + State.MISSING, + State.PARTIAL, + ]: item.fail_if_duplicate(self.pulp_client) if item.pulp_state in [State.IN_REPOS, State.PARTIAL, State.NEEDS_UPDATE]: diff --git a/tests/push/test_push_rpm_duplicate.py b/tests/push/test_push_rpm_duplicate.py index 5782ffd3..b724ba05 100644 --- a/tests/push/test_push_rpm_duplicate.py +++ b/tests/push/test_push_rpm_duplicate.py @@ -12,14 +12,14 @@ def test_push_rpm_duplicate_fail( fake_controller, fake_push, command_tester, caplog, monkeypatch ): - """Test that push detects and fails in the case where a RPM with the same NVR but different checksum is pushed to Pulp in the destionation repository.""" + """Test that push detects and fails in the case where a RPM with the same cdn_path but different checksum is pushed to Pulp in the destination repository.""" monkeypatch.setattr(constants, "ALLOW_DUPLICATE_UNITS", False) client = fake_controller.client rpm_dest = client.get_repository("dest1").result() - # Make this file exist but not in all the desired repos. + # Make this file exist. existing_rpm = RpmUnit( name="some-rpm", version="1.0.0", @@ -33,18 +33,19 @@ def test_push_rpm_duplicate_fail( fake_controller.insert_units(rpm_dest, [existing_rpm]) # Unit is now in dest1 repository. - # Set up a pushsource backend which requests push of the RPM with the same NVR/signing key but different checksum + # Set up a pushsource backend which requests push of the RPM with the same cdn_path/NVR/signing key but different checksum + # to different destination repository. Source.register_backend( "test", lambda: [ RpmPushItem( name="some-rpm-1.0.0-1.noarch.rpm", - dest=["dest1", "dest2"], - # different checksum but same GPG key + dest=["dest2"], + # different checksum but same GPG key sha256sum="e823456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef", md5sum=32 * "a", src="fake/path", - signing_key="F21541EB", + signing_key="F21541EB", ), ], ) From ffd2d4486ab55622241b6c684b5d8e3362122343 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 12 Jun 2026 12:02:25 +0000 Subject: [PATCH 3/3] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/push/test_push_rpm_duplicate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/push/test_push_rpm_duplicate.py b/tests/push/test_push_rpm_duplicate.py index b724ba05..9db3a3bc 100644 --- a/tests/push/test_push_rpm_duplicate.py +++ b/tests/push/test_push_rpm_duplicate.py @@ -34,7 +34,7 @@ def test_push_rpm_duplicate_fail( # Unit is now in dest1 repository. # Set up a pushsource backend which requests push of the RPM with the same cdn_path/NVR/signing key but different checksum - # to different destination repository. + # to different destination repository. Source.register_backend( "test", lambda: [