From f6219ddf196f1de0c4d12e1bb5c1f6d737a6e844 Mon Sep 17 00:00:00 2001
From: Joona Hoikkala <joohoi@users.noreply.github.com>
Date: Wed, 1 Aug 2018 22:00:47 +0300
Subject: [PATCH 1/7] Enable Apache VirtualHost for HTTP challenge validation
 if not enabled already (#6268)

If user provides a custom --apache-vhost-root path that's not parsed by Apache per default, Certbot fails the challenge validation. While the VirtualHost on custom path is correctly found, and edited, it's still not seen by Apache. This PR adds a temporary Include directive to the root Apache configuration when writing the challenge tokens to the VirtualHost.
---
 certbot-apache/certbot_apache/http_01.py         |  6 ++++++
 .../certbot_apache/tests/http_01_test.py         | 16 ++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/certbot-apache/certbot_apache/http_01.py b/certbot-apache/certbot_apache/http_01.py
index 37545e9cc..22598baca 100644
--- a/certbot-apache/certbot_apache/http_01.py
+++ b/certbot-apache/certbot_apache/http_01.py
@@ -6,6 +6,7 @@ from acme.magic_typing import Set  # pylint: disable=unused-import, no-name-in-m
 from certbot import errors
 from certbot.plugins import common
 from certbot_apache.obj import VirtualHost  # pylint: disable=unused-import
+from certbot_apache.parser import get_aug_path
 
 logger = logging.getLogger(__name__)
 
@@ -172,4 +173,9 @@ class ApacheHttp01(common.TLSSNI01):
             self.configurator.parser.add_dir(
                 vhost.path, "Include", self.challenge_conf_post)
 
+            if not vhost.enabled:
+                self.configurator.parser.add_dir(
+                    get_aug_path(self.configurator.parser.loc["default"]),
+                    "Include", vhost.filep)
+
             self.moded_vhosts.add(vhost)
diff --git a/certbot-apache/certbot_apache/tests/http_01_test.py b/certbot-apache/certbot_apache/tests/http_01_test.py
index 98bf412ae..9c729b08c 100644
--- a/certbot-apache/certbot_apache/tests/http_01_test.py
+++ b/certbot-apache/certbot_apache/tests/http_01_test.py
@@ -10,6 +10,7 @@ from certbot import achallenges
 from certbot import errors
 
 from certbot.tests import acme_util
+from certbot_apache.parser import get_aug_path
 from certbot_apache.tests import util
 
 
@@ -134,6 +135,21 @@ class ApacheHttp01Test(util.ApacheTest):
     def test_perform_3_achall_apache_2_4(self):
         self.combinations_perform_test(num_achalls=3, minor_version=4)
 
+    def test_activate_disabled_vhost(self):
+        vhosts = [v for v in self.config.vhosts if v.name == "certbot.demo"]
+        achalls = [
+            achallenges.KeyAuthorizationAnnotatedChallenge(
+                challb=acme_util.chall_to_challb(
+                    challenges.HTTP01(token=((b'a' * 16))),
+                    "pending"),
+                domain="certbot.demo", account_key=self.account_key)]
+        vhosts[0].enabled = False
+        self.common_perform_test(achalls, vhosts)
+        matches = self.config.parser.find_dir(
+            "Include", vhosts[0].filep,
+            get_aug_path(self.config.parser.loc["default"]))
+        self.assertEqual(len(matches), 1)
+
     def combinations_perform_test(self, num_achalls, minor_version):
         """Test perform with the given achall count and Apache version."""
         achalls = self.achalls[:num_achalls]

From 7bff0a02e4b86db47015987bbafa3d3a93246829 Mon Sep 17 00:00:00 2001
From: Joona Hoikkala <joohoi@users.noreply.github.com>
Date: Thu, 2 Aug 2018 18:17:38 +0300
Subject: [PATCH 2/7] 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
---
 certbot-apache/certbot_apache/configurator.py | 115 +++++++++++-------
 .../certbot_apache/override_arch.py           |   4 +-
 .../certbot_apache/override_centos.py         |  14 ++-
 .../certbot_apache/override_darwin.py         |   6 +-
 .../certbot_apache/override_debian.py         |  10 +-
 .../certbot_apache/override_gentoo.py         |  18 ++-
 .../certbot_apache/override_suse.py           |   2 +-
 certbot-apache/certbot_apache/parser.py       |   8 +-
 .../certbot_apache/tests/autohsts_test.py     |   3 +
 .../certbot_apache/tests/centos_test.py       |   2 +
 .../certbot_apache/tests/configurator_test.py |  33 ++---
 .../certbot_apache/tests/debian_test.py       |   2 +-
 .../certbot_apache/tests/gentoo_test.py       |   2 +-
 .../certbot_apache/tests/parser_test.py       |   6 +-
 certbot-apache/certbot_apache/tests/util.py   |  49 ++++----
 .../configurators/apache/common.py            |   3 -
 16 files changed, 150 insertions(+), 127 deletions(-)

diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py
index bb77e2e41..da632dc81 100644
--- a/certbot-apache/certbot_apache/configurator.py
+++ b/certbot-apache/certbot_apache/configurator.py
@@ -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):
         """
diff --git a/certbot-apache/certbot_apache/override_arch.py b/certbot-apache/certbot_apache/override_arch.py
index ea5155a3c..c5620e9f9 100644
--- a/certbot-apache/certbot_apache/override_arch.py
+++ b/certbot-apache/certbot_apache/override_arch.py
@@ -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(
diff --git a/certbot-apache/certbot_apache/override_centos.py b/certbot-apache/certbot_apache/override_centos.py
index 0b6b12b96..a4f1b84ec 100644
--- a/certbot-apache/certbot_apache/override_centos.py
+++ b/certbot-apache/certbot_apache/override_centos.py
@@ -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)
 
 
diff --git a/certbot-apache/certbot_apache/override_darwin.py b/certbot-apache/certbot_apache/override_darwin.py
index 53741d504..4e2a6acac 100644
--- a/certbot-apache/certbot_apache/override_darwin.py
+++ b/certbot-apache/certbot_apache/override_darwin.py
@@ -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(
diff --git a/certbot-apache/certbot_apache/override_debian.py b/certbot-apache/certbot_apache/override_debian.py
index 02dffc3f7..0caa619d2 100644
--- a/certbot-apache/certbot_apache/override_debian.py
+++ b/certbot-apache/certbot_apache/override_debian.py
@@ -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])
diff --git a/certbot-apache/certbot_apache/override_gentoo.py b/certbot-apache/certbot_apache/override_gentoo.py
index 165e44c96..556e3225e 100644
--- a/certbot-apache/certbot_apache/override_gentoo.py
+++ b/certbot-apache/certbot_apache/override_gentoo.py
@@ -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())
diff --git a/certbot-apache/certbot_apache/override_suse.py b/certbot-apache/certbot_apache/override_suse.py
index a67054b5b..83079b92c 100644
--- a/certbot-apache/certbot_apache/override_suse.py
+++ b/certbot-apache/certbot_apache/override_suse.py
@@ -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",
diff --git a/certbot-apache/certbot_apache/parser.py b/certbot-apache/certbot_apache/parser.py
index 02337f8d4..148f052d0 100644
--- a/certbot-apache/certbot_apache/parser.py
+++ b/certbot-apache/certbot_apache/parser.py
@@ -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:
diff --git a/certbot-apache/certbot_apache/tests/autohsts_test.py b/certbot-apache/certbot_apache/tests/autohsts_test.py
index 73da33f15..c5d720dd3 100644
--- a/certbot-apache/certbot_apache/tests/autohsts_test.py
+++ b/certbot-apache/certbot_apache/tests/autohsts_test.py
@@ -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),
diff --git a/certbot-apache/certbot_apache/tests/centos_test.py b/certbot-apache/certbot_apache/tests/centos_test.py
index 4ee8b5dcf..46b857c3e 100644
--- a/certbot-apache/certbot_apache/tests/centos_test.py
+++ b/certbot-apache/certbot_apache/tests/centos_test.py
@@ -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
diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py
index 350262634..6f1c358c2 100644
--- a/certbot-apache/certbot_apache/tests/configurator_test.py
+++ b/certbot-apache/certbot_apache/tests/configurator_test.py
@@ -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:
diff --git a/certbot-apache/certbot_apache/tests/debian_test.py b/certbot-apache/certbot_apache/tests/debian_test.py
index fde8d4c35..bb1d64278 100644
--- a/certbot-apache/certbot_apache/tests/debian_test.py
+++ b/certbot-apache/certbot_apache/tests/debian_test.py
@@ -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,
diff --git a/certbot-apache/certbot_apache/tests/gentoo_test.py b/certbot-apache/certbot_apache/tests/gentoo_test.py
index d32551267..0681e30b5 100644
--- a/certbot-apache/certbot_apache/tests/gentoo_test.py
+++ b/certbot-apache/certbot_apache/tests/gentoo_test.py
@@ -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()
 
diff --git a/certbot-apache/certbot_apache/tests/parser_test.py b/certbot-apache/certbot_apache/tests/parser_test.py
index f95f1b346..d62fd54e8 100644
--- a/certbot-apache/certbot_apache/tests/parser_test.py
+++ b/certbot-apache/certbot_apache/tests/parser_test.py
@@ -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)
diff --git a/certbot-apache/certbot_apache/tests/util.py b/certbot-apache/certbot_apache/tests/util.py
index 6d3cfa109..9329ccb20 100644
--- a/certbot-apache/certbot_apache/tests/util.py
+++ b/certbot-apache/certbot_apache/tests/util.py
@@ -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
 
 
diff --git a/certbot-compatibility-test/certbot_compatibility_test/configurators/apache/common.py b/certbot-compatibility-test/certbot_compatibility_test/configurators/apache/common.py
index 1d2cfdeca..82195264b 100644
--- a/certbot-compatibility-test/certbot_compatibility_test/configurators/apache/common.py
+++ b/certbot-compatibility-test/certbot_compatibility_test/configurators/apache/common.py
@@ -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")

From 4989668e0aaec8ff7d03661846d3340ca6b245af Mon Sep 17 00:00:00 2001
From: Josh Soref <jsoref@users.noreply.github.com>
Date: Fri, 3 Aug 2018 17:55:12 -0400
Subject: [PATCH 3/7] Clarify that configurations can be invalid (#6277)

---
 certbot/cert_manager.py | 2 +-
 certbot/renewal.py      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/certbot/cert_manager.py b/certbot/cert_manager.py
index d1205835a..771ca8caf 100644
--- a/certbot/cert_manager.py
+++ b/certbot/cert_manager.py
@@ -353,7 +353,7 @@ def _describe_certs(config, parsed_certs, parse_failures):
             notify("Found the following {0}certs:".format(match))
             notify(_report_human_readable(config, parsed_certs))
         if parse_failures:
-            notify("\nThe following renewal configuration files "
+            notify("\nThe following renewal configurations "
                "were invalid:")
             notify(_report_lines(parse_failures))
 
diff --git a/certbot/renewal.py b/certbot/renewal.py
index f50131028..ecc8b1f2f 100644
--- a/certbot/renewal.py
+++ b/certbot/renewal.py
@@ -359,7 +359,7 @@ def _renew_describe_results(config, renew_successes, renew_failures,
         notify_error(report(renew_failures, "failure"))
 
     if parse_failures:
-        notify("\nAdditionally, the following renewal configuration files "
+        notify("\nAdditionally, the following renewal configurations "
                "were invalid: ")
         notify(report(parse_failures, "parsefail"))
 

From d8057f0e17dc757fae662dad91a6fedc96ad6a2d Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@users.noreply.github.com>
Date: Mon, 6 Aug 2018 09:45:56 -0700
Subject: [PATCH 4/7] Fix Sphinx (#6070)

Fixes #4686.

In Sphinx 1.6, they changed how they handle images in latex and PDF files. You can learn more about this by reading the linked issue (or I can answer any questions), but the shortish version is we now need to use the extension sphinx.ext.imgconverter. This is only available in Sphinx 1.6+.

I also updated our pinned versions to use the latest Sphinx and a new dependency it pulled in called sphinxcontrib-websupport. To build the latex and PDF docs, you must first run:

apt-get install imagemagick latexmk texlive texlive-latex-extra

Afterwards, if you create the normal Certbot dev environment using this branch, activate the virtual environment, and from the root of the repo run make -C docs clean latex latexpdf, you'll successfully build the PDF docs.

* fix #4686

* bump minimum Sphinx req
---
 docs/conf.py              | 1 +
 setup.py                  | 4 ++--
 tools/dev_constraints.txt | 3 ++-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/docs/conf.py b/docs/conf.py
index 09bb44285..2e6c5a9b7 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -40,6 +40,7 @@ needs_sphinx = '1.0'
 # ones.
 extensions = [
     'sphinx.ext.autodoc',
+    'sphinx.ext.imgconverter',
     'sphinx.ext.intersphinx',
     'sphinx.ext.todo',
     'sphinx.ext.coverage',
diff --git a/setup.py b/setup.py
index 1827c4d42..a13b7cdb9 100644
--- a/setup.py
+++ b/setup.py
@@ -70,8 +70,8 @@ dev3_extras = [
 
 docs_extras = [
     'repoze.sphinx.autointerface',
-    # autodoc_member_order = 'bysource', autodoc_default_flags, and #4686
-    'Sphinx >=1.0,<=1.5.6',
+    # sphinx.ext.imgconverter
+    'Sphinx >=1.6',
     'sphinx_rtd_theme',
 ]
 
diff --git a/tools/dev_constraints.txt b/tools/dev_constraints.txt
index f14169bfc..ef7804328 100644
--- a/tools/dev_constraints.txt
+++ b/tools/dev_constraints.txt
@@ -60,8 +60,9 @@ s3transfer==0.1.11
 scandir==1.6
 simplegeneric==0.8.1
 snowballstemmer==1.2.1
-Sphinx==1.5.6
+Sphinx==1.7.5
 sphinx-rtd-theme==0.2.4
+sphinxcontrib-websupport==1.0.1
 tldextract==2.2.0
 tox==2.9.1
 tqdm==4.19.4

From b1003b7250fe0b53a683d1a48b732130f0d5aa99 Mon Sep 17 00:00:00 2001
From: Adrien Ferrand <adferrand@users.noreply.github.com>
Date: Thu, 16 Aug 2018 21:28:25 +0200
Subject: [PATCH 5/7] Fail fast during tests if python executable is not in the
 PATH (#6306)

---
 tests/boulder-integration.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh
index 2f9130489..fbe8d26aa 100755
--- a/tests/boulder-integration.sh
+++ b/tests/boulder-integration.sh
@@ -10,6 +10,9 @@
 
 set -eux
 
+# Check that python executable is available in the PATH. Fail immediatly if not.
+command -v python > /dev/null || (echo "Error, python executable is not in the PATH" && exit 1)
+
 . ./tests/integration/_common.sh
 export PATH="$PATH:/usr/sbin"  # /usr/sbin/nginx
 

From 0e9dd5e3d21d3907733cac7ba9bdf3f7a27105db Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@users.noreply.github.com>
Date: Tue, 21 Aug 2018 08:59:56 -0700
Subject: [PATCH 6/7] Remove visible warnings about missing apachectl. (#6307)

---
 certbot/plugins/util.py      | 4 ++--
 certbot/plugins/util_test.py | 9 +++------
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/certbot/plugins/util.py b/certbot/plugins/util.py
index ad2257e1d..3f03bf375 100644
--- a/certbot/plugins/util.py
+++ b/certbot/plugins/util.py
@@ -51,6 +51,6 @@ def path_surgery(cmd):
         return True
     else:
         expanded = " expanded" if any(added) else ""
-        logger.warning("Failed to find executable %s in%s PATH: %s", cmd,
-                       expanded, path)
+        logger.debug("Failed to find executable %s in%s PATH: %s", cmd,
+                     expanded, path)
         return False
diff --git a/certbot/plugins/util_test.py b/certbot/plugins/util_test.py
index 2c0e476ae..9757d8de7 100644
--- a/certbot/plugins/util_test.py
+++ b/certbot/plugins/util_test.py
@@ -16,9 +16,8 @@ class GetPrefixTest(unittest.TestCase):
 class PathSurgeryTest(unittest.TestCase):
     """Tests for certbot.plugins.path_surgery."""
 
-    @mock.patch("certbot.plugins.util.logger.warning")
     @mock.patch("certbot.plugins.util.logger.debug")
-    def test_path_surgery(self, mock_debug, mock_warn):
+    def test_path_surgery(self, mock_debug):
         from certbot.plugins.util import path_surgery
         all_path = {"PATH": "/usr/local/bin:/bin/:/usr/sbin/:/usr/local/sbin/"}
         with mock.patch.dict('os.environ', all_path):
@@ -26,14 +25,12 @@ class PathSurgeryTest(unittest.TestCase):
                 mock_exists.return_value = True
                 self.assertEqual(path_surgery("eg"), True)
                 self.assertEqual(mock_debug.call_count, 0)
-                self.assertEqual(mock_warn.call_count, 0)
                 self.assertEqual(os.environ["PATH"], all_path["PATH"])
         no_path = {"PATH": "/tmp/"}
         with mock.patch.dict('os.environ', no_path):
             path_surgery("thingy")
-            self.assertEqual(mock_debug.call_count, 1)
-            self.assertEqual(mock_warn.call_count, 1)
-            self.assertTrue("Failed to find" in mock_warn.call_args[0][0])
+            self.assertEqual(mock_debug.call_count, 2)
+            self.assertTrue("Failed to find" in mock_debug.call_args[0][0])
             self.assertTrue("/usr/local/bin" in os.environ["PATH"])
             self.assertTrue("/tmp" in os.environ["PATH"])
 

From 6e23b81dba7566267c98e4b946bbc90f755b562d Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@users.noreply.github.com>
Date: Wed, 29 Aug 2018 14:11:13 -0700
Subject: [PATCH 7/7] Separate integration (#5814)

Main piece of #5810.

* Rename Certbot integration tests

* Remove nginx from certbot tests

* allow for running individual integration tests

* fail under 65

* Add set -e

* Track Nginx coverage and omit it from report later.

* Use INTEGRATION_TEST in script

* add INTEGRATION_TEST=all

* update min certbot percentage
---
 .travis.yml                          |   4 +-
 tests/boulder-integration.sh         | 505 +--------------------------
 tests/certbot-boulder-integration.sh | 492 ++++++++++++++++++++++++++
 3 files changed, 505 insertions(+), 496 deletions(-)
 create mode 100755 tests/certbot-boulder-integration.sh

diff --git a/.travis.yml b/.travis.yml
index b4d702ff1..acdf365bd 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -13,11 +13,11 @@ before_script:
 matrix:
   include:
     - python: "2.7"
-      env: TOXENV=py27_install BOULDER_INTEGRATION=v1
+      env: BOULDER_INTEGRATION=v1 INTEGRATION_TEST=all TOXENV=py27_install
       sudo: required
       services: docker
     - python: "2.7"
-      env: TOXENV=py27_install BOULDER_INTEGRATION=v2
+      env: BOULDER_INTEGRATION=v2 INTEGRATION_TEST=all TOXENV=py27_install
       sudo: required
       services: docker
     - python: "2.7"
diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh
index fbe8d26aa..3e16fcbbc 100755
--- a/tests/boulder-integration.sh
+++ b/tests/boulder-integration.sh
@@ -1,499 +1,16 @@
 #!/bin/bash
-# Simple integration test. Make sure to activate virtualenv beforehand
-# (source venv/bin/activate) and that you are running Boulder test
-# instance (see ./boulder-fetch.sh).
-#
-# Environment variables:
-#   SERVER: Passed as "certbot --server" argument.
-#
-# Note: this script is called by Boulder integration test suite!
 
-set -eux
+set -e
 
-# Check that python executable is available in the PATH. Fail immediatly if not.
-command -v python > /dev/null || (echo "Error, python executable is not in the PATH" && exit 1)
-
-. ./tests/integration/_common.sh
-export PATH="$PATH:/usr/sbin"  # /usr/sbin/nginx
-
-cleanup_and_exit() {
-    EXIT_STATUS=$?
-    if SERVER_STILL_RUNNING=`ps -p $python_server_pid -o pid=`
-    then
-        echo Kill server subprocess, left running by abnormal exit
-        kill $SERVER_STILL_RUNNING
-    fi
-    if [ -f "$HOOK_DIRS_TEST" ]; then
-        rm -f "$HOOK_DIRS_TEST"
-    fi
-    exit $EXIT_STATUS
-}
-
-trap cleanup_and_exit EXIT
-
-export HOOK_DIRS_TEST="$(mktemp)"
-renewal_hooks_root="$config_dir/renewal-hooks"
-renewal_hooks_dirs=$(echo "$renewal_hooks_root/"{pre,deploy,post})
-renewal_dir_pre_hook="$(echo $renewal_hooks_dirs | cut -f 1 -d " ")/hook.sh"
-renewal_dir_deploy_hook="$(echo $renewal_hooks_dirs | cut -f 2 -d " ")/hook.sh"
-renewal_dir_post_hook="$(echo $renewal_hooks_dirs | cut -f 3 -d " ")/hook.sh"
-
-# Creates hooks in Certbot's renewal hook directory that write to a file
-CreateDirHooks() {
-    for hook_dir in $renewal_hooks_dirs; do
-        mkdir -p $hook_dir
-        hook_path="$hook_dir/hook.sh"
-        cat << EOF > "$hook_path"
-#!/bin/bash -xe
-if [ "\$0" = "$renewal_dir_deploy_hook" ]; then
-    if [ -z "\$RENEWED_DOMAINS" -o -z "\$RENEWED_LINEAGE" ]; then
-        echo "Environment variables not properly set!" >&2
-        exit 1
+if [ "$INTEGRATION_TEST" = "certbot" ]; then
+    tests/certbot-boulder-integration.sh
+elif [ "$INTEGRATION_TEST" = "nginx" ]; then
+    certbot-nginx/tests/boulder-integration.sh
+else
+   tests/certbot-boulder-integration.sh
+    # Most CI systems set this variable to true.
+    # If the tests are running as part of CI, Nginx should be available.
+    if ${CI:-false} || type nginx; then
+        certbot-nginx/tests/boulder-integration.sh
     fi
 fi
-echo \$(basename \$(dirname "\$0")) >> "\$HOOK_DIRS_TEST"
-EOF
-        chmod +x "$hook_path"
-    done
-}
-
-# Asserts that the hooks created by CreateDirHooks have been run once and
-# resets the file.
-#
-# Arguments:
-#     The number of times the deploy hook should have been run. (It should run
-#     once for each certificate that was issued in that run of Certbot.)
-CheckDirHooks() {
-    expected="pre\n"
-    for ((i=0; i<$1; i++)); do
-        expected=$expected"deploy\n"
-    done
-    expected=$expected"post"
-
-    if ! diff "$HOOK_DIRS_TEST" <(echo -e "$expected"); then
-        echo "Unexpected directory hook output!" >&2
-        echo "Expected:" >&2
-        echo -e "$expected" >&2
-        echo "Got:" >&2
-        cat "$HOOK_DIRS_TEST" >&2
-        exit 1
-    fi
-
-    rm -f "$HOOK_DIRS_TEST"
-    export HOOK_DIRS_TEST="$(mktemp)"
-}
-
-common_no_force_renew() {
-    certbot_test_no_force_renew \
-        --authenticator standalone \
-        --installer null \
-        "$@"
-}
-
-common() {
-    common_no_force_renew \
-        --renew-by-default \
-        "$@"
-}
-
-export HOOK_TEST="/tmp/hook$$"
-CheckHooks() {
-    if [ $(head -n1 "$HOOK_TEST") = "wtf.pre" ]; then
-        expected="wtf.pre\ndeploy\n"
-        if [ $(sed '3q;d' "$HOOK_TEST") = "deploy" ]; then
-            expected=$expected"deploy\nwtf2.pre\n"
-        else
-            expected=$expected"wtf2.pre\ndeploy\n"
-        fi
-        expected=$expected"deploy\ndeploy\nwtf.post\nwtf2.post"
-    else
-        expected="wtf2.pre\ndeploy\n"
-        if [ $(sed '3q;d' "$HOOK_TEST") = "deploy" ]; then
-            expected=$expected"deploy\nwtf.pre\n"
-        else
-            expected=$expected"wtf.pre\ndeploy\n"
-        fi
-        expected=$expected"deploy\ndeploy\nwtf2.post\nwtf.post"
-    fi
-
-    if ! cmp --quiet <(echo -e "$expected") "$HOOK_TEST" ; then
-        echo Hooks did not run as expected\; got >&2
-        cat "$HOOK_TEST" >&2
-        echo -e "Expected\n$expected" >&2
-        rm "$HOOK_TEST"
-        exit 1
-    fi
-    rm "$HOOK_TEST"
-}
-
-# Checks if deploy is in the hook output and deletes the file
-DeployInHookOutput() {
-    CONTENTS=$(cat "$HOOK_TEST")
-    rm "$HOOK_TEST"
-    grep deploy <(echo "$CONTENTS")
-}
-
-# Asserts that there is a saved renew_hook for a lineage.
-#
-# Arguments:
-#     Name of lineage to check
-CheckSavedRenewHook() {
-    if ! grep renew_hook "$config_dir/renewal/$1.conf"; then
-        echo "Hook wasn't saved as renew_hook" >&2
-        exit 1
-    fi
-}
-
-# Asserts the deploy hook was properly run and saved and deletes the hook file
-#
-# Arguments:
-#   Lineage name of the issued cert
-CheckDeployHook() {
-    if ! DeployInHookOutput; then
-        echo "The deploy hook wasn't run" >&2
-        exit 1
-    fi
-    CheckSavedRenewHook $1
-}
-
-# Asserts the renew hook wasn't run but was saved and deletes the hook file
-#
-# Arguments:
-#   Lineage name of the issued cert
-# Asserts the deploy hook wasn't run and deletes the hook file
-CheckRenewHook() {
-    if DeployInHookOutput; then
-        echo "The renew hook was incorrectly run" >&2
-        exit 1
-    fi
-    CheckSavedRenewHook $1
-}
-
-# Return success only if input contains exactly $1 lines of text, of
-# which $2 different values occur in the first field.
-TotalAndDistinctLines() {
-    total=$1
-    distinct=$2
-    awk '{a[$1] = 1}; END {exit(NR !='$total' || length(a) !='$distinct')}'
-}
-
-# Cleanup coverage data
-coverage erase
-
-# test for regressions of #4719
-get_num_tmp_files() {
-    ls -1 /tmp | wc -l
-}
-num_tmp_files=$(get_num_tmp_files)
-common --csr / && echo expected error && exit 1 || true
-common --help
-common --help all
-common --version
-if [ $(get_num_tmp_files) -ne $num_tmp_files ]; then
-    echo "New files or directories created in /tmp!"
-    exit 1
-fi
-CreateDirHooks
-
-common register
-for dir in $renewal_hooks_dirs; do
-    if [ ! -d "$dir" ]; then
-        echo "Hook directory not created by Certbot!" >&2
-        exit 1
-    fi
-done
-
-common unregister
-
-common register --email ex1@domain.org,ex2@domain.org
-
-common register --update-registration --email ex1@domain.org
-
-common register --update-registration --email ex1@domain.org,ex2@domain.org
-
-common plugins --init --prepare | grep webroot
-
-# We start a server listening on the port for the
-# unrequested challenge to prevent regressions in #3601.
-python ./tests/run_http_server.py $http_01_port &
-python_server_pid=$!
-
-certname="le1.wtf"
-common --domains le1.wtf --preferred-challenges tls-sni-01 auth \
-       --cert-name $certname \
-       --pre-hook 'echo wtf.pre >> "$HOOK_TEST"' \
-       --post-hook 'echo wtf.post >> "$HOOK_TEST"'\
-       --deploy-hook 'echo deploy >> "$HOOK_TEST"'
-kill $python_server_pid
-CheckDeployHook $certname
-
-python ./tests/run_http_server.py $tls_sni_01_port &
-python_server_pid=$!
-certname="le2.wtf"
-common --domains le2.wtf --preferred-challenges http-01 run \
-       --cert-name $certname \
-       --pre-hook 'echo wtf.pre >> "$HOOK_TEST"' \
-       --post-hook 'echo wtf.post >> "$HOOK_TEST"'\
-       --deploy-hook 'echo deploy >> "$HOOK_TEST"'
-kill $python_server_pid
-CheckDeployHook $certname
-
-certname="le.wtf"
-common certonly -a manual -d le.wtf --rsa-key-size 4096 --cert-name $certname \
-    --manual-auth-hook ./tests/manual-http-auth.sh \
-    --manual-cleanup-hook ./tests/manual-http-cleanup.sh \
-    --pre-hook 'echo wtf2.pre >> "$HOOK_TEST"' \
-    --post-hook 'echo wtf2.post >> "$HOOK_TEST"' \
-    --renew-hook 'echo deploy >> "$HOOK_TEST"'
-CheckRenewHook $certname
-
-certname="dns.le.wtf"
-common -a manual -d dns.le.wtf --preferred-challenges dns,tls-sni run \
-    --cert-name $certname \
-    --manual-auth-hook ./tests/manual-dns-auth.sh \
-    --manual-cleanup-hook ./tests/manual-dns-cleanup.sh \
-    --pre-hook 'echo wtf2.pre >> "$HOOK_TEST"' \
-    --post-hook 'echo wtf2.post >> "$HOOK_TEST"' \
-    --renew-hook 'echo deploy >> "$HOOK_TEST"'
-CheckRenewHook $certname
-
-common certonly --cert-name newname -d newname.le.wtf
-
-export CSR_PATH="${root}/csr.der" KEY_PATH="${root}/key.pem" \
-       OPENSSL_CNF=examples/openssl.cnf
-./examples/generate-csr.sh le3.wtf
-common auth --csr "$CSR_PATH" \
-       --cert-path "${root}/csr/cert.pem" \
-       --chain-path "${root}/csr/chain.pem"
-openssl x509 -in "${root}/csr/cert.pem" -text
-openssl x509 -in "${root}/csr/chain.pem" -text
-
-common --domains le3.wtf install \
-       --cert-path "${root}/csr/cert.pem" \
-       --key-path "${root}/key.pem"
-
-CheckCertCount() {
-    CERTCOUNT=`ls "${root}/conf/archive/$1/cert"* | wc -l`
-    if [ "$CERTCOUNT" -ne "$2" ] ; then
-        echo Wrong cert count, not "$2" `ls "${root}/conf/archive/$1/"*`
-        exit 1
-    fi
-}
-
-CheckCertCount "le.wtf" 1
-# This won't renew (because it's not time yet)
-common_no_force_renew renew
-CheckCertCount "le.wtf" 1
-if [ -s "$HOOK_DIRS_TEST" ]; then
-    echo "Directory hooks were executed for non-renewal!" >&2;
-    exit 1
-fi
-
-rm -rf "$renewal_hooks_root"
-# renew using HTTP manual auth hooks
-common renew --cert-name le.wtf --authenticator manual
-CheckCertCount "le.wtf" 2
-
-# test renewal with no executables in hook directories
-for hook_dir in $renewal_hooks_dirs; do
-    touch "$hook_dir/file"
-    mkdir "$hook_dir/dir"
-done
-# renew using DNS manual auth hooks
-common renew --cert-name dns.le.wtf --authenticator manual
-CheckCertCount "dns.le.wtf" 2
-
-# test with disabled directory hooks
-rm -rf "$renewal_hooks_root"
-CreateDirHooks
-# This will renew because the expiry is less than 10 years from now
-sed -i "4arenew_before_expiry = 4 years" "$root/conf/renewal/le.wtf.conf"
-common_no_force_renew renew --rsa-key-size 2048 --no-directory-hooks
-CheckCertCount "le.wtf" 3
-if [ -s "$HOOK_DIRS_TEST" ]; then
-    echo "Directory hooks were executed with --no-directory-hooks!" >&2
-    exit 1
-fi
-
-# The 4096 bit setting should persist to the first renewal, but be overridden in the second
-
-size1=`wc -c ${root}/conf/archive/le.wtf/privkey1.pem | cut -d" " -f1`
-size2=`wc -c ${root}/conf/archive/le.wtf/privkey2.pem | cut -d" " -f1`
-size3=`wc -c ${root}/conf/archive/le.wtf/privkey3.pem | cut -d" " -f1`
-# 4096 bit PEM keys are about ~3270 bytes, 2048 ones are about 1700 bytes
-if [ "$size1" -lt 3000 ] || [ "$size2" -lt 3000 ] || [ "$size3" -gt 1800 ] ; then
-    echo key sizes violate assumptions:
-    ls -l "${root}/conf/archive/le.wtf/privkey"*
-    exit 1
-fi
-
-# --renew-by-default is used, so renewal should occur
-[ -f "$HOOK_TEST" ] && rm -f "$HOOK_TEST"
-common renew
-CheckCertCount "le.wtf" 4
-CheckHooks
-CheckDirHooks 5
-
-# test with overlapping directory hooks on the command line
-common renew --cert-name le2.wtf \
-    --pre-hook "$renewal_dir_pre_hook" \
-    --deploy-hook "$renewal_dir_deploy_hook" \
-    --post-hook "$renewal_dir_post_hook"
-CheckDirHooks 1
-
-# test with overlapping directory hooks in the renewal conf files
-common renew --cert-name le2.wtf
-CheckDirHooks 1
-
-# manual-dns-auth.sh will skip completing the challenge for domains that begin
-# with fail.
-common -a manual -d dns1.le.wtf,fail.dns1.le.wtf \
-    --allow-subset-of-names \
-    --preferred-challenges dns,tls-sni \
-    --manual-auth-hook ./tests/manual-dns-auth.sh \
-    --manual-cleanup-hook ./tests/manual-dns-cleanup.sh
-
-if common certificates | grep "fail\.dns1\.le\.wtf"; then
-    echo "certificate should not have been issued for domain!" >&2
-    exit 1
-fi
-
-# reuse-key
-common --domains reusekey.le.wtf --reuse-key
-common renew --cert-name reusekey.le.wtf
-CheckCertCount "reusekey.le.wtf" 2
-ls -l "${root}/conf/archive/reusekey.le.wtf/privkey"*
-# The final awk command here exits successfully if its input consists of
-# exactly two lines with identical first fields, and unsuccessfully otherwise.
-sha256sum "${root}/conf/archive/reusekey.le.wtf/privkey"* | TotalAndDistinctLines 2 1
-
-# don't reuse key (just by forcing reissuance without --reuse-key)
-common --cert-name reusekey.le.wtf --domains reusekey.le.wtf --force-renewal
-CheckCertCount "reusekey.le.wtf" 3
-ls -l "${root}/conf/archive/reusekey.le.wtf/privkey"*
-# Exactly three lines, of which exactly two identical first fields.
-sha256sum "${root}/conf/archive/reusekey.le.wtf/privkey"* | TotalAndDistinctLines 3 2
-
-# Nonetheless, all three certificates are different even though two of them
-# share the same subject key.
-sha256sum "${root}/conf/archive/reusekey.le.wtf/cert"* | TotalAndDistinctLines 3 3
-
-# ECDSA
-openssl ecparam -genkey -name secp384r1 -out "${root}/privkey-p384.pem"
-SAN="DNS:ecdsa.le.wtf" openssl req -new -sha256 \
-    -config "${OPENSSL_CNF:-openssl.cnf}" \
-    -key "${root}/privkey-p384.pem" \
-    -subj "/" \
-    -reqexts san \
-    -outform der \
-    -out "${root}/csr-p384.der"
-common auth --csr "${root}/csr-p384.der" \
-    --cert-path "${root}/csr/cert-p384.pem" \
-    --chain-path "${root}/csr/chain-p384.pem"
-openssl x509 -in "${root}/csr/cert-p384.pem" -text | grep 'ASN1 OID: secp384r1'
-
-# OCSP Must Staple
-common auth --must-staple --domains "must-staple.le.wtf"
-openssl x509 -in "${root}/conf/live/must-staple.le.wtf/cert.pem" -text | grep '1.3.6.1.5.5.7.1.24'
-
-# revoke by account key
-common revoke --cert-path "$root/conf/live/le.wtf/cert.pem" --delete-after-revoke
-# revoke renewed
-common revoke --cert-path "$root/conf/live/le1.wtf/cert.pem" --no-delete-after-revoke
-if [ ! -d "$root/conf/live/le1.wtf" ]; then
-    echo "cert deleted when --no-delete-after-revoke was used!"
-    exit 1
-fi
-common delete --cert-name le1.wtf
-# revoke by cert key
-common revoke --cert-path "$root/conf/live/le2.wtf/cert.pem" \
-    --key-path "$root/conf/live/le2.wtf/privkey.pem"
-
-# Get new certs to test revoke with a reason, by account and by cert key
-common --domains le1.wtf
-common revoke --cert-path "$root/conf/live/le1.wtf/cert.pem" \
-    --reason cessationOfOperation
-common --domains le2.wtf
-common revoke --cert-path "$root/conf/live/le2.wtf/cert.pem" \
-    --key-path "$root/conf/live/le2.wtf/privkey.pem" \
-    --reason keyCompromise
-
-common unregister
-
-out=$(common certificates)
-subdomains="le dns.le newname.le must-staple.le"
-for subdomain in $subdomains; do
-    domain="$subdomain.wtf"
-    if ! echo $out | grep "$domain"; then
-        echo "$domain not in certificates output!"
-        exit 1;
-    fi
-done
-
-# Testing that revocation also deletes by default
-subdomains="le1 le2"
-for subdomain in $subdomains; do
-    domain="$subdomain.wtf"
-    if echo $out | grep "$domain"; then
-        echo "Revoked $domain in certificates output! Should not be!"
-        exit 1;
-    fi
-done
-
-# Test that revocation raises correct error if --cert-name and --cert-path don't match
-common --domains le1.wtf
-common --domains le2.wtf
-out=$(common revoke --cert-path "$root/conf/live/le1.wtf/fullchain.pem" --cert-name "le2.wtf" 2>&1) || true
-if ! echo $out | grep "or both must point to the same certificate lineages."; then
-    echo "Non-interactive revoking with mismatched --cert-name and --cert-path "
-    echo "did not raise the correct error!"
-    exit 1
-fi
-
-# Revoking by matching --cert-name and --cert-path deletes
-common --domains le1.wtf
-common revoke --cert-path "$root/conf/live/le1.wtf/fullchain.pem" --cert-name "le1.wtf"
-out=$(common certificates)
-if echo $out | grep "le1.wtf"; then
-    echo "Cert le1.wtf should've been deleted! Was revoked via matching --cert-path & --cert-name"
-    exit 1
-fi
-
-# Test that revocation doesn't delete if multiple lineages share an archive dir
-common --domains le1.wtf
-common --domains le2.wtf
-sed -i "s|^archive_dir = .*$|archive_dir = $root/conf/archive/le1.wtf|" "$root/conf/renewal/le2.wtf.conf"
-#common update_symlinks # not needed, but a bit more context for what this test is about
-out=$(common revoke --cert-path "$root/conf/live/le1.wtf/cert.pem")
-if ! echo $out | grep "Not deleting revoked certs due to overlapping archive dirs"; then
-    echo "Deleted a cert that had an overlapping archive dir with another lineage!"
-    exit 1
-fi
-
-cert_name="must-staple.le.wtf"
-common delete --cert-name $cert_name
-archive="$root/conf/archive/$cert_name"
-conf="$root/conf/renewal/$cert_name.conf"
-live="$root/conf/live/$cert_name"
-for path in $archive $conf $live; do
-    if [ -e $path ]; then
-        echo "Lineage not properly deleted!"
-        exit 1
-    fi
-done
-
-# Test ACMEv2-only features
-if [ "${BOULDER_INTEGRATION:-v1}" = "v2" ]; then
-    common -a manual -d '*.le4.wtf,le4.wtf' --preferred-challenges dns \
-        --manual-auth-hook ./tests/manual-dns-auth.sh \
-        --manual-cleanup-hook ./tests/manual-dns-cleanup.sh
-fi
-
-coverage report --fail-under 65 --include 'certbot/*' --show-missing
-
-# Most CI systems set this variable to true.
-# If the tests are running as part of CI, Nginx should be available.
-if ${CI:-false} || type nginx;
-then
-    . ./certbot-nginx/tests/boulder-integration.sh
-fi
diff --git a/tests/certbot-boulder-integration.sh b/tests/certbot-boulder-integration.sh
new file mode 100755
index 000000000..8b8b931e5
--- /dev/null
+++ b/tests/certbot-boulder-integration.sh
@@ -0,0 +1,492 @@
+#!/bin/bash
+# Simple integration test. Make sure to activate virtualenv beforehand
+# (source venv/bin/activate) and that you are running Boulder test
+# instance (see ./boulder-fetch.sh).
+#
+# Environment variables:
+#   SERVER: Passed as "certbot --server" argument.
+#
+# Note: this script is called by Boulder integration test suite!
+
+set -eux
+
+# Check that python executable is available in the PATH. Fail immediatly if not.
+command -v python > /dev/null || (echo "Error, python executable is not in the PATH" && exit 1)
+
+. ./tests/integration/_common.sh
+export PATH="$PATH:/usr/sbin"  # /usr/sbin/nginx
+
+cleanup_and_exit() {
+    EXIT_STATUS=$?
+    if SERVER_STILL_RUNNING=`ps -p $python_server_pid -o pid=`
+    then
+        echo Kill server subprocess, left running by abnormal exit
+        kill $SERVER_STILL_RUNNING
+    fi
+    if [ -f "$HOOK_DIRS_TEST" ]; then
+        rm -f "$HOOK_DIRS_TEST"
+    fi
+    exit $EXIT_STATUS
+}
+
+trap cleanup_and_exit EXIT
+
+export HOOK_DIRS_TEST="$(mktemp)"
+renewal_hooks_root="$config_dir/renewal-hooks"
+renewal_hooks_dirs=$(echo "$renewal_hooks_root/"{pre,deploy,post})
+renewal_dir_pre_hook="$(echo $renewal_hooks_dirs | cut -f 1 -d " ")/hook.sh"
+renewal_dir_deploy_hook="$(echo $renewal_hooks_dirs | cut -f 2 -d " ")/hook.sh"
+renewal_dir_post_hook="$(echo $renewal_hooks_dirs | cut -f 3 -d " ")/hook.sh"
+
+# Creates hooks in Certbot's renewal hook directory that write to a file
+CreateDirHooks() {
+    for hook_dir in $renewal_hooks_dirs; do
+        mkdir -p $hook_dir
+        hook_path="$hook_dir/hook.sh"
+        cat << EOF > "$hook_path"
+#!/bin/bash -xe
+if [ "\$0" = "$renewal_dir_deploy_hook" ]; then
+    if [ -z "\$RENEWED_DOMAINS" -o -z "\$RENEWED_LINEAGE" ]; then
+        echo "Environment variables not properly set!" >&2
+        exit 1
+    fi
+fi
+echo \$(basename \$(dirname "\$0")) >> "\$HOOK_DIRS_TEST"
+EOF
+        chmod +x "$hook_path"
+    done
+}
+
+# Asserts that the hooks created by CreateDirHooks have been run once and
+# resets the file.
+#
+# Arguments:
+#     The number of times the deploy hook should have been run. (It should run
+#     once for each certificate that was issued in that run of Certbot.)
+CheckDirHooks() {
+    expected="pre\n"
+    for ((i=0; i<$1; i++)); do
+        expected=$expected"deploy\n"
+    done
+    expected=$expected"post"
+
+    if ! diff "$HOOK_DIRS_TEST" <(echo -e "$expected"); then
+        echo "Unexpected directory hook output!" >&2
+        echo "Expected:" >&2
+        echo -e "$expected" >&2
+        echo "Got:" >&2
+        cat "$HOOK_DIRS_TEST" >&2
+        exit 1
+    fi
+
+    rm -f "$HOOK_DIRS_TEST"
+    export HOOK_DIRS_TEST="$(mktemp)"
+}
+
+common_no_force_renew() {
+    certbot_test_no_force_renew \
+        --authenticator standalone \
+        --installer null \
+        "$@"
+}
+
+common() {
+    common_no_force_renew \
+        --renew-by-default \
+        "$@"
+}
+
+export HOOK_TEST="/tmp/hook$$"
+CheckHooks() {
+    if [ $(head -n1 "$HOOK_TEST") = "wtf.pre" ]; then
+        expected="wtf.pre\ndeploy\n"
+        if [ $(sed '3q;d' "$HOOK_TEST") = "deploy" ]; then
+            expected=$expected"deploy\nwtf2.pre\n"
+        else
+            expected=$expected"wtf2.pre\ndeploy\n"
+        fi
+        expected=$expected"deploy\ndeploy\nwtf.post\nwtf2.post"
+    else
+        expected="wtf2.pre\ndeploy\n"
+        if [ $(sed '3q;d' "$HOOK_TEST") = "deploy" ]; then
+            expected=$expected"deploy\nwtf.pre\n"
+        else
+            expected=$expected"wtf.pre\ndeploy\n"
+        fi
+        expected=$expected"deploy\ndeploy\nwtf2.post\nwtf.post"
+    fi
+
+    if ! cmp --quiet <(echo -e "$expected") "$HOOK_TEST" ; then
+        echo Hooks did not run as expected\; got >&2
+        cat "$HOOK_TEST" >&2
+        echo -e "Expected\n$expected" >&2
+        rm "$HOOK_TEST"
+        exit 1
+    fi
+    rm "$HOOK_TEST"
+}
+
+# Checks if deploy is in the hook output and deletes the file
+DeployInHookOutput() {
+    CONTENTS=$(cat "$HOOK_TEST")
+    rm "$HOOK_TEST"
+    grep deploy <(echo "$CONTENTS")
+}
+
+# Asserts that there is a saved renew_hook for a lineage.
+#
+# Arguments:
+#     Name of lineage to check
+CheckSavedRenewHook() {
+    if ! grep renew_hook "$config_dir/renewal/$1.conf"; then
+        echo "Hook wasn't saved as renew_hook" >&2
+        exit 1
+    fi
+}
+
+# Asserts the deploy hook was properly run and saved and deletes the hook file
+#
+# Arguments:
+#   Lineage name of the issued cert
+CheckDeployHook() {
+    if ! DeployInHookOutput; then
+        echo "The deploy hook wasn't run" >&2
+        exit 1
+    fi
+    CheckSavedRenewHook $1
+}
+
+# Asserts the renew hook wasn't run but was saved and deletes the hook file
+#
+# Arguments:
+#   Lineage name of the issued cert
+# Asserts the deploy hook wasn't run and deletes the hook file
+CheckRenewHook() {
+    if DeployInHookOutput; then
+        echo "The renew hook was incorrectly run" >&2
+        exit 1
+    fi
+    CheckSavedRenewHook $1
+}
+
+# Return success only if input contains exactly $1 lines of text, of
+# which $2 different values occur in the first field.
+TotalAndDistinctLines() {
+    total=$1
+    distinct=$2
+    awk '{a[$1] = 1}; END {exit(NR !='$total' || length(a) !='$distinct')}'
+}
+
+# Cleanup coverage data
+coverage erase
+
+# test for regressions of #4719
+get_num_tmp_files() {
+    ls -1 /tmp | wc -l
+}
+num_tmp_files=$(get_num_tmp_files)
+common --csr / && echo expected error && exit 1 || true
+common --help
+common --help all
+common --version
+if [ $(get_num_tmp_files) -ne $num_tmp_files ]; then
+    echo "New files or directories created in /tmp!"
+    exit 1
+fi
+CreateDirHooks
+
+common register
+for dir in $renewal_hooks_dirs; do
+    if [ ! -d "$dir" ]; then
+        echo "Hook directory not created by Certbot!" >&2
+        exit 1
+    fi
+done
+
+common unregister
+
+common register --email ex1@domain.org,ex2@domain.org
+
+common register --update-registration --email ex1@domain.org
+
+common register --update-registration --email ex1@domain.org,ex2@domain.org
+
+common plugins --init --prepare | grep webroot
+
+# We start a server listening on the port for the
+# unrequested challenge to prevent regressions in #3601.
+python ./tests/run_http_server.py $http_01_port &
+python_server_pid=$!
+
+certname="le1.wtf"
+common --domains le1.wtf --preferred-challenges tls-sni-01 auth \
+       --cert-name $certname \
+       --pre-hook 'echo wtf.pre >> "$HOOK_TEST"' \
+       --post-hook 'echo wtf.post >> "$HOOK_TEST"'\
+       --deploy-hook 'echo deploy >> "$HOOK_TEST"'
+kill $python_server_pid
+CheckDeployHook $certname
+
+python ./tests/run_http_server.py $tls_sni_01_port &
+python_server_pid=$!
+certname="le2.wtf"
+common --domains le2.wtf --preferred-challenges http-01 run \
+       --cert-name $certname \
+       --pre-hook 'echo wtf.pre >> "$HOOK_TEST"' \
+       --post-hook 'echo wtf.post >> "$HOOK_TEST"'\
+       --deploy-hook 'echo deploy >> "$HOOK_TEST"'
+kill $python_server_pid
+CheckDeployHook $certname
+
+certname="le.wtf"
+common certonly -a manual -d le.wtf --rsa-key-size 4096 --cert-name $certname \
+    --manual-auth-hook ./tests/manual-http-auth.sh \
+    --manual-cleanup-hook ./tests/manual-http-cleanup.sh \
+    --pre-hook 'echo wtf2.pre >> "$HOOK_TEST"' \
+    --post-hook 'echo wtf2.post >> "$HOOK_TEST"' \
+    --renew-hook 'echo deploy >> "$HOOK_TEST"'
+CheckRenewHook $certname
+
+certname="dns.le.wtf"
+common -a manual -d dns.le.wtf --preferred-challenges dns,tls-sni run \
+    --cert-name $certname \
+    --manual-auth-hook ./tests/manual-dns-auth.sh \
+    --manual-cleanup-hook ./tests/manual-dns-cleanup.sh \
+    --pre-hook 'echo wtf2.pre >> "$HOOK_TEST"' \
+    --post-hook 'echo wtf2.post >> "$HOOK_TEST"' \
+    --renew-hook 'echo deploy >> "$HOOK_TEST"'
+CheckRenewHook $certname
+
+common certonly --cert-name newname -d newname.le.wtf
+
+export CSR_PATH="${root}/csr.der" KEY_PATH="${root}/key.pem" \
+       OPENSSL_CNF=examples/openssl.cnf
+./examples/generate-csr.sh le3.wtf
+common auth --csr "$CSR_PATH" \
+       --cert-path "${root}/csr/cert.pem" \
+       --chain-path "${root}/csr/chain.pem"
+openssl x509 -in "${root}/csr/cert.pem" -text
+openssl x509 -in "${root}/csr/chain.pem" -text
+
+common --domains le3.wtf install \
+       --cert-path "${root}/csr/cert.pem" \
+       --key-path "${root}/key.pem"
+
+CheckCertCount() {
+    CERTCOUNT=`ls "${root}/conf/archive/$1/cert"* | wc -l`
+    if [ "$CERTCOUNT" -ne "$2" ] ; then
+        echo Wrong cert count, not "$2" `ls "${root}/conf/archive/$1/"*`
+        exit 1
+    fi
+}
+
+CheckCertCount "le.wtf" 1
+# This won't renew (because it's not time yet)
+common_no_force_renew renew
+CheckCertCount "le.wtf" 1
+if [ -s "$HOOK_DIRS_TEST" ]; then
+    echo "Directory hooks were executed for non-renewal!" >&2;
+    exit 1
+fi
+
+rm -rf "$renewal_hooks_root"
+# renew using HTTP manual auth hooks
+common renew --cert-name le.wtf --authenticator manual
+CheckCertCount "le.wtf" 2
+
+# test renewal with no executables in hook directories
+for hook_dir in $renewal_hooks_dirs; do
+    touch "$hook_dir/file"
+    mkdir "$hook_dir/dir"
+done
+# renew using DNS manual auth hooks
+common renew --cert-name dns.le.wtf --authenticator manual
+CheckCertCount "dns.le.wtf" 2
+
+# test with disabled directory hooks
+rm -rf "$renewal_hooks_root"
+CreateDirHooks
+# This will renew because the expiry is less than 10 years from now
+sed -i "4arenew_before_expiry = 4 years" "$root/conf/renewal/le.wtf.conf"
+common_no_force_renew renew --rsa-key-size 2048 --no-directory-hooks
+CheckCertCount "le.wtf" 3
+if [ -s "$HOOK_DIRS_TEST" ]; then
+    echo "Directory hooks were executed with --no-directory-hooks!" >&2
+    exit 1
+fi
+
+# The 4096 bit setting should persist to the first renewal, but be overridden in the second
+
+size1=`wc -c ${root}/conf/archive/le.wtf/privkey1.pem | cut -d" " -f1`
+size2=`wc -c ${root}/conf/archive/le.wtf/privkey2.pem | cut -d" " -f1`
+size3=`wc -c ${root}/conf/archive/le.wtf/privkey3.pem | cut -d" " -f1`
+# 4096 bit PEM keys are about ~3270 bytes, 2048 ones are about 1700 bytes
+if [ "$size1" -lt 3000 ] || [ "$size2" -lt 3000 ] || [ "$size3" -gt 1800 ] ; then
+    echo key sizes violate assumptions:
+    ls -l "${root}/conf/archive/le.wtf/privkey"*
+    exit 1
+fi
+
+# --renew-by-default is used, so renewal should occur
+[ -f "$HOOK_TEST" ] && rm -f "$HOOK_TEST"
+common renew
+CheckCertCount "le.wtf" 4
+CheckHooks
+CheckDirHooks 5
+
+# test with overlapping directory hooks on the command line
+common renew --cert-name le2.wtf \
+    --pre-hook "$renewal_dir_pre_hook" \
+    --deploy-hook "$renewal_dir_deploy_hook" \
+    --post-hook "$renewal_dir_post_hook"
+CheckDirHooks 1
+
+# test with overlapping directory hooks in the renewal conf files
+common renew --cert-name le2.wtf
+CheckDirHooks 1
+
+# manual-dns-auth.sh will skip completing the challenge for domains that begin
+# with fail.
+common -a manual -d dns1.le.wtf,fail.dns1.le.wtf \
+    --allow-subset-of-names \
+    --preferred-challenges dns,tls-sni \
+    --manual-auth-hook ./tests/manual-dns-auth.sh \
+    --manual-cleanup-hook ./tests/manual-dns-cleanup.sh
+
+if common certificates | grep "fail\.dns1\.le\.wtf"; then
+    echo "certificate should not have been issued for domain!" >&2
+    exit 1
+fi
+
+# reuse-key
+common --domains reusekey.le.wtf --reuse-key
+common renew --cert-name reusekey.le.wtf
+CheckCertCount "reusekey.le.wtf" 2
+ls -l "${root}/conf/archive/reusekey.le.wtf/privkey"*
+# The final awk command here exits successfully if its input consists of
+# exactly two lines with identical first fields, and unsuccessfully otherwise.
+sha256sum "${root}/conf/archive/reusekey.le.wtf/privkey"* | TotalAndDistinctLines 2 1
+
+# don't reuse key (just by forcing reissuance without --reuse-key)
+common --cert-name reusekey.le.wtf --domains reusekey.le.wtf --force-renewal
+CheckCertCount "reusekey.le.wtf" 3
+ls -l "${root}/conf/archive/reusekey.le.wtf/privkey"*
+# Exactly three lines, of which exactly two identical first fields.
+sha256sum "${root}/conf/archive/reusekey.le.wtf/privkey"* | TotalAndDistinctLines 3 2
+
+# Nonetheless, all three certificates are different even though two of them
+# share the same subject key.
+sha256sum "${root}/conf/archive/reusekey.le.wtf/cert"* | TotalAndDistinctLines 3 3
+
+# ECDSA
+openssl ecparam -genkey -name secp384r1 -out "${root}/privkey-p384.pem"
+SAN="DNS:ecdsa.le.wtf" openssl req -new -sha256 \
+    -config "${OPENSSL_CNF:-openssl.cnf}" \
+    -key "${root}/privkey-p384.pem" \
+    -subj "/" \
+    -reqexts san \
+    -outform der \
+    -out "${root}/csr-p384.der"
+common auth --csr "${root}/csr-p384.der" \
+    --cert-path "${root}/csr/cert-p384.pem" \
+    --chain-path "${root}/csr/chain-p384.pem"
+openssl x509 -in "${root}/csr/cert-p384.pem" -text | grep 'ASN1 OID: secp384r1'
+
+# OCSP Must Staple
+common auth --must-staple --domains "must-staple.le.wtf"
+openssl x509 -in "${root}/conf/live/must-staple.le.wtf/cert.pem" -text | grep '1.3.6.1.5.5.7.1.24'
+
+# revoke by account key
+common revoke --cert-path "$root/conf/live/le.wtf/cert.pem" --delete-after-revoke
+# revoke renewed
+common revoke --cert-path "$root/conf/live/le1.wtf/cert.pem" --no-delete-after-revoke
+if [ ! -d "$root/conf/live/le1.wtf" ]; then
+    echo "cert deleted when --no-delete-after-revoke was used!"
+    exit 1
+fi
+common delete --cert-name le1.wtf
+# revoke by cert key
+common revoke --cert-path "$root/conf/live/le2.wtf/cert.pem" \
+    --key-path "$root/conf/live/le2.wtf/privkey.pem"
+
+# Get new certs to test revoke with a reason, by account and by cert key
+common --domains le1.wtf
+common revoke --cert-path "$root/conf/live/le1.wtf/cert.pem" \
+    --reason cessationOfOperation
+common --domains le2.wtf
+common revoke --cert-path "$root/conf/live/le2.wtf/cert.pem" \
+    --key-path "$root/conf/live/le2.wtf/privkey.pem" \
+    --reason keyCompromise
+
+common unregister
+
+out=$(common certificates)
+subdomains="le dns.le newname.le must-staple.le"
+for subdomain in $subdomains; do
+    domain="$subdomain.wtf"
+    if ! echo $out | grep "$domain"; then
+        echo "$domain not in certificates output!"
+        exit 1;
+    fi
+done
+
+# Testing that revocation also deletes by default
+subdomains="le1 le2"
+for subdomain in $subdomains; do
+    domain="$subdomain.wtf"
+    if echo $out | grep "$domain"; then
+        echo "Revoked $domain in certificates output! Should not be!"
+        exit 1;
+    fi
+done
+
+# Test that revocation raises correct error if --cert-name and --cert-path don't match
+common --domains le1.wtf
+common --domains le2.wtf
+out=$(common revoke --cert-path "$root/conf/live/le1.wtf/fullchain.pem" --cert-name "le2.wtf" 2>&1) || true
+if ! echo $out | grep "or both must point to the same certificate lineages."; then
+    echo "Non-interactive revoking with mismatched --cert-name and --cert-path "
+    echo "did not raise the correct error!"
+    exit 1
+fi
+
+# Revoking by matching --cert-name and --cert-path deletes
+common --domains le1.wtf
+common revoke --cert-path "$root/conf/live/le1.wtf/fullchain.pem" --cert-name "le1.wtf"
+out=$(common certificates)
+if echo $out | grep "le1.wtf"; then
+    echo "Cert le1.wtf should've been deleted! Was revoked via matching --cert-path & --cert-name"
+    exit 1
+fi
+
+# Test that revocation doesn't delete if multiple lineages share an archive dir
+common --domains le1.wtf
+common --domains le2.wtf
+sed -i "s|^archive_dir = .*$|archive_dir = $root/conf/archive/le1.wtf|" "$root/conf/renewal/le2.wtf.conf"
+#common update_symlinks # not needed, but a bit more context for what this test is about
+out=$(common revoke --cert-path "$root/conf/live/le1.wtf/cert.pem")
+if ! echo $out | grep "Not deleting revoked certs due to overlapping archive dirs"; then
+    echo "Deleted a cert that had an overlapping archive dir with another lineage!"
+    exit 1
+fi
+
+cert_name="must-staple.le.wtf"
+common delete --cert-name $cert_name
+archive="$root/conf/archive/$cert_name"
+conf="$root/conf/renewal/$cert_name.conf"
+live="$root/conf/live/$cert_name"
+for path in $archive $conf $live; do
+    if [ -e $path ]; then
+        echo "Lineage not properly deleted!"
+        exit 1
+    fi
+done
+
+# Test ACMEv2-only features
+if [ "${BOULDER_INTEGRATION:-v1}" = "v2" ]; then
+    common -a manual -d '*.le4.wtf,le4.wtf' --preferred-challenges dns \
+        --manual-auth-hook ./tests/manual-dns-auth.sh \
+        --manual-cleanup-hook ./tests/manual-dns-cleanup.sh
+fi
+
+coverage report --fail-under 64 --include 'certbot/*' --show-missing