[archive] fix add_string()/do_*_sub() regression

A change in the handling of add_string() operations in the archive
class causes the Plugin string substitution methods to fail (since
the archive was enforcing a check that the path did not already
exist - for substitutions this is always the case).

Maintain the check for content that is being copied into the
archive anew, but make the add_string() method override this and
disable the existence checks.

Signed-off-by: Bryn M. Reeves <bmr@redhat.com>

2 files changed, 12 insertions, 14 deletions

diff --git a/sos/archive.py b/sos/archive.py
index d53baf41..e153c09a 100644
--- a/sos/archive.py
+++ b/sos/archive.py
@@ -158,7 +158,7 @@ class FileCacheArchive(Archive):
             name = name.lstrip(os.sep)
         return (os.path.join(self._archive_root, name))
 
-    def _check_path(self, src, path_type, dest=None):
+    def _check_path(self, src, path_type, dest=None, force=False):
         """Check a new destination path in the archive.
 
             Since it is possible for multiple plugins to collect the same
@@ -185,6 +185,7 @@ class FileCacheArchive(Archive):
             :param src: the source path to be copied to the archive
             :param path_type: the type of object to be copied
             :param dest: an optional destination path
+            :param force: force file creation even if the path exists
             :returns: An absolute destination path if the path should be
                       copied now or `None` otherwise
         """
@@ -208,6 +209,9 @@ class FileCacheArchive(Archive):
                 stat.ISSOCK(mode)
             ])
 
+        if force:
+            return dest
+
         # Check destination path presence and type
         if os.path.exists(dest):
             # Use lstat: we care about the current object, not the referent.
@@ -274,9 +278,11 @@ class FileCacheArchive(Archive):
         with self._path_lock:
             src = dest
 
-            dest = self._check_path(dest, P_FILE)
-            if not dest:
-                return
+            # add_string() is a special case: it must always take precedence
+            # over any exixting content in the archive, since it is used by
+            # the Plugin postprocessing hooks to perform regex substitution
+            # on file content.
+            dest = self._check_path(dest, P_FILE, force=True)
 
             f = codecs.open(dest, 'w', encoding='utf-8')
             if isinstance(content, bytes):
diff --git a/tests/archive_tests.py b/tests/archive_tests.py
index 76bd912e..b4dd8d0f 100644
--- a/tests/archive_tests.py
+++ b/tests/archive_tests.py
@@ -94,21 +94,13 @@ class TarFileArchiveTest(unittest.TestCase):
         self.assertEquals('this is my content', afp.read())
 
     def test_rewrite_file(self):
-        """Test that re-writing a file does not modify the content.
-
-            In sos we do not have a use for overwriting archive content
-            in-place (it is an error if different plugins attempt to
-            store different content at the same path).
-
-            We do not enforce the content check at runtime since it
-            would be prohibitively costly: instead just verify in the
-            unit suite that the original content is preserved.
+        """Test that re-writing a file with add_string() modifies the content.
         """
         self.tf.add_string('this is my content', 'tests/string_test.txt')
         self.tf.add_string('this is my new content', 'tests/string_test.txt')
 
         afp = self.tf.open_file('tests/string_test.txt')
-        self.assertEquals('this is my content', afp.read())
+        self.assertEquals('this is my new content', afp.read())
 
     def test_make_link(self):
         self.tf.add_file('tests/ziptest')