Ensure signing only happens when using *_sign policies

This commit adds two new sync policies, `additive_sign` and
`mirror_content_only_sign`, which are identical to the non `_sign`
versions, but will sign packages during the sync process.

It also adds a test to ensure that signing *doesn't* happen when not using
a `*_sign` policy.

And, because sync is smart enough to skip packages that have identical
nevras, we need to ensure that all packages are garbage collected before
running the tests, so we're adding the delete_orphans_pre fixture to both
tests.

Signed-off-by: Jonathan Dieter <jdieter@ciq.com>

Author: jdieter

pulp_rpm/app/constants.py

@@ -53,14 +53,18 @@
 
 SYNC_POLICIES = SimpleNamespace(
     ADDITIVE="additive",
+    ADDITIVE_SIGN="additive_sign",
     MIRROR_COMPLETE="mirror_complete",
     MIRROR_CONTENT_ONLY="mirror_content_only",
+    MIRROR_CONTENT_ONLY_SIGN="mirror_content_only_sign",
 )
 
 SYNC_POLICY_CHOICES = (
     (SYNC_POLICIES.ADDITIVE, SYNC_POLICIES.ADDITIVE),
+    (SYNC_POLICIES.ADDITIVE_SIGN, SYNC_POLICIES.ADDITIVE_SIGN),
     (SYNC_POLICIES.MIRROR_COMPLETE, SYNC_POLICIES.MIRROR_COMPLETE),
     (SYNC_POLICIES.MIRROR_CONTENT_ONLY, SYNC_POLICIES.MIRROR_CONTENT_ONLY),
+    (SYNC_POLICIES.MIRROR_CONTENT_ONLY_SIGN, SYNC_POLICIES.MIRROR_CONTENT_ONLY_SIGN),
 )
 
 CR_PACKAGE_ATTRS = SimpleNamespace(

pulp_rpm/app/serializers/repository.py

@@ -472,14 +472,16 @@ class RpmRepositorySyncURLSerializer(RepositorySyncURLSerializer):
     )
     sync_policy = serializers.ChoiceField(
         help_text=_(
-            "Options: 'additive', 'mirror_complete', 'mirror_content_only'. Default: 'additive'. "
+            "Options: 'additive', 'additive_sign', 'mirror_complete', 'mirror_content_only', "
+            "'mirror_content_only_sign'. Default: 'additive'. "
             "Modifies how the sync is performed. 'mirror_complete' will clone the original "
             "metadata and create an automatic publication from it, but comes with some "
             "limitations and does not work for certain repositories. 'mirror_content_only' will "
             "change the repository contents to match the remote but the metadata will be "
             "regenerated and will not be bit-for-bit identical. 'additive' will retain the "
             "existing contents of the repository and add the contents of the repository being "
-            "synced."
+            "synced. The '_sign' variants will also sign RPM packages during sync if the "
+            "repository has a signing service configured."
         ),
         choices=SYNC_POLICY_CHOICES,
         required=False,

pulp_rpm/app/tasks/synchronizing.py

@@ -566,7 +566,9 @@ def is_subrepo(directory):
                 namespace=directory,
             )
 
-            dv = RpmDeclarativeVersion(first_stage=stage, repository=repo, mirror=mirror)
+            dv = RpmDeclarativeVersion(
+                first_stage=stage, repository=repo, mirror=mirror, sync_policy=sync_policy
+            )
             repo_version = dv.create() or repo.latest_version()
 
             repo_config["sync_details"]["most_recent_version"] = repo_version.number
