From 388baa5a1eb49f22187548a6a7dee5d38dfe98aa Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 11 Mar 2016 12:29:31 -0800
Subject: [PATCH 1/8] Start splitting renew.py out of cli.py

---
 letsencrypt/cli.py            | 306 +---------------------------------
 letsencrypt/main.py           |  14 +-
 letsencrypt/plugins/common.py |   2 +-
 letsencrypt/renew.py          | 304 +++++++++++++++++++++++++++++++++
 letsencrypt/tests/cli_test.py |  24 +--
 5 files changed, 333 insertions(+), 317 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index b76311777..e34bd971b 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -2,7 +2,6 @@
 # pylint: disable=too-many-lines
 from __future__ import print_function
 import argparse
-import copy
 import glob
 import json
 import logging
@@ -14,19 +13,14 @@ import traceback
 import configargparse
 import OpenSSL
 import six
-import zope.component
-import zope.interface.exceptions
-import zope.interface.verify
 
 import letsencrypt
 
-from letsencrypt import configuration
 from letsencrypt import constants
 from letsencrypt import crypto_util
 from letsencrypt import errors
 from letsencrypt import interfaces
 from letsencrypt import le_util
-from letsencrypt import storage
 
 from letsencrypt.display import ops as display_ops
 from letsencrypt.plugins import disco as plugins_disco
@@ -35,16 +29,7 @@ from letsencrypt.plugins import disco as plugins_disco
 logger = logging.getLogger(__name__)
 
 # Global, to save us from a lot of argument passing within the scope of this module
-_parser = None
-
-# These are the items which get pulled out of a renewal configuration
-# file's renewalparams and actually used in the client configuration
-# during the renewal process. We have to record their types here because
-# the renewal configuration process loses this information.
-STR_CONFIG_ITEMS = ["config_dir", "logs_dir", "work_dir", "user_agent",
-                    "server", "account", "authenticator", "installer",
-                    "standalone_supported_challenges"]
-INT_CONFIG_ITEMS = ["rsa_key_size", "tls_sni_01_port", "http01_port"]
+helpful_parser = None
 
 # For help strings, figure out how the user ran us.
 # When invoked from letsencrypt-auto, sys.argv[0] is something like:
@@ -115,21 +100,6 @@ def usage_strings(plugins):
     return USAGE % (apache_doc, nginx_doc), SHORT_USAGE
 
 
-def should_renew(config, lineage):
-    "Return true if any of the circumstances for automatic renewal apply."
-    if config.renew_by_default:
-        logger.info("Auto-renewal forced with --force-renewal...")
-        return True
-    if lineage.should_autorenew(interactive=True):
-        logger.info("Cert is due for renewal, auto-renewing...")
-        return True
-    if config.dry_run:
-        logger.info("Cert not due for renewal, but simulating renewal for dry run")
-        return True
-    logger.info("Cert not yet due for renewal")
-    return False
-
-
 def diagnose_configurator_problem(cfg_type, requested, plugins):
     """
     Raise the most helpful error message about a plugin being unavailable
@@ -271,20 +241,20 @@ def record_chosen_plugins(config, plugins, auth, inst):
     cn.installer = plugins.find_init(inst).name if inst else "none"
 
 
-def _set_by_cli(var):
+def set_by_cli(var):
     """
     Return True if a particular config variable has been set by the user
     (CLI or config file) including if the user explicitly set it to the
     default.  Returns False if the variable was assigned a default value.
     """
-    detector = _set_by_cli.detector
+    detector = set_by_cli.detector
     if detector is None:
         # Setup on first run: `detector` is a weird version of config in which
         # the default value of every attribute is wrangled to be boolean-false
         plugins = plugins_disco.PluginsRegistry.find_all()
         # reconstructed_args == sys.argv[1:], or whatever was passed to main()
-        reconstructed_args = _parser.args + [_parser.verb]
-        detector = _set_by_cli.detector = prepare_and_parse_args(
+        reconstructed_args = helpful_parser.args + [helpful_parser.verb]
+        detector = set_by_cli.detector = prepare_and_parse_args(
             plugins, reconstructed_args, detect_defaults=True)
         # propagate plugin requests: eg --standalone modifies config.authenticator
         auth, inst = cli_plugin_requests(detector)
@@ -312,265 +282,7 @@ def _set_by_cli(var):
     else:
         return False
 # static housekeeping var
-_set_by_cli.detector = None
-
-def _restore_required_config_elements(config, renewalparams):
-    """Sets non-plugin specific values in config from renewalparams
-
-    :param configuration.NamespaceConfig config: configuration for the
-        current lineage
-    :param configobj.Section renewalparams: parameters from the renewal
-        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 _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 _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 = 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)
-
-
-def _restore_plugin_configs(config, renewalparams):
-    """Sets plugin specific values in config from renewalparams
-
-    :param configuration.NamespaceConfig config: configuration for the
-        current lineage
-    :param configobj.Section renewalparams: Parameters from the renewal
-        configuration file that defines this lineage
-
-    """
-    # Now use parser to get plugin-prefixed items with correct types
-    # XXX: the current approach of extracting only prefixed items
-    #      related to the actually-used installer and authenticator
-    #      works as long as plugins don't need to read plugin-specific
-    #      variables set by someone else (e.g., assuming Apache
-    #      configurator doesn't need to read webroot_ variables).
-    # Note: if a parameter that used to be defined in the parser is no
-    #      longer defined, stored copies of that parameter will be
-    #      deserialized as strings by this logic even if they were
-    #      originally meant to be some other type.
-    if renewalparams["authenticator"] == "webroot":
-        _restore_webroot_config(config, renewalparams)
-        plugin_prefixes = []
-    else:
-        plugin_prefixes = [renewalparams["authenticator"]]
-
-    if renewalparams.get("installer", None) is not None:
-        plugin_prefixes.append(renewalparams["installer"])
-    for plugin_prefix in set(plugin_prefixes):
-        for config_item, config_value in six.iteritems(renewalparams):
-            if config_item.startswith(plugin_prefix + "_") and not _set_by_cli(config_item):
-                # Values None, True, and False need to be treated specially,
-                # As they don't get parsed correctly based on type
-                if config_value in ("None", "True", "False"):
-                    # bool("False") == True
-                    # pylint: disable=eval-used
-                    setattr(config.namespace, config_item, eval(config_value))
-                    continue
-                for action in _parser.parser._actions:  # pylint: disable=protected-access
-                    if action.type is not None and action.dest == config_item:
-                        setattr(config.namespace, config_item,
-                                action.type(config_value))
-                        break
-                else:
-                    setattr(config.namespace, config_item, str(config_value))
-
-def _restore_webroot_config(config, renewalparams):
-    """
-    webroot_map is, uniquely, a dict, and the general-purpose configuration
-    restoring logic is not able to correctly parse it from the serialized
-    form.
-    """
-    if "webroot_map" in renewalparams:
-        # if the user does anything that would create a new webroot map on the
-        # CLI, don't use the old one
-        if not (_set_by_cli("webroot_map") or _set_by_cli("webroot_path")):
-            setattr(config.namespace, "webroot_map", renewalparams["webroot_map"])
-    elif "webroot_path" in renewalparams:
-        logger.info("Ancient renewal conf file without webroot-map, restoring webroot-path")
-        wp = renewalparams["webroot_path"]
-        if isinstance(wp, str):  # prior to 0.1.0, webroot_path was a string
-            wp = [wp]
-        setattr(config.namespace, "webroot_path", wp)
-
-
-def _reconstitute(config, full_path):
-    """Try to instantiate a RenewableCert, updating config with relevant items.
-
-    This is specifically for use in renewal and enforces several checks
-    and policies to ensure that we can try to proceed with the renwal
-    request. The config argument is modified by including relevant options
-    read from the renewal configuration file.
-
-    :param configuration.NamespaceConfig config: configuration for the
-        current lineage
-    :param str full_path: Absolute path to the configuration file that
-        defines this lineage
-
-    :returns: the RenewableCert object or None if a fatal error occurred
-    :rtype: `storage.RenewableCert` or NoneType
-
-    """
-    try:
-        renewal_candidate = storage.RenewableCert(
-            full_path, configuration.RenewerConfiguration(config))
-    except (errors.CertStorageError, IOError):
-        logger.warning("Renewal configuration file %s is broken. Skipping.", full_path)
-        logger.debug("Traceback was:\n%s", traceback.format_exc())
-        return None
-    if "renewalparams" not in renewal_candidate.configuration:
-        logger.warning("Renewal configuration file %s lacks "
-                       "renewalparams. Skipping.", full_path)
-        return None
-    renewalparams = renewal_candidate.configuration["renewalparams"]
-    if "authenticator" not in renewalparams:
-        logger.warning("Renewal configuration file %s does not specify "
-                       "an authenticator. Skipping.", full_path)
-        return None
-    # Now restore specific values along with their data types, if
-    # those elements are present.
-    try:
-        _restore_required_config_elements(config, renewalparams)
-        _restore_plugin_configs(config, renewalparams)
-    except (ValueError, errors.Error) as error:
-        logger.warning(
-            "An error occured while parsing %s. The error was %s. "
-            "Skipping the file.", full_path, error.message)
-        logger.debug("Traceback was:\n%s", traceback.format_exc())
-        return None
-
-    try:
-        for d in renewal_candidate.names():
-            process_domain(config, d)
-    except errors.ConfigurationError as error:
-        logger.warning("Renewal configuration file %s references a cert "
-                       "that contains an invalid domain name. The problem "
-                       "was: %s. Skipping.", full_path, error)
-        return None
-
-    return renewal_candidate
-
-def _renewal_conf_files(config):
-    """Return /path/to/*.conf in the renewal conf directory"""
-    return glob.glob(os.path.join(config.renewal_configs_dir, "*.conf"))
-
-
-def _renew_describe_results(config, renew_successes, renew_failures,
-                            renew_skipped, parse_failures):
-    status = lambda x, msg: "  " + "\n  ".join(i + " (" + msg +")" for i in x)
-    if config.dry_run:
-        print("** DRY RUN: simulating 'letsencrypt renew' close to cert expiry")
-        print("**          (The test certificates below have not been saved.)")
-    print()
-    if renew_skipped:
-        print("The following certs are not due for renewal yet:")
-        print(status(renew_skipped, "skipped"))
-    if not renew_successes and not renew_failures:
-        print("No renewals were attempted.")
-    elif renew_successes and not renew_failures:
-        print("Congratulations, all renewals succeeded. The following certs "
-              "have been renewed:")
-        print(status(renew_successes, "success"))
-    elif renew_failures and not renew_successes:
-        print("All renewal attempts failed. The following certs could not be "
-              "renewed:")
-        print(status(renew_failures, "failure"))
-    elif renew_failures and renew_successes:
-        print("The following certs were successfully renewed:")
-        print(status(renew_successes, "success"))
-        print("\nThe following certs could not be renewed:")
-        print(status(renew_failures, "failure"))
-
-    if parse_failures:
-        print("\nAdditionally, the following renewal configuration files "
-              "were invalid: ")
-        print(status(parse_failures, "parsefail"))
-
-    if config.dry_run:
-        print("** DRY RUN: simulating 'letsencrypt renew' close to cert expiry")
-        print("**          (The test certificates above have not been saved.)")
-
-
-def renew(config, unused_plugins):
-    """Renew previously-obtained certificates."""
-
-    if config.domains != []:
-        raise errors.Error("Currently, the renew verb is only capable of "
-                           "renewing all installed certificates that are due "
-                           "to be renewed; individual domains cannot be "
-                           "specified with this action. If you would like to "
-                           "renew specific certificates, use the certonly "
-                           "command. The renew verb may provide other options "
-                           "for selecting certificates to renew in the future.")
-    renewer_config = configuration.RenewerConfiguration(config)
-    renew_successes = []
-    renew_failures = []
-    renew_skipped = []
-    parse_failures = []
-    for renewal_file in _renewal_conf_files(renewer_config):
-        print("Processing " + renewal_file)
-        lineage_config = copy.deepcopy(config)
-
-        # Note that this modifies config (to add back the configuration
-        # elements from within the renewal configuration file).
-        try:
-            renewal_candidate = _reconstitute(lineage_config, renewal_file)
-        except Exception as e:  # pylint: disable=broad-except
-            logger.warning("Renewal configuration file %s produced an "
-                           "unexpected error: %s. Skipping.", renewal_file, e)
-            logger.debug("Traceback was:\n%s", traceback.format_exc())
-            parse_failures.append(renewal_file)
-            continue
-
-        try:
-            if renewal_candidate is None:
-                parse_failures.append(renewal_file)
-            else:
-                # XXX: ensure that each call here replaces the previous one
-                zope.component.provideUtility(lineage_config)
-                if should_renew(lineage_config, renewal_candidate):
-                    plugins = plugins_disco.PluginsRegistry.find_all()
-                    from letsencrypt import main
-                    main.obtain_cert(lineage_config, plugins, renewal_candidate)
-                    renew_successes.append(renewal_candidate.fullchain)
-                else:
-                    renew_skipped.append(renewal_candidate.fullchain)
-        except Exception as e:  # pylint: disable=broad-except
-            # obtain_cert (presumably) encountered an unanticipated problem.
-            logger.warning("Attempting to renew cert from %s produced an "
-                           "unexpected error: %s. Skipping.", renewal_file, e)
-            logger.debug("Traceback was:\n%s", traceback.format_exc())
-            renew_failures.append(renewal_candidate.fullchain)
-
-    # Describe all the results
-    _renew_describe_results(config, renew_successes, renew_failures,
-                            renew_skipped, parse_failures)
-
-    if renew_failures or parse_failures:
-        raise errors.Error("{0} renew failure(s), {1} parse failure(s)".format(
-            len(renew_failures), len(parse_failures)))
-    else:
-        logger.debug("no renewal failures")
-
+set_by_cli.detector = None
 
 def read_file(filename, mode="rb"):
     """Returns the given file's contents.
@@ -839,7 +551,7 @@ class HelpfulArgumentParser(object):
     def modify_arg_for_default_detection(self, *args, **kwargs):
         """
         Adding an arg, but ensure that it has a default that evaluates to false,
-        so that _set_by_cli can tell if it was set.  Only called if detect_defaults==True.
+        so that set_by_cli can tell if it was set.  Only called if detect_defaults==True.
 
         :param list *args: the names of this argument flag
         :param dict **kwargs: various argparse settings for this argument
@@ -1108,8 +820,8 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):
     _plugins_parsing(helpful, plugins)
 
     if not detect_defaults:
-        global _parser # pylint: disable=global-statement
-        _parser = helpful
+        global helpful_parser # pylint: disable=global-statement
+        helpful_parser = helpful
     return helpful.parse_args()
 
 
diff --git a/letsencrypt/main.py b/letsencrypt/main.py
index 56725d300..8d59993df 100644
--- a/letsencrypt/main.py
+++ b/letsencrypt/main.py
@@ -20,9 +20,9 @@ from letsencrypt import interfaces
 from letsencrypt import le_util
 from letsencrypt import log
 from letsencrypt import reporter
+from letsencrypt import renew
 from letsencrypt import storage
 
-from letsencrypt.cli import choose_configurator_plugins, _renewal_conf_files, should_renew
 from letsencrypt.display import util as display_util, ops as display_ops
 from letsencrypt.plugins import disco as plugins_disco
 
@@ -184,7 +184,7 @@ def _handle_identical_cert_request(config, cert):
     :rtype: tuple
 
     """
-    if should_renew(config, cert):
+    if renew.should_renew(config, cert):
         return "renew", cert
     if config.reinstall:
         # Set with --reinstall, force an identical certificate to be
@@ -261,7 +261,7 @@ def _find_duplicative_certs(config, domains):
     # Verify the directory is there
     le_util.make_or_verify_dir(configs_dir, mode=0o755, uid=os.geteuid())
 
-    for renewal_file in _renewal_conf_files(cli_config):
+    for renewal_file in renew.renewal_conf_files(cli_config):
         try:
             candidate_lineage = storage.RenewableCert(renewal_file, cli_config)
         except (errors.CertStorageError, IOError):
@@ -404,7 +404,7 @@ def install(config, plugins):
     # this function ...
 
     try:
-        installer, _ = choose_configurator_plugins(config, plugins, "install")
+        installer, _ = cli.choose_configurator_plugins(config, plugins, "install")
     except errors.PluginSelectionError as e:
         return e.message
 
@@ -479,7 +479,7 @@ def run(config, plugins):  # pylint: disable=too-many-branches,too-many-locals
     # TODO: Make run as close to auth + install as possible
     # Possible difficulties: config.csr was hacked into auth
     try:
-        installer, authenticator = choose_configurator_plugins(config, plugins, "run")
+        installer, authenticator = cli.choose_configurator_plugins(config, plugins, "run")
     except errors.PluginSelectionError as e:
         return e.message
 
@@ -509,7 +509,7 @@ def obtain_cert(config, plugins, lineage=None):
     # pylint: disable=too-many-locals
     try:
         # installers are used in auth mode to determine domain names
-        installer, authenticator = choose_configurator_plugins(config, plugins, "certonly")
+        installer, authenticator = cli.choose_configurator_plugins(config, plugins, "certonly")
     except errors.PluginSelectionError as e:
         logger.info("Could not choose appropriate plugin: %s", e)
         raise
@@ -705,7 +705,7 @@ def main(cli_args=sys.argv[1:]):
 # due to issues with import cycles and testing, it lives here
 VERBS = {"auth": obtain_cert, "certonly": obtain_cert,
          "config_changes": config_changes, "everything": run,
-         "install": install, "plugins": plugins_cmd, "renew": cli.renew,
+         "install": install, "plugins": plugins_cmd, "renew": renew.renew,
          "revoke": revoke, "rollback": rollback, "run": run}
 
 
diff --git a/letsencrypt/plugins/common.py b/letsencrypt/plugins/common.py
index 319692344..f6a2c3d76 100644
--- a/letsencrypt/plugins/common.py
+++ b/letsencrypt/plugins/common.py
@@ -52,7 +52,7 @@ class Plugin(object):
         NOTE: if you add argpase arguments such that users setting them can
         create a config entry that python's bool() would consider false (ie,
         the use might set the variable to "", [], 0, etc), please ensure that
-        cli._set_by_cli() works for your variable.
+        cli.set_by_cli() works for your variable.
 
         """
 
