From 9432671887a206c80a3fd0ac61a64987f02bdacc Mon Sep 17 00:00:00 2001 From: Jeffrey Regan Date: Wed, 18 Jul 2018 11:33:30 -0700 Subject: [PATCH] Replace os.Stat with IsDir, simplifying FS abstraction. --- Gopkg.lock | 2 +- pkg/app/application_test.go | 3 +-- pkg/commands/addbase.go | 5 ++-- pkg/commands/addbase_test.go | 10 ++++---- pkg/commands/addpatch.go | 5 ++-- pkg/commands/addresource.go | 6 ++--- pkg/commands/kustomizationfile.go | 14 +++++------ pkg/commands/kustomizationfile_test.go | 2 +- pkg/fs/fakefs.go | 20 +++++++++------ pkg/fs/fakefs_test.go | 35 ++++++++------------------ pkg/fs/fs.go | 5 ++-- pkg/fs/realfs.go | 20 ++++++++++++--- pkg/fs/realfs_test.go | 15 ++++++++++- pkg/internal/loadertest/fakeloader.go | 6 ++--- pkg/resmap/secret.go | 4 ++- 15 files changed, 83 insertions(+), 69 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 8a442aaa9..5f9d782c0 100644 --- a/Gopkg.lock +++ b/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 diff --git a/pkg/app/application_test.go b/pkg/app/application_test.go index 39a8d9f9a..c97fd110c 100644 --- a/pkg/app/application_test.go +++ b/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.") } diff --git a/pkg/commands/addbase.go b/pkg/commands/addbase.go index deca221c5..b97b40cb3 100644 --- a/pkg/commands/addbase.go +++ b/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) diff --git a/pkg/commands/addbase_test.go b/pkg/commands/addbase_test.go index 17dadd7df..3360a3844 100644 --- a/pkg/commands/addbase_test.go +++ b/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) } } diff --git a/pkg/commands/addpatch.go b/pkg/commands/addpatch.go index 3eaaed9e5..fd311261a 100644 --- a/pkg/commands/addpatch.go +++ b/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) diff --git a/pkg/commands/addresource.go b/pkg/commands/addresource.go index 254aa7af1..638ee8ca1 100644 --- a/pkg/commands/addresource.go +++ b/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 diff --git a/pkg/commands/kustomizationfile.go b/pkg/commands/kustomizationfile.go index fbd595c10..dbe4378e2 100644 --- a/pkg/commands/kustomizationfile.go +++ b/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 diff --git a/pkg/commands/kustomizationfile_test.go b/pkg/commands/kustomizationfile_test.go index bc31638c5..d3e50bd1f 100644 --- a/pkg/commands/kustomizationfile_test.go +++ b/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 { diff --git a/pkg/fs/fakefs.go b/pkg/fs/fakefs.go index 572522edf..cea0decf4 100644 --- a/pkg/fs/fakefs.go +++ b/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. diff --git a/pkg/fs/fakefs_test.go b/pkg/fs/fakefs_test.go index c7c7b8d05..7477e5cd3 100644 --- a/pkg/fs/fakefs_test.go +++ b/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") } } diff --git a/pkg/fs/fs.go b/pkg/fs/fs.go index acc5e7d38..42e2e6fdc 100644 --- a/pkg/fs/fs.go +++ b/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 diff --git a/pkg/fs/realfs.go b/pkg/fs/realfs.go index 47fca063a..fa3409b84 100644 --- a/pkg/fs/realfs.go +++ b/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) } diff --git a/pkg/fs/realfs_test.go b/pkg/fs/realfs_test.go index b2c5e3e86..afe64c8c9 100644 --- a/pkg/fs/realfs_test.go +++ b/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) diff --git a/pkg/internal/loadertest/fakeloader.go b/pkg/internal/loadertest/fakeloader.go index b4b016c83..d9d41ae8d 100644 --- a/pkg/internal/loadertest/fakeloader.go +++ b/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. diff --git a/pkg/resmap/secret.go b/pkg/resmap/secret.go index d693d7b12..1fab2fe14 100644 --- a/pkg/resmap/secret.go +++ b/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() {