Merge pull request #1385 from joohoi/modmacro

Ignore mod_macro vhosts, and display notification

letsencrypt-apache/letsencrypt_apache/configurator.py

@@ -306,6 +306,8 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         best_points = 0
 
         for vhost in self.vhosts:
+            if vhost.modmacro is True:
+                continue
             if target_name in vhost.get_names():
                 points = 2
             elif any(addr.get_addr() == target_name for addr in vhost.addrs):
@@ -325,7 +327,10 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         # No winners here... is there only one reasonable vhost?
         if best_candidate is None:
             # reasonable == Not all _default_ addrs
-            reasonable_vhosts = self._non_default_vhosts()
+            vhosts = self._non_default_vhosts()
+            # remove mod_macro hosts from reasonable vhosts
+            reasonable_vhosts = [vh for vh
+                                 in vhosts if vh.modmacro is False]
             if len(reasonable_vhosts) == 1:
                 best_candidate = reasonable_vhosts[0]
 
@@ -347,8 +352,12 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         """
         all_names = set()
 
+        vhost_macro = []
+
         for vhost in self.vhosts:
             all_names.update(vhost.get_names())
+            if vhost.modmacro:
+                vhost_macro.append(vhost.filep)
 
             for addr in vhost.addrs:
                 if common.hostname_regex.match(addr.get_addr()):
@@ -358,6 +367,12 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
                     if name:
                         all_names.add(name)
 
+        if len(vhost_macro) > 0:
+            zope.component.getUtility(interfaces.IDisplay).notification(
+                "Apache mod_macro seems to be in use in file(s):\n{0}"
+                "\n\nUnfortunately mod_macro is not yet supported".format(
+                    "\n  ".join(vhost_macro)))
+
         return all_names
 
     def get_name_from_ip(self, addr):  # pylint: disable=no-self-use
@@ -394,11 +409,34 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
             "ServerAlias", None, start=host.path, exclude=False)
 
         for alias in serveralias_match:
-            host.aliases.add(self.parser.get_arg(alias))
+            serveralias = self.parser.get_arg(alias)
+            if not self._is_mod_macro(serveralias):
+                host.aliases.add(serveralias)
+            else:
+                host.modmacro = True
 
         if servername_match:
             # Get last ServerName as each overwrites the previous
-            host.name = self.parser.get_arg(servername_match[-1])
+            servername = self.parser.get_arg(servername_match[-1])
+            if not self._is_mod_macro(servername):
+                host.name = servername
+            else:
+                host.modmacro = True
+
+    def _is_mod_macro(self, name):
+        """Helper function for _add_servernames().
+        Checks if the ServerName / ServerAlias belongs to a macro
+
+        :param str name: Name to check and filter out if it's a variable
+
+        :returns: boolean
+        :rtype: boolean
+
+        """
+
+        if "$" in name:
+            return True
+        return False
 
     def _create_vhost(self, path):
         """Used by get_virtual_hosts to create vhost objects
@@ -1234,7 +1272,7 @@ def get_file_path(vhost_path):
     avail_fp = vhost_path[6:]
     # This can be optimized...
     while True:
-        # Cast both to lowercase to be case insensitive
+        # Cast all to lowercase to be case insensitive
         find_if = avail_fp.lower().find("/ifmodule")
         if find_if != -1:
             avail_fp = avail_fp[:find_if]
@@ -1243,6 +1281,10 @@ def get_file_path(vhost_path):
         if find_vh != -1:
             avail_fp = avail_fp[:find_vh]
             continue
+        find_macro = avail_fp.lower().find("/macro")
+        if find_macro != -1:
+            avail_fp = avail_fp[:find_macro]
+            continue
         break
     return avail_fp
 

letsencrypt-apache/letsencrypt_apache/obj.py

@@ -102,6 +102,7 @@ class VirtualHost(object):  # pylint: disable=too-few-public-methods
 
     :ivar bool ssl: SSLEngine on in vhost
     :ivar bool enabled: Virtual host is enabled
+    :ivar bool modmacro: VirtualHost is using mod_macro
 
     https://httpd.apache.org/docs/2.4/vhosts/details.html
 
