launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-fix-buildd-manager-permissions into lp:launchpad.

Commit message:
Grant buildd-manager access to Job, SnapBuildJob, and SnappySeries for scheduling snap store upload jobs.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1587943 in Launchpad itself: "buildd-manager lacks access to new snap-related tables"
  https://bugs.launchpad.net/launchpad/+bug/1587943

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-fix-buildd-manager-permissions/+merge/296237

Grant buildd-manager access to Job, SnapBuildJob, and SnappySeries for scheduling snap store upload jobs.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-fix-buildd-manager-permissions into lp:launchpad.

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2016-05-23 16:00:10 +0000
+++ database/schema/security.cfg	2016-06-01 17:01:29 +0000
@@ -979,6 +979,7 @@
 public.gitref                                 = SELECT
 public.gitrepository                          = SELECT
 public.gpgkey                                 = SELECT
+public.job                                    = SELECT, INSERT
 public.libraryfilealias                       = SELECT, INSERT
 public.libraryfilecontent                     = SELECT, INSERT
 public.livefs                                 = SELECT
@@ -1001,7 +1002,9 @@
 public.snap                                   = SELECT
 public.snaparch                               = SELECT
 public.snapbuild                              = SELECT, UPDATE
+public.snapbuildjob                           = SELECT, INSERT
 public.snapfile                               = SELECT
+public.snappyseries                           = SELECT
 public.sourcepackagename                      = SELECT
 public.sourcepackagepublishinghistory         = SELECT
 public.sourcepackagerecipe                    = SELECT

=== modified file 'lib/lp/buildmaster/tests/test_buildfarmjobbehaviour.py'
--- lib/lp/buildmaster/tests/test_buildfarmjobbehaviour.py	2015-04-20 09:48:57 +0000
+++ lib/lp/buildmaster/tests/test_buildfarmjobbehaviour.py	2016-06-01 17:01:29 +0000
@@ -41,6 +41,7 @@
     TestCase,
     TestCaseWithFactory,
     )
