diff --git a/CHANGELOG.md b/CHANGELOG.md index 3cc3ff41f..8669d34a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,12 +16,14 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). ### Fixed -* +* Renewal parameter `webroot_path` is always saved, avoiding some regressions + when `webroot` authenticator plugin is invoked with no challenge to perform. Despite us having broken lockstep, we are continuing to release new versions of all Certbot components during releases for the time being, however, the only package with changes other than its version number was: +* certbot * certbot-dns-rfc2136 More details about these changes can be found on our GitHub repo. diff --git a/certbot/plugins/webroot_test.py b/certbot/plugins/webroot_test.py index 3a902b91f..bca1045d8 100644 --- a/certbot/plugins/webroot_test.py +++ b/certbot/plugins/webroot_test.py @@ -295,6 +295,19 @@ class WebrootActionTest(unittest.TestCase): self.assertEqual( config.webroot_map[self.achall.domain], self.path) + def test_webroot_map_partial_without_perform(self): + # This test acknowledges the fact that webroot_map content will be partial if webroot + # plugin perform method is not invoked (corner case when all auths are already valid). + # To not be a problem, the webroot_path must always been conserved during renew. + # This condition is challenged by: + # certbot.tests.renewal_tests::RenewalTest::test_webroot_params_conservation + # See https://github.com/certbot/certbot/pull/7095 for details. + other_webroot_path = tempfile.mkdtemp() + args = self.parser.parse_args("-w {0} -d {1} -w {2} -d bar".format( + self.path, self.achall.domain, other_webroot_path).split()) + self.assertEqual(args.webroot_map, {self.achall.domain: self.path}) + self.assertEqual(args.webroot_path, [self.path, other_webroot_path]) + def _get_config_after_perform(self, config): from certbot.plugins.webroot import Authenticator auth = Authenticator(config, "webroot") diff --git a/certbot/renewal.py b/certbot/renewal.py index f932269b6..4b4a5a082 100644 --- a/certbot/renewal.py +++ b/certbot/renewal.py @@ -106,11 +106,11 @@ def _restore_webroot_config(config, renewalparams): restoring logic is not able to correctly parse it from the serialized form. """ - if "webroot_map" in renewalparams: - if not cli.set_by_cli("webroot_map"): - config.webroot_map = renewalparams["webroot_map"] - elif "webroot_path" in renewalparams: - logger.debug("Ancient renewal conf file without webroot-map, restoring webroot-path") + if "webroot_map" in renewalparams and not cli.set_by_cli("webroot_map"): + config.webroot_map = renewalparams["webroot_map"] + # To understand why webroot_path and webroot_map processing are not mutually exclusive, + # see https://github.com/certbot/certbot/pull/7095 + if "webroot_path" in renewalparams and not cli.set_by_cli("webroot_path"): wp = renewalparams["webroot_path"] if isinstance(wp, six.string_types): # prior to 0.1.0, webroot_path was a string wp = [wp] diff --git a/certbot/tests/renewal_test.py b/certbot/tests/renewal_test.py index dac585239..e33869e13 100644 --- a/certbot/tests/renewal_test.py +++ b/certbot/tests/renewal_test.py @@ -28,6 +28,29 @@ class RenewalTest(test_util.ConfigTestCase): renewal._restore_webroot_config(config, renewalparams) self.assertEqual(config.webroot_path, ['/var/www/']) + @mock.patch('certbot.renewal.cli.set_by_cli') + def test_webroot_params_conservation(self, mock_set_by_cli): + # For more details about why this test is important, see: + # certbot.plugins.webroot_test::WebrootActionTest::test_webroot_map_partial_without_perform + from certbot import renewal + mock_set_by_cli.return_value = False + + renewalparams = { + 'webroot_map': {'test.example.com': '/var/www/test'}, + 'webroot_path': ['/var/www/test', '/var/www/other'], + } + renewal._restore_webroot_config(self.config, renewalparams) # pylint: disable=protected-access + self.assertEqual(self.config.webroot_map, {'test.example.com': '/var/www/test'}) + self.assertEqual(self.config.webroot_path, ['/var/www/test', '/var/www/other']) + + renewalparams = { + 'webroot_map': {}, + 'webroot_path': '/var/www/test', + } + renewal._restore_webroot_config(self.config, renewalparams) # pylint: disable=protected-access + self.assertEqual(self.config.webroot_map, {}) + self.assertEqual(self.config.webroot_path, ['/var/www/test']) + class RestoreRequiredConfigElementsTest(test_util.ConfigTestCase): """Tests for certbot.renewal.restore_required_config_elements.""" @@ -89,5 +112,6 @@ class RestoreRequiredConfigElementsTest(test_util.ConfigTestCase): self.assertRaises( errors.Error, self._call, self.config, renewalparams) + if __name__ == "__main__": unittest.main() # pragma: no cover