launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #22142
[Merge] lp:~cjwatson/launchpad/snap-allow-network into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-allow-network into lp:launchpad.
Commit message:
Add Snap.allow_network: if false, do not dispatch a proxy token to builds of that snap.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-allow-network/+merge/336924
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.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-allow-network into lp:launchpad.
=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py 2016-11-12 21:02:10 +0000
+++ lib/lp/scripts/garbo.py 2018-01-31 14:51:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Database garbage collection."""
@@ -41,6 +41,7 @@
Or,
Row,
SQL,
+ Update,
)
from storm.info import ClassAlias
from storm.store import EmptyResultSet
@@ -1603,6 +1604,32 @@
transaction.commit()
+class SnapAllowNetworkPopulator(TunableLoop):
+ """Populates Snap.allow_network with True."""
+
+ maximum_chunk_size = 5000
+
+ def __init__(self, log, abort_time=None):
+ super(SnapAllowNetworkPopulator, self).__init__(log, abort_time)
+ self.start_at = 1
+ self.store = IMasterStore(Snap)
+
+ def findSnaps(self):
+ return self.store.find(
+ Snap,
+ Snap.id >= self.start_at,
+ Snap._allow_network == None).order_by(Snap.id)
+
+ def isDone(self):
+ return self.findSnaps().is_empty()
+
+ def __call__(self, chunk_size):
+ ids = [snap.id for snap in self.findSnaps()]
+ self.store.execute(Update(
+ {Snap._allow_network: True}, where=Snap.id.is_in(ids), table=Snap))
+ transaction.commit()
+
+
class BaseDatabaseGarbageCollector(LaunchpadCronScript):
"""Abstract base class to run a collection of TunableLoops."""
script_name = None # Script name for locking and database user. Override.
@@ -1893,6 +1920,7 @@
ProductVCSPopulator,
RevisionAuthorEmailLinker,
ScrubPOFileTranslator,
+ SnapAllowNetworkPopulator,
SnapBuildJobPruner,
SnapStoreSeriesPopulator,
SuggestiveTemplatesCacheUpdater,
=== modified file 'lib/lp/scripts/tests/test_garbo.py'
--- lib/lp/scripts/tests/test_garbo.py 2018-01-02 10:54:31 +0000
+++ lib/lp/scripts/tests/test_garbo.py 2018-01-31 14:51:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test the database garbage collector."""
@@ -15,6 +15,7 @@
from StringIO import StringIO
import time
+from psycopg2 import IntegrityError
from pytz import UTC
from storm.exceptions import LostObjectError
from storm.expr import (
@@ -1553,6 +1554,34 @@
# Snaps with more than one possible store series are untouched.
self.assertIsNone(snaps[5].store_series)
+ def test_SnapAllowNetworkPopulator(self):
+ switch_dbuser('testadmin')
+ old_snaps = [self.factory.makeSnap() for _ in range(2)]
+ for snap in old_snaps:
+ removeSecurityProxy(snap)._allow_network = None
+ try:
+ Store.of(old_snaps[0]).flush()
+ except IntegrityError:
+ # Now enforced by DB NOT NULL constraint; backfilling is no
+ # longer necessary.
+ return
+ allow_network_snaps = [
+ self.factory.makeSnap(allow_network=True) for _ in range(2)]
+ disallow_network_snaps = [
+ self.factory.makeSnap(allow_network=False) for _ in range(2)]
+ transaction.commit()
+
+ self.runDaily()
+
+ # Old snaps are backfilled.
+ for snap in old_snaps:
+ self.assertIs(True, removeSecurityProxy(snap)._allow_network)
+ # Other snaps are left alone.
+ for snap in allow_network_snaps:
+ self.assertIs(True, removeSecurityProxy(snap)._allow_network)
+ for snap in disallow_network_snaps:
+ self.assertIs(False, removeSecurityProxy(snap)._allow_network)
+
class TestGarboTasks(TestCaseWithFactory):
layer = LaunchpadZopelessLayer
=== modified file 'lib/lp/snappy/browser/snap.py'
--- lib/lp/snappy/browser/snap.py 2017-03-27 19:28:36 +0000
+++ lib/lp/snappy/browser/snap.py 2018-01-31 14:51:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Snap views."""
@@ -319,6 +319,7 @@
'name',
'private',
'require_virtualized',
+ 'allow_network',
'auto_build',
'store_upload',
])
@@ -648,7 +649,7 @@
page_title = 'Administer'
- field_names = ['private', 'require_virtualized']
+ field_names = ['private', 'require_virtualized', 'allow_network']
def validate(self, data):
super(SnapAdminView, self).validate(data)
=== modified file 'lib/lp/snappy/browser/tests/test_snap.py'
--- lib/lp/snappy/browser/tests/test_snap.py 2017-10-20 13:35:42 +0000
+++ lib/lp/snappy/browser/tests/test_snap.py 2018-01-31 14:51:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test snap package views."""
@@ -573,7 +573,7 @@
user=self.person)
def test_admin_snap(self):
- # Admins can change require_virtualized and privacy.
+ # Admins can change require_virtualized, privacy, and allow_network.
login("admin@xxxxxxxxxxxxx")
commercial_admin = self.factory.makePerson(
member_of=[getUtility(ILaunchpadCelebrities).commercial_admin])
@@ -581,16 +581,19 @@
snap = self.factory.makeSnap(registrant=self.person)
self.assertTrue(snap.require_virtualized)
self.assertFalse(snap.private)
+ self.assertTrue(snap.allow_network)
browser = self.getViewBrowser(snap, user=commercial_admin)
browser.getLink("Administer snap package").click()
browser.getControl("Require virtualized builders").selected = False
browser.getControl("Private").selected = True
+ browser.getControl("Allow external network access").selected = False
browser.getControl("Update snap package").click()
login_person(self.person)
self.assertFalse(snap.require_virtualized)
self.assertTrue(snap.private)
+ self.assertFalse(snap.allow_network)
def test_admin_snap_privacy_mismatch(self):
# Cannot make snap public if it still contains private information.
=== modified file 'lib/lp/snappy/interfaces/snap.py'
--- lib/lp/snappy/interfaces/snap.py 2017-08-22 11:36:30 +0000
+++ lib/lp/snappy/interfaces/snap.py 2018-01-31 14:51:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Snap package interfaces."""
@@ -581,6 +581,13 @@
value_type=Reference(schema=IProcessor),
readonly=False))
+ allow_network = exported(Bool(
+ title=_("Allow external network access"),
+ required=True, readonly=False,
+ description=_(
+ "Allow access to external network resources via a proxy. "
+ "Resources hosted on Launchpad itself are always allowed.")))
+
class ISnap(
ISnapView, ISnapEdit, ISnapEditableAttributes, ISnapAdminAttributes,
=== modified file 'lib/lp/snappy/model/snap.py'
--- lib/lp/snappy/model/snap.py 2017-11-10 11:23:27 +0000
+++ lib/lp/snappy/model/snap.py 2018-01-31 14:51:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -195,6 +195,21 @@
private = Bool(name='private')
+ _allow_network = Bool(name='allow_network')
+
+ @property
+ def allow_network(self):
+ # XXX cjwatson 2018-01-31: Remove once this column has been
+ # backfilled.
+ if self._allow_network is None:
+ return True
+ else:
+ return self._allow_network
+
+ @allow_network.setter
+ def allow_network(self, value):
+ self._allow_network = value
+
store_upload = Bool(name='store_upload', allow_none=False)
store_series_id = Int(name='store_series', allow_none=True)
@@ -209,8 +224,8 @@
def __init__(self, registrant, owner, distro_series, name,
description=None, branch=None, git_ref=None, auto_build=False,
auto_build_archive=None, auto_build_pocket=None,
- require_virtualized=True, date_created=DEFAULT,
- private=False, store_upload=False, store_series=None,
+ require_virtualized=True, date_created=DEFAULT, private=False,
+ allow_network=True, store_upload=False, store_series=None,
store_name=None, store_secrets=None, store_channels=None):
"""Construct a `Snap`."""
super(Snap, self).__init__()
@@ -228,6 +243,7 @@
self.date_created = date_created
self.date_last_modified = date_created
self.private = private
+ self.allow_network = allow_network
self.store_upload = store_upload
self.store_series = store_series
self.store_name = store_name
@@ -660,8 +676,9 @@
git_path=None, git_ref=None, auto_build=False,
auto_build_archive=None, auto_build_pocket=None,
require_virtualized=True, processors=None, date_created=DEFAULT,
- private=False, store_upload=False, store_series=None,
- store_name=None, store_secrets=None, store_channels=None):
+ private=False, allow_network=True, store_upload=False,
+ store_series=None, store_name=None, store_secrets=None,
+ store_channels=None):
"""See `ISnapSet`."""
if not registrant.inTeam(owner):
if owner.is_team:
@@ -703,9 +720,10 @@
auto_build_archive=auto_build_archive,
auto_build_pocket=auto_build_pocket,
require_virtualized=require_virtualized, date_created=date_created,
- private=private, store_upload=store_upload,
- store_series=store_series, store_name=store_name,
- store_secrets=store_secrets, store_channels=store_channels)
+ private=private, allow_network=allow_network,
+ store_upload=store_upload, store_series=store_series,
+ store_name=store_name, store_secrets=store_secrets,
+ store_channels=store_channels)
store.add(snap)
if processors is None:
=== modified file 'lib/lp/snappy/model/snapbuildbehaviour.py'
--- lib/lp/snappy/model/snapbuildbehaviour.py 2017-07-25 18:01:04 +0000
+++ lib/lp/snappy/model/snapbuildbehaviour.py 2018-01-31 14:51:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""An `IBuildFarmJobBehaviour` for `SnapBuild`.
@@ -83,7 +83,7 @@
"""
build = self.build
args = {}
- if config.snappy.builder_proxy_host:
+ if config.snappy.builder_proxy_host and build.snap.allow_network:
token = yield self._requestProxyToken()
args["proxy_url"] = (
"http://{username}:{password}@{host}:{port}".format(
=== modified file 'lib/lp/snappy/tests/test_snap.py'
--- lib/lp/snappy/tests/test_snap.py 2018-01-23 18:54:30 +0000
+++ lib/lp/snappy/tests/test_snap.py 2018-01-31 14:51:33 +0000
@@ -612,6 +612,7 @@
self.assertIsNone(snap.auto_build_pocket)
self.assertTrue(snap.require_virtualized)
self.assertFalse(snap.private)
+ self.assertTrue(snap.allow_network)
def test_creation_git(self):
# The metadata entries supplied when a Snap is created for a Git
@@ -632,6 +633,7 @@
self.assertIsNone(snap.auto_build_pocket)
self.assertTrue(snap.require_virtualized)
self.assertFalse(snap.private)
+ self.assertTrue(snap.allow_network)
def test_creation_git_url(self):
# A Snap can be backed directly by a URL for an external Git
@@ -1286,6 +1288,7 @@
self.assertIsNone(snap["git_path"])
self.assertIsNone(snap["git_ref_link"])
self.assertTrue(snap["require_virtualized"])
+ self.assertTrue(snap["allow_network"])
def test_new_git(self):
# Ensure Snap creation based on a Git branch works.
@@ -1306,6 +1309,7 @@
self.assertEqual(ref.path, snap["git_path"])
self.assertEqual(self.getURL(ref), snap["git_ref_link"])
self.assertTrue(snap["require_virtualized"])
+ self.assertTrue(snap["allow_network"])
def test_new_private(self):
# Ensure private Snap creation works.
=== modified file 'lib/lp/snappy/tests/test_snapbuildbehaviour.py'
--- lib/lp/snappy/tests/test_snapbuildbehaviour.py 2017-10-20 13:35:42 +0000
+++ lib/lp/snappy/tests/test_snapbuildbehaviour.py 2018-01-31 14:51:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test snap package build behaviour."""
@@ -396,6 +396,15 @@
]))
@defer.inlineCallbacks
+ def test_extraBuildArgs_disallow_network(self):
+ # If external network access is not allowed for the snap,
+ # _extraBuildArgs does not dispatch a proxy token.
+ job = self.makeJob(allow_network=False)
+ args = yield job._extraBuildArgs()
+ self.assertNotIn("proxy_url", args)
+ self.assertNotIn("revocation_endpoint", args)
+
+ @defer.inlineCallbacks
def test_composeBuildRequest_proxy_url_set(self):
job = self.makeJob()
build_request = yield job.composeBuildRequest(None)
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2017-04-25 11:36:10 +0000
+++ lib/lp/testing/factory.py 2018-01-31 14:51:33 +0000
@@ -2,7 +2,7 @@
# NOTE: The first line above must stay first; do not move the copyright
# notice to the top. See http://www.python.org/dev/peps/pep-0263/.
#
-# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Testing infrastructure for the Launchpad application.
@@ -4650,9 +4650,9 @@
name=None, branch=None, git_ref=None, auto_build=False,
auto_build_archive=None, auto_build_pocket=None,
is_stale=None, require_virtualized=True, processors=None,
- date_created=DEFAULT, private=False, store_upload=False,
- store_series=None, store_name=None, store_secrets=None,
- store_channels=None):
+ date_created=DEFAULT, private=False, allow_network=True,
+ store_upload=False, store_series=None, store_name=None,
+ store_secrets=None, store_channels=None):
"""Make a new Snap."""
if registrant is None:
registrant = self.makePerson()
@@ -4676,9 +4676,9 @@
date_created=date_created, branch=branch, git_ref=git_ref,
auto_build=auto_build, auto_build_archive=auto_build_archive,
auto_build_pocket=auto_build_pocket, private=private,
- store_upload=store_upload, store_series=store_series,
- store_name=store_name, store_secrets=store_secrets,
- store_channels=store_channels)
+ allow_network=allow_network, store_upload=store_upload,
+ store_series=store_series, store_name=store_name,
+ store_secrets=store_secrets, store_channels=store_channels)
if is_stale is not None:
removeSecurityProxy(snap).is_stale = is_stale
IStore(snap).flush()
Follow ups
