refactor: store workspaces as {workspace_id}/{repo_name} for natural git clone layout
Working copies were stored as /data/working-copies/{repo_id}/{workspace_name}/,
so git clone was forced into a user-named directory. That meant the container
mount basename was the workspace name (e.g. main) instead of the repo name.
- Generate the workspace UUID before cloning and clone into
/data/working-copies/{workspace_id}/ so git creates {repo_name}/ naturally
- Set workspace.path to /data/working-copies/{workspace_id}/{repo_name}/
- Update _migrate_clone_into_workspace() to use the same layout
- _get_repository_mount_name() now prefers workspace.path basename and only
falls back to remote URL / repo.name for legacy repo-only instances
- Update unit tests to assert workspace path basename is used for mounts
Quality gates:
- pytest tests/unit: 219 passed
- ruff: clean on changed files
- mypy: clean on changed files
This commit is contained in:
Developer
@@ -2,7 +2,7 @@
|
||||
dir: apps/api/src/services/shared
|
||||
|
||||
## role
|
||||
Provides reusable, cross-cutting infrastructure services for security, I/O, container operations, and workspace management used throughout the API layer.
|
||||
Provides common infrastructure and utility services used across the API backend, including cross-cutting concerns like request tracing, file operations, notifications, Docker container management, and workspace lifecycle handling.
|
||||
## parent
|
||||
index: apps/api/src/services/.pi-map.index.md
|
||||
map: apps/api/src/services/.pi-map.md
|
||||
|
||||
@@ -4,7 +4,7 @@ dir: apps/api/src/services/shared
|
||||
index: apps/api/src/services/shared/.pi-map.index.md
|
||||
|
||||
## role
|
||||
Provides reusable, cross-cutting infrastructure services for security, I/O, container operations, and workspace management used throughout the API layer.
|
||||
Provides common infrastructure and utility services used across the API backend, including cross-cutting concerns like request tracing, file operations, notifications, Docker container management, and workspace lifecycle handling.
|
||||
## files
|
||||
- __init__.py | Re-exports shared service classes and functions from a services package to provide a unified public API | dep: src.services.shared.correlation, src.services.shared.file_service, src.services.shared.notification_service, src.services.shared.permission_fixer, src.services.shared.readiness_probe, src.services.shared.ssh_keys, src.services.shared.tunnel, src.services.shared.workspace_manager, correlation, file_service, notification_service, permission_fixer, readiness_probe, ssh_keys, tunnel, workspace_manager
|
||||
- correlation.py | Provides async correlation ID tracking via context variables and FastAPI middleware for request tracing. | exp: class:CorrelationIdMiddleware, method:dispatch(self, request: Request, call_next), call:request.headers.get, call:str, call:uuid.uuid4, call:CORRELATION_ID.set, call:call_next, call:CORRELATION_ID.reset, func:get_correlation_id() → str, call:CORRELATION_ID.get, call:str, call:uuid.uuid4 | dep: contextvars, uuid, fastapi, starlette.middleware.base, fastapi.Request, starlette.middleware.base.BaseHTTPMiddleware
|
||||
@@ -14,11 +14,11 @@ Provides reusable, cross-cutting infrastructure services for security, I/O, cont
|
||||
- readiness_probe.py | Executes a retryable readiness probe command inside a Docker container with configurable timeout and interval | exp: func:execute_probe(container_id: str, command: str, timeout, interval) → tuple[bool, list[str]], call:asyncio.get_event_loop().time, call:logs.append, call:logger.debug, call:subprocess.run, call:result.stdout.strip, call:result.stderr.strip, call:asyncio.sleep | dep: asyncio, logging, subprocess
|
||||
- ssh_keys.py | Decrypts and writes SSH key files to instance directories for container mounting, with optional ownership configuration and SSH config generation. | exp: func:_get_fernet() → Fernet, call:Settings, call:hashlib.sha256(settings.session_secret.encode()).digest, call:settings.session_secret.encode, call:base64.urlsafe_b64encode, call:Fernet, func:_sanitize_filename(name: str) → str, call:re.sub, call:sanitized.strip, func:prepare_ssh_key_files(instance_dir: str, ssh_key, subdir, uid, gid, key_filename, write_config) → str, call:Path, call:ssh_dir.mkdir, call:_get_fernet, call:fernet.decrypt(ssh_key.private_key_encrypted.encode()).decode, call:ssh_key.private_key_encrypted.encode, call:private_key_path.write_text, call:os.chmod, call:public_key_path.write_text, call:config_path.write_text, call:os.chown, call:logger.debug, call:logger.warning, call:os.getuid, call:str, func:write_ssh_config(ssh_dir: str, key_filenames: list[str], uid, gid) → None, call:Path, call:ssh_dir_path.mkdir, call:lines.append, call:"\n".join, call:config_path.write_text, call:os.chmod, call:os.chown, func:cleanup_ssh_key_files(instance_dir: str) → None, call:Path, call:ssh_dir.exists, call:ssh_dir.iterdir, call:file_path.unlink, call:ssh_dir.rmdir | dep: logging, os, re, pathlib, cryptography.fernet, src.config, base64, hashlib
|
||||
- tunnel.py | Re-exports Docker tunnel functions from a nested module for backward compatibility. | dep: src.services.docker.tunnel
|
||||
- workspace_manager.py | Manages Git workspace lifecycle operations including creation, deletion, sync, and migration of legacy tool instances into workspace-bound repositories. | exp: class:SyncResult, class:WorkspaceHasInstancesError, method:__init__(self, instances: list[dict]) → None, call:super().__init__, call:len, class:WorkspaceManager, method:_workspace_path(self, repo_id: uuid.UUID, name: str) → str, call:os.path.join, call:str, method:create(self, repo: GitRepository, user_id: uuid.UUID, name: str, branch, session) → Workspace, call:self._workspace_path, call:os.path.dirname, call:os.makedirs, call:contextlib.suppress, call:os.chmod, call:logger.info, call:os.path.exists, call:logger.warning, call:shutil.rmtree, call:getattr, call:session.execute, call:select(SSHKey).where, call:result.scalar_one_or_none, call:_get_fernet, call:fernet.decrypt( ssh_key_obj.private_key_encrypted.encode() ).decode, call:ssh_key_obj.private_key_encrypted.encode, call:GitService.clone, call:self._make_world_writable, call:Workspace, call:datetime.now, raise:ValueError, method:delete(self, workspace: Workspace, force, session) → None, call:self._get_instances, call:self._stop_and_delete_instance, call:os.path.exists, call:shutil.rmtree, call:logger.info, call:session.delete, raise:ValueError, raise:WorkspaceHasInstancesError, method:sync(self, workspace: Workspace, session) → SyncResult, call:logger.info, call:session.get, call:getattr, call:session.execute, call:select(SSHKey).where, call:result.scalar_one_or_none, call:_get_fernet, call:fernet.decrypt( ssh_key_obj.private_key_encrypted.encode() ).decode, call:ssh_key_obj.private_key_encrypted.encode, call:GitService.fetch, call:GitService.branch_exists_remotely, call:SyncResult, call:GitService.pull, call:self._make_world_writable, call:datetime.now, method:_make_world_writable(self, path: str) → None, call:contextlib.suppress, call:os.chmod, call:os.walk, call:os.path.join, call:os.stat, method:_get_instances(self, workspace: Workspace, session: AsyncSession) → list[ToolInstance], call:session.execute, call:select(ToolInstance).where, call:list, call:result.scalars().all, method:_stop_and_delete_instance(self, instance: ToolInstance, session: AsyncSession) → None, call:delete_tool_instance, call:logger.info, call:logger.error, method:ensure_instance_workspace(self, instance: ToolInstance, session: AsyncSession) → Workspace, call:session.get, call:self._workspace_name_exists, call:self._migrate_clone_into_workspace, call:self.create, call:session.add, call:session.commit, call:session.refresh, call:logger.info, raise:RuntimeError, method:_workspace_name_exists(self, session: AsyncSession, repo_id: uuid.UUID, name: str) → bool, call:session.execute, call:select(Workspace).where, call:result.scalar_one_or_none, method:_migrate_clone_into_workspace(self, instance: ToolInstance, repo: "GitRepository", session: AsyncSession, name: str) → Workspace, call:os.path.dirname, call:os.path.join, call:os.path.exists, call:self._workspace_path, call:os.makedirs, call:contextlib.suppress, call:os.chmod, call:shutil.rmtree, call:shutil.move, call:self._make_world_writable, call:Workspace, call:datetime.now, call:session.add, call:session.flush, raise:RuntimeError | dep: contextlib, logging, os, shutil, stat, uuid, dataclasses, datetime, typing, sqlalchemy, src.models, src.services.git.git_service, src.services.shared.ssh_keys, sqlalchemy.ext.asyncio, src.services.tool.instance_service
|
||||
- workspace_manager.py | Manages workspace lifecycle operations including creation, deletion, synchronization, and migration of legacy tool instances into workspaces with git repository cloning. | exp: class:SyncResult, class:WorkspaceHasInstancesError, method:__init__(self, instances: list[dict]) → None, call:super().__init__, call:len, class:WorkspaceManager, method:_workspace_path(self, workspace_id: uuid.UUID, repo: "GitRepository") → str, call:os.path.join, call:str, call:self._repo_directory_name, method:create(self, repo: GitRepository, user_id: uuid.UUID, name: str, branch, session) → Workspace, call:self._repo_directory_name, call:uuid.uuid4, call:os.path.join, call:str, call:logger.info, call:os.path.exists, call:logger.warning, call:shutil.rmtree, call:os.makedirs, call:contextlib.suppress, call:os.chmod, call:getattr, call:session.execute, call:select(SSHKey).where, call:result.scalar_one_or_none, call:_get_fernet, call:fernet.decrypt( ssh_key_obj.private_key_encrypted.encode() ).decode, call:ssh_key_obj.private_key_encrypted.encode, call:GitService.clone, call:os.path.isdir, call:os.listdir, call:len, call:self._make_world_writable, call:Workspace, call:datetime.now, raise:ValueError, raise:RuntimeError, method:delete(self, workspace: Workspace, force, session) → None, call:self._get_instances, call:self._stop_and_delete_instance, call:os.path.exists, call:shutil.rmtree, call:logger.info, call:session.delete, raise:ValueError, raise:WorkspaceHasInstancesError, method:sync(self, workspace: Workspace, session) → SyncResult, call:logger.info, call:session.get, call:getattr, call:session.execute, call:select(SSHKey).where, call:result.scalar_one_or_none, call:_get_fernet, call:fernet.decrypt( ssh_key_obj.private_key_encrypted.encode() ).decode, call:ssh_key_obj.private_key_encrypted.encode, call:GitService.fetch, call:GitService.branch_exists_remotely, call:SyncResult, call:GitService.pull, call:self._make_world_writable, call:datetime.now, method:_make_world_writable(self, path: str) → None, call:contextlib.suppress, call:os.chmod, call:os.walk, call:os.path.join, call:os.stat, method:_get_instances(self, workspace: Workspace, session: AsyncSession) → list[ToolInstance], call:session.execute, call:select(ToolInstance).where, call:list, call:result.scalars().all, method:_stop_and_delete_instance(self, instance: ToolInstance, session: AsyncSession) → None, call:delete_tool_instance, call:logger.info, call:logger.error, method:ensure_instance_workspace(self, instance: ToolInstance, session: AsyncSession) → Workspace, call:session.get, call:self._workspace_name_exists, call:self._migrate_clone_into_workspace, call:self.create, call:session.add, call:session.commit, call:session.refresh, call:logger.info, raise:RuntimeError, method:_workspace_name_exists(self, session: AsyncSession, repo_id: uuid.UUID, name: str) → bool, call:session.execute, call:select(Workspace).where, call:result.scalar_one_or_none, method:_migrate_clone_into_workspace(self, instance: ToolInstance, repo: "GitRepository", session: AsyncSession, name: str) → Workspace, call:os.path.dirname, call:os.path.join, call:os.path.exists, call:uuid.uuid4, call:str, call:self._repo_directory_name, call:os.makedirs, call:contextlib.suppress, call:os.chmod, call:shutil.rmtree, call:shutil.move, call:self._make_world_writable, call:Workspace, call:datetime.now, call:session.add, call:session.flush, raise:RuntimeError | dep: contextlib, logging, os, shutil, stat, uuid, dataclasses, datetime, typing, sqlalchemy, src.models, src.services.git.git_service, src.services.shared.ssh_keys, src.utils.git_url_parser, sqlalchemy.ext.asyncio, src.services.tool.instance_service
|
||||
## arch
|
||||
Modular utility services pattern with async singletons, context variables for request tracing, defensive security (path traversal/ownership isolation), Docker exec abstraction, and Git-backed workspace lifecycle management.
|
||||
Modular service-oriented architecture with singleton async services, context variable-based state propagation for request correlation, defensive path traversal protections, and Docker-centric container orchestration patterns with privilege escalation for filesystem operations.
|
||||
## tags
|
||||
error, workspace, call:self., get, key, src, call:session.execute, call:ssh
|
||||
error, call:self., get, workspace, src, key, call:str, call:session.execute
|
||||
## symbols
|
||||
- CorrelationIdMiddleware
|
||||
- FileEntry
|
||||
|
||||
@@ -17,6 +17,7 @@ from sqlalchemy import select
|
||||
from src.models import Workspace
|
||||
from src.services.git.git_service import GitService
|
||||
from src.services.shared.ssh_keys import _get_fernet
|
||||
from src.utils.git_url_parser import extract_base_repo_url
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
@@ -47,9 +48,32 @@ class WorkspaceManager:
|
||||
|
||||
BASE_PATH = "/data/working-copies"
|
||||
|
||||
def _workspace_path(self, repo_id: uuid.UUID, name: str) -> str:
|
||||
"""Return the filesystem path for a workspace."""
|
||||
return os.path.join(self.BASE_PATH, str(repo_id), name)
|
||||
@staticmethod
|
||||
def _repo_directory_name(repo: "GitRepository") -> str:
|
||||
"""Return the directory name git would create for a standard clone.
|
||||
|
||||
Prefers the name parsed from the remote URL and falls back to the
|
||||
user-provided repository name when no remote URL is available.
|
||||
"""
|
||||
if repo.remote_url:
|
||||
base_url = extract_base_repo_url(repo.remote_url) or repo.remote_url
|
||||
name = base_url.rstrip("/").split("/")[-1]
|
||||
if name.endswith(".git"):
|
||||
name = name[:-4]
|
||||
if name:
|
||||
return name
|
||||
return repo.name
|
||||
|
||||
def _workspace_path(self, workspace_id: uuid.UUID, repo: "GitRepository") -> str:
|
||||
"""Return the filesystem path for a workspace.
|
||||
|
||||
Layout: /data/working-copies/{workspace_id}/{repo_name}/
|
||||
The workspace_id prevents collisions between workspaces, and the
|
||||
repo_name matches the directory git clone naturally creates.
|
||||
"""
|
||||
return os.path.join(
|
||||
self.BASE_PATH, str(workspace_id), self._repo_directory_name(repo)
|
||||
)
|
||||
|
||||
async def create(
|
||||
self,
|
||||
@@ -74,24 +98,31 @@ class WorkspaceManager:
|
||||
Raises:
|
||||
RuntimeError: If git clone fails.
|
||||
"""
|
||||
path = self._workspace_path(repo.id, name)
|
||||
parent = os.path.dirname(path)
|
||||
os.makedirs(parent, exist_ok=True)
|
||||
# Ensure container users (various UIDs) can write to workspace dirs
|
||||
with contextlib.suppress(OSError):
|
||||
os.chmod(parent, 0o777)
|
||||
|
||||
logger.info(
|
||||
"Creating workspace: name=%s, repo=%s, branch=%s", name, repo.id, branch
|
||||
)
|
||||
|
||||
if not repo.remote_url:
|
||||
raise ValueError("Repository has no remote URL")
|
||||
|
||||
repo_dir_name = self._repo_directory_name(repo)
|
||||
workspace_id = uuid.uuid4()
|
||||
parent_path = os.path.join(self.BASE_PATH, str(workspace_id))
|
||||
clone_target = parent_path # git clone creates {repo_dir_name}/ inside this
|
||||
|
||||
logger.info(
|
||||
"Creating workspace: id=%s name=%s repo=%s branch=%s",
|
||||
workspace_id,
|
||||
name,
|
||||
repo.id,
|
||||
branch,
|
||||
)
|
||||
|
||||
# Remove stale directory from previous failed/aborted clone
|
||||
if os.path.exists(path):
|
||||
logger.warning("Removing stale workspace directory: %s", path)
|
||||
shutil.rmtree(path, ignore_errors=True)
|
||||
if os.path.exists(parent_path):
|
||||
logger.warning("Removing stale workspace directory: %s", parent_path)
|
||||
shutil.rmtree(parent_path, ignore_errors=True)
|
||||
|
||||
os.makedirs(parent_path, exist_ok=True)
|
||||
# Ensure container users (various UIDs) can write to workspace dirs
|
||||
with contextlib.suppress(OSError):
|
||||
os.chmod(parent_path, 0o777)
|
||||
|
||||
# Load SSH key if repo has one
|
||||
ssh_key = None
|
||||
@@ -108,19 +139,38 @@ class WorkspaceManager:
|
||||
ssh_key_obj.private_key_encrypted.encode()
|
||||
).decode()
|
||||
|
||||
await GitService.clone(repo.remote_url, branch, path, ssh_key=ssh_key)
|
||||
self._make_world_writable(path)
|
||||
await GitService.clone(repo.remote_url, branch, clone_target, ssh_key=ssh_key)
|
||||
|
||||
expected_path = os.path.join(parent_path, repo_dir_name)
|
||||
if not os.path.isdir(expected_path):
|
||||
# git clone can create a different directory name than expected for
|
||||
# some URL shapes; fall back to the single directory git created.
|
||||
entries = [
|
||||
entry
|
||||
for entry in os.listdir(parent_path)
|
||||
if os.path.isdir(os.path.join(parent_path, entry))
|
||||
]
|
||||
if len(entries) == 1:
|
||||
expected_path = os.path.join(parent_path, entries[0])
|
||||
else:
|
||||
raise RuntimeError(
|
||||
f"Expected git clone to create {repo_dir_name}/ under "
|
||||
f"{parent_path}, but found: {entries}"
|
||||
)
|
||||
|
||||
self._make_world_writable(expected_path)
|
||||
|
||||
workspace = Workspace(
|
||||
id=workspace_id,
|
||||
name=name,
|
||||
repo_id=repo.id,
|
||||
user_id=user_id,
|
||||
branch=branch,
|
||||
path=path,
|
||||
path=expected_path,
|
||||
status="ready",
|
||||
last_sync_at=datetime.now(),
|
||||
)
|
||||
logger.info("Workspace created: %s", workspace.id)
|
||||
logger.info("Workspace created: %s at %s", workspace.id, workspace.path)
|
||||
return workspace
|
||||
|
||||
async def delete(
|
||||
@@ -375,24 +425,30 @@ class WorkspaceManager:
|
||||
if not os.path.exists(clone_path):
|
||||
raise RuntimeError(f"Clone path not found: {clone_path}")
|
||||
|
||||
path = self._workspace_path(repo.id, name)
|
||||
parent = os.path.dirname(path)
|
||||
os.makedirs(parent, exist_ok=True)
|
||||
workspace_id = uuid.uuid4()
|
||||
parent_path = os.path.join(self.BASE_PATH, str(workspace_id))
|
||||
repo_dir_name = self._repo_directory_name(repo)
|
||||
target_path = os.path.join(parent_path, repo_dir_name)
|
||||
|
||||
os.makedirs(parent_path, exist_ok=True)
|
||||
with contextlib.suppress(OSError):
|
||||
os.chmod(parent, 0o777)
|
||||
os.chmod(parent_path, 0o777)
|
||||
|
||||
if os.path.exists(path):
|
||||
shutil.rmtree(path, ignore_errors=True)
|
||||
if os.path.exists(target_path):
|
||||
shutil.rmtree(target_path, ignore_errors=True)
|
||||
|
||||
shutil.move(clone_path, path)
|
||||
self._make_world_writable(path)
|
||||
# Move the existing clone into the repo-named subdirectory so the
|
||||
# workspace path matches the natural git clone layout.
|
||||
shutil.move(clone_path, target_path)
|
||||
self._make_world_writable(target_path)
|
||||
|
||||
workspace = Workspace(
|
||||
id=workspace_id,
|
||||
name=name,
|
||||
repo_id=repo.id,
|
||||
user_id=instance.owner_id,
|
||||
branch=instance.branch or "main",
|
||||
path=path,
|
||||
path=target_path,
|
||||
status="ready",
|
||||
last_sync_at=datetime.now(),
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user