From 7cbbf20b0c5003f9bd500249278034fae889168d Mon Sep 17 00:00:00 2001 From: Ryan Pineo Date: Fri, 12 May 2017 15:45:54 -0400 Subject: [PATCH 1/4] support version 0.12.0 of configargparse fixes #4648 (cherry picked from commit 42d07d756df0cf96c9d20b44e772858391d48384) --- certbot/tests/util_test.py | 17 +++++++++++++++++ certbot/util.py | 7 ++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/certbot/tests/util_test.py b/certbot/tests/util_test.py index 59a4f10b289..60b3089bb6c 100644 --- a/certbot/tests/util_test.py +++ b/certbot/tests/util_test.py @@ -6,6 +6,7 @@ import stat import unittest +import configargparse import mock import six from six.moves import reload_module # pylint: disable=import-error @@ -368,6 +369,22 @@ def test_help(self): pass self.assertTrue("--old-option" not in stdout.getvalue()) + def test_when_configargparse_set(self): + '''In configargparse versions < 0.12.0 ACTION_TYPES_THAT_DONT_NEED_A_VALUE is a set.''' + orig = configargparse.ACTION_TYPES_THAT_DONT_NEED_A_VALUE + configargparse.ACTION_TYPES_THAT_DONT_NEED_A_VALUE = set() + self._call("--old-option", 1) + self.assertEqual(len(configargparse.ACTION_TYPES_THAT_DONT_NEED_A_VALUE), 1) + configargparse.ACTION_TYPES_THAT_DONT_NEED_A_VALUE = orig + + def test_when_configargparse_tuple(self): + '''In configargparse versions >= 0.12.0 ACTION_TYPES_THAT_DONT_NEED_A_VALUE is a tuple.''' + orig = configargparse.ACTION_TYPES_THAT_DONT_NEED_A_VALUE + configargparse.ACTION_TYPES_THAT_DONT_NEED_A_VALUE = tuple() + self._call("--old-option", 1) + self.assertEqual(len(configargparse.ACTION_TYPES_THAT_DONT_NEED_A_VALUE), 1) + configargparse.ACTION_TYPES_THAT_DONT_NEED_A_VALUE = orig + class EnforceLeValidity(unittest.TestCase): """Test enforce_le_validity.""" diff --git a/certbot/util.py b/certbot/util.py index 1cbef7e80fe..d362b447a8b 100644 --- a/certbot/util.py +++ b/certbot/util.py @@ -476,7 +476,12 @@ 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): + 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) From 25d9fbd657b4e32b5d60588a3db21a54909d842b Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 15 May 2017 12:22:47 -0700 Subject: [PATCH 2/4] Modify special action types only once (cherry picked from commit 65f7f3e12b42673a87b8da69fab60c4ecfa218ad) --- certbot-apache/certbot_apache/configurator.py | 24 +++++++++++++++++-- .../certbot_apache/tests/configurator_test.py | 15 ++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index 2885cd0efec..13b325d7fc5 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/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("") + 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 bffbeee6aaa..75589dce569 100644 --- a/certbot-apache/certbot_apache/tests/configurator_test.py +++ b/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 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]) From b33669edacb94dc7304fe1e28a285c55ec43de0d Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Mon, 15 May 2017 22:32:12 +0300 Subject: [PATCH 3/4] Force augeas file reload to recalculate span indicies (cherry picked from commit f5b61d56bde973f791b32421f8f33d0e95d5cc7d) --- certbot-apache/certbot_apache/augeas_configurator.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/certbot-apache/certbot_apache/augeas_configurator.py b/certbot-apache/certbot_apache/augeas_configurator.py index e053d046800..444fbb76394 100644 --- a/certbot-apache/certbot_apache/augeas_configurator.py +++ b/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) From 8c313449f9028716e13039e6e47d322fab5d4cdf Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 15 May 2017 15:01:54 -0700 Subject: [PATCH 4/4] Make 42d07d7 more closely follow repo conventions (cherry picked from commit d467295d2a3f71148c4c04ace5212b4697f30168) --- certbot/tests/util_test.py | 36 +++++++++++++++++++++--------------- certbot/util.py | 5 +++-- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/certbot/tests/util_test.py b/certbot/tests/util_test.py index 60b3089bb6c..3d51a1d96bd 100644 --- a/certbot/tests/util_test.py +++ b/certbot/tests/util_test.py @@ -6,7 +6,6 @@ import stat import unittest -import configargparse import mock import six from six.moves import reload_module # pylint: disable=import-error @@ -369,21 +368,28 @@ def test_help(self): pass self.assertTrue("--old-option" not in stdout.getvalue()) - def test_when_configargparse_set(self): - '''In configargparse versions < 0.12.0 ACTION_TYPES_THAT_DONT_NEED_A_VALUE is a set.''' - orig = configargparse.ACTION_TYPES_THAT_DONT_NEED_A_VALUE - configargparse.ACTION_TYPES_THAT_DONT_NEED_A_VALUE = set() - self._call("--old-option", 1) - self.assertEqual(len(configargparse.ACTION_TYPES_THAT_DONT_NEED_A_VALUE), 1) - configargparse.ACTION_TYPES_THAT_DONT_NEED_A_VALUE = orig + def test_set_constant(self): + """Test when ACTION_TYPES_THAT_DONT_NEED_A_VALUE is a set. - def test_when_configargparse_tuple(self): - '''In configargparse versions >= 0.12.0 ACTION_TYPES_THAT_DONT_NEED_A_VALUE is a tuple.''' - orig = configargparse.ACTION_TYPES_THAT_DONT_NEED_A_VALUE - configargparse.ACTION_TYPES_THAT_DONT_NEED_A_VALUE = tuple() - self._call("--old-option", 1) - self.assertEqual(len(configargparse.ACTION_TYPES_THAT_DONT_NEED_A_VALUE), 1) - configargparse.ACTION_TYPES_THAT_DONT_NEED_A_VALUE = orig + 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): diff --git a/certbot/util.py b/certbot/util.py index d362b447a8b..43e402883a6 100644 --- a/certbot/util.py +++ b/certbot/util.py @@ -476,9 +476,10 @@ def __call__(self, unused1, unused2, unused3, option_string=None): sys.stderr.write( "Use of {0} is deprecated.\n".format(option_string)) - # In version 0.12.0 ACTION_TYPES_THAT_DONT_NEED_A_VALUE was changed from a set - # to a tuple. + # 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,)