Revert "More thoroughly rename during `certbot rename`. (#4320)" (#4467)

This reverts commit 43dccfc67187b0ae3d677fbe0c7fffab20e6179c.

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