Merge pull request #878 from letsencrypt/automation_is_enabled

Ensures renewal settings are reported correctly
This commit is contained in:
bmw 2015-10-05 14:54:02 -07:00
commit 0aaf26bdda
4 changed files with 75 additions and 40 deletions

View file

@ -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(

View file

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

View file

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

View file

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