diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index 6b685bdaa..2c2906f28 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/certbot-apache/certbot_apache/configurator.py @@ -1712,14 +1712,23 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): @constants.override def enable_site(self, vhost): - """Handles enabling the new VirtualHost if underlying OS uses such - configuration scheme. The implementation is distribution specific, - and handled in the distribution specific overrides. + """Enables an available site, Apache reload required. + + .. note:: Does not make sure that the site correctly works or that all + modules are enabled appropriately. + .. note:: The distribution specific override replaces functionality + of this method where available. + + :param vhost: vhost to enable + :type vhost: :class:`~certbot_apache.obj.VirtualHost` + + :raises .errors.NotSupportedError: If filesystem layout is not + supported. + """ if vhost.enabled: return - # Handle non-debian systems if not self.parser.parsed_in_original(vhost.filep): # Add direct include to root conf logger.info("Enabling site %s by adding Include to root configuration", @@ -1729,7 +1738,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): vhost.enabled = True return - def enable_mod(self, mod_name, temp=False): + @constants.override + def enable_mod(self, mod_name, temp=False): # pylint: disable=unused-argument """Enables module in Apache. Both enables and reloads Apache so module is active. @@ -1737,65 +1747,26 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): :param str mod_name: Name of the module to enable. (e.g. 'ssl') :param bool temp: Whether or not this is a temporary action. + .. note:: The distribution specific override replaces functionality + of this method where available. + :raises .errors.NotSupportedError: If the filesystem layout is not supported. :raises .errors.MisconfigurationError: If a2enmod or a2dismod cannot be run. """ - # Support Debian specific setup - avail_path = os.path.join(self.parser.root, "mods-available") - enabled_path = os.path.join(self.parser.root, "mods-enabled") - if not os.path.isdir(avail_path) or not os.path.isdir(enabled_path): - raise errors.NotSupportedError( - "Unsupported directory layout. You may try to enable mod %s " - "and try again." % mod_name) + mod_message = ("Apache needs to have module \"{0}\" active for the " + + "requested installation options. Unfortunately Certbot is unable " + + "to install or enable it for you. Please install the module, and " + + "run Certbot again.") + raise errors.MisconfigurationError(mod_message.format(mod_name)) - deps = apache_util.get_mod_deps(mod_name) - - # Enable all dependencies - for dep in deps: - if (dep + "_module") not in self.parser.modules: - self._enable_mod_debian(dep, temp) - self._add_parser_mod(dep) - - note = "Enabled dependency of %s module - %s" % (mod_name, dep) - if not temp: - self.save_notes += note + os.linesep - logger.debug(note) - - # Enable actual module - self._enable_mod_debian(mod_name, temp) - self._add_parser_mod(mod_name) - - if not temp: - self.save_notes += "Enabled %s module in Apache\n" % mod_name - logger.info("Enabled Apache %s module", mod_name) - - # Modules can enable additional config files. Variables may be defined - # within these new configuration sections. - # Reload is not necessary as DUMP_RUN_CFG uses latest config. - self.parser.update_runtime_variables() - - def _add_parser_mod(self, mod_name): + def add_parser_mod(self, mod_name): """Shortcut for updating parser modules.""" self.parser.modules.add(mod_name + "_module") self.parser.modules.add("mod_" + mod_name + ".c") - def _enable_mod_debian(self, mod_name, temp): - """Assumes mods-available, mods-enabled layout.""" - # Generate reversal command. - # Try to be safe here... check that we can probably reverse before - # applying enmod command - if not util.exe_exists(self.conf("dismod")): - raise errors.MisconfigurationError( - "Unable to find a2dismod, please make sure a2enmod and " - "a2dismod are configured correctly for certbot.") - - self.reverter.register_undo_command( - temp, [self.conf("dismod"), mod_name]) - util.run_script([self.conf("enmod"), mod_name]) - def restart(self): """Runs a config test and reloads the Apache server. diff --git a/certbot-apache/certbot_apache/constants.py b/certbot-apache/certbot_apache/constants.py index 8e116d971..56ce2df4e 100644 --- a/certbot-apache/certbot_apache/constants.py +++ b/certbot-apache/certbot_apache/constants.py @@ -268,8 +268,9 @@ def override(method): overriding method if found, in other case, return the default""" try: # Try to find overriding method + caller = {"class": caller_class, "method": method} return getattr(caller_class.os_info, - method.__name__)(*args, **kwargs) + method.__name__)(caller, *args, **kwargs) except AttributeError: # Override not found, return the default return method(caller_class, *args, **kwargs) diff --git a/certbot-apache/certbot_apache/override_debian.py b/certbot-apache/certbot_apache/override_debian.py index 9ed9f3d09..741a4fc27 100644 --- a/certbot-apache/certbot_apache/override_debian.py +++ b/certbot-apache/certbot_apache/override_debian.py @@ -19,17 +19,12 @@ class Override(object): """ self.config = config - def enable_site(self, vhost): + def enable_site(self, caller, vhost): """Enables an available site, Apache reload required. .. note:: Does not make sure that the site correctly works or that all modules are enabled appropriately. - .. todo:: This function should number subdomains before the domain - vhost - - .. todo:: Make sure link is not broken... - :param vhost: vhost to enable :type vhost: :class:`~certbot_apache.obj.VirtualHost` @@ -43,6 +38,10 @@ class Override(object): enabled_path = ("%s/sites-enabled/%s" % (self.config.parser.root, os.path.basename(vhost.filep))) + if not os.path.isdir(os.path.dirname(enabled_path)): + # For some reason, sites-enabled / sites-available do not exist + # Call the original caller method + caller["method"](caller["class"], vhost) self.config.reverter.register_file_creation(False, enabled_path) try: os.symlink(vhost.filep, enabled_path) @@ -65,7 +64,8 @@ class Override(object): logger.info("Enabling available site: %s", vhost.filep) self.config.save_notes += "Enabled site %s\n" % vhost.filep - def enable_mod(self, mod_name, temp=False): + def enable_mod(self, caller, mod_name, temp=False): + # pylint: disable=unused-argument """Enables module in Apache. Both enables and reloads Apache so module is active. @@ -87,6 +87,7 @@ class Override(object): "and try again." % mod_name) deps = apache_util.get_mod_deps(mod_name) + # Enable all dependencies for dep in deps: if (dep + "_module") not in self.config.parser.modules: diff --git a/certbot-apache/certbot_apache/parser.py b/certbot-apache/certbot_apache/parser.py index b15608d61..6d4c85dae 100644 --- a/certbot-apache/certbot_apache/parser.py +++ b/certbot-apache/certbot_apache/parser.py @@ -19,8 +19,6 @@ logger = logging.getLogger(__name__) class ApacheParser(object): """Class handles the fine details of parsing the Apache Configuration. - .. todo:: Make parsing general... remove sites-available etc... - :ivar str root: Normalized absolute path to the server root directory. Without trailing slash. :ivar set modules: All module names that are currently enabled. diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py index 0ef47caaa..c96c25ee4 100644 --- a/certbot-apache/certbot_apache/tests/configurator_test.py +++ b/certbot-apache/certbot_apache/tests/configurator_test.py @@ -347,7 +347,10 @@ class MultipleVhostsTest(util.ApacheTest): def test_enable_mod_nonexistent(self): from certbot_apache import override_centos self.config.os_info = override_centos.Override(self.config) - self.assertEqual(self.config.enable_mod("ssl"), None) + self.assertRaises( + errors.MisconfigurationError, + self.config.enable_mod, + "whatever") def test_enable_site_already_enabled(self): self.assertTrue(self.vh_truth[1].enabled) @@ -366,28 +369,22 @@ class MultipleVhostsTest(util.ApacheTest): self.assertEqual(self.config.enable_site(self.vh_truth[1]), None) def test_enable_site_nondebian(self): - mock_c = "certbot_apache.configurator.ApacheConfigurator.conf" - def conf_side_effect(arg): - """ Mock function for ApacheConfigurator.conf """ - confvars = {"handle-sites": False} - if arg in confvars: - return confvars[arg] + from certbot_apache import override_centos + self.config.os_info = override_centos.Override(self.config) inc_path = "/path/to/whereever" vhost = self.vh_truth[0] - with mock.patch(mock_c) as mock_conf: - mock_conf.side_effect = conf_side_effect - vhost.enabled = False - vhost.filep = inc_path - self.assertFalse(self.config.parser.find_dir("Include", inc_path)) - self.assertFalse( - os.path.dirname(inc_path) in self.config.parser.existing_paths) - self.config.enable_site(vhost) - self.assertTrue(self.config.parser.find_dir("Include", inc_path)) - self.assertTrue( - os.path.dirname(inc_path) in self.config.parser.existing_paths) - self.assertTrue( - os.path.basename(inc_path) in self.config.parser.existing_paths[ - os.path.dirname(inc_path)]) + vhost.enabled = False + vhost.filep = inc_path + self.assertFalse(self.config.parser.find_dir("Include", inc_path)) + self.assertFalse( + os.path.dirname(inc_path) in self.config.parser.existing_paths) + self.config.enable_site(vhost) + self.assertTrue(self.config.parser.find_dir("Include", inc_path)) + self.assertTrue( + os.path.dirname(inc_path) in self.config.parser.existing_paths) + self.assertTrue( + os.path.basename(inc_path) in self.config.parser.existing_paths[ + os.path.dirname(inc_path)]) def test_deploy_cert_enable_new_vhost(self): # Create @@ -478,25 +475,19 @@ class MultipleVhostsTest(util.ApacheTest): def test_deploy_cert_not_parsed_path(self): # Make sure that we add include to root config for vhosts when - # handle-sites is false + # running Debian, but missing sites-enabled structure self.config.parser.modules.add("ssl_module") self.config.parser.modules.add("mod_ssl.c") tmp_path = os.path.realpath(tempfile.mkdtemp("vhostroot")) os.chmod(tmp_path, 0o755) mock_p = "certbot_apache.configurator.ApacheConfigurator._get_ssl_vhost_path" mock_a = "certbot_apache.parser.ApacheParser.add_include" - mock_c = "certbot_apache.configurator.ApacheConfigurator.conf" - orig_conf = self.config.conf - def conf_side_effect(arg): - """ Mock function for ApacheConfigurator.conf """ - confvars = {"handle-sites": False} - if arg in confvars: - return confvars[arg] - else: - return orig_conf("arg") - with mock.patch(mock_c) as mock_conf: - mock_conf.side_effect = conf_side_effect + from certbot_apache import override_debian + self.config.os_info = override_debian.Override(self.config) + + with mock.patch("os.path.isdir") as mock_osp: + mock_osp.return_value = False with mock.patch(mock_p) as mock_path: mock_path.return_value = os.path.join(tmp_path, "whatever.conf") with mock.patch(mock_a) as mock_add: diff --git a/certbot-apache/certbot_apache/tests/constants_test.py b/certbot-apache/certbot_apache/tests/constants_test.py index 5ab324101..ba35a6e7b 100644 --- a/certbot-apache/certbot_apache/tests/constants_test.py +++ b/certbot-apache/certbot_apache/tests/constants_test.py @@ -29,6 +29,7 @@ class ConstantsTest(unittest.TestCase): self.assertEqual(constants.os_constant("server_root"), "/etc/apache2") self.assertEqual(constants.os_constant("vhost_root"), "/etc/apache2/sites-available") + self.assertEqual(constants.get_override(None), None) @mock.patch("certbot.util.get_systemd_os_like") @mock.patch("certbot.util.get_os_info")