add more ability to control upload

docs/configuration.rst

@@ -106,8 +106,10 @@ Web server settings. If any of ``host``/``port`` is not set, web integration wil
 * ``debug`` - enable debug toolbar, boolean, optional, default ``no``.
 * ``debug_check_host`` - check hosts to access debug toolbar, boolean, optional, default ``no``.
 * ``debug_allowed_hosts`` - allowed hosts to get access to debug toolbar, space separated list of string, optional.
+* ``enable_archive_upload`` - allow to upload packages via HTTP (i.e. call of ``/api/v1/service/upload`` uri), boolean, optional, default ``no``.
 * ``host`` - host to bind, string, optional.
 * ``index_url`` - full url of the repository index page, string, optional.
+* ``max_body_size`` - max body size in bytes to be validated for archive upload, integer, optional. If not set, validation will be disabled.
 * ``password`` - password to authorize in web service in order to update service status, string, required in case if authorization enabled.
 * ``port`` - port to bind, int, optional.
 * ``static_path`` - path to directory with static files, string, required.

docs/faq.rst

@@ -773,7 +773,14 @@ In this example the following settings are assumed:
 Master node configuration
 """""""""""""""""""""""""
 
-The only requirements for the master node is that API must be available for worker nodes to call (e.g. port must be exposed to internet, or local network in case of VPN, etc). In addition, the following settings are recommended:
+The only requirements for the master node is that API must be available for worker nodes to call (e.g. port must be exposed to internet, or local network in case of VPN, etc) and file upload must be enabled:
+
+.. code-block:: ini
+
+   [web]
+   enable_archive_upload = yes
+
+In addition, the following settings are recommended for the master node:
 
 *
   As it has been mentioned above, it is recommended to enable authentication (see `How to enable basic authorization`_) and create system user which will be used later. Later this user (if any) will be referenced as ``worker-user``.
@@ -784,7 +791,7 @@ The only requirements for the master node is that API must be available for work
   .. code-block:: ini
 
      [web]
-     wait_timeout = -1
+     wait_timeout = 0
 
 Worker nodes configuration
 """"""""""""""""""""""""""
@@ -838,8 +845,18 @@ Worker nodes configuration
       [build]
       triggers = ahriman.core.gitremote.RemotePullTrigger ahriman.core.upload.UploadTrigger ahriman.core.report.ReportTrigger ahriman.core.gitremote.RemotePushTrigger
 
-Double node docker example
+In addition, the following settings are recommended for workers:
-""""""""""""""""""""""""""
+
+*
+  You might want to wait until report trigger will be completed; in this case the following option must be set:
+
+  .. code-block:: ini
+
+     [remote-call]
+     wait_timeout = 0
+
+Double node minimal docker example
+""""""""""""""""""""""""""""""""""
 
 Master node config (``master.ini``) as:
 
@@ -848,6 +865,11 @@ Master node config (``master.ini``) as:
    [auth]
    target = mapping
 
+   [web]
+   enable_archive_upload = yes
+   wait_timeout = 0
+
+
 Command to run master node:
 
 .. code-block:: shell
@@ -873,6 +895,7 @@ The user ``worker-user`` has been created additionally. Worker node config (``wo
 
    [remote-call]
    manual = yes
+   wait_timeout = 0
 
    [build]
    triggers = ahriman.core.gitremote.RemotePullTrigger ahriman.core.upload.UploadTrigger ahriman.core.report.ReportTrigger ahriman.core.gitremote.RemotePushTrigger

src/ahriman/core/configuration/schema.py

@@ -228,6 +228,10 @@ CONFIGURATION_SCHEMA: ConfigurationSchema = {
                 "coerce": "list",
                 "schema": {"type": "string"},
             },
+            "enable_archive_upload": {
+                "type": "boolean",
+                "coerce": "boolean",
+            },
             "host": {
                 "type": "string",
                 "is_ip_address": ["localhost"],
@@ -236,6 +240,11 @@ CONFIGURATION_SCHEMA: ConfigurationSchema = {
                 "type": "string",
                 "is_url": ["http", "https"],
             },
+            "max_body_size": {
+                "type": "integer",
+                "coerce": "integer",
+                "min": 0,
+            },
             "password": {
                 "type": "string",
             },

src/ahriman/core/status/web_client.py

@@ -22,6 +22,7 @@ import logging
 import requests
 
 from collections.abc import Generator
+from functools import cached_property
 from typing import Any, IO, Literal
 from urllib.parse import quote_plus as urlencode
 
@@ -44,6 +45,7 @@ class WebClient(Client, LazyLogging):
         address(str): address of the web service
         suppress_errors(bool): suppress logging errors (e.g. if no web server available)
         user(User | None): web service user descriptor
+        use_unix_socket(bool): use websocket or not
     """
 
     _login_url = "/api/v1/login"
