Merge branch 'renewer-cleanup' of https://github.com/kuba/lets-encrypt-preview

docs/api/renewer.rst

@@ -0,0 +1,5 @@
+:mod:`letsencrypt.renewer`
+--------------------------
+
+.. automodule:: letsencrypt.renewer
+   :members:

docs/api/storage.rst

@@ -0,0 +1,5 @@
+:mod:`letsencrypt.storage`
+--------------------------
+
+.. automodule:: letsencrypt.storage
+   :members:

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)
 
 
@@ -340,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
 
 

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
+        :returns: Certificate, private key, and certificate chain (all
-        :rtype: `tuple` of (str, str, str)
+            PEM-encoded).
+        :rtype: `tuple` of `str`
 
         """
         if self.auth_handler is None:
@@ -155,9 +156,11 @@ class Client(object):
 
         return cert_pem, cert_key.pem, chain_pem
 
-    def obtain_and_enroll_certificate(self, domains, authenticator, installer,
+    def obtain_and_enroll_certificate(
-                                      plugins, csr=None):
+            self, domains, authenticator, installer, plugins, csr=None):
-        """Get a new certificate for the specified domains using the specified
+        """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,21 +178,15 @@ 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, privkey, chain = self.obtain_certificate(domains, csr)
-        cert_pem, privkey, chain_pem = 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,
+        return storage.RenewableCert.new_lineage(
-                                                 privkey, chain_pem,
+            domains[0], cert, privkey, chain, vars(self.config.namespace))
-                                                 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)."""
-        return self._obtain_certificate(domains, None)
 
     def save_certificate(self, certr, cert_path, chain_path):
         # pylint: disable=no-self-use
@@ -238,32 +235,28 @@ 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_path, cert_path, chain_path):
         """Install certificate
 
         :param list domains: list of domains to install the certificate
+        :param str privkey_path: path to certificate private key
+        :param str cert_path: certificate file path (optional)
+        :param str chain_path: chain file path
 
-        :param lineage: RenewableCert object representing the certificate
-        :type lineage: :class:`letsencrypt.storage.RenewableCert`
         """
         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
+        chain_path = None if chain_path is None else os.path.abspath(chain_path)
-        # 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.)
 
         for dom in domains:
             # TODO: Provide a fullchain reference for installers like
             #       nginx that want it
-            self.installer.deploy_cert(dom,
+            self.installer.deploy_cert(
-                                       lineage.cert,
+                dom, os.path.abspath(cert_path),
-                                       lineage.privkey,
+                os.path.abspath(privkey_path), chain_path)
-                                       lineage.chain)
 
         self.installer.save("Deployed Let's Encrypt Certificate")
         # sites may have been enabled / final cleanup

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."""
 
 

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.")
 
@@ -197,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 domain: domain to deploy certificate file
-        :param str cert: certificate filename
+        :param str cert_path: absolute path to the certificate file
-        :param str key: private key filename
+        :param str key_path: absolute path to the private key file
+        :param str chain_path: absolute path to the certificate chain file
 
         """
 

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:
+    """Send email notification.
-    defined by subject and message body by what."""
+
+    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 <root>")
     msg.add_header("To", whom)

letsencrypt/renewer.py

@@ -1,18 +1,17 @@
-"""Renewer tool to handle autorenewal and autodeployment of renewed
+"""Renewer tool.
-certs within lineages of successor certificates, according to
-configuration."""
 
-# 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
 
 import configobj
 
 from letsencrypt import configuration
-from letsencrypt import constants
 from letsencrypt import client
 from letsencrypt import crypto_util
 from letsencrypt import notify
@@ -20,25 +19,30 @@ from letsencrypt import storage
 from letsencrypt.plugins import disco as plugins_disco
 
 
-class AttrDict(dict):
+class _AttrDict(dict):
-    """A trick to allow accessing dictionary keys as object
+    """Attribute dictionary.
-    attributes."""
+
+    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
 
 
 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
+    :param int old_version: The version of the certificate lineage
-        to which the renewal should be attempted.
+        relative to which the renewal should be attempted.
 
