Merge 8c31344 into 4be7efb

certbot-apache/certbot_apache/augeas_configurator.py

@@ -120,8 +120,8 @@ def save(self, title=None, temporary=False):
 
         # If the augeas tree didn't change, no files were saved and a backup
         # should not be created
+        save_files = set()
         if save_paths:
-            save_files = set()
             for path in save_paths:
                 save_files.add(self.aug.get(path)[6:])
 
@@ -140,6 +140,12 @@ def save(self, title=None, temporary=False):
         self.save_notes = ""
         self.aug.save()
 
+        # Force reload if files were modified
+        # This is needed to recalculate augeas directive span
+        if save_files:
+            for sf in save_files:
+                self.aug.remove("/files/"+sf)
+            self.aug.load()
         if title and not temporary:
             try:
                 self.reverter.finalize_checkpoint(title)

certbot-apache/certbot_apache/configurator.py

@@ -1065,8 +1065,28 @@ def _get_vhost_block(self, vhost):
         span_end = span_val[6]
         with open(span_filep, 'r') as fh:
             fh.seek(span_start)
-            vh_contents = fh.read(span_end-span_start)
-        return vh_contents.split("\n")
+            vh_contents = fh.read(span_end-span_start).split("\n")
+        self._remove_closing_vhost_tag(vh_contents)
+        return vh_contents
+
+    def _remove_closing_vhost_tag(self, vh_contents):
+        """Removes the closing VirtualHost tag if it exists.
+
+        This method modifies vh_contents directly to remove the closing
+        tag. If the closing vhost tag is found, everything on the line
+        after it is also removed. Whether or not this tag is included
+        in the result of span depends on the Augeas version.
+
+        :param list vh_contents: VirtualHost block contents to check
+
+        """
+        for offset, line in enumerate(reversed(vh_contents)):
+            if line:
+                line_index = line.lower().find("</virtualhost>")
+                if line_index != -1:
+                    content_index = len(vh_contents) - offset - 1
+                    vh_contents[content_index] = line[:line_index]
+                break
 
     def _update_ssl_vhosts_addrs(self, vh_path):
         ssl_addrs = set()

certbot-apache/certbot_apache/tests/configurator_test.py

@@ -597,6 +597,21 @@ def test_prepare_server_https_mixed_listen(self):
         # already listens to the correct port
         self.assertEqual(mock_add_dir.call_count, 0)
 
+    def test_make_vhost_ssl_with_mock_span(self):
+        # span excludes the closing </VirtualHost> tag in older versions
+        # of Augeas
+        return_value = [self.vh_truth[0].filep, 1, 12, 0, 0, 0, 1142]
+        with mock.patch.object(self.config.aug, 'span') as mock_span:
+            mock_span.return_value = return_value
+            self.test_make_vhost_ssl()
+
+    def test_make_vhost_ssl_with_mock_span2(self):
+        # span includes the closing </VirtualHost> tag in newer versions
+        # of Augeas
+        return_value = [self.vh_truth[0].filep, 1, 12, 0, 0, 0, 1157]
+        with mock.patch.object(self.config.aug, 'span') as mock_span:
+            mock_span.return_value = return_value
+            self.test_make_vhost_ssl()
 
     def test_make_vhost_ssl(self):
         ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[0])

certbot/tests/util_test.py

@@ -368,6 +368,29 @@ def test_help(self):
                 pass
         self.assertTrue("--old-option" not in stdout.getvalue())
 
+    def test_set_constant(self):
+        """Test when ACTION_TYPES_THAT_DONT_NEED_A_VALUE is a set.
+
+        This variable is a set in configargparse versions < 0.12.0.
+
+        """
+        self._test_constant_common(set)
+
+    def test_tuple_constant(self):
+        """Test when ACTION_TYPES_THAT_DONT_NEED_A_VALUE is a tuple.
+
+        This variable is a tuple in configargparse versions >= 0.12.0.
+
+        """
+        self._test_constant_common(tuple)
+
+    def _test_constant_common(self, typ):
+        with mock.patch("certbot.util.configargparse") as mock_configargparse:
+            mock_configargparse.ACTION_TYPES_THAT_DONT_NEED_A_VALUE = typ()
+            self._call("--old-option", 1)
+        self.assertEqual(
+            len(mock_configargparse.ACTION_TYPES_THAT_DONT_NEED_A_VALUE), 1)
+
 
 class EnforceLeValidity(unittest.TestCase):
     """Test enforce_le_validity."""

certbot/util.py

@@ -476,7 +476,13 @@ def __call__(self, unused1, unused2, unused3, option_string=None):
             sys.stderr.write(
                 "Use of {0} is deprecated.\n".format(option_string))
 
-    configargparse.ACTION_TYPES_THAT_DONT_NEED_A_VALUE.add(ShowWarning)
+    # In version 0.12.0 ACTION_TYPES_THAT_DONT_NEED_A_VALUE was changed from a
+    # set to a tuple.
+    if isinstance(configargparse.ACTION_TYPES_THAT_DONT_NEED_A_VALUE, set):
+        # pylint: disable=no-member
+        configargparse.ACTION_TYPES_THAT_DONT_NEED_A_VALUE.add(ShowWarning)
+    else:
+        configargparse.ACTION_TYPES_THAT_DONT_NEED_A_VALUE += (ShowWarning,)
     add_argument(argument_name, action=ShowWarning,
                  help=argparse.SUPPRESS, nargs=nargs)