When extracting, return a list of strings

* Use the new "as_events" arg to get a list of ExtractEvents. But this
  is no longer the default.

Signed-off-by: Philippe Ombredanne <[email protected]>

Author: pombredanne

CHANGELOG.rst

@@ -7,11 +7,16 @@ v32.0.0
 This is a minor release with bug fixes and an output change.
 
 - We no longer support Python 3.6, only 3.7 and up.
+
 - "utils.extract_tar" function now behaves correctly with links and return
-  ExtractEvent to track extraction errors and warnings.
-  This replaces the simpler list of error messages.
+  either a list of error message strings (the previous default) but with updated
+  messages or a list of ExtractEvent to better track extraction errors and warnings.
+  The behavious is driven by the "as_events" argument.
+
 - In all places where extract is callable (Image, Layer) there is a new
-  skip_symlinks argument defaulting to True. If True, we skip symlinks and links.
+  "skip_symlinks" argument defaulting to True. If True, we skip symlinks and links.
+  The same applies with the "as_events" available in these places as these
+  functions now return a list (rather than nothing before).
 
 
 v31.1.0

src/container_inspector/image.py

@@ -356,19 +356,20 @@ def bottom_layer(self):
         """
         return self.layers[0]
 
-    def extract_layers(self, extracted_location, skip_symlinks=True):
+    def extract_layers(self, extracted_location, as_events=False, skip_symlinks=True):
         """
         Extract all layer archives to the `extracted_location` directory.
         Each layer is extracted to its own directory named after its `layer_id`.
         Skip symlinks and links if ``skip_symlinks`` is True.
-        Return a list of ExtractEvent.
+        Return a list of ExtractEvent if ``as_events`` is True or a list of message strings otherwise.
         """
         all_events = []
         for layer in self.layers:
             exloc = os.path.join(extracted_location, layer.layer_id)
             events = layer.extract(
                 extracted_location=exloc,
                 skip_symlinks=skip_symlinks,
+                as_events=as_events,
             )
             all_events.extend(events)
         return events
@@ -458,17 +459,18 @@ def get_installed_packages(self, packages_getter):
                 yield purl, package, layer
 
     @staticmethod
-    def extract(archive_location, extracted_location, skip_symlinks=False):
+    def extract(archive_location, extracted_location, as_events=False, skip_symlinks=False):
         """
         Extract the image archive tarball at ``archive_location`` to
         ``extracted_location``.
         Skip symlinks and links if ``skip_symlinks`` is True.
-        Return a list of ExtractEvent.
+        Return a list of ExtractEvent if ``as_events`` is True or a list of message strings otherwise.
         """
         return utils.extract_tar(
             location=archive_location,
             target_dir=extracted_location,
             skip_symlinks=skip_symlinks,
+            as_events=as_events,
         )
 
     @staticmethod
@@ -500,6 +502,10 @@ def get_images_from_tarball(
             extracted_location=extracted_location,
             skip_symlinks=skip_symlinks,
         )
+        if TRACE:
+            logger.debug(f'get_images_from_tarball: events')
+            for e in _events:
+                logger.debug(str(e))
 
         return Image.get_images_from_dir(
             extracted_location=extracted_location,
@@ -1086,18 +1092,19 @@ def __attrs_post_init__(self, *args, **kwargs):
         if not self.size:
             self.size = os.path.getsize(self.archive_location)
 
-    def extract(self, extracted_location, skip_symlinks=False):
+    def extract(self, extracted_location, as_events=False, skip_symlinks=False):
         """
         Extract this layer archive in the `extracted_location` directory and set
         this Layer ``extracted_location`` attribute to ``extracted_location``.
         Skip symlinks and links if ``skip_symlinks`` is True.
-        Return a list of ExtractEvent.
+        Return a list of ExtractEvent if ``as_events`` is True or a list of message strings otherwise.
         """
         self.extracted_location = extracted_location
         return utils.extract_tar(
             location=self.archive_location,
             target_dir=extracted_location,
             skip_symlinks=skip_symlinks,
+            as_events=as_events,
         )
 
     def get_resources(self, with_dir=False, walker=os.walk):

src/container_inspector/rootfs.py

@@ -76,13 +76,13 @@ def rebuild_rootfs(img, target_dir, skip_symlinks=True):
         # Note that we are not preserving any special file and any file permission
         extracted_loc = tempfile.mkdtemp('container_inspector-docker')
         # TODO: do not ignore extract events
-        _extract_events = layer.extract(
+        _events = layer.extract(
             extracted_location=extracted_loc,
             skip_symlinks=skip_symlinks,
         )
         if TRACE:
             logger.debug(f'  Extracted layer to: {extracted_loc} with skip_symlinks: {skip_symlinks}')
-            for ev in _extract_events:
+            for ev in _events:
                 logger.debug(f'  {ev}')
 
         # 2. find whiteouts in that layer.

src/container_inspector/utils.py

@@ -99,12 +99,16 @@ class ExtractEvent(NamedTuple):
     # even message
     message: str
 
+    def to_string(self):
+        return f"{self.type}: {self.message}"
 
-def extract_tar(location, target_dir, skip_symlinks=True, trace=TRACE):
+
+def extract_tar(location, target_dir, as_events=False, skip_symlinks=True, trace=TRACE):
     """
-    Extract a tar archive at ``location`` in the ``target_dir`` directory and
-    return a list of ExtractEvent possibly empty.
-    Skip symlinks and hardlinks if skip_symlinks is True.
+    Extract a tar archive at ``location`` in the ``target_dir`` directory.
+    Return a list of ExtractEvent is ``as_events`` is True, or a list of message
+    strings otherwise. This list can be empty. Skip symlinks and hardlinks if
+    skip_symlinks is True.
 
     Ignore special device files.
     Do not preserve the permissions and owners.
@@ -159,12 +163,13 @@ def extract_tar(location, target_dir, skip_symlinks=True, trace=TRACE):
                 events.append(ExtractEvent(type=ExtractEvent.ERROR, source=tarinfo.name, message=msg))
                 if trace:
                     logger.debug(f'extract_tar: {msg}')
-
+    if not as_events:
+        events = [e.to_string() for e in events]
     return events
 
 
-def extract_tar_with_symlinks(location, target_dir):
-    return extract_tar(location=location, target_dir=target_dir, skip_symlinks=False)
+def extract_tar_with_symlinks(location, target_dir, as_events=False):
+    return extract_tar(location=location, target_dir=target_dir, as_events=as_events, skip_symlinks=False,)
 
 
 def lower_keys(mapping):

tests/test_utils.py

@@ -50,8 +50,10 @@ def clean_events(self, extract_dir, events):
         """
         events_results = []
         for e in events:
-            ne = e._replace(source=e.source.replace(extract_dir, ''),
-                message=e.message.replace(self.test_data_dir, ''))
+            ne = e._replace(
+                source=e.source.replace(extract_dir, ''),
+                message=e.message.replace(self.test_data_dir, ''),
+            )
             events_results.append(ne._asdict())
 
         return events_results
@@ -78,7 +80,7 @@ def test_extract_tar_relative(self):
         expected = ()
         test_dir = self.get_test_loc('tar/tar_relative.tar')
         extract_dir = self.get_temp_dir()
-        events = utils.extract_tar(location=test_dir, target_dir=extract_dir)
+        events = utils.extract_tar(location=test_dir, target_dir=extract_dir, as_events=True)
         check_files(target_dir=extract_dir, expected=expected)
         events = self.clean_events(extract_dir, events)
         expected_events = [
@@ -95,14 +97,29 @@ def test_extract_tar_relative(self):
 
         assert events == expected_events
 
+    def test_extract_tar_relative_as_strings(self):
+        expected = ()
+        test_dir = self.get_test_loc('tar/tar_relative.tar')
+        extract_dir = self.get_temp_dir()
+        events = utils.extract_tar(location=test_dir, target_dir=extract_dir, as_events=False)
+        check_files(target_dir=extract_dir, expected=expected)
+
+        events = [e.replace(self.test_data_dir, '') for e in events]
+        expected_events = [
+            'warning: /tar/tar_relative.tar: skipping unsupported ../a_parent_folder.txt with relative path.',
+            'warning: /tar/tar_relative.tar: skipping unsupported ../../another_folder/b_two_root.txt with relative path.',
+            'warning: /tar/tar_relative.tar: skipping unsupported ../folder/subfolder/b_subfolder.txt with relative path.',
+            ]
+        assert events == expected_events
+
     def test_extract_tar_absolute(self):
         expected = (
             'tmp/subdir/a.txt',
             'tmp/subdir/b.txt',
         )
         test_dir = self.get_test_loc('tar/absolute_path.tar')
         extract_dir = self.get_temp_dir()
-        events = utils.extract_tar(location=test_dir, target_dir=extract_dir)
+        events = utils.extract_tar(location=test_dir, target_dir=extract_dir, as_events=True)
         check_files(target_dir=extract_dir, expected=expected)
 
         events = self.clean_events(extract_dir, events)
@@ -124,10 +141,9 @@ def test_extract_tar_not_skipping_links(self):
         test_tarball = self.get_test_loc('utils/layer_with_links.tar')
         extract_dir = self.get_temp_dir()
 
-        events = utils.extract_tar(location=test_tarball, target_dir=extract_dir, skip_symlinks=False)
+        events = utils.extract_tar(location=test_tarball, target_dir=extract_dir, as_events=True, skip_symlinks=False)
 
         results = self.clean_paths(extract_dir)
-
         expected_results = self.get_test_loc('utils/layer_with_links.tar.expected.json', must_exist=False)
         check_expected(results, expected_results, regen=False)
 
@@ -139,82 +155,41 @@ def test_extract_tar_skipping_links(self):
         test_tarball = self.get_test_loc('utils/layer_with_links.tar')
         extract_dir = self.get_temp_dir()
 
-        events = utils.extract_tar(location=test_tarball, target_dir=extract_dir, skip_symlinks=True)
-
-        results = sorted([p.replace(extract_dir, '')
-            for p in fileutils.resource_iter(
-                location=extract_dir,
-                with_dirs=True,
-                follow_symlinks=True,
-            )
-        ])
+        events = utils.extract_tar(location=test_tarball, target_dir=extract_dir, as_events=True, skip_symlinks=True)
 
+        results = self.clean_paths(extract_dir)
         expected_results = self.get_test_loc('utils/layer_with_links.tar.expected-skipping.json', must_exist=False)
         check_expected(results, expected_results, regen=False)
 
-        events_results = []
-        for e in events:
-            ne = e._replace(
-                source=e.source.replace(extract_dir, ''),
-                message=e.message.replace(self.test_data_dir, ''),
-            )
-            events_results.append(ne._asdict())
-
+        events_results = self.clean_events(extract_dir, events)
         expected_events = self.get_test_loc('utils/layer_with_links.tar.expected-events-skipping.json', must_exist=False)
         check_expected(events_results, expected_events, regen=False)
 
     def test_extract_tar_with_symlinks(self):
         test_tarball = self.get_test_loc('utils/layer_with_links.tar')
         extract_dir = self.get_temp_dir()
 
-        events = utils.extract_tar_with_symlinks(location=test_tarball, target_dir=extract_dir)
-        results = sorted([p.replace(extract_dir, '')
-            for p in fileutils.resource_iter(
-                location=extract_dir,
-                with_dirs=True,
-                follow_symlinks=True,
-            )
-        ])
+        events = utils.extract_tar_with_symlinks(location=test_tarball, as_events=True, target_dir=extract_dir)
 
+        results = self.clean_paths(extract_dir)
         expected_results = self.get_test_loc('utils/layer_with_links.tar.expected.json', must_exist=False)
         check_expected(results, expected_results, regen=False)
 
-        events_results = []
-        for e in events:
-            ne = e._replace(
-                source=e.source.replace(extract_dir, ''),
-                message=e.message.replace(self.test_data_dir, ''),
-            )
-            events_results.append(ne._asdict())
-
+        events_results = self.clean_events(extract_dir, events)
         expected_events = self.get_test_loc('utils/layer_with_links.tar.expected-events.json', must_exist=False)
         check_expected(events_results, expected_events, regen=False)
 
     def test_extract_tar_with_broken_links_skipping_links(self):
         test_tarball = self.get_test_loc('utils/layer_with_links_missing_targets.tar')
         extract_dir = self.get_temp_dir()
 
-        events = utils.extract_tar(location=test_tarball, target_dir=extract_dir, skip_symlinks=True)
-
-        results = sorted([p.replace(extract_dir, '')
-            for p in fileutils.resource_iter(
-                location=extract_dir,
-                with_dirs=True,
-                follow_symlinks=True,
-            )
-        ])
+        events = utils.extract_tar(location=test_tarball, target_dir=extract_dir, as_events=True, skip_symlinks=True)
 
+        results = self.clean_paths(extract_dir)
         expected_results = self.get_test_loc('utils/layer_with_links_missing_targets.tar.expected.json', must_exist=False)
         check_expected(results, expected_results, regen=False)
 
-        events_results = []
-        for e in events:
-            ne = e._replace(
-                source=e.source.replace(extract_dir, ''),
-                message=e.message.replace(self.test_data_dir, ''),
-            )
-            events_results.append(ne._asdict())
-
+        events_results = self.clean_events(extract_dir, events)
         expected_events = self.get_test_loc('utils/layer_with_links_missing_targets.tar.expected-events.json', must_exist=False)
         check_expected(events_results, expected_events, regen=False)
 
@@ -223,25 +198,12 @@ def test_extract_tar_with_symlinks_with_broken_links(self):
         extract_dir = self.get_temp_dir()
 
         events = utils.extract_tar_with_symlinks(location=test_tarball, target_dir=extract_dir)
-        results = sorted([p.replace(extract_dir, '')
-            for p in fileutils.resource_iter(
-                location=extract_dir,
-                with_dirs=True,
-                follow_symlinks=True,
-            )
-        ])
 
+        results = self.clean_paths(extract_dir)
         expected_results = self.get_test_loc('utils/layer_with_links_missing_targets.tar.expected-broken.json', must_exist=False)
         check_expected(results, expected_results, regen=False)
 
-        events_results = []
-        for e in events:
-            ne = e._replace(
-                source=e.source.replace(extract_dir, ''),
-                message=e.message.replace(self.test_data_dir, ''),
-            )
-            events_results.append(ne._asdict())
-
+        events_results = self.clean_events(extract_dir, events)
         expected_events = self.get_test_loc('utils/layer_with_links_missing_targets.tar.expected-events-broken.json', must_exist=False)
         check_expected(events_results, expected_events, regen=False)