Final fixes

This commit is contained in:
Joona Hoikkala 2017-01-24 15:23:21 +02:00
parent 40bb890589
commit 03028eee47
No known key found for this signature in database
GPG key ID: C14AAE0F5ADCB854
4 changed files with 49 additions and 39 deletions

View file

@ -145,7 +145,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
return os.path.join(self.config.config_dir,
constants.MOD_SSL_CONF_DEST)
def prepare(self):
"""Prepare the authenticator/installer.
@ -288,11 +287,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
if chain_path is not None:
self.save_notes += "\tSSLCertificateChainFile %s\n" % chain_path
# Make sure vhost is enabled if distro with enabled / available
if self.conf("handle-sites"):
if not vhost.enabled:
self.enable_site(vhost)
def choose_vhost(self, target_name, temp=False):
"""Chooses a virtual host based on the given domain name.
@ -589,15 +583,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
if realpath not in vhost_paths.keys():
vhs.append(new_vhost)
vhost_paths[realpath] = new_vhost.filep
elif realpath == new_vhost.filep:
# Prefer "real" vhost paths instead of symlinked ones
# ex: sites-enabled/vh.conf -> sites-available/vh.conf
# remove old (most likely) symlinked one
vhs = [v for v in vhs if v.filep != vhost_paths[realpath]]
vhs.append(new_vhost)
vhost_paths[realpath] = realpath
return vhs
def is_name_vhost(self, target_addr):
@ -836,10 +821,18 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
# This is for compliance with versions of Apache < 2.4
self._add_name_vhost_if_necessary(ssl_vhost)
# Enable the new vhost if needed
if self.conf("handle-sites"):
if not self.is_site_enabled(ssl_fp):
self.enable_site(ssl_vhost)
return ssl_vhost
def _get_ssl_vhost_path(self, non_ssl_vh_fp):
# Get filepath of new ssl_vhost
# Make sure we use the realpath (eg. sites-available instead of
# sites-enabled
non_ssl_vh_fp = os.path.realpath(non_ssl_vh_fp)
if non_ssl_vh_fp.endswith(".conf"):
return non_ssl_vh_fp[:-(len(".conf"))] + self.conf("le_vhost_ext")
else:

View file

@ -618,7 +618,6 @@ class ApacheParser(object):
for name in location:
if os.path.isfile(os.path.join(self.root, name)):
return os.path.join(self.root, name)
raise errors.NoInstallationError("Could not find configuration root")

View file

