launchpad-reviewers team mailing list archive

[Simone Pelosi <mp+473393@xxxxxxxxxxxxxxxxxx>]

Simone Pelosi has proposed merging ~pelpsi/launchpad:fetch-service-configuration-for-rock-builds into launchpad:master.

Commit message:
Fetch service configuration for Rock builds
    
Integrate use_fetch_service flag to activate or deactivate fetch service
for a given rock recipe.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pelpsi/launchpad/+git/launchpad/+merge/473393
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pelpsi/launchpad:fetch-service-configuration-for-rock-builds into launchpad:master.

diff --git a/lib/lp/rocks/interfaces/rockrecipe.py b/lib/lp/rocks/interfaces/rockrecipe.py
index 49fe8cd..8938e74 100644
--- a/lib/lp/rocks/interfaces/rockrecipe.py
+++ b/lib/lp/rocks/interfaces/rockrecipe.py
@@ -497,6 +497,18 @@ class IRockRecipeAdminAttributes(Interface):
         description=_("Only build this rock recipe on virtual builders."),
     )
 
+    use_fetch_service = exported(
+        Bool(
+            title=_("Use fetch service"),
+            required=True,
+            readonly=False,
+            description=_(
+                "If set, Rock builds will use the fetch-service instead "
+                "of the builder-proxy to access external resources."
+            ),
+        )
+    )
+
 
 class IRockRecipe(
     IRockRecipeView,
@@ -528,6 +540,7 @@ class IRockRecipeSet(Interface):
         store_secrets=None,
         store_channels=None,
         date_created=None,
+        use_fetch_service=False,
     ):
         """Create an `IRockRecipe`."""
 
diff --git a/lib/lp/rocks/model/rockrecipe.py b/lib/lp/rocks/model/rockrecipe.py
index cb7d748..a597ad5 100644
--- a/lib/lp/rocks/model/rockrecipe.py
+++ b/lib/lp/rocks/model/rockrecipe.py
@@ -241,6 +241,8 @@ class RockRecipe(StormBase):
 
     _store_channels = JSON("store_channels", allow_none=True)
 
+    _use_fetch_service = Bool(name="use_fetch_service", allow_none=False)
+
     def __init__(
         self,
         registrant,
@@ -259,6 +261,7 @@ class RockRecipe(StormBase):
         store_secrets=None,
         store_channels=None,
         date_created=DEFAULT,
+        use_fetch_service=False,
     ):
         """Construct a `RockRecipe`."""
         if not getFeatureFlag(ROCK_RECIPE_ALLOW_CREATE):
@@ -284,6 +287,7 @@ class RockRecipe(StormBase):
         self.store_name = store_name
         self.store_secrets = store_secrets
         self.store_channels = store_channels
+        self.use_fetch_service = use_fetch_service
 
     def __repr__(self):
         return "<RockRecipe ~%s/%s/+rock/%s>" % (
@@ -325,6 +329,14 @@ class RockRecipe(StormBase):
         """See `IRockRecipe`."""
         self._store_channels = value or None
 
+    @property
+    def use_fetch_service(self):
+        return self._use_fetch_service
+
+    @use_fetch_service.setter
+    def use_fetch_service(self, value):
+        self._use_fetch_service = value
+
     def getAllowedInformationTypes(self, user):
         """See `IRockRecipe`."""
         # XXX jugmac00 2024-08-29: Only allow free information types until
@@ -473,6 +485,7 @@ class RockRecipeSet:
         store_secrets=None,
         store_channels=None,
         date_created=DEFAULT,
+        use_fetch_service=False,
     ):
         """See `IRockRecipeSet`."""
         if not registrant.inTeam(owner):
@@ -517,6 +530,7 @@ class RockRecipeSet:
             store_secrets=store_secrets,
             store_channels=store_channels,
             date_created=date_created,
+            use_fetch_service=use_fetch_service,
         )
         store.add(recipe)
 
