Ines Almeida has proposed merging ~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into launchpad-buildd:master.
Commit message: WIP: Update end-of-session code for snaps that use the fetch service Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~ines-almeida/launchpad-buildd/+git/launchpad-buildd/+merge/462875 This is WIP code with some TODOs and open questions to answer. The goal is to get early feedback and direction. -- Your team Launchpad code reviewers is requested to review the proposed merge of ~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service into launchpad-buildd:master.
diff --git a/lpbuildd/proxy.py b/lpbuildd/proxy.py index ea33dda..5b3d576 100644 --- a/lpbuildd/proxy.py +++ b/lpbuildd/proxy.py @@ -3,6 +3,7 @@ import base64 import io +import os from urllib.parse import urlparse from twisted.application import strports @@ -214,8 +215,21 @@ class BuilderProxyFactory(http.HTTPFactory): class BuildManagerProxyMixin: + + @property + def _use_fetch_service(self): + return ( + hasattr(self, 'use_fetch_service') + and getattr(self, "use_fetch_service") + ) + def startProxy(self): """Start the local builder proxy, if necessary.""" + + if self._use_fetch_service: + # TODO what to do if it is using the fetch service? + return [] + if not self.proxy_url: return [] proxy_port = self._builder._config.get("builder", "proxyport") @@ -232,6 +246,11 @@ class BuildManagerProxyMixin: def stopProxy(self): """Stop the local builder proxy, if necessary.""" + + if self._use_fetch_service: + # TODO what to do if it is using the fetch service? + return + if self.proxy_service is None: return self.proxy_service.disownServiceParent() @@ -243,6 +262,21 @@ class BuildManagerProxyMixin: return self._builder.log("Revoking proxy token...\n") try: - revoke_proxy_token(self.proxy_url, self.revocation_endpoint) + response = revoke_proxy_token( + self.proxy_url, + self.revocation_endpoint, + self._use_fetch_service, + ) except RevokeProxyTokenError as e: self._builder.log(f"{e}\n") + return + + # When using the fetch service, we want to retrieve and store the + # metadata we receive at the end of the session. + # This file will be uploaded when we run the `gatherResults` method + if self._use_fetch_service: + output_path = os.path.join( + "/build", self.name, "fetch-service-metadata.json" + ) + with self.backend.open(output_path, mode="wb") as f: + f.write(response.content) diff --git a/lpbuildd/snap.py b/lpbuildd/snap.py index 02390d4..765bdbf 100644 --- a/lpbuildd/snap.py +++ b/lpbuildd/snap.py @@ -38,6 +38,7 @@ class SnapBuildManager(BuildManagerProxyMixin, DebianBuildManager): self.branch = extra_args.get("branch") self.git_repository = extra_args.get("git_repository") self.git_path = extra_args.get("git_path") + self.use_fetch_service = extra_args.get("use_fetch_service") self.proxy_url = extra_args.get("proxy_url") self.revocation_endpoint = extra_args.get("revocation_endpoint") self.build_source_tarball = extra_args.get( @@ -100,6 +101,8 @@ class SnapBuildManager(BuildManagerProxyMixin, DebianBuildManager): if self.target_architectures: for arch in self.target_architectures: args.extend(["--target-arch", arch]) + if self.use_fetch_service: + args.append("--use_fetch_service") args.append(self.name) self.runTargetSubProcess("buildsnap", *args) @@ -141,7 +144,14 @@ class SnapBuildManager(BuildManagerProxyMixin, DebianBuildManager): # `.comp` files are the binary result of building snap # components, see spec SD149. if entry.endswith( - (".snap", ".manifest", ".debug", ".dpkg.yaml", ".comp") + ( + ".snap", + ".manifest", + ".debug", + ".dpkg.yaml", + ".comp", + ".json", + ) ): self.addWaitingFileFromBackend(path) if self.build_source_tarball: diff --git a/lpbuildd/target/build_snap.py b/lpbuildd/target/build_snap.py index d9052f2..ef74958 100644 --- a/lpbuildd/target/build_snap.py +++ b/lpbuildd/target/build_snap.py @@ -102,6 +102,12 @@ class BuildSnap( action="store_true", help="disable proxy access after the pull phase has finished", ) + parser.add_argument( + "--use_fetch_service", + default=False, + action="store_true", + help="use the fetch service instead of the old builder proxy", + ) parser.add_argument("name", help="name of snap to build") def install_svn_servers(self): @@ -227,9 +233,18 @@ class BuildSnap( self.args.disable_proxy_after_pull and self.args.upstream_proxy_url and self.args.revocation_endpoint + and not self.args.use_fetch_service ): logger.info("Revoking proxy token...") try: + # TODO Should we revoke the proxy token even when we are + # using the fetch service here? If there were 2 revocation + # endpoints (one for revoking the token, another to end the + # session and get the metadata), then we would revoke the token + # here, and only end the session later. Given currently, it's + # only one endpoint, then we would closing the session and + # getting the metadata here, which doesn't seem right. + # Note: regardless of the revoke_proxy_token( self.args.upstream_proxy_url, self.args.revocation_endpoint ) diff --git a/lpbuildd/tests/test_snap.py b/lpbuildd/tests/test_snap.py index 9bef60a..bd5fe12 100644 --- a/lpbuildd/tests/test_snap.py +++ b/lpbuildd/tests/test_snap.py @@ -754,6 +754,13 @@ class TestSnapBuildManagerIteration(TestCase): yield self.startBuild(args, expected_options) @defer.inlineCallbacks + def test_iterate_use_fetch_service(self): + # The build manager can be told to use the fetch service as its proxy. + args = {"use_fetch_service": True} + expected_options = ["--use_fetch_service"] + yield self.startBuild(args, expected_options) + + @defer.inlineCallbacks def test_iterate_disable_proxy_after_pull(self): self.builder._config.set("builder", "proxyport", "8222") args = { @@ -874,3 +881,33 @@ class TestSnapBuildManagerIteration(TestCase): # 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. + + @responses.activate + def test_revokeProxyToken_fetch_service(self): + session_id = "123" + token = "test_token" + + responses.add( + "DELETE", f"http://fetch-service.example/{session_id}" + ) + + self.buildmanager.backend_name = "fake" + self.buildmanager.initiate( + {}, "chroot.tar.gz", {"name": "test-snap", "series": "xenial"} + ) + self.buildmanager.use_fetch_service = True + self.buildmanager.revocation_endpoint = ( + f"http://fetch-service.example/{session_id}" + ) + self.buildmanager.proxy_url = f"http://session_id:{token}@proxy.example" + + self.buildmanager.revokeProxyToken() + + self.assertEqual(1, len(responses.calls)) + request = responses.calls[0].request + self.assertEqual(f"Basic {token}", request.headers["Authorization"]) + self.assertEqual( + f"http://fetch-service.example/{session_id}", request.url + ) + + #TODO add tests about the writting of the metadata file for fetch service diff --git a/lpbuildd/tests/test_util.py b/lpbuildd/tests/test_util.py index 005d42b..ea3d8e4 100644 --- a/lpbuildd/tests/test_util.py +++ b/lpbuildd/tests/test_util.py @@ -1,9 +1,12 @@ # Copyright 2017 Canonical Ltd. This software is licensed under the # GNU Affero General Public License version 3 (see the file LICENSE). +import base64 +import responses + from testtools import TestCase -from lpbuildd.util import get_arch_bits, set_personality, shell_escape +from lpbuildd.util import get_arch_bits, revoke_proxy_token, set_personality, shell_escape class TestShellEscape(TestCase): @@ -59,3 +62,44 @@ class TestSetPersonality(TestCase): ["linux64", "sbuild"], set_personality(["sbuild"], "amd64", series="trusty"), ) + + +class TestRevokeToken(TestCase): + + @responses.activate + def test_revoke_proxy_token(self): + """Proxy token revocation uses the right authentication""" + + proxy_url = "https://username:password@host:port" + revocation_endpoint = "https://builder.api.endpoint.token" + token = base64.b64encode(b"username:password").decode() + + responses.add(responses.DELETE, revocation_endpoint) + + response = revoke_proxy_token(proxy_url, revocation_endpoint) + self.assertEqual( + f"Basic {token}", response.request.headers["Authorization"] + ) + + @responses.activate + def test_revoke_fetch_service_token(self): + """Proxy token revocation for the fetch service, uses the right + authentication and returns metadata """ + + proxy_url = f"https://session_id:test-token@host:port" + revocation_endpoint = "https://builder.api.endpoint/session_id" + response_metadata = {"metadata": "test"} + + responses.add( + responses.DELETE, revocation_endpoint, json=response_metadata + ) + + response = revoke_proxy_token( + proxy_url, + revocation_endpoint, + use_fetch_service=True, + ) + self.assertEqual( + f"Basic test-token", response.request.headers["Authorization"] + ) + self.assertEqual(response_metadata, response.json()) diff --git a/lpbuildd/util.py b/lpbuildd/util.py index eb35f01..e3188b1 100644 --- a/lpbuildd/util.py +++ b/lpbuildd/util.py @@ -1,6 +1,7 @@ # Copyright 2015-2017 Canonical Ltd. This software is licensed under the # GNU Affero General Public License version 3 (see the file LICENSE). +import base64 import os import subprocess import sys @@ -68,15 +69,37 @@ class RevokeProxyTokenError(Exception): return f"Unable to revoke token for {self.username}: {self.exception}" -def revoke_proxy_token(proxy_url, revocation_endpoint): +def revoke_proxy_token(proxy_url, revocation_endpoint, use_fetch_service=False): """Revoke builder proxy token. + The proxy_url for the current Builder Proxy: + http://{username}:{password}@{host}:{port} + + We use the username-password combo for authentication. + + The proxy_url for the fetch service: + http://{session_id}:{token}@{host}:{port} + + We use the token for authentication. + :raises RevokeProxyTokenError: if attempting to revoke the token failed. """ url = urlparse(proxy_url) + + # For the fetch service URL case, the {session_id}:{token} will be parsed + # as `username` and `password` respectively + if use_fetch_service: + token = url.password + else: + token = base64.b64encode( + f"{url.username}:{url.password}".encode() + ).decode() + try: - requests.delete( - revocation_endpoint, auth=(url.username, url.password), timeout=15 + return requests.delete( + revocation_endpoint, + headers={'Authorization': f'Basic {token}'}, + timeout=15, ) except requests.RequestException as e: raise RevokeProxyTokenError(url.username, e)
_______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp