Merge lp:~cjwatson/launchpad/archive-build-score-api into lp:launchpad

[Colin Watson]

Allow launchpad-buildd-admins to set Archive.relative_build_score using the webservice.

== Summary ==

Since non-virtualised PPAs use the same build farm as the primary Ubuntu archive, members of launchpad-buildd-admins sometimes find themselves in the position of arbitrating between things like daily recipe builds and other such continuous integration work in PPAs (e.g. golang, unity) and urgent Ubuntu builds during freezes. We'd like to have the option of being able to modify the relative build scores of archives using the API, so that we can tweak this kind of thing case-by-case with minimal latency. See bug 1002387 for further references.

== Proposed fix ==

Export Archive.relative_build_score, and make it accessible to launchpad-buildd-admins as well as to admins and commercial-admins.

== Implementation details ==

I used a similar launchpad.Moderate implementation as I did for Packageset.relative_build_score (https://code.launchpad.net/~cjwatson/launchpad/packageset-score/+merge/105915), since that happened to be unused. The permission system is getting a bit creaky here.

To compensate for added lines of code, I realised that lib/lp/soyuz/scripts/tests/sync_source_home has been unused since r14690, so I removed it.

A careful review to make sure I didn't screw up the security implementation would be appreciated.

== Tests ==

bin/test -vvct soyuz.tests.test_buildpackagejob

== Demo and Q/A ==

Check that members of launchpad-buildd-admins can read and write the relative_build_job property on an archive using lp-shell. It probably wouldn't hurt to also check that admins and commercial-admins can still do so, and that other users (e.g. the archive owner) still can't.

[Richard Harding (rharding)]

review: Approve (code)

[Curtis Hovey (sinzui)]

Hi Colin.

What are your thoughts about Bug #577478? If you think
88 + description=_(
89 + "A delta to apply to all build scores for the archive. Builds "
90 + "with a higher score will build sooner.")))

is intelligible, then I think we should mark the bug as fixed. If you want to revise the description to make it intelligible, then we can say the bug is fixed with your branch.

[Colin Watson (cjwatson)]

It's fine for buildd-admins, since we're all steeped in this stuff all the time. I have no idea whether commercial-admins find it intelligible.

The "Builds with a higher score will build sooner" bit was added in r12134.2.1, and personally I'd be inclined to say that it addresses Jono's bug.

=== modified file 'lib/lp/security.py'
--- lib/lp/security.py	2012-05-15 16:44:03 +0000
+++ lib/lp/security.py	2012-05-22 12:58:19 +0000
@@ -2414,6 +2414,22 @@
         return False
 
 
+class ModerateArchive(AuthorizationBase):
+    """Restrict changing the build score on archives.
+
+    Buildd admins can change this, as a site-wide resource that requires
+    arbitration, especially between distribution builds and builds in
+    non-virtualized PPAs.  Commercial admins can also change this since it
+    affects the relative priority of (private) PPAs.
+    """
+    permission = 'launchpad.Moderate'
+    usedfor = IArchive
+
+    def checkAuthenticated(self, user):
+        return (user.in_buildd_admin or user.in_commercial_admin or
+                user.in_admin)
+
+
 class ViewArchiveAuthToken(AuthorizationBase):
     """Restrict viewing of archive tokens.
 

=== modified file 'lib/lp/soyuz/configure.zcml'
--- lib/lp/soyuz/configure.zcml	2012-05-16 13:55:07 +0000
+++ lib/lp/soyuz/configure.zcml	2012-05-22 12:58:19 +0000
@@ -391,6 +391,8 @@
         class="lp.soyuz.model.archive.Archive">
         <allow
             interface="lp.soyuz.interfaces.archive.IArchivePublic"/>
+        <allow
+            interface="lp.soyuz.interfaces.archive.IArchiveRestricted"/>
         <require
             permission="launchpad.View"
             interface="lp.soyuz.interfaces.archive.IArchiveView"/>
@@ -413,9 +415,12 @@
             set_attributes="authorized_size build_debug_symbols buildd_secret
                             enabled_restricted_families
                             external_dependencies private
-                            require_virtualized relative_build_score
+                            require_virtualized
                             suppress_subscription_notifications"/>
         <require
+            permission="launchpad.Moderate"
+            set_schema="lp.soyuz.interfaces.archive.IArchiveRestricted"/>
+        <require
             permission="launchpad.Admin"
             set_attributes="distribution name signing_key"/>
     </class>

=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py	2012-05-04 14:52:44 +0000
+++ lib/lp/soyuz/interfaces/archive.py	2012-05-22 12:58:19 +0000
@@ -464,12 +464,6 @@
         title=_('Date created'), required=False, readonly=True,
         description=_("The time when the archive was created."))
 
-    relative_build_score = Int(
-        title=_("Relative build score"), required=True, readonly=False,
-        description=_(
-            "A delta to apply to all build scores for the archive. Builds "
-            "with a higher score will build sooner."))
-
     external_dependencies = exported(
         Text(title=_("External dependencies"), required=False,
         readonly=False, description=_(
@@ -909,7 +903,6 @@
     def getOverridePolicy():
         """Returns an instantiated `IOverridePolicy` for the archive."""
 
-
     buildd_secret = TextLine(
         title=_("Build farm secret"), required=False,
         description=_(
@@ -1710,8 +1703,18 @@
         """
 
 
