Specify archive directory in renewal configuration file (#3661)

* Switch to using absolute path in symlink

* save archive_dir to config and read it back

* cli_config.archive_dir --> cli_config.default_archive_dir

* Use archive_dir specified in renewal config file

* add helpful broken symlink info

* add docstring to method

* Add tests

* remove extraneous test imports

* fix tests

* py2.6 syntax fix

* git problems

* no dict comprehension in python2.6

* add test coverage

* More py26 wrangling

certbot/cert_manager.py

@@ -0,0 +1,22 @@
+"""Tools for managing certificates."""
+from certbot import configuration
+from certbot import renewal
+from certbot import storage
+
+def update_live_symlinks(config):
+    """Update the certificate file family symlinks to use archive_dir.
+
+    Use the information in the config file to make symlinks point to
+    the correct archive directory.
+
+    .. note:: This assumes that the installation is using a Reverter object.
+
+    :param config: Configuration.
+    :type config: :class:`certbot.interfaces.IConfig`
+
+    """
+    renewer_config = configuration.RenewerConfiguration(config)
+    for renewal_file in renewal.renewal_conf_files(renewer_config):
+        storage.RenewableCert(renewal_file,
+            configuration.RenewerConfiguration(renewer_config),
+            update_symlinks=True)

certbot/cli.py

@@ -67,6 +67,7 @@ cert. Major SUBCOMMANDS are:
   register             Perform tasks related to registering with the CA
   rollback             Rollback server configuration changes made during install
   config_changes       Show changes made to server config during installation
+  update_symlinks      Update cert symlinks based on renewal config file
   plugins              Display information about installed plugins
 
 """.format(cli_command)
@@ -322,7 +323,7 @@ class HelpfulArgumentParser(object):
                       "install": main.install, "plugins": main.plugins_cmd,
                       "register": main.register, "renew": main.renew,
                       "revoke": main.revoke, "rollback": main.rollback,
-                      "everything": main.run}
+                      "everything": main.run, "update_symlinks": main.update_symlinks}
 
         # List of topics for which additional help can be provided
         HELP_TOPICS = ["all", "security", "paths", "automation", "testing"] + list(self.VERBS)
@@ -680,7 +681,6 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
         help="Domain names to apply. For multiple domains you can use "
              "multiple -d flags or enter a comma separated list of domains "
              "as a parameter.")
-
     helpful.add(
         [None, "testing", "renew", "certonly"],
         "--dry-run", action="store_true", dest="dry_run",

certbot/configuration.py

@@ -96,7 +96,7 @@ class RenewerConfiguration(object):
         return getattr(self.namespace, name)
 
     @property
-    def archive_dir(self):  # pylint: disable=missing-docstring
+    def default_archive_dir(self):  # pylint: disable=missing-docstring
         return os.path.join(self.namespace.config_dir, constants.ARCHIVE_DIR)
 
     @property

certbot/main.py

@@ -16,6 +16,7 @@ from acme import messages
 import certbot
 
 from certbot import account
+from certbot import cert_manager
 from certbot import client
 from certbot import cli
 from certbot import crypto_util
@@ -475,6 +476,13 @@ def config_changes(config, unused_plugins):
     """
     client.view_config_changes(config, num=config.num)
 
+def update_symlinks(config, unused_plugins):
+    """Update the certificate file family symlinks
+
+    Use the information in the config file to make symlinks point to
+    the correct archive directory.
+    """
+    cert_manager.update_live_symlinks(config)
 
 def revoke(config, unused_plugins):  # TODO: coop with renewal config
     """Revoke a previously obtained certificate."""
@@ -540,7 +548,6 @@ def _csr_obtain_cert(config, le_client):
             certr, chain, config.cert_path, config.chain_path, config.fullchain_path)
         _report_new_cert(config, cert_path, cert_fullchain)
 
-
 def obtain_cert(config, plugins, lineage=None):
     """Authenticate & obtain cert, but do not install it.
 

certbot/storage.py

@@ -54,11 +54,12 @@ def add_time_interval(base_time, interval, textparser=parsedatetime.Calendar()):
     return textparser.parseDT(interval, base_time, tzinfo=tzinfo)[0]
 
 
-def write_renewal_config(o_filename, n_filename, target, relevant_data):
+def write_renewal_config(o_filename, n_filename, archive_dir, target, relevant_data):
     """Writes a renewal config file with the specified name and values.
 
     :param str o_filename: Absolute path to the previous version of config file
     :param str n_filename: Absolute path to the new destination of config file
