Simplified Loader.New() to always take a directory (not a file)

pkg/loader/fileloader.go

@@ -45,20 +45,10 @@ func (l *fileLoader) IsScheme(root string, location string) bool {
 	return filepath.IsAbs(fullFilePath)
 }
 
-// Returns the directory of the calculated full file path.
-// Example: "/home/seans/project", "subdir/file.txt" -> "/home/seans/project/subdir".
-func (l *fileLoader) Root(root string, location string) (string, error) {
-	fullFilePath, err := l.FullLocation(root, location)
-	if err != nil {
-		return "", err
-	}
-	return filepath.Dir(fullFilePath), nil
-}
-
 // If location is a full file path, then ignore root. If location is relative, then
 // join the root path with the location path. Either root or location can be empty,
-// but not both.
-// Example: "/home/seans/project", "subdir/file.txt" -> "/home/seans/project/subdir/file.txt".
+// but not both. Special case for ".": Expands to current working directory.
+// Example: "/home/seans/project", "subdir/bar" -> "/home/seans/project/subdir/bar".
 func (l *fileLoader) FullLocation(root string, location string) (string, error) {
 	// First, validate the parameters
 	if len(root) == 0 && len(location) == 0 {
@@ -72,7 +62,7 @@ func (l *fileLoader) FullLocation(root string, location string) (string, error)
 		}
 		location = currentDir
 	}
-	// Assume the location is an full file path. If not, then join root with location.
+	// Assume the location is a full file path. If not, then join root with location.
 	fullLocation := location
 	if !filepath.IsAbs(location) {
 		fullLocation = filepath.Join(root, location)

pkg/loader/loader.go

@@ -39,8 +39,6 @@ type loaderImpl struct {
 type SchemeLoader interface {
 	// Does this location correspond to this scheme.
 	IsScheme(root string, location string) bool
-	// Calulcate a new root.
-	Root(root string, location string) (string, error)
 	// Combines the root and path into a full location string.
 	FullLocation(root string, path string) (string, error)
 	// Load bytes at scheme-specific location or an error.
@@ -60,13 +58,17 @@ func (l *loaderImpl) Root() string {
 	return l.root
 }
 
-// Returns a new Loader rooted at newRoot.
+// Returns a new Loader rooted at newRoot. "newRoot" MUST be
+// a directory (not a file). The directory can have a trailing
+// slash or not.
+// Example: "/home/seans/project" or "/home/seans/project/"
+// NOT "/home/seans/project/file.yaml".
 func (l *loaderImpl) New(newRoot string) (Loader, error) {
 	scheme, err := l.getSchemeLoader(newRoot)
 	if err != nil {
 		return nil, err
 	}
-	root, err := scheme.Root(l.root, newRoot)
+	root, err := scheme.FullLocation(l.root, newRoot)
 	if err != nil {
 		return nil, err
 	}
@@ -74,6 +76,8 @@ func (l *loaderImpl) New(newRoot string) (Loader, error) {
 }
 
 // Load returns all the bytes read from scheme-specific location or an error.
+// "location" can be an absolute path, or if relative, full location is
+// calculated from the Root().
 func (l *loaderImpl) Load(location string) ([]byte, error) {
 	scheme, err := l.getSchemeLoader(location)
 	if err != nil {

pkg/loader/loader_test.go

@@ -49,15 +49,16 @@ func TestLoader_Root(t *testing.T) {
 		t.Fatalf("Expected error for unknown scheme not returned")
 	}
 
-	loader, err := rootLoader.New("/home/seans/project/file.yaml")
+	// Test with trailing slash in directory.
+	loader, err := rootLoader.New("/home/seans/project/")
 	if err != nil {
 		t.Fatalf("Unexpected in New(): %v\n", err)
 	}
-	if "/home/seans/project" != loader.Root() {
+	if "/home/seans/project/" != loader.Root() {
 		t.Fatalf("Incorrect Loader Root: %s\n", loader.Root())
 	}
 
-	subLoader, err := loader.New("subdir/file.yaml")
+	subLoader, err := loader.New("subdir")
 	if err != nil {
 		t.Fatalf("Unexpected in New(): %v\n", err)
 	}
@@ -65,7 +66,8 @@ func TestLoader_Root(t *testing.T) {
 		t.Fatalf("Incorrect Loader Root: %s\n", subLoader.Root())
 	}
 
-	anotherLoader, err := loader.New("/home/seans/project2/file.yaml")
+	// Test without trailing slash in directory.
+	anotherLoader, err := loader.New("/home/seans/project2")
 	if err != nil {
 		t.Fatalf("Unexpected in New(): %v\n", err)
 	}
@@ -92,7 +94,7 @@ func TestLoader_Load(t *testing.T) {
 	fakefs.WriteFile("/home/seans/project2/file.yaml", []byte("This is another yaml file"))
 	rootLoader := initializeRootLoader(fakefs)
 
-	loader, err := rootLoader.New("/home/seans/project/file.yaml")
+	loader, err := rootLoader.New("/home/seans/project")
 	if err != nil {
 		t.Fatalf("Unexpected in New(): %v\n", err)
 	}

pkg/loader/loadertest/fake_loader.go

@@ -18,7 +18,6 @@ package loadertest
 
 import (
 	"os"
-	"strings"
 
 	"k8s.io/kubectl/pkg/kinflate/util/fs"
 	"k8s.io/kubectl/pkg/loader"
@@ -31,19 +30,15 @@ type FakeLoader struct {
 }
 
 // NewFakeLoader returns a Loader that delegates calls, and encapsulates
-// a fake file system that the Loader reads from. "initialFilePath" parameter
-// must be an absolute path, and it can be a directory if it has a
-// trailing slash: "/home/seans/project/" (dir) -- "/home/seans/project" (file)
-func NewFakeLoader(initialFilePath string) FakeLoader {
+// a fake file system that the Loader reads from. "initialDir" parameter
+// must be an full, absolute directory (trailing slash doesn't matter).
+func NewFakeLoader(initialDir string) FakeLoader {
 	// Create fake filesystem and inject it into initial Loader.
 	fakefs := fs.MakeFakeFS()
-	if strings.HasSuffix(initialFilePath, "/") {
-		fakefs.Mkdir(initialFilePath, 0x777)
-	}
 	var schemes []loader.SchemeLoader
 	schemes = append(schemes, loader.NewFileLoader(fakefs))
 	rootLoader := loader.Init(schemes)
-	loader, _ := rootLoader.New(initialFilePath)
+	loader, _ := rootLoader.New(initialDir)
 	return FakeLoader{fs: fakefs, delegate: loader}
 }