launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/remove-ubuntu-security-celebrity into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/remove-ubuntu-security-celebrity/+merge/119934

Ubuntu security update publication has now finally been converted to Archive.copyPackage, and https://code.launchpad.net/~cjwatson/launchpad/pcj-auto-approve-pocket/+merge/119497 will fix the last glitch in this once I can manage to get it landed (which I'll ensure happens before landing this).  The security team have confirmed that they're OK with us removing support for the old system.
-- 
https://code.launchpad.net/~cjwatson/launchpad/remove-ubuntu-security-celebrity/+merge/119934
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/remove-ubuntu-security-celebrity into lp:launchpad.

=== modified file 'lib/lp/app/doc/celebrities.txt'
--- lib/lp/app/doc/celebrities.txt	2012-05-24 20:25:54 +0000
+++ lib/lp/app/doc/celebrities.txt	2012-08-16 13:44:04 +0000
@@ -182,18 +182,6 @@
     True
 
 
-Ubuntu security team
---------------------
-
-There is a celebrity representing the 'ubuntu-security' team, which is
-mainly used for granting special permissions on the ubuntu primary
-archive.
-
-    >>> ubuntu_security = personset.getByName('ubuntu-security')
-    >>> celebs.ubuntu_security == ubuntu_security
-    True
-
-
 Ubuntu technical board
 ----------------------
 

=== modified file 'lib/lp/app/interfaces/launchpad.py'
--- lib/lp/app/interfaces/launchpad.py	2011-12-24 16:54:44 +0000
+++ lib/lp/app/interfaces/launchpad.py	2012-08-16 13:44:04 +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).
 
 """Interfaces for the Launchpad application.
@@ -64,7 +64,6 @@
     ubuntu_archive_mirror = Attribute("The main archive mirror for Ubuntu.")
     ubuntu_bugzilla = Attribute("The Ubuntu Bugzilla.")
     ubuntu_cdimage_mirror = Attribute("The main cdimage mirror for Ubuntu.")
-    ubuntu_security = Attribute("The 'ubuntu-security' team.")
     ubuntu_techboard = Attribute("The Ubuntu technical board.")
     vcs_imports = Attribute("The 'vcs-imports' team.")
 

=== modified file 'lib/lp/app/utilities/celebrities.py'
--- lib/lp/app/utilities/celebrities.py	2011-07-21 22:42:14 +0000
+++ lib/lp/app/utilities/celebrities.py	2012-08-16 13:44:04 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Classes that implement ICelebrity interfaces."""
@@ -151,7 +151,6 @@
     sourceforge_tracker = CelebrityDescriptor(IBugTrackerSet, 'sf')
     ubuntu = CelebrityDescriptor(IDistributionSet, 'ubuntu')
     ubuntu_bugzilla = CelebrityDescriptor(IBugTrackerSet, 'ubuntu-bugzilla')
-    ubuntu_security = PersonCelebrityDescriptor('ubuntu-security')
     ubuntu_techboard = PersonCelebrityDescriptor('techboard')
     vcs_imports = PersonCelebrityDescriptor('vcs-imports')
 

=== modified file 'lib/lp/registry/interfaces/role.py'
--- lib/lp/registry/interfaces/role.py	2012-05-04 10:35:16 +0000
+++ lib/lp/registry/interfaces/role.py	2012-08-16 13:44:04 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 # pylint: disable-msg=E0211,E0213,W0611
@@ -107,9 +107,6 @@
     in_rosetta_experts = Bool(
         title=_("True if this person is a rosetta expert."),
         required=True, readonly=True)
-    in_ubuntu_security = Bool(
-        title=_("True if this person is on the Ubuntu security team."),
-        required=True, readonly=True)
     in_ubuntu_techboard = Bool(
         title=_("True if this person is on the Ubuntu tech board."),
         required=True, readonly=True)

=== modified file 'lib/lp/security.py'
--- lib/lp/security.py	2012-08-14 23:27:07 +0000
+++ lib/lp/security.py	2012-08-16 13:44:04 +0000
@@ -2430,9 +2430,6 @@
     PPA upload rights are managed via `IArchive.checkArchivePermission`;
 
     Appending to PRIMARY, PARTNER or COPY archives is restricted to owners.
