From 7dc80a7e4c3faa14835e973db48a6141e0712c63 Mon Sep 17 00:00:00 2001
From: Jarkko Jaakola <jarkko.jaakola@aiven.io>
Date: Thu, 9 Jan 2025 14:47:58 +0200
Subject: [PATCH] fix: max http request size for Karapace REST

---
 src/karapace/config.py    | 24 +++++++++++
 tests/unit/test_config.py | 84 +++++++++++++++------------------------
 2 files changed, 57 insertions(+), 51 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/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