diff --git a/certbot/renewal.py b/certbot/renewal.py index c9eca9a3a..6448e5a8e 100644 --- a/certbot/renewal.py +++ b/certbot/renewal.py @@ -1,6 +1,7 @@ """Functionality for autorenewal and associated juggling of configurations""" from __future__ import print_function import copy +import itertools import logging import os import traceback @@ -30,6 +31,10 @@ STR_CONFIG_ITEMS = ["config_dir", "logs_dir", "work_dir", "user_agent", "server", "account", "authenticator", "installer", "standalone_supported_challenges", "renew_hook"] INT_CONFIG_ITEMS = ["rsa_key_size", "tls_sni_01_port", "http01_port"] +BOOL_CONFIG_ITEMS = ["must_staple", "allow_subset_of_names"] + +CONFIG_ITEMS = set(itertools.chain( + BOOL_CONFIG_ITEMS, INT_CONFIG_ITEMS, STR_CONFIG_ITEMS)) def _reconstitute(config, full_path): @@ -68,7 +73,7 @@ def _reconstitute(config, full_path): # Now restore specific values along with their data types, if # those elements are present. try: - _restore_required_config_elements(config, renewalparams) + restore_required_config_elements(config, renewalparams) _restore_plugin_configs(config, renewalparams) except (ValueError, errors.Error) as error: logger.warning( @@ -148,7 +153,7 @@ def _restore_plugin_configs(config, renewalparams): setattr(config.namespace, config_item, cast(config_value)) -def _restore_required_config_elements(config, renewalparams): +def restore_required_config_elements(config, renewalparams): """Sets non-plugin specific values in config from renewalparams :param configuration.NamespaceConfig config: configuration for the @@ -157,30 +162,69 @@ def _restore_required_config_elements(config, renewalparams): configuration file that defines this lineage """ - # string-valued items to add if they're present - for config_item in STR_CONFIG_ITEMS: - if config_item in renewalparams and not cli.set_by_cli(config_item): - value = renewalparams[config_item] - # Unfortunately, we've lost type information from ConfigObj, - # so we don't know if the original was NoneType or str! - if value == "None": - value = None - setattr(config.namespace, config_item, value) - # int-valued items to add if they're present - for config_item in INT_CONFIG_ITEMS: - if config_item in renewalparams and not cli.set_by_cli(config_item): - config_value = renewalparams[config_item] - # the default value for http01_port was None during private beta - if config_item == "http01_port" and config_value == "None": - logger.info("updating legacy http01_port value") - int_value = cli.flag_default("http01_port") - else: - try: - int_value = int(config_value) - except ValueError: - raise errors.Error( - "Expected a numeric value for {0}".format(config_item)) - setattr(config.namespace, config_item, int_value) + + required_items = itertools.chain( + six.moves.zip(BOOL_CONFIG_ITEMS, itertools.repeat(_restore_bool)), + six.moves.zip(INT_CONFIG_ITEMS, itertools.repeat(_restore_int)), + six.moves.zip(STR_CONFIG_ITEMS, itertools.repeat(_restore_str))) + for item_name, restore_func in required_items: + if item_name in renewalparams and not cli.set_by_cli(item_name): + value = restore_func(item_name, renewalparams[item_name]) + setattr(config.namespace, item_name, value) + + +def _restore_bool(name, value): + """Restores an boolean key-value pair from a renewal config file. + + :param str name: option name + :param str value: option value + + :returns: converted option value to be stored in the runtime config + :rtype: bool + + :raises errors.Error: if value can't be converted to an bool + + """ + lowercase_value = value.lower() + if lowercase_value not in ("true", "false"): + raise errors.Error( + "Expected True or False for {0} but found {1}".format(name, value)) + return lowercase_value == "true" + + +def _restore_int(name, value): + """Restores an integer key-value pair from a renewal config file. + + :param str name: option name + :param str value: option value + + :returns: converted option value to be stored in the runtime config + :rtype: int + + :raises errors.Error: if value can't be converted to an int + + """ + if name == "http01_port" and value == "None": + logger.info("updating legacy http01_port value") + return cli.flag_default("http01_port") + + try: + return int(value) + except ValueError: + raise errors.Error("Expected a numeric value for {0}".format(name)) + + +def _restore_str(unused_name, value): + """Restores an string key-value pair from a renewal config file. + + :param str unused_name: option name + :param str value: option value + + :returns: converted option value to be stored in the runtime config + :rtype: str or None + + """ + return None if value == "None" else value def should_renew(config, lineage): diff --git a/certbot/storage.py b/certbot/storage.py index e4fc21a85..af0e9d701 100644 --- a/certbot/storage.py +++ b/certbot/storage.py @@ -183,9 +183,9 @@ def _relevant(option): from certbot import renewal from certbot.plugins import disco as plugins_disco plugins = list(plugins_disco.PluginsRegistry.find_all()) - return (option in renewal.STR_CONFIG_ITEMS - or option in renewal.INT_CONFIG_ITEMS - or any(option.startswith(x + "_") for x in plugins)) + + return (option in renewal.CONFIG_ITEMS or + any(option.startswith(x + "_") for x in plugins)) def relevant_values(all_values): diff --git a/certbot/tests/renewal_test.py b/certbot/tests/renewal_test.py index 8155595c2..07c4eac00 100644 --- a/certbot/tests/renewal_test.py +++ b/certbot/tests/renewal_test.py @@ -5,6 +5,7 @@ import unittest import tempfile from certbot import configuration +from certbot import errors from certbot import storage from certbot.tests import util @@ -15,15 +16,55 @@ class RenewalTest(unittest.TestCase): self.tmp_dir = tempfile.mkdtemp() self.config_dir = os.path.join(self.tmp_dir, 'config') - @mock.patch("certbot.cli.set_by_cli") + @mock.patch('certbot.cli.set_by_cli') def test_ancient_webroot_renewal_conf(self, mock_set_by_cli): mock_set_by_cli.return_value = False rc_path = util.make_lineage(self, 'sample-renewal-ancient.conf') args = mock.MagicMock(account=None, email=None, webroot_path=None) config = configuration.NamespaceConfig(args) lineage = storage.RenewableCert(rc_path, config) - renewalparams = lineage.configuration["renewalparams"] + renewalparams = lineage.configuration['renewalparams'] # pylint: disable=protected-access from certbot import renewal renewal._restore_webroot_config(config, renewalparams) - self.assertEqual(config.webroot_path, ["/var/www/"]) + self.assertEqual(config.webroot_path, ['/var/www/']) + + +class RestoreRequiredConfigElementsTest(unittest.TestCase): + """Tests for certbot.renewal.restore_required_config_elements.""" + def setUp(self): + self.config = mock.MagicMock() + + @classmethod + def _call(cls, *args, **kwargs): + from certbot.renewal import restore_required_config_elements + return restore_required_config_elements(*args, **kwargs) + + @mock.patch('certbot.renewal.cli.set_by_cli') + def test_allow_subset_of_names_success(self, mock_set_by_cli): + mock_set_by_cli.return_value = False + self._call(self.config, {'allow_subset_of_names': 'True'}) + self.assertTrue(self.config.namespace.allow_subset_of_names is True) + + @mock.patch('certbot.renewal.cli.set_by_cli') + def test_allow_subset_of_names_failure(self, mock_set_by_cli): + mock_set_by_cli.return_value = False + renewalparams = {'allow_subset_of_names': 'maybe'} + self.assertRaises( + errors.Error, self._call, self.config, renewalparams) + + @mock.patch('certbot.renewal.cli.set_by_cli') + def test_must_staple_success(self, mock_set_by_cli): + mock_set_by_cli.return_value = False + self._call(self.config, {'must_staple': 'True'}) + self.assertTrue(self.config.namespace.must_staple is True) + + @mock.patch('certbot.renewal.cli.set_by_cli') + def test_must_staple_failure(self, mock_set_by_cli): + mock_set_by_cli.return_value = False + renewalparams = {'must_staple': 'maybe'} + self.assertRaises( + errors.Error, self._call, self.config, renewalparams) + +if __name__ == "__main__": + unittest.main() # pragma: no cover diff --git a/certbot/tests/storage_test.py b/certbot/tests/storage_test.py index a1fda6535..f52f31d3d 100644 --- a/certbot/tests/storage_test.py +++ b/certbot/tests/storage_test.py @@ -551,7 +551,8 @@ class RenewableCertTests(BaseRenewableCertTest): from certbot.storage import relevant_values with mock.patch("certbot.cli.helpful_parser", mock_parser): - return relevant_values(values) + # make a copy to ensure values isn't modified + return relevant_values(values.copy()) def test_relevant_values(self): """Test that relevant_values() can reject an irrelevant value.""" @@ -567,10 +568,18 @@ class RenewableCertTests(BaseRenewableCertTest): def test_relevant_values_nondefault(self): """Test that relevant_values() can retain a non-default value.""" values = {"rsa_key_size": 12} - # A copy is given to _test_relevant_values_common - # to make sure values isn't modified by the method self.assertEqual( - self._test_relevant_values_common(values.copy()), values) + self._test_relevant_values_common(values), values) + + def test_relevant_values_bool(self): + values = {"allow_subset_of_names": True} + self.assertEqual( + self._test_relevant_values_common(values), values) + + def test_relevant_values_str(self): + values = {"authenticator": "apache"} + self.assertEqual( + self._test_relevant_values_common(values), values) @mock.patch("certbot.storage.relevant_values") def test_new_lineage(self, mock_rv): diff --git a/certbot/tests/testdata/sample-renewal.conf b/certbot/tests/testdata/sample-renewal.conf index 08032af86..52b3ec45c 100644 --- a/certbot/tests/testdata/sample-renewal.conf +++ b/certbot/tests/testdata/sample-renewal.conf @@ -73,4 +73,5 @@ tls_sni_01_port = 443 logs_dir = /var/log/letsencrypt apache_vhost_root = /etc/apache2/sites-available configurator = None +must_staple = True [[webroot_map]]