diff --git a/letsencrypt/renew.py b/letsencrypt/renew.py
index e69de29bb..6eb3fa27c 100644
--- a/letsencrypt/renew.py
+++ b/letsencrypt/renew.py
@@ -0,0 +1,304 @@
+"""Functionality for autorenewal and associated juggling of configurations"""
+from __future__ import print_function
+import copy
+import glob
+import logging
+import os
+import traceback
+
+import six
+import zope.component
+
+from letsencrypt import configuration
+from letsencrypt import cli
+from letsencrypt import errors
+from letsencrypt import storage
+from letsencrypt.plugins import disco as plugins_disco
+
+logger = logging.getLogger(__name__)
+
+# These are the items which get pulled out of a renewal configuration
+# file's renewalparams and actually used in the client configuration
+# during the renewal process. We have to record their types here because
+# the renewal configuration process loses this information.
+STR_CONFIG_ITEMS = ["config_dir", "logs_dir", "work_dir", "user_agent",
+                    "server", "account", "authenticator", "installer",
+                    "standalone_supported_challenges"]
+INT_CONFIG_ITEMS = ["rsa_key_size", "tls_sni_01_port", "http01_port"]
+
+
+def _renew_describe_results(config, renew_successes, renew_failures,
+                            renew_skipped, parse_failures):
+    status = lambda x, msg: "  " + "\n  ".join(i + " (" + msg +")" for i in x)
+    if config.dry_run:
+        print("** DRY RUN: simulating 'letsencrypt renew' close to cert expiry")
+        print("**          (The test certificates below have not been saved.)")
+    print()
+    if renew_skipped:
+        print("The following certs are not due for renewal yet:")
+        print(status(renew_skipped, "skipped"))
+    if not renew_successes and not renew_failures:
+        print("No renewals were attempted.")
+    elif renew_successes and not renew_failures:
+        print("Congratulations, all renewals succeeded. The following certs "
+              "have been renewed:")
+        print(status(renew_successes, "success"))
+    elif renew_failures and not renew_successes:
+        print("All renewal attempts failed. The following certs could not be "
+              "renewed:")
+        print(status(renew_failures, "failure"))
+    elif renew_failures and renew_successes:
+        print("The following certs were successfully renewed:")
+        print(status(renew_successes, "success"))
+        print("\nThe following certs could not be renewed:")
+        print(status(renew_failures, "failure"))
+
+    if parse_failures:
+        print("\nAdditionally, the following renewal configuration files "
+              "were invalid: ")
+        print(status(parse_failures, "parsefail"))
+
+    if config.dry_run:
+        print("** DRY RUN: simulating 'letsencrypt renew' close to cert expiry")
+        print("**          (The test certificates above have not been saved.)")
+
+
+def renewal_conf_files(config):
+    """Return /path/to/*.conf in the renewal conf directory"""
+    return glob.glob(os.path.join(config.renewal_configs_dir, "*.conf"))
+
+
+def _reconstitute(config, full_path):
+    """Try to instantiate a RenewableCert, updating config with relevant items.
+
+    This is specifically for use in renewal and enforces several checks
+    and policies to ensure that we can try to proceed with the renwal
+    request. The config argument is modified by including relevant options
+    read from the renewal configuration file.
+
+    :param configuration.NamespaceConfig config: configuration for the
+        current lineage
+    :param str full_path: Absolute path to the configuration file that
+        defines this lineage
+
+    :returns: the RenewableCert object or None if a fatal error occurred
+    :rtype: `storage.RenewableCert` or NoneType
+
+    """
+    try:
+        renewal_candidate = storage.RenewableCert(
+            full_path, configuration.RenewerConfiguration(config))
+    except (errors.CertStorageError, IOError):
+        logger.warning("Renewal configuration file %s is broken. Skipping.", full_path)
+        logger.debug("Traceback was:\n%s", traceback.format_exc())
+        return None
+    if "renewalparams" not in renewal_candidate.configuration:
+        logger.warning("Renewal configuration file %s lacks "
+                       "renewalparams. Skipping.", full_path)
+        return None
+    renewalparams = renewal_candidate.configuration["renewalparams"]
+    if "authenticator" not in renewalparams:
+        logger.warning("Renewal configuration file %s does not specify "
+                       "an authenticator. Skipping.", full_path)
+        return None
+    # Now restore specific values along with their data types, if
+    # those elements are present.
+    try:
+        _restore_required_config_elements(config, renewalparams)
+        _restore_plugin_configs(config, renewalparams)
+    except (ValueError, errors.Error) as error:
+        logger.warning(
+            "An error occured while parsing %s. The error was %s. "
+            "Skipping the file.", full_path, error.message)
+        logger.debug("Traceback was:\n%s", traceback.format_exc())
+        return None
+
+    try:
+        for d in renewal_candidate.names():
+            cli.process_domain(config, d)
+    except errors.ConfigurationError as error:
+        logger.warning("Renewal configuration file %s references a cert "
+                       "that contains an invalid domain name. The problem "
+                       "was: %s. Skipping.", full_path, error)
+        return None
+
+    return renewal_candidate
+
+
+def _restore_webroot_config(config, renewalparams):
+    """
+    webroot_map is, uniquely, a dict, and the general-purpose configuration
+    restoring logic is not able to correctly parse it from the serialized
+    form.
+    """
+    if "webroot_map" in renewalparams:
+        # if the user does anything that would create a new webroot map on the
+        # CLI, don't use the old one
+        if not (cli.set_by_cli("webroot_map") or cli.set_by_cli("webroot_path")):
+            setattr(config.namespace, "webroot_map", renewalparams["webroot_map"])
+    elif "webroot_path" in renewalparams:
+        logger.info("Ancient renewal conf file without webroot-map, restoring webroot-path")
+        wp = renewalparams["webroot_path"]
+        if isinstance(wp, str):  # prior to 0.1.0, webroot_path was a string
+            wp = [wp]
+        setattr(config.namespace, "webroot_path", wp)
+
+
+def _restore_plugin_configs(config, renewalparams):
+    """Sets plugin specific values in config from renewalparams
+
+    :param configuration.NamespaceConfig config: configuration for the
+        current lineage
+    :param configobj.Section renewalparams: Parameters from the renewal
+        configuration file that defines this lineage
+
+    """
+    # Now use parser to get plugin-prefixed items with correct types
+    # XXX: the current approach of extracting only prefixed items
+    #      related to the actually-used installer and authenticator
+    #      works as long as plugins don't need to read plugin-specific
+    #      variables set by someone else (e.g., assuming Apache
+    #      configurator doesn't need to read webroot_ variables).
+    # Note: if a parameter that used to be defined in the parser is no
+    #      longer defined, stored copies of that parameter will be
+    #      deserialized as strings by this logic even if they were
+    #      originally meant to be some other type.
+    if renewalparams["authenticator"] == "webroot":
+        _restore_webroot_config(config, renewalparams)
+        plugin_prefixes = []
+    else:
+        plugin_prefixes = [renewalparams["authenticator"]]
+
+    if renewalparams.get("installer", None) is not None:
+        plugin_prefixes.append(renewalparams["installer"])
+    for plugin_prefix in set(plugin_prefixes):
+        for config_item, config_value in six.iteritems(renewalparams):
+            if config_item.startswith(plugin_prefix + "_") and not cli.set_by_cli(config_item):
+                # Values None, True, and False need to be treated specially,
+                # As they don't get parsed correctly based on type
+                if config_value in ("None", "True", "False"):
+                    # bool("False") == True
+                    # pylint: disable=eval-used
+                    setattr(config.namespace, config_item, eval(config_value))
+                    continue
+                # If argparse has a type for this variable, use it:
+                # pylint: disable=protected-access
+                for action in cli.helpful_parser.parser._actions:
+                    if action.type is not None and action.dest == config_item:
+                        setattr(config.namespace, config_item,
+                                action.type(config_value))
+                        break
+                else:
+                    setattr(config.namespace, config_item, str(config_value))
+
+
+def _restore_required_config_elements(config, renewalparams):
+    """Sets non-plugin specific values in config from renewalparams
+
+    :param configuration.NamespaceConfig config: configuration for the
+        current lineage
+    :param configobj.Section renewalparams: parameters from the renewal
+        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)
+
+
+def should_renew(config, lineage):
+    "Return true if any of the circumstances for automatic renewal apply."
+    if config.renew_by_default:
+        logger.info("Auto-renewal forced with --force-renewal...")
+        return True
+    if lineage.should_autorenew(interactive=True):
+        logger.info("Cert is due for renewal, auto-renewing...")
+        return True
+    if config.dry_run:
+        logger.info("Cert not due for renewal, but simulating renewal for dry run")
+        return True
+    logger.info("Cert not yet due for renewal")
+    return False
+
+
+def renew(config, unused_plugins):
+    """Renew previously-obtained certificates."""
+
+    if config.domains != []:
+        raise errors.Error("Currently, the renew verb is only capable of "
+                           "renewing all installed certificates that are due "
+                           "to be renewed; individual domains cannot be "
+                           "specified with this action. If you would like to "
+                           "renew specific certificates, use the certonly "
+                           "command. The renew verb may provide other options "
+                           "for selecting certificates to renew in the future.")
+    renewer_config = configuration.RenewerConfiguration(config)
+    renew_successes = []
+    renew_failures = []
+    renew_skipped = []
+    parse_failures = []
+    for renewal_file in renewal_conf_files(renewer_config):
+        print("Processing " + renewal_file)
+        lineage_config = copy.deepcopy(config)
+
+        # Note that this modifies config (to add back the configuration
+        # elements from within the renewal configuration file).
+        try:
+            renewal_candidate = _reconstitute(lineage_config, renewal_file)
+        except Exception as e:  # pylint: disable=broad-except
+            logger.warning("Renewal configuration file %s produced an "
+                           "unexpected error: %s. Skipping.", renewal_file, e)
+            logger.debug("Traceback was:\n%s", traceback.format_exc())
+            parse_failures.append(renewal_file)
+            continue
+
+        try:
+            if renewal_candidate is None:
+                parse_failures.append(renewal_file)
+            else:
+                # XXX: ensure that each call here replaces the previous one
+                zope.component.provideUtility(lineage_config)
+                if should_renew(lineage_config, renewal_candidate):
+                    plugins = plugins_disco.PluginsRegistry.find_all()
+                    from letsencrypt import main
+                    main.obtain_cert(lineage_config, plugins, renewal_candidate)
+                    renew_successes.append(renewal_candidate.fullchain)
+                else:
+                    renew_skipped.append(renewal_candidate.fullchain)
+        except Exception as e:  # pylint: disable=broad-except
+            # obtain_cert (presumably) encountered an unanticipated problem.
+            logger.warning("Attempting to renew cert from %s produced an "
+                           "unexpected error: %s. Skipping.", renewal_file, e)
+            logger.debug("Traceback was:\n%s", traceback.format_exc())
+            renew_failures.append(renewal_candidate.fullchain)
+
+    # Describe all the results
+    _renew_describe_results(config, renew_successes, renew_failures,
+                            renew_skipped, parse_failures)
+
+    if renew_failures or parse_failures:
+        raise errors.Error("{0} renew failure(s), {1} parse failure(s)".format(
+            len(renew_failures), len(parse_failures)))
+    else:
+        logger.debug("no renewal failures")
diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index c5865206d..e79bcb95a 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -1,5 +1,4 @@
 """Tests for letsencrypt.cli."""
-
 from __future__ import print_function
 
 import argparse
@@ -24,6 +23,7 @@ from letsencrypt import crypto_util
 from letsencrypt import errors
 from letsencrypt import le_util
 from letsencrypt import main
+from letsencrypt import renew
 from letsencrypt import storage
 
 from letsencrypt.plugins import disco
@@ -555,7 +555,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
                           self._certonly_new_request_common, mock_client)
 
     def _test_renewal_common(self, due_for_renewal, extra_args, log_out=None,
-                             args=None, renew=True, error_expected=False):
+                             args=None, should_renew=True, error_expected=False):
         # pylint: disable=too-many-locals,too-many-arguments
         cert_path = 'letsencrypt/tests/testdata/cert.pem'
         chain_path = '/etc/letsencrypt/live/foo.bar/fullchain.pem'
@@ -594,7 +594,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
                                             "Unexpected renewal error:\n" +
                                             traceback.format_exc())
 
-            if renew:
+            if should_renew:
                 mock_client.obtain_certificate.assert_called_once_with(['isnot.org'])
             else:
                 self.assertEqual(mock_client.obtain_certificate.call_count, 0)
@@ -629,7 +629,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         self.assertEqual(get_utility().add_message.call_count, 1)
 
         _, _ = self._test_renewal_common(False, ['-tvv', '--debug', '--keep'],
-                                        log_out="not yet due", renew=False)
+                                        log_out="not yet due", should_renew=False)
 
     def _dump_log(self):
         with open(os.path.join(self.logs_dir, "letsencrypt.log")) as lf:
@@ -652,9 +652,9 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
     def test_renew_verb(self):
         self._make_test_renewal_conf('sample-renewal.conf')
         args = ["renew", "--dry-run", "-tvv"]
-        self._test_renewal_common(True, [], args=args, renew=True)
+        self._test_renewal_common(True, [], args=args, should_renew=True)
 
-    @mock.patch("letsencrypt.cli._set_by_cli")
+    @mock.patch("letsencrypt.cli.set_by_cli")
     def test_ancient_webroot_renewal_conf(self, mock_set_by_cli):
         mock_set_by_cli.return_value = False
         rc_path = self._make_test_renewal_conf('sample-renewal-ancient.conf')
@@ -664,7 +664,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
             configuration.RenewerConfiguration(config))
         renewalparams = lineage.configuration["renewalparams"]
         # pylint: disable=protected-access
-        cli._restore_webroot_config(config, renewalparams)
+        renew._restore_webroot_config(config, renewalparams)
         self.assertEqual(config.webroot_path, ["/var/www/"])
 
     def test_renew_verb_empty_config(self):
@@ -674,7 +674,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         with open(os.path.join(rd, 'empty.conf'), 'w'):
             pass  # leave the file empty
         args = ["renew", "--dry-run", "-tvv"]
-        self._test_renewal_common(False, [], args=args, renew=False, error_expected=True)
+        self._test_renewal_common(False, [], args=args, should_renew=False, error_expected=True)
 
     def _make_dummy_renewal_config(self):
         renewer_configs_dir = os.path.join(self.config_dir, 'renewal')
@@ -695,7 +695,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
             mock_rc.return_value = mock_lineage
             with mock.patch('letsencrypt.main.obtain_cert') as mock_obtain_cert:
                 self._test_renewal_common(True, None, error_expected=error_expected,
-                                          args=['renew'], renew=False)
+                                          args=['renew'], should_renew=False)
             if assert_oc_called is not None:
                 if assert_oc_called:
                     self.assertTrue(mock_obtain_cert.called)
@@ -751,13 +751,13 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
             with mock.patch('letsencrypt.main.obtain_cert') as mock_obtain_cert:
                 mock_obtain_cert.side_effect = Exception
                 self._test_renewal_common(True, None, error_expected=True,
-                                          args=['renew'], renew=False)
+                                          args=['renew'], should_renew=False)
 
     def test_renew_with_bad_cli_args(self):
         self._test_renewal_common(True, None, args='renew -d example.com'.split(),
-                                  renew=False, error_expected=True)
+                                  should_renew=False, error_expected=True)
         self._test_renewal_common(True, None, args='renew --csr {0}'.format(CSR).split(),
-                                  renew=False, error_expected=True)
+                                  should_renew=False, error_expected=True)
 
     @mock.patch('letsencrypt.cli.zope.component.getUtility')
     @mock.patch('letsencrypt.main._treat_as_renewal')

From 4ca25828b25c6f3ee3d5ce0591ccdbb4718f24e1 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 11 Mar 2016 13:46:37 -0800
Subject: [PATCH 2/8] Get tests passing

---
 letsencrypt/cli.py            | 1 -
 letsencrypt/plugins/disco.py  | 1 +
 letsencrypt/tests/cli_test.py | 8 ++++----
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index e34bd971b..bea8b198d 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -1,5 +1,4 @@
 """Let's Encrypt command CLI argument processing."""
