launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #31028
[Merge] ~pelpsi/launchpad-buildd:pass-information-via-XML-RPC-API-to-the-builders into launchpad-buildd:master
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.
"""
Follow ups
