diff --git a/.gitignore b/.gitignore index 9dac99790..9564cfbc7 100644 --- a/.gitignore +++ b/.gitignore @@ -16,6 +16,7 @@ dist/ *~ *.swp \#*# +.idea # auth --cert-path --chain-path -/*.pem \ No newline at end of file +/*.pem diff --git a/docs/pkgs/letsencrypt_apache.rst b/docs/pkgs/letsencrypt_apache.rst index f78caa971..c4e7e8e6e 100644 --- a/docs/pkgs/letsencrypt_apache.rst +++ b/docs/pkgs/letsencrypt_apache.rst @@ -10,6 +10,12 @@ .. automodule:: letsencrypt_apache.configurator :members: +:mod:`letsencrypt_apache.display_ops` +===================================== + +.. automodule:: letsencrypt_apache.display_ops + :members: + :mod:`letsencrypt_apache.dvsni` =============================== diff --git a/letsencrypt/client.py b/letsencrypt/client.py index c4f5507f3..8092d6179 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -397,7 +397,7 @@ class Client(object): for dom in domains: try: self.installer.enhance(dom, "redirect") - except errors.ConfiguratorError: + except errors.PluginError: logger.warn("Unable to perform redirect for %s", dom) self.installer.save("Add Redirects") diff --git a/letsencrypt/errors.py b/letsencrypt/errors.py index f753a29c0..bfe8ee28d 100644 --- a/letsencrypt/errors.py +++ b/letsencrypt/errors.py @@ -46,16 +46,16 @@ class DvsniError(DvAuthError): """Let's Encrypt DVSNI error.""" -# Configurator Errors -class ConfiguratorError(Error): - """Let's Encrypt Configurator error.""" +# Plugin Errors +class PluginError(Error): + """Let's Encrypt Plugin error.""" -class NoInstallationError(ConfiguratorError): +class NoInstallationError(PluginError): """Let's Encrypt No Installation error.""" -class MisconfigurationError(ConfiguratorError): +class MisconfigurationError(PluginError): """Let's Encrypt Misconfiguration error.""" diff --git a/letsencrypt_apache/configurator.py b/letsencrypt_apache/configurator.py index d3ebb35fb..c8083b406 100644 --- a/letsencrypt_apache/configurator.py +++ b/letsencrypt_apache/configurator.py @@ -5,7 +5,6 @@ import re import shutil import socket import subprocess -import sys import zope.interface @@ -21,6 +20,7 @@ from letsencrypt.plugins import common from letsencrypt_apache import augeas_configurator from letsencrypt_apache import constants +from letsencrypt_apache import display_ops from letsencrypt_apache import dvsni from letsencrypt_apache import obj from letsencrypt_apache import parser @@ -86,7 +86,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): zope.interface.implements(interfaces.IAuthenticator, interfaces.IInstaller) zope.interface.classProvides(interfaces.IPluginFactory) - description = "Apache Web Server" + description = "Apache Web Server - Alpha" @classmethod def add_parser_arguments(cls, add): @@ -103,7 +103,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): add("le-vhost-ext", default=constants.CLI_DEFAULTS["le_vhost_ext"], help="SSL vhost configuration extension.") - def __init__(self, *args, **kwargs): """Initialize an Apache Configurator. @@ -111,7 +110,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): (used mostly for unittesting) """ - version = kwargs.pop('version', None) + version = kwargs.pop("version", None) super(ApacheConfigurator, self).__init__(*args, **kwargs) # Verify that all directories and files exist with proper permissions @@ -137,7 +136,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): def prepare(self): """Prepare the authenticator/installer.""" self.parser = parser.ApacheParser( - self.aug, self.conf('server-root'), self.mod_ssl_conf) + self.aug, self.conf("server-root"), self.mod_ssl_conf) # Check for errors in parsing files with Augeas self.check_parsing_errors("httpd.aug") @@ -174,7 +173,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ vhost = self.choose_vhost(domain) - # TODO(jdkasten): vhost might be None + path = {} path["cert_path"] = self.parser.find_dir(parser.case_i( @@ -221,14 +220,16 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): def choose_vhost(self, target_name): """Chooses a virtual host based on the given domain name. - .. todo:: This should maybe return list if no obvious answer - is presented. + If there is no clear virtual host to be selected, the user is prompted + with all available choices. :param str target_name: domain name :returns: ssl vhost associated with name :rtype: :class:`~letsencrypt_apache.obj.VirtualHost` + :raises .errors.PluginError: If no vhost is available + """ # Allows for domain names to be associated with a virtual host # Client isn't using create_dn_server_assoc(self, dn, vh) yet @@ -254,11 +255,24 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.assoc[target_name] = vhost return vhost - # No matches, search for the default - for vhost in self.vhosts: - if "_default_:443" in vhost.addrs: - return vhost - return None + vhost = display_ops.select_vhost(target_name, self.vhosts) + if vhost is not None: + self.assoc[target_name] = vhost + else: + logger.error( + "No vhost exists with servername or alias of: %s. " + "No vhost was selected. Please specify servernames " + "in the Apache config", target_name) + raise errors.PluginError("No vhost selected") + + # TODO: Ask the user if they would like to add ServerName/Alias to VH + + return vhost + + # # No matches, search for the default + # for vhost in self.vhosts: + # if "_default_:443" in vhost.addrs: + # return vhost def create_dn_server_assoc(self, domain, vhost): """Create an association between a domain name and virtual host. @@ -408,10 +422,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): is appropriately listening on port 443. """ - if not mod_loaded("ssl_module", self.conf('ctl')): + if not self.mod_loaded("ssl_module"): logger.info("Loading mod_ssl into Apache Server") - enable_mod("ssl", self.conf('init-script'), - self.conf('enmod')) + self.enable_mod("ssl") # Check for Listen 443 # Note: This could be made to also look for ip:443 combo @@ -465,6 +478,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :returns: SSL vhost :rtype: :class:`~letsencrypt_apache.obj.VirtualHost` + :raises .errors.PluginError: If more than one virtual host is in + the file or if plugin is unable to write/read vhost files. + """ avail_fp = nonssl_vhost.filep # Get filepath of new ssl_vhost @@ -486,7 +502,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): new_file.write("\n") except IOError: logger.fatal("Error writing/reading to file in make_vhost_ssl") - sys.exit(49) + raise errors.PluginError("Unable to write/read in make_vhost_ssl") self.aug.load() @@ -509,7 +525,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): (ssl_fp, parser.case_i("VirtualHost"))) if len(vh_p) != 1: logger.error("Error: should only be one vhost in %s", avail_fp) - sys.exit(1) + raise errors.PluginError("Only one vhost per file is allowed") self.parser.add_dir(vh_p[0], "SSLCertificateFile", "/etc/ssl/certs/ssl-cert-snakeoil.pem") @@ -564,9 +580,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): return self._enhance_func[enhancement]( self.choose_vhost(domain), options) except ValueError: - raise errors.ConfiguratorError( + raise errors.PluginError( "Unsupported enhancement: {}".format(enhancement)) - except errors.ConfiguratorError: + except errors.PluginError: logger.warn("Failed %s for %s", enhancement, domain) def _enable_redirect(self, ssl_vhost, unused_options): @@ -592,8 +608,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :rtype: (bool, :class:`~letsencrypt_apache.obj.VirtualHost`) """ - if not mod_loaded("rewrite_module", self.conf('ctl')): - enable_mod("rewrite", self.conf('init-script'), self.conf('enmod')) + if not self.mod_loaded("rewrite_module"): + self.enable_mod("rewrite") general_v = self._general_vhost(ssl_vhost) if general_v is None: @@ -612,7 +628,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): return else: logger.info("Unknown redirect exists for this vhost") - raise errors.ConfiguratorError( + raise errors.PluginError( "Unknown redirect already exists " "in {}".format(general_v.filep)) # Add directives to server @@ -683,7 +699,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # Make sure adding the vhost will be safe conflict, host_or_addrs = self._conflicting_host(ssl_vhost) if conflict: - raise errors.ConfiguratorError( + raise errors.PluginError( "Unable to create a redirection vhost - {}".format( host_or_addrs)) @@ -850,7 +866,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): if len(cert_path) != 1 or len(key_path) != 1: logger.error("Too many cert or key directives in vhost %s", vhost.filep) - sys.exit(40) + errors.MisconfigurationError( + "Too many cert/key directives in vhost") cert = os.path.abspath(self.aug.get(cert_path[0])) key = os.path.abspath(self.aug.get(key_path[0])) @@ -904,6 +921,58 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): return True return False + def enable_mod(self, mod_name): + """Enables module in Apache. + + Both enables and restarts Apache so module is active. + + :param str mod_name: Name of the module to enable. + + """ + try: + # Use check_output so the command will finish before reloading + # TODO: a2enmod is debian specific... + subprocess.check_call([self.conf("enmod"), mod_name], + stdout=open("/dev/null", "w"), + stderr=open("/dev/null", "w")) + apache_restart(self.conf("init")) + except (OSError, subprocess.CalledProcessError): + logger.exception("Error enabling mod_%s", mod_name) + raise errors.MisconfigurationError( + "Missing enable_mod binary or lack privileges") + + def mod_loaded(self, module): + """Checks to see if mod_ssl is loaded + + Uses ``apache_ctl`` to get loaded module list. This also effectively + serves as a config_test. + + :returns: If ssl_module is included and active in Apache + :rtype: bool + + """ + try: + proc = subprocess.Popen( + [self.conf("ctl"), "-M"], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + stdout, stderr = proc.communicate() + + except (OSError, ValueError): + logger.error( + "Error accessing %s for loaded modules!", self.conf("ctl")) + raise errors.MisconfigurationError("Error accessing loaded modules") + # Small errors that do not impede + if proc.returncode != 0: + logger.warn("Error in checking loaded module list: %s", stderr) + raise errors.MisconfigurationError( + "Apache is unable to check whether or not the module is " + "loaded because Apache is misconfigured.") + + if module in stdout: + return True + return False + def restart(self): """Restarts apache server. @@ -911,7 +980,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :rtype: bool """ - return apache_restart(self.conf('init-script')) + return apache_restart(self.conf("init-script")) def config_test(self): # pylint: disable=no-self-use """Check the configuration of Apache for errors. @@ -922,13 +991,13 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ try: proc = subprocess.Popen( - [self.conf('ctl'), "configtest"], + [self.conf("ctl"), "configtest"], stdout=subprocess.PIPE, stderr=subprocess.PIPE) stdout, stderr = proc.communicate() except (OSError, ValueError): logger.fatal("Unable to run /usr/sbin/apache2ctl configtest") - sys.exit(1) + raise errors.PluginError("Unable to run apache2ctl") if proc.returncode != 0: # Enter recovery routine... @@ -961,24 +1030,24 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :returns: version :rtype: tuple - :raises .ConfiguratorError: if unable to find Apache version + :raises .PluginError: if unable to find Apache version """ try: proc = subprocess.Popen( - [self.conf('ctl'), "-v"], + [self.conf("ctl"), "-v"], stdout=subprocess.PIPE, stderr=subprocess.PIPE) text = proc.communicate()[0] except (OSError, ValueError): - raise errors.ConfiguratorError( - "Unable to run %s -v" % self.conf('ctl')) + raise errors.PluginError( + "Unable to run %s -v" % self.conf("ctl")) regex = re.compile(r"Apache/([0-9\.]*)", re.IGNORECASE) matches = regex.findall(text) if len(matches) != 1: - raise errors.ConfiguratorError("Unable to find Apache version") + raise errors.PluginError("Unable to find Apache version") return tuple([int(i) for i in matches[0].split(".")]) @@ -1043,63 +1112,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.restart() -def enable_mod(mod_name, apache_init_script, apache_enmod): - """Enables module in Apache. - - Both enables and restarts Apache so module is active. - - :param str mod_name: Name of the module to enable. - :param str apache_init_script: Path to the Apache init script. - :param str apache_enmod: Path to the Apache a2enmod script. - - """ - try: - # Use check_output so the command will finish before reloading - # TODO: a2enmod is debian specific... - subprocess.check_call([apache_enmod, mod_name], - stdout=open("/dev/null", "w"), - stderr=open("/dev/null", "w")) - apache_restart(apache_init_script) - except (OSError, subprocess.CalledProcessError): - logger.exception("Error enabling mod_%s", mod_name) - sys.exit(1) - - -def mod_loaded(module, apache_ctl): - """Checks to see if mod_ssl is loaded - - Uses ``apache_ctl`` to get loaded module list. This also effectively - serves as a config_test. - - :param str apache_ctl: Path to apache2ctl binary. - - :returns: If ssl_module is included and active in Apache - :rtype: bool - - """ - try: - proc = subprocess.Popen( - [apache_ctl, "-M"], - stdout=subprocess.PIPE, - stderr=subprocess.PIPE) - stdout, stderr = proc.communicate() - - except (OSError, ValueError): - logger.error( - "Error accessing %s for loaded modules!", apache_ctl) - raise errors.ConfiguratorError("Error accessing loaded modules") - # Small errors that do not impede - if proc.returncode != 0: - logger.warn("Error in checking loaded module list: %s", stderr) - raise errors.MisconfigurationError( - "Apache is unable to check whether or not the module is " - "loaded because Apache is misconfigured.") - - if module in stdout: - return True - return False - - def apache_restart(apache_init_script): """Restarts the Apache Server. @@ -1129,7 +1141,7 @@ def apache_restart(apache_init_script): except (OSError, ValueError): logger.fatal( "Apache Restart Failed - Please Check the Configuration") - sys.exit(1) + raise errors.MisconfigurationError("Unable to restart Apache process") return True diff --git a/letsencrypt_apache/display_ops.py b/letsencrypt_apache/display_ops.py new file mode 100644 index 000000000..352845376 --- /dev/null +++ b/letsencrypt_apache/display_ops.py @@ -0,0 +1,94 @@ +"""Contains UI methods for Apache operations.""" +import logging +import os + +import zope.component + +from letsencrypt import interfaces + +import letsencrypt.display.util as display_util + + +logger = logging.getLogger(__name__) + + +def select_vhost(domain, vhosts): + """Select an appropriate Apache Vhost. + + :param vhosts: Available Apache Virtual Hosts + :type vhosts: :class:`list` of type `~obj.Vhost` + + :returns: VirtualHost or `None` + :rtype: `~obj.Vhost` or `None` + + """ + if not vhosts: + return None + while True: + code, tag = _vhost_menu(domain, vhosts) + if code == display_util.HELP: + _more_info_vhost(vhosts[tag]) + elif code == display_util.OK: + return vhosts[tag] + else: + return None + + +def _vhost_menu(domain, vhosts): + """Select an appropriate Apache Vhost. + + :param vhosts: Available Apache Virtual Hosts + :type vhosts: :class:`list` of type `~obj.Vhost` + + :returns: Display tuple - ('code', tag') + :rtype: `tuple` + + """ + # Free characters in the line of display text (9 is for ' | ' formatting) + free_chars = display_util.WIDTH - len("HTTPS") - len("Enabled") - 9 + + if free_chars < 2: + logger.debug("Display size is too small for " + "letsencrypt_apache.display_ops._vhost_menu()") + # This runs the edge off the screen, but it doesn't cause an "error" + filename_size = 1 + disp_name_size = 1 + else: + # Filename is a bit more important and probably longer with 000-* + filename_size = int(free_chars * .6) + disp_name_size = free_chars - filename_size + + choices = [] + for vhost in vhosts: + if len(vhost.names) == 1: + disp_name = next(iter(vhost.names)) + elif len(vhost.names) == 0: + disp_name = "" + else: + disp_name = "Multiple Names" + + choices.append( + "{fn:{fn_size}s} | {name:{name_size}s} | {https:5s} | " + "{active:7s}".format( + fn=os.path.basename(vhost.filep)[:filename_size], + name=disp_name[:disp_name_size], + https="HTTPS" if vhost.ssl else "", + active="Enabled" if vhost.enabled else "", + fn_size=filename_size, + name_size=disp_name_size) + ) + + code, tag = zope.component.getUtility(interfaces.IDisplay).menu( + "We were unable to find a vhost with a ServerName or Address of {0}.{1}" + "Which virtual host would you like to choose?".format( + domain, os.linesep), + choices, help_label="More Info", ok_label="Select") + + return code, tag + + +def _more_info_vhost(vhost): + zope.component.getUtility(interfaces.IDisplay).notification( + "Virtual Host Information:{0}{1}{0}{2}".format( + os.linesep, "-" * (display_util.WIDTH - 4), str(vhost)), + height=display_util.HEIGHT) diff --git a/letsencrypt_apache/dvsni.py b/letsencrypt_apache/dvsni.py index 5a9f8bc77..2542b242f 100644 --- a/letsencrypt_apache/dvsni.py +++ b/letsencrypt_apache/dvsni.py @@ -1,5 +1,4 @@ """ApacheDVSNI""" -import logging import os from letsencrypt.plugins import common @@ -7,9 +6,6 @@ from letsencrypt.plugins import common from letsencrypt_apache import parser -logger = logging.getLogger(__name__) - - class ApacheDvsni(common.Dvsni): """Class performs DVSNI challenges within the Apache configurator. @@ -60,12 +56,6 @@ class ApacheDvsni(common.Dvsni): default_addr = "*:443" for achall in self.achalls: vhost = self.configurator.choose_vhost(achall.domain) - if vhost is None: - logger.error( - "No vhost exists with servername or alias of: %s. " - "No _default_:443 vhost exists. Please specify servernames " - "in the Apache config", achall.domain) - return None # TODO - @jdkasten review this code to make sure it makes sense self.configurator.make_server_sni_ready(vhost, default_addr) diff --git a/letsencrypt_apache/obj.py b/letsencrypt_apache/obj.py index fecf46ff9..13e00edd8 100644 --- a/letsencrypt_apache/obj.py +++ b/letsencrypt_apache/obj.py @@ -15,7 +15,6 @@ class VirtualHost(object): # pylint: disable=too-few-public-methods :ivar bool enabled: Virtual host is enabled """ - def __init__(self, filep, path, addrs, ssl, enabled, names=None): # pylint: disable=too-many-arguments """Initialize a VH.""" @@ -31,14 +30,19 @@ class VirtualHost(object): # pylint: disable=too-few-public-methods self.names.add(name) def __str__(self): - addr_str = ", ".join(str(addr) for addr in self.addrs) - return ("file: %s\n" - "vh_path: %s\n" - "addrs: %s\n" - "names: %s\n" - "ssl: %s\n" - "enabled: %s" % (self.filep, self.path, addr_str, - self.names, self.ssl, self.enabled)) + return ( + "File: {filename}\n" + "Vhost path: {vhpath}\n" + "Addresses: {addrs}\n" + "Names: {names}\n" + "TLS Enabled: {tls}\n" + "Site Enabled: {active}".format( + filename=self.filep, + vhpath=self.path, + addrs=", ".join(str(addr) for addr in self.addrs), + names=", ".join(name for name in self.names), + tls="Yes" if self.ssl else "No", + active="Yes" if self.enabled else "No")) def __eq__(self, other): if isinstance(other, self.__class__): diff --git a/letsencrypt_apache/tests/configurator_test.py b/letsencrypt_apache/tests/configurator_test.py index cfd8c0574..9304b634f 100644 --- a/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt_apache/tests/configurator_test.py @@ -28,7 +28,7 @@ class TwoVhost80Test(util.ApacheTest): def setUp(self): super(TwoVhost80Test, self).setUp() - with mock.patch("letsencrypt_apache.configurator." + with mock.patch("letsencrypt_apache.configurator.ApacheConfigurator." "mod_loaded") as mock_load: mock_load.return_value = True self.config = util.get_apache_configurator( @@ -196,14 +196,14 @@ class TwoVhost80Test(util.ApacheTest): mock_popen().communicate.return_value = ( "Server Version: Apache (Debian)", "") - self.assertRaises(errors.ConfiguratorError, self.config.get_version) + self.assertRaises(errors.PluginError, self.config.get_version) mock_popen().communicate.return_value = ( "Server Version: Apache/2.3{0} Apache/2.4.7".format(os.linesep), "") - self.assertRaises(errors.ConfiguratorError, self.config.get_version) + self.assertRaises(errors.PluginError, self.config.get_version) mock_popen.side_effect = OSError("Can't find program") - self.assertRaises(errors.ConfiguratorError, self.config.get_version) + self.assertRaises(errors.PluginError, self.config.get_version) if __name__ == "__main__": diff --git a/letsencrypt_apache/tests/display_ops_test.py b/letsencrypt_apache/tests/display_ops_test.py new file mode 100644 index 000000000..ae7cb9b73 --- /dev/null +++ b/letsencrypt_apache/tests/display_ops_test.py @@ -0,0 +1,58 @@ +"""Test letsencrypt_apache.display_ops.""" +import sys +import unittest + +import mock +import zope.component + +from letsencrypt_apache.tests import util + +from letsencrypt.display import util as display_util + + +class SelectVhostTest(unittest.TestCase): + """Tests for letsencrypt_apache.display_ops.select_vhost.""" + + def setUp(self): + zope.component.provideUtility(display_util.FileDisplay(sys.stdout)) + self.base_dir = "/example_path" + self.vhosts = util.get_vh_truth( + self.base_dir, "debian_apache_2_4/two_vhost_80") + + @classmethod + def _call(cls, vhosts): + from letsencrypt_apache.display_ops import select_vhost + return select_vhost("example.com", vhosts) + + @mock.patch("letsencrypt_apache.display_ops.zope.component.getUtility") + def test_successful_choice(self, mock_util): + mock_util().menu.return_value = (display_util.OK, 3) + self.assertEqual(self.vhosts[3], self._call(self.vhosts)) + + @mock.patch("letsencrypt_apache.display_ops.zope.component.getUtility") + def test_more_info_cancel(self, mock_util): + mock_util().menu.side_effect = [ + (display_util.HELP, 1), + (display_util.HELP, 0), + (display_util.CANCEL, -1), + ] + + self.assertEqual(None, self._call(self.vhosts)) + self.assertEqual(mock_util().notification.call_count, 2) + + def test_no_vhosts(self): + self.assertEqual(self._call([]), None) + + @mock.patch("letsencrypt_apache.display_ops.display_util") + @mock.patch("letsencrypt_apache.display_ops.zope.component.getUtility") + @mock.patch("letsencrypt_apache.display_ops.logging") + def test_small_display(self, mock_logging, mock_util, mock_display_util): + mock_display_util.WIDTH = 20 + mock_util().menu.return_value = (display_util.OK, 0) + self._call(self.vhosts) + + self.assertTrue(mock_logging.is_called) + + +if __name__ == "__main__": + unittest.main() # pragma: no cover diff --git a/letsencrypt_apache/tests/dvsni_test.py b/letsencrypt_apache/tests/dvsni_test.py index 27e9b2584..933656e94 100644 --- a/letsencrypt_apache/tests/dvsni_test.py +++ b/letsencrypt_apache/tests/dvsni_test.py @@ -20,7 +20,7 @@ class DvsniPerformTest(util.ApacheTest): def setUp(self): super(DvsniPerformTest, self).setUp() - with mock.patch("letsencrypt_apache.configurator." + with mock.patch("letsencrypt_apache.configurator.ApacheConfigurator." "mod_loaded") as mock_load: mock_load.return_value = True config = util.get_apache_configurator( diff --git a/letsencrypt_apache/tests/parser_test.py b/letsencrypt_apache/tests/parser_test.py index 85cc8abbd..3d5e80362 100644 --- a/letsencrypt_apache/tests/parser_test.py +++ b/letsencrypt_apache/tests/parser_test.py @@ -112,7 +112,7 @@ class ApacheParserTest(util.ApacheTest): mock_path.isfile.return_value = False # pylint: disable=protected-access - self.assertRaises(errors.ConfiguratorError, + self.assertRaises(errors.PluginError, self.parser._set_locations, self.ssl_options) mock_path.isfile.side_effect = [True, False, False] diff --git a/letsencrypt_nginx/configurator.py b/letsencrypt_nginx/configurator.py index e86e58d30..b1dfdca31 100644 --- a/letsencrypt_nginx/configurator.py +++ b/letsencrypt_nginx/configurator.py @@ -320,9 +320,9 @@ class NginxConfigurator(common.Plugin): return self._enhance_func[enhancement]( self.choose_vhost(domain), options) except (KeyError, ValueError): - raise errors.ConfiguratorError( + raise errors.PluginError( "Unsupported enhancement: {0}".format(enhancement)) - except errors.ConfiguratorError: + except errors.PluginError: logger.warn("Failed %s for %s", enhancement, domain) ###################################### @@ -385,7 +385,7 @@ class NginxConfigurator(common.Plugin): :returns: version :rtype: tuple - :raises .ConfiguratorError: + :raises .PluginError: Unable to find Nginx version or version is unsupported """ @@ -396,7 +396,7 @@ class NginxConfigurator(common.Plugin): stderr=subprocess.PIPE) text = proc.communicate()[1] # nginx prints output to stderr except (OSError, ValueError): - raise errors.ConfiguratorError( + raise errors.PluginError( "Unable to run %s -V" % self.conf('ctl')) version_regex = re.compile(r"nginx/([0-9\.]*)", re.IGNORECASE) @@ -409,19 +409,19 @@ class NginxConfigurator(common.Plugin): ssl_matches = ssl_regex.findall(text) if not version_matches: - raise errors.ConfiguratorError("Unable to find Nginx version") + raise errors.PluginError("Unable to find Nginx version") if not ssl_matches: - raise errors.ConfiguratorError( + raise errors.PluginError( "Nginx build is missing SSL module (--with-http_ssl_module).") if not sni_matches: - raise errors.ConfiguratorError("Nginx build doesn't support SNI") + raise errors.PluginError("Nginx build doesn't support SNI") nginx_version = tuple([int(i) for i in version_matches[0].split(".")]) # nginx < 0.8.48 uses machine hostname as default server_name instead of # the empty string if nginx_version < (0, 8, 48): - raise errors.ConfiguratorError("Nginx version must be 0.8.48+") + raise errors.PluginError("Nginx version must be 0.8.48+") return nginx_version diff --git a/letsencrypt_nginx/tests/configurator_test.py b/letsencrypt_nginx/tests/configurator_test.py index e134605e9..83085cc9f 100644 --- a/letsencrypt_nginx/tests/configurator_test.py +++ b/letsencrypt_nginx/tests/configurator_test.py @@ -45,7 +45,7 @@ class NginxConfiguratorTest(util.NginxTest): def test_enhance(self): self.assertRaises( - errors.ConfiguratorError, self.config.enhance, 'myhost', 'redirect') + errors.PluginError, self.config.enhance, 'myhost', 'redirect') def test_get_chall_pref(self): self.assertEqual([challenges.DVSNI], @@ -215,19 +215,19 @@ class NginxConfiguratorTest(util.NginxTest): " (based on LLVM 3.5svn)", "TLS SNI support enabled", "configure arguments: --with-http_ssl_module"])) - self.assertRaises(errors.ConfiguratorError, self.config.get_version) + self.assertRaises(errors.PluginError, self.config.get_version) mock_popen().communicate.return_value = ( "", "\n".join(["nginx version: nginx/1.4.2", "TLS SNI support enabled"])) - self.assertRaises(errors.ConfiguratorError, self.config.get_version) + self.assertRaises(errors.PluginError, self.config.get_version) mock_popen().communicate.return_value = ( "", "\n".join(["nginx version: nginx/1.4.2", "built by clang 6.0 (clang-600.0.56)" " (based on LLVM 3.5svn)", "configure arguments: --with-http_ssl_module"])) - self.assertRaises(errors.ConfiguratorError, self.config.get_version) + self.assertRaises(errors.PluginError, self.config.get_version) mock_popen().communicate.return_value = ( "", "\n".join(["nginx version: nginx/0.8.1", @@ -235,10 +235,10 @@ class NginxConfiguratorTest(util.NginxTest): " (based on LLVM 3.5svn)", "TLS SNI support enabled", "configure arguments: --with-http_ssl_module"])) - self.assertRaises(errors.ConfiguratorError, self.config.get_version) + self.assertRaises(errors.PluginError, self.config.get_version) mock_popen.side_effect = OSError("Can't find program") - self.assertRaises(errors.ConfiguratorError, self.config.get_version) + self.assertRaises(errors.PluginError, self.config.get_version) @mock.patch("letsencrypt_nginx.configurator.subprocess.Popen") def test_nginx_restart(self, mock_popen):