-# pylint: disable=too-many-lines
 from __future__ import print_function
 import argparse
 import glob
diff --git a/letsencrypt/plugins/disco.py b/letsencrypt/plugins/disco.py
index 9ed6ac596..27d2fb541 100644
--- a/letsencrypt/plugins/disco.py
+++ b/letsencrypt/plugins/disco.py
@@ -4,6 +4,7 @@ import logging
 import pkg_resources
 
 import zope.interface
+import zope.interface.verify
 
 from letsencrypt import constants
 from letsencrypt import errors
diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index e79bcb95a..f1f539016 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -517,7 +517,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
                 args += '-d foo.bar -a standalone certonly'.split()
                 self._call(args)
 
-    @mock.patch('letsencrypt.cli.zope.component.getUtility')
+    @mock.patch('letsencrypt.main.zope.component.getUtility')
     def test_certonly_dry_run_new_request_success(self, mock_get_utility):
         mock_client = mock.MagicMock()
         mock_client.obtain_and_enroll_certificate.return_value = None
@@ -530,7 +530,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         self.assertEqual(mock_get_utility().add_message.call_count, 1)
 
     @mock.patch('letsencrypt.crypto_util.notAfter')
-    @mock.patch('letsencrypt.cli.zope.component.getUtility')
+    @mock.patch('letsencrypt.main.zope.component.getUtility')
     def test_certonly_new_request_success(self, mock_get_utility, mock_notAfter):
         cert_path = '/etc/letsencrypt/live/foo.bar'
         date = '1970-01-01'
