mirror of
https://github.com/certbot/certbot.git
synced 2026-06-14 19:20:09 -04:00
Address review comments
This commit is contained in:
Joona Hoikkala
parent
18d0e14101
commit
f01a1a6fc7
3 changed files with 55 additions and 45 deletions
|
|
@ -12,7 +12,7 @@ from certbot import interfaces
|
|||
from certbot_apache import apache_util
|
||||
from certbot_apache import configurator
|
||||
from certbot_apache import parser
|
||||
from certbot.errors import ConfigurationError
|
||||
from certbot.errors import MisconfigurationError
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
|
@ -82,26 +82,25 @@ class CentOSConfigurator(configurator.ApacheConfigurator):
|
|||
path_args = self.parser.get_all_args(noarg_path)
|
||||
if loadmod_args:
|
||||
if loadmod_args != path_args:
|
||||
msg = ("Certbot encountered multiple different LoadModule directives "
|
||||
"for LoadModule ssl_module. Please remove or comment out the "
|
||||
"one(s) that are not in use.")
|
||||
raise ConfigurationError(msg)
|
||||
msg = ("Certbot encountered multiple LoadModule directives "
|
||||
"for LoadModule ssl_module with differing library paths. "
|
||||
"Please remove or comment out the one(s) that are not in "
|
||||
"use, and run Certbot again.")
|
||||
raise MisconfigurationError(msg)
|
||||
else:
|
||||
loadmod_args = path_args
|
||||
|
||||
loadmod_paths.append(noarg_path)
|
||||
|
||||
if self.parser.not_modssl_ifmodule(noarg_path): # pylint: disable=no-member
|
||||
# Populate the list of known !mod_ssl.c IfModules
|
||||
nodir_path = noarg_path.rpartition("/directive")[0]
|
||||
correct_ifmods.append(nodir_path)
|
||||
|
||||
if self.parser.loc["default"] in noarg_path:
|
||||
if self.parser.not_modssl_ifmodule(noarg_path): #pylint: disable=no-member
|
||||
if self.parser.loc["default"] in noarg_path:
|
||||
# LoadModule already in the main configuration file
|
||||
if "ifmodule/" in noarg_path or "ifmodule[1]" in noarg_path.lower():
|
||||
# It's the first or only IfModule in the file
|
||||
return
|
||||
# Populate the list of known !mod_ssl.c IfModules
|
||||
nodir_path = noarg_path.rpartition("/directive")[0]
|
||||
correct_ifmods.append(nodir_path)
|
||||
else:
|
||||
loadmod_paths.append(noarg_path)
|
||||
|
||||
if not loadmod_args:
|
||||
# Do not try to enable mod_ssl
|
||||
|
|
@ -109,9 +108,9 @@ class CentOSConfigurator(configurator.ApacheConfigurator):
|
|||
|
||||
# Force creation as the directive wasn't found from the beginning of
|
||||
# httpd.conf
|
||||
rootconf_ifmod = self.parser.get_ifmod(
|
||||
rootconf_ifmod = self.parser.create_ifmod(
|
||||
parser.get_aug_path(self.parser.loc["default"]),
|
||||
"!mod_ssl.c", beginning=True, force_create=True)
|
||||
"!mod_ssl.c", beginning=True)
|
||||
# parser.get_ifmod returns a path postfixed with "/", remove that
|
||||
self.parser.add_dir(rootconf_ifmod[:-1], "LoadModule", loadmod_args)
|
||||
correct_ifmods.append(rootconf_ifmod[:-1])
|
||||
|
|
@ -121,19 +120,17 @@ class CentOSConfigurator(configurator.ApacheConfigurator):
|
|||
# configured like this already.
|
||||
for loadmod_path in loadmod_paths:
|
||||
nodir_path = loadmod_path.split("/directive")[0]
|
||||
# Only create if !mod_ssl.c ifmodule not in path
|
||||
if not self.parser.not_modssl_ifmodule(loadmod_path): # pylint: disable=no-member
|
||||
# Remove the old LoadModule directive
|
||||
self.aug.remove(loadmod_path)
|
||||
# Remove the old LoadModule directive
|
||||
self.aug.remove(loadmod_path)
|
||||
|
||||
# Create a new IfModule !mod_ssl.c if not already found on path
|
||||
ssl_ifmod = self.parser.get_ifmod(nodir_path, "!mod_ssl.c",
|
||||
beginning=True)[:-1]
|
||||
if ssl_ifmod not in correct_ifmods:
|
||||
self.parser.add_dir(ssl_ifmod, "LoadModule", loadmod_args)
|
||||
correct_ifmods.append(ssl_ifmod)
|
||||
self.save_notes += ("Wrapped pre-existing LoadModule ssl_module "
|
||||
"inside of <IfModule !mod_ssl> block.\n")
|
||||
# Create a new IfModule !mod_ssl.c if not already found on path
|
||||
ssl_ifmod = self.parser.get_ifmod(nodir_path, "!mod_ssl.c",
|
||||
beginning=True)[:-1]
|
||||
if ssl_ifmod not in correct_ifmods:
|
||||
self.parser.add_dir(ssl_ifmod, "LoadModule", loadmod_args)
|
||||
correct_ifmods.append(ssl_ifmod)
|
||||
self.save_notes += ("Wrapped pre-existing LoadModule ssl_module "
|
||||
"inside of <IfModule !mod_ssl> block.\n")
|
||||
|
||||
|
||||
class CentOSParser(parser.ApacheParser):
|
||||
|
|
@ -175,7 +172,7 @@ class CentOSParser(parser.ApacheParser):
|
|||
if parts[2].startswith("["):
|
||||
# Append the index from tail
|
||||
ifmod_path += parts[2].partition("/")[0]
|
||||
# Get the origianl path trimmed to correct length
|
||||
# Get the original path trimmed to correct length
|
||||
# This is required to preserve cases
|
||||
ifmod_real_path = path[0:len(ifmod_path)]
|
||||
if "!mod_ssl.c" in self.get_all_args(ifmod_real_path):
|
||||
|
|
|
|||
|
|
@ -292,7 +292,7 @@ class ApacheParser(object):
|
|||
for i, arg in enumerate(args):
|
||||
self.aug.set("%s/arg[%d]" % (nvh_path, i + 1), arg)
|
||||
|
||||
def get_ifmod(self, aug_conf_path, mod, beginning=False, force_create=False):
|
||||
def get_ifmod(self, aug_conf_path, mod, beginning=False):
|
||||
"""Returns the path to <IfMod mod> and creates one if it doesn't exist.
|
||||
|
||||
:param str aug_conf_path: Augeas configuration path
|
||||
|
|
@ -303,21 +303,34 @@ class ApacheParser(object):
|
|||
"""
|
||||
if_mods = self.aug.match(("%s/IfModule/*[self::arg='%s']" %
|
||||
(aug_conf_path, mod)))
|
||||
if len(if_mods) == 0 or force_create:
|
||||
if beginning:
|
||||
c_path_arg = "{}/IfModule[1]/arg".format(aug_conf_path)
|
||||
# Insert IfModule before the first directive
|
||||
self.aug.insert("{}/directive[1]".format(aug_conf_path),
|
||||
"IfModule", True)
|
||||
else:
|
||||
c_path = "{}/IfModule[last() + 1]".format(aug_conf_path)
|
||||
c_path_arg = "{}/IfModule[last()]/arg".format(aug_conf_path)
|
||||
self.aug.set(c_path, "")
|
||||
self.aug.set(c_path_arg, mod)
|
||||
if_mods = self.aug.match(("%s/IfModule/*[self::arg='%s']" %
|
||||
(aug_conf_path, mod)))
|
||||
if not if_mods:
|
||||
return self.create_ifmod(aug_conf_path, mod, beginning)
|
||||
|
||||
# Strip off "arg" at end of first ifmod path
|
||||
return if_mods[0][:len(if_mods[0]) - 3]
|
||||
return if_mods[0].rpartition("arg")[0]
|
||||
|
||||
def create_ifmod(self, aug_conf_path, mod, beginning=False):
|
||||
"""Creates a new <IfMod mod> and returs it path.
|
||||
|
||||
:param str aug_conf_path: Augeas configuration path
|
||||
:param str mod: module ie. mod_ssl.c
|
||||
:param bool beginning: If the IfModule should be created to the beginning
|
||||
of augeas path DOM tree.
|
||||
|
||||
"""
|
||||
if beginning:
|
||||
c_path_arg = "{}/IfModule[1]/arg".format(aug_conf_path)
|
||||
# Insert IfModule before the first directive
|
||||
self.aug.insert("{}/directive[1]".format(aug_conf_path),
|
||||
"IfModule", True)
|
||||
retpath = "{}/IfModule[1]/".format(aug_conf_path)
|
||||
else:
|
||||
c_path = "{}/IfModule[last() + 1]".format(aug_conf_path)
|
||||
c_path_arg = "{}/IfModule[last()]/arg".format(aug_conf_path)
|
||||
self.aug.set(c_path, "")
|
||||
retpath = "{}/IfModule[last()]/".format(aug_conf_path)
|
||||
self.aug.set(c_path_arg, mod)
|
||||
return retpath
|
||||
|
||||
def add_dir(self, aug_conf_path, directive, args):
|
||||
"""Appends directive to the end fo the file given by aug_conf_path.
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ from certbot_apache import obj
|
|||
from certbot_apache import override_centos
|
||||
from certbot_apache import parser
|
||||
from certbot_apache.tests import util
|
||||
from certbot.errors import ConfigurationError
|
||||
from certbot.errors import MisconfigurationError
|
||||
|
||||
def get_vh_truth(temp_dir, config_name):
|
||||
"""Return the ground truth for the specified directory."""
|
||||
|
|
@ -179,7 +179,7 @@ class CentOS6Tests(util.ApacheTest):
|
|||
pre_matches = self.config.parser.find_dir("LoadModule",
|
||||
"ssl_module", exclude=False)
|
||||
|
||||
self.assertRaises(ConfigurationError, self.config.deploy_cert,
|
||||
self.assertRaises(MisconfigurationError, self.config.deploy_cert,
|
||||
"random.demo", "example/cert.pem", "example/key.pem",
|
||||
"example/cert_chain.pem", "example/fullchain.pem")
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue