diff --git a/letsencrypt/errors.py b/letsencrypt/errors.py index 82331fced..b15728c39 100644 --- a/letsencrypt/errors.py +++ b/letsencrypt/errors.py @@ -5,10 +5,6 @@ class Error(Exception): """Generic Let's Encrypt client error.""" -class SubprocessError(Error): - """Subprocess handling error.""" - - class AccountStorageError(Error): """Generic `.AccountStorage` error.""" @@ -21,6 +17,14 @@ class ReverterError(Error): """Let's Encrypt Reverter error.""" +class SubprocessError(Error): + """Subprocess handling error.""" + + +class CertStorageError(Error): + """Generic `.CertStorage` error.""" + + # Auth Handler Errors class AuthorizationError(Error): """Authorization error.""" diff --git a/letsencrypt/renewer.py b/letsencrypt/renewer.py index bc5277333..e26e8742b 100644 --- a/letsencrypt/renewer.py +++ b/letsencrypt/renewer.py @@ -20,6 +20,7 @@ from letsencrypt import configuration from letsencrypt import cli from letsencrypt import client from letsencrypt import crypto_util +from letsencrypt import errors from letsencrypt import notify from letsencrypt import storage @@ -164,7 +165,7 @@ def main(config=None, args=sys.argv[1:]): # dramatically improve performance for large deployments # where autorenewal is widely turned off. cert = storage.RenewableCert(rc_config, cli_config=cli_config) - except ValueError: + except errors.CertStorageError: # This indicates an invalid renewal configuration file, such # as one missing a required parameter (in the future, perhaps # also one that is internally inconsistent or is missing a diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index 4ad1216e6..431f56aff 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -11,6 +11,7 @@ import pytz import pyrfc3339 from letsencrypt import constants +from letsencrypt import errors from letsencrypt import le_util ALL_FOUR = ("cert", "privkey", "chain", "fullchain") @@ -90,7 +91,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes renewal config file. :param .RenewerConfiguration cli_config: - :raises ValueError: if the configuration file's name didn't end + :raises .CertStorageError: if the configuration file's name didn't end in ".conf", or the file is missing or broken. :raises TypeError: if the provided renewal configuration isn't a ConfigObj object. @@ -99,7 +100,8 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes self.cli_config = cli_config if isinstance(configfile, configobj.ConfigObj): if not os.path.basename(configfile.filename).endswith(".conf"): - raise ValueError("renewal config file name must end in .conf") + raise errors.CertStorageError( + "renewal config file name must end in .conf") self.lineagename = os.path.basename( configfile.filename)[:-len(".conf")] else: @@ -117,8 +119,9 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes self.configuration.merge(self.configfile) if not all(x in self.configuration for x in ALL_FOUR): - raise ValueError("renewal config file {0} is missing a required " - "file reference".format(configfile)) + raise errors.CertStorageError( + "renewal config file {0} is missing a required " + "file reference".format(configfile)) self.cert = self.configuration["cert"] self.privkey = self.configuration["privkey"] @@ -213,7 +216,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes """ if kind not in ALL_FOUR: - raise ValueError("unknown kind of item") + raise errors.CertStorageError("unknown kind of item") link = getattr(self, kind) if not os.path.exists(link): return None @@ -236,7 +239,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes """ if kind not in ALL_FOUR: - raise ValueError("unknown kind of item") + raise errors.CertStorageError("unknown kind of item") pattern = re.compile(r"^{0}([0-9]+)\.pem$".format(kind)) target = self.current_target(kind) if target is None or not os.path.exists(target): @@ -263,12 +266,12 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes """ if kind not in ALL_FOUR: - raise ValueError("unknown kind of item") + raise errors.CertStorageError("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 lternative versions of the specified kind of item exist? + """Which alternative 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. @@ -281,7 +284,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes """ if kind not in ALL_FOUR: - raise ValueError("unknown kind of item") + raise errors.CertStorageError("unknown kind of item") where = os.path.dirname(self.current_target(kind)) files = os.listdir(where) pattern = re.compile(r"^{0}([0-9]+)\.pem$".format(kind)) @@ -308,7 +311,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes :rtype: int """ - # TODO: this can raise ValueError if there is no version overlap + # TODO: this can raise CertStorageError if there is no version overlap # (it should probably return None instead) # TODO: this can raise a spurious AttributeError if the current # link for any kind is missing (it should probably return None) @@ -355,7 +358,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes """ if kind not in ALL_FOUR: - raise ValueError("unknown kind of item") + raise errors.CertStorageError("unknown kind of item") link = getattr(self, kind) filename = "{0}{1}.pem".format(kind, version) # Relative rather than absolute target directory @@ -550,7 +553,8 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes config_file, config_filename = le_util.unique_lineage_name( cli_config.renewal_configs_dir, lineagename) if not config_filename.endswith(".conf"): - raise ValueError("renewal config file name must end in .conf") + raise errors.CertStorageError( + "renewal config file name must end in .conf") # Determine where on disk everything will go # lineagename will now potentially be modified based on which @@ -559,9 +563,11 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes archive = os.path.join(cli_config.archive_dir, lineagename) live_dir = os.path.join(cli_config.live_dir, lineagename) if os.path.exists(archive): - raise ValueError("archive directory exists for " + lineagename) + raise errors.CertStorageError( + "archive directory exists for " + lineagename) if os.path.exists(live_dir): - raise ValueError("live directory exists for " + lineagename) + raise errors.CertStorageError( + "live directory exists for " + lineagename) os.mkdir(archive) os.mkdir(live_dir) relative_archive = os.path.join("..", "..", "archive", lineagename) diff --git a/letsencrypt/tests/renewer_test.py b/letsencrypt/tests/renewer_test.py index 65bfce314..1b58d9e0f 100644 --- a/letsencrypt/tests/renewer_test.py +++ b/letsencrypt/tests/renewer_test.py @@ -10,6 +10,7 @@ import mock import pytz from letsencrypt import configuration +from letsencrypt import errors from letsencrypt.storage import ALL_FOUR from letsencrypt.tests import test_util @@ -78,7 +79,8 @@ class RenewableCertTests(unittest.TestCase): for kind in ALL_FOUR: config["cert"] = "nonexistent_" + kind + ".pem" config.filename = "nonexistent_sillyfile" - self.assertRaises(ValueError, storage.RenewableCert, config, defaults) + self.assertRaises( + errors.CertStorageError, storage.RenewableCert, config, defaults) self.assertRaises(TypeError, storage.RenewableCert, "fun", defaults) def test_renewal_incomplete_config(self): @@ -92,7 +94,8 @@ class RenewableCertTests(unittest.TestCase): config["chain"] = "imaginary_chain.pem" config["fullchain"] = "imaginary_fullchain.pem" config.filename = "imaginary_config.conf" - self.assertRaises(ValueError, storage.RenewableCert, config, defaults) + self.assertRaises( + errors.CertStorageError, storage.RenewableCert, config, defaults) def test_consistent(self): # pylint: disable=too-many-statements oldcert = self.test_rc.cert @@ -481,11 +484,13 @@ class RenewableCertTests(unittest.TestCase): # Now trigger the detection of already existing files os.mkdir(os.path.join( self.cli_config.live_dir, "the-lineage.com-0002")) - self.assertRaises(ValueError, storage.RenewableCert.new_lineage, + self.assertRaises(errors.CertStorageError, + storage.RenewableCert.new_lineage, "the-lineage.com", "cert3", "privkey3", "chain3", None, self.defaults, self.cli_config) os.mkdir(os.path.join(self.cli_config.archive_dir, "other-example.com")) - self.assertRaises(ValueError, storage.RenewableCert.new_lineage, + self.assertRaises(errors.CertStorageError, + storage.RenewableCert.new_lineage, "other-example.com", "cert4", "privkey4", "chain4", None, self.defaults, self.cli_config) # Make sure it can accept renewal parameters @@ -518,20 +523,27 @@ class RenewableCertTests(unittest.TestCase): def test_invalid_config_filename(self, mock_uln): from letsencrypt import storage mock_uln.return_value = "this_does_not_end_with_dot_conf", "yikes" - self.assertRaises(ValueError, storage.RenewableCert.new_lineage, + self.assertRaises(errors.CertStorageError, + storage.RenewableCert.new_lineage, "example.com", "cert", "privkey", "chain", None, self.defaults, self.cli_config) def test_bad_kind(self): - self.assertRaises(ValueError, self.test_rc.current_target, "elephant") - self.assertRaises(ValueError, self.test_rc.current_version, "elephant") - self.assertRaises(ValueError, self.test_rc.version, "elephant", 17) - self.assertRaises(ValueError, self.test_rc.available_versions, - "elephant") - self.assertRaises(ValueError, self.test_rc.newest_available_version, - "elephant") - self.assertRaises(ValueError, self.test_rc.update_link_to, - "elephant", 17) + self.assertRaises( + errors.CertStorageError, self.test_rc.current_target, "elephant") + self.assertRaises( + errors.CertStorageError, self.test_rc.current_version, "elephant") + self.assertRaises( + errors.CertStorageError, self.test_rc.version, "elephant", 17) + self.assertRaises( + errors.CertStorageError, + self.test_rc.available_versions, "elephant") + self.assertRaises( + errors.CertStorageError, + self.test_rc.newest_available_version, "elephant") + self.assertRaises( + errors.CertStorageError, + self.test_rc.update_link_to, "elephant", 17) def test_ocsp_revoked(self): # XXX: This is currently hardcoded to False due to a lack of an @@ -651,7 +663,7 @@ class RenewableCertTests(unittest.TestCase): f.write("incomplete = configfile\n") renewer.main(self.defaults, args=[ '--config-dir', self.cli_config.config_dir]) - # The ValueError is caught inside and nothing happens. + # The errors.CertStorageError is caught inside and nothing happens. if __name__ == "__main__":