fix(cache): build job URLs on the address its runner registered (#1153)

The cache server built every URL it hands a job from its own listen address, so jobs whose runner reaches it through a reverse proxy were sent to the internal one. This covered the v1 `archiveLocation`, the v2 signed cache URLs and `ACTIONS_RESULTS_URL`.

Runners now register the address their jobs reach the server at, next to the instance URL they already send. The cache-server needs no configuration of its own, and runners that reach it differently each get their own correct address.

Closes https://gitea.com/gitea/runner/issues/1152

---------

Co-authored-by: silverwind <[email protected]>
Reviewed-on: https://gitea.com/gitea/runner/pulls/1153
Reviewed-by: silverwind <[email protected]>
Reviewed-by: bircni <[email protected]>
Co-authored-by: Max P. <[email protected]>

act/artifactcache/handler.go

@@ -59,6 +59,9 @@ type JobCredential struct {
 	// remote runner registers with.
 	Results     string `json:"results"`
 	InsecureTLS bool   `json:"insecure_tls"`
+
+	// PublicURL is this server as a reverse proxy makes the job reach it, not the listen address.
+	PublicURL string `json:"public_url"`
 }
 
 // credEntry holds a registered job's credential along with an active
@@ -212,6 +215,13 @@ func (h *Handler) ExternalURL() string {
 	return fmt.Sprintf("http://%s:%d", h.outboundIP, h.port)
 }
 
+func (h *Handler) baseURL(cred JobCredential) string {
+	if base := strings.TrimRight(cred.PublicURL, "/"); base != "" {
+		return base
+	}
+	return h.ExternalURL()
+}
+
 // RegisterJob makes token a valid bearer credential for cache requests from
 // the given repository and returns a function that removes it. The runner
 // calls this at job start and defers the returned func so that the credential
@@ -359,7 +369,7 @@ func (h *Handler) find(w http.ResponseWriter, r *http.Request, _ httprouter.Para
 	}
 	h.responseJSON(w, r, 200, map[string]any{
 		"result":          "hit",
-		"archiveLocation": h.signedArtifactURL(cache.ID, time.Now().Add(artifactURLTTL)),
+		"archiveLocation": h.signedArtifactURL(cred, cache.ID, time.Now().Add(artifactURLTTL)),
 		"cacheKey":        cache.Key,
 	})
 }
@@ -641,7 +651,7 @@ func (h *Handler) ResultsURL(cred JobCredential) string {
 	if h == nil || cred.Results == "" {
 		return ""
 	}
-	return h.ExternalURL()
+	return h.baseURL(cred)
 }
 
 func (h *Handler) internalRegister(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
@@ -700,16 +710,16 @@ func (h *Handler) computeSignature(purpose string, cacheID, exp int64) string {
 }
 
 // signedURL builds a URL under path that signedAuth accepts for the same purpose.
-func (h *Handler) signedURL(path, purpose string, cacheID uint64, exp time.Time) string {
+func (h *Handler) signedURL(cred JobCredential, path, purpose string, cacheID uint64, exp time.Time) string {
 	expUnix := exp.Unix()
 	q := url.Values{}
 	q.Set("exp", strconv.FormatInt(expUnix, 10))
 	q.Set("sig", h.computeSignature(purpose, int64(cacheID), expUnix))
-	return fmt.Sprintf("%s%s/%d?%s", h.ExternalURL(), path, cacheID, q.Encode())
+	return fmt.Sprintf("%s%s/%d?%s", h.baseURL(cred), path, cacheID, q.Encode())
 }
 
-func (h *Handler) signedArtifactURL(cacheID uint64, exp time.Time) string {
-	return h.signedURL(apiPath+"/artifacts", "", cacheID, exp)
+func (h *Handler) signedArtifactURL(cred JobCredential, cacheID uint64, exp time.Time) string {
+	return h.signedURL(cred, apiPath+"/artifacts", "", cacheID, exp)
 }
 
 // if not found, return (nil, nil) instead of an error.

act/artifactcache/handler_test.go

@@ -45,7 +45,7 @@ var testClient = &http.Client{Transport: &bearerTransport{token: testToken}}
 // tests use it to reach the get handler directly without going through a
 // find/cache-hit round trip.
 func signArtifactURL(h *Handler, id int64) string {
-	return h.signedArtifactURL(uint64(id), time.Now().Add(artifactURLTTL))
+	return h.signedArtifactURL(JobCredential{}, uint64(id), time.Now().Add(artifactURLTTL))
 }
 
 func TestHandler(t *testing.T) {
@@ -998,7 +998,7 @@ func TestHandler_ArtifactSignature(t *testing.T) {
 	})
 
 	t.Run("tampered signature", func(t *testing.T) {
-		good := handler.signedArtifactURL(1, time.Now().Add(artifactURLTTL))
+		good := signArtifactURL(handler, 1)
 		bad := good[:len(good)-4] + "dead"
 		resp, err := testClient.Get(bad)
 		require.NoError(t, err)
@@ -1007,7 +1007,7 @@ func TestHandler_ArtifactSignature(t *testing.T) {
 	})
 
 	t.Run("expired signature", func(t *testing.T) {
-		expired := handler.signedArtifactURL(1, time.Now().Add(-time.Second))
+		expired := handler.signedArtifactURL(JobCredential{}, 1, time.Now().Add(-time.Second))
 		resp, err := testClient.Get(expired)
 		require.NoError(t, err)
 		resp.Body.Close()
@@ -1019,7 +1019,7 @@ func TestHandler_ArtifactSignature(t *testing.T) {
 		other, err := StartHandler(dir2, "", 0, "", nil)
 		require.NoError(t, err)
 		defer other.Close()
-		otherURL := other.signedArtifactURL(1, time.Now().Add(artifactURLTTL))
+		otherURL := signArtifactURL(other, 1)
 		// Rewrite the host so the request still lands on our handler, but
 		// the signature was computed with a different secret.
 		parts := strings.SplitN(otherURL, apiPath, 2)

act/artifactcache/handler_v2.go

@@ -97,7 +97,7 @@ func (h *Handler) v2CreateCacheEntry(w http.ResponseWriter, r *http.Request, _ h
 
 	h.responseJSON(w, r, http.StatusOK, map[string]any{
 		"ok":                true,
-		"signed_upload_url": h.signedURL(blobPath, blobUploadPurpose, cache.ID, time.Now().Add(blobUploadURLTTL)),
+		"signed_upload_url": h.signedURL(cred, blobPath, blobUploadPurpose, cache.ID, time.Now().Add(blobUploadURLTTL)),
 	})
 }
 
@@ -168,7 +168,7 @@ func (h *Handler) v2GetCacheEntryDownloadURL(w http.ResponseWriter, r *http.Requ
 
 	h.responseJSON(w, r, http.StatusOK, map[string]any{
 		"ok":                  true,
-		"signed_download_url": h.signedArtifactURL(cache.ID, time.Now().Add(artifactURLTTL)),
+		"signed_download_url": h.signedArtifactURL(cred, cache.ID, time.Now().Add(artifactURLTTL)),
 		"matched_key":         cache.Key,
 	})
 }

act/artifactcache/handler_v2_test.go

@@ -12,6 +12,7 @@ import (
 	"net/http"
 	"path/filepath"
 	"strconv"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -198,6 +199,18 @@ func TestCacheServiceV2Lookups(t *testing.T) {
 		assert.NotEmpty(t, reserved["signed_upload_url"])
 	})
 
+	t.Run("a proxied job is handed the address its runner registered", func(t *testing.T) {
+		const proxy = "https://cache.example.invalid"
+		handler.RegisterJob("proxied", JobCredential{Repo: testRepo, PublicURL: proxy + "/"})
+		client := &http.Client{Transport: &bearerTransport{token: "proxied"}}
+
+		created := v2Call(t, handler, client, "CreateCacheEntry", map[string]any{"key": "proxied-key", "version": "v1"})
+		assert.True(t, strings.HasPrefix(created["signed_upload_url"].(string), proxy+blobPath+"/"))
+
+		got := v2Call(t, handler, client, "GetCacheEntryDownloadURL", map[string]any{"key": "deps-abc", "version": "v1"})
+		assert.True(t, strings.HasPrefix(got["signed_download_url"].(string), proxy+apiPath+"/artifacts/"))
+	})
+
 	t.Run("finalizing without a reservation is not ok", func(t *testing.T) {
 		got := v2Call(t, handler, testClient, "FinalizeCacheEntryUpload", map[string]any{
 			"key": "never-reserved", "version": "v1", "size_bytes": 1,