matchbox/storage: Add profile and group deletes to the Store

* Add deleteFile to the Dir restricted filesystem accessor

matchbox/storage/filestore.go

@@ -56,6 +56,11 @@ func (s *fileStore) GroupGet(id string) (*storagepb.Group, error) {
 	return group, err
 }
 
+// GroupDelete deletes a machine Group by id.
+func (s *fileStore) GroupDelete(id string) error {
+	return Dir(s.root).deleteFile(filepath.Join("groups", id+".json"))
+}
+
 // GroupList lists all machine Groups.
 func (s *fileStore) GroupList() ([]*storagepb.Group, error) {
 	files, err := Dir(s.root).readDir("groups")
@@ -101,6 +106,11 @@ func (s *fileStore) ProfileGet(id string) (*storagepb.Profile, error) {
 	return profile, err
 }
 
+// ProfileDelete deletes a profile by id.
+func (s *fileStore) ProfileDelete(id string) error {
+	return Dir(s.root).deleteFile(filepath.Join("profiles", id+".json"))
+}
+
 // ProfileList lists all profiles.
 func (s *fileStore) ProfileList() ([]*storagepb.Profile, error) {
 	files, err := Dir(s.root).readDir("profiles")

matchbox/storage/filestore_test.go

@@ -13,7 +13,7 @@ import (
 	fake "github.com/coreos/matchbox/matchbox/storage/testfakes"
 )
 
-func TestGroupPut(t *testing.T) {
+func TestGroupPutAndDelete(t *testing.T) {
 	dir, err := setup(&fake.FixedStore{})
 	assert.Nil(t, err)
 	defer os.RemoveAll(dir)
@@ -22,11 +22,20 @@ func TestGroupPut(t *testing.T) {
 	// assert that:
 	// - Group creation was successful
 	// - Group can be retrieved by id
+	// - Group can be deleted by id
 	err = store.GroupPut(fake.Group)
 	assert.Nil(t, err)
+
 	group, err := store.GroupGet(fake.Group.Id)
 	assert.Nil(t, err)
 	assert.Equal(t, fake.Group, group)
+
+	err = store.GroupDelete(fake.Group.Id)
+	assert.Nil(t, err)
+	_, err = store.GroupGet(fake.Group.Id)
+	if assert.Error(t, err) {
+		assert.IsType(t, err, &os.PathError{})
+	}
 }
 
 func TestGroupGet(t *testing.T) {
@@ -82,7 +91,7 @@ func TestGroupList(t *testing.T) {
 	}
 }
 
-func TestProfilePut(t *testing.T) {
+func TestProfilePutAndDelete(t *testing.T) {
 	dir, err := setup(&fake.FixedStore{})
 	assert.Nil(t, err)
 	defer os.RemoveAll(dir)
@@ -91,11 +100,20 @@ func TestProfilePut(t *testing.T) {
 	// assert that:
 	// - Profile creation was successful
 	// - Profile can be retrieved by id
+	// - Profile can be deleted by id
 	err = store.ProfilePut(fake.Profile)
 	assert.Nil(t, err)
+
 	profile, err := store.ProfileGet(fake.Profile.Id)
 	assert.Nil(t, err)
 	assert.Equal(t, fake.Profile, profile)
+
+	err = store.ProfileDelete(fake.Profile.Id)
+	assert.Nil(t, err)
+	_, err = store.ProfileGet(fake.Profile.Id)
+	if assert.Error(t, err) {
+		assert.IsType(t, err, &os.PathError{})
+	}
 }
 
 func TestProfileGet(t *testing.T) {

matchbox/storage/fs.go

@@ -55,6 +55,16 @@ func (d Dir) writeFile(path string, data []byte) error {
 	return ioutil.WriteFile(path, data, defaultFileMode)
 }
 
+// deleteFile removes the file at the given path, restricted to a specific
+// directory tree.
+func (d Dir) deleteFile(path string) error {
+	path, err := d.sanitize(path)
+	if err != nil {
+		return err
+	}
+	return os.Remove(path)
+}
+
 // Borrowed directly from net/http Dir.Open and FileServer.
 func (d Dir) sanitize(name string) (string, error) {
 	if filepath.Separator != '/' && strings.ContainsRune(name, filepath.Separator) ||

matchbox/storage/fs_test.go

@@ -25,6 +25,7 @@ func TestDir(t *testing.T) {
 	assert.Nil(t, err)
 	defer os.RemoveAll(tdir)
 
+	// create a Dir, restricted to the temp dir
 	dir := Dir(tdir)
 	// write files rooted in the dir
 	for _, c := range cases {
@@ -41,6 +42,17 @@ func TestDir(t *testing.T) {
 		assert.Nil(t, err)
 		assert.Equal(t, []byte(c.expected), b)
 	}
+	// delete the files that were written
+	for _, c := range cases {
+		err := dir.deleteFile(c.path)
+		assert.Nil(t, err)
+	}
+	// ensure the expected files were removed
+	for _, c := range cases {
+		rpath := filepath.Join(tdir, c.expected)
+		_, err := os.Stat(rpath)
+		assert.True(t, os.IsNotExist(err), "expected path %s would not exist", rpath)
+	}
 }
 
 func TestSanitizePath(t *testing.T) {

matchbox/storage/storage.go

@@ -18,6 +18,8 @@ type Store interface {
 	GroupPut(group *storagepb.Group) error
 	// GroupGet returns a machine Group by id.
 	GroupGet(id string) (*storagepb.Group, error)
+	// GroupDelete deletes a machine Group by id.
+	GroupDelete(id string) error
 	// GroupList lists all machine Groups.
 	GroupList() ([]*storagepb.Group, error)
 
@@ -25,6 +27,8 @@ type Store interface {
 	ProfilePut(profile *storagepb.Profile) error
 	// ProfileGet gets a profile by id.
 	ProfileGet(id string) (*storagepb.Profile, error)
+	// ProfileDelete deletes a profile by id.
+	ProfileDelete(id string) error
 	// ProfileList lists all profiles.
 	ProfileList() ([]*storagepb.Profile, error)
 

matchbox/storage/testfakes/broken_store.go

@@ -23,6 +23,11 @@ func (s *BrokenStore) GroupGet(id string) (*storagepb.Group, error) {
 	return nil, errIntentional
 }
 
+// GroupDelete returns an error.
+func (s *BrokenStore) GroupDelete(id string) error {
+	return errIntentional
+}
+
 // GroupList returns an error.
 func (s *BrokenStore) GroupList() (groups []*storagepb.Group, err error) {
 	return groups, errIntentional
@@ -38,6 +43,11 @@ func (s *BrokenStore) ProfileGet(id string) (*storagepb.Profile, error) {
 	return nil, errIntentional
 }
 
+// ProfileDelete returns an error.
+func (s *BrokenStore) ProfileDelete(id string) error {
+	return errIntentional
+}
+
 // ProfileList returns an error.
 func (s *BrokenStore) ProfileList() (profiles []*storagepb.Profile, err error) {
 	return profiles, errIntentional

matchbox/storage/testfakes/empty_store.go

@@ -19,6 +19,11 @@ func (s *EmptyStore) GroupGet(id string) (*storagepb.Group, error) {
 	return nil, fmt.Errorf("Group not found")
 }
 
+// GroupDelete returns a nil error (successful deletion).
+func (s *EmptyStore) GroupDelete(id string) error {
+	return nil
+}
+
 // GroupList returns an empty list of groups.
 func (s *EmptyStore) GroupList() (groups []*storagepb.Group, err error) {
 	return groups, nil
@@ -34,6 +39,11 @@ func (s *EmptyStore) ProfileGet(id string) (*storagepb.Profile, error) {
 	return nil, fmt.Errorf("Profile not found")
 }
 
+// ProfileDelete returns a nil error (successful deletion).
+func (s *EmptyStore) ProfileDelete(id string) error {
+	return nil
+}
+
 // ProfileList returns an empty list of profiles.
 func (s *EmptyStore) ProfileList() (profiles []*storagepb.Profile, err error) {
 	return profiles, nil

matchbox/storage/testfakes/fixed_store.go

@@ -40,6 +40,12 @@ func (s *FixedStore) GroupGet(id string) (*storagepb.Group, error) {
 	return nil, fmt.Errorf("Group not found")
 }
 
+// GroupDelete deletes the Group from the Groups map with the given id.
+func (s *FixedStore) GroupDelete(id string) error {
+	delete(s.Groups, id)
+	return nil
+}
+
 // GroupList returns the groups in the Groups map.
 func (s *FixedStore) GroupList() ([]*storagepb.Group, error) {
 	groups := make([]*storagepb.Group, len(s.Groups))
@@ -65,6 +71,12 @@ func (s *FixedStore) ProfileGet(id string) (*storagepb.Profile, error) {
 	return nil, fmt.Errorf("Profile not found")
 }
 
+// ProfileDelete deletes the Profile from the Profiles map with the given id.
+func (s *FixedStore) ProfileDelete(id string) error {
+	delete(s.Profiles, id)
+	return nil
+}
+
 // ProfileList returns the profiles in the Profiles map.
 func (s *FixedStore) ProfileList() ([]*storagepb.Profile, error) {
 	profiles := make([]*storagepb.Profile, len(s.Profiles))