Skip to content

Commit

Permalink
Let UserHomeDir panic instead of returning an error.
Browse files Browse the repository at this point in the history
Basically nothing will work if the user's home directory cannot be
determined, yet we diligently (well almost) propagated that error in
a lot of places in the code without acting on it. In some places, we
simply left the error unchecked.

This commit changes this so that a panic is thrown, and removes all
error propagation.

Signed-off-by: Thomas Hallgren <[email protected]>
  • Loading branch information
thallgren committed Mar 29, 2023
1 parent 7552627 commit a588b6d
Show file tree
Hide file tree
Showing 24 changed files with 145 additions and 305 deletions.
8 changes: 2 additions & 6 deletions integration_test/cloud_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,7 @@ func (s *notConnectedSuite) Test_RootdCloudLogLevel() {
// of rushing to the end of the file and remembering where we left off when we start looking
// for new lines.
var lines int64
logDir, err := filelocation.AppUserLogDir(ctx)
require.NoError(err)
rootLogName := filepath.Join(logDir, "daemon.log")
rootLogName := filepath.Join(filelocation.AppUserLogDir(ctx), "daemon.log")
rootLog, err := os.Open(rootLogName)
require.NoError(err)
scn := bufio.NewScanner(rootLog)
Expand Down Expand Up @@ -180,9 +178,7 @@ func (s *notConnectedSuite) Test_UserdCloudLogLevel() {
// of rushing to the end of the file and remembering where we left off when we start looking
// for new lines.
var lines int64
logDir, err := filelocation.AppUserLogDir(ctx)
require.NoError(err)
logName := filepath.Join(logDir, "connector.log")
logName := filepath.Join(filelocation.AppUserLogDir(ctx), "connector.log")
logF, err := os.Open(logName)
require.NoError(err)
scn := bufio.NewScanner(logF)
Expand Down
3 changes: 1 addition & 2 deletions integration_test/gather_logs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,7 @@ func (s *multipleInterceptsSuite) cleanLogDir(ctx context.Context) {
}

func cleanLogDir(ctx context.Context, require *require.Assertions, nsRx, mgrNamespace, svcNameRx string) {
logDir, err := filelocation.AppUserLogDir(ctx)
require.NoError(err)
logDir := filelocation.AppUserLogDir(ctx)
files, err := os.ReadDir(logDir)
require.NoError(err)
match := regexp.MustCompile(
Expand Down
12 changes: 4 additions & 8 deletions integration_test/itest/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,6 @@ func (s *cluster) CapturePodLogs(ctx context.Context, app, container, ns string)
ctx = dcontext.WithoutCancel(ctx)

present := struct{}{}
logDir, _ := filelocation.AppUserLogDir(ctx)

// Use another logger to avoid errors due to logs arriving after the tests complete.
ctx = dlog.WithLogger(ctx, dlog.WrapLogrus(logrus.StandardLogger()))
Expand All @@ -434,7 +433,8 @@ func (s *cluster) CapturePodLogs(ctx context.Context, app, container, ns string)
if _, ok := s.logCapturingPods.LoadOrStore(pod, present); ok {
continue
}
logFile, err := os.Create(filepath.Join(logDir, fmt.Sprintf("%s-%s.log", dtime.Now().Format("20060102T150405"), pod)))
logFile, err := os.Create(
filepath.Join(filelocation.AppUserLogDir(ctx), fmt.Sprintf("%s-%s.log", dtime.Now().Format("20060102T150405"), pod)))
if err != nil {
s.logCapturingPods.Delete(pod)
dlog.Errorf(ctx, "unable to create pod logfile %s: %v", logFile.Name(), err)
Expand Down Expand Up @@ -735,15 +735,11 @@ func Telepresence(ctx context.Context, args ...string) (string, string, error) {
func TelepresenceCmd(ctx context.Context, args ...string) *dexec.Cmd {
t := getT(ctx)
t.Helper()
configDir, err := filelocation.AppUserConfigDir(ctx)
require.NoError(t, err)
logDir, err := filelocation.AppUserLogDir(ctx)
require.NoError(t, err)

var stdout, stderr strings.Builder
ctx = WithEnv(ctx, map[string]string{
"DEV_TELEPRESENCE_CONFIG_DIR": configDir,
"DEV_TELEPRESENCE_LOG_DIR": logDir,
"DEV_TELEPRESENCE_CONFIG_DIR": filelocation.AppUserConfigDir(ctx),
"DEV_TELEPRESENCE_LOG_DIR": filelocation.AppUserLogDir(ctx),
})

gh := GetGlobalHarness(ctx)
Expand Down
9 changes: 1 addition & 8 deletions integration_test/itest/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,7 @@ type itestConfig struct {
}

func LoadEnv(ctx context.Context) context.Context {
dir, err := filelocation.AppUserConfigDir(ctx)
if err != nil {
if !os.IsNotExist(err) {
getT(ctx).Fatal(err)
}
return ctx
}
cf := filepath.Join(dir, "itest.yml")
cf := filepath.Join(filelocation.AppUserConfigDir(ctx), "itest.yml")
data, err := os.ReadFile(cf)
if err != nil {
if !os.IsNotExist(err) {
Expand Down
4 changes: 1 addition & 3 deletions integration_test/kubeconfig_extension_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,7 @@ func (s *notConnectedSuite) Test_DNSIncludes() {
return map[string]any{"dns": map[string][]string{"include-suffixes": {".org"}}}
})
require := s.Require()
logDir, err := filelocation.AppUserLogDir(ctx)
require.NoError(err)
logFile := filepath.Join(logDir, "daemon.log")
logFile := filepath.Join(filelocation.AppUserLogDir(ctx), "daemon.log")

// Check that config view -c includes the includeSuffixes
stdout := itest.TelepresenceOk(ctx, "config", "--context", "extra", "view", "--client-only")
Expand Down
4 changes: 1 addition & 3 deletions integration_test/loglevel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@ func (s *notConnectedSuite) Test_RootDaemonLogLevel() {
ctx := s.Context()
itest.TelepresenceOk(ctx, "connect", "--manager-namespace", s.ManagerNamespace())
itest.TelepresenceQuitOk(ctx)
logDir, err := filelocation.AppUserLogDir(ctx)
require.NoError(err)
rootLogName := filepath.Join(logDir, "daemon.log")
rootLogName := filepath.Join(filelocation.AppUserLogDir(ctx), "daemon.log")
rootLog, err := os.Open(rootLogName)
require.NoError(err)
defer rootLog.Close()
Expand Down
6 changes: 1 addition & 5 deletions integration_test/wpad_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,8 @@ import (
// Test_WpadNotForwarded tests that DNS request aren't forwarded
// to the cluster.
func (s *connectedSuite) Test_WpadNotForwarded() {
require := s.Require()
ctx := s.Context()

logDir, err := filelocation.AppUserLogDir(ctx)
require.NoError(err)
logFile := filepath.Join(logDir, "daemon.log")
logFile := filepath.Join(filelocation.AppUserLogDir(ctx), "daemon.log")

tests := []struct {
qn string
Expand Down
25 changes: 4 additions & 21 deletions pkg/client/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,8 @@ func SaveToUserCache(ctx context.Context, object any, file string) error {
return err
}

// get base path of user cache
cacheDir, err := filelocation.AppUserCacheDir(ctx)
if err != nil {
return err
}
// add file path (ex. "ispec/00-00-0000.json")
fullFilePath := filepath.Join(cacheDir, file)
fullFilePath := filepath.Join(filelocation.AppUserCacheDir(ctx), file)
// get dir of joined path
dir := filepath.Dir(fullFilePath)
if err := dos.MkdirAll(ctx, dir, 0o700); err != nil {
Expand All @@ -34,11 +29,7 @@ func SaveToUserCache(ctx context.Context, object any, file string) error {
}

func LoadFromUserCache(ctx context.Context, dest any, file string) error {
dir, err := filelocation.AppUserCacheDir(ctx)
if err != nil {
return err
}
path := filepath.Join(dir, file)
path := filepath.Join(filelocation.AppUserCacheDir(ctx), file)
jsonContent, err := os.ReadFile(path)
if err != nil {
return err
Expand All @@ -50,22 +41,14 @@ func LoadFromUserCache(ctx context.Context, dest any, file string) error {
}

func DeleteFromUserCache(ctx context.Context, file string) error {
dir, err := filelocation.AppUserCacheDir(ctx)
if err != nil {
return err
}
if err := os.Remove(filepath.Join(dir, file)); err != nil && !os.IsNotExist(err) {
if err := os.Remove(filepath.Join(filelocation.AppUserCacheDir(ctx), file)); err != nil && !os.IsNotExist(err) {
return err
}
return nil
}

func ExistsInCache(ctx context.Context, fileName string) (bool, error) {
dir, err := filelocation.AppUserCacheDir(ctx)
if err != nil {
return false, err
}
path := filepath.Join(dir, fileName)
path := filepath.Join(filelocation.AppUserCacheDir(ctx), fileName)
if _, err := os.Stat(path); err != nil {
if errors.Is(err, os.ErrNotExist) {
return false, nil
Expand Down
12 changes: 2 additions & 10 deletions pkg/client/cache/daemons.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,7 @@ func LoadDaemonInfos(ctx context.Context) ([]*DaemonInfo, error) {
}

func daemonInfoFiles(ctx context.Context) ([]fs.DirEntry, error) {
dir, err := filelocation.AppUserCacheDir(ctx)
if err != nil {
return nil, err
}
files, err := os.ReadDir(filepath.Join(dir, daemonsDirName))
files, err := os.ReadDir(filepath.Join(filelocation.AppUserCacheDir(ctx), daemonsDirName))
if err != nil {
if os.IsNotExist(err) {
err = nil
Expand Down Expand Up @@ -129,11 +125,7 @@ func DaemonInfoFile(name string, port int) string {
//
// The alive poll ends and the DaemonInfo is deleted when the context is cancelled.
func KeepDaemonInfoAlive(ctx context.Context, file string) error {
dir, err := filelocation.AppUserCacheDir(ctx)
if err != nil {
return err
}
daemonFile := filepath.Join(dir, daemonsDirName, file)
daemonFile := filepath.Join(filelocation.AppUserCacheDir(ctx), daemonsDirName, file)
ticker := time.NewTicker(keepAliveInterval)
defer ticker.Stop()
now := time.Now()
Expand Down
8 changes: 2 additions & 6 deletions pkg/client/cache/watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,10 @@ import (
// WatchUserCache uses a file system watcher that receives events when the file changes
// and calls the given function when that happens.
func WatchUserCache(ctx context.Context, subdir string, onChange func(context.Context) error, files ...string) error {
dir, err := filelocation.AppUserCacheDir(ctx)
if err != nil {
return err
}
dir = filepath.Join(dir, subdir)
dir := filepath.Join(filelocation.AppUserCacheDir(ctx), subdir)

// Ensure that the user cache directory exists.
if err = dos.MkdirAll(ctx, dir, 0o755); err != nil {
if err := dos.MkdirAll(ctx, dir, 0o755); err != nil {
return err
}
watcher, err := fsnotify.NewWatcher()
Expand Down
6 changes: 1 addition & 5 deletions pkg/client/cli/cmd/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,8 @@ func runConfigView(cmd *cobra.Command, _ []string) error {
// Unable to establish a session, so try to convey the local config instead. It
// may be helpful in diagnosing the problem.
ctx := cmd.Context()
cfgDir, err := filelocation.AppUserConfigDir(ctx)
if err != nil {
return err
}
cfg.Config = client.GetConfig(cmd.Context())
cfg.ClientFile = filepath.Join(cfgDir, client.ConfigFile)
cfg.ClientFile = filepath.Join(filelocation.AppUserConfigDir(ctx), client.ConfigFile)

rq := daemon.GetRequest(ctx)
kc, err := client.NewKubeconfig(ctx, rq.KubeFlags, rq.ManagerNamespace)
Expand Down
7 changes: 1 addition & 6 deletions pkg/client/cli/cmd/gather_logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,6 @@ func (gl *gatherLogsCommand) gatherLogs(cmd *cobra.Command, _ []string) error {
scout.Start(ctx)
defer scout.Close()

// Get the log directory and return the error if we can't get it
logDir, err := filelocation.AppUserLogDir(ctx)
if err != nil {
return errcat.User.New(err)
}

// If the user did not provide an outputFile, we'll use their current working directory
if gl.outputFile == "" {
pwd, err := os.Getwd()
Expand Down Expand Up @@ -174,6 +168,7 @@ func (gl *gatherLogsCommand) gatherLogs(cmd *cobra.Command, _ []string) error {
}

// Get all logs from the logDir that match the daemons the user cares about.
logDir := filelocation.AppUserLogDir(ctx)
logFiles, err := os.ReadDir(logDir)
if err != nil {
return errcat.User.New(err)
Expand Down
12 changes: 3 additions & 9 deletions pkg/client/cli/connect/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,7 @@ func launchDaemon(ctx context.Context, cr *daemon.Request) error {

// Ensure that the logfile is present before the daemon starts so that it isn't created with
// root permissions.
logDir, err := filelocation.AppUserLogDir(ctx)
if err != nil {
return err
}
logDir := filelocation.AppUserLogDir(ctx)
logFile := filepath.Join(logDir, "daemon.log")
if _, err := os.Stat(logFile); err != nil {
if !os.IsNotExist(err) {
Expand Down Expand Up @@ -111,11 +108,8 @@ func Disconnect(ctx context.Context, quitDaemons bool) error {
}

func ensureAppUserConfigDir(ctx context.Context) (string, error) {
configDir, err := filelocation.AppUserConfigDir(ctx)
if err != nil {
return "", errcat.NoDaemonLogs.New(err)
}
if err = os.MkdirAll(configDir, 0o700); err != nil {
configDir := filelocation.AppUserConfigDir(ctx)
if err := os.MkdirAll(configDir, 0o700); err != nil {
return "", errcat.NoDaemonLogs.Newf("unable to ensure that config directory %q exists: %w", configDir, err)
}
return configDir, nil
Expand Down
18 changes: 3 additions & 15 deletions pkg/client/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -867,8 +867,7 @@ func ReplaceConfig(ctx context.Context, config *Config) {

// GetConfigFile gets the path to the configFile as stored in filelocation.AppUserConfigDir.
func GetConfigFile(c context.Context) string {
dir, _ := filelocation.AppUserConfigDir(c)
return filepath.Join(dir, ConfigFile)
return filepath.Join(filelocation.AppUserConfigDir(c), ConfigFile)
}

// GetDefaultConfig returns the default configuration settings.
Expand Down Expand Up @@ -917,12 +916,7 @@ func LoadConfig(c context.Context) (cfg *Config, err error) {
}
}()

var dirs []string
dirs, err = filelocation.AppSystemConfigDirs(c)
if err != nil {
return nil, err
}

dirs := filelocation.AppSystemConfigDirs(c)
dflt := GetDefaultConfig()
cfg = &dflt
readMerge := func(dir string) error {
Expand Down Expand Up @@ -954,13 +948,7 @@ func LoadConfig(c context.Context) (cfg *Config, err error) {
return nil, err
}
}
appDir, err := filelocation.AppUserConfigDir(c)
if err != nil {
if os.IsNotExist(err) {
return cfg, nil
}
return nil, err
}
appDir := filelocation.AppUserConfigDir(c)
if err = readMerge(appDir); err != nil {
return nil, err
}
Expand Down
30 changes: 5 additions & 25 deletions pkg/client/docker/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,18 +63,6 @@ func EnsureNetwork(ctx context.Context, name string) {

// DaemonOptions returns the options necessary to pass to a docker run when starting a daemon container.
func DaemonOptions(ctx context.Context, name string) ([]string, *net.TCPAddr, error) {
tpConfig, err := filelocation.AppUserConfigDir(ctx)
if err != nil {
return nil, nil, errcat.NoDaemonLogs.New(err)
}
tpCache, err := filelocation.AppUserCacheDir(ctx)
if err != nil {
return nil, nil, errcat.NoDaemonLogs.New(err)
}
tpLog, err := filelocation.AppUserLogDir(ctx)
if err != nil {
return nil, nil, errcat.NoDaemonLogs.New(err)
}
as, err := dnet.FreePortsTCP(1)
if err != nil {
return nil, nil, err
Expand All @@ -89,9 +77,9 @@ func DaemonOptions(ctx context.Context, name string) ([]string, *net.TCPAddr, er
"-e", fmt.Sprintf("TELEPRESENCE_UID=%d", os.Getuid()),
"-e", fmt.Sprintf("TELEPRESENCE_GID=%d", os.Getgid()),
"-p", fmt.Sprintf("%s:%d", addr, port),
"-v", fmt.Sprintf("%s:%s:ro", tpConfig, dockerTpConfig),
"-v", fmt.Sprintf("%s:%s", tpCache, dockerTpCache),
"-v", fmt.Sprintf("%s:%s", tpLog, dockerTpLog),
"-v", fmt.Sprintf("%s:%s:ro", filelocation.AppUserConfigDir(ctx), dockerTpConfig),
"-v", fmt.Sprintf("%s:%s", filelocation.AppUserCacheDir(ctx), dockerTpCache),
"-v", fmt.Sprintf("%s:%s", filelocation.AppUserLogDir(ctx), dockerTpLog),
}
if runtime.GOOS == "linux" {
opts = append(opts, "--add-host", "host.docker.internal:host-gateway")
Expand Down Expand Up @@ -254,11 +242,7 @@ func startAuthenticatorService(ctx context.Context, portFile string, kubeFlags m
}

func ensureAuthenticatorService(ctx context.Context, kubeFlags map[string]string, configFiles []string) (uint16, error) {
tpCache, err := filelocation.AppUserCacheDir(ctx)
if err != nil {
return 0, err
}
portFile := filepath.Join(tpCache, kubeAuthPortFile)
portFile := filepath.Join(filelocation.AppUserCacheDir(ctx), kubeAuthPortFile)
st, err := os.Stat(portFile)
if err != nil {
if !os.IsNotExist(err) {
Expand Down Expand Up @@ -327,11 +311,7 @@ func EnableK8SAuthenticator(ctx context.Context) error {
// Store the file using its context name under the <telepresence cache>/kube directory
const kubeConfigs = "kube"
kubeConfigFile := config.CurrentContext
tpCache, err := filelocation.AppUserCacheDir(ctx)
if err != nil {
return err
}
kubeConfigDir := filepath.Join(tpCache, kubeConfigs)
kubeConfigDir := filepath.Join(filelocation.AppUserCacheDir(ctx), kubeConfigs)
if err = os.MkdirAll(kubeConfigDir, 0o700); err != nil {
return err
}
Expand Down
Loading

0 comments on commit a588b6d

Please sign in to comment.