Import order

certbot-apache/certbot_apache/apache_util.py

@@ -0,0 +1,75 @@
+""" Utility functions for certbot-apache plugin """
+import os
+
+def get_mod_deps(mod_name):
+    """Get known module dependencies.
+
+    .. note:: This does not need to be accurate in order for the client to
+        run.  This simply keeps things clean if the user decides to revert
+        changes.
+    .. warning:: If all deps are not included, it may cause incorrect parsing
+        behavior, due to enable_mod's shortcut for updating the parser's
+        currently defined modules (`.ApacheConfigurator._add_parser_mod`)
+        This would only present a major problem in extremely atypical
+        configs that use ifmod for the missing deps.
+
+    """
+    deps = {
+        "ssl": ["setenvif", "mime"]
+    }
+    return deps.get(mod_name, [])
+
+
+def get_file_path(vhost_path):
+    """Get file path from augeas_vhost_path.
+
+    Takes in Augeas path and returns the file name
+
+    :param str vhost_path: Augeas virtual host path
+
+    :returns: filename of vhost
+    :rtype: str
+
+    """
+    if not vhost_path or not vhost_path.startswith("/files/"):
+        return None
+
+    return _split_aug_path(vhost_path)[0]
+
+
+def get_internal_aug_path(vhost_path):
+    """Get the Augeas path for a vhost with the file path removed.
+
+    :param str vhost_path: Augeas virtual host path
+
+    :returns: Augeas path to vhost relative to the containing file
+    :rtype: str
+
+    """
+    return _split_aug_path(vhost_path)[1]
+
+
+def _split_aug_path(vhost_path):
+    """Splits an Augeas path into a file path and an internal path.
+
+    After removing "/files", this function splits vhost_path into the
+    file path and the remaining Augeas path.
+
+    :param str vhost_path: Augeas virtual host path
+
+    :returns: file path and internal Augeas path
+    :rtype: `tuple` of `str`
+
+    """
+    # Strip off /files
+    file_path = vhost_path[6:]
+    internal_path = []
+
+    # Remove components from the end of file_path until it becomes valid
+    while not os.path.exists(file_path):
+        file_path, _, internal_path_part = file_path.rpartition("/")
+        internal_path.append(internal_path_part)
+
+    return file_path, "/".join(reversed(internal_path))
+
+

certbot-apache/certbot_apache/configurator.py

@@ -19,6 +19,7 @@ from certbot import util
 from certbot.plugins import common
 from certbot.plugins.util import path_surgery
 
+from certbot_apache import apache_util
 from certbot_apache import augeas_configurator
 from certbot_apache import constants
 from certbot_apache import display_ops
@@ -122,6 +123,11 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         version = kwargs.pop("version", None)
         super(ApacheConfigurator, self).__init__(*args, **kwargs)
 
+        # Detect OS and store distro based override class here to avoid
+        # calling the detection code every time we need something. Also provides
+        # access to the configurator object for the override class.
+        self.os_info = constants.get_override(self)
+
         # Add name_server association dict
         self.assoc = dict()
         # Outstanding challenges
@@ -205,7 +211,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         # Get all of the available vhosts
         self.vhosts = self.get_virtual_hosts()
 
-        install_ssl_options_conf(self.mod_ssl_conf, self.updated_mod_ssl_conf_digest)
+        constants.install_ssl_options_conf(self.mod_ssl_conf,
+                                           self.updated_mod_ssl_conf_digest)
 
         # Prevent two Apache plugins from modifying a config at once
         try:
@@ -585,7 +592,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
             if addr.get_port() == "443":
                 is_ssl = True
 
-        filename = get_file_path(self.aug.get("/augeas/files%s/path" % get_file_path(path)))
+        filename = apache_util.get_file_path(
+            self.aug.get("/augeas/files%s/path" % apache_util.get_file_path(path)))
         if filename is None:
             return None
 
@@ -624,7 +632,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                 new_vhost = self._create_vhost(path)
                 if not new_vhost:
                     continue
-                internal_path = get_internal_aug_path(new_vhost.path)
+                internal_path = apache_util.get_internal_aug_path(new_vhost.path)
                 realpath = os.path.realpath(new_vhost.filep)
                 if realpath not in file_paths:
                     file_paths[realpath] = new_vhost.filep
