Split out _reconstitute() from renew()

letsencrypt/cli.py

@@ -726,45 +726,33 @@ def install(config, plugins):
     le_client.enhance_config(domains, config)
 
 
-def renew(cli_config, plugins):
+def _reconstitute(full_path, config):
-    """Renew previously-obtained certificates."""
+    """Try to instantiate a RenewableCert, updating config with relevant items.
-    cli_config = configuration.RenewerConfiguration(cli_config)
-    if cli_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.")
-    configs_dir = cli_config.renewal_configs_dir
-    for renewal_file in reversed(os.listdir(configs_dir)):
-        if not renewal_file.endswith(".conf"):
-            continue
-        print("Processing " + renewal_file)
-        # XXX: does this succeed in making a fully independent config object
-        #      each time?
-        config = configuration.RenewerConfiguration(copy.deepcopy(cli_config))
-        config.noninteractive_mode = True
-        full_path = os.path.join(configs_dir, renewal_file)
 
+    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.
+
+    :returns: the RenewableCert object or None if a fatal error occurred
+    :rtype: `storage.RenewableCert` or NoneType
+    """
 
     try:
         renewal_candidate = storage.RenewableCert(full_path, config)
     except (errors.CertStorageError, IOError):
         logger.warning("Renewal configuration file %s is broken. "
                        "Skipping.", full_path)
-            continue
+        return None
     if "renewalparams" not in renewal_candidate.configuration:
         logger.warning("Renewal configuration file %s lacks "
                        "renewalparams. Skipping.", full_path)
-            continue
+        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)
-            continue
+        return None
-        # XXX: also need: nginx_, apache_, and plesk_ items
     # string-valued items to add if they're present
     for config_item in STR_CONFIG_ITEMS:
         if config_item in renewalparams:
@@ -784,7 +772,7 @@ def renew(cli_config, plugins):
                 logger.warning("Renewal configuration file %s specifies "
                                "a non-numeric value for %s. Skipping.",
                                full_path, config_item)
-                    continue
+                return None
     # 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
@@ -799,8 +787,11 @@ def renew(cli_config, plugins):
     for plugin_prefix in set(renewalparams):
         for config_item in renewalparams.keys():
             if renewalparams[config_item] == "None":
-                    # Avoid confusion when, for example, csr = None (avoid
+                # Avoid confusion when, for example, "csr = None" (avoid
                 # trying to read the file called "None")
+                # Should we omit the item entirely rather than setting
+                # its value to None?
+                config.__setattr__(config_item, None)
                 continue
             if config_item.startswith(plugin_prefix + "_"):
                 for action in _parser.parser._actions:
@@ -814,8 +805,7 @@ def renew(cli_config, plugins):
     # to correctly parse it from the serialized form.
     if "webroot_map" in renewalparams:
         config.__setattr__("webroot_map", renewalparams["webroot_map"])
-        # XXX: ensure that each call here replaces the previous one
+
-        zope.component.provideUtility(config)
     try:
         domains = [le_util.enforce_domain_sanity(x) for x in
                    renewal_candidate.names()]
@@ -823,9 +813,50 @@ def renew(cli_config, plugins):
         logger.warning("Renewal configuration file %s references a cert "
                        "that mentions a domain name that we regarded as "
                        "invalid. Skipping.", full_path)
-            continue
+        return None
 
     config.__setattr__("domains", domains)
+    # XXX: ensure that each call here replaces the previous one
+    zope.component.provideUtility(config)
+    return renewal_candidate
+
+
+def renew(cli_config, plugins):
+    """Renew previously-obtained certificates."""
+    cli_config = configuration.RenewerConfiguration(cli_config)
+    if cli_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.")
+    configs_dir = cli_config.renewal_configs_dir
+    for renewal_file in os.listdir(configs_dir):
+        if not renewal_file.endswith(".conf"):
+            continue
+        print("Processing " + renewal_file)
+        # XXX: does this succeed in making a fully independent config object
+        #      each time?
+        config = configuration.RenewerConfiguration(copy.deepcopy(cli_config))
+        config.noninteractive_mode = True
+        full_path = os.path.join(configs_dir, renewal_file)
+
+        # Note that this modifies config (to add back the configuration
+        # elements from within the renewal configuration file).
+        try:
+            renewal_candidate = _reconstitute(full_path, config)
+        except Exception as e:
+            # reconstitute encountered an unanticipated problem.
+            logger.warning("Renewal configuration file %s produced an "
+                           "unexpected error: %s. Skipping.", full_path, e)
+            continue
+
+        if renewal_candidate is None:
+            # reconstitute indicated an error or problem which has
+            # already been logged. Go on to the next config.
+            continue
 
         print("Trying...")
         # Because obtain_cert itself indirectly decides whether to renew