@@ -736,7 +736,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
 
     def test_renew_reconstitute_error(self):
         # pylint: disable=protected-access
-        with mock.patch('letsencrypt.cli._reconstitute') as mock_reconstitute:
+        with mock.patch('letsencrypt.main.renew._reconstitute') as mock_reconstitute:
             mock_reconstitute.side_effect = Exception
             self._test_renew_common(assert_oc_called=False, error_expected=True)
 
@@ -759,7 +759,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         self._test_renewal_common(True, None, args='renew --csr {0}'.format(CSR).split(),
                                   should_renew=False, error_expected=True)
 
-    @mock.patch('letsencrypt.cli.zope.component.getUtility')
+    @mock.patch('letsencrypt.main.zope.component.getUtility')
     @mock.patch('letsencrypt.main._treat_as_renewal')
     @mock.patch('letsencrypt.main._init_le_client')
     def test_certonly_reinstall(self, mock_init, mock_renewal, mock_get_utility):

From e644560a55087a5c0fbef7aff25d9c5ab7bec134 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 11 Mar 2016 14:00:57 -0800
Subject: [PATCH 3/8] neaten

---
 letsencrypt/renew.py | 72 ++++++++++++++++++++++----------------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/letsencrypt/renew.py b/letsencrypt/renew.py
