launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

The proposal to merge lp:~cjwatson/launchpad/pcj-reupload into lp:launchpad has been updated.

Description changed to:

== 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 ==

+179.  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)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/pcj-reupload/+merge/111124
-- 
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.