fix: log an error if unoconv or unshare fails

Makefile

@@ -8,8 +8,7 @@ start: ## Start the development server
 	@docker-compose up -d --build
 
 test: ## Test the project
-	#docker-compose exec document-merge-service poetry run sh -c "black --check . && flake8 && mypy document_merge_service && pytest --no-cov-on-fail --cov --create-db"
-	docker-compose exec document-merge-service poetry run sh -c "pytest --no-cov-on-fail --cov --create-db --pdb"
+	docker-compose exec document-merge-service poetry run sh -c "black --check . && flake8 && mypy document_merge_service && pytest --no-cov-on-fail --cov --create-db"
 
 shell: ## Shell into document merge service
 	@docker-compose exec document-merge-service poetry shell

README.md

@@ -18,6 +18,8 @@ After installing and configuring those, download [docker-compose.yml](https://ra
 docker-compose up -d
 ```
 
+The container needs CAP_SYS_ADMIN.
+
 You can now access the api at [http://localhost:8000/api/v1/](http://localhost:8000/api/v1/) which includes a browsable api.
 
 

docker-compose.yml

@@ -2,8 +2,8 @@ version: "3.4"
 services:
   document-merge-service:
     privileged: true
-    # CAP_SYS_ADMIN wound be enabled for the container to unshare,
-    # but not on github CI
+    # CAP_SYS_ADMIN should be enabled for the container to unshare,
+    # but github CI needs more for some raason.
     # cap_add:
     #   - CAP_SYS_ADMIN
     image: ghcr.io/adfinis/document-merge-service:latest

document_merge_service/api/tests/test_unoconv.py

@@ -10,6 +10,7 @@
 import pytest
 from psutil import process_iter
 
+from .. import unoconv
 from ..unoconv import Unoconv, run_fork_safe
 
 
@@ -51,8 +52,27 @@ def test_fork(dms_test_bin):  # pragma: no cover
 
 
 def run_fork_load(test_file):
-    unoconv = Unoconv("/usr/bin/python3", shutil.which("unoconv"))
-    return unoconv.process(test_file, "pdf")
+    conv = Unoconv("/usr/bin/python3", shutil.which("unoconv"))
+    return conv.process(test_file, "pdf")
+
+
+def test_unoconv_unshare_error(loadtest_data, caplog):
+    test_file = Path(loadtest_data, "1.docx")
+    conv = Unoconv("/usr/bin/python3", shutil.which("unoconv"))
+    try:
+        save = unoconv._unshare
+        unoconv._unshare = "false"
+        conv.process(test_file, "pdf")
+        assert "CAP_SYS_ADMIN" in caplog.text
+    finally:
+        unoconv._unshare = save
+
+
+def test_unoconv_error(caplog):
+    test_file = "/asdfasdfa"
+    conv = Unoconv("/usr/bin/python3", shutil.which("unoconv"))
+    conv.process(test_file, "pdf")
+    assert "unoconv failed with returncode" in caplog.text
 
 
 def try_fork_load(arg):
@@ -67,17 +87,14 @@ def try_fork_load(arg):
         return e
 
 
-def test_fork_load(capsys):
+def test_fork_load(capsys, loadtest_data):
     count = 8
-    base = Path(__file__).parent.parent.absolute()
-    data = Path(base, "data")
-    load = Path(data, "loadtest")
     test_files = []
-    test_files += [Path(data, "docx-mailmerge.docx")] * count
-    test_files += [Path(load, "1.doc")] * count
-    test_files += [Path(load, "2.docx")] * count
-    test_files += [Path(load, "3.docx")] * count
-    test_files += [Path(load, "4.docx")] * count
+    test_files += [Path(loadtest_data.parent, "docx-mailmerge.docx")] * count
+    test_files += [Path(loadtest_data, "1.doc")] * count
+    test_files += [Path(loadtest_data, "2.docx")] * count
+    test_files += [Path(loadtest_data, "3.docx")] * count
+    test_files += [Path(loadtest_data, "4.docx")] * count
     random.shuffle(test_files)
     try:
         pool = ThreadPool(8)

document_merge_service/api/unoconv.py

@@ -1,13 +1,24 @@
+import logging
 import os
 import re
 import signal
 from collections import namedtuple
 from mimetypes import guess_type
-from subprocess import PIPE, CalledProcessError, CompletedProcess, Popen, TimeoutExpired
+from subprocess import (
+    PIPE,
+    CalledProcessError,
+    CompletedProcess,
+    Popen,
+    TimeoutExpired,
+    run as srun,
+)
 from uuid import uuid4
 
 from django.core.exceptions import ImproperlyConfigured
 
+logger = logging.getLogger(__name__)
+_unshare = "unshare"
+
 UnoconvResult = namedtuple(
     "UnoconvResult", ["stdout", "stderr", "returncode", "content_type"]
 )
@@ -202,5 +213,13 @@ def process(self, filename, convert):
             returncode=p.returncode,
             content_type=content_type,
         )
+        try:
+            srun([_unshare, "true"], check=True)
+        except CalledProcessError:
+            logger.error(
+                "Could not unshare, this process needs CAP_SYS_ADMIN to unshare."
+            )
+        else:
+            logger.error(f"unoconv failed with returncode: {p.returncode}")
 
         return result

document_merge_service/conftest.py

@@ -89,3 +89,9 @@ def dms_test_bin():
     test_path.chmod(0o755)
     yield test_path
     test_path.unlink()
+
+
+@pytest.fixture
+def loadtest_data():
+    base = Path(__file__).parent.absolute()
+    return Path(base, "api", "data", "loadtest")