[Plugin] Improve add_string_as_file collection and manifest recording

This commit allows plugins that call `add_string_as_file` to specify if
the string should be written to `sos_strings/$name/` (current behavior)
or if it should be written to `sos_commands/$name/` which may be
desireable for organizational purposes for plugin collections.

`add_string_as_file()` has also been updated to write to a plugin's
manifest section for any files written this way. Accordingly, the method
will now accept a `tags` parameter to add specified tags to the manifest
entry.

Certain plugins directly calling this method have been updated, but the
existing logic to write truncated data to `sos_strings/` remains
untouched, and will not generate manifest entries (as those should
already be handled by the method that trigged the truncated collection).

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

9 files changed, 79 insertions, 20 deletions

diff --git a/sos/report/__init__.py b/sos/report/__init__.py
index b033f621..e35c7e8d 100644
--- a/sos/report/__init__.py
+++ b/sos/report/__init__.py
@@ -1162,13 +1162,9 @@ class SoSReport(SoSComponent):
                                         cmd['file']
                                     )))
 
-            for content, f in plug.copy_strings:
+            for content, f, tags in plug.copy_strings:
                 section.add(CreatedFile(name=f,
-                                        href=os.path.join(
-                                            "..",
-                                            "sos_strings",
-                                            plugname,
-                                            f)))
+                                        href=os.path.join("..", f)))
 
             report.add(section)
 
diff --git a/sos/report/plugins/__init__.py b/sos/report/plugins/__init__.py
index 6c32c665..3c2b64d9 100644
--- a/sos/report/plugins/__init__.py
+++ b/sos/report/plugins/__init__.py
@@ -613,6 +613,7 @@ class Plugin():
         self.manifest.add_field('timeout_hit', False)
         self.manifest.add_list('commands', [])
         self.manifest.add_list('files', [])
+        self.manifest.add_field('strings', {})
 
     def timeout_from_options(self, optname, plugoptname, default_timeout):
         """Returns either the default [plugin|cmd] timeout value, the value as
@@ -1919,7 +1920,8 @@ class Plugin():
             # adds a mixed case variable name, still get that as well
             self._env_vars.update([env, env.upper(), env.lower()])
 
-    def add_string_as_file(self, content, filename, pred=None):
+    def add_string_as_file(self, content, filename, pred=None, plug_dir=False,
+                           tags=[]):
         """Add a string to the archive as a file
 
         :param content: The string to write to the archive
@@ -1931,6 +1933,14 @@ class Plugin():
         :param pred: A predicate to gate if the string should be added to the
                      archive or not
         :type pred: ``SoSPredicate``
