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

[Tom Wardill]

Handle layer symlinks in OCI builds

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.

[Colin Watson (cjwatson)]

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

[Thiago F. Pappacena (pappacena)]

LGTM

review: Approve

diff --git a/debian/changelog b/debian/changelog
index b0372ad..4bc4041 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+launchpad-buildd (196) UNRELEASED; urgency=medium
+
+  * Handle symlinks in OCI image files
+
+ -- Tom Wardill <[e-mail]>  Fri, 23 Apr 2021 15:51:11 +0100
+
 launchpad-buildd (195) bionic; urgency=medium
 
   * sbuild-package: Temporarily remove lxd group membership (LP: #1820348).
diff --git a/lpbuildd/oci.py b/lpbuildd/oci.py
index cf8d1d7..7a17939 100644
--- a/lpbuildd/oci.py
+++ b/lpbuildd/oci.py
@@ -8,6 +8,7 @@ __metaclass__ = type
 import hashlib
 import json
 import os
+import shutil
 import tarfile
 import tempfile
 
@@ -168,6 +169,7 @@ class OCIBuildManager(SnapBuildProxyMixin, DebianBuildManager):
 
         current_dir = ''
         directory_tar = None
+        symlinks = []
         try:
             # The tarfile is a stream and must be processed in order
             for file in tar:
@@ -184,6 +186,16 @@ class OCIBuildManager(SnapBuildProxyMixin, DebianBuildManager):
                         os.path.join(
                             extract_path, '{}.tar.gz'.format(file.name)),
                         'w|gz')
+                if file.issym():
+                    # symlinks can't be extracted or derefenced from a stream
+                    # as you can't seek backwards.
+                    # Work out what the symlink is referring to, then
+                    # we can deal with it later
+                    self._builder.log(
+                        "Found symlink at {} referencing {}".format(
+                            file.name, file.linkpath))
+                    symlinks.append(file)
+                    continue
                 if current_dir and file.name.endswith('layer.tar'):
                     # This is the actual layer data, we want to add it to
                     # the directory gzip
@@ -203,6 +215,24 @@ class OCIBuildManager(SnapBuildProxyMixin, DebianBuildManager):
             if directory_tar is not None:
                 directory_tar.close()
 
+        # deal with any symlinks we had
+        for symlink in symlinks:
+            # These are paths that finish in "<layer_id>/layer.tar"
+            # we want the directory name, which should always be
+            # the second component
+            source_name = os.path.join(
+                extract_path,
+                "{}.tar.gz".format(symlink.linkpath.split('/')[-2]))
+            target_name = os.path.join(
+                extract_path,
+                '{}.tar.gz'.format(symlink.name.split('/')[-2]))
+            # Do a copy to dereference the symlink
+            self._builder.log(
+                "Deferencing symlink from {} to {}".format(
+                    source_name, target_name))
+            shutil.copy(source_name, target_name)
+
+
         # We need these mapping files
         sha_directory = tempfile.mkdtemp()
         # This can change depending on the kernel options / docker package
diff --git a/lpbuildd/tests/oci_tarball.py b/lpbuildd/tests/oci_tarball.py
index 70c055b..21ec013 100644
--- a/lpbuildd/tests/oci_tarball.py
+++ b/lpbuildd/tests/oci_tarball.py
@@ -17,14 +17,18 @@ class OCITarball:
     @property
     def config(self):
         return self._makeFile(
-            {"rootfs": {"diff_ids": ["sha256:diff1", "sha256:diff2"]}},
+            {"rootfs": {"diff_ids":
+                ["sha256:diff1", "sha256:diff2", "sha256:diff3"]}},
             'config.json')
 
     @property
     def manifest(self):
         return self._makeFile(
             [{"Config": "config.json",
-              "Layers": ["layer-1/layer.tar", "layer-2/layer.tar"]}],
+              "Layers": [
+                  "layer-1/layer.tar",
+                  "layer-2/layer.tar",
+                  "layer-3/layer.tar"]}],
             'manifest.json')
 
     @property
@@ -32,16 +36,32 @@ class OCITarball:
         return self._makeFile([], 'repositories')
 
     def layer_file(self, directory, layer_name):
+        layer_directory = os.path.join(directory, layer_name)
+        os.mkdir(layer_directory)
         contents = "{}-contents".format(layer_name)
         tarinfo = tarfile.TarInfo(contents)
         tarinfo.size = len(contents)
         layer_contents = io.BytesIO(contents.encode("UTF-8"))
         layer_tar_path = os.path.join(
-            directory, '{}.tar.gz'.format(layer_name))
-        layer_tar = tarfile.open(layer_tar_path, 'w:gz')
+            layer_directory, 'layer.tar')
+        layer_tar = tarfile.open(layer_tar_path, 'w')
         layer_tar.addfile(tarinfo, layer_contents)
         layer_tar.close()
-        return layer_tar_path
+        return layer_directory
+
+    def add_symlink_layer(self, directory, source_layer, target_layer):
+        target_layer_directory = os.path.join(directory, target_layer)
+        source_layer_directory = os.path.join(directory, source_layer)
+
+        target = os.path.join(target_layer_directory, "layer.tar")
+        source = os.path.join(source_layer_directory, "layer.tar")
+
+        os.mkdir(target_layer_directory)
+        os.symlink(
+            os.path.relpath(source, target_layer_directory),
+            os.path.join(target_layer_directory, "layer.tar")
+        )
+        return target_layer_directory
 
     def build_tar_file(self):
         tar_directory = tempfile.mkdtemp()
@@ -53,7 +73,12 @@ class OCITarball:
 
         for layer_name in ['layer-1', 'layer-2']:
             layer = self.layer_file(tar_directory, layer_name)
-            tar.add(layer, arcname='{}.tar.gz'.format(layer_name))
+            tar.add(layer, arcname=layer_name)
+
+        # add a symlink for 'layer-3'
+        target_layer = self.add_symlink_layer(
+            tar_directory, 'layer-2', 'layer-3')
+        tar.add(target_layer, arcname="layer-3")
 
         tar.close()
 
diff --git a/lpbuildd/tests/test_oci.py b/lpbuildd/tests/test_oci.py
index 15a4d64..78b0dbb 100644
--- a/lpbuildd/tests/test_oci.py
+++ b/lpbuildd/tests/test_oci.py
@@ -43,7 +43,8 @@ class MockBuildManager(OCIBuildManager):
 class MockOCITarSave():
     @property
     def stdout(self):
-        return io.open(OCITarball().build_tar_file(), 'rb')
+        tar_path = OCITarball().build_tar_file()
+        return io.open(tar_path, 'rb')
 
 
 class TestOCIBuildManagerIteration(TestCase):
@@ -147,6 +148,7 @@ class TestOCIBuildManagerIteration(TestCase):
             'manifest.json',
             'layer-1.tar.gz',
             'layer-2.tar.gz',
+            'layer-3.tar.gz',
             'digests.json',
             'config.json',
             ]
@@ -167,6 +169,11 @@ class TestOCIBuildManagerIteration(TestCase):
                 "source": "",
                 "digest": "testsha",
                 "layer_id": "layer-2"
+            },
+            "sha256:diff3": {
+                "source": "",
+                "digest": "testsha",
+                "layer_id": "layer-3"
             }
         }]
         self.assertEqual(digests_expected, json.loads(digests_contents))
@@ -233,6 +240,7 @@ class TestOCIBuildManagerIteration(TestCase):
             'manifest.json',
             'layer-1.tar.gz',
             'layer-2.tar.gz',
+            'layer-3.tar.gz',
             'digests.json',
             'config.json',
             ]
@@ -253,6 +261,11 @@ class TestOCIBuildManagerIteration(TestCase):
                 "source": "",
                 "digest": "testsha",
                 "layer_id": "layer-2"
+            },
+            "sha256:diff3": {
+                "source": "",
+                "digest": "testsha",
+                "layer_id": "layer-3"
             }
         }]
         self.assertEqual(digests_expected, json.loads(digests_contents))