diff --git a/certbot-apache/certbot_apache/_internal/configurator.py b/certbot-apache/certbot_apache/_internal/configurator.py index 82b3dcaec..93e06c4c5 100644 --- a/certbot-apache/certbot_apache/_internal/configurator.py +++ b/certbot-apache/certbot_apache/_internal/configurator.py @@ -166,7 +166,7 @@ class ApacheConfigurator(common.Configurator): return apache_util.find_ssl_apache_conf("old") return apache_util.find_ssl_apache_conf("current") - def _prepare_options(self) -> None: + def _set_options(self) -> None: """ Set the values possibly changed by command line parameters to OS_DEFAULTS constant dictionary @@ -181,11 +181,19 @@ class ApacheConfigurator(common.Configurator): else: setattr(self.options, o, getattr(self.OS_DEFAULTS, o)) - # Special cases + def _override_cmds(self) -> None: + """ + Override the various command binaries w/ whatever we have set for + options.ctl + """ self.options.version_cmd[0] = self.options.ctl self.options.restart_cmd[0] = self.options.ctl self.options.conftest_cmd[0] = self.options.ctl + def _prepare_options(self) -> None: + self._set_options() + self._override_cmds() + @classmethod def add_parser_arguments(cls, add: Callable[..., None]) -> None: # When adding, modifying or deleting command line arguments, be sure to diff --git a/certbot-apache/certbot_apache/_internal/entrypoint.py b/certbot-apache/certbot_apache/_internal/entrypoint.py index 3f8a00411..74c59ca0e 100644 --- a/certbot-apache/certbot_apache/_internal/entrypoint.py +++ b/certbot-apache/certbot_apache/_internal/entrypoint.py @@ -16,32 +16,35 @@ from certbot import util OVERRIDE_CLASSES: Dict[str, Type[configurator.ApacheConfigurator]] = { "arch": override_arch.ArchConfigurator, - "cloudlinux": override_centos.CentOSConfigurator, "darwin": override_darwin.DarwinConfigurator, "debian": override_debian.DebianConfigurator, "ubuntu": override_debian.DebianConfigurator, - "centos": override_centos.CentOSConfigurator, - "centos linux": override_centos.CentOSConfigurator, - "fedora_old": override_centos.CentOSConfigurator, "fedora": override_fedora.FedoraConfigurator, "linuxmint": override_debian.DebianConfigurator, - "ol": override_centos.CentOSConfigurator, - "oracle": override_centos.CentOSConfigurator, - "redhatenterpriseserver": override_centos.CentOSConfigurator, - "red hat enterprise linux server": override_centos.CentOSConfigurator, - "rhel": override_centos.CentOSConfigurator, "amazon": override_centos.CentOSConfigurator, "gentoo": override_gentoo.GentooConfigurator, "gentoo base system": override_gentoo.GentooConfigurator, "opensuse": override_suse.OpenSUSEConfigurator, "suse": override_suse.OpenSUSEConfigurator, "sles": override_suse.OpenSUSEConfigurator, - "scientific": override_centos.CentOSConfigurator, - "scientific linux": override_centos.CentOSConfigurator, "void": override_void.VoidConfigurator, } +def rhel_derived_os(os_name) -> bool: + """ + Returns whether the given OS is RHEL derived, i.e. tracks RHEL's versioning + scheme, and thus should use our CentOS configurator + """ + return os_name in [ + "cloudlinux", + "centos", "centos linux", + "ol", "oracle", + "rhel", "redhatenterpriseserver", "red hat enterprise linux server", + "scientific", "scientific linux", + ] + + def get_configurator() -> Type[configurator.ApacheConfigurator]: """ Get correct configurator class based on the OS fingerprint """ os_name, os_version = util.get_os_info() @@ -51,7 +54,14 @@ def get_configurator() -> Type[configurator.ApacheConfigurator]: # Special case for older Fedora versions min_version = util.parse_loose_version('29') if os_name == 'fedora' and util.parse_loose_version(os_version) < min_version: - os_name = 'fedora_old' + return override_centos.OldCentOSConfigurator + + if rhel_derived_os(os_name): + old = util.parse_loose_version(os_version) < util.parse_loose_version('9') + if old: + return override_centos.OldCentOSConfigurator + else: + return override_centos.CentOSConfigurator try: override_class = OVERRIDE_CLASSES[os_name] diff --git a/certbot-apache/certbot_apache/_internal/override_centos.py b/certbot-apache/certbot_apache/_internal/override_centos.py index 5af0f18b3..fbc2cd50d 100644 --- a/certbot-apache/certbot_apache/_internal/override_centos.py +++ b/certbot-apache/certbot_apache/_internal/override_centos.py @@ -16,22 +16,9 @@ from certbot.errors import MisconfigurationError logger = logging.getLogger(__name__) -class CentOSConfigurator(configurator.ApacheConfigurator): +class BaseCentOSConfigurator(configurator.ApacheConfigurator): """CentOS specific ApacheConfigurator override class""" - OS_DEFAULTS = OsOptions( - server_root="/etc/httpd", - vhost_root="/etc/httpd/conf.d", - vhost_files="*.conf", - logs_root="/var/log/httpd", - ctl="apachectl", - version_cmd=['apachectl', '-v'], - restart_cmd=['apachectl', 'graceful'], - restart_cmd_alt=['apachectl', 'restart'], - conftest_cmd=['apachectl', 'configtest'], - challenge_location="/etc/httpd/conf.d", - ) - def config_test(self) -> None: """ Override config_test to mitigate configtest error in vanilla installation @@ -64,29 +51,6 @@ class CentOSConfigurator(configurator.ApacheConfigurator): # Finish with actual config check to see if systemctl restart helped super().config_test() - def _prepare_options(self) -> None: - """ - Override the options dictionary initialization in order to support - alternative restart cmd used in CentOS. - """ - super()._prepare_options() - if not self.options.restart_cmd_alt: # pragma: no cover - raise ValueError("OS option restart_cmd_alt must be set for CentOS.") - - # Make sure new versions (>=9) of RHEL and other derived distros (that - # track RHEL's versioning scheme) use the httpd executable - os_name, os_version = util.get_os_info() - rhel_derived = os_name not in [ - "fedora", - "amazon", # Amazon has a yearly release version - ] - new = util.parse_loose_version(os_version) >= util.parse_loose_version('9') - if rhel_derived and new: - self.options.ctl = 'httpd' - self.options.version_cmd[0] = self.options.ctl - else: - self.options.restart_cmd_alt[0] = self.options.ctl - def get_parser(self) -> "CentOSParser": """Initializes the ApacheParser""" return CentOSParser( @@ -169,6 +133,59 @@ class CentOSConfigurator(configurator.ApacheConfigurator): "inside of block.\n") +class CentOSConfigurator(BaseCentOSConfigurator): + + OS_DEFAULTS = OsOptions( + server_root="/etc/httpd", + vhost_root="/etc/httpd/conf.d", + vhost_files="*.conf", + logs_root="/var/log/httpd", + ctl="httpd", + version_cmd=['httpd', '-v'], + restart_cmd=['apachectl', 'graceful'], + restart_cmd_alt=['apachectl', 'restart'], + conftest_cmd=['apachectl', 'configtest'], + challenge_location="/etc/httpd/conf.d", + ) + + def _prepare_options(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 + """ + 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.") + + +class OldCentOSConfigurator(BaseCentOSConfigurator): + + OS_DEFAULTS = OsOptions( + server_root="/etc/httpd", + vhost_root="/etc/httpd/conf.d", + vhost_files="*.conf", + logs_root="/var/log/httpd", + ctl="apachectl", + version_cmd=['apachectl', '-v'], + restart_cmd=['apachectl', 'graceful'], + restart_cmd_alt=['apachectl', 'restart'], + conftest_cmd=['apachectl', 'configtest'], + challenge_location="/etc/httpd/conf.d", + ) + + def _prepare_options(self) -> None: + """ + Override the options dictionary initialization in order to support + alternative restart cmd used in CentOS. + """ + super()._prepare_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.restart_cmd_alt[0] = self.options.ctl + + class CentOSParser(parser.ApacheParser): """CentOS specific ApacheParser override class""" def __init__(self, *args: Any, **kwargs: Any) -> None: diff --git a/certbot-apache/tests/centos6_test.py b/certbot-apache/tests/centos6_test.py index 85f1333e9..d41bab896 100644 --- a/certbot-apache/tests/centos6_test.py +++ b/certbot-apache/tests/centos6_test.py @@ -43,7 +43,7 @@ class CentOS6Tests(util.ApacheTest): self.config = util.get_apache_configurator( self.config_path, self.vhost_path, self.config_dir, self.work_dir, - version=(2, 2, 15), os_info="centos") + version=(2, 2, 15), config_class=override_centos.OldCentOSConfigurator) self.vh_truth = get_vh_truth( self.temp_dir, "centos6_apache/apache") diff --git a/certbot-apache/tests/centos_test.py b/certbot-apache/tests/centos_test.py index fb7e5dc5c..3cfc27395 100644 --- a/certbot-apache/tests/centos_test.py +++ b/certbot-apache/tests/centos_test.py @@ -47,12 +47,13 @@ class FedoraRestartTest(util.ApacheTest): vhost_root=vhost_root) self.config = util.get_apache_configurator( self.config_path, self.vhost_path, self.config_dir, self.work_dir, - os_info="fedora_old") + os_info="fedora_old", + config_class=override_centos.OldCentOSConfigurator) self.vh_truth = get_vh_truth( self.temp_dir, "centos7_apache/apache") def _run_fedora_test(self): - self.assertIsInstance(self.config, override_centos.CentOSConfigurator) + self.assertIsInstance(self.config, override_centos.OldCentOSConfigurator) with mock.patch("certbot.util.get_os_info") as mock_info: mock_info.return_value = ["fedora", "28"] self.config.config_test() @@ -99,27 +100,21 @@ class UseCorrectApacheExecutableTest(util.ApacheTest): config_root=config_root, vhost_root=vhost_root) - @mock.patch("certbot.util.get_os_info") - def test_old_centos_rhel_and_fedora(self, mock_get_os_info): - for os_info in [("centos", "7"), ("rhel", "7"), ("fedora", "28"), ("scientific", "6")]: - mock_get_os_info.return_value = os_info - config = util.get_apache_configurator( - self.config_path, self.vhost_path, self.config_dir, self.work_dir, - os_info="centos") - self.assertEqual(config.options.ctl, "apachectl") - self.assertEqual(config.options.version_cmd, ["apachectl", "-v"]) - self.assertEqual(config.options.restart_cmd, ["apachectl", "graceful"]) + def test_old_centos(self): + config = util.get_apache_configurator( + self.config_path, self.vhost_path, self.config_dir, self.work_dir, + config_class=override_centos.OldCentOSConfigurator) + self.assertEqual(config.options.ctl, "apachectl") + self.assertEqual(config.options.version_cmd, ["apachectl", "-v"]) + self.assertEqual(config.options.restart_cmd, ["apachectl", "graceful"]) - @mock.patch("certbot.util.get_os_info") - def test_new_rhel_derived(self, mock_get_os_info): - for os_info in [("centos", "9"), ("rhel", "9"), ("oracle", "9")]: - mock_get_os_info.return_value = os_info - config = util.get_apache_configurator( - self.config_path, self.vhost_path, self.config_dir, self.work_dir, - os_info="centos") - self.assertEqual(config.options.ctl, "httpd") - self.assertEqual(config.options.version_cmd, ["httpd", "-v"]) - self.assertEqual(config.options.restart_cmd, ["apachectl", "graceful"]) + def test_new_rhel_derived(self): + 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.version_cmd, ["httpd", "-v"]) + self.assertEqual(config.options.restart_cmd, ["apachectl", "graceful"]) class MultipleVhostsTestCentOS(util.ApacheTest): @@ -138,7 +133,7 @@ class MultipleVhostsTestCentOS(util.ApacheTest): mock_get_os_info.return_value = ("centos", "9") self.config = util.get_apache_configurator( self.config_path, self.vhost_path, self.config_dir, self.work_dir, - os_info="centos") + config_class=override_centos.CentOSConfigurator) self.vh_truth = get_vh_truth( self.temp_dir, "centos7_apache/apache") diff --git a/certbot-apache/tests/entrypoint_test.py b/certbot-apache/tests/entrypoint_test.py index 2a2694415..ca2481fce 100644 --- a/certbot-apache/tests/entrypoint_test.py +++ b/certbot-apache/tests/entrypoint_test.py @@ -8,6 +8,7 @@ except ImportError: # pragma: no cover from certbot_apache._internal import configurator from certbot_apache._internal import entrypoint +from certbot_apache._internal import override_centos class EntryPointTest(unittest.TestCase): @@ -28,6 +29,20 @@ class EntryPointTest(unittest.TestCase): self.assertEqual(entrypoint.get_configurator(), entrypoint.OVERRIDE_CLASSES[distro]) + @mock.patch("certbot.util.get_os_info") + def test_old_centos_rhel_and_fedora(self, mock_get_os_info): + for os_info in [("centos", "7"), ("rhel", "7"), ("fedora", "28"), ("scientific", "6")]: + mock_get_os_info.return_value = os_info + self.assertEqual(entrypoint.get_configurator(), + override_centos.OldCentOSConfigurator) + + @mock.patch("certbot.util.get_os_info") + def test_new_rhel_derived(self, mock_get_os_info): + for os_info in [("centos", "9"), ("rhel", "9"), ("oracle", "9")]: + mock_get_os_info.return_value = os_info + self.assertEqual(entrypoint.get_configurator(), + override_centos.CentOSConfigurator) + def test_nonexistent_like(self): with mock.patch("certbot.util.get_os_info") as mock_info: mock_info.return_value = ("nonexistent", "irrelevant") diff --git a/certbot-apache/tests/util.py b/certbot-apache/tests/util.py index a4191b3fe..601605314 100644 --- a/certbot-apache/tests/util.py +++ b/certbot-apache/tests/util.py @@ -82,7 +82,8 @@ def get_apache_configurator( os_info="generic", conf_vhost_path=None, use_parsernode=False, - openssl_version="1.1.1a"): + openssl_version="1.1.1a", + config_class=None): """Create an Apache Configurator with the specified options. :param conf: Function that returns binary paths. self.conf in Configurator @@ -110,10 +111,11 @@ def get_apache_configurator( "update_runtime_variables"): with mock.patch("certbot_apache._internal.apache_util.parse_from_subprocess") as mock_sp: mock_sp.return_value = [] - try: - config_class = entrypoint.OVERRIDE_CLASSES[os_info] - except KeyError: - config_class = configurator.ApacheConfigurator + if config_class is None: + 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, use_parsernode=use_parsernode, openssl_version=openssl_version)