diff --git a/letsencrypt-apache/letsencrypt_apache/configurator.py b/letsencrypt-apache/letsencrypt_apache/configurator.py index 8bcdb1fbe..c35922e21 100644 --- a/letsencrypt-apache/letsencrypt_apache/configurator.py +++ b/letsencrypt-apache/letsencrypt_apache/configurator.py @@ -13,10 +13,8 @@ import zope.interface from acme import challenges from letsencrypt import achallenges -from letsencrypt import constants as core_constants from letsencrypt import errors from letsencrypt import interfaces -from letsencrypt import le_util from letsencrypt_apache import augeas_configurator from letsencrypt_apache import constants @@ -369,15 +367,12 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): serveralias_match = self.parser.find_dir( "ServerAlias", None, host.path, exclude=False) - aliases = [] for alias in serveralias_match: - aliases.append(self.parser.get_arg(alias)) + host.aliases.add(self.parser.get_arg(alias)) if servername_match: # Get last ServerName as each overwrites the previous - host.add_names(self.parser.get_arg(servername_match[-1]), aliases) - else: - host.add_names(None, aliases) + host.name = self.parser.get_arg(servername_match[-1]) def _create_vhost(self, path): """Used by get_virtual_hosts to create vhost objects @@ -498,16 +493,12 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): self.save_notes += "Added Listen %s directive to %s\n" % ( port, self.parser.loc["listen"]) - def make_addrs_sni_ready( - self, addrs, default_addr=obj.Addr(("*", "443"))): + def make_addrs_sni_ready(self, addrs): """Checks to see if the server is ready for SNI challenges. :param addrs: Addresses to check SNI compatibility :type addrs: :class:`~letsencrypt_apache.obj.Addr` - :param default_addr: TODO - investigate function further - :type default_addr: :class:~letsencrypt_apache.obj.Addr - """ # Version 2.4 and later are automatically SNI ready. if self.version >= (2, 4): @@ -823,10 +814,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): "ErrorLog /var/log/apache2/redirect.error.log\n" "LogLevel warn\n" "\n" - % ( - " ".join(str(addr) for addr in self._get_redirect_addrs(ssl_vhost)), - servername, serveralias, - " ".join(constants.REWRITE_HTTPS_ARGS))) + % (" ".join(str(addr) for addr in self._get_redirect_addrs(ssl_vhost)), + servername, serveralias, + " ".join(constants.REWRITE_HTTPS_ARGS))) def _write_out_redirect(self, ssl_vhost, text): # This is the default name @@ -867,7 +857,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): return None - def _get_redirect_addrs(self, ssl_vhost): + def _get_redirect_addrs(self, ssl_vhost): # pylint: disable=no-self-use redirects = set() for addr in ssl_vhost.addrs: redirects.add(addr.get_addr_obj("80")) diff --git a/letsencrypt-apache/letsencrypt_apache/obj.py b/letsencrypt-apache/letsencrypt_apache/obj.py index f192ad6da..040b69082 100644 --- a/letsencrypt-apache/letsencrypt_apache/obj.py +++ b/letsencrypt-apache/letsencrypt_apache/obj.py @@ -23,10 +23,16 @@ class Addr(common.Addr): def _addr_less_specific(self, addr): """Returns if addr.get_addr() is more specific than self.get_addr().""" + # pylint: disable=protected-access return addr._rank_specific_addr() > self._rank_specific_addr() def _rank_specific_addr(self): - """Returns numerical rank for get_addr()""" + """Returns numerical rank for get_addr() + + :returns: 2 - FQ, 1 - wildcard, 0 - _default_ + :rtype: int + + """ if self.get_addr() == "_default_": return 0 elif self.get_addr() == "*": @@ -101,14 +107,14 @@ 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=[]): + def __init__(self, filep, path, addrs, ssl, enabled, name=None, aliases=None): # pylint: disable=too-many-arguments """Initialize a VH.""" self.filep = filep self.path = path self.addrs = addrs self.name = name - self.aliases = aliases + self.aliases = aliases if aliases is not None else set() self.ssl = ssl self.enabled = enabled @@ -119,7 +125,8 @@ class VirtualHost(object): # pylint: disable=too-few-public-methods def get_names(self): """Return a set of all names.""" - all_names = set(self.aliases) + all_names = set() + all_names.update(self.aliases) # Strip out any scheme:// and field from servername if self.name is not None: all_names.add(VirtualHost.strip_name.findall(self.name)[0]) @@ -216,4 +223,4 @@ class VirtualHost(object): # pylint: disable=too-few-public-methods else: return False - return True \ No newline at end of file + return True diff --git a/letsencrypt-apache/letsencrypt_apache/tests/augeas_configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/augeas_configurator_test.py index b9b6712a7..8cb1fb3a8 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/augeas_configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/augeas_configurator_test.py @@ -7,11 +7,6 @@ import mock from letsencrypt import errors -from letsencrypt.tests import acme_util - -from letsencrypt_apache import configurator -from letsencrypt_apache import obj - from letsencrypt_apache.tests import util @@ -28,11 +23,12 @@ class AugeasConfiguratorTest(util.ApacheTest): self.temp_dir, "debian_apache_2_4/two_vhost_80") def tearDown(self): - shutil.rmtree(self.temp_dir) shutil.rmtree(self.config_dir) shutil.rmtree(self.work_dir) + shutil.rmtree(self.temp_dir) def test_bad_parse(self): + # pylint: disable=protected-access self.config.parser._parse_file(os.path.join( self.config.parser.root, "conf-available", "bad_conf_file.conf")) self.assertRaises( @@ -78,4 +74,4 @@ class AugeasConfiguratorTest(util.ApacheTest): if __name__ == "__main__": - unittest.main() # pragma: no cover \ No newline at end of file + unittest.main() # pragma: no cover diff --git a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py index 233b20b42..f7c3d12f3 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/configurator_test.py @@ -1,3 +1,4 @@ +# pylint: disable=too-many-public-methods """Test for letsencrypt_apache.configurator.""" import os import shutil @@ -38,7 +39,7 @@ class TwoVhost80Test(util.ApacheTest): shutil.rmtree(self.work_dir) @mock.patch("letsencrypt_apache.parser.ApacheParser") - def test_prepare_version(self, mock_parser): + def test_prepare_version(self, _): self.config.version = None self.config.config_test = mock.Mock() self.config.get_version = mock.Mock(return_value=(1, 1)) @@ -46,7 +47,7 @@ class TwoVhost80Test(util.ApacheTest): self.assertRaises( errors.NotSupportedError, self.config.prepare) - def test_add_parser_arguments(self): + def test_add_parser_arguments(self): # pylint: disable=no-self-use from letsencrypt_apache.configurator import ApacheConfigurator # Weak test.. ApacheConfigurator.add_parser_arguments(mock.MagicMock()) @@ -58,14 +59,14 @@ class TwoVhost80Test(util.ApacheTest): @mock.patch("letsencrypt_apache.configurator.socket.gethostbyaddr") def test_get_all_names_addrs(self, mock_gethost): - mock_gethost.side_effect = [("google.com","",""), socket.error] - vh = obj.VirtualHost( + mock_gethost.side_effect = [("google.com", "", ""), socket.error] + vhost = obj.VirtualHost( "fp", "ap", set([obj.Addr(("8.8.8.8", "443")), obj.Addr(("zombo.com",)), obj.Addr(("192.168.1.2"))]), True, False) - self.config.vhosts.append(vh) + self.config.vhosts.append(vhost) names = self.config.get_all_names() self.assertEqual(len(names), 5) @@ -115,6 +116,7 @@ class TwoVhost80Test(util.ApacheTest): self.vh_truth[3], self.config.choose_vhost("none.com")) def test_find_best_vhost(self): + # pylint: disable=protected-access self.assertEqual( self.vh_truth[3], self.config._find_best_vhost("letsencrypt.demo")) self.assertEqual( @@ -124,6 +126,7 @@ class TwoVhost80Test(util.ApacheTest): self.config._find_best_vhost("does-not-exist.com") is None) def test_find_best_vhost_variety(self): + # pylint: disable=protected-access ssl_vh = obj.VirtualHost( "fp", "ap", set([obj.Addr(("*", "443")), obj.Addr(("zombo.com",))]), True, False) @@ -131,10 +134,12 @@ class TwoVhost80Test(util.ApacheTest): self.assertEqual(self.config._find_best_vhost("zombo.com"), ssl_vh) def test_find_best_vhost_default(self): + # pylint: disable=protected-access # Assume only the two default vhosts. - self.config.vhosts = [vh for vh in self.config.vhosts - if vh.name not in - ["letsencrypt.demo", "encryption-example.demo"]] + self.config.vhosts = [ + vh for vh in self.config.vhosts + if vh.name not in ["letsencrypt.demo", "encryption-example.demo"] + ] self.assertEqual( self.config._find_best_vhost("example.demo"), self.vh_truth[2]) @@ -338,10 +343,12 @@ class TwoVhost80Test(util.ApacheTest): self.config.make_vhost_ssl, self.vh_truth[0]) def test_get_ssl_vhost_path(self): + # pylint: disable=protected-access self.assertTrue( self.config._get_ssl_vhost_path("example_path").endswith(".conf")) def test_add_name_vhost_if_necessary(self): + # pylint: disable=protected-access self.config.save = mock.Mock() self.config.version = (2, 2) self.config._add_name_vhost_if_necessary(self.vh_truth[0]) @@ -352,7 +359,7 @@ class TwoVhost80Test(util.ApacheTest): def test_perform(self, mock_restart, mock_dvsni_perform): # Only tests functionality specific to configurator.perform # Note: As more challenges are offered this will have to be expanded - auth_key, achall1, achall2 = self.get_achalls() + _, achall1, achall2 = self.get_achalls() dvsni_ret_val = [ challenges.DVSNIResponse(s="randomS1"), @@ -369,10 +376,10 @@ class TwoVhost80Test(util.ApacheTest): @mock.patch("letsencrypt_apache.configurator.ApacheConfigurator.restart") def test_cleanup(self, mock_restart): - auth_key, achall1, achall2 = self.get_achalls() + _, achall1, achall2 = self.get_achalls() - self.config._chall_out.add(achall1) - self.config._chall_out.add(achall2) + self.config._chall_out.add(achall1) # pylint: disable=protected-access + self.config._chall_out.add(achall2) # pylint: disable=protected-access self.config.cleanup([achall1]) self.assertFalse(mock_restart.called) @@ -382,9 +389,9 @@ class TwoVhost80Test(util.ApacheTest): @mock.patch("letsencrypt_apache.configurator.ApacheConfigurator.restart") def test_cleanup_no_errors(self, mock_restart): - auth_key, achall1, achall2 = self.get_achalls() + _, achall1, achall2 = self.get_achalls() - self.config._chall_out.add(achall1) + self.config._chall_out.add(achall1) # pylint: disable=protected-access self.config.cleanup([achall2]) self.assertFalse(mock_restart.called) @@ -430,7 +437,7 @@ class TwoVhost80Test(util.ApacheTest): @mock.patch("letsencrypt_apache.configurator.subprocess.Popen") def test_restart_failure(self, mock_popen): mock_popen().communicate.return_value = ("", "") - mock_popen.returncode=1 + mock_popen().returncode = 1 self.assertRaises(errors.MisconfigurationError, self.config.restart) @@ -454,7 +461,6 @@ class TwoVhost80Test(util.ApacheTest): self.assertRaises(errors.MisconfigurationError, self.config.config_test) - def test_get_all_certs_keys(self): c_k = self.config.get_all_certs_keys() @@ -493,7 +499,7 @@ class TwoVhost80Test(util.ApacheTest): @mock.patch("letsencrypt_apache.parser." "ApacheParser.update_runtime_variables") - def test_redirect_well_formed_http(self, unused): + def test_redirect_well_formed_http(self, _): # This will create an ssl vhost for letsencrypt.demo self.config.enhance("letsencrypt.demo", "redirect") @@ -571,6 +577,7 @@ class TwoVhost80Test(util.ApacheTest): self.assertEqual(len(self.config.vhosts), 5) def get_achalls(self): + """Return testing achallenges.""" auth_key = le_util.Key(self.rsa256_file, self.rsa256_pem) achall1 = achallenges.DVSNI( challb=acme_util.chall_to_challb( diff --git a/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py b/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py index a378d0b86..45e77bdb9 100644 --- a/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py +++ b/letsencrypt-apache/letsencrypt_apache/tests/parser_test.py @@ -109,6 +109,7 @@ class BasicParserTest(util.ParserTest): self.assertEqual(results["default"], results["name"]) def test_set_user_config_file(self): + # pylint: disable=protected-access path = os.path.join(self.parser.root, "httpd.conf") open(path, 'w').close() self.parser.add_dir(self.parser.loc["default"], "Include", "httpd.conf")