fsstore to fail when the root dir does not exist (istio#786)

fixes istio#724

Former-commit-id: 9b8b68573f67a396aa55733fabdba6b2b2f0e5fd

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 {

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 {

mixer/pkg/config/fsstore_test.go

@@ -25,17 +25,39 @@ 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)
 		}}
 	})
 }
 
+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)
 

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)
 	}

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")},