launchpad-reviewers team mailing list archive

[Colin Watson <mp+420258@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:cibuild-use-macaroons into launchpad:master.

Commit message:
Issue macaroons for private CI builds

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/420258

I added the issuing and verification logic in 163146dfdb, but we need to actually send the resulting credentials to builders as well.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:cibuild-use-macaroons into launchpad:master.

diff --git a/lib/lp/code/model/cibuildbehaviour.py b/lib/lp/code/model/cibuildbehaviour.py
index 35fe231..4d3e641 100644
--- a/lib/lp/code/model/cibuildbehaviour.py
+++ b/lib/lp/code/model/cibuildbehaviour.py
@@ -23,6 +23,9 @@ from lp.buildmaster.model.buildfarmjobbehaviour import (
     )
 from lp.code.enums import RevisionStatusResult
 from lp.code.interfaces.cibuild import ICIBuild
+from lp.code.interfaces.codehosting import LAUNCHPAD_SERVICES
+from lp.services.config import config
+from lp.services.twistedsupport import cancel_on_timeout
 from lp.soyuz.adapters.archivedependencies import (
     get_sources_list_for_building,
     )
@@ -60,6 +63,13 @@ class CIBuildBehaviour(BuilderProxyMixin, BuildFarmJobBehaviourBase):
             raise CannotBuild(
                 "Missing chroot for %s" % build.distro_arch_series.displayname)
 
+    def issueMacaroon(self):
+        """See `IBuildFarmJobBehaviour`."""
+        return cancel_on_timeout(
+            self._authserver.callRemote(
+                "issueMacaroon", "ci-build", "CIBuild", self.build.id),
+            config.builddmaster.authentication_timeout)
+
     @defer.inlineCallbacks
     def extraBuildArgs(self, logger=None):
         """Return extra builder arguments for this build."""
@@ -75,7 +85,13 @@ class CIBuildBehaviour(BuilderProxyMixin, BuildFarmJobBehaviourBase):
             yield get_sources_list_for_building(
                 self, build.distro_arch_series, None, logger=logger))
         args["jobs"] = removeSecurityProxy(build.stages)
-        args["git_repository"] = build.git_repository.git_https_url
+        if build.git_repository.private:
+            macaroon_raw = yield self.issueMacaroon()
+            url = build.git_repository.getCodebrowseUrl(
+                username=LAUNCHPAD_SERVICES, password=macaroon_raw)
+            args["git_repository"] = url
+        else:
+            args["git_repository"] = build.git_repository.git_https_url
         args["git_path"] = build.commit_sha1
         args["private"] = build.is_private
         return args
diff --git a/lib/lp/code/model/tests/test_cibuildbehaviour.py b/lib/lp/code/model/tests/test_cibuildbehaviour.py
index 505d4b5..48c12c9 100644
--- a/lib/lp/code/model/tests/test_cibuildbehaviour.py
+++ b/lib/lp/code/model/tests/test_cibuildbehaviour.py
@@ -12,14 +12,17 @@ from urllib.parse import urlsplit
 import uuid
 
 from fixtures import MockPatch
+from pymacaroons import Macaroon
 from testtools import ExpectedException
 from testtools.matchers import (
+    AfterPreprocessing,
     ContainsDict,
     Equals,
     Is,
     IsInstance,
     MatchesDict,
     MatchesListwise,
+    MatchesStructure,
     StartsWith,
     )
 from testtools.twistedsupport import (
@@ -60,6 +63,7 @@ from lp.buildmaster.tests.test_buildfarmjobbehaviour import (
     )
 from lp.code.enums import RevisionStatusResult
 from lp.code.model.cibuildbehaviour import CIBuildBehaviour
+from lp.services.authserver.testing import InProcessAuthServerFixture
 from lp.services.config import config
 from lp.services.log.logger import (
     BufferLogger,
@@ -304,12 +308,48 @@ class TestAsyncCIBuildBehaviour(StatsMixin, TestCIBuildBehaviourBase):
     def test_extraBuildArgs_private(self):
         # If the repository is private, extraBuildArgs sends the appropriate
         # arguments.
+        self.useFixture(InProcessAuthServerFixture())
+        self.pushConfig(
+            "launchpad", internal_macaroon_secret_key="some-secret")
         repository = self.factory.makeGitRepository(
             information_type=InformationType.USERDATA)
-        job = self.makeJob(git_repository=repository)
+        job = self.makeJob(git_repository=repository, stages=[[("test", 0)]])
+        expected_archives, expected_trusted_keys = (
+            yield get_sources_list_for_building(
+                job, job.build.distro_arch_series, None))
+        for archive_line in expected_archives:
+            self.assertIn("universe", archive_line)
         with dbuser(config.builddmaster.dbuser):
             args = yield job.extraBuildArgs()
-        self.assertTrue(args["private"])
+        split_browse_root = urlsplit(config.codehosting.git_browse_root)
+        self.assertThat(args, MatchesDict({
+            "archive_private": Is(False),
+            "archives": Equals(expected_archives),
+            "arch_tag": Equals("i386"),
+            "build_url": Equals(canonical_url(job.build)),
+            "fast_cleanup": Is(True),
+            "git_path": Equals(job.build.commit_sha1),
+            "git_repository": AfterPreprocessing(urlsplit, MatchesStructure(
+                scheme=Equals(split_browse_root.scheme),
+                username=Equals("+launchpad-services"),
+                password=AfterPreprocessing(
+                    Macaroon.deserialize, MatchesStructure(
+                        location=Equals(config.vhost.mainsite.hostname),
+                        identifier=Equals("ci-build"),
+                        caveats=MatchesListwise([
+                            MatchesStructure.byEquality(
+                                caveat_id="lp.ci-build %s" % job.build.id),
+                            ]))),
+                hostname=Equals(split_browse_root.hostname),
+                port=Equals(split_browse_root.port),
+                path=Equals("/" + job.build.git_repository.shortened_path))),
+            "jobs": Equals([[["test", 0]]]),
+            "private": Is(True),
+            "proxy_url": ProxyURLMatcher(job, self.now),
+            "revocation_endpoint": RevocationEndpointMatcher(job, self.now),
+            "series": Equals(job.build.distro_series.name),
+            "trusted_keys": Equals(expected_trusted_keys),
+            }))
 
     @defer.inlineCallbacks
     def test_composeBuildRequest_proxy_url_set(self):