+from lp.testing.dbuser import dbuser
 from lp.testing.factory import LaunchpadObjectFactory
 from lp.testing.fakemethod import FakeMethod
 from lp.testing.layers import (
@@ -271,18 +272,17 @@
         self.assertEquals(
             1, len(os.listdir(os.path.join(self.upload_root, result))))
 
+    @defer.inlineCallbacks
     def test_handleStatus_OK_normal_file(self):
         # A filemap with plain filenames should not cause a problem.
         # The call to handleStatus will attempt to get the file from
         # the slave resulting in a URL error in this test case.
-        def got_status(ignored):
-            self.assertEqual(BuildStatus.UPLOADING, self.build.status)
-            self.assertResultCount(1, "incoming")
-
-        d = self.behaviour.handleStatus(
-            self.build.buildqueue_record, 'OK',
-            {'filemap': {'myfile.py': 'test_file_hash'}})
-        return d.addCallback(got_status)
+        with dbuser(config.builddmaster.dbuser):
+            yield self.behaviour.handleStatus(
+                self.build.buildqueue_record, 'OK',
+                {'filemap': {'myfile.py': 'test_file_hash'}})
+        self.assertEqual(BuildStatus.UPLOADING, self.build.status)
+        self.assertResultCount(1, "incoming")
 
     @defer.inlineCallbacks
     def test_handleStatus_OK_absolute_filepath(self):
@@ -291,9 +291,10 @@
         with ExpectedException(
                 BuildDaemonError,
                 "Build returned a file named '/tmp/myfile.py'."):
-            yield self.behaviour.handleStatus(
-                self.build.buildqueue_record, 'OK',
-                {'filemap': {'/tmp/myfile.py': 'test_file_hash'}})
+            with dbuser(config.builddmaster.dbuser):
+                yield self.behaviour.handleStatus(
+                    self.build.buildqueue_record, 'OK',
+                    {'filemap': {'/tmp/myfile.py': 'test_file_hash'}})
 
     @defer.inlineCallbacks
     def test_handleStatus_OK_relative_filepath(self):
@@ -302,42 +303,38 @@
         with ExpectedException(
                 BuildDaemonError,
                 "Build returned a file named '../myfile.py'."):
-            yield self.behaviour.handleStatus(
-                self.build.buildqueue_record, 'OK',
-                {'filemap': {'../myfile.py': 'test_file_hash'}})
+            with dbuser(config.builddmaster.dbuser):
+                yield self.behaviour.handleStatus(
+                    self.build.buildqueue_record, 'OK',
+                    {'filemap': {'../myfile.py': 'test_file_hash'}})
 
+    @defer.inlineCallbacks
     def test_handleStatus_OK_sets_build_log(self):
         # The build log is set during handleStatus.
         self.assertEqual(None, self.build.log)
-        d = self.behaviour.handleStatus(
-            self.build.buildqueue_record, 'OK',
-            {'filemap': {'myfile.py': 'test_file_hash'}})
-
-        def got_status(ignored):
-            self.assertNotEqual(None, self.build.log)
-
-        return d.addCallback(got_status)
-
+        with dbuser(config.builddmaster.dbuser):
+            yield self.behaviour.handleStatus(
+                self.build.buildqueue_record, 'OK',
+                {'filemap': {'myfile.py': 'test_file_hash'}})
+        self.assertNotEqual(None, self.build.log)
+
+    @defer.inlineCallbacks
     def _test_handleStatus_notifies(self, status):
         # An email notification is sent for a given build status if
         # notifications are allowed for that status.
-
         expected_notification = (
             status in self.behaviour.ALLOWED_STATUS_NOTIFICATIONS)
 
-        def got_status(ignored):
-            if expected_notification:
-                self.failIf(
-                    len(pop_notifications()) == 0,
-                    "No notifications received")
-            else:
-                self.failIf(
-                    len(pop_notifications()) > 0,
-                    "Notifications received")
+        with dbuser(config.builddmaster.dbuser):
+            yield self.behaviour.handleStatus(
+                self.build.buildqueue_record, status, {})
 
-        d = self.behaviour.handleStatus(
-            self.build.buildqueue_record, status, {})
-        return d.addCallback(got_status)
+        if expected_notification:
+            self.assertNotEqual(
+                0, len(pop_notifications()), "Notifications received")
+        else:
+            self.assertEqual(
+                0, len(pop_notifications()), "Notifications received")
 
     def test_handleStatus_DEPFAIL_notifies(self):
         return self._test_handleStatus_notifies("DEPFAIL")
@@ -348,72 +345,71 @@
     def test_handleStatus_PACKAGEFAIL_notifies(self):
         return self._test_handleStatus_notifies("PACKAGEFAIL")
 
+    @defer.inlineCallbacks
     def test_handleStatus_ABORTED_cancels_cancelling(self):
-        self.build.updateStatus(BuildStatus.CANCELLING)
-
-        def got_status(ignored):
-            self.assertEqual(
-                0, len(pop_notifications()), "Notifications received")
-            self.assertEqual(BuildStatus.CANCELLED, self.build.status)
-
-        d = self.behaviour.handleStatus(
-            self.build.buildqueue_record, "ABORTED", {})
-        return d.addCallback(got_status)
+        with dbuser(config.builddmaster.dbuser):
+            self.build.updateStatus(BuildStatus.CANCELLING)
+            yield self.behaviour.handleStatus(
+                self.build.buildqueue_record, "ABORTED", {})
+        self.assertEqual(0, len(pop_notifications()), "Notifications received")
+        self.assertEqual(BuildStatus.CANCELLED, self.build.status)
 
     @defer.inlineCallbacks
     def test_handleStatus_ABORTED_illegal_when_building(self):
         self.builder.vm_host = "fake_vm_host"
         self.behaviour = self.interactor.getBuildBehaviour(
             self.build.buildqueue_record, self.builder, self.slave)
-        self.build.updateStatus(BuildStatus.BUILDING)
-
-        with ExpectedException(
-                BuildDaemonError,
-                "Build returned unexpected status: 'ABORTED'"):
-            yield self.behaviour.handleStatus(
-                self.build.buildqueue_record, "ABORTED", {})
+        with dbuser(config.builddmaster.dbuser):
+            self.build.updateStatus(BuildStatus.BUILDING)
+            with ExpectedException(
+                    BuildDaemonError,
+                    "Build returned unexpected status: 'ABORTED'"):
+                yield self.behaviour.handleStatus(
+                    self.build.buildqueue_record, "ABORTED", {})
 
     @defer.inlineCallbacks
     def test_handleStatus_ABORTED_cancelling_sets_build_log(self):
         # If a build is intentionally cancelled, the build log is set.
         self.assertEqual(None, self.build.log)
-        self.build.updateStatus(BuildStatus.CANCELLING)
-        yield self.behaviour.handleStatus(
-            self.build.buildqueue_record, "ABORTED", {})
+        with dbuser(config.builddmaster.dbuser):
+            self.build.updateStatus(BuildStatus.CANCELLING)
+            yield self.behaviour.handleStatus(
+                self.build.buildqueue_record, "ABORTED", {})
         self.assertNotEqual(None, self.build.log)
 
+    @defer.inlineCallbacks
     def test_date_finished_set(self):
         # The date finished is updated during handleStatus_OK.
         self.assertEqual(None, self.build.date_finished)
-        d = self.behaviour.handleStatus(
-            self.build.buildqueue_record, 'OK',
-            {'filemap': {'myfile.py': 'test_file_hash'}})
-
-        def got_status(ignored):
-            self.assertNotEqual(None, self.build.date_finished)
-
-        return d.addCallback(got_status)
+        with dbuser(config.builddmaster.dbuser):
+            yield self.behaviour.handleStatus(
+                self.build.buildqueue_record, 'OK',
+                {'filemap': {'myfile.py': 'test_file_hash'}})
+        self.assertNotEqual(None, self.build.date_finished)
 
     @defer.inlineCallbacks
     def test_givenback_collection(self):
         with ExpectedException(
                 BuildDaemonError,
                 "Build returned unexpected status: 'GIVENBACK'"):
-            yield self.behaviour.handleStatus(
-                self.build.buildqueue_record, "GIVENBACK", {})
+            with dbuser(config.builddmaster.dbuser):
+                yield self.behaviour.handleStatus(
+                    self.build.buildqueue_record, "GIVENBACK", {})
 
     @defer.inlineCallbacks
     def test_builderfail_collection(self):
         with ExpectedException(
                 BuildDaemonError,
                 "Build returned unexpected status: 'BUILDERFAIL'"):
-            yield self.behaviour.handleStatus(
-                self.build.buildqueue_record, "BUILDERFAIL", {})
+            with dbuser(config.builddmaster.dbuser):
+                yield self.behaviour.handleStatus(
+                    self.build.buildqueue_record, "BUILDERFAIL", {})
 
     @defer.inlineCallbacks
     def test_invalid_status_collection(self):
         with ExpectedException(
                 BuildDaemonError,
                 "Build returned unexpected status: 'BORKED'"):
-            yield self.behaviour.handleStatus(
-                self.build.buildqueue_record, "BORKED", {})
+            with dbuser(config.builddmaster.dbuser):
+                yield self.behaviour.handleStatus(
+                    self.build.buildqueue_record, "BORKED", {})

=== modified file 'lib/lp/snappy/tests/test_snapbuild.py'
--- lib/lp/snappy/tests/test_snapbuild.py	2016-05-16 15:08:30 +0000
+++ lib/lp/snappy/tests/test_snapbuild.py	2016-06-01 17:01:29 +0000
@@ -260,7 +260,8 @@
         self.build.snap.store_upload = True
         self.build.snap.store_secrets = {
             "root": "dummy-root", "discharge": "dummy-discharge"}
-        self.build.updateStatus(BuildStatus.FAILEDTOBUILD)
+        with dbuser(config.builddmaster.dbuser):
+            self.build.updateStatus(BuildStatus.FAILEDTOBUILD)
         self.assertContentEqual([], self.build.store_upload_jobs)
 
     def test_updateStatus_fullybuilt_triggers_store_uploads(self):
@@ -270,7 +271,8 @@
         self.build.snap.store_upload = True
         self.build.snap.store_secrets = {
             "root": "dummy-root", "discharge": "dummy-discharge"}
-        self.build.updateStatus(BuildStatus.FULLYBUILT)
+        with dbuser(config.builddmaster.dbuser):
+            self.build.updateStatus(BuildStatus.FULLYBUILT)
         self.assertEqual(1, len(list(self.build.store_upload_jobs)))
 
     def test_notify_fullybuilt(self):

=== modified file 'lib/lp/snappy/tests/test_snapbuildbehaviour.py'
--- lib/lp/snappy/tests/test_snapbuildbehaviour.py	2016-05-06 09:45:45 +0000
+++ lib/lp/snappy/tests/test_snapbuildbehaviour.py	2016-06-01 17:01:29 +0000
@@ -6,6 +6,7 @@
 __metaclass__ = type
 
 from datetime import datetime
+from textwrap import dedent
 import uuid
 
 import fixtures
@@ -293,15 +294,38 @@
 class MakeSnapBuildMixin:
     """Provide the common makeBuild method returning a queued build."""
 
+    def makeSnap(self):
+        self.useFixture(FeatureFixture({SNAP_FEATURE_FLAG: u"on"}))
+        # We can't use self.pushConfig here since this is used in a
+        # TrialTestCase instance.
+        config_name = self.factory.getUniqueString()
+        config.push(config_name, dedent("""
+            [snappy]
+            store_url: http://sca.example/
+            store_upload_url: http://updown.example/
+            """))
+        self.addCleanup(config.pop, config_name)
+        distroseries = self.factory.makeDistroSeries()
+        snappyseries = self.factory.makeSnappySeries(
+            usable_distro_series=[distroseries])
+        return self.factory.makeSnap(
+            distroseries=distroseries, store_upload=True,
+            store_series=snappyseries,
+            store_name=self.factory.getUniqueUnicode(),
+            store_secrets={
+                "root": "dummy-root", "discharge": "dummy-discharge"})
+
     def makeBuild(self):
-        self.useFixture(FeatureFixture({SNAP_FEATURE_FLAG: u"on"}))
-        build = self.factory.makeSnapBuild(status=BuildStatus.BUILDING)
+        snap = self.makeSnap()
+        build = self.factory.makeSnapBuild(
+            requester=snap.registrant, snap=snap, status=BuildStatus.BUILDING)
         build.queueBuild()
         return build
 
     def makeUnmodifiableBuild(self):
-        self.useFixture(FeatureFixture({SNAP_FEATURE_FLAG: u"on"}))
-        build = self.factory.makeSnapBuild(status=BuildStatus.BUILDING)
+        snap = self.makeSnap()
+        build = self.factory.makeSnapBuild(
+            requester=snap.registrant, snap=snap, status=BuildStatus.BUILDING)
         build.distro_series.status = SeriesStatus.OBSOLETE
         build.queueBuild()
         return build