config: introduce per-export configuration by amarok-60T · Pull Request #27 · polrstorage/arcticwolf

amarok-60T · 2026-05-08T13:28:26Z

Closes #26.

Overview

Arctic Wolf moves from a single-root NFSv3 server to multi-export. The repository now serves multiple independently configured exports, each with its own root, uid, FSAL backend, and (optional) read-only flag — the standard model for backing Kubernetes PersistentVolumes against a shared NFS server.

The change is delivered in five squashed commits, one per phase, all DCO-signed:

#	Commit	Phase
1	`e25f4c3`	Config schema refactor
2	`bb5590c`	File handle uid prefix
3	`ae04416`	FSAL trait split + `MultiExportFilesystem` wrapper (also pins `rust-version = "1.86"`)
4	`a90cb45`	MOUNT MNT dirpath + EXPORT (proc 5)
5	`88d8be7`	NFS per-export stats + read-only enforcement

All four CI jobs (Build / Test / Lint / NFS Integration Test) are green on the final head. The integration test runs against a real Linux NFS client mounting both /data (rw, PVC) and /backup (ro, emptyDir).

What changes (user-visible)

Config

[fsal] is removed. New [[exports]] array. Each entry must specify name (dirpath the client mounts), uid (u32 in 1..=u32::MAX, unique, embedded in file handles), backend (tagged enum — local only in v1), the backend-specific fields (for local: path), and an optional read_only (default false).
Startup validation rejects empty exports, uid == 0, duplicate uid, duplicate name, and names that do not start with /.
#[serde(deny_unknown_fields)] on Config and BackendConfig: any leftover [fsal] section in an old config now fails to parse with an explicit error, and typos inside a backend block (e.g. pat for path) are rejected at parse time. ExportConfig cannot apply #[serde(deny_unknown_fields)] directly because its backend field is #[serde(flatten)] and serde rejects that combination, but export-level typos (e.g. readOnly instead of read_only) are still rejected at config-load time: unknown keys at the export level cascade through the #[serde(flatten)] into BackendConfig, which carries #[serde(deny_unknown_fields)], so e.g. readOnly = true fails with unknown field 'readOnly', expected 'path'. Non-flattened sections ([server], [logging]) are independently caught by serde_ignored in Config::load().

Wire protocol (NFSv3 / MOUNT)

File handles are 32 bytes: bytes 0..4 = export uid (big-endian); bytes 4..32 = random opaque id. The MultiExportFilesystem router parses the prefix to dispatch to the correct inner LocalFilesystem.
MOUNT MNT now parses the client-supplied dirpath against the export list; unknown paths return MNT3ERR_NOENT.
MOUNT EXPORT (proc 5) is implemented; showmount -e <server> lists every configured export.
NFS write-class handlers (WRITE, CREATE, SETATTR, MKDIR, MKNOD, REMOVE, RMDIR, RENAME, SYMLINK, LINK) reject with NFS3ERR_ROFS when the handle belongs to a read_only export. COMMIT is intentionally not gated (matches Linux nfs-utils).
fsstat and pathconf report per-export statvfs(2) + pathconf(2) data. fsinfo keeps server-side advertised constants (rtmax/wtmax/dtpref/maxfilesize are not per-mount facts). Stale or short handles surface as NFS3ERR_STALE rather than falling through to NFS3ERR_IO.

File-handle wire format is a breaking change, acceptable since the project is pre-release. Clients caching handles across server restarts will see them invalidated; CHANGELOG-equivalent rationale lives in commit bb5590c.

Architecture

The FSAL surface splits into two traits, joined by a supertrait so RpcServer keeps a single Arc<dyn NfsBackend>:

Filesystem — per-handle operations (lookup, read, write, readdir, setattr, create/remove/mkdir/rmdir/rename/symlink/readlink/link/commit/mknod, plus the new fs_stats(handle))
ExportRegistry — per-export queries: root_handle_for(name), list_exports(), is_read_only(handle), export_for_handle(handle)
NfsBackend: Filesystem + ExportRegistry (blanket impl)

MultiExportFilesystem in src/fsal/multi_export.rs implements both traits and routes each Filesystem call to the inner LocalFilesystem identified by the handle's uid prefix. Production code only ever constructs Arc<dyn NfsBackend> via MultiExportFilesystem::build_from_config; a degenerate impl ExportRegistry for LocalFilesystem is gated behind #[cfg(test)] so unit tests can use a bare LocalFilesystem as &dyn NfsBackend without leaking the always-rw answer into a binary.

Review evolution

The PR went through three rounds of automated Copilot review and two independent Opus 4.7 (1M context) reviews before merge-readiness:

Copilot round 1 — frsize fallback in fs_stats when the platform returns 0, and absolute-path normalization in the local backend so config-relative paths don't silently break handle resolution.
Copilot round 2 — #[serde(deny_unknown_fields)] applied to Config and BackendConfig (with the documented ExportConfig flatten limitation), and pathconf / fsstat error mapping tightened so stale or short handles surface as NFS3ERR_STALE instead of NFS3ERR_IO.
Copilot round 3 — rust-version = "1.86" declared in Cargo.toml so the MSRV is explicit and CI can pin against it (folded into the Phase 3 commit, ae04416).
Opus 4.7 1M reviews — two independent passes over the full diff confirming trait-split shape, handle-routing invariants, read-only enforcement at every write entry point, and CI coverage parity.

Acceptance criteria (issue #26)

Test coverage

Rust unit tests — 100 passing. Coverage spans: config parsing and every validation failure mode (including [fsal] rejection and unknown backend discriminator); handle uid encode/decode round-trip and short-slice rejection; MultiExportFilesystem build, name and uid lookups, is_read_only paths, handle routing, cross-export rename/link rejection wired to NFS3ERR_XDEV; check_writable rw/ro/unknown-uid behaviour; LocalFilesystem fs_stats invariants; MOUNT MNT hit/miss/empty; MOUNT EXPORT byte-layout (empty / one entry / two entries).
Real Linux NFS client (CI) — deploy/k8s/deployment.yaml ships /data (rw) and /backup (ro). .github/workflows/nfstest-factory.yml verifies showmount lists both exports, both mount, cross-export isolation, MNT of an unknown path fails, writes to /backup return EROFS while reads still work, df reports per-export stats, and nfstest_posix runs against /data.

Out of scope / follow-ups

These are intentionally not in this PR; they'll be tracked as separate issues:

DUMP / UMNTALL — MOUNT procs that maintain an in-memory mount table. Low value, non-trivial state tracking.
root_squash / all_squash / anonuid / anongid — per-export uid/gid mapping.
Per-export client IP / CIDR ACLs — allowed_clients style host control.
subtree_check, secure, and other NFS export options — kernel-style fine-grained behaviour knobs.
Multiple FSAL backends per export — BackendConfig is already a tagged enum, but S3 / Ceph backends are not implemented; their arrival will widen BackendConfig::create_filesystem back to a trait object (currently narrowed to Box<LocalFilesystem> so NFS unit tests can call the inherent root_file_handle()).
Admin CLI ([FEATURE] Add admin CLI #25) — this PR was the prerequisite for that issue.
Runtime add/remove of exports — this PR assumes a server restart for config changes.
Substring-based NFS error mapping — src/nfs/rename.rs and src/nfs/link.rs use error.to_string().contains("cross-device") style matchers; a future refactor should switch to a typed error enum.
Standalone Python integration test files — tests/test_mount_multi_export.py / tests/test_export_isolation.py were folded into the nfstest CI workflow; splitting them back out would help developers reproduce CI behavior locally without spinning up the full k8s harness.

References

Issue: [FEATURE] Support multiple NFS exports #26
Single-export config that was removed: src/config.rs:38–43 (pre-[FEATURE] Support multiple NFS exports #26 line numbers)
MNT workaround that was replaced: src/mount/mnt.rs:36–50 (pre-[FEATURE] Support multiple NFS exports #26)
Previously unimplemented EXPORT procedure: src/mount/mod.rs:95–98 (pre-[FEATURE] Support multiple NFS exports #26)
Single-root LocalFilesystem: src/fsal/local/mod.rs:18–58 (pre-[FEATURE] Support multiple NFS exports #26)
FSAL trait (pre-split): src/fsal/mod.rs:96–312 (pre-[FEATURE] Support multiple NFS exports #26)
Single-FSAL startup wiring: src/main.rs:107–123 (pre-[FEATURE] Support multiple NFS exports #26)
Handle manager (now uid-aware): src/fsal/handle.rs:18–113 (pre-[FEATURE] Support multiple NFS exports #26)
RpcServer (now Arc<dyn NfsBackend>): src/rpc/server.rs:20–78 (pre-[FEATURE] Support multiple NFS exports #26)

Marking this PR ready for review.

gemini-code-assist · 2026-05-08T13:28:30Z

Important

Installation incomplete: to start using Gemini Code Assist, please ask the organization owner(s) to visit the Gemini Code Assist Admin Console and sign the Terms of Services.

- Add #[serde(deny_unknown_fields)] to Config so legacy [fsal] sections fail to parse instead of being silently ignored, matching issue polrstorage#26 acceptance criteria. - Update deploy/k8s/deployment.yaml ConfigMap to the new [[exports]] schema; the old [fsal] schema would have failed deserialization once the previous bullet landed. - Document the new config schema in CLAUDE.md so future contributors have a definitive reference. Fixes review must-fix items on PR polrstorage#27. Phase 1 of polrstorage#26. Signed-off-by: amarok-bot <freeze.amarok@gmail.com>

gemini-code-assist · 2026-05-11T08:15:05Z

Important

Installation incomplete: to start using Gemini Code Assist, please ask the organization owner(s) to visit the Gemini Code Assist Admin Console and sign the Terms of Services.

Copilot

Pull request overview

This PR refactors Arctic Wolf from a single-root NFSv3 server to a multi-export server with per-export configuration, routing, MOUNT support, per-export stats, and read-only enforcement—enabling Kubernetes-style multiple PersistentVolumes backed by one NFS endpoint.

Changes:

Replace single [fsal] config with [[exports]] (validated for non-empty, unique uid/name, leading /, uid != 0, and unknown top-level fields rejected).
Introduce export-uid-prefixed file handles and a MultiExportFilesystem router implementing NfsBackend = Filesystem + ExportRegistry.
Implement MOUNT dirpath resolution + EXPORT listing, per-export fsstat/pathconf via fs_stats, and enforce read_only across write-class NFS procedures.

Reviewed changes

Copilot reviewed 43 out of 43 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/rpc/server.rs	Switch server to hold `Arc<dyn NfsBackend>` and upcast for MOUNT vs NFS dispatch.
src/protocol/v3/mount.rs	Add helpers for MNT status-only replies and hand-rolled EXPORT list serialization + tests.
src/nfs/write.rs	Use `NfsBackend` and gate writes via `check_writable`; update unit tests for new FSAL API.
src/nfs/symlink.rs	Use `NfsBackend` and deny symlink creation on read-only exports.
src/nfs/setattr.rs	Use `NfsBackend` and deny setattr mutations on read-only exports; update tests.
src/nfs/rmdir.rs	Use `NfsBackend` and deny rmdir on read-only exports; update tests for new LocalFilesystem ctor/root handle accessor.
src/nfs/rename.rs	Use `NfsBackend` and deny rename when either side is read-only; update tests.
src/nfs/remove.rs	Use `NfsBackend` and deny remove on read-only exports; update tests.
src/nfs/readlink.rs	Use `NfsBackend` plumbing update.
src/nfs/readdirplus.rs	Use `NfsBackend` plumbing update; update tests for new LocalFilesystem ctor/root handle accessor.
src/nfs/readdir.rs	Use `NfsBackend` plumbing update.
src/nfs/read.rs	Use `NfsBackend` plumbing update; update unit tests for new FSAL API.
src/nfs/pathconf.rs	Make PATHCONF per-export by querying `fs_stats(handle)` instead of hardcoded values.
src/nfs/mod.rs	Add `access_check` module.
src/nfs/mknod.rs	Use `NfsBackend` and deny mknod on read-only exports.
src/nfs/mkdir.rs	Use `NfsBackend` and deny mkdir on read-only exports; update tests.
src/nfs/lookup.rs	Use `NfsBackend` plumbing update; update unit tests for new FSAL API.
src/nfs/link.rs	Use `NfsBackend` and deny link when either handle is read-only.
src/nfs/getattr.rs	Use `NfsBackend` plumbing update; update unit tests for new FSAL API.
src/nfs/fsstat.rs	Make FSSTAT per-export via `fs_stats(handle)` and adjust error mapping; update tests.
src/nfs/fsinfo.rs	Use `NfsBackend` plumbing update; update tests.
src/nfs/dispatcher.rs	Dispatcher now takes `&dyn NfsBackend` and routes all procedures accordingly (COMMIT explicitly ungated).
src/nfs/create.rs	Use `NfsBackend` and deny create on read-only exports; update tests.
src/nfs/commit.rs	Use `NfsBackend` plumbing update.
src/nfs/access.rs	Use `NfsBackend` plumbing update; update tests.
src/nfs/access_check.rs	New helper + tests for enforcing read-only exports in write-class procedures.
src/mount/mod.rs	Dispatch MOUNT using `&dyn ExportRegistry` and implement EXPORT procedure routing.
src/mount/mnt.rs	Make MNT resolve client dirpath to export root handle; return `MNT3ERR_NOENT` on miss + tests.
src/mount/export.rs	Implement MOUNT EXPORT (proc 5) returning configured exports + tests.
src/main.rs	Wire `Config` from library crate, build `MultiExportFilesystem` from `config.exports`, and print per-export banner.
src/lib.rs	Export `config` module and re-export FSAL types including `MultiExportFilesystem`/`NfsBackend`.
src/fsal/multi_export.rs	New router implementing `Filesystem` + `ExportRegistry`, dispatching by export-uid handle prefix.
src/fsal/mod.rs	Split FSAL surface into `Filesystem` + `ExportRegistry`, add `NfsBackend`, add per-export `FsStats`, and adjust test-only BackendConfig.
src/fsal/local/mod.rs	Local FSAL now bound to `export_uid`, provides `root_file_handle()`, implements `fs_stats()` via statvfs/pathconf, and adds test-only ExportRegistry impl.
src/fsal/handle.rs	Change handle format to `[uid prefix][random tail]`, add parsing helper, and switch to `getrandom`-backed handle tails.
src/config.rs	Replace `[fsal]` with validated `[[exports]]` schema; add `deny_unknown_fields` to top-level config and extensive validation tests.
nfstest/scripts/runner.py	Update nfstest runner to mount a configured export (`/data`) instead of `/`.
deploy/k8s/deployment.yaml	Update CI deployment config to define `/data` (rw) and `/backup` (ro) exports and mount corresponding volumes.
CLAUDE.md	Document the new `[[exports]]` schema, validation, and `[fsal]` removal.
Cargo.toml	Add `getrandom` dependency for randomized handle tails.
arcticwolf.example.toml	Update example config to use `[[exports]]` with `/data` and `/backup` and `read_only`.
ARCHITECTURE.md	Update architecture docs for multi-export, new traits, routing behavior, and completion status text.
.github/workflows/nfstest-factory.yml	Update integration workflow to verify EXPORT listing, mount both exports, isolation, ROFS enforcement, per-export df stats, and NOENT on unknown export.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 43 out of 43 changed files in this pull request and generated 3 comments.

Copilot

Pull request overview

Copilot reviewed 43 out of 43 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 43 out of 43 changed files in this pull request and generated 2 comments.

Phase 1 of polrstorage#26. Replaces the single `[fsal]` section with an `[[exports]]` array. Each export specifies `name`, `uid` (u32, 1..=u32::MAX, unique), `backend` (tagged enum, `local` only in v1), backend-specific fields (path for `local`), and an optional `read_only` flag. Config::validate() fails fast on empty exports, uid==0, duplicate uid, duplicate name, or names not starting with "/". Duplicate uid errors report both the original and conflicting export names so users can locate both occurrences quickly. The Config struct uses #[serde(deny_unknown_fields)] so legacy [fsal] sections fail to parse instead of being silently ignored. BackendConfig also denies unknown fields so a typo inside a backend block (e.g. `pat` for `path`) is rejected at parse time rather than silently dropped. ExportConfig itself cannot use deny_unknown_fields because the `backend` field is `#[serde(flatten)]` and serde rejects that combination — at the outer struct level the flattened type's own fields get reported as unknown. The trade-off: typos at the export level (e.g. `readOnly` for `read_only`) still parse silently and must be caught by review or external schema validation. BackendConfig::name() returns a static string for the active variant so banner/log code does not need to hardcode backend names. main.rs temporarily consumes config.exports[0] to keep the existing single-FSAL plumbing compiling — later phases (polrstorage#26) replace this with the per-export registry. It also prints a warning if more than one export is configured, since Phase 1 only serves the first. Updates: - arcticwolf.example.toml to the new schema - deploy/k8s/deployment.yaml ConfigMap to the new schema - CLAUDE.md with a Configuration Schema section documenting the required fields and validation rules Tests cover: parsing multiple exports, the local-backend tagged enum, unknown backend discriminators, unknown fields inside a backend block, empty TOML using defaults, and every validation failure mode (empty list, uid==0, duplicate uid, duplicate name, name not starting with "/"). Signed-off-by: amarok-bot <freeze.amarok@gmail.com>

Copilot

Pull request overview

Copilot reviewed 43 out of 44 changed files in this pull request and generated 2 comments.

Copilot

Pull request overview

Copilot reviewed 44 out of 45 changed files in this pull request and generated 3 comments.

Phase 2 of polrstorage#26. File handles now reserve the first 4 bytes for the export uid (big-endian); the remaining 28 bytes are the existing random opaque id. HandleManager and LocalFilesystem take an export_uid at construction so the wrapper added in Phase 3 can route operations by parsing the prefix. FileHandleExt::export_uid() returns Option<u32> and rejects slices shorter than the uid prefix, so the Phase 3 router can feed untrusted wire bytes through it without panicking. HandleManager uses the getrandom crate instead of opening /dev/urandom on every call, keeping handle creation off the syscall hot path. fsal::BackendConfig changes from a struct to an enum mirroring config::BackendConfig so create_filesystem(export_uid) can dispatch per backend; v1 still only ships the Local variant. This is a wire-format change to file handles. Acceptable since the project is pre-release. Signed-off-by: amarok-bot <freeze.amarok@gmail.com>

Phase 3 of polrstorage#26. Adds: - ExportRegistry trait — root_handle_for(name), list_exports(), is_read_only(handle), export_for_handle(handle). - NfsBackend super-trait (Filesystem + ExportRegistry) with a blanket impl, so RpcServer can keep a single Arc<dyn NfsBackend> while later phases hand each dispatcher the trait view it needs. - MultiExportFilesystem in src/fsal/multi_export.rs implementing both traits. It owns HashMap<u32, ExportEntry> keyed by export uid (with a name index for MOUNT MNT) and routes Filesystem operations to the inner LocalFilesystem identified by the handle's uid prefix. - Inherent LocalFilesystem::root_file_handle() so the wrapper can fetch each export's root without shadowing the async trait method. The trait method is kept until Phase 4, where MOUNT MNT switches to ExportRegistry::root_handle_for(). Error strings raised by the wrapper align with the substring-based matchers in src/nfs/* so the NFS status codes returned over the wire are correct: - Cross-export rename/link → NFS3ERR_XDEV via "cross-device ..." - Short or unknown-uid handle → NFS3ERR_STALE via "Invalid handle: ..." LocalFilesystem::resolve_handle's pre-existing "Invalid file handle" string is also aligned so existing single-export paths return STALE instead of falling through to IO. main.rs constructs MultiExportFilesystem::build_from_config(), prints every configured export at startup, and emits a warning when more than one export is configured to flag that MOUNT MNT will still route every mount request to the lowest-uid export until Phase 4. No MOUNT/NFS dispatcher signature changes yet (Phase 4/5 work). Signed-off-by: amarok-bot <freeze.amarok@gmail.com>

Phase 4 of polrstorage#26. MOUNT dispatcher now operates against &dyn ExportRegistry instead of &dyn Filesystem; rpc::server upcasts &dyn NfsBackend to that view at dispatch time. The MNT handler resolves the client-supplied dirpath via ExportRegistry::root_handle_for(name) and returns MNT3ERR_NOENT for unknown paths (constant bound to the xdrgen- generated mountstat3 enum). MOUNT EXPORT (procedure 5) is implemented in src/mount/export.rs. The exports/exportnode/groupnode wire format is hand-rolled in src/protocol/v3/mount.rs::serialize_exports because xdrgen 0.4 handles self-referential linked lists awkwardly; three byte-layout unit tests (empty, single entry, two entries) lock the wire format against regressions. showmount -e now lists every configured export. The Filesystem trait no longer exposes root_handle() — MOUNT MNT was its only caller. LocalFilesystem retains its inherent root_file_handle() for the wrapper to call internally; all NFS handler tests switch over. deploy/k8s/deployment.yaml now ships two exports (/data on a PVC, /backup on emptyDir) so the nfstest workflow exercises multi-export routing end-to-end. .github/workflows/nfstest-factory.yml verifies that showmount -e lists both exports, both mount independently, files written under /data are not visible under /backup, mounting an unknown export path fails, and nfstest_posix runs against /data. ARCHITECTURE.md is updated to drop root_handle() from the FSAL trait example and point readers at ExportRegistry::root_handle_for. No NFS dispatcher signature changes yet (Phase 5 work). Signed-off-by: amarok-bot <freeze.amarok@gmail.com>

Copilot

Pull request overview

Copilot reviewed 44 out of 45 changed files in this pull request and generated 1 comment.

Copilot

Pull request overview

Copilot reviewed 44 out of 45 changed files in this pull request and generated 1 comment.

Phase 5 of polrstorage#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-polrstorage#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 <freeze.amarok@gmail.com>

Copilot

Pull request overview

Copilot reviewed 44 out of 45 changed files in this pull request and generated no new comments.

amarok-60T force-pushed the feature/multi-export branch 2 times, most recently from dce7ed4 to 8f2139e Compare May 8, 2026 23:50

amarok-60T force-pushed the feature/multi-export branch 7 times, most recently from a6aa97e to be29508 Compare May 11, 2026 08:13

amarok-60T marked this pull request as ready for review May 11, 2026 08:15

Vicente-Cheng changed the title ~~config: introduce per-export configuration (phase 1 of #26)~~ config: introduce per-export configuration May 11, 2026

Vicente-Cheng requested review from Copilot and kchiu May 11, 2026 08:41

Copilot started reviewing on behalf of Vicente-Cheng May 11, 2026 08:42 View session