@@ -615,6 +617,7 @@ def __init__(self, *args, **kwargs):
 
         Adding it here, because we call RpmDeclarativeVersion multiple times in sync.
         """
+        self.sync_policy = kwargs.pop("sync_policy", None)
         kwargs["acs"] = True
         super().__init__(*args, **kwargs)
 
@@ -642,7 +645,7 @@ def pipeline_stages(self, new_version):
             [
                 ArtifactDownloader(),
                 ArtifactSaver(),
-                RpmArtifactSigningStage(self.repository),
+                RpmArtifactSigningStage(self.repository, self.sync_policy),
                 QueryExistingContents(),
                 RpmContentSaver(),
                 RpmInterrelateContent(),
@@ -1582,15 +1585,17 @@ class RpmArtifactSigningStage(Stage):
     This stage runs after ArtifactSaver, allowing us to sign the synchronized rpms
     """
 
-    def __init__(self, repository):
+    def __init__(self, repository, sync_policy=None):
         """
         Initialize the signing stage.
 
         Args:
             repository: RpmRepository instance that may have signing service configured
+            sync_policy: Sync policy being used for this sync operation
         """
         super().__init__()
         self.repository = repository
+        self.sync_policy = sync_policy
         # Populate the repository package signing service and fingerprint as they can't be
         # populated asynchronously
         _ = self.repository.package_signing_service
@@ -1614,9 +1619,24 @@ async def run(self):
                 f"No package signing service configured for repository {self.repository.name}"
             )
 
+        # Check if this is a re-sign sync policy
+        should_sign = (
+            self.sync_policy
+            in (SYNC_POLICIES.MIRROR_CONTENT_ONLY_SIGN, SYNC_POLICIES.ADDITIVE_SIGN)
+            and signing_service
+            and fingerprint
+        )
+
+        if should_sign:
+            log.info(f"Re-signing RPM packages during sync with policy: {self.sync_policy}")
+        elif signing_service and fingerprint:
+            log.info(
+                f"Signing service configured but not re-signing with policy: {self.sync_policy}"
+            )
+
         async for batch in self.batches():
             for d_content in batch:
-                if signing_service and fingerprint:
+                if should_sign:
                     for d_artifact in d_content.d_artifacts:
                         await self._sign_rpm_artifact(d_artifact, signing_service, fingerprint)
                 await self.put(d_content)

pulp_rpm/app/viewsets/repository.py

@@ -218,10 +218,14 @@ def sync(self, request, pk):
             sync_policy = SYNC_POLICIES.ADDITIVE if not mirror else SYNC_POLICIES.MIRROR_COMPLETE
 
         # validate some invariants that involve repository-wide settings.
-        if sync_policy in (SYNC_POLICIES.MIRROR_COMPLETE, SYNC_POLICIES.MIRROR_CONTENT_ONLY):
+        if sync_policy in (
+            SYNC_POLICIES.MIRROR_COMPLETE,
+            SYNC_POLICIES.MIRROR_CONTENT_ONLY,
+            SYNC_POLICIES.MIRROR_CONTENT_ONLY_SIGN,
+        ):
             err_msg = (
-                "Cannot use '{}' in combination with a 'mirror_complete' or "
-                "'mirror_content_only' sync policy."
+                "Cannot use '{}' in combination with a 'mirror_complete', "
+                "'mirror_content_only' or 'mirror_content_only_sign' sync policy."
             )
             if repository.retain_package_versions > 0:
                 raise DRFValidationError(err_msg.format("retain_package_versions"))

pulp_rpm/tests/functional/api/test_package_signing.py

@@ -237,7 +237,60 @@ def test_sign_chunked_package_on_upload(
         assert rpm_tool.verify_signature(downloaded_package)
 
 
-@pytest.mark.parallel
+def test_no_sign_packages_on_sync_if_additive(
+    init_and_sync,
+    tmp_path,
+    gen_object_with_cleanup,
+    download_content_unit,
+    signing_gpg_extra,
+    rpm_package_signing_service,
+    rpm_package_api,
+    rpm_repository_api,
+    rpm_repository_factory,
+    rpm_publication_factory,
+    rpm_distribution_factory,
+    delete_orphans_pre,
+):
+    """
+    Ensure that sync doesn't sign packages if they're synced with a non `*_sign` policy
+    """
+    from pulp_rpm.app.shared_utils import RpmTool
+
+    # Setup GPG and RPM tool
+    gpg_a, _ = signing_gpg_extra
+    fingerprint = gpg_a.fingerprint
+
+    rpm_tool = RpmTool(tmp_path)
+    rpm_tool.import_pubkey_string(gpg_a.pubkey)
+
+    # Create repository with package signing service configured
+    repository = rpm_repository_factory(
+        package_signing_service=rpm_package_signing_service.pulp_href,
+        package_signing_fingerprint=fingerprint,
+    )
+    init_and_sync(repository=repository)
+
+    # Get synced packages - refresh repository to get latest version
+    updated_repository = rpm_repository_api.read(repository.pulp_href)
+    packages = rpm_package_api.list(repository_version=updated_repository.latest_version_href)
+    assert packages.count > 0, "No packages were synced"
+
+    # Test the first package to verify it was signed during sync
+    test_package = packages.results[0]
+
+    # Verify that the final served package is not signed
+    publication = rpm_publication_factory(repository=repository.pulp_href)
+    distribution = rpm_distribution_factory(publication=publication.pulp_href)
+    downloaded_package = tmp_path / "package.rpm"
+    downloaded_package.write_bytes(
+        download_content_unit(
+            distribution.base_path, get_package_repo_path(test_package.location_href)
+        )
+    )
+    with pytest.raises(InvalidSignatureError, match="The package is not signed: .*"):
+        rpm_tool.verify_signature(downloaded_package)
+
+
 def test_sign_packages_on_sync(
     init_and_sync,
     tmp_path,
@@ -250,9 +303,10 @@ def test_sign_packages_on_sync(
     rpm_repository_factory,
     rpm_publication_factory,
     rpm_distribution_factory,
+    delete_orphans_pre,
 ):
     """
-    Sign an Rpm Package on repo sync endpoint
+    Ensure that sync does sign packages if they're synced with a `*_sign` policy
     """
     from pulp_rpm.app.shared_utils import RpmTool
 
@@ -271,7 +325,7 @@ def test_sign_packages_on_sync(
             package_signing_service=rpm_package_signing_service.pulp_href,
             package_signing_fingerprint=fingerprint,
         )
-        init_and_sync(repository=repository)
+        init_and_sync(repository=repository, sync_policy="additive_sign")
 
         # Get synced packages - refresh repository to get latest version
         updated_repository = rpm_repository_api.read(repository.pulp_href)

pulp_rpm/tests/functional/api/test_sync.py

@@ -716,9 +716,17 @@ def test_sync_modular_static_context(init_and_sync, get_content_summary, get_con
 
 
 @pytest.mark.parallel
-@pytest.mark.parametrize("sync_policy", ["mirror_content_only", "additive"])
+@pytest.mark.parametrize(
+    "sync_policy",
+    [
+        "mirror_content_only",
+        "mirror_content_only_sign",
+        "additive",
+        "additive_sign",
+    ],
+)
 def test_sync_skip_srpm(init_and_sync, sync_policy, get_content):
-    """In mirror_content_only mode, skip_types is allowed."""
+    """In mirror_content_only* mode, skip_types is allowed."""
     repository, _ = init_and_sync(
         url=SRPM_UNSIGNED_FIXTURE_URL, skip_types=["srpm"], sync_policy=sync_policy
     )
@@ -1113,7 +1121,14 @@ def test_additive_mode(init_and_sync, get_content):
 
 
 @pytest.mark.parallel
-@pytest.mark.parametrize("sync_policy", ["mirror_complete", "mirror_content_only"])
+@pytest.mark.parametrize(
+    "sync_policy",
+    [
+        "mirror_complete",
+        "mirror_content_only",
+        "mirror_content_only_sign",
+    ],
+)
 def test_mirror_mode(sync_policy, init_and_sync, rpm_publication_api, get_content_summary):
     """Test of mirror mode."""
     repository, remote = init_and_sync(url=SRPM_UNSIGNED_FIXTURE_URL, policy="on_demand")