-    :returns: int referring to newly created version of this cert lineage,
+    :returns: A number referring to newly created version of this cert
-        or False if renewal was not successful."""
+        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.
@@ -52,7 +56,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)
@@ -89,17 +93,14 @@ 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
     #       turned it off. (The boolean parameter should probably be
     #       called renewer_enabled.)
 
-    # Merge supplied config, if provided, on top of builtin defaults
+    config = storage.config_with_defaults(config)
-    defaults_copy = copy.deepcopy(constants.RENEWER_DEFAULTS)
-    defaults_copy.merge(config if config is not None else configobj.ConfigObj())
-    config = defaults_copy
     # 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

letsencrypt/storage.py

@@ -1,7 +1,4 @@
-"""The RenewableCert class, representing renewable lineages of
+"""Renewable certificates storage."""
-certificates and storing the associated cert data and metadata."""
-
-import copy
 import datetime
 import os
 import re
@@ -19,18 +16,25 @@ 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.
 
     The interval can be in the English-language format understood by
-    parsedatetime, e.g., '10 days', '3 weeks', '6 months', '9 hours',
+    parsedatetime, e.g., '10 days', '3 weeks', '6 months', '9 hours', or
-    or a sequence of such intervals like '6 months 1 week' or '3 days
+    a sequence of such intervals like '6 months 1 week' or '3 days 12
-    12 hours'.  If an integer is found with no associated unit, it is
+    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 +44,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
+    Note that the notion of "current version" for a lineage is
-    on disk in the structure of symbolic links, and is not explicitly
+    maintained on disk in the structure of symbolic links, and is not
-    stored in any instance variable in this object.  The RenewableCert
+    explicitly stored in any instance variable in this object. The
-    object is able to determine information about the current (or other)
+    RenewableCert object is able to determine information about the
-    version by accessing data on disk, but does not inherently know any
+    current (or other) version by accessing data on disk, but does not
-    of this information except by examining the symbolic links as needed.
+    inherently know any of this information except by examining the
-    The instance variables mentioned below point to symlinks that reflect
+    symbolic links as needed. The instance variables mentioned below
-    the notion of "current version" of each managed object, and it is
+    point to symlinks that reflect the notion of "current version" of
-    these paths that should be used when configuring servers to use the
+    each managed object, and it is these paths that should be used when
-    certificate managed in a lineage.  These paths are normally within
+    configuring servers to use the certificate managed in a lineage.
-    the "live" directory, and their symlink targets -- the actual cert
+    These paths are normally within the "live" directory, and their
-    files -- are normally found within the "archive" directory.
+    symlink targets -- the actual cert files -- are normally found
+    within the "archive" directory.
 
-    :ivar cert: The path to the symlink representing the current version
+    :ivar str cert: The path to the symlink representing the current
-        of the certificate managed by this lineage.
+        version of the certificate managed by this lineage.
-    :type cert: str
+    :ivar str privkey: The path to the symlink representing the current
-
+        version of the private key managed by this lineage.
-    :ivar privkey: The path to the symlink representing the current version
+    :ivar str chain: 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
         of the chain managed by this lineage.
-    :type chain: str
+    :ivar str fullchain: The path to the symlink representing the
-
+        current version of the fullchain (combined chain and cert)
-    :ivar fullchain: The path to the symlink representing the current version
+        managed by this lineage.
-        of the fullchain (combined chain and cert) managed by this lineage.
+    :ivar configobj.ConfigObj configuration: The renewal configuration
-    :type fullchain: str
+        options associated with this lineage, obtained from parsing the
-
+        renewal configuration file and/or systemwide defaults.
-    :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`"""
 
+    """
     def __init__(self, configfile, config_opts=None):
         """Instantiate a RenewableCert object from an existing lineage.
 
-        :param :class:`configobj.ConfigObj` configfile: an already-parsed
+        :param configobj.ConfigObj configfile: an already-parsed
-            ConfigObj object made from reading the renewal config file that
+        ConfigObj object made from reading the renewal config file
-            defines this lineage.
+        that defines this lineage.  :param configobj.ConfigObj
-        :param :class:`configobj.ConfigObj` config_opts: systemwide defaults
+        config_opts: systemwide defaults for renewal properties not
-            for renewal properties not otherwise specified in the individual
+        otherwise specified in the individual renewal config file.
-            renewal config file.
 
-        :raises ValueError: if the configuration file's name didn't end in
+        :raises ValueError: if the configuration file's name didn't end
-            ".conf", or the file is missing or broken.
+            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")
@@ -109,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)
+        self.configuration = config_with_defaults(config_opts)
-        defaults_copy.merge(config_opts if config_opts is not None
-                            else configobj.ConfigObj())
-        self.configuration = defaults_copy
         self.configuration.merge(self.configfile)
 
         if not all(x in self.configuration for x in ALL_FOUR):
@@ -127,10 +124,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
+        :returns: Whether the files stored in connection with this
-            lineage appear to be correct and consistent with one another.
+            lineage appear to be correct and consistent with one
-        :rtype: bool"""
+            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 +181,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 +203,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.
+        :returns: The path to the current version of the specified
-        :rtype: str"""
+            member.
+        :rtype: str
 
