launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #25165
[Merge] ~twom/launchpad-buildd:oci-build-path into launchpad-buildd:master
Tom Wardill has proposed merging ~twom/launchpad-buildd:oci-build-path into launchpad-buildd:master.
Commit message:
Add build path handling to OCI builds
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~twom/launchpad-buildd/+git/launchpad-buildd/+merge/389258
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.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~twom/launchpad-buildd:oci-build-path into launchpad-buildd:master.
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)
