Merge pull request #3053 from certbot/renewal-conf-versions

Store client version in renewal conf file

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

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):
@@ -63,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]
 
@@ -259,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"]

certbot/tests/le_util_test.py

@@ -10,6 +10,7 @@ import unittest
 import mock
 import six
 
+import certbot
 from certbot import errors
 
 
@@ -339,5 +340,32 @@ 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"))
+
+    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"))
+
+    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"))
+
+    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

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
@@ -137,6 +138,28 @@ 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
+
+        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
@@ -760,11 +783,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