Merge ~twom/launchpad-buildd:oci-build-path into launchpad-buildd:master

[Tom Wardill]

Add build path handling to OCI builds

A user may wish to use a subdirectory as the build context, we should allow this.
This does enforce that the Dockerfile will live in the given directory.

5d972a3... by Tom Wardill on 2020-08-17

Add changelog entry

[Colin Watson (cjwatson)]

review: Approve

diff --git a/debian/changelog b/debian/changelog
index e6caf70..3a3704c 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -37,6 +37,7 @@ launchpad-buildd (190) UNRELEASED; urgency=medium
 
   [ Tom Wardill ]
   * Improve error logging in OCI post-build 
+  * Add build_path directory context for OCI builds 
 
  -- Colin Watson <[e-mail]>  Tue, 28 Apr 2020 10:19:27 +0100
 
diff --git a/lpbuildd/oci.py b/lpbuildd/oci.py
index c89a7c0..1fccada 100644
--- a/lpbuildd/oci.py
+++ b/lpbuildd/oci.py
@@ -49,6 +49,7 @@ class OCIBuildManager(SnapBuildProxyMixin, DebianBuildManager):
         self.git_repository = extra_args.get("git_repository")
         self.git_path = extra_args.get("git_path")
         self.build_file = extra_args.get("build_file")
+        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.proxy_service = None
@@ -69,6 +70,8 @@ class OCIBuildManager(SnapBuildProxyMixin, DebianBuildManager):
             args.extend(["--git-path", self.git_path])
         if self.build_file is not None:
             args.extend(["--build-file", self.build_file])
+        if self.build_path is not None:
+            args.extend(["--build-path", self.build_path])
         try:
             snap_store_proxy_url = self._builder._config.get(
                 "proxy", "snapstore")
diff --git a/lpbuildd/target/build_oci.py b/lpbuildd/target/build_oci.py
index 1f220ab..377fc68 100644
--- a/lpbuildd/target/build_oci.py
+++ b/lpbuildd/target/build_oci.py
@@ -39,6 +39,9 @@ class BuildOCI(SnapBuildProxyOperationMixin, VCSOperationMixin,
         super(BuildOCI, cls).add_arguments(parser)
         parser.add_argument(
             "--build-file", help="path to Dockerfile in branch")
+        parser.add_argument(
+            "--build-path", default=".",
+            help="context directory for docker build")
         parser.add_argument("name", help="name of snap to build")
 
     def __init__(self, args, parser):
@@ -64,10 +67,10 @@ class BuildOCI(SnapBuildProxyOperationMixin, VCSOperationMixin,
                 systemd_file.flush()
                 self.backend.copy_in(systemd_file.name, file_path)
 
-    def _check_build_file_escape(self):
+    def _check_path_escape(self, path_to_check):
         """Check the build file path doesn't escape the build directory."""
         build_file_path = os.path.realpath(
-            os.path.join(self.buildd_path, self.args.build_file))
+            os.path.join(self.buildd_path, path_to_check))
         common_path = os.path.commonprefix((build_file_path, self.buildd_path))
         if common_path != self.buildd_path:
             raise InvalidBuildFilePath("Invalid build file path.")
@@ -120,9 +123,14 @@ class BuildOCI(SnapBuildProxyOperationMixin, VCSOperationMixin,
                     ["--build-arg", "{}={}".format(var, self.args.proxy_url)])
         args.extend(["--tag", self.args.name])
         if self.args.build_file is not None:
-            self._check_build_file_escape()
-            args.extend(["--file", self.args.build_file])
-        args.append(self.buildd_path)
+            build_file_path = os.path.join(
+                self.args.build_path, self.args.build_file)
+            self._check_path_escape(build_file_path)
+            args.extend(["--file", build_file_path])
+        build_context_path = os.path.join(
+            self.buildd_path, self.args.build_path)
+        self._check_path_escape(build_context_path)
+        args.append(build_context_path)
         self.run_build_command(args)
 
     def run(self):
diff --git a/lpbuildd/target/tests/test_build_oci.py b/lpbuildd/target/tests/test_build_oci.py
index 47fa1a5..a2c67d7 100644
--- a/lpbuildd/target/tests/test_build_oci.py
+++ b/lpbuildd/target/tests/test_build_oci.py
@@ -302,7 +302,7 @@ class TestBuildOCI(TestCase):
         self.assertThat(build_oci.backend.run.calls, MatchesListwise([
             RanBuildCommand(
                 ["docker", "build", "--no-cache", "--tag", "test-image",
-                 "/home/buildd/test-image"],
+                 "/home/buildd/test-image/."],
                 cwd="/home/buildd/test-image"),
             ]))
 
@@ -319,8 +319,44 @@ class TestBuildOCI(TestCase):
         self.assertThat(build_oci.backend.run.calls, MatchesListwise([
             RanBuildCommand(
                 ["docker", "build", "--no-cache", "--tag", "test-image",
-                 "--file", "build-aux/Dockerfile",
-                 "/home/buildd/test-image"],
+                 "--file", "./build-aux/Dockerfile",
+                 "/home/buildd/test-image/."],
+                cwd="/home/buildd/test-image"),
+            ]))
+
+    def test_build_with_path(self):
+        args = [
+            "build-oci",
+            "--backend=fake", "--series=xenial", "--arch=amd64", "1",
+            "--branch", "lp:foo", "--build-path", "a-sub-directory/",
+            "test-image",
+            ]
+        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"),
+            ]))
+
+    def test_build_with_file_and_path(self):
+        args = [
+            "build-oci",
+            "--backend=fake", "--series=xenial", "--arch=amd64", "1",
+            "--branch", "lp:foo", "--build-file", "build-aux/Dockerfile",
+            "--build-path", "test-build-path",
+            "test-image",
+            ]
+        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"),
             ]))
 
@@ -339,7 +375,7 @@ class TestBuildOCI(TestCase):
                 ["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"],
+                 "--tag", "test-image", "/home/buildd/test-image/."],
                 cwd="/home/buildd/test-image"),
             ]))
 
@@ -360,7 +396,7 @@ class TestBuildOCI(TestCase):
                 cwd="/home/buildd")),
             AnyMatch(RanBuildCommand(
                 ["docker", "build", "--no-cache", "--tag", "test-image",
-                 "/home/buildd/test-image"],
+                 "/home/buildd/test-image/."],
                 cwd="/home/buildd/test-image")),
             ))
 
@@ -451,3 +487,39 @@ class TestBuildOCI(TestCase):
             '/etc/hosts',
             os.path.join(build_oci.buildd_path, 'Dockerfile'))
         self.assertRaises(InvalidBuildFilePath, build_oci.build)
+
+    def test_build_with_invalid_build_path_parent(self):
+        args = [
+            "build-oci",
+            "--backend=fake", "--series=xenial", "--arch=amd64", "1",
+            "--branch", "lp:foo", "--build-path", "../",
+            "test-image",
+            ]
+        build_oci = parse_args(args=args).operation
+        build_oci.backend.add_dir('/build/test-directory')
+        self.assertRaises(InvalidBuildFilePath, build_oci.build)
+
+    def test_build_with_invalid_build_path_absolute(self):
+        args = [
+            "build-oci",
+            "--backend=fake", "--series=xenial", "--arch=amd64", "1",
+            "--branch", "lp:foo", "--build-path", "/etc",
+            "test-image",
+            ]
+        build_oci = parse_args(args=args).operation
+        build_oci.backend.add_dir('/build/test-directory')
+        self.assertRaises(InvalidBuildFilePath, build_oci.build)
+
+    def test_build_with_invalid_build_path_symlink(self):
+        args = [
+            "build-oci",
+            "--backend=fake", "--series=xenial", "--arch=amd64", "1",
+            "--branch", "lp:foo", "--build-path", "build/",
+            "test-image",
+            ]
+        build_oci = parse_args(args=args).operation
+        build_oci.buildd_path = self.useFixture(TempDir()).path
+        os.symlink(
+            '/etc/hosts',
+            os.path.join(build_oci.buildd_path, 'build'))
+        self.assertRaises(InvalidBuildFilePath, build_oci.build)