diff --git a/README.md b/README.md index b1828467..abaeb53b 100644 --- a/README.md +++ b/README.md @@ -303,6 +303,8 @@ Use `uses: builtin:checkout` to check out a repository without downloading `acti Each runner starts its own cache server, so runners do not share cached entries. A runner in a container on the jobs' Docker daemon attaches itself to each job network, so jobs reach its cache without extra setup. +As on GitHub, a job restores caches saved for its own ref, its pull request's base branch and the default branch, so a cache saved by a pull request is only restored by that pull request. + **Sharing a cache between runners** Run one `gitea-runner cache-server` and point every runner at it, using the same secret everywhere, for example from `openssl rand -hex 32`: diff --git a/act/artifactcache/doc.go b/act/artifactcache/doc.go index 4af79a18..2c375f78 100644 --- a/act/artifactcache/doc.go +++ b/act/artifactcache/doc.go @@ -6,7 +6,5 @@ // // Inspired by https://github.com/sp-ricard-valverde/github-act-cache-server // -// TODO: Authorization -// TODO: Restrictions for accessing a cache, see https://docs.github.com/en/actions/using-workflows/caching-dependencies-to-speed-up-workflows#restrictions-for-accessing-a-cache // TODO: Force deleting cache entries, see https://docs.github.com/en/actions/using-workflows/caching-dependencies-to-speed-up-workflows#force-deleting-cache-entries package artifactcache diff --git a/act/artifactcache/handler.go b/act/artifactcache/handler.go index 5dbef3fe..4c03f54a 100644 --- a/act/artifactcache/handler.go +++ b/act/artifactcache/handler.go @@ -59,6 +59,10 @@ type credKey struct{} type JobCredential struct { Repo string `json:"repo"` + Ref string `json:"ref"` + BaseRef string `json:"base_ref"` + DefaultRef string `json:"default_ref"` + // Results is the instance whose artifact service this server forwards for the job, and // InsecureTLS how the runner reaches it; see results.go. Results string `json:"-"` @@ -68,6 +72,10 @@ type JobCredential struct { PublicURL string `json:"public_url"` } +func (cred JobCredential) owns(cache *Cache) bool { + return cache.Repo == cred.Repo && cache.Ref == cred.Ref +} + // credEntry holds a registered job's credential along with an active // registration count. RegisterJob is reference-counted so that if two tasks // briefly share an ACTIONS_RUNTIME_TOKEN — e.g. a runner that retries a task @@ -653,7 +661,7 @@ func (h *Handler) find(w http.ResponseWriter, r *http.Request) { } defer db.Close() - cache, err := h.lookupCache(db, cred.Repo, keys, version) + cache, err := h.lookupCache(db, cred, keys, version) if err != nil { h.responseJSON(w, r, 500, err) return @@ -669,26 +677,27 @@ func (h *Handler) find(w http.ResponseWriter, r *http.Request) { }) } -// lookupCache returns the entry to restore for these keys, or (nil, nil) when there is none: -// either nothing matched, or the match had lost its blob to a prune, in which case the dangling -// entry is dropped on the way out. -func (h *Handler) lookupCache(db *cacheDB, repo string, keys []string, version string) (*Cache, error) { - cache, err := findCache(db, repo, keys, version) - if err != nil || cache == nil { - return nil, err +// lookupCache returns the entry to restore or (nil, nil), dropping matches whose blob was pruned. +func (h *Handler) lookupCache(db *cacheDB, cred JobCredential, keys []string, version string) (*Cache, error) { + for { + cache, err := findCache(db, cred, keys, version) + if err != nil || cache == nil { + return nil, err + } + ok, err := h.storage.Exist(cache.ID) + if err != nil { + return nil, err + } + if ok { + // Handing out a download URL counts as access, or eviction could drop the entry between + // this call and the GET that follows it. + h.touch(db, cache) + return cache, nil + } + if !h.deleteCache(db, cache) { + return nil, nil //nolint:nilnil // absence is not an error here + } } - ok, err := h.storage.Exist(cache.ID) - if err != nil { - return nil, err - } - if !ok { - h.deleteCache(db, cache) - return nil, nil //nolint:nilnil // absence is not an error here - } - // Handing out a download URL counts as access, or eviction could drop the entry between - // this call and the GET that follows it. - h.touch(db, cache) - return cache, nil } // POST /_apis/artifactcache/caches @@ -700,7 +709,7 @@ func (h *Handler) reserve(w http.ResponseWriter, r *http.Request) { return } - cache := &Cache{Repo: cred.Repo, Key: api.Key, Version: api.Version, Size: api.Size} + cache := &Cache{Repo: cred.Repo, Ref: cred.Ref, Key: api.Key, Version: api.Version, Size: api.Size} if cache.Size == 0 { cache.Size = -1 } @@ -749,7 +758,7 @@ func (h *Handler) upload(w http.ResponseWriter, r *http.Request) { return } - if cache.Repo != cred.Repo { + if !cred.owns(cache) { h.responseJSON(w, r, 403, fmt.Errorf("cache %d: forbidden", id)) return } @@ -808,7 +817,7 @@ func (h *Handler) commit(w http.ResponseWriter, r *http.Request) { return } - if cache.Repo != cred.Repo { + if !cred.owns(cache) { h.responseJSON(w, r, 403, fmt.Errorf("cache %d: forbidden", id)) return } @@ -1071,25 +1080,35 @@ func (h *Handler) signedArtifactURL(cred JobCredential, cacheID uint64, exp time return h.signedURL(cred, apiPath+"/artifacts", "", cacheID, exp) } -// if not found, return (nil, nil) instead of an error. -func findCache(db *cacheDB, repo string, keys []string, version string) (*Cache, error) { - for _, prefix := range keys { - // if a key in the list matches exactly, don't return partial matches - exact, err := findExactCache(db, repo, prefix, version, true) - if err != nil { - return nil, err +// findCache tries every key in one ref before the next ref, and returns (nil, nil) on a miss. +func findCache(db *cacheDB, cred JobCredential, keys []string, version string) (*Cache, error) { + refs := []string{cred.Ref} + for _, ref := range []string{cred.BaseRef, cred.DefaultRef} { + if ref != "" && !slices.Contains(refs, ref) { + refs = append(refs, ref) } - if exact != nil { - return exact, nil - } - caches, err := db.find(func(cache *Cache) bool { - return cache.Repo == repo && strings.HasPrefix(cache.Key, prefix) && cache.Version == version && cache.Complete - }) - if err != nil { - return nil, fmt.Errorf("find cache: %w", err) - } - if cache := latest(caches, (*Cache).createdAtNano); cache != nil { - return cache, nil + } + caches, err := db.find(func(cache *Cache) bool { + return cache.Repo == cred.Repo && cache.Version == version && cache.Complete && slices.Contains(refs, cache.Ref) && + slices.ContainsFunc(keys, func(prefix string) bool { return strings.HasPrefix(cache.Key, prefix) }) + }) + if err != nil { + return nil, fmt.Errorf("find cache: %w", err) + } + for _, ref := range refs { + for _, prefix := range keys { + var exact, partial []*Cache + for _, cache := range caches { + if cache.Ref == ref && cache.Key == prefix { + exact = append(exact, cache) + } else if cache.Ref == ref && strings.HasPrefix(cache.Key, prefix) { + partial = append(partial, cache) + } + } + // if a key in the list matches exactly, don't return partial matches + if cache := cmp.Or(latest(exact, (*Cache).createdAtNano), latest(partial, (*Cache).createdAtNano)); cache != nil { + return cache, nil + } } } return nil, nil //nolint:nilnil // pre-existing issue from nektos/act @@ -1111,9 +1130,9 @@ func latest(caches []*Cache, stamp func(*Cache) int64) *Cache { // A completed entry is the one to restore, sorted by when it was written. An incomplete one is a // reservation being uploaded to, sorted by when it was last written to, because the upload route // touches UsedAt on every part. -func findExactCache(db *cacheDB, repo, key, version string, complete bool) (*Cache, error) { +func findExactCache(db *cacheDB, cred JobCredential, key, version string, complete bool) (*Cache, error) { caches, err := db.find(func(cache *Cache) bool { - return cache.Key == key && cache.Repo == repo && cache.Version == version && cache.Complete == complete + return cred.owns(cache) && cache.Key == key && cache.Version == version && cache.Complete == complete }) if err != nil { return nil, fmt.Errorf("find cache: %w", err) @@ -1280,13 +1299,12 @@ func (h *Handler) evictExpired(db *cacheDB) { h.sweep(db, func(cache *Cache) bool { return cache.UsedAt < expired }) } -// evictSuperseded removes entries a newer one with the same key and version replaced. The -// grouping includes Repo so two repos sharing a (key, version) do not evict each other. +// evictSuperseded removes entries a newer one with the same repo, ref, key and version replaced. func (h *Handler) evictSuperseded(db *cacheDB) { - type entry struct{ repo, key, version string } + type entry struct{ repo, ref, key, version string } groups := make(map[entry][]*Cache) for _, cache := range h.caches(db, func(cache *Cache) bool { return cache.Complete }) { - group := entry{cache.Repo, cache.Key, cache.Version} + group := entry{cache.Repo, cache.Ref, cache.Key, cache.Version} groups[group] = append(groups[group], cache) } for _, caches := range groups { diff --git a/act/artifactcache/handler_test.go b/act/artifactcache/handler_test.go index ae731c74..0f990b01 100644 --- a/act/artifactcache/handler_test.go +++ b/act/artifactcache/handler_test.go @@ -759,11 +759,55 @@ func TestHandler_PrefixUsesNewestReservationWithinSecond(t *testing.T) { db, err := handler.openDB() require.NoError(t, err) defer db.Close() - cache, err := findCache(db, testRepo, []string{"deps-"}, "v") + cache, err := findCache(db, JobCredential{Repo: testRepo}, []string{"deps-"}, "v") require.NoError(t, err) require.Equal(t, newer, cache) } +func TestHandler_RestoreRefs(t *testing.T) { + t.Parallel() + const main, release, pull = "refs/heads/main", "refs/heads/release", "refs/pull/1/head" + now := time.Now().Unix() + entry := func(ref, key string, age int64) *Cache { + return &Cache{Repo: testRepo, Ref: ref, Key: key, Version: "v", Complete: true, CreatedAt: now - 3600 + age, UsedAt: now} + } + exactOnMain := entry(main, "deps-exact", -1) + onRelease := entry(release, "deps-release", 0) + onMain := entry(main, "deps-main", 1) + onPull := entry(pull, "deps-pull", 2) + unscoped := entry("", "deps-unscoped", 3) + handler := newTestHandler(t, Policy{}, exactOnMain, onRelease, onMain, onPull, unscoped, entry("refs/pull/2/head", "deps-lost", 4)) + for _, cache := range []*Cache{exactOnMain, onRelease, onMain, onPull, unscoped} { + writeBlob(t, handler, cache.ID) + } + + db, err := handler.openDB() + require.NoError(t, err) + defer db.Close() + + for _, tc := range []struct { + name string + cred JobCredential + keys []string + want *Cache + }{ + {"the default branch never restores what a pull request wrote", JobCredential{Ref: main, DefaultRef: main}, []string{"deps-"}, onMain}, + {"a pull request restores its own entry first", JobCredential{Ref: pull, BaseRef: release, DefaultRef: main}, []string{"deps-"}, onPull}, + {"a pull request falls back past a lost blob to its base branch before the default branch", JobCredential{Ref: "refs/pull/2/head", BaseRef: release, DefaultRef: main}, []string{"deps-"}, onRelease}, + {"a branch does not restore a sibling branch", JobCredential{Ref: "refs/heads/other", DefaultRef: main}, []string{"deps-"}, onMain}, + {"a prefix in the own ref wins over an exact key in the default branch", JobCredential{Ref: release, DefaultRef: main}, []string{"deps-exact", "deps-"}, onRelease}, + {"a job with a ref never restores entries saved without one", JobCredential{Ref: "refs/heads/other"}, []string{"deps-"}, nil}, + {"a job without a ref only restores entries saved without one", JobCredential{}, []string{"deps-"}, unscoped}, + } { + t.Run(tc.name, func(t *testing.T) { + tc.cred.Repo = testRepo + cache, err := handler.lookupCache(db, tc.cred, tc.keys, "v") + require.NoError(t, err) + assert.Equal(t, tc.want, cache) + }) + } +} + func TestHandler_evictOnCommit(t *testing.T) { t.Parallel() full := &Cache{Repo: testRepo, Key: "full", Version: "v", Complete: true, Size: 4 * miB, UsedAt: time.Now().Add(-time.Hour).Unix()} @@ -938,6 +982,7 @@ func TestHandler_CrossRepoIsolation(t *testing.T) { defer handler.Close() handler.RegisterJob("token-a", JobCredential{Repo: "owner/repoA"}) handler.RegisterJob("token-b", JobCredential{Repo: "owner/repoB"}) + handler.RegisterJob("token-a-pull", JobCredential{Repo: "owner/repoA", Ref: "refs/pull/1/head"}) base := handler.ExternalURL() + apiPath key := "shared-key" @@ -972,13 +1017,20 @@ func TestHandler_CrossRepoIsolation(t *testing.T) { resp.Body.Close() assert.Equal(t, http.StatusOK, resp.StatusCode) - req, err = http.NewRequest(http.MethodPatch, fmt.Sprintf("%s/caches/%d", base, id), bytes.NewReader([]byte("poison"))) - require.NoError(t, err) - req.Header.Set("Content-Range", "bytes 0-5/*") - resp, err = clientB.Do(req) - require.NoError(t, err) - resp.Body.Close() - assert.Equal(t, http.StatusForbidden, resp.StatusCode) + for _, client := range []*http.Client{clientB, {Transport: &bearerTransport{token: "token-a-pull"}}} { + req, err = http.NewRequest(http.MethodPatch, fmt.Sprintf("%s/caches/%d", base, id), bytes.NewReader([]byte("poison"))) + require.NoError(t, err) + req.Header.Set("Content-Range", "bytes 0-5/*") + resp, err = client.Do(req) + require.NoError(t, err) + resp.Body.Close() + assert.Equal(t, http.StatusForbidden, resp.StatusCode) + + resp, err = client.Post(fmt.Sprintf("%s/caches/%d", base, id), "", nil) + require.NoError(t, err) + resp.Body.Close() + assert.Equal(t, http.StatusForbidden, resp.StatusCode) + } } func TestHandler_ArtifactSignature(t *testing.T) { @@ -1083,49 +1135,20 @@ func TestHandler_ArtifactSignatureDownload(t *testing.T) { assert.Equal(t, content, body) } -func TestHandler_GC_PerRepoDedup(t *testing.T) { +func TestHandler_GC_DedupPerRepoAndRef(t *testing.T) { t.Parallel() - dir := filepath.Join(t.TempDir(), "artifactcache") - handler, err := StartHandler(Options{Dir: dir}) - require.NoError(t, err) - defer handler.Close() - handler.RegisterJob("tok-a", JobCredential{Repo: "owner/repoA"}) - handler.RegisterJob("tok-b", JobCredential{Repo: "owner/repoB"}) - - key := "shared-dedup-key" - version := "c19da02a2bd7e77277f1ac29ab45c09b7d46a4ee758284e26bb3045ad11d9d20" - - db, err := handler.openDB() - require.NoError(t, err) now := time.Now().Unix() stale := time.Now().Add(-inUseGrace - time.Minute).Unix() - a := &Cache{Repo: "owner/repoA", Key: key, Version: version, Complete: true, CreatedAt: stale, UsedAt: stale, Size: 1} - b := &Cache{Repo: "owner/repoB", Key: key, Version: version, Complete: true, CreatedAt: now, UsedAt: now, Size: 1} - require.NoError(t, handler.insertCache(db, a)) - require.NoError(t, handler.insertCache(db, b)) - require.NoError(t, handler.storage.Write(a.ID, 0, strings.NewReader("a"))) - _, err = handler.storage.Commit(a.ID, 1) - require.NoError(t, err) - require.NoError(t, handler.storage.Write(b.ID, 0, strings.NewReader("b"))) - _, err = handler.storage.Commit(b.ID, 1) - require.NoError(t, err) - require.NoError(t, db.Close()) - + entries := []*Cache{ + {Repo: "owner/repoA", Key: "shared", Version: "v", Complete: true, CreatedAt: stale, UsedAt: stale, Size: 1}, + {Repo: "owner/repoB", Key: "shared", Version: "v", Complete: true, CreatedAt: now, UsedAt: now, Size: 1}, + {Repo: "owner/repoA", Ref: "refs/pull/1/head", Key: "shared", Version: "v", Complete: true, CreatedAt: now, UsedAt: now, Size: 1}, + } + handler := newTestHandler(t, Policy{}, entries...) handler.gcAt = time.Time{} handler.gcCache() - db, err = handler.openDB() - require.NoError(t, err) - defer db.Close() - after, err := db.find(func(cache *Cache) bool { return cache.Key == key && cache.Version == version }) - require.NoError(t, err) - - repos := make(map[string]bool) - for _, c := range after { - repos[c.Repo] = true - } - assert.True(t, repos["owner/repoA"], "repoA's cache must survive dedup against repoB") - assert.True(t, repos["owner/repoB"], "repoB's cache must survive dedup against repoA") + assert.Len(t, keptKeys(t, handler, entries), len(entries)) } func TestHandler_InternalAPI_Disabled(t *testing.T) { @@ -1190,10 +1213,10 @@ func TestHandler_InternalAPI_AuthAndUsage(t *testing.T) { } assert.Equal(t, http.StatusUnauthorized, probe("via-internal-api")) - assert.Equal(t, http.StatusOK, post("/_internal/register", secret, `{"token":"via-internal-api","repo":"owner/repo","public_url":"http://runner.example","results":"https://gitea.example","insecure_tls":true}`)) + assert.Equal(t, http.StatusOK, post("/_internal/register", secret, `{"token":"via-internal-api","repo":"owner/repo","ref":"refs/pull/1/head","base_ref":"refs/heads/release","default_ref":"refs/heads/main","public_url":"http://runner.example","results":"https://gitea.example","insecure_tls":true}`)) cred, ok := handler.lookupCredential("via-internal-api") require.True(t, ok) - assert.Equal(t, JobCredential{Repo: testRepo, PublicURL: "http://runner.example"}, cred) + assert.Equal(t, JobCredential{Repo: testRepo, Ref: "refs/pull/1/head", BaseRef: "refs/heads/release", DefaultRef: "refs/heads/main", PublicURL: "http://runner.example"}, cred) assert.NotEqual(t, http.StatusUnauthorized, probe("via-internal-api")) assert.Equal(t, http.StatusOK, post("/_internal/revoke", secret, `{"token":"via-internal-api"}`)) assert.Equal(t, http.StatusUnauthorized, probe("via-internal-api")) diff --git a/act/artifactcache/handler_v2.go b/act/artifactcache/handler_v2.go index bb188f1c..07b5b5cb 100644 --- a/act/artifactcache/handler_v2.go +++ b/act/artifactcache/handler_v2.go @@ -73,7 +73,7 @@ func (h *Handler) v2CreateCacheEntry(w http.ResponseWriter, r *http.Request) { // An exact (key, version) match means the entry is already cached; the client then skips // the upload. A prefix match must not count here, or a shorter key would be reported as // existing and silently never saved. - if existing, err := findExactCache(db, cred.Repo, req.Key, req.Version, true); err != nil { + if existing, err := findExactCache(db, cred, req.Key, req.Version, true); err != nil { h.twirpError(w, r, twirpInternal, err) return } else if existing != nil { @@ -84,7 +84,7 @@ func (h *Handler) v2CreateCacheEntry(w http.ResponseWriter, r *http.Request) { // A second live reservation is finalized by whichever job calls last, against the other's upload. owner := hashedToken(bearerToken(r)) - if pending, err := findExactCache(db, cred.Repo, req.Key, req.Version, false); err != nil { + if pending, err := findExactCache(db, cred, req.Key, req.Version, false); err != nil { h.twirpError(w, r, twirpInternal, err) return } else if pending != nil && pending.UsedAt > time.Now().Add(-uploadStallTimeout).Unix() { @@ -103,6 +103,7 @@ func (h *Handler) v2CreateCacheEntry(w http.ResponseWriter, r *http.Request) { now := time.Now() cache := &Cache{ Repo: cred.Repo, + Ref: cred.Ref, Key: req.Key, Version: req.Version, Owner: owner, @@ -137,13 +138,13 @@ func (h *Handler) v2FinalizeCacheEntryUpload(w http.ResponseWriter, r *http.Requ } defer db.Close() - cache, err := findExactCache(db, cred.Repo, req.Key, req.Version, false) + cache, err := findExactCache(db, cred, req.Key, req.Version, false) if err == nil && cache != nil && cache.Owner != hashedToken(bearerToken(r)) { cache = nil // not the reservation this job made, so not this job's to commit } if err == nil && cache == nil { // A retry whose first response was lost finds the entry already committed. - cache, err = findExactCache(db, cred.Repo, req.Key, req.Version, true) + cache, err = findExactCache(db, cred, req.Key, req.Version, true) } if err != nil { h.twirpError(w, r, twirpInternal, err) @@ -186,7 +187,7 @@ func (h *Handler) v2GetCacheEntryDownloadURL(w http.ResponseWriter, r *http.Requ } defer db.Close() - cache, err := h.lookupCache(db, cred.Repo, req.keys(), req.Version) + cache, err := h.lookupCache(db, cred, req.keys(), req.Version) if err != nil { h.twirpError(w, r, twirpInternal, err) return diff --git a/act/artifactcache/handler_v2_test.go b/act/artifactcache/handler_v2_test.go index 422b2849..06e809c1 100644 --- a/act/artifactcache/handler_v2_test.go +++ b/act/artifactcache/handler_v2_test.go @@ -231,6 +231,24 @@ func TestCacheServiceV2Lookups(t *testing.T) { got := v2Call(t, handler, otherClient, "GetCacheEntryDownloadURL", map[string]any{"key": "deps-abc", "version": "v1"}) assert.Equal(t, false, got["ok"]) }) + + t.Run("each ref saves and restores its own entry for the same key", func(t *testing.T) { + clients := map[string]*http.Client{"x": testClient} + for _, ref := range []string{"y", "z"} { + handler.RegisterJob(ref, JobCredential{Repo: testRepo, Ref: "refs/pull/" + ref + "/head"}) + clients[ref] = &http.Client{Transport: &bearerTransport{token: ref}} + created := v2Call(t, handler, clients[ref], "CreateCacheEntry", map[string]any{"key": "deps-abc", "version": "v1"}) + require.Equal(t, true, created["ok"]) + require.Equal(t, http.StatusCreated, putBlob(t, created["signed_upload_url"].(string), []byte(ref))) + } + for _, ref := range []string{"y", "z"} { + require.Equal(t, true, v2Call(t, handler, clients[ref], "FinalizeCacheEntryUpload", map[string]any{"key": "deps-abc", "version": "v1", "size_bytes": 1})["ok"]) + } + for content, client := range clients { + got := v2Call(t, handler, client, "GetCacheEntryDownloadURL", map[string]any{"key": "deps-abc", "version": "v1"}) + assert.Equal(t, content, string(getURL(t, got["signed_download_url"].(string)))) + } + }) } func TestCacheServiceV2RefusalsAreTwirp(t *testing.T) { diff --git a/act/artifactcache/model.go b/act/artifactcache/model.go index 9dfd3201..607a1757 100644 --- a/act/artifactcache/model.go +++ b/act/artifactcache/model.go @@ -15,6 +15,7 @@ type Request struct { type Cache struct { ID uint64 `json:"id"` Repo string `json:"repo"` + Ref string `json:"ref,omitzero"` Key string `json:"key"` Version string `json:"version"` Size int64 `json:"cacheSize"` diff --git a/internal/app/run/runner.go b/internal/app/run/runner.go index 8fd09d3a..e26c48b2 100644 --- a/internal/app/run/runner.go +++ b/internal/app/run/runner.go @@ -533,7 +533,7 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report. // is removed when the task finishes, so a leaked token stops working as // soon as the job ends rather than remaining valid for the runner's // lifetime. - revokeCache, resultsURL := r.registerCacheForTask(giteaRuntimeToken, preset.Repository, cacheURL, reporter) + revokeCache, resultsURL := r.registerCacheForTask(giteaRuntimeToken, cacheCredential(preset, cacheURL), reporter) defer revokeCache() eventJSON, err := json.Marshal(preset.Event) @@ -690,34 +690,54 @@ func (r *Runner) instanceOutOfReach(instance string) bool { // // Safe with an empty token (older Gitea did not issue one). // It also returns the URL to advertise as ACTIONS_RESULTS_URL. -func (r *Runner) registerCacheForTask(token, repo, publicURL string, reporter *report.Reporter) (func(), string) { +func (r *Runner) registerCacheForTask(token string, cred artifactcache.JobCredential, reporter *report.Reporter) (func(), string) { if token == "" || r.cacheHandler == nil { return func() {}, "" } - cred := artifactcache.JobCredential{ - Repo: repo, - PublicURL: publicURL, - Results: r.envs["ACTIONS_RESULTS_URL"], // the instance as the job would reach it - InsecureTLS: r.cfg.Runner.Insecure, - } + cred.Results = r.envs["ACTIONS_RESULTS_URL"] // the instance as the job would reach it + cred.InsecureTLS = r.cfg.Runner.Insecure revoke := r.cacheHandler.RegisterJob(token, cred) if r.cfg.Cache.ExternalServer != "" { - revokeLocal, revokeExternal := revoke, r.registerExternalCacheJob(token, repo, publicURL, reporter) + revokeLocal, revokeExternal := revoke, r.registerExternalCacheJob(token, cred, reporter) revoke = func() { revokeLocal(); revokeExternal() } } return revoke, r.cacheHandler.ResultsURL(cred) } +// cacheCredential scopes the job's cache entries by ref, as GitHub does. +func cacheCredential(preset *model.GithubContext, publicURL string) artifactcache.JobCredential { + ref := preset.Ref + if strings.HasPrefix(preset.EventName, "pull_request") && preset.EventName != "pull_request_target" { + number, _ := preset.Event["number"].(float64) + ref = fmt.Sprintf("refs/pull/%d/head", int64(number)) // review events take github.ref from the reviewer's commit_id + } + return artifactcache.JobCredential{ + Repo: preset.Repository, + Ref: ref, + BaseRef: branchRef(preset.BaseRef), + DefaultRef: branchRef(model.AsString(model.NestedMapLookup(preset.Event, "repository", "default_branch"))), + PublicURL: publicURL, + } +} + +func branchRef(branch string) string { + if branch == "" { + return "" + } + return "refs/heads/" + branch +} + // registerExternalCacheJob POSTs to the remote cache-server's control-plane. // Failures are logged but not fatal: if registration fails, the cache will // 401 the job's requests — better than failing the whole task for a cache // outage. The warning is mirrored to the job log so users can see why their // cache calls 401, instead of having to read the runner daemon's stderr. -func (r *Runner) registerExternalCacheJob(token, repo, publicURL string, reporter *report.Reporter) func() { +func (r *Runner) registerExternalCacheJob(token string, cred artifactcache.JobCredential, reporter *report.Reporter) func() { base := strings.TrimRight(r.cfg.Cache.ExternalServer, "/") - if err := postInternalCache(base+"/_internal/register", r.cfg.Cache.ExternalSecret, map[string]any{ - "token": token, "repo": repo, "public_url": publicURL, - }); err != nil { + if err := postInternalCache(base+"/_internal/register", r.cfg.Cache.ExternalSecret, struct { + Token string `json:"token"` + artifactcache.JobCredential + }{token, cred}); err != nil { logging.Warnf("cache external_server register failed (%s): %v", base, err) if reporter != nil { reporter.Logf("::warning::%s", runner.EscapeCommandData(fmt.Sprintf( @@ -736,7 +756,7 @@ func (r *Runner) registerExternalCacheJob(token, repo, publicURL string, reporte } } -func postInternalCache(url, secret string, body map[string]any) error { +func postInternalCache(url, secret string, body any) error { buf, err := json.Marshal(body) if err != nil { return err diff --git a/internal/app/run/runner_cache_test.go b/internal/app/run/runner_cache_test.go index 4688f0e4..dceb9d29 100644 --- a/internal/app/run/runner_cache_test.go +++ b/internal/app/run/runner_cache_test.go @@ -17,6 +17,7 @@ import ( clientmocks "gitea.com/gitea/runner/internal/pkg/client/mocks" "gitea.com/gitea/runner/internal/pkg/config" + "gitea.dev/actionslib/pkg/model" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -24,7 +25,7 @@ import ( func TestRunner_registerCacheForTask_NoOps(t *testing.T) { for token, handler := range map[string]*artifactcache.Handler{"tok": nil, "": {}} { r := &Runner{cacheHandler: handler} - unregister, _ := r.registerCacheForTask(token, "owner/repo", "", nil) + unregister, _ := r.registerCacheForTask(token, artifactcache.JobCredential{Repo: "owner/repo"}, nil) require.NotNil(t, unregister, token) unregister() } @@ -39,7 +40,7 @@ func TestRunner_CacheFullFlow_MatchesToolkit(t *testing.T) { r := &Runner{cfg: &config.Config{}, cacheHandler: handler, envs: map[string]string{"ACTIONS_RESULTS_URL": "https://gitea.example"}} const publicURL = "http://a1b2c3d4e5f6:8088" token := "full-flow-token" - unregister, resultsURL := r.registerCacheForTask(token, "owner/repo", publicURL, nil) + unregister, resultsURL := r.registerCacheForTask(token, artifactcache.JobCredential{Repo: "owner/repo", PublicURL: publicURL}, nil) assert.Equal(t, publicURL, resultsURL) base := handler.ExternalURL() + "/_apis/artifactcache" @@ -112,6 +113,24 @@ func decodeJSON(resp *http.Response, v any) error { return json.UnmarshalRead(resp.Body, v) } +func TestCacheCredential(t *testing.T) { + repository := map[string]any{"default_branch": "main"} + review := &model.GithubContext{ + Repository: "owner/repo", EventName: "pull_request_review", Ref: "refs/heads/main", BaseRef: "release", + Event: map[string]any{"repository": repository, "number": float64(3)}, + } + assert.Equal(t, artifactcache.JobCredential{ + Repo: "owner/repo", Ref: "refs/pull/3/head", BaseRef: "refs/heads/release", DefaultRef: "refs/heads/main", PublicURL: "http://cache", + }, cacheCredential(review, "http://cache")) + + review.EventName, review.Ref = "pull_request_target", "refs/heads/release" + assert.Equal(t, "refs/heads/release", cacheCredential(review, "").Ref) + + assert.Equal(t, artifactcache.JobCredential{ + Repo: "owner/repo", Ref: "refs/heads/feature", DefaultRef: "refs/heads/main", + }, cacheCredential(&model.GithubContext{Repository: "owner/repo", Ref: "refs/heads/feature", Event: map[string]any{"repository": repository}}, "")) +} + func TestRunner_ExternalCacheServer_RegisterRevoke(t *testing.T) { const secret = "shared-secret-for-tests" remote, err := artifactcache.StartHandler(artifactcache.Options{Dir: t.TempDir(), OutboundIP: "127.0.0.1", InternalSecret: secret}) @@ -151,7 +170,7 @@ func TestRunner_ExternalCacheServer_RegisterRevoke(t *testing.T) { require.Equal(t, http.StatusUnauthorized, probe(), "token must be unknown to the remote server before registration") - unregister, resultsURL := r.registerCacheForTask(token, repo, publicURL, nil) + unregister, resultsURL := r.registerCacheForTask(token, artifactcache.JobCredential{Repo: repo, Ref: "refs/heads/main", PublicURL: publicURL}, nil) require.NotEqual(t, http.StatusUnauthorized, probe(), "token must be accepted after registerCacheForTask") @@ -194,6 +213,12 @@ func TestRunner_ExternalCacheServer_RegisterRevoke(t *testing.T) { require.NoError(t, decodeJSON(resp, &hit)) require.True(t, strings.HasPrefix(hit.ArchiveLocation, publicURL+"/"), hit.ArchiveLocation) + unregisterOther, _ := r.registerCacheForTask("external-other-token", artifactcache.JobCredential{Repo: repo, Ref: "refs/heads/other"}, nil) + defer unregisterOther() + resp = cacheRequest(t, "external-other-token", http.MethodGet, base+"/cache?keys=ext-key&version=v", "", "", "") + resp.Body.Close() + assert.Equal(t, http.StatusNoContent, resp.StatusCode, "the remote server must scope the entry by the ref the runner sent") + dl, err := http.Get(strings.Replace(hit.ArchiveLocation, publicURL, r.cacheHandler.ExternalURL(), 1)) require.NoError(t, err) defer dl.Body.Close() diff --git a/internal/app/run/runner_test.go b/internal/app/run/runner_test.go index eb1b3b6f..70087f49 100644 --- a/internal/app/run/runner_test.go +++ b/internal/app/run/runner_test.go @@ -18,6 +18,7 @@ import ( "testing" "time" + "gitea.com/gitea/runner/act/artifactcache" "gitea.com/gitea/runner/act/common" "gitea.com/gitea/runner/act/common/logging" "gitea.com/gitea/runner/act/runner" @@ -325,7 +326,7 @@ func TestNewRunnerCacheServiceV2(t *testing.T) { // The registration is what makes it true: the cache server takes the results service over, // having been told which instance to forward the artifact half to. - revoke, resultsURL := r.registerCacheForTask(token, "owner/repo", r.cacheHandler.ExternalURL(), nil) + revoke, resultsURL := r.registerCacheForTask(token, artifactcache.JobCredential{Repo: "owner/repo", PublicURL: r.cacheHandler.ExternalURL()}, nil) defer revoke() require.Equal(t, r.cacheHandler.ExternalURL(), resultsURL)