Review: use presence of SigningService at Repo as trigger

[noissue]

pulp_rpm/app/serializers/package.py

@@ -224,15 +224,6 @@ class PackageSerializer(SingleArtifactContentUploadSerializer, ContentChecksumSe
         ),
         read_only=True,
     )
-    sign_package = serializers.BooleanField(
-        help_text=_(
-            "Sign the package using the SigningService associated with the "
-            "Repository this is being upload to."
-        ),
-        default=False,
-        required=False,
-        write_only=True,
-    )
 
     def __init__(self, *args, **kwargs):
         """Initializer for RpmPackageSerializer."""
@@ -325,44 +316,40 @@ class Meta:
                 "size_package",
                 "time_build",
                 "time_file",
-                "sign_package",
             )
         )
         model = Package
 
     def validate(self, data):
         validated_data = super().validate(data)
-        sign_package = validated_data.get("sign_package")
-        temp_uploaded_file = validated_data.get("file")
-        associated_repo = validated_data.get("repository")
-        if sign_package is True:
-            if not temp_uploaded_file:
-                raise serializers.ValidationError(
-                    _("To sign a package on upload, a file must be provided.")
-                )
-            if not associated_repo:
-                raise serializers.ValidationError(
-                    _("To sign a package on upload, you should provide a Repository.")
-                )
-            signing_service_pk = associated_repo.package_signing_service.pk
-            signing_fingerprint = associated_repo.package_signing_fingerprint
-            if not signing_service_pk and not signing_fingerprint:
-                raise serializers.ValidationError(
-                    _(
-                        "To sign a package on upload, the related Repository should have"
-                        "both 'package_signing_service' and 'package_signing_fingerprint' set."
-                    )
+        sign_package = self.context.get("sign_package", None)
+        # choose branch, if not set externally
+        if sign_package is None:
+            sign_package = bool(
+                validated_data.get("repository")
+                and validated_data["repository"].package_signing_service
+            )
+            self.context["sign_package"] = sign_package
+
+        # normal branch
+        if sign_package is False:
+            return validated_data
+
+        # signing branch
+        if not validated_data["repository"].package_signing_fingerprint:
+            raise serializers.ValidationError(
+                _(
+                    "To sign a package on upload, the associated Repository must set both"
+                    "'package_signing_service' and 'package_signing_fingerprint'."
                 )
-            validated_data["signing_service_pk"] = signing_service_pk
-            validated_data["signing_fingerprint"] = signing_fingerprint
-        return validated_data
+            )
 
-    def create(self, validated_data):
-        # clean api-only option before creating model
-        validated_data.pop("sign_package", None)
-        validated_data.pop("signing_service_pk", None)
-        validated_data.pop("signing_fingerprint", None)
-        return super().create(validated_data)
+        if not validated_data.get("file"):
+            raise serializers.ValidationError(
+                _("To sign a package on upload, a file must be provided.")
+            )
+
+        return validated_data
 
 
 class MinimalPackageSerializer(PackageSerializer):

pulp_rpm/app/tasks/signing.py

@@ -36,4 +36,7 @@ def sign_and_create(
 
     # Create Package content
     data["artifact"] = get_url(artifact)
+    # The Package serializer validation method have two branches: the signing and non-signing.
+    # Here, the package is already signed, so we need to update the context for a proper validation.
+    context["sign_package"] = False
     general_create(app_label, serializer_name, data=data, context=context, *args, **kwargs)

pulp_rpm/app/viewsets/package.py

@@ -78,19 +78,18 @@ class PackageViewSet(SingleArtifactContentUploadViewSet):
         responses={202: AsyncOperationResponseSerializer},
     )
     def create(self, request):
-        # normal case
+        # validation decides if we want to sign and set that in the context space
         serializer = self.get_serializer(data=request.data)
         serializer.is_valid(raise_exception=True)
-        sign_package = serializer.validated_data.pop("sign_package")
-        if sign_package is not True:
+        if serializer.context["sign_package"] is False:
             return super().create(request)
 
         # signing case
         request.data.pop("file")
-        request.data.pop("sign_package")
-        temp_uploaded_file = serializer.validated_data.get("file")
-        signing_service_pk = serializer.validated_data.pop("signing_service_pk")
-        signing_fingerprint = serializer.validated_data.pop("signing_fingerprint")
+        validated_data = serializer.validated_data
+        temp_uploaded_file = validated_data["file"]
+        signing_service_pk = validated_data["repository"].package_signing_service.pk
+        signing_fingerprint = validated_data["repository"].package_signing_fingerprint
 
         # dispatch signing task
         pulp_temp_file = PulpTemporaryFile(file=temp_uploaded_file.temporary_file_path())

pulp_rpm/tests/functional/api/test_package_signing.py

@@ -75,9 +75,8 @@ def test_sign_package_on_upload(
     """
     # Setup RPM tool and package to upload
     gpg_a, gpg_b = signing_gpg_extra
-    assert rpm_package_signing_service.pubkey_fingerprint == gpg_a.fingerprint
-    assert rpm_package_signing_service.pubkey_fingerprint != gpg_b.fingerprint
-    fingerprint_set = (gpg_a.fingerprint, gpg_b.fingerprint)
+    fingerprint_set = set([gpg_a.fingerprint, gpg_b.fingerprint])
+    assert len(fingerprint_set) == 2
 
     rpm_tool = RpmTool(tmp_path)
     rpm_tool.import_pubkey_string(gpg_a.pubkey)
@@ -88,7 +87,7 @@ def test_sign_package_on_upload(
     with pytest.raises(InvalidSignatureError, match="The package is not signed: .*"):
         rpm_tool.verify_signature(file_to_upload)
 
-    # Upload Package to Repository with signing-option on
+    # Upload Package to Repository
     # The same file is uploaded, but signed with different keys each time
     for fingerprint in fingerprint_set:
         repository = rpm_repository_factory(
@@ -98,10 +97,9 @@ def test_sign_package_on_upload(
         upload_response = rpm_package_api.create(
             file=str(file_to_upload.absolute()),
             repository=repository.pulp_href,
-            sign_package=True,
         )
-        package_a_href = monitor_task(upload_response.task).created_resources[2]
-        pkg_location_href = rpm_package_api.read(package_a_href).location_href
+        package_href = monitor_task(upload_response.task).created_resources[2]
+        pkg_location_href = rpm_package_api.read(package_href).location_href
 
         # Verify that the final served package is signed
         publication = rpm_publication_factory(repository=repository.pulp_href)

staging_docs/user/guides/06-sign-packages.md

@@ -18,8 +18,10 @@ Sign an RPM Package when uploading it to a Repository.
 
 ### Instructions
 
-1. Create or update a Repository using both `package_signing_service` and `package_signing_fingerprint` params.
-2. Upload a Package with the `sign_package` param set to `True`.
+1. Configure a Repository to enable signing.
+    - Both `package_signing_service` and `package_signing_fingerprint` must be set.
+    - If they are set, any package upload to the Repository will be signed by the service.
+2. Upload a Package to this Repository.
 
 ### Example
 
@@ -28,13 +30,12 @@ Sign an RPM Package when uploading it to a Repository.
 http POST $API_ROOT/repositories/rpm/rpm \
   name="MyRepo" \
   package_signing_service=$SIGNING_SERVICE_HREF \
-  package_signing_fingerprint=$SIGNING_FINGERPRINT \
+  package_signing_fingerprint=$SIGNING_FINGERPRINT
 
-# Upload a package w/ signing enabled
+# Upload a package
 pulp rpm content upload \
   --repository ${REPOSITORY} \
-  --file ${FILE} \
-  --sign-package True
+  --file ${FILE}
 ```
 
 ### Known Limitations