From 6a8590b4ccaafb375a449f4e6c1f6f6638f503a5 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Sun, 19 Jul 2015 19:49:44 -0700 Subject: [PATCH] Improved Apache and unittests --- .../letsencrypt_apache/configurator.py | 5 +-- .../letsencrypt_apache/dvsni.py | 2 +- letsencrypt-apache/letsencrypt_apache/obj.py | 4 +- .../letsencrypt_apache/parser.py | 42 ++++++------------- .../letsencrypt_apache/tests/obj_test.py | 32 ++++++++++++++ .../letsencrypt_apache/tests/parser_test.py | 21 +++------- .../letsencrypt_apache/tests/util.py | 2 +- 7 files changed, 56 insertions(+), 52 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index af6dad395..07e1bb2bc 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -137,8 +137,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.config_test() self.parser = parser.ApacheParser( - self.aug, self.conf("server-root"), self.mod_ssl_conf, - self.conf("ctl")) + self.aug, self.conf("server-root"), self.conf("ctl")) # Check for errors in parsing files with Augeas self.check_parsing_errors("httpd.aug") @@ -622,7 +621,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): "insert_cert_file_path") self.parser.add_dir(vh_path, "SSLCertificateKeyFile", "insert_key_file_path") - self.parser.add_dir(vh_path, "Include", self.parser.loc["ssl_options"]) + self.parser.add_dir(vh_path, "Include", self.mod_ssl_conf) def _add_name_vhost_if_necessary(self, vhost): """Add NameVirtualHost Directives if necessary for new vhost. diff --git a/letsencrypt-apache/letsencrypt_apache/dvsni.py b/letsencrypt-apache/letsencrypt_apache/dvsni.py index e10c0cbf9..616a1a1ef 100644 --- a/letsencrypt-apache/letsencrypt_apache/dvsni.py +++ b/letsencrypt-apache/letsencrypt_apache/dvsni.py @@ -165,7 +165,7 @@ class ApacheDvsni(common.Dvsni): # https://docs.python.org/2.7/reference/lexical_analysis.html return self.VHOST_TEMPLATE.format( vhost=ips, server_name=achall.nonce_domain, - ssl_options_conf_path=self.configurator.parser.loc["ssl_options"], + ssl_options_conf_path=self.configurator.mod_ssl_conf, cert_path=self.get_cert_path(achall), key_path=self.get_key_path(achall), document_root=document_root).replace("\n", os.linesep) diff --git a/letsencrypt-apache/letsencrypt_apache/obj.py b/letsencrypt-apache/letsencrypt_apache/obj.py index ae84f7d26..a2df429c3 100644 --- a/letsencrypt-apache/letsencrypt_apache/obj.py +++ b/letsencrypt-apache/letsencrypt_apache/obj.py @@ -15,6 +15,9 @@ class Addr(common.Addr): and self.is_wildcard() and other.is_wildcard())) return False + def __ne__(self, other): + return not self.__eq__(other) + def is_wildcard(self): """Returns if address has a wildcard port.""" return self.tup[1] == "*" or not self.tup[1] @@ -35,7 +38,6 @@ class Addr(common.Addr): return self.get_addr_obj(port) - class VirtualHost(object): # pylint: disable=too-few-public-methods """Represents an Apache Virtualhost. diff --git a/letsencrypt-apache/letsencrypt_apache/parser.py b/letsencrypt-apache/letsencrypt_apache/parser.py index 48234bfe5..8feb54fa8 100644 --- a/letsencrypt-apache/letsencrypt_apache/parser.py +++ b/letsencrypt-apache/letsencrypt_apache/parser.py @@ -24,7 +24,7 @@ class ApacheParser(object): arg_var_interpreter = re.compile(r"\$\{[^ \}]*}") fnmatch_chars = set(["*", "?", "\\", "[", "]"]) - def __init__(self, aug, root, ssl_options, ctl): + def __init__(self, aug, root, ctl): # This uses the binary, so it can be done first. # https://httpd.apache.org/docs/2.4/mod/core.html#define # https://httpd.apache.org/docs/2.4/mod/core.html#ifdefine @@ -32,24 +32,27 @@ class ApacheParser(object): self.variables = {} self.update_runtime_variables(ctl) - # Find configuration root and make sure augeas can parse it. self.aug = aug + # Find configuration root and make sure augeas can parse it. self.root = os.path.abspath(root) - self.loc = self._set_locations(ssl_options) + self.loc = {"root": self._find_config_root()} self._parse_file(self.loc["root"]) - # Must also attempt to parse sites-available or equivalent - # Sites-available is not included naturally in configuration - self._parse_file(os.path.join(self.root, "sites-available") + "/*") - # This problem has been fixed in Augeas 1.0 self.standardize_excl() # Temporarily set modules to be empty, so that find_dirs can work # https://httpd.apache.org/docs/2.4/mod/core.html#ifmodule + # This needs to come before locations are set. self.modules = set() self._init_modules() + self.loc.update(self._set_locations(self.loc["root"])) + + # Must also attempt to parse sites-available or equivalent + # Sites-available is not included naturally in configuration + self._parse_file(os.path.join(self.root, "sites-available") + "/*") + def _init_modules(self): """Iterates on the configuration until no new modules are loaded. @@ -493,14 +496,13 @@ class ApacheParser(object): self.aug.load() - def _set_locations(self, ssl_options): + def _set_locations(self, root): """Set default location for directives. Locations are given as file_paths .. todo:: Make sure that files are included """ - root = self._find_config_root() default = self._set_user_config_file(root) temp = os.path.join(self.root, "ports.conf") @@ -511,8 +513,7 @@ class ApacheParser(object): listen = default name = default - return {"root": root, "default": default, "listen": listen, - "name": name, "ssl_options": ssl_options} + return {"default": default, "listen": listen, "name": name} def _find_config_root(self): """Find the Apache Configuration Root file.""" @@ -565,22 +566,3 @@ def get_aug_path(file_path): """ return "/files%s" % file_path - - -def strip_dir(path): - """Returns directory of file path. - - .. todo:: Replace this with Python standard function - - :param str path: path is a file path. not an augeas section or - directive path - - :returns: directory - :rtype: str - - """ - index = path.rfind("/") - if index > 0: - return path[:index+1] - # No directory - return "" diff --git a/letsencrypt-apache/letsencrypt_apache/tests/obj_test.py b/letsencrypt-apache/letsencrypt_apache/tests/obj_test.py index c882588f6..624c86372 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/obj_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/obj_test.py @@ -24,5 +24,37 @@ class VirtualHostTest(unittest.TestCase): self.assertFalse(vhost1b == 1234) +class AddrTest(unittest.TestCase): + """Test obj.Addr.""" + def setUp(self): + from letsencrypt_apache.obj import Addr + self.addr = Addr.fromstring("*:443") + + self.addr1 = Addr.fromstring("127.0.0.1") + self.addr2 = Addr.fromstring("127.0.0.1:*") + + def test_wildcard(self): + self.assertFalse(self.addr.is_wildcard()) + self.assertTrue(self.addr1.is_wildcard()) + self.assertTrue(self.addr2.is_wildcard()) + + def test_get_sni_addr(self): + from letsencrypt_apache.obj import Addr + self.assertEqual( + self.addr.get_sni_addr("443"), Addr.fromstring("*:443")) + self.assertEqual( + self.addr.get_sni_addr("225"), Addr.fromstring("*:225")) + self.assertEqual( + self.addr1.get_sni_addr("443"), Addr.fromstring("127.0.0.1")) + + def test_equal(self): + self.assertTrue(self.addr1 == self.addr2) + self.assertFalse(self.addr == self.addr1) + + def test_not_equal(self): + self.assertFalse(self.addr1 != self.addr2) + self.assertTrue(self.addr != self.addr1) + + if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py b/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py index 49575e977..3075a42c3 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py @@ -6,8 +6,6 @@ import unittest import augeas import mock -from letsencrypt import errors - from letsencrypt_apache.tests import util @@ -48,10 +46,6 @@ class BasicParserTest(util.ParserTest): self.assertEqual(len(test), 1) self.assertEqual(len(test2), 3) - def test_filter_args_num(self): - # TODO: TEST 2, TEST 1 - pass - def test_add_dir(self): aug_default = "/files" + self.parser.loc["default"] self.parser.add_dir(aug_default, "AddDirective", "test") @@ -90,16 +84,10 @@ class BasicParserTest(util.ParserTest): def test_set_locations(self): with mock.patch("letsencrypt_apache.parser.os.path") as mock_path: - mock_path.isfile.return_value = False - - # pylint: disable=protected-access - self.assertRaises(errors.PluginError, - self.parser._set_locations, self.ssl_options) - mock_path.isfile.side_effect = [True, False, False] # pylint: disable=protected-access - results = self.parser._set_locations(self.ssl_options) + results = self.parser._set_locations("root") self.assertEqual(results["default"], results["listen"]) self.assertEqual(results["default"], results["name"]) @@ -124,7 +112,7 @@ class ParserInitTest(util.ApacheTest): path = os.path.join( self.temp_dir, "debian_apache_2_4/////two_vhost_80/../two_vhost_80/apache2") - parser = ApacheParser(self.aug, path, None, "dummy_ctl") + parser = ApacheParser(self.aug, path, "dummy_ctl") self.assertEqual(parser.root, self.config_path) @@ -133,7 +121,7 @@ class ParserInitTest(util.ApacheTest): with mock.patch("letsencrypt_apache.parser.ApacheParser." "update_runtime_variables"): parser = ApacheParser( - self.aug, os.path.relpath(self.config_path), None, "dummy_ctl") + self.aug, os.path.relpath(self.config_path), "dummy_ctl") self.assertEqual(parser.root, self.config_path) @@ -142,8 +130,9 @@ class ParserInitTest(util.ApacheTest): with mock.patch("letsencrypt_apache.parser.ApacheParser." "update_runtime_variables"): parser = ApacheParser( - self.aug, self.config_path + os.path.sep, None, "dummy_ctl") + self.aug, self.config_path + os.path.sep, "dummy_ctl") self.assertEqual(parser.root, self.config_path) + if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/letsencrypt-apache/letsencrypt_apache/tests/util.py b/letsencrypt-apache/letsencrypt_apache/tests/util.py index 8b54b08a4..e0d2f177a 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/util.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/util.py @@ -54,7 +54,7 @@ class ParserTest(ApacheTest): # pytlint: disable=too-few-public-methods with mock.patch("letsencrypt_apache.parser.ApacheParser." "update_runtime_variables"): self.parser = ApacheParser( - self.aug, self.config_path, self.ssl_options, "dummy_ctl_path") + self.aug, self.config_path, "dummy_ctl_path") def get_apache_configurator(