+class IArchiveRestricted(Interface):
+    """A writeable interface for restricted attributes of archives."""
+
+    relative_build_score = exported(Int(
+        title=_("Relative build score"), required=True, readonly=False,
+        description=_(
+            "A delta to apply to all build scores for the archive. Builds "
+            "with a higher score will build sooner.")))
+
+
 class IArchive(IArchivePublic, IArchiveAppend, IArchiveEdit, IArchiveView,
-               IArchiveCommercial):
+               IArchiveCommercial, IArchiveRestricted):
     """Main Archive interface."""
     export_as_webservice_entry()
 

=== removed directory 'lib/lp/soyuz/scripts/tests/sync_source_home'
=== removed file 'lib/lp/soyuz/scripts/tests/sync_source_home/Debian_incoming_main_Sources'
--- lib/lp/soyuz/scripts/tests/sync_source_home/Debian_incoming_main_Sources	2010-02-04 16:15:47 +0000
+++ lib/lp/soyuz/scripts/tests/sync_source_home/Debian_incoming_main_Sources	1970-01-01 00:00:00 +0000
@@ -1,40 +0,0 @@
-Format: 1.0
-Package: bar
-Binary: bar
-Architecture: any
-Version: 1.0-1
-Maintainer: Launchpad team <[e-mail]>
-Standards-Version: 3.6.2
-Files: 
- 60e9314751eaa969e322698ed5a77f3f 512 bar_1.0-1.dsc
- fc1464e5985b962a042d5354452f361d 164 bar_1.0.orig.tar.gz
- a259bf88aedcdeded4e5d94ad4af3b4a 610 bar_1.0-1.diff.gz
-
-Format: 3.0 (quilt)
-Package: sample1
-Binary: sample1
-Architecture: all
-Version: 1.0-1
-Maintainer: Raphael Hertzog <[e-mail]>
-Standards-Version: 3.8.1
-Checksums-Sha1: 
- 724e3cdad5f4af13383e5d71f525465add1ce897 1741 sample1_1.0-1.dsc
- 296cb89762668772c3ae4799bd9ce7973f4154b2 171 sample1_1.0.orig-component1.tar.bz2
- ccb198be27c065880696cbeb6fb5aebf190c8544 157 sample1_1.0.orig-component2.tar.lzma
- ea3c0c6ee23410a9c454d9b35681a80460343456 201 sample1_1.0.orig-component3.tar.gz
- c62b2496d3ff0c64dcf6488c95e1d5cbc9f5f30d 201 sample1_1.0.orig.tar.gz
- b3c92ffaabc2e3e95e5f74e1de94351fddcb065c 1327 sample1_1.0-1.debian.tar.gz
-Checksums-Sha256: 
- d5390b56d3d5ea301bb565487eded8a6739d29ea5d680389f35b87b30f846412 1741 sample1_1.0-1.dsc
- a0871b73922a228ef8c1f4c1a7e1aef94d8a59d919218242655b7faae352d5c9 171 sample1_1.0.orig-component1.tar.bz2
- 39963ed90d70e918a6cae8d73c81f42dcb7f0dc283a4715f032a1e28b98d1a25 157 sample1_1.0.orig-component2.tar.lzma
- dc174acae810faf5f9a679e042f480905622fe4a854510dad6e200dbe6381c8d 201 sample1_1.0.orig-component3.tar.gz
- 9c0f0b42f6283614b204a47600cb9b0d8b19dfa4cb43c69e6a0a40f3d1064e0e 201 sample1_1.0.orig.tar.gz
- 6864ceeb315ad7b6a59636a0251525cbd8e9b76c6e934f3746271d4ebce5da3c 1327 sample1_1.0-1.debian.tar.gz
-Files: 
- aea108574786aa670dc5ef26dc225be2 1741 sample1_1.0-1.dsc
- 8a90ee77cf3c68c32bdb5c55d40d06f4 171 sample1_1.0.orig-component1.tar.bz2
- 3535d4766df47e531372802a0c2ddb82 157 sample1_1.0.orig-component2.tar.lzma
- 673b42066e8e858208b5d0c7a006c5fb 201 sample1_1.0.orig-component3.tar.gz
- 9047a93bf12a7c749045e9bc9ff9ee99 201 sample1_1.0.orig.tar.gz
- 9404bc99cdb06876a988d4c2262c073e 1327 sample1_1.0-1.debian.tar.gz

