launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #00742
[Merge] lp:~salgado/launchpad/vostok-upload-policy into lp:launchpad/devel
Guilherme Salgado has proposed merging lp:~salgado/launchpad/vostok-upload-policy into lp:launchpad/devel with lp:~salgado/launchpad/remove-security-upload-policy as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
This branch moves the code related to validating the upload type from nascentupload to the upload policy, as a separate method. This makes it easy to unit test that code as well as adding more flexibility for subclasses that might want to validate differently.
It also replaces the three can_upload_* variables with a single enum variable.
--
https://code.launchpad.net/~salgado/launchpad/vostok-upload-policy/+merge/33584
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~salgado/launchpad/vostok-upload-policy into lp:launchpad/devel.
=== modified file 'lib/lp/archiveuploader/nascentupload.py'
--- lib/lp/archiveuploader/nascentupload.py 2010-08-24 08:43:51 +0000
+++ lib/lp/archiveuploader/nascentupload.py 2010-08-24 20:37:50 +0000
@@ -146,10 +146,11 @@
UploadError will be raised and sent up to the caller. If this happens
the caller should call the reject method and process a rejection.
"""
+ policy = self.policy
self.logger.debug("Beginning processing.")
try:
- self.policy.setDistroSeriesAndPocket(self.changes.suite_name)
+ policy.setDistroSeriesAndPocket(self.changes.suite_name)
except NotFoundError:
self.reject(
"Unable to find distroseries: %s" % self.changes.suite_name)
@@ -185,36 +186,12 @@
isinstance(self.changes.files[0], CustomUploadFile)):
self.logger.debug("Single Custom Upload detected.")
else:
- if self.sourceful and not self.policy.can_upload_source:
- self.reject("Upload is sourceful, but policy refuses "
- "sourceful uploads.")
-
- elif self.binaryful and not self.policy.can_upload_binaries:
- messages = [
- "Upload rejected because it contains binary packages.",
- "Ensure you are using `debuild -S`, or an equivalent",
- "command, to generate only the source package before",
- "re-uploading.",
- ]
- if self.is_ppa:
- messages.append(
- "See https://help.launchpad.net/Packaging/PPA for more "
- "information.")
- self.reject(" ".join(messages))
-
- elif (self.sourceful and self.binaryful and
- not self.policy.can_upload_mixed):
- self.reject("Upload is source/binary but policy refuses "
- "mixed uploads.")
-
- elif self.sourceful and not self.changes.dsc:
+ policy.validateUploadType(self)
+
+ if self.sourceful and not self.changes.dsc:
self.reject(
"Unable to find the DSC file in the source upload.")
- else:
- # Upload content are consistent with the current policy.
- pass
-
# Apply the overrides from the database. This needs to be done
# before doing component verifications because the component
# actually comes from overrides for packages that are not NEW.
@@ -227,7 +204,7 @@
self.verify_acl()
# Perform policy checks.
- self.policy.checkUpload(self)
+ policy.checkUpload(self)
# That's all folks.
self.logger.debug("Finished checking upload.")
@@ -996,8 +973,6 @@
# so late in the game is that in the
# mixed-upload case we only have a
# sourcepackagerelease to verify here!
- assert self.policy.can_upload_mixed, (
- "Current policy does not allow mixed uploads.")
assert sourcepackagerelease, (
"No sourcepackagerelease was found.")
binary_package_file.verifySourcePackageRelease(
=== modified file 'lib/lp/archiveuploader/tests/__init__.py'
--- lib/lp/archiveuploader/tests/__init__.py 2010-08-20 20:31:18 +0000
+++ lib/lp/archiveuploader/tests/__init__.py 2010-08-24 20:37:50 +0000
@@ -117,6 +117,10 @@
# We require the changes to be signed but not the dsc
self.unsigned_dsc_ok = True
+ def validateUploadType(self, upload):
+ """We accept uploads of any type."""
+ pass
+
def policySpecificChecks(self, upload):
"""Nothing, let it go."""
pass
=== modified file 'lib/lp/archiveuploader/tests/nascentupload-announcements.txt'
--- lib/lp/archiveuploader/tests/nascentupload-announcements.txt 2010-08-24 20:37:48 +0000
+++ lib/lp/archiveuploader/tests/nascentupload-announcements.txt 2010-08-24 20:37:50 +0000
@@ -297,9 +297,10 @@
NEW binary upload to RELEASE pocket via 'sync' policy (we need to
override sync policy to allow binary uploads):
+ >>> from lp.archiveuploader.uploadpolicy import ArchiveUploadType
>>> modified_sync_policy = getPolicy(
... name='sync', distro='ubuntu', distroseries='hoary')
- >>> modified_sync_policy.can_upload_binaries = True
+ >>> modified_sync_policy.accepted_type = ArchiveUploadType.BINARY_ONLY
>>> bar_src = NascentUpload.from_changesfile_path(
... datadir('suite/bar_1.0-1_binary/bar_1.0-1_i386.changes'),
=== modified file 'lib/lp/archiveuploader/tests/nascentupload.txt'
--- lib/lp/archiveuploader/tests/nascentupload.txt 2010-08-20 12:11:30 +0000
+++ lib/lp/archiveuploader/tests/nascentupload.txt 2010-08-24 20:37:50 +0000
@@ -499,9 +499,10 @@
need unsigned changes and binaries uploads (same as security, but also
accepts non-security uploads)
+ >>> from lp.archiveuploader.uploadpolicy import ArchiveUploadType
>>> modified_sync_policy = getPolicy(
... name='sync', distro='ubuntu', distroseries='hoary')
- >>> modified_sync_policy.can_upload_binaries = True
+ >>> modified_sync_policy.accepted_type = ArchiveUploadType.BINARY_ONLY
>>> ed_bin = NascentUpload.from_changesfile_path(
... datadir('split-upload-test/ed_0.2-20_i386.changes'),
=== modified file 'lib/lp/archiveuploader/tests/nascentuploadfile.txt'
--- lib/lp/archiveuploader/tests/nascentuploadfile.txt 2010-08-20 12:11:30 +0000
+++ lib/lp/archiveuploader/tests/nascentuploadfile.txt 2010-08-24 20:37:50 +0000
@@ -531,9 +531,10 @@
The timestamp of the files in the .deb are tested against the policy for being
too new:
+ >>> from lp.archiveuploader.uploadpolicy import ArchiveUploadType
>>> old_only_policy = getPolicy(
... name='insecure', distro='ubuntu', distroseries='hoary')
- >>> old_only_policy.can_upload_binaries = True
+ >>> old_only_policy.accepted_type = ArchiveUploadType.BINARY_ONLY
>>> old_only_policy.future_time_grace = -5 * 365 * 24 * 60 * 60
>>> ed_binary_deb = DebBinaryUploadFile(ed_deb_path,
@@ -549,7 +550,7 @@
>>> new_only_policy = getPolicy(
... name='insecure', distro='ubuntu', distroseries='hoary')
- >>> new_only_policy.can_upload_binaries = True
+ >>> new_only_policy.accepted_type = ArchiveUploadType.BINARY_ONLY
>>> new_only_policy.earliest_year = 2010
>>> ed_binary_deb = DebBinaryUploadFile(ed_deb_path,
... 'e31eeb0b6b3b87e1ea79378df864ffff',
=== modified file 'lib/lp/archiveuploader/tests/test_nascentupload_documentation.py'
--- lib/lp/archiveuploader/tests/test_nascentupload_documentation.py 2010-08-23 13:39:29 +0000
+++ lib/lp/archiveuploader/tests/test_nascentupload_documentation.py 2010-08-24 20:37:50 +0000
@@ -30,6 +30,7 @@
getPolicy,
mock_logger_quiet,
)
+from lp.archiveuploader.uploadpolicy import ArchiveUploadType
from lp.registry.interfaces.distribution import IDistributionSet
from lp.soyuz.interfaces.component import IComponentSet
@@ -52,7 +53,7 @@
def getUploadForBinary(upload_path):
"""Return a NascentUpload object for binaries."""
policy = getPolicy(name='sync', distro='ubuntu', distroseries='hoary')
- policy.can_upload_binaries = True
+ policy.accepted_type = ArchiveUploadType.BINARY_ONLY
return NascentUpload.from_changesfile_path(
datadir(upload_path), policy, mock_logger_quiet)
=== modified file 'lib/lp/archiveuploader/tests/test_ppauploadprocessor.py'
--- lib/lp/archiveuploader/tests/test_ppauploadprocessor.py 2010-08-20 20:31:18 +0000
+++ lib/lp/archiveuploader/tests/test_ppauploadprocessor.py 2010-08-24 20:37:50 +0000
@@ -681,12 +681,9 @@
"bar_1.0-1-mixed", "~name16/ubuntu")
self.processUpload(self.uploadprocessor, upload_dir)
- self.assertEqual(
- self.uploadprocessor.last_processed_upload.rejection_message,
- 'Upload rejected because it contains binary packages. Ensure '
- 'you are using `debuild -S`, or an equivalent command, to '
- 'generate only the source package before re-uploading. See '
- 'https://help.launchpad.net/Packaging/PPA for more information.')
+ self.assertIn(
+ 'Source/binary (i.e. mixed) uploads are not allowed.',
+ self.uploadprocessor.last_processed_upload.rejection_message)
def testPGPSignatureNotPreserved(self):
"""PGP signatures should be removed from PPA .changes files.
=== added file 'lib/lp/archiveuploader/tests/test_uploadpolicy.py'
--- lib/lp/archiveuploader/tests/test_uploadpolicy.py 1970-01-01 00:00:00 +0000
+++ lib/lp/archiveuploader/tests/test_uploadpolicy.py 2010-08-24 20:37:50 +0000
@@ -0,0 +1,110 @@
+#!/usr/bin/python
+#
+# Copyright 2010 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from lp.archiveuploader.uploadpolicy import (
+ AbstractUploadPolicy, ArchiveUploadType)
+from lp.testing import TestCase
+
+
+class TestUploadPolicy_validateUploadType(TestCase):
+ """Test what kind (sourceful/binaryful/mixed) of uploads are accepted."""
+
+ def test_sourceful_accepted(self):
+ policy = make_policy(accepted_type=ArchiveUploadType.SOURCE_ONLY)
+ upload = make_fake_upload(sourceful=True)
+
+ policy.validateUploadType(upload)
+
+ self.assertEquals([], upload.rejections)
+
+ def test_binaryful_accepted(self):
+ policy = make_policy(accepted_type=ArchiveUploadType.BINARY_ONLY)
+ upload = make_fake_upload(binaryful=True)
+
+ policy.validateUploadType(upload)
+
+ self.assertEquals([], upload.rejections)
+
+ def test_mixed_accepted(self):
+ policy = make_policy(accepted_type=ArchiveUploadType.MIXED_ONLY)
+ upload = make_fake_upload(sourceful=True, binaryful=True)
+
+ policy.validateUploadType(upload)
+
+ self.assertEquals([], upload.rejections)
+
+ def test_sourceful_not_accepted(self):
+ policy = make_policy(accepted_type=ArchiveUploadType.BINARY_ONLY)
+ upload = make_fake_upload(sourceful=True)
+
+ policy.validateUploadType(upload)
+
+ self.assertIn(
+ 'Sourceful uploads are not accepted by this policy.',
+ upload.rejections)
+
+ def test_binaryful_not_accepted(self):
+ policy = make_policy(accepted_type=ArchiveUploadType.SOURCE_ONLY)
+ upload = make_fake_upload(binaryful=True)
+
+ policy.validateUploadType(upload)
+
+ self.assertTrue(len(upload.rejections) > 0)
+ self.assertIn(
+ 'Upload rejected because it contains binary packages.',
+ upload.rejections[0])
+
+ def test_mixed_not_accepted(self):
+ policy = make_policy(accepted_type=ArchiveUploadType.SOURCE_ONLY)
+ upload = make_fake_upload(sourceful=True, binaryful=True)
+
+ policy.validateUploadType(upload)
+
+ self.assertIn(
+ 'Source/binary (i.e. mixed) uploads are not allowed.',
+ upload.rejections)
+
+ def test_sourceful_when_only_mixed_accepted(self):
+ policy = make_policy(accepted_type=ArchiveUploadType.MIXED_ONLY)
+ upload = make_fake_upload(sourceful=True, binaryful=False)
+
+ policy.validateUploadType(upload)
+
+ self.assertIn(
+ 'Sourceful uploads are not accepted by this policy.',
+ upload.rejections)
+
+ def test_binaryful_when_only_mixed_accepted(self):
+ policy = make_policy(accepted_type=ArchiveUploadType.MIXED_ONLY)
+ upload = make_fake_upload(sourceful=False, binaryful=True)
+
+ policy.validateUploadType(upload)
+
+ self.assertTrue(len(upload.rejections) > 0)
+ self.assertIn(
+ 'Upload rejected because it contains binary packages.',
+ upload.rejections[0])
+
+
+class FakeNascentUpload:
+
+ def __init__(self, sourceful, binaryful):
+ self.sourceful = sourceful
+ self.binaryful = binaryful
+ self.is_ppa = False
+ self.rejections = []
+
+ def reject(self, msg):
+ self.rejections.append(msg)
+
+
+def make_fake_upload(sourceful=False, binaryful=False):
+ return FakeNascentUpload(sourceful, binaryful)
+
+
+def make_policy(accepted_type):
+ policy = AbstractUploadPolicy()
+ policy.accepted_type = accepted_type
+ return policy
=== modified file 'lib/lp/archiveuploader/uploadpolicy.py'
--- lib/lp/archiveuploader/uploadpolicy.py 2010-08-24 20:37:48 +0000
+++ lib/lp/archiveuploader/uploadpolicy.py 2010-08-24 20:37:50 +0000
@@ -7,6 +7,7 @@
__all__ = [
"AbstractUploadPolicy",
+ "ArchiveUploadType",
"BuildDaemonUploadPolicy",
"findPolicyByName",
"IArchiveUploadPolicy",
@@ -28,6 +29,9 @@
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.registry.interfaces.series import SeriesStatus
+from lazr.enum import EnumeratedType, Item
+
+
# Defined here so that uploadpolicy.py doesn't depend on lp.code.
SOURCE_PACKAGE_RECIPE_UPLOAD_POLICY_NAME = 'recipe'
# Number of seconds in an hour (used later)
@@ -52,6 +56,13 @@
"""
+class ArchiveUploadType(EnumeratedType):
+
+ SOURCE_ONLY = Item("Source only")
+ BINARY_ONLY = Item("Binary only")
+ MIXED_ONLY = Item("Mixed only")
+
+
class AbstractUploadPolicy:
"""Encapsulate the policy of an upload to a launchpad archive.
@@ -63,8 +74,9 @@
"""
implements(IArchiveUploadPolicy)
+ name = 'abstract'
options = None
- name = 'abstract'
+ accepted_type = None # Must be defined in subclasses.
def __init__(self):
"""Prepare a policy..."""
@@ -75,14 +87,43 @@
self.unsigned_changes_ok = False
self.unsigned_dsc_ok = False
self.create_people = True
- self.can_upload_source = True
- self.can_upload_binaries = True
- self.can_upload_mixed = True
# future_time_grace is in seconds. 28800 is 8 hours
self.future_time_grace = 8 * HOURS
# The earliest year we accept in a deb's file's mtime
self.earliest_year = 1984
+ def validateUploadType(self, upload):
+ """Check that the type of the given upload is accepted by this policy.
+
+ When the type (e.g. sourceful, binaryful or mixed) is not accepted,
+ the upload is rejected.
+ """
+ if upload.sourceful and upload.binaryful:
+ if self.accepted_type != ArchiveUploadType.MIXED_ONLY:
+ upload.reject(
+ "Source/binary (i.e. mixed) uploads are not allowed.")
+
+ elif upload.sourceful:
+ if self.accepted_type != ArchiveUploadType.SOURCE_ONLY:
+ upload.reject(
+ "Sourceful uploads are not accepted by this policy.")
+
+ else:
+ assert upload.binaryful, (
+ "Upload is not sourceful, binaryful or mixed.")
+ if self.accepted_type != ArchiveUploadType.BINARY_ONLY:
+ messages = [
+ "Upload rejected because it contains binary packages.",
+ "Ensure you are using `debuild -S`, or an equivalent",
+ "command, to generate only the source package before",
+ "re-uploading.",
+ ]
+ if upload.is_ppa:
+ messages.append(
+ "See https://help.launchpad.net/Packaging/PPA for more "
+ "information.")
+ upload.reject(" ".join(messages))
+
def getUploader(self, changes):
"""Get the person who is doing the uploading."""
return changes.signer
@@ -171,11 +212,7 @@
"""The insecure upload policy is used by the poppy interface."""
name = 'insecure'
-
- def __init__(self):
- AbstractUploadPolicy.__init__(self)
- self.can_upload_binaries = False
- self.can_upload_mixed = False
+ accepted_type = ArchiveUploadType.SOURCE_ONLY
def rejectPPAUploads(self, upload):
"""Insecure policy allows PPA upload."""
@@ -283,14 +320,13 @@
"""The build daemon upload policy is invoked by the slave scanner."""
name = 'buildd'
+ accepted_type = ArchiveUploadType.BINARY_ONLY
def __init__(self):
super(BuildDaemonUploadPolicy, self).__init__()
# We permit unsigned uploads because we trust our build daemons
self.unsigned_changes_ok = True
self.unsigned_dsc_ok = True
- self.can_upload_source = False
- self.can_upload_mixed = False
def setOptions(self, options):
AbstractUploadPolicy.setOptions(self, options)
@@ -314,15 +350,13 @@
"""This policy is invoked when processing sync uploads."""
name = 'sync'
+ accepted_type = ArchiveUploadType.SOURCE_ONLY
def __init__(self):
AbstractUploadPolicy.__init__(self)
# We don't require changes or dsc to be signed for syncs
self.unsigned_changes_ok = True
self.unsigned_dsc_ok = True
- # We don't want binaries in a sync
- self.can_upload_mixed = False
- self.can_upload_binaries = False
def policySpecificChecks(self, upload):
"""Perform sync specific checks."""
=== modified file 'lib/lp/code/model/sourcepackagerecipebuild.py'
--- lib/lp/code/model/sourcepackagerecipebuild.py 2010-08-24 09:51:26 +0000
+++ lib/lp/code/model/sourcepackagerecipebuild.py 2010-08-24 20:37:50 +0000
@@ -40,6 +40,7 @@
from canonical.launchpad.webapp import errorlog
from lp.app.errors import NotFoundError
from lp.archiveuploader.uploadpolicy import (
+ ArchiveUploadType,
BuildDaemonUploadPolicy,
IArchiveUploadPolicy,
SOURCE_PACKAGE_RECIPE_UPLOAD_POLICY_NAME,
@@ -78,11 +79,7 @@
"""Policy for uploading the results of a source package recipe build."""
name = SOURCE_PACKAGE_RECIPE_UPLOAD_POLICY_NAME
-
- def __init__(self):
- super(SourcePackageRecipeUploadPolicy, self).__init__()
- self.can_upload_source = True
- self.can_upload_binaries = False
+ accepted_type = ArchiveUploadType.SOURCE_ONLY
def getUploader(self, changes):
"""Return the person doing the upload."""
=== modified file 'lib/lp/soyuz/browser/tests/distroseriesqueue-views.txt'
--- lib/lp/soyuz/browser/tests/distroseriesqueue-views.txt 2010-08-20 12:11:30 +0000
+++ lib/lp/soyuz/browser/tests/distroseriesqueue-views.txt 2010-08-24 20:37:50 +0000
@@ -253,6 +253,7 @@
... sourcename="foo", distroseries=hoary, version="1.0-2",
... status=PackagePublishingStatus.PUBLISHED)
+ >>> from lp.archiveuploader.uploadpolicy import ArchiveUploadType
>>> from canonical.launchpad.ftests import import_public_test_keys
>>> from canonical.launchpad.interfaces import IComponentSet
>>> from lp.soyuz.model.component import ComponentSelection
@@ -264,7 +265,7 @@
>>> trash = ComponentSelection(distroseries=hoary, component=universe)
>>> sync_policy = getPolicy(name='sync', distro='ubuntu',
... distroseries='hoary')
- >>> sync_policy.can_upload_binaries = True
+ >>> sync_policy.accepted_type = ArchiveUploadType.BINARY_ONLY
>>> foo_upload = NascentUpload.from_changesfile_path(
... datadir('suite/foo_1.0-2_multi_binary/foo_1.0-2_i386.changes'),
... sync_policy, mock_logger_quiet)
