diff --git a/certbot-nginx/certbot_nginx/_internal/configurator.py b/certbot-nginx/certbot_nginx/_internal/configurator.py index 07397bfe8..ec9f9053d 100644 --- a/certbot-nginx/certbot_nginx/_internal/configurator.py +++ b/certbot-nginx/certbot_nginx/_internal/configurator.py @@ -769,9 +769,6 @@ class NginxConfigurator(common.Installer): except (KeyError, ValueError): raise errors.PluginError( "Unsupported enhancement: {0}".format(enhancement)) - except errors.PluginError: - logger.error("Failed %s for %s", enhancement, domain) - raise def _has_certbot_redirect(self, vhost, domain): test_redirect_block = _test_block_from_block(_redirect_block_for_domain(domain)) @@ -791,6 +788,10 @@ class NginxConfigurator(common.Installer): :raises .errors.PluginError: If no viable HTTPS host can be created or set with header header_substring. """ + if not header_substring in constants.HEADER_ARGS: + raise errors.NotSupportedError( + f"{header_substring} is not supported by the nginx plugin.") + vhosts = self.choose_vhosts(domain) if not vhosts: raise errors.PluginError( diff --git a/certbot/certbot/_internal/client.py b/certbot/certbot/_internal/client.py index 5c0bed220..1ae65aa46 100644 --- a/certbot/certbot/_internal/client.py +++ b/certbot/certbot/_internal/client.py @@ -2,7 +2,7 @@ import datetime import logging import platform -from typing import Optional +from typing import List, Optional, Union from cryptography.hazmat.backends import default_backend # See https://github.com/pyca/cryptography/issues/4275 @@ -598,7 +598,8 @@ class Client: with error_handler.ErrorHandler(self._rollback_and_restart, msg): self.installer.restart() - def apply_enhancement(self, domains, enhancement, options=None): + def apply_enhancement(self, domains: List[str], enhancement: str, + options: Optional[Union[List[str], str]] = None) -> None: """Applies an enhancement on all domains. :param list domains: list of ssl_vhosts (as strings) @@ -612,33 +613,28 @@ class Client: """ - msg = f"Could not set up {enhancement} enhancement" - with error_handler.ErrorHandler(self._recovery_routine_with_msg, msg): + enh_label = options if enhancement == "ensure-http-header" else enhancement + with error_handler.ErrorHandler(self._recovery_routine_with_msg, None): for dom in domains: try: self.installer.enhance(dom, enhancement, options) except errors.PluginEnhancementAlreadyPresent: - if enhancement == "ensure-http-header": - logger.info("Enhancement %s was already set.", - options) - else: - logger.info("Enhancement %s was already set.", - enhancement) + logger.info("Enhancement %s was already set.", enh_label) except errors.PluginError: - logger.error("Unable to set enhancement %s for %s", - enhancement, dom) + logger.error("Unable to set the %s enhancement for %s.", enh_label, dom) raise - self.installer.save("Add enhancement %s" % (enhancement)) + self.installer.save(f"Add enhancement {enh_label}") - def _recovery_routine_with_msg(self, success_msg): + def _recovery_routine_with_msg(self, success_msg: Optional[str]) -> None: """Calls the installer's recovery routine and prints success_msg :param str success_msg: message to show on successful recovery """ self.installer.recovery_routine() - display_util.notify(success_msg) + if success_msg: + display_util.notify(success_msg) def _rollback_and_restart(self, success_msg): """Rollback the most recent checkpoint and restart the webserver diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py index 51c6767f6..2c2d50abc 100644 --- a/certbot/tests/client_test.py +++ b/certbot/tests/client_test.py @@ -712,7 +712,7 @@ class EnhanceConfigTest(ClientTestCommon): if enhance_error: self.assertEqual(mock_logger.error.call_count, 1) - self.assertIn('Unable to set enhancement', mock_logger.error.call_args_list[0][0][0]) + self.assertEqual('Unable to set the %s enhancement for %s.', mock_logger.error.call_args_list[0][0][0]) if restart_error: mock_logger.critical.assert_called_with( 'Rolling back to previous server configuration...')