Launchpad itself

Merge ~pelpsi/launchpad:request-token-and-session-from-fetch-service into launchpad:master

  1. Git
  2. lp:~pelpsi/launchpad
  3. request-token-and-session-from-fetch-service
  4. Merge into master
Proposed by Simone Pelosi
Status: Superseded
Proposed branch: ~pelpsi/launchpad:request-token-and-session-from-fetch-service
Merge into: launchpad:master
Diff against target: 446 lines (+201/-11)
9 files modified
lib/lp/buildmaster/builderproxy.py (+68/-3)
lib/lp/buildmaster/downloader.py (+36/-0)
lib/lp/services/features/flags.py (+10/-0)
lib/lp/snappy/browser/snap.py (+16/-7)
lib/lp/snappy/browser/tests/test_snap.py (+26/-0)
lib/lp/snappy/interfaces/snap.py (+15/-0)
lib/lp/snappy/model/snap.py (+6/-0)
lib/lp/snappy/model/snapbuildbehaviour.py (+3/-1)
lib/lp/snappy/tests/test_snap.py (+21/-0)
Reviewer Review Type Date Requested Status
Launchpad code reviewers Pending
Review via email: mp+461458@code.launchpad.net

Commit message

Request fetch service session

NOTE: THIS IS A WIP! This message will be updated!

To post a comment you must log in.
~pelpsi/launchpad:request-token-and-session-from-fetch-service updated
81031ae... by Simone Pelosi

Add XXX comment

510379d... by Ines Almeida

Add Snap.use_fetch_service field to model and API

The field will only be updatable by admins

a9a5959... by Ines Almeida

Add UI for editing use_fetch_service in snap admin page

This is hidden by a new feature rule snap.fetch_service.enable by default

7486853... by Ines Almeida

test: add extra unit tests to verify that snap admin fields can only be updated by admin

920d684... by Simone Pelosi

Pass use_fetch_service as argument

Unmerged commits

920d684... by Simone Pelosi

Pass use_fetch_service as argument

Succeeded
[SUCCEEDED] docs:0 (build)
[SUCCEEDED] lint:0 (build)
[SUCCEEDED] mypy:0 (build)
1 → 3 of 3 results First • Previous • Next • Last
81031ae... by Simone Pelosi

Add XXX comment

dd43434... by Simone Pelosi

Added function to request fetch service session

ecd7236... by Simone Pelosi

First commit

7486853... by Ines Almeida

test: add extra unit tests to verify that snap admin fields can only be updated by admin

a9a5959... by Ines Almeida

Add UI for editing use_fetch_service in snap admin page

This is hidden by a new feature rule snap.fetch_service.enable by default

510379d... by Ines Almeida

Add Snap.use_fetch_service field to model and API

