From b1982b3b7c0e1fc55f728e445f15f70ea963ce52 Mon Sep 17 00:00:00 2001 From: Jarkko Jaakola Date: Thu, 9 Jan 2025 14:47:58 +0200 Subject: [PATCH] fix: max http request size for Karapace REST --- src/karapace/config.py | 24 +++++++++++ src/karapace/karapace_all.py | 1 + src/karapace/rapu.py | 2 +- tests/unit/test_config.py | 84 ++++++++++++++---------------------- 4 files changed, 59 insertions(+), 52 deletions(-) diff --git a/src/karapace/config.py b/src/karapace/config.py index e9488b772..6663f6bd7 100644 --- a/src/karapace/config.py +++ b/src/karapace/config.py @@ -146,6 +146,29 @@ def get_rest_base_uri(self) -> str: or f"{self.advertised_protocol}://{self.get_advertised_hostname()}:{self.get_advertised_port()}" ) + def get_max_request_size(self) -> int: + """The AIO web application requires max request size set. + + Set the aiohttp client max size if REST Proxy is enabled and producer max request configuration is altered + from default and aiohttp client max size is not set + Use the http request max size from the configuration without altering if set. + """ + if ( + self.karapace_rest + and self.producer_max_request_size > DEFAULT_PRODUCER_MAX_REQUEST + and self.http_request_max_size is None + ): + # REST Proxy API configuration for producer max request size must be taken into account + # also for the aiohttp.web.Application client max size. + # Always add the aiohttp default client max size as the headroom above the producer max request size. + # The input JSON size for REST Proxy is not easy to estimate, lot of small records in single request has + # a lot of overhead due to JSON structure. + return self.producer_max_request_size + DEFAULT_AIOHTTP_CLIENT_MAX_SIZE + elif self.http_request_max_size is None: + # Set the default aiohttp client max size + return DEFAULT_AIOHTTP_CLIENT_MAX_SIZE + return self.http_request_max_size + def to_env_str(self) -> str: env_lines: list[str] = [] for key, value in self.model_dump().items(): @@ -154,6 +177,7 @@ def to_env_str(self) -> str: return "\n".join(env_lines) def set_config_defaults(self, new_config: Mapping[str, str] | None = None) -> Config: + """TODO: Used from tests, this requires removal.""" config = deepcopy(self) if new_config: for key, value in new_config.items(): diff --git a/src/karapace/karapace_all.py b/src/karapace/karapace_all.py index 0c9acf253..79daae4de 100644 --- a/src/karapace/karapace_all.py +++ b/src/karapace/karapace_all.py @@ -23,6 +23,7 @@ def main( config: Config = Provide[KarapaceContainer.config], prometheus: PrometheusInstrumentation = Provide[KarapaceContainer.prometheus], ) -> int: + config.set_config_defaults() parser = argparse.ArgumentParser(prog="karapace", description="Karapace: Your Kafka essentials in one tool") parser.add_argument("--version", action="version", help="show program version", version=karapace_version.__version__) parser.parse_args() diff --git a/src/karapace/rapu.py b/src/karapace/rapu.py index 34d6e697c..9d7671f3b 100644 --- a/src/karapace/rapu.py +++ b/src/karapace/rapu.py @@ -173,7 +173,7 @@ def __init__( def _create_aiohttp_application(self, *, config: Config) -> aiohttp.web.Application: if config.http_request_max_size: - return aiohttp.web.Application(client_max_size=config.http_request_max_size) + return aiohttp.web.Application(client_max_size=config.get_max_request_size()) return aiohttp.web.Application() async def close_by_app(self, app: aiohttp.web.Application) -> None: diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index d1fa63204..a75bb99c9 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -5,55 +5,37 @@ See LICENSE for details """ +from karapace.config import Config from karapace.constants import DEFAULT_AIOHTTP_CLIENT_MAX_SIZE, DEFAULT_PRODUCER_MAX_REQUEST -from karapace.container import KarapaceContainer - - -def test_http_request_max_size(karapace_container: KarapaceContainer) -> None: - config = karapace_container.config().set_config_defaults( - { - "karapace_rest": False, - "producer_max_request_size": DEFAULT_PRODUCER_MAX_REQUEST + 1024, - } - ) - assert config.http_request_max_size == DEFAULT_AIOHTTP_CLIENT_MAX_SIZE - - config = karapace_container.config().set_config_defaults( - { - "karapace_rest": False, - "http_request_max_size": 1024, - } - ) - assert config.http_request_max_size == 1024 - - config = karapace_container.config().set_config_defaults( - { - "karapace_rest": True, - } - ) - assert config.http_request_max_size == DEFAULT_AIOHTTP_CLIENT_MAX_SIZE - - config = karapace_container.config().set_config_defaults( - { - "karapace_rest": True, - "producer_max_request_size": 1024, - } - ) - assert config.http_request_max_size == DEFAULT_AIOHTTP_CLIENT_MAX_SIZE - - config = karapace_container.config().set_config_defaults( - { - "karapace_rest": True, - "producer_max_request_size": DEFAULT_PRODUCER_MAX_REQUEST + 1024, - } - ) - assert config.http_request_max_size == DEFAULT_PRODUCER_MAX_REQUEST + 1024 + DEFAULT_AIOHTTP_CLIENT_MAX_SIZE - - config = karapace_container.config().set_config_defaults( - { - "karapace_rest": True, - "producer_max_request_size": DEFAULT_PRODUCER_MAX_REQUEST + 1024, - "http_request_max_size": 1024, - } - ) - assert config.http_request_max_size == 1024 + + +def test_http_request_max_size() -> None: + config = Config() + config.karapace_rest = False + config.producer_max_request_size = DEFAULT_PRODUCER_MAX_REQUEST + 1024 + assert config.get_max_request_size() == DEFAULT_AIOHTTP_CLIENT_MAX_SIZE + + config = Config() + config.karapace_rest = False + config.http_request_max_size = 1024 + assert config.get_max_request_size() == 1024 + + config = Config() + config.karapace_rest = True + assert config.get_max_request_size() == DEFAULT_AIOHTTP_CLIENT_MAX_SIZE + + config = Config() + config.karapace_rest = True + config.producer_max_request_size = 1024 + assert config.get_max_request_size() == DEFAULT_AIOHTTP_CLIENT_MAX_SIZE + + config = Config() + config.karapace_rest = True + config.producer_max_request_size = DEFAULT_PRODUCER_MAX_REQUEST + 1024 + assert config.get_max_request_size() == DEFAULT_PRODUCER_MAX_REQUEST + 1024 + DEFAULT_AIOHTTP_CLIENT_MAX_SIZE + + config = Config() + config.karapace_rest = True + config.http_request_max_size = 1024 + config.producer_max_request_size = DEFAULT_PRODUCER_MAX_REQUEST + 1024 + assert config.get_max_request_size() == 1024