Move ConfigObj parsing inside of RenewableCert.__init__

This commit is contained in:
Brad Warren 2015-10-13 18:25:31 -07:00
parent acc44f2b65
commit 0e6d652ce3
5 changed files with 62 additions and 90 deletions

View file

@ -12,7 +12,6 @@ import time
import traceback
import configargparse
import configobj
import OpenSSL
import zope.component
import zope.interface.exceptions
@ -167,25 +166,21 @@ def _init_le_client(args, config, authenticator, installer):
return client.Client(config, acc, authenticator, installer, acme=acme)
def _find_duplicative_certs(domains, config, renew_config):
def _find_duplicative_certs(config, domains):
"""Find existing certs that duplicate the request."""
identical_names_cert, subset_names_cert = None, None
configs_dir = renew_config.renewal_configs_dir
cli_config = configuration.RenewerConfiguration(config)
configs_dir = cli_config.renewal_configs_dir
# Verify the directory is there
le_util.make_or_verify_dir(configs_dir, mode=0o755, uid=os.geteuid())
cli_config = configuration.RenewerConfiguration(config)
for renewal_file in os.listdir(configs_dir):
try:
full_path = os.path.join(configs_dir, renewal_file)
rc_config = configobj.ConfigObj(renew_config.renewer_config_file)
rc_config.merge(configobj.ConfigObj(full_path))
rc_config.filename = full_path
candidate_lineage = storage.RenewableCert(
rc_config, config_opts=None, cli_config=cli_config)
except (configobj.ConfigObjError, errors.CertStorageError, IOError):
candidate_lineage = storage.RenewableCert(full_path, cli_config)
except (errors.CertStorageError, IOError):
logger.warning("Renewal configuration file %s is broken. "
"Skipping.", full_path)
continue
@ -217,7 +212,7 @@ def _treat_as_renewal(config, domains):
# kind of certificate to be obtained with renewal = False.)
if not config.duplicate:
ident_names_cert, subset_names_cert = _find_duplicative_certs(
domains, config, configuration.RenewerConfiguration(config))
config, domains)
# I am not sure whether that correctly reads the systemwide
# configuration file.
question = None

View file

@ -12,7 +12,6 @@ import logging
import os
import sys
import configobj
import OpenSSL
import zope.component
@ -141,7 +140,7 @@ def _create_parser():
return _paths_parser(parser)
def main(config=None, cli_args=sys.argv[1:]):
def main(cli_args=sys.argv[1:]):
"""Main function for autorenewer script."""
# TODO: Distinguish automated invocation from manual invocation,
# perhaps by looking at sys.argv[0] and inhibiting automated
@ -149,6 +148,11 @@ def main(config=None, cli_args=sys.argv[1:]):
# turned it off. (The boolean parameter should probably be
# called renewer_enabled.)
# TODO: When we have a more elaborate renewer command line, we will
# presumably also be able to specify a config file on the
# command line, which, if provided, should take precedence over
# te default config files
zope.component.provideUtility(display_util.FileDisplay(sys.stdout))
args = _create_parser().parse_args(cli_args)
@ -159,28 +163,12 @@ def main(config=None, cli_args=sys.argv[1:]):
cli_config = configuration.RenewerConfiguration(args)
config = storage.config_with_defaults(config)
# Now attempt to read the renewer config file and augment or replace
# the renewer defaults with any options contained in that file. If
# renewer_config_file is undefined or if the file is nonexistent or
# empty, this .merge() will have no effect. TODO: when we have a more
# elaborate renewer command line, we will presumably also be able to
# specify a config file on the command line, which, if provided, should
# take precedence over this one.
config.merge(configobj.ConfigObj(cli_config.renewer_config_file))
# Ensure that all of the needed folders have been created before continuing
le_util.make_or_verify_dir(cli_config.work_dir,
constants.CONFIG_DIRS_MODE, uid)
for i in os.listdir(cli_config.renewal_configs_dir):
print "Processing", i
if not i.endswith(".conf"):
continue
rc_config = configobj.ConfigObj(cli_config.renewer_config_file)
rc_config.merge(configobj.ConfigObj(
os.path.join(cli_config.renewal_configs_dir, i)))
# TODO: this is a dirty hack!
rc_config.filename = os.path.join(cli_config.renewal_configs_dir, i)
for renewal_file in os.listdir(cli_config.renewal_configs_dir):
print "Processing", renewal_file
try:
# TODO: Before trying to initialize the RenewableCert object,
# we could check here whether the combination of the config
@ -190,7 +178,7 @@ def main(config=None, cli_args=sys.argv[1:]):
# RenewableCert object for this cert at all, which could
# dramatically improve performance for large deployments
# where autorenewal is widely turned off.
cert = storage.RenewableCert(rc_config, cli_config=cli_config)
cert = storage.RenewableCert(renewal_file, cli_config)
except errors.CertStorageError:
# This indicates an invalid renewal configuration file, such
# as one missing a required parameter (in the future, perhaps

View file

@ -81,49 +81,49 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes
renewal configuration file and/or systemwide defaults.
"""
def __init__(self, configfile, config_opts=None, cli_config=None):
def __init__(self, config_filename, cli_config):
"""Instantiate a RenewableCert object from an existing lineage.
:param configobj.ConfigObj configfile: an already-parsed
ConfigObj object made from reading the renewal config file
:param str config_filename: the path to the renewal config file
that defines this lineage.
:param configobj.ConfigObj config_opts: systemwide defaults for
renewal properties not otherwise specified in the individual
renewal config file.
:param .RenewerConfiguration cli_config:
:param .RenewerConfiguration: parsed command line arguments
:raises .CertStorageError: if the configuration file's name didn't end
in ".conf", or the file is missing or broken.
:raises TypeError: if the provided renewal configuration isn't a
ConfigObj object.
"""
self.cli_config = cli_config
if isinstance(configfile, configobj.ConfigObj):
if not os.path.basename(configfile.filename).endswith(".conf"):
raise errors.CertStorageError(
"renewal config file name must end in .conf")
self.lineagename = os.path.basename(
configfile.filename)[:-len(".conf")]
else:
raise TypeError("RenewableCert config must be ConfigObj object")
if not config_filename.endswith(".conf"):
raise errors.CertStorageError(
"renewal config file name must end in .conf")
self.lineagename = os.path.basename(
config_filename[:-len(".conf")])
# self.configuration should be used to read parameters that
# may have been chosen based on default values from the
# systemwide renewal configuration; self.configfile should be
# used to make and save changes.
self.configfile = configfile
try:
self.configfile = configobj.ConfigObj(config_filename)
except configobj.ConfigObjError:
raise errors.CertStorageError(
"error parsing {0}".format(config_filename))
# TODO: Do we actually use anything from defaults and do we want to
# read further defaults from the systemwide renewal configuration
# file at this stage?
self.configuration = config_with_defaults(config_opts)
self.configuration.merge(self.configfile)
try:
self.configuration = config_with_defaults(
configobj.ConfigObj(cli_config.renewer_config_file))
self.configuration.merge(self.configfile)
except:
raise errors.CertStorageError(
"error parsing {0}".format(cli_config.renewer_config_file))
if not all(x in self.configuration for x in ALL_FOUR):
raise errors.CertStorageError(
"renewal config file {0} is missing a required "
"file reference".format(configfile))
"file reference".format(self.configfile))
self.cert = self.configuration["cert"]
self.privkey = self.configuration["privkey"]
@ -622,6 +622,8 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes
:param configobj.ConfigObj config: renewal configuration
defaults, affecting, for example, the locations of the
directories where the associated files will be saved
:param .RenewerConfiguration cli_config: parsed command line
arguments
:returns: the newly-created RenewalCert object
:rtype: :class:`storage.renewableCert`"""
@ -691,7 +693,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes
# TODO: add human-readable comments explaining other available
# parameters
new_config.write()
return cls(new_config, config, cli_config)
return cls(new_config.filename, cli_config)
def save_successor(self, prior_version, new_cert, new_privkey, new_chain):
"""Save new cert and chain as a successor of a prior version.

View file

@ -302,26 +302,25 @@ class DuplicativeCertsTest(renewer_test.BaseRenewableCertTest):
f.write(test_cert)
# No overlap at all
result = _find_duplicative_certs(['wow.net', 'hooray.org'],
self.config, self.cli_config)
result = _find_duplicative_certs(
self.cli_config, ['wow.net', 'hooray.org'])
self.assertEqual(result, (None, None))
# Totally identical
result = _find_duplicative_certs(['example.com', 'www.example.com'],
self.config, self.cli_config)
result = _find_duplicative_certs(
self.cli_config, ['example.com', 'www.example.com'])
self.assertTrue(result[0].configfile.filename.endswith('example.org.conf'))
self.assertEqual(result[1], None)
# Superset
result = _find_duplicative_certs(['example.com', 'www.example.com',
'something.new'], self.config,
self.cli_config)
result = _find_duplicative_certs(
self.cli_config, ['example.com', 'www.example.com', 'something.new'])
self.assertEqual(result[0], None)
self.assertTrue(result[1].configfile.filename.endswith('example.org.conf'))
# Partial overlap doesn't count
result = _find_duplicative_certs(['example.com', 'something.new'],
self.config, self.cli_config)
result = _find_duplicative_certs(
self.cli_config, ['example.com', 'something.new'])
self.assertEqual(result, (None, None))

View file

@ -63,11 +63,11 @@ class BaseRenewableCertTest(unittest.TestCase):
kind + ".pem")
config.filename = os.path.join(self.tempdir, "renewal",
"example.org.conf")
config.write()
self.config = config
self.defaults = configobj.ConfigObj()
self.test_rc = storage.RenewableCert(
self.config, self.defaults, self.cli_config)
self.test_rc = storage.RenewableCert(config.filename, self.cli_config)
def tearDown(self):
shutil.rmtree(self.tempdir)
@ -99,34 +99,26 @@ class RenewableCertTests(BaseRenewableCertTest):
def test_renewal_bad_config(self):
"""Test that the RenewableCert constructor will complain if
the renewal configuration file doesn't end in ".conf" or if it
isn't a ConfigObj."""
the renewal configuration file doesn't end in ".conf"
"""
from letsencrypt import storage
defaults = configobj.ConfigObj()
config = configobj.ConfigObj()
# These files don't exist and aren't created here; the point of the test
# is to confirm that the constructor rejects them outright because of
# the configfile's name.
for kind in ALL_FOUR:
config["cert"] = "nonexistent_" + kind + ".pem"
config.filename = "nonexistent_sillyfile"
self.assertRaises(
errors.CertStorageError, storage.RenewableCert, config, defaults)
self.assertRaises(TypeError, storage.RenewableCert, "fun", defaults)
self.assertRaises(errors.CertStorageError, storage.RenewableCert,
"fun", self.cli_config)
def test_renewal_incomplete_config(self):
"""Test that the RenewableCert constructor will complain if
the renewal configuration file is missing a required file element."""
from letsencrypt import storage
defaults = configobj.ConfigObj()
config = configobj.ConfigObj()
config["cert"] = "imaginary_cert.pem"
# Here the required privkey is missing.
config["chain"] = "imaginary_chain.pem"
config["fullchain"] = "imaginary_fullchain.pem"
config.filename = "imaginary_config.conf"
self.assertRaises(
errors.CertStorageError, storage.RenewableCert, config, defaults)
config.filename = os.path.join(self.tempdir, "imaginary_config.conf")
config.write()
self.assertRaises(errors.CertStorageError, storage.RenewableCert,
config.filename, self.cli_config)
def test_consistent(self):
# pylint: disable=too-many-statements,protected-access
@ -719,10 +711,6 @@ class RenewableCertTests(BaseRenewableCertTest):
mock_rc_instance.should_autorenew.return_value = True
mock_rc_instance.latest_common_version.return_value = 10
mock_rc.return_value = mock_rc_instance
with open(os.path.join(self.cli_config.renewal_configs_dir,
"README"), "w") as f:
f.write("This is a README file to make sure that the renewer is")
f.write("able to correctly ignore files that don't end in .conf.")
with open(os.path.join(self.cli_config.renewal_configs_dir,
"example.org.conf"), "w") as f:
# This isn't actually parsed in this test; we have a separate
@ -734,7 +722,7 @@ class RenewableCertTests(BaseRenewableCertTest):
"example.com.conf"), "w") as f:
f.write("cert = cert.pem\nprivkey = privkey.pem\n")
f.write("chain = chain.pem\nfullchain = fullchain.pem\n")
renewer.main(self.defaults, cli_args=self._common_cli_args())
renewer.main(cli_args=self._common_cli_args())
self.assertEqual(mock_rc.call_count, 2)
self.assertEqual(mock_rc_instance.update_all_links_to.call_count, 2)
self.assertEqual(mock_notify.notify.call_count, 4)
@ -747,7 +735,7 @@ class RenewableCertTests(BaseRenewableCertTest):
mock_happy_instance.should_autorenew.return_value = False
mock_happy_instance.latest_common_version.return_value = 10
mock_rc.return_value = mock_happy_instance
renewer.main(self.defaults, cli_args=self._common_cli_args())
renewer.main(cli_args=self._common_cli_args())
self.assertEqual(mock_rc.call_count, 4)
self.assertEqual(mock_happy_instance.update_all_links_to.call_count, 0)
self.assertEqual(mock_notify.notify.call_count, 4)
@ -758,7 +746,7 @@ class RenewableCertTests(BaseRenewableCertTest):
with open(os.path.join(self.cli_config.renewal_configs_dir,
"bad.conf"), "w") as f:
f.write("incomplete = configfile\n")
renewer.main(self.defaults, cli_args=self._common_cli_args())
renewer.main(cli_args=self._common_cli_args())
# The errors.CertStorageError is caught inside and nothing happens.