Rework tar normalization to a shared function

Add missing tests

Change-Id: I91b42fe530f87af7d78bb1a543b29a4b0d50872e
Signed-off-by: Michael Sherman <shermanm@uchicago.edu>

2 files changed, 82 insertions, 24 deletions

diff --git a/kolla/image/tasks.py b/kolla/image/tasks.py
index f8d07e221..9eec24a8f 100644
--- a/kolla/image/tasks.py
+++ b/kolla/image/tasks.py
@@ -44,6 +44,21 @@ class ArchivingError(Exception):
     pass
 
 
+def normalize_tarinfo(tarinfo):
+    # NOTE(mgoddard): Change ownership of files to root:root. This
+    # avoids an issue introduced by the fix for git CVE-2022-24765,
+    # which breaks PBR when the source checkout is not owned by the
+    # user installing it. LP#1969096
+    tarinfo.uid = tarinfo.gid = 0
+    tarinfo.uname = tarinfo.gname = "root"
+    # Pin mtime so archive bytes do not depend on checkout/extraction
+    # time, keeping the engine's layer cache valid when the content is
+    # unchanged. An integer mtime also stops tarfile's PAX writer
+    # emitting per-entry float-mtime extended headers.
+    tarinfo.mtime = 0
+    return tarinfo
+
+
 class EngineTask(task.Task):
     def __init__(self, conf):
         super(EngineTask, self).__init__()
@@ -194,16 +209,6 @@ class BuildTask(EngineTask):
 
         dest_archive = os.path.join(image.path, source['name'] + '-archive')
 
-        # NOTE(mgoddard): Change ownership of files to root:root. This
-        # avoids an issue introduced by the fix for git CVE-2022-24765,
-        # which breaks PBR when the source checkout is not owned by the
-        # user installing it. LP#1969096
-        def reset_userinfo(tarinfo):
-            tarinfo.uid = tarinfo.gid = 0
-            tarinfo.uname = tarinfo.gname = "root"
-            tarinfo.mtime = 0
-            return tarinfo
-
         if source.get('type') == 'url':
             self.logger.debug("Getting archive from %s", source['source'])
             try:
@@ -263,7 +268,7 @@ class BuildTask(EngineTask):
 
             with tarfile.open(dest_archive, 'w') as tar:
                 tar.add(clone_dir, arcname=os.path.basename(clone_dir),
-                        filter=reset_userinfo)
+                        filter=normalize_tarinfo)
 
         elif source.get('type') == 'local':
             self.logger.debug("Getting local archive from %s",
@@ -272,7 +277,7 @@ class BuildTask(EngineTask):
                 with tarfile.open(dest_archive, 'w') as tar:
                     tar.add(source['source'],
                             arcname=os.path.basename(source['source']),
-                            filter=reset_userinfo)
+                            filter=normalize_tarinfo)
             else:
                 shutil.copyfile(source['source'], dest_archive)
 
@@ -349,19 +354,10 @@ class BuildTask(EngineTask):
                         raise ArchivingError
             arc_path = os.path.join(image.path, '%s-archive' % arcname)
 
-            # NOTE(jneumann): Change ownership of files to root:root. This
-            # avoids an issue introduced by the fix for git CVE-2022-24765,
-            # which breaks PBR when the source checkout is not owned by the
-            # user installing it. LP#1969096
-            def reset_userinfo(tarinfo):
-                tarinfo.uid = tarinfo.gid = 0
-                tarinfo.uname = tarinfo.gname = "root"
-                tarinfo.mtime = 0
-                return tarinfo
-
             with tarfile.open(arc_path, 'w') as tar:
-                tar.add(items_path, arcname=arcname, filter=reset_userinfo)
-            os.utime(arc_path, (0, 0))
+                tar.add(items_path, arcname=arcname,
+                        filter=normalize_tarinfo)
+
             return len(os.listdir(items_path))
 
         self.logger.debug('Processing')
diff --git a/kolla/tests/test_build.py b/kolla/tests/test_build.py
index e4e4d2839..21f7fc59c 100644
--- a/kolla/tests/test_build.py
+++ b/kolla/tests/test_build.py
@@ -12,6 +12,7 @@
 
 import datetime
 import fixtures
+import io
 import jinja2
 import os
 import requests
@@ -370,6 +371,67 @@ class TasksTest(base.TestCase):
 
     @mock.patch.dict(os.environ, clear=True)
     @mock.patch(engine_client)
+    def test_local_directory_archive_reproducible(self, mock_client):
+        src_dir = self.useFixture(fixtures.TempDir()).path
+        sub_dir = os.path.join(src_dir, 'sub')
+        os.mkdir(sub_dir)
+        file_path = os.path.join(sub_dir, 'test.txt')
+        with open(file_path, 'w') as f:
+            f.write('Hello')
+
+        source = {'name': 'fake-image-base',
+                  'type': 'local',
+                  'enabled': True,
+                  'source': src_dir}
+        push_queue = mock.Mock()
+
+        archives = []
+        for mtime in (1000000000, 2000000000):
+            os.utime(file_path, (mtime, mtime))
+            os.utime(sub_dir, (mtime, mtime))
+            os.utime(src_dir, (mtime, mtime))
+            image = FAKE_IMAGE.copy()
+            image.path = self.useFixture(fixtures.TempDir()).path
+            builder = tasks.BuildTask(self.conf, image, push_queue)
+            dest_archive = builder.process_source(image, source)
+            self.assertIsNotNone(dest_archive)
+            with open(dest_archive, 'rb') as f:
+                archives.append(f.read())
+
+        self.assertEqual(archives[0], archives[1])
+        with tarfile.open(fileobj=io.BytesIO(archives[0])) as tar:
+            for member in tar.getmembers():
+                self.assertEqual(0, member.mtime)
+                self.assertEqual(0, member.uid)
+                self.assertEqual(0, member.gid)
+                self.assertEqual('root', member.uname)
+                self.assertEqual('root', member.gname)
+
+    @mock.patch.dict(os.environ, clear=True)
+    @mock.patch(engine_client)
+    def test_empty_plugins_archive_reproducible(self, mock_client):
+        self.dc = mock_client
+        push_queue = mock.Mock()
+
+        archives = []
+        for _ in range(2):
+            image = FAKE_IMAGE.copy()
+            image.path = self.useFixture(fixtures.TempDir()).path
+            builder = tasks.BuildTask(self.conf, image, push_queue)
+            builder.run()
+            self.assertTrue(builder.success)
+            with open(os.path.join(image.path, 'plugins-archive'),
+                      'rb') as f:
+                archives.append(f.read())
+
+        self.assertEqual(archives[0], archives[1])
+        with tarfile.open(fileobj=io.BytesIO(archives[0])) as tar:
+            members = tar.getmembers()
+            self.assertEqual(['plugins'], [m.name for m in members])
+            self.assertEqual(0, members[0].mtime)
+
+    @mock.patch.dict(os.environ, clear=True)
+    @mock.patch(engine_client)
     def test_malicious_tar(self, mock_client):
         tmpdir = tempfile.mkdtemp()
         file_name = 'test.txt'