diff --git a/certbot-ci/src/certbot_integration_tests/certbot_tests/assertions.py b/certbot-ci/src/certbot_integration_tests/certbot_tests/assertions.py index 69582fba5..072462162 100644 --- a/certbot-ci/src/certbot_integration_tests/certbot_tests/assertions.py +++ b/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 """ diff --git a/certbot-ci/src/certbot_integration_tests/certbot_tests/test_main.py b/certbot-ci/src/certbot_integration_tests/certbot_tests/test_main.py index 93dd2bd37..6a2b8a348 100644 --- a/certbot-ci/src/certbot_integration_tests/certbot_tests/test_main.py +++ b/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']) diff --git a/certbot/src/certbot/_internal/cli/__init__.py b/certbot/src/certbot/_internal/cli/__init__.py index 5e70c6cf7..29d4c034a 100644 --- a/certbot/src/certbot/_internal/cli/__init__.py +++ b/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' diff --git a/certbot/src/certbot/_internal/cli/cli_constants.py b/certbot/src/certbot/_internal/cli/cli_constants.py index 97bdd5691..d490714de 100644 --- a/certbot/src/certbot/_internal/cli/cli_constants.py +++ b/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",}} diff --git a/certbot/src/certbot/_internal/cli/cli_utils.py b/certbot/src/certbot/_internal/cli/cli_utils.py index 0efae8fff..798455fa4 100644 --- a/certbot/src/certbot/_internal/cli/cli_utils.py +++ b/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. diff --git a/certbot/src/certbot/_internal/client.py b/certbot/src/certbot/_internal/client.py index f2783d2b6..28054d933 100644 --- a/certbot/src/certbot/_internal/client.py +++ b/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") diff --git a/certbot/src/certbot/_internal/constants.py b/certbot/src/certbot/_internal/constants.py index 240a81898..bf4243bd9 100644 --- a/certbot/src/certbot/_internal/constants.py +++ b/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.""" diff --git a/certbot/src/certbot/_internal/hooks.py b/certbot/src/certbot/_internal/hooks.py index 7a4e965fb..a7c77b6ae 100644 --- a/certbot/src/certbot/_internal/hooks.py +++ b/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) diff --git a/certbot/src/certbot/_internal/renewal.py b/certbot/src/certbot/_internal/renewal.py index bc91d9359..159123801 100644 --- a/certbot/src/certbot/_internal/renewal.py +++ b/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: ") diff --git a/certbot/src/certbot/_internal/storage.py b/certbot/src/certbot/_internal/storage.py index 2cfa8014c..d775ba971 100644 --- a/certbot/src/certbot/_internal/storage.py +++ b/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() diff --git a/certbot/src/certbot/_internal/tests/cli_test.py b/certbot/src/certbot/_internal/tests/cli_test.py index 1aa1fe318..83a435550 100644 --- a/certbot/src/certbot/_internal/tests/cli_test.py +++ b/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'): diff --git a/certbot/src/certbot/_internal/tests/hook_test.py b/certbot/src/certbot/_internal/tests/hook_test.py index 305b306f6..c09461b57 100644 --- a/certbot/src/certbot/_internal/tests/hook_test.py +++ b/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): diff --git a/certbot/src/certbot/_internal/tests/renewal_test.py b/certbot/src/certbot/_internal/tests/renewal_test.py index 4ab2e0aa9..c78099b48 100644 --- a/certbot/src/certbot/_internal/tests/renewal_test.py +++ b/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) diff --git a/certbot/src/certbot/configuration.py b/certbot/src/certbot/configuration.py index 26214299d..2640c01f3 100644 --- a/certbot/src/certbot/configuration.py +++ b/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) diff --git a/newsfragments/9978.changed b/newsfragments/9978.changed new file mode 100644 index 000000000..277f43524 --- /dev/null +++ b/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.