Repository: puppetlabs/wash
Pull request #554 (2019-10-20 05:17:15)

Add the delete action
Subsequent PRs will implement Delete for the core plugins.

ekinanp: I tested this with the `local_fs` plugin; its delete implementation delegated to `rm -rf`. Things seem to be working OK.

Patch:
diff --git a/api/cache.go b/api/cache.go
index b528954..a1b4446 100644
--- a/api/cache.go
+++ b/api/cache.go
@@ -29,14 +29,11 @@ var cacheHandler handler = func(w http.ResponseWriter, r *http.Request) *errorRe
 		return errResp
 	}
 
-	deleted, err := plugin.ClearCacheFor(path)
-	if err != nil {
-		return unknownErrorResponse(fmt.Errorf("Could not use path %v in a regexp: %v", path, err))
-	}
+	deleted := plugin.ClearCacheFor(path)
 	activity.Record(r.Context(), "API: Cache DELETE %v %+v", path, deleted)
 
 	jsonEncoder := json.NewEncoder(w)
-	if err = jsonEncoder.Encode(deleted); err != nil {
+	if err := jsonEncoder.Encode(deleted); err != nil {
 		return unknownErrorResponse(fmt.Errorf("Could not marshal deleted keys for %v: %v", path, err))
 	}
 	return nil
diff --git a/api/client/client.go b/api/client/client.go
index bfc6abd..21bac8e 100644
--- a/api/client/client.go
+++ b/api/client/client.go
@@ -39,6 +39,7 @@ type Client interface {
 	// A "nil" schema means that the schema's unknown.
 	Schema(path string) (*apitypes.EntrySchema, error)
 	Screenview(name string, params analytics.Params) error
+	Delete(path string) error
 }
 
 // A domainSocketClient is a wash API client.
@@ -292,3 +293,9 @@ func (c *domainSocketClient) Screenview(name string, params analytics.Params) er
 	_, err = c.doRequest(http.MethodPost, "/analytics/screenview", url.Values{}, bytes.NewReader(jsonBody))
 	return err
 }
+
+// Delete deletes the entry at "path"
+func (c *domainSocketClient) Delete(path string) error {
+	_, err := c.doRequest(http.MethodDelete, "/fs/delete", url.Values{"path": []string{path}}, nil)
+	return err
+}
diff --git a/api/delete.go b/api/delete.go
new file mode 100644
index 0000000..a418e5b
--- /dev/null
+++ b/api/delete.go
@@ -0,0 +1,36 @@
+package api
+
+import (
+	"net/http"
+
+	"github.com/puppetlabs/wash/activity"
+	"github.com/puppetlabs/wash/plugin"
+)
+
+// swagger:route DELETE /fs/delete delete
+//
+// Deletes the entry at the specified path.
+//
+//     Schemes: http
+//
+//     Responses:
+//       200
+//       400: errorResp
+//       404: errorResp
+//       500: errorResp
+var deleteHandler handler = func(w http.ResponseWriter, r *http.Request) *errorResponse {
+	ctx := r.Context()
+	entry, path, errResp := getEntryFromRequest(r)
+	if errResp != nil {
+		return errResp
+	}
+	if !plugin.DeleteAction().IsSupportedOn(entry) {
+		return unsupportedActionResponse(path, plugin.DeleteAction())
+	}
+	err := plugin.Delete(ctx, entry.(plugin.Deletable))
+	if err != nil {
+		return erroredActionResponse(path, plugin.DeleteAction(), err.Error())
+	}
+	activity.Record(ctx, "API: Delete %v", path)
+	return nil
+}
diff --git a/api/list.go b/api/list.go
index 1d3bf9b..1e95e9a 100644
--- a/api/list.go
+++ b/api/list.go
@@ -55,12 +55,13 @@ var listHandler handler = func(w http.ResponseWriter, r *http.Request) *errorRes
 		return erroredActionResponse(path, plugin.ListAction(), err.Error())
 	}
 
-	result := make([]apitypes.Entry, 0, len(entries))
-	for _, entry := range entries {
+	result := make([]apitypes.Entry, 0, entries.Len())
+	entries.Range(func(_ string, entry plugin.Entry) bool {
 		apiEntry := toAPIEntry(entry)
 		apiEntry.Path = path + "/" + apiEntry.CName
 		result = append(result, apiEntry)
-	}
+		return true
+	})
 	// Sort entries so they have a deterministic order.
 	sort.Slice(result, func(i, j int) bool { return result[i].Name < result[j].Name })
 	activity.Record(ctx, "API: List %v %+v", path, result)
