From bd956592759dcc725815d97f4f854e6e7c3dda96 Mon Sep 17 00:00:00 2001 From: Will Greenberg Date: Mon, 24 Oct 2022 19:25:56 -0700 Subject: [PATCH] Another rework: use the apache_bin option for RHEL 9+ --- .../certbot_apache/_internal/apache_util.py | 16 ++++++---------- .../certbot_apache/_internal/configurator.py | 13 ++++++++++--- .../certbot_apache/_internal/override_centos.py | 17 ++++++++--------- .../certbot_apache/_internal/parser.py | 6 +++--- certbot-apache/tests/centos_test.py | 4 +++- certbot-apache/tests/util.py | 1 + 6 files changed, 31 insertions(+), 26 deletions(-) diff --git a/certbot-apache/certbot_apache/_internal/apache_util.py b/certbot-apache/certbot_apache/_internal/apache_util.py index a538d8f0d..54b9f9824 100644 --- a/certbot-apache/certbot_apache/_internal/apache_util.py +++ b/certbot-apache/certbot_apache/_internal/apache_util.py @@ -136,20 +136,18 @@ def included_in_paths(filepath: str, paths: Iterable[str]) -> bool: return any(fnmatch.fnmatch(filepath, path) for path in paths) -def parse_defines(apachectl: str) -> Dict[str, str]: +def parse_defines(define_cmd: List[str]) -> Dict[str, str]: """ Gets Defines from httpd process and returns a dictionary of the defined variables. - :param str apachectl: Path to apachectl executable + :param list define_cmd: httpd command to dump defines :returns: dictionary of defined variables :rtype: dict """ variables: Dict[str, str] = {} - define_cmd = [apachectl, "-t", "-D", - "DUMP_RUN_CFG"] matches = parse_from_subprocess(define_cmd, r"Define: ([^ \n]*)") try: matches.remove("DUMP_RUN_CFG") @@ -165,33 +163,31 @@ def parse_defines(apachectl: str) -> Dict[str, str]: return variables -def parse_includes(apachectl: str) -> List[str]: +def parse_includes(inc_cmd: List[str]) -> List[str]: """ Gets Include directives from httpd process and returns a list of their values. - :param str apachectl: Path to apachectl executable + :param list inc_cmd: httpd command to dump includes :returns: list of found Include directive values :rtype: list of str """ - inc_cmd: List[str] = [apachectl, "-t", "-D", "DUMP_INCLUDES"] return parse_from_subprocess(inc_cmd, r"\(.*\) (.*)") -def parse_modules(apachectl: str) -> List[str]: +def parse_modules(mod_cmd: List[str]) -> List[str]: """ Get loaded modules from httpd process, and return the list of loaded module names. - :param str apachectl: Path to apachectl executable + :param list mod_cmd: httpd command to dump loaded modules :returns: list of found LoadModule module names :rtype: list of str """ - mod_cmd = [apachectl, "-t", "-D", "DUMP_MODULES"] return parse_from_subprocess(mod_cmd, r"(.*)_module") diff --git a/certbot-apache/certbot_apache/_internal/configurator.py b/certbot-apache/certbot_apache/_internal/configurator.py index 93e06c4c5..3a7708dc0 100644 --- a/certbot-apache/certbot_apache/_internal/configurator.py +++ b/certbot-apache/certbot_apache/_internal/configurator.py @@ -85,6 +85,10 @@ class OsOptions: self.restart_cmd = ['apache2ctl', 'graceful'] if not restart_cmd else restart_cmd self.restart_cmd_alt = restart_cmd_alt self.conftest_cmd = ['apache2ctl', 'configtest'] if not conftest_cmd else conftest_cmd + syntax_tests_cmd_base = [ctl, '-t', '-D'] + self.get_defines_cmd = syntax_tests_cmd_base + ['DUMP_RUN_CFG'] + self.get_includes_cmd = syntax_tests_cmd_base + ['DUMP_INCLUDES'] + self.get_modules_cmd = syntax_tests_cmd_base + ['DUMP_MODULES'] self.enmod = enmod self.dismod = dismod self.le_vhost_ext = le_vhost_ext @@ -189,6 +193,9 @@ class ApacheConfigurator(common.Configurator): self.options.version_cmd[0] = self.options.ctl self.options.restart_cmd[0] = self.options.ctl self.options.conftest_cmd[0] = self.options.ctl + self.options.get_modules_cmd[0] = self.options.ctl + self.options.get_includes_cmd[0] = self.options.ctl + self.options.get_defines_cmd[0] = self.options.ctl def _prepare_options(self) -> None: self._set_options() @@ -487,9 +494,9 @@ class ApacheConfigurator(common.Configurator): if HAS_APACHECONFIG: apache_vars = { - "defines": apache_util.parse_defines(self.options.ctl), - "includes": apache_util.parse_includes(self.options.ctl), - "modules": apache_util.parse_modules(self.options.ctl), + "defines": apache_util.parse_defines(self.options.get_defines_cmd), + "includes": apache_util.parse_includes(self.options.get_includes_cmd), + "modules": apache_util.parse_modules(self.options.get_modules_cmd), } metadata["apache_vars"] = apache_vars diff --git a/certbot-apache/certbot_apache/_internal/override_centos.py b/certbot-apache/certbot_apache/_internal/override_centos.py index cd392b38a..acb29284d 100644 --- a/certbot-apache/certbot_apache/_internal/override_centos.py +++ b/certbot-apache/certbot_apache/_internal/override_centos.py @@ -140,7 +140,8 @@ class CentOSConfigurator(BaseCentOSConfigurator): vhost_root="/etc/httpd/conf.d", vhost_files="*.conf", logs_root="/var/log/httpd", - ctl="httpd", + ctl="apachectl", + apache_bin="httpd", version_cmd=['httpd', '-v'], restart_cmd=['apachectl', 'graceful'], restart_cmd_alt=['apachectl', 'restart'], @@ -148,16 +149,14 @@ class CentOSConfigurator(BaseCentOSConfigurator): challenge_location="/etc/httpd/conf.d", ) - def _prepare_options(self) -> None: + def _override_cmds(self) -> None: """ - By only calling self._set_options() and not the parent class' - _prepare_options() method, we ensure that httpd is used for version_cmd - but not restart_cmd, unless the user has specifically overwritten those - in their config + As of RHEL 9, apachectl can't be passed flags like "-t -D", so instead + use options.bin (which is "httpd") """ - self._set_options() - if not self.options.restart_cmd_alt: # pragma: no cover - raise ValueError("OS option restart_cmd_alt must be set for CentOS.") + self.options.get_modules_cmd[0] = self.options.bin + self.options.get_includes_cmd[0] = self.options.bin + self.options.get_defines_cmd[0] = self.options.bin class OldCentOSConfigurator(BaseCentOSConfigurator): diff --git a/certbot-apache/certbot_apache/_internal/parser.py b/certbot-apache/certbot_apache/_internal/parser.py index 46e61843f..019172d37 100644 --- a/certbot-apache/certbot_apache/_internal/parser.py +++ b/certbot-apache/certbot_apache/_internal/parser.py @@ -302,7 +302,7 @@ class ApacheParser: def update_defines(self) -> None: """Updates the dictionary of known variables in the configuration""" - self.variables = apache_util.parse_defines(self.configurator.options.ctl) + self.variables = apache_util.parse_defines(self.configurator.options.get_defines_cmd) def update_includes(self) -> None: """Get includes from httpd process, and add them to DOM if needed""" @@ -312,7 +312,7 @@ class ApacheParser: # configuration files _ = self.find_dir("Include") - matches = apache_util.parse_includes(self.configurator.options.ctl) + matches = apache_util.parse_includes(self.configurator.options.get_includes_cmd) if matches: for i in matches: if not self.parsed_in_current(i): @@ -321,7 +321,7 @@ class ApacheParser: def update_modules(self) -> None: """Get loaded modules from httpd process, and add them to DOM""" - matches = apache_util.parse_modules(self.configurator.options.ctl) + matches = apache_util.parse_modules(self.configurator.options.get_modules_cmd) for mod in matches: self.add_mod(mod.strip()) diff --git a/certbot-apache/tests/centos_test.py b/certbot-apache/tests/centos_test.py index 3cfc27395..b9e648813 100644 --- a/certbot-apache/tests/centos_test.py +++ b/certbot-apache/tests/centos_test.py @@ -112,9 +112,11 @@ class UseCorrectApacheExecutableTest(util.ApacheTest): config = util.get_apache_configurator( self.config_path, self.vhost_path, self.config_dir, self.work_dir, config_class=override_centos.CentOSConfigurator) - self.assertEqual(config.options.ctl, "httpd") + self.assertEqual(config.options.ctl, "apachectl") + self.assertEqual(config.options.bin, "httpd") self.assertEqual(config.options.version_cmd, ["httpd", "-v"]) self.assertEqual(config.options.restart_cmd, ["apachectl", "graceful"]) + self.assertEqual(config.options.conftest_cmd, ["apachectl", "configtest"]) class MultipleVhostsTestCentOS(util.ApacheTest): diff --git a/certbot-apache/tests/util.py b/certbot-apache/tests/util.py index 601605314..ba0fefb23 100644 --- a/certbot-apache/tests/util.py +++ b/certbot-apache/tests/util.py @@ -125,6 +125,7 @@ def get_apache_configurator( # Custom virtualhost path was requested config.config.apache_vhost_root = conf_vhost_path config.config.apache_ctl = config_class.OS_DEFAULTS.ctl + config.config.apache_bin = config_class.OS_DEFAULTS.bin config.prepare() return config