The field will only be updatable by admins

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/buildmaster/builderproxy.py b/lib/lp/buildmaster/builderproxy.py
2index be65e29..6857305 100644
3--- a/lib/lp/buildmaster/builderproxy.py
4+++ b/lib/lp/buildmaster/builderproxy.py
5@@ -20,7 +20,10 @@ from typing import Dict, Generator
6
7 from twisted.internet import defer
8
9-from lp.buildmaster.downloader import RequestProxyTokenCommand
10+from lp.buildmaster.downloader import (
11+ RequestFetchServiceSessionCommand,
12+ RequestProxyTokenCommand,
13+)
14 from lp.buildmaster.interfaces.builder import CannotBuild
15 from lp.buildmaster.interfaces.buildfarmjobbehaviour import BuildArgs
16 from lp.services.config import config
17@@ -36,9 +39,16 @@ class BuilderProxyMixin:
18
19 @defer.inlineCallbacks
20 def addProxyArgs(
21- self, args: BuildArgs, allow_internet: bool = True
22+ self,
23+ args: BuildArgs,
24+ allow_internet: bool = True,
25+ fetch_service: bool = False,
26 ) -> Generator[None, Dict[str, str], None]:
27- if _get_proxy_config("builder_proxy_host") and allow_internet:
28+ if (
29+ not fetch_service
30+ and _get_proxy_config("builder_proxy_host")
31+ and allow_internet
32+ ):
33 token = yield self._requestProxyToken()
34 args["proxy_url"] = (
35 "http://{username}:{password}@{host}:{port}".format(
36@@ -52,6 +62,25 @@ class BuilderProxyMixin:
37 endpoint=_get_proxy_config("builder_proxy_auth_api_endpoint"),
38 token=token["username"],
39 )
40+ if (
41+ fetch_service
42+ and _get_proxy_config("fetch_service_host")
43+ and allow_internet
44+ ):
45+ # http://<session-id>:<secret>@<addr>:9988/
46+ token = yield self._requestFetchServiceSession()
47+ args["proxy_url"] = (
48+ "http://{session_id}:{token}@{host}:{port}".format(
49+ session_id=token["id"],
50+ token=token["token"],
51+ host=_get_proxy_config("fetch_service_host"),
52+ port=_get_proxy_config("fetch_service_port"),
53+ )
54+ )
55+ args["revocation_endpoint"] = "{endpoint}/{token}".format(
56+ endpoint=_get_proxy_config("fetch_service_auth_api_endpoint"),
57+ token=token["username"],
58+ )
59
60 @defer.inlineCallbacks
61 def _requestProxyToken(self):
62@@ -86,3 +115,39 @@ class BuilderProxyMixin:
63 proxy_username=proxy_username,
64 )
65 return token
66+
67+ @defer.inlineCallbacks
68+ def _requestFetchServiceSession(self):
69+ # This is a different function if we have the needs to
70+ # differentiate more the behavior.
71+ admin_username = _get_proxy_config(
72+ "fetch_service_auth_api_admin_username"
73+ )
74+ if not admin_username:
75+ raise CannotBuild(
76+ "fetch_service_auth_api_admin_username is not configured."
77+ )
78+ secret = _get_proxy_config("fetch_service_auth_api_admin_secret")
79+ if not secret:
80+ raise CannotBuild(
81+ "fetch_service_auth_api_admin_secret is not configured."
82+ )
83+ url = _get_proxy_config("fetch_service_auth_api_endpoint")
84+ if not secret:
85+ raise CannotBuild(
86+ "fetch_service_auth_api_endpoint is not configured."
87+ )
88+ timestamp = int(time.time())
89+ proxy_username = "{build_id}-{timestamp}".format(
90+ build_id=self.build.build_cookie, timestamp=timestamp
91+ )
92+ auth_string = f"{admin_username}:{secret}".strip()
93+ auth_header = b"Basic " + base64.b64encode(auth_string.encode("ASCII"))
94+
95+ token = yield self._worker.process_pool.doWork(
96+ RequestFetchServiceSessionCommand,
97+ url=url,
98+ auth_header=auth_header,
99+ proxy_username=proxy_username,
100+ )
101+ return token
102diff --git a/lib/lp/buildmaster/downloader.py b/lib/lp/buildmaster/downloader.py
103index fc16852..ecc10f9 100644
104--- a/lib/lp/buildmaster/downloader.py
105+++ b/lib/lp/buildmaster/downloader.py
106@@ -10,6 +10,7 @@ anything from the rest of Launchpad.
107 __all__ = [
108 "DownloadCommand",
109 "RequestProcess",
110+ "RequestFetchServiceSessionCommand",
111 "RequestProxyTokenCommand",
112 ]
113
114@@ -37,6 +38,21 @@ class DownloadCommand(amp.Command):
115 }
116
117
118+class RequestFetchServiceSessionCommand(amp.Command):
119+ arguments = [
120+ (b"url", amp.Unicode()),
121+ (b"auth_header", amp.String()),
122+ (b"proxy_username", amp.Unicode()),
123+ ]
124+ response = [
125+ (b"id", amp.Unicode()),
126+ (b"token", amp.Unicode()),
127+ ]
128+ errors = {
129+ RequestException: b"REQUEST_ERROR",
130+ }
131+
132+
133 class RequestProxyTokenCommand(amp.Command):
134 arguments = [
135 (b"url", amp.Unicode()),
136@@ -94,3 +110,23 @@ class RequestProcess(AMPChild):
137 )
138 response.raise_for_status()
139 return response.json()
140+
141+ @RequestFetchServiceSessionCommand.responder
142+ def requestFetchServiceSessionCommand(
143+ self, url, auth_header, proxy_username
144+ ):
145+ with Session() as session:
146+ session.trust_env = False
147+ # XXX pelpsi 2024-02-28: should take the same
148+ # json body? From ST108 we should provide
149+ # {
150+ # "timeout": <int>, // session timeout in seconds
151+ # "policy": <string> // "strict" or "permissive"
152+ # }
153+ response = session.post(
154+ url,
155+ headers={"Authorization": auth_header},
156+ json={"username": proxy_username},
157+ )
158+ response.raise_for_status()
159+ return response.json()
160diff --git a/lib/lp/services/features/flags.py b/lib/lp/services/features/flags.py
161index f15a6df..ae0bf52 100644
162--- a/lib/lp/services/features/flags.py
163+++ b/lib/lp/services/features/flags.py
164@@ -304,6 +304,16 @@ flag_info = sorted(
165 "",
166 "",
167 ),
168+ (
169+ "snap.fetch_service.enable",
170+ "boolean",
171+ "If set, allow admins to set snap.use_fetch_service field, which "
172+ "sets a snap build to use the fetch-service instead of the "
173+ "builder-proxy",
174+ "",
175+ "",
176+ "",
177+ ),
178 ]
179 )
180
181diff --git a/lib/lp/snappy/browser/snap.py b/lib/lp/snappy/browser/snap.py
182index 5744565..dc4d330 100644
183--- a/lib/lp/snappy/browser/snap.py
184+++ b/lib/lp/snappy/browser/snap.py
185@@ -77,6 +77,7 @@ from lp.snappy.browser.widgets.snaparchive import SnapArchiveWidget
186 from lp.snappy.browser.widgets.storechannels import StoreChannelsWidget
187 from lp.snappy.interfaces.snap import (
188 SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG,
189+ SNAP_USE_FETCH_SERVICE_FEATURE_FLAG,
190 CannotAuthorizeStoreUploads,
191 CannotFetchSnapcraftYaml,
192 CannotParseSnapcraftYaml,
193@@ -525,6 +526,7 @@ class ISnapEditSchema(Interface):
194 "auto_build_channels",
195 "store_upload",
196 "pro_enable",
197+ "use_fetch_service",
198 ],
199 )
200
201@@ -929,13 +931,20 @@ class SnapAdminView(BaseSnapEditView):
202 # XXX pappacena 2021-02-19: Once we have the whole privacy work in
203 # place, we should move "project" and "information_type" from +admin
204 # page to +edit, to allow common users to edit this.
205- field_names = [
206- "project",
207- "information_type",
208- "require_virtualized",
209- "allow_internet",
210- "pro_enable",
211- ]
212+ @property
213+ def field_names(self):
214+ fields = [
215+ "project",
216+ "information_type",
217+ "require_virtualized",
218+ "allow_internet",
219+ "pro_enable",
220+ ]
221+
222+ if getFeatureFlag(SNAP_USE_FETCH_SERVICE_FEATURE_FLAG):
223+ fields.append("use_fetch_service")
224+
225+ return fields
226
227 @property
228 def initial_values(self):
229diff --git a/lib/lp/snappy/browser/tests/test_snap.py b/lib/lp/snappy/browser/tests/test_snap.py
230index cb4d4de..1127a50 100644
231--- a/lib/lp/snappy/browser/tests/test_snap.py
232+++ b/lib/lp/snappy/browser/tests/test_snap.py
233@@ -61,6 +61,7 @@ from lp.snappy.browser.snap import (
234 )
235 from lp.snappy.interfaces.snap import (
236 SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG,
237+ SNAP_USE_FETCH_SERVICE_FEATURE_FLAG,
238 CannotModifySnapProcessor,
239 ISnapSet,
240 SnapBuildRequestStatus,
241@@ -821,6 +822,11 @@ class TestSnapAdminView(BaseTestSnapView):
242
243 def test_admin_snap(self):
244 # Admins can change require_virtualized, privacy, and allow_internet.
245+
246+ self.useFixture(
247+ FeatureFixture({SNAP_USE_FETCH_SERVICE_FEATURE_FLAG: True})
248+ )
249+
250 login("admin@canonical.com")
251 admin = self.factory.makePerson(
252 member_of=[getUtility(ILaunchpadCelebrities).admin]
253@@ -835,6 +841,7 @@ class TestSnapAdminView(BaseTestSnapView):
254 self.assertFalse(snap.private)
255 self.assertTrue(snap.allow_internet)
256 self.assertFalse(snap.pro_enable)
257+ self.assertFalse(snap.use_fetch_service)
258
259 self.factory.makeAccessPolicy(
260 pillar=project, type=InformationType.PRIVATESECURITY
261@@ -847,6 +854,7 @@ class TestSnapAdminView(BaseTestSnapView):
262 browser.getControl(name="field.information_type").value = private
263 browser.getControl("Allow external network access").selected = False
264 browser.getControl("Enable Ubuntu Pro").selected = True
265+ browser.getControl("Use fetch service").selected = True
266 browser.getControl("Update snap package").click()
267
268 login_admin()
269@@ -855,6 +863,24 @@ class TestSnapAdminView(BaseTestSnapView):
270 self.assertTrue(snap.private)
271 self.assertFalse(snap.allow_internet)
272 self.assertTrue(snap.pro_enable)
273+ self.assertTrue(snap.use_fetch_service)
274+
275+ def test_admin_use_fetch_service_feature_flag(self):
276+ admin = self.factory.makePerson(
277+ member_of=[getUtility(ILaunchpadCelebrities).admin]
278+ )
279+ snap = self.factory.makeSnap(registrant=admin)
280+ browser = self.getViewBrowser(snap, user=admin)
281+
282+ browser.getLink("Administer snap package").click()
283+ self.assertFalse("Use fetch service" in browser.contents)
284+
285+ self.useFixture(
286+ FeatureFixture({SNAP_USE_FETCH_SERVICE_FEATURE_FLAG: True})
287+ )
288+
289+ browser.reload()
290+ self.assertTrue("Use fetch service" in browser.contents)
291
292 def test_admin_snap_private_without_project(self):
293 # Cannot make snap private if it doesn't have a project associated.
294diff --git a/lib/lp/snappy/interfaces/snap.py b/lib/lp/snappy/interfaces/snap.py
295index ae2e3df..249192b 100644
296--- a/lib/lp/snappy/interfaces/snap.py
297+++ b/lib/lp/snappy/interfaces/snap.py
298@@ -22,6 +22,7 @@ __all__ = [
299 "MissingSnapcraftYaml",
300 "NoSourceForSnap",
301 "NoSuchSnap",
302+ "SNAP_USE_FETCH_SERVICE_FEATURE_FLAG",
303 "SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG",
304 "SnapAuthorizationBadGeneratedMacaroon",
305 "SnapBuildAlreadyPending",
306@@ -101,6 +102,7 @@ from lp.soyuz.interfaces.archive import IArchive
307 from lp.soyuz.interfaces.distroarchseries import IDistroArchSeries
308
309 SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG = "snap.channels.snapcraft"
310+SNAP_USE_FETCH_SERVICE_FEATURE_FLAG = "snap.fetch_service.enable"
311
312
313 @error_status(http.client.BAD_REQUEST)
314@@ -1189,6 +1191,18 @@ class ISnapAdminAttributes(Interface):
315 )
316 )
317
318+ use_fetch_service = exported(
319+ Bool(
320+ title=_("Use fetch service"),
321+ required=True,
322+ readonly=False,
323+ description=_(
324+ "If set, Snap builds will use the fetch-service instead "
325+ "of the builder-proxy to access external resources."
326+ ),
327+ )
328+ )
329+
330 def subscribe(person, subscribed_by):
331 """Subscribe a person to this snap recipe."""
332
333@@ -1267,6 +1281,7 @@ class ISnapSet(Interface):
334 store_channels=None,
335 project=None,
336 pro_enable=None,
337+ use_fetch_service=False,
338 ):
339 """Create an `ISnap`."""
340
341diff --git a/lib/lp/snappy/model/snap.py b/lib/lp/snappy/model/snap.py
342index eeeb056..6ef1b3f 100644
343--- a/lib/lp/snappy/model/snap.py
344+++ b/lib/lp/snappy/model/snap.py
345@@ -394,6 +394,8 @@ class Snap(StormBase, WebhookTargetMixin):
346
347 _pro_enable = Bool(name="pro_enable", allow_none=True)
348
349+ use_fetch_service = Bool(name="use_fetch_service", allow_none=False)
350+
351 def __init__(
352 self,
353 registrant,
354@@ -419,6 +421,7 @@ class Snap(StormBase, WebhookTargetMixin):
355 store_channels=None,
356 project=None,
357 pro_enable=False,
358+ use_fetch_service=False,
359 ):
360 """Construct a `Snap`."""
361 super().__init__()
362@@ -454,6 +457,7 @@ class Snap(StormBase, WebhookTargetMixin):
363 self.store_secrets = store_secrets
364 self.store_channels = store_channels
365 self._pro_enable = pro_enable
366+ self.use_fetch_service = use_fetch_service
367
368 def __repr__(self):
369 return "<Snap ~%s/+snap/%s>" % (self.owner.name, self.name)
370@@ -1534,6 +1538,7 @@ class SnapSet:
371 store_channels=None,
372 project=None,
373 pro_enable=None,
374+ use_fetch_service=False,
375 ):
376 """See `ISnapSet`."""
377 if not registrant.inTeam(owner):
378@@ -1618,6 +1623,7 @@ class SnapSet:
379 store_channels=store_channels,
380 project=project,
381 pro_enable=pro_enable,
382+ use_fetch_service=use_fetch_service,
383 )
384 store.add(snap)
385 snap._reconcileAccess()
386diff --git a/lib/lp/snappy/model/snapbuildbehaviour.py b/lib/lp/snappy/model/snapbuildbehaviour.py
387index 66ae7cc..58fdd5f 100644
388--- a/lib/lp/snappy/model/snapbuildbehaviour.py
389+++ b/lib/lp/snappy/model/snapbuildbehaviour.py
390@@ -116,7 +116,9 @@ class SnapBuildBehaviour(BuilderProxyMixin, BuildFarmJobBehaviourBase):
391 """
392 build: ISnapBuild = self.build
393 args: BuildArgs = yield super().extraBuildArgs(logger=logger)
394- yield self.addProxyArgs(args, build.snap.allow_internet)
395+ yield self.addProxyArgs(
396+ args, build.snap.allow_internet, build.snap.use_fetch_service
397+ )
398 args["name"] = build.snap.store_name or build.snap.name
399 channels = build.channels or {}
400 if "snapcraft" not in channels:
401diff --git a/lib/lp/snappy/tests/test_snap.py b/lib/lp/snappy/tests/test_snap.py
402index 8f35aa1..bcda27e 100644
403--- a/lib/lp/snappy/tests/test_snap.py
404+++ b/lib/lp/snappy/tests/test_snap.py
405@@ -2318,6 +2318,7 @@ class TestSnapSet(TestCaseWithFactory):
406 self.assertTrue(snap.allow_internet)
407 self.assertFalse(snap.build_source_tarball)
408 self.assertFalse(snap.pro_enable)
409+ self.assertFalse(snap.use_fetch_service)
410
411 def test_creation_git(self):
412 # The metadata entries supplied when a Snap is created for a Git
413@@ -2342,6 +2343,7 @@ class TestSnapSet(TestCaseWithFactory):
414 self.assertTrue(snap.allow_internet)
415 self.assertFalse(snap.build_source_tarball)
416 self.assertFalse(snap.pro_enable)
417+ self.assertFalse(snap.use_fetch_service)
418
419 def test_creation_git_url(self):
420 # A Snap can be backed directly by a URL for an external Git
421@@ -2355,6 +2357,25 @@ class TestSnapSet(TestCaseWithFactory):
422 self.assertEqual(ref.path, snap.git_path)
423 self.assertEqual(ref, snap.git_ref)
424
425+ def test_edit_admin_only_fields(self):
426+ # The admin fields can only be updated by an admin
427+ [ref] = self.factory.makeGitRefs()
428+ components = self.makeSnapComponents(git_ref=ref)
429+ snap = getUtility(ISnapSet).new(**components)
430+
431+ admin_fields = [
432+ "allow_internet",
433+ "pro_enable",
434+ "require_virtualized",
435+ "use_fetch_service",
436+ ]
437+
438+ for field_name in admin_fields:
439+ login(ANONYMOUS)
440+ self.assertRaises(Unauthorized, setattr, snap, field_name, True)
441+ login_admin()
442+ setattr(snap, field_name, True)
443+
444 def test_create_private_snap_with_open_team_as_owner_fails(self):
445 components = self.makeSnapComponents()
446 with admin_logged_in():

Subscribers

People subscribed via source and target branches

to status/vote changes: