diff --git a/letsencrypt/client.py b/letsencrypt/client.py index c82131af3..7a78add38 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -268,19 +268,15 @@ class Client(object): :param .RenewableCert cert: Newly issued certificate """ - if ("autorenew" not in cert.configuration or - cert.configuration.as_bool("autorenew")): - if ("autodeploy" not in cert.configuration or - cert.configuration.as_bool("autodeploy")): + if cert.autorenewal_is_enabled(): + if cert.autodeployment_is_enabled(): msg = "Automatic renewal and deployment has " else: msg = "Automatic renewal but not automatic deployment has " + elif cert.autodeployment_is_enabled(): + msg = "Automatic deployment but not automatic renewal has " else: - if ("autodeploy" not in cert.configuration or - cert.configuration.as_bool("autodeploy")): - msg = "Automatic deployment but not automatic renewal has " - else: - msg = "Automatic renewal and deployment has not " + msg = "Automatic renewal and deployment has not " msg += ("been enabled for your certificate. These settings can be " "configured in the directories under {0}.").format( diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index be270a762..8a0f4829e 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -129,7 +129,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes self.chain = self.configuration["chain"] self.fullchain = self.configuration["fullchain"] - def consistent(self): + def _consistent(self): """Are the files associated with this lineage self-consistent? :returns: Whether the files stored in connection with this @@ -187,7 +187,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes # for x in ALL_FOUR))) == 1 return True - def fix(self): + def _fix(self): """Attempt to fix defects or inconsistencies in this lineage. .. todo:: Currently unimplemented. @@ -347,7 +347,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes smallest_current = min(self.current_version(x) for x in ALL_FOUR) return smallest_current < self.latest_common_version() - def update_link_to(self, kind, version): + def _update_link_to(self, kind, version): """Make the specified item point at the specified version. (Note that this method doesn't verify that the specified version @@ -379,7 +379,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes :param int version: the desired version""" for kind in ALL_FOUR: - self.update_link_to(kind, version) + self._update_link_to(kind, version) def _notafterbefore(self, method, version): """Internal helper function for finding notbefore/notafter.""" @@ -439,6 +439,18 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes with open(target) as f: return crypto_util.get_sans_from_cert(f.read()) + def autodeployment_is_enabled(self): + """Is automatic deployment enabled for this cert? + + If autodeploy is not specified, defaults to True. + + :returns: True if automatic deployment is enabled + :rtype: bool + + """ + return ("autodeploy" not in self.configuration or + self.configuration.as_bool("autodeploy")) + def should_autodeploy(self): """Should this lineage now automatically deploy a newer version? @@ -453,8 +465,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes :rtype: bool """ - if ("autodeploy" not in self.configuration or - self.configuration.as_bool("autodeploy")): + if self.autodeployment_is_enabled(): if self.has_pending_deployment(): interval = self.configuration.get("deploy_before_expiry", "5 days") @@ -488,6 +499,18 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes # certificate is not revoked). return False + def autorenewal_is_enabled(self): + """Is automatic renewal enabled for this cert? + + If autorenew is not specified, defaults to True. + + :returns: True if automatic renewal is enabled + :rtype: bool + + """ + return ("autorenew" not in self.configuration or + self.configuration.as_bool("autorenew")) + def should_autorenew(self): """Should we now try to autorenew the most recent cert version? @@ -504,8 +527,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes :rtype: bool """ - if ("autorenew" not in self.configuration or - self.configuration.as_bool("autorenew")): + if self.autorenewal_is_enabled(): # Consider whether to attempt to autorenew this cert now # Renewals on the basis of revocation diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 1a232bccb..1e63bdbb6 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -4,14 +4,12 @@ import shutil import tempfile import unittest -import configobj import OpenSSL import mock from acme import jose from letsencrypt import account -from letsencrypt import configuration from letsencrypt import errors from letsencrypt import le_util @@ -120,29 +118,28 @@ class ClientTest(unittest.TestCase): def test_report_renewal_status(self, mock_zope): # pylint: disable=protected-access cert = mock.MagicMock() - cert.configuration = configobj.ConfigObj() - cert.cli_config = configuration.RenewerConfiguration(self.config) + cert.cli_config.renewal_configs_dir = "/foo/bar/baz" - cert.configuration["autorenew"] = "True" - cert.configuration["autodeploy"] = "True" + cert.autorenewal_is_enabled.return_value = True + cert.autodeployment_is_enabled.return_value = True self.client._report_renewal_status(cert) msg = mock_zope().add_message.call_args[0][0] self.assertTrue("renewal and deployment has been" in msg) self.assertTrue(cert.cli_config.renewal_configs_dir in msg) - cert.configuration["autorenew"] = "False" + cert.autorenewal_is_enabled.return_value = False self.client._report_renewal_status(cert) msg = mock_zope().add_message.call_args[0][0] self.assertTrue("deployment but not automatic renewal" in msg) self.assertTrue(cert.cli_config.renewal_configs_dir in msg) - cert.configuration["autodeploy"] = "False" + cert.autodeployment_is_enabled.return_value = False self.client._report_renewal_status(cert) msg = mock_zope().add_message.call_args[0][0] self.assertTrue("renewal and deployment has not" in msg) self.assertTrue(cert.cli_config.renewal_configs_dir in msg) - cert.configuration["autorenew"] = "True" + cert.autorenewal_is_enabled.return_value = True self.client._report_renewal_status(cert) msg = mock_zope().add_message.call_args[0][0] self.assertTrue("renewal but not automatic deployment" in msg) diff --git a/letsencrypt/tests/renewer_test.py b/letsencrypt/tests/renewer_test.py index e67631605..6f115abf9 100644 --- a/letsencrypt/tests/renewer_test.py +++ b/letsencrypt/tests/renewer_test.py @@ -123,46 +123,47 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertRaises( errors.CertStorageError, storage.RenewableCert, config, defaults) - def test_consistent(self): # pylint: disable=too-many-statements + def test_consistent(self): + # pylint: disable=too-many-statements,protected-access oldcert = self.test_rc.cert self.test_rc.cert = "relative/path" # Absolute path for item requirement - self.assertFalse(self.test_rc.consistent()) + self.assertFalse(self.test_rc._consistent()) self.test_rc.cert = oldcert # Items must exist requirement - self.assertFalse(self.test_rc.consistent()) + self.assertFalse(self.test_rc._consistent()) # Items must be symlinks requirements fill_with_sample_data(self.test_rc) - self.assertFalse(self.test_rc.consistent()) + self.assertFalse(self.test_rc._consistent()) unlink_all(self.test_rc) # Items must point to desired place if they are relative for kind in ALL_FOUR: os.symlink(os.path.join("..", kind + "17.pem"), getattr(self.test_rc, kind)) - self.assertFalse(self.test_rc.consistent()) + self.assertFalse(self.test_rc._consistent()) unlink_all(self.test_rc) # Items must point to desired place if they are absolute for kind in ALL_FOUR: os.symlink(os.path.join(self.tempdir, kind + "17.pem"), getattr(self.test_rc, kind)) - self.assertFalse(self.test_rc.consistent()) + self.assertFalse(self.test_rc._consistent()) unlink_all(self.test_rc) # Items must point to things that exist for kind in ALL_FOUR: os.symlink(os.path.join("..", "..", "archive", "example.org", kind + "17.pem"), getattr(self.test_rc, kind)) - self.assertFalse(self.test_rc.consistent()) + self.assertFalse(self.test_rc._consistent()) # This version should work fill_with_sample_data(self.test_rc) - self.assertTrue(self.test_rc.consistent()) + self.assertTrue(self.test_rc._consistent()) # Items must point to things that follow the naming convention os.unlink(self.test_rc.fullchain) os.symlink(os.path.join("..", "..", "archive", "example.org", "fullchain_17.pem"), self.test_rc.fullchain) with open(self.test_rc.fullchain, "w") as f: f.write("wrongly-named fullchain") - self.assertFalse(self.test_rc.consistent()) + self.assertFalse(self.test_rc._consistent()) def test_current_target(self): # Relative path logic @@ -259,14 +260,15 @@ class RenewableCertTests(BaseRenewableCertTest): with open(where, "w") as f: f.write(kind) self.assertEqual(ver, self.test_rc.current_version(kind)) - self.test_rc.update_link_to("cert", 3) - self.test_rc.update_link_to("privkey", 2) + # pylint: disable=protected-access + self.test_rc._update_link_to("cert", 3) + self.test_rc._update_link_to("privkey", 2) self.assertEqual(3, self.test_rc.current_version("cert")) self.assertEqual(2, self.test_rc.current_version("privkey")) self.assertEqual(5, self.test_rc.current_version("chain")) self.assertEqual(5, self.test_rc.current_version("fullchain")) # Currently we are allowed to update to a version that doesn't exist - self.test_rc.update_link_to("chain", 3000) + self.test_rc._update_link_to("chain", 3000) # However, current_version doesn't allow querying the resulting # version (because it's a broken link). self.assertEqual(os.path.basename(os.readlink(self.test_rc.chain)), @@ -405,6 +407,14 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertEqual(self.test_rc.should_autodeploy(), result) self.assertEqual(self.test_rc.should_autorenew(), result) + def test_autodeployment_is_enabled(self): + self.assertTrue(self.test_rc.autodeployment_is_enabled()) + self.test_rc.configuration["autodeploy"] = "1" + self.assertTrue(self.test_rc.autodeployment_is_enabled()) + + self.test_rc.configuration["autodeploy"] = "0" + self.assertFalse(self.test_rc.autodeployment_is_enabled()) + def test_should_autodeploy(self): """Test should_autodeploy() on the basis of reasons other than expiry time window.""" @@ -425,6 +435,14 @@ class RenewableCertTests(BaseRenewableCertTest): f.write(kind) self.assertFalse(self.test_rc.should_autodeploy()) + def test_autorenewal_is_enabled(self): + self.assertTrue(self.test_rc.autorenewal_is_enabled()) + self.test_rc.configuration["autorenew"] = "1" + self.assertTrue(self.test_rc.autorenewal_is_enabled()) + + self.test_rc.configuration["autorenew"] = "0" + self.assertFalse(self.test_rc.autorenewal_is_enabled()) + @mock.patch("letsencrypt.storage.RenewableCert.ocsp_revoked") def test_should_autorenew(self, mock_ocsp): """Test should_autorenew on the basis of reasons other than @@ -507,7 +525,8 @@ class RenewableCertTests(BaseRenewableCertTest): self.defaults, self.cli_config) # This consistency check tests most relevant properties about the # newly created cert lineage. - self.assertTrue(result.consistent()) + # pylint: disable=protected-access + self.assertTrue(result._consistent()) self.assertTrue(os.path.exists(os.path.join( self.cli_config.renewal_configs_dir, "the-lineage.com.conf"))) with open(result.fullchain) as f: @@ -578,9 +597,10 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertRaises( errors.CertStorageError, self.test_rc.newest_available_version, "elephant") + # pylint: disable=protected-access self.assertRaises( errors.CertStorageError, - self.test_rc.update_link_to, "elephant", 17) + 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