Merge ~cjwatson/launchpad:build-private-bpb-immediately into launchpad:master
- Git
- lp:~cjwatson/launchpad
- build-private-bpb-immediately
- Merge into master
Proposed by
Colin Watson
Status: | Superseded |
---|---|
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) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Launchpad code reviewers | Pending | ||
Review via email: mp+373741@code.launchpad.net |
This proposal has been superseded by a proposal from 2022-09-21.
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:/
To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) wrote : | # |
Unmerged commits
- fdfa673... by Colin Watson
-
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
1 | diff --git a/lib/lp/buildmaster/tests/test_builder.py b/lib/lp/buildmaster/tests/test_builder.py | |||
2 | index 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 | 403 | build = getUtility(IBinaryPackageBuildSet).getByQueueEntry(next_job) | 403 | build = getUtility(IBinaryPackageBuildSet).getByQueueEntry(next_job) |
7 | 404 | self.assertEqual('joesppa', build.archive.name) | 404 | self.assertEqual('joesppa', build.archive.name) |
8 | 405 | 405 | ||
12 | 406 | # If the source for the build is still pending, it won't be | 406 | # If the source for the build is still pending, it will still be |
13 | 407 | # dispatched because the builder has to fetch the source files | 407 | # dispatched: the builder will use macaroon authentication to fetch |
14 | 408 | # from the (password protected) repo area, not the librarian. | 408 | # the source files from the librarian. |
15 | 409 | pub = build.current_source_publication | 409 | pub = build.current_source_publication |
16 | 410 | pub.status = PackagePublishingStatus.PENDING | 410 | pub.status = PackagePublishingStatus.PENDING |
17 | 411 | candidate = removeSecurityProxy(self.builder4)._findBuildCandidate() | 411 | candidate = removeSecurityProxy(self.builder4)._findBuildCandidate() |
19 | 412 | self.assertNotEqual(next_job.id, candidate.id) | 412 | self.assertEqual(next_job.id, candidate.id) |
20 | 413 | 413 | ||
21 | 414 | 414 | ||
22 | 415 | class TestFindBuildCandidateDistroArchive(TestFindBuildCandidateBase): | 415 | class TestFindBuildCandidateDistroArchive(TestFindBuildCandidateBase): |
23 | diff --git a/lib/lp/soyuz/model/binarypackagebuild.py b/lib/lp/soyuz/model/binarypackagebuild.py | |||
24 | index 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 | 89 | ) | 89 | ) |
29 | 90 | from lp.services.macaroons.model import MacaroonIssuerBase | 90 | from lp.services.macaroons.model import MacaroonIssuerBase |
30 | 91 | from lp.soyuz.adapters.buildarch import determine_architectures_to_build | 91 | from lp.soyuz.adapters.buildarch import determine_architectures_to_build |
35 | 92 | from lp.soyuz.enums import ( | 92 | from lp.soyuz.enums import ArchivePurpose |
32 | 93 | ArchivePurpose, | ||
33 | 94 | PackagePublishingStatus, | ||
34 | 95 | ) | ||
36 | 96 | from lp.soyuz.interfaces.archive import ( | 93 | from lp.soyuz.interfaces.archive import ( |
37 | 97 | InvalidExternalDependencies, | 94 | InvalidExternalDependencies, |
38 | 98 | validate_external_dependencies, | 95 | validate_external_dependencies, |
39 | @@ -1212,33 +1209,12 @@ class BinaryPackageBuildSet(SpecificBuildFarmJobSourceMixin): | |||
40 | 1212 | @staticmethod | 1209 | @staticmethod |
41 | 1213 | def addCandidateSelectionCriteria(processor, virtualized): | 1210 | def addCandidateSelectionCriteria(processor, virtualized): |
42 | 1214 | """See `ISpecificBuildFarmJobSource`.""" | 1211 | """See `ISpecificBuildFarmJobSource`.""" |
43 | 1215 | private_statuses = ( | ||
44 | 1216 | PackagePublishingStatus.PUBLISHED, | ||
45 | 1217 | PackagePublishingStatus.SUPERSEDED, | ||
46 | 1218 | PackagePublishingStatus.DELETED, | ||
47 | 1219 | ) | ||
48 | 1220 | return """ | 1212 | return """ |
50 | 1221 | SELECT TRUE FROM Archive, BinaryPackageBuild, DistroArchSeries | 1213 | SELECT TRUE FROM BinaryPackageBuild |
51 | 1222 | WHERE | 1214 | WHERE |
52 | 1223 | BinaryPackageBuild.build_farm_job = BuildQueue.build_farm_job AND | 1215 | BinaryPackageBuild.build_farm_job = BuildQueue.build_farm_job AND |
53 | 1224 | BinaryPackageBuild.distro_arch_series = | ||
54 | 1225 | DistroArchSeries.id AND | ||
55 | 1226 | BinaryPackageBuild.archive = Archive.id AND | ||
56 | 1227 | ((Archive.private IS TRUE AND | ||
57 | 1228 | EXISTS ( | ||
58 | 1229 | SELECT SourcePackagePublishingHistory.id | ||
59 | 1230 | FROM SourcePackagePublishingHistory | ||
60 | 1231 | WHERE | ||
61 | 1232 | SourcePackagePublishingHistory.distroseries = | ||
62 | 1233 | DistroArchSeries.distroseries AND | ||
63 | 1234 | SourcePackagePublishingHistory.sourcepackagerelease = | ||
64 | 1235 | BinaryPackageBuild.source_package_release AND | ||
65 | 1236 | SourcePackagePublishingHistory.archive = Archive.id AND | ||
66 | 1237 | SourcePackagePublishingHistory.status IN %s)) | ||
67 | 1238 | OR | ||
68 | 1239 | archive.private IS FALSE) AND | ||
69 | 1240 | BinaryPackageBuild.status = %s | 1216 | BinaryPackageBuild.status = %s |
71 | 1241 | """ % sqlvalues(private_statuses, BuildStatus.NEEDSBUILD) | 1217 | """ % sqlvalues(BuildStatus.NEEDSBUILD) |
72 | 1242 | 1218 | ||
73 | 1243 | @staticmethod | 1219 | @staticmethod |
74 | 1244 | def postprocessCandidate(job, logger): | 1220 | def postprocessCandidate(job, logger): |
75 | diff --git a/lib/lp/soyuz/model/binarypackagebuildbehaviour.py b/lib/lp/soyuz/model/binarypackagebuildbehaviour.py | |||
76 | index 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 | 10 | ] | 10 | ] |
81 | 11 | 11 | ||
82 | 12 | from twisted.internet import defer | 12 | from twisted.internet import defer |
83 | 13 | from zope.component import getUtility | ||
84 | 13 | from zope.interface import implementer | 14 | from zope.interface import implementer |
85 | 15 | from zope.security.proxy import removeSecurityProxy | ||
86 | 14 | 16 | ||
87 | 15 | from lp.buildmaster.interfaces.builder import CannotBuild | 17 | from lp.buildmaster.interfaces.builder import CannotBuild |
88 | 16 | from lp.buildmaster.interfaces.buildfarmjobbehaviour import ( | 18 | from lp.buildmaster.interfaces.buildfarmjobbehaviour import ( |
89 | @@ -20,13 +22,12 @@ from lp.buildmaster.model.buildfarmjobbehaviour import ( | |||
90 | 20 | BuildFarmJobBehaviourBase, | 22 | BuildFarmJobBehaviourBase, |
91 | 21 | ) | 23 | ) |
92 | 22 | from lp.registry.interfaces.pocket import PackagePublishingPocket | 24 | from lp.registry.interfaces.pocket import PackagePublishingPocket |
94 | 23 | from lp.services.webapp import urlappend | 25 | from lp.services.macaroons.interfaces import IMacaroonIssuer |
95 | 24 | from lp.soyuz.adapters.archivedependencies import ( | 26 | from lp.soyuz.adapters.archivedependencies import ( |
96 | 25 | get_primary_current_component, | 27 | get_primary_current_component, |
97 | 26 | get_sources_list_for_building, | 28 | get_sources_list_for_building, |
98 | 27 | ) | 29 | ) |
99 | 28 | from lp.soyuz.enums import ArchivePurpose | 30 | from lp.soyuz.enums import ArchivePurpose |
100 | 29 | from lp.soyuz.model.publishing import makePoolPath | ||
101 | 30 | 31 | ||
102 | 31 | 32 | ||
103 | 32 | @implementer(IBuildFarmJobBehaviour) | 33 | @implementer(IBuildFarmJobBehaviour) |
104 | @@ -63,14 +64,9 @@ class BinaryPackageBuildBehaviour(BuildFarmJobBehaviourBase): | |||
105 | 63 | # Build filemap structure with the files required in this build | 64 | # Build filemap structure with the files required in this build |
106 | 64 | # and send them to the slave. | 65 | # and send them to the slave. |
107 | 65 | if self.build.archive.private: | 66 | if self.build.archive.private: |
116 | 66 | # Builds in private archive may have restricted files that | 67 | issuer = getUtility(IMacaroonIssuer, 'binary-package-build') |
117 | 67 | # we can't obtain from the public librarian. Prepare a pool | 68 | password = removeSecurityProxy(issuer).issueMacaroon( |
118 | 68 | # URL from which to fetch them. | 69 | self.build).serialize() |
111 | 69 | pool_url = urlappend( | ||
112 | 70 | self.build.archive.archive_url, | ||
113 | 71 | makePoolPath( | ||
114 | 72 | self.build.source_package_release.sourcepackagename.name, | ||
115 | 73 | self.build.current_component.name)) | ||
119 | 74 | filemap = {} | 70 | filemap = {} |
120 | 75 | for source_file in self.build.source_package_release.files: | 71 | for source_file in self.build.source_package_release.files: |
121 | 76 | lfa = source_file.libraryfile | 72 | lfa = source_file.libraryfile |
122 | @@ -80,9 +76,10 @@ class BinaryPackageBuildBehaviour(BuildFarmJobBehaviourBase): | |||
123 | 80 | else: | 76 | else: |
124 | 81 | filemap[lfa.filename] = { | 77 | filemap[lfa.filename] = { |
125 | 82 | 'sha1': lfa.content.sha1, | 78 | 'sha1': lfa.content.sha1, |
129 | 83 | 'url': urlappend(pool_url, lfa.filename), | 79 | 'url': lfa.https_url, |
130 | 84 | 'username': 'buildd', | 80 | 'username': '', |
131 | 85 | 'password': self.build.archive.buildd_secret} | 81 | 'password': password, |
132 | 82 | } | ||
133 | 86 | return filemap | 83 | return filemap |
134 | 87 | 84 | ||
135 | 88 | def verifyBuildRequest(self, logger): | 85 | def verifyBuildRequest(self, logger): |
136 | diff --git a/lib/lp/soyuz/tests/test_binarypackagebuildbehaviour.py b/lib/lp/soyuz/tests/test_binarypackagebuildbehaviour.py | |||
137 | index 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 | 12 | import shutil | 12 | import shutil |
142 | 13 | import tempfile | 13 | import tempfile |
143 | 14 | 14 | ||
144 | 15 | from pymacaroons import Macaroon | ||
145 | 15 | from storm.store import Store | 16 | from storm.store import Store |
147 | 16 | from testtools.matchers import MatchesListwise | 17 | from testtools.matchers import ( |
148 | 18 | Contains, | ||
149 | 19 | Equals, | ||
150 | 20 | Matcher, | ||
151 | 21 | MatchesListwise, | ||
152 | 22 | Mismatch, | ||
153 | 23 | ) | ||
154 | 17 | from testtools.twistedsupport import AsynchronousDeferredRunTest | 24 | from testtools.twistedsupport import AsynchronousDeferredRunTest |
155 | 18 | import transaction | 25 | import transaction |
156 | 19 | from twisted.internet import defer | 26 | from twisted.internet import defer |
157 | @@ -21,7 +28,6 @@ from twisted.trial.unittest import TestCase as TrialTestCase | |||
158 | 21 | from zope.component import getUtility | 28 | from zope.component import getUtility |
159 | 22 | from zope.security.proxy import removeSecurityProxy | 29 | from zope.security.proxy import removeSecurityProxy |
160 | 23 | 30 | ||
161 | 24 | from lp.archivepublisher.diskpool import poolify | ||
162 | 25 | from lp.archivepublisher.interfaces.archivesigningkey import ( | 31 | from lp.archivepublisher.interfaces.archivesigningkey import ( |
163 | 26 | IArchiveSigningKey, | 32 | IArchiveSigningKey, |
164 | 27 | ) | 33 | ) |
165 | @@ -58,6 +64,7 @@ from lp.registry.interfaces.sourcepackage import SourcePackageFileType | |||
166 | 58 | from lp.services.config import config | 64 | from lp.services.config import config |
167 | 59 | from lp.services.librarian.interfaces import ILibraryFileAliasSet | 65 | from lp.services.librarian.interfaces import ILibraryFileAliasSet |
168 | 60 | from lp.services.log.logger import BufferLogger | 66 | from lp.services.log.logger import BufferLogger |
169 | 67 | from lp.services.macaroons.interfaces import IMacaroonIssuer | ||
170 | 61 | from lp.services.webapp import canonical_url | 68 | from lp.services.webapp import canonical_url |
171 | 62 | from lp.soyuz.adapters.archivedependencies import ( | 69 | from lp.soyuz.adapters.archivedependencies import ( |
172 | 63 | get_sources_list_for_building, | 70 | get_sources_list_for_building, |
173 | @@ -74,6 +81,27 @@ from lp.testing.keyserver import InProcessKeyServerFixture | |||
174 | 74 | from lp.testing.layers import LaunchpadZopelessLayer | 81 | from lp.testing.layers import LaunchpadZopelessLayer |
175 | 75 | 82 | ||
176 | 76 | 83 | ||
177 | 84 | class BinaryPackageBuildMacaroonVerifies(Matcher): | ||
178 | 85 | """Matches if a binary-package-build macaroon passes verification.""" | ||
179 | 86 | |||
180 | 87 | def __init__(self, build, lfa): | ||
181 | 88 | self.build = build | ||
182 | 89 | self.lfa = lfa | ||
183 | 90 | |||
184 | 91 | def match(self, macaroon_raw): | ||
185 | 92 | macaroon = Macaroon.deserialize(macaroon_raw) | ||
186 | 93 | expected_caveat = ( | ||
187 | 94 | "lp.principal.binary-package-build %s" % | ||
188 | 95 | removeSecurityProxy(self.build).id) | ||
189 | 96 | mismatch = Contains(expected_caveat).match( | ||
190 | 97 | [caveat.caveat_id for caveat in macaroon.caveats]) | ||
191 | 98 | if mismatch is not None: | ||
192 | 99 | return mismatch | ||
193 | 100 | issuer = getUtility(IMacaroonIssuer, "binary-package-build") | ||
194 | 101 | if not issuer.verifyMacaroon(macaroon, self.lfa): | ||
195 | 102 | return Mismatch("Macaroon does not verify") | ||
196 | 103 | |||
197 | 104 | |||
198 | 77 | class TestBinaryBuildPackageBehaviour(TestCaseWithFactory): | 105 | class TestBinaryBuildPackageBehaviour(TestCaseWithFactory): |
199 | 78 | """Tests for the BinaryPackageBuildBehaviour. | 106 | """Tests for the BinaryPackageBuildBehaviour. |
200 | 79 | 107 | ||
201 | @@ -98,7 +126,7 @@ class TestBinaryBuildPackageBehaviour(TestCaseWithFactory): | |||
202 | 98 | expected = yield self.makeExpectedInteraction( | 126 | expected = yield self.makeExpectedInteraction( |
203 | 99 | builder, build, chroot, archive, archive_purpose, component, | 127 | builder, build, chroot, archive, archive_purpose, component, |
204 | 100 | extra_uploads, filemap_names) | 128 | extra_uploads, filemap_names) |
206 | 101 | self.assertEqual(expected, call_log) | 129 | self.assertThat(call_log, MatchesListwise(expected)) |
207 | 102 | 130 | ||
208 | 103 | @defer.inlineCallbacks | 131 | @defer.inlineCallbacks |
209 | 104 | def makeExpectedInteraction(self, builder, build, chroot, archive, | 132 | def makeExpectedInteraction(self, builder, build, chroot, archive, |
210 | @@ -115,7 +143,7 @@ class TestBinaryBuildPackageBehaviour(TestCaseWithFactory): | |||
211 | 115 | builder. We specify this separately from the archive because | 143 | builder. We specify this separately from the archive because |
212 | 116 | sometimes the behaviour object has to give a different purpose | 144 | sometimes the behaviour object has to give a different purpose |
213 | 117 | in order to trick the slave into building correctly. | 145 | in order to trick the slave into building correctly. |
215 | 118 | :return: A list of the calls we expect to be made. | 146 | :return: A list of matchers for the calls we expect to be made. |
216 | 119 | """ | 147 | """ |
217 | 120 | das = build.distro_arch_series | 148 | das = build.distro_arch_series |
218 | 121 | ds_name = das.distroseries.name | 149 | ds_name = das.distroseries.name |
219 | @@ -131,8 +159,9 @@ class TestBinaryBuildPackageBehaviour(TestCaseWithFactory): | |||
220 | 131 | extra_uploads = [] | 159 | extra_uploads = [] |
221 | 132 | 160 | ||
222 | 133 | upload_logs = [ | 161 | upload_logs = [ |
225 | 134 | ('ensurepresent',) + upload | 162 | MatchesListwise((Equals('ensurepresent'),) + upload) |
226 | 135 | for upload in [(chroot.http_url, '', '')] + extra_uploads] | 163 | for upload in [(Equals(chroot.http_url), Equals(''), Equals(''))] + |
227 | 164 | extra_uploads] | ||
228 | 136 | 165 | ||
229 | 137 | extra_args = { | 166 | extra_args = { |
230 | 138 | 'arch_indep': arch_indep, | 167 | 'arch_indep': arch_indep, |
231 | @@ -151,8 +180,9 @@ class TestBinaryBuildPackageBehaviour(TestCaseWithFactory): | |||
232 | 151 | 'trusted_keys': trusted_keys, | 180 | 'trusted_keys': trusted_keys, |
233 | 152 | } | 181 | } |
234 | 153 | build_log = [ | 182 | build_log = [ |
237 | 154 | ('build', build.build_cookie, 'binarypackage', | 183 | MatchesListwise([Equals(arg) for arg in ( |
238 | 155 | chroot.content.sha1, filemap_names, extra_args)] | 184 | 'build', build.build_cookie, 'binarypackage', |
239 | 185 | chroot.content.sha1, filemap_names, extra_args)])] | ||
240 | 156 | result = upload_logs + build_log | 186 | result = upload_logs + build_log |
241 | 157 | defer.returnValue(result) | 187 | defer.returnValue(result) |
242 | 158 | 188 | ||
243 | @@ -236,6 +266,8 @@ class TestBinaryBuildPackageBehaviour(TestCaseWithFactory): | |||
244 | 236 | 266 | ||
245 | 237 | @defer.inlineCallbacks | 267 | @defer.inlineCallbacks |
246 | 238 | def test_private_source_dispatch(self): | 268 | def test_private_source_dispatch(self): |
247 | 269 | self.pushConfig( | ||
248 | 270 | "launchpad", internal_macaroon_secret_key="some-secret") | ||
249 | 239 | archive = self.factory.makeArchive(private=True) | 271 | archive = self.factory.makeArchive(private=True) |
250 | 240 | slave = OkSlave() | 272 | slave = OkSlave() |
251 | 241 | builder = self.factory.makeBuilder() | 273 | builder = self.factory.makeBuilder() |
252 | @@ -246,13 +278,6 @@ class TestBinaryBuildPackageBehaviour(TestCaseWithFactory): | |||
253 | 246 | sprf = build.source_package_release.addFile( | 278 | sprf = build.source_package_release.addFile( |
254 | 247 | self.factory.makeLibraryFileAlias(db_only=True), | 279 | self.factory.makeLibraryFileAlias(db_only=True), |
255 | 248 | filetype=SourcePackageFileType.ORIG_TARBALL) | 280 | filetype=SourcePackageFileType.ORIG_TARBALL) |
256 | 249 | sprf_url = ( | ||
257 | 250 | 'http://private-ppa.launchpad.test/%s/%s/ubuntu/pool/%s/%s' | ||
258 | 251 | % (archive.owner.name, archive.name, | ||
259 | 252 | poolify( | ||
260 | 253 | build.source_package_release.sourcepackagename.name, | ||
261 | 254 | 'main'), | ||
262 | 255 | sprf.libraryfile.filename)) | ||
263 | 256 | lf = self.factory.makeLibraryFileAlias() | 281 | lf = self.factory.makeLibraryFileAlias() |
264 | 257 | transaction.commit() | 282 | transaction.commit() |
265 | 258 | build.distro_arch_series.addOrUpdateChroot(lf) | 283 | build.distro_arch_series.addOrUpdateChroot(lf) |
266 | @@ -264,7 +289,9 @@ class TestBinaryBuildPackageBehaviour(TestCaseWithFactory): | |||
267 | 264 | interactor.getBuildBehaviour(bq, builder, slave), BufferLogger()) | 289 | interactor.getBuildBehaviour(bq, builder, slave), BufferLogger()) |
268 | 265 | yield self.assertExpectedInteraction( | 290 | yield self.assertExpectedInteraction( |
269 | 266 | slave.call_log, builder, build, lf, archive, ArchivePurpose.PPA, | 291 | slave.call_log, builder, build, lf, archive, ArchivePurpose.PPA, |
271 | 267 | extra_uploads=[(sprf_url, 'buildd', 'sekrit')], | 292 | extra_uploads=[( |
272 | 293 | Equals(sprf.libraryfile.https_url), Equals(''), | ||
273 | 294 | BinaryPackageBuildMacaroonVerifies(build, sprf.libraryfile))], | ||
274 | 268 | filemap_names=[sprf.libraryfile.filename]) | 295 | filemap_names=[sprf.libraryfile.filename]) |
275 | 269 | 296 | ||
276 | 270 | @defer.inlineCallbacks | 297 | @defer.inlineCallbacks |
I'm going to reject this and replace it with a new reduced version, since master has moved on quite a bit and part of this has essentially been merged.