diff --git a/api/server.go b/api/server.go
index 54f4dfa..8e63278 100644
--- a/api/server.go
+++ b/api/server.go
@@ -123,6 +123,7 @@ func StartAPI(
 	r.Handle("/fs/stream", streamHandler).Methods(http.MethodGet)
 	r.Handle("/fs/exec", execHandler).Methods(http.MethodPost)
 	r.Handle("/fs/schema", schemaHandler).Methods(http.MethodGet)
+	r.Handle("/fs/delete", deleteHandler).Methods(http.MethodDelete)
 	r.Handle("/cache", cacheHandler).Methods(http.MethodDelete)
 	r.Handle("/history", historyHandler).Methods(http.MethodGet)
 	r.Handle("/history/{index:[0-9]+}", historyEntryHandler).Methods(http.MethodGet)
diff --git a/cmd/delete.go b/cmd/delete.go
new file mode 100644
index 0000000..57dd99f
--- /dev/null
+++ b/cmd/delete.go
@@ -0,0 +1,81 @@
+package cmd
+
+import (
+	"fmt"
+	"sync"
+
+	"github.com/spf13/cobra"
+
+	cmdutil "github.com/puppetlabs/wash/cmd/util"
+	"github.com/puppetlabs/wash/plugin"
+)
+
+func deleteCommand() *cobra.Command {
+	deleteCmd := &cobra.Command{
+		Use:   "delete <path> [<path>]",
+		Short: "Deletes the entries at the specified paths",
+		Long: `Deletes the entries at the specified paths, prompting the user for confirmation
+before deleting each entry.`,
+		Args: cobra.MinimumNArgs(1),
+		RunE: toRunE(deleteMain),
+	}
+	deleteCmd.Flags().BoolP("force", "f", false, "Skip confirmation")
+
+	return deleteCmd
+}
+
+func deleteMain(cmd *cobra.Command, args []string) exitCode {
+	paths := args
+	force, err := cmd.Flags().GetBool("force")
+	if err != nil {
+		panic(err.Error())
+	}
+
+	conn := cmdutil.NewClient()
+
+	// Deletion's done in parallel for a better UX.
+	var pathsToDelete []string
+
+	// First, confirm the paths to delete.
+	for _, path := range paths {
+		var deletionConfirmed bool
+		if force || !plugin.IsInteractive() {
+			deletionConfirmed = true
+		} else {
+			msg := fmt.Sprintf("remove %v?", path)
+			input, err := plugin.Prompt(msg)
+			if err != nil {
+				cmdutil.ErrPrintf("failed to get confirmation: %v", err)
+				return exitCode{1}
+			}
+			// Assume confirmation if input starts with "y" or "Y". This matches rm.
+			deletionConfirmed = len(input) > 0 && (input[0] == 'y' || input[0] == 'Y')
+		}
+		if deletionConfirmed {
+			pathsToDelete = append(pathsToDelete, path)
+		}
+	}
+
+	// Next, process each request in its own goroutine. We use a writeMux because
+	// ErrPrintf is not thread-safe.
+	var wg sync.WaitGroup
+	var anyFailed bool
+	var writeMux sync.Mutex
+	for _, path := range pathsToDelete {
+		wg.Add(1)
+		go func(path string) {
+			if err := conn.Delete(path); err != nil {
+				writeMux.Lock()
+				defer writeMux.Unlock()
+				cmdutil.ErrPrintf("%v\n", err)
+				anyFailed = true
+			}
+			wg.Done()
+		}(path)
+	}
+	wg.Wait()
+	if anyFailed {
+		return exitCode{1}
+	}
+	return exitCode{0}
+}
diff --git a/cmd/root.go b/cmd/root.go
index 002116f..1ca341e 100644
--- a/cmd/root.go
+++ b/cmd/root.go
@@ -163,6 +163,7 @@ then starts your system shell with shortcuts configured for wash subcommands.`,
 	addCommand(rootCmd, infoCommand())
 	addCommand(rootCmd, streeCommand())
 	addCommand(rootCmd, docsCommand())
+	addCommand(rootCmd, deleteCommand())
 
 	return rootCmd
 }
diff --git a/cmd/validate.go b/cmd/validate.go
index f8aa4b4..f10e699 100644
--- a/cmd/validate.go
+++ b/cmd/validate.go
@@ -146,9 +146,10 @@ func validateMain(cmd *cobra.Command, args []string) exitCode {
 
 	// We use a worker pool to limit work-in-progress. Put the plugin on the worker pool.
 	wp := cmdutil.NewPool(parallel)
-	for _, e := range entries {
+	entries.Range(func(_ string, e plugin.Entry) bool {
 		wp.Submit(func() { processEntry(ctx, pw, wp, e, all, errs) })
-	}
+		return true
+	})
 
 	// Wait for work to complete.
 	wp.Finish()
@@ -272,18 +273,17 @@ func processEntry(ctx context.Context, pw progress.Writer, wp cmdutil.Pool, e pl
 			return
 		}
 		cancelFunc()
-		entries := obj.(map[string]plugin.Entry)
+		entries := obj.(*plugin.EntryMap)
 
 		if all {
-			for _, entry := range entries {
-				// Make a local copy for the lambda to capture.
-				entry := entry
+			entries.Range(func(_ string, entry plugin.Entry) bool {
 				wp.Submit(func() { processEntry(ctx, pw, wp, entry, all, errs) })
-			}
+				return true
+			})
 		} else {
 			// Group children by ones that look "similar", and select one from each group to test.
 			groups := make(map[criteria][]plugin.Entry)
-			for _, entry := range entries {
+			entries.Range(func(_ string, entry plugin.Entry) bool {
 				// Skip files that we shouldn't read. This includes character devices, devices, and
 				// those we don't have permission to read.
 				attr := plugin.Attributes(entry)
@@ -291,7 +291,7 @@ func processEntry(ctx context.Context, pw progress.Writer, wp cmdutil.Pool, e pl
 					(mode&os.ModeCharDevice == os.ModeCharDevice ||
 						mode&os.ModeNamedPipe == os.ModeNamedPipe ||
 						mode&os.ModeDevice == os.ModeDevice || mode&0400 == 0) {
-					continue
+					return true
 				}
 
 				ccrit := newCriteria(entry)
@@ -300,7 +300,9 @@ func processEntry(ctx context.Context, pw progress.Writer, wp cmdutil.Pool, e pl
 				if ccrit.typeID == "" || ccrit.typeID != crit.typeID {
 					groups[ccrit] = append(groups[ccrit], entry)
 				}
-			}
+
+				return true
+			})
 
 			for _, items := range groups {
 				entry := items[rand.Intn(len(items))]
diff --git a/fuse/dir.go b/fuse/dir.go
index 72f2d72..09b16a6 100644
--- a/fuse/dir.go
+++ b/fuse/dir.go
@@ -24,7 +24,7 @@ func newDir(p *dir, e plugin.Parent) *dir {
 	return &dir{newFuseNode("d", p, e)}
 }
 
-func (d *dir) children(ctx context.Context) (map[string]plugin.Entry, error) {
+func (d *dir) children(ctx context.Context) (*plugin.EntryMap, error) {
 	// Check for an updated entry in case it has static state.
 	updatedEntry, err := d.refind(ctx)
 	if err != nil {
@@ -53,7 +53,7 @@ func (d *dir) Lookup(ctx context.Context, req *fuse.LookupRequest, resp *fuse.Lo
 	}
 
 	cname := req.Name
-	entry, ok := entries[cname]
+	entry, ok := entries.Load(cname)
 	if !ok {
 		log.Debugf("FUSE: %v not found in %v", req.Name, d)
 		return nil, fuse.ENOENT
@@ -79,15 +79,16 @@ func (d *dir) ReadDirAll(ctx context.Context) ([]fuse.Dirent, error) {
 		return nil, err
 	}
 
-	res := make([]fuse.Dirent, 0, len(entries))
-	for cname, entry := range entries {
+	res := make([]fuse.Dirent, 0, entries.Len())
+	entries.Range(func(cname string, entry plugin.Entry) bool {
 		var de fuse.Dirent
 		de.Name = cname
 		if plugin.ListAction().IsSupportedOn(entry) {
 			de.Type = fuse.DT_Dir
 		}
 		res = append(res, de)
-	}
+		return true
+	})
 	activity.Record(ctx, "FUSE: Listed in %v: %+v", d, res)
 	return res, nil
 }
diff --git a/plugin/action.go b/plugin/action.go
index b453ae5..c19828b 100644
--- a/plugin/action.go
+++ b/plugin/action.go
@@ -33,6 +33,7 @@ var listAction = newAction("list", "Parent")
 var readAction = newAction("read", "Readable")
 var streamAction = newAction("stream", "Streamable")
 var execAction = newAction("exec", "Execable")
+var deleteAction = newAction("delete", "Deletable")
 
 // ListAction represents the list action
 func ListAction() Action {
@@ -54,6 +55,11 @@ func ExecAction() Action {
 	return execAction
 }
 
+// DeleteAction represents the delete action
+func DeleteAction() Action {
+	return deleteAction
+}
+
 // Actions returns all of the available Wash actions as a map
 // of <action_name> => <action_object>.
 func Actions() map[string]Action {
@@ -98,6 +104,9 @@ func SupportedActionsOf(entry Entry) []string {
 		if _, ok := entry.(Execable); ok {
 			actions = append(actions, ExecAction().Name)
 		}
+		if _, ok := entry.(Deletable); ok {
+			actions = append(actions, DeleteAction().Name)
+		}
 
 		return actions
 	}
diff --git a/plugin/analyticsWrappers.go b/plugin/analyticsWrappers.go
index 1e01f5b..cb049bc 100644
--- a/plugin/analyticsWrappers.go
+++ b/plugin/analyticsWrappers.go
@@ -9,7 +9,7 @@ import (
 
 // ListWithAnalytics is a wrapper to plugin.List. Use it when you need to report
 // a 'List' invocation to analytics. Otherwise, use plugin.List
-func ListWithAnalytics(ctx context.Context, p Parent) (map[string]Entry, error) {
+func ListWithAnalytics(ctx context.Context, p Parent) (*EntryMap, error) {
 	submitMethodInvocation(ctx, p, "List")
 	return List(ctx, p)
 }
@@ -35,6 +35,13 @@ func ExecWithAnalytics(ctx context.Context, e Execable, cmd string, args []strin
 	return Exec(ctx, e, cmd, args, opts)
 }
 
+// DeleteWithAnalytics is a wrapper to plugin.Delete. Use it when you need to report a
+// 'Delete' invocation to analytics. Otherwise, use plugin.Delete.
+func DeleteWithAnalytics(ctx context.Context, d Deletable) error {
+	submitMethodInvocation(ctx, d, "Delete")
+	return Delete(ctx, d)
+}
+
 func submitMethodInvocation(ctx context.Context, e Entry, method string) {
 	isCorePluginEntry := e.Schema() != nil
 	if !isCorePluginEntry {
diff --git a/plugin/cache.go b/plugin/cache.go
index 8d93f71..309991f 100644
--- a/plugin/cache.go
+++ b/plugin/cache.go
@@ -63,15 +63,15 @@ const opQualifier = "^[a-zA-Z]+::"
 // This method exists to simplify ClearCacheFor's tests.
 // Specifically, it lets us decouple the regex's correctness
 // from the cache's implementation.
-func opKeysRegex(path string) (*regexp.Regexp, error) {
+func opKeysRegex(path string) *regexp.Regexp {
 	var expr string
 	if path == "/" {
 		expr = opQualifier + "/.*"
 	} else {
-		expr = opQualifier + "/" + strings.Trim(path, "/") + "($|/.*)"
+		expr = opQualifier + "/" + regexp.QuoteMeta(strings.Trim(path, "/")) + "($|/.*)"
 	}
 
-	return regexp.Compile(expr)
+	return regexp.MustCompile(expr)
 }
 
 // ClearCacheFor removes entries from the cache that match or are children of the provided path.
@@ -79,13 +79,9 @@ func opKeysRegex(path string) (*regexp.Regexp, error) {
 //
 // TODO: If path == "/", we could optimize this by calling cache.Flush(). Not important
 // right now, but may be worth considering in the future.
-func ClearCacheFor(path string) ([]string, error) {
-	rx, err := opKeysRegex(path)
-	if err != nil {
-		return nil, err
-	}
-
-	return cache.Delete(rx), nil
+func ClearCacheFor(path string) []string {
+	rx := opKeysRegex(path)
+	return cache.Delete(rx)
 }
 
 type opFunc func() (interface{}, error)
@@ -150,7 +146,7 @@ func (c DuplicateCNameErr) Error() string {
 //
 // CachedList returns a map of <entry_cname> => <entry_object> to optimize
 // querying a specific entry.
-func cachedList(ctx context.Context, p Parent) (map[string]Entry, error) {
+func cachedList(ctx context.Context, p Parent) (*EntryMap, error) {
 	cachedEntries, err := cachedDefaultOp(ctx, ListOp, p, func() (interface{}, error) {
 		// Including the entry's ID allows plugin authors to use any Cached* methods defined on the
 		// children after their creation. This is necessary when the child's Cached* methods are used
@@ -160,11 +156,11 @@ func cachedList(ctx context.Context, p Parent) (map[string]Entry, error) {
 			return nil, err
 		}
 
-		searchedEntries := make(map[string]Entry)
+		searchedEntries := newEntryMap()
 		for _, entry := range entries {
 			cname := CName(entry)
 
-			if duplicateEntry, ok := searchedEntries[cname]; ok {
+			if duplicateEntry, ok := searchedEntries.mp[cname]; ok {
 				return nil, DuplicateCNameErr{
 					ParentID:                 p.id(),
 					FirstChildName:           duplicateEntry.name(),
@@ -179,7 +175,8 @@ func cachedList(ctx context.Context, p Parent) (map[string]Entry, error) {
 				// Skip entries that are expected to be inaccessible.
 				continue
 			}
-			searchedEntries[cname] = entry
+
+			searchedEntries.mp[cname] = entry
 
 			// Ensure ID is set on all entries so that we can use it for caching later in places
 			// where the context doesn't include the parent's ID.
@@ -195,7 +192,7 @@ func cachedList(ctx context.Context, p Parent) (map[string]Entry, error) {
 		return nil, err
 	}
 
-	return cachedEntries.(map[string]Entry), nil
+	return cachedEntries.(*EntryMap), nil
 }
 
 // CachedOpen caches a Readable object's Open method.
diff --git a/plugin/entryMap.go b/plugin/entryMap.go
new file mode 100644
index 0000000..ff338f4
--- /dev/null
+++ b/plugin/entryMap.go
@@ -0,0 +1,62 @@
+package plugin
+
+import "sync"
+
+// EntryMap is a thread-safe map of <entry_cname> => <entry_object>.
+// It's API is (mostly) symmetric with sync.Map.
+type EntryMap struct {
+	mp  map[string]Entry
+	mux sync.RWMutex
+}
+
+func newEntryMap() *EntryMap {
+	return &EntryMap{
+		mp: make(map[string]Entry),
+	}
+}
+
+// Load retrieves an entry
+func (m *EntryMap) Load(cname string) (Entry, bool) {
+	m.mux.RLock()
+	defer m.mux.RUnlock()
+
+	entry, ok := m.mp[cname]
+	return entry, ok
+}
+
+// Delete deletes the entry from the map
+func (m *EntryMap) Delete(cname string) {
+	m.mux.Lock()
+	defer m.mux.Unlock()
+
+	delete(m.mp, cname)
+}
+
+// Len returns the number of entries in the map
+func (m *EntryMap) Len() int {
+	m.mux.RLock()
+	defer m.mux.RUnlock()
+
+	return len(m.mp)
+}
+
+// Range iterates over the map, applying f to each (cname, entry)
+// pair. If f returns false, then Each will break out of the loop.
+func (m *EntryMap) Range(f func(string, Entry) bool) {
+	m.mux.RLock()
+	defer m.mux.RUnlock()
+
+	for cname, entry := range m.mp {
+		if !f(cname, entry) {
+			break
+		}
+	}
+}
+
+// Map returns m's underlying map. It can only be called by the tests.
+func (m *EntryMap) Map() map[string]Entry {
+	if notRunningTests() {
+		panic("plugin.EntryMap#Map can only be called by the tests")
+	}
+	return m.mp
+}
diff --git a/plugin/externalPluginEntry.go b/plugin/externalPluginEntry.go
index 80f5146..447341b 100644
--- a/plugin/externalPluginEntry.go
+++ b/plugin/externalPluginEntry.go
@@ -375,6 +375,11 @@ func (e *externalPluginEntry) Metadata(ctx context.Context) (JSONObject, error)
 	return metadata, nil
 }
 
+func (e *externalPluginEntry) Delete(ctx context.Context) error {
+	_, err := e.script.InvokeAndWait(ctx, "delete", e)
+	return err
+}
+
 func (e *externalPluginEntry) Stream(ctx context.Context) (io.ReadCloser, error) {
 	inv := e.script.NewInvocation(ctx, "stream", e)
 	cmd := inv.command
diff --git a/plugin/findEntry.go b/plugin/findEntry.go
index 0fe0488..8cc4df0 100644
--- a/plugin/findEntry.go
+++ b/plugin/findEntry.go
@@ -19,7 +19,7 @@ func FindEntry(ctx context.Context, start Entry, segments []string) (Entry, erro
 			}
 
 			// Search for the specific entry
-			entry, ok := entries[segment]
+			entry, ok := entries.Load(segment)
 			if !ok {
 				reason := fmt.Sprintf("The %v entry does not exist", segment)
 				if len(visitedSegments) != 0 {
diff --git a/plugin/methodWrappers.go b/plugin/methodWrappers.go
index 2c9cce1..137326b 100644
--- a/plugin/methodWrappers.go
+++ b/plugin/methodWrappers.go
@@ -105,11 +105,11 @@ func Schema(e Entry) (*EntrySchema, error) {
 	return schema(e)
 }
 
-// List lists the parent's children. It returns a map of <entry_cname> => <entry_object>
-// to optimize querying a specific entry.
+// List lists the parent's children. It returns an EntryMap to optimize querying a specific
+// entry.
 //
 // Note that List's results could be cached.
-func List(ctx context.Context, p Parent) (map[string]Entry, error) {
+func List(ctx context.Context, p Parent) (*EntryMap, error) {
 	return cachedList(ctx, p)
 }
 
@@ -137,3 +137,20 @@ func Exec(ctx context.Context, e Execable, cmd string, args []string, opts ExecO
 func Stream(ctx context.Context, s Streamable) (io.ReadCloser, error) {
 	return s.Stream(ctx)
 }
+
+// Delete deletes the given entry.
+func Delete(ctx context.Context, d Deletable) error {
+	if err := d.Delete(ctx); err != nil {
+		return err
+	}
+	ClearCacheFor(d.id())
+	// Delete this entry from the parent's cached list result
+	segments := strings.Split(d.id(), "/")
+	parentID := strings.Join(segments[:len(segments)-1], "/")
+	entries, _ := cache.Get(defaultOpCodeToNameMap[ListOp], parentID)
+	if entries != nil {
+		cname := segments[len(segments)-1]
+		entries.(*EntryMap).Delete(cname)
+	}
+	return nil
+}
diff --git a/plugin/registry.go b/plugin/registry.go
index 93e6e58..1721e9b 100644
--- a/plugin/registry.go
+++ b/plugin/registry.go
@@ -50,6 +50,11 @@ func (r *Registry) RegisterPlugin(root Root, config map[string]interface{}) erro
 				msg := fmt.Sprintf("r.RegisterPlugin: the %v plugin's already been registered", root.name())
 				panic(msg)
 			}
+
+			if DeleteAction().IsSupportedOn(root) {
+				msg := fmt.Sprintf("r.RegisterPlugin: the %v plugin's root implements delete", root.name())
+				panic(msg)
+			}
 		}
 
 		r.plugins[root.name()] = root
diff --git a/plugin/types.go b/plugin/types.go
index ca76851..e1b58f2 100644
--- a/plugin/types.go
+++ b/plugin/types.go
@@ -194,3 +194,12 @@ type Readable interface {
 	Entry
 	Open(context.Context) (SizedReader, error)
 }
+
+// Deletable is an entry that can be deleted. Entries that implement Delete
+// should ensure that it and all its children are removed. If the entry has
+// any dependencies that need to be deleted, then Delete should return an
+// error.
+type Deletable interface {
+	Entry
+	Delete(context.Context) error
+}


Test patch:
diff --git a/api/cache_test.go b/api/cache_test.go
index 658a4fd..e3e5b12 100644
--- a/api/cache_test.go
+++ b/api/cache_test.go
@@ -96,7 +96,7 @@ func (suite *CacheHandlerTestSuite) TestClearCache() {
 
 	expectedChildren := make(map[string]plugin.Entry)
 	if children, err := plugin.List(reqCtx, parent); suite.Nil(err) {
-		suite.Equal(expectedChildren, children)
+		suite.Equal(expectedChildren, children.Map())
 	}
 
 	// Test clearing a different cache
@@ -107,7 +107,7 @@ func (suite *CacheHandlerTestSuite) TestClearCache() {
 	suite.Equal("[]\n", w.Body.String())
 
 	if children, err := plugin.List(context.Background(), parent); suite.Nil(err) {
-		suite.Equal(expectedChildren, children)
+		suite.Equal(expectedChildren, children.Map())
 	}
 
 	// Test clearing the cache
@@ -118,7 +118,7 @@ func (suite *CacheHandlerTestSuite) TestClearCache() {
 	suite.Equal(`["List::/dir"]`, strings.TrimSpace(w.Body.String()))
 
 	if children, err := plugin.List(context.Background(), parent); suite.Nil(err) {
-		suite.Equal(expectedChildren, children)
+		suite.Equal(expectedChildren, children.Map())
 	}
 
 	parent.AssertNumberOfCalls(suite.T(), "List", 2)
diff --git a/cmd/internal/cmdtest/client.go b/cmd/internal/cmdtest/client.go
index e1f2c0e..df99e2e 100644
--- a/cmd/internal/cmdtest/client.go
+++ b/cmd/internal/cmdtest/client.go
@@ -73,3 +73,9 @@ func (c *MockClient) Screenview(name string, params analytics.Params) error {
 	args := c.Called(name, params)
 	return args.Error(1)
 }
+
+// Delete mocks Client#Delete
+func (c *MockClient) Delete(path string) error {
+	args := c.Called(path)
+	return args.Error(0)
+}
diff --git a/plugin/cache_test.go b/plugin/cache_test.go
index 7c805cd..8bede6e 100644
--- a/plugin/cache_test.go
+++ b/plugin/cache_test.go
@@ -62,17 +62,6 @@ func (suite *CacheTestSuite) TearDownTest() {
 	UnsetTestCache()
 }
 
-func (suite *CacheTestSuite) opKeysRegex(path string) *regexp.Regexp {
-	rx, err := opKeysRegex(path)
-	if err != nil {
-		suite.FailNow(
-			fmt.Sprintf("opKeysRegex unexpectedly errored with %v", err),
-		)
-	}
-
-	return rx
-}
-
 func (suite *CacheTestSuite) TestOpNameRegex() {
 	suite.Regexp(opNameRegex, "a")
 	suite.Regexp(opNameRegex, "A")
@@ -89,7 +78,7 @@ func (suite *CacheTestSuite) TestOpNameRegex() {
 }
 
 func (suite *CacheTestSuite) TestOpKeysRegex() {
-	rx := suite.opKeysRegex("/a")
+	rx := opKeysRegex("/a")
 
 	// Test that it matches children
 	suite.Regexp(rx, "Test::/a/b")
@@ -103,22 +92,25 @@ func (suite *CacheTestSuite) TestOpKeysRegex() {
 	suite.NotRegexp(rx, "Test::/bc/d")
 
 	// Test that it matches root, and children of root
-	rx = suite.opKeysRegex("/")
+	rx = opKeysRegex("/")
 	suite.Regexp(rx, "Test::/")
 	suite.Regexp(rx, "Test::/a")
 	suite.Regexp(rx, "Test::/a/b")
 
+	// Test that it matches a path containing regex characters
+	rx = opKeysRegex("/foo*[]")
+	suite.Regexp(rx, "Test::/foo*[]")
+	suite.Regexp(rx, "Test::/foo*[]/bar(")
+	suite.Regexp(rx, "Test::/foo*[]/bar(/baz)")
 }
 
 func (suite *CacheTestSuite) TestClearCache() {
 	path := "/a"
-	rx := suite.opKeysRegex(path)
+	rx := opKeysRegex(path)
 
 	suite.cache.On("Delete", rx).Return([]string{"/a"})
-	deleted, err := ClearCacheFor(path)
-	if !suite.NoError(err) {
-		suite.Equal([]string{"/a"}, deleted)
-	}
+	deleted := ClearCacheFor(path)
+	suite.Equal([]string{"/a"}, deleted)
 }
 
 type cacheTestsMockEntry struct {
@@ -274,7 +266,8 @@ func toMap(children []Entry) map[string]Entry {
 
 func (suite *CacheTestSuite) TestCachedListDefaultOp() {
 	mockChildren := []Entry{newCacheTestsMockEntry("mockChild")}
-	mungedOpValue := toMap(mockChildren)
+	mungedOpValue := newEntryMap()
+	mungedOpValue.mp = toMap(mockChildren)
 	suite.testCachedDefaultOp(ListOp, "List", mockChildren, mungedOpValue, func(ctx context.Context, e Entry) (interface{}, error) {
 		return cachedList(ctx, e.(Parent))
 	})
@@ -323,9 +316,9 @@ func (suite *CacheTestSuite) TestCachedListSetEntryID() {
 	entry.On("List", mock.Anything).Return(mockChildren, nil).Once()
 	children, err := cachedList(ctx, entry)
 	if suite.NoError(err) {
-		if suite.Equal(toMap(mockChildren), children) {
-			suite.Equal("/foo#child1", children["foo#child1"].id())
-			suite.Equal("/child2", children["child2"].id())
+		if suite.Equal(toMap(mockChildren), children.mp) {
+			suite.Equal("/foo#child1", children.mp["foo#child1"].id())
+			suite.Equal("/child2", children.mp["child2"].id())
 		}
 	}
 
@@ -336,9 +329,9 @@ func (suite *CacheTestSuite) TestCachedListSetEntryID() {
 	entry.On("List", mock.Anything).Return(mockChildren, nil).Once()
 	children, err = cachedList(ctx, entry)
 	if suite.NoError(err) {
-		if suite.Equal(toMap(mockChildren), children) {
-			suite.Equal("/parent/foo#child1", children["foo#child1"].id())
-			suite.Equal("/parent/child2", children["child2"].id())
+		if suite.Equal(toMap(mockChildren), children.mp) {
+			suite.Equal("/parent/foo#child1", children.mp["foo#child1"].id())
+			suite.Equal("/parent/child2", children.mp["child2"].id())
 		}
 	}
 }
diff --git a/plugin/entryMap_test.go b/plugin/entryMap_test.go
new file mode 100644
index 0000000..6bc5d59
--- /dev/null
+++ b/plugin/entryMap_test.go
@@ -0,0 +1,141 @@
+package plugin
+
+import (
+	"strconv"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/suite"
+)
+
+type EntryMapTestSuite struct {
+	suite.Suite
+}
+
+func (suite *EntryMapTestSuite) TestLoad_Delete_Len() {
+	m := newEntryMap()
+
+	_, ok := m.Load("foo")
+	suite.False(ok)
+
+	suite.Equal(0, m.Len())
+
+	expectedEntry := newCacheTestsMockEntry("foo")
+	m.mp["foo"] = expectedEntry
+
+	actualEntry, ok := m.Load("foo")
+	suite.Equal(expectedEntry, actualEntry)
+	suite.True(ok)
+
+	suite.Equal(1, m.Len())
+
+	m.Delete("foo")
+	_, ok = m.Load("foo")
+	suite.False(ok)
+
+	suite.Equal(0, m.Len())
+}
+
+func (suite *EntryMapTestSuite) TestRange() {
+	m := newEntryMap()
+
+	entries := []Entry{
+		newCacheTestsMockEntry("foo"),
+		newCacheTestsMockEntry("bar"),
+	}
+	for _, entry := range entries {
+		m.mp[CName(entry)] = entry
+	}
+
+	// Test that iteration works
+	entryMap := make(map[string]Entry)
+	m.Range(func(cname string, entry Entry) bool {
+		entryMap[cname] = entry
+		return true
+	})
+	suite.Equal(m.Map(), entryMap)
+
+	// Test that break works
+	count := 0
+	m.Range(func(cname string, entry Entry) bool {
+		if cname == "foo" {
+			return false
+		}
+		count++
+		return true
+	})
+	// <= 1 is to account for the fact that Go map's iteration order
+	// is random.
+	suite.True(count <= 1)
+}
+
+func (suite *EntryMapTestSuite) TestConcurrentReadWrite() {
+	m := newEntryMap()
+
+	var wg sync.WaitGroup
+	var startCh = make(chan struct{})
+	var doneCh = make(chan struct{})
+
+	// Load, Delete, and Range all acquire locks to avoid a concurrent
+	// read/write panic. Thus, this test launches goroutines that invoke
+	// each method concurrently. Idea is that if any one of those methods
+	// fail to acquire a lock (including the right lock, like a Write lock
+	// for Delete), then this test will panic. NumGoroutinesPerMethod is
+	// arbitrary, idea is it should be high enough that we can detect a panic
+	// but low enough that the test won't take too long.
+	//
+	// The test passes if all the goroutines successfully return.
+	//
+	// NOTE: Testing Len() is a bit tricky because a concurrent len(mp) and
+	// delete(mp) does not cause a read/write panic. Since its implementation is
+	// simple enough, we omit Len() to avoid further complicating the tests.
+	NumGoroutinesPerMethod := 40
+	for i := 0; i < NumGoroutinesPerMethod; i++ {
+		key := strconv.Itoa(i)
+		m.mp[key] = nil
+
+		// Load
+		wg.Add(1)
+		go func(i int) {
+			defer wg.Done()
+			<-startCh
+			m.Load(key)
+		}(i)
+
+		// Delete
+		wg.Add(1)
+		go func(i int) {
+			defer wg.Done()
+			<-startCh
+			m.Delete(key)
+		}(i)
+
+		// Range
+		wg.Add(1)
+		go func(i int) {
+			defer wg.Done()
+			<-startCh
+			m.Range(func(_ string, _ Entry) bool {
+				return false
+			})
+		}(i)
+	}
+	time.AfterFunc(5*time.Second, func() {
+		select {
+		case <-doneCh:
+			// Pass-thru
+		default:
+			panic("goroutines did not successfully return after 5 seconds. Did you forget to release a lock?")
+		}
+	})
+
+	// Start the goroutines and wait for them to finish.
+	close(startCh)
+	wg.Wait()
+	close(doneCh)
+}
+
+func TestEntryMap(t *testing.T) {
+	suite.Run(t, new(EntryMapTestSuite))
+}
diff --git a/plugin/externalPluginEntry_test.go b/plugin/externalPluginEntry_test.go
index 376e691..16fc9d0 100644
--- a/plugin/externalPluginEntry_test.go
+++ b/plugin/externalPluginEntry_test.go
@@ -689,6 +689,32 @@ func (suite *ExternalPluginEntryTestSuite) TestMetadata_Implemented() {
 	}
 }
 
+func (suite *ExternalPluginEntryTestSuite) TestDelete() {
+	mockScript := &mockExternalPluginScript{path: "plugin_script"}
+	entry := &externalPluginEntry{
+		EntryBase: NewEntry("foo"),
+		methods:   map[string]interface{}{"delete": nil},
+		script:    mockScript,
+	}
+	entry.SetTestID("/foo")
+
+	ctx := context.Background()
+	mockInvokeAndWait := func(err error) {
+		mockScript.OnInvokeAndWait(ctx, "delete", entry).Return(mockInvocation([]byte{}), err).Once()
+	}
+
+	// Test that if InvokeAndWait errors, then Delete returns its error
+	mockErr := fmt.Errorf("execution error")
+	mockInvokeAndWait(mockErr)
+	err := entry.Delete(ctx)
+	suite.EqualError(mockErr, err.Error())
+
+	// Test that Delete properly deletes the entry
+	mockInvokeAndWait(nil)
+	err = entry.Delete(ctx)
+	suite.NoError(err)
+}
+
 // TODO: Add tests for stdoutStreamer, Stream and Exec
 // once the API for Stream and Exec's at a more stable
 // state.
diff --git a/plugin/methodWrappers_test.go b/plugin/methodWrappers_test.go
index a94a325..ae68fcd 100644
--- a/plugin/methodWrappers_test.go
+++ b/plugin/methodWrappers_test.go
@@ -1,33 +1,37 @@
 package plugin
 
 import (
+	"context"
+	"fmt"
 	"testing"
 	"time"
 
-	"github.com/puppetlabs/wash/datastore"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/suite"
 )
 
 type MethodWrappersTestSuite struct {
 	suite.Suite
+	cache *cacheTestsMockCache
 }
 
-func (suite *MethodWrappersTestSuite) SetupSuite() {
-	SetTestCache(datastore.NewMemCache())
+func (suite *MethodWrappersTestSuite) SetupTest() {
+	suite.cache = &cacheTestsMockCache{}
+	SetTestCache(suite.cache)
 }
 
-func (suite *MethodWrappersTestSuite) TearDownSuite() {
+func (suite *MethodWrappersTestSuite) TearDownTest() {
 	UnsetTestCache()
+	suite.cache = nil
 }
 
 func (suite *MethodWrappersTestSuite) TestName() {
-	e := newmethodWrappersTestsMockEntry("foo")
+	e := newMethodWrappersTestsMockEntry("foo")
 	suite.Equal(Name(e), "foo")
 }
 
 func (suite *MethodWrappersTestSuite) TestCName() {
-	e := newmethodWrappersTestsMockEntry("foo/bar/baz")
+	e := newMethodWrappersTestsMockEntry("foo/bar/baz")
 	suite.Equal("foo#bar#baz", CName(e))
 
 	e.SetSlashReplacer(':')
@@ -35,7 +39,7 @@ func (suite *MethodWrappersTestSuite) TestCName() {
 }
 
 func (suite *MethodWrappersTestSuite) TestID() {
-	e := newmethodWrappersTestsMockEntry("foo/bar")
+	e := newMethodWrappersTestsMockEntry("foo/bar")
 
 	e.SetTestID("")
 	suite.Panics(
@@ -52,11 +56,26 @@ type methodWrappersTestsMockEntry struct {
 	mock.Mock
 }
 
-func (e *methodWrappersTestsMockEntry) Schema() *EntrySchema {
+func (m *methodWrappersTestsMockEntry) Schema() *EntrySchema {
 	return nil
 }
 
-func newmethodWrappersTestsMockEntry(name string) *methodWrappersTestsMockEntry {
+func (m *methodWrappersTestsMockEntry) List(ctx context.Context) ([]Entry, error) {
+	args := m.Called(ctx)
+	return args.Get(0).([]Entry), args.Error(1)
+}
+
+func (m *methodWrappersTestsMockEntry) Delete(ctx context.Context) error {
+	args := m.Called(ctx)
+	return args.Error(0)
+}
+
+func (m *methodWrappersTestsMockEntry) Open(ctx context.Context) (SizedReader, error) {
+	args := m.Called(ctx)
+	return args.Get(0).(SizedReader), args.Error(1)
+}
+
+func newMethodWrappersTestsMockEntry(name string) *methodWrappersTestsMockEntry {
 	e := &methodWrappersTestsMockEntry{
 		EntryBase: NewEntry(name),
 	}
@@ -67,19 +86,76 @@ func newmethodWrappersTestsMockEntry(name string) *methodWrappersTestsMockEntry
 }
 
 func (suite *MethodWrappersTestSuite) TestAttributes() {
-	e := newmethodWrappersTestsMockEntry("mockEntry")
+	suite.cache.On("Get", mock.Anything, mock.Anything).Return(nil, nil)
+
+	e := newMethodWrappersTestsMockEntry("mockEntry")
 	e.attr = EntryAttributes{}
 	e.attr.SetCtime(time.Now())
 	suite.Equal(e.attr, Attributes(e))
 }
 
 func (suite *MethodWrappersTestSuite) TestPrefetched() {
-	e := newmethodWrappersTestsMockEntry("mockEntry")
+	e := newMethodWrappersTestsMockEntry("mockEntry")
 	suite.False(IsPrefetched(e))
 	e.Prefetched()
 	suite.True(IsPrefetched(e))
 }
 
+func (suite *MethodWrappersTestSuite) TestDelete_ReturnsDeleteError() {
+	ctx := context.Background()
+	e := newMethodWrappersTestsMockEntry("foo")
+
+	expectedErr := fmt.Errorf("an error")
+	e.On("Delete", ctx).Return(expectedErr)
+
+	err := Delete(ctx, e)
+	suite.Equal(expectedErr, err)
+}
+
+func (suite *MethodWrappersTestSuite) TestDelete_DeletesEntry() {
+	ctx := context.Background()
+	e := newMethodWrappersTestsMockEntry("foo")
+	e.On("Delete", ctx).Return(nil)
+
+	suite.cache.On("Get", mock.Anything, mock.Anything).Return(nil, nil)
+	suite.cache.On("Delete", mock.Anything).Return([]string{})
+
+	err := Delete(ctx, e)
+	if suite.NoError(err) {
+		e.AssertExpectations(suite.T())
+	}
+}
+
+func (suite *MethodWrappersTestSuite) TestDelete_ClearsEntryCache() {
+	e := newMethodWrappersTestsMockEntry("foo")
+	e.On("Delete", mock.Anything).Return(nil)
+
+	suite.cache.On("Get", mock.Anything, mock.Anything).Return(nil, nil)
+	suite.cache.On("Delete", opKeysRegex(e.id())).Return([]string{})
+
+	err := Delete(context.Background(), e)
+	if suite.NoError(err) {
+		suite.cache.AssertExpectations(suite.T())
+	}
+}
+
+func (suite *MethodWrappersTestSuite) TestDelete_DeletesEntryFromParentsCachedEntryMap() {
+	e := newMethodWrappersTestsMockEntry("bar")
+	e.SetTestID("/foo/bar")
+	e.On("Delete", mock.Anything).Return(nil)
+
+	entryMap := newEntryMap()
+	entryMap.mp["bar"] = e
+	suite.cache.On("Get", "List", "/foo").Return(entryMap, nil)
+	suite.cache.On("Delete", mock.Anything).Return([]string{})
+
+	err := Delete(context.Background(), e)
+	if suite.NoError(err) {
+		suite.cache.AssertCalled(suite.T(), "Get", "List", "/foo")
+		suite.NotContains(entryMap.mp, "bar")
+	}
+}
+
 func TestMethodWrappers(t *testing.T) {
 	suite.Run(t, new(MethodWrappersTestSuite))
 }
diff --git a/plugin/registry_test.go b/plugin/registry_test.go
index 2d1cf5c..fd6b6b7 100644
--- a/plugin/registry_test.go
+++ b/plugin/registry_test.go
@@ -117,6 +117,28 @@ func (suite *RegistryTestSuite) TestRegisterPluginRegisteredPlugin() {
 	suite.Panics(panicFunc, "r.RegisterPlugin: the mine plugin's already been registered")
 }
 
+type mockRootWithDelete struct {
+	*mockRoot
+}
+
+func (m *mockRootWithDelete) Delete(ctx context.Context) error {
+	return nil
+}
+
+func (suite *RegistryTestSuite) TestRegisterPluginPluginRootImplementsDelete() {
+	panicFunc := func() {
+		reg := NewRegistry()
+		m := &mockRootWithDelete{
+			mockRoot: &mockRoot{
+				EntryBase: NewEntry("mine"),
+			},
+		}
+		_ = reg.RegisterPlugin(m, map[string]interface{}{})
+	}
+
+	suite.Panics(panicFunc, "r.RegisterPlugin: the mine plugin's root implements delete")
+}
+
 func TestRegistry(t *testing.T) {
 	suite.Run(t, new(RegistryTestSuite))
 }
diff --git a/volume/fs_test.go b/volume/fs_test.go
index 9f8426e..0a4c0ea 100644
--- a/volume/fs_test.go
+++ b/volume/fs_test.go
@@ -79,12 +79,13 @@ func (suite *fsTestSuite) find(parent plugin.Parent, path string) plugin.Entry {
 		if !suite.NoError(err) {
 			suite.FailNow("Listing entries failed")
 		}
-		for nm, match := range entries {
+		entries.Range(func(nm string, match plugin.Entry) bool {
 			if nm == name {
 				entry = match
-				break
+				return false
 			}
-		}
+			return true
+		})
 	}
 	suite.Equal(names[len(names)-1], plugin.Name(entry))
 	return entry
@@ -158,17 +159,17 @@ func (suite *fsTestSuite) TestFSListTwice() {
 	if !suite.NoError(err) {
 		suite.FailNow("Listing entries failed")
 	}
-	suite.Equal(1, len(entries))
-	suite.Contains(entries, "path")
+	suite.Equal(1, entries.Len())
+	suite.Contains(entries.Map(), "path")
 
-	entries1, err := plugin.List(suite.ctx, entries["path"].(plugin.Parent))
+	entries1, err := plugin.List(suite.ctx, entries.Map()["path"].(plugin.Parent))
 	if suite.NoError(err) {
-		suite.Equal(1, len(entries1))
-		suite.Contains(entries1, "has")
-		entries2, err := plugin.List(suite.ctx, entries1["has"].(plugin.Parent))
+		suite.Equal(1, entries1.Len())
+		suite.Contains(entries1.Map(), "has")
+		entries2, err := plugin.List(suite.ctx, entries1.Map()["has"].(plugin.Parent))
 		if suite.NoError(err) {
-			suite.Equal(1, len(entries2))
-			suite.Contains(entries2, "got")
+			suite.Equal(1, entries2.Len())
+			suite.Contains(entries2.Map(), "got")
 		}
 	}
 }
@@ -194,14 +195,13 @@ func (suite *fsTestSuite) TestFSListExpiredCache() {
 	if !suite.NoError(err) {
 		suite.FailNow("Listing entries failed")
 	}
-	suite.Equal(1, len(entries))
-	suite.Contains(entries, "path")
+	suite.Equal(1, entries.Len())
+	suite.Contains(entries.Map(), "path")
 
-	_, err = plugin.ClearCacheFor("/instance/fs")
-	suite.NoError(err)
+	_ = plugin.ClearCacheFor("/instance/fs")
 	entries, err = plugin.List(suite.ctx, entry)
 	if suite.NoError(err) {
-		suite.Equal(1, len(entries))
+		suite.Equal(1, entries.Len())
 	}
 	suite.Implements((*plugin.Parent)(nil), suite.find(fs, "var/log"))
 }