From d4de026372780f079af3e6b811da991bad824ffe Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 23 May 2016 13:37:47 -0700 Subject: [PATCH 01/11] Add get_strict_version --- certbot/le_util.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/certbot/le_util.py b/certbot/le_util.py index f5148b949..1e5997d92 100644 --- a/certbot/le_util.py +++ b/certbot/le_util.py @@ -1,6 +1,9 @@ """Utilities for all Certbot.""" import argparse import collections +# distutils.version under virtualenv confuses pylint +# For more info, see: https://github.com/PyCQA/pylint/issues/73 +import distutils.version # pylint: disable=import-error,no-name-in-module import errno import logging import os @@ -342,3 +345,17 @@ def enforce_domain_sanity(domain): if not fqdn.match(domain): raise errors.ConfigurationError("Requested domain {0} is not a FQDN".format(domain)) return domain + + +def get_strict_version(normalized): + """Converts a normalized version to a strict version. + + :param str normalized: normalized version string + + :returns: An equivalent strict version + :rtype: distutils.version.StrictVersion + + """ + # strict version ending with "a" and a number designates a pre-release + # pylint: disable=no-member + return distutils.version.StrictVersion(normalized.replace(".dev", "a")) From 40a0354b360214485fbd19d90e24c9c9d040856c Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 23 May 2016 13:50:14 -0700 Subject: [PATCH 02/11] Start get_strict_version tests --- certbot/tests/le_util_test.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/certbot/tests/le_util_test.py b/certbot/tests/le_util_test.py index b6da4525f..db76615ee 100644 --- a/certbot/tests/le_util_test.py +++ b/certbot/tests/le_util_test.py @@ -339,5 +339,18 @@ class EnforceDomainSanityTest(unittest.TestCase): u"eichh\u00f6rnchen.example.com") +class GetStrictVersionTest(unittest.TestCase): + """Tests for certbot.le_util.get_strict_version.""" + + @classmethod + def _call(cls, *args, **kwargs): + from certbot.le_util import get_strict_version + return get_strict_version(*args, **kwargs) + + def test_two_dev_versions(self): + self.assertTrue( + self._call("0.0.0.dev20151006") < self._call("0.0.0.dev20151008")) + + if __name__ == "__main__": unittest.main() # pragma: no cover From 4caba1fc75749eb6c4f45dd61e314f62e26b962b Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 23 May 2016 13:53:09 -0700 Subject: [PATCH 03/11] Test mixed versions --- certbot/tests/le_util_test.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/certbot/tests/le_util_test.py b/certbot/tests/le_util_test.py index db76615ee..2dcecae2c 100644 --- a/certbot/tests/le_util_test.py +++ b/certbot/tests/le_util_test.py @@ -351,6 +351,10 @@ class GetStrictVersionTest(unittest.TestCase): self.assertTrue( self._call("0.0.0.dev20151006") < self._call("0.0.0.dev20151008")) + def test_one_dev_one_release_version(self): + self.assertTrue(self._call("1.0.0.dev0") < self._call("1.0.0")) + self.assertTrue(self._call("1.0.0") < self._call("1.0.1.dev0")) + if __name__ == "__main__": unittest.main() # pragma: no cover From ac37b9de6fdd5f935df4a5f83e0840f88df24d29 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 23 May 2016 13:55:17 -0700 Subject: [PATCH 04/11] test release version comparison --- certbot/tests/le_util_test.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/certbot/tests/le_util_test.py b/certbot/tests/le_util_test.py index 2dcecae2c..5bf93a406 100644 --- a/certbot/tests/le_util_test.py +++ b/certbot/tests/le_util_test.py @@ -355,6 +355,11 @@ class GetStrictVersionTest(unittest.TestCase): self.assertTrue(self._call("1.0.0.dev0") < self._call("1.0.0")) self.assertTrue(self._call("1.0.0") < self._call("1.0.1.dev0")) + def test_two_release_versions(self): + self.assertTrue(self._call("0.0.0") < self._call("0.0.1")) + self.assertTrue(self._call("0.0.0") < self._call("0.1.0")) + self.assertTrue(self._call("0.0.0") < self._call("1.0.0")) + if __name__ == "__main__": unittest.main() # pragma: no cover From 536234c5595347a472060beccdd9fd2e83791483 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 23 May 2016 13:59:16 -0700 Subject: [PATCH 05/11] try and catch problems if we do something silly with our version in the future --- certbot/tests/le_util_test.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/certbot/tests/le_util_test.py b/certbot/tests/le_util_test.py index 5bf93a406..6e4eef0f1 100644 --- a/certbot/tests/le_util_test.py +++ b/certbot/tests/le_util_test.py @@ -10,6 +10,7 @@ import unittest import mock import six +import certbot from certbot import errors @@ -360,6 +361,11 @@ class GetStrictVersionTest(unittest.TestCase): self.assertTrue(self._call("0.0.0") < self._call("0.1.0")) self.assertTrue(self._call("0.0.0") < self._call("1.0.0")) + def test_current_version(self): + current_version = self._call(certbot.__version__) + self.assertTrue(self._call("0.6.0") < current_version) + self.assertTrue(current_version < self._call("99.99.99")) + if __name__ == "__main__": unittest.main() # pragma: no cover From 0e9aec20a76d06895173ef815962447d75bb87d8 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 23 May 2016 14:04:13 -0700 Subject: [PATCH 06/11] Add CURRENT_VERSION constant --- certbot/storage.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/certbot/storage.py b/certbot/storage.py index c4bfb3e28..e2b26d322 100644 --- a/certbot/storage.py +++ b/certbot/storage.py @@ -8,6 +8,7 @@ import configobj import parsedatetime import pytz +import certbot from certbot import constants from certbot import crypto_util from certbot import errors @@ -17,6 +18,7 @@ from certbot import le_util logger = logging.getLogger(__name__) ALL_FOUR = ("cert", "privkey", "chain", "fullchain") +CURRENT_VERSION = le_util.get_strict_version(certbot.__version__) def config_with_defaults(config=None): From c3c9441a599226efe261d140d547977eb6ac8d71 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 23 May 2016 14:09:31 -0700 Subject: [PATCH 07/11] Save version in renewal config file --- certbot/storage.py | 1 + 1 file changed, 1 insertion(+) diff --git a/certbot/storage.py b/certbot/storage.py index e2b26d322..d4a469ca1 100644 --- a/certbot/storage.py +++ b/certbot/storage.py @@ -65,6 +65,7 @@ def write_renewal_config(o_filename, n_filename, target, relevant_data): """ config = configobj.ConfigObj(o_filename) + config["version"] = certbot.__version__ for kind in ALL_FOUR: config[kind] = target[kind] From 3576d372a67882a584c207b7bf720c10fc8d69c6 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 23 May 2016 14:32:43 -0700 Subject: [PATCH 08/11] Add warning about parsing old configuration file --- certbot/storage.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/certbot/storage.py b/certbot/storage.py index d4a469ca1..6c13eb844 100644 --- a/certbot/storage.py +++ b/certbot/storage.py @@ -262,6 +262,14 @@ class RenewableCert(object): # pylint: disable=too-many-instance-attributes "renewal config file {0} is missing a required " "file reference".format(self.configfile)) + conf_version = self.configuration.get("version") + if (conf_version is not None and + le_util.get_strict_version(conf_version) > CURRENT_VERSION): + logger.warning( + "Attempting to parse the version %s renewal configuration " + "file found at %s with version %s of Certbot. This might not " + "work.", conf_version, config_filename, certbot.__version__) + self.cert = self.configuration["cert"] self.privkey = self.configuration["privkey"] self.chain = self.configuration["chain"] From ea53a14b57910bf9749d9304a3e93e02bea36d02 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 23 May 2016 14:58:14 -0700 Subject: [PATCH 09/11] test version is stored --- certbot/tests/storage_test.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/certbot/tests/storage_test.py b/certbot/tests/storage_test.py index be626edc5..aeba5c3ae 100644 --- a/certbot/tests/storage_test.py +++ b/certbot/tests/storage_test.py @@ -10,6 +10,7 @@ import configobj import mock import pytz +import certbot from certbot import configuration from certbot import errors from certbot.storage import ALL_FOUR @@ -760,11 +761,14 @@ class RenewableCertTests(BaseRenewableCertTest): with open(temp2, "r") as f: content = f.read() # useful value was updated - assert "useful = new_value" in content + self.assertTrue("useful = new_value" in content) # associated comment was preserved - assert "A useful value" in content + self.assertTrue("A useful value" in content) # useless value was deleted - assert "useless" not in content + self.assertTrue("useless" not in content) + # check version was stored + self.assertTrue("version = {0}".format(certbot.__version__) in content) + if __name__ == "__main__": unittest.main() # pragma: no cover From 15ddf2ea32a4f020b655c7ac4ca6d62cb0cafd50 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 23 May 2016 15:16:43 -0700 Subject: [PATCH 10/11] Test init with newer conf file --- certbot/tests/storage_test.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/certbot/tests/storage_test.py b/certbot/tests/storage_test.py index aeba5c3ae..b1444f311 100644 --- a/certbot/tests/storage_test.py +++ b/certbot/tests/storage_test.py @@ -138,6 +138,18 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertRaises(errors.CertStorageError, storage.RenewableCert, config.filename, self.cli_config) + def test_renewal_newer_version(self): + from certbot import storage + + self._write_out_ex_kinds() + self.config["version"] = "99.99.99" + self.config.write() + + with mock.patch("certbot.storage.logger") as mock_logger: + storage.RenewableCert(self.config.filename, self.cli_config) + self.assertTrue(mock_logger.warning.called) + self.assertTrue("version" in mock_logger.warning.call_args[0][0]) + def test_consistent(self): # pylint: disable=too-many-statements,protected-access oldcert = self.test_rc.cert From 4a85d59d9353e7a0567c0bc56c9729d75b96d209 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 24 May 2016 22:03:30 -0700 Subject: [PATCH 11/11] Add test for missing renewal conf version --- certbot/tests/storage_test.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/certbot/tests/storage_test.py b/certbot/tests/storage_test.py index b1444f311..f19b7d89d 100644 --- a/certbot/tests/storage_test.py +++ b/certbot/tests/storage_test.py @@ -138,6 +138,16 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertRaises(errors.CertStorageError, storage.RenewableCert, config.filename, self.cli_config) + def test_no_renewal_version(self): + from certbot import storage + + self._write_out_ex_kinds() + self.assertTrue("version" not in self.config) + + with mock.patch("certbot.storage.logger") as mock_logger: + storage.RenewableCert(self.config.filename, self.cli_config) + self.assertFalse(mock_logger.warning.called) + def test_renewal_newer_version(self): from certbot import storage