fix: use host bind mount for repos so tool instances can access workspace files

Replace named Docker volume (repo_data) with bind mount (/data/repos) in both development and production compose files. The named volume trapped repo files inside the API container; tool instances started via Docker socket on the host could not see them, causing /workspace to mount as an empty directory. Also fix 6 pre-existing test failures in test_tool_instances_legacy.py caused by get_container_id/get_container_name moving to docker.py and new helpers (_ensure_web_bind_address, _ensure_container_name_in_compose) being added. - docker-compose.yml: repo_data:/data/repos -> /data/repos:/data/repos - docker-compose.traefik.yml: same change + remove repo_data volume decl - tests: update patch targets and add missing mock parameters Quality gates: pytest test_tool_instances_legacy.py (10 passed)
2026-06-02 12:47:14 +02:00
parent 4814ec2363
commit c051929f8c
3 changed files with 50 additions and 33 deletions
@@ -411,8 +411,9 @@ class TestStartInstanceLegacyFallback:
    @patch("src.api.tool_instances.wait_for_container_running")
    @patch("src.api.tool_instances.execute_compose_command")
    @patch("src.api.tool_instances.get_container_id")
-    @patch("src.api.tool_instances.get_container_name")
    @patch("src.api.tool_instances.connect_container_to_network")
+    @patch("src.api.tool_instances._ensure_container_name_in_compose")
+    @patch("src.api.tool_instances._ensure_web_bind_address")
    @patch("src.api.tool_instances._sanitize_compose_file")
    @patch("src.api.tool_instances._prepare_manifest_instance")
    @patch("src.api.tool_instances._get_user")
@@ -423,8 +424,9 @@ class TestStartInstanceLegacyFallback:
        mock_get_user,
        mock_prepare_manifest,
        mock_sanitize,
+        mock_ensure_web_bind,
+        mock_ensure_container_name,
        mock_connect_network,
-        mock_get_container_name,
        mock_get_container_id,
        mock_execute_compose,
        mock_wait_container,
@@ -440,7 +442,6 @@ class TestStartInstanceLegacyFallback:
        mock_get_project.return_value = AsyncMock()
        mock_execute_compose.return_value = (0, "started", "")
        mock_get_container_id.return_value = "abc123"
-        mock_get_container_name.return_value = "test-container"
        mock_connect_network.return_value = True
        mock_wait_container.return_value = {
            "success": True,
@@ -509,8 +510,9 @@ class TestStartInstanceLegacyFallback:
    @patch("src.api.tool_instances.wait_for_container_running")
    @patch("src.api.tool_instances.execute_compose_command")
    @patch("src.api.tool_instances.get_container_id")
-    @patch("src.api.tool_instances.get_container_name")
    @patch("src.api.tool_instances.connect_container_to_network")
+    @patch("src.api.tool_instances._ensure_container_name_in_compose")
+    @patch("src.api.tool_instances._ensure_web_bind_address")
    @patch("src.api.tool_instances._sanitize_compose_file")
    @patch("src.api.tool_instances._prepare_manifest_instance")
    @patch("src.api.tool_instances._get_user")
@@ -521,8 +523,9 @@ class TestStartInstanceLegacyFallback:
        mock_get_user,
        mock_prepare_manifest,
        mock_sanitize,
+        mock_ensure_web_bind,
+        mock_ensure_container_name,
        mock_connect_network,
-        mock_get_container_name,
        mock_get_container_id,
        mock_execute_compose,
        mock_wait_container,
@@ -538,7 +541,6 @@ class TestStartInstanceLegacyFallback:
        mock_get_project.return_value = AsyncMock()
        mock_execute_compose.return_value = (0, "started", "")
        mock_get_container_id.return_value = "abc123"
-        mock_get_container_name.return_value = "test-container"
        mock_connect_network.return_value = True
        mock_wait_container.return_value = {
            "success": True,
@@ -606,8 +608,9 @@ class TestStartInstanceLegacyFallback:
    @patch("src.api.tool_instances.wait_for_container_running")
    @patch("src.api.tool_instances.execute_compose_command")
    @patch("src.api.tool_instances.get_container_id")
-    @patch("src.api.tool_instances.get_container_name")
    @patch("src.api.tool_instances.connect_container_to_network")
+    @patch("src.api.tool_instances._ensure_container_name_in_compose")
+    @patch("src.api.tool_instances._ensure_web_bind_address")
    @patch("src.api.tool_instances._sanitize_compose_file")
    @patch("src.api.tool_instances._prepare_manifest_instance")
    @patch("src.api.tool_instances._get_user")
@@ -618,8 +621,9 @@ class TestStartInstanceLegacyFallback:
        mock_get_user,
        mock_prepare_manifest,
        mock_sanitize,
+        mock_ensure_web_bind,
+        mock_ensure_container_name,
        mock_connect_network,
-        mock_get_container_name,
        mock_get_container_id,
        mock_execute_compose,
        mock_wait_container,
@@ -635,7 +639,6 @@ class TestStartInstanceLegacyFallback:
        mock_get_project.return_value = AsyncMock()
        mock_execute_compose.return_value = (0, "started", "")
        mock_get_container_id.return_value = "abc123"
-        mock_get_container_name.return_value = "test-container"
        mock_connect_network.return_value = True
        mock_wait_container.return_value = {
            "success": True,
@@ -705,12 +708,14 @@ class TestStartInstanceSshPermissions:
    """SSH key mounts trigger permission fixes after container starts."""

    @patch("src.api.tool_instances.write_compose_file")
+    @patch("src.api.tool_instances.prepare_ssh_key_files")
    @patch("src.api.tool_instances.apply_ssh_permissions")
    @patch("src.api.tool_instances.wait_for_container_running")
    @patch("src.api.tool_instances.execute_compose_command")
    @patch("src.api.tool_instances.get_container_id")
-    @patch("src.api.tool_instances.get_container_name")
    @patch("src.api.tool_instances.connect_container_to_network")
+    @patch("src.api.tool_instances._ensure_container_name_in_compose")
+    @patch("src.api.tool_instances._ensure_web_bind_address")
    @patch("src.api.tool_instances._sanitize_compose_file")
    @patch("src.api.tool_instances._get_user")
    @patch("src.api.tool_instances._get_owned_project")
@@ -719,12 +724,14 @@ class TestStartInstanceSshPermissions:
        mock_get_project,
        mock_get_user,
        mock_sanitize,
+        mock_ensure_web_bind,
+        mock_ensure_container_name,
        mock_connect_network,
-        mock_get_container_name,
        mock_get_container_id,
        mock_execute_compose,
        mock_wait_container,
        mock_apply_ssh,
+        mock_prepare_ssh,
        mock_write_compose,
        mock_session,
        fake_user_id,
@@ -743,7 +750,6 @@ class TestStartInstanceSshPermissions:
        mock_get_project.return_value = AsyncMock()
        mock_execute_compose.return_value = (0, "started", "")
        mock_get_container_id.return_value = "abc123"
-        mock_get_container_name.return_value = "test-container"
        mock_connect_network.return_value = True
        mock_wait_container.return_value = {
            "success": True,
@@ -836,12 +842,14 @@ class TestStartInstanceSshPermissions:
        assert result["status"] == "running"
        mock_apply_ssh.assert_called_once_with("abc123", "/home/user/.ssh", "user")

+    @patch("src.api.tool_instances.prepare_ssh_key_files")
    @patch("src.api.tool_instances.apply_ssh_permissions")
    @patch("src.api.tool_instances.wait_for_container_running")
    @patch("src.api.tool_instances.execute_compose_command")
    @patch("src.api.tool_instances.get_container_id")
-    @patch("src.api.tool_instances.get_container_name")
    @patch("src.api.tool_instances.connect_container_to_network")
+    @patch("src.api.tool_instances._ensure_container_name_in_compose")
+    @patch("src.api.tool_instances._ensure_web_bind_address")
    @patch("src.api.tool_instances._sanitize_compose_file")
    @patch("src.api.tool_instances._get_user")
    @patch("src.api.tool_instances._get_owned_project")
@@ -850,12 +858,14 @@ class TestStartInstanceSshPermissions:
        mock_get_project,
        mock_get_user,
        mock_sanitize,
+        mock_ensure_web_bind,
+        mock_ensure_container_name,
        mock_connect_network,
-        mock_get_container_name,
        mock_get_container_id,
        mock_execute_compose,
        mock_wait_container,
        mock_apply_ssh,
+        mock_prepare_ssh,
        mock_session,
        fake_user_id,
        fake_project_id,
@@ -870,7 +880,6 @@ class TestStartInstanceSshPermissions:
        mock_get_project.return_value = AsyncMock()
        mock_execute_compose.return_value = (0, "started", "")
        mock_get_container_id.return_value = "abc123"
-        mock_get_container_name.return_value = "test-container"
        mock_connect_network.return_value = True
        mock_wait_container.return_value = {
            "success": True,
@@ -933,14 +942,15 @@ class TestStartInstanceSshPermissions:
        mock_session.get.side_effect = _get

        with patch("os.path.exists", return_value=True):
-            result = await start_instance(
-                project_id=fake_project_id,
-                repo_id=fake_repo_id,
-                instance_id=fake_instance_id,
-                data=None,
-                user_id=fake_user_id,
-                session=mock_session,
-            )
+            with patch("src.api.tool_instances._modify_compose_file"):
+                result = await start_instance(
+                    project_id=fake_project_id,
+                    repo_id=fake_repo_id,
+                    instance_id=fake_instance_id,
+                    data=None,
+                    user_id=fake_user_id,
+                    session=mock_session,
+                )

        assert result["status"] == "running"
        mock_apply_ssh.assert_called_once_with("abc123", "/root/.ssh", "root")
@@ -952,8 +962,9 @@ class TestStartInstanceManifestBranch:
    @patch("src.api.tool_instances.wait_for_container_running")
    @patch("src.api.tool_instances.execute_compose_command")
    @patch("src.api.tool_instances.get_container_id")
-    @patch("src.api.tool_instances.get_container_name")
    @patch("src.api.tool_instances.connect_container_to_network")
+    @patch("src.api.tool_instances._ensure_container_name_in_compose")
+    @patch("src.api.tool_instances._ensure_web_bind_address")
    @patch("src.api.tool_instances._sanitize_compose_file")
    @patch("src.api.tool_instances._prepare_manifest_instance")
    @patch("src.api.tool_instances.write_compose_file")
@@ -966,8 +977,9 @@ class TestStartInstanceManifestBranch:
        mock_write_compose,
        mock_prepare_manifest,
        mock_sanitize,
+        mock_ensure_web_bind,
+        mock_ensure_container_name,
        mock_connect_network,
-        mock_get_container_name,
        mock_get_container_id,
        mock_execute_compose,
        mock_wait_container,
@@ -987,7 +999,6 @@ class TestStartInstanceManifestBranch:
        mock_get_project.return_value = AsyncMock()
        mock_execute_compose.return_value = (0, "started", "")
        mock_get_container_id.return_value = "abc123"
-        mock_get_container_name.return_value = "test-container"
        mock_connect_network.return_value = True
        mock_wait_container.return_value = {
            "success": True,
@@ -13,7 +13,11 @@ services:
    volumes:
      - postgres_data:/var/lib/postgresql/data
    healthcheck:
-      test: ["CMD-SHELL", "pg_isready -U ${POSTGRES_USER:-headquarter} -d ${POSTGRES_DB:-headquarter}"]
+      test:
+        [
+          "CMD-SHELL",
+          "pg_isready -U ${POSTGRES_USER:-headquarter} -d ${POSTGRES_DB:-headquarter}",
+        ]
      interval: 10s
      timeout: 5s
      retries: 5
@@ -92,7 +96,7 @@ services:
      AUTHENTIK_AUTHORIZE_URL: ${AUTHENTIK_AUTHORIZE_URL:-}
      AUTHENTIK_TOKEN_URL: ${AUTHENTIK_TOKEN_URL:-}
    volumes:
-      - repo_data:/data/repos
+      - /data/repos:/data/repos
      - /data/instances:/data/instances
      - avatar_uploads:/app/uploads
      - /var/run/docker.sock:/var/run/docker.sock
@@ -116,7 +120,6 @@ services:
 volumes:
  postgres_data:
  redis_data:
-  repo_data:
  avatar_uploads:

 networks:
@@ -1,4 +1,4 @@
-version: '3.8'
+version: "3.8"

 services:
  # PostgreSQL Database
@@ -14,7 +14,11 @@ services:
    ports:
      - "5432:5432"
    healthcheck:
-      test: ["CMD-SHELL", "pg_isready -U ${POSTGRES_USER:-headquarter} -d ${POSTGRES_DB:-headquarter}"]
+      test:
+        [
+          "CMD-SHELL",
+          "pg_isready -U ${POSTGRES_USER:-headquarter} -d ${POSTGRES_DB:-headquarter}",
+        ]
      interval: 10s
      timeout: 5s
      retries: 5
@@ -57,7 +61,7 @@ services:
      REPO_BASE_PATH: /data/repos
      INSTANCE_BASE_PATH: /data/instances
    volumes:
-      - repo_data:/data/repos
+      - /data/repos:/data/repos
      - /data/instances:/data/instances
    ports:
      - "8000:8000"
@@ -91,7 +95,6 @@ services:
 volumes:
  postgres_data:
  redis_data:
-  repo_data:

 networks:
  backend: