Merge pull request #602 from quinox/preserve_comments

Preserve comments, don't silently truncate config files
This commit is contained in:
James Kasten 2015-07-13 11:47:29 -07:00
commit 01481aabd9
8 changed files with 137 additions and 44 deletions

View file

@ -3,8 +3,9 @@ import string
from pyparsing import (
Literal, White, Word, alphanums, CharsNotIn, Forward, Group,
Optional, OneOrMore, Regex, ZeroOrMore, pythonStyleComment)
Optional, OneOrMore, Regex, ZeroOrMore)
from pyparsing import stringEnd
from pyparsing import restOfLine
class RawNginxParser(object):
# pylint: disable=expression-not-assigned
@ -24,7 +25,8 @@ class RawNginxParser(object):
modifier = Literal("=") | Literal("~*") | Literal("~") | Literal("^~")
# rules
assignment = (key + Optional(space + value) + semicolon)
comment = Literal('#') + restOfLine()
assignment = (key + Optional(space + value, default=None) + semicolon)
location_statement = Optional(space + modifier) + Optional(space + location)
if_statement = Literal("if") + space + Regex(r"\(.+\)") + space
block = Forward()
@ -32,10 +34,10 @@ class RawNginxParser(object):
block << Group(
(Group(key + location_statement) ^ Group(if_statement))
+ left_bracket
+ Group(ZeroOrMore(Group(assignment) | block))
+ Group(ZeroOrMore(Group(comment | assignment) | block))
+ right_bracket)
script = OneOrMore(Group(assignment) | block).ignore(pythonStyleComment)
script = OneOrMore(Group(comment | assignment) | block) + stringEnd
def __init__(self, source):
self.source = source
@ -60,30 +62,30 @@ class RawNginxDumper(object):
"""Iterates the dumped nginx content."""
blocks = blocks or self.blocks
for key, values in blocks:
if current_indent:
yield spacer
indentation = spacer * current_indent
if isinstance(key, list):
if current_indent:
yield ''
yield indentation + spacer.join(key) + ' {'
for parameter in values:
if isinstance(parameter[0], list):
dumped = self.__iter__(
[parameter],
current_indent + self.indentation)
for line in dumped:
yield line
else:
dumped = spacer.join(parameter) + ';'
yield spacer * (
current_indent + self.indentation) + dumped
dumped = self.__iter__([parameter], current_indent + self.indentation)
for line in dumped:
yield line
yield indentation + '}'
else:
yield spacer * current_indent + key + spacer + values + ';'
if key == '#':
yield spacer * current_indent + key + values
else:
if values is None:
yield spacer * current_indent + key + ';'
else:
yield spacer * current_indent + key + spacer + values + ';'
def as_string(self):
"""Return the parsed block as a string."""
return '\n'.join(self)
return '\n'.join(self) + '\n'
# Shortcut functions to respect Python's serialization interface

View file

@ -111,6 +111,10 @@ class NginxConfiguratorTest(util.NginxTest):
self.config.parser.load()
parsed_example_conf = util.filter_comments(self.config.parser.parsed[example_conf])
parsed_server_conf = util.filter_comments(self.config.parser.parsed[server_conf])
parsed_nginx_conf = util.filter_comments(self.config.parser.parsed[nginx_conf])
access_log = os.path.join(self.work_dir, "access.log")
error_log = os.path.join(self.work_dir, "error.log")
self.assertEqual([[['server'],
@ -125,9 +129,9 @@ class NginxConfiguratorTest(util.NginxTest):
['ssl_certificate_key', 'example/key.pem'],
['include',
self.config.parser.loc["ssl_options"]]]]],
self.config.parser.parsed[example_conf])
parsed_example_conf)
self.assertEqual([['server_name', 'somename alias another.alias']],
self.config.parser.parsed[server_conf])
parsed_server_conf)
self.assertEqual([['server'],
[['listen', '8000'],
['listen', 'somename:8080'],
@ -142,7 +146,7 @@ class NginxConfiguratorTest(util.NginxTest):
['ssl_certificate_key', '/etc/nginx/key.pem'],
['include',
self.config.parser.loc["ssl_options"]]]],
self.config.parser.parsed[nginx_conf][-1][-1][-1])
parsed_nginx_conf[-1][-1][-1])
def test_get_all_certs_keys(self):
nginx_conf = self.config.parser.abs_path('nginx.conf')

View file

@ -2,6 +2,8 @@
import operator
import unittest
from pyparsing import ParseException
from letsencrypt_nginx.nginxparser import (
RawNginxParser, load, dumps, dump)
from letsencrypt_nginx.tests import util
@ -42,7 +44,7 @@ class TestRawNginxParser(unittest.TestCase):
['server_name', 'foo.com'],
['root', '/home/ubuntu/sites/foo/'],
[['location', '/status'], [
['check_status'],
['check_status', None],
[['types'], [['image/jpeg', 'jpg']]],
]]
]]])
@ -52,17 +54,20 @@ class TestRawNginxParser(unittest.TestCase):
'server {\n'
' listen 80;\n'
' server_name foo.com;\n'
' root /home/ubuntu/sites/foo/;\n \n'
' root /home/ubuntu/sites/foo/;\n'
'\n'
' location /status {\n'
' check_status;\n \n'
' check_status;\n'
'\n'
' types {\n'
' image/jpeg jpg;\n'
' }\n'
' }\n'
'}')
'}\n')
def test_parse_from_file(self):
parsed = load(open(util.get_data_filename('foo.conf')))
with open(util.get_data_filename('foo.conf')) as handle:
parsed = util.filter_comments(load(handle))
self.assertEqual(
parsed,
[['user', 'www-data'],
@ -85,7 +90,8 @@ class TestRawNginxParser(unittest.TestCase):
)
def test_parse_from_file2(self):
parsed = load(open(util.get_data_filename('edge_cases.conf')))
with open(util.get_data_filename('edge_cases.conf')) as handle:
parsed = util.filter_comments(load(handle))
self.assertEqual(
parsed,
[[['server'], [['server_name', 'simple']]],
@ -104,8 +110,13 @@ class TestRawNginxParser(unittest.TestCase):
['blah', '"hello;world"'],
['try_files', '$uri @rewrites']]]]]])
def test_abort_on_parse_failure(self):
with open(util.get_data_filename('broken.conf')) as handle:
self.assertRaises(ParseException, load, handle)
def test_dump_as_file(self):
parsed = load(open(util.get_data_filename('nginx.conf')))
with open(util.get_data_filename('nginx.conf')) as handle:
parsed = util.filter_comments(load(handle))
parsed[-1][-1].append([['server'],
[['listen', '443 ssl'],
['server_name', 'localhost'],
@ -117,12 +128,38 @@ class TestRawNginxParser(unittest.TestCase):
[['location', '/'],
[['root', 'html'],
['index', 'index.html index.htm']]]]])
_file = open(util.get_data_filename('nginx.new.conf'), 'w')
dump(parsed, _file)
_file.close()
parsed_new = load(open(util.get_data_filename('nginx.new.conf')))
with open(util.get_data_filename('nginx.new.conf'), 'w') as handle:
dump(parsed, handle)
with open(util.get_data_filename('nginx.new.conf')) as handle:
parsed_new = util.filter_comments(load(handle))
self.assertEquals(parsed, parsed_new)
def test_comments(self):
with open(util.get_data_filename('minimalistic_comments.conf')) as handle:
parsed = load(handle)
with open(util.get_data_filename('minimalistic_comments.new.conf'), 'w') as handle:
dump(parsed, handle)
with open(util.get_data_filename('minimalistic_comments.new.conf')) as handle:
parsed_new = load(handle)
self.assertEquals(parsed, parsed_new)
self.assertEqual(parsed_new, [
['#', " Use bar.conf when it's a full moon!"],
['include', 'foo.conf'],
['#', ' Kilroy was here'],
['check_status', None],
[['server'],
[['#', ''],
['#', " Don't forget to open up your firewall!"],
['#', ''],
['listen', '1234'],
['#', ' listen 80;']]],
])
if __name__ == '__main__':
unittest.main() # pragma: no cover

View file

@ -0,0 +1,12 @@
# A faulty configuration file
pid logs/nginx.pid;
events {
worker_connections 1024;
}
include foo.conf;
@@@

View file

@ -0,0 +1,12 @@
# Use bar.conf when it's a full moon!
include foo.conf; # Kilroy was here
check_status;
server {
#
# Don't forget to open up your firewall!
#
listen 1234;
# listen 80;
}

View file

@ -0,0 +1,11 @@
# Use bar.conf when it's a full moon!
include foo.conf;
# Kilroy was here
check_status;
server {
#
# Don't forget to open up your firewall!
#
listen 1234;
# listen 80;
}

View file

@ -20,52 +20,52 @@ http {
tcp_nopush on;
keepalive_timeout 0;
gzip on;
server {
listen 8080;
server_name localhost;
server_name ~^(www\.)?(example|bar)\.;
charset koi8-r;
access_log logs/host.access.log main;
location / {
root html;
index index.html index.htm;
}
error_page 404 /404.html;
error_page 500 502 503 504 /50x.html;
location = /50x.html {
root html;
}
location ~ \.php$ {
proxy_pass http://127.0.0.1;
}
location ~ \.php$ {
root html;
fastcgi_pass 127.0.0.1:9000;
fastcgi_index index.php;
fastcgi_param SCRIPT_FILENAME /scripts$fastcgi_script_name;
}
location ~ /\.ht {
deny all;
}
}
server {
listen 8000;
listen somename:8080;
include server.conf;
location / {
root html;
index index.html index.htm;
}
}
server {
listen 443 ssl;
server_name localhost;
@ -74,10 +74,10 @@ http {
ssl_session_cache shared:SSL:1m;
ssl_session_timeout 5m;
ssl_ciphers HIGH:!aNULL:!MD5;
location / {
root html;
index index.html index.htm;
}
}
}
}

View file

@ -59,3 +59,18 @@ def get_nginx_configurator(
version=version)
config.prepare()
return config
def filter_comments(tree):
"""Filter comment nodes from parsed configurations."""
def traverse(tree):
"""Generator dropping comment nodes"""
for key, values in tree:
if isinstance(key, list):
yield [key, filter_comments(values)]
else:
if key != '#':
yield [key, values]
return list(traverse(tree))