launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #08984
[Merge] lp:~cjwatson/launchpad/pcj-reupload into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/pcj-reupload into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #334858 in Launchpad itself: "Require a way to copy [P]PPA packages into Ubuntu"
https://bugs.launchpad.net/launchpad/+bug/334858
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/pcj-reupload/+merge/111124
== Summary ==
PackageCopyJobs can't unembargo restricted files when copying from private to public archives. The only ways to do this right now are to use unembargo-package.py (which requires direct DB access) or to use a delayed copy (which everyone hates, and which only works with the synchronous Archive.syncSource API). We want Archive.copyPackage to DTRT, which means beefing up PCJs.
== Proposed fix ==
Push the update_files_privacy code down from UnembargoSecurityPackage to _do_direct_copy. Since we want to be careful about doing this by accident, guard this with a new unembargo flag which can be passed to Archive.copyPackage etc.
== Pre-implementation notes ==
I had a conversation with wgrant and others on #launchpad-ops last week, prompted by an attempt by the Ubuntu security team to use Archive.copyPackage for unembargoing (which didn't work because the webapp doesn't have the right security.cfg privileges to use delayed copies). This mostly informed me that everyone hates delayed copies and wants them to die. This branch doesn't do that, but it lays some more of the groundwork.
== Implementation details ==
I had to use a removeSecurityProxy call in update_files_privacy; I think it's OK but would appreciate a double- and indeed triple-check.
I switched the default for allow_delayed_copies in do_copy to False. This required a certain amount of cleanup elsewhere, so I could be persuaded to move that into a separate branch if need be, but I found it useful in making sure that the new direct-copy-based mechanism actually works.
do_copy now takes a logger, because I moved the "Re-uploaded %s to librarian" messages down from UnembargoSecurityPackage. This can probably go away once unembargo-package.py dies.
It wouldn't surprise me if some further fixes were needed to the direct copy mechanism. Please do review this carefully.
== LOC Rationale ==
+181. This is, I believe, a fairly large step on the road to being able to remove at least (a) unembargo-package.py and its associated script class, (b) delayed copies, (c) the ubuntu-security celebrity, and quite possibly (d) copy-package.py and its associated script class. I'm not sure exactly how much all that will come to, but back-of-the-envelope sums indicate somewhere north of 600 lines, not to mention a non-trivial reduction in sheer confusion.
== Tests ==
bin/test -vvct soyuz.tests.test_archive -t test_copypackage -t test_packagecopyjob -t lib/lp/soyuz/stories/ppa/xx-ppa-files.txt
== Demo and Q/A ==
The ultimate straight-through test of all this is:
* Make sure ~ubuntu-security has an ArchivePermission on the security pocket of Ubuntu on dogfood (they do on production).
* Have a member of ~ubuntu-security attempt to use Archive.copyPackage to copy a test package from a private PPA to the -security pocket of a stable Ubuntu distroseries on dogfood.
* Verify that all the files in the copied source and binary publications are on the public librarian.
* Run the publisher to make sure that it is possible to publish this copy.
== Lint ==
False positive due to failure to understand property setters:
./lib/lp/soyuz/model/archive.py
348: redefinition of function 'private' from line 344
Pre-existing lint, not worth fixing up here.
./lib/lp/soyuz/scripts/tests/test_copypackage.py
1462: E501 line too long (82 characters)
--
https://code.launchpad.net/~cjwatson/launchpad/pcj-reupload/+merge/111124
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/pcj-reupload into lp:launchpad.
=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py 2012-06-19 03:26:57 +0000
+++ lib/lp/soyuz/interfaces/archive.py 2012-06-20 00:20:28 +0000
@@ -1277,13 +1277,14 @@
sponsored=Reference(
schema=IPerson,
title=_("Sponsored Person"),
- description=_("The person who is being sponsored for this copy."))
+ description=_("The person who is being sponsored for this copy.")),
+ unembargo=Bool(title=_("Unembargo restricted files")),
)
@export_write_operation()
@operation_for_version('devel')
def copyPackage(source_name, version, from_archive, to_pocket,
person, to_series=None, include_binaries=False,
- sponsored=None):
+ sponsored=None, unembargo=False):
"""Copy a single named source into this archive.
Asynchronously copy a specific version of a named source to the
@@ -1304,6 +1305,9 @@
this will ensure that the person's email address is used as the
"From:" on the announcement email and will also be recorded as
the creator of the new source publication.
+ :param unembargo: if True, allow copying restricted files from a
+ private archive to a public archive, and re-upload them to the
+ public librarian when doing so.
:raises NoSuchSourcePackageName: if the source name is invalid
:raises PocketNotFound: if the pocket name is invalid
@@ -1335,13 +1339,14 @@
sponsored=Reference(
schema=IPerson,
title=_("Sponsored Person"),
- description=_("The person who is being sponsored for this copy."))
+ description=_("The person who is being sponsored for this copy.")),
+ unembargo=Bool(title=_("Unembargo restricted files")),
)
@export_write_operation()
@operation_for_version('devel')
def copyPackages(source_names, from_archive, to_pocket, person,
to_series=None, from_series=None, include_binaries=False,
- sponsored=None):
+ sponsored=None, unembargo=False):
"""Copy multiple named sources into this archive from another.
Asynchronously copy the most recent PUBLISHED versions of the named
@@ -1365,6 +1370,9 @@
this will ensure that the person's email address is used as the
"From:" on the announcement email and will also be recorded as
the creator of the new source publication.
+ :param unembargo: if True, allow copying restricted files from a
+ private archive to a public archive, and re-upload them to the
+ public librarian when doing so.
:raises NoSuchSourcePackageName: if the source name is invalid
:raises PocketNotFound: if the pocket name is invalid
=== modified file 'lib/lp/soyuz/interfaces/packagecopyjob.py'
--- lib/lp/soyuz/interfaces/packagecopyjob.py 2012-01-06 16:23:59 +0000
+++ lib/lp/soyuz/interfaces/packagecopyjob.py 2012-06-20 00:20:28 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2011 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -128,7 +128,7 @@
target_archive, target_distroseries, target_pocket,
include_binaries=False, package_version=None,
copy_policy=PackageCopyPolicy.INSECURE, requester=None,
- sponsored=None):
+ sponsored=None, unembargo=False):
"""Create a new `IPlainPackageCopyJob`.
:param package_name: The name of the source package to copy.
@@ -146,11 +146,12 @@
:param requester: The user requesting the copy.
:param sponsored: The user who is being sponsored to make the copy.
The person who is making this request then becomes the sponsor.
+ :param unembargo: See `do_copy`.
"""
def createMultiple(target_distroseries, copy_tasks, requester,
copy_policy=PackageCopyPolicy.INSECURE,
- include_binaries=False):
+ include_binaries=False, unembargo=False):
"""Create multiple new `IPlainPackageCopyJob`s at once.
:param target_distroseries: The `IDistroSeries` to which to copy the
@@ -161,6 +162,7 @@
:param requester: The user requesting the copy.
:param copy_policy: Applicable `PackageCopyPolicy`.
:param include_binaries: As in `do_copy`.
+ :param unembargo: As in `do_copy`.
:return: An iterable of `PackageCopyJob` ids.
"""
@@ -210,6 +212,10 @@
schema=IPerson, title=_('Sponsored Person'),
required=False, readonly=True)
+ unembargo = Bool(
+ title=_("Unembargo restricted files"),
+ required=False, readonly=True)
+
def addSourceOverride(override):
"""Add an `ISourceOverride` to the metadata."""
=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py 2012-06-19 03:26:57 +0000
+++ lib/lp/soyuz/model/archive.py 2012-06-20 00:20:28 +0000
@@ -1632,7 +1632,7 @@
def copyPackage(self, source_name, version, from_archive, to_pocket,
person, to_series=None, include_binaries=False,
- sponsored=None):
+ sponsored=None, unembargo=False):
"""See `IArchive`."""
self._checkCopyPackageFeatureFlags()
@@ -1653,11 +1653,11 @@
target_pocket=pocket,
package_version=version, include_binaries=include_binaries,
copy_policy=PackageCopyPolicy.INSECURE, requester=person,
- sponsored=sponsored)
+ sponsored=sponsored, unembargo=unembargo)
def copyPackages(self, source_names, from_archive, to_pocket,
person, to_series=None, from_series=None,
- include_binaries=None, sponsored=None):
+ include_binaries=None, sponsored=None, unembargo=False):
"""See `IArchive`."""
self._checkCopyPackageFeatureFlags()
@@ -1698,7 +1698,8 @@
job_source.createMultiple(
series, copy_tasks, person,
copy_policy=PackageCopyPolicy.MASS_SYNC,
- include_binaries=include_binaries, sponsored=sponsored)
+ include_binaries=include_binaries, sponsored=sponsored,
+ unembargo=unembargo)
def _collectLatestPublishedSources(self, from_archive, from_series,
source_names):
@@ -1780,7 +1781,7 @@
# copy packages they wouldn't otherwise be able to.
do_copy(
sources, self, series, pocket, include_binaries, person=person,
- check_permissions=False)
+ check_permissions=False, allow_delayed_copies=True)
def getAuthToken(self, person):
"""See `IArchive`."""
=== modified file 'lib/lp/soyuz/model/packagecopyjob.py'
--- lib/lp/soyuz/model/packagecopyjob.py 2012-06-14 05:18:22 +0000
+++ lib/lp/soyuz/model/packagecopyjob.py 2012-06-20 00:20:28 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2011 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -246,7 +246,7 @@
@classmethod
def _makeMetadata(cls, target_pocket, package_version,
- include_binaries, sponsored=None):
+ include_binaries, sponsored=None, unembargo=False):
"""Produce a metadata dict for this job."""
if sponsored:
sponsored_name = sponsored.name
@@ -257,6 +257,7 @@
'package_version': package_version,
'include_binaries': bool(include_binaries),
'sponsored': sponsored_name,
+ 'unembargo': unembargo,
}
@classmethod
@@ -264,12 +265,13 @@
target_archive, target_distroseries, target_pocket,
include_binaries=False, package_version=None,
copy_policy=PackageCopyPolicy.INSECURE, requester=None,
- sponsored=None):
+ sponsored=None, unembargo=False):
"""See `IPlainPackageCopyJobSource`."""
assert package_version is not None, "No package version specified."
assert requester is not None, "No requester specified."
metadata = cls._makeMetadata(
- target_pocket, package_version, include_binaries, sponsored)
+ target_pocket, package_version, include_binaries, sponsored,
+ unembargo)
job = PackageCopyJob(
job_type=cls.class_job_type,
source_archive=source_archive,
@@ -287,7 +289,7 @@
@classmethod
def _composeJobInsertionTuple(cls, target_distroseries, copy_policy,
include_binaries, job_id, copy_task,
- sponsored):
+ sponsored, unembargo):
"""Create an SQL fragment for inserting a job into the database.
:return: A string representing an SQL tuple containing initializers
@@ -302,7 +304,8 @@
target_pocket,
) = copy_task
metadata = cls._makeMetadata(
- target_pocket, package_version, include_binaries, sponsored)
+ target_pocket, package_version, include_binaries, sponsored,
+ unembargo)
data = (
cls.class_job_type, target_distroseries, copy_policy,
source_archive, target_archive, package_name, job_id,
@@ -312,14 +315,15 @@
@classmethod
def createMultiple(cls, target_distroseries, copy_tasks, requester,
copy_policy=PackageCopyPolicy.INSECURE,
- include_binaries=False, sponsored=None):
+ include_binaries=False, sponsored=None,
+ unembargo=False):
"""See `IPlainPackageCopyJobSource`."""
store = IMasterStore(Job)
job_ids = Job.createMultiple(store, len(copy_tasks), requester)
job_contents = [
cls._composeJobInsertionTuple(
target_distroseries, copy_policy, include_binaries, job_id,
- task, sponsored)
+ task, sponsored, unembargo)
for job_id, task in zip(job_ids, copy_tasks)]
return bulk.create(
(PackageCopyJob.job_type, PackageCopyJob.target_distroseries,
@@ -397,6 +401,10 @@
return None
return getUtility(IPersonSet).getByName(name)
+ @property
+ def unembargo(self):
+ return self.metadata['unembargo']
+
def _createPackageUpload(self, unapproved=False):
pu = self.target_distroseries.createQueueEntry(
pocket=self.target_pocket, archive=self.target_archive,
@@ -550,7 +558,8 @@
include_binaries=self.include_binaries, check_permissions=True,
person=self.requester, overrides=[override],
send_email=send_email, announce_from_person=self.requester,
- sponsored=self.sponsored, packageupload=pu)
+ sponsored=self.sponsored, packageupload=pu,
+ unembargo=self.unembargo)
# Add a PackageDiff for this new upload if it has ancestry.
if ancestry is not None:
=== modified file 'lib/lp/soyuz/scripts/packagecopier.py'
--- lib/lp/soyuz/scripts/packagecopier.py 2012-05-21 07:34:15 +0000
+++ lib/lp/soyuz/scripts/packagecopier.py 2012-06-20 00:20:28 +0000
@@ -23,6 +23,7 @@
import apt_pkg
from lazr.delegates import delegates
from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
from lp.app.errors import NotFoundError
from lp.buildmaster.enums import BuildStatus
@@ -150,9 +151,13 @@
old_lfa.restricted == archive.private or
old_lfa.restricted == False):
continue
- new_lfa = re_upload_file(
- old_lfa, restricted=archive.private)
- setattr(obj, attr_name, new_lfa)
+ new_lfa = re_upload_file(old_lfa, restricted=archive.private)
+ # Most of the attributes set here are not normally editable.
+ # However, since we've just created all the publication records
+ # here, and since we know that the calling user must have access to
+ # the private source archive, we can get away with removing the
+ # security proxy.
+ setattr(removeSecurityProxy(obj), attr_name, new_lfa)
re_uploaded_files.append(new_lfa)
return re_uploaded_files
@@ -240,8 +245,8 @@
Allows the checker function to identify conflicting copy candidates
within the copying batch.
"""
- def __init__(self, archive, include_binaries, allow_delayed_copies=True,
- strict_binaries=True):
+ def __init__(self, archive, include_binaries, allow_delayed_copies=False,
+ strict_binaries=True, unembargo=False):
"""Initialize a copy checker.
:param archive: the target `IArchive`.
@@ -253,11 +258,14 @@
:param strict_binaries: If 'include_binaries' is True then setting
this to True will make the copy fail if binaries cannot be also
copied.
+ :param unembargo: If True, allow copying from a private archive to a
+ public archive.
"""
self.archive = archive
self.include_binaries = include_binaries
self.strict_binaries = strict_binaries
self.allow_delayed_copies = allow_delayed_copies
+ self.unembargo = unembargo
self._inventory = {}
def _getInventoryKey(self, candidate):
@@ -510,10 +518,9 @@
"version older than the %s published in %s" %
(ancestry.displayname, ancestry.distroseries.name))
- delayed = (
- self.allow_delayed_copies and
- not self.archive.private and
- has_restricted_files(source))
+ requires_unembargo = (
+ not self.archive.private and has_restricted_files(source))
+ delayed = self.allow_delayed_copies and requires_unembargo
if delayed:
upload_conflict = getUtility(IPackageUploadSet).findSourceUpload(
@@ -524,16 +531,20 @@
raise CannotCopy(
'same version already uploaded and waiting in '
'ACCEPTED queue')
+ elif requires_unembargo and not self.unembargo:
+ raise CannotCopy(
+ "Cannot copy restricted files to a public archive without "
+ "explicit unembargo option.")
# Copy is approved, update the copy inventory.
self.addCopy(source, delayed)
def do_copy(sources, archive, series, pocket, include_binaries=False,
- allow_delayed_copies=True, person=None, check_permissions=True,
+ allow_delayed_copies=False, person=None, check_permissions=True,
overrides=None, send_email=False, strict_binaries=True,
close_bugs=True, create_dsd_job=True, announce_from_person=None,
- sponsored=None, packageupload=None):
+ sponsored=None, packageupload=None, unembargo=False, logger=None):
"""Perform the complete copy of the given sources incrementally.
Verifies if each copy can be performed using `CopyChecker` and
@@ -554,7 +565,7 @@
copied along with the source.
:param allow_delayed_copies: boolean indicating whether or not private
sources can be copied to public archives using delayed_copies.
- Defaults to True, only set as False in the UnembargoPackage context.
+ Defaults to False.
:param person: the requester `IPerson`.
:param check_permissions: boolean indicating whether or not the
requester's permissions to copy should be checked.
@@ -581,6 +592,10 @@
publishing record will be set to this person.
:param packageupload: The `IPackageUpload` that caused this publication
to be created.
+ :param unembargo: If True, allow copying restricted files from a private
+ archive to a public archive, and re-upload them to the public
+ librarian when doing so.
+ :param logger: An optional logger.
:raise CannotCopy when one or more copies were not allowed. The error
will contain the reason why each copy was denied.
@@ -593,7 +608,7 @@
errors = []
copy_checker = CopyChecker(
archive, include_binaries, allow_delayed_copies,
- strict_binaries=strict_binaries)
+ strict_binaries=strict_binaries, unembargo=unembargo)
for source in sources:
if series is None:
@@ -662,7 +677,7 @@
include_binaries, override, close_bugs=close_bugs,
create_dsd_job=create_dsd_job,
close_bugs_since_version=old_version, creator=creator,
- sponsor=sponsor, packageupload=packageupload)
+ sponsor=sponsor, packageupload=packageupload, logger=logger)
if send_email:
notify(
person, source.sourcepackagerelease, [], [], archive,
@@ -679,7 +694,7 @@
def _do_direct_copy(source, archive, series, pocket, include_binaries,
override=None, close_bugs=True, create_dsd_job=True,
close_bugs_since_version=None, creator=None,
- sponsor=None, packageupload=None):
+ sponsor=None, packageupload=None, logger=None):
"""Copy publishing records to another location.
Copy each item of the given list of `SourcePackagePublishingHistory`
@@ -709,6 +724,7 @@
:param sponsor: the sponsor `IPerson`, if this copy is being sponsored.
:param packageupload: The `IPackageUpload` that caused this publication
to be created.
+ :param logger: An optional logger.
:return: a list of `ISourcePackagePublishingHistory` and
`BinaryPackagePublishingHistory` corresponding to the copied
@@ -747,25 +763,32 @@
else:
source_copy = source_in_destination.first()
- if not include_binaries:
- source_copy.createMissingBuilds()
- return copies
-
- # Copy missing binaries for the matching architectures in the
- # destination series. ISPPH.getBuiltBinaries() return only
- # unique publication per binary package releases (i.e. excludes
- # irrelevant arch-indep publications) and IBPPH.copy is prepared
- # to expand arch-indep publications.
- binary_copies = getUtility(IPublishingSet).copyBinariesTo(
- source.getBuiltBinaries(), series, pocket, archive, policy=policy)
-
- if binary_copies is not None:
- copies.extend(binary_copies)
+ if include_binaries:
+ # Copy missing binaries for the matching architectures in the
+ # destination series. ISPPH.getBuiltBinaries() return only unique
+ # publication per binary package releases (i.e. excludes irrelevant
+ # arch-indep publications) and IBPPH.copy is prepared to expand
+ # arch-indep publications.
+ binary_copies = getUtility(IPublishingSet).copyBinariesTo(
+ source.getBuiltBinaries(), series, pocket, archive, policy=policy)
+
+ if binary_copies is not None:
+ copies.extend(binary_copies)
# Always ensure the needed builds exist in the copy destination
# after copying the binaries.
source_copy.createMissingBuilds()
+ if not archive.private and has_restricted_files(source):
+ # Fix copies by overriding them according to the current ancestry
+ # and re-upload files with privacy mismatch.
+ for pub_record in copies:
+ pub_record.overrideFromAncestry()
+ for new_file in update_files_privacy(pub_record):
+ if logger is not None:
+ logger.info(
+ "Re-uploaded %s to librarian" % new_file.filename)
+
return copies
@@ -864,7 +887,7 @@
usage = '%prog -s warty mozilla-firefox --to-suite hoary'
description = 'MOVE or COPY a published package to another suite.'
- allow_delayed_copies = True
+ allow_delayed_copies = False
def add_my_options(self):
@@ -896,6 +919,11 @@
'--to-partner', dest='to_partner', default=False,
action='store_true', help='Destination set to PARTNER archive.')
+ self.parser.add_option(
+ '--unembargo', dest='unembargo', default=False,
+ action='store_true',
+ help='Allow copying from a private archive to a public archive.')
+
def checkCopyOptions(self):
"""Check if the locations options are sane.
@@ -920,6 +948,27 @@
"Cannot operate with destination PARTNER and PPA "
"simultaneously.")
+ def checkPrivacyOptions(self):
+ """Check privacy-related location options.
+
+ We can copy from a private archive to a public archive, but only
+ with the --unembargo option (to avoid accidents). Unembargoing into
+ the release pocket of a distribution is not permitted.
+ """
+ if (self.location.archive.private and
+ not self.destination.archive.private):
+ if not self.options.unembargo:
+ raise SoyuzScriptError(
+ "Copying from a private archive to a public archive "
+ "requires the --unembargo option.")
+
+ if not self.destination.archive.canModifySuite(
+ self.destination.distroseries, self.destination.pocket):
+ raise SoyuzScriptError(
+ "Can't unembargo into suite '%s' of a distribution." %
+ self.destination.distroseries.getSuite(
+ self.destination.pocket))
+
def mainTask(self):
"""Execute package copy procedure.
@@ -944,6 +993,8 @@
self.setupDestination()
+ self.checkPrivacyOptions()
+
self.logger.info("FROM: %s" % (self.location))
self.logger.info("TO: %s" % (self.destination))
@@ -963,7 +1014,8 @@
sources, self.destination.archive,
self.destination.distroseries, self.destination.pocket,
self.options.include_binaries, self.allow_delayed_copies,
- check_permissions=False)
+ check_permissions=False, unembargo=self.options.unembargo,
+ logger=self.logger)
except CannotCopy, error:
self.logger.error(str(error))
return []
@@ -1017,8 +1069,7 @@
them from the PPA to the Ubuntu archive and re-upload any files
from the restricted librarian into the non-restricted one.
- This script simply wraps up PackageCopier with some nicer options,
- and implements the file re-uploading.
+ This script simply wraps up PackageCopier with some nicer options.
An assumption is made, to reduce the number of command line options,
that packages are always copied between the same distroseries. The user
@@ -1032,7 +1083,6 @@
description = ("Unembargo packages in a private PPA by copying to the "
"specified location and re-uploading any files to the "
"unrestricted librarian.")
- allow_delayed_copies = False
def add_my_options(self):
"""Add -d, -s, dry-run and confirmation options."""
@@ -1050,36 +1100,26 @@
help="Private PPA name.")
def setUpCopierOptions(self):
- """Set up options needed by PackageCopier.
-
- :return: False if there is a problem with the options.
- """
+ """Set up options needed by PackageCopier."""
# Set up the options for PackageCopier that are needed in addition
# to the ones that this class sets up.
self.options.to_partner = False
self.options.to_ppa = False
self.options.partner_archive = None
self.options.include_binaries = True
+ self.options.unembargo = True
self.options.to_distribution = self.options.distribution_name
- from_suite = self.options.suite.split("-")
- if len(from_suite) == 1:
- self.logger.error("Can't unembargo into the release pocket")
- return False
- else:
- # The PackageCopier parent class uses options.suite as the
- # source suite, so we need to override it to remove the
- # pocket since PPAs are pocket-less.
- self.options.to_suite = self.options.suite
- self.options.suite = from_suite[0]
+ # The PackageCopier parent class uses options.suite as the source
+ # suite, so we need to override it to remove the pocket since PPAs
+ # are pocket-less.
+ self.options.to_suite = self.options.suite
+ self.options.suite = self.options.suite.split("-")[0]
self.options.version = None
self.options.component = None
- return True
-
def mainTask(self):
"""Invoke PackageCopier to copy the package(s) and re-upload files."""
- if not self.setUpCopierOptions():
- return None
+ self.setUpCopierOptions()
# Generate the location for PackageCopier after overriding the
# options.
@@ -1088,13 +1128,5 @@
# Invoke the package copy operation.
copies = PackageCopier.mainTask(self)
- # Fix copies by overriding them according to the current ancestry
- # and re-upload files with privacy mismatch.
- for pub_record in copies:
- pub_record.overrideFromAncestry()
- for new_file in update_files_privacy(pub_record):
- self.logger.info(
- "Re-uploaded %s to librarian" % new_file.filename)
-
# Return this for the benefit of the test suite.
return copies
=== modified file 'lib/lp/soyuz/scripts/tests/test_copypackage.py'
--- lib/lp/soyuz/scripts/tests/test_copypackage.py 2012-02-10 09:31:39 +0000
+++ lib/lp/soyuz/scripts/tests/test_copypackage.py 2012-06-20 00:20:28 +0000
@@ -462,7 +462,8 @@
* Finally check whether is a delayed-copy or not according to the
given state.
"""
- copy_checker = CopyChecker(self.archive, include_binaries=False)
+ copy_checker = CopyChecker(
+ self.archive, include_binaries=False, allow_delayed_copies=delayed)
self.assertIs(
None,
copy_checker.checkCopy(
@@ -490,7 +491,8 @@
* Finally check whether is a delayed-copy or not according to the
given state.
"""
- copy_checker = CopyChecker(self.archive, include_binaries=True)
+ copy_checker = CopyChecker(
+ self.archive, include_binaries=True, allow_delayed_copies=delayed)
self.assertIs(
None,
copy_checker.checkCopy(
@@ -1027,24 +1029,22 @@
self.layer.txn.commit()
do_copy(
[source], archive, series, pocket, include_binaries=False,
- check_permissions=False)
+ allow_delayed_copies=True, check_permissions=False)
# Repeating the copy is denied.
- copy_checker = CopyChecker(archive, include_binaries=False)
+ copy_checker = CopyChecker(
+ archive, include_binaries=False, allow_delayed_copies=True)
self.assertRaisesWithContent(
CannotCopy,
'same version already uploaded and waiting in ACCEPTED queue',
copy_checker.checkCopy, source, series, pocket, None, False)
def test_checkCopy_suppressing_delayed_copies(self):
- # `CopyChecker` by default will request delayed-copies when it's
- # the case (restricted files being copied to public archives).
- # However this feature can be turned off, and the operation can
- # be performed as a direct-copy by passing 'allow_delayed_copies'
- # as False when initializing `CopyChecker`.
- # This aspect is currently only used in `UnembargoSecurityPackage`
- # script class, because it performs the file privacy fixes in
- # place.
+ # `CopyChecker` can request delayed-copies by passing
+ # `allow_delayed_copies` as True, which was an old mechanism to
+ # support restricted files being copied to public archives. If this
+ # is disabled, which is the default, the operation will be performed
+ # as a direct-copy.
# Create a private archive with a restricted source publication.
private_archive = self.factory.makeArchive(
@@ -1058,19 +1058,21 @@
series = source.distroseries
pocket = source.pocket
- # Normally `CopyChecker` would store a delayed-copy representing
- # this operation, since restricted files are being copied to
- # public archives.
- copy_checker = CopyChecker(archive, include_binaries=False)
+ # `CopyChecker` can store a delayed-copy representing this
+ # operation, since restricted files are being copied to public
+ # archives.
+ copy_checker = CopyChecker(
+ archive, include_binaries=False, allow_delayed_copies=True)
copy_checker.checkCopy(
source, series, pocket, check_permissions=False)
[checked_copy] = list(copy_checker.getCheckedCopies())
self.assertTrue(checked_copy.delayed)
# When 'allow_delayed_copies' is off, a direct-copy will be
- # scheduled.
+ # scheduled. This requires an explicit option to say that we know
+ # we're going to be exposing previously restricted files.
copy_checker = CopyChecker(
- archive, include_binaries=False, allow_delayed_copies=False)
+ archive, include_binaries=False, unembargo=True)
copy_checker.checkCopy(
source, series, pocket, check_permissions=False)
[checked_copy] = list(copy_checker.getCheckedCopies())
@@ -2010,7 +2012,7 @@
to_distribution='ubuntu', to_suite='hoary',
component=None, from_ppa=None, to_ppa=None,
from_partner=False, to_partner=False,
- confirm_all=True, include_binaries=True):
+ confirm_all=True, include_binaries=True, unembargo=False):
"""Return a PackageCopier instance.
Allow tests to use a set of default options and pass an
@@ -2027,6 +2029,9 @@
if include_binaries:
test_args.append('-b')
+ if unembargo:
+ test_args.append('--unembargo')
+
if sourceversion is not None:
test_args.extend(['-e', sourceversion])
@@ -2923,21 +2928,31 @@
# Run the copy package script storing the logged information.
copy_helper = self.getCopier(
sourcename='foo', from_ppa='joe',
- include_binaries=True, from_suite='hoary', to_suite='hoary')
+ include_binaries=True, from_suite='hoary',
+ to_suite='hoary-security', unembargo=True)
copied = copy_helper.mainTask()
- # The private files are copied via a delayed-copy request.
- self.assertEqual(len(copied), 1)
+ # The private files are copied via a direct-copy request.
+ self.assertEqual(len(copied), 3)
self.assertEqual(
['INFO FROM: joe: hoary-RELEASE',
- 'INFO TO: Primary Archive for Ubuntu Linux: hoary-RELEASE',
+ 'INFO TO: Primary Archive for Ubuntu Linux: hoary-SECURITY',
'INFO Copy candidates:',
'INFO \tfoo 1.0 in hoary',
'INFO \tfoo-bin 1.0 in hoary hppa',
'INFO \tfoo-bin 1.0 in hoary i386',
+ 'INFO Re-uploaded foo_1.0.dsc to librarian',
+ 'INFO Re-uploaded foo_1.0_source.changes to librarian',
+ 'INFO Re-uploaded foo-bin_1.0_all.deb to librarian',
+ 'INFO Re-uploaded foo-bin_1.0_i386.changes to librarian',
+ 'INFO Re-uploaded '
+ 'buildlog_ubuntu-hoary-i386.foo_1.0_FULLYBUILT.txt.gz to '
+ 'librarian',
'INFO Copied:',
- 'INFO \tDelayed copy of foo - 1.0 (source, i386)',
- 'INFO 1 package successfully copied.',
+ 'INFO \tfoo 1.0 in hoary',
+ 'INFO \tfoo-bin 1.0 in hoary hppa',
+ 'INFO \tfoo-bin 1.0 in hoary i386',
+ 'INFO 3 packages successfully copied.',
],
copy_helper.logger.getLogBuffer().splitlines())
@@ -3059,27 +3074,41 @@
def testUnembargoSuite(self):
"""Test that passing different suites works as expected."""
+ # Set up a private PPA.
+ joe = self.factory.makePerson(name="joe")
+ ubuntu = getUtility(IDistributionSet).getByName('ubuntu')
+ self.factory.makeArchive(
+ owner=joe, name='ppa', private=True, distribution=ubuntu)
+
test_args = [
- "--ppa", "cprov",
+ "--ppa", "joe",
"-s", "warty-backports",
"foo",
]
script = UnembargoSecurityPackage(
name='unembargo', test_args=test_args)
- self.assertTrue(script.setUpCopierOptions())
- self.assertEqual(
- script.options.to_suite, "warty-backports",
- "Got %s, expected warty-backports")
+ script.setUpCopierOptions()
+ script.setupLocation()
+ script.setupDestination()
+ script.destination.distroseries.status = SeriesStatus.CURRENT
+ script.checkPrivacyOptions()
+ self.assertEqual("warty-backports", script.options.to_suite)
- # Change the suite to one with the release pocket, it should
- # copy nothing as you're not allowed to unembargo into the
- # release pocket.
- test_args[3] = "hoary"
+ # Change the suite to one with the release pocket. It should copy
+ # nothing as you're not allowed to unembargo into the release
+ # pocket of a stable series.
+ test_args[3] = "warty"
script = UnembargoSecurityPackage(
name='unembargo', test_args=test_args)
script.logger = BufferLogger()
- self.assertFalse(script.setUpCopierOptions())
+ script.setUpCopierOptions()
+ script.setupLocation()
+ script.setupDestination()
+ self.assertRaisesWithContent(
+ SoyuzScriptError,
+ "Can't unembargo into suite 'warty' of a distribution.",
+ script.checkPrivacyOptions)
def testCopyClosesBugs(self):
"""Copying packages closes bugs.
=== modified file 'lib/lp/soyuz/stories/ppa/xx-ppa-files.txt'
--- lib/lp/soyuz/stories/ppa/xx-ppa-files.txt 2012-04-10 14:01:17 +0000
+++ lib/lp/soyuz/stories/ppa/xx-ppa-files.txt 2012-06-20 00:20:28 +0000
@@ -266,17 +266,24 @@
binaries across to no-priv's public ppa.
>>> ignored = login_person(no_priv)
- >>> from lp.soyuz.interfaces.publishing import PackagePublishingPocket
+ >>> from lp.soyuz.interfaces.publishing import (
+ ... ISourcePackagePublishingHistory,
+ ... PackagePublishingPocket,
+ ... )
>>> from lp.soyuz.scripts.packagecopier import do_copy
>>> copies = do_copy(
... no_priv_private_ppa.getPublishedSources(name='test-pkg'),
... no_priv.archive, series=ubuntu['warty'],
... pocket=PackagePublishingPocket.RELEASE,
... include_binaries=True, allow_delayed_copies=False,
- ... person=no_priv)
+ ... person=no_priv, unembargo=True)
+ >>> source_copy = [copy for copy in copies
+ ... if ISourcePackagePublishingHistory.providedBy(copy)
+ ... and copy.source_package_version == "1.0"][0]
+ >>> dsc_file = source_copy.sourcepackagerelease.files[0].libraryfile
- >>> from zope.security.proxy import removeSecurityProxy
- >>> removeSecurityProxy(dsc_file).restricted = False
+ >>> dsc_file.restricted
+ False
>>> file_librarian_url = dsc_file.http_url
>>> file_lp_url = str(
... 'http://launchpad.dev/~no-priv/+archive/ppa/+files/%s' %
=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py 2012-06-13 09:19:07 +0000
+++ lib/lp/soyuz/tests/test_archive.py 2012-06-20 00:20:28 +0000
@@ -2243,18 +2243,20 @@
ubuntu.main_archive.getPublishedSources(
name=source.source_package_name).count())
- def _setup_copy_data(self, target_purpose=None):
+ def _setup_copy_data(self, source_private=False, target_purpose=None,
+ target_status=SeriesStatus.DEVELOPMENT):
if target_purpose is None:
target_purpose = ArchivePurpose.PPA
- source_archive = self.factory.makeArchive()
+ source_archive = self.factory.makeArchive(private=source_private)
target_archive = self.factory.makeArchive(purpose=target_purpose)
source = self.factory.makeSourcePackagePublishingHistory(
archive=source_archive, status=PackagePublishingStatus.PUBLISHED)
- source_name = source.source_package_name
- version = source.source_package_version
+ with person_logged_in(source_archive.owner):
+ source_name = source.source_package_name
+ version = source.source_package_version
to_pocket = PackagePublishingPocket.RELEASE
to_series = self.factory.makeDistroSeries(
- distribution=target_archive.distribution)
+ distribution=target_archive.distribution, status=target_status)
return (source, source_archive, source_name, target_archive,
to_pocket, to_series, version)
@@ -2345,6 +2347,27 @@
to_pocket.name, to_series=to_series.name, include_binaries=False,
person=target_archive.owner)
+ def test_copyPackage_unembargo_creates_unembargo_job(self):
+ (source, source_archive, source_name, target_archive, to_pocket,
+ to_series, version) = self._setup_copy_data(
+ source_private=True, target_purpose=ArchivePurpose.PRIMARY,
+ target_status=SeriesStatus.CURRENT)
+ with person_logged_in(target_archive.distribution.owner):
+ target_archive.newComponentUploader(
+ source_archive.owner, "universe")
+ to_pocket = PackagePublishingPocket.SECURITY
+ with person_logged_in(source_archive.owner):
+ target_archive.copyPackage(
+ source_name, version, source_archive, to_pocket.name,
+ to_series=to_series.name, include_binaries=False,
+ person=source_archive.owner, unembargo=True)
+
+ # There should be one copy job, with the unembargo flag set.
+ job_source = getUtility(IPlainPackageCopyJobSource)
+ copy_job = job_source.getActiveJobs(target_archive).one()
+ self.assertEqual(target_archive, copy_job.target_archive)
+ self.assertTrue(copy_job.unembargo)
+
def test_copyPackages_with_single_package(self):
(source, source_archive, source_name, target_archive, to_pocket,
to_series, version) = self._setup_copy_data()
=== modified file 'lib/lp/soyuz/tests/test_packagecopyjob.py'
--- lib/lp/soyuz/tests/test_packagecopyjob.py 2012-06-14 05:18:22 +0000
+++ lib/lp/soyuz/tests/test_packagecopyjob.py 2012-06-20 00:20:28 +0000
@@ -1100,6 +1100,70 @@
self.assertEqual(BugTaskStatus.FIXRELEASED, bugtask281.status)
self.assertEqual(BugTaskStatus.NEW, bugtask280.status)
+ def test_copying_unembargoes_files(self):
+ # The unembargo flag causes the job to re-upload restricted files to
+ # the public librarian.
+ publisher = SoyuzTestPublisher()
+ publisher.prepareBreezyAutotest()
+ distroseries = publisher.breezy_autotest
+ distroseries.status = SeriesStatus.CURRENT
+
+ target_archive = self.factory.makeArchive(
+ distroseries.distribution, purpose=ArchivePurpose.PRIMARY)
+ source_archive = self.factory.makeArchive(private=True)
+
+ # Publish a package in the source archive.
+ spph = publisher.getPubSource(
+ distroseries=distroseries, sourcename="copyme",
+ version="2.8-1", status=PackagePublishingStatus.PUBLISHED,
+ component='multiverse', section='web',
+ archive=source_archive)
+ for source_file in spph.sourcepackagerelease.files:
+ self.assertTrue(source_file.libraryfile.restricted)
+
+ # Now, run the copy job.
+ source = getUtility(IPlainPackageCopyJobSource)
+ requester = self.factory.makePerson()
+ with person_logged_in(target_archive.owner):
+ target_archive.newPocketUploader(
+ requester, PackagePublishingPocket.SECURITY)
+ job = source.create(
+ package_name="copyme",
+ package_version="2.8-1",
+ source_archive=source_archive,
+ target_archive=target_archive,
+ target_distroseries=distroseries,
+ target_pocket=PackagePublishingPocket.SECURITY,
+ include_binaries=False,
+ unembargo=True,
+ requester=requester)
+ self.assertTrue(job.unembargo)
+
+ # Run the job so it gains a PackageUpload.
+ self.assertRaises(SuspendJobException, self.runJob, job)
+ job.suspend()
+ switch_dbuser("launchpad_main")
+
+ # Accept the upload to release the job then run it.
+ pu = getUtility(IPackageUploadSet).getByPackageCopyJobIDs(
+ [removeSecurityProxy(job).context.id]).one()
+ self.assertEqual(PackageUploadStatus.NEW, pu.status)
+ pu.acceptFromQueue()
+ self.assertEqual(PackageUploadStatus.ACCEPTED, pu.status)
+ self.runJob(job)
+
+ # The job should have set the PU status to DONE:
+ self.assertEqual(PackageUploadStatus.DONE, pu.status)
+
+ # Make sure packages were actually copied.
+ copied_sources = target_archive.getPublishedSources(name="copyme")
+ self.assertIsNot(None, copied_sources.any())
+
+ # Check that files were unembargoed.
+ for copied_source in copied_sources:
+ for source_file in copied_source.sourcepackagerelease.files:
+ self.assertFalse(source_file.libraryfile.restricted)
+
def test_findMatchingDSDs_matches_all_DSDs_for_job(self):
# findMatchingDSDs finds matching DSDs for any of the packages
# in the job.
Follow ups
