mirror of
https://github.com/certbot/certbot.git
synced 2026-06-03 22:08:07 -04:00
Address @kuba's review comments
This commit is contained in:
yan
parent
636f5aa313
commit
4bcc18d9d3
4 changed files with 113 additions and 117 deletions
|
|
@ -90,11 +90,14 @@ class NginxConfigurator(object):
|
|||
# Entry point in main.py for installing cert
|
||||
def deploy_cert(self, domain, cert, key, cert_chain=None):
|
||||
# pylint: disable=unused-argument
|
||||
"""Deploys certificate to specified virtual host. Aborts if the
|
||||
vhost is missing ssl_certificate or ssl_certificate_key.
|
||||
"""Deploys certificate to specified virtual host.
|
||||
|
||||
Nginx doesn't have a cert chain directive, so the last parameter is
|
||||
always ignored. It expects the cert file to have the concatenated chain.
|
||||
.. note:: Aborts if the vhost is missing ssl_certificate or
|
||||
ssl_certificate_key.
|
||||
|
||||
.. note:: Nginx doesn't have a cert chain directive, so the last
|
||||
parameter is always ignored. It expects the cert file to have
|
||||
the concatenated chain.
|
||||
|
||||
.. note:: This doesn't save the config files!
|
||||
|
||||
|
|
@ -130,9 +133,11 @@ class NginxConfigurator(object):
|
|||
# Vhost parsing methods
|
||||
#######################
|
||||
def choose_vhost(self, target_name):
|
||||
"""Chooses a virtual host based on the given domain name. NOTE: This
|
||||
makes the vhost SSL-enabled if it isn't already. Follows Nginx's server
|
||||
block selection rules but prefers blocks that are already SSL.
|
||||
"""Chooses a virtual host based on the given domain name.
|
||||
|
||||
.. note:: This makes the vhost SSL-enabled if it isn't already. Follows
|
||||
Nginx's server block selection rules preferring blocks that are
|
||||
already SSL.
|
||||
|
||||
.. todo:: This should maybe return list if no obvious answer
|
||||
is presented.
|
||||
|
|
@ -149,10 +154,10 @@ class NginxConfigurator(object):
|
|||
vhost = None
|
||||
|
||||
matches = self._get_ranked_matches(target_name)
|
||||
if len(matches) == 0:
|
||||
if not matches:
|
||||
# No matches at all :'(
|
||||
pass
|
||||
elif matches[0]['rank'] in range(2, 6):
|
||||
elif matches[0]['rank'] in xrange(2, 6):
|
||||
# Wildcard match - need to find the longest one
|
||||
rank = matches[0]['rank']
|
||||
wildcards = [x for x in matches if x['rank'] == rank]
|
||||
|
|
@ -167,8 +172,7 @@ class NginxConfigurator(object):
|
|||
return vhost
|
||||
|
||||
def _get_ranked_matches(self, target_name):
|
||||
"""
|
||||
Returns a ranked list of vhosts that match target_name.
|
||||
"""Returns a ranked list of vhosts that match target_name.
|
||||
|
||||
:param str target_name: The name to match
|
||||
:returns: list of dicts containing the vhost, the matching name, and
|
||||
|
|
@ -374,10 +378,10 @@ class NginxConfigurator(object):
|
|||
sni_regex = re.compile(r"TLS SNI support enabled", re.IGNORECASE)
|
||||
sni_matches = sni_regex.findall(text)
|
||||
|
||||
if len(version_matches) == 0:
|
||||
if not version_matches:
|
||||
raise errors.LetsEncryptConfiguratorError(
|
||||
"Unable to find Nginx version")
|
||||
if len(sni_matches) == 0:
|
||||
if not sni_matches:
|
||||
raise errors.LetsEncryptConfiguratorError(
|
||||
"Nginx build doesn't support SNI")
|
||||
|
||||
|
|
|
|||
|
|
@ -8,9 +8,7 @@ from pyparsing import (
|
|||
|
||||
class RawNginxParser(object):
|
||||
# pylint: disable=expression-not-assigned
|
||||
"""
|
||||
A class that parses nginx configuration with pyparsing
|
||||
"""
|
||||
"""A class that parses nginx configuration with pyparsing."""
|
||||
|
||||
# constants
|
||||
left_bracket = Literal("{").suppress()
|
||||
|
|
@ -39,31 +37,23 @@ class RawNginxParser(object):
|
|||
self.source = source
|
||||
|
||||
def parse(self):
|
||||
"""
|
||||
Returns the parsed tree.
|
||||
"""
|
||||
"""Returns the parsed tree."""
|
||||
return self.script.parseString(self.source)
|
||||
|
||||
def as_list(self):
|
||||
"""
|
||||
Returns the list of tree.
|
||||
"""
|
||||
"""Returns the parsed tree as a list."""
|
||||
return self.parse().asList()
|
||||
|
||||
|
||||
class RawNginxDumper(object):
|
||||
# pylint: disable=too-few-public-methods
|
||||
"""
|
||||
A class that dumps nginx configuration from the provided tree.
|
||||
"""
|
||||
"""A class that dumps nginx configuration from the provided tree."""
|
||||
def __init__(self, blocks, indentation=4):
|
||||
self.blocks = blocks
|
||||
self.indentation = indentation
|
||||
|
||||
def __iter__(self, blocks=None, current_indent=0, spacer=' '):
|
||||
"""
|
||||
Iterates the dumped nginx content.
|
||||
"""
|
||||
"""Iterates the dumped nginx content."""
|
||||
blocks = blocks or self.blocks
|
||||
for key, values in blocks:
|
||||
if current_indent:
|
||||
|
|
@ -88,9 +78,7 @@ class RawNginxDumper(object):
|
|||
yield spacer * current_indent + key + spacer + values + ';'
|
||||
|
||||
def as_string(self):
|
||||
"""
|
||||
Return the parsed block as a string.
|
||||
"""
|
||||
"""Return the parsed block as a string."""
|
||||
return '\n'.join(self)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -48,17 +48,17 @@ class TestRawNginxParser(unittest.TestCase):
|
|||
]]])
|
||||
|
||||
self.assertEqual(dumped,
|
||||
'user www-data;\n' +
|
||||
'server {\n' +
|
||||
' listen 80;\n' +
|
||||
' server_name foo.com;\n' +
|
||||
' root /home/ubuntu/sites/foo/;\n \n' +
|
||||
' location /status {\n' +
|
||||
' check_status;\n \n' +
|
||||
' types {\n' +
|
||||
' image/jpeg jpg;\n' +
|
||||
' }\n' +
|
||||
' }\n' +
|
||||
'user www-data;\n'
|
||||
'server {\n'
|
||||
' listen 80;\n'
|
||||
' server_name foo.com;\n'
|
||||
' root /home/ubuntu/sites/foo/;\n \n'
|
||||
' location /status {\n'
|
||||
' check_status;\n \n'
|
||||
' types {\n'
|
||||
' image/jpeg jpg;\n'
|
||||
' }\n'
|
||||
' }\n'
|
||||
'}')
|
||||
|
||||
def test_parse_from_file(self):
|
||||
|
|
|
|||
|
|
@ -6,9 +6,9 @@ import shutil
|
|||
import unittest
|
||||
|
||||
from letsencrypt.client.errors import LetsEncryptMisconfigurationError
|
||||
from letsencrypt.client.plugins.nginx.nginxparser import dumps
|
||||
from letsencrypt.client.plugins.nginx.obj import Addr, VirtualHost
|
||||
from letsencrypt.client.plugins.nginx.parser import NginxParser, get_best_match
|
||||
from letsencrypt.client.plugins.nginx import nginxparser
|
||||
from letsencrypt.client.plugins.nginx import obj
|
||||
from letsencrypt.client.plugins.nginx import parser
|
||||
from letsencrypt.client.plugins.nginx.tests import util
|
||||
|
||||
|
||||
|
|
@ -26,52 +26,52 @@ class NginxParserTest(util.NginxTest):
|
|||
def test_root_normalized(self):
|
||||
path = os.path.join(self.temp_dir, "foo/////"
|
||||
"bar/../../testdata")
|
||||
parser = NginxParser(path, None)
|
||||
self.assertEqual(parser.root, self.config_path)
|
||||
nparser = parser.NginxParser(path, None)
|
||||
self.assertEqual(nparser.root, self.config_path)
|
||||
|
||||
def test_root_absolute(self):
|
||||
parser = NginxParser(os.path.relpath(self.config_path), None)
|
||||
self.assertEqual(parser.root, self.config_path)
|
||||
nparser = parser.NginxParser(os.path.relpath(self.config_path), None)
|
||||
self.assertEqual(nparser.root, self.config_path)
|
||||
|
||||
def test_root_no_trailing_slash(self):
|
||||
parser = NginxParser(self.config_path + os.path.sep, None)
|
||||
self.assertEqual(parser.root, self.config_path)
|
||||
nparser = parser.NginxParser(self.config_path + os.path.sep, None)
|
||||
self.assertEqual(nparser.root, self.config_path)
|
||||
|
||||
def test_load(self):
|
||||
"""Test recursive conf file parsing.
|
||||
|
||||
"""
|
||||
parser = NginxParser(self.config_path, self.ssl_options)
|
||||
parser.load()
|
||||
self.assertEqual(set([parser.abs_path(x) for x in
|
||||
nparser = parser.NginxParser(self.config_path, self.ssl_options)
|
||||
nparser.load()
|
||||
self.assertEqual(set([nparser.abs_path(x) for x in
|
||||
['foo.conf', 'nginx.conf', 'server.conf',
|
||||
'sites-enabled/default',
|
||||
'sites-enabled/example.com']]),
|
||||
set(parser.parsed.keys()))
|
||||
set(nparser.parsed.keys()))
|
||||
self.assertEqual([['server_name', 'somename alias another.alias']],
|
||||
parser.parsed[parser.abs_path('server.conf')])
|
||||
nparser.parsed[nparser.abs_path('server.conf')])
|
||||
self.assertEqual([[['server'], [['listen', '69.50.225.155:9000'],
|
||||
['listen', '127.0.0.1'],
|
||||
['server_name', '.example.com'],
|
||||
['server_name', 'example.*']]]],
|
||||
parser.parsed[parser.abs_path(
|
||||
nparser.parsed[nparser.abs_path(
|
||||
'sites-enabled/example.com')])
|
||||
|
||||
def test_abs_path(self):
|
||||
parser = NginxParser(self.config_path, self.ssl_options)
|
||||
self.assertEqual('/etc/nginx/*', parser.abs_path('/etc/nginx/*'))
|
||||
nparser = parser.NginxParser(self.config_path, self.ssl_options)
|
||||
self.assertEqual('/etc/nginx/*', nparser.abs_path('/etc/nginx/*'))
|
||||
self.assertEqual(os.path.join(self.config_path, 'foo/bar/'),
|
||||
parser.abs_path('foo/bar/'))
|
||||
nparser.abs_path('foo/bar/'))
|
||||
|
||||
def test_filedump(self):
|
||||
parser = NginxParser(self.config_path, self.ssl_options)
|
||||
parser.filedump('test')
|
||||
nparser = parser.NginxParser(self.config_path, self.ssl_options)
|
||||
nparser.filedump('test')
|
||||
# pylint: disable=protected-access
|
||||
parsed = parser._parse_files(parser.abs_path(
|
||||
parsed = nparser._parse_files(nparser.abs_path(
|
||||
'sites-enabled/example.com.test'))
|
||||
self.assertEqual(3, len(glob.glob(parser.abs_path('*.test'))))
|
||||
self.assertEqual(3, len(glob.glob(nparser.abs_path('*.test'))))
|
||||
self.assertEqual(2, len(
|
||||
glob.glob(parser.abs_path('sites-enabled/*.test'))))
|
||||
glob.glob(nparser.abs_path('sites-enabled/*.test'))))
|
||||
self.assertEqual([[['server'], [['listen', '69.50.225.155:9000'],
|
||||
['listen', '127.0.0.1'],
|
||||
['server_name', '.example.com'],
|
||||
|
|
@ -79,31 +79,34 @@ class NginxParserTest(util.NginxTest):
|
|||
parsed[0])
|
||||
|
||||
def test_get_vhosts(self):
|
||||
parser = NginxParser(self.config_path, self.ssl_options)
|
||||
vhosts = parser.get_vhosts()
|
||||
nparser = parser.NginxParser(self.config_path, self.ssl_options)
|
||||
vhosts = nparser.get_vhosts()
|
||||
|
||||
vhost1 = VirtualHost(parser.abs_path('nginx.conf'),
|
||||
[Addr('', '8080', False, False)],
|
||||
False, True, set(['localhost',
|
||||
r'~^(www\.)?(example|bar)\.']),
|
||||
[])
|
||||
vhost2 = VirtualHost(parser.abs_path('nginx.conf'),
|
||||
[Addr('somename', '8080', False, False),
|
||||
Addr('', '8000', False, False)],
|
||||
False, True, set(['somename',
|
||||
'another.alias', 'alias']), [])
|
||||
vhost3 = VirtualHost(parser.abs_path('sites-enabled/example.com'),
|
||||
[Addr('69.50.225.155', '9000', False, False),
|
||||
Addr('127.0.0.1', '', False, False)],
|
||||
False, True, set(['.example.com', 'example.*']),
|
||||
[])
|
||||
vhost4 = VirtualHost(parser.abs_path('sites-enabled/default'),
|
||||
[Addr('myhost', '', False, True)],
|
||||
False, True, set(['www.example.org']), [])
|
||||
vhost5 = VirtualHost(parser.abs_path('foo.conf'),
|
||||
[Addr('*', '80', True, True)],
|
||||
True, True, set(['*.www.foo.com',
|
||||
'*.www.example.com']), [])
|
||||
vhost1 = obj.VirtualHost(nparser.abs_path('nginx.conf'),
|
||||
[obj.Addr('', '8080', False, False)],
|
||||
False, True,
|
||||
set(['localhost',
|
||||
r'~^(www\.)?(example|bar)\.']),
|
||||
[])
|
||||
vhost2 = obj.VirtualHost(nparser.abs_path('nginx.conf'),
|
||||
[obj.Addr('somename', '8080', False, False),
|
||||
obj.Addr('', '8000', False, False)],
|
||||
False, True,
|
||||
set(['somename', 'another.alias', 'alias']),
|
||||
[])
|
||||
vhost3 = obj.VirtualHost(nparser.abs_path('sites-enabled/example.com'),
|
||||
[obj.Addr('69.50.225.155', '9000',
|
||||
False, False),
|
||||
obj.Addr('127.0.0.1', '', False, False)],
|
||||
False, True,
|
||||
set(['.example.com', 'example.*']), [])
|
||||
vhost4 = obj.VirtualHost(nparser.abs_path('sites-enabled/default'),
|
||||
[obj.Addr('myhost', '', False, True)],
|
||||
False, True, set(['www.example.org']), [])
|
||||
vhost5 = obj.VirtualHost(nparser.abs_path('foo.conf'),
|
||||
[obj.Addr('*', '80', True, True)],
|
||||
True, True, set(['*.www.foo.com',
|
||||
'*.www.example.com']), [])
|
||||
|
||||
self.assertEqual(5, len(vhosts))
|
||||
example_com = [x for x in vhosts if 'example.com' in x.filep][0]
|
||||
|
|
@ -118,39 +121,39 @@ class NginxParserTest(util.NginxTest):
|
|||
self.assertEquals(vhost2, somename)
|
||||
|
||||
def test_add_server_directives(self):
|
||||
parser = NginxParser(self.config_path, self.ssl_options)
|
||||
parser.add_server_directives(parser.abs_path('nginx.conf'),
|
||||
set(['localhost',
|
||||
r'~^(www\.)?(example|bar)\.']),
|
||||
[['foo', 'bar'], ['ssl_certificate',
|
||||
'/etc/ssl/cert.pem']])
|
||||
nparser = parser.NginxParser(self.config_path, self.ssl_options)
|
||||
nparser.add_server_directives(nparser.abs_path('nginx.conf'),
|
||||
set(['localhost',
|
||||
r'~^(www\.)?(example|bar)\.']),
|
||||
[['foo', 'bar'], ['ssl_certificate',
|
||||
'/etc/ssl/cert.pem']])
|
||||
ssl_re = re.compile(r'foo bar;\n\s+ssl_certificate /etc/ssl/cert.pem')
|
||||
self.assertEqual(1, len(re.findall(ssl_re, dumps(parser.parsed[
|
||||
parser.abs_path('nginx.conf')]))))
|
||||
parser.add_server_directives(parser.abs_path('server.conf'),
|
||||
set(['alias', 'another.alias',
|
||||
'somename']),
|
||||
[['foo', 'bar'], ['ssl_certificate',
|
||||
'/etc/ssl/cert2.pem']])
|
||||
self.assertEqual(parser.parsed[parser.abs_path('server.conf')],
|
||||
self.assertEqual(1, len(re.findall(ssl_re, nginxparser.dumps(
|
||||
nparser.parsed[nparser.abs_path('nginx.conf')]))))
|
||||
nparser.add_server_directives(nparser.abs_path('server.conf'),
|
||||
set(['alias', 'another.alias',
|
||||
'somename']),
|
||||
[['foo', 'bar'], ['ssl_certificate',
|
||||
'/etc/ssl/cert2.pem']])
|
||||
self.assertEqual(nparser.parsed[nparser.abs_path('server.conf')],
|
||||
[['server_name', 'somename alias another.alias'],
|
||||
['foo', 'bar'],
|
||||
['ssl_certificate', '/etc/ssl/cert2.pem']])
|
||||
|
||||
def test_replace_server_directives(self):
|
||||
parser = NginxParser(self.config_path, self.ssl_options)
|
||||
nparser = parser.NginxParser(self.config_path, self.ssl_options)
|
||||
target = set(['.example.com', 'example.*'])
|
||||
filep = parser.abs_path('sites-enabled/example.com')
|
||||
parser.add_server_directives(
|
||||
filep = nparser.abs_path('sites-enabled/example.com')
|
||||
nparser.add_server_directives(
|
||||
filep, target, [['server_name', 'foo bar']], True)
|
||||
self.assertEqual(
|
||||
parser.parsed[filep],
|
||||
nparser.parsed[filep],
|
||||
[[['server'], [['listen', '69.50.225.155:9000'],
|
||||
['listen', '127.0.0.1'],
|
||||
['server_name', 'foo bar'],
|
||||
['server_name', 'foo bar']]]])
|
||||
self.assertRaises(LetsEncryptMisconfigurationError,
|
||||
parser.add_server_directives,
|
||||
nparser.add_server_directives,
|
||||
filep, set(['foo', 'bar']),
|
||||
[['ssl_certificate', 'cert.pem']], True)
|
||||
|
||||
|
|
@ -184,17 +187,18 @@ class NginxParserTest(util.NginxTest):
|
|||
(None, None)]
|
||||
|
||||
for i, winner in enumerate(winners):
|
||||
self.assertEqual(winner, get_best_match(target_name, names[i]))
|
||||
self.assertEqual(winner,
|
||||
parser.get_best_match(target_name, names[i]))
|
||||
|
||||
def test_get_all_certs_keys(self):
|
||||
parser = NginxParser(self.config_path, self.ssl_options)
|
||||
filep = parser.abs_path('sites-enabled/example.com')
|
||||
parser.add_server_directives(filep,
|
||||
set(['.example.com', 'example.*']),
|
||||
[['ssl_certificate', 'foo.pem'],
|
||||
['ssl_certificate_key', 'bar.key'],
|
||||
['listen', '443 ssl']])
|
||||
c_k = parser.get_all_certs_keys()
|
||||
nparser = parser.NginxParser(self.config_path, self.ssl_options)
|
||||
filep = nparser.abs_path('sites-enabled/example.com')
|
||||
nparser.add_server_directives(filep,
|
||||
set(['.example.com', 'example.*']),
|
||||
[['ssl_certificate', 'foo.pem'],
|
||||
['ssl_certificate_key', 'bar.key'],
|
||||
['listen', '443 ssl']])
|
||||
c_k = nparser.get_all_certs_keys()
|
||||
self.assertEqual(set([('foo.pem', 'bar.key', filep)]), c_k)
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue