From 6f9abde89418f851b01a68c2c8359ff9f256e68b Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Thu, 15 Dec 2016 16:32:31 -0800
Subject: [PATCH] Support intricate combinations of pre-hooks in different
 lineages

---
 certbot/hooks.py           | 34 ++++++++++++++++++++++------------
 certbot/tests/hook_test.py | 25 +++++++++++++++----------
 2 files changed, 37 insertions(+), 22 deletions(-)

diff --git a/certbot/hooks.py b/certbot/hooks.py
index 0eec93ab8..1783a37bd 100644
--- a/certbot/hooks.py
+++ b/certbot/hooks.py
@@ -37,12 +37,14 @@ def validate_hook(shell_cmd, hook_name):
 
 def pre_hook(config):
     "Run pre-hook if it's defined and hasn't been run."
-    if config.pre_hook and not pre_hook.already:
-        logger.info("Running pre-hook command: %s", config.pre_hook)
-        _run_hook(config.pre_hook)
-    pre_hook.already = True
+    cmd = config.pre_hook
+    if cmd and cmd not in pre_hook.already:
+        logger.info("Running pre-hook command: %s", cmd)
+        _run_hook(cmd)
+        pre_hook.already[cmd] = True
+
+pre_hook.already = {}
 
-pre_hook.already = False
 
 def post_hook(config, renew_final=False):
     """Run post hook if defined.
@@ -50,16 +52,24 @@ def post_hook(config, renew_final=False):
     If the verb is renew, we might have more certs to renew, so we wait until
     we're called with renew_final=True before actually doing anything.
     """
-    if config.post_hook:
-        if not pre_hook.already:
-            logger.info("No renewals attempted, so not running post-hook")
-            if config.verb != "renew":
-                logger.warning("Sanity failure in renewal hooks")
-            return
-        if renew_final or config.verb != "renew":
+
+    if config.verb == "renew":
+        if not renew_final:
+            if config.post_hook:
+                post_hook.eventually[config.post_hook] = True
+        else:
+            for cmd in post_hook.eventually:
+                logger.info("Running post-hook command: %s", cmd)
+                _run_hook(cmd)
+            if len(post_hook.eventually) == 0:
+                logger.info("No renewals attempted, so not running post-hook")
+    else: # certonly / run
+        if config.post_hook:
             logger.info("Running post-hook command: %s", config.post_hook)
             _run_hook(config.post_hook)
 
+post_hook.eventually = {}
+
 def renew_hook(config, domains, lineage_path):
     "Run post-renewal hook if defined."
     if config.renew_hook:
diff --git a/certbot/tests/hook_test.py b/certbot/tests/hook_test.py
index be7fb852d..3c55b675c 100644
--- a/certbot/tests/hook_test.py
+++ b/certbot/tests/hook_test.py
@@ -46,34 +46,39 @@ class HookTest(unittest.TestCase):
         mockwhich.return_value = None
         self.assertEqual(hooks._prog("funky"), None)
 
-    def _test_a_hook(self, config, hook_function, calls_expected):
+    def _test_a_hook(self, config, hook_function, calls_expected, **kwargs):
         with mock.patch('certbot.hooks.logger') as mock_logger:
             mock_logger.warning = mock.MagicMock()
             with mock.patch('certbot.hooks._run_hook') as mock_run_hook:
-                hook_function(config)
-                hook_function(config)
+                hook_function(config, **kwargs)
+                hook_function(config, **kwargs)
                 self.assertEqual(mock_run_hook.call_count, calls_expected)
             return mock_logger.warning
 
     def test_pre_hook(self):
-        hooks.pre_hook.already = False
+        hooks.pre_hook.already = {}
         config = mock.MagicMock(pre_hook="true")
         self._test_a_hook(config, hooks.pre_hook, 1)
+        self._test_a_hook(config, hooks.pre_hook, 0)
+        config = mock.MagicMock(pre_hook="more_true")
+        self._test_a_hook(config, hooks.pre_hook, 1)
+        self._test_a_hook(config, hooks.pre_hook, 0)
         config = mock.MagicMock(pre_hook="")
         self._test_a_hook(config, hooks.pre_hook, 0)
 
     def test_post_hook(self):
-        hooks.pre_hook.already = False
-        # if pre-hook isn't called, post-hook shouldn't be
         config = mock.MagicMock(post_hook="true", verb="splonk")
-        self._test_a_hook(config, hooks.post_hook, 0)
-
-        config = mock.MagicMock(post_hook="true", verb="splonk")
-        self._test_a_hook(config, hooks.pre_hook, 1)
         self._test_a_hook(config, hooks.post_hook, 2)
 
         config = mock.MagicMock(post_hook="true", verb="renew")
         self._test_a_hook(config, hooks.post_hook, 0)
+        self._test_a_hook(config, hooks.post_hook, 2, renew_final=True)
+        self._test_a_hook(config, hooks.post_hook, 0)
+        self._test_a_hook(config, hooks.post_hook, 2, renew_final=True)
+
+        config = mock.MagicMock(post_hook="more_true", verb="renew")
+        self._test_a_hook(config, hooks.post_hook, 0)
+        self._test_a_hook(config, hooks.post_hook, 4, renew_final=True)
 
     def test_renew_hook(self):
         with mock.patch.dict('os.environ', {}):