diff --git a/certbot-nginx/certbot_nginx/_internal/configurator.py b/certbot-nginx/certbot_nginx/_internal/configurator.py index a70572c33..3da38ff2d 100644 --- a/certbot-nginx/certbot_nginx/_internal/configurator.py +++ b/certbot-nginx/certbot_nginx/_internal/configurator.py @@ -77,6 +77,9 @@ class NginxConfigurator(common.Installer): add("ctl", default=constants.CLI_DEFAULTS["ctl"], help="Path to the " "'nginx' binary, used for 'configtest' and retrieving nginx " "version number.") + add("sleep-seconds", default=constants.CLI_DEFAULTS["sleep_seconds"], type=int, + help="Number of seconds to wait for nginx configuration changes " + "to apply when reloading.") @property def nginx_conf(self): @@ -912,7 +915,7 @@ class NginxConfigurator(common.Installer): :raises .errors.MisconfigurationError: If either the reload fails. """ - nginx_restart(self.conf('ctl'), self.nginx_conf) + nginx_restart(self.conf('ctl'), self.nginx_conf, self.conf('sleep-seconds')) def config_test(self): """Check the configuration of Nginx for errors. @@ -1159,7 +1162,7 @@ def _redirect_block_for_domain(domain): return redirect_block -def nginx_restart(nginx_ctl, nginx_conf): +def nginx_restart(nginx_ctl, nginx_conf, sleep_duration): """Restarts the Nginx Server. .. todo:: Nginx restart is fatal if the configuration references @@ -1167,6 +1170,8 @@ def nginx_restart(nginx_ctl, nginx_conf): before restart. :param str nginx_ctl: Path to the Nginx binary. + :param str nginx_conf: Path to the Nginx configuration file. + :param int sleep_duration: How long to sleep after sending the reload signal. """ try: @@ -1190,10 +1195,12 @@ def nginx_restart(nginx_ctl, nginx_conf): except (OSError, ValueError): raise errors.MisconfigurationError("nginx restart failed") - # Nginx can take a moment to recognize a newly added TLS SNI servername, so sleep - # for a second. TODO: Check for expected servername and loop until it - # appears or return an error if looping too long. - time.sleep(1) + # Nginx can take a significant duration of time to fully apply a new config, depending + # on size and contents (https://github.com/certbot/certbot/issues/7422). Lacking a way + # to reliably identify when this process is complete, we provide the user with control + # over how long Certbot will sleep after reloading the configuration. + if sleep_duration > 0: + time.sleep(sleep_duration) def _determine_default_server_root(): diff --git a/certbot-nginx/certbot_nginx/_internal/constants.py b/certbot-nginx/certbot_nginx/_internal/constants.py index c10eb5098..1f058e7ef 100644 --- a/certbot-nginx/certbot_nginx/_internal/constants.py +++ b/certbot-nginx/certbot_nginx/_internal/constants.py @@ -1,6 +1,9 @@ """nginx plugin constants.""" import platform +from acme.magic_typing import Any +from acme.magic_typing import Dict + FREEBSD_DARWIN_SERVER_ROOT = "/usr/local/etc/nginx" LINUX_SERVER_ROOT = "/etc/nginx" PKGSRC_SERVER_ROOT = "/usr/pkg/etc/nginx" @@ -15,7 +18,8 @@ else: CLI_DEFAULTS = dict( server_root=server_root_tmp, ctl="nginx", -) + sleep_seconds=1 +) # type: Dict[str, Any] """CLI defaults.""" diff --git a/certbot-nginx/tests/configurator_test.py b/certbot-nginx/tests/configurator_test.py index 2c3264a5f..0a5adcd3f 100644 --- a/certbot-nginx/tests/configurator_test.py +++ b/certbot-nginx/tests/configurator_test.py @@ -460,11 +460,13 @@ class NginxConfiguratorTest(util.NginxTest): self.assertEqual(self.config._get_openssl_version(), "") @mock.patch("certbot_nginx._internal.configurator.subprocess.Popen") - def test_nginx_restart(self, mock_popen): + @mock.patch("certbot_nginx._internal.configurator.time") + def test_nginx_restart(self, mock_time, mock_popen): mocked = mock_popen() mocked.communicate.return_value = ('', '') mocked.returncode = 0 self.config.restart() + mock_time.sleep.assert_called_once_with(0.1234) @mock.patch("certbot_nginx._internal.configurator.subprocess.Popen") def test_nginx_restart_fail(self, mock_popen): diff --git a/certbot-nginx/tests/test_util.py b/certbot-nginx/tests/test_util.py index 4b26f7935..f545dc5bc 100644 --- a/certbot-nginx/tests/test_util.py +++ b/certbot-nginx/tests/test_util.py @@ -56,6 +56,7 @@ class NginxTest(test_log_util.AssertLogsMixin, test_util.ConfigTestCase): backups = os.path.join(work_dir, "backups") self.configuration.nginx_server_root = config_path + self.configuration.nginx_sleep_seconds = 0.1234 self.configuration.le_vhost_ext = "-le-ssl.conf" self.configuration.config_dir = config_dir self.configuration.work_dir = work_dir diff --git a/certbot/CHANGELOG.md b/certbot/CHANGELOG.md index 4eca065af..92e89e540 100644 --- a/certbot/CHANGELOG.md +++ b/certbot/CHANGELOG.md @@ -9,6 +9,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/). * Third-party plugins can be used without prefix (`plugin_name` instead of `dist_name:plugin_name`): this concerns the plugin name, CLI flags, and keys in credential files. The prefixed form is still supported but is deprecated, and will be removed in a future release. +* Added `--nginx-sleep-seconds` (default `1`) for environments where nginx takes a long time to reload. ### Changed