index 6eb3fa27c..bd00543c8 100644
--- a/letsencrypt/renew.py
+++ b/letsencrypt/renew.py
@@ -27,42 +27,6 @@ STR_CONFIG_ITEMS = ["config_dir", "logs_dir", "work_dir", "user_agent",
 INT_CONFIG_ITEMS = ["rsa_key_size", "tls_sni_01_port", "http01_port"]
 
 
-def _renew_describe_results(config, renew_successes, renew_failures,
-                            renew_skipped, parse_failures):
-    status = lambda x, msg: "  " + "\n  ".join(i + " (" + msg +")" for i in x)
-    if config.dry_run:
-        print("** DRY RUN: simulating 'letsencrypt renew' close to cert expiry")
-        print("**          (The test certificates below have not been saved.)")
-    print()
-    if renew_skipped:
-        print("The following certs are not due for renewal yet:")
-        print(status(renew_skipped, "skipped"))
-    if not renew_successes and not renew_failures:
-        print("No renewals were attempted.")
-    elif renew_successes and not renew_failures:
-        print("Congratulations, all renewals succeeded. The following certs "
-              "have been renewed:")
-        print(status(renew_successes, "success"))
-    elif renew_failures and not renew_successes:
-        print("All renewal attempts failed. The following certs could not be "
-              "renewed:")
-        print(status(renew_failures, "failure"))
-    elif renew_failures and renew_successes:
-        print("The following certs were successfully renewed:")
-        print(status(renew_successes, "success"))
-        print("\nThe following certs could not be renewed:")
-        print(status(renew_failures, "failure"))
-
-    if parse_failures:
-        print("\nAdditionally, the following renewal configuration files "
-              "were invalid: ")
-        print(status(parse_failures, "parsefail"))
-
-    if config.dry_run:
-        print("** DRY RUN: simulating 'letsencrypt renew' close to cert expiry")
-        print("**          (The test certificates above have not been saved.)")
-
-
 def renewal_conf_files(config):
     """Return /path/to/*.conf in the renewal conf directory"""
     return glob.glob(os.path.join(config.renewal_configs_dir, "*.conf"))
@@ -242,6 +206,42 @@ def should_renew(config, lineage):
     return False
 
 
+def _renew_describe_results(config, renew_successes, renew_failures,
+                            renew_skipped, parse_failures):
+    status = lambda ms, category: "  " + "\n  ".join(m + " (%s)" % category for m in ms)
+    if config.dry_run:
+        print("** DRY RUN: simulating 'letsencrypt renew' close to cert expiry")
+        print("**          (The test certificates below have not been saved.)")
+    print()
+    if renew_skipped:
+        print("The following certs are not due for renewal yet:")
+        print(status(renew_skipped, "skipped"))
+    if not renew_successes and not renew_failures:
+        print("No renewals were attempted.")
+    elif renew_successes and not renew_failures:
+        print("Congratulations, all renewals succeeded. The following certs "
+              "have been renewed:")
+        print(status(renew_successes, "success"))
+    elif renew_failures and not renew_successes:
+        print("All renewal attempts failed. The following certs could not be "
+              "renewed:")
+        print(status(renew_failures, "failure"))
+    elif renew_failures and renew_successes:
+        print("The following certs were successfully renewed:")
+        print(status(renew_successes, "success"))
+        print("\nThe following certs could not be renewed:")
+        print(status(renew_failures, "failure"))
+
+    if parse_failures:
+        print("\nAdditionally, the following renewal configuration files "
+              "were invalid: ")
+        print(status(parse_failures, "parsefail"))
+
+    if config.dry_run:
+        print("** DRY RUN: simulating 'letsencrypt renew' close to cert expiry")
+        print("**          (The test certificates above have not been saved.)")
+
+
 def renew(config, unused_plugins):
     """Renew previously-obtained certificates."""
 

From 6f7f036e2cac155b3bfddd614fb603bd7ce0ac9d Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 11 Mar 2016 14:07:35 -0800
Subject: [PATCH 4/8] Neaten further

---
 letsencrypt/renew.py | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/letsencrypt/renew.py b/letsencrypt/renew.py
index bd00543c8..367eda5b0 100644
--- a/letsencrypt/renew.py
+++ b/letsencrypt/renew.py
@@ -208,34 +208,35 @@ def should_renew(config, lineage):
 
 def _renew_describe_results(config, renew_successes, renew_failures,
                             renew_skipped, parse_failures):
-    status = lambda ms, category: "  " + "\n  ".join(m + " (%s)" % category for m in ms)
+    def _status(msgs, category):
+        return "  " + "\n  ".join("%s (%s)" % (m, category) for m in msgs)
     if config.dry_run:
         print("** DRY RUN: simulating 'letsencrypt renew' close to cert expiry")
         print("**          (The test certificates below have not been saved.)")
     print()
     if renew_skipped:
         print("The following certs are not due for renewal yet:")
-        print(status(renew_skipped, "skipped"))
+        print(_status(renew_skipped, "skipped"))
     if not renew_successes and not renew_failures:
         print("No renewals were attempted.")
     elif renew_successes and not renew_failures:
         print("Congratulations, all renewals succeeded. The following certs "
               "have been renewed:")
-        print(status(renew_successes, "success"))
+        print(_status(renew_successes, "success"))
     elif renew_failures and not renew_successes:
         print("All renewal attempts failed. The following certs could not be "
               "renewed:")
-        print(status(renew_failures, "failure"))
+        print(_status(renew_failures, "failure"))
     elif renew_failures and renew_successes:
         print("The following certs were successfully renewed:")
-        print(status(renew_successes, "success"))
+        print(_status(renew_successes, "success"))
         print("\nThe following certs could not be renewed:")
-        print(status(renew_failures, "failure"))
+        print(_status(renew_failures, "failure"))
 
     if parse_failures:
         print("\nAdditionally, the following renewal configuration files "
               "were invalid: ")
-        print(status(parse_failures, "parsefail"))
+        print(_status(parse_failures, "parsefail"))
 
     if config.dry_run:
         print("** DRY RUN: simulating 'letsencrypt renew' close to cert expiry")

From 86ab35df4f66dfc15a99cf0c581b9100b77cc643 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 11 Mar 2016 15:07:13 -0800
Subject: [PATCH 5/8] Move argparse type extraction back into cli.py

---
 letsencrypt/cli.py   | 14 ++++++++++++++
 letsencrypt/renew.py | 13 +++----------
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index bea8b198d..017e0a62e 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -283,6 +283,16 @@ def set_by_cli(var):
 # static housekeeping var
 set_by_cli.detector = None
 
+
+def argparse_type(variable):
+    "Return our argparse type function for a config variable (default: str)"
+    # pylint: disable=protected-access
+    for action in helpful_parser.parser._actions:
+        if action.type is not None and action.dest == variable:
+            return action.type
+    return str
+
+
 def read_file(filename, mode="rb"):
     """Returns the given file's contents.
 
@@ -304,6 +314,10 @@ def read_file(filename, mode="rb"):
 
 def flag_default(name):
     """Default value for CLI flag."""
+    # XXX: this is an internal housekeeping notion of defaults before
+    # argparse has been set up; it is not accurate for all flags.  Call it
+    # with caution.  Plugin defaults are missing, and some things are using
+    # defaults defined in this file, not in constants.py :(
     return constants.CLI_DEFAULTS[name]
 
 
diff --git a/letsencrypt/renew.py b/letsencrypt/renew.py
index 367eda5b0..9dc1ff4df 100644
--- a/letsencrypt/renew.py
+++ b/letsencrypt/renew.py
@@ -139,21 +139,14 @@ def _restore_plugin_configs(config, renewalparams):
         for config_item, config_value in six.iteritems(renewalparams):
             if config_item.startswith(plugin_prefix + "_") and not cli.set_by_cli(config_item):
                 # Values None, True, and False need to be treated specially,
-                # As they don't get parsed correctly based on type
+                # As their types aren't handled correctly by configobj
                 if config_value in ("None", "True", "False"):
                     # bool("False") == True
                     # pylint: disable=eval-used
                     setattr(config.namespace, config_item, eval(config_value))
-                    continue
-                # If argparse has a type for this variable, use it:
-                # pylint: disable=protected-access
-                for action in cli.helpful_parser.parser._actions:
-                    if action.type is not None and action.dest == config_item:
-                        setattr(config.namespace, config_item,
-                                action.type(config_value))
-                        break
                 else:
-                    setattr(config.namespace, config_item, str(config_value))
+                    cast = cli.argparse_type(config_item)
+                    setattr(config.namespace, config_item, cast(config_value))
 
 
 def _restore_required_config_elements(config, renewalparams):

From 0847c73fd8c6626e3d64f99930cef934224a1649 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 17 Mar 2016 16:22:29 -0700
Subject: [PATCH 6/8] Fix dependency issue

---
 letsencrypt/cli.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 9bbbe923a..0ba5d321e 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -355,10 +355,11 @@ class HelpfulArgumentParser(object):
 
     def __init__(self, args, plugins, detect_defaults=False):
         from letsencrypt import main
+        from letsencrypt import renew
         self.VERBS = {"auth": main.obtain_cert, "certonly": main.obtain_cert,
                       "config_changes": main.config_changes, "run": main.run,
                       "install": main.install, "plugins": main.plugins_cmd,
-                      "renew": renew, "revoke": main.revoke,
+                      "renew": renew.renew, "revoke": main.revoke,
                       "rollback": main.rollback, "everything": main.run}
 
         # List of topics for which additional help can be provided

From 278b8852fd3844f66af03d3ee6586ac10aee6c32 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 23 Mar 2016 14:26:58 -0700
Subject: [PATCH 7/8] Merge snafu

---
 letsencrypt/tests/cli_test.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index adbe72701..55a504f82 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -54,7 +54,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         shutil.rmtree(self.tmp_dir)
         # Reset globals in cli
         # pylint: disable=protected-access
-        cli._parser = cli._set_by_cli.detector = None
+        cli._parser = cli.set_by_cli.detector = None
 
     def _call(self, args):
         "Run the cli with output streams and actual client mocked out"

From a9faa3b4ba9b3353e3eb66dda48e303dc881312c Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Wed, 23 Mar 2016 16:14:41 -0700
Subject: [PATCH 8/8] Shim renew() in main.py, keep the work in renewal.py

---
 letsencrypt/cli.py                   |  3 +--
 letsencrypt/main.py                  | 11 ++++++++---
 letsencrypt/{renew.py => renewal.py} |  4 ++--
 letsencrypt/tests/cli_test.py        |  6 +++---
 4 files changed, 14 insertions(+), 10 deletions(-)
 rename letsencrypt/{renew.py => renewal.py} (99%)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 933b51432..bc39c3a8d 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -355,11 +355,10 @@ class HelpfulArgumentParser(object):
 
     def __init__(self, args, plugins, detect_defaults=False):
         from letsencrypt import main
-        from letsencrypt import renew
         self.VERBS = {"auth": main.obtain_cert, "certonly": main.obtain_cert,
                       "config_changes": main.config_changes, "run": main.run,
                       "install": main.install, "plugins": main.plugins_cmd,
-                      "renew": renew.renew, "revoke": main.revoke,
+                      "renew": main.renew, "revoke": main.revoke,
                       "rollback": main.rollback, "everything": main.run}
 
         # List of topics for which additional help can be provided
diff --git a/letsencrypt/main.py b/letsencrypt/main.py
index 90dfb456a..a3ebde64e 100644
--- a/letsencrypt/main.py
+++ b/letsencrypt/main.py
@@ -27,7 +27,7 @@ from letsencrypt import interfaces
 from letsencrypt import le_util
 from letsencrypt import log
 from letsencrypt import reporter
-from letsencrypt import renew
+from letsencrypt import renewal
 from letsencrypt import storage
 
 from letsencrypt.display import util as display_util, ops as display_ops
@@ -186,7 +186,7 @@ def _handle_identical_cert_request(config, cert):
     :rtype: tuple
 
     """
-    if renew.should_renew(config, cert):
+    if renewal.should_renew(config, cert):
         return "renew", cert
     if config.reinstall:
         # Set with --reinstall, force an identical certificate to be
@@ -263,7 +263,7 @@ def _find_duplicative_certs(config, domains):
     # Verify the directory is there
     le_util.make_or_verify_dir(configs_dir, mode=0o755, uid=os.geteuid())
 
-    for renewal_file in renew.renewal_conf_files(cli_config):
+    for renewal_file in renewal.renewal_conf_files(cli_config):
         try:
             candidate_lineage = storage.RenewableCert(renewal_file, cli_config)
         except (errors.CertStorageError, IOError):
@@ -552,6 +552,11 @@ def obtain_cert(config, plugins, lineage=None):
                   config.installer, "server; fullchain is", lineage.fullchain)
     _suggest_donation_if_appropriate(config, action)
 