diff --git a/lib/lp/rocks/model/rockrecipebuildbehaviour.py b/lib/lp/rocks/model/rockrecipebuildbehaviour.py
index f1f9d04..65301ba 100644
--- a/lib/lp/rocks/model/rockrecipebuildbehaviour.py
+++ b/lib/lp/rocks/model/rockrecipebuildbehaviour.py
@@ -79,7 +79,9 @@ class RockRecipeBuildBehaviour(BuilderProxyMixin, BuildFarmJobBehaviourBase):
         """
         build = self.build
         args: BuildArgs = yield super().extraBuildArgs(logger=logger)
-        yield self.startProxySession(args)
+        yield self.startProxySession(
+            args, use_fetch_service=build.recipe.use_fetch_service
+        )
         args["name"] = build.recipe.store_name or build.recipe.name
         channels = build.channels or {}
         # We have to remove the security proxy that Zope applies to this
diff --git a/lib/lp/rocks/tests/test_rockrecipe.py b/lib/lp/rocks/tests/test_rockrecipe.py
index 1f9c807..e9a95bf 100644
--- a/lib/lp/rocks/tests/test_rockrecipe.py
+++ b/lib/lp/rocks/tests/test_rockrecipe.py
@@ -368,6 +368,18 @@ class TestRockRecipe(TestCaseWithFactory):
             build = recipe.requestBuild(build_request, das)
             self.assertEqual(build_virt, build.virtualized)
 
+    def test_requestBuild_fetch_service(self):
+        # Activate fetch service for a rock recipe.
+        recipe = self.factory.makeRockRecipe(use_fetch_service=True)
+        self.assertEqual(True, recipe.use_fetch_service)
+        distro_series = self.factory.makeDistroSeries()
+        das = self.makeBuildableDistroArchSeries(
+            distroseries=distro_series,
+        )
+        build_request = self.factory.makeRockRecipeBuildRequest(recipe=recipe)
+        build = recipe.requestBuild(build_request, das)
+        self.assertEqual(True, build.recipe.use_fetch_service)
+
     def test_requestBuild_nonvirtualized(self):
         # A non-virtualized processor can build a rock recipe iff the
         # recipe has require_virtualized set to False.
@@ -443,6 +455,7 @@ class TestRockRecipeSet(TestCaseWithFactory):
         self.assertIsNone(recipe.store_name)
         self.assertIsNone(recipe.store_secrets)
         self.assertEqual([], recipe.store_channels)
+        self.assertFalse(recipe.use_fetch_service)
 
     def test_creation_no_source(self):
         # Attempting to create a rock recipe without a Git repository
diff --git a/lib/lp/rocks/tests/test_rockrecipebuildbehaviour.py b/lib/lp/rocks/tests/test_rockrecipebuildbehaviour.py
index 88cea3a..3dcd1db 100644
--- a/lib/lp/rocks/tests/test_rockrecipebuildbehaviour.py
+++ b/lib/lp/rocks/tests/test_rockrecipebuildbehaviour.py
@@ -3,12 +3,17 @@
 
 """Test rock recipe build behaviour."""
 
+import base64
 import os.path
+import time
+import uuid
 
+import fixtures
 import transaction
 from pymacaroons import Macaroon
 from testtools import ExpectedException
 from testtools.matchers import (
+    ContainsDict,
     Equals,
     Is,
     IsInstance,
@@ -28,12 +33,20 @@ from lp.archivepublisher.interfaces.archivegpgsigningkey import (
     IArchiveGPGSigningKey,
 )
 from lp.buildmaster.enums import BuildBaseImageType, BuildStatus
+from lp.buildmaster.interactor import shut_down_default_process_pool
 from lp.buildmaster.interfaces.builder import CannotBuild
 from lp.buildmaster.interfaces.buildfarmjobbehaviour import (
     IBuildFarmJobBehaviour,
 )
 from lp.buildmaster.interfaces.processor import IProcessorSet
-from lp.buildmaster.tests.mock_workers import MockBuilder, OkWorker
+from lp.buildmaster.tests.fetchservice import (
+    InProcessFetchServiceAuthAPIFixture,
+)
+from lp.buildmaster.tests.mock_workers import (
+    MockBuilder,
+    OkWorker,
+    WorkerTestHelpers,
+)
 from lp.buildmaster.tests.test_buildfarmjobbehaviour import (
     TestGetUploadMethodsMixin,
     TestHandleStatusMixin,
@@ -425,6 +438,151 @@ class TestAsyncRockRecipeBuildBehaviour(
         )
 
 
+class TestAsyncSnapBuildBehaviourFetchService(
+    StatsMixin, TestRockRecipeBuildBehaviourBase
+):
+    run_tests_with = AsynchronousDeferredRunTestForBrokenTwisted.make_factory(
+        timeout=30
+    )
+
+    @defer.inlineCallbacks
+    def setUp(self):
+        super().setUp()
+        self.session = {
+            "id": "1",
+            "token": uuid.uuid4().hex,
+        }
+        self.fetch_service_url = (
+            "http://{session_id}:{token}@{host}:{port}".format(
+                session_id=self.session["id"],
+                token=self.session["token"],
+                host=config.builddmaster.fetch_service_host,
+                port=config.builddmaster.fetch_service_port,
+            )
+        )
+        self.fetch_service_api = self.useFixture(
+            InProcessFetchServiceAuthAPIFixture()
+        )
+        yield self.fetch_service_api.start()
+        self.now = time.time()
+        self.useFixture(fixtures.MockPatch("time.time", return_value=self.now))
+        self.addCleanup(shut_down_default_process_pool)
+        self.setUpStats()
+
+    def makeJob(self, **kwargs):
+        # We need a builder worker in these tests, in order that requesting
+        # a proxy token can piggyback on its reactor and pool.
+        job = super().makeJob(**kwargs)
+        builder = MockBuilder()
+        builder.processor = job.build.processor
+        worker = self.useFixture(WorkerTestHelpers()).getClientWorker()
+        job.setBuilder(builder, worker)
+        self.addCleanup(worker.pool.closeCachedConnections)
+        return job
+
+    @defer.inlineCallbacks
+    def test_requestFetchServiceSession_unconfigured(self):
+        """Create a rock recipe build request with an incomplete fetch service
+        configuration.
+
+        If `fetch_service_host` is not provided the function will return
+        without populating `proxy_url` and `revocation_endpoint`.
+        """
+        self.pushConfig("builddmaster", fetch_service_host=None)
+        job = self.makeJob(use_fetch_service=True)
+        with dbuser(config.builddmaster.dbuser):
+            args = yield job.extraBuildArgs()
+        self.assertEqual([], self.fetch_service_api.sessions.requests)
+        self.assertNotIn("proxy_url", args)
+        self.assertNotIn("revocation_endpoint", args)
+
+    @defer.inlineCallbacks
+    def test_requestFetchServiceSession_no_certificate(self):
+        """Create a rock recipe 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)
+        job = self.makeJob(use_fetch_service=True)
+        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 rock recipe build request with an incomplete fetch service
+        configuration.
+
+        If `fetch_service_control_admin_secret` is not provided
+        the function raises a `CannotBuild` error.
+        """
+        self.pushConfig(
+            "builddmaster", fetch_service_control_admin_secret=None
+        )
+        job = self.makeJob(use_fetch_service=True)
+        expected_exception_msg = (
+            "fetch_service_control_admin_secret is not configured."
+        )
+        with ExpectedException(CannotBuild, expected_exception_msg):
+            yield job.extraBuildArgs()
+
+    @defer.inlineCallbacks
+    def test_requestFetchServiceSession(self):
+        """Create a rock recipe build request with a successful fetch service
+        configuration.
+
+        `proxy_url` and `revocation_endpoint` are correctly populated.
+        """
+        job = self.makeJob(use_fetch_service=True)
+        args = yield job.extraBuildArgs()
+        request_matcher = MatchesDict(
+            {
+                "method": Equals(b"POST"),
+                "uri": Equals(b"/session"),
+                "headers": ContainsDict(
+                    {
+                        b"Authorization": MatchesListwise(
+                            [
+                                Equals(
+                                    b"Basic "
+                                    + base64.b64encode(
+                                        b"admin-launchpad.test:admin-secret"
+                                    )
+                                )
+                            ]
+                        ),
+                        b"Content-Type": MatchesListwise(
+                            [
+                                Equals(b"application/json"),
+                            ]
+                        ),
+                    }
+                ),
+                "json": MatchesDict(
+                    {
+                        "policy": Equals("permissive"),
+                    }
+                ),
+            }
+        )
+        self.assertThat(
+            self.fetch_service_api.sessions.requests,
+            MatchesListwise([request_matcher]),
+        )
+        self.assertIn("proxy_url", args)
+        self.assertIn("revocation_endpoint", args)
+        self.assertTrue(args["use_fetch_service"])
+        self.assertIn("secrets", args)
+        self.assertIn("fetch_service_mitm_certificate", args["secrets"])
+        self.assertIn(
+            "fake-cert", args["secrets"]["fetch_service_mitm_certificate"]
+        )
+
+
 class MakeRockRecipeBuildMixin:
     """Provide the common makeBuild method returning a queued build."""
 
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index 609fc08..edf5ab9 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -6909,6 +6909,7 @@ class LaunchpadObjectFactory(ObjectFactory):
         store_secrets=None,
         store_channels=None,
         date_created=DEFAULT,
+        use_fetch_service=False,
     ):
         """Make a new rock recipe."""
         if registrant is None:
@@ -6957,6 +6958,7 @@ class LaunchpadObjectFactory(ObjectFactory):
             store_secrets=store_secrets,
             store_channels=store_channels,
             date_created=date_created,
+            use_fetch_service=use_fetch_service,
         )
         if is_stale is not None:
             removeSecurityProxy(recipe).is_stale = is_stale