=== removed file 'lib/lp/soyuz/scripts/tests/sync_source_home/bar_1.0-1.diff.gz'
Binary files lib/lp/soyuz/scripts/tests/sync_source_home/bar_1.0-1.diff.gz	2008-02-14 12:38:50 +0000 and lib/lp/soyuz/scripts/tests/sync_source_home/bar_1.0-1.diff.gz	1970-01-01 00:00:00 +0000 differ
=== removed file 'lib/lp/soyuz/scripts/tests/sync_source_home/bar_1.0-1.dsc'
--- lib/lp/soyuz/scripts/tests/sync_source_home/bar_1.0-1.dsc	2008-02-14 12:38:50 +0000
+++ lib/lp/soyuz/scripts/tests/sync_source_home/bar_1.0-1.dsc	1970-01-01 00:00:00 +0000
@@ -1,21 +0,0 @@
------BEGIN PGP SIGNED MESSAGE-----
-Hash: SHA1
-
-Format: 1.0
-Source: bar
-Binary: bar
-Architecture: any
-Version: 1.0-1
-Maintainer: Launchpad team <[e-mail]>
-Standards-Version: 3.6.2
-Files: 
- fc1464e5985b962a042d5354452f361d 164 bar_1.0.orig.tar.gz
- a259bf88aedcdeded4e5d94ad4af3b4a 610 bar_1.0-1.diff.gz
-
------BEGIN PGP SIGNATURE-----
-Version: GnuPG v1.4.6 (GNU/Linux)
-
-iD8DBQFHtDONjn63CGxkqMURAoPtAJ0dQMXvjTu0YZVHgGvqGJkhTvqUqwCfV+Pj
-5G9UWQ5qwtNMnC4ZSuiAF6k=
-=zCGh
------END PGP SIGNATURE-----

=== removed file 'lib/lp/soyuz/scripts/tests/sync_source_home/bar_1.0.orig.tar.gz'
Binary files lib/lp/soyuz/scripts/tests/sync_source_home/bar_1.0.orig.tar.gz	2009-04-28 12:59:43 +0000 and lib/lp/soyuz/scripts/tests/sync_source_home/bar_1.0.orig.tar.gz	1970-01-01 00:00:00 +0000 differ
=== removed file 'lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0-1.debian.tar.gz'
Binary files lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0-1.debian.tar.gz	2010-02-04 16:15:47 +0000 and lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0-1.debian.tar.gz	1970-01-01 00:00:00 +0000 differ
=== removed file 'lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0-1.dsc'
--- lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0-1.dsc	2010-02-04 16:15:47 +0000
+++ lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0-1.dsc	1970-01-01 00:00:00 +0000
@@ -1,37 +0,0 @@
------BEGIN PGP SIGNED MESSAGE-----
-Hash: SHA1
-
-Format: 3.0 (quilt)
-Source: sample1
-Binary: sample1
-Architecture: all
-Version: 1.0-1
-Maintainer: Raphael Hertzog <[e-mail]>
-Standards-Version: 3.8.1
-Build-Depends: quilt, debhelper (>= 7)
-Checksums-Sha1: 
- 296cb89762668772c3ae4799bd9ce7973f4154b2 171 sample1_1.0.orig-component1.tar.bz2
- ccb198be27c065880696cbeb6fb5aebf190c8544 157 sample1_1.0.orig-component2.tar.lzma
- ea3c0c6ee23410a9c454d9b35681a80460343456 201 sample1_1.0.orig-component3.tar.gz
- c62b2496d3ff0c64dcf6488c95e1d5cbc9f5f30d 201 sample1_1.0.orig.tar.gz
- b3c92ffaabc2e3e95e5f74e1de94351fddcb065c 1327 sample1_1.0-1.debian.tar.gz
-Checksums-Sha256: 
- a0871b73922a228ef8c1f4c1a7e1aef94d8a59d919218242655b7faae352d5c9 171 sample1_1.0.orig-component1.tar.bz2
- 39963ed90d70e918a6cae8d73c81f42dcb7f0dc283a4715f032a1e28b98d1a25 157 sample1_1.0.orig-component2.tar.lzma
- dc174acae810faf5f9a679e042f480905622fe4a854510dad6e200dbe6381c8d 201 sample1_1.0.orig-component3.tar.gz
- 9c0f0b42f6283614b204a47600cb9b0d8b19dfa4cb43c69e6a0a40f3d1064e0e 201 sample1_1.0.orig.tar.gz
- 6864ceeb315ad7b6a59636a0251525cbd8e9b76c6e934f3746271d4ebce5da3c 1327 sample1_1.0-1.debian.tar.gz
-Files: 
- 8a90ee77cf3c68c32bdb5c55d40d06f4 171 sample1_1.0.orig-component1.tar.bz2
- 3535d4766df47e531372802a0c2ddb82 157 sample1_1.0.orig-component2.tar.lzma
- 673b42066e8e858208b5d0c7a006c5fb 201 sample1_1.0.orig-component3.tar.gz
- 9047a93bf12a7c749045e9bc9ff9ee99 201 sample1_1.0.orig.tar.gz
- 9404bc99cdb06876a988d4c2262c073e 1327 sample1_1.0-1.debian.tar.gz
-
------BEGIN PGP SIGNATURE-----
-Version: GnuPG v1.4.10 (GNU/Linux)
-
-iEYEARECAAYFAktq7/kACgkQPa9Uoh7vUnahEQCgmRTupXtrHegzFOb5jQMOCSXS
-/p4AoIroJtmjVryFpWqAABZTFGjeWo85
-=vwh8
------END PGP SIGNATURE-----

