diff --git a/certbot-apache/certbot_apache/augeas_configurator.py b/certbot-apache/certbot_apache/augeas_configurator.py index e053d0468..444fbb763 100644 --- a/certbot-apache/certbot_apache/augeas_configurator.py +++ b/certbot-apache/certbot_apache/augeas_configurator.py @@ -120,8 +120,8 @@ class AugeasConfigurator(common.Plugin): # 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 @@ class AugeasConfigurator(common.Plugin): 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) diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index 2885cd0ef..13b325d7f 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/certbot-apache/certbot_apache/configurator.py @@ -1065,8 +1065,28 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): 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("") + 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() diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py index bffbeee6a..75589dce5 100644 --- a/certbot-apache/certbot_apache/tests/configurator_test.py +++ b/certbot-apache/certbot_apache/tests/configurator_test.py @@ -597,6 +597,21 @@ class MultipleVhostsTest(util.ApacheTest): # 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 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 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]) diff --git a/certbot/tests/util_test.py b/certbot/tests/util_test.py index 59a4f10b2..3d51a1d96 100644 --- a/certbot/tests/util_test.py +++ b/certbot/tests/util_test.py @@ -368,6 +368,29 @@ class AddDeprecatedArgumentTest(unittest.TestCase): 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.""" diff --git a/certbot/util.py b/certbot/util.py index 1cbef7e80..43e402883 100644 --- a/certbot/util.py +++ b/certbot/util.py @@ -476,7 +476,13 @@ def add_deprecated_argument(add_argument, argument_name, nargs): 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)