Skip to content

Commit 65e5e18

Browse files
author
Roman Hrokholskyi
committed
feat: separate DB_DIR and BACKUP_DIR, constrain local destinations
Split DATA_DIR into DB_DIR (Postgres + Redis, fast local storage) and BACKUP_DIR (archives, point to large/external mount). Each configurable independently in .env. Backend: local destination paths must be under /data/backups (container mount point). Subdirectories auto-created on demand. Frontend: path input shows fixed /data/backups/ prefix with editable subdirectory suffix so users understand the folder structure.
1 parent bd363be commit 65e5e18

6 files changed

Lines changed: 89 additions & 66 deletions

File tree

.env.example

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,11 @@ JWT_SECRET=changeme-to-a-random-secret
99
JWT_ALGORITHM=HS256
1010
ACCESS_TOKEN_EXPIRE_MINUTES=30
1111

12+
# Host paths for persistent data
13+
# DB_DIR: Postgres + Redis (keep on fast local storage)
14+
# BACKUP_DIR: backup archives (point to large/external mount if needed)
15+
DB_DIR=./data
16+
BACKUP_DIR=./data/backups
17+
1218
# App
1319
ENVIRONMENT=development

backend/api/app/services/destination_service.py

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import asyncio
2+
import os
23
import shutil
34
import uuid
45
from pathlib import Path
@@ -49,21 +50,21 @@ async def _enrich_destinations(
4950
return result
5051

5152

53+
BACKUP_ROOT = Path(os.environ.get("BACKUP_DIR", "/data/backups"))
54+
55+
5256
async def create_destination(
5357
db: AsyncSession, user: User, body: DestinationCreate
5458
) -> DestinationRead:
5559
if body.storage_type == StorageType.LOCAL:
56-
path = Path(body.path)
57-
if not path.is_absolute():
58-
raise HTTPException(
59-
status_code=status.HTTP_400_BAD_REQUEST,
60-
detail="Local destination path must be absolute",
61-
)
62-
if not path.is_dir():
60+
path = Path(body.path).resolve()
61+
if not path.is_relative_to(BACKUP_ROOT.resolve()):
6362
raise HTTPException(
6463
status_code=status.HTTP_400_BAD_REQUEST,
65-
detail=f"Path does not exist or is not a directory: {body.path}",
64+
detail=f"Local path must be under {BACKUP_ROOT}",
6665
)
66+
# Auto-create the subdirectory
67+
path.mkdir(parents=True, exist_ok=True)
6768

6869
if body.is_default:
6970
await destination_repo.clear_default(db)
@@ -105,17 +106,13 @@ async def update_destination(
105106
update_data = body.model_dump(exclude_unset=True)
106107

107108
if destination.storage_type == StorageType.LOCAL and "path" in update_data:
108-
path = Path(update_data["path"])
109-
if not path.is_absolute():
110-
raise HTTPException(
111-
status_code=status.HTTP_400_BAD_REQUEST,
112-
detail="Local destination path must be absolute",
113-
)
114-
if not path.is_dir():
109+
path = Path(update_data["path"]).resolve()
110+
if not path.is_relative_to(BACKUP_ROOT.resolve()):
115111
raise HTTPException(
116112
status_code=status.HTTP_400_BAD_REQUEST,
117-
detail=f"Path does not exist or is not a directory: {update_data['path']}",
113+
detail=f"Local path must be under {BACKUP_ROOT}",
118114
)
115+
path.mkdir(parents=True, exist_ok=True)
119116
if update_data.get("is_default"):
120117
await destination_repo.clear_default(db)
121118

backend/api/tests/integration/conftest.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import tempfile
2+
from pathlib import Path
13
from unittest.mock import AsyncMock, patch
24

35
import pytest_asyncio
@@ -29,6 +31,7 @@ async def override_get_db():
2931
new_callable=AsyncMock,
3032
return_value=None,
3133
),
34+
patch("app.services.destination_service.BACKUP_ROOT", Path(tempfile.gettempdir())),
3235
):
3336
async with AsyncClient(
3437
transport=ASGITransport(app=fastapi_app), base_url="http://test"
@@ -50,10 +53,13 @@ async def override_get_db():
5053

5154
fastapi_app.dependency_overrides[get_db] = override_get_db
5255

53-
with patch(
54-
"app.services.destination_service._get_available_bytes",
55-
new_callable=AsyncMock,
56-
return_value=None,
56+
with (
57+
patch(
58+
"app.services.destination_service._get_available_bytes",
59+
new_callable=AsyncMock,
60+
return_value=None,
61+
),
62+
patch("app.services.destination_service.BACKUP_ROOT", Path(tempfile.gettempdir())),
5763
):
5864
async with AsyncClient(
5965
transport=ASGITransport(app=fastapi_app), base_url="http://test"

backend/api/tests/test_destination_service.py

Lines changed: 37 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,58 @@
1-
from unittest.mock import AsyncMock, patch
1+
import tempfile
2+
from pathlib import Path
3+
from unittest.mock import AsyncMock, patch, MagicMock
4+
5+
import pytest
26

37
from app.services import destination_service
48
from shared.enums import StorageType
59
from shared.models import Destination
610
from shared.schemas import DestinationCreate
711

8-
# All tests patch Path.is_dir so fake paths like /tmp/test pass validation,
9-
# and _get_available_bytes so disk checks don't run on test fixtures.
10-
_PATCHES = [
11-
patch(
12+
# Shared patches: BACKUP_ROOT=/tmp so test paths like /tmp/test pass,
13+
# mkdir is no-op, disk check returns None.
14+
_patches = {
15+
"root": patch("app.services.destination_service.BACKUP_ROOT", Path(tempfile.gettempdir())),
16+
"mkdir": patch("app.services.destination_service.Path.mkdir", MagicMock()),
17+
"avail": patch(
1218
"app.services.destination_service._get_available_bytes",
1319
new_callable=AsyncMock,
1420
return_value=None,
1521
),
16-
patch("app.services.destination_service.Path.is_dir", return_value=True),
17-
]
22+
}
23+
24+
25+
def _apply_patches():
26+
mocks = {k: p.start() for k, p in _patches.items()}
27+
return mocks
28+
29+
30+
def _stop_patches():
31+
for p in _patches.values():
32+
p.stop()
1833

1934

20-
def _apply(fn):
21-
for p in reversed(_PATCHES):
22-
fn = p(fn)
23-
return fn
35+
@pytest.fixture(autouse=True)
36+
def _patch_destination_service():
37+
_apply_patches()
38+
yield
39+
_stop_patches()
2440

2541

26-
@_apply
27-
async def test_create_destination(mock_is_dir, mock_avail, db_session, admin_user):
28-
body = DestinationCreate(alias="Test Dest", path="/tmp/test")
42+
async def test_create_destination(db_session, admin_user):
43+
body = DestinationCreate(alias="Test Dest", path=f"{tempfile.gettempdir()}/test")
2944
result = await destination_service.create_destination(db_session, admin_user, body)
3045
assert result.alias == "Test Dest"
3146
assert result.storage_type == StorageType.LOCAL
3247
assert result.is_default is False
3348

3449

35-
@_apply
36-
async def test_create_default_clears_old_default(mock_is_dir, mock_avail, db_session, admin_user):
37-
body1 = DestinationCreate(
38-
alias="First", path="/tmp/first", is_default=True
39-
)
50+
async def test_create_default_clears_old_default(db_session, admin_user):
51+
body1 = DestinationCreate(alias="First", path=f"{tempfile.gettempdir()}/first", is_default=True)
4052
first = await destination_service.create_destination(db_session, admin_user, body1)
4153
assert first.is_default is True
4254

43-
body2 = DestinationCreate(
44-
alias="Second", path="/tmp/second", is_default=True
45-
)
55+
body2 = DestinationCreate(alias="Second", path=f"{tempfile.gettempdir()}/second", is_default=True)
4656
second = await destination_service.create_destination(db_session, admin_user, body2)
4757
assert second.is_default is True
4858

@@ -51,9 +61,8 @@ async def test_create_default_clears_old_default(mock_is_dir, mock_avail, db_ses
5161
assert refreshed_first.is_default is False
5262

5363

54-
@_apply
55-
async def test_list_destinations(mock_is_dir, mock_avail, db_session, admin_user):
56-
body = DestinationCreate(alias="Listed", path="/tmp/listed")
64+
async def test_list_destinations(db_session, admin_user):
65+
body = DestinationCreate(alias="Listed", path=f"{tempfile.gettempdir()}/listed")
5766
await destination_service.create_destination(db_session, admin_user, body)
5867

5968
results = await destination_service.list_destinations(db_session)
@@ -62,12 +71,9 @@ async def test_list_destinations(mock_is_dir, mock_avail, db_session, admin_user
6271
assert "Listed" in aliases
6372

6473

65-
@_apply
66-
async def test_delete_destination(mock_is_dir, mock_avail, db_session, admin_user):
67-
body = DestinationCreate(alias="ToDelete", path="/tmp/delete")
68-
created = await destination_service.create_destination(
69-
db_session, admin_user, body
70-
)
74+
async def test_delete_destination(db_session, admin_user):
75+
body = DestinationCreate(alias="ToDelete", path=f"{tempfile.gettempdir()}/delete")
76+
created = await destination_service.create_destination(db_session, admin_user, body)
7177

7278
await destination_service.delete_destination(db_session, str(created.id))
7379

docker-compose.yml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ services:
1515
POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:-gitbacker}
1616
POSTGRES_DB: ${POSTGRES_DB:-gitbacker}
1717
volumes:
18-
- ${DATA_DIR:-./data}/postgres:/var/lib/postgresql/data
18+
- ${DB_DIR:-./data/postgres}:/var/lib/postgresql/data
1919
healthcheck:
2020
test: ["CMD-SHELL", "pg_isready -U ${POSTGRES_USER:-gitbacker}"]
2121
interval: 5s
@@ -26,7 +26,7 @@ services:
2626
redis:
2727
image: redis:7-alpine
2828
volumes:
29-
- ${DATA_DIR:-./data}/redis:/data
29+
- ${DB_DIR:-./data/redis}:/data
3030
healthcheck:
3131
test: ["CMD", "redis-cli", "ping"]
3232
interval: 5s
@@ -49,7 +49,7 @@ services:
4949
ACCESS_TOKEN_EXPIRE_MINUTES: ${ACCESS_TOKEN_EXPIRE_MINUTES:-30}
5050
ENVIRONMENT: production
5151
volumes:
52-
- ${DATA_DIR:-./data}/backups:/data/backups
52+
- ${BACKUP_DIR:-./data/backups}:/data/backups
5353
ports:
5454
- "${API_PORT:-8000}:8000"
5555
restart: unless-stopped
@@ -65,7 +65,7 @@ services:
6565
DATABASE_URL: postgresql+psycopg2://${POSTGRES_USER:-gitbacker}:${POSTGRES_PASSWORD:-gitbacker}@postgres:5432/${POSTGRES_DB:-gitbacker}
6666
REDIS_URL: redis://redis:6379/0
6767
volumes:
68-
- ${DATA_DIR:-./data}/backups:/data/backups
68+
- ${BACKUP_DIR:-./data/backups}:/data/backups
6969
restart: unless-stopped
7070

7171
frontend:

frontend/src/app/destinations/page.tsx

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,12 @@ export default function DestinationsPage() {
8282
enabled: !!token,
8383
});
8484

85+
const BACKUP_ROOT = "/data/backups";
86+
const fullPath = path ? `${BACKUP_ROOT}/${path}` : BACKUP_ROOT;
87+
8588
const createMutation = useMutation({
8689
mutationFn: () =>
87-
createDestination(token!, { alias, path }),
90+
createDestination(token!, { alias, path: fullPath }),
8891
onSuccess: () => {
8992
queryClient.invalidateQueries({ queryKey: ["destinations"] });
9093
setOpen(false);
@@ -150,17 +153,22 @@ export default function DestinationsPage() {
150153
/>
151154
</div>
152155
<div className="space-y-2">
153-
<Label htmlFor="path">Path</Label>
154-
<Input
155-
id="path"
156-
value={path}
157-
onChange={(e) => setPath(e.target.value)}
158-
placeholder="/data/backups"
159-
required
160-
/>
156+
<Label htmlFor="path">Subdirectory</Label>
157+
<div className="flex items-center rounded-md border border-input">
158+
<span className="shrink-0 select-none border-r bg-muted px-3 py-2 text-xs font-mono text-muted-foreground">
159+
/data/backups/
160+
</span>
161+
<input
162+
id="path"
163+
value={path}
164+
onChange={(e) => setPath(e.target.value.replace(/^\/+/, ""))}
165+
placeholder="e.g. critical"
166+
className="flex-1 bg-transparent px-3 py-2 text-sm font-mono outline-none placeholder:text-muted-foreground"
167+
/>
168+
</div>
161169
<p className="text-xs text-muted-foreground">
162-
Absolute path to an existing directory. Must be accessible by the
163-
API and worker containers (mount it in docker-compose.yml).
170+
Optional subfolder within the backup volume. Leave empty to use
171+
the root. The directory will be created automatically.
164172
</p>
165173
</div>
166174
<Button

0 commit comments

Comments
 (0)