Merge lp:~twom/launchpad-buildd/oci-dockerfile-location-fix into lp:launchpad-buildd

Proposed by Tom Wardill
Status: Merged
Approved by: Tom Wardill
Approved revision: 421
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: lp:~twom/launchpad-buildd/oci-dockerfile-location-fix
Merge into: lp:launchpad-buildd
Diff against target: 188 lines (+75/-9)
3 files modified
debian/changelog (+6/-0)
lpbuildd/target/build_oci.py (+17/-3)
lpbuildd/target/tests/test_build_oci.py (+52/-6)
To merge this branch: bzr merge lp:~twom/launchpad-buildd/oci-dockerfile-location-fix
Reviewer Review Type Date Requested Status
Colin Watson Approve
Review via email: mp+381432@code.launchpad.net

Commit message

Dockerfile needs to be an absolute path

Description of the change

Passing the Dockerfile argument to the docker CLI requires an absolute path.
Join the path we have for the dockerfile with the build directory.

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) :
review: Needs Fixing
416. By Tom Wardill

Use cwd instead of absolute path

417. By ubuntu@buildd

Correct changelog

418. By Tom Wardill

Check for an escaping Dockerfile path

Revision history for this message
Colin Watson (cjwatson) :
review: Approve
419. By Tom Wardill

Better logic, named exception

420. By Tom Wardill

Symlink test

421. By ubuntu@buildd

Sort imports

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'debian/changelog'
--- debian/changelog 2020-03-18 14:51:35 +0000
+++ debian/changelog 2020-03-31 14:13:37 +0000
@@ -1,3 +1,9 @@
1launchpad-buildd (188) UNRELEASED; urgency=medium
2
3 * Fix cwd for OCI builds, allows Dockerfile parameter.
4
5 -- Tom Wardill <tom.wardill@canonical.com> Tue, 31 Mar 2020 10:13:58 +0000
6
1launchpad-buildd (187) xenial; urgency=medium7launchpad-buildd (187) xenial; urgency=medium
28
3 [ Colin Watson ]9 [ Colin Watson ]
410
=== modified file 'lpbuildd/target/build_oci.py'
--- lpbuildd/target/build_oci.py 2020-02-26 10:52:29 +0000
+++ lpbuildd/target/build_oci.py 2020-03-31 14:13:37 +0000
@@ -25,6 +25,10 @@
25logger = logging.getLogger(__name__)25logger = logging.getLogger(__name__)
2626
2727
28class InvalidBuildFilePath(Exception):
29 pass
30
31
28class BuildOCI(SnapBuildProxyOperationMixin, VCSOperationMixin,32class BuildOCI(SnapBuildProxyOperationMixin, VCSOperationMixin,
29 SnapStoreOperationMixin, Operation):33 SnapStoreOperationMixin, Operation):
3034
@@ -40,6 +44,7 @@
40 def __init__(self, args, parser):44 def __init__(self, args, parser):
41 super(BuildOCI, self).__init__(args, parser)45 super(BuildOCI, self).__init__(args, parser)
42 self.bin = os.path.dirname(sys.argv[0])46 self.bin = os.path.dirname(sys.argv[0])
47 self.buildd_path = os.path.join("/home/buildd", self.args.name)
4348
44 def _add_docker_engine_proxy_settings(self):49 def _add_docker_engine_proxy_settings(self):
45 """Add systemd file for docker proxy settings."""50 """Add systemd file for docker proxy settings."""
@@ -59,6 +64,14 @@
59 systemd_file.flush()64 systemd_file.flush()
60 self.backend.copy_in(systemd_file.name, file_path)65 self.backend.copy_in(systemd_file.name, file_path)
6166
67 def _check_build_file_escape(self):
68 """Check the build file path doesn't escape the build directory."""
69 build_file_path = os.path.realpath(
70 os.path.join(self.buildd_path, self.args.build_file))
71 common_path = os.path.commonprefix((build_file_path, self.buildd_path))
72 if common_path != self.buildd_path:
73 raise InvalidBuildFilePath("Invalid build file path.")
74
62 def run_build_command(self, args, env=None, **kwargs):75 def run_build_command(self, args, env=None, **kwargs):
63 """Run a build command in the target.76 """Run a build command in the target.
6477
@@ -71,7 +84,8 @@
71 full_env["SHELL"] = "/bin/sh"84 full_env["SHELL"] = "/bin/sh"
72 if env:85 if env:
73 full_env.update(env)86 full_env.update(env)
74 return self.backend.run(args, env=full_env, **kwargs)87 return self.backend.run(
88 args, cwd=self.buildd_path, env=full_env, **kwargs)
7589
76 def install(self):90 def install(self):
77 logger.info("Running install phase...")91 logger.info("Running install phase...")
@@ -106,9 +120,9 @@
106 ["--build-arg", "{}={}".format(var, self.args.proxy_url)])120 ["--build-arg", "{}={}".format(var, self.args.proxy_url)])
107 args.extend(["--tag", self.args.name])121 args.extend(["--tag", self.args.name])
108 if self.args.build_file is not None:122 if self.args.build_file is not None:
123 self._check_build_file_escape()
109 args.extend(["--file", self.args.build_file])124 args.extend(["--file", self.args.build_file])
110 buildd_path = os.path.join("/home/buildd", self.args.name)125 args.append(self.buildd_path)
111 args.append(buildd_path)
112 self.run_build_command(args)126 self.run_build_command(args)
113127
114 def run(self):128 def run(self):
115129
=== modified file 'lpbuildd/target/tests/test_build_oci.py'
--- lpbuildd/target/tests/test_build_oci.py 2020-02-26 10:52:29 +0000
+++ lpbuildd/target/tests/test_build_oci.py 2020-03-31 14:13:37 +0000
@@ -25,6 +25,7 @@
25 )25 )
2626
27from lpbuildd.target.build_oci import (27from lpbuildd.target.build_oci import (
28 InvalidBuildFilePath,
28 RETCODE_FAILURE_BUILD,29 RETCODE_FAILURE_BUILD,
29 RETCODE_FAILURE_INSTALL,30 RETCODE_FAILURE_INSTALL,
30 )31 )
@@ -83,7 +84,9 @@
83 build_oci = parse_args(args=args).operation84 build_oci = parse_args(args=args).operation
84 build_oci.run_build_command(["echo", "hello world"])85 build_oci.run_build_command(["echo", "hello world"])
85 self.assertThat(build_oci.backend.run.calls, MatchesListwise([86 self.assertThat(build_oci.backend.run.calls, MatchesListwise([
86 RanBuildCommand(["echo", "hello world"]),87 RanBuildCommand(
88 ["echo", "hello world"],
89 cwd="/home/buildd/test-image"),
87 ]))90 ]))
8891
89 def test_run_build_command_env(self):92 def test_run_build_command_env(self):
@@ -96,7 +99,10 @@
96 build_oci.run_build_command(99 build_oci.run_build_command(
97 ["echo", "hello world"], env={"FOO": "bar baz"})100 ["echo", "hello world"], env={"FOO": "bar baz"})
98 self.assertThat(build_oci.backend.run.calls, MatchesListwise([101 self.assertThat(build_oci.backend.run.calls, MatchesListwise([
99 RanBuildCommand(["echo", "hello world"], FOO="bar baz"),102 RanBuildCommand(
103 ["echo", "hello world"],
104 FOO="bar baz",
105 cwd="/home/buildd/test-image")
100 ]))106 ]))
101107
102 def test_install_bzr(self):108 def test_install_bzr(self):
@@ -296,7 +302,8 @@
296 self.assertThat(build_oci.backend.run.calls, MatchesListwise([302 self.assertThat(build_oci.backend.run.calls, MatchesListwise([
297 RanBuildCommand(303 RanBuildCommand(
298 ["docker", "build", "--no-cache", "--tag", "test-image",304 ["docker", "build", "--no-cache", "--tag", "test-image",
299 "/home/buildd/test-image"]),305 "/home/buildd/test-image"],
306 cwd="/home/buildd/test-image"),
300 ]))307 ]))
301308
302 def test_build_with_file(self):309 def test_build_with_file(self):
@@ -313,7 +320,8 @@
313 RanBuildCommand(320 RanBuildCommand(
314 ["docker", "build", "--no-cache", "--tag", "test-image",321 ["docker", "build", "--no-cache", "--tag", "test-image",
315 "--file", "build-aux/Dockerfile",322 "--file", "build-aux/Dockerfile",
316 "/home/buildd/test-image"]),323 "/home/buildd/test-image"],
324 cwd="/home/buildd/test-image"),
317 ]))325 ]))
318326
319 def test_build_proxy(self):327 def test_build_proxy(self):
@@ -331,7 +339,8 @@
331 ["docker", "build", "--no-cache",339 ["docker", "build", "--no-cache",
332 "--build-arg", "http_proxy=http://proxy.example:3128/",340 "--build-arg", "http_proxy=http://proxy.example:3128/",
333 "--build-arg", "https_proxy=http://proxy.example:3128/",341 "--build-arg", "https_proxy=http://proxy.example:3128/",
334 "--tag", "test-image", "/home/buildd/test-image"]),342 "--tag", "test-image", "/home/buildd/test-image"],
343 cwd="/home/buildd/test-image"),
335 ]))344 ]))
336345
337 def test_run_succeeds(self):346 def test_run_succeeds(self):
@@ -351,7 +360,8 @@
351 cwd="/home/buildd")),360 cwd="/home/buildd")),
352 AnyMatch(RanBuildCommand(361 AnyMatch(RanBuildCommand(
353 ["docker", "build", "--no-cache", "--tag", "test-image",362 ["docker", "build", "--no-cache", "--tag", "test-image",
354 "/home/buildd/test-image"])),363 "/home/buildd/test-image"],
364 cwd="/home/buildd/test-image")),
355 ))365 ))
356366
357 def test_run_install_fails(self):367 def test_run_install_fails(self):
@@ -405,3 +415,39 @@
405 build_oci.backend.build_path = self.useFixture(TempDir()).path415 build_oci.backend.build_path = self.useFixture(TempDir()).path
406 build_oci.backend.run = FailBuild()416 build_oci.backend.run = FailBuild()
407 self.assertEqual(RETCODE_FAILURE_BUILD, build_oci.run())417 self.assertEqual(RETCODE_FAILURE_BUILD, build_oci.run())
418
419 def test_build_with_invalid_file_path_parent(self):
420 args = [
421 "build-oci",
422 "--backend=fake", "--series=xenial", "--arch=amd64", "1",
423 "--branch", "lp:foo", "--build-file", "../build-aux/Dockerfile",
424 "test-image",
425 ]
426 build_oci = parse_args(args=args).operation
427 build_oci.backend.add_dir('/build/test-directory')
428 self.assertRaises(InvalidBuildFilePath, build_oci.build)
429
430 def test_build_with_invalid_file_path_absolute(self):
431 args = [
432 "build-oci",
433 "--backend=fake", "--series=xenial", "--arch=amd64", "1",
434 "--branch", "lp:foo", "--build-file", "/etc/Dockerfile",
435 "test-image",
436 ]
437 build_oci = parse_args(args=args).operation
438 build_oci.backend.add_dir('/build/test-directory')
439 self.assertRaises(InvalidBuildFilePath, build_oci.build)
440
441 def test_build_with_invalid_file_path_symlink(self):
442 args = [
443 "build-oci",
444 "--backend=fake", "--series=xenial", "--arch=amd64", "1",
445 "--branch", "lp:foo", "--build-file", "Dockerfile",
446 "test-image",
447 ]
448 build_oci = parse_args(args=args).operation
449 build_oci.buildd_path = self.useFixture(TempDir()).path
450 os.symlink(
451 '/etc/hosts',
452 os.path.join(build_oci.buildd_path, 'Dockerfile'))
453 self.assertRaises(InvalidBuildFilePath, build_oci.build)

Subscribers

People subscribed via source and target branches