+    :param str archive_dir: Absolute path to the archive directory
     :param dict target: Maps ALL_FOUR to their symlink paths
     :param dict relevant_data: Renewal configuration options to save
 
@@ -68,6 +69,7 @@ def write_renewal_config(o_filename, n_filename, target, relevant_data):
     """
     config = configobj.ConfigObj(o_filename)
     config["version"] = certbot.__version__
+    config["archive_dir"] = archive_dir
     for kind in ALL_FOUR:
         config[kind] = target[kind]
 
@@ -95,10 +97,11 @@ def write_renewal_config(o_filename, n_filename, target, relevant_data):
     return config
 
 
-def update_configuration(lineagename, target, cli_config):
+def update_configuration(lineagename, archive_dir, target, cli_config):
     """Modifies lineagename's config to contain the specified values.
 
     :param str lineagename: Name of the lineage being modified
+    :param str archive_dir: Absolute path to the archive directory
     :param dict target: Maps ALL_FOUR to their symlink paths
     :param .RenewerConfiguration cli_config: parsed command line
         arguments
@@ -117,7 +120,7 @@ def update_configuration(lineagename, target, cli_config):
 
     # Save only the config items that are relevant to renewal
     values = relevant_values(vars(cli_config.namespace))
-    write_renewal_config(config_filename, temp_filename, target, values)
+    write_renewal_config(config_filename, temp_filename, archive_dir, target, values)
     os.rename(temp_filename, config_filename)
 
     return configobj.ConfigObj(config_filename)
@@ -204,7 +207,7 @@ class RenewableCert(object):  # pylint: disable=too-many-instance-attributes
         renewal configuration file and/or systemwide defaults.
 
     """
-    def __init__(self, config_filename, cli_config):
+    def __init__(self, config_filename, cli_config, update_symlinks=False):
         """Instantiate a RenewableCert object from an existing lineage.
 
         :param str config_filename: the path to the renewal config file
@@ -256,8 +259,19 @@ class RenewableCert(object):  # pylint: disable=too-many-instance-attributes
         self.live_dir = os.path.dirname(self.cert)
 
         self._fix_symlinks()
+        if update_symlinks:
+            self._update_symlinks()
         self._check_symlinks()
 
+    @property
+    def archive_dir(self):
+        """Returns the default or specified archive directory"""
+        if "archive_dir" in self.configuration:
+            return self.configuration["archive_dir"]
+        else:
+            return os.path.join(
+                self.cli_config.default_archive_dir, self.lineagename)
+
     def _check_symlinks(self):
         """Raises an exception if a symlink doesn't exist"""
         for kind in ALL_FOUR:
@@ -270,6 +284,16 @@ class RenewableCert(object):  # pylint: disable=too-many-instance-attributes
                 raise errors.CertStorageError("target {0} of symlink {1} does "
                                               "not exist".format(target, link))
 
+    def _update_symlinks(self):
+        """Updates symlinks to use archive_dir"""
+        for kind in ALL_FOUR:
+            link = getattr(self, kind)
+            previous_link = get_link_target(link)
+            new_link = os.path.join(self.archive_dir, os.path.basename(previous_link))
+
+            os.unlink(link)
+            os.symlink(new_link, link)
+
     def _consistent(self):
         """Are the files associated with this lineage self-consistent?
 
@@ -297,16 +321,16 @@ class RenewableCert(object):  # pylint: disable=too-many-instance-attributes
 
             # Each element's link must point within the cert lineage's
             # directory within the official archive directory
-            desired_directory = os.path.join(
-                self.cli_config.archive_dir, self.lineagename)
-            if not os.path.samefile(os.path.dirname(target),
-                                    desired_directory):
+            if not os.path.samefile(os.path.dirname(target), self.archive_dir):
                 logger.debug("Element's link does not point within the "
                              "cert lineage's directory within the "
                              "official archive directory. Link: %s, "
                              "target directory: %s, "
-                             "archive directory: %s.",
-                             link, os.path.dirname(target), desired_directory)
+                             "archive directory: %s. If you've specified "
+                             "the archive directory in the renewal configuration "
+                             "file, you may need to update links by running "
+                             "certbot update_symlinks.",
+                             link, os.path.dirname(target), self.archive_dir)
                 return False
 
             # The link must point to a file that exists
@@ -759,7 +783,7 @@ class RenewableCert(object):  # pylint: disable=too-many-instance-attributes
         """
 
         # Examine the configuration and find the new lineage's name
