Change GenericUpdater parameter to lineage (#6030)

In order to give more flexibility for plugins using interfaces.GenericUpdater interface, lineage needs to be passed to the updater method instead of individual domains. All of the (present and potential) installers do not work on per domain basis, while the lineage does contain a list of them for installers which do.

This also means that we don't unnecessarily run the updater method multiple times, potentially invoking expensive tooling up to $max_san_amount times.

* Make GenericUpdater use lineage as parameter and get invoked only once per lineage

certbot/interfaces.py

@@ -604,10 +604,10 @@ class IReporter(zope.interface.Interface):
 # 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.
+# lineage, the relevant update function will be called for it. 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.
 
 @six.add_metaclass(abc.ABCMeta)
 class GenericUpdater(object):
@@ -623,7 +623,7 @@ class GenericUpdater(object):
     """
 
     @abc.abstractmethod
-    def generic_updates(self, domain, *args, **kwargs):
+    def generic_updates(self, lineage, *args, **kwargs):
         """Perform any update types defined by the installer.
 
         If an installer is a subclass of the class containing this method, this
@@ -631,9 +631,10 @@ class GenericUpdater(object):
         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.
+        This method is called once for each lineage.
 
-        :param str domain: domain to handle the updates for
+        :param lineage: Certificate lineage object
+        :type lineage: storage.RenewableCert
 
         """
 
@@ -661,8 +662,7 @@ class RenewDeployer(object):
 
         This method is called once for each lineage renewed
 
-        :param lineage: Certificate lineage object that is set if certificate
-            was renewed on this run.
+        :param lineage: Certificate lineage object
         :type lineage: storage.RenewableCert
 
         """

certbot/tests/renewupdater_test.py

@@ -19,7 +19,7 @@ class RenewUpdaterTest(unittest.TestCase):
                 # pylint: disable=unused-argument
                 self.restart = mock.MagicMock()
                 self.callcounter = mock.MagicMock()
-            def generic_updates(self, domain, *args, **kwargs):
+            def generic_updates(self, lineage, *args, **kwargs):
                 self.callcounter(*args, **kwargs)
 
         class MockInstallerRenewDeployer(interfaces.RenewDeployer):
@@ -46,9 +46,7 @@ class RenewUpdaterTest(unittest.TestCase):
     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_getsave.return_value = mock.MagicMock()
         mock_generic_updater = self.generic_updater
 
         # Generic Updater
@@ -59,14 +57,13 @@ class RenewUpdaterTest(unittest.TestCase):
 
         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)
+        updater.run_generic_updaters(config, None, mock.MagicMock())
+        self.assertEqual(mock_generic_updater.callcounter.call_count, 1)
         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))

certbot/updater.py

@@ -61,7 +61,6 @@ def _run_updaters(lineage, installer, config):
     :returns: `None`
     :rtype: None
     """
-    for domain in lineage.names():
-        if not config.disable_renew_updates:
-            if isinstance(installer, interfaces.GenericUpdater):
-                installer.generic_updates(domain)
+    if not config.disable_renew_updates:
+        if isinstance(installer, interfaces.GenericUpdater):
+            installer.generic_updates(lineage)