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: Rejected
Rejected by: Simone Pelosi
Proposed branch: ~pelpsi/launchpad:request-token-and-session-from-fetch-service
Merge into: launchpad:master
Prerequisite: ~ines-almeida/launchpad:fetch-service-option-model-update
Diff against target: 356 lines (+152/-27)
7 files modified
lib/lp/buildmaster/builderproxy.py (+68/-3)
lib/lp/buildmaster/downloader.py (+36/-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 (+1/-0)
lib/lp/snappy/model/snap.py (+5/-1)
lib/lp/snappy/tests/test_snap.py (+0/-16)
Reviewer Review Type Date Requested Status
Launchpad code reviewers Pending
Review via email: mp+461586@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.

Unmerged commits

c311291... by Simone Pelosi

Added function to request fetch service session

Failed
[SUCCEEDED] docs:0 (build)
[FAILED] lint:0 (build)
[WAITING] mypy:0 (build)
1 → 3 of 3 results First • Previous • Next • Last
ae19d2a... by Simone Pelosi

First commit

d33a53f... by Ines Almeida

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

c7bb90d... 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

79a96e8... by Ines Almeida

Add Snap.use_fetch_service field to model and API

The field will only be updatable by admins

131462a... by Ines Almeida

Add Snap.use_fetch_service field to model and API

The field will only be updatable by admins. Although we can't hide the API endpoint itself, we are hidding the endpoint setting and getting behind a new feature flag "snap.fetch_service.enable"

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/snappy/browser/snap.py b/lib/lp/snappy/browser/snap.py
161index 5744565..dc4d330 100644
162--- a/lib/lp/snappy/browser/snap.py
163+++ b/lib/lp/snappy/browser/snap.py
164@@ -77,6 +77,7 @@ from lp.snappy.browser.widgets.snaparchive import SnapArchiveWidget
165 from lp.snappy.browser.widgets.storechannels import StoreChannelsWidget
166 from lp.snappy.interfaces.snap import (
167 SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG,
168+ SNAP_USE_FETCH_SERVICE_FEATURE_FLAG,
169 CannotAuthorizeStoreUploads,
170 CannotFetchSnapcraftYaml,
171 CannotParseSnapcraftYaml,
172@@ -525,6 +526,7 @@ class ISnapEditSchema(Interface):
173 "auto_build_channels",
174 "store_upload",
175 "pro_enable",
176+ "use_fetch_service",
177 ],
178 )
179
180@@ -929,13 +931,20 @@ class SnapAdminView(BaseSnapEditView):
181 # XXX pappacena 2021-02-19: Once we have the whole privacy work in
182 # place, we should move "project" and "information_type" from +admin
183 # page to +edit, to allow common users to edit this.
184- field_names = [
185- "project",
186- "information_type",
187- "require_virtualized",
188- "allow_internet",
189- "pro_enable",
190- ]
191+ @property
192+ def field_names(self):
193+ fields = [
194+ "project",
195+ "information_type",
196+ "require_virtualized",
197+ "allow_internet",
198+ "pro_enable",
199+ ]
200+
201+ if getFeatureFlag(SNAP_USE_FETCH_SERVICE_FEATURE_FLAG):
202+ fields.append("use_fetch_service")
203+
204+ return fields
205
206 @property
207 def initial_values(self):
208diff --git a/lib/lp/snappy/browser/tests/test_snap.py b/lib/lp/snappy/browser/tests/test_snap.py
209index cb4d4de..1127a50 100644
210--- a/lib/lp/snappy/browser/tests/test_snap.py
211+++ b/lib/lp/snappy/browser/tests/test_snap.py
212@@ -61,6 +61,7 @@ from lp.snappy.browser.snap import (
213 )
214 from lp.snappy.interfaces.snap import (
215 SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG,
216+ SNAP_USE_FETCH_SERVICE_FEATURE_FLAG,
217 CannotModifySnapProcessor,
218 ISnapSet,
219 SnapBuildRequestStatus,
220@@ -821,6 +822,11 @@ class TestSnapAdminView(BaseTestSnapView):
221
222 def test_admin_snap(self):
223 # Admins can change require_virtualized, privacy, and allow_internet.
224+
225+ self.useFixture(
226+ FeatureFixture({SNAP_USE_FETCH_SERVICE_FEATURE_FLAG: True})
227+ )
228+
229 login("admin@canonical.com")
230 admin = self.factory.makePerson(
231 member_of=[getUtility(ILaunchpadCelebrities).admin]
232@@ -835,6 +841,7 @@ class TestSnapAdminView(BaseTestSnapView):
233 self.assertFalse(snap.private)
234 self.assertTrue(snap.allow_internet)
235 self.assertFalse(snap.pro_enable)
236+ self.assertFalse(snap.use_fetch_service)
237
238 self.factory.makeAccessPolicy(
239 pillar=project, type=InformationType.PRIVATESECURITY
240@@ -847,6 +854,7 @@ class TestSnapAdminView(BaseTestSnapView):
241 browser.getControl(name="field.information_type").value = private
242 browser.getControl("Allow external network access").selected = False
243 browser.getControl("Enable Ubuntu Pro").selected = True
244+ browser.getControl("Use fetch service").selected = True
245 browser.getControl("Update snap package").click()
246
247 login_admin()
248@@ -855,6 +863,24 @@ class TestSnapAdminView(BaseTestSnapView):
249 self.assertTrue(snap.private)
250 self.assertFalse(snap.allow_internet)
251 self.assertTrue(snap.pro_enable)
252+ self.assertTrue(snap.use_fetch_service)
253+
254+ def test_admin_use_fetch_service_feature_flag(self):
255+ admin = self.factory.makePerson(
256+ member_of=[getUtility(ILaunchpadCelebrities).admin]
257+ )
258+ snap = self.factory.makeSnap(registrant=admin)
259+ browser = self.getViewBrowser(snap, user=admin)
260+
261+ browser.getLink("Administer snap package").click()
262+ self.assertFalse("Use fetch service" in browser.contents)
263+
264+ self.useFixture(
265+ FeatureFixture({SNAP_USE_FETCH_SERVICE_FEATURE_FLAG: True})
266+ )
267+
268+ browser.reload()
269+ self.assertTrue("Use fetch service" in browser.contents)
270
271 def test_admin_snap_private_without_project(self):
272 # Cannot make snap private if it doesn't have a project associated.
273diff --git a/lib/lp/snappy/interfaces/snap.py b/lib/lp/snappy/interfaces/snap.py
274index 8364c58..002c2a3 100644
275--- a/lib/lp/snappy/interfaces/snap.py
276+++ b/lib/lp/snappy/interfaces/snap.py
277@@ -22,6 +22,7 @@ __all__ = [
278 "MissingSnapcraftYaml",
279 "NoSourceForSnap",
280 "NoSuchSnap",
281+ "SNAP_USE_FETCH_SERVICE_FEATURE_FLAG",
282 "SNAP_SNAPCRAFT_CHANNEL_FEATURE_FLAG",
283 "SNAP_USE_FETCH_SERVICE_FEATURE_FLAG",
284 "SnapAuthorizationBadGeneratedMacaroon",
285diff --git a/lib/lp/snappy/model/snap.py b/lib/lp/snappy/model/snap.py
286index cf18262..8f70dcf 100644
287--- a/lib/lp/snappy/model/snap.py
288+++ b/lib/lp/snappy/model/snap.py
289@@ -396,7 +396,7 @@ class Snap(StormBase, WebhookTargetMixin):
290
291 _pro_enable = Bool(name="pro_enable", allow_none=True)
292
293- _use_fetch_service = Bool(name="use_fetch_service", allow_none=False)
294+ use_fetch_service = Bool(name="use_fetch_service", allow_none=False)
295
296 def __init__(
297 self,
298@@ -424,6 +424,7 @@ class Snap(StormBase, WebhookTargetMixin):
299 project=None,
300 pro_enable=False,
301 use_fetch_service=False,
302+ use_fetch_service=False,
303 ):
304 """Construct a `Snap`."""
305 super().__init__()
306@@ -460,6 +461,7 @@ class Snap(StormBase, WebhookTargetMixin):
307 self.store_channels = store_channels
308 self._pro_enable = pro_enable
309 self.use_fetch_service = use_fetch_service
310+ self.use_fetch_service = use_fetch_service
311
312 def __repr__(self):
313 return "<Snap ~%s/+snap/%s>" % (self.owner.name, self.name)
314@@ -1552,6 +1554,7 @@ class SnapSet:
315 project=None,
316 pro_enable=None,
317 use_fetch_service=False,
318+ use_fetch_service=False,
319 ):
320 """See `ISnapSet`."""
321 if not registrant.inTeam(owner):
322@@ -1637,6 +1640,7 @@ class SnapSet:
323 project=project,
324 pro_enable=pro_enable,
325 use_fetch_service=use_fetch_service,
326+ use_fetch_service=use_fetch_service,
327 )
328 store.add(snap)
329 snap._reconcileAccess()
330diff --git a/lib/lp/snappy/tests/test_snap.py b/lib/lp/snappy/tests/test_snap.py
331index c3dc03c..144a3ee 100644
332--- a/lib/lp/snappy/tests/test_snap.py
333+++ b/lib/lp/snappy/tests/test_snap.py
334@@ -2377,22 +2377,6 @@ class TestSnapSet(TestCaseWithFactory):
335 login_admin()
336 setattr(snap, field_name, True)
337
338- def test_snap_use_fetch_service_feature_flag(self):
339- # The snap.use_fetch_service API only works when feature flag is set
340- [ref] = self.factory.makeGitRefs()
341- components = self.makeSnapComponents(git_ref=ref)
342- snap = getUtility(ISnapSet).new(**components)
343-
344- login_admin()
345- self.assertEqual(None, snap.use_fetch_service)
346- snap.use_fetch_service = True
347- self.assertEqual(None, snap.use_fetch_service)
348-
349- with MemoryFeatureFixture({SNAP_USE_FETCH_SERVICE_FEATURE_FLAG: "on"}):
350- self.assertFalse(snap.use_fetch_service)
351- snap.use_fetch_service = True
352- self.assertTrue(snap.use_fetch_service)
353-
354 def test_create_private_snap_with_open_team_as_owner_fails(self):
355 components = self.makeSnapComponents()
356 with admin_logged_in():

Subscribers

People subscribed via source and target branches

to status/vote changes: