[archive] Don't shell out for compressing the archive

As sos is now python3-only, we can avoid shelling-out to compression
utilities like `xz` or `gzip`, and instead use the method provided by
the built-in `tarfile` module.

Resolves: #2523

Signed-off-by: Jake Hunsaker <jhunsake@redhat.com>

5 files changed, 70 insertions, 62 deletions

diff --git a/sos/archive.py b/sos/archive.py
index 421f993c..4dd31d75 100644
--- a/sos/archive.py
+++ b/sos/archive.py
@@ -19,7 +19,8 @@ import stat
 from datetime import datetime
 from threading import Lock
 
-from sos.utilities import sos_get_command_output, is_executable
+from importlib.util import find_spec
+from sos.utilities import sos_get_command_output
 
 try:
     import selinux
@@ -558,17 +559,16 @@ class FileCacheArchive(Archive):
     def finalize(self, method):
         self.log_info("finalizing archive '%s' using method '%s'"
                       % (self._archive_root, method))
-        self._build_archive()
+        try:
+            res = self._build_archive(method)
+        except Exception as err:
+            self.log_error("An error occurred compressing the archive: %s"
+                           % err)
+            return self.name()
+
         self.cleanup()
         self.log_info("built archive at '%s' (size=%d)" % (self._archive_name,
                       os.stat(self._archive_name).st_size))
-        self.method = method
-        try:
-            res = self._compress()
-        except Exception as e:
-            exp_msg = "An error occurred compressing the archive: "
-            self.log_error("%s %s" % (exp_msg, e))
-            return self.name()
 
         if self.enc_opts['encrypt']:
             try:
@@ -682,44 +682,26 @@ class TarFileArchive(FileCacheArchive):
         # the limit of the underlying FileCacheArchive.
         return super(TarFileArchive, self).name_max()
 
-    def _build_archive(self):
-        tar = tarfile.open(self._archive_name, mode="w")
+    def _build_archive(self, method):
+        if method == 'auto':
+            method = 'xz' if find_spec('lzma') is not None else 'gzip'
+        _comp_mode = method.strip('ip')
+        self._archive_name = self._archive_name + ".%s" % _comp_mode
+        # tarfile does not currently have a consistent way to define comnpress
+        # level for both xz and gzip ('preset' for xz, 'compresslevel' for gz)
+        if method == 'gzip':
+            kwargs = {'compresslevel': 6}
+        else:
+            kwargs = {'preset': 3}
+        tar = tarfile.open(self._archive_name, mode="w:%s" % _comp_mode,
+                           **kwargs)
         # we need to pass the absolute path to the archive root but we
         # want the names used in the archive to be relative.
         tar.add(self._archive_root, arcname=os.path.split(self._name)[1],
                 filter=self.copy_permissions_filter)
         tar.close()
+        self._suffix += ".%s" % _comp_mode
+        return self.name()
 
-    def _compress(self):
-        methods = []
-        # Make sure that valid compression commands exist.
-        for method in ['xz', 'gzip']:
-            if is_executable(method):
-                methods.append(method)
-            else:
-                self.log_info("\"%s\" compression method unavailable" % method)
-        if self.method in methods:
-            methods = [self.method]
-
-        exp_msg = "No compression utilities found."
-        last_error = Exception(exp_msg)
-        for cmd in methods:
-            suffix = "." + cmd.replace('ip', '')
-            cmd = self._policy.get_cmd_for_compress_method(cmd, self._threads)
-            try:
-                exec_cmd = "%s %s" % (cmd, self.name())
-                r = sos_get_command_output(exec_cmd, stderr=True, timeout=0)
-
-                if r['status']:
-                    self.log_error(r['output'])
-                    raise Exception("%s exited with %s" % (exec_cmd,
-                                    r['status']))
-
-                self._suffix += suffix
-                return self.name()
-
-            except Exception as e:
-                last_error = e
-        raise last_error
 
 # vim: set et ts=4 sw=4 :
diff --git a/sos/policies/__init__.py b/sos/policies/__init__.py
index 64e0da14..fb8db1d7 100644
--- a/sos/policies/__init__.py
+++ b/sos/policies/__init__.py
@@ -261,24 +261,6 @@ any third party.
     def _get_pkg_name_for_binary(self, binary):
         return binary
 
-    def get_cmd_for_compress_method(self, method, threads):
-        """Determine the command to use for compressing the archive
-
-        :param method: The compression method/binary to use
-        :type method: ``str``
-
-        :param threads: Number of threads compression should use
-        :type threads: ``int``
-
-        :returns: Full command to use to compress the archive
-        :rtype: ``str``
-        """
-        cmd = method
-        if cmd.startswith("xz"):
-            # XZ set compression to -2 and use threads
-            cmd = "%s -2 -T%d" % (cmd, threads)
-        return cmd
-
     def get_tmp_dir(self, opt_tmp_dir):
         if not opt_tmp_dir:
             return tempfile.gettempdir()
diff --git a/tests/report_tests/compression_tests.py b/tests/report_tests/compression_tests.py
new file mode 100644
index 00000000..95fba481
--- /dev/null
+++ b/tests/report_tests/compression_tests.py
@@ -0,0 +1,35 @@
+# This file is part of the sos project: https://github.com/sosreport/sos
+#
+# This copyrighted material is made available to anyone wishing to use,
+# modify, copy, or redistribute it subject to the terms and conditions of
+# version 2 of the GNU General Public License.
+#
+# See the LICENSE file in the source distribution for further information.
+
+
+from sos_tests import StageOneReportTest
+
+
+class AutoCompressionTest(StageOneReportTest):
+    """Tests to ensure that 'auto' defaults to lzma, as it is in the standard
+    library
+
+    :avocado: tags=stageone
+    """
+
+    sos_cmd = '-o kernel -z auto'
+
+    def test_lzma_compressed(self):
+        self.assertTrue(self.archive.endswith('.tar.xz'))
+
+
+class GzipCompressionTest(StageOneReportTest):
+    """Tests to ensure that users can manually specify the use of gzip
+
+    :avocado: tags=stageone
+    """
+
+    sos_cmd = '-o kernel -z gzip'
+
+    def test_gzip_compressed(self):
+        self.assertTrue(self.archive.endswith('.tar.gz'))
diff --git a/tests/sos_tests.py b/tests/sos_tests.py
index 1279755b..4f50f800 100644
--- a/tests/sos_tests.py
+++ b/tests/sos_tests.py
@@ -503,6 +503,15 @@ class StageOneReportTest(BaseSoSReportTest):
         self.assertFileExists(self.archive)
         self.assertTrue(os.stat(self.archive).st_uid == 0)
 
+    def test_checksum_is_valid(self):
+        """Ensure that a checksum was generated, reported, and is correct
+        """
+        _chk = re.findall('sha256\t.*\n', self.cmd_output.stdout)
+        _chk = _chk[0].split('sha256\t')[1].strip()
+        assert _chk, "No checksum reported"
+        _found = process.run("sha256sum %s" % self.archive).stdout.decode().split()[0]
+        self.assertEqual(_chk, _found)
+
     def test_no_new_kmods_loaded(self):
         """Ensure that no additional kernel modules have been loaded during an
         execution of a test
diff --git a/tests/unittests/archive_tests.py b/tests/unittests/archive_tests.py
index db97feba..8af7cd53 100644
--- a/tests/unittests/archive_tests.py
+++ b/tests/unittests/archive_tests.py
@@ -27,14 +27,14 @@ class TarFileArchiveTest(unittest.TestCase):
         shutil.rmtree(self.tmpdir)
 
     def check_for_file(self, filename):
-        rtf = tarfile.open(os.path.join(self.tmpdir, 'test.tar'))
+        rtf = tarfile.open(os.path.join(self.tmpdir, 'test.tar.xz'))
         rtf.getmember(filename)
         rtf.close()
 
     def test_create(self):
         self.tf.finalize('auto')
         self.assertTrue(os.path.exists(os.path.join(self.tmpdir,
-                                                    'test.tar')))
+                                                    'test.tar.xz')))
 
     def test_add_file(self):
         self.tf.add_file('tests/unittests/ziptest')