diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 31f83bfe6..ae09b45e0 100644 --- a/letsencrypt/cli.py +++ b/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() diff --git a/letsencrypt/configuration.py b/letsencrypt/configuration.py index 04053c8c3..2bbf1b019 100644 --- a/letsencrypt/configuration.py +++ b/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) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index fd00c4465..a5757399e 100644 --- a/letsencrypt/tests/cli_test.py +++ b/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)