Merge pull request #3218 from certbot/nginx-dirty-write-only

Only write nginx config files if they've been modified

certbot-nginx/certbot_nginx/nginxparser.py

@@ -1,4 +1,5 @@
 """Very low-level nginx config parser based on pyparsing."""
+# Forked from https://github.com/fatiherikli/nginxparser (MIT Licensed)
 import copy
 import logging
 import string
@@ -81,8 +82,7 @@ class RawNginxDumper(object):
                 yield b.pop(0) # indentation
                 if not b:
                     continue
-            key = b.pop(0)
+            key, values = b.pop(0), b.pop(0)
-            values = b.pop(0)
 
             if isinstance(key, list):
                 yield "".join(key) + '{'
@@ -91,9 +91,9 @@ class RawNginxDumper(object):
                         yield line
                 yield '}'
             else:
-                if isinstance(key, str) and key.strip() == '#':
+                if isinstance(key, str) and key.strip() == '#':  # comment
                     yield key + values
-                else:
+                else:                                            # assignment
                     gap = ""
                     # Sometimes the parser has stuck some gap whitespace in here;
                     # if so rotate it into gap
@@ -163,6 +163,7 @@ class UnspacedList(list):
     def __init__(self, list_source):
         # ensure our argument is not a generator, and duplicate any sublists
         self.spaced = copy.deepcopy(list(list_source))
+        self.dirty = False
 
         # Turn self into a version of the source list that has spaces removed
         # and all sub-lists also UnspacedList()ed
@@ -198,20 +199,24 @@ class UnspacedList(list):
         item, spaced_item = self._coerce(x)
         self.spaced.insert(self._spaced_position(i), spaced_item)
         list.insert(self, i, item)
+        self.dirty = True
 
     def append(self, x):
         item, spaced_item = self._coerce(x)
         self.spaced.append(spaced_item)
         list.append(self, item)
+        self.dirty = True
 
     def extend(self, x):
         item, spaced_item = self._coerce(x)
         self.spaced.extend(spaced_item)
         list.extend(self, item)
+        self.dirty = True
 
     def __add__(self, other):
         l = copy.deepcopy(self)
         l.extend(other)
+        l.dirty = True
         return l
 
     def pop(self, _i=None):
@@ -231,16 +236,24 @@ class UnspacedList(list):
         item, spaced_item = self._coerce(value)
         self.spaced.__setitem__(self._spaced_position(i), spaced_item)
         list.__setitem__(self, i, item)
+        self.dirty = True
 
     def __delitem__(self, i):
         self.spaced.__delitem__(self._spaced_position(i))
         list.__delitem__(self, i)
+        self.dirty = True
 
-    def __deepcopy__(self, unused_memo):
+    def __deepcopy__(self, memo):
         l = UnspacedList(self[:])
-        l.spaced = copy.deepcopy(self.spaced)
+        l.spaced = copy.deepcopy(self.spaced, memo=memo)
+        l.dirty = self.dirty
         return l
 
+    def is_dirty(self):
+        """Recurse through the parse tree to figure out if any sublists are dirty"""
+        if self.dirty:
+            return True
+        return any((isinstance(x, list) and x.is_dirty() for x in self))
 
     def _spaced_position(self, idx):
         "Convert from indexes in the unspaced list to positions in the spaced one"

certbot-nginx/certbot_nginx/parser.py

@@ -205,11 +205,12 @@ class NginxParser(object):
         raise errors.NoInstallationError(
             "Could not find configuration root")
 
-    def filedump(self, ext='tmp'):
+    def filedump(self, ext='tmp', lazy=True):
         """Dumps parsed configurations into files.
 
         :param str ext: The file extension to use for the dumped files. If
             empty, this overrides the existing conf files.
+        :param bool lazy: Only write files that have been modified
 
         """
         # Best-effort atomicity is enforced above us by reverter.py
@@ -218,6 +219,8 @@ class NginxParser(object):
             if ext:
                 filename = filename + os.path.extsep + ext
             try:
+                if lazy and not tree.is_dirty():
+                    continue
                 out = nginxparser.dumps(tree)
                 logger.debug('Writing nginx conf tree to %s:\n%s', filename, out)
                 with open(filename, 'w') as _file:

certbot-nginx/certbot_nginx/tests/nginxparser_test.py

@@ -231,6 +231,17 @@ class TestUnspacedList(unittest.TestCase):
         del ul3[2]
         self.assertEqual(ul3, ["some", "things", "why", "did", "whether"])
 
+    def test_is_dirty(self):
+        self.assertEqual(False, self.ul2.is_dirty())
+        ul3 = UnspacedList([])
+        ul3.append(self.ul)
+        self.assertEqual(False, self.ul.is_dirty())
+        self.assertEqual(True, ul3.is_dirty())
+        ul4 = UnspacedList([[1], [2, 3, 4]])
+        self.assertEqual(False, ul4.is_dirty())
+        ul4[1][2] = 5
+        self.assertEqual(True, ul4.is_dirty())
+
 
 if __name__ == '__main__':
     unittest.main()  # pragma: no cover

certbot-nginx/certbot_nginx/tests/parser_test.py

@@ -66,7 +66,7 @@ class NginxParserTest(util.NginxTest):
 
     def test_filedump(self):
         nparser = parser.NginxParser(self.config_path, self.ssl_options)
-        nparser.filedump('test')
+        nparser.filedump('test', lazy=False)
         # pylint: disable=protected-access
         parsed = nparser._parse_files(nparser.abs_path(
             'sites-enabled/example.com.test'))