-        for i in (cli_config.renewal_configs_dir, cli_config.archive_dir,
+        for i in (cli_config.renewal_configs_dir, cli_config.default_archive_dir,
                   cli_config.live_dir):
             if not os.path.exists(i):
                 os.makedirs(i, 0o700)
@@ -774,7 +798,7 @@ class RenewableCert(object):  # pylint: disable=too-many-instance-attributes
         # lineagename will now potentially be modified based on which
         # renewal configuration file could actually be created
         lineagename = os.path.basename(config_filename)[:-len(".conf")]
-        archive = os.path.join(cli_config.archive_dir, lineagename)
+        archive = os.path.join(cli_config.default_archive_dir, lineagename)
         live_dir = os.path.join(cli_config.live_dir, lineagename)
         if os.path.exists(archive):
             raise errors.CertStorageError(
@@ -786,13 +810,12 @@ class RenewableCert(object):  # pylint: disable=too-many-instance-attributes
         os.mkdir(live_dir)
         logger.debug("Archive directory %s and live "
                      "directory %s created.", archive, live_dir)
-        relative_archive = os.path.join("..", "..", "archive", lineagename)
 
         # Put the data into the appropriate files on disk
         target = dict([(kind, os.path.join(live_dir, kind + ".pem"))
                        for kind in ALL_FOUR])
         for kind in ALL_FOUR:
-            os.symlink(os.path.join(relative_archive, kind + "1.pem"),
+            os.symlink(os.path.join(archive, kind + "1.pem"),
                        target[kind])
         with open(target["cert"], "w") as f:
             logger.debug("Writing certificate to %s.", target["cert"])
@@ -816,7 +839,8 @@ class RenewableCert(object):  # pylint: disable=too-many-instance-attributes
         # Save only the config items that are relevant to renewal
         values = relevant_values(vars(cli_config.namespace))
 
-        new_config = write_renewal_config(config_filename, config_filename, target, values)
+        new_config = write_renewal_config(config_filename, config_filename, archive,
+            target, values)
         return cls(new_config.filename, cli_config)
 
     def save_successor(self, prior_version, new_cert,
@@ -851,14 +875,9 @@ class RenewableCert(object):  # pylint: disable=too-many-instance-attributes
 
         self.cli_config = cli_config
         target_version = self.next_free_version()
-        archive = self.cli_config.archive_dir
-        # XXX if anyone ever moves a renewal configuration file, this will
-        # break... perhaps prefix should be the dirname of the previous
-        # cert.pem?
-        prefix = os.path.join(archive, self.lineagename)
         target = dict(
             [(kind,
-              os.path.join(prefix, "{0}{1}.pem".format(kind, target_version)))
+              os.path.join(self.archive_dir, "{0}{1}.pem".format(kind, target_version)))
              for kind in ALL_FOUR])
 
         # Distinguish the cases where the privkey has changed and where it
@@ -868,7 +887,7 @@ class RenewableCert(object):  # pylint: disable=too-many-instance-attributes
             # The behavior below keeps the prior key by creating a new
             # symlink to the old key or the target of the old key symlink.
             old_privkey = os.path.join(
-                prefix, "privkey{0}.pem".format(prior_version))
+                self.archive_dir, "privkey{0}.pem".format(prior_version))
             if os.path.islink(old_privkey):
                 old_privkey = os.readlink(old_privkey)
             else:
@@ -894,7 +913,7 @@ class RenewableCert(object):  # pylint: disable=too-many-instance-attributes
         symlinks = dict((kind, self.configuration[kind]) for kind in ALL_FOUR)
         # Update renewal config file
         self.configfile = update_configuration(
-            self.lineagename, symlinks, cli_config)
+            self.lineagename, self.archive_dir, symlinks, cli_config)
         self.configuration = config_with_defaults(self.configfile)
 
         return target_version

certbot/tests/cert_manager_test.py

@@ -0,0 +1,101 @@
+"""Tests for certbot.cert_manager."""
+# pylint disable=protected-access
+import os
+import shutil
+import tempfile
+import unittest
+
+import configobj
+import mock
+
+from certbot import configuration
+from certbot.storage import ALL_FOUR
+
+class CertManagerTest(unittest.TestCase):
+    """Tests for certbot.cert_manager
+    """
+    def setUp(self):
+        self.tempdir = tempfile.mkdtemp()
+
+        os.makedirs(os.path.join(self.tempdir, "renewal"))
+
+        mock_namespace = mock.MagicMock(
+            config_dir=self.tempdir,
+            work_dir=self.tempdir,
+            logs_dir=self.tempdir,
+        )
+
+        self.cli_config = configuration.RenewerConfiguration(
+            namespace=mock_namespace
+        )
+
+        self.domains = {
+            "example.org": None,
+            "other.com": os.path.join(self.tempdir, "specialarchive")
+        }
+        self.configs = dict((domain, self._set_up_config(domain, self.domains[domain]))
+            for domain in self.domains)
+
+        # We also create a file that isn't a renewal config in the same
+        # location to test that logic that reads in all-and-only renewal
+        # configs will ignore it and NOT attempt to parse it.
+        junk = open(os.path.join(self.tempdir, "renewal", "IGNORE.THIS"), "w")
+        junk.write("This file should be ignored!")
+        junk.close()
+
+    def _set_up_config(self, domain, custom_archive):
+        # TODO: maybe provide RenewerConfiguration.make_dirs?
+        # TODO: main() should create those dirs, c.f. #902
+        os.makedirs(os.path.join(self.tempdir, "live", domain))
+        config = configobj.ConfigObj()
+
+        if custom_archive is not None:
+            os.makedirs(custom_archive)
+            config["archive_dir"] = custom_archive
+        else:
+            os.makedirs(os.path.join(self.tempdir, "archive", domain))
+
+        for kind in ALL_FOUR:
+            config[kind] = os.path.join(self.tempdir, "live", domain,
+                                        kind + ".pem")
+
+        config.filename = os.path.join(self.tempdir, "renewal",
+                                       domain + ".conf")
+        config.write()
+        return config
+
+    def tearDown(self):
+        shutil.rmtree(self.tempdir)
+
+    def test_update_live_symlinks(self):
+        """Test update_live_symlinks"""
+        # pylint: disable=too-many-statements
+        # create files with incorrect symlinks
+        from certbot import cert_manager
+        archive_paths = {}
+        for domain in self.domains:
+            custom_archive = self.domains[domain]
+            if custom_archive is not None:
+                archive_dir_path = custom_archive
+            else:
+                archive_dir_path = os.path.join(self.tempdir, "archive", domain)
+            archive_paths[domain] = dict((kind,
+                os.path.join(archive_dir_path, kind + "1.pem")) for kind in ALL_FOUR)
+            for kind in ALL_FOUR:
+                live_path = self.configs[domain][kind]
+                archive_path = archive_paths[domain][kind]
+                open(archive_path, 'a').close()
+                # path is incorrect but base must be correct
+                os.symlink(os.path.join(self.tempdir, kind + "1.pem"), live_path)
+
+        # run update symlinks
+        cert_manager.update_live_symlinks(self.cli_config)
+
+        # check that symlinks go where they should
+        for domain in self.domains:
+            for kind in ALL_FOUR:
+                self.assertEqual(os.readlink(self.configs[domain][kind]),
+                    archive_paths[domain][kind])
+
+if __name__ == "__main__":
+    unittest.main()  # pragma: no cover

certbot/tests/cli_test.py

@@ -272,6 +272,11 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         _, _, _, client = self._call(['config_changes'])
         self.assertEqual(1, client.view_config_changes.call_count)
 
+    @mock.patch('certbot.cert_manager.update_live_symlinks')
+    def test_update_symlinks(self, mock_cert_manager):
+        self._call_no_clientmock(['update_symlinks'])
+        self.assertEqual(1, mock_cert_manager.call_count)
+
     def test_plugins(self):
         flags = ['--init', '--prepare', '--authenticators', '--installers']
         for args in itertools.chain(

certbot/tests/configuration_test.py

@@ -104,7 +104,7 @@ class RenewerConfigurationTest(unittest.TestCase):
         constants.RENEWAL_CONFIGS_DIR = 'renewal_configs'
         constants.RENEWER_CONFIG_FILENAME = 'r.conf'
 
-        self.assertEqual(self.config.archive_dir, '/tmp/config/a')
+        self.assertEqual(self.config.default_archive_dir, '/tmp/config/a')
         self.assertEqual(self.config.live_dir, '/tmp/config/l')
         self.assertEqual(
             self.config.renewal_configs_dir, '/tmp/config/renewal_configs')
@@ -127,7 +127,7 @@ class RenewerConfigurationTest(unittest.TestCase):
         mock_namespace.logs_dir = logs_base
         config = RenewerConfiguration(NamespaceConfig(mock_namespace))
 
-        self.assertTrue(os.path.isabs(config.archive_dir))
+        self.assertTrue(os.path.isabs(config.default_archive_dir))
         self.assertTrue(os.path.isabs(config.live_dir))
         self.assertTrue(os.path.isabs(config.renewal_configs_dir))
         self.assertTrue(os.path.isabs(config.renewer_config_file))

certbot/tests/storage_test.py

@@ -593,7 +593,7 @@ class RenewableCertTests(BaseRenewableCertTest):
         self.assertRaises(errors.CertStorageError,
                           storage.RenewableCert.new_lineage, "the-lineage.com",
                           "cert3", "privkey3", "chain3", self.cli_config)
-        os.mkdir(os.path.join(self.cli_config.archive_dir, "other-example.com"))
+        os.mkdir(os.path.join(self.cli_config.default_archive_dir, "other-example.com"))
         self.assertRaises(errors.CertStorageError,
                           storage.RenewableCert.new_lineage,
                           "other-example.com", "cert4",
@@ -613,7 +613,7 @@ class RenewableCertTests(BaseRenewableCertTest):
 
         from certbot import storage
         shutil.rmtree(self.cli_config.renewal_configs_dir)
-        shutil.rmtree(self.cli_config.archive_dir)
+        shutil.rmtree(self.cli_config.default_archive_dir)
         shutil.rmtree(self.cli_config.live_dir)
 
         storage.RenewableCert.new_lineage(
@@ -624,7 +624,7 @@ class RenewableCertTests(BaseRenewableCertTest):
         self.assertTrue(os.path.exists(os.path.join(
             self.cli_config.live_dir, "the-lineage.com", "privkey.pem")))
         self.assertTrue(os.path.exists(os.path.join(
-            self.cli_config.archive_dir, "the-lineage.com", "privkey1.pem")))
+            self.cli_config.default_archive_dir, "the-lineage.com", "privkey1.pem")))
 
     @mock.patch("certbot.storage.util.unique_lineage_name")
     def test_invalid_config_filename(self, mock_uln):
@@ -721,9 +721,10 @@ class RenewableCertTests(BaseRenewableCertTest):
         target = {}
         for x in ALL_FOUR:
             target[x] = "somewhere"
+        archive_dir = "the_archive"
         relevant_data = {"useful": "new_value"}
         from certbot import storage
-        storage.write_renewal_config(temp, temp2, target, relevant_data)
+        storage.write_renewal_config(temp, temp2, archive_dir, target, relevant_data)
         with open(temp2, "r") as f:
             content = f.read()
         # useful value was updated
@@ -735,6 +736,21 @@ class RenewableCertTests(BaseRenewableCertTest):
         # check version was stored
         self.assertTrue("version = {0}".format(certbot.__version__) in content)
 
+    def test_update_symlinks(self):
+        from certbot import storage
+        archive_dir_path = os.path.join(self.tempdir, "archive", "example.org")
+        for kind in ALL_FOUR:
+            live_path = self.config[kind]
+            basename = kind + "1.pem"
+            archive_path = os.path.join(archive_dir_path, basename)
+            open(archive_path, 'a').close()
+            os.symlink(os.path.join(self.tempdir, basename), live_path)
+        self.assertRaises(errors.CertStorageError,
+                          storage.RenewableCert, self.config.filename,
+                          self.cli_config)
+        storage.RenewableCert(self.config.filename, self.cli_config,
+            update_symlinks=True)
+
 
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover