Allow Nginx to insert include files with comments inside (#4666) (#4668)

* add failing test case

* allow include files to insert comments

* lint

certbot-nginx/certbot_nginx/parser.py

@@ -529,7 +529,10 @@ def _add_directive(block, directive, replace):
 
     """
     directive = nginxparser.UnspacedList(directive)
-    if len(directive) == 0 or directive[0] == '#':
+    def is_whitespace_or_comment(directive):
+        """Is this directive either a whitespace or comment directive?"""
+        return len(directive) == 0 or directive[0] == '#'
+    if is_whitespace_or_comment(directive):
         # whitespace or comment
         block.append(directive)
         return
@@ -574,7 +577,8 @@ def _add_directive(block, directive, replace):
             for included_directive in included_directives:
                 included_dir_loc = find_location(included_directive)
                 included_dir_name = included_directive[0]
-                if not can_append(included_dir_loc, included_dir_name):
+                if not is_whitespace_or_comment(included_directive) \
+                    and not can_append(included_dir_loc, included_dir_name):
                     if block[included_dir_loc] != included_directive:
                         raise errors.MisconfigurationError(err_fmt.format(included_directive,
                             block[included_dir_loc]))

certbot-nginx/certbot_nginx/tests/parser_test.py

@@ -13,7 +13,7 @@ from certbot_nginx import parser
 from certbot_nginx.tests import util
 
 
-class NginxParserTest(util.NginxTest):
+class NginxParserTest(util.NginxTest): #pylint: disable=too-many-public-methods
     """Nginx Parser Test."""
 
     def setUp(self):
@@ -230,6 +230,33 @@ class NginxParserTest(util.NginxTest):
                            ['ssl_certificate', '/etc/ssl/cert2.pem']],
                           replace=False)
 
+    def test_comment_is_repeatable(self):
+        nparser = parser.NginxParser(self.config_path)
+        example_com = nparser.abs_path('sites-enabled/example.com')
+        mock_vhost = obj.VirtualHost(example_com,
+                                     None, None, None,
+                                     set(['.example.com', 'example.*']),
+                                     None, [0])
+        nparser.add_server_directives(mock_vhost,
+                                      [['\n  ', '#', ' ', 'what a nice comment']],
+                                      replace=False)
+        nparser.add_server_directives(mock_vhost,
+                                      [['\n  ', 'include', ' ',
+                                      nparser.abs_path('comment_in_file.conf')]],
+                                      replace=False)
+        from certbot_nginx.parser import COMMENT
+        self.assertEqual(nparser.parsed[example_com],
+            [[['server'], [['listen', '69.50.225.155:9000'],
+                           ['listen', '127.0.0.1'],
+                           ['server_name', '.example.com'],
+                           ['server_name', 'example.*'],
+                           ['#', ' ', 'what a nice comment'],
+                           [],
+                           ['include', nparser.abs_path('comment_in_file.conf')],
+                           ['#', COMMENT],
+                           []]]]
+)
+
     def test_replace_server_directives(self):
         nparser = parser.NginxParser(self.config_path)
         target = set(['.example.com', 'example.*'])

certbot-nginx/certbot_nginx/tests/testdata/etc_nginx/comment_in_file.conf

@@ -0,0 +1 @@
+# a comment inside a file