linstor: use user and group name instead of id

Recent LINSTOR versions support passing the user id and group as names instead of UID and GID. This means that we can pass "nobody:nobody" directly instead of making a guess about what the UID and GID will be. Try to be backward compatible by parsing back the UID and GID if the MkfsParams property exists and passing the numbers back to LINSTOR as user and group strings.
LINBIT · Jul 1, 2024 · 75a1cff · 75a1cff
1 parent 86b70a5
commit 75a1cff
Show file tree

Hide file tree

Showing 6 changed files with 46 additions and 44 deletions.
diff --git a/cmd/nfs.go b/cmd/nfs.go
@@ -117,7 +117,7 @@ linstor-gateway nfs create restricted 10.10.22.44/16 2G --allowed-ips 10.10.0.0/
 						Number:              i + 1,
 						SizeKiB:             uint64(val.Value / unit.K),
 						FileSystem:          filesystem,
-						FileSystemRootOwner: common.UidGid{Uid: 65534, Gid: 65534}, // corresponds to "nobody:nobody"
+						FileSystemRootOwner: common.UserGroup{User: "nobody", Group: "nobody"},
 					},
 				})
 			}

diff --git a/pkg/common/resource.go b/pkg/common/resource.go
@@ -10,20 +10,20 @@ import (
 	"github.com/LINBIT/golinstor/client"
 )
 
