Merge ~pappacena/launchpad-buildd:security-manifest into launchpad-buildd:master

[Thiago F. Pappacena]

Including manifest file in resulting OCI image.

[Tom Wardill (twom)]

This looks good to me as is, though I think we should try and work out what we do in the case of not being able to add/generate the manifest, and whether that's enough to fail a build or not.

review: Approve

[Thiago F. Pappacena (pappacena)]

twom, I agree we should discuss this. The current approach (implemented in follow up MPs) is enclosing the manifest file content generation in try/except blocks, and always generating at least a minimum version of the file.

The steps to copy the file into the image are not protected, but there is a bit less uncertainty in those steps. For that reason, I thought it would be better to fail the build if something goes wrong in the file injection phase. But of course, if we agree I can add the guard there too.

[Colin Watson (cjwatson)]

review: Approve

diff --git a/lpbuildd/target/build_oci.py b/lpbuildd/target/build_oci.py
index af56671..3e4f283 100644
--- a/lpbuildd/target/build_oci.py
+++ b/lpbuildd/target/build_oci.py
@@ -6,6 +6,7 @@ from __future__ import print_function
 __metaclass__ = type
 
 from collections import OrderedDict
+import json
 import logging
 import os.path
 import sys
@@ -53,6 +54,10 @@ class BuildOCI(SnapBuildProxyOperationMixin, VCSOperationMixin,
         super(BuildOCI, self).__init__(args, parser)
         self.bin = os.path.dirname(sys.argv[0])
         self.buildd_path = os.path.join("/home/buildd", self.args.name)
+        # Temp directory where we store files that will be included in the
+        # final filesystem of the image.
+        self.backend_tmp_fs_dir = "/tmp/image-root-dir/"
+        self.security_manifest_target_path = "/.rocks/manifest.json"
 
     def _add_docker_engine_proxy_settings(self):
         """Add systemd file for docker proxy settings."""
@@ -119,6 +124,65 @@ class BuildOCI(SnapBuildProxyOperationMixin, VCSOperationMixin,
         env = self.build_proxy_environment(proxy_url=self.args.proxy_url)
         self.vcs_fetch(self.args.name, cwd="/home/buildd", env=env)
 
+    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
+        }
+        local_filename = tempfile.mktemp()
+        destination_path = self.security_manifest_target_path.lstrip(
+            os.path.sep)
+        destination = os.path.join(self.backend_tmp_fs_dir, destination_path)
+        with open(local_filename, 'w') as fd:
+            json.dump(content, fd, indent=2)
+        self.backend.copy_in(local_filename, destination)
+        return destination
+
+    def initTempRootDir(self):
+        """Initialize in the backend the directories that will be included in
+        resulting image's filesystem.
+        """
+        security_manifest_dir = os.path.dirname(
+            self.security_manifest_target_path)
+        dir = os.path.join(
+            self.backend_tmp_fs_dir,
+            security_manifest_dir.lstrip(os.path.sep))
+        self.backend.run(["mkdir", "-p", dir])
+
+    def createImageContainer(self):
+        """Creates a container from the built image, so we can play with
+        it's filesystem."""
+        self.run_build_command([
+            "docker", "create", "--name", self.args.name, self.args.name])
+
+    def removeImageContainer(self):
+        self.run_build_command(["docker", "rm", self.args.name])
+
+    def commitImage(self):
+        """Commits the tmp container, overriding the originally built image."""
+        self.run_build_command([
+            "docker", "commit", self.args.name, self.args.name])
+
+    def addFilesToImageContainer(self):
+        """Flushes all files from temp root dir (in the backend) to the
+        resulting image container."""
+        # The extra '.' in the end is important. It indicates to docker that
+        # the directory itself should be copied, instead of the list of
+        # files in the directory. It makes docker keep the paths.
+        src = os.path.join(self.backend_tmp_fs_dir, ".")
+        self.run_build_command(["docker", "cp", src, "%s:/" % self.args.name])
+
+    def addSecurityManifest(self):
+        self.createImageContainer()
+        self.initTempRootDir()
+        self.createSecurityManifest()
+        self.addFilesToImageContainer()
+        self.commitImage()
+        self.removeImageContainer()
+
     def build(self):
         logger.info("Running build phase...")
         args = ["docker", "build", "--no-cache"]
@@ -143,6 +207,7 @@ class BuildOCI(SnapBuildProxyOperationMixin, VCSOperationMixin,
         self._check_path_escape(build_context_path)
         args.append(build_context_path)
         self.run_build_command(args)
+        self.addSecurityManifest()
 
     def run(self):
         try:
diff --git a/lpbuildd/target/tests/test_build_oci.py b/lpbuildd/target/tests/test_build_oci.py
index e58344f..87b37b2 100644
--- a/lpbuildd/target/tests/test_build_oci.py
+++ b/lpbuildd/target/tests/test_build_oci.py
@@ -290,6 +290,23 @@ class TestBuildOCI(TestCase):
                 cwd="/home/buildd/test-image", **env),
             ]))
 
+    def assertRanPostBuildCommands(self, build_oci):
+        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']),
+            RanBuildCommand(
+                ['docker', 'cp', '/tmp/image-root-dir/.', 'test-image:/'],
+                cwd="/home/buildd/test-image"),
+            RanBuildCommand(
+                ['docker', 'commit', 'test-image', 'test-image'],
+                cwd="/home/buildd/test-image"),
+            RanBuildCommand(
+                ['docker', 'rm', 'test-image'],
+                cwd="/home/buildd/test-image"),
+        ]))
+
     def test_build(self):
         args = [
             "build-oci",
@@ -299,12 +316,11 @@ class TestBuildOCI(TestCase):
         build_oci = parse_args(args=args).operation
         build_oci.backend.add_dir('/build/test-directory')
         build_oci.build()
-        self.assertThat(build_oci.backend.run.calls, MatchesListwise([
-            RanBuildCommand(
-                ["docker", "build", "--no-cache", "--tag", "test-image",
-                 "/home/buildd/test-image/."],
-                cwd="/home/buildd/test-image"),
-            ]))
+        self.assertThat(build_oci.backend.run.calls[0], RanBuildCommand(
+            ["docker", "build", "--no-cache", "--tag", "test-image",
+             "/home/buildd/test-image/."],
+            cwd="/home/buildd/test-image"))
+        self.assertRanPostBuildCommands(build_oci)
 
     def test_build_with_file(self):
         args = [
@@ -316,13 +332,12 @@ class TestBuildOCI(TestCase):
         build_oci = parse_args(args=args).operation
         build_oci.backend.add_dir('/build/test-directory')
         build_oci.build()
-        self.assertThat(build_oci.backend.run.calls, MatchesListwise([
-            RanBuildCommand(
-                ["docker", "build", "--no-cache", "--tag", "test-image",
-                 "--file", "./build-aux/Dockerfile",
-                 "/home/buildd/test-image/."],
-                cwd="/home/buildd/test-image"),
-            ]))
+        self.assertThat(build_oci.backend.run.calls[0], RanBuildCommand(
+            ["docker", "build", "--no-cache", "--tag", "test-image",
+             "--file", "./build-aux/Dockerfile",
+             "/home/buildd/test-image/."],
+            cwd="/home/buildd/test-image"))
+        self.assertRanPostBuildCommands(build_oci)
 
     def test_build_with_path(self):
         args = [
@@ -334,12 +349,11 @@ class TestBuildOCI(TestCase):
         build_oci = parse_args(args=args).operation
         build_oci.backend.add_dir('/build/test-directory')
         build_oci.build()
-        self.assertThat(build_oci.backend.run.calls, MatchesListwise([
-            RanBuildCommand(
-                ["docker", "build", "--no-cache", "--tag", "test-image",
-                 "/home/buildd/test-image/a-sub-directory/"],
-                cwd="/home/buildd/test-image"),
-            ]))
+        self.assertThat(build_oci.backend.run.calls[0], RanBuildCommand(
+            ["docker", "build", "--no-cache", "--tag", "test-image",
+             "/home/buildd/test-image/a-sub-directory/"],
+            cwd="/home/buildd/test-image"))
+        self.assertRanPostBuildCommands(build_oci)
 
     def test_build_with_file_and_path(self):
         args = [
@@ -352,13 +366,12 @@ class TestBuildOCI(TestCase):
         build_oci = parse_args(args=args).operation
         build_oci.backend.add_dir('/build/test-directory')
         build_oci.build()
-        self.assertThat(build_oci.backend.run.calls, MatchesListwise([
-            RanBuildCommand(
-                ["docker", "build", "--no-cache", "--tag", "test-image",
-                 "--file", "test-build-path/build-aux/Dockerfile",
-                 "/home/buildd/test-image/test-build-path"],
-                cwd="/home/buildd/test-image"),
-            ]))
+        self.assertThat(build_oci.backend.run.calls[0], RanBuildCommand(
+            ["docker", "build", "--no-cache", "--tag", "test-image",
+             "--file", "test-build-path/build-aux/Dockerfile",
+             "/home/buildd/test-image/test-build-path"],
+            cwd="/home/buildd/test-image"))
+        self.assertRanPostBuildCommands(build_oci)
 
     def test_build_with_args(self):
         args = [
@@ -372,14 +385,13 @@ class TestBuildOCI(TestCase):
         build_oci = parse_args(args=args).operation
         build_oci.backend.add_dir('/build/test-directory')
         build_oci.build()
-        self.assertThat(build_oci.backend.run.calls, MatchesListwise([
-            RanBuildCommand(
-                ["docker", "build", "--no-cache", "--tag", "test-image",
-                 "--file", "test-build-path/build-aux/Dockerfile",
-                 "--build-arg=VAR1=xxx", "--build-arg=VAR2=yyy",
-                 "/home/buildd/test-image/test-build-path"],
-                cwd="/home/buildd/test-image"),
-            ]))
+        self.assertThat(build_oci.backend.run.calls[0], RanBuildCommand(
+            ["docker", "build", "--no-cache", "--tag", "test-image",
+             "--file", "test-build-path/build-aux/Dockerfile",
+             "--build-arg=VAR1=xxx", "--build-arg=VAR2=yyy",
+             "/home/buildd/test-image/test-build-path"],
+            cwd="/home/buildd/test-image"))
+        self.assertRanPostBuildCommands(build_oci)
 
     def test_build_proxy(self):
         args = [
@@ -391,14 +403,13 @@ class TestBuildOCI(TestCase):
         build_oci = parse_args(args=args).operation
         build_oci.backend.add_dir('/build/test-directory')
         build_oci.build()
-        self.assertThat(build_oci.backend.run.calls, MatchesListwise([
-            RanBuildCommand(
-                ["docker", "build", "--no-cache",
-                 "--build-arg", "http_proxy=http://proxy.example:3128/",
-                 "--build-arg", "https_proxy=http://proxy.example:3128/",
-                 "--tag", "test-image", "/home/buildd/test-image/."],
-                cwd="/home/buildd/test-image"),
-            ]))
+        self.assertThat(build_oci.backend.run.calls[0], RanBuildCommand(
+            ["docker", "build", "--no-cache",
+             "--build-arg", "http_proxy=http://proxy.example:3128/",
+             "--build-arg", "https_proxy=http://proxy.example:3128/",
+             "--tag", "test-image", "/home/buildd/test-image/."],
+            cwd="/home/buildd/test-image"))
+        self.assertRanPostBuildCommands(build_oci)
 
     def test_run_succeeds(self):
         args = [