launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/ids-pocket-permissions into lp:launchpad.

Commit message:
Copy per-distroseries/pocket permissions when initialising a new series.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1040572 in Launchpad itself: "initialize_distroseries doesn't handle pocket permissions"
  https://bugs.launchpad.net/launchpad/+bug/1040572

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/ids-pocket-permissions/+merge/130355

== Summary ==

''Summarise the problem that you're solving.''

Bug 1040572: initialize_distroseries doesn't copy pocket permissions.  Mostly these are archive-wide and don't need to be copied, but queue admin permissions can be per-distroseries.

== Proposed fix ==

Yet another special case in initialize_distroseries.

== LOC Rationale ==

+34.  On the other hand I have about 5800 lines of credit, and trading some of that off against less manual work when creating a new Ubuntu series seems like a good plan.

== Tests ==

bin/test -vvct lp.soyuz.scripts.tests.test_initialize_distroseries -t lp.soyuz.tests.test_initializedistroseriesjob

== Demo and Q/A ==

Advice welcome.  We could possibly initialise a new series on mawson, but that will take ages; it may not be worth it given that we'll shortly be running it in production and since recent work we should have no problem checking for and undoing any mistakes in copying permissions ...
-- 
https://code.launchpad.net/~cjwatson/launchpad/ids-pocket-permissions/+merge/130355
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/ids-pocket-permissions into lp:launchpad.

=== modified file 'lib/lp/soyuz/scripts/initialize_distroseries.py'
--- lib/lp/soyuz/scripts/initialize_distroseries.py	2012-07-05 09:43:58 +0000
+++ lib/lp/soyuz/scripts/initialize_distroseries.py	2012-10-18 13:45:26 +0000
@@ -282,6 +282,7 @@
         self._set_nominatedarchindep()
         self._copy_packages()
         self._copy_packagesets()
+        self._copy_pocket_permissions()
         self._create_dsds()
         self._set_initialized()
         transaction.commit()
@@ -702,3 +703,19 @@
                 new_series_ps.add(parent_to_child[old_series_child])
             new_series_ps.add(old_series_ps.sourcesIncluded(
                 direct_inclusion=True))
+
+    def _copy_pocket_permissions(self):
+        """Copy per-distroseries/pocket permissions from the parent series."""
+        for parent in self.derivation_parents:
+            if self.distroseries.distribution == parent.distribution:
+                self._store.execute("""
+                    INSERT INTO Archivepermission
+                    (person, permission, archive, packageset, explicit,
+                     pocket, distroseries)
+                    SELECT person, permission, %s, packageset, explicit,
+                           pocket, %s
+                    FROM Archivepermission
+                    WHERE packageset IS NULL AND distroseries = %s
+                    """ % sqlvalues(
+                        self.distroseries.main_archive, self.distroseries.id,
+                        parent.id))

=== modified file 'lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py'
--- lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py	2012-09-27 02:53:00 +0000
+++ lib/lp/soyuz/scripts/tests/test_initialize_distroseries.py	2012-10-18 13:45:26 +0000
@@ -5,12 +5,6 @@
 
 __metaclass__ = type
 
-import os
-import subprocess
-import sys
-
-from testtools.content import Content
-from testtools.content_type import UTF8_TEXT
 import transaction
 from zope.component import getUtility
 
@@ -21,7 +15,6 @@
     )
 from lp.registry.interfaces.distroseriesparent import IDistroSeriesParentSet
 from lp.registry.interfaces.pocket import PackagePublishingPocket
-from lp.services.config import config
 from lp.services.database.lpstorm import IStore
 from lp.services.features.testing import FeatureFixture
 from lp.soyuz.enums import (
@@ -737,6 +730,7 @@
         # copy the archivepermissions.
         parent, unused = self.setupParent()
         uploader = self.factory.makePerson()
+        releaser = self.factory.makePerson()
         test1 = self.factory.makePackageset(
             u'test1', u'test 1 packageset', parent.owner,
             distroseries=parent)
@@ -747,41 +741,64 @@
         archive_permset = getUtility(IArchivePermissionSet)
         archive_permset.newPackagesetUploader(
             parent.main_archive, uploader, test1)
+        archive_permset.newPocketQueueAdmin(
+            parent.main_archive, releaser, PackagePublishingPocket.RELEASE,
+            distroseries=parent)
         # Create child series in the same distribution.
         child = self.factory.makeDistroSeries(
             distribution=parent.distribution,
             previous_series=parent)
         self._fullInitialize([parent], child=child)
+        # Create a third series without any special permissions.
+        third = self.factory.makeDistroSeries(
+            distribution=parent.distribution)
 
-        # The uploader can upload to the new distroseries.
-        self.assertTrue(archive_permset.isSourceUploadAllowed(
-                parent.main_archive, 'udev', uploader,
-                distroseries=parent))
-        self.assertTrue(archive_permset.isSourceUploadAllowed(
-                child.main_archive, 'udev', uploader,
-                distroseries=child))
+        # The uploader can upload to the new distroseries, but not to third.
+        # Likewise, the release team member can administer the release
+        # pocket in the new distroseries, but not in third.
+        for series, allowed in ((parent, True), (child, True), (third, False)):
+            self.assertEqual(
+                allowed,
+                archive_permset.isSourceUploadAllowed(
+                    series.main_archive, 'udev', uploader,
+                    distroseries=series))
+            self.assertEqual(
+                allowed,
+                series.main_archive.canAdministerQueue(
+                    releaser, pocket=PackagePublishingPocket.RELEASE,
+                    distroseries=series))
 
     def test_no_cross_distro_perm_copying(self):
         # No cross-distro archivepermissions copying should happen.
         self.parent, self.parent_das = self.setupParent()
         uploader = self.factory.makePerson()
+        releaser = self.factory.makePerson()
         test1 = getUtility(IPackagesetSet).new(
             u'test1', u'test 1 packageset', self.parent.owner,
             distroseries=self.parent)
         test1.addSources('udev')
-        getUtility(IArchivePermissionSet).newPackagesetUploader(
+        archive_permset = getUtility(IArchivePermissionSet)
+        archive_permset.newPackagesetUploader(
             self.parent.main_archive, uploader, test1)
+        archive_permset.newPocketQueueAdmin(
+            self.parent.main_archive, releaser,
+            PackagePublishingPocket.RELEASE, distroseries=self.parent)
         child = self._fullInitialize([self.parent])
 
-        # The uploader cannot upload to the new distroseries.
-        self.assertTrue(
-            getUtility(IArchivePermissionSet).isSourceUploadAllowed(
-                self.parent.main_archive, 'udev', uploader,
-                distroseries=self.parent))
-        self.assertFalse(
-            getUtility(IArchivePermissionSet).isSourceUploadAllowed(
-                child.main_archive, 'udev', uploader,
-                distroseries=child))
+        # The uploader cannot upload to the new distroseries.  Likewise, the
+        # release team member cannot administer the release pocket in the
+        # new distroseries.
+        for series, allowed in ((self.parent, True), (child, False)):
+            self.assertEqual(
+                allowed,
+                archive_permset.isSourceUploadAllowed(
+                    series.main_archive, 'udev', uploader,
+                    distroseries=series))
+            self.assertEqual(
+                allowed,
+                series.main_archive.canAdministerQueue(
+                    releaser, pocket=PackagePublishingPocket.RELEASE,
+                    distroseries=series))
 
     def test_packageset_owner_preserved_within_distro(self):
         # When initializing a new series within a distro, the copied