=== removed file 'lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0.orig-component1.tar.bz2'
Binary files lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0.orig-component1.tar.bz2	2010-02-04 16:15:47 +0000 and lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0.orig-component1.tar.bz2	1970-01-01 00:00:00 +0000 differ
=== removed file 'lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0.orig-component2.tar.lzma'
Binary files lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0.orig-component2.tar.lzma	2010-02-04 16:15:47 +0000 and lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0.orig-component2.tar.lzma	1970-01-01 00:00:00 +0000 differ
=== removed file 'lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0.orig-component3.tar.gz'
Binary files lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0.orig-component3.tar.gz	2010-02-04 16:15:47 +0000 and lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0.orig-component3.tar.gz	1970-01-01 00:00:00 +0000 differ
=== removed file 'lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0.orig.tar.gz'
Binary files lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0.orig.tar.gz	2010-02-04 16:15:47 +0000 and lib/lp/soyuz/scripts/tests/sync_source_home/sample1_1.0.orig.tar.gz	1970-01-01 00:00:00 +0000 differ
=== modified file 'lib/lp/soyuz/tests/test_buildpackagejob.py'
--- lib/lp/soyuz/tests/test_buildpackagejob.py	2012-05-17 16:24:42 +0000
+++ lib/lp/soyuz/tests/test_buildpackagejob.py	2012-05-22 12:58:19 +0000
@@ -13,9 +13,9 @@
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
+from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.buildmaster.enums import BuildStatus
 from lp.buildmaster.interfaces.builder import IBuilderSet
