New interfaces for installers to run tasks on renew verb (#5879)

* ServerTLSUpdater and InstallerSpecificUpdater implementation

* Fixed tests and added disables for linter :/

* Added error logging for misconfigurationerror from plugin check

* Remove redundant parameter from interfaces

* Renaming the interfaces

* Finalize interface renaming and move tests to own file

* Refactored the runners

* Refactor the cli params

* Fix the interface args

* Fixed documentation

* Documentation and naming fixes

* Remove ServerTLSConfigurationUpdater

* Remove unnecessary linter disable

* Rename run_renewal_updaters to run_generic_updaters

* Do not raise exception, but make log message more informative and visible for the user

* Run renewal deployer before installer restart

certbot/cli.py

@@ -1192,6 +1192,14 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
         default=flag_default("directory_hooks"), dest="directory_hooks",
         help="Disable running executables found in Certbot's hook directories"
         " during renewal. (default: False)")
+    helpful.add(
+        "renew", "--disable-renew-updates", action="store_true",
+        default=flag_default("disable_renew_updates"), dest="disable_renew_updates",
+        help="Disable automatic updates to your server configuration that"
+        " would otherwise be done by the selected installer plugin, and triggered"
+        " when the user executes \"certbot renew\", regardless of if the certificate"
+        " is renewed. This setting does not apply to important TLS configuration"
+        " updates.")
 
     helpful.add_deprecated_argument("--agree-dev-preview", 0)
     helpful.add_deprecated_argument("--dialog", 0)

certbot/constants.py

@@ -64,6 +64,7 @@ CLI_DEFAULTS = dict(
     pref_challs=[],
     validate_hooks=True,
     directory_hooks=True,
+    disable_renew_updates=False,
 
     # Subparsers
     num=None,

certbot/interfaces.py

@@ -256,6 +256,10 @@ class IConfig(zope.interface.Interface):
         "user; only needed if your config is somewhere unsafe like /tmp/."
         "This is a boolean")
 
+    disable_renew_updates = zope.interface.Attribute(
+        "If updates provided by installer enhancements when Certbot is being run"
+        " with \"renew\" verb should be disabled.")
+
 class IInstaller(IPlugin):
     """Generic Certbot Installer Interface.
 
@@ -591,3 +595,74 @@ class IReporter(zope.interface.Interface):
 
     def print_messages(self):
         """Prints messages to the user and clears the message queue."""
+
+
+# Updater interfaces
+#
+# When "certbot renew" is run, Certbot will iterate over each lineage and check
+# if the selected installer for that lineage is a subclass of each updater
+# class. If it is and the update of that type is configured to be run for that
+# lineage, the relevant update function will be called for each domain in the
+# lineage. These functions are never called for other subcommands, so if an
+# installer wants to perform an update during the run or install subcommand, it
+# should do so when :func:`IInstaller.deploy_cert` is called.
+
+class GenericUpdater(object):
+    """Interface for update types not currently specified by Certbot.
+
+    This class allows plugins to perform types of updates that Certbot hasn't
+    defined (yet).
+
+    To make use of this interface, the installer should implement the interface
+    methods, and interfaces.GenericUpdater.register(InstallerClass) should
+    be called from the installer code.
+
+    """
+
+    __metaclass__ = abc.ABCMeta
+
+    @abc.abstractmethod
+    def generic_updates(self, domain, *args, **kwargs):
+        """Perform any update types defined by the installer.
+
+        If an installer is a subclass of the class containing this method, this
+        function will always be called when "certbot renew" is run. If the
+        update defined by the installer should be run conditionally, the
+        installer needs to handle checking the conditions itself.
+
+        This method is called once for each domain.
+
+        :param str domain: domain to handle the updates for
+
+        """
+
+
+class RenewDeployer(object):
+    """Interface for update types run when a lineage is renewed
+
+    This class allows plugins to perform types of updates that need to run at
+    lineage renewal that Certbot hasn't defined (yet).
+
+    To make use of this interface, the installer should implement the interface
+    methods, and interfaces.RenewDeployer.register(InstallerClass) should
+    be called from the installer code.
+    """
+
+    __metaclass__ = abc.ABCMeta
+
+    @abc.abstractmethod
+    def renew_deploy(self, lineage, *args, **kwargs):
+        """Perform updates defined by installer when a certificate has been renewed
+
+        If an installer is a subclass of the class containing this method, this
+        function will always be called when a certficate has been renewed by
+        running "certbot renew". For example if a plugin needs to copy a
+        certificate over, or change configuration based on the new certificate.
+
+        This method is called once for each lineage renewed
+
+        :param lineage: Certificate lineage object that is set if certificate
+            was renewed on this run.
+        :type lineage: storage.RenewableCert
+
+        """

certbot/main.py

@@ -29,6 +29,7 @@ from certbot import log
 from certbot import renewal
 from certbot import reporter
 from certbot import storage
+from certbot import updater
 from certbot import util
 
 from certbot.display import util as display_util, ops as display_ops
@@ -1145,10 +1146,9 @@ def renew_cert(config, plugins, lineage):
     except errors.PluginSelectionError as e:
         logger.info("Could not choose appropriate plugin: %s", e)
         raise
-
     le_client = _init_le_client(config, auth, installer)
 
-    _get_and_save_cert(le_client, config, lineage=lineage)
+    renewed_lineage = _get_and_save_cert(le_client, config, lineage=lineage)
 
     notify = zope.component.getUtility(interfaces.IDisplay).notification
     if installer is None:
@@ -1158,9 +1158,11 @@ def renew_cert(config, plugins, lineage):
         # In case of a renewal, reload server to pick up new certificate.
         # In principle we could have a configuration option to inhibit this
         # from happening.
+        updater.run_renewal_deployer(renewed_lineage, installer, config)
         installer.restart()
         notify("new certificate deployed with reload of {0} server; fullchain is {1}".format(
                config.installer, lineage.fullchain), pause=False)
+        # Run deployer
 
 def certonly(config, plugins):
     """Authenticate & obtain cert, but do not install it.

certbot/renewal.py

@@ -12,13 +12,14 @@ import zope.component
 import OpenSSL
 
 from certbot import cli
-
 from certbot import crypto_util
 from certbot import errors
 from certbot import interfaces
 from certbot import util
 from certbot import hooks
 from certbot import storage
+from certbot import updater
+
 from certbot.plugins import disco as plugins_disco
 
 logger = logging.getLogger(__name__)
@@ -411,9 +412,9 @@ def handle_renewal_request(config):
                 # XXX: ensure that each call here replaces the previous one
                 zope.component.provideUtility(lineage_config)
                 renewal_candidate.ensure_deployed()
+                from certbot import main
+                plugins = plugins_disco.PluginsRegistry.find_all()
                 if should_renew(lineage_config, renewal_candidate):
-                    plugins = plugins_disco.PluginsRegistry.find_all()
-                    from certbot import main
                     # domains have been restored into lineage_config by reconstitute
                     # but they're unnecessary anyway because renew_cert here
                     # will just grab them from the certificate
@@ -426,6 +427,10 @@ def handle_renewal_request(config):
                         "cert", renewal_candidate.latest_common_version()))
                     renew_skipped.append("%s expires on %s" % (renewal_candidate.fullchain,
                                          expiry.strftime("%Y-%m-%d")))
+                # Run updater interface methods
+                updater.run_generic_updaters(lineage_config, plugins,
+                                             renewal_candidate)
+
         except Exception as e:  # pylint: disable=broad-except
             # obtain_cert (presumably) encountered an unanticipated problem.
             logger.warning("Attempting to renew cert (%s) from %s produced an "

certbot/tests/main_test.py

@@ -23,6 +23,7 @@ from certbot import configuration
 from certbot import crypto_util
 from certbot import errors
 from certbot import main
+from certbot import updater
 from certbot import util
 
 from certbot.plugins import disco
@@ -1232,7 +1233,9 @@ class MainTest(test_util.ConfigTestCase):  # pylint: disable=too-many-public-met
         self._test_renew_common(renewalparams=renewalparams, error_expected=True,
                                 names=names, assert_oc_called=False)
 
-    def test_renew_with_configurator(self):
+    @mock.patch('certbot.plugins.selection.choose_configurator_plugins')
+    def test_renew_with_configurator(self, mock_sel):
+        mock_sel.return_value = (mock.MagicMock(), mock.MagicMock())
         renewalparams = {'authenticator': 'webroot'}
         self._test_renew_common(
             renewalparams=renewalparams, assert_oc_called=True,
@@ -1448,6 +1451,18 @@ class MainTest(test_util.ConfigTestCase):  # pylint: disable=too-many-public-met
                             email in mock_utility().add_message.call_args[0][0])
                         self.assertTrue(mock_handle.called)
 
+    @mock.patch('certbot.plugins.selection.choose_configurator_plugins')
+    def test_plugin_selection_error(self, mock_choose):
+        mock_choose.side_effect = errors.PluginSelectionError
+        self.assertRaises(errors.PluginSelectionError, main.renew_cert,
+                          None, None, None)
+
+        with mock.patch('certbot.updater.logger.warning') as mock_log:
+            updater.run_generic_updaters(None, None, None)
+            self.assertTrue(mock_log.called)
+            self.assertTrue("Could not choose appropriate plugin for updaters"
+                            in mock_log.call_args[0][0])
+
 
 class UnregisterTest(unittest.TestCase):
     def setUp(self):

certbot/tests/renewupdater_test.py

@@ -0,0 +1,76 @@
+"""Tests for renewal updater interfaces"""
+import unittest
+import mock
+
+from certbot import interfaces
+from certbot import main
+from certbot import updater
+
+import certbot.tests.util as test_util
+
+
+class RenewUpdaterTest(unittest.TestCase):
+    """Tests for interfaces.RenewDeployer and interfaces.GenericUpdater"""
+
+    def setUp(self):
+        class MockInstallerGenericUpdater(interfaces.GenericUpdater):
+            """Mock class that implements GenericUpdater"""
+            def __init__(self, *args, **kwargs):
+                # pylint: disable=unused-argument
+                self.restart = mock.MagicMock()
+                self.callcounter = mock.MagicMock()
+            def generic_updates(self, domain, *args, **kwargs):
+                self.callcounter(*args, **kwargs)
+
+        class MockInstallerRenewDeployer(interfaces.RenewDeployer):
+            """Mock class that implements RenewDeployer"""
+            def __init__(self, *args, **kwargs):
+                # pylint: disable=unused-argument
+                self.callcounter = mock.MagicMock()
+            def renew_deploy(self, lineage, *args, **kwargs):
+                self.callcounter(*args, **kwargs)
+
+        self.generic_updater = MockInstallerGenericUpdater()
+        self.renew_deployer = MockInstallerRenewDeployer()
+
+    def get_config(self, args):
+        """Get mock config from dict of parameters"""
+        config = mock.MagicMock()
+        for key in args.keys():
+            config.__dict__[key] = args[key]
+        return config
+
+    @mock.patch('certbot.main._get_and_save_cert')
+    @mock.patch('certbot.plugins.selection.choose_configurator_plugins')
+    @test_util.patch_get_utility()
+    def test_server_updates(self, _, mock_select, mock_getsave):
+        config = self.get_config({"disable_renew_updates": False})
+
+        lineage = mock.MagicMock()
+        lineage.names.return_value = ['firstdomain', 'seconddomain']
+        mock_getsave.return_value = lineage
+        mock_generic_updater = self.generic_updater
+
+        # Generic Updater
+        mock_select.return_value = (mock_generic_updater, None)
+        with mock.patch('certbot.main._init_le_client'):
+            main.renew_cert(config, None, mock.MagicMock())
+        self.assertTrue(mock_generic_updater.restart.called)
+
+        mock_generic_updater.restart.reset_mock()
+        mock_generic_updater.callcounter.reset_mock()
+        updater.run_generic_updaters(config, None, lineage)
+        self.assertEqual(mock_generic_updater.callcounter.call_count, 2)
+        self.assertFalse(mock_generic_updater.restart.called)
+
+    def test_renew_deployer(self):
+        config = self.get_config({"disable_renew_updates": False})
+        lineage = mock.MagicMock()
+        lineage.names.return_value = ['firstdomain', 'seconddomain']
+        mock_deployer = self.renew_deployer
+        updater.run_renewal_deployer(lineage, mock_deployer, config)
+        self.assertTrue(mock_deployer.callcounter.called_with(lineage))
+
+
+if __name__ == '__main__':
+    unittest.main()  # pragma: no cover

certbot/updater.py

@@ -0,0 +1,67 @@
+"""Updaters run at renewal"""
+import logging
+
+from certbot import errors
+from certbot import interfaces
+
+from certbot.plugins import selection as plug_sel
+
+logger = logging.getLogger(__name__)
+
+def run_generic_updaters(config, plugins, lineage):
+    """Run updaters that the plugin supports
+
+    :param config: Configuration object
+    :type config: interfaces.IConfig
+
+    :param plugins: List of plugins
+    :type plugins: `list` of `str`
+
+    :param lineage: Certificate lineage object
+    :type lineage: storage.RenewableCert
+
+    :returns: `None`
+    :rtype: None
+    """
+    try:
+        # installers are used in auth mode to determine domain names
+        installer, _ = plug_sel.choose_configurator_plugins(config, plugins, "certonly")
+    except errors.PluginSelectionError as e:
+        logger.warning("Could not choose appropriate plugin for updaters: %s", e)
+        return
+    _run_updaters(lineage, installer, config)
+
+def run_renewal_deployer(lineage, installer, config):
+    """Helper function to run deployer interface method if supported by the used
+    installer plugin.
+
+    :param lineage: Certificate lineage object
+    :type lineage: storage.RenewableCert
+
+    :param installer: Installer object
+    :type installer: interfaces.IInstaller
+
+    :returns: `None`
+    :rtype: None
+    """
+    if not config.disable_renew_updates and isinstance(installer,
+                                                       interfaces.RenewDeployer):
+        installer.renew_deploy(lineage)
+
+def _run_updaters(lineage, installer, config):
+    """Helper function to run the updater interface methods if supported by the
+    used installer plugin.
+
+    :param lineage: Certificate lineage object
+    :type lineage: storage.RenewableCert
+
+    :param installer: Installer object
+    :type installer: interfaces.IInstaller
+
+    :returns: `None`
+    :rtype: None
+    """
+    for domain in lineage.names():
+        if not config.disable_renew_updates:
+            if isinstance(installer, interfaces.GenericUpdater):
+                installer.generic_updates(domain)