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

[Thiago F. Pappacena]

Adding support for --build-arg when building OCI images

[Colin Watson (cjwatson)]

review: Approve

3d24c4c... by Thiago F. Pappacena on 2020-08-27

Improving docs and using lpbuildd.util.shell_escape instead of pipes

[Thiago F. Pappacena (pappacena)]

Pushed the requested changes. This should be good to be landed now.

diff --git a/README b/README
index d2eb28e..05a74c8 100644
--- a/README
+++ b/README
@@ -29,6 +29,34 @@ fakeroot debian/rules clean
 rm launchpad-buildd*deb
 rm ../launchpad-buildd*changes
 
+
+Developing
+************
+
+First of all, it is recommended that you create a lxc container, since the
+following steps will make changes in your system. And since some build types
+will only work with virtualized containers, creating a lxc vm is the best way
+to go. You can create a container with the following command:
+
+        lxc launch --vm ubuntu:18.04 lp-builddev
+
+Note that you may want to have a profile to share the source code with the
+container before running the above command.
+
+Then, inside the container, install the necessary dependencies:
+
+        sudo apt-get update
+        cat system-dependencies.txt | sudo xargs apt-get install -y
+
+This should be enough for you to be able to run `make check`, which runs the
+test suite both in python2 and python3.
+
+More information on how to integrate it with Launchpad can be found here:
+https://dev.launchpad.net/Soyuz/HowToDevelopWithBuildd
+
+Now, happy coding.
+
+
 Production deployment notes
 ***************************
 
diff --git a/lpbuildd/oci.py b/lpbuildd/oci.py
index 1fccada..480c5d6 100644
--- a/lpbuildd/oci.py
+++ b/lpbuildd/oci.py
@@ -21,6 +21,7 @@ from lpbuildd.debian import (
     DebianBuildState,
     )
 from lpbuildd.snap import SnapBuildProxyMixin
+from lpbuildd.util import shell_escape
 
 
 RETCODE_SUCCESS = 0
@@ -49,6 +50,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_args = extra_args.get("build_args", {})
         self.build_path = extra_args.get("build_path")
         self.proxy_url = extra_args.get("proxy_url")
         self.revocation_endpoint = extra_args.get("revocation_endpoint")
@@ -70,6 +72,10 @@ 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_args:
+            for k, v in self.build_args.items():
+                args.extend([
+                    "--build-arg=%s=%s" % (shell_escape(k), shell_escape(v))])
         if self.build_path is not None:
             args.extend(["--build-path", self.build_path])
         try:
diff --git a/lpbuildd/target/build_oci.py b/lpbuildd/target/build_oci.py
index 377fc68..af56671 100644
--- a/lpbuildd/target/build_oci.py
+++ b/lpbuildd/target/build_oci.py
@@ -42,6 +42,11 @@ class BuildOCI(SnapBuildProxyOperationMixin, VCSOperationMixin,
         parser.add_argument(
             "--build-path", default=".",
             help="context directory for docker build")
+        parser.add_argument(
+            "--build-arg", default=[], action='append',
+            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("name", help="name of snap to build")
 
     def __init__(self, args, parser):
@@ -127,6 +132,12 @@ class BuildOCI(SnapBuildProxyOperationMixin, VCSOperationMixin,
                 self.args.build_path, self.args.build_file)
             self._check_path_escape(build_file_path)
             args.extend(["--file", build_file_path])
+
+        # Keep this at the end, so we give the user a chance to override any
+        # build-arg we set automatically (like http_proxy).
+        for arg in self.args.build_arg:
+            args.extend(["--build-arg=%s" % arg])
+
         build_context_path = os.path.join(
             self.buildd_path, self.args.build_path)
         self._check_path_escape(build_context_path)
diff --git a/lpbuildd/target/tests/test_build_oci.py b/lpbuildd/target/tests/test_build_oci.py
index a2c67d7..e58344f 100644
--- a/lpbuildd/target/tests/test_build_oci.py
+++ b/lpbuildd/target/tests/test_build_oci.py
@@ -360,6 +360,27 @@ class TestBuildOCI(TestCase):
                 cwd="/home/buildd/test-image"),
             ]))
 
+    def test_build_with_args(self):
+        args = [
+            "build-oci",
+            "--backend=fake", "--series=xenial", "--arch=amd64", "1",
+            "--branch", "lp:foo", "--build-file", "build-aux/Dockerfile",
+            "--build-path", "test-build-path",
+            "--build-arg=VAR1=xxx", "--build-arg=VAR2=yyy",
+            "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",
+                 "--build-arg=VAR1=xxx", "--build-arg=VAR2=yyy",
+                 "/home/buildd/test-image/test-build-path"],
+                cwd="/home/buildd/test-image"),
+            ]))
+
     def test_build_proxy(self):
         args = [
             "build-oci",
diff --git a/lpbuildd/tests/test_oci.py b/lpbuildd/tests/test_oci.py
index f2dd421..6079634 100644
--- a/lpbuildd/tests/test_oci.py
+++ b/lpbuildd/tests/test_oci.py
@@ -183,7 +183,7 @@ class TestOCIBuildManagerIteration(TestCase):
         self.assertFalse(self.builder.wasCalled("buildFail"))
 
     @defer.inlineCallbacks
-    def test_iterate_with_file(self):
+    def test_iterate_with_file_and_args(self):
         # This sha would change as it includes file attributes in the
         # tar file. Fix it so we can test against a known value.
         sha_mock = self.useFixture(
@@ -195,12 +195,15 @@ class TestOCIBuildManagerIteration(TestCase):
             "git_repository": "https://git.launchpad.dev/~example/+git/snap",
             "git_path": "master",
             "build_file": "build-aux/Dockerfile",
+            "build_args": {"VAR1": "xxx", "VAR2": "yyy"}
             }
         expected_options = [
             "--git-repository", "https://git.launchpad.dev/~example/+git/snap",
             "--git-path", "master",
             "--build-file", "build-aux/Dockerfile",
-            ]
+            "--build-arg=VAR1=xxx",
+            "--build-arg=VAR2=yyy",
+        ]
         yield self.startBuild(args, expected_options)
 
         log_path = os.path.join(self.buildmanager._cachepath, "buildlog")