Skip to content

Commit bdab933

Browse files
committed
nfs: per-export filesystem stats and read-only enforcement
Phase 5 of #26. The NFS dispatcher widens to &dyn NfsBackend so every per-operation handler can borrow either the Filesystem or ExportRegistry view it needs without a parallel dispatch table. Filesystem::fs_stats(handle) is a new trait method returning per- export statvfs(2) + pathconf(2) data. LocalFilesystem implements it via raw libc calls inside tokio::task::spawn_blocking so the runtime is not stalled; MultiExportFilesystem routes by the handle's uid prefix. fsstat and pathconf reply with per-export data; fsinfo keeps server-side advertised constants because its fields (rtmax/wtmax/dtpref/maxfilesize) are not per-mount facts. A new check_writable(registry, handle) helper short-circuits all ten write-class handlers (write/create/setattr/mkdir/mknod/remove/ rmdir/rename/symlink/link) with NFS3ERR_ROFS when the handle's export has read_only=true. RENAME and LINK gate both involved handles since both write to the source export. COMMIT is intentionally not gated, matching Linux nfs-utils behaviour. impl ExportRegistry for LocalFilesystem is gated behind #[cfg(test)] so unit tests can use a bare LocalFilesystem as &dyn NfsBackend while production code can never reach the degenerate "always rw, empty exports" answers; main.rs only constructs &dyn NfsBackend via MultiExportFilesystem. deploy/k8s/deployment.yaml flips /backup to read_only=true so the nfstest workflow exercises the EROFS path against a real Linux client. .github/workflows/nfstest-factory.yml adds an explicit write/mkdir failure assertion against /backup, confirms reads still succeed, and verifies per-export df stats. ARCHITECTURE.md is refreshed to match the post-#26 trait layout (Filesystem keeps per-handle ops + fs_stats; ExportRegistry holds per-export queries; NfsBackend super-trait combines them) and the post-Phase-4 dispatcher signatures. Stale 13/22 implementation status tables and the "hardcoded /tmp/nfs_exports" TODO are removed. main.rs now consumes config via use arcticwolf::config instead of declaring its own mod config. Signed-off-by: amarok-bot <[email protected]>
1 parent c39e15e commit bdab933

32 files changed

Lines changed: 768 additions & 208 deletions

.github/workflows/nfstest-factory.yml

Lines changed: 53 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -85,24 +85,69 @@ jobs:
8585
8686
- name: Verify per-export isolation
8787
run: |
88-
# Files written through one export must not appear in the other
88+
# Files written through /data must not show up in /backup
8989
# the multi-export router dispatches by the uid prefix in each
9090
# file handle, so /data and /backup own disjoint trees even
91-
# though they share a single NFS server.
91+
# though they share a single NFS server. /backup is read_only
92+
# in Phase 5 of #26, so we only seed /data and check that the
93+
# filename doesn't leak into the other mount.
9294
sudo bash -c 'echo data-only > /mnt/nfs_data/hello-data.txt'
93-
sudo bash -c 'echo backup-only > /mnt/nfs_backup/hello-backup.txt'
9495
sudo test -f /mnt/nfs_data/hello-data.txt
95-
sudo test -f /mnt/nfs_backup/hello-backup.txt
96-
if sudo test -e /mnt/nfs_data/hello-backup.txt; then
97-
echo "ISOLATION FAILURE: /backup file visible in /data mount" >&2
98-
exit 1
99-
fi
10096
if sudo test -e /mnt/nfs_backup/hello-data.txt; then
10197
echo "ISOLATION FAILURE: /data file visible in /backup mount" >&2
10298
exit 1
10399
fi
104100
echo "per-export isolation verified"
105101
102+
# Phase 5 of #26: every write-class NFS procedure short-circuits to
103+
# NFS3ERR_ROFS when the handle's export is configured read_only=true.
104+
# The kernel client surfaces that as EROFS, so any write against
105+
# /mnt/nfs_backup must fail at the syscall layer while reads still
106+
# succeed.
107+
- name: Verify /backup rejects writes with EROFS
108+
run: |
109+
set +e
110+
sudo bash -c 'echo nope > /mnt/nfs_backup/ro-write.txt' 2> /tmp/rofs.err
111+
rc=$?
112+
set -e
113+
if [ $rc -eq 0 ]; then
114+
echo "FAILURE: write to read-only /backup unexpectedly succeeded" >&2
115+
exit 1
116+
fi
117+
# NFS3ERR_ROFS → EROFS → "Read-only file system" in shell error.
118+
# Match loosely so locale variations don't break the assertion.
119+
if ! grep -qi "read-only" /tmp/rofs.err; then
120+
echo "FAILURE: expected EROFS, got:" >&2
121+
cat /tmp/rofs.err >&2
122+
exit 1
123+
fi
124+
# mkdir against the ro export must also fail.
125+
if sudo mkdir /mnt/nfs_backup/ro-dir 2>/dev/null; then
126+
echo "FAILURE: mkdir on read-only /backup unexpectedly succeeded" >&2
127+
exit 1
128+
fi
129+
# Reads must still work — only write-class ops are gated.
130+
sudo ls /mnt/nfs_backup/ >/dev/null
131+
echo "read-only export correctly rejects writes"
132+
133+
# Phase 5 of #26: fsstat now consults statvfs(2) on the export's
134+
# root rather than returning hardcoded values. df should report
135+
# numbers consistent with the underlying mount on the pod (a PVC
136+
# for /data, an emptyDir for /backup); we don't pin exact figures
137+
# because the host filesystem varies between CI runs, but the
138+
# values must be non-zero and df must succeed for both mounts.
139+
- name: Show per-export df stats
140+
run: |
141+
df -h /mnt/nfs_data /mnt/nfs_backup
142+
data_total=$(df --output=size /mnt/nfs_data | tail -n1 | tr -d ' ')
143+
backup_total=$(df --output=size /mnt/nfs_backup | tail -n1 | tr -d ' ')
144+
if [ "$data_total" = "0" ] || [ "$backup_total" = "0" ]; then
145+
echo "FAILURE: fsstat reports zero size for an export" >&2
146+
df --output=source,size,used,avail /mnt/nfs_data /mnt/nfs_backup >&2
147+
exit 1
148+
fi
149+
echo "per-export fsstat stats look sane"
150+
106151
- name: Verify MNT NOENT for unknown dirpath
107152
run: |
108153
sudo mkdir -p /mnt/nfs_missing

ARCHITECTURE.md

Lines changed: 105 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,11 @@ arcticwolf/
6969
│ │ └── pathconf.rs # PATHCONF (proc 20)
7070
│ │
7171
│ ├── fsal/ # Filesystem Abstraction Layer
72-
│ │ ├── mod.rs # FSAL trait definition
73-
│ │ └── local.rs # Local filesystem backend
72+
│ │ ├── mod.rs # Filesystem trait, ExportRegistry trait, NfsBackend
73+
│ │ ├── handle.rs # FileHandle (with export uid prefix)
74+
│ │ ├── local/ # Local filesystem backend
75+
│ │ │ └── mod.rs
76+
│ │ └── multi_export.rs # MultiExportFilesystem: routes by handle uid prefix
7477
│ │
7578
│ └── main.rs # Server entry point
7679
@@ -265,29 +268,32 @@ async fn handle_connection(mut socket: TcpStream) -> Result<()> {
265268
**Example** (`src/nfs/dispatcher.rs`):
266269
```rust
267270
pub async fn dispatch(
268-
xid: u32,
269-
proc: u32,
271+
call: &rpc_call_msg,
270272
args_data: &[u8],
271-
filesystem: &dyn Filesystem,
273+
backend: &dyn NfsBackend,
272274
) -> Result<BytesMut> {
273-
match proc {
274-
0 => null::handle_null(xid),
275-
1 => getattr::handle_getattr(xid, args_data, filesystem),
276-
2 => setattr::handle_setattr(xid, args_data, filesystem),
277-
3 => lookup::handle_lookup(xid, args_data, filesystem),
278-
4 => access::handle_access(xid, args_data, filesystem),
279-
6 => read::handle_read(xid, args_data, filesystem),
280-
7 => write::handle_write(xid, args_data, filesystem),
281-
8 => create::handle_create(xid, args_data, filesystem),
282-
16 => readdir::handle_readdir(xid, args_data, filesystem),
283-
18 => fsstat::handle_fsstat(xid, args_data, filesystem),
284-
19 => fsinfo::handle_fsinfo(xid, args_data, filesystem),
285-
20 => pathconf::handle_pathconf(xid, args_data, filesystem),
286-
_ => Err(anyhow!("Unknown NFS procedure: {}", proc)),
275+
let xid = call.xid;
276+
match call.proc_ {
277+
0 => null::handle_null(xid).await,
278+
1 => getattr::handle_getattr(xid, args_data, backend).await,
279+
2 => setattr::handle_setattr(xid, args_data, backend).await,
280+
3 => lookup::handle_lookup(xid, args_data, backend).await,
281+
// ... 4–17 elided ...
282+
18 => fsstat::handle_fsstat(xid, args_data, backend).await,
283+
19 => fsinfo::handle_fsinfo(xid, args_data, backend).await,
284+
20 => pathconf::handle_pathconf(xid, args_data, backend).await,
285+
21 => commit::handle_commit(xid, args_data, backend).await,
286+
_ => create_notsupp_response(xid),
287287
}
288288
}
289289
```
290290

291+
Dispatchers now take `&dyn NfsBackend` (a trait combining `Filesystem +
292+
ExportRegistry`). MOUNT receives the `ExportRegistry` view to resolve
293+
`dirpath → root handle`; NFS handlers get both views from the same `Arc`
294+
so write-class handlers can call `check_writable(backend, handle)` before
295+
mutating an export.
296+
291297
### Layer 5: Protocol Handlers (`src/portmap/`, `src/mount/`, `src/nfs/`)
292298

293299
**Purpose**: Business logic for individual protocol operations.
@@ -296,16 +302,16 @@ pub async fn dispatch(
296302

297303
**Example** (`src/nfs/getattr.rs`):
298304
```rust
299-
pub fn handle_getattr(
305+
pub async fn handle_getattr(
300306
xid: u32,
301307
args_data: &[u8],
302-
filesystem: &dyn Filesystem,
308+
backend: &dyn NfsBackend,
303309
) -> Result<BytesMut> {
304310
// 1. Deserialize arguments
305311
let args = NfsMessage::deserialize_getattr3args(args_data)?;
306312

307-
// 2. Call FSAL to get attributes
308-
let attrs = filesystem.getattr(&args.object.0)?;
313+
// 2. Call FSAL to get attributes (routes by handle uid prefix)
314+
let attrs = backend.getattr(&args.object.0).await?;
309315

310316
// 3. Convert FSAL attributes to NFS format
311317
let nfs_attrs = NfsMessage::fsal_to_fattr3(&attrs);
@@ -330,44 +336,87 @@ pub fn handle_getattr(
330336

331337
**Purpose**: Abstract different filesystem backends behind a common interface.
332338

333-
**FSAL Trait**:
339+
**FSAL Trait** (abbreviated — see `src/fsal/mod.rs` for full docs):
334340
```rust
341+
#[async_trait]
335342
pub trait Filesystem: Send + Sync {
336-
// Metadata operations
337-
fn getattr(&self, handle: &FileHandle) -> Result<FileAttributes>;
338-
fn setattr_size(&self, handle: &FileHandle, size: u64) -> Result<()>;
339-
fn setattr_mode(&self, handle: &FileHandle, mode: u32) -> Result<()>;
340-
fn setattr_owner(&self, handle: &FileHandle, uid: Option<u32>, gid: Option<u32>) -> Result<()>;
343+
// Metadata
344+
async fn getattr(&self, handle: &FileHandle) -> Result<FileAttributes>;
345+
async fn setattr_size(&self, handle: &FileHandle, size: u64) -> Result<()>;
346+
async fn setattr_mode(&self, handle: &FileHandle, mode: u32) -> Result<()>;
347+
async fn setattr_owner(&self, handle: &FileHandle, uid: Option<u32>, gid: Option<u32>)
348+
-> Result<()>;
341349

342350
// Lookup and navigation
343-
fn lookup(&self, dir_handle: &FileHandle, name: &str) -> Result<FileHandle>;
351+
async fn lookup(&self, dir_handle: &FileHandle, name: &str) -> Result<FileHandle>;
352+
async fn readlink(&self, handle: &FileHandle) -> Result<String>;
353+
354+
// Data
355+
async fn read(&self, handle: &FileHandle, offset: u64, count: u32) -> Result<Vec<u8>>;
356+
async fn write(&self, handle: &FileHandle, offset: u64, data: &[u8]) -> Result<u32>;
357+
async fn commit(&self, handle: &FileHandle, offset: u64, count: u32) -> Result<()>;
358+
359+
// Directory listing
360+
async fn readdir(&self, dir_handle: &FileHandle, cookie: u64, count: u32)
361+
-> Result<(Vec<DirEntry>, bool)>;
362+
363+
// Namespace mutation
364+
async fn create(&self, dir_handle: &FileHandle, name: &str, mode: u32) -> Result<FileHandle>;
365+
async fn remove(&self, dir_handle: &FileHandle, name: &str) -> Result<()>;
366+
async fn mkdir(&self, dir_handle: &FileHandle, name: &str, mode: u32) -> Result<FileHandle>;
367+
async fn rmdir(&self, dir_handle: &FileHandle, name: &str) -> Result<()>;
368+
async fn rename(&self, from_dir: &FileHandle, from_name: &str,
369+
to_dir: &FileHandle, to_name: &str) -> Result<()>;
370+
async fn symlink(&self, dir_handle: &FileHandle, name: &str, target: &str)
371+
-> Result<FileHandle>;
372+
async fn link(&self, file_handle: &FileHandle, dir_handle: &FileHandle, name: &str)
373+
-> Result<FileHandle>;
374+
async fn mknod(&self, dir_handle: &FileHandle, name: &str,
375+
file_type: FileType, mode: u32, rdev: (u32, u32)) -> Result<FileHandle>;
344376

345-
// Data operations
346-
fn read(&self, handle: &FileHandle, offset: u64, count: u32) -> Result<Vec<u8>>;
347-
fn write(&self, handle: &FileHandle, offset: u64, data: &[u8]) -> Result<u32>;
377+
// Per-export filesystem stats — backs FSSTAT/FSINFO/PATHCONF
378+
// (the static, server-wide fields stay in the handlers)
379+
async fn fs_stats(&self, handle: &FileHandle) -> Result<FsStats>;
380+
}
381+
```
348382

349-
// Directory operations
350-
fn readdir(&self, dir_handle: &FileHandle, cookie: u64, count: u32)
351-
-> Result<Vec<DirEntry>>;
383+
Note: `fsstat` / `fsinfo` / `pathconf` are **NFS handlers**, not FSAL trait
384+
methods. The trait exposes a single `fs_stats(handle)` that returns the
385+
per-export bits (free space, name-max, link-max, …); the handlers combine
386+
that with server-wide constants (`rtmax`, `wtmax`, etc.) to assemble the
387+
NFSv3 replies.
352388

353-
// File creation
354-
fn create(&self, dir_handle: &FileHandle, name: &str, mode: u32)
355-
-> Result<FileHandle>;
389+
Per-export root handles also do not live on `Filesystem` — a multi-export
390+
backend has no single "the" root. MOUNT obtains them via the separate
391+
`ExportRegistry` trait below.
356392

357-
// Filesystem info
358-
fn fsstat(&self) -> Result<FsStats>;
359-
fn fsinfo(&self) -> FsInfo;
360-
fn pathconf(&self) -> PathConf;
393+
**ExportRegistry Trait**:
394+
```rust
395+
pub trait ExportRegistry: Send + Sync {
396+
/// Resolve the dirpath advertised to clients to that export's root handle.
397+
fn root_handle_for(&self, name: &str) -> Option<FileHandle>;
398+
399+
/// Enumerate every configured export (used by MOUNT EXPORT + banners).
400+
fn list_exports(&self) -> Vec<ExportInfo>;
401+
402+
/// True if the export that owns `handle` is read-only.
403+
fn is_read_only(&self, handle: &FileHandle) -> bool;
404+
405+
/// Decode the export uid embedded in `handle`'s prefix.
406+
fn export_for_handle(&self, handle: &FileHandle) -> Option<u32>;
361407
}
362-
```
363408

364-
Per-export root handles do not live on `Filesystem` — a multi-export backend
365-
has no single "the" root. The MOUNT path obtains them through
366-
`ExportRegistry::root_handle_for(name)` (see `src/fsal/mod.rs`), which
367-
resolves the dirpath supplied by the client to the matching export's root.
409+
/// Combined trait used by `RpcServer`: one `Arc<dyn NfsBackend>` hands
410+
/// MOUNT the `ExportRegistry` view and NFS the `Filesystem` view.
411+
pub trait NfsBackend: Filesystem + ExportRegistry {}
412+
impl<T: Filesystem + ExportRegistry> NfsBackend for T {}
413+
```
368414

369415
**Current Implementation**:
370-
- `local.rs`: Local filesystem backend using std::fs
416+
- `local/mod.rs`: Local filesystem backend (one instance per export, std::fs)
417+
- `multi_export.rs`: `MultiExportFilesystem` wraps a set of backends and
418+
dispatches `Filesystem` calls by the uid prefix in each `FileHandle`;
419+
it also implements `ExportRegistry` for the MOUNT path
371420

372421
**Future Backends**:
373422
- `memory.rs`: In-memory filesystem (testing)
@@ -732,44 +781,10 @@ make build
732781

733782
## Implementation Status
734783

735-
### Completed NFSv3 Procedures (13/22)
736-
737-
| Procedure | Number | Status | Description |
738-
|-----------|--------|--------|-------------|
739-
| NULL | 0 || Null procedure (ping) |
740-
| GETATTR | 1 || Get file attributes |
741-
| SETATTR | 2 || Set file attributes (truncate, chmod, chown) |
742-
| LOOKUP | 3 || Lookup filename |
743-
| ACCESS | 4 || Check access permissions |
744-
| READ | 6 || Read from file |
745-
| WRITE | 7 || Write to file |
746-
| CREATE | 8 || Create file |
747-
| READDIR | 16 || Read directory entries |
748-
| READDIRPLUS | 17 || Read directory with attributes and handles |
749-
| FSSTAT | 18 || Get filesystem statistics |
750-
| FSINFO | 19 || Get filesystem info |
751-
| PATHCONF | 20 || Get POSIX path configuration |
752-
753-
**Key Features Working:**
754-
- Basic file operations (read, write, create)
755-
- File attribute management (getattr, setattr)
756-
- Directory listing (readdir, readdirplus)
757-
- Shell redirection (`echo "hello" > file.txt`)
758-
- Real Linux NFS client compatibility
759-
760-
### Not Yet Implemented (9/22)
761-
762-
| Procedure | Number | Priority | Description |
763-
|-----------|--------|----------|-------------|
764-
| READLINK | 5 | Medium | Read symbolic link |
765-
| MKDIR | 9 | High | Create directory |
766-
| SYMLINK | 10 | Medium | Create symbolic link |
767-
| MKNOD | 11 | Low | Create special device |
768-
| REMOVE | 12 | High | Remove file |
769-
| RMDIR | 13 | High | Remove directory |
770-
| RENAME | 14 | High | Rename file/directory |
771-
| LINK | 15 | Medium | Create hard link |
772-
| COMMIT | 21 | Medium | Commit cached data to stable storage |
784+
All 22 NFSv3 procedures are implemented — see the
785+
[Procedure Implementation](#procedure-implementation) table above for the
786+
authoritative per-procedure status. The remaining work tracked here is
787+
testing, security, and production-readiness rather than new procedures.
773788

774789
### Python Test Improvements Needed
775790

@@ -809,9 +824,8 @@ The current Python integration tests (`tests/test_nfs_*.py`) need the following
809824
- Validate file handle security
810825

811826
**Configuration:**
812-
- Support multiple export points (currently hardcoded to `/tmp/nfs_exports`)
813-
- Add configuration file support (export paths, permissions, etc.)
814-
- Add runtime configuration reload
827+
- Add runtime configuration reload (TOML config + multi-export are already
828+
in place — see `src/config.rs` and `src/fsal/multi_export.rs`)
815829

816830
**Production Readiness:**
817831
- Add metrics and monitoring (Prometheus, etc.)
@@ -843,4 +857,4 @@ The current Python integration tests (`tests/test_nfs_*.py`) need the following
843857

844858
---
845859

846-
**Last Updated**: 2025-12-05
860+
**Last Updated**: 2026-05-11 (refreshed for multi-export feature, #26)

deploy/k8s/deployment.yaml

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,10 @@ data:
1212
mount_port = 0
1313
1414
# Two exports so the multi-export wiring is exercised end-to-end in
15-
# CI: /data is the read-write export the POSIX suite drives against,
16-
# /backup is a second rw export used to verify per-export isolation
17-
# and showmount listing.
15+
# CI: /data is the read-write export the POSIX suite drives against;
16+
# /backup is mounted read_only so the Phase 5 enforcement path (every
17+
# write-class NFS procedure short-circuits to NFS3ERR_ROFS) gets
18+
# covered against a real Linux NFS client.
1819
[[exports]]
1920
name = "/data"
2021
uid = 1
@@ -25,7 +26,7 @@ data:
2526
[[exports]]
2627
name = "/backup"
2728
uid = 2
28-
read_only = false
29+
read_only = true
2930
backend = "local"
3031
path = "/tmp/nfs_backup"
3132

0 commit comments

Comments
 (0)