diff --git a/certbot/cli.py b/certbot/cli.py index cff111f42..ba1f23708 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -1,6 +1,7 @@ """Certbot command line argument & config processing.""" from __future__ import print_function import argparse +import copy import glob import logging import logging.handlers @@ -211,6 +212,35 @@ def set_by_cli(var): set_by_cli.detector = None +def has_default_value(option, value): + """Does option have the default value? + + If the default value of option is not known, False is returned. + + :param str option: configuration variable being considered + :param value: value of the configuration variable named option + + :returns: True if option has the default value, otherwise, False + :rtype: bool + + """ + return (option in helpful_parser.defaults and + helpful_parser.defaults[option] == value) + + +def option_was_set(option, value): + """Was option set by the user or does it differ from the default? + + :param str option: configuration variable being considered + :param value: value of the configuration variable named option + + :returns: True if the option was set, otherwise, False + :rtype: bool + + """ + return set_by_cli(option) or not has_default_value(option, value) + + def argparse_type(variable): "Return our argparse type function for a config variable (default: str)" # pylint: disable=protected-access @@ -320,6 +350,7 @@ class HelpfulArgumentParser(object): sys.exit(0) self.visible_topics = self.determine_help_topics(self.help_arg) self.groups = {} # elements are added by .add_group() + self.defaults = {} # elements are added by .parse_args() def parse_args(self): """Parses command line arguments and returns the result. @@ -335,6 +366,9 @@ class HelpfulArgumentParser(object): if self.detect_defaults: return parsed_args + self.defaults = dict((key, copy.deepcopy(self.parser.get_default(key))) + for key in vars(parsed_args)) + # Do any post-parsing homework here if self.verb == "renew" and not parsed_args.dialog_mode: diff --git a/certbot/storage.py b/certbot/storage.py index 60886e306..82fdbfd54 100644 --- a/certbot/storage.py +++ b/certbot/storage.py @@ -7,8 +7,10 @@ import re import configobj import parsedatetime import pytz +import six import certbot +from certbot import cli from certbot import constants from certbot import crypto_util from certbot import errors @@ -158,36 +160,13 @@ def relevant_values(all_values): :param dict all_values: The original values. :returns: A new dictionary containing items that can be used in renewal. - :rtype dict:""" + :rtype dict: - from certbot import cli - - def _is_cli_default(option, value): - # Look through the CLI parser defaults and see if this option is - # both present and equal to the specified value. If not, return - # False. - # pylint: disable=protected-access - for x in cli.helpful_parser.parser._actions: - if x.dest == option: - if x.default == value: - return True - else: - break - return False - - values = dict() - for option, value in all_values.iteritems(): - # Try to find reasons to store this item in the - # renewal config. It can be stored if it is relevant and - # (it is set_by_cli() or flag_default() is different - # from the value or flag_default() doesn't exist). - if _relevant(option): - if (cli.set_by_cli(option) - or not _is_cli_default(option, value)): -# or option not in constants.CLI_DEFAULTS -# or constants.CLI_DEFAULTS[option] != value): - values[option] = value - return values + """ + return dict( + (option, value) + for option, value in six.iteritems(all_values) + if _relevant(option) and cli.option_was_set(option, value)) class RenewableCert(object): # pylint: disable=too-many-instance-attributes diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index f9557abfb..9d6335b40 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -447,6 +447,19 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods short_args += '--server example.com'.split() self._check_server_conflict_message(short_args, '--staging') + def test_option_was_set(self): + key_size_option = 'rsa_key_size' + key_size_value = cli.flag_default(key_size_option) + self._get_argument_parser()( + '--rsa-key-size {0}'.format(key_size_value).split()) + + self.assertTrue(cli.option_was_set(key_size_option, key_size_value)) + self.assertTrue(cli.option_was_set('no_verify_ssl', True)) + + config_dir_option = 'config_dir' + self.assertFalse(cli.option_was_set( + config_dir_option, cli.flag_default(config_dir_option))) + def _assert_dry_run_flag_worked(self, namespace, existing_account): self.assertTrue(namespace.dry_run) self.assertTrue(namespace.break_my_certs) diff --git a/certbot/tests/storage_test.py b/certbot/tests/storage_test.py index 0d907eca3..261500b98 100644 --- a/certbot/tests/storage_test.py +++ b/certbot/tests/storage_test.py @@ -11,6 +11,7 @@ import mock import pytz import certbot +from certbot import cli from certbot import configuration from certbot import errors from certbot.storage import ALL_FOUR @@ -517,39 +518,33 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertFalse(os.path.islink(self.test_rc.version("privkey", 10))) self.assertFalse(os.path.exists(temp_config_file)) - @mock.patch("certbot.cli.helpful_parser") - def test_relevant_values(self, mock_parser): + def _test_relevant_values_common(self, values): + option = "rsa_key_size" + mock_parser = mock.Mock(args=["--standalone"], verb="certonly", + defaults={option: cli.flag_default(option)}) + + from certbot.storage import relevant_values + with mock.patch("certbot.cli.helpful_parser", mock_parser): + return relevant_values(values) + + def test_relevant_values(self): """Test that relevant_values() can reject an irrelevant value.""" - # pylint: disable=protected-access - from certbot import storage - mock_parser.verb = "certonly" - mock_parser.args = ["--standalone"] - mock_action = mock.Mock(dest="rsa_key_size", default=2048) - mock_parser.parser._actions = [mock_action] - self.assertEqual(storage.relevant_values({"hello": "there"}), {}) + self.assertEqual( + self._test_relevant_values_common({"hello": "there"}), {}) - @mock.patch("certbot.cli.helpful_parser") - def test_relevant_values_default(self, mock_parser): + def test_relevant_values_default(self): """Test that relevant_values() can reject a default value.""" - # pylint: disable=protected-access - from certbot import storage - mock_parser.verb = "certonly" - mock_parser.args = ["--standalone"] - mock_action = mock.Mock(dest="rsa_key_size", default=2048) - mock_parser.parser._actions = [mock_action] - self.assertEqual(storage.relevant_values({"rsa_key_size": 2048}), {}) + option = "rsa_key_size" + values = {option: cli.flag_default(option)} + self.assertEqual(self._test_relevant_values_common(values), {}) - @mock.patch("certbot.cli.helpful_parser") - def test_relevant_values_nondefault(self, mock_parser): + def test_relevant_values_nondefault(self): """Test that relevant_values() can retain a non-default value.""" - # pylint: disable=protected-access - from certbot import storage - mock_parser.verb = "certonly" - mock_parser.args = ["--standalone"] - mock_action = mock.Mock(dest="rsa_key_size", default=2048) - mock_parser.parser._actions = [mock_action] - self.assertEqual(storage.relevant_values({"rsa_key_size": 12}), - {"rsa_key_size": 12}) + 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) @mock.patch("certbot.storage.relevant_values") def test_new_lineage(self, mock_rv):