launchpad-reviewers team mailing list archive

[Simone Pelosi <mp+464337@xxxxxxxxxxxxxxxxxx>]

Simone Pelosi has proposed merging ~pelpsi/launchpad:pass-information-via-XML-RPC-API-to-the-builders into launchpad:master.

Commit message:
Passing certificate to the builders

Builders need certificate to configure correctly env to use
fetch service.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pelpsi/launchpad/+git/launchpad/+merge/464337
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pelpsi/launchpad:pass-information-via-XML-RPC-API-to-the-builders into launchpad:master.

diff --git a/charm/launchpad-buildd-manager/config.yaml b/charm/launchpad-buildd-manager/config.yaml
index caac316..630ce00 100644
--- a/charm/launchpad-buildd-manager/config.yaml
+++ b/charm/launchpad-buildd-manager/config.yaml
@@ -71,6 +71,10 @@ options:
       Fetch service host, it could be either a single instance 
       or a load balancer in front.
     default: ""
+  fetch_service_mitm_certificate:
+    type: string
+    description: Fetch service certificate.
+    default: ""
   fetch_service_port:
     type: int
     description: Fetch service port.
diff --git a/lib/lp/buildmaster/builderproxy.py b/lib/lp/buildmaster/builderproxy.py
index 3375042..5f3cf5b 100644
--- a/lib/lp/buildmaster/builderproxy.py
+++ b/lib/lp/buildmaster/builderproxy.py
@@ -82,6 +82,13 @@ class BuilderProxyMixin:
                 session_id=session["id"],
             )
 
+            # Append the fetch-service certificate to BuildArgs secrets.
+            if "secrets" not in args:
+                args["secrets"] = {}
+            args["secrets"]["fetch_service_mitm_certificate"] = (
+                _get_value_from_config("fetch_service_mitm_certificate")
+            )
+
     @defer.inlineCallbacks
     def _requestProxyToken(self):
         admin_username = _get_value_from_config(
diff --git a/lib/lp/buildmaster/tests/fetchservice.py b/lib/lp/buildmaster/tests/fetchservice.py
index 3fd879c..51f73e3 100644
--- a/lib/lp/buildmaster/tests/fetchservice.py
+++ b/lib/lp/buildmaster/tests/fetchservice.py
@@ -75,19 +75,18 @@ class InProcessFetchServiceAuthAPIFixture(fixtures.Fixture):
         self.addCleanup(site.stopFactory)
         port = yield endpoint.listen(site)
         self.addCleanup(port.stopListening)
-        config.push(
-            "in-process-fetch-service-api-fixture",
-            dedent(
-                """
-                [builddmaster]
-                fetch_service_control_admin_secret: admin-secret
-                fetch_service_control_admin_username: admin-launchpad.test
-                fetch_service_control_endpoint: http://{host}:{port}/session
-                fetch_service_host: {host}
-                fetch_service_port: {port}
-                """
-            ).format(host=port.getHost().host, port=port.getHost().port),
-        )
+        configs = dedent(
+            """
+            [builddmaster]
+            fetch_service_control_admin_secret: admin-secret
+            fetch_service_control_admin_username: admin-launchpad.test
+            fetch_service_control_endpoint: http://{host}:{port}/session
+            fetch_service_host: {host}
+            fetch_service_port: {port}
+            fetch_service_mitm_certificate: fake-cert
+            """
+        ).format(host=port.getHost().host, port=port.getHost().port)
+        config.push("in-process-fetch-service-api-fixture", configs)
         self.addCleanup(config.pop, "in-process-fetch-service-api-fixture")
 
 
diff --git a/lib/lp/services/config/schema-lazr.conf b/lib/lp/services/config/schema-lazr.conf
index 94ca842..07f04c6 100644
--- a/lib/lp/services/config/schema-lazr.conf
+++ b/lib/lp/services/config/schema-lazr.conf
@@ -178,6 +178,9 @@ fetch_service_control_admin_username: none
 # Endpoint for fetch service authentication service
 fetch_service_control_endpoint: none
 
+# Fetch service certificate
+fetch_service_mitm_certificate: none
+
 # Fetch service host, it could be either a single instance 
 # or a load balancer in front
 fetch_service_host: none
@@ -1883,6 +1886,9 @@ fetch_service_control_admin_username: none
 # Endpoint for fetch service control service.
 fetch_service_control_endpoint: none
 
+# Fetch service certificate
+fetch_service_mitm_certificate: none
+
 # Fetch service host, it could be either a single instance 
 # or a load balancer in front.
 fetch_service_host: none
diff --git a/lib/lp/snappy/tests/test_snapbuildbehaviour.py b/lib/lp/snappy/tests/test_snapbuildbehaviour.py
index 2ccc18d..d20bf33 100644
--- a/lib/lp/snappy/tests/test_snapbuildbehaviour.py
+++ b/lib/lp/snappy/tests/test_snapbuildbehaviour.py
@@ -303,6 +303,28 @@ class TestAsyncSnapBuildBehaviourFetchService(
         self.assertNotIn("revocation_endpoint", args)
 
     @defer.inlineCallbacks
+    def test_requestFetchServiceSession_no_certificate(self):
+        """Create a snap build request with an incomplete fetch service
+        configuration.
+
+        If `fetch_service_mitm_certificate` is not provided
+        the function raises a `CannotBuild` error.
+        """
+        self.pushConfig("builddmaster", fetch_service_mitm_certificate=None)
+        self.useFixture(
+            FeatureFixture({SNAP_USE_FETCH_SERVICE_FEATURE_FLAG: "on"})
+        )
+
+        snap = self.factory.makeSnap(use_fetch_service=True)
+        request = self.factory.makeSnapBuildRequest(snap=snap)
+        job = self.makeJob(snap=snap, build_request=request)
+        expected_exception_msg = (
+            "fetch_service_mitm_certificate is not configured."
+        )
+        with ExpectedException(CannotBuild, expected_exception_msg):
+            yield job.extraBuildArgs()
+
+    @defer.inlineCallbacks
     def test_requestFetchServiceSession_no_secret(self):
         """Create a snap build request with an incomplete fetch service
         configuration.
@@ -379,6 +401,11 @@ class TestAsyncSnapBuildBehaviourFetchService(
         )
         self.assertIn("proxy_url", args)
         self.assertIn("revocation_endpoint", args)
+        self.assertIn("secrets", args)
+        self.assertIn("fetch_service_mitm_certificate", args["secrets"])
+        self.assertIn(
+            "fake-cert", args["secrets"]["fetch_service_mitm_certificate"]
+        )
 
     @defer.inlineCallbacks
     def test_requestFetchServiceSession_flag_off(self):