Improved Apache and unittests

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.aug, self.conf("server-root"), self.conf("ctl"))
-            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.

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)

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.
 

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,
+        return {"default": default, "listen": listen, "name": name}
-                "name": name, "ssl_options": ssl_options}
 
     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 ""

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

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

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(