+        """
         if kind not in ALL_FOUR:
             raise ValueError("unknown kind of item")
         link = getattr(self, kind)
@@ -217,16 +221,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
+        For example, if kind is "chain" and the current chain link
-        is "chain" and the current chain link points to a file named
+        points to a file named "chain7.pem", returns the integer 7.
-        "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 +246,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
+        .. warning:: The specified version may not exist in this
-        is no guarantee that the file path returned by this method actually
+           lineage. There is no guarantee that the file path returned
-        exists.
+           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.
+        :returns: The path to the specified version of the specified member.
-        :rtype: str"""
+        :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",
+        :param str kind: the lineage member item (
-            "chain", or "fullchain")
+            ``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 +285,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",
+        :param str kind: the lineage member item (``cert``,
-            "chain", or "fullchain")
+            ``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,
+        :returns: the newest available version for which all members
-            privkey, chain, and fullchain) exist
+            (``cert, ``privkey``, ``chain``, and ``fullchain``) exist
-        :rtype: int"""
+        :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 +312,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
+        :returns: the smallest version number that is larger than any
-            of any item currently stored in this lineage
+            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 +328,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
+        :returns: ``True`` if there is a complete version of this
-            a larger version number than the current version, and False
+            lineage with a larger version number than the current
-            otherwise
+            version, and ``False`` otherwis
-        :rtype: bool"""
+        :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 +347,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 +393,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
+        :returns: the notBefore value from the specified cert version in
-            lineage
+            this lineage
-        :rtype: :class:`datetime.datetime`"""
+        :rtype: :class:`datetime.datetime`
 
+        """
         return self._notafterbefore(lambda x509: x509.get_notBefore(), version)
 
     def notafter(self, version=None):
@@ -396,24 +407,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
+        :returns: the notAfter value from the specified cert version in
-            lineage
+            this lineage
-        :rtype: :class:`datetime.datetime`"""
+        :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
+        This is a policy question and does not only depend on whether
-        is a newer version of the cert.  (This considers whether autodeployment
+        there is a newer version of the cert. (This considers whether
-        is enabled, whether a relevant newer version exists, and whether the
+        autodeployment is enabled, whether a relevant newer version
-        time interval for autodeployment has been reached.)
+        exists, and whether the time interval for autodeployment has
+        been reached.)
 
-        :returns: whether the lineage now ought to autodeploy an existing
+        :returns: whether the lineage now ought to autodeploy an
-            newer cert version
+            existing newer cert version
-        :rtype: bool"""
+        :rtype: bool
 
+        """
         if ("autodeploy" not in self.configuration or
                 self.configuration.as_bool("autodeploy")):
             if self.has_pending_deployment():
@@ -431,17 +445,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
+        Also returns True if the cert version is declared as intended
-        revoked according to Let's Encrypt OCSP extensions.  (If no version
+        to be revoked according to Let's Encrypt OCSP extensions.
-        is specified, uses the current version.)
+        (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 +466,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
+        This is a policy question and does not only depend on whether
-        cert is expired.  (This considers whether autorenewal is enabled,
+        the cert is expired. (This considers whether autorenewal is
-        whether the cert is revoked, and whether the time interval for
+        enabled, whether the cert is revoked, and whether the time
-        autorenewal has been reached.)
+        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,38 +503,35 @@ 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
+        Attempts to create a certificate lineage -- enrolled for
-        future renewal -- with the (suggested) lineage name lineagename,
+        potential future renewal -- with the (suggested) lineage name
-        and the associated cert, privkey, and chain (the associated
+        lineagename, and the associated cert, privkey, and chain (the
-        fullchain will be created automatically).  Optional configurator
+        associated fullchain will be created automatically). Optional
-        and renewalparams record the configuration that was originally
+        configurator and renewalparams record the configuration that was
-        used to obtain this cert, so that it can be reused later during
+        originally used to obtain this cert, so that it can be reused
-        automated renewal.
+        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
 
         :returns: the newly-created RenewalCert object
         :rtype: :class:`storage.renewableCert`"""
 
-        defaults_copy = copy.deepcopy(constants.RENEWER_DEFAULTS)
+        config = config_with_defaults(config)
-        defaults_copy.merge(config if config is not None
-                            else configobj.ConfigObj())
-        config = defaults_copy
         # 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
@@ -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
+        Returns the new version number that was created.
-        update links to deploy this version.
 
-        :param int prior_version: the old version to which this version is
+        .. note:: this function does NOT update links to deploy this
-            regarded as a successor (used to choose a privkey, if the key
+                  version
-            has not changed, but otherwise this information is not permanently
+
-            recorded anywhere)
+        :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,
+        :param str new_privkey: the new private key, in PEM format,
-            if the private key has not changed
+            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

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))

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.
+    """Unlink all four items associated with this RenewableCert."""
-    (Helper function.)"""
     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.
+    """Put dummy data into all four files of this RenewableCert."""
-    (Helper function.)"""
     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
+    """Tests for letsencrypt.renewer.*."""
-    within renewer.py."""
     def setUp(self):
         from letsencrypt import storage
         self.tempdir = tempfile.mkdtemp()
@@ -167,7 +165,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 +213,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 +248,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 +259,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 +276,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 +369,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 +401,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):
@@ -590,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")
@@ -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

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
 

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 %s\n" % cert_path
-        self.save_notes += "\tssl_certificate_key %s\n" % key
+        self.save_notes += "\tssl_certificate_key %s\n" % key_path
 
     #######################
     # Vhost parsing methods

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