Merge lp:~cjwatson/launchpad/snap-allow-network into lp:launchpad

Proposed by Colin Watson on 2018-01-31
Status: Needs review
Proposed branch: lp:~cjwatson/launchpad/snap-allow-network
Merge into: lp:launchpad
Diff against target: 398 lines (+124/-25)
10 files modified
lib/lp/scripts/garbo.py (+29/-1)
lib/lp/scripts/tests/test_garbo.py (+30/-1)
lib/lp/snappy/browser/snap.py (+3/-2)
lib/lp/snappy/browser/tests/test_snap.py (+5/-2)
lib/lp/snappy/interfaces/snap.py (+8/-1)
lib/lp/snappy/model/snap.py (+26/-8)
lib/lp/snappy/model/snapbuildbehaviour.py (+2/-2)
lib/lp/snappy/tests/test_snap.py (+4/-0)
lib/lp/snappy/tests/test_snapbuildbehaviour.py (+10/-1)
lib/lp/testing/factory.py (+7/-7)
To merge this branch: bzr merge lp:~cjwatson/launchpad/snap-allow-network
Reviewer Review Type Date Requested Status
Launchpad code reviewers 2018-01-31 Pending
Review via email: mp+336924@code.launchpad.net

Commit Message

Add Snap.allow_network: if false, do not dispatch a proxy token to builds of that snap.

Description of the Change

This will allow snaps that are intended to be delivered with Ubuntu images to be restricted to build from only resources on Launchpad, and thus be reproducible, supportable, etc.

Requires https://code.launchpad.net/~cjwatson/launchpad/db-snap-allow-network/+merge/336923.

To post a comment you must log in.

Unmerged revisions

18537. By Colin Watson on 2018-01-31

Add Snap.allow_network: if false, do not dispatch a proxy token to builds of that snap.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/scripts/garbo.py'
2--- lib/lp/scripts/garbo.py 2016-11-12 21:02:10 +0000
3+++ lib/lp/scripts/garbo.py 2018-01-31 14:51:33 +0000
4@@ -1,4 +1,4 @@
5-# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
6+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
7 # GNU Affero General Public License version 3 (see the file LICENSE).
8
9 """Database garbage collection."""
10@@ -41,6 +41,7 @@
11 Or,
12 Row,
13 SQL,
14+ Update,
15 )
16 from storm.info import ClassAlias
17 from storm.store import EmptyResultSet
18@@ -1603,6 +1604,32 @@
19 transaction.commit()
20
21
22+class SnapAllowNetworkPopulator(TunableLoop):
23+ """Populates Snap.allow_network with True."""
24+
25+ maximum_chunk_size = 5000
26+
27+ def __init__(self, log, abort_time=None):
28+ super(SnapAllowNetworkPopulator, self).__init__(log, abort_time)
29+ self.start_at = 1
30+ self.store = IMasterStore(Snap)
31+
32+ def findSnaps(self):
33+ return self.store.find(
34+ Snap,
35+ Snap.id >= self.start_at,
36+ Snap._allow_network == None).order_by(Snap.id)
37+
38+ def isDone(self):
39+ return self.findSnaps().is_empty()
40+
41+ def __call__(self, chunk_size):
42+ ids = [snap.id for snap in self.findSnaps()]
43+ self.store.execute(Update(
44+ {Snap._allow_network: True}, where=Snap.id.is_in(ids), table=Snap))
45+ transaction.commit()
46+
47+
48 class BaseDatabaseGarbageCollector(LaunchpadCronScript):
49 """Abstract base class to run a collection of TunableLoops."""
50 script_name = None # Script name for locking and database user. Override.
51@@ -1893,6 +1920,7 @@
52 ProductVCSPopulator,
53 RevisionAuthorEmailLinker,
54 ScrubPOFileTranslator,
55+ SnapAllowNetworkPopulator,
56 SnapBuildJobPruner,
57 SnapStoreSeriesPopulator,
58 SuggestiveTemplatesCacheUpdater,
59
60=== modified file 'lib/lp/scripts/tests/test_garbo.py'
61--- lib/lp/scripts/tests/test_garbo.py 2018-01-02 10:54:31 +0000
62+++ lib/lp/scripts/tests/test_garbo.py 2018-01-31 14:51:33 +0000
63@@ -1,4 +1,4 @@
64-# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
65+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
66 # GNU Affero General Public License version 3 (see the file LICENSE).
67
68 """Test the database garbage collector."""
69@@ -15,6 +15,7 @@
70 from StringIO import StringIO
71 import time
72
73+from psycopg2 import IntegrityError
74 from pytz import UTC
75 from storm.exceptions import LostObjectError
76 from storm.expr import (
77@@ -1553,6 +1554,34 @@
78 # Snaps with more than one possible store series are untouched.
79 self.assertIsNone(snaps[5].store_series)
80
81+ def test_SnapAllowNetworkPopulator(self):
82+ switch_dbuser('testadmin')
83+ old_snaps = [self.factory.makeSnap() for _ in range(2)]
84+ for snap in old_snaps:
85+ removeSecurityProxy(snap)._allow_network = None
86+ try:
87+ Store.of(old_snaps[0]).flush()
88+ except IntegrityError:
89+ # Now enforced by DB NOT NULL constraint; backfilling is no
90+ # longer necessary.
91+ return
92+ allow_network_snaps = [
93+ self.factory.makeSnap(allow_network=True) for _ in range(2)]
94+ disallow_network_snaps = [
95+ self.factory.makeSnap(allow_network=False) for _ in range(2)]
96+ transaction.commit()
97+
98+ self.runDaily()
99+
100+ # Old snaps are backfilled.
101+ for snap in old_snaps:
102+ self.assertIs(True, removeSecurityProxy(snap)._allow_network)
103+ # Other snaps are left alone.
104+ for snap in allow_network_snaps:
105+ self.assertIs(True, removeSecurityProxy(snap)._allow_network)
106+ for snap in disallow_network_snaps:
107+ self.assertIs(False, removeSecurityProxy(snap)._allow_network)
108+
109
110 class TestGarboTasks(TestCaseWithFactory):
111 layer = LaunchpadZopelessLayer
112
113=== modified file 'lib/lp/snappy/browser/snap.py'
114--- lib/lp/snappy/browser/snap.py 2017-03-27 19:28:36 +0000
115+++ lib/lp/snappy/browser/snap.py 2018-01-31 14:51:33 +0000
116@@ -1,4 +1,4 @@
117-# Copyright 2015-2017 Canonical Ltd. This software is licensed under the
118+# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
119 # GNU Affero General Public License version 3 (see the file LICENSE).
120
121 """Snap views."""
122@@ -319,6 +319,7 @@
123 'name',
124 'private',
125 'require_virtualized',
126+ 'allow_network',
127 'auto_build',
128 'store_upload',
129 ])
130@@ -648,7 +649,7 @@
131
132 page_title = 'Administer'
133
134- field_names = ['private', 'require_virtualized']
135+ field_names = ['private', 'require_virtualized', 'allow_network']
136
137 def validate(self, data):
138 super(SnapAdminView, self).validate(data)
139
140=== modified file 'lib/lp/snappy/browser/tests/test_snap.py'
141--- lib/lp/snappy/browser/tests/test_snap.py 2017-10-20 13:35:42 +0000
142+++ lib/lp/snappy/browser/tests/test_snap.py 2018-01-31 14:51:33 +0000
143@@ -1,4 +1,4 @@
144-# Copyright 2015-2017 Canonical Ltd. This software is licensed under the
145+# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
146 # GNU Affero General Public License version 3 (see the file LICENSE).
147
148 """Test snap package views."""
149@@ -573,7 +573,7 @@
150 user=self.person)
151
152 def test_admin_snap(self):
153- # Admins can change require_virtualized and privacy.
154+ # Admins can change require_virtualized, privacy, and allow_network.
155 login("admin@canonical.com")
156 commercial_admin = self.factory.makePerson(
157 member_of=[getUtility(ILaunchpadCelebrities).commercial_admin])
158@@ -581,16 +581,19 @@
159 snap = self.factory.makeSnap(registrant=self.person)
160 self.assertTrue(snap.require_virtualized)
161 self.assertFalse(snap.private)
162+ self.assertTrue(snap.allow_network)
163
164 browser = self.getViewBrowser(snap, user=commercial_admin)
165 browser.getLink("Administer snap package").click()
166 browser.getControl("Require virtualized builders").selected = False
167 browser.getControl("Private").selected = True
168+ browser.getControl("Allow external network access").selected = False
169 browser.getControl("Update snap package").click()
170
171 login_person(self.person)
172 self.assertFalse(snap.require_virtualized)
173 self.assertTrue(snap.private)
174+ self.assertFalse(snap.allow_network)
175
176 def test_admin_snap_privacy_mismatch(self):
177 # Cannot make snap public if it still contains private information.
178
179=== modified file 'lib/lp/snappy/interfaces/snap.py'
180--- lib/lp/snappy/interfaces/snap.py 2017-08-22 11:36:30 +0000
181+++ lib/lp/snappy/interfaces/snap.py 2018-01-31 14:51:33 +0000
182@@ -1,4 +1,4 @@
183-# Copyright 2015-2017 Canonical Ltd. This software is licensed under the
184+# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
185 # GNU Affero General Public License version 3 (see the file LICENSE).
186
187 """Snap package interfaces."""
188@@ -581,6 +581,13 @@
189 value_type=Reference(schema=IProcessor),
190 readonly=False))
191
192+ allow_network = exported(Bool(
193+ title=_("Allow external network access"),
194+ required=True, readonly=False,
195+ description=_(
196+ "Allow access to external network resources via a proxy. "
197+ "Resources hosted on Launchpad itself are always allowed.")))
198+
199
200 class ISnap(
201 ISnapView, ISnapEdit, ISnapEditableAttributes, ISnapAdminAttributes,
202
203=== modified file 'lib/lp/snappy/model/snap.py'
204--- lib/lp/snappy/model/snap.py 2017-11-10 11:23:27 +0000
205+++ lib/lp/snappy/model/snap.py 2018-01-31 14:51:33 +0000
206@@ -1,4 +1,4 @@
207-# Copyright 2015-2017 Canonical Ltd. This software is licensed under the
208+# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
209 # GNU Affero General Public License version 3 (see the file LICENSE).
210
211 __metaclass__ = type
212@@ -195,6 +195,21 @@
213
214 private = Bool(name='private')
215
216+ _allow_network = Bool(name='allow_network')
217+
218+ @property
219+ def allow_network(self):
220+ # XXX cjwatson 2018-01-31: Remove once this column has been
221+ # backfilled.
222+ if self._allow_network is None:
223+ return True
224+ else:
225+ return self._allow_network
226+
227+ @allow_network.setter
228+ def allow_network(self, value):
229+ self._allow_network = value
230+
231 store_upload = Bool(name='store_upload', allow_none=False)
232
233 store_series_id = Int(name='store_series', allow_none=True)
234@@ -209,8 +224,8 @@
235 def __init__(self, registrant, owner, distro_series, name,
236 description=None, branch=None, git_ref=None, auto_build=False,
237 auto_build_archive=None, auto_build_pocket=None,
238- require_virtualized=True, date_created=DEFAULT,
239- private=False, store_upload=False, store_series=None,
240+ require_virtualized=True, date_created=DEFAULT, private=False,
241+ allow_network=True, store_upload=False, store_series=None,
242 store_name=None, store_secrets=None, store_channels=None):
243 """Construct a `Snap`."""
244 super(Snap, self).__init__()
245@@ -228,6 +243,7 @@
246 self.date_created = date_created
247 self.date_last_modified = date_created
248 self.private = private
249+ self.allow_network = allow_network
250 self.store_upload = store_upload
251 self.store_series = store_series
252 self.store_name = store_name
253@@ -660,8 +676,9 @@
254 git_path=None, git_ref=None, auto_build=False,
255 auto_build_archive=None, auto_build_pocket=None,
256 require_virtualized=True, processors=None, date_created=DEFAULT,
257- private=False, store_upload=False, store_series=None,
258- store_name=None, store_secrets=None, store_channels=None):
259+ private=False, allow_network=True, store_upload=False,
260+ store_series=None, store_name=None, store_secrets=None,
261+ store_channels=None):
262 """See `ISnapSet`."""
263 if not registrant.inTeam(owner):
264 if owner.is_team:
265@@ -703,9 +720,10 @@
266 auto_build_archive=auto_build_archive,
267 auto_build_pocket=auto_build_pocket,
268 require_virtualized=require_virtualized, date_created=date_created,
269- private=private, store_upload=store_upload,
270- store_series=store_series, store_name=store_name,
271- store_secrets=store_secrets, store_channels=store_channels)
272+ private=private, allow_network=allow_network,
273+ store_upload=store_upload, store_series=store_series,
274+ store_name=store_name, store_secrets=store_secrets,
275+ store_channels=store_channels)
276 store.add(snap)
277
278 if processors is None:
279
280=== modified file 'lib/lp/snappy/model/snapbuildbehaviour.py'
281--- lib/lp/snappy/model/snapbuildbehaviour.py 2017-07-25 18:01:04 +0000
282+++ lib/lp/snappy/model/snapbuildbehaviour.py 2018-01-31 14:51:33 +0000
283@@ -1,4 +1,4 @@
284-# Copyright 2015-2017 Canonical Ltd. This software is licensed under the
285+# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
286 # GNU Affero General Public License version 3 (see the file LICENSE).
287
288 """An `IBuildFarmJobBehaviour` for `SnapBuild`.
289@@ -83,7 +83,7 @@
290 """
291 build = self.build
292 args = {}
293- if config.snappy.builder_proxy_host:
294+ if config.snappy.builder_proxy_host and build.snap.allow_network:
295 token = yield self._requestProxyToken()
296 args["proxy_url"] = (
297 "http://{username}:{password}@{host}:{port}".format(
298
299=== modified file 'lib/lp/snappy/tests/test_snap.py'
300--- lib/lp/snappy/tests/test_snap.py 2018-01-23 18:54:30 +0000
301+++ lib/lp/snappy/tests/test_snap.py 2018-01-31 14:51:33 +0000
302@@ -612,6 +612,7 @@
303 self.assertIsNone(snap.auto_build_pocket)
304 self.assertTrue(snap.require_virtualized)
305 self.assertFalse(snap.private)
306+ self.assertTrue(snap.allow_network)
307
308 def test_creation_git(self):
309 # The metadata entries supplied when a Snap is created for a Git
310@@ -632,6 +633,7 @@
311 self.assertIsNone(snap.auto_build_pocket)
312 self.assertTrue(snap.require_virtualized)
313 self.assertFalse(snap.private)
314+ self.assertTrue(snap.allow_network)
315
316 def test_creation_git_url(self):
317 # A Snap can be backed directly by a URL for an external Git
318@@ -1286,6 +1288,7 @@
319 self.assertIsNone(snap["git_path"])
320 self.assertIsNone(snap["git_ref_link"])
321 self.assertTrue(snap["require_virtualized"])
322+ self.assertTrue(snap["allow_network"])
323
324 def test_new_git(self):
325 # Ensure Snap creation based on a Git branch works.
326@@ -1306,6 +1309,7 @@
327 self.assertEqual(ref.path, snap["git_path"])
328 self.assertEqual(self.getURL(ref), snap["git_ref_link"])
329 self.assertTrue(snap["require_virtualized"])
330+ self.assertTrue(snap["allow_network"])
331
332 def test_new_private(self):
333 # Ensure private Snap creation works.
334
335=== modified file 'lib/lp/snappy/tests/test_snapbuildbehaviour.py'
336--- lib/lp/snappy/tests/test_snapbuildbehaviour.py 2017-10-20 13:35:42 +0000
337+++ lib/lp/snappy/tests/test_snapbuildbehaviour.py 2018-01-31 14:51:33 +0000
338@@ -1,4 +1,4 @@
339-# Copyright 2015-2017 Canonical Ltd. This software is licensed under the
340+# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
341 # GNU Affero General Public License version 3 (see the file LICENSE).
342
343 """Test snap package build behaviour."""
344@@ -396,6 +396,15 @@
345 ]))
346
347 @defer.inlineCallbacks
348+ def test_extraBuildArgs_disallow_network(self):
349+ # If external network access is not allowed for the snap,
350+ # _extraBuildArgs does not dispatch a proxy token.
351+ job = self.makeJob(allow_network=False)
352+ args = yield job._extraBuildArgs()
353+ self.assertNotIn("proxy_url", args)
354+ self.assertNotIn("revocation_endpoint", args)
355+
356+ @defer.inlineCallbacks
357 def test_composeBuildRequest_proxy_url_set(self):
358 job = self.makeJob()
359 build_request = yield job.composeBuildRequest(None)
360
361=== modified file 'lib/lp/testing/factory.py'
362--- lib/lp/testing/factory.py 2017-04-25 11:36:10 +0000
363+++ lib/lp/testing/factory.py 2018-01-31 14:51:33 +0000
364@@ -2,7 +2,7 @@
365 # NOTE: The first line above must stay first; do not move the copyright
366 # notice to the top. See http://www.python.org/dev/peps/pep-0263/.
367 #
368-# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
369+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
370 # GNU Affero General Public License version 3 (see the file LICENSE).
371
372 """Testing infrastructure for the Launchpad application.
373@@ -4650,9 +4650,9 @@
374 name=None, branch=None, git_ref=None, auto_build=False,
375 auto_build_archive=None, auto_build_pocket=None,
376 is_stale=None, require_virtualized=True, processors=None,
377- date_created=DEFAULT, private=False, store_upload=False,
378- store_series=None, store_name=None, store_secrets=None,
379- store_channels=None):
380+ date_created=DEFAULT, private=False, allow_network=True,
381+ store_upload=False, store_series=None, store_name=None,
382+ store_secrets=None, store_channels=None):
383 """Make a new Snap."""
384 if registrant is None:
385 registrant = self.makePerson()
386@@ -4676,9 +4676,9 @@
387 date_created=date_created, branch=branch, git_ref=git_ref,
388 auto_build=auto_build, auto_build_archive=auto_build_archive,
389 auto_build_pocket=auto_build_pocket, private=private,
390- store_upload=store_upload, store_series=store_series,
391- store_name=store_name, store_secrets=store_secrets,
392- store_channels=store_channels)
393+ allow_network=allow_network, store_upload=store_upload,
394+ store_series=store_series, store_name=store_name,
395+ store_secrets=store_secrets, store_channels=store_channels)
396 if is_stale is not None:
397 removeSecurityProxy(snap).is_stale = is_stale
398 IStore(snap).flush()