Merge ~cjwatson/launchpad:build-private-bpb-immediately into launchpad:master

Proposed by Colin Watson on 2019-10-07
Status: Needs review
Proposed branch: ~cjwatson/launchpad:build-private-bpb-immediately
Merge into: launchpad:master
Diff against target: 276 lines (+60/-60)
4 files modified
lib/lp/buildmaster/tests/test_builder.py (+4/-4)
lib/lp/soyuz/model/binarypackagebuild.py (+3/-27)
lib/lp/soyuz/model/binarypackagebuildbehaviour.py (+10/-13)
lib/lp/soyuz/tests/test_binarypackagebuildbehaviour.py (+43/-16)
Reviewer Review Type Date Requested Status
Launchpad code reviewers 2019-10-07 Pending
Review via email: mp+373741@code.launchpad.net

Commit message

Dispatch private BPBs immediately

We now use macaroon authentication for source files.

Description of the change

I haven't yet tested this beyond what's in the test suite.

This is essentially the same as https://code.launchpad.net/~cjwatson/launchpad/build-private-bpb-immediately/+merge/345104, converted to git and rebased on master.

To post a comment you must log in.

Unmerged commits

fdfa673... by Colin Watson on 2018-05-04

Dispatch private BPBs immediately, using macaroon auth for source files.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/buildmaster/tests/test_builder.py b/lib/lp/buildmaster/tests/test_builder.py
2index a97d3c9..3635643 100644
3--- a/lib/lp/buildmaster/tests/test_builder.py
4+++ b/lib/lp/buildmaster/tests/test_builder.py
5@@ -403,13 +403,13 @@ class TestFindBuildCandidatePrivatePPA(TestFindBuildCandidatePPABase):
6 build = getUtility(IBinaryPackageBuildSet).getByQueueEntry(next_job)
7 self.assertEqual('joesppa', build.archive.name)
8
9- # If the source for the build is still pending, it won't be
10- # dispatched because the builder has to fetch the source files
11- # from the (password protected) repo area, not the librarian.
12+ # If the source for the build is still pending, it will still be
13+ # dispatched: the builder will use macaroon authentication to fetch
14+ # the source files from the librarian.
15 pub = build.current_source_publication
16 pub.status = PackagePublishingStatus.PENDING
17 candidate = removeSecurityProxy(self.builder4)._findBuildCandidate()
18- self.assertNotEqual(next_job.id, candidate.id)
19+ self.assertEqual(next_job.id, candidate.id)
20
21
22 class TestFindBuildCandidateDistroArchive(TestFindBuildCandidateBase):
23diff --git a/lib/lp/soyuz/model/binarypackagebuild.py b/lib/lp/soyuz/model/binarypackagebuild.py
24index 748e102..1f87038 100644
25--- a/lib/lp/soyuz/model/binarypackagebuild.py
26+++ b/lib/lp/soyuz/model/binarypackagebuild.py
27@@ -89,10 +89,7 @@ from lp.services.macaroons.interfaces import (
28 )
29 from lp.services.macaroons.model import MacaroonIssuerBase
30 from lp.soyuz.adapters.buildarch import determine_architectures_to_build
31-from lp.soyuz.enums import (
32- ArchivePurpose,
33- PackagePublishingStatus,
34- )
35+from lp.soyuz.enums import ArchivePurpose
36 from lp.soyuz.interfaces.archive import (
37 InvalidExternalDependencies,
38 validate_external_dependencies,
39@@ -1212,33 +1209,12 @@ class BinaryPackageBuildSet(SpecificBuildFarmJobSourceMixin):
40 @staticmethod
41 def addCandidateSelectionCriteria(processor, virtualized):
42 """See `ISpecificBuildFarmJobSource`."""
43- private_statuses = (
44- PackagePublishingStatus.PUBLISHED,
45- PackagePublishingStatus.SUPERSEDED,
46- PackagePublishingStatus.DELETED,
47- )
48 return """
49- SELECT TRUE FROM Archive, BinaryPackageBuild, DistroArchSeries
50+ SELECT TRUE FROM BinaryPackageBuild
51 WHERE
52 BinaryPackageBuild.build_farm_job = BuildQueue.build_farm_job AND
53- BinaryPackageBuild.distro_arch_series =
54- DistroArchSeries.id AND
55- BinaryPackageBuild.archive = Archive.id AND
56- ((Archive.private IS TRUE AND
57- EXISTS (
58- SELECT SourcePackagePublishingHistory.id
59- FROM SourcePackagePublishingHistory
60- WHERE
61- SourcePackagePublishingHistory.distroseries =
62- DistroArchSeries.distroseries AND
63- SourcePackagePublishingHistory.sourcepackagerelease =
64- BinaryPackageBuild.source_package_release AND
65- SourcePackagePublishingHistory.archive = Archive.id AND
66- SourcePackagePublishingHistory.status IN %s))
67- OR
68- archive.private IS FALSE) AND
69 BinaryPackageBuild.status = %s
70- """ % sqlvalues(private_statuses, BuildStatus.NEEDSBUILD)
71+ """ % sqlvalues(BuildStatus.NEEDSBUILD)
72
73 @staticmethod
74 def postprocessCandidate(job, logger):
75diff --git a/lib/lp/soyuz/model/binarypackagebuildbehaviour.py b/lib/lp/soyuz/model/binarypackagebuildbehaviour.py
76index afb1df9..9d0591b 100644
77--- a/lib/lp/soyuz/model/binarypackagebuildbehaviour.py
78+++ b/lib/lp/soyuz/model/binarypackagebuildbehaviour.py
79@@ -10,7 +10,9 @@ __all__ = [
80 ]
81
82 from twisted.internet import defer
83+from zope.component import getUtility
84 from zope.interface import implementer
85+from zope.security.proxy import removeSecurityProxy
86
87 from lp.buildmaster.interfaces.builder import CannotBuild
88 from lp.buildmaster.interfaces.buildfarmjobbehaviour import (
89@@ -20,13 +22,12 @@ from lp.buildmaster.model.buildfarmjobbehaviour import (
90 BuildFarmJobBehaviourBase,
91 )
92 from lp.registry.interfaces.pocket import PackagePublishingPocket
93-from lp.services.webapp import urlappend
94+from lp.services.macaroons.interfaces import IMacaroonIssuer
95 from lp.soyuz.adapters.archivedependencies import (
96 get_primary_current_component,
97 get_sources_list_for_building,
98 )
99 from lp.soyuz.enums import ArchivePurpose
100-from lp.soyuz.model.publishing import makePoolPath
101
102
103 @implementer(IBuildFarmJobBehaviour)
104@@ -63,14 +64,9 @@ class BinaryPackageBuildBehaviour(BuildFarmJobBehaviourBase):
105 # Build filemap structure with the files required in this build
106 # and send them to the slave.
107 if self.build.archive.private:
108- # Builds in private archive may have restricted files that
109- # we can't obtain from the public librarian. Prepare a pool
110- # URL from which to fetch them.
111- pool_url = urlappend(
112- self.build.archive.archive_url,
113- makePoolPath(
114- self.build.source_package_release.sourcepackagename.name,
115- self.build.current_component.name))
116+ issuer = getUtility(IMacaroonIssuer, 'binary-package-build')
117+ password = removeSecurityProxy(issuer).issueMacaroon(
118+ self.build).serialize()
119 filemap = {}
120 for source_file in self.build.source_package_release.files:
121 lfa = source_file.libraryfile
122@@ -80,9 +76,10 @@ class BinaryPackageBuildBehaviour(BuildFarmJobBehaviourBase):
123 else:
124 filemap[lfa.filename] = {
125 'sha1': lfa.content.sha1,
126- 'url': urlappend(pool_url, lfa.filename),
127- 'username': 'buildd',
128- 'password': self.build.archive.buildd_secret}
129+ 'url': lfa.https_url,
130+ 'username': '',
131+ 'password': password,
132+ }
133 return filemap
134
135 def verifyBuildRequest(self, logger):
136diff --git a/lib/lp/soyuz/tests/test_binarypackagebuildbehaviour.py b/lib/lp/soyuz/tests/test_binarypackagebuildbehaviour.py
137index 61e4847..be27e00 100644
138--- a/lib/lp/soyuz/tests/test_binarypackagebuildbehaviour.py
139+++ b/lib/lp/soyuz/tests/test_binarypackagebuildbehaviour.py
140@@ -12,8 +12,15 @@ import os
141 import shutil
142 import tempfile
143
144+from pymacaroons import Macaroon
145 from storm.store import Store
146-from testtools.matchers import MatchesListwise
147+from testtools.matchers import (
148+ Contains,
149+ Equals,
150+ Matcher,
151+ MatchesListwise,
152+ Mismatch,
153+ )
154 from testtools.twistedsupport import AsynchronousDeferredRunTest
155 import transaction
156 from twisted.internet import defer
157@@ -21,7 +28,6 @@ from twisted.trial.unittest import TestCase as TrialTestCase
158 from zope.component import getUtility
159 from zope.security.proxy import removeSecurityProxy
160
161-from lp.archivepublisher.diskpool import poolify
162 from lp.archivepublisher.interfaces.archivesigningkey import (
163 IArchiveSigningKey,
164 )
165@@ -58,6 +64,7 @@ from lp.registry.interfaces.sourcepackage import SourcePackageFileType
166 from lp.services.config import config
167 from lp.services.librarian.interfaces import ILibraryFileAliasSet
168 from lp.services.log.logger import BufferLogger
169+from lp.services.macaroons.interfaces import IMacaroonIssuer
170 from lp.services.webapp import canonical_url
171 from lp.soyuz.adapters.archivedependencies import (
172 get_sources_list_for_building,
173@@ -74,6 +81,27 @@ from lp.testing.keyserver import InProcessKeyServerFixture
174 from lp.testing.layers import LaunchpadZopelessLayer
175
176
177+class BinaryPackageBuildMacaroonVerifies(Matcher):
178+ """Matches if a binary-package-build macaroon passes verification."""
179+
180+ def __init__(self, build, lfa):
181+ self.build = build
182+ self.lfa = lfa
183+
184+ def match(self, macaroon_raw):
185+ macaroon = Macaroon.deserialize(macaroon_raw)
186+ expected_caveat = (
187+ "lp.principal.binary-package-build %s" %
188+ removeSecurityProxy(self.build).id)
189+ mismatch = Contains(expected_caveat).match(
190+ [caveat.caveat_id for caveat in macaroon.caveats])
191+ if mismatch is not None:
192+ return mismatch
193+ issuer = getUtility(IMacaroonIssuer, "binary-package-build")
194+ if not issuer.verifyMacaroon(macaroon, self.lfa):
195+ return Mismatch("Macaroon does not verify")
196+
197+
198 class TestBinaryBuildPackageBehaviour(TestCaseWithFactory):
199 """Tests for the BinaryPackageBuildBehaviour.
200
201@@ -98,7 +126,7 @@ class TestBinaryBuildPackageBehaviour(TestCaseWithFactory):
202 expected = yield self.makeExpectedInteraction(
203 builder, build, chroot, archive, archive_purpose, component,
204 extra_uploads, filemap_names)
205- self.assertEqual(expected, call_log)
206+ self.assertThat(call_log, MatchesListwise(expected))
207
208 @defer.inlineCallbacks
209 def makeExpectedInteraction(self, builder, build, chroot, archive,
210@@ -115,7 +143,7 @@ class TestBinaryBuildPackageBehaviour(TestCaseWithFactory):
211 builder. We specify this separately from the archive because
212 sometimes the behaviour object has to give a different purpose
213 in order to trick the slave into building correctly.
214- :return: A list of the calls we expect to be made.
215+ :return: A list of matchers for the calls we expect to be made.
216 """
217 das = build.distro_arch_series
218 ds_name = das.distroseries.name
219@@ -131,8 +159,9 @@ class TestBinaryBuildPackageBehaviour(TestCaseWithFactory):
220 extra_uploads = []
221
222 upload_logs = [
223- ('ensurepresent',) + upload
224- for upload in [(chroot.http_url, '', '')] + extra_uploads]
225+ MatchesListwise((Equals('ensurepresent'),) + upload)
226+ for upload in [(Equals(chroot.http_url), Equals(''), Equals(''))] +
227+ extra_uploads]
228
229 extra_args = {
230 'arch_indep': arch_indep,
231@@ -151,8 +180,9 @@ class TestBinaryBuildPackageBehaviour(TestCaseWithFactory):
232 'trusted_keys': trusted_keys,
233 }
234 build_log = [
235- ('build', build.build_cookie, 'binarypackage',
236- chroot.content.sha1, filemap_names, extra_args)]
237+ MatchesListwise([Equals(arg) for arg in (
238+ 'build', build.build_cookie, 'binarypackage',
239+ chroot.content.sha1, filemap_names, extra_args)])]
240 result = upload_logs + build_log
241 defer.returnValue(result)
242
243@@ -236,6 +266,8 @@ class TestBinaryBuildPackageBehaviour(TestCaseWithFactory):
244
245 @defer.inlineCallbacks
246 def test_private_source_dispatch(self):
247+ self.pushConfig(
248+ "launchpad", internal_macaroon_secret_key="some-secret")
249 archive = self.factory.makeArchive(private=True)
250 slave = OkSlave()
251 builder = self.factory.makeBuilder()
252@@ -246,13 +278,6 @@ class TestBinaryBuildPackageBehaviour(TestCaseWithFactory):
253 sprf = build.source_package_release.addFile(
254 self.factory.makeLibraryFileAlias(db_only=True),
255 filetype=SourcePackageFileType.ORIG_TARBALL)
256- sprf_url = (
257- 'http://private-ppa.launchpad.test/%s/%s/ubuntu/pool/%s/%s'
258- % (archive.owner.name, archive.name,
259- poolify(
260- build.source_package_release.sourcepackagename.name,
261- 'main'),
262- sprf.libraryfile.filename))
263 lf = self.factory.makeLibraryFileAlias()
264 transaction.commit()
265 build.distro_arch_series.addOrUpdateChroot(lf)
266@@ -264,7 +289,9 @@ class TestBinaryBuildPackageBehaviour(TestCaseWithFactory):
267 interactor.getBuildBehaviour(bq, builder, slave), BufferLogger())
268 yield self.assertExpectedInteraction(
269 slave.call_log, builder, build, lf, archive, ArchivePurpose.PPA,
270- extra_uploads=[(sprf_url, 'buildd', 'sekrit')],
271+ extra_uploads=[(
272+ Equals(sprf.libraryfile.https_url), Equals(''),
273+ BinaryPackageBuildMacaroonVerifies(build, sprf.libraryfile))],
274 filemap_names=[sprf.libraryfile.filename])
275
276 @defer.inlineCallbacks

Subscribers

People subscribed via source and target branches