launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/bpb-external-dependencies into lp:launchpad.

Commit message:
Allow PPA admins to set external_dependencies on individual binary package builds.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #671190 in Launchpad itself: "cannot specify specific chroot for a build"
  https://bugs.launchpad.net/launchpad/+bug/671190

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/bpb-external-dependencies/+merge/281744

Allow PPA admins to set external_dependencies on individual binary package builds, allowing them to bootstrap cyclic dependencies without needing a bootstrap archive sources.list.d file in the chroot.

Using launchpad.Moderate is a little odd, but no other existing permission on that object was suitable, and this doesn't seem completely wrong.  The alternatives that I can think of would be opening this facility up to buildd admins, or introducing a new role-based permission for PPA admins.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/bpb-external-dependencies into lp:launchpad.

=== modified file 'lib/lp/security.py'
--- lib/lp/security.py	2015-12-02 02:15:45 +0000
+++ lib/lp/security.py	2016-01-06 12:39:19 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Security policies for using content objects."""
@@ -1957,6 +1957,24 @@
         return auth_spr.checkUnauthenticated()
 
 
+class ModerateBinaryPackageBuild(ViewBinaryPackageBuild):
+    permission = 'launchpad.Moderate'
+
+    def checkAuthenticated(self, user):
+        # Only people who can see the build and administer its archive can
+        # edit restricted attributes of builds.  (Currently this allows
+        # setting BinaryPackageBuild.external_dependencies; people who can
+        # administer the archive can already achieve the same effect by
+        # setting Archive.external_dependencies.)
+        return (
+            super(ModerateBinaryPackageBuild, self).checkAuthenticated(
+                user) and
+            AdminArchive(self.obj.archive).checkAuthenticated(user))
+
+    def checkUnauthenticated(self, user):
+        return False
+
+
 class ViewTranslationTemplatesBuild(DelegatedAuthorization):
     """Permission to view an `ITranslationTemplatesBuild`.
 

=== modified file 'lib/lp/soyuz/configure.zcml'
--- lib/lp/soyuz/configure.zcml	2015-11-26 15:46:38 +0000
+++ lib/lp/soyuz/configure.zcml	2016-01-06 12:39:19 +0000
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
+<!-- Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
      GNU Affero General Public License version 3 (see the file LICENSE).
 -->
 
@@ -467,11 +467,15 @@
         class="lp.soyuz.model.binarypackagebuild.BinaryPackageBuild">
         <require
             permission="launchpad.View"
-            interface="lp.soyuz.interfaces.binarypackagebuild.IBinaryPackageBuildView"/>
+            interface="lp.soyuz.interfaces.binarypackagebuild.IBinaryPackageBuildView
+                       lp.soyuz.interfaces.binarypackagebuild.IBinaryPackageBuildRestricted"/>
         <require
             permission="launchpad.Edit"
             interface="lp.soyuz.interfaces.binarypackagebuild.IBinaryPackageBuildEdit"/>
         <require
+            permission="launchpad.Moderate"
+            set_schema="lp.soyuz.interfaces.binarypackagebuild.IBinaryPackageBuildRestricted"/>
+        <require
             permission="launchpad.Admin"
             interface="lp.soyuz.interfaces.binarypackagebuild.IBinaryPackageBuildAdmin"/>
     </class>

