From c23b2b16db59e3583b79390c906e6d474f6d31d8 Mon Sep 17 00:00:00 2001 From: Tom Wieczorek Date: Thu, 30 Nov 2023 16:38:31 +0100 Subject: [PATCH] Add dir.Exists as an error-aware alternative to dir.IsDirectory The dir.IsDirectory function is potentially hiding errors from callers. In some cases, it's good to have a definitive answer to the question if a directory really exists or not, or if it couldn't be determined because of some OS error. The new function dir.Exists allows for that distinction. Signed-off-by: Tom Wieczorek --- internal/pkg/dir/dir.go | 32 ++++++++++++ internal/pkg/dir/dir_test.go | 81 +++++++++++++++++++++++++++++++ internal/pkg/dir/dir_unix_test.go | 17 +++++++ 3 files changed, 130 insertions(+) diff --git a/internal/pkg/dir/dir.go b/internal/pkg/dir/dir.go index 1b92e1a1b2d3..be46f5fce028 100644 --- a/internal/pkg/dir/dir.go +++ b/internal/pkg/dir/dir.go @@ -21,8 +21,40 @@ import ( "fmt" "os" "strings" + "syscall" ) +// Exists checks if the given path is an existing directory. It returns true if +// the directory exists. It's different from [IsDirectory] because the latter +// will return false even if there's a real error, such as permission or general +// file system access problems. Note: This function primarily checks for the +// existence of a path and whether it's a directory. It does not guarantee that +// a directory with the given path can actually be created if it doesn't exist. +func Exists(path string) (bool, error) { + stat, err := os.Stat(path) + switch { + case err == nil: + return stat.IsDir(), nil + + case errors.Is(err, os.ErrNotExist): + if path == "" { + // Golang's os.Stat doesn't specify AT_EMPTY_PATH in the stat syscall. + return false, fmt.Errorf("%w (try a dot instead of an empty path)", err) + } + + // The path doesn't exist. + return false, nil + + case errors.Is(err, syscall.ENOTDIR): + // Some prefix of the path exists, but is not a directory. + // Anyhow, the path itself doesn't exist. + return false, nil + + default: + return false, err + } +} + // IsDirectory check the given path exists and is a directory func IsDirectory(name string) bool { fi, err := os.Stat(name) diff --git a/internal/pkg/dir/dir_test.go b/internal/pkg/dir/dir_test.go index b850ddd4e33b..289262d56700 100644 --- a/internal/pkg/dir/dir_test.go +++ b/internal/pkg/dir/dir_test.go @@ -19,12 +19,93 @@ package dir_test import ( "os" "path/filepath" + "strings" + "syscall" "testing" "github.com/k0sproject/k0s/internal/pkg/dir" + "github.com/k0sproject/k0s/internal/testutil" + + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +func TestExists(t *testing.T) { + t.Run("existing_directory", func(t *testing.T) { + tmpDir := t.TempDir() + exists, err := dir.Exists(tmpDir) + if assert.NoError(t, err) { + assert.True(t, exists, "Freshly created temp dir should exist") + } + }) + + t.Run("nonexistent_path", func(t *testing.T) { + tmpDir := t.TempDir() + p := filepath.Join(tmpDir, "some-path") + + exists, err := dir.Exists(p) + if assert.NoError(t, err) { + assert.False(t, exists, "Nonexistent path shouldn't exist") + } + }) + + t.Run("empty_path", func(t *testing.T) { + tmpDir := t.TempDir() + defer testutil.Chdir(t, tmpDir)() + _, err := dir.Exists("") + assert.ErrorIs(t, err, os.ErrNotExist) + assert.ErrorContains(t, err, "try a dot instead of an empty path") + + exists, err := dir.Exists(".") + if assert.NoError(t, err) { + assert.True(t, exists, "Current directory should exist") + } + }) + + t.Run("obstructed_path", func(t *testing.T) { + tmpDir := t.TempDir() + p := filepath.Join(tmpDir, "some-path") + require.NoError(t, os.WriteFile(p, []byte("obstructed"), 0644)) + + exists, err := dir.Exists(p) + if assert.NoError(t, err) { + assert.False(t, exists, "Obstructed path shouldn't exist") + } + + exists, err = dir.Exists(filepath.Join(p, "sub-path")) + if assert.NoError(t, err) { + assert.False(t, exists, "Obstructed sub-path shouldn't exist") + } + }) + + t.Run("dangling_symlink", func(t *testing.T) { + tmpDir := t.TempDir() + nonexistent := filepath.Join(tmpDir, "nonexistent") + dangling := filepath.Join(tmpDir, "dangling") + require.NoError(t, os.Symlink(nonexistent, dangling)) + + exists, err := dir.Exists(dangling) + if assert.NoError(t, err) { + assert.False(t, exists, "Dangling symlink shouldn't exist") + } + + require.NoError(t, os.Mkdir(nonexistent, 0755)) + + exists, err = dir.Exists(dangling) + if assert.NoError(t, err) { + assert.True(t, exists, "Symlinked directory should exist") + } + }) + + t.Run("long_path", func(t *testing.T) { + tmpDir := t.TempDir() + // The upper limit is 255 for virtually all OSes/FSes + longPath := strings.Repeat("x", 256) + _, err := dir.Exists(filepath.Join(tmpDir, longPath)) + assert.ErrorIs(t, err, syscall.ENAMETOOLONG) + }) +} + func TestGetAll(t *testing.T) { t.Run("empty", func(t *testing.T) { diff --git a/internal/pkg/dir/dir_unix_test.go b/internal/pkg/dir/dir_unix_test.go index ca03dd879e85..dd4802f9d2b1 100644 --- a/internal/pkg/dir/dir_unix_test.go +++ b/internal/pkg/dir/dir_unix_test.go @@ -21,14 +21,31 @@ package dir_test import ( "os" "path/filepath" + "syscall" "testing" "github.com/k0sproject/k0s/internal/pkg/dir" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/sys/unix" ) +func TestExists_Unix(t *testing.T) { + t.Run("no_permissions", func(t *testing.T) { + tmpDir := t.TempDir() + require.NoError(t, os.Chmod(tmpDir, 0644)) + + exists, err := dir.Exists(tmpDir) + if assert.NoError(t, err) { + assert.True(t, exists, "Freshly created temp dir should exist") + } + + _, err = dir.Exists(filepath.Join(tmpDir, "no-perms")) + assert.ErrorIs(t, err, syscall.EACCES, "Shouldn't have permission to check that path") + }) +} + func TestInit(t *testing.T) { tmpDir := t.TempDir()