@@ -56,13 +58,21 @@ class WebClient(Client, LazyLogging):
         Args:
             configuration(Configuration): configuration instance
         """
-        self.address, use_unix_socket = self.parse_address(configuration)
+        self.address, self.use_unix_socket = self.parse_address(configuration)
         self.user = User.from_option(
             configuration.get("web", "username", fallback=None),
             configuration.get("web", "password", fallback=None))
         self.suppress_errors = configuration.getboolean("settings", "suppress_http_log_errors", fallback=False)
 
-        self.__session = self._create_session(use_unix_socket=use_unix_socket)
+    @cached_property
+    def session(self) -> requests.Session:
+        """
+        get or create session
+
+        Returns:
+            request.Session: created session object
+        """
+        return self._create_session(use_unix_socket=self.use_unix_socket)
 
     @staticmethod
     def _logs_url(package_base: str) -> str:
@@ -130,7 +140,7 @@ class WebClient(Client, LazyLogging):
             if session is not None:
                 yield session  # use session from arguments
             else:
-                yield self.__session  # use instance generated session
+                yield self.session  # use instance generated session
         except requests.RequestException as e:
             if self.suppress_errors:
                 return

src/ahriman/core/upload/http_upload.py

@@ -20,6 +20,7 @@
 import hashlib
 import requests
 
+from functools import cached_property
 from pathlib import Path
 from typing import Any
 
@@ -52,6 +53,16 @@ class HttpUpload(Upload):
         self.auth = (password, username) if password and username else None
         self.timeout = configuration.getint(section, "timeout", fallback=30)
 
+    @cached_property
+    def session(self) -> requests.Session:
+        """
+        get or create session
+
+        Returns:
+            request.Session: created session object
+        """
+        return requests.Session()
+
     @staticmethod
     def calculate_hash(path: Path) -> str:
         """
@@ -110,7 +121,7 @@ class HttpUpload(Upload):
             requests.Response: request response object
         """
         try:
-            response = requests.request(method, url, auth=self.auth, timeout=self.timeout, **kwargs)
+            response = self.session.request(method, url, auth=self.auth, timeout=self.timeout, **kwargs)
             response.raise_for_status()
         except requests.HTTPError as e:
             self.logger.exception("could not perform %s request to %s: %s", method, url, exception_response_text(e))

src/ahriman/core/upload/remote_service.py

@@ -17,16 +17,19 @@
 # You should have received a copy of the GNU General Public License
 # along with this program. If not, see <http://www.gnu.org/licenses/>.
 #
+import requests
+
+from functools import cached_property
 from pathlib import Path
 from typing import IO
 
 from ahriman.core.configuration import Configuration
 from ahriman.core.status.web_client import WebClient
-from ahriman.core.upload.upload import Upload
+from ahriman.core.upload.http_upload import HttpUpload
 from ahriman.models.package import Package
 
 
-class RemoteService(Upload):
+class RemoteService(HttpUpload):
     """
     upload files to another server instance
 
@@ -43,11 +46,19 @@ class RemoteService(Upload):
             configuration(Configuration): configuration instance
             section(str): settings section name
         """
-        Upload.__init__(self, architecture, configuration)
+        HttpUpload.__init__(self, architecture, configuration, section)
-        del section
-
         self.client = WebClient(configuration)
 
+    @cached_property
+    def session(self) -> requests.Session:
+        """
+        get or create session
+
+        Returns:
+            request.Session: created session object
+        """
+        return self.client.session
+
     def package_upload(self, path: Path, package: Package) -> None:
         """
         upload single package to remote
@@ -66,7 +77,7 @@ class RemoteService(Upload):
                 part: tuple[str, IO[bytes], str, dict[str, str]] = (
                     descriptor.filename, archive, "application/octet-stream", {}
                 )
-                self.client.make_request("POST", "/api/v1/service/upload", files={"archive": part})
+                self._request("POST", f"{self.client.address}/api/v1/service/upload", files={"archive": part})
 
     def sync(self, path: Path, built_packages: list[Package]) -> None:
         """

src/ahriman/web/views/service/process.py

@@ -44,7 +44,7 @@ class ProcessView(BaseView):
             200: {"description": "Success response", "schema": ProcessSchema},
             401: {"description": "Authorization required", "schema": ErrorSchema},
             403: {"description": "Access is forbidden", "schema": ErrorSchema},
-            404: {"description": "Process ID is unknown", "schema": ErrorSchema},
+            404: {"description": "Not found", "schema": ErrorSchema},
             500: {"description": "Internal server error", "schema": ErrorSchema},
         },
         security=[{"token": [GET_PERMISSION]}],

src/ahriman/web/views/service/upload.py

@@ -18,9 +18,11 @@
 # along with this program. If not, see <http://www.gnu.org/licenses/>.
 #
 import aiohttp_apispec  # type: ignore[import]
+import shutil
+import tempfile
 
 from aiohttp import BodyPartReader
-from aiohttp.web import HTTPBadRequest, HTTPCreated
+from aiohttp.web import HTTPBadRequest, HTTPCreated, HTTPNotFound
 from pathlib import Path
 
 from ahriman.models.user_access import UserAccess
@@ -47,6 +49,7 @@ class UploadView(BaseView):
             400: {"description": "Bad data is supplied", "schema": ErrorSchema},
             401: {"description": "Authorization required", "schema": ErrorSchema},
             403: {"description": "Access is forbidden", "schema": ErrorSchema},
+            404: {"description": "Not found", "schema": ErrorSchema},
             500: {"description": "Internal server error", "schema": ErrorSchema},
         },
         security=[{"token": [POST_PERMISSION]}],
@@ -61,6 +64,9 @@ class UploadView(BaseView):
             HTTPBadRequest: if bad data is supplied
             HTTPCreated: on success response
         """
+        if not self.configuration.getboolean("web", "enable_archive_upload", fallback=False):
+            raise HTTPNotFound()
+
         try:
             reader = await self.request.multipart()
         except Exception as e:
@@ -81,12 +87,27 @@ class UploadView(BaseView):
         if Path(archive_name).resolve().name != archive_name:
             raise HTTPBadRequest(reason="Filename must be valid archive name")
 
-        output = self.configuration.repository_paths.packages / archive_name
+        max_body_size = self.configuration.getint("web", "max_body_size", fallback=None)
-        with output.open("wb") as archive:
+        current_size = 0
+
+        # in order to handle errors automatically we create temporary file for long operation (transfer) and then copy
+        # it to valid location
+        with tempfile.NamedTemporaryFile() as cache:
             while True:
                 chunk = await part.read_chunk()
                 if not chunk:
                     break
-                archive.write(chunk)
+
+                current_size += len(chunk)
+                if max_body_size is not None and current_size > max_body_size:
+                    raise HTTPBadRequest(reason="Body part is too large")
+
+                cache.write(chunk)
+
+            # and now copy temporary file to correct location
+            cache.seek(0)  # reset file position
+            output = self.configuration.repository_paths.packages / archive_name
+            with output.open("wb") as archive:
+                shutil.copyfileobj(cache, archive)
 
         raise HTTPCreated()

tests/ahriman/core/upload/test_http_upload.py

@@ -47,7 +47,7 @@ def test_request(github: Github, mocker: MockerFixture) -> None:
     must call request method
     """
     response_mock = MagicMock()
-    request_mock = mocker.patch("requests.request", return_value=response_mock)
+    request_mock = mocker.patch("requests.Session.request", return_value=response_mock)
 
     github._request("GET", "url", arg="arg")
     request_mock.assert_called_once_with("GET", "url", auth=github.auth, timeout=github.timeout, arg="arg")
@@ -58,6 +58,6 @@ def test_request_exception(github: Github, mocker: MockerFixture) -> None:
     """
     must call request method and log HTTPError exception
     """
-    mocker.patch("requests.request", side_effect=requests.HTTPError())
+    mocker.patch("requests.Session.request", side_effect=requests.HTTPError())
     with pytest.raises(requests.HTTPError):
         github._request("GET", "url", arg="arg")

tests/ahriman/core/upload/test_remote_service.py

@@ -7,17 +7,26 @@ from ahriman.core.upload.remote_service import RemoteService
 from ahriman.models.package import Package
 
 
+def test_session(remote_service: RemoteService, mocker: MockerFixture) -> None:
+    """
+    must generate ahriman session
+    """
+    upload_mock = mocker.patch("ahriman.core.status.web_client.WebClient._create_session")
+    assert remote_service.session
+    upload_mock.assert_called_once_with(use_unix_socket=False)
+
+
 def test_package_upload(remote_service: RemoteService, package_ahriman: Package, mocker: MockerFixture) -> None:
     """
     must upload package to remote host
     """
     open_mock = mocker.patch("pathlib.Path.open")
-    upload_mock = mocker.patch("ahriman.core.status.web_client.WebClient.make_request")
+    upload_mock = mocker.patch("ahriman.core.upload.http_upload.HttpUpload._request")
     filename = package_ahriman.packages[package_ahriman.base].filename
 
     remote_service.sync(Path("local"), [package_ahriman])
     open_mock.assert_called_once_with("rb")
-    upload_mock.assert_called_once_with("POST", "/api/v1/service/upload", files={
+    upload_mock.assert_called_once_with("POST", f"{remote_service.client.address}/api/v1/service/upload", files={
         "archive": (filename, pytest.helpers.anyvar(int), "application/octet-stream", {})
     })
 

tests/ahriman/web/views/service/test_views_service_upload.py

@@ -20,9 +20,10 @@ async def test_get_permission() -> None:
 
 async def test_post(client: TestClient, mocker: MockerFixture) -> None:
     """
