Completely aligns deploy and renew hook behaviors, fully fixing #9978 (#10534)

https://github.com/certbot/certbot/pull/10146 was supposed to do this,
but because of multiple code paths, it did not. This PR simplifies the
code by creating a single code path.

In particular:

- `hooks.renew_hook()` is removed. There are now only calls to
`hooks.deploy_hook()`, which is called during certonly, run, and renew,
and runs both cli and directory hooks.
- `cli_config.renew_hook` is removed. Both `--renew-hook` (hidden option
kept for backwards compatibility purposes and `--deploy-hook` now set
`cli_config.deploy_hook`, which is used internally. When either or both
flags are used multiple times, the last value is kept, which is the
argparse default.
- references to running a "renew hook" internally are changed to "deploy
hook"
- To maintain downgrade compatibility, `deploy_hook` is written out to
renewal config files as `renew_hook`. This is achieved by translating to
and from `renew_hook` in `storage.py` and changing
`renewal.STR_CONFIG_ITEMS` to contain `deploy_hook`.

This results in the following behavior changes:
- Directory hooks are now run when getting a new cert using certonly/run
- If someone set a renew hook on the cli using `--renew-hook`, it would
previously not be run when getting a new (non-renewed) cert, but now
will be. But this option is hidden and should no longer be used anyway.
- When using `certbot reconfigure`, if someone sets `--renew-hook`
certbot will now also ask if someone would like to do a test run of the
new hook, whereas before it would only do so for `--deploy-hook`.

---------

Co-authored-by: Brad Warren <bmw@users.noreply.github.com>

certbot-ci/src/certbot_integration_tests/certbot_tests/assertions.py

@@ -78,9 +78,9 @@ def assert_saved_lineage_option(config_dir: str, lineage: str,
         assert f"{option} = {value if value else ''}" in file_h.read()
 
 
-def assert_saved_renew_hook(config_dir: str, lineage: str) -> None:
+def assert_saved_deploy_hook(config_dir: str, lineage: str) -> None:
     """
-    Assert that the renew hook configuration of a lineage has been saved.
+    Assert that the deploy hook configuration of a lineage has been saved.
     :param str config_dir: location of the certbot configuration
     :param str lineage: lineage domain name
     """

certbot-ci/src/certbot_integration_tests/certbot_tests/test_main.py

@@ -28,7 +28,7 @@ from certbot_integration_tests.certbot_tests.assertions import assert_equals_wor
 from certbot_integration_tests.certbot_tests.assertions import assert_hook_execution
 from certbot_integration_tests.certbot_tests.assertions import assert_rsa_key
 from certbot_integration_tests.certbot_tests.assertions import assert_saved_lineage_option
-from certbot_integration_tests.certbot_tests.assertions import assert_saved_renew_hook
+from certbot_integration_tests.certbot_tests.assertions import assert_saved_deploy_hook
 from certbot_integration_tests.certbot_tests.assertions import assert_world_no_permissions
 from certbot_integration_tests.certbot_tests.assertions import assert_world_read_permissions
 from certbot_integration_tests.certbot_tests.assertions import EVERYBODY_SID
@@ -108,7 +108,7 @@ def test_http_01(context: IntegrationTestsContext) -> None:
     ])
 
     assert_hook_execution(context.hook_probe, 'deploy')
-    assert_saved_renew_hook(context.config_dir, certname)
+    assert_saved_deploy_hook(context.config_dir, certname)
     assert_saved_lineage_option(context.config_dir, certname, 'key_type', 'ecdsa')
 
 
@@ -178,12 +178,11 @@ def test_manual_http_auth(context: IntegrationTestsContext) -> None:
             '--manual-cleanup-hook', scripts[1],
             '--pre-hook', misc.echo('wtf_pre', context.hook_probe),
             '--post-hook', misc.echo('wtf_post', context.hook_probe),
-            '--renew-hook', misc.echo('renew', context.hook_probe),
+            '--deploy-hook', misc.echo('deploy', context.hook_probe),
         ])
 
-    with pytest.raises(AssertionError):
-        assert_hook_execution(context.hook_probe, 'renew')
-    assert_saved_renew_hook(context.config_dir, certname)
+    assert_hook_execution(context.hook_probe, 'deploy')
+    assert_saved_deploy_hook(context.config_dir, certname)
 
 
 def test_manual_dns_auth(context: IntegrationTestsContext) -> None:
@@ -196,12 +195,11 @@ def test_manual_dns_auth(context: IntegrationTestsContext) -> None:
         '--manual-cleanup-hook', context.manual_dns_cleanup_hook,
         '--pre-hook', misc.echo('wtf_pre', context.hook_probe),
         '--post-hook', misc.echo('wtf_post', context.hook_probe),
-        '--renew-hook', misc.echo('renew', context.hook_probe),
+        '--deploy-hook', misc.echo('deploy', context.hook_probe),
     ])
 
-    with pytest.raises(AssertionError):
-        assert_hook_execution(context.hook_probe, 'renew')
-    assert_saved_renew_hook(context.config_dir, certname)
+    assert_hook_execution(context.hook_probe, 'deploy')
+    assert_saved_deploy_hook(context.config_dir, certname)
 
     context.certbot(['renew', '--cert-name', certname, '--authenticator', 'manual'])
 

certbot/src/certbot/_internal/cli/__init__.py

@@ -22,10 +22,8 @@ from certbot._internal.cli.cli_constants import HELP_AND_VERSION_USAGE
 from certbot._internal.cli.cli_constants import SHORT_USAGE
 from certbot._internal.cli.cli_constants import VAR_MODIFIERS
 from certbot._internal.cli.cli_constants import ZERO_ARG_ACTIONS
-from certbot._internal.cli.cli_utils import _DeployHookAction
 from certbot._internal.cli.cli_utils import _EncodeReasonAction
 from certbot._internal.cli.cli_utils import _PrefChallAction
-from certbot._internal.cli.cli_utils import _RenewHookAction
 from certbot._internal.cli.cli_utils import _user_agent_comment_type
 from certbot._internal.cli.cli_utils import add_domains
 from certbot._internal.cli.cli_utils import CaseInsensitiveList
@@ -431,13 +429,13 @@ def prepare_and_parse_args(plugins: plugins_disco.PluginsRegistry, args: list[st
         " multiple renewed certificates have identical post-hooks, only"
         " one will be run.")
     helpful.add(["renew", "reconfigure"], "--renew-hook",
-                action=_RenewHookAction, help=argparse.SUPPRESS)
+                dest="deploy_hook", help=argparse.SUPPRESS)
     helpful.add(
         "renew", "--no-random-sleep-on-renew", action="store_false",
         default=flag_default("random_sleep_on_renew"), dest="random_sleep_on_renew",
         help=argparse.SUPPRESS)
     helpful.add(
-        ["certonly", "renew", "reconfigure", "run"], "--deploy-hook", action=_DeployHookAction,
+        ["certonly", "renew", "reconfigure", "run"], "--deploy-hook",
         help='Command to be run in a shell once for each successfully'
         ' issued certificate, including on subsequent renewals.'
         ' Unless --disable-hook-validation is used, the command’s first word'

certbot/src/certbot/_internal/cli/cli_constants.py

@@ -80,7 +80,6 @@ ZERO_ARG_ACTIONS = {"store_const", "store_true",
 # This dictionary is used recursively, so if A modifies B and B modifies C,
 # it is determined that C was modified by the user if A was modified.
 VAR_MODIFIERS = {"account": {"server",},
-                 "renew_hook": {"deploy_hook",},
                  "server": {"dry_run", "staging",},
                  "webroot_map": {"webroot_path",}}
 

certbot/src/certbot/_internal/cli/cli_utils.py

@@ -221,32 +221,6 @@ class _PrefChallAction(argparse.Action):
         namespace.pref_challs.extend(challs)
 
 
-class _DeployHookAction(argparse.Action):
-    """Action class for parsing deploy hooks."""
-
-    def __call__(self, parser: argparse.ArgumentParser, namespace: argparse.Namespace,
-                 values: Union[str, Sequence[Any], None],
-                 option_string: Optional[str] = None) -> None:
-        renew_hook_set = namespace.deploy_hook != namespace.renew_hook
-        if renew_hook_set and namespace.renew_hook != values:
-            raise argparse.ArgumentError(
-                self, "conflicts with --renew-hook value")
-        namespace.deploy_hook = namespace.renew_hook = values
-
-
-class _RenewHookAction(argparse.Action):
-    """Action class for parsing renew hooks."""
-
-    def __call__(self, parser: argparse.ArgumentParser, namespace: argparse.Namespace,
-                 values: Union[str, Sequence[Any], None],
-                 option_string: Optional[str] = None) -> None:
-        deploy_hook_set = namespace.deploy_hook is not None
-        if deploy_hook_set and namespace.deploy_hook != values:
-            raise argparse.ArgumentError(
-                self, "conflicts with --deploy-hook value")
-        namespace.renew_hook = values
-
-
 def nonnegative_int(value: str) -> int:
     """Converts value to an int and checks that it is not negative.
 

certbot/src/certbot/_internal/client.py

@@ -121,7 +121,7 @@ def ua_flags(config: configuration.NamespaceConfig) -> str:
         flags.append("asn")
     if config.noninteractive_mode:
         flags.append("n")
-    hook_names = ("pre", "post", "renew", "manual_auth", "manual_cleanup")
+    hook_names = ("pre", "post", "deploy", "manual_auth", "manual_cleanup")
     hooks = [getattr(config, h + "_hook") for h in hook_names]
     if any(hooks):
         flags.append("hook")

certbot/src/certbot/_internal/constants.py

@@ -195,16 +195,18 @@ RENEWAL_CONFIGS_DIR = "renewal"
 to `certbot.configuration.NamespaceConfig.config_dir`."""
 
 RENEWAL_HOOKS_DIR = "renewal-hooks"
-"""Basename of directory containing hooks to run with the renew command."""
+"""Basename of directory containing hooks to run when getting or renewing certificates."""
 
 RENEWAL_PRE_HOOKS_DIR = "pre"
-"""Basename of directory containing pre-hooks to run with the renew command."""
+"""Basename of directory containing pre-hooks to run
+before attempting to get or renew certs"""
 
 RENEWAL_DEPLOY_HOOKS_DIR = "deploy"
-"""Basename of directory containing deploy-hooks to run with the renew command."""
+"""Basename of directory containing deploy-hooks to run
+upon successfully getting or renewing certs."""
 
 RENEWAL_POST_HOOKS_DIR = "post"
-"""Basename of directory containing post-hooks to run with the renew command."""
+"""Basename of directory containing post-hooks to run after attempting to get or renew certs."""
 
 FORCE_INTERACTIVE_FLAG = "--force-interactive"
 """Flag to disable TTY checking in certbot.display.util."""

certbot/src/certbot/_internal/hooks.py

@@ -21,7 +21,6 @@ def validate_hooks(config: configuration.NamespaceConfig) -> None:
     validate_hook(config.pre_hook, "pre")
     validate_hook(config.post_hook, "post")
     validate_hook(config.deploy_hook, "deploy")
-    validate_hook(config.renew_hook, "renew")
 
 
 def _prog(shell_cmd: str) -> Optional[str]:
@@ -192,26 +191,11 @@ def run_saved_post_hooks(renewed_sans: list[san.SAN], failed_sans: list[san.SAN]
 
 def deploy_hook(config: configuration.NamespaceConfig, sans: list[san.SAN],
                 lineage_path: str) -> None:
-    """Run post-issuance hook if defined.
-
-    :param configuration.NamespaceConfig config: Certbot settings
-    :param sans: domains and/or IP addresses in the obtained certificate
-    :type sans: `list` of `str`
-    :param str lineage_path: live directory path for the new cert
-
-    """
-    if config.deploy_hook:
-        _run_deploy_hook(config.deploy_hook, sans,
-                         lineage_path, config.dry_run, config.run_deploy_hooks)
-
-
-def renew_hook(config: configuration.NamespaceConfig, sans: list[san.SAN],
-               lineage_path: str) -> None:
-    """Run post-renewal hooks.
+    """Run post-renewal/post-issuance hooks.
 
     This function runs any hooks found in
-    config.renewal_deploy_hooks_dir followed by any renew-hook in the
-    config. If the renew-hook in the config is a path to a script in
+    config.renewal_deploy_hooks_dir followed by any deploy-hook in the
+    config. If the deploy-hook in the config is a path to a script in
     config.renewal_deploy_hooks_dir, it is not run twice.
 
     If Certbot is doing a dry run, no hooks are run and messages are
@@ -224,9 +208,9 @@ def renew_hook(config: configuration.NamespaceConfig, sans: list[san.SAN],
 
     """
     executed_hooks = set()
-    all_hooks: list[str] = (list_hooks(config.renewal_deploy_hooks_dir)if config.directory_hooks
+    all_hooks: list[str] = (list_hooks(config.renewal_deploy_hooks_dir) if config.directory_hooks
         else [])
-    all_hooks += [config.renew_hook] if config.renew_hook else []
+    all_hooks += [config.deploy_hook] if config.deploy_hook else []
     for hook in all_hooks:
         if hook in executed_hooks:
             logger.info("Skipping deploy-hook '%s' as it was already run.", hook)

certbot/src/certbot/_internal/renewal.py

@@ -48,7 +48,7 @@ ARI_RETRY_AFTER_CONFIG_ITEM = "ari_retry_after"
 # the renewal configuration process loses this information.
 STR_CONFIG_ITEMS = ["config_dir", "logs_dir", "work_dir", "user_agent",
                     "server", "account", "authenticator", "installer",
-                    "renew_hook", "pre_hook", "post_hook", "http01_address",
+                    "deploy_hook", "pre_hook", "post_hook", "http01_address",
                     "preferred_chain", "key_type", "elliptic_curve",
                     "preferred_profile", "required_profile"]
 INT_CONFIG_ITEMS = ["rsa_key_size", "http01_port"]
@@ -572,7 +572,7 @@ def renew_cert(config: configuration.NamespaceConfig, sans: Optional[list[san.SA
         lineage.update_all_links_to(lineage.latest_common_version())
         lineage.truncate()
 
-    hooks.renew_hook(config, sans, lineage.live_dir)
+    hooks.deploy_hook(config, sans, lineage.live_dir)
 
 
 def report(msgs: Iterable[str], category: str) -> str:
@@ -606,7 +606,7 @@ def _renew_describe_results(config: configuration.NamespaceConfig, renew_success
     if not renew_successes and not renew_failures:
         notify(f"No {renewal_noun}s were attempted.")
         if (config.pre_hook is not None or
-                config.renew_hook is not None or config.post_hook is not None):
+                config.deploy_hook is not None or config.post_hook is not None):
             notify("No hooks were run.")
     elif renew_successes and not renew_failures:
         notify(f"Congratulations, all {renewal_noun}s succeeded: ")

certbot/src/certbot/_internal/storage.py

@@ -212,6 +212,16 @@ def make_renewal_configobj(archive_dir: str, target: Mapping[str, str],
         config[kind] = target[kind]
     config['renewalparams'] = {}
     config['renewalparams'].update(relevant_values(cli_config))
+
+    # MAGIC CODE ALERT
+    # In keeping with the code in RenewableCert.__init__, we use deploy_hook internally,
+    # but write out the value as renew_hook to allow downgrade compatibility.
+    # So, if there's a deploy_hook (the internal name), change it to renew_hook (the renewal
+    # config file name).
+    if "deploy_hook" in config["renewalparams"]:
+        config["renewalparams"]["renew_hook"] = config["renewalparams"]["deploy_hook"]
+        del config["renewalparams"]["deploy_hook"]
+
     return config
 
 
@@ -494,6 +504,17 @@ class RenewableCert(interfaces.RenewableCert):
         self.fullchain = self.configuration["fullchain"]
         self.live_dir = os.path.dirname(self.cert)
 
+        # MAGIC CODE ALERT
+        # We changed the name of the internal property from deploy hook to renew hook
+        # There are already configs out there with renew_hook saved, so we're going to keep saving
+        # it out as renew_hook to allow downgrade compatibility. Load it in as deploy
+        # hook. Then, renewal.py's STR_CONFIG_ITEMS will check against the new internal name.
+        if "renewalparams" in self.configuration:
+            if "renew_hook" in self.configuration["renewalparams"]:
+                self.configuration["renewalparams"]["deploy_hook"] = \
+                    self.configuration["renewalparams"]["renew_hook"]
+                del self.configuration["renewalparams"]["renew_hook"]
+
         self._fix_symlinks()
         self._check_symlinks()
 

certbot/src/certbot/_internal/tests/cli_test.py

@@ -400,7 +400,7 @@ class ParseTest(unittest.TestCase):
         args = 'renew --deploy-hook foo'.split()
         plugins = disco.PluginsRegistry.find_all()
         config = cli.prepare_and_parse_args(plugins, args)
-        assert config.set_by_user('renew_hook')
+        assert config.set_by_user('deploy_hook')
 
     @mock.patch('certbot._internal.plugins.webroot._validate_webroot')
     def test_user_set_webroot_map(self, mock_validate_webroot):
@@ -425,9 +425,16 @@ class ParseTest(unittest.TestCase):
             self.parse("-n --force-interactive".split())
 
     def test_deploy_hook_conflict(self):
-        with mock.patch("certbot._internal.cli.sys.stderr"):
-            with pytest.raises(SystemExit):
-                self.parse("--renew-hook foo --deploy-hook bar".split())
+        namespace = self.parse(["--renew-hook", "foo",
+                                "--deploy-hook", "bar",
+                                "--disable-hook-validation"])
+        assert_set_by_user_with_value(namespace, 'deploy_hook', "bar")
+
+    def test_renew_hook_conflict(self):
+        namespace = self.parse(["--deploy-hook", "foo",
+                                "--renew-hook", "bar",
+                                "--disable-hook-validation"])
+        assert_set_by_user_with_value(namespace, 'deploy_hook', "bar")
 
     def test_deploy_hook_matches_renew_hook(self):
         value = "foo"
@@ -435,19 +442,12 @@ class ParseTest(unittest.TestCase):
                                 "--deploy-hook", value,
                                 "--disable-hook-validation"])
         assert_set_by_user_with_value(namespace, 'deploy_hook', value)
-        assert_set_by_user_with_value(namespace, 'renew_hook', value)
 
-    def test_deploy_hook_sets_renew_hook(self):
+    def test_renew_hook_sets_deploy_hook(self):
         value = "foo"
         namespace = self.parse(
-            ["--deploy-hook", value, "--disable-hook-validation"])
+            ["--renew-hook", value, "--disable-hook-validation"])
         assert_set_by_user_with_value(namespace, 'deploy_hook', value)
-        assert_set_by_user_with_value(namespace, 'renew_hook', value)
-
-    def test_renew_hook_conflict(self):
-        with mock.patch("certbot._internal.cli.sys.stderr"):
-            with pytest.raises(SystemExit):
-                self.parse("--deploy-hook foo --renew-hook bar".split())
 
     def test_renew_hook_matches_deploy_hook(self):
         value = "foo"
@@ -455,14 +455,20 @@ class ParseTest(unittest.TestCase):
                                 "--renew-hook", value,
                                 "--disable-hook-validation"])
         assert_set_by_user_with_value(namespace, 'deploy_hook', value)
-        assert_set_by_user_with_value(namespace, 'renew_hook', value)
 
     def test_renew_hook_does_not_set_renew_hook(self):
         value = "foo"
         namespace = self.parse(
             ["--renew-hook", value, "--disable-hook-validation"])
-        assert namespace.deploy_hook is None
-        assert_set_by_user_with_value(namespace, 'renew_hook', value)
+        assert not hasattr(namespace, "renew_hook")
+        assert_set_by_user_with_value(namespace, 'deploy_hook', value)
+
+    def test_deploy_hook_does_not_set_renew_hook(self):
+        value = "foo"
+        namespace = self.parse(
+            ["--deploy-hook", value, "--disable-hook-validation"])
+        assert not hasattr(namespace, "renew_hook")
+        assert_set_by_user_with_value(namespace, 'deploy_hook', value)
 
     def test_max_log_backups_error(self):
         with mock.patch('certbot._internal.cli.sys.stderr'):

certbot/src/certbot/_internal/tests/hook_test.py

@@ -38,9 +38,7 @@ class ValidateHooksTest(unittest.TestCase):
         self._call(config)
 
         types = [call[0][1] for call in mock_validate_hook.call_args_list]
-        assert {"pre", "post", "deploy",} == set(types[:-1])
-        # This ensures error messages are about deploy hooks when appropriate
-        assert "renew" == types[-1]
+        assert {"pre", "post", "deploy",} == set(types)
 
 
 class ValidateHookTest(test_util.TempDirTestCase):
@@ -369,20 +367,22 @@ class DeployHookTest(RenewalHookTest):
         from certbot._internal.hooks import deploy_hook
         return deploy_hook(*args, **kwargs)
 
-    @mock.patch("certbot._internal.hooks.logger")
-    def test_dry_run(self, mock_logger):
+    def setUp(self):
+        super().setUp()
         self.config.deploy_hook = "foo"
-        self.config.dry_run = True
-        mock_execute = self._call_with_mock_execute(
-            self.config, ["example.org"], "/foo/bar")
-        assert mock_execute.called is False
-        assert mock_logger.info.called
 
-    @mock.patch("certbot._internal.hooks.logger")
-    def test_no_hook(self, mock_logger):
+        filesystem.makedirs(self.config.renewal_deploy_hooks_dir)
+        self.dir_hook = os.path.join(self.config.renewal_deploy_hooks_dir,
+                                     "bar")
+        create_hook(self.dir_hook)
+
+    def test_no_hooks(self):
         self.config.deploy_hook = None
-        mock_execute = self._call_with_mock_execute(
-            self.config, ["example.org"], "/foo/bar")
+        os.remove(self.dir_hook)
+
+        with mock.patch("certbot._internal.hooks.logger") as mock_logger:
+            mock_execute = self._call_with_mock_execute(
+                self.config, ["example.org"], "/foo/bar")
         assert mock_execute.called is False
         assert mock_logger.info.called is False
 
@@ -392,31 +392,13 @@ class DeployHookTest(RenewalHookTest):
         self.config.deploy_hook = "foo"
         mock_execute = self._call_with_mock_execute(
             self.config, domains, lineage)
-        mock_execute.assert_called_once_with("deploy-hook", self.config.deploy_hook, env=mock.ANY)
-
-
-class RenewHookTest(RenewalHookTest):
-    """Tests for certbot._internal.hooks.renew_hook"""
-
-    @classmethod
-    def _call(cls, *args, **kwargs):
-        from certbot._internal.hooks import renew_hook
-        return renew_hook(*args, **kwargs)
-
-    def setUp(self):
-        super().setUp()
-        self.config.renew_hook = "foo"
-
-        filesystem.makedirs(self.config.renewal_deploy_hooks_dir)
-        self.dir_hook = os.path.join(self.config.renewal_deploy_hooks_dir,
-                                     "bar")
-        create_hook(self.dir_hook)
+        assert mock_execute.call_count == 2
 
     def test_disabled_dir_hooks(self):
         self.config.directory_hooks = False
         mock_execute = self._call_with_mock_execute(
             self.config, ["example.org"], "/foo/bar")
-        mock_execute.assert_called_once_with("deploy-hook", self.config.renew_hook, env=mock.ANY)
+        mock_execute.assert_called_once_with("deploy-hook", self.config.deploy_hook, env=mock.ANY)
 
     @mock.patch("certbot._internal.hooks.logger")
     def test_dry_run(self, mock_logger):
@@ -426,18 +408,8 @@ class RenewHookTest(RenewalHookTest):
         assert mock_execute.called is False
         assert mock_logger.info.call_count == 2
 
-    def test_no_hooks(self):
-        self.config.renew_hook = None
-        os.remove(self.dir_hook)
-
-        with mock.patch("certbot._internal.hooks.logger") as mock_logger:
-            mock_execute = self._call_with_mock_execute(
-                self.config, ["example.org"], "/foo/bar")
-        assert mock_execute.called is False
-        assert mock_logger.info.called is False
-
     def test_overlap(self):
-        self.config.renew_hook = self.dir_hook
+        self.config.deploy_hook = self.dir_hook
         mock_execute = self._call_with_mock_execute(
             self.config, ["example.net", "example.org"], "/foo/bar")
         mock_execute.assert_called_once_with("deploy-hook", self.dir_hook, env=mock.ANY)
@@ -446,7 +418,7 @@ class RenewHookTest(RenewalHookTest):
         mock_execute = self._call_with_mock_execute(
             self.config, ["example.org"], "/foo/bar")
         mock_execute.assert_any_call("deploy-hook", self.dir_hook, env=mock.ANY)
-        mock_execute.assert_called_with("deploy-hook", self.config.renew_hook, env=mock.ANY)
+        mock_execute.assert_called_with("deploy-hook", self.config.deploy_hook, env=mock.ANY)
 
 
 class ListHooksTest(test_util.TempDirTestCase):

certbot/src/certbot/_internal/tests/renewal_test.py

@@ -96,7 +96,7 @@ class RenewalTest(test_util.ConfigTestCase):
 
         from certbot._internal import renewal
 
-        with mock.patch('certbot._internal.renewal.hooks.renew_hook'):
+        with mock.patch('certbot._internal.renewal.hooks.deploy_hook'):
             renewal.renew_cert(self.config, None, le_client, lineage)
 
         assert self.config.elliptic_curve == 'secp256r1'
@@ -121,7 +121,7 @@ class RenewalTest(test_util.ConfigTestCase):
 
         from certbot._internal import renewal
 
-        with mock.patch('certbot._internal.renewal.hooks.renew_hook'):
+        with mock.patch('certbot._internal.renewal.hooks.deploy_hook'):
             renewal.renew_cert(self.config, None, le_client, lineage)
 
         assert self.config.elliptic_curve == 'secp256r1'
@@ -145,7 +145,7 @@ class RenewalTest(test_util.ConfigTestCase):
 
         from certbot._internal import renewal
 
-        with mock.patch('certbot._internal.renewal.hooks.renew_hook'):
+        with mock.patch('certbot._internal.renewal.hooks.deploy_hook'):
             renewal.renew_cert(self.config, None, le_client, lineage)
 
         assert self.config.elliptic_curve == 'secp256r1'
@@ -154,9 +154,9 @@ class RenewalTest(test_util.ConfigTestCase):
         # None is passed as the existing key, i.e. the key is not actually being reused.
         le_client.obtain_certificate.assert_called_with(mock.ANY, None)
 
-    @mock.patch('certbot._internal.renewal.hooks.renew_hook')
+    @mock.patch('certbot._internal.renewal.hooks.deploy_hook')
     @mock.patch.object(configuration.NamespaceConfig, 'set_by_user')
-    def test_reuse_key_conflicts(self, mock_set_by_user, unused_mock_renew_hook):
+    def test_reuse_key_conflicts(self, mock_set_by_user, unused_mock_deploy_hook):
         mock_set_by_user.return_value = False
 
         # When renewing with reuse_key and a conflicting key parameter (size, curve)

certbot/src/certbot/configuration.py

@@ -419,25 +419,31 @@ class NamespaceConfig:
 
     @property
     def renewal_hooks_dir(self) -> str:
-        """Path to directory with hooks to run with the renew subcommand."""
+        """Path to directory with hooks to run when getting or renewing certificates."""
         return os.path.join(self.namespace.config_dir,
                             constants.RENEWAL_HOOKS_DIR)
 
     @property
     def renewal_pre_hooks_dir(self) -> str:
-        """Path to the pre-hook directory for the renew subcommand."""
+        """Path to the pre-hook directory for hooks to run
+        before attempting to get or renew certs.
+        """
         return os.path.join(self.renewal_hooks_dir,
                             constants.RENEWAL_PRE_HOOKS_DIR)
 
     @property
     def renewal_deploy_hooks_dir(self) -> str:
-        """Path to the deploy-hook directory for the renew subcommand."""
+        """Path to the deploy-hook directory for hooks to run
+        upon successfully getting or renewing certs.
+        """
         return os.path.join(self.renewal_hooks_dir,
                             constants.RENEWAL_DEPLOY_HOOKS_DIR)
 
     @property
     def renewal_post_hooks_dir(self) -> str:
-        """Path to the post-hook directory for the renew subcommand."""
+        """Path to the post-hook directory for hooks to run
+        after attempting to get or renew certs.
+        """
         return os.path.join(self.renewal_hooks_dir,
                             constants.RENEWAL_POST_HOOKS_DIR)
 

newsfragments/9978.changed

@@ -0,0 +1 @@
+Deploy directory hooks are now also run when using `certbot certonly` or `certbot run` to get a new cert. This change was made for pre and post directory hooks in our 3.2.0 release so this change unifies Certbot's behavior here.