chore: cache: move repo != cache.Repo in readCache

- it only is used after calling readCache
- add unit test

it reduces the number of testcase to be considered in handler

act/artifactcache/caches.go

@@ -20,7 +20,7 @@ import (
 type caches interface {
 	openDB() (*bolthold.Store, error)
 	validateMac(rundata RunData) (string, error)
-	readCache(id uint64) (*Cache, error)
+	readCache(id uint64, repo string) (*Cache, error)
 	useCache(id uint64) error
 	setgcAt(at time.Time)
 	gcCache()
@@ -139,7 +139,7 @@ func insertCache(db *bolthold.Store, cache *Cache) error {
 	return nil
 }
 
-func (c *cachesImpl) readCache(id uint64) (*Cache, error) {
+func (c *cachesImpl) readCache(id uint64, repo string) (*Cache, error) {
 	db, err := c.openDB()
 	if err != nil {
 		return nil, err
@@ -149,6 +149,10 @@ func (c *cachesImpl) readCache(id uint64) (*Cache, error) {
 	if err := db.Get(id, cache); err != nil {
 		return nil, fmt.Errorf("readCache: Get(%v): %w", id, err)
 	}
+	if cache.Repo != repo {
+		return nil, fmt.Errorf("readCache: Get(%v): cache.Repo %s != repo %s", id, cache.Repo, repo)
+	}
+
 	return cache, nil
 }
 

act/artifactcache/caches_test.go

@@ -0,0 +1,54 @@
+package artifactcache
+
+import (
+	"testing"
+	"time"
+
+	"github.com/sirupsen/logrus"
+	"github.com/timshannon/bolthold"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestCacheReadWrite(t *testing.T) {
+	caches, err := newCaches(t.TempDir(), "secret", logrus.New())
+	require.NoError(t, err)
+	t.Run("NotFound", func(t *testing.T) {
+		found, err := caches.readCache(456, "repo")
+		assert.Nil(t, found)
+		assert.ErrorIs(t, err, bolthold.ErrNotFound)
+	})
+
+	repo := "repository"
+	cache := &Cache{
+		Repo:    repo,
+		Key:     "key",
+		Version: "version",
+		Size:    444,
+	}
+	now := time.Now().Unix()
+	cache.CreatedAt = now
+	cache.UsedAt = now
+	cache.Repo = repo
+
+	t.Run("Insert", func(t *testing.T) {
+		db, err := caches.openDB()
+		require.NoError(t, err)
+		defer db.Close()
+		assert.NoError(t, insertCache(db, cache))
+	})
+
+	t.Run("Found", func(t *testing.T) {
+		found, err := caches.readCache(cache.ID, cache.Repo)
+		require.NoError(t, err)
+		assert.Equal(t, cache.ID, found.ID)
+	})
+
+	t.Run("InvalidRepo", func(t *testing.T) {
+		invalidRepo := "INVALID REPO"
+		found, err := caches.readCache(cache.ID, invalidRepo)
+		assert.Nil(t, found)
+		assert.ErrorContains(t, err, invalidRepo)
+	})
+}

act/artifactcache/handler.go

@@ -265,7 +265,7 @@ func (h *handler) upload(w http.ResponseWriter, r *http.Request, params httprout
 		return
 	}
 
-	cache, err := h.caches.readCache(id)
+	cache, err := h.caches.readCache(id, repo)
 	if err != nil {
 		if errors.Is(err, bolthold.ErrNotFound) {
 			h.responseJSON(w, r, 404, fmt.Errorf("cache %d: not reserved", id))
@@ -275,11 +275,6 @@ func (h *handler) upload(w http.ResponseWriter, r *http.Request, params httprout
 		return
 	}
 
-	if cache.Repo != repo {
-		// can only happen if the cache is corrupted
-		h.responseFatalJSON(w, r, fmt.Errorf("cache repo is not valid"))
-		return
-	}
 	if cache.WriteIsolationKey != rundata.WriteIsolationKey {
 		h.responseJSON(w, r, 403, fmt.Errorf("cache authorized for write isolation %q, but attempting to operate on %q", rundata.WriteIsolationKey, cache.WriteIsolationKey))
 		return
@@ -320,7 +315,7 @@ func (h *handler) commit(w http.ResponseWriter, r *http.Request, params httprout
 		return
 	}
 
-	cache, err := h.caches.readCache(id)
+	cache, err := h.caches.readCache(id, repo)
 	if err != nil {
 		if errors.Is(err, bolthold.ErrNotFound) {
 			h.responseJSON(w, r, 404, fmt.Errorf("cache %d: not reserved", id))
@@ -330,11 +325,6 @@ func (h *handler) commit(w http.ResponseWriter, r *http.Request, params httprout
 		return
 	}
 
-	if cache.Repo != repo {
-		// can only happen if the cache is corrupted
-		h.responseFatalJSON(w, r, fmt.Errorf("cache repo is not valid"))
-		return
-	}
 	if cache.WriteIsolationKey != rundata.WriteIsolationKey {
 		h.responseJSON(w, r, 403, fmt.Errorf("cache authorized for write isolation %q, but attempting to operate on %q", rundata.WriteIsolationKey, cache.WriteIsolationKey))
 		return
@@ -384,7 +374,7 @@ func (h *handler) get(w http.ResponseWriter, r *http.Request, params httprouter.
 		return
 	}
 
-	cache, err := h.caches.readCache(id)
+	cache, err := h.caches.readCache(id, repo)
 	if err != nil {
 		if errors.Is(err, bolthold.ErrNotFound) {
 			h.responseJSON(w, r, 404, fmt.Errorf("cache %d: not reserved", id))
@@ -394,11 +384,6 @@ func (h *handler) get(w http.ResponseWriter, r *http.Request, params httprouter.
 		return
 	}
 
-	if cache.Repo != repo {
-		// can only happen if the cache is corrupted
-		h.responseFatalJSON(w, r, fmt.Errorf("cache repo is not valid"))
-		return
-	}
 	// reads permitted against caches w/ the same isolation key, or no isolation key
 	if cache.WriteIsolationKey != rundata.WriteIsolationKey && cache.WriteIsolationKey != "" {
 		h.responseJSON(w, r, 403, fmt.Errorf("cache authorized for write isolation %q, but attempting to operate on %q", rundata.WriteIsolationKey, cache.WriteIsolationKey))