From bfed5cfc6e370ee07d0ee483c4f1517934a10ec6 Mon Sep 17 00:00:00 2001 From: AdaAibaby <8872412+adababys@users.noreply.github.com> Date: Fri, 23 Oct 2026 13:53:14 +0800 Subject: [PATCH] feat(volumes): optional fail-fast guard that the volume path is on the expected filesystem Volume create does os.MkdirAll on the path derived from PERSISTENT_VOLUME_MOUNTS assuming local disk. When an operator intends a distributed filesystem (JuiceFS, CephFS, GlusterFS, SeaweedFS, BeeGFS, ...) for volumes but it is not actually mounted, MkdirAll silently creates the volume on the local root disk: the create "succeeds", data is not shared across sandboxes, and the failure only surfaces later when the local disk fills. Add an optional guard: VOLUME_EXPECTED_FSTYPE. When set, CreateVolume verifies, before MkdirAll, that the volume path is on a mount of that filesystem type (read from /proc/mounts, matching the longest mount-point prefix so a not-yet- created leaf resolves to its enclosing mount), and fails fast otherwise. Empty (the default) keeps today's local behavior with no mount check, so this is a no-op unless opted in. Distilled minimal core of the earlier #3196: that PR added a six-backend plugin framework (local/juicefs/cephfs/glusterfs/seaweedfs/beegfs, 22 files, ~1338 lines), but the five distributed backends were identical wrappers differing only in the /proc/mounts fstype string they check - every one delegated create/delete to the same directory operations. The real, non-speculative value is exactly that mount-type verification, which this implements in one file + test and one config field, without the backend zoo or any default behavior change. The broader pluggable-backend design is left to the maintainers (issue #3195 stays open). Tests cover fstype resolution (inside mount, mount point itself, prefix-colliding sibling, fallback to root, root), and the guard (no-op when unset, pass on expected fs, reject when the distributed FS is not mounted and the path lands on the local root, reject wrong fstype, error on missing mounts file). Relates to #3195. Distills #3196. --- packages/orchestrator/pkg/cfg/model.go | 5 ++ .../orchestrator/pkg/volumes/mount_guard.go | 87 +++++++++++++++++++ .../pkg/volumes/mount_guard_test.go | 85 ++++++++++++++++++ .../orchestrator/pkg/volumes/volume_create.go | 7 ++ 4 files changed, 184 insertions(+) create mode 100644 packages/orchestrator/pkg/volumes/mount_guard.go create mode 100644 packages/orchestrator/pkg/volumes/mount_guard_test.go diff --git a/packages/orchestrator/pkg/cfg/model.go b/packages/orchestrator/pkg/cfg/model.go index 88c2c92d5d..24b45197bf 100644 --- a/packages/orchestrator/pkg/cfg/model.go +++ b/packages/orchestrator/pkg/cfg/model.go @@ -109,6 +109,11 @@ type Config struct { NBDPoolSize int `env:"NBD_POOL_SIZE" envDefault:"64"` Services []string `env:"ORCHESTRATOR_SERVICES" envDefault:"orchestrator"` PersistentVolumeMounts map[string]string `env:"PERSISTENT_VOLUME_MOUNTS"` + // VolumeExpectedFSType, when set, makes volume create fail fast unless the + // volume path is on a mount of this filesystem type (e.g. "fuse.juicefs", + // "ceph"), guarding against the configured distributed FS not being mounted. + // Empty (default) keeps the local-disk behavior with no mount check. + VolumeExpectedFSType string `env:"VOLUME_EXPECTED_FSTYPE"` // TLS is the mutual TLS settings of the shared port and the peer hop. TLS TLSConfig diff --git a/packages/orchestrator/pkg/volumes/mount_guard.go b/packages/orchestrator/pkg/volumes/mount_guard.go new file mode 100644 index 0000000000..5f912c2771 --- /dev/null +++ b/packages/orchestrator/pkg/volumes/mount_guard.go @@ -0,0 +1,87 @@ +//go:build linux + +package volumes + +import ( + "bufio" + "fmt" + "os" + "strings" +) + +// procMountsPath is the kernel's mount table. Overridable in tests. +const procMountsPath = "/proc/mounts" + +// mountFSTypeFrom returns the filesystem type of the mount that contains target, +// read from a mount table in /proc/mounts format. It selects the mount whose +// mount point is the longest prefix of target, so a not-yet-created leaf resolves +// to its enclosing mount. Returns ("", nil) if no mount point is a prefix. +func mountFSTypeFrom(mountsPath, target string) (string, error) { + f, err := os.Open(mountsPath) + if err != nil { + return "", fmt.Errorf("open %q: %w", mountsPath, err) + } + defer f.Close() + + bestLen := -1 + bestType := "" + + sc := bufio.NewScanner(f) + for sc.Scan() { + fields := strings.Fields(sc.Text()) + if len(fields) < 3 { + continue + } + mp, fsType := fields[1], fields[2] + if !pathHasPrefix(target, mp) { + continue + } + if len(mp) > bestLen { + bestLen = len(mp) + bestType = fsType + } + } + if err := sc.Err(); err != nil { + return "", fmt.Errorf("scan %q: %w", mountsPath, err) + } + + return bestType, nil +} + +// pathHasPrefix reports whether mp is a path-component prefix of target (so +// "/mnt/jfs" matches "/mnt/jfs/x" and "/mnt/jfs" itself, but not "/mnt/jfs-2"). +func pathHasPrefix(target, mp string) bool { + if mp == "/" { + return true + } + if target == mp { + return true + } + + return strings.HasPrefix(target, mp+"/") +} + +// verifyVolumeMount fails fast when expectedFSType is set and the mount +// containing path is not of that type - the usual cause being that the +// configured distributed filesystem (JuiceFS, CephFS, ...) is not actually +// mounted, so volume data would silently land on the local root disk. Empty +// expectedFSType disables the check, preserving the default local behavior. +func verifyVolumeMount(path, expectedFSType string) error { + return verifyVolumeMountFrom(procMountsPath, path, expectedFSType) +} + +func verifyVolumeMountFrom(mountsPath, path, expectedFSType string) error { + if expectedFSType == "" { + return nil + } + + got, err := mountFSTypeFrom(mountsPath, path) + if err != nil { + return fmt.Errorf("verify volume mount for %q: %w", path, err) + } + if got != expectedFSType { + return fmt.Errorf("volume path %q is on a %q mount, expected %q - is the configured volume filesystem mounted?", path, got, expectedFSType) + } + + return nil +} diff --git a/packages/orchestrator/pkg/volumes/mount_guard_test.go b/packages/orchestrator/pkg/volumes/mount_guard_test.go new file mode 100644 index 0000000000..2a9db0bcaf --- /dev/null +++ b/packages/orchestrator/pkg/volumes/mount_guard_test.go @@ -0,0 +1,85 @@ +//go:build linux + +package volumes + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func writeMounts(t *testing.T, content string) string { + t.Helper() + p := filepath.Join(t.TempDir(), "mounts") + require.NoError(t, os.WriteFile(p, []byte(content), 0o644)) + return p +} + +// a realistic /proc/mounts: root local, a JuiceFS FUSE mount, and a +// prefix-colliding sibling that must NOT match /mnt/jfs. +const sampleMounts = `/dev/sda1 / ext4 rw,relatime 0 0 +JuiceFS:vol /mnt/jfs fuse.juicefs rw,nosuid,nodev 0 0 +/dev/sdb1 /mnt/jfs-backup ext4 rw,relatime 0 0 +` + +func TestMountFSTypeFrom(t *testing.T) { + t.Parallel() + mp := writeMounts(t, sampleMounts) + + for name, tc := range map[string]struct { + target string + want string + }{ + "inside juicefs mount": {"/mnt/jfs/team/vol", "fuse.juicefs"}, + "juicefs mount point itself": {"/mnt/jfs", "fuse.juicefs"}, + "prefix-colliding sibling": {"/mnt/jfs-backup/x", "ext4"}, + "falls back to root fs": {"/var/lib/whatever", "ext4"}, + "root itself": {"/", "ext4"}, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + got, err := mountFSTypeFrom(mp, tc.target) + require.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} + +func TestVerifyVolumeMountFrom(t *testing.T) { + t.Parallel() + mp := writeMounts(t, sampleMounts) + + t.Run("empty expected type is a no-op (default)", func(t *testing.T) { + t.Parallel() + assert.NoError(t, verifyVolumeMountFrom(mp, "/anywhere/at/all", "")) + }) + + t.Run("path on expected fs passes", func(t *testing.T) { + t.Parallel() + assert.NoError(t, verifyVolumeMountFrom(mp, "/mnt/jfs/team/vol", "fuse.juicefs")) + }) + + t.Run("distributed FS not mounted -> path on local root rejected", func(t *testing.T) { + t.Parallel() + // JuiceFS expected but the volume path resolves to the ext4 root mount: + // exactly the silent-to-local-disk footgun the guard prevents. + err := verifyVolumeMountFrom(mp, "/mnt/jfs-not-mounted/team/vol", "fuse.juicefs") + require.Error(t, err) + assert.Contains(t, err.Error(), "expected \"fuse.juicefs\"") + }) + + t.Run("wrong fs type rejected", func(t *testing.T) { + t.Parallel() + err := verifyVolumeMountFrom(mp, "/mnt/jfs-backup/vol", "fuse.juicefs") + require.Error(t, err) + }) + + t.Run("missing mounts file surfaces an error when enabled", func(t *testing.T) { + t.Parallel() + err := verifyVolumeMountFrom(filepath.Join(t.TempDir(), "nope"), "/mnt/jfs/v", "fuse.juicefs") + require.Error(t, err) + }) +} diff --git a/packages/orchestrator/pkg/volumes/volume_create.go b/packages/orchestrator/pkg/volumes/volume_create.go index a6fde8d067..6814768db4 100644 --- a/packages/orchestrator/pkg/volumes/volume_create.go +++ b/packages/orchestrator/pkg/volumes/volume_create.go @@ -32,6 +32,13 @@ func (s *Service) CreateVolume( attribute.String("path", fullPath), )) + // Fail fast if a volume filesystem type is configured but the path is not on + // such a mount - otherwise MkdirAll would silently create the volume on the + // local root disk when the distributed FS is not mounted. No-op by default. + if err := verifyVolumeMount(fullPath, s.config.VolumeExpectedFSType); err != nil { + return nil, err + } + if err := os.MkdirAll(fullPath, 0o700); err != nil { return nil, fmt.Errorf("failed to create volume: %w", err) }