merge with a recent branch of renew_verb

letsencrypt/cli.py

@@ -175,7 +175,7 @@ def _determine_account(config):
             acc = accounts[0]
         else:  # no account registered yet
             if config.email is None and not config.register_unsafely_without_email:
-                config.email = display_ops.get_email()
+                config.namespace.email = display_ops.get_email()
 
             def _tos_cb(regr):
                 if config.tos:
@@ -197,7 +197,7 @@ def _determine_account(config):
                 raise errors.Error(
                     "Unable to register an account with ACME server")
 
-    config.account = acc.id
+    config.namespace.account = acc.id
     return acc, acme
 
 
@@ -738,7 +738,15 @@ def _diff_from_default(default_conf, cli_conf, value):
     else:
         return False
 
-def _restore_required_config_elements(full_path, config, renewalparams, cli_config, default_conf):
+def _restore_required_config_elements(config, renewalparams, cli_config, default_conf):
+    """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 _diff_from_default(default_conf, cli_config, config_item):
@@ -755,12 +763,18 @@ def _restore_required_config_elements(full_path, config, renewalparams, cli_conf
                 value = int(renewalparams[config_item])
                 setattr(config.namespace, config_item, value)
             except ValueError:
-                logger.warning("Renewal configuration file %s specifies "
-                               "a non-numeric value for %s. Skipping.",
-                               full_path, config_item)
-                raise
+                raise errors.Error(
+                    "Expected a numeric value for {0}".format(config_item))
 
 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
@@ -793,7 +807,7 @@ def _restore_plugin_configs(config, renewalparams):
     return True
 
 
-def _reconstitute(full_path, config, cli_config, default_conf):
+def _reconstitute(config, full_path, cli_config, default_conf):
     """Try to instantiate a RenewableCert, updating config with relevant items.
 
     This is specifically for use in renewal and enforces several checks
@@ -801,15 +815,21 @@ def _reconstitute(full_path, config, cli_config, default_conf):
     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, config)
+        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.info("Traceback was:\n%s", traceback.format_exc())
+        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 "
@@ -823,11 +843,13 @@ def _reconstitute(full_path, config, cli_config, default_conf):
     # Now restore specific values along with their data types, if
     # those elements are present.
     try:
-        _restore_required_config_elements(full_path, config, renewalparams, cli_config, default_conf)
+        _restore_required_config_elements(config, renewalparams, cli_config, default_conf)
         _restore_plugin_configs(config, renewalparams)
-    except ValueError:
-        # There was a data type error which has already been
-        # logged.
+    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
 
     # webroot_map is, uniquely, a dict, and the general-purpose
@@ -855,10 +877,9 @@ def _renewal_conf_files(config):
     return glob.glob(os.path.join(config.renewal_configs_dir, "*.conf"))
 
 
-def renew(cli_config, unused_plugins):
+def renew(config, unused_plugins):
     """Renew previously-obtained certificates."""
-    cli_config = configuration.RenewerConfiguration(cli_config)
-    if cli_config.domains != []:
+    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 "
@@ -866,29 +887,30 @@ def renew(cli_config, unused_plugins):
                            "renew specific certificates, use the certonly "
                            "command. The renew verb may provide other options "
                            "for selecting certificates to renew in the future.")
-    if cli_config.csr is not None:
+    if config.csr is not None:
         raise errors.Error("Currently, the renew verb cannot be used when "
                            "specifying a CSR file. Please try the certonly "
                            "command instead.")
-    for renewal_file in _renewal_conf_files(cli_config):
+    renewer_config = configuration.RenewerConfiguration(config)
+    for renewal_file in _renewal_conf_files(renewer_config):
         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))
         default_args = prepare_and_parse_args(plugins, [])
         default_conf = configuration.NamespaceConfig(default_args)
+        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(renewal_file, config, cli_config, default_conf)
+            renewal_candidate = _reconstitute(lineage_config, renewal_file, config, default_conf)
         except Exception as e: # pylint: disable=broad-except
             # reconstitute encountered an unanticipated problem.
             logger.warning("Renewal configuration file %s produced an "
                            "unexpected error: %s. Skipping.", renewal_file, e)
-            logger.info("Traceback was:\n%s", traceback.format_exc())
+            logger.debug("Traceback was:\n%s", traceback.format_exc())
             continue
 
         if renewal_candidate is None:
@@ -896,14 +918,14 @@ def renew(cli_config, unused_plugins):
             # already been logged. Go on to the next config.
             continue
         # XXX: ensure that each call here replaces the previous one
-        zope.component.provideUtility(config)
+        zope.component.provideUtility(lineage_config)
 
         print("Trying...")
         # Because obtain_cert itself indirectly decides whether to renew
         # or not, we couldn't currently make a UI/logging distinction at
         # this stage to indicate whether renewal was actually attempted
         # (or successful).
-        obtain_cert(config, plugins_disco.PluginsRegistry.find_all(),
+        obtain_cert(lineage_config, plugins_disco.PluginsRegistry.find_all(),
                     renewal_candidate)
 
 def revoke(config, unused_plugins):  # TODO: coop with renewal config
@@ -1720,6 +1742,7 @@ def main(cli_args=sys.argv[1:]):
     elif config.text_mode:
         displayer = display_util.FileDisplay(sys.stdout)
     elif config.verb == "renew":
+        config.noninteractive_mode = True
         displayer = display_util.NoninteractiveDisplay(sys.stdout)
     else:
         displayer = display_util.NcursesDisplay()

letsencrypt/configuration.py

@@ -1,4 +1,5 @@
 """Let's Encrypt user-supplied configuration."""
+import copy
 import os
 import urlparse
 
@@ -78,21 +79,22 @@ class NamespaceConfig(object):
         return os.path.join(
             self.namespace.work_dir, constants.TEMP_CHECKPOINT_DIR)
 
+    def __deepcopy__(self, _memo):
+        # Work around https://bugs.python.org/issue1515 for py26 tests :( :(
+        # https://travis-ci.org/letsencrypt/letsencrypt/jobs/106900743#L3276
+        new_ns = copy.deepcopy(self.namespace)
+        return type(self)(new_ns)
+
 
 class RenewerConfiguration(object):
     """Configuration wrapper for renewer."""
 
     def __init__(self, namespace):
         self.namespace = namespace
-        # We're done setting up the attic. Now pull up the ladder after ourselves...
-        self.__setattr__ = self.__setattr_implementation__
 
     def __getattr__(self, name):
         return getattr(self.namespace, name)
 
-    def __setattr_implementation__(self, var, value):
-        return self.namespace.__setattr__(var, value)
-
     @property
     def archive_dir(self):  # pylint: disable=missing-docstring
         return os.path.join(self.namespace.config_dir, constants.ARCHIVE_DIR)

letsencrypt/tests/cli_test.py

@@ -32,9 +32,6 @@ CERT = test_util.vector_path('cert.pem')
 CSR = test_util.vector_path('csr.der')
 KEY = test_util.vector_path('rsa256_key.pem')
 
-def hack(x):
-    return x
-
 
 class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
     """Tests for different commands."""
@@ -573,6 +570,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
                 self.assertEqual(mock_client.obtain_certificate.call_count, 0)
         except:
             self._dump_log()
+            raise
 
         return mock_lineage, mock_get_utility
 
@@ -605,11 +603,7 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
             print lf.read()
 
 
-    # Work around https://bugs.python.org/issue1515 for py26 tests :( :(
-    # https://travis-ci.org/letsencrypt/letsencrypt/jobs/106900743#L3276
-    @mock.patch('letsencrypt.cli.copy.deepcopy')
-    def test_renewal_verb(self, hack_copy):
-
+    def test_renewal_verb(self):
         with open(test_util.vector_path('sample-renewal.conf')) as src:
             # put the correct path for cert.pem, chain.pem etc in the renewal conf
             renewal_conf = src.read().replace("MAGICDIR", test_util.vector_path())
@@ -618,7 +612,6 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         rc = os.path.join(rd, "sample-renewal.conf")
         with open(rc, "w") as dest:
             dest.write(renewal_conf)
-        hack_copy.side_effect = hack
         args = ["renew", "--dry-run", "-tvv"]
         self._test_renewal_common(True, [], args=args, renew=True)