Repository: puppetlabs/wash
Pull request #265 (2019-05-20 18:09:54)

Update the external plugin interface with the recent changes
Contributions to this project require sign-off consistent with the [Developers Certificate of Origin](https://developercertificate.org). This can be as simple as using `git commit -s` on each commit.


Patch:
diff --git a/README.md b/README.md
index aece77b..8a56e2c 100644
--- a/README.md
+++ b/README.md
@@ -4,8 +4,6 @@
 
 `wash` helps you deal with all your remote or cloud-native infrastructure using the UNIX-y patterns and tools you already know and love!
 
-• [COMMUNITY](#community-feedback) • [FEATURES](#features) • [INSTALLATION](#installation) • [USAGE](#usage) • [ISSUES](#known-issues) • [ROADMAP](#roadmap) • [CONTRIBUTING](#contributing) •
-
 [![asciicast](https://asciinema.org/a/245046.svg)](https://asciinema.org/a/245046?cols=120&rows=30)
 
 Exploring, understanding, and inspecting modern infrastructure should be simple and straightforward. Whether it's containers, VMs, network devices, IoT stuff, or anything in between...they all have different ways of enumerating what you have, getting a stream of output, running commands, etc. Every vendor has its own tools and APIs that expose these features, each one different, each one bespoke. Thus, they are difficult to compose together to solve higher-level problems. And that's no fun at all!
diff --git a/plugin/action.go b/plugin/action.go
index 2001afe..3a5f17f 100644
--- a/plugin/action.go
+++ b/plugin/action.go
@@ -70,9 +70,11 @@ func Actions() map[string]Action {
 func SupportedActionsOf(entry Entry) []string {
 	switch t := entry.(type) {
 	case *externalPluginRoot:
-		return t.supportedActions
+		return t.methods
 	case *externalPluginEntry:
-		return t.supportedActions
+		// The supported actions are a subset of an external plugin
+		// entry's methods
+		return t.methods
 	default:
 		actions := make([]string, 0)
 
diff --git a/plugin/entryAttributes.go b/plugin/entryAttributes.go
index 0cdc4b6..a06374b 100644
--- a/plugin/entryAttributes.go
+++ b/plugin/entryAttributes.go
@@ -264,11 +264,13 @@ func (a *EntryAttributes) UnmarshalJSON(data []byte) error {
 		}
 		a.SetSize(sz)
 	}
-	meta, ok := mp["meta"].(JSONObject)
-	if !ok {
-		return fmt.Errorf("meta is not a JSON object")
+	if rawMeta, ok := mp["meta"]; ok {
+		meta, isObj := rawMeta.(JSONObject)
+		if !isObj {
+			return fmt.Errorf("meta is not a JSON object")
+		}
+		a.SetMeta(meta)
 	}
-	a.SetMeta(meta)
 	return nil
 }
 
diff --git a/plugin/externalPluginEntry.go b/plugin/externalPluginEntry.go
index 16501ae..f60caf7 100644
--- a/plugin/externalPluginEntry.go
+++ b/plugin/externalPluginEntry.go
@@ -7,12 +7,11 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
-	"os/exec"
 	"strings"
-	"syscall"
 	"time"
 
 	"github.com/puppetlabs/wash/activity"
+	"github.com/puppetlabs/wash/plugin/internal"
 )
 
 type decodedCacheTTLs struct {
@@ -24,7 +23,7 @@ type decodedCacheTTLs struct {
 // decodedExternalPluginEntry describes a decoded serialized entry.
 type decodedExternalPluginEntry struct {
 	Name                 string           `json:"name"`
-	SupportedActions     []string         `json:"supported_actions"`
+	Methods             []string          `json:"methods"`
 	SlashReplacementChar string           `json:"slash_replacement_char"`
 	CacheTTLs            decodedCacheTTLs `json:"cache_ttls"`
 	Attributes           EntryAttributes  `json:"attributes"`
@@ -35,13 +34,12 @@ func (e decodedExternalPluginEntry) toExternalPluginEntry() (*externalPluginEntr
 	if e.Name == "" {
 		return nil, fmt.Errorf("the entry name must be provided")
 	}
-	if e.SupportedActions == nil {
-		return nil, fmt.Errorf("the entry's supported actions must be provided")
+	if e.Methods == nil {
+		return nil, fmt.Errorf("the entry's methods must be provided")
 	}
-
 	entry := &externalPluginEntry{
 		EntryBase:        NewEntry(e.Name),
-		supportedActions: e.SupportedActions,
+		methods:          e.Methods,
 		state:            e.State,
 	}
 	entry.SetAttributes(e.Attributes)
@@ -54,17 +52,14 @@ func (e decodedExternalPluginEntry) toExternalPluginEntry() (*externalPluginEntr
 
 		entry.SetSlashReplacementChar([]rune(e.SlashReplacementChar)[0])
 	}
-
 	return entry, nil
 }
 
-// externalPluginEntry represents an entry from an external plugin. It consists
-// of its name, its object (as serialized JSON), and the path to its
-// main plugin script.
+// externalPluginEntry represents an external plugin entry
 type externalPluginEntry struct {
 	EntryBase
 	script           externalPluginScript
-	supportedActions []string
+	methods         []string
 	state            string
 }
 
@@ -80,25 +75,32 @@ func (e *externalPluginEntry) setCacheTTLs(ttls decodedCacheTTLs) {
 	}
 }
 
-// List lists the entry's children, if it has any.
+// implements returns true if the entry implements the given method,
+// false otherwise
+func (e *externalPluginEntry) implements(method string) bool {
+	for _, m := range e.methods {
+		if method == m {
+			return true
+		}
+	}
+	return false
+}
+
 func (e *externalPluginEntry) List(ctx context.Context) ([]Entry, error) {
-	stdout, err := e.script.InvokeAndWait(ctx, "list", e.id(), e.state)
+	stdout, err := e.script.InvokeAndWait(ctx, "list", e)
 	if err != nil {
 		return nil, err
 	}
-
 	var decodedEntries []decodedExternalPluginEntry
 	if err := json.Unmarshal(stdout, &decodedEntries); err != nil {
-		activity.Record(
+		return nil, newStdoutDecodeErr(
 			ctx,
-			"could not decode the entries from stdout\nreceived:\n%v\nexpected something like:\n%v",
-			strings.TrimSpace(string(stdout)),
-			"[{\"name\":\"<name_of_first_entry>\",\"supported_actions\":[\"list\"]},{\"name\":\"<name_of_second_entry>\",\"supported_actions\":[\"list\"]}]",
+			"the entries",
+			err,
+			stdout,
+			"[{\"name\":\"entry1\",\"methods\":[\"list\"]},{\"name\":\"entry2\",\"methods\":[\"list\"]}]",
 		)
-
-		return nil, fmt.Errorf("could not decode the entries from stdout: %v", err)
 	}
-
 	entries := make([]Entry, len(decodedEntries))
 	for i, decodedExternalPluginEntry := range decodedEntries {
 		entry, err := decodedExternalPluginEntry.toExternalPluginEntry()
@@ -106,108 +108,73 @@ func (e *externalPluginEntry) List(ctx context.Context) ([]Entry, error) {
 			return nil, err
 		}
 		entry.script = e.script
-
 		entries[i] = entry
 	}
-
 	return entries, nil
 }
 
-// Open returns the entry's content
 func (e *externalPluginEntry) Open(ctx context.Context) (SizedReader, error) {
-	stdout, err := e.script.InvokeAndWait(ctx, "read", e.id(), e.state)
+	stdout, err := e.script.InvokeAndWait(ctx, "read", e)
 	if err != nil {
 		return nil, err
 	}
-
 	return bytes.NewReader(stdout), nil
 }
 
-// Metadata displays the entry's metadata
 func (e *externalPluginEntry) Metadata(ctx context.Context) (JSONObject, error) {
-	stdout, err := e.script.InvokeAndWait(ctx, "metadata", e.id(), e.state)
+	if !e.implements("metadata") {
+		// The entry does not override the "Metadata" method so invoke
+		// the default
+		return e.EntryBase.Metadata(ctx)
+	}
+	stdout, err := e.script.InvokeAndWait(ctx, "metadata", e)
 	if err != nil {
 		return nil, err
 	}
-
 	var metadata JSONObject
 	if err := json.Unmarshal(stdout, &metadata); err != nil {
-		activity.Record(
+		return nil, newStdoutDecodeErr(
 			ctx,
-			"could not decode the metadata from stdout\nreceived:\n%v\nexpected something like:\n%v",
-			strings.TrimSpace(string(stdout)),
+			"the metadata",
+			err,
+			stdout,
 			"{\"key1\":\"value1\",\"key2\":\"value2\"}",
 		)
-
-		return nil, fmt.Errorf("could not decode the metadata from stdout: %v", err)
 	}
-
 	return metadata, nil
 }
 
-type stdoutStreamer struct {
-	stdoutRdr io.ReadCloser
-	cmd       *exec.Cmd
-}
-
-func (s *stdoutStreamer) Read(p []byte) (int, error) {
-	return s.stdoutRdr.Read(p)
-}
-
-func (s *stdoutStreamer) Close() error {
-	var err error
-
-	if closeErr := s.stdoutRdr.Close(); closeErr != nil {
-		err = fmt.Errorf("error closing stdout: %v", closeErr)
-	}
-
-	if signalErr := s.cmd.Process.Signal(syscall.SIGTERM); signalErr != nil {
-		signalErr = fmt.Errorf(
-			"error sending SIGTERM to process with PID %v: %v",
-			s.cmd.Process.Pid,
-			signalErr,
-		)
-
-		if err != nil {
-			err = fmt.Errorf("%v; and %v", err, signalErr)
-		} else {
-			err = signalErr
-		}
-	}
-
-	return err
-}
-
-// Stream streams the entry's content
 func (e *externalPluginEntry) Stream(ctx context.Context) (io.ReadCloser, error) {
-	cmd, stdoutR, stderrR, err := CreateCommand(
-		e.script.Path(),
-		"stream",
-		e.id(),
-		e.state,
-	)
-
+	cmd := e.script.NewInvocation(ctx, "stream", e)
+	stdoutR, err := cmd.StdoutPipe()
 	if err != nil {
 		return nil, err
 	}
-
-	cmdStr := fmt.Sprintf("%v %v %v %v", e.script.Path(), "stream", e.id(), e.state)
-
-	activity.Record(ctx, "Starting command: %v", cmdStr)
+	stderrR, err := cmd.StderrPipe()
+	if err != nil {
+		return nil, err
+	}
+	activity.Record(ctx, "Starting %v", cmd)
 	if err := cmd.Start(); err != nil {
-		activity.Record(ctx, "Closed command stdout: %v", stdoutR.Close())
-		activity.Record(ctx, "Closed command stderr: %v", stderrR.Close())
 		return nil, err
 	}
+	// "wait" will be used in Stream's error handlers. It will be wrapped
+	// in a "defer" call to ensure that cmd.Wait()'s called once we've read
+	// all of stdout/stderr. These are the preconditions specified in
+	// exec.Cmd#Wait's docs.
+	wait := func() {
+		if err := cmd.Wait(); err != nil {
+			activity.Record(ctx, "Failed waiting for %v to finish: %v", cmd, err)
+		}
+	}
 
+	// Wait for the header to appear on stdout. This lets us know that
+	// the plugin's ready for streaming.
 	header := "200"
 	headerRdrCh := make(chan error, 1)
 	go func() {
-		defer close(headerRdrCh)
-
 		headerBytes := []byte(header + "\n")
 		buf := make([]byte, len(headerBytes), cap(headerBytes))
-
 		n, err := stdoutR.Read(buf)
 		if err != nil {
 			headerRdrCh <- err
@@ -217,68 +184,33 @@ func (e *externalPluginEntry) Stream(ctx context.Context) (io.ReadCloser, error)
 			headerRdrCh <- fmt.Errorf("read an invalid header: %v", string(buf))
 			return
 		}
-
-		// Good to go.
 		headerRdrCh <- nil
 	}()
-
 	timeout := 5 * time.Second
 	timer := time.After(timeout)
-
-	// Waiting for the command to finish ensures that the
-	// stdout + stderr readers are closed
-	//
-	// TODO: Talk about how to handle timeout here, i.e.
-	// whether we should kill the command or not. Should we
-	// do this in a separate goroutine?
-	waitForCommandToFinish := func() {
-		activity.Record(ctx, "Waiting for command: %v", cmdStr)
-		err = cmd.Wait()
-		if err != nil {
-			activity.Record(ctx, "Failed waiting for command: %v", err)
-		}
-	}
-
 	select {
 	case err := <-headerRdrCh:
 		if err != nil {
-			defer waitForCommandToFinish()
-
-			// Try to get more context from stderr, if there is
-			// any
+			defer wait()
+			// Try to get more context from stderr
 			stderr, readErr := ioutil.ReadAll(stderrR)
 			if readErr == nil && len(stderr) != 0 {
 				err = fmt.Errorf(string(stderr))
 			}
-
 			return nil, fmt.Errorf("failed to read the header: %v", err)
 		}
-
-		// Keep reading from stderr so that the streaming isn't
-		// blocked when its buffer is full.
+		// err == nil, meaning we've received the header. Keep reading from
+		// stderr so that the streaming isn't blocked when its buffer is full.
 		go func() {
-			defer func() {
-				activity.Record(ctx, "Closed stream stderr: %v", stderrR.Close())
-			}()
-
-			buf := make([]byte, 4096)
-			for {
-				_, err := stderrR.Read(buf)
-				if err != nil {
-					break
-				}
-			}
+			_, _ = io.Copy(ioutil.Discard, stderrR)
 		}()
-
-		return &stdoutStreamer{stdoutR, cmd}, nil
+		return &stdoutStreamer{cmd, stdoutR}, nil
 	case <-timer:
-		defer waitForCommandToFinish()
-
-		// We timed out while waiting for the streaming header to appear,
-		// so log an appropriate error message using whatever was printed
-		// on stderr
+		defer wait()
+		// We timed out while waiting for the streaming header to appear.
+		// Return an appropriate error message using whatever was printed
+		// on stderr.
 		errMsgFmt := fmt.Sprintf("did not see the %v header after %v seconds:", header, timeout) + " %v"
-
 		stderr, err := ioutil.ReadAll(stderrR)
 		if err != nil {
 			return nil, fmt.Errorf(
@@ -286,62 +218,66 @@ func (e *externalPluginEntry) Stream(ctx context.Context) (io.ReadCloser, error)
 				fmt.Sprintf("cannot report reason: stderr i/o error: %v", err),
 			)
 		}
-
 		if len(stderr) == 0 {
 			return nil, fmt.Errorf(
 				errMsgFmt,
 				fmt.Sprintf("cannot report reason: nothing was printed to stderr"),
 			)
 		}
-
 		return nil, fmt.Errorf(errMsgFmt, string(stderr))
 	}
 }
 
-// Exec executes a command on the given entry
 func (e *externalPluginEntry) Exec(ctx context.Context, cmd string, args []string, opts ExecOptions) (ExecCommand, error) {
-	// TODO: Figure out how to pass-in opts when we have entries
-	// besides Stdin. Could do something like
-	//   <plugin_script> exec <path> <state> <opts> <cmd> <args...>
-	cmdObj := exec.CommandContext(
-		ctx,
-		e.script.Path(),
-		append(
-			[]string{"exec", e.id(), e.state, cmd},
-			args...,
-		)...,
-	)
-
-	// Set-up stdin
+	// Start the command.
+	cmdObj := e.script.NewInvocation(ctx, "exec", e, append([]string{cmd}, args...)...)
+	execCmd := NewExecCommand(ctx)
+	cmdObj.SetStdout(execCmd.Stdout())
+	cmdObj.SetStderr(execCmd.Stderr())
 	if opts.Stdin != nil {
-		cmdObj.Stdin = opts.Stdin
+		cmdObj.SetStdin(opts.Stdin)
 	}
-
-	execCmd := NewExecCommand(ctx)
-
-	// Set-up the output streams
-	cmdObj.Stdout = execCmd.Stdout()
-	cmdObj.Stderr = execCmd.Stderr()
-
-	// Start the command
-	activity.Record(ctx, "Starting command: %v %v", cmdObj.Path, strings.Join(cmdObj.Args, " "))
+	activity.Record(ctx, "Starting %v", cmdObj)
 	if err := cmdObj.Start(); err != nil {
 		return nil, err
 	}
-	// Note that CommandContext handles context-cancellation cleanup for us,
-	// so we don't have to use execCmd.SetStopFunc.
+	// internal.Command handles context-cancellation cleanup
+	// for us, so we don't have to use execCmd.SetStopFunc.
 
-	// Wait for the command to finish
+	// Asynchronously wait for the command to finish
 	go func() {
 		err := cmdObj.Wait()
 		execCmd.CloseStreamsWithError(nil)
-		exitCode := cmdObj.ProcessState.ExitCode()
+		exitCode := cmdObj.ProcessState().ExitCode()
 		if exitCode < 0 {
 			execCmd.SetExitCodeErr(err)
 		} else {
 			execCmd.SetExitCode(exitCode)
 		}
 	}()
-	
 	return execCmd, nil
 }
+
+type stdoutStreamer struct {
+	cmd    *internal.Command 
+	stdout io.ReadCloser
+}
+
+func (s *stdoutStreamer) Read(p []byte) (int, error) {
+	return s.stdout.Read(p)
+}
+
+func (s *stdoutStreamer) Close() error {
+	return s.cmd.Wait()
+}
+
+func newStdoutDecodeErr(ctx context.Context, decodedThing string, reason error, stdout []byte, example string) error {
+	activity.Record(
+		ctx,
+		"could not decode %v from stdout\nreceived:\n%v\nexpected something like:\n%v",
+		decodedThing,
+		strings.TrimSpace(string(stdout)),
+		example,
+	)
+	return fmt.Errorf("could not decode %v from stdout: %v", decodedThing, reason)
+}
\ No newline at end of file
diff --git a/plugin/externalPluginRoot.go b/plugin/externalPluginRoot.go
index 2563990..07c7bd8 100644
--- a/plugin/externalPluginRoot.go
+++ b/plugin/externalPluginRoot.go
@@ -4,9 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"strings"
-
-	"github.com/puppetlabs/wash/activity"
+	"time"
 )
 
 // externalPluginRoot represents an external plugin's root.
@@ -24,42 +22,38 @@ func newExternalPluginRoot(script string) *externalPluginRoot {
 
 // Init initializes the external plugin root
 func (r *externalPluginRoot) Init() error {
-	ctx := context.Background()
-	script := r.script
-
-	stdout, err := r.script.InvokeAndWait(ctx, "init")
+	// Give external plugins about five-seconds to finish their
+	// initialization
+	ctx, cancelFunc := context.WithTimeout(context.Background(), 5 * time.Second)
+	defer cancelFunc()
+	stdout, err := r.script.InvokeAndWait(ctx, "init", nil)
 	if err != nil {
-		return err
+		select {
+		case <-ctx.Done():
+			return fmt.Errorf("timed out while waiting for init to finish")
+		default:
+			return err
+		}
 	}
-
 	var decodedRoot decodedExternalPluginEntry
 	if err := json.Unmarshal(stdout, &decodedRoot); err != nil {
-		activity.Record(
-			ctx,
-			"could not decode the plugin root from stdout\nreceived:\n%v\nexpected something like:\n%v",
-			strings.TrimSpace(string(stdout)),
-			"{\"name\":\"<name_of_root_dir>\",\"supported_actions\":[\"list\"]}",
+		return newStdoutDecodeErr(
+			context.Background(),
+			"the plugin root",
+			err,
+			stdout,
+			"{\"name\":\"plugin_name\",\"methods\":[\"list\"]}",
 		)
-
-		return fmt.Errorf("could not decode the plugin root from stdout: %v", err)
 	}
-
 	entry, err := decodedRoot.toExternalPluginEntry()
 	if err != nil {
 		return err
 	}
-
+	if !ListAction().IsSupportedOn(entry) {
+		return fmt.Errorf("plugin roots must implement 'list'")
+	}
+	script := r.script
 	r.externalPluginEntry = entry
 	r.externalPluginEntry.script = script
-
 	return nil
-}
-
-// List lists the root's entries.
-//
-// We need this b/c *externalPluginEntry#List has a different receiver
-// (*externalPluginEntry) than *externalPluginRoot (i.e. b/c Go's
-// typechecker complains about it)
-func (r *externalPluginRoot) List(ctx context.Context) ([]Entry, error) {
-	return r.externalPluginEntry.List(ctx)
-}
+}
\ No newline at end of file
diff --git a/plugin/externalPluginScript.go b/plugin/externalPluginScript.go
index b1bc5c5..7befed3 100644
--- a/plugin/externalPluginScript.go
+++ b/plugin/externalPluginScript.go
@@ -4,16 +4,16 @@ import (
 	"bytes"
 	"context"
 	"fmt"
-	"os/exec"
-	"strings"
 
 	"github.com/puppetlabs/wash/activity"
+	"github.com/puppetlabs/wash/plugin/internal"
 )
 
 // externalPluginScript represents an external plugin's script
 type externalPluginScript interface {
 	Path() string
-	InvokeAndWait(ctx context.Context, args ...string) ([]byte, error)
+	InvokeAndWait(ctx context.Context, method string, entry *externalPluginEntry, args ...string) ([]byte, error)
+	NewInvocation(ctx context.Context, method string, entry *externalPluginEntry, args ...string) *internal.Command
 }
 
 type externalPluginScriptImpl struct {
@@ -24,37 +24,51 @@ func (s externalPluginScriptImpl) Path() string {
 	return s.path
 }
 
-// InvokeAndWait shells out to the plugin script, passing it the given
-// arguments. It waits for the script to exit, then returns its standard
-// output.
-func (s externalPluginScriptImpl) InvokeAndWait(ctx context.Context, args ...string) ([]byte, error) {
-	activity.Record(ctx, "Running command: %v %v", s.Path(), strings.Join(args, " "))
-
-	// Use CommandContext to ensure that the process is killed when the context
-	// is cancelled
-	cmd := exec.CommandContext(ctx, s.Path(), args...)
-
+// InvokeAndWait invokes method on entry by shelling out to the plugin script.
+// It waits for the script to exit, then returns its standard output.
+func (s externalPluginScriptImpl) InvokeAndWait(
+	ctx context.Context,
+	method string,
+	entry *externalPluginEntry,
+	args ...string,
+) ([]byte, error) {
+	cmd := s.NewInvocation(ctx, method, entry, args...)
 	var stdoutBuf, stderrBuf bytes.Buffer
-	cmd.Stdout = &stdoutBuf
-	cmd.Stderr = &stderrBuf
-
+	cmd.SetStdout(&stdoutBuf)
+	cmd.SetStderr(&stderrBuf)
+	activity.Record(ctx, "Invoking %v", cmd)
 	err := cmd.Run()
-
-	exitCode := cmd.ProcessState.ExitCode()
+	exitCode := cmd.ProcessState().ExitCode()
 	if exitCode < 0 {
 		return nil, err
 	}
-
-	stdout := stdoutBuf.Bytes()
 	stderr := stderrBuf.Bytes()
 	if exitCode == 0 {
 		if len(stderr) != 0 {
 			activity.Record(ctx, "stderr: %v", string(stderr))
 		}
 	} else {
-		// TODO: Wrap standard error into a structured Wash error
 		return nil, fmt.Errorf("script returned a non-zero exit code of %v. stderr output: %v", exitCode, string(stderr))
 	}
-
-	return stdout, nil
+	return stdoutBuf.Bytes(), nil
 }
+
+func (s externalPluginScriptImpl) NewInvocation(
+	ctx context.Context,
+	method string,
+	entry *externalPluginEntry,
+	args ...string,
+) *internal.Command {
+	if method == "init" {
+		return internal.NewCommand(ctx, s.Path(), "init")
+	}
+	if entry == nil {
+		msg := fmt.Sprintf("s.NewInvocation called with method '%v' and entry == nil", method)
+		panic(msg)
+	}
+	return internal.NewCommand(
+		ctx,
+		s.Path(), 
+		append([]string{method, entry.id(), entry.state}, args...)...,
+	)
+}
\ No newline at end of file
diff --git a/plugin/helpers.go b/plugin/helpers.go
index 03cdd59..7951d23 100644
--- a/plugin/helpers.go
+++ b/plugin/helpers.go
@@ -2,8 +2,6 @@ package plugin
 
 import (
 	"fmt"
-	"io"
-	"os/exec"
 	"strings"
 	"time"
 
@@ -99,26 +97,4 @@ func (e ErrCouldNotDetermineSizeAttr) Error() string {
 // Attributes returns the entry's attribtues
 func Attributes(e Entry) EntryAttributes {
 	return e.attributes()
-}
-
-// CreateCommand creates a cmd object encapsulating the given cmd and its args.
-// It returns the cmd object + its stdout and stderr pipes.
-//
-// TODO: Maybe useful to create our own Command object that wraps *exec.Cmd.
-// This way, we can extend it. For example, we could add a method that returns the
-// full command string, which would be useful for logging.
-func CreateCommand(cmd string, args ...string) (*exec.Cmd, io.ReadCloser, io.ReadCloser, error) {
-	cmdObj := exec.Command(cmd, args...)
-
-	stdout, err := cmdObj.StdoutPipe()
-	if err != nil {
-		return nil, nil, nil, err
-	}
-
-	stderr, err := cmdObj.StderrPipe()
-	if err != nil {
-		return nil, nil, nil, err
-	}
-
-	return cmdObj, stdout, stderr, nil
 }
\ No newline at end of file
diff --git a/plugin/internal/command.go b/plugin/internal/command.go
new file mode 100644
index 0000000..7e73482
--- /dev/null
+++ b/plugin/internal/command.go
@@ -0,0 +1,196 @@
+// Package internal contains utility classes and helpers that are used
+// by the plugin package. Its purpose is to modularize the plugin package's
+// code without exporting its implementation.
+package internal
+
+import (
+	"context"
+	"fmt"
+	"io"
+	"os"
+	"os/exec"
+	"strings"
+	"sync"
+	"syscall"
+	"time"
+
+	"github.com/puppetlabs/wash/activity"
+)
+
+// Command is a wrapper to exec.Cmd. It handles context-cancellation cleanup
+// and defines a String() method to make logging easier.
+//
+// NOTE: We make exec.Cmd a property because directly embedding it would
+// export other methods like exec.Cmd#Output and exec.Cmd#CombinedOutput.
+// These methods depend on Run(), Start(), and Wait(), which are methods
+// that this class overrides. Thus, if someone invoked them through our
+// Command class, then those methods will not work correctly because they
+// will reference exec.Cmd's implementations of Run(), Start(), and Wait().
+// Making exec.Cmd a property avoids this issue at the type-level. However,
+// it does mean we have to implement our own wrappers. These wrappers are
+// found at the bottom of the file.
+type Command struct {
+	c             *exec.Cmd
+	ctx           context.Context
+	pgid          int
+	waitResult    error
+	waitDoneCh    chan struct{}
+	waitOnce      sync.Once
+}
+
+// NewCommand creates a new command object that's tied to the passed-in
+// context. When cmd.Start() is invoked, the command will run in its
+// own process group. When the context is cancelled, a SIGTERM signal will
+// be sent to the command's process group. If after five seconds the command's
+// process has not been terminated, then a SIGKILL signal is sent to the
+// command's process group.
+func NewCommand(ctx context.Context, cmd string, args ...string) *Command {
+	if ctx == nil {
+		panic("plugin.newCommand called with a nil context")
+	}
+	cmdObj := &Command{
+		c:             exec.Command(cmd, args...),
+		ctx:           ctx,
+		pgid:          -1,
+		waitDoneCh:    make(chan struct{}),
+	}
+	cmdObj.c.SysProcAttr = &syscall.SysProcAttr{
+		Setpgid: true,
+	}
+	return cmdObj
+}
+
+// Start is a wrapper to exec.Cmd#Start
+func (cmd *Command) Start() error {
+	err := cmd.c.Start()
+	if err != nil {
+		return err
+	}
+	// Get the command's PGID for logging. If this fails, we'll try
+	// again in cmd.signal() when it is needed.
+	pgid, err := syscall.Getpgid(cmd.c.Process.Pid)
+	if err != nil {
+		activity.Record(cmd.ctx, "%v: could not get pgid: %v", cmd, err)
+	} else {
+		cmd.pgid = pgid
+	}
+	// Setup the context-cancellation cleanup
+	go func() {
+		select {
+		case <-cmd.waitDoneCh:
+			return
+		case <-cmd.ctx.Done():
+			// Pass-thru
+		}
+		activity.Record(cmd.ctx, "%v: Context cancelled. Sending SIGTERM signal", cmd)
+		if err := cmd.signal(syscall.SIGTERM); err != nil {
+			activity.Record(cmd.ctx, "%v: Failed to send SIGTERM signal: %v", cmd, err)
+		} else {
+			// SIGTERM was sent. Send SIGKILL after five seconds if the command failed
+			// to terminate.
+			time.AfterFunc(5 * time.Second, func() {
+				select {
+				case <-cmd.waitDoneCh:
+					return
+				default:
+					// Pass-thru
+				}
+				activity.Record(cmd.ctx, "%v: Did not terminate after five seconds. Sending SIGKILL signal", cmd)
+				if err := cmd.signal(syscall.SIGKILL); err != nil {
+					activity.Record(cmd.ctx, "%v: Failed to send SIGKILL signal: %v", cmd, err)
+				}
+			})
+		}
+		// Call Wait() to release cmd's resources. Leave error-logging up to the
+		// callers
+		_ = cmd.Wait()
+	}()
+	return nil
+}
+
+// String returns a stringified version of the command
+// that's useful for logging
+func (cmd *Command) String() string {
+	str := ""
+	if cmd.c.Process != nil {
+		str += fmt.Sprintf("(PID %v) ", cmd.c.Process.Pid)
+	}
+	if cmd.pgid >= 0 {
+		str += fmt.Sprintf("(PGID %v) ", cmd.pgid)
+	}
+	str += strings.Join(cmd.c.Args, " ")
+	return "'" + str + "'"
+}
+
+// Run is a wrapper to exec.Cmd#Run
+func (cmd *Command) Run() error {
+	// Copied from exec.Cmd#Run
+	if err := cmd.Start(); err != nil {
+		return err
+	}
+	return cmd.Wait()
+}
+
+// Wait is a thread-safe wrapper to exec.Cmd#Wait
+func (cmd *Command) Wait() error {
+	// According to https://github.com/golang/go/issues/28461,
+	// exec.Cmd#Wait is not thread-safe, so we need to implement
+	// our own version.
+	cmd.waitOnce.Do(func() {
+		cmd.waitResult = cmd.c.Wait()
+		close(cmd.waitDoneCh)
+	})
+	return cmd.waitResult
+}
+
+func (cmd *Command) signal(sig syscall.Signal) error {
+	if cmd.c.Process == nil {
+		panic("cmd.signal called with cmd.Process == nil")
+	}
+	if cmd.pgid < 0 {
+		// We failed to get the pgid in cmd.Start(), so try again
+		pgid, err := syscall.Getpgid(cmd.c.Process.Pid)
+		if err != nil {
+			return fmt.Errorf("could not get pgid: %v", err)
+		}
+		cmd.pgid = pgid
+	}
+	err := syscall.Kill(-cmd.pgid, sig)
+	if err != nil {
+		return err
+	}
+	return nil
+}
+
+// exec.Cmd wrappers go here
+
+// SetStdout wraps exec.Cmd#Stdout
+func (cmd *Command) SetStdout(stdout io.Writer) {
+	cmd.c.Stdout = stdout
+}
+
+// SetStderr wraps exec.Cmd#Stderr
+func (cmd *Command) SetStderr(stderr io.Writer) {
+	cmd.c.Stderr = stderr
+}
+
+// SetStdin wraps exec.Cmd#Stdin
+func (cmd *Command) SetStdin(stdin io.Reader) {
+	cmd.c.Stdin = stdin
+}
+
+// StdoutPipe wraps exec.Cmd#StdoutPipe
+func (cmd *Command) StdoutPipe() (io.ReadCloser, error) {
+	return cmd.c.StdoutPipe()
+}
+
+// StderrPipe wraps exec.Cmd#StderrPipe
+func (cmd *Command) StderrPipe() (io.ReadCloser, error) {
+	return cmd.c.StderrPipe()
+}
+
+// ProcessState returns the command's process state.
+// Call this after the command's finished running.
+func (cmd *Command) ProcessState() *os.ProcessState {
+	return cmd.c.ProcessState
+}
\ No newline at end of file
diff --git a/website/static/docs/external_plugins/examples/sshfs.sh b/website/static/docs/external_plugins/examples/sshfs.sh
index 6195c7c..36beb5f 100755
--- a/website/static/docs/external_plugins/examples/sshfs.sh
+++ b/website/static/docs/external_plugins/examples/sshfs.sh
@@ -48,14 +48,28 @@ function vm_exec {
   local vm="$1"
   local cmd="$2"
 
-  ssh root@"${vm}" "${cmd}"
+  local tty="$3"
+  if [[ -z "${tty}" || "${tty}" == "true" ]]; then
+    # Wash can prematurely kill our process while the remote SSH
+    # command is running. Setting up a tty ensures that the remote
+    # SSH command is killed when the calling process (our
+    # process) is killed. This avoids orphaned SSH processes.
+    #
+    # NOTE: We make TTY optional because it is one of the passed-in
+    # Exec options.
+    #
+    # NOTE: This type of plugin-specific cleanup is the plugin
+    # author's responsibility, not Wash's.
+    ssh -tt root@"${vm}" "${cmd}"
+  else
+    ssh root@"${vm}" "${cmd}"
+  fi
 }
 
 function to_json_array {
   local list="$1"
 
   echo -n "["
-
   local has_multiple_elem=""
   for elem in ${list}; do
     if [[ -n ${has_multiple_elem} ]]; then
@@ -63,36 +77,34 @@ function to_json_array {
     else
       has_multiple_elem="true"
     fi
-
     echo -n "${elem}"
   done
-
   echo -n "]"
 }
 
-# Prints the given entry's JSON object. This is used by the
-# `init` and `list` actions.
+# Prints the given entry's JSON object. This is used by `init`
+# and `list`.
 #
-# Remember, only the entry name and supported actions are
-# required.
+# Remember, only the entry name and its implemented methods
+# are required. The attributes should be provided if your
+# entry's a resource (e.g. like a file, container, VM, database,
+# etc.).
 function print_entry_json() {
   local name="$1"
-  local supported_actions="$2"
+  local methods="$2"
 
   local attributes_json="$3"
   if [[ -z "${attributes_json}" ]]; then
     # The attributes_json is optional. We chose to print something
     # here to make the code a little cleaner. Don't worry, Wash knows
-    # that an empty attributes object translates to the entry not having
-    # any filesystem attributes.
+    # that an empty attributes JSON means that the entry doesn't have
+    # any attributes.
     attributes_json="{}"
   fi
-
-  local supported_actions_json=`to_json_array "${supported_actions}"`
-
+  local methods_json=`to_json_array "${methods}"`
   echo "{\
 \"name\":\"${name}\",\
-\"supported_actions\":${supported_actions_json},\
+\"methods\":${methods_json},\
 \"attributes\":${attributes_json}\
 }"
 }
@@ -109,28 +121,28 @@ function print_file_json {
   local path="$7"
 
   name=`basename "${path}"`
-
-  intMode=$(echo $((16#${mode})))
+  intMode=$((16#${mode}))
   if [[ ${isDir} -eq 0 ]]; then
-    supported_actions='"list"'
-
+    methods='"list"'
     # Unfortunately, Wash doesn't handle symlinks well. Thus
     # for now, we'll assume that sym-linked directories are
     # regular directories.
-    intMode=$(echo $((${intMode} | 16384)))
+    intMode=$((${intMode} | 16384))
   else
-    supported_actions='"read" "stream"'
+    methods='"read" "stream"'
   fi
-
-  attributes_json=$(echo "{\
+  # We could include additional information about the
+  # file/directory in the "meta" attribute (e.g. like its
+  # inode number), but doing so complicates the code a
+  # bit.
+  attributes_json="{\
 \"atime\":${atime},\
 \"mtime\":${mtime},\
 \"ctime\":${ctime},\
 \"mode\":${intMode},\
 \"size\":${size}\
-}")
-
-  print_entry_json "${name}" "${supported_actions}" "${attributes_json}"
+}"
+  print_entry_json "${name}" "${methods}" "${attributes_json}"
 }
 
 # Prints the children of the specified directory. The code here
@@ -145,7 +157,10 @@ function print_children {
   # The code here is equivalent to ls'ing the directory, then invoking
   # `test-d` and stat on each entry to obtain the following information:
   #   <is_dir> <sizeAttr> <atime> <mtime> <ctime> <mode> <path>
-  stat_output=`vm_exec ${vm} "find ${dir} -mindepth 1 -maxdepth 1 | xargs -n1 -r -I {} bash -c '(test -d \\$@; echo -n \"\\$? \") && stat -c \"%s %X %Y %Z %f %n\" \\$@' _ {}"`
+  #
+  # TODO: Handle TTY strangeness in the returned output. For now, it is
+  # enough to set it to false.
+  stat_output=`vm_exec ${vm} "find ${dir} -mindepth 1 -maxdepth 1 -exec bash -c 'test -d \\$0; echo -n \"\\$? \"' {} \; -exec stat -c '%s %X %Y %Z %f %n' {} \;" false`
   if [[ -z "${stat_output}" ]]; then
     echo "[]"
     return 0
@@ -164,33 +179,27 @@ function print_children {
   # ]
   #
   # which satisfies Wash.
-
   echo "["
-
   # Print all children except for the last child.
   num_children=`echo "${stat_output}" | wc -l | awk '{print $1}'`
   if [[ num_children -gt 1 ]]; then
     export -f to_json_array
     export -f print_file_json
     export -f print_entry_json
-    export -f vm_exec
-    
+
     echo "${stat_output}"\
        | head -n$((num_children-1))\
        | xargs -n7 -P 10 -I {} bash -c 'print_file_json $@' _ {}\
        | sed s/$/,/
   fi
-
   # Now print the last child
   print_file_json `echo "${stat_output}" | tail -n1`
-
   echo "]"
 }
 
-action="$1"
-if [[ "${action}" == "init" ]]; then
-  # Our root's name is "sshfs." It only supports the "list"
-  # action.
+method="$1"
+if [[ "${method}" == "init" ]]; then
+  # Our root's name is "sshfs." It only implements "list"
   print_entry_json "sshfs" '"list"'
   exit 0
 fi
@@ -199,17 +208,38 @@ path="$2"
 
 path=`strip_root ${path}`
 if [[ "${path}" == "" ]]; then
-  # Our action's being invoked on the root. Since Wash only passes
-  # in supported actions, and since our root only supports the
-  # "list" action, we can assume that action == "list" here.
+  # Wash is invoking a method on our root. Since Wash only passes
+  # in implemented methods, and since our root only implements
+  # "list", we can assume that method == "list" here.
   #
   # Since we've structured our filesystem as /sshfs/<vm>/...,
-  # ls'ing our root consists of listing the VMs.
+  # "listing" our root consists of listing the VMs.
   function print_vm_json() {
       local name="$1"
+      local methods='"list" "exec" "metadata"'
+      # A VM can be modified, so some sort of mtime attribute
+      # makes sense. The other attributes (ctime, atime, mode,
+      # size) don't make sense, so don't set them. Notice how we
+      # also include the VM's partial metadata via the "meta" attribute.
+      # This would typically be the raw JSON object returned by
+      # an API's "list" endpoint (e.g. like a "/vms" REST endpoint).
+      # However, since we're not using any kind of API in our sshfs
+      # example, we'll just set "meta" to something random.
+      #
+      # NOTE: The mtime is in Unix seconds. It corresponds to
+      # May 17th, 2019 at 3:15 AM UTC. We recommend passing back
+      # Unix seconds for all your time-attribute values since they
+      # are the easiest for Wash to parse.
+      local mtime="1558062927"
+      local attributes="{\
+\"mtime\":${mtime},\
+\"meta\":{\
+\"LastModifiedTime\":${mtime},\
+\"Owner\":\"Alice\"\
+}\
+}"
 
-      # VMs support the "list", "exec" and "metadata" actions
-      print_entry_json "${name}" '"list" "exec" "metadata"'
+      print_entry_json "${name}" "${methods}" "${attributes}"
   }
 
   to_json_array "`print_vm_json ${SSHFS_VM_ONE}` `print_vm_json ${SSHFS_VM_TWO}`"
@@ -221,10 +251,9 @@ vm=`get_root ${path}`
 
 path=`strip_root ${path}`
 if [[ "${path}" == "" ]]; then
-  # Our action's being invoked on a VM. Since a VM only supports the
-  # "list", "exec" and "metadata" actions, we case our code on those
-  # actions
-  case "${action}" in
+  # The method's being invoked on a VM. Since a VM implements "list",
+  # "exec", and "metadata", we case our code on those methods.
+  case "${method}" in
   "list")
     # "list"'ing a VM is equivalent to listing its root
     print_children ${vm} "/"
@@ -238,35 +267,42 @@ if [[ "${path}" == "" ]]; then
     shift
     args="$@"
 
-    # exec'ing <cmd> <args> on a VM is equivalent to invoking them
+    # exec'ing <cmd> <args> on a VM is equivalent to exec'ing it
     # on the VM via. ssh (vm_exec)
     #
-    # TODO: Handle stdin
-    vm_exec "${vm}" "${cmd} ${args}"
+    # TODO: Handle stdin + other exec options.
+    vm_exec "${vm}" "${cmd} ${args}" "false"
     exit "$?"
   ;;
   "metadata")
-    # We could provide more metadata here, such as the VM's platform,
-    # architecture, and processor information. However, the example
-    # below is good enough for Wash.
+    # Wash is requesting the VM's full metadata.
+    #
+    # NOTE: Only implement "metadata" if there is additional information
+    # about your resource that is not provided by the "meta" attribute.
+    # In our example, the additional information is the VM's platform.
+    #
+    # NOTE: Since "metadata" is meant to return a complete description of
+    # the entry, it should be a superset of the "meta" attribute.
     echo "{\
-\"provider\":\"vmware\"\
+\"LastModifiedTime\":1558062927,\
+\"Owner\":\"Alice\",\
+\"Platform\":\"CentOS\"\
 }"
     exit 0
   ;;
   *)
-    # Notice how we print errors to stderr then exit with a non-zero
+    # We print errors to stderr then exit with a non-zero
     # exit code. This tells Wash that our invocation failed.
-    echo "missing a case statement for the ${action} action" >2
+    echo "missing a case statement for the '${method}'' method" >2
     exit 1
   ;;
   esac
 fi
 
 # Our path is an absolute path in the VM's filesystem.
-# Thus, we can just case on all the possible actions that can
-# be passed-in.
-case "${action}" in
+# Thus, we can just case on all the possible methods that
+# can be passed-in.
+case "${method}" in
 "list")
   print_children ${vm} "${path}/"
   exit 0
@@ -276,20 +312,20 @@ case "${action}" in
   exit 0
 ;;
 "stream")
-  # Notice how we print the header first before anything else.
-  # This way, Wash knows that we're about to stream some data.
+  # Notice how we print the "200" header first before anything
+  # else. In HTTP, "200" is the "OK" status code. Thus, printing
+  # this header tells Wash that everything's "OK", and that we
+  # are about to stream some stuff.
   echo "200"
 
-  # We could also `cat` here, which is useful for large files.
-  # Instead, we choose `tail -f` to show that external plugins
-  # can implement their own `tail -f` like behavior. Don't worry,
-  # Wash will send the SIGTERM signal to our process when it no
-  # longer needs our streamed data.
+  # Use `tail -f` to stream the file's content. Wash will send
+  # the SIGTERM signal to our process when it no longer needs
+  # our streamed data.
   vm_exec "${vm}" "tail -f ${path}"
   exit 0
 ;;
 *)
-  echo "missing a case statement for the ${action} action" >2
+  echo "missing a case statement for the '${method}' method" >2
   exit 1
 ;;
 esac
\ No newline at end of file


Test patch:
diff --git a/plugin/externalPluginEntry_test.go b/plugin/externalPluginEntry_test.go
index 5a1bd4d..a2b2df2 100644
--- a/plugin/externalPluginEntry_test.go
+++ b/plugin/externalPluginEntry_test.go
@@ -8,6 +8,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/puppetlabs/wash/plugin/internal"
+
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/suite"
 )
@@ -21,16 +23,37 @@ func (m *mockExternalPluginScript) Path() string {
 	return m.path
 }
 
-func (m *mockExternalPluginScript) InvokeAndWait(ctx context.Context, args ...string) ([]byte, error) {
-	retValues := m.Called(ctx, args)
+func (m *mockExternalPluginScript) InvokeAndWait(
+	ctx context.Context,
+	method string,
+	entry *externalPluginEntry,
+	args ...string,
+) ([]byte, error) {
+	retValues := m.Called(ctx, method, entry, args)
 	return retValues.Get(0).([]byte), retValues.Error(1)
 }
 
+func (m *mockExternalPluginScript) NewInvocation(
+	ctx context.Context,
+	method string,
+	entry *externalPluginEntry,
+	args ...string,
+) *internal.Command {
+	// A stub's still necessary to satisfy the externalPluginScript
+	// interface
+	panic("mockExternalPluginScript#NewInvocation called by tests")
+}
+
 // We make ctx an interface{} so that this method could
 // be used when the caller generates a context using e.g.
 // context.Background()
-func (m *mockExternalPluginScript) OnInvokeAndWait(ctx interface{}, args ...string) *mock.Call {
-	return m.On("InvokeAndWait", ctx, args)
+func (m *mockExternalPluginScript) OnInvokeAndWait(
+	ctx interface{},
+	method string,
+	entry *externalPluginEntry,
+	args ...string,
+) *mock.Call {
+	return m.On("InvokeAndWait", ctx, method, entry, args)
 }
 
 type ExternalPluginEntryTestSuite struct {
@@ -45,20 +68,20 @@ func (suite *ExternalPluginEntryTestSuite) TestDecodeExternalPluginEntryRequired
 	decodedEntry.Name = "decodedEntry"
 
 	_, err = decodedEntry.toExternalPluginEntry()
-	suite.Regexp(regexp.MustCompile("action"), err)
-	decodedEntry.SupportedActions = []string{"list"}
+	suite.Regexp(regexp.MustCompile("methods"), err)
+	decodedEntry.Methods = []string{"list"}
 
 	entry, err := decodedEntry.toExternalPluginEntry()
 	if suite.NoError(err) {
 		suite.Equal(decodedEntry.Name, entry.name())
-		suite.Equal(decodedEntry.SupportedActions, entry.supportedActions)
+		suite.Equal(decodedEntry.Methods, entry.methods)
 	}
 }
 
 func newMockDecodedEntry(name string) decodedExternalPluginEntry {
 	return decodedExternalPluginEntry{
 		Name:             name,
-		SupportedActions: []string{"list"},
+		Methods:          []string{"list"},
 	}
 }
 
@@ -129,11 +152,11 @@ func (suite *ExternalPluginEntryTestSuite) TestSetCacheTTLs() {
 // and TestMetadata that could be refactored. Not worth doing right
 // now since the refactor may make the tests harder to understand,
 // but could be worth considering later if we add similarly structured
-// actions.
+// methods.
 
 func (suite *ExternalPluginEntryTestSuite) TestList() {
 	mockScript := &mockExternalPluginScript{path: "plugin_script"}
-	entry := externalPluginEntry{
+	entry := &externalPluginEntry{
 		EntryBase: NewEntry("foo"),
 		script:    mockScript,
 	}
@@ -141,7 +164,7 @@ func (suite *ExternalPluginEntryTestSuite) TestList() {
 
 	ctx := context.Background()
 	mockInvokeAndWait := func(stdout []byte, err error) {
-		mockScript.OnInvokeAndWait(ctx, "list", entry.id(), entry.state).Return(stdout, err).Once()
+		mockScript.OnInvokeAndWait(ctx, "list", entry).Return(stdout, err).Once()
 	}
 
 	// Test that if InvokeAndWait errors, then List returns its error
@@ -158,7 +181,7 @@ func (suite *ExternalPluginEntryTestSuite) TestList() {
 
 	// Test that List properly decodes the entries from stdout
 	stdout := "[" +
-		"{\"name\":\"foo\",\"supported_actions\":[\"list\"]}" +
+		"{\"name\":\"foo\",\"methods\":[\"list\"]}" +
 		"]"
 	mockInvokeAndWait([]byte(stdout), nil)
 	entries, err := entry.List(ctx)
@@ -166,7 +189,7 @@ func (suite *ExternalPluginEntryTestSuite) TestList() {
 		expectedEntries := []Entry{
 			&externalPluginEntry{
 				EntryBase:        NewEntry("foo"),
-				supportedActions: []string{"list"},
+				methods:          []string{"list"},
 				script:           entry.script,
 			},
 		}
@@ -177,7 +200,7 @@ func (suite *ExternalPluginEntryTestSuite) TestList() {
 
 func (suite *ExternalPluginEntryTestSuite) TestOpen() {
 	mockScript := &mockExternalPluginScript{path: "plugin_script"}
-	entry := externalPluginEntry{
+	entry := &externalPluginEntry{
 		EntryBase: NewEntry("foo"),
 		script:    mockScript,
 	}
@@ -185,7 +208,7 @@ func (suite *ExternalPluginEntryTestSuite) TestOpen() {
 
 	ctx := context.Background()
 	mockInvokeAndWait := func(stdout []byte, err error) {
-		mockScript.OnInvokeAndWait(ctx, "read", entry.id(), entry.state).Return(stdout, err).Once()
+		mockScript.OnInvokeAndWait(ctx, "read", entry).Return(stdout, err).Once()
 	}
 
 	// Test that if InvokeAndWait errors, then Open returns its error
@@ -204,17 +227,37 @@ func (suite *ExternalPluginEntryTestSuite) TestOpen() {
 	}
 }
 
-func (suite *ExternalPluginEntryTestSuite) TestMetadata() {
+func (suite *ExternalPluginEntryTestSuite) TestMetadata_NotImplemented() {
 	mockScript := &mockExternalPluginScript{path: "plugin_script"}
 	entry := externalPluginEntry{
 		EntryBase: NewEntry("foo"),
 		script:    mockScript,
 	}
+	expectedMeta := JSONObject{"foo": "bar"}
+	entry.Attributes().SetMeta(expectedMeta)
+
+	// If metadata is not implemented, then e.Metadata should return
+	// EntryBase#Metadata, which returns the meta attribute.
+	meta, err := entry.Metadata(context.Background())
+	if suite.NoError(err) {
+		suite.Equal(expectedMeta, meta)
+	}
+	// Make sure that Wash did not shell out to the plugin script
+	mockScript.AssertNotCalled(suite.T(), "InvokeAndWait")
+}
+
+func (suite *ExternalPluginEntryTestSuite) TestMetadata_Implemented() {
+	mockScript := &mockExternalPluginScript{path: "plugin_script"}
+	entry := &externalPluginEntry{
+		EntryBase: NewEntry("foo"),
+		methods:   []string{"metadata"},
+		script:    mockScript,
+	}
 	entry.SetTestID("/foo")
 
 	ctx := context.Background()
 	mockInvokeAndWait := func(stdout []byte, err error) {
-		mockScript.OnInvokeAndWait(ctx, "metadata", entry.id(), entry.state).Return(stdout, err).Once()
+		mockScript.OnInvokeAndWait(ctx, "metadata", entry).Return(stdout, err).Once()
 	}
 
 	// Test that if InvokeAndWait errors, then Metadata returns its error
diff --git a/plugin/externalPluginRoot_test.go b/plugin/externalPluginRoot_test.go
index 68f8bf1..e9627a7 100644
--- a/plugin/externalPluginRoot_test.go
+++ b/plugin/externalPluginRoot_test.go
@@ -20,7 +20,11 @@ func (suite *ExternalPluginRootTestSuite) TestInit() {
 	}}
 
 	mockInvokeAndWait := func(stdout []byte, err error) {
-		mockScript.OnInvokeAndWait(mock.AnythingOfType("context.Context"), "init").Return(stdout, err).Once()
+		mockScript.OnInvokeAndWait(
+			mock.AnythingOfType("context.Context"),
+			"init",
+			nil,
+		).Return(stdout, err).Once()
 	}
 
 	// Test that if InvokeAndWait errors, then Init returns its error
@@ -35,15 +39,21 @@ func (suite *ExternalPluginRootTestSuite) TestInit() {
 	err = root.Init()
 	suite.Regexp(regexp.MustCompile("stdout"), err)
 
+	// Test that Init returns an error if the root does not implement
+	// "list"
+	mockInvokeAndWait([]byte("{\"name\":\"root\",\"methods\":[]}"), nil)
+	err = root.Init()
+	suite.Regexp("implement.*list", err)
+
 	// Test that Init properly decodes the root from stdout
-	stdout := "{\"name\":\"foo\",\"supported_actions\":[\"list\"]}"
+	stdout := "{\"name\":\"foo\",\"wmethods\":[\"list\"]}"
 	mockInvokeAndWait([]byte(stdout), nil)
 	err = root.Init()
 	if suite.NoError(err) {
 		expectedRoot := &externalPluginRoot{
 			externalPluginEntry: &externalPluginEntry{
 				EntryBase:        NewEntry("foo"),
-				supportedActions: []string{"list"},
+				methods:          []string{"list"},
 				script:           root.script,
 			},
 		}
diff --git a/plugin/testdata/external.sh b/plugin/testdata/external.sh
index 8bb27c9..effe8fe 100755
--- a/plugin/testdata/external.sh
+++ b/plugin/testdata/external.sh
@@ -1,7 +1,7 @@
 #!/bin/sh
 echo '{
   "name": "test",
-  "supported_actions": [
+  "methods": [
     "list"
   ]
 }'
\ No newline at end of file