fix: re-detect the cache address per job and prefer IPv4 (#1398)

The cache address was detected once at startup, and the interface fallback ranked ethernet names above IPv4. A runner that started before DHCPv4 finished could advertise a global IPv6 address to IPv4-only job networks until it restarted, failing every artifact and cache call.

IPv4 now ranks first, and the address is detected again for each job unless `cache.host` sets it. Detecting an IPv6 address logs a warning.

Fixes: https://gitea.com/gitea/runner/issues/1379
Co-authored-by: silverwind <[email protected]>
Co-authored-by: silverwind <[email protected]>
Reviewed-on: https://gitea.com/gitea/runner/pulls/1398
Reviewed-by: silverwind <[email protected]>
Co-authored-by: bircni <[email protected]>

act/artifactcache/handler.go

@@ -104,7 +104,9 @@ type Handler struct {
 	stopSync context.CancelFunc
 	syncing  sync.WaitGroup
 
+	outboundIPMu      sync.RWMutex
 	outboundIP        string
+	detectOutboundIP  func() net.IP // nil when the address is configured
 	instanceTransport http.RoundTripper
 
 	// internalSecret guards /_internal/{register,revoke}. When set, a remote
@@ -158,6 +160,7 @@ func StartHandler(opts Options) (*Handler, error) {
 		policy:            opts.Policy.withDefaults(),
 		freeDisk:          disk.FreeBytes,
 		dbLock:            make(chan struct{}, 1),
+		outboundIP:        opts.OutboundIP,
 		instanceTransport: opts.InstanceTransport,
 	}
 
@@ -167,12 +170,11 @@ func StartHandler(opts Options) (*Handler, error) {
 	logger = logger.With("module", "artifactcache")
 	h.logger = logger
 
-	if opts.OutboundIP != "" {
-		h.outboundIP = opts.OutboundIP
-	} else if ip := common.GetOutboundIP(); ip == nil {
-		return nil, errors.New("unable to determine outbound IP address")
-	} else {
-		h.outboundIP = ip.String()
+	if opts.OutboundIP == "" {
+		h.detectOutboundIP = common.GetOutboundIP
+		if h.RefreshExternalURL() == "" {
+			logger.Warn("unable to determine an outbound IP address for the cache server yet, set cache.host to configure one")
+		}
 	}
 
 	if opts.Upstream != "" {
@@ -304,11 +306,36 @@ func (h *Handler) cacheForwarder(upstream string) (http.Handler, error) {
 	}), nil
 }
 
+// ExternalURL is "" while no address is known.
 func (h *Handler) ExternalURL() string {
-	// TODO: make the external url configurable if necessary
+	if h == nil {
+		return ""
+	}
+	h.outboundIPMu.RLock()
+	defer h.outboundIPMu.RUnlock()
+	if h.outboundIP == "" {
+		return ""
+	}
 	return "http://" + net.JoinHostPort(h.outboundIP, strconv.Itoa(h.port))
 }
 
+// RefreshExternalURL re-detects an unconfigured address, which can change after startup.
+func (h *Handler) RefreshExternalURL() string {
+	if h != nil && h.detectOutboundIP != nil {
+		if ip := h.detectOutboundIP(); ip != nil {
+			h.outboundIPMu.Lock()
+			if address := ip.String(); address != h.outboundIP {
+				h.outboundIP = address
+				if ip.To4() == nil {
+					h.logger.Warnf("detected IPv6 address %s for the cache server, jobs on IPv4-only networks cannot reach it, set cache.host if they fail to", address)
+				}
+			}
+			h.outboundIPMu.Unlock()
+		}
+	}
+	return h.ExternalURL()
+}
+
 func (h *Handler) baseURL(cred JobCredential) string {
 	if base := strings.TrimRight(cred.PublicURL, "/"); base != "" {
 		return base

act/artifactcache/handler_test.go

@@ -11,6 +11,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"net"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -22,6 +23,8 @@ import (
 	"testing/synctest"
 	"time"
 
+	"gitea.com/gitea/runner/act/common/logging/logtest"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.etcd.io/bbolt"
@@ -50,7 +53,23 @@ func signArtifactURL(h *Handler, id int64) string {
 
 func TestHandler_ExternalURL(t *testing.T) {
 	t.Parallel()
-	assert.Equal(t, "http://[2001:db8::1]:8080", (&Handler{outboundIP: "2001:db8::1", port: 8080}).ExternalURL())
+	assert.Empty(t, (*Handler)(nil).RefreshExternalURL())
+	assert.Equal(t, "http://cache.example:8080", (&Handler{outboundIP: "cache.example", port: 8080}).RefreshExternalURL())
+
+	logger, logs := logtest.NewLogger()
+	var ip net.IP
+	handler := &Handler{port: 8080, logger: logger, detectOutboundIP: func() net.IP { return ip }}
+	assert.Empty(t, handler.RefreshExternalURL())
+	ip = net.ParseIP("2001:db8::1")
+	assert.Equal(t, "http://[2001:db8::1]:8080", handler.RefreshExternalURL())
+	assert.Equal(t, "http://[2001:db8::1]:8080", handler.RefreshExternalURL())
+	ip = net.ParseIP("192.0.2.1")
+	assert.Equal(t, "http://[2001:db8::1]:8080", handler.ExternalURL())
+	assert.Equal(t, "http://192.0.2.1:8080", handler.RefreshExternalURL())
+	ip = nil
+	assert.Equal(t, "http://192.0.2.1:8080", handler.RefreshExternalURL())
+	require.Len(t, logs.AllRecords(), 1)
+	assert.Contains(t, logs.LastRecord().Message, "IPv6")
 }
 
 func TestHandler_Upstream(t *testing.T) {

act/common/outbound_ip.go

@@ -6,6 +6,7 @@ package common
 
 import (
 	"net"
+	"slices"
 	"sort"
 	"strings"
 )
@@ -55,19 +56,22 @@ func GetOutboundIP() net.IP {
 }
 
 func preferredIP(ips []interfaceIP) net.IP {
+	ips = slices.DeleteFunc(ips, func(ip interfaceIP) bool {
+		return len(ip.Name) == 15 && strings.HasPrefix(ip.Name, "br-") && strings.Trim(ip.Name[3:], "0123456789abcdef") == "" // a Docker network's bridge, removed with the network
+	})
 	if len(ips) == 0 {
 		return nil
 	}
 	sort.Slice(ips, func(i, j int) bool {
 		first, second := ips[i], ips[j]
 
-		// ethernet is preferred
-		if vi, vj := strings.HasPrefix(first.Name, "e"), strings.HasPrefix(second.Name, "e"); vi != vj {
+		// IPv4 is preferred, as job networks are IPv4-only by default
+		if vi, vj := first.To4() != nil, second.To4() != nil; vi != vj {
 			return vi
 		}
 
-		// IPv4 is preferred
-		if vi, vj := first.To4() != nil, second.To4() != nil; vi != vj {
+		// ethernet is preferred
+		if vi, vj := strings.HasPrefix(first.Name, "e"), strings.HasPrefix(second.Name, "e"); vi != vj {
 			return vi
 		}
 

act/common/outbound_ip_test.go

@@ -13,8 +13,9 @@ import (
 func TestPreferredIP(t *testing.T) {
 	assert.Nil(t, preferredIP(nil))
 	assert.Equal(t, net.ParseIP("2001:db8::1"), preferredIP([]interfaceIP{{IP: net.ParseIP("2001:db8::1")}}))
-	assert.Equal(t, net.ParseIP("2001:db8::1"), preferredIP([]interfaceIP{
+	assert.Equal(t, net.ParseIP("192.0.2.1"), preferredIP([]interfaceIP{
 		{IP: net.ParseIP("192.0.2.1"), Name: "wlan0"},
 		{IP: net.ParseIP("2001:db8::1"), Name: "eth0"},
+		{IP: net.ParseIP("172.18.0.1"), Name: "br-0123456789ab"},
 	}))
 }

internal/app/cmd/exec.go

@@ -188,10 +188,14 @@ func (i *executeArgs) eventJSON() (string, error) {
 func (i *executeArgs) LoadEnvs() map[string]string {
 	envs := parseKVAndFile(i.envs, i.Envfile())
 
-	envs["ACTIONS_CACHE_URL"] = i.cacheHandler.ExternalURL() + "/"
+	cacheURL := i.cacheHandler.ExternalURL()
+	if cacheURL == "" {
+		return envs
+	}
+	envs["ACTIONS_CACHE_URL"] = cacheURL + "/"
 	// The same server answers cache v2, which docker buildx reads from the results origin alone.
 	if envs["ACTIONS_RESULTS_URL"] == "" {
-		envs["ACTIONS_RESULTS_URL"] = cmp.Or(os.Getenv("ACTIONS_RESULTS_URL"), i.cacheHandler.ExternalURL())
+		envs["ACTIONS_RESULTS_URL"] = cmp.Or(os.Getenv("ACTIONS_RESULTS_URL"), cacheURL)
 	}
 	envs[runner.CacheServiceV2Env] = "true"
 

internal/app/run/proxy.go

@@ -55,7 +55,7 @@ func JobProxyEnv(jobEnvs map[string]string, cacheURL string, serviceNames []stri
 	setPair("https_proxy", "HTTPS_PROXY", cfg.HTTPSProxy)
 
 	// no_proxy is merged rather than replaced: the hosts above are structural, and an
-	// operator cannot list the cache server's startup-assigned address in advance.
+	// operator cannot list the cache server's detected address in advance.
 	noProxy := appendNoProxy(cfg.NoProxy, direct...)
 	for _, name := range []string{"no_proxy", "NO_PROXY"} {
 		if existing, ok := jobEnvs[name]; ok {

internal/app/run/runner.go

@@ -73,7 +73,7 @@ type Runner struct {
 	cacheHandler *artifactcache.Handler
 	capabilities string
 
-	isolatedCacheContainer func() string
+	isolatedCacheContainer func(cacheURL string) string
 	instanceTransport      http.RoundTripper
 
 	runningTasks            sync.Map
@@ -118,7 +118,6 @@ func NewRunner(cfg *config.Config, reg *config.Registration, cli client.Client)
 			// go on
 		} else {
 			cacheHandler = handler
-			envs["ACTIONS_CACHE_URL"] = handler.ExternalURL() + "/"
 		}
 	}
 
@@ -141,7 +140,7 @@ func NewRunner(cfg *config.Config, reg *config.Registration, cli client.Client)
 		now:            time.Now,
 		runHealthCheck: executeHealthCheck,
 	}
-	runner.isolatedCacheContainer = sync.OnceValue(runner.detectIsolatedCacheContainer)
+	runner.isolatedCacheContainer = lastResult(runner.detectIsolatedCacheContainer)
 	runner.instanceTransport = instanceTransport
 	return runner
 }
@@ -196,7 +195,7 @@ func (r *Runner) cleanupOrphanDockerResources(ctx context.Context) {
 		return
 	}
 	cutoff := r.now().Add(-orphanDockerResourceAge)
-	if err := removeOrphanNetworks(ctx, r.uuid, r.isolatedCacheContainer(), cutoff); err != nil {
+	if err := removeOrphanNetworks(ctx, r.uuid, r.isolatedCacheContainer(r.cacheHandler.ExternalURL()), cutoff); err != nil {
 		logging.Warnf("failed to clean up networks left behind by earlier jobs: %v", err)
 	}
 	if err := removeOrphanJobVolumes(ctx, r.uuid, cutoff); err != nil {
@@ -485,7 +484,7 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report.
 		return fallbackPlatform()
 	}
 	platform := platformPicker(job.RunsOn())
-	cacheURL, cacheContainer := r.cacheForJob(job, platform)
+	cacheURL, cacheContainer := r.cacheForJob(job, platform, reporter.Logf)
 	if cacheURL != "" {
 		envs["ACTIONS_CACHE_URL"] = cacheURL + "/"
 	}
@@ -664,11 +663,24 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report.
 	return execErr
 }
 
-func (r *Runner) builtInCacheURL() string {
-	if r.cacheHandler == nil {
-		return ""
+func lastResult(compute func(key string) (string, bool)) func(key string) string {
+	var (
+		mu      sync.Mutex
+		lastKey string
+		result  string
+	)
+	return func(key string) string {
+		mu.Lock()
+		defer mu.Unlock()
+		if key == lastKey {
+			return result
+		}
+		value, ok := compute(key)
+		if ok {
+			lastKey, result = key, value
+		}
+		return value
 	}
-	return r.envs["ACTIONS_CACHE_URL"]
 }
 
 func (r *Runner) setResultsService(envs map[string]string, resultsURL string) {
@@ -699,7 +711,7 @@ 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 string, cred artifactcache.JobCredential, reporter *report.Reporter) (func(), string) {
-	if token == "" || r.cacheHandler == nil {
+	if token == "" || cred.PublicURL == "" {
 		return func() {}, ""
 	}
 	cred.Results = r.envs["ACTIONS_RESULTS_URL"] // the instance as the job would reach it
@@ -930,35 +942,39 @@ func warnIgnoredCacheSecret(cfg *config.Config) {
 }
 
 // cacheForJob returns the cache address for the job, and the container to attach to its network to reach it there.
-func (r *Runner) cacheForJob(job *model.Job, platform string) (string, string) {
+func (r *Runner) cacheForJob(job *model.Job, platform string, logf func(string, ...any)) (string, string) {
+	cacheURL := r.cacheHandler.RefreshExternalURL()
 	jobContainer := job.Container()
-	cacheContainer := ""
-	if !strings.HasPrefix(platform, labels.SchemeKubernetes+"://") && (jobContainer != nil && jobContainer.Image != "" || platform != labels.SelfHostedPlatform) {
-		cacheContainer = r.isolatedCacheContainer()
+	if cacheURL == "" || strings.HasPrefix(platform, labels.SchemeKubernetes+"://") || (jobContainer == nil || jobContainer.Image == "") && platform == labels.SelfHostedPlatform {
+		return cacheURL, ""
 	}
-	if cacheContainer == "" {
-		return strings.TrimSuffix(r.builtInCacheURL(), "/"), ""
+	if cacheContainer := r.isolatedCacheContainer(cacheURL); cacheContainer != "" {
+		isolatedURL, _ := url.Parse(cacheURL)
+		isolatedURL.Host = net.JoinHostPort(cacheContainer, isolatedURL.Port())
+		return isolatedURL.String(), cacheContainer
 	}
-	cacheURL, _ := url.Parse(r.cacheHandler.ExternalURL())
-	cacheURL.Host = net.JoinHostPort(cacheContainer, cacheURL.Port())
-	return cacheURL.String(), cacheContainer
+	network, enableIPv6 := r.cfg.Container.Network, r.cfg.Container.NetworkCreateOptions.EnableIPv6
+	if addr, err := netip.ParseAddr(hostOf(cacheURL)); err == nil && addr.Is6() && r.cfg.Cache.Host == "" &&
+		network != "host" && (network != "" || enableIPv6 == nil || !*enableIPv6) {
+		logf("::warning::%s", runner.EscapeCommandData("The runner's cache server is at the IPv6 address "+addr.String()+
+			", so cache and artifact steps fail if this job's network has no IPv6. Set cache.host to an address the job reaches, or enable IPv6 on the job network."))
+	}
+	return cacheURL, ""
 }
 
-func (r *Runner) detectIsolatedCacheContainer() string {
-	if r.cacheHandler == nil {
-		return ""
-	}
-	addr, err := netip.ParseAddr(hostOf(r.cacheHandler.ExternalURL()))
+func (r *Runner) detectIsolatedCacheContainer(cacheURL string) (string, bool) {
+	addr, err := netip.ParseAddr(hostOf(cacheURL))
 	if err != nil {
-		return ""
+		return "", true
 	}
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 	defer cancel()
 	cacheContainer, err := container.IsolatedCacheContainer(ctx, addr, r.cfg.Container.Network)
 	if err != nil {
 		logging.Warnf("cannot check whether jobs reach the cache server: %v", err)
+		return "", false
 	}
-	return cacheContainer
+	return cacheContainer, true
 }
 
 // taskLogFields returns key/value pairs for logging.With.

internal/app/run/runner_cache_test.go

@@ -155,12 +155,11 @@ func TestRunner_ExternalCacheServer_RegisterRevoke(t *testing.T) {
 	}, &config.Registration{}, cli)
 	t.Cleanup(func() { _ = r.Close() })
 	require.NotNil(t, r.cacheHandler)
-	require.Equal(t, r.cacheHandler.ExternalURL()+"/", r.envs["ACTIONS_CACHE_URL"])
 
 	const publicURL = "http://a1b2c3d4e5f6:8088"
 	token := "external-task-token"
 	repo := "owner/repoX"
-	base := r.envs["ACTIONS_CACHE_URL"] + "_apis/artifactcache"
+	base := r.cacheHandler.ExternalURL() + "/_apis/artifactcache"
 	probe := func() int {
 		resp := cacheRequest(t, token, http.MethodGet, base+"/cache?keys=k&version=v", "", "", "")
 		resp.Body.Close()
@@ -213,7 +212,7 @@ 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)
+	unregisterOther, _ := r.registerCacheForTask("external-other-token", artifactcache.JobCredential{Repo: repo, Ref: "refs/heads/other", PublicURL: publicURL}, nil)
 	defer unregisterOther()
 	resp = cacheRequest(t, "external-other-token", http.MethodGet, base+"/cache?keys=ext-key&version=v", "", "", "")
 	resp.Body.Close()

internal/app/run/runner_idle_cleanup_test.go

@@ -341,7 +341,7 @@ func TestRunnerOnIdleRemovesOrphanNetworks(t *testing.T) {
 	require.NoError(t, err)
 
 	r := &Runner{uuid: "runner-1", cfg: cfg, labels: labels.Labels{kubernetesLabel}, now: func() time.Time { return now }}
-	r.isolatedCacheContainer = func() string { return "a1b2c3d4e5f6" }
+	r.isolatedCacheContainer = func(_cacheURL string) string { return "a1b2c3d4e5f6" }
 	r.OnIdle(t.Context())
 	assert.Equal(t, []string{"runner-1"}, swept)
 	assert.Equal(t, swept, sweptVolumes)
@@ -353,7 +353,7 @@ func TestRunnerOnIdleRemovesOrphanNetworks(t *testing.T) {
 	dockerReachable = func(context.Context) bool { return false }
 	t.Cleanup(func() { dockerReachable = origDockerReachable })
 	hostOnly := &Runner{uuid: "runner-2", cfg: &config.Config{Runner: cfg.Runner}, now: func() time.Time { return now }}
-	hostOnly.isolatedCacheContainer = func() string { return "" }
+	hostOnly.isolatedCacheContainer = func(_cacheURL string) string { return "" }
 	hostOnly.OnIdle(t.Context())
 	assert.Equal(t, []string{"runner-1"}, swept)
 	assert.Equal(t, swept, sweptVolumes)

internal/app/run/runner_test.go

@@ -306,7 +306,7 @@ func TestNewRunnerLeavesProxyToTheTask(t *testing.T) {
 
 	require.NotContains(t, r.envs, "http_proxy")
 	require.NotContains(t, r.envs, "no_proxy")
-	proxyEnv := JobProxyEnv(r.envs, r.builtInCacheURL(), nil)
+	proxyEnv := JobProxyEnv(r.envs, r.cacheHandler.ExternalURL(), nil)
 	assert.Contains(t, proxyEnv["no_proxy"], "runner.local")
 	assert.NotContains(t, proxyEnv["no_proxy"], "cache.local")
 }
@@ -355,7 +355,7 @@ func TestNewRunnerCacheServiceV2(t *testing.T) {
 	defer resp.Body.Close()
 
 	assert.Equal(t, http.StatusOK, resp.StatusCode, "the advertised results service serves no cache service")
-	assert.Equal(t, r.envs["ACTIONS_CACHE_URL"], r.builtInCacheURL(), "the address only the runner knows is bypassed for the operator")
+	assert.NotContains(t, r.envs, "ACTIONS_CACHE_URL")
 
 	envs := r.cloneEnvs()
 	r.setResultsService(envs, resultsURL)
@@ -390,10 +390,10 @@ func TestNewRunnerCacheServiceV2(t *testing.T) {
 
 	workflow, err := model.ReadWorkflow(strings.NewReader(`jobs: {native: {runs-on: native}, linux: {runs-on: linux}, containerized: {runs-on: native, container: alpine}, empty: {runs-on: native, container: ""}}`))
 	require.NoError(t, err)
-	r.isolatedCacheContainer = func() string { return "a1b2c3d4e5f6" }
+	r.isolatedCacheContainer = func(_cacheURL string) string { return "a1b2c3d4e5f6" }
 	pickPlatform := func(runsOn []string) string { return map[string]string{"native": labels.SelfHostedPlatform}[runsOn[0]] }
 	for job, isolated := range map[string]bool{"native": false, "linux": true, "containerized": true, "empty": false} {
-		cacheURL, cacheContainer := r.cacheForJob(workflow.GetJob(job), pickPlatform(workflow.GetJob(job).RunsOn()))
+		cacheURL, cacheContainer := r.cacheForJob(workflow.GetJob(job), pickPlatform(workflow.GetJob(job).RunsOn()), t.Logf)
 		if isolated {
 			assert.Equal(t, "a1b2c3d4e5f6", cacheContainer, job)
 			assert.Equal(t, strings.Replace(r.cacheHandler.ExternalURL(), "127.0.0.1", cacheContainer, 1), cacheURL, job)
@@ -404,6 +404,41 @@ func TestNewRunnerCacheServiceV2(t *testing.T) {
 	}
 }
 
+func TestCacheForJobIPv6WarningAndIsolationRecheck(t *testing.T) {
+	handler, err := artifactcache.StartHandler(artifactcache.Options{OutboundIP: "2001:db8::1", Upstream: "http://cache.invalid"})
+	require.NoError(t, err)
+	defer handler.Close()
+	var checked []string
+	r := &Runner{cfg: &config.Config{}, cacheHandler: handler, isolatedCacheContainer: lastResult(func(cacheURL string) (string, bool) {
+		checked = append(checked, cacheURL)
+		return "", cacheURL != "http://unreachable"
+	})}
+	warnings := 0
+	logf := func(_format string, _args ...any) { warnings++ }
+	workflow, err := model.ReadWorkflow(strings.NewReader(`jobs: {native: {runs-on: native}, containerized: {runs-on: native, container: alpine}}`))
+	require.NoError(t, err)
+	for _, job := range []string{"native", "containerized", "containerized"} {
+		cacheURL, _ := r.cacheForJob(workflow.GetJob(job), labels.SelfHostedPlatform, logf)
+		assert.Equal(t, handler.ExternalURL(), cacheURL)
+	}
+	assert.Equal(t, 2, warnings)
+	assert.Equal(t, []string{handler.ExternalURL()}, checked)
+
+	r.cfg.Container.NetworkCreateOptions.EnableIPv6 = new(true)
+	r.cacheForJob(workflow.GetJob("containerized"), labels.SelfHostedPlatform, logf)
+	assert.Equal(t, 2, warnings)
+	r.cfg.Container.Network = "bridge"
+	r.cacheForJob(workflow.GetJob("containerized"), labels.SelfHostedPlatform, logf)
+	assert.Equal(t, 3, warnings)
+	r.cfg.Cache.Host = "2001:db8::1"
+	r.cacheForJob(workflow.GetJob("containerized"), labels.SelfHostedPlatform, logf)
+	assert.Equal(t, 3, warnings)
+
+	r.isolatedCacheContainer("http://unreachable")
+	r.isolatedCacheContainer("http://unreachable")
+	assert.Len(t, checked, 3)
+}
+
 func TestRestoreCalledWorkflowContext(t *testing.T) {
 	newTask := func(workflowCall map[string]any) *runnerv1.Task {
 		values := map[string]any{