Move more debian specific functionality to override and add caller access for overrides

This commit is contained in:
Joona Hoikkala 2017-10-10 00:05:13 +03:00
parent d371b68e9f
commit 184925d241
6 changed files with 59 additions and 96 deletions

View file

@ -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.

View file

@ -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)

View file

@ -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:

View file

@ -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.

View file

@ -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:

View file

@ -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")