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 <[email protected]> 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:[email protected]: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:[email protected]"; 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:[email protected]/";, + "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:[email protected]/";, + "--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:[email protected]"; 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)
_______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
