Repository: puppetlabs/wash
Pull request #211 (2019-04-15 17:59:51)

Extend Wash API to support local files and directories
This commit adds the apifs package, which is used by the API to convert
local files/directories into Wash entries. This means that custom commands
built on the API are no longer required to special-case on them.

Signed-off-by: Enis Inan <enis.inan@puppet.com>

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/api/error.go b/api/error.go
index d27e7fd..2ff9a49 100644
--- a/api/error.go
+++ b/api/error.go
@@ -161,17 +161,3 @@ func relativePathResponse(path string) *errorResponse {
 
 	return &errorResponse{http.StatusBadRequest, body}
 }
-
-func nonWashEntryResponse(path string) *errorResponse {
-	fields := apitypes.ErrorFields{
-		"path": path,
-	}
-
-	body := newErrorObj(
-		apitypes.NonWashEntry,
-		fmt.Sprintf("%v is not a Wash entry.", path),
-		fields,
-	)
-
-	return &errorResponse{http.StatusBadRequest, body}
-}
diff --git a/api/fs/core.go b/api/fs/core.go
new file mode 100644
index 0000000..a162c04
--- /dev/null
+++ b/api/fs/core.go
@@ -0,0 +1,48 @@
+// Package apifs is used by the Wash API to convert local files/directories
+// into Wash entries
+package apifs
+
+import (
+	"os"
+
+	"github.com/puppetlabs/wash/plugin"
+)
+
+// fsnode => filesystem node
+type fsnode struct {
+	plugin.EntryBase
+	path string
+}
+
+func newFSNode(finfo os.FileInfo, path string) *fsnode {
+	// TODO: finfo.Sys() contains more detailed file attributes,
+	// but it's platform-specific. We should eventually use it for
+	// a more complete implementation of apifs.
+	attr := plugin.EntryAttributes{}
+	attr.
+		SetMtime(finfo.ModTime()).
+		SetMode(finfo.Mode()).
+		SetSize(uint64(finfo.Size())).
+		SetMeta(plugin.ToMeta(finfo))
+
+	n := &fsnode{
+		EntryBase: plugin.NewEntry(finfo.Name()),
+		path:      path,
+	}
+	n.DisableDefaultCaching()
+	n.SetAttributes(attr)
+	return n
+}
+
+// NewEntry constructs a new Wash entry from the given FS
+// path
+func NewEntry(path string) (plugin.Entry, error) {
+	finfo, err := os.Stat(path)
+	if err != nil {
+		return nil, err
+	}
+	if finfo.IsDir() {
+		return newDir(finfo, path), nil
+	}
+	return newFile(finfo, path), nil
+}
diff --git a/api/fs/dir.go b/api/fs/dir.go
new file mode 100644
index 0000000..98713af
--- /dev/null
+++ b/api/fs/dir.go
@@ -0,0 +1,36 @@
+package apifs
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+
+	"github.com/puppetlabs/wash/plugin"
+)
+
+type dir struct {
+	*fsnode
+}
+
+func newDir(finfo os.FileInfo, path string) *dir {
+	return &dir{
+		newFSNode(finfo, path),
+	}
+}
+
+func (d *dir) List(ctx context.Context) ([]plugin.Entry, error) {
+	matches, err := filepath.Glob(filepath.Join(d.path, "*"))
+	if err != nil {
+		return nil, err
+	}
+
+	entries := make([]plugin.Entry, len(matches))
+	for i, match := range matches {
+		entry, err := NewEntry(match)
+		if err != nil {
+			return nil, err
+		}
+		entries[i] = entry
+	}
+	return entries, nil
+}
diff --git a/api/fs/file.go b/api/fs/file.go
new file mode 100644
index 0000000..e0c777e
--- /dev/null
+++ b/api/fs/file.go
@@ -0,0 +1,28 @@
+package apifs
+
+import (
+	"bytes"
+	"context"
+	"io/ioutil"
+	"os"
+
+	"github.com/puppetlabs/wash/plugin"
+)
+
+type file struct {
+	*fsnode
+}
+
+func newFile(finfo os.FileInfo, path string) *file {
+	return &file{
+		newFSNode(finfo, path),
+	}
+}
+
+func (f *file) Open(ctx context.Context) (plugin.SizedReader, error) {
+	content, err := ioutil.ReadFile(f.path)
+	if err != nil {
+		return nil, err
+	}
+	return bytes.NewReader(content), nil
+}
diff --git a/api/helpers.go b/api/helpers.go
index cc43855..8de8521 100644
--- a/api/helpers.go
+++ b/api/helpers.go
@@ -3,9 +3,11 @@ package api
 import (
 	"context"
 	"fmt"
+	"os"
 	"path/filepath"
 	"strings"
 
+	apifs "github.com/puppetlabs/wash/api/fs"
 	apitypes "github.com/puppetlabs/wash/api/types"
 	"github.com/puppetlabs/wash/plugin"
 )
@@ -70,8 +72,22 @@ func getEntryFromPath(ctx context.Context, path string) (plugin.Entry, *errorRes
 	mountpoint := ctx.Value(mountpointKey).(string)
 	trimmedPath := strings.TrimPrefix(path, mountpoint)
 	if trimmedPath == path {
-		// TODO: Handle these later by wrapping them into entries
-		return nil, nonWashEntryResponse(path)
+		// Local file/directory, so convert it to a Wash entry
+		//
+		// TODO: The code here means that the Wash server cannot be
+		// mounted on a remote machine. This is not an immediate issue,
+		// but it does mean that we'll need to re-evaluate this code once
+		// we get to the point where supporting remote Wash servers is
+		// desirable.
+		e, err := apifs.NewEntry(path)
+		if err != nil {
+			if os.IsNotExist(err) {
+				return nil, entryNotFoundResponse(path, err.Error())
+			}
+			err = fmt.Errorf("could not stat the regular file/dir pointed to by %v: %v", path, err)
+			return nil, unknownErrorResponse(err)
+		}
+		return e, nil
 	}
 	// Don't interpret trailing slash as a new segment, and ignore optional leading slash
 	path = strings.Trim(trimmedPath, "/")
diff --git a/api/types/error.go b/api/types/error.go
index 75ee0ea..affb415 100644
--- a/api/types/error.go
+++ b/api/types/error.go
@@ -40,5 +40,4 @@ const (
 	ErroredAction      = "puppetlabs.wash/errored-action"
 	DuplicateCName     = "puppetlabs.wash/duplicate-cname"
 	RelativePath       = "puppetlabs.wash/relative-path"
-	NonWashEntry       = "puppetlabs.wash/non-wash-entry"
 )
diff --git a/cmd/list.go b/cmd/list.go
index 8c3e9c7..57e7b57 100644
--- a/cmd/list.go
+++ b/cmd/list.go
@@ -2,12 +2,9 @@ package cmd
 
 import (
 	"fmt"
-	"os"
-	"path/filepath"
 	"strings"
 	"time"
 
-	"github.com/Benchkram/errz"
 	"github.com/spf13/cobra"
 
 	"github.com/puppetlabs/wash/api/client"
@@ -42,7 +39,7 @@ func format(t time.Time) string {
 	return t.Format(time.RFC822)
 }
 
-func formatListEntries(apiPath string, ls []apitypes.Entry) string {
+func formatListEntries(ls []apitypes.Entry) string {
 	table := make([][]string, len(ls))
 	for i, entry := range ls {
 		var mtimeStr string
@@ -69,58 +66,6 @@ func formatListEntries(apiPath string, ls []apitypes.Entry) string {
 	return cmdutil.FormatTable(headers(), table)
 }
 
-func listResource(apiPath string) error {
-	conn := client.ForUNIXSocket(config.Socket)
-	e, err := conn.Info(apiPath)
-	if err != nil {
-		return err
-	}
-	entries := []apitypes.Entry{e}
-	if e.Supports(plugin.ListAction) {
-		children, err := conn.List(apiPath)
-		if err != nil {
-			return err
-		}
-		entries = append(entries, children...)
-	}
-
-	fmt.Print(formatListEntries(apiPath, entries))
-	return nil
-}
-
-func listPath(path string) error {
-	finfo, err := os.Stat(path)
-	if err != nil {
-		return err
-	}
-
-	var table [][]string
-	if finfo.IsDir() {
-		matches, err := filepath.Glob(filepath.Join(path, "*"))
-		errz.Fatal(err)
-		table = make([][]string, len(matches)+1)
-		table[0] = []string{".", format(finfo.ModTime()), "list"}
-		for i, match := range matches {
-			finfo, err := os.Stat(match)
-			if err != nil {
-				return err
-			}
-			actions := "read"
-			if finfo.IsDir() {
-				actions = "list"
-			}
-			table[i+1] = []string{finfo.Name(), format(finfo.ModTime()), actions}
-		}
-	} else {
-		table = [][]string{[]string{finfo.Name(), format(finfo.ModTime()), "read"}}
-	}
-	// Most operating systems don't track when a thing was created, just the last time it was
-	// modified. Some filesystems track the inode birth time, but Go doesn't expose that. List
-	// modification time for now, and note that this differs from what `ls -l` shows on macOS.
-	fmt.Print(cmdutil.FormatTable(headers(), table))
-	return nil
-}
-
 func listMain(cmd *cobra.Command, args []string) exitCode {
 	// If no path is declared, try to list the current directory/resource
 	path := "."
@@ -128,27 +73,22 @@ func listMain(cmd *cobra.Command, args []string) exitCode {
 		path = args[0]
 	}
 
-	err := listResource(path)
-	if err == nil {
-		return exitCode{0}
-	}
-	// err != nil
-
-	apiErr, ok := err.(*apitypes.ErrorObj)
-	if !ok {
-		cmdutil.ErrPrintf("%v\n", err)
-		return exitCode{1}
-	}
-	if apiErr.Kind != apitypes.NonWashEntry {
-		cmdutil.ErrPrintf("%v\n", err)
-		return exitCode{1}
-	}
-
-	err = listPath(path)
+	conn := client.ForUNIXSocket(config.Socket)
+	e, err := conn.Info(path)
 	if err != nil {
 		cmdutil.ErrPrintf("%v\n", err)
 		return exitCode{1}
 	}
+	entries := []apitypes.Entry{e}
+	if e.Supports(plugin.ListAction) {
+		children, err := conn.List(path)
+		if err != nil {
+			cmdutil.ErrPrintf("%v\n", err)
+			return exitCode{1}
+		}
+		entries = append(entries, children...)
+	}
 
+	fmt.Print(formatListEntries(entries))
 	return exitCode{0}
 }
diff --git a/cmd/tail.go b/cmd/tail.go
index aa06ae0..ff367e9 100644
--- a/cmd/tail.go
+++ b/cmd/tail.go
@@ -56,10 +56,6 @@ func tailStream(conn client.DomainSocketClient, agg chan line, path string) io.C
 	stream, err := conn.Stream(path)
 	if err != nil {
 		if errObj, ok := err.(*apitypes.ErrorObj); ok {
-			if errObj.Kind == apitypes.NonWashEntry {
-				// Not a resource
-				return nil
-			}
 			if errObj.Kind == apitypes.UnsupportedAction {
 				// The resource exists but does not support the streaming action
 				return nil
diff --git a/plugin/cache.go b/plugin/cache.go
index 19b19ba..369f48e 100644
--- a/plugin/cache.go
+++ b/plugin/cache.go
@@ -149,10 +149,6 @@ func (c DuplicateCNameErr) Error() string {
 // querying a specific entry.
 func CachedList(ctx context.Context, g Group) (map[string]Entry, error) {
 	cachedEntries, err := cachedDefaultOp(ctx, ListOp, g, func() (interface{}, error) {
-		if g.id() == "" {
-			panic("cannot List an entry that you just created")
-		}
-
 		// 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
 		// to calculate its attributes. Note that the child's ID is set in cachedOp.
@@ -240,6 +236,10 @@ func cachedOp(ctx context.Context, opName string, entry Entry, ttl time.Duration
 		}
 	}
 
+	if ttl < 0 {
+		return op()
+	}
+
 	if entry.id() == "" {
 		// Try to set the ID based on parent ID
 		if obj := ctx.Value(parentID); obj != nil {
@@ -250,9 +250,5 @@ func cachedOp(ctx context.Context, opName string, entry Entry, ttl time.Duration
 		}
 	}
 
-	if ttl < 0 {
-		return op()
-	}
-
 	return cache.GetOrUpdate(opName, entry.id(), ttl, false, op)
 }
diff --git a/plugin/helpers.go b/plugin/helpers.go
index b458f20..3d77796 100644
--- a/plugin/helpers.go
+++ b/plugin/helpers.go
@@ -63,6 +63,10 @@ func CName(e Entry) string {
 // NOTE: <plugin_name> is really <plugin_cname>. However since <plugin_name>
 // can never contain a '/', <plugin_cname> reduces to <plugin_name>.
 func ID(e Entry) string {
+	if e.id() == "" {
+		msg := fmt.Sprintf("plugin.ID: entry %v (cname %v) has no ID", e.name(), CName(e))
+		panic(msg)
+	}
 	return e.id()
 }
 


Test patch:
diff --git a/api/helpers_test.go b/api/helpers_test.go
index 8d12a14..dc68e3e 100644
--- a/api/helpers_test.go
+++ b/api/helpers_test.go
@@ -118,8 +118,7 @@ func (suite *HelpersTestSuite) TestGetEntryFromPath() {
 	_, err := getEntryFromPath(ctx, "relative")
 	suite.Error(relativePathResponse("relative"), err)
 
-	_, err = getEntryFromPath(ctx, "/file")
-	suite.Error(nonWashEntryResponse("/file"), err)
+	// TODO: Add tests for non-Wash entries (i.e. for apifs)
 
 	entry, err := getEntryFromPath(ctx, mountpoint)
 	if suite.Nil(err) {
diff --git a/plugin/cache_test.go b/plugin/cache_test.go
index ff00157..43b2ac1 100644
--- a/plugin/cache_test.go
+++ b/plugin/cache_test.go
@@ -219,13 +219,8 @@ func (suite *CacheTestSuite) testCachedDefaultOp(
 
 	entry := newCacheTestsMockEntry("mock")
 
-	// Test that cachedDefaultOp panics if entry.id() == "" if not passed
-	// a suitable context.
-	suite.Panics(panicFunc, "entry.id() returned an empty ID")
-
 	// Test that cachedDefaultOp does _not_ call cache#GetOrUpdate for an
 	// entry that's turned off caching
-	entry.SetTestID("id")
 	entry.On(opName, mock.Anything).Return(opValue, nil)
 	entry.DisableCachingFor(op)
 	v, err := cachedDefaultOp(ctx, entry)
@@ -234,6 +229,11 @@ func (suite *CacheTestSuite) testCachedDefaultOp(
 	}
 	suite.cache.AssertNotCalled(suite.T(), "GetOrUpdate")
 
+	// Test that cachedDefaultOp panics if entry.id() == "" if not passed
+	// a suitable context.
+	suite.Panics(panicFunc, "entry.id() returned an empty ID")
+	entry.SetTestID("id")
+
 	// Test that cachedDefaultOp does call cache#GetOrUpdate for an
 	// entry that's enabled caching, and that it passes-in the
 	// right arguments.
diff --git a/plugin/helpers_test.go b/plugin/helpers_test.go
index ba30da9..3686823 100644
--- a/plugin/helpers_test.go
+++ b/plugin/helpers_test.go
@@ -38,9 +38,13 @@ func (suite *HelpersTestSuite) TestCName() {
 }
 
 func (suite *HelpersTestSuite) TestID() {
-	e := NewEntry("foo")
-	e.setID("/foo/bar")
+	e := NewEntry("foo/bar")
 
+	suite.Panics(
+		func() { ID(&e) },
+		"plugin.ID: entry foo (cname foo#bar) has no ID",
+	)
+	e.setID("/foo/bar")
 	suite.Equal(ID(&e), "/foo/bar")
 }