diff --git a/pkg/fs/fakefs.go b/pkg/fs/fakefs.go index f38260185..fb21f81bd 100644 --- a/pkg/fs/fakefs.go +++ b/pkg/fs/fakefs.go @@ -102,9 +102,16 @@ func (fs *fakeFs) Open(name string) (File, error) { return fs.m[name], nil } -// EvalSymlinks does nothing and cannot fail. -func (fs *fakeFs) EvalSymlinks(path string) (string, error) { - return path, nil +// CleanedAbs does nothing and cannot fail. +func (fs *fakeFs) CleanedAbs(path string) (string, string, error) { + if fs.IsDir(path) { + return path, "", nil + } + d := filepath.Dir(path) + if d == path { + return d, "", nil + } + return d, filepath.Base(path), nil } // Exists returns true if file is known. diff --git a/pkg/fs/fs.go b/pkg/fs/fs.go index 5d647850a..65f5e0148 100644 --- a/pkg/fs/fs.go +++ b/pkg/fs/fs.go @@ -30,7 +30,7 @@ type FileSystem interface { RemoveAll(name string) error Open(name string) (File, error) IsDir(name string) bool - EvalSymlinks(path string) (string, error) + CleanedAbs(path string) (string, string, error) Exists(name string) bool Glob(pattern string) ([]string, error) ReadFile(name string) ([]byte, error) diff --git a/pkg/fs/realfs.go b/pkg/fs/realfs.go index c4fad35c4..75f6f2987 100644 --- a/pkg/fs/realfs.go +++ b/pkg/fs/realfs.go @@ -17,7 +17,9 @@ limitations under the License. package fs import ( + "fmt" "io/ioutil" + "log" "os" "path/filepath" ) @@ -53,9 +55,41 @@ func (realFS) RemoveAll(name string) error { // Open delegates to os.Open. func (realFS) Open(name string) (File, error) { return os.Open(name) } -// EvalSymlinks delegates to filepath.EvalSymlinks. -func (realFS) EvalSymlinks(path string) (string, error) { - return filepath.EvalSymlinks(path) +// CleanedAbs returns a cleaned, absolute path +// with no symbolic links split into directory +// and file components. If the entire path is +// a directory, the file component is an empty +// string. +func (x realFS) CleanedAbs(path string) (string, string, error) { + absRoot, err := filepath.Abs(path) + if err != nil { + return "", "", fmt.Errorf( + "abs path error on '%s' : %v", path, err) + } + deLinked, err := filepath.EvalSymlinks(absRoot) + if err != nil { + return "", "", fmt.Errorf( + "evalsymlink failure on '%s' : %v", path, err) + } + if x.IsDir(deLinked) { + return deLinked, "", nil + } + d := filepath.Dir(deLinked) + if !x.IsDir(d) { + // Programmer/assumption error. + log.Fatalf("first part of '%s' not a directory", deLinked) + } + if d == deLinked { + // Programmer/assumption error. + log.Fatalf("d '%s' should be a subset of deLinked", d) + } + f := filepath.Base(deLinked) + if filepath.Join(d, f) != deLinked { + // Programmer/assumption error. + log.Fatalf("these should be equal: '%s', '%s'", + filepath.Join(d, f), deLinked) + } + return d, f, nil } // Exists returns true if os.Stat succeeds. diff --git a/pkg/fs/realfs_test.go b/pkg/fs/realfs_test.go index c1bf8a734..7add5fb64 100644 --- a/pkg/fs/realfs_test.go +++ b/pkg/fs/realfs_test.go @@ -17,18 +17,17 @@ limitations under the License. package fs import ( + "io/ioutil" "os" "path" + "path/filepath" "reflect" "testing" ) -func TestReadFilesRealFS(t *testing.T) { +func makeTestDir(t *testing.T) (FileSystem, string) { x := MakeRealFS() - testDir := "kustomize_testing_dir" - err := x.Mkdir(testDir) - defer os.RemoveAll(testDir) - + testDir, err := ioutil.TempDir("", "kustomize_testing_dir") if err != nil { t.Fatalf("unexpected error %s", err) } @@ -38,8 +37,113 @@ func TestReadFilesRealFS(t *testing.T) { if !x.IsDir(testDir) { t.Fatalf("expected directory") } + return x, testDir +} - err = x.WriteFile(path.Join(testDir, "foo"), []byte(`foo`)) +func TestCleanedAbs_1(t *testing.T) { + x, testDir := makeTestDir(t) + defer os.RemoveAll(testDir) + + d, f, err := x.CleanedAbs("") + if err != nil { + t.Fatalf("unexpected err=%v", err) + } + wd, err := os.Getwd() + if err != nil { + t.Fatalf("unexpected err=%v", err) + } + if d != wd { + t.Fatalf("unexpected d=%s", d) + } + if f != "" { + t.Fatalf("unexpected f=%s", f) + } +} + +func TestCleanedAbs_2(t *testing.T) { + x, testDir := makeTestDir(t) + defer os.RemoveAll(testDir) + + d, f, err := x.CleanedAbs("/") + if err != nil { + t.Fatalf("unexpected err=%v", err) + } + if d != "/" { + t.Fatalf("unexpected d=%s", d) + } + if f != "" { + t.Fatalf("unexpected f=%s", f) + } +} + +func TestCleanedAbs_3(t *testing.T) { + x, testDir := makeTestDir(t) + defer os.RemoveAll(testDir) + + err := x.WriteFile( + filepath.Join(testDir, "foo"), []byte(`foo`)) + if err != nil { + t.Fatalf("unexpected err=%v", err) + } + + d, f, err := x.CleanedAbs(filepath.Join(testDir, "foo")) + if err != nil { + t.Fatalf("unexpected err=%v", err) + } + if d != testDir { + t.Fatalf("unexpected d=%s", d) + } + if f != "foo" { + t.Fatalf("unexpected f=%s", f) + } + +} + +func TestCleanedAbs_4(t *testing.T) { + x, testDir := makeTestDir(t) + defer os.RemoveAll(testDir) + + err := x.MkdirAll(filepath.Join(testDir, "d1", "d2")) + if err != nil { + t.Fatalf("unexpected err=%v", err) + } + err = x.WriteFile( + filepath.Join(testDir, "d1", "d2", "bar"), + []byte(`bar`)) + if err != nil { + t.Fatalf("unexpected err=%v", err) + } + + d, f, err := x.CleanedAbs( + filepath.Join(testDir, "d1", "d2")) + if err != nil { + t.Fatalf("unexpected err=%v", err) + } + if d != filepath.Join(testDir, "d1", "d2") { + t.Fatalf("unexpected d=%s", d) + } + if f != "" { + t.Fatalf("unexpected f=%s", f) + } + + d, f, err = x.CleanedAbs( + filepath.Join(testDir, "d1", "d2", "bar")) + if err != nil { + t.Fatalf("unexpected err=%v", err) + } + if d != filepath.Join(testDir, "d1", "d2") { + t.Fatalf("unexpected d=%s", d) + } + if f != "bar" { + t.Fatalf("unexpected f=%s", f) + } +} + +func TestReadFilesRealFS(t *testing.T) { + x, testDir := makeTestDir(t) + defer os.RemoveAll(testDir) + + err := x.WriteFile(path.Join(testDir, "foo"), []byte(`foo`)) if err != nil { t.Fatalf("unexpected error %s", err) } diff --git a/pkg/loader/fileloader.go b/pkg/loader/fileloader.go index 9973e4e4b..0384020f8 100644 --- a/pkg/loader/fileloader.go +++ b/pkg/loader/fileloader.go @@ -123,11 +123,16 @@ func newFileLoaderAt( return nil, fmt.Errorf( "loader root cannot be empty") } - absRoot, err := cleanedAbs(root, fSys) + absRoot, f, err := fSys.CleanedAbs(root) if err != nil { return nil, fmt.Errorf( "absolute path error in '%s' : %v", root, err) } + if f != "" { + return nil, fmt.Errorf( + "got file '%s', but '%s' must be a directory to be a root", + f, root) + } if err := isPathEqualToOrAbove(absRoot, roots); err != nil { return nil, err } @@ -143,22 +148,6 @@ func newFileLoaderAt( }, nil } -// cleanedAbs returns a cleaned, absolute path -// with no symbolic links. -func cleanedAbs(path string, fSys fs.FileSystem) (string, error) { - absRoot, err := filepath.Abs(path) - if err != nil { - return "", fmt.Errorf( - "abs path error on '%s' : %v", path, err) - } - deLinked, err := fSys.EvalSymlinks(absRoot) - if err != nil { - return "", fmt.Errorf( - "evalsymlink failure on '%s' : %v", path, err) - } - return deLinked, nil -} - // New returns a new Loader, rooted relative to current loader, // or rooted in a temp directory holding a git repo clone. func (l *fileLoader) New(path string) (ifc.Loader, error) { @@ -226,11 +215,16 @@ func (l *fileLoader) Load(path string) ([]byte, error) { if filepath.IsAbs(path) { return nil, l.loadOutOfBounds(path) } - path, err := cleanedAbs( - filepath.Join(l.Root(), path), l.fSys) + d, f, err := l.fSys.CleanedAbs( + filepath.Join(l.Root(), path)) if err != nil { return nil, err } + if f == "" { + return nil, fmt.Errorf( + "'%s' must be a file (got d='%s')", path, d) + } + path = filepath.Join(d, f) if !l.isInOrBelowRoot(path) { return nil, l.loadOutOfBounds(path) } diff --git a/pkg/loader/fileloader_test.go b/pkg/loader/fileloader_test.go index bc94c2c09..f1e861005 100644 --- a/pkg/loader/fileloader_test.go +++ b/pkg/loader/fileloader_test.go @@ -75,7 +75,7 @@ func TestNewFileLoaderAt_DemandsDirectory(t *testing.T) { if err == nil { t.Fatalf("Expected error - a file should not work.") } - if !strings.Contains(err.Error(), "does not exist or is not a directory") { + if !strings.Contains(err.Error(), "must be a directory to be a root") { t.Fatalf("unexpected err: %v", err) } } diff --git a/pkg/resmap/factory_test.go b/pkg/resmap/factory_test.go index 8d4cb0d54..27e12b86d 100644 --- a/pkg/resmap/factory_test.go +++ b/pkg/resmap/factory_test.go @@ -145,7 +145,7 @@ func TestNewFromConfigMaps(t *testing.T) { expected ResMap } - l := loadertest.NewFakeLoader("/whatever/project/") + l := loadertest.NewFakeLoader("/whatever/project") testCases := []testCase{ { description: "construct config map from env",