Merge 6de51293ae into 4d32635344

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, ",")
 }

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)