+def renew(config, unused_plugins):
+    """Renew previously-obtained certificates."""
+    renewal.renew_all_lineages(config)
+
+
 
 def setup_log_file_handler(config, logfile, fmt):
     """Setup file debug logging."""
diff --git a/letsencrypt/renew.py b/letsencrypt/renewal.py
similarity index 99%
rename from letsencrypt/renew.py
rename to letsencrypt/renewal.py
index 9dc1ff4df..27546bec9 100644
--- a/letsencrypt/renew.py
+++ b/letsencrypt/renewal.py
@@ -236,8 +236,8 @@ def _renew_describe_results(config, renew_successes, renew_failures,
         print("**          (The test certificates above have not been saved.)")
 
 
-def renew(config, unused_plugins):
-    """Renew previously-obtained certificates."""
+def renew_all_lineages(config):
+    """Examine each lineage; renew if due and report results"""
 
     if config.domains != []:
         raise errors.Error("Currently, the renew verb is only capable of "
diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index 55a504f82..0c2bf5d54 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -23,7 +23,7 @@ from letsencrypt import crypto_util
 from letsencrypt import errors
 from letsencrypt import le_util
 from letsencrypt import main
-from letsencrypt import renew
+from letsencrypt import renewal
 from letsencrypt import storage
 
 from letsencrypt.plugins import disco
@@ -666,7 +666,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
             configuration.RenewerConfiguration(config))
         renewalparams = lineage.configuration["renewalparams"]
         # pylint: disable=protected-access
-        renew._restore_webroot_config(config, renewalparams)
+        renewal._restore_webroot_config(config, renewalparams)
         self.assertEqual(config.webroot_path, ["/var/www/"])
 
     def test_renew_verb_empty_config(self):
@@ -745,7 +745,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
 
     def test_renew_reconstitute_error(self):
         # pylint: disable=protected-access
-        with mock.patch('letsencrypt.main.renew._reconstitute') as mock_reconstitute:
+        with mock.patch('letsencrypt.main.renewal._reconstitute') as mock_reconstitute:
             mock_reconstitute.side_effect = Exception
             self._test_renew_common(assert_oc_called=False, error_expected=True)