@@ -640,7 +648,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                     for v in vhs:
                         if v.filep == file_paths[realpath]:
                             internal_paths[realpath].remove(
-                                get_internal_aug_path(v.path))
+                                apache_util.get_internal_aug_path(v.path))
                         else:
                             new_vhs.append(v)
                     vhs = new_vhs
@@ -1702,17 +1710,14 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
 
         return redirects
 
-
+    @constants.override
     def enable_site(self, vhost):
         """Enables an available site, Apache reload required.
 
         .. note:: Does not make sure that the site correctly works or that all
                   modules are enabled appropriately.
-
+        .. note:: The distribution specific override replaces functionality
-        .. todo:: This function should number subdomains before the domain
+                  of this method where available.
-                  vhost
-
-        .. todo:: Make sure link is not broken...
 
         :param vhost: vhost to enable
         :type vhost: :class:`~certbot_apache.obj.VirtualHost`
@@ -1724,39 +1729,17 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         if vhost.enabled:
             return
 
-        # Handle non-debian systems
+        if not self.parser.parsed_in_original(vhost.filep):
-        if not self.conf("handle-sites"):
+            # Add direct include to root conf
-            if not self.parser.parsed_in_original(vhost.filep):
+            logger.info("Enabling site %s by adding Include to root configuration",
-                # Add direct include to root conf
+                        vhost.filep)
-                self.parser.add_include(self.parser.loc["default"], vhost.filep)
+            self.save_notes += "Enabled site %s\n" % vhost.filep
-                vhost.enabled = True
+            self.parser.add_include(self.parser.loc["default"], vhost.filep)
-            return
+            vhost.enabled = True
+        return
 
-        enabled_path = ("%s/sites-enabled/%s" %
+    @constants.override
-                        (self.parser.root, os.path.basename(vhost.filep)))
+    def enable_mod(self, mod_name, temp=False): # pylint: disable=unused-argument
-        self.reverter.register_file_creation(False, enabled_path)
-        try:
-            os.symlink(vhost.filep, enabled_path)
-        except OSError as err:
-            if os.path.islink(enabled_path) and os.path.realpath(
-               enabled_path) == vhost.filep:
-                # Already in shape
-                vhost.enabled = True
-                return
-            else:
-                logger.warning(
-                    "Could not symlink %s to %s, got error: %s", enabled_path,
-                    vhost.filep, err.strerror)
-                errstring = ("Encountered error while trying to enable a " +
-                             "newly created VirtualHost located at {0} by " +
-                             "linking to it from {1}")
-                raise errors.NotSupportedError(errstring.format(vhost.filep,
-                                                                enabled_path))
-        vhost.enabled = True
-        logger.info("Enabling available site: %s", vhost.filep)
-        self.save_notes += "Enabled site %s\n" % vhost.filep
-
-    def enable_mod(self, mod_name, temp=False):
         """Enables module in Apache.
 
         Both enables and reloads Apache so module is active.
@@ -1764,65 +1747,26 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         :param str mod_name: Name of the module to enable. (e.g. 'ssl')
         :param bool temp: Whether or not this is a temporary action.
 
+        .. note:: The distribution specific override replaces functionality
+                  of this method where available.
+
         :raises .errors.NotSupportedError: If the filesystem layout is not
             supported.
         :raises .errors.MisconfigurationError: If a2enmod or a2dismod cannot be
             run.
 
         """
-        # Support Debian specific setup
+        mod_message = ("Apache needs to have module  \"{0}\" active for the " +
-        avail_path = os.path.join(self.parser.root, "mods-available")
+            "requested installation options. Unfortunately Certbot is unable " +
-        enabled_path = os.path.join(self.parser.root, "mods-enabled")
+            "to install or enable it for you. Please install the module, and " +
-        if not os.path.isdir(avail_path) or not os.path.isdir(enabled_path):
+            "run Certbot again.")
-            raise errors.NotSupportedError(
+        raise errors.MisconfigurationError(mod_message.format(mod_name))
-                "Unsupported directory layout. You may try to enable mod %s "
-                "and try again." % mod_name)
 
-        deps = _get_mod_deps(mod_name)
+    def add_parser_mod(self, mod_name):
-
-        # Enable all dependencies
-        for dep in deps:
-            if (dep + "_module") not in self.parser.modules:
-                self._enable_mod_debian(dep, temp)
-                self._add_parser_mod(dep)
-
-                note = "Enabled dependency of %s module - %s" % (mod_name, dep)
-                if not temp:
-                    self.save_notes += note + os.linesep
-                logger.debug(note)
-
-        # Enable actual module
-        self._enable_mod_debian(mod_name, temp)
-        self._add_parser_mod(mod_name)
-
-        if not temp:
-            self.save_notes += "Enabled %s module in Apache\n" % mod_name
-        logger.info("Enabled Apache %s module", mod_name)
-
-        # Modules can enable additional config files. Variables may be defined
-        # within these new configuration sections.
-        # Reload is not necessary as DUMP_RUN_CFG uses latest config.
-        self.parser.update_runtime_variables()
-
-    def _add_parser_mod(self, mod_name):
         """Shortcut for updating parser modules."""
         self.parser.modules.add(mod_name + "_module")
         self.parser.modules.add("mod_" + mod_name + ".c")
 
-    def _enable_mod_debian(self, mod_name, temp):
-        """Assumes mods-available, mods-enabled layout."""
-        # Generate reversal command.
-        # Try to be safe here... check that we can probably reverse before
-        # applying enmod command
-        if not util.exe_exists(self.conf("dismod")):
-            raise errors.MisconfigurationError(
-                "Unable to find a2dismod, please make sure a2enmod and "
-                "a2dismod are configured correctly for certbot.")
-
-        self.reverter.register_undo_command(
-            temp, [self.conf("dismod"), mod_name])
-        util.run_script([self.conf("enmod"), mod_name])
-
     def restart(self):
         """Runs a config test and reloads the Apache server.
 
@@ -1945,84 +1889,3 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
             self.restart()
             self.parser.init_modules()
 
-
-def _get_mod_deps(mod_name):
-    """Get known module dependencies.
-
-    .. note:: This does not need to be accurate in order for the client to
-        run.  This simply keeps things clean if the user decides to revert
-        changes.
-    .. warning:: If all deps are not included, it may cause incorrect parsing
-        behavior, due to enable_mod's shortcut for updating the parser's
-        currently defined modules (`.ApacheConfigurator._add_parser_mod`)
-        This would only present a major problem in extremely atypical
-        configs that use ifmod for the missing deps.
-
-    """
-    deps = {
-        "ssl": ["setenvif", "mime"]
-    }
-    return deps.get(mod_name, [])
-
-
-def get_file_path(vhost_path):
-    """Get file path from augeas_vhost_path.
-
-    Takes in Augeas path and returns the file name
-
-    :param str vhost_path: Augeas virtual host path
-
-    :returns: filename of vhost
-    :rtype: str
-
-    """
-    if not vhost_path or not vhost_path.startswith("/files/"):
-        return None
-
-    return _split_aug_path(vhost_path)[0]
-
-
-def get_internal_aug_path(vhost_path):
-    """Get the Augeas path for a vhost with the file path removed.
-
-    :param str vhost_path: Augeas virtual host path
-
-    :returns: Augeas path to vhost relative to the containing file
-    :rtype: str
-
-    """
-    return _split_aug_path(vhost_path)[1]
-
-
-def _split_aug_path(vhost_path):
-    """Splits an Augeas path into a file path and an internal path.
-
-    After removing "/files", this function splits vhost_path into the
-    file path and the remaining Augeas path.
-
-    :param str vhost_path: Augeas virtual host path
-
-    :returns: file path and internal Augeas path
-    :rtype: `tuple` of `str`
-
-    """
-    # Strip off /files
-    file_path = vhost_path[6:]
-    internal_path = []
-
-    # Remove components from the end of file_path until it becomes valid
-    while not os.path.exists(file_path):
-        file_path, _, internal_path_part = file_path.rpartition("/")
-        internal_path.append(internal_path_part)
-
-    return file_path, "/".join(reversed(internal_path))
-
-
-def install_ssl_options_conf(options_ssl, options_ssl_digest):
-    """Copy Certbot's SSL options file into the system's config dir if required."""
-
-    # XXX if we ever try to enforce a local privilege boundary (eg, running
-    # certbot for unprivileged users via setuid), this function will need
-    # to be modified.
-    return common.install_version_controlled_file(options_ssl, options_ssl_digest,
-        constants.os_constant("MOD_SSL_CONF_SRC"), constants.ALL_SSL_OPTIONS_HASHES)

certbot-apache/certbot_apache/constants.py

@@ -2,6 +2,11 @@
 import pkg_resources
 from certbot import util
 
+from certbot.plugins import common
+
+from certbot_apache import override_centos
+from certbot_apache import override_debian
+
 CLI_DEFAULTS_DEFAULT = dict(
     server_root="/etc/apache2",
     vhost_root="/etc/apache2/sites-available",
@@ -17,6 +22,7 @@ CLI_DEFAULTS_DEFAULT = dict(
     handle_mods=False,
     handle_sites=False,
     challenge_location="/etc/apache2",
+    override_class=None,
     MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
         "certbot_apache", "options-ssl-apache.conf")
 )
@@ -35,6 +41,7 @@ CLI_DEFAULTS_DEBIAN = dict(
     handle_mods=True,
     handle_sites=True,
     challenge_location="/etc/apache2",
+    override_class=override_debian.Override,
     MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
         "certbot_apache", "options-ssl-apache.conf")
 )
@@ -53,6 +60,7 @@ CLI_DEFAULTS_CENTOS = dict(
     handle_mods=False,
     handle_sites=False,
     challenge_location="/etc/httpd/conf.d",
+    override_class=override_centos.Override,
     MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
         "certbot_apache", "centos-options-ssl-apache.conf")
 )
@@ -71,6 +79,7 @@ CLI_DEFAULTS_GENTOO = dict(
     handle_mods=False,
     handle_sites=False,
     challenge_location="/etc/apache2/vhosts.d",
+    override_class=None,
     MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
         "certbot_apache", "options-ssl-apache.conf")
 )
@@ -89,6 +98,7 @@ CLI_DEFAULTS_DARWIN = dict(
     handle_mods=False,
     handle_sites=False,
     challenge_location="/etc/apache2/other",
+    override_class=None,
     MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
         "certbot_apache", "options-ssl-apache.conf")
 )
@@ -107,6 +117,7 @@ CLI_DEFAULTS_SUSE = dict(
     handle_mods=False,
     handle_sites=False,
     challenge_location="/etc/apache2/vhosts.d",
+    override_class=None,
     MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
         "certbot_apache", "options-ssl-apache.conf")
 )
@@ -125,6 +136,7 @@ CLI_DEFAULTS_ARCH = dict(
     handle_mods=False,
     handle_sites=False,
     challenge_location="/etc/httpd/conf",
+    override_class=None,
     MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
         "certbot_apache", "options-ssl-apache.conf")
 )
