Merge ~twom/launchpad-buildd:oci-can-be-layer-symlinks into launchpad-buildd:master

Proposed by Tom Wardill
Status: Merged
Approved by: Tom Wardill
Approved revision: 71f6e156728ae16140d91ac71d04958fbb8c170e
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~twom/launchpad-buildd:oci-can-be-layer-symlinks
Merge into: launchpad-buildd:master
Diff against target: 205 lines (+81/-7)
4 files modified
debian/changelog (+6/-0)
lpbuildd/oci.py (+30/-0)
lpbuildd/tests/oci_tarball.py (+31/-6)
lpbuildd/tests/test_oci.py (+14/-1)
Reviewer Review Type Date Requested Status
Thiago F. Pappacena (community) Approve
Colin Watson Approve
Review via email: mp+401710@code.launchpad.net

Commit message

Handle layer symlinks in OCI builds

Description of the change

Occasionally, docker can produce a saved image tarball that contains symlinks between layers.
This causes the buildd to fail to extract from the image as you can't dereference in a streamed tarfile object.

Instead, keep track of symlinks and then use the paths that are available to copy the existing file to the target place.

Also modify our test created tarball to include an example of this.

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) wrote :

I don't think I really follow this very clearly, but I also don't see any obvious mistakes in it, so :-)

Also don't forget to add an entry to debian/changelog.

review: Approve
Revision history for this message
Thiago F. Pappacena (pappacena) wrote :

LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index b0372ad..4bc4041 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,9 @@
6+launchpad-buildd (196) UNRELEASED; urgency=medium
7+
8+ * Handle symlinks in OCI image files
9+
10+ -- Tom Wardill <tom.wardill@canonical.com> Fri, 23 Apr 2021 15:51:11 +0100
11+
12 launchpad-buildd (195) bionic; urgency=medium
13
14 * sbuild-package: Temporarily remove lxd group membership (LP: #1820348).
15diff --git a/lpbuildd/oci.py b/lpbuildd/oci.py
16index cf8d1d7..7a17939 100644
17--- a/lpbuildd/oci.py
18+++ b/lpbuildd/oci.py
19@@ -8,6 +8,7 @@ __metaclass__ = type
20 import hashlib
21 import json
22 import os
23+import shutil
24 import tarfile
25 import tempfile
26
27@@ -168,6 +169,7 @@ class OCIBuildManager(SnapBuildProxyMixin, DebianBuildManager):
28
29 current_dir = ''
30 directory_tar = None
31+ symlinks = []
32 try:
33 # The tarfile is a stream and must be processed in order
34 for file in tar:
35@@ -184,6 +186,16 @@ class OCIBuildManager(SnapBuildProxyMixin, DebianBuildManager):
36 os.path.join(
37 extract_path, '{}.tar.gz'.format(file.name)),
38 'w|gz')
39+ if file.issym():
40+ # symlinks can't be extracted or derefenced from a stream
41+ # as you can't seek backwards.
42+ # Work out what the symlink is referring to, then
43+ # we can deal with it later
44+ self._builder.log(
45+ "Found symlink at {} referencing {}".format(
46+ file.name, file.linkpath))
47+ symlinks.append(file)
48+ continue
49 if current_dir and file.name.endswith('layer.tar'):
50 # This is the actual layer data, we want to add it to
51 # the directory gzip
52@@ -203,6 +215,24 @@ class OCIBuildManager(SnapBuildProxyMixin, DebianBuildManager):
53 if directory_tar is not None:
54 directory_tar.close()
55
56+ # deal with any symlinks we had
57+ for symlink in symlinks:
58+ # These are paths that finish in "<layer_id>/layer.tar"
59+ # we want the directory name, which should always be
60+ # the second component
61+ source_name = os.path.join(
62+ extract_path,
63+ "{}.tar.gz".format(symlink.linkpath.split('/')[-2]))
64+ target_name = os.path.join(
65+ extract_path,
66+ '{}.tar.gz'.format(symlink.name.split('/')[-2]))
67+ # Do a copy to dereference the symlink
68+ self._builder.log(
69+ "Deferencing symlink from {} to {}".format(
70+ source_name, target_name))
71+ shutil.copy(source_name, target_name)
72+
73+
74 # We need these mapping files
75 sha_directory = tempfile.mkdtemp()
76 # This can change depending on the kernel options / docker package
77diff --git a/lpbuildd/tests/oci_tarball.py b/lpbuildd/tests/oci_tarball.py
78index 70c055b..21ec013 100644
79--- a/lpbuildd/tests/oci_tarball.py
80+++ b/lpbuildd/tests/oci_tarball.py
81@@ -17,14 +17,18 @@ class OCITarball:
82 @property
83 def config(self):
84 return self._makeFile(
85- {"rootfs": {"diff_ids": ["sha256:diff1", "sha256:diff2"]}},
86+ {"rootfs": {"diff_ids":
87+ ["sha256:diff1", "sha256:diff2", "sha256:diff3"]}},
88 'config.json')
89
90 @property
91 def manifest(self):
92 return self._makeFile(
93 [{"Config": "config.json",
94- "Layers": ["layer-1/layer.tar", "layer-2/layer.tar"]}],
95+ "Layers": [
96+ "layer-1/layer.tar",
97+ "layer-2/layer.tar",
98+ "layer-3/layer.tar"]}],
99 'manifest.json')
100
101 @property
102@@ -32,16 +36,32 @@ class OCITarball:
103 return self._makeFile([], 'repositories')
104
105 def layer_file(self, directory, layer_name):
106+ layer_directory = os.path.join(directory, layer_name)
107+ os.mkdir(layer_directory)
108 contents = "{}-contents".format(layer_name)
109 tarinfo = tarfile.TarInfo(contents)
110 tarinfo.size = len(contents)
111 layer_contents = io.BytesIO(contents.encode("UTF-8"))
112 layer_tar_path = os.path.join(
113- directory, '{}.tar.gz'.format(layer_name))
114- layer_tar = tarfile.open(layer_tar_path, 'w:gz')
115+ layer_directory, 'layer.tar')
116+ layer_tar = tarfile.open(layer_tar_path, 'w')
117 layer_tar.addfile(tarinfo, layer_contents)
118 layer_tar.close()
119- return layer_tar_path
120+ return layer_directory
121+
122+ def add_symlink_layer(self, directory, source_layer, target_layer):
123+ target_layer_directory = os.path.join(directory, target_layer)
124+ source_layer_directory = os.path.join(directory, source_layer)
125+
126+ target = os.path.join(target_layer_directory, "layer.tar")
127+ source = os.path.join(source_layer_directory, "layer.tar")
128+
129+ os.mkdir(target_layer_directory)
130+ os.symlink(
131+ os.path.relpath(source, target_layer_directory),
132+ os.path.join(target_layer_directory, "layer.tar")
133+ )
134+ return target_layer_directory
135
136 def build_tar_file(self):
137 tar_directory = tempfile.mkdtemp()
138@@ -53,7 +73,12 @@ class OCITarball:
139
140 for layer_name in ['layer-1', 'layer-2']:
141 layer = self.layer_file(tar_directory, layer_name)
142- tar.add(layer, arcname='{}.tar.gz'.format(layer_name))
143+ tar.add(layer, arcname=layer_name)
144+
145+ # add a symlink for 'layer-3'
146+ target_layer = self.add_symlink_layer(
147+ tar_directory, 'layer-2', 'layer-3')
148+ tar.add(target_layer, arcname="layer-3")
149
150 tar.close()
151
152diff --git a/lpbuildd/tests/test_oci.py b/lpbuildd/tests/test_oci.py
153index 15a4d64..78b0dbb 100644
154--- a/lpbuildd/tests/test_oci.py
155+++ b/lpbuildd/tests/test_oci.py
156@@ -43,7 +43,8 @@ class MockBuildManager(OCIBuildManager):
157 class MockOCITarSave():
158 @property
159 def stdout(self):
160- return io.open(OCITarball().build_tar_file(), 'rb')
161+ tar_path = OCITarball().build_tar_file()
162+ return io.open(tar_path, 'rb')
163
164
165 class TestOCIBuildManagerIteration(TestCase):
166@@ -147,6 +148,7 @@ class TestOCIBuildManagerIteration(TestCase):
167 'manifest.json',
168 'layer-1.tar.gz',
169 'layer-2.tar.gz',
170+ 'layer-3.tar.gz',
171 'digests.json',
172 'config.json',
173 ]
174@@ -167,6 +169,11 @@ class TestOCIBuildManagerIteration(TestCase):
175 "source": "",
176 "digest": "testsha",
177 "layer_id": "layer-2"
178+ },
179+ "sha256:diff3": {
180+ "source": "",
181+ "digest": "testsha",
182+ "layer_id": "layer-3"
183 }
184 }]
185 self.assertEqual(digests_expected, json.loads(digests_contents))
186@@ -233,6 +240,7 @@ class TestOCIBuildManagerIteration(TestCase):
187 'manifest.json',
188 'layer-1.tar.gz',
189 'layer-2.tar.gz',
190+ 'layer-3.tar.gz',
191 'digests.json',
192 'config.json',
193 ]
194@@ -253,6 +261,11 @@ class TestOCIBuildManagerIteration(TestCase):
195 "source": "",
196 "digest": "testsha",
197 "layer_id": "layer-2"
198+ },
199+ "sha256:diff3": {
200+ "source": "",
201+ "digest": "testsha",
202+ "layer_id": "layer-3"
203 }
204 }]
205 self.assertEqual(digests_expected, json.loads(digests_contents))

Subscribers

People subscribed via source and target branches