diff --git a/letsencrypt/main.py b/letsencrypt/main.py index 4e35a5227..0afccc85e 100644 --- a/letsencrypt/main.py +++ b/letsencrypt/main.py @@ -463,7 +463,7 @@ def config_changes(config, unused_plugins): def revoke(config, unused_plugins): # TODO: coop with renewal config """Revoke a previously obtained certificate.""" # For user-agent construction - config.namespace.installer = config.namespace.authenticator = "none" + config.namespace.installer = config.namespace.authenticator = "None" if config.key_path is not None: # revocation by cert key logger.debug("Revoking %s using cert key %s", config.cert_path[0], config.key_path[0]) diff --git a/letsencrypt/plugins/selection.py b/letsencrypt/plugins/selection.py index 057a09768..20f6ac512 100644 --- a/letsencrypt/plugins/selection.py +++ b/letsencrypt/plugins/selection.py @@ -137,8 +137,8 @@ noninstaller_plugins = ["webroot", "manual", "standalone"] def record_chosen_plugins(config, plugins, auth, inst): "Update the config entries to reflect the plugins we actually selected." cn = config.namespace - cn.authenticator = plugins.find_init(auth).name if auth else "none" - cn.installer = plugins.find_init(inst).name if inst else "none" + cn.authenticator = plugins.find_init(auth).name if auth else "None" + cn.installer = plugins.find_init(inst).name if inst else "None" def choose_configurator_plugins(config, plugins, verb): diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index cff2d53e1..59daa1a0d 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -50,13 +50,12 @@ def add_time_interval(base_time, interval, textparser=parsedatetime.Calendar()): return textparser.parseDT(interval, base_time, tzinfo=tzinfo)[0] -def write_renewal_config(filename, target, cli_config): +def write_renewal_config(filename, target, relevant_data): """Writes a renewal config file with the specified name and values. :param str filename: Absolute path to the config file :param dict target: Maps ALL_FOUR to their symlink paths - :param .RenewerConfiguration cli_config: parsed command line - arguments + :param dict relevant_data: Renewal configuration options to save :returns: Configuration object for the new config file :rtype: configobj.ConfigObj @@ -67,18 +66,11 @@ def write_renewal_config(filename, target, cli_config): for kind in ALL_FOUR: config[kind] = target[kind] - # XXX: We clearly need a more general and correct way of getting - # options into the configobj for the RenewableCert instance. - # This is a quick-and-dirty way to do it to allow integration - # testing to start. (Note that the config parameter to new_lineage - # ideally should be a ConfigObj, but in this case a dict will be - # accepted in practice.) - renewalparams = vars(cli_config.namespace) - if renewalparams: - config["renewalparams"] = renewalparams + if relevant_data: + config["renewalparams"] = relevant_data config.comments["renewalparams"] = ["", - "Options and defaults used" - " in the renewal process"] + "Options used in " + "the renewal process"] # TODO: add human-readable comments explaining other available # parameters @@ -106,7 +98,10 @@ def update_configuration(lineagename, target, cli_config): # If an existing tempfile exists, delete it if os.path.exists(temp_filename): os.unlink(temp_filename) - write_renewal_config(temp_filename, target, cli_config) + + # Save only the config items that are relevant to renewal + values = relevant_values(vars(cli_config.namespace)) + write_renewal_config(temp_filename, target, values) os.rename(temp_filename, config_filename) return configobj.ConfigObj(config_filename) @@ -127,6 +122,60 @@ def get_link_target(link): return os.path.abspath(target) +def _relevant(option): + """ + Is this option one that could be restored for future renewal purposes? + :param str option: the name of the option + + :rtype: bool + """ + # The list() here produces a list of the plugin names as strings. + from letsencrypt import renewal + from letsencrypt.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)) + + +def relevant_values(all_values): + """Return a new dict containing only items relevant for renewal. + + :param dict all_values: The original values. + + :returns: A new dictionary containing items that can be used in renewal. + :rtype dict:""" + + from letsencrypt 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 + + class RenewableCert(object): # pylint: disable=too-many-instance-attributes """Renewable certificate. @@ -690,6 +739,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes :rtype: :class:`storage.renewableCert` """ + # Examine the configuration and find the new lineage's name for i in (cli_config.renewal_configs_dir, cli_config.archive_dir, cli_config.live_dir): @@ -744,7 +794,11 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes # Document what we've done in a new renewal config file config_file.close() - new_config = write_renewal_config(config_filename, target, cli_config) + + # Save only the config items that are relevant to renewal + values = relevant_values(vars(cli_config.namespace)) + + new_config = write_renewal_config(config_filename, target, values) return cls(new_config.filename, cli_config) def save_successor(self, prior_version, new_cert, diff --git a/letsencrypt/tests/storage_test.py b/letsencrypt/tests/storage_test.py index 7bc31dab3..49b4f0821 100644 --- a/letsencrypt/tests/storage_test.py +++ b/letsencrypt/tests/storage_test.py @@ -493,7 +493,12 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertTrue(self.test_rc.should_autorenew()) mock_ocsp.return_value = False - def test_save_successor(self): + @mock.patch("letsencrypt.storage.relevant_values") + def test_save_successor(self, mock_rv): + # Mock relevant_values() to claim that all values are relevant here + # (to avoid instantiating parser) + mock_rv.side_effect = lambda x: x + for ver in xrange(1, 6): for kind in ALL_FOUR: where = getattr(self.test_rc, kind) @@ -557,8 +562,47 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertFalse(os.path.islink(self.test_rc.version("privkey", 10))) self.assertFalse(os.path.exists(temp_config_file)) - def test_new_lineage(self): + @mock.patch("letsencrypt.cli.helpful_parser") + def test_relevant_values(self, mock_parser): + """Test that relevant_values() can reject an irrelevant value.""" + # pylint: disable=protected-access + from letsencrypt 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"}), {}) + + @mock.patch("letsencrypt.cli.helpful_parser") + def test_relevant_values_default(self, mock_parser): + """Test that relevant_values() can reject a default value.""" + # pylint: disable=protected-access + from letsencrypt 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}), {}) + + @mock.patch("letsencrypt.cli.helpful_parser") + def test_relevant_values_nondefault(self, mock_parser): + """Test that relevant_values() can retain a non-default value.""" + # pylint: disable=protected-access + from letsencrypt 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}) + + @mock.patch("letsencrypt.storage.relevant_values") + def test_new_lineage(self, mock_rv): """Test for new_lineage() class method.""" + # Mock relevant_values to say everything is relevant here (so we + # don't have to mock the parser to help it decide!) + mock_rv.side_effect = lambda x: x + from letsencrypt import storage result = storage.RenewableCert.new_lineage( "the-lineage.com", "cert", "privkey", "chain", self.cli_config) @@ -592,8 +636,13 @@ class RenewableCertTests(BaseRenewableCertTest): # TODO: Conceivably we could test that the renewal parameters actually # got saved - def test_new_lineage_nonexistent_dirs(self): + @mock.patch("letsencrypt.storage.relevant_values") + def test_new_lineage_nonexistent_dirs(self, mock_rv): """Test that directories can be created if they don't exist.""" + # Mock relevant_values to say everything is relevant here (so we + # don't have to mock the parser to help it decide!) + mock_rv.side_effect = lambda x: x + from letsencrypt import storage shutil.rmtree(self.cli_config.renewal_configs_dir) shutil.rmtree(self.cli_config.archive_dir) diff --git a/letsencrypt/tests/testdata/sample-renewal-ancient.conf b/letsencrypt/tests/testdata/sample-renewal-ancient.conf index ff246ba7c..dd3075b8e 100644 --- a/letsencrypt/tests/testdata/sample-renewal-ancient.conf +++ b/letsencrypt/tests/testdata/sample-renewal-ancient.conf @@ -14,7 +14,7 @@ apache_dismod = a2dismod register_unsafely_without_email = False apache_handle_modules = True uir = None -installer = none +installer = None nginx_ctl = nginx config_dir = MAGICDIR text_mode = False diff --git a/letsencrypt/tests/testdata/sample-renewal.conf b/letsencrypt/tests/testdata/sample-renewal.conf index d6ebbd845..08032af86 100644 --- a/letsencrypt/tests/testdata/sample-renewal.conf +++ b/letsencrypt/tests/testdata/sample-renewal.conf @@ -14,7 +14,7 @@ apache_dismod = a2dismod register_unsafely_without_email = False apache_handle_modules = True uir = None -installer = none +installer = None nginx_ctl = nginx config_dir = MAGICDIR text_mode = False