Merge pull request #177 from monopole/improveFsAbstraction

Replace os.Stat with IsDir and Exists, simplifying FS abstraction.

Gopkg.lock

@@ -296,6 +296,6 @@
 [solve-meta]
   analyzer-name = "dep"
   analyzer-version = 1
-  inputs-digest = "586d4cb9094e9b5c0731f16e931b953e9c0f709b7125a7537ae3625ada179eee"
+  inputs-digest = "74d444cd05ac6f803960180ec8ccfd5a4358077f7c79a5218a243554cb599274"
   solver-name = "gps-cdcl"
   solver-version = 1

pkg/app/application_test.go

@@ -18,7 +18,6 @@ package app
 
 import (
 	"encoding/base64"
-	"os"
 	"reflect"
 	"testing"
 
@@ -264,7 +263,7 @@ func makeLoader2(t *testing.T) loader.Loader {
 	if err != nil {
 		t.Fatalf("Failed to setup fake loader.")
 	}
-	err = loader.AddDirectory("/testpath/base", os.ModeDir)
+	err = loader.AddDirectory("/testpath/base")
 	if err != nil {
 		t.Fatalf("Failed to setup fake loader.")
 	}

pkg/commands/addbase.go

@@ -84,9 +84,8 @@ func (o *addBaseOptions) RunAddBase(fsys fs.FileSystem) error {
 	// split directory paths
 	paths := strings.Split(o.baseDirectoryPaths, ",")
 	for _, path := range paths {
-		_, err := fsys.Stat(path)
-		if err != nil {
-			return err
+		if !fsys.Exists(path) {
+			return errors.New(path + " does not exist")
 		}
 		if stringInSlice(path, m.Bases) {
 			return fmt.Errorf("base %s already in kustomization file", path)

pkg/commands/addbase_test.go

@@ -33,7 +33,7 @@ func TestAddBaseHappyPath(t *testing.T) {
 	fakeFS := fs.MakeFakeFS()
 	bases := strings.Split(baseDirectoryPaths, ",")
 	for _, base := range bases {
-		fakeFS.Mkdir(base, 0777)
+		fakeFS.Mkdir(base)
 	}
 	fakeFS.WriteFile(constants.KustomizationFileName, []byte(kustomizationContent))
 
@@ -60,7 +60,7 @@ func TestAddBaseAlreadyThere(t *testing.T) {
 	// Create fake directories
 	bases := strings.Split(baseDirectoryPaths, ",")
 	for _, base := range bases {
-		fakeFS.Mkdir(base, 0777)
+		fakeFS.Mkdir(base)
 	}
 	fakeFS.WriteFile(constants.KustomizationFileName, []byte(kustomizationContent))
 
@@ -77,9 +77,9 @@ func TestAddBaseAlreadyThere(t *testing.T) {
 	}
 	var expectedErrors []string
 	for _, base := range bases {
-		error := "base " + base + " already in kustomization file"
-		expectedErrors = append(expectedErrors, error)
-		if !stringInSlice(error, expectedErrors) {
+		msg := "base " + base + " already in kustomization file"
+		expectedErrors = append(expectedErrors, msg)
+		if !stringInSlice(msg, expectedErrors) {
 			t.Errorf("unexpected error %v", err)
 		}
 	}

pkg/commands/addpatch.go

@@ -70,9 +70,8 @@ func (o *addPatchOptions) Complete(cmd *cobra.Command, args []string) error {
 
 // RunAddPatch runs addPatch command (do real work).
 func (o *addPatchOptions) RunAddPatch(fsys fs.FileSystem) error {
-	_, err := fsys.Stat(o.patchFilePath)
-	if err != nil {
-		return err
+	if !fsys.Exists(o.patchFilePath) {
+		return errors.New(o.patchFilePath + " doesn't exist")
 	}
 
 	mf, err := newKustomizationFile(constants.KustomizationFileName, fsys)

pkg/commands/addresource.go

@@ -70,11 +70,9 @@ func (o *addResourceOptions) Complete(cmd *cobra.Command, args []string) error {
 
 // RunAddResource runs addResource command (do real work).
 func (o *addResourceOptions) RunAddResource(fsys fs.FileSystem) error {
-	_, err := fsys.Stat(o.resourceFilePath)
-	if err != nil {
-		return err
+	if !fsys.Exists(o.resourceFilePath) {
+		return errors.New(o.resourceFilePath + " does not exist")
 	}
-
 	mf, err := newKustomizationFile(constants.KustomizationFileName, fsys)
 	if err != nil {
 		return err

pkg/commands/kustomizationfile.go

@@ -45,25 +45,23 @@ func newKustomizationFile(mPath string, fsys fs.FileSystem) (*kustomizationFile,
 }
 
 func (mf *kustomizationFile) validate() error {
-	f, err := mf.fsys.Stat(mf.path)
-	if err != nil {
+	if !mf.fsys.Exists(mf.path) {
 		errorMsg := fmt.Sprintf("Missing kustomization file '%s'.\n", mf.path)
 		merr := interror.KustomizationError{KustomizationPath: mf.path, ErrorMsg: errorMsg}
 		return merr
 	}
-	if f.IsDir() {
+	if mf.fsys.IsDir(mf.path) {
 		mf.path = path.Join(mf.path, constants.KustomizationFileName)
-		_, err = mf.fsys.Stat(mf.path)
-		if err != nil {
+		if !mf.fsys.Exists(mf.path) {
 			errorMsg := fmt.Sprintf("Missing kustomization file '%s'.\n", mf.path)
 			merr := interror.KustomizationError{KustomizationPath: mf.path, ErrorMsg: errorMsg}
 			return merr
 		}
 	} else {
 		if !strings.HasSuffix(mf.path, constants.KustomizationFileName) {
-			errorMsg := fmt.Sprintf("Kustomization file path (%s) should have %s suffix\n", mf.path, constants.KustomizationFileSuffix)
-			merr := interror.KustomizationError{KustomizationPath: mf.path, ErrorMsg: errorMsg}
-			return merr
+			errorMsg := fmt.Sprintf("Kustomization file path (%s) should have %s suffix\n",
+				mf.path, constants.KustomizationFileSuffix)
+			return interror.KustomizationError{KustomizationPath: mf.path, ErrorMsg: errorMsg}
 		}
 	}
 	return nil

pkg/commands/kustomizationfile_test.go

@@ -62,7 +62,7 @@ func TestEmptyFile(t *testing.T) {
 func TestNewNotExist(t *testing.T) {
 	badSuffix := "foo.bar"
 	fakeFS := fs.MakeFakeFS()
-	fakeFS.Mkdir(".", 0644)
+	fakeFS.Mkdir(".")
 	fakeFS.Create(badSuffix)
 	_, err := newKustomizationFile(constants.KustomizationFileName, fakeFS)
 	if err == nil {

pkg/fs/fakefs.go

@@ -18,7 +18,6 @@ package fs
 
 import (
 	"fmt"
-	"os"
 )
 
 var _ FileSystem = &FakeFS{}
@@ -42,7 +41,7 @@ func (fs *FakeFS) Create(name string) (File, error) {
 }
 
 // Mkdir assures a fake directory appears in the in-memory file system.
-func (fs *FakeFS) Mkdir(name string, perm os.FileMode) error {
+func (fs *FakeFS) Mkdir(name string) error {
 	fs.m[name] = makeDir(name)
 	return nil
 }
@@ -55,12 +54,19 @@ func (fs *FakeFS) Open(name string) (File, error) {
 	return fs.m[name], nil
 }
 
-// Stat always returns nil FileInfo, and returns an error if file does not exist.
-func (fs *FakeFS) Stat(name string) (os.FileInfo, error) {
-	if f, found := fs.m[name]; found {
-		return &Fakefileinfo{f}, nil
+// Exists returns true if file is known.
+func (fs *FakeFS) Exists(name string) bool {
+	_, found := fs.m[name]
+	return found
+}
+
+// IsDir returns true if the file exists and is a directory.
+func (fs *FakeFS) IsDir(name string) bool {
+	f, found := fs.m[name]
+	if !found {
+		return false
 	}
-	return nil, fmt.Errorf("file %q does not exist", name)
+	return f.dir
 }
 
 // ReadFile always returns an empty bytes and error depending on content of m.

pkg/fs/fakefs_test.go

@@ -21,34 +21,25 @@ import (
 	"testing"
 )
 
-func TestStatNotExist(t *testing.T) {
+func TestExists(t *testing.T) {
 	x := MakeFakeFS()
-	info, err := x.Stat("foo")
-	if info != nil {
-		t.Fatalf("expected nil info")
-	}
-	if err == nil {
-		t.Fatalf("expected error")
+	if x.Exists("foo") {
+		t.Fatalf("expected no foo")
 	}
 }
 
-func TestStat(t *testing.T) {
+func TestIsDir(t *testing.T) {
 	x := MakeFakeFS()
 	expectedName := "my-dir"
-	err := x.Mkdir(expectedName, 0666)
+	err := x.Mkdir(expectedName)
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
-	info, err := x.Stat(expectedName)
-	if err != nil {
-		t.Fatalf("unexpected error: %v", err)
+	if !x.Exists(expectedName) {
+		t.Fatalf(expectedName + " should exist")
 	}
-	name := info.Name()
-	if name != expectedName {
-		t.Fatalf("expected %v but got %v", expectedName, name)
-	}
-	if !info.IsDir() {
-		t.Fatalf("expected IsDir() return true")
+	if !x.IsDir(expectedName) {
+		t.Fatalf(expectedName + " should be a dir")
 	}
 }
 
@@ -61,12 +52,8 @@ func TestCreate(t *testing.T) {
 	if err != nil {
 		t.Fatalf("unexpected error")
 	}
-	info, err := x.Stat("foo")
-	if info == nil {
-		t.Fatalf("expected non-nil info")
-	}
-	if err != nil {
-		t.Fatalf("expected no error")
+	if !x.Exists("foo") {
+		t.Fatalf("expected foo to exist")
 	}
 }
 

pkg/fs/fs.go

@@ -25,9 +25,10 @@ import (
 // FileSystem groups basic os filesystem methods.
 type FileSystem interface {
 	Create(name string) (File, error)
-	Mkdir(name string, perm os.FileMode) error
+	Mkdir(name string) error
 	Open(name string) (File, error)
-	Stat(name string) (os.FileInfo, error)
+	IsDir(name string) bool
+	Exists(name string) bool
 	ReadFile(name string) ([]byte, error)
 	ReadFiles(name string) (map[string][]byte, error)
 	WriteFile(name string, data []byte) error

pkg/fs/realfs.go

@@ -36,13 +36,27 @@ func MakeRealFS() FileSystem {
 func (realFS) Create(name string) (File, error) { return os.Create(name) }
 
 // Mkdir delegates to os.Mkdir.
-func (realFS) Mkdir(name string, perm os.FileMode) error { return os.Mkdir(name, perm) }
+func (realFS) Mkdir(name string) error {
+	return os.Mkdir(name, 0777|os.ModeDir)
+}
 
 // Open delegates to os.Open.
 func (realFS) Open(name string) (File, error) { return os.Open(name) }
 
-// Stat delegates to os.Stat.
-func (realFS) Stat(name string) (os.FileInfo, error) { return os.Stat(name) }
+// Exists returns true if os.Stat succeeds.
+func (realFS) Exists(name string) bool {
+	_, err := os.Stat(name)
+	return err == nil
+}
+
+// IsDir delegates to os.Stat and FileInfo.IsDir
+func (realFS) IsDir(name string) bool {
+	info, err := os.Stat(name)
+	if err != nil {
+		return false
+	}
+	return info.IsDir()
+}
 
 // ReadFile delegates to ioutil.ReadFile.
 func (realFS) ReadFile(name string) ([]byte, error) { return ioutil.ReadFile(name) }

pkg/fs/realfs_test.go

@@ -26,17 +26,30 @@ import (
 func TestReadFilesRealFS(t *testing.T) {
 	x := MakeRealFS()
 	testDir := "kustomize_testing_dir"
-	err := x.Mkdir(testDir, 0777)
+	err := x.Mkdir(testDir)
 	defer os.RemoveAll(testDir)
 
 	if err != nil {
 		t.Fatalf("unexpected error %s", err)
 	}
+	if !x.Exists(testDir) {
+		t.Fatalf("expected existence")
+	}
+	if !x.IsDir(testDir) {
+		t.Fatalf("expected directory")
+	}
 
 	err = x.WriteFile(path.Join(testDir, "foo"), []byte(`foo`))
 	if err != nil {
 		t.Fatalf("unexpected error %s", err)
 	}
+	if !x.Exists(path.Join(testDir, "foo")) {
+		t.Fatalf("expected foo")
+	}
+	if x.IsDir(path.Join(testDir, "foo")) {
+		t.Fatalf("expected foo not to be a directory")
+	}
+
 	err = x.WriteFile(path.Join(testDir, "bar"), []byte(`bar`))
 	if err != nil {
 		t.Fatalf("unexpected error %s", err)

pkg/internal/loadertest/fakeloader.go

@@ -18,8 +18,6 @@ limitations under the License.
 package loadertest
 
 import (
-	"os"
-
 	"github.com/kubernetes-sigs/kustomize/pkg/fs"
 	"github.com/kubernetes-sigs/kustomize/pkg/loader"
 )
@@ -49,8 +47,8 @@ func (f FakeLoader) AddFile(fullFilePath string, content []byte) error {
 }
 
 // AddDirectory adds a fake directory to the file system.
-func (f FakeLoader) AddDirectory(fullDirPath string, mode os.FileMode) error {
-	return f.fs.Mkdir(fullDirPath, mode)
+func (f FakeLoader) AddDirectory(fullDirPath string) error {
+	return f.fs.Mkdir(fullDirPath)
 }
 
 // Root returns root.

pkg/resmap/secret.go

@@ -18,11 +18,12 @@ package resmap
 
 import (
 	"context"
-	"os"
 	"os/exec"
 	"path/filepath"
 	"time"
 
+	"os"
+
 	"github.com/kubernetes-sigs/kustomize/pkg/resource"
 	"github.com/kubernetes-sigs/kustomize/pkg/types"
 	"github.com/pkg/errors"
@@ -62,6 +63,7 @@ func makeSecret(p string, sArgs types.SecretArgs) (*corev1.Secret, error) {
 	return s, nil
 }
 
+// Run a command, return its output as the secret.
 func createSecretKey(wd string, command string) ([]byte, error) {
 	fi, err := os.Stat(wd)
 	if err != nil || !fi.IsDir() {