+
+        :param plug_dir: Should the string be saved under the plugin's dir in
+                         sos_commands/? If false, save to sos_strings/
+        :type plug_dir: ``bool`
+
+        :param tags: A tag or set of tags to add to the manifest entry for this
+                     collection
+        :type tags: ``str`` or a ``list`` of strings
         """
 
         # Generate summary string for logging
@@ -1943,7 +1953,13 @@ class Plugin():
                            (summary, self.get_predicate(pred=pred)))
             return
 
-        self.copy_strings.append((content, filename))
+        sos_dir = 'sos_commands' if plug_dir else 'sos_strings'
+        filename = os.path.join(sos_dir, self.name(), filename)
+
+        if isinstance(tags, str):
+            tags = [tags]
+
+        self.copy_strings.append((content, filename, tags))
         self._log_debug("added string ...'%s' as '%s'" % (summary, filename))
 
     def _collect_cmd_output(self, cmd, suggest_filename=None,
@@ -2622,7 +2638,7 @@ class Plugin():
             self._collect_cmd_output(**soscmd.__dict__)
 
     def _collect_strings(self):
-        for string, file_name in self.copy_strings:
+        for string, file_name, tags in self.copy_strings:
             if self._timeout_hit:
                 return
             content = ''
@@ -2633,10 +2649,12 @@ class Plugin():
             self._log_info("collecting string ...'%s' as '%s'"
                            % (content, file_name))
             try:
-                self.archive.add_string(string,
-                                        os.path.join('sos_strings',
-                                                     self.name(),
-                                                     file_name))
+                self.archive.add_string(string, file_name)
+                _name = file_name.split('/')[-1].replace('.', '_')
+                self.manifest.strings[_name] = {
+                    'path': file_name,
+                    'tags': tags
+                }
             except Exception as e:
                 self._log_debug("could not add string '%s': %s"
                                 % (file_name, e))
diff --git a/sos/report/plugins/jars.py b/sos/report/plugins/jars.py
index f2a26679..0d3cf37e 100644
--- a/sos/report/plugins/jars.py
+++ b/sos/report/plugins/jars.py
@@ -79,7 +79,7 @@ class Jars(Plugin, RedHatPlugin):
                 results["jars"].append(record)
 
         results_str = json.dumps(results, indent=4, separators=(",", ": "))
-        self.add_string_as_file(results_str, "jars.json")
+        self.add_string_as_file(results_str, "jars.json", plug_dir=True)
 
     @staticmethod
     def is_jar(path):
diff --git a/sos/report/plugins/python.py b/sos/report/plugins/python.py
index d5416f28..e2ab39ab 100644
--- a/sos/report/plugins/python.py
+++ b/sos/report/plugins/python.py
@@ -95,6 +95,7 @@ class RedHatPython(Python, RedHatPlugin):
                                     filepath
                                 )
 
-            self.add_string_as_file(json.dumps(digests), 'digests.json')
+            self.add_string_as_file(json.dumps(digests), 'digests.json',
+                                    plug_dir=True)
 
 # vim: set et ts=4 sw=4 :
diff --git a/sos/report/plugins/unpackaged.py b/sos/report/plugins/unpackaged.py
index 9d68077c..9205e53f 100644
--- a/sos/report/plugins/unpackaged.py
+++ b/sos/report/plugins/unpackaged.py
@@ -80,6 +80,7 @@ class Unpackaged(Plugin, RedHatPlugin):
             all_fsystem += all_files_system(d)
         not_packaged = [x for x in all_fsystem if x not in all_frpm]
         not_packaged_expanded = format_output(not_packaged)
-        self.add_string_as_file('\n'.join(not_packaged_expanded), 'unpackaged')
+        self.add_string_as_file('\n'.join(not_packaged_expanded), 'unpackaged',
+                                plug_dir=True)
 
 # vim: set et ts=4 sw=4 :
diff --git a/sos/report/plugins/yum.py b/sos/report/plugins/yum.py
index d551573e..148464cb 100644
--- a/sos/report/plugins/yum.py
+++ b/sos/report/plugins/yum.py
@@ -69,7 +69,8 @@ class Yum(Plugin, RedHatPlugin):
                     os.path.basename(p)[:-3] for p in plugins.split()
                 ]
                 plugnames = "%s\n" % "\n".join(plugnames)
-                self.add_string_as_file(plugnames, "plugin-names")
+                self.add_string_as_file(plugnames, "plugin-names",
+                                        plug_dir=True)
 
         self.add_copy_spec("/etc/yum/pluginconf.d")
 
diff --git a/tests/report_tests/plugin_tests/logs.py b/tests/report_tests/plugin_tests/logs.py
index 9512b0d1..14f6bd5d 100644
--- a/tests/report_tests/plugin_tests/logs.py
+++ b/tests/report_tests/plugin_tests/logs.py
@@ -74,3 +74,9 @@ class LogsSizeLimitTest(StageTwoReportTest):
         self.assertFileExists(tailed)
         journ = self.get_name_in_archive('sos_commands/logs/journalctl_--no-pager')
         assert os.path.islink(journ), "Journal in sos_commands/logs is not a symlink"
+
+    def test_string_not_in_manifest(self):
+        # we don't want truncated collections appearing in the strings section
+        # of the manifest for the plugin
+        manifest = self.get_plugin_manifest('logs')
+        self.assertFalse(manifest['strings'])
diff --git a/tests/report_tests/plugin_tests/string_collection_tests.py b/tests/report_tests/plugin_tests/string_collection_tests.py
new file mode 100644
index 00000000..d98401b3
--- /dev/null
+++ b/tests/report_tests/plugin_tests/string_collection_tests.py
@@ -0,0 +1,37 @@
+# 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 CollectStringTest(StageOneReportTest):
+    """Test to ensure that add_string_as_file() is working for plugins that
+    directly call it as part of their collections
+
+    :avocado: tags=stageone
+    """
+
+    sos_cmd = '-v -o unpackaged,python -k python.hashes'
+    # unpackaged is only a RedHatPlugin
+    redhat_only = True
+
+    def test_unpackaged_list_collected(self):
+        self.assertFileCollected('sos_commands/unpackaged/unpackaged')
+
+    def test_python_hashes_collected(self):
+        self.assertFileCollected('sos_commands/python/digests.json')
+
+    def test_no_strings_dir(self):
+        self.assertFileNotCollected('sos_strings/')
+
+    def test_manifest_strings_correct(self):
+        pkgman = self.get_plugin_manifest('unpackaged')
+        self.assertTrue(pkgman['strings']['unpackaged'])
+        pyman = self.get_plugin_manifest('python')
+        self.assertTrue(pyman['strings']['digests_json'])
diff --git a/tests/unittests/plugin_tests.py b/tests/unittests/plugin_tests.py
index fcd24143..0105e3b8 100644
--- a/tests/unittests/plugin_tests.py
+++ b/tests/unittests/plugin_tests.py
@@ -339,10 +339,9 @@ class AddCopySpecTests(unittest.TestCase):
         self.mp.sysroot = '/'
         fn = create_file(2)  # create 2MB file, consider a context manager
         self.mp.add_copy_spec(fn, 1)
-        content, fname = self.mp.copy_strings[0]
+        content, fname, _tags = self.mp.copy_strings[0]
         self.assertTrue("tailed" in fname)
         self.assertTrue("tmp" in fname)
-        self.assertTrue("/" not in fname)
         self.assertEquals(1024 * 1024, len(content))
         os.unlink(fn)
 
@@ -371,7 +370,7 @@ class AddCopySpecTests(unittest.TestCase):
         create_file(2, dir=tmpdir)
         self.mp.add_copy_spec(tmpdir + "/*", 1)
         self.assertEquals(len(self.mp.copy_strings), 1)
-        content, fname = self.mp.copy_strings[0]
+        content, fname, _tags = self.mp.copy_strings[0]
         self.assertTrue("tailed" in fname)
         self.assertEquals(1024 * 1024, len(content))
         shutil.rmtree(tmpdir)