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
1=== modified file 'debian/changelog'
2--- debian/changelog 2020-03-18 14:51:35 +0000
3+++ debian/changelog 2020-03-31 14:13:37 +0000
4@@ -1,3 +1,9 @@
5+launchpad-buildd (188) UNRELEASED; urgency=medium
6+
7+ * Fix cwd for OCI builds, allows Dockerfile parameter.
8+
9+ -- Tom Wardill <tom.wardill@canonical.com> Tue, 31 Mar 2020 10:13:58 +0000
10+
11 launchpad-buildd (187) xenial; urgency=medium
12
13 [ Colin Watson ]
14
15=== modified file 'lpbuildd/target/build_oci.py'
16--- lpbuildd/target/build_oci.py 2020-02-26 10:52:29 +0000
17+++ lpbuildd/target/build_oci.py 2020-03-31 14:13:37 +0000
18@@ -25,6 +25,10 @@
19 logger = logging.getLogger(__name__)
20
21
22+class InvalidBuildFilePath(Exception):
23+ pass
24+
25+
26 class BuildOCI(SnapBuildProxyOperationMixin, VCSOperationMixin,
27 SnapStoreOperationMixin, Operation):
28
29@@ -40,6 +44,7 @@
30 def __init__(self, args, parser):
31 super(BuildOCI, self).__init__(args, parser)
32 self.bin = os.path.dirname(sys.argv[0])
33+ self.buildd_path = os.path.join("/home/buildd", self.args.name)
34
35 def _add_docker_engine_proxy_settings(self):
36 """Add systemd file for docker proxy settings."""
37@@ -59,6 +64,14 @@
38 systemd_file.flush()
39 self.backend.copy_in(systemd_file.name, file_path)
40
41+ def _check_build_file_escape(self):
42+ """Check the build file path doesn't escape the build directory."""
43+ build_file_path = os.path.realpath(
44+ os.path.join(self.buildd_path, self.args.build_file))
45+ common_path = os.path.commonprefix((build_file_path, self.buildd_path))
46+ if common_path != self.buildd_path:
47+ raise InvalidBuildFilePath("Invalid build file path.")
48+
49 def run_build_command(self, args, env=None, **kwargs):
50 """Run a build command in the target.
51
52@@ -71,7 +84,8 @@
53 full_env["SHELL"] = "/bin/sh"
54 if env:
55 full_env.update(env)
56- return self.backend.run(args, env=full_env, **kwargs)
57+ return self.backend.run(
58+ args, cwd=self.buildd_path, env=full_env, **kwargs)
59
60 def install(self):
61 logger.info("Running install phase...")
62@@ -106,9 +120,9 @@
63 ["--build-arg", "{}={}".format(var, self.args.proxy_url)])
64 args.extend(["--tag", self.args.name])
65 if self.args.build_file is not None:
66+ self._check_build_file_escape()
67 args.extend(["--file", self.args.build_file])
68- buildd_path = os.path.join("/home/buildd", self.args.name)
69- args.append(buildd_path)
70+ args.append(self.buildd_path)
71 self.run_build_command(args)
72
73 def run(self):
74
75=== modified file 'lpbuildd/target/tests/test_build_oci.py'
76--- lpbuildd/target/tests/test_build_oci.py 2020-02-26 10:52:29 +0000
77+++ lpbuildd/target/tests/test_build_oci.py 2020-03-31 14:13:37 +0000
78@@ -25,6 +25,7 @@
79 )
80
81 from lpbuildd.target.build_oci import (
82+ InvalidBuildFilePath,
83 RETCODE_FAILURE_BUILD,
84 RETCODE_FAILURE_INSTALL,
85 )
86@@ -83,7 +84,9 @@
87 build_oci = parse_args(args=args).operation
88 build_oci.run_build_command(["echo", "hello world"])
89 self.assertThat(build_oci.backend.run.calls, MatchesListwise([
90- RanBuildCommand(["echo", "hello world"]),
91+ RanBuildCommand(
92+ ["echo", "hello world"],
93+ cwd="/home/buildd/test-image"),
94 ]))
95
96 def test_run_build_command_env(self):
97@@ -96,7 +99,10 @@
98 build_oci.run_build_command(
99 ["echo", "hello world"], env={"FOO": "bar baz"})
100 self.assertThat(build_oci.backend.run.calls, MatchesListwise([
101- RanBuildCommand(["echo", "hello world"], FOO="bar baz"),
102+ RanBuildCommand(
103+ ["echo", "hello world"],
104+ FOO="bar baz",
105+ cwd="/home/buildd/test-image")
106 ]))
107
108 def test_install_bzr(self):
109@@ -296,7 +302,8 @@
110 self.assertThat(build_oci.backend.run.calls, MatchesListwise([
111 RanBuildCommand(
112 ["docker", "build", "--no-cache", "--tag", "test-image",
113- "/home/buildd/test-image"]),
114+ "/home/buildd/test-image"],
115+ cwd="/home/buildd/test-image"),
116 ]))
117
118 def test_build_with_file(self):
119@@ -313,7 +320,8 @@
120 RanBuildCommand(
121 ["docker", "build", "--no-cache", "--tag", "test-image",
122 "--file", "build-aux/Dockerfile",
123- "/home/buildd/test-image"]),
124+ "/home/buildd/test-image"],
125+ cwd="/home/buildd/test-image"),
126 ]))
127
128 def test_build_proxy(self):
129@@ -331,7 +339,8 @@
130 ["docker", "build", "--no-cache",
131 "--build-arg", "http_proxy=http://proxy.example:3128/",
132 "--build-arg", "https_proxy=http://proxy.example:3128/",
133- "--tag", "test-image", "/home/buildd/test-image"]),
134+ "--tag", "test-image", "/home/buildd/test-image"],
135+ cwd="/home/buildd/test-image"),
136 ]))
137
138 def test_run_succeeds(self):
139@@ -351,7 +360,8 @@
140 cwd="/home/buildd")),
141 AnyMatch(RanBuildCommand(
142 ["docker", "build", "--no-cache", "--tag", "test-image",
143- "/home/buildd/test-image"])),
144+ "/home/buildd/test-image"],
145+ cwd="/home/buildd/test-image")),
146 ))
147
148 def test_run_install_fails(self):
149@@ -405,3 +415,39 @@
150 build_oci.backend.build_path = self.useFixture(TempDir()).path
151 build_oci.backend.run = FailBuild()
152 self.assertEqual(RETCODE_FAILURE_BUILD, build_oci.run())
153+
154+ def test_build_with_invalid_file_path_parent(self):
155+ args = [
156+ "build-oci",
157+ "--backend=fake", "--series=xenial", "--arch=amd64", "1",
158+ "--branch", "lp:foo", "--build-file", "../build-aux/Dockerfile",
159+ "test-image",
160+ ]
161+ build_oci = parse_args(args=args).operation
162+ build_oci.backend.add_dir('/build/test-directory')
163+ self.assertRaises(InvalidBuildFilePath, build_oci.build)
164+
165+ def test_build_with_invalid_file_path_absolute(self):
166+ args = [
167+ "build-oci",
168+ "--backend=fake", "--series=xenial", "--arch=amd64", "1",
169+ "--branch", "lp:foo", "--build-file", "/etc/Dockerfile",
170+ "test-image",
171+ ]
172+ build_oci = parse_args(args=args).operation
173+ build_oci.backend.add_dir('/build/test-directory')
174+ self.assertRaises(InvalidBuildFilePath, build_oci.build)
175+
176+ def test_build_with_invalid_file_path_symlink(self):
177+ args = [
178+ "build-oci",
179+ "--backend=fake", "--series=xenial", "--arch=amd64", "1",
180+ "--branch", "lp:foo", "--build-file", "Dockerfile",
181+ "test-image",
182+ ]
183+ build_oci = parse_args(args=args).operation
184+ build_oci.buildd_path = self.useFixture(TempDir()).path
185+ os.symlink(
186+ '/etc/hosts',
187+ os.path.join(build_oci.buildd_path, 'Dockerfile'))
188+ self.assertRaises(InvalidBuildFilePath, build_oci.build)

Subscribers

People subscribed via source and target branches