Skip to content

Commit a26c67b

Browse files
authored
[runner] Rework and fix user processing (#3456)
* Drop --home-dir option, use process user's home dir instead * Fix ownership of Git credentials, consider Git credentials errors non-fatal Closes: #3419
1 parent 8b383ba commit a26c67b

File tree

17 files changed

+715
-379
lines changed

17 files changed

+715
-379
lines changed

runner/cmd/runner/main.go

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616

1717
"github.com/dstackai/dstack/runner/consts"
1818
"github.com/dstackai/dstack/runner/internal/executor"
19+
linuxuser "github.com/dstackai/dstack/runner/internal/linux/user"
1920
"github.com/dstackai/dstack/runner/internal/log"
2021
"github.com/dstackai/dstack/runner/internal/runner/api"
2122
"github.com/dstackai/dstack/runner/internal/ssh"
@@ -30,7 +31,6 @@ func main() {
3031

3132
func mainInner() int {
3233
var tempDir string
33-
var homeDir string
3434
var httpPort int
3535
var sshPort int
3636
var sshAuthorizedKeys []string
@@ -61,13 +61,6 @@ func mainInner() int {
6161
Destination: &tempDir,
6262
TakesFile: true,
6363
},
64-
&cli.StringFlag{
65-
Name: "home-dir",
66-
Usage: "HomeDir directory for credentials and $HOME",
67-
Value: consts.RunnerHomeDir,
68-
Destination: &homeDir,
69-
TakesFile: true,
70-
},
7164
&cli.IntFlag{
7265
Name: "http-port",
7366
Usage: "Set a http port",
@@ -87,7 +80,7 @@ func mainInner() int {
8780
},
8881
},
8982
Action: func(ctx context.Context, cmd *cli.Command) error {
90-
return start(ctx, tempDir, homeDir, httpPort, sshPort, sshAuthorizedKeys, logLevel, Version)
83+
return start(ctx, tempDir, httpPort, sshPort, sshAuthorizedKeys, logLevel, Version)
9184
},
9285
},
9386
},
@@ -104,7 +97,7 @@ func mainInner() int {
10497
return 0
10598
}
10699

107-
func start(ctx context.Context, tempDir string, homeDir string, httpPort int, sshPort int, sshAuthorizedKeys []string, logLevel int, version string) error {
100+
func start(ctx context.Context, tempDir string, httpPort int, sshPort int, sshAuthorizedKeys []string, logLevel int, version string) error {
108101
if err := os.MkdirAll(tempDir, 0o755); err != nil {
109102
return fmt.Errorf("create temp directory: %w", err)
110103
}
@@ -114,15 +107,39 @@ func start(ctx context.Context, tempDir string, homeDir string, httpPort int, ss
114107
return fmt.Errorf("create default log file: %w", err)
115108
}
116109
defer func() {
117-
closeErr := defaultLogFile.Close()
118-
if closeErr != nil {
119-
log.Error(ctx, "Failed to close default log file", "err", closeErr)
110+
if err := defaultLogFile.Close(); err != nil {
111+
log.Error(ctx, "Failed to close default log file", "err", err)
120112
}
121113
}()
122-
123114
log.DefaultEntry.Logger.SetOutput(io.MultiWriter(os.Stdout, defaultLogFile))
124115
log.DefaultEntry.Logger.SetLevel(logrus.Level(logLevel))
125116

117+
currentUser, err := linuxuser.FromCurrentProcess()
118+
if err != nil {
119+
return fmt.Errorf("get current process user: %w", err)
120+
}
121+
if !currentUser.IsRoot() {
122+
return fmt.Errorf("must be root: %s", currentUser)
123+
}
124+
if currentUser.HomeDir == "" {
125+
log.Warning(ctx, "Current user does not have home dir, using /root as a fallback", "user", currentUser)
126+
currentUser.HomeDir = "/root"
127+
}
128+
// Fix the current process HOME, just in case some internals require it (e.g., they use os.UserHomeDir() or
129+
// spawn a child process which uses that variable)
130+
envHome, envHomeIsSet := os.LookupEnv("HOME")
131+
if envHome != currentUser.HomeDir {
132+
if !envHomeIsSet {
133+
log.Warning(ctx, "HOME is not set, setting the value", "home", currentUser.HomeDir)
134+
} else {
135+
log.Warning(ctx, "HOME is incorrect, fixing the value", "current", envHome, "home", currentUser.HomeDir)
136+
}
137+
if err := os.Setenv("HOME", currentUser.HomeDir); err != nil {
138+
return fmt.Errorf("set HOME: %w", err)
139+
}
140+
}
141+
log.Trace(ctx, "Running as", "user", currentUser)
142+
126143
// NB: The Mkdir/Chown/Chmod code below relies on the fact that RunnerDstackDir path is _not_ nested (/dstack).
127144
// Adjust it if the path is changed to, e.g., /opt/dstack
128145
const dstackDir = consts.RunnerDstackDir
@@ -163,7 +180,7 @@ func start(ctx context.Context, tempDir string, homeDir string, httpPort int, ss
163180
}
164181
}()
165182

166-
ex, err := executor.NewRunExecutor(tempDir, homeDir, dstackDir, sshd)
183+
ex, err := executor.NewRunExecutor(tempDir, dstackDir, *currentUser, sshd)
167184
if err != nil {
168185
return fmt.Errorf("create executor: %w", err)
169186
}

runner/consts/consts.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,6 @@ const (
2626
// NOTE: RunnerRuntimeDir would be a more appropriate name, but it's called tempDir
2727
// throughout runner's codebase
2828
RunnerTempDir = "/tmp/runner"
29-
// Currently, it's a directory where authorized_keys, git credentials, etc. are placed
30-
// The current user's homedir (as of 2024-12-28, it's always root) should be used
31-
// instead of the hardcoded value
32-
RunnerHomeDir = "/root"
3329
// A directory for:
3430
// 1. Files used by the runner and related components (e.g., sshd stores its config and log inside /dstack/ssh)
3531
// 2. Files shared between users (e.g., sshd authorized_keys, MPI hostfile)

runner/internal/common/utils.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func ExpandPath(pth string, base string, home string) (string, error) {
4949
return pth, nil
5050
}
5151

52-
func MkdirAll(ctx context.Context, pth string, uid int, gid int) error {
52+
func MkdirAll(ctx context.Context, pth string, uid int, gid int, perm os.FileMode) error {
5353
paths := []string{pth}
5454
for {
5555
pth = path.Dir(pth)
@@ -60,7 +60,7 @@ func MkdirAll(ctx context.Context, pth string, uid int, gid int) error {
6060
}
6161
for _, p := range slices.Backward(paths) {
6262
if _, err := os.Stat(p); errors.Is(err, os.ErrNotExist) {
63-
if err := os.Mkdir(p, 0o755); err != nil {
63+
if err := os.Mkdir(p, perm); err != nil {
6464
return err
6565
}
6666
if uid != -1 || gid != -1 {

runner/internal/common/utils_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,15 +120,15 @@ func TestExpandtPath_ErrorTildeUsernameNotSupported_TildeUsernameWithPath(t *tes
120120
func TestMkdirAll_AbsPath_NotExists(t *testing.T) {
121121
absPath := path.Join(t.TempDir(), "a/b/c")
122122
require.NoDirExists(t, absPath)
123-
err := MkdirAll(context.Background(), absPath, -1, -1)
123+
err := MkdirAll(context.Background(), absPath, -1, -1, 0o755)
124124
require.NoError(t, err)
125125
require.DirExists(t, absPath)
126126
}
127127

128128
func TestMkdirAll_AbsPath_Exists(t *testing.T) {
129129
absPath, err := os.Getwd()
130130
require.NoError(t, err)
131-
err = MkdirAll(context.Background(), absPath, -1, -1)
131+
err = MkdirAll(context.Background(), absPath, -1, -1, 0o755)
132132
require.NoError(t, err)
133133
require.DirExists(t, absPath)
134134
}
@@ -139,7 +139,7 @@ func TestMkdirAll_RelPath_NotExists(t *testing.T) {
139139
relPath := "a/b/c"
140140
absPath := path.Join(cwd, relPath)
141141
require.NoDirExists(t, absPath)
142-
err := MkdirAll(context.Background(), relPath, -1, -1)
142+
err := MkdirAll(context.Background(), relPath, -1, -1, 0o755)
143143
require.NoError(t, err)
144144
require.DirExists(t, absPath)
145145
}
@@ -151,7 +151,7 @@ func TestMkdirAll_RelPath_Exists(t *testing.T) {
151151
absPath := path.Join(cwd, relPath)
152152
err := os.MkdirAll(absPath, 0o755)
153153
require.NoError(t, err)
154-
err = MkdirAll(context.Background(), relPath, -1, -1)
154+
err = MkdirAll(context.Background(), relPath, -1, -1, 0o755)
155155
require.NoError(t, err)
156156
require.DirExists(t, absPath)
157157
}

0 commit comments

Comments
 (0)