[plugins] Cache github plugin (#1997)

mikeland73 · web-flow · commit c8d2ffd0b28c · 2024-04-15T15:10:07.000-07:00
## Summary Excessive github requests are slow and can also lead to API rate limiting. This caches content for 24 hours. Ideally we store resolved plugin data in the lock file and cache indefinitely (until user does update) but that's a bit more work so this is a stopgap. Users can use `devbox update` to clear cache. cc: @Lagoja ## How was it tested? Added print statements to track http requests and observed them going away after content is cached.
diff --git a/devbox.lock b/devbox.lock
@@ -2,28 +2,56 @@
   "lockfile_version": "1",
   "packages": {
     "go@latest": {
-      "last_modified": "2024-03-06T17:57:40Z",
-      "resolved": "github:NixOS/nixpkgs/58ae79ea707579c40102ddf62d84b902a987c58b#go_1_22",
+      "last_modified": "2024-03-22T11:26:23Z",
+      "resolved": "github:NixOS/nixpkgs/a3ed7406349a9335cb4c2a71369b697cecd9d351#go",
       "source": "devbox-search",
       "version": "1.22.1",
       "systems": {
         "aarch64-darwin": {
-          "store_path": "/nix/store/q955gd41jh08l5pgbmx7y5jlqwwk4rqk-go-1.22.1"
+          "outputs": [
+            {
+              "name": "out",
+              "path": "/nix/store/n1k6wf8q10q7k0863gb78b1rf0j07r7r-go-1.22.1",
+              "default": true
+            }
+          ],
+          "store_path": "/nix/store/n1k6wf8q10q7k0863gb78b1rf0j07r7r-go-1.22.1"
         },
         "aarch64-linux": {
-          "store_path": "/nix/store/ca92iff1jpd9a3l3i8222rzbkq949mlh-go-1.22.1"
+          "outputs": [
+            {
+              "name": "out",
+              "path": "/nix/store/fl6cjlp5bvykfz1kxrw687zxzld25pn7-go-1.22.1",
+              "default": true
+            }
+          ],
+          "store_path": "/nix/store/fl6cjlp5bvykfz1kxrw687zxzld25pn7-go-1.22.1"
         },
         "x86_64-darwin": {
-          "store_path": "/nix/store/f64hpgi6lnn8vrf22cff2y8307c7zns3-go-1.22.1"
+          "outputs": [
+            {
+              "name": "out",
+              "path": "/nix/store/xgdp7gnf6vzr2ick2ip1lhq4cww65p7w-go-1.22.1",
+              "default": true
+            }
+          ],
+          "store_path": "/nix/store/xgdp7gnf6vzr2ick2ip1lhq4cww65p7w-go-1.22.1"
         },
         "x86_64-linux": {
-          "store_path": "/nix/store/jw9qrkqlkm699ncbrbs1d4czfg4wg8da-go-1.22.1"
+          "outputs": [
+            {
+              "name": "out",
+              "path": "/nix/store/bp39dh48cdqp89hk5mpdi1lxdf0mjl7x-go-1.22.1",
+              "default": true
+            }
+          ],
+          "store_path": "/nix/store/bp39dh48cdqp89hk5mpdi1lxdf0mjl7x-go-1.22.1"
         }
       }
     },
     "runx:golangci/golangci-lint@latest": {
-      "resolved": "golangci/golangci-lint@v1.56.2",
-      "version": "v1.56.2"
+      "resolved": "golangci/golangci-lint@v1.57.2",
+      "version": "v1.57.2"
     },
     "runx:mvdan/gofumpt@latest": {
       "resolved": "mvdan/gofumpt@v0.6.0",
diff --git a/examples/plugins/github/devbox.d/jetify-com.devbox-plugin-example.my-github-plugin/some-file.txt b/examples/plugins/github/devbox.d/jetify-com.devbox-plugin-example.my-github-plugin/some-file.txt
@@ -0,0 +1 @@
+some data
diff --git a/go.mod b/go.mod
@@ -23,6 +23,7 @@ require (
 	github.com/getsentry/sentry-go v0.27.0
 	github.com/google/go-cmp v0.6.0
 	github.com/google/uuid v1.6.0
+	github.com/gosimple/slug v1.14.0
 	github.com/hashicorp/go-envparse v0.1.0
 	github.com/joho/godotenv v1.5.1
 	github.com/mattn/go-isatty v0.0.20
@@ -93,7 +94,6 @@ require (
 	github.com/google/go-github/v53 v53.2.0 // indirect
 	github.com/google/go-querystring v1.1.0 // indirect
 	github.com/google/renameio/v2 v2.0.0 // indirect
-	github.com/gosimple/slug v1.14.0 // indirect
 	github.com/gosimple/unidecode v1.0.1 // indirect
 	github.com/gregjones/httpcache v0.0.0-20190611155906-901d90724c79 // indirect
 	github.com/h2non/filetype v1.1.3 // indirect
diff --git a/go.sum b/go.sum
@@ -360,8 +360,8 @@ github.com/zealic/go2node v0.1.0 h1:ofxpve08cmLJBwFdI0lPCk9jfwGWOSD+s6216x0oAaA=
 github.com/zealic/go2node v0.1.0/go.mod h1:GrkFr+HctXwP7vzcU9RsgtAeJjTQ6Ud0IPCQAqpTfBg=
 go.jetpack.io/envsec v0.0.16-0.20240329013200-4174c0acdb00 h1:Kb+OlWOntAq+1nF+01ntqnQEqSJkFmLLS0RX5sl5zak=
 go.jetpack.io/envsec v0.0.16-0.20240329013200-4174c0acdb00/go.mod h1:dVG2n8fBAGpQczW8yk/6wuXb9uEhzaJF7wGXkGLRRCU=
-go.jetpack.io/pkg v0.0.0-20240411004921-791796648f19 h1:ZloUPW4zNknwS2mYSmMVAtMfl3H2zZUCz+MxU5s79fY=
-go.jetpack.io/pkg v0.0.0-20240411004921-791796648f19/go.mod h1:JfVScypw14E4GR2TA2Nv29JqalRtgv2C9QjeQcNS4UM=
+go.jetpack.io/pkg v0.0.0-20240415163115-0f42b5c54fe1 h1:qLatba17X9hZ0+3m+9FzDu48cQBhCZjDHWJDctPGYp4=
+go.jetpack.io/pkg v0.0.0-20240415163115-0f42b5c54fe1/go.mod h1:JfVScypw14E4GR2TA2Nv29JqalRtgv2C9QjeQcNS4UM=
 go.jetpack.io/pkg v0.0.0-20240415190428-d17de207b432 h1:5B9uUxqzwDLUi+m/SVawTMszOmCK6KnUWb4yiCZJumg=
 go.jetpack.io/pkg v0.0.0-20240415190428-d17de207b432/go.mod h1:JfVScypw14E4GR2TA2Nv29JqalRtgv2C9QjeQcNS4UM=
 go.jetpack.io/typeid v1.0.1-0.20240410183543-96a4fd53d1e2 h1:w9uWg8BAim374iWzxEuDhf6MJ/cMQVR/0xi/L3DgfT0=
diff --git a/internal/cachehash/hash.go b/internal/cachehash/hash.go
@@ -19,11 +19,16 @@ import (
 )
 
 // Bytes returns a hex-encoded hash of b.
-// TODO: This doesn't need to return an error.
-func Bytes(b []byte) (string, error) {
+func Bytes(b []byte) string {
 	h := newHash()
 	h.Write(b)
-	return hex.EncodeToString(h.Sum(nil)), nil
+	return hex.EncodeToString(h.Sum(nil))
+}
+
+// Bytes6 returns the first 6 characters of the hash of b.
+func Bytes6(b []byte) string {
+	hash := Bytes(b)
+	return hash[:min(len(hash), 6)]
 }
 
 // File returns a hex-encoded hash of a file's contents.
@@ -50,7 +55,7 @@ func JSON(a any) (string, error) {
 	if err != nil {
 		return "", redact.Errorf("marshal to json for hashing: %v", err)
 	}
-	return Bytes(b)
+	return Bytes(b), nil
 }
 
 // JSONFile compacts the JSON in a file and returns its hex-encoded hash.
@@ -66,7 +71,7 @@ func JSONFile(path string) (string, error) {
 	if err := json.Compact(buf, b); err != nil {
 		return "", redact.Errorf("compact json for hashing: %v", err)
 	}
-	return Bytes(buf.Bytes())
+	return Bytes(buf.Bytes()), nil
 }
 
 func newHash() hash.Hash { return sha256.New() }
diff --git a/internal/devbox/devbox.go b/internal/devbox/devbox.go
@@ -188,7 +188,7 @@ func (d *Devbox) ConfigHash() (string, error) {
 		}
 		buf.WriteString(h)
 	}
-	return cachehash.Bytes(buf.Bytes())
+	return cachehash.Bytes(buf.Bytes()), nil
 }
 
 func (d *Devbox) NixPkgsCommitHash() string {
@@ -1224,8 +1224,7 @@ var ignoreDevEnvVar = map[string]bool{
 }
 
 func (d *Devbox) ProjectDirHash() string {
-	h, _ := cachehash.Bytes([]byte(d.projectDir))
-	return h
+	return cachehash.Bytes([]byte(d.projectDir))
 }
 
 func (d *Devbox) addHashToEnv(env map[string]string) error {
diff --git a/internal/devbox/update.go b/internal/devbox/update.go
@@ -14,6 +14,7 @@ import (
 	"go.jetpack.io/devbox/internal/lock"
 	"go.jetpack.io/devbox/internal/nix"
 	"go.jetpack.io/devbox/internal/nix/nixprofile"
+	"go.jetpack.io/devbox/internal/plugin"
 	"go.jetpack.io/devbox/internal/searcher"
 	"go.jetpack.io/devbox/internal/shellgen"
 	"go.jetpack.io/devbox/internal/ux"
@@ -74,7 +75,7 @@ func (d *Devbox) Update(ctx context.Context, opts devopt.UpdateOpts) error {
 	// It will return an error if .devbox/gen/flake is missing
 	// TODO: Remove this if it's not needed.
 	_ = nix.FlakeUpdate(shellgen.FlakePath(d))
-	return nil
+	return plugin.Update()
 }
 
 func (d *Devbox) inputsToUpdate(
diff --git a/internal/devconfig/config.go b/internal/devconfig/config.go
@@ -277,7 +277,7 @@ func (c *Config) Hash() (string, error) {
 		return "", err
 	}
 	data = append(data, hash...)
-	return cachehash.Bytes(data)
+	return cachehash.Bytes(data), nil
 }
 
 func (c *Config) IsEnvsecEnabled() bool {
diff --git a/internal/devconfig/configfile/file.go b/internal/devconfig/configfile/file.go
@@ -84,7 +84,7 @@ func (c *ConfigFile) Hash() (string, error) {
 	}
 	ast := c.ast.root.Clone()
 	ast.Minimize()
-	return cachehash.Bytes(ast.Pack())
+	return cachehash.Bytes(ast.Pack()), nil
 }
 
 func (c *ConfigFile) Equals(other *ConfigFile) bool {
diff --git a/internal/devpkg/package.go b/internal/devpkg/package.go
@@ -442,7 +442,7 @@ func (p *Package) Hash() string {
 	}
 
 	if sum == "" {
-		sum, _ = cachehash.Bytes([]byte(p.installable.String()))
+		sum = cachehash.Bytes([]byte(p.installable.String()))
 	}
 	return sum[:min(len(sum), 6)]
 }
diff --git a/internal/plugin/github.go b/internal/plugin/github.go
@@ -9,14 +9,18 @@ import (
 	"os"
 	"regexp"
 	"strings"
+	"time"
 
 	"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"
+	"go.jetpack.io/pkg/filecache"
 )
 
+var githubCache = filecache.New[[]byte]("devbox/plugin/github")
+
 type githubPlugin struct {
 	ref  flake.Ref
 	name string
@@ -57,32 +61,47 @@ func (p *githubPlugin) CanonicalName() string {
 }
 
 func (p *githubPlugin) Hash() string {
-	h, _ := cachehash.Bytes([]byte(p.ref.String()))
-	return h
+	return cachehash.Bytes([]byte(p.ref.String()))
 }
 
 func (p *githubPlugin) FileContent(subpath string) ([]byte, error) {
-	req, err := p.request(subpath)
-	if err != nil {
-		return nil, err
-	}
-
-	client := &http.Client{}
-	res, err := client.Do(req)
+	contentURL, err := p.url(subpath)
 	if err != nil {
 		return nil, err
 	}
-	defer res.Body.Close()
-	if res.StatusCode != http.StatusOK {
-		return nil, usererr.New(
-			"failed to get plugin %s @ %s (Status code %d). \nPlease make "+
-				"sure a plugin.json file exists in plugin directory.",
-			p.LockfileKey(),
-			req.URL.String(),
-			res.StatusCode,
-		)
-	}
-	return io.ReadAll(res.Body)
+	return githubCache.GetOrSet(
+		contentURL,
+		func() ([]byte, time.Duration, error) {
+			req, err := p.request(contentURL)
+			if err != nil {
+				return nil, 0, err
+			}
+
+			client := &http.Client{}
+			res, err := client.Do(req)
+			if err != nil {
+				return nil, 0, err
+			}
+			defer res.Body.Close()
+			if res.StatusCode != http.StatusOK {
+				return nil, 0, usererr.New(
+					"failed to get plugin %s @ %s (Status code %d). \nPlease make "+
+						"sure a plugin.json file exists in plugin directory.",
+					p.LockfileKey(),
+					req.URL.String(),
+					res.StatusCode,
+				)
+			}
+			body, err := io.ReadAll(res.Body)
+			if err != nil {
+				return nil, 0, err
+			}
+			// Cache for 24 hours. Once we store the plugin in the lockfile, we
+			// should cache this indefinitely and only invalidate if the plugin
+			// is updated.
+			return body, 24 * time.Hour, nil
+		},
+	)
 }
 
 func (p *githubPlugin) url(subpath string) (string, error) {
@@ -98,12 +117,7 @@ func (p *githubPlugin) url(subpath string) (string, error) {
 	)
 }
 
-func (p *githubPlugin) request(subpath string) (*http.Request, error) {
-	contentURL, err := p.url(subpath)
-	if err != nil {
-		return nil, err
-	}
-
+func (p *githubPlugin) request(contentURL string) (*http.Request, error) {
 	req, err := http.NewRequest(http.MethodGet, contentURL, nil)
 	if err != nil {
 		return nil, err
diff --git a/internal/plugin/github_test.go b/internal/plugin/github_test.go
@@ -116,16 +116,19 @@ func TestGithubPluginAuth(t *testing.T) {
 	expectedURL := "https://raw.githubusercontent.com/jetpack-io/devbox-plugins/master/test"
 
 	t.Run("generate request for public Github repository", func(t *testing.T) {
-		actual, err := githubPlugin.request("test")
+		url, err := githubPlugin.url("test")
+		assert.NoError(t, err)
+		actual, err := githubPlugin.request(url)
 		assert.NoError(t, err)
 		assert.Equal(t, expectedURL, actual.URL.String())
 		assert.Equal(t, "", actual.Header.Get("Authorization"))
 	})
 
 	t.Run("generate request for private Github repository", func(t *testing.T) {
 		t.Setenv("GITHUB_TOKEN", "gh_abcd")
-
-		actual, err := githubPlugin.request("test")
+		url, err := githubPlugin.url("test")
+		assert.NoError(t, err)
+		actual, err := githubPlugin.request(url)
 		assert.NoError(t, err)
 		assert.Equal(t, expectedURL, actual.URL.String())
 		assert.Equal(t, "token gh_abcd", actual.Header.Get("Authorization"))
diff --git a/internal/plugin/local.go b/internal/plugin/local.go
@@ -43,8 +43,7 @@ func (l *LocalPlugin) IsLocal() bool {
 }
 
 func (l *LocalPlugin) Hash() string {
-	h, _ := cachehash.Bytes([]byte(filepath.Clean(l.Path())))
-	return h
+	return cachehash.Bytes([]byte(filepath.Clean(l.Path())))
 }
 
 func (l *LocalPlugin) FileContent(subpath string) ([]byte, error) {
diff --git a/internal/plugin/update.go b/internal/plugin/update.go
@@ -0,0 +1,5 @@
+package plugin
+
+func Update() error {
+	return githubCache.Clear()
+}

Original file line number	Diff line number	Diff line change
`@@ -19,11 +19,16 @@ import (`
`19`	`19`	`)`
`20`	`20`
`21`	`21`	`// Bytes returns a hex-encoded hash of b.`
`22`		`-// TODO: This doesn't need to return an error.`
`23`		`-func Bytes(b []byte) (string, error) {`
	`22`	`+func Bytes(b []byte) string {`
`24`	`23`	`h := newHash()`
`25`	`24`	`h.Write(b)`
`26`		`- return hex.EncodeToString(h.Sum(nil)), nil`
	`25`	`+ return hex.EncodeToString(h.Sum(nil))`
	`26`	`+}`
	`27`	`+`
	`28`	`+// Bytes6 returns the first 6 characters of the hash of b.`
	`29`	`+func Bytes6(b []byte) string {`
	`30`	`+ hash := Bytes(b)`
	`31`	`+ return hash[:min(len(hash), 6)]`
`27`	`32`	`}`
`28`	`33`
`29`	`34`	`// File returns a hex-encoded hash of a file's contents.`
`@@ -50,7 +55,7 @@ func JSON(a any) (string, error) {`
`50`	`55`	`if err != nil {`
`51`	`56`	`return "", redact.Errorf("marshal to json for hashing: %v", err)`
`52`	`57`	`}`
`53`		`- return Bytes(b)`
	`58`	`+ return Bytes(b), nil`
`54`	`59`	`}`
`55`	`60`
`56`	`61`	`// JSONFile compacts the JSON in a file and returns its hex-encoded hash.`
`@@ -66,7 +71,7 @@ func JSONFile(path string) (string, error) {`
`66`	`71`	`if err := json.Compact(buf, b); err != nil {`
`67`	`72`	`return "", redact.Errorf("compact json for hashing: %v", err)`
`68`	`73`	`}`
`69`		`- return Bytes(buf.Bytes())`
	`74`	`+ return Bytes(buf.Bytes()), nil`
`70`	`75`	`}`
`71`	`76`
`72`	`77`	`func newHash() hash.Hash { return sha256.New() }`
Original file line number	Diff line number	Diff line change
`@@ -188,7 +188,7 @@ func (d *Devbox) ConfigHash() (string, error) {`
`188`	`188`	`}`
`189`	`189`	`buf.WriteString(h)`
`190`	`190`	`}`
`191`		`- return cachehash.Bytes(buf.Bytes())`
	`191`	`+ return cachehash.Bytes(buf.Bytes()), nil`
`192`	`192`	`}`
`193`	`193`
`194`	`194`	`func (d *Devbox) NixPkgsCommitHash() string {`
`@@ -1224,8 +1224,7 @@ var ignoreDevEnvVar = map[string]bool{`
`1224`	`1224`	`}`
`1225`	`1225`
`1226`	`1226`	`func (d *Devbox) ProjectDirHash() string {`
`1227`		`- h, _ := cachehash.Bytes([]byte(d.projectDir))`
`1228`		`- return h`
	`1227`	`+ return cachehash.Bytes([]byte(d.projectDir))`
`1229`	`1228`	`}`
`1230`	`1229`
`1231`	`1230`	`func (d *Devbox) addHashToEnv(env map[string]string) error {`
Original file line number	Diff line number	Diff line change
`@@ -277,7 +277,7 @@ func (c *Config) Hash() (string, error) {`
`277`	`277`	`return "", err`
`278`	`278`	`}`
`279`	`279`	`data = append(data, hash...)`
`280`		`- return cachehash.Bytes(data)`
	`280`	`+ return cachehash.Bytes(data), nil`
`281`	`281`	`}`
`282`	`282`
`283`	`283`	`func (c *Config) IsEnvsecEnabled() bool {`
Original file line number	Diff line number	Diff line change
`@@ -84,7 +84,7 @@ func (c *ConfigFile) Hash() (string, error) {`
`84`	`84`	`}`
`85`	`85`	`ast := c.ast.root.Clone()`
`86`	`86`	`ast.Minimize()`
`87`		`- return cachehash.Bytes(ast.Pack())`
	`87`	`+ return cachehash.Bytes(ast.Pack()), nil`
`88`	`88`	`}`
`89`	`89`
`90`	`90`	`func (c ConfigFile) Equals(other ConfigFile) bool {`
Original file line number	Diff line number	Diff line change
`@@ -442,7 +442,7 @@ func (p *Package) Hash() string {`
`442`	`442`	`}`
`443`	`443`
`444`	`444`	`if sum == "" {`
`445`		`- sum, _ = cachehash.Bytes([]byte(p.installable.String()))`
	`445`	`+ sum = cachehash.Bytes([]byte(p.installable.String()))`
`446`	`446`	`}`
`447`	`447`	`return sum[:min(len(sum), 6)]`
`448`	`448`	`}`