launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #29649
[Merge] ~cjwatson/launchpad-buildd:snap-pull-build-internet into launchpad-buildd:master
Colin Watson has proposed merging ~cjwatson/launchpad-buildd:snap-pull-build-internet into launchpad-buildd:master.
Commit message:
snap: Add option to disable proxy after pull phase
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-buildd/+git/launchpad-buildd/+merge/437027
We're still working on all the details, but this is a relatively simple way to let us lock down certain snap recipe builds a bit more.
I converted the existing revocation code to use `requests`, since that's been on my to-do list for a long time. There's a minor testing gap because the version of `responses` in Ubuntu 20.04 doesn't allow us a way to inspect the `timeout` argument passed when making the request, but we'll be able to fix that once we upgrade builders to Ubuntu 22.04, which is in the works anyway.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad-buildd:snap-pull-build-internet into launchpad-buildd:master.
diff --git a/debian/changelog b/debian/changelog
index af71f9c..a5371dc 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,6 +1,8 @@
launchpad-buildd (230) UNRELEASED; urgency=medium
* Apply black and isort.
+ * Add an option to disable the proxy after the "pull" phase of a snap
+ recipe build.
-- Colin Watson <cjwatson@xxxxxxxxxx> Tue, 07 Feb 2023 20:02:04 +0000
diff --git a/lpbuildd/proxy.py b/lpbuildd/proxy.py
index 6b4ade2..ea33dda 100644
--- a/lpbuildd/proxy.py
+++ b/lpbuildd/proxy.py
@@ -3,9 +3,7 @@
import base64
import io
-from urllib.error import HTTPError, URLError
from urllib.parse import urlparse
-from urllib.request import Request, urlopen
from twisted.application import strports
from twisted.internet import reactor
@@ -14,6 +12,8 @@ from twisted.python.compat import intToBytes
from twisted.web import http, proxy
from zope.interface import implementer
+from lpbuildd.util import RevokeProxyTokenError, revoke_proxy_token
+
class BuilderProxyClient(proxy.ProxyClient):
def __init__(self, command, rest, version, headers, data, father):
@@ -224,7 +224,7 @@ class BuildManagerProxyMixin:
"tcp:%s" % proxy_port, proxy_factory
)
self.proxy_service.setServiceParent(self._builder.service)
- if self.backend_name == "lxd":
+ if hasattr(self.backend, "ipv4_network"):
proxy_host = self.backend.ipv4_network.ip
else:
proxy_host = "localhost"
@@ -242,15 +242,7 @@ class BuildManagerProxyMixin:
if not self.revocation_endpoint:
return
self._builder.log("Revoking proxy token...\n")
- url = urlparse(self.proxy_url)
- auth = f"{url.username}:{url.password}"
- encoded_auth = base64.b64encode(auth.encode()).decode()
- headers = {"Authorization": f"Basic {encoded_auth}"}
- req = Request(self.revocation_endpoint, None, headers)
- req.get_method = lambda: "DELETE"
try:
- urlopen(req, timeout=15)
- except (HTTPError, URLError) as e:
- self._builder.log(
- f"Unable to revoke token for {url.username}: {e}"
- )
+ revoke_proxy_token(self.proxy_url, self.revocation_endpoint)
+ except RevokeProxyTokenError as e:
+ self._builder.log(f"{e}\n")
diff --git a/lpbuildd/snap.py b/lpbuildd/snap.py
index bc35997..d922242 100644
--- a/lpbuildd/snap.py
+++ b/lpbuildd/snap.py
@@ -46,6 +46,9 @@ class SnapBuildManager(BuildManagerProxyMixin, DebianBuildManager):
self.private = extra_args.get("private", False)
self.proxy_service = None
self.target_architectures = extra_args.get("target_architectures")
+ self.disable_proxy_after_pull = extra_args.get(
+ "disable_proxy_after_pull"
+ )
super().initiate(files, chroot, extra_args)
@@ -65,6 +68,18 @@ class SnapBuildManager(BuildManagerProxyMixin, DebianBuildManager):
args.extend(self.startProxy())
if self.revocation_endpoint:
args.extend(["--revocation-endpoint", self.revocation_endpoint])
+ if (
+ self.disable_proxy_after_pull
+ and self.proxy_url
+ and self.revocation_endpoint
+ ):
+ args.extend(
+ [
+ "--upstream-proxy-url",
+ self.proxy_url,
+ "--disable-proxy-after-pull",
+ ]
+ )
if self.branch is not None:
args.extend(["--branch", self.branch])
if self.git_repository is not None:
diff --git a/lpbuildd/target/build_snap.py b/lpbuildd/target/build_snap.py
index 6e4484b..44dcc1c 100644
--- a/lpbuildd/target/build_snap.py
+++ b/lpbuildd/target/build_snap.py
@@ -12,6 +12,7 @@ from lpbuildd.target.operation import Operation
from lpbuildd.target.proxy import BuilderProxyOperationMixin
from lpbuildd.target.snapstore import SnapStoreOperationMixin
from lpbuildd.target.vcs import VCSOperationMixin
+from lpbuildd.util import RevokeProxyTokenError, revoke_proxy_token
RETCODE_FAILURE_INSTALL = 200
RETCODE_FAILURE_BUILD = 201
@@ -93,6 +94,19 @@ class BuildSnap(
action="append",
help="build for the specified architectures",
)
+ parser.add_argument(
+ "--upstream-proxy-url",
+ help=(
+ "URL of the builder proxy upstream of the one run internally "
+ "by launchpad-buildd"
+ ),
+ )
+ parser.add_argument(
+ "--disable-proxy-after-pull",
+ default=False,
+ action="store_true",
+ help="disable proxy access after the pull phase has finished",
+ )
parser.add_argument("name", help="name of snap to build")
def install_svn_servers(self):
@@ -212,6 +226,18 @@ class BuildSnap(
],
cwd="/build",
)
+ if (
+ self.args.disable_proxy_after_pull
+ and self.args.upstream_proxy_url
+ and self.args.revocation_endpoint
+ ):
+ logger.info("Revoking proxy token...")
+ try:
+ revoke_proxy_token(
+ self.args.upstream_proxy_url, self.args.revocation_endpoint
+ )
+ except RevokeProxyTokenError as e:
+ logger.info(str(e))
def build(self):
"""Run all build, stage and snap phases."""
diff --git a/lpbuildd/target/tests/test_build_snap.py b/lpbuildd/target/tests/test_build_snap.py
index 1eb4f05..95bf948 100644
--- a/lpbuildd/target/tests/test_build_snap.py
+++ b/lpbuildd/target/tests/test_build_snap.py
@@ -1,6 +1,7 @@
# Copyright 2017-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
+import base64
import json
import os.path
import stat
@@ -545,6 +546,63 @@ class TestBuildSnap(TestCase):
),
)
+ @responses.activate
+ def test_pull_disable_proxy_after_pull(self):
+ self.useFixture(FakeLogger())
+ responses.add("DELETE", "http://proxy-auth.example/tokens/1";)
+ args = [
+ "buildsnap",
+ "--backend=fake",
+ "--series=xenial",
+ "--arch=amd64",
+ "1",
+ "--build-url",
+ "https://launchpad.example/build";,
+ "--branch",
+ "lp:foo",
+ "--proxy-url",
+ "http://localhost:8222/";,
+ "--upstream-proxy-url",
+ "http://username:password@proxy.example:3128/";,
+ "--revocation-endpoint",
+ "http://proxy-auth.example/tokens/1";,
+ "--disable-proxy-after-pull",
+ "test-snap",
+ ]
+ build_snap = parse_args(args=args).operation
+ build_snap.pull()
+ env = {
+ "SNAPCRAFT_LOCAL_SOURCES": "1",
+ "SNAPCRAFT_SETUP_CORE": "1",
+ "SNAPCRAFT_BUILD_INFO": "1",
+ "SNAPCRAFT_IMAGE_INFO": (
+ '{"build_url": "https://launchpad.example/build"}'
+ ),
+ "SNAPCRAFT_BUILD_ENVIRONMENT": "host",
+ "http_proxy": "http://localhost:8222/";,
+ "https_proxy": "http://localhost:8222/";,
+ "GIT_PROXY_COMMAND": "/usr/local/bin/lpbuildd-git-proxy",
+ "SNAPPY_STORE_NO_CDN": "1",
+ }
+ self.assertThat(
+ build_snap.backend.run.calls,
+ MatchesListwise(
+ [
+ RanBuildCommand(
+ ["snapcraft", "pull"], cwd="/build/test-snap", **env
+ ),
+ ]
+ ),
+ )
+ self.assertEqual(1, len(responses.calls))
+ request = responses.calls[0].request
+ auth = base64.b64encode(b"username:password").decode()
+ self.assertEqual(f"Basic {auth}", request.headers["Authorization"])
+ self.assertEqual("http://proxy-auth.example/tokens/1";, request.url)
+ # XXX cjwatson 2023-02-07: Ideally we'd check the timeout as well,
+ # but the version of responses in Ubuntu 20.04 doesn't store it
+ # anywhere we can get at it.
+
def test_pull_build_source_tarball(self):
args = [
"buildsnap",
diff --git a/lpbuildd/tests/fakebuilder.py b/lpbuildd/tests/fakebuilder.py
index 9d70df6..15a820a 100644
--- a/lpbuildd/tests/fakebuilder.py
+++ b/lpbuildd/tests/fakebuilder.py
@@ -16,6 +16,8 @@ import subprocess
from collections import defaultdict
from configparser import NoOptionError, NoSectionError
+from twisted.application import service
+
from lpbuildd.target.backend import Backend
from lpbuildd.util import set_personality, shell_escape
@@ -99,6 +101,9 @@ class FakeBuilder:
self._cachepath = tempdir
self._config = FakeConfig()
self.waitingfiles = {}
+ self.service = service.IServiceCollection(
+ service.Application("FakeBuilder")
+ )
for fake_method in (
"emptyLog",
"log",
diff --git a/lpbuildd/tests/test_charm.py b/lpbuildd/tests/test_charm.py
index d90049c..18736dd 100644
--- a/lpbuildd/tests/test_charm.py
+++ b/lpbuildd/tests/test_charm.py
@@ -1,9 +1,10 @@
# Copyright 2021 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
+import base64
import os
-from unittest import mock
+import responses
from fixtures import EnvironmentVariable, TempDir
from testtools import TestCase
from testtools.deferredruntest import AsynchronousDeferredRunTest
@@ -243,17 +244,23 @@ class TestCharmBuildManagerIteration(TestCase):
)
self.assertFalse(self.builder.wasCalled("buildFail"))
- @mock.patch("lpbuildd.proxy.urlopen")
- def test_revokeProxyToken(self, urlopen_mock):
- self.buildmanager.revocation_endpoint = "http://revoke_endpoint";
- self.buildmanager.proxy_url = "http://username:password@proxy_url";
+ @responses.activate
+ def test_revokeProxyToken(self):
+ responses.add(
+ "DELETE", f"http://proxy-auth.example/tokens/{self.buildid}";
+ )
+ self.buildmanager.revocation_endpoint = (
+ f"http://proxy-auth.example/tokens/{self.buildid}";
+ )
+ self.buildmanager.proxy_url = "http://username:password@proxy.example";
self.buildmanager.revokeProxyToken()
- self.assertEqual(1, urlopen_mock.call_count)
- args, kwargs = urlopen_mock.call_args
- request = args[0]
+ self.assertEqual(1, len(responses.calls))
+ request = responses.calls[0].request
+ auth = base64.b64encode(b"username:password").decode()
+ self.assertEqual(f"Basic {auth}", request.headers["Authorization"])
self.assertEqual(
- {"Authorization": "Basic dXNlcm5hbWU6cGFzc3dvcmQ="},
- request.headers,
+ f"http://proxy-auth.example/tokens/{self.buildid}";, request.url
)
- self.assertEqual("http://revoke_endpoint";, request.get_full_url())
- self.assertEqual({"timeout": 15}, kwargs)
+ # XXX cjwatson 2023-02-07: Ideally we'd check the timeout as well,
+ # but the version of responses in Ubuntu 20.04 doesn't store it
+ # anywhere we can get at it.
diff --git a/lpbuildd/tests/test_snap.py b/lpbuildd/tests/test_snap.py
index 3a5de6b..0e44093 100644
--- a/lpbuildd/tests/test_snap.py
+++ b/lpbuildd/tests/test_snap.py
@@ -1,9 +1,10 @@
# Copyright 2015-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
+import base64
import os
-from unittest import mock
+import responses
from fixtures import EnvironmentVariable, TempDir
from testtools import TestCase
from testtools.content import text_content
@@ -675,6 +676,36 @@ class TestSnapBuildManagerIteration(TestCase):
]
yield self.startBuild(args, expected_options)
+ @defer.inlineCallbacks
+ def test_iterate_disable_proxy_after_pull(self):
+ self.builder._config.set("builder", "proxyport", "8222")
+ args = {
+ "disable_proxy_after_pull": True,
+ "git_repository": "https://git.launchpad.dev/~example/+git/snap";,
+ "git_path": "master",
+ "proxy_url": "http://username:password@proxy.example/";,
+ "revocation_endpoint": (
+ f"http://proxy-auth.example/tokens/{self.buildid}";
+ ),
+ }
+ expected_options = [
+ "--proxy-url",
+ "http://localhost:8222/";,
+ "--revocation-endpoint",
+ f"http://proxy-auth.example/tokens/{self.buildid}";,
+ "--upstream-proxy-url",
+ "http://username:password@proxy.example/";,
+ "--disable-proxy-after-pull",
+ "--git-repository",
+ "https://git.launchpad.dev/~example/+git/snap";,
+ "--git-path",
+ "master",
+ ]
+ try:
+ yield self.startBuild(args, expected_options)
+ finally:
+ self.buildmanager.stopProxy()
+
def getListenerURL(self, listener):
port = listener.getHost().port
return "http://localhost:%d/"; % port
@@ -746,17 +777,23 @@ class TestSnapBuildManagerIteration(TestCase):
# the code under test since the stock twisted.web.proxy doesn't support
# CONNECT.
- @mock.patch("lpbuildd.proxy.urlopen")
- def test_revokeProxyToken(self, urlopen_mock):
- self.buildmanager.revocation_endpoint = "http://revoke_endpoint";
- self.buildmanager.proxy_url = "http://username:password@proxy_url";
+ @responses.activate
+ def test_revokeProxyToken(self):
+ responses.add(
+ "DELETE", f"http://proxy-auth.example/tokens/{self.buildid}";
+ )
+ self.buildmanager.revocation_endpoint = (
+ f"http://proxy-auth.example/tokens/{self.buildid}";
+ )
+ self.buildmanager.proxy_url = "http://username:password@proxy.example";
self.buildmanager.revokeProxyToken()
- self.assertEqual(1, urlopen_mock.call_count)
- args, kwargs = urlopen_mock.call_args
- request = args[0]
+ self.assertEqual(1, len(responses.calls))
+ request = responses.calls[0].request
+ auth = base64.b64encode(b"username:password").decode()
+ self.assertEqual(f"Basic {auth}", request.headers["Authorization"])
self.assertEqual(
- {"Authorization": "Basic dXNlcm5hbWU6cGFzc3dvcmQ="},
- request.headers,
+ f"http://proxy-auth.example/tokens/{self.buildid}";, request.url
)
- self.assertEqual("http://revoke_endpoint";, request.get_full_url())
- self.assertEqual({"timeout": 15}, kwargs)
+ # XXX cjwatson 2023-02-07: Ideally we'd check the timeout as well,
+ # but the version of responses in Ubuntu 20.04 doesn't store it
+ # anywhere we can get at it.
diff --git a/lpbuildd/util.py b/lpbuildd/util.py
index 3359e17..eb35f01 100644
--- a/lpbuildd/util.py
+++ b/lpbuildd/util.py
@@ -5,6 +5,9 @@ import os
import subprocess
import sys
from shlex import quote
+from urllib.parse import urlparse
+
+import requests
def shell_escape(s):
@@ -53,3 +56,27 @@ def set_personality(args, arch, series=None):
setarch_cmd.append("--uname-2.6")
return setarch_cmd + args
+
+
+class RevokeProxyTokenError(Exception):
+ def __init__(self, username, exception):
+ super().__init__(self)
+ self.username = username
+ self.exception = exception
+
+ def __str__(self):
+ return f"Unable to revoke token for {self.username}: {self.exception}"
+
+
+def revoke_proxy_token(proxy_url, revocation_endpoint):
+ """Revoke builder proxy token.
+
+ :raises RevokeProxyTokenError: if attempting to revoke the token failed.
+ """
+ url = urlparse(proxy_url)
+ try:
+ requests.delete(
+ revocation_endpoint, auth=(url.username, url.password), timeout=15
+ )
+ except requests.RequestException as e:
+ raise RevokeProxyTokenError(url.username, e)