-type UidGid struct {
-	Uid int
-	Gid int
+type UserGroup struct {
+	User  string
+	Group string
 }
 
-func (u *UidGid) String() string {
-	return fmt.Sprintf("%d:%d", u.Uid, u.Gid)
+func (u *UserGroup) String() string {
+	return fmt.Sprintf("%s:%s", u.User, u.Group)
 }
 
 type VolumeConfig struct {
-	Number              int    `json:"number"`
-	SizeKiB             uint64 `json:"size_kib"`
-	FileSystem          string `json:"file_system,omitempty"`
-	FileSystemRootOwner UidGid `json:"file_system_root_owner,omitempty"`
+	Number              int       `json:"number"`
+	SizeKiB             uint64    `json:"size_kib"`
+	FileSystem          string    `json:"file_system,omitempty"`
+	FileSystemRootOwner UserGroup `json:"file_system_root_owner,omitempty"`
 }
 
 type ResourceStatus struct {

diff --git a/pkg/common/resource_config.go b/pkg/common/resource_config.go
@@ -2,12 +2,14 @@ package common
 
 import (
 	"fmt"
-	"github.com/LINBIT/golinstor/client"
-	"github.com/LINBIT/linstor-gateway/pkg/reactor"
-	log "github.com/sirupsen/logrus"
 	"net"
 	"path/filepath"
 	"strings"
+
+	"github.com/LINBIT/golinstor/client"
+	log "github.com/sirupsen/logrus"
+
+	"github.com/LINBIT/linstor-gateway/pkg/reactor"
 )
 
 const (
@@ -37,7 +39,7 @@ func ClusterPrivateVolume() VolumeConfig {
 		Number:              0,
 		SizeKiB:             clusterPrivateVolumeSizeKiB,
 		FileSystem:          clusterPrivateVolumeFileSystem,
-		FileSystemRootOwner: UidGid{Uid: 0, Gid: 0},
+		FileSystemRootOwner: UserGroup{User: "root", Group: "root"},
 	}
 }
 

diff --git a/pkg/linstorcontrol/linstorcontrol.go b/pkg/linstorcontrol/linstorcontrol.go
@@ -7,7 +7,6 @@ import (
 	"errors"
 	"fmt"
 	"sort"
-	"strconv"
 
 	"github.com/icza/gog"
 
@@ -219,8 +218,8 @@ func (l *Linstor) EnsureResource(ctx context.Context, res Resource, mayExist boo
 		volProps := map[string]string{}
 		if vol.FileSystem != "" {
 			volProps[apiconsts.NamespcFilesystem+"/"+apiconsts.KeyFsType] = vol.FileSystem
-			volProps[apiconsts.NamespcFilesystem+"/"+apiconsts.KeyFsUser] = strconv.Itoa(vol.FileSystemRootOwner.Uid)
-			volProps[apiconsts.NamespcFilesystem+"/"+apiconsts.KeyFsGroup] = strconv.Itoa(vol.FileSystemRootOwner.Gid)
+			volProps[apiconsts.NamespcFilesystem+"/"+apiconsts.KeyFsUser] = vol.FileSystemRootOwner.User
+			volProps[apiconsts.NamespcFilesystem+"/"+apiconsts.KeyFsGroup] = vol.FileSystemRootOwner.Group
 		}
 		var volFlags []string
 		if res.GrossSize {

diff --git a/pkg/nfs/resource_config.go b/pkg/nfs/resource_config.go
@@ -153,22 +153,16 @@ func FromPromoter(cfg *reactor.PromoterConfig, definition *client.ResourceDefini
 	return r, nil
 }
 
-func parseRootOwner(mkfsParams string) (common.UidGid, error) {
+func parseRootOwner(mkfsParams string) (common.UserGroup, error) {
 	rootOwnerRegex := regexp.MustCompile(`-E root_owner=(\d+):(\d+)`)
 	matches := rootOwnerRegex.FindStringSubmatch(mkfsParams)
 	if len(matches) != 3 {
-		return common.UidGid{}, fmt.Errorf("failed to parse uid and gid from MkfsParams: %q", mkfsParams)
+		return common.UserGroup{}, fmt.Errorf("failed to parse uid and gid from MkfsParams: %q", mkfsParams)
 	}
-
-	uid, err := strconv.Atoi(matches[1])
-	if err != nil {
-		return common.UidGid{}, fmt.Errorf("failed to parse uid from MkfsParams: %q", matches[1])
-	}
-	gid, err := strconv.Atoi(matches[2])
-	if err != nil {
-		return common.UidGid{}, fmt.Errorf("failed to parse gid from MkfsParams: %q", matches[2])
-	}
-	return common.UidGid{Uid: uid, Gid: gid}, nil
+	return common.UserGroup{
+		User:  matches[1],
+		Group: matches[2],
+	}, nil
 }
 
 // parseVolume converts a resource agent of the type "ocf:heartbeat:Filesystem"
@@ -203,16 +197,22 @@ func parseVolume(agent *reactor.ResourceAgent, volumes []client.VolumeDefinition
 	if val, ok := vol.Props[apiconsts.NamespcFilesystem+"/Type"]; ok {
 		filesystem = val
 	}
-	var rootOwner common.UidGid
-	if val, ok := vol.Props[apiconsts.NamespcFilesystem+"/MkfsParams"]; ok {
-		rootOwner, err = parseRootOwner(val)
-		if err != nil {
-			log.WithFields(log.Fields{
-				"err":      err,
-				"volume":   vol.VolumeNumber,
-				"resource": resName,
-			}).Warnf("invalid MkfsParams for volume: %q", val)
+	var rootOwner common.UserGroup
+	if val, ok := vol.Props[apiconsts.NamespcFilesystem+apiconsts.KeyFsUser]; ok {
+		rootOwner.User = val
+	}
+	if val, ok := vol.Props[apiconsts.NamespcFilesystem+apiconsts.KeyFsGroup]; ok {
+		rootOwner.Group = val
+	}
+	if rootOwner.User == "" || rootOwner.Group == "" {
+		// for backwards compatibility, we also support the root_owner parameter in MkfsParams
+		if val, ok := vol.Props[apiconsts.NamespcFilesystem+apiconsts.KeyFsMkfsparameters]; ok {
+			rootOwner, err = parseRootOwner(val)
+			if err != nil {
+				return nil, err
+			}
 		}
+
 	}
 	if vol.VolumeNumber == nil {
 		vol.VolumeNumber = gog.Ptr(int32(0))

diff --git a/pkg/nfs/resource_config_test.go b/pkg/nfs/resource_config_test.go
@@ -17,8 +17,9 @@ import (
 
 func filesystemProps(vol VolumeConfig) map[string]string {
 	return map[string]string{
-		apiconsts.NamespcFilesystem + "/Type":       vol.FileSystem,
-		apiconsts.NamespcFilesystem + "/MkfsParams": "-E nodiscard -E root_owner=" + vol.FileSystemRootOwner.String(),
+		apiconsts.NamespcFilesystem + "/Type":              vol.FileSystem,
+		apiconsts.NamespcFilesystem + apiconsts.KeyFsUser:  vol.FileSystemRootOwner.User,
+		apiconsts.NamespcFilesystem + apiconsts.KeyFsGroup: vol.FileSystemRootOwner.Group,
 	}
 }
 
@@ -38,7 +39,7 @@ func TestResource_RoundTrip(t *testing.T) {
 					Number:              1,
 					SizeKiB:             1024,
 					FileSystem:          "ext4",
-					FileSystemRootOwner: common.UidGid{Uid: 47, Gid: 11},
+					FileSystemRootOwner: common.UserGroup{User: "someone", Group: "somegroup"},
 				},
 				ExportPath: "/",
 			},
@@ -273,7 +274,7 @@ func TestParseRootOwner(t *testing.T) {
 	tests := []struct {
 		name    string
 		input   string
-		want    common.UidGid
+		want    common.UserGroup
 		wantErr bool
 	}{{
 		name:    "empty",
@@ -286,11 +287,11 @@ func TestParseRootOwner(t *testing.T) {
 	}, {
 		name:  "normal",
 		input: "-E root_owner=123:456",
-		want:  common.UidGid{Uid: 123, Gid: 456},
+		want:  common.UserGroup{User: "123", Group: "456"},
 	}, {
 		name:  "other_text",
 		input: "-E something=else -E root_owner=123:456 -E other=stuff",
-		want:  common.UidGid{Uid: 123, Gid: 456},
+		want:  common.UserGroup{User: "123", Group: "456"},
 	}}
 	for i := range tests {
 		tcase := &tests[i]