Merge ~pappacena/launchpad:oci-build-info-to-buildd into launchpad:master

[Thiago F. Pappacena]

Adding extra meta info when requesting an OCI build to buildd

This information will be useful on buildd side to create the OCI security manifest file.

[Colin Watson (cjwatson)]

Mostly looks OK, thanks. The processor name vs. architecture tag issue definitely needs fixing; my other comments are nice-to-have.

review: Approve

66ce7db... by Thiago F. Pappacena on 2020-10-20

Improving tests and small refactoring

[Thiago F. Pappacena (pappacena)]

Pushed the requested changes. I'll land this.

diff --git a/database/schema/security.cfg b/database/schema/security.cfg
index d5a23f1..11aba6f 100644
--- a/database/schema/security.cfg
+++ b/database/schema/security.cfg
@@ -1004,6 +1004,7 @@ public.ocipushrule                            = SELECT
 public.ocirecipe                              = SELECT
 public.ocirecipebuild                         = SELECT, UPDATE
 public.ocirecipebuildjob                      = SELECT, INSERT
+public.ocirecipejob                           = SELECT
 public.openididentifier                       = SELECT
 public.packageset                             = SELECT
 public.packagesetgroup                        = SELECT
diff --git a/lib/lp/oci/model/ocirecipebuildbehaviour.py b/lib/lp/oci/model/ocirecipebuildbehaviour.py
index 1008884..7a8d7fb 100644
--- a/lib/lp/oci/model/ocirecipebuildbehaviour.py
+++ b/lib/lp/oci/model/ocirecipebuildbehaviour.py
@@ -38,6 +38,7 @@ from lp.buildmaster.model.buildfarmjobbehaviour import (
 from lp.oci.interfaces.ocirecipebuild import IOCIFileSet
 from lp.registry.interfaces.series import SeriesStatus
 from lp.services.librarian.utils import copy_and_close
+from lp.services.webapp import canonical_url
 from lp.snappy.model.snapbuildbehaviour import SnapProxyMixin
 from lp.soyuz.adapters.archivedependencies import (
     get_sources_list_for_building,
@@ -77,6 +78,40 @@ class OCIRecipeBuildBehaviour(SnapProxyMixin, BuildFarmJobBehaviourBase):
             raise CannotBuild(
                 "Missing chroot for %s" % build.distro_arch_series.displayname)
 
+    def _getBuildInfoArgs(self):
+        def format_user(user):
+            if user is None:
+                return None
+            hide_email = not user.preferredemail or user.hide_email_addresses
+            return {
+                "name": user.name,
+                "email": (None if hide_email else user.preferredemail.email)}
+        build = self.build
+        build_request = build.build_request
+        builds = list(build_request.builds) if build_request else [build]
+        info = {
+            "architectures": [],
+            "recipe_owner": format_user(self.build.recipe.owner),
+            "build_request_id": None,
+            "build_request_timestamp": None,
+            # With build_request set, all builds in this list will have the
+            # same requester. Without build_request, we only care about the
+            # only existing build in this list.
+            "build_requester": format_user(builds[0].requester),
+            # Build URL per architecture.
+            "build_urls": {},
+        }
+        if build_request:
+            info["build_request_id"] = build_request.id
+            info["build_request_timestamp"] = (
+                build_request.date_requested.isoformat())
+        info["architectures"] = [i.distro_arch_series.architecturetag
+                                 for i in builds]
+        info["build_urls"] = {
+            i.distro_arch_series.architecturetag: canonical_url(i)
+            for i in builds}
+        return info
+
     @defer.inlineCallbacks
     def extraBuildArgs(self, logger=None):
         """
@@ -101,6 +136,7 @@ class OCIRecipeBuildBehaviour(SnapProxyMixin, BuildFarmJobBehaviourBase):
         # XML-RPC.
         args['build_args'] = removeSecurityProxy(build.recipe.build_args)
         args['build_path'] = build.recipe.build_path
+        args['metadata'] = self._getBuildInfoArgs()
 
         if build.recipe.git_ref is not None:
             args["git_repository"] = (
diff --git a/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py b/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py
index ded60cf..562cb26 100644
--- a/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py
+++ b/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py
@@ -29,6 +29,7 @@ from testtools.matchers import (
     IsInstance,
     MatchesDict,
     MatchesListwise,
+    MatchesSetwise,
     StartsWith,
     )
 from testtools.twistedsupport import (
@@ -104,7 +105,7 @@ class MakeOCIBuildMixin:
         build.queueBuild()
         return build
 
-    def makeJob(self, git_ref, recipe=None, build=None, **kwargs):
+    def makeJob(self, git_ref=None, recipe=None, build=None, **kwargs):
         """Create a sample `IOCIRecipeBuildBehaviour`."""
         if build is None:
             if recipe is None:
@@ -112,6 +113,8 @@ class MakeOCIBuildMixin:
             else:
                 build = self.factory.makeOCIRecipeBuild(
                     recipe=recipe, **kwargs)
+        if git_ref is None:
+            [git_ref] = self.factory.makeGitRefs()
         build.recipe.git_ref = git_ref
         build.recipe.build_args = {"BUILD_VAR": "123"}
 
@@ -131,7 +134,7 @@ class MakeOCIBuildMixin:
         return job
 
 
-class TestOCIBuildBehaviour(TestCaseWithFactory):
+class TestOCIBuildBehaviour(TestCaseWithFactory, MakeOCIBuildMixin):
 
     layer = LaunchpadZopelessLayer
 
@@ -150,6 +153,99 @@ class TestOCIBuildBehaviour(TestCaseWithFactory):
         job = IBuildFarmJobBehaviour(build)
         self.assertProvides(job, IBuildFarmJobBehaviour)
 
+    def makeRecipe(self, processor_names, **kwargs):
+        recipe = self.factory.makeOCIRecipe(**kwargs)
+        processors_list = []
+        distroseries = self.factory.makeDistroSeries(
+            distribution=recipe.oci_project.distribution)
+        for proc_name in processor_names:
+            proc = getUtility(IProcessorSet).getByName(proc_name)
+            distro = self.factory.makeDistroArchSeries(
+                distroseries=distroseries, architecturetag=proc_name,
+                processor=proc)
+            distro.addOrUpdateChroot(self.factory.makeLibraryFileAlias())
+            processors_list.append(proc)
+        recipe.setProcessors(processors_list)
+        return recipe
+
+    def makeBuildRequest(self, recipe, requester):
+        build_request = recipe.requestBuilds(requester)
+        # Create the builds for the build request, and set them at the build
+        # request job.
+        builds = recipe.requestBuildsFromJob(requester, build_request)
+        job = removeSecurityProxy(build_request).job
+        removeSecurityProxy(job).builds = builds
+        return build_request
+
+    def test_getBuildInfoArgs_with_build_request(self):
+        owner = self.factory.makePerson()
+        owner.setPreferredEmail(self.factory.makeEmail('[e-mail]', owner))
+        oci_project = self.factory.makeOCIProject(registrant=owner)
+        recipe = self.makeRecipe(
+            processor_names=["amd64", "386"],
+            oci_project=oci_project, registrant=owner, owner=owner)
+        build_request = self.makeBuildRequest(recipe, recipe.owner)
+        self.assertEqual(2, build_request.builds.count())
+        build = build_request.builds[0]
+        build_per_proc = {i.processor.name: i for i in build_request.builds}
+        job = self.makeJob(build=build)
+
+        self.assertThat(job._getBuildInfoArgs(), MatchesDict({
+            "architectures": MatchesSetwise(Equals("amd64"), Equals("386")),
+            "recipe_owner": Equals({
+                "name": recipe.owner.name,
+                "email": "[e-mail]"}),
+            "build_request_id": Equals(build_request.id),
+            "build_requester": Equals({
+                "name": build.requester.name,
+                "email": "[e-mail]"}),
+            "build_request_timestamp": Equals(
+                build_request.date_requested.isoformat()),
+            "build_urls": MatchesDict({
+                "amd64": Equals(canonical_url(build_per_proc["amd64"])),
+                "386": Equals(canonical_url(build_per_proc["386"]))
+            }),
+        }))
+
+    def test_getBuildInfoArgs_hide_email(self):
+        owner = self.factory.makePerson()
+        owner.setPreferredEmail(self.factory.makeEmail('[e-mail]', owner))
+        owner.hide_email_addresses = True
+        oci_project = self.factory.makeOCIProject(registrant=owner)
+        recipe = self.makeRecipe(
+            processor_names=["amd64"],
+            oci_project=oci_project, registrant=owner, owner=owner)
+        build_request = self.makeBuildRequest(recipe, recipe.owner)
+        build = build_request.builds[0]
+        job = self.makeJob(build=build)
+
+        self.assertThat(job._getBuildInfoArgs(), MatchesDict({
+            "architectures": Equals(["amd64"]),
+            "recipe_owner": Equals({"name": recipe.owner.name, "email": None}),
+            "build_request_id": Equals(build_request.id),
+            "build_requester": Equals({
+                "name": build.requester.name, "email": None}),
+            "build_request_timestamp": Equals(
+                build_request.date_requested.isoformat()),
+            "build_urls": MatchesDict({
+                "amd64": Equals(canonical_url(build_request.builds[0]))
+            }),
+        }))
+
+    def test_getBuildInfoArgs_without_build_request(self):
+        recipe = self.makeRecipe(processor_names=["amd64"])
+        distro_arch_series = removeSecurityProxy(
+            recipe.getAllowedArchitectures()[0])
+        build = self.factory.makeOCIRecipeBuild(
+            recipe=recipe, distro_arch_series=distro_arch_series)
+        job = self.makeJob(build=build)
+        self.assertThat(job._getBuildInfoArgs(), ContainsDict({
+            "architectures": Equals(["amd64"]),
+            "build_request_id": Equals(None),
+            "build_request_timestamp": Equals(None),
+            "build_urls": MatchesDict({"amd64": Equals(canonical_url(build))}),
+        }))
+
 
 class TestAsyncOCIRecipeBuildBehaviour(
         StatsMixin, MakeOCIBuildMixin, TestCaseWithFactory):
@@ -279,7 +375,14 @@ class TestAsyncOCIRecipeBuildBehaviour(
             "revocation_endpoint":  RevocationEndpointMatcher(job, self.now),
             "series": Equals(job.build.distro_arch_series.distroseries.name),
             "trusted_keys": Equals(expected_trusted_keys),
-            }))
+            # 'metadata' has detailed tests at TestOCIBuildBehaviour class.
+            "metadata": ContainsDict({
+                "architectures": Equals(["i386"]),
+                "build_request_id": Equals(None),
+                "build_request_timestamp": Equals(None),
+                "build_urls": Equals({"i386": canonical_url(job.build)})
+            })
+        }))
 
     @defer.inlineCallbacks
     def test_extraBuildArgs_git_HEAD(self):
@@ -311,7 +414,14 @@ class TestAsyncOCIRecipeBuildBehaviour(
             "revocation_endpoint":  RevocationEndpointMatcher(job, self.now),
             "series": Equals(job.build.distro_arch_series.distroseries.name),
             "trusted_keys": Equals(expected_trusted_keys),
-            }))
+            # 'metadata' has detailed tests at TestOCIBuildBehaviour class.
+            "metadata": ContainsDict({
+                "architectures": Equals(["i386"]),
+                "build_request_id": Equals(None),
+                "build_request_timestamp": Equals(None),
+                "build_urls": Equals({"i386": canonical_url(job.build)})
+            })
+        }))
 
     @defer.inlineCallbacks
     def test_composeBuildRequest_proxy_url_set(self):