From 5ba12b02640e808ef8afb7af0e8e9a62902aef89 Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Fri, 9 May 2025 17:00:42 +0200 Subject: [PATCH 1/5] Initial version for diagnostic logs: - add new v2 endpoint to stats module - save file to DIAGNOSTIC_LOGS_URL - propagate to config --- deployment/community/.env.template | 4 ++ server/.test.env | 1 + server/application.py | 1 + server/mergin/config.py | 4 ++ server/mergin/stats/api.yaml | 40 +++++++++++++++++-- server/mergin/stats/config.py | 10 +++++ server/mergin/stats/controller.py | 31 ++++++++++++++- server/mergin/stats/utils.py | 21 ++++++++++ server/mergin/tests/fixtures.py | 4 ++ server/mergin/tests/test_statistics.py | 41 +++++++++++++++++++- server/mergin/tests/test_statistics_utils.py | 37 ++++++++++++++++++ 11 files changed, 187 insertions(+), 7 deletions(-) create mode 100644 server/mergin/stats/utils.py create mode 100644 server/mergin/tests/test_statistics_utils.py diff --git a/deployment/community/.env.template b/deployment/community/.env.template index 741e0e9c..ce910efd 100644 --- a/deployment/community/.env.template +++ b/deployment/community/.env.template @@ -208,3 +208,7 @@ GEVENT_WORKER=True # Deprecated from 2024.7.0, replacement is to set GEVENT_WORKER=True NO_MONKEY_PATCH=False +# Diagnostic logs + +DIAGNOSTIC_LOGS_DIR=/diagnostic_logs + diff --git a/server/.test.env b/server/.test.env index 908db5b9..bdaa7bfa 100644 --- a/server/.test.env +++ b/server/.test.env @@ -23,3 +23,4 @@ GEODIFF_WORKING_DIR=/tmp/geodiff SECURITY_BEARER_SALT='bearer' SECURITY_EMAIL_SALT='email' SECURITY_PASSWORD_SALT='password' +DIAGNOSTIC_LOGS_DIR=/tmp/diagnostic_logs diff --git a/server/application.py b/server/application.py index 5cb08b69..b1ab79ac 100644 --- a/server/application.py +++ b/server/application.py @@ -46,6 +46,7 @@ "GLOBAL_READ", "GLOBAL_WRITE", "ENABLE_SUPERADMIN_ASSIGNMENT", + "DIAGNOSTIC_LOGS_URL", ] ) register_stats(application) diff --git a/server/mergin/config.py b/server/mergin/config.py index f633e171..f54c39cc 100644 --- a/server/mergin/config.py +++ b/server/mergin/config.py @@ -107,3 +107,7 @@ class Configuration(object): # using gevent type of worker impose some requirements on code, e.g. to be greenlet safe, custom timeouts GEVENT_WORKER = config("GEVENT_WORKER", default=False, cast=bool) GEVENT_REQUEST_TIMEOUT = config("GEVENT_REQUEST_TIMEOUT", default=30, cast=int) + DIAGNOSTIC_LOGS_URL = config( + "DIAGNOSTIC_LOGS_URL", + default="", + ) diff --git a/server/mergin/stats/api.yaml b/server/mergin/stats/api.yaml index 382da567..e1a5d492 100644 --- a/server/mergin/stats/api.yaml +++ b/server/mergin/stats/api.yaml @@ -18,7 +18,7 @@ paths: schema: $ref: "#/components/schemas/ServerVersion" "400": - $ref: "#/components/responses/BadStatusResp" + $ref: "#/components/responses/BadRequestResp" "404": $ref: "#/components/responses/NotFoundResp" x-openapi-router-controller: mergin.stats.controller @@ -50,17 +50,51 @@ paths: schema: type: string "400": - $ref: "#/components/responses/BadStatusResp" + $ref: "#/components/responses/BadRequestResp" "404": $ref: "#/components/responses/NotFoundResp" + /v2/diagnostic-logs: + post: + summary: Save diagnostic logs + operationId: save_diagnostic_log + x-openapi-router-controller: mergin.stats.controller + parameters: + - name: app + in: query + description: Application name (e.g., "input-android-0.9.0") + required: true + schema: + type: string + - name: username + in: query + description: Username + required: true + schema: + type: string + requestBody: + required: true + content: + text/plain: + schema: + type: string + description: Log content in plain text + responses: + "200": + description: Log saved successfully + "400": + $ref: "#/components/responses/BadRequestResp" + "413": + $ref: "#/components/responses/RequestTooLarge" components: responses: UnauthorizedError: description: Authentication information is missing or invalid. NotFoundResp: description: Project not found. - BadStatusResp: + BadRequestResp: description: Invalid request. + RequestTooLarge: + description: Request Entity Too Large. schemas: ServerVersion: type: object diff --git a/server/mergin/stats/config.py b/server/mergin/stats/config.py index 795df286..e251e851 100644 --- a/server/mergin/stats/config.py +++ b/server/mergin/stats/config.py @@ -2,9 +2,13 @@ # # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial +import os from decouple import config +config_dir = os.path.abspath(os.path.dirname(__file__)) + + class Configuration(object): # send statistics about usage COLLECT_STATISTICS = config("COLLECT_STATISTICS", default=True, cast=bool) @@ -16,3 +20,9 @@ class Configuration(object): STATISTICS_URL = config( "STATISTICS_URL", default="https://api.merginmaps.com/monitoring/v1" ).rstrip("/") + DIAGNOSTIC_LOGS_DIR = config( + "DIAGNOSTIC_LOGS_DIR", + default=os.path.join( + config_dir, os.pardir, os.pardir, os.pardir, "diagnostic_logs" + ), + ) diff --git a/server/mergin/stats/controller.py b/server/mergin/stats/controller.py index 4606f1e5..87fd6f34 100644 --- a/server/mergin/stats/controller.py +++ b/server/mergin/stats/controller.py @@ -2,14 +2,15 @@ # # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial -from dataclasses import asdict import requests -from flask import abort, current_app, make_response +from flask import abort, current_app, make_response, request from datetime import datetime, time from csv import DictWriter +from pathvalidate import sanitize_filename from mergin.auth.app import auth_required from mergin.stats.models import MerginStatistics, ServerCallhomeData +from mergin.stats.utils import save_diagnostic_log_file from .config import Configuration from ..app import parse_version_string, db @@ -88,3 +89,29 @@ def download_report(date_from: str, date_to: str): response.headers["Content-Disposition"] = f"attachment; filename=usage-report.csv" response.mimetype = "text/csv" return response + + +def save_diagnostic_log(app: str, username: str): + """Save diagnostic logs""" + # if server is using external storage, we don't want to save logs + if app.config.get("DIAGNOSTIC_LOGS_URL"): + abort(404) + + # check if plain text body is not larger than 1MB + max_size = 1024 * 1024 + if request.content_length > max_size: + abort(413) + # get body from request + body = request.get_data() + if not body: + abort(400) + if len(body) > max_size: + abort(413) + + # save diagnostic log file + folder = current_app.config.get("DIAGNOSTIC_LOGS_DIR") + save_diagnostic_log_file( + app, username, body, current_app.config["DIAGNOSTIC_LOGS_DIR"] + ) + + return "Log saved successfully", 200 diff --git a/server/mergin/stats/utils.py b/server/mergin/stats/utils.py new file mode 100644 index 00000000..ae0e1155 --- /dev/null +++ b/server/mergin/stats/utils.py @@ -0,0 +1,21 @@ +from datetime import datetime, timezone +import os + +from pathvalidate import sanitize_filename + + +def save_diagnostic_log_file( + app: str, username: str, body: bytes, to_folder: str +) -> str: + """Save diagnostic log file to DIAGNOSTIC_LOGS_DIR""" + + content = body.decode("utf-8") + datetime_iso_str = datetime.now(tz=timezone.utc).isoformat() + file_name = sanitize_filename( + username + "_" + app + "_" + datetime_iso_str + ".log" + ) + os.makedirs(to_folder, exist_ok=True) + with open(os.path.join(to_folder, file_name), "w") as f: + f.write(content) + + return file_name diff --git a/server/mergin/tests/fixtures.py b/server/mergin/tests/fixtures.py index 6b83ecf9..7cff688e 100644 --- a/server/mergin/tests/fixtures.py +++ b/server/mergin/tests/fixtures.py @@ -3,6 +3,7 @@ # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial import os +import shutil import sys import uuid from copy import deepcopy @@ -80,6 +81,9 @@ def teardown(): if p.storage is not None ] cleanup(flask_app.test_client(), dirs) + diagnostic_logs_dir = flask_app.config.get("DIAGNOSTIC_LOGS_DIR") + if os.path.exists(diagnostic_logs_dir): + shutil.rmtree(diagnostic_logs_dir) request.addfinalizer(teardown) return flask_app diff --git a/server/mergin/tests/test_statistics.py b/server/mergin/tests/test_statistics.py index 6f339505..dc73ef96 100644 --- a/server/mergin/tests/test_statistics.py +++ b/server/mergin/tests/test_statistics.py @@ -2,11 +2,12 @@ # # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial -import csv from dataclasses import asdict -from datetime import timedelta, timezone, datetime +from datetime import timezone, datetime import json +import os from unittest.mock import patch +from flask import url_for import requests from sqlalchemy.sql.operators import is_ @@ -220,3 +221,39 @@ def test_download_report(app, client): empty_file = f"{','.join(keys)}\r\n" assert resp.data.decode("UTF-8") == empty_file assert len(lines) == 1 + + +def test_save_diagnostic_log(client, app): + """Test save diagnostic log endpoint""" + url = url_for("/.mergin_stats_controller_save_diagnostic_log") + resp = client.post(url) + assert resp.status_code == 400 + + # bad request + resp = client.post(url, data="test") + assert resp.status_code == 400 + + url = url_for( + "/.mergin_stats_controller_save_diagnostic_log", + app="test_app", + username="test_user", + ) + + # too large request + resp = client.post(url, data="x" * (1024 * 1024 + 1)) + assert resp.status_code == 413 + + # valid request + resp = client.post(url, data="test") + assert resp.status_code == 200 + + # check if file was created + log_dir = app.config["DIAGNOSTIC_LOGS_DIR"] + assert os.path.exists(log_dir) + files = os.listdir(log_dir) + assert len(files) == 1 + assert files[0].startswith("test_user_test_app_") + assert files[0].endswith(".log") + with open(os.path.join(log_dir, files[0]), "r") as f: + content = f.read() + assert content == "test" diff --git a/server/mergin/tests/test_statistics_utils.py b/server/mergin/tests/test_statistics_utils.py new file mode 100644 index 00000000..fd83ebc2 --- /dev/null +++ b/server/mergin/tests/test_statistics_utils.py @@ -0,0 +1,37 @@ +from datetime import datetime, timezone +import os +from unittest.mock import patch + +from pathvalidate import sanitize_filename + +from ..stats.utils import save_diagnostic_log_file + + +def test_save_diagnostic_log_file(client, app): + """Test save diagnostic log file""" + # Mock datetime value + test_date = "2025-05-09T12:00:00+00:00" + app_name = "t" * 256 + username = "test-user" + body = b"Test log content" + to_folder = app.config["DIAGNOSTIC_LOGS_DIR"] + + saved_file_name = save_diagnostic_log_file(app_name, username, body, to_folder) + saved_file_path = os.path.join(to_folder, saved_file_name) + assert os.path.exists(saved_file_path) + assert len(saved_file_name) == 255 + + with patch("mergin.stats.utils.datetime") as mock_datetime: + mock_datetime.now.return_value = datetime.fromisoformat(test_date) + app_name = "test_<>app" + saved_file_name = save_diagnostic_log_file(app_name, username, body, to_folder) + # Check if the file was created + assert saved_file_name == sanitize_filename( + username + "_" + app_name + "_" + test_date + ".log" + ) + saved_file_path = os.path.join(to_folder, saved_file_name) + assert os.path.exists(saved_file_path) + # Check the content of the file + with open(saved_file_path, "r") as f: + content = f.read() + assert content == body.decode("utf-8") From 0079a0be1d9c64cc7d851fef5035f965f16e64f4 Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Fri, 9 May 2025 17:22:10 +0200 Subject: [PATCH 2/5] fix config getting --- server/mergin/stats/controller.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/mergin/stats/controller.py b/server/mergin/stats/controller.py index 87fd6f34..a0c38d67 100644 --- a/server/mergin/stats/controller.py +++ b/server/mergin/stats/controller.py @@ -94,7 +94,7 @@ def download_report(date_from: str, date_to: str): def save_diagnostic_log(app: str, username: str): """Save diagnostic logs""" # if server is using external storage, we don't want to save logs - if app.config.get("DIAGNOSTIC_LOGS_URL"): + if current_app.config.get("DIAGNOSTIC_LOGS_URL"): abort(404) # check if plain text body is not larger than 1MB @@ -111,7 +111,7 @@ def save_diagnostic_log(app: str, username: str): # save diagnostic log file folder = current_app.config.get("DIAGNOSTIC_LOGS_DIR") save_diagnostic_log_file( - app, username, body, current_app.config["DIAGNOSTIC_LOGS_DIR"] + app, username, body, current_app.config.get("DIAGNOSTIC_LOGS_DIR") ) return "Log saved successfully", 200 From 58ffbde8205c2384e7be550fcf7d2b5e57ff16f0 Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Mon, 12 May 2025 09:31:47 +0200 Subject: [PATCH 3/5] small variable change --- server/mergin/stats/controller.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/server/mergin/stats/controller.py b/server/mergin/stats/controller.py index a0c38d67..57f9eb49 100644 --- a/server/mergin/stats/controller.py +++ b/server/mergin/stats/controller.py @@ -110,8 +110,6 @@ def save_diagnostic_log(app: str, username: str): # save diagnostic log file folder = current_app.config.get("DIAGNOSTIC_LOGS_DIR") - save_diagnostic_log_file( - app, username, body, current_app.config.get("DIAGNOSTIC_LOGS_DIR") - ) + save_diagnostic_log_file(app, username, body, folder) return "Log saved successfully", 200 From 4c5b6e28ea1d011ecc0a82814254dd6113cd7245 Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Tue, 13 May 2025 16:22:36 +0200 Subject: [PATCH 4/5] Refactor: - move some endpoints to global app.py from stats - move some tests to `test_controller` Updated: - make max size configurable --- server/mergin/api.yaml | 84 ++++++++++++ server/mergin/app.py | 6 + server/mergin/config.py | 11 ++ server/mergin/controller.py | 61 +++++++++ server/mergin/stats/api.yaml | 71 ---------- server/mergin/stats/config.py | 9 -- server/mergin/stats/controller.py | 52 +------- server/mergin/stats/utils.py | 21 --- server/mergin/tests/test_controller.py | 131 +++++++++++++++++++ server/mergin/tests/test_statistics.py | 77 +---------- server/mergin/tests/test_statistics_utils.py | 37 ------ server/mergin/tests/test_utils.py | 68 +++++----- server/mergin/utils.py | 21 ++- 13 files changed, 352 insertions(+), 297 deletions(-) create mode 100644 server/mergin/api.yaml create mode 100644 server/mergin/controller.py delete mode 100644 server/mergin/stats/utils.py create mode 100644 server/mergin/tests/test_controller.py delete mode 100644 server/mergin/tests/test_statistics_utils.py diff --git a/server/mergin/api.yaml b/server/mergin/api.yaml new file mode 100644 index 00000000..6ebd2777 --- /dev/null +++ b/server/mergin/api.yaml @@ -0,0 +1,84 @@ +openapi: 3.0.0 +info: + description: Common Mergin Maps API + version: "0.1" + title: Common Mergin Maps API +servers: + - url: / +paths: + /v1/latest-version: + get: + summary: Fetch latest available server version + operationId: get_latest_version + responses: + "200": + description: Latest version info + content: + application/json: + schema: + $ref: "#/components/schemas/ServerVersion" + "400": + $ref: "#/components/responses/BadRequestResp" + "404": + $ref: "#/components/responses/NotFoundResp" + x-openapi-router-controller: mergin.controller + /v2/diagnostic-logs: + post: + summary: Save diagnostic log to the server + description: This endpoint allows users to upload diagnostic logs for troubleshooting purposes from mobile and plugin. + operationId: save_diagnostic_log + x-openapi-router-controller: mergin.controller + parameters: + - name: app + in: query + description: Application name (e.g., "input-android-0.9.0") + required: true + schema: + type: string + requestBody: + required: true + content: + text/plain: + schema: + type: string + description: Log content in plain text + responses: + "200": + description: Log saved successfully + "400": + $ref: "#/components/responses/BadRequestResp" + "404": + $ref: "#/components/responses/NotFoundResp" + "413": + $ref: "#/components/responses/RequestTooLarge" +components: + responses: + UnauthorizedError: + description: Authentication information is missing or invalid. + NotFoundResp: + description: Not found + BadRequestResp: + description: Invalid request. + RequestTooLarge: + description: Request Entity Too Large. + schemas: + ServerVersion: + type: object + properties: + version: + type: string + example: 2023.1.1 + major: + type: integer + example: 2023 + minor: + type: integer + example: 1 + fix: + nullable: true + type: integer + example: 1 + info_url: + nullable: true + type: string + example: "https://github.com/MerginMaps/mergin/releases/tag/2023.1" diff --git a/server/mergin/app.py b/server/mergin/app.py index 654d5c04..8910c6d1 100644 --- a/server/mergin/app.py +++ b/server/mergin/app.py @@ -177,6 +177,12 @@ def create_app(public_keys: List[str] = None) -> Flask: options={"swagger_ui": False, "serve_spec": False}, validate_responses=True, ) + app.add_api( + "api.yaml", + arguments={"title": "Mergin"}, + options={"swagger_ui": False, "serve_spec": False}, + validate_responses=True, + ) app.app.config.from_object(SyncConfig) app.app.connexion_app = app diff --git a/server/mergin/config.py b/server/mergin/config.py index f54c39cc..8855b5a8 100644 --- a/server/mergin/config.py +++ b/server/mergin/config.py @@ -7,6 +7,8 @@ from .version import get_version +config_dir = os.path.abspath(os.path.dirname(__file__)) + class Configuration(object): # flask/connexion variables @@ -111,3 +113,12 @@ class Configuration(object): "DIAGNOSTIC_LOGS_URL", default="", ) + DIAGNOSTIC_LOGS_DIR = config( + "DIAGNOSTIC_LOGS_DIR", + default=os.path.join( + config_dir, os.pardir, os.pardir, os.pardir, "diagnostic_logs" + ), + ) + DIAGNOSTIC_LOGS_MAX_SIZE = config( + "DIAGNOSTIC_LOGS_MAX_SIZE", default=1024 * 1024, cast=int + ) diff --git a/server/mergin/controller.py b/server/mergin/controller.py new file mode 100644 index 00000000..93824e68 --- /dev/null +++ b/server/mergin/controller.py @@ -0,0 +1,61 @@ +import json +import logging +import os +from flask import abort, current_app, request +from flask_login import current_user +from magic import from_buffer +import time + +import requests + +from .utils import save_diagnostic_log_file +from .app import parse_version_string, db + + +def get_latest_version(): + """Parse information about available server updates from 3rd party service""" + try: + req = requests.get(current_app.config["STATISTICS_URL"] + "/latest-versions") + except requests.exceptions.RequestException: + abort(400, "Updates information not available") + + if not req.ok: + abort(400, "Updates information not available") + + data = req.json().get(current_app.config["SERVER_TYPE"].lower(), None) + if not data: + abort(400, "Updates information not available") + + parsed_version = parse_version_string(data.get("version", "")) + if not parsed_version: + abort(400, "Updates information not available") + + data = {**data, **parsed_version} + return data, 200 + + +def save_diagnostic_log(): + """Save diagnostic logs""" + # if server is using external storage, we don't want to save logs + if current_app.config.get("DIAGNOSTIC_LOGS_URL"): + abort(404) + + # check if plain text body is not larger than 1MB + max_size = current_app.config.get("DIAGNOSTIC_LOGS_MAX_SIZE") + if request.content_length > max_size: + abort(413) + # get body from request + body = request.get_data() + if not body: + abort(400) + if len(body) > max_size: + abort(413) + mime_type = from_buffer(body, mime=True) + if mime_type != "text/plain": + abort(400) + + app = request.args.get("app") + username = current_user.username if current_user.is_authenticated else "anonymous" + save_diagnostic_log_file(app, username, body) + + return "Log saved successfully", 200 diff --git a/server/mergin/stats/api.yaml b/server/mergin/stats/api.yaml index e1a5d492..757351bd 100644 --- a/server/mergin/stats/api.yaml +++ b/server/mergin/stats/api.yaml @@ -6,22 +6,6 @@ info: servers: - url: / paths: - /v1/latest-version: - get: - summary: Fetch latest available server version - operationId: get_latest_version - responses: - "200": - description: Latest version info - content: - application/json: - schema: - $ref: "#/components/schemas/ServerVersion" - "400": - $ref: "#/components/responses/BadRequestResp" - "404": - $ref: "#/components/responses/NotFoundResp" - x-openapi-router-controller: mergin.stats.controller /app/admin/report: get: summary: Download statistics for server @@ -53,38 +37,6 @@ paths: $ref: "#/components/responses/BadRequestResp" "404": $ref: "#/components/responses/NotFoundResp" - /v2/diagnostic-logs: - post: - summary: Save diagnostic logs - operationId: save_diagnostic_log - x-openapi-router-controller: mergin.stats.controller - parameters: - - name: app - in: query - description: Application name (e.g., "input-android-0.9.0") - required: true - schema: - type: string - - name: username - in: query - description: Username - required: true - schema: - type: string - requestBody: - required: true - content: - text/plain: - schema: - type: string - description: Log content in plain text - responses: - "200": - description: Log saved successfully - "400": - $ref: "#/components/responses/BadRequestResp" - "413": - $ref: "#/components/responses/RequestTooLarge" components: responses: UnauthorizedError: @@ -93,26 +45,3 @@ components: description: Project not found. BadRequestResp: description: Invalid request. - RequestTooLarge: - description: Request Entity Too Large. - schemas: - ServerVersion: - type: object - properties: - version: - type: string - example: 2023.1.1 - major: - type: integer - example: 2023 - minor: - type: integer - example: 1 - fix: - nullable: true - type: integer - example: 1 - info_url: - nullable: true - type: string - example: "https://github.com/MerginMaps/mergin/releases/tag/2023.1" diff --git a/server/mergin/stats/config.py b/server/mergin/stats/config.py index e251e851..cfd5b4d9 100644 --- a/server/mergin/stats/config.py +++ b/server/mergin/stats/config.py @@ -6,9 +6,6 @@ from decouple import config -config_dir = os.path.abspath(os.path.dirname(__file__)) - - class Configuration(object): # send statistics about usage COLLECT_STATISTICS = config("COLLECT_STATISTICS", default=True, cast=bool) @@ -20,9 +17,3 @@ class Configuration(object): STATISTICS_URL = config( "STATISTICS_URL", default="https://api.merginmaps.com/monitoring/v1" ).rstrip("/") - DIAGNOSTIC_LOGS_DIR = config( - "DIAGNOSTIC_LOGS_DIR", - default=os.path.join( - config_dir, os.pardir, os.pardir, os.pardir, "diagnostic_logs" - ), - ) diff --git a/server/mergin/stats/controller.py b/server/mergin/stats/controller.py index 57f9eb49..d58a4417 100644 --- a/server/mergin/stats/controller.py +++ b/server/mergin/stats/controller.py @@ -3,17 +3,15 @@ # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial import requests -from flask import abort, current_app, make_response, request +from flask import abort, current_app, make_response from datetime import datetime, time from csv import DictWriter -from pathvalidate import sanitize_filename from mergin.auth.app import auth_required from mergin.stats.models import MerginStatistics, ServerCallhomeData -from mergin.stats.utils import save_diagnostic_log_file from .config import Configuration -from ..app import parse_version_string, db +from ..app import db class CsvTextBuilder(object): @@ -28,28 +26,6 @@ def write(self, row): self.data.append(row) -def get_latest_version(): - """Parse information about available server updates from 3rd party service""" - try: - req = requests.get(Configuration.STATISTICS_URL + "/latest-versions") - except requests.exceptions.RequestException: - abort(400, "Updates information not available") - - if not req.ok: - abort(400, "Updates information not available") - - data = req.json().get(current_app.config["SERVER_TYPE"].lower(), None) - if not data: - abort(400, "Updates information not available") - - parsed_version = parse_version_string(data.get("version", "")) - if not parsed_version: - abort(400, "Updates information not available") - - data = {**data, **parsed_version} - return data, 200 - - @auth_required(permissions=["admin"]) def download_report(date_from: str, date_to: str): """Download statistics from server instance""" @@ -89,27 +65,3 @@ def download_report(date_from: str, date_to: str): response.headers["Content-Disposition"] = f"attachment; filename=usage-report.csv" response.mimetype = "text/csv" return response - - -def save_diagnostic_log(app: str, username: str): - """Save diagnostic logs""" - # if server is using external storage, we don't want to save logs - if current_app.config.get("DIAGNOSTIC_LOGS_URL"): - abort(404) - - # check if plain text body is not larger than 1MB - max_size = 1024 * 1024 - if request.content_length > max_size: - abort(413) - # get body from request - body = request.get_data() - if not body: - abort(400) - if len(body) > max_size: - abort(413) - - # save diagnostic log file - folder = current_app.config.get("DIAGNOSTIC_LOGS_DIR") - save_diagnostic_log_file(app, username, body, folder) - - return "Log saved successfully", 200 diff --git a/server/mergin/stats/utils.py b/server/mergin/stats/utils.py deleted file mode 100644 index ae0e1155..00000000 --- a/server/mergin/stats/utils.py +++ /dev/null @@ -1,21 +0,0 @@ -from datetime import datetime, timezone -import os - -from pathvalidate import sanitize_filename - - -def save_diagnostic_log_file( - app: str, username: str, body: bytes, to_folder: str -) -> str: - """Save diagnostic log file to DIAGNOSTIC_LOGS_DIR""" - - content = body.decode("utf-8") - datetime_iso_str = datetime.now(tz=timezone.utc).isoformat() - file_name = sanitize_filename( - username + "_" + app + "_" + datetime_iso_str + ".log" - ) - os.makedirs(to_folder, exist_ok=True) - with open(os.path.join(to_folder, file_name), "w") as f: - f.write(content) - - return file_name diff --git a/server/mergin/tests/test_controller.py b/server/mergin/tests/test_controller.py new file mode 100644 index 00000000..9a266832 --- /dev/null +++ b/server/mergin/tests/test_controller.py @@ -0,0 +1,131 @@ +import os +from flask import url_for, current_app +import json +from unittest.mock import MagicMock, patch + +import requests + +from ..auth.models import User +from ..app import db +from .utils import Response + + +def test_healthcheck(client): + # anonymous user + client.get(url_for("/.mergin_auth_controller_logout")) + client.application.config["WTF_CSRF_ENABLED"] = True + maint_file = current_app.config["MAINTENANCE_FILE"] + resp = client.post("/alive") + assert resp.status_code == 200 + resp_data = json.loads(resp.data) + print(resp.headers) + print(resp_data) + assert "processing_time_ms" in resp_data + print(type(resp_data)) + assert not resp_data["maintenance"] + + # create maintenance mode + with open(maint_file, "w+"): + resp = client.post("/alive") + assert resp.status_code == 200 + resp_data = json.loads(resp.data) + assert resp_data["maintenance"] + os.remove(maint_file) + + # tests with invalid method + resp = client.get("/alive") + assert resp.status_code == 405 + + # mock some db issue + _connect = db.engine.connect + db.engine.connect = MagicMock(side_effect=Exception("Some db issue")) + resp = client.post("/alive") + assert resp.status_code == 500 + # undo mock + db.engine.connect = _connect + + +def test_server_updates(client): + """Test proxy endpoint to fetch server updates information""" + assert client.application.config["SERVER_TYPE"] == "ce" + url = "/v1/latest-version" + + with patch("requests.get") as mock: + api_data = { + "ee": {"version": "2023.1.2", "info_url": "https://release-info.com"}, + "ce": {"version": "2023.1.2", "info_url": "https://release-info.com"}, + } + mock.return_value = Response(True, api_data) + resp = client.get(url) + assert resp.status_code == 200 + assert resp.json["version"] == api_data["ce"]["version"] + assert resp.json["major"] == 2023 + assert resp.json["minor"] == 1 + assert resp.json["fix"] == 2 + + # remove fix version + api_data["ce"]["version"] = "2023.2" + resp = client.get(url) + assert resp.status_code == 200 + assert resp.json["major"] == 2023 + assert resp.json["minor"] == 2 + assert resp.json["fix"] is None + + # invalid response + del api_data["ce"]["version"] + resp = client.get(url) + assert resp.status_code == 400 + + # 3rd party api failure + mock.side_effect = requests.exceptions.RequestException("Some failure") + resp = client.get(url) + assert resp.status_code == 400 + + +def test_save_diagnostic_log(client, app): + """Test save diagnostic log endpoint""" + user = User.query.filter(User.username == "mergin").first() + url = url_for("mergin_controller_save_diagnostic_log") + resp = client.post(url) + assert resp.status_code == 400 + + # bad request + resp = client.post(url, data="test") + assert resp.status_code == 400 + + url = url_for("mergin_controller_save_diagnostic_log", app="test_app") + + # too large request + max_size = app.config["DIAGNOSTIC_LOGS_MAX_SIZE"] + resp = client.post(url, data="x" * (max_size + 1)) + assert resp.status_code == 413 + + # invalid mime type + resp = client.post(url, data="#!/usr/bin/python\n") + assert resp.status_code == 400 + + # valid request + resp = client.post(url, data="test") + assert resp.status_code == 200 + + # check if file was created + log_dir = app.config["DIAGNOSTIC_LOGS_DIR"] + assert os.path.exists(log_dir) + files = os.listdir(log_dir) + assert len(files) == 1 + assert files[0].startswith(f"{user.username}_test_app_") + assert files[0].endswith(".log") + with open(os.path.join(log_dir, files[0]), "r") as f: + content = f.read() + assert content == "test" + os.remove(os.path.join(log_dir, files[0])) + + # anonymous user + client.get(url_for("/.mergin_auth_controller_logout")) + # valid request + resp = client.post(url, data="test") + assert resp.status_code == 200 + files = os.listdir(log_dir) + assert len(files) == 1 + assert files[0].startswith(f"anonymous_test_app_") + assert files[0].endswith(".log") diff --git a/server/mergin/tests/test_statistics.py b/server/mergin/tests/test_statistics.py index dc73ef96..189b04b0 100644 --- a/server/mergin/tests/test_statistics.py +++ b/server/mergin/tests/test_statistics.py @@ -5,14 +5,12 @@ from dataclasses import asdict from datetime import timezone, datetime import json -import os from unittest.mock import patch -from flask import url_for import requests from sqlalchemy.sql.operators import is_ from mergin.auth.models import User -from mergin.sync.models import Project, ProjectRole +from mergin.sync.models import ProjectRole from ..app import db from ..stats.tasks import get_callhome_data, save_statistics, send_statistics @@ -123,43 +121,6 @@ def test_send_statistics(app, caplog): assert info.last_reported -def test_server_updates(client): - """Test proxy endpoint to fetch server updates information""" - assert client.application.config["SERVER_TYPE"] == "ce" - url = "/v1/latest-version" - - with patch("requests.get") as mock: - api_data = { - "ee": {"version": "2023.1.2", "info_url": "https://release-info.com"}, - "ce": {"version": "2023.1.2", "info_url": "https://release-info.com"}, - } - mock.return_value = Response(True, api_data) - resp = client.get(url) - assert resp.status_code == 200 - assert resp.json["version"] == api_data["ce"]["version"] - assert resp.json["major"] == 2023 - assert resp.json["minor"] == 1 - assert resp.json["fix"] == 2 - - # remove fix version - api_data["ce"]["version"] = "2023.2" - resp = client.get(url) - assert resp.status_code == 200 - assert resp.json["major"] == 2023 - assert resp.json["minor"] == 2 - assert resp.json["fix"] is None - - # invalid response - del api_data["ce"]["version"] - resp = client.get(url) - assert resp.status_code == 400 - - # 3rd party api failure - mock.side_effect = requests.exceptions.RequestException("Some failure") - resp = client.get(url) - assert resp.status_code == 400 - - def test_save_statistics(app, client): """Test save statistics celery job""" info = MerginInfo.query.first() @@ -221,39 +182,3 @@ def test_download_report(app, client): empty_file = f"{','.join(keys)}\r\n" assert resp.data.decode("UTF-8") == empty_file assert len(lines) == 1 - - -def test_save_diagnostic_log(client, app): - """Test save diagnostic log endpoint""" - url = url_for("/.mergin_stats_controller_save_diagnostic_log") - resp = client.post(url) - assert resp.status_code == 400 - - # bad request - resp = client.post(url, data="test") - assert resp.status_code == 400 - - url = url_for( - "/.mergin_stats_controller_save_diagnostic_log", - app="test_app", - username="test_user", - ) - - # too large request - resp = client.post(url, data="x" * (1024 * 1024 + 1)) - assert resp.status_code == 413 - - # valid request - resp = client.post(url, data="test") - assert resp.status_code == 200 - - # check if file was created - log_dir = app.config["DIAGNOSTIC_LOGS_DIR"] - assert os.path.exists(log_dir) - files = os.listdir(log_dir) - assert len(files) == 1 - assert files[0].startswith("test_user_test_app_") - assert files[0].endswith(".log") - with open(os.path.join(log_dir, files[0]), "r") as f: - content = f.read() - assert content == "test" diff --git a/server/mergin/tests/test_statistics_utils.py b/server/mergin/tests/test_statistics_utils.py deleted file mode 100644 index fd83ebc2..00000000 --- a/server/mergin/tests/test_statistics_utils.py +++ /dev/null @@ -1,37 +0,0 @@ -from datetime import datetime, timezone -import os -from unittest.mock import patch - -from pathvalidate import sanitize_filename - -from ..stats.utils import save_diagnostic_log_file - - -def test_save_diagnostic_log_file(client, app): - """Test save diagnostic log file""" - # Mock datetime value - test_date = "2025-05-09T12:00:00+00:00" - app_name = "t" * 256 - username = "test-user" - body = b"Test log content" - to_folder = app.config["DIAGNOSTIC_LOGS_DIR"] - - saved_file_name = save_diagnostic_log_file(app_name, username, body, to_folder) - saved_file_path = os.path.join(to_folder, saved_file_name) - assert os.path.exists(saved_file_path) - assert len(saved_file_name) == 255 - - with patch("mergin.stats.utils.datetime") as mock_datetime: - mock_datetime.now.return_value = datetime.fromisoformat(test_date) - app_name = "test_<>app" - saved_file_name = save_diagnostic_log_file(app_name, username, body, to_folder) - # Check if the file was created - assert saved_file_name == sanitize_filename( - username + "_" + app_name + "_" + test_date + ".log" - ) - saved_file_path = os.path.join(to_folder, saved_file_name) - assert os.path.exists(saved_file_path) - # Check the content of the file - with open(saved_file_path, "r") as f: - content = f.read() - assert content == body.decode("utf-8") diff --git a/server/mergin/tests/test_utils.py b/server/mergin/tests/test_utils.py index f3249daa..514928ea 100644 --- a/server/mergin/tests/test_utils.py +++ b/server/mergin/tests/test_utils.py @@ -3,14 +3,18 @@ # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial import base64 +from datetime import datetime import json import os import pytest from flask import url_for, current_app from sqlalchemy import desc -from unittest.mock import MagicMock +import os +from unittest.mock import patch +from pathvalidate import sanitize_filename + +from ..utils import save_diagnostic_log_file -from ..app import db from ..sync.utils import ( parse_gpkgb_header_size, gpkg_wkb_to_wkt, @@ -109,36 +113,6 @@ def test_parse_gpkg(): assert not wkt -def test_healthcheck(client): - client.application.config["WTF_CSRF_ENABLED"] = True - maint_file = current_app.config["MAINTENANCE_FILE"] - resp = client.post("/alive") - assert resp.status_code == 200 - resp_data = json.loads(resp.data) - assert "processing_time_ms" in resp_data - assert not resp_data["maintenance"] - - # create maintenance mode - with open(maint_file, "w+"): - resp = client.post("/alive") - assert resp.status_code == 200 - resp_data = json.loads(resp.data) - assert resp_data["maintenance"] - os.remove(maint_file) - - # tests with invalid method - resp = client.get("/alive") - assert resp.status_code == 405 - - # mock some db issue - _connect = db.engine.connect - db.engine.connect = MagicMock(side_effect=Exception("Some db issue")) - resp = client.post("/alive") - assert resp.status_code == 500 - # undo mock - db.engine.connect = _connect - - def test_is_name_allowed(): test_cases = [ ("project", True), @@ -270,3 +244,33 @@ def test_get_x_accell_uri(client): url_parts = () assert get_x_accel_uri(*url_parts) == "/download" + + +def test_save_diagnostic_log_file(client, app): + """Test save diagnostic log file""" + # Mock datetime value + test_date = "2025-05-09T12:00:00+00:00" + app_name = "t" * 256 + username = "test-user" + body = b"Test log content" + to_folder = app.config["DIAGNOSTIC_LOGS_DIR"] + + saved_file_name = save_diagnostic_log_file(app_name, username, body) + saved_file_path = os.path.join(to_folder, saved_file_name) + assert os.path.exists(saved_file_path) + assert len(saved_file_name) == 255 + + with patch("mergin.utils.datetime") as mock_datetime: + mock_datetime.now.return_value = datetime.fromisoformat(test_date) + app_name = "test_<>app" + saved_file_name = save_diagnostic_log_file(app_name, username, body) + # Check if the file was created + assert saved_file_name == sanitize_filename( + username + "_" + app_name + "_" + test_date + ".log" + ) + saved_file_path = os.path.join(to_folder, saved_file_name) + assert os.path.exists(saved_file_path) + # Check the content of the file + with open(saved_file_path, "r") as f: + content = f.read() + assert content == body.decode("utf-8") diff --git a/server/mergin/utils.py b/server/mergin/utils.py index c570acf5..9acc6124 100644 --- a/server/mergin/utils.py +++ b/server/mergin/utils.py @@ -3,9 +3,12 @@ # SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-MerginMaps-Commercial import math from collections import namedtuple -from datetime import timedelta +from datetime import datetime, timedelta, timezone from enum import Enum +import os +from flask import current_app from flask_sqlalchemy import Model +from pathvalidate import sanitize_filename from sqlalchemy import Column, JSON from sqlalchemy.sql.elements import UnaryExpression from typing import Optional @@ -116,3 +119,19 @@ def format_time_delta(delta: timedelta) -> str: else: difference = "N/A" return difference + + +def save_diagnostic_log_file(app: str, username: str, body: bytes) -> str: + """Save diagnostic log file to DIAGNOSTIC_LOGS_DIR""" + + content = body.decode("utf-8") + datetime_iso_str = datetime.now(tz=timezone.utc).isoformat() + file_name = sanitize_filename( + username + "_" + app + "_" + datetime_iso_str + ".log" + ) + to_folder = current_app.config.get("DIAGNOSTIC_LOGS_DIR") + os.makedirs(to_folder, exist_ok=True) + with open(os.path.join(to_folder, file_name), "w") as f: + f.write(content) + + return file_name From 5c295e7abbb0e5d001e11955dacee71ac879cd63 Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Tue, 13 May 2025 17:07:43 +0200 Subject: [PATCH 5/5] Added diagnostic logs to deployment --- deployment/community/README.md | 2 ++ deployment/community/docker-compose.yml | 1 + deployment/enterprise/.env.template | 3 +++ deployment/enterprise/docker-compose.yml | 1 + 4 files changed, 7 insertions(+) diff --git a/deployment/community/README.md b/deployment/community/README.md index e99b90c3..9f6ab420 100644 --- a/deployment/community/README.md +++ b/deployment/community/README.md @@ -13,4 +13,6 @@ cp .env.template .prod.env Next step is to create data directory for mergin maps `projects` with proper permissions. Should you prefer a different location, please do search and replace it in config files (`.prod.env`, `docker-compose.yml`). Make sure your volume is large enough since mergin maps keeps all projects files, their history and also needs some space for temporary processing. +If you want to persist diagnostic logs, create data directory `diagnostic_logs` with proper permissions. + For more details about deployment please check [docs](https://merginmaps.com/docs/server/install/#deployment). diff --git a/deployment/community/docker-compose.yml b/deployment/community/docker-compose.yml index 9ee27549..8b5d5156 100644 --- a/deployment/community/docker-compose.yml +++ b/deployment/community/docker-compose.yml @@ -29,6 +29,7 @@ services: user: 901:999 volumes: - ./projects:/data + - ./diagnostic_logs:/diagnostic_logs - ../common/entrypoint.sh:/app/entrypoint.sh env_file: - .prod.env diff --git a/deployment/enterprise/.env.template b/deployment/enterprise/.env.template index f6bda021..762bdd14 100644 --- a/deployment/enterprise/.env.template +++ b/deployment/enterprise/.env.template @@ -215,3 +215,6 @@ VECTOR_TILES_STYLE_URL=https://tiles-ee.merginmaps.com//styles/default.json #QGIS_EXTRACTOR_TIMEOUT=60 #OVERVIEW_MAX_FILE_SIZE=1048576 # 1MB + +# Diagnostic logs from Mobile and QGIS Plugin +DIAGNOSTIC_LOGS_DIR=/diagnostic_logs diff --git a/deployment/enterprise/docker-compose.yml b/deployment/enterprise/docker-compose.yml index cb5084c8..3bf085e4 100644 --- a/deployment/enterprise/docker-compose.yml +++ b/deployment/enterprise/docker-compose.yml @@ -12,6 +12,7 @@ services: command: ["gunicorn -w 4 --config config.py application:application"] volumes: - ./data:/data # map data dir to host + - ./diagnostic_logs:/diagnostic_logs # diagnostic logs dir - ../common/entrypoint.sh:/app/entrypoint.sh env_file: - .prod.env