Make Apache control script and binary paths configurable on command line (#6238)

This PR adds two new command line parameters, --apache-ctlpath and --apache-binpath both of which are used to construct commands that we shell out for. The way that we previously fetched values either from Certbot configuration object or the dictionary of distribution based constants is now also unified, and the active options are parsed in prepare() to make it easier to override needed values for the distributions needing this behavior. Fixes: #5338 * Added the command line options and parsing * Refactor existing code * Distro override updates * Handle vhost_root from cli * Fix compatibility tests * Add comment about changes to command line arguments * Check None properly * Made help texts consistent * Keep the old defaults * Move to shorter CLI parameter names * No need for specific bin path, nor apache_cmd anymore * Make sure that we use user provided vhost-root value * Fix alt restart commands in overrides * Fix version_cmd defaults in overrides * Fix comparison * Remove cruft, and use configuration object for parser parameter
2026-07-30 18:14:21 +02:00 · 2018-08-02 08:17:38 -07:00
parent f6219ddf19
commit 7bff0a02e4
16 changed files with 150 additions and 127 deletions
@@ -1,5 +1,6 @@
 """Apache Configuration based off of Augeas Configurator."""
 # pylint: disable=too-many-lines
+import copy
 import fnmatch
 import logging
 import os
@@ -97,48 +98,72 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
        vhost_root="/etc/apache2/sites-available",
        vhost_files="*",
        logs_root="/var/log/apache2",
+        ctl="apache2ctl",
        version_cmd=['apache2ctl', '-v'],
-        apache_cmd="apache2ctl",
        restart_cmd=['apache2ctl', 'graceful'],
        conftest_cmd=['apache2ctl', 'configtest'],
        enmod=None,
        dismod=None,
        le_vhost_ext="-le-ssl.conf",
-        handle_mods=False,
+        handle_modules=False,
        handle_sites=False,
        challenge_location="/etc/apache2",
        MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
            "certbot_apache", "options-ssl-apache.conf")
    )

-    def constant(self, key):
-        """Get constant for OS_DEFAULTS"""
-        return self.OS_DEFAULTS.get(key)
+    def option(self, key):
+        """Get a value from options"""
+        return self.options.get(key)
+
+    def _prepare_options(self):
+        """
+        Set the values possibly changed by command line parameters to
+        OS_DEFAULTS constant dictionary
+        """
+        opts = ["enmod", "dismod", "le_vhost_ext", "server_root", "vhost_root",
+                "logs_root", "challenge_location", "handle_modules", "handle_sites",
+                "ctl"]
+        for o in opts:
+            # Config options use dashes instead of underscores
+            if self.conf(o.replace("_", "-")) is not None:
+                self.options[o] = self.conf(o.replace("_", "-"))
+            else:
+                self.options[o] = self.OS_DEFAULTS[o]
+
+        # Special cases
+        self.options["version_cmd"][0] = self.option("ctl")
+        self.options["restart_cmd"][0] = self.option("ctl")
+        self.options["conftest_cmd"][0] = self.option("ctl")

    @classmethod
    def add_parser_arguments(cls, add):
+        # When adding, modifying or deleting command line arguments, be sure to
+        # include the changes in the list used in method _prepare_options() to
+        # ensure consistent behavior.
        add("enmod", default=cls.OS_DEFAULTS["enmod"],
-            help="Path to the Apache 'a2enmod' binary.")
+            help="Path to the Apache 'a2enmod' binary")
        add("dismod", default=cls.OS_DEFAULTS["dismod"],
-            help="Path to the Apache 'a2dismod' binary.")
+            help="Path to the Apache 'a2dismod' binary")
        add("le-vhost-ext", default=cls.OS_DEFAULTS["le_vhost_ext"],
-            help="SSL vhost configuration extension.")
+            help="SSL vhost configuration extension")
        add("server-root", default=cls.OS_DEFAULTS["server_root"],
-            help="Apache server root directory.")
+            help="Apache server root directory")
        add("vhost-root", default=None,
            help="Apache server VirtualHost configuration root")
        add("logs-root", default=cls.OS_DEFAULTS["logs_root"],
            help="Apache server logs directory")
        add("challenge-location",
            default=cls.OS_DEFAULTS["challenge_location"],
-            help="Directory path for challenge configuration.")
-        add("handle-modules", default=cls.OS_DEFAULTS["handle_mods"],
-            help="Let installer handle enabling required modules for you. " +
+            help="Directory path for challenge configuration")
+        add("handle-modules", default=cls.OS_DEFAULTS["handle_modules"],
+            help="Let installer handle enabling required modules for you " +
                 "(Only Ubuntu/Debian currently)")
        add("handle-sites", default=cls.OS_DEFAULTS["handle_sites"],
-            help="Let installer handle enabling sites for you. " +
+            help="Let installer handle enabling sites for you " +
                 "(Only Ubuntu/Debian currently)")
-        util.add_deprecated_argument(add, argument_name="ctl", nargs=1)
+        add("ctl", default=cls.OS_DEFAULTS["ctl"],
+            help="Full path to Apache control script")
        util.add_deprecated_argument(
            add, argument_name="init-script", nargs=1)

@@ -169,7 +194,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
        self.parser = None
        self.version = version
        self.vhosts = None
-        self.vhostroot = None
+        self.options = copy.deepcopy(self.OS_DEFAULTS)
        self._enhance_func = {"redirect": self._enable_redirect,
                              "ensure-http-header": self._set_http_header,
                              "staple-ocsp": self._enable_ocsp_stapling}
@@ -201,12 +226,10 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
        except ImportError:
            raise errors.NoInstallationError("Problem in Augeas installation")

+        self._prepare_options()
+
        # Verify Apache is installed
-        restart_cmd = self.constant("restart_cmd")[0]
-        if not util.exe_exists(restart_cmd):
-            if not path_surgery(restart_cmd):
-                raise errors.NoInstallationError(
-                    'Cannot find Apache control command {0}'.format(restart_cmd))
+        self._verify_exe_availability(self.option("ctl"))

        # Make sure configuration is valid
        self.config_test()
@@ -226,12 +249,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                "version 1.2.0 or higher, please make sure you have you have "
                "those installed.")

-        # Parse vhost-root if defined on cli
-        if not self.conf("vhost-root"):
-            self.vhostroot = self.constant("vhost_root")
-        else:
-            self.vhostroot = os.path.abspath(self.conf("vhost-root"))
-
        self.parser = self.get_parser()

        # Check for errors in parsing files with Augeas
@@ -245,13 +262,20 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):

        # Prevent two Apache plugins from modifying a config at once
        try:
-            util.lock_dir_until_exit(self.conf("server-root"))
+            util.lock_dir_until_exit(self.option("server_root"))
        except (OSError, errors.LockError):
            logger.debug("Encountered error:", exc_info=True)
            raise errors.PluginError(
-                "Unable to lock %s", self.conf("server-root"))
+                "Unable to lock %s", self.option("server_root"))
        self._prepared = True

+    def _verify_exe_availability(self, exe):
+        """Checks availability of Apache executable"""
+        if not util.exe_exists(exe):
+            if not path_surgery(exe):
+                raise errors.NoInstallationError(
+                    'Cannot find Apache executable {0}'.format(exe))
+
    def _check_aug_version(self):
        """ Checks that we have recent enough version of libaugeas.
        If augeas version is recent enough, it will support case insensitive
@@ -269,8 +293,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):

    def get_parser(self):
        """Initializes the ApacheParser"""
+        # If user provided vhost_root value in command line, use it
        return parser.ApacheParser(
-            self.aug, self.conf("server-root"), self.conf("vhost-root"),
+            self.aug, self.option("server_root"), self.conf("vhost-root"),
            self.version, configurator=self)

    def _wildcard_domain(self, domain):
@@ -1037,7 +1062,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
        :param boolean temp: If the change is temporary
        """

-        if self.conf("handle-modules"):
+        if self.option("handle_modules"):
            if self.version >= (2, 4) and ("socache_shmcb_module" not in
                                           self.parser.modules):
                self.enable_mod("socache_shmcb", temp=temp)
@@ -1066,7 +1091,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):

        Duplicates vhost and adds default ssl options
        New vhost will reside as (nonssl_vhost.path) +
-        ``self.constant("le_vhost_ext")``
+        ``self.option("le_vhost_ext")``

        .. note:: This function saves the configuration