=== modified file 'lib/lp/soyuz/interfaces/binarypackagebuild.py'
--- lib/lp/soyuz/interfaces/binarypackagebuild.py	2015-05-14 08:50:41 +0000
+++ lib/lp/soyuz/interfaces/binarypackagebuild.py	2016-01-06 12:39:19 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """BinaryPackageBuild interfaces."""
@@ -266,6 +266,25 @@
         """
 
 
+class IBinaryPackageBuildRestricted(Interface):
+    """Restricted `IBinaryPackageBuild` attributes.
+
+    These attributes need launchpad.View to see, and launchpad.Moderate to
+    change.
+    """
+    external_dependencies = exported(
+        Text(
+            title=_("External dependencies"), required=False, readonly=False,
+            description=_(
+                "Newline-separated list of repositories to be used to "
+                "retrieve any external build dependencies when performing "
+                "this build, in the format:\n"
+                "deb http[s]://[user:pass@]<host>[/path] series[-pocket] "
+                "[components]\n"
+                "This is intended for bootstrapping build-dependency loops.")),
+        as_of="devel")
+
+
 class IBinaryPackageBuildAdmin(Interface):
     """A Build interface for items requiring launchpad.Admin."""
 
@@ -277,7 +296,7 @@
 
 class IBinaryPackageBuild(
     IBinaryPackageBuildView, IBinaryPackageBuildEdit,
-    IBinaryPackageBuildAdmin):
+    IBinaryPackageBuildRestricted, IBinaryPackageBuildAdmin):
     """A Build interface"""
     export_as_webservice_entry(singular_name='build', plural_name='builds')
 

=== modified file 'lib/lp/soyuz/model/binarypackagebuild.py'
--- lib/lp/soyuz/model/binarypackagebuild.py	2015-12-08 23:40:07 +0000
+++ lib/lp/soyuz/model/binarypackagebuild.py	2016-01-06 12:39:19 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -86,6 +86,10 @@
     PackagePublishingStatus,
     )
 from lp.soyuz.adapters.buildarch import determine_architectures_to_build
+from lp.soyuz.interfaces.archive import (
+    InvalidExternalDependencies,
+    validate_external_dependencies,
+    )
 from lp.soyuz.interfaces.binarypackagebuild import (
     BuildSetStatus,
     CannotBeRescored,
@@ -149,6 +153,14 @@
         or not processor.supports_nonvirtualized)
 
 
+def storm_validate_external_dependencies(build, attr, value):
+    assert attr == 'external_dependencies'
+    errors = validate_external_dependencies(value)
+    if len(errors) > 0:
+        raise InvalidExternalDependencies(errors)
+    return value
+
+
 @implementer(IBinaryPackageBuild)
 class BinaryPackageBuild(PackageBuildMixin, SQLBase):
     _table = 'BinaryPackageBuild'
@@ -213,6 +225,10 @@
     source_package_name = Reference(
         source_package_name_id, 'SourcePackageName.id')
 
+    external_dependencies = Unicode(
+        name='external_dependencies',
+        validator=storm_validate_external_dependencies)
+
     def getLatestSourcePublication(self):
         from lp.soyuz.model.publishing import SourcePackagePublishingHistory
         store = Store.of(self)

=== modified file 'lib/lp/soyuz/stories/webservice/xx-builds.txt'
--- lib/lp/soyuz/stories/webservice/xx-builds.txt	2015-05-12 01:43:31 +0000
+++ lib/lp/soyuz/stories/webservice/xx-builds.txt	2016-01-06 12:39:19 +0000
@@ -122,6 +122,7 @@
     dependencies: None
     distribution_link: u'http://.../ubuntu'
     duration: u'0:01:20'
+    external_dependencies: None
     pocket: u'Release'
     resource_type_link: u'http://.../#build'
     score: None

=== modified file 'lib/lp/soyuz/tests/test_binarypackagebuild.py'
--- lib/lp/soyuz/tests/test_binarypackagebuild.py	2015-12-08 23:40:07 +0000
+++ lib/lp/soyuz/tests/test_binarypackagebuild.py	2016-01-06 12:39:19 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test Build features."""
@@ -562,6 +562,65 @@
         entry = self.webservice.get(build_url, api_version='devel').jsonBody()
         self.assertEqual(name, entry['source_package_name'])
 
+    def test_external_dependencies_random_user(self):
+        # Normal users can look but not touch.
+        person = self.factory.makePerson()
+        build_url = api_url(self.build)
+        logout()
+        webservice = webservice_for_person(
+            person, permission=OAuthPermission.WRITE_PUBLIC)
+        entry = webservice.get(build_url, api_version="devel").jsonBody()
+        self.assertIsNone(entry["external_dependencies"])
+        response = webservice.patch(
+            entry["self_link"], "application/json",
+            dumps({"external_dependencies": "random"}))
+        self.assertEqual(401, response.status)
+
+    def test_external_dependencies_owner(self):
+        # Normal archive owners can look but not touch.
+        build_url = api_url(self.build)
+        logout()
+        entry = self.webservice.get(build_url, api_version="devel").jsonBody()
+        self.assertIsNone(entry["external_dependencies"])
+        response = self.webservice.patch(
+            entry["self_link"], "application/json",
+            dumps({"external_dependencies": "random"}))
+        self.assertEqual(401, response.status)
+
+    def test_external_dependencies_ppa_owner_invalid(self):
+        # PPA admins can look and touch.
+        ppa_admin_team = getUtility(ILaunchpadCelebrities).ppa_admin
+        ppa_admin = self.factory.makePerson(member_of=[ppa_admin_team])
+        build_url = api_url(self.build)
+        logout()
+        webservice = webservice_for_person(
+            ppa_admin, permission=OAuthPermission.WRITE_PUBLIC)
+        entry = webservice.get(build_url, api_version="devel").jsonBody()
+        self.assertIsNone(entry["external_dependencies"])
+        response = webservice.patch(
+            entry["self_link"], "application/json",
+            dumps({"external_dependencies": "random"}))
+        self.assertEqual(400, response.status)
+        self.assertIn("Invalid external dependencies", response.body)
+
+    def test_external_dependencies_ppa_owner_valid(self):
+        # PPA admins can look and touch.
+        ppa_admin_team = getUtility(ILaunchpadCelebrities).ppa_admin
+        ppa_admin = self.factory.makePerson(member_of=[ppa_admin_team])
+        build_url = api_url(self.build)
+        logout()
+        webservice = webservice_for_person(
+            ppa_admin, permission=OAuthPermission.WRITE_PUBLIC)
+        entry = webservice.get(build_url, api_version="devel").jsonBody()
+        self.assertIsNone(entry["external_dependencies"])
+        dependencies = "deb http://example.org suite components"
+        response = webservice.patch(
+            entry["self_link"], "application/json",
+            dumps({"external_dependencies": dependencies}))
+        self.assertEqual(209, response.status)
+        self.assertEqual(
+            dependencies, response.jsonBody()["external_dependencies"])
+
 
 class TestPostprocessCandidate(TestCaseWithFactory):