diff --git a/Makefile b/Makefile index 284f53c..524c656 100644 --- a/Makefile +++ b/Makefile @@ -6,7 +6,7 @@ clean: rm -f go-code-tester *.log *.out cover* go-code-tester: - git clone --depth 1 git@github.com:CSM/actions.git temp-repo + git clone --depth 1 git@github.com:dell/actions.git temp-repo cp temp-repo/go-code-tester/entrypoint.sh ./go-code-tester chmod +x go-code-tester rm -rf temp-repo diff --git a/README.md b/README.md index b074c70..97652a3 100644 --- a/README.md +++ b/README.md @@ -16,3 +16,4 @@ You may obtain a copy of the License at # Mount A portable Go library for filesystem related operations such as mount, format, etc. + diff --git a/go.mod b/go.mod index 6506e99..b90f244 100644 --- a/go.mod +++ b/go.mod @@ -1,11 +1,11 @@ module github.com/dell/gofsutil -go 1.25 +go 1.26 require ( github.com/sirupsen/logrus v1.9.3 github.com/stretchr/testify v1.11.0 - golang.org/x/sys v0.39.0 + golang.org/x/sys v0.43.0 ) require ( diff --git a/go.sum b/go.sum index 898b2a3..e499f3f 100644 --- a/go.sum +++ b/go.sum @@ -10,8 +10,8 @@ github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.11.0 h1:ib4sjIrwZKxE5u/Japgo/7SJV3PvgjGiRNAvTVGqQl8= github.com/stretchr/testify v1.11.0/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.39.0 h1:CvCKL8MeisomCi6qNZ+wbb0DN9E5AATixKsvNtMoMFk= -golang.org/x/sys v0.39.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= +golang.org/x/sys v0.43.0 h1:Rlag2XtaFTxp19wS8MXlJwTvoh8ArU6ezoyFsMyCTNI= +golang.org/x/sys v0.43.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/gofsutil.go b/gofsutil.go index 7b6bc0f..98cb963 100644 --- a/gofsutil.go +++ b/gofsutil.go @@ -49,6 +49,11 @@ type FSinterface interface { fsInfo(ctx context.Context, path string) (int64, int64, int64, int64, int64, int64, error) getNVMeController(device string) (string, error) + // Space reclamation — private (architecture-specific) + fstrim(ctx context.Context, mountPoint string) (*FstrimResult, error) + blkdiscard(ctx context.Context, devicePath string) (*BlkdiscardResult, error) + checkDiscardSupport(ctx context.Context, devicePath string) (*DiscardCapability, error) + // Architecture agnostic implementations, generally just wrappers GetDiskFormat(ctx context.Context, disk string) (string, error) Format(ctx context.Context, source, target, fsType string, options ...string) error @@ -75,6 +80,11 @@ type FSinterface interface { GetMpathNameFromDevice(ctx context.Context, device string) (string, error) FsInfo(ctx context.Context, path string) (int64, int64, int64, int64, int64, int64, error) GetNVMeController(device string) (string, error) + + // Space reclamation — public (architecture-agnostic wrappers) + Fstrim(ctx context.Context, mountPoint string) (*FstrimResult, error) + Blkdiscard(ctx context.Context, devicePath string) (*BlkdiscardResult, error) + CheckDiscardSupport(ctx context.Context, devicePath string) (*DiscardCapability, error) } // MultipathDevDiskByIDPrefix is a pathname prefix for items located in /dev/disk/by-id diff --git a/gofsutil_fs_test.go b/gofsutil_fs_test.go index 3b0d942..9571720 100644 --- a/gofsutil_fs_test.go +++ b/gofsutil_fs_test.go @@ -1322,13 +1322,13 @@ func TestFS_FindFSType(t *testing.T) { wantErr bool }{ { - name: "Success", + name: "Error_Mount_Path_Not_Found", args: args{ ctx: context.Background(), mountpoint: "mount_path", }, wantFsType: "", - wantErr: false, + wantErr: true, }, } for _, tt := range tests { diff --git a/gofsutil_fsck.go b/gofsutil_fsck.go new file mode 100644 index 0000000..bfde72a --- /dev/null +++ b/gofsutil_fsck.go @@ -0,0 +1,405 @@ +package gofsutil + +import ( + "bytes" + "context" + "errors" + "fmt" + "os" + "os/exec" + "strings" + "syscall" + "time" + + log "github.com/sirupsen/logrus" +) + +const ( + StartedFSCheckEvent = "Starting file system check" + FoundNoErrorsEvent = "Found no errors in file system" + FoundErrorsEvent = "Found errors in file system" + FoundDirtyLogEvent = "Found dirty log in file system" + FSCheckTimedOutEvent = "File system check timed out" + FSCheckFailedEvent = "File system check failed" + + StartFSRepairEvent = "Starting file system repair" + FinishedFSRepairEvent = "File system errors fixed" + FSRepairTimedOutEvent = "File system repair timed out" + FSRepairFailedEvent = "File system errors could not be fixed" + StartLogReplayEvent = "Starting file system log replay" + LogReplayFailedEvent = "File system log replay failed" + LogReplayDoneEvent = "File system log replay done" +) + +var FSCheckLinesToLog = 20 + +// e2fsck exit codes: https://www.man7.org/linux/man-pages/man8/e2fsck.8.html +type extExitCode int + +func (e extExitCode) isFoundErrors() bool { + // 1 or 2 were never observed, but according to e2fsck documentation + // they indicate presense of repairable errors. + return e&1 != 0 || e&2 != 0 || e&4 != 0 +} + +func (e extExitCode) isCanceledByUser() bool { + return e&32 != 0 +} + +// xfsrepair exit codes: https://www.man7.org/linux/man-pages/man8/xfs_repair.8.html +const ( + xfsCodeNoErrorsFound = 0 + xfsCodeFoundRepairableErrors = 1 + xfsCodeFoundDirtyLog = 2 + xfsCodeErrorsRepaired = 0 +) + +// FSChecker can do file system error checks using "e2fsck -nf" for ext and "xfs_repair -n" for xfs. +// It can also fix repairable errors using "e2fsck -p" or "xfs_repair" respectively. +// FSChecker instance for a given file system is created using GetFSChecker. +type FSChecker interface { + // Check checks the fs for errors. If repairable errors are found and doRepair is true, + // Check tries to do a safe repair. If repairable errors are found and doRepair is false, + // Check returns an error. If unrepairable errors are found, or repair fails, Check returns an error. + // If the context passed to Check times out or is closed before fs check and repair completes, + // the process in interrupted and Check returns an error. + Check(ctx context.Context, doRepair bool) error + // repair is called when needed by the Check function to fix repairable file system errors. + repair(ctx context.Context) error +} + +// Allows monitoring fs check and repair lifecycle +type FSCheckObserver interface { + OnEvent(message string) +} + +type fsCheckerCommon struct { + devPath string + observer FSCheckObserver +} + +type ( + extChecker fsCheckerCommon + xfsChecker fsCheckerCommon +) + +type NoopFSCheckObserver struct{} + +func (n *NoopFSCheckObserver) OnEvent(_ string) {} + +func GetFSChecker(devPath, fsType string, observer FSCheckObserver) (FSChecker, error) { + if observer == nil { + observer = &NoopFSCheckObserver{} + } + + ch := &fsCheckerCommon{ + devPath: devPath, + observer: observer, + } + + fsType = strings.ToLower(fsType) + + switch fsType { + case "ext4", "ext3", "ext2", "ext": + return (*extChecker)(ch), nil + case "xfs": + return (*xfsChecker)(ch), nil + } + + return nil, fmt.Errorf("unsupported fs type: %s", fsType) +} + +func (ch *extChecker) Check(ctx context.Context, doRepair bool) error { + defer logDuration("ext check/repair", time.Now()) + + ch.observer.OnEvent(StartedFSCheckEvent) + + // Do up to 2 iterations: first for check and optional repair, second for post-repair check + for pass := 0; pass < 2; pass++ { + // Check for file system errors + rc, err := OSExecFn(ctx, "e2fsck", "-nf", ch.devPath) + code := extExitCode(rc) + + if err == nil { + // Implies exit code 0 + if pass == 0 { + ch.observer.OnEvent(FoundNoErrorsEvent) + } else { + ch.observer.OnEvent(FinishedFSRepairEvent) + } + return nil + + } else if code.isCanceledByUser() || isProcKilled(err) { + ch.observer.OnEvent(FSCheckTimedOutEvent) + return fmt.Errorf("checking file system on %s interrupted due to context timeout (%d)", ch.devPath, rc) + + } else if code.isFoundErrors() { + if pass == 0 { + // First pass found fs errors + ch.observer.OnEvent(FoundErrorsEvent) + if doRepair { + err := ch.repair(ctx) + if err != nil { + // Error out on timeout + return err + } + // Do another pass for final check + continue + } + return fmt.Errorf("file system on %s has errors (%d)", ch.devPath, rc) + } + + // Final (post-repair) pass still found fs errors + ch.observer.OnEvent(FSRepairFailedEvent) + return fmt.Errorf("repairing file system on %s failed (%d): %w", ch.devPath, rc, err) + } + + if pass == 0 { + ch.observer.OnEvent(FSCheckFailedEvent) + return fmt.Errorf("checking file system on %s failed (%d): %w", ch.devPath, rc, err) + } + ch.observer.OnEvent(FSRepairFailedEvent) + return fmt.Errorf("repairing file system on %s failed (%d): %w", ch.devPath, rc, err) + } + + // Should never get here, but compiler requires this. + return fmt.Errorf("unexpected error") +} + +func (ch *extChecker) repair(ctx context.Context) error { + defer logDuration("ext repair", time.Now()) + + ch.observer.OnEvent(StartFSRepairEvent) + + // Safely repair ext with the preen option. Not all errors can be fixed + // in this mode, but this will be verified during the final full check. + + rc, err := OSExecFn(ctx, "e2fsck", "-p", ch.devPath) + code := extExitCode(rc) + + log.Debugf("e2fsck -p %s: exit code %d", ch.devPath, rc) + + if code.isCanceledByUser() || isProcKilled(err) { + ch.observer.OnEvent(FSRepairTimedOutEvent) + return fmt.Errorf("repairing file system on %s interrupted due to context timeout (%d)", ch.devPath, rc) + } + // All other errors and exit codes in the preen mode are not indicative of success + // or failure (see tests/fsckdata/README.md), so we'll rely on the final errors check. + + return nil +} + +func (ch *xfsChecker) Check(ctx context.Context, doRepair bool) error { + defer logDuration("xfs check/repair", time.Now()) + + ch.observer.OnEvent(StartedFSCheckEvent) + + // Normally do one pass. Second pass is needed if dirty log is found. + for pass := 0; pass < 2; pass++ { + + rc, err := OSExecFn(ctx, "xfs_repair", "-n", ch.devPath) + + if err == nil && rc == xfsCodeNoErrorsFound { + ch.observer.OnEvent(FoundNoErrorsEvent) + return nil + + } else if rc == xfsCodeFoundRepairableErrors { + ch.observer.OnEvent(FoundErrorsEvent) + if doRepair { + return ch.repair(ctx) + } + return fmt.Errorf("file system on %s has errors that can be fixed (%d)", ch.devPath, rc) + + } else if rc == xfsCodeFoundDirtyLog { + if pass == 0 { + ch.observer.OnEvent(FoundDirtyLogEvent) + err := ch.replayLog(ctx) + if err != nil { + return fmt.Errorf("failed to replay log of file system on %s: %w", ch.devPath, err) + } + // Re-run fs check again and possibly do repair + continue + } + return fmt.Errorf("file system on %s still has dirty log after replay (%d)", ch.devPath, rc) + + } else if isProcKilled(err) { + ch.observer.OnEvent(FSCheckTimedOutEvent) + return fmt.Errorf("checking file system on %s interrupted due to context timeout: %w", ch.devPath, err) + } + + ch.observer.OnEvent(FSCheckFailedEvent) + return fmt.Errorf("checking file system on %s failed (%d): %w", ch.devPath, rc, err) + } + + // Should never get here, but compiler requires this. + return fmt.Errorf("unexpected error") +} + +func (ch *xfsChecker) repair(ctx context.Context) error { + defer logDuration("xfs repair", time.Now()) + + ch.observer.OnEvent(StartFSRepairEvent) + + rc, err := OSExecFn(ctx, "xfs_repair", ch.devPath) + + if err == nil && rc == xfsCodeErrorsRepaired { + ch.observer.OnEvent(FinishedFSRepairEvent) + return nil + } else if isProcKilled(err) { + ch.observer.OnEvent(FSRepairTimedOutEvent) + return fmt.Errorf("repairing file system on %s interrupted due to context timeout: %w", ch.devPath, err) + } + + ch.observer.OnEvent(FSRepairFailedEvent) + return fmt.Errorf("repairing file system on %s failed (%d): %w", ch.devPath, rc, err) +} + +// try mount-unmounting to a temp dir to cause the log replay by kernel +func (ch *xfsChecker) replayLog(ctx context.Context) (retErr error) { + ch.observer.OnEvent(StartLogReplayEvent) + defer func() { + if retErr != nil { + ch.observer.OnEvent(LogReplayFailedEvent) + } else { + ch.observer.OnEvent(LogReplayDoneEvent) + } + }() + + fsMounted := false + tmpMountPoint, err := os.MkdirTemp("", "replay") + if err != nil { + return fmt.Errorf("failed to create temp dir: %w", err) + } + defer func() { + // SAFETY: Don't attempt to remove tmpMountPoint if the fs is still mounted to it. + if !fsMounted { + // SAFETY: Intentionally not using RemoveAll, since this directory is expected to be empty after unmount. + err := os.Remove(tmpMountPoint) + if err != nil { + log.Errorf("Failed to remove %s: %v", tmpMountPoint, err) + } + } + }() + + // SAFETY: Mounting fs read-only to prevent accidental data alteration. + rc, err := OSExecFn(ctx, "mount", "-o", "ro", ch.devPath, tmpMountPoint) + + if err == nil && rc == 0 { + fsMounted = true + rc, err := OSExecFn(ctx, "umount", tmpMountPoint) + + if err == nil && rc == 0 { + fsMounted = false + return nil + } + return fmt.Errorf("failed to unmount file system on %s to %s (%d): %w", ch.devPath, tmpMountPoint, rc, err) + } + return fmt.Errorf("failed to mount file system %s to %s (%d): %w", ch.devPath, tmpMountPoint, rc, err) +} + +// Mockable function for testing +var OSExecFn = execOSCommand + +func execOSCommand(ctx context.Context, name string, args ...string) (rc int, err error) { + cmd := exec.CommandContext(ctx, name, args...) // #nosec G702 + + // Start the child process in a new process group + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + // Give the Cancel (SIGINT) some time to work before a hard kill + cmd.WaitDelay = 2 * time.Second + + cmd.Cancel = func() error { + // Send SIGINT to the child's process group to also signal its children. + return syscall.Kill(-cmd.Process.Pid, syscall.SIGINT) + } + + // Collect all output in one buffer + errBuffer := bytes.Buffer{} + cmd.Stdout = &errBuffer + cmd.Stderr = &errBuffer + + err = cmd.Run() + if err != nil { + var exitError *exec.ExitError + if errors.As(err, &exitError) { + rc = exitError.ExitCode() + } else { + rc = -1 + } + out := truncOutput(errBuffer, name, args...) + if out.Len() > 0 { + log.Errorln(out.String()) + } + } + + return rc, err +} + +func isProcKilled(err error) bool { + var ee *exec.ExitError + if errors.As(err, &ee) { + if ws, ok := ee.Sys().(syscall.WaitStatus); ok { + if ws.Signaled() { + // The process terminated due to an unhandled signal. + // Normally, ws.Signal() would be syscall.SIGINT or syscall.SIGKILL + // that we sent upon the context expiration. But we will not check + // for specific signal here, since the process can be killed externally, + // and we still want to interpret this as interruption. + return true + } + } + } + return false +} + +func logDuration(stage string, startTime time.Time) { + log.Infof("%s took %.3fs", stage, time.Since(startTime).Seconds()) +} + +// Collect up to FSCheckLinesToLog last lines in the output. Empty lines are ignored. +func truncOutput(errBuf bytes.Buffer, name string, args ...string) *bytes.Buffer { + outBuf := bytes.Buffer{} + + if errBuf.Len() == 0 { + return &outBuf + } + + n := FSCheckLinesToLog + + // Split by newline and filter out empty lines + allLines := strings.Split(errBuf.String(), "\n") + + lines := make([]string, 0, n) + + // Iterate all err lines in the reverse order and filter out empty lines + for i := len(allLines) - 1; i >= 0; i-- { + if len(lines) >= n { + lines = append(lines, "...") + break + } + if trimmed := strings.TrimRight(allLines[i], " \t"); trimmed != "" { + lines = append(lines, trimmed) + } + } + + if len(lines) == 0 { + return &outBuf + } + + cmdStr := name + if len(args) > 0 { + cmdStr += " " + strings.Join(args, " ") + } + + outBuf.WriteString("stderr from command: " + cmdStr + "\n") + + // Write the collected lines to outBuf taking into account the reverse order + for i := len(lines) - 1; i >= 0; i-- { + outBuf.WriteString(lines[i]) + if i > 0 { + outBuf.WriteByte('\n') + } + } + + return &outBuf +} diff --git a/gofsutil_fsck_test.go b/gofsutil_fsck_test.go new file mode 100644 index 0000000..84e71f6 --- /dev/null +++ b/gofsutil_fsck_test.go @@ -0,0 +1,1302 @@ +package gofsutil + +import ( + "bytes" + "context" + "errors" + "flag" + "fmt" + "os" + "os/exec" + "os/signal" + "path/filepath" + "reflect" + "regexp" + "strings" + "syscall" + "testing" + "time" +) + +// ---- Test entrypoint - either runs tests (default) or acts as a helper process (if -helperProc is passed) ---- + +func TestMain(m *testing.M) { + helperProc := flag.Bool("helperProc", false, "start a helper process instead of running tests") + helperReadyDir := flag.String("helperReadyDir", "", "ready dir for helper process") + helperNoTrap := flag.Bool("helperNoTrap", false, "helper process without SIGINT trap") + + flag.Parse() + + if *helperProc == true { + // Act as the helper child process; do NOT run the test suite. + if err := os.MkdirAll(*helperReadyDir, 0o755); err != nil { + panic(err) + } + fmt.Println("== helper started") + sigCh := make(chan os.Signal, 1) + if *helperNoTrap == false { + signal.Notify(sigCh, syscall.SIGINT) + } + select { + case <-sigCh: + fmt.Println("== helper got SIGINT") + os.Exit(32) + case <-time.After(time.Second * 30): + fmt.Println("== helper finished") + os.Exit(0) + } + } + + // Normal test execution + os.Exit(m.Run()) +} + +// ---- Test utilities --------------------------------------------------------- + +type capturingObserver struct { + events []string +} + +func (c *capturingObserver) OnEvent(msg string) { + c.events = append(c.events, msg) +} + +func setMockExec(f func(ctx context.Context, name string, args ...string) (int, error)) func() { + orig := OSExecFn + OSExecFn = f + return func() { OSExecFn = orig } +} + +func assertEvents(t *testing.T, got []string, want []string) { + t.Helper() + if !reflect.DeepEqual(got, want) { + t.Fatalf("unexpected events.\n got : %#v\n want: %#v", got, want) + } +} + +// ---- GetFSChecker ----------------------------------------------------------- + +func TestGetFSChecker_Supported(t *testing.T) { + obs := &capturingObserver{} + + for _, fsType := range []string{"ext4", "ext3", "ext2", "ext"} { + ch, err := GetFSChecker("/dev/sda1", fsType, obs) + if err != nil { + t.Fatalf("unexpected error for %s: %v", fsType, err) + } + if ch == nil { + t.Fatalf("expected non-nil checker for %s", fsType) + } + if _, ok := ch.(*extChecker); !ok { + t.Fatalf("expected extChecker for %s, got %T", fsType, ch) + } + } + + ch2, err := GetFSChecker("/dev/sdb1", "xfs", obs) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if ch2 == nil { + t.Fatalf("expected non-nil checker for xfs") + } + if _, ok := ch2.(*xfsChecker); !ok { + t.Fatalf("expected xfsChecker, got %T", ch2) + } +} + +func TestGetFSChecker_UnsupportedReturnsNil(t *testing.T) { + obs := &capturingObserver{} + ch, err := GetFSChecker("/dev/sdc1", "btrfs", obs) + if err == nil { + t.Fatalf("expected error, got nil") + } + if ch != nil { + t.Fatalf("expected nil checker for unsupported fs") + } +} + +// ---- EXT: Check ------------------------------------------------------------- + +func TestEXTChecker_Check_NoErrors(t *testing.T) { + restore := setMockExec(func(_ context.Context, name string, args ...string) (int, error) { + if name == "e2fsck" && len(args) == 2 && args[0] == "-nf" { + return 0, nil // no errors + } + return -1, errors.New("unexpected call") + }) + defer restore() + + obs := &capturingObserver{} + ch, _ := GetFSChecker("/dev/sda1", "ext4", obs) + err := ch.Check(context.Background(), false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + want := []string{ + StartedFSCheckEvent, + FoundNoErrorsEvent, + } + assertEvents(t, obs.events, want) +} + +func TestEXTChecker_Check_Errors_NoRepair(t *testing.T) { + // e2fsck -nf -> rc=1 with non-nil err (errors found), doRepair=false + // err != nil required to bypass the err==nil->FoundNoErrors path + restore := setMockExec(func(_ context.Context, name string, args ...string) (int, error) { + if name == "e2fsck" && len(args) >= 1 && args[0] == "-nf" { + return 1, errors.New("exit status 1") + } + return -1, errors.New("unexpected call") + }) + defer restore() + + obs := &capturingObserver{} + ch, _ := GetFSChecker("/dev/sda1", "ext4", obs) + err := ch.Check(context.Background(), false) + if err == nil { + t.Fatalf("expected error, got nil") + } + + want := []string{ + StartedFSCheckEvent, + FoundErrorsEvent, + } + assertEvents(t, obs.events, want) +} + +func TestEXTChecker_Check_Errors_DoRepair_Success(t *testing.T) { + // Check first (rc=1, err non-nil), then repair (rc=0), then final check (rc=0) + // e2fsck -nf -> rc=1, err non-nil (errors found) + // e2fsck -p -> rc=0, err nil (repair succeeds) + // e2fsck -nf -> rc=0, err nil -> FinishedFSRepairEvent + callCount := 0 + restore := setMockExec(func(_ context.Context, name string, args ...string) (int, error) { + if name == "e2fsck" && len(args) >= 1 && args[0] == "-nf" { + callCount++ + if callCount == 1 { + // First check finds errors + return 1, errors.New("exit status 1") + } + // Second check after repair finds no errors + return 0, nil + } + if name == "e2fsck" && len(args) >= 1 && args[0] == "-p" { + return 0, nil + } + return -1, errors.New("unexpected call") + }) + defer restore() + + obs := &capturingObserver{} + ch, _ := GetFSChecker("/dev/sda1", "ext4", obs) + err := ch.Check(context.Background(), true) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + want := []string{ + StartedFSCheckEvent, + FoundErrorsEvent, + StartFSRepairEvent, + FinishedFSRepairEvent, + } + assertEvents(t, obs.events, want) +} + +func TestEXTChecker_Check_Errors_DoRepair_TimedOut(t *testing.T) { + // Check first (rc=1, err non-nil), then repair times out (rc=32) + // e2fsck -nf -> rc=1, err non-nil (errors found) + // e2fsck -p -> rc=32 (canceled by user); check never runs + callCount := 0 + restore := setMockExec(func(_ context.Context, name string, args ...string) (int, error) { + if name == "e2fsck" && len(args) >= 1 && args[0] == "-nf" { + callCount++ + if callCount == 1 { + // First check finds errors + return 1, errors.New("exit status 1") + } + } + if name == "e2fsck" && len(args) >= 1 && args[0] == "-p" { + return 32, errors.New("signal: killed") + } + return -1, errors.New("unexpected call") + }) + defer restore() + + obs := &capturingObserver{} + ch, _ := GetFSChecker("/dev/sda1", "ext4", obs) + err := ch.Check(context.Background(), true) + if err == nil { + t.Fatalf("expected error, got nil") + } + + want := []string{ + StartedFSCheckEvent, + FoundErrorsEvent, + StartFSRepairEvent, + FSRepairTimedOutEvent, + } + assertEvents(t, obs.events, want) +} + +func TestEXTChecker_Check_Errors_DoRepair_Failed(t *testing.T) { + // Check first (rc=1, err non-nil), then repair (rc=4, err nil), then final check still finds errors (rc=4) + // e2fsck -nf -> rc=1, err non-nil (errors found) + // e2fsck -p -> rc=4, err nil (repair fails silently) + // e2fsck -nf -> rc=4, err non-nil (final check finds errors) + callCount := 0 + restore := setMockExec(func(_ context.Context, name string, args ...string) (int, error) { + if name == "e2fsck" && len(args) >= 1 && args[0] == "-nf" { + callCount++ + if callCount == 1 { + // First check finds errors + return 1, errors.New("exit status 1") + } + // Second check after repair still finds errors + return 4, errors.New("exit status 4") + } + if name == "e2fsck" && len(args) >= 1 && args[0] == "-p" { + return 4, nil // repair fails silently, no error + } + return -1, errors.New("unexpected call") + }) + defer restore() + + obs := &capturingObserver{} + ch, _ := GetFSChecker("/dev/sda1", "ext4", obs) + err := ch.Check(context.Background(), true) + if err == nil { + t.Fatalf("expected error, got nil") + } + + want := []string{ + StartedFSCheckEvent, + FoundErrorsEvent, + StartFSRepairEvent, + FSRepairFailedEvent, + } + assertEvents(t, obs.events, want) +} + +func TestEXTChecker_Check_DoRepair_Success_CheckStillFindsErrors(t *testing.T) { + // Check first (rc=1, err non-nil), then repair succeeds (rc=0), but final check still finds errors (rc=1) + // e2fsck -nf -> rc=1, err non-nil (errors found) + // e2fsck -p -> rc=0, err nil (repair succeeds) + // e2fsck -nf -> rc=1, err non-nil (final check still finds errors) + callCount := 0 + restore := setMockExec(func(_ context.Context, name string, args ...string) (int, error) { + if name == "e2fsck" && len(args) >= 1 && args[0] == "-nf" { + callCount++ + if callCount == 1 { + // First check finds errors + return 1, errors.New("exit status 1") + } + // Second check after repair still finds errors + return 1, errors.New("exit status 1") + } + if name == "e2fsck" && len(args) >= 1 && args[0] == "-p" { + return 0, nil // repair succeeds + } + return -1, errors.New("unexpected call") + }) + defer restore() + + obs := &capturingObserver{} + ch, _ := GetFSChecker("/dev/sda1", "ext4", obs) + err := ch.Check(context.Background(), true) + if err == nil { + t.Fatalf("expected error, got nil") + } + + want := []string{ + StartedFSCheckEvent, + FoundErrorsEvent, + StartFSRepairEvent, + FSRepairFailedEvent, + } + assertEvents(t, obs.events, want) +} + +func TestEXTChecker_Check_TimedOut(t *testing.T) { + // e2fsck -nf -> rc=32, non-nil err (isCanceledByUser) + // err != nil bypasses the err==nil path; isCanceledByUser() -> FSCheckTimedOutEvent + restore := setMockExec(func(_ context.Context, name string, args ...string) (int, error) { + if name == "e2fsck" && len(args) >= 1 && args[0] == "-nf" { + return 32, errors.New("signal: killed") + } + return -1, errors.New("unexpected call") + }) + defer restore() + + obs := &capturingObserver{} + ch, _ := GetFSChecker("/dev/sda1", "ext4", obs) + err := ch.Check(context.Background(), false) + if err == nil { + t.Fatalf("expected error, got nil") + } + + want := []string{ + StartedFSCheckEvent, + FSCheckTimedOutEvent, + } + assertEvents(t, obs.events, want) +} + +func TestEXTChecker_Check_Failed_Generic(t *testing.T) { + // e2fsck -nf -> rc=8 and non-nil err -> generic failure path + restore := setMockExec(func(_ context.Context, name string, args ...string) (int, error) { + if name == "e2fsck" && len(args) >= 1 && args[0] == "-nf" { + return 8, errors.New("exit status 8") + } + return -1, errors.New("unexpected call") + }) + defer restore() + + obs := &capturingObserver{} + ch, _ := GetFSChecker("/dev/sda1", "ext4", obs) + err := ch.Check(context.Background(), false) + if err == nil { + t.Fatalf("expected error, got nil") + } + + want := []string{ + StartedFSCheckEvent, + FSCheckFailedEvent, + } + assertEvents(t, obs.events, want) +} + +func TestEXTChecker_Check_FinalPass_GenericFailure(t *testing.T) { + // Test the uncovered final error path: second pass with unexpected exit code + // First pass: rc=1 (errors found) -> repair -> second pass: rc=8 (unexpected) + callCount := 0 + restore := setMockExec(func(_ context.Context, name string, args ...string) (int, error) { + if name == "e2fsck" && len(args) >= 1 && args[0] == "-nf" { + callCount++ + if callCount == 1 { + // First pass finds errors to trigger repair + return 1, errors.New("exit status 1") + } + // Second pass returns unexpected exit code (not 1, 2, 4, or 32) + return 8, errors.New("exit status 8") + } + if name == "e2fsck" && len(args) >= 1 && args[0] == "-p" { + return 0, nil // repair succeeds + } + return -1, errors.New("unexpected call") + }) + defer restore() + + obs := &capturingObserver{} + ch, _ := GetFSChecker("/dev/sda1", "ext4", obs) + err := ch.Check(context.Background(), true) + if err == nil { + t.Fatalf("expected error, got nil") + } + + want := []string{ + StartedFSCheckEvent, + FoundErrorsEvent, + StartFSRepairEvent, + FSRepairFailedEvent, + } + assertEvents(t, obs.events, want) +} + +// ---- XFS: Check + Replay + Repair ------------------------------------------ + +func TestXFSChecker_Check_NoErrors(t *testing.T) { + // xfs_repair -n -> 0 + restore := setMockExec(func(_ context.Context, name string, args ...string) (int, error) { + if name == "xfs_repair" && len(args) == 2 && args[0] == "-n" { + return 0, nil + } + return -1, errors.New("unexpected call") + }) + defer restore() + + obs := &capturingObserver{} + ch, _ := GetFSChecker("/dev/sdb1", "xfs", obs) + err := ch.Check(context.Background(), false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + want := []string{ + StartedFSCheckEvent, + FoundNoErrorsEvent, + } + assertEvents(t, obs.events, want) +} + +func TestXFSChecker_Check_Repairable_NoRepair(t *testing.T) { + // xfs_repair -n -> 1 + restore := setMockExec(func(_ context.Context, name string, args ...string) (int, error) { + if name == "xfs_repair" && len(args) >= 1 && args[0] == "-n" { + return 1, nil + } + return -1, errors.New("unexpected call") + }) + defer restore() + + obs := &capturingObserver{} + ch, _ := GetFSChecker("/dev/sdb1", "xfs", obs) + err := ch.Check(context.Background(), false) + if err == nil { + t.Fatalf("expected error, got nil") + } + + want := []string{ + StartedFSCheckEvent, + FoundErrorsEvent, + } + assertEvents(t, obs.events, want) +} + +func TestXFSChecker_Check_Repairable_DoRepair_Success(t *testing.T) { + // -n -> 1, repair -> rc=0 + call := 0 + restore := setMockExec(func(_ context.Context, name string, args ...string) (int, error) { + switch name { + case "xfs_repair": + if len(args) >= 1 && args[0] == "-n" { + call++ + return 1, nil + } + // repair without -n + if len(args) == 1 { + call++ + return 0, nil + } + } + return -1, errors.New("unexpected call") + }) + defer restore() + + obs := &capturingObserver{} + ch, _ := GetFSChecker("/dev/sdb1", "xfs", obs) + err := ch.Check(context.Background(), true) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + want := []string{ + StartedFSCheckEvent, + FoundErrorsEvent, + StartFSRepairEvent, + FinishedFSRepairEvent, + } + assertEvents(t, obs.events, want) +} + +func TestXFSChecker_Check_Repairable_DoRepair_Failed(t *testing.T) { + // -n -> 1, repair -> rc!=0 or err!=nil + restore := setMockExec(func(_ context.Context, name string, args ...string) (int, error) { + if name == "xfs_repair" && len(args) >= 1 && args[0] == "-n" { + return 1, nil + } + if name == "xfs_repair" && len(args) == 1 { + return 1, errors.New("repair failed") + } + return -1, errors.New("unexpected call") + }) + defer restore() + + obs := &capturingObserver{} + ch, _ := GetFSChecker("/dev/sdb1", "xfs", obs) + err := ch.Check(context.Background(), true) + if err == nil { + t.Fatalf("expected error, got nil") + } + + want := []string{ + StartedFSCheckEvent, + FoundErrorsEvent, + StartFSRepairEvent, + FSRepairFailedEvent, + } + assertEvents(t, obs.events, want) +} + +func TestXFSChecker_Check_DirtyLog_Replay_Success_Then_NoErrors(t *testing.T) { + // first -n -> 2 (dirty log) + // mount -> 0, umount -> 0 + // second -n -> 0 + checkCount := 0 + restore := setMockExec(func(_ context.Context, name string, args ...string) (int, error) { + switch name { + case "xfs_repair": + if len(args) >= 1 && args[0] == "-n" { + checkCount++ + if checkCount == 1 { + return 2, nil + } + return 0, nil + } + case "mount": + return 0, nil + case "umount": + return 0, nil + } + return -1, errors.New("unexpected call") + }) + defer restore() + + obs := &capturingObserver{} + ch, _ := GetFSChecker("/dev/sdb1", "xfs", obs) + err := ch.Check(context.Background(), false) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + want := []string{ + StartedFSCheckEvent, + FoundDirtyLogEvent, + StartLogReplayEvent, + LogReplayDoneEvent, + FoundNoErrorsEvent, + } + assertEvents(t, obs.events, want) +} + +func TestXFSChecker_Check_DirtyLog_Replay_Success_Then_Repairable_DoRepair_Success(t *testing.T) { + // first -n -> 2 + // mount/umount -> 0 + // second -n -> 1 (repairable) + // repair -> 0 + checkCount := 0 + restore := setMockExec(func(_ context.Context, name string, args ...string) (int, error) { + switch name { + case "xfs_repair": + if len(args) >= 1 && args[0] == "-n" { + checkCount++ + if checkCount == 1 { + return 2, nil + } + return 1, nil + } + // actual repair + if len(args) == 1 { + return 0, nil + } + case "mount": + return 0, nil + case "umount": + return 0, nil + } + return -1, errors.New("unexpected call") + }) + defer restore() + + obs := &capturingObserver{} + ch, _ := GetFSChecker("/dev/sdb1", "xfs", obs) + err := ch.Check(context.Background(), true) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + want := []string{ + StartedFSCheckEvent, + FoundDirtyLogEvent, + StartLogReplayEvent, + LogReplayDoneEvent, + FoundErrorsEvent, + StartFSRepairEvent, + FinishedFSRepairEvent, + } + assertEvents(t, obs.events, want) +} + +func TestXFSChecker_Check_DirtyLog_Replay_Success_Then_StillDirty(t *testing.T) { + // first -n -> 2 + // mount/umount -> 0 + // second -n -> 2 again (still dirty) + checkCount := 0 + restore := setMockExec(func(_ context.Context, name string, args ...string) (int, error) { + switch name { + case "xfs_repair": + if len(args) >= 1 && args[0] == "-n" { + checkCount++ + return 2, nil + } + case "mount": + return 0, nil + case "umount": + return 0, nil + } + return -1, errors.New("unexpected call") + }) + defer restore() + + obs := &capturingObserver{} + ch, _ := GetFSChecker("/dev/sdb1", "xfs", obs) + err := ch.Check(context.Background(), false) + if err == nil { + t.Fatalf("expected error, got nil") + } + + // Note: On second dirty log, the implementation returns an error *without* + // emitting FSCheckFailedEvent. We assert exactly the emitted events. + want := []string{ + StartedFSCheckEvent, + FoundDirtyLogEvent, + StartLogReplayEvent, + LogReplayDoneEvent, + } + assertEvents(t, obs.events, want) +} + +func TestXFSChecker_Check_DirtyLog_Replay_MountFail(t *testing.T) { + // -n -> 2; mount fails + restore := setMockExec(func(_ context.Context, name string, args ...string) (int, error) { + switch name { + case "xfs_repair": + if len(args) >= 1 && args[0] == "-n" { + return 2, nil + } + case "mount": + return 1, errors.New("mount failed") + } + return -1, errors.New("unexpected call") + }) + defer restore() + + obs := &capturingObserver{} + ch, _ := GetFSChecker("/dev/sdb1", "xfs", obs) + err := ch.Check(context.Background(), false) + if err == nil { + t.Fatalf("expected error, got nil") + } + + want := []string{ + StartedFSCheckEvent, + FoundDirtyLogEvent, + StartLogReplayEvent, + LogReplayFailedEvent, + } + assertEvents(t, obs.events, want) +} + +func TestXFSChecker_Check_DirtyLog_Replay_UmountFail(t *testing.T) { + // -n -> 2; mount ok; umount fails + restore := setMockExec(func(_ context.Context, name string, args ...string) (int, error) { + switch name { + case "xfs_repair": + if len(args) >= 1 && args[0] == "-n" { + return 2, nil + } + case "mount": + return 0, nil + case "umount": + return 1, errors.New("umount failed") + } + return -1, errors.New("unexpected call") + }) + defer restore() + + obs := &capturingObserver{} + ch, _ := GetFSChecker("/dev/sdb1", "xfs", obs) + err := ch.Check(context.Background(), false) + if err == nil { + t.Fatalf("expected error, got nil") + } + + want := []string{ + StartedFSCheckEvent, + FoundDirtyLogEvent, + StartLogReplayEvent, + LogReplayFailedEvent, + } + assertEvents(t, obs.events, want) +} + +func TestXFSChecker_Check_Failed_Generic(t *testing.T) { + // -n -> rc=7, err non-nil => generic failed branch + restore := setMockExec(func(_ context.Context, name string, args ...string) (int, error) { + if name == "xfs_repair" && len(args) >= 1 && args[0] == "-n" { + return 7, errors.New("xfs_repair failed") + } + return -1, errors.New("unexpected call") + }) + defer restore() + + obs := &capturingObserver{} + ch, _ := GetFSChecker("/dev/sdb1", "xfs", obs) + err := ch.Check(context.Background(), false) + if err == nil { + t.Fatalf("expected error, got nil") + } + + want := []string{ + StartedFSCheckEvent, + FSCheckFailedEvent, + } + assertEvents(t, obs.events, want) +} + +// nil observer -> NoopFSCheckObserver is used; xfs_repair -n returns 0 +func TestXFSChecker_WithNilObserver(t *testing.T) { + checkCalled := false + restore := setMockExec(func(_ context.Context, name string, args ...string) (int, error) { + if name == "xfs_repair" && len(args) == 2 && args[0] == "-n" { + checkCalled = true + return 0, nil // happy path: no errors + } + return -1, errors.New("unexpected call") + }) + defer restore() + + // Pass observer=nil to force NoopFSCheckObserver + checker, err := GetFSChecker("/dev/sdb1", "xfs", nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if checker == nil { + t.Fatalf("expected non-nil checker") + } + + // Run check - this should internally emit events to NoopFSCheckObserver.OnEvent() + if err := checker.Check(context.Background(), false); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !checkCalled { + t.Fatalf("expected check to be called") + } +} + +// ---- Test ExecOSCommand --------------------------------------------------------- + +// 1) Success path: command exits with 0 +func TestExecFn_Success(t *testing.T) { + // Use /bin/sh -c "true" to guarantee rc=0 + rc, err := execOSCommand(context.Background(), "/bin/sh", "-c", "true") + if err != nil { + t.Fatalf("expected nil error, got %v", err) + } + if rc != 0 { + t.Fatalf("expected rc=0, got %d", rc) + } +} + +// 2) Non-zero exit code: verify we get rc from ExitError and err != nil +func TestExecFn_NonZeroExit(t *testing.T) { + // exit 7 ensures a specific rc is propagated + rc, err := execOSCommand(context.Background(), "/bin/sh", "-c", "exit 7") + if err == nil { + t.Fatalf("expected non-nil error, got nil") + } + if rc != 7 { + t.Fatalf("expected rc=7, got %d", rc) + } +} + +// 3) Command not found: rc should be -1 (since it's not an ExitError) and err != nil +func TestExecFn_CommandNotFound(t *testing.T) { + rc, err := execOSCommand(context.Background(), "this-command-should-not-exist-xyz") + if err == nil { + t.Fatalf("expected error for non-existent command, got nil") + } + if rc != -1 { + t.Fatalf("expected rc=-1 for non-ExitError cases, got %d", rc) + } +} + +// 4) Non-zero exit code with stderr output: verify we get stderr from the command +func TestExecFn_ExitWithStderr(t *testing.T) { + // command writes to stderr and exits with code 7 + rc, err := execOSCommand(context.Background(), "/bin/sh", + "-c", "echo command-error-message >&2; exit 7") + //"-c", "for i in $(seq 15); do echo error-line-$i >&2; done; exit 7")) + if err == nil { + t.Fatalf("expected non-nil error, got nil") + } + if rc != 7 { + t.Fatalf("expected rc=7, got %d", rc) + } +} + +// 5) Deterministic cancellation after confirming the command started. +// The command (helper process) installs a SIGINT trap that exits with rc=32, +// then creates a "ready" dir and sleeps. We wait for the ready dir, +// then cancel the context and expect rc=32 with non-nil error. +func TestExecFn_CancelAfterStarted_TrapSIGINT_RC32(t *testing.T) { + // Prepare a temp path for readiness signal. + tmpDir := t.TempDir() + readyDir := filepath.Join(tmpDir, "ready") + + // The helper process: + // - create the ready file to signal it's actually started + // - monitors INT signal + // - exits 0 upon completion or 32 upon SIGINT receival + + // Context we will cancel after we see the ready dir created. + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + type result struct { + rc int + err error + } + done := make(chan result, 1) + + // Run execOSCommand in a goroutine so we can wait for readiness. + go func() { + rc, err := execOSCommand(ctx, os.Args[0], "-helperProc=true", "-helperReadyDir="+readyDir) + done <- result{rc: rc, err: err} + }() + + // Wait deterministically until the helper created the dir (or time out the test). + waitCtx, stopWait := context.WithTimeout(context.Background(), 15*time.Second) + defer stopWait() + + // Poll for the ready dir existence. + for { + select { + case <-waitCtx.Done(): + t.Fatalf("timed out waiting for helper readiness dir %s", readyDir) + default: + if _, err := os.Stat(readyDir); err == nil { + goto READY + } + time.Sleep(300 * time.Millisecond) + } + } + +READY: + t.Logf("The process has started, interrupting it by cancelling ctx") + cancel() + + // Wait for the command to exit and verify rc and error. + select { + case r := <-done: + if r.err == nil { + t.Fatalf("expected non-nil error after context cancel, got nil (rc=%d)", r.rc) + } + if r.rc != 32 { + t.Fatalf("expected rc=32 from SIGINT trap, got %d (err=%v)", r.rc, r.err) + } + // Context should be canceled + if ctx.Err() == nil { + t.Fatalf("expected ctx.Err() to be non-nil (context canceled), got nil") + } + case <-time.After(15 * time.Second): + t.Fatalf("command did not exit in time after cancellation") + } +} + +func TestXFSChecker_Check_TimedOut(t *testing.T) { + // Simulate xfs_repair -n invocation executing a helper process that does NOT trap SIGINT. + // When we cancel the context, execOSCommand sends SIGINT to the process group, the helper + // dies due to the signal, and isProcKilled(err) must evaluate to true, emitting FSCheckTimedOutEvent. + tmpDir := t.TempDir() + readyDir := filepath.Join(tmpDir, "ready") + + restore := setMockExec(func(ctx context.Context, name string, args ...string) (int, error) { + if name == "xfs_repair" && len(args) == 2 && args[0] == "-n" { + // Run the no-trap helper to be killed by signal. + return execOSCommand(ctx, os.Args[0], "-helperProc=true", "-helperNoTrap=true", "-helperReadyDir="+readyDir) + } + return -1, errors.New("unexpected call") + }) + defer restore() + + // Create cancelable context; we'll cancel once helper signals readiness. + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + type result struct { + err error + } + done := make(chan result, 1) + + obs := &capturingObserver{} + ch, _ := GetFSChecker("/dev/sdb1", "xfs", obs) + + go func() { + done <- result{err: ch.Check(ctx, false)} + }() + + // Wait deterministically for helper readiness, then cancel. + waitCtx, stopWait := context.WithTimeout(context.Background(), 15*time.Second) + defer stopWait() + for { + select { + case <-waitCtx.Done(): + t.Fatalf("timed out waiting for helper readiness dir %s", readyDir) + default: + if _, err := os.Stat(readyDir); err == nil { + cancel() + goto CANCELED_CHECK + } + time.Sleep(200 * time.Millisecond) + } + } + +CANCELED_CHECK: + + select { + case r := <-done: + if r.err == nil { + t.Fatalf("expected error after timeout, got nil") + } + case <-time.After(15 * time.Second): + t.Fatalf("xfs check did not exit in time after cancellation") + } + + want := []string{ + StartedFSCheckEvent, + FSCheckTimedOutEvent, + } + assertEvents(t, obs.events, want) +} + +func TestXFSChecker_Repair_TimedOut(t *testing.T) { + // First phase: xfs_repair -n should report repairable (rc=1). + // Second phase: actual repair should run helper without SIGINT trap so that + // canceling the context results in a signal-terminated process, triggering + // FSRepairTimedOutEvent via isProcKilled(err). + tmpDir := t.TempDir() + readyDir := filepath.Join(tmpDir, "ready-repair") + + // We need to distinguish between the -n check and the repair call. + restore := setMockExec(func(ctx context.Context, name string, args ...string) (int, error) { + if name == "xfs_repair" { + // Check call + if len(args) >= 1 && args[0] == "-n" { + return 1, nil // repairable + } + // Repair call (no -n): run helper-no-trap to be killed by signal + if len(args) == 1 { + return execOSCommand(ctx, os.Args[0], "-helperProc=true", "-helperNoTrap=true", "-helperReadyDir="+readyDir) + } + } + return -1, errors.New("unexpected call") + }) + defer restore() + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + type result struct { + err error + } + done := make(chan result, 1) + + obs := &capturingObserver{} + ch, _ := GetFSChecker("/dev/sdb1", "xfs", obs) + + go func() { + done <- result{err: ch.Check(ctx, true)} + }() + + // Wait for the repair helper readiness, then cancel. + waitCtx, stopWait := context.WithTimeout(context.Background(), 15*time.Second) + defer stopWait() + for { + select { + case <-waitCtx.Done(): + t.Fatalf("timed out waiting for helper readiness dir %s", readyDir) + default: + if _, err := os.Stat(readyDir); err == nil { + cancel() + goto CANCELED_REPAIR + } + time.Sleep(200 * time.Millisecond) + } + } + +CANCELED_REPAIR: + + select { + case r := <-done: + if r.err == nil { + t.Fatalf("expected error after repair timeout, got nil") + } + case <-time.After(10 * time.Second): + t.Fatalf("xfs repair did not exit in time after cancellation") + } + + want := []string{ + StartedFSCheckEvent, + FoundErrorsEvent, + StartFSRepairEvent, + FSRepairTimedOutEvent, + } + assertEvents(t, obs.events, want) +} + +// Helper to set FSCheckLinesToLog for a test and restore afterwards. +// Requires FSCheckLinesToLog to be a var, not const. +func withFSN(t *testing.T, n int, fn func()) { + t.Helper() + orig := FSCheckLinesToLog + FSCheckLinesToLog = n + defer func() { FSCheckLinesToLog = orig }() + fn() +} + +func TestCollectStderr_EmptyBuffer(t *testing.T) { + withFSN(t, 3, func() { + var errBuf bytes.Buffer // empty + + got := truncOutput(errBuf, "cmd") + if got == nil { + t.Fatalf("expected non-nil buffer") + } + if got.Len() != 0 { + t.Fatalf("expected empty buffer, got %q", got.String()) + } + }) +} + +func TestTruncOutput_WhitespaceOnly(t *testing.T) { + withFSN(t, 3, func() { + var errBuf bytes.Buffer + errBuf.WriteString(" \n\t\n \n") + + got := truncOutput(errBuf, "cmd", "-v") + if got.Len() != 0 { + t.Fatalf("expected empty buffer for whitespace-only input, got %q", got.String()) + } + }) +} + +func TestTruncOutput_FewerThanN(t *testing.T) { + withFSN(t, 3, func() { + var errBuf bytes.Buffer + // Leading spaces preserved, trailing spaces removed on non-empty lines. + errBuf.WriteString(" line1 \n\n\tline2\t\t\n") + + got := truncOutput(errBuf, "cmd", "a", "b") + + want := strings.Join([]string{ + "stderr from command: cmd a b", + " line1", + "\tline2", + }, "\n") + + if got.String() != want { + t.Fatalf("unexpected output\n--- got ---\n%q\n--- want ---\n%q", got.String(), want) + } + }) +} + +func TestTruncOutput_ExactlyN(t *testing.T) { + withFSN(t, 3, func() { + var errBuf bytes.Buffer + // Exactly 3 non-empty (after trim-space emptiness check). + errBuf.WriteString("one \n two\t \n\tthree\n") + + got := truncOutput(errBuf, "tool") + + want := strings.Join([]string{ + "stderr from command: tool", + "one", // trailing space trimmed + " two", // leading spaces preserved, trailing trimmed + "\tthree", + }, "\n") + + if got.String() != want { + t.Fatalf("unexpected output\n--- got ---\n%q\n--- want ---\n%q", got.String(), want) + } + }) +} + +func TestTruncOutput_MoreThanN(t *testing.T) { + withFSN(t, 3, func() { + var errBuf bytes.Buffer + // Non-empty trimmed lines (show leading variations): a, " b", "\tc", "d", " e " + // Also include blanks to ensure they are skipped. + errBuf.WriteString(" \n a \n b\t \n\tc\t\t\n\n d\n e \n") + + got := truncOutput(errBuf, "tool", "--flag") + + // Expect ellipsis and last 3 non-empty: "\tc" (leading tab, trailing trimmed), + // "d" (no change), " e" (leading preserved, trailing trimmed). + want := strings.Join([]string{ + "stderr from command: tool --flag", + "...", + "\tc", + " d", + " e", + }, "\n") + + if got.String() != want { + t.Fatalf("unexpected output for >N with ellipsis\n--- got ---\n%q\n--- want ---\n%q", got.String(), want) + } + }) +} + +// Test EXT checker with real filesystem images. +// The images are sample images found in the e2fsprogs test suite . + +func TestEXTChecker_WithRealImages(t *testing.T) { + // Check if e2fsck and gunzip are available in PATH + if _, err := exec.LookPath("e2fsck"); err != nil { + t.Skip("e2fsck binary not found in PATH, skipping test") + } + if _, err := exec.LookPath("gunzip"); err != nil { + t.Skip("gunzip binary not found in PATH, skipping test") + } + if !canSetupLoopback() { + t.Skip("insufficient permissions to setup loopback devices, skipping test") + } + + // Find all img_*.gz files in tests/fsckdata + fsckdataDir := filepath.Join("tests", "fsckdata") + entries, err := os.ReadDir(fsckdataDir) + if err != nil { + t.Fatalf("failed to read fsckdata directory: %v", err) + } + + // Process each archived image + for _, entry := range entries { + if entry.IsDir() || !strings.HasSuffix(entry.Name(), ".gz") { + continue + } + + t.Run(entry.Name(), func(t *testing.T) { + testEXTCheckerWithImage(t, fsckdataDir, entry.Name()) + }) + } +} + +func testEXTCheckerWithImage(t *testing.T, fsckdataDir, gzFile string) { + t.Helper() + + // Extract expected exit code from filename + // Pattern: img_[number]_.... + var expectedCode int + n, err := fmt.Sscanf(gzFile, "img_%d_", &expectedCode) + if err != nil || n != 1 { + t.Fatalf("invalid filename format: %s", gzFile) + } + + // Create temporary directory for this test + tempDir := t.TempDir() + imagePath := filepath.Join(fsckdataDir, gzFile) + unpackedPath := filepath.Join(tempDir, "image") + + // Unpack the gz file + t.Logf("Unpacking %s to %s", imagePath, unpackedPath) + if err := unpackGz(imagePath, unpackedPath); err != nil { + t.Fatalf("failed to unpack %s: %v", gzFile, err) + } + + // Setup loopback device + loopDev, err := setupLoopback(unpackedPath) + if err != nil { + t.Fatalf("failed to setup loopback device: %v", err) + } + defer func() { + if err := detachLoopback(loopDev); err != nil { + t.Logf("warning: failed to detach loopback %s: %v", loopDev, err) + } + }() + + // Create ext checker and run in check-and-repair mode + obs := &capturingObserver{} + checker, err := GetFSChecker(loopDev, "ext", obs) + if err != nil { + t.Fatalf("failed to get fs checker: %v", err) + } + + // Run check with repair enabled + err = checker.Check(context.Background(), true) + + // Verify expectations based on expected code + if expectedCode == 0 { + // Should succeed + if err != nil { + t.Fatalf("expected check to succeed for %s, got error: %v", gzFile, err) + } + t.Logf("✓ %s: check succeeded as expected", gzFile) + } else { + // Should fail with error containing the exit code + if err == nil { + t.Fatalf("expected check to fail for %s, but it succeeded", gzFile) + } + + // Check if error message contains the expected exit code + expectedErrPattern := fmt.Sprintf("\\(%d\\)", expectedCode) + matched, regexErr := regexp.MatchString(expectedErrPattern, err.Error()) + if regexErr != nil { + t.Fatalf("failed to match error pattern: %v", regexErr) + } + if !matched { + t.Fatalf("error message should contain (%d), got: %s", expectedCode, err.Error()) + } + t.Logf("✓ %s: check failed with expected error code %d", gzFile, expectedCode) + } +} + +// unpackGz unpacks a gz file to the target path +func unpackGz(gzPath, targetPath string) error { + gzFile, err := os.Open(gzPath) + if err != nil { + return fmt.Errorf("failed to open gz file: %w", err) + } + defer gzFile.Close() + + targetFile, err := os.Create(targetPath) + if err != nil { + return fmt.Errorf("failed to create target file: %w", err) + } + defer targetFile.Close() + + // Use gunzip command to unpack + cmd := exec.Command("gunzip", "-c", gzPath) + cmd.Stdout = targetFile + if err := cmd.Run(); err != nil { + return fmt.Errorf("failed to unpack gz file: %w", err) + } + + return nil +} + +// canSetupLoopback checks if user has permissions to setup loopback devices +func canSetupLoopback() bool { + // Try to find a free loop device + cmd := exec.Command("losetup", "-f") + output, err := cmd.CombinedOutput() + if err != nil { + return false + } + + // Check if we got a valid loop device path + loopDevice := strings.TrimSpace(string(output)) + if !strings.HasPrefix(loopDevice, "/dev/loop") { + return false + } + + // Test if we can actually access the device + if _, err := os.Stat(loopDevice); err != nil { + return false + } + + return true +} + +// setupLoopback creates a loopback device for the given image file +func setupLoopback(imagePath string) (string, error) { + // Use losetup to create loopback device + cmd := exec.Command("losetup", "-f", "--show", imagePath) + output, err := cmd.Output() + if err != nil { + return "", fmt.Errorf("failed to setup loopback: %w", err) + } + + loopDev := strings.TrimSpace(string(output)) + if loopDev == "" { + return "", errors.New("empty loopback device path") + } + + return loopDev, nil +} + +// detachLoopback detaches a loopback device +func detachLoopback(loopDev string) error { + cmd := exec.Command("losetup", "-d", loopDev) + return cmd.Run() +} diff --git a/gofsutil_mock.go b/gofsutil_mock.go index 4766506..2dac90d 100644 --- a/gofsutil_mock.go +++ b/gofsutil_mock.go @@ -45,6 +45,13 @@ var ( // GONVMEValidDevices mocks existing devices GONVMEValidDevices map[string]bool + // GOFSMockFstrimResult is returned by Fstrim in mock mode when set. + GOFSMockFstrimResult *FstrimResult + // GOFSMockBlkdiscardResult is returned by Blkdiscard in mock mode when set. + GOFSMockBlkdiscardResult *BlkdiscardResult + // GOFSMockDiscardCapability is returned by CheckDiscardSupport in mock mode when set. + GOFSMockDiscardCapability *DiscardCapability + // GOFSMock allows you to induce errors in the various routine. GOFSMock struct { InduceBindMountError bool @@ -71,6 +78,9 @@ var ( InduceGetMpathNameFromDeviceError bool InduceFilesystemInfoError bool InduceGetNVMeControllerError bool + InduceFstrimError bool + InduceBlkdiscardError bool + InduceCheckDiscardSupportError bool } ) @@ -569,3 +579,60 @@ func (fs *mockfs) getNVMeController(device string) (string, error) { } return "", fmt.Errorf("controller not found for device %s", device) } + +// ==================================================================== +// Space reclamation mock implementations + +func (fs *mockfs) fstrim(_ context.Context, _ string) (*FstrimResult, error) { + if GOFSMock.InduceFstrimError { + return nil, errors.New("fstrim induced error") + } + if GOFSMockFstrimResult != nil { + return GOFSMockFstrimResult, nil + } + return &FstrimResult{ + BytesTrimmed: 1073741824, // 1 GiB + Duration: 500 * time.Millisecond, + }, nil +} + +// Fstrim delegates to fs.fstrim following the gofsutil mock pattern. +func (fs *mockfs) Fstrim(ctx context.Context, mountPoint string) (*FstrimResult, error) { + return fs.fstrim(ctx, mountPoint) +} + +func (fs *mockfs) blkdiscard(_ context.Context, _ string) (*BlkdiscardResult, error) { + if GOFSMock.InduceBlkdiscardError { + return nil, errors.New("blkdiscard induced error") + } + if GOFSMockBlkdiscardResult != nil { + return GOFSMockBlkdiscardResult, nil + } + return &BlkdiscardResult{ + BytesDiscarded: 107374182400, // 100 GiB + Duration: 2 * time.Second, + }, nil +} + +// Blkdiscard delegates to fs.blkdiscard following the gofsutil mock pattern. +func (fs *mockfs) Blkdiscard(ctx context.Context, devicePath string) (*BlkdiscardResult, error) { + return fs.blkdiscard(ctx, devicePath) +} + +func (fs *mockfs) checkDiscardSupport(_ context.Context, _ string) (*DiscardCapability, error) { + if GOFSMock.InduceCheckDiscardSupportError { + return nil, errors.New("checkDiscardSupport induced error") + } + if GOFSMockDiscardCapability != nil { + return GOFSMockDiscardCapability, nil + } + return &DiscardCapability{ + Supported: true, + DiscardMaxBytes: 4294967295, + }, nil +} + +// CheckDiscardSupport delegates to fs.checkDiscardSupport following the gofsutil mock pattern. +func (fs *mockfs) CheckDiscardSupport(ctx context.Context, devicePath string) (*DiscardCapability, error) { + return fs.checkDiscardSupport(ctx, devicePath) +} diff --git a/gofsutil_mount_linux.go b/gofsutil_mount_linux.go index 41446e2..f42e2c5 100644 --- a/gofsutil_mount_linux.go +++ b/gofsutil_mount_linux.go @@ -43,17 +43,57 @@ var ( ) // getDiskFormat uses 'lsblk' to see if the given disk is unformatted +// For SDC devices (/dev/scini*), it uses 'blkid' instead as lsblk doesn't work with character devices func (fs *FS) getDiskFormat(_ context.Context, disk string) (string, error) { path := filepath.Clean(disk) if err := validatePath(path); err != nil { return "", err } - args := []string{"-n", "-o", "FSTYPE", disk} - f := log.Fields{ "disk": disk, } + + // Check if this is an SDC device (character device /dev/scini*) + // SDC devices are character devices, not block devices, so lsblk doesn't work + if strings.HasPrefix(disk, "/dev/scini") { + log.WithFields(f).Info("checking if SDC disk is formatted using blkid") + buf, err := getExecCommandCombinedOutput("blkid", "-o", "value", "-s", "TYPE", disk) + out := strings.TrimSpace(string(buf)) + log.WithField("output", out).Debug("blkid output") + + if err != nil { + // blkid returns exit code 2 when no filesystem is found (unformatted) + // This is expected for unformatted devices + var exitCode int + if exitError, ok := err.(*exec.ExitError); ok { + exitCode = exitError.ExitCode() + } + + if exitCode == 2 || out == "" { + // Exit code 2 or empty output means unformatted device + log.WithFields(f).WithField("exitCode", exitCode).Debug("blkid indicates unformatted SDC device") + return "", nil + } + + // Other errors are actual failures + log.WithFields(f).WithField("exitCode", exitCode).WithError(err).Error("blkid failed for SDC device") + return "", err + } + + if out != "" { + // The device is formatted + log.WithFields(f).WithField("fstype", out).Info("SDC device has filesystem") + return out, nil + } + + // Empty output means unformatted + return "", nil + } + + // For non-SDC devices, use lsblk as before + args := []string{"-n", "-o", "FSTYPE", disk} + log.WithFields(f).WithField("args", args).Info( "checking if disk is formatted using lsblk") buf, err := getExecCommandCombinedOutput("lsblk", args...) @@ -76,7 +116,25 @@ func (fs *FS) getDiskFormat(_ context.Context, disk string) (string, error) { } if len(lines) == 1 { - // The device is unformatted and has no dependent devices + // lsblk returned empty for a single device. This could mean the device + // is truly unformatted, OR lsblk could not read the FS type (e.g. + // device-mapper symlinks inside containers where sysfs is incomplete). + // Fall back to blkid which reads the superblock directly. + blkidArgs := []string{"-o", "value", "-s", "TYPE", disk} + log.WithFields(f).WithField("args", blkidArgs).Info( + "lsblk returned empty; probing with blkid as fallback") + blkBuf, blkErr := getExecCommandCombinedOutput("blkid", blkidArgs...) + blkOut := strings.TrimSpace(string(blkBuf)) + log.WithField("output", blkOut).Debug("blkid fallback output") + + if blkErr == nil && blkOut != "" { + // blkid found a filesystem that lsblk missed + log.WithFields(f).WithField("fstype", blkOut).Info( + "blkid fallback detected filesystem") + return blkOut, nil + } + + // blkid also returned empty or errored — device is truly unformatted return "", nil } @@ -295,8 +353,8 @@ func (fs *FS) bindMount( // isLsblkNew returns true if lsblk version is greater than 2.3 and false otherwise func (fs *FS) isLsblkNew() (bool, error) { lsblkNew := false - checkVersCmd := "lsblk -V" - bufcheck, errcheck := exec.Command("bash", "-c", checkVersCmd).Output() + // Use exec.Command with argv-style arguments instead of bash -c to avoid shell injection + bufcheck, errcheck := exec.Command("lsblk", "-V").Output() if errcheck != nil { return lsblkNew, errcheck } @@ -326,24 +384,49 @@ func (fs *FS) getMpathNameFromDevice( return "", err } - var cmd string + // Validate device to prevent OS command injection (CWE-78) + if err := validateDeviceID(device); err != nil { + return "", fmt.Errorf("invalid device for getMpathNameFromDevice: %w", err) + } + + var args []string lsblkNew, err := fs.isLsblkNew() if err != nil { return "", err } if lsblkNew { - cmd = "lsblk -Px MODE | awk '/" + device + "/{c=2}c&&c--' | grep TYPE=\\\"mpath\\\"" + args = []string{"-Px", "MODE"} } else { - cmd = "lsblk -P | awk '/" + device + "/{c=2}c&&c--' | grep TYPE=\\\"mpath\\\"" + args = []string{"-P"} } - fmt.Println(cmd) - buf, _ := exec.Command("bash", "-c", cmd).Output() // #nosec G204 + // Use exec.Command with argv-style arguments instead of bash -c + buf, err := exec.Command("lsblk", args...).Output() + if err != nil { + return "", nil // lsblk may fail if device doesn't exist, return empty + } output := string(buf) - mpathDeviceRegx := regexp.MustCompile(`NAME="\S+"`) - mpath := mpathDeviceRegx.FindString(output) - if mpath != "" { - return strings.Split(mpath, "\"")[1], nil + + // Filter output in Go instead of using awk/grep in shell + mpathDeviceRegx := regexp.MustCompile(`NAME="(\S+)"`) + lines := strings.Split(output, "\n") + foundDevice := false + linesToCheck := 0 + + for _, line := range lines { + if strings.Contains(line, device) { + foundDevice = true + linesToCheck = 2 + } + if foundDevice && linesToCheck > 0 { + if strings.Contains(line, `TYPE="mpath"`) { + match := mpathDeviceRegx.FindStringSubmatch(line) + if len(match) > 1 { + return match[1], nil + } + } + linesToCheck-- + } } return "", nil @@ -403,56 +486,101 @@ func (fs *FS) getMountInfoFromDevice( return nil, err } - var cmd string + // Validate devID to prevent OS command injection (CWE-78) + // This is critical as devID is used in pattern matching against lsblk output + if err := validateDeviceID(devID); err != nil { + return nil, fmt.Errorf("invalid device ID for getMountInfoFromDevice: %w", err) + } + var output string lsblkNew, err := fs.isLsblkNew() if err != nil { return nil, err } - //check if devID has powerpath devices - /* #nosec G204 */ - checkCmd := "lsblk --pairs --output NAME,MAJ:MIN,RM,SIZE,RO,TYPE,MOUNTPOINT | awk '/emcpower.+" + devID + "/ {print $0}'" - log.Debugf("ppath checkcommand values is %s", checkCmd) - /* #nosec G204 */ - buf, err := exec.Command("bash", "-c", checkCmd).Output() + // check if devID has powerpath devices + // Use exec.Command with argv-style arguments instead of bash -c to avoid shell injection + // Get lsblk output and filter in Go instead of using awk + buf, err := exec.Command("lsblk", "--pairs", "--output", "NAME,MAJ:MIN,RM,SIZE,RO,TYPE,MOUNTPOINT").Output() if err != nil { return nil, err } - output = string(buf) + lsblkOutput := string(buf) + lines := strings.Split(lsblkOutput, "\n") + + // Filter for powerpath devices in Go instead of awk - use regexp.QuoteMeta for safe pattern matching + ppathRegex := regexp.MustCompile(`emcpower.+` + regexp.QuoteMeta(devID)) + var ppathMatches []string + for _, line := range lines { + if ppathRegex.MatchString(line) { + ppathMatches = append(ppathMatches, line) + } + } + output = strings.Join(ppathMatches, "\n") if output == "" { // output is nil, powerpath device not found, continuing for multipath or single device log.Info("powerpath command output is nil, continuing for multipath or single device") - /* #nosec G204 */ - checkCmd = "lsblk --pairs --output NAME,MAJ:MIN,RM,SIZE,RO,TYPE,MOUNTPOINT | awk '/mpath.+" + devID + "/ {print $0}'" - log.Debugf("mpath checkcommand values is %s", checkCmd) - /* #nosec G204 */ - buf, err = exec.Command("bash", "-c", checkCmd).Output() - if err != nil { - return nil, err + // Filter for multipath devices in Go instead of awk + mpathRegex := regexp.MustCompile(`mpath.+` + regexp.QuoteMeta(devID)) + var mpathMatches []string + for _, line := range lines { + if mpathRegex.MatchString(line) { + mpathMatches = append(mpathMatches, line) + } } - output = string(buf) + output = strings.Join(mpathMatches, "\n") log.Debugf("multipath exec command output is : %+v", output) + var lsblkArgs []string if output != "" { + log.Info("Multipath device found") if lsblkNew { - cmd = "lsblk --pairs --sort MODE --output NAME,MAJ:MIN,RM,SIZE,RO,TYPE,MOUNTPOINT | awk '/" + devID + "/{if (a && a !~ /" + devID + "/) print a; print} {a=$0}'" + lsblkArgs = []string{"--pairs", "--sort", "MODE", "--output", "NAME,MAJ:MIN,RM,SIZE,RO,TYPE,MOUNTPOINT"} } else { - cmd = "lsblk --pairs --output NAME,MAJ:MIN,RM,SIZE,RO,TYPE,MOUNTPOINT | awk '/" + devID + "/{if (a && a !~ /" + devID + "/) print a; print} {a=$0}'" + lsblkArgs = []string{"--pairs", "--output", "NAME,MAJ:MIN,RM,SIZE,RO,TYPE,MOUNTPOINT"} } } else { + log.Info("Multipath device not found") // multipath device not found, continue as single device - /* #nosec G204 */ - cmd = "lsblk --pairs --output NAME,MAJ:MIN,RM,SIZE,RO,TYPE,MOUNTPOINT | awk '/" + devID + "/ {print $0}'" + lsblkArgs = []string{"--pairs", "--output", "NAME,MAJ:MIN,RM,SIZE,RO,TYPE,MOUNTPOINT"} } - log.Debugf("command value is %s", cmd) - /* #nosec G204 */ - buf, err = exec.Command("bash", "-c", cmd).Output() + + // Execute lsblk with argv-style arguments instead of bash -c + buf, err = exec.Command("lsblk", lsblkArgs...).Output() if err != nil { return nil, err } - output = string(buf) + lsblkOutput = string(buf) + lines = strings.Split(lsblkOutput, "\n") + + // Filter output in Go instead of awk - use regexp.QuoteMeta for safe pattern matching + devRegex := regexp.MustCompile(regexp.QuoteMeta(devID)) + if output != "" { + // Multipath device found - replicate awk: '/{devID}/{if (a && a !~ /{devID}/) print a; print} {a=$0}' + var matchedLines []string + var prevLine string + for _, line := range lines { + if devRegex.MatchString(line) { + if prevLine != "" && !devRegex.MatchString(prevLine) { + matchedLines = append(matchedLines, prevLine) + } + matchedLines = append(matchedLines, line) + } + prevLine = line + } + output = strings.Join(matchedLines, "\n") + } else { + // Single device - replicate awk: '/{devID}/ {print $0}' + var matchedLines []string + for _, line := range lines { + if devRegex.MatchString(line) { + matchedLines = append(matchedLines, line) + } + } + output = strings.Join(matchedLines, "\n") + } log.Debugf("command output is : %+v", output) } + if output == "" { return nil, fmt.Errorf("Device not found") } @@ -512,13 +640,12 @@ func (fs *FS) findFSType( return "", fmt.Errorf("Failed to validate path: %s error %v", mountpoint, err) } - cmd := "findmnt -n \"" + path + "\" | awk '{print $3}'" - /* #nosec G204 */ - buf, err := exec.Command("bash", "-c", cmd).Output() + // Use exec.Command with argv-style arguments instead of bash -c to avoid shell injection + buf, err := exec.Command("findmnt", "-n", "-o", "FSTYPE", path).Output() if err != nil { return "", fmt.Errorf("Failed to find mount information for (%s) error (%v)", mountpoint, err) } - fsType = strings.TrimSuffix(string(buf), "\n") + fsType = strings.TrimSpace(string(buf)) return fsType, err } @@ -630,11 +757,10 @@ func (fs *FS) deviceRescan(_ context.Context, return err } device := path + "/device/rescan" - args := []string{"-c", "echo 1 > " + device} log.Infof("Executing rescan command on device (%s)", devicePath) - /* #nosec G204 */ - buf, err := exec.Command("bash", args...).CombinedOutput() - out := string(buf) + // Write directly to sysfs instead of executing bash -c "echo 1 > ...". + out := "1\n" + err := os.WriteFile(device, []byte(out), 0o644) log.WithField("output", out).Debug("Rescan output") if err != nil { log.Errorf("Failed to rescan device with error (%s)", err.Error()) diff --git a/gofsutil_mount_linux_test.go b/gofsutil_mount_linux_test.go index 056cead..edc7db2 100644 --- a/gofsutil_mount_linux_test.go +++ b/gofsutil_mount_linux_test.go @@ -85,6 +85,214 @@ func TestGetDiskFormatUnknownData(t *testing.T) { } } +func TestGetDiskFormatSDCWithFilesystem(t *testing.T) { + // Create a test FS + fs := &FS{} + + // Create an SDC disk path + disk := "/dev/scinia" + + // Mock the output + defaultGetExecCommandCombinedOutput := getExecCommandCombinedOutput + defer func() { + getExecCommandCombinedOutput = defaultGetExecCommandCombinedOutput + }() + + getExecCommandCombinedOutput = func(name string, args ...string) ([]byte, error) { + // Verify blkid is called with correct arguments + if name != "blkid" { + t.Errorf("expected blkid command, got %s", name) + } + expectedArgs := []string{"-o", "value", "-s", "TYPE", disk} + if len(args) != len(expectedArgs) { + t.Errorf("expected %d arguments, got %d", len(expectedArgs), len(args)) + } + for i, arg := range expectedArgs { + if args[i] != arg { + t.Errorf("expected arg[%d] = %s, got %s", i, arg, args[i]) + } + } + return []byte("xfs"), nil + } + + // Call getDiskFormat + fstype, err := fs.getDiskFormat(context.Background(), disk) + if err != nil { + t.Errorf("expected no error, got %v", err) + } + if fstype != "xfs" { + t.Errorf("expected xfs, got %s", fstype) + } +} + +func TestGetDiskFormatSDCUnformatted(t *testing.T) { + // Create a test FS + fs := &FS{} + + // Create an SDC disk path + disk := "/dev/scinia" + + // Mock the output + defaultGetExecCommandCombinedOutput := getExecCommandCombinedOutput + defer func() { + getExecCommandCombinedOutput = defaultGetExecCommandCombinedOutput + }() + + getExecCommandCombinedOutput = func(name string, _ ...string) ([]byte, error) { + // Verify blkid is called + if name != "blkid" { + t.Errorf("expected blkid command, got %s", name) + } + // Simulate blkid with empty output (unformatted device) + return []byte(""), nil + } + + // Call getDiskFormat + fstype, err := fs.getDiskFormat(context.Background(), disk) + if err != nil { + t.Errorf("expected no error for unformatted SDC device, got %v", err) + } + if fstype != "" { + t.Errorf("expected empty fstype for unformatted device, got %s", fstype) + } +} + +func TestGetDiskFormatSDCNonSCDDevice(t *testing.T) { + // Create a test FS + fs := &FS{} + + // Create a non-SDC disk path (should use lsblk) + disk := "/dev/sda1" + + // Mock the output + defaultGetExecCommandCombinedOutput := getExecCommandCombinedOutput + defer func() { + getExecCommandCombinedOutput = defaultGetExecCommandCombinedOutput + }() + + getExecCommandCombinedOutput = func(name string, _ ...string) ([]byte, error) { + // Verify lsblk is called for non-SDC devices + if name != "lsblk" { + t.Errorf("expected lsblk command for non-SDC device, got %s", name) + } + return []byte("ext4"), nil + } + + // Call getDiskFormat + fstype, err := fs.getDiskFormat(context.Background(), disk) + if err != nil { + t.Errorf("expected no error, got %v", err) + } + if fstype != "ext4" { + t.Errorf("expected ext4, got %s", fstype) + } +} + +func TestGetDiskFormatBlkidFallbackDetectsFS(t *testing.T) { + fs := &FS{} + disk := "/dev/disk/by-id/dm-uuid-mpath-test123" + + defaultGetExecCommandCombinedOutput := getExecCommandCombinedOutput + defer func() { + getExecCommandCombinedOutput = defaultGetExecCommandCombinedOutput + }() + + var lsblkCalled, blkidCalled bool + getExecCommandCombinedOutput = func(name string, args ...string) ([]byte, error) { + if name == "lsblk" { + lsblkCalled = true + return []byte("\n"), nil + } + if name == "blkid" { + blkidCalled = true + expectedArgs := []string{"-o", "value", "-s", "TYPE", disk} + if len(args) != len(expectedArgs) { + t.Errorf("expected %d blkid arguments, got %d", len(expectedArgs), len(args)) + } + for i, arg := range expectedArgs { + if i < len(args) && args[i] != arg { + t.Errorf("expected blkid arg[%d] = %s, got %s", i, arg, args[i]) + } + } + return []byte("ext4"), nil + } + t.Errorf("unexpected command: %s", name) + return nil, errors.New("unexpected command") + } + + fstype, err := fs.getDiskFormat(context.Background(), disk) + if err != nil { + t.Errorf("expected no error, got %v", err) + } + if fstype != "ext4" { + t.Errorf("expected ext4, got %s", fstype) + } + if !lsblkCalled { + t.Errorf("expected lsblk to be called first") + } + if !blkidCalled { + t.Errorf("expected blkid to be called as fallback") + } +} + +func TestGetDiskFormatBlkidFallbackUnformatted(t *testing.T) { + fs := &FS{} + disk := "/dev/disk/by-id/dm-uuid-mpath-test456" + + defaultGetExecCommandCombinedOutput := getExecCommandCombinedOutput + defer func() { + getExecCommandCombinedOutput = defaultGetExecCommandCombinedOutput + }() + + getExecCommandCombinedOutput = func(name string, _ ...string) ([]byte, error) { + if name == "lsblk" { + return []byte("\n"), nil + } + if name == "blkid" { + return []byte(""), nil + } + t.Errorf("unexpected command: %s", name) + return nil, errors.New("unexpected command") + } + + fstype, err := fs.getDiskFormat(context.Background(), disk) + if err != nil { + t.Errorf("expected no error, got %v", err) + } + if fstype != "" { + t.Errorf("expected empty fstype, got %s", fstype) + } +} + +func TestGetDiskFormatBlkidFallbackError(t *testing.T) { + fs := &FS{} + disk := "/dev/disk/by-id/dm-uuid-mpath-test789" + + defaultGetExecCommandCombinedOutput := getExecCommandCombinedOutput + defer func() { + getExecCommandCombinedOutput = defaultGetExecCommandCombinedOutput + }() + + getExecCommandCombinedOutput = func(name string, _ ...string) ([]byte, error) { + if name == "lsblk" { + return []byte("\n"), nil + } + if name == "blkid" { + return []byte(""), errors.New("exit status 2") + } + t.Errorf("unexpected command: %s", name) + return nil, errors.New("unexpected command") + } + + fstype, err := fs.getDiskFormat(context.Background(), disk) + if err != nil { + t.Errorf("expected no error, got %v", err) + } + if fstype != "" { + t.Errorf("expected empty fstype, got %s", fstype) + } +} + func Test_formatAndMount(t *testing.T) { fs := &MockFS{} ctx := context.WithValue(context.Background(), ContextKey("RequestID"), "test-req-id") diff --git a/gofsutil_reclaim.go b/gofsutil_reclaim.go new file mode 100644 index 0000000..11432df --- /dev/null +++ b/gofsutil_reclaim.go @@ -0,0 +1,78 @@ +// Copyright © 2025 Dell Inc. or its subsidiaries. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package gofsutil + +import ( + "context" + "time" +) + +// FstrimResult holds the result of an fstrim operation on a mounted filesystem. +type FstrimResult struct { + // BytesTrimmed is the number of bytes reported as trimmed by fstrim -v. + BytesTrimmed int64 + // Duration is the wall-clock time the fstrim command took. + Duration time.Duration +} + +// BlkdiscardResult holds the result of a blkdiscard operation on a block device. +type BlkdiscardResult struct { + // BytesDiscarded is the total size of the device (all bytes are discarded). + BytesDiscarded int64 + // Duration is the wall-clock time the blkdiscard command took. + Duration time.Duration +} + +// DiscardCapability describes whether a block device supports discard operations +// (SCSI UNMAP / NVMe Deallocate). +type DiscardCapability struct { + // Supported is true when the device reports discard_max_bytes > 0. + Supported bool + // DiscardMaxBytes is the value read from /sys/block//queue/discard_max_bytes. + DiscardMaxBytes int64 + // Reason is a human-readable explanation when Supported is false. + Reason string +} + +// Fstrim runs fstrim on the specified mount point, returning the bytes trimmed. +// The context deadline/timeout controls the maximum execution time. +func Fstrim(ctx context.Context, mountPoint string) (*FstrimResult, error) { + return fs.Fstrim(ctx, mountPoint) +} + +// Blkdiscard runs blkdiscard on the specified block device path, returning the +// bytes discarded. The context deadline/timeout controls the maximum execution time. +func Blkdiscard(ctx context.Context, devicePath string) (*BlkdiscardResult, error) { + return fs.Blkdiscard(ctx, devicePath) +} + +// CheckDiscardSupport checks whether a block device supports discard operations +// (SCSI UNMAP / NVMe Deallocate) by reading sysfs attributes. +func CheckDiscardSupport(ctx context.Context, devicePath string) (*DiscardCapability, error) { + return fs.CheckDiscardSupport(ctx, devicePath) +} + +// Fstrim runs fstrim on the specified mount point, returning the bytes trimmed. +func (f *FS) Fstrim(ctx context.Context, mountPoint string) (*FstrimResult, error) { + return f.fstrim(ctx, mountPoint) +} + +// Blkdiscard runs blkdiscard on the specified block device, returning the bytes discarded. +func (f *FS) Blkdiscard(ctx context.Context, devicePath string) (*BlkdiscardResult, error) { + return f.blkdiscard(ctx, devicePath) +} + +// CheckDiscardSupport checks whether a block device supports discard (UNMAP/Deallocate). +func (f *FS) CheckDiscardSupport(ctx context.Context, devicePath string) (*DiscardCapability, error) { + return f.checkDiscardSupport(ctx, devicePath) +} diff --git a/gofsutil_reclaim_linux.go b/gofsutil_reclaim_linux.go new file mode 100644 index 0000000..9c16bd7 --- /dev/null +++ b/gofsutil_reclaim_linux.go @@ -0,0 +1,186 @@ +// Copyright © 2025 Dell Inc. or its subsidiaries. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package gofsutil + +import ( + "context" + "fmt" + "os" + "os/exec" + "path/filepath" + "regexp" + "strconv" + "strings" + "syscall" + "time" +) + +// reclaimExecFn is the mockable function variable used to execute system commands +// for reclaim operations (fstrim, blkdiscard). It can be overridden in tests. +var reclaimExecFn = defaultReclaimExec + +func defaultReclaimExec(ctx context.Context, name string, args ...string) ([]byte, error) { + cmd := exec.CommandContext(ctx, name, args...) // #nosec G204 + // Start the child process in a new process group + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + cmd.WaitDelay = 2 * time.Second + cmd.Cancel = func() error { + return syscall.Kill(-cmd.Process.Pid, syscall.SIGINT) + } + return cmd.CombinedOutput() +} + +// fstrim runs the fstrim command on the specified mount point. +func (f *FS) fstrim(ctx context.Context, mountPoint string) (*FstrimResult, error) { + start := time.Now() + output, err := reclaimExecFn(ctx, "fstrim", "-v", mountPoint) + duration := time.Since(start) + + if err != nil { + if strings.Contains(err.Error(), "killed") { + return nil, fmt.Errorf("fstrim timed out on %s: %w", mountPoint, err) + } + return nil, fmt.Errorf("fstrim failed on %s: %w", mountPoint, err) + } + + bytesTrimmed := parseFstrimBytes(string(output)) + return &FstrimResult{ + BytesTrimmed: bytesTrimmed, + Duration: duration, + }, nil +} + +// blkdiscard runs the blkdiscard command on the specified block device. +func (f *FS) blkdiscard(ctx context.Context, devicePath string) (*BlkdiscardResult, error) { + deviceSize, err := getBlockDeviceSize(devicePath) + if err != nil { + return nil, fmt.Errorf("failed to get device size for %s: %w", devicePath, err) + } + + start := time.Now() + _, err = reclaimExecFn(ctx, "blkdiscard", devicePath) + duration := time.Since(start) + + if err != nil { + if strings.Contains(err.Error(), "killed") { + return nil, fmt.Errorf("blkdiscard timed out on %s: %w", devicePath, err) + } + return nil, fmt.Errorf("blkdiscard failed on %s: %w", devicePath, err) + } + + return &BlkdiscardResult{ + BytesDiscarded: deviceSize, + Duration: duration, + }, nil +} + +// checkDiscardSupport checks sysfs to determine discard capability. +func (f *FS) checkDiscardSupport(_ context.Context, devicePath string) (*DiscardCapability, error) { + // Resolve symlinks to get the real device name + resolved, err := filepath.EvalSymlinks(devicePath) + if err != nil { + resolved = devicePath + } + devName := filepath.Base(resolved) + + // Read discard_max_bytes from sysfs + discardMaxPath := filepath.Join(sysBlockDir, devName, "queue", "discard_max_bytes") + data, err := os.ReadFile(discardMaxPath) + if err != nil { + return nil, fmt.Errorf("failed to read %s: %w", discardMaxPath, err) + } + + maxBytesStr := strings.TrimSpace(string(data)) + maxBytes, err := strconv.ParseInt(maxBytesStr, 10, 64) + if err != nil { + return nil, fmt.Errorf("failed to parse discard_max_bytes from %s: %w", discardMaxPath, err) + } + + // If discard_max_bytes is 0, device does not support discard + if maxBytes == 0 { + return &DiscardCapability{ + Supported: false, + DiscardMaxBytes: 0, + Reason: "discard_max_bytes=0", + }, nil + } + + // Check for dm devices device by reading dm/uuid + if strings.HasPrefix(devName, "dm-") { + dmUUIDPath := filepath.Join(sysBlockDir, devName, "dm", "uuid") + dmUUID, err := os.ReadFile(dmUUIDPath) + if err == nil { + uuid := strings.TrimSpace(string(dmUUID)) + if strings.HasPrefix(strings.ToUpper(uuid), "CRYPT-") { + // dm-crypt device: check discard_granularity for allow-discards + granularityPath := filepath.Join(sysBlockDir, devName, "queue", "discard_granularity") + granData, gErr := os.ReadFile(granularityPath) + if gErr != nil || strings.TrimSpace(string(granData)) == "0" { + return &DiscardCapability{ + Supported: false, + DiscardMaxBytes: maxBytes, + Reason: "dm-crypt device without allow-discards", + }, nil + } + } + } + } + + return &DiscardCapability{ + Supported: true, + DiscardMaxBytes: maxBytes, + }, nil +} + +// fstrimBytesRegex matches the byte count in fstrim -v output. +var fstrimBytesRegex = regexp.MustCompile(`(\d+)\s+bytes?`) + +// parseFstrimBytes extracts the bytes value from fstrim -v output. +// Example output: "/mnt/data: 42949672960 bytes were trimmed" +func parseFstrimBytes(output string) int64 { + matches := fstrimBytesRegex.FindStringSubmatch(output) + if len(matches) < 2 { + return 0 + } + val, err := strconv.ParseInt(matches[1], 10, 64) + if err != nil { + return 0 + } + return val +} + +// getBlockDeviceSize returns the size in bytes of a block device by reading +// /sys/block//size (which reports the size in 512-byte sectors). +// Symlinks are resolved via filepath.EvalSymlinks to handle device mapper paths. +func getBlockDeviceSize(devicePath string) (int64, error) { + // Resolve symlinks to get the real device name + resolved, err := filepath.EvalSymlinks(devicePath) + if err != nil { + resolved = devicePath + } + devName := filepath.Base(resolved) + + sizePath := filepath.Join(sysBlockDir, devName, "size") + data, err := os.ReadFile(sizePath) + if err != nil { + return 0, fmt.Errorf("failed to read %s: %w", sizePath, err) + } + + sizeStr := strings.TrimSpace(string(data)) + sectors, err := strconv.ParseInt(sizeStr, 10, 64) + if err != nil { + return 0, fmt.Errorf("failed to parse size from %s: %w", sizePath, err) + } + + return sectors * 512, nil +} diff --git a/gofsutil_reclaim_linux_test.go b/gofsutil_reclaim_linux_test.go new file mode 100644 index 0000000..eeb8d53 --- /dev/null +++ b/gofsutil_reclaim_linux_test.go @@ -0,0 +1,548 @@ +// Copyright © 2025 Dell Inc. or its subsidiaries. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package gofsutil + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// ---- Linux Test Helpers ----------------------------------------------------- + +// setupMockSysfs creates a temp directory mimicking /sys/block// with +// the specified files and their content. Returns the temp base path. +// The caller should override sysBlockDir to point to this path. +func setupMockSysfs(t *testing.T, devName string, files map[string]string) string { + t.Helper() + tmpDir := t.TempDir() + devDir := filepath.Join(tmpDir, devName) + for relPath, content := range files { + fullPath := filepath.Join(devDir, relPath) + err := os.MkdirAll(filepath.Dir(fullPath), 0o755) + require.NoError(t, err, "failed to create sysfs mock directory") + err = os.WriteFile(fullPath, []byte(content), 0o644) + require.NoError(t, err, "failed to write sysfs mock file") + } + return tmpDir +} + +// setupReclaimMockExec overrides reclaimExecFn for testing and returns a +// cleanup function that restores the original. Follows the setMockExec +// pattern from gofsutil_fsck_test.go. +func setupReclaimMockExec(handler func(ctx context.Context, name string, args ...string) ([]byte, error)) func() { + orig := reclaimExecFn + reclaimExecFn = handler + return func() { reclaimExecFn = orig } +} + +// overrideSysBlockDir temporarily overrides the sysBlockDir package variable +// for tests that need mock sysfs access. Returns a cleanup function. +func overrideSysBlockDir(t *testing.T, newDir string) { + t.Helper() + orig := sysBlockDir + sysBlockDir = newDir + t.Cleanup(func() { sysBlockDir = orig }) +} + +// ---- Unit Tests: parseFstrimBytes (U-001 to U-008) ------------------------- + +// Test ID: U-001 +// Test ID: U-002 +// Test ID: U-003 +// Test ID: U-004 +// Test ID: U-005 +// Test ID: U-006 +// Test ID: U-007 +// Test ID: U-008 +func TestParseFstrimBytes(t *testing.T) { + tests := []struct { + testID string + name string + input string + expected int64 + }{ + { + testID: "U-001", + name: "standard_output", + input: "/mnt/data: 42949672960 bytes were trimmed", + expected: 42949672960, + }, + { + testID: "U-002", + name: "zero_bytes_trimmed", + input: "/mnt/data: 0 bytes were trimmed", + expected: 0, + }, + { + testID: "U-003", + name: "singular_byte", + input: "/mnt: 1 byte trimmed", + expected: 1, + }, + { + testID: "U-004", + name: "empty_string", + input: "", + expected: 0, + }, + { + testID: "U-005", + name: "no_match", + input: "fstrim: some error occurred", + expected: 0, + }, + { + testID: "U-006", + name: "large_value", + input: "/mnt: 9223372036854775807 bytes were trimmed", + expected: 9223372036854775807, // math.MaxInt64 + }, + { + testID: "U-007", + name: "bytes_without_were", + input: "/mnt: 1024 bytes trimmed", + expected: 1024, + }, + { + testID: "U-008", + name: "multiline_output", + input: "\n/mnt: 512 bytes were trimmed\n", + expected: 512, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := parseFstrimBytes(tt.input) + assert.Equal(t, tt.expected, result, + "[%s] parseFstrimBytes(%q) should return %d", tt.testID, tt.input, tt.expected) + }) + } +} + +// ---- Unit Tests: getBlockDeviceSize (U-009 to U-013) ------------------------ + +// Test ID: U-009 +func TestGetBlockDeviceSize_ValidDevice(t *testing.T) { + // Create temp sysfs with size file: 2097152 sectors = 1 GiB + mockDir := setupMockSysfs(t, "sda", map[string]string{ + "size": "2097152\n", + }) + overrideSysBlockDir(t, mockDir) + + size, err := getBlockDeviceSize("/dev/sda") + + require.NoError(t, err, "getBlockDeviceSize should not return error for valid device") + assert.Equal(t, int64(1073741824), size, + "Expected 2097152 sectors * 512 = 1073741824 bytes") +} + +// Test ID: U-010 +func TestGetBlockDeviceSize_MissingSysfsFile(t *testing.T) { + // Point to a non-existent directory + overrideSysBlockDir(t, "/tmp/nonexistent-sysfs-path-for-test") + + _, err := getBlockDeviceSize("/dev/nonexistent") + + require.Error(t, err, "getBlockDeviceSize should return error for missing sysfs file") +} + +// Test ID: U-011 +func TestGetBlockDeviceSize_InvalidContent(t *testing.T) { + mockDir := setupMockSysfs(t, "sdb", map[string]string{ + "size": "not_a_number\n", + }) + overrideSysBlockDir(t, mockDir) + + _, err := getBlockDeviceSize("/dev/sdb") + + require.Error(t, err, "getBlockDeviceSize should return error for non-numeric content") + assert.Contains(t, err.Error(), "failed to parse size", + "Error should mention 'failed to parse size'") +} + +// Test ID: U-012 +func TestGetBlockDeviceSize_ZeroSectors(t *testing.T) { + mockDir := setupMockSysfs(t, "sdc", map[string]string{ + "size": "0\n", + }) + overrideSysBlockDir(t, mockDir) + + size, err := getBlockDeviceSize("/dev/sdc") + + require.NoError(t, err, "getBlockDeviceSize should not return error for zero sectors") + assert.Equal(t, int64(0), size, + "Expected 0 sectors * 512 = 0 bytes") +} + +// Test ID: U-013 +func TestGetBlockDeviceSize_MapperDeviceSymlink(t *testing.T) { + // Create temp sysfs with dm-0/size and a symlink from mapper/vol -> dm-0 + tmpDir := t.TempDir() + + // Create the dm-0 device in sysfs + dm0Dir := filepath.Join(tmpDir, "dm-0") + require.NoError(t, os.MkdirAll(dm0Dir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dm0Dir, "size"), []byte("4194304\n"), 0o644)) + + overrideSysBlockDir(t, tmpDir) + + // Create a symlink: /tmp/.../mapper/vol -> /tmp/.../devdir/dm-0 + mapperDir := filepath.Join(tmpDir, "mapper") + require.NoError(t, os.MkdirAll(mapperDir, 0o755)) + devDir := filepath.Join(tmpDir, "devdir") + require.NoError(t, os.MkdirAll(devDir, 0o755)) + // Create the actual dm-0 device file (symlink target) + dmTarget := filepath.Join(devDir, "dm-0") + require.NoError(t, os.WriteFile(dmTarget, []byte{}, 0o644)) + // Create symlink + volLink := filepath.Join(mapperDir, "vol") + require.NoError(t, os.Symlink(dmTarget, volLink)) + + // Call with mapper path - stub won't resolve but test verifies compilation + size, err := getBlockDeviceSize(volLink) + + // The stub returns 0, nil. In the real implementation, it would resolve the + // symlink and read dm-0/size (4194304 sectors * 512 = 2147483648 bytes). + require.NoError(t, err) + assert.Equal(t, int64(2147483648), size, + "Expected 4194304 sectors * 512 = 2147483648 bytes for mapper device") +} + +// ---- Unit Tests: checkDiscardSupport (U-014 to U-021) ----------------------- + +// Test ID: U-014 +func TestCheckDiscardSupport_SupportedDevice(t *testing.T) { + mockDir := setupMockSysfs(t, "sda", map[string]string{ + "queue/discard_max_bytes": "4294967295\n", + }) + overrideSysBlockDir(t, mockDir) + + fsObj := &FS{} + result, err := fsObj.checkDiscardSupport(context.Background(), "/dev/sda") + + require.NoError(t, err, "checkDiscardSupport should not error for supported device") + require.NotNil(t, result, "Expected non-nil DiscardCapability") + assert.True(t, result.Supported, "Expected Supported == true") + assert.Equal(t, int64(4294967295), result.DiscardMaxBytes, + "Expected DiscardMaxBytes == 4294967295") + assert.Empty(t, result.Reason, "Expected empty Reason for supported device") +} + +// Test ID: U-015 +func TestCheckDiscardSupport_UnsupportedZeroMaxBytes(t *testing.T) { + mockDir := setupMockSysfs(t, "sdb", map[string]string{ + "queue/discard_max_bytes": "0\n", + }) + overrideSysBlockDir(t, mockDir) + + fsObj := &FS{} + result, err := fsObj.checkDiscardSupport(context.Background(), "/dev/sdb") + + require.NoError(t, err, "checkDiscardSupport should not error for unsupported device") + require.NotNil(t, result, "Expected non-nil DiscardCapability") + assert.False(t, result.Supported, "Expected Supported == false when discard_max_bytes=0") + assert.Equal(t, int64(0), result.DiscardMaxBytes, + "Expected DiscardMaxBytes == 0") + assert.Contains(t, result.Reason, "discard_max_bytes=0", + "Reason should mention discard_max_bytes=0") +} + +// Test ID: U-016 +func TestCheckDiscardSupport_MissingSysfsPath(t *testing.T) { + overrideSysBlockDir(t, "/tmp/nonexistent-sysfs-path-for-test") + + fsObj := &FS{} + _, err := fsObj.checkDiscardSupport(context.Background(), "/dev/nonexistent") + + require.Error(t, err, "checkDiscardSupport should error for missing sysfs path") + assert.Contains(t, err.Error(), "failed to read", + "Error should mention 'failed to read'") +} + +// Test ID: U-017 +func TestCheckDiscardSupport_InvalidDiscardMaxBytes(t *testing.T) { + mockDir := setupMockSysfs(t, "sdc", map[string]string{ + "queue/discard_max_bytes": "abc\n", + }) + overrideSysBlockDir(t, mockDir) + + fsObj := &FS{} + _, err := fsObj.checkDiscardSupport(context.Background(), "/dev/sdc") + + require.Error(t, err, "checkDiscardSupport should error for non-numeric discard_max_bytes") + assert.Contains(t, err.Error(), "failed to parse discard_max_bytes", + "Error should mention 'failed to parse discard_max_bytes'") +} + +// Test ID: U-018 +func TestCheckDiscardSupport_DmCryptWithoutAllowDiscards(t *testing.T) { + mockDir := setupMockSysfs(t, "dm-0", map[string]string{ + "queue/discard_max_bytes": "4294967295\n", + "dm/uuid": "CRYPT-LUKS2-abc123\n", + "queue/discard_granularity": "0\n", + }) + overrideSysBlockDir(t, mockDir) + + fsObj := &FS{} + result, err := fsObj.checkDiscardSupport(context.Background(), "/dev/dm-0") + + require.NoError(t, err, "checkDiscardSupport should not error") + require.NotNil(t, result, "Expected non-nil DiscardCapability") + assert.False(t, result.Supported, + "Expected Supported == false for dm-crypt without allow-discards") + assert.Contains(t, result.Reason, "dm-crypt", + "Reason should mention dm-crypt") + assert.Contains(t, result.Reason, "allow-discards", + "Reason should mention allow-discards") +} + +// Test ID: U-019 +func TestCheckDiscardSupport_DmCryptWithAllowDiscards(t *testing.T) { + mockDir := setupMockSysfs(t, "dm-1", map[string]string{ + "queue/discard_max_bytes": "4294967295\n", + "dm/uuid": "CRYPT-LUKS2-abc123\n", + "queue/discard_granularity": "4096\n", + }) + overrideSysBlockDir(t, mockDir) + + fsObj := &FS{} + result, err := fsObj.checkDiscardSupport(context.Background(), "/dev/dm-1") + + require.NoError(t, err, "checkDiscardSupport should not error") + require.NotNil(t, result, "Expected non-nil DiscardCapability") + assert.True(t, result.Supported, + "Expected Supported == true for dm-crypt with allow-discards") + assert.Equal(t, int64(4294967295), result.DiscardMaxBytes, + "Expected DiscardMaxBytes to match sysfs value") +} + +// Test ID: U-020 +func TestCheckDiscardSupport_NonCryptDmDevice(t *testing.T) { + mockDir := setupMockSysfs(t, "dm-2", map[string]string{ + "queue/discard_max_bytes": "4294967295\n", + "dm/uuid": "LVM-abc123\n", + }) + overrideSysBlockDir(t, mockDir) + + fsObj := &FS{} + result, err := fsObj.checkDiscardSupport(context.Background(), "/dev/dm-2") + + require.NoError(t, err, "checkDiscardSupport should not error") + require.NotNil(t, result, "Expected non-nil DiscardCapability") + assert.True(t, result.Supported, + "Expected Supported == true for non-CRYPT dm device with valid discard") +} + +// Test ID: U-021 +func TestCheckDiscardSupport_NoDmUuidFile(t *testing.T) { + mockDir := setupMockSysfs(t, "sdd", map[string]string{ + "queue/discard_max_bytes": "4294967295\n", + // No dm/uuid file - not a DM device + }) + overrideSysBlockDir(t, mockDir) + + fsObj := &FS{} + result, err := fsObj.checkDiscardSupport(context.Background(), "/dev/sdd") + + require.NoError(t, err, "checkDiscardSupport should not error") + require.NotNil(t, result, "Expected non-nil DiscardCapability") + assert.True(t, result.Supported, + "Expected Supported == true for device without dm/uuid and discard > 0") +} + +// ---- Unit Tests: fstrim command execution (U-022 to U-025) ------------------ + +// Test ID: U-022 +func TestFstrim_Success(t *testing.T) { + restore := setupReclaimMockExec(func(_ context.Context, name string, _ ...string) ([]byte, error) { + if name == "fstrim" { + return []byte("/mnt/data: 1048576 bytes were trimmed"), nil + } + return nil, nil + }) + defer restore() + + fsObj := &FS{} + result, err := fsObj.fstrim(context.Background(), "/mnt/data") + + require.NoError(t, err, "fstrim should succeed") + require.NotNil(t, result, "Expected non-nil FstrimResult") + assert.Equal(t, int64(1048576), result.BytesTrimmed, + "Expected BytesTrimmed == 1048576") + assert.Greater(t, result.Duration.Nanoseconds(), int64(0), + "Expected Duration > 0") +} + +// Test ID: U-023 +func TestFstrim_CommandFailure(t *testing.T) { + restore := setupReclaimMockExec(func(_ context.Context, name string, _ ...string) ([]byte, error) { + if name == "fstrim" { + return []byte("fstrim: /mnt/data: the discard operation is not supported"), os.ErrPermission + } + return nil, nil + }) + defer restore() + + fsObj := &FS{} + _, err := fsObj.fstrim(context.Background(), "/mnt/data") + + require.Error(t, err, "fstrim should fail when command exits non-zero") + assert.Contains(t, err.Error(), "fstrim failed on", + "Error should contain 'fstrim failed on'") +} + +// Test ID: U-024 +func TestFstrim_ContextTimeout(t *testing.T) { + restore := setupReclaimMockExec(func(_ context.Context, name string, _ ...string) ([]byte, error) { + if name == "fstrim" { + // Simulate a killed process error + return nil, &timeoutExecError{msg: "signal: killed"} + } + return nil, nil + }) + defer restore() + + fsObj := &FS{} + _, err := fsObj.fstrim(context.Background(), "/mnt/data") + + require.Error(t, err, "fstrim should fail on context timeout") + assert.Contains(t, err.Error(), "fstrim timed out", + "Error should contain 'fstrim timed out'") +} + +// Test ID: U-025 +func TestFstrim_ZeroBytesOutput(t *testing.T) { + restore := setupReclaimMockExec(func(_ context.Context, name string, _ ...string) ([]byte, error) { + if name == "fstrim" { + return []byte("/mnt: 0 bytes were trimmed"), nil + } + return nil, nil + }) + defer restore() + + fsObj := &FS{} + result, err := fsObj.fstrim(context.Background(), "/mnt") + + require.NoError(t, err, "fstrim should succeed with 0 bytes trimmed") + require.NotNil(t, result, "Expected non-nil FstrimResult") + assert.Equal(t, int64(0), result.BytesTrimmed, + "Expected BytesTrimmed == 0") +} + +// ---- Unit Tests: blkdiscard command execution (U-026 to U-029) -------------- + +// Test ID: U-026 +func TestBlkdiscard_Success(t *testing.T) { + // Mock sysfs for device size + mockDir := setupMockSysfs(t, "sda", map[string]string{ + "size": "2097152\n", // 1 GiB in 512-byte sectors + }) + overrideSysBlockDir(t, mockDir) + + restore := setupReclaimMockExec(func(_ context.Context, name string, _ ...string) ([]byte, error) { + if name == "blkdiscard" { + return []byte(""), nil + } + return nil, nil + }) + defer restore() + + fsObj := &FS{} + result, err := fsObj.blkdiscard(context.Background(), "/dev/sda") + + require.NoError(t, err, "blkdiscard should succeed") + require.NotNil(t, result, "Expected non-nil BlkdiscardResult") + assert.Equal(t, int64(1073741824), result.BytesDiscarded, + "Expected BytesDiscarded == device size (1073741824)") +} + +// Test ID: U-027 +func TestBlkdiscard_DeviceSizeReadFailure(t *testing.T) { + // Point to non-existent sysfs + overrideSysBlockDir(t, "/tmp/nonexistent-sysfs-path-for-test") + + fsObj := &FS{} + _, err := fsObj.blkdiscard(context.Background(), "/dev/nonexistent") + + require.Error(t, err, "blkdiscard should fail when device size cannot be read") + assert.Contains(t, err.Error(), "failed to get device size", + "Error should contain 'failed to get device size'") +} + +// Test ID: U-028 +func TestBlkdiscard_CommandFailure(t *testing.T) { + // Mock sysfs for device size (so we get past the size check) + mockDir := setupMockSysfs(t, "sda", map[string]string{ + "size": "2097152\n", + }) + overrideSysBlockDir(t, mockDir) + + restore := setupReclaimMockExec(func(_ context.Context, name string, _ ...string) ([]byte, error) { + if name == "blkdiscard" { + return []byte("blkdiscard: /dev/sda: BLKDISCARD ioctl failed: Operation not permitted"), os.ErrPermission + } + return nil, nil + }) + defer restore() + + fsObj := &FS{} + _, err := fsObj.blkdiscard(context.Background(), "/dev/sda") + + require.Error(t, err, "blkdiscard should fail when command exits non-zero") + assert.Contains(t, err.Error(), "blkdiscard failed on", + "Error should contain 'blkdiscard failed on'") +} + +// Test ID: U-029 +func TestBlkdiscard_ContextTimeout(t *testing.T) { + // Mock sysfs for device size + mockDir := setupMockSysfs(t, "sda", map[string]string{ + "size": "2097152\n", + }) + overrideSysBlockDir(t, mockDir) + + restore := setupReclaimMockExec(func(_ context.Context, name string, _ ...string) ([]byte, error) { + if name == "blkdiscard" { + // Simulate a killed process error + return nil, &timeoutExecError{msg: "signal: killed"} + } + return nil, nil + }) + defer restore() + + fsObj := &FS{} + _, err := fsObj.blkdiscard(context.Background(), "/dev/sda") + + require.Error(t, err, "blkdiscard should fail on context timeout") + assert.Contains(t, err.Error(), "blkdiscard timed out", + "Error should contain 'blkdiscard timed out'") +} + +// ---- Test helpers for simulating exec errors -------------------------------- + +// timeoutExecError is a test helper that simulates a process-killed error +// for testing timeout/cancellation paths. It is NOT an exec.ExitError, but +// is used to represent the error that would be returned when a process is killed. +type timeoutExecError struct { + msg string +} + +func (e *timeoutExecError) Error() string { + return e.msg +} diff --git a/gofsutil_reclaim_test.go b/gofsutil_reclaim_test.go new file mode 100644 index 0000000..1e33d5a --- /dev/null +++ b/gofsutil_reclaim_test.go @@ -0,0 +1,282 @@ +// Copyright © 2025 Dell Inc. or its subsidiaries. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package gofsutil + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// ---- Test Helpers ----------------------------------------------------------- + +// resetGOFSMock resets all reclamation-related GOFSMock flags and custom result +// variables to zero values. Called via t.Cleanup() to avoid cross-test pollution. +func resetGOFSMock(t *testing.T) { + t.Helper() + t.Cleanup(func() { + GOFSMock.InduceFstrimError = false + GOFSMock.InduceBlkdiscardError = false + GOFSMock.InduceCheckDiscardSupportError = false + GOFSMockFstrimResult = nil + GOFSMockBlkdiscardResult = nil + GOFSMockDiscardCapability = nil + }) +} + +// withMockFS sets the package-level fs to a mockfs instance and restores +// the original on test cleanup. Returns the original fs for inspection if needed. +func withMockFS(t *testing.T) { + t.Helper() + origFS := fs + UseMockFS() + t.Cleanup(func() { fs = origFS }) +} + +// ---- Contract Tests --------------------------------------------------------- + +// Test ID: C-001 +// Package-level Fstrim() delegates to fs.Fstrim() which delegates to fs.fstrim(). +// Verifies the full call chain through the FSinterface. +func TestFstrim_PackageLevelDelegatesToFS(t *testing.T) { + withMockFS(t) + resetGOFSMock(t) + + ctx := context.Background() + result, err := Fstrim(ctx, "/mnt/data") + + require.NoError(t, err) + require.NotNil(t, result, "Fstrim should return a non-nil result via mock") + assert.Equal(t, int64(1073741824), result.BytesTrimmed, + "Expected default mock BytesTrimmed of 1 GiB (1073741824)") +} + +// Test ID: C-002 +// Package-level Blkdiscard() delegates to fs.Blkdiscard() which delegates to fs.blkdiscard(). +// Verifies the full call chain through the FSinterface. +func TestBlkdiscard_PackageLevelDelegatesToFS(t *testing.T) { + withMockFS(t) + resetGOFSMock(t) + + ctx := context.Background() + result, err := Blkdiscard(ctx, "/dev/sda") + + require.NoError(t, err) + require.NotNil(t, result, "Blkdiscard should return a non-nil result via mock") + assert.Equal(t, int64(107374182400), result.BytesDiscarded, + "Expected default mock BytesDiscarded of 100 GiB (107374182400)") +} + +// Test ID: C-003 +// Package-level CheckDiscardSupport() delegates through the full call chain. +// Verifies the full call chain through the FSinterface. +func TestCheckDiscardSupport_PackageLevelDelegatesToFS(t *testing.T) { + withMockFS(t) + resetGOFSMock(t) + + ctx := context.Background() + result, err := CheckDiscardSupport(ctx, "/dev/sda") + + require.NoError(t, err) + require.NotNil(t, result, "CheckDiscardSupport should return a non-nil result via mock") + assert.True(t, result.Supported, "Expected default mock Supported == true") + assert.Equal(t, int64(4294967295), result.DiscardMaxBytes, + "Expected default mock DiscardMaxBytes of 4294967295") +} + +// Test ID: C-004 +// GOFSMock.InduceFstrimError causes Fstrim() to return error through mock chain. +func TestMockFstrim_InduceError(t *testing.T) { + withMockFS(t) + resetGOFSMock(t) + GOFSMock.InduceFstrimError = true + + ctx := context.Background() + result, err := Fstrim(ctx, "/mnt/data") + + require.Error(t, err, "Expected an error when InduceFstrimError is true") + assert.Nil(t, result, "Expected nil result when error is induced") + assert.Contains(t, err.Error(), "fstrim induced error", + "Error should contain 'fstrim induced error'") +} + +// Test ID: C-005 +// GOFSMock.InduceBlkdiscardError causes Blkdiscard() to return error through mock chain. +func TestMockBlkdiscard_InduceError(t *testing.T) { + withMockFS(t) + resetGOFSMock(t) + GOFSMock.InduceBlkdiscardError = true + + ctx := context.Background() + result, err := Blkdiscard(ctx, "/dev/sda") + + require.Error(t, err, "Expected an error when InduceBlkdiscardError is true") + assert.Nil(t, result, "Expected nil result when error is induced") + assert.Contains(t, err.Error(), "blkdiscard induced error", + "Error should contain 'blkdiscard induced error'") +} + +// Test ID: C-006 +// GOFSMock.InduceCheckDiscardSupportError causes CheckDiscardSupport() to return error. +func TestMockCheckDiscardSupport_InduceError(t *testing.T) { + withMockFS(t) + resetGOFSMock(t) + GOFSMock.InduceCheckDiscardSupportError = true + + ctx := context.Background() + result, err := CheckDiscardSupport(ctx, "/dev/sda") + + require.Error(t, err, "Expected an error when InduceCheckDiscardSupportError is true") + assert.Nil(t, result, "Expected nil result when error is induced") + assert.Contains(t, err.Error(), "checkDiscardSupport induced error", + "Error should contain 'checkDiscardSupport induced error'") +} + +// Test ID: C-007 +// GOFSMockFstrimResult overrides the default mock return value. +func TestMockFstrim_CustomResult(t *testing.T) { + withMockFS(t) + resetGOFSMock(t) + GOFSMockFstrimResult = &FstrimResult{BytesTrimmed: 42} + + ctx := context.Background() + result, err := Fstrim(ctx, "/mnt/data") + + require.NoError(t, err) + require.NotNil(t, result, "Expected non-nil custom result") + assert.Equal(t, int64(42), result.BytesTrimmed, + "Expected custom BytesTrimmed value of 42") +} + +// Test ID: C-008 +// GOFSMockBlkdiscardResult overrides the default mock return value. +func TestMockBlkdiscard_CustomResult(t *testing.T) { + withMockFS(t) + resetGOFSMock(t) + GOFSMockBlkdiscardResult = &BlkdiscardResult{BytesDiscarded: 999} + + ctx := context.Background() + result, err := Blkdiscard(ctx, "/dev/sda") + + require.NoError(t, err) + require.NotNil(t, result, "Expected non-nil custom result") + assert.Equal(t, int64(999), result.BytesDiscarded, + "Expected custom BytesDiscarded value of 999") +} + +// Test ID: C-009 +// GOFSMockDiscardCapability overrides the default mock return value. +func TestMockCheckDiscardSupport_CustomCapability(t *testing.T) { + withMockFS(t) + resetGOFSMock(t) + GOFSMockDiscardCapability = &DiscardCapability{Supported: false, Reason: "test"} + + ctx := context.Background() + result, err := CheckDiscardSupport(ctx, "/dev/sda") + + require.NoError(t, err) + require.NotNil(t, result, "Expected non-nil custom capability") + assert.False(t, result.Supported, "Expected custom Supported == false") + assert.Equal(t, "test", result.Reason, + "Expected custom Reason 'test'") +} + +// ---- Unit Tests: Mock Method Defaults (U-030 to U-032) --------------------- + +// Test ID: U-030 +// Mock fstrim() returns default FstrimResult when no overrides are set. +func TestMockFstrim_DefaultResult(t *testing.T) { + resetGOFSMock(t) + m := &mockfs{} + + ctx := context.Background() + result, err := m.fstrim(ctx, "/mnt/data") + + require.NoError(t, err) + require.NotNil(t, result, "Expected non-nil default mock result") + assert.Equal(t, int64(1073741824), result.BytesTrimmed, + "Expected default BytesTrimmed of 1 GiB") + assert.Equal(t, 500*time.Millisecond, result.Duration, + "Expected default Duration of 500ms") +} + +// Test ID: U-031 +// Mock blkdiscard() returns default BlkdiscardResult when no overrides are set. +func TestMockBlkdiscard_DefaultResult(t *testing.T) { + resetGOFSMock(t) + m := &mockfs{} + + ctx := context.Background() + result, err := m.blkdiscard(ctx, "/dev/sda") + + require.NoError(t, err) + require.NotNil(t, result, "Expected non-nil default mock result") + assert.Equal(t, int64(107374182400), result.BytesDiscarded, + "Expected default BytesDiscarded of 100 GiB") + assert.Equal(t, 2*time.Second, result.Duration, + "Expected default Duration of 2s") +} + +// Test ID: U-032 +// Mock checkDiscardSupport() returns default DiscardCapability when no overrides are set. +func TestMockCheckDiscardSupport_DefaultResult(t *testing.T) { + resetGOFSMock(t) + m := &mockfs{} + + ctx := context.Background() + result, err := m.checkDiscardSupport(ctx, "/dev/sda") + + require.NoError(t, err) + require.NotNil(t, result, "Expected non-nil default mock result") + assert.True(t, result.Supported, "Expected default Supported == true") + assert.Equal(t, int64(4294967295), result.DiscardMaxBytes, + "Expected default DiscardMaxBytes of 4294967295") + assert.Empty(t, result.Reason, "Expected empty Reason for supported device") +} + +// ---- Unit Tests: Type Zero Values (U-036 to U-038) ------------------------- + +// Test ID: U-036 +// FstrimResult zero value has BytesTrimmed == 0 and Duration == 0. +func TestFstrimResult_ZeroValue(t *testing.T) { + result := FstrimResult{} + assert.Equal(t, int64(0), result.BytesTrimmed, + "Zero-value FstrimResult should have BytesTrimmed == 0") + assert.Equal(t, time.Duration(0), result.Duration, + "Zero-value FstrimResult should have Duration == 0") +} + +// Test ID: U-037 +// BlkdiscardResult zero value has BytesDiscarded == 0 and Duration == 0. +func TestBlkdiscardResult_ZeroValue(t *testing.T) { + result := BlkdiscardResult{} + assert.Equal(t, int64(0), result.BytesDiscarded, + "Zero-value BlkdiscardResult should have BytesDiscarded == 0") + assert.Equal(t, time.Duration(0), result.Duration, + "Zero-value BlkdiscardResult should have Duration == 0") +} + +// Test ID: U-038 +// DiscardCapability zero value has Supported == false, DiscardMaxBytes == 0, Reason == "". +func TestDiscardCapability_ZeroValue(t *testing.T) { + dc := DiscardCapability{} + assert.False(t, dc.Supported, + "Zero-value DiscardCapability should have Supported == false") + assert.Equal(t, int64(0), dc.DiscardMaxBytes, + "Zero-value DiscardCapability should have DiscardMaxBytes == 0") + assert.Equal(t, "", dc.Reason, + "Zero-value DiscardCapability should have Reason == empty string") +} diff --git a/gofsutil_test.go b/gofsutil_test.go index 66b6d36..b85d7cf 100644 --- a/gofsutil_test.go +++ b/gofsutil_test.go @@ -367,10 +367,11 @@ func TestFindFSType(t *testing.T) { mountpoint := "/mnt/test" result, err := FindFSType(ctx, mountpoint) - if err != nil { - t.Errorf("expected no error, got %v", err) + // Expect an error since /mnt/test doesn't exist and findmnt will fail + if err == nil { + t.Errorf("expected error for non-existent mountpoint, got nil") } - t.Logf("Filesystem type: %s", result) + t.Logf("Filesystem type: %s, error: %v", result, err) } func TestDeviceRescan(t *testing.T) { diff --git a/gofsutil_utils.go b/gofsutil_utils.go index f9515ba..addf416 100644 --- a/gofsutil_utils.go +++ b/gofsutil_utils.go @@ -64,3 +64,23 @@ func validateMultipathArgs(options ...string) error { return nil } + +// validateDeviceID validates device identifiers to prevent OS command injection. +// Device IDs should only contain alphanumeric characters, underscores, hyphens, and dots. +// This follows the same pattern used in goiscsi for CVE-2022-34374 (DSA-2022-202). +func validateDeviceID(devID string) error { + if devID == "" { + return errors.New("device ID cannot be empty") + } + // Allow only alphanumeric characters, underscores, hyphens, and dots + // This covers valid device names like: sda, sda1, nvme0n1, mpath0, emcpowera, + // dm-0, 3600601xxxxxxx, vol-abc123, etc. + matched, err := regexp.MatchString(`^[a-zA-Z0-9_\-\.]+$`, devID) + if err != nil { + return errors.New("failed to validate device ID: " + err.Error()) + } + if !matched { + return errors.New("device ID contains invalid characters: " + devID) + } + return nil +} diff --git a/gofsutil_utils_test.go b/gofsutil_utils_test.go index 4032be4..2848590 100644 --- a/gofsutil_utils_test.go +++ b/gofsutil_utils_test.go @@ -196,3 +196,269 @@ func TestValidateMultipathArgs(t *testing.T) { }) } } + +// TestValidateDeviceID tests the validateDeviceID function for OS command injection prevention +// This test covers CVE-2022-34374 style vulnerabilities (CWE-78) +func TestValidateDeviceID(t *testing.T) { + tests := []struct { + name string + devID string + wantError bool + errorMsg string + }{ + // Valid device IDs + { + name: "valid simple device", + devID: "sda", + wantError: false, + }, + { + name: "valid device with number", + devID: "sda1", + wantError: false, + }, + { + name: "valid nvme device", + devID: "nvme0n1", + wantError: false, + }, + { + name: "valid nvme partition", + devID: "nvme0n1p1", + wantError: false, + }, + { + name: "valid mpath device", + devID: "mpath0", + wantError: false, + }, + { + name: "valid mpath device with letters", + devID: "mpatha", + wantError: false, + }, + { + name: "valid emcpower device", + devID: "emcpowera", + wantError: false, + }, + { + name: "valid dm device", + devID: "dm-0", + wantError: false, + }, + { + name: "valid device with underscore", + devID: "vol_abc123", + wantError: false, + }, + { + name: "valid device with hyphen", + devID: "vol-abc123", + wantError: false, + }, + { + name: "valid device with dot", + devID: "vol.abc123", + wantError: false, + }, + { + name: "valid WWN-style ID", + devID: "3600601xxxxxxx", + wantError: false, + }, + { + name: "valid long alphanumeric", + devID: "60000970000120001263533030313434", + wantError: false, + }, + // Invalid device IDs - command injection attempts + { + name: "empty device ID", + devID: "", + wantError: true, + errorMsg: "device ID cannot be empty", + }, + { + name: "single quote injection", + devID: "x'; id; echo '", + wantError: true, + errorMsg: "device ID contains invalid characters", + }, + { + name: "semicolon injection", + devID: "sda; rm -rf /", + wantError: true, + errorMsg: "device ID contains invalid characters", + }, + { + name: "backtick injection", + devID: "sda`id`", + wantError: true, + errorMsg: "device ID contains invalid characters", + }, + { + name: "dollar sign injection", + devID: "sda$(id)", + wantError: true, + errorMsg: "device ID contains invalid characters", + }, + { + name: "pipe injection", + devID: "sda|cat /etc/passwd", + wantError: true, + errorMsg: "device ID contains invalid characters", + }, + { + name: "ampersand injection", + devID: "sda&id", + wantError: true, + errorMsg: "device ID contains invalid characters", + }, + { + name: "newline injection", + devID: "sda\nid", + wantError: true, + errorMsg: "device ID contains invalid characters", + }, + { + name: "space injection", + devID: "sda id", + wantError: true, + errorMsg: "device ID contains invalid characters", + }, + { + name: "forward slash", + devID: "/dev/sda", + wantError: true, + errorMsg: "device ID contains invalid characters", + }, + { + name: "backslash", + devID: "sda\\nid", + wantError: true, + errorMsg: "device ID contains invalid characters", + }, + { + name: "double quote injection", + devID: `sda"; id; echo "`, + wantError: true, + errorMsg: "device ID contains invalid characters", + }, + { + name: "greater than redirect", + devID: "sda>/tmp/pwned", + wantError: true, + errorMsg: "device ID contains invalid characters", + }, + { + name: "less than redirect", + devID: "sda