only call 'user.Current' when we really need to (#24156)

Calling 'user.Current' can be extremely slow in some instances, so avoid
it when possible.
This commit is contained in:
Andrew LeFevre
2023-04-13 21:16:26 +00:00
committed by GitHub
parent e8e9ad39ae
commit 94baceef87
7 changed files with 126 additions and 16 deletions
+2 -2
View File
@@ -23,7 +23,6 @@ import (
"io/fs"
"net"
"os"
"os/user"
"path/filepath"
"strings"
@@ -31,6 +30,7 @@ import (
"golang.org/x/crypto/ssh"
"gopkg.in/yaml.v2"
"github.com/gravitational/teleport/api/utils"
"github.com/gravitational/teleport/api/utils/keypaths"
"github.com/gravitational/teleport/api/utils/keys"
"github.com/gravitational/teleport/api/utils/sshutils"
@@ -314,7 +314,7 @@ func FullProfilePath(dir string) string {
// defaultProfilePath retrieves the default path of the TSH profile.
func defaultProfilePath() string {
home := os.TempDir()
if u, err := user.Current(); err == nil && u.HomeDir != "" {
if u, err := utils.CurrentUser(); err == nil && u.HomeDir != "" {
home = u.HomeDir
}
return filepath.Join(home, profileDir)
+64
View File
@@ -0,0 +1,64 @@
/*
Copyright 2023 Gravitational, Inc.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package utils
import (
"os/user"
"time"
"github.com/gravitational/trace"
)
const lookupTimeout = 10 * time.Second
type userGetter func() (*user.User, error)
type userResult struct {
user *user.User
err error
}
// CurrentUser is just like [user.Current], except an error is returned
// if the user lookup exceeds 10 seconds. This is because if
// [user.Current] is called on a domain joined host, the user lookup
// will contact potentially multiple domain controllers which can hang.
func CurrentUser() (*user.User, error) {
return currentUser(user.Current, lookupTimeout)
}
func currentUser(getUser userGetter, timeout time.Duration) (*user.User, error) {
result := make(chan userResult, 1)
go func() {
u, err := getUser()
result <- userResult{
user: u,
err: err,
}
}()
timer := time.NewTimer(timeout)
defer timer.Stop()
select {
case u := <-result:
return u.user, trace.Wrap(u.err)
case <-timer.C:
return nil, trace.LimitExceeded(
"looking up the current host user exceeded timeout, try explicitly specifying the Teleport user or host user with --user or --login",
)
}
}
+44
View File
@@ -0,0 +1,44 @@
/*
Copyright 2023 Gravitational, Inc.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package utils
import (
"os/user"
"testing"
"time"
"github.com/gravitational/trace"
"github.com/stretchr/testify/require"
)
func TestCurrentUser(t *testing.T) {
t.Parallel()
u, err := currentUser(func() (*user.User, error) {
time.Sleep(100 * time.Millisecond)
return nil, nil
}, 10*time.Millisecond)
require.Nil(t, u)
require.NotNil(t, err)
require.True(t, trace.IsLimitExceeded(err))
u, err = currentUser(func() (*user.User, error) {
return nil, nil
}, lookupTimeout)
require.Nil(t, u)
require.NoError(t, err)
}
+11 -8
View File
@@ -29,7 +29,6 @@ import (
"net/url"
"os"
"os/exec"
"os/user"
"path/filepath"
"runtime"
"strconv"
@@ -1727,7 +1726,7 @@ func (tc *TeleportClient) runShellOrCommandOnSingleNode(ctx context.Context, clt
if len(tc.Config.LocalForwardPorts) == 0 {
fmt.Println("Executing command locally without connecting to any servers. This makes no sense.")
}
return runLocalCommand(command)
return runLocalCommand(tc.Config.HostLogin, command)
}
if len(command) > 0 {
@@ -4152,7 +4151,7 @@ func connectToSSHAgent() agent.ExtendedAgent {
// Username returns the current user's username
func Username() (string, error) {
u, err := user.Current()
u, err := apiutils.CurrentUser()
if err != nil {
return "", trace.Wrap(err)
}
@@ -4325,18 +4324,22 @@ func ParseSearchKeywords(spec string, customDelimiter rune) []string {
// Executes the given command on the client machine (localhost). If no command is given,
// executes shell
func runLocalCommand(command []string) error {
func runLocalCommand(hostLogin string, command []string) error {
if len(command) == 0 {
user, err := user.Current()
if err != nil {
return trace.Wrap(err)
if hostLogin == "" {
user, err := apiutils.CurrentUser()
if err != nil {
return trace.Wrap(err)
}
hostLogin = user.Username
}
shell, err := shell.GetLoginShell(user.Username)
shell, err := shell.GetLoginShell(hostLogin)
if err != nil {
return trace.Wrap(err)
}
command = []string{shell}
}
cmd := exec.Command(command[0], command[1:]...)
cmd.Stderr = os.Stderr
cmd.Stdin = os.Stdin
+2 -2
View File
@@ -17,7 +17,6 @@ limitations under the License.
package mysql
import (
"os/user"
"path/filepath"
"strconv"
"strings"
@@ -25,6 +24,7 @@ import (
"github.com/gravitational/trace"
"gopkg.in/ini.v1"
"github.com/gravitational/teleport/api/utils"
"github.com/gravitational/teleport/lib/client/db/profile"
)
@@ -40,7 +40,7 @@ type OptionFile struct {
func DefaultConfigPath() (string, error) {
// Default location is .my.cnf file in the user's home directory.
usr, err := user.Current()
usr, err := utils.CurrentUser()
if err != nil {
return "", trace.ConvertSystemError(err)
}
+2 -2
View File
@@ -17,7 +17,6 @@ limitations under the License.
package postgres
import (
"os/user"
"path/filepath"
"strconv"
"strings"
@@ -25,6 +24,7 @@ import (
"github.com/gravitational/trace"
"gopkg.in/ini.v1"
"github.com/gravitational/teleport/api/utils"
"github.com/gravitational/teleport/lib/client/db/profile"
)
@@ -42,7 +42,7 @@ type ServiceFile struct {
func Load() (*ServiceFile, error) {
// Default location is .pg_service.conf file in the user's home directory.
// TODO(r0mant): Check PGSERVICEFILE and PGSYSCONFDIR env vars as well.
user, err := user.Current()
user, err := utils.CurrentUser()
if err != nil {
return nil, trace.ConvertSystemError(err)
}
+1 -2
View File
@@ -595,10 +595,9 @@ func Run(ctx context.Context, args []string, opts ...cliOption) error {
app := utils.InitCLIParser("tsh", "Teleport Command Line Client").Interspersed(true)
app.Flag("login", "Remote host login").Short('l').Envar(loginEnvVar).StringVar(&cf.NodeLogin)
localUser, _ := client.Username()
app.Flag("proxy", "Teleport proxy address").Envar(proxyEnvVar).StringVar(&cf.Proxy)
app.Flag("nocache", "do not cache cluster discovery locally").Hidden().BoolVar(&cf.NoCache)
app.Flag("user", fmt.Sprintf("Teleport user [%s]", localUser)).Envar(userEnvVar).StringVar(&cf.Username)
app.Flag("user", "Teleport user, defaults to current local user").Envar(userEnvVar).StringVar(&cf.Username)
app.Flag("mem-profile", "Write memory profile to file").Hidden().StringVar(&memProfile)
app.Flag("cpu-profile", "Write CPU profile to file").Hidden().StringVar(&cpuProfile)
app.Flag("trace-profile", "Write trace profile to file").Hidden().StringVar(&traceProfile)