diff options
author | Erica Portnoy <ebportnoy@gmail.com> | 2017-04-05 22:45:03 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-04-05 22:45:03 +0300 |
commit | 18486d6ea0a9695efea8fa9e639afae5bb77afc4 (patch) | |
tree | 3312db36dce3d3ae1b2451eeb245fbf758698189 | |
parent | 664c9aaae54e17b28f26e60eaf6622d8919aa760 (diff) |
Revert "More thoroughly rename during `certbot rename`. (#4320)" (#4467)
This reverts commit 43dccfc67187b0ae3d677fbe0c7fffab20e6179c.
-rw-r--r-- | certbot/cert_manager.py | 37 | ||||
-rw-r--r-- | certbot/cli.py | 15 | ||||
-rw-r--r-- | certbot/main.py | 9 | ||||
-rw-r--r-- | certbot/storage.py | 102 | ||||
-rw-r--r-- | certbot/tests/cert_manager_test.py | 21 |
5 files changed, 27 insertions, 157 deletions
diff --git a/certbot/cert_manager.py b/certbot/cert_manager.py index 7b1811c99..35d539a16 100644 --- a/certbot/cert_manager.py +++ b/certbot/cert_manager.py @@ -13,7 +13,6 @@ from certbot import storage from certbot import util from certbot.display import util as display_util -from certbot.plugins import disco as plugins_disco logger = logging.getLogger(__name__) @@ -47,7 +46,6 @@ def rename_lineage(config): certname = _get_certname(config, "rename") - # what is the new name we want to use? new_certname = config.new_certname if not new_certname: code, new_certname = disp.input( @@ -56,34 +54,13 @@ def rename_lineage(config): if code != display_util.OK or not new_certname: raise errors.Error("User ended interaction.") - try: - # copy files to new name - new_lineage = storage.duplicate_lineage(config, certname, new_certname) - - # install the new name's files - config.certname = new_certname - plugins = plugins_disco.PluginsRegistry.find_all() - from certbot.main import install - install(config, plugins, new_lineage, False) - except (errors.CertStorageError, errors.ConfigurationError, IOError, OSError) as e: - # delete the new files - config.certname = new_certname - # we might not have created anything to delete - try: - storage.delete_files(config, new_certname) - except errors.CertStorageError: - pass - reporter = zope.component.getUtility(interfaces.IReporter) - reporter.add_message("Unable to rename certificate", reporter.HIGH_PRIORITY) - raise e - else: - # delete old files - config.certname = certname - storage.delete_files(config, certname) - disp.notification("Renamed files for {0} to {1}." - .format(certname, new_certname), pause=False) - finally: - config.certname = certname + lineage = lineage_for_certname(config, certname) + if not lineage: + raise errors.ConfigurationError("No existing certificate with name " + "{0} found.".format(certname)) + storage.rename_renewal_config(certname, new_certname, config) + disp.notification("Successfully renamed {0} to {1}." + .format(certname, new_certname), pause=False) def certificates(config): """Display information about certs configured with Certbot diff --git a/certbot/cli.py b/certbot/cli.py index 1c91935d3..cb769872e 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -81,7 +81,6 @@ obtain, install, and renew certificates: manage certificates: certificates Display information about certs you have from Certbot revoke Revoke a certificate (supply --cert-path) - rename Rename a certificate delete Delete a certificate manage your account with Let's Encrypt: @@ -364,10 +363,6 @@ VERB_HELP = [ "opts": "Options for revocation of certs", "usage": "\n\n certbot revoke --cert-path /path/to/fullchain.pem [options]\n\n" }), - ("rename", { - "short": "Change a certificate's name (for management purposes)", - "opts": "Options for changing certificate names" - }), ("register", { "short": "Register for account with Let's Encrypt / other ACME server", "opts": "Options for account registration & modification" @@ -425,7 +420,6 @@ class HelpfulArgumentParser(object): "plugins": main.plugins_cmd, "register": main.register, "unregister": main.unregister, - "rename": main.rename, "renew": main.renew, "revoke": main.revoke, "rollback": main.rollback, @@ -796,7 +790,7 @@ def _add_all_groups(helpful): helpful.add_group("paths", description="Arguments changing execution paths & servers") helpful.add_group("manage", description="Various subcommands and flags are available for managing your certificates:", - verbs=["certificates", "delete", "rename", "renew", "revoke", "update_symlinks"]) + verbs=["certificates", "delete", "renew", "revoke", "update_symlinks"]) # VERBS for verb, docs in VERB_HELP: @@ -849,18 +843,13 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis "multiple -d flags or enter a comma separated list of domains " "as a parameter. (default: Ask)") helpful.add( - [None, "run", "certonly", "manage", "rename", "delete", "certificates"], + [None, "run", "certonly", "manage", "delete", "certificates"], "--cert-name", dest="certname", metavar="CERTNAME", default=None, help="Certificate name to apply. Only one certificate name can be used " "per Certbot run. To see certificate names, run 'certbot certificates'. " "When creating a new certificate, specifies the new certificate's name.") helpful.add( - ["rename", "manage"], - "--updated-cert-name", dest="new_certname", - metavar="NEW_CERTNAME", default=None, - help="New name for the certificate. Must be a valid filename.") - helpful.add( [None, "testing", "renew", "certonly"], "--dry-run", action="store_true", dest="dry_run", help="Perform a test run of the client, obtaining test (invalid) certs" diff --git a/certbot/main.py b/certbot/main.py index 256503d4e..090479d20 100644 --- a/certbot/main.py +++ b/certbot/main.py @@ -460,16 +460,15 @@ def register(config, unused_plugins): eff.handle_subscription(config) add_msg("Your e-mail address was updated to {0}.".format(config.email)) -def _install_cert(config, le_client, domains, lineage=None, enhance=True): +def _install_cert(config, le_client, domains, lineage=None): path_provider = lineage if lineage else config assert path_provider.cert_path is not None le_client.deploy_certificate(domains, path_provider.key_path, path_provider.cert_path, path_provider.chain_path, path_provider.fullchain_path) - if enhance: - le_client.enhance_config(domains, path_provider.chain_path) + le_client.enhance_config(domains, path_provider.chain_path) -def install(config, plugins, lineage=None, enhance=True): +def install(config, plugins): """Install a previously obtained cert in a server.""" # XXX: Update for renewer/RenewableCert # FIXME: be consistent about whether errors are raised or returned from @@ -482,7 +481,7 @@ def install(config, plugins, lineage=None, enhance=True): domains, _ = _find_domains_or_certname(config, installer) le_client = _init_le_client(config, authenticator=None, installer=installer) - _install_cert(config, le_client, domains, lineage, enhance) + _install_cert(config, le_client, domains) def plugins_cmd(config, plugins): # TODO: Use IDisplay rather than print diff --git a/certbot/storage.py b/certbot/storage.py index 34dc57884..a1462b72d 100644 --- a/certbot/storage.py +++ b/certbot/storage.py @@ -34,9 +34,7 @@ def renewal_conf_files(config): return glob.glob(os.path.join(config.renewal_configs_dir, "*.conf")) def renewal_file_for_certname(config, certname): - """Return /path/to/certname.conf in the renewal conf directory - :raises .CertStorageError: if file is missing - """ + """Return /path/to/certname.conf in the renewal conf directory""" path = os.path.join(config.renewal_configs_dir, "{0}.conf".format(certname)) if not os.path.exists(path): raise errors.CertStorageError("No certificate found with name {0} (expected " @@ -132,8 +130,6 @@ def rename_renewal_config(prev_name, new_name, cli_config): except OSError: raise errors.ConfigurationError("Please specify a valid filename " "for the new certificate name.") - else: - return new_filename def update_configuration(lineagename, archive_dir, target, cli_config): @@ -150,25 +146,19 @@ def update_configuration(lineagename, archive_dir, target, cli_config): """ config_filename = renewal_filename_for_lineagename(cli_config, lineagename) - - def _save_renewal_values(unused_config, temp_filename): - # Save only the config items that are relevant to renewal - values = relevant_values(vars(cli_config.namespace)) - write_renewal_config(config_filename, temp_filename, archive_dir, target, values) - _modify_config_with_tempfile(config_filename, _save_renewal_values) - - return configobj.ConfigObj(config_filename) - -def _modify_config_with_tempfile(filename, function): - temp_filename = filename + ".new" + temp_filename = config_filename + ".new" # If an existing tempfile exists, delete it if os.path.exists(temp_filename): os.unlink(temp_filename) - config = configobj.ConfigObj(filename) - function(config, temp_filename) - os.rename(temp_filename, filename) + # Save only the config items that are relevant to renewal + values = relevant_values(vars(cli_config.namespace)) + write_renewal_config(config_filename, temp_filename, archive_dir, target, values) + os.rename(temp_filename, config_filename) + + return configobj.ConfigObj(config_filename) + def get_link_target(link): """Get an absolute path to the target of link. @@ -253,7 +243,6 @@ def delete_files(config, certname): """Delete all files related to the certificate. If some files are not found, ignore them and continue. - :raises .CertStorageError: if lineage is missing """ renewal_filename = renewal_file_for_certname(config, certname) # file exists @@ -315,79 +304,6 @@ def delete_files(config, certname): except OSError: logger.debug("Unable to remove %s", archive_path) -def duplicate_lineage(config, certname, new_certname): - """Create a duplicate of certname with name new_certname - - :raises .CertStorageError: for storage errors - :raises .ConfigurationError: for cli and renewal configuration errors - :raises IOError: for filename errors - :raises OSError: for OS errors - """ - - # copy renewal config file - prev_filename = renewal_filename_for_lineagename(config, certname) - new_filename = renewal_filename_for_lineagename(config, new_certname) - if os.path.exists(new_filename): - raise errors.ConfigurationError("The new certificate name " - "is already in use.") - try: - shutil.copy2(prev_filename, new_filename) - except (OSError, IOError): - raise errors.ConfigurationError("Please specify a valid filename " - "for the new certificate name.") - logger.debug("Copied %s to %s", prev_filename, new_filename) - - # load config file - try: - renewal_config = configobj.ConfigObj(new_filename) - except configobj.ConfigObjError: - # config is corrupted - logger.warning("Could not parse %s. Only the certificate has been renamed.", - new_filename) - raise errors.CertStorageError( - "error parsing {0}".format(new_filename)) - - def copy_to_new_dir(prev_dir): - """Replace certname with new_certname in prev_dir""" - new_dir = prev_dir.replace(certname, new_certname) - # make dir iff it doesn't exist - shutil.copytree(prev_dir, new_dir, symlinks=True) - logger.debug("Copied %s to %s", prev_dir, new_dir) - return new_dir - - # archive dir - prev_archive_dir = _full_archive_path(renewal_config, config, certname) - new_archive_dir = prev_archive_dir - if not certname in prev_archive_dir: - raise errors.CertStorageError("Archive directory does not conform to defaults: " - "{0} not in {1}", certname, prev_archive_dir) - else: - new_archive_dir = copy_to_new_dir(prev_archive_dir) - - # live dir - # if things aren't in their default places, don't try to change things. - prev_live_dir = _full_live_path(config, certname) - prev_links = dict((kind, renewal_config.get(kind)) for kind in ALL_FOUR) - if (certname not in prev_live_dir or - len(set(os.path.dirname(renewal_config.get(kind)) for kind in ALL_FOUR)) != 1): - raise errors.CertStorageError("Live directory does not conform to defaults.") - else: - copy_to_new_dir(prev_live_dir) - new_links = dict((k, prev_links[k].replace(certname, new_certname)) for k in prev_links) - - # Update renewal config file - def _update_and_write(renewal_config, temp_filename): - renewal_config["archive_dir"] = new_archive_dir - renewal_config["version"] = certbot.__version__ - for kind in ALL_FOUR: - renewal_config[kind] = new_links[kind] - with open(temp_filename, "wb") as f: - renewal_config.write(outfile=f) - _modify_config_with_tempfile(new_filename, _update_and_write) - - # Update symlinks - return RenewableCert(new_filename, config, update_symlinks=True) - class RenewableCert(object): # pylint: disable=too-many-instance-attributes,too-many-public-methods diff --git a/certbot/tests/cert_manager_test.py b/certbot/tests/cert_manager_test.py index d0e563979..b5731fbd6 100644 --- a/certbot/tests/cert_manager_test.py +++ b/certbot/tests/cert_manager_test.py @@ -384,19 +384,14 @@ class RenameLineageTest(BaseCertManagerTest): @test_util.patch_get_utility() @mock.patch('certbot.cert_manager.lineage_for_certname') def test_no_existing_certname(self, mock_lineage_for_certname, unused_get_utility): - mock_config = mock.Mock(certname="one", new_certname="two", - renewal_configs_dir="/tmp/etc/letsencrypt/renewal/") + mock_config = mock.Mock(certname="one", new_certname="two") mock_lineage_for_certname.return_value = None - self.assertRaises(errors.ConfigurationError, self._call, mock_config) + self.assertRaises(errors.ConfigurationError, + self._call, mock_config) - @mock.patch("certbot.storage.RenewableCert._update_symlinks") @test_util.patch_get_utility() @mock.patch("certbot.storage.RenewableCert._check_symlinks") - @mock.patch("certbot.storage.relevant_values") - def test_rename_cert(self, mock_rv, mock_check, unused_get_utility, unused_update_symlinks): - # Mock relevant_values() to claim that all values are relevant here - # (to avoid instantiating parser) - mock_rv.side_effect = lambda x: x + def test_rename_cert(self, mock_check, unused_get_utility): mock_check.return_value = True mock_config = self.mock_config self._call(mock_config) @@ -405,15 +400,9 @@ class RenameLineageTest(BaseCertManagerTest): self.assertTrue(updated_lineage is not None) self.assertEqual(updated_lineage.lineagename, mock_config.new_certname) - @mock.patch("certbot.storage.RenewableCert._update_symlinks") @test_util.patch_get_utility() @mock.patch("certbot.storage.RenewableCert._check_symlinks") - @mock.patch("certbot.storage.relevant_values") - def test_rename_cert_interactive_certname(self, mock_rv, mock_check, mock_get_utility, - unused_update_symlinks): - # python 3.4 and 3.5 order things differently, so remove other.com for this test - os.remove(self.configs["other.com"].filename) - mock_rv.side_effect = lambda x: x + def test_rename_cert_interactive_certname(self, mock_check, mock_get_utility): mock_check.return_value = True mock_config = self.mock_config mock_config.certname = None |