Simone Pelosi has proposed merging ~pelpsi/launchpad-buildd:pass-information-via-XML-RPC-API-to-the-builders into launchpad-buildd:master with ~ines-almeida/launchpad-buildd:update-close-session-for-fetch-service as a prerequisite.
Commit message: Pass information via XML-RPC API to the builders Pass fetch-service proxy url containing token and session information. Set env variables to use proxy globally and pass certificate. Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~pelpsi/launchpad-buildd/+git/launchpad-buildd/+merge/464293 -- Your team Launchpad code reviewers is requested to review the proposed merge of ~pelpsi/launchpad-buildd:pass-information-via-XML-RPC-API-to-the-builders into launchpad-buildd:master.
diff --git a/lpbuildd/proxy.py b/lpbuildd/proxy.py index 1a05ff3..4645091 100644 --- a/lpbuildd/proxy.py +++ b/lpbuildd/proxy.py @@ -224,18 +224,25 @@ class BuildManagerProxyMixin: """Start the local builder proxy, if necessary. This starts an internal proxy that stands before the Builder - Proxy/Fetch Service, so build systems, which do not comply - with standard `http(s)_proxy` environment variables, would - still work with the builder proxy. + Proxy/Fetch Service. It is important for certain build types. """ if not self.proxy_url: return [] - proxy_port = self._builder._config.get("builder", "proxyport") proxy_factory = BuilderProxyFactory(self, self.proxy_url, timeout=60) + proxy_port = self._builder._config.get("builder", "proxyport") + + if self._use_fetch_service: + proxy_url = self.proxy_url.split("@") + proxy_port = proxy_url.split(":")[-1] + self.proxy_service = strports.service( "tcp:%s" % proxy_port, proxy_factory ) self.proxy_service.setServiceParent(self._builder.service) + + if self._use_fetch_service: + return ["--proxy-url", f"{self.proxy_url}"] + if hasattr(self.backend, "ipv4_network"): proxy_host = self.backend.ipv4_network.ip else: diff --git a/lpbuildd/target/build_snap.py b/lpbuildd/target/build_snap.py index 82470d5..d649fad 100644 --- a/lpbuildd/target/build_snap.py +++ b/lpbuildd/target/build_snap.py @@ -106,7 +106,7 @@ class BuildSnap( "--use_fetch_service", default=False, action="store_true", - help="use the fetch service instead of the builder proxy", + help="use the fetch service instead of the old builder proxy", ) parser.add_argument("name", help="name of snap to build") diff --git a/lpbuildd/target/tests/test_build_snap.py b/lpbuildd/target/tests/test_build_snap.py index 8719e57..30f0c8f 100644 --- a/lpbuildd/target/tests/test_build_snap.py +++ b/lpbuildd/target/tests/test_build_snap.py @@ -194,6 +194,55 @@ class TestBuildSnap(TestCase): build_snap.backend.backend_fs["/root/.subversion/servers"], ) + def test_install_fetch_service(self): + args = [ + "buildsnap", + "--backend=fake", + "--series=xenial", + "--arch=amd64", + "1", + "--git-repository", + "lp:foo", + "--proxy-url", + "http://session:token@fetch-service.proxy:9988", + "test-snap", + "--use_fetch_service" + ] + build_snap = parse_args(args=args).operation + build_snap.bin = "/builderbin" + self.useFixture(FakeFilesystem()).add("/builderbin") + os.mkdir("/builderbin") + with open("/builderbin/lpbuildd-git-proxy", "w") as proxy_script: + proxy_script.write("proxy script\n") + os.fchmod(proxy_script.fileno(), 0o755) + build_snap.install() + self.assertThat( + build_snap.backend.run.calls, + MatchesListwise( + [ + RanAptGet( + "install", "python3", "socat", "git", "snapcraft" + ), + RanCommand(["mkdir", "-p", "/root/.subversion"]), + ] + ), + ) + self.assertEqual( + (b"proxy script\n", stat.S_IFREG | 0o755), + build_snap.backend.backend_fs["/usr/local/bin/lpbuildd-git-proxy"], + ) + self.assertEqual( + ( + b"[global]\n" + b"http-proxy-host = fetch-service.proxy\n" + b"http-proxy-port = 9988\n" + b"http-proxy-username = session\n" + b"http-proxy-password = token\n", + stat.S_IFREG | 0o644, + ), + build_snap.backend.backend_fs["/root/.subversion/servers"], + ) + def test_install_channels(self): args = [ "buildsnap", @@ -479,6 +528,68 @@ class TestBuildSnap(TestCase): with open(status_path) as status: self.assertEqual({"revision_id": "0" * 40}, json.load(status)) + def test_repo_fetch_service(self): + args = [ + "buildsnap", + "--backend=fake", + "--series=xenial", + "--arch=amd64", + "1", + "--git-repository", + "lp:foo", + "--proxy-url", + "http://session:token@fetch-service.proxy:9988", + "test-snap", + "--use_fetch_service" + ] + build_snap = parse_args(args=args).operation + build_snap.backend.build_path = self.useFixture(TempDir()).path + build_snap.backend.run = FakeRevisionID("0" * 40) + build_snap.repo() + env = { + "http_proxy": "http://session:token@fetch-service.proxy:9988", + "https_proxy": "http://session:token@fetch-service.proxy:9988", + "GIT_PROXY_COMMAND": "/usr/local/bin/lpbuildd-git-proxy", + "SNAPPY_STORE_NO_CDN": "1", + } + self.assertThat( + build_snap.backend.run.calls, + MatchesListwise( + [ + RanBuildCommand( + ["git", "clone", "-n", "lp:foo", "test-snap"], + cwd="/build", + **env, + ), + RanBuildCommand( + ["git", "checkout", "-q", "HEAD"], + cwd="/build/test-snap", + **env, + ), + RanBuildCommand( + [ + "git", + "submodule", + "update", + "--init", + "--recursive", + ], + cwd="/build/test-snap", + **env, + ), + RanBuildCommand( + ["git", "rev-parse", "HEAD^{}"], + cwd="/build/test-snap", + get_output=True, + universal_newlines=True, + ), + ] + ), + ) + status_path = os.path.join(build_snap.backend.build_path, "status") + with open(status_path) as status: + self.assertEqual({"revision_id": "0" * 40}, json.load(status)) + def test_pull(self): args = [ "buildsnap", @@ -551,6 +662,48 @@ class TestBuildSnap(TestCase): ), ) + def test_pull_fetch_service(self): + args = [ + "buildsnap", + "--backend=fake", + "--series=xenial", + "--arch=amd64", + "1", + "--build-url", + "https://launchpad.example/build", + "--branch", + "lp:foo", + "--proxy-url", + "http://session:token@fetch-service.proxy:9988", + "test-snap", + "--use_fetch_service" + ] + 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://session:token@fetch-service.proxy:9988", + "https_proxy": "http://session:token@fetch-service.proxy:9988", + "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 + ), + ] + ), + ) + @responses.activate def test_pull_disable_proxy_after_pull(self): self.useFixture(FakeLogger()) @@ -767,6 +920,52 @@ class TestBuildSnap(TestCase): ), ) + def test_build_fetch_service(self): + args = [ + "buildsnap", + "--backend=fake", + "--series=xenial", + "--arch=amd64", + "1", + "--build-url", + "https://launchpad.example/build", + "--branch", + "lp:foo", + "--proxy-url", + "http://session:token@fetch-service.proxy:9988", + "test-snap", + ] + build_snap = parse_args(args=args).operation + build_snap.backend.run = FakeSnapcraft( + build_snap.backend, "test-snap_1.snap" + ) + build_snap.build() + env = { + "SNAPCRAFT_BUILD_INFO": "1", + "SNAPCRAFT_IMAGE_INFO": ( + '{"build_url": "https://launchpad.example/build"}' + ), + "SNAPCRAFT_BUILD_ENVIRONMENT": "host", + "http_proxy": "http://session:token@fetch-service.proxy:9988", + "https_proxy": "http://session:token@fetch-service.proxy:9988", + "GIT_PROXY_COMMAND": "/usr/local/bin/lpbuildd-git-proxy", + "SNAPPY_STORE_NO_CDN": "1", + } + self.assertThat( + build_snap.backend.run.calls, + MatchesListwise( + [ + RanBuildCommand( + ["snapcraft"], cwd="/build/test-snap", **env + ), + RanBuildCommand( + ["sha512sum", "test-snap_1.snap"], + cwd="/build/test-snap", + ), + ] + ), + ) + def test_build_private(self): args = [ "buildsnap", diff --git a/lpbuildd/tests/test_snap.py b/lpbuildd/tests/test_snap.py index bb0af3a..94b6706 100644 --- a/lpbuildd/tests/test_snap.py +++ b/lpbuildd/tests/test_snap.py @@ -888,23 +888,19 @@ class TestSnapBuildManagerIteration(TestCase): session_id = "123" responses.add( - "DELETE", - f"http://control.fetch-service.example/{session_id}/token", + "DELETE", f"http://fetch-service.example/{session_id}/token" ) self.buildmanager.use_fetch_service = True self.buildmanager.revocation_endpoint = ( - f"http://control.fetch-service.example/{session_id}/token" - ) - self.buildmanager.proxy_url = ( - "http://session_id:token@proxy.fetch-service.example" + f"http://fetch-service.example/{session_id}/token" ) + self.buildmanager.proxy_url = "http://session_id:test@proxy.example" self.buildmanager.revokeProxyToken() self.assertEqual(1, len(responses.calls)) request = responses.calls[0].request self.assertEqual( - f"http://control.fetch-service.example/{session_id}/token", - request.url, + f"http://fetch-service.example/{session_id}/token", request.url ) diff --git a/lpbuildd/tests/test_util.py b/lpbuildd/tests/test_util.py index ab65f5f..15c861c 100644 --- a/lpbuildd/tests/test_util.py +++ b/lpbuildd/tests/test_util.py @@ -74,8 +74,8 @@ class TestRevokeToken(TestCase): def test_revoke_proxy_token(self): """Proxy token revocation uses the right authentication""" - proxy_url = "http://username:password@proxy.example" - revocation_endpoint = "http://proxy-auth.example/tokens/build_id" + proxy_url = "http://username:password@host:port" + revocation_endpoint = "http://builder.api.endpoint.token" token = base64.b64encode(b"username:password").decode() responses.add(responses.DELETE, revocation_endpoint) @@ -83,19 +83,16 @@ class TestRevokeToken(TestCase): revoke_proxy_token(proxy_url, revocation_endpoint) self.assertEqual(1, len(responses.calls)) request = responses.calls[0].request - self.assertEqual( - "http://proxy-auth.example/tokens/build_id", request.url - ) + self.assertEqual("http://builder.api.endpoint.token/", request.url) self.assertEqual(f"Basic {token}", request.headers["Authorization"]) @responses.activate def test_revoke_fetch_service_token(self): """Proxy token revocation for the fetch service""" - proxy_url = "http://session_id:token@proxy.fetch-service.example" - revocation_endpoint = ( - "http://control.fetch-service.example/session_id/token" - ) + token = "test-token" + proxy_url = f"http://session_id:{token}@host:port" + revocation_endpoint = "http://fetch-service.example/session_id/token" responses.add(responses.DELETE, revocation_endpoint) @@ -108,6 +105,5 @@ class TestRevokeToken(TestCase): self.assertEqual(1, len(responses.calls)) request = responses.calls[0].request self.assertEqual( - "http://control.fetch-service.example/session_id/token", - request.url, + "http://fetch-service.example/session_id/token", request.url ) diff --git a/lpbuildd/util.py b/lpbuildd/util.py index 664f92b..0171e1f 100644 --- a/lpbuildd/util.py +++ b/lpbuildd/util.py @@ -80,12 +80,12 @@ def revoke_proxy_token( We use the username-password combo from the proxy_url for authentication to revoke its token. - If using the fetch service: + If using thefetch service: The call to revoke a token does not require authentication. - XXX ines-almeida 2024-04-15: this might change depending on - conversations about fetch service authentication. We might decide to - instead use the token itself as the authentication. + TODO this might change depending on conversations about fetch service + authentication. We might decide to instead use the token itself as the + authentication. :raises RevokeProxyTokenError: if attempting to revoke the token failed. """
_______________________________________________ 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