launchpad-reviewers team mailing list archive

["Thiago F. Pappacena" <pappacena@xxxxxxxxx>]

Thiago F. Pappacena has proposed merging ~pappacena/launchpad-buildd:security-manifest-build-metadata into launchpad-buildd:master with ~pappacena/launchpad-buildd:security-manifest as a prerequisite.

Commit message:
Adding build metadata to security manifest

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pappacena/launchpad-buildd/+git/launchpad-buildd/+merge/392564
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad-buildd:security-manifest-build-metadata into launchpad-buildd:master.

diff --git a/lpbuildd/oci.py b/lpbuildd/oci.py
index 480c5d6..cdcdab3 100644
--- a/lpbuildd/oci.py
+++ b/lpbuildd/oci.py
@@ -54,6 +54,7 @@ class OCIBuildManager(SnapBuildProxyMixin, DebianBuildManager):
         self.build_path = extra_args.get("build_path")
         self.proxy_url = extra_args.get("proxy_url")
         self.revocation_endpoint = extra_args.get("revocation_endpoint")
+        self.metadata = extra_args.get("metadata")
         self.proxy_service = None
 
         super(OCIBuildManager, self).initiate(files, chroot, extra_args)
@@ -84,6 +85,11 @@ class OCIBuildManager(SnapBuildProxyMixin, DebianBuildManager):
             args.extend(["--snap-store-proxy-url", snap_store_proxy_url])
         except (NoSectionError, NoOptionError):
             pass
+        if self.metadata is not None:
+            try:
+                args.extend(["--metadata", json.dumps(self.metadata)])
+            except TypeError as e:
+                print("Could not JSONify metadata: %s" % e)
         args.append(self.name)
         self.runTargetSubProcess("build-oci", *args)
 
diff --git a/lpbuildd/target/build_oci.py b/lpbuildd/target/build_oci.py
index 3e4f283..1fbc053 100644
--- a/lpbuildd/target/build_oci.py
+++ b/lpbuildd/target/build_oci.py
@@ -48,6 +48,9 @@ class BuildOCI(SnapBuildProxyOperationMixin, VCSOperationMixin,
             help="A docker build ARG in the format of key=value. "
                  "This option can be repeated many times. For example: "
                  "--build-arg VAR1=A --build-arg VAR2=B")
+        parser.add_argument(
+            "--metadata", default=None,
+            help="Metadata about this build, used to generate manifest file.")
         parser.add_argument("name", help="name of snap to build")
 
     def __init__(self, args, parser):
