diff --git a/letsencrypt/storage.py b/letsencrypt/storage.py index 7d8801f10..76b54fab3 100644 --- a/letsencrypt/storage.py +++ b/letsencrypt/storage.py @@ -208,8 +208,8 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes # happen as a result of random tampering by a sysadmin, or # filesystem errors, or crashes.) - def _lockfiles(self): - """Returns the kind and path of all used lockfiles. + def _symlink_lockfiles(self): + """Returns the kind and path of all symlink lockfiles. :returns: list of (kind, lockfile) tuples :rtype: list @@ -230,7 +230,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes has no effect. """ - lockfiles = self._lockfiles() + lockfiles = self._symlink_lockfiles() if all(os.path.exists(lockfile[1]) for lockfile in lockfiles): for kind, lockfile in lockfiles: link = getattr(self, kind) @@ -418,7 +418,7 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes """ with error_handler.ErrorHandler(self._fix_symlinks): - lockfiles = self._lockfiles() + lockfiles = self._symlink_lockfiles() for kind, lockfile in lockfiles: with open(lockfile, "w") as f: f.write("{0}\n".format(self.current_target(kind))) diff --git a/letsencrypt/tests/renewer_test.py b/letsencrypt/tests/renewer_test.py index bc8afbcb5..e8ad168e7 100644 --- a/letsencrypt/tests/renewer_test.py +++ b/letsencrypt/tests/renewer_test.py @@ -290,7 +290,7 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertEqual("cert8.pem", os.path.basename(self.test_rc.version("cert", 8))) - def test_update_all_links_to(self): + def test_update_all_links_to_success(self): for ver in xrange(1, 6): for kind in ALL_FOUR: where = getattr(self.test_rc, kind) @@ -308,6 +308,38 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertEqual(ver, self.test_rc.current_version(kind)) self.assertEqual(self.test_rc.latest_common_version(), 5) + def test_update_all_links_to_partial_failure(self): + def unlink_or_raise(path, real_unlink=os.unlink): + # pylint: disable=missing-docstring + if "fullchain" in path and not path.endswith(".pem"): + raise ValueError + else: + real_unlink(path) + + self._write_out_ex_kinds() + with mock.patch("letsencrypt.storage.os.unlink") as mock_unlink: + mock_unlink.side_effect = unlink_or_raise + self.assertRaises(ValueError, self.test_rc.update_all_links_to, 12) + + for kind in ALL_FOUR: + self.assertEqual(self.test_rc.current_version(kind), 12) + + def test_update_all_links_to_full_failure(self): + def unlink_or_raise(path, real_unlink=os.unlink): + # pylint: disable=missing-docstring + if "fullchain" in path: + raise ValueError + else: + real_unlink(path) + + self._write_out_ex_kinds() + with mock.patch("letsencrypt.storage.os.unlink") as mock_unlink: + mock_unlink.side_effect = unlink_or_raise + self.assertRaises(ValueError, self.test_rc.update_all_links_to, 12) + + for kind in ALL_FOUR: + self.assertEqual(self.test_rc.current_version(kind), 11) + def test_has_pending_deployment(self): for ver in xrange(1, 6): for kind in ALL_FOUR: