[plugins] Implements github plugins (#1281)

mikeland73 · web-flow · commit dd412550c565 · 2023-07-19T11:54:36.000-07:00
## Summary This implements github plugins. They use the following syntax in `devbox.json`: ```json "include": [ "github:jetpack-io/devbox-plugin-example/6ea0ef9e12ab58dbbb145ca8f3a465611cb603a9" ] ``` revision is optional and defaults to master/main. Repos must have `devbox.json` at top level. TODO: - [x] Better error message for missing devbox.json and other files. - [ ] Add to lockfile? The are currently locked using optional revision - [ ] Updates: They are currently don't update unless change is made that busts local.lock cache. ## How was it tested? <img width="781" alt="image" src="https://github.com/jetpack-io/devbox/assets/544948/abb258dd-ef78-45c6-8652-e0111635208a">
diff --git a/examples/plugins/github-with-revision/devbox.d/jetpack-io-devbox-plugin-example/some-file.txt b/examples/plugins/github-with-revision/devbox.d/jetpack-io-devbox-plugin-example/some-file.txt
@@ -0,0 +1 @@
+some data
diff --git a/examples/plugins/github-with-revision/devbox.json b/examples/plugins/github-with-revision/devbox.json
@@ -0,0 +1,16 @@
+{
+  "packages": [],
+  "shell": {
+    "init_hook": [
+      "echo 'Welcome to devbox!' > /dev/null"
+    ],
+    "scripts": {
+      "run_test": [
+        "./test.sh"
+      ]
+    }
+  },
+  "include": [
+    "github:jetpack-io/devbox-plugin-example/6ea0ef9e12ab58dbbb145ca8f3a465611cb603a9"
+  ]
+}
diff --git a/examples/plugins/github-with-revision/devbox.lock b/examples/plugins/github-with-revision/devbox.lock
@@ -0,0 +1,4 @@
+{
+  "lockfile_version": "1",
+  "packages": {}
+}
diff --git a/examples/plugins/github-with-revision/test.sh b/examples/plugins/github-with-revision/test.sh
@@ -0,0 +1,9 @@
+#!/bin/bash
+
+expected="I AM SET"
+if [ "$MY_ENV_VAR" == "$expected" ]; then
+  echo "Success! MY_ENV_VAR is set to '$MY_ENV_VAR'"
+else
+  echo "MY_ENV_VAR environment variable is not set to '$expected'"
+  exit 1
+fi
diff --git a/examples/plugins/github/devbox.d/jetpack-io-devbox-plugin-example/some-file.txt b/examples/plugins/github/devbox.d/jetpack-io-devbox-plugin-example/some-file.txt
@@ -0,0 +1 @@
+some data
diff --git a/examples/plugins/github/devbox.json b/examples/plugins/github/devbox.json
@@ -0,0 +1,16 @@
+{
+  "packages": [],
+  "shell": {
+    "init_hook": [
+      "echo 'Welcome to devbox!' > /dev/null"
+    ],
+    "scripts": {
+      "run_test": [
+        "./test.sh"
+      ]
+    }
+  },
+  "include": [
+    "github:jetpack-io/devbox-plugin-example"
+  ]
+}
diff --git a/examples/plugins/github/devbox.lock b/examples/plugins/github/devbox.lock
@@ -0,0 +1,4 @@
+{
+  "lockfile_version": "1",
+  "packages": {}
+}
diff --git a/examples/plugins/github/test.sh b/examples/plugins/github/test.sh
@@ -0,0 +1,9 @@
+#!/bin/bash
+
+expected="I AM SET (new value)"
+if [ "$MY_ENV_VAR" == "$expected" ]; then
+  echo "Success! MY_ENV_VAR is set to '$MY_ENV_VAR'"
+else
+  echo "MY_ENV_VAR environment variable is not set to '$expected'"
+  exit 1
+fi
diff --git a/internal/boxcli/midcobra/debug.go b/internal/boxcli/midcobra/debug.go
@@ -56,12 +56,12 @@ func (d *DebugMiddleware) postRun(cmd *cobra.Command, args []string, runErr erro
 	if runErr == nil {
 		return
 	}
-	if usererr.HasUserMessage(runErr) {
-		if usererr.IsWarning(runErr) {
+	if userErr, hasUserErr := usererr.Extract(runErr); hasUserErr {
+		if usererr.IsWarning(userErr) {
 			ux.Fwarning(cmd.ErrOrStderr(), runErr.Error())
 			return
 		}
-		color.New(color.FgRed).Fprintf(cmd.ErrOrStderr(), "\nError: %s\n\n", runErr.Error())
+		color.New(color.FgRed).Fprintf(cmd.ErrOrStderr(), "\nError: %s\n\n", userErr.Error())
 	} else {
 		color.New(color.FgRed).Fprintf(cmd.ErrOrStderr(), "Error: %v\n\n", runErr)
 	}
diff --git a/internal/boxcli/usererr/usererr.go b/internal/boxcli/usererr/usererr.go
@@ -53,7 +53,7 @@ func NewWarning(msg string, args ...any) error {
 func WithUserMessage(source error, msg string, args ...any) error {
 	// We don't want to wrap the error if it already has a user message. Doing
 	// so would obscure the original error message which is likely more useful.
-	if source == nil || HasUserMessage(source) {
+	if source == nil || hasUserMessage(source) {
 		return source
 	}
 	return &combined{
@@ -63,7 +63,7 @@ func WithUserMessage(source error, msg string, args ...any) error {
 }
 
 func WithLoggedUserMessage(source error, msg string, args ...any) error {
-	if source == nil || HasUserMessage(source) {
+	if source == nil || hasUserMessage(source) {
 		return source
 	}
 	return &combined{
@@ -73,9 +73,13 @@ func WithLoggedUserMessage(source error, msg string, args ...any) error {
 	}
 }
 
-func HasUserMessage(err error) bool {
+// Extract unwraps and returns the user error if it exists.
+func Extract(err error) (error, bool) { // nolint: revive
 	c := &combined{}
-	return errors.As(err, &c) // note double pointer
+	if errors.As(err, &c) {
+		return c, true
+	}
+	return nil, false
 }
 
 // ShouldLogError returns true if the it's a logged user error or is a non-user error
@@ -126,3 +130,8 @@ func (c *combined) Format(s fmt.State, verb rune) {
 		Format(s fmt.State, verb rune)
 	}).Format(s, verb)
 }
+
+func hasUserMessage(err error) bool {
+	_, hasUserMessage := Extract(err)
+	return hasUserMessage
+}
diff --git a/internal/devpkg/package.go b/internal/devpkg/package.go
@@ -22,12 +22,14 @@ import (
 	"go.jetpack.io/devbox/internal/lock"
 	"go.jetpack.io/devbox/internal/nix"
 	"go.jetpack.io/devbox/internal/ux"
+	"go.jetpack.io/devbox/plugins"
 )
 
 // Package represents a "package" added to the devbox.json config.
 // A unique feature of flakes is that they have well-defined "inputs" and "outputs".
 // This Package will be aggregated into a specific "flake input" (see shellgen.flakeInput).
 type Package struct {
+	plugins.BuiltIn
 	url.URL
 	lockfile lock.Locker
 
@@ -78,7 +80,7 @@ func PackageFromString(raw string, locker lock.Locker) *Package {
 		pkgURL, _ = url.Parse(normalizedURL)
 	}
 
-	return &Package{*pkgURL, locker, raw, ""}
+	return &Package{URL: *pkgURL, lockfile: locker, Raw: raw}
 }
 
 // isLocal specifies whether this package is a local flake.
diff --git a/internal/plugin/files.go b/internal/plugin/files.go
@@ -9,29 +9,38 @@ import (
 	"strings"
 
 	"github.com/pkg/errors"
+	"go.jetpack.io/devbox/internal/devpkg"
 	"go.jetpack.io/devbox/plugins"
 )
 
 func getConfigIfAny(pkg Includable, projectDir string) (*config, error) {
-	configFiles, err := plugins.BuiltIn.ReadDir(".")
-	if err != nil {
-		return nil, errors.WithStack(err)
-	}
-
-	if local, ok := pkg.(*localPlugin); ok {
-		content, err := os.ReadFile(local.path)
+	switch pkg := pkg.(type) {
+	case *devpkg.Package:
+		return getBuiltinPluginConfig(pkg, projectDir)
+	case *githubPlugin:
+		return getConfigFromGithub(pkg, projectDir)
+	case *localPlugin:
+		content, err := os.ReadFile(pkg.path)
 		if err != nil && !os.IsNotExist(err) {
 			return nil, errors.WithStack(err)
 		}
 		return buildConfig(pkg, projectDir, string(content))
 	}
+	return nil, errors.Errorf("unknown plugin type %T", pkg)
+}
 
-	for _, file := range configFiles {
-		if file.IsDir() || strings.HasSuffix(file.Name(), ".go") {
-			continue
-		}
-		// Try to find perfect match first
-		content, err := plugins.BuiltIn.ReadFile(file.Name())
+func getBuiltinPluginConfig(pkg Includable, projectDir string) (*config, error) {
+	builtins, err := plugins.Builtins()
+	if err != nil {
+		return nil, errors.WithStack(err)
+	}
+
+	for _, file := range builtins {
+		// We deserialize first so we can check the Match field. If it's there
+		// we use it, otherwise we use the name.
+		// TODO(landau): this is weird, hard to understand code. Fetching the file
+		// content of configs should probably not use FileContent()
+		content, err := pkg.FileContent(file.Name())
 		if err != nil {
 			return nil, errors.WithStack(err)
 		}
@@ -52,9 +61,10 @@ func getConfigIfAny(pkg Includable, projectDir string) (*config, error) {
 	return nil, nil
 }
 
-func getFileContent(pkg Includable, contentPath string) ([]byte, error) {
-	if local, ok := pkg.(*localPlugin); ok {
-		return os.ReadFile(local.contentPath(contentPath))
+func getConfigFromGithub(pkg *githubPlugin, projectDir string) (*config, error) {
+	content, err := pkg.FileContent("devbox.json")
+	if err != nil {
+		return nil, errors.WithStack(err)
 	}
-	return plugins.BuiltIn.ReadFile(contentPath)
+	return buildConfig(pkg, projectDir, string(content))
 }
diff --git a/internal/plugin/github.go b/internal/plugin/github.go
@@ -0,0 +1,84 @@
+package plugin
+
+import (
+	"io"
+	"net/http"
+	"net/url"
+	"strings"
+
+	"go.jetpack.io/devbox/internal/boxcli/usererr"
+	"go.jetpack.io/devbox/internal/cuecfg"
+)
+
+type githubPlugin struct {
+	raw      string
+	org      string
+	repo     string
+	revision string
+}
+
+// newGithubPlugin returns a plugin that is hosted on github.
+// url is of the form org/repo
+// The repo must have a devbox.json file in the root of the repo.
+func newGithubPlugin(url string) (*githubPlugin, error) {
+	parts := strings.Split(url, "/")
+
+	if len(parts) < 2 || len(parts) > 3 {
+		return nil, usererr.New(
+			"invalid github plugin url %q. Must be of the form org/repo/[revision]",
+			url,
+		)
+	}
+
+	plugin := &githubPlugin{
+		raw:      url,
+		org:      parts[0],
+		repo:     parts[1],
+		revision: "master",
+	}
+
+	if len(parts) == 3 {
+		plugin.revision = parts[2]
+	}
+
+	return plugin, nil
+}
+
+func (p *githubPlugin) CanonicalName() string {
+	return p.org + "-" + p.repo
+}
+
+func (p *githubPlugin) Hash() string {
+	h, _ := cuecfg.Hash(p.CanonicalName())
+	return h
+}
+
+func (p *githubPlugin) FileContent(subpath string) ([]byte, error) {
+	// Github redirects "master" to "main" in new repos. They don't do the reverse
+	// so setting master here is better.
+	contentURL, err := url.JoinPath(
+		"https://raw.githubusercontent.com/",
+		p.org,
+		p.repo,
+		p.revision,
+		subpath,
+	)
+	if err != nil {
+		return nil, err
+	}
+
+	res, err := http.Get(contentURL)
+	if err != nil {
+		return nil, err
+	}
+	defer res.Body.Close()
+	if res.StatusCode != http.StatusOK {
+		return nil, usererr.New(
+			"failed to get plugin github:%s (Status code %d). \nPlease make sure a "+
+				"devbox.json file exists in the root of the repo.",
+			p.raw,
+			res.StatusCode,
+		)
+	}
+	return io.ReadAll(res.Body)
+}
diff --git a/internal/plugin/includes.go b/internal/plugin/includes.go
@@ -15,6 +15,7 @@ import (
 type Includable interface {
 	CanonicalName() string
 	Hash() string
+	FileContent(subpath string) ([]byte, error)
 }
 
 func (m *Manager) ParseInclude(include string) (Includable, error) {
@@ -26,6 +27,8 @@ func (m *Manager) ParseInclude(include string) (Includable, error) {
 	} else if includeType == "path" {
 		absPath := filepath.Join(m.ProjectDir(), name)
 		return newLocalPlugin(absPath)
+	} else if includeType == "github" {
+		return newGithubPlugin(name)
 	}
 	return nil, usererr.New("unknown include type %q", includeType)
 }
@@ -71,11 +74,11 @@ func (l *localPlugin) IsLocal() bool {
 	return true
 }
 
-func (l *localPlugin) contentPath(subpath string) string {
-	return filepath.Join(filepath.Dir(l.path), subpath)
-}
-
 func (l *localPlugin) Hash() string {
 	h, _ := cuecfg.FileHash(l.path)
 	return h
 }
+
+func (l *localPlugin) FileContent(subpath string) ([]byte, error) {
+	return os.ReadFile(filepath.Join(filepath.Dir(l.path), subpath))
+}
diff --git a/internal/plugin/plugin.go b/internal/plugin/plugin.go
@@ -134,7 +134,7 @@ func (m *Manager) createFile(
 ) error {
 	name := pkg.CanonicalName()
 	debug.Log("Creating file %q from contentPath: %q", filePath, contentPath)
-	content, err := getFileContent(pkg, contentPath)
+	content, err := pkg.FileContent(contentPath)
 	if err != nil {
 		return errors.WithStack(err)
 	}
diff --git a/plugins/plugins.go b/plugins/plugins.go
@@ -1,6 +1,29 @@
 package plugins
 
-import "embed"
+import (
+	"embed"
+	"io/fs"
+	"strings"
+
+	"github.com/samber/lo"
+)
 
 //go:embed *.json */*
-var BuiltIn embed.FS
+var builtIn embed.FS
+
+func Builtins() ([]fs.DirEntry, error) {
+	entries, err := builtIn.ReadDir(".")
+	if err != nil {
+		return nil, err
+	}
+	return lo.Filter(entries, func(e fs.DirEntry, _ int) bool {
+		return !e.IsDir() && !strings.HasSuffix(e.Name(), ".go")
+	}), nil
+}
+
+type BuiltIn struct {
+}
+
+func (f *BuiltIn) FileContent(contentPath string) ([]byte, error) {
+	return builtIn.ReadFile(contentPath)
+}

-Original file line number
+Diff line change
@@ @@ -0,0 +1,4 @@ @@
 +{
 +  "lockfile_version": "1",
 +  "packages": {}
 +}
Original file line number	Diff line number	Diff line change
`@@ -56,12 +56,12 @@ func (d DebugMiddleware) postRun(cmd cobra.Command, args []string, runErr erro`
`56`	`56`	`if runErr == nil {`
`57`	`57`	`return`
`58`	`58`	`}`
`59`		`- if usererr.HasUserMessage(runErr) {`
`60`		`- if usererr.IsWarning(runErr) {`
	`59`	`+ if userErr, hasUserErr := usererr.Extract(runErr); hasUserErr {`
	`60`	`+ if usererr.IsWarning(userErr) {`
`61`	`61`	`ux.Fwarning(cmd.ErrOrStderr(), runErr.Error())`
`62`	`62`	`return`
`63`	`63`	`}`
`64`		`- color.New(color.FgRed).Fprintf(cmd.ErrOrStderr(), "\nError: %s\n\n", runErr.Error())`
	`64`	`+ color.New(color.FgRed).Fprintf(cmd.ErrOrStderr(), "\nError: %s\n\n", userErr.Error())`
`65`	`65`	`} else {`
`66`	`66`	`color.New(color.FgRed).Fprintf(cmd.ErrOrStderr(), "Error: %v\n\n", runErr)`
`67`	`67`	`}`
Original file line number	Diff line number	Diff line change
`@@ -53,7 +53,7 @@ func NewWarning(msg string, args ...any) error {`
`53`	`53`	`func WithUserMessage(source error, msg string, args ...any) error {`
`54`	`54`	`// We don't want to wrap the error if it already has a user message. Doing`
`55`	`55`	`// so would obscure the original error message which is likely more useful.`
`56`		`- if source == nil \|\| HasUserMessage(source) {`
	`56`	`+ if source == nil \|\| hasUserMessage(source) {`
`57`	`57`	`return source`
`58`	`58`	`}`
`59`	`59`	`return &combined{`
`@@ -63,7 +63,7 @@ func WithUserMessage(source error, msg string, args ...any) error {`
`63`	`63`	`}`
`64`	`64`
`65`	`65`	`func WithLoggedUserMessage(source error, msg string, args ...any) error {`
`66`		`- if source == nil \|\| HasUserMessage(source) {`
	`66`	`+ if source == nil \|\| hasUserMessage(source) {`
`67`	`67`	`return source`
`68`	`68`	`}`
`69`	`69`	`return &combined{`
`@@ -73,9 +73,13 @@ func WithLoggedUserMessage(source error, msg string, args ...any) error {`
`73`	`73`	`}`
`74`	`74`	`}`
`75`	`75`
`76`		`-func HasUserMessage(err error) bool {`
	`76`	`+// Extract unwraps and returns the user error if it exists.`
	`77`	`+func Extract(err error) (error, bool) { // nolint: revive`
`77`	`78`	`c := &combined{}`
`78`		`- return errors.As(err, &c) // note double pointer`
	`79`	`+ if errors.As(err, &c) {`
	`80`	`+ return c, true`
	`81`	`+ }`
	`82`	`+ return nil, false`
`79`	`83`	`}`
`80`	`84`
`81`	`85`	`// ShouldLogError returns true if the it's a logged user error or is a non-user error`
`@@ -126,3 +130,8 @@ func (c *combined) Format(s fmt.State, verb rune) {`
`126`	`130`	`Format(s fmt.State, verb rune)`
`127`	`131`	`}).Format(s, verb)`
`128`	`132`	`}`
	`133`	`+`
	`134`	`+func hasUserMessage(err error) bool {`
	`135`	`+ _, hasUserMessage := Extract(err)`
	`136`	`+ return hasUserMessage`
	`137`	`+}`