Implement deploy hook for ocsp prefetch functionality

certbot-apache/certbot_apache/_internal/prefetch_ocsp.py

@@ -367,6 +367,24 @@ class OCSPPrefetchMixin(object):
                    "enhancement: %s\nOCSP prefetch was not enabled.")
             raise errors.PluginError(msg % str(err))
 
+    def deploy_ocsp_prefetch(self, lineage):
+        """When certificate gets renewed, ensure that we're able to serve an appropriate OCSP
+        staple after the restart that replaces the certificate."""
+
+        self._ocsp_prefetch_fetch_state()
+        if not self._ocsp_prefetch:
+            # No OCSP prefetching enabled for any certificate
+            return
+
+        if lineage.cert_path in self._ocsp_prefetch:
+            pf = self._ocsp_prefetch[lineage.cert_path]
+            try:
+                self._ocsp_try_refresh(lineage.cert_path)
+            except OCSPCertificateError:
+                # This error was logged and handled already down the stack. Return to avoid save.
+                return
+            self._ocsp_prefetch_save(lineage.cert_path, pf["chain_path"])
+
     def update_ocsp_prefetch(self, _unused_lineage):
         """Checks all certificates that are managed by OCSP prefetch, and
         refreshes OCSP responses for them if required."""
@@ -381,20 +399,33 @@ class OCSPPrefetchMixin(object):
             pf = self._ocsp_prefetch[cert_path]
             if self._ocsp_refresh_needed(pf["lastupdate"]):
                 try:
-                    self._ocsp_refresh(cert_path, pf["chain_path"])
-                except OCSPCertificateError as err:
-                    self._ocsp_prefetch_remove(cert_path)
-                    msg = ("Error when trying to prefetch OCSP staple: {} " +
-                           "OCSP prefetch functionality removed for the certificate").format(err)
-                    logger.warning(msg)
+                    self._ocsp_try_refresh(cert_path)
+                except OCSPCertificateError:
+                    # We want to skip saving in this case, as we just removed the
+                    # certificate from prefetch pool.
                     continue
-                except errors.PluginError as err:
-                    msg = "Encountered a issue when trying to renew OCSP staple: {}".format(err)
-                    logger.warning(msg)
-                # Save the status to pluginstorage. Do this even if errored out to prevent
-                # spurious errors when handling multiple renewals.
                 self._ocsp_prefetch_save(cert_path, pf["chain_path"])
 
+    def _ocsp_try_refresh(self, cert_path):
+        """Attempt to refresh OCSP staple for a certificate.
+
+        :param str cert_path: Path to certificate
+        """
+
+        pf = self._ocsp_prefetch[cert_path]
+
+        try:
+            self._ocsp_refresh(cert_path, pf["chain_path"])
+        except OCSPCertificateError as err:
+            self._ocsp_prefetch_remove(cert_path)
+            msg = ("Error when trying to prefetch OCSP staple: {} " +
+                    "OCSP prefetch functionality removed for the certificate").format(err)
+            logger.warning(msg)
+            raise
+        except errors.PluginError as err:
+            msg = "Encountered a issue when trying to renew OCSP staple: {}".format(err)
+            logger.warning(msg)
+
     def restart(self):
         """Reloads the Apache server. When restarting, Apache deletes
         the DBM cache file used to store OCSP staples. In this override

certbot-apache/tests/ocsp_prefetch_test.py

@@ -19,7 +19,7 @@ from certbot import errors
 from certbot.compat import os
 import util
 
-from certbot_apache._internal.prefetch_ocsp import DBMHandler
+from certbot_apache._internal.prefetch_ocsp import DBMHandler, OCSPCertificateError
 
 
 class MockDBM(object):
@@ -186,6 +186,35 @@ class OCSPPrefetchTest(util.ApacheTest):
                               self.lineage,
                               ["domainnotfound"])
 
+    def test_deploy_ocsp_prefetch_noop(self):
+        dumb_lineage = mock.MagicMock(cert_path="not_found")
+        refresh = "certbot_apache._internal.override_debian.DebianConfigurator._ocsp_try_refresh"
+
+        with mock.patch(refresh) as mock_refresh:
+            self.config.deploy_ocsp_prefetch(dumb_lineage)
+            self.assertFalse(mock_refresh.called)
+
+    @mock.patch("certbot_apache._internal.override_debian.DebianConfigurator._ocsp_try_refresh")
+    def test_deploy_ocsp_prefetch(self, mock_refresh):
+        self.config._ocsp_prefetch_save("artificial_path", "irrelevant")
+        mock_lineage = mock.MagicMock(cert_path="artificial_path")
+        s_path = "certbot_apache._internal.override_debian.DebianConfigurator._ocsp_prefetch_save"
+        with mock.patch(s_path) as mock_save:
+            self.config.deploy_ocsp_prefetch(mock_lineage)
+            self.assertTrue(mock_refresh.called)
+            self.assertTrue(mock_save.called)
+
+    @mock.patch("certbot_apache._internal.override_debian.DebianConfigurator._ocsp_try_refresh")
+    def test_deploy_ocsp_prefetch_error(self, mock_refresh):
+        self.config._ocsp_prefetch_save("artificial_path", "irrelevant")
+        mock_lineage = mock.MagicMock(cert_path="artificial_path")
+        mock_refresh.side_effect = OCSPCertificateError("Error")
+        s_path = "certbot_apache._internal.override_debian.DebianConfigurator._ocsp_prefetch_save"
+        with mock.patch(s_path) as mock_save:
+            self.config.deploy_ocsp_prefetch(mock_lineage)
+            self.assertTrue(mock_refresh.called)
+            self.assertFalse(mock_save.called)
+
     @mock.patch("certbot_apache._internal.constants.OCSP_INTERNAL_TTL", 0)
     def test_ocsp_prefetch_refresh(self):
         def ocsp_req_mock(_cert, _chain, workfile):

certbot/certbot/plugins/enhancements.py

@@ -234,7 +234,7 @@ _INDEX = [
         "cli_action": "store_true",
         "class": OCSPPrefetchEnhancement,
         "updater_function": "update_ocsp_prefetch",
-        "deployer_function": None,
+        "deployer_function": "deploy_ocsp_prefetch",
         "enable_function": "enable_ocsp_prefetch"
     }
 ]  # type: List[Dict[str, Any]]