diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index 8b996c675..722e94e18 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/certbot-apache/certbot_apache/configurator.py @@ -2000,10 +2000,27 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :raises .errors.MisconfigurationError: If reload fails """ + error = "" try: util.run_script(self.constant("restart_cmd")) except errors.SubprocessError as err: - raise errors.MisconfigurationError(str(err)) + logger.info("Unable to restart apache using %s", + self.constant("restart_cmd")) + alt_restart = self.constant("restart_cmd_alt") + if alt_restart: + logger.debug("Trying alternative restart command: %s", + alt_restart) + # There is an alternative restart command available + # This usually is "restart" verb while original is "graceful" + try: + util.run_script(self.constant( + "restart_cmd_alt")) + return + except errors.SubprocessError as secerr: + error = str(secerr) + else: + error = str(err) + raise errors.MisconfigurationError(error) def config_test(self): # pylint: disable=no-self-use """Check the configuration of Apache for errors. diff --git a/certbot-apache/certbot_apache/override_centos.py b/certbot-apache/certbot_apache/override_centos.py index db6cd6fba..6e75e361d 100644 --- a/certbot-apache/certbot_apache/override_centos.py +++ b/certbot-apache/certbot_apache/override_centos.py @@ -21,6 +21,7 @@ class CentOSConfigurator(configurator.ApacheConfigurator): version_cmd=['apachectl', '-v'], apache_cmd="apachectl", restart_cmd=['apachectl', 'graceful'], + restart_cmd_alt=['apachectl', 'restart'], conftest_cmd=['apachectl', 'configtest'], enmod=None, dismod=None, diff --git a/certbot-apache/certbot_apache/override_gentoo.py b/certbot-apache/certbot_apache/override_gentoo.py index 92f1d4a20..165e44c96 100644 --- a/certbot-apache/certbot_apache/override_gentoo.py +++ b/certbot-apache/certbot_apache/override_gentoo.py @@ -21,6 +21,7 @@ class GentooConfigurator(configurator.ApacheConfigurator): version_cmd=['/usr/sbin/apache2', '-v'], apache_cmd="apache2ctl", restart_cmd=['apache2ctl', 'graceful'], + restart_cmd_alt=['apache2ctl', 'restart'], conftest_cmd=['apache2ctl', 'configtest'], enmod=None, dismod=None, diff --git a/certbot-apache/certbot_apache/tests/centos_test.py b/certbot-apache/certbot_apache/tests/centos_test.py index d7a2a2fd9..4ee8b5dcf 100644 --- a/certbot-apache/certbot_apache/tests/centos_test.py +++ b/certbot-apache/certbot_apache/tests/centos_test.py @@ -4,6 +4,8 @@ import unittest import mock +from certbot import errors + from certbot_apache import obj from certbot_apache import override_centos from certbot_apache.tests import util @@ -121,5 +123,17 @@ class MultipleVhostsTestCentOS(util.ApacheTest): self.assertTrue("MOCK_NOSEP" in self.config.parser.variables.keys()) self.assertEqual("NOSEP_VAL", self.config.parser.variables["NOSEP_TWO"]) + @mock.patch("certbot_apache.configurator.util.run_script") + def test_alt_restart_works(self, mock_run_script): + mock_run_script.side_effect = [None, errors.SubprocessError, None] + self.config.restart() + self.assertEquals(mock_run_script.call_count, 3) + + @mock.patch("certbot_apache.configurator.util.run_script") + def test_alt_restart_errors(self, mock_run_script): + mock_run_script.side_effect = [None, + errors.SubprocessError, + errors.SubprocessError] + self.assertRaises(errors.MisconfigurationError, self.config.restart) if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/certbot-apache/certbot_apache/tests/gentoo_test.py b/certbot-apache/certbot_apache/tests/gentoo_test.py index cfbaffac7..d32551267 100644 --- a/certbot-apache/certbot_apache/tests/gentoo_test.py +++ b/certbot-apache/certbot_apache/tests/gentoo_test.py @@ -4,6 +4,8 @@ import unittest import mock +from certbot import errors + from certbot_apache import override_gentoo from certbot_apache import obj from certbot_apache.tests import util @@ -123,5 +125,11 @@ class MultipleVhostsTestGentoo(util.ApacheTest): self.assertEquals(len(self.config.parser.modules), 4) self.assertTrue("mod_another.c" in self.config.parser.modules) + @mock.patch("certbot_apache.configurator.util.run_script") + def test_alt_restart_works(self, mock_run_script): + mock_run_script.side_effect = [None, errors.SubprocessError, None] + self.config.restart() + self.assertEquals(mock_run_script.call_count, 3) + if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/certbot-nginx/certbot_nginx/configurator.py b/certbot-nginx/certbot_nginx/configurator.py index 13fe493fc..3ba8bcb06 100644 --- a/certbot-nginx/certbot_nginx/configurator.py +++ b/certbot-nginx/certbot_nginx/configurator.py @@ -914,7 +914,7 @@ class NginxConfigurator(common.Installer): raise errors.PluginError("Nginx build doesn't support SNI") product_name, product_version = version_matches[0] - if product_name is not 'nginx': + if product_name != 'nginx': logger.warning("NGINX derivative %s is not officially supported by" " certbot", product_name) diff --git a/certbot-nginx/certbot_nginx/parser.py b/certbot-nginx/certbot_nginx/parser.py index 577e783fc..f06cd17a7 100644 --- a/certbot-nginx/certbot_nginx/parser.py +++ b/certbot-nginx/certbot_nginx/parser.py @@ -743,7 +743,7 @@ def _parse_server_raw(server): if addr.ssl: parsed_server['ssl'] = True elif directive[0] == 'server_name': - parsed_server['names'].update(directive[1:]) + parsed_server['names'].update(x.strip('"\'') for x in directive[1:]) elif _is_ssl_on_directive(directive): parsed_server['ssl'] = True apply_ssl_to_all_addrs = True diff --git a/certbot-nginx/certbot_nginx/tests/configurator_test.py b/certbot-nginx/certbot_nginx/tests/configurator_test.py index 34abf2f0d..e88dcb8e0 100644 --- a/certbot-nginx/certbot_nginx/tests/configurator_test.py +++ b/certbot-nginx/certbot_nginx/tests/configurator_test.py @@ -639,7 +639,7 @@ class NginxConfiguratorTest(util.NginxTest): self.assertEqual([[['server'], [['listen', 'myhost', 'default_server'], ['listen', 'otherhost', 'default_server'], - ['server_name', 'www.example.org'], + ['server_name', '"www.example.org"'], [['location', '/'], [['root', 'html'], ['index', 'index.html', 'index.htm']]]]], diff --git a/certbot-nginx/certbot_nginx/tests/testdata/etc_nginx/sites-enabled/default b/certbot-nginx/certbot_nginx/tests/testdata/etc_nginx/sites-enabled/default index 4f67fa7d1..e167761d1 100644 --- a/certbot-nginx/certbot_nginx/tests/testdata/etc_nginx/sites-enabled/default +++ b/certbot-nginx/certbot_nginx/tests/testdata/etc_nginx/sites-enabled/default @@ -1,7 +1,7 @@ server { listen myhost default_server; listen otherhost default_server; - server_name www.example.org; + server_name "www.example.org"; location / { root html; diff --git a/docs/api/constants.rst b/docs/api/constants.rst index e225056a2..99ecc240a 100644 --- a/docs/api/constants.rst +++ b/docs/api/constants.rst @@ -3,3 +3,7 @@ .. automodule:: certbot.constants :members: + :exclude-members: SSL_DHPARAMS_SRC + +.. autodata:: SSL_DHPARAMS_SRC + :annotation: = '/path/to/certbot/ssl-dhparams.pem'