From 9f8ab4677ef4a8caaca077ae93bb288c6b35bc62 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 5 Nov 2015 14:49:21 +0200 Subject: [PATCH 01/11] Ignoring mod_macro virtualhosts, but displaying a notification why it's done --- .../letsencrypt_apache/configurator.py | 43 +++++++++++++++++-- letsencrypt-apache/letsencrypt_apache/obj.py | 6 ++- 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index d376fe4b6..e1f53f269 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -348,8 +348,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()): @@ -359,6 +363,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 @@ -395,11 +405,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 @@ -1236,7 +1269,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] @@ -1245,6 +1278,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 diff --git a/letsencrypt-apache/letsencrypt_apache/obj.py b/letsencrypt-apache/letsencrypt_apache/obj.py index 58a6c740e..d71cfb1ad 100644 --- a/letsencrypt-apache/letsencrypt_apache/obj.py +++ b/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, modmacro=False, + name=None, aliases=None): + # 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.""" From aa0161fbec624c08c14a0d7aedb4f1cedbb2ad32 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Thu, 5 Nov 2015 18:53:26 +0200 Subject: [PATCH 02/11] Let's avoid breaking backwards compatibility --- letsencrypt-apache/letsencrypt_apache/obj.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/obj.py b/letsencrypt-apache/letsencrypt_apache/obj.py index d71cfb1ad..80f92cab3 100644 --- a/letsencrypt-apache/letsencrypt_apache/obj.py +++ b/letsencrypt-apache/letsencrypt_apache/obj.py @@ -113,8 +113,8 @@ 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, modmacro=False, - 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.""" From 4bd0330ae7b6a568350e18b358ce66deceeb0f4f Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Fri, 6 Nov 2015 10:55:35 +0200 Subject: [PATCH 03/11] Do not suggest mod_macro vhost for the best vhost --- .../letsencrypt_apache/configurator.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index e1f53f269..a1227b0bb 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -307,6 +307,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): @@ -326,12 +328,27 @@ 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() + # remove mod_macro hosts from reasonable vhosts + reasonable_vhosts = self._without_modmacro( + self._non_default_vhosts()) if len(reasonable_vhosts) == 1: best_candidate = reasonable_vhosts[0] + if best_candidate is not None and best_candidate.modmacro is True: + return None return best_candidate + def _without_modmacro(self, vhosts): + """Return all non mod_macro vhosts + + :param vhosts: List of VirtualHosts + :type vhosts: (:class:`list` of :class:`~letsencrypt_apache.obj.VirtualHost`) + + :returns: List of VirtualHosts without mod_macro + :rtype: (:class:`list` of :class:`~letsencrypt_apache.obj.VirtualHost`) + """ + return [vh for vh in vhosts if vh.modmacro == False] + def _non_default_vhosts(self): """Return all non _default_ only vhosts.""" return [vh for vh in self.vhosts if not all( From ba3db558d5295d0ec2866c40a27a2abb102d7c79 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Fri, 6 Nov 2015 10:56:50 +0200 Subject: [PATCH 04/11] Tests taking mod_macro into account --- .../tests/configurator_test.py | 20 ++++++++++++------- .../tests/display_ops_test.py | 4 ++-- .../letsencrypt_apache/tests/parser_test.py | 2 +- .../sites-available/mod_macro-example.conf | 15 ++++++++++++++ .../sites-enabled/mod_macro-example.conf | 1 + .../letsencrypt_apache/tests/util.py | 5 +++++ 6 files changed, 37 insertions(+), 10 deletions(-) create mode 100644 letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-available/mod_macro-example.conf create mode 120000 letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-enabled/mod_macro-example.conf diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 7c2137c45..c74fa3241 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/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): @@ -174,7 +180,7 @@ class TwoVhost80Test(util.ApacheTest): 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 +351,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 +593,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.""" diff --git a/letsencrypt-apache/letsencrypt_apache/tests/display_ops_test.py b/letsencrypt-apache/letsencrypt_apache/tests/display_ops_test.py index d7cfb09b3..6db319d87 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/display_ops_test.py +++ b/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__": diff --git a/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py b/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py index d2e4dec14..bc1f316f9 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py +++ b/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"] diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-available/mod_macro-example.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-available/mod_macro-example.conf new file mode 100644 index 000000000..6a6579007 --- /dev/null +++ b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-available/mod_macro-example.conf @@ -0,0 +1,15 @@ + + + ServerName $domain + ServerAlias www.$domain + DocumentRoot /var/www/html + + ErrorLog ${APACHE_LOG_DIR}/error.log + CustomLog ${APACHE_LOG_DIR}/access.log combined + + +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 diff --git a/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-enabled/mod_macro-example.conf b/letsencrypt-apache/letsencrypt_apache/tests/testdata/debian_apache_2_4/two_vhost_80/apache2/sites-enabled/mod_macro-example.conf new file mode 120000 index 000000000..44f254304 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/letsencrypt-apache/letsencrypt_apache/tests/util.py b/letsencrypt-apache/letsencrypt_apache/tests/util.py index 2594ba773..a8bfe0e4b 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/util.py +++ b/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 From 93a53d507821ac0612aadb62d70edf5f3856c468 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Fri, 6 Nov 2015 11:11:14 +0200 Subject: [PATCH 05/11] Added test for removing mod_macro vhosts from vhost list --- .../letsencrypt_apache/tests/configurator_test.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index c74fa3241..4224749bf 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -178,6 +178,10 @@ class TwoVhost80Test(util.ApacheTest): self.assertEqual( self.config._find_best_vhost("example.demo"), self.vh_truth[2]) + def test_without_modmacro(self): + self.assertEqual(len(self.vh_truth)-1, + len(self.config._without_modmacro(self.vh_truth))) + def test_non_default_vhosts(self): # pylint: disable=protected-access self.assertEqual(len(self.config._non_default_vhosts()), 4) From 88b89a04b1e2aabdfc692c2143223da31ba5c03d Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Fri, 6 Nov 2015 12:08:28 +0200 Subject: [PATCH 06/11] Fix pylint in the new test --- letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 4224749bf..2e335ea00 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -179,6 +179,7 @@ class TwoVhost80Test(util.ApacheTest): self.config._find_best_vhost("example.demo"), self.vh_truth[2]) def test_without_modmacro(self): + # pylint: disable=protected-access self.assertEqual(len(self.vh_truth)-1, len(self.config._without_modmacro(self.vh_truth))) From f0c059752fcd8b36862e12c77a98ce351c6d5442 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Fri, 6 Nov 2015 12:15:53 +0200 Subject: [PATCH 07/11] Added test for mod_macro check in domain names --- .../letsencrypt_apache/tests/configurator_test.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 2e335ea00..963948d6e 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -183,6 +183,11 @@ class TwoVhost80Test(util.ApacheTest): self.assertEqual(len(self.vh_truth)-1, len(self.config._without_modmacro(self.vh_truth))) + 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()), 4) From 8fb3956ecd2b46c42e18d97b19129f4e31266a89 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Fri, 6 Nov 2015 12:45:44 +0200 Subject: [PATCH 08/11] Removed dead code --- letsencrypt-apache/letsencrypt_apache/configurator.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index a1227b0bb..9da00b5c3 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -334,8 +334,6 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): if len(reasonable_vhosts) == 1: best_candidate = reasonable_vhosts[0] - if best_candidate is not None and best_candidate.modmacro is True: - return None return best_candidate def _without_modmacro(self, vhosts): From effc1ed8e4595f784983c83d9063e5cf8ace8d6f Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Fri, 6 Nov 2015 19:53:13 +0200 Subject: [PATCH 09/11] Fix VirtualHost magic methods to take mod_macro addition into account --- letsencrypt-apache/letsencrypt_apache/obj.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/obj.py b/letsencrypt-apache/letsencrypt_apache/obj.py index 80f92cab3..175ce3f92 100644 --- a/letsencrypt-apache/letsencrypt_apache/obj.py +++ b/letsencrypt-apache/letsencrypt_apache/obj.py @@ -145,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 From 980b4ac3fae6ccc9d4def3fdeb35b160af55bdca Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Fri, 6 Nov 2015 20:06:56 +0200 Subject: [PATCH 10/11] PEP8 and sphinx documentation fixes --- letsencrypt-apache/letsencrypt_apache/configurator.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 9da00b5c3..d3849b7fd 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -340,12 +340,13 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """Return all non mod_macro vhosts :param vhosts: List of VirtualHosts - :type vhosts: (:class:`list` of :class:`~letsencrypt_apache.obj.VirtualHost`) + :type vhosts: :class:`list` of + :class:`~letsencrypt_apache.obj.VirtualHost` :returns: List of VirtualHosts without mod_macro - :rtype: (:class:`list` of :class:`~letsencrypt_apache.obj.VirtualHost`) + :rtype: :class:`list` of :class:`~letsencrypt_apache.obj.VirtualHost` """ - return [vh for vh in vhosts if vh.modmacro == False] + return [vh for vh in vhosts if vh.modmacro is False] def _non_default_vhosts(self): """Return all non _default_ only vhosts.""" From ce501f94a38a9ab23a666af34d1390cb4250cc28 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Sat, 7 Nov 2015 08:31:46 +0200 Subject: [PATCH 11/11] Simplify the code --- .../letsencrypt_apache/configurator.py | 17 +++-------------- .../tests/configurator_test.py | 5 ----- 2 files changed, 3 insertions(+), 19 deletions(-) diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index d3849b7fd..6b4caef50 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -328,26 +328,15 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): # No winners here... is there only one reasonable vhost? if best_candidate is None: # reasonable == Not all _default_ addrs + vhosts = self._non_default_vhosts() # remove mod_macro hosts from reasonable vhosts - reasonable_vhosts = self._without_modmacro( - self._non_default_vhosts()) + reasonable_vhosts = [vh for vh + in vhosts if vh.modmacro is False] if len(reasonable_vhosts) == 1: best_candidate = reasonable_vhosts[0] return best_candidate - def _without_modmacro(self, vhosts): - """Return all non mod_macro vhosts - - :param vhosts: List of VirtualHosts - :type vhosts: :class:`list` of - :class:`~letsencrypt_apache.obj.VirtualHost` - - :returns: List of VirtualHosts without mod_macro - :rtype: :class:`list` of :class:`~letsencrypt_apache.obj.VirtualHost` - """ - return [vh for vh in vhosts if vh.modmacro is False] - def _non_default_vhosts(self): """Return all non _default_ only vhosts.""" return [vh for vh in self.vhosts if not all( diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 963948d6e..61009b89f 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -178,11 +178,6 @@ class TwoVhost80Test(util.ApacheTest): self.assertEqual( self.config._find_best_vhost("example.demo"), self.vh_truth[2]) - def test_without_modmacro(self): - # pylint: disable=protected-access - self.assertEqual(len(self.vh_truth)-1, - len(self.config._without_modmacro(self.vh_truth))) - def test_is_mod_macro(self): # pylint: disable=protected-access self.assertEqual(self.config._is_mod_macro("$domain"), True)