jetify-com · mikeland73 · Feb 29, 2024 · Feb 22, 2024 · Feb 22, 2024 · Feb 22, 2024
diff --git a/internal/plugin/files.go b/internal/plugin/files.go
@@ -17,9 +17,13 @@ func getConfigIfAny(pkg Includable, projectDir string) (*config, error) {
 	case *devpkg.Package:
 		return getBuiltinPluginConfigIfExists(pkg, projectDir)
 	case *githubPlugin:
-		return pkg.buildConfig(projectDir)
+		content, err := pkg.Fetch()
+		if err != nil {
+			return nil, errors.WithStack(err)
+		}
+		return buildConfig(pkg, projectDir, string(content))
 	case *localPlugin:
-		content, err := os.ReadFile(pkg.path)
+		content, err := os.ReadFile(pkg.Path)
 		if err != nil && !os.IsNotExist(err) {
 			return nil, errors.WithStack(err)
 		}

diff --git a/internal/plugin/github.go b/internal/plugin/github.go
@@ -4,56 +4,56 @@ import (
 	"io"
 	"net/http"
 	"net/url"
-	"strings"
 
-	"github.com/pkg/errors"
+	"github.com/samber/lo"
 	"go.jetpack.io/devbox/internal/boxcli/usererr"
 	"go.jetpack.io/devbox/internal/cachehash"
 )
 
 type githubPlugin struct {
-	raw      string
-	org      string
-	repo     string
-	revision string
-	dir      string
+	RefLike
 }
 
-// newGithubPlugin returns a plugin that is hosted on github.
-// url is of the form org/repo?dir=<dir>
-// The (optional) dir must have a plugin.json"
-func newGithubPlugin(rawURL string) (*githubPlugin, error) {
-	pluginURL, err := url.Parse(rawURL)
+func newGithubPlugin(ref RefLike) *githubPlugin {
+	if ref.Ref.Ref == "" && ref.Rev == "" {
+		ref.Ref.Ref = "master"
+	}
+	return &githubPlugin{RefLike: ref}
+}
+
+func (p *githubPlugin) Fetch() ([]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.Owner,
+		p.Repo,
+		lo.Ternary(p.Rev == "", "master", p.Rev),
+		p.withFilename(p.Dir),
+	)
 	if err != nil {
 		return nil, err
 	}
 
-	parts := strings.SplitN(pluginURL.Path, "/", 3)
-
-	if len(parts) < 2 {
+	res, err := http.Get(contentURL)
+	if err != nil {
+		return nil, err
+	}
+	defer res.Body.Close()
+	if res.StatusCode != http.StatusOK {
 		return nil, usererr.New(
-			"invalid github plugin url %q. Must be of the form org/repo/[revision]",
-			rawURL,
+			"failed to fetch github import:%s (Status code %d). \nPlease make sure a "+
+				"%s file exists in the directory.",
+			contentURL,
+			res.StatusCode,
+			p.filename,
 		)
 	}
-
-	plugin := &githubPlugin{
-		raw:      rawURL,
-		org:      parts[0],
-		repo:     parts[1],
-		revision: "master",
-		dir:      pluginURL.Query().Get("dir"),
-	}
-
-	if len(parts) == 3 {
-		plugin.revision = parts[2]
-	}
-
-	return plugin, nil
+	return io.ReadAll(res.Body)
 }
 
 func (p *githubPlugin) CanonicalName() string {
-	return p.org + "-" + p.repo
+	return p.Owner + "-" + p.Repo
 }
 
 func (p *githubPlugin) Hash() string {
@@ -66,10 +66,10 @@ func (p *githubPlugin) FileContent(subpath string) ([]byte, error) {
 	// so setting master here is better.
 	contentURL, err := url.JoinPath(
 		"https://raw.githubusercontent.com/",
-		p.org,
-		p.repo,
-		p.revision,
-		p.dir,
+		p.Owner,
+		p.Repo,
+		lo.Ternary(p.Rev == "", "master", p.Rev),
+		p.Dir,
 		subpath,
 	)
 	if err != nil {
@@ -85,17 +85,9 @@ func (p *githubPlugin) FileContent(subpath string) ([]byte, error) {
 		return nil, usererr.New(
 			"failed to get plugin github:%s (Status code %d). \nPlease make sure a "+
 				"plugin.json file exists in plugin directory.",
-			p.raw,
+			p.String(),
 			res.StatusCode,
 		)
 	}
 	return io.ReadAll(res.Body)
 }
-
-func (p *githubPlugin) buildConfig(projectDir string) (*config, error) {
-	content, err := p.FileContent("plugin.json")
-	if err != nil {
-		return nil, errors.WithStack(err)
-	}
-	return buildConfig(p, projectDir, string(content))
-}
diff --git a/internal/plugin/github_test.go b/internal/plugin/github_test.go
@@ -4,58 +4,84 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"go.jetpack.io/devbox/nix/flake"
 )
 
 func TestNewGithubPlugin(t *testing.T) {
 	testCases := []struct {
 		name     string
+		Include  string
 		expected githubPlugin
 	}{
 		{
-			name: "parse basic github plugin",
+			name:    "parse basic github plugin",
+			Include: "github:jetpack-io/devbox-plugins",
 			expected: githubPlugin{
-				raw:      "jetpack-io/devbox-plugins",
-				org:      "jetpack-io",
-				repo:     "devbox-plugins",
-				revision: "master",
+				RefLike: RefLike{
+					Ref: flake.Ref{
+						Type:  "github",
+						Owner: "jetpack-io",
+						Repo:  "devbox-plugins",
+						Ref:   "master",
+					},
+					filename: pluginConfigName,
+				},
 			},
 		},
 		{
-			name: "parse github plugin with dir param",
+			name:    "parse github plugin with dir param",
+			Include: "github:jetpack-io/devbox-plugins?dir=mongodb",
 			expected: githubPlugin{
-				raw:      "jetpack-io/devbox-plugins?dir=mongodb",
-				org:      "jetpack-io",
-				repo:     "devbox-plugins",
-				revision: "master",
-				dir:      "mongodb",
+				RefLike: RefLike{
+					Ref: flake.Ref{
+						Type:  "github",
+						Owner: "jetpack-io",
+						Repo:  "devbox-plugins",
+						Ref:   "master",
+						Dir:   "mongodb",
+					},
+					filename: pluginConfigName,
+				},
 			},
 		},
 		{
-			name: "parse github plugin with dir param and rev",
+			name:    "parse github plugin with dir param and rev",
+			Include: "github:jetpack-io/devbox-plugins/my-branch?dir=mongodb",
 			expected: githubPlugin{
-				raw:      "jetpack-io/devbox-plugins/my-branch?dir=mongodb",
-				org:      "jetpack-io",
-				repo:     "devbox-plugins",
-				revision: "my-branch",
-				dir:      "mongodb",
+				RefLike: RefLike{
+					Ref: flake.Ref{
+						Type:  "github",
+						Owner: "jetpack-io",
+						Repo:  "devbox-plugins",
+						Ref:   "my-branch",
+						Dir:   "mongodb",
+					},
+					filename: pluginConfigName,
+				},
 			},
 		},
 		{
-			name: "parse github plugin with dir param and rev",
+			name:    "parse github plugin with dir param and rev",
+			Include: "github:jetpack-io/devbox-plugins/initials/my-branch?dir=mongodb",
 			expected: githubPlugin{
-				raw:      "jetpack-io/devbox-plugins/initials/my-branch?dir=mongodb",
-				org:      "jetpack-io",
-				repo:     "devbox-plugins",
-				revision: "initials/my-branch",
-				dir:      "mongodb",
+				RefLike: RefLike{
+					Ref: flake.Ref{
+						Type:  "github",
+						Owner: "jetpack-io",
+						Repo:  "devbox-plugins",
+						Ref:   "initials/my-branch",
+						Dir:   "mongodb",
+					},
+					filename: pluginConfigName,
+				},
 			},
 		},
 	}
 
 	for _, testCase := range testCases {
 		t.Run(testCase.name, func(t *testing.T) {
-			actual, _ := newGithubPlugin(testCase.expected.raw)
-			assert.Equal(t, actual, &testCase.expected)
+			actual, _ := parseReflike(testCase.Include)
+			assert.Equal(t, &testCase.expected, actual)
 		})
 	}
 }
diff --git a/internal/plugin/includes.go b/internal/plugin/includes.go
@@ -1,84 +1,14 @@
 package plugin
 
 import (
-	"encoding/json"
-	"os"
-	"path/filepath"
-	"regexp"
 	"strings"
 
-	"go.jetpack.io/devbox/internal/boxcli/usererr"
-	"go.jetpack.io/devbox/internal/cachehash"
 	"go.jetpack.io/devbox/internal/devpkg"
 )
 
-type Includable interface {
-	CanonicalName() string
-	Hash() string
-	FileContent(subpath string) ([]byte, error)
-}
-
 func (m *Manager) ParseInclude(include string) (Includable, error) {
-	includeType, name, _ := strings.Cut(include, ":")
-	if name == "" {
-		return nil, usererr.New("include name is required")
-	} else if includeType == "plugin" {
+	if t, name, _ := strings.Cut(include, ":"); t == "plugin" {
 		return devpkg.PackageFromStringWithDefaults(name, m.lockfile), nil
-	} 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)
-}
-
-type localPlugin struct {
-	name string
-	path string
-}
-
-var nameRegex = regexp.MustCompile(`^[a-zA-Z0-9_\- ]+$`)
-
-func newLocalPlugin(path string) (*localPlugin, error) {
-	content, err := os.ReadFile(path)
-	if err != nil {
-		return nil, err
-	}
-	m := map[string]any{}
-	if err := json.Unmarshal(content, &m); err != nil {
-		return nil, err
-	}
-	name, ok := m["name"].(string)
-	if !ok || name == "" {
-		return nil,
-			usererr.New("plugin %s is missing a required field 'name'", path)
 	}
-	if !nameRegex.MatchString(name) {
-		return nil, usererr.New(
-			"plugin %s has an invalid name %q. Name must match %s",
-			path, name, nameRegex,
-		)
-	}
-	return &localPlugin{
-		name: name,
-		path: path,
-	}, nil
-}
-
-func (l *localPlugin) CanonicalName() string {
-	return l.name
-}
-
-func (l *localPlugin) IsLocal() bool {
-	return true
-}
-
-func (l *localPlugin) Hash() string {
-	h, _ := cachehash.Bytes([]byte(l.path))
-	return h
-}
-
-func (l *localPlugin) FileContent(subpath string) ([]byte, error) {
-	return os.ReadFile(filepath.Join(filepath.Dir(l.path), subpath))
+	return parseReflike(include)
 }