-    must call post request correctly for alias
+    must process file upload via http
     """
     open_mock = mocker.patch("pathlib.Path.open")
+    copy_mock = mocker.patch("shutil.copyfileobj")
     # no content validation here because it has invalid schema
 
     data = FormData()
@@ -31,61 +32,84 @@ async def test_post(client: TestClient, mocker: MockerFixture) -> None:
     response = await client.post("/api/v1/service/upload", data=data)
     assert response.ok
     open_mock.assert_called_once_with("wb")
+    copy_mock.assert_called_once_with(pytest.helpers.anyvar(int), pytest.helpers.anyvar(int))
+
+
+async def test_post_not_found(client: TestClient, mocker: MockerFixture) -> None:
+    """
+    must return 404 if request was disabled
+    """
+    mocker.patch("ahriman.core.configuration.Configuration.getboolean", return_value=False)
+    data = FormData()
+    data.add_field("archive", BytesIO(b"content"), filename="filename", content_type="application/octet-stream")
+    response_schema = pytest.helpers.schema_response(UploadView.post, code=404)
+
+    response = await client.post("/api/v1/service/upload", data=data)
+    assert response.status == 404
+    assert not response_schema.validate(await response.json())
 
 
 async def test_post_not_multipart(client: TestClient) -> None:
     """
     must return 400 on invalid payload
     """
+    response_schema = pytest.helpers.schema_response(UploadView.post, code=400)
+
     response = await client.post("/api/v1/service/upload")
     assert response.status == 400
+    assert not response_schema.validate(await response.json())
 
 
-async def test_post_not_bodypart(client: TestClient, mocker: MockerFixture) -> None:
+async def test_post_not_body_part(client: TestClient, mocker: MockerFixture) -> None:
     """
     must return 400 on invalid iterator in multipart
     """
+    response_schema = pytest.helpers.schema_response(UploadView.post, code=400)
     mocker.patch("aiohttp.MultipartReader.next", return_value=42)  # surprise, motherfucker
     data = FormData()
     data.add_field("archive", BytesIO(b"content"), filename="filename", content_type="application/octet-stream")
 
     response = await client.post("/api/v1/service/upload", data=data)
     assert response.status == 400
+    assert not response_schema.validate(await response.json())
 
 
 async def test_post_not_archive(client: TestClient) -> None:
     """
     must return 400 on invalid multipart key
     """
+    response_schema = pytest.helpers.schema_response(UploadView.post, code=400)
     data = FormData()
     data.add_field("random", BytesIO(b"content"), filename="filename", content_type="application/octet-stream")
 
     response = await client.post("/api/v1/service/upload", data=data)
     assert response.status == 400
-
+    assert not response_schema.validate(await response.json())
-
-async def test_post_no_filename(client: TestClient, mocker: MockerFixture) -> None:
-    """
-    must return 400 if filename is not set
-    """
-    mocker.patch("aiohttp.BodyPartReader.filename", return_value=None)
-    data = FormData()
-    data.add_field("random", BytesIO(b"content"), filename="filename", content_type="application/octet-stream")
-
-    response = await client.post("/api/v1/service/upload", data=data)
-    assert response.status == 400
 
 
 async def test_post_filename_invalid(client: TestClient) -> None:
     """
     must return 400 if filename is invalid
     """
+    response_schema = pytest.helpers.schema_response(UploadView.post, code=400)
+
     data = FormData()
     data.add_field("archive", BytesIO(b"content"), filename="..", content_type="application/octet-stream")
     response = await client.post("/api/v1/service/upload", data=data)
     assert response.status == 400
+    assert not response_schema.validate(await response.json())
 
+
+async def test_post_file_too_big(client: TestClient, mocker: MockerFixture) -> None:
+    """
+    must return 400 if file is too big
+    """
+    mocker.patch("pathlib.Path.open")
+    mocker.patch("ahriman.core.configuration.Configuration.getint", return_value=0)
     data = FormData()
-    data.add_field("archive", BytesIO(b"content"), filename="", content_type="application/octet-stream")
+    data.add_field("archive", BytesIO(b"content"), filename="filename", content_type="application/octet-stream")
+    response_schema = pytest.helpers.schema_response(UploadView.post, code=400)
+
     response = await client.post("/api/v1/service/upload", data=data)
     assert response.status == 400
+    assert not response_schema.validate(await response.json())

tests/testresources/core/ahriman.ini

@@ -112,6 +112,7 @@ username = arcan1s
 debug = no
 debug_check_host = no
 debug_allowed_hosts =
+enable_archive_upload = yes
 host = 127.0.0.1
 static_path = ../web/templates/static
 templates = ../web/templates