@@ -1165,18 +1190,16 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
        """

        if self.conf("vhost-root") and os.path.exists(self.conf("vhost-root")):
-            # Defined by user on CLI
-
-            fp = os.path.join(os.path.realpath(self.vhostroot),
+            fp = os.path.join(os.path.realpath(self.option("vhost_root")),
                              os.path.basename(non_ssl_vh_fp))
        else:
            # Use non-ssl filepath
            fp = os.path.realpath(non_ssl_vh_fp)

        if fp.endswith(".conf"):
-            return fp[:-(len(".conf"))] + self.conf("le_vhost_ext")
+            return fp[:-(len(".conf"))] + self.option("le_vhost_ext")
        else:
-            return fp + self.conf("le_vhost_ext")
+            return fp + self.option("le_vhost_ext")

    def _sift_rewrite_rule(self, line):
        """Decides whether a line should be copied to a SSL vhost.
@@ -2025,7 +2048,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                            addr in self._get_proposed_addrs(ssl_vhost)),
                   servername, serveralias,
                   " ".join(rewrite_rule_args),
-                   self.conf("logs-root")))
+                   self.option("logs_root")))

    def _write_out_redirect(self, ssl_vhost, text):
        # This is the default name
@@ -2037,7 +2060,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
            if len(ssl_vhost.name) < (255 - (len(redirect_filename) + 1)):
                redirect_filename = "le-redirect-%s.conf" % ssl_vhost.name

-        redirect_filepath = os.path.join(self.vhostroot,
+        redirect_filepath = os.path.join(self.option("vhost_root"),
                                         redirect_filename)

        # Register the new file that will be created
@@ -2158,18 +2181,18 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
        """
        error = ""
        try:
-            util.run_script(self.constant("restart_cmd"))
+            util.run_script(self.option("restart_cmd"))
        except errors.SubprocessError as err:
            logger.info("Unable to restart apache using %s",
-                        self.constant("restart_cmd"))
-            alt_restart = self.constant("restart_cmd_alt")
+                        self.option("restart_cmd"))
+            alt_restart = self.option("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(
+                    util.run_script(self.option(
                        "restart_cmd_alt"))
                    return
                except errors.SubprocessError as secerr:
@@ -2185,7 +2208,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):

        """
        try:
-            util.run_script(self.constant("conftest_cmd"))
+            util.run_script(self.option("conftest_cmd"))
        except errors.SubprocessError as err:
            raise errors.MisconfigurationError(str(err))

@@ -2201,11 +2224,11 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):

        """
        try:
-            stdout, _ = util.run_script(self.constant("version_cmd"))
+            stdout, _ = util.run_script(self.option("version_cmd"))
        except errors.SubprocessError:
            raise errors.PluginError(
                "Unable to run %s -v" %
-                self.constant("version_cmd"))
+                self.option("version_cmd"))

        regex = re.compile(r"Apache/([0-9\.]*)", re.IGNORECASE)
        matches = regex.findall(stdout)
@@ -2295,7 +2318,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
        # certbot for unprivileged users via setuid), this function will need
        # to be modified.
        return common.install_version_controlled_file(options_ssl, options_ssl_digest,
-            self.constant("MOD_SSL_CONF_SRC"), constants.ALL_SSL_OPTIONS_HASHES)
+            self.option("MOD_SSL_CONF_SRC"), constants.ALL_SSL_OPTIONS_HASHES)

    def enable_autohsts(self, _unused_lineage, domains):
        """
@@ -16,14 +16,14 @@ class ArchConfigurator(configurator.ApacheConfigurator):
        vhost_root="/etc/httpd/conf",
        vhost_files="*.conf",
        logs_root="/var/log/httpd",
+        ctl="apachectl",
        version_cmd=['apachectl', '-v'],
-        apache_cmd="apachectl",
        restart_cmd=['apachectl', 'graceful'],
        conftest_cmd=['apachectl', 'configtest'],
        enmod=None,
        dismod=None,
        le_vhost_ext="-le-ssl.conf",
-        handle_mods=False,
+        handle_modules=False,
        handle_sites=False,
        challenge_location="/etc/httpd/conf",
        MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
@@ -18,25 +18,33 @@ class CentOSConfigurator(configurator.ApacheConfigurator):
        vhost_root="/etc/httpd/conf.d",
        vhost_files="*.conf",
        logs_root="/var/log/httpd",
+        ctl="apachectl",
        version_cmd=['apachectl', '-v'],
-        apache_cmd="apachectl",
        restart_cmd=['apachectl', 'graceful'],
        restart_cmd_alt=['apachectl', 'restart'],
        conftest_cmd=['apachectl', 'configtest'],
        enmod=None,
        dismod=None,
        le_vhost_ext="-le-ssl.conf",
-        handle_mods=False,
+        handle_modules=False,
        handle_sites=False,
        challenge_location="/etc/httpd/conf.d",
        MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
            "certbot_apache", "centos-options-ssl-apache.conf")
    )

+    def _prepare_options(self):
+        """
+        Override the options dictionary initialization in order to support
+        alternative restart cmd used in CentOS.
+        """
+        super(CentOSConfigurator, self)._prepare_options()
+        self.options["restart_cmd_alt"][0] = self.option("ctl")
+
    def get_parser(self):
        """Initializes the ApacheParser"""
        return CentOSParser(
-            self.aug, self.conf("server-root"), self.conf("vhost-root"),
+            self.aug, self.option("server_root"), self.option("vhost_root"),
            self.version, configurator=self)


@@ -16,14 +16,14 @@ class DarwinConfigurator(configurator.ApacheConfigurator):
        vhost_root="/etc/apache2/other",
        vhost_files="*.conf",
        logs_root="/var/log/apache2",
-        version_cmd=['/usr/sbin/httpd', '-v'],
-        apache_cmd="/usr/sbin/httpd",
+        ctl="apachectl",
+        version_cmd=['apachectl', '-v'],
        restart_cmd=['apachectl', 'graceful'],
        conftest_cmd=['apachectl', 'configtest'],
        enmod=None,
        dismod=None,
        le_vhost_ext="-le-ssl.conf",
-        handle_mods=False,
+        handle_modules=False,
        handle_sites=False,
        challenge_location="/etc/apache2/other",
        MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
@@ -23,14 +23,14 @@ class DebianConfigurator(configurator.ApacheConfigurator):
        vhost_root="/etc/apache2/sites-available",
        vhost_files="*",
        logs_root="/var/log/apache2",
+        ctl="apache2ctl",
        version_cmd=['apache2ctl', '-v'],
-        apache_cmd="apache2ctl",
        restart_cmd=['apache2ctl', 'graceful'],
        conftest_cmd=['apache2ctl', 'configtest'],
        enmod="a2enmod",
        dismod="a2dismod",
        le_vhost_ext="-le-ssl.conf",
-        handle_mods=True,
+        handle_modules=True,
        handle_sites=True,
        challenge_location="/etc/apache2",
        MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
@@ -134,11 +134,11 @@ class DebianConfigurator(configurator.ApacheConfigurator):
        # 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")):
+        if not util.exe_exists(self.option("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"), "-f", mod_name])
-        util.run_script([self.conf("enmod"), mod_name])
+            temp, [self.option("dismod"), "-f", mod_name])
+        util.run_script([self.option("enmod"), mod_name])
@@ -18,25 +18,33 @@ class GentooConfigurator(configurator.ApacheConfigurator):
        vhost_root="/etc/apache2/vhosts.d",
        vhost_files="*.conf",
        logs_root="/var/log/apache2",
-        version_cmd=['/usr/sbin/apache2', '-v'],
-        apache_cmd="apache2ctl",
+        ctl="apache2ctl",
+        version_cmd=['apache2ctl', '-v'],
        restart_cmd=['apache2ctl', 'graceful'],
        restart_cmd_alt=['apache2ctl', 'restart'],
        conftest_cmd=['apache2ctl', 'configtest'],
        enmod=None,
        dismod=None,
        le_vhost_ext="-le-ssl.conf",
-        handle_mods=False,
+        handle_modules=False,
        handle_sites=False,
        challenge_location="/etc/apache2/vhosts.d",
        MOD_SSL_CONF_SRC=pkg_resources.resource_filename(
            "certbot_apache", "options-ssl-apache.conf")
    )

+    def _prepare_options(self):
+        """
+        Override the options dictionary initialization in order to support
+        alternative restart cmd used in Gentoo.
+        """
+        super(GentooConfigurator, self)._prepare_options()
+        self.options["restart_cmd_alt"][0] = self.option("ctl")
+
    def get_parser(self):
        """Initializes the ApacheParser"""
        return GentooParser(
-            self.aug, self.conf("server-root"), self.conf("vhost-root"),
+            self.aug, self.option("server_root"), self.option("vhost_root"),
            self.version, configurator=self)


@@ -61,7 +69,7 @@ class GentooParser(parser.ApacheParser):

    def update_modules(self):
        """Get loaded modules from httpd process, and add them to DOM"""
-        mod_cmd = [self.configurator.constant("apache_cmd"), "modules"]
+        mod_cmd = [self.configurator.option("ctl"), "modules"]
        matches = self.parse_from_subprocess(mod_cmd, r"(.*)_module")
        for mod in matches:
            self.add_mod(mod.strip())
@@ -16,8 +16,8 @@ class OpenSUSEConfigurator(configurator.ApacheConfigurator):
        vhost_root="/etc/apache2/vhosts.d",
        vhost_files="*.conf",
        logs_root="/var/log/apache2",
+        ctl="apache2ctl",
        version_cmd=['apache2ctl', '-v'],
-        apache_cmd="apache2ctl",
        restart_cmd=['apache2ctl', 'graceful'],
        conftest_cmd=['apache2ctl', 'configtest'],
        enmod="a2enmod",
@@ -69,7 +69,7 @@ class ApacheParser(object):
        # Must also attempt to parse additional virtual host root
        if vhostroot:
            self.parse_file(os.path.abspath(vhostroot) + "/" +
-                            self.configurator.constant("vhost_files"))
+                            self.configurator.option("vhost_files"))

        # check to see if there were unparsed define statements
        if version < (2, 4):
@@ -152,7 +152,7 @@ class ApacheParser(object):
        """Get Defines from httpd process"""

        variables = dict()
-        define_cmd = [self.configurator.constant("apache_cmd"), "-t", "-D",
+        define_cmd = [self.configurator.option("ctl"), "-t", "-D",
                      "DUMP_RUN_CFG"]
        matches = self.parse_from_subprocess(define_cmd, r"Define: ([^ \n]*)")
        try:
@@ -179,7 +179,7 @@ class ApacheParser(object):
        # configuration files
        _ = self.find_dir("Include")

-        inc_cmd = [self.configurator.constant("apache_cmd"), "-t", "-D",
+        inc_cmd = [self.configurator.option("ctl"), "-t", "-D",
                   "DUMP_INCLUDES"]
        matches = self.parse_from_subprocess(inc_cmd, r"\(.*\) (.*)")
        if matches:
@@ -190,7 +190,7 @@ class ApacheParser(object):
    def update_modules(self):
        """Get loaded modules from httpd process, and add them to DOM"""

-        mod_cmd = [self.configurator.constant("apache_cmd"), "-t", "-D",
+        mod_cmd = [self.configurator.option("ctl"), "-t", "-D",
                       "DUMP_MODULES"]
        matches = self.parse_from_subprocess(mod_cmd, r"(.*)_module")
        for mod in matches:
@@ -119,6 +119,9 @@ class AutoHSTSTest(util.ApacheTest):
            cur_val = maxage.format(constants.AUTOHSTS_STEPS[i+1])
            self.assertEquals(self.get_autohsts_value(self.vh_truth[7].path),
                              cur_val)
+        # Ensure that the value is raised to max
+        self.assertEquals(self.get_autohsts_value(self.vh_truth[7].path),
+                          maxage.format(constants.AUTOHSTS_STEPS[-1]))
        # Make permanent
        self.config.deploy_autohsts(mock_lineage)
        self.assertEquals(self.get_autohsts_value(self.vh_truth[7].path),
@@ -135,5 +135,7 @@ class MultipleVhostsTestCentOS(util.ApacheTest):
                                       errors.SubprocessError,
                                       errors.SubprocessError]
        self.assertRaises(errors.MisconfigurationError, self.config.restart)
+
+
 if __name__ == "__main__":
    unittest.main()  # pragma: no cover
@@ -116,8 +116,9 @@ class MultipleVhostsTest(util.ApacheTest):
        ApacheConfigurator.add_parser_arguments(mock.MagicMock())

    def test_constant(self):
-        self.assertEqual(self.config.constant("server_root"), "/etc/apache2")
-        self.assertEqual(self.config.constant("nonexistent"), None)
+        self.assertTrue("debian_apache_2_4/multiple_vhosts/apache" in
+                        self.config.option("server_root"))
+        self.assertEqual(self.config.option("nonexistent"), None)

    @certbot_util.patch_get_utility()
    def test_get_all_names(self, mock_getutility):
@@ -651,22 +652,10 @@ class MultipleVhostsTest(util.ApacheTest):
        self.assertEqual(ssl_vhost_slink.name, "nonsym.link")

    def test_make_vhost_ssl_nonexistent_vhost_path(self):
-        def conf_side_effect(arg):
-            """ Mock function for ApacheConfigurator.conf """
-            confvars = {
-                "vhost-root": "/tmp/nonexistent",
-                "le_vhost_ext": "-le-ssl.conf",
-                "handle-sites": True}
-            return confvars[arg]
-
-        with mock.patch(
-                "certbot_apache.configurator.ApacheConfigurator.conf"
-        ) as mock_conf:
-            mock_conf.side_effect = conf_side_effect
-            ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[1])
-            self.assertEqual(os.path.dirname(ssl_vhost.filep),
-                             os.path.dirname(os.path.realpath(
-                                 self.vh_truth[1].filep)))
+        ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[1])
+        self.assertEqual(os.path.dirname(ssl_vhost.filep),
+                            os.path.dirname(os.path.realpath(
+                                self.vh_truth[1].filep)))

    def test_make_vhost_ssl(self):
        ssl_vhost = self.config.make_vhost_ssl(self.vh_truth[0])
@@ -1583,7 +1572,7 @@ class AugeasVhostsTest(util.ApacheTest):
                          broken_vhost)

 class MultiVhostsTest(util.ApacheTest):
-    """Test vhosts with illegal names dependent on augeas version."""
+    """Test configuration with multiple virtualhosts in a single file."""
    # pylint: disable=protected-access

    def setUp(self):  # pylint: disable=arguments-differ
@@ -1703,7 +1692,7 @@ class InstallSslOptionsConfTest(util.ApacheTest):
                                             self.config.updated_mod_ssl_conf_digest)

    def _current_ssl_options_hash(self):
-        return crypto_util.sha256sum(self.config.constant("MOD_SSL_CONF_SRC"))
+        return crypto_util.sha256sum(self.config.option("MOD_SSL_CONF_SRC"))

    def _assert_current_file(self):
        self.assertTrue(os.path.isfile(self.config.mod_ssl_conf))
@@ -1739,7 +1728,7 @@ class InstallSslOptionsConfTest(util.ApacheTest):
            self.assertFalse(mock_logger.warning.called)
        self.assertTrue(os.path.isfile(self.config.mod_ssl_conf))
        self.assertEqual(crypto_util.sha256sum(
-            self.config.constant("MOD_SSL_CONF_SRC")),
+            self.config.option("MOD_SSL_CONF_SRC")),
            self._current_ssl_options_hash())
        self.assertNotEqual(crypto_util.sha256sum(self.config.mod_ssl_conf),
            self._current_ssl_options_hash())
@@ -1755,7 +1744,7 @@ class InstallSslOptionsConfTest(util.ApacheTest):
                "%s has been manually modified; updated file "
                "saved to %s. We recommend updating %s for security purposes.")
        self.assertEqual(crypto_util.sha256sum(
-            self.config.constant("MOD_SSL_CONF_SRC")),
+            self.config.option("MOD_SSL_CONF_SRC")),
            self._current_ssl_options_hash())
        # only print warning once
        with mock.patch("certbot.plugins.common.logger") as mock_logger:
@@ -20,7 +20,7 @@ class MultipleVhostsTestDebian(util.ApacheTest):
    def setUp(self):  # pylint: disable=arguments-differ
        super(MultipleVhostsTestDebian, self).setUp()
        self.config = util.get_apache_configurator(
-            self.config_path, None, self.config_dir, self.work_dir,
+            self.config_path, self.vhost_path, self.config_dir, self.work_dir,
            os_info="debian")
        self.config = self.mock_deploy_cert(self.config)
        self.vh_truth = util.get_vh_truth(self.temp_dir,
@@ -117,7 +117,7 @@ class MultipleVhostsTestGentoo(util.ApacheTest):
        self.config.parser.modules = set()

        with mock.patch("certbot.util.get_os_info") as mock_osi:
-            # Make sure we have the have the CentOS httpd constants
+            # Make sure we have the have the Gentoo httpd constants
            mock_osi.return_value = ("gentoo", "123")
            self.config.parser.update_runtime_variables()

@@ -282,11 +282,11 @@ class BasicParserTest(util.ParserTest):
        self.assertRaises(
            errors.PluginError, self.parser.update_runtime_variables)

-    @mock.patch("certbot_apache.configurator.ApacheConfigurator.constant")
+    @mock.patch("certbot_apache.configurator.ApacheConfigurator.option")
    @mock.patch("certbot_apache.parser.subprocess.Popen")
-    def test_update_runtime_vars_bad_ctl(self, mock_popen, mock_const):
+    def test_update_runtime_vars_bad_ctl(self, mock_popen, mock_opt):
        mock_popen.side_effect = OSError
-        mock_const.return_value = "nonexistent"
+        mock_opt.return_value = "nonexistent"
        self.assertRaises(
            errors.MisconfigurationError,
            self.parser.update_runtime_variables)
@@ -97,9 +97,10 @@ def get_apache_configurator(  # pylint: disable=too-many-arguments, too-many-loc
    backups = os.path.join(work_dir, "backups")
    mock_le_config = mock.MagicMock(
        apache_server_root=config_path,
-        apache_vhost_root=conf_vhost_path,
+        apache_vhost_root=None,
        apache_le_vhost_ext="-le-ssl.conf",
        apache_challenge_location=config_path,
+        apache_enmod=None,
        backup_dir=backups,
        config_dir=config_dir,
        http01_port=80,
@@ -107,33 +108,25 @@ def get_apache_configurator(  # pylint: disable=too-many-arguments, too-many-loc
        in_progress_dir=os.path.join(backups, "IN_PROGRESS"),
        work_dir=work_dir)

-    orig_os_constant = configurator.ApacheConfigurator(mock_le_config,
-                                                       name="apache",
-                                                       version=version).constant
-
-    def mock_os_constant(key, vhost_path=vhost_path):
-        """Mock default vhost path"""
-        if key == "vhost_root":
-            return vhost_path
-        else:
-            return orig_os_constant(key)
-
-    with mock.patch("certbot_apache.configurator.ApacheConfigurator.constant") as mock_cons:
-        mock_cons.side_effect = mock_os_constant
-        with mock.patch("certbot_apache.configurator.util.run_script"):
-            with mock.patch("certbot_apache.configurator.util."
-                            "exe_exists") as mock_exe_exists:
-                mock_exe_exists.return_value = True
-                with mock.patch("certbot_apache.parser.ApacheParser."
-                                "update_runtime_variables"):
-                    try:
-                        config_class = entrypoint.OVERRIDE_CLASSES[os_info]
-                    except KeyError:
-                        config_class = configurator.ApacheConfigurator
-                    config = config_class(config=mock_le_config, name="apache",
-                        version=version)
-
-                    config.prepare()
+    with mock.patch("certbot_apache.configurator.util.run_script"):
+        with mock.patch("certbot_apache.configurator.util."
+                        "exe_exists") as mock_exe_exists:
+            mock_exe_exists.return_value = True
+            with mock.patch("certbot_apache.parser.ApacheParser."
+                            "update_runtime_variables"):
+                try:
+                    config_class = entrypoint.OVERRIDE_CLASSES[os_info]
+                except KeyError:
+                    config_class = configurator.ApacheConfigurator
+                config = config_class(config=mock_le_config, name="apache",
+                    version=version)
+                if not conf_vhost_path:
+                    config_class.OS_DEFAULTS["vhost_root"] = vhost_path
+                else:
+                    # Custom virtualhost path was requested
+                    config.config.apache_vhost_root = conf_vhost_path
+                config.config.apache_ctl = config_class.OS_DEFAULTS["ctl"]
+                config.prepare()
    return config


@@ -59,9 +59,6 @@ class Proxy(configurators_common.Proxy):
            setattr(self.le_config, "apache_" + k,
                    entrypoint.ENTRYPOINT.OS_DEFAULTS[k])

-        # An alias
-        self.le_config.apache_handle_modules = self.le_config.apache_handle_mods
-
        self._configurator = entrypoint.ENTRYPOINT(
            config=configuration.NamespaceConfig(self.le_config),
            name="apache")