diff --git a/act/artifactcache/handler.go b/act/artifactcache/handler.go index 34322247..3cf2174a 100644 --- a/act/artifactcache/handler.go +++ b/act/artifactcache/handler.go @@ -52,7 +52,13 @@ type credKey struct{} // poison another repo's cache, even from inside a container that reaches the // cache server over the docker bridge network. type JobCredential struct { - Repo string + Repo string `json:"repo"` + + // Results is the instance whose artifact service this server forwards for the job, and + // InsecureTLS how the runner reaches it; see results.go. The tags are the wire format a + // remote runner registers with. + Results string `json:"results"` + InsecureTLS bool `json:"insecure_tls"` } // credEntry holds a registered job's credential along with an active @@ -165,6 +171,7 @@ func StartHandler(dir, outboundIP string, port uint16, internalSecret string, lo router.POST(internalPath+"/register", h.internalAuth(h.internalRegister)) router.POST(internalPath+"/revoke", h.internalAuth(h.internalRevoke)) h.registerV2Routes(router) + router.NotFound = http.HandlerFunc(h.forwardOrNotFound) h.router = router @@ -211,10 +218,11 @@ func (h *Handler) ExternalURL() string { // is only accepted while the job is running. // // Registrations are reference-counted: if a token is already registered, the -// existing repo is kept and the refcount is incremented. The entry is -// removed only when every revoker returned by RegisterJob has been called. +// credential it was registered with is kept and the refcount is incremented. +// The entry is removed only when every revoker returned by RegisterJob has +// been called. // This keeps a stray re-registration from silently revoking a live job. -func (h *Handler) RegisterJob(token, repo string) func() { +func (h *Handler) RegisterJob(token string, cred JobCredential) func() { if h == nil || token == "" { return func() {} } @@ -223,7 +231,7 @@ func (h *Handler) RegisterJob(token, repo string) func() { existing.refs++ } else { h.creds[token] = &credEntry{ - cred: JobCredential{Repo: repo}, + cred: cred, refs: 1, } } @@ -619,7 +627,7 @@ func (h *Handler) internalAuth(handler httprouter.Handle) httprouter.Handle { type internalRegisterBody struct { Token string `json:"token"` - Repo string `json:"repo"` + JobCredential } type internalRevokeBody struct { @@ -627,6 +635,15 @@ type internalRevokeBody struct { } // POST /_internal/register +// ResultsURL is what a job registered with cred should be given as ACTIONS_RESULTS_URL, or "" when +// the credential names no instance to forward the artifact half to. +func (h *Handler) ResultsURL(cred JobCredential) string { + if h == nil || cred.Results == "" { + return "" + } + return h.ExternalURL() +} + func (h *Handler) internalRegister(w http.ResponseWriter, r *http.Request, _ httprouter.Params) { var body internalRegisterBody if err := json.NewDecoder(r.Body).Decode(&body); err != nil { @@ -637,8 +654,9 @@ func (h *Handler) internalRegister(w http.ResponseWriter, r *http.Request, _ htt h.responseJSON(w, r, http.StatusBadRequest, errors.New("token is required")) return } - h.RegisterJob(body.Token, body.Repo) - h.responseJSON(w, r, http.StatusOK) + h.RegisterJob(body.Token, body.JobCredential) + // A server too old to forward answers without this, which is how the caller knows. + h.responseJSON(w, r, http.StatusOK, map[string]any{"results_url": h.ResultsURL(body.JobCredential)}) } // POST /_internal/revoke diff --git a/act/artifactcache/handler_test.go b/act/artifactcache/handler_test.go index 8ca0eead..3723e937 100644 --- a/act/artifactcache/handler_test.go +++ b/act/artifactcache/handler_test.go @@ -52,7 +52,7 @@ func TestHandler(t *testing.T) { dir := filepath.Join(t.TempDir(), "artifactcache") handler, err := StartHandler(dir, "", 0, "", nil) require.NoError(t, err) - handler.RegisterJob(testToken, testRepo) + handler.RegisterJob(testToken, JobCredential{Repo: testRepo}) base := fmt.Sprintf("%s%s", handler.ExternalURL(), apiPath) @@ -890,7 +890,7 @@ func TestHandler_UnregisterRevokes(t *testing.T) { require.NoError(t, err) defer handler.Close() - unregister := handler.RegisterJob("tmp-token", testRepo) + unregister := handler.RegisterJob("tmp-token", JobCredential{Repo: testRepo}) base := handler.ExternalURL() + apiPath req, err := http.NewRequest(http.MethodGet, base+"/cache?keys=x&version=y", nil) @@ -920,8 +920,8 @@ func TestHandler_CrossRepoIsolation(t *testing.T) { handler, err := StartHandler(dir, "", 0, "", nil) require.NoError(t, err) defer handler.Close() - handler.RegisterJob("token-a", "owner/repoA") - handler.RegisterJob("token-b", "owner/repoB") + handler.RegisterJob("token-a", JobCredential{Repo: "owner/repoA"}) + handler.RegisterJob("token-b", JobCredential{Repo: "owner/repoB"}) base := handler.ExternalURL() + apiPath key := "shared-key" @@ -986,7 +986,7 @@ func TestHandler_ArtifactSignature(t *testing.T) { handler, err := StartHandler(dir, "", 0, "", nil) require.NoError(t, err) defer handler.Close() - handler.RegisterJob(testToken, testRepo) + handler.RegisterJob(testToken, JobCredential{Repo: testRepo}) base := handler.ExternalURL() + apiPath @@ -1059,7 +1059,7 @@ func TestHandler_ArtifactSignatureDownload(t *testing.T) { handler, err := StartHandler(dir, "", 0, "", nil) require.NoError(t, err) defer handler.Close() - handler.RegisterJob(testToken, testRepo) + handler.RegisterJob(testToken, JobCredential{Repo: testRepo}) base := handler.ExternalURL() + apiPath key := "download-key" @@ -1100,8 +1100,8 @@ func TestHandler_RegisterJob_RefCounted(t *testing.T) { require.NoError(t, err) defer handler.Close() - first := handler.RegisterJob("shared", testRepo) - second := handler.RegisterJob("shared", testRepo) + first := handler.RegisterJob("shared", JobCredential{Repo: testRepo}) + second := handler.RegisterJob("shared", JobCredential{Repo: testRepo}) base := handler.ExternalURL() + apiPath probe := func() int { @@ -1131,8 +1131,8 @@ func TestHandler_GC_PerRepoDedup(t *testing.T) { handler, err := StartHandler(dir, "", 0, "", nil) require.NoError(t, err) defer handler.Close() - handler.RegisterJob("tok-a", "owner/repoA") - handler.RegisterJob("tok-b", "owner/repoB") + handler.RegisterJob("tok-a", JobCredential{Repo: "owner/repoA"}) + handler.RegisterJob("tok-b", JobCredential{Repo: "owner/repoB"}) key := "shared-dedup-key" version := "c19da02a2bd7e77277f1ac29ab45c09b7d46a4ee758284e26bb3045ad11d9d20" diff --git a/act/artifactcache/handler_v2.go b/act/artifactcache/handler_v2.go index 848df7e0..6c69d4e3 100644 --- a/act/artifactcache/handler_v2.go +++ b/act/artifactcache/handler_v2.go @@ -23,6 +23,9 @@ import ( // endpoints, and uploads the archive to the returned URL with the Azure blob protocol. // Both API versions are served from the same store, so a repository keeps its cache // when a workflow moves between action versions. +// +// Responses carry the proto field names, which is what Gitea's own results API emits and the only +// spelling the Go clients parse. The JavaScript toolkit accepts either. const ( cacheServiceV2Path = "/twirp/github.actions.results.api.v1.CacheService" @@ -93,8 +96,8 @@ func (h *Handler) v2CreateCacheEntry(w http.ResponseWriter, r *http.Request, _ h } h.responseJSON(w, r, http.StatusOK, map[string]any{ - "ok": true, - "signedUploadUrl": h.signedURL(blobPath, blobUploadPurpose, cache.ID, time.Now().Add(blobUploadURLTTL)), + "ok": true, + "signed_upload_url": h.signedURL(blobPath, blobUploadPurpose, cache.ID, time.Now().Add(blobUploadURLTTL)), }) } @@ -134,7 +137,7 @@ func (h *Handler) v2FinalizeCacheEntryUpload(w http.ResponseWriter, r *http.Requ h.responseJSON(w, r, http.StatusOK, map[string]any{ "ok": true, // int64 fields travel as strings in the proto JSON mapping. - "entryId": strconv.FormatUint(cache.ID, 10), + "entry_id": strconv.FormatUint(cache.ID, 10), }) } @@ -164,9 +167,9 @@ func (h *Handler) v2GetCacheEntryDownloadURL(w http.ResponseWriter, r *http.Requ } h.responseJSON(w, r, http.StatusOK, map[string]any{ - "ok": true, - "signedDownloadUrl": h.signedArtifactURL(cache.ID, time.Now().Add(artifactURLTTL)), - "matchedKey": cache.Key, + "ok": true, + "signed_download_url": h.signedArtifactURL(cache.ID, time.Now().Add(artifactURLTTL)), + "matched_key": cache.Key, }) } @@ -209,6 +212,8 @@ func (h *Handler) v2UploadBlob(w http.ResponseWriter, r *http.Request, params ht return } + // The Azure SDK client dereferences this without checking, so its absence panics the caller. + w.Header().Set("x-ms-request-id", strconv.FormatInt(time.Now().UnixNano(), 10)) w.WriteHeader(http.StatusCreated) } diff --git a/act/artifactcache/handler_v2_test.go b/act/artifactcache/handler_v2_test.go index 19e393ed..86e7d7a7 100644 --- a/act/artifactcache/handler_v2_test.go +++ b/act/artifactcache/handler_v2_test.go @@ -45,6 +45,11 @@ func putBlob(t *testing.T, url string, content []byte) int { resp, err := http.DefaultClient.Do(req) require.NoError(t, err) defer resp.Body.Close() + if resp.StatusCode == http.StatusCreated { + // The Azure SDK client dereferences this header without checking, so a blob upload that + // omits it panics the caller rather than failing it. + require.NotEmpty(t, resp.Header.Get("x-ms-request-id")) + } return resp.StatusCode } @@ -67,7 +72,7 @@ func startTestHandler(t *testing.T) *Handler { handler, err := StartHandler(filepath.Join(t.TempDir(), "artifactcache"), "127.0.0.1", 0, "", nil) require.NoError(t, err) t.Cleanup(func() { _ = handler.Close() }) - handler.RegisterJob(testToken, testRepo) + handler.RegisterJob(testToken, JobCredential{Repo: testRepo}) return handler } @@ -78,7 +83,7 @@ func saveV2(t *testing.T, handler *Handler, key, version string, content []byte) created := v2Call(t, handler, testClient, "CreateCacheEntry", map[string]any{"key": key, "version": version}) require.Equal(t, true, created["ok"]) - uploadURL, _ = created["signedUploadUrl"].(string) + uploadURL, _ = created["signed_upload_url"].(string) require.NotEmpty(t, uploadURL) require.Equal(t, http.StatusCreated, putBlob(t, uploadURL, content)) @@ -100,7 +105,7 @@ func TestCacheServiceV2RoundTrip(t *testing.T) { finalized, uploadURL := saveV2(t, handler, "deps-v1", "abc123", content) require.Equal(t, true, finalized["ok"]) - assert.NotEmpty(t, finalized["entryId"]) + assert.NotEmpty(t, finalized["entry_id"]) // The upload URL outlives the finalize call, so replaying it must not poison the entry, // and it is an upload URL only: nothing reads a blob back through it. @@ -112,8 +117,8 @@ func TestCacheServiceV2RoundTrip(t *testing.T) { got := v2Call(t, handler, testClient, "GetCacheEntryDownloadURL", map[string]any{"key": "deps-v1", "version": "abc123"}) require.Equal(t, true, got["ok"]) - assert.Equal(t, "deps-v1", got["matchedKey"]) - downloadURL, _ := got["signedDownloadUrl"].(string) + assert.Equal(t, "deps-v1", got["matched_key"]) + downloadURL, _ := got["signed_download_url"].(string) require.NotEmpty(t, downloadURL) assert.Equal(t, content, getURL(t, downloadURL)) } @@ -124,7 +129,7 @@ func TestCacheServiceV2BlockUpload(t *testing.T) { handler := startTestHandler(t) created := v2Call(t, handler, testClient, "CreateCacheEntry", map[string]any{"key": "blocks", "version": "v1"}) - uploadURL, _ := created["signedUploadUrl"].(string) + uploadURL, _ := created["signed_upload_url"].(string) require.NotEmpty(t, uploadURL) blocks := map[string][]byte{} @@ -154,7 +159,7 @@ func TestCacheServiceV2BlockUpload(t *testing.T) { got := v2Call(t, handler, testClient, "GetCacheEntryDownloadURL", map[string]any{"key": "blocks", "version": "v1"}) require.Equal(t, true, got["ok"]) - assert.Equal(t, "hello world!", string(getURL(t, got["signedDownloadUrl"].(string)))) + assert.Equal(t, "hello world!", string(getURL(t, got["signed_download_url"].(string)))) } func TestCacheServiceV2Lookups(t *testing.T) { @@ -175,7 +180,7 @@ func TestCacheServiceV2Lookups(t *testing.T) { "key": "deps-zzz", field: []string{"deps-"}, "version": "v1", }) require.Equal(t, true, got["ok"]) - assert.Equal(t, "deps-abc", got["matchedKey"]) + assert.Equal(t, "deps-abc", got["matched_key"]) }) } @@ -190,7 +195,7 @@ func TestCacheServiceV2Lookups(t *testing.T) { t.Run("a prefix of an existing key is still reserved", func(t *testing.T) { reserved := v2Call(t, handler, testClient, "CreateCacheEntry", map[string]any{"key": "deps", "version": "v1"}) require.Equal(t, true, reserved["ok"]) - assert.NotEmpty(t, reserved["signedUploadUrl"]) + assert.NotEmpty(t, reserved["signed_upload_url"]) }) t.Run("finalizing without a reservation is not ok", func(t *testing.T) { @@ -203,7 +208,7 @@ func TestCacheServiceV2Lookups(t *testing.T) { // The size the client declares is what Commit validates the assembled archive against. t.Run("finalizing with the wrong size is not ok", func(t *testing.T) { created := v2Call(t, handler, testClient, "CreateCacheEntry", map[string]any{"key": "wrong-size", "version": "v1"}) - require.Equal(t, http.StatusCreated, putBlob(t, created["signedUploadUrl"].(string), []byte("four"))) + require.Equal(t, http.StatusCreated, putBlob(t, created["signed_upload_url"].(string), []byte("four"))) got := v2Call(t, handler, testClient, "FinalizeCacheEntryUpload", map[string]any{ "key": "wrong-size", "version": "v1", "size_bytes": 99, @@ -227,7 +232,7 @@ func TestCacheServiceV2Lookups(t *testing.T) { // The cache of one repository must stay invisible to another, as it does for the v1 API. t.Run("another repository sees nothing", func(t *testing.T) { - handler.RegisterJob("other-runtime-token", "other/repo") + handler.RegisterJob("other-runtime-token", JobCredential{Repo: "other/repo"}) otherClient := &http.Client{Transport: &bearerTransport{token: "other-runtime-token"}} got := v2Call(t, handler, otherClient, "GetCacheEntryDownloadURL", map[string]any{"key": "deps-abc", "version": "v1"}) diff --git a/act/artifactcache/results.go b/act/artifactcache/results.go new file mode 100644 index 00000000..409e0e04 --- /dev/null +++ b/act/artifactcache/results.go @@ -0,0 +1,59 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package artifactcache + +import ( + "crypto/tls" + "net/http" + "net/http/httputil" + "net/url" + "strings" +) + +// The results service is one origin serving every github.actions.results.api.v1 service, and +// Gitea implements only the artifact half of it. Forwarding that half from here makes this origin +// the whole service, so ACTIONS_RESULTS_URL can point at it truthfully, which is what the clients +// this runner cannot patch need, docker buildx among them. +// +// The instance to forward to travels with the job registration rather than with configuration, so +// a cache server shared between runners serves each of their instances. +const artifactServicePath = "/twirp/github.actions.results.api.v1.ArtifactService/" + +// forwardOrNotFound is the router's fallback: the artifact service of the instance the job +// registered with, and the 404 the router would have written otherwise. +func (h *Handler) forwardOrNotFound(w http.ResponseWriter, r *http.Request) { + cred, ok := h.lookupCredential(bearerToken(r)) + if !ok || cred.Results == "" || !strings.HasPrefix(r.URL.Path, artifactServicePath) { + http.NotFound(w, r) + return + } + target, err := url.Parse(strings.TrimSuffix(cred.Results, "/")) + if err != nil { + h.logger.Errorf("artifact service forward to %q: %v", cred.Results, err) + w.WriteHeader(http.StatusBadGateway) + return + } + h.logger.Debugf("%s %s: forwarding to %s", r.Method, r.URL.Path, target) + + proxy := &httputil.ReverseProxy{ + Rewrite: func(r *httputil.ProxyRequest) { + r.SetURL(target) + // Gitea builds the URLs it hands back from this Host, and their scheme from the + // connection unless a forwarded header overrides it, so artifact bodies go to Gitea + // directly and never through here. + r.Out.Host = target.Host + }, + ErrorHandler: func(w http.ResponseWriter, _ *http.Request, err error) { + h.logger.Warnf("artifact service forward to %s: %v", target, err) + w.WriteHeader(http.StatusBadGateway) + }, + } + if cred.InsecureTLS { + proxy.Transport = insecureTransport + } + proxy.ServeHTTP(w, r) +} + +// insecureTransport is shared, because a transport per request would pool no connections. +var insecureTransport = &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}} //nolint:gosec // the runner reaches its instance on the operator's say-so diff --git a/act/artifactcache/results_test.go b/act/artifactcache/results_test.go new file mode 100644 index 00000000..efbc439a --- /dev/null +++ b/act/artifactcache/results_test.go @@ -0,0 +1,56 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package artifactcache + +import ( + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The artifact half is forwarded under the Host Gitea knows itself by, so the URLs it hands back +// still point at Gitea, and nothing else is proxied. +func TestFrontResultsService(t *testing.T) { + var gotHost, gotPath, gotProto string + gitea := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotHost, gotPath, gotProto = r.Host, r.URL.Path, r.Header.Get("X-Forwarded-Proto") + _, _ = io.WriteString(w, `{"ok":true}`) + })) + defer gitea.Close() + + handler, err := StartHandler(t.TempDir(), "127.0.0.1", 0, "", nil) + require.NoError(t, err) + defer handler.Close() + const token = "forward-token" + + client := &http.Client{Transport: &bearerTransport{token: token}} + post := func(path string) int { + req, err := http.NewRequestWithContext(t.Context(), http.MethodPost, handler.ExternalURL()+path, nil) + require.NoError(t, err) + resp, err := client.Do(req) + require.NoError(t, err) + resp.Body.Close() + return resp.StatusCode + } + + assert.Equal(t, http.StatusNotFound, post(artifactServicePath+"CreateArtifact"), + "an unregistered token is forwarded nowhere") + + defer handler.RegisterJob(token, JobCredential{Repo: "owner/repo", Results: gitea.URL})() + + assert.Equal(t, http.StatusOK, post(artifactServicePath+"CreateArtifact")) + assert.Equal(t, strings.TrimPrefix(gitea.URL, "http://"), gotHost, "Gitea must see the host it mints its URLs from") + assert.Empty(t, gotProto, "a forwarded scheme would make an https Gitea mint http URLs") + assert.Equal(t, artifactServicePath+"CreateArtifact", gotPath) + + gotPath = "" + assert.Equal(t, http.StatusNotFound, post("/twirp/github.actions.results.api.v1.OtherService/Do")) + assert.Equal(t, http.StatusNotFound, post("/api/v1/repos/owner/repo")) + assert.Empty(t, gotPath, "only the artifact service is forwarded") +} diff --git a/act/runner/runner.go b/act/runner/runner.go index e38e8dea..b92da463 100644 --- a/act/runner/runner.go +++ b/act/runner/runner.go @@ -74,6 +74,7 @@ type Config struct { ContainerNetworkCreateOptions container.NewDockerNetworkCreateExecutorInput // the default network create options ActionCache ActionCache // Use a custom ActionCache Implementation ProxyEnv map[string]string // the proxy variables the job runs with, also given to service containers and image builds + PatchToolkit bool // edit the @actions toolkit bundled into an action so it works against Gitea, see toolkit_patch.go PresetGitHubContext *model.GithubContext // the preset github context, overrides some fields like DefaultBranch, Env, Secrets etc. EventJSON string // the content of JSON file to use for event.json in containers, overrides EventPath diff --git a/act/runner/step_action_remote.go b/act/runner/step_action_remote.go index d144049a..f063244c 100644 --- a/act/runner/step_action_remote.go +++ b/act/runner/step_action_remote.go @@ -236,10 +236,10 @@ func (sar *stepActionRemote) toolkitBundles() (string, []string) { return dir, actionScriptPaths(filepath.Join(dir, sar.remoteAction.Path), sar.action) } -// patchActionToolkit edits the bundled toolkit so it works against Gitea, which lets the cache -// client use the v2 API this runner serves. A no-op unless the runner serves it. +// patchActionToolkit edits the bundled toolkit so it works against Gitea: the artifact actions +// stop refusing, and the cache client keeps to the cache server whichever API version it picks. func (sar *stepActionRemote) patchActionToolkit(ctx context.Context) error { - if sar.RunContext.GetEnv()[CacheServiceV2Env] != "" { + if sar.RunContext.Config.PatchToolkit { dir, scripts := sar.toolkitBundles() patchToolkit(ctx, dir, scripts) } diff --git a/act/runner/toolkit_patch_e2e_test.go b/act/runner/toolkit_patch_e2e_test.go index 9e12fc44..c77cb43b 100644 --- a/act/runner/toolkit_patch_e2e_test.go +++ b/act/runner/toolkit_patch_e2e_test.go @@ -5,11 +5,15 @@ package runner import ( "context" + "encoding/base64" "io" "net/http" + "net/http/httptest" "os" "os/exec" + "path" "path/filepath" + "strconv" "strings" "testing" "time" @@ -59,41 +63,65 @@ func bundleFromGitHub(t *testing.T, repo, ref, path string) string { return bundle } -// runCacheAction runs one entrypoint the way a job would: a real Gitea server URL, and a results -// URL that points at Gitea rather than at the runner. Nothing about the environment is rewritten, -// so only the patch can make the client choose v2 and find the cache server. -func runCacheAction(t *testing.T, script, workspace, runnerTemp, cacheURL, token, key string) string { +// jobEnv is the environment a job gets from this runner, which is what decides where an action's +// toolkit looks for the cache and artifact services. +type jobEnv struct { + workspace, runnerTemp string + cacheURL, resultsURL string + token string +} + +// runActionEntrypoint runs one action entrypoint the way a job would. Adding another action to these tests +// means downloading its entrypoint with bundleFromGitHub and calling this with its inputs, whose +// names are the ones the action's own action.yml uses. +func runActionEntrypoint(t *testing.T, script string, env jobEnv, inputs map[string]string) string { t.Helper() - state := filepath.Join(runnerTemp, "state") - output := filepath.Join(runnerTemp, "output") + state := filepath.Join(env.runnerTemp, "state") + output := filepath.Join(env.runnerTemp, "output") for _, name := range []string{state, output} { require.NoError(t, os.WriteFile(name, nil, 0o600)) } cmd := exec.CommandContext(t.Context(), "node", script) - cmd.Dir = workspace + cmd.Dir = env.workspace cmd.Env = append(os.Environ(), - "INPUT_PATH=to-cache", - "INPUT_KEY="+key, - "ACTIONS_RUNTIME_TOKEN="+token, - "ACTIONS_CACHE_URL="+cacheURL+"/", - // Unreachable on purpose: the artifact service lives here, the cache service must not. - "ACTIONS_RESULTS_URL=https://gitea.example", + "ACTIONS_RUNTIME_TOKEN="+env.token, + "ACTIONS_CACHE_URL="+env.cacheURL+"/", + "ACTIONS_RESULTS_URL="+env.resultsURL, "ACTIONS_CACHE_SERVICE_V2=true", "GITHUB_SERVER_URL=https://gitea.example.com", + "GITHUB_REPOSITORY=testuser/testrepo", + "GITHUB_RUN_ID=1", "GITHUB_REF=refs/heads/main", "GITHUB_EVENT_NAME=push", - "GITHUB_WORKSPACE="+workspace, - "RUNNER_TEMP="+runnerTemp, + "GITHUB_WORKSPACE="+env.workspace, + "RUNNER_TEMP="+env.runnerTemp, "GITHUB_STATE="+state, "GITHUB_OUTPUT="+output, ) + for name, value := range inputs { + cmd.Env = append(cmd.Env, "INPUT_"+strings.ToUpper(name)+"="+value) + } out, err := cmd.CombinedOutput() - t.Logf("%s:\n%s", filepath.Base(filepath.Dir(script)), out) + t.Logf("%s:\n%s", filepath.Base(script), out) require.NoError(t, err, "%s failed", script) return string(out) } +// patchedAction downloads one entrypoint and patches it exactly as a downloaded action would be, +// keeping the untouched original in the sidecar beside it. +func patchedAction(t *testing.T, repo, ref, entrypoint string) string { + t.Helper() + + body, err := os.ReadFile(bundleFromGitHub(t, repo, ref, entrypoint)) + require.NoError(t, err) + dir := tempDirPath(t) + script := filepath.Join(dir, filepath.Base(entrypoint)) + require.NoError(t, os.WriteFile(script, body, 0o600)) + patchToolkit(t.Context(), dir, []string{script}) + return script +} + // tempDirPath is TempDir with symlinks resolved, because macOS hands out /var paths that resolve // to /private/var and the client derives archive paths relative to the workspace. func tempDirPath(t *testing.T) string { @@ -113,43 +141,52 @@ func tempDirPath(t *testing.T) string { func TestCacheServiceV2EndToEnd(t *testing.T) { requireHostTools(t, "node") - // A stand-in action directory, patched exactly as a downloaded one would be. - actionDir := tempDirPath(t) - scripts := map[string]string{} - for _, stage := range []string{"restore", "save"} { - body, err := os.ReadFile(bundleFromGitHub(t, "actions/cache", actionsCacheRef, "dist/"+stage+"/index.js")) - require.NoError(t, err) - scripts[stage] = filepath.Join(actionDir, stage+".js") - require.NoError(t, os.WriteFile(scripts[stage], body, 0o600)) - } - patchToolkit(t.Context(), actionDir, []string{scripts["restore"], scripts["save"]}) + restore := patchedAction(t, "actions/cache", actionsCacheRef, "dist/restore/index.js") + save := patchedAction(t, "actions/cache", actionsCacheRef, "dist/save/index.js") handler, err := artifactcache.StartHandler(filepath.Join(t.TempDir(), "cache"), "127.0.0.1", 0, "", nil) require.NoError(t, err) t.Cleanup(func() { _ = handler.Close() }) const token, repo = "e2e-runtime-token", "testuser/testrepo" - handler.RegisterJob(token, repo) + handler.RegisterJob(token, artifactcache.JobCredential{Repo: repo}) - workspace, runnerTemp := tempDirPath(t), tempDirPath(t) - require.NoError(t, os.MkdirAll(filepath.Join(workspace, "to-cache"), 0o755)) + env := jobEnv{ + workspace: tempDirPath(t), + runnerTemp: tempDirPath(t), + cacheURL: handler.ExternalURL(), + // The results service is the cache server's too, which is what the runner advertises. + resultsURL: handler.ExternalURL(), + token: token, + } + require.NoError(t, os.MkdirAll(filepath.Join(env.workspace, "to-cache"), 0o755)) content := []byte("cached through the patched gate") - require.NoError(t, os.WriteFile(filepath.Join(workspace, "to-cache", "data.txt"), content, 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(env.workspace, "to-cache", "data.txt"), content, 0o600)) const key = "patched-gate-key" - missed := runCacheAction(t, scripts["restore"], workspace, runnerTemp, handler.ExternalURL(), token, key) + inputs := map[string]string{"path": "to-cache", "key": key} + + missed := runActionEntrypoint(t, restore, env, inputs) require.Contains(t, missed, "Cache service version: v2", "the patch did not take, the client stayed on v1") require.Contains(t, missed, "Cache not found for input keys: "+key) - saved := runCacheAction(t, scripts["save"], workspace, runnerTemp, handler.ExternalURL(), token, key) + saved := runActionEntrypoint(t, save, env, inputs) require.Contains(t, saved, "Cache saved with key: "+key) - restored := tempDirPath(t) - hit := runCacheAction(t, scripts["restore"], restored, runnerTemp, handler.ExternalURL(), token, key) + env.workspace = tempDirPath(t) + hit := runActionEntrypoint(t, restore, env, inputs) require.Contains(t, hit, "Cache restored from key: "+key) - got, err := os.ReadFile(filepath.Join(restored, "to-cache", "data.txt")) + got, err := os.ReadFile(filepath.Join(env.workspace, "to-cache", "data.txt")) require.NoError(t, err) assert.Equal(t, content, got) + + // Untouched, the same client takes a Gitea host for GHES and stays on v1, which reaches the + // cache server on its own address. That is what a runner without a results service of its own + // leaves its jobs with, so it has to round trip too. + env.workspace = tempDirPath(t) + v1 := runActionEntrypoint(t, filepath.Join(sidecarDir(filepath.Dir(restore)), "index.js"), env, inputs) + require.Contains(t, v1, "Cache service version: v1") + require.Contains(t, v1, "Cache restored from key: "+key) } // The gate and the URL getter are separate functions, and a bundler may put either first: the gap @@ -158,19 +195,32 @@ func TestCacheServiceV2EndToEnd(t *testing.T) { // toolkit, patched but not run, is what keeps a future release from quietly matching only one of // the two shapes and leaving every cache on v1. func TestToolkitPatchAcrossActions(t *testing.T) { - for _, tc := range []struct{ repo, ref, path string }{ - {"actions/setup-go", "v7.0.0", "dist/setup/index.js"}, - {"actions/setup-node", "v6.0.0", "dist/cache-save/index.js"}, - {"actions/setup-python", "v6.0.0", "dist/setup/index.js"}, - {"ruby/setup-ruby", "v1.271.0", "dist/index.js"}, - {"pnpm/action-setup", "v6.0.9", "dist/index.js"}, + for _, tc := range []struct { + repo, ref, path string + wantPatched bool + }{ + // The cache toolkit, in each bundler shape and from a spread of ecosystems, including the + // actions that drive a Go or a Rust cache client of their own. + {"actions/cache", actionsCacheRef, "dist/restore/index.js", true}, + {"actions/setup-node", "v7.0.0", "dist/cache-save/index.js", true}, + {"actions/setup-python", "v7.0.0", "dist/setup/index.js", true}, + {"actions/setup-go", "v7.0.0", "dist/setup/index.js", true}, + {"actions/setup-java", "v5.7.0", "dist/setup/index.js", true}, + {"ruby/setup-ruby", "v1.321.0", "dist/index.js", true}, + {"pnpm/action-setup", "v6.0.9", "dist/index.js", true}, + {"oven-sh/setup-bun", "v2.2.0", "dist/setup/index.js", true}, + {"Swatinem/rust-cache", "v2.9.1", "dist/restore/index.js", true}, + {"docker/build-push-action", "v7.3.0", "dist/index.cjs", true}, // The artifact toolkit, where the gate is a refusal and there is nothing to redirect. // v4.4.0 is the first release whose gate carries the localhost test this matches; the // releases before it refuse in a shape the runner leaves alone. - {"actions/upload-artifact", "v4.4.0", "dist/upload/index.js"}, - {"actions/upload-artifact", "v7.0.1", "dist/upload/index.js"}, - {"actions/download-artifact", "v6.0.0", "dist/index.js"}, - {"oven-sh/setup-bun", "v2.2.0", "dist/setup/index.js"}, + {"actions/upload-artifact", "v4.4.0", "dist/upload/index.js", true}, + {"actions/upload-artifact", "v7.0.1", "dist/upload/index.js", true}, + {"actions/download-artifact", "v8.0.1", "dist/index.js", true}, + // Neither toolkit's gate, so these have to come back byte for byte. sccache-action is the + // one that exports ACTIONS_CACHE_SERVICE_V2 itself, for the Rust client it installs. + {"actions/checkout", "v7.0.1", "dist/index.js", false}, + {"mozilla-actions/sccache-action", "v0.0.11", "dist/setup/index.js", false}, } { t.Run(tc.repo+"@"+tc.ref, func(t *testing.T) { t.Parallel() @@ -179,7 +229,11 @@ func TestToolkitPatchAcrossActions(t *testing.T) { require.NoError(t, err) out, patched := patchedBundle(data) - assert.True(t, patched, "the version gate was not patched") + require.Equal(t, tc.wantPatched, patched) + if !tc.wantPatched { + assert.Equal(t, data, out, "an untouched bundle must come back byte for byte") + return + } assert.NotContains(t, string(out), ".LOCALHOST", "a copy of the gate was missed") if !strings.Contains(string(data), CacheServiceV2Env) { @@ -194,3 +248,121 @@ func TestToolkitPatchAcrossActions(t *testing.T) { }) } } + +// The stock artifact actions refuse on a Gitea host until the gate is opened, and then they talk +// to the results service, which is this runner's cache server forwarding the artifact half on to +// Gitea. Running the real upload-artifact against a stand-in Gitea covers both halves at once: +// the patch, and the forwarding the job's registration set up. +func TestUploadArtifactThroughTheResultsService(t *testing.T) { + requireHostTools(t, "node") + + var called []string + var zipped []byte + gitea := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + method := path.Base(r.URL.Path) + called = append(called, method) + w.Header().Set("x-ms-request-id", "stub") + switch method { + case "CreateArtifact": + _, _ = io.WriteString(w, `{"ok":true,"signed_upload_url":"http://`+r.Host+ + `/twirp/github.actions.results.api.v1.ArtifactService/UploadArtifact?sig=x"}`) + case "FinalizeArtifact": + _, _ = io.WriteString(w, `{"ok":true,"artifact_id":"1"}`) + case "ListArtifacts": + _, _ = io.WriteString(w, `{"artifacts":[{"workflow_run_backend_id":"11",`+ + `"workflow_job_run_backend_id":"22","database_id":"1","name":"an-artifact","size":"`+ + strconv.Itoa(len(zipped))+`"}]}`) + case "GetSignedArtifactURL": + _, _ = io.WriteString(w, `{"signed_url":"http://`+r.Host+`/download"}`) + case "download": + w.Header().Set("Content-Type", "application/zip") + _, _ = w.Write(zipped) + default: // the zip on its way up, in the blocks the Azure protocol puts it in + body, _ := io.ReadAll(r.Body) + switch r.URL.Query().Get("comp") { + case "block": + zipped = append(zipped, body...) + case "blocklist": // the ordering document, not content + default: + zipped = body + } + w.WriteHeader(http.StatusCreated) + } + })) + defer gitea.Close() + + handler, err := artifactcache.StartHandler(filepath.Join(t.TempDir(), "cache"), "127.0.0.1", 0, "", nil) + require.NoError(t, err) + t.Cleanup(func() { _ = handler.Close() }) + // The artifact client decodes the runtime token for the run ids it puts in its requests, where + // the cache client only presents it, so this one has to be shaped like Gitea's. + token := "e30." + base64.RawURLEncoding.EncodeToString([]byte(`{"scp":"Actions.Results:11:22"}`)) + ".sig" + defer handler.RegisterJob(token, artifactcache.JobCredential{Repo: "testuser/testrepo", Results: gitea.URL})() + + upload := patchedAction(t, "actions/upload-artifact", "v7.0.1", "dist/upload/index.js") + + env := jobEnv{ + workspace: tempDirPath(t), + runnerTemp: tempDirPath(t), + cacheURL: handler.ExternalURL(), + resultsURL: handler.ExternalURL(), + token: token, + } + uploaded := []byte("through the results service") + require.NoError(t, os.WriteFile(filepath.Join(env.workspace, "artifact.txt"), uploaded, 0o600)) + + out := runActionEntrypoint(t, upload, env, map[string]string{ + "name": "an-artifact", "path": "artifact.txt", "if-no-files-found": "error", + "retention-days": "0", "compression-level": "6", "overwrite": "false", + "include-hidden-files": "false", "archive": "true", + }) + + require.Contains(t, out, "has been successfully uploaded") + + // And back down again: listing and downloading go the same way, and the signed URL the + // artifact service hands out is fetched straight from it. + download := patchedAction(t, "actions/download-artifact", "v8.0.1", "dist/index.js") + env.workspace = tempDirPath(t) + out = runActionEntrypoint(t, download, env, map[string]string{ + "name": "an-artifact", "path": "downloaded", "merge-multiple": "false", + "skip-decompress": "false", "include-hidden-files": "false", "github-token": "", + }) + + require.Contains(t, out, "Artifact download completed") + assert.Subset(t, called, + []string{"CreateArtifact", "UploadArtifact", "FinalizeArtifact", "ListArtifacts", "GetSignedArtifactURL"}, + "the artifact service was not reached through the cache server") + got, err := os.ReadFile(filepath.Join(env.workspace, "downloaded", "artifact.txt")) + require.NoError(t, err) + assert.Equal(t, uploaded, got) +} + +// The setup actions carry the same toolkit and reach the same service, from a key of their own +// making. setup-node is the cheapest of them to run: given a lockfile and no version to install, +// it does the cache lookup and nothing else. +func TestSetupActionFindsTheCacheService(t *testing.T) { + requireHostTools(t, "node", "npm") + + setup := patchedAction(t, "actions/setup-node", "v7.0.0", "dist/setup/index.js") + + handler, err := artifactcache.StartHandler(filepath.Join(t.TempDir(), "cache"), "127.0.0.1", 0, "", nil) + require.NoError(t, err) + t.Cleanup(func() { _ = handler.Close() }) + const token = "setup-runtime-token" + defer handler.RegisterJob(token, artifactcache.JobCredential{Repo: "testuser/testrepo"})() + + env := jobEnv{ + workspace: tempDirPath(t), + runnerTemp: tempDirPath(t), + cacheURL: handler.ExternalURL(), + resultsURL: handler.ExternalURL(), + token: token, + } + require.NoError(t, os.WriteFile(filepath.Join(env.workspace, "package-lock.json"), + []byte(`{"lockfileVersion":3}`), 0o600)) + + out := runActionEntrypoint(t, setup, env, map[string]string{"cache": "npm"}) + + require.Contains(t, out, "Cache service version: v2") + require.Contains(t, out, "npm cache is not found", "the lookup did not reach the cache server") +} diff --git a/act/runner/toolkit_patch_test.go b/act/runner/toolkit_patch_test.go index 2b62b6d5..e0e44268 100644 --- a/act/runner/toolkit_patch_test.go +++ b/act/runner/toolkit_patch_test.go @@ -283,7 +283,7 @@ func TestPatchBundleAfterTheActionMoved(t *testing.T) { // The wiring: a step patches its own bundles only when the runner serves the v2 API, and a step // that fails gets them back. The action's path inside its repository is part of where they live. func TestStepActionRemoteToolkitPatch(t *testing.T) { - newStep := func(t *testing.T, env map[string]string) (*stepActionRemote, string) { + newStep := func(t *testing.T, patch bool) (*stepActionRemote, string) { t.Helper() sar := &stepActionRemote{ @@ -291,8 +291,7 @@ func TestStepActionRemoteToolkitPatch(t *testing.T) { remoteAction: &remoteAction{Org: "owner", Repo: "repo", Path: "sub", Ref: "v1"}, action: &model.Action{Runs: model.ActionRuns{Using: "node20", Main: "index.js"}}, RunContext: &RunContext{ - Env: env, - Config: &Config{ActionCacheDir: t.TempDir()}, + Config: &Config{ActionCacheDir: t.TempDir(), PatchToolkit: patch}, }, } script := filepath.Join(sar.actionDir(), "sub", "index.js") @@ -301,8 +300,8 @@ func TestStepActionRemoteToolkitPatch(t *testing.T) { return sar, script } - t.Run("left alone when the runner does not serve the v2 API", func(t *testing.T) { - sar, script := newStep(t, map[string]string{}) + t.Run("left alone when the runner does not patch", func(t *testing.T) { + sar, script := newStep(t, false) require.NoError(t, sar.patchActionToolkit(t.Context())) body, err := os.ReadFile(script) @@ -311,7 +310,7 @@ func TestStepActionRemoteToolkitPatch(t *testing.T) { }) t.Run("patched, and put back when the step fails", func(t *testing.T) { - sar, script := newStep(t, map[string]string{CacheServiceV2Env: "true"}) + sar, script := newStep(t, true) require.NoError(t, sar.patchActionToolkit(t.Context())) body, err := os.ReadFile(script) diff --git a/internal/app/cmd/exec.go b/internal/app/cmd/exec.go index 4eb76cfc..5421b120 100644 --- a/internal/app/cmd/exec.go +++ b/internal/app/cmd/exec.go @@ -416,7 +416,7 @@ func runExec(ctx context.Context, execArgs *executeArgs) func(cmd *cobra.Command env[actionsRuntimeTokenEnvName] = actionsRuntimeToken os.Setenv(actionsRuntimeTokenEnvName, actionsRuntimeToken) } - handler.RegisterJob(actionsRuntimeToken, "__local/__exec") + handler.RegisterJob(actionsRuntimeToken, artifactcache.JobCredential{Repo: "__local/__exec"}) // no service aliases: exec builds one config for the whole plan run.BypassProxyForDockerHost(os.Getenv("DOCKER_HOST")) @@ -427,6 +427,7 @@ func runExec(ctx context.Context, execArgs *executeArgs) func(cmd *cobra.Command config := &runner.Config{ Workdir: execArgs.Workdir(), BindWorkdir: false, + PatchToolkit: true, // the cache server started above is what the patch points at ReuseContainers: false, ForcePull: execArgs.forcePull, ForceRebuild: execArgs.forceRebuild, diff --git a/internal/app/run/runner.go b/internal/app/run/runner.go index 4da04447..fe36263f 100644 --- a/internal/app/run/runner.go +++ b/internal/app/run/runner.go @@ -89,7 +89,8 @@ func NewRunner(cfg *config.Config, reg *config.Registration, cli client.Client) var cacheHandler *artifactcache.Handler if cfg.Cache.Enabled == nil || *cfg.Cache.Enabled { if cfg.Cache.ExternalServer != "" { - envs["ACTIONS_CACHE_URL"] = cfg.Cache.ExternalServer + // The v1 client appends its path to this without a separator, so the slash is required. + envs["ACTIONS_CACHE_URL"] = strings.TrimRight(cfg.Cache.ExternalServer, "/") + "/" } else { warnIgnoredCacheSecret(cfg) handler, err := artifactcache.StartHandler( @@ -109,12 +110,6 @@ func NewRunner(cfg *config.Config, reg *config.Registration, cli client.Client) } } - if envs["ACTIONS_CACHE_URL"] != "" && (cfg.Cache.V2 == nil || *cfg.Cache.V2) { - // act patches the GHES check out of an action's bundle when it sees this, so the client - // uses the cache service v2 API this server also answers; see act/runner/toolkit_patch.go. - envs[runner.CacheServiceV2Env] = "true" - } - // set artifact gitea api artifactGiteaAPI := strings.TrimSuffix(cli.Address(), "/") + "/api/actions_pipeline/" envs["ACTIONS_RUNTIME_URL"] = artifactGiteaAPI @@ -440,7 +435,13 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report. // lifetime. Only applies to the embedded cache server; when the operator // points the runner at an external cache via cfg.Cache.ExternalServer, it // is that server's responsibility to authenticate requests. - defer r.registerCacheForTask(giteaRuntimeToken, preset.Repository, reporter)() + revokeCache, resultsURL := r.registerCacheForTask(giteaRuntimeToken, preset.Repository, reporter) + defer revokeCache() + // A cache server that agreed to forward the artifact half is the whole results service, so + // the job is pointed at it and the v2 variable is finally true. + if resultsURL != "" { + envs["ACTIONS_RESULTS_URL"], envs[runner.CacheServiceV2Env] = resultsURL, "true" + } eventJSON, err := json.Marshal(preset.Event) if err != nil { @@ -481,6 +482,7 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report. AllocatePTY: r.cfg.Runner.AllocatePTY, ActionOfflineMode: r.cfg.Cache.OfflineMode, ActionCloneDepth: actionCloneDepth, + PatchToolkit: r.patchToolkit(), ReuseContainers: false, ForcePull: r.cfg.Container.ForcePull, @@ -553,6 +555,12 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report. return execErr } +// patchToolkit reports whether act should edit the toolkit bundled into an action. It follows the +// cache URL, because that is what the edits point the client at; see act/runner/toolkit_patch.go. +func (r *Runner) patchToolkit() bool { + return r.envs["ACTIONS_CACHE_URL"] != "" && (r.cfg.Cache.V2 == nil || *r.cfg.Cache.V2) +} + // registerCacheForTask tells the cache server to accept requests authenticated // with the given runtime token for the duration of this task. Returns a // function the caller must invoke (typically via defer) to revoke the @@ -565,18 +573,25 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report. // repo scoping over the network. // // Safe with an empty token (older Gitea did not issue one). -func (r *Runner) registerCacheForTask(token, repo string, reporter *report.Reporter) func() { +// It also returns the URL to advertise as ACTIONS_RESULTS_URL, which is the cache server itself +// when it agreed to forward this instance's artifact service, and "" when it did not. +func (r *Runner) registerCacheForTask(token, repo string, reporter *report.Reporter) (func(), string) { if token == "" { - return func() {} + return func() {}, "" + } + cred := artifactcache.JobCredential{ + Repo: repo, + Results: r.envs["ACTIONS_RESULTS_URL"], // the instance as the job would reach it + InsecureTLS: r.cfg.Runner.Insecure, } if r.cacheHandler != nil { - return r.cacheHandler.RegisterJob(token, repo) + return r.cacheHandler.RegisterJob(token, cred), r.cacheHandler.ResultsURL(cred) } if r.cfg.Cache.ExternalServer != "" && r.cfg.Cache.ExternalSecret != "" { - return r.registerExternalCacheJob(token, repo, reporter) + return r.registerExternalCacheJob(token, cred, reporter) } // No cache server to register against: caching is disabled, or the built-in server failed to start. - return func() {} + return func() {}, "" } // registerExternalCacheJob POSTs to the remote cache-server's control-plane. @@ -584,47 +599,54 @@ func (r *Runner) registerCacheForTask(token, repo string, reporter *report.Repor // 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 string, reporter *report.Reporter) func() { +func (r *Runner) registerExternalCacheJob(token string, cred artifactcache.JobCredential, reporter *report.Reporter) (func(), string) { base := strings.TrimRight(r.cfg.Cache.ExternalServer, "/") - if err := postInternalCache(base+"/_internal/register", r.cfg.Cache.ExternalSecret, - map[string]string{"token": token, "repo": repo}); err != nil { + resultsURL := "" + if body, err := postInternalCache(base+"/_internal/register", r.cfg.Cache.ExternalSecret, map[string]any{ + "token": token, "repo": cred.Repo, "results": cred.Results, "insecure_tls": cred.InsecureTLS, + }); err != nil { log.Warnf("cache external_server register failed (%s): %v", base, err) if reporter != nil { reporter.Logf("::warning::cache external_server register failed (%s): %v — cache requests from this job will be unauthenticated and likely return 401", base, err) } + } else { + resultsURL, _ = body["results_url"].(string) // absent from a server too old to forward } return func() { - if err := postInternalCache(base+"/_internal/revoke", r.cfg.Cache.ExternalSecret, - map[string]string{"token": token}); err != nil { + if _, err := postInternalCache(base+"/_internal/revoke", r.cfg.Cache.ExternalSecret, + map[string]any{"token": token}); err != nil { log.Warnf("cache external_server revoke failed (%s): %v", base, err) if reporter != nil { reporter.Logf("::warning::cache external_server revoke failed (%s): %v", base, err) } } - } + }, resultsURL } -func postInternalCache(url, secret string, body map[string]string) error { +func postInternalCache(url, secret string, body map[string]any) (map[string]any, error) { buf, err := json.Marshal(body) if err != nil { - return err + return nil, err } req, err := http.NewRequest(http.MethodPost, url, bytes.NewReader(buf)) if err != nil { - return err + return nil, err } req.Header.Set("Authorization", "Bearer "+secret) req.Header.Set("Content-Type", "application/json") client := &http.Client{Timeout: 5 * time.Second} resp, err := client.Do(req) if err != nil { - return err + return nil, err } defer resp.Body.Close() if resp.StatusCode/100 != 2 { - return fmt.Errorf("status %d", resp.StatusCode) + return nil, fmt.Errorf("status %d", resp.StatusCode) } - return nil + answer := map[string]any{} + // A server too old to answer with a body is not an error, it simply tells us nothing. + _ = json.NewDecoder(resp.Body).Decode(&answer) + return answer, nil } func (r *Runner) RunningCount() int64 { diff --git a/internal/app/run/runner_cache_test.go b/internal/app/run/runner_cache_test.go index 4a6440b7..083728b1 100644 --- a/internal/app/run/runner_cache_test.go +++ b/internal/app/run/runner_cache_test.go @@ -7,7 +7,9 @@ import ( "bytes" "encoding/json" "fmt" + "io" "net/http" + "net/http/httptest" "path/filepath" "strings" "testing" @@ -29,7 +31,7 @@ func TestRunner_registerCacheForTask(t *testing.T) { r := &Runner{cfg: emptyCfg(), cacheHandler: handler} token := "run-token-123" - unregister := r.registerCacheForTask(token, "owner/repo", nil) + unregister, _ := r.registerCacheForTask(token, "owner/repo", nil) base := handler.ExternalURL() + "/_apis/artifactcache" probe := func() int { @@ -53,7 +55,7 @@ func TestRunner_registerCacheForTask(t *testing.T) { func TestRunner_registerCacheForTask_NoOps(t *testing.T) { t.Run("nil cacheHandler", func(t *testing.T) { r := &Runner{cfg: emptyCfg()} - unregister := r.registerCacheForTask("tok", "owner/repo", nil) + unregister, _ := r.registerCacheForTask("tok", "owner/repo", nil) require.NotNil(t, unregister) unregister() }) @@ -65,7 +67,7 @@ func TestRunner_registerCacheForTask_NoOps(t *testing.T) { defer handler.Close() r := &Runner{cfg: emptyCfg(), cacheHandler: handler} - unregister := r.registerCacheForTask("", "owner/repo", nil) + unregister, _ := r.registerCacheForTask("", "owner/repo", nil) require.NotNil(t, unregister) unregister() }) @@ -81,7 +83,7 @@ func TestRunner_CacheFullFlow_MatchesToolkit(t *testing.T) { r := &Runner{cfg: emptyCfg(), cacheHandler: handler} token := "full-flow-token" - unregister := r.registerCacheForTask(token, "owner/repo", nil) + unregister, _ := r.registerCacheForTask(token, "owner/repo", nil) defer unregister() base := handler.ExternalURL() + "/_apis/artifactcache" @@ -154,18 +156,27 @@ func decodeJSON(resp *http.Response, v any) error { } // End-to-end against a remote cache-server: token unknown → 401, register → -// reserve/upload/commit/find/download all OK, revoke → 401 again. +// reserve/upload/commit/find/download all OK, revoke → 401 again. Registering also names the +// instance, and the server answering with its own address is what makes a shared cache server the +// whole results service, as the built-in one is. func TestRunner_ExternalCacheServer_RegisterRevoke(t *testing.T) { dir := filepath.Join(t.TempDir(), "remote-cache") const secret = "shared-secret-for-tests" remote, err := artifactcache.StartHandler(dir, "127.0.0.1", 0, secret, nil) require.NoError(t, err) defer remote.Close() + gitea := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = io.WriteString(w, `{"ok":true}`) + })) + defer gitea.Close() - r := &Runner{cfg: &config.Config{Cache: config.Cache{ - ExternalServer: remote.ExternalURL(), - ExternalSecret: secret, - }}} + r := &Runner{ + cfg: &config.Config{Cache: config.Cache{ + ExternalServer: remote.ExternalURL(), + ExternalSecret: secret, + }}, + envs: map[string]string{"ACTIONS_RESULTS_URL": gitea.URL}, + } token := "external-task-token" repo := "owner/repoX" @@ -182,10 +193,21 @@ func TestRunner_ExternalCacheServer_RegisterRevoke(t *testing.T) { require.Equal(t, http.StatusUnauthorized, probe(), "token must be unknown to the remote server before registration") - unregister := r.registerCacheForTask(token, repo, nil) + unregister, resultsURL := r.registerCacheForTask(token, repo, nil) require.NotEqual(t, http.StatusUnauthorized, probe(), "token must be accepted after registerCacheForTask") + // The server took the results service over, so the artifact half reaches Gitea through it. + require.Equal(t, remote.ExternalURL(), resultsURL) + artifact, err := http.NewRequestWithContext(t.Context(), http.MethodPost, + resultsURL+"/twirp/github.actions.results.api.v1.ArtifactService/CreateArtifact", nil) + require.NoError(t, err) + artifact.Header.Set("Authorization", "Bearer "+token) + forwarded, err := http.DefaultClient.Do(artifact) + require.NoError(t, err) + forwarded.Body.Close() + require.Equal(t, http.StatusOK, forwarded.StatusCode) + // Full reserve→upload→commit→find→download cycle, identical to what // @actions/cache does, against the remote (external) server. body := []byte("payload-from-task") diff --git a/internal/app/run/runner_test.go b/internal/app/run/runner_test.go index 80409f21..d262a3a2 100644 --- a/internal/app/run/runner_test.go +++ b/internal/app/run/runner_test.go @@ -5,6 +5,8 @@ package run import ( "context" + "net/http" + "strings" "testing" "gitea.com/gitea/runner/act/runner" @@ -124,21 +126,57 @@ func taskWithDefaultActionsURL(url string) *runnerv1.Task { } } -// The cache service v2 API is announced to jobs unless it is turned off. Announcing it is what -// makes act patch the GHES check out of an action's bundle, so the client can reach it. +// The results service is decided per task, because that is where the job's token and its +// instance are both known. NewRunner only leaves the address Gitea serves. func TestNewRunnerCacheServiceV2(t *testing.T) { - announced := func(v2 *bool) string { - cfg := &config.Config{} - cfg.Cache.V2, cfg.Cache.Dir, cfg.Cache.Host = v2, t.TempDir(), "127.0.0.1" - cli := clientmocks.NewClient(t) - cli.On("Address").Return("https://gitea.example/").Maybe() + cfg := &config.Config{} + cfg.Cache.Dir, cfg.Cache.Host = t.TempDir(), "127.0.0.1" + cli := clientmocks.NewClient(t) + cli.On("Address").Return("https://gitea.example/").Maybe() - r := NewRunner(cfg, &config.Registration{Name: "runner"}, cli) - t.Cleanup(func() { _ = r.Close() }) - return r.envs[runner.CacheServiceV2Env] - } + r := NewRunner(cfg, &config.Registration{Name: "runner"}, cli) + t.Cleanup(func() { _ = r.Close() }) + const token = "task-token" - off := false - assert.Equal(t, "true", announced(nil)) - assert.Empty(t, announced(&off)) + assert.Equal(t, "https://gitea.example", r.envs["ACTIONS_RESULTS_URL"]) + assert.Empty(t, r.envs[runner.CacheServiceV2Env], "a promise the runner has not made yet") + assert.True(t, r.patchToolkit()) + + // 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", nil) + defer revoke() + require.Equal(t, r.cacheHandler.ExternalURL(), resultsURL) + + // And what is advertised has to answer the cache service. A client without the GHES escape + // hatch, docker buildx among them, posts its cache calls at exactly this URL and nowhere else, + // so a 404 here is the failure this whole change is about. + req, err := http.NewRequestWithContext(t.Context(), http.MethodPost, + resultsURL+"/twirp/github.actions.results.api.v1.CacheService/GetCacheEntryDownloadURL", + strings.NewReader(`{"key":"k","version":"v"}`)) + require.NoError(t, err) + req.Header.Set("Authorization", "Bearer "+token) + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + assert.Equal(t, http.StatusOK, resp.StatusCode, "the advertised results service serves no cache service") +} + +// The v1 cache client appends its path to ACTIONS_CACHE_URL without a separator, so a configured +// server that is missing the slash would send it to a URL whose port swallows the path. +func TestNewRunnerNormalizesTheExternalCacheServer(t *testing.T) { + cfg := &config.Config{} + cfg.Cache.ExternalServer = "http://cache.local:8088//" + cli := clientmocks.NewClient(t) + cli.On("Address").Return("https://gitea.example/").Maybe() + + r := NewRunner(cfg, &config.Registration{Name: "runner"}, cli) + + assert.Equal(t, "http://cache.local:8088/", r.envs["ACTIONS_CACHE_URL"]) + // Nothing to front the results service with, so the variable stays unset, but the bundles are + // still patched: artifacts v4 need that, and the patch keeps the cache client on the cache URL. + assert.Equal(t, "https://gitea.example", r.envs["ACTIONS_RESULTS_URL"]) + assert.Empty(t, r.envs[runner.CacheServiceV2Env]) + assert.True(t, r.patchToolkit()) } diff --git a/internal/pkg/config/config.example.yaml b/internal/pkg/config/config.example.yaml index 2733f365..433dec29 100644 --- a/internal/pkg/config/config.example.yaml +++ b/internal/pkg/config/config.example.yaml @@ -138,7 +138,7 @@ cache: # Ignored when external_server is set. port: 0 # URL of a shared `gitea-runner cache-server` to use instead of starting a local one. - # Set on every runner that should share a cache pool. Must end with "/". + # Set on every runner that should share a cache pool. A trailing slash is optional. # Example: "http://cache-host:8088/" # Requires external_secret (below) to match the value on the cache-server. external_server: ""