-from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.registry.interfaces.sourcepackage import SourcePackageUrgency
 from lp.services.webapp.interfaces import (
@@ -41,7 +41,9 @@
 from lp.soyuz.model.processor import ProcessorFamilySet
 from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
 from lp.testing import (
+    anonymous_logged_in,
     api_url,
+    person_logged_in,
     TestCaseWithFactory,
     )
 from lp.testing.layers import (
@@ -380,44 +382,81 @@
         removeSecurityProxy(packageset).relative_build_score = 100
         self.assertCorrectScore(job, "RELEASE", "main", "low", 100)
 
+    def assertScoreReadableByAnyone(self, obj):
+        """An object's build score is readable by anyone."""
+        with person_logged_in(obj.owner):
+            obj_url = api_url(obj)
+        removeSecurityProxy(obj).relative_build_score = 100
+        webservice = webservice_for_person(
+            self.factory.makePerson(), permission=OAuthPermission.WRITE_PUBLIC)
+        entry = webservice.get(obj_url, api_version="devel").jsonBody()
+        self.assertEqual(100, entry["relative_build_score"])
+
+    def assertScoreNotWriteableByOwner(self, obj):
+        """Being an object's owner does not allow changing its build score.
+
+        This affects a site-wide resource, and is thus restricted to
+        launchpad-buildd-admins.
+        """
+        with person_logged_in(obj.owner):
+            obj_url = api_url(obj)
+        webservice = webservice_for_person(
+            obj.owner, permission=OAuthPermission.WRITE_PUBLIC)
+        entry = webservice.get(obj_url, api_version="devel").jsonBody()
+        response = webservice.patch(
+            entry["self_link"], "application/json",
+            dumps(dict(relative_build_score=100)))
+        self.assertEqual(401, response.status)
+        new_entry = webservice.get(obj_url, api_version="devel").jsonBody()
+        self.assertEqual(0, new_entry["relative_build_score"])
+
+    def assertScoreWriteableByTeam(self, obj, team):
+        """Members of TEAM can change an object's build score."""
+        with person_logged_in(obj.owner):
+            obj_url = api_url(obj)
+        person = self.factory.makePerson(member_of=[team])
+        webservice = webservice_for_person(
+            person, permission=OAuthPermission.WRITE_PUBLIC)
+        entry = webservice.get(obj_url, api_version="devel").jsonBody()
+        response = webservice.patch(
+            entry["self_link"], "application/json",
+            dumps(dict(relative_build_score=100)))
+        self.assertEqual(209, response.status)
+        self.assertEqual(100, response.jsonBody()["relative_build_score"])
+
     def test_score_packageset_readable(self):
         # A packageset's build score is readable by anyone.
         packageset = self.factory.makePackageset()
-        removeSecurityProxy(packageset).relative_build_score = 100
-        webservice = webservice_for_person(
-            self.factory.makePerson(), permission=OAuthPermission.WRITE_PUBLIC)
-        entry = webservice.get(
-            api_url(packageset), api_version="devel").jsonBody()
-        self.assertEqual(100, entry["relative_build_score"])
+        self.assertScoreReadableByAnyone(packageset)
 
     def test_score_packageset_forbids_non_buildd_admin(self):
         # Being the owner of a packageset is not enough to allow changing
         # its build score, since this affects a site-wide resource.
-        person = self.factory.makePerson()
-        packageset = self.factory.makePackageset(owner=person)
-        webservice = webservice_for_person(
-            person, permission=OAuthPermission.WRITE_PUBLIC)
-        entry = webservice.get(
-            api_url(packageset), api_version="devel").jsonBody()
-        response = webservice.patch(
-            entry["self_link"], "application/json",
-            dumps(dict(relative_build_score=100)))
-        self.assertEqual(401, response.status)
-        new_entry = webservice.get(
-            api_url(packageset), api_version="devel").jsonBody()
-        self.assertEqual(0, new_entry["relative_build_score"])
+        packageset = self.factory.makePackageset()
+        self.assertScoreNotWriteableByOwner(packageset)
 
     def test_score_packageset_allows_buildd_admin(self):
-        buildd_admins = getUtility(IPersonSet).getByName(
-            "launchpad-buildd-admins")
-        buildd_admin = self.factory.makePerson(member_of=[buildd_admins])
+        # Buildd admins can change a packageset's build score.
         packageset = self.factory.makePackageset()
-        webservice = webservice_for_person(
-            buildd_admin, permission=OAuthPermission.WRITE_PUBLIC)
-        entry = webservice.get(
-            api_url(packageset), api_version="devel").jsonBody()
-        response = webservice.patch(
-            entry["self_link"], "application/json",
-            dumps(dict(relative_build_score=100)))
-        self.assertEqual(209, response.status)
-        self.assertEqual(100, response.jsonBody()["relative_build_score"])
+        self.assertScoreWriteableByTeam(
+            packageset, getUtility(ILaunchpadCelebrities).buildd_admin)
+
+    def test_score_archive_readable(self):
+        # An archive's build score is readable by anyone.
+        archive = self.factory.makeArchive()
+        self.assertScoreReadableByAnyone(archive)
+
+    def test_score_archive_forbids_non_buildd_admin(self):
+        # Being the owner of an archive is not enough to allow changing its
+        # build score, since this affects a site-wide resource.
+        archive = self.factory.makeArchive()
+        self.assertScoreNotWriteableByOwner(archive)
+
+    def test_score_archive_allows_buildd_and_commercial_admin(self):
+        # Buildd and commercial admins can change an archive's build score.
+        archive = self.factory.makeArchive()
+        self.assertScoreWriteableByTeam(
+            archive, getUtility(ILaunchpadCelebrities).buildd_admin)
+        with anonymous_logged_in():
+            self.assertScoreWriteableByTeam(
+                archive, getUtility(ILaunchpadCelebrities).commercial_admin)