diff --git a/internal/plugin/github.go b/internal/plugin/github.go index c2d44cf252a..453f1aeba3b 100644 --- a/internal/plugin/github.go +++ b/internal/plugin/github.go @@ -5,14 +5,41 @@ import ( "io" "net/http" "net/url" + "regexp" + "strings" + "github.com/pkg/errors" + "github.com/samber/lo" "go.jetpack.io/devbox/internal/boxcli/usererr" "go.jetpack.io/devbox/internal/cachehash" "go.jetpack.io/devbox/nix/flake" ) type githubPlugin struct { - ref flake.Ref + ref flake.Ref + name string +} + +// Github only allows alphanumeric, hyphen, underscore, and period in repo names. +// but we clean up just in case. +var githubNameRegexp = regexp.MustCompile("[^a-zA-Z0-9-_.]+") + +func newGithubPlugin(ref flake.Ref) (*githubPlugin, error) { + plugin := &githubPlugin{ref: ref} + // For backward compatibility, we don't strictly require name to be present + // in github plugins. If it's missing, we just use the directory as the name. + name, err := getPluginNameFromContent(plugin) + if err != nil && !errors.Is(err, errNameMissing) { + return nil, err + } + if name == "" { + name = strings.ReplaceAll(ref.Dir, "/", "-") + } + plugin.name = githubNameRegexp.ReplaceAllString( + strings.Join(lo.Compact([]string{ref.Owner, ref.Repo, name}), "."), + " ", + ) + return plugin, nil } func (p *githubPlugin) Fetch() ([]byte, error) { @@ -20,7 +47,7 @@ func (p *githubPlugin) Fetch() ([]byte, error) { } func (p *githubPlugin) CanonicalName() string { - return p.ref.Owner + "-" + p.ref.Repo + return p.name } func (p *githubPlugin) Hash() string { diff --git a/internal/plugin/github_test.go b/internal/plugin/github_test.go index 206f0612e3c..34bbaa80c09 100644 --- a/internal/plugin/github_test.go +++ b/internal/plugin/github_test.go @@ -1,8 +1,10 @@ package plugin import ( + "strings" "testing" + "github.com/samber/lo" "github.com/stretchr/testify/assert" "go.jetpack.io/devbox/nix/flake" ) @@ -23,6 +25,7 @@ func TestNewGithubPlugin(t *testing.T) { Owner: "jetpack-io", Repo: "devbox-plugins", }, + name: "jetpack-io.devbox-plugins", }, expectedURL: "https://raw.githubusercontent.com/jetpack-io/devbox-plugins/master", }, @@ -36,6 +39,7 @@ func TestNewGithubPlugin(t *testing.T) { Repo: "devbox-plugins", Dir: "mongodb", }, + name: "jetpack-io.devbox-plugins.mongodb", }, expectedURL: "https://raw.githubusercontent.com/jetpack-io/devbox-plugins/master/mongodb", }, @@ -50,6 +54,7 @@ func TestNewGithubPlugin(t *testing.T) { Ref: "my-branch", Dir: "mongodb", }, + name: "jetpack-io.devbox-plugins.mongodb", }, expectedURL: "https://raw.githubusercontent.com/jetpack-io/devbox-plugins/my-branch/mongodb", }, @@ -64,6 +69,7 @@ func TestNewGithubPlugin(t *testing.T) { Ref: "initials/my-branch", Dir: "mongodb", }, + name: "jetpack-io.devbox-plugins.mongodb", }, expectedURL: "https://raw.githubusercontent.com/jetpack-io/devbox-plugins/initials/my-branch/mongodb", }, @@ -71,7 +77,8 @@ func TestNewGithubPlugin(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { - actual, _ := parseIncludable(testCase.Include, "") + actual, err := newGithubPluginForTest(testCase.Include) + assert.NoError(t, err) assert.Equal(t, &testCase.expected, actual) u, err := testCase.expected.url("") assert.Nil(t, err) @@ -79,3 +86,19 @@ func TestNewGithubPlugin(t *testing.T) { }) } } + +// keep in sync with newGithubPlugin +func newGithubPluginForTest(include string) (*githubPlugin, error) { + ref, err := flake.ParseRef(include) + if err != nil { + return nil, err + } + + plugin := &githubPlugin{ref: ref} + name := strings.ReplaceAll(ref.Dir, "/", "-") + plugin.name = githubNameRegexp.ReplaceAllString( + strings.Join(lo.Compact([]string{ref.Owner, ref.Repo, name}), "."), + " ", + ) + return plugin, nil +} diff --git a/internal/plugin/includable.go b/internal/plugin/includable.go index b3ef2e43cc9..77045da139d 100644 --- a/internal/plugin/includable.go +++ b/internal/plugin/includable.go @@ -1,15 +1,18 @@ package plugin import ( + "encoding/json" "fmt" + "regexp" + "go.jetpack.io/devbox/internal/boxcli/usererr" "go.jetpack.io/devbox/nix/flake" ) type Includable interface { CanonicalName() string - Hash() string FileContent(subpath string) ([]byte, error) + Hash() string LockfileKey() string } @@ -22,8 +25,41 @@ func parseIncludable(includableRef, workingDir string) (Includable, error) { case flake.TypePath: return newLocalPlugin(ref, workingDir) case flake.TypeGitHub: - return &githubPlugin{ref: ref}, nil + return newGithubPlugin(ref) default: return nil, fmt.Errorf("unsupported ref type %q", ref.Type) } } + +type fetcher interface { + Includable + Fetch() ([]byte, error) +} + +var ( + nameRegex = regexp.MustCompile(`^[a-zA-Z0-9_\- ]+$`) + errNameMissing = usererr.New("'name' is missing") +) + +func getPluginNameFromContent(plugin fetcher) (string, error) { + content, err := plugin.Fetch() + if err != nil { + return "", err + } + m := map[string]any{} + if err := json.Unmarshal(content, &m); err != nil { + return "", err + } + name, ok := m["name"].(string) + if !ok || name == "" { + return "", + fmt.Errorf("%w in plugin %s", errNameMissing, plugin.LockfileKey()) + } + if !nameRegex.MatchString(name) { + return "", usererr.New( + "plugin %s has an invalid name %q. Name must match %s", + plugin.LockfileKey(), name, nameRegex, + ) + } + return name, nil +} diff --git a/internal/plugin/local.go b/internal/plugin/local.go index 2851c6eca8e..91a123dc264 100644 --- a/internal/plugin/local.go +++ b/internal/plugin/local.go @@ -1,13 +1,10 @@ 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/nix/flake" ) @@ -18,29 +15,12 @@ type LocalPlugin struct { pluginDir string } -var nameRegex = regexp.MustCompile(`^[a-zA-Z0-9_\- ]+$`) - func newLocalPlugin(ref flake.Ref, pluginDir string) (*LocalPlugin, error) { plugin := &LocalPlugin{ref: ref, pluginDir: pluginDir} - content, err := plugin.Fetch() + name, err := getPluginNameFromContent(plugin) 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'", plugin.Path()) - } - if !nameRegex.MatchString(name) { - return nil, usererr.New( - "plugin %s has an invalid name %q. Name must match %s", - plugin.Path(), name, nameRegex, - ) - } plugin.name = name return plugin, nil }