launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/request-daily-builds-permissions into lp:launchpad.

Commit message:
Adjust request-daily-builds DB permissions to handle recent changes in how snap builds are requested.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/request-daily-builds-permissions/+merge/351871

Most of the work here is in making the functional tests extensive enough to be able to catch this.  Aside from that, granting some extra SELECT permissions to particular scripts is usually fairly trivial.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/request-daily-builds-permissions into lp:launchpad.

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2018-06-15 12:54:27 +0000
+++ database/schema/security.cfg	2018-07-31 13:12:41 +0000
@@ -799,11 +799,14 @@
 public.distribution                             = SELECT
 public.distroarchseries                         = SELECT
 public.distroseries                             = SELECT
+public.gitref                                   = SELECT
+public.gitrepository                            = SELECT
 public.job                                      = SELECT, INSERT
 public.libraryfilealias                         = SELECT
 public.person                                   = SELECT
 public.pocketchroot                             = SELECT
 public.processor                                = SELECT
+public.product                                  = SELECT
 public.snap                                     = SELECT, UPDATE
 public.snaparch                                 = SELECT
 public.snapbuild                                = SELECT, INSERT

=== modified file 'lib/lp/code/scripts/tests/test_request_daily_builds.py'
--- lib/lp/code/scripts/tests/test_request_daily_builds.py	2018-07-30 18:10:28 +0000
+++ lib/lp/code/scripts/tests/test_request_daily_builds.py	2018-07-31 13:12:41 +0000
@@ -3,6 +3,7 @@
 
 """Test the request_daily_builds script."""
 
+import base64
 from collections import defaultdict
 import json
 import threading
@@ -11,7 +12,6 @@
     WSGIRequestHandler,
     )
 
-from six.moves.urllib_parse import urlparse
 import transaction
 
 from lp.code.interfaces.codehosting import BRANCH_ID_ALIAS_PREFIX
@@ -28,6 +28,13 @@
 from lp.testing.layers import ZopelessAppServerLayer
 
 
+class SilentWSGIRequestHandler(WSGIRequestHandler):
+    """A request handler that doesn't log requests."""
+
+    def log_message(self, fmt, *args):
+        pass
+
+
 class FakeLoggerheadApplication:
     """A WSGI application that provides some fake loggerhead endpoints."""
 
@@ -87,34 +94,81 @@
         self.contents[branch_id][file_id] = contents
 
 
-class FakeLoggerheadRequestHandler(WSGIRequestHandler):
-    """A request handler that doesn't log requests."""
-
-    def log_message(self, fmt, *args):
-        pass
-
-
 class FakeLoggerheadServer(threading.Thread):
     """Thread that runs a fake loggerhead server."""
 
-    def __init__(self, address):
+    def __init__(self):
         super(FakeLoggerheadServer, self).__init__()
         self.app = FakeLoggerheadApplication()
         self.server = make_server(
-            address, 0, self.app, handler_class=FakeLoggerheadRequestHandler)
-
-    def run(self):
-        self.server.serve_forever()
-
-    def getURL(self):
-        host, port = self.server.server_address
-        return 'http://%s:%d/' % (host, port)
-
-    def addInventory(self, branch_id, path, file_id):
-        self.app.addInventory(branch_id, path, file_id)
-
-    def addBlob(self, branch_id, file_id, contents):
-        self.app.addBlob(branch_id, file_id, contents)
+            'localhost', 0, self.app, handler_class=SilentWSGIRequestHandler)
+
+    def run(self):
+        self.server.serve_forever()
+
+    def getURL(self):
+        host, port = self.server.server_address
+        return 'http://%s:%d/' % (host, port)
+
+    def addInventory(self, branch, path, file_id):
+        self.app.addInventory(branch.id, path, file_id)
+
+    def addBlob(self, branch, file_id, contents):
+        self.app.addBlob(branch.id, file_id, contents)
+
+    def stop(self):
+        self.server.shutdown()
+
+
+class FakeTurnipApplication:
+    """A WSGI application that provides some fake turnip endpoints."""
+
+    def __init__(self):
+        self.contents = defaultdict(dict)
+
+    def _not_found(self, start_response):
+        start_response('404 Not Found', [('Content-Type', 'text/plain')])
+        return [b'']
+
+    def __call__(self, environ, start_response):
+        segments = environ['PATH_INFO'].lstrip('/').split('/')
+        if (len(segments) < 4 or
+                segments[0] != 'repo' or segments[2] != 'blob'):
+            return self._not_found(start_response)
+        repository_id = segments[1]
+        if repository_id not in self.contents:
+            return self._not_found(start_response)
+        filename = '/'.join(segments[3:])
+        if filename not in self.contents[repository_id]:
+            return self._not_found(start_response)
+        blob = self.contents[repository_id][filename]
+        response = {'size': len(blob), 'data': base64.b64encode(blob)}
+        start_response(
+            '200 OK', [('Content-Type', 'application/octet-stream')])
+        return [json.dumps(response).encode('UTF-8')]
+
+    def addBlob(self, repository, filename, contents):
+        self.contents[repository.getInternalPath()][filename] = contents
+
+
+class FakeTurnipServer(threading.Thread):
+    """Thread that runs a fake turnip server."""
+
+    def __init__(self):
+        super(FakeTurnipServer, self).__init__()
+        self.app = FakeTurnipApplication()
+        self.server = make_server(
+            'localhost', 0, self.app, handler_class=SilentWSGIRequestHandler)
+
+    def run(self):
+        self.server.serve_forever()
+
+    def getURL(self):
+        host, port = self.server.server_address
+        return 'http://%s:%d/' % (host, port)
+
+    def addBlob(self, repository_id, filename, contents):
+        self.app.addBlob(repository_id, filename, contents)
 
     def stop(self):
         self.server.shutdown()
@@ -129,11 +183,10 @@
         self.useFixture(FeatureFixture(SNAP_TESTING_FLAGS))
 
     def makeLoggerheadServer(self):
-        loggerhead_server = FakeLoggerheadServer(
-            urlparse(config.codehosting.internal_bzr_api_endpoint).hostname)
+        loggerhead_server = FakeLoggerheadServer()
         config_name = self.factory.getUniqueString()
         config_fixture = self.useFixture(ConfigFixture(
-            config_name, self.layer.config_fixture.instance_name))
+            config_name, config.instance_name))
         setting_lines = [
             '[codehosting]',
             'internal_bzr_api_endpoint: %s' % loggerhead_server.getURL(),
@@ -144,6 +197,21 @@
         self.addCleanup(loggerhead_server.stop)
         return loggerhead_server
 
+    def makeTurnipServer(self):
+        turnip_server = FakeTurnipServer()
+        config_name = self.factory.getUniqueString()
+        config_fixture = self.useFixture(ConfigFixture(
+            config_name, config.instance_name))
+        setting_lines = [
+            '[codehosting]',
+            'internal_git_api_endpoint: %s' % turnip_server.getURL(),
+            ]
+        config_fixture.add_section('\n' + '\n'.join(setting_lines))
+        self.useFixture(ConfigUseFixture(config_name))
+        turnip_server.start()
+        self.addCleanup(turnip_server.stop)
+        return turnip_server
+
     def test_request_daily_builds(self):
         """Ensure the request_daily_builds script requests daily builds."""
         processor = self.factory.makeProcessor(supports_virtualized=True)
@@ -152,48 +220,67 @@
         fake_chroot = self.factory.makeLibraryFileAlias(
             filename="fake_chroot.tar.gz", db_only=True)
         distroarchseries.addOrUpdateChroot(fake_chroot)
-        prod_branch = self.factory.makeProductBranch()
-        prod_recipe = self.factory.makeSourcePackageRecipe(
+        product = self.factory.makeProduct()
+        prod_branch = self.factory.makeBranch(product=product)
+        [prod_ref] = self.factory.makeGitRefs(target=product)
+        bzr_prod_recipe = self.factory.makeSourcePackageRecipe(
             build_daily=True, is_stale=True, branches=[prod_branch])
-        prod_snap = self.factory.makeSnap(
+        git_prod_recipe = self.factory.makeSourcePackageRecipe(
+            build_daily=True, is_stale=True, branches=[prod_ref])
+        bzr_prod_snap = self.factory.makeSnap(
             distroseries=distroarchseries.distroseries,
             processors=[distroarchseries.processor],
             auto_build=True, is_stale=True, branch=prod_branch)
-        pack_branch = self.factory.makePackageBranch()
-        pack_recipe = self.factory.makeSourcePackageRecipe(
+        git_prod_snap = self.factory.makeSnap(
+            distroseries=distroarchseries.distroseries,
+            processors=[distroarchseries.processor],
+            auto_build=True, is_stale=True, git_ref=prod_ref)
+        package = self.factory.makeSourcePackage()
+        pack_branch = self.factory.makeBranch(sourcepackage=package)
+        [pack_ref] = self.factory.makeGitRefs(
+            target=package.distribution_sourcepackage)
+        bzr_pack_recipe = self.factory.makeSourcePackageRecipe(
             build_daily=True, is_stale=True, branches=[pack_branch])
-        pack_snap = self.factory.makeSnap(
+        git_pack_recipe = self.factory.makeSourcePackageRecipe(
+            build_daily=True, is_stale=True, branches=[pack_ref])
+        bzr_pack_snap = self.factory.makeSnap(
             distroseries=distroarchseries.distroseries,
             processors=[distroarchseries.processor],
             auto_build=True, is_stale=True, branch=pack_branch)
-        self.assertEqual(0, prod_recipe.pending_builds.count())
-        self.assertEqual(0, prod_snap.pending_builds.count())
-        self.assertEqual(0, pack_recipe.pending_builds.count())
-        self.assertEqual(0, pack_snap.pending_builds.count())
+        git_pack_snap = self.factory.makeSnap(
+            distroseries=distroarchseries.distroseries,
+            processors=[distroarchseries.processor],
+            auto_build=True, is_stale=True, git_ref=pack_ref)
+        items = [
+            bzr_prod_recipe, git_prod_recipe, bzr_prod_snap, git_prod_snap,
+            bzr_pack_recipe, git_pack_recipe, bzr_pack_snap, git_pack_snap,
+            ]
+        for item in items:
+            self.assertEqual(0, item.pending_builds.count())
         transaction.commit()
         loggerhead_server = self.makeLoggerheadServer()
-        loggerhead_server.addInventory(prod_branch.id, 'snap', 'prod_snap')
-        loggerhead_server.addInventory(
-            prod_branch.id, 'snap/snapcraft.yaml', 'prod_snapcraft_yaml')
-        loggerhead_server.addBlob(
-            prod_branch.id, 'prod_snapcraft_yaml', b'name: prod-snap')
-        loggerhead_server.addInventory(pack_branch.id, 'snap', 'pack_snap')
-        loggerhead_server.addInventory(
-            pack_branch.id, 'snap/snapcraft.yaml', 'pack_snapcraft_yaml')
-        loggerhead_server.addBlob(
-            pack_branch.id, 'pack_snapcraft_yaml', b'name: pack-snap')
+        loggerhead_server.addInventory(prod_branch, 'snap', 'prod_snap')
+        loggerhead_server.addInventory(
+            prod_branch, 'snap/snapcraft.yaml', 'prod_snapcraft_yaml')
+        loggerhead_server.addBlob(
+            prod_branch, 'prod_snapcraft_yaml', b'name: prod-snap')
+        loggerhead_server.addInventory(pack_branch, 'snap', 'pack_snap')
+        loggerhead_server.addInventory(
+            pack_branch, 'snap/snapcraft.yaml', 'pack_snapcraft_yaml')
+        loggerhead_server.addBlob(
+            pack_branch, 'pack_snapcraft_yaml', b'name: pack-snap')
+        turnip_server = self.makeTurnipServer()
+        turnip_server.addBlob(
+            prod_ref.repository, 'snap/snapcraft.yaml', b'name: prod-snap')
+        turnip_server.addBlob(
+            pack_ref.repository, 'snap/snapcraft.yaml', b'name: pack-snap')
         retcode, stdout, stderr = run_script(
             'cronscripts/request_daily_builds.py', [])
-        self.assertIn('Requested 2 daily recipe builds.', stderr)
-        self.assertIn('Requested 2 automatic snap package builds.', stderr)
-        self.assertEqual(1, prod_recipe.pending_builds.count())
-        self.assertEqual(1, prod_snap.pending_builds.count())
-        self.assertEqual(1, pack_recipe.pending_builds.count())
-        self.assertEqual(1, pack_snap.pending_builds.count())
-        self.assertFalse(prod_recipe.is_stale)
-        self.assertFalse(prod_snap.is_stale)
-        self.assertFalse(pack_recipe.is_stale)
-        self.assertFalse(pack_snap.is_stale)
+        self.assertIn('Requested 4 daily recipe builds.', stderr)
+        self.assertIn('Requested 4 automatic snap package builds.', stderr)
+        for item in items:
+            self.assertEqual(1, item.pending_builds.count())
+            self.assertFalse(item.is_stale)
 
     def test_request_daily_builds_oops(self):
         """Ensure errors are handled cleanly."""