From 74f089ae3bb6b2fc67a27417c4ae4279bf8bde4b Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Thu, 28 May 2015 07:12:19 +0000 Subject: [PATCH 01/13] Revert pre-renewer deploy_certificate API --- letsencrypt/cli.py | 5 ++--- letsencrypt/client.py | 21 +++++++++------------ 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index fb9a7244a..a848e9c54 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -103,7 +103,7 @@ def run(args, config, plugins): installer, plugins) if not lineage: return "Certificate could not be obtained" - acme.deploy_certificate(doms, lineage) + acme.deploy_certificate(doms, lineage.privkey, lineage.cert, lineage.chain) acme.enhance_config(doms, args.redirect) @@ -145,8 +145,7 @@ def install(args, config, plugins): acme, doms = _common_run( args, config, acc, authenticator=None, installer=installer) assert args.cert_path is not None - # XXX: This API has changed as a result of RenewableCert! - # acme.deploy_certificate(doms, acc.key, args.cert_path, args.chain_path) + acme.deploy_certificate(doms, acc.key, args.cert_path, args.chain_path) acme.enhance_config(doms, args.redirect) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 58bdb6fda..a0272d7b7 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -238,32 +238,29 @@ class Client(object): return os.path.abspath(act_cert_path), cert_chain_abspath - def deploy_certificate(self, domains, lineage): + def deploy_certificate(self, domains, privkey, cert_path, chain_path): """Install certificate :param list domains: list of domains to install the certificate - :param lineage: RenewableCert object representing the certificate - :type lineage: :class:`letsencrypt.storage.RenewableCert` + :param privkey: private key for certificate + :type privkey: :class:`letsencrypt.le_util.Key` + + :param str cert_path: certificate file path (optional) + :param str chain_path: chain file path + """ if self.installer is None: logging.warning("No installer specified, client is unable to deploy" "the certificate") raise errors.LetsEncryptClientError("No installer available") - # TODO: Is it possible not to have a chain at all? (The - # RenewableCert class currently doesn't support this case, but - # perhaps the CA can issue according to ACME without providing - # a chain, which would currently be a problem for instantiating - # RenewableCert, and subsequently also for this method.) + chain_path = None if chain_path is None else os.path.abspath(chain_path) for dom in domains: # TODO: Provide a fullchain reference for installers like # nginx that want it - self.installer.deploy_cert(dom, - lineage.cert, - lineage.privkey, - lineage.chain) + self.installer.deploy_cert(dom, cert_path, privkey, chain_path) self.installer.save("Deployed Let's Encrypt Certificate") # sites may have been enabled / final cleanup From b75df9f4deda597bf062ae230ee39390c1aa6cd8 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Thu, 28 May 2015 07:43:40 +0000 Subject: [PATCH 02/13] range -> xrange --- letsencrypt/tests/le_util_test.py | 2 +- letsencrypt/tests/renewer_test.py | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/letsencrypt/tests/le_util_test.py b/letsencrypt/tests/le_util_test.py index 3f5f08c4c..475c82534 100644 --- a/letsencrypt/tests/le_util_test.py +++ b/letsencrypt/tests/le_util_test.py @@ -142,7 +142,7 @@ class UniqueLineageNameTest(unittest.TestCase): self.assertTrue(isinstance(name, str)) def test_multiple(self): - for _ in range(10): + for _ in xrange(10): f, name = self._call("wow") self.assertTrue(isinstance(f, file)) self.assertTrue(isinstance(name, str)) diff --git a/letsencrypt/tests/renewer_test.py b/letsencrypt/tests/renewer_test.py index 4f1fe6bfc..7631ef8bf 100644 --- a/letsencrypt/tests/renewer_test.py +++ b/letsencrypt/tests/renewer_test.py @@ -167,7 +167,7 @@ class RenewableCertTests(unittest.TestCase): self.assertEqual(self.test_rc.current_version("cert"), None) def test_latest_and_next_versions(self): - for ver in range(1, 6): + for ver in xrange(1, 6): for kind in ALL_FOUR: where = getattr(self.test_rc, kind) if os.path.islink(where): @@ -215,7 +215,7 @@ class RenewableCertTests(unittest.TestCase): self.assertEqual(self.test_rc.next_free_version(), 18) def test_update_link_to(self): - for ver in range(1, 6): + for ver in xrange(1, 6): for kind in ALL_FOUR: where = getattr(self.test_rc, kind) if os.path.islink(where): @@ -250,7 +250,7 @@ class RenewableCertTests(unittest.TestCase): os.path.basename(self.test_rc.version("cert", 8))) def test_update_all_links_to(self): - for ver in range(1, 6): + for ver in xrange(1, 6): for kind in ALL_FOUR: where = getattr(self.test_rc, kind) if os.path.islink(where): @@ -261,14 +261,14 @@ class RenewableCertTests(unittest.TestCase): f.write(kind) self.assertEqual(ver, self.test_rc.current_version(kind)) self.assertEqual(self.test_rc.latest_common_version(), 5) - for ver in range(1, 6): + for ver in xrange(1, 6): self.test_rc.update_all_links_to(ver) for kind in ALL_FOUR: self.assertEqual(ver, self.test_rc.current_version(kind)) self.assertEqual(self.test_rc.latest_common_version(), 5) def test_has_pending_deployment(self): - for ver in range(1, 6): + for ver in xrange(1, 6): for kind in ALL_FOUR: where = getattr(self.test_rc, kind) if os.path.islink(where): @@ -278,7 +278,7 @@ class RenewableCertTests(unittest.TestCase): with open(where, "w") as f: f.write(kind) self.assertEqual(ver, self.test_rc.current_version(kind)) - for ver in range(1, 6): + for ver in xrange(1, 6): self.test_rc.update_all_links_to(ver) for kind in ALL_FOUR: self.assertEqual(ver, self.test_rc.current_version(kind)) @@ -371,7 +371,7 @@ class RenewableCertTests(unittest.TestCase): self.assertFalse(self.test_rc.should_autodeploy()) self.test_rc.configuration["autodeploy"] = "1" # No pending deployment - for ver in range(1, 6): + for ver in xrange(1, 6): for kind in ALL_FOUR: where = getattr(self.test_rc, kind) if os.path.islink(where): @@ -403,7 +403,7 @@ class RenewableCertTests(unittest.TestCase): mock_ocsp.return_value = False def test_save_successor(self): - for ver in range(1, 6): + for ver in xrange(1, 6): for kind in ALL_FOUR: where = getattr(self.test_rc, kind) if os.path.islink(where): From 81ac25f89c83958d718e957ece36dc211f9f573e Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Thu, 28 May 2015 07:17:20 +0000 Subject: [PATCH 03/13] Add API docs for renewer and storage --- docs/api/renewer.rst | 5 +++++ docs/api/storage.rst | 5 +++++ 2 files changed, 10 insertions(+) create mode 100644 docs/api/renewer.rst create mode 100644 docs/api/storage.rst diff --git a/docs/api/renewer.rst b/docs/api/renewer.rst new file mode 100644 index 000000000..cc42c6eab --- /dev/null +++ b/docs/api/renewer.rst @@ -0,0 +1,5 @@ +:mod:`letsencrypt.renewer` +-------------------------- + +.. automodule:: letsencrypt.renewer + :members: diff --git a/docs/api/storage.rst b/docs/api/storage.rst new file mode 100644 index 000000000..198d85b46 --- /dev/null +++ b/docs/api/storage.rst @@ -0,0 +1,5 @@ +:mod:`letsencrypt.storage` +-------------------------- + +.. automodule:: letsencrypt.storage + :members: From a00dc88ad12270fd28070f01ed06ae2abe7b194e Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Thu, 28 May 2015 07:24:18 +0000 Subject: [PATCH 04/13] Fix renewer pr docstrings --- letsencrypt/client.py | 19 ++- letsencrypt/notify.py | 8 +- letsencrypt/renewer.py | 32 ++-- letsencrypt/storage.py | 271 ++++++++++++++++-------------- letsencrypt/tests/renewer_test.py | 15 +- 5 files changed, 189 insertions(+), 156 deletions(-) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index a0272d7b7..3fb02f7a6 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -155,9 +155,11 @@ class Client(object): return cert_pem, cert_key.pem, chain_pem - def obtain_and_enroll_certificate(self, domains, authenticator, installer, - plugins, csr=None): - """Get a new certificate for the specified domains using the specified + def obtain_and_enroll_certificate( + self, domains, authenticator, installer, plugins, csr=None): + """Obtain and enroll certificate. + + Get a new certificate for the specified domains using the specified authenticator and installer, and then create a new renewable lineage containing it. @@ -175,8 +177,8 @@ class Client(object): :returns: A new :class:`letsencrypt.storage.RenewableCert` instance referred to the enrolled cert lineage, or False if the cert could not be obtained. + """ - # TODO: fully identify object types in docstring. cert_pem, privkey, chain_pem = self._obtain_certificate(domains, csr) self.config.namespace.authenticator = plugins.find_init( authenticator).name @@ -187,8 +189,13 @@ class Client(object): vars(self.config.namespace)) def obtain_certificate(self, domains): - """Public method to obtain a certificate for the specified domains - using this client object. Returns the tuple (cert, privkey, chain).""" + """Obtain certificate. + + Public method to obtain a certificate for the specified domains + using this client object. Returns the tuple (cert, privkey, + chain). + + """ return self._obtain_certificate(domains, None) def save_certificate(self, certr, cert_path, chain_path): diff --git a/letsencrypt/notify.py b/letsencrypt/notify.py index b4eec938f..cfbfa82b0 100644 --- a/letsencrypt/notify.py +++ b/letsencrypt/notify.py @@ -7,8 +7,12 @@ import subprocess def notify(subject, whom, what): - """Try to notify the addressee (whom) by e-mail, with Subject: - defined by subject and message body by what.""" + """Send email notification. + + Try to notify the addressee (``whom``) by e-mail, with Subject: + defined by ``subject`` and message body by ``what``. + + """ msg = email.message_from_string(what) msg.add_header("From", "Let's Encrypt renewal agent ") msg.add_header("To", whom) diff --git a/letsencrypt/renewer.py b/letsencrypt/renewer.py index c436c2ccd..40bc5cf60 100644 --- a/letsencrypt/renewer.py +++ b/letsencrypt/renewer.py @@ -1,11 +1,12 @@ -"""Renewer tool to handle autorenewal and autodeployment of renewed -certs within lineages of successor certificates, according to -configuration.""" +"""Renewer tool. -# TODO: sanity checking consistency, validity, freshness? +Renewer tool handles autorenewal and autodeployment of renewed certs +within lineages of successor certificates, according to configuration. -# TODO: call new installer API to restart servers after deployment +.. todo:: Sanity checking consistency, validity, freshness? +.. todo:: Call new installer API to restart servers after deployment +""" import copy import os @@ -21,8 +22,11 @@ from letsencrypt.plugins import disco as plugins_disco class AttrDict(dict): - """A trick to allow accessing dictionary keys as object - attributes.""" + """Attribute dictionary. + + A trick to allow accessing dictionary keys as object attributes. + + """ def __init__(self, *args, **kwargs): super(AttrDict, self).__init__(*args, **kwargs) self.__dict__ = self @@ -31,14 +35,16 @@ class AttrDict(dict): def renew(cert, old_version): """Perform automated renewal of the referenced cert, if possible. - :param class:`letsencrypt.storage.RenewableCert` cert: the certificate + :param letsencrypt.storage.RenewableCert cert: The certificate lineage to attempt to renew. - :param int old_version: the version of the certificate lineage relative - to which the renewal should be attempted. + :param int old_version: The version of the certificate lineage + relative to which the renewal should be attempted. - :returns: int referring to newly created version of this cert lineage, - or False if renewal was not successful.""" + :returns: A number referring to newly created version of this cert + lineage, or ``False`` if renewal was not successful. + :rtype: `int` or `bool` + """ # TODO: handle partial success (some names can be renewed but not # others) # TODO: handle obligatory key rotation vs. optional key rotation vs. @@ -89,7 +95,7 @@ def renew(cert, old_version): def main(config=None): - """main function for autorenewer script.""" + """Main function for autorenewer script.""" # TODO: Distinguish automated invocation from manual invocation, # perhaps by looking at sys.argv[0] and inhibiting automated # invocations if /etc/letsencrypt/renewal.conf defaults have diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index 1fb17c561..3b2cd58b0 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -1,6 +1,4 @@ -"""The RenewableCert class, representing renewable lineages of -certificates and storing the associated cert data and metadata.""" - +"""Renewable certificates storage.""" import copy import datetime import os @@ -23,14 +21,14 @@ def parse_time_interval(interval, textparser=parsedatetime.Calendar()): """Parse the time specified time interval. The interval can be in the English-language format understood by - parsedatetime, e.g., '10 days', '3 weeks', '6 months', '9 hours', - or a sequence of such intervals like '6 months 1 week' or '3 days - 12 hours'. If an integer is found with no associated unit, it is + parsedatetime, e.g., '10 days', '3 weeks', '6 months', '9 hours', or + a sequence of such intervals like '6 months 1 week' or '3 days 12 + hours'. If an integer is found with no associated unit, it is interpreted by default as a number of days. - :param str interval: the time interval to parse. + :param str interval: The time interval to parse. - :returns: the interpretation of the time interval. + :returns: The interpretation of the time interval. :rtype: :class:`datetime.timedelta`""" if interval.strip().isdigit(): @@ -40,59 +38,55 @@ def parse_time_interval(interval, textparser=parsedatetime.Calendar()): class RenewableCert(object): # pylint: disable=too-many-instance-attributes - """Represents a lineage of certificates that is under the management + """Renewable certificate. + + Represents a lineage of certificates that is under the management of the Let's Encrypt client, indicated by the existence of an associated renewal configuration file. - Note that the notion of "current version" for a lineage is maintained - on disk in the structure of symbolic links, and is not explicitly - stored in any instance variable in this object. The RenewableCert - object is able to determine information about the current (or other) - version by accessing data on disk, but does not inherently know any - of this information except by examining the symbolic links as needed. - The instance variables mentioned below point to symlinks that reflect - the notion of "current version" of each managed object, and it is - these paths that should be used when configuring servers to use the - certificate managed in a lineage. These paths are normally within - the "live" directory, and their symlink targets -- the actual cert - files -- are normally found within the "archive" directory. + Note that the notion of "current version" for a lineage is + maintained on disk in the structure of symbolic links, and is not + explicitly stored in any instance variable in this object. The + RenewableCert object is able to determine information about the + current (or other) version by accessing data on disk, but does not + inherently know any of this information except by examining the + symbolic links as needed. The instance variables mentioned below + point to symlinks that reflect the notion of "current version" of + each managed object, and it is these paths that should be used when + configuring servers to use the certificate managed in a lineage. + These paths are normally within the "live" directory, and their + symlink targets -- the actual cert files -- are normally found + within the "archive" directory. - :ivar cert: The path to the symlink representing the current version - of the certificate managed by this lineage. - :type cert: str - - :ivar privkey: The path to the symlink representing the current version - of the private key managed by this lineage. - :type privkey: str - - :ivar chain: The path to the symlink representing the current version + :ivar str cert: The path to the symlink representing the current + version of the certificate managed by this lineage. + :ivar str privkey: The path to the symlink representing the current + version of the private key managed by this lineage. + :ivar str chain: The path to the symlink representing the current version of the chain managed by this lineage. - :type chain: str - - :ivar fullchain: The path to the symlink representing the current version - of the fullchain (combined chain and cert) managed by this lineage. - :type fullchain: str - - :ivar configuration: The renewal configuration options associated with - this lineage, obtained from parsing the renewal configuration file - and/or systemwide defaults. - :type configuration: :class:`configobj.ConfigObj`""" + :ivar str fullchain: The path to the symlink representing the + current version of the fullchain (combined chain and cert) + managed by this lineage. + :ivar configobj.ConfigObj configuration: The renewal configuration + options associated with this lineage, obtained from parsing the + renewal configuration file and/or systemwide defaults. + """ def __init__(self, configfile, config_opts=None): """Instantiate a RenewableCert object from an existing lineage. - :param :class:`configobj.ConfigObj` configfile: an already-parsed - ConfigObj object made from reading the renewal config file that - defines this lineage. - :param :class:`configobj.ConfigObj` config_opts: systemwide defaults - for renewal properties not otherwise specified in the individual - renewal config file. + :param configobj.ConfigObj configfile: an already-parsed + ConfigObj object made from reading 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. - :raises ValueError: if the configuration file's name didn't end in - ".conf", or the file is missing or broken. + :raises ValueError: 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.""" + ConfigObj object. + """ if isinstance(configfile, configobj.ConfigObj): if not os.path.basename(configfile.filename).endswith(".conf"): raise ValueError("renewal config file name must end in .conf") @@ -127,10 +121,12 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes def consistent(self): """Are the files associated with this lineage self-consistent? - :returns: whether the files stored in connection with this - lineage appear to be correct and consistent with one another. - :rtype: bool""" + :returns: Whether the files stored in connection with this + lineage appear to be correct and consistent with one + another. + :rtype: bool + """ # Each element must be referenced with an absolute path if any(not os.path.isabs(x) for x in (self.cert, self.privkey, self.chain, self.fullchain)): @@ -182,7 +178,10 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes def fix(self): """Attempt to fix defects or inconsistencies in this lineage. - (Currently unimplemented.)""" + + .. todo:: Currently unimplemented. + + """ # TODO: Figure out what kinds of fixes are possible. For # example, checking if there is a valid version that # we can update the symlinks to. (Maybe involve @@ -201,9 +200,11 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes :param str kind: the lineage member item ("cert", "privkey", "chain", or "fullchain") - :returns: the path to the current version of the specified member. - :rtype: str""" + :returns: The path to the current version of the specified + member. + :rtype: str + """ if kind not in ALL_FOUR: raise ValueError("unknown kind of item") link = getattr(self, kind) @@ -217,16 +218,16 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes def current_version(self, kind): """Returns numerical version of the specified item. - For example, if kind - is "chain" and the current chain link points to a file named - "chain7.pem", returns the integer 7. + For example, if kind is "chain" and the current chain link + points to a file named "chain7.pem", returns the integer 7. :param str kind: the lineage member item ("cert", "privkey", "chain", or "fullchain") :returns: the current version of the specified member. - :rtype: int""" + :rtype: int + """ if kind not in ALL_FOUR: raise ValueError("unknown kind of item") pattern = re.compile(r"^{0}([0-9]+)\.pem$".format(kind)) @@ -242,34 +243,36 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes def version(self, kind, version): """The filename that corresponds to the specified version and kind. - Warning: the specified version may not exist in this lineage. There - is no guarantee that the file path returned by this method actually - exists. + .. warning:: The specified version may not exist in this + lineage. There is no guarantee that the file path returned + by this method actually exists. :param str kind: the lineage member item ("cert", "privkey", "chain", or "fullchain") :param int version: the desired version - :returns: the path to the specified version of the specified member. - :rtype: str""" + :returns: The path to the specified version of the specified member. + :rtype: str + """ if kind not in ALL_FOUR: raise ValueError("unknown kind of item") where = os.path.dirname(self.current_target(kind)) return os.path.join(where, "{0}{1}.pem".format(kind, version)) def available_versions(self, kind): - """Which alternative versions of the specified kind of item exist? + """Which lternative versions of the specified kind of item exist? The archive directory where the current version is stored is consulted to obtain the list of alternatives. - :param str kind: the lineage member item ("cert", "privkey", - "chain", or "fullchain") + :param str kind: the lineage member item ( + ``cert``, ``privkey``, ``chain``, or ``fullchain``) :returns: all of the version numbers that currently exist - :rtype: list of int""" + :rtype: `list` of `int` + """ if kind not in ALL_FOUR: raise ValueError("unknown kind of item") where = os.path.dirname(self.current_target(kind)) @@ -279,23 +282,25 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes return sorted([int(m.groups()[0]) for m in matches if m]) def newest_available_version(self, kind): - """What is the newest available version of the specified kind of item? + """Newest available version of the specified kind of item? - :param str kind: the lineage member item ("cert", "privkey", - "chain", or "fullchain") + :param str kind: the lineage member item (``cert``, + ``privkey``, ``chain``, or ``fullchain``) :returns: the newest available version of this member - :rtype: int""" + :rtype: int + """ return max(self.available_versions(kind)) def latest_common_version(self): - """What is the newest version for which all items are available? + """Newest version for which all items are available? - :returns: the newest available version for which all members (cert, - privkey, chain, and fullchain) exist - :rtype: int""" + :returns: the newest available version for which all members + (``cert, ``privkey``, ``chain``, and ``fullchain``) exist + :rtype: int + """ # TODO: this can raise ValueError if there is no version overlap # (it should probably return None instead) # TODO: this can raise a spurious AttributeError if the current @@ -304,13 +309,13 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes return max(n for n in versions[0] if all(n in v for v in versions[1:])) def next_free_version(self): - """What is the smallest version newer than all full or partial versions? + """Smallest version newer than all full or partial versions? - :returns: the smallest version number that is larger than any version - of any item currently stored in this lineage + :returns: the smallest version number that is larger than any + version of any item currently stored in this lineage :rtype: int - """ + """ # TODO: consider locking/mutual exclusion between updating processes # This isn't self.latest_common_version() + 1 because we don't want # collide with a version that might exist for one file type but not @@ -320,11 +325,12 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes def has_pending_deployment(self): """Is there a later version of all of the managed items? - :returns: True if there is a complete version of this lineage with - a larger version number than the current version, and False - otherwise - :rtype: bool""" + :returns: ``True`` if there is a complete version of this + lineage with a larger version number than the current + version, and ``False`` otherwis + :rtype: bool + """ # TODO: consider whether to assume consistency or treat # inconsistent/consistent versions differently smallest_current = min(self.current_version(x) for x in ALL_FOUR) @@ -338,8 +344,9 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes :param str kind: the lineage member item ("cert", "privkey", "chain", or "fullchain") - :param int version: the desired version""" + :param int version: the desired version + """ if kind not in ALL_FOUR: raise ValueError("unknown kind of item") link = getattr(self, kind) @@ -383,10 +390,11 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes :param int version: the desired version number - :returns: the notBefore value from the specified cert version in this - lineage - :rtype: :class:`datetime.datetime`""" + :returns: the notBefore value from the specified cert version in + this lineage + :rtype: :class:`datetime.datetime` + """ return self._notafterbefore(lambda x509: x509.get_notBefore(), version) def notafter(self, version=None): @@ -396,24 +404,27 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes :param int version: the desired version number - :returns: the notAfter value from the specified cert version in this - lineage - :rtype: :class:`datetime.datetime`""" + :returns: the notAfter value from the specified cert version in + this lineage + :rtype: :class:`datetime.datetime` + """ return self._notafterbefore(lambda x509: x509.get_notAfter(), version) def should_autodeploy(self): """Should this lineage now automatically deploy a newer version? - This is a policy question and does not only depend on whether there - is a newer version of the cert. (This considers whether autodeployment - is enabled, whether a relevant newer version exists, and whether the - time interval for autodeployment has been reached.) + This is a policy question and does not only depend on whether + there is a newer version of the cert. (This considers whether + autodeployment is enabled, whether a relevant newer version + exists, and whether the time interval for autodeployment has + been reached.) - :returns: whether the lineage now ought to autodeploy an existing - newer cert version - :rtype: bool""" + :returns: whether the lineage now ought to autodeploy an + existing newer cert version + :rtype: bool + """ if ("autodeploy" not in self.configuration or self.configuration.as_bool("autodeploy")): if self.has_pending_deployment(): @@ -431,17 +442,19 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes # pylint: disable=no-self-use,unused-argument """Is the specified cert version revoked according to OCSP? - Also returns True if the cert version is declared as intended to be - revoked according to Let's Encrypt OCSP extensions. (If no version - is specified, uses the current version.) + Also returns True if the cert version is declared as intended + to be revoked according to Let's Encrypt OCSP extensions. + (If no version is specified, uses the current version.) - This method is not yet implemented and currently always returns False. + This method is not yet implemented and currently always returns + False. :param int version: the desired version number :returns: whether the certificate is or will be revoked - :rtype: bool""" + :rtype: bool + """ # XXX: This query and its associated network service aren't # implemented yet, so we currently return False (indicating that the # certificate is not revoked). @@ -450,18 +463,19 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes def should_autorenew(self): """Should we now try to autorenew the most recent cert version? - This is a policy question and does not only depend on whether the - cert is expired. (This considers whether autorenewal is enabled, - whether the cert is revoked, and whether the time interval for - autorenewal has been reached.) + This is a policy question and does not only depend on whether + the cert is expired. (This considers whether autorenewal is + enabled, whether the cert is revoked, and whether the time + interval for autorenewal has been reached.) Note that this examines the numerically most recent cert version, not the currently deployed version. :returns: whether an attempt should now be made to autorenew the most current cert version in this lineage - :rtype: bool""" + :rtype: bool + """ if ("autorenew" not in self.configuration or self.configuration.as_bool("autorenew")): # Consider whether to attempt to autorenew this cert now @@ -486,28 +500,28 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes # pylint: disable=too-many-locals,too-many-arguments """Create a new certificate lineage. - Attempts to create a certificate lineage -- enrolled for potential - future renewal -- with the (suggested) lineage name lineagename, - and the associated cert, privkey, and chain (the associated - fullchain will be created automatically). Optional configurator - and renewalparams record the configuration that was originally - used to obtain this cert, so that it can be reused later during - automated renewal. + Attempts to create a certificate lineage -- enrolled for + potential future renewal -- with the (suggested) lineage name + lineagename, and the associated cert, privkey, and chain (the + associated fullchain will be created automatically). Optional + configurator and renewalparams record the configuration that was + originally used to obtain this cert, so that it can be reused + later during automated renewal. Returns a new RenewableCert object referring to the created lineage. (The actual lineage name, as well as all the relevant file paths, will be available within this object.) :param str lineagename: the suggested name for this lineage - (normally the current cert's first subject DNS name) + (normally the current cert's first subject DNS name) :param str cert: the initial certificate version in PEM format :param str privkey: the private key in PEM format :param str chain: the certificate chain in PEM format - :param :class:`configobj.ConfigObj` renewalparams: parameters that + :param configobj.ConfigObj renewalparams: parameters that should be used when instantiating authenticator and installer objects in the future to attempt to renew this cert or deploy new versions of it - :param :class:`configobj.ConfigObj` config: renewal configuration + :param configobj.ConfigObj config: renewal configuration defaults, affecting, for example, the locations of the directories where the associated files will be saved @@ -585,21 +599,24 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes def save_successor(self, prior_version, new_cert, new_privkey, new_chain): """Save new cert and chain as a successor of a prior version. - Returns the new version number that was created. Note: does NOT - update links to deploy this version. + Returns the new version number that was created. - :param int prior_version: the old version to which this version is - regarded as a successor (used to choose a privkey, if the key - has not changed, but otherwise this information is not permanently - recorded anywhere) + .. note:: this function does NOT update links to deploy this + version + + :param int prior_version: the old version to which this version + is regarded as a successor (used to choose a privkey, if the + key has not changed, but otherwise this information is not + permanently recorded anywhere) :param str new_cert: the new certificate, in PEM format - :param str new_privkey: the new private key, in PEM format, or None, - if the private key has not changed + :param str new_privkey: the new private key, in PEM format, + or ``None``, if the private key has not changed :param str new_chain: the new chain, in PEM format :returns: the new version number that was created - :rtype: int""" + :rtype: int + """ # XXX: assumes official archive location rather than examining links # XXX: consider using os.open for availablity of os.O_EXCL # XXX: ensure file permissions are correct; also create directories diff --git a/letsencrypt/tests/renewer_test.py b/letsencrypt/tests/renewer_test.py index 7631ef8bf..061cd71e5 100644 --- a/letsencrypt/tests/renewer_test.py +++ b/letsencrypt/tests/renewer_test.py @@ -1,5 +1,4 @@ -"""Tests for letsencrypt/renewer.py""" - +"""Tests for letsencrypt.renewer.""" import datetime import os import tempfile @@ -13,23 +12,22 @@ import pytz from letsencrypt.storage import ALL_FOUR + def unlink_all(rc_object): - """Unlink all four items associated with this RenewableCert. - (Helper function.)""" + """Unlink all four items associated with this RenewableCert.""" for kind in ALL_FOUR: os.unlink(getattr(rc_object, kind)) def fill_with_sample_data(rc_object): - """Put dummy data into all four files of this RenewableCert. - (Helper function.)""" + """Put dummy data into all four files of this RenewableCert.""" for kind in ALL_FOUR: with open(getattr(rc_object, kind), "w") as f: f.write(kind) + class RenewableCertTests(unittest.TestCase): # pylint: disable=too-many-public-methods - """Tests for the RenewableCert class as well as other functions - within renewer.py.""" + """Tests for letsencrypt.renewer.*.""" def setUp(self): from letsencrypt import storage self.tempdir = tempfile.mkdtemp() @@ -646,5 +644,6 @@ class RenewableCertTests(unittest.TestCase): renewer.main(self.defaults) # The ValueError is caught inside and nothing happens. + if __name__ == "__main__": unittest.main() # pragma: no cover From da8f3e19a4be73a0adde280a277ec594e27922e9 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Thu, 28 May 2015 07:58:40 +0000 Subject: [PATCH 05/13] Remove confusingly unused --enroll-autorenew. --- letsencrypt/cli.py | 3 --- letsencrypt/interfaces.py | 4 ---- 2 files changed, 7 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index a848e9c54..a32a1feaf 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -339,9 +339,6 @@ def _paths_parser(parser): add("--chain-path", default=flag_default("chain_path"), help=config_help("chain_path")) - add("--enroll-autorenew", default=None, action="store_true", - help=config_help("enroll_autorenew")) - return parser diff --git a/letsencrypt/interfaces.py b/letsencrypt/interfaces.py index e154bd5d3..fd4b3d25b 100644 --- a/letsencrypt/interfaces.py +++ b/letsencrypt/interfaces.py @@ -176,10 +176,6 @@ class IConfig(zope.interface.Interface): le_vhost_ext = zope.interface.Attribute( "SSL vhost configuration extension.") - enroll_autorenew = zope.interface.Attribute( - "Register this certificate in the database to be renewed" - " automatically.") - cert_path = zope.interface.Attribute("Let's Encrypt certificate file path.") chain_path = zope.interface.Attribute("Let's Encrypt chain file path.") From f798c117f7badf3d411eb9706bb751bb098730a0 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Thu, 28 May 2015 08:01:44 +0000 Subject: [PATCH 06/13] assertEqual(False, ...) -> assertFalse --- letsencrypt/tests/renewer_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/tests/renewer_test.py b/letsencrypt/tests/renewer_test.py index 061cd71e5..0f85674d4 100644 --- a/letsencrypt/tests/renewer_test.py +++ b/letsencrypt/tests/renewer_test.py @@ -588,7 +588,7 @@ class RenewableCertTests(unittest.TestCase): self.assertEqual(mock_da.call_count, 1) mock_client.obtain_certificate.return_value = (None, None, None) # This should fail because the renewal itself appears to fail - self.assertEqual(False, renewer.renew(self.test_rc, 1)) + self.assertFalse(renewer.renew(self.test_rc, 1)) @mock.patch("letsencrypt.renewer.notify") From 3809a817ea6a7c74e7b63cbe7b70c268cc77a19c Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Thu, 28 May 2015 08:02:43 +0000 Subject: [PATCH 07/13] Bump coverage --- tox.cover.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.cover.sh b/tox.cover.sh index 273dea04e..80b6474d7 100755 --- a/tox.cover.sh +++ b/tox.cover.sh @@ -18,5 +18,5 @@ cover () { # don't use sequential composition (;), if letsencrypt_nginx returns # 0, coveralls submit will be triggered (c.f. .travis.yml, # after_success) -cover letsencrypt 94 && cover acme 100 && \ +cover letsencrypt 95 && cover acme 100 && \ cover letsencrypt_apache 78 && cover letsencrypt_nginx 96 From 2178315f8a94bebbfd8ef01f07e9b7aec1fe1391 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Thu, 28 May 2015 15:17:55 +0000 Subject: [PATCH 08/13] Various docstring fixes. - Use r""" \* """ - transform plugins note to ..warning - ' -> ` for cross-reference - fix some "more than one target found for cross-reference" warnings --- docs/plugins.rst | 16 ++++++++-------- letsencrypt/auth_handler.py | 10 +++++----- letsencrypt/interfaces.py | 2 +- letsencrypt_nginx/obj.py | 23 +++++++++++++---------- 4 files changed, 27 insertions(+), 24 deletions(-) diff --git a/docs/plugins.rst b/docs/plugins.rst index c4df1180a..33d63e84f 100644 --- a/docs/plugins.rst +++ b/docs/plugins.rst @@ -4,16 +4,16 @@ Plugins Let's Encrypt client supports dynamic discovery of plugins through the `setuptools entry points`_. This way you can, for example, create a -custom implementation of -`~letsencrypt.interfaces.IAuthenticator` or the -'~letsencrypt.interfaces.IInstaller' without having to -merge it with the core upstream source code. An example is provided in +custom implementation of `~letsencrypt.interfaces.IAuthenticator` or +the `~letsencrypt.interfaces.IInstaller` without having to merge it +with the core upstream source code. An example is provided in ``examples/plugins/`` directory. -Please be aware though that as this client is still in a developer-preview -stage, the API may undergo a few changes. If you believe the plugin will be -beneficial to the community, please consider submitting a pull request to the -repo and we will update it with any necessary API changes. +.. warning:: Please be aware though that as this client is still in a + developer-preview stage, the API may undergo a few changes. If you + believe the plugin will be beneficial to the community, please + consider submitting a pull request to the repo and we will update + it with any necessary API changes. .. _`setuptools entry points`: https://pythonhosted.org/setuptools/setuptools.html#dynamic-discovery-of-services-and-plugins diff --git a/letsencrypt/auth_handler.py b/letsencrypt/auth_handler.py index e83cdd717..37d818dbe 100644 --- a/letsencrypt/auth_handler.py +++ b/letsencrypt/auth_handler.py @@ -53,12 +53,12 @@ class AuthHandler(object): """Retrieve all authorizations for challenges. :param set domains: Domains for authorization - :param bool best_effort: Whether or not all authorizations are required - (this is useful in renewal) + :param bool best_effort: Whether or not all authorizations are + required (this is useful in renewal) - :returns: tuple of lists of authorization resources. Takes the form of - (`completed`, `failed`) - rtype: tuple + :returns: tuple of lists of authorization resources. Takes the + form of (`completed`, `failed`) + :rtype: tuple :raises AuthorizationError: If unable to retrieve all authorizations diff --git a/letsencrypt/interfaces.py b/letsencrypt/interfaces.py index e154bd5d3..7c61dc554 100644 --- a/letsencrypt/interfaces.py +++ b/letsencrypt/interfaces.py @@ -49,7 +49,7 @@ class IPluginFactory(zope.interface.Interface): """Inject argument parser options (flags). 1. Be nice and prepend all options and destinations with - `~.option_namespace` and `~.dest_namespace`. + `~.common.option_namespace` and `~common.dest_namespace`. 2. Inject options (flags) only. Positional arguments are not allowed, as this would break the CLI. diff --git a/letsencrypt_nginx/obj.py b/letsencrypt_nginx/obj.py index 9b3fa2e42..9e1f78e48 100644 --- a/letsencrypt_nginx/obj.py +++ b/letsencrypt_nginx/obj.py @@ -5,22 +5,25 @@ from letsencrypt_apache.obj import Addr as ApacheAddr class Addr(ApacheAddr): - """Represents an Nginx address, i.e. what comes after the 'listen' + r"""Represents an Nginx address, i.e. what comes after the 'listen' directive. - According to http://nginx.org/en/docs/http/ngx_http_core_module.html#listen, - this may be address[:port], port, or unix:path. The latter is ignored here. + According to the `documentation`_, this may be address[:port], port, + or unix:path. The latter is ignored here. - The default value if no directive is specified is *:80 (superuser) or - *:8000 (otherwise). If no port is specified, the default is 80. If no - address is specified, listen on all addresses. + The default value if no directive is specified is \*:80 (superuser) + or \*:8000 (otherwise). If no port is specified, the default is + 80. If no address is specified, listen on all addresses. - .. todo:: Old-style nginx configs define SSL vhosts in a separate block - instead of using 'ssl' in the listen directive + .. _documentation: + http://nginx.org/en/docs/http/ngx_http_core_module.html#listen + + .. todo:: Old-style nginx configs define SSL vhosts in a separate + block instead of using 'ssl' in the listen directive. :param str addr: addr part of vhost address, may be hostname, IPv4, IPv6, - "", or "*" - :param str port: port number or "*" or "" + "", or "\*" + :param str port: port number or "\*" or "" :param bool ssl: Whether the directive includes 'ssl' :param bool default: Whether the directive includes 'default_server' From cc969fc40613a613495e57aff8919c61c9cb8473 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Thu, 28 May 2015 18:07:26 +0000 Subject: [PATCH 09/13] Fix deploy_certificate (docs and use abspath) --- letsencrypt/client.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 3fb02f7a6..f874efd9c 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -245,14 +245,11 @@ class Client(object): return os.path.abspath(act_cert_path), cert_chain_abspath - def deploy_certificate(self, domains, privkey, cert_path, chain_path): + def deploy_certificate(self, domains, privkey_path, cert_path, chain_path): """Install certificate :param list domains: list of domains to install the certificate - - :param privkey: private key for certificate - :type privkey: :class:`letsencrypt.le_util.Key` - + :param str privkey_path: path to certificate private key :param str cert_path: certificate file path (optional) :param str chain_path: chain file path @@ -267,7 +264,9 @@ class Client(object): for dom in domains: # TODO: Provide a fullchain reference for installers like # nginx that want it - self.installer.deploy_cert(dom, cert_path, privkey, chain_path) + self.installer.deploy_cert( + dom, os.path.abspath(cert_path), + os.path.abspath(privkey_path), chain_path) self.installer.save("Deployed Let's Encrypt Certificate") # sites may have been enabled / final cleanup From c813efdce7a69d6510b8ea2a1b69d351fc983d09 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Thu, 28 May 2015 18:07:49 +0000 Subject: [PATCH 10/13] Merge obtain_certificate and _obtain_certificate --- letsencrypt/client.py | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index f874efd9c..b64f3b542 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -100,12 +100,12 @@ class Client(object): self.account.save() - def _obtain_certificate(self, domains, csr=None): + def obtain_certificate(self, domains, csr=None): """Obtains a certificate from the ACME server. :meth:`.register` must be called before :meth:`.obtain_certificate` - .. todo:: This function does not currently handle csr correctly... + .. todo:: This function does not currently handle CSR correctly. :param set domains: domains to get a certificate @@ -113,8 +113,9 @@ class Client(object): this CSR can be different than self.authkey :type csr: :class:`CSR` - :returns: cert_pem, key_pem, chain_pem - :rtype: `tuple` of (str, str, str) + :returns: Certificate, private key, and certificate chain (all + PEM-encoded). + :rtype: `tuple` of `str` """ if self.auth_handler is None: @@ -179,24 +180,13 @@ class Client(object): not be obtained. """ - cert_pem, privkey, chain_pem = self._obtain_certificate(domains, csr) + cert, privkey, chain = self.obtain_certificate(domains, csr) self.config.namespace.authenticator = plugins.find_init( authenticator).name if installer is not None: self.config.namespace.installer = plugins.find_init(installer).name - return storage.RenewableCert.new_lineage(domains[0], cert_pem, - privkey, chain_pem, - vars(self.config.namespace)) - - def obtain_certificate(self, domains): - """Obtain certificate. - - Public method to obtain a certificate for the specified domains - using this client object. Returns the tuple (cert, privkey, - chain). - - """ - return self._obtain_certificate(domains, None) + return storage.RenewableCert.new_lineage( + domains[0], cert, privkey, chain, vars(self.config.namespace)) def save_certificate(self, certr, cert_path, chain_path): # pylint: disable=no-self-use From 8d1135c0498dbd2c8f2ff89ff6292f852b4d453d Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Thu, 28 May 2015 18:09:05 +0000 Subject: [PATCH 11/13] Private _AttrDict --- letsencrypt/renewer.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/letsencrypt/renewer.py b/letsencrypt/renewer.py index 40bc5cf60..834a69548 100644 --- a/letsencrypt/renewer.py +++ b/letsencrypt/renewer.py @@ -21,14 +21,14 @@ from letsencrypt import storage from letsencrypt.plugins import disco as plugins_disco -class AttrDict(dict): +class _AttrDict(dict): """Attribute dictionary. A trick to allow accessing dictionary keys as object attributes. """ def __init__(self, *args, **kwargs): - super(AttrDict, self).__init__(*args, **kwargs) + super(_AttrDict, self).__init__(*args, **kwargs) self.__dict__ = self @@ -58,7 +58,7 @@ def renew(cert, old_version): return False # Instantiate the appropriate authenticator plugins = plugins_disco.PluginsRegistry.find_all() - config = configuration.NamespaceConfig(AttrDict(renewalparams)) + config = configuration.NamespaceConfig(_AttrDict(renewalparams)) # XXX: this loses type data (for example, the fact that key_size # was an int, not a str) config.rsa_key_size = int(config.rsa_key_size) From d01b17f1e2b65b374dccc1a827bcda1fd32324c8 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Thu, 28 May 2015 18:31:06 +0000 Subject: [PATCH 12/13] IInstaller.deploy_cert docs/naming fixes --- letsencrypt/interfaces.py | 9 +++++---- letsencrypt_apache/configurator.py | 11 +++-------- letsencrypt_nginx/configurator.py | 14 +++++--------- 3 files changed, 13 insertions(+), 21 deletions(-) diff --git a/letsencrypt/interfaces.py b/letsencrypt/interfaces.py index fd4b3d25b..021e5063d 100644 --- a/letsencrypt/interfaces.py +++ b/letsencrypt/interfaces.py @@ -193,12 +193,13 @@ class IInstaller(IPlugin): def get_all_names(): """Returns all names that may be authenticated.""" - def deploy_cert(domain, cert, key, cert_chain=None): + def deploy_cert(domain, cert_path, key_path, chain_path=None): """Deploy certificate. - :param str domain: domain to deploy certificate - :param str cert: certificate filename - :param str key: private key filename + :param str domain: domain to deploy certificate file + :param str cert_path: absolute path to the certificate file + :param str key_path: absolute path to the private key file + :param str chain_path: absolute path to the certificate chain file """ diff --git a/letsencrypt_apache/configurator.py b/letsencrypt_apache/configurator.py index 102718e13..40f570f80 100644 --- a/letsencrypt_apache/configurator.py +++ b/letsencrypt_apache/configurator.py @@ -146,7 +146,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): temp_install(self.conf('mod-ssl-conf')) - def deploy_cert(self, domain, cert_path, key, chain_path=None): + def deploy_cert(self, domain, cert_path, key_path, chain_path=None): """Deploys certificate to specified virtual host. Currently tries to find the last directives to deploy the cert in @@ -161,11 +161,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): .. todo:: Might be nice to remove chain directive if none exists This shouldn't happen within letsencrypt though - :param str domain: domain to deploy certificate - :param str cert_path: certificate filename - :param str key: private key filename - :param str chain_path: certificate chain filename - """ vhost = self.choose_vhost(domain) # TODO(jdkasten): vhost might be None @@ -192,7 +187,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): logging.info("Deploying Certificate to VirtualHost %s", vhost.filep) self.aug.set(path["cert_path"][0], cert_path) - self.aug.set(path["cert_key"][0], key) + self.aug.set(path["cert_key"][0], key_path) if chain_path is not None: if not path["chain_path"]: self.parser.add_dir( @@ -204,7 +199,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): (vhost.filep, ", ".join(str(addr) for addr in vhost.addrs))) self.save_notes += "\tSSLCertificateFile %s\n" % cert_path - self.save_notes += "\tSSLCertificateKeyFile %s\n" % key + self.save_notes += "\tSSLCertificateKeyFile %s\n" % key_path if chain_path is not None: self.save_notes += "\tSSLCertificateChainFile %s\n" % chain_path diff --git a/letsencrypt_nginx/configurator.py b/letsencrypt_nginx/configurator.py index d2deee15a..f7b53f3fa 100644 --- a/letsencrypt_nginx/configurator.py +++ b/letsencrypt_nginx/configurator.py @@ -105,7 +105,7 @@ class NginxConfigurator(common.Plugin): temp_install(self.conf('mod-ssl-conf')) # Entry point in main.py for installing cert - def deploy_cert(self, domain, cert, key, cert_chain=None): + def deploy_cert(self, domain, cert_path, key_path, chain_path=None): # pylint: disable=unused-argument """Deploys certificate to specified virtual host. @@ -118,14 +118,10 @@ class NginxConfigurator(common.Plugin): .. note:: This doesn't save the config files! - :param str domain: domain to deploy certificate - :param str cert: certificate filename - :param str key: private key filename - :param str cert_chain: certificate chain filename - """ vhost = self.choose_vhost(domain) - directives = [['ssl_certificate', cert], ['ssl_certificate_key', key]] + directives = [['ssl_certificate', cert_path], + ['ssl_certificate_key', key_path]] try: self.parser.add_server_directives(vhost.filep, vhost.names, @@ -143,8 +139,8 @@ class NginxConfigurator(common.Plugin): self.save_notes += ("Changed vhost at %s with addresses of %s\n" % (vhost.filep, ", ".join(str(addr) for addr in vhost.addrs))) - self.save_notes += "\tssl_certificate %s\n" % cert - self.save_notes += "\tssl_certificate_key %s\n" % key + self.save_notes += "\tssl_certificate %s\n" % cert_path + self.save_notes += "\tssl_certificate_key %s\n" % key_path ####################### # Vhost parsing methods From a3d452c112a63fc9b9b2eb543037e46017a4c30d Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Thu, 28 May 2015 19:27:30 +0000 Subject: [PATCH 13/13] storage.config_with_defaults --- letsencrypt/constants.py | 5 ++--- letsencrypt/renewer.py | 7 +------ letsencrypt/storage.py | 18 +++++++++--------- 3 files changed, 12 insertions(+), 18 deletions(-) diff --git a/letsencrypt/constants.py b/letsencrypt/constants.py index 4b3b79e36..088f92aea 100644 --- a/letsencrypt/constants.py +++ b/letsencrypt/constants.py @@ -1,5 +1,4 @@ """Let's Encrypt constants.""" -import configobj import logging from acme import challenges @@ -27,7 +26,7 @@ CLI_DEFAULTS = dict( """Defaults for CLI flags and `.IConfig` attributes.""" -RENEWER_DEFAULTS = configobj.ConfigObj(dict( +RENEWER_DEFAULTS = dict( renewer_config_file="/etc/letsencrypt/renewer.conf", renewal_configs_dir="/etc/letsencrypt/configs", archive_dir="/etc/letsencrypt/archive", @@ -35,7 +34,7 @@ RENEWER_DEFAULTS = configobj.ConfigObj(dict( renewer_enabled="yes", renew_before_expiry="30 days", deploy_before_expiry="20 days", -)) +) """Defaults for renewer script.""" diff --git a/letsencrypt/renewer.py b/letsencrypt/renewer.py index 834a69548..d95fb7d95 100644 --- a/letsencrypt/renewer.py +++ b/letsencrypt/renewer.py @@ -7,13 +7,11 @@ within lineages of successor certificates, according to configuration. .. todo:: Call new installer API to restart servers after deployment """ -import copy import os import configobj from letsencrypt import configuration -from letsencrypt import constants from letsencrypt import client from letsencrypt import crypto_util from letsencrypt import notify @@ -102,10 +100,7 @@ def main(config=None): # turned it off. (The boolean parameter should probably be # called renewer_enabled.) - # Merge supplied config, if provided, on top of builtin defaults - defaults_copy = copy.deepcopy(constants.RENEWER_DEFAULTS) - defaults_copy.merge(config if config is not None else configobj.ConfigObj()) - config = defaults_copy + 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 diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index 3b2cd58b0..2648be3ba 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -1,5 +1,4 @@ """Renewable certificates storage.""" -import copy import datetime import os import re @@ -17,6 +16,13 @@ from letsencrypt import le_util ALL_FOUR = ("cert", "privkey", "chain", "fullchain") +def config_with_defaults(config=None): + """Merge supplied config, if provided, on top of builtin defaults.""" + defaults_copy = configobj.ConfigObj(constants.RENEWER_DEFAULTS) + defaults_copy.merge(config if config is not None else configobj.ConfigObj()) + return defaults_copy + + def parse_time_interval(interval, textparser=parsedatetime.Calendar()): """Parse the time specified time interval. @@ -103,10 +109,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes # 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? - defaults_copy = copy.deepcopy(constants.RENEWER_DEFAULTS) - defaults_copy.merge(config_opts if config_opts is not None - else configobj.ConfigObj()) - self.configuration = defaults_copy + self.configuration = config_with_defaults(config_opts) self.configuration.merge(self.configfile) if not all(x in self.configuration for x in ALL_FOUR): @@ -528,10 +531,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes :returns: the newly-created RenewalCert object :rtype: :class:`storage.renewableCert`""" - defaults_copy = copy.deepcopy(constants.RENEWER_DEFAULTS) - defaults_copy.merge(config if config is not None - else configobj.ConfigObj()) - config = defaults_copy + config = config_with_defaults(config) # This attempts 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