From 43ef65e6b3cb40bd5318b567961a3b891710dc62 Mon Sep 17 00:00:00 2001
From: Jun Mukai <jun.mukai@gmail.com>
Date: Tue, 6 Jun 2017 15:58:47 -0700
Subject: [PATCH] fsstore to fail when the root dir does not exist (#786)

fixes #724

Former-commit-id: 9b8b68573f67a396aa55733fabdba6b2b2f0e5fd
---
 mixer/pkg/config/compatStore.go  |  6 +++++-
 mixer/pkg/config/fsstore.go      | 11 +++++++++--
 mixer/pkg/config/fsstore_test.go | 32 ++++++++++++++++++++++++++++----
 mixer/pkg/config/store.go        |  2 +-
 mixer/pkg/config/store_test.go   |  2 +-
 5 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/mixer/pkg/config/compatStore.go b/mixer/pkg/config/compatStore.go
index 393798179615..def736cd87a3 100644
--- a/mixer/pkg/config/compatStore.go
+++ b/mixer/pkg/config/compatStore.go
@@ -37,7 +37,11 @@ func NewCompatFSStore(globalConfigFile string, serviceConfigFile string) (KeyVal
 	if dir, err = ioutil.TempDir(os.TempDir(), "fsStore"); err != nil {
 		return nil, err
 	}
-	fs := newFSStore(dir).(*fsStore)
+	kvs, err := newFSStore(dir)
+	if err != nil {
+		return nil, err
+	}
+	fs := kvs.(*fsStore)
 
 	for k, v := range dm {
 		if data, err = fs.readfile(v); err != nil {
diff --git a/mixer/pkg/config/fsstore.go b/mixer/pkg/config/fsstore.go
index b28c295dd373..1b71e6740e3d 100644
--- a/mixer/pkg/config/fsstore.go
+++ b/mixer/pkg/config/fsstore.go
@@ -52,7 +52,14 @@ type readFileFunc func(filename string) ([]byte, error)
 type mkdirAllFunc func(path string, perm os.FileMode) error
 type removeFunc func(name string) error
 
-func newFSStore(root string) KeyValueStore {
+func newFSStore(root string) (KeyValueStore, error) {
+	finfo, err := os.Stat(root)
+	if err != nil {
+		return nil, err
+	}
+	if !finfo.IsDir() {
+		return nil, fmt.Errorf("%s is not a directory", root)
+	}
 	s := &fsStore{
 		root:     root,
 		tmpdir:   root + "/TMP",
@@ -69,7 +76,7 @@ func newFSStore(root string) KeyValueStore {
 		f, err = ioutil.TempFile(s.tmpdir, "fsStore")
 		return f, err
 	}
-	return s
+	return s, nil
 }
 
 func (f *fsStore) String() string {
diff --git a/mixer/pkg/config/fsstore_test.go b/mixer/pkg/config/fsstore_test.go
index 7b9f2b154719..cc0a85894344 100644
--- a/mixer/pkg/config/fsstore_test.go
+++ b/mixer/pkg/config/fsstore_test.go
@@ -25,7 +25,7 @@ import (
 func TestFSStore(t *testing.T) {
 	testStore(t, func() *kvMgr {
 		fsroot, _ := ioutil.TempDir("/tmp/", "fsStore")
-		f := newFSStore(fsroot)
+		f, _ := newFSStore(fsroot)
 		_ = os.MkdirAll(fsroot, os.ModeDir|os.ModePerm)
 		return &kvMgr{f, func() {
 			_ = os.RemoveAll(fsroot)
@@ -33,9 +33,31 @@ func TestFSStore(t *testing.T) {
 	})
 }
 
+func TestFSStore_FailToCreate(t *testing.T) {
+	_, err := newFSStore("/no/such/path")
+	if err == nil {
+		t.Errorf("expected failure, but succeeded")
+	}
+
+	tmpfile, err := ioutil.TempFile("", "dummy")
+	if err != nil {
+		t.Fatalf("can't create a tempfile for the test: %v", err)
+	}
+	defer func() { _ = os.Remove(tmpfile.Name()) }()
+
+	_, err = newFSStore(tmpfile.Name())
+	if err == nil {
+		t.Errorf("expected failure, but succeeded")
+	}
+	if !strings.Contains(err.Error(), "not a directory") {
+		t.Errorf("unexpected error %v", err)
+	}
+}
+
 func TestFSStore_Get(t *testing.T) {
 	fsroot, _ := ioutil.TempDir(os.TempDir(), "fsStore")
-	f := newFSStore(fsroot).(*fsStore)
+	store, _ := newFSStore(fsroot)
+	f := store.(*fsStore)
 	_ = os.MkdirAll(fsroot, os.ModeDir|os.ModePerm)
 	defer func(f string) { _ = os.RemoveAll(f) }(fsroot)
 
@@ -73,7 +95,8 @@ func TestFSStore_SetErrors(t *testing.T) {
 		{"close", errors.New("close error")},
 	} {
 		t.Run(tt.when, func(t *testing.T) {
-			f := newFSStore(fsroot).(*fsStore)
+			store, _ := newFSStore(fsroot)
+			f := store.(*fsStore)
 			if tt.when == "mkdir" {
 				f.mkdirAll = func(path string, perm os.FileMode) error {
 					return tt.err
@@ -117,7 +140,8 @@ func (f *fakeWriteCloser) Name() string { return "fakeWriteCloser" }
 
 func TestFSStore_Delete(t *testing.T) {
 	fsroot, _ := ioutil.TempDir(os.TempDir(), "fsStore")
-	f := newFSStore(fsroot).(*fsStore)
+	store, _ := newFSStore(fsroot)
+	f := store.(*fsStore)
 	_ = os.MkdirAll(fsroot, os.ModeDir|os.ModePerm)
 	defer func(f string) { _ = os.RemoveAll(f) }(fsroot)
 
diff --git a/mixer/pkg/config/store.go b/mixer/pkg/config/store.go
index 24cc42794b82..ed6678320af8 100644
--- a/mixer/pkg/config/store.go
+++ b/mixer/pkg/config/store.go
@@ -104,7 +104,7 @@ func NewStore(configURL string) (KeyValueStore, error) {
 
 	switch u.Scheme {
 	case FSUrl:
-		return newFSStore(u.Path), nil
+		return newFSStore(u.Path)
 	case RedisURL:
 		return newRedisStore(u)
 	}
diff --git a/mixer/pkg/config/store_test.go b/mixer/pkg/config/store_test.go
index 35092537fda7..e4c938f34d08 100644
--- a/mixer/pkg/config/store_test.go
+++ b/mixer/pkg/config/store_test.go
@@ -24,7 +24,7 @@ func TestNewStore(t *testing.T) {
 		url string
 		err error
 	}{
-		{"fs:///tmp/testdata/configroot", nil},
+		{"fs:///tmp", nil},
 		{"redis://:passwd@localhost:6379/1", errors.New("getsockopt")}, // connection error to the server
 		{"etcd:///tmp/testdata/configroot", errors.New("unknown")},
 		{"/tmp/testdata/configroot", errors.New("unknown")},