@ -92,6 +92,29 @@ class MultipleVhostsTest(util.ApacheTest):
self.assertRaises(
errors.NotSupportedError, self.config.prepare)
@mock.patch("certbot_apache.parser.ApacheParser._parse_file")
@mock.patch("certbot_apache.parser.ApacheParser.update_runtime_variables")
def test_prepare_custom_vhost_dir(self, _, mock_parsefile):
server_root = self.config.conf("server-root")
vhost_root = self.config.conf("vhost-root")
version = self.config.version
with mock.patch("certbot_apache.constants.os_constant") as mock_const:
mock_const.side_effect = [True, "*.conf"]
parser.ApacheParser(self.config.aug,
server_root,
vhost_root,
version)
self.assertEqual(mock_parsefile.call_count, 9)
mock_parsefile.reset_mock()
with mock.patch("certbot_apache.constants.os_constant") as mock_const:
mock_const.side_effect = [False, "*.conf"]
parser.ApacheParser(self.config.aug,
server_root,
vhost_root.replace(
"sites-available", "sites-enabled"),
self.config.version)
self.assertEqual(mock_parsefile.call_count, 10)
def test_add_parser_arguments(self): # pylint: disable=no-self-use
from certbot_apache.configurator import ApacheConfigurator
@ -163,9 +186,7 @@ class MultipleVhostsTest(util.ApacheTest):
found += 1
break
else:
if "default-ssl-port-only.conf" not in vhost.filep:
# Ignore default-ssl-port-only without ServerName or Alias
raise Exception("Missed: %s" % vhost) # pragma: no cover
raise Exception("Missed: %s" % vhost) # pragma: no cover
self.assertEqual(found, 8)
@ -320,14 +341,9 @@ class MultipleVhostsTest(util.ApacheTest):
self.assertRaises(
errors.MisconfigurationError, self.config.enable_mod, "ssl")
# def test_enable_site(self):
# Default 443 vhost
# self.assertFalse(self.vh_truth[8].enabled)
# self.config.enable_site(self.vh_truth[8])
# self.assertTrue(self.vh_truth[8].enabled)
# Go again to make sure nothing fails
#self.config.enable_site(self.vh_truth[8])
def test_enable_site_already_enabled(self):
self.assertTrue(self.vh_truth[1].enabled)
self.config.enable_site(self.vh_truth[1])
def test_enable_site_failure(self):
self.assertRaises(
@ -364,12 +380,12 @@ class MultipleVhostsTest(util.ApacheTest):
# Verify one directive was found in the correct file
self.assertEqual(len(loc_cert), 1)
self.assertEqual(
os.path.realpath(configurator.get_file_path(loc_cert[0])),
configurator.get_file_path(loc_cert[0]),
self.vh_truth[1].filep)
self.assertEqual(len(loc_key), 1)
self.assertEqual(
os.path.realpath(configurator.get_file_path(loc_key[0])),
configurator.get_file_path(loc_key[0]),
self.vh_truth[1].filep)
def test_deploy_cert_newssl_no_fullchain(self):
@ -430,17 +446,17 @@ class MultipleVhostsTest(util.ApacheTest):
# Verify one directive was found in the correct file
self.assertEqual(len(loc_cert), 1)
self.assertEqual(
os.path.realpath(configurator.get_file_path(loc_cert[0])),
configurator.get_file_path(loc_cert[0]),
self.vh_truth[1].filep)
self.assertEqual(len(loc_key), 1)
self.assertEqual(
os.path.realpath(configurator.get_file_path(loc_key[0])),
configurator.get_file_path(loc_key[0]),
self.vh_truth[1].filep)
self.assertEqual(len(loc_chain), 1)
self.assertEqual(
os.path.realpath(configurator.get_file_path(loc_chain[0])),
configurator.get_file_path(loc_chain[0]),
self.vh_truth[1].filep)
# One more time for chain directive setting
@ -592,7 +608,7 @@ class MultipleVhostsTest(util.ApacheTest):
self.assertEqual(set([obj.Addr.fromstring("*:443")]), ssl_vhost.addrs)
self.assertEqual(ssl_vhost.name, "encryption-example.demo")
self.assertTrue(ssl_vhost.ssl)
self.assertFalse(ssl_vhost.enabled)
self.assertTrue(ssl_vhost.enabled)
self.assertTrue(self.config.parser.find_dir(
"SSLCertificateFile", None, ssl_vhost.path, False))
@ -873,10 +889,10 @@ class MultipleVhostsTest(util.ApacheTest):
"SSLUseStapling", "on", ssl_vhost.path)
self.assertEqual(len(ssl_use_stapling_aug_path), 1)
ssl_vhost_aug_path = parser.get_aug_path(ssl_vhost.filep)
stapling_cache_aug_path = self.config.parser.find_dir('SSLStaplingCache',
"shmcb:/var/run/apache2/stapling_cache(128000)",
ssl_vhost.path)
ssl_vhost_aug_path)
self.assertEqual(len(stapling_cache_aug_path), 1)
@ -1294,13 +1310,17 @@ class AugeasVhostsTest(util.ApacheTest):
for name in names:
self.assertFalse(name in self.config.choose_vhost(name).aliases)
def test_choose_vhost_without_matching_wildcard(self):
@mock.patch("certbot_apache.obj.VirtualHost.conflicts")
def test_choose_vhost_without_matching_wildcard(self, mock_conflicts):
mock_conflicts.return_value = False
mock_path = "certbot_apache.display_ops.select_vhost"
with mock.patch(mock_path, lambda _, vhosts: vhosts[0]):
for name in ("a.example.net", "other.example.net"):
self.assertTrue(name in self.config.choose_vhost(name).aliases)
def test_choose_vhost_wildcard_not_found(self):
@mock.patch("certbot_apache.obj.VirtualHost.conflicts")
def test_choose_vhost_wildcard_not_found(self, mock_conflicts):
mock_conflicts.return_value = False
mock_path = "certbot_apache.display_ops.select_vhost"
names = (
"abc.example.net", "not.there.tld", "aa.wildcard.tld"

View file

@ -121,8 +121,6 @@ def get_vh_truth(temp_dir, config_name):
prefix = os.path.join(
temp_dir, config_name, "apache2/sites-enabled")
alt_prefix = os.path.join(
temp_dir, config_name, "apache2/sites-available")
aug_pre = "/files" + prefix
vh_truth = [
obj.VirtualHost(