@@ -118,20 +121,57 @@ class BuildOCI(SnapBuildProxyOperationMixin, VCSOperationMixin,
         # /home/buildd instead.  Make sure it exists.
         self.backend.run(["mkdir", "-p", "/home/buildd"])
 
+    def getCurrentVCSRevision(self):
+        if self.args.branch is not None:
+            revision_cmd = ["bzr", "revno"]
+        else:
+            revision_cmd = ["git", "rev-parse", "HEAD"]
+        return self.backend.run(
+            revision_cmd, cwd=os.path.join("/home/buildd", self.args.name),
+            get_output=True)
+
     def repo(self):
         """Collect git or bzr branch."""
         logger.info("Running repo phase...")
         env = self.build_proxy_environment(proxy_url=self.args.proxy_url)
         self.vcs_fetch(self.args.name, cwd="/home/buildd", env=env)
 
+    def _getSecurityManifestContent(self):
+        try:
+            metadata = json.loads(self.args.metadata) or {}
+        except TypeError:
+            metadata = {}
+        recipe_owner = metadata.get("recipe_owner", {})
+        build_requester = metadata.get("build_requester", {})
+        emails = [i.get("email") for i in (recipe_owner, build_requester)
+                  if i.get("email")]
+
+        return {
+            "manifest-version": "1",
+            "name": self.args.name,
+            "architectures": metadata.get("architectures") or [self.args.arch],
+            "publisher-emails": emails,
+            "image-info": {
+                "build-request-id": metadata.get("build_request_id"),
+                "build-request-timestamp": metadata.get(
+                    "build_request_timestamp"),
+                "build-urls": metadata.get("build_urls") or {}
+            },
+            "vcs-info": [{
+                "source": self.args.git_repository,
+                "source-branch": self.args.git_path,
+                "source-commit": self.getCurrentVCSRevision(),
+                "source-subdir": self.args.build_path,
+                "source-build-file": self.args.build_file,
+                "source-build-args": self.args.build_arg
+            }]
+        }
+
     def createSecurityManifest(self):
         """Generates the security manifest file, returning the tmp file name
         where it is stored in the backend.
         """
-        content = {
-            "manifest-version": "0",
-            "name": self.args.name
-        }
+        content = self._getSecurityManifestContent()
         local_filename = tempfile.mktemp()
         destination_path = self.security_manifest_target_path.lstrip(
             os.path.sep)
diff --git a/lpbuildd/target/tests/test_build_oci.py b/lpbuildd/target/tests/test_build_oci.py
index 87b37b2..166ff9e 100644
--- a/lpbuildd/target/tests/test_build_oci.py
+++ b/lpbuildd/target/tests/test_build_oci.py
@@ -3,6 +3,8 @@
 
 __metaclass__ = type
 
+import datetime
+import json
 import os.path
 import stat
 import subprocess
@@ -73,6 +75,82 @@ class RanBuildCommand(RanCommand):
         super(RanBuildCommand, self).__init__(args, **kwargs)
 
 
+class TestBuildOCIManifestGeneration(TestCase):
+    def test_getSecurityManifestContent(self):
+        now = datetime.datetime.now().isoformat()
+        metadata = {
+            "architectures": ["amd64", "386"],
+            "recipe_owner": dict(name="pappacena", email="me@xxxxxxx"),
+            "build_request_id": 123,
+            "build_request_timestamp": now,
+            "build_requester": dict(name="someone", email="someone@xxxxxxx"),
+            "build_urls": {
+                "amd64": "http://lp.net/build/1";,
+                "386": "http://lp.net/build/2";
+            },
+        }
+        args = [
+            "build-oci",
+            "--backend=fake", "--series=xenial", "--arch=amd64", "1",
+            "--git-repository", "lp:git-repo", "--git-path", "refs/heads/main",
+            "--build-arg", "VAR1=1", "--build-arg", "VAR2=22",
+            "--build-file", "SomeDockerfile", "--build-path", "docker/builder",
+            "--metadata", json.dumps(metadata),
+            "test-image"
+        ]
+        build_oci = parse_args(args=args).operation
+        build_oci.backend.run.result = "a1b2c3d4e5f5"
+        self.assertEqual(build_oci._getSecurityManifestContent(), {
+            "manifest-version": "1",
+            "name": "test-image",
+            "architectures": ["amd64", "386"],
+            "publisher-emails": ["me@xxxxxxx", "someone@xxxxxxx"],
+            "image-info": {
+                "build-request-id": 123,
+                "build-request-timestamp": now,
+                "build-urls": {
+                    "386": "http://lp.net/build/2";,
+                    "amd64": "http://lp.net/build/1"}},
+            "vcs-info": [{
+                "source": "lp:git-repo",
+                "source-branch": "refs/heads/main",
+                "source-build-args": ["VAR1=1", "VAR2=22"],
+                "source-build-file": "SomeDockerfile",
+                "source-commit": "a1b2c3d4e5f5",
+                "source-subdir": "docker/builder"
+            }]
+        })
+
+    def test_getSecurityManifestContent_without_manifest(self):
+        """With minimal parameters, the manifest content should give
+        something back without breaking."""
+        args = [
+            "build-oci",
+            "--backend=fake", "--series=xenial", "--arch=amd64", "1",
+            "--git-repository", "lp:git-repo", "--git-path", "refs/heads/main",
+            "test-image"
+        ]
+        build_oci = parse_args(args=args).operation
+        self.assertEqual(build_oci._getSecurityManifestContent(), {
+            "manifest-version": "1",
+            "name": "test-image",
+            "architectures": ["amd64"],
+            "publisher-emails": [],
+            "image-info": {
+                "build-request-id": None,
+                "build-request-timestamp": None,
+                "build-urls": {}},
+            "vcs-info": [{
+                "source": "lp:git-repo",
+                "source-branch": "refs/heads/main",
+                "source-build-args": [],
+                "source-build-file": None,
+                "source-commit": None,
+                "source-subdir": "."
+            }]
+        })
+
+
 class TestBuildOCI(TestCase):
 
     def test_run_build_command_no_env(self):
@@ -291,11 +369,16 @@ class TestBuildOCI(TestCase):
             ]))
 
     def assertRanPostBuildCommands(self, build_oci):
+        rev_num_args = (
+            ['bzr', 'revno'] if build_oci.args.branch
+            else ['git', 'rev-parse', 'HEAD'])
         self.assertThat(build_oci.backend.run.calls[1:], MatchesListwise([
             RanBuildCommand(
                 ['docker', 'create', '--name', 'test-image', 'test-image'],
                 cwd="/home/buildd/test-image"),
             RanCommand(['mkdir', '-p', '/tmp/image-root-dir/.rocks']),
+            RanCommand(
+                rev_num_args, cwd="/home/buildd/test-image", get_output=True),
             RanBuildCommand(
                 ['docker', 'cp', '/tmp/image-root-dir/.', 'test-image:/'],
                 cwd="/home/buildd/test-image"),