Merge pull request #9597 from certbot/revert-nginx-lua-detection

nginx: fix performance regression caused by #9475

certbot-nginx/certbot_nginx/_internal/configurator.py

@@ -1061,8 +1061,8 @@ def get_version(self) -> Tuple[int, ...]:
 
         product_name, product_version = version_matches[0]
         if product_name != 'nginx':
-            logger.warning("nginx derivative %s is not officially supported by "
-                           "Certbot.", product_name)
+            logger.warning("NGINX derivative %s is not officially supported by"
+                           " certbot", product_name)
 
         nginx_version = tuple(int(i) for i in product_version.split("."))
 

certbot-nginx/certbot_nginx/_internal/nginxparser.py

@@ -33,18 +33,6 @@
 logger = logging.getLogger(__name__)
 
 
-class UnsupportedDirectiveException(RuntimeError):
-    """Exception when encountering an nginx directive which is not supported
-    by this parser."""
-
-    directive_name: str
-    line_no: int
-
-    def __init__(self, directive_name: str, line_no: int) -> None:
-        self.directive_name = directive_name
-        self.line_no = line_no
-
-
 class RawNginxParser:
     # pylint: disable=pointless-statement
     """A class that parses nginx configuration with pyparsing."""
@@ -86,7 +74,6 @@ class RawNginxParser:
 
     def __init__(self, source: str) -> None:
         self.source = source
-        self.whitespace_token_group.addParseAction(self._check_disallowed_directive)
 
     def parse(self) -> ParseResults:
         """Returns the parsed tree."""
@@ -96,12 +83,6 @@ def as_list(self) -> List[Any]:
         """Returns the parsed tree as a list."""
         return self.parse().asList()
 
-    def _check_disallowed_directive(self, _source: str, line: int, results: ParseResults) -> None:
-        # *_by_lua_block might be first or second result, due to optional leading whitespace
-        toks = [t for t in results[0:2] if isinstance(t, str) and t.endswith("_by_lua_block")]
-        if toks:
-            raise UnsupportedDirectiveException(toks[0], line)
-
 
 class RawNginxDumper:
     """A class that dumps nginx configuration from the provided tree."""

certbot-nginx/certbot_nginx/_internal/parser.py

@@ -223,12 +223,6 @@ def _parse_files(self, filepath: str, override: bool = False) -> List[UnspacedLi
                                "supported.", item)
             except pyparsing.ParseException as err:
                 logger.warning("Could not parse file: %s due to %s", item, err)
-            except nginxparser.UnsupportedDirectiveException as e:
-                logger.warning(
-                    "%s:%d contained the '%s' directive, which is not supported by Certbot. The "
-                    "file has been ignored, which may prevent Certbot from functioning properly. "
-                    "Consider using the --webroot plugin and manually installing the certificate.",
-                    item, e.line_no, e.directive_name)
         return trees
 
     def _find_config_root(self) -> str:

certbot-nginx/tests/nginxparser_test.py

@@ -14,7 +14,6 @@
 from certbot_nginx._internal.nginxparser import loads
 from certbot_nginx._internal.nginxparser import RawNginxParser
 from certbot_nginx._internal.nginxparser import UnspacedList
-from certbot_nginx._internal.nginxparser import UnsupportedDirectiveException
 import test_util as util
 
 FIRST = operator.itemgetter(0)
@@ -357,68 +356,6 @@ def test_edge_cases(self):
         parsed = loads("")
         assert parsed == []
 
-    def test_lua(self):
-        # https://github.com/certbot/certbot/issues/9066
-        with pytest.raises(UnsupportedDirectiveException):
-            loads("""
-        location /foo {
-            content_by_lua_block {
-                ngx.say('Hello World')
-            }
-        }
-        """)
-
-        # Without leading whitespace
-        with pytest.raises(UnsupportedDirectiveException):
-            loads("""
-        location /foo {content_by_lua_block {
-                ngx.say('Hello World')
-            }
-        }
-        """)
-
-        # Doesn't trigger if it's commented or not in the right position.
-        parsed = loads("""
-        location /foo {server_name content_by_lua_block;
-            #content_by_lua_block {
-            # ngx.say('Hello World')
-            # }
-        }
-        """)
-        assert parsed == \
-            [
-                [['location', '/foo'],
-                [['server_name', 'content_by_lua_block'],
-                 ['#', 'content_by_lua_block {'],
-                 ['#', " ngx.say('Hello World')"],
-                 ['#', ' }']
-                ]]
-            ]
-
-        # *_by_lua should parse successfully.
-        parsed = loads("""
-            location / {
-                set $a 32;
-                set $b 56;
-                set_by_lua $sum
-                    'return tonumber(ngx.arg[1]) + tonumber(ngx.arg[2])'
-                    $a $b;
-                content_by_lua '
-                    ngx.say("foo");
-                ';
-            }
-        """)
-        assert parsed == \
-            [
-                [['location', '/'],
-                 [['set', '$a', '32'],
-                  ['set', '$b', '56'],
-                  ['set_by_lua', '$sum',
-                   "'return tonumber(ngx.arg[1]) + tonumber(ngx.arg[2])'", '$a', '$b'
-                  ],
-                  ['content_by_lua', '\'\n                    ngx.say("foo");\n                \'']
-                 ]]
-            ]
 
 class TestUnspacedList(unittest.TestCase):
     """Test the UnspacedList data structure"""

certbot-nginx/tests/parser_test.py

@@ -4,8 +4,6 @@
 import shutil
 import sys
 from typing import List
-import unittest
-from unittest import mock
 
 import pytest
 
@@ -533,14 +531,6 @@ def test_invalid_unicode_characters_in_ssl_options(self):
             for output in log.output
         )
 
-    @mock.patch('certbot_nginx._internal.parser.logger.warning')
-    def test_load_unsupported_directive_logged(self, mock_warn):
-        nparser = parser.NginxParser(self.config_path)
-        nparser.config_root = nparser.abs_path('unsupported_directives.conf')
-        nparser.load()
-        assert mock_warn.call_count == 1
-        assert "which is not supported by Certbot" in mock_warn.call_args[0][0]
-
 
 if __name__ == "__main__":
     sys.exit(pytest.main(sys.argv[1:] + [__file__]))  # pragma: no cover

certbot/CHANGELOG.md

@@ -15,7 +15,7 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 
 ### Fixed
 
-*
+* Reverted [#9475](https://github.com/certbot/certbot/pull/9475) due to a performance regression in large nginx deployments.
 
 More details about these changes can be found on our GitHub repo.