@@ -112,7 +113,9 @@ class VirtualHost(object):  # pylint: disable=too-few-public-methods
     # ?: is used for not returning enclosed characters
     strip_name = re.compile(r"^(?:.+://)?([^ :$]*)")
 
-    def __init__(self, filep, path, addrs, ssl, enabled, name=None, aliases=None):
+    def __init__(self, filep, path, addrs, ssl, enabled, name=None,
+                 aliases=None, modmacro=False):
+
         # pylint: disable=too-many-arguments
         """Initialize a VH."""
         self.filep = filep
@@ -122,6 +125,7 @@ class VirtualHost(object):  # pylint: disable=too-few-public-methods
         self.aliases = aliases if aliases is not None else set()
         self.ssl = ssl
         self.enabled = enabled
+        self.modmacro = modmacro
 
     def get_names(self):
         """Return a set of all names."""
@@ -141,21 +145,25 @@ class VirtualHost(object):  # pylint: disable=too-few-public-methods
             "Name: {name}\n"
             "Aliases: {aliases}\n"
             "TLS Enabled: {tls}\n"
-            "Site Enabled: {active}".format(
+            "Site Enabled: {active}\n"
+            "mod_macro Vhost: {modmacro}".format(
                 filename=self.filep,
                 vhpath=self.path,
                 addrs=", ".join(str(addr) for addr in self.addrs),
                 name=self.name if self.name is not None else "",
                 aliases=", ".join(name for name in self.aliases),
                 tls="Yes" if self.ssl else "No",
-                active="Yes" if self.enabled else "No"))
+                active="Yes" if self.enabled else "No",
+                modmacro="Yes" if self.modmacro else "No"))
 
     def __eq__(self, other):
         if isinstance(other, self.__class__):
             return (self.filep == other.filep and self.path == other.path and
                     self.addrs == other.addrs and
                     self.get_names() == other.get_names() and
-                    self.ssl == other.ssl and self.enabled == other.enabled)
+                    self.ssl == other.ssl and
+                    self.enabled == other.enabled and
+                    self.modmacro == other.modmacro)
 
         return False
 

letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py

@@ -59,14 +59,20 @@ class TwoVhost80Test(util.ApacheTest):
         # Weak test..
         ApacheConfigurator.add_parser_arguments(mock.MagicMock())
 
-    def test_get_all_names(self):
+    @mock.patch("zope.component.getUtility")
+    def test_get_all_names(self, mock_getutility):
+        mock_getutility.notification = mock.MagicMock(return_value=True)
         names = self.config.get_all_names()
         self.assertEqual(names, set(
             ["letsencrypt.demo", "encryption-example.demo", "ip-172-30-0-17"]))
 
+    @mock.patch("zope.component.getUtility")
     @mock.patch("letsencrypt_apache.configurator.socket.gethostbyaddr")
-    def test_get_all_names_addrs(self, mock_gethost):
+    def test_get_all_names_addrs(self, mock_gethost, mock_getutility):
         mock_gethost.side_effect = [("google.com", "", ""), socket.error]
+        notification = mock.Mock()
+        notification.notification = mock.Mock(return_value=True)
+        mock_getutility.return_value = notification
         vhost = obj.VirtualHost(
             "fp", "ap",
             set([obj.Addr(("8.8.8.8", "443")),
@@ -97,7 +103,7 @@ class TwoVhost80Test(util.ApacheTest):
 
         """
         vhs = self.config.get_virtual_hosts()
-        self.assertEqual(len(vhs), 4)
+        self.assertEqual(len(vhs), 5)
         found = 0
 
         for vhost in vhs:
@@ -108,7 +114,7 @@ class TwoVhost80Test(util.ApacheTest):
             else:
                 raise Exception("Missed: %s" % vhost)  # pragma: no cover
 
-        self.assertEqual(found, 4)
+        self.assertEqual(found, 5)
 
     @mock.patch("letsencrypt_apache.display_ops.select_vhost")
     def test_choose_vhost_none_avail(self, mock_select):
@@ -172,9 +178,14 @@ class TwoVhost80Test(util.ApacheTest):
         self.assertEqual(
             self.config._find_best_vhost("example.demo"), self.vh_truth[2])
 
+    def test_is_mod_macro(self):
+        # pylint: disable=protected-access
+        self.assertEqual(self.config._is_mod_macro("$domain"), True)
+        self.assertEqual(self.config._is_mod_macro("www.example.com"), False)
+
     def test_non_default_vhosts(self):
         # pylint: disable=protected-access
-        self.assertEqual(len(self.config._non_default_vhosts()), 3)
+        self.assertEqual(len(self.config._non_default_vhosts()), 4)
 
     def test_is_site_enabled(self):
         """Test if site is enabled.
@@ -345,7 +356,7 @@ class TwoVhost80Test(util.ApacheTest):
         self.assertEqual(self.config.is_name_vhost(self.vh_truth[0]),
                          self.config.is_name_vhost(ssl_vhost))
 
-        self.assertEqual(len(self.config.vhosts), 5)
+        self.assertEqual(len(self.config.vhosts), 6)
 
     def test_make_vhost_ssl_extra_vhs(self):
         self.config.aug.match = mock.Mock(return_value=["p1", "p2"])
@@ -587,7 +598,7 @@ class TwoVhost80Test(util.ApacheTest):
         self.vh_truth[1].aliases = set(["yes.default.com"])
 
         self.config._enable_redirect(self.vh_truth[1], "")  # pylint: disable=protected-access
-        self.assertEqual(len(self.config.vhosts), 5)
+        self.assertEqual(len(self.config.vhosts), 6)
 
     def get_achalls(self):
         """Return testing achallenges."""

letsencrypt-apache/letsencrypt_apache/tests/display_ops_test.py

@@ -57,7 +57,7 @@ class SelectVhostTest(unittest.TestCase):
 
     @mock.patch("letsencrypt_apache.display_ops.zope.component.getUtility")
     def test_multiple_names(self, mock_util):
-        mock_util().menu.return_value = (display_util.OK, 4)
+        mock_util().menu.return_value = (display_util.OK, 5)
 
         self.vhosts.append(
             obj.VirtualHost(
@@ -65,7 +65,7 @@ class SelectVhostTest(unittest.TestCase):
                 False, False,
                 "wildcard.com", set(["*.wildcard.com"])))
 
-        self.assertEqual(self.vhosts[4], self._call(self.vhosts))
+        self.assertEqual(self.vhosts[5], self._call(self.vhosts))
 
 
 if __name__ == "__main__":

letsencrypt-apache/letsencrypt_apache/tests/parser_test.py

@@ -52,7 +52,7 @@ class BasicParserTest(util.ParserTest):
         test2 = self.parser.find_dir("documentroot")
 
         self.assertEqual(len(test), 1)
-        self.assertEqual(len(test2), 3)
+        self.assertEqual(len(test2), 4)
 
     def test_add_dir(self):
         aug_default = "/files" + self.parser.loc["default"]

letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-available/mod_macro-example.conf

@@ -0,0 +1,15 @@
+<Macro VHost $name $domain>
+<VirtualHost *:80>
+    ServerName $domain
+    ServerAlias www.$domain
+    DocumentRoot /var/www/html
+
+    ErrorLog ${APACHE_LOG_DIR}/error.log
+    CustomLog ${APACHE_LOG_DIR}/access.log combined
+</VirtualHost>
+</Macro>
+Use VHost macro1 test.com
+Use VHost macro2 hostname.org
+Use VHost macro3 apache.org
+
+# vim: syntax=apache ts=4 sw=4 sts=4 sr noet

letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-enabled/mod_macro-example.conf

@@ -0,0 +1 @@
+../sites-available/mod_macro-example.conf

letsencrypt-apache/letsencrypt_apache/tests/util.py

@@ -124,6 +124,11 @@ def get_vh_truth(temp_dir, config_name):
                 os.path.join(aug_pre, "letsencrypt.conf/VirtualHost"),
                 set([obj.Addr.fromstring("*:80")]), False, True,
                 "letsencrypt.demo"),
+            obj.VirtualHost(
+                os.path.join(prefix, "mod_macro-example.conf"),
+                os.path.join(aug_pre,
+                             "mod_macro-example.conf/Macro/VirtualHost"),
+                set([obj.Addr.fromstring("*:80")]), False, True, modmacro=True)
         ]
         return vh_truth