Restructuring IDRange

olljanat · Oct 28, 2024 · e1c226e · e1c226e
1 parent 0fbf592
commit e1c226e
Show file tree

Hide file tree

Showing 7 changed files with 41 additions and 48 deletions.
diff --git a/drivers/exec/driver.go b/drivers/exec/driver.go
@@ -24,7 +24,6 @@ import (
 	"github.com/hashicorp/nomad/helper/pluginutils/loader"
 	"github.com/hashicorp/nomad/helper/pointer"
 	"github.com/hashicorp/nomad/helper/users"
-	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/hashicorp/nomad/plugins/base"
 	"github.com/hashicorp/nomad/plugins/drivers"
 	"github.com/hashicorp/nomad/plugins/drivers/fsisolation"
@@ -169,10 +168,10 @@ type Config struct {
 	DeniedHostGidsStr string `codec:"denied_host_gids"`
 
 	// DeniedHostUids configures which host uids are disallowed
-	DeniedHostUids []structs.IDRange `codec:"-"`
+	DeniedHostUids []validators.IDRange `codec:"-"`
 
 	// DeniedHostGids configures which host gids are disallowed
-	DeniedHostGids []structs.IDRange `codec:"-"`
+	DeniedHostGids []validators.IDRange `codec:"-"`
 }
 
 func (c *Config) validate() error {
@@ -264,10 +263,9 @@ func (tc *TaskConfig) validate() error {
 }
 
 func (tc *TaskConfig) validateUserIds(cfg *drivers.TaskConfig, driverConfig *Config) error {
-	usernameToLookup := getUsername(cfg)
-	user, err := users.Lookup(usernameToLookup)
+	user, err := users.Lookup(cfg.User)
 	if err != nil {
-		return fmt.Errorf("failed to identify user %q: %w", usernameToLookup, err)
+		return fmt.Errorf("failed to identify user %q: %w", cfg.User, err)
 	}
 
 	return validators.HasValidIds(user, driverConfig.DeniedHostUids, driverConfig.DeniedHostGids)
@@ -483,6 +481,13 @@ func (d *Driver) StartTask(cfg *drivers.TaskConfig) (*drivers.TaskHandle, *drive
 		return nil, nil, fmt.Errorf("failed driver config validation: %v", err)
 	}
 
+	if cfg.User == "" {
+		fmt.Println("User is not empty, setting to nobody")
+		cfg.User = "nobody"
+	} else {
+		fmt.Printf("User is not empty, user is %s\n", cfg.User)
+	}
+
 	if err := driverConfig.validateUserIds(cfg, &d.config); err != nil {
 		return nil, nil, fmt.Errorf("failed host user validation: %v", err)
 	}
@@ -506,7 +511,7 @@ func (d *Driver) StartTask(cfg *drivers.TaskConfig) (*drivers.TaskHandle, *drive
 		return nil, nil, fmt.Errorf("failed to create executor: %v", err)
 	}
 
-	user := getUsername(cfg)
+	user := cfg.User
 
 	if cfg.DNS != nil {
 		dnsMount, err := resolvconf.GenerateDNSMount(cfg.TaskDir().Dir, cfg.DNS)
@@ -736,12 +741,3 @@ func (d *Driver) ExecTaskStreamingRaw(ctx context.Context,
 
 	return handle.exec.ExecStreaming(ctx, command, tty, stream)
 }
-
-func getUsername(cfg *drivers.TaskConfig) string {
-	username := "nobody"
-	if cfg.User != "" {
-		username = cfg.User
-	}
-
-	return username
-}
diff --git a/drivers/exec/driver_test.go b/drivers/exec/driver_test.go
@@ -23,6 +23,7 @@ import (
 	"github.com/hashicorp/nomad/client/lib/numalib"
 	ctestutils "github.com/hashicorp/nomad/client/testutil"
 	"github.com/hashicorp/nomad/drivers/shared/executor"
+	"github.com/hashicorp/nomad/drivers/shared/validators"
 	"github.com/hashicorp/nomad/helper/pluginutils/hclutils"
 	"github.com/hashicorp/nomad/helper/testlog"
 	"github.com/hashicorp/nomad/helper/testtask"
@@ -916,13 +917,13 @@ func TestDriver_TaskConfig_validateUserIds(t *testing.T) {
 	nobodyUid, _, _, err := users.LookupUnix("nobody")
 	require.NoError(t, err)
 
-	allowAll := []structs.IDRange{}
-	denyCurrent := []structs.IDRange{{Lower: uint64(currentUid), Upper: uint64(currentUid)}}
-	denyNobody := []structs.IDRange{{Lower: uint64(nobodyUid), Upper: uint64(nobodyUid)}}
+	allowAll := []validators.IDRange{}
+	denyCurrent := []validators.IDRange{{Lower: uint64(currentUid), Upper: uint64(currentUid)}}
+	denyNobody := []validators.IDRange{{Lower: uint64(nobodyUid), Upper: uint64(nobodyUid)}}
 	configAllowCurrent := Config{DeniedHostUids: allowAll}
 	configDenyCurrent := Config{DeniedHostUids: denyCurrent}
 	configDenyAnonymous := Config{DeniedHostUids: denyNobody}
-	driverConfigNoUserSpecified := drivers.TaskConfig{}
+	driverConfigNoUserSpecified := drivers.TaskConfig{User: "nobody"}
 	driverConfigSpecifyCurrent := drivers.TaskConfig{User: current.Name}
 	currentUserErrStr := fmt.Sprintf("running as uid %d is disallowed", currentUid)
 	anonUserErrStr := fmt.Sprintf("running as uid %d is disallowed", nobodyUid)
@@ -939,9 +940,9 @@ func TestDriver_TaskConfig_validateUserIds(t *testing.T) {
 	} {
 		err := (&TaskConfig{}).validateUserIds(&tc.driverConfig, &tc.config)
 		if tc.expectedErr == "" {
-			require.NoError(t, err)
+			must.NoError(t, err)
 		} else {
-			require.ErrorContains(t, err, tc.expectedErr)
+			must.ErrorContains(t, err, tc.expectedErr)
 		}
 	}
 }

diff --git a/drivers/rawexec/driver.go b/drivers/rawexec/driver.go
@@ -21,7 +21,6 @@ import (
 	"github.com/hashicorp/nomad/helper/pluginutils/hclutils"
 	"github.com/hashicorp/nomad/drivers/shared/validators"
 	"github.com/hashicorp/nomad/helper/pluginutils/loader"
-	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/hashicorp/nomad/plugins/base"
 	"github.com/hashicorp/nomad/plugins/drivers"
 	"github.com/hashicorp/nomad/plugins/drivers/fsisolation"
@@ -148,10 +147,10 @@ type Config struct {
 	DeniedHostGidsStr string `codec:"denied_host_gids"`
 
 	// DeniedHostUids configures which host uids are disallowed
-	DeniedHostUids []structs.IDRange
+	DeniedHostUids []validators.IDRange
 
 	// DeniedHostGids configures which host gids are disallowed
-	DeniedHostGids []structs.IDRange
+	DeniedHostGids []validators.IDRange
 }
 
 // TaskConfig is the driver configuration of a task within a job

diff --git a/drivers/rawexec/driver_unix_test.go b/drivers/rawexec/driver_unix_test.go
@@ -22,10 +22,10 @@ import (
 
 	"github.com/hashicorp/nomad/ci"
 	clienttestutil "github.com/hashicorp/nomad/client/testutil"
+	"github.com/hashicorp/nomad/drivers/shared/validators"
 	"github.com/hashicorp/nomad/helper/testtask"
 	"github.com/hashicorp/nomad/helper/users"
 	"github.com/hashicorp/nomad/helper/uuid"
-	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/hashicorp/nomad/plugins/base"
 	basePlug "github.com/hashicorp/nomad/plugins/base"
 	"github.com/hashicorp/nomad/plugins/drivers"
@@ -558,8 +558,8 @@ func TestRawExec_Validate(t *testing.T) {
 
 	currentUserErrStr := fmt.Sprintf("running as uid %d is disallowed", currentUid)
 
-	allowAll := []structs.IDRange{}
-	denyCurrent := []structs.IDRange{{Lower: currentUid, Upper: currentUid}}
+	allowAll := []validators.IDRange{}
+	denyCurrent := []validators.IDRange{{Lower: currentUid, Upper: currentUid}}
 	configAllowCurrent := Config{DeniedHostUids: allowAll}
 	configDenyCurrent := Config{DeniedHostUids: denyCurrent}
 	driverConfigNoUserSpecified := drivers.TaskConfig{}

diff --git a/drivers/shared/validators/validators.go b/drivers/shared/validators/validators.go
@@ -8,13 +8,17 @@ import (
 	"os/user"
 	"strconv"
 	"strings"
-
-	"github.com/hashicorp/nomad/nomad/structs"
 )
 
+// IDRange defines a range of uids or gids (to eventually restrict)
+type IDRange struct {
+	Lower uint64 `codec:"from"`
+	Upper uint64 `codec:"to"`
+}
+
 // ParseIdRange is used to ensure that the configuration for ID ranges is valid.
-func ParseIdRange(rangeType string, deniedRanges string) ([]structs.IDRange, error) {
-	var idRanges []structs.IDRange
+func ParseIdRange(rangeType string, deniedRanges string) ([]IDRange, error) {
+	var idRanges []IDRange
 	parts := strings.Split(deniedRanges, ",")
 
 	// exit early if empty string
@@ -36,7 +40,7 @@ func ParseIdRange(rangeType string, deniedRanges string) ([]structs.IDRange, err
 
 // HasValidIds is used when running a task to ensure the
 // given user is in the ID range defined in the task config
-func HasValidIds(user *user.User, deniedHostUIDs, deniedHostGIDs []structs.IDRange) error {
+func HasValidIds(user *user.User, deniedHostUIDs, deniedHostGIDs []IDRange) error {
 	uid, err := strconv.ParseUint(user.Uid, 10, 32)
 	if err != nil {
 		return fmt.Errorf("unable to convert userid %s to integer", user.Uid)
@@ -78,10 +82,10 @@ func HasValidIds(user *user.User, deniedHostUIDs, deniedHostGIDs []structs.IDRan
 	return nil
 }
 
-func parseRangeString(boundsString string) (*structs.IDRange, error) {
+func parseRangeString(boundsString string) (*IDRange, error) {
 	uidDenyRangeParts := strings.Split(boundsString, "-")
 
-	var idRange structs.IDRange
+	var idRange IDRange
 
 	switch len(uidDenyRangeParts) {
 	case 0:

diff --git a/drivers/shared/validators/validators_test.go b/drivers/shared/validators/validators_test.go
@@ -7,7 +7,6 @@ import (
 	"os/user"
 	"testing"
 
-	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/shoenig/test/must"
 )
 
@@ -47,23 +46,23 @@ func Test_IDRangeValid(t *testing.T) {
 }
 
 func Test_HasValidIds(t *testing.T) {
-	var validRange = structs.IDRange{
+	var validRange = IDRange{
 		Lower: 1,
 		Upper: 100,
 	}
 
-	var validRangeSingle = structs.IDRange{
+	var validRangeSingle = IDRange{
 		Lower: 1,
 		Upper: 1,
 	}
 
-	emptyRanges := []structs.IDRange{}
-	validRangesList := []structs.IDRange{validRange, validRangeSingle}
+	emptyRanges := []IDRange{}
+	validRangesList := []IDRange{validRange, validRangeSingle}
 
 	testCases := []struct {
 		name        string
-		uidRanges   []structs.IDRange
-		gidRanges   []structs.IDRange
+		uidRanges   []IDRange
+		gidRanges   []IDRange
 		uid         string
 		gid         string
 		expectedErr string

diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go
@@ -13756,9 +13756,3 @@ func NewRpcError(err error, code *int64) *RpcError {
 func (r *RpcError) Error() string {
 	return r.Message
 }
-
-// IDRange defines a range of uids or gids (to eventually restrict)
-type IDRange struct {
-	Lower uint64 `codec:"from"`
-	Upper uint64 `codec:"to"`
-}