diff --git a/docs/configuration.rst b/docs/configuration.rst index 6de7e245..91e11d0a 100644 --- a/docs/configuration.rst +++ b/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. diff --git a/docs/faq.rst b/docs/faq.rst index 440d7c0b..c665a510 100644 --- a/docs/faq.rst +++ b/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 diff --git a/src/ahriman/core/configuration/schema.py b/src/ahriman/core/configuration/schema.py index d3cb9f21..1dfc833d 100644 --- a/src/ahriman/core/configuration/schema.py +++ b/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", }, diff --git a/src/ahriman/core/status/web_client.py b/src/ahriman/core/status/web_client.py index 882fa860..34371793 100644 --- a/src/ahriman/core/status/web_client.py +++ b/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 diff --git a/src/ahriman/core/upload/http_upload.py b/src/ahriman/core/upload/http_upload.py index df423292..69eb6334 100644 --- a/src/ahriman/core/upload/http_upload.py +++ b/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)) diff --git a/src/ahriman/core/upload/remote_service.py b/src/ahriman/core/upload/remote_service.py index 4fb66c2d..e6eeea30 100644 --- a/src/ahriman/core/upload/remote_service.py +++ b/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 . # +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) - del section - + HttpUpload.__init__(self, architecture, configuration, 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: """ diff --git a/src/ahriman/web/views/service/process.py b/src/ahriman/web/views/service/process.py index 7a1c70d0..c27057f8 100644 --- a/src/ahriman/web/views/service/process.py +++ b/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]}], diff --git a/src/ahriman/web/views/service/upload.py b/src/ahriman/web/views/service/upload.py index 232862a6..5bc13fa9 100644 --- a/src/ahriman/web/views/service/upload.py +++ b/src/ahriman/web/views/service/upload.py @@ -18,9 +18,11 @@ # along with this program. If not, see . # 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 - with output.open("wb") as archive: + max_body_size = self.configuration.getint("web", "max_body_size", fallback=None) + 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() diff --git a/tests/ahriman/core/upload/test_http_upload.py b/tests/ahriman/core/upload/test_http_upload.py index 544c5562..07dbb23f 100644 --- a/tests/ahriman/core/upload/test_http_upload.py +++ b/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") diff --git a/tests/ahriman/core/upload/test_remote_service.py b/tests/ahriman/core/upload/test_remote_service.py index 0b4879bd..179e239b 100644 --- a/tests/ahriman/core/upload/test_remote_service.py +++ b/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", {}) }) diff --git a/tests/ahriman/web/views/service/test_views_service_upload.py b/tests/ahriman/web/views/service/test_views_service_upload.py index ed2fe8c9..0cdc7e5d 100644 --- a/tests/ahriman/web/views/service/test_views_service_upload.py +++ b/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 - - -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 + assert not response_schema.validate(await response.json()) 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()) diff --git a/tests/testresources/core/ahriman.ini b/tests/testresources/core/ahriman.ini index 74d87690..d12b4c3b 100644 --- a/tests/testresources/core/ahriman.ini +++ b/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 \ No newline at end of file