-
-    Appending to ubuntu main archives can also be done by the
-    'ubuntu-security' celebrity.
     """
     permission = 'launchpad.Append'
     usedfor = IArchive
@@ -2447,12 +2444,6 @@
         if self.obj.is_ppa and self.obj.checkArchivePermission(user.person):
             return True
 
-        celebrities = getUtility(ILaunchpadCelebrities)
-        if (self.obj.is_main and
-            self.obj.distribution == celebrities.ubuntu and
-            user.in_ubuntu_security):
-            return True
-
         return False
 
 

=== modified file 'lib/lp/soyuz/doc/archive.txt'
--- lib/lp/soyuz/doc/archive.txt	2012-08-14 19:37:38 +0000
+++ lib/lp/soyuz/doc/archive.txt	2012-08-16 13:44:04 +0000
@@ -1882,42 +1882,6 @@
     >>> login('foo.bar@xxxxxxxxxxxxx')
     >>> cprov_archive.enable()
 
-Another important aspect of the upload permission for ubuntu main
-archives (PRIMARY, PARTNER and DEBUG) is that in addition to owners
-and users which were specifically granted permissions, members of the
-ubuntu-security' team also have 'launchpad.Append' on them.
-
-In the sampledata, Carlos does not have permission to append contents
-to the Ubuntu main archives.
-
-    >>> primary, partner, debug = ubuntu.all_distro_archives
-
-    >>> login('carlos@xxxxxxxxxxxxx')
-    >>> check_permission('launchpad.Append', primary)
-    False
-    >>> check_permission('launchpad.Append', partner)
-    False
-    >>> check_permission('launchpad.Append', debug)
-    False
-
-When Carlos becomes a member of the 'ubuntu-security' team he is
-allowed to append to ubuntu main archives. In practice it means that
-Carlos can now *copy* packages directly to ubuntu.
-
-    # Make Carlos a member of the ubuntu-security team.
-    >>> login('foo.bar@xxxxxxxxxxxxx')
-    >>> ubuntu_security = getUtility(IPersonSet).getByName(
-    ...     'ubuntu-security')
-    >>> ignored = ubuntu_security.addMember(carlos, cprov)
-
-    >>> login('carlos@xxxxxxxxxxxxx')
-    >>> check_permission('launchpad.Append', primary)
-    True
-    >>> check_permission('launchpad.Append', partner)
-    True
-    >>> check_permission('launchpad.Append', debug)
-    True
-
 
 Rebuild archives
 ----------------

=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py	2012-08-14 19:37:38 +0000
+++ lib/lp/soyuz/model/archive.py	2012-08-16 13:44:04 +0000
@@ -1823,8 +1823,7 @@
         # Perform the copy, may raise CannotCopy. Don't do any further
         # permission checking: this method is protected by
         # launchpad.Append, which is mostly more restrictive than archive
-        # permissions, except that it also allows ubuntu-security to
-        # copy packages they wouldn't otherwise be able to.
+        # permissions.
         do_copy(
             sources, self, series, pocket, include_binaries, person=person,
             check_permissions=False, allow_delayed_copies=True)

=== modified file 'lib/lp/soyuz/tests/test_archive.py'
--- lib/lp/soyuz/tests/test_archive.py	2012-08-14 23:27:07 +0000
+++ lib/lp/soyuz/tests/test_archive.py	2012-08-16 13:44:04 +0000
@@ -2160,7 +2160,7 @@
         self.assertEqual('universe', filtered.component.name)
 
 
-class TestSyncSourceFeatureFlag(TestCaseWithFactory):
+class TestCopyPackageFeatureFlag(TestCaseWithFactory):
 
     layer = DatabaseFunctionalLayer
 
@@ -2183,45 +2183,16 @@
             None, None, None, None, None)
 
 
-class TestSyncSource(TestCaseWithFactory):
+class TestCopyPackage(TestCaseWithFactory):
 
     layer = DatabaseFunctionalLayer
 
     def setUp(self):
-        super(TestSyncSource, self).setUp()
+        super(TestCopyPackage, self).setUp()
         self.useFixture(FeatureFixture({
             u"soyuz.copypackageppa.enabled": 'on',
             }))
 
-    def test_security_team_can_copy_to_primary(self):
-        # A member of ubuntu-security can use syncSource on any package
-        # in the Ubuntu primary archive, regardless of their normal
-        # upload permissions.
-        # This is until we can open syncSource up more widely and sort
-        # out the permissions that everyone needs.
-        with celebrity_logged_in('admin'):
-            security_person = self.factory.makePerson()
-            getUtility(ILaunchpadCelebrities).ubuntu_security.addMember(
-                security_person, security_person)
-        ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
-        source = self.factory.makeSourcePackagePublishingHistory(
-            archive=self.factory.makeArchive(purpose=ArchivePurpose.PPA),
-            distroseries=ubuntu.currentseries)
-        self.assertEqual(
-            0,
-            ubuntu.main_archive.getPublishedSources(
-                name=source.source_package_name).count())
-        with person_logged_in(security_person):
-            ubuntu.main_archive.syncSource(
-                source_name=source.source_package_name,
-                version=source.source_package_version,
-                from_archive=source.archive,
-                to_pocket='Security')
-        self.assertEqual(
-            1,
-            ubuntu.main_archive.getPublishedSources(
-                name=source.source_package_name).count())
-
     def _setup_copy_data(self, source_private=False, target_purpose=None,
                          target_status=SeriesStatus.DEVELOPMENT):
         if target_purpose is None: