diff --git a/collector/filesystem_linux.go b/collector/filesystem_linux.go index 3739f0fee8..a5c53df0a8 100644 --- a/collector/filesystem_linux.go +++ b/collector/filesystem_linux.go @@ -16,7 +16,6 @@ package collector import ( - "bytes" "errors" "fmt" "log/slog" @@ -237,13 +236,13 @@ func isFilesystemReadOnly(labels filesystemLabels) bool { } func mountOptionsString(m map[string]string) string { - b := new(bytes.Buffer) + parts := make([]string, 0, len(m)) for key, value := range m { if value == "" { - fmt.Fprintf(b, "%s", key) + parts = append(parts, key) } else { - fmt.Fprintf(b, "%s=%s", key, value) + parts = append(parts, key+"="+value) } } - return b.String() + return strings.Join(parts, ",") } diff --git a/collector/filesystem_linux_test.go b/collector/filesystem_linux_test.go index a838e93271..958dc97b9c 100644 --- a/collector/filesystem_linux_test.go +++ b/collector/filesystem_linux_test.go @@ -18,6 +18,8 @@ package collector import ( "io" "log/slog" + "sort" + "strings" "testing" "github.com/alecthomas/kingpin/v2" @@ -168,6 +170,88 @@ func TestMountsFallback(t *testing.T) { } } +func TestMountOptionsString(t *testing.T) { + tests := []struct { + name string + input map[string]string + wantParts []string + }{ + { + name: "single flag option", + input: map[string]string{"ro": ""}, + wantParts: []string{"ro"}, + }, + { + name: "single key=value option", + input: map[string]string{"errors": "remount-ro"}, + wantParts: []string{"errors=remount-ro"}, + }, + { + name: "multiple options including ro", + input: map[string]string{"ro": "", "relatime": "", "errors": "remount-ro"}, + wantParts: []string{"errors=remount-ro", "relatime", "ro"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := mountOptionsString(tt.input) + parts := strings.Split(result, ",") + sort.Strings(parts) + sort.Strings(tt.wantParts) + if len(parts) != len(tt.wantParts) { + t.Fatalf("mountOptionsString(%v) = %q, got %d parts, want %d", tt.input, result, len(parts), len(tt.wantParts)) + } + for i := range parts { + if parts[i] != tt.wantParts[i] { + t.Errorf("mountOptionsString(%v) = %q, sorted part[%d] = %q, want %q", tt.input, result, i, parts[i], tt.wantParts[i]) + } + } + }) + } +} + +func TestMountOptionsStringReadOnlyDetection(t *testing.T) { + tests := []struct { + name string + mountOptions map[string]string + superOptions map[string]string + wantReadOnly bool + }{ + { + name: "ro among multiple mount options (ZFS / remount-ro scenario)", + mountOptions: map[string]string{"ro": "", "relatime": "", "errors": "remount-ro"}, + superOptions: map[string]string{"rw": ""}, + wantReadOnly: true, + }, + { + name: "ro in super options only", + mountOptions: map[string]string{"rw": "", "relatime": ""}, + superOptions: map[string]string{"ro": "", "user_id": "1000"}, + wantReadOnly: true, + }, + { + name: "no ro anywhere", + mountOptions: map[string]string{"rw": "", "nosuid": "", "nodev": ""}, + superOptions: map[string]string{"rw": ""}, + wantReadOnly: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + labels := filesystemLabels{ + mountOptions: mountOptionsString(tt.mountOptions), + superOptions: mountOptionsString(tt.superOptions), + } + if got := isFilesystemReadOnly(labels); got != tt.wantReadOnly { + t.Errorf("isFilesystemReadOnly(%+v) = %v, want %v (mountOptions=%q superOptions=%q)", + tt, got, tt.wantReadOnly, labels.mountOptions, labels.superOptions) + } + }) + } +} + func TestPathRootfs(t *testing.T) { if _, err := kingpin.CommandLine.Parse([]string{"--path.procfs", "./fixtures_bindmount/proc", "--path.rootfs", "/host"}); err != nil { t.Fatal(err)