@@ -192,6 +204,14 @@ UIR_ARGS = ["always", "set", "Content-Security-Policy",
 HEADER_ARGS = {"Strict-Transport-Security": HSTS_ARGS,
                "Upgrade-Insecure-Requests": UIR_ARGS}
 
+def install_ssl_options_conf(options_ssl, options_ssl_digest):
+    """Copy Certbot's SSL options file into the system's config dir if required."""
+
+    # XXX if we ever try to enforce a local privilege boundary (eg, running
+    # certbot for unprivileged users via setuid), this function will need
+    # to be modified.
+    return common.install_version_controlled_file(options_ssl, options_ssl_digest,
+        os_constant("MOD_SSL_CONF_SRC"), ALL_SSL_OPTIONS_HASHES)
 
 def os_constant(key):
     """
@@ -227,3 +247,31 @@ def os_like_constants():
             if os_name in CLI_DEFAULTS.keys():
                 return CLI_DEFAULTS[os_name]
     return {}
+
+def get_override(caller):
+    """
+    Initialize the override class and pass the caller class to it
+
+    :param caller: `class` of caller
+    :return: Override class or `None` if not found
+    """
+    override_class = os_constant("override_class")
+    if override_class:
+        return override_class(caller)
+    return None
+
+def override(method):
+    """Decorator for ApacheConfigurator for distribution specific method
+    overrides."""
+    def override_args(caller_class, *args, **kwargs):
+        """Check if distro specific class overrides called method, return
+        overriding method if found, in other case, return the default"""
+        try:
+            # Try to find overriding method
+            caller = {"class": caller_class, "method": method}
+            return getattr(caller_class.os_info,
+                           method.__name__)(caller, *args, **kwargs)
+        except AttributeError:
+            # Override not found, return the default
+            return method(caller_class, *args, **kwargs)
+    return override_args

certbot-apache/certbot_apache/override_centos.py

@@ -0,0 +1,10 @@
+""" Distribution specific overrides for CentOS family (RHEL/Fedora/CentOS """
+class Override(object):
+    """CentOS override class"""
+    def __init__(self, config):
+        """
+        Initializes the override class.
+
+        :param config: caller `certbot_apache.configurator.ApacheConfigurator`
+        """
+        self.config = config

certbot-apache/certbot_apache/override_debian.py

@@ -0,0 +1,126 @@
+""" Distribution specific override class for Debian family (Ubuntu/Debian) """
+import logging
+import os
+
+from certbot import errors
+from certbot import util
+
+from certbot_apache import apache_util
+
+logger = logging.getLogger(__name__)
+
+class Override(object):
+    """ Debian override class """
+    def __init__(self, config):
+        """
+        Initializes the override class.
+
+        :param config: caller `certbot_apache.configurator.ApacheConfigurator`
+        """
+        self.config = config
+
+    def enable_site(self, caller, vhost):
+        """Enables an available site, Apache reload required.
+
+        .. note:: Does not make sure that the site correctly works or that all
+                  modules are enabled appropriately.
+
+        :param vhost: vhost to enable
+        :type vhost: :class:`~certbot_apache.obj.VirtualHost`
+
+        :raises .errors.NotSupportedError: If filesystem layout is not
+            supported.
+
+        """
+        if vhost.enabled:
+            return
+
+        enabled_path = ("%s/sites-enabled/%s" %
+                        (self.config.parser.root,
+                         os.path.basename(vhost.filep)))
+        if not os.path.isdir(os.path.dirname(enabled_path)):
+            # For some reason, sites-enabled / sites-available do not exist
+            # Call the original caller method
+            caller["method"](caller["class"], vhost)
+        self.config.reverter.register_file_creation(False, enabled_path)
+        try:
+            os.symlink(vhost.filep, enabled_path)
+        except OSError as err:
+            if os.path.islink(enabled_path) and os.path.realpath(
+               enabled_path) == vhost.filep:
+                # Already in shape
+                vhost.enabled = True
+                return
+            else:
+                logger.warning(
+                    "Could not symlink %s to %s, got error: %s", enabled_path,
+                    vhost.filep, err.strerror)
+                errstring = ("Encountered error while trying to enable a " +
+                             "newly created VirtualHost located at {0} by " +
+                             "linking to it from {1}")
+                raise errors.NotSupportedError(errstring.format(vhost.filep,
+                                                                enabled_path))
+        vhost.enabled = True
+        logger.info("Enabling available site: %s", vhost.filep)
+        self.config.save_notes += "Enabled site %s\n" % vhost.filep
+
+    def enable_mod(self, caller, mod_name, temp=False):
+        # pylint: disable=unused-argument
+        """Enables module in Apache.
+
+        Both enables and reloads Apache so module is active.
+
+        :param str mod_name: Name of the module to enable. (e.g. 'ssl')
+        :param bool temp: Whether or not this is a temporary action.
+
+        :raises .errors.NotSupportedError: If the filesystem layout is not
+            supported.
+        :raises .errors.MisconfigurationError: If a2enmod or a2dismod cannot be
+            run.
+
+        """
+        avail_path = os.path.join(self.config.parser.root, "mods-available")
+        enabled_path = os.path.join(self.config.parser.root, "mods-enabled")
+        if not os.path.isdir(avail_path) or not os.path.isdir(enabled_path):
+            raise errors.NotSupportedError(
+                "Unsupported directory layout. You may try to enable mod %s "
+                "and try again." % mod_name)
+
+        deps = apache_util.get_mod_deps(mod_name)
+
+        # Enable all dependencies
+        for dep in deps:
+            if (dep + "_module") not in self.config.parser.modules:
+                self._enable_mod_debian(dep, temp)
+                self.config.add_parser_mod(dep)
+                note = "Enabled dependency of %s module - %s" % (mod_name, dep)
+                if not temp:
+                    self.config.save_notes += note + os.linesep
+                logger.debug(note)
+
+        # Enable actual module
+        self._enable_mod_debian(mod_name, temp)
+        self.config.add_parser_mod(mod_name)
+
+        if not temp:
+            self.config.save_notes += "Enabled %s module in Apache\n" % mod_name
+        logger.info("Enabled Apache %s module", mod_name)
+
+        # Modules can enable additional config files. Variables may be defined
+        # within these new configuration sections.
+        # Reload is not necessary as DUMP_RUN_CFG uses latest config.
+        self.config.parser.update_runtime_variables()
+
+    def _enable_mod_debian(self, mod_name, temp):
+        """Assumes mods-available, mods-enabled layout."""
+        # Generate reversal command.
+        # Try to be safe here... check that we can probably reverse before
+        # applying enmod command
+        if not util.exe_exists(self.config.conf("dismod")):
+            raise errors.MisconfigurationError(
+                "Unable to find a2dismod, please make sure a2enmod and "
+                "a2dismod are configured correctly for certbot.")
+
+        self.config.reverter.register_undo_command(
+            temp, [self.config.conf("dismod"), mod_name])
+        util.run_script([self.config.conf("enmod"), mod_name])

certbot-apache/certbot_apache/parser.py

@@ -19,8 +19,6 @@ logger = logging.getLogger(__name__)
 class ApacheParser(object):
     """Class handles the fine details of parsing the Apache Configuration.
 
-    .. todo:: Make parsing general... remove sites-available etc...
-
     :ivar str root: Normalized absolute path to the server root
         directory. Without trailing slash.
     :ivar set modules: All module names that are currently enabled.

certbot-apache/certbot_apache/tests/configurator_test.py

@@ -19,10 +19,10 @@ from certbot import errors
 from certbot.tests import acme_util
 from certbot.tests import util as certbot_util
 
-from certbot_apache import configurator
 from certbot_apache import constants
 from certbot_apache import parser
 from certbot_apache import obj
+from certbot_apache import apache_util
 
 from certbot_apache.tests import util
 
@@ -41,6 +41,9 @@ class MultipleVhostsTest(util.ApacheTest):
             """Mock default vhost path"""
             if key == "vhost_root":
                 return vhost_path
+            elif key == "override_class":
+                from certbot_apache import override_debian
+                return override_debian.Override
             else:
                 return orig_os_constant(key)
 
@@ -164,13 +167,11 @@ class MultipleVhostsTest(util.ApacheTest):
         self.assertTrue("certbot.demo" in names)
 
     def test_get_bad_path(self):
-        from certbot_apache.configurator import get_file_path
+        self.assertEqual(apache_util.get_file_path(None), None)
-        self.assertEqual(get_file_path(None), None)
+        self.assertEqual(apache_util.get_file_path("nonexistent"), None)
-        self.assertEqual(get_file_path("nonexistent"), None)
         self.assertEqual(self.config._create_vhost("nonexistent"), None) # pylint: disable=protected-access
 
     def test_get_aug_internal_path(self):
-        from certbot_apache.configurator import get_internal_aug_path
         internal_paths = [
             "Virtualhost", "IfModule/VirtualHost", "VirtualHost", "VirtualHost",
             "Macro/VirtualHost", "IfModule/VirtualHost", "VirtualHost",
@@ -178,7 +179,8 @@ class MultipleVhostsTest(util.ApacheTest):
 
         for i, internal_path in enumerate(internal_paths):
             self.assertEqual(
-                get_internal_aug_path(self.vh_truth[i].path), internal_path)
+                apache_util.get_internal_aug_path(self.vh_truth[i].path),
+                internal_path)
 
     def test_bad_servername_alias(self):
         ssl_vh1 = obj.VirtualHost(
@@ -345,6 +347,14 @@ class MultipleVhostsTest(util.ApacheTest):
         self.assertRaises(
             errors.MisconfigurationError, self.config.enable_mod, "ssl")
 
+    def test_enable_mod_nonexistent(self):
+        from certbot_apache import override_centos
+        self.config.os_info = override_centos.Override(self.config)
+        self.assertRaises(
+            errors.MisconfigurationError,
+            self.config.enable_mod,
+            "whatever")
+
     def test_enable_site_already_enabled(self):
         self.assertTrue(self.vh_truth[1].enabled)
         self.config.enable_site(self.vh_truth[1])
@@ -356,29 +366,28 @@ class MultipleVhostsTest(util.ApacheTest):
             self.config.enable_site,
             obj.VirtualHost("asdf", "afsaf", set(), False, False))
 
+    def test_enable_site_nonexistent(self):
+        from certbot_apache import override_centos
+        self.config.os_info = override_centos.Override(self.config)
+        self.assertEqual(self.config.enable_site(self.vh_truth[1]), None)
+
     def test_enable_site_nondebian(self):
-        mock_c = "certbot_apache.configurator.ApacheConfigurator.conf"
+        from certbot_apache import override_centos
-        def conf_side_effect(arg):
+        self.config.os_info = override_centos.Override(self.config)
-            """ Mock function for ApacheConfigurator.conf """
-            confvars = {"handle-sites": False}
-            if arg in confvars:
-                return confvars[arg]
         inc_path = "/path/to/wherever"
         vhost = self.vh_truth[0]
-        with mock.patch(mock_c) as mock_conf:
+        vhost.enabled = False
-            mock_conf.side_effect = conf_side_effect
+        vhost.filep = inc_path
-            vhost.enabled = False
+        self.assertFalse(self.config.parser.find_dir("Include", inc_path))
-            vhost.filep = inc_path
+        self.assertFalse(
-            self.assertFalse(self.config.parser.find_dir("Include", inc_path))
+            os.path.dirname(inc_path) in self.config.parser.existing_paths)
-            self.assertFalse(
+        self.config.enable_site(vhost)
-                os.path.dirname(inc_path) in self.config.parser.existing_paths)
+        self.assertTrue(self.config.parser.find_dir("Include", inc_path))
-            self.config.enable_site(vhost)
+        self.assertTrue(
-            self.assertTrue(self.config.parser.find_dir("Include", inc_path))
+            os.path.dirname(inc_path) in self.config.parser.existing_paths)
-            self.assertTrue(
+        self.assertTrue(
-                os.path.dirname(inc_path) in self.config.parser.existing_paths)
+            os.path.basename(inc_path) in self.config.parser.existing_paths[
-            self.assertTrue(
+                os.path.dirname(inc_path)])
-                os.path.basename(inc_path) in self.config.parser.existing_paths[
-                    os.path.dirname(inc_path)])
 
     def test_deploy_cert_enable_new_vhost(self):
         # Create
@@ -427,12 +436,12 @@ class MultipleVhostsTest(util.ApacheTest):
         # Verify one directive was found in the correct file
         self.assertEqual(len(loc_cert), 1)
         self.assertEqual(
-            configurator.get_file_path(loc_cert[0]),
+            apache_util.get_file_path(loc_cert[0]),
             self.vh_truth[1].filep)
 
         self.assertEqual(len(loc_key), 1)
         self.assertEqual(
-            configurator.get_file_path(loc_key[0]),
+            apache_util.get_file_path(loc_key[0]),
             self.vh_truth[1].filep)
 
     def test_deploy_cert_newssl_no_fullchain(self):
@@ -469,25 +478,16 @@ class MultipleVhostsTest(util.ApacheTest):
 
     def test_deploy_cert_not_parsed_path(self):
         # Make sure that we add include to root config for vhosts when
-        # handle-sites is false
+        # running Debian, but missing sites-enabled structure
         self.config.parser.modules.add("ssl_module")
         self.config.parser.modules.add("mod_ssl.c")
         tmp_path = os.path.realpath(tempfile.mkdtemp("vhostroot"))
         os.chmod(tmp_path, 0o755)
         mock_p = "certbot_apache.configurator.ApacheConfigurator._get_ssl_vhost_path"
         mock_a = "certbot_apache.parser.ApacheParser.add_include"
-        mock_c = "certbot_apache.configurator.ApacheConfigurator.conf"
-        orig_conf = self.config.conf
-        def conf_side_effect(arg):
-            """ Mock function for ApacheConfigurator.conf """
-            confvars = {"handle-sites": False}
-            if arg in confvars:
-                return confvars[arg]
-            else:
-                return orig_conf("arg")
 
-        with mock.patch(mock_c) as mock_conf:
+        with mock.patch("os.path.isdir") as mock_osp:
-            mock_conf.side_effect = conf_side_effect
+            mock_osp.return_value = False
             with mock.patch(mock_p) as mock_path:
                 mock_path.return_value = os.path.join(tmp_path, "whatever.conf")
                 with mock.patch(mock_a) as mock_add:
@@ -558,17 +558,17 @@ class MultipleVhostsTest(util.ApacheTest):
         # Verify one directive was found in the correct file
         self.assertEqual(len(loc_cert), 1)
         self.assertEqual(
-            configurator.get_file_path(loc_cert[0]),
+            apache_util.get_file_path(loc_cert[0]),
             self.vh_truth[1].filep)
 
         self.assertEqual(len(loc_key), 1)
         self.assertEqual(
-            configurator.get_file_path(loc_key[0]),
+            apache_util.get_file_path(loc_key[0]),
             self.vh_truth[1].filep)
 
         self.assertEqual(len(loc_chain), 1)
         self.assertEqual(
-            configurator.get_file_path(loc_chain[0]),
+            apache_util.get_file_path(loc_chain[0]),
             self.vh_truth[1].filep)
 
         # One more time for chain directive setting
@@ -952,7 +952,7 @@ class MultipleVhostsTest(util.ApacheTest):
         self.assertTrue(isinstance(self.config.get_chall_pref(""), list))
 
     def test_install_ssl_options_conf(self):
-        from certbot_apache.configurator import install_ssl_options_conf
+        from certbot_apache.constants import install_ssl_options_conf
         path = os.path.join(self.work_dir, "test_it")
         other_path = os.path.join(self.work_dir, "other_test_it")
         install_ssl_options_conf(path, other_path)
@@ -1571,7 +1571,7 @@ class InstallSslOptionsConfTest(util.ApacheTest):
             self.config_path, self.vhost_path, self.config_dir, self.work_dir)
 
     def _call(self):
-        from certbot_apache.configurator import install_ssl_options_conf
+        from certbot_apache.constants import install_ssl_options_conf
         install_ssl_options_conf(self.config.mod_ssl_conf, self.config.updated_mod_ssl_conf_digest)
 
     def _current_ssl_options_hash(self):

certbot-apache/certbot_apache/tests/constants_test.py

@@ -29,6 +29,7 @@ class ConstantsTest(unittest.TestCase):
         self.assertEqual(constants.os_constant("server_root"), "/etc/apache2")
         self.assertEqual(constants.os_constant("vhost_root"),
                          "/etc/apache2/sites-available")
+        self.assertEqual(constants.get_override(None), None)
 
     @mock.patch("certbot.util.get_systemd_os_like")
     @mock.patch("certbot.util.get_os_info")

certbot-apache/certbot_apache/tests/tls_sni_01_test.py

@@ -9,6 +9,7 @@ from certbot.plugins import common_test
 
 from certbot_apache import obj
 from certbot_apache.tests import util
+from certbot_apache import override_debian
 
 from six.moves import xrange  # pylint: disable=redefined-builtin, import-error
 
@@ -25,6 +26,7 @@ class TlsSniPerformTest(util.ApacheTest):
         config = util.get_apache_configurator(
             self.config_path, self.vhost_path, self.config_dir, self.work_dir)
         config.config.tls_sni_01_port = 443
+        config.os_info = override_debian.Override(config)
 
         from certbot_apache import tls_sni_01
         self.sni = tls_sni_01.ApacheTlsSni01(config)